mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 14:09:42 +02:00
fix(split-pane): keep the disconnected marker visible, skip detached sessions
Address Ark0N's review on #506: - The history pull's own `\x1bc` reset erased the "Pane B disconnected" marker onclose wrote, painting a fresh, current-looking history while onData kept silently dropping every keystroke on the dead socket — a Codeman restart drops the socket while the tmux session (and so the HTTP pull) survives, making this easy to hit. onclose now tracks the closure via `_wsClosed` in addition to writing the marker (extracted into `_writeDisconnectedMarker()`), and a replay re-stamps it in the pull's `finally` block, after the live-frame flush, whichever order the close and the pull land in. - `_maybeLoadMoreHistory()` now stands aside for a detached session, mirroring `_sendResize()`'s existing check and app.js's `_maybeRefetchFullHistory()` — its own window already owns its PTY size and scrollback. - Wording: a non-shell CLI's history is out of scope for this pull, not absent (codex and Claude's inline renderer do grow tmux history); the alternate-screen skip only matters for a direct-PTY shell, since tmux never surfaces the alt buffer to the browser xterm. CLAUDE.md points at the invariants heading directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
17232b01f6
commit
140ca35e2d
@@ -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. ⚠️ 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. → 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)
|
||||
|
||||
**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
@@ -71,6 +71,7 @@
|
||||
this.fitAddon = null;
|
||||
this.ws = null;
|
||||
this._wsReady = false;
|
||||
this._wsClosed = false;
|
||||
this._destroyed = false;
|
||||
// Single-flight state for _loadBuffer()/_refreshBuffer() below.
|
||||
this._bufferLoading = false;
|
||||
@@ -294,7 +295,8 @@
|
||||
// user's place in Pane B's scrollback for a transient blip.
|
||||
this.ws.onclose = () => {
|
||||
this._wsReady = false;
|
||||
this.terminal?.write('\r\n\x1b[2m[Pane B disconnected — close and reopen the split to reconnect]\x1b[0m\r\n');
|
||||
this._wsClosed = true;
|
||||
this._writeDisconnectedMarker();
|
||||
};
|
||||
|
||||
this.ws.onerror = () => {
|
||||
@@ -302,6 +304,12 @@
|
||||
};
|
||||
}
|
||||
|
||||
// Extracted so both onclose and a history-pull replay that lands on an
|
||||
// already-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');
|
||||
}
|
||||
|
||||
// Fetches and writes the session's current scrollback. Used both by
|
||||
// connect() (initial load) and by the `{t:'r'}` server-refresh frame
|
||||
// (below) — the primary pane's own _onSessionNeedsRefresh (app.js) is
|
||||
@@ -386,12 +394,18 @@
|
||||
// tmux holds every line — and nothing here ever went back to ask, so the
|
||||
// history was unreachable. The primary pane has the same pull
|
||||
// (app.js _maybeRefetchFullHistory); Pane B is a separate xterm and needs its
|
||||
// own. Shell only: a repaint-mode agent CLI keeps no tmux history to recover,
|
||||
// and its load already takes `full=1`. Skipped on the alternate screen
|
||||
// (nano, vim, less), where the wheel belongs to the app, not the scrollback.
|
||||
// own. Shell only: a non-shell CLI's history is out of scope for this pull
|
||||
// (its load already takes `full=1`; codex and Claude's inline renderer do
|
||||
// grow tmux history, this just isn't how they recover it). The alternate-
|
||||
// screen skip (nano, vim, less) only matters for a direct-PTY shell — under
|
||||
// tmux the browser xterm never enters the alternate buffer.
|
||||
_maybeLoadMoreHistory() {
|
||||
if (this.sessionMode !== 'shell' || this._destroyed || !this.terminal) return;
|
||||
if (this._bufferLoading) return;
|
||||
// Mirrors app.js _maybeRefetchFullHistory and this pane's own
|
||||
// _sendResize(): a detached session's own window already owns its PTY
|
||||
// size and scrollback, so Pane B has nothing of its own to reconcile.
|
||||
if (this.detachedSessions?.has(this.sessionId)) return;
|
||||
const active = this.terminal.buffer.active;
|
||||
if (active.type !== 'normal' || active.viewportY !== 0) return;
|
||||
// Momentum scrolling fires this dozens of times per flick, so cooldown
|
||||
@@ -477,6 +491,14 @@
|
||||
if (entry.clear) this.terminal?.clear();
|
||||
else this.terminal?.write(entry.data);
|
||||
}
|
||||
// A replay's own `\x1bc` wipes the disconnected marker onclose wrote,
|
||||
// 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
|
||||
// left had the pull never run.
|
||||
if (replayed && this._wsClosed) this._writeDisconnectedMarker();
|
||||
this._endBufferLoad();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,6 +54,8 @@ type PaneUnderTest = {
|
||||
_historyPullUseless: boolean;
|
||||
_liveQueue: unknown[] | null;
|
||||
_onWheel: unknown;
|
||||
_wsClosed: boolean;
|
||||
detachedSessions: Set<string> | undefined;
|
||||
destroy(): void;
|
||||
_loadBuffer(): Promise<void>;
|
||||
_refreshBuffer(): void;
|
||||
@@ -62,6 +64,7 @@ type PaneUnderTest = {
|
||||
_onLiveOutput(data: string): void;
|
||||
_onLiveClear(): void;
|
||||
_installWheelListener(): void;
|
||||
_writeDisconnectedMarker(): void;
|
||||
};
|
||||
|
||||
const fetchMock = vi.fn();
|
||||
@@ -93,8 +96,12 @@ function loadSplitTerminalPane() {
|
||||
|
||||
const SplitTerminalPane = loadSplitTerminalPane();
|
||||
|
||||
function makePane(mode = 'claude', mount: unknown = {}): PaneUnderTest & { terminal: FakeTerminal } {
|
||||
const pane = new SplitTerminalPane('s1', mount, { mode });
|
||||
function makePane(
|
||||
mode = 'claude',
|
||||
mount: unknown = {},
|
||||
opts: { detachedSessions?: Set<string> } = {}
|
||||
): PaneUnderTest & { terminal: FakeTerminal } {
|
||||
const pane = new SplitTerminalPane('s1', mount, { mode, ...opts });
|
||||
pane.terminal = {
|
||||
// xterm invokes a write's callback once everything before it is parsed.
|
||||
write: vi.fn((_data: string, done?: () => void) => done?.()),
|
||||
@@ -322,6 +329,17 @@ describe('SplitTerminalPane scroll-to-top history pull', () => {
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('stands aside for a detached session, mirroring _sendResize()', async () => {
|
||||
// A detached session's own window already owns its PTY size and
|
||||
// scrollback (buildSplitPickerSessions() already refuses to open one).
|
||||
const pane = makePane('shell', {}, { detachedSessions: new Set(['s1']) });
|
||||
|
||||
pane._maybeLoadMoreHistory();
|
||||
await settle();
|
||||
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('a flick fires once: overlapping triggers are dropped, then the cooldown holds', async () => {
|
||||
const pane = makePane('shell');
|
||||
const response = deferred<ReturnType<typeof jsonResponse>>();
|
||||
@@ -647,4 +665,62 @@ describe('SplitTerminalPane scroll-to-top history pull', () => {
|
||||
expect(connect).toContain('this._onLiveClear();');
|
||||
expect(connect).not.toContain('this.terminal.clear();');
|
||||
});
|
||||
|
||||
it('re-stamps the disconnected marker after a replay if the socket closed before the pull started', async () => {
|
||||
// onclose already wrote the marker once; a replay's own `\x1bc` would wipe
|
||||
// it and paint a fresh, current-looking history while onData keeps
|
||||
// silently dropping every keystroke on the dead socket.
|
||||
const pane = makePane('shell');
|
||||
pane._wsClosed = true;
|
||||
fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(100)));
|
||||
|
||||
void pane._pullHistory();
|
||||
await settle();
|
||||
|
||||
const marker = expect.stringContaining('Pane B disconnected');
|
||||
const writes = pane.terminal.write.mock.calls.map((c) => c[0]);
|
||||
expect(writes.at(-1)).toEqual(expect.stringMatching(/Pane B disconnected/));
|
||||
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).
|
||||
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
|
||||
response.resolve(jsonResponse(rowsOf(100)));
|
||||
await pull;
|
||||
|
||||
expect(pane.terminal.write.mock.calls.at(-1)?.[0]).toEqual(expect.stringMatching(/Pane B disconnected/));
|
||||
});
|
||||
|
||||
it('does not re-stamp the marker when the socket is still open', async () => {
|
||||
const pane = makePane('shell');
|
||||
fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(100)));
|
||||
|
||||
void pane._pullHistory();
|
||||
await settle();
|
||||
|
||||
for (const call of pane.terminal.write.mock.calls) {
|
||||
expect(call[0]).toEqual(expect.not.stringMatching(/Pane B disconnected/));
|
||||
}
|
||||
});
|
||||
|
||||
it('does not re-stamp the marker when the pull never replayed (skip/downgrade path)', async () => {
|
||||
// Nothing erased the marker in this path, so re-stamping it would be a
|
||||
// second, redundant write.
|
||||
const pane = makePane('shell');
|
||||
pane._wsClosed = true;
|
||||
fetchMock.mockResolvedValueOnce(jsonResponse(rowsOf(30))); // held in full already: no replay
|
||||
|
||||
void pane._pullHistory();
|
||||
await settle();
|
||||
|
||||
expect(pane.terminal.write).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user