From 94c8b9b68e6577f55246ce0916890fcee75a4b72 Mon Sep 17 00:00:00 2001 From: zhanba Date: Wed, 2 Sep 2026 22:59:45 +0800 Subject: [PATCH 01/12] feat(vscode): promote timed out commands to background jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep long-running commands alive and interactive after foreground timeout by adopting the original PTY into a background terminal, while preserving output and UI metadata. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../vscode-webui-bridge/types/execution.ts | 4 + packages/tools/src/execute-command.ts | 4 +- .../chat/lib/batched-tool-call-adapters.ts | 3 + .../features/chat/lib/tool-call-life-cycle.ts | 3 + .../chat/lib/use-background-job-display.tsx | 4 +- .../__tests__/execute-command.test.tsx | 33 + .../tools/components/execute-command.tsx | 9 +- .../src/lib/vscode-running-task-adaptor.ts | 3 + .../__test__/execute-command-with-pty.test.ts | 96 ++- .../terminal/__test__/pty-process.test.ts | 113 ++++ .../terminal/__test__/pty-terminal.test.ts | 93 +++ .../terminal/__test__/terminal-job.test.ts | 484 ++++++------- .../terminal/execute-command-with-pty.ts | 178 ++--- .../src/integrations/terminal/pty-process.ts | 227 +++++++ .../src/integrations/terminal/pty-terminal.ts | 70 ++ .../src/integrations/terminal/terminal-job.ts | 633 ++++++++++-------- .../tools/__test__/execute-command.test.ts | 102 +++ packages/vscode/src/tools/execute-command.ts | 44 +- 18 files changed, 1450 insertions(+), 653 deletions(-) create mode 100644 packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts create mode 100644 packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts create mode 100644 packages/vscode/src/integrations/terminal/pty-process.ts create mode 100644 packages/vscode/src/integrations/terminal/pty-terminal.ts diff --git a/packages/common/src/vscode-webui-bridge/types/execution.ts b/packages/common/src/vscode-webui-bridge/types/execution.ts index fd1234460d..33a024277e 100644 --- a/packages/common/src/vscode-webui-bridge/types/execution.ts +++ b/packages/common/src/vscode-webui-bridge/types/execution.ts @@ -13,6 +13,10 @@ export type ExecuteCommandResult = { status: "idle" | "running" | "completed"; isTruncated: boolean; error?: string; // Optional error message if the execution aborted / failed + _meta?: { + backgroundJobId: string; + outputFile?: string; + }; }; export type SaveCheckpointOptions = { diff --git a/packages/tools/src/execute-command.ts b/packages/tools/src/execute-command.ts index 06780c07e4..751415fa0d 100644 --- a/packages/tools/src/execute-command.ts +++ b/packages/tools/src/execute-command.ts @@ -59,7 +59,7 @@ Before executing the command, please follow these steps: Usage notes: - The command argument is required. ${backgroundUsageNotes} -- For foreground commands, you can specify an optional timeout in seconds (up to 300s). If not specified, commands will timeout after ${ExecuteCommandDefaultTimeoutSec}s. +- For foreground commands, you can specify an optional timeout in seconds (up to 300s). If not specified, the foreground wait is ${ExecuteCommandDefaultTimeoutSec}s. On supported interactive hosts, a command still running at that point is moved to a background terminal instead of being stopped; its result includes the background job metadata. Other hosts stop the command and report a timeout. - If the output exceeds 30000 characters, output will be truncated before being returned to you. - When issuing multiple commands: - If the commands are independent and can run in parallel, make multiple executeCommand tool calls in a single message. For example, if you need to run "git status" and "git diff", send a single message with two executeCommand tool calls in parallel. @@ -189,7 +189,7 @@ Important: .max(60 * 5) .optional() .describe( - `Optional timeout in seconds, max 300 seconds. By default the timeout is ${ExecuteCommandDefaultTimeoutSec} seconds.`, + `Optional foreground wait in seconds, max 300 seconds. The default is ${ExecuteCommandDefaultTimeoutSec} seconds. Supported interactive hosts move a command that is still running to the background.`, ), }), outputSchema: z.object({ diff --git a/packages/vscode-webui/src/features/chat/lib/batched-tool-call-adapters.ts b/packages/vscode-webui/src/features/chat/lib/batched-tool-call-adapters.ts index f13e14a7c5..7e0e1fe90c 100644 --- a/packages/vscode-webui/src/features/chat/lib/batched-tool-call-adapters.ts +++ b/packages/vscode-webui/src/features/chat/lib/batched-tool-call-adapters.ts @@ -192,6 +192,9 @@ export function createSubtaskBatchedToolCall({ if (output.error) { toolOutput.error = output.error; } + if (output._meta) { + toolOutput._meta = output._meta; + } addToolOutput({ tool: toolCall.toolName, toolCallId: toolCall.toolCallId, diff --git a/packages/vscode-webui/src/features/chat/lib/tool-call-life-cycle.ts b/packages/vscode-webui/src/features/chat/lib/tool-call-life-cycle.ts index 6ab985006d..e37bda0511 100644 --- a/packages/vscode-webui/src/features/chat/lib/tool-call-life-cycle.ts +++ b/packages/vscode-webui/src/features/chat/lib/tool-call-life-cycle.ts @@ -343,6 +343,9 @@ export class ManagedToolCallLifeCycle if (output.error) { result.error = output.error; } + if (output._meta) { + result._meta = output._meta; + } this.transitTo("execute:streaming", { type: "complete", result, diff --git a/packages/vscode-webui/src/features/chat/lib/use-background-job-display.tsx b/packages/vscode-webui/src/features/chat/lib/use-background-job-display.tsx index 5d604436b1..4400294811 100644 --- a/packages/vscode-webui/src/features/chat/lib/use-background-job-display.tsx +++ b/packages/vscode-webui/src/features/chat/lib/use-background-job-display.tsx @@ -15,7 +15,6 @@ export const useBackgroundJobDisplay = (messages: Message[]) => { if ( p.type === "tool-executeCommand" && p.state !== "input-streaming" && - p.input?.background === true && p.output?._meta?.backgroundJobId ) { ids.add(p.output._meta.backgroundJobId); @@ -32,8 +31,7 @@ export const useBackgroundJobDisplay = (messages: Message[]) => { if ( p.type === "tool-executeCommand" && p.state !== "input-streaming" && - p.input?.background === true && - p.input.command && + p.input?.command && p.output?._meta?.backgroundJobId ) { map.set(p.output._meta.backgroundJobId, { diff --git a/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx b/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx index d690d3b85c..689f7b9876 100644 --- a/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx +++ b/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx @@ -88,4 +88,37 @@ describe("executeCommandTool", () => { expect(panel.dataset.jobId).toBe("bgjob-cmd-test"); expect(panel.dataset.outputFile).toBe("/tmp/bgjob-cmd-test.log"); }); + + it("shows the job panel when a foreground command is promoted", () => { + render( + , + ); + + const panel = screen.getByTestId("background-job-panel"); + expect(panel.dataset.jobId).toBe("bgjob-cmd-promoted"); + expect(panel.dataset.outputFile).toBe("/tmp/bgjob-cmd-promoted.log"); + expect(screen.getByText("toolInvocation.backgroundExecute")).toBeTruthy(); + }); }); diff --git a/packages/vscode-webui/src/features/tools/components/execute-command.tsx b/packages/vscode-webui/src/features/tools/components/execute-command.tsx index fa131e840e..d2d39d32f2 100644 --- a/packages/vscode-webui/src/features/tools/components/execute-command.tsx +++ b/packages/vscode-webui/src/features/tools/components/execute-command.tsx @@ -28,13 +28,16 @@ export const executeCommandTool: React.FC> = ({ }, [lifecycle.abort]); const { cwd, command, background } = tool.input || {}; + const backgroundJobMetadata = + tool.state === "output-available" ? tool.output._meta : undefined; + const runsInBackground = background || Boolean(backgroundJobMetadata); const cwdNode = cwd ? ( {" "} {t("toolInvocation.in")} {cwd} ) : null; - const text = background + const text = runsInBackground ? t("toolInvocation.backgroundExecute") : t("toolInvocation.executeCommand"); const title = ( @@ -53,9 +56,7 @@ export const executeCommandTool: React.FC> = ({ throw new Error("Unexpected streaming result for executeCommand tool"); } - if (background) { - const backgroundJobMetadata = - tool.state === "output-available" ? tool.output._meta : undefined; + if (runsInBackground) { const availableCommand = tool.state === "input-available" || tool.state === "output-available" ? tool.input.command diff --git a/packages/vscode-webui/src/lib/vscode-running-task-adaptor.ts b/packages/vscode-webui/src/lib/vscode-running-task-adaptor.ts index 7e7b306edf..56cf2fc21b 100644 --- a/packages/vscode-webui/src/lib/vscode-running-task-adaptor.ts +++ b/packages/vscode-webui/src/lib/vscode-running-task-adaptor.ts @@ -241,6 +241,9 @@ function waitForExecuteCommandOutput( } else if (reason === "aborted") { result.error = "Aborted by background task runner"; } + if (value._meta) { + result._meta = value._meta; + } resolve(result); }; diff --git a/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts b/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts index 83c05cb56f..5e61379944 100644 --- a/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts @@ -1,42 +1,43 @@ import * as assert from "node:assert"; import { describe, it } from "mocha"; +import proxyquire from "proxyquire"; +import sinon from "sinon"; import { buildPtyEnv, buildPtyShellCommand, - toNonInteractivePtyCommand, + executeCommandWithPty, + getNodePtyModulePaths, } from "../execute-command-with-pty"; describe("execute-command-with-pty", () => { - it("should wrap command to detach stdin on posix", () => { - const wrapped = toNonInteractivePtyCommand("git pull", "darwin"); - assert.ok(wrapped.includes("git pull")); - assert.ok(wrapped.includes(" { + assert.deepStrictEqual(getNodePtyModulePaths("/vscode/app"), [ + "/vscode/app/node_modules.asar/node-pty", + "/vscode/app/node_modules/node-pty", + ]); }); - it("should not wrap command on windows", () => { - const wrapped = toNonInteractivePtyCommand("git pull", "win32"); - assert.strictEqual(wrapped, "git pull"); - }); + it("spawns with VS Code's packaged node-pty", async function () { + if (process.platform === "win32") this.skip(); - it("should build shell command with stdin-detached wrapper on posix", () => { - const shellCommand = buildPtyShellCommand("echo hello", "darwin"); - assert.ok(shellCommand, "Expected a shell command to be built"); - assert.ok( - shellCommand?.args.at(-1)?.includes(" { - const shellCommand = buildPtyShellCommand("echo hello", "win32"); + it("builds an interactive shell command without detaching stdin", () => { + const shellCommand = buildPtyShellCommand("echo hello"); assert.ok(shellCommand, "Expected a shell command to be built"); - assert.ok( - !shellCommand?.args.at(-1)?.includes(" { + it("enforces terminal environment precedence", () => { const env = buildPtyEnv({ GIT_TERMINAL_PROMPT: "1", GCM_INTERACTIVE: "always", @@ -46,4 +47,53 @@ describe("execute-command-with-pty", () => { assert.strictEqual(env.GCM_INTERACTIVE, "never"); assert.strictEqual(env.GIT_EDITOR, "true"); }); + + it("returns the running pty instead of killing it on timeout", async () => { + const clock = sinon.useFakeTimers(); + let dataListener: ((data: string) => void) | undefined; + let exitListener: ((event: { exitCode: number }) => void) | undefined; + const ptyProcess = { + kill: sinon.stub(), + onData: (listener: (data: string) => void) => { + dataListener = listener; + return { dispose: sinon.stub() }; + }, + onExit: (listener: (event: { exitCode: number }) => void) => { + exitListener = listener; + return { dispose: sinon.stub() }; + }, + }; + const spawn = sinon.stub().resolves(ptyProcess); + const { executeCommandWithPty } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../execute-command-with-pty", { + "./pty-process": { + PtyProcess: { spawn }, + }, + }) as typeof import("../execute-command-with-pty"); + + try { + const resultPromise = executeCommandWithPty({ + command: "sleep 10", + cwd: "/tmp", + timeout: 1, + }); + await Promise.resolve(); + dataListener?.("started\n"); + await clock.tickAsync(1_000); + const result = await resultPromise; + + assert.strictEqual(result.type, "timedOut"); + assert.strictEqual( + result.type === "timedOut" ? result.ptyProcess : undefined, + ptyProcess, + ); + assert.strictEqual(result.output, "started\n"); + assert.strictEqual(ptyProcess.kill.callCount, 0); + assert.ok(exitListener); + } finally { + clock.restore(); + } + }); }); diff --git a/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts new file mode 100644 index 0000000000..427f0e3c93 --- /dev/null +++ b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts @@ -0,0 +1,113 @@ +import * as assert from "node:assert"; +import { describe, it } from "mocha"; +import proxyquire from "proxyquire"; +import sinon from "sinon"; + +interface FakePty { + onData(listener: (data: string) => void): void; + onExit(listener: (event: { exitCode: number }) => void): void; + write(data: string): void; + resize(columns: number, rows: number): void; + kill(signal?: string): void; +} + +function createHarness(kill = sinon.stub()) { + let dataListener: ((data: string) => void) | undefined; + let exitListener: ((event: { exitCode: number }) => void) | undefined; + const fakePty: FakePty = { + onData: (listener) => { + dataListener = listener; + }, + onExit: (listener) => { + exitListener = listener; + }, + write: sinon.stub(), + resize: sinon.stub(), + kill, + }; + const { PtyProcess } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../pty-process", { + vscode: { + env: { appRoot: "/app" }, + Uri: { + file: (path: string) => ({ path, toString: () => path }), + joinPath: (base: { path: string }, ...paths: string[]) => ({ + toString: () => [base.path, ...paths].join("/"), + }), + }, + }, + "@getpochi/common": { + getLogger: () => ({ + debug: sinon.stub(), + warn: sinon.stub(), + }), + }, + }) as typeof import("../pty-process"); + const ProcessConstructor = PtyProcess as unknown as new ( + process: FakePty, + ) => import("../pty-process").PtyProcess; + const ptyProcess = new ProcessConstructor(fakePty); + return { + data: (chunk: string) => dataListener?.(chunk), + exit: (exitCode: number) => exitListener?.({ exitCode }), + kill, + ptyProcess, + }; +} + +describe("PtyProcess", () => { + it("reports exit after node-pty delivers output preceding socket close", () => { + const harness = createHarness(); + const events: string[] = []; + harness.ptyProcess.onData((data: string) => events.push(`data:${data}`)); + harness.ptyProcess.onExit(({ exitCode }: { exitCode: number }) => + events.push(`exit:${exitCode}`), + ); + + harness.data("trailing output\n"); + harness.exit(0); + + assert.deepStrictEqual(events, ["data:trailing output\n", "exit:0"]); + }); + + it("allows late exit delivery to be cancelled", async () => { + const harness = createHarness(); + const exits: number[] = []; + harness.exit(0); + const subscription = harness.ptyProcess.onExit( + ({ exitCode }: { exitCode: number }) => exits.push(exitCode), + ); + + subscription.dispose(); + await Promise.resolve(); + assert.deepStrictEqual(exits, []); + }); + + it("escalates SIGTERM to SIGKILL after the grace period", async () => { + const clock = sinon.useFakeTimers(); + try { + const harness = createHarness(); + harness.ptyProcess.kill(); + assert.deepStrictEqual(harness.kill.args, [["SIGTERM"]]); + + await clock.tickAsync(2_000); + assert.deepStrictEqual(harness.kill.args, [["SIGTERM"], ["SIGKILL"]]); + harness.exit(137); + } finally { + clock.restore(); + } + }); + + it("catches kill races and allows a repeated stop to hard-kill", () => { + const kill = sinon.stub(); + kill.onFirstCall().throws(new Error("already exited")); + const harness = createHarness(kill); + + assert.doesNotThrow(() => harness.ptyProcess.kill()); + assert.doesNotThrow(() => harness.ptyProcess.kill()); + assert.deepStrictEqual(kill.args, [["SIGTERM"], ["SIGKILL"]]); + harness.exit(137); + }); +}); diff --git a/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts b/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts new file mode 100644 index 0000000000..4744e7f13b --- /dev/null +++ b/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts @@ -0,0 +1,93 @@ +import * as assert from "node:assert"; +import { describe, it } from "mocha"; +import proxyquire from "proxyquire"; +import sinon from "sinon"; + +class TestEventEmitter { + private readonly listeners = new Set<(event: T) => void>(); + readonly event = (listener: (event: T) => void) => { + this.listeners.add(listener); + return { dispose: () => this.listeners.delete(listener) }; + }; + fire(event: T): void { + for (const listener of [...this.listeners]) listener(event); + } + dispose(): void { + this.listeners.clear(); + } +} + +describe("PtyTerminal", () => { + it("replays output and forwards input and dimensions", () => { + let dataListener: ((data: string) => void) | undefined; + let exitListener: ((event: { exitCode: number }) => void) | undefined; + const ptyProcess = { + subscribeWithReplay: (listener: (data: string) => void) => { + dataListener = listener; + return { + replay: ["before timeout\n"], + disposable: { dispose: sinon.stub() }, + }; + }, + onExit: (listener: (event: { exitCode: number }) => void) => { + exitListener = listener; + return { dispose: sinon.stub() }; + }, + write: sinon.stub(), + resize: sinon.stub(), + }; + const onCloseRequested = sinon.stub(); + const { PtyTerminal } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../pty-terminal", { + vscode: { EventEmitter: TestEventEmitter }, + }) as typeof import("../pty-terminal"); + const terminal = new PtyTerminal(ptyProcess as never, onCloseRequested); + const output: string[] = []; + const exits: Array = []; + terminal.onDidWrite((data) => output.push(data)); + terminal.onDidClose((exitCode) => exits.push(exitCode)); + + dataListener?.("while opening\n"); + terminal.open(); + dataListener?.("after opening\n"); + terminal.handleInput("hello\r"); + terminal.setDimensions({ columns: 120, rows: 40 }); + exitListener?.({ exitCode: 0 }); + terminal.close(); + + assert.deepStrictEqual(output, [ + "before timeout\n", + "while opening\n", + "after opening\n", + ]); + assert.ok(ptyProcess.write.calledOnceWithExactly("hello\r")); + assert.ok(ptyProcess.resize.calledOnceWithExactly(120, 40)); + assert.deepStrictEqual(exits, [0]); + assert.strictEqual(onCloseRequested.callCount, 0); + }); + + it("requests a stop when the user closes a running terminal", () => { + const onCloseRequested = sinon.stub(); + const ptyProcess = { + subscribeWithReplay: () => ({ + replay: [], + disposable: { dispose: sinon.stub() }, + }), + onExit: () => ({ dispose: sinon.stub() }), + write: sinon.stub(), + resize: sinon.stub(), + }; + const { PtyTerminal } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../pty-terminal", { + vscode: { EventEmitter: TestEventEmitter }, + }) as typeof import("../pty-terminal"); + + new PtyTerminal(ptyProcess as never, onCloseRequested).close(); + + assert.strictEqual(onCloseRequested.callCount, 1); + }); +}); diff --git a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts index dac54b64e7..dbc9b93c8e 100644 --- a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts @@ -8,34 +8,66 @@ interface Disposable { } class TestEventEmitter { - private listeners: Array<((event: T) => void) | undefined> = []; - + private listeners = new Set<(event: T) => void>(); readonly event = (listener: (event: T) => void): Disposable => { - const index = this.listeners.length; - this.listeners.push(listener); + this.listeners.add(listener); + return { dispose: () => this.listeners.delete(listener) }; + }; + fire(event: T): void { + for (const listener of [...this.listeners]) listener(event); + } +} +class TestPtyProcess { + private readonly dataListeners = new Set<(data: string) => void>(); + private readonly exitListeners = new Set< + (event: { exitCode: number }) => void + >(); + readonly replay: string[] = []; + killCalls = 0; + + subscribeWithReplay(listener: (data: string) => void) { + this.dataListeners.add(listener); return { - dispose: () => { - this.listeners[index] = undefined; - }, + replay: [...this.replay], + disposable: { dispose: () => this.dataListeners.delete(listener) }, }; - }; + } - fire(event: T): void { - const listenerCount = this.listeners.length; - for (let i = 0; i < listenerCount; i++) { - this.listeners[i]?.(event); - } + onExit(listener: (event: { exitCode: number }) => void) { + this.exitListeners.add(listener); + return { dispose: () => this.exitListeners.delete(listener) }; + } + + clearReplay(): void { + this.replay.length = 0; + } + + emitData(data: string): void { + this.replay.push(data); + for (const listener of [...this.dataListeners]) listener(data); + } + + emitExit(exitCode: number): void { + for (const listener of [...this.exitListeners]) listener({ exitCode }); + } + + kill(): void { + this.killCalls++; } } class TestExecutionError extends Error { - static create(message: string): TestExecutionError { + aborted = false; + + static create(message: string) { return new TestExecutionError(message); } - static createAbortError(): TestExecutionError { - return new TestExecutionError("Background job aborted."); + static createAbortError() { + const error = new TestExecutionError("aborted"); + error.aborted = true; + return error; } } @@ -43,49 +75,38 @@ async function flushPromises(): Promise { await new Promise((resolve) => setImmediate(resolve)); } -function createHarness(options?: { read?: () => AsyncIterable }) { +function createHarness(options?: { + replay?: string[]; + appendError?: Error; + closeError?: Error; + createTerminalError?: Error; +}) { const closeEmitter = new TestEventEmitter(); - const shellIntegrationEmitter = new TestEventEmitter<{ - terminal: FakeTerminal; - shellIntegration: FakeShellIntegration; - }>(); - const executionEndEmitter = new TestEventEmitter<{ - execution: FakeExecution; - exitCode: number | undefined; - }>(); - const execution: FakeExecution = { - read: options?.read ?? (async function* () {}), - }; - const shellIntegration: FakeShellIntegration = { - executeCommand: () => execution, - }; let terminalDisposeCalls = 0; const terminal: FakeTerminal = { - shellIntegration, show: () => {}, dispose: () => { terminalDisposeCalls++; closeEmitter.fire(terminal); }, }; - const finalizeCalls: Array = []; const lifecycle: string[] = []; - const outputManager = { - output: { value: undefined }, - addChunk: () => {}, - finalize: (error?: TestExecutionError) => finalizeCalls.push(error), - }; + const finalizeCalls: Array = []; + const ptyProcess = new TestPtyProcess(); + ptyProcess.replay.push(...(options?.replay ?? [])); const vscode = { EventEmitter: TestEventEmitter, ThemeIcon: class { constructor(readonly id: string) {} }, - window: { - onDidCloseTerminal: closeEmitter.event, - onDidChangeTerminalShellIntegration: shellIntegrationEmitter.event, - onDidEndTerminalShellExecution: executionEndEmitter.event, - }, + window: { onDidCloseTerminal: closeEmitter.event }, + }; + + const outputManager = { + output: { value: undefined }, + addChunk: (chunk: string) => lifecycle.push(`manager:${chunk}`), + finalize: (error?: TestExecutionError) => finalizeCalls.push(error), }; const { TerminalJob } = proxyquire @@ -94,24 +115,23 @@ function createHarness(options?: { read?: () => AsyncIterable }) { .load("../terminal-job", { vscode, "../layout": { - createTerminal: () => terminal, + createTerminal: () => { + if (options?.createTerminalError) throw options.createTerminalError; + return terminal; + }, }, "@/lib/logger": { - getLogger: () => ({ - debug: () => {}, - info: () => {}, - }), - }, - "@getpochi/common/env-utils": { - getTerminalEnv: () => ({}), + getLogger: () => ({ debug: () => {}, info: () => {} }), }, "@getpochi/common/tool-utils": { BackgroundJobOutputFile: class { async append(chunk: string) { lifecycle.push(`output:${chunk}`); + if (options?.appendError) throw options.appendError; } async close() { lifecycle.push("file-closed"); + if (options?.closeError) throw options.closeError; } }, PlainOutputSanitizer: class { @@ -124,39 +144,55 @@ function createHarness(options?: { read?: () => AsyncIterable }) { }, createBackgroundJobId: () => "bgjob-cmd-test", getBackgroundJobOutputPath: () => "/tmp/bgjob-cmd-test.log", - getShellPath: () => "/bin/sh", }, "./output": { OutputManager: { create: () => outputManager, + delete: () => lifecycle.push("manager-deleted"), }, }, - "./utils": { - ExecutionError: TestExecutionError, + "./pty-terminal": { + PtyTerminal: class { + private readonly exitSubscription: Disposable; + constructor(process: TestPtyProcess) { + this.exitSubscription = process.onExit(() => { + closeEmitter.fire(terminal); + }); + } + dispose() { + this.exitSubscription.dispose(); + } + }, }, + "./utils": { ExecutionError: TestExecutionError }, }) as typeof import("../terminal-job"); - const job = TerminalJob.create({ - name: "test job", - command: "sleep 10", - cwd: "/tmp", - taskId: "task-test", - }); const finishEvents: BackgroundJobTerminalEvent[] = []; TerminalJob.onDidFinish((event) => { lifecycle.push("event-fired"); finishEvents.push(event); }); + let job: ReturnType | undefined; + let adoptionError: unknown; + try { + job = TerminalJob.adopt(ptyProcess as never, { + name: "test job", + command: "sleep 10", + cwd: "/tmp", + taskId: "task-test", + }); + } catch (error) { + adoptionError = error; + } return { TerminalJob, - closeEmitter, - execution, - executionEndEmitter, + adoptionError, finalizeCalls, finishEvents, - job, + job: job as ReturnType, lifecycle, + ptyProcess, terminal, get terminalDisposeCalls() { return terminalDisposeCalls; @@ -164,222 +200,208 @@ function createHarness(options?: { read?: () => AsyncIterable }) { }; } -interface FakeExecution { - read(): AsyncIterable; -} - -interface FakeShellIntegration { - executeCommand(command: string): FakeExecution; -} - interface FakeTerminal { - shellIntegration: FakeShellIntegration; show(): void; dispose(): void; } describe("TerminalJob", () => { - it("closes the terminal after a background command completes", async () => { - const harness = createHarness(); - - await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 0, + it("does not launch shell integration for an already-aborted job", async () => { + const closeEmitter = new TestEventEmitter(); + const executeCommandCalls: string[] = []; + const terminal: FakeTerminal & { + shellIntegration: { executeCommand(command: string): never }; + } = { + show: () => { + throw new Error("aborted terminal should not be shown"); + }, + dispose: () => closeEmitter.fire(terminal), + shellIntegration: { + executeCommand: (command: string) => { + executeCommandCalls.push(command); + throw new Error("aborted command should not execute"); + }, + }, + }; + class TestPtySpawnError extends Error { + cause = new Error("pty unavailable"); + } + const finishEvents: BackgroundJobTerminalEvent[] = []; + const vscode = { + EventEmitter: TestEventEmitter, + ThemeIcon: class { + constructor(readonly id: string) {} + }, + window: { onDidCloseTerminal: closeEmitter.event }, + }; + const outputManager = { + output: { value: undefined }, + addChunk: () => {}, + finalize: () => {}, + }; + const { TerminalJob } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../terminal-job", { + vscode, + "../layout": { createTerminal: () => terminal }, + "@/lib/logger": { + getLogger: () => ({ + debug: () => {}, + info: () => {}, + warn: () => {}, + }), + }, + "@getpochi/common/env-utils": { getTerminalEnv: () => ({}) }, + "@getpochi/common/tool-utils": { + BackgroundJobOutputFile: class { + async append() {} + async close() {} + }, + PlainOutputSanitizer: class { + write(chunk: string) { + return chunk; + } + end() { + return ""; + } + }, + createBackgroundJobId: () => "bgjob-cmd-aborted", + getBackgroundJobOutputPath: () => "/tmp/bgjob-cmd-aborted.log", + getShellPath: () => "/bin/zsh", + }, + "./output": { + OutputManager: { + create: () => outputManager, + delete: () => {}, + }, + }, + "./pty-process": { + PtyProcess: { + spawn: async () => { + throw new TestPtySpawnError(); + }, + }, + PtySpawnError: TestPtySpawnError, + }, + "./pty-terminal": { PtyTerminal: class {} }, + "./utils": { ExecutionError: TestExecutionError }, + }) as typeof import("../terminal-job"); + TerminalJob.onDidFinish((event) => finishEvents.push(event)); + const abortController = new AbortController(); + abortController.abort(); + + await TerminalJob.create({ + name: "aborted shell job", + command: "echo should-not-run", + cwd: "/tmp", + taskId: "task-test", + abortSignal: abortController.signal, }); await flushPromises(); - assert.strictEqual(harness.finalizeCalls.length, 1); - assert.strictEqual(harness.finalizeCalls[0], undefined); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "file-closed", - "event-fired", - ]); - assert.deepStrictEqual(harness.finishEvents, [ - { - taskId: "task-test", - backgroundJobId: "bgjob-cmd-test", - outputFile: "/tmp/bgjob-cmd-test.log", - status: "completed", - command: "sleep 10", - exitCode: 0, - finishedAt: harness.finishEvents[0]?.finishedAt, - }, - ]); - assert.strictEqual(harness.terminalDisposeCalls, 1); - assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); + assert.deepStrictEqual(executeCommandCalls, []); + assert.strictEqual(finishEvents[0]?.status, "stopped"); }); - it("finalizes a running job when its terminal closes", async () => { - const { TerminalJob, finalizeCalls, job, terminal } = createHarness(); - - await flushPromises(); - terminal.dispose(); + it("rolls back an adopted pty when terminal initialization fails", async () => { + const initializationError = new Error("terminal creation failed"); + const harness = createHarness({ createTerminalError: initializationError }); await flushPromises(); - assert.strictEqual(TerminalJob.get(job.id), undefined); - assert.strictEqual(finalizeCalls.length, 1); - assert.match( - finalizeCalls[0]?.message ?? "", - /user closed terminal/, + assert.strictEqual(harness.adoptionError, initializationError); + assert.ok(harness.ptyProcess.killCalls >= 1); + assert.strictEqual( + harness.TerminalJob.get("bgjob-cmd-test"), + undefined, ); + assert.ok(harness.lifecycle.includes("manager-deleted")); + assert.ok(harness.lifecycle.includes("file-closed")); }); - it("discards a trailing replacement character when manually stopped", async () => { - let finishOutput: (() => void) | undefined; - const outputStopped = new Promise((resolve) => { - finishOutput = resolve; - }); - const harness = createHarness({ - read: async function* () { - yield "ready\uFFFD"; - await outputStopped; - }, - }); - - await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:ready", - ]); - - harness.job.kill(); - finishOutput?.(); - await flushPromises(); + it("replays foreground output and completes", async () => { + const harness = createHarness({ replay: ["before timeout\n"] }); + harness.ptyProcess.emitData("after timeout\n"); + harness.ptyProcess.emitExit(0); await flushPromises(); assert.deepStrictEqual(harness.lifecycle, [ "output:$ sleep 10\n", - "output:ready", + "output:before timeout\n", + "manager:before timeout\n", + "output:after timeout\n", + "manager:after timeout\n", "file-closed", "event-fired", + "manager-deleted", ]); - assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); + assert.strictEqual(harness.finishEvents[0]?.status, "completed"); + assert.strictEqual(harness.terminalDisposeCalls, 1); + assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); }); - it("discards an interrupted UTF-8 marker before Ctrl+C output", async () => { - let finishOutput: (() => void) | undefined; - const outputStopped = new Promise((resolve) => { - finishOutput = resolve; - }); - const harness = createHarness({ - read: async function* () { - yield "ready\uFFFD"; - yield "^"; - yield "C\r\n"; - yield " \r\r"; - await outputStopped; - }, - }); + it("marks a killed command as stopped", async () => { + const harness = createHarness(); + harness.job.kill(); + assert.strictEqual(harness.ptyProcess.killCalls, 1); + harness.ptyProcess.emitExit(143); await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:ready", - ]); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 130, - }); - finishOutput?.(); - await flushPromises(); + assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); + }); + + it("marks a nonzero natural exit as failed", async () => { + const harness = createHarness(); + harness.ptyProcess.emitExit(2); await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:ready", - "output:^C\r\n \r\r", - "file-closed", - "event-fired", - ]); + assert.strictEqual(harness.finishEvents[0]?.status, "failed"); + assert.strictEqual(harness.finishEvents[0]?.exitCode, 2); + assert.match(harness.finishEvents[0]?.error ?? "", /exited with code 2/); }); - it("preserves a trailing replacement character after normal completion", async () => { - const harness = createHarness({ - read: async function* () { - yield "valid replacement: \uFFFD"; - }, - }); - - await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 0, - }); + it("publishes a close failure through the output manager", async () => { + const harness = createHarness({ closeError: new Error("flush failed") }); + harness.ptyProcess.emitExit(0); await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:valid replacement: ", - "output:\uFFFD", - "file-closed", - "event-fired", - ]); - assert.strictEqual(harness.finishEvents[0]?.status, "completed"); + assert.match(harness.finalizeCalls[0]?.message ?? "", /flush failed/); + assert.strictEqual(harness.finishEvents[0]?.status, "failed"); }); - it("waits for trailing output before notifying about a failed job", async () => { - let releaseOutput: (() => void) | undefined; - const outputReady = new Promise((resolve) => { - releaseOutput = resolve; - }); + it("observes output persistence failures before process exit", async () => { const harness = createHarness({ - read: async function* () { - await outputReady; - yield "failure details"; - }, - }); - - await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 1, + appendError: new Error("output disk full"), }); await flushPromises(); + assert.strictEqual(harness.ptyProcess.killCalls, 1); assert.strictEqual(harness.finishEvents.length, 0); - assert.deepStrictEqual(harness.lifecycle, ["output:$ sleep 10\n"]); - releaseOutput?.(); + harness.ptyProcess.emitExit(143); await flushPromises(); + assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); + assert.match(harness.finishEvents[0]?.error ?? "", /output disk full/); + }); + + it("removes an interrupted replacement marker", async () => { + const harness = createHarness(); + harness.ptyProcess.emitData("ready\uFFFD^C\r\n"); + harness.job.kill(); + harness.ptyProcess.emitExit(130); await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:failure details", - "file-closed", - "event-fired", - ]); - assert.strictEqual(harness.finishEvents[0]?.status, "failed"); - assert.strictEqual(harness.finishEvents[0]?.exitCode, 1); + assert.ok(harness.lifecycle.includes("output:ready")); + assert.ok(!harness.lifecycle.some((entry) => entry.includes("\uFFFD"))); }); - it("notifies when a background command fails without command output", async () => { + it("preserves a replacement character after normal completion", async () => { const harness = createHarness(); - - await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 2, - }); + harness.ptyProcess.emitData("valid replacement: \uFFFD"); + harness.ptyProcess.emitExit(0); await flushPromises(); - assert.strictEqual(harness.finalizeCalls.length, 1); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "file-closed", - "event-fired", - ]); - assert.strictEqual(harness.finishEvents.length, 1); - assert.strictEqual(harness.finishEvents[0]?.status, "failed"); - assert.strictEqual(harness.finishEvents[0]?.exitCode, 2); - assert.match( - harness.finishEvents[0]?.error ?? "", - /exited with code 2/, - ); - assert.strictEqual(harness.terminalDisposeCalls, 1); - assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); + assert.ok(harness.lifecycle.includes("output:\uFFFD")); }); }); diff --git a/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts b/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts index 30cfdfabcb..7d16ef8f3a 100644 --- a/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts +++ b/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts @@ -1,56 +1,26 @@ -import { getLogger } from "@getpochi/common"; -import { getTerminalEnv } from "@getpochi/common/env-utils"; -import { buildShellCommand } from "@getpochi/common/tool-utils"; -import type * as nodePty from "node-pty"; -import * as vscode from "vscode"; +import { PtyProcess } from "./pty-process"; import type { ExecuteCommandOptions } from "./types"; import { ExecutionError, truncateOutput } from "./utils"; -const logger = getLogger("ExecuteCommandWithPty"); - -export class PtySpawnError extends Error { - constructor(cause: unknown) { - super("Failed to spawn pty."); - this.name = "PtySpawnError"; - this.cause = cause; - } -} - -const nodePtyPath = vscode.Uri.joinPath( - vscode.Uri.file(vscode.env.appRoot), - "node_modules", - "node-pty", - "lib", - "index.js", -).toString(); - -export const toNonInteractivePtyCommand = ( - command: string, - platform: NodeJS.Platform = process.platform, -): string => { - if (platform === "win32") { - return command; - } - - return `( ${command} ) | undefined, -): NodeJS.ProcessEnv => { - return { - ...process.env, - ...envs, - ...getTerminalEnv(), - }; -}; - -export const buildPtyShellCommand = ( - command: string, - platform: NodeJS.Platform = process.platform, -) => { - return buildShellCommand(toNonInteractivePtyCommand(command, platform)); -}; +export { + PtySpawnError, + buildPtyEnv, + buildPtyShellCommand, + getNodePtyModulePaths, +} from "./pty-process"; + +export type PtyCommandResult = + | { + type: "completed"; + output: string; + isTruncated: boolean; + } + | { + type: "timedOut"; + ptyProcess: PtyProcess; + output: string; + isTruncated: boolean; + }; export const executeCommandWithPty = async ({ command, @@ -59,70 +29,68 @@ export const executeCommandWithPty = async ({ abortSignal, onData, envs, -}: ExecuteCommandOptions) => { - const shellCommand = buildPtyShellCommand(command); - if (!shellCommand) { - throw new PtySpawnError("Failed to get shell."); - } - - let pty: typeof nodePty; - try { - pty = await import(nodePtyPath); - } catch (error) { - throw new PtySpawnError(error); - } - - return new Promise<{ output: string; isTruncated: boolean }>( - (resolve, reject) => { - const { command: shell, args } = shellCommand; - logger.debug( - `Executing command with pty: ${command} in ${cwd}, shell: ${shell}, args: ${args}`, - ); - const ptyProcess = pty.spawn(shell, args, { - // Using 'xterm-256color' here helps ensure that the majority of Linux distributions will use a - // color prompt as defined in the default ~/.bashrc file. - name: "xterm-256color", - cols: 80, - rows: 30, - cwd, - env: buildPtyEnv(envs), - }); - - let output = ""; - let timeoutId: ReturnType | undefined; - - if (timeout > 0) { - timeoutId = setTimeout(() => { - ptyProcess.kill("SIGTERM"); - reject(ExecutionError.createTimeoutError(timeout)); - }, timeout * 1000); - } - - const onAbort = () => { - ptyProcess.kill("SIGTERM"); +}: ExecuteCommandOptions): Promise => { + const ptyProcess = await PtyProcess.spawn({ command, cwd, envs }); + + return new Promise((resolve, reject) => { + let output = ""; + let settled = false; + let timeoutId: ReturnType | undefined; + + const cleanup = () => { + if (timeoutId) clearTimeout(timeoutId); + abortSignal?.removeEventListener("abort", onAbort); + dataListener.dispose(); + exitListener.dispose(); + }; + + const settle = (callback: () => void) => { + if (settled) return; + settled = true; + cleanup(); + callback(); + }; + + const onAbort = () => { + settle(() => { + ptyProcess.kill(); reject(ExecutionError.createAbortError()); - }; - abortSignal?.addEventListener("abort", onAbort); - - const dataListener = ptyProcess.onData((data: string) => { - output = output + data; - onData?.(truncateOutput(output)); }); + }; - const exitListener = ptyProcess.onExit(({ exitCode }) => { - if (timeoutId) clearTimeout(timeoutId); - abortSignal?.removeEventListener("abort", onAbort); - dataListener.dispose(); - exitListener.dispose(); + const dataListener = ptyProcess.onData((data) => { + output += data; + onData?.(truncateOutput(output)); + }); + const exitListener = ptyProcess.onExit(({ exitCode }) => { + settle(() => { if (exitCode === 0) { - resolve(truncateOutput(output)); + resolve({ type: "completed", ...truncateOutput(output) }); } else { reject( ExecutionError.create(`Command exited with code ${exitCode}.`), ); } }); - }, - ); + }); + + if (abortSignal?.aborted) { + onAbort(); + return; + } + abortSignal?.addEventListener("abort", onAbort, { once: true }); + + if (timeout > 0) { + timeoutId = setTimeout(() => { + settle(() => { + resolve({ + type: "timedOut", + ptyProcess, + ...truncateOutput(output), + }); + }); + }, timeout * 1000); + } + }); }; diff --git a/packages/vscode/src/integrations/terminal/pty-process.ts b/packages/vscode/src/integrations/terminal/pty-process.ts new file mode 100644 index 0000000000..9486a70782 --- /dev/null +++ b/packages/vscode/src/integrations/terminal/pty-process.ts @@ -0,0 +1,227 @@ +import { createRequire } from "node:module"; +import path from "node:path"; +import { getLogger } from "@getpochi/common"; +import { getTerminalEnv } from "@getpochi/common/env-utils"; +import { buildShellCommand } from "@getpochi/common/tool-utils"; +import type * as nodePty from "node-pty"; +import * as vscode from "vscode"; + +const logger = getLogger("PtyProcess"); +const TerminationGraceMs = 2_000; +const HardKillExitGraceMs = 1_000; +const requireFromExtensionHost = createRequire(__filename); + +export class PtySpawnError extends Error { + constructor(cause: unknown) { + super("Failed to spawn pty."); + this.name = "PtySpawnError"; + this.cause = cause; + } +} + +export interface PtyProcessOptions { + command: string; + cwd: string; + envs?: Record; +} + +export interface PtyProcessExit { + exitCode: number; + signal?: number; +} + +type DataListener = (data: string) => void; +type ExitListener = (event: PtyProcessExit) => void; + +export const getNodePtyModulePaths = (appRoot = vscode.env.appRoot) => [ + path.join(appRoot, "node_modules.asar", "node-pty"), + path.join(appRoot, "node_modules", "node-pty"), +]; + +const loadNodePty = (): typeof nodePty => { + const errors: unknown[] = []; + for (const modulePath of getNodePtyModulePaths()) { + try { + return requireFromExtensionHost(modulePath) as typeof nodePty; + } catch (error) { + errors.push(error); + } + } + throw new AggregateError(errors, "Failed to load VS Code's node-pty module."); +}; + +export const buildPtyEnv = ( + envs: Record | undefined, +): NodeJS.ProcessEnv => ({ + ...process.env, + ...envs, + ...getTerminalEnv(), +}); + +export const buildPtyShellCommand = (command: string) => + buildShellCommand(command); + +export class PtyProcess { + private readonly dataListeners = new Set(); + private readonly exitListeners = new Set(); + private readonly history: string[] = []; + private rawExitEvent: PtyProcessExit | undefined; + private exitEvent: PtyProcessExit | undefined; + private forceKillTimer: ReturnType | undefined; + private hardKillExitTimer: ReturnType | undefined; + private terminationRequested = false; + + private constructor(private readonly process: nodePty.IPty) { + process.onData((data) => { + this.history.push(data); + for (const listener of this.dataListeners) { + listener(data); + } + }); + process.onExit((event) => { + if (this.rawExitEvent) return; + this.rawExitEvent = event; + this.clearTerminationTimers(); + // UnixTerminal emits node-pty's exit only after its PTY socket closes, + // so all data callbacks have already been delivered at this boundary. + this.publishExit(event); + }); + } + + static async spawn({ command, cwd, envs }: PtyProcessOptions) { + const shellCommand = buildPtyShellCommand(command); + if (!shellCommand) { + throw new PtySpawnError("Failed to get shell."); + } + + let pty: typeof nodePty; + try { + pty = loadNodePty(); + } catch (error) { + throw new PtySpawnError(error); + } + + try { + const { command: shell, args } = shellCommand; + logger.debug( + `Spawning pty command: ${command} in ${cwd}, shell: ${shell}, args: ${args}`, + ); + return new PtyProcess( + pty.spawn(shell, args, { + name: "xterm-256color", + cols: 80, + rows: 30, + cwd, + env: buildPtyEnv(envs), + }), + ); + } catch (error) { + throw new PtySpawnError(error); + } + } + + onData(listener: DataListener): vscode.Disposable { + this.dataListeners.add(listener); + return { dispose: () => this.dataListeners.delete(listener) }; + } + + subscribeWithReplay(listener: DataListener): { + replay: readonly string[]; + disposable: vscode.Disposable; + } { + const replay = [...this.history]; + const disposable = this.onData(listener); + return { replay, disposable }; + } + + clearReplay(): void { + this.history.length = 0; + } + + /** Fires at node-pty's socket-close boundary, after queued output drains. */ + onExit(listener: ExitListener): vscode.Disposable { + if (this.exitEvent) { + let cancelled = false; + const event = this.exitEvent; + queueMicrotask(() => { + if (!cancelled) listener(event); + }); + return { + dispose: () => { + cancelled = true; + }, + }; + } + this.exitListeners.add(listener); + return { dispose: () => this.exitListeners.delete(listener) }; + } + + write(data: string): void { + if (this.rawExitEvent) return; + try { + this.process.write(data); + } catch (error) { + logger.debug("Failed to write to exited pty process", error); + } + } + + resize(columns: number, rows: number): void { + if (this.rawExitEvent || columns <= 0 || rows <= 0) return; + try { + this.process.resize(columns, rows); + } catch (error) { + logger.debug("Failed to resize exited pty process", error); + } + } + + kill(signal = "SIGTERM"): void { + if (this.rawExitEvent) return; + + if (signal === "SIGKILL" || this.terminationRequested) { + this.sendSignal("SIGKILL"); + this.scheduleSyntheticHardKillExit(); + return; + } + + this.terminationRequested = true; + this.sendSignal(signal); + this.forceKillTimer = setTimeout(() => { + if (this.rawExitEvent) return; + this.sendSignal("SIGKILL"); + this.scheduleSyntheticHardKillExit(); + }, TerminationGraceMs); + } + + private sendSignal(signal: string): void { + try { + this.process.kill(signal); + } catch (error) { + logger.debug(`Failed to send ${signal} to exited pty process`, error); + } + } + + private scheduleSyntheticHardKillExit(): void { + if (this.hardKillExitTimer || this.rawExitEvent) return; + this.hardKillExitTimer = setTimeout(() => { + if (this.rawExitEvent) return; + logger.warn("Pty did not emit an exit event after SIGKILL"); + const event = { exitCode: 137, signal: 9 }; + this.rawExitEvent = event; + this.publishExit(event); + }, HardKillExitGraceMs); + } + + private publishExit(event: PtyProcessExit): void { + if (this.exitEvent) return; + this.exitEvent = event; + for (const listener of this.exitListeners) listener(event); + this.exitListeners.clear(); + } + + private clearTerminationTimers(): void { + if (this.forceKillTimer) clearTimeout(this.forceKillTimer); + if (this.hardKillExitTimer) clearTimeout(this.hardKillExitTimer); + this.forceKillTimer = undefined; + this.hardKillExitTimer = undefined; + } +} diff --git a/packages/vscode/src/integrations/terminal/pty-terminal.ts b/packages/vscode/src/integrations/terminal/pty-terminal.ts new file mode 100644 index 0000000000..c7288de4ba --- /dev/null +++ b/packages/vscode/src/integrations/terminal/pty-terminal.ts @@ -0,0 +1,70 @@ +import * as vscode from "vscode"; +import type { PtyProcess } from "./pty-process"; + +export class PtyTerminal implements vscode.Pseudoterminal, vscode.Disposable { + private readonly writeEmitter = new vscode.EventEmitter(); + private readonly closeEmitter = new vscode.EventEmitter(); + private readonly disposables: vscode.Disposable[] = []; + private readonly pendingOutput: string[] = []; + private opened = false; + private exited = false; + private disposed = false; + + readonly onDidWrite = this.writeEmitter.event; + readonly onDidClose = this.closeEmitter.event; + + constructor( + private readonly ptyProcess: PtyProcess, + private readonly onCloseRequested: () => void, + ) { + const subscription = ptyProcess.subscribeWithReplay((data) => { + if (this.opened) { + this.writeEmitter.fire(data); + } else { + this.pendingOutput.push(data); + } + }); + this.pendingOutput.unshift(...subscription.replay); + this.disposables.push(subscription.disposable); + this.disposables.push( + ptyProcess.onExit(({ exitCode }) => { + if (this.exited) return; + this.exited = true; + this.closeEmitter.fire(exitCode); + }), + ); + } + + open(): void { + if (this.opened) return; + this.opened = true; + for (const data of this.pendingOutput.splice(0)) { + this.writeEmitter.fire(data); + } + } + + close(): void { + if (!this.exited) { + this.onCloseRequested(); + } + this.dispose(); + } + + handleInput(data: string): void { + this.ptyProcess.write(data); + } + + setDimensions(dimensions: vscode.TerminalDimensions): void { + this.ptyProcess.resize(dimensions.columns, dimensions.rows); + } + + dispose(): void { + if (this.disposed) return; + this.disposed = true; + for (const disposable of this.disposables) { + disposable.dispose(); + } + this.writeEmitter.dispose(); + this.closeEmitter.dispose(); + } +} diff --git a/packages/vscode/src/integrations/terminal/terminal-job.ts b/packages/vscode/src/integrations/terminal/terminal-job.ts index 4b9c90141f..ff33fd9599 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -11,32 +11,22 @@ import { import * as vscode from "vscode"; import { createTerminal } from "../layout"; import { OutputManager } from "./output"; +import { PtyProcess, PtySpawnError } from "./pty-process"; +import { PtyTerminal } from "./pty-terminal"; import { ExecutionError } from "./utils"; const logger = getLogger("TerminalJob"); -/** - * Configuration options for creating a TerminalJob - */ export interface TerminalJobConfig { - /** Name of the terminal */ name: string; - /** Command to execute in the terminal */ command: string; - /** Working directory for the terminal */ cwd: string; - /** Location for the terminal */ - location?: vscode.TerminalEditorLocationOptions | undefined; - /** AbortSignal to cancel the terminal job */ + location?: vscode.TerminalEditorLocationOptions; abortSignal?: AbortSignal; - /** Task that owns the job and receives its terminal notification. */ taskId: string; + envs?: Record; } -/** - * A wrapper class around vscode.Terminal that provides enhanced functionality - * for running commands and managing terminal lifecycle - */ export class TerminalJob implements vscode.Disposable { private static readonly jobs = new Map(); private static readonly onDidDisposeEmitter = @@ -46,20 +36,23 @@ export class TerminalJob implements vscode.Disposable { new vscode.EventEmitter(); static readonly onDidFinish = TerminalJob.onDidFinishEmitter.event; - private readonly terminal: vscode.Terminal; - private readonly terminalClosed: Promise; - private disposables: vscode.Disposable[] = []; - private closeListener: vscode.Disposable | undefined; - private rejectTerminalClosed: ((error: ExecutionError) => void) | undefined; - private disposed = false; - private shellIntegration: vscode.TerminalShellIntegration | undefined; - private execution: vscode.TerminalShellExecution | undefined; - private outputManager: OutputManager; - private readonly outputWriter: BackgroundJobOutputFile; - private exitCode: number | undefined; + private terminal!: vscode.Terminal; + private outputManager!: OutputManager; + private outputWriter!: BackgroundJobOutputFile; + private readonly sanitizer = new PlainOutputSanitizer(); + private readonly disposables: vscode.Disposable[] = []; + private outputQueue: Promise = Promise.resolve(); + private pendingTerminalSuffix = ""; + private persistenceError: ExecutionError | undefined; private stopRequested = false; + private ptyExited = false; private finished = false; - private outputStreamFinished: Promise | undefined; + private disposed = false; + private shellExecution: vscode.TerminalShellExecution | undefined; + private terminalCloseError: ExecutionError | undefined; + private readonly terminalCloseRejectors = new Set< + (error: ExecutionError) => void + >(); readonly id: string; readonly outputFile: string; @@ -72,329 +65,395 @@ export class TerminalJob implements vscode.Disposable { return this.config.command; } - private constructor(private readonly config: TerminalJobConfig) { + private constructor( + private readonly config: TerminalJobConfig, + private readonly ptyProcess?: PtyProcess, + ) { this.id = createBackgroundJobId("command"); this.outputFile = getBackgroundJobOutputPath(config.taskId, this.id); - this.outputWriter = new BackgroundJobOutputFile(this.outputFile); - this.outputManager = OutputManager.create({ - id: this.id, - command: config.command, - }); - TerminalJob.jobs.set(this.id, this); - // Create the terminal with the provided configuration - this.terminal = createTerminal({ - name: config.name, - cwd: config.cwd, - location: config.location, - shellPath: getShellPath(), - env: getTerminalEnv(), - iconPath: new vscode.ThemeIcon("piano"), - hideFromUser: false, - isTransient: false, - }); - - this.terminalClosed = new Promise((_, reject) => { - this.rejectTerminalClosed = reject; - }); - - // Keep the terminal and job lifecycle synchronized when the user closes - // the terminal before execution finishes. - this.closeListener = vscode.window.onDidCloseTerminal((terminal) => { - if (terminal === this.terminal) { - this.stopRequested = true; - this.rejectTerminalClosed?.( - ExecutionError.create( - "Background job finished as user closed terminal.", - ), - ); - this.rejectTerminalClosed = undefined; - this.dispose(); + try { + this.outputWriter = new BackgroundJobOutputFile(this.outputFile); + this.outputManager = OutputManager.create({ + id: this.id, + command: config.command, + }); + TerminalJob.jobs.set(this.id, this); + this.enqueueFileOutput(`$ ${config.command}\n`, false); + if (ptyProcess) { + this.initializePtyTerminal(ptyProcess); + } else { + this.initializeShellTerminal(); } - }); - - this.terminal.show(); - - this.execute(); + this.initializeLifecycle(); + if (!this.stopRequested) { + this.terminal.show(); + if (!ptyProcess) { + void this.executeWithShellIntegration(); + } + } else if (!ptyProcess) { + void this.finalize(undefined, ExecutionError.createAbortError()); + } + } catch (error) { + this.cleanupAfterInitializationFailure(); + throw error; + } logger.info( `Created terminal job "${config.name}" with command: ${config.command}`, ); } - async execute(): Promise { - let executionError: ExecutionError | undefined; - try { - await this.outputWriter.append(`$ ${this.config.command}\n`); - - // Wait for shell integration if not available - const shellIntegration = await Promise.race([ - this.waitForShellIntegration(), - this.terminalClosed, - ]); - - this.execution = shellIntegration.executeCommand(this.config.command); - logger.debug( - `Executed command in terminal "${this.config.name}": ${this.config.command}`, - ); - this.outputStreamFinished = this.processOutputStream( - this.execution.read(), - ); - - await Promise.race([ - this.waitForExecutionFinish(), - this.createAbortPromise(), - this.terminalClosed, - ]); - await this.outputStreamFinished; - } catch (error) { - if (error instanceof ExecutionError) { - executionError = error; - } else { - executionError = ExecutionError.create( - `Command execution failed: ${error}`, - ); - } - } finally { + static async create(config: TerminalJobConfig): Promise { + // Preserve the shell-integration implementation on Windows, where the + // extension's node-pty foreground implementation is not supported yet. + if (process.platform !== "win32") { try { - const pendingTerminalSuffix = (await this.outputStreamFinished) ?? ""; - const finalSuffix = - this.stopRequested || this.exitCode === 130 - ? pendingTerminalSuffix.replace(/\uFFFD+/u, "") - : pendingTerminalSuffix; - if (finalSuffix.length > 0) { - await this.outputWriter.append(finalSuffix); - this.outputManager.addChunk(finalSuffix); - } - } catch (outputError) { - executionError = ExecutionError.create( - outputError instanceof Error - ? outputError.message - : String(outputError), + const ptyProcess = await PtyProcess.spawn({ + command: config.command, + cwd: config.cwd, + envs: config.envs, + }); + return TerminalJob.adopt(ptyProcess, config); + } catch (error) { + if (!(error instanceof PtySpawnError)) throw error; + logger.warn( + "Failed to spawn background pty; falling back to shell integration", + error.cause, ); } - this.outputManager.finalize(executionError); - await this.finish(executionError); - this.cleanupExecution(); } + return new TerminalJob(config); } - /** - * Dispose of execution-scoped listeners. - */ - private cleanupExecution(): void { - for (const d of this.disposables) { - d.dispose(); + static adopt(ptyProcess: PtyProcess, config: TerminalJobConfig): TerminalJob { + try { + return new TerminalJob(config, ptyProcess); + } catch (error) { + ptyProcess.kill("SIGKILL"); + throw error; } - this.disposables = []; } - /** - * Creates a promise that rejects when the abort signal is triggered - */ - private createAbortPromise(): Promise { - return new Promise((_, reject) => { - const abortError = ExecutionError.createAbortError(); + static get(id: string | vscode.Terminal): TerminalJob | undefined { + return typeof id === "string" + ? TerminalJob.jobs.get(id) + : Array.from(TerminalJob.jobs.values()).find( + (job) => job.terminal === id, + ); + } - // Check if already aborted - if (this.config.abortSignal?.aborted) { - reject(abortError); - return; - } + kill(): void { + this.requestStop("kill requested"); + } - // Set up abort listener - const abortListener = () => { - logger.info(`Command execution aborted: ${this.config.command}`); - this.stopRequested = true; - this.terminal.dispose(); - reject(abortError); - }; + dispose(): void { + if (this.disposed) return; + this.disposed = true; + TerminalJob.jobs.delete(this.id); + OutputManager.delete(this.id); + TerminalJob.onDidDisposeEmitter.fire(this); + for (const disposable of this.disposables.splice(0)) { + disposable.dispose(); + } + logger.debug(`Disposed terminal job "${this.config.name}"`); + } - this.config.abortSignal?.addEventListener("abort", abortListener, { - once: true, - }); + private initializePtyTerminal(ptyProcess: PtyProcess): void { + const outputSubscription = ptyProcess.subscribeWithReplay((data) => { + this.enqueueRawOutput(data); + }); + this.disposables.push(outputSubscription.disposable); + for (const data of outputSubscription.replay) { + this.enqueueRawOutput(data); + } - // Clean up timeout if promise chain is resolved elsewhere - // This is a fallback cleanup mechanism - const cleanup = () => { - this.config.abortSignal?.removeEventListener("abort", abortListener); - }; + // This listener is registered before PtyTerminal's close listener so a + // process-driven terminal close is not mistaken for a user cancellation. + this.disposables.push( + ptyProcess.onExit(() => { + this.ptyExited = true; + }), + ); + const ptyTerminal = new PtyTerminal(ptyProcess, () => { + this.requestStop("user closed terminal"); + }); + this.disposables.push(ptyTerminal); + ptyProcess.clearReplay(); - // Store cleanup function for potential use in dispose - this.disposables.push({ - dispose: cleanup, - }); + this.terminal = createTerminal({ + name: this.config.name, + pty: ptyTerminal, + location: this.config.location, + iconPath: new vscode.ThemeIcon("piano"), + isTransient: false, }); + this.disposables.push( + ptyProcess.onExit(({ exitCode }) => { + void this.finalize(exitCode); + }), + ); } - /** - * Processes the output stream and adds lines to the output manager - */ - private async processOutputStream( - outputStream: AsyncIterable, - ): Promise { - const sanitizer = new PlainOutputSanitizer(); - let pendingTerminalSuffix = ""; - const appendPlainText = async (plainText: string) => { - if (plainText.length === 0) return; - - const text = pendingTerminalSuffix + plainText; - const trailingTerminalSuffix = - text.match(/\uFFFD+(?:\^C[ \t\r\n]*|\^)?$/u)?.[0] ?? ""; - const completeText = text.slice( - 0, - text.length - trailingTerminalSuffix.length, - ); - pendingTerminalSuffix = trailingTerminalSuffix; - if (completeText.length === 0) return; + private initializeShellTerminal(): void { + this.terminal = createTerminal({ + name: this.config.name, + cwd: this.config.cwd, + location: this.config.location, + shellPath: getShellPath(), + env: { + ...this.config.envs, + ...getTerminalEnv(), + }, + iconPath: new vscode.ThemeIcon("piano"), + hideFromUser: false, + isTransient: false, + }); + } - await this.outputWriter.append(completeText); - this.outputManager.addChunk(completeText); - }; + private initializeLifecycle(): void { + this.disposables.push( + vscode.window.onDidCloseTerminal((terminal) => { + if ( + terminal !== this.terminal || + this.finished || + (this.ptyProcess && this.ptyExited) + ) { + return; + } + this.stopRequested = true; + if (this.ptyProcess) { + this.ptyProcess.kill(); + } else { + this.terminalCloseError = ExecutionError.create( + "Background job finished as user closed terminal.", + ); + for (const reject of this.terminalCloseRejectors) { + reject(this.terminalCloseError); + } + this.terminalCloseRejectors.clear(); + } + }), + ); - for await (const chunk of outputStream) { - await appendPlainText(sanitizer.write(chunk)); + const onAbort = () => this.requestStop("abort signal"); + if (this.config.abortSignal?.aborted) { + onAbort(); + } else if (this.config.abortSignal) { + this.config.abortSignal.addEventListener("abort", onAbort, { + once: true, + }); + this.disposables.push({ + dispose: () => + this.config.abortSignal?.removeEventListener("abort", onAbort), + }); } - await appendPlainText(sanitizer.end()); - - // VS Code exposes terminal output as decoded strings, so the original - // bytes are unavailable here. Keep a trailing U+FFFD, an optional ^C, and - // the terminal's trailing line-cleanup whitespace buffered until the - // execution result identifies an interrupted command. Normal completion - // still preserves legitimate replacement text. - return pendingTerminalSuffix; } - /** - * Kills the terminal job. - */ - kill(): void { - this.stopRequested = true; - this.terminal.dispose(); + private async executeWithShellIntegration(): Promise { + let executionError: ExecutionError | undefined; + let outputError: ExecutionError | undefined; + let outputFinished: Promise | undefined; + let exitCode: number | undefined; + try { + const shellIntegration = await Promise.race([ + this.waitForShellIntegration(), + this.waitForTerminalClose(), + ]); + if (this.stopRequested) { + throw ExecutionError.createAbortError(); + } + this.shellExecution = shellIntegration.executeCommand( + this.config.command, + ); + outputFinished = this.processShellOutput( + this.shellExecution.read(), + ).catch((error) => { + outputError = + error instanceof ExecutionError + ? error + : ExecutionError.create(`Failed to read command output: ${error}`); + }); + exitCode = await Promise.race([ + this.waitForShellExecutionFinish(), + this.waitForAbort(), + this.waitForTerminalClose(), + ]); + } catch (error) { + executionError = + error instanceof ExecutionError + ? error + : ExecutionError.create(`Command execution failed: ${error}`); + } finally { + await outputFinished; + } + executionError ??= outputError; + await this.finalize(exitCode, executionError); } - /** - * Dispose of the terminal and clean up resources - */ - dispose(): void { - if (this.disposed) { - return; + private async processShellOutput( + output: AsyncIterable, + ): Promise { + for await (const chunk of output) { + this.enqueueRawOutput(chunk); } - this.disposed = true; - - TerminalJob.jobs.delete(this.id); - TerminalJob.onDidDisposeEmitter.fire(this); - - this.closeListener?.dispose(); - this.closeListener = undefined; - - this.cleanupExecution(); - - logger.debug(`Disposed terminal job "${this.config.name}"`); } - /** - * Wait for shell integration to become available - */ - private async waitForShellIntegration( - timeoutMs = 15000, + private waitForShellIntegration( + timeoutMs = 15_000, ): Promise { if (this.terminal.shellIntegration) { - this.shellIntegration = this.terminal.shellIntegration; - return this.shellIntegration; + return Promise.resolve(this.terminal.shellIntegration); } - - return new Promise((resolve, reject) => { - // Set up timeout + return new Promise((resolve, reject) => { const timeout = setTimeout(() => { listener.dispose(); reject(new Error("Timeout waiting for shell integration")); }, timeoutMs); - - // Set up event listener for shell integration const listener = vscode.window.onDidChangeTerminalShellIntegration( ({ terminal, shellIntegration }) => { - if (terminal === this.terminal) { - logger.debug("Terminal shell integration acquired"); - this.shellIntegration = shellIntegration; - - // Clean up and resolve - clearTimeout(timeout); - listener.dispose(); - resolve(shellIntegration); - } + if (terminal !== this.terminal) return; + clearTimeout(timeout); + listener.dispose(); + resolve(shellIntegration); }, ); + this.disposables.push({ dispose: () => clearTimeout(timeout) }, listener); }); } - private waitForExecutionFinish(): Promise { + private waitForShellExecutionFinish(): Promise { return new Promise((resolve, reject) => { - // Listen for shell execution end. this.disposables.push( vscode.window.onDidEndTerminalShellExecution((event) => { - if (event.execution === this.execution) { - logger.debug("Terminal shell execution ended", event.exitCode); - this.exitCode = event.exitCode; - if (event.exitCode === undefined) { - reject( - ExecutionError.create( - "Background job execution finished with unknown exit code.", - ), - ); - } else if (event.exitCode !== 0) { - reject( - ExecutionError.create( - `Background job execution exited with code ${event.exitCode}.`, - ), - ); - } else { - resolve(); - } + if (event.execution !== this.shellExecution) return; + if (event.exitCode === undefined) { + reject( + ExecutionError.create( + "Background job execution finished with unknown exit code.", + ), + ); + } else { + resolve(event.exitCode); } }), ); }); } - /** - * Create a new TerminalJob instance - */ - static create(config: TerminalJobConfig): TerminalJob { - return new TerminalJob(config); + private waitForAbort(): Promise { + return new Promise((_, reject) => { + const onAbort = () => reject(ExecutionError.createAbortError()); + if (this.config.abortSignal?.aborted) { + onAbort(); + return; + } + this.config.abortSignal?.addEventListener("abort", onAbort, { + once: true, + }); + this.disposables.push({ + dispose: () => + this.config.abortSignal?.removeEventListener("abort", onAbort), + }); + }); } - /** - * Retrieves a `TerminalJob` instance by its ID. - * - * @param id - The ID of the job or the terminal instance. - * @returns The `TerminalJob` instance, or `undefined` if not found. - */ - static get(id: string | vscode.Terminal): TerminalJob | undefined { - return typeof id === "string" - ? TerminalJob.jobs.get(id) - : Array.from(TerminalJob.jobs.values()).find( - (job) => job.terminal === id, - ); + private waitForTerminalClose(): Promise { + if (this.terminalCloseError) { + return Promise.reject(this.terminalCloseError); + } + return new Promise((_, reject) => { + this.terminalCloseRejectors.add(reject); + }); + } + + private requestStop(reason: string): void { + if (this.finished || this.stopRequested) return; + this.stopRequested = true; + logger.info(`Stopping terminal job ${this.id}: ${reason}`); + if (this.ptyProcess) { + this.ptyProcess.kill(); + } else { + this.terminal.dispose(); + } + } + + private enqueueRawOutput(data: string): void { + if (this.persistenceError) return; + this.enqueuePlainOutput(this.sanitizer.write(data)); } - private async finish(error?: ExecutionError): Promise { + private enqueuePlainOutput(plainText: string): void { + if (plainText.length === 0 || this.persistenceError) return; + const text = this.pendingTerminalSuffix + plainText; + const trailingTerminalSuffix = + text.match(/\uFFFD+(?:\^C[ \t\r\n]*|\^)?$/u)?.[0] ?? ""; + const completeText = text.slice( + 0, + text.length - trailingTerminalSuffix.length, + ); + this.pendingTerminalSuffix = trailingTerminalSuffix; + this.enqueueFileOutput(completeText, true); + } + + private enqueueFileOutput(text: string, addToManager: boolean): void { + if (text.length === 0 || this.persistenceError) return; + const write = this.outputQueue.then(async () => { + await this.outputWriter.append(text); + if (addToManager) this.outputManager.addChunk(text); + }); + this.outputQueue = write.catch((error) => { + if (this.persistenceError) return; + this.persistenceError = ExecutionError.create( + error instanceof Error ? error.message : String(error), + ); + this.requestStop("background output persistence failed"); + }); + } + + private async finalize( + exitCode: number | undefined, + initialError?: ExecutionError, + ): Promise { if (this.finished) return; this.finished = true; + for (const disposable of this.disposables.splice(0)) { + disposable.dispose(); + } + this.terminalCloseRejectors.clear(); + + let executionError = initialError ?? this.persistenceError; + if (exitCode !== undefined && exitCode !== 0 && !this.stopRequested) { + executionError = ExecutionError.create( + `Background job execution exited with code ${exitCode}.`, + ); + } + + if (!this.persistenceError) { + this.enqueuePlainOutput(this.sanitizer.end()); + const finalSuffix = + this.stopRequested || exitCode === 130 + ? this.pendingTerminalSuffix.replace(/\uFFFD+/gu, "") + : this.pendingTerminalSuffix; + this.pendingTerminalSuffix = ""; + this.enqueueFileOutput(finalSuffix, true); + } + await this.outputQueue; + executionError ??= this.persistenceError; - let finalError = error; try { await this.outputWriter.close(); - } catch (closeError) { - finalError = ExecutionError.create( - closeError instanceof Error ? closeError.message : String(closeError), + } catch (error) { + executionError = ExecutionError.create( + error instanceof Error ? error.message : String(error), ); } + this.outputManager.finalize(executionError); const status = - this.stopRequested || finalError?.aborted + this.stopRequested || executionError?.aborted ? "stopped" - : this.exitCode === 0 && !finalError + : exitCode === 0 && !executionError ? "completed" : "failed"; TerminalJob.onDidFinishEmitter.fire({ @@ -403,14 +462,28 @@ export class TerminalJob implements vscode.Disposable { outputFile: this.outputFile, status, command: this.config.command, - ...(this.exitCode !== undefined ? { exitCode: this.exitCode } : {}), - ...(finalError ? { error: finalError.message } : {}), + ...(exitCode !== undefined ? { exitCode } : {}), + ...(executionError ? { error: executionError.message } : {}), finishedAt: Date.now(), }); - if (!this.disposed) { - this.terminal.dispose(); - this.dispose(); + this.terminal.dispose(); + this.dispose(); + } + + private cleanupAfterInitializationFailure(): void { + TerminalJob.jobs.delete(this.id); + OutputManager.delete(this.id); + for (const disposable of this.disposables.splice(0)) { + disposable.dispose(); + } + this.terminalCloseRejectors.clear(); + this.terminal?.dispose(); + if (this.outputWriter) { + void this.outputQueue + .finally(() => this.outputWriter.close()) + .catch(() => {}); } + this.ptyProcess?.kill("SIGKILL"); } } diff --git a/packages/vscode/src/tools/__test__/execute-command.test.ts b/packages/vscode/src/tools/__test__/execute-command.test.ts index 4febcb9214..6025402ac2 100644 --- a/packages/vscode/src/tools/__test__/execute-command.test.ts +++ b/packages/vscode/src/tools/__test__/execute-command.test.ts @@ -8,6 +8,10 @@ type SignalValue = { status: "idle" | "running" | "completed"; isTruncated: boolean; error?: string; + _meta?: { + backgroundJobId: string; + outputFile?: string; + }; }; describe("executeCommand Tool", () => { @@ -285,4 +289,102 @@ describe("executeCommand Tool", () => { }), ); }); + + it("adopts the running pty when the foreground wait times out", async () => { + const ptyProcess = { kill: sinon.stub() }; + const executeCommandWithPty = sinon.stub().resolves({ + type: "timedOut", + ptyProcess, + output: "still running", + isTruncated: false, + }); + const adopt = sinon.stub().returns({ + id: "bgjob-cmd-promoted", + outputFile: "/tmp/bgjob-cmd-promoted.log", + }); + const maybePersistToolResult = sinon.stub(); + const getViewColumnForTerminal = sinon.stub().returns(3); + const { executeCommand } = proxyquire.noCallThru().load( + "../execute-command", + { + "@/integrations/layout": { getViewColumnForTerminal }, + "@/integrations/terminal/terminal-job": { + TerminalJob: { create: sinon.stub(), adopt }, + }, + "@/lib/background-job-terminal-name": { + getBackgroundJobTerminalName: () => "Promoted", + }, + "@getpochi/common": { + getLogger: () => ({ warn: sinon.stub() }), + }, + "@getpochi/common/tool-utils": { + getShellPath: () => "/bin/zsh", + maybePersistToolResult, + }, + "@quilted/threads/signals": { + ThreadSignal: { + serialize: (signal: { + value: SignalValue; + subscribe: (subscriber: (value: SignalValue) => void) => () => void; + }) => ({ + get value() { + return signal.value; + }, + start(subscriber: (value: SignalValue) => void) { + return signal.subscribe(subscriber); + }, + }), + }, + }, + "../integrations/terminal/execute-command-with-node": { + executeCommandWithNode: sinon.stub(), + }, + "../integrations/terminal/execute-command-with-pty": { + PtySpawnError: class PtySpawnError extends Error {}, + executeCommandWithPty, + }, + }, + ) as typeof import("../execute-command"); + const abortSignal = new AbortController().signal; + const result = await executeCommand( + { command: "sleep 10", timeout: 1 }, + { + abortSignal, + cwd: "/workspace", + messages: [], + toolCallId: "call-promoted", + taskId: "task-1", + }, + ); + const values: SignalValue[] = []; + ( + (result as unknown as { streamingOutput: unknown }).streamingOutput as { + start: (subscriber: (value: SignalValue) => void) => () => void; + } + ).start((value) => values.push(value)); + await new Promise((resolve) => setImmediate(resolve)); + + assert.ok( + adopt.calledOnceWithExactly(ptyProcess, { + name: "Promoted", + command: "sleep 10", + cwd: "/workspace", + location: { viewColumn: 3 }, + abortSignal, + taskId: "task-1", + }), + ); + assert.strictEqual(ptyProcess.kill.callCount, 0); + assert.strictEqual(maybePersistToolResult.callCount, 0); + assert.deepStrictEqual(values.at(-1), { + content: + 'Background command "bgjob-cmd-promoted" started. Its output is written to "/tmp/bgjob-cmd-promoted.log". Do not infer job status from empty or partial output, and do not sleep or poll. Continue independent work, or use attemptCompletion if nothing else remains. After the completion notification resumes the task with its final status, read the output file if needed.', + status: "completed", + isTruncated: false, + _meta: { + backgroundJobId: "bgjob-cmd-promoted", + outputFile: "/tmp/bgjob-cmd-promoted.log", + }, + }); + }); }); diff --git a/packages/vscode/src/tools/execute-command.ts b/packages/vscode/src/tools/execute-command.ts index 53dbb4d38d..9c7a006e03 100644 --- a/packages/vscode/src/tools/execute-command.ts +++ b/packages/vscode/src/tools/execute-command.ts @@ -26,6 +26,7 @@ import { PtySpawnError, executeCommandWithPty, } from "../integrations/terminal/execute-command-with-pty"; +import { ExecutionError } from "../integrations/terminal/utils"; const logger = getLogger("ExecuteCommand"); const ExecuteCommandStreamingThrottleMs = 300; @@ -64,13 +65,14 @@ export const executeCommand: ToolFunctionType< const viewColumn = getViewColumnForTerminal(); const location = viewColumn ? { viewColumn } : undefined; - const job = TerminalJob.create({ + const job = await TerminalJob.create({ name: getBackgroundJobTerminalName(command), command, cwd, location, abortSignal, taskId, + ...(envs ? { envs } : {}), }); return createBackgroundCommandResult(job.id, job.outputFile); @@ -133,12 +135,43 @@ export const executeCommand: ToolFunctionType< throttledFlush.call(); }, }) - .then(async ({ output: commandOutput, isTruncated }) => { + .then(async (result) => { done = true; throttledFlush.cancel(); + + if (result.type === "timedOut") { + if (!taskId) { + result.ptyProcess.kill(); + throw ExecutionError.createTimeoutError(timeout); + } + + const viewColumn = getViewColumnForTerminal(); + const location = viewColumn ? { viewColumn } : undefined; + const job = TerminalJob.adopt(result.ptyProcess, { + name: getBackgroundJobTerminalName(command), + command, + cwd, + location, + abortSignal, + taskId, + ...(envs ? { envs } : {}), + }); + const backgroundResult = createBackgroundCommandResult( + job.id, + job.outputFile, + ); + output.value = { + content: backgroundResult.output, + status: "completed", + isTruncated: backgroundResult.isTruncated, + _meta: backgroundResult._meta, + }; + return; + } + output.value = await persistCompletedOutput({ - output: commandOutput, - isTruncated, + output: result.output, + isTruncated: result.isTruncated, }); }) .catch(async (error) => { @@ -208,7 +241,7 @@ async function executeCommandImpl({ } } - return await executeCommandWithNode({ + const result = await executeCommandWithNode({ command, cwd, timeout, @@ -216,4 +249,5 @@ async function executeCommandImpl({ envs, onData, }); + return { type: "completed" as const, ...result }; } From bbb8cd87d8d091ad1a911c1a5ff735f3c070dc29 Mon Sep 17 00:00:00 2001 From: zhanba Date: Thu, 3 Sep 2026 18:20:40 +0800 Subject: [PATCH 02/12] fix(vscode-webui): clarify command background promotion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep promoted commands visually distinct from commands started in the background while presenting the transition as one concise, durable status. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../__tests__/execute-command.test.tsx | 22 ++++++++++++++++--- .../tools/components/execute-command.tsx | 15 +++++++++---- .../vscode-webui/src/i18n/locales/en.json | 2 ++ .../vscode-webui/src/i18n/locales/jp.json | 2 ++ .../vscode-webui/src/i18n/locales/ko.json | 2 ++ .../vscode-webui/src/i18n/locales/zh.json | 2 ++ 6 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx b/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx index 689f7b9876..33b1cdfa61 100644 --- a/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx +++ b/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx @@ -51,7 +51,9 @@ vi.mock("../command-execution-panel", () => ({ data-output-file={outputFile} /> ), - CommandExecutionPanel: () => null, + CommandExecutionPanel: ({ command }: { command: string }) => ( +
{command}
+ ), CommandPanelContainer: () => null, CopyCommandButton: () => null, })); @@ -87,9 +89,12 @@ describe("executeCommandTool", () => { const panel = screen.getByTestId("background-job-panel"); expect(panel.dataset.jobId).toBe("bgjob-cmd-test"); expect(panel.dataset.outputFile).toBe("/tmp/bgjob-cmd-test.log"); + expect(screen.getByText("toolInvocation.backgroundExecute")).toBeTruthy(); + expect(screen.queryByTestId("command-promotion-transition")).toBeNull(); + expect(screen.queryByTestId("foreground-command-panel")).toBeNull(); }); - it("shows the job panel when a foreground command is promoted", () => { + it("shows the foreground-to-background transition when promoted", () => { render( { />, ); + expect(screen.getByText("toolInvocation.startedCommand")).toBeTruthy(); + expect(screen.queryByTestId("foreground-command-panel")).toBeNull(); + const transition = screen.getByTestId("command-promotion-transition"); + expect(transition).toBeTruthy(); + expect( + screen.getByText("toolInvocation.promotedToBackground"), + ).toBeTruthy(); + expect(transition.parentElement?.textContent).toContain( + "toolInvocation.startedCommand", + ); + expect(screen.getAllByTestId("background-job-panel")).toHaveLength(1); + const panel = screen.getByTestId("background-job-panel"); expect(panel.dataset.jobId).toBe("bgjob-cmd-promoted"); expect(panel.dataset.outputFile).toBe("/tmp/bgjob-cmd-promoted.log"); - expect(screen.getByText("toolInvocation.backgroundExecute")).toBeTruthy(); }); }); diff --git a/packages/vscode-webui/src/features/tools/components/execute-command.tsx b/packages/vscode-webui/src/features/tools/components/execute-command.tsx index d2d39d32f2..26ba40b58b 100644 --- a/packages/vscode-webui/src/features/tools/components/execute-command.tsx +++ b/packages/vscode-webui/src/features/tools/components/execute-command.tsx @@ -30,22 +30,29 @@ export const executeCommandTool: React.FC> = ({ const { cwd, command, background } = tool.input || {}; const backgroundJobMetadata = tool.state === "output-available" ? tool.output._meta : undefined; - const runsInBackground = background || Boolean(backgroundJobMetadata); + const isPromoted = !background && Boolean(backgroundJobMetadata); const cwdNode = cwd ? ( {" "} {t("toolInvocation.in")} {cwd} ) : null; - const text = runsInBackground + const text = background ? t("toolInvocation.backgroundExecute") - : t("toolInvocation.executeCommand"); + : isPromoted + ? t("toolInvocation.startedCommand") + : t("toolInvocation.executeCommand"); const title = ( <> {text} {cwdNode} + {isPromoted && ( + + {t("toolInvocation.promotedToBackground")} + + )} ); @@ -56,7 +63,7 @@ export const executeCommandTool: React.FC> = ({ throw new Error("Unexpected streaming result for executeCommand tool"); } - if (runsInBackground) { + if (background || isPromoted) { const availableCommand = tool.state === "input-available" || tool.state === "output-available" ? tool.input.command diff --git a/packages/vscode-webui/src/i18n/locales/en.json b/packages/vscode-webui/src/i18n/locales/en.json index 8372b62d66..cb391a79d3 100644 --- a/packages/vscode-webui/src/i18n/locales/en.json +++ b/packages/vscode-webui/src/i18n/locales/en.json @@ -350,6 +350,7 @@ "in": "in", "executeCommand": "I will execute the following command", "executingCommand": "Executing command", + "startedCommand": "Started command", "for": "for", "matching": "matching", "searching": "Searching", @@ -366,6 +367,7 @@ "readTerminal": "Reading terminal output", "backgroundExecute": "I will execute the following command in the background", "backgroundExecuting": "Executing command in background", + "promotedToBackground": "; continued in background", "updatingToDos": "Updating TODOs", "editing": "Editing ", "usingSkill": "Using Skill", diff --git a/packages/vscode-webui/src/i18n/locales/jp.json b/packages/vscode-webui/src/i18n/locales/jp.json index 9d80a08f52..626ce66bd2 100644 --- a/packages/vscode-webui/src/i18n/locales/jp.json +++ b/packages/vscode-webui/src/i18n/locales/jp.json @@ -346,6 +346,7 @@ "in": "の", "executeCommand": "以下のコマンドを実行します", "executingCommand": "コマンドを実行中", + "startedCommand": "コマンドを開始", "for": "で", "matching": "一致する", "searching": "検索中", @@ -362,6 +363,7 @@ "readTerminal": "ターミナル出力を読み込み中", "backgroundExecute": "以下のコマンドをバックグラウンドで実行します", "backgroundExecuting": "バックグラウンドでコマンドを実行中", + "promotedToBackground": "、その後バックグラウンドで継続", "updatingToDos": "TODOを更新中", "editing": "編集しています ", "moreTools": "、他{{count}}個のツール", diff --git a/packages/vscode-webui/src/i18n/locales/ko.json b/packages/vscode-webui/src/i18n/locales/ko.json index 2234b732c1..03c94b7a16 100644 --- a/packages/vscode-webui/src/i18n/locales/ko.json +++ b/packages/vscode-webui/src/i18n/locales/ko.json @@ -344,6 +344,7 @@ "in": "에서", "executeCommand": "다음 명령을 실행합니다", "executingCommand": "명령 실행 중", + "startedCommand": "명령 시작", "for": "대상", "matching": "매칭", "searching": "검색 중", @@ -360,6 +361,7 @@ "readTerminal": "터미널 출력 읽는 중", "backgroundExecute": "다음 명령을 백그라운드에서 실행합니다", "backgroundExecuting": "백그라운드에서 명령 실행 중", + "promotedToBackground": ", 이후 백그라운드에서 계속 실행", "updatingToDos": "할 일 업데이트 중", "editing": "편집하는 중 ", "moreTools": ", 그리고 {{count}}개의 다른 도구들", diff --git a/packages/vscode-webui/src/i18n/locales/zh.json b/packages/vscode-webui/src/i18n/locales/zh.json index afbb559c07..b4edca98d5 100644 --- a/packages/vscode-webui/src/i18n/locales/zh.json +++ b/packages/vscode-webui/src/i18n/locales/zh.json @@ -344,6 +344,7 @@ "in": "在", "executeCommand": "我将执行以下命令", "executingCommand": "正在执行命令", + "startedCommand": "已启动命令", "for": "匹配", "matching": "匹配", "searching": "搜索中", @@ -360,6 +361,7 @@ "readTerminal": "正在读取终端输出", "backgroundExecute": "我将在后台执行以下命令", "backgroundExecuting": "正在后台执行命令", + "promotedToBackground": "；随后转入后台", "updatingToDos": "更新待办事项中", "editing": "正在编辑 ", "moreTools": "，以及另外 {{count}} 个工具", From 8e18611cf336d69811c76b9898a8dffbd4328ebc Mon Sep 17 00:00:00 2001 From: zhanba Date: Thu, 3 Sep 2026 21:21:10 +0800 Subject: [PATCH 03/12] feat(cli): promote timed out commands to background jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the original non-interactive process running after the foreground wait expires and clarify timeout behavior across CLI and VS Code. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../cli/src/lib/background-job-manager.ts | 130 ++++++--- .../tools/__tests__/execute-command.test.ts | 63 +++++ packages/cli/src/tools/execute-command.ts | 252 +++++++++++++----- packages/tools/src/execute-command.ts | 3 +- 4 files changed, 333 insertions(+), 115 deletions(-) diff --git a/packages/cli/src/lib/background-job-manager.ts b/packages/cli/src/lib/background-job-manager.ts index 0add65a521..2cd5e0dbb2 100644 --- a/packages/cli/src/lib/background-job-manager.ts +++ b/packages/cli/src/lib/background-job-manager.ts @@ -1,6 +1,7 @@ import { type ChildProcess, spawn } from "node:child_process"; import { tmpdir } from "node:os"; import path from "node:path"; +import type { Readable } from "node:stream"; import { StringDecoder } from "node:string_decoder"; import type { BackgroundJobTerminalEvent } from "@getpochi/common"; import { assertBackgroundJobReadInterval } from "@getpochi/common"; @@ -25,6 +26,7 @@ export interface BackgroundJob { lastReadAt?: number; stopRequested?: boolean; finalizing?: boolean; + disposeAbort?: () => void; } export interface BackgroundJobStartResult { @@ -32,6 +34,11 @@ export interface BackgroundJobStartResult { outputFile: string; } +export interface BackgroundJobInitialOutput { + stdout: Buffer[]; + stderr: Buffer[]; +} + export interface BackgroundJobManagerOptions { taskId?: string; outputDir?: string; @@ -50,6 +57,31 @@ export class BackgroundJobManager { command: string, cwd: string, envs?: Record, + ): BackgroundJobStartResult { + const child = spawn(command, { + shell: getShellPath(), + cwd, + env: { ...process.env, ...getTerminalEnv(), ...envs }, + stdio: ["ignore", "pipe", "pipe"], + }); + + return this.register(child, command); + } + + adopt( + child: ChildProcess, + command: string, + initialOutput: BackgroundJobInitialOutput, + abortSignal?: AbortSignal, + ): BackgroundJobStartResult { + return this.register(child, command, initialOutput, abortSignal); + } + + private register( + child: ChildProcess, + command: string, + initialOutput: BackgroundJobInitialOutput = { stdout: [], stderr: [] }, + abortSignal?: AbortSignal, ): BackgroundJobStartResult { const id = createBackgroundJobId("command"); const outputFile = this.options.outputDir @@ -58,15 +90,6 @@ export class BackgroundJobManager { ? getBackgroundJobOutputPath(this.options.taskId, id) : path.join(tmpdir(), "pochi-background-jobs", `${id}.log`); const outputWriter = new BackgroundJobOutputFile(outputFile); - - const shell = getShellPath(); - const child = spawn(command, { - shell, - cwd, - env: { ...process.env, ...getTerminalEnv(), ...envs }, - stdio: ["ignore", "pipe", "pipe"], - }); - const job: BackgroundJob = { id, command, @@ -80,47 +103,70 @@ export class BackgroundJobManager { this.jobs.set(id, job); - const appendOutput = async (chunk: string) => { - if (chunk.length === 0) return; - await outputWriter.append(chunk); - if (job.output.length + chunk.length > this.maxOutputSize) { - const keep = this.maxOutputSize - chunk.length; - if (keep > 0) { - job.output = job.output.slice(-keep) + chunk; + let appendTail = Promise.resolve(); + const appendOutput = (chunk: string): Promise => { + appendTail = appendTail.then(async () => { + if (chunk.length === 0) return; + await outputWriter.append(chunk); + if (job.output.length + chunk.length > this.maxOutputSize) { + const keep = this.maxOutputSize - chunk.length; + if (keep > 0) { + job.output = job.output.slice(-keep) + chunk; + } else { + job.output = chunk.slice(-this.maxOutputSize); + } } else { - job.output = chunk.slice(-this.maxOutputSize); + job.output += chunk; + } + }); + return appendTail; + }; + + const consumeOutput = async ( + stream: Readable | null, + initialChunks: Buffer[], + ) => { + const decoder = new StringDecoder("utf8"); + const sanitizer = new PlainOutputSanitizer(); + for (const chunk of initialChunks) { + await appendOutput(sanitizer.write(decoder.write(chunk))); + } + if (stream) { + for await (const chunk of stream) { + await appendOutput(sanitizer.write(decoder.write(chunk))); } - } else { - job.output += chunk; } + + // StringDecoder buffers an incomplete trailing UTF-8 sequence. A + // manually stopped process may end in the middle of a character, so + // discard that partial sequence instead of flushing it as U+FFFD. + if (!job.stopRequested) { + await appendOutput(sanitizer.write(decoder.end())); + } + await appendOutput(sanitizer.end()); }; let outputError: unknown; - const outputFinished = Promise.all( - [child.stdout, child.stderr] - .filter((stream) => stream !== null) - .map(async (stream) => { - const decoder = new StringDecoder("utf8"); - const sanitizer = new PlainOutputSanitizer(); - for await (const chunk of stream) { - await appendOutput(sanitizer.write(decoder.write(chunk))); - } - - // StringDecoder buffers an incomplete trailing UTF-8 sequence. A - // manually stopped process may end in the middle of a character, so - // discard that partial sequence instead of flushing it as U+FFFD. - // For a naturally completed process, preserve Node's usual behavior - // for genuinely malformed output by flushing the decoder. - if (!job.stopRequested) { - await appendOutput(sanitizer.write(decoder.end())); - } - await appendOutput(sanitizer.end()); - }), - ).catch((error) => { + const outputFinished = Promise.all([ + consumeOutput(child.stdout, initialOutput.stdout), + consumeOutput(child.stderr, initialOutput.stderr), + ]).catch((error) => { outputError = error; child.kill(); }); + if (abortSignal) { + const onAbort = () => { + if (job.status !== "running" || job.finalizing) return; + job.stopRequested = true; + child.kill(); + }; + abortSignal.addEventListener("abort", onAbort, { once: true }); + job.disposeAbort = () => + abortSignal.removeEventListener("abort", onAbort); + if (abortSignal.aborted) onAbort(); + } + child.on("close", async (code) => { const status = job.stopRequested ? "stopped" @@ -146,6 +192,9 @@ export class BackgroundJobManager { await this.finalize(job, "failed", undefined, error.message); }); + child.stdout?.resume(); + child.stderr?.resume(); + return { backgroundJobId: id, outputFile }; } @@ -172,6 +221,7 @@ export class BackgroundJobManager { finalError = closeError instanceof Error ? closeError.message : String(closeError); } + job.disposeAbort?.(); job.status = finalStatus; job.finalizing = false; diff --git a/packages/cli/src/tools/__tests__/execute-command.test.ts b/packages/cli/src/tools/__tests__/execute-command.test.ts index 75c68b7af0..eecc91b038 100644 --- a/packages/cli/src/tools/__tests__/execute-command.test.ts +++ b/packages/cli/src/tools/__tests__/execute-command.test.ts @@ -1,5 +1,9 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { getToolRules } from "@getpochi/tools"; import { describe, expect, it } from "vitest"; +import { BackgroundJobManager } from "../../lib/background-job-manager"; import { executeCommand } from "../execute-command"; describe("executeCommand", () => { @@ -29,6 +33,37 @@ describe("executeCommand", () => { ).rejects.toThrow("Command execution timed out after 1 seconds"); }); + it("should continue the same process as a background job after timeout", async () => { + const outputDir = await mkdtemp(join(tmpdir(), "pochi-cli-promotion-")); + try { + const backgroundJobManager = new BackgroundJobManager({ outputDir }); + const script = [ + "process.stdout.write('start:' + process.pid + ';');", + "setTimeout(() => process.stdout.write('finish:' + process.pid + ';'), 1200);", + ].join(""); + const result = await executeCommand({ backgroundJobManager })( + { + command: `${JSON.stringify(process.execPath)} -e ${JSON.stringify(script)}`, + timeout: 1, + }, + mockToolExecutionOptions, + ); + + expect(result._meta?.backgroundJobId).toMatch(/^bgjob-cmd-/); + expect(result._meta?.outputFile).toBeDefined(); + expect(await backgroundJobManager.waitForAllJobs(5000)).toBe( + "completed", + ); + + const output = await readFile(result._meta?.outputFile ?? "", "utf8"); + const match = output.match(/^start:(\d+);finish:(\d+);$/); + expect(match).not.toBeNull(); + expect(match?.[1]).toBe(match?.[2]); + } finally { + await rm(outputDir, { recursive: true, force: true }); + } + }); + it("should handle abort signal", async () => { const abortController = new AbortController(); const options = { @@ -48,6 +83,34 @@ describe("executeCommand", () => { await expect(promise).rejects.toThrow("Command execution was aborted"); }); + it("should stop a promoted background job when aborted", async () => { + const outputDir = await mkdtemp(join(tmpdir(), "pochi-cli-abort-")); + try { + const backgroundJobManager = new BackgroundJobManager({ + taskId: "test-task", + outputDir, + }); + const abortController = new AbortController(); + const eventPromise = new Promise< + Parameters[0]>[0] + >((resolve) => backgroundJobManager.onDidFinish(resolve)); + + const result = await executeCommand({ backgroundJobManager })( + { command: "sleep 10", timeout: 1 }, + { + ...mockToolExecutionOptions, + abortSignal: abortController.signal, + }, + ); + abortController.abort(); + + expect(result._meta?.backgroundJobId).toMatch(/^bgjob-cmd-/); + expect((await eventPromise).status).toBe("stopped"); + } finally { + await rm(outputDir, { recursive: true, force: true }); + } + }); + it("should use default timeout when not specified", async () => { // This test just ensures the default timeout doesn't interfere with quick commands const result = await executeCommand()( diff --git a/packages/cli/src/tools/execute-command.ts b/packages/cli/src/tools/execute-command.ts index 228b79e2ac..f0167baee4 100644 --- a/packages/cli/src/tools/execute-command.ts +++ b/packages/cli/src/tools/execute-command.ts @@ -1,8 +1,4 @@ -import { - type ExecException, - type ExecOptionsWithStringEncoding, - exec, -} from "node:child_process"; +import { spawn } from "node:child_process"; import * as path from "node:path"; import { getTerminalEnv } from "@getpochi/common/env-utils"; import { @@ -16,7 +12,11 @@ import { type ToolFunctionType, createBackgroundCommandResult, } from "@getpochi/tools"; -import type { ToolCallOptions } from "../types"; +import type { BackgroundJobManager } from "../lib/background-job-manager"; + +interface ExecuteCommandContext { + backgroundJobManager?: BackgroundJobManager; +} export class ExecuteCommandError extends Error { public code: number; @@ -43,7 +43,7 @@ export class ExecuteCommandError extends Error { export const executeCommand = ( - context?: ToolCallOptions, + context?: ExecuteCommandContext, ): ToolFunctionType => async ( { @@ -75,94 +75,198 @@ export const executeCommand = } try { - const { stdout = "", stderr = "" } = await execWithExitCode(command, { - shell: getShellPath(), - timeout: timeout * 1000, // Convert to milliseconds + const result = await executeForegroundCommand({ + command, cwd: resolvedCwd, - signal: abortSignal, - env: { ...process.env, ...envs, ...getTerminalEnv() }, + envs, + timeout, + abortSignal, + backgroundJobManager: context?.backgroundJobManager, }); - return processCommandOutput(stdout, stderr); + if ("backgroundJobId" in result) { + return createBackgroundCommandResult( + result.backgroundJobId, + result.outputFile, + ); + } + + return processCommandOutput(result.stdout, result.stderr); } catch (error) { if (error instanceof ExecuteCommandError) { throw error; } - // Handle abort signal if (error instanceof Error && error.name === "AbortError") { throw new Error("Command execution was aborted"); } - // Handle other execution errors const errorMessage = error instanceof Error ? error.message : String(error); throw new Error(errorMessage); } }; -function isExecException(error: unknown): error is ExecException { - return ( - error instanceof Error && - "cmd" in error && - "killed" in error && - "code" in error && - "signal" in error - ); +interface ExecuteForegroundCommandOptions { + command: string; + cwd: string; + envs?: Record; + timeout: number; + abortSignal?: AbortSignal; + backgroundJobManager?: BackgroundJobManager; } -async function execWithExitCode( - command: string, - options: ExecOptionsWithStringEncoding, -): Promise<{ stdout: string; stderr: string; code: 0 }> { - return await new Promise<{ stdout: string; stderr: string; code: 0 }>( - (resolve, reject) => { - const child = exec(command, options, (err, stdout = "", stderr = "") => { - if (!err) { - resolve({ - stdout, - stderr, - code: 0, - }); - return; - } - - if (isExecException(err)) { - if ( - err.signal === "SIGTERM" && - err.killed && - options.timeout && - options.timeout > 0 - ) { - reject( - new ExecuteCommandError({ - message: `Command execution timed out after ${options.timeout / 1000} seconds.`, - stdout: err.stdout ?? stdout ?? "", - stderr: err.stderr ?? stderr ?? "", - code: err.code ?? 1, - }), - ); - return; - } - - reject( - new ExecuteCommandError({ - message: `Command exited with code ${err.code ?? 1}`, - stdout: err.stdout ?? stdout ?? "", - stderr: err.stderr ?? stderr ?? "", - code: err.code ?? 1, - }), - ); - return; - } - - reject(err); - }); +interface CompletedCommandResult { + stdout: string; + stderr: string; +} - // Close stdin to force non-interactive behavior and avoid hanging prompts. - child.stdin?.end(); - }, - ); +interface PromotedCommandResult { + backgroundJobId: string; + outputFile: string; +} + +function executeForegroundCommand({ + command, + cwd, + envs, + timeout, + abortSignal, + backgroundJobManager, +}: ExecuteForegroundCommandOptions): Promise< + CompletedCommandResult | PromotedCommandResult +> { + return new Promise((resolve, reject) => { + const child = spawn(command, { + shell: getShellPath(), + cwd, + env: { ...process.env, ...envs, ...getTerminalEnv() }, + stdio: ["ignore", "pipe", "pipe"], + }); + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + let state: "foreground" | "promoted" | "settled" = "foreground"; + let stopReason: "abort" | "timeout" | undefined; + + const onStdout = (chunk: Buffer | string) => { + stdoutChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + }; + const onStderr = (chunk: Buffer | string) => { + stderrChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + }; + child.stdout.on("data", onStdout); + child.stderr.on("data", onStderr); + + const getOutput = () => ({ + stdout: Buffer.concat(stdoutChunks).toString("utf8"), + stderr: Buffer.concat(stderrChunks).toString("utf8"), + }); + + let timeoutHandle: ReturnType | undefined; + const removeForegroundListeners = () => { + if (timeoutHandle) clearTimeout(timeoutHandle); + abortSignal?.removeEventListener("abort", onAbort); + child.stdout.removeListener("data", onStdout); + child.stderr.removeListener("data", onStderr); + child.removeListener("close", onClose); + child.removeListener("error", onError); + }; + + const settleStoppedCommand = () => { + if (state !== "foreground") return; + state = "settled"; + removeForegroundListeners(); + if (stopReason === "abort") { + reject(new DOMException("Command execution was aborted", "AbortError")); + return; + } + + const output = getOutput(); + reject( + new ExecuteCommandError({ + message: `Command execution timed out after ${timeout} seconds.`, + ...output, + code: 1, + }), + ); + }; + + const onClose = (code: number | null) => { + if (stopReason) { + settleStoppedCommand(); + return; + } + if (state !== "foreground") return; + state = "settled"; + removeForegroundListeners(); + const output = getOutput(); + if (code === 0) { + resolve(output); + return; + } + reject( + new ExecuteCommandError({ + message: `Command exited with code ${code ?? 1}`, + ...output, + code: code ?? 1, + }), + ); + }; + + const onError = (error: Error) => { + if (state !== "foreground") return; + if (stopReason) { + settleStoppedCommand(); + return; + } + state = "settled"; + removeForegroundListeners(); + reject(error); + }; + + function onAbort() { + if (state !== "foreground") return; + stopReason = "abort"; + if (!child.kill()) settleStoppedCommand(); + } + + const onTimeout = () => { + if (state !== "foreground") return; + if (!backgroundJobManager) { + stopReason = "timeout"; + if (!child.kill()) settleStoppedCommand(); + return; + } + + state = "promoted"; + child.stdout.pause(); + child.stderr.pause(); + removeForegroundListeners(); + try { + resolve( + backgroundJobManager.adopt( + child, + command, + { stdout: stdoutChunks, stderr: stderrChunks }, + abortSignal, + ), + ); + } catch (error) { + state = "settled"; + child.kill(); + reject(error); + } + }; + + child.on("close", onClose); + child.on("error", onError); + abortSignal?.addEventListener("abort", onAbort, { once: true }); + if (abortSignal?.aborted) { + onAbort(); + } else { + timeoutHandle = setTimeout(onTimeout, timeout * 1000); + } + }); } function processCommandOutput( diff --git a/packages/tools/src/execute-command.ts b/packages/tools/src/execute-command.ts index 751415fa0d..d7feda8013 100644 --- a/packages/tools/src/execute-command.ts +++ b/packages/tools/src/execute-command.ts @@ -59,7 +59,8 @@ Before executing the command, please follow these steps: Usage notes: - The command argument is required. ${backgroundUsageNotes} -- For foreground commands, you can specify an optional timeout in seconds (up to 300s). If not specified, the foreground wait is ${ExecuteCommandDefaultTimeoutSec}s. On supported interactive hosts, a command still running at that point is moved to a background terminal instead of being stopped; its result includes the background job metadata. Other hosts stop the command and report a timeout. +- For foreground commands, you can specify an optional timeout in seconds (up to 300s). If not specified, the foreground wait is ${ExecuteCommandDefaultTimeoutSec}s. +- When the foreground timeout expires, the same process continues as a background job without being stopped or restarted, and the result includes its \`backgroundJobId\` and \`outputFile\`. CLI background jobs are non-interactive; in a VS Code task on macOS or Linux, the job also continues in an interactive terminal. If background promotion is unavailable, including in VS Code on Windows, the command is stopped and a timeout error is returned. - If the output exceeds 30000 characters, output will be truncated before being returned to you. - When issuing multiple commands: - If the commands are independent and can run in parallel, make multiple executeCommand tool calls in a single message. For example, if you need to run "git status" and "git diff", send a single message with two executeCommand tool calls in parallel. From d089adcd8a4de63778887a0d1069a9d9e3576e7e Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 11:37:09 +0800 Subject: [PATCH 04/12] feat(vscode): detach background command terminal views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep managed commands running when their terminal view is hidden or closed, while exposing reactive show, hide, and close controls to the webview. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- packages/common/src/base/environment.ts | 4 +- .../src/vscode-webui-bridge/webview-stub.ts | 8 ++ .../common/src/vscode-webui-bridge/webview.ts | 7 + .../__tests__/use-background-command.test.tsx | 66 +++++++++ .../src/lib/hooks/use-background-command.ts | 33 +++++ packages/vscode-webui/src/lib/vscode.ts | 1 + .../terminal/__test__/pty-process.test.ts | 13 ++ .../terminal/__test__/pty-terminal.test.ts | 12 +- .../terminal/__test__/terminal-job.test.ts | 78 +++++++++- .../src/integrations/terminal/pty-process.ts | 20 ++- .../src/integrations/terminal/pty-terminal.ts | 1 + .../src/integrations/terminal/terminal-job.ts | 136 ++++++++++++++---- .../integrations/terminal/terminal-state.ts | 75 ++++++++-- .../integrations/webview/vscode-host-impl.ts | 17 +++ 14 files changed, 413 insertions(+), 58 deletions(-) create mode 100644 packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx create mode 100644 packages/vscode-webui/src/lib/hooks/use-background-command.ts diff --git a/packages/common/src/base/environment.ts b/packages/common/src/base/environment.ts index 76b3e1acdc..86be522297 100644 --- a/packages/common/src/base/environment.ts +++ b/packages/common/src/base/environment.ts @@ -70,7 +70,9 @@ export const Environment = z.object({ }), ) .optional() - .describe("Visible terminals in the VS Code workspace."), + .describe( + "Available terminals and detachable background command sessions in the VS Code workspace.", + ), }) .describe("Information about the workspace."), info: z diff --git a/packages/common/src/vscode-webui-bridge/webview-stub.ts b/packages/common/src/vscode-webui-bridge/webview-stub.ts index 5efc3dd1d0..e3f05215e6 100644 --- a/packages/common/src/vscode-webui-bridge/webview-stub.ts +++ b/packages/common/src/vscode-webui-bridge/webview-stub.ts @@ -289,6 +289,14 @@ const VSCodeHostStub = { }, }); }, + readBackgroundCommand: async (_backgroundJobId: string) => { + return Promise.resolve({ + isVisible: {} as ThreadSignalSerialization, + show: async (): Promise => Promise.resolve(), + hide: async (): Promise => Promise.resolve(), + close: async (): Promise => Promise.resolve(), + }); + }, readModelList: async () => { return Promise.resolve( {} as { diff --git a/packages/common/src/vscode-webui-bridge/webview.ts b/packages/common/src/vscode-webui-bridge/webview.ts index 2182344566..0f39216350 100644 --- a/packages/common/src/vscode-webui-bridge/webview.ts +++ b/packages/common/src/vscode-webui-bridge/webview.ts @@ -188,6 +188,13 @@ export interface VSCodeHostApi { openBackgroundJobTerminal: (backgroundJobId: string) => Promise; }>; + readBackgroundCommand(backgroundJobId: string): Promise<{ + isVisible: ThreadSignalSerialization; + show: () => Promise; + hide: () => Promise; + close: () => Promise; + }>; + readBackgroundJobNotifications(taskId: string): Promise<{ notifications: ThreadSignalSerialization; acknowledge: (notificationId: string) => Promise; diff --git a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx new file mode 100644 index 0000000000..03c38b5e6b --- /dev/null +++ b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx @@ -0,0 +1,66 @@ +// @vitest-environment jsdom + +import { signal } from "@preact/signals-core"; +import { useQuery } from "@tanstack/react-query"; +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useBackgroundCommand } from "../use-background-command"; + +vi.mock("@tanstack/react-query", () => ({ + useQuery: vi.fn(), +})); + +vi.mock("../../vscode", () => ({ + vscodeHost: { + readBackgroundCommand: vi.fn(), + }, +})); + +describe("useBackgroundCommand", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns reactive visibility and background command controls", async () => { + const isVisible = signal(false); + const show = vi.fn(); + const hide = vi.fn(); + const close = vi.fn(); + vi.mocked(useQuery).mockReturnValue({ + data: { + isVisible, + show, + hide, + close, + }, + } as never); + + const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); + + expect(result.current.isVisible).toBe(false); + + act(() => { + isVisible.value = true; + }); + expect(result.current.isVisible).toBe(true); + + await act(async () => result.current.show?.()); + await act(async () => result.current.hide?.()); + await act(async () => result.current.close?.()); + + expect(show).toHaveBeenCalledOnce(); + expect(hide).toHaveBeenCalledOnce(); + expect(close).toHaveBeenCalledOnce(); + }); + + it("returns undefined state while the command is loading", () => { + vi.mocked(useQuery).mockReturnValue({ data: undefined } as never); + + const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); + + expect(result.current.isVisible).toBeUndefined(); + expect(result.current.show).toBeUndefined(); + expect(result.current.hide).toBeUndefined(); + expect(result.current.close).toBeUndefined(); + }); +}); diff --git a/packages/vscode-webui/src/lib/hooks/use-background-command.ts b/packages/vscode-webui/src/lib/hooks/use-background-command.ts new file mode 100644 index 0000000000..5e52248736 --- /dev/null +++ b/packages/vscode-webui/src/lib/hooks/use-background-command.ts @@ -0,0 +1,33 @@ +import { vscodeHost } from "@/lib/vscode"; +import { threadSignal } from "@quilted/threads/signals"; +import { useQuery } from "@tanstack/react-query"; + +/** + * Controls the detachable terminal view for a running background command. + * Hiding the terminal does not stop the command. + * @useSignals this comment is needed to enable signals in this hook + */ +export const useBackgroundCommand = (backgroundJobId: string) => { + const { data } = useQuery({ + queryKey: ["backgroundCommand", backgroundJobId], + queryFn: () => fetchBackgroundCommand(backgroundJobId), + staleTime: Number.POSITIVE_INFINITY, + }); + + return { + isVisible: data?.isVisible.value, + show: data?.show, + hide: data?.hide, + close: data?.close, + }; +}; + +async function fetchBackgroundCommand(backgroundJobId: string) { + const result = await vscodeHost.readBackgroundCommand(backgroundJobId); + return { + isVisible: threadSignal(result.isVisible), + show: result.show, + hide: result.hide, + close: result.close, + }; +} diff --git a/packages/vscode-webui/src/lib/vscode.ts b/packages/vscode-webui/src/lib/vscode.ts index 32b21e18e3..5e5e9a43a6 100644 --- a/packages/vscode-webui/src/lib/vscode.ts +++ b/packages/vscode-webui/src/lib/vscode.ts @@ -133,6 +133,7 @@ function createVSCodeHost(): VSCodeHostApi { "showInformationMessage", "showWarningMessage", "readVisibleTerminals", + "readBackgroundCommand", "readModelList", "readUserStorage", "readCustomAgents", diff --git a/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts index 427f0e3c93..a79fde2774 100644 --- a/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts @@ -72,6 +72,19 @@ describe("PtyProcess", () => { assert.deepStrictEqual(events, ["data:trailing output\n", "exit:0"]); }); + it("bounds replay history while retaining the latest output", () => { + const harness = createHarness(); + harness.data("a".repeat(600_000)); + harness.data("b".repeat(600_000)); + + const subscription = harness.ptyProcess.subscribeWithReplay(() => {}); + const replay = subscription.replay.join(""); + + assert.strictEqual(replay.length, 1_000_000); + assert.strictEqual(replay, `${"a".repeat(400_000)}${"b".repeat(600_000)}`); + subscription.disposable.dispose(); + }); + it("allows late exit delivery to be cancelled", async () => { const harness = createHarness(); const exits: number[] = []; diff --git a/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts b/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts index 4744e7f13b..70dcffafee 100644 --- a/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts @@ -68,16 +68,19 @@ describe("PtyTerminal", () => { assert.strictEqual(onCloseRequested.callCount, 0); }); - it("requests a stop when the user closes a running terminal", () => { + it("detaches without stopping the underlying process", () => { const onCloseRequested = sinon.stub(); + const dataSubscription = { dispose: sinon.stub() }; + const exitSubscription = { dispose: sinon.stub() }; const ptyProcess = { subscribeWithReplay: () => ({ replay: [], - disposable: { dispose: sinon.stub() }, + disposable: dataSubscription, }), - onExit: () => ({ dispose: sinon.stub() }), + onExit: () => exitSubscription, write: sinon.stub(), resize: sinon.stub(), + kill: sinon.stub(), }; const { PtyTerminal } = proxyquire .noCallThru() @@ -89,5 +92,8 @@ describe("PtyTerminal", () => { new PtyTerminal(ptyProcess as never, onCloseRequested).close(); assert.strictEqual(onCloseRequested.callCount, 1); + assert.strictEqual(ptyProcess.kill.callCount, 0); + assert.strictEqual(dataSubscription.dispose.callCount, 1); + assert.strictEqual(exitSubscription.dispose.callCount, 1); }); }); diff --git a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts index dbc9b93c8e..4c35ab674d 100644 --- a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts @@ -39,10 +39,6 @@ class TestPtyProcess { return { dispose: () => this.exitListeners.delete(listener) }; } - clearReplay(): void { - this.replay.length = 0; - } - emitData(data: string): void { this.replay.push(data); for (const listener of [...this.dataListeners]) listener(data); @@ -83,8 +79,13 @@ function createHarness(options?: { }) { const closeEmitter = new TestEventEmitter(); let terminalDisposeCalls = 0; + let terminalShowCalls = 0; + const terminalShowPreserveFocus: Array = []; const terminal: FakeTerminal = { - show: () => {}, + show: (preserveFocus) => { + terminalShowCalls++; + terminalShowPreserveFocus.push(preserveFocus); + }, dispose: () => { terminalDisposeCalls++; closeEmitter.fire(terminal); @@ -92,6 +93,7 @@ function createHarness(options?: { }; const lifecycle: string[] = []; const finalizeCalls: Array = []; + const ptyTerminalCloseCallbacks: Array<() => void> = []; const ptyProcess = new TestPtyProcess(); ptyProcess.replay.push(...(options?.replay ?? [])); @@ -154,7 +156,11 @@ function createHarness(options?: { "./pty-terminal": { PtyTerminal: class { private readonly exitSubscription: Disposable; - constructor(process: TestPtyProcess) { + constructor( + process: TestPtyProcess, + onCloseRequested: () => void, + ) { + ptyTerminalCloseCallbacks.push(onCloseRequested); this.exitSubscription = process.onExit(() => { closeEmitter.fire(terminal); }); @@ -193,15 +199,20 @@ function createHarness(options?: { job: job as ReturnType, lifecycle, ptyProcess, + ptyTerminalCloseCallbacks, terminal, get terminalDisposeCalls() { return terminalDisposeCalls; }, + get terminalShowCalls() { + return terminalShowCalls; + }, + terminalShowPreserveFocus, }; } interface FakeTerminal { - show(): void; + show(preserveFocus?: boolean): void; dispose(): void; } @@ -340,6 +351,58 @@ describe("TerminalJob", () => { assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); }); + it("detaches and recreates the terminal without stopping the pty", async () => { + const harness = createHarness(); + assert.strictEqual(harness.job.isVisible, true); + assert.strictEqual(harness.terminalShowCalls, 1); + assert.deepStrictEqual(harness.terminalShowPreserveFocus, [false]); + + harness.job.hide(); + + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.ptyProcess.killCalls, 0); + assert.strictEqual(harness.terminalDisposeCalls, 1); + + harness.job.show(); + + assert.strictEqual(harness.job.isVisible, true); + assert.strictEqual(harness.ptyProcess.killCalls, 0); + assert.strictEqual(harness.terminalShowCalls, 2); + assert.deepStrictEqual(harness.terminalShowPreserveFocus, [false, false]); + + harness.ptyTerminalCloseCallbacks[0]?.(); + assert.strictEqual(harness.job.isVisible, true); + + harness.ptyProcess.emitExit(0); + await flushPromises(); + }); + + it("keeps the pty running when the VS Code terminal is closed", async () => { + const harness = createHarness(); + + harness.terminal.dispose(); + + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.ptyProcess.killCalls, 0); + + harness.ptyProcess.emitExit(0); + await flushPromises(); + }); + + it("closes the terminal when explicitly closing the pty process", async () => { + const harness = createHarness(); + + harness.job.closePtyProcess(); + + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.terminalDisposeCalls, 1); + assert.strictEqual(harness.ptyProcess.killCalls, 1); + + harness.ptyProcess.emitExit(143); + await flushPromises(); + assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); + }); + it("marks a killed command as stopped", async () => { const harness = createHarness(); harness.job.kill(); @@ -349,6 +412,7 @@ describe("TerminalJob", () => { await flushPromises(); assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); + assert.strictEqual(harness.terminalDisposeCalls, 1); }); it("marks a nonzero natural exit as failed", async () => { diff --git a/packages/vscode/src/integrations/terminal/pty-process.ts b/packages/vscode/src/integrations/terminal/pty-process.ts index 9486a70782..345a8ce929 100644 --- a/packages/vscode/src/integrations/terminal/pty-process.ts +++ b/packages/vscode/src/integrations/terminal/pty-process.ts @@ -9,6 +9,7 @@ import * as vscode from "vscode"; const logger = getLogger("PtyProcess"); const TerminationGraceMs = 2_000; const HardKillExitGraceMs = 1_000; +const ReplayHistoryMaxCharacters = 1_000_000; const requireFromExtensionHost = createRequire(__filename); export class PtySpawnError extends Error { @@ -65,6 +66,7 @@ export class PtyProcess { private readonly dataListeners = new Set(); private readonly exitListeners = new Set(); private readonly history: string[] = []; + private historyCharacters = 0; private rawExitEvent: PtyProcessExit | undefined; private exitEvent: PtyProcessExit | undefined; private forceKillTimer: ReturnType | undefined; @@ -73,7 +75,7 @@ export class PtyProcess { private constructor(private readonly process: nodePty.IPty) { process.onData((data) => { - this.history.push(data); + this.appendHistory(data); for (const listener of this.dataListeners) { listener(data); } @@ -134,8 +136,20 @@ export class PtyProcess { return { replay, disposable }; } - clearReplay(): void { - this.history.length = 0; + private appendHistory(data: string): void { + this.history.push(data); + this.historyCharacters += data.length; + while (this.historyCharacters > ReplayHistoryMaxCharacters) { + const firstChunk = this.history[0] ?? ""; + const overflow = this.historyCharacters - ReplayHistoryMaxCharacters; + if (firstChunk.length <= overflow) { + this.history.shift(); + this.historyCharacters -= firstChunk.length; + } else { + this.history[0] = firstChunk.slice(overflow); + this.historyCharacters -= overflow; + } + } } /** Fires at node-pty's socket-close boundary, after queued output drains. */ diff --git a/packages/vscode/src/integrations/terminal/pty-terminal.ts b/packages/vscode/src/integrations/terminal/pty-terminal.ts index c7288de4ba..ba8cc0bb1c 100644 --- a/packages/vscode/src/integrations/terminal/pty-terminal.ts +++ b/packages/vscode/src/integrations/terminal/pty-terminal.ts @@ -44,6 +44,7 @@ export class PtyTerminal implements vscode.Pseudoterminal, vscode.Disposable { } close(): void { + this.opened = false; if (!this.exited) { this.onCloseRequested(); } diff --git a/packages/vscode/src/integrations/terminal/terminal-job.ts b/packages/vscode/src/integrations/terminal/terminal-job.ts index ff33fd9599..ff8de6e0f0 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -8,6 +8,7 @@ import { getBackgroundJobOutputPath, getShellPath, } from "@getpochi/common/tool-utils"; +import { signal } from "@preact/signals-core"; import * as vscode from "vscode"; import { createTerminal } from "../layout"; import { OutputManager } from "./output"; @@ -35,8 +36,13 @@ export class TerminalJob implements vscode.Disposable { private static readonly onDidFinishEmitter = new vscode.EventEmitter(); static readonly onDidFinish = TerminalJob.onDidFinishEmitter.event; + private static readonly onDidChangeVisibilityEmitter = + new vscode.EventEmitter(); + static readonly onDidChangeVisibility = + TerminalJob.onDidChangeVisibilityEmitter.event; - private terminal!: vscode.Terminal; + private terminal: vscode.Terminal | undefined; + private ptyTerminal: PtyTerminal | undefined; private outputManager!: OutputManager; private outputWriter!: BackgroundJobOutputFile; private readonly sanitizer = new PlainOutputSanitizer(); @@ -56,6 +62,7 @@ export class TerminalJob implements vscode.Disposable { readonly id: string; readonly outputFile: string; + readonly terminalVisibility = signal(false); get output() { return this.outputManager.output; @@ -65,6 +72,18 @@ export class TerminalJob implements vscode.Disposable { return this.config.command; } + get name() { + return this.config.name; + } + + get isPtyTerminal() { + return this.ptyProcess !== undefined; + } + + get isVisible() { + return this.terminalVisibility.value; + } + private constructor( private readonly config: TerminalJobConfig, private readonly ptyProcess?: PtyProcess, @@ -87,7 +106,7 @@ export class TerminalJob implements vscode.Disposable { } this.initializeLifecycle(); if (!this.stopRequested) { - this.terminal.show(); + this.show(); if (!ptyProcess) { void this.executeWithShellIntegration(); } @@ -143,6 +162,36 @@ export class TerminalJob implements vscode.Disposable { ); } + static list(): readonly TerminalJob[] { + return Array.from(TerminalJob.jobs.values()); + } + + show(): void { + if (this.finished || this.stopRequested) return; + if (this.ptyProcess && !this.terminal) { + this.createPtyTerminalView(); + } + this.terminal?.show(false); + this.setVisible(true); + } + + hide(): void { + if (!this.ptyProcess || !this.terminal) return; + const terminal = this.terminal; + const ptyTerminal = this.ptyTerminal; + this.terminal = undefined; + this.ptyTerminal = undefined; + this.setVisible(false); + terminal.dispose(); + ptyTerminal?.dispose(); + } + + closePtyProcess(): void { + if (!this.ptyProcess) return; + this.hide(); + this.requestStop("close requested"); + } + kill(): void { this.requestStop("kill requested"); } @@ -169,18 +218,27 @@ export class TerminalJob implements vscode.Disposable { } // This listener is registered before PtyTerminal's close listener so a - // process-driven terminal close is not mistaken for a user cancellation. + // process-driven terminal close is not mistaken for a user action. this.disposables.push( ptyProcess.onExit(() => { this.ptyExited = true; }), ); - const ptyTerminal = new PtyTerminal(ptyProcess, () => { - this.requestStop("user closed terminal"); - }); - this.disposables.push(ptyTerminal); - ptyProcess.clearReplay(); + this.createPtyTerminalView(); + + this.disposables.push( + ptyProcess.onExit(({ exitCode }) => { + void this.finalize(exitCode); + }), + ); + } + private createPtyTerminalView(): void { + if (!this.ptyProcess || this.terminal) return; + const ptyTerminal = new PtyTerminal(this.ptyProcess, () => { + this.detachPtyTerminalView(ptyTerminal); + }); + this.ptyTerminal = ptyTerminal; this.terminal = createTerminal({ name: this.config.name, pty: ptyTerminal, @@ -188,11 +246,27 @@ export class TerminalJob implements vscode.Disposable { iconPath: new vscode.ThemeIcon("piano"), isTransient: false, }); - this.disposables.push( - ptyProcess.onExit(({ exitCode }) => { - void this.finalize(exitCode); - }), - ); + } + + private detachPtyTerminalView(ptyTerminal?: PtyTerminal): void { + if ( + this.finished || + this.ptyExited || + (ptyTerminal && ptyTerminal !== this.ptyTerminal) + ) { + return; + } + const attachedPtyTerminal = ptyTerminal ?? this.ptyTerminal; + this.terminal = undefined; + this.ptyTerminal = undefined; + attachedPtyTerminal?.dispose(); + this.setVisible(false); + } + + private setVisible(visible: boolean): void { + if (this.terminalVisibility.value === visible) return; + this.terminalVisibility.value = visible; + TerminalJob.onDidChangeVisibilityEmitter.fire(this); } private initializeShellTerminal(): void { @@ -214,25 +288,20 @@ export class TerminalJob implements vscode.Disposable { private initializeLifecycle(): void { this.disposables.push( vscode.window.onDidCloseTerminal((terminal) => { - if ( - terminal !== this.terminal || - this.finished || - (this.ptyProcess && this.ptyExited) - ) { + if (terminal !== this.terminal || this.finished) return; + if (this.ptyProcess) { + if (!this.ptyExited) this.detachPtyTerminalView(); return; } + this.stopRequested = true; - if (this.ptyProcess) { - this.ptyProcess.kill(); - } else { - this.terminalCloseError = ExecutionError.create( - "Background job finished as user closed terminal.", - ); - for (const reject of this.terminalCloseRejectors) { - reject(this.terminalCloseError); - } - this.terminalCloseRejectors.clear(); + this.terminalCloseError = ExecutionError.create( + "Background job finished as user closed terminal.", + ); + for (const reject of this.terminalCloseRejectors) { + reject(this.terminalCloseError); } + this.terminalCloseRejectors.clear(); }), ); @@ -302,7 +371,7 @@ export class TerminalJob implements vscode.Disposable { private waitForShellIntegration( timeoutMs = 15_000, ): Promise { - if (this.terminal.shellIntegration) { + if (this.terminal?.shellIntegration) { return Promise.resolve(this.terminal.shellIntegration); } return new Promise((resolve, reject) => { @@ -374,7 +443,7 @@ export class TerminalJob implements vscode.Disposable { if (this.ptyProcess) { this.ptyProcess.kill(); } else { - this.terminal.dispose(); + this.terminal?.dispose(); } } @@ -467,7 +536,11 @@ export class TerminalJob implements vscode.Disposable { finishedAt: Date.now(), }); - this.terminal.dispose(); + this.terminal?.dispose(); + this.terminal = undefined; + this.ptyTerminal?.dispose(); + this.ptyTerminal = undefined; + this.setVisible(false); this.dispose(); } @@ -479,6 +552,7 @@ export class TerminalJob implements vscode.Disposable { } this.terminalCloseRejectors.clear(); this.terminal?.dispose(); + this.ptyTerminal?.dispose(); if (this.outputWriter) { void this.outputQueue .finally(() => this.outputWriter.close()) diff --git a/packages/vscode/src/integrations/terminal/terminal-state.ts b/packages/vscode/src/integrations/terminal/terminal-state.ts index 41d04f7332..40efe650d5 100644 --- a/packages/vscode/src/integrations/terminal/terminal-state.ts +++ b/packages/vscode/src/integrations/terminal/terminal-state.ts @@ -67,11 +67,42 @@ export class TerminalState implements vscode.Disposable { } public openBackgroundJobTerminal(backgroundJobId: string) { + const job = TerminalJob.get(backgroundJobId); + if (job) { + job.show(); + return; + } + const terminal = vscode.window.terminals.find( (t) => this.getTerminalId(t) === backgroundJobId, ); - if (!terminal) return; - terminal.show(); + terminal?.show(); + } + + public getBackgroundCommandTerminalVisibility(backgroundJobId: string) { + return this.getPtyTerminalJob(backgroundJobId).terminalVisibility; + } + + public showBackgroundCommandTerminal(backgroundJobId: string): void { + this.getPtyTerminalJob(backgroundJobId).show(); + } + + public hideBackgroundCommandTerminal(backgroundJobId: string): void { + this.getPtyTerminalJob(backgroundJobId).hide(); + } + + public closeBackgroundCommand(backgroundJobId: string): void { + this.getPtyTerminalJob(backgroundJobId).closePtyProcess(); + } + + private getPtyTerminalJob(backgroundJobId: string): TerminalJob { + const job = TerminalJob.get(backgroundJobId); + if (!job?.isPtyTerminal) { + throw new Error( + `Detachable background command with ID "${backgroundJobId}" not found.`, + ); + } + return job; } /** @@ -88,6 +119,9 @@ export class TerminalState implements vscode.Disposable { vscode.window.onDidCloseTerminal(this.onTerminalClosed), ); this.disposables.push(TerminalJob.onDidDispose(this.onTerminalChanged)); + this.disposables.push( + TerminalJob.onDidChangeVisibility(this.onTerminalChanged), + ); this.disposables.push( TerminalJob.onDidFinish((event) => { void this.taskDataStore.addBackgroundJobNotification( @@ -212,25 +246,40 @@ export class TerminalState implements vscode.Disposable { } private listVisibleTerminals(): TerminalInfo[] { - return vscode.window.terminals - .filter((t) => { - if ("hideFromUser" in t.creationOptions) { - return !t.creationOptions.hideFromUser; + const listedJobIds = new Set(); + const terminals: TerminalInfo[] = vscode.window.terminals + .filter((terminal) => { + if ("hideFromUser" in terminal.creationOptions) { + return !terminal.creationOptions.hideFromUser; } return true; }) - .map((t) => { - const id = this.getTerminalId(t); - if (!TerminalJob.get(t)) { - TerminalHistoryManager.getOrCreate(id).terminalName = t.name; + .map((terminal) => { + const id = this.getTerminalId(terminal); + const job = TerminalJob.get(terminal); + if (job) { + listedJobIds.add(job.id); + } else { + TerminalHistoryManager.getOrCreate(id).terminalName = terminal.name; } return { - name: t.name, - isActive: t === vscode.window.activeTerminal, + name: terminal.name, + isActive: terminal === vscode.window.activeTerminal, backgroundJobId: id, - outputFile: this.getTerminalOutputFile(t), + outputFile: this.getTerminalOutputFile(terminal), }; }); + + for (const job of TerminalJob.list()) { + if (!job.isPtyTerminal || listedJobIds.has(job.id)) continue; + terminals.push({ + name: job.name, + isActive: false, + backgroundJobId: job.id, + outputFile: job.outputFile, + }); + } + return terminals; } private getTerminalOutputFile(terminal: vscode.Terminal): string | undefined { diff --git a/packages/vscode/src/integrations/webview/vscode-host-impl.ts b/packages/vscode/src/integrations/webview/vscode-host-impl.ts index 838bb8782c..980ba3ce7c 100644 --- a/packages/vscode/src/integrations/webview/vscode-host-impl.ts +++ b/packages/vscode/src/integrations/webview/vscode-host-impl.ts @@ -466,6 +466,23 @@ export class VSCodeHostImpl implements VSCodeHostApi, vscode.Disposable { }; }; + readBackgroundCommand = async (backgroundJobId: string) => ({ + isVisible: ThreadSignal.serialize( + this.terminalState.getBackgroundCommandTerminalVisibility( + backgroundJobId, + ), + ), + show: async () => { + this.terminalState.showBackgroundCommandTerminal(backgroundJobId); + }, + hide: async () => { + this.terminalState.hideBackgroundCommandTerminal(backgroundJobId); + }, + close: async () => { + this.terminalState.closeBackgroundCommand(backgroundJobId); + }, + }); + readBackgroundJobNotifications = async (taskId: string) => ({ notifications: ThreadSignal.serialize( this.taskStateStore.getBackgroundJobNotificationsSignal(taskId), From 6273810e32fc18b2567fff99b885a1542d19ac9e Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 12:01:05 +0800 Subject: [PATCH 05/12] fix(vscode): handle finished background command controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Represent command lifecycle as a discriminated union so completed jobs disable terminal actions without surfacing lookup errors. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../common/src/vscode-webui-bridge/index.ts | 1 + .../src/vscode-webui-bridge/webview-stub.ts | 3 +- .../common/src/vscode-webui-bridge/webview.ts | 6 +- .../__tests__/use-background-command.test.tsx | 22 +++++-- .../src/lib/hooks/use-background-command.ts | 63 ++++++++++++++++--- .../terminal/__test__/terminal-job.test.ts | 8 +++ .../src/integrations/terminal/terminal-job.ts | 25 +++++--- .../integrations/terminal/terminal-state.ts | 27 ++------ .../integrations/webview/vscode-host-impl.ts | 30 ++++----- 9 files changed, 124 insertions(+), 61 deletions(-) diff --git a/packages/common/src/vscode-webui-bridge/index.ts b/packages/common/src/vscode-webui-bridge/index.ts index 8b97b48439..defd4d62f2 100644 --- a/packages/common/src/vscode-webui-bridge/index.ts +++ b/packages/common/src/vscode-webui-bridge/index.ts @@ -1,4 +1,5 @@ export type { + BackgroundCommandState, VSCodeHostApi, WebviewHostApi, } from "./webview"; diff --git a/packages/common/src/vscode-webui-bridge/webview-stub.ts b/packages/common/src/vscode-webui-bridge/webview-stub.ts index e3f05215e6..b495a69137 100644 --- a/packages/common/src/vscode-webui-bridge/webview-stub.ts +++ b/packages/common/src/vscode-webui-bridge/webview-stub.ts @@ -14,6 +14,7 @@ import type { import type { BrowserSession } from "../browser/types"; import type { UserInfo } from "../configuration"; import type { + BackgroundCommandState, BuiltinSubAgentInfo, CaptureEvent, ChangedFileContent, @@ -291,7 +292,7 @@ const VSCodeHostStub = { }, readBackgroundCommand: async (_backgroundJobId: string) => { return Promise.resolve({ - isVisible: {} as ThreadSignalSerialization, + state: {} as ThreadSignalSerialization, show: async (): Promise => Promise.resolve(), hide: async (): Promise => Promise.resolve(), close: async (): Promise => Promise.resolve(), diff --git a/packages/common/src/vscode-webui-bridge/webview.ts b/packages/common/src/vscode-webui-bridge/webview.ts index 0f39216350..d2618994e1 100644 --- a/packages/common/src/vscode-webui-bridge/webview.ts +++ b/packages/common/src/vscode-webui-bridge/webview.ts @@ -51,6 +51,10 @@ import type { DisplayModel } from "./types/model"; import type { PochiCredentials } from "./types/pochi"; import type { VSCodeSettings } from "./types/vscode-settings"; +export type BackgroundCommandState = + | { status: "running"; isVisible: boolean } + | { status: "finished" }; + export interface VSCodeHostApi { readResourceURI(): Promise; @@ -189,7 +193,7 @@ export interface VSCodeHostApi { }>; readBackgroundCommand(backgroundJobId: string): Promise<{ - isVisible: ThreadSignalSerialization; + state: ThreadSignalSerialization; show: () => Promise; hide: () => Promise; close: () => Promise; diff --git a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx index 03c38b5e6b..b5874e1b4c 100644 --- a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx +++ b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx @@ -21,14 +21,16 @@ describe("useBackgroundCommand", () => { vi.clearAllMocks(); }); - it("returns reactive visibility and background command controls", async () => { - const isVisible = signal(false); + it("returns reactive running controls and a finished state", async () => { + const state = signal< + { status: "running"; isVisible: boolean } | { status: "finished" } + >({ status: "running", isVisible: false }); const show = vi.fn(); const hide = vi.fn(); const close = vi.fn(); vi.mocked(useQuery).mockReturnValue({ data: { - isVisible, + state, show, hide, close, @@ -37,10 +39,11 @@ describe("useBackgroundCommand", () => { const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); + expect(result.current.status).toBe("running"); expect(result.current.isVisible).toBe(false); act(() => { - isVisible.value = true; + state.value = { status: "running", isVisible: true }; }); expect(result.current.isVisible).toBe(true); @@ -51,6 +54,16 @@ describe("useBackgroundCommand", () => { expect(show).toHaveBeenCalledOnce(); expect(hide).toHaveBeenCalledOnce(); expect(close).toHaveBeenCalledOnce(); + + act(() => { + state.value = { status: "finished" }; + }); + + expect(result.current.status).toBe("finished"); + expect(result.current.isVisible).toBe(false); + expect(result.current.show).toBeUndefined(); + expect(result.current.hide).toBeUndefined(); + expect(result.current.close).toBeUndefined(); }); it("returns undefined state while the command is loading", () => { @@ -58,6 +71,7 @@ describe("useBackgroundCommand", () => { const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); + expect(result.current.status).toBeUndefined(); expect(result.current.isVisible).toBeUndefined(); expect(result.current.show).toBeUndefined(); expect(result.current.hide).toBeUndefined(); diff --git a/packages/vscode-webui/src/lib/hooks/use-background-command.ts b/packages/vscode-webui/src/lib/hooks/use-background-command.ts index 5e52248736..e274d9da13 100644 --- a/packages/vscode-webui/src/lib/hooks/use-background-command.ts +++ b/packages/vscode-webui/src/lib/hooks/use-background-command.ts @@ -2,30 +2,79 @@ import { vscodeHost } from "@/lib/vscode"; import { threadSignal } from "@quilted/threads/signals"; import { useQuery } from "@tanstack/react-query"; +type BackgroundCommandAction = () => Promise; + +export type UseBackgroundCommandResult = + | { + status: undefined; + isVisible: undefined; + show: undefined; + hide: undefined; + close: undefined; + } + | { + status: "running"; + isVisible: boolean; + show: BackgroundCommandAction; + hide: BackgroundCommandAction; + close: BackgroundCommandAction; + } + | { + status: "finished"; + isVisible: false; + show: undefined; + hide: undefined; + close: undefined; + }; + /** * Controls the detachable terminal view for a running background command. * Hiding the terminal does not stop the command. * @useSignals this comment is needed to enable signals in this hook */ -export const useBackgroundCommand = (backgroundJobId: string) => { +export const useBackgroundCommand = ( + backgroundJobId: string, +): UseBackgroundCommandResult => { const { data } = useQuery({ queryKey: ["backgroundCommand", backgroundJobId], queryFn: () => fetchBackgroundCommand(backgroundJobId), staleTime: Number.POSITIVE_INFINITY, }); + if (!data) { + return { + status: undefined, + isVisible: undefined, + show: undefined, + hide: undefined, + close: undefined, + } as const; + } + + const state = data.state.value; + if (state.status === "finished") { + return { + status: "finished", + isVisible: false, + show: undefined, + hide: undefined, + close: undefined, + } as const; + } + return { - isVisible: data?.isVisible.value, - show: data?.show, - hide: data?.hide, - close: data?.close, - }; + status: "running", + isVisible: state.isVisible, + show: data.show, + hide: data.hide, + close: data.close, + } as const; }; async function fetchBackgroundCommand(backgroundJobId: string) { const result = await vscodeHost.readBackgroundCommand(backgroundJobId); return { - isVisible: threadSignal(result.isVisible), + state: threadSignal(result.state), show: result.show, hide: result.hide, close: result.close, diff --git a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts index 4c35ab674d..ab4c3a1cb1 100644 --- a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts @@ -332,10 +332,18 @@ describe("TerminalJob", () => { it("replays foreground output and completes", async () => { const harness = createHarness({ replay: ["before timeout\n"] }); + assert.deepStrictEqual(harness.job.backgroundCommandState.value, { + status: "running", + isVisible: true, + }); + harness.ptyProcess.emitData("after timeout\n"); harness.ptyProcess.emitExit(0); await flushPromises(); + assert.deepStrictEqual(harness.job.backgroundCommandState.value, { + status: "finished", + }); assert.deepStrictEqual(harness.lifecycle, [ "output:$ sleep 10\n", "output:before timeout\n", diff --git a/packages/vscode/src/integrations/terminal/terminal-job.ts b/packages/vscode/src/integrations/terminal/terminal-job.ts index ff8de6e0f0..1fb816faf0 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -62,7 +62,9 @@ export class TerminalJob implements vscode.Disposable { readonly id: string; readonly outputFile: string; - readonly terminalVisibility = signal(false); + readonly backgroundCommandState = signal< + { status: "running"; isVisible: boolean } | { status: "finished" } + >({ status: "running", isVisible: false }); get output() { return this.outputManager.output; @@ -81,7 +83,8 @@ export class TerminalJob implements vscode.Disposable { } get isVisible() { - return this.terminalVisibility.value; + const state = this.backgroundCommandState.value; + return state.status === "running" && state.isVisible; } private constructor( @@ -263,9 +266,10 @@ export class TerminalJob implements vscode.Disposable { this.setVisible(false); } - private setVisible(visible: boolean): void { - if (this.terminalVisibility.value === visible) return; - this.terminalVisibility.value = visible; + private setVisible(isVisible: boolean): void { + const state = this.backgroundCommandState.value; + if (state.status === "finished" || state.isVisible === isVisible) return; + this.backgroundCommandState.value = { status: "running", isVisible }; TerminalJob.onDidChangeVisibilityEmitter.fire(this); } @@ -490,6 +494,12 @@ export class TerminalJob implements vscode.Disposable { disposable.dispose(); } this.terminalCloseRejectors.clear(); + this.terminal?.dispose(); + this.terminal = undefined; + this.ptyTerminal?.dispose(); + this.ptyTerminal = undefined; + this.setVisible(false); + this.backgroundCommandState.value = { status: "finished" }; let executionError = initialError ?? this.persistenceError; if (exitCode !== undefined && exitCode !== 0 && !this.stopRequested) { @@ -536,11 +546,6 @@ export class TerminalJob implements vscode.Disposable { finishedAt: Date.now(), }); - this.terminal?.dispose(); - this.terminal = undefined; - this.ptyTerminal?.dispose(); - this.ptyTerminal = undefined; - this.setVisible(false); this.dispose(); } diff --git a/packages/vscode/src/integrations/terminal/terminal-state.ts b/packages/vscode/src/integrations/terminal/terminal-state.ts index 40efe650d5..e64869ece3 100644 --- a/packages/vscode/src/integrations/terminal/terminal-state.ts +++ b/packages/vscode/src/integrations/terminal/terminal-state.ts @@ -79,30 +79,11 @@ export class TerminalState implements vscode.Disposable { terminal?.show(); } - public getBackgroundCommandTerminalVisibility(backgroundJobId: string) { - return this.getPtyTerminalJob(backgroundJobId).terminalVisibility; - } - - public showBackgroundCommandTerminal(backgroundJobId: string): void { - this.getPtyTerminalJob(backgroundJobId).show(); - } - - public hideBackgroundCommandTerminal(backgroundJobId: string): void { - this.getPtyTerminalJob(backgroundJobId).hide(); - } - - public closeBackgroundCommand(backgroundJobId: string): void { - this.getPtyTerminalJob(backgroundJobId).closePtyProcess(); - } - - private getPtyTerminalJob(backgroundJobId: string): TerminalJob { + public getBackgroundCommand( + backgroundJobId: string, + ): TerminalJob | undefined { const job = TerminalJob.get(backgroundJobId); - if (!job?.isPtyTerminal) { - throw new Error( - `Detachable background command with ID "${backgroundJobId}" not found.`, - ); - } - return job; + return job?.isPtyTerminal ? job : undefined; } /** diff --git a/packages/vscode/src/integrations/webview/vscode-host-impl.ts b/packages/vscode/src/integrations/webview/vscode-host-impl.ts index 980ba3ce7c..7d5d3101dd 100644 --- a/packages/vscode/src/integrations/webview/vscode-host-impl.ts +++ b/packages/vscode/src/integrations/webview/vscode-host-impl.ts @@ -178,6 +178,9 @@ import { } from "./widget-html-actions"; const logger = getLogger("VSCodeHostImpl"); +const FinishedBackgroundCommandState = computed( + () => ({ status: "finished" }) as const, +); @scoped(Lifecycle.ContainerScoped) @injectable() @@ -466,22 +469,19 @@ export class VSCodeHostImpl implements VSCodeHostApi, vscode.Disposable { }; }; - readBackgroundCommand = async (backgroundJobId: string) => ({ - isVisible: ThreadSignal.serialize( - this.terminalState.getBackgroundCommandTerminalVisibility( - backgroundJobId, + readBackgroundCommand = async (backgroundJobId: string) => { + const backgroundCommand = + this.terminalState.getBackgroundCommand(backgroundJobId); + return { + state: ThreadSignal.serialize( + backgroundCommand?.backgroundCommandState ?? + FinishedBackgroundCommandState, ), - ), - show: async () => { - this.terminalState.showBackgroundCommandTerminal(backgroundJobId); - }, - hide: async () => { - this.terminalState.hideBackgroundCommandTerminal(backgroundJobId); - }, - close: async () => { - this.terminalState.closeBackgroundCommand(backgroundJobId); - }, - }); + show: async () => backgroundCommand?.show(), + hide: async () => backgroundCommand?.hide(), + close: async () => backgroundCommand?.closePtyProcess(), + }; + }; readBackgroundJobNotifications = async (taskId: string) => ({ notifications: ThreadSignal.serialize( From 4eec2b2f8a0bb7b19163eb2434a89a2a465a5cfe Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 12:58:59 +0800 Subject: [PATCH 06/12] feat(vscode): manage hidden background commands globally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep detachable commands hidden until explicitly opened and expose one reactive manager so UI controls always resolve the current job by ID. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../common/src/vscode-webui-bridge/index.ts | 2 +- .../src/vscode-webui-bridge/webview-stub.ts | 15 ++-- .../common/src/vscode-webui-bridge/webview.ts | 14 ++-- .../command-execution-panel.test.tsx | 26 ++++++ .../components/command-execution-panel.tsx | 16 +++- .../__tests__/use-background-command.test.tsx | 80 ------------------ .../use-background-commands.test.tsx | 77 +++++++++++++++++ .../src/lib/hooks/use-background-command.ts | 82 ------------------- .../src/lib/hooks/use-background-commands.ts | 59 +++++++++++++ packages/vscode-webui/src/lib/vscode.ts | 2 +- .../terminal/__test__/terminal-job.test.ts | 40 +++++---- .../src/integrations/terminal/terminal-job.ts | 43 ++++++---- .../integrations/terminal/terminal-state.ts | 42 ++++++++-- .../integrations/webview/vscode-host-impl.ts | 30 ++++--- 14 files changed, 290 insertions(+), 238 deletions(-) delete mode 100644 packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx create mode 100644 packages/vscode-webui/src/lib/hooks/__tests__/use-background-commands.test.tsx delete mode 100644 packages/vscode-webui/src/lib/hooks/use-background-command.ts create mode 100644 packages/vscode-webui/src/lib/hooks/use-background-commands.ts diff --git a/packages/common/src/vscode-webui-bridge/index.ts b/packages/common/src/vscode-webui-bridge/index.ts index defd4d62f2..4d61ab1014 100644 --- a/packages/common/src/vscode-webui-bridge/index.ts +++ b/packages/common/src/vscode-webui-bridge/index.ts @@ -1,5 +1,5 @@ export type { - BackgroundCommandState, + BackgroundCommands, VSCodeHostApi, WebviewHostApi, } from "./webview"; diff --git a/packages/common/src/vscode-webui-bridge/webview-stub.ts b/packages/common/src/vscode-webui-bridge/webview-stub.ts index b495a69137..76e69cd2b9 100644 --- a/packages/common/src/vscode-webui-bridge/webview-stub.ts +++ b/packages/common/src/vscode-webui-bridge/webview-stub.ts @@ -14,7 +14,7 @@ import type { import type { BrowserSession } from "../browser/types"; import type { UserInfo } from "../configuration"; import type { - BackgroundCommandState, + BackgroundCommands, BuiltinSubAgentInfo, CaptureEvent, ChangedFileContent, @@ -290,12 +290,15 @@ const VSCodeHostStub = { }, }); }, - readBackgroundCommand: async (_backgroundJobId: string) => { + readBackgroundCommands: async () => { return Promise.resolve({ - state: {} as ThreadSignalSerialization, - show: async (): Promise => Promise.resolve(), - hide: async (): Promise => Promise.resolve(), - close: async (): Promise => Promise.resolve(), + backgroundCommands: {} as ThreadSignalSerialization, + show: async (_backgroundJobId: string): Promise => + Promise.resolve(), + hide: async (_backgroundJobId: string): Promise => + Promise.resolve(), + close: async (_backgroundJobId: string): Promise => + Promise.resolve(), }); }, readModelList: async () => { diff --git a/packages/common/src/vscode-webui-bridge/webview.ts b/packages/common/src/vscode-webui-bridge/webview.ts index d2618994e1..24911f2052 100644 --- a/packages/common/src/vscode-webui-bridge/webview.ts +++ b/packages/common/src/vscode-webui-bridge/webview.ts @@ -51,9 +51,7 @@ import type { DisplayModel } from "./types/model"; import type { PochiCredentials } from "./types/pochi"; import type { VSCodeSettings } from "./types/vscode-settings"; -export type BackgroundCommandState = - | { status: "running"; isVisible: boolean } - | { status: "finished" }; +export type BackgroundCommands = Record; export interface VSCodeHostApi { readResourceURI(): Promise; @@ -192,11 +190,11 @@ export interface VSCodeHostApi { openBackgroundJobTerminal: (backgroundJobId: string) => Promise; }>; - readBackgroundCommand(backgroundJobId: string): Promise<{ - state: ThreadSignalSerialization; - show: () => Promise; - hide: () => Promise; - close: () => Promise; + readBackgroundCommands(): Promise<{ + backgroundCommands: ThreadSignalSerialization; + show: (backgroundJobId: string) => Promise; + hide: (backgroundJobId: string) => Promise; + close: (backgroundJobId: string) => Promise; }>; readBackgroundJobNotifications(taskId: string): Promise<{ diff --git a/packages/vscode-webui/src/features/tools/components/__tests__/command-execution-panel.test.tsx b/packages/vscode-webui/src/features/tools/components/__tests__/command-execution-panel.test.tsx index d15e7d17d3..707760910c 100644 --- a/packages/vscode-webui/src/features/tools/components/__tests__/command-execution-panel.test.tsx +++ b/packages/vscode-webui/src/features/tools/components/__tests__/command-execution-panel.test.tsx @@ -4,10 +4,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { BackgroundJobPanel } from "../command-execution-panel"; const openBackgroundJobTerminal = vi.fn(); +const showBackgroundCommand = vi.fn(); const openFile = vi.fn(); let terminals: | { backgroundJobId: string; name: string; isActive: boolean }[] | undefined = []; +let backgroundCommands: Record | undefined = {}; let jobInfo: { command: string | undefined; displayId: string } | undefined; vi.stubGlobal( @@ -29,6 +31,13 @@ vi.mock("@/features/chat", () => ({ useBackgroundJobInfo: () => jobInfo, })); +vi.mock("@/lib/hooks/use-background-commands", () => ({ + useBackgroundCommands: () => ({ + backgroundCommands, + show: showBackgroundCommand, + }), +})); + vi.mock("@/lib/hooks/use-visible-terminals", () => ({ useVisibleTerminals: () => ({ terminals, @@ -70,8 +79,10 @@ const renderBackgroundJobPanel = (outputFile?: string) => describe("BackgroundJobPanel job control", () => { beforeEach(() => { openBackgroundJobTerminal.mockClear(); + showBackgroundCommand.mockClear(); openFile.mockClear(); terminals = []; + backgroundCommands = {}; jobInfo = { command: "bun run dev", displayId: "%1" }; }); @@ -87,6 +98,21 @@ describe("BackgroundJobPanel job control", () => { expect(openFile).not.toHaveBeenCalled(); }); + it("opens a detachable background command through its dedicated control", () => { + terminals = [ + { backgroundJobId: "bgjob-cmd-1", name: "zsh", isActive: false }, + ]; + backgroundCommands = { + "bgjob-cmd-1": { isVisible: false }, + }; + + renderBackgroundJobPanel("/tmp/bgjob-cmd-1.log"); + + fireEvent.click(screen.getByLabelText("commandExecutionPanel.openJob")); + expect(showBackgroundCommand).toHaveBeenCalledWith("bgjob-cmd-1"); + expect(openBackgroundJobTerminal).not.toHaveBeenCalled(); + }); + it("opens the output file once the user terminal is gone", () => { renderTerminalPanel("/tmp/term-1.log"); diff --git a/packages/vscode-webui/src/features/tools/components/command-execution-panel.tsx b/packages/vscode-webui/src/features/tools/components/command-execution-panel.tsx index 24c15e6559..68cfd4bc88 100644 --- a/packages/vscode-webui/src/features/tools/components/command-execution-panel.tsx +++ b/packages/vscode-webui/src/features/tools/components/command-execution-panel.tsx @@ -6,6 +6,7 @@ import { TooltipTrigger, } from "@/components/ui/tooltip"; import { useBackgroundJobInfo } from "@/features/chat"; +import { useBackgroundCommands } from "@/lib/hooks/use-background-commands"; import { useCopyToClipboard } from "@/lib/hooks/use-copy-to-clipboard"; import { useDebounceState } from "@/lib/hooks/use-debounce-state"; import { useVisibleTerminals } from "@/lib/hooks/use-visible-terminals"; @@ -245,8 +246,12 @@ export const BackgroundJobPanel: FC<{ const [expanded, setExpanded] = useState(false); const toggleExpanded = () => setExpanded((prev) => !prev); const info = useBackgroundJobInfo(backgroundJobId); + const { backgroundCommands, show: showBackgroundCommand } = + useBackgroundCommands(); const { terminals, openBackgroundJobTerminal } = useVisibleTerminals(); const isUserTerminal = backgroundJobId.startsWith("term-"); + const isDetachableBackgroundCommand = + backgroundCommands?.[backgroundJobId] !== undefined; // Live name wins over the snapshot: the terminal may have been renamed // since the read. The snapshot keeps historical reads meaningful after the // terminal is closed. @@ -282,12 +287,21 @@ export const BackgroundJobPanel: FC<{ if (outputFile) vscodeHost.openFile(outputFile); return; } - openBackgroundJobTerminal?.(backgroundJobId); + if (isDetachableBackgroundCommand) { + showBackgroundCommand?.(backgroundJobId); + } else if (isUserTerminal || backgroundCommands !== undefined) { + // Keep the legacy terminal path for user terminals and shell fallbacks. + openBackgroundJobTerminal?.(backgroundJobId); + } }, [ + backgroundCommands, backgroundJobId, + isDetachableBackgroundCommand, isTerminalClosed, + isUserTerminal, openBackgroundJobTerminal, outputFile, + showBackgroundCommand, ]); const closedLabel = canOpenOutputFile diff --git a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx deleted file mode 100644 index b5874e1b4c..0000000000 --- a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-command.test.tsx +++ /dev/null @@ -1,80 +0,0 @@ -// @vitest-environment jsdom - -import { signal } from "@preact/signals-core"; -import { useQuery } from "@tanstack/react-query"; -import { act, renderHook } from "@testing-library/react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { useBackgroundCommand } from "../use-background-command"; - -vi.mock("@tanstack/react-query", () => ({ - useQuery: vi.fn(), -})); - -vi.mock("../../vscode", () => ({ - vscodeHost: { - readBackgroundCommand: vi.fn(), - }, -})); - -describe("useBackgroundCommand", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns reactive running controls and a finished state", async () => { - const state = signal< - { status: "running"; isVisible: boolean } | { status: "finished" } - >({ status: "running", isVisible: false }); - const show = vi.fn(); - const hide = vi.fn(); - const close = vi.fn(); - vi.mocked(useQuery).mockReturnValue({ - data: { - state, - show, - hide, - close, - }, - } as never); - - const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); - - expect(result.current.status).toBe("running"); - expect(result.current.isVisible).toBe(false); - - act(() => { - state.value = { status: "running", isVisible: true }; - }); - expect(result.current.isVisible).toBe(true); - - await act(async () => result.current.show?.()); - await act(async () => result.current.hide?.()); - await act(async () => result.current.close?.()); - - expect(show).toHaveBeenCalledOnce(); - expect(hide).toHaveBeenCalledOnce(); - expect(close).toHaveBeenCalledOnce(); - - act(() => { - state.value = { status: "finished" }; - }); - - expect(result.current.status).toBe("finished"); - expect(result.current.isVisible).toBe(false); - expect(result.current.show).toBeUndefined(); - expect(result.current.hide).toBeUndefined(); - expect(result.current.close).toBeUndefined(); - }); - - it("returns undefined state while the command is loading", () => { - vi.mocked(useQuery).mockReturnValue({ data: undefined } as never); - - const { result } = renderHook(() => useBackgroundCommand("bgjob-cmd-1")); - - expect(result.current.status).toBeUndefined(); - expect(result.current.isVisible).toBeUndefined(); - expect(result.current.show).toBeUndefined(); - expect(result.current.hide).toBeUndefined(); - expect(result.current.close).toBeUndefined(); - }); -}); diff --git a/packages/vscode-webui/src/lib/hooks/__tests__/use-background-commands.test.tsx b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-commands.test.tsx new file mode 100644 index 0000000000..63c2dbcaad --- /dev/null +++ b/packages/vscode-webui/src/lib/hooks/__tests__/use-background-commands.test.tsx @@ -0,0 +1,77 @@ +// @vitest-environment jsdom + +import type { BackgroundCommands } from "@getpochi/common/vscode-webui-bridge"; +import { signal } from "@preact/signals-core"; +import { useQuery } from "@tanstack/react-query"; +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useBackgroundCommands } from "../use-background-commands"; + +vi.mock("@tanstack/react-query", () => ({ + useQuery: vi.fn(), +})); + +vi.mock("../../vscode", () => ({ + vscodeHost: { + readBackgroundCommands: vi.fn(), + }, +})); + +describe("useBackgroundCommands", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns all running commands and controls them by id", async () => { + const backgroundCommands = signal({ + "bgjob-cmd-1": { isVisible: false }, + "bgjob-cmd-2": { isVisible: true }, + }); + const show = vi.fn(); + const hide = vi.fn(); + const close = vi.fn(); + vi.mocked(useQuery).mockReturnValue({ + data: { + backgroundCommands, + show, + hide, + close, + }, + } as never); + + const { result } = renderHook(() => useBackgroundCommands()); + + expect(result.current.backgroundCommands).toEqual({ + "bgjob-cmd-1": { isVisible: false }, + "bgjob-cmd-2": { isVisible: true }, + }); + + act(() => { + backgroundCommands.value = { + "bgjob-cmd-2": { isVisible: false }, + }; + }); + expect(result.current.backgroundCommands).toEqual({ + "bgjob-cmd-2": { isVisible: false }, + }); + + await act(async () => result.current.show?.("bgjob-cmd-2")); + await act(async () => result.current.hide?.("bgjob-cmd-2")); + await act(async () => result.current.close?.("bgjob-cmd-2")); + + expect(show).toHaveBeenCalledWith("bgjob-cmd-2"); + expect(hide).toHaveBeenCalledWith("bgjob-cmd-2"); + expect(close).toHaveBeenCalledWith("bgjob-cmd-2"); + }); + + it("returns undefined state while commands are loading", () => { + vi.mocked(useQuery).mockReturnValue({ data: undefined } as never); + + const { result } = renderHook(() => useBackgroundCommands()); + + expect(result.current.backgroundCommands).toBeUndefined(); + expect(result.current.show).toBeUndefined(); + expect(result.current.hide).toBeUndefined(); + expect(result.current.close).toBeUndefined(); + }); +}); diff --git a/packages/vscode-webui/src/lib/hooks/use-background-command.ts b/packages/vscode-webui/src/lib/hooks/use-background-command.ts deleted file mode 100644 index e274d9da13..0000000000 --- a/packages/vscode-webui/src/lib/hooks/use-background-command.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { vscodeHost } from "@/lib/vscode"; -import { threadSignal } from "@quilted/threads/signals"; -import { useQuery } from "@tanstack/react-query"; - -type BackgroundCommandAction = () => Promise; - -export type UseBackgroundCommandResult = - | { - status: undefined; - isVisible: undefined; - show: undefined; - hide: undefined; - close: undefined; - } - | { - status: "running"; - isVisible: boolean; - show: BackgroundCommandAction; - hide: BackgroundCommandAction; - close: BackgroundCommandAction; - } - | { - status: "finished"; - isVisible: false; - show: undefined; - hide: undefined; - close: undefined; - }; - -/** - * Controls the detachable terminal view for a running background command. - * Hiding the terminal does not stop the command. - * @useSignals this comment is needed to enable signals in this hook - */ -export const useBackgroundCommand = ( - backgroundJobId: string, -): UseBackgroundCommandResult => { - const { data } = useQuery({ - queryKey: ["backgroundCommand", backgroundJobId], - queryFn: () => fetchBackgroundCommand(backgroundJobId), - staleTime: Number.POSITIVE_INFINITY, - }); - - if (!data) { - return { - status: undefined, - isVisible: undefined, - show: undefined, - hide: undefined, - close: undefined, - } as const; - } - - const state = data.state.value; - if (state.status === "finished") { - return { - status: "finished", - isVisible: false, - show: undefined, - hide: undefined, - close: undefined, - } as const; - } - - return { - status: "running", - isVisible: state.isVisible, - show: data.show, - hide: data.hide, - close: data.close, - } as const; -}; - -async function fetchBackgroundCommand(backgroundJobId: string) { - const result = await vscodeHost.readBackgroundCommand(backgroundJobId); - return { - state: threadSignal(result.state), - show: result.show, - hide: result.hide, - close: result.close, - }; -} diff --git a/packages/vscode-webui/src/lib/hooks/use-background-commands.ts b/packages/vscode-webui/src/lib/hooks/use-background-commands.ts new file mode 100644 index 0000000000..b5b4acb203 --- /dev/null +++ b/packages/vscode-webui/src/lib/hooks/use-background-commands.ts @@ -0,0 +1,59 @@ +import { vscodeHost } from "@/lib/vscode"; +import type { BackgroundCommands } from "@getpochi/common/vscode-webui-bridge"; +import { threadSignal } from "@quilted/threads/signals"; +import { useQuery } from "@tanstack/react-query"; + +type BackgroundCommandAction = (backgroundJobId: string) => Promise; + +export type UseBackgroundCommandsResult = + | { + backgroundCommands: undefined; + show: undefined; + hide: undefined; + close: undefined; + } + | { + backgroundCommands: BackgroundCommands; + show: BackgroundCommandAction; + hide: BackgroundCommandAction; + close: BackgroundCommandAction; + }; + +/** + * Returns all running detachable background commands and controls their + * terminal views by id. Hiding a terminal does not stop its command. + * @useSignals this comment is needed to enable signals in this hook + */ +export const useBackgroundCommands = (): UseBackgroundCommandsResult => { + const { data } = useQuery({ + queryKey: ["backgroundCommands"], + queryFn: fetchBackgroundCommands, + staleTime: Number.POSITIVE_INFINITY, + }); + + if (!data) { + return { + backgroundCommands: undefined, + show: undefined, + hide: undefined, + close: undefined, + } as const; + } + + return { + backgroundCommands: data.backgroundCommands.value, + show: data.show, + hide: data.hide, + close: data.close, + } as const; +}; + +async function fetchBackgroundCommands() { + const result = await vscodeHost.readBackgroundCommands(); + return { + backgroundCommands: threadSignal(result.backgroundCommands), + show: result.show, + hide: result.hide, + close: result.close, + }; +} diff --git a/packages/vscode-webui/src/lib/vscode.ts b/packages/vscode-webui/src/lib/vscode.ts index 5e5e9a43a6..357a20f7ce 100644 --- a/packages/vscode-webui/src/lib/vscode.ts +++ b/packages/vscode-webui/src/lib/vscode.ts @@ -133,7 +133,7 @@ function createVSCodeHost(): VSCodeHostApi { "showInformationMessage", "showWarningMessage", "readVisibleTerminals", - "readBackgroundCommand", + "readBackgroundCommands", "readModelList", "readUserStorage", "readCustomAgents", diff --git a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts index ab4c3a1cb1..40ad9a8a97 100644 --- a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts @@ -315,35 +315,34 @@ describe("TerminalJob", () => { assert.strictEqual(finishEvents[0]?.status, "stopped"); }); - it("rolls back an adopted pty when terminal initialization fails", async () => { + it("keeps an adopted pty running when terminal view creation fails", async () => { const initializationError = new Error("terminal creation failed"); const harness = createHarness({ createTerminalError: initializationError }); - await flushPromises(); - assert.strictEqual(harness.adoptionError, initializationError); - assert.ok(harness.ptyProcess.killCalls >= 1); + assert.throws(() => harness.job.show(), initializationError); + assert.strictEqual(harness.adoptionError, undefined); + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.ptyProcess.killCalls, 0); assert.strictEqual( harness.TerminalJob.get("bgjob-cmd-test"), - undefined, + harness.job, ); - assert.ok(harness.lifecycle.includes("manager-deleted")); - assert.ok(harness.lifecycle.includes("file-closed")); + + harness.ptyProcess.emitExit(0); + await flushPromises(); }); it("replays foreground output and completes", async () => { const harness = createHarness({ replay: ["before timeout\n"] }); - assert.deepStrictEqual(harness.job.backgroundCommandState.value, { - status: "running", - isVisible: true, - }); + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.job.isFinished, false); harness.ptyProcess.emitData("after timeout\n"); harness.ptyProcess.emitExit(0); await flushPromises(); - assert.deepStrictEqual(harness.job.backgroundCommandState.value, { - status: "finished", - }); + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.job.isFinished, true); assert.deepStrictEqual(harness.lifecycle, [ "output:$ sleep 10\n", "output:before timeout\n", @@ -355,12 +354,17 @@ describe("TerminalJob", () => { "manager-deleted", ]); assert.strictEqual(harness.finishEvents[0]?.status, "completed"); - assert.strictEqual(harness.terminalDisposeCalls, 1); + assert.strictEqual(harness.terminalDisposeCalls, 0); assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); }); - it("detaches and recreates the terminal without stopping the pty", async () => { + it("opens, detaches, and recreates the terminal without stopping the pty", async () => { const harness = createHarness(); + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.terminalShowCalls, 0); + + harness.job.show(); + assert.strictEqual(harness.job.isVisible, true); assert.strictEqual(harness.terminalShowCalls, 1); assert.deepStrictEqual(harness.terminalShowPreserveFocus, [false]); @@ -387,6 +391,7 @@ describe("TerminalJob", () => { it("keeps the pty running when the VS Code terminal is closed", async () => { const harness = createHarness(); + harness.job.show(); harness.terminal.dispose(); @@ -399,6 +404,7 @@ describe("TerminalJob", () => { it("closes the terminal when explicitly closing the pty process", async () => { const harness = createHarness(); + harness.job.show(); harness.job.closePtyProcess(); @@ -420,7 +426,7 @@ describe("TerminalJob", () => { await flushPromises(); assert.strictEqual(harness.finishEvents[0]?.status, "stopped"); - assert.strictEqual(harness.terminalDisposeCalls, 1); + assert.strictEqual(harness.terminalDisposeCalls, 0); }); it("marks a nonzero natural exit as failed", async () => { diff --git a/packages/vscode/src/integrations/terminal/terminal-job.ts b/packages/vscode/src/integrations/terminal/terminal-job.ts index 1fb816faf0..ef2454c92b 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -30,6 +30,9 @@ export interface TerminalJobConfig { export class TerminalJob implements vscode.Disposable { private static readonly jobs = new Map(); + private static readonly onDidCreateEmitter = + new vscode.EventEmitter(); + static readonly onDidCreate = TerminalJob.onDidCreateEmitter.event; private static readonly onDidDisposeEmitter = new vscode.EventEmitter(); static readonly onDidDispose = TerminalJob.onDidDisposeEmitter.event; @@ -62,9 +65,7 @@ export class TerminalJob implements vscode.Disposable { readonly id: string; readonly outputFile: string; - readonly backgroundCommandState = signal< - { status: "running"; isVisible: boolean } | { status: "finished" } - >({ status: "running", isVisible: false }); + readonly terminalVisibility = signal(false); get output() { return this.outputManager.output; @@ -82,9 +83,12 @@ export class TerminalJob implements vscode.Disposable { return this.ptyProcess !== undefined; } + get isFinished() { + return this.finished; + } + get isVisible() { - const state = this.backgroundCommandState.value; - return state.status === "running" && state.isVisible; + return this.terminalVisibility.value; } private constructor( @@ -109,7 +113,6 @@ export class TerminalJob implements vscode.Disposable { } this.initializeLifecycle(); if (!this.stopRequested) { - this.show(); if (!ptyProcess) { void this.executeWithShellIntegration(); } @@ -121,6 +124,7 @@ export class TerminalJob implements vscode.Disposable { throw error; } + TerminalJob.onDidCreateEmitter.fire(this); logger.info( `Created terminal job "${config.name}" with command: ${config.command}`, ); @@ -227,7 +231,6 @@ export class TerminalJob implements vscode.Disposable { this.ptyExited = true; }), ); - this.createPtyTerminalView(); this.disposables.push( ptyProcess.onExit(({ exitCode }) => { @@ -242,13 +245,19 @@ export class TerminalJob implements vscode.Disposable { this.detachPtyTerminalView(ptyTerminal); }); this.ptyTerminal = ptyTerminal; - this.terminal = createTerminal({ - name: this.config.name, - pty: ptyTerminal, - location: this.config.location, - iconPath: new vscode.ThemeIcon("piano"), - isTransient: false, - }); + try { + this.terminal = createTerminal({ + name: this.config.name, + pty: ptyTerminal, + location: this.config.location, + iconPath: new vscode.ThemeIcon("piano"), + isTransient: false, + }); + } catch (error) { + this.ptyTerminal = undefined; + ptyTerminal.dispose(); + throw error; + } } private detachPtyTerminalView(ptyTerminal?: PtyTerminal): void { @@ -267,9 +276,8 @@ export class TerminalJob implements vscode.Disposable { } private setVisible(isVisible: boolean): void { - const state = this.backgroundCommandState.value; - if (state.status === "finished" || state.isVisible === isVisible) return; - this.backgroundCommandState.value = { status: "running", isVisible }; + if (this.terminalVisibility.value === isVisible) return; + this.terminalVisibility.value = isVisible; TerminalJob.onDidChangeVisibilityEmitter.fire(this); } @@ -499,7 +507,6 @@ export class TerminalJob implements vscode.Disposable { this.ptyTerminal?.dispose(); this.ptyTerminal = undefined; this.setVisible(false); - this.backgroundCommandState.value = { status: "finished" }; let executionError = initialError ?? this.persistenceError; if (exitCode !== undefined && exitCode !== 0 && !this.stopRequested) { diff --git a/packages/vscode/src/integrations/terminal/terminal-state.ts b/packages/vscode/src/integrations/terminal/terminal-state.ts index e64869ece3..27d7a66920 100644 --- a/packages/vscode/src/integrations/terminal/terminal-state.ts +++ b/packages/vscode/src/integrations/terminal/terminal-state.ts @@ -7,6 +7,7 @@ import { PlainOutputSanitizer, cleanupStaleTerminalOutputFiles, } from "@getpochi/common/tool-utils"; +import type { BackgroundCommands } from "@getpochi/common/vscode-webui-bridge"; import { signal } from "@preact/signals-core"; import { injectable, singleton } from "tsyringe"; import * as vscode from "vscode"; @@ -55,14 +56,15 @@ export class TerminalState implements vscode.Disposable { } >(); - // Signal containing the current active terminals + // Signals containing the current terminals and detachable background commands. visibleTerminals = signal([]); + backgroundCommands = signal({}); constructor(private readonly taskDataStore: TaskDataStore) { void cleanupStaleTerminalOutputFiles().catch((error) => { logger.debug(`Failed to clean up stale terminal output files: ${error}`); }); - this.visibleTerminals.value = this.listVisibleTerminals(); + this.refreshTerminalState(); this.setupEventListeners(); } @@ -79,11 +81,23 @@ export class TerminalState implements vscode.Disposable { terminal?.show(); } - public getBackgroundCommand( + public showBackgroundCommand(backgroundJobId: string): void { + this.getBackgroundCommand(backgroundJobId)?.show(); + } + + public hideBackgroundCommand(backgroundJobId: string): void { + this.getBackgroundCommand(backgroundJobId)?.hide(); + } + + public closeBackgroundCommand(backgroundJobId: string): void { + this.getBackgroundCommand(backgroundJobId)?.closePtyProcess(); + } + + private getBackgroundCommand( backgroundJobId: string, ): TerminalJob | undefined { const job = TerminalJob.get(backgroundJobId); - return job?.isPtyTerminal ? job : undefined; + return job?.isPtyTerminal && !job.isFinished ? job : undefined; } /** @@ -99,6 +113,7 @@ export class TerminalState implements vscode.Disposable { this.disposables.push( vscode.window.onDidCloseTerminal(this.onTerminalClosed), ); + this.disposables.push(TerminalJob.onDidCreate(this.onTerminalChanged)); this.disposables.push(TerminalJob.onDidDispose(this.onTerminalChanged)); this.disposables.push( TerminalJob.onDidChangeVisibility(this.onTerminalChanged), @@ -125,13 +140,16 @@ export class TerminalState implements vscode.Disposable { ); } - /** - * Update the active terminals signal when terminals change - */ + /** Update terminal and background command signals when terminal state changes. */ private onTerminalChanged = () => { - this.visibleTerminals.value = this.listVisibleTerminals(); + this.refreshTerminalState(); }; + private refreshTerminalState(): void { + this.visibleTerminals.value = this.listVisibleTerminals(); + this.backgroundCommands.value = this.listBackgroundCommands(); + } + private onTerminalClosed = (terminal: vscode.Terminal) => { const id = this.terminalIds.get(terminal); if (id) { @@ -226,6 +244,14 @@ export class TerminalState implements vscode.Disposable { return id; } + private listBackgroundCommands(): BackgroundCommands { + return Object.fromEntries( + TerminalJob.list() + .filter((job) => job.isPtyTerminal && !job.isFinished) + .map((job) => [job.id, { isVisible: job.isVisible }]), + ); + } + private listVisibleTerminals(): TerminalInfo[] { const listedJobIds = new Set(); const terminals: TerminalInfo[] = vscode.window.terminals diff --git a/packages/vscode/src/integrations/webview/vscode-host-impl.ts b/packages/vscode/src/integrations/webview/vscode-host-impl.ts index 7d5d3101dd..bc0813fe2c 100644 --- a/packages/vscode/src/integrations/webview/vscode-host-impl.ts +++ b/packages/vscode/src/integrations/webview/vscode-host-impl.ts @@ -178,9 +178,6 @@ import { } from "./widget-html-actions"; const logger = getLogger("VSCodeHostImpl"); -const FinishedBackgroundCommandState = computed( - () => ({ status: "finished" }) as const, -); @scoped(Lifecycle.ContainerScoped) @injectable() @@ -469,19 +466,20 @@ export class VSCodeHostImpl implements VSCodeHostApi, vscode.Disposable { }; }; - readBackgroundCommand = async (backgroundJobId: string) => { - const backgroundCommand = - this.terminalState.getBackgroundCommand(backgroundJobId); - return { - state: ThreadSignal.serialize( - backgroundCommand?.backgroundCommandState ?? - FinishedBackgroundCommandState, - ), - show: async () => backgroundCommand?.show(), - hide: async () => backgroundCommand?.hide(), - close: async () => backgroundCommand?.closePtyProcess(), - }; - }; + readBackgroundCommands = async () => ({ + backgroundCommands: ThreadSignal.serialize( + this.terminalState.backgroundCommands, + ), + show: async (backgroundJobId: string) => { + this.terminalState.showBackgroundCommand(backgroundJobId); + }, + hide: async (backgroundJobId: string) => { + this.terminalState.hideBackgroundCommand(backgroundJobId); + }, + close: async (backgroundJobId: string) => { + this.terminalState.closeBackgroundCommand(backgroundJobId); + }, + }); readBackgroundJobNotifications = async (taskId: string) => ({ notifications: ThreadSignal.serialize( From b22081da28b2cb9ab7ac763245fa6e857a424874 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 13:47:40 +0800 Subject: [PATCH 07/12] fix(cli): bound foreground command output capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spool foreground output before timeout promotion so large commands retain complete logs without unbounded replay memory. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../foreground-output-capture.test.ts | 19 +++ .../cli/src/lib/background-job-manager.ts | 26 +-- .../cli/src/lib/foreground-output-capture.ts | 156 ++++++++++++++++++ .../tools/__tests__/execute-command.test.ts | 31 ++++ packages/cli/src/tools/execute-command.ts | 62 +++---- 5 files changed, 255 insertions(+), 39 deletions(-) create mode 100644 packages/cli/src/lib/__tests__/foreground-output-capture.test.ts create mode 100644 packages/cli/src/lib/foreground-output-capture.ts diff --git a/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts new file mode 100644 index 0000000000..128e4189a4 --- /dev/null +++ b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts @@ -0,0 +1,19 @@ +import { PassThrough } from "node:stream"; +import { describe, expect, it } from "vitest"; +import { ForegroundOutputCapture } from "../foreground-output-capture"; + +describe("ForegroundOutputCapture", () => { + it("keeps only the bounded replay tail across stdout and stderr", async () => { + const stdout = new PassThrough(); + const stderr = new PassThrough(); + const capture = new ForegroundOutputCapture(stdout, stderr, 5); + + stdout.write("1234"); + stderr.write("abcdef"); + + await expect(capture.finish()).resolves.toEqual({ + stdout: "", + stderr: "bcdef", + }); + }); +}); diff --git a/packages/cli/src/lib/background-job-manager.ts b/packages/cli/src/lib/background-job-manager.ts index 2cd5e0dbb2..c237ab5276 100644 --- a/packages/cli/src/lib/background-job-manager.ts +++ b/packages/cli/src/lib/background-job-manager.ts @@ -34,9 +34,14 @@ export interface BackgroundJobStartResult { outputFile: string; } +export type BackgroundJobInitialOutputStream = + | Iterable + | AsyncIterable; + export interface BackgroundJobInitialOutput { - stdout: Buffer[]; - stderr: Buffer[]; + stdout: BackgroundJobInitialOutputStream; + stderr: BackgroundJobInitialOutputStream; + dispose?: () => Promise; } export interface BackgroundJobManagerOptions { @@ -124,11 +129,11 @@ export class BackgroundJobManager { const consumeOutput = async ( stream: Readable | null, - initialChunks: Buffer[], + initialOutputStream: BackgroundJobInitialOutputStream, ) => { const decoder = new StringDecoder("utf8"); const sanitizer = new PlainOutputSanitizer(); - for (const chunk of initialChunks) { + for await (const chunk of initialOutputStream) { await appendOutput(sanitizer.write(decoder.write(chunk))); } if (stream) { @@ -150,10 +155,12 @@ export class BackgroundJobManager { const outputFinished = Promise.all([ consumeOutput(child.stdout, initialOutput.stdout), consumeOutput(child.stderr, initialOutput.stderr), - ]).catch((error) => { - outputError = error; - child.kill(); - }); + ]) + .finally(() => initialOutput.dispose?.()) + .catch((error) => { + outputError = error; + child.kill(); + }); if (abortSignal) { const onAbort = () => { @@ -192,9 +199,6 @@ export class BackgroundJobManager { await this.finalize(job, "failed", undefined, error.message); }); - child.stdout?.resume(); - child.stderr?.resume(); - return { backgroundJobId: id, outputFile }; } diff --git a/packages/cli/src/lib/foreground-output-capture.ts b/packages/cli/src/lib/foreground-output-capture.ts new file mode 100644 index 0000000000..04403701ec --- /dev/null +++ b/packages/cli/src/lib/foreground-output-capture.ts @@ -0,0 +1,156 @@ +import { randomUUID } from "node:crypto"; +import { createReadStream, createWriteStream } from "node:fs"; +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import type { Readable } from "node:stream"; +import { StringDecoder } from "node:string_decoder"; +import type { BackgroundJobInitialOutput } from "./background-job-manager"; + +const ForegroundReplayMaxCharacters = 1024 * 1024; + +type OutputChannel = "stdout" | "stderr"; + +interface ReplayChunk { + channel: OutputChannel; + value: string; +} + +interface StreamCapture { + stream: Readable; + outputPath: string; + writer: ReturnType; + writerFinished: Promise; + decoder: StringDecoder; + onData: (chunk: Buffer | string) => void; +} + +export class ForegroundOutputCapture { + private readonly replayChunks: ReplayChunk[] = []; + private replayCharacters = 0; + private stopped = false; + private disposed = false; + private readonly stdoutCapture: StreamCapture; + private readonly stderrCapture: StreamCapture; + + constructor( + stdout: Readable, + stderr: Readable, + private readonly maxReplayCharacters = ForegroundReplayMaxCharacters, + ) { + this.stdoutCapture = this.createStreamCapture("stdout", stdout); + this.stderrCapture = this.createStreamCapture("stderr", stderr); + } + + private createStreamCapture( + channel: OutputChannel, + stream: Readable, + ): StreamCapture { + const outputPath = path.join( + tmpdir(), + `pochi-foreground-${randomUUID()}-${channel}.log`, + ); + const writer = createWriteStream(outputPath, { flags: "wx" }); + const writerFinished = new Promise((resolve, reject) => { + writer.once("finish", resolve); + writer.once("error", reject); + }); + void writerFinished.catch(() => undefined); + const decoder = new StringDecoder("utf8"); + const onData = (chunk: Buffer | string) => { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + this.appendReplay(channel, decoder.write(buffer)); + if (!writer.write(buffer)) { + stream.pause(); + writer.once("drain", () => { + if (!this.stopped) stream.resume(); + }); + } + }; + stream.on("data", onData); + + return { stream, outputPath, writer, writerFinished, decoder, onData }; + } + + private appendReplay(channel: OutputChannel, value: string): void { + if (value.length === 0) return; + this.replayChunks.push({ channel, value }); + this.replayCharacters += value.length; + + while (this.replayCharacters > this.maxReplayCharacters) { + const firstChunk = this.replayChunks[0]; + if (!firstChunk) break; + const overflow = this.replayCharacters - this.maxReplayCharacters; + if (firstChunk.value.length <= overflow) { + this.replayChunks.shift(); + this.replayCharacters -= firstChunk.value.length; + } else { + firstChunk.value = firstChunk.value.slice(overflow); + this.replayCharacters -= overflow; + } + } + } + + private stop(): void { + if (this.stopped) return; + this.stopped = true; + for (const capture of [this.stdoutCapture, this.stderrCapture]) { + capture.stream.pause(); + capture.stream.removeListener("data", capture.onData); + capture.writer.end(); + } + } + + async finish(): Promise<{ stdout: string; stderr: string }> { + this.stop(); + this.appendReplay("stdout", this.stdoutCapture.decoder.end()); + this.appendReplay("stderr", this.stderrCapture.decoder.end()); + + try { + await Promise.all([ + this.stdoutCapture.writerFinished, + this.stderrCapture.writerFinished, + ]); + let stdout = ""; + let stderr = ""; + for (const chunk of this.replayChunks) { + if (chunk.channel === "stdout") stdout += chunk.value; + else stderr += chunk.value; + } + return { stdout, stderr }; + } finally { + await this.dispose(); + } + } + + promote(): BackgroundJobInitialOutput { + this.stop(); + return { + stdout: this.readCapture(this.stdoutCapture), + stderr: this.readCapture(this.stderrCapture), + dispose: () => this.dispose(), + }; + } + + private async *readCapture( + capture: StreamCapture, + ): AsyncGenerator { + await capture.writerFinished; + for await (const chunk of createReadStream(capture.outputPath)) { + yield chunk; + } + } + + private async dispose(): Promise { + if (this.disposed) return; + this.disposed = true; + await Promise.allSettled([ + this.stdoutCapture.writerFinished, + this.stderrCapture.writerFinished, + ]); + await Promise.all([ + rm(this.stdoutCapture.outputPath, { force: true }), + rm(this.stderrCapture.outputPath, { force: true }), + ]); + } +} diff --git a/packages/cli/src/tools/__tests__/execute-command.test.ts b/packages/cli/src/tools/__tests__/execute-command.test.ts index eecc91b038..0a5d097905 100644 --- a/packages/cli/src/tools/__tests__/execute-command.test.ts +++ b/packages/cli/src/tools/__tests__/execute-command.test.ts @@ -64,6 +64,37 @@ describe("executeCommand", () => { } }); + it("preserves large pre-timeout output without an unbounded replay", async () => { + const outputDir = await mkdtemp(join(tmpdir(), "pochi-cli-large-promotion-")); + try { + const backgroundJobManager = new BackgroundJobManager({ outputDir }); + const outputSize = 2 * 1024 * 1024; + const script = [ + `process.stdout.write('a'.repeat(${outputSize}));`, + "setTimeout(() => process.stdout.write('finished'), 1200);", + ].join(""); + const result = await executeCommand({ backgroundJobManager })( + { + command: `${JSON.stringify(process.execPath)} -e ${JSON.stringify(script)}`, + timeout: 1, + }, + mockToolExecutionOptions, + ); + + const backgroundJobId = result._meta?.backgroundJobId ?? ""; + expect(await backgroundJobManager.waitForAllJobs(5000)).toBe( + "completed", + ); + const output = await readFile(result._meta?.outputFile ?? "", "utf8"); + expect(output).toBe(`${"a".repeat(outputSize)}finished`); + + const job = (backgroundJobManager as any).jobs.get(backgroundJobId); + expect(job.output.length).toBeLessThanOrEqual(1024 * 1024); + } finally { + await rm(outputDir, { recursive: true, force: true }); + } + }); + it("should handle abort signal", async () => { const abortController = new AbortController(); const options = { diff --git a/packages/cli/src/tools/execute-command.ts b/packages/cli/src/tools/execute-command.ts index f0167baee4..e4fffda8ca 100644 --- a/packages/cli/src/tools/execute-command.ts +++ b/packages/cli/src/tools/execute-command.ts @@ -13,6 +13,7 @@ import { createBackgroundCommandResult, } from "@getpochi/tools"; import type { BackgroundJobManager } from "../lib/background-job-manager"; +import { ForegroundOutputCapture } from "../lib/foreground-output-capture"; interface ExecuteCommandContext { backgroundJobManager?: BackgroundJobManager; @@ -143,45 +144,37 @@ function executeForegroundCommand({ env: { ...process.env, ...envs, ...getTerminalEnv() }, stdio: ["ignore", "pipe", "pipe"], }); - const stdoutChunks: Buffer[] = []; - const stderrChunks: Buffer[] = []; + const outputCapture = new ForegroundOutputCapture( + child.stdout, + child.stderr, + ); let state: "foreground" | "promoted" | "settled" = "foreground"; let stopReason: "abort" | "timeout" | undefined; - const onStdout = (chunk: Buffer | string) => { - stdoutChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); - }; - const onStderr = (chunk: Buffer | string) => { - stderrChunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); - }; - child.stdout.on("data", onStdout); - child.stderr.on("data", onStderr); - - const getOutput = () => ({ - stdout: Buffer.concat(stdoutChunks).toString("utf8"), - stderr: Buffer.concat(stderrChunks).toString("utf8"), - }); - let timeoutHandle: ReturnType | undefined; const removeForegroundListeners = () => { if (timeoutHandle) clearTimeout(timeoutHandle); abortSignal?.removeEventListener("abort", onAbort); - child.stdout.removeListener("data", onStdout); - child.stderr.removeListener("data", onStderr); child.removeListener("close", onClose); child.removeListener("error", onError); }; - const settleStoppedCommand = () => { + const settleStoppedCommand = async () => { if (state !== "foreground") return; state = "settled"; removeForegroundListeners(); + let output: CompletedCommandResult; + try { + output = await outputCapture.finish(); + } catch (error) { + reject(error); + return; + } if (stopReason === "abort") { reject(new DOMException("Command execution was aborted", "AbortError")); return; } - const output = getOutput(); reject( new ExecuteCommandError({ message: `Command execution timed out after ${timeout} seconds.`, @@ -191,15 +184,21 @@ function executeForegroundCommand({ ); }; - const onClose = (code: number | null) => { + const onClose = async (code: number | null) => { if (stopReason) { - settleStoppedCommand(); + await settleStoppedCommand(); return; } if (state !== "foreground") return; state = "settled"; removeForegroundListeners(); - const output = getOutput(); + let output: CompletedCommandResult; + try { + output = await outputCapture.finish(); + } catch (error) { + reject(error); + return; + } if (code === 0) { resolve(output); return; @@ -213,28 +212,33 @@ function executeForegroundCommand({ ); }; - const onError = (error: Error) => { + const onError = async (error: Error) => { if (state !== "foreground") return; if (stopReason) { - settleStoppedCommand(); + await settleStoppedCommand(); return; } state = "settled"; removeForegroundListeners(); + try { + await outputCapture.finish(); + } catch { + // Preserve the process error when output cleanup also fails. + } reject(error); }; function onAbort() { if (state !== "foreground") return; stopReason = "abort"; - if (!child.kill()) settleStoppedCommand(); + if (!child.kill()) void settleStoppedCommand(); } const onTimeout = () => { if (state !== "foreground") return; if (!backgroundJobManager) { stopReason = "timeout"; - if (!child.kill()) settleStoppedCommand(); + if (!child.kill()) void settleStoppedCommand(); return; } @@ -242,18 +246,20 @@ function executeForegroundCommand({ child.stdout.pause(); child.stderr.pause(); removeForegroundListeners(); + const initialOutput = outputCapture.promote(); try { resolve( backgroundJobManager.adopt( child, command, - { stdout: stdoutChunks, stderr: stderrChunks }, + initialOutput, abortSignal, ), ); } catch (error) { state = "settled"; child.kill(); + void initialOutput.dispose?.(); reject(error); } }; From 9d59ec91e3a481dbcfb89a48b15a8e3c06fd6f64 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 14:08:47 +0800 Subject: [PATCH 08/12] fix(command): harden promoted output handling --- .../foreground-output-capture.test.ts | 50 +++++++++++++ .../cli/src/lib/foreground-output-capture.ts | 13 +++- .../__test__/execute-command-with-pty.test.ts | 35 ++++++++++ .../terminal/__test__/terminal-job.test.ts | 45 ++++++++++-- .../terminal/execute-command-with-pty.ts | 10 ++- .../src/integrations/terminal/pty-process.ts | 18 +++++ .../src/integrations/terminal/terminal-job.ts | 70 ++++++++++++++++--- 7 files changed, 221 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts index 128e4189a4..c86e8197c0 100644 --- a/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts +++ b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts @@ -1,3 +1,6 @@ +import { once } from "node:events"; +import type { WriteStream } from "node:fs"; +import { stat } from "node:fs/promises"; import { PassThrough } from "node:stream"; import { describe, expect, it } from "vitest"; import { ForegroundOutputCapture } from "../foreground-output-capture"; @@ -16,4 +19,51 @@ describe("ForegroundOutputCapture", () => { stderr: "bcdef", }); }); + + it("creates private spool files and removes them after promotion replay", async () => { + const stdout = new PassThrough(); + const stderr = new PassThrough(); + const capture = new ForegroundOutputCapture(stdout, stderr); + const internals = capture as unknown as { + stdoutCapture: { outputPath: string; writer: WriteStream }; + stderrCapture: { outputPath: string; writer: WriteStream }; + }; + await Promise.all([ + once(internals.stdoutCapture.writer, "open"), + once(internals.stderrCapture.writer, "open"), + ]); + + if (process.platform !== "win32") { + const [stdoutStat, stderrStat] = await Promise.all([ + stat(internals.stdoutCapture.outputPath), + stat(internals.stderrCapture.outputPath), + ]); + expect(stdoutStat.mode & 0o777).toBe(0o600); + expect(stderrStat.mode & 0o777).toBe(0o600); + } + + stdout.write("stdout"); + stderr.write("stderr"); + const promoted = capture.promote(); + const collect = async ( + chunks: AsyncIterable | Iterable, + ) => { + const output: Buffer[] = []; + for await (const chunk of chunks) { + output.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + } + return Buffer.concat(output).toString("utf8"); + }; + + await expect( + Promise.all([collect(promoted.stdout), collect(promoted.stderr)]), + ).resolves.toEqual(["stdout", "stderr"]); + await expect(stat(internals.stdoutCapture.outputPath)).rejects.toMatchObject( + { code: "ENOENT" }, + ); + await expect(stat(internals.stderrCapture.outputPath)).rejects.toMatchObject( + { code: "ENOENT" }, + ); + await promoted.dispose?.(); + }); }); diff --git a/packages/cli/src/lib/foreground-output-capture.ts b/packages/cli/src/lib/foreground-output-capture.ts index 04403701ec..80e8b0f19d 100644 --- a/packages/cli/src/lib/foreground-output-capture.ts +++ b/packages/cli/src/lib/foreground-output-capture.ts @@ -50,7 +50,10 @@ export class ForegroundOutputCapture { tmpdir(), `pochi-foreground-${randomUUID()}-${channel}.log`, ); - const writer = createWriteStream(outputPath, { flags: "wx" }); + const writer = createWriteStream(outputPath, { + flags: "wx", + mode: 0o600, + }); const writerFinished = new Promise((resolve, reject) => { writer.once("finish", resolve); writer.once("error", reject); @@ -136,8 +139,12 @@ export class ForegroundOutputCapture { capture: StreamCapture, ): AsyncGenerator { await capture.writerFinished; - for await (const chunk of createReadStream(capture.outputPath)) { - yield chunk; + try { + for await (const chunk of createReadStream(capture.outputPath)) { + yield chunk; + } + } finally { + await rm(capture.outputPath, { force: true }); } } diff --git a/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts b/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts index 5e61379944..9dfdd08e48 100644 --- a/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/execute-command-with-pty.test.ts @@ -96,4 +96,39 @@ describe("execute-command-with-pty", () => { clock.restore(); } }); + + it("treats a natural signal exit as a command failure", async () => { + let exitListener: + | ((event: { exitCode: number; signal?: number }) => void) + | undefined; + const ptyProcess = { + kill: sinon.stub(), + onData: () => ({ dispose: sinon.stub() }), + onExit: ( + listener: (event: { exitCode: number; signal?: number }) => void, + ) => { + exitListener = listener; + return { dispose: sinon.stub() }; + }, + }; + const spawn = sinon.stub().resolves(ptyProcess); + const { executeCommandWithPty } = proxyquire + .noCallThru() + .noPreserveCache() + .load("../execute-command-with-pty", { + "./pty-process": { + PtyProcess: { spawn }, + }, + }) as typeof import("../execute-command-with-pty"); + + const resultPromise = executeCommandWithPty({ + command: "kill -TERM $$", + cwd: "/tmp", + timeout: 5, + }); + await Promise.resolve(); + exitListener?.({ exitCode: 0, signal: 15 }); + + await assert.rejects(resultPromise, /exited with code 143/); + }); }); diff --git a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts index 40ad9a8a97..ba2a15e64d 100644 --- a/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts +++ b/packages/vscode/src/integrations/terminal/__test__/terminal-job.test.ts @@ -21,10 +21,12 @@ class TestEventEmitter { class TestPtyProcess { private readonly dataListeners = new Set<(data: string) => void>(); private readonly exitListeners = new Set< - (event: { exitCode: number }) => void + (event: { exitCode: number; signal?: number }) => void >(); readonly replay: string[] = []; killCalls = 0; + pauseCalls = 0; + resumeCalls = 0; subscribeWithReplay(listener: (data: string) => void) { this.dataListeners.add(listener); @@ -34,7 +36,7 @@ class TestPtyProcess { }; } - onExit(listener: (event: { exitCode: number }) => void) { + onExit(listener: (event: { exitCode: number; signal?: number }) => void) { this.exitListeners.add(listener); return { dispose: () => this.exitListeners.delete(listener) }; } @@ -44,13 +46,23 @@ class TestPtyProcess { for (const listener of [...this.dataListeners]) listener(data); } - emitExit(exitCode: number): void { - for (const listener of [...this.exitListeners]) listener({ exitCode }); + emitExit(exitCode: number, signal?: number): void { + for (const listener of [...this.exitListeners]) { + listener({ exitCode, ...(signal !== undefined ? { signal } : {}) }); + } } kill(): void { this.killCalls++; } + + pauseOutput(): void { + this.pauseCalls++; + } + + resumeOutput(): void { + this.resumeCalls++; + } } class TestExecutionError extends Error { @@ -358,6 +370,21 @@ describe("TerminalJob", () => { assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); }); + it("applies backpressure while persisted pty output is queued", async () => { + const harness = createHarness(); + const chunk = "x".repeat(600 * 1024); + + harness.ptyProcess.emitData(chunk); + harness.ptyProcess.emitData(chunk); + assert.strictEqual(harness.ptyProcess.pauseCalls, 1); + + await flushPromises(); + assert.strictEqual(harness.ptyProcess.resumeCalls, 1); + + harness.ptyProcess.emitExit(0); + await flushPromises(); + }); + it("opens, detaches, and recreates the terminal without stopping the pty", async () => { const harness = createHarness(); assert.strictEqual(harness.job.isVisible, false); @@ -439,6 +466,16 @@ describe("TerminalJob", () => { assert.match(harness.finishEvents[0]?.error ?? "", /exited with code 2/); }); + it("marks a natural signal exit as failed", async () => { + const harness = createHarness(); + harness.ptyProcess.emitExit(0, 15); + await flushPromises(); + + assert.strictEqual(harness.finishEvents[0]?.status, "failed"); + assert.strictEqual(harness.finishEvents[0]?.exitCode, 143); + assert.match(harness.finishEvents[0]?.error ?? "", /signal 15/); + }); + it("publishes a close failure through the output manager", async () => { const harness = createHarness({ closeError: new Error("flush failed") }); harness.ptyProcess.emitExit(0); diff --git a/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts b/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts index 7d16ef8f3a..13f10fe7f2 100644 --- a/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts +++ b/packages/vscode/src/integrations/terminal/execute-command-with-pty.ts @@ -63,13 +63,17 @@ export const executeCommandWithPty = async ({ onData?.(truncateOutput(output)); }); - const exitListener = ptyProcess.onExit(({ exitCode }) => { + const exitListener = ptyProcess.onExit(({ exitCode, signal }) => { settle(() => { - if (exitCode === 0) { + const effectiveExitCode = + signal !== undefined && signal > 0 ? 128 + signal : exitCode; + if (effectiveExitCode === 0) { resolve({ type: "completed", ...truncateOutput(output) }); } else { reject( - ExecutionError.create(`Command exited with code ${exitCode}.`), + ExecutionError.create( + `Command exited with code ${effectiveExitCode}.`, + ), ); } }); diff --git a/packages/vscode/src/integrations/terminal/pty-process.ts b/packages/vscode/src/integrations/terminal/pty-process.ts index 345a8ce929..25f9a4a62c 100644 --- a/packages/vscode/src/integrations/terminal/pty-process.ts +++ b/packages/vscode/src/integrations/terminal/pty-process.ts @@ -188,6 +188,24 @@ export class PtyProcess { } } + pauseOutput(): void { + if (this.rawExitEvent) return; + try { + this.process.pause(); + } catch (error) { + logger.debug("Failed to pause exited pty process", error); + } + } + + resumeOutput(): void { + if (this.rawExitEvent) return; + try { + this.process.resume(); + } catch (error) { + logger.debug("Failed to resume exited pty process", error); + } + } + kill(signal = "SIGTERM"): void { if (this.rawExitEvent) return; diff --git a/packages/vscode/src/integrations/terminal/terminal-job.ts b/packages/vscode/src/integrations/terminal/terminal-job.ts index ef2454c92b..2354d621cb 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -17,6 +17,9 @@ import { PtyTerminal } from "./pty-terminal"; import { ExecutionError } from "./utils"; const logger = getLogger("TerminalJob"); +const PtyOutputPauseThresholdCharacters = 1024 * 1024; +const PtyOutputResumeThresholdCharacters = + PtyOutputPauseThresholdCharacters / 2; export interface TerminalJobConfig { name: string; @@ -51,6 +54,8 @@ export class TerminalJob implements vscode.Disposable { private readonly sanitizer = new PlainOutputSanitizer(); private readonly disposables: vscode.Disposable[] = []; private outputQueue: Promise = Promise.resolve(); + private pendingOutputCharacters = 0; + private ptyOutputPaused = false; private pendingTerminalSuffix = ""; private persistenceError: ExecutionError | undefined; private stopRequested = false; @@ -233,8 +238,16 @@ export class TerminalJob implements vscode.Disposable { ); this.disposables.push( - ptyProcess.onExit(({ exitCode }) => { - void this.finalize(exitCode); + ptyProcess.onExit(({ exitCode, signal }) => { + const effectiveExitCode = + signal !== undefined && signal > 0 ? 128 + signal : exitCode; + const signalError = + signal !== undefined && signal > 0 && !this.stopRequested + ? ExecutionError.create( + `Background job execution terminated by signal ${signal}.`, + ) + : undefined; + void this.finalize(effectiveExitCode, signalError); }), ); } @@ -479,17 +492,49 @@ export class TerminalJob implements vscode.Disposable { private enqueueFileOutput(text: string, addToManager: boolean): void { if (text.length === 0 || this.persistenceError) return; + this.pendingOutputCharacters += text.length; + this.updatePtyOutputFlowControl(); const write = this.outputQueue.then(async () => { await this.outputWriter.append(text); if (addToManager) this.outputManager.addChunk(text); }); - this.outputQueue = write.catch((error) => { - if (this.persistenceError) return; - this.persistenceError = ExecutionError.create( - error instanceof Error ? error.message : String(error), - ); - this.requestStop("background output persistence failed"); - }); + this.outputQueue = write + .catch((error) => { + if (this.persistenceError) return; + this.persistenceError = ExecutionError.create( + error instanceof Error ? error.message : String(error), + ); + this.requestStop("background output persistence failed"); + }) + .finally(() => { + this.pendingOutputCharacters = Math.max( + 0, + this.pendingOutputCharacters - text.length, + ); + this.updatePtyOutputFlowControl(); + }); + } + + private updatePtyOutputFlowControl(): void { + if (!this.ptyProcess) return; + if ( + !this.ptyOutputPaused && + this.pendingOutputCharacters >= PtyOutputPauseThresholdCharacters + ) { + this.ptyOutputPaused = true; + this.ptyProcess.pauseOutput(); + return; + } + if ( + this.ptyOutputPaused && + this.pendingOutputCharacters <= PtyOutputResumeThresholdCharacters && + !this.stopRequested && + !this.finished && + !this.persistenceError + ) { + this.ptyOutputPaused = false; + this.ptyProcess.resumeOutput(); + } } private async finalize( @@ -509,7 +554,12 @@ export class TerminalJob implements vscode.Disposable { this.setVisible(false); let executionError = initialError ?? this.persistenceError; - if (exitCode !== undefined && exitCode !== 0 && !this.stopRequested) { + if ( + exitCode !== undefined && + exitCode !== 0 && + !this.stopRequested && + !executionError + ) { executionError = ExecutionError.create( `Background job execution exited with code ${exitCode}.`, ); From 031010996db6183d4cb0f730fd809e65edcbf023 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 14:54:31 +0800 Subject: [PATCH 09/12] docs(common): clarify terminal session description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Describe the environment field in terms relevant to the model without implying managed command terminals are always hidden. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- packages/common/src/base/environment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/common/src/base/environment.ts b/packages/common/src/base/environment.ts index 86be522297..f4af81d06d 100644 --- a/packages/common/src/base/environment.ts +++ b/packages/common/src/base/environment.ts @@ -71,7 +71,7 @@ export const Environment = z.object({ ) .optional() .describe( - "Available terminals and detachable background command sessions in the VS Code workspace.", + "Terminal sessions available in the VS Code workspace, including active managed background commands.", ), }) .describe("Information about the workspace."), From 621aca8a63f427404e86d05b79ba07884dcea6f6 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 15:27:02 +0800 Subject: [PATCH 10/12] test(cli): avoid oversized output diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep promoted output integrity checks strict while preventing multi-megabyte assertion diffs from stalling CI. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- packages/cli/src/tools/__tests__/execute-command.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/tools/__tests__/execute-command.test.ts b/packages/cli/src/tools/__tests__/execute-command.test.ts index 0a5d097905..0fb5f0bd6c 100644 --- a/packages/cli/src/tools/__tests__/execute-command.test.ts +++ b/packages/cli/src/tools/__tests__/execute-command.test.ts @@ -86,7 +86,10 @@ describe("executeCommand", () => { "completed", ); const output = await readFile(result._meta?.outputFile ?? "", "utf8"); - expect(output).toBe(`${"a".repeat(outputSize)}finished`); + const suffix = "finished"; + expect(output.length).toBe(outputSize + suffix.length); + expect(output.slice(-suffix.length)).toBe(suffix); + expect(output.slice(0, outputSize).search(/[^a]/)).toBe(-1); const job = (backgroundJobManager as any).jobs.get(backgroundJobId); expect(job.output.length).toBeLessThanOrEqual(1024 * 1024); From 370fed6fe94d9eafd2c2e34727fc980d626c0e91 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 15:36:47 +0800 Subject: [PATCH 11/12] test(cli): wait for promoted output flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the Linux promotion regression deterministic by keeping the child alive until its final stdout marker has been flushed. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- packages/cli/src/tools/__tests__/execute-command.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/tools/__tests__/execute-command.test.ts b/packages/cli/src/tools/__tests__/execute-command.test.ts index 0fb5f0bd6c..8b4032b9b9 100644 --- a/packages/cli/src/tools/__tests__/execute-command.test.ts +++ b/packages/cli/src/tools/__tests__/execute-command.test.ts @@ -71,7 +71,9 @@ describe("executeCommand", () => { const outputSize = 2 * 1024 * 1024; const script = [ `process.stdout.write('a'.repeat(${outputSize}));`, - "setTimeout(() => process.stdout.write('finished'), 1200);", + "setTimeout(() => {", + "process.stdout.write('finished', () => process.exit(0));", + "}, 1200);", ].join(""); const result = await executeCommand({ backgroundJobManager })( { From 8a0685024b7ed0ac3deff628762ead0929b78b26 Mon Sep 17 00:00:00 2001 From: zhanba Date: Fri, 4 Sep 2026 15:50:36 +0800 Subject: [PATCH 12/12] fix(cli): preserve live output during promotion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attach to promoted process streams before replaying spooled output so output emitted during handoff cannot be lost when the child exits. 🤖 Generated with [Pochi](https://getpochi.com) | [Task](https://app.getpochi.com/share/p-59931c0d563a44bcbfd37fb93d8271a4) Co-Authored-By: Pochi --- .../__tests__/background-job-manager.test.ts | 41 ++++++++++++++ .../cli/src/lib/background-job-manager.ts | 54 ++++++++++++++++--- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/lib/__tests__/background-job-manager.test.ts b/packages/cli/src/lib/__tests__/background-job-manager.test.ts index 850c419e0c..56695b74d5 100644 --- a/packages/cli/src/lib/__tests__/background-job-manager.test.ts +++ b/packages/cli/src/lib/__tests__/background-job-manager.test.ts @@ -1,6 +1,9 @@ +import type { ChildProcess } from "node:child_process"; +import { EventEmitter } from "node:events"; import { mkdtemp, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { PassThrough } from "node:stream"; import { describe, expect, it } from "vitest"; import { BackgroundJobManager } from "../background-job-manager"; @@ -44,6 +47,44 @@ describe("BackgroundJobManager", () => { expect(result2?.output).toBe(""); }); + it("captures live adopted output while replaying initial output", async () => { + const outputDir = await mkdtemp(join(tmpdir(), "pochi-bgjob-adopt-test-")); + try { + const manager = new BackgroundJobManager({ outputDir }); + const stdout = new PassThrough(); + const stderr = new PassThrough(); + const child = Object.assign(new EventEmitter(), { + stdout, + stderr, + kill: () => true, + }) as unknown as ChildProcess; + let releaseReplay: () => void = () => {}; + const replayGate = new Promise((resolve) => { + releaseReplay = resolve; + }); + const initialStdout = (async function* () { + yield Buffer.from("before"); + await replayGate; + })(); + + const { outputFile } = manager.adopt(child, "test", { + stdout: initialStdout, + stderr: [], + }); + stdout.end("after"); + stderr.end(); + stdout.destroy(); + stderr.destroy(); + releaseReplay(); + child.emit("close", 0); + + expect(await manager.waitForAllJobs(1000)).toBe("completed"); + expect(await readFile(outputFile, "utf8")).toBe("beforeafter"); + } finally { + await rm(outputDir, { recursive: true, force: true }); + } + }); + it("should guide agents away from rapid empty reads", () => { const manager = new BackgroundJobManager(); const { backgroundJobId } = manager.start("sleep 10", "."); diff --git a/packages/cli/src/lib/background-job-manager.ts b/packages/cli/src/lib/background-job-manager.ts index c237ab5276..f3370fa4c0 100644 --- a/packages/cli/src/lib/background-job-manager.ts +++ b/packages/cli/src/lib/background-job-manager.ts @@ -133,14 +133,56 @@ export class BackgroundJobManager { ) => { const decoder = new StringDecoder("utf8"); const sanitizer = new PlainOutputSanitizer(); - for await (const chunk of initialOutputStream) { - await appendOutput(sanitizer.write(decoder.write(chunk))); - } - if (stream) { - for await (const chunk of stream) { + const initialOutputFinished = (async () => { + for await (const chunk of initialOutputStream) { await appendOutput(sanitizer.write(decoder.write(chunk))); } - } + })(); + const liveOutputFinished = stream + ? new Promise((resolve, reject) => { + let liveOutputTail = initialOutputFinished; + let settled = false; + const cleanup = () => { + stream.removeListener("data", onData); + stream.removeListener("end", onFinished); + stream.removeListener("close", onFinished); + stream.removeListener("error", onError); + }; + const settle = (error?: unknown) => { + if (settled) return; + settled = true; + cleanup(); + liveOutputTail.then( + () => (error === undefined ? resolve() : reject(error)), + reject, + ); + }; + const onData = (chunk: Buffer | string) => { + stream.pause(); + liveOutputTail = liveOutputTail + .then(() => appendOutput(sanitizer.write(decoder.write(chunk)))) + .then(() => { + if (!settled) stream.resume(); + }); + void liveOutputTail.catch(onError); + }; + const onFinished = () => settle(); + const onError = (error: unknown) => settle(error); + + // Foreground capture pauses the child streams before handing them + // off. Install the live listener first, then resume. Pausing again + // on each chunk keeps the handoff bounded while initial output is + // replayed and retains the chunk even if the stream closes. + stream.on("data", onData); + stream.once("end", onFinished); + stream.once("close", onFinished); + stream.once("error", onError); + void initialOutputFinished.catch(onError); + stream.resume(); + }) + : Promise.resolve(); + + await Promise.all([initialOutputFinished, liveOutputFinished]); // StringDecoder buffers an incomplete trailing UTF-8 sequence. A // manually stopped process may end in the middle of a character, so