Fix autonomous merge review findings

[justin808]

Summary

Corrects the three blocking findings from the late adversarial review of #252 and all accepted review/QA findings on this corrective PR:

• validate every raw canonical calibration record and its identity before repository, date, or PR-count filtering, so malformed or duplicate evidence cannot disappear from an enforcement decision;
• accept the closed-schema autonomous_merge: {} seed while recursively rejecting unresolved full-token <...> policy values, including ADR-style rationale and glob placeholders;
• preserve ordinary prose that merely contains angle-bracket text inside autonomous_merge, while retaining legacy embedded-placeholder rejection for every required and optional non-autonomous policy value;
• restore workflow/prompt parity for the two new completion stop states;
• scope parity-contract extraction to each owning section and preserve fail-closed empty collection regressions outside autonomous_merge.

Source review: #252 (comment)
Codex review finding: #267 (comment)
CodeRabbit review finding: #267 (comment)
Current-head Codex review finding: #267 (comment)

Exact evidence

• Head: 2d9275bf760f60aba5dae381562dfc35bd4814b1
• Base: 137f8b9fac3f0758564dc601df10993c4257f06c
• Tree: 2c9d130eeaf0adbd950c9c0afbc39b0d3514ec27

Focused regressions:

• ruby skills/pr-batch/bin/autonomous-merge-calibrate-test.rb — 36 runs, 307 assertions
• ruby bin/agent-workflow-seam-doctor-test.rb — 272 runs, 2,313 assertions
• ruby skills/pr-batch/bin/autonomous-merge-contract-test.rb — 5 runs, 107 assertions
• ruby skills/pr-batch/bin/goal-completion-contract-test.rb — 76 runs, 1,086 assertions
• hostile cross-repository malformed record under --repo example/one --format decision — nonzero exit and empty stdout
• malformed repository/date/PR-count exclusions and raw duplicate identities — all fail before filtering
• ADR-style nested placeholders — rejected; embedded angle-bracket prose — accepted
• required and optional non-autonomous embedded legacy placeholders — rejected recursively
• duplicate-paragraph mutation — owning-section drift detected
• empty [] and {} outside autonomous_merge — both fail closed
• git diff --check origin/main...HEAD — clean

Full validation:

• bin/validate — PASS on the exact head, including 106 RuboCop files with no offenses
• stage dependency gate — eligible on the exact head; exact-head CI, independent review, unresolved threads, and merge readiness remain preserved gates

Hosted current-head CI, configured reviews, the paginated unresolved-thread audit, and fresh independent QA r5 all pass on the exact head.

Risk and rollback

Confidence is high locally because each accepted failure mode has a direct hostile regression and the full repository validation is green. The corrective branch remains shadow-safe: enforcement must remain disabled if the calibration contract is rolled back or becomes incomplete.

Rollback is a revert of 2d9275bf760f60aba5dae381562dfc35bd4814b1, 04ca1ee757b1acde5e8c54d201a3d398aee5c3d7, 206e6462def52d57ffa2f5516d90d84a5f4a41b2, 811d1c96a830d3e605baa0e7d5811d0491e6d08a, and 1b0b24c7e0de8b6148e6e55967665b8bf2aa884b.

Merge authority

ask: do not merge without explicit maintainer authorization after all current-head gates pass.

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

[coderabbitai]

Walkthrough

The changes update autonomous-merge policy placeholder handling, validate all calibration PR records before filtering, and synchronize two new completion states across canonical tests and workflow documentation.

Changes

Autonomous-merge policy validation

Layer / File(s)	Summary
Autonomous-merge policy resolution bin/agent-workflow-seam-doctor, bin/agent-workflow-seam-doctor-test.rb	Empty collections are accepted only under autonomous_merge; full-string angle placeholders remain unresolved, while prose containing angle brackets is accepted and empty collections elsewhere remain invalid.

Calibration repository validation

Layer / File(s)	Summary
Validation before calibration filtering skills/pr-batch/bin/autonomous-merge-calibrate, skills/pr-batch/bin/autonomous-merge-calibrate-test.rb	Every calibration PR is validated for repository shape, identity, timestamps, metrics, categories, and duplicate identity before repository, date, or count filtering.

Completion-state vocabulary

Layer / File(s)	Summary
Canonical completion-state alignment skills/pr-batch/bin/goal-completion-contract-test.rb, skills/pr-batch/SKILL.md, workflows/pr-processing.md	Adds ready-human-review-required and autonomous-merge-evidence-unknown, then verifies both documentation checklists match the canonical vocabulary.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

• shakacode/agent-workflows#265 — Updates both completion-state lists and adds synchronization contract tests.
• shakacode/agent-workflows#263 — Validates calibration PR repository shapes before filtering.

Possibly related PRs

• shakacode/agent-workflows#22 — Connects to the seam-doctor policy and template validation behavior.
• shakacode/agent-workflows#52 — Connects to completion-contract validation of readiness vocabulary.
• shakacode/agent-workflows#252 — Modifies autonomous-merge policy parsing and validation behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the PR’s main theme: fixing autonomous-merge issues found in reviews.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/fix-autonomous-merge-review-findings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review

Reviewed the diff (gh pr diff 267) against the stated goal of fixing the three adversarial-review findings from #252. All three fixes look correct and are appropriately scoped.

1. bin/agent-workflow-seam-doctor — accept autonomous_merge: {} seed

• Adding next if key == "autonomous_merge" (line ~1669) correctly excludes this key from the generic unresolved_policy_value? sweep, which would otherwise flag an empty mapping ({}) as unresolved (Hash#empty? → true).
• Verified this doesn't create a validation gap: autonomous_merge is not in REQUIRED_POLICY_KEYS, and it's still separately validated a few lines down via AutonomousMergePolicy.parse(...) whenever config.key?("autonomous_merge"). That parser correctly treats {} as "no overrides, use portable defaults," and still rejects non-Hash values ("autonomous_merge must be a mapping") and unknown nested keys. So malformed values still surface an error — just via a different, more specific message than before. No regression.
• New tests cover the accept case, the accept-with-empty-arrays case, and confirm the skip is scoped to only this key (custom_runtime_paths: [] still correctly fails).

2. skills/pr-batch/bin/autonomous-merge-calibrate — fail closed in --repo mode

• Good catch and correct fix. The old code only validated PR shape/repository-string when options[:repositories] was empty; in --repo mode it silently dropped anything that wasn't is_a?(Hash) && repositories.include?(pr["repository"]), so a malformed or cross-repo record would just vanish from calibration without any signal.
• The new validate_calibration_pr_repository! is now called unconditionally via prs.each { |pr| validate_calibration_pr_repository!(pr) } before filtering, so corrupt entries abort the run regardless of --repo usage. The subsequent .select { |pr| repositories.include?(pr.fetch("repository")) } is then safe to use fetch since every entry has already been validated.
• Minor (not a bug, just worth noting): this does mean a malformed record for an unrelated repository elsewhere in a large shared dataset will now abort a --repo-scoped calibration run that wouldn't have used that record anyway. That appears to be the intentional fail-closed tradeoff described in the PR body, and seems reasonable for a tool that's feeding merge-eligibility decisions.
• The refactor extracting validate_calibration_pr_repository! out of validate_calibration_pr_identity! is clean and doesn't change identity-validation behavior (still regex-checks OWNER/REPO form, still checks a positive integer number).
• New regression test (test_every_pr_repository_shape_is_validated_before_repository_filtering) exercises both the --repo and dataset-scope paths and asserts the specific abort message — solid coverage of the actual finding.

3. Workflow/skill doc parity for new completion states

• skills/pr-batch/SKILL.md and workflows/pr-processing.md now both list ready-human-review-required and autonomous-merge-evidence-unknown alongside the other completion states, matching CANONICAL_READINESS_STATES in skills/pr-batch/bin/goal-completion-contract-test.rb and the other docs that already had them (skills/spec/SKILL.md, skills/triage/SKILL.md, skills/pr-monitoring/SKILL.md, etc.).
• The new test_completion_state_checklists_match_canonical_readiness_vocabulary test parses the "Completion states" paragraph out of both files via regex and diffs it against the canonical list, which should catch future drift in either direction (missing or extra/misordered states).

Other notes

• Diff is tightly scoped to the seven files needed for these three fixes — no unrelated churn.
• I wasn't able to execute the Ruby test suites directly in this environment (sandboxed), so I verified correctness by tracing the logic by hand and cross-checking against AutonomousMergePolicy.parse, REQUIRED_POLICY_KEYS, and the other completion-state doc sources; the PR description's reported local test results (33/295, 268/2294, 75/1084 assertions, all green) are consistent with what I'd expect given the code changes.
• No security concerns — if anything, both code changes tighten validation (fail closed) rather than loosening it. No performance concerns; all changes are on cold validation/reporting paths, not hot loops.

Overall: looks correct, well-tested, and consistent with the "ask" merge-authority note in the PR description (holding for explicit maintainer sign-off given this touches autonomous-merge enforcement paths seems right).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b0b24c7e0

ℹ️ 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".

[claude]

Review

Overview

This PR fixes three findings from the adversarial review of #252:

1. skills/pr-batch/bin/autonomous-merge-calibrate — fail-closed fix: previously, --repo-filtered runs skipped validating that every prs[] entry was a well-formed {repository, ...} mapping, so malformed/cross-repo records could silently vanish via the select filter instead of aborting. The fix extracts validate_calibration_pr_repository! and now validates every record unconditionally before filtering, in both the --repo and dataset-scope paths. Well-tested by the new test_every_pr_repository_shape_is_validated_before_repository_filtering, which explicitly exercises both code paths.
2. bin/agent-workflow-seam-doctor — accepts the closed-schema autonomous_merge: {} seed and known-runtime-valid empty arrays (human_review_paths, policy_paths, generated_paths) while still rejecting nested placeholder scalars like <base branch>. Covered by new tests including a regression test (test_empty_arrays_outside_autonomous_merge_remain_unresolved) confirming other policy keys are unaffected.
3. skills/pr-batch/SKILL.md / workflows/pr-processing.md — restores parity by adding the ready-human-review-required and autonomous-merge-evidence-unknown completion states to the "Completion states" checklists, which had drifted out of sync with the canonical vocabulary already used elsewhere (ADR 0003, pr-monitoring, triage, plan-pr-batch). A new contract test (test_completion_state_checklists_match_canonical_readiness_vocabulary) guards against future drift between these two surfaces.

Code quality

• The refactor of validate_calibration_pr_identity! into a shared validate_calibration_pr_repository! helper is clean and removes duplicated inline validation logic.
• Test additions are proportionate and specific to each fixed defect (no unrelated churn).
• Left two non-blocking inline notes: one on the scope of the empty_collections_unresolved propagation in the seam doctor (worth confirming intent), and one minor redundant-validation nit in the calibrate script.

Security / correctness

• No security concerns — this is validation/consistency tooling, no external input execution, no injection surface.
• The --repo fail-closed fix is a genuine correctness improvement: previously a corrupt calibration record could be dropped from consideration without any signal, which is exactly the kind of silent-failure risk that undermines an enforcement decision.

Test coverage

Good — every fix has a direct regression test, and the PR description reports full local validation (bin/validate clean, RuboCop clean across 106 files). Didn't find gaps in what's covered relative to the stated scope of the fix.

Overall: a well-scoped, well-tested bug-fix PR. No blocking issues found.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

bin/agent-workflow-seam-doctor-test.rb (1)

483-494: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover empty mappings outside autonomous_merge too.

The implementation changes both Array and Hash handling, but this regression test only exercises custom_runtime_paths: []. Add custom_runtime_paths: {} to verify the default unresolved behavior remains intact for hashes as well.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/agent-workflow-seam-doctor-test.rb` around lines 483 - 494, Extend
test_empty_arrays_outside_autonomous_merge_remain_unresolved to also run with
custom_runtime_paths set to an empty hash, and assert the same unsuccessful
status and unresolved-policy message. Preserve the existing empty-array case
while covering both empty Array and Hash handling outside autonomous_merge.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@skills/pr-batch/bin/goal-completion-contract-test.rb`:
- Around line 574-581: Update
test_completion_state_checklists_match_canonical_readiness_vocabulary so its
Completion states extraction is anchored to the owning section for each
document, rather than matching the first occurrence anywhere. Ensure actual is
derived from the checklist paragraph within that section, while preserving the
existing canonical vocabulary comparison.

---

Nitpick comments:
In `@bin/agent-workflow-seam-doctor-test.rb`:
- Around line 483-494: Extend
test_empty_arrays_outside_autonomous_merge_remain_unresolved to also run with
custom_runtime_paths set to an empty hash, and assert the same unsuccessful
status and unresolved-policy message. Preserve the existing empty-array case
while covering both empty Array and Hash handling outside autonomous_merge.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2bc8a591-5e4e-4574-9658-8e3e8fecd270

📥 Commits

Reviewing files that changed from the base of the PR and between 137f8b9 and 811d1c9.

📒 Files selected for processing (7)

• bin/agent-workflow-seam-doctor
• bin/agent-workflow-seam-doctor-test.rb
• skills/pr-batch/SKILL.md
• skills/pr-batch/bin/autonomous-merge-calibrate
• skills/pr-batch/bin/autonomous-merge-calibrate-test.rb
• skills/pr-batch/bin/goal-completion-contract-test.rb
• workflows/pr-processing.md

[claude]

Review: Fix autonomous merge review findings (#267)

Reviewed via gh pr diff 267 plus the surrounding source (bin/agent-workflow-seam-doctor, skills/pr-batch/bin/autonomous-merge-calibrate, skills/pr-batch/bin/goal-completion-contract-test.rb, and the two markdown docs) to confirm each change matches its stated rationale.

Overview

Four independent fixes, each with targeted regression tests:

1. autonomous-merge-calibrate: validates every calibration entry's repository shape before --repo filtering, instead of only when no --repo filter is given.
2. agent-workflow-seam-doctor: unresolved_policy_value? gains an empty_collections_unresolved: keyword so autonomous_merge: {} and empty arrays inside autonomous_merge (e.g. human_review_paths: []) are accepted, while nested placeholder scalars anywhere in the tree are still rejected.
3. workflows/pr-processing.md + skills/pr-batch/SKILL.md: adds the two new completion states (ready-human-review-required, autonomous-merge-evidence-unknown) to the "Completion states" checklist in both places so they stay in parity.
4. goal-completion-contract-test.rb: new completion_state_checklist helper scopes the "Completion states" paragraph extraction to the owning section (Required Interview / Short Invocation) rather than matching globally, with a dedicated test proving an earlier duplicate paragraph can't mask drift.

Correctness

• Calibrate fix is a real fail-closed improvement, not just a refactor. The old --repo path only checked pr.is_a?(Hash) and .selected on repository match — a malformed or wrong-repo entry silently disappeared instead of aborting. The new validate_calibration_pr_repository! runs unconditionally over the raw dataset first, so corrupt entries now abort regardless of --repo filtering. Confirmed by tracing the control flow; the new test_every_pr_repository_shape_is_validated_before_repository_filtering test covers both the --repo and dataset-scope paths with the two now-fixed failure modes (non-Hash entry, and an entry with a repository string that isn't strictly OWNER/REPO).
• Worth flagging as an intentional behavior change (not a bug): with --repo, every PR in the dataset must now have a well-formed repository field, even ones in repos the caller isn't filtering for. That's the whole point of the fix, but a dataset with unrelated cross-repo garbage will now hard-fail a narrowly-scoped --repo run where it previously wouldn't have. Given the PR's own framing ("corrupt or cross-repository records cannot disappear from enforcement decisions") this looks intended.
• Seam-doctor change is scoped correctly: empty_collections_unresolved: key != "autonomous_merge" only relaxes the check for the autonomous_merge top-level key, and AutonomousMergePolicy.parse (in bin/agent_doctor/autonomous_merge_policy.rb) already treats all autonomous_merge subkeys as optional with sane defaults, so relaxing the generic empty-collection heuristic there doesn't create a validation gap — the dedicated parser still checks shape/values. String placeholders (e.g. <base branch>) are still caught at any nesting depth since that check is independent of the new keyword.
• Verified the do...end blocks in the Array/Hash branches of unresolved_policy_value? bind to value.any? (not misparsed against the ||), since there's no enclosing paren-less command call to steal the block — the standard, correct pattern, not the classic puts foo.each do...end precedence gotcha.
• The doc/test parity changes are consistent: both workflows/pr-processing.md and skills/pr-batch/SKILL.md list the same 9 states in the same order, matching the test's CANONICAL_READINESS_STATES array.

Minor nits (non-blocking)

• validate_calibration_pr_repository! is now called twice per selected PR (once in the new upfront prs.each validation pass, once again inside validate_calibration_pr_identity!). Harmless given typical calibration dataset sizes, but could be collapsed later if it bothers anyone.

Test coverage

Each fix has a directly corresponding regression test (nested-placeholder rejection, empty-collection acceptance/rejection, pre-filter validation ordering, doc/test parity with a mutation-detection sanity check for the new markdown-scoping helper). I wasn't able to execute the Ruby test suites directly in this sandbox (tool restriction on running interpreters), so correctness was verified by static trace rather than by re-running bin/agent-workflow-seam-doctor-test.rb / autonomous-merge-calibrate-test.rb / goal-completion-contract-test.rb myself — the PR description's reported counts (269 runs / 2,299 assertions, etc.) are consistent with the code paths added.

Security

No security concerns; both the calibrate and seam-doctor changes move validation strictness in the fail-closed direction, which is the intended direction for this policy-enforcement tooling.

Overall: focused, well-tested fixes that match their stated rationale. No blocking issues found.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 04ca1ee757

ℹ️ 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".

[claude]

Review Summary

Overview: this PR addresses three blocking findings from review of #252 — fail-closed validation ordering in the calibration script, a corrected placeholder-detection regex plus an autonomous_merge: {}/empty-collection carve-out in the seam doctor, and doc/test parity for two new completion states (ready-human-review-required, autonomous-merge-evidence-unknown). All changes are narrowly scoped with dedicated regression tests.

Correctness — skills/pr-batch/bin/autonomous-merge-calibrate

• The core fix is solid: full per-record validation (validate_calibration_pr!) now runs over all raw dataset entries before any --repo/--since/--pr-count filtering, instead of only validating whatever survived a repo-based filter. Previously, --repo <x> silently dropped malformed or cross-repo records without ever validating them — a real fail-open gap for a system that gates autonomous merge decisions. Good catch, well tested (test_hostile_cross_repo_record_cannot_emit_an_enforced_decision, test_raw_dataset_identities_must_be_unique_before_repository_filtering, etc.).
• Left an inline note on a subtler side effect: the strict count-field check now gates on declared_scope_complete alone rather than effective_scope_complete (which also required the query window to be covered by the dataset's declared scope). This looks intentional and correct given effective_scope_complete is still used properly for the report's scope_complete field and the window-coverage graduation blocker, but it's a behavior change not mentioned in the PR's "Exact evidence" section and worth a one-line confirmation.
• Identity uniqueness is now checked over the full raw dataset rather than just the filtered view — also correct and tested.

bin/agent-workflow-seam-doctor

• unresolved_template_value? switching from keyword-matched substring regexes (SEAM_PLACEHOLDER/CI_PARITY_SEAM_PLACEHOLDER) to a single full-string anchor (FULL_STRING_ANGLE_PLACEHOLDER = /\A<[^<>\n]*[^\s<>\n][^<>\n]*>\z/) is a good generalization: it stops flagging legitimate prose that happens to contain <base branch>-style text mid-sentence, while still catching any full-value placeholder (not just the previously hardcoded keyword list), which is actually broader/more correct coverage than before.
• Left an inline note: CI_PARITY_SEAM_PLACEHOLDER now appears to have no remaining callers in the file — worth removing as dead code.
• The empty_collections_unresolved: keyword threaded through unresolved_policy_value? correctly scopes the "empty array/hash is unresolved" rule to be skipped only for the autonomous_merge key (and everything nested under it), which matches the stated intent of accepting the closed-schema {} seed and its runtime-valid empty arrays, while still flagging empty collections elsewhere (custom_runtime_paths test). Regex change doesn't affect REQUIRED_POLICY_KEYS semantics since those are typically non-bracket strings.
• No ReDoS/perf concerns — all new/changed regexes are linear (no nested unbounded quantifiers).

Docs / test parity (skills/pr-batch/SKILL.md, workflows/pr-processing.md, goal-completion-contract-test.rb)

• The new completion_state_checklist test helper correctly scopes extraction to the owning markdown section before matching the Completion states: paragraph, and the added test_completion_state_checklists_ignore_earlier_duplicate_paragraphs test is a nice regression guard against the exact "duplicate paragraph masks drift" failure mode called out in the PR description.
• Verified the two new states are consistently listed in skills/pr-monitoring/SKILL.md, skills/triage/SKILL.md, and skills/plan-pr-batch/SKILL.md as well, so parity looks complete across the surfaces that reference the canonical vocabulary, even though only two of them are exercised by the new contract test.
• Minor: completion_state_checklist will raise NoMethodError on nil if extract_markdown_section doesn't find the heading (test-only code, low risk).

Security

No security concerns beyond the validation-ordering fix itself, which if anything improves the fail-closed posture of the autonomous-merge calibration pipeline (bad/foreign data can no longer silently disappear before being judged).

Overall

Focused, well-tested fixes for real correctness gaps. No blocking issues found; the two inline notes are informational/cleanup rather than bugs.

[claude]

Review Summary

This is a well-scoped corrective PR addressing four adversarial-review findings from #252. I reviewed the diff against the checked-out merge commit and traced the logic carefully; I could not execute the Ruby test suites in this sandbox (commands were blocked by the environment), so correctness below is based on static analysis plus the extensive test additions in the diff itself.

What it does

• bin/agent-workflow-seam-doctor: adds a placeholder_mode: (:legacy vs :full_string) and empty_collections_unresolved: parameter to the policy-value validators. autonomous_merge now uses a full-string anchored placeholder regex (FULL_STRING_ANGLE_PLACEHOLDER) so an entire value like <base branch> is rejected while ordinary prose that merely contains <...> text is accepted, and empty {}/[] are accepted only for autonomous_merge (an explicit closed-schema seed), while remaining fail-closed everywhere else. All keyword call sites were updated consistently.
• skills/pr-batch/bin/autonomous-merge-calibrate: the real bug fix. Previously, --repo filtering happened via Array#select before per-record identity/shape validation, so a malformed record for an unrequested repository (or a non-Hash entry) would silently disappear instead of failing the run. Now every raw record is validated (validate_calibration_pr!) up front, before repository/date/PR-count filtering, and identity uniqueness is checked against the full raw set. Good, targeted fix with a direct hostile-input regression test (test_hostile_cross_repo_record_cannot_emit_an_enforced_decision).
• workflows/pr-processing.md / skills/pr-batch/SKILL.md: restores the two completion states (ready-human-review-required, autonomous-merge-evidence-unknown) to the "Short Invocation"/"Required Interview" checklists so they match the canonical vocabulary already documented elsewhere (ADR 0003, other skills).
• goal-completion-contract-test.rb: adds a section-scoped extraction helper (completion_state_checklist) plus a regression test guarding against an earlier-duplicate-paragraph masking a real drift in the target section.

Findings

Left one inline comment on skills/pr-batch/bin/autonomous-merge-calibrate (line 163): the fix bundles two changes — (1) validating before filtering (the documented fix) and (2) switching the per-record "PR-count fields must be Integer" check from effective_scope_complete (scope.complete && window_covered) to declared_scope_complete (scope.complete alone). The second change means a dataset with scope.complete: true will now hard-abort on any record with nil metric fields even when the requested --since/--pr-count window isn't covered by the checkpoint — previously that combination degraded gracefully to a shadow disposition with a graduation blocker. This looks like an intentional part of the fail-closed tightening, but I didn't find a test exercising that exact combination, so it's worth an explicit confirmation + regression test.

Other observations (non-blocking)

• The recursive unresolved_policy_value? full-string check for autonomous_merge won't catch a placeholder value that spans multiple lines (the regex excludes \n in its character classes, consistent with the existing legacy SEAM_PLACEHOLDER), so a multi-line-only placeholder value would slip through. This matches pre-existing convention rather than being introduced by this PR, so low priority.
• Test coverage is thorough: the new/renamed calibrate tests correctly replace previously-malformed fixture PRs with valid ones now that malformed records can no longer survive to the filtering-boundary tests, and the seam-doctor tests exercise both the accept (empty seed, embedded prose) and reject (nested full-token, ADR rationale, glob placeholders) paths for autonomous_merge.
• Security-relevant surface (policy/placeholder validation and calibration evidence integrity) is the focus of this PR, and the changes uniformly move toward fail-closed behavior rather than away from it — no new security concerns identified.

Overall: solid, narrowly-scoped fix with good regression coverage. The one item flagged above is worth a maintainer confirmation before merge given the "ask" merge authority already specified in the PR body.

[justin808]

Completed-batch audit: replay evidence follows.

[justin808]

Address-review closeout for exact head 2d9275bf760f60aba5dae381562dfc35bd4814b1.

Mattered and resolved:

• Fable B1-B3 from merged PR Implement autonomous merge eligibility gates #252 were independently reproduced and fixed by this corrective PR.
• Codex’s nested-placeholder finding was fixed at 811d1c96a830d3e605baa0e7d5811d0491e6d08a.
• CodeRabbit’s owning-section and empty-hash proof gaps were fixed at 206e6462def52d57ffa2f5516d90d84a5f4a41b2.
• Fresh QA’s raw-record and ADR-placeholder attacks were fixed at 04ca1ee757b1acde5e8c54d201a3d398aee5c3d7.
• Codex’s current-head embedded-placeholder regression was fixed at 2d9275bf760f60aba5dae381562dfc35bd4814b1.

Optional observations were adjudicated and resolved: autonomous-merge empty collections remain owned by the closed-schema parser; complete checkpoints validate required metrics before filters; the restored legacy mode keeps the CI-parity matcher live. Positive/status-only and stale old-head artifacts were skipped as non-actionable.

Final evidence: fresh QA r5 PASS, hosted validate PASS, Claude PASS, CodeRabbit PASS, paginated review-thread inventory complete with zero unresolved threads, and no viewer-pending review drafts. Durable audit: #267 (comment)

Canonical state: ready-no-merge-authority; merge_authority: ask.