From cce1866c0b8eb028130b5618923de7c51ce83423 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 11 Sep 2026 16:39:50 +0800 Subject: [PATCH 1/9] fix(core): run web terminal PTYs on the bundled ConPTY backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web terminal spawned its PTYs with node-pty's default inbox ConPTY backend, where a natural shell exit orphans the conhost.exe --headless the backend spawned: the native exit watcher erases the pty baton before delivering onExit, pty_baton has no destructor, and the erased HPCON is unreachable from JS (microsoft/node-pty#965). Every exited web terminal leaked ~8 MB for the life of the CLI, and a pin bump does not fix it. Mirror the #11497 shell path: spawn with useConptyDll on Windows so the bundled backend releases its host reference right after spawn and the host exits with its last client. The bundled backend adds one synchronous throw point — a missing or unloadable conpty.dll — and a web terminal has no child_process fallback, so a failed bundled spawn retries once on the inbox backend: the pre-fix leaking behavior beats a terminal that cannot start at all. A failed spawn produced no child, so the retry cannot double-spawn. The 4001 non-retryable close path is unchanged. Also switch the agent-view PTY host to the bundled backend (same leak, lower rate), refresh the conpty-host release docs, and add a VERIFIED_NODE_PTY pin test so a node-pty bump forces re-verification of the native teardown semantics the release path depends on. Refs #11352 --- packages/cli/src/agent-view/pty-host.test.ts | 22 +++ packages/cli/src/agent-view/pty-host.ts | 5 + .../core/src/services/conpty-host.test.ts | 129 +++++++++++++ packages/core/src/services/conpty-host.ts | 18 +- .../services/web-terminal-registry.test.ts | 172 +++++++++++++++--- .../src/services/web-terminal-registry.ts | 34 +++- 6 files changed, 343 insertions(+), 37 deletions(-) create mode 100644 packages/core/src/services/conpty-host.test.ts diff --git a/packages/cli/src/agent-view/pty-host.test.ts b/packages/cli/src/agent-view/pty-host.test.ts index d76e85c2a40..23f89958260 100644 --- a/packages/cli/src/agent-view/pty-host.test.ts +++ b/packages/cli/src/agent-view/pty-host.test.ts @@ -349,6 +349,28 @@ describe('launchAgentViewPtyHost', () => { expect(pty.process.killCalls).toEqual([undefined, undefined]); }); + it('spawns the worker with the bundled ConPTY backend on Windows', async () => { + // The inbox backend orphans a `conhost.exe --headless` per natural worker + // exit (microsoft/node-pty#965); Windows workers must spawn with the + // bundled backend, which releases its host reference right after spawn. + const pty = createFakePty(); + const original = process.platform; + Object.defineProperty(process, 'platform', { + value: 'win32', + configurable: true, + }); + try { + await launchAgentViewPtyHost(createLaunch(), { pty }); + } finally { + Object.defineProperty(process, 'platform', { + value: original, + configurable: true, + }); + } + + expect(pty.spawnCalls[0]?.options.useConptyDll).toBe(true); + }); + it('resets the input decoder between attach sessions', async () => { const pty = createFakePty(); const handle = await launchAgentViewPtyHost(createLaunch(), { pty }); diff --git a/packages/cli/src/agent-view/pty-host.ts b/packages/cli/src/agent-view/pty-host.ts index 1a270df01fd..7755760536a 100644 --- a/packages/cli/src/agent-view/pty-host.ts +++ b/packages/cli/src/agent-view/pty-host.ts @@ -33,6 +33,7 @@ export interface AgentViewPtySpawnOptions { rows: number; env: Record; handleFlowControl: boolean; + useConptyDll: boolean; } export interface AgentViewPtyDisposable { @@ -349,6 +350,10 @@ export async function launchAgentViewPtyHost( rows: launch.terminal.rows, env: workerEnv, handleFlowControl: false, + // Windows: the inbox ConPTY backend orphans a `conhost.exe --headless` + // per natural worker exit (microsoft/node-pty#965); the bundled backend + // releases its host reference right after spawn. Mirrors #11497 / #11352. + useConptyDll: process.platform === 'win32', }); let inputDecoder = new StringDecoder('utf8'); diff --git a/packages/core/src/services/conpty-host.test.ts b/packages/core/src/services/conpty-host.test.ts new file mode 100644 index 00000000000..f8e58ec6362 --- /dev/null +++ b/packages/core/src/services/conpty-host.test.ts @@ -0,0 +1,129 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { readFileSync } from 'node:fs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { osPlatform } = vi.hoisted(() => ({ osPlatform: vi.fn() })); + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + const patched = { ...actual, platform: osPlatform }; + return { ...patched, default: patched }; +}); + +import { + disposeConoutWorker, + noteConPtyHostReleased, + releaseConPtyHost, +} from './conpty-host.js'; + +// The exact @lydell/node-pty pin whose JS field shape and native teardown +// semantics the release path in conpty-host.ts was verified against. A +// version bump turns this test red on purpose: re-check the WindowsPtyAgent +// fields and src/win/conpty.cc (the baton erase became unconditional in +// 1.2.0-beta.14 — see conpty-host.ts) before updating the constant. +const VERIFIED_NODE_PTY = '1.2.0-beta.10'; + +describe('conpty-host', () => { + beforeEach(() => { + vi.clearAllMocks(); + osPlatform.mockReturnValue('win32'); + }); + + it('pins the verified @lydell/node-pty version', () => { + const packageJson = JSON.parse( + readFileSync(new URL('../../package.json', import.meta.url), 'utf8'), + ) as { optionalDependencies: Record }; + + expect(packageJson.optionalDependencies['@lydell/node-pty']).toBe( + VERIFIED_NODE_PTY, + ); + }); + + it('drives the release through the WindowsPtyAgent internals shape', () => { + // The field shape releaseConPtyHost consumes: a node-pty bump that + // renames these fields silently degrades the release to a warn + no-op. + const nativeKill = vi.fn(); + const conoutDispose = vi.fn(); + const pty = { + _agent: { + _pty: 42, + _useConptyDll: false, + _ptyNative: { kill: nativeKill }, + _conoutSocketWorker: { dispose: conoutDispose }, + }, + }; + + releaseConPtyHost(pty); + + expect(nativeKill).toHaveBeenCalledWith(42, false); + expect(conoutDispose).toHaveBeenCalledOnce(); + }); + + it('disposes only the worker when a bundled PTY was already killed', () => { + // A kill() that really ran records the note; on the bundled backend the + // release must then skip the native close but still dispose the worker + // node-pty defers until more output. + const nativeKill = vi.fn(); + const conoutDispose = vi.fn(); + const pty = { + _agent: { + _pty: 42, + _useConptyDll: true, + _ptyNative: { kill: nativeKill }, + _conoutSocketWorker: { dispose: conoutDispose }, + }, + }; + noteConPtyHostReleased(pty); + + releaseConPtyHost(pty); + + expect(nativeKill).not.toHaveBeenCalled(); + expect(conoutDispose).toHaveBeenCalledOnce(); + }); + + it('leaves a noted inbox PTY completely alone', () => { + // The `_useConptyDll` discriminator: with the inbox backend a noted PTY + // needs neither a second native close nor the worker dispose. + const nativeKill = vi.fn(); + const conoutDispose = vi.fn(); + const pty = { + _agent: { + _pty: 42, + _useConptyDll: false, + _ptyNative: { kill: nativeKill }, + _conoutSocketWorker: { dispose: conoutDispose }, + }, + }; + noteConPtyHostReleased(pty); + + releaseConPtyHost(pty); + + expect(nativeKill).not.toHaveBeenCalled(); + expect(conoutDispose).not.toHaveBeenCalled(); + }); + + it('never touches the PTY off Windows', () => { + osPlatform.mockReturnValue('linux'); + const nativeKill = vi.fn(); + const conoutDispose = vi.fn(); + const pty = { + _agent: { + _pty: 42, + _useConptyDll: true, + _ptyNative: { kill: nativeKill }, + _conoutSocketWorker: { dispose: conoutDispose }, + }, + }; + + releaseConPtyHost(pty); + disposeConoutWorker(pty); + + expect(nativeKill).not.toHaveBeenCalled(); + expect(conoutDispose).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/services/conpty-host.ts b/packages/core/src/services/conpty-host.ts index c06a19e81f9..3d6bee81669 100644 --- a/packages/core/src/services/conpty-host.ts +++ b/packages/core/src/services/conpty-host.ts @@ -108,10 +108,11 @@ export const disposeConoutWorker = (ptyProcess: unknown): void => { * Releases what node-pty leaves behind when a Windows PTY finishes. * * **What this function itself reliably frees is the conout worker thread.** - * Shell PTYs now use node-pty's bundled ConPTY backend, which releases its host - * reference immediately after spawn so the host exits with its last client. - * Web-terminal PTYs still use the Windows inbox backend, whose natural-exit - * host leak is not fixed here. + * Shell, web-terminal, and agent-view PTYs now use node-pty's bundled ConPTY + * backend, which releases its host reference immediately after spawn so the + * host exits with its last client. The web terminal keeps the Windows inbox + * backend only as a spawn-failure retry (#11352); that fallback's + * natural-exit host leak is not fixed here. * * With the inbox backend, a finished PTY strands both the ConPTY host and the * `worker_threads` Worker node-pty runs to read the conout pipe. With bundled @@ -146,10 +147,11 @@ export const disposeConoutWorker = (ptyProcess: unknown): void => { * `kill()` stays the single closer. The inbox conhost half of #11303 is * therefore not fixed by this function on the natural-exit path. * - * The web-terminal PTY (`web-terminal-registry.ts`) and agent-view PTY host do - * not use the bundled backend, so they can still strand an inbox host per - * exited terminal. Do not add a test that treats a stubbed `_ptyNative.kill` - * call as evidence that the inbox host was released. + * The web-terminal PTY (`web-terminal-registry.ts`) and the agent-view PTY + * host spawn with the bundled backend too (#11352); only the web terminal's + * inbox spawn-failure retry can still strand an inbox host per exited + * terminal. Do not add a test that treats a stubbed `_ptyNative.kill` call as + * evidence that the inbox host was released. * * **Why not just call `ptyProcess.kill()`.** On the inbox backend it forks a * helper that can fall back after a natural exit to terminating a recycled diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index b503fadd963..d5c1fe29ff2 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -15,8 +15,9 @@ const { spawn, getPty, spawnSync, osPlatform } = vi.hoisted(() => ({ vi.mock('node:child_process', () => ({ spawnSync })); vi.mock('../utils/getPty.js', () => ({ getPty })); -// Only conpty-host reads os.platform(); killPtyTree branches on -// process.platform, so this steers the ConPTY release without touching it. +// conpty-host reads os.platform() for its win32 release gate, and the +// registry for the bundled-vs-inbox ConPTY backend choice; killPtyTree +// branches on process.platform, so this steers both without touching it. // Windows CI is skipped on PRs, so the win32 path has to be reachable here. // Everything else passes through -- Storage (via debugLogger) needs the real // os.homedir()/os.tmpdir(). @@ -43,6 +44,31 @@ describe('WebTerminalRegistry', () => { let disposeData: ReturnType; let disposeExit: ReturnType; + const createSpawnedPty = () => ({ + pid: 1, + write, + resize, + kill, + // node-pty's WindowsPtyAgent internals, which releaseConPtyHost drives + // directly instead of going through kill(). See #11303. + _agent: { + _pty: 42, + _useConptyDll: false, + _ptyNative: { kill: nativeKill }, + _conoutSocketWorker: { dispose: conoutDispose }, + }, + onData: vi.fn((listener: (data: string) => void) => { + onData = listener; + return { dispose: disposeData }; + }), + onExit: vi.fn( + (listener: (e: { exitCode: number; signal?: number }) => void) => { + onExit = listener; + return { dispose: disposeExit }; + }, + ), + }); + beforeEach(() => { vi.clearAllMocks(); write = vi.fn(); @@ -54,28 +80,7 @@ describe('WebTerminalRegistry', () => { disposeExit = vi.fn(); spawnSync.mockReturnValue({ stdout: '' }); osPlatform.mockReturnValue(process.platform); - spawn.mockReturnValue({ - pid: 1, - write, - resize, - kill, - // node-pty's WindowsPtyAgent internals, which releaseConPtyHost drives - // directly instead of going through kill(). See #11303. - _agent: { - _pty: 42, - _useConptyDll: false, - _ptyNative: { kill: nativeKill }, - _conoutSocketWorker: { dispose: conoutDispose }, - }, - onData: vi.fn((listener) => { - onData = listener; - return { dispose: disposeData }; - }), - onExit: vi.fn((listener) => { - onExit = listener; - return { dispose: disposeExit }; - }), - }); + spawn.mockImplementation(() => createSpawnedPty()); getPty.mockResolvedValue({ module: { spawn }, name: 'node-pty' }); }); @@ -200,6 +205,9 @@ describe('WebTerminalRegistry', () => { }); it('returns stable errors when PTY loading or spawning fails', async () => { + // Pin off Windows: on win32 a failed bundled spawn retries once on the + // inbox backend — covered by the bundled-backend cases below. + osPlatform.mockReturnValue('linux'); const registry = new WebTerminalRegistry(); getPty.mockResolvedValueOnce(null); await expect( @@ -214,6 +222,122 @@ describe('WebTerminalRegistry', () => { ).resolves.toEqual({ error: 'Failed to spawn shell' }); }); + it('spawns Windows terminals with the bundled ConPTY backend', async () => { + // Mirrors the shellExecutionService bundled-backend case: the inbox + // backend orphans a `conhost.exe --headless` per natural shell exit + // (microsoft/node-pty#965), so Windows terminals must spawn with the + // bundled backend. Hardcoding `useConptyDll: false` turns this red. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + + await registry.create({ + terminalId: 'terminal:bundled', + workspaceCwd: '/workspace', + }); + + expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: true }); + }); + + it('spawns non-Windows terminals without the bundled backend', async () => { + osPlatform.mockReturnValue('linux'); + const registry = new WebTerminalRegistry(); + + await registry.create({ + terminalId: 'terminal:posix-backend', + workspaceCwd: '/workspace', + }); + + // Inert on the POSIX prebuilds, but pinned so the option stays a + // deliberate platform branch rather than an unconditional `true`. + expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: false }); + }); + + it('retries a failed bundled spawn once on the inbox backend', async () => { + // The bundled backend throws synchronously when its conpty.dll is missing + // or unloadable; a web terminal has no child_process fallback, so the + // registry drops to the pre-fix inbox behavior rather than fail the + // terminal outright. Deleting the retry branch in create() turns this + // red. + osPlatform.mockReturnValue('win32'); + spawn.mockImplementationOnce(() => { + throw new Error('Failed to load conpty.dll, error code: 126'); + }); + const registry = new WebTerminalRegistry(); + + const created = await registry.create({ + terminalId: 'terminal:bundled-retry', + workspaceCwd: '/workspace', + }); + + expect(created).toEqual({ terminalId: 'terminal:bundled-retry' }); + expect(spawn).toHaveBeenCalledTimes(2); + expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: true }); + expect(spawn.mock.calls[1]?.[2]).toMatchObject({ useConptyDll: false }); + // The retried PTY is fully wired: output reaches the session buffer. + onData('ready'); + expect(registry.readSnapshot('terminal:bundled-retry')?.output).toBe( + 'ready', + ); + }); + + it('reports a spawn failure and frees the id when both backends fail', async () => { + osPlatform.mockReturnValue('win32'); + spawn.mockImplementation(() => { + throw new Error('spawn failed'); + }); + const registry = new WebTerminalRegistry(); + + await expect( + registry.create({ + terminalId: 'terminal:bundled-double-fail', + workspaceCwd: '/workspace', + }), + ).resolves.toEqual({ error: 'Failed to spawn shell' }); + expect(spawn).toHaveBeenCalledTimes(2); + + // finishCreating ran on the failure path: the same id is creatable again + // instead of being stuck on "is being created". + spawn.mockImplementation(() => createSpawnedPty()); + await expect( + registry.create({ + terminalId: 'terminal:bundled-double-fail', + workspaceCwd: '/workspace', + }), + ).resolves.toEqual({ terminalId: 'terminal:bundled-double-fail' }); + }); + + it('frees a bundled-shape session once across kill and exit-time release', async () => { + osPlatform.mockReturnValue('win32'); + // Bundled ConPTY shape: the host reference was released at spawn, so the + // native close is only reachable through kill(); the noted release must + // skip a second close yet still dispose the conout worker, which node-pty + // otherwise defers until more output that never comes. Mirrors the shell + // path's bundled cancel case in shellExecutionService.test.ts. + spawn.mockImplementationOnce(() => { + const pty = createSpawnedPty(); + pty._agent._useConptyDll = true; + return pty; + }); + kill.mockImplementation(() => { + nativeKill(42, true); + }); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:bundled-exit-release', + workspaceCwd: '/workspace', + }); + + expect(registry.release('terminal:bundled-exit-release')).toBe(true); + onExit({ exitCode: 0 }); + await new Promise((resolve) => setImmediate(resolve)); + + expect(kill).toHaveBeenCalledOnce(); + // kill()'s own close is the only native close: the noted bundled release + // adds none, and the exit-time release is a no-op after it. + expect(nativeKill).toHaveBeenCalledOnce(); + expect(conoutDispose).toHaveBeenCalledOnce(); + }); + it('caps replay output and records exit state', async () => { const registry = new WebTerminalRegistry(); const created = await registry.create({ diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index e0f48f10021..4e4f9101705 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -5,6 +5,7 @@ */ import { spawnSync } from 'node:child_process'; +import os from 'node:os'; import { getPty } from '../utils/getPty.js'; import { disposeConoutWorker, @@ -234,6 +235,7 @@ export class WebTerminalRegistry { delete env['NO_COLOR']; delete env['FORCE_COLOR']; delete env['npm_config_prefix']; + const useBundledConpty = os.platform() === 'win32'; let spawned: SpawnedWebTerminalPty; let proc: WebTerminalPty; const sessionRef: { current?: PtySession } = {}; @@ -280,8 +282,11 @@ export class WebTerminalRegistry { // Nothing needs the PTY once the shell is gone: write() and resize() // already short-circuit on `exited`, and readSnapshot() replays the // JS-side `buffer`, not the console. Waiting for release() instead left - // every exited web terminal holding node-pty's conout worker — and, - // upstream, its conhost.exe — for up to IDLE_RECLAIM_MS, because the + // every exited web terminal holding node-pty's conout worker — and, on + // the inbox ConPTY backend, its conhost.exe (microsoft/node-pty#965); + // the bundled backend this registry now spawns with releases its host + // reference at spawn, so the conhost half survives only on the inbox + // retry fallback — for up to IDLE_RECLAIM_MS, because the // route keeps the session alive for scrollback and the client treats the // 4000 close as non-retryable, so only a tab close releases it. Exited // sessions also do not count against the admission cap, so accumulation @@ -297,8 +302,8 @@ export class WebTerminalRegistry { }; let dataDisposable: { dispose(): void } | undefined; let exitDisposable: { dispose(): void } | undefined; - try { - spawned = ptyImpl.module.spawn(file, args, { + const spawnPty = (useBundled: boolean) => + ptyImpl.module.spawn(file, args, { name: 'xterm-256color', cols: 80, rows: 24, @@ -310,7 +315,26 @@ export class WebTerminalRegistry { CLICOLOR: '1', PROMPT_EOL_MARK: '', }, - }) as SpawnedWebTerminalPty; + // Windows: with the inbox ConPTY backend a natural shell exit orphans + // the `conhost.exe --headless` it spawned (microsoft/node-pty#965); + // the bundled backend releases its host reference right after spawn. + // Mirrors the #11497 shell path. Off Windows the option is inert: + // `useConptyDll` appears nowhere in the POSIX prebuilds. + useConptyDll: useBundled, + }); + try { + try { + spawned = spawnPty(useBundledConpty) as SpawnedWebTerminalPty; + } catch (firstError) { + // The bundled backend adds a synchronous throw point: its conpty.dll + // missing or unloadable. A web terminal has no child_process + // fallback, so retry once on the inbox backend — the pre-fix leaking + // behavior beats a terminal that cannot start at all. A failed spawn + // produced no child process, so the retry cannot double-spawn (the + // `ptySpawned` argument from #11497). + if (!useBundledConpty) throw firstError; + spawned = spawnPty(false) as SpawnedWebTerminalPty; + } dataDisposable = spawned.onData(handleData); exitDisposable = spawned.onExit(handleExit); proc = { From 73dd42fcbfa8e226c72e3e1e0a6c6a94023c3a0f Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 04:54:56 +0800 Subject: [PATCH 2/9] fix(core): derive node-pty pins and widen the conpty-host pin guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The six `@lydell/node-pty*` pins live in three unsynchronized places — the root manifest, packages/core's manifest, and a hardcoded map in scripts/prepare-package.js — but the conpty-host tripwire asserted only one key of one manifest, so bumping the win32 prebuild keys or the root manifest left it green and skipped the human re-check it exists to force. Derive the six pins in writeDistPackageJson from packages/core's manifest (deleting the third hardcoded table), and widen the tripwire to assert all six core keys plus an equality against the root manifest's six — the declaration packages/cli's agent-view actually resolves in a dev tree. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtxennouyn --- .../core/src/services/conpty-host.test.ts | 33 +++++++++++++++++-- scripts/prepare-package.js | 20 +++++++---- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/core/src/services/conpty-host.test.ts b/packages/core/src/services/conpty-host.test.ts index f8e58ec6362..dd83a4d7435 100644 --- a/packages/core/src/services/conpty-host.test.ts +++ b/packages/core/src/services/conpty-host.test.ts @@ -35,13 +35,40 @@ describe('conpty-host', () => { }); it('pins the verified @lydell/node-pty version', () => { - const packageJson = JSON.parse( + const coreManifest = JSON.parse( readFileSync(new URL('../../package.json', import.meta.url), 'utf8'), ) as { optionalDependencies: Record }; + const rootManifest = JSON.parse( + readFileSync( + new URL('../../../../package.json', import.meta.url), + 'utf8', + ), + ) as { optionalDependencies: Record }; - expect(packageJson.optionalDependencies['@lydell/node-pty']).toBe( - VERIFIED_NODE_PTY, + const corePins = Object.fromEntries( + Object.entries(coreManifest.optionalDependencies).filter(([name]) => + name.startsWith('@lydell/node-pty'), + ), + ); + const rootPins = Object.fromEntries( + Object.entries(rootManifest.optionalDependencies).filter(([name]) => + name.startsWith('@lydell/node-pty'), + ), ); + + // Every platform pin core declares must be the verified version, so a bump + // of any one — not just the loader key — turns this red and forces the + // human re-check the comment above describes. The WindowsPtyAgent field + // shape and conpty.cc baton-erase semantics live in the win32 prebuilds, so + // those keys must trip the same guard as the loader. + expect(Object.keys(corePins)).toHaveLength(6); + expect( + Object.values(corePins).every((version) => version === VERIFIED_NODE_PTY), + ).toBe(true); + // packages/cli declares no node-pty and resolves the root-hoisted copy, so + // the root manifest's six must stay in lockstep with core's six — the + // declaration agent-view actually loads in a dev tree. + expect(corePins).toEqual(rootPins); }); it('drives the release through the WindowsPtyAgent internals shape', () => { diff --git a/scripts/prepare-package.js b/scripts/prepare-package.js index 0b0facd162d..a5c3450e50f 100644 --- a/scripts/prepare-package.js +++ b/scripts/prepare-package.js @@ -301,6 +301,19 @@ function writeDistPackageJson(rootDir, distDir) { `packages/core declares ${declared ?? 'none'})`, ); } + // The six `@lydell/node-pty*` platform pins ship in the tarball's + // optionalDependencies and decide which bundled ConPTY `conpty.dll` / + // `OpenConsole.exe` real Windows users get. Derive them from + // packages/core/package.json — the single source conpty-host.ts verifies its + // release path against (and its pin tripwire asserts) — instead of a third + // hardcoded table, so a bump of the native backend in one manifest cannot + // silently ship an unverified pin here. Mirrors the sharp / audio-capture + // derivations above. + const nodePtyPins = Object.fromEntries( + Object.entries(coreManifest.optionalDependencies ?? {}).filter(([name]) => + name.startsWith('@lydell/node-pty'), + ), + ); const distPackageJson = { name: rootPackageJson.name, @@ -349,12 +362,7 @@ function writeDistPackageJson(rootDir, distDir) { dependencies: {}, optionalDependencies: { '@qwen-code/audio-capture': rootPackageJson.version, - '@lydell/node-pty': '1.2.0-beta.10', - '@lydell/node-pty-darwin-arm64': '1.2.0-beta.10', - '@lydell/node-pty-darwin-x64': '1.2.0-beta.10', - '@lydell/node-pty-linux-x64': '1.2.0-beta.10', - '@lydell/node-pty-win32-arm64': '1.2.0-beta.10', - '@lydell/node-pty-win32-x64': '1.2.0-beta.10', + ...nodePtyPins, '@teddyzhu/clipboard': '0.0.5', '@teddyzhu/clipboard-darwin-arm64': '0.0.5', '@teddyzhu/clipboard-darwin-x64': '0.0.5', From 9e54d5ab3bbfe8486199ff827b030fae2b915018 Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 04:55:02 +0800 Subject: [PATCH 3/9] fix(core): answer and scrub bundled-backend terminal queries in the web terminal Switching the web-terminal registry to the bundled ConPTY backend let a probing shell's DA / DSR query bytes into the recorded scrollback; a reconnect that replays the buffer made the client's xterm.js re-answer each query and write the fresh reply back into the still-live shell's stdin. The bundled backend answers no queries itself, and the browser is not guaranteed to be attached when the startup probe fires. Mirror the other half of the shell path (#11497): on win32, feed a headless terminal the PTY stream so it answers the probe server-side, and strip the query bytes from both the scrollback and the live stream so a replay cannot re-emit them. Also bring the releasePtyResources docstring in line with the bundled backend (R1-8). Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtxennouyn --- .../services/web-terminal-registry.test.ts | 49 ++++++++- .../src/services/web-terminal-registry.ts | 104 ++++++++++++++++-- 2 files changed, 136 insertions(+), 17 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index f8b768da2fe..533d72988b2 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -5,16 +5,23 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import pkg from '@xterm/headless'; -const { spawn, getPty, spawnSync, osPlatform } = vi.hoisted(() => ({ - spawn: vi.fn(), - getPty: vi.fn(), - spawnSync: vi.fn(), - osPlatform: vi.fn(), -})); +const { Terminal } = pkg; + +const { spawn, getPty, spawnSync, osPlatform, loadXtermHeadless } = vi.hoisted( + () => ({ + spawn: vi.fn(), + getPty: vi.fn(), + spawnSync: vi.fn(), + osPlatform: vi.fn(), + loadXtermHeadless: vi.fn(), + }), +); vi.mock('node:child_process', () => ({ spawnSync })); vi.mock('../utils/getPty.js', () => ({ getPty })); +vi.mock('../utils/load-xterm-headless.js', () => ({ loadXtermHeadless })); // conpty-host reads os.platform() for its win32 release gate, and the // registry for the bundled-vs-inbox ConPTY backend choice; killPtyTree // branches on process.platform, so this steers both without touching it. @@ -95,6 +102,7 @@ describe('WebTerminalRegistry', () => { osPlatform.mockReturnValue(process.platform); spawn.mockImplementation(() => createSpawnedPty()); getPty.mockResolvedValue({ module: { spawn }, name: 'node-pty' }); + loadXtermHeadless.mockResolvedValue({ Terminal }); }); afterEach(() => { @@ -251,6 +259,35 @@ describe('WebTerminalRegistry', () => { expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: true }); }); + it('answers the bundled-backend DA probe and keeps it out of the scrollback', async () => { + // The bundled ConPTY backend answers no terminal queries itself, so + // PowerShell's startup DA probe would stall for its full timeout unless a + // terminal answers it server-side — and its query bytes, if recorded in the + // scrollback, would be re-answered by the client's xterm.js on reconnect + // and written back into the still-live shell as input. The forwarder must + // write the reply back to the PTY once, and the scrub must keep the query + // out of readSnapshot's replay. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:da-probe', + workspaceCwd: '/workspace', + }); + + onData('Microsoft Windows [Version 10.0.22631]\r\n'); + onData('\x1b[c'); + onData('C:\\work> '); + + await vi.waitFor(() => { + expect(write).toHaveBeenCalledWith('\x1b[?1;2c'); + }); + + const output = registry.readSnapshot('terminal:da-probe')?.output ?? ''; + expect(output).toContain('Microsoft Windows'); + expect(output).toContain('C:\\work> '); + expect(output).not.toContain('\x1b[c'); + }); + it('spawns non-Windows terminals without the bundled backend', async () => { osPlatform.mockReturnValue('linux'); const registry = new WebTerminalRegistry(); diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index 4e4f9101705..b6d3ee8e30a 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -6,7 +6,9 @@ import { spawnSync } from 'node:child_process'; import os from 'node:os'; +import type { Terminal } from '@xterm/headless'; import { getPty } from '../utils/getPty.js'; +import { loadXtermHeadless } from '../utils/load-xterm-headless.js'; import { disposeConoutWorker, noteConPtyHostReleased, @@ -60,6 +62,26 @@ export const MAX_CONCURRENT_WEB_TERMINALS = 8; /** Reclaim a PTY session after this long with no connected listener. */ const IDLE_RECLAIM_MS = 15 * 60 * 1000; +/** + * Terminal queries a probing shell emits — requests, not display content — are + * CSI sequences whose final byte is `c` (Device Attributes, e.g. PowerShell's + * startup DA probe) or `n` (Device Status Report, e.g. PSReadLine's + * cursor-position request). The bundled ConPTY backend answers none of them + * itself (see shellExecutionService.ts), so they reach this registry as + * ordinary PTY output. Recording them in the scrollback lets a reconnect that + * replays `session.buffer` make the client's xterm.js re-answer each query and + * write the fresh reply back into the still-live shell's stdin. Both final + * bytes are unambiguous requests, so stripping them can never drop rendered + * content. Only complete sequences within one chunk are stripped — node-pty + * delivers these 3-6 byte probes as a single chunk. + */ +function stripTerminalQueries(data: string): string { + // `no-control-regex` fires on the ESC byte, which is the whole point here: + // these are terminal CSI query sequences, not stray controls. + // eslint-disable-next-line no-control-regex + return data.replace(/\x1b\[[?0-9;>]*[cn]/g, ''); +} + interface PtySession { pty: WebTerminalPty; workspaceCwd: string; @@ -73,6 +95,8 @@ interface PtySession { reclaimTimer?: ReturnType; dataDisposable?: { dispose(): void }; exitDisposable?: { dispose(): void }; + queryTerminal?: Terminal; + queryReplyDisposable?: { dispose(): void }; /** * Set once the PTY-side resources above have been freed. The exit-time * release frees them while the session stays in the map for scrollback @@ -236,8 +260,34 @@ export class WebTerminalRegistry { delete env['FORCE_COLOR']; delete env['npm_config_prefix']; const useBundledConpty = os.platform() === 'win32'; + // The bundled ConPTY backend answers no terminal queries itself (see + // shellExecutionService.ts), so a probing shell — PowerShell's startup DA + // probe under COMSPEC=powershell — would otherwise stall for its full + // timeout and leave its query bytes in the scrollback. Load a headless + // terminal up front (the import is cached, so only the first terminal pays + // it) to answer the probe server-side; handleData feeds it the PTY stream + // and strips the query bytes from the scrollback so a reconnect replay + // cannot make the client re-answer them into the still-live shell. + let queryTerminal: Terminal | undefined; + if (useBundledConpty) { + try { + const { Terminal: HeadlessTerminal } = await loadXtermHeadless(); + queryTerminal = new HeadlessTerminal({ + allowProposedApi: true, + cols: 80, + rows: 24, + logLevel: 'off', + }); + } catch { + // No responder available: the query stays unanswered (a bounded ~2s + // stall), never injected — the strip below still keeps it out of the + // scrollback. + queryTerminal = undefined; + } + } let spawned: SpawnedWebTerminalPty; let proc: WebTerminalPty; + let queryReplyDisposable: { dispose(): void } | undefined; const sessionRef: { current?: PtySession } = {}; const earlyOutput: string[] = []; let earlyExit: { exitCode: number; signal?: number } | undefined; @@ -251,14 +301,30 @@ export class WebTerminalRegistry { 0, session.unacknowledgedInputBytes - Buffer.byteLength(data), ); - if (Buffer.byteLength(data) > MAX_BUFFER_BYTES) { - data = Buffer.from(data).subarray(-MAX_BUFFER_BYTES).toString('utf8'); - while (Buffer.byteLength(data) > MAX_BUFFER_BYTES) data = data.slice(1); + // Feed the headless responder so a bundled-backend query is answered + // server-side (the browser is not guaranteed to be attached when the + // startup probe fires). Strip the query from the scrollback AND from + // what reaches the live listeners: the browser's xterm.js would also + // answer it, and a reconnect replay of `buffer` would re-emit it. + if (queryTerminal) { + try { + queryTerminal.write(data); + } catch { + // Terminal disposed mid-stream (release raced a trailing chunk). + } + } + let buffered = useBundledConpty ? stripTerminalQueries(data) : data; + if (Buffer.byteLength(buffered) > MAX_BUFFER_BYTES) { + buffered = Buffer.from(buffered) + .subarray(-MAX_BUFFER_BYTES) + .toString('utf8'); + while (Buffer.byteLength(buffered) > MAX_BUFFER_BYTES) + buffered = buffered.slice(1); session.buffer = []; session.bufferBytes = 0; } - session.buffer.push(data); - session.bufferBytes += Buffer.byteLength(data); + session.buffer.push(buffered); + session.bufferBytes += Buffer.byteLength(buffered); while ( session.buffer.length > MAX_BUFFER_CHUNKS || session.bufferBytes > MAX_BUFFER_BYTES @@ -268,7 +334,7 @@ export class WebTerminalRegistry { session.bufferBytes -= Buffer.byteLength(dropped); } } - for (const listener of session.outputListeners) listener(data); + for (const listener of session.outputListeners) listener(buffered); }; const handleExit = (e: { exitCode: number; signal?: number }) => { const session = sessionRef.current; @@ -379,6 +445,15 @@ export class WebTerminalRegistry { releaseConPtyHost(spawned); }, }; + if (queryTerminal) { + queryReplyDisposable = queryTerminal.onData((reply) => { + try { + proc.write(reply); + } catch { + // A reply racing shell exit finds a dead PTY — drop it. + } + }); + } } catch { this.finishCreating(terminalId); return { error: 'Failed to spawn shell' }; @@ -395,6 +470,8 @@ export class WebTerminalRegistry { exitListeners: new Set(), dataDisposable, exitDisposable, + queryTerminal, + queryReplyDisposable, ptyResourcesReleased: false, }; sessionRef.current = session; @@ -544,11 +621,14 @@ export class WebTerminalRegistry { /** * Free a session's PTY-side resources exactly once: detach the data/exit - * listeners, then release the ConPTY host / conout worker that node-pty - * strands on a natural exit. Without the second half every terminal the user - * exits leaks a worker for the life of the CLI — the same defect the - * shell-tool path has. The conhost.exe half is not freed on that path (the - * native baton is already gone); see releaseConPtyHost. See #11303. + * listeners, dispose the bundled-backend query responder, then release the + * ConPTY host / conout worker that node-pty strands on a natural exit. + * Without the second half every terminal the user exits leaks a worker for + * the life of the CLI — the same defect the shell-tool path has. On the + * bundled ConPTY backend this registry spawns with, the host reference is + * already released at spawn, so only the conout worker is left to free here; + * the conhost.exe half survives solely on the inbox spawn-failure retry, + * where the native baton is already gone. See releaseConPtyHost and #11303. * * Deliberately leaves the session's map entry and its `buffer` alone, and * never signals the pid: on the exited path the shell is gone and its pid may @@ -566,6 +646,8 @@ export class WebTerminalRegistry { session.ptyResourcesReleased = true; session.dataDisposable?.dispose(); session.exitDisposable?.dispose(); + session.queryReplyDisposable?.dispose(); + session.queryTerminal?.dispose(); session.pty.releaseHost?.(); } From f5ede636058eeaf6b7a17153b9506873de9f9cff Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 11:00:35 +0800 Subject: [PATCH 4/9] fix(core): close query-strip gaps and a race in the web terminal Three review findings on the bundled-ConPTY web terminal path: - The scrollback scrub was a stateless per-chunk regex over only the `c`/`n` finals, so the query families xterm.js answers beyond those (tertiary DA, DECRQM, DECRQSS, XTVERSION, DECREQTPARM, OSC colour queries) still reached the scrollback and were re-answered into the live shell on reconnect. Replace it with a stateful stripper covering every family and carrying a probe split across two chunks. - resize() resized only session.pty, leaving the headless query responder on its 80x24 spawn grid, so geometry-dependent replies were computed against the wrong size. Forward the resize to session.queryTerminal. - loadXtermHeadless() was a second suspension point whose cancelledCreations/ disposed guards were not re-checked, so a release()/releaseWorkspace()/ dispose() landing during the import leaked a PTY the caller had cancelled. Re-check both after the import. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtxrimarz8 --- .../services/web-terminal-registry.test.ts | 150 ++++++++++++++++++ .../src/services/web-terminal-registry.ts | 93 ++++++++--- 2 files changed, 219 insertions(+), 24 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index 533d72988b2..30b645d055d 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -288,6 +288,156 @@ describe('WebTerminalRegistry', () => { expect(output).not.toContain('\x1b[c'); }); + it('strips every query family the browser client answers, not just DA/DSR', async () => { + // The client's xterm.js answers more than the DA / DSR probes: tertiary + // DA (`=`), DECRQM (`$ p`), DECRQSS (`$ q`), XTVERSION (`> q`), + // DECREQTPARM (`x`) and the OSC colour queries all fire a reply too, so a + // regex over just the `c`/`n` finals would leave them in the scrollback to + // be re-answered into the live shell. Every one of these must come out. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:queries', + workspaceCwd: '/workspace', + }); + + onData('prompt> '); + onData('\x1b[c'); // DA1 + onData('\x1b[>c'); // DA2 + onData('\x1b[=c'); // DA3 + onData('\x1b[6n'); // DSR cursor position + onData('\x1b[?6n'); // DEC DSR cursor position + onData('\x1b[?1$p'); // DECRQM + onData('\x1b[>0q'); // XTVERSION + onData('\x1b[1$q'); // DECRQSS + onData('\x1b[3x'); // DECREQTPARM + onData('\x1b]10;?\x07'); // OSC foreground-colour query + onData('\x1b]11;?\x07'); // OSC background-colour query + onData('\x1b]4;5;?\x07'); // OSC palette-colour query + onData('done'); + + const output = registry.readSnapshot('terminal:queries')?.output ?? ''; + expect(output).toContain('prompt> '); + expect(output).toContain('done'); + expect(output).not.toContain('\x1b'); + }); + + it('strips a query split across two chunks', async () => { + // node-pty can deliver a probe split across two chunks; a stateless + // per-chunk matcher would leave the halves in the scrollback and only the + // second half would never be re-answered as a whole. The stripper carries + // the incomplete CSI prefix to the next chunk and strips it whole. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:split', + workspaceCwd: '/workspace', + }); + + onData('before '); + onData('\x1b['); + onData('6n'); + onData(' after'); + + const output = registry.readSnapshot('terminal:split')?.output ?? ''; + expect(output).toContain('before '); + expect(output).toContain(' after'); + expect(output).not.toContain('\x1b'); + }); + + it('keeps non-query escapes (SGR, cursor motion) in the scrollback', async () => { + // The finals/intermediates the scrub removes are all requests; SGR (`m`), + // erase (`J`) and cursor-position (`H`) are display/control content and + // must survive so the client still re-renders colours on reconnect. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:nonquery', + workspaceCwd: '/workspace', + }); + + onData('\x1b[1;31mred\x1b[0m'); + onData('\x1b[2J'); + onData('\x1b[12;1H'); + + const output = registry.readSnapshot('terminal:nonquery')?.output ?? ''; + expect(output).toContain('\x1b[1;31mred\x1b[0m'); + expect(output).toContain('\x1b[2J'); + expect(output).toContain('\x1b[12;1H'); + }); + + it('resizes the headless responder with the client grid', async () => { + // The responder is constructed once at 80x24; a client resize that only + // touched session.pty would leave geometry-dependent replies (DSR cursor + // position) computed on the wrong grid. resize() must forward to it too. + osPlatform.mockReturnValue('win32'); + const resizeSpy = vi + .spyOn(Terminal.prototype, 'resize') + .mockImplementation(() => {}); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:responder-resize', + workspaceCwd: '/workspace', + }); + + expect(registry.resize('terminal:responder-resize', 120, 40)).toBe(true); + expect(resize).toHaveBeenCalledWith(120, 40); + expect(resizeSpy).toHaveBeenCalledWith(120, 40); + resizeSpy.mockRestore(); + }); + + it('cancels an in-flight create released during the headless load', async () => { + // `loadXtermHeadless` is the second suspension point after getPty(); a + // release() landing during it must cancel the spawn, not leak a PTY the + // caller already gave up on (the getPty() re-check alone does not cover + // this window). + osPlatform.mockReturnValue('win32'); + let resolveHeadless: ((value: unknown) => void) | undefined; + loadXtermHeadless.mockReturnValueOnce( + new Promise((resolve) => { + resolveHeadless = resolve; + }), + ); + const registry = new WebTerminalRegistry(); + const creating = registry.create({ + terminalId: 'terminal:pending-headless', + workspaceCwd: '/workspace', + }); + + await vi.waitFor(() => expect(loadXtermHeadless).toHaveBeenCalled()); + expect(registry.release('terminal:pending-headless')).toBe(true); + resolveHeadless?.({ Terminal }); + + await expect(creating).resolves.toEqual({ + error: 'Web terminal creation cancelled', + }); + expect(spawn).not.toHaveBeenCalled(); + }); + + it('does not spawn after disposal wins during the headless load', async () => { + osPlatform.mockReturnValue('win32'); + let resolveHeadless: ((value: unknown) => void) | undefined; + loadXtermHeadless.mockReturnValueOnce( + new Promise((resolve) => { + resolveHeadless = resolve; + }), + ); + const registry = new WebTerminalRegistry(); + const creating = registry.create({ + terminalId: 'terminal:pending-headless-dispose', + workspaceCwd: '/workspace', + }); + + await vi.waitFor(() => expect(loadXtermHeadless).toHaveBeenCalled()); + registry.dispose(); + resolveHeadless?.({ Terminal }); + + await expect(creating).resolves.toEqual({ + error: 'Web terminal registry disposed', + }); + expect(spawn).not.toHaveBeenCalled(); + }); + it('spawns non-Windows terminals without the bundled backend', async () => { osPlatform.mockReturnValue('linux'); const registry = new WebTerminalRegistry(); diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index b6d3ee8e30a..2c1b0f15122 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -63,23 +63,49 @@ export const MAX_CONCURRENT_WEB_TERMINALS = 8; const IDLE_RECLAIM_MS = 15 * 60 * 1000; /** - * Terminal queries a probing shell emits — requests, not display content — are - * CSI sequences whose final byte is `c` (Device Attributes, e.g. PowerShell's - * startup DA probe) or `n` (Device Status Report, e.g. PSReadLine's - * cursor-position request). The bundled ConPTY backend answers none of them - * itself (see shellExecutionService.ts), so they reach this registry as - * ordinary PTY output. Recording them in the scrollback lets a reconnect that - * replays `session.buffer` make the client's xterm.js re-answer each query and - * write the fresh reply back into the still-live shell's stdin. Both final - * bytes are unambiguous requests, so stripping them can never drop rendered - * content. Only complete sequences within one chunk are stripped — node-pty - * delivers these 3-6 byte probes as a single chunk. + * Terminal queries a probing shell emits — requests, not display content — + * reach this registry as ordinary PTY output because the bundled ConPTY + * backend answers none of them itself (see shellExecutionService.ts). Recording + * one in the scrollback lets a reconnect that replays `session.buffer` make the + * client's xterm.js re-answer it and write the fresh reply back into the + * still-live shell's stdin. The sequences matched below are exactly the query + * families xterm.js answers — Device Attributes (`c`, incl. the `>`/`=` + * intermediates), Device Status Report (`n`), DECREQTPARM (`x`), DECRQM + * (`$ p`), DECRQSS (`$ q`), XTVERSION (`> q`) and the OSC 10/11/4 colour + * queries — and none of those finals/intermediates is display content, so + * stripping them cannot drop rendered output. */ -function stripTerminalQueries(data: string): string { - // `no-control-regex` fires on the ESC byte, which is the whole point here: - // these are terminal CSI query sequences, not stray controls. +// `no-control-regex` fires on the ESC/BEL bytes, which is the whole point here: +// these are terminal query sequences, not stray controls. +const TERMINAL_QUERY_SEQUENCE_RE = // eslint-disable-next-line no-control-regex - return data.replace(/\x1b\[[?0-9;>]*[cn]/g, ''); + /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1b\](?:10|11|4;[0-9]+);\?(?:\x07|\x1b\\)/g; + +/** + * An incomplete trailing escape sequence — a query node-pty split across two + * chunks. It is carried to the next chunk and stripped as a whole rather than + * left to leak the partial probe into the scrollback. + */ +const PARTIAL_ESCAPE_SUFFIX_RE = + // eslint-disable-next-line no-control-regex + /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:[^\x07\x1b]|\x1b(?!\\))*)$/; + +/** + * Stateful per-session stripper: `node-pty` may deliver a probe split across + * two chunks, so an incomplete trailing escape sequence is held back until the + * next chunk completes (or never, if the stream simply ends — a trailing + * partial probe is not display content, so dropping it is harmless). + */ +class TerminalQueryStripper { + private pending = ''; + + strip(data: string): string { + const combined = this.pending + data; + const partial = PARTIAL_ESCAPE_SUFFIX_RE.exec(combined); + this.pending = partial ? partial[0] : ''; + const complete = partial ? combined.slice(0, partial.index) : combined; + return complete.replace(TERMINAL_QUERY_SEQUENCE_RE, ''); + } } interface PtySession { @@ -270,21 +296,35 @@ export class WebTerminalRegistry { // cannot make the client re-answer them into the still-live shell. let queryTerminal: Terminal | undefined; if (useBundledConpty) { - try { - const { Terminal: HeadlessTerminal } = await loadXtermHeadless(); - queryTerminal = new HeadlessTerminal({ + // `loadXtermHeadless` is a suspension point AFTER the getPty() re-checks + // above: a release()/releaseWorkspace()/dispose() landing during it must + // cancel the spawn here, or create() would leak a PTY the caller already + // gave up on. The rejection arm is folded into the same re-check — no + // responder is still a valid terminal, but a cancelled one is not. + const headlessModule = await loadXtermHeadless().catch(() => undefined); + if (this.cancelledCreations.has(terminalId)) { + this.finishCreating(terminalId); + return { error: 'Web terminal creation cancelled' }; + } + if (this.disposed) { + this.finishCreating(terminalId); + return { error: 'Web terminal registry disposed' }; + } + if (headlessModule) { + queryTerminal = new headlessModule.Terminal({ allowProposedApi: true, cols: 80, rows: 24, logLevel: 'off', }); - } catch { - // No responder available: the query stays unanswered (a bounded ~2s - // stall), never injected — the strip below still keeps it out of the - // scrollback. - queryTerminal = undefined; } + // No responder (headlessModule undefined): the query stays unanswered (a + // bounded ~2s stall), never injected — the strip still keeps it out of + // the scrollback. } + const queryStripper = useBundledConpty + ? new TerminalQueryStripper() + : undefined; let spawned: SpawnedWebTerminalPty; let proc: WebTerminalPty; let queryReplyDisposable: { dispose(): void } | undefined; @@ -313,7 +353,7 @@ export class WebTerminalRegistry { // Terminal disposed mid-stream (release raced a trailing chunk). } } - let buffered = useBundledConpty ? stripTerminalQueries(data) : data; + let buffered = queryStripper ? queryStripper.strip(data) : data; if (Buffer.byteLength(buffered) > MAX_BUFFER_BYTES) { buffered = Buffer.from(buffered) .subarray(-MAX_BUFFER_BYTES) @@ -549,6 +589,11 @@ export class WebTerminalRegistry { if (!session || session.exited) return false; try { session.pty.resize(cols, rows); + // Keep the headless query responder on the same grid the client renders: + // geometry-dependent replies (DSR cursor position, DECRQSS) must be + // computed against the browser's actual size, not the 80x24 it spawned + // with. + session.queryTerminal?.resize(cols, rows); return true; } catch { return false; From 1bd418b328ac72654df50d2ab297a0b62eee6cfd Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 17:42:38 +0800 Subject: [PATCH 5/9] fix(core): strip DCS DECRQSS queries in the terminal query filter Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmty6iqs0zv --- .../services/web-terminal-registry.test.ts | 37 ++++++++++++++++++- .../src/services/web-terminal-registry.ts | 14 ++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index 30b645d055d..43c980b4858 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -290,7 +290,7 @@ describe('WebTerminalRegistry', () => { it('strips every query family the browser client answers, not just DA/DSR', async () => { // The client's xterm.js answers more than the DA / DSR probes: tertiary - // DA (`=`), DECRQM (`$ p`), DECRQSS (`$ q`), XTVERSION (`> q`), + // DA (`=`), DECRQM (`$ p`), DECRQSS (DCS `ESC P $ q`), XTVERSION (`> q`), // DECREQTPARM (`x`) and the OSC colour queries all fire a reply too, so a // regex over just the `c`/`n` finals would leave them in the scrollback to // be re-answered into the live shell. Every one of these must come out. @@ -309,7 +309,7 @@ describe('WebTerminalRegistry', () => { onData('\x1b[?6n'); // DEC DSR cursor position onData('\x1b[?1$p'); // DECRQM onData('\x1b[>0q'); // XTVERSION - onData('\x1b[1$q'); // DECRQSS + onData('\x1bP$qm\x1b\\'); // DECRQSS (DCS) onData('\x1b[3x'); // DECREQTPARM onData('\x1b]10;?\x07'); // OSC foreground-colour query onData('\x1b]11;?\x07'); // OSC background-colour query @@ -345,6 +345,39 @@ describe('WebTerminalRegistry', () => { expect(output).not.toContain('\x1b'); }); + it('answers a DCS DECRQSS query once and keeps it out of the scrollback', async () => { + // Real DECRQSS is a DCS request (ESC P $ q ESC \), not the CSI + // `$q` form. xterm.js answers the DCS server-side; leaving the DCS bytes + // in the scrollback would let a reconnect replay re-answer them into the + // still-live shell, doubling the reply. The scrub must remove the DCS in + // both the complete and the chunk-split form, with the reply written back + // exactly once. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:decrqss', + workspaceCwd: '/workspace', + }); + + const reply = '\x1bP1$r0m\x1b\\'; + + // Complete form. + onData('\x1bP$qm\x1b\\'); + await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); + expect(write).toHaveBeenCalledTimes(1); + + // Split form, split before the ST terminator: the DCS halves must be + // carried across chunks and stripped whole, answering once more. + write.mockClear(); + onData('\x1bP$q'); + onData('m\x1b\\'); + await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); + expect(write).toHaveBeenCalledTimes(1); + + const output = registry.readSnapshot('terminal:decrqss')?.output ?? ''; + expect(output).not.toContain('\x1b'); + }); + it('keeps non-query escapes (SGR, cursor motion) in the scrollback', async () => { // The finals/intermediates the scrub removes are all requests; SGR (`m`), // erase (`J`) and cursor-position (`H`) are display/control content and diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index 2c1b0f15122..ce2c8f8312b 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -71,24 +71,26 @@ const IDLE_RECLAIM_MS = 15 * 60 * 1000; * still-live shell's stdin. The sequences matched below are exactly the query * families xterm.js answers — Device Attributes (`c`, incl. the `>`/`=` * intermediates), Device Status Report (`n`), DECREQTPARM (`x`), DECRQM - * (`$ p`), DECRQSS (`$ q`), XTVERSION (`> q`) and the OSC 10/11/4 colour - * queries — and none of those finals/intermediates is display content, so - * stripping them cannot drop rendered output. + * (`$ p`), DECRQSS (the DCS request `ESC P $ q ... ESC \`), XTVERSION + * (`> q`) and the OSC 10/11/4 colour queries — and none of those + * finals/intermediates is display content, so stripping them cannot drop + * rendered output. */ // `no-control-regex` fires on the ESC/BEL bytes, which is the whole point here: // these are terminal query sequences, not stray controls. const TERMINAL_QUERY_SEQUENCE_RE = // eslint-disable-next-line no-control-regex - /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1b\](?:10|11|4;[0-9]+);\?(?:\x07|\x1b\\)/g; + /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1b\](?:10|11|4;[0-9]+);\?(?:\x07|\x1b\\)|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*\x1b\\/g; /** * An incomplete trailing escape sequence — a query node-pty split across two * chunks. It is carried to the next chunk and stripped as a whole rather than - * left to leak the partial probe into the scrollback. + * left to leak the partial probe into the scrollback. DECRQSS arrives as DCS + * (`ESC P $ q ... ESC \`), so its partial form is held back the same way. */ const PARTIAL_ESCAPE_SUFFIX_RE = // eslint-disable-next-line no-control-regex - /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:[^\x07\x1b]|\x1b(?!\\))*)$/; + /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:[^\x07\x1b]|\x1b(?!\\))*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*)$/; /** * Stateful per-session stripper: `node-pty` may deliver a probe split across From f6c306adfd3a6d553c781a8e9ddaedb5f61e951d Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 21:43:10 +0800 Subject: [PATCH 6/9] fix(core): bound the terminal query stripper and tie it to its responder The stateful stripper held any unterminated OSC (a title SET included) with no cap, so one stray ESC ] swallowed every later chunk and grew pending unbounded; the DCS alternative also required the full introducer, leaking its halves that reassemble into a re-answered DECRQSS. Hold only viable query prefixes, cap the hold, end an OSC partial at a bare ESC (accepting the C1 ST terminator), hold the DCS introducer, and gate the stripper on the responder so a failed headless load no longer strips with nobody left to answer. Correct the query-set doc comment. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtyf3dx109 --- .../services/web-terminal-registry.test.ts | 89 +++++++++++++++++-- .../src/services/web-terminal-registry.ts | 45 +++++++--- 2 files changed, 115 insertions(+), 19 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index 43c980b4858..2a3acb12630 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -288,12 +288,43 @@ describe('WebTerminalRegistry', () => { expect(output).not.toContain('\x1b[c'); }); + it('does not strip queries when the headless responder fails to load', async () => { + // The stripper is tied to the responder it complements. loadXtermHeadless + // memoizes a rejection for the life of the process, so once it fails every + // later Windows terminal has no responder; stripping anyway would delete + // each DA/DSR/DECRQM probe from both the scrollback and the live listener, + // so the attached browser — which would have answered it at the merge base + // — never sees it and the shell stalls for its full probe timeout with no + // recovery short of a daemon restart. With no responder the query must + // reach the listener untouched, and nothing may be written back. + osPlatform.mockReturnValue('win32'); + loadXtermHeadless.mockRejectedValueOnce(new Error('headless load failed')); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:no-responder', + workspaceCwd: '/workspace', + }); + + const received: string[] = []; + registry.addOutputListener('terminal:no-responder', (data) => { + received.push(data); + }); + + onData('\x1b[c'); + + expect(received.join('')).toBe('\x1b[c'); + expect(write).not.toHaveBeenCalled(); + }); + it('strips every query family the browser client answers, not just DA/DSR', async () => { - // The client's xterm.js answers more than the DA / DSR probes: tertiary - // DA (`=`), DECRQM (`$ p`), DECRQSS (DCS `ESC P $ q`), XTVERSION (`> q`), - // DECREQTPARM (`x`) and the OSC colour queries all fire a reply too, so a - // regex over just the `c`/`n` finals would leave them in the scrollback to - // be re-answered into the live shell. Every one of these must come out. + // The client's xterm.js answers more than the DA / DSR probes: DECRQM + // (`$ p`), DECRQSS (DCS `ESC P $ q`) and the OSC colour queries all fire a + // reply too, so a regex over just the `c`/`n` finals would leave them in + // the scrollback to be re-answered into the live shell. `=`-DA3, + // XTVERSION (`> q`) and DECREQTPARM (`x`) are stripped as well — their + // finals/intermediates are never display content, so they must come out + // regardless of whether anyone would re-answer them. Every one of these + // must come out. osPlatform.mockReturnValue('win32'); const registry = new WebTerminalRegistry(); await registry.create({ @@ -345,6 +376,27 @@ describe('WebTerminalRegistry', () => { expect(output).not.toContain('\x1b'); }); + it('does not swallow payload after an unterminated non-query OSC', async () => { + // An OSC that is a title SET (not one of the 10/11/4 colour QUERIES) is + // not a viable probe prefix, so the stripper must not hold it: otherwise a + // program killed mid title-write (or a file containing a bare `1B 5D`) + // would leave `pending` matching every later chunk and eat all subsequent + // rendered bytes forever. The visible text after the partial OSC must + // still reach the scrollback. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:osc-partial', + workspaceCwd: '/workspace', + }); + + onData('\x1b]0;title'); + onData('visible'); + + const output = registry.readSnapshot('terminal:osc-partial')?.output ?? ''; + expect(output).toContain('visible'); + }); + it('answers a DCS DECRQSS query once and keeps it out of the scrollback', async () => { // Real DECRQSS is a DCS request (ESC P $ q ESC \), not the CSI // `$q` form. xterm.js answers the DCS server-side; leaving the DCS bytes @@ -374,6 +426,22 @@ describe('WebTerminalRegistry', () => { await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); expect(write).toHaveBeenCalledTimes(1); + // The DCS introducer itself may straddle a chunk boundary: a split after + // `ESC P` or after `ESC P $` must hold the introducer rather than leak the + // halves, which would reassemble in the buffer into a complete DECRQSS the + // client re-answers on replay. + write.mockClear(); + onData('\x1bP'); + onData('$qm\x1b\\'); + await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); + expect(write).toHaveBeenCalledTimes(1); + + write.mockClear(); + onData('\x1bP$'); + onData('qm\x1b\\'); + await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); + expect(write).toHaveBeenCalledTimes(1); + const output = registry.readSnapshot('terminal:decrqss')?.output ?? ''; expect(output).not.toContain('\x1b'); }); @@ -483,6 +551,17 @@ describe('WebTerminalRegistry', () => { // Inert on the POSIX prebuilds, but pinned so the option stays a // deliberate platform branch rather than an unconditional `true`. expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: false }); + + // The responder-plus-strip feature is win32-gated, and the POSIX side of + // that gate is pinned here: on Linux/macOS there is no server-side + // answerer, the browser is the only terminal that can reply, so a query + // must be live-forwarded untouched rather than stripped — and the headless + // responder must never be constructed. + onData('\x1b[c'); + expect(registry.readSnapshot('terminal:posix-backend')?.output).toContain( + '\x1b[c', + ); + expect(loadXtermHeadless).not.toHaveBeenCalled(); }); it('retries a failed bundled spawn once on the inbox backend', async () => { diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index ce2c8f8312b..f341d989d55 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -68,11 +68,13 @@ const IDLE_RECLAIM_MS = 15 * 60 * 1000; * backend answers none of them itself (see shellExecutionService.ts). Recording * one in the scrollback lets a reconnect that replays `session.buffer` make the * client's xterm.js re-answer it and write the fresh reply back into the - * still-live shell's stdin. The sequences matched below are exactly the query - * families xterm.js answers — Device Attributes (`c`, incl. the `>`/`=` - * intermediates), Device Status Report (`n`), DECREQTPARM (`x`), DECRQM - * (`$ p`), DECRQSS (the DCS request `ESC P $ q ... ESC \`), XTVERSION - * (`> q`) and the OSC 10/11/4 colour queries — and none of those + * still-live shell's stdin. The sequences matched below are the query + * families xterm.js answers — Device Attributes (`c`, incl. the `>` + * intermediate), Device Status Report (`n`/`?n`), DECRQM (`$ p`), DECRQSS + * (the DCS request `ESC P $ q ... ESC \`) and the OSC 10/11/4 colour queries + * — plus `=`-DA3, XTVERSION (`> q`) and DECREQTPARM (`x`), whose + * finals/intermediates are never display content (neither xterm build answers + * those three, so stripping them is not removing an answerer). None of these * finals/intermediates is display content, so stripping them cannot drop * rendered output. */ @@ -85,27 +87,38 @@ const TERMINAL_QUERY_SEQUENCE_RE = /** * An incomplete trailing escape sequence — a query node-pty split across two * chunks. It is carried to the next chunk and stripped as a whole rather than - * left to leak the partial probe into the scrollback. DECRQSS arrives as DCS - * (`ESC P $ q ... ESC \`), so its partial form is held back the same way. + * left to leak the partial probe into the scrollback. Only viable query + * prefixes are held: the OSC arm keeps the `?` predicate (so an unterminated + * title/colour SET is not mistaken for a query), ends at any ESC that is not + * `ESC \` exactly as xterm cancels, and accepts the C1 ST byte `\x9c` as a + * terminator. DECRQSS arrives as DCS (`ESC P $ q ... ESC \`), so both the + * introducer (`\x1bP`, `\x1bP$`) and the full body are held back the same way. */ const PARTIAL_ESCAPE_SUFFIX_RE = // eslint-disable-next-line no-control-regex - /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:[^\x07\x1b]|\x1b(?!\\))*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*)$/; + /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:10|11|4;[0-9]+);\?(?:[^\x07\x1b\x9c])*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*|\x1bP\$?)$/; /** * Stateful per-session stripper: `node-pty` may deliver a probe split across * two chunks, so an incomplete trailing escape sequence is held back until the * next chunk completes (or never, if the stream simply ends — a trailing - * partial probe is not display content, so dropping it is harmless). + * partial probe is a few bytes of a query nobody answered, so dropping it is + * harmless). The hold is capped at MAX_HELD_ESCAPE_CHARS: a real query is a + * few dozen bytes, so anything longer is payload, not a split probe, and is + * flushed whole rather than swallowed. */ +const MAX_HELD_ESCAPE_CHARS = 256; + class TerminalQueryStripper { private pending = ''; strip(data: string): string { const combined = this.pending + data; const partial = PARTIAL_ESCAPE_SUFFIX_RE.exec(combined); - this.pending = partial ? partial[0] : ''; - const complete = partial ? combined.slice(0, partial.index) : combined; + const hold = + partial !== null && partial[0].length <= MAX_HELD_ESCAPE_CHARS; + this.pending = hold ? partial![0] : ''; + const complete = hold ? combined.slice(0, partial!.index) : combined; return complete.replace(TERMINAL_QUERY_SEQUENCE_RE, ''); } } @@ -321,10 +334,14 @@ export class WebTerminalRegistry { }); } // No responder (headlessModule undefined): the query stays unanswered (a - // bounded ~2s stall), never injected — the strip still keeps it out of - // the scrollback. + // bounded ~2s stall), never injected. } - const queryStripper = useBundledConpty + // Tie the stripper to the responder it complements. Without a responder + // the browser is the only terminal that can answer, so stripping here + // would delete a query nobody is left to answer — permanently, because + // loadXtermHeadless memoizes its rejection for the life of the process. + // Only strip when a responder is actually present to answer the queries. + const queryStripper = queryTerminal ? new TerminalQueryStripper() : undefined; let spawned: SpawnedWebTerminalPty; From b73b17c99422214673fe8e232b527f3b5fa6064b Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sat, 12 Sep 2026 22:58:48 +0800 Subject: [PATCH 7/9] style(core): format web-terminal-registry.ts with prettier Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-conflict/jmtyhy9mj0e --- packages/core/src/services/web-terminal-registry.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index f341d989d55..c5d19734153 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -115,8 +115,7 @@ class TerminalQueryStripper { strip(data: string): string { const combined = this.pending + data; const partial = PARTIAL_ESCAPE_SUFFIX_RE.exec(combined); - const hold = - partial !== null && partial[0].length <= MAX_HELD_ESCAPE_CHARS; + const hold = partial !== null && partial[0].length <= MAX_HELD_ESCAPE_CHARS; this.pending = hold ? partial![0] : ''; const complete = hold ? combined.slice(0, partial!.index) : combined; return complete.replace(TERMINAL_QUERY_SEQUENCE_RE, ''); From d8975fa0d923d021faf006ab7e77e76944afe9cb Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sun, 13 Sep 2026 01:38:37 +0800 Subject: [PATCH 8/9] fix(web-terminal): stop scrubbing the OSC colour family the server cannot answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stateful stripper added here removed the OSC 10/11/12/4 colour queries from both the scrollback and the live listener, but the pinned @xterm/headless 5.5.0 responder answers none of them: `onData` carries DA/DSR/DECRQM/DECRQSS only, while `_setOrReportSpecialColor` reports on the internal `_onColor` emitter that the headless Terminal does not expose (`term.onColor` is undefined) and that nothing in this file subscribes to. Deleting the family from the browser's stream as well left a probing program unanswered, where the browser's own xterm.js 6.0.0 `_handleColorEvent` is the answerer and answered it at the merge base. Narrow both regexes to the families the responder actually answers (plus the never-display-content DA3 / XTVERSION / DECREQTPARM trio that no build answers), drop the half-covered colour arm from the partial matcher — so the strip set and the doc block no longer disagree about what the colour family is — and correct the doc block to state the split. Covered by a new test asserting the whole family reaches both the live listener and the snapshot untouched, and that nothing is written back; it fails if the colour arm is restored. The colour queries that a reconnect replay re-answers are part of the replay-suppression redesign tracked in #11734. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtyno0zh0n --- .../services/web-terminal-registry.test.ts | 70 ++++++++++++++----- .../src/services/web-terminal-registry.ts | 48 ++++++++----- 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index 2a3acb12630..bf21b75cda3 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -316,15 +316,14 @@ describe('WebTerminalRegistry', () => { expect(write).not.toHaveBeenCalled(); }); - it('strips every query family the browser client answers, not just DA/DSR', async () => { - // The client's xterm.js answers more than the DA / DSR probes: DECRQM - // (`$ p`), DECRQSS (DCS `ESC P $ q`) and the OSC colour queries all fire a - // reply too, so a regex over just the `c`/`n` finals would leave them in - // the scrollback to be re-answered into the live shell. `=`-DA3, - // XTVERSION (`> q`) and DECREQTPARM (`x`) are stripped as well — their - // finals/intermediates are never display content, so they must come out - // regardless of whether anyone would re-answer them. Every one of these - // must come out. + it('strips every query family the headless responder answers, not just DA/DSR', async () => { + // The server-side responder answers more than the DA / DSR probes: DECRQM + // (`$ p`) and DECRQSS (DCS `ESC P $ q`) fire a reply too, so a regex over + // just the `c`/`n` finals would leave them in the scrollback to be + // re-answered into the live shell. `=`-DA3, XTVERSION (`> q`) and + // DECREQTPARM (`x`) are stripped as well — their finals/intermediates are + // never display content, so they must come out regardless of whether + // anyone would re-answer them. Every one of these must come out. osPlatform.mockReturnValue('win32'); const registry = new WebTerminalRegistry(); await registry.create({ @@ -342,9 +341,6 @@ describe('WebTerminalRegistry', () => { onData('\x1b[>0q'); // XTVERSION onData('\x1bP$qm\x1b\\'); // DECRQSS (DCS) onData('\x1b[3x'); // DECREQTPARM - onData('\x1b]10;?\x07'); // OSC foreground-colour query - onData('\x1b]11;?\x07'); // OSC background-colour query - onData('\x1b]4;5;?\x07'); // OSC palette-colour query onData('done'); const output = registry.readSnapshot('terminal:queries')?.output ?? ''; @@ -353,6 +349,44 @@ describe('WebTerminalRegistry', () => { expect(output).not.toContain('\x1b'); }); + it('leaves the OSC colour queries for the browser client to answer', async () => { + // The pinned @xterm/headless 5.5.0 responder answers no colour query: + // `onData` carries DSR/DA/DECRQM/DECRQSS only, while + // `_setOrReportSpecialColor` reports on the internal `_onColor` emitter + // that the headless Terminal does not expose (`term.onColor` is undefined) + // and nothing here subscribes to. Scrubbing the family therefore deleted it + // from the browser's stream as well, leaving a probing program unanswered + // where the browser's xterm.js 6.0.0 `_handleColorEvent` answered it at the + // merge base. The queries must reach the client untouched — whole or split + // across chunks, every form of the family — and nothing may be written + // back. The colour queries a reconnect replay re-answers are tracked in + // #11734. + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:colour', + workspaceCwd: '/workspace', + }); + + const received: string[] = []; + registry.addOutputListener('terminal:colour', (data) => { + received.push(data); + }); + + onData('\x1b]10;?\x07'); // OSC foreground-colour query + onData('\x1b]11;?'); // OSC background-colour query, split + onData('\x07'); // ... completed by the next chunk + onData('\x1b]12;?\x07'); // OSC cursor-colour query + onData('\x1b]4;5;?\x07'); // OSC palette-colour query + onData('\x1b]4;0;?;1;?\x07'); // OSC multi-index palette query + + const family = + '\x1b]10;?\x07\x1b]11;?\x07\x1b]12;?\x07\x1b]4;5;?\x07\x1b]4;0;?;1;?\x07'; + expect(received.join('')).toBe(family); + expect(registry.readSnapshot('terminal:colour')?.output).toBe(family); + expect(write).not.toHaveBeenCalled(); + }); + it('strips a query split across two chunks', async () => { // node-pty can deliver a probe split across two chunks; a stateless // per-chunk matcher would leave the halves in the scrollback and only the @@ -377,12 +411,12 @@ describe('WebTerminalRegistry', () => { }); it('does not swallow payload after an unterminated non-query OSC', async () => { - // An OSC that is a title SET (not one of the 10/11/4 colour QUERIES) is - // not a viable probe prefix, so the stripper must not hold it: otherwise a - // program killed mid title-write (or a file containing a bare `1B 5D`) - // would leave `pending` matching every later chunk and eat all subsequent - // rendered bytes forever. The visible text after the partial OSC must - // still reach the scrollback. + // No OSC is a viable probe prefix — no OSC family is stripped any more — + // so the stripper must not hold one: otherwise a program killed mid + // title-write (or a file containing a bare `1B 5D`) would leave `pending` + // matching every later chunk and eat all subsequent rendered bytes + // forever. The visible text after the partial OSC must still reach the + // scrollback. osPlatform.mockReturnValue('win32'); const registry = new WebTerminalRegistry(); await registry.create({ diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index c5d19734153..c4ca8db496d 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -68,35 +68,49 @@ const IDLE_RECLAIM_MS = 15 * 60 * 1000; * backend answers none of them itself (see shellExecutionService.ts). Recording * one in the scrollback lets a reconnect that replays `session.buffer` make the * client's xterm.js re-answer it and write the fresh reply back into the - * still-live shell's stdin. The sequences matched below are the query - * families xterm.js answers — Device Attributes (`c`, incl. the `>` - * intermediate), Device Status Report (`n`/`?n`), DECRQM (`$ p`), DECRQSS - * (the DCS request `ESC P $ q ... ESC \`) and the OSC 10/11/4 colour queries - * — plus `=`-DA3, XTVERSION (`> q`) and DECREQTPARM (`x`), whose - * finals/intermediates are never display content (neither xterm build answers - * those three, so stripping them is not removing an answerer). None of these - * finals/intermediates is display content, so stripping them cannot drop - * rendered output. + * still-live shell's stdin — and leaving one in the live stream lets the + * browser answer it a second time, alongside the server-side responder. + * + * So only the families the pinned `@xterm/headless` 5.5.0 responder actually + * answers are matched, giving every probe exactly one answerer: Device + * Attributes (`c`, incl. the `>` intermediate), Device Status Report + * (`n`/`?n`), DECRQM (`$ p`) and DECRQSS (the DCS request + * `ESC P $ q ... ESC \`). `=`-DA3, XTVERSION (`> q`) and DECREQTPARM (`x`) are + * matched as well although no build answers them: their finals/intermediates + * are never display content, so removing them cannot drop rendered output, and + * leaving them in the replay would only re-parse a request nobody answers. + * + * The OSC 10/11/12/4 colour queries are deliberately NOT matched. The pinned + * responder answers none of them: `onData` carries DA/DSR/DECRQM/DECRQSS only, + * while `_setOrReportSpecialColor` reports on the internal `_onColor` emitter, + * which the headless `Terminal` does not expose (`term.onColor` is `undefined` + * on 5.5.0) and which nothing in this file subscribes to. Scrubbing that + * family therefore deleted it from the browser's stream too and left a probing + * program unanswered — where the browser's own xterm.js 6.0.0 + * `_handleColorEvent` is the answerer and answered it at the merge base. The + * colour queries a reconnect replay re-answers are part of the + * replay-suppression redesign tracked in #11734. */ // `no-control-regex` fires on the ESC/BEL bytes, which is the whole point here: // these are terminal query sequences, not stray controls. const TERMINAL_QUERY_SEQUENCE_RE = // eslint-disable-next-line no-control-regex - /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1b\](?:10|11|4;[0-9]+);\?(?:\x07|\x1b\\)|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*\x1b\\/g; + /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*\x1b\\/g; /** * An incomplete trailing escape sequence — a query node-pty split across two * chunks. It is carried to the next chunk and stripped as a whole rather than - * left to leak the partial probe into the scrollback. Only viable query - * prefixes are held: the OSC arm keeps the `?` predicate (so an unterminated - * title/colour SET is not mistaken for a query), ends at any ESC that is not - * `ESC \` exactly as xterm cancels, and accepts the C1 ST byte `\x9c` as a - * terminator. DECRQSS arrives as DCS (`ESC P $ q ... ESC \`), so both the - * introducer (`\x1bP`, `\x1bP$`) and the full body are held back the same way. + * left to leak the partial probe into the scrollback — or, on the live path, + * to render the tail of a probe as text. Only viable query prefixes are held: + * a CSI introducer with its parameter run (the complete sequence is filtered by + * the regex above, so a non-query CSI still passes through), and DECRQSS, which + * arrives as DCS (`ESC P $ q ... ESC \`) — both the introducer (`\x1bP`, + * `\x1bP$`) and the full body are held back the same way. No OSC prefix is + * held: no OSC family is stripped any more. */ const PARTIAL_ESCAPE_SUFFIX_RE = // eslint-disable-next-line no-control-regex - /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1b\](?:10|11|4;[0-9]+);\?(?:[^\x07\x1b\x9c])*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*|\x1bP\$?)$/; + /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*|\x1bP\$?)$/; /** * Stateful per-session stripper: `node-pty` may deliver a probe split across From b7934e39c60c8a3eb473f31a015d907e34cf3163 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Sun, 13 Sep 2026 01:59:42 +0800 Subject: [PATCH 9/9] fix(web-terminal): separate live replies from reconnect history Preserve raw PTY bytes and limit the Windows server responder to primary DA. Mark snapshots explicitly and restore history into a detached browser terminal while the old terminal continues forwarding user input. Activate live replies at the snapshot write callback, reject incompatible peers, and preserve the parallel color-query regression fix. Refs #11734. Co-authored-by: Qwen-Coder --- docs/design/web-terminal-replay.md | 69 +++++ docs/design/web-terminal-replay.zh-CN.md | 38 +++ .../cli/src/serve/routes/terminal.test.ts | 25 +- packages/cli/src/serve/routes/terminal.ts | 13 + .../services/web-terminal-registry.test.ts | 238 +++--------------- .../src/services/web-terminal-registry.ts | 114 +-------- .../terminal/TerminalPanel.test.tsx | 158 ++++++++++-- .../components/terminal/TerminalPanel.tsx | 127 +++++++--- 8 files changed, 422 insertions(+), 360 deletions(-) create mode 100644 docs/design/web-terminal-replay.md create mode 100644 docs/design/web-terminal-replay.zh-CN.md diff --git a/docs/design/web-terminal-replay.md b/docs/design/web-terminal-replay.md new file mode 100644 index 00000000000..23c94ef8b84 --- /dev/null +++ b/docs/design/web-terminal-replay.md @@ -0,0 +1,69 @@ +# Web terminal replay and query ownership + +[English](web-terminal-replay.md) | [简体中文](web-terminal-replay.zh-CN.md) + +## Goal and scope + +PR #11643 switches Windows web-terminal PTYs to bundled ConPTY to avoid the +inbox backend's natural-exit host leak. Its review follow-up #11734 requires +preserving live terminal answers without answering old queries on reconnect. +The cross-package correction was approved on 2026-09-13. + +Remove the server's escape-sequence filters. This does not change POSIX PTY +backend selection, add dependencies, or broaden agent-view behavior. The replay +boundary applies to Web Shell terminals on all hosts, not only VS Code. + +## Ownership + +The Windows headless terminal parses the original stream but forwards only its +primary DA answer, including before a browser attaches. Snapshot metadata tells +the browser to consume primary DA through xterm's native CSI handler in that +case. When headless cannot load, the browser owns live DA; a startup probe before +attachment may time out. The existing inbox spawn-failure retry remains. + +Colors, modes and cursor/geometry queries belong to the browser that renders the +live output. Headless does not supply those answers. PTY output and bounded +scrollback retain their original bytes, including split escape sequences. + +## Transport and compatibility + +The workspace-resolved `/terminal` connection requests `replay=1`. Before the +initial binary output, the server sends a NUL-prefixed JSON control frame: + +```json +{ "type": "snapshot", "replay": true, "handlesPrimaryDa": true } +``` + +The next binary frame is the snapshot; later binary frames are live output. +Keeping output binary preserves its size bound and prevents PTY bytes from being +interpreted as control frames. A newly created PTY uses `replay:false`: its +buffered startup queries have not been answered by a browser yet. + +Legacy clients are rejected before PTY creation with a reload message. The new +client rejects an unmarked binary snapshot from an older daemon. Both must be +updated together; release-only connections still work. Workspace validation, +ownership checks, heartbeat and output/input backpressure remain unchanged. + +## Browser restoration + +Restore history into a terminal opened on a detached DOM host without an input +forwarder. Keep the old visible terminal forwarding keyboard/paste/IME input. +The snapshot write callback installs the new forwarder and swaps the visible +host before queued live writes parse. Dispose the old terminal after the swap. +Fresh startup output gets its forwarder before parsing. A retryable disconnection +or unmount disposes an unfinished restore; a stale callback cannot replace a +newer connection's view. This uses public xterm APIs, not a global flag that drops +both replies and user input. + +## Verification and acceptance + +- Registry: original live/snapshot bytes survive; only primary DA is answered. +- Route: snapshot metadata precedes binary history; fresh and reconnect paths + differ; old clients fail clearly without spawning. +- Browser: replay produces no outgoing answers, queued live color/mode queries + do, and typing during replay still reaches stdin. Initial queries work; + primary DA has one owner; interrupted replay cannot replace the current view. +- Run focused tests and a mounted real-browser check after implementation. + Native Windows process counts, fallback DLL loading and interactive rendering + require separate Windows acceptance. Browser evidence is not proof of the + original native leak fix. diff --git a/docs/design/web-terminal-replay.zh-CN.md b/docs/design/web-terminal-replay.zh-CN.md new file mode 100644 index 00000000000..dd12c22fa6f --- /dev/null +++ b/docs/design/web-terminal-replay.zh-CN.md @@ -0,0 +1,38 @@ +# Web terminal 回放与查询应答归属 + +[English](web-terminal-replay.md) | [简体中文](web-terminal-replay.zh-CN.md) + +## 目标与范围 + +PR #11643 将 Windows web-terminal PTY 切到 bundled ConPTY,避免 inbox 后端在自然退出时泄漏 host。其评审后续 #11734 要求保留实时终端应答,并且重连时不再应答历史查询。2026-09-13 已批准这次跨包修正。 + +移除服务端转义序列过滤器。不改变 POSIX PTY 后端选择,不增加依赖,也不扩展 agent-view 行为。回放边界适用于所有平台的 Web Shell 终端,并非仅限 VS Code。 + +## 应答归属 + +Windows headless 终端解析原始流,但只转发 primary DA 应答,包括浏览器尚未连接时的探测。快照元数据通知浏览器在这种情况下通过 xterm 原生 CSI handler 消费 primary DA。headless 无法加载时,浏览器负责实时 DA;连接前的启动探测可能超时。保留现有 inbox spawn 失败回退。 + +颜色、模式、光标和几何查询由渲染实时输出的浏览器应答。headless 不提供这些答案。PTY 输出及有界 scrollback 保留原始字节,包括跨块的转义序列。 + +## 传输与兼容性 + +已解析工作区的 `/terminal` 连接携带 `replay=1`。服务端在首个二进制输出前发送以 NUL 开头的 JSON 控制帧: + +```json +{ "type": "snapshot", "replay": true, "handlesPrimaryDa": true } +``` + +下一个二进制帧是快照,后续二进制帧是实时输出。继续以二进制传输输出,既保留大小边界,也避免将 PTY 字节解释成控制帧。新建 PTY 使用 `replay:false`,因为浏览器还没有应答其缓存的启动查询。 + +旧客户端会在创建 PTY 前被拒绝,并收到刷新提示。新客户端拒绝旧 daemon 未标记的二进制快照。两端需要同步升级;仅释放会话的连接仍可使用。工作区校验、归属检查、心跳以及输入输出背压保持不变。 + +## 浏览器恢复 + +在脱离页面的 DOM 容器中打开新终端,不安装输入转发器,用它恢复历史。旧的可见终端继续转发键盘、粘贴和 IME 输入。快照 write 回调安装新转发器并切换可见容器,然后排队的实时 write 才会解析。切换后释放旧终端。首次启动输出在解析前就安装转发器。可重试的断线或组件卸载会释放尚未完成的恢复实例,旧回调不能替换新连接的视图。只使用 xterm 公开 API,不使用同时丢弃应答和用户输入的全局标志。 + +## 验证与验收 + +- Registry:实时与快照字节不变;只应答 primary DA。 +- 路由:快照元数据先于二进制历史;区分首次创建与重连;旧客户端明确失败且不 spawn。 +- 浏览器:回放没有向外应答,排队的实时颜色和模式查询能应答,回放期间键盘输入仍到达 stdin。首次查询有效;primary DA 只有一个应答方;中断的回放不能替换当前视图。 +- 实现完成后运行定向测试和真实浏览器中的组件检查。原生 Windows 进程计数、DLL 回退加载和交互渲染需要单独的 Windows 验收。浏览器证据不等于原始原生泄漏已经修复。 diff --git a/packages/cli/src/serve/routes/terminal.test.ts b/packages/cli/src/serve/routes/terminal.test.ts index 828b0828c94..17a2ac81cdc 100644 --- a/packages/cli/src/serve/routes/terminal.test.ts +++ b/packages/cli/src/serve/routes/terminal.test.ts @@ -18,7 +18,7 @@ const context = { const resolveWorkspace = (selector: string) => selector === '/workspace' ? context : undefined; const request = { - url: '/terminal?terminalId=terminal%3Amanual-1&cwd=%2Fworkspace', + url: '/terminal?terminalId=terminal%3Amanual-1&cwd=%2Fworkspace&replay=1', } as IncomingMessage; class FakeWebSocket extends EventEmitter { @@ -45,6 +45,7 @@ function registryWithSnapshot( exited: boolean; exitCode?: number; workspaceCwd: string; + handlesPrimaryDa?: boolean; } | undefined, ) { @@ -61,6 +62,18 @@ function registryWithSnapshot( } describe('terminal WebSocket route', () => { + it('rejects legacy replay clients before creating a PTY', async () => { + const registry = registryWithSnapshot(undefined); + const ws = new FakeWebSocket(); + await createTerminalWsHandler(registry, resolveWorkspace).onConnection( + ws as unknown as WebSocket, + { url: request.url!.replace('&replay=1', '') } as IncomingMessage, + ); + expect(ws.close).toHaveBeenCalledWith(4002, 'Terminal protocol mismatch'); + expect(registry.create).not.toHaveBeenCalled(); + expect(registry.addOutputListener).not.toHaveBeenCalled(); + }); + it('rejects invalid workspaces and terminal ids before creating a PTY', async () => { const registry = registryWithSnapshot(undefined); const unknown = new FakeWebSocket(); @@ -119,6 +132,9 @@ describe('terminal WebSocket route', () => { env: { PATH: '/runtime/bin' }, }); expect(sentOutput(ws)).toContain('prompt $ '); + expect(ws.sent[0]).toBe( + '\x00{"type":"snapshot","replay":false,"handlesPrimaryDa":false}', + ); expect(registry.write).toHaveBeenCalledWith( 'terminal:manual-1', 'echo ready\r', @@ -320,6 +336,7 @@ describe('terminal WebSocket route', () => { output: '', exited: false, workspaceCwd: '/workspace', + handlesPrimaryDa: true, }); vi.mocked(registry.addOutputListener).mockImplementation( (_terminalId, listener) => { @@ -343,6 +360,10 @@ describe('terminal WebSocket route', () => { expect(sentOutput(first)).toContain('live'); expect(sentOutput(second)).toContain('live'); + expect(first.sent.slice(0, 2)).toEqual([ + '\x00{"type":"snapshot","replay":true,"handlesPrimaryDa":true}', + Buffer.from(''), + ]); expect(registry.create).not.toHaveBeenCalled(); }); @@ -429,7 +450,7 @@ describe('terminal WebSocket route', () => { await createTerminalWsHandler(registry, resolveWorkspace).onConnection( ws as unknown as WebSocket, { - url: `/terminal?terminalId=${terminalId}&cwd=%2Fworkspace`, + url: `/terminal?terminalId=${terminalId}&cwd=%2Fworkspace&replay=1`, } as IncomingMessage, ); diff --git a/packages/cli/src/serve/routes/terminal.ts b/packages/cli/src/serve/routes/terminal.ts index a22edeb35d4..3a7fad9752d 100644 --- a/packages/cli/src/serve/routes/terminal.ts +++ b/packages/cli/src/serve/routes/terminal.ts @@ -151,6 +151,14 @@ export function createTerminalWsHandler( ws.close(4004, 'Terminal released'); return; } + if (url.searchParams.get('replay') !== '1') { + sendControl(ws, { + type: 'error', + message: 'Terminal protocol changed; reload this page.', + }); + ws.close(4002, 'Terminal protocol mismatch'); + return; + } const workspaceSelector = selector; let created = false; @@ -328,6 +336,11 @@ export function createTerminalWsHandler( ws.off('error', markClosed); ws.on('error', cleanup); if (!ensureWorkspaceAvailable()) return; + sendControl(ws, { + type: 'snapshot', + replay: !created, + handlesPrimaryDa: snapshot.handlesPrimaryDa === true, + }); if (!sendOutput(ws, snapshot.output)) { cleanup(); ws.close(1013, 'Terminal output backpressure'); diff --git a/packages/core/src/services/web-terminal-registry.test.ts b/packages/core/src/services/web-terminal-registry.test.ts index bf21b75cda3..669d58ed728 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -259,44 +259,45 @@ describe('WebTerminalRegistry', () => { expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: true }); }); - it('answers the bundled-backend DA probe and keeps it out of the scrollback', async () => { - // The bundled ConPTY backend answers no terminal queries itself, so - // PowerShell's startup DA probe would stall for its full timeout unless a - // terminal answers it server-side — and its query bytes, if recorded in the - // scrollback, would be re-answered by the client's xterm.js on reconnect - // and written back into the still-live shell as input. The forwarder must - // write the reply back to the PTY once, and the scrub must keep the query - // out of readSnapshot's replay. + it('preserves the PTY stream and answers only primary DA on Windows', async () => { osPlatform.mockReturnValue('win32'); const registry = new WebTerminalRegistry(); await registry.create({ - terminalId: 'terminal:da-probe', + terminalId: 'terminal:queries', workspaceCwd: '/workspace', }); - - onData('Microsoft Windows [Version 10.0.22631]\r\n'); - onData('\x1b[c'); - onData('C:\\work> '); - + const received: string[] = []; + registry.addOutputListener('terminal:queries', (data) => + received.push(data), + ); + const chunks = [ + '\x1b]0;title', + 'visible\x1b[1;31mred\x1b[0m\x1b[2J\x1b[12;1H', + '\x1bP', + '$qm\x1b\\', + '\x1bP$', + 'qm\x1b\\', + '\x1b[6n\x1b[?2026$p\x1b[>c', + '\x1b]10;?\x07\x1b]11;?\x07\x1b]12;?\x07', + '\x1b]4;0;?;1;?\x07', + '\x1b[', + 'c', + ]; + for (const chunk of chunks) onData(chunk); + + // DA is last, so its answer also waits for all preceding queries to parse. await vi.waitFor(() => { - expect(write).toHaveBeenCalledWith('\x1b[?1;2c'); + expect(write).toHaveBeenCalledExactlyOnceWith('\x1b[?1;2c'); }); - - const output = registry.readSnapshot('terminal:da-probe')?.output ?? ''; - expect(output).toContain('Microsoft Windows'); - expect(output).toContain('C:\\work> '); - expect(output).not.toContain('\x1b[c'); + expect(received).toEqual(chunks); + expect(registry.readSnapshot('terminal:queries')).toMatchObject({ + output: chunks.join(''), + handlesPrimaryDa: true, + }); + registry.dispose(); }); - it('does not strip queries when the headless responder fails to load', async () => { - // The stripper is tied to the responder it complements. loadXtermHeadless - // memoizes a rejection for the life of the process, so once it fails every - // later Windows terminal has no responder; stripping anyway would delete - // each DA/DSR/DECRQM probe from both the scrollback and the live listener, - // so the attached browser — which would have answered it at the merge base - // — never sees it and the shell stalls for its full probe timeout with no - // recovery short of a daemon restart. With no responder the query must - // reach the listener untouched, and nothing may be written back. + it('leaves primary DA to the browser when headless cannot load', async () => { osPlatform.mockReturnValue('win32'); loadXtermHeadless.mockRejectedValueOnce(new Error('headless load failed')); const registry = new WebTerminalRegistry(); @@ -304,7 +305,6 @@ describe('WebTerminalRegistry', () => { terminalId: 'terminal:no-responder', workspaceCwd: '/workspace', }); - const received: string[] = []; registry.addOutputListener('terminal:no-responder', (data) => { received.push(data); @@ -313,40 +313,11 @@ describe('WebTerminalRegistry', () => { onData('\x1b[c'); expect(received.join('')).toBe('\x1b[c'); + expect( + registry.readSnapshot('terminal:no-responder')?.handlesPrimaryDa, + ).not.toBe(true); expect(write).not.toHaveBeenCalled(); - }); - - it('strips every query family the headless responder answers, not just DA/DSR', async () => { - // The server-side responder answers more than the DA / DSR probes: DECRQM - // (`$ p`) and DECRQSS (DCS `ESC P $ q`) fire a reply too, so a regex over - // just the `c`/`n` finals would leave them in the scrollback to be - // re-answered into the live shell. `=`-DA3, XTVERSION (`> q`) and - // DECREQTPARM (`x`) are stripped as well — their finals/intermediates are - // never display content, so they must come out regardless of whether - // anyone would re-answer them. Every one of these must come out. - osPlatform.mockReturnValue('win32'); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:queries', - workspaceCwd: '/workspace', - }); - - onData('prompt> '); - onData('\x1b[c'); // DA1 - onData('\x1b[>c'); // DA2 - onData('\x1b[=c'); // DA3 - onData('\x1b[6n'); // DSR cursor position - onData('\x1b[?6n'); // DEC DSR cursor position - onData('\x1b[?1$p'); // DECRQM - onData('\x1b[>0q'); // XTVERSION - onData('\x1bP$qm\x1b\\'); // DECRQSS (DCS) - onData('\x1b[3x'); // DECREQTPARM - onData('done'); - - const output = registry.readSnapshot('terminal:queries')?.output ?? ''; - expect(output).toContain('prompt> '); - expect(output).toContain('done'); - expect(output).not.toContain('\x1b'); + registry.dispose(); }); it('leaves the OSC colour queries for the browser client to answer', async () => { @@ -387,140 +358,6 @@ describe('WebTerminalRegistry', () => { expect(write).not.toHaveBeenCalled(); }); - it('strips a query split across two chunks', async () => { - // node-pty can deliver a probe split across two chunks; a stateless - // per-chunk matcher would leave the halves in the scrollback and only the - // second half would never be re-answered as a whole. The stripper carries - // the incomplete CSI prefix to the next chunk and strips it whole. - osPlatform.mockReturnValue('win32'); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:split', - workspaceCwd: '/workspace', - }); - - onData('before '); - onData('\x1b['); - onData('6n'); - onData(' after'); - - const output = registry.readSnapshot('terminal:split')?.output ?? ''; - expect(output).toContain('before '); - expect(output).toContain(' after'); - expect(output).not.toContain('\x1b'); - }); - - it('does not swallow payload after an unterminated non-query OSC', async () => { - // No OSC is a viable probe prefix — no OSC family is stripped any more — - // so the stripper must not hold one: otherwise a program killed mid - // title-write (or a file containing a bare `1B 5D`) would leave `pending` - // matching every later chunk and eat all subsequent rendered bytes - // forever. The visible text after the partial OSC must still reach the - // scrollback. - osPlatform.mockReturnValue('win32'); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:osc-partial', - workspaceCwd: '/workspace', - }); - - onData('\x1b]0;title'); - onData('visible'); - - const output = registry.readSnapshot('terminal:osc-partial')?.output ?? ''; - expect(output).toContain('visible'); - }); - - it('answers a DCS DECRQSS query once and keeps it out of the scrollback', async () => { - // Real DECRQSS is a DCS request (ESC P $ q ESC \), not the CSI - // `$q` form. xterm.js answers the DCS server-side; leaving the DCS bytes - // in the scrollback would let a reconnect replay re-answer them into the - // still-live shell, doubling the reply. The scrub must remove the DCS in - // both the complete and the chunk-split form, with the reply written back - // exactly once. - osPlatform.mockReturnValue('win32'); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:decrqss', - workspaceCwd: '/workspace', - }); - - const reply = '\x1bP1$r0m\x1b\\'; - - // Complete form. - onData('\x1bP$qm\x1b\\'); - await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); - expect(write).toHaveBeenCalledTimes(1); - - // Split form, split before the ST terminator: the DCS halves must be - // carried across chunks and stripped whole, answering once more. - write.mockClear(); - onData('\x1bP$q'); - onData('m\x1b\\'); - await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); - expect(write).toHaveBeenCalledTimes(1); - - // The DCS introducer itself may straddle a chunk boundary: a split after - // `ESC P` or after `ESC P $` must hold the introducer rather than leak the - // halves, which would reassemble in the buffer into a complete DECRQSS the - // client re-answers on replay. - write.mockClear(); - onData('\x1bP'); - onData('$qm\x1b\\'); - await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); - expect(write).toHaveBeenCalledTimes(1); - - write.mockClear(); - onData('\x1bP$'); - onData('qm\x1b\\'); - await vi.waitFor(() => expect(write).toHaveBeenCalledWith(reply)); - expect(write).toHaveBeenCalledTimes(1); - - const output = registry.readSnapshot('terminal:decrqss')?.output ?? ''; - expect(output).not.toContain('\x1b'); - }); - - it('keeps non-query escapes (SGR, cursor motion) in the scrollback', async () => { - // The finals/intermediates the scrub removes are all requests; SGR (`m`), - // erase (`J`) and cursor-position (`H`) are display/control content and - // must survive so the client still re-renders colours on reconnect. - osPlatform.mockReturnValue('win32'); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:nonquery', - workspaceCwd: '/workspace', - }); - - onData('\x1b[1;31mred\x1b[0m'); - onData('\x1b[2J'); - onData('\x1b[12;1H'); - - const output = registry.readSnapshot('terminal:nonquery')?.output ?? ''; - expect(output).toContain('\x1b[1;31mred\x1b[0m'); - expect(output).toContain('\x1b[2J'); - expect(output).toContain('\x1b[12;1H'); - }); - - it('resizes the headless responder with the client grid', async () => { - // The responder is constructed once at 80x24; a client resize that only - // touched session.pty would leave geometry-dependent replies (DSR cursor - // position) computed on the wrong grid. resize() must forward to it too. - osPlatform.mockReturnValue('win32'); - const resizeSpy = vi - .spyOn(Terminal.prototype, 'resize') - .mockImplementation(() => {}); - const registry = new WebTerminalRegistry(); - await registry.create({ - terminalId: 'terminal:responder-resize', - workspaceCwd: '/workspace', - }); - - expect(registry.resize('terminal:responder-resize', 120, 40)).toBe(true); - expect(resize).toHaveBeenCalledWith(120, 40); - expect(resizeSpy).toHaveBeenCalledWith(120, 40); - resizeSpy.mockRestore(); - }); - it('cancels an in-flight create released during the headless load', async () => { // `loadXtermHeadless` is the second suspension point after getPty(); a // release() landing during it must cancel the spawn, not leak a PTY the @@ -586,11 +423,7 @@ describe('WebTerminalRegistry', () => { // deliberate platform branch rather than an unconditional `true`. expect(spawn.mock.calls[0]?.[2]).toMatchObject({ useConptyDll: false }); - // The responder-plus-strip feature is win32-gated, and the POSIX side of - // that gate is pinned here: on Linux/macOS there is no server-side - // answerer, the browser is the only terminal that can reply, so a query - // must be live-forwarded untouched rather than stripped — and the headless - // responder must never be constructed. + // POSIX leaves every query, including primary DA, to the browser. onData('\x1b[c'); expect(registry.readSnapshot('terminal:posix-backend')?.output).toContain( '\x1b[c', @@ -928,6 +761,7 @@ describe('WebTerminalRegistry', () => { exited: true, exitCode: 3, workspaceCwd: '/workspace', + handlesPrimaryDa: true, }, ); }); diff --git a/packages/core/src/services/web-terminal-registry.ts b/packages/core/src/services/web-terminal-registry.ts index c4ca8db496d..1ad9d2d81bd 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -40,6 +40,7 @@ export interface WebTerminalSnapshot { exited: boolean; exitCode?: number; workspaceCwd: string; + handlesPrimaryDa?: boolean; } export interface CreateWebTerminalOptions { @@ -62,80 +63,6 @@ export const MAX_CONCURRENT_WEB_TERMINALS = 8; /** Reclaim a PTY session after this long with no connected listener. */ const IDLE_RECLAIM_MS = 15 * 60 * 1000; -/** - * Terminal queries a probing shell emits — requests, not display content — - * reach this registry as ordinary PTY output because the bundled ConPTY - * backend answers none of them itself (see shellExecutionService.ts). Recording - * one in the scrollback lets a reconnect that replays `session.buffer` make the - * client's xterm.js re-answer it and write the fresh reply back into the - * still-live shell's stdin — and leaving one in the live stream lets the - * browser answer it a second time, alongside the server-side responder. - * - * So only the families the pinned `@xterm/headless` 5.5.0 responder actually - * answers are matched, giving every probe exactly one answerer: Device - * Attributes (`c`, incl. the `>` intermediate), Device Status Report - * (`n`/`?n`), DECRQM (`$ p`) and DECRQSS (the DCS request - * `ESC P $ q ... ESC \`). `=`-DA3, XTVERSION (`> q`) and DECREQTPARM (`x`) are - * matched as well although no build answers them: their finals/intermediates - * are never display content, so removing them cannot drop rendered output, and - * leaving them in the replay would only re-parse a request nobody answers. - * - * The OSC 10/11/12/4 colour queries are deliberately NOT matched. The pinned - * responder answers none of them: `onData` carries DA/DSR/DECRQM/DECRQSS only, - * while `_setOrReportSpecialColor` reports on the internal `_onColor` emitter, - * which the headless `Terminal` does not expose (`term.onColor` is `undefined` - * on 5.5.0) and which nothing in this file subscribes to. Scrubbing that - * family therefore deleted it from the browser's stream too and left a probing - * program unanswered — where the browser's own xterm.js 6.0.0 - * `_handleColorEvent` is the answerer and answered it at the merge base. The - * colour queries a reconnect replay re-answers are part of the - * replay-suppression redesign tracked in #11734. - */ -// `no-control-regex` fires on the ESC/BEL bytes, which is the whole point here: -// these are terminal query sequences, not stray controls. -const TERMINAL_QUERY_SEQUENCE_RE = - // eslint-disable-next-line no-control-regex - /\x1b\[[0-9;>?=]*[cnx]|\x1b\[[0-9;?]*\$[pq]|\x1b\[>[0-9;]*q|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*\x1b\\/g; - -/** - * An incomplete trailing escape sequence — a query node-pty split across two - * chunks. It is carried to the next chunk and stripped as a whole rather than - * left to leak the partial probe into the scrollback — or, on the live path, - * to render the tail of a probe as text. Only viable query prefixes are held: - * a CSI introducer with its parameter run (the complete sequence is filtered by - * the regex above, so a non-query CSI still passes through), and DECRQSS, which - * arrives as DCS (`ESC P $ q ... ESC \`) — both the introducer (`\x1bP`, - * `\x1bP$`) and the full body are held back the same way. No OSC prefix is - * held: no OSC family is stripped any more. - */ -const PARTIAL_ESCAPE_SUFFIX_RE = - // eslint-disable-next-line no-control-regex - /(?:\x1b|\x1b\[[0-9;>?=$]*|\x1bP\$q(?:[^\x1b]|\x1b(?!\\))*|\x1bP\$?)$/; - -/** - * Stateful per-session stripper: `node-pty` may deliver a probe split across - * two chunks, so an incomplete trailing escape sequence is held back until the - * next chunk completes (or never, if the stream simply ends — a trailing - * partial probe is a few bytes of a query nobody answered, so dropping it is - * harmless). The hold is capped at MAX_HELD_ESCAPE_CHARS: a real query is a - * few dozen bytes, so anything longer is payload, not a split probe, and is - * flushed whole rather than swallowed. - */ -const MAX_HELD_ESCAPE_CHARS = 256; - -class TerminalQueryStripper { - private pending = ''; - - strip(data: string): string { - const combined = this.pending + data; - const partial = PARTIAL_ESCAPE_SUFFIX_RE.exec(combined); - const hold = partial !== null && partial[0].length <= MAX_HELD_ESCAPE_CHARS; - this.pending = hold ? partial![0] : ''; - const complete = hold ? combined.slice(0, partial!.index) : combined; - return complete.replace(TERMINAL_QUERY_SEQUENCE_RE, ''); - } -} - interface PtySession { pty: WebTerminalPty; workspaceCwd: string; @@ -314,14 +241,8 @@ export class WebTerminalRegistry { delete env['FORCE_COLOR']; delete env['npm_config_prefix']; const useBundledConpty = os.platform() === 'win32'; - // The bundled ConPTY backend answers no terminal queries itself (see - // shellExecutionService.ts), so a probing shell — PowerShell's startup DA - // probe under COMSPEC=powershell — would otherwise stall for its full - // timeout and leave its query bytes in the scrollback. Load a headless - // terminal up front (the import is cached, so only the first terminal pays - // it) to answer the probe server-side; handleData feeds it the PTY stream - // and strips the query bytes from the scrollback so a reconnect replay - // cannot make the client re-answer them into the still-live shell. + // PowerShell can probe primary DA before a browser attaches. The bundled + // backend needs a server answer; renderer-dependent queries stay client-owned. let queryTerminal: Terminal | undefined; if (useBundledConpty) { // `loadXtermHeadless` is a suspension point AFTER the getPty() re-checks @@ -343,20 +264,12 @@ export class WebTerminalRegistry { allowProposedApi: true, cols: 80, rows: 24, + scrollback: 0, logLevel: 'off', }); } - // No responder (headlessModule undefined): the query stays unanswered (a - // bounded ~2s stall), never injected. + // Without headless, the browser answers live DA; startup may time out. } - // Tie the stripper to the responder it complements. Without a responder - // the browser is the only terminal that can answer, so stripping here - // would delete a query nobody is left to answer — permanently, because - // loadXtermHeadless memoizes its rejection for the life of the process. - // Only strip when a responder is actually present to answer the queries. - const queryStripper = queryTerminal - ? new TerminalQueryStripper() - : undefined; let spawned: SpawnedWebTerminalPty; let proc: WebTerminalPty; let queryReplyDisposable: { dispose(): void } | undefined; @@ -373,11 +286,6 @@ export class WebTerminalRegistry { 0, session.unacknowledgedInputBytes - Buffer.byteLength(data), ); - // Feed the headless responder so a bundled-backend query is answered - // server-side (the browser is not guaranteed to be attached when the - // startup probe fires). Strip the query from the scrollback AND from - // what reaches the live listeners: the browser's xterm.js would also - // answer it, and a reconnect replay of `buffer` would re-emit it. if (queryTerminal) { try { queryTerminal.write(data); @@ -385,7 +293,7 @@ export class WebTerminalRegistry { // Terminal disposed mid-stream (release raced a trailing chunk). } } - let buffered = queryStripper ? queryStripper.strip(data) : data; + let buffered = data; if (Buffer.byteLength(buffered) > MAX_BUFFER_BYTES) { buffered = Buffer.from(buffered) .subarray(-MAX_BUFFER_BYTES) @@ -519,6 +427,8 @@ export class WebTerminalRegistry { }; if (queryTerminal) { queryReplyDisposable = queryTerminal.onData((reply) => { + // Only primary DA is independent of the browser's size, modes and theme. + if (reply !== '\x1b[?1;2c') return; try { proc.write(reply); } catch { @@ -527,6 +437,8 @@ export class WebTerminalRegistry { }); } } catch { + queryReplyDisposable?.dispose(); + queryTerminal?.dispose(); this.finishCreating(terminalId); return { error: 'Failed to spawn shell' }; } @@ -590,6 +502,7 @@ export class WebTerminalRegistry { output: session.buffer.join(''), exited: session.exited, workspaceCwd: session.workspaceCwd, + ...(session.queryTerminal ? { handlesPrimaryDa: true } : {}), ...(session.exitCode !== undefined ? { exitCode: session.exitCode } : {}), }; } @@ -621,11 +534,6 @@ export class WebTerminalRegistry { if (!session || session.exited) return false; try { session.pty.resize(cols, rows); - // Keep the headless query responder on the same grid the client renders: - // geometry-dependent replies (DSR cursor position, DECRQSS) must be - // computed against the browser's actual size, not the 80x24 it spawned - // with. - session.queryTerminal?.resize(cols, rows); return true; } catch { return false; diff --git a/packages/web-shell/client/components/terminal/TerminalPanel.test.tsx b/packages/web-shell/client/components/terminal/TerminalPanel.test.tsx index 25757f20747..6a7be661532 100644 --- a/packages/web-shell/client/components/terminal/TerminalPanel.test.tsx +++ b/packages/web-shell/client/components/terminal/TerminalPanel.test.tsx @@ -11,23 +11,46 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const workspace = vi.hoisted(() => ({ baseUrl: 'http://localhost' })); const fit = vi.hoisted(() => vi.fn()); -const terminal = vi.hoisted(() => ({ - options: {} as Record, - cols: 80, - rows: 24, - loadAddon: vi.fn(), - open: vi.fn(), - reset: vi.fn(), - write: vi.fn(), - writeln: vi.fn(), - focus: vi.fn(), - blur: vi.fn(), - refresh: vi.fn(), - dispose: vi.fn(), - onData: vi.fn((_listener: (data: string) => void) => ({ dispose: vi.fn() })), -})); +const { terminal, terminalInstances, createMockTerminal } = vi.hoisted(() => { + const createMockTerminal = () => ({ + options: {} as Record, + cols: 80, + rows: 24, + loadAddon: vi.fn(), + open: vi.fn(), + resize: vi.fn(), + write: vi.fn<(text: string, callback?: () => void) => void>(), + writeln: vi.fn(), + focus: vi.fn(), + blur: vi.fn(), + refresh: vi.fn(), + dispose: vi.fn(), + onData: vi.fn((_listener: (data: string) => void) => ({ + dispose: vi.fn(), + })), + parser: { + registerCsiHandler: vi.fn( + (_id: { final: string }, _handler: () => boolean) => ({ + dispose: vi.fn(), + }), + ), + }, + }); + return { + terminal: createMockTerminal(), + terminalInstances: [] as Array>, + createMockTerminal, + }; +}); -vi.mock('@xterm/xterm', () => ({ Terminal: vi.fn(() => terminal) })); +vi.mock('@xterm/xterm', () => ({ + Terminal: vi.fn((options: Record) => { + const instance = terminalInstances.length ? createMockTerminal() : terminal; + instance.options = options; + terminalInstances.push(instance); + return instance; + }), +})); vi.mock('@xterm/addon-fit', () => ({ FitAddon: vi.fn(() => ({ fit })), })); @@ -69,7 +92,7 @@ class FakeWebSocket { onerror: (() => void) | null = null; onclose: ((event: CloseEvent) => void) | null = null; readonly send = vi.fn(); - readonly close = vi.fn(() => { + readonly close = vi.fn((_code?: number, _reason?: string) => { this.readyState = FakeWebSocket.CLOSED; }); @@ -102,6 +125,7 @@ describe('TerminalPanel', () => { beforeEach(() => { vi.useFakeTimers(); vi.clearAllMocks(); + terminalInstances.length = 0; workspace.baseUrl = 'http://localhost'; FakeWebSocket.instances.length = 0; vi.stubGlobal('WebSocket', FakeWebSocket); @@ -144,6 +168,14 @@ describe('TerminalPanel', () => { it('keeps binary PTY output distinct from text control frames', async () => { const ws = render(); act(() => ws.open()); + act(() => { + ws.message( + '\x00{"type":"snapshot","replay":true,"handlesPrimaryDa":false}', + ); + ws.message(new ArrayBuffer(0)); + terminalInstances[1]!.write.mock.calls[0]?.[1]?.(); + }); + const restored = terminalInstances[1]!; await act(async () => { ws.message( @@ -153,14 +185,14 @@ describe('TerminalPanel', () => { await Promise.resolve(); }); - expect(terminal.write).toHaveBeenCalledWith( + expect(restored.write).toHaveBeenCalledWith( '\x00{"type":"exit","exitCode":0}', ); - expect(terminal.writeln).toHaveBeenCalledWith( + expect(restored.writeln).toHaveBeenCalledWith( expect.stringContaining('[Error: denied]'), ); - const handleInput = terminal.onData.mock.calls.at(-1)?.[0]; + const handleInput = restored.onData.mock.calls.at(-1)?.[0]; ws.send.mockClear(); act(() => handleInput?.('\x00{"type":"release"}')); const sent = ws.send.mock.calls[0]?.[0]; @@ -192,20 +224,95 @@ describe('TerminalPanel', () => { expect(FakeWebSocket.instances).toHaveLength(1); }); - it('reconnects transient failures without clearing the notice early', async () => { + it('keeps the old terminal interactive until reconnect replay finishes', async () => { const ws = render(); act(() => ws.open()); - terminal.reset.mockClear(); act(() => ws.closeWith(1006)); expect(terminal.writeln).toHaveBeenCalledWith( expect.stringContaining('Connection lost'), ); - expect(terminal.reset).not.toHaveBeenCalled(); + expect(terminal.dispose).not.toHaveBeenCalled(); await act(async () => vi.advanceTimersByTimeAsync(500)); expect(FakeWebSocket.instances).toHaveLength(2); act(() => FakeWebSocket.instances[1]!.open()); - expect(terminal.reset).toHaveBeenCalledOnce(); + const reconnected = FakeWebSocket.instances[1]!; + act(() => { + reconnected.message( + '\x00{"type":"snapshot","replay":true,"handlesPrimaryDa":true}', + ); + reconnected.message( + new TextEncoder().encode('history\x1b]11;?\x07').buffer, + ); + reconnected.message(new TextEncoder().encode('live\x1b]11;?\x07').buffer); + }); + const restored = terminalInstances[1]!; + expect(restored.write.mock.calls.map(([text]) => text)).toEqual([ + 'history\x1b]11;?\x07', + 'live\x1b]11;?\x07', + ]); + expect(restored.onData).not.toHaveBeenCalled(); + const oldInput = terminal.onData.mock.calls[0]![0]; + reconnected.send.mockClear(); + act(() => oldInput('k')); + expect(new TextDecoder().decode(reconnected.send.mock.calls[0]![0])).toBe( + 'k', + ); + expect(terminal.dispose).not.toHaveBeenCalled(); + + act(() => restored.write.mock.calls[0]![1]!()); + expect(terminal.dispose).toHaveBeenCalledOnce(); + expect(restored.onData).toHaveBeenCalledOnce(); + expect(restored.parser.registerCsiHandler.mock.calls[0]![1]()).toBe(true); + act(() => restored.onData.mock.calls[0]![0]('live reply')); + expect( + new TextDecoder().decode(reconnected.send.mock.calls.at(-1)![0]), + ).toBe('live reply'); + }); + + it('allows first-connection queries and leaves DA to the browser without a server responder', () => { + const ws = render(); + act(() => { + ws.open(); + ws.message( + '\x00{"type":"snapshot","replay":false,"handlesPrimaryDa":false}', + ); + ws.message(new TextEncoder().encode('\x1b[c').buffer); + }); + const next = terminalInstances[1]!; + expect(next.onData).toHaveBeenCalledOnce(); + expect(next.parser.registerCsiHandler.mock.calls[0]![1]()).toBe(false); + act(() => next.onData.mock.calls[0]![0]('\x1b[?1;2c')); + expect(new TextDecoder().decode(ws.send.mock.calls.at(-1)![0])).toBe( + '\x1b[?1;2c', + ); + }); + + it('disposes an interrupted replay and ignores its late write callback', () => { + const ws = render(); + act(() => { + ws.open(); + ws.message( + '\x00{"type":"snapshot","replay":true,"handlesPrimaryDa":true}', + ); + ws.message(new TextEncoder().encode('history').buffer); + ws.closeWith(1006); + }); + const next = terminalInstances[1]!; + expect(next.dispose).toHaveBeenCalledOnce(); + act(() => next.write.mock.calls[0]![1]!()); + expect(terminal.dispose).not.toHaveBeenCalled(); + expect(next.onData).not.toHaveBeenCalled(); + }); + + it('rejects an old server that does not mark the snapshot boundary', () => { + const ws = render(); + act(() => { + ws.open(); + ws.message(new TextEncoder().encode('unmarked history').buffer); + }); + expect(ws.close).toHaveBeenCalledWith(4002, 'Terminal protocol mismatch'); + expect(terminal.write).not.toHaveBeenCalled(); }); it('sends an explicit release control when the tab is closed', () => { @@ -225,7 +332,7 @@ describe('TerminalPanel', () => { ); expect(FakeWebSocket.instances[0]?.url).toBe( - 'ws://localhost/base/terminal?terminalId=terminal%3Adetached&cwd=%2Fworkspace&release=1', + 'ws://localhost/base/terminal?terminalId=terminal%3Adetached&replay=1&cwd=%2Fworkspace&release=1', ); }); @@ -316,6 +423,7 @@ describe('TerminalPanel', () => { expect(url.pathname).toBe('/terminal'); expect(url.searchParams.get('terminalId')).toBe('terminal:one'); expect(url.searchParams.get('cwd')).toBe('/workspace'); + expect(url.searchParams.get('replay')).toBe('1'); act(() => ws.open()); expect(ws.send).toHaveBeenCalledWith( diff --git a/packages/web-shell/client/components/terminal/TerminalPanel.tsx b/packages/web-shell/client/components/terminal/TerminalPanel.tsx index 986c6d79e73..d5901a870cb 100644 --- a/packages/web-shell/client/components/terminal/TerminalPanel.tsx +++ b/packages/web-shell/client/components/terminal/TerminalPanel.tsx @@ -81,6 +81,7 @@ function buildWsUrl( ); url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:'; url.searchParams.set('terminalId', terminalId); + url.searchParams.set('replay', '1'); if (cwd) url.searchParams.set('cwd', cwd); if (release) url.searchParams.set('release', '1'); return url.toString(); @@ -141,15 +142,25 @@ export function TerminalPanel({ } if (!containerRef.current) return; - const term = new Terminal({ - cursorBlink: true, - fontSize: 13, - fontFamily: 'Menlo, Monaco, "Courier New", monospace', - theme: xtermTheme(theme), - }); - const fit = new FitAddon(); - term.loadAddon(fit); - term.open(containerRef.current); + function createTerminal(handlesPrimaryDa = false) { + const term = new Terminal({ + allowProposedApi: true, + cursorBlink: true, + fontSize: 13, + fontFamily: 'Menlo, Monaco, "Courier New", monospace', + theme: termRef.current?.options.theme ?? xtermTheme(theme), + }); + const fit = new FitAddon(); + const host = document.createElement('div'); + host.style.height = '100%'; + term.loadAddon(fit); + term.parser.registerCsiHandler({ final: 'c' }, () => handlesPrimaryDa); + term.open(host); + return { term, fit, host }; + } + + let { term, fit, host } = createTerminal(); + containerRef.current.replaceChildren(host); termRef.current = term; fitRef.current = fit; @@ -190,6 +201,48 @@ export function TerminalPanel({ let releaseAttempts = 0; let releaseRetryTimer: ReturnType | undefined; let ended = false; + let awaitingSnapshot = true; + let snapshot: { replay: boolean; handlesPrimaryDa: boolean } | undefined; + let restoring: ReturnType | undefined; + + function sendInput(data: string) { + if (!activeRef.current) return; + const ws = wsRef.current; + if (ws?.readyState === WebSocket.OPEN) { + ws.send(new TextEncoder().encode(data)); + } + } + let disposable = term.onData(sendInput); + + function restoreSnapshot(text: string, ws: WebSocket) { + const next = createTerminal(snapshot!.handlesPrimaryDa); + const replay = snapshot!.replay; + snapshot = undefined; + awaitingSnapshot = false; + restoring = next; + next.term.resize(term.cols, term.rows); + // A new PTY's buffered output has never been answered. Reconnect history has. + const liveInput = replay ? undefined : next.term.onData(sendInput); + next.term.write(text, () => { + if (disposed || wsRef.current !== ws || restoring !== next) { + liveInput?.dispose(); + return; + } + const previous = term; + next.term.options.theme = term.options.theme; + disposable.dispose(); + ({ term, fit, host } = next); + termRef.current = term; + fitRef.current = fit; + disposable = liveInput ?? term.onData(sendInput); + restoring = undefined; + containerRef.current!.replaceChildren(host); + previous.dispose(); + fit.fit(); + sendCurrentResize(); + if (activeRef.current) term.focus(); + }); + } function handleControl(raw: string): boolean { try { @@ -197,12 +250,26 @@ export function TerminalPanel({ type?: unknown; exitCode?: unknown; message?: unknown; + replay?: unknown; + handlesPrimaryDa?: unknown; }; - if (ctrl.type === 'exit') { + if ( + ctrl.type === 'snapshot' && + awaitingSnapshot && + !snapshot && + typeof ctrl.replay === 'boolean' && + typeof ctrl.handlesPrimaryDa === 'boolean' + ) { + snapshot = { + replay: ctrl.replay, + handlesPrimaryDa: ctrl.handlesPrimaryDa, + }; + return true; + } else if (ctrl.type === 'exit') { ended = true; const exitCode = typeof ctrl.exitCode === 'number' ? String(ctrl.exitCode) : '?'; - term.writeln( + (restoring?.term ?? term).writeln( `\r\n\x1b[33m[${t('terminal.notice.exited', { exitCode })}]\x1b[0m`, ); return true; @@ -211,7 +278,7 @@ export function TerminalPanel({ typeof ctrl.message === 'string' ? ctrl.message : t('terminal.notice.unknownError'); - term.writeln( + (restoring?.term ?? term).writeln( `\r\n\x1b[31m[${t('terminal.notice.error', { message })}]\x1b[0m`, ); return true; @@ -228,12 +295,23 @@ export function TerminalPanel({ } } - function handleMessage(event: MessageEvent) { - if (disposed) return; + function handleMessage(event: MessageEvent, ws: WebSocket) { + if (disposed || wsRef.current !== ws) return; if (typeof event.data === 'string') { writeMessage(event.data); } else { - term.write(new TextDecoder().decode(event.data as ArrayBuffer)); + const text = new TextDecoder().decode(event.data as ArrayBuffer); + if (snapshot) { + restoreSnapshot(text, ws); + } else if (awaitingSnapshot) { + ended = true; + term.writeln( + '\r\nTerminal protocol changed; restart the daemon and reload this page.', + ); + ws.close(4002, 'Terminal protocol mismatch'); + } else { + (restoring?.term ?? term).write(text); + } } } @@ -257,9 +335,8 @@ export function TerminalPanel({ ws.close(); return; } - // Keep the reconnect notice visible until a connection succeeds; the - // backend immediately replays the complete scrollback after this. - term.reset(); + awaitingSnapshot = true; + snapshot = undefined; reconnectDelay = RECONNECT_INITIAL_MS; lostNoticeWritten = false; if (ws.readyState === WebSocket.OPEN) { @@ -274,7 +351,7 @@ export function TerminalPanel({ } }; - ws.onmessage = releaseOnly ? null : handleMessage; + ws.onmessage = releaseOnly ? null : (event) => handleMessage(event, ws); ws.onerror = () => { // Errors surface through close; reconnect is handled there. @@ -292,6 +369,8 @@ export function TerminalPanel({ } if (disposed || releaseRequested) return; if (ended || NON_RETRYABLE_CLOSE_CODES.has(event.code)) return; + restoring?.term.dispose(); + restoring = undefined; if (!lostNoticeWritten) { term.writeln( `\r\n\x1b[33m[${t('terminal.notice.reconnecting')}]\x1b[0m`, @@ -319,15 +398,6 @@ export function TerminalPanel({ }; releaseCallbacks.set(terminalId, release); - // xterm → WebSocket (raw keystrokes = stdin, control = resize) - const disposable = term.onData((data: string) => { - if (!activeRef.current) return; - const ws = wsRef.current; - if (ws && ws.readyState === WebSocket.OPEN) { - ws.send(new TextEncoder().encode(data)); - } - }); - // Resize observer → send new size const resizeObserver = new ResizeObserver(() => { if (!fitRef.current || !termRef.current) return; @@ -356,6 +426,7 @@ export function TerminalPanel({ clearTimeout(resizeTimeout); resizeObserver.disconnect(); disposable.dispose(); + restoring?.term.dispose(); if ( !releaseRequested || wsRef.current?.readyState !== WebSocket.CONNECTING