From a54ad816841af06fa0ecc64a23dfbddd9ec39464 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 6 Oct 2026 10:07:40 +0200 Subject: [PATCH] fix(split): Pane B and its PTY never disagree about size (#464) - Font size, family and weight changes refit Pane B AND tell its PTY. They used to reflow the xterm only, leaving the CLI wrapping at the old column count, the garbled-redraw class #464 fixed for the primary pane. - The resize frame reports the size the xterm actually holds, with no 40x10 floor (the divider's 20% clamp leaves about 28 columns), skips an unchanged size, and is always re-sent on a fresh socket so it re-registers as a desktop viewer. - The server's {t:'zc'} geometry report is handled: a different column count is adopted, rows stay local, using the primary pane's own reconcilePtyGeometry verdict. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/terminal-tile.js | 60 ++++++++++++--- src/web/public/terminal-ui.js | 6 +- test/terminal-font-weight.test.ts | 12 +-- test/terminal-tile-input.test.ts | 122 +++++++++++++++++++++++++++--- 4 files changed, 171 insertions(+), 29 deletions(-) diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index c80747b2..0c673528 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -107,6 +107,11 @@ this._stoppedCode = null; this._markerText = TerminalTile.MARKER_RECONNECTING; this.onExit = typeof opts.onExit === 'function' ? opts.onExit : null; + // The `{ cols, rows }` last sent in a `{t:'z'}` frame, so an unchanged size + // is not resent (each one costs a `tmux resize-window` and a SIGWINCH). + // Cleared on every open: a fresh socket must announce its size, which is + // also what registers it as a desktop viewer server-side. + this._lastSentDims = null; } async connect() { @@ -325,6 +330,8 @@ } else if (msg.t === 'ia') { // Input ACK. The frame names no session, so it is this pane's. global.app?._onWsInputAck?.(msg.seq, msg, this.sessionId); + } else if (msg.t === 'zc') { + this._onPtyGeometryReport(msg.c, msg.r); } } catch { /* Malformed frame — ignore, matches primary pane's tolerance. */ @@ -377,6 +384,7 @@ this._wsClosed = false; this._markerOwed = false; this._reconnectAttempts = 0; + this._lastSentDims = null; this._registerInputSocket(); this._sendResize(); if (reconnected) this._refreshBuffer(); @@ -757,27 +765,55 @@ this.fitAddon.fit(); } - fit() { + // Reflow to the container and tell the PTY, as one step: the xterm and the + // PTY must never disagree about size (#464), and a font change is a size + // change too, so the font setters call this rather than localFit(). + // `force` resends an unchanged size. + fit({ force = false } = {}) { this.localFit(); - this._sendResize(); + this._sendResize({ force }); } - _sendResize() { - if (!this._wsReady || !this.fitAddon) return; + _sendResize({ force = false } = {}) { + if (!this._wsReady || !this.fitAddon || !this.terminal) return; // One PTY cannot hold two sizes (mirrors sendResize's own // detachedElsewhere yield in terminal-ui.js): the session got detached // to its own window AFTER this split was opened, so its own window now // owns the PTY's size and Pane B must stand aside. if (this.detachedSessions?.has(this.sessionId)) return; + // A hidden pane (a web tab over it, a zoomed neighbour) measures NaN, and + // fit() then leaves the xterm alone: there is no size worth reporting. const dims = this.fitAddon.proposeDimensions(); - if (!dims) return; - // Send the real proposed dimensions unclamped, matching the primary - // pane's convention (terminal-ui.js's getTerminalDimensions()) — the - // server enforces its own valid range ([1,500]/[1,200] in ws-routes.ts). - // A 40/10 floor here misreported Pane B's real width to the PTY at the - // divider's own reachable 20% floor position, causing real - // output-wrapping bugs. - this.ws.send(JSON.stringify({ t: 'z', c: dims.cols, r: dims.rows, v: 'desktop' })); + if (!dims || !Number.isFinite(dims.cols) || !Number.isFinite(dims.rows)) return; + // Report what the xterm actually holds, so the PTY gets exactly the size + // the pane renders at. Unclamped, unlike the primary pane's 40x10 floor: + // a floor here misreported Pane B's width at the divider's reachable 20% + // position (about 28 columns), causing real output-wrapping bugs, and a + // floored xterm would be wider than its container. The server enforces + // its own valid range ([1,500]/[1,200] in ws-routes.ts). + const cols = this.terminal.cols; + const rows = this.terminal.rows; + const last = this._lastSentDims; + if (!force && last && last.cols === cols && last.rows === rows) return; + this._lastSentDims = { cols, rows }; + this.ws.send(JSON.stringify({ t: 'z', c: cols, r: rows, v: 'desktop' })); + } + + // The geometry the PTY actually holds (`{t:'zc'}`, the server's answer to + // every resize). A PTY and a terminal that disagree on WIDTH render + // garbled, so a different column count is adopted; rows stay local, as in + // the primary pane (_onPtyGeometryReport in terminal-ui.js, #464). The + // pure verdict is the primary's too (reconcilePtyGeometry, constants.js). + _onPtyGeometryReport(cols, rows) { + const terminal = this.terminal; + if (!terminal) return; + const verdict = global.CodemanTerminalGeometry?.reconcilePtyGeometry?.( + { cols: terminal.cols, rows: terminal.rows }, + { cols, rows } + ); + if (!verdict?.adopt) return; + terminal.resize(verdict.cols, terminal.rows); + this._lastSentDims = { cols: verdict.cols, rows: terminal.rows }; } destroy() { diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index be4528b3..8a0c9f08 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -5735,7 +5735,7 @@ Object.assign(CodemanApp.prototype, { this._predictiveEcho?.refreshFont(); this._forEachTile?.((tile) => { tile.terminal.options.fontSize = size; - tile.localFit(); + tile.fit(); // a font change is a size change: tell its PTY too (#464) }); }, @@ -5764,7 +5764,7 @@ Object.assign(CodemanApp.prototype, { this._predictiveEcho?.refreshFont(); this._forEachTile?.((tile) => { tile.terminal.options.fontFamily = resolved; - tile.localFit(); + tile.fit(); // a font change is a size change: tell its PTY too (#464) }); }, @@ -5820,7 +5820,7 @@ Object.assign(CodemanApp.prototype, { this._forEachTile?.((tile) => { tile.terminal.options.fontWeight = fontWeight; tile.terminal.options.fontWeightBold = fontWeightBold; - tile.localFit(); + tile.fit(); // a font change is a size change: tell its PTY too (#464) }); }, diff --git a/test/terminal-font-weight.test.ts b/test/terminal-font-weight.test.ts index 15b5eb92..ee872011 100644 --- a/test/terminal-font-weight.test.ts +++ b/test/terminal-font-weight.test.ts @@ -67,7 +67,7 @@ function makeApp( opts: { teammates?: number; terminal?: ReturnType | null; - splitPane?: { terminal: ReturnType; localFit: () => void } | null; + splitPane?: { terminal: ReturnType; fit: () => void } | null; } = {} ) { const fit = vi.fn(); @@ -108,10 +108,12 @@ function makeApp( } describe('applyTerminalFontWeights', () => { - it('reaches an open split pane: same weights, refit in place', () => { + it('reaches an open split pane: same weights, refit AND reported to its PTY', () => { const splitTerminal = fakeTerminal(); - const localFit = vi.fn(); - const { app } = makeApp({ splitPane: { terminal: splitTerminal, localFit } }); + // fit(), not localFit(): a weight change can move the cell size, and the + // split pane's PTY must hear about the new geometry like the primary's does. + const fit = vi.fn(); + const { app } = makeApp({ splitPane: { terminal: splitTerminal, fit } }); (app as unknown as { applyTerminalFontWeights: (s: unknown) => void }).applyTerminalFontWeights({ terminalFontWeight: '300', @@ -120,7 +122,7 @@ describe('applyTerminalFontWeights', () => { expect(splitTerminal.options.fontWeight).toBe(300); expect(splitTerminal.options.fontWeightBold).toBe(700); - expect(localFit).toHaveBeenCalledTimes(1); + expect(fit).toHaveBeenCalledTimes(1); }); it('writes both slots to the live terminal', () => { diff --git a/test/terminal-tile-input.test.ts b/test/terminal-tile-input.test.ts index b7e38889..cfd99e14 100644 --- a/test/terminal-tile-input.test.ts +++ b/test/terminal-tile-input.test.ts @@ -58,6 +58,20 @@ class FakeSocket { } } +/** The fit addon: proposes `FakeFit.proposed` and, like the real one, resizes to it (NaN = hidden pane). */ +class FakeFit { + static proposed = { cols: 80, rows: 24 }; + term: FakeTerminal | null = null; + fit() { + const { cols, rows } = FakeFit.proposed; + if (!Number.isFinite(cols) || !Number.isFinite(rows)) return; + this.term?.resize(cols, rows); + } + proposeDimensions() { + return { ...FakeFit.proposed }; + } +} + class FakeTerminal { static last: FakeTerminal | null = null; options: Record; @@ -69,7 +83,9 @@ class FakeTerminal { this.options = { ...options }; FakeTerminal.last = this; } - loadAddon() {} + loadAddon(addon: FakeFit) { + addon.term = this; + } open() {} onData(cb: (data: string) => void) { this.dataCb = cb; @@ -84,7 +100,9 @@ class FakeTerminal { clear() { this.writes.push(''); } + resizes: Array<[number, number]> = []; resize(cols: number, rows: number) { + this.resizes.push([cols, rows]); this.cols = cols; this.rows = rows; } @@ -116,14 +134,7 @@ function loadContext() { HTMLCanvasElement: class HTMLCanvasElement {}, WebSocket: FakeSocket, Terminal: FakeTerminal, - FitAddon: { - FitAddon: class { - fit() {} - proposeDimensions() { - return { cols: 80, rows: 24 }; - } - }, - }, + FitAddon: { FitAddon: FakeFit }, fetch: (...args: unknown[]) => fetchMock(...args), location: { protocol: 'http:', host: 'codeman.test' }, document: { addEventListener: vi.fn(), documentElement: { dataset: {} } }, @@ -176,6 +187,8 @@ type Tile = { connect(): Promise; destroy(): void; reconnectNow(): void; + fit(opts?: { force?: boolean }): void; + detachedSessions?: Set; ws: FakeSocket | null; _reconnectAttempts: number; }; @@ -198,6 +211,7 @@ afterEach(() => { }); beforeEach(() => { + FakeFit.proposed = { cols: 80, rows: 24 }; FakeSocket.instances = []; fetchMock.mockReset(); fetchMock.mockImplementation(async () => ({ @@ -493,3 +507,93 @@ describe('TerminalTile destroy()', () => { expect(onExit).not.toHaveBeenCalled(); }); }); + +describe('TerminalTile geometry (#464: the pane and its PTY never disagree)', () => { + const resizeFrames = (ws: FakeSocket) => ws.sent.filter((f) => f.t === 'z'); + + it('announces its size as a desktop viewer when the socket opens', async () => { + const { ws } = await connectTile(makeApp()); + ws.open(); + + expect(resizeFrames(ws)).toEqual([{ t: 'z', c: 80, r: 24, v: 'desktop' }]); + }); + + it('does not resend an unchanged size, sends a changed one, and force resends', async () => { + const { tile, ws } = await connectTile(makeApp()); + ws.open(); + + tile.fit(); + expect(resizeFrames(ws)).toHaveLength(1); + + FakeFit.proposed = { cols: 100, rows: 30 }; + tile.fit(); + expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 100, r: 30, v: 'desktop' }); + + tile.fit({ force: true }); + expect(resizeFrames(ws)).toHaveLength(3); + }); + + it('re-announces an unchanged size on a reconnected socket', async () => { + vi.useFakeTimers(); + const { ws } = await connectTile(makeApp()); + ws.open(); + ws.onclose?.({ code: 1006 }); + await vi.advanceTimersByTimeAsync(300); + const ws2 = FakeSocket.instances[1]; + + ws2.open(); + + expect(resizeFrames(ws2)).toEqual([{ t: 'z', c: 80, r: 24, v: 'desktop' }]); + }); + + it('applies no 40-column floor: a pane at the divider clamp gets its real width on both sides', async () => { + const { tile, ws, term } = await connectTile(makeApp()); + ws.open(); + + FakeFit.proposed = { cols: 28, rows: 30 }; + tile.fit(); + + expect(term.cols).toBe(28); + expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 28, r: 30, v: 'desktop' }); + }); + + it('reports nothing while hidden (the fit addon measures NaN)', async () => { + const { tile, ws, term } = await connectTile(makeApp()); + ws.open(); + + FakeFit.proposed = { cols: NaN, rows: NaN }; + tile.fit(); + + expect(resizeFrames(ws)).toHaveLength(1); + expect([term.cols, term.rows]).toEqual([80, 24]); + }); + + it('stands aside for a session detached into its own window', async () => { + const { tile, ws } = await connectTile(makeApp(), { detachedSessions: new Set(['s-tile']) }); + ws.open(); + FakeFit.proposed = { cols: 120, rows: 40 }; + + tile.fit(); + + expect(resizeFrames(ws)).toEqual([]); + }); + + it('adopts the column count the PTY reports, keeping its own rows', async () => { + const { ws, term } = await connectTile(makeApp()); + ws.open(); + + ws.receive({ t: 'zc', c: 132, r: 50 }); + + expect([term.cols, term.rows]).toEqual([132, 24]); + }); + + it('leaves the pane alone when the PTY agrees on width', async () => { + const { ws, term } = await connectTile(makeApp()); + ws.open(); + const before = term.resizes.length; + + ws.receive({ t: 'zc', c: 80, r: 60 }); + + expect(term.resizes.length).toBe(before); + }); +});