diff --git a/packages/app/e2e/terminal-stuck-size.spec.ts b/packages/app/e2e/terminal-stuck-size.spec.ts index ebdfd0110..9c498e44a 100644 --- a/packages/app/e2e/terminal-stuck-size.spec.ts +++ b/packages/app/e2e/terminal-stuck-size.spec.ts @@ -1,5 +1,6 @@ import { test, expect, type Page } from "./fixtures"; import { TerminalE2EHarness } from "./helpers/terminal-dsl"; +import { getTerminalBufferText } from "./helpers/terminal-perf"; import { buildHostWorkspaceRoute } from "../src/utils/host-routes"; import { getServerId } from "./helpers/server-id"; @@ -8,18 +9,14 @@ import { getServerId } from "./helpers/server-id"; * 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. + * mounted terminal starts its claim from terminal-pane's pane-focus reflow effect. Previously, if + * that claim was emitted while the app was not actively visible, `handleTerminalResize` dropped + * it without a retry, leaving the PTY at 80x24 while xterm rendered the real pane size. * * 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. + * instead of racing real OS focus. * * 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. @@ -58,31 +55,6 @@ async function readRenderedTerminalSize(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(); @@ -112,116 +84,10 @@ function requireTerminalSize(size: RenderedTerminalSize | null): RenderedTermina 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; +function parseLatestSttySize(bufferText: string): RenderedTerminalSize | null { + const matches = [...bufferText.matchAll(/S\d+=(\d+) (\d+)=/g)]; + const match = matches.at(-1); + return match?.[1] && match[2] ? { rows: Number(match[1]), cols: Number(match[2]) } : null; } test.describe("terminal PTY size claim under lost window focus", () => { @@ -239,16 +105,14 @@ test.describe("terminal PTY size claim under lost window focus", () => { 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. + // Wait for the daemon to know about the first terminal 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; @@ -265,14 +129,19 @@ test.describe("terminal PTY size claim under lost window focus", () => { 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. + // Keep the app blurred through the complete mount-time refit ladder, which runs out to 2s. 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([]); + await harness.client.subscribeTerminal(newTerminalId); + + // Confirm the PTY is still at its spawn size before focus returns. + harness.client.sendTerminalInput(newTerminalId, { + type: "input", + data: 'echo "S0=$(stty size)="\n', + }); + await expect + .poll(async () => getTerminalBufferText(page), { timeout: 15_000 }) + .toMatch(/S0=24 80=/); // ...and comes back. await setWindowFocused(page, true); @@ -282,29 +151,20 @@ test.describe("terminal PTY size claim under lost window focus", () => { // 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 + // 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', - }); - + let probe = 1; 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 }); + .poll( + async () => { + harness.client.sendTerminalInput(newTerminalId, { + type: "input", + data: `echo "S${probe++}=$(stty size)="\n`, + }); + return parseLatestSttySize(await getTerminalBufferText(page)); + }, + { timeout: 15_000, intervals: [100, 250, 500] }, + ) + .toEqual({ rows: rendered.rows, cols: rendered.cols }); }); }); diff --git a/packages/app/src/components/terminal-pane-focus-claim.test.ts b/packages/app/src/components/terminal-pane-focus-claim.test.ts new file mode 100644 index 000000000..67e540f52 --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-claim.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "vitest"; +import { + EMPTY_FOCUS_CLAIM_STATE, + canRequestFocusClaim, + reconcileFocusClaim, + settleFocusClaim, + type FocusClaimState, +} from "./terminal-pane-focus-claim"; + +function request( + state: FocusClaimState, + input: { key: string | null; canRequest: boolean }, +): FocusClaimState { + return reconcileFocusClaim(state, input).state; +} + +describe("terminal pane focus claim", () => { + it("waits for both the client and renderer before requesting a claim", () => { + const withoutClient = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: false, + isConnected: true, + isRendererReady: true, + }); + const withoutRenderer = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: true, + isConnected: true, + isRendererReady: false, + }); + + expect([withoutClient, withoutRenderer]).toEqual([false, false]); + }); + + it("does not deliver a requested claim after the host disconnects", () => { + const disconnected = canRequestFocusClaim({ + isWorkspaceFocused: true, + isAppVisible: true, + isClientReady: true, + isConnected: false, + isRendererReady: true, + }); + + expect(disconnected).toBe(false); + }); + + it("claims once per continuous pane-focus period after send", () => { + const firstRequest = reconcileFocusClaim(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const sent = settleFocusClaim(firstRequest.state, { + key: "ws:term-1", + sent: true, + }); + const repeated = reconcileFocusClaim(sent, { + key: "ws:term-1", + canRequest: true, + }); + + expect(firstRequest.shouldRequest).toBe(true); + expect(repeated).toEqual({ + state: { claimedKey: "ws:term-1", requestedKey: null }, + shouldRequest: false, + }); + }); + + it("defers until the claim can be requested", () => { + const unavailable = reconcileFocusClaim(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: false, + }); + const available = reconcileFocusClaim(unavailable.state, { + key: "ws:term-1", + canRequest: true, + }); + + expect(unavailable.shouldRequest).toBe(false); + expect(available.shouldRequest).toBe(true); + }); + + it("retries when readiness changes before the requested claim lands", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const hiddenBeforeDelivery = request(requested, { + key: "ws:term-1", + canRequest: false, + }); + const visibleAgain = reconcileFocusClaim(hiddenBeforeDelivery, { + key: "ws:term-1", + canRequest: true, + }); + + expect(visibleAgain).toEqual({ + state: { claimedKey: null, requestedKey: "ws:term-1" }, + shouldRequest: true, + }); + }); + + it("retries a requested claim that was not sent", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const dropped = settleFocusClaim(requested, { + key: "ws:term-1", + sent: false, + }); + const retry = reconcileFocusClaim(dropped, { + key: "ws:term-1", + canRequest: true, + }); + + expect(retry.shouldRequest).toBe(true); + }); + + it("re-arms after pane blur or terminal change", () => { + const requested = request(EMPTY_FOCUS_CLAIM_STATE, { + key: "ws:term-1", + canRequest: true, + }); + const sent = settleFocusClaim(requested, { key: "ws:term-1", sent: true }); + const blurred = request(sent, { key: null, canRequest: true }); + const refocused = reconcileFocusClaim(blurred, { key: "ws:term-1", canRequest: true }); + const changed = reconcileFocusClaim(sent, { key: "ws:term-2", canRequest: true }); + + expect(refocused.shouldRequest).toBe(true); + expect(changed.shouldRequest).toBe(true); + }); +}); diff --git a/packages/app/src/components/terminal-pane-focus-claim.ts b/packages/app/src/components/terminal-pane-focus-claim.ts new file mode 100644 index 000000000..03f9ec2fa --- /dev/null +++ b/packages/app/src/components/terminal-pane-focus-claim.ts @@ -0,0 +1,73 @@ +export interface FocusClaimState { + claimedKey: string | null; + requestedKey: string | null; +} + +export interface FocusClaimStep { + state: FocusClaimState; + shouldRequest: boolean; +} + +interface FocusClaimReadiness { + isWorkspaceFocused: boolean; + isAppVisible: boolean; + isClientReady: boolean; + isConnected: boolean; + isRendererReady: boolean; +} + +export const EMPTY_FOCUS_CLAIM_STATE: FocusClaimState = { + claimedKey: null, + requestedKey: null, +}; + +export function canRequestFocusClaim(input: FocusClaimReadiness): boolean { + return ( + input.isWorkspaceFocused && + input.isAppVisible && + input.isClientReady && + input.isConnected && + input.isRendererReady + ); +} + +export function reconcileFocusClaim( + state: FocusClaimState, + input: { key: string | null; canRequest: boolean }, +): FocusClaimStep { + if (input.key === null) { + return { state: EMPTY_FOCUS_CLAIM_STATE, shouldRequest: false }; + } + if (state.claimedKey === input.key) { + return { + state: { claimedKey: input.key, requestedKey: null }, + shouldRequest: false, + }; + } + if (!input.canRequest) { + return { + state: { claimedKey: state.claimedKey, requestedKey: null }, + shouldRequest: false, + }; + } + if (state.requestedKey === input.key) { + return { state, shouldRequest: false }; + } + return { + state: { claimedKey: state.claimedKey, requestedKey: input.key }, + shouldRequest: true, + }; +} + +export function settleFocusClaim( + state: FocusClaimState, + input: { key: string; sent: boolean }, +): FocusClaimState { + if (state.requestedKey !== input.key) { + return state; + } + return { + claimedKey: input.sent ? input.key : state.claimedKey, + requestedKey: null, + }; +} diff --git a/packages/app/src/components/terminal-pane-focus-latch.test.ts b/packages/app/src/components/terminal-pane-focus-latch.test.ts deleted file mode 100644 index 41dd60de2..000000000 --- a/packages/app/src/components/terminal-pane-focus-latch.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -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 deleted file mode 100644 index 9d3d6196b..000000000 --- a/packages/app/src/components/terminal-pane-focus-latch.ts +++ /dev/null @@ -1,33 +0,0 @@ -/** - * 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 52955c206..3eb607857 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -21,7 +21,12 @@ import { resolvePendingModifierDataInput, } from "@/utils/terminal-keys"; import { getWorkspaceTerminalSession } from "@/terminal/runtime/workspace-terminal-session"; -import { resolveFocusLatchStep } from "./terminal-pane-focus-latch"; +import { + EMPTY_FOCUS_CLAIM_STATE, + canRequestFocusClaim, + reconcileFocusClaim, + settleFocusClaim, +} from "./terminal-pane-focus-claim"; import { TerminalStreamController, type TerminalStreamControllerStatus, @@ -222,7 +227,7 @@ export function TerminalPane({ const pendingTerminalInputRef = useRef([]); const keyboardRefitTimeoutsRef = useRef>>([]); const lastAutoFocusKeyRef = useRef(null); - const lastPaneFocusResizeKeyRef = useRef(null); + const paneFocusResizeClaimRef = useRef(EMPTY_FOCUS_CLAIM_STATE); const initialSnapshot = workspaceTerminalSession.snapshots.get({ terminalId }); useEffect(() => { @@ -261,40 +266,48 @@ export function TerminalPane({ ); useEffect(() => { - // 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) { + if (isMobile || !isPaneFocused || !terminalId) { + lastAutoFocusKeyRef.current = null; + return; + } + if (!isWorkspaceFocused) { + return; + } + const focusKey = `${scopeKey}:${terminalId}`; + if (lastAutoFocusKeyRef.current !== focusKey) { + lastAutoFocusKeyRef.current = focusKey; requestTerminalFocus(); } }, [isMobile, isPaneFocused, isWorkspaceFocused, requestTerminalFocus, scopeKey, terminalId]); useEffect(() => { - // 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, + const canRequest = canRequestFocusClaim({ + isWorkspaceFocused, + isAppVisible, + isClientReady: client !== null, + isConnected, + isRendererReady: rendererReadyStreamKey === terminalStreamKey, }); - lastPaneFocusResizeKeyRef.current = step.latchedKey; - if (step.fire) { + const step = reconcileFocusClaim(paneFocusResizeClaimRef.current, { + key: !isPaneFocused || !terminalId ? null : `${scopeKey}:${terminalId}`, + canRequest, + }); + paneFocusResizeClaimRef.current = step.state; + if (step.shouldRequest) { lastSentTerminalSizeRef.current = null; requestTerminalReflow(); } }, [ + client, isAppVisible, + isConnected, isPaneFocused, isWorkspaceFocused, + rendererReadyStreamKey, requestTerminalReflow, scopeKey, terminalId, + terminalStreamKey, ]); const handleTerminalFocus = useCallback(() => { @@ -635,23 +648,40 @@ export function TerminalPane({ const normalizedCols = Math.floor(cols); const nextSize = { rows: normalizedRows, cols: normalizedCols }; measuredTerminalSizeRef.current = nextSize; - if (!input.shouldClaim || !client || !terminalId || !isWorkspaceFocused || !isAppVisible) { + if (!input.shouldClaim) { return; } - const previousSent = lastSentTerminalSizeRef.current; - if ( - previousSent && - previousSent.rows === normalizedRows && - previousSent.cols === normalizedCols - ) { - return; - } - lastSentTerminalSizeRef.current = nextSize; - client.sendTerminalInput(terminalId, { - type: "resize", - rows: normalizedRows, - cols: normalizedCols, + let sent = false; + const canSend = canRequestFocusClaim({ + isWorkspaceFocused, + isAppVisible, + isClientReady: client !== null, + isConnected, + isRendererReady: true, }); + if (client && terminalId && canSend) { + const previousSent = lastSentTerminalSizeRef.current; + if ( + !previousSent || + previousSent.rows !== normalizedRows || + previousSent.cols !== normalizedCols + ) { + lastSentTerminalSizeRef.current = nextSize; + client.sendTerminalInput(terminalId, { + type: "resize", + rows: normalizedRows, + cols: normalizedCols, + }); + } + sent = true; + } + const requestedKey = paneFocusResizeClaimRef.current.requestedKey; + if (requestedKey) { + paneFocusResizeClaimRef.current = settleFocusClaim(paneFocusResizeClaimRef.current, { + key: requestedKey, + sent, + }); + } }, ); diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts index baa6ae350..1c191b1b1 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.browser.test.ts @@ -66,6 +66,10 @@ async function waitFor(input: { predicate: () => boolean; timeoutMs?: number }): } } +function settleMountRefits(): Promise { + return new Promise((resolve) => setTimeout(resolve, 2_600)); +} + function createTerminalHost(input: { width: number; height: number; @@ -204,8 +208,17 @@ describe("terminal emulator runtime in a real browser", () => { const mounted = createTerminalHost({ width: 720, height: 360 }); await waitFor({ predicate: () => mounted.sizes.length > 0 }); + await settleMountRefits(); - expect(mounted.sizes[0]?.shouldClaim).toBe(false); + expect(mounted.sizes.length).toBeGreaterThan(1); + expect(mounted.sizes.filter((size) => size.shouldClaim)).toEqual([]); + + const settledSize = latestSize(mounted.sizes); + mounted.runtime.resize({ force: true, shouldClaim: true }); + + expect(mounted.sizes.filter((size) => size.shouldClaim)).toEqual([ + { ...settledSize, shouldClaim: true }, + ]); }); it("reports a larger PTY size when the terminal container grows", async () => { 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 deleted file mode 100644 index ff3d36649..000000000 --- a/packages/app/src/terminal/runtime/terminal-mount-claim.browser.test.ts +++ /dev/null @@ -1,159 +0,0 @@ -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); - }); -});