From 5d9acc6f43137c599563267f20a5a41dab977714 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Fri, 10 Jul 2026 23:14:06 +0200 Subject: [PATCH 1/4] docs: enforce module documentation accountability --- .github/workflows/docs-review.yml | 21 + docs/authoring/module-signing.md | 2 +- openspec/CHANGE_ORDER.md | 5 +- .../.openspec.yaml | 2 + .../CHANGE_VALIDATION.md | 67 ++++ .../TDD_EVIDENCE.md | 152 ++++++++ .../design.md | 60 +++ .../proposal.md | 57 +++ .../documentation-accountability/spec.md | 52 +++ .../specs/module-command-overview/spec.md | 43 +++ .../specs/modules-docs-publishing/spec.md | 18 + .../modules-pre-commit-quality-parity/spec.md | 51 +++ .../docs-16-core-accountability-sync/tasks.md | 62 +++ pyproject.toml | 1 + ...check-core-documentation-accountability.py | 97 +++++ scripts/generate-command-overview.py | 78 ++++ scripts/pre-commit-quality-checks.sh | 22 +- .../pre-commit-verify-modules-signature.sh | 67 ++-- scripts/sign-modules.py | 360 ++++++++++++------ scripts/verify-modules-signature.py | 43 ++- .../unit/docs/test_llms_overview_freshness.py | 34 ++ tests/unit/test_check_docs_commands_script.py | 111 +++--- .../test_core_documentation_accountability.py | 90 +++++ tests/unit/test_pre_commit_quality_parity.py | 12 +- ..._commit_verify_modules_signature_script.py | 29 +- .../test_verify_modules_signature_script.py | 37 ++ 26 files changed, 1345 insertions(+), 228 deletions(-) create mode 100644 openspec/changes/docs-16-core-accountability-sync/.openspec.yaml create mode 100644 openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/design.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/proposal.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/specs/module-command-overview/spec.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/specs/modules-docs-publishing/spec.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md create mode 100644 openspec/changes/docs-16-core-accountability-sync/tasks.md create mode 100644 scripts/check-core-documentation-accountability.py create mode 100644 tests/unit/test_core_documentation_accountability.py diff --git a/.github/workflows/docs-review.yml b/.github/workflows/docs-review.yml index 8965c0d6..c46ab962 100644 --- a/.github/workflows/docs-review.yml +++ b/.github/workflows/docs-review.yml @@ -9,8 +9,14 @@ on: - "**/*.md" - "**/*.mdc" - "docs/**" + - "packages/**" - "packages/*/resources/**" + - "registry/**" - "requirements-docs-ci.txt" + - "pyproject.toml" + - ".pre-commit-config.yaml" + - "scripts/pre-commit-quality-checks.sh" + - "scripts/check-core-documentation-accountability.py" - "scripts/check-docs-commands.py" - "scripts/check-prompt-commands.py" - "scripts/check-command-contract.py" @@ -20,8 +26,11 @@ on: - "scripts/docs_site_validation.py" - "tests/unit/test_check_docs_commands_script.py" - "tests/unit/test_check_prompt_commands_script.py" + - "tests/unit/test_core_documentation_accountability.py" + - "tests/unit/test_pre_commit_quality_parity.py" - "tests/unit/docs/test_docs_review.py" - "tests/unit/docs/test_code_review_docs_parity.py" + - "tests/unit/docs/test_llms_overview_freshness.py" - ".github/workflows/docs-review.yml" push: branches: [main, dev] @@ -29,8 +38,14 @@ on: - "**/*.md" - "**/*.mdc" - "docs/**" + - "packages/**" - "packages/*/resources/**" + - "registry/**" - "requirements-docs-ci.txt" + - "pyproject.toml" + - ".pre-commit-config.yaml" + - "scripts/pre-commit-quality-checks.sh" + - "scripts/check-core-documentation-accountability.py" - "scripts/check-docs-commands.py" - "scripts/check-prompt-commands.py" - "scripts/check-command-contract.py" @@ -40,8 +55,11 @@ on: - "scripts/docs_site_validation.py" - "tests/unit/test_check_docs_commands_script.py" - "tests/unit/test_check_prompt_commands_script.py" + - "tests/unit/test_core_documentation_accountability.py" + - "tests/unit/test_pre_commit_quality_parity.py" - "tests/unit/docs/test_docs_review.py" - "tests/unit/docs/test_code_review_docs_parity.py" + - "tests/unit/docs/test_llms_overview_freshness.py" - ".github/workflows/docs-review.yml" workflow_dispatch: @@ -129,6 +147,9 @@ jobs: hatch run check-command-contract exit "${PIPESTATUS[0]:-$?}" + - name: Validate core documentation accountability + run: hatch run check-core-documentation-accountability + - name: Upload docs review logs if: always() uses: actions/upload-artifact@v4 diff --git a/docs/authoring/module-signing.md b/docs/authoring/module-signing.md index 602a4fa1..7cc23986 100644 --- a/docs/authoring/module-signing.md +++ b/docs/authoring/module-signing.md @@ -147,7 +147,7 @@ Workflow **`sign-modules-on-approval.yml`** runs when a review is **submitted** ### Pre-commit -The first pre-commit hook runs **`scripts/pre-commit-verify-modules-signature.sh`**, which mirrors CI: **`--require-signature` on branch `main`**, or when **`GITHUB_BASE_REF=main`** in Actions pull-request contexts; otherwise the same baseline formal verify as PRs to **`dev`** (`--payload-from-filesystem --enforce-version-bump`, no **`--require-signature`**). On failure it runs **`sign-modules.py --allow-unsigned --payload-from-filesystem`** (`--changed-only` vs **`HEAD`**, then vs **`HEAD~1`** for manifests still failing), **`git add`** those `module-package.yaml` paths, and re-verifies. It does **not** rewrite **`registry/`** (publish workflows own signed artifacts and index updates). **`yaml-lint`** allows a semver **ahead** manifest vs **`registry/index.json`** until **`publish-modules`** reconciles. +The first pre-commit hook runs **`scripts/pre-commit-verify-modules-signature.sh`**, which mirrors CI: **`--require-signature` on branch `main`**, or when **`GITHUB_BASE_REF=main`** in Actions pull-request contexts; otherwise it verifies payload checksums and version bumps without requiring a signature. On non-main branches, an existing signature with no locally available public key is not treated as checksum drift. If repair is needed, the hook runs `sign-modules.py --staged-only --allow-unsigned --payload-from-filesystem` and re-stages only manifests for module payloads staged in the pending commit; it never rewrites unrelated manifests. It does **not** rewrite **`registry/`** (publish workflows own signed artifacts and index updates). **`yaml-lint`** allows a semver **ahead** manifest vs **`registry/index.json`** until **`publish-modules`** reconciles. ## Rotation Procedure diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index 997e0ea5..31d4f1ed 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -7,7 +7,7 @@ must be read together with the core repo change order in `nold-ai/specfact-cli`. | Bucket | Count | Location | |---|---:|---| -| **Active** | 12 | [`openspec/changes/`](changes/) | +| **Active** | 13 | [`openspec/changes/`](changes/) | | **Parked** | 16 | [`openspec/parking-lot/`](parking-lot/) | | **Archived** | 43 | [`openspec/changes/archive/`](changes/archive/) | @@ -77,7 +77,8 @@ AI IDE, rerun, and compare improved evidence. | Order | Change folder | GitHub # | Positioning | Blocked by | |---:|---|---|---|---| -| 1 | `docs-14-module-release-history` | [#124](https://github.com/nold-ai/specfact-cli-modules/issues/124) | Release-history documentation for shipped modules | docs-13, publish workflow | +| 1 | `docs-16-core-accountability-sync` | [#339](https://github.com/nold-ai/specfact-cli-modules/issues/339) | Fail-closed reciprocal core-documentation accountability and generated module-command freshness | core #643 shipped | +| 2 | `docs-14-module-release-history` | [#124](https://github.com/nold-ai/specfact-cli-modules/issues/124) | Release-history documentation for shipped modules | docs-13, publish workflow | ## Modify Queue Before Implementation diff --git a/openspec/changes/docs-16-core-accountability-sync/.openspec.yaml b/openspec/changes/docs-16-core-accountability-sync/.openspec.yaml new file mode 100644 index 00000000..eb5fa80e --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-10 diff --git a/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md new file mode 100644 index 00000000..1486bf23 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md @@ -0,0 +1,67 @@ +# Change Validation: docs-16-core-accountability-sync + +- **Validated on:** 2026-07-10 Europe/Berlin +- **Workflow:** proposal-readiness review and strict OpenSpec validation +- **Strict command:** `openspec validate docs-16-core-accountability-sync --strict` +- **Result:** PASS (0 issues) + +## Scope Summary + +- **Extended capabilities:** reciprocal `documentation-accountability`, + `module-command-overview`, `modules-pre-commit-quality-parity`, and + `modules-docs-publishing`. +- **Outcome:** modules fails closed when paired core documentation is stale and + always regenerates/verifies AI command artifacts for module and registry + changes. + +## Dependency And Ownership Review + +- Core #643 / `cli-val-05-ci-integration` owns the authoritative checker, + official-inventory loading, and core catalogue/ownership rules. +- Modules owns paired-core resolution, local and PR gate wiring, generated + module-command artifacts, and modules-side regression coverage. +- #339 is open, assigned, labelled, parented by #162, and in the SpecFact CLI + project Todo state as reviewed on 2026-07-10 Europe/Berlin. +- No native GitHub `blocked_by` relation is present for the completed core + prerequisite; recheck that governance detail before production implementation. + +## Validation Outcome + +`openspec validate docs-16-core-accountability-sync --strict` passed with zero +issues. Production behavior remains intentionally unimplemented; the unchecked +task list preserves the required `spec -> tests -> failing evidence -> code -> +passing evidence` sequence. + +## Proposal Quality Evidence + +- `hatch run validate-agent-rule-signals` passed. +- `hatch run yaml-lint` passed. +- At 2026-07-10 22:16 Europe/Berlin, `hatch run specfact code review run` with + `--enforcement changed --bug-hunt --json --out .specfact/code-review.json` + reviewed every changed OpenSpec artifact and reported zero findings. + +## Implementation Quality Evidence + +- The core wrapper, Docs Review integration, pre-commit routing, and generated + command inventory guard were implemented with failing-before and + passing-after evidence in `TDD_EVIDENCE.md`. +- The installed local Block 2 pre-commit hook passed its generated-artifact, + core-accountability, docs, review, and contract stages using an isolated + temporary index. +- Type, lint, YAML, import-boundary, contract, direct docs-gate, and strict + OpenSpec checks passed. The final code-review JSON is fresh and reports zero + errors and zero warnings. +- The full test suite has one environmental limitation: the local Semgrep + runtime cannot initialize its CA trust store, yielding unrelated fixture + failures; see `TDD_EVIDENCE.md` for the exact evidence. + +## Final Code-Review Evidence + +At 2026-07-10 23:05 Europe/Berlin, `hatch run specfact code review run` with +`--enforcement changed --bug-hunt --json` reported zero errors and zero +warnings. Four informational advisory notes remain: the two CLI argument +parser helpers are long with low branch complexity, the no-dependency +`icontract` fallback has an intentionally optional description parameter, and +one pre-existing verification test is long with low branch complexity. They +are recorded as non-blocking design observations, not unresolved review +findings. diff --git a/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md new file mode 100644 index 00000000..628514b5 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md @@ -0,0 +1,152 @@ +# TDD Evidence: docs-16-core-accountability-sync + +## Failing-before + +### 2026-07-10 Europe/Berlin + +Command: + +```bash +hatch run pytest tests/unit/test_core_documentation_accountability.py \ + tests/unit/test_pre_commit_quality_parity.py \ + tests/unit/test_check_docs_commands_script.py \ + tests/unit/docs/test_llms_overview_freshness.py -q +``` + +Result: failed as expected before production edits (7 failed, 21 passed). + +- `scripts/check-core-documentation-accountability.py` did not exist. +- Pre-commit did not classify `packages/**` or `registry/**` as + documentation-relevant and did not invoke a core-accountability gate. +- Docs Review did not trigger for package/registry inventory changes or run the + core-accountability command. +- The command-overview generator had no authoritative inventory-to-mount + validation, so an official record with no command mount was not rejected. + +## Passing-after + +### 2026-07-10 Europe/Berlin + +Focused regression command: + +```bash +hatch run pytest tests/unit/test_core_documentation_accountability.py \ + tests/unit/test_pre_commit_quality_parity.py \ + tests/unit/test_check_docs_commands_script.py \ + tests/unit/docs/test_llms_overview_freshness.py -q +``` + +Result: passed (50 passed). + +- The modules wrapper resolves explicit, sibling, and paired-worktree core + checkouts, fails closed with setup guidance, and delegates to the core-owned + checker. +- The command overview rejects an official manifest/registry record with no + command mount. +- Pre-commit and Docs Review policy tests prove package and registry changes + trigger generated-artifact and core-accountability validation. + +Direct gate commands: + +```bash +hatch run check-core-documentation-accountability +hatch run check-command-overview +hatch run check-command-contract +hatch run python scripts/check-docs-commands.py +``` + +Result: passed. The core checker reported `documentation-accountability: OK`, +the generated contract validated 91 command paths, and docs validation reported +no findings. + +Installed-hook proof: + +```bash +hatch run pre-commit install +hatch run pre-commit run modules-block2 --hook-stage pre-commit +``` + +Result: passed using an isolated temporary Git index and pre-commit cache. The +installed Block 2 hook regenerated and verified the three generated command +artifacts, passed core accountability and docs validation, then passed its +review and contract stages without changing the real staging area. + +## Final Quality Evidence + +### 2026-07-10 Europe/Berlin + +- The combined docs-accountability and deterministic-signature regression + suite passed: 60 tests. +- `hatch run format`, `hatch run type-check`, `hatch run lint`, `hatch run + yaml-lint`, `hatch run check-bundle-imports`, and `hatch run contract-test` + passed. +- `openspec validate docs-16-core-accountability-sync --strict` passed. +- `hatch run specfact code review run --enforcement changed --bug-hunt --json + --out .specfact/code-review.json` reported zero errors and zero warnings. + Its four remaining entries are informational design advisories only. +- The initial `hatch run test` run completed 842 passing tests but could not + complete cleanly in this environment: Semgrep failed to initialize its system + CA trust store, producing 17 unrelated fixture failures. A docs-workflow path + assertion was corrected during that run; its focused regression test passes. + +## Deterministic signature-hook failing-before + +### 2026-07-10 Europe/Berlin + +Command: + +```bash +hatch run pytest tests/unit/test_verify_modules_signature_script.py \ + tests/unit/test_pre_commit_verify_modules_signature_script.py -q +``` + +Result: failed as expected (2 failed, 7 passed). + +- `verify_manifest` could not accept an optional missing-public-key mode, so a + locally unavailable public key made every existing signed manifest fail even + when signatures were not required for the branch. +- The non-main hook still selected `--changed-only` remediation and the + `HEAD~1` fallback that passed every failed manifest explicitly, bypassing + changed-only selection and risking unrelated patch bumps. + +## Deterministic signature-hook passing-after + +### 2026-07-10 Europe/Berlin + +Commands: + +```bash +hatch run pytest tests/unit/test_verify_modules_signature_script.py \ + tests/unit/test_pre_commit_verify_modules_signature_script.py -q +hatch run ./scripts/verify-modules-signature.py \ + --payload-from-filesystem --enforce-version-bump --allow-missing-public-key +``` + +Result: passed (10 tests; all 7 module manifests checksum-verified). + +- An optional existing signature with no local public key no longer causes + non-main checksum verification to fail or start remediation. +- The hook uses `--staged-only` and removes the failed-manifest/`HEAD~1` + fallback; repair candidates come from `git diff --cached` only. + +Installed-hook proof: + +```bash +hatch run pre-commit run --hook-stage pre-commit +``` + +Result: passed with an isolated temporary index. SHA-1 hashes of every +`packages/*/module-package.yaml` file were identical before and after the full +hook, and `git diff HEAD -- packages` remained empty. The hook exercised the +generated-artifact, core-accountability, docs, review, and contract stages. + +## Semgrep Environment Note + +### 2026-07-10 Europe/Berlin + +Inside the restricted execution sandbox, Homebrew Semgrep 1.168.0 fails before +scanning because its OCaml CA-store provider reports empty system trust anchors. +Outside that sandbox, `semgrep --version && semgrep scan --config tools/semgrep +--quiet tests/fixtures/semgrep/good_print_in_src.py` exits successfully. This +is host-sandbox access to macOS certificate anchors, not a repository rule, +source, or terminal configuration defect. diff --git a/openspec/changes/docs-16-core-accountability-sync/design.md b/openspec/changes/docs-16-core-accountability-sync/design.md new file mode 100644 index 00000000..d5d12a3f --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/design.md @@ -0,0 +1,60 @@ +# Design: docs-16 core documentation accountability sync + +## Context + +The core repository already owns `check-documentation-accountability.py`. It +derives official module packages and grouped roots from a supplied modules +checkout, then validates core catalogues, generated command artifacts, and +ownership handoffs. Modules must call that authority instead of maintaining a +second inventory or repeating core-catalogue rules. + +The modules command overview is deterministic but its pre-commit routing only +runs generation for selected docs and prompt paths. It therefore needs broader +module/registry input routing plus inventory coverage that makes new or remapped +official records fail rather than pass with unchanged generated output. + +## Decisions + +1. **Use a subprocess wrapper, not a copied checker.** The wrapper resolves + `SPECFACT_CLI_REPO`, then a sibling `specfact-cli` checkout, then the mapped + paired worktree. It invokes the core checker with the current modules root. + Missing checkout or checker is a non-zero, actionable setup failure. +2. **Keep CI branch pairing deterministic.** Docs Review first uses a matching + core branch name; if unavailable, it uses only `dev` or `main` from the PR + base/ref fallback. Checkout and checker failures block the workflow. +3. **Run generated-artifact work for every module/registry change.** Local + pre-commit regenerates and stages only deterministic changes after rejecting + relevant unstaged inputs. CI runs check-only validation and never modifies + artifacts. Unrelated implementation-only changes may produce identical + generated files; freshness still proves that result. +4. **Make manifest/registry inventory authoritative for overview coverage.** + The overview generator/checker must reject disagreement, omission, rename, + or grouped-root remapping between official inventory records and command + mounts. Adding a module therefore requires an explicit represented mount and + regenerated artifacts. +5. **Make non-main signature repair staged-only and non-destructive.** Baseline + verification still validates every payload checksum. When a non-required + signature has no local public key, its cryptographic verification is skipped + rather than treated as checksum drift. Automatic checksum/version repair is + limited to module payloads staged for the pending commit; unrelated or + pre-existing failures stop without rewriting manifests. Main keeps strict + signature verification and no checksum-only auto-repair. + +## Risks And Mitigations + +- **Paired-core resolution drift** — Cover explicit, sibling, paired-worktree, + and unavailable paths; document the required environment variable. +- **Accidental staging of unrelated generated output** — Refuse local + auto-staging when relevant inputs have unstaged hunks. +- **Workflow filters omit a validation input** — Test Docs Review paths for + manifests, registry, package source/resources/docs, tooling, workflow, and + generated artifacts. +- **Signature repair mutates unrelated manifests** — Derive repair candidates + from the staged index only, remove the failed-manifest fallback, and test that + missing optional public keys do not start repair. + +## Rollback + +Revert the wrapper, gate wiring, and inventory/freshness checks together. No +data migration, registry publication, module signature, or runtime rollback is +required. diff --git a/openspec/changes/docs-16-core-accountability-sync/proposal.md b/openspec/changes/docs-16-core-accountability-sync/proposal.md new file mode 100644 index 00000000..02551402 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/proposal.md @@ -0,0 +1,57 @@ +# docs-16: Core documentation accountability sync + +## Why + +A modules-only manifest, registry, command, resource, or bundle-docs change can +make core catalogue and ownership documentation stale. Core change +`cli-val-05-ci-integration` supplies the authoritative, fail-closed +documentation-accountability checker, but modules currently does not invoke it +reciprocally. Local pre-commit also regenerates `llms.txt` only for selected +docs and prompt paths, so a module or registry change can bypass the generated +command-artifact update path. + +## What Changes + +- **ADD** a thin modules-side wrapper that resolves a paired core checkout and + invokes its authoritative documentation-accountability checker against the + current modules checkout; the wrapper owns no duplicate official-module + inventory or core catalogue rules. +- **MODIFY** local pre-commit and Docs Review routing so package, registry, + command, resource, docs, generated-artifact, dependency, and gate changes run + the same fail-closed documentation validation before any docs-only safe + bypass. +- **MODIFY** generated command-overview validation so every `packages/**` or + `registry/**` change regenerates and verifies `llms.txt` and both generated + command-reference artifacts; CI remains read-only and rejects drift. +- **MODIFY** command-overview inventory validation so an official manifest or + grouped-root change cannot silently remain absent from generated module + command artifacts. +- **MODIFY** non-main module-signature pre-commit remediation so an unrelated + docs or workflow commit cannot rewrite module manifests; checksum repair is + limited to staged module payloads and unavailable optional public keys do not + trigger destructive local repair. + +## Impact + +- **Affected specs**: `documentation-accountability`, + `module-command-overview`, `modules-pre-commit-quality-parity`, and + `modules-docs-publishing`. +- **Affected surfaces**: modules pre-commit helper, Docs Review workflow, + command-overview generator/checks, paired-checkout resolution, module + signature verification/remediation, and their focused regression tests. +- **No public CLI or module-runtime API changes**: this is validation and + generated-documentation hardening only. + +## Source Tracking + +- **GitHub Issue**: [#339](https://github.com/nold-ai/specfact-cli-modules/issues/339) +- **Parent Epic**: [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162) +- **Project**: SpecFact CLI (`Todo`) +- **Labels**: `bug`, `documentation`, `change-proposal` +- **Prerequisite**: core [#643](https://github.com/nold-ai/specfact-cli/issues/643), + implemented by `specfact-cli/cli-val-05-ci-integration`; it owns the + authoritative checker and is closed as of 2026-07-10 Europe/Berlin. +- **Blockers / blocked-by**: no native GitHub dependency is currently recorded; + implementation readiness must recheck and reconcile the relationship before + production edits. +- **Last Synced Status**: #339 open, assigned, Todo; #643 closed, Done diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md new file mode 100644 index 00000000..3dc62959 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md @@ -0,0 +1,52 @@ +## ADDED Requirements + +### Requirement: Modules SHALL enforce core documentation accountability from the authoritative checker + +The modules repository SHALL invoke the authoritative core +documentation-accountability checker against the current modules checkout. The +modules-side integration SHALL resolve the core checkout from +`SPECFACT_CLI_REPO`, then the documented sibling checkout, then the matching +paired worktree, and SHALL fail closed with setup guidance when none supplies +the required checker. It SHALL NOT duplicate official-module inventory or core +catalogue validation rules. + +#### Scenario: Module inventory change exposes stale core documentation + +- **GIVEN** a modules manifest or grouped command root changes +- **AND** the paired core catalogue, generated command artifact, or ownership + handoff remains stale +- **WHEN** the modules documentation-accountability gate runs +- **THEN** the core authoritative checker exits non-zero +- **AND** reports the stale core documentation context. + +#### Scenario: Paired core checkout is unavailable + +- **GIVEN** `SPECFACT_CLI_REPO`, the sibling checkout, and the paired worktree + do not provide the core checker +- **WHEN** the modules local gate runs +- **THEN** it exits non-zero with instructions to configure or create a paired + core checkout +- **AND** it does not skip or substitute a duplicated local checker. + +### Requirement: Core accountability SHALL block matching local and PR gates + +The same modules-side core-accountability command SHALL run before a docs-only +safe bypass in pre-commit and as a blocking Docs Review workflow step. Docs +Review SHALL use the same-named core branch when available, otherwise the +sanitized `dev` or `main` base fallback, and SHALL fail when the checkout or +checker cannot be resolved. + +#### Scenario: Modules PR checks a matching core branch + +- **GIVEN** a modules pull-request branch has a same-named branch in the core + repository +- **WHEN** Docs Review runs +- **THEN** it checks out that core branch +- **AND** core-accountability failures block the workflow. + +#### Scenario: Modules PR falls back to the base core branch + +- **GIVEN** the same-named core branch does not exist +- **WHEN** Docs Review runs for a `dev` or `main` pull request +- **THEN** it uses the corresponding base branch +- **AND** it fails rather than silently skipping accountability validation. diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/module-command-overview/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/module-command-overview/spec.md new file mode 100644 index 00000000..719e93dd --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/specs/module-command-overview/spec.md @@ -0,0 +1,43 @@ +## MODIFIED Requirements + +### Requirement: Modules Publish Generated Command Overview Artifacts + +The modules repository SHALL generate deterministic command overview artifacts +from the actual module command tree and authoritative official-module manifest +and registry inventory. + +#### Scenario: Module command overview artifacts are generated + +- **GIVEN** the module command overview generator runs in the modules repository +- **WHEN** it writes artifacts +- **THEN** it produces `llms.txt`, `docs/reference/commands.generated.md`, and + `docs/reference/commands.generated.json` +- **AND** every command record includes command path, owning repo, owning module + package, install prerequisite, short help, arguments/options, subcommands, + source import path when known, and hidden/deprecated status +- **AND** generated output is stable for the same source tree. + +#### Scenario: Official inventory is not represented by command mounts + +- **GIVEN** an official package or grouped root in manifests and the registry is + missing, renamed, or remapped relative to the command-mount inventory +- **WHEN** command overview generation or freshness validation runs +- **THEN** it exits non-zero and identifies the unrepresented or inconsistent + official record +- **AND** it does not certify unchanged generated artifacts as current. + +#### Scenario: README links generated overview + +- **GIVEN** a user or AI agent opens the modules repository README +- **WHEN** they look for command usage +- **THEN** the README links to the generated module command overview artifact. + +#### Scenario: Stale generated artifacts fail checks + +- **GIVEN** any path under `packages/**` or `registry/**` changes, or command + overview, prompt, docs, or command-validation tooling changes +- **WHEN** the command overview freshness check runs +- **THEN** local pre-commit regenerates and stages all generated artifacts after + rejecting relevant unstaged inputs +- **AND** CI performs a read-only check and fails if the artifacts are stale +- **AND** the failure reports the command needed to regenerate them. diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/modules-docs-publishing/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/modules-docs-publishing/spec.md new file mode 100644 index 00000000..63eba734 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/specs/modules-docs-publishing/spec.md @@ -0,0 +1,18 @@ +## MODIFIED Requirements + +### Requirement: Docs review CI SHALL run the same deterministic docs validators as local checks + +The Docs Review workflow SHALL run the deterministic validators used by local +pre-commit, including generated command-overview freshness, command-contract +validation, applicable prompt-command validation, and fail-closed core +documentation accountability, plus docs unit tests. + +#### Scenario: Module-only pull request validates core accountability + +- **WHEN** a pull request changes module manifests, registry data, package + source/resources/docs, generated artifacts, validation tooling, dependency + configuration, or Docs Review workflow inputs +- **THEN** Docs Review runs generated-artifact and core-accountability checks +- **AND** the workflow fails when the paired core catalogue or ownership + handoff is stale +- **AND** it does not report a passing docs review without those checks. diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md new file mode 100644 index 00000000..d4df80d7 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md @@ -0,0 +1,51 @@ +## MODIFIED Requirements + +### Requirement: Docs-only pre-commit changes SHALL run docs validation before safe bypass + +The modules repo pre-commit helper SHALL run deterministic docs validation, +generated command-overview freshness, command-contract validation, prompt +command validation when applicable, and core documentation accountability for +staged documentation-relevant changes before skipping code-specific review and +contract-test stages. Any `packages/**` or `registry/**` change is +documentation-relevant for generated command artifacts and accountability. + +#### Scenario: Manifest-only commit cannot bypass documentation gates + +- **WHEN** only a module manifest or registry record is staged +- **THEN** pre-commit regenerates and verifies the generated command artifacts +- **AND** runs the core-accountability gate before the safe-change decision +- **AND** fails if the generated artifacts or core documentation are stale. + +#### Scenario: Docs-only commit with valid documentation skips code-specific checks + +- **WHEN** only documentation-relevant paths are staged and all required docs, + generated-artifact, and accountability gates pass +- **THEN** pre-commit may skip code review and contract-test stages +- **AND** it reports each completed documentation gate before applying the safe + bypass. + +### Requirement: Non-main module-signature remediation SHALL be deterministic and staged-only + +The non-main module signature hook SHALL validate payload checksums and +version policy without rewriting manifests solely because an existing optional +signature cannot be verified locally. If checksum/version remediation is +needed, it SHALL target only module payloads staged for the pending commit. +Unchanged, unstaged, or unrelated failed manifests SHALL not be passed as +explicit repair inputs. + +#### Scenario: Docs-only commit has no module manifest mutation + +- **WHEN** a commit stages only docs, workflow, OpenSpec, or gate files +- **AND** optional manifest signatures cannot be cryptographically verified + because no local public key is configured +- **THEN** the non-main signature hook does not rewrite, version-bump, or stage + any `packages/*/module-package.yaml` file +- **AND** it continues with checksum validation and downstream docs gates. + +#### Scenario: Staged module payload repair is scoped + +- **WHEN** a staged module payload causes checksum or version drift on a + non-main branch +- **THEN** automatic checksum/version repair may update only that staged + module's manifest +- **AND** the hook re-verifies without repairing unrelated manifests. diff --git a/openspec/changes/docs-16-core-accountability-sync/tasks.md b/openspec/changes/docs-16-core-accountability-sync/tasks.md new file mode 100644 index 00000000..7b39a999 --- /dev/null +++ b/openspec/changes/docs-16-core-accountability-sync/tasks.md @@ -0,0 +1,62 @@ +# docs-16: Tasks — core documentation accountability sync + +## 1. Governance And Readiness + +- [x] 1.1 Refresh and consult GitHub hierarchy state; verify #339 parent, + labels, project, blockers, blocked-by, and concurrency status before + production edits. +- [x] 1.2 Verify core #643 / `cli-val-05-ci-integration` remains available on + the selected core branch and owns the authoritative checker. +- [x] 1.3 Validate this change with `openspec validate + docs-16-core-accountability-sync --strict` before implementation. + +## 2. Failing-First Regression Coverage + +- [x] 2.1 Add failing wrapper tests for explicit, sibling, paired-worktree, + missing-core, and missing-checker resolution paths. +- [x] 2.2 Add failing tests that stale core catalogues and ownership handoffs + fail when a modules manifest or grouped root changes. +- [x] 2.3 Add failing generator/checker tests for manifest/registry + disagreement, an unrepresented official package, renamed package, and + remapped grouped root. +- [x] 2.4 Add failing pre-commit and workflow-policy tests proving every + `packages/**` and `registry/**` change runs generated-artifact freshness and + core accountability before safe bypass. +- [x] 2.5 Record failing-before commands and output in `TDD_EVIDENCE.md` before + production edits. +- [x] 2.6 Add failing tests proving non-main optional-signature verification + does not mutate docs-only commits and staged-only repair cannot fall back to + all failed manifests. + +## 3. Fail-Closed Gate Implementation + +- [x] 3.1 Implement the thin modules-side core-accountability wrapper and Hatch + command; do not duplicate the core inventory or catalogue checks. +- [x] 3.2 Expand pre-commit routing for manifest, registry, package command, + resource, docs, dependency, generated-artifact, and gate changes; refuse + auto-staging when relevant inputs are unstaged. +- [x] 3.3 Make generated overview validation reject authoritative inventory and + command-mount disagreement, then regenerate all three artifacts locally. +- [x] 3.4 Update Docs Review path filters and steps to run strict docs, + generated-artifact, command-contract, prompt-command, and core-accountability + checks against the resolved paired core ref. +- [x] 3.5 Make module-signature verification tolerate an unavailable optional + public key on non-main branches and scope automatic repair to staged module + payloads only. + +## 4. Verification And Evidence + +- [x] 4.1 Run focused wrapper, generator, pre-commit, and workflow tests; + record passing evidence in `TDD_EVIDENCE.md`. +- [x] 4.2 Run the local pre-commit helper with representative docs-only, + manifest-only, registry-only, and command-source staged changes. +- [x] 4.3 Run `hatch run check-command-overview`, command-contract and docs + validation, and the core-accountability wrapper directly. +- [x] 4.4 Run required touched-scope quality gates, including format, + type-check, lint, YAML validation, contract tests, and strict OpenSpec + validation. +- [x] 4.5 Run `hatch run specfact code review run --enforcement changed + --bug-hunt --json --out .specfact/code-review.json`; remediate every finding, + rerun as needed, and record fresh evidence. +- [x] 4.6 Run focused signature-hook regression tests and a docs-only + pre-commit simulation that proves no manifest is modified. diff --git a/pyproject.toml b/pyproject.toml index 67514128..a21972b8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,6 +57,7 @@ validate-cli-contracts = "python tools/validate_cli_contracts.py" validate-prompt-commands = "python scripts/check-prompt-commands.py" generate-command-overview = "python tools/ensure_core_dependency.py && python scripts/generate-command-overview.py --write" check-command-overview = "python tools/ensure_core_dependency.py && python scripts/generate-command-overview.py --check" +check-core-documentation-accountability = "python scripts/check-core-documentation-accountability.py" check-command-contract = "python tools/ensure_core_dependency.py && python scripts/check-command-contract.py" check-bundle-imports = "python scripts/check-bundle-imports.py" sign-modules = "python scripts/sign-modules.py {args}" diff --git a/scripts/check-core-documentation-accountability.py b/scripts/check-core-documentation-accountability.py new file mode 100644 index 00000000..f5233d20 --- /dev/null +++ b/scripts/check-core-documentation-accountability.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# ruff: noqa: N999 +"""Run the authoritative core documentation-accountability checker for modules.""" + +from __future__ import annotations + +import argparse +import os +import subprocess +import sys +from pathlib import Path + +from beartype import beartype +from icontract import ensure, require + + +REPO_ROOT = Path(__file__).resolve().parents[1] +CORE_CHECKER_RELATIVE_PATH = Path("scripts") / "check-documentation-accountability.py" + + +def _paired_worktree_checkout() -> Path | None: + marker = "specfact-cli-modules-worktrees" + if marker not in REPO_ROOT.parts: + return None + marker_index = REPO_ROOT.parts.index(marker) + base = Path(*REPO_ROOT.parts[:marker_index]) + suffix = Path(*REPO_ROOT.parts[marker_index + 1 :]) + return base / "specfact-cli-worktrees" / suffix + + +def _core_candidates(explicit_path: str | None) -> list[Path]: + candidates: list[Path] = [] + configured = explicit_path or os.environ.get("SPECFACT_CLI_REPO", "").strip() + if configured: + candidates.append(Path(configured).expanduser()) + candidates.append(REPO_ROOT.parent / "specfact-cli") + + marker = "specfact-cli-modules-worktrees" + if marker in REPO_ROOT.parts: + marker_index = REPO_ROOT.parts.index(marker) + candidates.append(Path(*REPO_ROOT.parts[:marker_index]) / "specfact-cli") + paired_checkout = _paired_worktree_checkout() + if paired_checkout is not None: + candidates.append(paired_checkout) + return candidates + + +@beartype +@ensure(lambda result: result.is_dir()) +def resolve_core_checkout(explicit_path: str | None = None) -> Path: + """Resolve a core checkout containing its authoritative checker.""" + checked_paths: list[Path] = [] + for candidate in _core_candidates(explicit_path): + resolved = candidate.resolve() + if resolved in checked_paths: + continue + checked_paths.append(resolved) + if (resolved / CORE_CHECKER_RELATIVE_PATH).is_file(): + return resolved + checked = ", ".join(str(path) for path in checked_paths) + raise ValueError( + "Cannot resolve specfact-cli documentation-accountability checker. " + "Set SPECFACT_CLI_REPO to a core checkout containing " + f"{CORE_CHECKER_RELATIVE_PATH}. Checked: {checked}" + ) + + +@beartype +@require(lambda core_root, modules_root: core_root.is_dir() and modules_root.is_dir()) +@ensure(lambda result: isinstance(result, int)) +def run_accountability(core_root: Path, modules_root: Path) -> int: + """Delegate validation to the core-owned checker without copying its rules.""" + command = [ + sys.executable, + str(core_root / CORE_CHECKER_RELATIVE_PATH), + "--modules-repo", + str(modules_root.resolve()), + ] + return subprocess.run(command, check=False).returncode + + +@beartype +@ensure(lambda result: isinstance(result, int)) +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--core-repo", help="Path to the paired specfact-cli checkout") + args = parser.parse_args(argv) + try: + core_root = resolve_core_checkout(args.core_repo) + except ValueError as exc: + sys.stderr.write(f"core-documentation-accountability: {exc}\n") + return 1 + return run_accountability(core_root, REPO_ROOT) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/generate-command-overview.py b/scripts/generate-command-overview.py index 30df6718..f1a952a1 100644 --- a/scripts/generate-command-overview.py +++ b/scripts/generate-command-overview.py @@ -10,10 +10,12 @@ import json import os import sys +from collections.abc import Mapping from pathlib import Path from typing import Any, cast import click +import yaml from beartype import beartype from icontract import ensure from typer.main import get_command @@ -72,6 +74,81 @@ def _ensure_package_paths() -> None: sys.path.insert(0, src) +def _is_official_nold_module(data: Mapping[str, object]) -> bool: + publisher = data.get("publisher") + return data.get("tier") == "official" and isinstance(publisher, Mapping) and publisher.get("name") == "nold-ai" + + +def _official_manifest_inventory() -> dict[str, str]: + inventory: dict[str, str] = {} + for manifest_path in sorted((REPO_ROOT / "packages").glob("*/module-package.yaml")): + data = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + if not isinstance(data, dict) or not _is_official_nold_module(data): + continue + package_id = data.get("name") + grouped_root = data.get("bundle_group_command") + if not isinstance(package_id, str) or not isinstance(grouped_root, str) or not grouped_root: + raise ValueError(f"Invalid official module manifest: {manifest_path}") + if package_id in inventory: + raise ValueError(f"Duplicate official module manifest: {package_id}") + inventory[package_id] = grouped_root + if not inventory: + raise ValueError(f"No official module manifests found under {REPO_ROOT / 'packages'}") + return inventory + + +def _official_registry_inventory() -> set[str]: + registry_path = REPO_ROOT / "registry" / "index.json" + registry = json.loads(registry_path.read_text(encoding="utf-8")) + if not isinstance(registry, dict) or not isinstance(registry.get("modules"), list): + raise ValueError(f"Invalid marketplace registry: {registry_path}") + package_ids: set[str] = set() + for entry in registry["modules"]: + if not isinstance(entry, dict) or not _is_official_nold_module(entry): + continue + package_id = entry.get("id") + if not isinstance(package_id, str): + raise ValueError(f"Invalid official registry entry: {registry_path}") + package_ids.add(package_id) + return package_ids + + +@beartype +@ensure(lambda result: result is None) +def validate_official_mount_inventory() -> None: + """Reject official package records that cannot appear in generated output.""" + manifests = _official_manifest_inventory() + registry_ids = _official_registry_inventory() + if set(manifests) != registry_ids: + raise ValueError( + "Official manifests and marketplace registry disagree: " + f"manifests={sorted(manifests)}, registry={sorted(registry_ids)}" + ) + + mount_roots: dict[str, set[str]] = {} + for _, _, prefix, package_id in MODULE_APP_MOUNTS: + if len(prefix) < 2 or prefix[0] != "specfact": + raise ValueError(f"Invalid command mount for {package_id}: {prefix}") + mount_roots.setdefault(package_id, set()).add(prefix[1]) + + missing = sorted(set(manifests) - set(mount_roots)) + unexpected = sorted(set(mount_roots) - set(manifests)) + mismatched_roots = [ + f"{package_id} (expected {root}, mounts {sorted(mount_roots[package_id])})" + for package_id, root in sorted(manifests.items()) + if package_id in mount_roots and root not in mount_roots[package_id] + ] + findings: list[str] = [] + if missing: + findings.append(f"missing command mounts for {missing}") + if unexpected: + findings.append(f"command mounts without official manifests for {unexpected}") + if mismatched_roots: + findings.append(f"grouped root mismatch for {mismatched_roots}") + if findings: + raise ValueError("Official module command inventory is inconsistent: " + "; ".join(findings)) + + def _command_options(command: click.Command) -> list[str]: options: set[str] = set() for param in command.params: @@ -164,6 +241,7 @@ def _walk(command: click.Command, path: tuple[str, ...], source: str, module_id: @ensure(lambda result: all("command" in record for record in result)) def build_records() -> list[dict[str, Any]]: _ensure_package_paths() + validate_official_mount_inventory() records: list[dict[str, Any]] = [] for module_name, attr_name, prefix, module_id in MODULE_APP_MOUNTS: module = importlib.import_module(module_name) diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index 9298c541..3fbf414c 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -78,7 +78,7 @@ staged_docs_validation_paths() { while IFS= read -r line; do [ -z "${line}" ] && continue case "${line}" in - docs/*|*.md|requirements-docs-ci.txt|scripts/check-docs-commands.py|scripts/check-command-contract.py|scripts/docs_site_validation.py|scripts/generate-command-overview.py|llms.txt) + packages/**|registry/**|docs/*|*.md|*.mdc|requirements-docs-ci.txt|pyproject.toml|.pre-commit-config.yaml|scripts/pre-commit-quality-checks.sh|scripts/check-core-documentation-accountability.py|scripts/check-docs-commands.py|scripts/check-prompt-commands.py|scripts/check-command-contract.py|scripts/docs_site_validation.py|scripts/generate-command-overview.py|docs/reference/commands.generated.*|llms.txt|.github/workflows/docs-review.yml|tests/unit/test_core_documentation_accountability.py|tests/unit/test_pre_commit_quality_parity.py|tests/unit/test_check_docs_commands_script.py|tests/unit/test_check_prompt_commands_script.py|tests/unit/docs/test_docs_review.py|tests/unit/docs/test_code_review_docs_parity.py|tests/unit/docs/test_llms_overview_freshness.py) printf '%s\n' "${line}" ;; esac @@ -166,8 +166,8 @@ run_command_overview_validation_gate() { local unstaged_inputs unstaged_inputs="$( { - git diff --name-only -- packages scripts/generate-command-overview.py scripts/check-command-contract.py pyproject.toml - git ls-files --others --exclude-standard -- packages scripts/generate-command-overview.py scripts/check-command-contract.py pyproject.toml + git diff --name-only -- packages registry scripts/generate-command-overview.py scripts/check-command-contract.py pyproject.toml + git ls-files --others --exclude-standard -- packages registry scripts/generate-command-overview.py scripts/check-command-contract.py pyproject.toml } | sort -u )" if [[ -n "${unstaged_inputs}" ]]; then @@ -202,6 +202,20 @@ run_command_overview_validation_gate() { fi } +run_core_documentation_accountability_gate() { + if ! needs_docs_site_validation; then + return 0 + fi + info "📄 Core documentation accountability — running \`hatch run check-core-documentation-accountability\`" + if hatch run check-core-documentation-accountability; then + success "✅ Core documentation accountability passed" + else + error "❌ Core documentation accountability failed" + warn "💡 Set SPECFACT_CLI_REPO to the paired core checkout and synchronize its catalogue documentation" + exit 1 + fi +} + check_safe_change() { local files files=$(staged_files) @@ -353,6 +367,7 @@ run_block1_lint() { run_block2() { warn "🔍 modules pre-commit — Block 2 — hook: review + contract tests" run_command_overview_validation_gate + run_core_documentation_accountability_gate run_docs_site_validation_gate run_prompt_command_validation_gate if check_safe_change; then @@ -374,6 +389,7 @@ run_all() { run_lint_if_staged_python success "✅ Block 1 complete (all stages passed or skipped as expected)" run_command_overview_validation_gate + run_core_documentation_accountability_gate run_docs_site_validation_gate run_prompt_command_validation_gate if check_safe_change; then diff --git a/scripts/pre-commit-verify-modules-signature.sh b/scripts/pre-commit-verify-modules-signature.sh index b75c0a7a..17b035d4 100755 --- a/scripts/pre-commit-verify-modules-signature.sh +++ b/scripts/pre-commit-verify-modules-signature.sh @@ -9,9 +9,9 @@ # checksums with `scripts/sign-modules.py # --allow-unsigned --payload-from-filesystem` when they lack a release signing key. # -# On the `omit` policy, if verify fails, the hook runs `sign-modules.py --changed-only` against -# `HEAD` (staged + unstaged changes) with `--bump-version patch --allow-unsigned`, re-stages only -# manifests that script reports updating, then re-runs verify so commits self-heal formal drift. +# On the `omit` policy, checksum/version repair is limited to module payloads staged for the +# pending commit. Existing optional signatures without a locally available public key do not +# trigger repair. The hook never rewrites unrelated manifests. # Registry rows and published tarballs are intentionally left to CI (`publish-modules`); do not # rewrite registry/index.json or registry/modules from pre-commit. set -euo pipefail @@ -30,6 +30,21 @@ sig_policy="${sig_policy//$'\n'/}" _base=(hatch run ./scripts/verify-modules-signature.py --payload-from-filesystem --enforce-version-bump) +_staged_module_manifests() { + local path bundle manifest + while IFS= read -r path; do + [[ -n "${path}" ]] || continue + case "${path}" in + packages/*/*) + bundle="${path#packages/}" + bundle="${bundle%%/*}" + manifest="packages/${bundle}/module-package.yaml" + [[ -f "${manifest}" ]] && printf '%s\n' "${manifest}" + ;; + esac + done < <(git diff --cached --name-only -- packages) | sort -u +} + _stage_manifests_from_sign_output() { # sign-modules prints lines like "packages//module-package.yaml: checksum" or ": version a -> b" local line mf @@ -49,8 +64,9 @@ case "${sig_policy}" in ;; omit) echo "🔐 Verifying module manifests (formal: payload checksum + version bump; signatures not required on this branch — see docs/reference/module-security.md)" >&2 + _omit_base=("${_base[@]}" --allow-missing-public-key) set +e - _verify_out="$("${_base[@]}" 2>&1)" + _verify_out="$("${_omit_base[@]}" 2>&1)" _verify_rc=$? set -e if ((_verify_rc == 0)); then @@ -58,16 +74,20 @@ case "${sig_policy}" in fi printf '%s\n' "${_verify_out}" >&2 - _failed_manifests=() + _staged_manifests=() while IFS= read -r mf; do - [[ -n "${mf}" ]] && _failed_manifests+=("${mf}") - done < <(printf '%s\n' "${_verify_out}" | grep '^FAIL packages/' | sed -n 's/^FAIL \(packages\/[^[:space:]]*\/module-package\.yaml\):.*/\1/p' | sort -u) + [[ -n "${mf}" ]] && _staged_manifests+=("${mf}") + done < <(_staged_module_manifests) + if ((${#_staged_manifests[@]} == 0)); then + echo "❌ Module verification failed, but no module payload is staged; refusing to rewrite unrelated manifests." >&2 + exit 1 + fi - echo "⚠️ Module verify failed; auto-remediating checksums and patch bumps for changed modules..." >&2 + echo "⚠️ Module verify failed; auto-remediating staged module checksums and patch bumps..." >&2 _sign_log="$(mktemp "${TMPDIR:-/tmp}/specfact-sign-modules.XXXXXX")" trap 'rm -f "${_sign_log}"' EXIT if ! hatch run ./scripts/sign-modules.py \ - --changed-only \ + --staged-only \ --base-ref HEAD \ --bump-version patch \ --allow-unsigned \ @@ -84,37 +104,12 @@ case "${sig_policy}" in _stage_manifests_from_sign_output <"${_sign_log}" echo "🔐 Re-verifying after auto-remediation..." >&2 set +e - _verify2_out="$("${_base[@]}" 2>&1)" + _verify2_out="$("${_omit_base[@]}" 2>&1)" _verify2_rc=$? set -e - if ((_verify2_rc != 0)) && ((${#_failed_manifests[@]} > 0)); then - # Covers committed manifest drift (no diff vs HEAD) or partial first-pass fixes. - printf '%s\n' "${_verify2_out}" >&2 - echo "⚠️ Retrying sign for failing manifests (compare base HEAD~1)..." >&2 - if ! hatch run ./scripts/sign-modules.py \ - --changed-only \ - --base-ref HEAD~1 \ - --bump-version patch \ - --allow-unsigned \ - --payload-from-filesystem \ - "${_failed_manifests[@]}" >>"${_sign_log}" 2>&1 - then - cat "${_sign_log}" >&2 - echo "❌ sign-modules fallback remediation failed." >&2 - exit 1 - fi - _stage_manifests_from_sign_output <"${_sign_log}" - echo "🔐 Re-verifying after fallback remediation..." >&2 - if ! "${_base[@]}"; then - echo "❌ Module verify still failing after remediation (manual fix or signing key may be required)." >&2 - exit 1 - fi - echo "✅ Module manifests updated and staged; continuing the commit." >&2 - exit 0 - fi if ((_verify2_rc != 0)); then printf '%s\n' "${_verify2_out}" >&2 - echo "❌ Module verify still failing after remediation (manual fix or signing key may be required)." >&2 + echo "❌ Module verify still failing after staged-only remediation; no unrelated manifest will be rewritten." >&2 exit 1 fi echo "✅ Module manifests updated and staged; continuing the commit." >&2 diff --git a/scripts/sign-modules.py b/scripts/sign-modules.py index 13e18082..4ef93325 100755 --- a/scripts/sign-modules.py +++ b/scripts/sign-modules.py @@ -15,6 +15,18 @@ from typing import Any import yaml +from icontract import ensure, require + + +try: + from cryptography.hazmat.primitives import hashes, serialization + from cryptography.hazmat.primitives.asymmetric import ed25519, padding, rsa +except ImportError: # pragma: no cover - exercised only without signing dependencies + hashes = None + ed25519 = None + padding = None + rsa = None + serialization = None _IGNORED_MODULE_DIR_NAMES = {"__pycache__", ".pytest_cache", ".mypy_cache", ".ruff_cache", "logs"} @@ -25,6 +37,7 @@ class _IndentedSafeDumper(yaml.SafeDumper): """Safe dumper that indents sequence items under their parent key.""" + @ensure(lambda result: result is None) def increase_indent(self, flow: bool = False, indentless: bool = False): return super().increase_indent(flow=flow, indentless=False) @@ -35,109 +48,144 @@ def _canonical_payload(manifest_data: dict[str, Any]) -> bytes: return yaml.safe_dump(payload, sort_keys=True, allow_unicode=False).encode("utf-8") -def _module_payload(module_dir: Path, payload_from_filesystem: bool = False) -> bytes: - if not module_dir.exists() or not module_dir.is_dir(): - msg = f"Module directory not found: {module_dir}" - raise ValueError(msg) - module_dir_resolved = module_dir.resolve() +def _is_hashable_module_file(path: Path, *, module_dir: Path, ignored_dirs: set[str]) -> bool: + rel = path.resolve().relative_to(module_dir) + return ( + not any(part in ignored_dirs for part in rel.parts) and path.suffix.lower() not in _IGNORED_MODULE_FILE_SUFFIXES + ) - def _is_hashable(path: Path, ignored_dirs: set[str]) -> bool: - rel = path.resolve().relative_to(module_dir_resolved) - if any(part in ignored_dirs for part in rel.parts): - return False - return path.suffix.lower() not in _IGNORED_MODULE_FILE_SUFFIXES - entries: list[str] = [] - ignored_dirs = _PAYLOAD_FROM_FS_IGNORED_DIRS if payload_from_filesystem else _IGNORED_MODULE_DIR_NAMES +def _sorted_hashable_files(paths: list[Path], *, module_dir: Path, ignored_dirs: set[str]) -> list[Path]: + return sorted( + ( + path + for path in paths + if path.is_file() and _is_hashable_module_file(path, module_dir=module_dir, ignored_dirs=ignored_dirs) + ), + key=lambda path: path.resolve().relative_to(module_dir).as_posix(), + ) - files: list[Path] + +def _git_module_files(module_dir: Path, *, ignored_dirs: set[str]) -> list[Path]: + listed = subprocess.run( + ["git", "ls-files", module_dir.as_posix()], + check=True, + capture_output=True, + text=True, + ).stdout.splitlines() + files = [Path.cwd() / line.strip() for line in listed if line.strip()] + return _sorted_hashable_files(files, module_dir=module_dir, ignored_dirs=ignored_dirs) + + +def _module_files(module_dir: Path, *, payload_from_filesystem: bool) -> list[Path]: + ignored_dirs = _PAYLOAD_FROM_FS_IGNORED_DIRS if payload_from_filesystem else _IGNORED_MODULE_DIR_NAMES if payload_from_filesystem: - files = sorted( - (p for p in module_dir.rglob("*") if p.is_file() and _is_hashable(p, ignored_dirs)), - key=lambda p: p.resolve().relative_to(module_dir_resolved).as_posix(), - ) - else: - try: - listed = subprocess.run( - ["git", "ls-files", module_dir.as_posix()], - check=True, - capture_output=True, - text=True, - ).stdout.splitlines() - git_files = [(Path.cwd() / line.strip()) for line in listed if line.strip()] - files = sorted( - (path for path in git_files if path.is_file() and _is_hashable(path, ignored_dirs)), - key=lambda p: p.resolve().relative_to(module_dir_resolved).as_posix(), - ) - except Exception: - files = sorted( - (path for path in module_dir.rglob("*") if path.is_file() and _is_hashable(path, ignored_dirs)), - key=lambda p: p.resolve().relative_to(module_dir_resolved).as_posix(), - ) + return _sorted_hashable_files(list(module_dir.rglob("*")), module_dir=module_dir, ignored_dirs=ignored_dirs) + try: + return _git_module_files(module_dir, ignored_dirs=ignored_dirs) + except (subprocess.CalledProcessError, OSError): + return _sorted_hashable_files(list(module_dir.rglob("*")), module_dir=module_dir, ignored_dirs=ignored_dirs) + + +def _payload_file_data(path: Path, *, module_dir: Path) -> bytes: + rel = path.resolve().relative_to(module_dir).as_posix() + if rel not in {"module-package.yaml", "metadata.yaml"}: + return path.read_bytes() + raw = yaml.safe_load(path.read_text(encoding="utf-8")) + if not isinstance(raw, dict): + raise ValueError(f"Invalid manifest YAML: {path}") + return _canonical_payload(raw) - for path in files: - rel = path.resolve().relative_to(module_dir_resolved).as_posix() - if rel in {"module-package.yaml", "metadata.yaml"}: - raw = yaml.safe_load(path.read_text(encoding="utf-8")) - if not isinstance(raw, dict): - msg = f"Invalid manifest YAML: {path}" - raise ValueError(msg) - data = _canonical_payload(raw) - else: - data = path.read_bytes() - entries.append(f"{rel}:{hashlib.sha256(data).hexdigest()}") + +def _module_payload(module_dir: Path, payload_from_filesystem: bool = False) -> bytes: + if not module_dir.is_dir(): + raise ValueError(f"Module directory not found: {module_dir}") + resolved_module_dir = module_dir.resolve() + entries = [] + for path in _module_files(resolved_module_dir, payload_from_filesystem=payload_from_filesystem): + relative_path = path.resolve().relative_to(resolved_module_dir).as_posix() + digest = hashlib.sha256(_payload_file_data(path, module_dir=resolved_module_dir)).hexdigest() + entries.append(f"{relative_path}:{digest}") return "\n".join(entries).encode("utf-8") -def _load_private_key( - key_file: Path | None = None, - *, - passphrase: str | None = None, - prompt_for_passphrase: bool = False, -) -> Any | None: - pem = os.environ.get("SPECFACT_MODULE_PRIVATE_SIGN_KEY", "").strip() - if not pem: - pem = os.environ.get("SPECFACT_MODULE_SIGNING_PRIVATE_KEY_PEM", "").strip() - configured_file = os.environ.get("SPECFACT_MODULE_PRIVATE_SIGN_KEY_FILE", "").strip() - if not configured_file: - configured_file = os.environ.get("SPECFACT_MODULE_SIGNING_PRIVATE_KEY_FILE", "").strip() +def _first_environment_value(*names: str) -> str: + for name in names: + value = os.environ.get(name, "").strip() + if value: + return value + return "" + + +def _private_key_pem(key_file: Path | None) -> str: + inline_pem = _first_environment_value( + "SPECFACT_MODULE_PRIVATE_SIGN_KEY", + "SPECFACT_MODULE_SIGNING_PRIVATE_KEY_PEM", + ) + if inline_pem: + return inline_pem + configured_file = _first_environment_value( + "SPECFACT_MODULE_PRIVATE_SIGN_KEY_FILE", + "SPECFACT_MODULE_SIGNING_PRIVATE_KEY_FILE", + ) effective_file = key_file or (Path(configured_file) if configured_file else None) - if not pem and effective_file: - pem = effective_file.read_text(encoding="utf-8") - if not pem: - return None + return effective_file.read_text(encoding="utf-8") if effective_file else "" - try: - from cryptography.hazmat.primitives import serialization - except Exception as exc: + +def _signing_serialization() -> Any: + if serialization is None: raise ValueError( "Unable to import cryptography backend for signing. " "Install signing dependencies (`python3 -m pip install cryptography cffi`) " "or run via project environment (`hatch run python scripts/sign-modules.py ...`)." - ) from exc + ) + return serialization + - password_bytes = passphrase.encode("utf-8") if passphrase is not None else None +def _signing_primitives() -> tuple[Any, Any, Any, Any]: + if any(dependency is None for dependency in (hashes, ed25519, padding, rsa)): + raise ValueError("cryptography signing primitives are unavailable") + return hashes, ed25519, padding, rsa + +def _is_encrypted_key_error(error: ValueError | TypeError) -> bool: + message = str(error).lower() + return "password was not given" in message or "private key is encrypted" in message + + +def _load_private_key_from_pem(pem: str, password: bytes | None) -> Any: + return _signing_serialization().load_pem_private_key(pem.encode("utf-8"), password=password) + + +def _prompt_for_private_key(pem: str) -> Any: + prompted = getpass.getpass("Enter signing key passphrase: ") try: - return serialization.load_pem_private_key(pem.encode("utf-8"), password=password_bytes) - except Exception as exc: - message = str(exc).lower() - needs_password = "password was not given" in message or "private key is encrypted" in message - if needs_password and prompt_for_passphrase: - prompted = getpass.getpass("Enter signing key passphrase: ") - try: - return serialization.load_pem_private_key( - pem.encode("utf-8"), - password=prompted.encode("utf-8"), - ) - except Exception as retry_exc: - raise ValueError(f"Failed to load private key from PEM: {retry_exc}") from retry_exc - if needs_password and passphrase is None: + return _load_private_key_from_pem(pem, prompted.encode("utf-8")) + except (TypeError, ValueError) as error: + raise ValueError(f"Failed to load private key from PEM: {error}") from error + + +def _load_private_key( + key_file: Path | None = None, + *, + passphrase: str | None = None, + prompt_for_passphrase: bool = False, +) -> Any | None: + pem = _private_key_pem(key_file) + if not pem: + return None + password = passphrase.encode("utf-8") if passphrase is not None else None + try: + return _load_private_key_from_pem(pem, password) + except (TypeError, ValueError) as error: + if _is_encrypted_key_error(error) and prompt_for_passphrase: + return _prompt_for_private_key(pem) + if _is_encrypted_key_error(error) and passphrase is None: raise ValueError( "Private key is encrypted. Provide passphrase via --passphrase, --passphrase-stdin, " "or SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE." - ) from exc - raise ValueError(f"Failed to load private key from PEM: {exc}") from exc + ) from error + raise ValueError(f"Failed to load private key from PEM: {error}") from error def _resolve_passphrase(args: argparse.Namespace) -> str | None: @@ -174,11 +222,11 @@ def _read_manifest_version_from_git(git_ref: str, path: Path) -> str | None: capture_output=True, text=True, ) - except Exception: + except (subprocess.CalledProcessError, OSError): return None try: raw = yaml.safe_load(output.stdout) - except Exception: + except yaml.YAMLError: return None if not isinstance(raw, dict): return None @@ -227,11 +275,24 @@ def _module_has_git_changes_since(module_dir: Path, git_ref: str) -> bool: capture_output=True, text=True, ).stdout.strip() - except Exception: + except (subprocess.CalledProcessError, OSError): return False return bool(changed or untracked) +def _module_has_staged_changes(module_dir: Path) -> bool: + try: + changed = subprocess.run( + ["git", "diff", "--cached", "--name-only", "--", module_dir.as_posix()], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + except (subprocess.CalledProcessError, OSError): + return False + return bool(changed) + + def _parse_semver(version: str) -> tuple[int, int, int]: parts = version.split(".") if len(parts) != 3 or any(not part.isdigit() for part in parts): @@ -264,6 +325,10 @@ def _write_manifest(path: Path, data: dict[str, Any]) -> None: ) +def _report(message: str) -> None: + sys.stdout.write(f"{message}\n") + + def _auto_bump_manifest_version(manifest_path: Path, *, base_ref: str, bump_type: str) -> bool: current_version = _read_manifest_version(manifest_path) if not current_version: @@ -279,7 +344,7 @@ def _auto_bump_manifest_version(manifest_path: Path, *, base_ref: str, bump_type bumped = _bump_semver(current_version, bump_type) raw["version"] = bumped _write_manifest(manifest_path, raw) - print(f"{manifest_path}: version {current_version} -> {bumped}") + _report(f"{manifest_path}: version {current_version} -> {bumped}") return True @@ -310,19 +375,20 @@ def _enforce_version_bump_before_signing( def _sign_payload(payload: bytes, private_key: Any) -> str: - from cryptography.hazmat.primitives import hashes - from cryptography.hazmat.primitives.asymmetric import ed25519, padding, rsa + crypto_hashes, crypto_ed25519, crypto_padding, crypto_rsa = _signing_primitives() - if isinstance(private_key, ed25519.Ed25519PrivateKey): + if isinstance(private_key, crypto_ed25519.Ed25519PrivateKey): signature = private_key.sign(payload) - elif isinstance(private_key, rsa.RSAPrivateKey): - signature = private_key.sign(payload, padding.PKCS1v15(), hashes.SHA256()) + elif isinstance(private_key, crypto_rsa.RSAPrivateKey): + signature = private_key.sign(payload, crypto_padding.PKCS1v15(), crypto_hashes.SHA256()) else: msg = "Unsupported private key type for signing (RSA and Ed25519 only)" raise ValueError(msg) return base64.b64encode(signature).decode("ascii") +@require(lambda manifest_path: manifest_path.is_file(), "manifest_path must be a file") +@ensure(lambda result: result is None, "sign_manifest returns None") def sign_manifest(manifest_path: Path, private_key: Any | None, *, payload_from_filesystem: bool = False) -> None: raw = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) if not isinstance(raw, dict): @@ -340,10 +406,10 @@ def sign_manifest(manifest_path: Path, private_key: Any | None, *, payload_from_ _write_manifest(manifest_path, raw) status = "checksum+signature" if "signature" in integrity else "checksum" - print(f"{manifest_path}: {status}") + _report(f"{manifest_path}: {status}") -def main() -> int: +def _build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument( "--key-file", @@ -370,7 +436,10 @@ def main() -> int: parser.add_argument( "--payload-from-filesystem", action="store_true", - help="Build payload from filesystem (rglob) with same excludes as publish tarball, so checksum matches install verification.", + help=( + "Build payload from filesystem (rglob) with same excludes as publish tarball, " + "so checksum matches install verification." + ), ) parser.add_argument( "--allow-same-version", @@ -382,6 +451,11 @@ def main() -> int: action="store_true", help="Select only manifests whose module payload changed since --base-ref.", ) + parser.add_argument( + "--staged-only", + action="store_true", + help="Select only manifests whose module payload is staged for the pending commit.", + ) parser.add_argument( "--base-ref", default="HEAD", @@ -394,45 +468,48 @@ def main() -> int: help="Auto-bump changed module version when unchanged from --base-ref before signing.", ) parser.add_argument("manifests", nargs="*", help="module-package.yaml path(s)") - args = parser.parse_args() + return parser + + +def _selection_mode(args: argparse.Namespace, parser: argparse.ArgumentParser) -> str: + if args.changed_only and args.staged_only: + parser.error("Use only one of --changed-only or --staged-only.") + if args.changed_only: + return "changed" + if args.staged_only: + return "staged" + parser.error("Provide one or more manifests, or use --changed-only or --staged-only.") + return "" + + +def _discover_manifests(mode: str, base_ref: str) -> list[Path]: + if mode == "changed": + return [manifest for manifest in _iter_manifests() if _module_has_git_changes_since(manifest.parent, base_ref)] + return [manifest for manifest in _iter_manifests() if _module_has_staged_changes(manifest.parent)] + + +def _select_manifests(args: argparse.Namespace, parser: argparse.ArgumentParser) -> list[Path]: + + if args.manifests: + if args.staged_only: + parser.error("Do not combine explicit manifests with --staged-only.") + return [Path(manifest) for manifest in args.manifests] + + mode = _selection_mode(args, parser) - passphrase = _resolve_passphrase(args) try: - private_key = _load_private_key( - args.key_file, - passphrase=passphrase, - prompt_for_passphrase=sys.stdin.isatty() and not args.passphrase_stdin, - ) + _ensure_valid_git_ref(args.base_ref) except ValueError as exc: parser.error(str(exc)) - if private_key is None and not args.allow_unsigned: - parser.error( - "No signing key provided. Use --key-file (recommended) " - "or set SPECFACT_MODULE_PRIVATE_SIGN_KEY / SPECFACT_MODULE_PRIVATE_SIGN_KEY_FILE. " - "For local testing only, re-run with --allow-unsigned." - ) + return _discover_manifests(mode, args.base_ref) - manifests: list[Path] - if args.manifests: - manifests = [Path(manifest) for manifest in args.manifests] - elif args.changed_only: - try: - _ensure_valid_git_ref(args.base_ref) - except ValueError as exc: - parser.error(str(exc)) - manifests = [ - manifest for manifest in _iter_manifests() if _module_has_git_changes_since(manifest.parent, args.base_ref) - ] - else: - parser.error("Provide one or more manifests, or use --changed-only.") - - if args.changed_only and not manifests: - print(f"No changed module manifests detected since {args.base_ref}.") - return 0 +def _sign_selected_manifests( + manifests: list[Path], args: argparse.Namespace, parser: argparse.ArgumentParser, private_key: Any | None +) -> None: for manifest_path in manifests: try: - if args.changed_only and args.bump_version: + if (args.changed_only or args.staged_only) and args.bump_version: _auto_bump_manifest_version( manifest_path, base_ref=args.base_ref, @@ -446,6 +523,37 @@ def main() -> int: sign_manifest(manifest_path, private_key, payload_from_filesystem=args.payload_from_filesystem) except ValueError as exc: parser.error(str(exc)) + + +@ensure(lambda result: result in {0, 1}, "main returns a process exit code") +def main() -> int: + parser = _build_parser() + args = parser.parse_args() + + passphrase = _resolve_passphrase(args) + try: + private_key = _load_private_key( + args.key_file, + passphrase=passphrase, + prompt_for_passphrase=sys.stdin.isatty() and not args.passphrase_stdin, + ) + except ValueError as exc: + parser.error(str(exc)) + if private_key is None and not args.allow_unsigned: + parser.error( + "No signing key provided. Use --key-file (recommended) " + "or set SPECFACT_MODULE_PRIVATE_SIGN_KEY / SPECFACT_MODULE_PRIVATE_SIGN_KEY_FILE. " + "For local testing only, re-run with --allow-unsigned." + ) + + manifests = _select_manifests(args, parser) + + if (args.changed_only or args.staged_only) and not manifests: + scope = f"since {args.base_ref}" if args.changed_only else "in the staged index" + _report(f"No changed module manifests detected {scope}.") + return 0 + + _sign_selected_manifests(manifests, args, parser, private_key) return 0 diff --git a/scripts/verify-modules-signature.py b/scripts/verify-modules-signature.py index 565bc20c..72f1c092 100755 --- a/scripts/verify-modules-signature.py +++ b/scripts/verify-modules-signature.py @@ -176,6 +176,25 @@ def _checksum_matches(module_dir: Path, *, algo: str, digest: str, payload_from_ return actual == digest +def _verify_present_signature( + payload: bytes, + signature: str, + *, + require_signature: bool, + public_key_pem: str, + allow_missing_public_key: bool, +) -> None: + if not signature: + if require_signature: + raise ValueError("missing integrity.signature") + return + if not public_key_pem: + if require_signature or not allow_missing_public_key: + raise ValueError("public key required to verify signature") + return + _verify_signature(payload, signature, public_key_pem) + + def _cryptography_backend_available() -> bool: return all( dependency is not None for dependency in (InvalidSignature, hashes, serialization, ed25519, padding, rsa) @@ -353,6 +372,7 @@ def verify_manifest( require_signature: bool, public_key_pem: str, payload_from_filesystem: bool = False, + allow_missing_public_key: bool = False, ) -> str: raw = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) if not isinstance(raw, dict): @@ -380,13 +400,13 @@ def verify_manifest( else: raise ValueError("checksum mismatch") - signature = str(integrity.get("signature", "")).strip() - if require_signature and not signature: - raise ValueError("missing integrity.signature") - if signature: - if not public_key_pem: - raise ValueError("public key required to verify signature") - _verify_signature(payload, signature, public_key_pem) + _verify_present_signature( + payload, + str(integrity.get("signature", "")).strip(), + require_signature=require_signature, + public_key_pem=public_key_pem, + allow_missing_public_key=allow_missing_public_key, + ) return verification_mode @@ -446,6 +466,14 @@ def _parse_verify_cli_args() -> argparse.Namespace: "--payload-from-filesystem." ), ) + parser.add_argument( + "--allow-missing-public-key", + action="store_true", + help=( + "Permit checksum verification when an optional existing signature " + "cannot be verified because no local public key is available." + ), + ) parser.add_argument( "--metadata-only", action="store_true", @@ -475,6 +503,7 @@ def _verify_manifests_for_cli(args: argparse.Namespace, public_key_pem: str, man require_signature=args.require_signature, public_key_pem=public_key_pem, payload_from_filesystem=args.payload_from_filesystem, + allow_missing_public_key=args.allow_missing_public_key, ) suffix = ( " (filesystem payload)" diff --git a/tests/unit/docs/test_llms_overview_freshness.py b/tests/unit/docs/test_llms_overview_freshness.py index 40270938..9bd3dd9d 100644 --- a/tests/unit/docs/test_llms_overview_freshness.py +++ b/tests/unit/docs/test_llms_overview_freshness.py @@ -15,6 +15,8 @@ import pytest +from tests.unit._script_test_utils import load_module_from_path + REPO_ROOT = Path(__file__).resolve().parents[3] GENERATOR = REPO_ROOT / "scripts" / "generate-command-overview.py" @@ -48,3 +50,35 @@ def test_llms_and_command_overview_are_current() -> None: "Regenerate with 'hatch run generate-command-overview' and commit the result.\n" f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" ) + + +def test_command_overview_rejects_unrepresented_official_inventory(tmp_path: Path, monkeypatch) -> None: + generator = load_module_from_path("generate_command_overview_inventory", GENERATOR) + manifest = tmp_path / "packages" / "specfact-example" / "module-package.yaml" + manifest.parent.mkdir(parents=True) + manifest.write_text( + "\n".join( + ( + "name: nold-ai/specfact-example", + "tier: official", + "publisher:", + " name: nold-ai", + "bundle_group_command: example", + "commands:", + " - example", + ) + ) + + "\n", + encoding="utf-8", + ) + registry = tmp_path / "registry" / "index.json" + registry.parent.mkdir() + registry.write_text( + '{"modules": [{"id": "nold-ai/specfact-example", "tier": "official", "publisher": {"name": "nold-ai"}}]}\n', + encoding="utf-8", + ) + monkeypatch.setattr(generator, "REPO_ROOT", tmp_path) + monkeypatch.setattr(generator, "MODULE_APP_MOUNTS", ()) + + with pytest.raises(ValueError, match="missing command mounts"): + generator.validate_official_mount_inventory() diff --git a/tests/unit/test_check_docs_commands_script.py b/tests/unit/test_check_docs_commands_script.py index bdf849d2..589e3f7e 100644 --- a/tests/unit/test_check_docs_commands_script.py +++ b/tests/unit/test_check_docs_commands_script.py @@ -96,77 +96,76 @@ def test_command_example_is_valid_allows_root_help_but_not_unknown_subgroups() - assert not _script_attr(script, "_command_example_is_valid")("specfact policy validate --repo .", valid_paths) -def test_build_valid_command_paths_rejects_malformed_generated_json(tmp_path: Path, monkeypatch) -> None: +@pytest.mark.parametrize( + ("content", "message"), + ( + ('{"command": "specfact backlog"}\n', "expected a JSON list"), + ('[{"owner_package": "specfact-backlog"}]\n', "missing 'command'"), + ), +) +def test_build_valid_command_paths_rejects_malformed_generated_input( + tmp_path: Path, monkeypatch, content: str, message: str +) -> None: script = _load_script() generated = tmp_path / "commands.generated.json" - generated.write_text('{"command": "specfact backlog"}\n', encoding="utf-8") + generated.write_text(content, encoding="utf-8") monkeypatch.setattr(script, "GENERATED_COMMANDS_PATH", generated) - with pytest.raises(ValueError, match="expected a JSON list"): + with pytest.raises(ValueError, match=message): _script_attr(script, "_build_valid_command_paths")() -def test_build_valid_command_paths_rejects_malformed_generated_entries(tmp_path: Path, monkeypatch) -> None: - script = _load_script() - generated = tmp_path / "commands.generated.json" - generated.write_text('[{"owner_package": "specfact-backlog"}]\n', encoding="utf-8") - monkeypatch.setattr(script, "GENERATED_COMMANDS_PATH", generated) - - with pytest.raises(ValueError, match="missing 'command'"): - _script_attr(script, "_build_valid_command_paths")() - - -def test_validate_legacy_resource_paths_reports_stale_core_owned_paths(tmp_path: Path) -> None: - script = _load_script() - doc_path = tmp_path / "legacy.md" - doc_path.write_text( - "Copy the prompt from src/specfact_cli/prompts/review.md before running the workflow.\n", - encoding="utf-8", - ) - +def _scan_path_findings(script, path: Path, text: str, finding_function: str): + path.write_text(text, encoding="utf-8") scan = _script_attr(script, "_scan_text_by_path_for_findings") - per_line = _script_attr(script, "_legacy_resource_findings_for_line") - findings = scan({doc_path: doc_path.read_text(encoding="utf-8")}, per_line) - - assert len(findings) == 1 - assert findings[0].category == "legacy-resource" - assert "src/specfact_cli/prompts" in findings[0].message - - -def test_validate_core_docs_links_rejects_unknown_route(tmp_path: Path) -> None: + per_line = _script_attr(script, finding_function) + return scan({path: path.read_text(encoding="utf-8")}, per_line) + + +@pytest.mark.parametrize( + "case", + ( + ( + "legacy.md", + "Copy the prompt from src/specfact_cli/prompts/review.md before running the workflow.\n", + "_legacy_resource_findings_for_line", + "legacy-resource", + "src/specfact_cli/prompts", + ), + ( + "links.md", + "[Broken](https://docs.specfact.io/missing/page/)\n" + "[Allowed](https://docs.specfact.io/reference/documentation-url-contract/)\n", + "_core_docs_link_findings_for_line", + "cross-site-link", + "missing/page", + ), + ), +) +def test_docs_validation_reports_stale_reference_findings(tmp_path: Path, case: tuple[str, str, str, str, str]) -> None: + filename, text, finding_function, category, message = case script = _load_script() - doc_path = tmp_path / "links.md" - doc_path.write_text( - "[Broken](https://docs.specfact.io/missing/page/)\n" - "[Allowed](https://docs.specfact.io/reference/documentation-url-contract/)\n", - encoding="utf-8", - ) - - scan = _script_attr(script, "_scan_text_by_path_for_findings") - per_line = _script_attr(script, "_core_docs_link_findings_for_line") - findings = scan({doc_path: doc_path.read_text(encoding="utf-8")}, per_line) + findings = _scan_path_findings(script, tmp_path / filename, text, finding_function) assert len(findings) == 1 - assert findings[0].category == "cross-site-link" - assert "missing/page" in findings[0].message + assert findings[0].category == category + assert message in findings[0].message def test_validate_core_docs_links_allows_core_handoff_routes(tmp_path: Path) -> None: """Handoff URLs used in modules docs must stay in ALLOWED_CORE_DOCS_ROUTES (see scripts/check-docs-commands.py).""" script = _load_script() doc_path = tmp_path / "handoff.md" - doc_path.write_text( + findings = _scan_path_findings( + script, + doc_path, "[Debug](https://docs.specfact.io/core-cli/debug-logging/)\n" "[Debug anchor](https://docs.specfact.io/core-cli/debug-logging/#examining-ado-api-errors)\n" "[Directory](https://docs.specfact.io/reference/directory-structure/)\n" "[Feature keys](https://docs.specfact.io/reference/feature-keys/)\n", - encoding="utf-8", + "_core_docs_link_findings_for_line", ) - scan = _script_attr(script, "_scan_text_by_path_for_findings") - per_line = _script_attr(script, "_core_docs_link_findings_for_line") - findings = scan({doc_path: doc_path.read_text(encoding="utf-8")}, per_line) - assert not findings @@ -206,6 +205,22 @@ def test_docs_review_workflow_uses_matching_core_branch_when_available() -> None ) +def test_docs_review_checks_module_inventory_and_core_accountability() -> None: + workflow = (REPO_ROOT / ".github" / "workflows" / "docs-review.yml").read_text(encoding="utf-8") + + for path in ( + '"packages/**"', + '"registry/**"', + '"scripts/check-core-documentation-accountability.py"', + '"tests/unit/test_core_documentation_accountability.py"', + ): + assert path in workflow + assert "hatch run check-core-documentation-accountability" in workflow + assert workflow.index("hatch run check-core-documentation-accountability") < workflow.index( + "Upload docs review logs" + ) + + def test_iter_validation_docs_paths_scans_repo_wide_docs_tree() -> None: script = _load_script() diff --git a/tests/unit/test_core_documentation_accountability.py b/tests/unit/test_core_documentation_accountability.py new file mode 100644 index 00000000..47dd17cc --- /dev/null +++ b/tests/unit/test_core_documentation_accountability.py @@ -0,0 +1,90 @@ +from __future__ import annotations + +from pathlib import Path + +import pytest + +from tests.unit._script_test_utils import load_module_from_path + + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT_PATH = REPO_ROOT / "scripts" / "check-core-documentation-accountability.py" + + +def _load_script(): + return load_module_from_path("check_core_documentation_accountability", SCRIPT_PATH) + + +def _core_checkout(tmp_path: Path) -> Path: + checker = tmp_path / "scripts" / "check-documentation-accountability.py" + checker.parent.mkdir(parents=True) + checker.write_text("raise SystemExit(0)\n", encoding="utf-8") + return tmp_path + + +def test_resolve_core_checkout_prefers_explicit_environment_path(tmp_path: Path, monkeypatch) -> None: + script = _load_script() + core_root = _core_checkout(tmp_path / "core") + monkeypatch.setenv("SPECFACT_CLI_REPO", str(core_root)) + + assert script.resolve_core_checkout() == core_root.resolve() + + +@pytest.mark.parametrize( + ("modules_relative", "core_relative"), + ( + (Path("specfact-cli-modules"), Path("specfact-cli")), + ( + Path("specfact-cli-modules-worktrees") / "feature" / "docs-16", + Path("specfact-cli-worktrees") / "feature" / "docs-16", + ), + ), +) +def test_resolve_core_checkout_uses_documented_local_fallbacks( + tmp_path: Path, monkeypatch, modules_relative: Path, core_relative: Path +) -> None: + script = _load_script() + modules_root = tmp_path / modules_relative + modules_root.mkdir(parents=True) + core_root = _core_checkout(tmp_path / core_relative) + monkeypatch.delenv("SPECFACT_CLI_REPO", raising=False) + monkeypatch.setattr(script, "REPO_ROOT", modules_root) + + assert script.resolve_core_checkout() == core_root.resolve() + + +def test_resolve_core_checkout_fails_closed_with_setup_guidance(tmp_path: Path, monkeypatch) -> None: + script = _load_script() + monkeypatch.setenv("SPECFACT_CLI_REPO", str(tmp_path / "missing")) + monkeypatch.setattr(script, "REPO_ROOT", tmp_path / "modules") + + with pytest.raises(ValueError, match="Set SPECFACT_CLI_REPO"): + script.resolve_core_checkout() + + +def test_run_accountability_propagates_core_checker_failure(tmp_path: Path, monkeypatch) -> None: + script = _load_script() + core_root = _core_checkout(tmp_path / "core") + modules_root = tmp_path / "modules" + modules_root.mkdir() + calls: list[list[str]] = [] + + class _Result: + returncode = 7 + + def fake_run(command: list[str], check: bool) -> _Result: + calls.append(command) + assert check is False + return _Result() + + monkeypatch.setattr(script.subprocess, "run", fake_run) + + assert script.run_accountability(core_root, modules_root) == 7 + assert calls == [ + [ + script.sys.executable, + str(core_root / "scripts" / "check-documentation-accountability.py"), + "--modules-repo", + str(modules_root.resolve()), + ] + ] diff --git a/tests/unit/test_pre_commit_quality_parity.py b/tests/unit/test_pre_commit_quality_parity.py index 9bfb5a76..0415104f 100644 --- a/tests/unit/test_pre_commit_quality_parity.py +++ b/tests/unit/test_pre_commit_quality_parity.py @@ -37,12 +37,15 @@ "block1-yaml", "run_block2", "run_docs_site_validation_gate", + "run_core_documentation_accountability_gate", "hatch run python scripts/check-docs-commands.py", + "hatch run check-core-documentation-accountability", "SPECFACT_CODE_REVIEW_ENFORCEMENT", "enforcement=${enforcement}", "needs_docs_site_validation", "Command overview inputs have unstaged changes", - "git diff --name-only -- packages scripts/generate-command-overview.py scripts/check-command-contract.py pyproject.toml", + "git diff --name-only -- packages registry scripts/generate-command-overview.py " + "scripts/check-command-contract.py pyproject.toml", "usage_error", "show_help", "also: -h | --help | help", @@ -131,6 +134,13 @@ def test_modules_pre_commit_script_enforces_required_quality_commands() -> None: assert fragment in script_text +def test_pre_commit_treats_all_module_and_registry_changes_as_docs_relevant() -> None: + script_text = (REPO_ROOT / "scripts" / "pre-commit-quality-checks.sh").read_text(encoding="utf-8") + + assert "packages/**|registry/**" in script_text + assert script_text.index("run_core_documentation_accountability_gate") < script_text.index("check_safe_change") + + def test_code_review_gate_parses_staged_added_lines() -> None: review_gate = _load_pre_commit_code_review_module() diff_text = """\ diff --git a/tests/unit/test_pre_commit_verify_modules_signature_script.py b/tests/unit/test_pre_commit_verify_modules_signature_script.py index d2186961..f4d796bf 100644 --- a/tests/unit/test_pre_commit_verify_modules_signature_script.py +++ b/tests/unit/test_pre_commit_verify_modules_signature_script.py @@ -2,8 +2,11 @@ from pathlib import Path +from tests.unit._script_test_utils import load_module_from_path + REPO_ROOT = Path(__file__).resolve().parents[2] +SIGN_SCRIPT_PATH = REPO_ROOT / "scripts" / "sign-modules.py" def _pre_commit_verify_script_text() -> str: @@ -37,11 +40,29 @@ def test_pre_commit_verify_modules_signature_script_omit_branch_remediation_shap omit_block = _tail.split("omit)", 1)[1].split("*)", 1)[0] assert "--require-signature" not in omit_block assert "--metadata-only" not in omit_block - assert '"${_base[@]}"' in omit_block + assert "--allow-missing-public-key" in omit_block assert "sign-modules.py" in omit_block - assert "--changed-only" in omit_block + assert "--staged-only" in omit_block assert "--bump-version patch" in omit_block assert "--allow-unsigned" in omit_block assert "_stage_manifests_from_sign_output" in omit_block - assert "HEAD~1" in omit_block - assert "_failed_manifests" in omit_block + assert "git diff --cached" in text + assert "HEAD~1" not in omit_block + assert "_failed_manifests" not in omit_block + + +def test_sign_modules_staged_change_detection_reads_only_the_index(monkeypatch) -> None: + sign_script = load_module_from_path("sign_modules_staged_only", SIGN_SCRIPT_PATH) + commands: list[list[str]] = [] + + class _Result: + stdout = "packages/specfact-example/src/example.py\n" + + def fake_run(command: list[str], **_kwargs) -> _Result: + commands.append(command) + return _Result() + + monkeypatch.setattr(sign_script.subprocess, "run", fake_run) + + assert sign_script._module_has_staged_changes(Path("packages/specfact-example")) # pylint: disable=protected-access + assert commands == [["git", "diff", "--cached", "--name-only", "--", "packages/specfact-example"]] diff --git a/tests/unit/test_verify_modules_signature_script.py b/tests/unit/test_verify_modules_signature_script.py index eb25349d..82994610 100644 --- a/tests/unit/test_verify_modules_signature_script.py +++ b/tests/unit/test_verify_modules_signature_script.py @@ -70,6 +70,43 @@ def test_verify_manifest_falls_back_to_filesystem_payload_when_checksum_matches( assert verification_mode == "filesystem" +def test_verify_manifest_allows_unverified_optional_signature_without_public_key(tmp_path: Path) -> None: + verify_script = _load_verify_script() + module_dir = tmp_path / "packages" / "specfact-example" + module_dir.mkdir(parents=True) + (module_dir / "src.py").write_text("VALUE = 1\n", encoding="utf-8") + manifest_path = module_dir / "module-package.yaml" + manifest_path.write_text( + yaml.safe_dump({"name": "nold-ai/specfact-example", "version": "0.1.0"}, sort_keys=False), + encoding="utf-8", + ) + payload = verify_script._module_payload(module_dir, payload_from_filesystem=True) # pylint: disable=protected-access + manifest_path.write_text( + yaml.safe_dump( + { + "name": "nold-ai/specfact-example", + "version": "0.1.0", + "integrity": { + "checksum": f"sha256:{verify_script.hashlib.sha256(payload).hexdigest()}", + "signature": "a" * 44, + }, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + + verification_mode = verify_script.verify_manifest( + manifest_path, + require_signature=False, + public_key_pem="", + payload_from_filesystem=True, + allow_missing_public_key=True, + ) + + assert verification_mode == "filesystem" + + def test_verify_manifest_integrity_shape_only_accepts_checksum_only_manifest(tmp_path: Path) -> None: verify_script = _load_verify_script() module_dir = tmp_path / "packages" / "specfact-example" From b578ae40c71c7c061a6c046272cf10420e39b6fe Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Fri, 10 Jul 2026 23:41:23 +0200 Subject: [PATCH 2/4] fix: address documentation accountability review --- docs/authoring/module-signing.md | 2 +- openspec/CHANGE_ORDER.md | 1 + .../CHANGE_VALIDATION.md | 18 +++++----- .../TDD_EVIDENCE.md | 7 ++-- .../design.md | 4 +-- .../documentation-accountability/spec.md | 6 ++-- ...check-core-documentation-accountability.py | 7 ++-- scripts/generate-command-overview.py | 2 ++ scripts/pre-commit-quality-checks.sh | 2 +- scripts/sign-modules.py | 34 +++++++++++++++++-- .../unit/docs/test_llms_overview_freshness.py | 20 +++++++++++ .../test_core_documentation_accountability.py | 13 +++++++ tests/unit/test_pre_commit_quality_parity.py | 1 + ..._commit_verify_modules_signature_script.py | 12 ++++++- 14 files changed, 104 insertions(+), 25 deletions(-) diff --git a/docs/authoring/module-signing.md b/docs/authoring/module-signing.md index 7cc23986..72ae7065 100644 --- a/docs/authoring/module-signing.md +++ b/docs/authoring/module-signing.md @@ -147,7 +147,7 @@ Workflow **`sign-modules-on-approval.yml`** runs when a review is **submitted** ### Pre-commit -The first pre-commit hook runs **`scripts/pre-commit-verify-modules-signature.sh`**, which mirrors CI: **`--require-signature` on branch `main`**, or when **`GITHUB_BASE_REF=main`** in Actions pull-request contexts; otherwise it verifies payload checksums and version bumps without requiring a signature. On non-main branches, an existing signature with no locally available public key is not treated as checksum drift. If repair is needed, the hook runs `sign-modules.py --staged-only --allow-unsigned --payload-from-filesystem` and re-stages only manifests for module payloads staged in the pending commit; it never rewrites unrelated manifests. It does **not** rewrite **`registry/`** (publish workflows own signed artifacts and index updates). **`yaml-lint`** allows a semver **ahead** manifest vs **`registry/index.json`** until **`publish-modules`** reconciles. +The first pre-commit hook runs **`scripts/pre-commit-verify-modules-signature.sh`**, which mirrors CI: **`--require-signature` on branch `main`**, or when **`GITHUB_BASE_REF=main`** in Actions pull-request contexts; otherwise it verifies payload checksums and version bumps without requiring a signature. On non-main branches, an existing signature with no locally available public key is not treated as checksum drift. If repair is needed, the hook runs `sign-modules.py --staged-only --base-ref HEAD --bump-version patch --allow-unsigned --payload-from-filesystem` (patch-bumping versions that still match `HEAD`) and re-stages only manifests for module payloads staged in the pending commit; it never rewrites unrelated manifests. It does **not** rewrite **`registry/`** (publish workflows own signed artifacts and index updates). **`yaml-lint`** allows a semver **ahead** manifest vs **`registry/index.json`** until **`publish-modules`** reconciles. ## Rotation Procedure diff --git a/openspec/CHANGE_ORDER.md b/openspec/CHANGE_ORDER.md index 31d4f1ed..74fc3d3f 100644 --- a/openspec/CHANGE_ORDER.md +++ b/openspec/CHANGE_ORDER.md @@ -136,6 +136,7 @@ ceremony rather than validation evidence: ### Wave 4 - Gated Extensions +- `docs-16-core-accountability-sync` - `architecture-02-module-well-architected` - `docs-14-module-release-history` diff --git a/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md index 1486bf23..7c53adea 100644 --- a/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md +++ b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md @@ -28,9 +28,9 @@ ## Validation Outcome `openspec validate docs-16-core-accountability-sync --strict` passed with zero -issues. Production behavior remains intentionally unimplemented; the unchecked -task list preserves the required `spec -> tests -> failing evidence -> code -> -passing evidence` sequence. +issues. Production behavior is implemented, every change task is complete, and +the recorded evidence follows the required `spec -> tests -> failing evidence +-> code -> passing evidence` sequence. ## Proposal Quality Evidence @@ -57,11 +57,9 @@ passing evidence` sequence. ## Final Code-Review Evidence -At 2026-07-10 23:05 Europe/Berlin, `hatch run specfact code review run` with +At 2026-07-10 23:40 Europe/Berlin, `hatch run specfact code review run` with `--enforcement changed --bug-hunt --json` reported zero errors and zero -warnings. Four informational advisory notes remain: the two CLI argument -parser helpers are long with low branch complexity, the no-dependency -`icontract` fallback has an intentionally optional description parameter, and -one pre-existing verification test is long with low branch complexity. They -are recorded as non-blocking design observations, not unresolved review -findings. +warnings. Two informational advisory notes remain: the signer CLI argument +parser is long with low branch complexity, and the no-dependency `icontract` +fallback has an intentionally optional description parameter. They are recorded +as non-blocking design observations, not unresolved review findings. diff --git a/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md index 628514b5..f3359ca4 100644 --- a/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md +++ b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md @@ -38,7 +38,7 @@ hatch run pytest tests/unit/test_core_documentation_accountability.py \ Result: passed (50 passed). -- The modules wrapper resolves explicit, sibling, and paired-worktree core +- The modules wrapper resolves explicit, paired-worktree, and sibling core checkouts, fails closed with setup guidance, and delegates to the core-owned checker. - The command overview rejects an official manifest/registry record with no @@ -83,7 +83,10 @@ review and contract stages without changing the real staging area. - `openspec validate docs-16-core-accountability-sync --strict` passed. - `hatch run specfact code review run --enforcement changed --bug-hunt --json --out .specfact/code-review.json` reported zero errors and zero warnings. - Its four remaining entries are informational design advisories only. + Its two remaining entries are informational design advisories only. +- Follow-up review regressions passed: 22 tests covering paired-worktree + precedence, duplicate official registry entries, optional signer contracts, + and pre-commit matcher routing. - The initial `hatch run test` run completed 842 passing tests but could not complete cleanly in this environment: Semgrep failed to initialize its system CA trust store, producing 17 unrelated fixture failures. A docs-workflow path diff --git a/openspec/changes/docs-16-core-accountability-sync/design.md b/openspec/changes/docs-16-core-accountability-sync/design.md index d5d12a3f..e076b25d 100644 --- a/openspec/changes/docs-16-core-accountability-sync/design.md +++ b/openspec/changes/docs-16-core-accountability-sync/design.md @@ -16,8 +16,8 @@ official records fail rather than pass with unchanged generated output. ## Decisions 1. **Use a subprocess wrapper, not a copied checker.** The wrapper resolves - `SPECFACT_CLI_REPO`, then a sibling `specfact-cli` checkout, then the mapped - paired worktree. It invokes the core checker with the current modules root. + `SPECFACT_CLI_REPO`, then the matching paired worktree, then a sibling + `specfact-cli` checkout. It invokes the core checker with the current modules root. Missing checkout or checker is a non-zero, actionable setup failure. 2. **Keep CI branch pairing deterministic.** Docs Review first uses a matching core branch name; if unavailable, it uses only `dev` or `main` from the PR diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md index 3dc62959..e8cd2d2c 100644 --- a/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md +++ b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md @@ -5,8 +5,8 @@ The modules repository SHALL invoke the authoritative core documentation-accountability checker against the current modules checkout. The modules-side integration SHALL resolve the core checkout from -`SPECFACT_CLI_REPO`, then the documented sibling checkout, then the matching -paired worktree, and SHALL fail closed with setup guidance when none supplies +`SPECFACT_CLI_REPO`, then the matching paired worktree, then the documented +sibling checkout, and SHALL fail closed with setup guidance when none supplies the required checker. It SHALL NOT duplicate official-module inventory or core catalogue validation rules. @@ -21,7 +21,7 @@ catalogue validation rules. #### Scenario: Paired core checkout is unavailable -- **GIVEN** `SPECFACT_CLI_REPO`, the sibling checkout, and the paired worktree +- **GIVEN** `SPECFACT_CLI_REPO`, the paired worktree, and the sibling checkout do not provide the core checker - **WHEN** the modules local gate runs - **THEN** it exits non-zero with instructions to configure or create a paired diff --git a/scripts/check-core-documentation-accountability.py b/scripts/check-core-documentation-accountability.py index f5233d20..696c05cc 100644 --- a/scripts/check-core-documentation-accountability.py +++ b/scripts/check-core-documentation-accountability.py @@ -33,15 +33,16 @@ def _core_candidates(explicit_path: str | None) -> list[Path]: configured = explicit_path or os.environ.get("SPECFACT_CLI_REPO", "").strip() if configured: candidates.append(Path(configured).expanduser()) + + paired_checkout = _paired_worktree_checkout() + if paired_checkout is not None: + candidates.append(paired_checkout) candidates.append(REPO_ROOT.parent / "specfact-cli") marker = "specfact-cli-modules-worktrees" if marker in REPO_ROOT.parts: marker_index = REPO_ROOT.parts.index(marker) candidates.append(Path(*REPO_ROOT.parts[:marker_index]) / "specfact-cli") - paired_checkout = _paired_worktree_checkout() - if paired_checkout is not None: - candidates.append(paired_checkout) return candidates diff --git a/scripts/generate-command-overview.py b/scripts/generate-command-overview.py index f1a952a1..e8419bde 100644 --- a/scripts/generate-command-overview.py +++ b/scripts/generate-command-overview.py @@ -109,6 +109,8 @@ def _official_registry_inventory() -> set[str]: package_id = entry.get("id") if not isinstance(package_id, str): raise ValueError(f"Invalid official registry entry: {registry_path}") + if package_id in package_ids: + raise ValueError(f"Duplicate official registry entry: {package_id} ({registry_path})") package_ids.add(package_id) return package_ids diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index 3fbf414c..5b6c3f99 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -78,7 +78,7 @@ staged_docs_validation_paths() { while IFS= read -r line; do [ -z "${line}" ] && continue case "${line}" in - packages/**|registry/**|docs/*|*.md|*.mdc|requirements-docs-ci.txt|pyproject.toml|.pre-commit-config.yaml|scripts/pre-commit-quality-checks.sh|scripts/check-core-documentation-accountability.py|scripts/check-docs-commands.py|scripts/check-prompt-commands.py|scripts/check-command-contract.py|scripts/docs_site_validation.py|scripts/generate-command-overview.py|docs/reference/commands.generated.*|llms.txt|.github/workflows/docs-review.yml|tests/unit/test_core_documentation_accountability.py|tests/unit/test_pre_commit_quality_parity.py|tests/unit/test_check_docs_commands_script.py|tests/unit/test_check_prompt_commands_script.py|tests/unit/docs/test_docs_review.py|tests/unit/docs/test_code_review_docs_parity.py|tests/unit/docs/test_llms_overview_freshness.py) + packages/**|registry/**|docs/*|*.md|*.mdc|requirements-docs-ci.txt|pyproject.toml|.pre-commit-config.yaml|scripts/pre-commit-quality-checks.sh|scripts/check-core-documentation-accountability.py|scripts/check-docs-commands.py|scripts/check-prompt-commands.py|scripts/check-command-contract.py|scripts/docs_site_validation.py|scripts/generate-command-overview.py|llms.txt|.github/workflows/docs-review.yml|tests/unit/test_core_documentation_accountability.py|tests/unit/test_pre_commit_quality_parity.py|tests/unit/test_check_docs_commands_script.py|tests/unit/test_check_prompt_commands_script.py|tests/unit/docs/test_docs_review.py|tests/unit/docs/test_code_review_docs_parity.py|tests/unit/docs/test_llms_overview_freshness.py) printf '%s\n' "${line}" ;; esac diff --git a/scripts/sign-modules.py b/scripts/sign-modules.py index 4ef93325..d3fb3641 100755 --- a/scripts/sign-modules.py +++ b/scripts/sign-modules.py @@ -11,11 +11,41 @@ import os import subprocess import sys +from collections.abc import Callable +from functools import wraps from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, TypeVar, cast import yaml -from icontract import ensure, require + + +_FuncT = TypeVar("_FuncT", bound=Callable[..., Any]) + +if TYPE_CHECKING: + from icontract import ensure, require +else: + try: + from icontract import ensure, require + except ImportError: # pragma: no cover - exercised in plain-python signing environments + + def require( + _condition: Callable[..., bool], + _description: str | None = None, + ) -> Callable[[_FuncT], _FuncT]: + def decorator(func: _FuncT) -> _FuncT: + @wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + return func(*args, **kwargs) + + return cast(_FuncT, wrapper) + + return decorator + + def ensure( + _condition: Callable[..., bool], + _description: str | None = None, + ) -> Callable[[_FuncT], _FuncT]: + return require(_condition, _description) try: diff --git a/tests/unit/docs/test_llms_overview_freshness.py b/tests/unit/docs/test_llms_overview_freshness.py index 9bd3dd9d..c04585b5 100644 --- a/tests/unit/docs/test_llms_overview_freshness.py +++ b/tests/unit/docs/test_llms_overview_freshness.py @@ -82,3 +82,23 @@ def test_command_overview_rejects_unrepresented_official_inventory(tmp_path: Pat with pytest.raises(ValueError, match="missing command mounts"): generator.validate_official_mount_inventory() + + +def test_command_overview_rejects_duplicate_official_registry_entries(tmp_path: Path, monkeypatch) -> None: + generator = load_module_from_path("generate_command_overview_duplicate_registry", GENERATOR) + registry = tmp_path / "registry" / "index.json" + registry.parent.mkdir() + registry.write_text( + """{ + "modules": [ + {"id": "nold-ai/specfact-example", "tier": "official", "publisher": {"name": "nold-ai"}}, + {"id": "nold-ai/specfact-example", "tier": "official", "publisher": {"name": "nold-ai"}} + ] +} +""", + encoding="utf-8", + ) + monkeypatch.setattr(generator, "REPO_ROOT", tmp_path) + + with pytest.raises(ValueError, match="Duplicate official registry entry: nold-ai/specfact-example"): + generator._official_registry_inventory() # pylint: disable=protected-access diff --git a/tests/unit/test_core_documentation_accountability.py b/tests/unit/test_core_documentation_accountability.py index 47dd17cc..e02ade87 100644 --- a/tests/unit/test_core_documentation_accountability.py +++ b/tests/unit/test_core_documentation_accountability.py @@ -53,6 +53,19 @@ def test_resolve_core_checkout_uses_documented_local_fallbacks( assert script.resolve_core_checkout() == core_root.resolve() +def test_resolve_core_checkout_prefers_matching_paired_worktree_over_sibling(tmp_path: Path, monkeypatch) -> None: + script = _load_script() + modules_root = tmp_path / "specfact-cli-modules-worktrees" / "feature" / "docs-16" + modules_root.mkdir(parents=True) + sibling_core = _core_checkout(tmp_path / "specfact-cli") + paired_core = _core_checkout(tmp_path / "specfact-cli-worktrees" / "feature" / "docs-16") + monkeypatch.delenv("SPECFACT_CLI_REPO", raising=False) + monkeypatch.setattr(script, "REPO_ROOT", modules_root) + + assert sibling_core.is_dir() + assert script.resolve_core_checkout() == paired_core.resolve() + + def test_resolve_core_checkout_fails_closed_with_setup_guidance(tmp_path: Path, monkeypatch) -> None: script = _load_script() monkeypatch.setenv("SPECFACT_CLI_REPO", str(tmp_path / "missing")) diff --git a/tests/unit/test_pre_commit_quality_parity.py b/tests/unit/test_pre_commit_quality_parity.py index 0415104f..73e07e91 100644 --- a/tests/unit/test_pre_commit_quality_parity.py +++ b/tests/unit/test_pre_commit_quality_parity.py @@ -138,6 +138,7 @@ def test_pre_commit_treats_all_module_and_registry_changes_as_docs_relevant() -> script_text = (REPO_ROOT / "scripts" / "pre-commit-quality-checks.sh").read_text(encoding="utf-8") assert "packages/**|registry/**" in script_text + assert "docs/reference/commands.generated.*" not in script_text assert script_text.index("run_core_documentation_accountability_gate") < script_text.index("check_safe_change") diff --git a/tests/unit/test_pre_commit_verify_modules_signature_script.py b/tests/unit/test_pre_commit_verify_modules_signature_script.py index f4d796bf..19267369 100644 --- a/tests/unit/test_pre_commit_verify_modules_signature_script.py +++ b/tests/unit/test_pre_commit_verify_modules_signature_script.py @@ -2,7 +2,7 @@ from pathlib import Path -from tests.unit._script_test_utils import load_module_from_path +from tests.unit._script_test_utils import block_contract_imports, load_module_from_path REPO_ROOT = Path(__file__).resolve().parents[2] @@ -51,6 +51,16 @@ def test_pre_commit_verify_modules_signature_script_omit_branch_remediation_shap assert "_failed_manifests" not in omit_block +def test_sign_modules_loads_without_icontract(monkeypatch) -> None: + block_contract_imports(monkeypatch) + + sign_script = load_module_from_path("sign_modules_without_icontract", SIGN_SCRIPT_PATH) + + assert callable(sign_script.main) + assert callable(sign_script.ensure) + assert callable(sign_script.require) + + def test_sign_modules_staged_change_detection_reads_only_the_index(monkeypatch) -> None: sign_script = load_module_from_path("sign_modules_staged_only", SIGN_SCRIPT_PATH) commands: list[list[str]] = [] From 31797b72b265b886b148b7f37d3a470e8e716c1f Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 11 Jul 2026 00:14:45 +0200 Subject: [PATCH 3/4] fix: harden docs accountability promotion --- .github/workflows/docs-review.yml | 8 +- .../CHANGE_VALIDATION.md | 4 +- .../TDD_EVIDENCE.md | 11 ++ .../documentation-accountability/spec.md | 10 +- .../modules-pre-commit-quality-parity/spec.md | 12 +- pyproject.toml | 2 +- ...check-core-documentation-accountability.py | 31 +++- .../pre-commit-verify-modules-signature.sh | 12 +- scripts/sign-modules.py | 174 ++++++++++++++---- tests/unit/test_check_docs_commands_script.py | 52 +++++- .../test_core_documentation_accountability.py | 30 ++- tests/unit/test_pre_commit_quality_parity.py | 3 +- ..._commit_verify_modules_signature_script.py | 106 ++++++++++- 13 files changed, 382 insertions(+), 73 deletions(-) diff --git a/.github/workflows/docs-review.yml b/.github/workflows/docs-review.yml index c46ab962..e7a955e6 100644 --- a/.github/workflows/docs-review.yml +++ b/.github/workflows/docs-review.yml @@ -124,7 +124,13 @@ jobs: run: | mkdir -p logs/docs-review DOCS_REVIEW_LOG="logs/docs-review/docs-review_$(date -u +%Y%m%d_%H%M%S).log" - python -m pytest tests/unit/docs/test_docs_review.py tests/unit/docs/test_code_review_docs_parity.py -q 2>&1 | tee "$DOCS_REVIEW_LOG" + python -m pytest \ + tests/unit/docs/test_docs_review.py \ + tests/unit/docs/test_code_review_docs_parity.py \ + tests/unit/test_core_documentation_accountability.py \ + tests/unit/test_pre_commit_quality_parity.py \ + tests/unit/docs/test_llms_overview_freshness.py \ + -q 2>&1 | tee "$DOCS_REVIEW_LOG" exit "${PIPESTATUS[0]:-$?}" - name: Validate docs commands and cross-site links diff --git a/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md index 7c53adea..879b39fc 100644 --- a/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md +++ b/openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md @@ -1,6 +1,6 @@ # Change Validation: docs-16-core-accountability-sync -- **Validated on:** 2026-07-10 Europe/Berlin +- **Validated on:** 2026-07-11 Europe/Berlin - **Workflow:** proposal-readiness review and strict OpenSpec validation - **Strict command:** `openspec validate docs-16-core-accountability-sync --strict` - **Result:** PASS (0 issues) @@ -57,7 +57,7 @@ the recorded evidence follows the required `spec -> tests -> failing evidence ## Final Code-Review Evidence -At 2026-07-10 23:40 Europe/Berlin, `hatch run specfact code review run` with +At 2026-07-11 Europe/Berlin, `hatch run specfact code review run` with `--enforcement changed --bug-hunt --json` reported zero errors and zero warnings. Two informational advisory notes remain: the signer CLI argument parser is long with low branch complexity, and the no-dependency `icontract` diff --git a/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md index f3359ca4..dada9da6 100644 --- a/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md +++ b/openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md @@ -153,3 +153,14 @@ Outside that sandbox, `semgrep --version && semgrep scan --config tools/semgrep --quiet tests/fixtures/semgrep/good_print_in_src.py` exits successfully. This is host-sandbox access to macOS certificate anchors, not a repository rule, source, or terminal configuration defect. + +## PR #341 Review Follow-up + +### 2026-07-11 Europe/Berlin + +Failing-before evidence covered a delegated-checker timeout, Git inspection +failures, staged-only signing with differing staged and unstaged module content, +and workflow step/coverage assertions. Passing-after evidence: 37 focused +tests, shell syntax validation, strict OpenSpec validation, accountability and +generated-artifact gates, contract tests, and changed-file review with zero +errors and warnings. diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md index e8cd2d2c..bab3570c 100644 --- a/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md +++ b/openspec/changes/docs-16-core-accountability-sync/specs/documentation-accountability/spec.md @@ -8,7 +8,8 @@ modules-side integration SHALL resolve the core checkout from `SPECFACT_CLI_REPO`, then the matching paired worktree, then the documented sibling checkout, and SHALL fail closed with setup guidance when none supplies the required checker. It SHALL NOT duplicate official-module inventory or core -catalogue validation rules. +catalogue validation rules. The delegated checker SHALL have a configurable, +positive execution timeout and SHALL fail closed when that timeout expires. #### Scenario: Module inventory change exposes stale core documentation @@ -28,6 +29,13 @@ catalogue validation rules. core checkout - **AND** it does not skip or substitute a duplicated local checker. +#### Scenario: Core checker exceeds its timeout + +- **WHEN** the delegated core checker does not finish before the configured + positive timeout +- **THEN** the modules gate exits non-zero with timeout guidance +- **AND** it does not continue to downstream documentation or safe-bypass work. + ### Requirement: Core accountability SHALL block matching local and PR gates The same modules-side core-accountability command SHALL run before a docs-only diff --git a/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md b/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md index d4df80d7..b4858d88 100644 --- a/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md +++ b/openspec/changes/docs-16-core-accountability-sync/specs/modules-pre-commit-quality-parity/spec.md @@ -31,7 +31,9 @@ version policy without rewriting manifests solely because an existing optional signature cannot be verified locally. If checksum/version remediation is needed, it SHALL target only module payloads staged for the pending commit. Unchanged, unstaged, or unrelated failed manifests SHALL not be passed as -explicit repair inputs. +explicit repair inputs. Staged-only payload checksums and manifest/version +inputs SHALL be derived from the staged Git index, not unstaged working-tree +content. #### Scenario: Docs-only commit has no module manifest mutation @@ -49,3 +51,11 @@ explicit repair inputs. - **THEN** automatic checksum/version repair may update only that staged module's manifest - **AND** the hook re-verifies without repairing unrelated manifests. + +#### Scenario: Staged repair ignores unstaged payload edits + +- **WHEN** a module has staged payload changes and different unstaged edits in + the same module +- **THEN** staged-only repair computes the integrity checksum from the staged + snapshot +- **AND** it fails rather than overwriting an unstaged manifest edit. diff --git a/pyproject.toml b/pyproject.toml index a21972b8..63b214e9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,7 +57,7 @@ validate-cli-contracts = "python tools/validate_cli_contracts.py" validate-prompt-commands = "python scripts/check-prompt-commands.py" generate-command-overview = "python tools/ensure_core_dependency.py && python scripts/generate-command-overview.py --write" check-command-overview = "python tools/ensure_core_dependency.py && python scripts/generate-command-overview.py --check" -check-core-documentation-accountability = "python scripts/check-core-documentation-accountability.py" +check-core-documentation-accountability = "python tools/ensure_core_dependency.py && python scripts/check-core-documentation-accountability.py" check-command-contract = "python tools/ensure_core_dependency.py && python scripts/check-command-contract.py" check-bundle-imports = "python scripts/check-bundle-imports.py" sign-modules = "python scripts/sign-modules.py {args}" diff --git a/scripts/check-core-documentation-accountability.py b/scripts/check-core-documentation-accountability.py index 696c05cc..82dfcba8 100644 --- a/scripts/check-core-documentation-accountability.py +++ b/scripts/check-core-documentation-accountability.py @@ -16,6 +16,7 @@ REPO_ROOT = Path(__file__).resolve().parents[1] CORE_CHECKER_RELATIVE_PATH = Path("scripts") / "check-documentation-accountability.py" +DEFAULT_CHECKER_TIMEOUT_SECONDS = 120 def _paired_worktree_checkout() -> Path | None: @@ -68,8 +69,14 @@ def resolve_core_checkout(explicit_path: str | None = None) -> Path: @beartype @require(lambda core_root, modules_root: core_root.is_dir() and modules_root.is_dir()) +@require(lambda timeout_seconds: timeout_seconds > 0, "timeout must be positive") @ensure(lambda result: isinstance(result, int)) -def run_accountability(core_root: Path, modules_root: Path) -> int: +def run_accountability( + core_root: Path, + modules_root: Path, + *, + timeout_seconds: int = DEFAULT_CHECKER_TIMEOUT_SECONDS, +) -> int: """Delegate validation to the core-owned checker without copying its rules.""" command = [ sys.executable, @@ -77,7 +84,24 @@ def run_accountability(core_root: Path, modules_root: Path) -> int: "--modules-repo", str(modules_root.resolve()), ] - return subprocess.run(command, check=False).returncode + try: + return subprocess.run(command, check=False, timeout=timeout_seconds).returncode + except subprocess.TimeoutExpired: + sys.stderr.write(f"core-documentation-accountability: checker timed out after {timeout_seconds} seconds\n") + return 1 + + +def _checker_timeout_seconds() -> int: + configured = os.environ.get("SPECFACT_CORE_ACCOUNTABILITY_TIMEOUT_SECONDS", "").strip() + if not configured: + return DEFAULT_CHECKER_TIMEOUT_SECONDS + try: + timeout_seconds = int(configured) + except ValueError as exc: + raise ValueError("SPECFACT_CORE_ACCOUNTABILITY_TIMEOUT_SECONDS must be a positive integer") from exc + if timeout_seconds <= 0: + raise ValueError("SPECFACT_CORE_ACCOUNTABILITY_TIMEOUT_SECONDS must be a positive integer") + return timeout_seconds @beartype @@ -88,10 +112,11 @@ def main(argv: list[str] | None = None) -> int: args = parser.parse_args(argv) try: core_root = resolve_core_checkout(args.core_repo) + timeout_seconds = _checker_timeout_seconds() except ValueError as exc: sys.stderr.write(f"core-documentation-accountability: {exc}\n") return 1 - return run_accountability(core_root, REPO_ROOT) + return run_accountability(core_root, REPO_ROOT, timeout_seconds=timeout_seconds) if __name__ == "__main__": diff --git a/scripts/pre-commit-verify-modules-signature.sh b/scripts/pre-commit-verify-modules-signature.sh index 17b035d4..834e5a63 100755 --- a/scripts/pre-commit-verify-modules-signature.sh +++ b/scripts/pre-commit-verify-modules-signature.sh @@ -65,11 +65,7 @@ case "${sig_policy}" in omit) echo "🔐 Verifying module manifests (formal: payload checksum + version bump; signatures not required on this branch — see docs/reference/module-security.md)" >&2 _omit_base=("${_base[@]}" --allow-missing-public-key) - set +e - _verify_out="$("${_omit_base[@]}" 2>&1)" - _verify_rc=$? - set -e - if ((_verify_rc == 0)); then + if _verify_out="$("${_omit_base[@]}" 2>&1)"; then exit 0 fi printf '%s\n' "${_verify_out}" >&2 @@ -103,11 +99,7 @@ case "${sig_policy}" in _stage_manifests_from_sign_output <"${_sign_log}" echo "🔐 Re-verifying after auto-remediation..." >&2 - set +e - _verify2_out="$("${_omit_base[@]}" 2>&1)" - _verify2_rc=$? - set -e - if ((_verify2_rc != 0)); then + if ! _verify2_out="$("${_omit_base[@]}" 2>&1)"; then printf '%s\n' "${_verify2_out}" >&2 echo "❌ Module verify still failing after staged-only remediation; no unrelated manifest will be rewritten." >&2 exit 1 diff --git a/scripts/sign-modules.py b/scripts/sign-modules.py index d3fb3641..ba351d84 100755 --- a/scripts/sign-modules.py +++ b/scripts/sign-modules.py @@ -96,6 +96,13 @@ def _sorted_hashable_files(paths: list[Path], *, module_dir: Path, ignored_dirs: ) +def _sorted_index_files(paths: list[Path], *, module_dir: Path, ignored_dirs: set[str]) -> list[Path]: + return sorted( + (path for path in paths if _is_hashable_module_file(path, module_dir=module_dir, ignored_dirs=ignored_dirs)), + key=lambda path: path.resolve().relative_to(module_dir).as_posix(), + ) + + def _git_module_files(module_dir: Path, *, ignored_dirs: set[str]) -> list[Path]: listed = subprocess.run( ["git", "ls-files", module_dir.as_posix()], @@ -107,8 +114,25 @@ def _git_module_files(module_dir: Path, *, ignored_dirs: set[str]) -> list[Path] return _sorted_hashable_files(files, module_dir=module_dir, ignored_dirs=ignored_dirs) -def _module_files(module_dir: Path, *, payload_from_filesystem: bool) -> list[Path]: +def _index_module_files(module_dir: Path, *, ignored_dirs: set[str]) -> list[Path]: + repo_root = Path.cwd().resolve() + try: + relative_module_dir = module_dir.resolve().relative_to(repo_root).as_posix() + listed = subprocess.run( + ["git", "ls-files", "--cached", "-z", "--", relative_module_dir], + check=True, + capture_output=True, + ).stdout.split(b"\0") + except (subprocess.CalledProcessError, OSError, ValueError) as exc: + raise ValueError(f"Unable to read staged module payload for {module_dir}: {exc}") from exc + files = [repo_root / path.decode("utf-8") for path in listed if path] + return _sorted_index_files(files, module_dir=module_dir, ignored_dirs=ignored_dirs) + + +def _module_files(module_dir: Path, *, payload_from_filesystem: bool, staged_snapshot: bool) -> list[Path]: ignored_dirs = _PAYLOAD_FROM_FS_IGNORED_DIRS if payload_from_filesystem else _IGNORED_MODULE_DIR_NAMES + if staged_snapshot: + return _index_module_files(module_dir, ignored_dirs=ignored_dirs) if payload_from_filesystem: return _sorted_hashable_files(list(module_dir.rglob("*")), module_dir=module_dir, ignored_dirs=ignored_dirs) try: @@ -117,24 +141,48 @@ def _module_files(module_dir: Path, *, payload_from_filesystem: bool) -> list[Pa return _sorted_hashable_files(list(module_dir.rglob("*")), module_dir=module_dir, ignored_dirs=ignored_dirs) -def _payload_file_data(path: Path, *, module_dir: Path) -> bytes: +def _index_file_bytes(path: Path) -> bytes: + try: + relative_path = path.resolve().relative_to(Path.cwd().resolve()).as_posix() + return subprocess.run( + ["git", "show", f":{relative_path}"], + check=True, + capture_output=True, + ).stdout + except (subprocess.CalledProcessError, OSError, ValueError) as exc: + raise ValueError(f"Unable to read staged file {path}: {exc}") from exc + + +def _payload_file_data(path: Path, *, module_dir: Path, staged_snapshot: bool) -> bytes: rel = path.resolve().relative_to(module_dir).as_posix() if rel not in {"module-package.yaml", "metadata.yaml"}: - return path.read_bytes() - raw = yaml.safe_load(path.read_text(encoding="utf-8")) + return _index_file_bytes(path) if staged_snapshot else path.read_bytes() + content = _index_file_bytes(path) if staged_snapshot else path.read_bytes() + raw = yaml.safe_load(content) if not isinstance(raw, dict): raise ValueError(f"Invalid manifest YAML: {path}") return _canonical_payload(raw) -def _module_payload(module_dir: Path, payload_from_filesystem: bool = False) -> bytes: +def _module_payload( + module_dir: Path, + payload_from_filesystem: bool = False, + *, + staged_snapshot: bool = False, +) -> bytes: if not module_dir.is_dir(): raise ValueError(f"Module directory not found: {module_dir}") resolved_module_dir = module_dir.resolve() entries = [] - for path in _module_files(resolved_module_dir, payload_from_filesystem=payload_from_filesystem): + for path in _module_files( + resolved_module_dir, + payload_from_filesystem=payload_from_filesystem, + staged_snapshot=staged_snapshot, + ): relative_path = path.resolve().relative_to(resolved_module_dir).as_posix() - digest = hashlib.sha256(_payload_file_data(path, module_dir=resolved_module_dir)).hexdigest() + digest = hashlib.sha256( + _payload_file_data(path, module_dir=resolved_module_dir, staged_snapshot=staged_snapshot) + ).hexdigest() entries.append(f"{relative_path}:{digest}") return "\n".join(entries).encode("utf-8") @@ -233,10 +281,16 @@ def _resolve_passphrase(args: argparse.Namespace) -> str | None: return None -def _read_manifest_version(path: Path) -> str | None: - raw = yaml.safe_load(path.read_text(encoding="utf-8")) +def _manifest_data(path: Path, *, staged_snapshot: bool) -> dict[str, Any]: + content = _index_file_bytes(path) if staged_snapshot else path.read_bytes() + raw = yaml.safe_load(content) if not isinstance(raw, dict): - return None + raise ValueError(f"Invalid manifest YAML: {path}") + return raw + + +def _read_manifest_version(path: Path, *, staged_snapshot: bool = False) -> str | None: + raw = _manifest_data(path, staged_snapshot=staged_snapshot) value = raw.get("version") if value is None: return None @@ -305,11 +359,24 @@ def _module_has_git_changes_since(module_dir: Path, git_ref: str) -> bool: capture_output=True, text=True, ).stdout.strip() - except (subprocess.CalledProcessError, OSError): - return False + except (subprocess.CalledProcessError, OSError) as exc: + raise ValueError(f"Unable to inspect module changes since {git_ref}: {exc}") from exc return bool(changed or untracked) +def _module_has_index_changes_since(module_dir: Path, git_ref: str) -> bool: + try: + changed = subprocess.run( + ["git", "diff", "--cached", "--name-only", git_ref, "--", module_dir.as_posix()], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + except (subprocess.CalledProcessError, OSError) as exc: + raise ValueError(f"Unable to inspect staged module changes since {git_ref}: {exc}") from exc + return bool(changed) + + def _module_has_staged_changes(module_dir: Path) -> bool: try: changed = subprocess.run( @@ -318,8 +385,8 @@ def _module_has_staged_changes(module_dir: Path) -> bool: capture_output=True, text=True, ).stdout.strip() - except (subprocess.CalledProcessError, OSError): - return False + except (subprocess.CalledProcessError, OSError) as exc: + raise ValueError(f"Unable to inspect staged module changes for {module_dir}: {exc}") from exc return bool(changed) @@ -355,12 +422,31 @@ def _write_manifest(path: Path, data: dict[str, Any]) -> None: ) +def _has_unstaged_manifest_changes(manifest_path: Path) -> bool: + try: + changed = subprocess.run( + ["git", "diff", "--name-only", "--", manifest_path.as_posix()], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + except (subprocess.CalledProcessError, OSError) as exc: + raise ValueError(f"Unable to inspect unstaged manifest changes for {manifest_path}: {exc}") from exc + return bool(changed) + + def _report(message: str) -> None: sys.stdout.write(f"{message}\n") -def _auto_bump_manifest_version(manifest_path: Path, *, base_ref: str, bump_type: str) -> bool: - current_version = _read_manifest_version(manifest_path) +def _auto_bump_manifest_version( + manifest_path: Path, + *, + base_ref: str, + bump_type: str, + staged_snapshot: bool, +) -> bool: + current_version = _read_manifest_version(manifest_path, staged_snapshot=staged_snapshot) if not current_version: raise ValueError(f"Manifest missing version: {manifest_path}") @@ -368,9 +454,7 @@ def _auto_bump_manifest_version(manifest_path: Path, *, base_ref: str, bump_type if previous_version is None or current_version != previous_version: return False - raw = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) - if not isinstance(raw, dict): - raise ValueError(f"Invalid manifest YAML: {manifest_path}") + raw = _manifest_data(manifest_path, staged_snapshot=staged_snapshot) bumped = _bump_semver(current_version, bump_type) raw["version"] = bumped _write_manifest(manifest_path, raw) @@ -379,12 +463,16 @@ def _auto_bump_manifest_version(manifest_path: Path, *, base_ref: str, bump_type def _enforce_version_bump_before_signing( - manifest_path: Path, *, allow_same_version: bool, comparison_ref: str = "HEAD" + manifest_path: Path, + *, + allow_same_version: bool, + comparison_ref: str = "HEAD", + staged_snapshot: bool, ) -> None: if allow_same_version: return - current_version = _read_manifest_version(manifest_path) + current_version = _read_manifest_version(manifest_path, staged_snapshot=staged_snapshot) if not current_version: raise ValueError(f"Manifest missing version: {manifest_path}") @@ -395,7 +483,12 @@ def _enforce_version_bump_before_signing( return module_dir = manifest_path.parent - if not _module_has_git_changes_since(module_dir, comparison_ref): + has_changes = ( + _module_has_index_changes_since(module_dir, comparison_ref) + if staged_snapshot + else _module_has_git_changes_since(module_dir, comparison_ref) + ) + if not has_changes: return raise ValueError( @@ -419,13 +512,22 @@ def _sign_payload(payload: bytes, private_key: Any) -> str: @require(lambda manifest_path: manifest_path.is_file(), "manifest_path must be a file") @ensure(lambda result: result is None, "sign_manifest returns None") -def sign_manifest(manifest_path: Path, private_key: Any | None, *, payload_from_filesystem: bool = False) -> None: - raw = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) - if not isinstance(raw, dict): - msg = f"Invalid manifest YAML: {manifest_path}" - raise ValueError(msg) - - payload = _module_payload(manifest_path.parent, payload_from_filesystem=payload_from_filesystem) +def sign_manifest( + manifest_path: Path, + private_key: Any | None, + *, + payload_from_filesystem: bool = False, + staged_snapshot: bool = False, +) -> None: + if staged_snapshot and _has_unstaged_manifest_changes(manifest_path): + raise ValueError(f"Refusing to overwrite unstaged manifest changes: {manifest_path}") + raw = _manifest_data(manifest_path, staged_snapshot=staged_snapshot) + + payload = _module_payload( + manifest_path.parent, + payload_from_filesystem=payload_from_filesystem, + staged_snapshot=staged_snapshot, + ) checksum = f"sha256:{hashlib.sha256(payload).hexdigest()}" integrity: dict[str, str] = {"checksum": checksum} @@ -544,13 +646,20 @@ def _sign_selected_manifests( manifest_path, base_ref=args.base_ref, bump_type=args.bump_version, + staged_snapshot=args.staged_only, ) _enforce_version_bump_before_signing( manifest_path, allow_same_version=args.allow_same_version, comparison_ref=args.base_ref if args.changed_only else "HEAD", + staged_snapshot=args.staged_only, + ) + sign_manifest( + manifest_path, + private_key, + payload_from_filesystem=args.payload_from_filesystem, + staged_snapshot=args.staged_only, ) - sign_manifest(manifest_path, private_key, payload_from_filesystem=args.payload_from_filesystem) except ValueError as exc: parser.error(str(exc)) @@ -576,7 +685,10 @@ def main() -> int: "For local testing only, re-run with --allow-unsigned." ) - manifests = _select_manifests(args, parser) + try: + manifests = _select_manifests(args, parser) + except ValueError as exc: + parser.error(str(exc)) if (args.changed_only or args.staged_only) and not manifests: scope = f"since {args.base_ref}" if args.changed_only else "in the staged index" diff --git a/tests/unit/test_check_docs_commands_script.py b/tests/unit/test_check_docs_commands_script.py index 589e3f7e..7ba0aae6 100644 --- a/tests/unit/test_check_docs_commands_script.py +++ b/tests/unit/test_check_docs_commands_script.py @@ -1,8 +1,10 @@ from __future__ import annotations from pathlib import Path +from typing import cast import pytest +import yaml from tests.unit._script_test_utils import load_module_from_path @@ -205,20 +207,50 @@ def test_docs_review_workflow_uses_matching_core_branch_when_available() -> None ) +def _docs_review_workflow() -> dict[str, object]: + workflow = yaml.load( + (REPO_ROOT / ".github" / "workflows" / "docs-review.yml").read_text(encoding="utf-8"), + Loader=yaml.BaseLoader, + ) + assert isinstance(workflow, dict) + return workflow + + +def _docs_review_steps(workflow: dict[str, object]) -> list[dict[str, str]]: + jobs = cast(dict[str, dict[str, list[dict[str, str]]]], workflow["jobs"]) + return jobs["docs-review"]["steps"] + + +def _step_named(steps: list[dict[str, str]], name: str) -> tuple[int, dict[str, str]]: + return next((index, step) for index, step in enumerate(steps) if step.get("name") == name) + + def test_docs_review_checks_module_inventory_and_core_accountability() -> None: - workflow = (REPO_ROOT / ".github" / "workflows" / "docs-review.yml").read_text(encoding="utf-8") + workflow = _docs_review_workflow() + triggers = cast(dict[str, dict[str, list[str]]], workflow["on"]) for path in ( - '"packages/**"', - '"registry/**"', - '"scripts/check-core-documentation-accountability.py"', - '"tests/unit/test_core_documentation_accountability.py"', + "packages/**", + "registry/**", + "scripts/check-core-documentation-accountability.py", + "tests/unit/test_core_documentation_accountability.py", ): - assert path in workflow - assert "hatch run check-core-documentation-accountability" in workflow - assert workflow.index("hatch run check-core-documentation-accountability") < workflow.index( - "Upload docs review logs" - ) + assert path in triggers["pull_request"]["paths"] + assert path in triggers["push"]["paths"] + + steps = _docs_review_steps(workflow) + _review_index, review_step = _step_named(steps, "Run docs review suite") + for test_path in ( + "tests/unit/test_core_documentation_accountability.py", + "tests/unit/test_pre_commit_quality_parity.py", + "tests/unit/docs/test_llms_overview_freshness.py", + ): + assert test_path in review_step["run"] + accountability_index, accountability_step = _step_named(steps, "Validate core documentation accountability") + upload_index, _upload_step = _step_named(steps, "Upload docs review logs") + assert accountability_step["run"] == "hatch run check-core-documentation-accountability" + assert accountability_step.get("continue-on-error") not in {True, "true"} + assert accountability_index < upload_index def test_iter_validation_docs_paths_scans_repo_wide_docs_tree() -> None: diff --git a/tests/unit/test_core_documentation_accountability.py b/tests/unit/test_core_documentation_accountability.py index e02ade87..0e744707 100644 --- a/tests/unit/test_core_documentation_accountability.py +++ b/tests/unit/test_core_documentation_accountability.py @@ -1,5 +1,7 @@ from __future__ import annotations +import subprocess +import tomllib from pathlib import Path import pytest @@ -85,9 +87,9 @@ def test_run_accountability_propagates_core_checker_failure(tmp_path: Path, monk class _Result: returncode = 7 - def fake_run(command: list[str], check: bool) -> _Result: + def fake_run(command: list[str], **kwargs: object) -> _Result: calls.append(command) - assert check is False + assert kwargs == {"check": False, "timeout": 120} return _Result() monkeypatch.setattr(script.subprocess, "run", fake_run) @@ -101,3 +103,27 @@ def fake_run(command: list[str], check: bool) -> _Result: str(modules_root.resolve()), ] ] + + +def test_run_accountability_fails_closed_when_core_checker_times_out(tmp_path: Path, monkeypatch, capsys) -> None: + script = _load_script() + core_root = _core_checkout(tmp_path / "core") + modules_root = tmp_path / "modules" + modules_root.mkdir() + + def fake_run(command: list[str], **_kwargs: object) -> None: + raise subprocess.TimeoutExpired(command, timeout=3) + + monkeypatch.setattr(script.subprocess, "run", fake_run) + + assert script.run_accountability(core_root, modules_root, timeout_seconds=3) == 1 + assert "timed out after 3 seconds" in capsys.readouterr().err + + +def test_hatch_accountability_command_bootstraps_core_dependency() -> None: + pyproject = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) + command = pyproject["tool"]["hatch"]["envs"]["default"]["scripts"]["check-core-documentation-accountability"] + + assert ( + command == "python tools/ensure_core_dependency.py && python scripts/check-core-documentation-accountability.py" + ) diff --git a/tests/unit/test_pre_commit_quality_parity.py b/tests/unit/test_pre_commit_quality_parity.py index 73e07e91..23653872 100644 --- a/tests/unit/test_pre_commit_quality_parity.py +++ b/tests/unit/test_pre_commit_quality_parity.py @@ -139,7 +139,8 @@ def test_pre_commit_treats_all_module_and_registry_changes_as_docs_relevant() -> assert "packages/**|registry/**" in script_text assert "docs/reference/commands.generated.*" not in script_text - assert script_text.index("run_core_documentation_accountability_gate") < script_text.index("check_safe_change") + run_block2 = script_text.split("run_block2() {", 1)[1].split("\n}\n\nrun_all()", 1)[0] + assert run_block2.index("run_core_documentation_accountability_gate") < run_block2.index("check_safe_change") def test_code_review_gate_parses_staged_added_lines() -> None: diff --git a/tests/unit/test_pre_commit_verify_modules_signature_script.py b/tests/unit/test_pre_commit_verify_modules_signature_script.py index 19267369..1a1e265b 100644 --- a/tests/unit/test_pre_commit_verify_modules_signature_script.py +++ b/tests/unit/test_pre_commit_verify_modules_signature_script.py @@ -1,7 +1,13 @@ from __future__ import annotations +import hashlib +import subprocess +import sys from pathlib import Path +import pytest +import yaml + from tests.unit._script_test_utils import block_contract_imports, load_module_from_path @@ -38,17 +44,19 @@ def test_pre_commit_verify_modules_signature_script_omit_branch_remediation_shap marker = 'case "${sig_policy}" in' _tail = text.split(marker, 1)[1] omit_block = _tail.split("omit)", 1)[1].split("*)", 1)[0] - assert "--require-signature" not in omit_block - assert "--metadata-only" not in omit_block - assert "--allow-missing-public-key" in omit_block - assert "sign-modules.py" in omit_block - assert "--staged-only" in omit_block - assert "--bump-version patch" in omit_block - assert "--allow-unsigned" in omit_block - assert "_stage_manifests_from_sign_output" in omit_block + required = { + "--allow-missing-public-key", + "sign-modules.py", + "--staged-only", + "--bump-version patch", + "--allow-unsigned", + "_stage_manifests_from_sign_output", + } + forbidden = {"--require-signature", "--metadata-only", "HEAD~1", "_failed_manifests", "set +e"} + + assert all(fragment in omit_block for fragment in required) + assert all(fragment not in omit_block for fragment in forbidden) assert "git diff --cached" in text - assert "HEAD~1" not in omit_block - assert "_failed_manifests" not in omit_block def test_sign_modules_loads_without_icontract(monkeypatch) -> None: @@ -76,3 +84,81 @@ def fake_run(command: list[str], **_kwargs) -> _Result: assert sign_script._module_has_staged_changes(Path("packages/specfact-example")) # pylint: disable=protected-access assert commands == [["git", "diff", "--cached", "--name-only", "--", "packages/specfact-example"]] + + +def test_sign_modules_fails_closed_when_staged_change_detection_cannot_run(monkeypatch) -> None: + sign_script = load_module_from_path("sign_modules_staged_failure", SIGN_SCRIPT_PATH) + + def failed_git(*_args, **_kwargs): + raise subprocess.CalledProcessError(128, ["git", "diff"]) + + monkeypatch.setattr(sign_script.subprocess, "run", failed_git) + + with pytest.raises(ValueError, match="Unable to inspect staged module changes"): + sign_script._module_has_staged_changes(Path("packages/specfact-example")) # pylint: disable=protected-access + + +def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True) + + +def _index_payload(repo_root: Path, module_relative: Path) -> bytes: + listed = _git(repo_root, "ls-files", "--cached", "-z", "--", module_relative.as_posix()).stdout.split("\0") + entries: list[str] = [] + for relative in sorted(path for path in listed if path): + content = subprocess.run( + ["git", "show", f":{relative}"], + cwd=repo_root, + check=True, + capture_output=True, + ).stdout + module_path = Path(relative).relative_to(module_relative).as_posix() + if module_path in {"module-package.yaml", "metadata.yaml"}: + manifest = yaml.safe_load(content) + assert isinstance(manifest, dict) + manifest.pop("integrity", None) + content = yaml.safe_dump(manifest, sort_keys=True, allow_unicode=False).encode("utf-8") + entries.append(f"{module_path}:{hashlib.sha256(content).hexdigest()}") + return "\n".join(entries).encode("utf-8") + + +def test_staged_only_signing_hashes_the_index_not_unstaged_module_content(tmp_path: Path) -> None: + module_relative = Path("packages/specfact-example") + module_dir = tmp_path / module_relative + module_dir.mkdir(parents=True) + manifest_path = module_dir / "module-package.yaml" + manifest_path.write_text("name: nold-ai/specfact-example\nversion: 0.1.0\n", encoding="utf-8") + source_path = module_dir / "src.py" + source_path.write_text("VALUE = 'base'\n", encoding="utf-8") + _git(tmp_path, "init") + _git(tmp_path, "config", "user.email", "tests@example.invalid") + _git(tmp_path, "config", "user.name", "SpecFact tests") + _git(tmp_path, "add", ".") + _git(tmp_path, "commit", "-m", "baseline") + + source_path.write_text("VALUE = 'staged'\n", encoding="utf-8") + _git(tmp_path, "add", source_path.relative_to(tmp_path).as_posix()) + source_path.write_text("VALUE = 'unstaged'\n", encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(SIGN_SCRIPT_PATH), + "--staged-only", + "--allow-unsigned", + "--allow-same-version", + "--payload-from-filesystem", + ], + cwd=tmp_path, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + written_manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + assert isinstance(written_manifest, dict) + assert ( + written_manifest["integrity"]["checksum"] + == f"sha256:{hashlib.sha256(_index_payload(tmp_path, module_relative)).hexdigest()}" + ) From 42c65fbc1b74729dbe5ddf6ed802393bb76dfc30 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Sat, 11 Jul 2026 00:39:25 +0200 Subject: [PATCH 4/4] test: cover staged module auto-bump signing --- scripts/sign-modules.py | 16 +++++++ ..._commit_verify_modules_signature_script.py | 46 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/scripts/sign-modules.py b/scripts/sign-modules.py index ba351d84..5a34a557 100755 --- a/scripts/sign-modules.py +++ b/scripts/sign-modules.py @@ -435,6 +435,18 @@ def _has_unstaged_manifest_changes(manifest_path: Path) -> bool: return bool(changed) +def _stage_manifest(manifest_path: Path) -> None: + try: + subprocess.run( + ["git", "add", "--", manifest_path.as_posix()], + check=True, + capture_output=True, + text=True, + ) + except (subprocess.CalledProcessError, OSError) as exc: + raise ValueError(f"Unable to stage updated manifest {manifest_path}: {exc}") from exc + + def _report(message: str) -> None: sys.stdout.write(f"{message}\n") @@ -446,6 +458,8 @@ def _auto_bump_manifest_version( bump_type: str, staged_snapshot: bool, ) -> bool: + if staged_snapshot and _has_unstaged_manifest_changes(manifest_path): + raise ValueError(f"Refusing to overwrite unstaged manifest changes: {manifest_path}") current_version = _read_manifest_version(manifest_path, staged_snapshot=staged_snapshot) if not current_version: raise ValueError(f"Manifest missing version: {manifest_path}") @@ -458,6 +472,8 @@ def _auto_bump_manifest_version( bumped = _bump_semver(current_version, bump_type) raw["version"] = bumped _write_manifest(manifest_path, raw) + if staged_snapshot: + _stage_manifest(manifest_path) _report(f"{manifest_path}: version {current_version} -> {bumped}") return True diff --git a/tests/unit/test_pre_commit_verify_modules_signature_script.py b/tests/unit/test_pre_commit_verify_modules_signature_script.py index 1a1e265b..215330db 100644 --- a/tests/unit/test_pre_commit_verify_modules_signature_script.py +++ b/tests/unit/test_pre_commit_verify_modules_signature_script.py @@ -162,3 +162,49 @@ def test_staged_only_signing_hashes_the_index_not_unstaged_module_content(tmp_pa written_manifest["integrity"]["checksum"] == f"sha256:{hashlib.sha256(_index_payload(tmp_path, module_relative)).hexdigest()}" ) + + +def test_staged_only_auto_bump_signs_the_staged_snapshot(tmp_path: Path) -> None: + module_relative = Path("packages/specfact-example") + module_dir = tmp_path / module_relative + module_dir.mkdir(parents=True) + manifest_path = module_dir / "module-package.yaml" + manifest_path.write_text("name: nold-ai/specfact-example\nversion: 0.1.0\n", encoding="utf-8") + source_path = module_dir / "src.py" + source_path.write_text("VALUE = 'base'\n", encoding="utf-8") + _git(tmp_path, "init") + _git(tmp_path, "config", "user.email", "tests@example.invalid") + _git(tmp_path, "config", "user.name", "SpecFact tests") + _git(tmp_path, "add", ".") + _git(tmp_path, "commit", "-m", "baseline") + + source_path.write_text("VALUE = 'staged'\n", encoding="utf-8") + _git(tmp_path, "add", source_path.relative_to(tmp_path).as_posix()) + source_path.write_text("VALUE = 'unstaged'\n", encoding="utf-8") + + result = subprocess.run( + [ + sys.executable, + str(SIGN_SCRIPT_PATH), + "--staged-only", + "--bump-version", + "patch", + "--allow-unsigned", + "--payload-from-filesystem", + ], + cwd=tmp_path, + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 0, result.stderr + written_manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + staged_manifest = yaml.safe_load(_git(tmp_path, "show", f":{manifest_path.relative_to(tmp_path)}").stdout) + assert isinstance(written_manifest, dict) + assert isinstance(staged_manifest, dict) + assert written_manifest["version"] == staged_manifest["version"] == "0.1.1" + assert ( + written_manifest["integrity"]["checksum"] + == f"sha256:{hashlib.sha256(_index_payload(tmp_path, module_relative)).hexdigest()}" + )