Add plan-review eval harness + content-hash plan-approval gate (workflow-eval step 1b)#716
Conversation
Overall AssessmentExecutive Summary
MethodologyP1 — Campaign verdict lacks immutable protocol provenanceLocation: Impact: The manifest does not record hashes of Concrete fix: Snapshot/hash the decision rule, full config, candidate artifacts, extraction prompt/model, and registered contrasts into the run manifest. Store that identity in Code QualityP1 —
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment⛔ Blocker — one P0 security vulnerability and three unmitigated P1 findings. Executive Summary
MethodologyP1 — Protocol provenance does not identify the executed protocolLocation: Impact: Configs and reviewer artifacts are loaded before the long-running matrix, but Concrete fix: Snapshot protocol/config/artifact bytes before constructing configs or reviewers, build execution from that snapshot, record it immediately, and abort or mark non-gating if an end-of-run hash differs. Add a mid-run mutation regression test. P1 — [Newly identified] Arms can review different plan bytesLocation: Impact: Concrete fix: Capture each plan’s bytes once before scheduling, include its digest in the case snapshot, and render every arm from those immutable bytes. Reject any end-of-run corpus drift. No library methodology or paper-parity issue was found. Code QualityThe prior contradiction between required temporary writes and PerformanceP3 — Tracked peer-cancellation delayImpact: A surviving dual-review subprocess may continue after its peer fails. Concrete fix: No approval action required; tracked in MaintainabilityP1 — [Newly identified] Same-basename plans overwrite approvalsLocation: Impact: Review keys use only the canonical basename. Two plans such as Concrete fix: Include a canonical-path digest in the review filename in the helper, hook, and check path. Add a two-path/same-basename concurrency test. P2 — Aborted reviews leave “temporary” snapshots behindLocation: Impact: Cleanup occurs only during Concrete fix: Add an argv-safe Tech DebtP3 — Recorded limitationsCodex read isolation is tracked in SecurityP0 — Untrusted plan basename reaches Bash command substitutionLocation: Impact: The generated --state-file "<state-path>"Double quotes do not suppress Concrete fix: Never interpolate the returned path into shell source. Pass it through a file-ingress argument or return a safe opaque token that the helper resolves internally. Generated filenames should also contain only a strict safe charset. Documentation / TestsP2 — Tests bypass the vulnerable workflow boundaryLocation: Impact: The shellish-name test invokes Python with an argv list, so it cannot detect the prose-level shell injection. The provenance test checks only dictionary inequality, not mutation during Concrete fix: Add an isolated end-to-end command test using Static parsing passed for 14 changed Python files and four JSON files. Pytest was unavailable in the review environment. Path to Approval
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyP1 — Prior protocol-provenance finding remains unresolvedLocation: Impact: Concrete fix: Read every protocol/config/candidate artifact once into an immutable snapshot, derive the manifest identity from those exact bytes, and construct configs/reviewers exclusively from that snapshot. P1 — [Newly identified] Severity disagreements bypass FP deduplicationLocation: Impact: Duplicate identity is Concrete fix: Deduplicate by normalized finding identity independent of severity—at minimum normalized summary—or require a stable finding ID. Reject conflicting severities for the same finding. Code QualityNo additional findings. PerformanceP3 — Tracked peer-cancellation delayImpact: A surviving dual-review subprocess can run until timeout after its peer fails. Concrete fix: No approval action required; tracked in MaintainabilityP1 — [Newly identified] Hashed review-path contract was not propagatedLocation: Impact: The helper now writes Concrete fix: Consume the helper’s returned Tech DebtNo untracked blocking debt found. SecurityP3 — Tracked Codex read-isolation limitationImpact: Read-only sandboxing does not confine Codex reads to the worktree. Concrete fix: No approval action required; tracked in Documentation / TestsP2 — Remediation boundaries lack regression testsImpact: Current tests cover helper/hook key agreement but not command-level discovery, transient A→B→A protocol mutation, or same-summary/different-severity FP duplication. Concrete fix: Add focused regressions for those three cases. Static parsing passed for 14 changed Python files and four JSON files. Path to Approval
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyP1 — Prior protocol-provenance finding remains partially unresolvedLocation: Impact: The immutable protocol snapshot covers configs and candidate files, but not Concrete fix: Include the control prompt’s exact bytes in Code QualityNo additional findings. PerformanceP3 — Tracked peer-cancellation delayImpact: A surviving dual-review subprocess may run until timeout after its peer fails. Concrete fix: No approval action required; tracked in MaintainabilityP1 — [Newly identified]
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — both prior P1 findings are resolved. No unmitigated P0/P1 findings remain. Executive Summary
MethodologyP3 — Documented evaluation-protocol divergencesImpact: Control criteria are inlined, and dual verification lacks production conversation context. Both are explicitly disclosed in Concrete fix: None required. The mechanical regression, FP, equal-exposure, registered-contrast, and corpus-floor gates match Code QualityP2 — Arbitrary-path claim exceeds the frontmatter contractLocation: Impact: Ingress Concrete fix: Preserve paths through structured ingress, or explicitly reject CR/LF and trailing-whitespace paths with a clear error; add regressions. PerformanceP3 — Tracked peer-cancellation delayImpact: A surviving dual-review subprocess can run until timeout after its peer fails. Concrete fix: No approval action required; tracked in MaintainabilityNo findings. Tech DebtNo untracked blocking debt found. SecurityP3 — Tracked Codex read-isolation limitationImpact: Read-only sandboxing does not confine Codex reads to the worktree. Concrete fix: No approval action required; tracked in Documentation / TestsNo blocking findings. Focused regressions cover both prior P1 fixes. Static validation passed for 24 Python files and four JSON files, plus direct protocol/verdict checks. |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyP1 — [Newly identified] Protocol identity is not end-to-endLocation: Impact: A campaign run under protocol A can be extracted and bundled under modified protocol B, then have files restored to A before Concrete fix: Require the manifest’s protocol identity to match before P1 — [Newly identified] Blinded arm mapping is not verifiedLocation: Impact: Concrete fix: Recompute the deterministic mapping from manifest run keys and rendered arm set during verdict, require exact equality, and add a swapped-mapping rejection test. P3 — Documented fidelity differencesLocation: Impact: Inlined control criteria, context-free merge verification, and detached read-only worktrees differ from production but are explicitly documented. Concrete fix: None required. No library method or Methodology Registry entry is affected. Code QualityP1 — [Newly identified] Missing outer
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyP1 — Prior protocol-identity finding remains partially unresolvedLocation: Impact: Concrete fix: Include all extraction/execution sources in protocol identity, derive every post-run input from one immutable protocol snapshot, record that identity in extraction metadata and P1 — [Newly identified] Schema-invalid
|
…val step 1) Builds the measurement infrastructure for the plan-review workflow family (the first target of the eval-per-workflow-surface program) and lands the purely mechanical gate fixes. The live review engine does not change. tools/plan-review-eval/ — five-arm harness over eval_core (verify-corpus / smoke / run / extract / compare / verdict): pinned-git-SHA control arm, candidate single+dual arms, Sonnet-5 and gpt-5.6-terra probe arms, all k=2. Pre-registered DECISION_RULE.md; neutral-severity extraction with blinded bundles whose bundle_id is the hash of the grader-visible bytes; mechanical verdict gates (GO / NO-GO / PARITY / UNDETERMINED) with protocol-violation labeling (NON-GATING) for rehearsal or subset runs. Campaign provenance is end-to-end: one immutable protocol snapshot per stage feeds both the recorded identity and execution; the identity covers the decision rule, configs, every candidate artifact, the control prompt, and EVERY Python source in the plan-review-eval and eval_core trees (walked, not hand-listed); stages gate at entry against the snapshot and re-check at exit with a fresh read; the protocol sha is stamped into extraction metadata and blinding.json and re-verified downstream; the blind mapping is recomputed at verdict, never trusted. The corpus loader enforces the manifest.schema.json type contract before dataclass construction, so a schema-invalid value (e.g. "must_catch": "false") can never flip a campaign gate through truthiness. Real plan corpora stay local-only (gitignored); one fabricated fixture case is committed for CI, smoke, and the dress rehearsal. .claude/hooks/check-plan-review.py replaces the sentinel-file gate: the ExitPlanMode hook now validates a plan_sha256 recorded in the review file's frontmatter against the CURRENT plan bytes — no sentinel, no mtime, no touch hack, no cross-worktree races. It fails closed on payload drift, missing tool_input, unreadable plans, malformed frontmatter, and path mismatches (realpath-normalized both sides). .claude/scripts/plan_snapshot.py provides the tested snapshot/persist/check/abort protocol the commands use: invocation-unique digest+nonce state tokens, recorded-digest certification (persist re-verifies the live plan against the reviewed snapshot), canonical-path-digest review filenames, and strict path ingress (absolute, no CR/LF, no whitespace edges). /review-plan and /revise-plan are thin wrappers over the helper; every check/snapshot call site requires confirming the helper's echoed plan_path (shared per-worktree ingress). The recorded threat model (DEFERRED.md): the gate prevents accidents, not local adversaries. Tests: hook decision table, snapshot protocol (incl. hostile-stem bash end-to-end), harness units (corpus contracts, hermetic pinned-SHA sourcing, arm schema, synthetic k=2 verdict tables, protocol-identity regressions), and command-contract scans.
8e67cf6 to
89cc78a
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyPrior protocol-identity finding remains partially unresolvedSeverity: P1 Impact: The identity excludes the external Concrete fix: Include every executable dependency in an immutable source snapshot, bind its protocol hash into every run key/result, and refuse cache reuse when an existing or partial campaign’s protocol differs. Add cached-resume, external-wrapper-drift, and late-import A→B→A regressions. Corpus schema enforcement remains incompleteSeverity: P1 Impact: Only the type of Concrete fix: Apply the complete JSON schema or equivalent strict validation, including enum, fixture, string-list, and known-topic ID contracts. Add tests proving unknown strata cannot satisfy the corpus floor and string [Newly identified] Claude redaction destroys repository pathsSeverity: P1 Impact: The pattern Concrete fix: Restrict redaction to configured model IDs and genuinely model-shaped Claude tokens. Add regressions preserving Documented fidelity differencesSeverity: P3 Impact: Inlined control criteria, context-free merge verification, and detached worktrees differ from production. Concrete fix: None required; these are explicitly documented. Code QualityNo additional findings. The prior concurrent revise-plan ingress issue is resolved by checking returned PerformanceTracked peer-cancellation delaySeverity: P3 Concrete fix: No action required for this PR; tracked in MaintainabilityNo additional findings. Tech DebtNo untracked deferrable work found. SecurityTracked Codex read isolationSeverity: P3 Concrete fix: No action required here; tracked in Documentation / TestsNo additional documentation finding. The new tests cover the exact prior boolean and ingress cases, but not the three issues above. Pytest was unavailable in the review environment; static parsing succeeded for 20 affected Python files. Path to Approval
|
…on, full corpus schema
Pre-registration integrity gets its invariant-level closure. A subdirectory
IS a campaign: its protocol identity is registered write-once, BEFORE any
reviewer call (a crash mid-matrix leaves a registered campaign with empty
run_keys, never an orphaned cache), and any invocation under a different
protocol refuses to touch it — so the store's cached outcomes can never be
re-attributed to a later protocol by editing DECISION_RULE.md and re-running
the same subdir. Smoke keeps re-registering its fixture subdir freely (never
gating, never graded). The external codex wrapper
(.claude/scripts/openai_review.py, importlib-executed by dual arms) joins
the evaluator identity, and the snapshot now proves imported code ==
hashed bytes via a read->import->re-read stability bracket: the executing
modules are imported inside the bracket and the snapshot aborts if any
source changed across it (closing the snapshot-then-import window).
Claude redaction is narrowed to MODEL-shaped tokens ("claude" + version or
family tail, e.g. claude-fable-5, Claude 3.5, claude_opus; standalone
family words unchanged). The previous pattern treated the repository's own
.claude/ tree as a model name — `.claude/hooks/check-plan-review.py` became
`.[model-redacted]/hooks/...` in blinded bundles, mangling exactly the
evidence quotes graders score against ground truth and downgrading genuine
catches to misses. Bare "claude" carries no arm signal (every arm runs the
claude CLI); regressions pin both directions, including the composed
evidence-quote case. Shared-engine change: reviewer-eval inherits the
narrowed pattern (its blinding tests stay green).
The corpus loader now enforces the complete manifest.schema.json contract:
stratum must be a known enum value (an unknown stratum directory previously
loaded, verified, and counted toward the pre-registered >=8-case floor),
class_keywords/expected_files/allow_severities must be string lists (a
string class_keywords silently became a character list in grader-visible
evidence), fixture and provenance must be objects with string fields, and
known_fp_topics entries must be objects.
Regressions: external-wrapper identity membership; mid-import source-drift
abort; write-once-per-protocol refusal (resume under the recorded protocol
stays allowed); registration-before-observation on a mid-matrix crash;
unknown-stratum and string-class_keywords rejection; .claude-path/CLAUDE.md
preservation alongside model-id redaction.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyProtocol identity remains incompleteSeverity: P1 Impact: Concrete fix: Import the wrapper inside the read/import/re-read bracket and pass that exact module to Corpus schema discriminator is not enforcedSeverity: P1 Impact: The schema requires Concrete fix: Enforce all required fields and constants from Campaign registration does not freeze the campaignSeverity: P1 — newly identified in the new registration logic Impact: Concrete fix: Register and enforce an immutable campaign fingerprint containing protocol identity, case IDs and content tags, resolved base SHAs, config IDs, Documented fidelity differencesSeverity: P3 Impact: Inlined control criteria, context-free merge verification, and detached worktrees differ from production. Concrete fix: None required; explicitly documented. Code QualityNo additional findings. PerformanceTracked dual-peer cancellation delaySeverity: P3 Concrete fix: No action required here; tracked in MaintainabilityNo additional findings. Tech DebtNo untracked deferrable work found. SecurityTracked Codex read isolationSeverity: P3 Concrete fix: No action required here; tracked in Documentation / TestsThe redaction regression is adequately covered. Tests are still needed for the three P1 paths above. Pytest was unavailable in the review environment; static parsing succeeded for 14 affected Python files. Path to Approval
|
…nator, frozen sample plan The codex wrapper now executes provably-hashed bytes: _import_executing_ modules() loads .claude/scripts/openai_review.py INSIDE the snapshot's read->import->re-read stability bracket and the snapshot hands that module object to PlanReviewer (openai_mod parameter), so a disk edit after the snapshot can never reach dual-arm execution — the last late-import window in the provenance chain. The corpus schema's discriminator is enforced end-to-end: fixture.kind must be the schema constant "plan_at_sha" at load (a copied "git_range" case previously verified and counted toward the corpus floor while being silently materialized as a plan_at_sha checkout — reviewing the wrong repository state), base_sha is schema-required, and materialize() refuses unknown kinds as defense in depth. Campaign registration now freezes the SAMPLE PLAN, not just the protocol: the registered manifest carries a campaign fingerprint (arms, k, overrides, and per-case declared base_sha + frozen plan bytes), and an invocation differing in protocol OR fingerprint refuses the subdirectory — an outcome-dependent change to cases, plan content, or repeat schedule after results exist is a new campaign, never a rewrite of this one. README gains "Provenance guarantees and limits": what the machinery guarantees (whole-tree identity incl. the external wrapper, bracket-proven execution, write-once registration-before-observation, stage gates, bound extraction/blinding artifacts, recomputed mapping, byte-hash bundle ids) and the documented out-of-scope floor (sub-bracket TOCTOU detected-not- prevented; hostile local writers per the DEFERRED.md threat model; smoke's never-graded fixture subdir). Regressions: bracketed-wrapper wiring (reviewer._mod IS the snapshot's module), fingerprint freeze across six mutation axes (k, arms, overrides, plan bytes, base sha, case set), registration carries the fingerprint, unknown fixture kind rejected at load AND at materialize().
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
MethodologyCampaign fingerprint omits scoring metadataSeverity: P1 — prior finding partially unresolved Impact: Registration hashes only case ID, base SHA, and plan bytes. The same campaign can be rerun after changing its stratum, ground truth, Concrete fix: Fingerprint the canonical complete case snapshot—including every verdict/grader-visible field, fixture metadata, resolved base SHA, and frozen plan bytes. Add regressions mutating each scoring field. Required corpus field remains optionalSeverity: P1 — prior schema finding partially unresolved Impact: The schema requires Concrete fix: Reject ground-truth entries lacking Documented fidelity differencesSeverity: P3 Impact: Inlined control criteria, context-free merge verification, and detached worktrees differ from production. Concrete fix: None required; documented. Code QualityNo additional findings. PerformanceSeverity: P3 — tracked in MaintainabilityNo additional findings. Tech DebtNo untracked deferrable work found. SecuritySeverity: P3 — tracked in Documentation / TestsThe two P1 paths above need regressions. Pytest was unavailable in the review environment; static parsing succeeded for 20 affected Python files. Path to Approval
|
…re rationale The campaign fingerprint now covers HOW observations are scored, not just which are made: the loader stamps every case with the canonical hash of its complete definition (ground truth, must_catch, FP allowances, known-FP topics, weight, notes — sorted-keys JSON of the raw case.json), and that case_sha joins the registered fingerprint beside base_sha and the frozen plan bytes. Editing any scoring field after results exist now refuses the subdirectory — outcome-dependent redefinition of ground truth under a registered campaign is closed. ground_truth[].rationale is schema-required and now enforced: the loader rejects a missing, empty, or whitespace rationale (graders match findings against it), and the schema pins minLength 1 so the documented contract and the executable enforcement agree. Regressions: missing/empty/whitespace rationale rejected; two cases identical in plan bytes and base_sha but differing in a scoring field produce different case_shas and different campaign fingerprints, and the fingerprint echoes the loader's stamped hash.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — both prior P1 findings are resolved; no unmitigated P0/P1 issues remain. Executive Summary
MethodologyBoth prior findings are resolved:
The Methodology Registry is unaffected because no causal estimator behavior changed. Code QualityNo findings. Performance
MaintainabilityNo findings. Tech DebtNo untracked deferrable work found. Security
Documentation / TestsNo findings. The new regressions cover both previously reported defects. Full pytest execution was unavailable because |
igerber
added
ready-for-ci
… tests The label-gated CI run failed only in the single-process Pure Python Fallback leg: test_evals_runtime.py's 28 bare `import run_eval` sites are ambiguous now that TWO harnesses ship a run_eval.py, and which module wins depends on sys.path insert order at collection time. In CI's alphabetical collection the plan-review-eval suite's insert displaced reviewer-eval's, so the reviewer-eval tests executed against the WRONG harness (27 failures: missing CorpusLoader/_resolve_configs, foreign treatment fields). Local runs always listed test_plan_review_eval.py first, which masked the order dependence; the parallelized matrix legs split the suites across workers and never saw it. Fix mirrors test_plan_review_eval.py's existing loader: reviewer-eval tests load their harness CLI by explicit file path under the unique module name reviewer_eval_run_eval (lru-cached), eliminating the ambiguous bare import entirely. With run_eval de-ambiguated, no shared bare module name remains between the harnesses (adapters vs plan_adapters are distinct; verdict.py exists only in plan-review-eval; eval_core is a single shared copy). Verified in both collection orders: the CI-order repro goes 27 failures -> 0, and the original order stays green (299 passed).
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall AssessmentExecutive Summary
Methodology
Prior findings are resolved:
Code QualityNo findings. Performance
MaintainabilityNo findings. Tech DebtNo untracked deferrable work found. Security
Documentation / TestsNo additional findings. Pytest is not installed in the review environment; static parsing and focused executable probes passed. Path to Approval
|
…ings The extraction stage no longer categorically excludes questions. The control engine's native format raises implementation-blocking ambiguities under "Questions for the Author", so a blanket question exclusion dropped those findings for arm A while the candidate's flat-finding phrasing survived — a systematic bias toward the candidate arms capable of manufacturing a false GO. The rule now extracts a question as a finding when it identifies a missing decision, contradiction, unresolved dependency, or execution blocker (at least `major`; `blocker` when marked blocking), and excludes only non-defect clarifications that assert no gap. The dress rehearsal gains a format-parity extraction probe (DECISION_RULE.md): a committed control-format sample review (corpus/fixture/fx-mini-plan/control_format_review.md) whose ONLY mention of the seeded nonexistent-symbol defect sits under the questions section must survive extraction as a finding, or the rehearsal fails. A deterministic contract test pins the prompt rules (no categorical exclusion, substantive-question inclusion, qualified-only exclusion) and the sample's sole-mention structure.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive Summary
MethodologyThe affected methodology is the evaluation protocol only; no causal estimator or cited paper applies.
The previous extraction-bias P1 is resolved. Code QualityNo findings. Performance
MaintainabilityNo findings. Tech DebtNo untracked deferrable work found. Security
Documentation / Tests
|
igerber
added
ready-for-ci
The label-gated CI matrix is otherwise green (9768 passed); the one failure was test_control_prompt_participates_in_protocol_identity, whose load_artifacts call materializes the control criteria from the REAL pinned SHA — absent in CI's depth-1 checkout. The step-1 plan's own rule (hermetic tmp-repo tests cover the git-show mechanism; real-pin tests skip on shallow clones) was applied to test_configured_pin_resolves_in_full_clone but missed on this round-4 regression test. The probe is factored into _skip_unless_pin_available(), shared by both real-pin tests; the control-prompt test still asserts the snapshot's control_prompt_sha plumbing unconditionally and skips only the load_artifacts materialization when the pin is unreachable. Verified in a local depth-1 clone (the exact CI condition): both tests skip, the rest of the suite passes; full clone still runs both.
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment✅ Looks good — no unmitigated P0 or P1 findings. Executive Summary
Methodology
Code QualityNo findings. Performance
MaintainabilityNo findings. Tech DebtNo untracked findings. Security
Documentation / Tests
|
1 participant
Summary
tools/plan-review-eval/(new): five-arm k=2 harness measuring plan-review engine candidates against the pinned production workflow before anything ships — control criteria sourced viagit showat a pinned full SHA; candidate engine (P0–P3 criteria, dual Claude+codex blind review with merge+verify, extraction prompts) as lab artifacts incandidates/; pre-registeredDECISION_RULE.md(regression/FP/improvement gates, equal-exposure UNDETERMINED rule, neutral severity vocabulary, corpus floor); extraction-based blinded grading whose Bundle ID is the hash of the exact grader-visible bytes; mechanicalverdict.pywith strict grading-table validation (complete grids, negative-assessment cells, registered contrasts only, realized-grid vs declared-schedule check). Real plan corpus stays local-only (gitignored); a fabricated fixture case is committed for CI/smoke/dress-rehearsal. Live-smoke-verified: the control arm reviewed the fixture plan end-to-end (headlessclaude -p --safe-mode, detached worktree atbase_sha) and caught both seeded defects.tools/eval_core/evolutions (generic, additive; reviewer-eval unchanged and green):Configgainsvariant/modearm dimensions,build_bundletakes a header override, blinding patterns cover Claude-family names, known-FP topic ids render in bundles. The worktree-cleanup containment fix (guards before any git command; symlinked leaves unlinked, never followed; strict trusted-root containment) is applied to both harnesses' adapters..claude/hooks/check-plan-review.pyreplaces the shell hook — aplan_sha256content-hash gate replaces the.last-reviewedsentinel + mtime design, structurally killing the concurrent-worktree sentinel race, thels -tfallback, and thetouch-after-revision hack; fails closed on payload drift and malformed payloads (payload contract verified against the deployed CLI binary's own ExitPlanMode input schema). A new tested helper.claude/scripts/plan_snapshot.pyowns the snapshot/persist protocol (invocation-unique immutable snapshots keyed by a state token; persistence certifies the recorded snapshot digest only after re-verifying the live plan; canonical review keys matching the hook; hostile-meta-proof frontmatter);review-plan.md/revise-plan.md/CLAUDE.md are thin wrappers around it. Revisions now require a fresh review (or an honestly-labeled Skipped marker) — no re-stamp path.Methodology references (required if estimator / math changes)
tools/plan-review-eval/DECISION_RULE.md(gate soundness, equal-exposure rule, corpus floor) andtools/plan-review-eval/candidates/(criteria, merge+verify, extraction prompts) as first-class review surfaces — the step-2 campaign may not run until they have survived review (campaign-readiness gate).Validation
tests/test_plan_review_hook.py(26-case allow/deny matrix incl. the two-concurrent-plans race, payload drift/type discipline, symlink+tilde normalization; every deny asserts the PreToolUse protocol),tests/test_plan_snapshot.py(12 behavioral: invocation-unique snapshots, A→B→A identity certification, rewritten-snapshot rejection, cross-wiring impossibility, hostile paths/meta),tests/test_plan_review_eval.py(69: loader contracts, hermetic pinned-SHA sourcing, arm-matrix decomposition, verdict gate boundaries on synthetic k=2 tables, UNDETERMINED on unequal exposure, bundle-id-as-artifact-hash, fixture exclusion, argv confinement),tests/test_command_contract.py(helper-invocation + path-validation contracts),tests/test_evals_*.py(cleanup-signature update only). 250 pass across the seven suites in one process;verify-corpuspasses for both harnesses (1/1 and 11/11).--safe-modechanges) — both caught the two seeded defects with exact repo citations.Security / privacy