-
Notifications
You must be signed in to change notification settings - Fork 0
docs: enforce module documentation accountability #340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
openspec/changes/docs-16-core-accountability-sync/.openspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-07-10 |
65 changes: 65 additions & 0 deletions
65
openspec/changes/docs-16-core-accountability-sync/CHANGE_VALIDATION.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # 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 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 | ||
|
|
||
| - `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:40 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` | ||
| fallback has an intentionally optional description parameter. They are recorded | ||
| as non-blocking design observations, not unresolved review findings. |
155 changes: 155 additions & 0 deletions
155
openspec/changes/docs-16-core-accountability-sync/TDD_EVIDENCE.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| # 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, 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 | ||
| 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 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 | ||
| 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. |
60 changes: 60 additions & 0 deletions
60
openspec/changes/docs-16-core-accountability-sync/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 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 | ||
| 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. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.