fix: address issues #135, #133, #131, #129#165
Merged
Conversation
The flag was initialized to false but never flipped to true when a task reached a terminal state (completed/failed/cancelled), making it dead code. Set it to true in both the exit handler and the spawn-error path. Closes OpenBMB#135 Co-authored-by: Cursor <cursoragent@cursor.com>
The 10ms polling loop woke up ~100 times/sec just to check a flag. Replace with a deferred Promise resolved by handleMessage on hello_ok or rejected by handleClose/timeout. Remove the now-unused sleep helper. Closes OpenBMB#133 Co-authored-by: Cursor <cursoragent@cursor.com>
Only allocate the stream state object for the provider actually in use, instead of always creating both Anthropic and OpenAI state per stream. The normalizeStreamEvent function falls back to lazy init if the caller passes a partial state object. Closes OpenBMB#131 Co-authored-by: Cursor <cursoragent@cursor.com>
Two bugs fixed: 1. unmountedRef was set to true in the token-change cleanup, permanently blocking subsequent connect() calls. Moved unmount detection to a separate effect so it only fires on real unmount. 2. Old WebSocket onclose fired asynchronously after cleanup and spawned phantom reconnect timers with a stale token. Added a generation counter (connectIdRef) — all callbacks bail out when their generation no longer matches. Cleanup also nulls out event handlers on the old socket before closing it. Closes OpenBMB#129 Co-authored-by: Cursor <cursoragent@cursor.com>
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
2 participants
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
Batch fix for four open issues reported by @Livezt:
completionStatusSentInAttachmentflag was initialized tofalsebut never flipped totruewhen a task reached a terminal state. Set it in both theexithandler and thespawn-error path.waitForHelloused a 10ms polling loop (~100 wakeups/sec). Replaced with a deferred Promise resolved byhandleMessageonhello_okor rejected on close/timeout.createStreamNormalizerState()always allocated both Anthropic and OpenAI stream state objects. Now accepts aprotocolparameter and only creates the one in use;normalizeStreamEventlazily initializes on fallback.unmountedRefwas incorrectly set totrueon token-change cleanup (not just unmount), permanently blocking reconnection. Old WSonclosealso spawned phantom reconnect timers with stale tokens. Fixed with a generation counter (connectIdRef) and proper handler cleanup.Test plan
completionStatusSentInAttachment = trueafter completion/failure/cancellationGatewayBrowserClient.connect()resolves onhello_okwithout CPU-intensive pollingCloses #135, closes #133, closes #131, closes #129
Made with Cursor