Require final-head QA evidence after later commits

[justin808]

Lane Card

• Thread: awfb-coord-orchid
• Batch/lane: awfb-20260709-1753 / final-head-qa; dashboard_url: UNKNOWN
• Target: Follow-up: Require final-head QA evidence after post-QA commits #81
• Branch: codex/awfb-81-final-head-qa; pr_url: Require final-head QA evidence after later commits #108
• Phase: final-gates; claim: m5-codex-awfb-81 / 1 / m5-codex-awfb-81-20260710T051000Z; coordinator: /root

Summary

• Add an exact full-head field to replayable QA Evidence.
• Let the closeout replay helper compare required QA against an expected final PR head.
• Fail closed on stale, missing, contradictory, reversed-range, or duplicate scalar evidence while preserving historical no-option replay.
• Require post-QA commits to invalidate and replay affected QA before readiness or merge.

Closes #81.

TDD evidence

• Initial RED: the old helper did not support --expected-head-sha (1 run, 1 assertion, 1 failure).
• Review-fix RED: duplicate old/final head_sha values plus stale tested_at incorrectly returned SATISFIED (1 run, 2 assertions, 1 failure).
• Range-direction RED: tested_at: FINAL..LATER incorrectly returned SATISFIED (1 run, 2 assertions, 1 failure).
• Range-compatibility RED: the first contradiction guard incorrectly rejected a valid BASE..FINAL range (1 run, 2 assertions, 1 failure).
• Final focused suite: 25 runs, 108 assertions, 0 failures.

Validation

• Current head 829c4bc73cd304ded68c444c741d10ec42a858f6:
  • ruby skills/post-merge-audit/bin/closeout-evidence-replay-test.rb — PASS (25 runs, 108 assertions)
  • implementation-stage isolated timing replay — PASS (1 run, 7 assertions); full timing-helper suite — PASS (14 runs, 81 assertions)
  • coordinator and independent-QA bin/validate replays — PASS; final groups 97/792, 32/232, and 14/43; RuboCop inspected 43 files with no offenses
  • ruby -c skills/post-merge-audit/bin/closeout-evidence-replay — PASS
  • git diff --check origin/main...HEAD — PASS
  • hosted Validate and Claude Code Review — PASS on this exact head
• Direct behavior:
  • stale or missing final-head QA — UNKNOWN
  • duplicate scalar QA/priority fields — UNKNOWN with deterministic errors
  • BASE..FINAL or single final-head evidence — SATISFIED
  • FINAL..LATER — UNKNOWN
  • uppercase/lowercase full SHA equivalents — normalized and accepted
  • required-no and legacy no-option replay — unchanged

Review and change control

• Worker self-review and coordinator type-design review completed.
• Independent report-only review found two fail-open boundary cases; both were reproduced RED, fixed, and re-reviewed clean with no remaining blocking, discuss, or optional findings.
• The configured local second-model review was attempted but could not start because its service usage quota was exhausted; it is not counted as evidence. Hosted current-head Claude review passed.
• Claude's one schema-consistency nit was auto-deferred/declined with rationale and no post-QA code churn. Its multi-marker note confirms the intentionally conservative fail-closed behavior.
• CodeRabbit could not start because of its service quota and produced no findings; no repeat request was made during the quota window.
• Changelog classification: changelog_present; this changes shipped developer closeout behavior and has an Unreleased Fixed entry.
• Review-fix rounds before push: 2; post-push commits: 0; review-thread code-fix rounds: 0; human decision points: 0.

New-gate stale-base race control

Selected control: sweep all open PRs that touch the newly enforced developer-workflow/QA closeout surface, then require affected PRs to update to post-merge main and replay final-head QA before readiness.

• PR Add model routing to PR batch planning #106 at 27dbb5e7267e211a0aff303eb8aa16940478e960 is draft/dirty, edits workflows/pr-processing.md, and has no QA marker; expected-head replay is UNKNOWN.
• Final sweep: PR Add one-command consumer seam initialization #107 at 2a1f204cb63136d7ccd29802494f00b9635cf1e5 is non-draft/UNSTABLE, has required QA still in_progress, and has no head_sha; expected-head replay is UNKNOWN.
• Neither affected in-flight PR qualifies under this new gate. After this PR lands, each must update to current main, decide/finish required QA at its then-final head, and run per-PR --expected-head-sha replay before merge.

Codex Decision Log

• Non-blocking: issue paths were initially unknown.
  • Decision: remap only the canonical closeout checklist, existing replay helper/test, and changelog.
  • Why: the helper supplies executable evidence while workflows/pr-processing.md is the canonical pre-merge checklist; $pr-batch already delegates there.
  • Review later: None.
• Non-blocking: head_sha and human tested_at can disagree.
  • Decision: require the expected head in both fields and reject repeated scalar keys; for ranges, the expected head must be the final full SHA token.
  • Why: this rejects stale/contradictory evidence without breaking valid BASE..FINAL audit ranges.
  • Review later: None.
• Non-blocking: multiple historical QA markers can coexist in one body.
  • Decision: retain the existing aggregate rule that any UNKNOWN marker keeps the result UNKNOWN.
  • Why: stale evidence must be refreshed or replaced; a newer marker must not mask an older contradictory record.
  • Review later: None.
• Optional review: make every fallback result include empty diagnostic keys.
  • Decision: decline after final-head QA.
  • Why: fallback outputs retain their historical minimal shape; additive diagnostic keys are optional, and this polish would invalidate current-head evidence without fixing behavior.
  • Review later: None.

QA Evidence

• QA lane: m5-codex-awfb-qa, independent read-only worktree /Users/justin/codex/agent-repos/worktrees/awfb-qa-108; target claim released; shared heartbeat done
• Scope checked: final-head helper, duplicate scalar protection, SHA/range matching, CLI compatibility, regression suite, canonical workflow guidance, and repository-wide validation
• Tested at: PR Require final-head QA evidence after later commits #108 head 829c4bc73cd304ded68c444c741d10ec42a858f6
• Automated checks: focused replay suite 25 runs/108 assertions; exactly one independent-QA bin/validate exit 0; final marker replay SATISFIED with no missing fields/errors
• Manual checks: 10-case real CLI matrix covering matching/stale/duplicate/range direction/required-no/legacy/argument ordering/case normalization
• Findings: none; one non-failing local mise migration warning did not affect exit 0
• QA required: yes
• QA required rationale: this changes replay and merge-closeout workflow behavior
• QA lane status: satisfied
• Release-blocking status: clear
• Process-gap disposition: checklist+replay

[coderabbitai]

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e6cd530-f694-4467-aa4a-d66c284d2fdb

📥 Commits

Reviewing files that changed from the base of the PR and between bd198d0 and 829c4bc.

📒 Files selected for processing (4)

• CHANGELOG.md
• skills/post-merge-audit/bin/closeout-evidence-replay
• skills/post-merge-audit/bin/closeout-evidence-replay-test.rb
• workflows/pr-processing.md

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/awfb-81-final-head-qa

---

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: Require final-head QA evidence after later commits (#108)

Overview

Adds an --expected-head-sha option to closeout-evidence-replay that gates required: yes QA evidence on matching the current final PR head (both the head_sha field and the last full SHA token in tested_at), rejects duplicate scalar marker keys instead of silently overwriting them, and updates workflows/pr-processing.md to require re-running QA replay against the true final head before merge. Solid, well-scoped change with strong TDD-style test coverage for the new gate (stale head, duplicate keys, reversed ranges, case normalization, not_applicable passthrough).

Code quality

• The new option parsing via OptionParser is a clean upgrade from manual argv handling, preserves prior -h/--help/no-args exit codes, and correctly rejects trailing unexpected positional args.
• SHA comparisons are consistently downcased on both the CLI-provided value and marker fields — avoids case-sensitivity foot-guns.
• The tested_at_matches_head? regex is simple/linear, no ReDoS concerns, and correctly requires a full un-abbreviated 40-char SHA.
• I traced the fail-closed logic by hand against all the new tests (stale head, duplicate head_sha + stale tested_at, stale tested_at alone, missing head_sha, matching case, case-normalization, BASE..FINAL range acceptance, FINAL..LATER rejection) — all check out correctly and match the implementation.
• Left one inline nit: the new "errors" (and "expected_head_sha") key isn't added to the unknown(reason) / priority_not_applicable(reason) fallback builders used when no markers are present at all, so the JSON schema is inconsistent depending on whether markers exist.

Potential design consideration (not a blocker)

aggregate_verdict (pre-existing, unchanged) still applies "any UNKNOWN marker poisons the whole result" across all qa-evidence markers in a body. Combined with the new --expected-head-sha gate, if a PR body accumulates multiple historical qa-evidence markers (older + current), a stale/superseded marker will force the aggregate to UNKNOWN even when a current, correctly-updated marker is also present and would satisfy the gate on its own. Likely intentional (fail closed), but not covered by the new tests — only duplicate keys within a single marker are tested, not multiple separate qa-evidence marker blocks combined with --expected-head-sha. Worth an explicit test/doc note confirming this is the desired behavior.

Security

No security concerns — local CLI parsing PR-body text/files, no shell-out, no injection surface. The stricter full-SHA validation and case normalization are good hardening relative to the prior loose usage checks.

Test coverage / docs

• New tests are thorough and directly mirror the PR description's "TDD evidence" (stale head, duplicate keys, reversed range, range-compatibility, not_applicable passthrough, full-SHA validation error message).
• workflows/pr-processing.md updates clearly document the new gate, the "commit after QA invalidates evidence" rule, and case-normalization/duplicate-key behavior — consistent with the code.
• CHANGELOG entry is appropriately placed under Fixed and matches the existing terse, bold-summary style.

Overall: a well-tested, fail-closed improvement to the QA evidence replay gate. One minor schema-consistency nit left inline; one open design question about multi-marker aggregation noted above for discussion.

[justin808]

Address-review summary

Scan scope: full PR history through 2026-07-10T05:45:22Z; no earlier summary cutoff existed.

Mattered

• None. No MUST-FIX or DISCUSS findings were present.

Optional

• The fallback errors/expected_head_sha shape-consistency nit was auto-deferred/declined under the maintainer-attention contract. Existing no-marker fallbacks retain their historical minimal shape, additive diagnostics remain optional, and a post-QA code-only polish commit would invalidate exact-head evidence.
• Conservative multi-marker aggregation was explicitly affirmed: any stale or UNKNOWN QA marker keeps the aggregate UNKNOWN. Operators must refresh/replace stale evidence rather than append a newer marker that masks it.

Skipped

• CodeRabbit's quota notice was status-only and contained no review finding. Hosted current-head Claude review completed successfully.

No follow-up issue was created. Future full-PR scans should start after this comment unless check all reviews is requested.