ci(pr-automation): reserve turns a repair can spend

[Benoît Cortier (CBenoit)]

A semantic rejection lets a repair spend one tool-enabled call on evidence before it answers, so a repair attempt costs two provider calls wherever such a repair is possible and one otherwise. The runtime reserved a single call per attempt, so a long investigation left the last attempts unreachable: the stage died on the turn ceiling with its repair budget unspent, reporting an opaque limit rather than what the model kept getting wrong.

The general reviewer hit exactly this. It produced a review, had it rejected twice on the same validation error, then exceeded its turn count with one of its two repairs still unused, so the pull request went unreviewed while every job reported success.

Reserve the calls a repair can actually spend, withhold the evidence call when the answer after it would not fit, and raise the general reviewer's ceiling so its investigation window is unchanged.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Benoît Cortier (CBenoit)]

Investigation notes and validation detail, kept out of the commit message.

How the run was diagnosed

Run 34446208599 looked like a clean success, and the only annotation was maximum turn count exceeded, which reads as an infrastructure limit. The artifacts told a different story:

• review-general/result.json was {"output":"","reason":"maximum turn count exceeded"}.
• The stage diagnostics showed the model actually finished a review: a finalizing attempt with finishReason: "stop" and 1886 output tokens.
• outputRejections recorded two semantic rejections, both invalid specialist candidate dispositions; record exactly one disposition per specialist candidate and cite only non-rejected candidates as sources.
• turnCount: 30 against max_turns: 30, but outputRepairCount: 1 against max_output_repair_attempts: 2.

That last pair is the tell: the stage died with half its repair budget unspent.

Counting the providerAttempts array confirms the arithmetic: 27 investigating, 1 finalizing, then 2 repairing. The two repair calls were a single repair round, because a tool-assisted repair spends one call fetching evidence and a second returning corrected JSON.

The reporting path was not at fault

Worth stating explicitly, since the green run invites the opposite conclusion. Everything downstream handled the failure correctly:

• report logged {"status":"failed","failed":["general","validate"],"stage_retries":0}.
• resolve-review-state logged hasReview:false and failed:true.
• write-state wrote a neutral check; no ai-reviewed label and no review comment were posted on docs(agents): protect generated changelogs #1746.

The run stays green because review state is written to the pull request rather than by failing the job. That is by design, so the fix belongs in the runtime, not in reporting.

Why two turns per attempt, and not more

A repair round is genuinely bounded at two provider calls. toolsPermitted is cleared after the first tool round, so the next response must be tool-free or the runtime raises repair response attempted a tool call. Reserving 1 + 2 * max_output_repair_attempts is therefore exact rather than merely generous.

It does over-reserve for schema and JSON rejections, which never get tools and so cost one call per attempt. That is the safe direction: over-reserving costs investigation turns, while under-reserving silently discards configured repair attempts.

Why max_turns moved to 32

The fix alone would have cut the general reviewer's investigation window from 27 to 25. That reviewer had just saturated all 27 turns on a single-file docs change, so the reduction was not free. Raising max_turns to 32 keeps the window at 27, making the change behavior-neutral for investigation and purely additive for repair.

The specialists were left alone. They run at max_turns: 50 and lose 2 of 47 investigation turns, which is not worth tuning around.

Verifying the regression test actually catches the bug

The new test was checked against the unfixed runtime rather than assumed to be meaningful. Reverting only the reservation expression makes it fail with exactly the production error:

✖ a saturated investigation still leaves every repair attempt reachable
  AgentFailure: maximum turn count exceeded

It uses a client that returns a tool call whenever tools are advertised and a scripted answer otherwise, so it adapts to whatever investigation window the formula produces instead of hardcoding a turn sequence. That is what makes it sensitive to the reservation arithmetic specifically.

Test fixture changes

Five tests broke on the larger reservation, all for the same reason: they assert an activity label such as investigating on the first response, and the wider reservation shrank their investigation window to zero so that response was routed through finalize instead. The fixtures were given more turns to preserve each test's original window, rather than rewriting the assertions to match the new behavior.

Failure taxonomy is deliberately unchanged for genuine turn-limit configurations. limitFailure still reports maximum turn count exceeded with category limit, and the existing max_turns: 1 test still asserts exactly that.

Validation

The full CI sequence for the openai-agent job was run locally:

• node --test .github/pr-automation/automation.test.js — 115/115
• npm test — 74/74
• npm run build
• git diff --exit-code -- dist — clean

The dist/index.js rebuild matters: the action declares main: dist/index.js, so a src change without a rebuild would have shipped as a no-op.

Unrelated hypothesis that was ruled out

A parallel investigation suspected that manual workflow_dispatch reruns were losing the target pull request, because those runs report headBranch: master. That is expected rather than a defect. workflow_dispatch and repository_dispatch always run at a repository ref, so headBranch is the dispatch ref by construction. Targeting flows through resolve-pr, which resolves the pr-number input into pr-number, head-sha, and base-sha outputs consumed by every downstream job. This run demonstrates it: a repository_dispatch on master that carried head SHA b73b36b6 correctly through every job and artifact name.

Note

LLM-assisted content (no human feedback).

[Copilot]

🟡 Changes recommended

The unconditional reservation unnecessarily reduces investigation capacity for agents without tool-assisted semantic validation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes CI review automation so all configured output-repair attempts remain reachable.

Changes:

• Reserves two provider turns per tool-assisted repair.
• Preserves the general reviewer’s investigation budget.
• Adds regression tests and rebuilds the bundled action.

Protocol review was skipped because no RDP behavior changes.

File summaries

File	Description
.github/actions/openai-agent/src/agent.js	Revises turn reservation logic.
.github/actions/openai-agent/test/agent.test.js	Adds repair-budget regression coverage.
.github/actions/openai-agent/test/main.test.js	Adjusts fixture turn budgets.
.github/pr-automation/agents/general-reviewer.json	Preserves 27 investigation turns.
.github/actions/openai-agent/dist/index.js	Bundles the runtime change.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Benoît Cortier (CBenoit)]

Revised across two review rounds. Superseded by the notes below; kept for the record.

The reservation is conditional on a tool-assisted repair being possible, not merely on a repair happening: kind: "validator" is only reachable when runAgent is given a validator, and tools additionally require max_tool_calls > 0. The first version reserved two calls unconditionally and silently cost the classifier in labeler.yml a turn. Making it conditional also removed the need for the test fixture bumps that version carried, so main.test.js is no longer part of the diff.

A second guard was added after review: toolsPermitted now also requires state.providerCalls + 2 <= config.max_turns, so a repair does not spend its evidence call when the answer after it would not fit. For every current agent config that guard is always true, so production behaviour is unchanged; it only fires where the clamp bites, converting an opaque limit into the semantic rejection reason.

Validation

• npm test in .github/actions/openai-agent: 75/75.
• node --test .github/pr-automation/automation.test.js: 115/115.
• npm run build then git diff --exit-code -- dist: clean. The action runs dist/index.js, so a src-only change would ship as a no-op.
• Reverting the reservation expression alone fails both new tests. Reverting the toolsPermitted guard alone fails the undersized-ceiling test with maximum turn count exceeded - the same error run 34446208599 produced.

How run 34446208599 failed

The stage was not a review-generation failure and the publication path was not at fault. The model finished a review (finishReason: "stop", 1886 output tokens), and reporting worked correctly end to end: report logged {"status":"failed","failed":["general","validate"]}, resolve-review-state logged hasReview: false, failed: true, and write-state completed. The review was discarded because both candidates were rejected by validateGeneral, and the second repair attempt was unreachable.

With max_turns: 30 and max_output_repair_attempts: 2: 27 investigation calls, finalization on 28 (rejected), then one repair round on 29 (evidence) and 30 (answer, rejected). The loop then satisfied outputRepairs(1) < 2 but failed providerCalls(30) < max_turns(30). The outputRepairCount: 1 in the diagnostics is the tell.

Exhaustion now exits through the repair-limit branch, so the failure carries the actual validation reason (output remained invalid after the repair limit: semantic: ...) instead of an opaque limit. Retryability is unchanged, since AgentFailure defaults retryable to false for both categories, so the retry-gate behaviour in review-pipeline.yml is unaffected.

Still out of scope

• Why the dispositions were invalid. Both rejections were the generic catch-all from validateGeneral: invalid specialist candidate dispositions; .... The raw model output is not logged, so the specific trigger is unknown. A structured diagnostic (distinguishing a count mismatch from a bad reviewer, an unknown finding_id, or a rationale that fails normalization) would make these repairable, but that is a separate change to the validator rather than to turn accounting.
• A latent asymmetry in that validator. The schema caps rationale at 800 characters while normalizeText caps at 800 UTF-8 bytes, so non-ASCII text near the limit can pass the schema and fail the validator.
• Degenerate configs. With max_turns <= 3 and several repair attempts the clamp still leaves later attempts unfunded. Such a config asks for more repairs than its ceiling can pay for; rejecting it at load time is the principled fix and belongs with config validation.
• Specialist agents (max_turns: 50) lose 2 of 47 investigation turns, left alone deliberately.

On the general reviewer's 30 -> 32: this keeps its investigation window at 27 so the fix is behaviour-neutral for investigation, and spends the 2 extra calls only on the repair path that was broken. The counter-argument is fair - that PR was a single documentation file and still saturated 27 investigation turns and 38 tool calls, so the exploration budget may itself deserve scrutiny. Holding at 30 would also fix the bug, at the cost of narrowing investigation to 25. Happy to drop it to 30 if you would rather not carry the increase.

Note

Written by Copilot App (Claude Opus 5), on behalf of Benoît Cortier (@CBenoit).

[github-actions]

Independent review confirms the core fix: with a validator and tool budget, a semantic repair can spend two provider calls (evidence plus corrected answer), so reserving 1 + 2*attempts is exact, and the conditional correctly mirrors toolsPermitted's reachability (kind 'validator' requires a validator; tools require max_tool_calls > 0). The regression test adapts to whatever window the formula produces and fails against the old expression; the max_turns bump to 32 preserves the 27-turn investigation window. Both code-compressor nits (stale fixture comment, redundant Boolean coercion) verified and accepted; one independent low-severity note on clamped degenerate configs where a tool-assisted repair still cannot finish.

[github-actions]

The PR fixes turn accounting in the openai-agent runtime: reservedFinalTurns now reserves two provider calls per repair attempt when tool-assisted repair is possible (validator present and max_tool_calls > 0) and one otherwise, matching how repair() actually spends calls; a new toolsPermitted guard withholds the evidence call when the follow-up answer would not fit, and the general reviewer's max_turns rises 30 to 32 to keep its 27-turn investigation window. I traced the repair loop, validateCandidate, the reservation arithmetic, and both new tests: the formula is exact for the reachable worst case, the conditional mirrors the toolsPermitted condition so validator-free callers keep their prior window, and the saturated test is genuinely sensitive to the reservation. I found no correctness defect; only the pre-existing, PR-acknowledged residual that a very small max_turns with multiple repairs can still exhaust the ceiling before the last repair attempt.