LearnNewsExamplesServices
Frontmatter
id17141
titleMake PR Round 2 terminal across every action-demand channel
stateOpen
labels
enhancementairefactoringtestingarchitecture
assigneesneo-opus-grace
createdAtAug 15, 2026, 12:00 AM
updatedAtAug 16, 2026, 5:18 AM
githubUrlhttps://github.com/neomjs/neo/issues/17141
authorneo-gpt-emmy
commentsCount5
parentIssuenull
subIssues
17214 COMMENT and APPROVE mint action packets the review budget never sees
subIssuesCompleted1
subIssuesTotal1
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]

Make PR Round 2 terminal across every action-demand channel

Open Backlog/active-chunk-16 enhancementairefactoringtestingarchitecture
neo-gpt-emmy
neo-gpt-emmy commented on Aug 15, 2026, 12:00 AM

Context

Discussion #17134 graduated after same-day evidence showed Neo's review-budget machine counts formal CHANGES_REQUESTED objects while action-demand pressure migrates through COMMENTED, PR comments, and A2A. The anchor PR #17103 recorded one ordinary RC but three reviewer-pushed COMMENTED rounds and six issue comments; the author still ran six repair/CI cycles. The current managed-path refusal makes this worse by explicitly saying “Use COMMENT for the RC2 closure packet.”

The converged decision is not softer review. It is one brutal Round 1 per reviewer family, author fix-or-defend, then a micro disposition-only Round 2. A rare repair-minted re-entry exists only with a causal old-head/new-head receipt. Every reviewer-pushed author-action demand is budget-bearing by substance, regardless of GitHub state.

Graduation fold: https://github.com/neomjs/neo/discussions/17134#discussioncomment-18022614

Live latest-open and A2A duplicate sweeps are recorded at creation time below.

The Problem

PullRequestService currently owns a global reviewBudgetOrdinaryRcLimit: 2 and counts submitted CHANGES_REQUESTED reviews. Once exhausted, its refusal at ai/services/github-workflow/PullRequestService.mjs:2011-2018 instructs reviewers to move closure into COMMENT; the managed state guard at :2648-2653 only enforces that direction. It does not reject a post-budget COMMENT that introduces another Required Actions packet.

The skill repeats the old economics. .agents/skills/pr-review/SKILL.md:11 routes every Cycle N to the full 89-line follow-up template; that template asks for a fresh premise snapshot, Strategic-Fit, Delta Depth Floor, conditional audits, evidence, contract completeness, metrics, and Required Actions. The conditional review-cost-circuit-breaker.md:7-22 still defines two ordinary RCs and an RC2 COMMENTED closure.

The result is a formally clean ledger over an operationally unbounded loop. Reviewers can always find a defensible next improvement; each author delta reruns expensive CI and consumes the capacity that could have delivered another feature or regression repair.

The Architectural Reality

  • PullRequestService.managePrReview() is the existing atomic admission point for managed formal reviews. It already reads the current PR review population, classifies terminal Drop+Supersede, enforces a numeric budget, validates closure-state markers, appends a durable audit, and exposes an explicit override. This is the owning runtime substrate; no second review tracker is needed.
  • Canonical reviewer-family facts live in the agent identity graph (ai/graph/identityRoots.mjs / identity-era projection), not in login-prefix inference. Family budget classification must consume that authority or fail closed when the reviewer cannot be classified.
  • The pr-review router is already eight lines of hot substrate. Per create-skill progressive-disclosure rules, it must not grow; terminal-round details belong in the conditional review-cost payload and compact Round-2 asset.
  • Direct gh/UI and A2A cannot be prevented by the managed mutation alone. Existing post-submit telemetry can diagnose GitHub bypasses; the author-side terminal-state citation remains the enforcement right for non-GitHub A2A demand. The ticket must not pretend one regex controls every transport.
  • Scoped structure map at creation: ai/services/github-workflow contains the 2,113-LOC owning PullRequestService.mjs among 15 direct siblings; .agents/skills/pr-review contains an 8-LOC router plus conditional assets/audits. Full-repo structure map hit its existing max-string failure; both owning roots mapped successfully.

The Fix

  1. Change the ordinary budget from two global RCs to one ordinary RC per canonical reviewer family per PR. Count across heads and retractions so a routine repair does not reset the budget.
  2. Retain one exceptional repair-minted re-entry through the existing override path only when its receipt names the prior head, repaired head, the Round-1-head fact that made the defect nonexistent/undiscoverable, and the exact repair coordinate that created/exposed it. One such re-entry is terminal after disposition; “noticed later” is invalid.
  3. Replace the post-budget COMMENT instruction. On the managed path, classify reviewer-pushed COMMENT bodies by substance: a new ### Required Actions, unchecked action checkbox, or new RA-* demand after the family budget is exhausted refuses. A bounded carried-action disposition, APPROVED verdict, or complete Drop+Supersede remains admissible.
  4. Preserve direct-gh/UI bypass telemetry and add the same semantic diagnostic where the post-submit workflow has enough review history. Do not claim hard prevention for A2A; document the author's right to decline an out-of-budget reviewer-pushed demand by citing the terminal receipt. Author-pulled input remains legitimate on every channel.
  5. Replace the broad Cycle-N follow-up asset with a micro Round-2 disposition table over the verbatim existing actions: ADDRESSED, DEFENDED, or STILL_OPEN. A still-open action keeps the original RC authoritative; it does not create a new action list. Drop fresh metrics, broad audit reruns, and a mandatory new Depth Floor from ordinary Round 2. Preserve full structure only for a validated Drop+Supersede.
  6. Make plain APPROVE the default good outcome. Approve+Follow-Up is allowed only when the finding passes the standalone-ticket counterfactual: it would be filed if discovered outside review, has independent value/ownership, and the current head is merge-safe.
  7. Retain PR-body truth folding as the bounded current head. Round 2 consumes PR body + exact delta + the original action packet, not the unbounded comment thread.
  8. Add a bounded cohort receipt using existing GitHub data: repair turns, first-RC→terminal wall time, all-channel review bytes, CI retriggers caused by review deltas, escaped regression/revert count, existential defects first discovered after Round 1, and repair-minted re-entry count.

Contract Ledger

Target Surface Source of Authority Proposed Behavior Fallback / Edge Case Docs Evidence
PullRequestService.reviewBudgetOrdinaryRcLimit D#17134 graduation fold 2 → 1, scoped per canonical reviewer family and counted across PR heads/retractions Unknown family fails closed with an identity diagnostic; no login-prefix inference JSDoc + review-cost payload same-family/different-family mutation matrix
Review-family classification canonical agent identity graph / active identity era Resolve review author to stable family before budget accounting unclassified reviewer cannot spend or bypass a family budget silently JSDoc at resolver active identities + unknown + renamed-handle fixtures
reviewBudgetOverrideReason repair-minted re-entry D#17134 Ada row E + fold One causal re-entry names old/new heads, prior-head fact, and repair coordinate missing/malformed/second re-entry refuses; ordinary later discovery does not qualify review-cost payload four-field validation + second-use refusal
Post-budget COMMENT admission managePrReview() existing audit path Reject reviewer-pushed new action packets after the family budget; admit carried dispositions, approval, or complete D+S Direct UI/gh is diagnosed post-submit; A2A remains author-enforced, not falsely claimed blocked service JSDoc + payload COMMENT action-demand false-positive/negative corpus
Round-2 review asset D#17134 terminal disposition contract Compact verbatim carried-action table: addressed / defended / still-open; no new action packet Existential repair-minted defect routes through guarded re-entry or D+S asset + guide pointer template validator fixtures and byte delta
A+FU disposition D#17134 standalone-ticket counterfactual APPROVE by default; A+FU only for independently valuable standalone work on a merge-safe head “tick nicer” finding dies in review; no micro-ticket guide/payload positive existing-owner case + micro-nit refusal
Cohort receipt D#17134 prospective falsifier Publish bounded all-channel economics and escape-rate receipt for the first cohort If trigger fires, reopen the decision; remove temporary cohort instrumentation after receipt ticket closeout comment exact GitHub corpus receipt

Discussion Criteria Mapping

D#17134 graduation criterion Ticket AC
One comprehensive Round 1; Round 2 terminal AC 1, 4, 5
ordinaryLimit: 2 → 1 or justified alternative AC 1
Action demands bind by substance, not channel enum AC 3, 4
Repair-minted surface gets rare named-fact re-entry AC 2
Round 1 must execute or demand an executable falsifier AC 6
Net-reduce review substrate AC 5, 7
Prospective escaped-regression falsifier AC 8

Acceptance Criteria

  • One ordinary CHANGES_REQUESTED from a reviewer family spends that family's budget across later heads and retractions; another active family retains its independent one-round authority.
  • A second ordinary RC from the same family refuses unless one unused repair-minted receipt passes the full old-head/new-head/prior-fact/repair-coordinate validator; a second re-entry refuses.
  • Post-budget managed COMMENT with new Required Actions / unchecked action items / RA demands refuses, while a verbatim carried-action disposition, APPROVED review, and complete terminal Drop+Supersede remain valid.
  • Direct GitHub bypasses are reported by the existing audit path where observable; docs explicitly bound A2A enforcement to the author-side terminal citation rather than claiming impossible transport control.
  • Cycle 2's asset is disposition-only over verbatim prior actions. STILL_OPEN preserves the original RC without creating a new action packet; ordinary fresh findings become accepted risk.
  • Round-1 guidance requires artifact/population execution or a carried executable falsifier, including citation-inversion, full-suite, and claimed-domain boundary checks derived from the measured escape ledger.
  • The always-loaded pr-review/SKILL.md router has zero positive byte delta; total .agents/skills/pr-review/** bytes and procedural steps net-decrease. The PR body includes the /turn-memory-pre-flight Map-vs-Atlas audit.
  • Plain APPROVE is the default merge-safe terminal outcome. A+FU validates the standalone-ticket counterfactual and independent ownership/value.
  • The exact #17103 baseline is pinned as 1 RC + 3 reviewer-pushed COMMENTED rounds + 6 issue comments; the first cohort receipt publishes all declared metrics and a revalidationTrigger verdict.
  • Focused tests independently falsify same-family reset, cross-family suppression, COMMENT channel evasion, malformed re-entry, second re-entry, carried-action false positive, direct-bypass telemetry, and template-byte regression.

Out of Scope

  • Weakening the cross-family approval requirement or the human-only merge gate.
  • Preventing A2A messages at the transport layer.
  • Turning contribution throughput into a quota or merge criterion.
  • Adding a new review tracker, skill, always-loaded rule body, or generic workflow framework.
  • Grandfathering new ordinary action packets onto open PRs after cutover.

Avoided Traps

  • Only change 2 → 1. Rejected: the current refusal explicitly routes overflow into COMMENT, so this would move the loophole one cycle earlier.
  • Count only formal review states. Rejected: #17103 is the live falsifier; one formal RC drove six author cycles.
  • Ban all later comments/A2A. Rejected: author-pulled input and bounded carried-action dispositions are legitimate; initiator × demand is the meaningful discriminator.
  • Add another prose gate. Rejected: the implementation must net-reduce skill substrate and reuse the existing managed audit path.
  • Make A+FU the safe default. Rejected: prior enforcement polluted the backlog with micro-tickets; plain APPROVE is the healthy terminal state.

Decision Record impact

none — this changes review-process substrate and its machine enforcement; it does not change an accepted architectural ADR.

Decision Record

Not needed. Authority is the graduated process decision in D#17134.

Related

Unresolved Liveness

  • Owner: ticket assignee.
  • Expiry: publication of the first bounded post-cutover cohort receipt.
  • revalidationTrigger: escaped regressions materially rise; existential defects routinely first appear after Round 1; repair-minted re-entry fires routinely; formal RC count falls without all-channel review-byte or wall-clock improvement.
  • Retirement: remove temporary cohort instrumentation after the receipt; retain only the stable budget/disposition contract.

Origin Session ID: 019fe0b3-53bc-7ef2-8665-41a0ef3f7b62

Retrieval Hint: D#17134 terminal PR review COMMENT overflow ordinaryLimit family surface-version repair-minted re-entry

tobiu referenced in commit b027c0c - "feat(ai): charge the review budget per reviewer family, and make the exception checkable (#17163) (#17164) on Aug 15, 2026, 2:53 PM
tobiu referenced in commit d710fd9 - "feat(ai): Round 2 becomes a disposition over Round 1, with its own validation tier (#17179) on Aug 15, 2026, 7:39 PM