diff --git a/packages/app/e2e/terminal-stuck-size.spec.ts b/packages/app/e2e/terminal-stuck-size.spec.ts new file mode 100644 index 000000000..ebdfd0110 --- /dev/null +++ b/packages/app/e2e/terminal-stuck-size.spec.ts @@ -0,0 +1,310 @@ +import { test, expect, type Page } from "./fixtures"; +import { TerminalE2EHarness } from "./helpers/terminal-dsl"; +import { buildHostWorkspaceRoute } from "../src/utils/host-routes"; +import { getServerId } from "./helpers/server-id"; + +/** + * Regression: a terminal created while the window is unfocused must still claim its PTY size + * once focus returns. + * + * The PTY is only ever resized by an explicit client claim (`terminal_input` resize). A freshly + * mounted terminal produces exactly one claim, from terminal-pane's pane-focus reflow effect. If + * that claim is emitted while the app is not actively visible (window blurred / document hidden), + * `handleTerminalResize` drops it — and nothing used to re-send it, leaving the PTY at 80x24 + * while xterm renders the real pane size: vim/top squeezed into a corner until the user resizes + * the window. + * + * The repro is the mundane user flow: with a workspace already showing a terminal, open another + * one and switch to a different app while it spawns. We stage the blur deterministically by + * stubbing `document.hasFocus()` (which `useAppVisible` consults on window focus/blur events) + * instead of racing real OS focus. The first terminal matters: `useAppVisible` only observes + * focus events while a consumer is mounted, so it is what makes the blur visible to the app — + * exactly as in the real flow. + * + * The assertion reads the PTY's own opinion of its size (`stty size`) with input written + * daemon-side — clicking or typing in the pane would fire the focus-claim path and mask the bug. + */ + +/** + * Simulates the window losing/regaining OS focus, which is what `useAppVisible` reads through + * `document.hasFocus()` plus the window focus/blur events. + * + * This has to be stubbed: headless Chromium never actually blurs. Opening a second page and + * calling bringToFront() leaves the first page at `hasFocus() === true`, `visibilityState + * === "visible"`, and fires no focus/blur events at all — so there is no way to produce a real + * blur from Playwright. This stubs the environment signal, not the app: the daemon, the + * WebSocket, the terminal, and every code path under test stay real. + */ +async function setWindowFocused(page: Page, focused: boolean): Promise { + await page.evaluate((isFocused) => { + Object.defineProperty(document, "hasFocus", { + configurable: true, + value: () => isFocused, + }); + window.dispatchEvent(new Event(isFocused ? "focus" : "blur")); + }, focused); +} + +interface RenderedTerminalSize { + rows: number; + cols: number; +} + +async function readRenderedTerminalSize(page: Page): Promise { + return await page.evaluate(() => { + const terminal = (window as Window & { __paseoTerminal?: { rows: number; cols: number } }) + .__paseoTerminal; + return terminal ? { rows: terminal.rows, cols: terminal.cols } : null; + }); +} + +async function readTerminalBufferText(page: Page): Promise { + return await page.evaluate(() => { + const terminal = ( + window as Window & { + __paseoTerminal?: { + buffer: { + active: { + length: number; + getLine(index: number): { translateToString(trim: boolean): string } | undefined; + }; + }; + }; + } + ).__paseoTerminal; + if (!terminal) { + return ""; + } + const lines: string[] = []; + for (let index = 0; index < terminal.buffer.active.length; index += 1) { + lines.push(terminal.buffer.active.getLine(index)?.translateToString(true) ?? ""); + } + return lines.join("\n"); + }); +} + +async function createTerminalViaMenu(page: Page): Promise { + await page.getByTestId("workspace-new-tab-menu-trigger").click(); + await page.getByTestId("workspace-new-tab-menu-terminal").click(); +} + +async function listTerminalIds(harness: TerminalE2EHarness): Promise { + const listed = await harness.client.listTerminals(harness.tempRepo.path, undefined, { + workspaceId: harness.workspaceId, + }); + return listed.terminals.map((terminal) => terminal.id); +} + +// These narrow instead of asserting: an `expect(...).toBeTruthy()` plus an early return would let +// the test pass without ever reaching the assertions that prove the fix. +function exactlyOne(values: T[], what: string): T { + const [value] = values; + if (values.length !== 1 || value === undefined) { + throw new Error(`Expected exactly one ${what}, got ${values.length}`); + } + return value; +} + +function requireTerminalSize(size: RenderedTerminalSize | null): RenderedTerminalSize { + if (!size) { + throw new Error("No xterm instance rendered on the page"); + } + return size; +} + +function claimsFor(frames: ResizeClaim[], terminalId: string): ResizeClaim[] { + return frames.filter((frame) => frame.terminalId === terminalId); +} + +function parseSttySize(bufferText: string): RenderedTerminalSize { + const match = /S=(\d+) (\d+)=/.exec(bufferText); + if (!match?.[1] || !match[2]) { + throw new Error(`stty size did not print in the terminal buffer:\n${bufferText}`); + } + return { rows: Number(match[1]), cols: Number(match[2]) }; +} + +interface ResizeClaim { + terminalId: string; + rows: number; + cols: number; +} + +/** + * Records every resize claim the app puts on the wire. Claims travel two ways: as a JSON + * `terminal_input` before the stream has a binary slot, and as a binary frame + * ([opcode 0x03][slot][JSON {rows, cols}]) once subscribed. The slot -> terminal mapping comes + * from subscribe_terminal_response on the receive side. + */ +function captureResizeClaims(page: Page): ResizeClaim[] { + const resizeFrames: ResizeClaim[] = []; + const slotTerminals = new Map(); + const unmappedBinaryResizes: Array<{ slot: number; rows: number; cols: number }> = []; + + const recordBinaryResize = (slot: number, size: { rows: number; cols: number }) => { + const terminalId = slotTerminals.get(slot); + if (terminalId) { + resizeFrames.push({ terminalId, ...size }); + } else { + unmappedBinaryResizes.push({ slot, ...size }); + } + }; + + const handleSentFrame = (frame: { payload: string | Buffer }) => { + const payload = frame.payload; + if (typeof payload !== "string") { + if (payload.length >= 2 && payload[0] === 0x03) { + try { + const parsed = JSON.parse(payload.subarray(2).toString("utf8")) as { + rows?: number; + cols?: number; + }; + if (parsed.rows !== undefined && parsed.cols !== undefined) { + recordBinaryResize(payload[1] ?? -1, { rows: parsed.rows, cols: parsed.cols }); + } + } catch { + // not a terminal resize frame — ignore + } + } + return; + } + if (!payload.includes("resize")) { + return; + } + try { + const outer = JSON.parse(payload) as { message?: Record }; + const message = outer.message ?? {}; + if (message.type !== "terminal_input") { + return; + } + const inner = message.message as { type?: string; rows?: number; cols?: number }; + if (inner?.type === "resize" && inner.rows !== undefined && inner.cols !== undefined) { + resizeFrames.push({ + terminalId: String(message.terminalId ?? ""), + rows: inner.rows, + cols: inner.cols, + }); + } + } catch { + // not JSON — ignore + } + }; + + const handleReceivedFrame = (frame: { payload: string | Buffer }) => { + const payload = frame.payload; + if (typeof payload !== "string" || !payload.includes("subscribe_terminal_response")) { + return; + } + try { + const outer = JSON.parse(payload) as { message?: Record }; + const message = outer.message ?? {}; + if (message.type !== "subscribe_terminal_response") { + return; + } + const responsePayload = (message.payload ?? {}) as { terminalId?: string; slot?: number }; + if ( + typeof responsePayload.terminalId === "string" && + typeof responsePayload.slot === "number" + ) { + slotTerminals.set(responsePayload.slot, responsePayload.terminalId); + for (const entry of unmappedBinaryResizes.splice(0)) { + recordBinaryResize(entry.slot, { rows: entry.rows, cols: entry.cols }); + } + } + } catch { + // not JSON — ignore + } + }; + + page.on("websocket", (ws) => { + ws.on("framesent", handleSentFrame); + ws.on("framereceived", handleReceivedFrame); + }); + + return resizeFrames; +} + +test.describe("terminal PTY size claim under lost window focus", () => { + let harness: TerminalE2EHarness; + + test.beforeEach(async () => { + harness = await TerminalE2EHarness.create({ tempPrefix: "terminal-stuck-size" }); + }); + + test.afterEach(async () => { + await harness.cleanup(); + }); + + test("a terminal created while the window is blurred claims its size when focus returns", async ({ + page, + }) => { + await page.setViewportSize({ width: 1280, height: 900 }); + const resizeFrames = captureResizeClaims(page); + + await page.goto(buildHostWorkspaceRoute(getServerId(), harness.workspaceId)); + await expect(page.getByTestId("workspace-new-tab-menu-trigger")).toBeVisible({ + timeout: 30_000, + }); + + // A terminal is already open — this also keeps useAppVisible's listeners mounted so the + // upcoming blur is actually observed by the app. Wait for the daemon to know about it rather + // than sleeping: if it were missing from this snapshot it would be misread as "new" below. + await createTerminalViaMenu(page); + await expect(harness.terminalSurface(page).first()).toBeVisible({ timeout: 30_000 }); + const countTerminals = async () => (await listTerminalIds(harness)).length; + await expect.poll(countTerminals, { timeout: 15_000 }).toBe(1); + const knownIds = new Set(await listTerminalIds(harness)); + + // The user clicks away... + await setWindowFocused(page, false); + + // ...opens another terminal while the window is unfocused... + await createTerminalViaMenu(page); + const findNewTerminalIds = async () => + (await listTerminalIds(harness)).filter((id) => !knownIds.has(id)); + await expect.poll(async () => (await findNewTerminalIds()).length, { timeout: 15_000 }).toBe(1); + const newTerminalId = exactlyOne(await findNewTerminalIds(), "new terminal"); + + // Let every mount-time refit run (the ladder goes out to 2s) while the window stays blurred. + // This one has to be a wait, not a poll: the assertion is that nothing happens. + await page.waitForTimeout(2_500); + + // Nothing may reach the daemon while the app is hidden — handleTerminalResize drops claims + // that fire while !isAppVisible. Without this, deleting that gate would still leave the test + // green: the claim would simply arrive early instead of after focus returns. + expect(claimsFor(resizeFrames, newTerminalId)).toEqual([]); + + // ...and comes back. + await setWindowFocused(page, true); + + // __paseoTerminal points at the most recently mounted xterm — the new terminal. + const rendered = requireTerminalSize(await readRenderedTerminalSize(page)); + // Sanity: the pane really rendered at a desktop size, not the PTY default. + expect(rendered.cols).toBeGreaterThan(80); + + // The claim must reach the wire once focus is back. Poll for it rather than sleeping: this + // is the behavior under test, so waiting a fixed duration would trade a real assertion for + // a timing bet. + const claimsForNewTerminal = () => claimsFor(resizeFrames, newTerminalId); + await expect + .poll(claimsForNewTerminal, { timeout: 15_000 }) + .toContainEqual({ terminalId: newTerminalId, rows: rendered.rows, cols: rendered.cols }); + + // And the PTY itself must agree. Ask it via the daemon, never via the page: focusing or + // typing in the pane triggers the focus-claim path and would mask the bug. + await harness.client.subscribeTerminal(newTerminalId); + harness.client.sendTerminalInput(newTerminalId, { + type: "input", + data: 'echo "S=$(stty size)="\n', + }); + + await expect + .poll(async () => readTerminalBufferText(page), { timeout: 15_000 }) + .toMatch(/S=\d+ \d+=/); + + const ptySize = parseSttySize(await readTerminalBufferText(page)); + + // The PTY must match what xterm rendered — not the daemon's 80x24 spawn default. + expect(ptySize).toEqual({ rows: rendered.rows, cols: rendered.cols }); + }); +}); diff --git a/packages/app/src/components/terminal-pane-focus-latch.test.ts b/packages/app/src/components/terminal-pane-focus-latch.test.ts new file mode 100644 index 000000000..41dd60de2 --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-latch.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from "vitest"; +import { resolveFocusLatchStep, type FocusLatchStep } from "./terminal-pane-focus-latch"; + +/** + * Pins the ordering that caused "terminal stuck at 80x24": the latch must record that the action + * RAN, not that the effect ran. A pane that mounts before its workspace has focus (or while the + * app is hidden) must still fire once readiness arrives — burning the latch on the attempt + * permanently disarms the only resize claim a fresh terminal ever sends. + */ + +function run(steps: Array<{ key: string | null; canFire: boolean }>): FocusLatchStep[] { + const results: FocusLatchStep[] = []; + let latchedKey: string | null = null; + for (const step of steps) { + const result = resolveFocusLatchStep({ ...step, latchedKey }); + latchedKey = result.latchedKey; + results.push(result); + } + return results; +} + +describe("resolveFocusLatchStep", () => { + it("fires immediately when the pane is focused and ready at mount", () => { + const [step] = run([{ key: "ws:term-1", canFire: true }]); + expect(step).toEqual({ latchedKey: "ws:term-1", fire: true }); + }); + + it("defers instead of burning the latch when readiness arrives late", () => { + // The bug: pane focused while the workspace is not (or the app is hidden). The old code + // latched on the first pass and never fired again. + const steps = run([ + { key: "ws:term-1", canFire: false }, // mount: pane focused, workspace not focused yet + { key: "ws:term-1", canFire: true }, // workspace focus / visibility arrives + ]); + expect(steps[0]).toEqual({ latchedKey: null, fire: false }); + expect(steps[1]).toEqual({ latchedKey: "ws:term-1", fire: true }); + }); + + it("fires exactly once per continuous pane-focus period", () => { + const steps = run([ + { key: "ws:term-1", canFire: true }, + { key: "ws:term-1", canFire: true }, // unrelated re-render + { key: "ws:term-1", canFire: false }, // workspace blurs... + { key: "ws:term-1", canFire: true }, // ...and refocuses: not a new pane-focus period + ]); + expect(steps).toEqual([ + { latchedKey: "ws:term-1", fire: true }, + { latchedKey: "ws:term-1", fire: false }, + { latchedKey: "ws:term-1", fire: false }, + { latchedKey: "ws:term-1", fire: false }, + ]); + }); + + it("re-arms when the pane blurs and fires again on refocus", () => { + const steps = run([ + { key: "ws:term-1", canFire: true }, + { key: null, canFire: true }, // pane blurred / terminal gone + { key: "ws:term-1", canFire: true }, + ]); + expect(steps).toEqual([ + { latchedKey: "ws:term-1", fire: true }, + { latchedKey: null, fire: false }, + { latchedKey: "ws:term-1", fire: true }, + ]); + }); + + it("fires for a different terminal in the same pane", () => { + const steps = run([ + { key: "ws:term-1", canFire: true }, + { key: "ws:term-2", canFire: true }, + ]); + expect(steps).toEqual([ + { latchedKey: "ws:term-1", fire: true }, + { latchedKey: "ws:term-2", fire: true }, + ]); + }); + + it("stays deferred across repeated not-ready passes, then fires once", () => { + const steps = run([ + { key: "ws:term-1", canFire: false }, + { key: "ws:term-1", canFire: false }, + { key: "ws:term-1", canFire: true }, + { key: "ws:term-1", canFire: true }, + ]); + expect(steps).toEqual([ + { latchedKey: null, fire: false }, + { latchedKey: null, fire: false }, + { latchedKey: "ws:term-1", fire: true }, + { latchedKey: "ws:term-1", fire: false }, + ]); + }); +}); diff --git a/packages/app/src/components/terminal-pane-focus-latch.ts b/packages/app/src/components/terminal-pane-focus-latch.ts new file mode 100644 index 000000000..9d3d6196b --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-latch.ts @@ -0,0 +1,33 @@ +/** + * Once-per-pane-focus latch for terminal-pane's focus/reflow effects. + * + * Each effect wants to fire exactly once per continuous pane-focus period per terminal, but only + * while the action can actually land (workspace focused, and for resize claims the app visible — + * `handleTerminalResize` drops claims emitted while hidden and nothing re-sends them). The latch + * must therefore record "the action ran", never "we tried": latching before the readiness gate + * permanently disarms the effect when a pane mounts before its workspace has focus, which is how + * a terminal ends up rendering full-size while its PTY stays at the daemon's 80x24 default. + */ +export interface FocusLatchStep { + latchedKey: string | null; + fire: boolean; +} + +export function resolveFocusLatchStep(input: { + /** Identity of the pane-focus period; null resets the latch (pane blurred, no terminal). */ + key: string | null; + latchedKey: string | null; + /** Whether the action can land right now; while false the latch stays untouched. */ + canFire: boolean; +}): FocusLatchStep { + if (input.key === null) { + return { latchedKey: null, fire: false }; + } + if (input.latchedKey === input.key) { + return { latchedKey: input.latchedKey, fire: false }; + } + if (!input.canFire) { + return { latchedKey: input.latchedKey, fire: false }; + } + return { latchedKey: input.key, fire: true }; +} diff --git a/packages/app/src/components/terminal-pane.tsx b/packages/app/src/components/terminal-pane.tsx index f6d9c655f..52955c206 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -21,7 +21,7 @@ import { resolvePendingModifierDataInput, } from "@/utils/terminal-keys"; import { getWorkspaceTerminalSession } from "@/terminal/runtime/workspace-terminal-session"; -import { rememberTerminalViewportSize } from "@/terminal/runtime/terminal-size-cache"; +import { resolveFocusLatchStep } from "./terminal-pane-focus-latch"; import { TerminalStreamController, type TerminalStreamControllerStatus, @@ -261,42 +261,41 @@ export function TerminalPane({ ); useEffect(() => { - if (isMobile || !isPaneFocused || !terminalId) { - lastAutoFocusKeyRef.current = null; - return; + // Latch only when the focus actually ran: latching before the workspace-focus gate would + // permanently disarm this effect for panes that mount before their workspace has focus. + const step = resolveFocusLatchStep({ + key: isMobile || !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, + latchedKey: lastAutoFocusKeyRef.current, + canFire: isWorkspaceFocused, + }); + lastAutoFocusKeyRef.current = step.latchedKey; + if (step.fire) { + requestTerminalFocus(); } - - const nextFocusKey = `${scopeKey}:${terminalId}`; - if (lastAutoFocusKeyRef.current === nextFocusKey) { - return; - } - - lastAutoFocusKeyRef.current = nextFocusKey; - if (!isWorkspaceFocused) { - return; - } - - requestTerminalFocus(); }, [isMobile, isPaneFocused, isWorkspaceFocused, requestTerminalFocus, scopeKey, terminalId]); useEffect(() => { - if (!isPaneFocused || !terminalId) { - lastPaneFocusResizeKeyRef.current = null; - return; + // This reflow produces the one resize claim a freshly mounted terminal ever sends; defer it + // (without burning the latch) until the claim can land — handleTerminalResize drops claims + // while the workspace is unfocused or the app is hidden, and nothing re-sends a dropped one. + const step = resolveFocusLatchStep({ + key: !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, + latchedKey: lastPaneFocusResizeKeyRef.current, + canFire: isWorkspaceFocused && isAppVisible, + }); + lastPaneFocusResizeKeyRef.current = step.latchedKey; + if (step.fire) { + lastSentTerminalSizeRef.current = null; + requestTerminalReflow(); } - - const focusResizeKey = `${scopeKey}:${terminalId}`; - if (lastPaneFocusResizeKeyRef.current === focusResizeKey) { - return; - } - lastPaneFocusResizeKeyRef.current = focusResizeKey; - if (!isWorkspaceFocused) { - return; - } - - lastSentTerminalSizeRef.current = null; - requestTerminalReflow(); - }, [isPaneFocused, isWorkspaceFocused, requestTerminalReflow, scopeKey, terminalId]); + }, [ + isAppVisible, + isPaneFocused, + isWorkspaceFocused, + requestTerminalReflow, + scopeKey, + terminalId, + ]); const handleTerminalFocus = useCallback(() => { if (isWorkspaceFocused && isPaneFocused) { @@ -636,9 +635,6 @@ export function TerminalPane({ const normalizedCols = Math.floor(cols); const nextSize = { rows: normalizedRows, cols: normalizedCols }; measuredTerminalSizeRef.current = nextSize; - // Seed future terminals in this workspace with the current pane size so they are born at - // the right size instead of the daemon's 80x24 default (see terminal-size-cache). - rememberTerminalViewportSize({ serverId, cwd, size: nextSize }); if (!input.shouldClaim || !client || !terminalId || !isWorkspaceFocused || !isAppVisible) { return; } diff --git a/packages/app/src/hooks/use-app-visible.ts b/packages/app/src/hooks/use-app-visible.ts index 953df7f61..89a046b3c 100644 --- a/packages/app/src/hooks/use-app-visible.ts +++ b/packages/app/src/hooks/use-app-visible.ts @@ -1,4 +1,4 @@ -import { useEffect, useSyncExternalStore } from "react"; +import { useSyncExternalStore } from "react"; import { AppState } from "react-native"; import { getIsAppActivelyVisible } from "@/utils/app-visibility"; import { isWeb } from "@/constants/platform"; @@ -17,6 +17,18 @@ function notify(): void { } } +// Track visibility for the app's whole lifetime, not per consumer: transitions that happen while +// no consumer is mounted must still be reflected in the snapshot the next consumer reads, or a +// component mounting right after a focus change acts on stale visibility. +// AppState needs no environment guard of its own — react-native-web's implementation already +// no-ops when there is no DOM, unlike the raw document/window listeners below. +AppState.addEventListener("change", notify); +if (isWeb && typeof document !== "undefined") { + document.addEventListener("visibilitychange", notify); + window.addEventListener("focus", notify); + window.addEventListener("blur", notify); +} + function subscribe(listener: () => void): () => void { listeners.add(listener); return () => listeners.delete(listener); @@ -27,24 +39,5 @@ function getSnapshot(): boolean { } export function useAppVisible(): boolean { - useEffect(() => { - const appStateSubscription = AppState.addEventListener("change", notify); - - if (isWeb && typeof document !== "undefined") { - document.addEventListener("visibilitychange", notify); - window.addEventListener("focus", notify); - window.addEventListener("blur", notify); - } - - return () => { - appStateSubscription.remove(); - if (isWeb && typeof document !== "undefined") { - document.removeEventListener("visibilitychange", notify); - window.removeEventListener("focus", notify); - window.removeEventListener("blur", notify); - } - }; - }, []); - return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); } diff --git a/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts index 92cb82b03..8bb916205 100644 --- a/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts +++ b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts @@ -5,7 +5,6 @@ import type { WorkspaceDescriptor } from "@/stores/session-store"; import { useTranslation } from "react-i18next"; import { useReplicaQuery } from "@/data/query"; import { workspaceTerminalsPushRoute } from "@/data/push-router"; -import { estimateTerminalViewportSize } from "@/terminal/runtime/terminal-size-cache"; import { buildTerminalsQueryKey, canCreateWorkspaceTerminal, @@ -136,20 +135,14 @@ export function useWorkspaceTerminals(input: UseWorkspaceTerminalsInput) { if (!client || !workspaceDirectory) { throw new Error(t("workspace.terminal.hostDisconnected")); } - // Seed the new PTY with the workspace's measured pane size so it isn't born at 80x24. - const estimatedSize = - estimateTerminalViewportSize({ serverId: normalizedServerId, cwd: workspaceDirectory }) ?? - undefined; const payload = _input?.profile ? await client.createTerminal(workspaceDirectory, _input.profile.name, undefined, { command: _input.profile.command, args: _input.profile.args, workspaceId: normalizedWorkspaceId || undefined, - size: estimatedSize, }) : await client.createTerminal(workspaceDirectory, undefined, undefined, { workspaceId: normalizedWorkspaceId || undefined, - size: estimatedSize, }); // The daemon reports a failed spawn (e.g. a profile command that isn't // installed) via payload.error with a null terminal. Surface it instead diff --git a/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts b/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts new file mode 100644 index 000000000..ff3d36649 --- /dev/null +++ b/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts @@ -0,0 +1,159 @@ +import { page } from "@vitest/browser/context"; +import { afterEach, describe, expect, it } from "vitest"; +import { TerminalEmulatorRuntime } from "./terminal-emulator-runtime"; + +/** + * Pins the mechanism behind "terminal stuck at 80x24". + * + * The daemon spawns a PTY at 80x24 and only resizes it when the client sends a resize frame. + * `handleTerminalResize` in terminal-pane only forwards a frame when the emitted fit carries + * `shouldClaim: true`. + * + * These tests show that a freshly mounted terminal NEVER emits such a fit. Every mount-time + * refit (the mount fit, the retry ladder, fonts.ready, the WebGL swap, visibility restore) is + * emitted with `shouldClaim: false`. The ResizeObserver's first delivery IS `shouldClaim: true`, + * but it is emitted without `force`, so the runtime's own dedupe drops it: the size has not + * changed since the mount fit. + * + * So after a fresh mount, in a pane whose size never changes, the client sends the daemon + * NOTHING. The PTY keeps whatever size it was created with. The terminal's size then depends + * entirely on either the size carried at subscribe, or an explicit forced reflow + * (`requestTerminalReflow` -> `runtime.resize({force, shouldClaim})`) -- which is exactly the + * path terminal-pane's focus effects can permanently latch off. + */ + +interface EmittedSize { + rows: number; + cols: number; + shouldClaim: boolean; +} + +interface MountedTerminal { + root: HTMLDivElement; + runtime: TerminalEmulatorRuntime; + sizes: EmittedSize[]; +} + +const mounted: MountedTerminal[] = []; + +afterEach(() => { + for (const entry of mounted.splice(0)) { + entry.runtime.unmount(); + entry.root.remove(); + } +}); + +function mountTerminal(input: { width: number; height: number }): MountedTerminal { + const root = document.createElement("div"); + root.style.cssText = `width:${input.width}px;height:${input.height}px;position:fixed;left:0;top:0;overflow:hidden`; + const host = document.createElement("div"); + host.style.cssText = "width:100%;height:100%"; + root.appendChild(host); + document.body.appendChild(root); + + const sizes: EmittedSize[] = []; + const runtime = new TerminalEmulatorRuntime(); + runtime.setCallbacks({ + callbacks: { + onResize: (size) => { + sizes.push(size as EmittedSize); + }, + }, + }); + runtime.mount({ + root, + host, + initialSnapshot: null, + scrollback: 1_000, + theme: { background: "#0b0b0b", foreground: "#e6e6e6", cursor: "#e6e6e6" }, + }); + + const entry = { root, runtime, sizes }; + mounted.push(entry); + return entry; +} + +function nextFrame(): Promise { + return new Promise((resolve) => { + requestAnimationFrame(() => { + resolve(); + }); + }); +} + +async function waitFor(input: { predicate: () => boolean; timeoutMs?: number }): Promise { + const startedAt = performance.now(); + const timeoutMs = input.timeoutMs ?? 2_000; + + while (!input.predicate()) { + if (performance.now() - startedAt > timeoutMs) { + throw new Error("Timed out waiting for terminal browser condition"); + } + await nextFrame(); + } +} + +/** + * Outlast the runtime's whole refit ladder (0/16/48/120/250/500/1000/2000ms) plus fonts and the + * WebGL swap, so nothing is still in flight when we assert. Unlike every other wait in this file + * this one cannot be a predicate: the point is to prove that no claim EVER arrives, and there is + * no event that says "the mount is done emitting". + */ +function settleMountRefits(): Promise { + return new Promise((resolve) => setTimeout(resolve, 2_600)); +} + +describe("a freshly mounted terminal never claims its size", () => { + it("emits many fits on mount, and not one of them claims the PTY size", async () => { + await page.viewport(1400, 900); + const terminal = mountTerminal({ width: 900, height: 600 }); + + await settleMountRefits(); + + // It really did refit repeatedly while mounting... + expect(terminal.sizes.length).toBeGreaterThan(1); + // ...and it measured a real viewport, not the daemon's default. + expect(terminal.sizes.at(-1)?.cols).not.toBe(80); + + // ...but every single one of those fits declined to own the PTY size, so terminal-pane + // forwards NOTHING to the daemon. This is why a terminal can sit at 80x24 while its xterm + // renders perfectly at the real size. + const claims = terminal.sizes.filter((size) => size.shouldClaim); + expect(claims).toEqual([]); + }); + + it("only an explicit forced reflow claims — the path the focus latch can kill", async () => { + await page.viewport(1400, 900); + const terminal = mountTerminal({ width: 900, height: 600 }); + await settleMountRefits(); + expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([]); + const settledSize = terminal.sizes.at(-1); + + // This is what requestTerminalReflow does. It is the ONLY thing that makes a + // stable-sized, freshly mounted terminal tell the daemon how big it is. + terminal.runtime.resize({ force: true, shouldClaim: true }); + + // It claims the size the terminal actually settled at, so the PTY ends up matching what the + // user sees rendered. + expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([ + { rows: settledSize?.rows, cols: settledSize?.cols, shouldClaim: true }, + ]); + }); + + it("a genuine size change does claim — which is why resizing the window unsticks it", async () => { + await page.viewport(1400, 900); + const terminal = mountTerminal({ width: 900, height: 600 }); + await settleMountRefits(); + expect(terminal.sizes.filter((size) => size.shouldClaim)).toEqual([]); + + // The ResizeObserver fit IS shouldClaim:true; it is only suppressed while the size is + // unchanged. Change the box and it gets through -- the user-visible "resize the window and + // it fixes itself" behaviour. + terminal.root.style.width = "1300px"; + + await waitFor({ predicate: () => terminal.sizes.some((size) => size.shouldClaim) }); + const claims = terminal.sizes.filter((size) => size.shouldClaim); + // The claim carries the NEW width, not the size it mounted at. + expect(claims.at(-1)?.cols).toBeGreaterThan(terminal.sizes[0]?.cols ?? 0); + }); +}); diff --git a/packages/app/src/terminal/runtime/terminal-size-cache.test.ts b/packages/app/src/terminal/runtime/terminal-size-cache.test.ts deleted file mode 100644 index 41aaad19e..000000000 --- a/packages/app/src/terminal/runtime/terminal-size-cache.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { beforeEach, describe, expect, it } from "vitest"; -import { - estimateTerminalViewportSize, - rememberTerminalViewportSize, - resetTerminalViewportSizeCacheForTests, -} from "./terminal-size-cache"; - -describe("terminal-size-cache", () => { - beforeEach(() => { - resetTerminalViewportSizeCacheForTests(); - }); - - it("returns null before any size has been measured", () => { - expect(estimateTerminalViewportSize({ serverId: "s1", cwd: "/repo" })).toBeNull(); - }); - - it("returns the last measured size for the same workspace", () => { - rememberTerminalViewportSize({ serverId: "s1", cwd: "/repo", size: { rows: 55, cols: 136 } }); - expect(estimateTerminalViewportSize({ serverId: "s1", cwd: "/repo" })).toEqual({ - rows: 55, - cols: 136, - }); - }); - - it("falls back to the most recent size from another workspace", () => { - rememberTerminalViewportSize({ serverId: "s1", cwd: "/repo-a", size: { rows: 40, cols: 100 } }); - // No terminal has been measured in /repo-b yet — the estimate uses the global most-recent size. - expect(estimateTerminalViewportSize({ serverId: "s1", cwd: "/repo-b" })).toEqual({ - rows: 40, - cols: 100, - }); - }); - - it("prefers the same-workspace size over the global most-recent", () => { - rememberTerminalViewportSize({ serverId: "s1", cwd: "/repo-a", size: { rows: 40, cols: 100 } }); - rememberTerminalViewportSize({ serverId: "s1", cwd: "/repo-b", size: { rows: 55, cols: 136 } }); - // /repo-a keeps its own measured size even though /repo-b was measured more recently. - expect(estimateTerminalViewportSize({ serverId: "s1", cwd: "/repo-a" })).toEqual({ - rows: 40, - cols: 100, - }); - }); - - it("keys by server + cwd so different hosts do not collide", () => { - rememberTerminalViewportSize({ serverId: "s1", cwd: "/repo", size: { rows: 40, cols: 100 } }); - rememberTerminalViewportSize({ serverId: "s2", cwd: "/repo", size: { rows: 55, cols: 136 } }); - expect(estimateTerminalViewportSize({ serverId: "s1", cwd: "/repo" })).toEqual({ - rows: 40, - cols: 100, - }); - expect(estimateTerminalViewportSize({ serverId: "s2", cwd: "/repo" })).toEqual({ - rows: 55, - cols: 136, - }); - }); -}); diff --git a/packages/app/src/terminal/runtime/terminal-size-cache.ts b/packages/app/src/terminal/runtime/terminal-size-cache.ts deleted file mode 100644 index 3f965ce25..000000000 --- a/packages/app/src/terminal/runtime/terminal-size-cache.ts +++ /dev/null @@ -1,50 +0,0 @@ -export interface TerminalViewportSize { - rows: number; - cols: number; -} - -function cacheKey(input: { serverId: string; cwd: string }): string { - // JSON-encode the pair so a `:` inside serverId or cwd (e.g. a Windows `C:\` path) can't - // make two different (serverId, cwd) pairs collide onto the same key. - return JSON.stringify([input.serverId, input.cwd]); -} - -const sizeByWorkspace = new Map(); -let mostRecentSize: TerminalViewportSize | null = null; - -/** - * Remember the latest measured terminal viewport size for a workspace. Every terminal in a - * workspace renders into the same pane, so this is the best estimate of the size a *new* - * terminal in that workspace will render at — used to seed the PTY at creation time instead - * of the daemon's 80x24 default (which otherwise shows briefly, or sticks, until the first - * resize lands). - */ -export function rememberTerminalViewportSize(input: { - serverId: string; - cwd: string; - size: TerminalViewportSize; -}): void { - const size: TerminalViewportSize = { rows: input.size.rows, cols: input.size.cols }; - sizeByWorkspace.set(cacheKey(input), size); - mostRecentSize = size; -} - -/** - * Best estimate of the viewport size a new terminal in this workspace will render at: - * the last measured size for the same workspace, else the most recently measured size - * anywhere (panes are usually the same size across workspaces on one device), else null - * when nothing has been measured yet this session — in which case the daemon keeps its - * 80x24 default and the first resize corrects it as before. - */ -export function estimateTerminalViewportSize(input: { - serverId: string; - cwd: string; -}): TerminalViewportSize | null { - return sizeByWorkspace.get(cacheKey(input)) ?? mostRecentSize; -} - -/** Test-only: clear all remembered sizes so cases don't leak into each other. */ -export function resetTerminalViewportSizeCacheForTests(): void { - sizeByWorkspace.clear(); - mostRecentSize = null; -} diff --git a/packages/protocol/src/messages.create-terminal-size.test.ts b/packages/protocol/src/messages.create-terminal-size.test.ts index 0cc88dd98..10a61dae4 100644 --- a/packages/protocol/src/messages.create-terminal-size.test.ts +++ b/packages/protocol/src/messages.create-terminal-size.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from "vitest"; import { CreateTerminalRequestSchema } from "./messages"; -// COMPAT(createTerminalSize): the size field is optional so old clients (which send no size) -// still parse, and old daemons ignore it. These tests pin that contract. +// The optional size field stays accepted permanently: released v0.1.107 clients send it, newer +// app clients don't, and programmatic callers may. These tests pin that contract. describe("CreateTerminalRequest size", () => { const base = { type: "create_terminal_request" as const, diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index a5ad3d22d..98fd8fe63 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -2122,10 +2122,10 @@ export const CreateTerminalRequestSchema = z.object({ agentId: z.string().optional(), command: z.string().optional(), args: z.array(z.string()).optional(), - // COMPAT(createTerminalSize): added in v0.1.107, drop the optional gate when floor >= v0.1.107. - // The client seeds the PTY with its measured viewport size so a new terminal isn't born at the - // 80x24 default and then visibly reflowed. Old daemons ignore this field and start at 80x24; - // the client's first resize corrects it as before. + // Initial PTY size. Added in v0.1.107; the app no longer sends it (the estimate cache that fed + // it was removed — the pane-focus resize claim sizes the PTY instead). Kept and honored + // permanently: released v0.1.107 clients still send it, and programmatic callers may pass an + // exact size. Daemons without it start at 80x24 and the first resize corrects that. size: z .object({ rows: z.number().int().positive(),