From 3af1ff6fae737e73bac2b848a3df9b9ad7b5d94a Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Thu, 1 Oct 2026 11:14:35 +0200 Subject: [PATCH] fix(split-pane): keep Pane B's disconnected marker last when the socket closes mid-pull (#506 review) - terminal-split.js: move the socket's close into _onSocketClosed(), which defers the marker while a history pull holds live output (_liveQueue); _pullHistory() records closedBefore and its finally writes the marker after the queue flush when the socket closed during the pull, replayed or not, so it never lands above held frames or between replay chunks - tests: drive the real close path for a close mid-fetch ending in a skip, a downgrade or a failed fetch, a close during the chunked replay, and a close with no pull running; pin the onclose wiring in the static guard; describe the mid-fetch case on its own - CLAUDE.md: turn the plain-text split-pane pointer into a link - architecture-invariants.md: describe the deferred marker Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/terminal-split.js | 36 +++++++----- test/split-pane-terminal-unit.test.ts | 84 +++++++++++++++++++++++++-- 4 files changed, 103 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2e5ffb1b..450bfcbe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -270,7 +270,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Circuit breakers**: the Ralph breaker prevents respawn thrashing (`CLOSED` → `HALF_OPEN` → `OPEN`; reset via `/api/sessions/:id/ralph-circuit-breaker/reset`). **Distinct: the PTY-exit breaker** (`session-pty-exit-breaker.ts`) trips after repeated rapid PTY exits and blocks auto-restarts. ⚠️ It resets ONLY via an explicit `{clearBreaker:true}` body on `POST /api/sessions/:id/interactive`; the frontend's auto-reattach in `selectSession()` sends no body and must never clear it. → [architecture-invariants#circuit-breakers-ralph--pty-exit](docs/architecture-invariants.md#circuit-breakers-ralph-and-pty-exit) -**Full-scrollback replay**: `GET /api/sessions/:id/terminal?full=1` returns the whole tmux scrollback ALONE (`source='mux-full-history'`), superseding the byte buffer. First load of each non-shell TUI session requests it (`_fullHistoryLoaded`); Shell selection and drop recovery use a bounded 1 MiB `?tail=`, and a Shell scroll-to-top pulls a bounded `?full=1&tail=` window (a window no longer than the browser's buffer is skipped before the downgrade guard, so it never marks the session exhausted); the unbounded pull stays behind **Load full history**. A Shell split-pane Pane B has its own copy of the bounded pull against its own xterm (`SplitTerminalPane._pullHistory`, terminal-split.js); keep the two in step. → invariants: "Split-pane sessions" ⚠️ The capture ends with a RELATIVE cursor move back to the pane's caret (never `CUP`), so no line-deleting transform may run over it; those skips key on `isFullCapture`, never on `?full=1` alone. ⚠️ A re-pull must never shrink the buffer (`_replayWouldShrinkBuffer()`). ⚠️ `captureCols`/`captureRows` are absent when no frame was positioned: test `Number.isFinite`, never truthiness. ⚠️ A frame dropped at the 128 KiB render cap MUST be recovered, and the recovery verifies itself: `_scheduleDroppedOutputRecovery` re-arms (bounded by `DROP_RECOVERY_MAX_ATTEMPTS`) while `_onSessionNeedsRefresh` reports no repaint, but never after a capture-fetch `'deadline'`. → [architecture-invariants#full-scrollback-replay](docs/architecture-invariants.md#full-scrollback-replay) +**Full-scrollback replay**: `GET /api/sessions/:id/terminal?full=1` returns the whole tmux scrollback ALONE (`source='mux-full-history'`), superseding the byte buffer. First load of each non-shell TUI session requests it (`_fullHistoryLoaded`); Shell selection and drop recovery use a bounded 1 MiB `?tail=`, and a Shell scroll-to-top pulls a bounded `?full=1&tail=` window (a window no longer than the browser's buffer is skipped before the downgrade guard, so it never marks the session exhausted); the unbounded pull stays behind **Load full history**. A Shell split-pane Pane B has its own copy of the bounded pull against its own xterm (`SplitTerminalPane._pullHistory`, terminal-split.js); keep the two in step. → [architecture-invariants#split-pane-sessions](docs/architecture-invariants.md#split-pane-sessions) ⚠️ The capture ends with a RELATIVE cursor move back to the pane's caret (never `CUP`), so no line-deleting transform may run over it; those skips key on `isFullCapture`, never on `?full=1` alone. ⚠️ A re-pull must never shrink the buffer (`_replayWouldShrinkBuffer()`). ⚠️ `captureCols`/`captureRows` are absent when no frame was positioned: test `Number.isFinite`, never truthiness. ⚠️ A frame dropped at the 128 KiB render cap MUST be recovered, and the recovery verifies itself: `_scheduleDroppedOutputRecovery` re-arms (bounded by `DROP_RECOVERY_MAX_ATTEMPTS`) while `_onSessionNeedsRefresh` reports no repaint, but never after a capture-fetch `'deadline'`. → [architecture-invariants#full-scrollback-replay](docs/architecture-invariants.md#full-scrollback-replay) **Split-pane sessions** (`showSplitButton`, header button, default OFF, desktop-only, per-device): a second live session ("Pane B") beside the active one, in its own `SplitTerminalPane` (terminal-split.js) with its own xterm + WebSocket, resizable via a draggable divider. Deliberately plainer than the primary pane — no local-echo overlay, CJK IME, or touch handlers — and NOT persisted across reloads. → [architecture-invariants#split-pane-sessions](docs/architecture-invariants.md#split-pane-sessions) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 8acb6ba3..32f6c172 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. ⚠️ A replay's own `\x1bc` would otherwise wipe the "Pane B disconnected" marker `onclose` wrote 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) — `_pullHistory()` re-stamps the marker after the live-frame flush when the socket closed in either order (before the pull started, or mid-fetch), 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 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`. ### Gesture control: the setting diff --git a/src/web/public/terminal-split.js b/src/web/public/terminal-split.js index fe031b4a..f8d352c7 100644 --- a/src/web/public/terminal-split.js +++ b/src/web/public/terminal-split.js @@ -293,19 +293,26 @@ // normal while it quietly ate everything typed into it. v1 scope is // "say so", not reconnect — collapsing the split would lose the // user's place in Pane B's scrollback for a transient blip. - this.ws.onclose = () => { - this._wsReady = false; - this._wsClosed = true; - this._writeDisconnectedMarker(); - }; + this.ws.onclose = () => this._onSocketClosed(); this.ws.onerror = () => { // onclose fires after onerror — cleanup happens there. }; } - // Extracted so both onclose and a history-pull replay that lands on an - // already-closed socket can write it (see _pullHistory()'s finally block). + // The socket's close, split out of connect() so the tests can drive it. + // While a history pull is running the marker waits for the pull's finally + // block: written now, it would sit above the output the pull is still + // holding (flushed after it on a skip, a downgrade or a failed fetch) or + // land in the middle of a chunked replay. + _onSocketClosed() { + this._wsReady = false; + this._wsClosed = true; + if (!this._liveQueue) this._writeDisconnectedMarker(); + } + + // Extracted so both _onSocketClosed() and a history pull that ends on a + // closed socket can write it (see _pullHistory()'s finally block). _writeDisconnectedMarker() { this.terminal?.write('\r\n\x1b[2m[Pane B disconnected — close and reopen the split to reconnect]\x1b[0m\r\n'); } @@ -423,6 +430,8 @@ // 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; @@ -491,14 +500,15 @@ if (entry.clear) this.terminal?.clear(); else this.terminal?.write(entry.data); } - // A replay's own `\x1bc` wipes the disconnected marker onclose wrote, + // 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. Re-stamp it - // if the socket closed in either order (before the pull started, or - // while the fetch was in flight) — checked after the queue flush so - // it is the last thing on screen, matching what onclose would have + // 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 (replayed && this._wsClosed) this._writeDisconnectedMarker(); + if (this._wsClosed && (replayed || !closedBefore)) this._writeDisconnectedMarker(); this._endBufferLoad(); } } diff --git a/test/split-pane-terminal-unit.test.ts b/test/split-pane-terminal-unit.test.ts index 90de98cc..9f86c384 100644 --- a/test/split-pane-terminal-unit.test.ts +++ b/test/split-pane-terminal-unit.test.ts @@ -65,6 +65,7 @@ type PaneUnderTest = { _onLiveClear(): void; _installWheelListener(): void; _writeDisconnectedMarker(): void; + _onSocketClosed(): void; }; const fetchMock = vi.fn(); @@ -133,6 +134,8 @@ function deferred() { return { promise, resolve }; } +const isMarker = (data: unknown) => typeof data === 'string' && data.includes('Pane B disconnected'); + /** Lets every microtask the vm-side promise chain queued run. */ const settle = () => new Promise((r) => setTimeout(r, 0)); @@ -664,6 +667,18 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { expect(connect).toContain('this._installWheelListener();'); expect(connect).toContain('this._onLiveClear();'); expect(connect).not.toContain('this.terminal.clear();'); + // The tests below drive the close through _onSocketClosed() directly. + expect(connect).toContain('this.ws.onclose = () => this._onSocketClosed();'); + }); + + it('a close with no pull running writes the marker straight away', () => { + const pane = makePane('shell'); + + pane._onSocketClosed(); + + expect(pane._wsClosed).toBe(true); + expect(pane.terminal.write).toHaveBeenCalledTimes(1); + expect(isMarker(pane.terminal.write.mock.calls[0][0])).toBe(true); }); it('re-stamps the disconnected marker after a replay if the socket closed before the pull started', async () => { @@ -683,20 +698,77 @@ describe('SplitTerminalPane scroll-to-top history pull', () => { expect(pane.terminal.write).toHaveBeenCalledWith(marker); }); - it('re-stamps the disconnected marker after a replay if the socket closes mid-fetch', async () => { - // The other order Ark0N's review called out: the close lands while the - // capture is in flight, so the HTTP pull still succeeds (a Codeman - // restart drops the WS while the tmux session, and so the pull, survives). + 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 + // 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. const pane = makePane('shell'); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); const pull = pane._pullHistory(); - pane._wsClosed = true; // the close arrives mid-fetch, before the response + pane._onSocketClosed(); // the close arrives mid-fetch, before the response + expect(pane.terminal.write).not.toHaveBeenCalled(); response.resolve(jsonResponse(rowsOf(100))); await pull; - expect(pane.terminal.write.mock.calls.at(-1)?.[0]).toEqual(expect.stringMatching(/Pane B disconnected/)); + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes.filter(isMarker)).toHaveLength(1); + expect(isMarker(writes.at(-1))).toBe(true); + }); + + 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 close mid-fetch that ends in %s writes the marker last, after the held frames', + async (_label, rowsHeld, mockFetch) => { + // 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 pull = pane._pullHistory(); + pane._onLiveOutput('frame-A'); + pane._onLiveOutput('frame-B'); + pane._onSocketClosed(); + expect(pane.terminal.write).not.toHaveBeenCalled(); + await pull; + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes.slice(0, 2)).toEqual(['frame-A', 'frame-B']); + expect(writes).toHaveLength(3); + expect(isMarker(writes[2])).toBe(true); + expect(pane._liveQueue).toBeNull(); + } + ); + + it('a close during the chunked replay writes exactly one marker, at the end', async () => { + const pane = makePane('shell'); + const response = deferred>(); + fetchMock.mockReturnValueOnce(response.promise); + + const pull = pane._pullHistory(); + // Three chunks, so the replay is still mid-write once the fetch lands. + const bigReplay = Array.from({ length: 200 }, () => 'y'.repeat(400)).join('\n'); + response.resolve(jsonResponse(bigReplay)); + await settle(); + expect(rafQueue).toHaveLength(1); + + // Written now, the marker would land between two chunks of recovered history. + pane._onSocketClosed(); + rafQueue.shift()!(); + rafQueue.shift()!(); + await pull; + + const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + expect(writes[0]).toBe('\x1bc'); + expect(writes.filter(isMarker)).toHaveLength(1); + expect(isMarker(writes.at(-1))).toBe(true); }); it('does not re-stamp the marker when the socket is still open', async () => {