mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
50
packages/app/src/terminal/runtime/terminal-size-cache.ts
Normal file
50
packages/app/src/terminal/runtime/terminal-size-cache.ts
Normal file
@@ -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<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;
|
||||
}
|
||||
@@ -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<CreateTerminalPayload> {
|
||||
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({
|
||||
|
||||
34
packages/protocol/src/messages.create-terminal-size.test.ts
Normal file
34
packages/protocol/src/messages.create-terminal-size.test.ts
Normal file
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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(),
|
||||
});
|
||||
|
||||
|
||||
@@ -60,6 +60,8 @@ export interface TerminalManager {
|
||||
env?: Record<string, string>;
|
||||
command?: string;
|
||||
args?: string[];
|
||||
rows?: number;
|
||||
cols?: number;
|
||||
activityToken?: string;
|
||||
activityUrl?: string | null;
|
||||
}): Promise<TerminalSession>;
|
||||
@@ -315,6 +317,8 @@ export function createTerminalManager(
|
||||
env?: Record<string, string>;
|
||||
command?: string;
|
||||
args?: string[];
|
||||
rows?: number;
|
||||
cols?: number;
|
||||
activityToken?: string;
|
||||
activityUrl?: string | null;
|
||||
}): Promise<TerminalSession> {
|
||||
@@ -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,
|
||||
}),
|
||||
|
||||
@@ -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<TerminalManager["createTerminal"]>[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<void> {
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -27,6 +27,8 @@ export interface WorkerCreateTerminalOptions {
|
||||
env?: Record<string, string>;
|
||||
command?: string;
|
||||
args?: string[];
|
||||
rows?: number;
|
||||
cols?: number;
|
||||
activityToken?: string;
|
||||
activityUrl?: string | null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user