mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 06:59:42 +02:00
Merge pull request #431 from rounakdatta/feat/mobile-terminal-resilience
fix(terminal): four silent-failure paths — renderer freeze, replay race, reconnect gap, unbounded fetches
This commit is contained in:
@@ -136,9 +136,10 @@ async function stubTerminalDynamic(
|
||||
/**
|
||||
* Answer every fetch with the geometry the client itself is asking for, read
|
||||
* live from the page. That is the clamp signature: `getTerminalDimensions()`
|
||||
* floors at 40x10 while `fitAddon.fit()` does not, so a small enough viewport
|
||||
* makes the pane permanently bigger than the terminal at a size the client
|
||||
* requested itself.
|
||||
* floors at 40x10, and since #464 `syncTerminalGeometry()` applies that floor
|
||||
* to xterm too — so at a small enough viewport the pane, the report and the
|
||||
* terminal all agree on the floored size, which is the case the equality guard
|
||||
* is left covering.
|
||||
*/
|
||||
async function stubTerminalAtRequestedSize(page: Page, counter: { n: number; urls: string[] }) {
|
||||
await page.route('**/api/sessions/*/terminal*', async (route) => {
|
||||
@@ -498,13 +499,22 @@ describe('a capture bigger than the terminal', () => {
|
||||
}, 60_000);
|
||||
|
||||
it('does not replay a pane already at the size the client asked for', async () => {
|
||||
// `getTerminalDimensions()` floors at 40x10 while `fitAddon.fit()` does
|
||||
// not, so a viewport this small leaves the terminal shorter than the size
|
||||
// the client itself requests, and the pane obligingly draws at the floored
|
||||
// size. The captured height then exceeds the terminal's forever. A replay
|
||||
// cannot converge, because it re-requests the same floored size and
|
||||
// captures the same frame, so without the equality guard this retries on
|
||||
// every tab switch for the life of the page.
|
||||
// ⚠️ The premise of this case CHANGED with issue #464, and the old one can
|
||||
// never hold again. It used to be the clamp: `getTerminalDimensions()`
|
||||
// floors at 40x10 while `fitAddon.fit()` did not, so a viewport this small
|
||||
// left the terminal shorter than the size the client itself requested, the
|
||||
// pane drew at the floored size, and the captured height exceeded the
|
||||
// terminal's forever — a replay that re-requested the same floored size and
|
||||
// captured the same frame, on every tab switch, for the life of the page.
|
||||
//
|
||||
// `syncTerminalGeometry()` now applies the floor to xterm as well, so the
|
||||
// browser terminal IS the size it reports and that divergence is gone at
|
||||
// the source. The case survives on its own terms — a pane already drawing
|
||||
// at the requested size must not be replayed, because the retry would
|
||||
// capture the identical frame — and its premise is now the #464 invariant
|
||||
// itself, asserted below: the floored report and the terminal agree. That
|
||||
// is a stronger guard than the old one, since the clamp coming back would
|
||||
// fail it here rather than silently restoring the replay loop.
|
||||
context = await browser.newContext({ viewport: { width: 320, height: 200 } });
|
||||
page = await context.newPage();
|
||||
const sessionId = await openSession(page);
|
||||
@@ -514,8 +524,9 @@ describe('a capture bigger than the terminal', () => {
|
||||
await consumeFullHistory(page, sessionId, fetches);
|
||||
await select(page, sessionId, { forceReload: true });
|
||||
|
||||
// The premise: the floor really does bind here. Without this the case
|
||||
// would pass on any viewport, proving nothing.
|
||||
// The premise: the floor really does bind at this viewport — otherwise the
|
||||
// case would pass on any viewport, proving nothing — AND the terminal holds
|
||||
// exactly what it reports, which is what stops the old replay loop.
|
||||
const requested = await page.evaluate(
|
||||
() =>
|
||||
(
|
||||
@@ -523,7 +534,20 @@ describe('a capture bigger than the terminal', () => {
|
||||
).app.getTerminalDimensions?.() ?? null
|
||||
);
|
||||
expect(requested).not.toBeNull();
|
||||
expect(requested!.rows).toBeGreaterThan(await terminalRows(page));
|
||||
const proposed = await page.evaluate(
|
||||
() =>
|
||||
(
|
||||
window as unknown as { app: { fitAddon?: { proposeDimensions?: () => { cols: number; rows: number } } } }
|
||||
).app.fitAddon?.proposeDimensions?.() ?? null
|
||||
);
|
||||
expect(proposed, 'the terminal could not be measured').not.toBeNull();
|
||||
expect(
|
||||
proposed!.rows < requested!.rows || proposed!.cols < requested!.cols,
|
||||
`the floor must bind at this viewport, or the case proves nothing (proposed ${proposed!.cols}x${proposed!.rows}, reported ${requested!.cols}x${requested!.rows})`
|
||||
).toBe(true);
|
||||
// The #464 invariant: what the client reports is what the terminal holds.
|
||||
expect(requested!.rows).toBe(await terminalRows(page));
|
||||
expect(requested!.cols).toBe(await terminalCols(page));
|
||||
|
||||
expect(fetches.n).toBe(1);
|
||||
|
||||
|
||||
@@ -14,6 +14,12 @@
|
||||
* already stood aside on the same condition, so this follows a rule the code
|
||||
* had already established.
|
||||
*
|
||||
* ⚠️ It returns early BEFORE the local fit, not after (issue #464). The earlier
|
||||
* rule was "withhold the send, never the reflow", which leaves this window's
|
||||
* xterm at a shape the PTY was never told about — and a CLI computes its
|
||||
* repaints from the shape it was told, so that reflow bought a garbled frame
|
||||
* rather than a correct one. Withhold both, or neither.
|
||||
*
|
||||
* Loaded via `vm` with a stubbed context (no jsdom — jsdom is broken on this
|
||||
* box; see connection-indicator.test.ts), the same way terminal-buffer-flush
|
||||
* extracts the real mixin methods from terminal-ui.js.
|
||||
@@ -56,7 +62,18 @@ function makeApp(overrides: Record<string, unknown> = {}) {
|
||||
currentFetch = fetchMock;
|
||||
const app = {
|
||||
sendResize: mixin.sendResize,
|
||||
// The real chain: sendResize fits, floors and applies through one function
|
||||
// now, so the harness must let it (#464).
|
||||
syncTerminalGeometry: mixin.syncTerminalGeometry,
|
||||
_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
|
||||
// it measures nothing and settles on "no overflow", which is the truth here.
|
||||
_scheduleOverflowAffordanceSync: mixin._scheduleOverflowAffordanceSync,
|
||||
_syncTerminalOverflowAffordance: mixin._syncTerminalOverflowAffordance,
|
||||
_onPtyGeometryReport: vi.fn(),
|
||||
getTerminalDimensions: () => ({ cols: 120, rows: 40 }),
|
||||
terminal: { cols: 120, rows: 40, resize: vi.fn() },
|
||||
fitAddon: { fit: vi.fn() },
|
||||
detachedSessions: new Set<string>(),
|
||||
isSoloWindow: false,
|
||||
@@ -77,10 +94,16 @@ describe('detached sessions own their pane size', () => {
|
||||
expect(changed).toBe(false);
|
||||
// No request: the popup's size stands on the server.
|
||||
expect(fetchMock).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();
|
||||
// ⚠️ REVERSED by issue #464, deliberately. This used to assert that the
|
||||
// LOCAL fit still ran — "withhold the send, never the reflow" — on the
|
||||
// reasoning that it keeps the dashboard's own xterm correct. It does not:
|
||||
// it leaves this xterm at a shape the PTY was never told about, and Claude
|
||||
// Code computes every repaint from the shape it WAS told, so the frames
|
||||
// land on rows nothing erased. The popup that owns the PTY is drawing for
|
||||
// its own width either way, so the dashboard's reflow was a reflow nothing
|
||||
// was rendering for. Withholding the resize means withholding all of it.
|
||||
expect((app.fitAddon as { fit: ReturnType<typeof vi.fn> }).fit).not.toHaveBeenCalled();
|
||||
expect((app.terminal as { resize: ReturnType<typeof vi.fn> }).resize).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('the solo window still sizes the session it displays', async () => {
|
||||
|
||||
@@ -292,6 +292,10 @@ function loadRealSelectSessionHarness(options: { terminalFailure?: boolean } = {
|
||||
app._beginBufferLoad = vi.fn(() => 1);
|
||||
app._isLoadingBuffer = false;
|
||||
app.fitAddon = { fit: vi.fn() };
|
||||
// selectSession fits through the one owner now, which also applies the floor
|
||||
// it reports to the PTY (#464). Without it on the fake, the unconditional
|
||||
// call throws into selectSession's catch and nothing after it runs.
|
||||
app.syncTerminalGeometry = vi.fn(() => ({ cols: 120, rows: 40 }));
|
||||
app.sendResize = vi.fn(() => {
|
||||
resizeCalls++;
|
||||
return resizeCalls === 1 ? terminalBoundary.promise : Promise.resolve(false);
|
||||
|
||||
@@ -127,7 +127,11 @@ describe('the in-terminal truncation line is gone (static guard)', () => {
|
||||
expect(app).toContain("session?.mode !== 'shell' && !this._fullHistoryLoaded.has(sessionId)");
|
||||
expect(app).toContain("!restoredSnapshot && session?.mode !== 'shell'");
|
||||
expect(app).toContain('`/api/sessions/${sessionId}/terminal?tail=${TERMINAL_TAIL_SIZE}`');
|
||||
expect(app).toContain('fetch(`/api/sessions/${sessionId}/terminal?full=1`)');
|
||||
// Every terminal capture now goes through _fetchTerminalCapture, which adds
|
||||
// an abort deadline (a `?full=1` body can be megabytes and used to hang
|
||||
// indefinitely on a stalled mobile link). The URL and the full-vs-tail
|
||||
// decision this guard exists to pin are unchanged.
|
||||
expect(app).toContain('this._fetchTerminalCapture(`/api/sessions/${sessionId}/terminal?full=1`, { full: true })');
|
||||
expect(app).toContain("if (this.sessions.get(sessionId)?.mode !== 'shell')");
|
||||
expect(app).toContain("if (session?.mode === 'shell')");
|
||||
expect(app).toContain("if (!force && session?.mode === 'shell') return;");
|
||||
|
||||
@@ -223,7 +223,12 @@ describe('mobile prompt composer', () => {
|
||||
|
||||
it('wires session cleanup to composer draft cleanup', () => {
|
||||
const cleanupStart = appSource.indexOf(' _cleanupSessionData(sessionId) {');
|
||||
const cleanup = appSource.slice(cleanupStart, cleanupStart + 1200);
|
||||
expect(cleanupStart, '_cleanupSessionData not found — renamed?').toBeGreaterThan(-1);
|
||||
// The whole method, not a fixed byte window. A 1200-character slice made
|
||||
// this assertion depend on how much OTHER code sat above the line it cares
|
||||
// about, so an unrelated addition near the top of the method failed it.
|
||||
const cleanup = appSource.slice(cleanupStart, appSource.indexOf('\n }\n', cleanupStart));
|
||||
expect(cleanup.length, 'method body did not terminate').toBeGreaterThan(0);
|
||||
|
||||
expect(cleanup).toContain('KeyboardAccessoryBar.discardComposerDraft?.(sessionId)');
|
||||
});
|
||||
|
||||
@@ -19,6 +19,37 @@ function attachFakePty(session: Session, cols = 160, rows = 48) {
|
||||
return resize;
|
||||
}
|
||||
|
||||
describe('Session.ptyGeometry', () => {
|
||||
// ⚠️ `resize()` writes `_ptyCols`/`_ptyRows` only when `ptyProcess` is set,
|
||||
// and nothing seeds them from the spawn geometry — so the fields hold the
|
||||
// constructor defaults of 120x40 for any session whose pane is not running.
|
||||
// Reporting those to a client made it adopt a width no process had ever been
|
||||
// told, and on anything narrower than 120 columns claim another device owned
|
||||
// the pane when none existed (issue #464).
|
||||
it('reports nothing for a session that has no pane', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'shell' });
|
||||
expect(session.ptyGeometry).toBeNull();
|
||||
});
|
||||
|
||||
it('still reports nothing after a resize it could not apply', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'shell' });
|
||||
session.resize(45, 20, { viewportType: 'mobile' });
|
||||
// The resize was swallowed (no pty to resize), so there is no geometry to
|
||||
// report — NOT the 120x40 the fields still hold.
|
||||
expect(session.ptyGeometry).toBeNull();
|
||||
});
|
||||
|
||||
it('reports the pane geometry once a pane exists, and follows a resize', () => {
|
||||
// The contrast, so "always null" would fail this.
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'shell' });
|
||||
attachFakePty(session, 160, 48);
|
||||
expect(session.ptyGeometry).toEqual({ cols: 160, rows: 48 });
|
||||
|
||||
session.resize(62, 40, { viewportType: 'mobile' });
|
||||
expect(session.ptyGeometry).toEqual({ cols: 62, rows: 40 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('Session resize arbitration', () => {
|
||||
it('lets a mobile-only session shrink below the spawn default (no desktop connected)', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'shell' });
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
// Port: none (static source contract — no browser, no server).
|
||||
//
|
||||
// The service worker's precache list used to be maintained by hand with the
|
||||
// PRE-hash filenames, while scripts/build.mjs renamed those same files to
|
||||
// content-hashed names and rewrote only index.html. So in production every
|
||||
// precache entry pointed at a file that no longer existed, and
|
||||
// `cache.add(url).catch(() => {})` in the install handler swallowed all of it.
|
||||
// Measured against a running instance: 15 of 23 entries 404'd.
|
||||
//
|
||||
// Nothing caught it because nothing could: the two lists lived in different
|
||||
// files, in different languages, with no shared symbol. The fix is to derive
|
||||
// the list from the build's own manifest — and this test pins the contract that
|
||||
// makes that derivation possible, because the failure mode is silent in both
|
||||
// directions. A renamed anchor in sw.js means the build throws (loud, fine). A
|
||||
// build that stops rewriting means the worker precaches nothing while still
|
||||
// looking correct (silent, not fine).
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
const root = resolve(import.meta.dirname, '..');
|
||||
const sw = readFileSync(resolve(root, 'src/web/public/sw.js'), 'utf8');
|
||||
const build = readFileSync(resolve(root, 'scripts/build.mjs'), 'utf8');
|
||||
|
||||
// The exact declarations scripts/build.mjs rewrites. They must appear EXACTLY
|
||||
// once: the build asserts the same thing and throws otherwise, so a second
|
||||
// occurrence (in a comment, say) fails the build rather than shipping stale.
|
||||
const BUILD_ID_ANCHOR = "const BUILD_ID = 'dev';";
|
||||
const HASHED_ASSETS_ANCHOR = 'const HASHED_ASSETS = [];';
|
||||
|
||||
describe('service worker precache contract', () => {
|
||||
it('sw.js carries exactly one of each anchor the build rewrites', () => {
|
||||
expect(sw.split(BUILD_ID_ANCHOR).length - 1).toBe(1);
|
||||
expect(sw.split(HASHED_ASSETS_ANCHOR).length - 1).toBe(1);
|
||||
});
|
||||
|
||||
it('build.mjs rewrites those exact anchors', () => {
|
||||
expect(build).toContain(BUILD_ID_ANCHOR);
|
||||
expect(build).toContain(HASHED_ASSETS_ANCHOR);
|
||||
});
|
||||
|
||||
// The cache key must vary per build, or `activate`'s cleanup — which deletes
|
||||
// every cache whose key is not the current one — never deletes anything, and
|
||||
// hashed assets from every past release accumulate until the origin hits its
|
||||
// storage quota. That is what the old constant 'codeman-v1' did.
|
||||
it('derives the cache name from the build id rather than a constant', () => {
|
||||
expect(sw).toContain('const CACHE_NAME = `codeman-${BUILD_ID}`;');
|
||||
expect(sw).not.toMatch(/const CACHE_NAME = ['"]codeman-v\d+['"]/);
|
||||
});
|
||||
|
||||
// The whole point of the rewrite: the shell is derived, not hand-listed.
|
||||
it('builds the app shell from the hashed manifest', () => {
|
||||
expect(sw).toContain("...HASHED_ASSETS.map((p) => '/' + p)");
|
||||
});
|
||||
|
||||
// The regression itself: any pre-hash filename hand-listed in APP_SHELL will
|
||||
// 404 in production, because the build renames it.
|
||||
//
|
||||
// The HASHABLE list is PARSED out of scripts/build.mjs rather than copied
|
||||
// here. A hand-kept duplicate would be the same drift this whole PR exists to
|
||||
// fix — it would go stale the first time someone adds an asset to the build,
|
||||
// and then silently stop covering it.
|
||||
it('never hand-lists a filename the build content-hashes', () => {
|
||||
const block = build.slice(
|
||||
build.indexOf('const HASHABLE = ['),
|
||||
build.indexOf('];', build.indexOf('const HASHABLE = ['))
|
||||
);
|
||||
const hashedByBuild = [...block.matchAll(/'([^']+)'/g)].map((m) => m[1]);
|
||||
// Guard the parse itself: an empty list would make this test vacuously pass.
|
||||
expect(hashedByBuild.length, 'failed to parse HASHABLE out of scripts/build.mjs').toBeGreaterThan(10);
|
||||
expect(hashedByBuild).toContain('app.js');
|
||||
|
||||
const shell = sw.slice(sw.indexOf('const APP_SHELL'), sw.indexOf('].map(B);'));
|
||||
for (const name of hashedByBuild) {
|
||||
expect(shell, `APP_SHELL must not hand-list ${name} — the build renames it`).not.toContain(`'/${name}'`);
|
||||
}
|
||||
});
|
||||
|
||||
// Dev serves sw.js unrewritten, so the literals must be valid on their own:
|
||||
// an empty precache plus the unhashed modules cached on first use.
|
||||
it('is valid unrewritten, for dev', () => {
|
||||
expect(() => new Function(sw.replace(/self\./g, 'globalThis.'))).not.toThrow();
|
||||
});
|
||||
|
||||
// Without ignoreSearch the whole precache is unreachable, which is subtle
|
||||
// enough to be re-broken by anyone tidying this handler.
|
||||
//
|
||||
// `renderIndexHtml` runs `cacheBustAssets`, which appends `?v=<mtime>` to
|
||||
// EVERY same-origin `.js`/`.css` reference — content-hashed names included.
|
||||
// Observed on a running instance: `src="app.556be563.js?v=1789423735875"`.
|
||||
// `caches.match` is query-sensitive by default, so a precache keyed on
|
||||
// `/app.556be563.js` can never serve that request, and the install would be
|
||||
// downloading ~1.3MB per deploy that nothing can ever read back.
|
||||
it('falls back to the cache ignoring the cache-busting query string', () => {
|
||||
expect(sw).toContain('caches.match(request, { ignoreSearch: true })');
|
||||
expect(sw, 'a bare caches.match(request) cannot match the ?v=<mtime> URLs cacheBustAssets emits').not.toMatch(
|
||||
/caches\.match\(request\)\s*\)/
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -177,7 +177,11 @@ describe('selectSession font gate', () => {
|
||||
|
||||
it('waits for the font before the first fit', () => {
|
||||
const wait = body.indexOf('await this._terminalFontReady');
|
||||
const fit = body.indexOf('if (this.fitAddon) this.fitAddon.fit();');
|
||||
// `syncTerminalGeometry()` replaced the bare `fitAddon.fit()` here: it fits
|
||||
// AND applies the floor it reports, so xterm and the PTY cannot disagree
|
||||
// (#464). The gate this test guards is unchanged — the font must be
|
||||
// measured before the terminal is.
|
||||
const fit = body.indexOf('this.syncTerminalGeometry();');
|
||||
expect(wait).toBeGreaterThan(-1);
|
||||
expect(fit).toBeGreaterThan(-1);
|
||||
expect(wait).toBeLessThan(fit);
|
||||
|
||||
@@ -74,6 +74,18 @@ function makeApp(opts: { teammates?: number; terminal?: ReturnType<typeof fakeTe
|
||||
}
|
||||
const app = {
|
||||
applyTerminalFontWeights: mixin.applyTerminalFontWeights,
|
||||
// The REAL geometry chain, not stubs. A font change moves the cell size, so
|
||||
// it moves cols/rows, and `applyTerminalFontWeights` now routes its refit
|
||||
// through the one function that floors the result and reports it (#464).
|
||||
// Wiring the real methods keeps `fit` an assertion about what the terminal
|
||||
// actually did rather than about which helper happened to be called.
|
||||
_refitAfterCellSizeChange: mixin._refitAfterCellSizeChange,
|
||||
syncTerminalGeometry: mixin.syncTerminalGeometry,
|
||||
_resizeTerminalTo: mixin._resizeTerminalTo,
|
||||
getTerminalDimensions: mixin.getTerminalDimensions,
|
||||
// No session: `_refitAfterCellSizeChange` then refits locally and sends
|
||||
// nothing, which is what these cases are about.
|
||||
activeSessionId: null,
|
||||
_awaitTerminalFont: vi.fn(() => Promise.resolve()),
|
||||
terminal: opts.terminal === undefined ? fakeTerminal() : opts.terminal,
|
||||
fitAddon: { fit },
|
||||
|
||||
@@ -0,0 +1,465 @@
|
||||
// Port: none (pure helpers + a real headless xterm + source guards).
|
||||
//
|
||||
// Issue #464, "text gets muffled sometimes". The report is a phone screenshot
|
||||
// where lines of Claude Code's output are rendered twice and short tool
|
||||
// summaries sit inside longer prose rows with the prose's tail still showing.
|
||||
//
|
||||
// That is not a dropped frame or a frozen renderer; it is arithmetic. Ink 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 narrower than the PTY makes each logical line occupy more
|
||||
// physical rows than Ink counted, so `eraseLines(n)` clears too few of them and
|
||||
// the new frame paints over rows that were never erased.
|
||||
//
|
||||
// `renders each wrapped line twice when the PTY is wider` below reproduces it
|
||||
// against the repo's own xterm, and is written as a CONTRAST: the same stream at
|
||||
// a matching width must come out clean. An implementation that stopped fixing
|
||||
// anything would fail the second half, not quietly satisfy the first.
|
||||
//
|
||||
// The rest pins the invariant the fix rests on: there is exactly ONE function
|
||||
// that changes the terminal's size, it applies the same floor it reports, and
|
||||
// the server reports back the geometry the PTY actually holds so a client whose
|
||||
// resize was declined can adopt it instead of rendering against a screen that
|
||||
// does not exist.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import xtermHeadless from '@xterm/headless';
|
||||
|
||||
const { Terminal } = xtermHeadless as unknown as {
|
||||
Terminal: new (opts: Record<string, unknown>) => {
|
||||
write(data: string, cb?: () => void): void;
|
||||
buffer: {
|
||||
active: { length: number; getLine(y: number): { translateToString(trim?: boolean): string } | undefined };
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
const read = (rel: string) => readFileSync(resolve(import.meta.dirname, '..', rel), 'utf8');
|
||||
|
||||
type Dims = { cols: number; rows: number };
|
||||
|
||||
function loadGeometry() {
|
||||
const context = vm.createContext({ window: {}, globalThis: {} });
|
||||
vm.runInContext(read('src/web/public/constants.js'), context, { filename: 'constants.js' });
|
||||
return (
|
||||
context.window as {
|
||||
CodemanTerminalGeometry: {
|
||||
clampTerminalDimensions: (p: Partial<Dims> | null | undefined) => Dims | null;
|
||||
terminalGeometryAgrees: (a: Dims | null, b: Dims | null) => boolean;
|
||||
reconcilePtyGeometry: (local: Dims | null, pty: Partial<Dims> | null) => { adopt: boolean; oversized: boolean };
|
||||
TERMINAL_MIN_COLS: number;
|
||||
TERMINAL_MIN_ROWS: number;
|
||||
};
|
||||
}
|
||||
).CodemanTerminalGeometry;
|
||||
}
|
||||
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
// The failure itself, against the real terminal.
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/** ansi-escapes `eraseLines(n)`: \x1b[2K per row walking up, then column 1. */
|
||||
function eraseLines(n: number): string {
|
||||
let out = '';
|
||||
for (let i = 0; i < n; i++) out += '\x1b[2K' + (i < n - 1 ? '\x1b[1A' : '');
|
||||
return n ? out + '\x1b[G' : '';
|
||||
}
|
||||
|
||||
/** How many physical rows Ink thinks its frame took, wrapping at `cols`. */
|
||||
const rowsAt = (frame: string[], cols: number) =>
|
||||
frame.reduce((n, line) => n + Math.max(1, Math.ceil(line.length / cols)), 0);
|
||||
|
||||
/**
|
||||
* Ink's repaint loop: erase the previous frame, write the new one. The erase
|
||||
* count is computed at `ptyCols` — the width the PTY told the CLI about —
|
||||
* while the terminal is `xtermCols` wide.
|
||||
*/
|
||||
function inkStream(frames: string[][], ptyCols: number): string {
|
||||
let out = '';
|
||||
let previousRows = 0;
|
||||
for (const frame of frames) {
|
||||
out += eraseLines(previousRows) + frame.join('\r\n');
|
||||
previousRows = rowsAt(frame, ptyCols);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
async function render(data: string, cols: number, rows = 24): Promise<string[]> {
|
||||
const term = new Terminal({ cols, rows, allowProposedApi: true, scrollback: 500 });
|
||||
await new Promise<void>((done) => term.write(data, () => done()));
|
||||
const buf = term.buffer.active;
|
||||
const lines: string[] = [];
|
||||
for (let y = 0; y < buf.length; y++) lines.push(buf.getLine(y)?.translateToString(true) ?? '');
|
||||
while (lines.length && lines[lines.length - 1] === '') lines.pop();
|
||||
return lines;
|
||||
}
|
||||
|
||||
describe('a terminal that disagrees with the PTY about width', () => {
|
||||
const XTERM_COLS = 62;
|
||||
// Prose long enough to wrap, then a live region that shrinks as tool calls
|
||||
// collapse into one-line summaries — ordinary Claude Code output.
|
||||
const PROSE = [
|
||||
"• Password store entries exist, but GPG can't decrypt — that's the locked keyring after a pod restart. Let me get the browsers sorted.",
|
||||
];
|
||||
const FRAMES = [
|
||||
[...PROSE, ' Reading settings, scanning the pass store and checking whether the agent can reach AWS'],
|
||||
[...PROSE, ' Ran 1 shell command'],
|
||||
];
|
||||
|
||||
it('renders each wrapped line twice when the PTY is wider', async () => {
|
||||
const lines = await render(inkStream(FRAMES, 120), XTERM_COLS);
|
||||
const duplicated = lines.filter((line, i) => line !== '' && lines.indexOf(line) !== i);
|
||||
expect(
|
||||
duplicated.length,
|
||||
`a 120-column PTY against a ${XTERM_COLS}-column terminal must leave ghost rows:\n${lines.join('\n')}`
|
||||
).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// The contrast. Without this half, an implementation that fixed nothing —
|
||||
// or a stream that never ghosted in the first place — would still pass above.
|
||||
it('renders each line exactly once when the two agree', async () => {
|
||||
const lines = await render(inkStream(FRAMES, XTERM_COLS), XTERM_COLS);
|
||||
const duplicated = lines.filter((line, i) => line !== '' && lines.indexOf(line) !== i);
|
||||
expect(duplicated, `matched widths must render cleanly:\n${lines.join('\n')}`).toEqual([]);
|
||||
// And the frame that actually won is the last one.
|
||||
expect(lines[lines.length - 1]).toBe(' Ran 1 shell command');
|
||||
});
|
||||
});
|
||||
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
// The decisions, pure.
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('clampTerminalDimensions', () => {
|
||||
const { clampTerminalDimensions, TERMINAL_MIN_COLS, TERMINAL_MIN_ROWS } = loadGeometry();
|
||||
|
||||
it('floors a proposal too small to be a usable PTY', () => {
|
||||
expect(clampTerminalDimensions({ cols: 12, rows: 4 })).toEqual({
|
||||
cols: TERMINAL_MIN_COLS,
|
||||
rows: TERMINAL_MIN_ROWS,
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves a proposal that already clears the floor alone', () => {
|
||||
expect(clampTerminalDimensions({ cols: 62, rows: 40 })).toEqual({ cols: 62, rows: 40 });
|
||||
});
|
||||
|
||||
it('floors each axis independently — a short phone is not a narrow one', () => {
|
||||
// The everyday case behind #464: keyboard up, plenty of columns, under ten rows.
|
||||
expect(clampTerminalDimensions({ cols: 62, rows: 6 })).toEqual({ cols: 62, rows: TERMINAL_MIN_ROWS });
|
||||
});
|
||||
|
||||
it('reports nothing rather than a guess when the terminal cannot be measured', () => {
|
||||
for (const bad of [null, undefined, {}, { cols: NaN, rows: 10 }, { cols: 40, rows: Infinity }]) {
|
||||
expect(clampTerminalDimensions(bad as Partial<Dims>)).toBeNull();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('reconcilePtyGeometry', () => {
|
||||
const { reconcilePtyGeometry } = loadGeometry();
|
||||
|
||||
it('does nothing when the terminal already has the PTY\u2019s width', () => {
|
||||
expect(reconcilePtyGeometry({ cols: 62, rows: 40 }, { cols: 62, rows: 40 })).toEqual({ adopt: false, cols: null });
|
||||
});
|
||||
|
||||
it('adopts a width the client never asked for \u2014 a declined resize is still the truth', () => {
|
||||
// Session.resize ignores a small viewport while a desktop claim is live.
|
||||
expect(reconcilePtyGeometry({ cols: 62, rows: 40 }, { cols: 120, rows: 40 })).toEqual({ adopt: true, cols: 120 });
|
||||
});
|
||||
|
||||
// \u26a0\ufe0f The regression this pins: adopting the PTY's ROWS put a phone that took
|
||||
// a desktop's 43 into a viewport with room for 18, which painted the CLI's
|
||||
// input line below the container with nothing able to scroll to it. Width is
|
||||
// the axis the wrap arithmetic needs; rows only decide how much is on screen.
|
||||
it('never asks for the PTY\u2019s rows, however far off they are', () => {
|
||||
for (const ptyRows of [43, 4, 400]) {
|
||||
const out = reconcilePtyGeometry({ cols: 62, rows: 18 }, { cols: 120, rows: ptyRows });
|
||||
expect(out).toEqual({ adopt: true, cols: 120 });
|
||||
expect(out).not.toHaveProperty('rows');
|
||||
}
|
||||
});
|
||||
|
||||
it('does nothing when only the rows differ', () => {
|
||||
expect(reconcilePtyGeometry({ cols: 62, rows: 40 }, { cols: 62, rows: 12 })).toEqual({ adopt: false, cols: null });
|
||||
});
|
||||
|
||||
it('adopts a narrower PTY too \u2014 the width it was told is the width it draws for', () => {
|
||||
expect(reconcilePtyGeometry({ cols: 120, rows: 40 }, { cols: 80, rows: 40 })).toEqual({ adopt: true, cols: 80 });
|
||||
});
|
||||
|
||||
it('keeps its own geometry when the server reported none', () => {
|
||||
// A session with no pane answers `{}` (Session.ptyGeometry is null), and an
|
||||
// older server answers `{}` too. Neither is evidence about any PTY.
|
||||
for (const bad of [null, {}, { rows: 40 }, { cols: 'wide', rows: 40 }]) {
|
||||
expect(reconcilePtyGeometry({ cols: 62, rows: 40 }, bad as Partial<Dims>)).toEqual({ adopt: false, cols: null });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
// One owner of the terminal's size. These are source guards because the code
|
||||
// they cover needs a real DOM (FitAddon measures a rendered element), which
|
||||
// the CI gate has no way to give it.
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('exactly one function may change the terminal size', () => {
|
||||
const terminalUi = read('src/web/public/terminal-ui.js');
|
||||
const mobileHandlers = read('src/web/public/mobile-handlers.js');
|
||||
|
||||
function bodyOf(source: string, signature: string): string {
|
||||
const start = source.indexOf(signature);
|
||||
expect(start, `${signature} not found — renamed?`).toBeGreaterThan(-1);
|
||||
const end = source.indexOf('\n },', start);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
return source.slice(start, end);
|
||||
}
|
||||
|
||||
it('syncTerminalGeometry applies the floor it reports, not the raw proposal', () => {
|
||||
const body = bodyOf(terminalUi, 'syncTerminalGeometry() {');
|
||||
expect(body).toContain('this.fitAddon.fit()');
|
||||
// fit() resizes to proposeDimensions() RAW; the floored value is what goes
|
||||
// to the server, so the floored value is what xterm must end up holding.
|
||||
expect(body).toContain('this.getTerminalDimensions()');
|
||||
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
|
||||
// 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\(\)/;
|
||||
const offenders: string[] = [];
|
||||
for (const rel of [
|
||||
'src/web/public/terminal-ui.js',
|
||||
'src/web/public/mobile-handlers.js',
|
||||
'src/web/public/app.js',
|
||||
'src/web/public/ralph-panel.js',
|
||||
'src/web/public/settings-ui.js',
|
||||
'src/web/public/tab-rail-resize.js',
|
||||
'src/web/public/notification-manager.js',
|
||||
]) {
|
||||
read(rel)
|
||||
.split('\n')
|
||||
.forEach((line, i) => {
|
||||
const code = line.trim();
|
||||
if (code.startsWith('*') || code.startsWith('//')) return; // prose about fit(), not a call
|
||||
if (MAIN_TERMINAL_FIT.test(line)) offenders.push(`${rel}:${i + 1} ${code}`);
|
||||
});
|
||||
}
|
||||
// Exactly one: the owner's own fit.
|
||||
expect(
|
||||
offenders,
|
||||
'every fit of the main terminal must go through syncTerminalGeometry(), which applies ' +
|
||||
'the same floor it reports — a bare fit() leaves xterm at the RAW proposal while the ' +
|
||||
'server is told the floored one (issue #464)'
|
||||
).toHaveLength(1);
|
||||
expect(offenders[0]).toContain('terminal-ui.js');
|
||||
expect(bodyOf(terminalUi, 'syncTerminalGeometry() {')).toContain('this.fitAddon.fit()');
|
||||
expect(mobileHandlers).toContain('app.syncTerminalGeometry?.()');
|
||||
});
|
||||
|
||||
it('a font change tells the server, because it moves the cell size', () => {
|
||||
// Bigger glyphs mean fewer columns in the same box. These three refitted
|
||||
// and sent nothing, so the CLI kept wrapping at the old column count.
|
||||
for (const setter of [
|
||||
'setFontSize(size) {',
|
||||
'this.terminal.options.fontFamily === resolved',
|
||||
'this.terminal.options.fontWeight === fontWeight',
|
||||
]) {
|
||||
expect(terminalUi, `${setter} no longer present`).toContain(setter);
|
||||
}
|
||||
expect(bodyOf(terminalUi, 'setFontSize(size) {')).toContain('this._refitAfterCellSizeChange()');
|
||||
const helper = bodyOf(terminalUi, '_refitAfterCellSizeChange() {');
|
||||
expect(helper).toContain('this.sendResize(this.activeSessionId)');
|
||||
expect(helper).toContain('this.syncTerminalGeometry()');
|
||||
// Three call sites in the font setters (size, family, weight) plus the two
|
||||
// font-settle re-fits.
|
||||
expect((terminalUi.match(/_refitAfterCellSizeChange\(\)/g) ?? []).length).toBeGreaterThanOrEqual(6);
|
||||
});
|
||||
|
||||
it('the keyboard one-shot delegates rather than computing its own numbers', () => {
|
||||
const body = bodyOf(mobileHandlers, '_sendTerminalResize() {');
|
||||
expect(body).toContain('app.sendResize');
|
||||
// The hand-rolled POST floored what it sent and nothing else.
|
||||
expect(body).not.toContain('proposeDimensions');
|
||||
expect(body).not.toContain('Math.max');
|
||||
expect(body).not.toContain('fetch(');
|
||||
});
|
||||
|
||||
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()');
|
||||
expect(yieldAt, 'the detached-session yield is gone').toBeGreaterThan(-1);
|
||||
expect(fitAt, 'sendResize no longer syncs geometry').toBeGreaterThan(-1);
|
||||
expect(
|
||||
yieldAt,
|
||||
'withholding the server resize but reflowing anyway leaves this xterm at a shape ' +
|
||||
'the PTY was never told about — withhold both or neither'
|
||||
).toBeLessThan(fitAt);
|
||||
});
|
||||
|
||||
it('throttledResize withholds the fit wherever it withholds the SIGWINCH', () => {
|
||||
const start = terminalUi.indexOf('const throttledResize = () => {');
|
||||
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()');
|
||||
expect(guardAt).toBeGreaterThan(-1);
|
||||
expect(syncAt, 'the geometry sync must sit INSIDE the guard').toBeGreaterThan(guardAt);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the failed-load notice fits the narrowest terminal this app will render', () => {
|
||||
// ⚠️ `e587d845`'s commit message claimed a test asserted this against the
|
||||
// BUILT asset. It did not: that assertion lived in a throwaway probe that was
|
||||
// deleted with the rest of the scratch scripts, so the claim was wrong when it
|
||||
// was written. This is the real one, and it reads the source rather than
|
||||
// `dist/`, because `dist/` is not committed and a test that skips when it is
|
||||
// absent would pass for the wrong reason in CI.
|
||||
const app = read('src/web/public/app.js');
|
||||
const { TERMINAL_MIN_COLS } = loadGeometry();
|
||||
|
||||
/** The literal the catch writes, escapes resolved, SGR stripped. */
|
||||
function noticeLines(): string[] {
|
||||
const at = app.indexOf('if (clearedBeforeFresh && this.terminal)');
|
||||
expect(at, 'the failed-load branch is gone — renamed?').toBeGreaterThan(-1);
|
||||
// Anchored past the comments: one of them quotes a lone '.', which a
|
||||
// first-quote match happily returns instead of the notice.
|
||||
const writeAt = app.indexOf('this.terminal.write(', at);
|
||||
expect(writeAt, 'the failed-load branch no longer writes to the terminal').toBeGreaterThan(-1);
|
||||
const call = app.slice(writeAt, app.indexOf('\n }', writeAt));
|
||||
const literal = call.match(/'((?:[^'\\]|\\.)*)'/);
|
||||
expect(literal, 'no string literal in the failed-load branch').not.toBeNull();
|
||||
return literal![1]
|
||||
.replace(/\\x1b\[[0-9;]*m/g, '')
|
||||
.split('\\r\\n')
|
||||
.filter((line) => line.trim().length > 0);
|
||||
}
|
||||
|
||||
it('says what failed, that the session lives, and what to do', () => {
|
||||
const lines = noticeLines();
|
||||
expect(lines.length).toBe(3);
|
||||
expect(lines[0]).toMatch(/did not load/i);
|
||||
expect(lines[1]).toMatch(/live output/i);
|
||||
// A dead end is the most expensive defect here: the pane is blank and the
|
||||
// reader has no idea whether the session is recoverable.
|
||||
expect(lines[2], 'the notice must name the next step').toMatch(/reload/i);
|
||||
});
|
||||
|
||||
it('never wraps, down to the 40-column floor', () => {
|
||||
// A 52-character sentence measured at 320px wrapped and left a lone '.' on
|
||||
// a line of its own. The floor is the narrowest this app renders, and it is
|
||||
// two taps away on a small phone via increaseFontSize.
|
||||
for (const line of noticeLines()) {
|
||||
expect(
|
||||
line.length,
|
||||
`"${line}" is ${line.length} columns, over the ${TERMINAL_MIN_COLS} floor`
|
||||
).toBeLessThanOrEqual(TERMINAL_MIN_COLS);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not tell the reader to reopen the tab, which retries nothing', () => {
|
||||
// selectSession early-returns when the session is already active, so
|
||||
// clicking the tab you are already on does not re-fetch.
|
||||
expect(app).toContain('if (this.activeSessionId === sessionId && !forceReload)');
|
||||
expect(noticeLines().join(' ')).not.toMatch(/reopen|switch tab/i);
|
||||
});
|
||||
});
|
||||
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
// Resize stopped being write-only.
|
||||
// ───────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('the server reports the geometry the PTY actually holds', () => {
|
||||
it('Session reports its geometry only while a pane is actually drawing', () => {
|
||||
const session = read('src/session.ts');
|
||||
// ⚠️ `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.
|
||||
expect(session).toMatch(/get ptyGeometry\(\): \{ cols: number; rows: number \} \| null \{/);
|
||||
expect(session).toContain('return this.ptyProcess ? { cols: this._ptyCols, rows: this._ptyRows } : null;');
|
||||
expect(session, 'the raw getters would report the defaults again').not.toMatch(/get ptyCols\(\)/);
|
||||
});
|
||||
|
||||
it('the WebSocket answers a resize with what took', () => {
|
||||
const ws = read('src/web/routes/ws-routes.ts');
|
||||
const at = ws.indexOf('session.resize(msg.c, msg.r,');
|
||||
expect(at).toBeGreaterThan(-1);
|
||||
const after = ws.slice(at, at + 1400);
|
||||
expect(after).toContain('"t":"zc"');
|
||||
expect(after).toContain('const applied = session.ptyGeometry;');
|
||||
// No pane, no frame at all.
|
||||
expect(after).toContain('if (applied && socket.readyState === 1)');
|
||||
// Documented in the protocol block at the top of the file, like every other frame.
|
||||
expect(ws).toContain('{"t":"zc","c":N,"r":N}');
|
||||
});
|
||||
|
||||
it('the HTTP resize answers with what took, not an empty object', () => {
|
||||
const routes = read('src/web/routes/session-routes.ts');
|
||||
const at = routes.indexOf("app.post('/api/sessions/:id/resize'");
|
||||
expect(at).toBeGreaterThan(-1);
|
||||
const handler = routes.slice(at, at + 1600);
|
||||
expect(handler).toContain('return session.ptyGeometry ?? {};');
|
||||
});
|
||||
|
||||
it('the client adopts the report and re-bases its dedupe on it', () => {
|
||||
const terminalUi = read('src/web/public/terminal-ui.js');
|
||||
const start = terminalUi.indexOf('_onPtyGeometryReport(sessionId, cols, rows) {');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const body = terminalUi.slice(start, terminalUi.indexOf('\n },', start));
|
||||
expect(body).toContain('reconcilePtyGeometry');
|
||||
// Columns only: the local row count is carried through untouched.
|
||||
expect(body).toContain('this._resizeTerminalTo({ cols, rows: local.rows })');
|
||||
// Without this the next resize is deduped against a request that was
|
||||
// REFUSED, which suppresses the retry that recovers the pane.
|
||||
expect(body).toContain('this._lastResizeDims = { cols, rows: local.rows }');
|
||||
// And the WS frame is wired up at all.
|
||||
expect(read('src/web/public/app.js')).toContain("msg.t === 'zc'");
|
||||
});
|
||||
|
||||
it('a pane wider than the screen gets horizontal reach for as long as that lasts', () => {
|
||||
const css = read('src/web/public/styles.css');
|
||||
// .terminal-container is overflow:hidden, so adopting a wider PTY without
|
||||
// this puts the right-hand columns somewhere no gesture can reach them.
|
||||
// Read the rule's DECLARATIONS, comments stripped: the comments in this block
|
||||
// quote CSS with braces in it, which a `[^}]*` window cannot survive.
|
||||
const declarationsOf = (selector: string) => {
|
||||
const at = css.indexOf(`${selector} {`);
|
||||
expect(at, `${selector} not found`).toBeGreaterThan(-1);
|
||||
const body = css.slice(at + selector.length, css.indexOf('\n}', at));
|
||||
return body.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
};
|
||||
const oversized = declarationsOf('.terminal-container.term-overflows-x');
|
||||
expect(oversized).toContain('overflow-x: auto;');
|
||||
// Both axes, explicitly: mobile.css sets `overflow: visible` on the bare
|
||||
// selector, and a lone overflow-x would leave overflow-y computing to auto.
|
||||
expect(oversized).toContain('overflow-y: hidden;');
|
||||
// ⚠️ NO touch-action here, deliberately. `touch-action: pan-x` does nothing
|
||||
// for the sessions this targets — `touchstart` preventDefault()s every
|
||||
// 'content' tap, which cancels the browser's pan before it starts — and
|
||||
// granting it as well as the JS pan would move the pane twice for one
|
||||
// finger on the taps where that preventDefault does not run. The terminal's
|
||||
// own touchmove handler owns both axes; mobile.css's unscoped
|
||||
// `touch-action: none` is what keeps it the only owner.
|
||||
expect(css.slice(css.indexOf('.terminal-container.term-overflows-x'))).not.toMatch(
|
||||
/\.terminal-container\.term-overflows-x[^{]*\{[^}]*touch-action/
|
||||
);
|
||||
expect(read('src/web/public/terminal-ui.js')).toContain('const canPanHorizontally = () =>');
|
||||
expect(read('src/web/public/terminal-ui.js')).toContain("if (panAxis === 'x') {");
|
||||
// The class is only ever on while the terminal really is too wide, and it
|
||||
// is MEASURED rather than derived: the floor widens the terminal past a
|
||||
// narrow container with the PTY agreeing throughout, so a mismatch test
|
||||
// would never fire for it (360px, font 24: 218px unreachable).
|
||||
expect(read('src/web/public/terminal-ui.js')).toContain("classList.toggle('term-overflows-x', overflows)");
|
||||
expect(read('src/web/public/terminal-ui.js')).toContain(
|
||||
'screen.getBoundingClientRect().width - container.clientWidth > 1'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,290 @@
|
||||
// Port: none (pure helpers — no browser, no server).
|
||||
//
|
||||
// Three small decision functions behind the mobile terminal resilience work,
|
||||
// pinned here because the code that consumes them lives in app.js /
|
||||
// terminal-ui.js, which the CI gate cannot execute. Keeping the decision pure
|
||||
// and the DOM half thin is what makes any of this testable without a browser.
|
||||
//
|
||||
// The renderer-liveness case is the one worth reading. iOS DISCARDS scheduled
|
||||
// requestAnimationFrame callbacks when a PWA backgrounds — never delivered, not
|
||||
// deferred — and xterm's RenderDebouncer only clears its `_animationFrame`
|
||||
// handle from inside that callback. One drop leaves the handle permanently set,
|
||||
// so every later refresh() returns immediately and the terminal freezes while
|
||||
// its buffer keeps updating correctly. Codeman has exactly one xterm instance
|
||||
// per page load, so a single backgrounding can wedge it until a reload.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { createServer, type ServerResponse } from 'node:http';
|
||||
import type { AddressInfo } from 'node:net';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
function loadConstants() {
|
||||
const context = vm.createContext({ window: {}, globalThis: {} });
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
|
||||
vm.runInContext(source, context, { filename: 'constants.js' });
|
||||
const w = context.window as {
|
||||
CodemanRenderLiveness: {
|
||||
shouldKickRenderer: (s: {
|
||||
wroteAt: number;
|
||||
renderedAt: number;
|
||||
now: number;
|
||||
visible: boolean;
|
||||
thresholdMs?: number;
|
||||
}) => boolean;
|
||||
RENDER_STALL_MS: number;
|
||||
RENDER_LIVENESS_POLL_MS: number;
|
||||
};
|
||||
CodemanFetchDeadline: {
|
||||
terminalFetchDeadlineMs: (s: { full?: boolean; inflight?: number }) => number;
|
||||
FETCH_DEADLINE_TAIL_MS: number;
|
||||
FETCH_DEADLINE_FULL_MS: number;
|
||||
FETCH_DEADLINE_MAX_MS: number;
|
||||
};
|
||||
CodemanDiag: {
|
||||
sanitizeDiagEntry: (msg: unknown) => string;
|
||||
DIAG_ENTRY_MAX_CHARS: number;
|
||||
};
|
||||
};
|
||||
return w;
|
||||
}
|
||||
|
||||
describe('shouldKickRenderer', () => {
|
||||
const { CodemanRenderLiveness } = loadConstants();
|
||||
const { shouldKickRenderer, RENDER_STALL_MS } = CodemanRenderLiveness;
|
||||
|
||||
// The signature of the real failure: bytes were written, the element is
|
||||
// visible, and no frame has been produced since.
|
||||
it('kicks when a visible terminal owes a frame past the threshold', () => {
|
||||
expect(shouldKickRenderer({ wroteAt: 1000, renderedAt: 500, now: 1000 + RENDER_STALL_MS, visible: true })).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('does not kick before the threshold elapses', () => {
|
||||
expect(shouldKickRenderer({ wroteAt: 1000, renderedAt: 500, now: 1000 + RENDER_STALL_MS - 1, visible: true })).toBe(
|
||||
false
|
||||
);
|
||||
});
|
||||
|
||||
// A render at or after the last write means the pipeline is alive. This is
|
||||
// the common case on every healthy terminal and must never kick.
|
||||
it('does not kick when a render landed after the last write', () => {
|
||||
expect(shouldKickRenderer({ wroteAt: 1000, renderedAt: 1000, now: 99_999, visible: true })).toBe(false);
|
||||
expect(shouldKickRenderer({ wroteAt: 1000, renderedAt: 1200, now: 99_999, visible: true })).toBe(false);
|
||||
});
|
||||
|
||||
// A hidden terminal legitimately stops rendering — xterm pauses it. Kicking
|
||||
// there would fire on every backgrounded tab, forever.
|
||||
it('never kicks a hidden terminal', () => {
|
||||
expect(shouldKickRenderer({ wroteAt: 1000, renderedAt: 500, now: 99_999, visible: false })).toBe(false);
|
||||
});
|
||||
|
||||
// A quiet terminal is the normal state, not a stalled one. Gating on "no
|
||||
// render recently" instead of "owes a frame" would kick every idle session.
|
||||
it('never kicks a terminal that has never been written to', () => {
|
||||
expect(shouldKickRenderer({ wroteAt: 0, renderedAt: 0, now: 99_999, visible: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('tolerates missing and malformed input rather than throwing', () => {
|
||||
expect(shouldKickRenderer(undefined as never)).toBe(false);
|
||||
expect(shouldKickRenderer({} as never)).toBe(false);
|
||||
expect(shouldKickRenderer({ wroteAt: NaN, renderedAt: NaN, now: NaN, visible: true } as never)).toBe(false);
|
||||
});
|
||||
|
||||
it('polls coarsely enough not to wake an idle phone every second', () => {
|
||||
expect(CodemanRenderLiveness.RENDER_LIVENESS_POLL_MS).toBeGreaterThanOrEqual(1000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('terminalFetchDeadlineMs', () => {
|
||||
const { CodemanFetchDeadline } = loadConstants();
|
||||
const { terminalFetchDeadlineMs, FETCH_DEADLINE_TAIL_MS, FETCH_DEADLINE_FULL_MS, FETCH_DEADLINE_MAX_MS } =
|
||||
CodemanFetchDeadline;
|
||||
|
||||
// A full scrollback capture can be megabytes where a tail is one frame, so a
|
||||
// single fixed timeout is wrong in both directions on a mobile link.
|
||||
it('gives a full capture more budget than a tail', () => {
|
||||
expect(terminalFetchDeadlineMs({ full: true })).toBeGreaterThan(terminalFetchDeadlineMs({ full: false }));
|
||||
expect(terminalFetchDeadlineMs({ full: false })).toBe(FETCH_DEADLINE_TAIL_MS);
|
||||
expect(terminalFetchDeadlineMs({ full: true })).toBe(FETCH_DEADLINE_FULL_MS);
|
||||
});
|
||||
|
||||
// Eight tabs resuming must not all expire together because each assumed it
|
||||
// had the link to itself.
|
||||
it('scales with captures already in flight', () => {
|
||||
const alone = terminalFetchDeadlineMs({ full: false, inflight: 0 });
|
||||
const queued = terminalFetchDeadlineMs({ full: false, inflight: 3 });
|
||||
expect(queued).toBeGreaterThan(alone);
|
||||
});
|
||||
|
||||
it('is bounded — a stuck link still fails eventually', () => {
|
||||
expect(terminalFetchDeadlineMs({ full: true, inflight: 1000 })).toBe(FETCH_DEADLINE_MAX_MS);
|
||||
});
|
||||
|
||||
it('treats absent and nonsense input as a lone tail fetch', () => {
|
||||
expect(terminalFetchDeadlineMs({})).toBe(FETCH_DEADLINE_TAIL_MS);
|
||||
expect(terminalFetchDeadlineMs({ inflight: -5 } as never)).toBe(FETCH_DEADLINE_TAIL_MS);
|
||||
expect(terminalFetchDeadlineMs({ inflight: NaN } as never)).toBe(FETCH_DEADLINE_TAIL_MS);
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitizeDiagEntry', () => {
|
||||
const { CodemanDiag } = loadConstants();
|
||||
const { sanitizeDiagEntry, DIAG_ENTRY_MAX_CHARS } = CodemanDiag;
|
||||
|
||||
// The crash trail is joined with '\n' into one localStorage value and
|
||||
// beaconed, and at least one call site interpolates a WebSocket close
|
||||
// `reason`, which the server controls. A newline there forges entries.
|
||||
it('collapses every newline form so an entry cannot forge another', () => {
|
||||
expect(sanitizeDiagEntry('WS CLOSE reason=a\nFAKE ENTRY')).toBe('WS CLOSE reason=a FAKE ENTRY');
|
||||
expect(sanitizeDiagEntry('a\r\nb')).toBe('a b');
|
||||
expect(sanitizeDiagEntry('a
b
c')).toBe('a b c');
|
||||
});
|
||||
|
||||
it('bounds length so one entry cannot exhaust the storage quota', () => {
|
||||
const out = sanitizeDiagEntry('x'.repeat(DIAG_ENTRY_MAX_CHARS * 3));
|
||||
expect(out).toHaveLength(DIAG_ENTRY_MAX_CHARS);
|
||||
});
|
||||
|
||||
it('never throws on the values a diagnostic call site can actually pass', () => {
|
||||
expect(sanitizeDiagEntry(null)).toBe('');
|
||||
expect(sanitizeDiagEntry(undefined)).toBe('');
|
||||
expect(sanitizeDiagEntry(42)).toBe('42');
|
||||
expect(sanitizeDiagEntry({ toString: () => 'obj' })).toBe('obj');
|
||||
});
|
||||
});
|
||||
|
||||
// ── The deadline must cover the BODY, not just the handshake ────────────────
|
||||
//
|
||||
// `await fetch()` settles on response HEADERS. Clearing the abort timer there
|
||||
// leaves the body — the multi-megabyte `?full=1` capture the deadline exists
|
||||
// for — completely unbounded; it only ever covered a server that accepts a
|
||||
// connection and never replies at all.
|
||||
//
|
||||
// Measured on the pre-fix shape against a server that sends headers immediately
|
||||
// and stalls the body 4s under a 1s deadline: fetch resolved at 30ms, the timer
|
||||
// was cleared there, and the body completed at 4026ms unaborted.
|
||||
//
|
||||
// This exercises the real property with a real socket rather than asserting on
|
||||
// source text, because the bug was a lifetime mistake that reads correctly.
|
||||
describe('terminal capture deadline covers the response body', () => {
|
||||
// Mirrors _fetchTerminalCapture's lifetime: one timer spanning headers AND
|
||||
// body, cleared only once the body has been read.
|
||||
async function captureUnderDeadline(url: string, deadlineMs: number) {
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), deadlineMs);
|
||||
try {
|
||||
const res = await fetch(url, { signal: controller.signal });
|
||||
const headersAt = performance.now();
|
||||
const json = await res.json();
|
||||
return { json, headers: res.headers, headersAt };
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
|
||||
async function serve(handler: (res: ServerResponse) => void) {
|
||||
const srv = createServer((_req, res) => handler(res));
|
||||
await new Promise<void>((r) => srv.listen(0, '127.0.0.1', r));
|
||||
const { port } = srv.address() as AddressInfo;
|
||||
return { url: `http://127.0.0.1:${port}/`, close: () => srv.close() };
|
||||
}
|
||||
|
||||
it('aborts a stalled body instead of waiting on it forever', async () => {
|
||||
let finish: NodeJS.Timeout | undefined;
|
||||
const { url, close } = await serve((res) => {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.write(' '); // headers out immediately, body never completes in time
|
||||
finish = setTimeout(() => res.end('{"data":{}}'), 5000);
|
||||
});
|
||||
try {
|
||||
await expect(captureUnderDeadline(url, 300)).rejects.toThrow(/abort/i);
|
||||
} finally {
|
||||
if (finish) clearTimeout(finish);
|
||||
close();
|
||||
}
|
||||
});
|
||||
|
||||
// The two tests above exercise the PATTERN against a real socket, using a
|
||||
// local mirror — so on their own they would still pass if the real helper
|
||||
// regressed to clearing its timer at headers. This pins the real one.
|
||||
it('_fetchTerminalCapture reads the body before releasing its deadline', () => {
|
||||
const app = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const start = app.indexOf('async _fetchTerminalCapture(');
|
||||
expect(start, 'helper not found — renamed?').toBeGreaterThan(-1);
|
||||
const body = app.slice(start, app.indexOf('\n }', start));
|
||||
const jsonAt = body.indexOf('await res.json()');
|
||||
const finallyAt = body.indexOf('} finally {');
|
||||
expect(jsonAt, 'the body must be read inside the helper, not by callers').toBeGreaterThan(-1);
|
||||
expect(finallyAt).toBeGreaterThan(-1);
|
||||
expect(
|
||||
jsonAt,
|
||||
'await res.json() must run BEFORE the finally that clears the abort timer — ' +
|
||||
'fetch() settles on headers, so a timer cleared there leaves the body unbounded'
|
||||
).toBeLessThan(finallyAt);
|
||||
// And the returned shape the five call sites destructure.
|
||||
expect(body).toContain('return { json, headers: res.headers, headersAt };');
|
||||
});
|
||||
|
||||
// Nothing in the gate pinned the invariant this PR exists to establish, which
|
||||
// is the same drift it is fixing: a clear()+reset() pair reads as obviously
|
||||
// equivalent to the queued RIS and is exactly what someone tidies back in.
|
||||
it('_resetTerminalForReplay is a queued write and nothing else', () => {
|
||||
const app = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const start = app.indexOf('_resetTerminalForReplay() {');
|
||||
expect(start, 'helper not found — renamed?').toBeGreaterThan(-1);
|
||||
const body = app.slice(start, app.indexOf('\n }', start));
|
||||
// RIS, queued through write() so it lands after any bytes already parsing.
|
||||
expect(body).toContain("this.terminal.write('\\x1bc')");
|
||||
expect(
|
||||
body,
|
||||
'reset()/clear() are SYNCHRONOUS and skip the write queue, so bytes queued ' +
|
||||
'before them are parsed after and fuse into the snapshot written next'
|
||||
).not.toMatch(/\.(reset|clear)\(\)/);
|
||||
});
|
||||
|
||||
it('every replay path clears through that helper, never by hand', () => {
|
||||
const app = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
// The three paths that blank the terminal before rewriting it from a capture.
|
||||
for (const site of ['_onSessionNeedsRefresh(event = {}) {', 'async _onSessionClearTerminal(data) {']) {
|
||||
const start = app.indexOf(site);
|
||||
expect(start, `${site} not found — renamed?`).toBeGreaterThan(-1);
|
||||
const body = app.slice(start, start + 4000);
|
||||
expect(body, `${site} must clear via _resetTerminalForReplay`).toContain('this._resetTerminalForReplay()');
|
||||
expect(body, `${site} hand-rolled a clear again`).not.toContain('this.terminal.clear()');
|
||||
}
|
||||
// And the PAIR appears nowhere in the frontend. A lone `clear()` before
|
||||
// `showWelcome()` is fine — nothing is written after it, so there is nothing
|
||||
// for stray bytes to fuse into. `clear()` immediately followed by `reset()`
|
||||
// is the signature of someone blanking the terminal to rewrite it, which is
|
||||
// precisely the case that has to be queued instead.
|
||||
const pair = /\.clear\(\);\s*\n\s*this\.terminal\.reset\(\)/;
|
||||
for (const rel of [
|
||||
'src/web/public/app.js',
|
||||
'src/web/public/panels-ui.js',
|
||||
'src/web/public/terminal-ui.js',
|
||||
'src/web/public/session-ui.js',
|
||||
]) {
|
||||
const src = readFileSync(resolve(import.meta.dirname, '..', rel), 'utf8');
|
||||
expect(src, `${rel} blanks the terminal with clear()+reset() — use _resetTerminalForReplay()`).not.toMatch(pair);
|
||||
}
|
||||
});
|
||||
|
||||
it('returns the parsed envelope and headers on a healthy response', async () => {
|
||||
const { url, close } = await serve((res) => {
|
||||
res.writeHead(200, { 'Content-Type': 'application/json', 'server-timing': 'db;dur=12' });
|
||||
res.end('{"data":{"terminalBuffer":"hello"}}');
|
||||
});
|
||||
try {
|
||||
const out = await captureUnderDeadline(url, 5000);
|
||||
// Callers read `capture.json?.data`, `capture.headers.get(...)` and
|
||||
// `capture.headersAt` — all three must survive.
|
||||
expect((out.json as { data: { terminalBuffer: string } }).data.terminalBuffer).toBe('hello');
|
||||
expect(out.headers.get('server-timing')).toBe('db;dur=12');
|
||||
expect(typeof out.headersAt).toBe('number');
|
||||
} finally {
|
||||
close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -40,6 +40,14 @@ function loadKeyboardHandler(opts: { viewportY: number; baseY: number }) {
|
||||
const app: any = {
|
||||
terminal,
|
||||
fitAddon: { fit: () => calls.push('fit') },
|
||||
// The settle refits through the one function that also applies the floor
|
||||
// it reports (#464), so that is what the fake has to offer. Recorded under
|
||||
// its own name rather than 'fit': a bare fit here would be the divergence
|
||||
// this test's subject was changed to avoid.
|
||||
syncTerminalGeometry: () => {
|
||||
calls.push('syncTerminalGeometry');
|
||||
return { cols: 80, rows: 24 };
|
||||
},
|
||||
// The real predicate (terminal-ui.js isTerminalAtBottom), reproduced so the
|
||||
// test exercises the same tolerance the runtime uses.
|
||||
isTerminalAtBottom: () => terminal.buffer.active.viewportY >= terminal.buffer.active.baseY - 2,
|
||||
@@ -131,7 +139,7 @@ describe('keyboard settle preserves scroll intent (issue #259)', () => {
|
||||
kh._scheduleViewportSettle({});
|
||||
settle();
|
||||
|
||||
expect(calls).toContain('fit');
|
||||
expect(calls).toContain('syncTerminalGeometry');
|
||||
expect(calls).not.toContain('scrollToBottom');
|
||||
expect(calls.some((c) => c.startsWith('scrollToLine'))).toBe(false);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
// Port: none (dependency-range guard — no browser, no server).
|
||||
//
|
||||
// terminal-ui.js's `_kickRenderer` reaches into xterm internals to unwedge a
|
||||
// frozen RenderDebouncer:
|
||||
//
|
||||
// terminal._core._renderService._renderDebouncer._animationFrame
|
||||
// terminal._core._renderService.refreshRows(start, end)
|
||||
//
|
||||
// There is no public API for any of it — xterm exposes no way to ask "are you
|
||||
// still producing frames" or "drop your stale animation handle" — and the bug
|
||||
// it heals (iOS discarding a scheduled rAF, leaving that handle permanently set
|
||||
// so every later refresh() early-returns) is otherwise unrecoverable without a
|
||||
// page reload.
|
||||
//
|
||||
// That path CANNOT be asserted in this suite. `_renderService` is constructed
|
||||
// by `Terminal.open()`, which needs a real DOM, and the CI gate runs in node —
|
||||
// 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
|
||||
// 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.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
const root = resolve(import.meta.dirname, '..');
|
||||
const lock = JSON.parse(readFileSync(resolve(root, 'package-lock.json'), 'utf8')) as {
|
||||
packages: Record<string, { version?: string }>;
|
||||
};
|
||||
const terminalUi = readFileSync(resolve(root, 'src/web/public/terminal-ui.js'), 'utf8');
|
||||
|
||||
// The exact version `_kickRenderer`'s field path was verified against.
|
||||
//
|
||||
// Read from the LOCKFILE, not package.json. The declared range is `^6.0.0`, so
|
||||
// asserting on that string is the wrong test in both directions: a real upgrade
|
||||
// to 6.4.0 — which can absolutely rename a private field — resolves inside the
|
||||
// range and slips through, while an innocuous range edit that changes nothing
|
||||
// about the installed code fails. The lockfile is what actually ships.
|
||||
const VERIFIED_XTERM_VERSION = '6.0.0';
|
||||
|
||||
describe('xterm private-API dependency guard', () => {
|
||||
it('pins the resolved xterm version _kickRenderer was verified against', () => {
|
||||
expect(
|
||||
lock.packages['node_modules/@xterm/xterm']?.version,
|
||||
'xterm moved off the verified version — re-verify _kickRenderer in a real browser ' +
|
||||
'(terminal-ui.js: _core._renderService._renderDebouncer._animationFrame), then update ' +
|
||||
'VERIFIED_XTERM_VERSION here. The accessor is optional-chained, so a renamed field ' +
|
||||
'degrades to a silent no-op and the freeze it heals comes back unnoticed.'
|
||||
).toBe(VERIFIED_XTERM_VERSION);
|
||||
});
|
||||
|
||||
// If someone deletes the watchdog, this guard is pointless noise — keep the
|
||||
// two tied together so the range check cannot outlive what it protects.
|
||||
it('is guarding a watchdog that still exists', () => {
|
||||
expect(terminalUi).toContain('_kickRenderer()');
|
||||
expect(terminalUi).toContain('_renderDebouncer');
|
||||
expect(terminalUi).toContain('_animationFrame');
|
||||
});
|
||||
|
||||
// Every private read must stay optional-chained. This is the property that
|
||||
// makes reaching into internals acceptable at all: upstream can rename
|
||||
// anything and the worst case is that healing stops, never that the terminal
|
||||
// throws on a timer every two seconds.
|
||||
it('reads every private field defensively', () => {
|
||||
expect(terminalUi).toContain('this.terminal?._core?._renderService');
|
||||
const body = terminalUi.slice(terminalUi.indexOf('_kickRenderer() {'));
|
||||
const fn = body.slice(0, body.indexOf('\n },'));
|
||||
expect(fn).toContain('try {');
|
||||
expect(fn).toContain('catch');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user