From 69e04349a04d02a3625f649c851953b1fa87fec8 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 04:29:25 +0300 Subject: [PATCH] =?UTF-8?q?fix(client):=20=D0=B2=D1=85=D0=BE=D0=B4=20?= =?UTF-8?q?=D0=B2=20reconnect=20=D0=BE=D0=B3=D1=80=D0=B0=D0=BD=D0=B8=D1=87?= =?UTF-8?q?=D0=B8=D0=B2=D0=B0=D0=B5=D1=82=20=D0=BE=D0=B6=D0=B8=D0=B4=D0=B0?= =?UTF-8?q?=D0=BD=D0=B8=D0=B5=20getRun=20=D1=82=D0=B0=D0=B9=D0=BC=D0=B0?= =?UTF-8?q?=D1=83=D1=82=D0=BE=D0=BC=20(#541)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit При живом обрыве SSE вход в лестницу reconnect происходил только после резолва getRun(cid) (ре-сид из персиста). Путь REJECT обрабатывался через .catch, но ЗАВИСШИЙ getRun (соединение есть, ответа нет) был не ограничен: axios-клиент (lib/api-client.ts) не имеет timeout, а stalled-idle-cap взводится только ПОСЛЕ входа в reconnecting/polling. Итог — FSM залипала в `streaming` без баннера и без поллинга до сокет-таймаута браузера (минуты). Это тот самый класс тихого зависания, который устраняет эпик #497. Оборачиваю ожидание getRun гонкой с таймаутом (RECONNECT_RESEED_TIMEOUT_MS = 4s): по таймауту берётся ТОТ ЖЕ фолбэк, что и на reject — dropLivePartialAndReplayFromStart() + enterReconnect(runId), так что лестница/поллинг стартуют и stalled-idle-cap взводится. Локальный флаг `settled` делает ветви resolve/reject/timeout взаимоисключающими: поздний резолв getRun после сработавшего таймаута полностью игнорируется (не входит в reconnect повторно, не перетирает replay-from-start устаревшим ре-сидом, ничего не перевзводит). Таймер живёт в reseedTimerRef и очищается при размонтировании (никаких висящих setTimeout). Тесты: hang-кейс (getRun не резолвится -> по таймауту FSM в reconnecting, live-partial сброшен, replay-from-start) и late-resolve safety (поздний резолв — no-op). Мутация: замена гонки на голый getRun краснит оба #541-теста. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/components/chat-thread.test.tsx | 84 ++++++++++++++ .../ai-chat/components/chat-thread.tsx | 107 +++++++++++++----- 2 files changed, 163 insertions(+), 28 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 47e047cd..e0360dc3 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 @@ -1254,4 +1254,88 @@ describe("ChatThread — live reconnect + stalled", () => { }); expect(onResumeFallback).toHaveBeenCalledWith(false); // POLL_IDLE_CAP -> idle -> disarm }); + + it("#541: getRun HANGS on a live disconnect — the timeout race still enters reconnect (no silent freeze in `streaming`)", async () => { + // MUTATION-VERIFY: revert the race to a bare `getRun(cid).then/.catch` and this + // reddens — a HUNG (never-settling, NOT rejected) getRun leaves the FSM stuck in + // `streaming` with no reconnect banner and no poll (the axios client sets no + // request timeout, and the stalled-idle cap only arms AFTER reconnecting/polling). + renderLive(); + h.state.getRun.mockReset(); + h.state.getRun.mockReturnValue(new Promise(() => {})); // getRun HANGS forever + await disconnect(); // live partial = liveMsg (id "a2") + expect(h.state.getRun).toHaveBeenCalledWith("c1"); + // BEFORE the bound fires the FSM is still in the live turn — the very freeze bug. + expect(screen.queryByText(/reconnecting/i)).toBeNull(); + // The recovery-start bound fires -> the SAME fallback as the reject path. + act(() => { + vi.advanceTimersByTime(4_000); + }); + expect(screen.getByText(/reconnecting/i)).toBeTruthy(); + // The live partial (a2) was DROPPED from the store (replay-from-start, no stale + // tail-apply base). Mirrors the getRun-REJECT test's inspection. + const removedLivePartial = ( + h.state.setMessages as unknown as { + mock: { calls: [unknown][] }; + } + ).mock.calls.some(([updater]) => { + if (typeof updater !== "function") return false; + const out = (updater as (p: { id: string }[]) => { id: string }[])([ + { id: "a2" }, + { id: "u1" }, + ]); + return !out.some((m) => m.id === "a2"); + }); + expect(removedLivePartial).toBe(true); + // ...and the anchor was NULLED -> replay-from-start (no ?anchor=&n= over the partial). + advanceToAttempt(1); + expect(h.state.resumeStream).toHaveBeenCalledTimes(1); + expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe( + "/api/ai-chat/runs/c1/stream", + ); + }); + + it("#541: a getRun resolve AFTER the timeout already fired is IGNORED (no double reconnect, no stale re-seed)", async () => { + // The timeout wins first and enters the ladder via replay-from-start. When the + // hung getRun FINALLY answers, its late `.then` must be a full no-op: it must not + // re-seed the store from the (now stale) persisted row, must not re-set the + // anchor, and must not re-enter reconnect. The local `settled` flag makes the + // resolve/reject/timeout branches mutually exclusive. + // MUTATION-VERIFY: drop the `settled` guard (let the late `.then` run) and the + // late re-seed re-sets the anchor (URL regains ?anchor=a2&n=3) + calls setMessages. + renderLive(); + let resolveGetRun!: (v: unknown) => void; + h.state.getRun.mockReset(); + h.state.getRun.mockReturnValue( + new Promise((r) => { + resolveGetRun = r; + }), + ); + await disconnect(); + act(() => { + vi.advanceTimersByTime(4_000); // bound fires -> replay-from-start, reconnecting + }); + expect(screen.getByText(/reconnecting/i)).toBeTruthy(); + advanceToAttempt(1); + expect(h.state.resumeStream).toHaveBeenCalledTimes(1); + // Anchor is null -> replay-from-start URL (the pre-condition the late resolve must + // not undo). + expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe( + "/api/ai-chat/runs/c1/stream", + ); + const setMessagesCallsBefore = h.state.setMessages.mock.calls.length; + // NOW the hung getRun finally resolves with a persisted anchor (id a2, steps 3). + await act(async () => { + resolveGetRun(persistedAnchor()); + await Promise.resolve(); + }); + // The late resolve did NOT re-seed the store... + expect(h.state.setMessages.mock.calls.length).toBe(setMessagesCallsBefore); + // ...did NOT re-set the anchor (URL stays replay-from-start, no ?anchor=&n=)... + expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe( + "/api/ai-chat/runs/c1/stream", + ); + // ...and did NOT trigger a fresh reconnect attach. + expect(h.state.resumeStream).toHaveBeenCalledTimes(1); + }); }); 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 6eb7fdaa..546833e2 100644 --- a/apps/client/src/features/ai-chat/components/chat-thread.tsx +++ b/apps/client/src/features/ai-chat/components/chat-thread.tsx @@ -81,6 +81,17 @@ const STREAM_THROTTLE_MS = 50; // THREAD now (the FSM owns polling->stalled); the window just polls while armed. const DEGRADED_POLL_IDLE_MAX_MS = 10 * 60_000; +// #541: the bound on the persist re-seed (getRun) wait when ENTERING the reconnect +// ladder after a live SSE drop. The axios client (lib/api-client.ts) sets NO request +// timeout, and the stalled-idle cap only arms AFTER the FSM enters reconnecting/ +// polling — which never happens if getRun HANGS (connection established, no response). +// Without this bound a live drop + a hung getRun sticks the FSM in `streaming` with +// no banner and no poll until the browser socket timeout (minutes) — the silent-freeze +// class #497 eliminates. This is a deliberate RECOVERY-START bound (drop the live +// partial + enter the ladder so the poll/idle-cap arms), intentionally much shorter +// than any network socket timeout — not a network read timeout. +const RECONNECT_RESEED_TIMEOUT_MS = 4_000; + /** The #487 active (non-terminal) run statuses — mirrors the server's * ACTIVE_RUN_STATUSES. A run-fact is "active" only for these. */ function isActiveRunStatus(status: string | null | undefined): boolean { @@ -286,6 +297,9 @@ export default function ChatThread({ // stalled inactivity cap. Cleared by the cancelReconnect effect / the cap effect. const reconnectTimerRef = useRef | null>(null); const idleCapTimerRef = useRef | null>(null); + // #541: the persist re-seed (getRun) timeout race on the disconnect->reconnect + // entry. Held in a ref so unmount (DISPOSE cleanup) clears it — no dangling timer. + const reseedTimerRef = useRef | null>(null); // #491 tail-only: seed EVERY persisted row unchanged (no strip). The streaming // tail holds steps 0..N-1; the run-stream registry's tail (steps >= N) is APPENDED @@ -748,37 +762,68 @@ export default function ChatThread({ anchorRef.current = null; }; if (cid) { - void getRun(cid) - .then((res) => { - if (!mountedRef.current) return; - const persisted = res.message; - if (persisted && persisted.role === "assistant") { - anchorRef.current = { - id: persisted.id, - stepsPersisted: stepsPersistedOf(persisted), - }; - // Replace the live partial with the persisted row IN PLACE by id — - // the re-seed from persist. The attach's tail (steps >= N) then - // appends to a store holding EXACTLY steps 0..N-1: no duplication. - setMessages((prev) => mergeById(prev, rowToUiMessage(persisted))); - } else { - // No persisted assistant row (pre-first-frame break): drop the live - // partial + replay from start (no anchor/n) so nothing is duplicated. - dropLivePartialAndReplayFromStart(); - } - enterReconnect(res.run?.id ?? runId); - }) - .catch(() => { - if (!mountedRef.current) return; - // Persist read FAILED: we cannot re-seed from fresh persist, and a - // stale mount-time anchor over the live partial would tail-apply - // already-present steps -> duplication (a flaky-network blip: - // SSE + getRun both fail, network recovers in ~1s, the registry still - // covers from the mount frontier). Restore the removed-filter guarantee - // instead: drop the live partial + replay from start / 204 -> poll. + // #541: bound the persist re-seed wait with a timeout race. getRun goes + // through the axios client, which has NO request timeout; a HUNG getRun + // (connection open, no response) — distinct from a REJECT, which the + // `.catch` already handles — would otherwise never let us enter the ladder, + // leaving the FSM stuck in `streaming` with no banner and no poll until the + // browser socket timeout. `settled` makes the three branches (resolve / + // reject / timeout) mutually exclusive: whichever fires FIRST wins and the + // others become no-ops. So a LATE getRun resolve AFTER the timeout is fully + // ignored — it cannot (i) re-enter reconnect a second time, (ii) overwrite + // the timeout's replay-from-start with a stale re-seed, or (iii) re-arm any + // timer. On timeout we take the SAME fallback as the reject path (drop the + // live partial + enter the ladder, so the poll and the stalled-idle cap arm). + let settled = false; + const finishReseed = (apply: () => void): void => { + if (settled) return; + settled = true; + if (reseedTimerRef.current) { + clearTimeout(reseedTimerRef.current); + reseedTimerRef.current = null; + } + if (!mountedRef.current) return; + apply(); + }; + reseedTimerRef.current = setTimeout(() => { + finishReseed(() => { dropLivePartialAndReplayFromStart(); enterReconnect(runId); }); + }, RECONNECT_RESEED_TIMEOUT_MS); + void getRun(cid) + .then((res) => { + finishReseed(() => { + const persisted = res.message; + if (persisted && persisted.role === "assistant") { + anchorRef.current = { + id: persisted.id, + stepsPersisted: stepsPersistedOf(persisted), + }; + // Replace the live partial with the persisted row IN PLACE by id — + // the re-seed from persist. The attach's tail (steps >= N) then + // appends to a store holding EXACTLY steps 0..N-1: no duplication. + setMessages((prev) => mergeById(prev, rowToUiMessage(persisted))); + } else { + // No persisted assistant row (pre-first-frame break): drop the live + // partial + replay from start (no anchor/n) so nothing is duplicated. + dropLivePartialAndReplayFromStart(); + } + enterReconnect(res.run?.id ?? runId); + }); + }) + .catch(() => { + finishReseed(() => { + // Persist read FAILED: we cannot re-seed from fresh persist, and a + // stale mount-time anchor over the live partial would tail-apply + // already-present steps -> duplication (a flaky-network blip: + // SSE + getRun both fail, network recovers in ~1s, the registry still + // covers from the mount frontier). Restore the removed-filter guarantee + // instead: drop the live partial + replay from start / 204 -> poll. + dropLivePartialAndReplayFromStart(); + enterReconnect(runId); + }); + }); } else { dropLivePartialAndReplayFromStart(); enterReconnect(runId); @@ -903,6 +948,12 @@ export default function ChatThread({ } return () => { mountedRef.current = false; + // #541: clear the in-flight persist re-seed timeout (not an FSM effect timer, + // so DISPOSE does not touch it) — no dangling setTimeout after unmount. + if (reseedTimerRef.current) { + clearTimeout(reseedTimerRef.current); + reseedTimerRef.current = null; + } dispatch({ type: "DISPOSE" }); // aborts attach + timers, bumps epoch (I5) }; // Mount-only by design; the parent remounts per chat via `key`.