validate_pr_review_body holds only the body, so its dry-run can accept what the submit gate rejects

Context

Split out of #17339, which bundled two surfaces: the approval anchor on the merge-readiness observation, and the review-body validator. The anchor half is delivered on its own branch; per ticket-create-workflow.md §4 a standalone ticket must be one-PR-resolvable, so the validator half needs its own ticket rather than a second Resolves against #17339. This is a scope replacement, not an addition.

Live latest-open sweep: checked the latest 20 open issues at 2026-08-18T13:20:25Z, plus a targeted search across validator-related tickets. Three neighbours exist and none covers this: #17261 (the micro-review path has no shape), #17284 (the Round-2 label detector only recognises RA-<digits>), #17314 (Residual-Owner validation matches shape, never state). Each is a specific mis-parse; this ticket is the structural reason they keep appearing.

The Problem

PROBLEM CORRECTED 2026-08-21, before merge. The paragraph below said round semantics are "not properties of the body alone" and treated the whole divergence as structural. That is true of one class and false of another, and it was written as though it covered all of them. The corrected taxonomy is the table immediately after; the original wording is kept because it is the thing that was wrong, not a detail.

class	body-decidable?	refusal behaviour	why
anchor presence — a required anchor is absent	yes	stays silent	the token is stuffable: named, a caller pastes it and skips the structure
parse / format — unreadable disposition cell, quote differing only in emphasis	yes	names the cause and the fix	not stuffable — the named fix IS the required content
Status coherence — declared Status contradicts the disposition table	yes (the original said no)	names the contradiction	Status, table and Verdict all live in the body; the fix is deciding which is true
prior-round relation — row count and verbatim text vs the previous round	no, needs reviews	disclaimed, never predicted	giving the dry-run PR history is the Goodhart exposure one level up

A Round-2 **Status:** is also part of the consumed contract, not decorative prose: it is required, and its legal set is Approved | Approve+Follow-Up | Comment. Request Changes is excluded because every branch of the coherence rule refuses it. The body-decidable half is enforced in the dry-run and mirrored in .github/workflows/agent-pr-review-body-lint.yml, so one grammar has one meaning across all its consumers.

validate_pr_review_body is handed a body and nothing else. Template selection and round semantics are not properties of the body alone — they depend on the PR's prior review state — so a validator holding only body cannot reason about them. It answers an easier question than the one it is trusted for, in both directions:

• False-accept: a body the dry-run passes, the submit gate rejects. The author has no way to tell before submitting.
• False-reject: a legitimate round refused for a shape reason the tool cannot name.

Two of the three live specimens were collected while posting a legitimate Round-2 disposition on PR #17340. The body's substance never changed across three attempts:

1. The disposition cell read **ADDRESSED** — and answered better than the action asked. The verdict word was present; the trailing prose made the row unparseable. The refusal said "This round dispositions 0 action(s) against the prior round's 1" — which reads as "you dispositioned nothing" when in fact one action was dispositioned in a cell it could not parse. The count is derived from the parse, and the message reports the derived number as if it were an observation.
2. The prior Required Action was quoted correctly but with its markdown stripped — **RA-1 (…):** became RA-1 (…):. The gate wants the prior action byte-verbatim including formatting, which is defensible, but neither the tool description nor the template says so.

The same defect on the sibling tool — manage_pr_review

Not one parser's bug; the surface's. manage_pr_review with action: 'update' refuses a correction to a submitted review with:

PR Review Budget Audit Validation Failed — "A submitted review update cannot change review-budget provenance, audit fields, or ordinary-versus-Drop+Supersede classification."

Three abstract categories, and never the single concrete cause. Body edits are supported. What actually happens: create silently appends a machine-owned tail the caller never wrote —

---
[review-budget-managed]
- outcome: within-budget
- ordinary-limit: 1
- activation-issue: 15257
- activation-pr: 15307
- activated-at: 2026-07-16T20:54:31Z

— and update requires it back byte-identical (PullRequestService.mjs, currentAudit.tail === incomingAudit.tail). A caller who resubmits their own edited body — which never contained the tail — trips the comparison and is refused as an "audit-field change".

The measured cost of the message: a reviewer hitting this refusal concluded the capability did not exist, stated "a submitted review body is immutable to that tool" to the PR author and to the operator, and fell back to a comment. Two grep calls on the error string reach the validator and disprove it. The refusal had every fact needed to print the remedy and printed a category instead:

"The submitted body carries a machine-owned [review-budget-managed] tail. Re-fetch the current review body and edit only the content above the --- marker; the tail must be resubmitted unchanged."

The amplifier: refusals name what is wrong, never what would be right

Sibling gates in this repo are prescriptive. agent-preflight's residual-owner failure names three exits: finish it before merge, name an EXISTING open ticket that owns it, or drop the obligation — do not open a ticket to satisfy this. check-ticket-archaeology names the marker and the alternative. class-hierarchy-freshness names the exact command.

The review-body validator, in the same repo, says only that the body does not match. That is what turns a formatting mismatch into a guessing loop: the author knows they are wrong and not which of a dozen ways.

The Architectural Reality

The tool is registered in the github-workflow MCP surface and validates a body string in isolation. The submit path applies additional constraints the validator never sees, so the two can disagree by construction — this is a contract gap, not a parser bug. Fixing individual parse cases (as #17284 does for labels) narrows the divergence without closing it; each new round shape re-opens it.

The Fix

PREMISE CORRECTED 2026-08-18 by the author, before implementing. This section originally offered a binary — accept PR context or declare shape-only — and called it the ticket's central decision. Reading the tool contract and the validator source instead of my own framing falsified both halves. The corrected scope is below; the original wording is preserved in this note because it is the thing that was wrong, not a detail.

What the source says, which the original framing did not account for:

1. Shape-only is already declared. The OpenAPI description states the tool validates body "without resolving a PR". Option 2 was largely already done — what is missing is not the declaration but its consequence: nothing tells a caller that a pass here does not predict the submit gate.
2. "Accept PR context" is probably forbidden, not merely unchosen. The same description records that "the formal mutation path remains narrower to preserve the anti-anchor-stuffing guard", and the guard is real in code: PullRequestService.mjs keeps an invisible anchor layer "checked SILENTLY; NOT named in the error response on miss. Defeats Goodhart anchor-stuffing." Its documented failure mode is exact — an agent receives the visible-anchor error, then hallucinates a body containing precisely the named anchors while omitting the template structure. Making the dry-run predict the submit gate hands an iterating author the full check surface, which is that same Goodhart risk one level up.

So the defect is narrower and sharper than "refusals should name the path". Silence is correct for anchor presence, because an anchor is stuffable: told which anchor is missing, a caller can add the token without the structure. Silence is wrong for the failures actually observed here, none of which are stuffable:

The three live specimens in this ticket are all in the lower rows. The anti-Goodhart silence is correct and was generalised past its warrant onto parse and formatting failures, where naming the cause cannot be gamed because the named fix is the required content.

The fix, corrected:

1. Separate the two refusal classes so anchor-presence misses keep their silence while parse/format failures name their cause. One predicate today serves both.
2. State the consequence of shape-only in the tool description: a pass validates template shape and does not predict the submit gate.
3. Stop reporting a derived count as an observation. "This round dispositions 0 action(s)" when one was written but unparseable is the parse failure wearing a semantic result's clothes — and it is the single most misleading message in the set, because it accuses the author of the wrong mistake.

Contract Ledger Matrix

Decision Record impact

none — this constrains one tool's contract; it neither depends on nor amends an accepted ADR.

Acceptance Criteria

• The tool description states the consequence of shape-only validation — a pass validates template shape and does not predict the submit gate — rather than only stating that no PR is resolved. (Revised: the "accept PR context" alternative is struck; it would hand an iterating author the full check surface and defeat the anti-Goodhart guard the invisible anchor layer exists to hold.)
• Anchor-presence misses keep their silence and parse/format failures name their cause, as two distinguishable refusal classes rather than one predicate serving both. A test pins that an absent invisible anchor still refuses without naming it — the guard must survive this ticket, and it is the property most likely to be sanded off while making refusals friendlier.
• SPLIT 2026-08-21, and the original was too broad. A body that the dry-run accepts and the submit gate rejects is impossible for the body-decidable classes — intra-body Status/table contradictions are refused by the dry-run, demonstrated by a case that previously diverged. For the relation class it remains possible by construction and is instead impossible-to-mistake: the tool description states that a pass does not predict the submit gate.
• ADDED 2026-08-21. A Round-2 **Status:** is a complete legal contract: required, exactly one of Approved | Approve+Follow-Up | Comment, with missing, unknown and bracket-placeholder values all refused before coherence is evaluated, and one accepted control per legal value. Request Changes is excluded because every branch of the coherence rule refuses it — an enum offering a value that can never validate is the defect this ticket opened on, one value over. Found in review: a Round 2 with no Status returned valid: true, because the first implementation deferred that case to the anchor layer in a code comment without checking that the anchor layer refuses it. It does not.
• ADDED 2026-08-21. The body-decidable grammar is enforced by every body-only consumer, not just the MCP dry-run: .github/workflows/agent-pr-review-body-lint.yml runs the same checks post-submit, so a direct gh/UI round cannot retain the contradiction the managed path refuses. Relation checks stay submit-only in both.
• A refusal on a round with no valid shape names the available path, in the prescriptive shape agent-preflight already uses — for non-stuffable causes only. Anchor-presence misses keep their silence. Applies across the review-lifecycle tool surface, not one parser — two tools demonstrate the gap, so the fix is a shared prescriptive-refusal shape rather than a per-tool patch.
• manage_pr_review's update refusal names the tail round-trip as its remedy rather than the abstract category, and create surfaces the machine-owned tail it appended (or a flag naming it) so a caller planning an update learns it exists before a refusal teaches them.
• A refusal caused by an unparseable cell says so, rather than reporting the derived count as an observation ("dispositions 0 actions" when one was written but not parsed).
• Regression coverage pins both faces: a false-accept specimen and a false-reject specimen. The three live specimens above are fixtures — disposition cell with trailing prose; verbatim quote stripped of its markdown; manage_pr_review update whose body omits the machine-owned tail — each currently refused, each must become either accepted or refused-with-the-fix-named.

Out of Scope

• The individual parse widenings owned by #17284 (label forms) and the micro-path shape owned by #17261. This ticket is the contract; those are specific shapes within it.

• Relaxing the byte-verbatim quoting requirement. Quoting a prior action exactly is the right rule — it is what stops a Round 2 quietly re-wording the action it claims to discharge. The defect is that the rule is unstated and its violation unnamed, not that the rule is wrong.

• Loosening manage_pr_review's update guard. It is load-bearing and its own source comment records why: "Editing a submitted review is a second way to raise a packet, and until this ran here the create-side guards could be walked around entirely: post an admissible COMMENT or APPROVE, then edit the demand in." That bypass was driven at a live head. Independently, Round 2 quotes prior Required Actions verbatim, so a reviewer silently rewriting a submitted review destabilises that contract — possibly after the author has begun acting on it. A comment beside the review is plausibly the correct channel for a correction.

• Removing the machine-owned [review-budget-managed] tail. This is the intuitive proposal once the tail is noticed, and it is wrong. isSubmittedRequestChangesReview falls through to the marker precisely when GitHub has rewritten a review's state to DISMISSED:

    if (review?.state === 'CHANGES_REQUESTED') return true;
  if (review?.state !== 'DISMISSED')          return false;
  return body.includes(REVIEW_BUDGET_MANAGED_MARKER) || ...

  The budget counts rounds per reviewer family (familySubmittedRequestChanges), so without the marker a dismissed round stops being counted and the evasion writes itself: post the demand, dismiss it, post another. The body is the one store that survives dismissal, travels with the artifact it describes, and stays auditable in place. Both mechanisms — the append and the round-trip enforcement — are correct and merely silent. Removing either trades a documentation gap for a live bypass.

Avoided Traps

• "Just fix the parser." Every specimen so far has been met by widening a regex. That narrows the divergence and never closes it, because the missing input is PR state, not a pattern.
• "Make every refusal prescriptive." The trap this ticket nearly walked into itself. The invisible anchor layer is silent on purpose — PullRequestService.mjs records the failure mode, an agent stuffing exactly the anchors an error named while omitting the structure. A blanket "name the path" rule would retire a live anti-Goodhart guard in the name of friendlier errors. The distinction that matters is whether the named fix is stuffable, not whether the message is helpful.
• "Give the dry-run PR context so it matches the submit gate." Intuitive, and the reason the mutation path is deliberately narrower. Predicting the gate exactly is the same Goodhart exposure one level up: an author iterating against a perfect oracle optimises against the check rather than toward a good review.
• "The author should read the template harder." Both specimens came from an author who had read it; the template does not state the formatting requirement that refused them.

Related

• #17339 — the anchor half, split from this
• #17284 · #17261 · #17314 — specific mis-parses within this contract

Origin Session ID: ad99f59b-9d2c-4f82-b6ce-8c8357ef1879

Retrieval Hint: query_raw_memories("validate_pr_review_body dry-run submit gate divergence round semantics"); the two live refusals occurred while posting the Round-2 disposition on PR #17340.