diff --git a/src/web/public/app.js b/src/web/public/app.js index b495fb8c..617b22f9 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1332,7 +1332,11 @@ class CodemanApp { this._redock(id); } - /** Clear all dashboard-side detached state/timers for a session. */ + /** Clear all dashboard-side detached state/timers for a session, and take its + * sizing back: the popup owned the pane while it was open, so the dashboard's + * record of it is stale and the session it is showing needs re-measuring. + * ⚠️ Not idempotent — each call re-asserts, so a path that redocks twice for + * one close sends two SIGWINCHs. */ _redock(id) { const t = this._detachWatchTimers.get(id); if (t) { clearInterval(t); this._detachWatchTimers.delete(id); } @@ -1340,12 +1344,19 @@ class CodemanApp { this._detachOrphanStrikes.delete(id); this.detachedWindows.delete(id); this._markDetached(id, false); - // Sizing comes back with the session. While the popup owned it the dashboard - // sent no resizes, so the PTY still holds the popup's geometry; the session - // this window is actually showing has to be re-sized to this window, and - // `force` is required because the dimensions the dashboard last sent are - // the ones it is about to send again. - if (id === this.activeSessionId) { + // While the popup owned this session the dashboard sent no resizes, so + // `_lastResizeDims` — one value for the whole window — no longer describes + // the PTY, which the popup has been sizing. Clearing it makes the next + // sendResize report truthfully, on every redock path rather than only the + // active one: `selectSession` reads that answer to decide whether to wait + // for the TUI's redraw, and a false "unchanged" makes it fetch the frame + // before the redraw lands. + this._lastResizeDims = null; + // Sizing comes back with the session. `force` buys a guaranteed repaint for + // the case where popup and dashboard happened to agree on a size; the server + // already resizes on its own comparison against the real pane whenever the + // two differ. + if (this.sessions.has(id) && id === this.activeSessionId) { this.sendResize(id, { force: true })?.catch?.(() => {}); } } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 1c30ee80..b5b4d723 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -3846,6 +3846,14 @@ Object.assign(CodemanApp.prototype, { return; } + // The pane belongs to the popup showing it, so this window has nothing to + // restore. Say so rather than reporting a size that was never sent — the + // same button in that window does the job. + if (!this.isSoloWindow && this.detachedSessions?.has(this.activeSessionId)) { + this.showToast('This session is sized by its own window', 'warning'); + return; + } + const dims = this.getTerminalDimensions(); if (!dims) { this.showToast('Could not determine terminal size', 'error'); @@ -4827,16 +4835,20 @@ Object.assign(CodemanApp.prototype, { * @returns {Promise} Whether dimensions changed from the last send */ async sendResize(sessionId, options = {}) { + // Fit terminal to container before reading dimensions — ensures local + // terminal size matches what we report to the server PTY. + if (this.fitAddon) this.fitAddon.fit(); // One PTY cannot hold two sizes. A detached session is owned by its own // window, and the dashboard's terminal is narrower than that window because // the session rail takes width the popup does not have — so both sizing it // makes the CLI draw frames that fit neither, which garbles the popup. The // dashboard yields; the solo window sizes what it alone displays. // (_maybeRefetchFullHistory already stands aside for the same reason.) + // ⚠️ AFTER the fit, never before: the local reflow keeps the dashboard's own + // xterm right, and only the SERVER write is the dashboard's to withhold — + // the mobile-keyboard guard below draws exactly this line. tab-rail-resize + // performs its one settle-time refit through this call and has no fallback. if (!this.isSoloWindow && this.detachedSessions?.has(sessionId)) return false; - // Fit terminal to container before reading dimensions — ensures local - // terminal size matches what we report to the server PTY. - if (this.fitAddon) this.fitAddon.fit(); const dims = this.getTerminalDimensions(); if (!dims) return false; // Did the dimensions actually change since the last resize we sent? Callers diff --git a/test/detached-session-pane-sizing.test.ts b/test/detached-session-pane-sizing.test.ts index 65563691..c30e9eba 100644 --- a/test/detached-session-pane-sizing.test.ts +++ b/test/detached-session-pane-sizing.test.ts @@ -75,9 +75,12 @@ describe('detached sessions own their pane size', () => { const changed = await app.sendResize(SESSION); expect(changed).toBe(false); - // No measurement and no request: the popup's size stands. + // No request: the popup's size stands on the server. expect(fetchMock).not.toHaveBeenCalled(); - expect((app.fitAddon as { fit: ReturnType }).fit).not.toHaveBeenCalled(); + // The LOCAL fit still runs, so the dashboard's own xterm stays correct and + // tab-rail-resize's single settle-time refit is not swallowed. Same line the + // mobile-keyboard guard draws: withhold the send, never the reflow. + expect((app.fitAddon as { fit: ReturnType }).fit).toHaveBeenCalled(); }); it('the solo window still sizes the session it displays', async () => { @@ -97,3 +100,85 @@ describe('detached sessions own their pane size', () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); }); + +/** + * `_redock` lives on the CodemanApp class rather than the terminal mixin, so it + * needs app.js loaded. Same `vm` approach as terminal-flush-budget.test.ts. + */ +function loadAppClass() { + const dir = resolve(import.meta.dirname, '../src/web/public'); + const context = vm.createContext({ + console: { ...console, log: vi.fn(), warn: vi.fn(), error: vi.fn() }, + performance: { now: () => 0 }, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + WebSocket: { OPEN: 1 }, + fetch: vi.fn(), + document: { addEventListener: vi.fn(), getElementById: () => null, querySelector: () => null }, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + MobileDetection: { isTouchDevice: () => false }, + }); + const constants = readFileSync(resolve(dir, 'constants.js'), 'utf8'); + const appSource = readFileSync(resolve(dir, 'app.js'), 'utf8'); + vm.runInContext(`${constants}\n${appSource}\nglobalThis.__CodemanApp = CodemanApp;`, context); + return (context as { __CodemanApp: { prototype: Record } }).__CodemanApp; +} + +describe('redock takes the sizing back', () => { + const CodemanApp = loadAppClass(); + + function makeDashboard(activeSessionId: string | null) { + const app = Object.create(CodemanApp.prototype) as Record; + app.detachedSessions = new Set([SESSION]); + app.detachedWindows = new Map(); + app._detachWatchTimers = new Map(); + app._redockGrace = new Map(); + app._detachOrphanStrikes = new Map(); + app.sessions = new Map([[SESSION, { id: SESSION }]]); + app.activeSessionId = activeSessionId; + app._lastResizeDims = { cols: 120, rows: 40 }; + app.$ = () => null; + app.sendResize = vi.fn(() => Promise.resolve(true)); + return app; + } + + it('clears the stale dimensions so the next send reports truthfully', () => { + // The popup sized the pane while it owned the session, so this window's one + // global record of "what the PTY holds" is wrong. Left in place, the next + // sendResize returns "unchanged" and selectSession skips its redraw wait. + const app = makeDashboard(SESSION); + app._redock(SESSION); + expect(app._lastResizeDims).toBeNull(); + }); + + it('clears them even when the redocked session is not the active one', () => { + // Pop out A, switch to B, close the popup: no resize is due, but the stale + // record still has to go or selecting A later lies about it. + const app = makeDashboard('some-other-session'); + app._redock(SESSION); + expect(app._lastResizeDims).toBeNull(); + expect(app.sendResize).not.toHaveBeenCalled(); + }); + + it('re-asserts this window size for the session it is showing', () => { + const app = makeDashboard(SESSION); + app._redock(SESSION); + expect(app.sendResize).toHaveBeenCalledWith(SESSION, { force: true }); + // Un-marked first, or the yield in sendResize would swallow the re-assert. + expect(app.detachedSessions.has(SESSION)).toBe(false); + }); + + it('sends nothing for a session that is gone', () => { + // _onSessionDeleted redocks before cleanup, so the id can already be dead; + // the resize would be a guaranteed 404. + const app = makeDashboard(SESSION); + app.sessions.delete(SESSION); + app._redock(SESSION); + expect(app.sendResize).not.toHaveBeenCalled(); + }); +});