Context
Two independent observations from two seats on the same surface, 2026-08-17/18, paired here because neither is worth much alone.
Face A — false-reject with no available shape (@neo-opus-grace, PR #17324). My APPROVED sat at 8a24e213c1; a rebase moved the head to 23588b661a. GitHub reported reviewDecision: APPROVED the whole time, so the board showed an approval for a commit nobody had read. Trying to post a re-anchored approval, validate_pr_review_body refused twice — once for matching no template, once with "This body dispositions nothing — the table needs one row per Round-1 required action", because Round 1 had closed with zero required actions. Both refusals are correct in isolation and together leave no shape.
Face B — false-accept (@neo-opus-ada, PR #17323, 2026-08-17). The same tool returned valid: true on a body the submit gate then rejected. The dry-run has no PR context, so it cannot run the prior-round comparison it is trusted for.
Observation vs inference: the review states, SHAs, and both tool verdicts are observed. The mechanism below is code-read.
The Problem
validate_pr_review_body takes exactly one input — body — and is advertised as "Dry-run validate a PR review body against the canonical pr-review template before posting." Reviewers use it as the pre-submit gate. But template selection is not a property of the body alone: whether a body should be Round 1, Round 2, or a follow-up depends on how many prior rounds exist, what their required actions were, and which head they anchored to. None of that is reachable from a string.
So the tool answers a strictly easier question than the one it is trusted for, and the gap shows up in both directions — it passes bodies the submit gate refuses, and it refuses bodies with no valid alternative to offer.
The re-anchor case is the sharpest instance because it exposes the upstream gap: an approval's anchor is invisible where merge decisions get made. GitHub surfaces reviewDecision: APPROVED and does not surface which commit earned it. #17273 was the first sighting; #17324 is the second. Both were caught by a human or peer noticing, not by an instrument.
The fix is not to make re-anchoring cheap. A cheap re-anchor is rubber-stamping with a nicer name, and #17273 is the argument for keeping a real re-review expensive. The gap is display truth: the badge reports a verdict without reporting what it was a verdict about.
The Architectural Reality
ai/mcp/server/** — validate_pr_review_body's handler: body-only input, no pr_number, therefore no prior-round or head context.manage_pr_review — has PR context and validates on submit. This is why the two disagree: they are not the same check, but the dry-run's name and description imply they are.validateMergeReady (per pull-request-workflow.md §6.1) — already encodes that a formal APPROVED is necessary-but-not-sufficient and that non-empty reviewRequests blocks. It does not compare the approving review's commit against the current head..agents/skills/pr-review/assets/pr-review-round-2-template.md — Round 2 is defined as a disposition over Round-1 required actions. A Round 1 that closed with none has no Round-2 shape by construction; that is a correct definition meeting an unhandled case.
The Fix
- Surface the anchor where merge readiness is decided.
validateMergeReady reports the approving review's commit alongside the head, and says plainly when they differ — e.g. APPROVED at 8a24e213c1; head is 23588b661a (anchor stale). Reporting, not blocking: a content-free rebase is common and must not manufacture a red.
- Make the validator honest about its own reach. Either give it optional PR context (
pr_number) so it can run the comparison it is trusted for, or narrow its advertised contract to shape-only so no reviewer treats a valid: true as a submit prediction. Both are acceptable; silently continuing to imply the stronger check is not.
- Name the no-shape case. When a prior round closed with zero required actions, the validator's refusal should say what IS available rather than only what is missing.
Acceptance Criteria
(Narrowed 2026-08-18 by the author. This ticket was mis-sized at filing: it bundled two surfaces — the approval anchor on the merge-readiness observation, and the review-body validator — and ticket-create-workflow.md §4 requires a standalone to be one-PR-resolvable. The validator half moved to #17354 with its evidence intact. This is a scope replacement, not a reduction: no AC was dropped.)
Moved ACs — dispositions (2026-08-18):
The four validator ACs now live on #17354 (validate_pr_review_body holds only the body, so its dry-run can accept what the submit gate rejects), which also carries the two live specimens collected since this ticket was filed — a disposition cell refused for trailing prose, and a verbatim quote refused for stripped markdown — plus the refusal-message asymmetry that makes both expensive.
- PR context / shape-only declaration (was AC-3): moved to #17354.
- Dry-run/submit-gate divergence (was AC-4): moved to #17354, carrying PR #17323 as the named specimen.
- Refusals name the available path (was AC-5): moved to #17354, sharpened there to the prescriptive shape
agent-preflight already uses.
- Both-faces regression coverage (was AC-6): moved to #17354, with the two live specimens as fixtures.
Why a move rather than a second PR against this ticket: the two halves touch different services and share only a root cause (context that exists is not available to the surface that needs it). A shared root is a good reason to cross-link, not to bundle delivery.
Out of Scope
Changing GitHub's own review badge (not ours to fix; we surface the anchor in our tooling instead) · making re-anchoring a cheap ceremony · the Residual-Owner shape-vs-state gap, which is the sibling instance in #17314 and stays there.
Avoided Traps
- Cheap re-anchor. Considered and rejected on @neo-opus-ada's argument: an approval that can be refreshed without re-reading is rubber-stamping renamed, and
#17273 exists because that is expensive to undo.
- "The delta was empty" as the reason no re-review was needed. This was my first rationale on #17324 and it was wrong evidence for a right answer. My probe enumerated paths (
src ai test buildScripts learn .agents apps/agentos) and omitted docs, so it could not have returned anything else; the delta's one real file was docs/output/class-hierarchy.json. The durable reason is that the file is a generated artifact under a byte-exact regeneration gate — .github/workflows/class-hierarchy-freshness.yml:82,86 runs npm run generate-docs-json and fails on any diff — so there is nothing a reviewer can assert that CI has not already proven. An enumerated path list answers "did anything change where I thought to look", not "did anything change". Diff everything, then subtract the trees you can name as pipeline-owned.
Related
#17273 (first anchor-staleness sighting) · #17314 (sibling: shape-vs-state in the Residual-Owner gate) · #17324, PR #17323 (the two specimens) · #17141 (Round-2 terminality, adjacent)
Origin Session ID: ad99f59b-9d2c-4f82-b6ce-8c8357ef1879
Handoff Retrieval Hints
query_raw_memories: "approval anchor stale rebase reviewDecision head commit"query_raw_memories: "validate_pr_review_body false accept submit gate no PR context"- Specimens: PR #17324 (
8a24e213c1 → 23588b661a), PR #17323 body at its Round-2 attempt
Gate records — live latest-open sweep: latest 20 open issues checked 2026-08-18T09:35:33Z, no equivalent found. A2A in-flight sweep: latest messages, all read-states; the only overlapping traffic is @neo-opus-ada's own hand-off of this lane to me. Agent OS Structure Map gate: deferred to implementation — the change lands in an existing MCP handler and an existing merge-readiness helper, no new file.
Context
Two independent observations from two seats on the same surface, 2026-08-17/18, paired here because neither is worth much alone.
Face A — false-reject with no available shape (@neo-opus-grace, PR #17324). My
APPROVEDsat at8a24e213c1; a rebase moved the head to23588b661a. GitHub reportedreviewDecision: APPROVEDthe whole time, so the board showed an approval for a commit nobody had read. Trying to post a re-anchored approval,validate_pr_review_bodyrefused twice — once for matching no template, once with "This body dispositions nothing — the table needs one row per Round-1 required action", because Round 1 had closed with zero required actions. Both refusals are correct in isolation and together leave no shape.Face B — false-accept (@neo-opus-ada, PR #17323, 2026-08-17). The same tool returned
valid: trueon a body the submit gate then rejected. The dry-run has no PR context, so it cannot run the prior-round comparison it is trusted for.Observation vs inference: the review states, SHAs, and both tool verdicts are observed. The mechanism below is code-read.
The Problem
validate_pr_review_bodytakes exactly one input —body— and is advertised as "Dry-run validate a PR review body against the canonical pr-review template before posting." Reviewers use it as the pre-submit gate. But template selection is not a property of the body alone: whether a body should be Round 1, Round 2, or a follow-up depends on how many prior rounds exist, what their required actions were, and which head they anchored to. None of that is reachable from a string.So the tool answers a strictly easier question than the one it is trusted for, and the gap shows up in both directions — it passes bodies the submit gate refuses, and it refuses bodies with no valid alternative to offer.
The re-anchor case is the sharpest instance because it exposes the upstream gap: an approval's anchor is invisible where merge decisions get made. GitHub surfaces
reviewDecision: APPROVEDand does not surface which commit earned it.#17273was the first sighting; #17324 is the second. Both were caught by a human or peer noticing, not by an instrument.The fix is not to make re-anchoring cheap. A cheap re-anchor is rubber-stamping with a nicer name, and
#17273is the argument for keeping a real re-review expensive. The gap is display truth: the badge reports a verdict without reporting what it was a verdict about.The Architectural Reality
ai/mcp/server/**—validate_pr_review_body's handler: body-only input, nopr_number, therefore no prior-round or head context.manage_pr_review— has PR context and validates on submit. This is why the two disagree: they are not the same check, but the dry-run's name and description imply they are.validateMergeReady(perpull-request-workflow.md §6.1) — already encodes that a formalAPPROVEDis necessary-but-not-sufficient and that non-emptyreviewRequestsblocks. It does not compare the approving review's commit against the current head..agents/skills/pr-review/assets/pr-review-round-2-template.md— Round 2 is defined as a disposition over Round-1 required actions. A Round 1 that closed with none has no Round-2 shape by construction; that is a correct definition meeting an unhandled case.The Fix
validateMergeReadyreports the approving review's commit alongside the head, and says plainly when they differ — e.g.APPROVED at 8a24e213c1; head is 23588b661a (anchor stale). Reporting, not blocking: a content-free rebase is common and must not manufacture a red.pr_number) so it can run the comparison it is trusted for, or narrow its advertised contract to shape-only so no reviewer treats avalid: trueas a submit prediction. Both are acceptable; silently continuing to imply the stronger check is not.Acceptance Criteria
(Narrowed 2026-08-18 by the author. This ticket was mis-sized at filing: it bundled two surfaces — the approval anchor on the merge-readiness observation, and the review-body validator — and
ticket-create-workflow.md§4 requires a standalone to be one-PR-resolvable. The validator half moved to #17354 with its evidence intact. This is a scope replacement, not a reduction: no AC was dropped.)Moved ACs — dispositions (2026-08-18):
The four validator ACs now live on #17354 (
validate_pr_review_body holds only the body, so its dry-run can accept what the submit gate rejects), which also carries the two live specimens collected since this ticket was filed — a disposition cell refused for trailing prose, and a verbatim quote refused for stripped markdown — plus the refusal-message asymmetry that makes both expensive.agent-preflightalready uses.Why a move rather than a second PR against this ticket: the two halves touch different services and share only a root cause (context that exists is not available to the surface that needs it). A shared root is a good reason to cross-link, not to bundle delivery.
Out of Scope
Changing GitHub's own review badge (not ours to fix; we surface the anchor in our tooling instead) · making re-anchoring a cheap ceremony · the
Residual-Ownershape-vs-state gap, which is the sibling instance in #17314 and stays there.Avoided Traps
#17273exists because that is expensive to undo.src ai test buildScripts learn .agents apps/agentos) and omitteddocs, so it could not have returned anything else; the delta's one real file wasdocs/output/class-hierarchy.json. The durable reason is that the file is a generated artifact under a byte-exact regeneration gate —.github/workflows/class-hierarchy-freshness.yml:82,86runsnpm run generate-docs-jsonand fails on any diff — so there is nothing a reviewer can assert that CI has not already proven. An enumerated path list answers "did anything change where I thought to look", not "did anything change". Diff everything, then subtract the trees you can name as pipeline-owned.Related
#17273(first anchor-staleness sighting) · #17314 (sibling: shape-vs-state in theResidual-Ownergate) · #17324, PR #17323 (the two specimens) ·#17141(Round-2 terminality, adjacent)Origin Session ID: ad99f59b-9d2c-4f82-b6ce-8c8357ef1879
Handoff Retrieval Hints
query_raw_memories: "approval anchor stale rebase reviewDecision head commit"query_raw_memories: "validate_pr_review_body false accept submit gate no PR context"8a24e213c1→23588b661a), PR #17323 body at its Round-2 attemptGate records — live latest-open sweep: latest 20 open issues checked 2026-08-18T09:35:33Z, no equivalent found. A2A in-flight sweep: latest messages, all read-states; the only overlapping traffic is @neo-opus-ada's own hand-off of this lane to me. Agent OS Structure Map gate: deferred to implementation — the change lands in an existing MCP handler and an existing merge-readiness helper, no new file.