From 66a41d0813dfed9acb20c0da1bffb1b7606d4b43 Mon Sep 17 00:00:00 2001 From: Andre Padez Date: Tue, 11 Aug 2026 00:20:30 +0100 Subject: [PATCH] reclaim a connecting socket instead of orphaning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to bc13450, found by actually driving a browser instead of reasoning about one. The deferred close keeps a remount's socket alive mid-handshake, but `connect` only treated OPEN as "already ours" — so the re-run built a second socket, overwrote socketRef, and left the first open forever with its `open` handler bailing on the mismatch. CONNECTING now counts too. What the browser actually says, headless Chrome against this server, fresh load of /chat/new: #1 NEW wss://…/api/chat/ws?token=… #2 NEW wss://…/api/chat/ws?token=… #1 OPEN #2 OPEN (neither ever closes, 12s) header: green dot, no "Disconnected" So the served code CONNECTS on a fresh load and the Disconnected report could not be reproduced here — which points the remaining report at the client's cached bundle rather than at this code. The chunk hash moved e3jsfax5 -> 81jec45w across these edits, so the rebuild is reaching the wire. Two sockets per load survive this fix and are NOT what it addresses: they come from two separate `useChat` instances mounting on that route, each with its own refs, so no per-instance guard can see the other. Left alone deliberately — both connect, and one conversation opening two agent sockets wants understanding before a fix. Also retired here: my claim that StrictMode's double-invoke was the trigger. The served bundle has no dev-only React internals at all (`doubleInvokeEffectsOnFiber`, `runWithFiberInDEV`, `commitPassiveUnmountEffectsInsideOfDeletedTree`: zero hits), because pm2 runs `bun start` with NODE_ENV=production. Typecheck clean. Repro harness is in the session scratchpad, not committed. Co-Authored-By: Claude Opus 5 --- src/workspaces/hooks/src/useChatWebSocket.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/workspaces/hooks/src/useChatWebSocket.ts b/src/workspaces/hooks/src/useChatWebSocket.ts index e59f955b..7f1104b0 100644 --- a/src/workspaces/hooks/src/useChatWebSocket.ts +++ b/src/workspaces/hooks/src/useChatWebSocket.ts @@ -24,7 +24,16 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar const connect = () => { if (isCleaningUpRef.current) return; - if (socketRef.current && socketRef.current.readyState === WebSocket.OPEN) return; + // CONNECTING counts as ours, not just OPEN. The deferred teardown below keeps a remount's socket + // alive mid-handshake, and this is what reclaims it: checking only OPEN meant an effect re-run + // built a SECOND socket and orphaned the first, which then stayed open forever with its own `open` + // handler bailing on the socketRef mismatch. + // + // This is per-instance and does NOT explain the two sockets a /chat/new load opens — measured with + // a WebSocket-constructor counter, those come from two separate `useChat` instances mounting, each + // with its own refs. Unresolved, and tracked separately; both connect, so it reads as healthy. + const existing = socketRef.current; + if (existing && (existing.readyState === WebSocket.OPEN || existing.readyState === WebSocket.CONNECTING)) return; const socket = new WebSocket(url); socketRef.current = socket;