From 9d2da49572aba60fc178bc827637ed6843e1ebc6 Mon Sep 17 00:00:00 2001 From: Andre Padez Date: Tue, 11 Aug 2026 02:00:31 +0100 Subject: [PATCH] re-land the client half: hold the socket across a remount, and queue what was typed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three commits backed out a few hours ago as collateral, restored together because they are one fix: 243bd04 (queue sends until OPEN), bc13450 (defer the teardown close by a tick, cancellable) and 66a41d0 (count CONNECTING as ours, not only OPEN). Why they are needed, from evidence rather than reasoning. d9857ee fixed the server side and Andre still had nothing on https://macbook.pastilhas.dev after a restart. The decisive observation is an ABSENCE: his attempts appear nowhere in officer's log — no "Model selected for chat", no claude:stream for his session. Nothing reaches the server at all. Meanwhile a socket I drove by hand against the same wss:// url ran a full turn in 2.5s, so the transport is not it. That is `send` dropping the message. It returned silently on `readyState !== OPEN`, and the socket is not OPEN because the effect cleanup closed it on a remount while it was still CONNECTING, then closed its replacement the same way. Enter does nothing, forever, with the view sitting on Disconnected — and no error anywhere, on either side, which is why this reads as a dead server. Note what is still NOT fixed, and is written into the code comment rather than this message alone: a /chat/new load opens TWO sockets, from two separate useChat instances mounting. Measured with a constructor counter. Both connect, so it looks healthy; d9857ee is what makes it harmless. Correction to d9857ee's message, so the record is not wrong: it claims the localhost/domain split is latency changing the attach/close ordering. That is plausible and it is NOT what was demonstrated — the server-side fix alone did not help. It is still worth having (a stale close silencing a live client is real, and so is the idle GC gate), but the asymmetry is unexplained and the client drop above is what actually stopped a message. Client bundle changes, so this needs a hard reload as well as a restart. Not verified in a browser. Co-Authored-By: Claude Opus 5 --- src/workspaces/hooks/src/useChatWebSocket.ts | 75 ++++++++++++++++++-- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/src/workspaces/hooks/src/useChatWebSocket.ts b/src/workspaces/hooks/src/useChatWebSocket.ts index 776da795..5ec7bb2e 100644 --- a/src/workspaces/hooks/src/useChatWebSocket.ts +++ b/src/workspaces/hooks/src/useChatWebSocket.ts @@ -15,14 +15,39 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar const retryRef = useRef(0); const retryTimeoutRef = useRef(null); const isCleaningUpRef = useRef(false); + /** A deferred teardown, cancelled when the effect re-runs — see the cleanup below. */ + const closeTimerRef = useRef(null); const onMessageRef = useRef(onMessage); onMessageRef.current = onMessage; const onOpenRef = useRef(onOpen); onOpenRef.current = onOpen; + /** + * Messages typed before the socket was ready. + * + * `send` used to drop them: `readyState !== OPEN` returned, silently, with no error and no retry — so + * pressing enter did nothing and the turn never happened. That window is not rare. React's dev + * StrictMode double-invokes effects, so every socket is created, closed and recreated on mount, and a + * reconnect after a drop reopens it again; with several chat panes on screen there are several sockets + * doing this at once. One of them is always briefly not OPEN. + * + * Queued and flushed on open, in order. The mobile chat app does exactly this and for exactly this + * reason — the composer is allowed to fire before the transport is ready. + */ + const pendingRef = useRef([]); + 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; @@ -31,7 +56,12 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar if (socketRef.current !== socket) return; setIsConnected(true); retryRef.current = 0; + // BEFORE onOpen, deliberately: onOpen sends the resume/attach handshake, and anything the user + // typed while connecting belongs after that, not in front of it. + const queued = pendingRef.current; + pendingRef.current = []; onOpenRef.current?.(); + for (const message of queued) socket.send(message); }); socket.addEventListener('message', (ev) => { @@ -59,25 +89,56 @@ export const useChatWebSocket = ({ url, onMessage, onOpen }: UseChatWebSocketPar }; useEffect(() => { + // A pending teardown from a remount that is about to be undone — see below. + if (closeTimerRef.current !== null) { + clearTimeout(closeTimerRef.current); + closeTimerRef.current = null; + } isCleaningUpRef.current = false; connect(); + return () => { + /** + * Close LATER, not now. + * + * Closing here directly is correct for a real unmount and disastrous for a remount, and this hook + * cannot tell them apart at the moment it runs. React's dev StrictMode double-invokes every effect + * (mount → unmount → mount), and a subtree that is re-created — a resolved transcript, a parent key + * change — does the same. Each time, the socket was closed while still CONNECTING, the browser + * logged "closed before the connection is established", and the replacement was closed in turn, so + * the view could churn forever and never hold a connection while sitting on Disconnected. + * + * Deferring by a tick makes the two distinguishable. A remount re-runs the effect immediately and + * cancels this timer, so the live socket is kept and the handshake completes. A real unmount has + * nobody to cancel it and the socket closes a frame later, which costs nothing. + */ isCleaningUpRef.current = true; if (retryTimeoutRef.current !== null) { clearTimeout(retryTimeoutRef.current); retryTimeoutRef.current = null; } - if (socketRef.current) { - socketRef.current.close(); - socketRef.current = null; - } + const socket = socketRef.current; + closeTimerRef.current = window.setTimeout(() => { + closeTimerRef.current = null; + if (!isCleaningUpRef.current) return; // remounted: the effect above already reclaimed it + if (socket) socket.close(); + if (socketRef.current === socket) socketRef.current = null; + }, 0); }; }, [url]); const send = (data: Record) => { const socket = socketRef.current; - if (!socket || socket.readyState !== WebSocket.OPEN) return; - socket.send(JSON.stringify(data)); + const message = JSON.stringify(data); + if (socket && socket.readyState === WebSocket.OPEN) { + socket.send(message); + return; + } + // Not open yet, or reconnecting. Hold it rather than dropping it — see `pendingRef`. Bounded so a + // socket that never comes back cannot grow this without limit; the oldest go first, because the + // newest message is the one the user is still waiting on. + pendingRef.current.push(message); + if (pendingRef.current.length > 50) pendingRef.current.shift(); }; return { isConnected, send };