From c7a38e274e5bed2d4351ca32dc88c50799bea52f Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sat, 11 Jul 2026 09:37:07 +0300 Subject: [PATCH] =?UTF-8?q?fix(client):=20=D1=80=D0=B5=D0=B0=D0=BB=D1=8C?= =?UTF-8?q?=D0=BD=D1=8B=D0=B9=20=D0=BE=D0=B1=D1=80=D1=8B=D0=B2=20SSE=20?= =?UTF-8?q?=E2=86=92=20reconnect-=D0=BB=D0=B5=D1=81=D0=B5=D0=BD=D0=BA?= =?UTF-8?q?=D0=B0,=20=D0=BD=D0=B5=20=D1=82=D0=B5=D1=80=D0=BC=D0=B8=D0=BD?= =?UTF-8?q?=D0=B0=D0=BB=D1=8C=D0=BD=D1=8B=D0=B9=20error=20(#488)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Браузерный QA поймал баг, который юнит-тесты пропускали из-за вакуумной SDK-формы. Премиса (подтверждена по ai@6.0.207 AbstractChat.makeRequest, catch ~13763): при сетевом дропе SDK ставит isError=true БЕЗУСЛОВНО, а isError=true → isDisconnect=true ставится РЯДОМ (только для fetch/network TypeError). Т.е. реальный обрыв ВСЕГДА даёт { isError:true, isDisconnect:true }; форма { isDisconnect:true, isError:false } SDK'ом НЕ эмитится. Баг: в onFinish проверка `if (isError) return` стояла РАНЬШЕ ветки isDisconnect → реальный дроп уходил в терминальный error-баннер, а FINISH_DISCONNECT (единственный вход в reconnect-лесенку) не диспатчился НИКОГДА. Сценарии 1/2 (commit 2 «обрыв до первого кадра» и commit 3 «два обрыва») в браузере не работали. Фикс: маршрутизация по disconnect ПЕРВЫМ: isDisconnect → FINISH_DISCONNECT (reconnect); НЕ-disconnect error (isError && !isDisconnect, напр. провайдерский 500) → FINISH_ERROR (терминал); затем isAbort; затем clean. Порядок веток supersede- блока тоже disconnect-first (для консистентности; там всё равно всё дропается I1). Инварианты сохранены: epoch-штамп turnEpochRef на всех FINISH_*; honor-in-stopping в редьюсере честит ЛЮБОЙ финиш в фазе stopping → на пути Stop дроп-финиш уходит в idle, а не в ложный reconnect (новый тест). F1 supersede-drop чужого поколения цел. Тесты сделаны НЕ вакуумными: дроп подаётся реальной формой { isError:true, isDisconnect:true }, терминальная ошибка — { isError:true, isDisconnect:false }. MUTATION-VERIFY: багованный isError-first порядок → 6 reconnect-тестов краснеют (нет баннера «reconnecting»); с фиксом зелены. Полный ai-chat 35 файлов / 377 / 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/components/chat-thread.test.tsx | 42 +++++++++++++++---- .../ai-chat/components/chat-thread.tsx | 42 ++++++++++++------- 2 files changed, 61 insertions(+), 23 deletions(-) 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) {