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/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/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/conpty-host.test.ts b/packages/core/src/services/conpty-host.test.ts new file mode 100644 index 00000000000..dd83a4d7435 --- /dev/null +++ b/packages/core/src/services/conpty-host.test.ts @@ -0,0 +1,156 @@ +/** + * @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 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 }; + + 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', () => { + // 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 86bb8d20fc9..669d58ed728 100644 --- a/packages/core/src/services/web-terminal-registry.test.ts +++ b/packages/core/src/services/web-terminal-registry.test.ts @@ -5,18 +5,26 @@ */ 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 })); -// Only conpty-host reads os.platform(); killPtyTree branches on -// process.platform, so this steers the ConPTY release without touching it. +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. // 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 +51,44 @@ 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; + // Capture this session's spy by value: the describe-scope `disposeData` + // variable is reassigned every test, and a deferred exit-time release + // from a previous session may fire during a later test — it must hit + // its own spy, not the current one. Same reason the detach guard + // checks listener identity before clearing the shared `onData`. + const disposeDataSpy = disposeData; + return { + // Model node-pty's disposable detaching the listener, so a test can + // tell a synchronous dispose apart from the deferred one. + dispose: () => { + if (onData === listener) onData = () => {}; + disposeDataSpy(); + }, + }; + }), + onExit: vi.fn( + (listener: (e: { exitCode: number; signal?: number }) => void) => { + onExit = listener; + return { dispose: disposeExit }; + }, + ), + }); + beforeEach(() => { vi.clearAllMocks(); write = vi.fn(); @@ -54,42 +100,9 @@ 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; - // Capture this session's spy by value: the describe-scope `disposeData` - // variable is reassigned every test, and a deferred exit-time release - // from a previous session may fire during a later test — it must hit - // its own spy, not the current one. Same reason the detach guard - // checks listener identity before clearing the shared `onData`. - const disposeDataSpy = disposeData; - return { - // Model node-pty's disposable detaching the listener, so a test can - // tell a synchronous dispose apart from the deferred one. - dispose: () => { - if (onData === listener) onData = () => {}; - disposeDataSpy(); - }, - }; - }), - onExit: vi.fn((listener) => { - onExit = listener; - return { dispose: disposeExit }; - }), - }); + spawn.mockImplementation(() => createSpawnedPty()); getPty.mockResolvedValue({ module: { spawn }, name: 'node-pty' }); + loadXtermHeadless.mockResolvedValue({ Terminal }); }); afterEach(() => { @@ -213,6 +226,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( @@ -227,6 +243,280 @@ 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('preserves the PTY stream and answers only primary DA on Windows', async () => { + osPlatform.mockReturnValue('win32'); + const registry = new WebTerminalRegistry(); + await registry.create({ + terminalId: 'terminal:queries', + workspaceCwd: '/workspace', + }); + 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).toHaveBeenCalledExactlyOnceWith('\x1b[?1;2c'); + }); + expect(received).toEqual(chunks); + expect(registry.readSnapshot('terminal:queries')).toMatchObject({ + output: chunks.join(''), + handlesPrimaryDa: true, + }); + registry.dispose(); + }); + + 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(); + 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( + registry.readSnapshot('terminal:no-responder')?.handlesPrimaryDa, + ).not.toBe(true); + expect(write).not.toHaveBeenCalled(); + registry.dispose(); + }); + + 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('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(); + + 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 }); + + // POSIX leaves every query, including primary DA, to the browser. + 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 () => { + // 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({ @@ -471,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 e0f48f10021..1ad9d2d81bd 100644 --- a/packages/core/src/services/web-terminal-registry.ts +++ b/packages/core/src/services/web-terminal-registry.ts @@ -5,7 +5,10 @@ */ 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, @@ -37,6 +40,7 @@ export interface WebTerminalSnapshot { exited: boolean; exitCode?: number; workspaceCwd: string; + handlesPrimaryDa?: boolean; } export interface CreateWebTerminalOptions { @@ -72,6 +76,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 @@ -234,8 +240,39 @@ export class WebTerminalRegistry { delete env['NO_COLOR']; delete env['FORCE_COLOR']; delete env['npm_config_prefix']; + const useBundledConpty = os.platform() === 'win32'; + // 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 + // 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, + scrollback: 0, + logLevel: 'off', + }); + } + // Without headless, the browser answers live DA; startup may time out. + } 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; @@ -249,14 +286,25 @@ 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); + if (queryTerminal) { + try { + queryTerminal.write(data); + } catch { + // Terminal disposed mid-stream (release raced a trailing chunk). + } + } + let buffered = 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 @@ -266,7 +314,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; @@ -280,8 +328,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 +348,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 +361,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 = { @@ -355,7 +425,20 @@ export class WebTerminalRegistry { releaseConPtyHost(spawned); }, }; + 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 { + // A reply racing shell exit finds a dead PTY — drop it. + } + }); + } } catch { + queryReplyDisposable?.dispose(); + queryTerminal?.dispose(); this.finishCreating(terminalId); return { error: 'Failed to spawn shell' }; } @@ -371,6 +454,8 @@ export class WebTerminalRegistry { exitListeners: new Set(), dataDisposable, exitDisposable, + queryTerminal, + queryReplyDisposable, ptyResourcesReleased: false, }; sessionRef.current = session; @@ -417,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 } : {}), }; } @@ -520,11 +606,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 @@ -542,6 +631,8 @@ export class WebTerminalRegistry { session.ptyResourcesReleased = true; session.dataDisposable?.dispose(); session.exitDisposable?.dispose(); + session.queryReplyDisposable?.dispose(); + session.queryTerminal?.dispose(); session.pty.releaseHost?.(); } 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 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',