diff --git a/.coderabbit.yaml b/.coderabbit.yaml index cbac4179..2e6c5b59 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -43,6 +43,7 @@ reviews: # PRs targeting `dev` (not only the GitHub default branch, e.g. `main`) get automatic reviews. base_branches: - "^dev$" + - "^main$" path_instructions: - path: "src/specfact_cli/**/*.py" instructions: | diff --git a/.github/workflows/pr-orchestrator.yml b/.github/workflows/pr-orchestrator.yml index be22944a..d75d4130 100644 --- a/.github/workflows/pr-orchestrator.yml +++ b/.github/workflows/pr-orchestrator.yml @@ -5,16 +5,8 @@ name: PR Orchestrator - SpecFact CLI on: pull_request: branches: [main, dev] - paths-ignore: - - "**/*.md" - - "**/*.mdc" - - "docs/**" push: branches: [main, dev] - paths-ignore: - - "**/*.md" - - "**/*.mdc" - - "docs/**" workflow_dispatch: concurrency: @@ -33,6 +25,7 @@ jobs: runs-on: ubuntu-latest outputs: code_changed: ${{ steps.out.outputs.code_changed }} + workflow_changed: ${{ steps.out.outputs.workflow_changed }} skip_tests_dev_to_main: ${{ steps.out.outputs.skip_tests_dev_to_main }} steps: - uses: actions/checkout@v4 @@ -47,22 +40,52 @@ jobs: - '!**/*.md' - '!**/*.mdc' - '!docs/**' + - '!.github/workflows/**' + workflow: + - '.github/workflows/**' + - 'scripts/run_actionlint.sh' + - 'scripts/yaml-tools.sh' - id: out env: EVENT_NAME: ${{ github.event_name }} PR_BASE_REF: ${{ github.event.pull_request.base.ref }} PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | + PR_BASE_REF="${PR_BASE_REF:-}" + PR_HEAD_REF="${PR_HEAD_REF:-}" + PR_BASE_SHA="${PR_BASE_SHA:-}" + PR_HEAD_SHA="${PR_HEAD_SHA:-}" if [ "$EVENT_NAME" = "workflow_dispatch" ]; then echo "code_changed=true" >> "$GITHUB_OUTPUT" + echo "workflow_changed=true" >> "$GITHUB_OUTPUT" else echo "code_changed=${{ steps.filter.outputs.code }}" >> "$GITHUB_OUTPUT" + echo "workflow_changed=${{ steps.filter.outputs.workflow }}" >> "$GITHUB_OUTPUT" fi + SKIP_TESTS=false if [ "$EVENT_NAME" = "pull_request" ] && [ "$PR_BASE_REF" = "main" ] && [ "$PR_HEAD_REF" = "dev" ]; then - echo "skip_tests_dev_to_main=true" >> "$GITHUB_OUTPUT" - else - echo "skip_tests_dev_to_main=false" >> "$GITHUB_OUTPUT" + PR_BASE_REV="" + PR_HEAD_REV="" + MERGE_BASE="" + if [ -n "$PR_BASE_SHA" ]; then + PR_BASE_REV=$(git rev-parse "${PR_BASE_SHA}^{commit}" 2>/dev/null || true) + fi + if [ -n "$PR_HEAD_SHA" ]; then + PR_HEAD_REV=$(git rev-parse "${PR_HEAD_SHA}^{commit}" 2>/dev/null || true) + fi + if [ -n "$PR_BASE_REV" ] && [ -n "$PR_HEAD_REV" ]; then + MERGE_BASE=$(git merge-base "$PR_BASE_REV" "$PR_HEAD_REV" 2>/dev/null || true) + fi + if [ -n "$PR_BASE_REV" ] && [ -n "$PR_HEAD_REV" ] && [ "$PR_BASE_REV" = "$PR_HEAD_REV" ] && [ "$MERGE_BASE" = "$PR_HEAD_REV" ]; then + SKIP_TESTS=true + echo "✅ Proven release parity between dev and main; skipping duplicate validation." + else + echo "ℹ️ Release parity proof unavailable for dev→main PR; running required validation set." + fi fi + echo "skip_tests_dev_to_main=${SKIP_TESTS}" >> "$GITHUB_OUTPUT" verify-module-signatures: name: Verify Module Signatures @@ -99,6 +122,49 @@ jobs: python scripts/verify-modules-signature.py --require-signature --enforce-version-bump --payload-from-filesystem fi + workflow-lint: + name: Workflow Lint + needs: [changes] + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + + - name: Skip when no workflow changes are present + if: needs.changes.outputs.workflow_changed != 'true' && github.event_name != 'workflow_dispatch' + run: | + echo "✅ No workflow changes detected; skipping workflow lint." + + - name: Set up Python 3.12 + if: needs.changes.outputs.workflow_changed == 'true' || github.event_name == 'workflow_dispatch' + uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: "pip" + cache-dependency-path: | + pyproject.toml + + - name: Set up Go for actionlint + if: needs.changes.outputs.workflow_changed == 'true' || github.event_name == 'workflow_dispatch' + uses: actions/setup-go@v5 + with: + go-version: "1.24" + + - name: Install lint dependencies + if: needs.changes.outputs.workflow_changed == 'true' || github.event_name == 'workflow_dispatch' + run: | + python -m pip install --upgrade pip + pip install "hatch" "virtualenv<21" + go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.11 + echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" + + - name: Run workflow lint + if: needs.changes.outputs.workflow_changed == 'true' || github.event_name == 'workflow_dispatch' + run: | + echo "🔍 Running actionlint for workflow changes..." + hatch run lint-workflows + tests: name: Tests (Python 3.12) needs: [changes, verify-module-signatures] @@ -219,7 +285,7 @@ jobs: name: Compatibility (Python 3.11) runs-on: ubuntu-latest needs: [changes, verify-module-signatures] - if: needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' permissions: contents: read steps: @@ -258,9 +324,10 @@ jobs: mkdir -p logs/compat-py311 COMPAT_LOG="logs/compat-py311/compat_$(date -u +%Y%m%d_%H%M%S).log" { - hatch -e hatch-test.py3.11 run run -- -r fEw tests/unit tests/integration || echo "⚠️ Some tests failed (advisory)" + hatch -e hatch-test.py3.11 run run -- -r fEw tests/unit tests/integration hatch -e hatch-test.py3.11 run xml || true } 2>&1 | tee "$COMPAT_LOG" + exit "${PIPESTATUS[0]:-$?}" - name: Upload compat-py311 logs if: always() uses: actions/upload-artifact@v4 @@ -273,7 +340,7 @@ jobs: name: Contract-First CI runs-on: ubuntu-latest needs: [changes, verify-module-signatures] - if: needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' permissions: contents: read steps: @@ -313,8 +380,9 @@ jobs: run: | echo "🔍 Validating runtime contracts..." REPRO_LOG="logs/repro/repro_$(date -u +%Y%m%d_%H%M%S).log" - echo "Running specfact repro with required CrossHair... (log: $REPRO_LOG)" - hatch run specfact repro --verbose --crosshair-required --budget 120 2>&1 | tee "$REPRO_LOG" || echo "SpecFact repro found issues" + echo "Running contract-first validation with required CrossHair... (log: $REPRO_LOG)" + hatch run contract-test 2>&1 | tee "$REPRO_LOG" + exit "${PIPESTATUS[0]:-$?}" - name: Upload repro logs if: always() uses: actions/upload-artifact@v4 @@ -334,7 +402,7 @@ jobs: name: CLI Command Validation runs-on: ubuntu-latest needs: [changes, verify-module-signatures] - if: needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' permissions: contents: read steps: @@ -353,8 +421,8 @@ jobs: - name: Validate CLI commands run: | echo "🔍 Validating CLI commands..." - specfact --help || echo "⚠️ CLI not yet fully implemented" - echo "✅ CLI validation complete (advisory)" + specfact --help + echo "✅ CLI validation complete" quality-gates: name: Quality Gates (Advisory) @@ -412,7 +480,7 @@ jobs: name: Type Checking (basedpyright) runs-on: ubuntu-latest needs: [changes, verify-module-signatures] - if: needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' permissions: contents: read steps: @@ -447,7 +515,7 @@ jobs: name: Linting (ruff, pylint) runs-on: ubuntu-latest needs: [changes, verify-module-signatures] - if: needs.changes.outputs.skip_tests_dev_to_main != 'true' + if: needs.changes.outputs.code_changed == 'true' && needs.changes.outputs.skip_tests_dev_to_main != 'true' permissions: contents: read steps: @@ -477,7 +545,8 @@ jobs: python -m basedpyright --pythonpath "$(python -c 'import sys; print(sys.executable)')" ruff check . pylint src tests tools - } 2>&1 | tee "$LINT_LOG" || echo "⚠️ Linting incomplete" + } 2>&1 | tee "$LINT_LOG" + exit "${PIPESTATUS[0]:-$?}" - name: Upload lint logs if: always() uses: actions/upload-artifact@v4 diff --git a/.github/workflows/sign-modules.yml b/.github/workflows/sign-modules.yml index 5ea3f96f..29ab94c7 100644 --- a/.github/workflows/sign-modules.yml +++ b/.github/workflows/sign-modules.yml @@ -25,7 +25,7 @@ on: jobs: verify: - name: Verify module signatures + name: Verify Module Signatures runs-on: ubuntu-latest permissions: contents: read diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ef637d53..3e1d1bd2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,10 +1,10 @@ repos: - repo: local hooks: - - id: verify-module-signatures - name: Verify module signatures and version bumps - entry: hatch run ./scripts/verify-modules-signature.py --require-signature --enforce-version-bump - language: system + - id: specfact-smart-checks + name: SpecFact smart pre-commit checks + entry: scripts/pre-commit-smart-checks.sh + language: script pass_filenames: false always_run: true - id: check-doc-frontmatter @@ -14,10 +14,3 @@ repos: files: ^(docs/.*\.md|docs/\.doc-frontmatter-enforced|USAGE-FAQ\.md)$ pass_filenames: false always_run: false - - id: specfact-code-review-gate - name: Run code review gate on staged Python files - entry: hatch run python scripts/pre_commit_code_review.py - language: system - files: \.pyi?$ - # Show summary + copy-paste lines on success; pre-commit hides hook output otherwise. - verbose: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 485a2cea..c9d282fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ + # Changelog The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), @@ -508,7 +509,6 @@ All notable changes to this project will be documented in this file. - Bundle ID candidate derivation no longer falls back to the manifest filename stem (`bundle.yaml` -> `bundle`), preventing false rejection of valid explicit `bundle:` tags. - OpenSpec change order/archive tracking was synchronized for Wave 1 closure (`verification-01-wave1-delta-closure`) and related archived status markers. ---- ## [0.34.0] - 2026-02-18 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d8d5bcd1..bedc6241 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,9 +96,19 @@ hatch run lint hatch run contract-test-full ``` -The repo-owned pre-commit flow now also runs `specfact code review run` on -staged Python files and blocks commits only when the review verdict is -blocking. +The supported local hook path is the repo-owned smart-check wrapper installed by the commands +above. It keeps local semantics aligned with CI: + +- Merge-blocking local gates: module signature verification, formatter safety, Markdown/YAML checks, + workflow lint for staged workflow changes, and contract-test fast feedback when code changes. +- Review gate behavior: `specfact code review run` reviews staged Python files and blocks the + commit only on `FAIL`. `PASS_WITH_ADVISORY` remains green but still prints the JSON report path for + remediation in Copilot/Cursor. +- Advisory review surfaces: CodeRabbit and other PR review comments remain advisory unless a branch + protection rule explicitly promotes a check to required. +- Workflow linting requires `actionlint` on `PATH` or a Docker-enabled environment. CI installs a + pinned `actionlint` release explicitly; local contributors should install it globally, for example + with `go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.11`. ## Contributor License Agreement (CLA) @@ -199,6 +209,7 @@ The repository README, `docs/index.md`, and other first-contact surfaces must pr first-contact story. When editing those surfaces, make sure a new visitor can quickly answer: + - **What is SpecFact?** - **Why does it exist?** - **Why should I use it?** @@ -206,6 +217,7 @@ When editing those surfaces, make sure a new visitor can quickly answer: - **How do I get started?** Keep the hierarchy in this order: + 1. Product identity 2. Why it exists 3. User value diff --git a/docs/contributing/docs-sync.md b/docs/contributing/docs-sync.md index 6d25ec98..c79ee0e0 100644 --- a/docs/contributing/docs-sync.md +++ b/docs/contributing/docs-sync.md @@ -15,7 +15,7 @@ exempt: false exempt_reason: "" --- -# Documentation ownership and frontmatter +## Documentation ownership and frontmatter Core documentation uses **YAML frontmatter** for Jekyll (layout, title, permalink) and for **ownership** fields that drive the `scripts/check_doc_frontmatter.py` checker. @@ -46,6 +46,10 @@ The enforced-path file accepts glob patterns matched against repo-relative Markd `docs/` and root docs such as `USAGE-FAQ.md`; blank lines and lines starting with `#` are ignored by the checker. +Docs-only PRs rely on the dedicated `Docs Review` workflow as the required documentation gate. Its +cross-site link check remains advisory because the published site can lag deployment, while +frontmatter validation and docs test suites remain merge-blocking for docs-owned changes. + ## Troubleshooting - **Missing `doc_owner`**: add the field and a sensible `tracks` list for the code or specs this page describes. diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index 97aaa120..c3dbffb0 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -5,7 +5,7 @@ description: Install and use the official specfact-code-review module scaffold. permalink: /modules/code-review/ --- -# Code Review Module +## Code Review Module The `nold-ai/specfact-code-review` module extends `specfact code` with a governed `review` subgroup for structured review execution, scoring, and reporting. @@ -102,25 +102,27 @@ The scaffolded `ReviewReport` envelope carries these fields: ## Pre-Commit Review Gate -This repository wires `specfact code review run` into pre-commit before a -commit is considered green. +This repository wires `specfact code review run` into the smart pre-commit wrapper before a commit +is considered green. -The local hook entry lives in `.pre-commit-config.yaml`: +The supported local hook entry lives in `.pre-commit-config.yaml`: ```yaml repos: - repo: local hooks: - - id: specfact-code-review-gate - name: Run code review gate on staged Python files - entry: hatch run python scripts/pre_commit_code_review.py - language: system - files: \.pyi?$ - # Needed so you still see hook output when the gate passes (pre-commit hides it otherwise). - verbose: true + - id: specfact-smart-checks + name: SpecFact smart pre-commit checks + entry: scripts/pre-commit-smart-checks.sh + language: script + pass_filenames: false + always_run: true ``` -The helper script scopes the gate to staged Python files only and then runs: +The wrapper calls `scripts/pre_commit_code_review.py` only when staged Python files are present, +alongside the repo's other local required gates (module signatures, formatter safety, Markdown/YAML +checks, workflow lint when relevant, and contract-test fast feedback). The review helper itself +then runs: ```bash specfact code review run --json --out .specfact/code-review.json @@ -158,6 +160,12 @@ Commit behavior: - `PASS_WITH_ADVISORY` keeps the commit green - `FAIL` blocks the commit +Repository gate taxonomy: + +- Local smart-check wrapper: merge-blocking for its enforced local checks. +- `specfact code review run`: advisory unless it returns `FAIL`; `PASS_WITH_ADVISORY` stays commit-green. +- CodeRabbit review comments/status: advisory review assistance, not a merge-blocking branch-protection gate by themselves. + To install the repo-owned hook flow: ```bash diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index 30d61f21..2d4d4b80 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -61,6 +61,7 @@ Only changes that are **archived**, shown as **✓ Complete** by `openspec list` | ✅ code-review-08-review-run-integration | archived 2026-03-17 | | ✅ code-review-09-f4-automation-upgrade | archived 2026-03-17 | | ✅ doc-frontmatter-schema | archived 2026-03-29 | +| ✅ ci-02-trustworthy-green-checks | implemented 2026-03-30; archive pending | ### Pending @@ -170,7 +171,7 @@ Cross-repo dependency: `docs-07-core-handoff-conversion` depends on `specfact-cl | Module | Order | Change folder | GitHub # | Blocked by | |--------|-------|----------------|----------|------------| | — | — | ✅ ci-01 implemented 2026-02-16 (see Implemented above) | — | — | -| ci | 02 | ci-02-trustworthy-green-checks | [#465](https://github.com/nold-ai/specfact-cli/issues/465) | ci-01 ✅; code-review-08 ✅; Parent Feature: [#406](https://github.com/nold-ai/specfact-cli/issues/406) | +| ci | 02 | ✅ ci-02-trustworthy-green-checks (implemented 2026-03-30; archive pending) | [#465](https://github.com/nold-ai/specfact-cli/issues/465) | ci-01 ✅; code-review-08 ✅; Parent Feature: [#406](https://github.com/nold-ai/specfact-cli/issues/406) | ### Packaging and distribution diff --git a/openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md b/openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md index 82ec37e0..303e97db 100644 --- a/openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md +++ b/openspec/changes/ci-02-trustworthy-green-checks/TDD_EVIDENCE.md @@ -50,3 +50,110 @@ python3 -m pytest tests/integration/scripts/test_doc_frontmatter/test_integratio - Helper fixtures and tests now assert the implemented required fields and valid owner behavior. **Status:** ✅ Remediation implemented and verified. + +## Pre-Implementation Test Failure (Expected) - Workflow Policy + +### Test Run: 2026-03-30 - Trustworthy green-check workflow policy + +**Command:** + +```bash +cd /home/dom/git/nold-ai/specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run pytest tests/unit/workflows/test_trustworthy_green_checks.py -q +``` + +**Result:** ✅ 6 tests failed as expected before implementation. + +**Failure summary:** + +- `pr-orchestrator.yml` still used workflow-level `paths-ignore`, so required checks disappeared on + docs-only follow-up commits instead of reporting a status on the new head SHA. +- Required jobs still swallowed failures (`compat-py311`, `contract-first-ci`, `cli-validation`, + and `linting`) or did not yet expose a dedicated required workflow-lint job. +- `dev -> main` release PRs still used a branch-only fast path without any commit-parity proof. +- `.pre-commit-config.yaml` did not expose the supported smart-check wrapper, and `.coderabbit.yaml` + still auto-reviewed only `dev`-targeted PRs. +- The dedicated `sign-modules.yml` workflow emitted a different required check name casing than the + orchestrator. + +**Status:** ✅ TDD workflow confirmed for CI/check semantics hardening. + +## Pre-Implementation Test Failure (Expected) - Actionlint Fallback + +### Test Run: 2026-03-30 - Legacy actionlint wrapper fallback + +**Command:** + +```bash +cd /home/dom/git/nold-ai/specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run pytest tests/unit/workflows/test_trustworthy_green_checks.py -q -k actionlint_runner +``` + +**Result:** ✅ 1 test failed as expected before implementation. + +**Failure summary:** + +- `scripts/run_actionlint.sh` returned success immediately after `docker run`, so a missing Docker + daemon could silently bypass workflow lint instead of falling back to the local binary path. + +**Status:** ✅ TDD workflow confirmed for legacy actionlint fallback hardening. + +## Post-Implementation Test Success - Remediation Slice + +### Test Run: 2026-03-30 - Trustworthy green-check remediation slice + +**Commands:** + +```bash +cd /home/dom/git/nold-ai/specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run pytest tests/unit/workflows/test_trustworthy_green_checks.py tests/unit/scripts/test_pre_commit_smart_checks_docs.py tests/unit/scripts/test_pre_commit_code_review.py tests/unit/scripts/test_doc_frontmatter/test_validation.py tests/integration/scripts/test_doc_frontmatter/test_integration.py -q +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run specfact code review run --json --out .specfact/code-review.json tests/unit/workflows/test_trustworthy_green_checks.py tests/unit/scripts/test_doc_frontmatter/test_validation.py +``` + +**Result:** ✅ Targeted tests passed and the changed-file SpecFact review reported `PASS` with `0` findings. + +**Passing summary:** + +- `pr-orchestrator.yml` now triggers on every PR head commit, uses explicit required versus + advisory gate naming, fails required jobs closed, and runs a dedicated workflow-lint gate. +- `dev -> main` release validation no longer uses a branch-only shortcut; the skip path now + requires an explicit commit-parity proof and otherwise reruns the required validation set. +- `.pre-commit-config.yaml` now exposes the supported smart-check wrapper and `.coderabbit.yaml` + covers both `dev` and `main` PR targets. +- `scripts/run_actionlint.sh` now falls back cleanly when Docker is installed but the daemon is not + reachable. +- Contributor/docs surfaces now distinguish merge-blocking versus advisory outputs, and the updated + doc-frontmatter validation tests cover malformed YAML diagnostics. + +**Status:** ✅ Remediation implemented and verified for the change surface. + +### Follow-up verification: 2026-03-30T22:42:42+02:00 - actionlint install strategy cleanup + +**Commands:** + +```bash +cd /home/dom/git/nold-ai/specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run pytest tests/unit/workflows/test_trustworthy_green_checks.py -q +HATCH_DATA_DIR=/tmp/hatch-data HATCH_CACHE_DIR=/tmp/hatch-cache VIRTUALENV_OVERRIDE_APP_DATA=/tmp/virtualenv-appdata \ + hatch run type-check +``` + +**Result:** ✅ Follow-up workflow tests passed and type-check remained green after removing repo-local +`actionlint` downloads. + +**Passing summary:** + +- `scripts/run_actionlint.sh` now prefers a globally installed `actionlint`, then Docker when the + daemon is reachable, and otherwise exits with explicit install guidance instead of downloading + into the repository tree. +- `scripts/yaml-tools.sh` now delegates workflow linting to the same runner, so local and CI paths + share a single source of truth. +- `pr-orchestrator.yml` now installs a pinned `actionlint` release in CI before running + `hatch run lint-workflows`, so GitHub Actions does not rely on Docker availability for this gate. + +**Status:** ✅ Actionlint installation strategy aligned with repo expectations and re-verified. diff --git a/openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md b/openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md index fc72b678..c94f4947 100644 --- a/openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md +++ b/openspec/changes/ci-02-trustworthy-green-checks/specs/trustworthy-green-checks/spec.md @@ -59,9 +59,9 @@ out-of-scope follow-up commits because the entire workflow was skipped by top-le ### Requirement: Required checks use canonical names across workflows -When a logical required gate is emitted by more than one workflow or has related dedicated and -orchestrated forms, the repository SHALL standardize on one canonical emitted check name for branch -protection and documentation. +The repository SHALL standardize on one canonical emitted check name for any logical required gate +that is emitted by more than one workflow or exposed through related dedicated and orchestrated +forms. #### Scenario: Signature validation exists in both orchestrator and dedicated workflow form @@ -71,7 +71,9 @@ protection and documentation. ### Requirement: Supported local pre-commit installation matches core CI gate semantics -The repository-supported pre-commit installation path SHALL enforce the same core gate semantics that CI relies on for changed files, rather than leaving stronger checks only in an optional wrapper unknown to standard contributors. +The repository-supported pre-commit installation path SHALL enforce the same core gate semantics +that CI relies on for changed files, rather than leaving stronger checks only in an optional +wrapper unknown to standard contributors. #### Scenario: Contributor installs the documented local hooks @@ -81,10 +83,23 @@ The repository-supported pre-commit installation path SHALL enforce the same cor ### Requirement: Automatic PR review coverage includes both active protected targets -Automatic repository review configuration SHALL cover pull requests targeting both `dev` and `main` when both branches are active protected integration targets. +Automatic repository review configuration SHALL cover pull requests targeting both `dev` and +`main` when both branches are active protected integration targets. #### Scenario: Release-forward pull request targets main - **WHEN** a pull request targets `main` - **THEN** the configured automatic review system applies the same target-branch auto-review policy used for `dev` - **AND** release PRs do not silently lose their default automated review pass + +### Requirement: Merge-blocking and advisory outputs are documented distinctly + +Contributor-facing CI and hook documentation SHALL distinguish required merge gates from advisory +signals so maintainers can interpret a green PR consistently across local hooks, GitHub Actions, +and automated review outputs. + +#### Scenario: Contributor reviews the repository quality-gate guidance + +- **WHEN** a contributor reads the documented pre-commit, CI, or PR-review guidance +- **THEN** the documentation names which checks are merge-blocking required gates +- **AND** the documentation names which outputs remain advisory warnings or review assistance diff --git a/openspec/changes/ci-02-trustworthy-green-checks/tasks.md b/openspec/changes/ci-02-trustworthy-green-checks/tasks.md index 9c97a32f..ca59d231 100644 --- a/openspec/changes/ci-02-trustworthy-green-checks/tasks.md +++ b/openspec/changes/ci-02-trustworthy-green-checks/tasks.md @@ -8,70 +8,70 @@ Per `openspec/config.yaml`, tests before code for any behavior-changing task. Or ## 1. Create git worktree for this change -- [ ] 1.1 Fetch latest and create a worktree with a new branch from `origin/dev`. - - [ ] 1.1.1 `git fetch origin` - - [ ] 1.1.2 `git worktree add ../specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks -b feature/ci-02-trustworthy-green-checks origin/dev` - - [ ] 1.1.3 Change into the worktree: `cd ../specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks` - - [ ] 1.1.4 Run `hatch env create` in the worktree to create the virtual environment and install +- [x] 1.1 Fetch latest and create a worktree with a new branch from `origin/dev`. + - [x] 1.1.1 `git fetch origin` + - [x] 1.1.2 `git worktree add ../specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks -b feature/ci-02-trustworthy-green-checks origin/dev` + - [x] 1.1.3 Change into the worktree: `cd ../specfact-cli-worktrees/feature/ci-02-trustworthy-green-checks` + - [x] 1.1.4 Run `hatch env create` in the worktree to create the virtual environment and install the dev dependencies required by the documented repository workflow. - - [ ] 1.1.5 Verify the branch: `git branch --show-current` + - [x] 1.1.5 Verify the branch: `git branch --show-current` ## 2. Spec-first preparation -- [ ] 2.1 Finalize the gate taxonomy in `specs/trustworthy-green-checks/spec.md`. -- [ ] 2.2 Audit current enforcement semantics in: - - [ ] 2.2.1 `.github/workflows/pr-orchestrator.yml` - - [ ] 2.2.2 `.github/workflows/pre-merge-check.yml` - - [ ] 2.2.3 `.pre-commit-config.yaml` - - [ ] 2.2.4 `scripts/pre-commit-smart-checks.sh` - - [ ] 2.2.5 `.coderabbit.yaml` - - [ ] 2.2.6 archived/main doc-frontmatter specs, changelog, and contributor docs for review-driven markdown/spec drift +- [x] 2.1 Finalize the gate taxonomy in `specs/trustworthy-green-checks/spec.md`. +- [x] 2.2 Audit current enforcement semantics in: + - [x] 2.2.1 `.github/workflows/pr-orchestrator.yml` + - [x] 2.2.2 `.github/workflows/pre-merge-check.yml` + - [x] 2.2.3 `.pre-commit-config.yaml` + - [x] 2.2.4 `scripts/pre-commit-smart-checks.sh` + - [x] 2.2.5 `.coderabbit.yaml` + - [x] 2.2.6 archived/main doc-frontmatter specs, changelog, and contributor docs for review-driven markdown/spec drift ## 3. Test-first / validation-first evidence -- [ ] 3.1 Add or update workflow/unit tests that prove required jobs fail when underlying tools fail and that advisory jobs are explicitly marked as advisory. -- [ ] 3.2 Add or update tests for `dev -> main` skip semantics so follow-up commits invalidate unsafe fast-path assumptions. -- [ ] 3.3 Add or update tests/spec assertions that required checks still report on docs-only or otherwise out-of-scope PR commits instead of disappearing behind workflow-level path filters. -- [ ] 3.4 Add or update tests for pre-commit parity or supported-hook installation behavior. -- [ ] 3.5 Add or update tests for review JSON failure handling and doc-frontmatter helper expectations. -- [ ] 3.6 Run the new/updated tests before implementation and capture failing evidence in `TDD_EVIDENCE.md`. +- [x] 3.1 Add or update workflow/unit tests that prove required jobs fail when underlying tools fail and that advisory jobs are explicitly marked as advisory. +- [x] 3.2 Add or update tests for `dev -> main` skip semantics so follow-up commits invalidate unsafe fast-path assumptions. +- [x] 3.3 Add or update tests/spec assertions that required checks still report on docs-only or otherwise out-of-scope PR commits instead of disappearing behind workflow-level path filters. +- [x] 3.4 Add or update tests for pre-commit parity or supported-hook installation behavior. +- [x] 3.5 Add or update tests for review JSON failure handling and doc-frontmatter helper expectations. +- [x] 3.6 Run the new/updated tests before implementation and capture failing evidence in `TDD_EVIDENCE.md`. ## 4. Implementation: CI hardening -- [ ] 4.1 Remove failure-swallowing patterns from required jobs in `pr-orchestrator.yml`. -- [ ] 4.2 Rename or isolate remaining advisory jobs so their non-blocking status is explicit in job names and logs. -- [ ] 4.3 Add mandatory workflow validation in CI for `.github/workflows/**` changes. -- [ ] 4.4 Rework required-check triggers so required branch-protection checks always emit a status on every PR head commit, including docs-only follow-up pushes. -- [ ] 4.5 Normalize overlapping check/job names between orchestrator and dedicated workflows so branch protection targets a single canonical name per gate. -- [ ] 4.6 Rework `dev -> main` skip logic so only provably safe parity skips are allowed; otherwise run the required validation set. -- [ ] 4.7 Keep docs-only validation behavior explicit and compatible with docs-review workflow ownership. +- [x] 4.1 Remove failure-swallowing patterns from required jobs in `pr-orchestrator.yml`. +- [x] 4.2 Rename or isolate remaining advisory jobs so their non-blocking status is explicit in job names and logs. +- [x] 4.3 Add mandatory workflow validation in CI for `.github/workflows/**` changes. +- [x] 4.4 Rework required-check triggers so required branch-protection checks always emit a status on every PR head commit, including docs-only follow-up pushes. +- [x] 4.5 Normalize overlapping check/job names between orchestrator and dedicated workflows so branch protection targets a single canonical name per gate. +- [x] 4.6 Rework `dev -> main` skip logic so only provably safe parity skips are allowed; otherwise run the required validation set. +- [x] 4.7 Keep docs-only validation behavior explicit and compatible with docs-review workflow ownership. ## 5. Implementation: local and review parity -- [ ] 5.1 Align `.pre-commit-config.yaml` with the supported smart-check path, or make the supported hook-install path authoritative and explicit in repo docs. -- [ ] 5.2 Update `.coderabbit.yaml` so automatic review coverage includes both `dev` and `main` PR targets. -- [ ] 5.3 Document which review outputs are advisory versus merge-blocking. +- [x] 5.1 Align `.pre-commit-config.yaml` with the supported smart-check path, or make the supported hook-install path authoritative and explicit in repo docs. +- [x] 5.2 Update `.coderabbit.yaml` so automatic review coverage includes both `dev` and `main` PR targets. +- [x] 5.3 Document which review outputs are advisory versus merge-blocking. ## 6. Implementation: review remediation and publication readiness -- [ ] 6.1 Fix valid markdownlint/style findings in `CHANGELOG.md`, `CONTRIBUTING.md`, and `docs/contributing/docs-sync.md`. -- [ ] 6.2 Fix valid archived/main OpenSpec spec publication issues (purpose text, heading spacing, unique headings, API-name drift, and scenario completeness). -- [ ] 6.3 Harden `scripts/pre_commit_code_review.py` report parsing/output handling and `scripts/check_doc_frontmatter.py` parse-failure diagnostics. -- [ ] 6.4 Align helper fixtures/tests with implemented doc-frontmatter behavior. +- [x] 6.1 Fix valid markdownlint/style findings in `CHANGELOG.md`, `CONTRIBUTING.md`, and `docs/contributing/docs-sync.md`. +- [x] 6.2 Fix valid archived/main OpenSpec spec publication issues (purpose text, heading spacing, unique headings, API-name drift, and scenario completeness). +- [x] 6.3 Harden `scripts/pre_commit_code_review.py` report parsing/output handling and `scripts/check_doc_frontmatter.py` parse-failure diagnostics. +- [x] 6.4 Align helper fixtures/tests with implemented doc-frontmatter behavior. ## 7. Quality gates and documentation -- [ ] 7.1 `hatch run format` -- [ ] 7.2 `hatch run type-check` -- [ ] 7.3 `hatch run lint` -- [ ] 7.4 `hatch run yaml-lint` -- [ ] 7.5 `hatch run lint-workflows` -- [ ] 7.6 Run targeted tests for workflow, hook, and doc-frontmatter remediation changes. -- [ ] 7.7 Update contributor docs / CI docs describing trustworthy green-check semantics. -- [ ] 7.8 Run `openspec validate ci-02-trustworthy-green-checks --strict` and resolve all issues. +- [x] 7.1 `hatch run format` +- [x] 7.2 `hatch run type-check` +- [x] 7.3 `hatch run lint` +- [x] 7.4 `hatch run yaml-lint` +- [x] 7.5 `hatch run lint-workflows` +- [x] 7.6 Run targeted tests for workflow, hook, and doc-frontmatter remediation changes. +- [x] 7.7 Update contributor docs / CI docs describing trustworthy green-check semantics. +- [x] 7.8 Run `openspec validate ci-02-trustworthy-green-checks --strict` and resolve all issues. ## 8. Delivery -- [ ] 8.1 Update `openspec/CHANGE_ORDER.md` with implementation status when work begins/lands. -- [ ] 8.2 Stage and commit with a Conventional Commit message. -- [ ] 8.3 Push `feature/ci-02-trustworthy-green-checks` and open a PR to `dev`. +- [x] 8.1 Update `openspec/CHANGE_ORDER.md` with implementation status when work begins/lands. +- [x] 8.2 Stage and commit with a Conventional Commit message. +- [x] 8.3 Push `feature/ci-02-trustworthy-green-checks` and open a PR to `dev`. diff --git a/scripts/run_actionlint.sh b/scripts/run_actionlint.sh index 92b80b62..3be25e05 100755 --- a/scripts/run_actionlint.sh +++ b/scripts/run_actionlint.sh @@ -1,54 +1,41 @@ #!/usr/bin/env bash -# Actionlint runner: Docker-first with binary fallback. -# - If Docker is available, run the actionlint container from Docker Hub (latest by default; override with ACTIONLINT_TAG). -# - Otherwise, download a pinned fallback actionlint binary into tools/bin and execute it. +# Actionlint runner. +# - Prefer a globally installed actionlint binary. +# - Otherwise, run the official Docker image when Docker is available and the daemon is reachable. +# - Otherwise, fail with explicit install guidance. Do not download binaries into the repository tree. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(dirname "$SCRIPT_DIR")" -BIN_DIR="$REPO_ROOT/tools/bin" -ACTIONLINT_TAG="${ACTIONLINT_TAG:-latest}" -ACTIONLINT_FALLBACK_VERSION="1.7.1" +ACTIONLINT_TAG="${ACTIONLINT_TAG:-v1.7.11}" DOCKER_IMAGE="rhysd/actionlint:${ACTIONLINT_TAG}" -run_with_docker() { - if command -v docker >/dev/null 2>&1; then - docker run --rm \ - -v "$REPO_ROOT":/repo \ - -w /repo \ - "$DOCKER_IMAGE" -no-color - return 0 - fi - return 1 +has_actionlint() { + command -v actionlint >/dev/null 2>&1 } -ensure_binary() { - mkdir -p "$BIN_DIR" - if [[ -x "$BIN_DIR/actionlint" ]]; then - return 0 - fi - - echo "Downloading actionlint v${ACTIONLINT_FALLBACK_VERSION} binary..." >&2 - ARCHIVE="actionlint_${ACTIONLINT_FALLBACK_VERSION}_linux_amd64.tar.gz" - URL="https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_FALLBACK_VERSION}/${ARCHIVE}" - - TMPDIR="$(mktemp -d)" - trap 'rm -rf "$TMPDIR"' EXIT - - curl -sSL "$URL" -o "$TMPDIR/$ARCHIVE" - tar -xzf "$TMPDIR/$ARCHIVE" -C "$TMPDIR" - install -m 0755 "$TMPDIR/actionlint" "$BIN_DIR/actionlint" +has_docker() { + command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1 } -run_binary() { - "$BIN_DIR/actionlint" -no-color -} +if has_actionlint; then + cd "$REPO_ROOT" + actionlint -no-color + exit $? +fi -# Prefer Docker; if not available, use binary fallback. -if run_with_docker; then - exit 0 +if has_docker; then + docker run --rm \ + -v "$REPO_ROOT":/repo \ + -w /repo \ + "$DOCKER_IMAGE" -no-color + exit $? fi -ensure_binary -run_binary +echo "actionlint is required for workflow linting." >&2 +echo "Install it globally or use a Docker-enabled environment." >&2 +echo "Official install options: https://github.com/rhysd/actionlint" >&2 +echo "Example global install:" >&2 +echo " go install github.com/rhysd/actionlint/cmd/actionlint@${ACTIONLINT_TAG}" >&2 +exit 2 diff --git a/scripts/yaml-tools.sh b/scripts/yaml-tools.sh index 89caf163..3bc95252 100755 --- a/scripts/yaml-tools.sh +++ b/scripts/yaml-tools.sh @@ -7,7 +7,7 @@ set -euo pipefail # fmt Format all YAML files (including workflows) with Prettier # lint Lint non-workflow YAML files with yamllint and repo .yamllint # workflows-fmt Format only GitHub workflows with Prettier -# workflows-lint Lint GitHub workflows with actionlint (local, bin, or docker fallback) +# workflows-lint Lint GitHub workflows with actionlint # fix-all Run fmt (covers workflows too) # check-all Run lint + workflows-lint # @@ -51,17 +51,7 @@ run_yamllint() { } run_actionlint() { - if command -v actionlint >/dev/null 2>&1; then - actionlint -color=never - elif [[ -x "${REPO_ROOT}/bin/actionlint" ]]; then - "${REPO_ROOT}/bin/actionlint" -color=never - elif command -v docker >/dev/null 2>&1; then - docker run --rm -v "${REPO_ROOT}":/repo -w /repo rhysd/actionlint:latest -no-color - else - echo "actionlint not available. Install with:" >&2 - echo " curl -sSL https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash | bash -s -- -b ./bin" >&2 - exit 2 - fi + bash "${REPO_ROOT}/scripts/run_actionlint.sh" } usage() { diff --git a/src/specfact_cli/modules/_bundle_import.py b/src/specfact_cli/modules/_bundle_import.py index fed8d832..424556d3 100644 --- a/src/specfact_cli/modules/_bundle_import.py +++ b/src/specfact_cli/modules/_bundle_import.py @@ -19,9 +19,13 @@ def bootstrap_local_bundle_sources(anchor_file: str) -> None: anchor = Path(anchor_file).resolve() candidates: list[Path] = [] - env_repo = os.environ.get("SPECFACT_CLI_MODULES_REPO") - if env_repo: - candidates.append(Path(env_repo).expanduser().resolve()) + for env_name in ("SPECFACT_CLI_MODULES_REPO", "SPECFACT_MODULES_REPO"): + env_repo = os.environ.get(env_name) + if not env_repo: + continue + candidate = Path(env_repo).expanduser().resolve() + if candidate not in candidates: + candidates.append(candidate) for parent in anchor.parents: # Primary dev layout: .../nold-ai/specfact-cli-worktrees/... and sibling specfact-cli-modules diff --git a/tests/unit/migration/test_module_migration_07_cleanup.py b/tests/unit/migration/test_module_migration_07_cleanup.py index 12b0f405..5b6f88c2 100644 --- a/tests/unit/migration/test_module_migration_07_cleanup.py +++ b/tests/unit/migration/test_module_migration_07_cleanup.py @@ -23,6 +23,7 @@ def test_no_legacy_specfact_cli_modules_import_paths() -> None: root / "tests" / "unit" / "models" / "test_module_package_metadata.py", root / "tests" / "unit" / "migration" / "test_module_migration_07_cleanup.py", root / "tests" / "unit" / "specfact_cli" / "test_module_migration_compatibility.py", + root / "tests" / "unit" / "modules" / "test_bundle_import.py", } removed_module_pattern = re.compile(r"specfact_cli\.modules\.(?!init\.|module_registry\.|upgrade\.)") diff --git a/tests/unit/modules/test_bundle_import.py b/tests/unit/modules/test_bundle_import.py new file mode 100644 index 00000000..9b237477 --- /dev/null +++ b/tests/unit/modules/test_bundle_import.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +import sys +from pathlib import Path + +from specfact_cli.modules._bundle_import import bootstrap_local_bundle_sources + + +def _make_modules_repo(tmp_path: Path) -> Path: + modules_repo = tmp_path / "specfact-cli-modules" + package_src = modules_repo / "packages" / "specfact-codebase" / "src" + package_src.mkdir(parents=True) + return modules_repo + + +def test_bootstrap_local_bundle_sources_honors_specfact_modules_repo_env(monkeypatch: object, tmp_path: Path) -> None: + modules_repo = _make_modules_repo(tmp_path) + anchor = tmp_path / "workspace" / "src" / "specfact_cli" / "commands" / "repro.py" + anchor.parent.mkdir(parents=True) + anchor.write_text("# anchor\n", encoding="utf-8") + + expected_src = str((modules_repo / "packages" / "specfact-codebase" / "src").resolve()) + monkeypatch.setenv("SPECFACT_MODULES_REPO", str(modules_repo)) + monkeypatch.delenv("SPECFACT_CLI_MODULES_REPO", raising=False) + original_path = sys.path.copy() + if expected_src in sys.path: + sys.path.remove(expected_src) + + try: + bootstrap_local_bundle_sources(str(anchor)) + assert expected_src in sys.path + finally: + sys.path[:] = original_path diff --git a/tests/unit/scripts/test_doc_frontmatter/test_validation.py b/tests/unit/scripts/test_doc_frontmatter/test_validation.py index aa43fcaa..07ac162b 100644 --- a/tests/unit/scripts/test_doc_frontmatter/test_validation.py +++ b/tests/unit/scripts/test_doc_frontmatter/test_validation.py @@ -1,6 +1,8 @@ #!/usr/bin/env python3 """Tests for doc frontmatter validation (scripts/check_doc_frontmatter.py).""" +# pyright: reportUnknownMemberType=false + from __future__ import annotations import tempfile @@ -140,6 +142,41 @@ def test_validation_with_invalid_files( result = validation_main([]) assert result == 1 + def test_validation_surfaces_yaml_parse_errors( + self, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + check_doc_frontmatter_module: CheckDocFrontmatterModule, + ) -> None: + """Malformed frontmatter should report an actionable YAML parse failure.""" + validation_main = check_doc_frontmatter_module.main + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + monkeypatch.setenv("DOC_FRONTMATTER_ROOT", str(root)) + docs_dir = root / "docs" + docs_dir.mkdir() + (docs_dir / "broken.md").write_text( + """--- +title: Broken +doc_owner: specfact-cli +tracks: + - docs/** +last_reviewed: [broken +exempt: false +exempt_reason: "" +--- + +# Broken +""", + encoding="utf-8", + ) + write_enforced(root, "docs/broken.md") + result = validation_main([]) + assert result == 1 + err = capsys.readouterr().err + assert "YAML parse error" in err + assert "docs/broken.md" in err + class TestOwnerResolutionValidation: """Test owner resolution validation.""" diff --git a/tests/unit/workflows/test_trustworthy_green_checks.py b/tests/unit/workflows/test_trustworthy_green_checks.py new file mode 100644 index 00000000..1d44eeec --- /dev/null +++ b/tests/unit/workflows/test_trustworthy_green_checks.py @@ -0,0 +1,201 @@ +"""Workflow and hook policy tests for trustworthy green checks.""" + +# pyright: reportUnknownMemberType=false + +from __future__ import annotations + +from pathlib import Path +from typing import Any, cast + +import yaml + + +REPO_ROOT = Path(__file__).resolve().parents[3] +PR_ORCHESTRATOR = REPO_ROOT / ".github" / "workflows" / "pr-orchestrator.yml" +SIGN_MODULES = REPO_ROOT / ".github" / "workflows" / "sign-modules.yml" +PRE_COMMIT_CONFIG = REPO_ROOT / ".pre-commit-config.yaml" +CODERABBIT_CONFIG = REPO_ROOT / ".coderabbit.yaml" +LEGACY_ACTIONLINT_RUNNER = REPO_ROOT / "scripts" / "run_actionlint.sh" + + +def _load_yaml(path: Path) -> dict[str, Any]: + data = yaml.safe_load(path.read_text(encoding="utf-8")) + assert isinstance(data, dict), f"Expected mapping at {path}" + return cast(dict[str, Any], data) + + +def _workflow_on_block(workflow: dict[str, Any]) -> dict[str, Any]: + """Return the Actions trigger block, accounting for YAML 1.1 `on` coercion.""" + on_block = workflow.get("on") + if on_block is None: + on_block = cast(dict[object, Any], workflow).get(True) + assert isinstance(on_block, dict), "Workflow must define event mappings" + return cast(dict[str, Any], on_block) + + +def _load_jobs() -> dict[str, dict[str, Any]]: + workflow = _load_yaml(PR_ORCHESTRATOR) + jobs = workflow.get("jobs") + assert isinstance(jobs, dict), "Expected jobs mapping in pr-orchestrator workflow" + typed_jobs: dict[str, dict[str, Any]] = {} + for job_name, job_value in jobs.items(): + assert isinstance(job_name, str), "Job names must be strings" + assert isinstance(job_value, dict), f"Expected mapping for job {job_name}" + typed_jobs[job_name] = cast(dict[str, Any], job_value) + return typed_jobs + + +def _load_hooks() -> list[dict[str, Any]]: + config = _load_yaml(PRE_COMMIT_CONFIG) + repos = config.get("repos") + assert isinstance(repos, list) and repos, "Expected repos list in .pre-commit-config.yaml" + local_repo = next( + (r for r in repos if isinstance(r, dict) and r.get("repo") == "local"), + None, + ) + assert isinstance(local_repo, dict), "Expected a 'repo: local' entry in .pre-commit-config.yaml" + hooks = local_repo.get("hooks") + assert isinstance(hooks, list), "Expected hooks list in .pre-commit-config.yaml" + typed_hooks: list[dict[str, Any]] = [] + for hook in hooks: + assert isinstance(hook, dict), "Each hook must be a mapping" + typed_hooks.append(cast(dict[str, Any], hook)) + return typed_hooks + + +def test_pr_orchestrator_required_checks_trigger_on_every_pr_head_commit() -> None: + """Required checks must not disappear behind workflow-level path filters.""" + workflow = _load_yaml(PR_ORCHESTRATOR) + on_block = _workflow_on_block(workflow) + pull_request = on_block.get("pull_request") + push = on_block.get("push") + assert isinstance(pull_request, dict), "pull_request trigger must be a mapping" + assert isinstance(push, dict), "push trigger must be a mapping" + assert "paths-ignore" not in pull_request, "Required PR workflows must still emit statuses on docs-only commits" + assert "paths-ignore" not in push, "Required push workflows must still emit statuses on docs-only commits" + + +def test_pr_orchestrator_required_jobs_fail_closed() -> None: + """Required jobs should not hide tool failures behind warn-only shell patterns.""" + required_jobs = { + "verify-module-signatures", + "tests", + "compat-py311", + "contract-first-ci", + "cli-validation", + "type-checking", + "linting", + "workflow-lint", + } + jobs = _load_jobs() + for job_name in required_jobs: + job = jobs.get(job_name) + assert job is not None, f"Expected required job {job_name!r} in pr-orchestrator" + assert job.get("continue-on-error") is not True, f"Required job {job_name!r} must fail closed" + steps = job.get("steps") + assert isinstance(steps, list), f"Expected steps list for job {job_name!r}" + for step in steps: + if not isinstance(step, dict): + continue + run = step.get("run") + if not isinstance(run, str): + continue + assert "|| echo" not in run, f"Required job {job_name!r} still swallows failure with `|| echo`" + assert "continue-on-error" not in run, f"Required job {job_name!r} must not emulate continue-on-error" + + +def test_pr_orchestrator_release_skip_requires_parity_proof() -> None: + """Release fast-path skips must be gated by parity proof, not only branch names.""" + raw = PR_ORCHESTRATOR.read_text(encoding="utf-8") + assert 'echo "skip_tests_dev_to_main=true" >> "$GITHUB_OUTPUT"' not in raw + assert "git merge-base" in raw or "git rev-parse" in raw, "Expected commit-parity proof in release skip logic" + jobs = _load_jobs() + changes = jobs.get("changes") + assert changes is not None, "Expected changes job in pr-orchestrator" + outputs = changes.get("outputs") + assert isinstance(outputs, dict), "Expected outputs mapping for changes job" + assert "skip_tests_dev_to_main" in outputs, "Release skip decision should remain explicit" + tests_job = jobs.get("tests") + assert tests_job is not None, "Expected tests job in pr-orchestrator" + steps = tests_job.get("steps") + assert isinstance(steps, list), "Expected tests job to define steps" + skip_conditions = [step.get("if") for step in steps if isinstance(step, dict)] + # Normalize conditions by collapsing whitespace and removing surrounding quotes for robust matching + normalized_conditions = [ + " ".join(cond.replace('"', "'").split()) if isinstance(cond, str) else cond for cond in skip_conditions + ] + # Assert key patterns exist regardless of minor spacing/quoting differences + assert any( + "needs.changes.outputs.skip_tests_dev_to_main" in str(cond) and "== 'true'" in str(cond) + for cond in normalized_conditions + ), "Expected a condition checking skip_tests_dev_to_main == 'true'" + assert any( + "needs.changes.outputs.skip_tests_dev_to_main" in str(cond) and "!= 'true'" in str(cond) + for cond in normalized_conditions + ), "Expected a condition checking skip_tests_dev_to_main != 'true'" + + +def test_pr_orchestrator_advisory_jobs_are_named_as_advisory() -> None: + """Advisory jobs should advertise their non-blocking status in the emitted check name.""" + jobs = _load_jobs() + quality_gate = jobs.get("quality-gates") + assert quality_gate is not None, "Expected quality-gates job in pr-orchestrator" + name = quality_gate.get("name") + assert isinstance(name, str) + assert "Advisory" in name + + +def test_pr_orchestrator_contract_first_job_uses_hatch_contract_test() -> None: + """Contract-first CI should use the hatch contract-test script (no CLI bundle dependency).""" + raw = PR_ORCHESTRATOR.read_text(encoding="utf-8") + assert "hatch run contract-test" in raw + assert "hatch run specfact repro --verbose --crosshair-required --budget 120" not in raw + + +def test_module_signature_check_name_is_canonical_across_workflows() -> None: + """Orchestrator and dedicated signature workflows should emit the same required check name.""" + orchestrator_jobs = _load_jobs() + sign_modules = _load_yaml(SIGN_MODULES) + sign_jobs = sign_modules.get("jobs") + assert isinstance(sign_jobs, dict), "Expected jobs mapping in sign-modules workflow" + orchestrator_name = orchestrator_jobs["verify-module-signatures"].get("name") + sign_job = sign_jobs.get("verify") + assert isinstance(sign_job, dict), "Expected verify job in sign-modules workflow" + dedicated_name = sign_job.get("name") + assert orchestrator_name == dedicated_name == "Verify Module Signatures" + + +def test_pre_commit_config_installs_supported_smart_check_wrapper() -> None: + """The supported local hook path should expose the same gate semantics as CI.""" + hooks = _load_hooks() + matching = [hook for hook in hooks if hook.get("entry") == "scripts/pre-commit-smart-checks.sh"] + assert matching, "Expected .pre-commit-config.yaml to expose the smart-check wrapper hook" + hook = matching[0] + assert hook.get("id") == "specfact-smart-checks", "Hook id must remain stable for pre-commit consumers" + assert hook.get("pass_filenames") is False + assert hook.get("language") == "script" + + +def test_coderabbit_auto_review_covers_dev_and_main() -> None: + """Automatic review coverage should be consistent for both protected integration branches.""" + config = _load_yaml(CODERABBIT_CONFIG) + reviews = config.get("reviews") + assert isinstance(reviews, dict), "Expected reviews block in .coderabbit.yaml" + auto_review = reviews.get("auto_review") + assert isinstance(auto_review, dict), "Expected auto_review block in .coderabbit.yaml" + base_branches = auto_review.get("base_branches") + assert isinstance(base_branches, list), "Expected base_branches list in .coderabbit.yaml" + assert "^dev$" in base_branches + assert "^main$" in base_branches + + +def test_legacy_actionlint_runner_does_not_mask_docker_failures() -> None: + """The legacy actionlint wrapper must fail cleanly when Docker is unusable.""" + raw = LEGACY_ACTIONLINT_RUNNER.read_text(encoding="utf-8") + lines = raw.splitlines() + assert any("docker run --rm" in line for line in lines), "Expected docker run invocation" + assert "docker info >/dev/null 2>&1" in raw, "Expected docker daemon reachability check" + assert "tools/bin" not in raw, "Should not download binaries into repo tree" + assert "go install github.com/rhysd/actionlint/cmd/actionlint@" in raw, "Expected global install guidance" + # Both execution paths (local binary and docker) must propagate exit codes + assert raw.count("exit $?") >= 2, "Expected exit code propagation for both local and docker paths"