fix(chatgpt-review): unify timeout budget, native insertion, clipboard retry, heartbeat, stall recovery - #656
Merged
BorisTyshkevich merged 1 commit intoAug 9, 2026
Conversation
…ry clipboard, add heartbeat and stall recovery Diagnosed from real session/state logs across issue #630's whole 8-phase run (55 real session records, dozens of real raw CLI output files) plus live experiments against production ChatGPT tabs: 1. review()'s timeoutMs previously bounded only waitForCompletion -- assertReady (~15s) + upload (~30s) + fillAndSend (up to 240s via its own retry) + waitForPermanentConversationUrl (~15s) were ALL uncounted against it, so real worst-case wall time was timeoutMs + ~300s, not timeoutMs. Confirmed live: 6 real invocations across issue #630 were killed with zero output ever flushed ("Waiting for ChatGPT response..." and nothing else). Now one deadline covers the whole review() call; each setup phase still gets its own short ceiling, capped by Math.min against the overall deadline. 2. fillAndSend now inserts via document.execCommand('insertText') in-page first (measured live: 80KB lands in ~50ms regardless of size), falling back to Playwright's own actionability-checked .fill() only if that fails -- confirmed live that .fill() can still genuinely time out at 120s x 2 under real load even after the existing headroom fix (issue #630, 246.9s elapsed, both attempts exhausted). 3. copyPreferredResponseText retries a failed clipboard copy up to 2 more times for plan-author mode specifically, before falling back to innerText (which can never contain the literal '#' parsePlanAuthorResponse's heading check requires) -- confirmed live: "not complete Markdown with a heading" recurred 6 times across real issue #630/#585 invocations, all traceable to this single-attempt-no-retry gap. 4. SessionStore.writeHeartbeat + review()'s onHeartbeat callback persist a periodic progress snapshot (throttled to heartbeatIntervalMs) alongside the session record, so a process killed externally with zero stdout still leaves something recoverable. 5. waitForCompletion attempts one automatic stop+nudge recovery when generation shows zero text growth for noProgressStallMs while still "generating" -- a live tool call stuck mid-turn, previously only recoverable by a human manually clicking Stop and nudging the same conversation (now documented as a purely manual procedure in skills/ship/references/review-loops.md). Verified: 57/57 unit tests (11 new, covering each fix + the unified-deadline composition specifically), doctor mode against the real connected Chrome, and one full real end-to-end review() call against a fresh conversation (native insertion used, no Playwright .fill() fallback needed, completed in 14.7s, heartbeat fired with the expected shape, response captured correctly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz
BorisTyshkevich
deleted the
fix/chatgpt-review-robustness-and-performance
branch
August 9, 2026 09:01
This was referenced Aug 9, 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.
Summary
Investigated why
chatgpt-review's Playwright interactions sometimes failed/stalled during issue #630's long/shiprun, by reading the actual code, mining 55 realSessionStorerecords and dozens of real raw CLI output files from$TMPDIRacross the whole run, and running live experiments against real ChatGPT tabs. Found and fixed 5 distinct, evidence-backed root causes:timeoutMspreviously bounded onlywaitForCompletion; setup phases (assertReady/upload/fillAndSend/waitForPermanentConversationUrl) were each independently uncounted against it, so real worst-case wall time wastimeoutMs + ~300s. Confirmed live: 6 real invocations across Epic: extract the Fetch-native ClickHouse client into a reusable package #630 were killed with zero output ever flushed. Now one deadline covers the whole call.fillAndSendnow inserts viaexecCommand('insertText')in-page first (measured live: 80KB in ~50ms regardless of size), falling back to Playwright's own.fill()only if that fails. Confirmed live that.fill()can still genuinely time out at 120s×2 under real load even after the existing headroom fix (Epic: extract the Fetch-native ClickHouse client into a reusable package #630: 246.9s elapsed, both attempts exhausted).plan-authormode — a single failed clipboard read used to fall back toinnerText, which can never contain the literal#parsePlanAuthorResponse's heading check requires. Confirmed live: this exactinvalid_responserecurred 6 times across real Epic: extract the Fetch-native ClickHouse client into a reusable package #630/ADR-0005: adopt @clickhouse/client-web behind the SQL Browser transport adapter #585 invocations. Now retries up to 2 more times before falling back, for that mode specifically.skills/ship/references/review-loops.md.Test plan
node --test tests/*.test.mjs— 57/57 pass (11 new tests, one per fix plus the unified-deadline composition specifically)node scripts/chatgpt-review.mjs doctoragainst the real connected Chrome — passesreview()call against a fresh conversation: native insertion used (no.fill()fallback needed), completed in 14.7s, heartbeat fired with the expected shape, response captured correctly (disposable test conversation closed afterward, no real conversations touched)try/finally) against a fresh page, which worked immediately.🤖 Generated with Claude Code
https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz