From b873523eab7000d9da4aa594b0985c39c34d868d Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 23 Mar 2026 14:47:27 +0700 Subject: [PATCH] fix(terminal): skip resize snapshots and standardize key encoding --- .../app/src/components/split-container.tsx | 13 ++++ .../app/src/components/terminal-emulator.tsx | 62 ++++++++++--------- packages/app/src/components/terminal-pane.tsx | 4 +- .../runtime/terminal-stream-controller.ts | 33 ++++++++++ .../server/daemon-e2e/terminal.e2e.test.ts | 16 ++--- packages/server/src/server/session.ts | 7 +++ .../src/shared/terminal-key-input.test.ts | 9 +-- .../server/src/shared/terminal-key-input.ts | 15 +++-- packages/server/src/terminal/terminal.test.ts | 52 +++++++++++++++- packages/server/src/terminal/terminal.ts | 42 +++---------- 10 files changed, 167 insertions(+), 86 deletions(-) diff --git a/packages/app/src/components/split-container.tsx b/packages/app/src/components/split-container.tsx index 3089c9e01..c2bf78bb3 100644 --- a/packages/app/src/components/split-container.tsx +++ b/packages/app/src/components/split-container.tsx @@ -175,6 +175,19 @@ const MountedTabSlot = memo(function MountedTabSlot({ paneId, buildPaneContentModel, }: MountedTabSlotProps) { + useEffect(() => { + if (tabDescriptor.target.kind !== "terminal") { + return; + } + console.log("[terminal-tab-slot]", { + paneId, + tabId: tabDescriptor.tabId, + terminalId: tabDescriptor.target.terminalId, + isVisible, + isPaneFocused, + }); + }, [isPaneFocused, isVisible, paneId, tabDescriptor]); + const content = useMemo( () => buildPaneContentModel({ diff --git a/packages/app/src/components/terminal-emulator.tsx b/packages/app/src/components/terminal-emulator.tsx index 6404712cb..867522f22 100644 --- a/packages/app/src/components/terminal-emulator.tsx +++ b/packages/app/src/components/terminal-emulator.tsx @@ -1,7 +1,8 @@ "use dom"; -import { forwardRef, useEffect, useImperativeHandle, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState, type Ref } from "react"; import type { DOMProps } from "expo/dom"; +import { useDOMImperativeHandle, type DOMImperativeFactory } from "expo/dom"; import "@xterm/xterm/css/xterm.css"; import type { ITheme } from "@xterm/xterm"; import type { TerminalState } from "@server/shared/messages"; @@ -13,6 +14,12 @@ import { computeVerticalScrollbarGeometry, } from "./web-desktop-scrollbar.math"; +export interface TerminalEmulatorHandle { + writeOutput: (text: string) => void; + renderSnapshot: (state: TerminalState | null) => void; + clear: () => void; +} + const SCROLLBAR_HANDLE_WIDTH_IDLE = 6; const SCROLLBAR_HANDLE_WIDTH_ACTIVE = 9; const SCROLLBAR_HANDLE_GRAB_WIDTH = 18; @@ -64,13 +71,14 @@ function buildXtermThemeKey(theme: ITheme): string { interface TerminalEmulatorProps { dom?: DOMProps; + ref: Ref; streamKey: string; - initialSnapshot: TerminalState | null; testId?: string; xtermTheme?: ITheme; swipeGesturesEnabled?: boolean; onSwipeLeft?: () => void; onSwipeRight?: () => void; + initialSnapshot?: TerminalState | null; onInput?: (data: string) => Promise | void; onResize?: (input: { rows: number; cols: number }) => Promise | void; onTerminalKey?: (input: { @@ -86,12 +94,6 @@ interface TerminalEmulatorProps { resizeRequestToken?: number; } -export type TerminalEmulatorHandle = { - writeOutput: (text: string) => void; - renderSnapshot: (state: TerminalState | null) => void; - clear: () => void; -}; - declare global { interface Window {} } @@ -124,9 +126,9 @@ function ensureTerminalScrollbarStyle(): void { document.head.appendChild(styleElement); } -const TerminalEmulator = forwardRef(function TerminalEmulator({ +export default function TerminalEmulator({ + ref, streamKey, - initialSnapshot, testId = "terminal-surface", xtermTheme = { background: "#0b0b0b", @@ -136,6 +138,7 @@ const TerminalEmulator = forwardRef(null); const hostRef = useRef(null); const runtimeRef = useRef(null); @@ -165,6 +168,23 @@ const TerminalEmulator = forwardRef, + () => + ({ + writeOutput: (text: string) => { + runtimeRef.current?.write({ text }); + }, + renderSnapshot: (state: TerminalState | null) => { + runtimeRef.current?.renderSnapshot({ state }); + }, + clear: () => { + runtimeRef.current?.clear(); + }, + }) as unknown as DOMImperativeFactory, + [], + ); + useEffect(() => { mountedThemeRef.current = xtermTheme; runtimeRef.current?.setTheme({ theme: xtermTheme }); @@ -338,22 +358,6 @@ const TerminalEmulator = forwardRef ({ - writeOutput: (text: string) => { - runtimeRef.current?.write({ text }); - }, - renderSnapshot: (state: TerminalState | null) => { - runtimeRef.current?.renderSnapshot({ state }); - }, - clear: () => { - runtimeRef.current?.clear(); - }, - }), - [], - ); - useEffect(() => { if (focusRequestToken <= 0) { return; @@ -670,6 +674,4 @@ const TerminalEmulator = forwardRef ); -}); - -export default TerminalEmulator; +} diff --git a/packages/app/src/components/terminal-pane.tsx b/packages/app/src/components/terminal-pane.tsx index 87baf954c..0d8b84a28 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -107,13 +107,13 @@ export function TerminalPane({ const scopeKey = useMemo(() => terminalScopeKey({ serverId, cwd }), [serverId, cwd]); const lastReportedSizeRef = useRef<{ rows: number; cols: number } | null>(null); const streamControllerRef = useRef(null); - const emulatorRef = useRef(null); const workspaceTerminalSession = useMemo(() => getWorkspaceTerminalSession({ scopeKey }), [scopeKey]); const [isAttaching, setIsAttaching] = useState(false); const [streamError, setStreamError] = useState(null); const [modifiers, setModifiers] = useState(EMPTY_MODIFIERS); const [focusRequestToken, setFocusRequestToken] = useState(0); const [resizeRequestToken, setResizeRequestToken] = useState(0); + const emulatorRef = useRef(null); const terminalIdRef = useRef(terminalId); const pendingTerminalInputRef = useRef([]); const keyboardRefitTimeoutsRef = useRef>>([]); @@ -560,10 +560,10 @@ export function TerminalPane({ contentInsetAdjustmentBehavior: "never", }} streamKey={`${scopeKey}:${terminalId}`} - initialSnapshot={initialSnapshot} testId="terminal-surface" xtermTheme={xtermTheme} swipeGesturesEnabled={swipeGesturesEnabled} + initialSnapshot={initialSnapshot} onSwipeRight={() => { if (!swipeGesturesEnabled) { return; diff --git a/packages/app/src/terminal/runtime/terminal-stream-controller.ts b/packages/app/src/terminal/runtime/terminal-stream-controller.ts index b5978b64c..f7deb1523 100644 --- a/packages/app/src/terminal/runtime/terminal-stream-controller.ts +++ b/packages/app/src/terminal/runtime/terminal-stream-controller.ts @@ -40,6 +40,10 @@ export type TerminalStreamControllerOptions = { const TERMINAL_EXITED_ERROR = "Terminal exited"; +function logTerminalStreamController(event: string, details: Record): void { + console.log("[terminal-stream-controller]", event, details); +} + export class TerminalStreamController { private readonly decoder = new TextDecoder(); private readonly unsubscribeStreamEvents: () => void; @@ -65,10 +69,19 @@ export class TerminalStreamController { setTerminal(input: { terminalId: string | null }): void { if (this.disposed || input.terminalId === this.terminalId) { + logTerminalStreamController("set_terminal_noop", { + disposed: this.disposed, + currentTerminalId: this.terminalId, + requestedTerminalId: input.terminalId, + }); return; } const nextTerminalId = input.terminalId; const previousTerminalId = this.terminalId; + logTerminalStreamController("set_terminal", { + previousTerminalId, + nextTerminalId, + }); this.terminalId = nextTerminalId; this.decoder.decode(); if (previousTerminalId) { @@ -82,6 +95,11 @@ export class TerminalStreamController { void this.options.client .subscribeTerminal(nextTerminalId) .then((payload) => { + logTerminalStreamController("subscribe_resolved", { + nextTerminalId, + payloadTerminalId: payload.terminalId, + error: payload.error ?? null, + }); if (this.disposed || this.terminalId !== nextTerminalId) { return; } @@ -95,6 +113,10 @@ export class TerminalStreamController { return; } const preferredSize = this.options.getPreferredSize(); + logTerminalStreamController("preferred_size_after_subscribe", { + nextTerminalId, + preferredSize, + }); if (preferredSize) { this.options.client.sendTerminalInput(nextTerminalId, { type: "resize", @@ -109,6 +131,10 @@ export class TerminalStreamController { }); }) .catch((error: unknown) => { + logTerminalStreamController("subscribe_rejected", { + nextTerminalId, + error: error instanceof Error ? error.message : "Unable to subscribe to terminal", + }); if (this.disposed || this.terminalId !== nextTerminalId) { return; } @@ -122,6 +148,10 @@ export class TerminalStreamController { } handleTerminalExit(input: { terminalId: string }): void { + logTerminalStreamController("terminal_exit", { + currentTerminalId: this.terminalId, + exitedTerminalId: input.terminalId, + }); if (this.disposed || input.terminalId !== this.terminalId) { return; } @@ -138,6 +168,9 @@ export class TerminalStreamController { if (this.disposed) { return; } + logTerminalStreamController("dispose", { + terminalId: this.terminalId, + }); this.disposed = true; this.decoder.decode(); const terminalId = this.terminalId; diff --git a/packages/server/src/server/daemon-e2e/terminal.e2e.test.ts b/packages/server/src/server/daemon-e2e/terminal.e2e.test.ts index 73f3b5410..59fbe2989 100644 --- a/packages/server/src/server/daemon-e2e/terminal.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/terminal.e2e.test.ts @@ -606,16 +606,10 @@ describe("daemon E2E terminal", () => { rmSync(cwd, { recursive: true, force: true }); }, 30000); - test("resize sends a new snapshot with the updated dimensions", async () => { + test("resize updates server dimensions without sending a live snapshot", async () => { const cwd = tmpCwd(); const created = await ctx.client.createTerminal(cwd); const terminalId = created.terminal!.id; - - const resizedSnapshot = waitForTerminalSnapshot( - ctx.client, - terminalId, - (state) => state.rows === 10 && state.cols === 40, - ); await ctx.client.subscribeTerminal(terminalId); ctx.client.sendTerminalInput(terminalId, { type: "resize", @@ -623,9 +617,11 @@ describe("daemon E2E terminal", () => { cols: 40, }); - const snapshot = await resizedSnapshot; - expect(snapshot.rows).toBe(10); - expect(snapshot.cols).toBe(40); + await new Promise((resolve) => setTimeout(resolve, 300)); + + const state = ctx.daemon.daemon.terminalManager?.getTerminal(terminalId)?.getState(); + expect(state?.rows).toBe(10); + expect(state?.cols).toBe(40); rmSync(cwd, { recursive: true, force: true }); }, 30000); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index a68bcf37d..85f2a1ddd 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -7729,6 +7729,13 @@ export class Session { } this.ensureTerminalExitSubscription(session); + if (msg.message.type === "resize") { + const currentSize = session.getSize(); + if (currentSize.rows === msg.message.rows && currentSize.cols === msg.message.cols) { + return; + } + } + session.send(msg.message); } diff --git a/packages/server/src/shared/terminal-key-input.test.ts b/packages/server/src/shared/terminal-key-input.test.ts index 0248a2bae..239fb86c1 100644 --- a/packages/server/src/shared/terminal-key-input.test.ts +++ b/packages/server/src/shared/terminal-key-input.test.ts @@ -23,10 +23,11 @@ describe("encodeTerminalKeyInput", () => { expect(encodeTerminalKeyInput({ key: "Enter", shift: true })).toBe("\x1b[13;2u"); }); - it("encodes enter with other modifiers as plain carriage return", () => { - expect(encodeTerminalKeyInput({ key: "Enter", ctrl: true })).toBe("\r"); - expect(encodeTerminalKeyInput({ key: "Enter", alt: true })).toBe("\x1b\r"); - expect(encodeTerminalKeyInput({ key: "Enter", shift: true, ctrl: true })).toBe("\r"); + it("encodes enter with modifiers using CSI u", () => { + expect(encodeTerminalKeyInput({ key: "Enter", ctrl: true })).toBe("\x1b[13;5u"); + expect(encodeTerminalKeyInput({ key: "Enter", alt: true })).toBe("\x1b[13;3u"); + expect(encodeTerminalKeyInput({ key: "Enter", meta: true })).toBe("\x1b[13;9u"); + expect(encodeTerminalKeyInput({ key: "Enter", shift: true, ctrl: true })).toBe("\x1b[13;6u"); }); it("returns empty string for unsupported keys", () => { diff --git a/packages/server/src/shared/terminal-key-input.ts b/packages/server/src/shared/terminal-key-input.ts index 35bdb8890..77b941ecc 100644 --- a/packages/server/src/shared/terminal-key-input.ts +++ b/packages/server/src/shared/terminal-key-input.ts @@ -9,13 +9,14 @@ export interface TerminalKeyInput { function modifierParam(input: TerminalKeyInput): number { let value = 1; if (input.shift) value += 1; - if (input.alt || input.meta) value += 2; + if (input.alt) value += 2; if (input.ctrl) value += 4; + if (input.meta) value += 8; return value; } function applyAltLikePrefix(sequence: string, input: TerminalKeyInput): string { - return input.alt || input.meta ? `\x1b${sequence}` : sequence; + return input.alt ? `\x1b${sequence}` : sequence; } function encodePrintableKey(input: TerminalKeyInput): string { @@ -85,11 +86,13 @@ export function encodeTerminalKeyInput(input: TerminalKeyInput): string { } switch (key) { - case "Enter": - if (input.shift && !input.ctrl && !input.alt && !input.meta) { - return "\x1b[13;2u"; + case "Enter": { + const mod = modifierParam(input); + if (mod > 1) { + return `\x1b[13;${mod}u`; } - return applyAltLikePrefix("\r", input); + return "\r"; + } case "Tab": if (input.shift && !input.ctrl && !input.alt && !input.meta) { return "\x1b[Z"; diff --git a/packages/server/src/terminal/terminal.test.ts b/packages/server/src/terminal/terminal.test.ts index 7a818943b..828ac66af 100644 --- a/packages/server/src/terminal/terminal.test.ts +++ b/packages/server/src/terminal/terminal.test.ts @@ -402,6 +402,25 @@ describe("Terminal", () => { expect(state.grid.length).toBe(10); expect(state.grid[0].length).toBe(40); }); + + it("exposes the current size without extracting full state", async () => { + const session = trackSession( + await createTerminal({ + cwd: "/tmp", + shell: "/bin/sh", + env: { PS1: "$ " }, + rows: 24, + cols: 80, + }), + ); + + expect(session.getSize()).toEqual({ rows: 24, cols: 80 }); + + session.send({ type: "resize", rows: 10, cols: 40 }); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(session.getSize()).toEqual({ rows: 10, cols: 40 }); + }); }); describe("subscribe", () => { @@ -427,7 +446,7 @@ describe("Terminal", () => { unsubscribe(); }); - it("receives output and snapshot messages on updates", async () => { + it("receives output messages on updates without replay snapshots", async () => { const session = trackSession( await createTerminal({ cwd: "/tmp", @@ -443,6 +462,7 @@ describe("Terminal", () => { messages.push(msg); }); + await new Promise((resolve) => setTimeout(resolve, 100)); messages.length = 0; session.send({ type: "input", data: "echo test\r" }); @@ -450,7 +470,35 @@ describe("Terminal", () => { await waitForLines(session, ["$ echo test", "test", "$"]); expect(messages.some((message) => message.type === "output")).toBe(true); - expect(messages.some((message) => message.type === "snapshot")).toBe(true); + expect(messages.some((message) => message.type === "snapshot")).toBe(false); + + unsubscribe(); + }); + + it("does not emit snapshot messages for resize-only updates", async () => { + const session = trackSession( + await createTerminal({ + cwd: "/tmp", + shell: "/bin/sh", + env: { PS1: "$ " }, + rows: 24, + cols: 80, + }), + ); + + const messages: Array<{ type: string }> = []; + const unsubscribe = session.subscribe((msg) => { + messages.push(msg); + }); + + await new Promise((resolve) => setTimeout(resolve, 100)); + messages.length = 0; + + session.send({ type: "resize", rows: 30, cols: 100 }); + await new Promise((resolve) => setTimeout(resolve, 200)); + + expect(messages.some((message) => message.type === "snapshot")).toBe(false); + expect(session.getSize()).toEqual({ rows: 30, cols: 100 }); unsubscribe(); }); diff --git a/packages/server/src/terminal/terminal.ts b/packages/server/src/terminal/terminal.ts index 59acda802..28e2601c7 100644 --- a/packages/server/src/terminal/terminal.ts +++ b/packages/server/src/terminal/terminal.ts @@ -9,7 +9,6 @@ import type { TerminalCell, TerminalState } from "../shared/messages.js"; const { Terminal } = xterm; const require = createRequire(import.meta.url); let nodePtySpawnHelperChecked = false; -const STATE_BROADCAST_INTERVAL_MS = 33; export type ClientMessage = | { type: "input"; data: string } @@ -27,6 +26,7 @@ export interface TerminalSession { send(msg: ClientMessage): void; subscribe(listener: (msg: ServerMessage) => void): () => void; onExit(listener: () => void): () => void; + getSize(): { rows: number; cols: number }; getState(): TerminalState; kill(): void; } @@ -246,8 +246,6 @@ export async function createTerminal(options: CreateTerminalOptions): Promise | null = null; // Create xterm.js headless terminal const terminal = new Terminal({ @@ -301,38 +299,11 @@ export async function createTerminal(options: CreateTerminalOptions): Promise { - stateBroadcastTimer = null; - emitStateToListeners(); - }, STATE_BROADCAST_INTERVAL_MS); - } - // Pipe PTY output to terminal emulator ptyProcess.onData((data) => { if (killed) return; @@ -343,7 +314,6 @@ export async function createTerminal(options: CreateTerminalOptions): Promise