fix(realtime): preserve reconnect backoff across flaps#102348
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 10:10 PM ET / 02:10 UTC. Summary PR surface: Source +27, Tests +44. Total +71 across 2 files. Reproducibility: yes. at source level: current main resets the reconnect counter on raw websocket open, while close handling calls reconnect logic that uses the same counter for the retry cap and delay. I did not execute the regression in this read-only review, but the linked issue and PR both include real local websocket proof. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the shared-helper fix after exact-head freshness checks, keeping provider-specific APIs and config unchanged. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main resets the reconnect counter on raw websocket open, while close handling calls reconnect logic that uses the same counter for the retry cap and delay. I did not execute the regression in this read-only review, but the linked issue and PR both include real local websocket proof. Is this the best way to solve the issue? Yes. The shared websocket session helper is the best layer because all affected bundled realtime transcription providers route through it; provider-specific fixes or a new config knob would duplicate the invariant. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0307deacfac7. Label changesLabel justifications:
Evidence reviewedPR surface: Source +27, Tests +44. Total +71 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
proof: sufficient
Reset reconnect attempts only after a provider-ready connection stays stable, and cover both flap exhaustion and stable recovery. Co-authored-by: Ho Lim <[email protected]>
c37203f to
6f87b40
Compare
|
Maintainer rewrite is ready to land at What changed:
Proof:
Known gap: no external provider endpoint was deliberately flapped; the shared production session path was exercised against a real loopback WebSocket server. |
|
Merged via squash.
|
Reset reconnect attempts only after a provider-ready connection stays stable, and cover both flap exhaustion and stable recovery. Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
Realtime transcription WebSocket sessions reset
reconnectAttemptson every raw socket open. A provider that opened, reached ready, and then repeatedly dropped therefore stayed at the base reconnect delay forever and never reachedmaxReconnectAttempts.Closes #102251.
Why This Change Was Made
The shared session helper now records the provider-ready timestamp and preserves the retry budget across short ready-then-close flaps. A connection that remains provider-ready for 30 seconds earns a fresh budget when it later disconnects.
The maintainer rewrite is based on current
mainand keeps the inbound payload cap from #102443. It uses a timestamp checked at disconnect instead of a separate stable-reset timer, avoiding another timer/cleanup state machine and avoiding a new public tuning option.User Impact
A flapping realtime transcription upstream backs off and stops at the configured retry limit instead of creating an unbounded handshake/audio-reflush loop. Healthy long-lived sessions can still recover from later transient disconnects because a genuinely stable connection refreshes the retry budget.
Evidence
tbx_01kx37y4wrj3aja5vab6tdce8y:corepack pnpm test src/realtime-transcription/websocket-session.test.ts— 14/14 passed.pnpm check:changedchild gate — exit 0 across core prod/test types, lint, and relevant architecture guards.oxfmtandgit diff --check: clean.No live provider endpoint was deliberately flapped; the shared runtime path was exercised against a real local WebSocket server.
Co-authored-by: Ho Lim [email protected]