Fix terminal resize race (#2059)

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 },
+    ]);
+  });
+});

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 };
+}

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;
       }

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);
 }

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

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<void> {
+  return new Promise((resolve) => {
+    requestAnimationFrame(() => {
+      resolve();
+    });
+  });
+}
+
+async function waitFor(input: { predicate: () => boolean; timeoutMs?: number }): Promise<void> {
+  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<void> {
+  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);
+  });
+});

packages/app/src/terminal/runtime/terminal-size-cache.test.ts

@@ -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,
-    });
-  });
-});

packages/app/src/terminal/runtime/terminal-size-cache.ts

@@ -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<string, TerminalViewportSize>();
-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;
-}