fix(terminal): yield only the resize send, and take sizing back on redock

Review of the previous commit found four defects in it.

The guard sat above the local fit, so it suppressed a reflow as well as the
server write. tab-rail-resize performs its single settle-time refit through
sendResize and has no fallback for a truthy activeSessionId, so dragging the
rail stopped reflowing a detached session's terminal in the dashboard. The
mobile-keyboard guard fourteen lines below already draws the line correctly —
withhold the send, never the reflow — and the guard now sits after the fit.

_lastResizeDims is one value for the whole window, and both guards skip
updating it, so while a popup owns a session that value no longer describes
the PTY. _redock repaired it only for the active session. Pop out A, switch to
B, close the popup: selecting A later found unchanged dimensions, returned
"unchanged", and selectSession skipped its 400ms redraw wait — while the
server, comparing against the real pane, did resize and did raise SIGWINCH, so
the fetch painted the pre-redraw frame. _redock now clears the record on every
path, active or not.

_redock could also fire a resize for a session already gone: _onSessionDeleted
redocks before cleanup, so the id can be dead and the request is a guaranteed
404. It now checks the session still exists.

restoreTerminalSize — the header's redraw button and Ctrl+Shift+R — silently
did nothing for a detached session while still reporting success with
dimensions nothing was set to. It now says the session is sized by its own
window, where the same button works.

The `force` comment claimed a client-side dedupe that does not exist; the
deduplication is server-side against the real pane. Corrected to say what the
flag actually buys. The _redock doc comment now records that the function
writes to the server and is not idempotent.

Tests: _redock was the untested half and is the half three of these defects
sit in. It now has coverage for clearing the stale record on both the active
and inactive paths, re-asserting only for the session being shown, and staying
silent for a deleted session. The existing sendResize test now asserts the
local fit still runs.
This commit is contained in:
Michael Grundberg
2026-09-09 15:24:55 +02:00
parent 5ac516dd3b
commit 070e8da81b
3 changed files with 120 additions and 12 deletions
+87 -2
View File
@@ -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<typeof vi.fn> }).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<typeof vi.fn> }).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<string, unknown> } }).__CodemanApp;
}
describe('redock takes the sizing back', () => {
const CodemanApp = loadAppClass();
function makeDashboard(activeSessionId: string | null) {
const app = Object.create(CodemanApp.prototype) as Record<string, any>;
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();
});
});