diff --git a/apps/client/src/features/ai-chat/components/chat-thread.test.tsx b/apps/client/src/features/ai-chat/components/chat-thread.test.tsx index 2c5e6ce2..ee409e6a 100644 --- a/apps/client/src/features/ai-chat/components/chat-thread.test.tsx +++ b/apps/client/src/features/ai-chat/components/chat-thread.test.tsx @@ -248,14 +248,15 @@ describe("ChatThread — send now", () => { startLocalStreamWithRun(); fireEvent.click(screen.getByTestId("queue-btn")); fireEvent.click(screen.getByLabelText("Send now")); // -> superseding, aborts A - // A ends via isDisconnect (the server CAS closed it). The OLD overlap bug would - // route the LIVE new run into a false reconnect banner. + // A ends via a REAL network drop { isError:true, isDisconnect:true } (the server + // CAS closed it). The OLD overlap bug would route the LIVE new run into a false + // reconnect banner. await act(async () => { h.state.onFinish?.({ message: { id: "a1", role: "assistant", parts: [{ type: "text", text: "x" }] }, isAbort: false, isDisconnect: true, - isError: false, + isError: true, }); await Promise.resolve(); }); @@ -328,6 +329,23 @@ describe("ChatThread — send now", () => { expect(screen.getAllByLabelText("Remove queued message")).toHaveLength(1); }); + it("Stop then a REAL network-drop finish exits to idle (honor-in-stopping), NOT a false reconnect", () => { + // Regression for the disconnect-first reorder: on the STOP path, even a drop- + // form finish { isError:true, isDisconnect:true } arriving in `stopping` must be + // HONORED (reducer) and exit to idle — it must NOT enter the reconnect ladder. + startLocalStreamWithRun(); // live local stream, autonomous + fireEvent.click(screen.getByLabelText("Stop")); // STOP_REQUESTED -> stopping + act(() => { + h.state.onFinish?.({ + message: { id: "a1", role: "assistant", parts: [] }, + isAbort: false, + isDisconnect: true, + isError: true, + }); + }); + expect(screen.queryByText(/reconnecting/i)).toBeNull(); + }); + it("Send now is HIDDEN while observing a resumed run and VISIBLE on a local stream", () => { // Resumed (mount attach) -> observer -> hidden. renderThread({ autonomousRunsEnabled: true, initialRows: streamingTail() }); @@ -419,15 +437,18 @@ describe("ChatThread — turn-end decision (onFinish)", () => { expect(screen.getByText("Response stopped.")).toBeTruthy(); }); - it("ENDS — keeps the queue on a disconnect (non-autonomous) and shows the notice", () => { - finishWith({ isDisconnect: true }); + it("ENDS — keeps the queue on a REAL disconnect (non-autonomous) and shows the notice", () => { + // Real SDK drop form: { isError:true, isDisconnect:true }. With the buggy + // isError-first order this would fall to the terminal error branch (no notice). + finishWith({ isDisconnect: true, isError: true }); expect(h.state.sendMessage).not.toHaveBeenCalled(); expect( screen.getByText("Connection lost — the answer was interrupted."), ).toBeTruthy(); }); - it("ENDS — keeps the queue on a stream error (no notice)", () => { + it("ENDS — keeps the queue on a NON-disconnect stream error (no notice)", () => { + // A provider error is { isError:true, isDisconnect:false } — terminal. finishWith({ isError: true }); expect(h.state.sendMessage).not.toHaveBeenCalled(); expect(screen.queryByText("Response stopped.")).toBeNull(); @@ -437,7 +458,7 @@ describe("ChatThread — turn-end decision (onFinish)", () => { for (const flags of [ {}, { isAbort: true }, - { isDisconnect: true }, + { isDisconnect: true, isError: true }, { isError: true }, ]) { cleanup(); @@ -787,13 +808,18 @@ describe("ChatThread — live reconnect + stalled", () => { cleanup(); }); + // A REAL live SSE drop. ai@6.0.207 emits BOTH { isError:true, isDisconnect:true } + // for a network TypeError — NOT the { isError:false } form the old tests fed. This + // is the form browser QA hit; with the buggy isError-first routing these tests go + // red (a real drop would surface the terminal error banner, not the reconnect + // ladder). MUTATION-VERIFY of the disconnect-first fix. function disconnect(message: unknown = liveMsg) { act(() => { h.state.onFinish?.({ message, isAbort: false, isDisconnect: true, - isError: false, + isError: true, }); }); } diff --git a/apps/client/src/features/ai-chat/components/chat-thread.tsx b/apps/client/src/features/ai-chat/components/chat-thread.tsx index bd0b513c..3ec5930b 100644 --- a/apps/client/src/features/ai-chat/components/chat-thread.tsx +++ b/apps/client/src/features/ai-chat/components/chat-thread.tsx @@ -626,10 +626,13 @@ export default function ChatThread({ machineRef.current.phase.name === "superseding" && pendingSupersedeTextRef.current !== null ) { - if (isError) dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch }); - else if (isAbort) dispatch({ type: "FINISH_ABORT", epoch: stampEpoch }); - else if (isDisconnect) + // Disconnect-first (see the routing note below): a real drop is + // { isError:true, isDisconnect:true }. All of these are dropped by I1 here + // (superseding bumped the epoch) — the order only mirrors the main routing. + if (isDisconnect) dispatch({ type: "FINISH_DISCONNECT", hasVisibleContent: msgHasVisible, epoch: stampEpoch }); + else if (isError) dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch }); + else if (isAbort) dispatch({ type: "FINISH_ABORT", epoch: stampEpoch }); else dispatch({ type: "FINISH_CLEAN", epoch: stampEpoch }); const text = pendingSupersedeTextRef.current; pendingSupersedeTextRef.current = null; @@ -642,18 +645,14 @@ export default function ChatThread({ return; } - if (isError) { - dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch }); - setStopNotice(null); - return; - } - if (isAbort) { - // A user Stop / an interrupt abort finished. The FSM stopping/idle exit is - // by DATA (this terminal outcome, I4). - dispatch({ type: "FINISH_ABORT", epoch: stampEpoch }); - setStopNotice("manual"); - return; - } + // #488 (browser QA): route DISCONNECT FIRST. In ai@6.0.207 a real network drop + // yields BOTH `{ isError:true, isDisconnect:true }` — the SDK sets `isError` + // unconditionally in its catch and `isDisconnect` only ALONGSIDE it (for a + // fetch/network TypeError). Checking `isError` first therefore sent EVERY real + // drop to the terminal error banner and NEVER entered the reconnect ladder + // (`FINISH_DISCONNECT` is its only entry). A disconnect — the detached run + // keeps executing server-side — must win; only a NON-disconnect error (a + // provider 500, `{ isError:true, isDisconnect:false }`) is terminal. if (isDisconnect) { if (wasObserver) { // A resumed/attached OBSERVER stream dropped. Recover via the degraded @@ -722,6 +721,19 @@ export default function ChatThread({ } return; } + // A NON-disconnect stream error (a provider 500 etc.) -> terminal error banner. + if (isError) { + dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch }); + setStopNotice(null); + return; + } + if (isAbort) { + // A user Stop / an interrupt abort finished. The FSM stopping/idle exit is + // by DATA (this terminal outcome, I4 — honored in `stopping` by the reducer). + dispatch({ type: "FINISH_ABORT", epoch: stampEpoch }); + setStopNotice("manual"); + return; + } // Clean finish. if (wasObserver) { if (mountedRef.current) {