fix(chatgpt-review): plan-author early-exit once a complete protocol appears - #650
Merged
Merged
Conversation
…appears waitForCompletion() required the WHOLE turn to stop generating (no visible stop button) and hold stable for 7s before returning, for every mode alike. That's the right contract for pr/plan/issue's trailing VERDICT line, but plan-author's protocol is a fixed-content island: parsePlanAuthorResponse already requires exactly one delimiter pair, so once a valid PLAN_STATUS: READY/BLOCKED block appears, its content can never change no matter what ChatGPT keeps writing afterward (more reasoning, another tool call, a citation footnote — already tolerated by the same parser). Observed live on #630 phase 7: an Extra-High-effort plan-author turn kept the stop button visible for 20+ minutes of further tool calls after already emitting a complete, valid plan. waitForCompletion's !generating+stableMs requirement can never fire in that case, so the whole call times out despite a perfectly good answer already sitting in the DOM — the coordinator had to recover it by hand from the live page. For plan-author mode only: once the same complete-protocol match holds for two consecutive polls (guarding against a transient mid-stream coincidence), confirm it against the authoritative clipboard-copied Markdown (innerText can strip the literal '#' heading syntax the parser's heading check needs) and return immediately if it agrees, without waiting for generation to stop. Falls back to the ordinary wait if the clipboard copy disagrees. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
waitForCompletion()required the entire ChatGPT turn to stop generating (novisible "stop" button) AND hold stable for 7s before returning — for every CLI
mode alike. That's the right contract for
pr/plan/issuemode, whose answerends with a
VERDICT:line that could in principle still change if more textfollows. But
plan-authormode's protocol is a fixed-content island:parsePlanAuthorResponsealready requires exactly one delimiter pair, so once avalid
PLAN_STATUS: READY/BLOCKEDblock appears, its content can never changeno matter what ChatGPT keeps writing afterward — this is explicitly tolerated
already (a documented, tested behavior: a trailing citation footnote after
PLAN_ENDdoesn't invalidate the response).What happened live
On
/ship 630phase 7, an Extra-High-effortplan-authorturn kept the "stop"button visible for 20+ minutes of further reasoning/tool calls after already
emitting a complete, valid plan.
waitForCompletion's!generating + stableMsrequirement can never fire in that case, so the call timed outdespite a perfectly good answer already sitting in the DOM — the coordinator
had to recover it by hand by reading the live page directly.
Fix
For
plan-authormode only: once the same complete-protocol match (checked viathe real
parsePlanAuthorResponseparser, non-throwing) holds for twoconsecutive polls — guarding against a transient mid-stream coincidence —
confirm it against the authoritative clipboard-copied Markdown (
.innerText()can strip the literal
#heading syntax the parser's heading check requires)and return immediately if it agrees, without waiting for generation to stop.
Falls back to the ordinary wait if the clipboard copy disagrees (never trusts
an unconfirmed match). Other modes are unaffected — the early-exit is gated on
mode === 'plan-author'.Tests
Three new tests in
tests/browser.test.mjs:waiting for generation to stop.
(proves the change is properly scoped).
before trusting it while still generating (proves no unconfirmed early
return, and no infinite-loop risk).
48/48
skills/chatgpt-reviewtests pass (45 pre-existing + 3 new). Fullrepository gate green — this change touches only
skills/**, outside thecoverage-gated
src/**tree.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz