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) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-10-01 11:18:57 +02:00
parent 988f111cd0
commit 3af1ff6fae
4 changed files with 103 additions and 21 deletions
+1 -1
View File
@@ -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)
File diff suppressed because one or more lines are too long
+23 -13
View File
@@ -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();
}
}
+78 -6
View File
@@ -65,6 +65,7 @@ type PaneUnderTest = {
_onLiveClear(): void;
_installWheelListener(): void;
_writeDisconnectedMarker(): void;
_onSocketClosed(): void;
};
const fetchMock = vi.fn();
@@ -133,6 +134,8 @@ function deferred<T>() {
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<ReturnType<typeof jsonResponse>>();
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<ReturnType<typeof jsonResponse>>();
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 () => {