feat(governance): diff-bound commit acceptor layer#491
Conversation
…al→falsifier→rollback→evidence→memory)
Invariant: every code-modifying commit landing on main MUST be governed by
at least one acceptor under .claude/commit_acceptors/ that declares the
full six-step contract. Unbound code commits are rejected fail-closed by
the new Commit Acceptor Gate workflow on every PR and merge-queue entry.
Files added:
- .claude/commit_acceptor_policy.yaml (claim caps + forbidden imports)
- .claude/commit_acceptor_template.yaml (canonical schema, status DRAFT)
- .claude/commit_acceptors/canonical-action-result-comparator.yaml
(ACTIVE, documents PR #490)
- .claude/commit_acceptors/commit-acceptor-layer.yaml
(ACTIVE, self-acceptor for this PR)
- tools/commit_acceptor/{__init__.py,validate_commit_acceptor.py}
(validator + CLI)
- tests/unit/commit_acceptor/{__init__.py,test_validate_commit_acceptor.py}
(44 tests, all 41 spec probes)
- .github/workflows/commit-acceptor-gate.yml (PR + merge_group, 3.11/3.12)
- docs/reports/diff_bound_commit_acceptor_layer.md (closure report)
Forbidden schema fields (rejected anywhere): forbidden_symbols,
max_files_changed, generated_at. Forbidden import patterns enforced via
AST: trading, execution, forecast, policy. Distinct from CLAIMS layer
(.claude/claims/CLAIMS.yaml) — per-commit, diff-bound, not modified.
Local gates green: validator (static), validator (diff-binding +
require-acceptor-for-code-change), pytest 44/44, ruff, ruff format,
black, mypy --strict. Probe matrix 15/15 with idempotence
(sha256 of acceptor unchanged across two consecutive validator runs).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a33eb24eeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| p = entry.get("path") | ||
| if isinstance(p, str): | ||
| path_to_acceptors.setdefault(p, []).append((apath, accept)) |
There was a problem hiding this comment.
Scope diff binding to current commit acceptors
validate_diff_binding indexes diff_scope.changed_files from every acceptor file in .claude/commit_acceptors, including historical entries unrelated to the current PR. That lets a new code change pass --require-acceptor-for-code-change if an old acceptor already lists that path, which breaks the stated per-commit/diff-bound guarantee and allows ungoverned edits to be silently accepted on later commits.
Useful? React with 👍 / 👎.
| syms = acceptor.get("required_python_symbols") | ||
| if syms is not None and not isinstance(syms, list): | ||
| result.errors.append(f"{path}: required_python_symbols must be a list") |
There was a problem hiding this comment.
Validate required_python_symbols against repository code
The validator only checks that required_python_symbols is a list, but never verifies that those symbols actually exist after the change. As a result, acceptors with misspelled or nonexistent required symbols still validate successfully, so the contract field meant to guarantee exported runtime/API symbols provides no enforcement.
Useful? React with 👍 / 👎.
…FIED loop) (#492) Closes the ACTIVE→VERIFIED transition opened by PR #491. Where #491 shipped the diff-bound acceptor schema + validator + CI gate, this PR adds the runner that actually executes measurement_command and falsifier.command, captures stdout+stderr to declared artifact paths, hashes every artifact (sha256, lowercase 64-char hex), and writes the evidence_sha256 list back to the acceptor YAML. With --promote and a PASS verdict, status flips from ACTIVE to VERIFIED in-place. Files added: - tools/commit_acceptor/run_evidence.py (557 lines) - tests/unit/commit_acceptor/test_run_evidence.py (23 tests) - tmp/run_evidence_dogfood.json (evidence-of-evidence for the runner itself, run against the two existing acceptors) Public API: - EvidenceResult (frozen dataclass, sorted JSON serialisation) - run_acceptor(acceptor, repo_root, *, timeout_s, runner) -> EvidenceResult - update_acceptor_yaml(path, result, *, promote_to_verified) -> None - main(argv) -> int (CLI: --acceptor-id/--all, --promote, --re-verify, --timeout-s [10, 3600], --summary-out, --repo-root) Test count: 23/23 PASS (67/67 in tests/unit/commit_acceptor) Gates: ruff check + ruff format --check + black --check + mypy --strict + validate_commit_acceptor (with and without --require-acceptor- for-code-change) — all green. Falsifier mutation probes (all 6 caught by tests): #1 skip --promote success guard → test 8 FAILS as expected #2 truncate sha256 to 8 chars → test 22 FAILS as expected #3 always return verdict=PASS → tests 2 + 3 FAIL as expected #4 skip artifact existence check → test 4 FAILS as expected #5 stop skipping DRAFT acceptors → test 10 FAILS as expected #6 strip evidence_sha256 sort → test 6 FAILS as expected Dogfood verdict counts (from tmp/run_evidence_dogfood.json): PASS: 1 (commit-acceptor-layer) SIGNAL_FAILED: 1 (canonical-action-result-comparator — tests/unit/control not present in this branch; honest null) Security: subprocess.run(shell=True, ...) trusts maintainer-committed acceptor YAML. Acceptor schema is enforced by the validator (PR #491) before the runner ever sees a file. Per the chronology-discipline contract, this runner is execution proof, NOT chronology proof — it claims only "command exited 0 and these are the artifact hashes". Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r-layer (#493) The dogfood JSON committed in PR #492 has extension .json which the commit-acceptor policy treats as code, triggering "code change without acceptor" on the diff-binding CI gate. Add it to the self-acceptor's diff_scope so the gate is satisfied. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The commit-acceptor-gate job runs `pytest tests/unit/commit_acceptor` which transitively triggers the global `tests/conftest.py`. That conftest imports `core/utils/determinism.py`, which imports `numpy`. Without numpy in the venv, pytest fails during collection (before any test runs) with `ModuleNotFoundError: No module named 'numpy'` — turning both 3.11 and 3.12 matrix jobs red. Add `numpy` to the install line. Other deps unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…alidator The first-pass validator (PR #491) shipped with six bypasses surfaced by adversarial audit. All six are now closed; each fix is paired with a test that fails without it (mutation-probed, both directions where relevant). Hole 1 — Relative-import bypass (`from . import trading`): AST detector skipped relative imports entirely. Fixed: when node.level > 0, check each `alias.name` against forbidden patterns. Hole 2 — Relative-import false positive (`from .trading import x`): Symmetric defect. The relative module name `.trading` is a repo-local sibling submodule, not the forbidden absolute `trading` runtime. Fixed: for relative imports, only inspect alias names (NOT node.module). Hole 3 — Path traversal in `diff_scope.changed_files[*].path`: `../etc/passwd`, `geosync/../../escape`, `/abs`, `path\\windows` were accepted silently. Added `_is_safe_repo_relative_path` helper rejecting leading `/`, backslashes, and any `..` component. Applied to both `changed_files` and `forbidden_paths` for symmetry. Hole 4 — Empty/whitespace `id` and `promise` summary: `id: ""` and `promise: " "` passed schema validation. Fixed: explicit non-empty-after-strip checks on `id` (string) and on `promise` whether it is a string or a `{summary: ...}` mapping. Hole 5 — `promise: null` (None / wrong type): YAML `promise:` (no value) silently passed. Fixed: explicit `INVALID_PROMISE_BLOCK` rejection when promise is None or non-string-non-mapping (lists, ints). Hole 6 — Theater test for relative-import path: `test_14_relative_import_skipped` only asserted skip; never asserted catch on `from . import trading` (Hole 1). Replaced with `test_14_relative_import_two_directions` that asserts BOTH the catch (alias is forbidden) and the non-flag (relative module name is repo-local). New tests added (parametrized where relevant, 17 cases total): - test_14_relative_import_two_directions (both directions) - test_path_traversal_in_changed_files_rejected (6 params) - test_path_traversal_in_forbidden_paths_rejected (3 params) - test_empty_id_rejected, test_whitespace_id_rejected - test_empty_promise_summary_rejected, test_whitespace_promise_summary_rejected - test_promise_dict_with_empty_summary_rejected - test_null_promise_block_rejected - test_promise_wrong_type_rejected Probe matrix: each new test was mutation-probed by stashing the validator change and re-running the test selector — all 17 cases failed without the fix and passed with it. Full gate matrix (validator, diff binding, pytest, ruff, ruff format, black, mypy --strict) green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…al conftest deps The previous fix added numpy to the workflow venv, but tests/conftest.py also imports pandas (and transitively other deps via core/utils). Rather than mirror the entire repo's runtime dependency tree into a governance gate venv, use --confcutdir=tests/unit/commit_acceptor so pytest does not load the global conftest at all. The commit_acceptor unit tests are self-contained and need no shared fixtures. Net effect: workflow dependency line stays minimal (pyyaml/black/ruff /mypy/pytest only); CI no longer breaks when an unrelated dep is added to tests/conftest.py. Verified locally: 83/83 pass with --confcutdir; same set passes without the flag too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ifact + restore mypy plugin Three coupled fixes for commit-acceptor-validation jobs: 1. Remove tracked tmp/run_evidence_dogfood.json — sha256 hex digests in the evidence dogfood snapshot looked like high-entropy secrets to detect-secrets. The runner can produce a fresh snapshot on demand; committing one stale instance polluted the secret scanner. 2. Add tmp/ to .gitignore so future runner output stays out of git. 3. Add pydantic to commit-acceptor-gate workflow venv. The repo's mypy.ini declares pydantic.mypy as a plugin; mypy --strict cannot load it without the package installed, even when the files under inspection do not import pydantic. Self-acceptor updated to drop the dogfood path from changed_files. Verified locally: 83/83 tests pass with --confcutdir; static validator PASS; diff-binding gate PASS after this commit because the deletion no longer appears in the net origin/main..HEAD diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Invariant
Every code-modifying commit landing on
mainMUST be governed by at leastone acceptor under
.claude/commit_acceptors/declaring the fullsix-step contract:
Unbound code commits are rejected fail-closed by the new
Commit Acceptor Gateworkflow on every PR and merge-queue entry.Core invariant table
.claude/commit_acceptors/commit-acceptor-gate.ymlPR + merge_group, 3.11 / 3.12tests/unit/commit_acceptor/44 tests (41 spec probes)tools/commit_acceptor/validate_commit_acceptor.pyCLI.claude/claims/— per-commit, diff-boundSchema highlights
forbidden_symbols,max_files_changed,generated_atAST import boundary
AST walks
Import.names[*].nameandImportFrom.module. Match onequality OR
name.startswith(pattern + "."). Patterns:trading,execution,forecast,policy. Relative imports skipped;comments and string literals NOT inspected.
Evidence hashing semantics
signal_artifactand eachevidence[].pathhashed (sha256). DRAFT/ACTIVE:missing → warning. VERIFIED: missing or mismatched → error.
CLAIMS vs COMMIT_ACCEPTORS boundary
.claude/claims/CLAIMS.yaml(long-lived multi-commit registry) is NOTmodified or duplicated. Commit acceptors are orthogonal: per-commit,
diff-bound, fail-closed at the PR boundary.
Failure modes now blocked
silent code change without promise; trivially-passing falsifier;
forbidden import in changed .py; acceptor claiming files under its own
forbidden_paths; sprawling change > cap; banned schema fields; VERIFIED
acceptors with vanished evidence; validator self-mutating acceptor YAML.
What remains unproven
Cross-commit composition (acceptor stacking) is not modeled — each PR is
validated independently. Memory-update side-effects are declared
(append/replace/none) but not executed by the validator.
Test plan
--require-acceptor-for-code-change🤖 Generated with Claude Code