diff --git a/web/src/components/ChatSidebar.test.tsx b/web/src/components/ChatSidebar.test.tsx index 8a05ee6a6a905..523faa093c9b3 100644 --- a/web/src/components/ChatSidebar.test.tsx +++ b/web/src/components/ChatSidebar.test.tsx @@ -11,16 +11,23 @@ const apiMocks = vi.hoisted(() => ({ })), })); -const gatewayMocks = vi.hoisted(() => ({ - close: vi.fn(), - connect: vi.fn(async () => undefined), - on: vi.fn(() => () => undefined), - onState: vi.fn((handler: (state: string) => void) => { - handler("open"); - return () => undefined; - }), - request: vi.fn(async () => ({ session_id: "sidecar-1" })), -})); +const gatewayMocks = vi.hoisted(() => { + const handlers = new Map void>(); + return { + close: vi.fn(), + connect: vi.fn(async () => undefined), + handlers, + on: vi.fn((event: string, handler: (event: unknown) => void) => { + handlers.set(event, handler); + return () => handlers.delete(event); + }), + onState: vi.fn((handler: (state: string) => void) => { + handler("open"); + return () => undefined; + }), + request: vi.fn(async () => ({ session_id: "sidecar-1" })), + }; +}); const reloadMocks = vi.hoisted(() => ({ maybeReloadForLoopbackWsAuthFailure: vi.fn(() => true), @@ -69,6 +76,7 @@ class FakeWebSocket { private listeners = new Map void>>(); readonly url: string; + closed = false; constructor(url: string) { this.url = url; @@ -81,7 +89,9 @@ class FakeWebSocket { this.listeners.set(type, listeners); } - close() {} + close() { + this.closed = true; + } emit(type: string, event: EventLike) { for (const listener of this.listeners.get(type) ?? []) { @@ -131,3 +141,236 @@ describe("ChatSidebar event socket", () => { ).toHaveBeenCalledWith(4401); }); }); + +describe("ChatSidebar event socket reconnect", () => { + beforeEach(() => { + // Not loopback: exercise the gated-mode path so closes fall through to + // the reconnect logic instead of triggering a page reload. + reloadMocks.maybeReloadForLoopbackWsAuthFailure.mockReturnValue(false); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + async function renderSidebar() { + const { ChatSidebar } = await import("./ChatSidebar"); + await render(); + await vi.waitFor(() => expect(FakeWebSocket.instances).toHaveLength(1)); + } + + /** Advance timers and flush the async `connect()` that fires on the tick. */ + async function advance(ms: number) { + await act(async () => { + await vi.advanceTimersByTimeAsync(ms); + }); + } + + it("reconnects after a transient close", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + expect(FakeWebSocket.instances).toHaveLength(1); + + await advance(1_000); + expect(FakeWebSocket.instances).toHaveLength(2); + expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(2); + }); + + it("backs off exponentially across repeated failures", async () => { + await renderSidebar(); + + // 1s, then 2s, then 4s — a socket that never opens keeps backing off. + for (const [index, delay] of [1_000, 2_000, 4_000].entries()) { + await act(async () => { + FakeWebSocket.instances[index].emit("close", { code: 1006 }); + }); + + // The previous (shorter) delay must not be enough to fire this one. + if (index > 0) { + await advance(delay - 1); + expect(FakeWebSocket.instances).toHaveLength(index + 1); + } + + await advance(delay); + expect(FakeWebSocket.instances).toHaveLength(index + 2); + } + }); + + it("schedules only one retry when error and close both fire", async () => { + await renderSidebar(); + + // A failed socket emits `error` then `close`. Scheduling from both + // paths would queue two timers and leak the untracked one. + await act(async () => { + FakeWebSocket.instances[0].emit("error", {}); + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + + await advance(30_000); + expect(FakeWebSocket.instances).toHaveLength(2); + }); + + it("resets the backoff after a successful reconnect", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + await advance(1_000); + expect(FakeWebSocket.instances).toHaveLength(2); + + // Reconnected — the next drop should start from 1s again, not 2s. + await act(async () => { + FakeWebSocket.instances[1].emit("open", {}); + FakeWebSocket.instances[1].emit("close", { code: 1006 }); + }); + await advance(1_000); + expect(FakeWebSocket.instances).toHaveLength(3); + }); + + it("does not retry auth rejections", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 4403 }); + }); + + await advance(60_000); + expect(FakeWebSocket.instances).toHaveLength(1); + }); + + it("does not retry a normal closure", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1000 }); + }); + + await advance(60_000); + expect(FakeWebSocket.instances).toHaveLength(1); + }); + + it("gives up after the attempt cap instead of retrying forever", async () => { + await renderSidebar(); + + for (let i = 0; i < 40; i++) { + const socket = + FakeWebSocket.instances[FakeWebSocket.instances.length - 1]; + await act(async () => { + socket.emit("close", { code: 1006 }); + }); + await advance(30_000); + } + + // 15 retries + the initial connection. + expect(FakeWebSocket.instances.length).toBeLessThanOrEqual(16); + }); + + it("clears its own banner on a successful reconnect", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + expect(container.textContent).toContain("events feed disconnected"); + + await advance(1_000); + await act(async () => { + FakeWebSocket.instances[1].emit("open", {}); + }); + + // Banner gone entirely — including the "reconnect events feed" button, + // which only renders while `error` is set. + expect(container.textContent).not.toContain("events feed"); + }); + + it("does not clear a credential warning when the feed recovers", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + await advance(1_000); + + // A sidecar error lands while the events socket is still reconnecting. + // The banner is shared, so a blind `setError(null)` on reconnect would + // hide a real problem the user needs to see. + await act(async () => { + gatewayMocks.handlers.get("error")?.({ + payload: { message: "ANTHROPIC_API_KEY is not set" }, + }); + }); + + await act(async () => { + FakeWebSocket.instances[1].emit("open", {}); + }); + + expect(container.textContent).toContain("ANTHROPIC_API_KEY is not set"); + }); + + it("does not overwrite a sidecar error when the feed drops", async () => { + await renderSidebar(); + + // A sidecar error is already on the banner... + await act(async () => { + gatewayMocks.handlers.get("error")?.({ + payload: { message: "ANTHROPIC_API_KEY is not set" }, + }); + }); + + // ...when the events feed drops. `error` is that message's only home, + // so overwriting it loses the warning permanently — the feed's own + // banner would later clear itself to null and the sidecar never + // re-emits. + await act(async () => { + FakeWebSocket.instances[0].emit("error", {}); + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + + expect(container.textContent).toContain("ANTHROPIC_API_KEY is not set"); + // The disconnect message must not have replaced it. (Matching the + // banner text specifically — "reconnect events feed" is the button + // label, which is expected to be present whenever a banner shows.) + expect(container.textContent).not.toContain("events feed disconnected"); + }); + + it("still reconnects while a foreign banner suppresses its message", async () => { + await renderSidebar(); + + await act(async () => { + gatewayMocks.handlers.get("error")?.({ + payload: { message: "ANTHROPIC_API_KEY is not set" }, + }); + }); + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + + // Declining to write the banner must not disable the retry itself. + await advance(1_000); + expect(FakeWebSocket.instances).toHaveLength(2); + }); + + it("clears the reconnect timer and closes the socket on unmount", async () => { + await renderSidebar(); + + await act(async () => { + FakeWebSocket.instances[0].emit("close", { code: 1006 }); + }); + expect(vi.getTimerCount()).toBeGreaterThan(0); + + await act(async () => root.unmount()); + + expect(FakeWebSocket.instances[0].closed).toBe(true); + // The pending retry timer must be cleared, not merely neutered by the + // `unmounting` flag — a live timer keeps the effect closure alive. + expect(vi.getTimerCount()).toBe(0); + + await advance(60_000); + expect(FakeWebSocket.instances).toHaveLength(1); + }); +}); diff --git a/web/src/components/ChatSidebar.tsx b/web/src/components/ChatSidebar.tsx index fd4df2edcdeda..aa4bd69ea32ed 100644 --- a/web/src/components/ChatSidebar.tsx +++ b/web/src/components/ChatSidebar.tsx @@ -17,7 +17,9 @@ * the dashboard fanned out. The sidebar uses it for `session.info` * (live chat title) and `dashboard.new_session_requested`. The * `channel` id ties this listener to the same chat tab's PTY child — - * see `ChatPage.tsx` for where the id is generated. + * see `ChatPage.tsx` for where the id is generated. Transient drops + * (gateway restart, network blip) auto-reconnect with exponential + * backoff; auth rejections are terminal. See `lib/events-reconnect`. * * Best-effort throughout: WS failures show in the badge / banner, the * terminal pane keeps working unimpaired. @@ -33,6 +35,17 @@ import { ReasoningPicker } from "@/components/ReasoningPicker"; import { GatewayClient, type ConnectionState } from "@/lib/gatewayClient"; import { api, buildWsUrl } from "@/lib/api"; import { maybeReloadForLoopbackWsAuthFailure } from "@/lib/dashboard-auth-reload"; +import { + EVENTS_DISCONNECTED_MESSAGE, + EVENTS_MAX_RECONNECT_ATTEMPTS, + eventsGaveUpMessage, + eventsReconnectDelayMs, + eventsReconnectingMessage, + eventsRejectedMessage, + isEventsAuthRejection, + isEventsFeedMessage, + shouldRetryEventsClose, +} from "@/lib/events-reconnect"; import { titleFromSessionInfoPayload } from "@/lib/chat-title"; import { cn } from "@/lib/utils"; @@ -237,39 +250,101 @@ export function ChatSidebar({ return; } // In loopback mode the legacy ?token= path is fine; in gated - // mode we have to mint a single-use ticket from the cookie. The IIFE - // keeps the outer effect synchronous so its ``return cleanup`` stays - // at the top level; the local ``ws`` is hoisted to a closed-over - // binding the cleanup reads via ``wsRef``. + // mode we have to mint a single-use ticket from the cookie. `connect` + // keeps the outer effect synchronous so its ``return cleanup`` stays at + // the top level; `ws` is a closed-over binding the cleanup reads. let unmounting = false; let ws: WebSocket | null = null; - void (async () => { + let reconnectTimer: ReturnType | null = null; + let attempt = 0; + + // The banner is shared with `info.credential_warning` and the JSON-RPC + // sidecar, and `error` is those messages' only home — the sidecar does + // not re-emit. So the events feed may only write over an empty banner + // or one of its own messages, and may only clear its own. + const surface = (msg: string) => + !unmounting && + setError((current) => + isEventsFeedMessage(current) ? msg : (current ?? msg), + ); + + const clearEventsBanner = () => + !unmounting && + setError((current) => (isEventsFeedMessage(current) ? null : current)); + + // Single scheduling path. `close` always follows `error` for a failed + // socket, so scheduling from `error` too would queue two timers and + // leak the first — only the latest is tracked for cleanup. + const scheduleReconnect = () => { + if (unmounting || reconnectTimer) { + return; + } + if (attempt >= EVENTS_MAX_RECONNECT_ATTEMPTS) { + surface(eventsGaveUpMessage()); + return; + } + + const delay = eventsReconnectDelayMs(attempt); + attempt += 1; + surface(eventsReconnectingMessage(delay)); + + reconnectTimer = setTimeout(() => { + reconnectTimer = null; + void connect(); + }, delay); + }; + + const connect = async () => { + if (unmounting) { + return; + } + // Re-minted every attempt: tickets are single-use with a short TTL, + // so a reconnect cannot replay the URL from the first connection. const url = await buildWsUrl("/api/events", { channel }); if (unmounting) { return; } - ws = new WebSocket(url); + const socket = new WebSocket(url); + ws = socket; + + // A superseded socket's late close must not schedule a retry on top + // of the one that replaced it. + const isCurrent = () => ws === socket; + + socket.addEventListener("open", () => { + if (!isCurrent()) { + return; + } + attempt = 0; + clearEventsBanner(); + }); // `unmounting` suppresses the banner during cleanup — `ws.close()` // from the effect's return fires a close event with code 1005 that // would otherwise look like an unexpected drop. - const DISCONNECTED = "events feed disconnected — tool calls may not appear"; - const surface = (msg: string) => !unmounting && setError(msg); - - ws.addEventListener("error", () => surface(DISCONNECTED)); - - ws.addEventListener("close", (ev) => { - if (maybeReloadForLoopbackWsAuthFailure(ev.code)) { - return; - } - if (ev.code === 4401 || ev.code === 4403) { - surface(`events feed rejected (${ev.code}) — reload the page`); - } else if (ev.code !== 1000) { - surface(DISCONNECTED); + socket.addEventListener("error", () => { + if (isCurrent()) { + surface(EVENTS_DISCONNECTED_MESSAGE); } }); - ws.addEventListener("message", (ev) => { + socket.addEventListener("close", (ev) => { + if (!isCurrent()) { + return; + } + if (maybeReloadForLoopbackWsAuthFailure(ev.code)) { + return; + } + if (isEventsAuthRejection(ev.code)) { + surface(eventsRejectedMessage(ev.code)); + return; + } + if (shouldRetryEventsClose(ev.code)) { + scheduleReconnect(); + } + }); + + socket.addEventListener("message", (ev) => { let frame: RpcEnvelope; try { @@ -293,10 +368,16 @@ export function ChatSidebar({ onDashboardNewSessionRequest?.(); } }); - })(); + }; + + void connect(); return () => { unmounting = true; + if (reconnectTimer) { + clearTimeout(reconnectTimer); + reconnectTimer = null; + } ws?.close(); }; }, [channel, onDashboardNewSessionRequest, onSessionTitleChange, version]); @@ -397,7 +478,7 @@ export function ChatSidebar({ onClick={reconnect} prefix={} > - reconnect tools feed + reconnect events feed )} diff --git a/web/src/lib/events-reconnect.test.ts b/web/src/lib/events-reconnect.test.ts new file mode 100644 index 0000000000000..e059c7c225c78 --- /dev/null +++ b/web/src/lib/events-reconnect.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from "vitest"; + +import { + EVENTS_MAX_RECONNECT_ATTEMPTS, + EVENTS_RECONNECT_BASE_MS, + EVENTS_RECONNECT_MAX_MS, + eventsGaveUpMessage, + eventsReconnectDelayMs, + eventsReconnectingMessage, + eventsRejectedMessage, + isEventsAuthRejection, + isEventsFeedMessage, + shouldRetryEventsClose, + EVENTS_DISCONNECTED_MESSAGE, +} from "./events-reconnect"; + +describe("eventsReconnectDelayMs", () => { + it("doubles from the base delay", () => { + expect(eventsReconnectDelayMs(0)).toBe(EVENTS_RECONNECT_BASE_MS); + expect(eventsReconnectDelayMs(1)).toBe(EVENTS_RECONNECT_BASE_MS * 2); + expect(eventsReconnectDelayMs(2)).toBe(EVENTS_RECONNECT_BASE_MS * 4); + expect(eventsReconnectDelayMs(3)).toBe(EVENTS_RECONNECT_BASE_MS * 8); + }); + + it("clamps at the cap and never exceeds it", () => { + for ( + let attempt = 0; + attempt <= EVENTS_MAX_RECONNECT_ATTEMPTS + 5; + attempt++ + ) { + expect(eventsReconnectDelayMs(attempt)).toBeLessThanOrEqual( + EVENTS_RECONNECT_MAX_MS, + ); + } + expect(eventsReconnectDelayMs(99)).toBe(EVENTS_RECONNECT_MAX_MS); + }); + + it("is monotonically non-decreasing", () => { + let previous = 0; + for (let attempt = 0; attempt < 20; attempt++) { + const delay = eventsReconnectDelayMs(attempt); + expect(delay).toBeGreaterThanOrEqual(previous); + previous = delay; + } + }); + + it("stays finite for absurd attempt counts", () => { + expect(Number.isFinite(eventsReconnectDelayMs(10_000))).toBe(true); + }); +}); + +describe("shouldRetryEventsClose", () => { + it("retries transient drops", () => { + // 1005 (no status) and 1006 (abnormal) are what a killed gateway and a + // dropped network produce respectively. + for (const code of [1001, 1005, 1006, 1011, 1012, 1013]) { + expect(shouldRetryEventsClose(code)).toBe(true); + } + }); + + it("does not retry a normal closure", () => { + expect(shouldRetryEventsClose(1000)).toBe(false); + }); + + it("does not retry auth rejections", () => { + expect(shouldRetryEventsClose(4401)).toBe(false); + expect(shouldRetryEventsClose(4403)).toBe(false); + }); + + it("retries when the code is missing", () => { + expect(shouldRetryEventsClose(undefined)).toBe(true); + }); + + it("never both retries and reports an auth rejection", () => { + for (const code of [1000, 1005, 1006, 4401, 4403, 4500]) { + expect(shouldRetryEventsClose(code) && isEventsAuthRejection(code)).toBe( + false, + ); + } + }); +}); + +describe("isEventsFeedMessage", () => { + it("recognizes every message this module can surface", () => { + expect(isEventsFeedMessage(EVENTS_DISCONNECTED_MESSAGE)).toBe(true); + expect(isEventsFeedMessage(eventsReconnectingMessage(4_000))).toBe(true); + expect(isEventsFeedMessage(eventsRejectedMessage(4401))).toBe(true); + expect(isEventsFeedMessage(eventsGaveUpMessage())).toBe(true); + }); + + it("does not claim messages owned by other banner sources", () => { + // The banner is shared with info.credential_warning and the JSON-RPC + // sidecar — clearing those on an events reconnect would hide a real + // problem from the user. + expect(isEventsFeedMessage("ANTHROPIC_API_KEY is not set")).toBe(false); + expect(isEventsFeedMessage("WebSocket connection failed")).toBe(false); + expect(isEventsFeedMessage("gateway not connected")).toBe(false); + expect(isEventsFeedMessage(null)).toBe(false); + expect(isEventsFeedMessage("")).toBe(false); + }); +}); + +describe("reconnect message copy", () => { + it("renders the delay in whole seconds", () => { + expect(eventsReconnectingMessage(1_000)).toContain("1s"); + expect(eventsReconnectingMessage(30_000)).toContain("30s"); + }); + + it("names the close code in the rejection message", () => { + expect(eventsRejectedMessage(4403)).toContain("4403"); + }); + + it("does not reference the tools box removed in #51737", () => { + const messages = [ + EVENTS_DISCONNECTED_MESSAGE, + eventsReconnectingMessage(1_000), + eventsRejectedMessage(4401), + eventsGaveUpMessage(), + ]; + for (const message of messages) { + expect(message).not.toContain("tool calls"); + } + }); +}); diff --git a/web/src/lib/events-reconnect.ts b/web/src/lib/events-reconnect.ts new file mode 100644 index 0000000000000..f533bd924ea53 --- /dev/null +++ b/web/src/lib/events-reconnect.ts @@ -0,0 +1,82 @@ +/** + * Reconnect policy for the ChatSidebar `/api/events` subscriber socket. + * + * Pure helpers, no DOM: the component owns the socket and the timer, this + * module owns the arithmetic and the "is this close code worth retrying" + * decision so both can be unit-tested without a fake WebSocket. + */ + +export const EVENTS_RECONNECT_BASE_MS = 1_000; +export const EVENTS_RECONNECT_MAX_MS = 30_000; +export const EVENTS_MAX_RECONNECT_ATTEMPTS = 15; + +/** Normal closure — the server said goodbye, don't chase it. */ +const WS_CLOSE_NORMAL = 1000; +/** Ticket rejected / forbidden: retrying just burns tickets, user must reload. */ +const WS_CLOSE_AUTH_CODES = new Set([4401, 4403]); + +/** + * Exponential backoff, 1s → 2s → 4s → … → 30s cap. + * + * `attempt` is 0-based: attempt 0 is the first retry after the initial + * connection dropped. + */ +export function eventsReconnectDelayMs(attempt: number): number { + const exponent = Math.max(0, Math.trunc(attempt)); + + // 2 ** exponent overflows to Infinity long before it matters; Math.min + // still clamps correctly, but guard anyway so the delay stays a number. + const raw = EVENTS_RECONNECT_BASE_MS * 2 ** Math.min(exponent, 32); + + return Math.min(raw, EVENTS_RECONNECT_MAX_MS); +} + +/** + * Whether a close code should trigger a retry. + * + * Auth rejections are terminal (the banner tells the user to reload) and a + * normal 1000 close is intentional. Everything else — gateway restart, + * network drop, 1005/1006, proxy timeout — is worth retrying. + */ +export function shouldRetryEventsClose(code: number | undefined): boolean { + if (code === undefined) { + return true; + } + + return code !== WS_CLOSE_NORMAL && !WS_CLOSE_AUTH_CODES.has(code); +} + +export function isEventsAuthRejection(code: number | undefined): boolean { + return code !== undefined && WS_CLOSE_AUTH_CODES.has(code); +} + +// The sidebar's banner is shared with `info.credential_warning` and with the +// JSON-RPC sidecar's errors, so the events socket may only clear a message it +// wrote itself. Everything this module can put in the banner is listed here. +export const EVENTS_DISCONNECTED_MESSAGE = + "events feed disconnected — the chat title may not update"; + +export function eventsReconnectingMessage(delayMs: number): string { + return `events feed disconnected — reconnecting in ${Math.round(delayMs / 1000)}s…`; +} + +export function eventsRejectedMessage(code: number): string { + return `events feed rejected (${code}) — reload the page`; +} + +export function eventsGaveUpMessage(): string { + return `events feed disconnected — gave up after ${EVENTS_MAX_RECONNECT_ATTEMPTS} attempts, reload the page`; +} + +/** + * True when `message` is one this module produced, i.e. safe to clear on a + * successful reconnect. Guards against stomping a `credential_warning` or a + * sidecar error that happens to be showing when the feed recovers. + */ +export function isEventsFeedMessage(message: string | null): boolean { + if (!message) { + return false; + } + + return message.startsWith("events feed "); +}