Repository navigation
Fix open/watch crash when a review runs longer than 5 minutes - #144
Open
jamescbury wants to merge 1 commit into
Open
jamescbury wants to merge 1 commit into
jamescbury wants to merge 1 commit into
Conversation
runWatch issued a single unbounded long-poll to /api/review-events/watch, so undici's default 300s headersTimeout killed the request (TypeError: fetch failed / UND_ERR_HEADERS_TIMEOUT) whenever the reviewer took more than ~5 minutes to click Done Reviewing. The server also clamps its own wait to 300s, so an unbounded request was never honored anyway. Poll in bounded 240s chunks instead, resuming with afterSequence / nextSequence so no events are missed between polls (the server retains the last 100 events), and retry transient fetch failures (up to 5 consecutive, 2s apart) instead of crashing. --timeout semantics are unchanged, and overall timeouts longer than 300s are now honored too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@jamescbury is attempting to deploy a commit to the Lex Team on Vercel. A member of the Team first needs to authorize it. |
felciano
added a commit
to felciano/roughdraft
that referenced
this pull request
Oct 31, 2026
… runs longer than 5 minutes From Lex-Inc#144, unmerged upstream as of 2026-10-31. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXZP8LMTVHbkEsmdNZzjyf
This branch has not been deployed
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.
roughdraft openwaits for Done Reviewing with a single unbounded long-poll to/api/review-events/watch. Node's built-in fetch (undici) applies a default 300-secondheadersTimeout, so when a reviewer takes more than ~5 minutes the CLI crashes withTypeError: fetch failed/UND_ERR_HEADERS_TIMEOUT. The server also clamps its own wait to 300s inreview-events.ts, so an unbounded request was never going to be honored anyway.What this changes
runWatchnow polls in bounded 240-second chunks — safely under both undici's 300s headers timeout and the server's own 300s clamp — with a per-request abort guard.nextSequence(sent asafterSequence), so a Done Reviewing event that lands between polls is never missed; the server already retains the last 100 events for exactly this.--timeoutsemantics are unchanged, and overall timeouts longer than 300s now actually work (previously the server silently capped them).Testing
UND_ERR_HEADERS_TIMEOUT-style fetch failure mid-watch now retries and completes instead of crashing), and cover the bounded-poll contract,afterSequenceresumption across a server-side poll timeout, and--timeoutexpiry.pnpm checkandpnpm test:smokepass.roughdraft opensat on "Waiting for Done Reviewing..." for 6 minutes 40 seconds — past the old 300s crash point — then received the Done Reviewing event and exited 0.Fixes #127, fixes #108, fixes #93. I believe this also resolves #113 (the same 5-minute exit, attributed there to an SSE timeout), and it may explain some of the reports in #119.
🤖 Generated with Claude Code