diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index cbfdf3f2..7c807a81 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -780,7 +780,7 @@ Further detail: with many sessions the horizontal strip stops being scannable, w ### Split-pane sessions -**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is a new, independent `SplitTerminalPane` (terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket. ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `SplitTerminalPane` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without an explicit `this._splitPane?.fit()` call there Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket leaves Pane B visibly dead (a message written into its own xterm buffer) rather than silently swallowing keystrokes with nothing on screen to explain why — there is no reconnect logic for v1, matching the "deliberately plainer than Pane A" design. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattended agent loop. Shift/Ctrl+Enter POSTs to `/api/sessions/:id/send-key` (`{key:'S-Enter'|'C-Enter'}`, tmux `send-keys -H` for a real 0x0a) targeting THIS pane's own `sessionId` rather than the primary pane's `activeSessionId` — without it xterm's plain `\r` would submit an incomplete prompt instead of adding a line to it. Smart-copy Ctrl+C/Ctrl+Shift+C is re-implemented against `this.terminal` (Pane B's own) rather than reusing `app.copyTerminalSelection()`, which reads Pane A's terminal and would copy the wrong pane's selection; Ctrl+Shift+C never falls through even with nothing to copy, mirroring terminal-ui.js's own `ev.shiftKey` branch. ⚠️ **This is a UX-parity fix, not an interrupt-safety one** — verified live in a real browser: xterm's `evaluateKeyboardEvent` routes a shifted ctrl-letter into a branch that assigns `c.key` only for two special cases (`_`→US, `@`→NUL), so it emits no data for Ctrl+Shift+C at all regardless of any application gate; a synthetic keydown with the gate removed produces zero WS frames, proving no accidental interrupt reaches the PTY either way. What gating the whole copy block on `hasSelection()` (an earlier draft) actually cost: with no selection, a selection-less Ctrl+Shift+C fell straight to `return true`, silently ceding the keystroke to the BROWSER's own handling (e.g. Chrome's Inspect-Element binding) with no feedback and no copy attempt — Pane A always intercepts it. Ctrl+V stays on xterm's own default paste, since Pane B has no image-paste trap to route it to. ⚠️ `buildSplitPickerSessions()` also excludes any session with `pid === null` (an exited CLI, a crash-looped session whose breaker tripped, a restore that never re-attached): Pane B has no equivalent of `selectSession()`'s auto re-attach POST, so a pane opened onto one has nothing reading its tmux pane — no `terminal` events ever arrive, and `Session.write()` silently drops every keystroke with no ack either way, so the loss is invisible behind a socket that reports healthy. Pane B's input frames deliberately carry no `cid`/`seq` (`ws-routes.ts` supports that), matching the no-overlay/no-IME "deliberately plainer" list above, since it has no exactly-once delivery layer to key them against. ⚠️ A SHELL Pane B pulls scrollback itself when the wheel goes up at the top of its buffer (`_maybeLoadMoreHistory`/`_pullHistory`): tmux repaints a burst of output instead of scrolling it, so Pane B's own xterm holds about one screen of scrollback while tmux holds every line, and it loaded history exactly once at connect and never again. It is the same bounded pull as Pane A's (`?full=1&tail=TERMINAL_TAIL_SIZE`, no rewrite when the window holds no more rows than the pane already has or the pane is at its `scrollback + rows` cap, and a 60 s back-off instead of 4 s when that skipped window was truncated or the pane is full, since each ask costs the server a whole-history `capture-pane`), against Pane B's OWN terminal rather than `app.terminal`, so it cannot share `_maybeRefetchFullHistory`. The wheel listener is capture-phase because xterm `stopPropagation()`s the events it consumes; the alternate-screen skip (nano, vim, less) only matters for a direct-PTY shell, since under tmux the browser xterm never enters the alternate buffer; skipped too for a detached session (mirrors `_sendResize()`'s own check and app.js's `_maybeRefetchFullHistory`), since its own window already owns its PTY size and scrollback. Live frames arriving mid-replay, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`) and replayed in order only if they arrived after the capture (the response's arrival stands in for the capture instant, as in `_finishBufferLoad`, so a frame inside that one round trip can be lost or doubled); the fetch has a 10 s deadline because it holds the pane's live output while it runs. ⚠️ The "Pane B disconnected" marker must be the LAST thing on screen. A replay's own `\x1bc` would otherwise wipe a marker written before the pull and paint a fresh, current-looking history while `onData` keeps silently dropping every keystroke on the dead socket (a Codeman restart drops the socket while the tmux session, and so the HTTP pull, still succeeds), so `_pullHistory()` re-stamps it after the live-frame flush. A close DURING a pull writes nothing: `_onSocketClosed()` skips the marker while `_liveQueue` is live, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, or land mid-way through a chunked replay; the pull's `finally` then writes it once, replayed or not (`closedBefore` tells the two closes apart). Both are tracked via `_wsClosed` rather than routed through `_onLiveOutput()`, since a close landing before the response is stamped before the cutoff and would be dropped with the rest of the pre-capture queue. There is no "Load full history" banner in Pane B, so a shell history past that 1 MiB window stays out of reach there. Non-shell Pane B is unchanged: it already loads `full=1`, and its history is out of scope for this pull (codex and Claude's inline renderer do grow tmux history; this just isn't how they recover it). Design: `docs/split-pane-sessions-plan.md`. +**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is a new, independent `SplitTerminalPane` (terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket. ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `SplitTerminalPane` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without an explicit `this._splitPane?.fit()` call there Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket leaves Pane B visibly dead (a message written into its own xterm buffer) rather than silently swallowing keystrokes with nothing on screen to explain why — there is no reconnect logic for v1, matching the "deliberately plainer than Pane A" design. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattended agent loop. Shift/Ctrl+Enter POSTs to `/api/sessions/:id/send-key` (`{key:'S-Enter'|'C-Enter'}`, tmux `send-keys -H` for a real 0x0a) targeting THIS pane's own `sessionId` rather than the primary pane's `activeSessionId` — without it xterm's plain `\r` would submit an incomplete prompt instead of adding a line to it. Smart-copy Ctrl+C/Ctrl+Shift+C is re-implemented against `this.terminal` (Pane B's own) rather than reusing `app.copyTerminalSelection()`, which reads Pane A's terminal and would copy the wrong pane's selection; Ctrl+Shift+C never falls through even with nothing to copy, mirroring terminal-ui.js's own `ev.shiftKey` branch. ⚠️ **This is a UX-parity fix, not an interrupt-safety one** — verified live in a real browser: xterm's `evaluateKeyboardEvent` routes a shifted ctrl-letter into a branch that assigns `c.key` only for two special cases (`_`→US, `@`→NUL), so it emits no data for Ctrl+Shift+C at all regardless of any application gate; a synthetic keydown with the gate removed produces zero WS frames, proving no accidental interrupt reaches the PTY either way. What gating the whole copy block on `hasSelection()` (an earlier draft) actually cost: with no selection, a selection-less Ctrl+Shift+C fell straight to `return true`, silently ceding the keystroke to the BROWSER's own handling (e.g. Chrome's Inspect-Element binding) with no feedback and no copy attempt — Pane A always intercepts it. Ctrl+V stays on xterm's own default paste, since Pane B has no image-paste trap to route it to. ⚠️ `buildSplitPickerSessions()` also excludes any session with `pid === null` (an exited CLI, a crash-looped session whose breaker tripped, a restore that never re-attached): Pane B has no equivalent of `selectSession()`'s auto re-attach POST, so a pane opened onto one has nothing reading its tmux pane — no `terminal` events ever arrive, and `Session.write()` silently drops every keystroke with no ack either way, so the loss is invisible behind a socket that reports healthy. Pane B's input frames deliberately carry no `cid`/`seq` (`ws-routes.ts` supports that), matching the no-overlay/no-IME "deliberately plainer" list above, since it has no exactly-once delivery layer to key them against. ⚠️ A SHELL Pane B pulls scrollback itself when the wheel goes up at the top of its buffer (`_maybeLoadMoreHistory`/`_pullHistory`): tmux repaints a burst of output instead of scrolling it, so Pane B's own xterm holds about one screen of scrollback while tmux holds every line, and it loaded history exactly once at connect and never again. It is the same bounded pull as Pane A's (`?full=1&tail=TERMINAL_TAIL_SIZE`, no rewrite when the window holds no more rows than the pane already has or the pane is at its `scrollback + rows` cap, and a 60 s back-off instead of 4 s when that skipped window was truncated or the pane is full, since each ask costs the server a whole-history `capture-pane`), against Pane B's OWN terminal rather than `app.terminal`, so it cannot share `_maybeRefetchFullHistory`. The wheel listener is capture-phase because xterm `stopPropagation()`s the events it consumes; the alternate-screen skip (nano, vim, less) only matters for a direct-PTY shell, since under tmux the browser xterm never enters the alternate buffer; skipped too for a detached session (mirrors `_sendResize()`'s own check and app.js's `_maybeRefetchFullHistory`), since its own window already owns its PTY size and scrollback. Live frames arriving from the response onward, a `{t:'c'}` clear frame included, are held with their arrival time (`_liveQueue`, opened right after `await fetch(...)` beside `capturedAt`; a frame from before it is replaced by the capture or written unchanged, so the pane keeps painting during the round trip) and replayed in order only if they arrived after the capture (the response's arrival stands in for the capture instant, as in `_finishBufferLoad`, so a frame inside that one round trip can be lost or doubled); the request uses the primary pane's budget (`CodemanFetchDeadline.terminalFetchDeadlineMs({ full: true })`, 10 s only if that helper is absent) and the body read gets 10 s once the headers land, because from then on the pull holds the pane's live output. ⚠️ The "Pane B disconnected" marker must be the LAST thing on screen. A replay's own `\x1bc` would otherwise wipe a marker written before the pull and paint a fresh, current-looking history while `onData` keeps silently dropping every keystroke on the dead socket (a Codeman restart drops the socket while the tmux session, and so the HTTP pull, still succeeds), so `_pullHistory()` re-stamps it after the live-frame flush. A close DURING any load (a pull, a refresh, the initial load) writes nothing: `_onSocketClosed()` sets `_markerOwed` while `_bufferLoading` is true, since written there it would sit above the held frames the pull flushes after a skip, a downgrade, a failed fetch or the deadline, above a refresh's replay, or land mid-way through a chunked replay. Each load settles the marker in its OWN `finally` (`_stampMarkerIfOwed()`), after the queue flush and before a trailing refresh starts, so a nested refresh stamps its own and never an earlier one's onto its freshly cleared terminal. Anything that wipes the terminal on a closed socket (a replay's `\x1bc`, a refresh's `clear()`) sets `_markerOwed` too, so the marker is rewritten whether or not the close landed during the load. Tracked via `_wsClosed`/`_markerOwed` rather than routed through `_onLiveOutput()`, since a close landing before the response is stamped before the cutoff and would be dropped with the rest of the pre-capture queue. There is no "Load full history" banner in Pane B, so a shell history past that 1 MiB window stays out of reach there. Non-shell Pane B is unchanged: it already loads `full=1`, and its history is out of scope for this pull (codex and Claude's inline renderer do grow tmux history; this just isn't how they recover it). Design: `docs/split-pane-sessions-plan.md`. ### Gesture control: the setting diff --git a/src/web/public/terminal-split.js b/src/web/public/terminal-split.js index f8d352c7..96e8f34f 100644 --- a/src/web/public/terminal-split.js +++ b/src/web/public/terminal-split.js @@ -77,11 +77,14 @@ this._bufferLoading = false; this._bufferRefreshPending = false; // Scroll-to-top history pull (shell panes only), see _maybeLoadMoreHistory(). - // `_liveQueue` is non-null exactly while a pull is replaying: live frames - // are held there with their arrival time instead of written under it. + // `_liveQueue` is non-null from the pull's response until its finally + // block: live frames are held there with their arrival time instead of + // written under the replay. `_markerOwed` is the "disconnected" marker a + // load still has to write (see _onSocketClosed()/_stampMarkerIfOwed()). this._historyPullAt = 0; this._historyPullUseless = false; this._liveQueue = null; + this._markerOwed = false; this._onWheel = null; } @@ -308,7 +311,18 @@ _onSocketClosed() { this._wsReady = false; this._wsClosed = true; - if (!this._liveQueue) this._writeDisconnectedMarker(); + if (this._bufferLoading) this._markerOwed = true; + else this._writeDisconnectedMarker(); + } + + // Settles a marker the pane owes: set when a close lands during a load (the + // replay would otherwise sit below it) or when a load wipes the terminal on + // a closed socket. Called from each load's own finally, before a trailing + // refresh starts, so a nested refresh stamps its own. + _stampMarkerIfOwed() { + const owed = this._markerOwed; + this._markerOwed = false; + if (owed && this._wsClosed && !this._destroyed) this._writeDisconnectedMarker(); } // Extracted so both _onSocketClosed() and a history pull that ends on a @@ -351,6 +365,7 @@ } catch { /* Best-effort — live output still arrives once the socket connects. */ } finally { + this._stampMarkerIfOwed(); this._endBufferLoad(); } } @@ -430,27 +445,41 @@ // main thread) and replays it under the reader's current place. Holds the // single-flight flag across the fetch AND the replay, like _loadBuffer(). async _pullHistory() { - // A close before the pull already wrote its marker; one during it did not. - const closedBefore = this._wsClosed; this._bufferLoading = true; - this._liveQueue = []; let replayed = false; let capturedAt = 0; + // Two budgets on one signal. The request itself gets the primary pane's + // (CodemanFetchDeadline, constants.js): nothing is held while it runs. Once + // the headers land live output IS held, so the body read gets the short one + // instead: a body that hangs would otherwise freeze the pane for the long + // budget. Aborting lands in the catch below, which releases the flag and + // the queue. AbortSignal.timeout() alone cannot be re-armed, hence the + // controller; without AbortController the pull simply has no deadline. + const controller = global.AbortController ? new global.AbortController() : null; + let abortTimer = null; + const armDeadline = (ms) => { + if (!controller) return; + clearTimeout(abortTimer); + abortTimer = setTimeout(() => controller.abort(), ms); + }; try { - // A deadline, because live output is held for as long as this runs: a - // request that hangs would otherwise freeze the whole pane. Aborting - // lands in the catch below, which releases the flag and the queue. It - // covers the body read too, not just the headers. + armDeadline(global.CodemanFetchDeadline?.terminalFetchDeadlineMs?.({ full: true }) ?? HISTORY_PULL_TIMEOUT_MS); const res = await fetch(`/api/sessions/${this.sessionId}/terminal?full=1&tail=${TERMINAL_TAIL_SIZE}`, { - signal: global.AbortSignal?.timeout?.(HISTORY_PULL_TIMEOUT_MS), + signal: controller?.signal, }); + armDeadline(HISTORY_PULL_TIMEOUT_MS); // The cutoff below is the response's arrival, the same `since` rule the // primary pane uses (_finishBufferLoad). It is a client clock standing in // for the instant tmux took the capture, which lies somewhere in the // round trip, so a frame in that window can be lost or doubled. Bounded // by one round trip and not closable without a server-side capture time. capturedAt = performance.now(); + // Opened only now: a frame from before the response is either replaced by + // the capture or written unchanged, so holding it for the round trip + // bought nothing and froze the pane for as long as the fetch took. + this._liveQueue = []; const payload = (await res.json())?.data; + clearTimeout(abortTimer); const buffer = payload?.terminalBuffer; const term = this.terminal; if (!buffer || !term || this._destroyed) return; @@ -476,6 +505,7 @@ this._historyPullUseless = false; term.write('\x1bc'); replayed = true; + if (this._wsClosed) this._markerOwed = true; await writeChunked(term, buffer, () => this._destroyed); if (this._destroyed || !this.terminal) return; // xterm parses asynchronously: an empty write's callback fires only @@ -490,6 +520,7 @@ } catch { /* Best-effort — live output keeps arriving whatever happens here. */ } finally { + clearTimeout(abortTimer); const queued = this._liveQueue ?? []; this._liveQueue = null; // After a replay, only frames that arrived after the capture are news; @@ -500,15 +531,13 @@ if (entry.clear) this.terminal?.clear(); else this.terminal?.write(entry.data); } - // A replay's own `\x1bc` wipes a marker written before the pull, - // painting a fresh, current-looking history while onData keeps - // silently dropping every keystroke on the dead socket, so re-stamp it - // after a replay. A close DURING the pull wrote no marker at all - // (_onSocketClosed() defers it while the queue is live), so write it - // whether or not this pull replayed. Checked after the queue flush so - // it is the last thing on screen, matching what the close would have - // left had the pull never run. - if (this._wsClosed && (replayed || !closedBefore)) this._writeDisconnectedMarker(); + // Settled after the queue flush so the marker is the last thing on + // screen: a close during the pull wrote nothing (_onSocketClosed() defers + // it while a load runs), and a replay's own `\x1bc` (flagged above) wipes + // one written before it, which would paint a fresh, current-looking + // history while onData keeps silently dropping every keystroke on the + // dead socket. A trailing refresh (_endBufferLoad) settles its own. + this._stampMarkerIfOwed(); this._endBufferLoad(); } } @@ -525,6 +554,10 @@ return; } this.terminal?.clear(); + // The clear wipes a "disconnected" marker (a `{t:'r'}` frame can queue a + // trailing refresh behind a pull that the socket's close then interrupts), + // so a refresh on a closed socket owes it back once its replay is written. + if (this._wsClosed) this._markerOwed = true; void this._loadBuffer(); } diff --git a/test/split-pane-terminal-unit.test.ts b/test/split-pane-terminal-unit.test.ts index 9f86c384..0beb6334 100644 --- a/test/split-pane-terminal-unit.test.ts +++ b/test/split-pane-terminal-unit.test.ts @@ -71,6 +71,8 @@ type PaneUnderTest = { const fetchMock = vi.fn(); /** requestAnimationFrame stand-in: chunked writes queue here and are drained by hand. */ const rafQueue: Array<() => void> = []; +/** Recorded deadline timers (see the context's setTimeout); `fn` aborts the request. */ +const deadlines: Array<{ fn: () => void; ms: number; cleared: boolean }> = []; const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-split.js'), 'utf8'); function loadSplitTerminalPane() { @@ -80,9 +82,27 @@ function loadSplitTerminalPane() { // compares it with the pane's own row count. window: { app: { _estimateReplayRows: (text: string) => text.split('\n').length }, - AbortSignal: { timeout: (ms: number) => ({ timeoutMs: ms }) }, + // The primary pane's capture budget (constants.js): the full-capture default. + CodemanFetchDeadline: { terminalFetchDeadlineMs: () => 45_000 }, + AbortController: class { + signal = { aborted: false }; + abort() { + this.signal.aborted = true; + } + }, }, performance: { now: () => clock }, + // Deadline timers (>= 1 s) are recorded, never run: a test fires one by hand + // and reads what it aborted. Anything shorter (xterm chunk pacing) is real. + setTimeout: (fn: () => void, ms?: number) => { + if ((ms ?? 0) < 1000) return setTimeout(fn, ms); + deadlines.push({ fn, ms: ms as number, cleared: false }); + return -deadlines.length; // negative: never collides with a real timer id + }, + clearTimeout: (id: unknown) => { + if (typeof id === 'number' && id < 0) deadlines[-id - 1].cleared = true; + else clearTimeout(id as Parameters[0]); + }, fetch: (...args: unknown[]) => fetchMock(...args), requestAnimationFrame: (fn: () => void) => rafQueue.push(fn), // The constants.js globals the module reads at call time. @@ -126,6 +146,27 @@ function jsonResponse(terminalBuffer: string, extra: Record = { return { json: async () => ({ data: { terminalBuffer, ...extra } }) }; } +/** + * A response whose headers have landed but whose body has not: the window in + * which the pull's live queue is open and nothing else has happened yet. + * `release(buffer)` delivers the body; `fail()` errors the body read. + */ +function headersOnly() { + let release!: (body: ReturnType | Error) => void; + const body = new Promise<{ data: Record }>((resolve, reject) => { + release = (value) => { + if (value instanceof Error) reject(value); + else void value.json().then(resolve); + }; + }); + return { + response: { json: () => body }, + release: (terminalBuffer: string, extra: Record = {}) => + release(jsonResponse(terminalBuffer, extra)), + fail: () => release(new Error('body read failed')), + }; +} + function deferred() { let resolve!: (value: T) => void; const promise = new Promise((r) => { @@ -142,6 +183,7 @@ const settle = () => new Promise((r) => setTimeout(r, 0)); beforeEach(() => { fetchMock.mockReset(); rafQueue.length = 0; + deadlines.length = 0; clock = 0; }); @@ -301,10 +343,9 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { pane._maybeLoadMoreHistory(); await settle(); - // With a deadline: live output is held for as long as the pull runs, so a - // request that never answers would freeze the pane. + // With a deadline, so a request that never answers cannot pin the pane. expect(fetchMock).toHaveBeenCalledWith(`/api/sessions/s1/terminal?full=1&tail=${TERMINAL_TAIL_SIZE}`, { - signal: { timeoutMs: 10_000 }, + signal: expect.objectContaining({ aborted: false }), }); expect(term.write).toHaveBeenCalledWith('\x1bc'); expect(term.write).toHaveBeenCalledWith(rowsOf(100)); @@ -463,12 +504,14 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { fetchMock.mockReturnValueOnce(response.promise); pane._maybeLoadMoreHistory(); - expect(pane._liveQueue).toEqual([]); + // The queue opens with the response, not the request. + expect(pane._liveQueue).toBeNull(); - // Arrives before the response does: it is IN the capture already. + // Arrives before the response does: written straight through (the pane + // keeps painting during the round trip), and the replay then replaces it. clock = 1; pane._onLiveOutput('early'); - expect(term.write).not.toHaveBeenCalledWith('early'); + expect(term.write).toHaveBeenCalledWith('early'); await settle(); // 200 rows (more than the pane holds, so it replays) of 400 columns each: @@ -490,7 +533,9 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { await settle(); const written = term.write.mock.calls.map((call) => call[0]); - expect(written).not.toContain('early'); + // 'early' went out before the reset, so the replay wiped it and it is not repeated. + expect(written.indexOf('early')).toBeLessThan(written.indexOf('\x1bc')); + expect(written.filter((w) => w === 'early')).toHaveLength(1); expect(written.at(-1)).toBe('late'); expect(pane._liveQueue).toBeNull(); expect(pane._bufferLoading).toBe(false); @@ -498,26 +543,29 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { it('writes every held frame when the pull ends without replaying', async () => { const pane = makePane('shell'); - const response = deferred>(); - fetchMock.mockReturnValueOnce(response.promise); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); pane._maybeLoadMoreHistory(); - pane._onLiveOutput('held'); await settle(); - response.resolve(jsonResponse(rowsOf(30))); // nothing to gain: no replay + pane._onLiveOutput('held'); + expect(pane.terminal.write).not.toHaveBeenCalledWith('held'); + held.release(rowsOf(30)); // nothing to gain: no replay await settle(); - // Nothing replaced the terminal, so the frame is news even though it - // arrived before the response did. + // Nothing replaced the terminal, so the held frame is news. expect(pane.terminal.write).toHaveBeenCalledWith('held'); }); it('a failed fetch releases the flag and the queue, so live output flows again', async () => { const pane = makePane('shell'); - fetchMock.mockRejectedValueOnce(new Error('offline')); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); pane._maybeLoadMoreHistory(); + await settle(); pane._onLiveOutput('held'); + held.fail(); // the body read dies with the queue open await settle(); expect(pane._bufferLoading).toBe(false); @@ -554,17 +602,18 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { done?.(); }); term.clear.mockImplementation(() => order.push('clear')); - const response = deferred>(); - fetchMock.mockReturnValueOnce(response.promise); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); pane._maybeLoadMoreHistory(); + await settle(); pane._onLiveOutput('before'); pane._onLiveClear(); pane._onLiveOutput('after'); // Held: clearing now would wipe a half-written snapshot. expect(order).toEqual([]); - response.resolve(jsonResponse(rowsOf(30))); // nothing to gain: no replay + held.release(rowsOf(30)); // nothing to gain: no replay await settle(); expect(order).toEqual(['write:before', 'clear', 'write:after']); @@ -583,25 +632,27 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { pane._maybeLoadMoreHistory(); clock = 1; - pane._onLiveClear(); // already reflected in the capture + pane._onLiveClear(); // before the response: applied now, already in the capture + expect(term.clear).toHaveBeenCalledTimes(1); clock = 2; response.resolve(jsonResponse(rowsOf(100))); await settle(); expect(term.write).toHaveBeenCalledWith('\x1bc'); - expect(term.clear).not.toHaveBeenCalled(); + expect(term.clear).toHaveBeenCalledTimes(1); // not replayed after the capture }); it('destroy() mid-pull leaves nothing running and nothing written to the dead terminal', async () => { const pane = makePane('shell'); const term = pane.terminal; - const response = deferred>(); - fetchMock.mockReturnValueOnce(response.promise); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); pane._maybeLoadMoreHistory(); + await settle(); pane._onLiveOutput('held'); pane.destroy(); - response.resolve(jsonResponse(rowsOf(100))); + held.release(rowsOf(100)); await settle(); expect(pane._bufferLoading).toBe(false); @@ -613,10 +664,13 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { it('a pull whose request is aborted (the deadline) frees the pane', async () => { const pane = makePane('shell'); - fetchMock.mockRejectedValueOnce(new Error('The operation timed out')); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); pane._maybeLoadMoreHistory(); + await settle(); pane._onLiveOutput('held'); + held.fail(); // the deadline aborts the body read await settle(); expect(pane._bufferLoading).toBe(false); @@ -698,18 +752,18 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { expect(pane.terminal.write).toHaveBeenCalledWith(marker); }); - it('writes the disconnected marker once, after the replay, if the socket closes mid-fetch', async () => { - // The close lands while the capture is in flight, so the HTTP pull still + it('ends on the disconnected marker if the socket closes mid-fetch, before the queue opens', async () => { + // The close lands while the request is in flight, so the HTTP pull still // succeeds (a Codeman restart drops the WS while the tmux session, and so - // the pull, survives) and the replay that follows is what the marker must - // end up below. + // the pull, survives). The marker waits for the load to finish, then goes + // below the replay. const pane = makePane('shell'); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); const pull = pane._pullHistory(); - pane._onSocketClosed(); // the close arrives mid-fetch, before the response - expect(pane.terminal.write).not.toHaveBeenCalled(); + pane._onSocketClosed(); + expect(pane.terminal.write.mock.calls.map((c) => c[0]).filter(isMarker)).toHaveLength(0); response.resolve(jsonResponse(rowsOf(100))); await pull; @@ -718,25 +772,101 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { expect(isMarker(writes.at(-1))).toBe(true); }); + it('a close mid-fetch that ends without a replay still writes exactly one marker', async () => { + const pane = makePane('shell'); + const response = deferred>(); + fetchMock.mockReturnValueOnce(response.promise); + + const pull = pane._pullHistory(); + pane._onSocketClosed(); + response.resolve(jsonResponse(rowsOf(30))); // already held in full: no replay + await pull; + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes.filter(isMarker)).toHaveLength(1); + }); + + it('a close during a refresh load lands the marker below the replay, not above it', async () => { + // A {t:'r'} refresh clears and refetches; a close mid-fetch used to write + // the marker at once, and the replay then landed underneath it. + const pane = makePane('shell'); + const response = deferred>(); + fetchMock.mockReturnValueOnce(response.promise); + + pane._refreshBuffer(); + pane._onSocketClosed(); + response.resolve(jsonResponse('refreshed')); + await settle(); + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes.indexOf('refreshed')).toBeLessThan(writes.findIndex(isMarker)); + expect(isMarker(writes.at(-1))).toBe(true); + expect(writes.filter(isMarker)).toHaveLength(1); + }); + + it('back-to-back refreshes on a closed socket leave exactly one marker, at the end', async () => { + // R1's finally runs the trailing refresh R2; each load settles its own + // marker, so R1 never stamps onto R2's freshly cleared terminal. + const pane = makePane('shell'); + pane._wsClosed = true; + const first = deferred>(); + const second = deferred>(); + fetchMock.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + + pane._refreshBuffer(); // R1 + pane._refreshBuffer(); // coalesced into the trailing R2 + first.resolve(jsonResponse('first')); + await settle(); + // R1 settled its marker, then R2 cleared and is still fetching. + expect(pane.terminal.clear).toHaveBeenCalledTimes(2); + second.resolve(jsonResponse('second')); + await settle(); + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes.at(-1)).toSatisfy(isMarker); + expect(writes.lastIndexOf('second')).toBe(writes.length - 2); + }); + + it('the pull gives the request the long budget and the body read the short one', async () => { + const pane = makePane('shell'); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); + + const pull = pane._pullHistory(); + expect(deadlines).toHaveLength(1); + expect(deadlines[0].ms).toBe(45_000); + await settle(); // headers landed + expect(deadlines).toHaveLength(2); + expect(deadlines[0].cleared).toBe(true); + expect(deadlines[1].ms).toBe(10_000); + + held.release(rowsOf(30)); + await pull; + expect(deadlines[1].cleared).toBe(true); // nothing left to abort a settled pull + }); + it.each([ - ['a skip', 40, () => fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(30)))], - ['a downgrade', 500, () => fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(5)))], - ['a failed fetch', 40, () => fetchMock.mockRejectedValueOnce(new Error('offline'))], + ['a skip', 40, (h: ReturnType) => h.release(rowsOf(30))], + ['a downgrade', 500, (h: ReturnType) => h.release(rowsOf(5))], + ['a failed body read', 40, (h: ReturnType) => h.fail()], ])( - 'a close mid-fetch that ends in %s writes the marker last, after the held frames', - async (_label, rowsHeld, mockFetch) => { + 'a close with the queue open that ends in %s writes the marker last, after the held frames', + async (_label, rowsHeld, finish) => { // No replay ever runs here, so nothing would wipe a marker written at the // close; written straight away it sat ABOVE the output the pull was still // holding, which the finally block then flushed underneath it. const pane = makePane('shell'); pane.terminal.buffer.active.length = rowsHeld; - mockFetch(); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response); const pull = pane._pullHistory(); + await settle(); // the response landed: the queue is open pane._onLiveOutput('frame-A'); pane._onLiveOutput('frame-B'); pane._onSocketClosed(); expect(pane.terminal.write).not.toHaveBeenCalled(); + finish(held); await pull; const writes = pane.terminal.write.mock.calls.map((c) => c[0]); @@ -771,6 +901,38 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { expect(isMarker(writes.at(-1))).toBe(true); }); + it('a refresh queued behind a pull on a closed socket does not wipe the marker', async () => { + // The refresh's clear() runs after the pull's finally block has written the + // marker, so without a re-stamp the dead pane would look current again. + const pane = makePane('shell'); + const held = headersOnly(); + fetchMock.mockResolvedValueOnce(held.response).mockResolvedValueOnce(jsonResponse('refreshed')); + + const pull = pane._pullHistory(); + await settle(); + pane._refreshBuffer(); // coalesced into one trailing re-run + pane._onSocketClosed(); // deferred: the queue is open + held.release(rowsOf(30)); // no replay + await pull; + await settle(); + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(pane.terminal.clear).toHaveBeenCalledTimes(1); + expect(writes).toContain('refreshed'); + expect(isMarker(writes.at(-1))).toBe(true); + expect(writes.lastIndexOf('refreshed')).toBeLessThan(writes.length - 1); + }); + + it('a refresh on an open socket does not stamp a marker', async () => { + const pane = makePane('shell'); + fetchMock.mockResolvedValueOnce(jsonResponse('refreshed')); + + pane._refreshBuffer(); + await settle(); + + expect(pane.terminal.write.mock.calls.map((c) => c[0]).filter(isMarker)).toHaveLength(0); + }); + it('does not re-stamp the marker when the socket is still open', async () => { const pane = makePane('shell'); fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(100)));