Merge pull request #678: fix(sse): retry an EventSource connect attempt that never opens

src/web/CLAUDE.md

@@ -86,7 +86,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L
 
 **Connection-loss UI** (`computeConnectionLossUi()` in constants.js, writer `_updateConnectionLossUi()` in app.js): the service worker serves the cached app shell, so an unreachable server (phone off the tailnet, VPN down, server stopped) used to render a normal-looking empty dashboard whose only tell was the 8px header dot, which reads as "no sessions", not "no connection". Two surfaces now: a full-screen **overlay** while no server state has loaded this page load (nothing behind it is worth preserving), and a non-blocking **banner** once it has (the terminal scrollback stays readable). ⚠️ A **2.5s grace** is load-bearing: a COM deploy restarts the server and SSE is back in ~200ms, and a banner on every deploy trains the user to ignore it. `navigator.onLine === false` skips the grace, since that is never a blip. Retry re-arms SSE **and** the terminal WS (`planWsReconnect` can 'give-up', and the SSE backoff caps at 30s).
 
-**SSE staleness watchdog** (`computeSseStale()` in constants.js, `_checkSseStale()` + a 5s interval in app.js): an `EventSource` can stop delivering without erroring, so the client forces a reconnect when nothing arrives. ⚠️ The server keepalive must stay the named `sse:heartbeat` event (`cleanupDeadClients()`, sse-stream-manager.ts), never an SSE comment, which `EventSource` cannot observe; its no-op client listener must stay registered. ⚠️ Judge staleness only while `connected` and online (the loop breaker). ⚠️ The liveness stamp lives inside `addListener`. ⚠️ Clear the interval only at the top of `connectSSE()`, or intervals stack. → [architecture-invariants#sse-staleness-watchdog](../../docs/architecture-invariants.md#sse-staleness-watchdog)
+**SSE staleness watchdog** (`computeSseStale()` in constants.js, `_checkSseStale()` + a 5s interval in app.js): an `EventSource` can stop delivering without erroring, so the client forces a reconnect when nothing arrives. ⚠️ The server keepalive must stay the named `sse:heartbeat` event (`cleanupDeadClients()`, sse-stream-manager.ts), never an SSE comment, which `EventSource` cannot observe; its no-op client listener must stay registered. ⚠️ Judge staleness only while `connected` and online (the loop breaker). ⚠️ The liveness stamp lives inside `addListener`. ⚠️ Clear the interval only at the top of `connectSSE()`, or intervals stack. ⚠️ The watchdog cannot see an attempt that never OPENS (no response headers: no `open`, no `error`, never `connected`), so each attempt is bounded by `SSE_CONNECT_TIMEOUT_MS` (15s, `CodemanSseStale.CONNECT_TIMEOUT_MS`) and then retried through `_handleSseConnectionFailure()`, the same backoff `onerror` uses. Its timer is cleared at the top of `connectSSE()` BEFORE the offline early return, on `open`, and in `_handleSseConnectionFailure()`; the timer and the handler both ignore an `EventSource` that has been replaced. The window `offline` handler leaves it running on purpose (`navigator.onLine` can report offline on a working network). Tests: `test/sse-connect-timeout.test.ts`. → [architecture-invariants#sse-staleness-watchdog](../../docs/architecture-invariants.md#sse-staleness-watchdog)
 
 **Z-index layers** (keep new overlays consistent with this stack): local echo overlay (7; with local echo on it also draws the iOS IME composition preview, as an underlined tail after its pending text via `setComposition`), iOS IME composition preview span when local echo is off (6 inside `.xterm-helpers`, whose own z-index 5 is its EFFECTIVE layer, so it sits UNDER the overlay and must never be used while the overlay shows text), terminal touch-selection bar (900, below floating agent windows), subagent windows + split picker menu + Tiles count menu (1000), plan agents (1100), mobile/tablet fixed header (1200), modals on ≤768px (1300, must beat the fixed header), log viewers (2000), connection-loss overlay (2500), image popups (3000), response viewer (5000, backdrop 4999), folder grid (5090, above the response viewer it opens from, below the file preview its tiles open), file-preview overlay (5100, must outrank the response viewer that launches it), toasts/path picker (10000+), custom-model center-status banner (10001; its `[hidden]` must re-assert `display: none` or `dismiss()` leaves an invisible click-blocker), custom-model swap-confirm/context-warning modals (10010). → [architecture-invariants#z-index-layers](../../docs/architecture-invariants.md#z-index-layers)
 

src/web/public/app.js

@@ -997,6 +997,12 @@ class CodemanApp {
     this._sseLastMessageAt = 0;
     this._sseStaleTimeoutMs = window.CodemanSseStale?.TIMEOUT_MS ?? 45000;
     this._sseStaleWatchdog = null;
+    // Connect timeout: the watchdog only judges a 'connected' stream, and an
+    // attempt whose request hangs never opens AND never errors. Each attempt
+    // is bounded by this timer (armed in connectSSE, cleared on open, on
+    // failure and at the top of every connectSSE so timers never stack).
+    this._sseConnectTimeout = null;
+    this._sseConnectTimeoutMs = window.CodemanSseStale?.CONNECT_TIMEOUT_MS ?? 15000;
 
     // Connection-loss UI (banner + full-screen overlay). The decision itself is
     // pure and lives in constants.js (computeConnectionLossUi); these are just
@@ -2066,6 +2072,10 @@ class CodemanApp {
   }
 
   connectSSE() {
+    // First, before the offline early return: any path into connectSSE
+    // supersedes the previous attempt, and its deadline must go with it.
+    this._clearTimer('_sseConnectTimeout');
+
     // Check if browser is offline
     if (!navigator.onLine) {
       this.setConnectionStatus('offline');
@@ -2109,7 +2119,21 @@ class CodemanApp {
     const _sseParams = new URLSearchParams({ clientId: this._clientId });
     const _sseFilterId = this._sseFilterSessionId(this.activeSessionId);
     if (_sseFilterId) _sseParams.set('sessions', _sseFilterId);
-    this.eventSource = new EventSource(CodemanBase.url(`/api/events?${_sseParams.toString()}`));
+    const eventSource = new EventSource(CodemanBase.url(`/api/events?${_sseParams.toString()}`));
+    this.eventSource = eventSource;
+
+    // Bound the attempt. A request that never gets response headers leaves
+    // the EventSource in CONNECTING with no `open` and no `error`, so without
+    // this the page waits forever. readyState OPEN covers the narrow race
+    // where the connection opened but its `open` callback has not run yet.
+    const connectTimeout = setTimeout(() => {
+      if (this._sseConnectTimeout !== connectTimeout) return;
+      this._sseConnectTimeout = null;
+      if (this.eventSource !== eventSource || eventSource.readyState === EventSource.OPEN) return;
+      console.warn(`[SSE] connection did not open within ${this._sseConnectTimeoutMs}ms, retrying`);
+      this._handleSseConnectionFailure(eventSource);
+    }, this._sseConnectTimeoutMs);
+    this._sseConnectTimeout = connectTimeout;
 
     // Store all event listeners for cleanup on reconnect.
     //
@@ -2138,6 +2162,7 @@ class CodemanApp {
     };
 
     this.eventSource.onopen = () => {
+      this._clearTimer('_sseConnectTimeout');
       this.reconnectAttempts = 0;
       // Start the liveness clock here, not at the first frame: the watchdog
       // only ever fires while the status is 'connected', and this is the
@@ -2145,31 +2170,7 @@ class CodemanApp {
       this._sseLastMessageAt = Date.now();
       this.setConnectionStatus('connected');
     };
-    this.eventSource.onerror = () => {
-      this.reconnectAttempts++;
-      if (this.reconnectAttempts >= this.maxReconnectAttempts) {
-        this.setConnectionStatus('disconnected');
-      } else {
-        this.setConnectionStatus('reconnecting');
-      }
-      // Close the failed connection before scheduling reconnect
-      if (this.eventSource) {
-        this.eventSource.close();
-        this.eventSource = null;
-      }
-      // Clear any existing reconnect timeout before setting new one (prevents orphaned timeouts)
-      this._clearTimer('sseReconnectTimeout');
-      // Exponential backoff: 200ms, 500ms, 1s, 2s, 4s, ... up to 30s
-      // Fast first retry (200ms) for server-restart case (COM deploy),
-      // then ramp up for real network issues.
-      const delay = this.reconnectAttempts <= 1 ? 200
-        : Math.min(500 * Math.pow(2, this.reconnectAttempts - 2), 30000);
-      // Feeds the "Retrying in Ns" countdown. With a 30s cap on the backoff, a
-      // silent wait that long is indistinguishable from a hung app.
-      this._nextSseRetryAt = Date.now() + delay;
-      this._updateConnectionLossUi();
-      this.sseReconnectTimeout = setTimeout(() => this.connectSSE(), delay);
-    };
+    this.eventSource.onerror = () => this._handleSseConnectionFailure(eventSource);
 
     // Create stable handler wrappers once (reused across reconnects so
     // removeEventListener always matches the original reference)
@@ -2332,6 +2333,38 @@ class CodemanApp {
     this._sseStaleWatchdog = setInterval(() => this._checkSseStale(), 5000);
   }
 
+  /**
+   * Close a failed SSE attempt (onerror, or the connect timeout) and schedule
+   * the normal exponential-backoff retry.
+   */
+  _handleSseConnectionFailure(eventSource) {
+    // A late error or timeout from an EventSource that has since been
+    // replaced must not close or reschedule the connection that replaced it.
+    if (this.eventSource !== eventSource) return;
+    this._clearTimer('_sseConnectTimeout');
+    this.reconnectAttempts++;
+    if (this.reconnectAttempts >= this.maxReconnectAttempts) {
+      this.setConnectionStatus('disconnected');
+    } else {
+      this.setConnectionStatus('reconnecting');
+    }
+    // Close the failed connection before scheduling reconnect
+    this.eventSource = null;
+    eventSource.close();
+    // Clear any existing reconnect timeout before setting new one (prevents orphaned timeouts)
+    this._clearTimer('sseReconnectTimeout');
+    // Exponential backoff: 200ms, 500ms, 1s, 2s, 4s, ... up to 30s
+    // Fast first retry (200ms) for server-restart case (COM deploy),
+    // then ramp up for real network issues.
+    const delay = this.reconnectAttempts <= 1 ? 200
+      : Math.min(500 * Math.pow(2, this.reconnectAttempts - 2), 30000);
+    // Feeds the "Retrying in Ns" countdown. With a 30s cap on the backoff, a
+    // silent wait that long is indistinguishable from a hung app.
+    this._nextSseRetryAt = Date.now() + delay;
+    this._updateConnectionLossUi();
+    this.sseReconnectTimeout = setTimeout(() => this.connectSSE(), delay);
+  }
+
   /**
    * Force a reconnect if the SSE stream has gone quiet while still claiming to
    * be connected. Called by the 5s watchdog and on tab-visible.

src/web/public/constants.js

@@ -828,6 +828,15 @@ function formatKeepWarmStatus(status, now, statusLine) {
 // Pure: no DOM, no timers, no side effects. `now` is passed in.
 const SSE_STALE_TIMEOUT_MS = 45000; // three missed 15s heartbeats
 
+// The watchdog above cannot see a connect attempt that never opens: an
+// EventSource whose request gets no response headers (a resumed phone on a
+// VPN, a proxy that accepted the socket and went silent) stays CONNECTING
+// without firing `open` or `error`, and its status is never 'connected'.
+// connectSSE() bounds each attempt by this much and then retries it through
+// the normal backoff. Generous next to a healthy open (well under a second)
+// so a slow link is not cut off mid-handshake.
+const SSE_CONNECT_TIMEOUT_MS = 15000;
+
 function computeSseStale(input) {
   const {
     lastMessageAt = null,
@@ -1545,6 +1554,7 @@ if (typeof window !== 'undefined') {
   window.CodemanSseStale = {
     compute: computeSseStale,
     TIMEOUT_MS: SSE_STALE_TIMEOUT_MS,
+    CONNECT_TIMEOUT_MS: SSE_CONNECT_TIMEOUT_MS,
   };
   window.CodemanPromptCache = {
     format: formatPromptCacheStatus,