From 536eca8f3d3f7d20025414c54ebcf2df6466c5fd Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Thu, 22 Oct 2026 13:28:43 -0400 Subject: [PATCH 1/3] fix(sse): retry an EventSource connect attempt that never opens An EventSource whose request never gets response headers stays in CONNECTING without firing open or error, so the backoff never runs and the staleness watchdog (which only judges a 'connected' stream) cannot see it. Bound each attempt with a 15s connect timeout: if open has not fired, close the attempt and retry through the same path onerror uses. The timer is cleared on open, on failure and at the top of every connectSSE(), and both the timeout and the failure handler ignore an EventSource that has since been replaced. --- src/web/public/app.js | 85 ++++++++---- src/web/public/constants.js | 10 ++ test/sse-connect-timeout.test.ts | 224 +++++++++++++++++++++++++++++++ 3 files changed, 293 insertions(+), 26 deletions(-) create mode 100644 test/sse-connect-timeout.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index 62716497..0471bb4d 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -988,6 +988,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 @@ -2057,6 +2063,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'); @@ -2100,7 +2110,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. // @@ -2129,6 +2153,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 @@ -2136,31 +2161,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) @@ -2323,6 +2324,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. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index cd44962f..abb8381e 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -783,6 +783,15 @@ function formatPromptCacheStatus(cache, 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, @@ -1500,6 +1509,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, diff --git a/test/sse-connect-timeout.test.ts b/test/sse-connect-timeout.test.ts new file mode 100644 index 00000000..941292c7 --- /dev/null +++ b/test/sse-connect-timeout.test.ts @@ -0,0 +1,224 @@ +/** + * @fileoverview SSE connect timeout: an EventSource whose first request never + * gets response headers sits in CONNECTING forever without firing `open` or + * `error` (a resumed phone on a VPN, a proxy that accepted the TCP connection + * and went silent). The staleness watchdog (test/sse-staleness.test.ts) cannot + * see it, because it only judges a stream that is already 'connected'. So + * `connectSSE()` bounds every attempt: if `open` has not fired within + * `_sseConnectTimeoutMs`, the attempt is closed and retried through the same + * backoff path `onerror` uses. + * + * Real code: constants.js + app.js in a `vm` context with a fake EventSource + * and vitest fake timers. Port: N/A. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +class FakeEventSource { + static CONNECTING = 0; + static OPEN = 1; + static CLOSED = 2; + static all: FakeEventSource[] = []; + readyState = FakeEventSource.CONNECTING; + closed = false; + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + constructor(public url: string) { + FakeEventSource.all.push(this); + } + addEventListener() {} + removeEventListener() {} + close() { + this.closed = true; + this.readyState = FakeEventSource.CLOSED; + } + open() { + this.readyState = FakeEventSource.OPEN; + this.onopen?.(); + } + fail() { + this.onerror?.(); + } +} + +const navigatorStub = { onLine: true }; +const windowStub: Record = { addEventListener: vi.fn(), removeEventListener: vi.fn() }; +const read = (f: string) => readFileSync(resolve(import.meta.dirname, `../src/web/public/${f}`), 'utf8'); +const context = vm.createContext({ + console: { ...console, log: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn() }, + performance: { now: () => 0 }, + setInterval: () => 0, + clearInterval: () => {}, + // Resolved at call time so vitest's fake timers (installed per test) apply. + setTimeout: (fn: () => void, ms?: number) => globalThis.setTimeout(fn, ms), + clearTimeout: (id: ReturnType) => globalThis.clearTimeout(id), + HTMLCanvasElement: class {}, + EventSource: FakeEventSource, + URLSearchParams, + navigator: navigatorStub, + location: { protocol: 'http:', host: 'codeman.test', pathname: '/', search: '', hash: '' }, + document: { + addEventListener: () => {}, + removeEventListener: () => {}, + documentElement: { dataset: {} }, + getElementById: () => null, + querySelector: () => null, + querySelectorAll: () => [], + }, + localStorage: { getItem: () => null, setItem: () => {}, removeItem: () => {} }, + window: windowStub, + MobileDetection: { isTouchDevice: () => false, isHandheldDevice: () => false, getDeviceType: () => 'desktop' }, +}); +vm.runInContext(`${read('constants.js')}\n${read('app.js')}\nglobalThis.__CodemanApp = CodemanApp;`, context); +const CodemanApp = (context as unknown as { __CodemanApp: { prototype: object } }).__CodemanApp; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type App = Record; + +function makeApp(): App { + const app = Object.create(CodemanApp.prototype) as App; + app._clientId = 'client-1'; + app.activeSessionId = null; + app.reconnectAttempts = 0; + app.maxReconnectAttempts = 10; + app.sseReconnectTimeout = null; + app._sseListenerCleanup = null; + app._sseStaleWatchdog = null; + app._sseLastMessageAt = 0; + app._nextSseRetryAt = null; + app.eventSource = null; + // What the constructor seeds (the policy constant lives in constants.js). + app._sseConnectTimeout = null; + app._sseConnectTimeoutMs = (windowStub.CodemanSseStale as { CONNECT_TIMEOUT_MS?: number }).CONNECT_TIMEOUT_MS; + app.statuses = [] as string[]; + app.setConnectionStatus = vi.fn((s: string) => app.statuses.push(s)); + app._updateConnectionLossUi = vi.fn(); + return app; +} + +const TIMEOUT = 15000; + +describe('SSE connect timeout', () => { + beforeEach(() => { + vi.useFakeTimers(); + FakeEventSource.all = []; + navigatorStub.onLine = true; + }); + afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + it('defaults to 15s', () => { + expect((windowStub.CodemanSseStale as { CONNECT_TIMEOUT_MS?: number }).CONNECT_TIMEOUT_MS).toBe(TIMEOUT); + const app = makeApp(); + app.connectSSE(); + vi.advanceTimersByTime(TIMEOUT - 1); + expect(FakeEventSource.all[0].closed).toBe(false); + vi.advanceTimersByTime(1); + expect(FakeEventSource.all[0].closed).toBe(true); + }); + + it('closes an attempt that never opens and retries it through the normal backoff', () => { + const app = makeApp(); + app.connectSSE(); + const first = FakeEventSource.all[0]; + vi.advanceTimersByTime(TIMEOUT); + + expect(first.closed).toBe(true); + expect(app.eventSource).toBe(null); + expect(app.reconnectAttempts).toBe(1); + expect(app.statuses.at(-1)).toBe('reconnecting'); + // Feeds the connection-loss UI countdown, same as onerror. + expect(app._nextSseRetryAt).toBeGreaterThan(0); + expect(app._updateConnectionLossUi).toHaveBeenCalled(); + + // First retry uses the fast 200ms backoff step. + vi.advanceTimersByTime(199); + expect(FakeEventSource.all).toHaveLength(1); + vi.advanceTimersByTime(1); + expect(FakeEventSource.all).toHaveLength(2); + expect(app.eventSource).toBe(FakeEventSource.all[1]); + }); + + it('keeps retrying each hung attempt, with backoff growing', () => { + const app = makeApp(); + app.connectSSE(); + vi.advanceTimersByTime(TIMEOUT + 200); // attempt 1 times out, attempt 2 starts + vi.advanceTimersByTime(TIMEOUT + 500); // attempt 2 times out, attempt 3 starts + expect(FakeEventSource.all).toHaveLength(3); + expect(app.reconnectAttempts).toBe(2); + expect(FakeEventSource.all.slice(0, 2).every((es) => es.closed)).toBe(true); + }); + + it('an attempt that opens is never timed out', () => { + const app = makeApp(); + app.connectSSE(); + vi.advanceTimersByTime(1000); + FakeEventSource.all[0].open(); + vi.advanceTimersByTime(TIMEOUT * 4); + expect(FakeEventSource.all).toHaveLength(1); + expect(FakeEventSource.all[0].closed).toBe(false); + expect(app.reconnectAttempts).toBe(0); + }); + + it('an attempt that errors is retried once, not again by its timeout', () => { + const app = makeApp(); + app.connectSSE(); + FakeEventSource.all[0].fail(); + expect(app.reconnectAttempts).toBe(1); + vi.advanceTimersByTime(200); + expect(FakeEventSource.all).toHaveLength(2); + FakeEventSource.all[1].open(); + vi.advanceTimersByTime(TIMEOUT * 4); + expect(FakeEventSource.all).toHaveLength(2); + expect(FakeEventSource.all[1].closed).toBe(false); + expect(app.reconnectAttempts).toBe(0); + }); + + it('a reconnect replaces the pending timer instead of stacking a second one', () => { + const app = makeApp(); + app.connectSSE(); + vi.advanceTimersByTime(10000); + app.connectSSE(); // e.g. the user pressed Retry + expect(FakeEventSource.all).toHaveLength(2); + // The first attempt's deadline passes: nothing may happen to the second. + vi.advanceTimersByTime(TIMEOUT - 10000); + expect(FakeEventSource.all[1].closed).toBe(false); + expect(app.reconnectAttempts).toBe(0); + // The second attempt's own deadline still applies. + vi.advanceTimersByTime(10000); + expect(FakeEventSource.all[1].closed).toBe(true); + expect(app.reconnectAttempts).toBe(1); + }); + + it('going offline cancels the pending timer (the online event reconnects)', () => { + const app = makeApp(); + app.connectSSE(); + navigatorStub.onLine = false; + app.connectSSE(); + expect(app.statuses.at(-1)).toBe('offline'); + vi.advanceTimersByTime(TIMEOUT * 4); + expect(app.reconnectAttempts).toBe(0); + expect(app.statuses.at(-1)).toBe('offline'); + }); + + it('does not retry a connection that opened but whose open callback has not run yet', () => { + const app = makeApp(); + app.connectSSE(); + FakeEventSource.all[0].readyState = FakeEventSource.OPEN; + vi.advanceTimersByTime(TIMEOUT); + expect(FakeEventSource.all[0].closed).toBe(false); + expect(app.reconnectAttempts).toBe(0); + }); + + it('honours a shrunk _sseConnectTimeoutMs (what a browser test sets)', () => { + const app = makeApp(); + app._sseConnectTimeoutMs = 500; + app.connectSSE(); + vi.advanceTimersByTime(500); + expect(FakeEventSource.all[0].closed).toBe(true); + }); +}); From f4f9001a4bee66794bbe67bec63c76a71588e4f9 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 13:59:17 +0200 Subject: [PATCH 2/3] fix: apply the review items for #678 at merge docs: document the SSE connect timeout next to the staleness watchdog in src/web/CLAUDE.md and docs/architecture-invariants.md (clear order, replaced-EventSource guards, offline handler leaves the timer) test: rename the offline case to "connectSSE() while offline cancels the pending timer" so it names the path it drives Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/architecture-invariants.md | 2 ++ src/web/CLAUDE.md | 2 +- test/sse-connect-timeout.test.ts | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f446f4a1..bcdaaad8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1131,6 +1131,8 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u ⚠️ The watchdog interval is cleared at the top of `connectSSE()` and nowhere else (its only teardown path); clearing it elsewhere stacks intervals. Recovery needs no new sync path: the reconnect re-runs `handleInit` → `_resetAllAppState()`. The forced reconnect logs one diagnostic line, because a middlebox that strips heartbeats presents as "silently reconnects every 45s". +⚠️ **The connect attempt has its own deadline, because the watchdog cannot see it.** An `EventSource` whose request never gets response headers (a phone resumed on a VPN or tailnet, a proxy that accepted the socket and went silent) stays `CONNECTING` without firing `open` or `error`, so `onerror` never schedules a retry and the status is never `connected` for `computeSseStale()` to judge: the page sat on "connecting" until a reload. `connectSSE()` therefore bounds each attempt by `SSE_CONNECT_TIMEOUT_MS` (15s, exported as `CodemanSseStale.CONNECT_TIMEOUT_MS` in constants.js; read once into `_sseConnectTimeoutMs`) and, if `open` has not fired, retries through `_handleSseConnectionFailure()`, the former `onerror` body, so a hung attempt gets the same backoff, status change and "Retrying in Ns" countdown as an errored one. ⚠️ The timer is cleared in exactly three places: at the top of `connectSSE()` **before** the offline early return (every path into `connectSSE()` supersedes the previous attempt; removing that clear fails the offline case in the test), on `open`, and in `_handleSseConnectionFailure()`. ⚠️ Both the timeout callback and `_handleSseConnectionFailure()` return early for an `EventSource` that has since been replaced, so a late error and the timeout never count one failure twice, and the timeout also skips a connection whose `readyState` is already `OPEN` before its `open` callback ran. ⚠️ The window `offline` handler deliberately leaves the timer running, since `navigator.onLine` can report offline on a working network; only a `connectSSE()` made while offline cancels it. Tests: `test/sse-connect-timeout.test.ts` (the real constants.js + app.js in a `vm` context with a fake `EventSource` and fake timers). + ### PTY and browser terminal geometry **The PTY and the browser terminal must never disagree about size** (issue #464, `syncTerminalGeometry()` in terminal-ui.js): Claude Code's TUI wraps its frame at the width the PTY reported and erases the previous frame by walking the cursor up the number of rows it BELIEVES that frame occupied. A browser terminal of a different width makes each logical line take more physical rows than Ink counted, so `eraseLines(n)` clears too few and the new frame paints over rows nothing erased — the doubled lines and half-overwritten prose in #464, **measured** against this repo's xterm (a 120-column PTY against a 62-column terminal renders every wrapped line twice; `test/terminal-pty-geometry.test.ts` pins it, and pins the clean render at matching widths so the assertion cannot pass against code that fixes nothing). ⚠️ **`fitAddon.fit()` is NOT the way to resize this terminal.** It resizes xterm to `proposeDimensions()` RAW while every server-facing path reports those floored at 40x10, so whenever the floor bit the two diverged silently — **measured in Chrome at 430px**: font size 44 proposed 13 columns, the server was told 40, and xterm stayed at 13. `syncTerminalGeometry()` fits, floors and applies as one step and is the ONE function that may change the size; `test/terminal-pty-geometry.test.ts` sweeps the seven modules that touch the main terminal (terminal-ui, mobile-handlers, app, ralph-panel, settings-ui, tab-rail-resize, notification-manager) for a bare fit; the split pane and teammate terminals own their own sizing and are out of scope. ⚠️ **Withhold the fit wherever you withhold the SIGWINCH.** `throttledResize` (virtual keyboard up) and `sendResize` (session detached into its own window) used to reflow locally and skip only the server write, which is the one combination that cannot be right. ⚠️ **A font change is a geometry change**: `setFontSize`/`setFontFamily`/`setFontWeight` move the cell size and told the server nothing, so raising the font on a phone left the CLI wrapping at the old column count (`_refitAfterCellSizeChange`). ⚠️ **Resize is no longer write-only.** `Session.resize` DECLINES a small-viewport request while a desktop connection holds an active sizing claim and says nothing, so both transports now answer with `Session.ptyGeometry` (`{"t":"zc"}` on the socket, the body of the resize POST) and `_onPtyGeometryReport` adopts it — a terminal that keeps a WIDTH the PTY refused renders GARBLED, not merely wrong-sized. ⚠️ While that refusal stands (`_paneWidthRefused`), a resize ASKS for the container's width without applying it (`_geometryForResizeRequest`: rows follow the container, columns stay at the PTY's): fitting first re-wrapped the whole buffer to the container and back on every 30s mobile retry, and ran the scrollback clear for a resize that brings no redraw. `selectSession` clears the flag, since it belongs to the previous pane. ⚠️ **`ptyGeometry` is null without a live pane**, never the field values: they are seeded at spawn (`_notePtySpawnGeometry`, so a reattached pane reports its tmux window's real size) and moved by `resize()`, but a dead-pane session still holds the constructor defaults of 120x40 or a gone pane's size — reporting those made a client adopt a size no process was ever told and claim another device owned the pane when none existed. ⚠️ **COLUMNS ONLY.** Adopting the PTY's ROWS was a regression: a phone taking a desktop's 43 rows into a viewport with room for 18 painted an `.xterm-screen` far taller than its container, xterm's own viewport then had nothing to scroll, and the CLI's input line sat below the container with no gesture able to reach it — output visible, typing invisible, for as long as the claim stayed hot. Width is the axis the wrap arithmetic depends on; rows only decide how much is on screen, and keeping the local count keeps the composer at the bottom of a viewport that scrolls. ⚠️ **`.term-overflows-x` keys on what does not FIT, not on a PTY mismatch**, because the 40-column floor widens the terminal past a narrow container with the PTY agreeing throughout (measured at 360px: font 18 paints 433px, font 24 paints 578px — 38% unreachable, and `increaseFontSize` reaches 24 in two taps). `_syncTerminalOverflowAffordance()` MEASURES `.xterm-screen` against the container on the next frame rather than deriving it from cell arithmetic. ⚠️ That rule sets **both** overflow axes — mobile.css loads later with `.terminal-container { overflow: visible }`, and a bare `overflow-x` would leave overflow-y computing to `auto`, handing the browser a vertical scroll container the terminal's touch handler does not know about — and sets **no `touch-action`**: the terminal's own touchmove handler pans it (`canPanHorizontally`), because `touchstart` preventDefault()s every 'content' tap and that cancels a native `pan-x` before it starts (measured: a 140px swipe reached scrollLeft 141 without that preventDefault and 0 with it). Removing the class returns `scrollLeft` to 0, so a resolved mismatch cannot leave the pane parked off-screen. diff --git a/src/web/CLAUDE.md b/src/web/CLAUDE.md index f2cca22b..51ac21b0 100644 --- a/src/web/CLAUDE.md +++ b/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) diff --git a/test/sse-connect-timeout.test.ts b/test/sse-connect-timeout.test.ts index 941292c7..0f6d3232 100644 --- a/test/sse-connect-timeout.test.ts +++ b/test/sse-connect-timeout.test.ts @@ -194,7 +194,7 @@ describe('SSE connect timeout', () => { expect(app.reconnectAttempts).toBe(1); }); - it('going offline cancels the pending timer (the online event reconnects)', () => { + it('connectSSE() while offline cancels the pending timer (the online event reconnects)', () => { const app = makeApp(); app.connectSSE(); navigatorStub.onLine = false; From 5222239966d697727aba17469e3bc6ea22bd1200 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 24 Oct 2026 14:36:42 +0200 Subject: [PATCH 3/3] fix: address the release-prep review of #678 test/tile-grid-sse-filter.test.ts: search for the renamed 'const eventSource = new EventSource(' end marker and require both ends, so the connect-URL check covers that block again instead of the rest of app.js test/sse-connect-timeout.test.ts: cover the replaced-EventSource guard in _handleSseConnectionFailure() (a late error from a replaced attempt leaves the new one alone) Co-Authored-By: Claude Opus 5.5 (1M context) --- test/sse-connect-timeout.test.ts | 20 ++++++++++++++++++++ test/tile-grid-sse-filter.test.ts | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/test/sse-connect-timeout.test.ts b/test/sse-connect-timeout.test.ts index 0f6d3232..b379c2c8 100644 --- a/test/sse-connect-timeout.test.ts +++ b/test/sse-connect-timeout.test.ts @@ -194,6 +194,26 @@ describe('SSE connect timeout', () => { expect(app.reconnectAttempts).toBe(1); }); + it('a late error from a replaced EventSource leaves the connection that replaced it alone', () => { + const app = makeApp(); + app.connectSSE(); + const first = FakeEventSource.all[0]; + app.connectSSE(); // e.g. the user pressed Retry + const second = FakeEventSource.all[1]; + // A browser that still fires onerror on the closed first attempt. + first.fail(); + expect(app.eventSource).toBe(second); + expect(second.closed).toBe(false); + expect(app.reconnectAttempts).toBe(0); + // No retry was scheduled, and the second attempt keeps its own deadline. + vi.advanceTimersByTime(TIMEOUT - 1); + expect(FakeEventSource.all).toHaveLength(2); + expect(second.closed).toBe(false); + vi.advanceTimersByTime(1); + expect(second.closed).toBe(true); + expect(app.reconnectAttempts).toBe(1); + }); + it('connectSSE() while offline cancels the pending timer (the online event reconnects)', () => { const app = makeApp(); app.connectSSE(); diff --git a/test/tile-grid-sse-filter.test.ts b/test/tile-grid-sse-filter.test.ts index e444f3ee..b733a768 100644 --- a/test/tile-grid-sse-filter.test.ts +++ b/test/tile-grid-sse-filter.test.ts @@ -80,7 +80,11 @@ describe('the SSE filter while tiles own the terminal', () => { const app = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8'); const start = app.indexOf('const _sseParams = new URLSearchParams('); expect(start).toBeGreaterThan(-1); - const block = app.slice(start, app.indexOf('this.eventSource = new EventSource(', start)); + // Both ends must be found: a missing end marker makes slice() take the + // rest of app.js, and the checks below would then pass on any text. + const end = app.indexOf('const eventSource = new EventSource(', start); + expect(end).toBeGreaterThan(start); + const block = app.slice(start, end); expect(block).toContain('this._sseFilterSessionId(this.activeSessionId)'); expect(block).not.toMatch(/set\('sessions', this\.activeSessionId\)/); });