fix(dashboard): bound WS ticket minting on the events + PTY sockets (supersedes #81931)

web/src/components/ChatSidebar.test.tsx

@@ -3,6 +3,8 @@ import { act, type ReactNode } from "react";
 import { createRoot, type Root } from "react-dom/client";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+import { EVENTS_CONNECT_TIMEOUT_MS } from "@/lib/events-reconnect";
+
 const apiMocks = vi.hoisted(() => ({
   buildWsUrl: vi.fn(async () => "ws://localhost/api/events?channel=chat-1"),
   getModelInfo: vi.fn(async () => ({
@@ -113,6 +115,10 @@ async function render(ui: ReactNode) {
 beforeEach(() => {
   FakeWebSocket.instances = [];
   vi.clearAllMocks();
+  apiMocks.buildWsUrl.mockReset();
+  apiMocks.buildWsUrl.mockResolvedValue(
+    "ws://localhost/api/events?channel=chat-1",
+  );
   reloadMocks.maybeReloadForLoopbackWsAuthFailure.mockReturnValue(true);
   vi.stubGlobal("WebSocket", FakeWebSocket);
 });
@@ -180,6 +186,75 @@ describe("ChatSidebar event socket reconnect", () => {
     expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(2);
   });
 
+  it("keeps retrying when reconnect URL construction fails", async () => {
+    await renderSidebar();
+    apiMocks.buildWsUrl
+      .mockRejectedValueOnce(new Error("ticket endpoint unavailable"))
+      .mockResolvedValue("ws://localhost/api/events?channel=chat-1");
+
+    await act(async () => {
+      FakeWebSocket.instances[0].emit("close", { code: 1006 });
+    });
+
+    await advance(1_000);
+    expect(FakeWebSocket.instances).toHaveLength(1);
+    expect(container.textContent).toContain("reconnecting in 2s");
+
+    await advance(2_000);
+    expect(FakeWebSocket.instances).toHaveLength(2);
+    expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(3);
+  });
+
+  it("times out a stalled URL request and retries", async () => {
+    let resolveStalledRequest!: (url: string) => void;
+    await renderSidebar();
+    apiMocks.buildWsUrl
+      .mockImplementationOnce(
+        () =>
+          new Promise<string>((resolve) => {
+            resolveStalledRequest = resolve;
+          }),
+      )
+      .mockResolvedValue("ws://localhost/api/events?channel=chat-1");
+
+    await act(async () => {
+      FakeWebSocket.instances[0].emit("close", { code: 1006 });
+    });
+
+    await advance(1_000 + EVENTS_CONNECT_TIMEOUT_MS);
+    expect(FakeWebSocket.instances).toHaveLength(1);
+    expect(container.textContent).toContain("reconnecting in 2s");
+
+    // A late ticket response from the timed-out attempt must not create a
+    // superseded socket alongside the scheduled replacement.
+    await act(async () => {
+      resolveStalledRequest("ws://localhost/api/events?channel=stale");
+      await Promise.resolve();
+    });
+    expect(FakeWebSocket.instances).toHaveLength(1);
+
+    await advance(2_000);
+    expect(FakeWebSocket.instances).toHaveLength(2);
+    expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(3);
+  });
+
+  it("times out a stalled WebSocket handshake and retries", async () => {
+    await renderSidebar();
+
+    await act(async () => {
+      FakeWebSocket.instances[0].emit("close", { code: 1006 });
+    });
+    await advance(1_000);
+    expect(FakeWebSocket.instances).toHaveLength(2);
+
+    await advance(EVENTS_CONNECT_TIMEOUT_MS);
+    expect(FakeWebSocket.instances[1].closed).toBe(true);
+    expect(container.textContent).toContain("reconnecting in 2s");
+
+    await advance(2_000);
+    expect(FakeWebSocket.instances).toHaveLength(3);
+  });
+
   it("backs off exponentially across repeated failures", async () => {
     await renderSidebar();
 
@@ -210,7 +285,11 @@ describe("ChatSidebar event socket reconnect", () => {
       FakeWebSocket.instances[0].emit("close", { code: 1006 });
     });
 
-    await advance(30_000);
+    await advance(1_000);
+    await act(async () => {
+      FakeWebSocket.instances[1].emit("open", {});
+    });
+    await advance(29_000);
     expect(FakeWebSocket.instances).toHaveLength(2);
   });
 

web/src/components/ChatSidebar.tsx

@@ -36,6 +36,7 @@
 import { api, buildWsUrl } from "@/lib/api";
 import { maybeReloadForLoopbackWsAuthFailure } from "@/lib/dashboard-auth-reload";
 import {
+  EVENTS_CONNECT_TIMEOUT_MS,
   EVENTS_DISCONNECTED_MESSAGE,
   EVENTS_MAX_RECONNECT_ATTEMPTS,
   eventsGaveUpMessage,
@@ -100,7 +101,7 @@
  * can be tested without reading component source text. See
  * ``chat-sidebar-session-params.test.ts``.
  */
 export function sidecarSessionCreateParams(profile?: string): Record<string, unknown> {
   return {
     close_on_disconnect: true,
     source: "tool",
@@ -256,8 +257,17 @@
     let unmounting = false;
     let ws: WebSocket | null = null;
     let reconnectTimer: ReturnType<typeof setTimeout> | null = null;
+    let connectTimer: ReturnType<typeof setTimeout> | null = null;
+    let connectGeneration = 0;
     let attempt = 0;
 
+    const clearConnectTimer = () => {
+      if (connectTimer) {
+        clearTimeout(connectTimer);
+        connectTimer = null;
+      }
+    };
+
     // 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
@@ -298,14 +308,49 @@
       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) {
+
+      const generation = ++connectGeneration;
+      let socket: WebSocket | null = null;
+
+      // Cover the whole connection attempt, including gated-mode ticket
+      // minting. A failed or hanging pre-socket request otherwise emits no
+      // WebSocket close event and permanently strands the retry loop at its
+      // last "reconnecting in ..." banner.
+      clearConnectTimer();
+      connectTimer = setTimeout(() => {
+        connectTimer = null;
+        if (unmounting || generation !== connectGeneration) {
+          return;
+        }
+
+        // Invalidate any late ticket result or socket event from this attempt
+        // before scheduling its replacement.
+        connectGeneration += 1;
+        if (socket && ws === socket) {
+          ws = null;
+          socket.close();
+        }
+        scheduleReconnect();
+      }, EVENTS_CONNECT_TIMEOUT_MS);
+
+      try {
+        // 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 || generation !== connectGeneration) {
+          return;
+        }
+        socket = new WebSocket(url);
+        ws = socket;
+      } catch {
+        if (unmounting || generation !== connectGeneration) {
+          return;
+        }
+        clearConnectTimer();
+        connectGeneration += 1;
+        scheduleReconnect();
         return;
       }
-      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.
@@ -315,6 +360,7 @@
         if (!isCurrent()) {
           return;
         }
+        clearConnectTimer();
         attempt = 0;
         clearEventsBanner();
       });
@@ -332,6 +378,7 @@
         if (!isCurrent()) {
           return;
         }
+        clearConnectTimer();
         if (maybeReloadForLoopbackWsAuthFailure(ev.code)) {
           return;
         }
@@ -374,6 +421,8 @@
 
     return () => {
       unmounting = true;
+      connectGeneration += 1;
+      clearConnectTimer();
       if (reconnectTimer) {
         clearTimeout(reconnectTimer);
         reconnectTimer = null;

web/src/lib/events-reconnect.ts

@@ -9,6 +9,8 @@
 export const EVENTS_RECONNECT_BASE_MS = 1_000;
 export const EVENTS_RECONNECT_MAX_MS = 30_000;
 export const EVENTS_MAX_RECONNECT_ATTEMPTS = 15;
+/** Bound ticket minting plus the WebSocket opening handshake. */
+export const EVENTS_CONNECT_TIMEOUT_MS = 15_000;
 
 /** Normal closure — the server said goodbye, don't chase it. */
 const WS_CLOSE_NORMAL = 1000;

web/src/lib/pty-reconnect.ts

@@ -18,6 +18,13 @@ export const PTY_RESUME_RECONNECT_THROTTLE_MS = 1000;
 // and force-closed so `onclose` → scheduleReconnect can recover it.
 export const PTY_CONNECTING_TIMEOUT_MS = 8000;
 
+// The same budget for the phase *before* the socket exists: in gated mode a
+// connect first awaits a fresh single-use ticket. That request produces no
+// WebSocket, so a rejection or a hang is invisible to both `onclose` and the
+// CONNECTING timer above — the tab would sit on "connecting" forever with no
+// retry. Bound it so the failure routes into the ordinary backoff instead.
+export const PTY_TICKET_TIMEOUT_MS = 8000;
+
 // How long after a resumed socket opens we keep suppressing ANSI erase codes
 // (`ESC[K` / `ESC[X`) from the PTY stream. Ink's two-pass virtual scroll emits
 // them while replaying a long session; past that replay they are legitimate

web/src/pages/ChatPage.test.tsx

@@ -4,6 +4,8 @@ import { createRoot, type Root } from "react-dom/client";
 import { MemoryRouter } from "react-router";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+import { PTY_TICKET_TIMEOUT_MS } from "@/lib/pty-reconnect";
+
 class FakeFitAddon {
   fit() {}
 }
@@ -65,6 +67,9 @@ class FakeTerminal {
 }
 
 const maybeReloadForLoopbackWsAuthFailure = vi.fn(() => false);
+const apiMocks = vi.hoisted(() => ({
+  buildWsUrl: vi.fn(async () => "ws://localhost/api/pty?channel=chat-1"),
+}));
 
 vi.mock("@xterm/addon-fit", () => ({ FitAddon: FakeFitAddon }));
 vi.mock("@xterm/addon-unicode11", () => ({ Unicode11Addon: class {} }));
@@ -104,6 +109,10 @@ vi.mock("@/i18n", () => ({
 vi.mock("@/lib/dashboard-auth-reload", () => ({
   maybeReloadForLoopbackWsAuthFailure,
 }));
+vi.mock("@/lib/api", () => ({
+  api: apiMocks,
+  buildWsUrl: apiMocks.buildWsUrl,
+}));
 
 class FakeWebSocket {
   static instances: FakeWebSocket[] = [];
@@ -147,6 +156,8 @@ async function render(ui: ReactNode) {
 beforeEach(() => {
   FakeWebSocket.instances = [];
   maybeReloadForLoopbackWsAuthFailure.mockClear();
+  apiMocks.buildWsUrl.mockReset();
+  apiMocks.buildWsUrl.mockResolvedValue("ws://localhost/api/pty?channel=chat-1");
   vi.stubGlobal("WebSocket", FakeWebSocket);
   vi.stubGlobal(
     "ResizeObserver",
@@ -226,3 +237,90 @@ describe("ChatPage", () => {
     expect(maybeReloadForLoopbackWsAuthFailure).toHaveBeenCalledWith(4401);
   });
 });
+
+// The gated-mode ticket request runs before any socket exists, so a rejection
+// or a hang emits no `close` event and never arms PTY_CONNECTING_TIMEOUT_MS
+// (that timer is set after `new WebSocket`). Without its own deadline the tab
+// strands on "connecting" with no retry. Mirrors the ChatSidebar events-feed
+// coverage in src/components/ChatSidebar.test.tsx.
+describe("ChatPage PTY ticket connect deadline", () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  async function renderChat() {
+    const { default: ChatPage } = await import("./ChatPage");
+    await render(
+      <MemoryRouter initialEntries={["/chat"]}>
+        <ChatPage isActive />
+      </MemoryRouter>,
+    );
+  }
+
+  /** 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("retries when the ticket request rejects", async () => {
+    apiMocks.buildWsUrl.mockRejectedValueOnce(
+      new Error("ticket endpoint unavailable"),
+    );
+
+    await renderChat();
+    await advance(0);
+    expect(FakeWebSocket.instances).toHaveLength(0);
+
+    // First backoff step is 250ms; the retry must mint a fresh ticket.
+    await advance(250);
+    expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(2);
+    expect(FakeWebSocket.instances).toHaveLength(1);
+  });
+
+  it("times out a stalled ticket request and retries", async () => {
+    let resolveStalledRequest!: (url: string) => void;
+    apiMocks.buildWsUrl.mockImplementationOnce(
+      () =>
+        new Promise<string>((resolve) => {
+          resolveStalledRequest = resolve;
+        }),
+    );
+
+    await renderChat();
+    await advance(0);
+    expect(FakeWebSocket.instances).toHaveLength(0);
+
+    await advance(PTY_TICKET_TIMEOUT_MS);
+    expect(FakeWebSocket.instances).toHaveLength(0);
+
+    // A late ticket from the timed-out attempt must not open a socket behind
+    // the replacement the deadline scheduled.
+    await act(async () => {
+      resolveStalledRequest("ws://localhost/api/pty?channel=stale");
+      await Promise.resolve();
+    });
+    expect(FakeWebSocket.instances).toHaveLength(0);
+
+    await advance(250);
+    expect(FakeWebSocket.instances).toHaveLength(1);
+    expect(FakeWebSocket.instances[0].url).not.toContain("channel=stale");
+  });
+
+  it("leaves a settled ticket's socket to the CONNECTING timer", async () => {
+    await renderChat();
+    await advance(0);
+    await vi.waitFor(() => expect(FakeWebSocket.instances).toHaveLength(1));
+
+    // NS-591 regression: once the socket exists the ticket deadline is
+    // disarmed, so PTY_CONNECTING_TIMEOUT_MS stays the only thing that may
+    // force-close a wedged handshake — the two must not both fire.
+    await advance(PTY_TICKET_TIMEOUT_MS);
+    expect(apiMocks.buildWsUrl).toHaveBeenCalledTimes(1);
+  });
+});