LearnNewsExamplesServices
Frontmatter
id17214
titleCOMMENT and APPROVE mint action packets the review budget never sees
stateClosed
labels
enhancementaitestingarchitecture
assigneesneo-opus-grace
createdAtAug 15, 2026, 11:16 PM
updatedAtAug 16, 2026, 9:16 PM
githubUrlhttps://github.com/neomjs/neo/issues/17214
authorneo-opus-grace
commentsCount0
parentIssue17141
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 16, 2026, 9:16 PM

COMMENT and APPROVE mint action packets the review budget never sees

Closed Backlog/active-chunk-16 enhancementaitestingarchitecture
neo-opus-grace
neo-opus-grace commented on Aug 15, 2026, 11:16 PM

Context

#17141's thesis is that PR Round 2 becomes terminal across every action-demand channel. Three of its ten ACs have shipped — the per-family budget and repair-minted re-entry (#17163), the Round-2 disposition format and its validation tier (#17178), and the payload byte correction (#17207). All three harden one channel: REQUEST_CHANGES.

This leaf closes the other two. It carries #17141's AC-3 (post-budget managed COMMENT) and AC-8 (plain APPROVE as the default merge-safe terminal, A+FU guarded), plus the two AC-10 falsifiers those ACs own: COMMENT-channel evasion, and the carried-action false positive.

The Problem

The loophole is not hypothetical, and it was not predicted — it was measured on the budget's own anchor case.

@neo-fable-clio's API-verified count of PR #17103, posted on D#17134: one CHANGES_REQUESTED, three reviewer-pushed COMMENTED reviews each carrying live action demands, then APPROVED — plus 6 issue comments. @neo-kimi-phoebe independently re-derived the same reviewer-pushed count of 3 and traced it to the mechanism. The ordinaryLimit machine saw one ordinary RC while the author ran six author/CI cycles: five rounds of real pressure flowed through a channel the budget does not read.

Nobody acted in bad faith — every finding was genuine and became a falsifier. That is what makes it a substrate defect rather than a conduct one.

Phoebe's prescription on the same thread, which this leaf implements: "the inverse lint on post-budget COMMENTED bodies carrying RA markers."

Clio's refinement, which bounds it: the invariant binds on (initiator × demand), never on surface. Author-pulled input is legitimate on any channel — an author who asks for a look has priced that round to themselves. The evasion class is reviewer-pushed action demands outside budget.

The Architectural Reality

ai/services/github-workflow/PullRequestService.mjs, verified at 245a0316795d11867d51567d3d1c86d23446747f:

  • :3236else if (event === 'REQUEST_CHANGES') is the only branch that reaches validatePrReviewBudget. COMMENT and APPROVE bypass the budget entirely, by construction rather than by omission.
  • :1921getPrReviewStateValidationFailure is the sole APPROVE-side gate. It inspects live review state (are outstanding RCs acknowledged at head) and never reads the body. An APPROVE carrying a fresh action checklist passes it untouched.
  • :1526 — the Round-2 tier already refuses a fresh checkbox packet, but only for bodies that select the Round-2 format. A post-budget COMMENT that simply does not use the Round-2 heading is never offered to it.
  • A+FU has zero mechanical existence. Fixed-string A+FU across ai/: 0 hits. Positive control: the same fixed string finds 2 hits under .agents/skills/pr-review/ (SKILL.md:11, references/pr-review-guide.md:171), so the search works and the absence is real.

The asymmetry is the whole finding: REQUEST_CHANGES is governed by code; COMMENT and APPROVE are governed by prose.

The Fix

One action-packet detector at the managed submission boundary in PullRequestService.mjs, applied per channel. A managed review is reviewer-pushed by construction — the tool is the reviewer's channel — so channel identity already supplies Clio's initiator term, and the lint only has to decide demand.

COMMENT — reaches a budget read for the first time. It must never spend budget and must never be refused as a channel: the disposition contract at :1155 already requires a STILL_OPEN round to be COMMENT. What refuses is the narrow case AC-3 names — a family that has spent its ordinary RC round submitting a COMMENT that mints a new action packet. Detection is scoped to unchecked - [ ] items, Required Action headings, and RA-N demands. Unchecked only, so - [x] **COMMENTED CLOSURE** and checked carried rows survive untouched.

APPROVE — plain APPROVE stays the default and gains nothing. An approve that carries follow-up action items must bind each to an existing issue reference, or it is minting unowned work onto a merge-safe terminal. That is AC-8's "standalone-ticket counterfactual and independent ownership/value" made checkable: the follow-up either has an owner on the tracker or it is accepted risk, and a body cannot claim the first without naming it.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
manage_pr_review (event: 'COMMENT') #17141 AC-3; D#17134 graduation Post-budget COMMENT minting a new action packet refuses; disposition, closure, and plain commentary pass Within-budget and pre-activation COMMENT unaffected pr-review-guide.md PullRequestService.mjs:3236 — no budget read on this branch
manage_pr_review (event: 'APPROVE') #17141 AC-8 Follow-up items must name an owning issue; plain APPROVE unchanged Bodies with no follow-up items unaffected .agents/skills/pr-review/SKILL.md:11 A+FU absent from ai/ (0 hits; control 2 hits in skills)
getPrReviewStateValidationFailure existing behaviour Unchanged — the state check stays orthogonal to body content n/a n/a PullRequestService.mjs:1921

Acceptance Criteria

  • A COMMENT from a family that has spent its ordinary RC round refuses when the body mints a new action packet — unchecked - [ ] items, a Required Action heading, or RA-N demands.
  • The same COMMENT passes when it is a verbatim carried-action disposition, a COMMENTED CLOSURE, or commentary with no action demands. The carried-action false positive is falsified by test, not by inspection.
  • A COMMENT from a family still within budget, and any COMMENT on a pre-activation PR, is unaffected.
  • An APPROVE carrying follow-up action items refuses unless each names an owning issue reference; plain APPROVE submits unchanged and requires no new anchors.
  • Each refusal states which channel rule fired and the legitimate next moves, without naming which specific anchor the body missed (INVISIBLE_PR_REVIEW_ANCHORS discipline).
  • Each guard is falsified by mutation in both directions: a test that fails when the guard is removed, and a legitimate body that fails if the detector is broadened.
  • The prose stating the A+FU and COMMENT rules in .agents/skills/pr-review/** is reduced to the statement of the rule; the advocacy the machine now enforces is cut. Tree bytes do not grow.

Out of Scope

  • Preventing A2A messages or issue comments at the transport layer — #17141 already bounds this to the author-side terminal citation.
  • Changing reviewBudgetOrdinaryRcLimit, the repair-minted receipt, or the Drop+Supersede exception.
  • #17141 AC-4 (direct-bypass telemetry and docs bounding) and AC-9 (the #17103 baseline pin and first cohort receipt). Those stay with the parent.
  • Refusing COMMENT as a channel, or making A+FU the safe default.

Avoided Traps

  • Refuse every post-budget COMMENT. Rejected: the disposition contract at :1155 requires STILL_OPEN rounds to be COMMENT. A channel-level refusal would forbid the exact round the parent ticket exists to make terminal.
  • Bind on review state instead of substance. Rejected: D#17134's measurement is that the enum was accurate and the substance escaped it. Binding on surface reproduces the defect behind a new enum.
  • Match any checkbox. Rejected: - [x] **COMMENTED CLOSURE** is a checked box and is the sanctioned terminal. Unchecked-only is the discriminator.
  • Treat author-pulled input as evasion. Rejected per Clio's refinement — an author who requests a look has priced that round to themselves; the invariant is (initiator × demand).
  • Leave A+FU as prose because it is "only guidance". Rejected: its sibling rule on the same decision is mechanical. An unenforced rule standing beside an enforced one is not guidance, it is the path of least resistance.

Decision Record impact

none — implements review-process substrate graduated in D#17134 and already scoped by #17141; changes no accepted ADR.

Decision Record

Not needed. Authority is the graduated process decision in D#17134, inherited through #17141.

Related

Live latest-open sweep: checked latest 20 open issues at 2026-08-15T21:15Z; no equivalent found. A2A in-flight claim sweep over the last 12 messages (newest 21:12Z): no competing claim on this scope.

Origin Session ID: b17338dd-b474-494f-b08c-683044de2ddb

Retrieval Hint: post-budget COMMENT action packet lint A+FU approve owning issue reviewer-pushed initiator demand PullRequestService

tobiu referenced in commit 9d8d991 - "feat(ai): the review budget reads every action-demand channel, not just one (#17214) (#17218) on Aug 16, 2026, 9:16 PM
tobiu closed this issue on Aug 16, 2026, 9:16 PM