From 2658132384281de3c79db7d9bab5ec7996daefa3 Mon Sep 17 00:00:00 2001 From: Christoph Leiter Date: Sun, 12 Jul 2026 21:26:52 +0200 Subject: [PATCH] fix(terminal): create PTYs at the client viewport size, not 80x24 (#2023) New terminals were always spawned by the daemon at the hardcoded 80x24 default and only resized once the client's first resize arrived. On a slow first mount that leaves a window -- or a stuck state -- where a new terminal renders far smaller than its pane (e.g. vim in a corner). Seed the PTY with the client's measured viewport size at creation time so it is born at the right size and the race window is gone. - protocol: optional `size` on create_terminal_request. Old daemons ignore it and keep 80x24; old clients send nothing. Fully backward compatible, no capability gate. - client: a small last-measured-size cache keyed by serverId+cwd (every terminal in a workspace shares the pane) with a most-recent fallback; written on fit, read at create. - server: thread rows/cols through the controller, terminal manager, and the worker-thread boundary into the existing size-aware createTerminal. Tests: cache logic, protocol back-compat parse, controller forwarding. Co-authored-by: Claude Opus 4.8 --- packages/app/src/components/terminal-pane.tsx | 4 ++ .../terminals/use-workspace-terminals.ts | 7 +++ .../runtime/terminal-size-cache.test.ts | 56 +++++++++++++++++++ .../terminal/runtime/terminal-size-cache.ts | 50 +++++++++++++++++ packages/client/src/daemon-client.ts | 9 ++- .../src/messages.create-terminal-size.test.ts | 34 +++++++++++ packages/protocol/src/messages.ts | 10 ++++ .../server/src/terminal/terminal-manager.ts | 6 ++ .../terminal-session-controller.test.ts | 53 ++++++++++++++++++ .../terminal/terminal-session-controller.ts | 2 + .../src/terminal/terminal-worker-protocol.ts | 2 + 11 files changed, 232 insertions(+), 1 deletion(-) create mode 100644 packages/app/src/terminal/runtime/terminal-size-cache.test.ts create mode 100644 packages/app/src/terminal/runtime/terminal-size-cache.ts create mode 100644 packages/protocol/src/messages.create-terminal-size.test.ts diff --git a/packages/app/src/components/terminal-pane.tsx b/packages/app/src/components/terminal-pane.tsx index 05f398d3c..f6d9c655f 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -21,6 +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 { TerminalStreamController, type TerminalStreamControllerStatus, @@ -635,6 +636,9 @@ 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/screens/workspace/terminals/use-workspace-terminals.ts b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts index 8bb916205..92cb82b03 100644 --- a/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts +++ b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts @@ -5,6 +5,7 @@ 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, @@ -135,14 +136,20 @@ 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-size-cache.test.ts b/packages/app/src/terminal/runtime/terminal-size-cache.test.ts new file mode 100644 index 000000000..41aaad19e --- /dev/null +++ b/packages/app/src/terminal/runtime/terminal-size-cache.test.ts @@ -0,0 +1,56 @@ +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 new file mode 100644 index 000000000..3f965ce25 --- /dev/null +++ b/packages/app/src/terminal/runtime/terminal-size-cache.ts @@ -0,0 +1,50 @@ +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/client/src/daemon-client.ts b/packages/client/src/daemon-client.ts index 3919b0eb5..571f9221e 100644 --- a/packages/client/src/daemon-client.ts +++ b/packages/client/src/daemon-client.ts @@ -4275,7 +4275,13 @@ export class DaemonClient { cwd: string, name?: string, requestId?: string, - options?: { agentId?: string; command?: string; args?: string[]; workspaceId?: string }, + options?: { + agentId?: string; + command?: string; + args?: string[]; + workspaceId?: string; + size?: { rows: number; cols: number }; + }, ): Promise { const resolvedRequestId = this.createRequestId(requestId); const message = SessionInboundMessageSchema.parse({ @@ -4286,6 +4292,7 @@ export class DaemonClient { command: options?.command, args: options?.args, ...(options?.workspaceId !== undefined ? { workspaceId: options.workspaceId } : {}), + ...(options?.size !== undefined ? { size: options.size } : {}), requestId: resolvedRequestId, }); return this.sendCorrelatedRequest({ diff --git a/packages/protocol/src/messages.create-terminal-size.test.ts b/packages/protocol/src/messages.create-terminal-size.test.ts new file mode 100644 index 000000000..0cc88dd98 --- /dev/null +++ b/packages/protocol/src/messages.create-terminal-size.test.ts @@ -0,0 +1,34 @@ +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. +describe("CreateTerminalRequest size", () => { + const base = { + type: "create_terminal_request" as const, + cwd: "/work/repo", + requestId: "req-1", + }; + + it("parses a request without a size (old client / back-compat)", () => { + const parsed = CreateTerminalRequestSchema.parse({ ...base }); + expect(parsed).toEqual(base); + }); + + it("parses a request carrying a viewport size", () => { + const parsed = CreateTerminalRequestSchema.parse({ ...base, size: { rows: 55, cols: 136 } }); + expect(parsed.size).toEqual({ rows: 55, cols: 136 }); + }); + + it("rejects a non-positive or non-integer size", () => { + expect(() => + CreateTerminalRequestSchema.parse({ ...base, size: { rows: 0, cols: 80 } }), + ).toThrow(); + expect(() => + CreateTerminalRequestSchema.parse({ ...base, size: { rows: 24, cols: -1 } }), + ).toThrow(); + expect(() => + CreateTerminalRequestSchema.parse({ ...base, size: { rows: 24.5, cols: 80 } }), + ).toThrow(); + }); +}); diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index c19c9af8b..a1465c92b 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -1984,6 +1984,16 @@ 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. + size: z + .object({ + rows: z.number().int().positive(), + cols: z.number().int().positive(), + }) + .optional(), requestId: z.string(), }); diff --git a/packages/server/src/terminal/terminal-manager.ts b/packages/server/src/terminal/terminal-manager.ts index 8bd35ac5d..df4ced389 100644 --- a/packages/server/src/terminal/terminal-manager.ts +++ b/packages/server/src/terminal/terminal-manager.ts @@ -60,6 +60,8 @@ export interface TerminalManager { env?: Record; command?: string; args?: string[]; + rows?: number; + cols?: number; activityToken?: string; activityUrl?: string | null; }): Promise; @@ -315,6 +317,8 @@ export function createTerminalManager( env?: Record; command?: string; args?: string[]; + rows?: number; + cols?: number; activityToken?: string; activityUrl?: string | null; }): Promise { @@ -348,6 +352,8 @@ export function createTerminalManager( ...(options.title ? { title: options.title } : {}), ...(options.command ? { command: options.command } : {}), ...(options.args ? { args: options.args } : {}), + ...(options.rows !== undefined ? { rows: options.rows } : {}), + ...(options.cols !== undefined ? { cols: options.cols } : {}), ...(mergedEnv ? { env: mergedEnv } : {}), activityEnv, }), diff --git a/packages/server/src/terminal/terminal-session-controller.test.ts b/packages/server/src/terminal/terminal-session-controller.test.ts index 3c6dee01b..81c3a15d6 100644 --- a/packages/server/src/terminal/terminal-session-controller.test.ts +++ b/packages/server/src/terminal/terminal-session-controller.test.ts @@ -259,6 +259,59 @@ describe("terminal-session-controller legacy terminal creation", () => { }, ]); }); + + test("forwards the client-provided viewport size to the terminal manager", async () => { + const outboundMessages: SessionOutboundMessage[] = []; + const createTerminal = vi.fn( + async (options: Parameters[0]) => + listSession({ + id: "term-1", + name: options.name ?? "Terminal 1", + cwd: options.cwd, + workspaceId: options.workspaceId, + }), + ); + const terminalManager: TerminalManager = { + getTerminals: vi.fn(), + createTerminal, + registerCwdEnv: vi.fn(), + validateTerminalActivityToken: vi.fn(() => "unknown"), + getTerminal: vi.fn(), + getTerminalState: vi.fn(), + setTerminalTitle: vi.fn(), + setTerminalActivity: vi.fn(), + clearTerminalAttention: vi.fn(), + killTerminal: vi.fn(), + killTerminalAndWait: vi.fn(), + captureTerminal: vi.fn(), + listDirectories: vi.fn(() => []), + killAll: vi.fn(), + subscribeTerminalsChanged: vi.fn(() => vi.fn()), + subscribeTerminalActivity: vi.fn(() => vi.fn()), + subscribeTerminalWorkspaceContributionChanged: vi.fn(() => vi.fn()), + }; + const controller = new TerminalSessionController({ + terminalManager, + emit: (message) => outboundMessages.push(message), + emitBinary: vi.fn(), + hasBinaryChannel: () => true, + isPathWithinRoot: isSameOrDescendantPath, + sessionLogger: createLogger(), + listTerminalWorkspaceRefs: async () => [], + }); + + await controller.dispatch({ + type: "create_terminal_request", + cwd: "/work/repo", + workspaceId: "ws-1", + size: { rows: 55, cols: 136 }, + requestId: "req-size", + }); + + expect(createTerminal).toHaveBeenCalledWith( + expect.objectContaining({ cwd: "/work/repo", workspaceId: "ws-1", rows: 55, cols: 136 }), + ); + }); }); async function flushMicrotasks(): Promise { diff --git a/packages/server/src/terminal/terminal-session-controller.ts b/packages/server/src/terminal/terminal-session-controller.ts index dd6c54927..dd7d053d6 100644 --- a/packages/server/src/terminal/terminal-session-controller.ts +++ b/packages/server/src/terminal/terminal-session-controller.ts @@ -549,6 +549,8 @@ export class TerminalSessionController { name: msg.name, command: msg.command, args: msg.args, + rows: msg.size?.rows, + cols: msg.size?.cols, }); this.ensureExitSubscription(session); this.emit({ diff --git a/packages/server/src/terminal/terminal-worker-protocol.ts b/packages/server/src/terminal/terminal-worker-protocol.ts index 2da2f5f12..6b722c978 100644 --- a/packages/server/src/terminal/terminal-worker-protocol.ts +++ b/packages/server/src/terminal/terminal-worker-protocol.ts @@ -27,6 +27,8 @@ export interface WorkerCreateTerminalOptions { env?: Record; command?: string; args?: string[]; + rows?: number; + cols?: number; activityToken?: string; activityUrl?: string | null; }