diff --git a/CLAUDE.md b/CLAUDE.md index b83db57f..08f59a8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -348,7 +348,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **Mobile prompt composer** (PR #444, the first slice of #359, `keyboard-accessory.js`): the agent bars' Paste key is now **Compose**, a dialog with a native multiline textarea (autocorrect, autocapitalize, spellcheck) where Enter adds a line and only **Send** submits; the shell bar keeps the direct Paste dialog, since shell input is not an agent prompt. Opening it ADOPTS the whole editable terminal prompt (`_takePendingLocalEcho`): the local-echo overlay's pending text has never reached the PTY, but the flushed prefix has, so that prefix is erased with backspaces counted in CODE POINTS (`Array.from(text).length`; measured on Claude Code 2.1.278, `a` + emoji + `b` takes three, and the UTF-16 count sent four and ate the neighbour; `clearTerminalInput()` in terminal-ui.js moved with it). ⚠️ **Drafts are per-session and in memory only** (`_composerDrafts`, never persisted: prompts routinely carry secrets, and persisting them would need the 0600 treatment the intent store gets). Every non-Send exit (Cancel, backdrop, Escape, "Use terminal keyboard") leaves the taken text ONLY in the draft, with the dot on the key (`has-draft`, kept in step by `_syncComposerDraftIndicator`) as the signal that the terminal prompt is empty on purpose, and `_cleanupSessionData` discards the draft with the session. ⚠️ **Delivery is a hand-built bracketed-paste frame** (`\x1b[200~` + the text with newlines mapped to `\r` + `\x1b[201~`, byte-identical to what `terminal.paste()` would emit) through `_sendInputAsync` WITHOUT `{ useMux: true }`, plus a SEPARATE Enter 120 ms later WITH it (codex drops keys that share a PTY read with a bracketed paste). Not `terminal.paste()`, for two reasons: xterm's `bracketedPasteMode` mirror is false for every session after a tab switch or reload (`terminal.reset()` in the replay re-clones the DEC modes, the tmux capture carries no `?2004h`, and tmux never forwards the pane's DECSET to a client after attach), so a paste through xterm would go out unbracketed and the CLI would submit at the first `\r`; and xterm's onData is where the local-echo paste branch flushes pending overlay text AHEAD of the block, text the composer has already taken and erased from the PTY, so the frame goes straight to the wire with the composer as the prompt's only owner. The unconditional frame is safe for a CLI that never enabled DECSET 2004 because tmux does the gating (measured against a live pane: markers stripped for a `cat -v` pane, forwarded intact to a process that had emitted `?2004h`). ⚠️ The frame must never take the mux fallback: `TmuxManager.sendInput()` strips every `\r` and `\n`, which welds the lines together and submits them. ⚠️ The size guard sits on the `MAX_INPUT_LENGTH` boundary (64 KiB of UTF-16 code units): `ws-routes.ts` drops a longer frame WITHOUT an ACK, which would wedge the durable queue, so `_composerMaxLength` is derived from that limit minus both markers and an oversized prompt stays a draft with a toast. ⚠️ `.prompt-composer-overlay` is a `.paste-overlay` with a gutter of its own (a `padding` shorthand whose bottom is 12px plus the safe area), and the unconditional `.paste-overlay` fold rule at the end of styles.css is a later longhand at the same specificity, so it ERASED that gutter (measured at 393x852: `padding-bottom: 0px` flat, and the hinge strip REPLACING the gutter with the fold variables set): the composer has its own restatement after the fold rules, the dialog's `max-height` subtracts `--fold-block-end`, and `test/foldable-layout.test.ts` lists the composer in `ELEMENTS` by hand, because its derived overlay list keys on rules that declare `position: fixed; inset: 0` themselves. Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not under `test/mobile/**`). -**The PTY and the browser terminal must never disagree about size** (issue #464, `syncTerminalGeometry()` in terminal-ui.js): Claude Code's TUI wraps its frame at the width the PTY reported and erases the previous frame by walking the cursor up the number of rows it BELIEVES that frame occupied. A browser terminal of a different width makes each logical line take more physical rows than Ink counted, so `eraseLines(n)` clears too few and the new frame paints over rows nothing erased — the doubled lines and half-overwritten prose in #464, **measured** against this repo's xterm (a 120-column PTY against a 62-column terminal renders every wrapped line twice; `test/terminal-pty-geometry.test.ts` pins it, and pins the clean render at matching widths so the assertion cannot pass against code that fixes nothing). ⚠️ **`fitAddon.fit()` is NOT the way to resize this terminal.** It resizes xterm to `proposeDimensions()` RAW while every server-facing path reports those floored at 40x10, so whenever the floor bit the two diverged silently — **measured in Chrome at 430px**: font size 44 proposed 13 columns, the server was told 40, and xterm stayed at 13. `syncTerminalGeometry()` fits, floors and applies as one step and is the ONE function that may change the size; `test/terminal-pty-geometry.test.ts` sweeps every module for a bare fit. ⚠️ **Withhold the fit wherever you withhold the SIGWINCH.** `throttledResize` (virtual keyboard up) and `sendResize` (session detached into its own window) used to reflow locally and skip only the server write, which is the one combination that cannot be right. ⚠️ **A font change is a geometry change**: `setFontSize`/`setFontFamily`/`setFontWeight` move the cell size and told the server nothing, so raising the font on a phone left the CLI wrapping at the old column count (`_refitAfterCellSizeChange`). ⚠️ **Resize is no longer write-only.** `Session.resize` DECLINES a small-viewport request while a desktop connection holds an active sizing claim and says nothing, so both transports now answer with `Session.ptyGeometry` (`{"t":"zc"}` on the socket, the body of the resize POST) and `_onPtyGeometryReport` adopts it — a terminal that keeps a WIDTH the PTY refused renders GARBLED, not merely wrong-sized. ⚠️ **`ptyGeometry` is null without a live pane**, never the field values: `resize()` writes `_ptyCols`/`_ptyRows` only when `ptyProcess` is set and nothing seeds them from the spawn geometry, so a dead-pane session still holds the constructor defaults of 120x40 — reporting those made a client adopt a size no process was ever told and claim another device owned the pane when none existed. ⚠️ **COLUMNS ONLY.** Adopting the PTY's ROWS was a regression: a phone taking a desktop's 43 rows into a viewport with room for 18 painted an `.xterm-screen` far taller than its container, xterm's own viewport then had nothing to scroll, and the CLI's input line sat below the container with no gesture able to reach it — output visible, typing invisible, for as long as the claim stayed hot. Width is the axis the wrap arithmetic depends on; rows only decide how much is on screen, and keeping the local count keeps the composer at the bottom of a viewport that scrolls. ⚠️ **`.term-overflows-x` keys on what does not FIT, not on a PTY mismatch**, because the 40-column floor widens the terminal past a narrow container with the PTY agreeing throughout (measured at 360px: font 18 paints 433px, font 24 paints 578px — 38% unreachable, and `increaseFontSize` reaches 24 in two taps). `_syncTerminalOverflowAffordance()` MEASURES `.xterm-screen` against the container on the next frame rather than deriving it from cell arithmetic. ⚠️ That rule sets **both** overflow axes — mobile.css loads later with `.terminal-container { overflow: visible }`, and a bare `overflow-x` would leave overflow-y computing to `auto`, handing the browser a vertical scroll container the terminal's touch handler does not know about — and sets **no `touch-action`**: the terminal's own touchmove handler pans it (`canPanHorizontally`), because `touchstart` preventDefault()s every 'content' tap and that cancels a native `pan-x` before it starts (measured: a 140px swipe reached scrollLeft 141 without that preventDefault and 0 with it). Removing the class returns `scrollLeft` to 0, so a resolved mismatch cannot leave the pane parked off-screen. +**The PTY and the browser terminal must never disagree about size** (issue #464, `syncTerminalGeometry()` in terminal-ui.js): Claude Code's TUI wraps its frame at the width the PTY reported and erases the previous frame by walking the cursor up the number of rows it BELIEVES that frame occupied. A browser terminal of a different width makes each logical line take more physical rows than Ink counted, so `eraseLines(n)` clears too few and the new frame paints over rows nothing erased — the doubled lines and half-overwritten prose in #464, **measured** against this repo's xterm (a 120-column PTY against a 62-column terminal renders every wrapped line twice; `test/terminal-pty-geometry.test.ts` pins it, and pins the clean render at matching widths so the assertion cannot pass against code that fixes nothing). ⚠️ **`fitAddon.fit()` is NOT the way to resize this terminal.** It resizes xterm to `proposeDimensions()` RAW while every server-facing path reports those floored at 40x10, so whenever the floor bit the two diverged silently — **measured in Chrome at 430px**: font size 44 proposed 13 columns, the server was told 40, and xterm stayed at 13. `syncTerminalGeometry()` fits, floors and applies as one step and is the ONE function that may change the size; `test/terminal-pty-geometry.test.ts` sweeps the seven modules that touch the main terminal (terminal-ui, mobile-handlers, app, ralph-panel, settings-ui, tab-rail-resize, notification-manager) for a bare fit; the split pane and teammate terminals own their own sizing and are out of scope. ⚠️ **Withhold the fit wherever you withhold the SIGWINCH.** `throttledResize` (virtual keyboard up) and `sendResize` (session detached into its own window) used to reflow locally and skip only the server write, which is the one combination that cannot be right. ⚠️ **A font change is a geometry change**: `setFontSize`/`setFontFamily`/`setFontWeight` move the cell size and told the server nothing, so raising the font on a phone left the CLI wrapping at the old column count (`_refitAfterCellSizeChange`). ⚠️ **Resize is no longer write-only.** `Session.resize` DECLINES a small-viewport request while a desktop connection holds an active sizing claim and says nothing, so both transports now answer with `Session.ptyGeometry` (`{"t":"zc"}` on the socket, the body of the resize POST) and `_onPtyGeometryReport` adopts it — a terminal that keeps a WIDTH the PTY refused renders GARBLED, not merely wrong-sized. ⚠️ While that refusal stands (`_paneWidthRefused`), a resize ASKS for the container's width without applying it (`_geometryForResizeRequest`: rows follow the container, columns stay at the PTY's): fitting first re-wrapped the whole buffer to the container and back on every 30s mobile retry, and ran the scrollback clear for a resize that brings no redraw. `selectSession` clears the flag, since it belongs to the previous pane. ⚠️ **`ptyGeometry` is null without a live pane**, never the field values: they are seeded at spawn (`_notePtySpawnGeometry`, so a reattached pane reports its tmux window's real size) and moved by `resize()`, but a dead-pane session still holds the constructor defaults of 120x40 or a gone pane's size — reporting those made a client adopt a size no process was ever told and claim another device owned the pane when none existed. ⚠️ **COLUMNS ONLY.** Adopting the PTY's ROWS was a regression: a phone taking a desktop's 43 rows into a viewport with room for 18 painted an `.xterm-screen` far taller than its container, xterm's own viewport then had nothing to scroll, and the CLI's input line sat below the container with no gesture able to reach it — output visible, typing invisible, for as long as the claim stayed hot. Width is the axis the wrap arithmetic depends on; rows only decide how much is on screen, and keeping the local count keeps the composer at the bottom of a viewport that scrolls. ⚠️ **`.term-overflows-x` keys on what does not FIT, not on a PTY mismatch**, because the 40-column floor widens the terminal past a narrow container with the PTY agreeing throughout (measured at 360px: font 18 paints 433px, font 24 paints 578px — 38% unreachable, and `increaseFontSize` reaches 24 in two taps). `_syncTerminalOverflowAffordance()` MEASURES `.xterm-screen` against the container on the next frame rather than deriving it from cell arithmetic. ⚠️ That rule sets **both** overflow axes — mobile.css loads later with `.terminal-container { overflow: visible }`, and a bare `overflow-x` would leave overflow-y computing to `auto`, handing the browser a vertical scroll container the terminal's touch handler does not know about — and sets **no `touch-action`**: the terminal's own touchmove handler pans it (`canPanHorizontally`), because `touchstart` preventDefault()s every 'content' tap and that cancels a native `pan-x` before it starts (measured: a 140px swipe reached scrollLeft 141 without that preventDefault and 0 with it). Removing the class returns `scrollLeft` to 0, so a resolved mismatch cannot leave the pane parked off-screen. **Terminal resilience: replay clears, renderer liveness, fetch deadlines**: three rules that each close a way the terminal silently stops being correct. ⚠️ **A replay clear MUST be in-stream, never `reset()`/`clear()`.** xterm's `write()` is asynchronously queued while `Terminal.reset()` is synchronous and, per upstream, "does not clear input buffers and does not reset the parser" — so bytes queued just before a reset are parsed AFTER it and fuse into the snapshot written next. **Measured** against the real xterm in this repo: `write('p8'); reset(); write('rmissions')` renders `p8rmissions`; the queued `\x1bc` renders `rmissions` and clears scrollback. `_resetTerminalForReplay()` (app.js) is the ONE clear, a single queued `\x1bc` (RIS), and all three replay paths go through it; RIS rather than `\x1b[3J\x1b[H\x1b[2J` because the erase leaves modes, charsets, scroll regions and SGR state alone. Callers may still chunk the content — ordering in the queue is what matters, not writing it in one call. ⚠️ **The renderer watchdog reads xterm privates and CANNOT be covered by the gate.** `_kickRenderer()` (terminal-ui.js) cancels a stale `_core._renderService._renderDebouncer._animationFrame` and forces a repaint. **Verified against xterm 6.0.0** (jsdom, after `open()`): the field path resolves, a forced stale handle genuinely makes `refreshRows` a no-op, and the kick schedules a fresh frame. **Reasoned, not reproduced here**: the premise that iOS discards scheduled rAF callbacks when a PWA backgrounds, which is what leaves the handle stale — that half wants a real-device pass. Codeman has exactly ONE xterm for the whole page load, so one backgrounding would wedge it until a reload. `_renderService` only exists after `open()`, which needs a real DOM, and the gate runs in node — so `test/xterm-private-api.test.ts` pins the RESOLVED lockfile version (not the `^6.0.0` range, which a real upgrade slips through) and a bump means re-verifying by hand. Every access is optional-chained on purpose: a renamed field must degrade to a no-op, never throw on a 2s timer. ⚠️ **Every terminal capture carries a deadline, and the helper reads the BODY** (`_fetchTerminalCapture`, app.js). `await fetch()` settles on response HEADERS, so clearing the timer there leaves the body — the multi-megabyte `?full=1` capture this exists for — unbounded: **measured** at 4026ms under a 1000ms deadline before the fix. The helper therefore returns `{json, headers, headersAt}` rather than a `Response`, and `_terminalCaptureInflight` is scoped the same way so a body still streaming counts toward a capture starting beside it. It degrades to a plain fetch where `AbortController` is missing — the deadline is a safety net, not a dependency. Tests: `test/terminal-resilience.test.ts` (pure decisions), `test/xterm-private-api.test.ts`. diff --git a/src/session.ts b/src/session.ts index 501e7949..6a36f253 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1973,6 +1973,7 @@ export class Session extends EventEmitter { env: buildMuxAttachEnv(cliExportsTruecolor(this.mode)), }) ); + this._notePtySpawnGeometry(ptyCols, ptyRows); } catch (spawnErr) { console.error(`[Session] Failed to spawn PTY for ${options.spawnErrLabel}:`, spawnErr); this.emit('error', `Failed to attach to mux session: ${spawnErr}`); @@ -2684,6 +2685,7 @@ export class Session extends EventEmitter { env: { ...buildClaudeEnv(this.id), ...(this._envOverrides ?? {}) }, }) ); + this._notePtySpawnGeometry(120, 40); } catch (spawnErr) { console.error('[Session] Failed to spawn Claude PTY:', spawnErr); this._status = 'stopped'; @@ -3306,6 +3308,7 @@ export class Session extends EventEmitter { env: buildShellEnv(this.id), }) ); + this._notePtySpawnGeometry(120, 40); } catch (spawnErr) { console.error('[Session] Failed to spawn shell PTY:', spawnErr); this._status = 'stopped'; @@ -3415,6 +3418,7 @@ export class Session extends EventEmitter { env: { ...buildClaudeEnv(this.id), ...(this._envOverrides ?? {}) }, }) ); + this._notePtySpawnGeometry(120, 40); } catch (spawnErr) { console.error('[Session] Failed to spawn Claude PTY for runPrompt:', spawnErr); this.emit( @@ -4101,6 +4105,17 @@ export class Session extends EventEmitter { private _ptyCols = 120; private _ptyRows = 40; + /** + * Record the geometry a PTY was just spawned at. A reattached pane keeps the + * tmux window's size, not the constructor's 120x40, and without this + * `ptyGeometry` reported the old numbers for a live pane and the dedupe in + * `resize()` skipped a real resize that happened to match them. + */ + private _notePtySpawnGeometry(cols: number, rows: number): void { + this._ptyCols = cols; + this._ptyRows = rows; + } + /** * The geometry the CLI is actually drawing for, or null when nothing is * drawing. @@ -4111,11 +4126,11 @@ export class Session extends EventEmitter { * than wrong-sized output, because Claude Code's repaints are computed from * the width it was told (issue #464). Both transports report this back. * - * ⚠️ NULL WITHOUT A PANE, never the field values. `resize()` writes - * `_ptyCols`/`_ptyRows` only when `ptyProcess` is set, and nothing seeds them - * from the spawn geometry, so a session with a dead pane — or one created - * through the API and never started — still holds the constructor defaults - * of 120x40. Reporting those made a client adopt a size no process had ever + * ⚠️ NULL WITHOUT A PANE, never the field values. The fields are seeded at + * spawn (`_notePtySpawnGeometry`) and moved by `resize()`, but a session with + * a dead pane (or one created through the API and never started) still + * holds the constructor defaults of 120x40, or the size of a pane that is + * gone. Reporting those made a client adopt a size no process had ever * been told, and on anything narrower than 120 columns it claimed another * device owned the pane when none existed. `reconcilePtyGeometry` treats a * report with no finite numbers as no evidence, which is the truth here. @@ -4211,7 +4226,7 @@ export class Session extends EventEmitter { if (Date.now() - this._lastDesktopActivityAt < Session.DESKTOP_CLAIM_IDLE_MS) { // Declined. The caller is told nothing here on purpose — the decision // belongs to the session, not the socket — but the caller MUST report - // `ptyCols`/`ptyRows` back afterwards so the asking client can adopt + // `ptyGeometry` back afterwards so the asking client can adopt // the shape it did not get. Both transports do; see issue #464. return; } diff --git a/src/web/public/app.js b/src/web/public/app.js index b4e52067..1849aa99 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -6652,6 +6652,10 @@ class CodemanApp { // Fit terminal to container BEFORE writing any buffer data. // If the browser was resized while viewing another session, the terminal // canvas may be at stale dimensions — content would render at wrong width. + // A width refusal belonged to the PREVIOUS session's pane, and while it + // stands sendResize keeps the columns it last adopted; this pane's own + // report re-establishes it if another device holds this one too. + this._paneWidthRefused = false; this.syncTerminalGeometry(); // Also push the new dimensions to the PTY. Without this, codex/codeman diff --git a/src/web/public/constants.js b/src/web/public/constants.js index cf895d94..4855fc3d 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -1770,11 +1770,6 @@ function clampTerminalDimensions(proposed) { }; } -/** Whether two geometries are the same screen. Either being absent is a mismatch. */ -function terminalGeometryAgrees(a, b) { - return !!a && !!b && a.cols === b.cols && a.rows === b.rows; -} - /** * What to do when the server reports the PTY's real geometry. * @@ -1829,7 +1824,6 @@ if (typeof window !== 'undefined') { }; window.CodemanTerminalGeometry = { clampTerminalDimensions, - terminalGeometryAgrees, reconcilePtyGeometry, TERMINAL_MIN_COLS, TERMINAL_MIN_ROWS, diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index a478c40b..903661e5 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1093,7 +1093,8 @@ Object.assign(CodemanApp.prototype, { // a shape the PTY is never told about, and Claude Code computes its // repaints from the shape it was told. Withhold both, or neither — // a reflow nothing is rendering for buys nothing and costs correctness. - const dims = this.activeSessionId && !keyboardUp && !detachedElsewhere ? this.syncTerminalGeometry() : null; + const dims = + this.activeSessionId && !keyboardUp && !detachedElsewhere ? this._geometryForResizeRequest() : null; // ⚠️ A null measurement is NOT a reason to report the floor. It used to // fall back to a bare 40x10, which tells the PTY a shape nothing measured // and xterm does not hold — the write-only guess this whole change exists @@ -1112,9 +1113,12 @@ Object.assign(CodemanApp.prototype, { // IMPORTANT: Only clear when we're actually sending SIGWINCH (dims changed). // Clearing without a subsequent Ink redraw leaves the terminal blank. const activeResizeSession = this.activeSessionId ? this.sessions.get(this.activeSessionId) : null; + // Not while another device holds the width: the columns were not + // reflowed here, and a refused resize brings no redraw after it. if ( activeResizeSession && activeResizeSession.mode !== 'shell' && + !this._paneWidthRefused && this.terminal && this.isTerminalAtBottom() ) { @@ -5615,6 +5619,33 @@ Object.assign(CodemanApp.prototype, { return dims; }, + /** + * The geometry to ASK the server for, applied locally only as far as the PTY + * can follow it. + * + * Ordinarily that is all of it: `syncTerminalGeometry()`. While another + * device holds the width (`_paneWidthRefused`, set by `_onPtyGeometryReport`) + * it is not. Fitting then re-wraps xterm to the container's columns, the + * request is refused, the report puts the PTY's columns back, and the whole + * buffer re-wraps twice per ask, with the viewport pointing at a different + * part of the scrollback in between. The mobile retry asks every 30 seconds, + * so that happened on a timer for as long as the refusal lasted. So the + * columns stay at the PTY's (the #464 invariant: the browser never draws at + * a width the PTY does not have), the rows follow the container (they are + * never adopted, see reconcilePtyGeometry), and the container's columns go + * out as the request. An accepted request is adopted by the report. + * + * @returns {{cols: number, rows: number}|null} the geometry to request + */ + _geometryForResizeRequest() { + if (!this._paneWidthRefused) return this.syncTerminalGeometry(); + const wanted = this.getTerminalDimensions(); + if (!wanted || !this.terminal) return null; + if (!this._resizeTerminalTo({ cols: this.terminal.cols, rows: wanted.rows })) return null; + this._scheduleOverflowAffordanceSync(); + return wanted; + }, + /** * Re-measure after something changed the CELL size, and tell the server. * @@ -5677,8 +5708,9 @@ Object.assign(CodemanApp.prototype, { // a pane it does not own is not its to refit either. if (!this.isSoloWindow && this.detachedSessions?.has(sessionId)) return false; // Fit, floor, and apply in one step so the numbers below are the numbers - // xterm is actually holding. - const dims = this.syncTerminalGeometry(); + // xterm is actually holding (or, while another device holds the width, + // the numbers this container would hold if the PTY followed). + const dims = this._geometryForResizeRequest(); if (!dims) return false; // Did the dimensions actually change since the last resize we sent? Callers // use this to skip work (e.g. the post-resize TUI-redraw settle) when no diff --git a/test/detached-session-pane-sizing.test.ts b/test/detached-session-pane-sizing.test.ts index 05a5a1e0..16b6f59b 100644 --- a/test/detached-session-pane-sizing.test.ts +++ b/test/detached-session-pane-sizing.test.ts @@ -65,6 +65,7 @@ function makeApp(overrides: Record = {}) { // The real chain: sendResize fits, floors and applies through one function // now, so the harness must let it (#464). syncTerminalGeometry: mixin.syncTerminalGeometry, + _geometryForResizeRequest: mixin._geometryForResizeRequest, _resizeTerminalTo: mixin._resizeTerminalTo, // Real, so a geometry change really does re-check whether the terminal now // overflows its container (#464 item 4) — the fake DOM has no container, so diff --git a/test/terminal-pty-geometry.test.ts b/test/terminal-pty-geometry.test.ts index 57d30ca7..22c4a208 100644 --- a/test/terminal-pty-geometry.test.ts +++ b/test/terminal-pty-geometry.test.ts @@ -24,7 +24,7 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import vm from 'node:vm'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import xtermHeadless from '@xterm/headless'; const { Terminal } = xtermHeadless as unknown as { @@ -47,7 +47,6 @@ function loadGeometry() { context.window as { CodemanTerminalGeometry: { clampTerminalDimensions: (p: Partial | null | undefined) => Dims | null; - terminalGeometryAgrees: (a: Dims | null, b: Dims | null) => boolean; reconcilePtyGeometry: (local: Dims | null, pty: Partial | null) => { adopt: boolean; oversized: boolean }; TERMINAL_MIN_COLS: number; TERMINAL_MIN_ROWS: number; @@ -226,12 +225,15 @@ describe('exactly one function may change the terminal size', () => { expect(body).toContain('this._resizeTerminalTo(dims)'); }); - // A whole-repo sweep rather than a spot check, so a NEW call site trips it - // rather than quietly reopening #464. Scoped to the MAIN terminal: the split + // A sweep of every module that touches the main terminal rather than a spot + // check, so a NEW call site there trips it rather than quietly reopening + // #464. A module outside the list below is not covered. Scoped to the MAIN + // terminal: the split // pane, the teammate windows and the log viewer are separate xterm instances // with their own PTYs (or none), and each owns its own sizing. it('no other call site fits the main terminal behind its back', () => { - const MAIN_TERMINAL_FIT = /^(?!.*(?:_splitPane|entry\.fitAddon)).*fitAddon[?.]*\.fit\(\)/; + // `fit(` optionally called through `?.`, so `fitAddon?.fit?.()` counts too. + const MAIN_TERMINAL_FIT = /^(?!.*(?:_splitPane|entry\.fitAddon)).*fitAddon[?.]*\.fit(?:\?\.)?\(\)/; const offenders: string[] = []; for (const rel of [ 'src/web/public/terminal-ui.js', @@ -293,7 +295,7 @@ describe('exactly one function may change the terminal size', () => { it('sendResize yields a detached session BEFORE touching geometry, not after', () => { const body = bodyOf(terminalUi, 'async sendResize(sessionId, options = {}) {'); const yieldAt = body.indexOf('detachedSessions?.has(sessionId)) return false'); - const fitAt = body.indexOf('this.syncTerminalGeometry()'); + const fitAt = body.indexOf('this._geometryForResizeRequest()'); expect(yieldAt, 'the detached-session yield is gone').toBeGreaterThan(-1); expect(fitAt, 'sendResize no longer syncs geometry').toBeGreaterThan(-1); expect( @@ -308,7 +310,7 @@ describe('exactly one function may change the terminal size', () => { expect(start).toBeGreaterThan(-1); const block = terminalUi.slice(start, terminalUi.indexOf("window.addEventListener('resize', throttledResize)")); const guardAt = block.indexOf('!keyboardUp && !detachedElsewhere'); - const syncAt = block.indexOf('this.syncTerminalGeometry()'); + const syncAt = block.indexOf('this._geometryForResizeRequest()'); expect(guardAt).toBeGreaterThan(-1); expect(syncAt, 'the geometry sync must sit INSIDE the guard').toBeGreaterThan(guardAt); }); @@ -463,3 +465,102 @@ describe('the server reports the geometry the PTY actually holds', () => { ); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// A refused width must not be re-applied locally on every ask. The real mixin +// methods, run against a fake terminal whose FitAddon behaves like xterm's. +// ─────────────────────────────────────────────────────────────────────────── + +describe('while another device holds the width', () => { + const SESSION = 'session-A'; + + function makeApp() { + const FakeCodemanApp = function () {} as unknown as { prototype: Record }; + const context = vm.createContext({ + console, + setTimeout, + clearTimeout, + setInterval: vi.fn(), + clearInterval: vi.fn(), + CodemanApp: FakeCodemanApp, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn(), innerWidth: 400 }, + document: { addEventListener: vi.fn(), getElementById: () => null }, + }); + vm.runInContext(read('src/web/public/constants.js'), context, { filename: 'constants.js' }); + vm.runInContext(read('src/web/public/terminal-ui.js'), context, { filename: 'terminal-ui.js' }); + const mixin = FakeCodemanApp.prototype as Record unknown>; + // The phone's container: 57x13. Every resize xterm performs is recorded, + // because a resize is a re-wrap of the whole buffer. + const resizes: Array<[number, number]> = []; + const terminal = { + cols: 80, + rows: 24, + resize(cols: number, rows: number) { + resizes.push([cols, rows]); + this.cols = cols; + this.rows = rows; + }, + }; + const proposal = { cols: 57, rows: 13 }; + const sent: Array<{ c: number; r: number }> = []; + const app = Object.assign(Object.create(mixin), { + terminal, + fitAddon: { + proposeDimensions: () => ({ ...proposal }), + // xterm's FitAddon resizes to the raw proposal. + fit: () => terminal.resize(proposal.cols, proposal.rows), + }, + activeSessionId: SESSION, + detachedSessions: new Set(), + isSoloWindow: false, + _lastResizeDims: null, + _wsReady: true, + _wsSessionId: SESSION, + _ws: { send: (frame: string) => sent.push(JSON.parse(frame)) }, + _notePaneOwnedElsewhere: vi.fn(), + }) as Record & { + sendResize: (id: string) => Promise; + _onPtyGeometryReport: (id: string, cols: number, rows: number) => void; + _paneWidthRefused?: boolean; + }; + return { app, terminal, resizes, sent, proposal }; + } + + it('asks for its own width again without re-wrapping to it until the PTY follows', async () => { + const { app, terminal, resizes, sent } = makeApp(); + await app.sendResize(SESSION); + expect(sent.at(-1)).toMatchObject({ c: 57, r: 13 }); + // Refused: the desktop keeps the pane at 198 columns. + app._onPtyGeometryReport(SESSION, 198, 43); + expect(terminal.cols).toBe(198); + expect(app._paneWidthRefused).toBe(true); + + // The retry timer asks again. Nothing about the screen changed, so xterm + // must not be re-wrapped to 57 and back (it used to be, every 30 seconds). + resizes.length = 0; + await app.sendResize(SESSION); + app._onPtyGeometryReport(SESSION, 198, 43); + expect(resizes).toEqual([]); + expect(terminal.cols).toBe(198); + // It still ASKS for this screen's width, which is how it recovers. + expect(sent.at(-1)).toMatchObject({ c: 57, r: 13 }); + + // The desktop went idle and the request took: the report is adopted. + await app.sendResize(SESSION); + app._onPtyGeometryReport(SESSION, 57, 13); + expect(terminal.cols).toBe(57); + expect(app._paneWidthRefused).toBe(false); + }); + + it('still follows the container\u2019s rows while the width is held elsewhere', async () => { + const { app, terminal, resizes, proposal } = makeApp(); + await app.sendResize(SESSION); + app._onPtyGeometryReport(SESSION, 198, 43); + resizes.length = 0; + proposal.rows = 20; // keyboard dismissed + await app.sendResize(SESSION); + // Rows only: the columns stay at the width the PTY has. + expect(resizes).toEqual([[198, 20]]); + expect(terminal.cols).toBe(198); + }); +}); diff --git a/test/xterm-private-api.test.ts b/test/xterm-private-api.test.ts index 36a0b891..a02f51df 100644 --- a/test/xterm-private-api.test.ts +++ b/test/xterm-private-api.test.ts @@ -17,9 +17,9 @@ // a headless Terminal reports `_renderService: undefined`, so a test here would // pass whether or not the field still exists, which is worse than no test. // -// So this guards the next best thing: the dependency range those field names -// were verified against. A major bump fails here, loudly, and sends someone to -// re-verify `_kickRenderer` by hand in a browser. The failure mode being +// So this guards the next best thing: the exact xterm version those field names +// were verified against, as resolved in the lockfile. ANY bump fails here, +// loudly, and sends someone to re-verify `_kickRenderer` by hand in a browser. The failure mode being // defended against is silent — every access in `_kickRenderer` is // optional-chained, so a renamed field degrades it to a permanent no-op with no // error, no log, and a terminal that simply freezes again.