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/__tests__/foreground-output-capture.test.ts b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts new file mode 100644 index 0000000000..c86e8197c0 --- /dev/null +++ b/packages/cli/src/lib/__tests__/foreground-output-capture.test.ts @@ -0,0 +1,69 @@ +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"; + +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", + }); + }); + + 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/background-job-manager.ts b/packages/cli/src/lib/background-job-manager.ts index 0add65a521..f3370fa4c0 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,16 @@ export interface BackgroundJobStartResult { outputFile: string; } +export type BackgroundJobInitialOutputStream = + | Iterable + | AsyncIterable; + +export interface BackgroundJobInitialOutput { + stdout: BackgroundJobInitialOutputStream; + stderr: BackgroundJobInitialOutputStream; + dispose?: () => Promise; +} + export interface BackgroundJobManagerOptions { taskId?: string; outputDir?: string; @@ -50,6 +62,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 +95,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,46 +108,113 @@ 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; } - } else { - job.output += chunk; + }); + return appendTail; + }; + + const consumeOutput = async ( + stream: Readable | null, + initialOutputStream: BackgroundJobInitialOutputStream, + ) => { + const decoder = new StringDecoder("utf8"); + const sanitizer = new PlainOutputSanitizer(); + 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 + // 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) => { - outputError = error; - child.kill(); - }); + const outputFinished = Promise.all([ + consumeOutput(child.stdout, initialOutput.stdout), + consumeOutput(child.stderr, initialOutput.stderr), + ]) + .finally(() => initialOutput.dispose?.()) + .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 @@ -172,6 +267,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/lib/foreground-output-capture.ts b/packages/cli/src/lib/foreground-output-capture.ts new file mode 100644 index 0000000000..80e8b0f19d --- /dev/null +++ b/packages/cli/src/lib/foreground-output-capture.ts @@ -0,0 +1,163 @@ +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", + mode: 0o600, + }); + 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; + try { + for await (const chunk of createReadStream(capture.outputPath)) { + yield chunk; + } + } finally { + await rm(capture.outputPath, { force: true }); + } + } + + 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 75c68b7af0..8b4032b9b9 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,73 @@ 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("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', () => process.exit(0));", + "}, 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"); + 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); + } finally { + await rm(outputDir, { recursive: true, force: true }); + } + }); + it("should handle abort signal", async () => { const abortController = new AbortController(); const options = { @@ -48,6 +119,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..e4fffda8ca 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,12 @@ import { type ToolFunctionType, createBackgroundCommandResult, } from "@getpochi/tools"; -import type { ToolCallOptions } from "../types"; +import type { BackgroundJobManager } from "../lib/background-job-manager"; +import { ForegroundOutputCapture } from "../lib/foreground-output-capture"; + +interface ExecuteCommandContext { + backgroundJobManager?: BackgroundJobManager; +} export class ExecuteCommandError extends Error { public code: number; @@ -43,7 +44,7 @@ export class ExecuteCommandError extends Error { export const executeCommand = ( - context?: ToolCallOptions, + context?: ExecuteCommandContext, ): ToolFunctionType => async ( { @@ -75,94 +76,203 @@ 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 outputCapture = new ForegroundOutputCapture( + child.stdout, + child.stderr, + ); + let state: "foreground" | "promoted" | "settled" = "foreground"; + let stopReason: "abort" | "timeout" | undefined; + + let timeoutHandle: ReturnType | undefined; + const removeForegroundListeners = () => { + if (timeoutHandle) clearTimeout(timeoutHandle); + abortSignal?.removeEventListener("abort", onAbort); + child.removeListener("close", onClose); + child.removeListener("error", onError); + }; + + 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; + } + + reject( + new ExecuteCommandError({ + message: `Command execution timed out after ${timeout} seconds.`, + ...output, + code: 1, + }), + ); + }; + + const onClose = async (code: number | null) => { + if (stopReason) { + await settleStoppedCommand(); + return; + } + if (state !== "foreground") return; + state = "settled"; + removeForegroundListeners(); + let output: CompletedCommandResult; + try { + output = await outputCapture.finish(); + } catch (error) { + reject(error); + return; + } + if (code === 0) { + resolve(output); + return; + } + reject( + new ExecuteCommandError({ + message: `Command exited with code ${code ?? 1}`, + ...output, + code: code ?? 1, + }), + ); + }; + + const onError = async (error: Error) => { + if (state !== "foreground") return; + if (stopReason) { + 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()) void settleStoppedCommand(); + } + + const onTimeout = () => { + if (state !== "foreground") return; + if (!backgroundJobManager) { + stopReason = "timeout"; + if (!child.kill()) void settleStoppedCommand(); + return; + } + + state = "promoted"; + child.stdout.pause(); + child.stderr.pause(); + removeForegroundListeners(); + const initialOutput = outputCapture.promote(); + try { + resolve( + backgroundJobManager.adopt( + child, + command, + initialOutput, + abortSignal, + ), + ); + } catch (error) { + state = "settled"; + child.kill(); + void initialOutput.dispose?.(); + 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/common/src/base/environment.ts b/packages/common/src/base/environment.ts index 76b3e1acdc..f4af81d06d 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( + "Terminal sessions available in the VS Code workspace, including active managed background commands.", + ), }) .describe("Information about the workspace."), info: z diff --git a/packages/common/src/vscode-webui-bridge/index.ts b/packages/common/src/vscode-webui-bridge/index.ts index 8b97b48439..4d61ab1014 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 { + BackgroundCommands, VSCodeHostApi, WebviewHostApi, } from "./webview"; 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/common/src/vscode-webui-bridge/webview-stub.ts b/packages/common/src/vscode-webui-bridge/webview-stub.ts index 5efc3dd1d0..76e69cd2b9 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 { + BackgroundCommands, BuiltinSubAgentInfo, CaptureEvent, ChangedFileContent, @@ -289,6 +290,17 @@ const VSCodeHostStub = { }, }); }, + readBackgroundCommands: async () => { + return 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 () => { 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..24911f2052 100644 --- a/packages/common/src/vscode-webui-bridge/webview.ts +++ b/packages/common/src/vscode-webui-bridge/webview.ts @@ -51,6 +51,8 @@ import type { DisplayModel } from "./types/model"; import type { PochiCredentials } from "./types/pochi"; import type { VSCodeSettings } from "./types/vscode-settings"; +export type BackgroundCommands = Record; + export interface VSCodeHostApi { readResourceURI(): Promise; @@ -188,6 +190,13 @@ export interface VSCodeHostApi { openBackgroundJobTerminal: (backgroundJobId: string) => Promise; }>; + readBackgroundCommands(): Promise<{ + backgroundCommands: ThreadSignalSerialization; + show: (backgroundJobId: string) => Promise; + hide: (backgroundJobId: string) => Promise; + close: (backgroundJobId: string) => Promise; + }>; + readBackgroundJobNotifications(taskId: string): Promise<{ notifications: ThreadSignalSerialization; acknowledge: (notificationId: string) => Promise; diff --git a/packages/tools/src/execute-command.ts b/packages/tools/src/execute-command.ts index 06780c07e4..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, 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. +- 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. @@ -189,7 +190,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__/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/__tests__/execute-command.test.tsx b/packages/vscode-webui/src/features/tools/components/__tests__/execute-command.test.tsx index d690d3b85c..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,5 +89,52 @@ 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 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"); }); }); 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/features/tools/components/execute-command.tsx b/packages/vscode-webui/src/features/tools/components/execute-command.tsx index fa131e840e..26ba40b58b 100644 --- a/packages/vscode-webui/src/features/tools/components/execute-command.tsx +++ b/packages/vscode-webui/src/features/tools/components/execute-command.tsx @@ -28,6 +28,9 @@ export const executeCommandTool: React.FC> = ({ }, [lifecycle.abort]); const { cwd, command, background } = tool.input || {}; + const backgroundJobMetadata = + tool.state === "output-available" ? tool.output._meta : undefined; + const isPromoted = !background && Boolean(backgroundJobMetadata); const cwdNode = cwd ? ( {" "} @@ -36,13 +39,20 @@ export const executeCommandTool: React.FC> = ({ ) : null; const text = background ? t("toolInvocation.backgroundExecute") - : t("toolInvocation.executeCommand"); + : isPromoted + ? t("toolInvocation.startedCommand") + : t("toolInvocation.executeCommand"); const title = ( <> {text} {cwdNode} + {isPromoted && ( + + {t("toolInvocation.promotedToBackground")} + + )} ); @@ -53,9 +63,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 (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}} 个工具", 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-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-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-webui/src/lib/vscode.ts b/packages/vscode-webui/src/lib/vscode.ts index 32b21e18e3..357a20f7ce 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", + "readBackgroundCommands", "readModelList", "readUserStorage", "readCustomAgents", 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..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 @@ -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,88 @@ 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(); + } + }); + + 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__/pty-process.test.ts b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts new file mode 100644 index 0000000000..a79fde2774 --- /dev/null +++ b/packages/vscode/src/integrations/terminal/__test__/pty-process.test.ts @@ -0,0 +1,126 @@ +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("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[] = []; + 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..70dcffafee --- /dev/null +++ b/packages/vscode/src/integrations/terminal/__test__/pty-terminal.test.ts @@ -0,0 +1,99 @@ +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("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: dataSubscription, + }), + onExit: () => exitSubscription, + write: sinon.stub(), + resize: sinon.stub(), + kill: 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); + 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 dac54b64e7..ba2a15e64d 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,74 @@ 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; signal?: number }) => void + >(); + readonly replay: string[] = []; + killCalls = 0; + pauseCalls = 0; + resumeCalls = 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; signal?: number }) => void) { + this.exitListeners.add(listener); + return { dispose: () => this.exitListeners.delete(listener) }; + } + + emitData(data: string): void { + this.replay.push(data); + for (const listener of [...this.dataListeners]) listener(data); + } + + 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 { - 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 +83,44 @@ 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; + let terminalShowCalls = 0; + const terminalShowPreserveFocus: Array = []; const terminal: FakeTerminal = { - shellIntegration, - show: () => {}, + show: (preserveFocus) => { + terminalShowCalls++; + terminalShowPreserveFocus.push(preserveFocus); + }, 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 ptyTerminalCloseCallbacks: Array<() => void> = []; + 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 +129,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,262 +158,365 @@ 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, + onCloseRequested: () => void, + ) { + ptyTerminalCloseCallbacks.push(onCloseRequested); + 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, + ptyTerminalCloseCallbacks, terminal, get terminalDisposeCalls() { return terminalDisposeCalls; }, + get terminalShowCalls() { + return terminalShowCalls; + }, + terminalShowPreserveFocus, }; } -interface FakeExecution { - read(): AsyncIterable; -} - -interface FakeShellIntegration { - executeCommand(command: string): FakeExecution; -} - interface FakeTerminal { - shellIntegration: FakeShellIntegration; - show(): void; + show(preserveFocus?: boolean): void; dispose(): void; } describe("TerminalJob", () => { - it("closes the terminal after a background command completes", async () => { - const harness = createHarness(); + 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.deepStrictEqual(executeCommandCalls, []); + assert.strictEqual(finishEvents[0]?.status, "stopped"); + }); + + it("keeps an adopted pty running when terminal view creation fails", async () => { + const initializationError = new Error("terminal creation failed"); + const harness = createHarness({ createTerminalError: initializationError }); + + 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"), + harness.job, + ); + + harness.ptyProcess.emitExit(0); await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 0, - }); + }); + + it("replays foreground output and completes", async () => { + const harness = createHarness({ replay: ["before timeout\n"] }); + 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.strictEqual(harness.finalizeCalls.length, 1); - assert.strictEqual(harness.finalizeCalls[0], undefined); + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.job.isFinished, true); assert.deepStrictEqual(harness.lifecycle, [ "output:$ sleep 10\n", + "output:before timeout\n", + "manager:before timeout\n", + "output:after timeout\n", + "manager:after timeout\n", "file-closed", "event-fired", + "manager-deleted", ]); - 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.finishEvents[0]?.status, "completed"); + assert.strictEqual(harness.terminalDisposeCalls, 0); assert.strictEqual(harness.TerminalJob.get(harness.job.id), undefined); }); - it("finalizes a running job when its terminal closes", async () => { - const { TerminalJob, finalizeCalls, job, terminal } = createHarness(); + 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(); - terminal.dispose(); - await flushPromises(); + assert.strictEqual(harness.ptyProcess.resumeCalls, 1); - assert.strictEqual(TerminalJob.get(job.id), undefined); - assert.strictEqual(finalizeCalls.length, 1); - assert.match( - finalizeCalls[0]?.message ?? "", - /user closed terminal/, - ); + harness.ptyProcess.emitExit(0); + await flushPromises(); }); - 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; - }, - }); + 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); - await flushPromises(); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:ready", - ]); + harness.job.show(); - harness.job.kill(); - finishOutput?.(); + 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.job.show(); + + harness.terminal.dispose(); + + assert.strictEqual(harness.job.isVisible, false); + assert.strictEqual(harness.ptyProcess.killCalls, 0); + + harness.ptyProcess.emitExit(0); await flushPromises(); + }); - assert.deepStrictEqual(harness.lifecycle, [ - "output:$ sleep 10\n", - "output:ready", - "file-closed", - "event-fired", - ]); + it("closes the terminal when explicitly closing the pty process", async () => { + const harness = createHarness(); + harness.job.show(); + + 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("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"); + assert.strictEqual(harness.terminalDisposeCalls, 0); + }); + + 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"; - }, - }); - + it("marks a natural signal exit as failed", async () => { + const harness = createHarness(); + harness.ptyProcess.emitExit(0, 15); await flushPromises(); - harness.executionEndEmitter.fire({ - execution: harness.execution, - exitCode: 0, - }); + + 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); 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..13f10fe7f2 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,72 @@ 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(); - - if (exitCode === 0) { - resolve(truncateOutput(output)); + }; + + const dataListener = ptyProcess.onData((data) => { + output += data; + onData?.(truncateOutput(output)); + }); + + const exitListener = ptyProcess.onExit(({ exitCode, signal }) => { + settle(() => { + 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}.`, + ), ); } }); - }, - ); + }); + + 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..25f9a4a62c --- /dev/null +++ b/packages/vscode/src/integrations/terminal/pty-process.ts @@ -0,0 +1,259 @@ +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 ReplayHistoryMaxCharacters = 1_000_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 historyCharacters = 0; + 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.appendHistory(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 }; + } + + 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. */ + 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); + } + } + + 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; + + 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..ba8cc0bb1c --- /dev/null +++ b/packages/vscode/src/integrations/terminal/pty-terminal.ts @@ -0,0 +1,71 @@ +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 { + this.opened = false; + 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..2354d621cb 100644 --- a/packages/vscode/src/integrations/terminal/terminal-job.ts +++ b/packages/vscode/src/integrations/terminal/terminal-job.ts @@ -8,61 +8,69 @@ 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"; +import { PtyProcess, PtySpawnError } from "./pty-process"; +import { PtyTerminal } from "./pty-terminal"; import { ExecutionError } from "./utils"; const logger = getLogger("TerminalJob"); +const PtyOutputPauseThresholdCharacters = 1024 * 1024; +const PtyOutputResumeThresholdCharacters = + PtyOutputPauseThresholdCharacters / 2; -/** - * 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 onDidCreateEmitter = + new vscode.EventEmitter(); + static readonly onDidCreate = TerminalJob.onDidCreateEmitter.event; private static readonly onDidDisposeEmitter = new vscode.EventEmitter(); static readonly onDidDispose = TerminalJob.onDidDisposeEmitter.event; private static readonly onDidFinishEmitter = 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 static readonly onDidChangeVisibilityEmitter = + new vscode.EventEmitter(); + static readonly onDidChangeVisibility = + TerminalJob.onDidChangeVisibilityEmitter.event; + + private terminal: vscode.Terminal | undefined; + private ptyTerminal: PtyTerminal | undefined; + private outputManager!: OutputManager; + private outputWriter!: BackgroundJobOutputFile; + 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; + 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; + readonly terminalVisibility = signal(false); get output() { return this.outputManager.output; @@ -72,329 +80,516 @@ export class TerminalJob implements vscode.Disposable { return this.config.command; } - private constructor(private readonly config: TerminalJobConfig) { - 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); + get name() { + return this.config.name; + } - // 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, - }); + get isPtyTerminal() { + return this.ptyProcess !== undefined; + } - this.terminalClosed = new Promise((_, reject) => { - this.rejectTerminalClosed = reject; - }); + get isFinished() { + return this.finished; + } - // 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(); - } - }); + get isVisible() { + return this.terminalVisibility.value; + } - this.terminal.show(); + private constructor( + private readonly config: TerminalJobConfig, + private readonly ptyProcess?: PtyProcess, + ) { + this.id = createBackgroundJobId("command"); + this.outputFile = getBackgroundJobOutputPath(config.taskId, this.id); - this.execute(); + 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.initializeLifecycle(); + if (!this.stopRequested) { + if (!ptyProcess) { + void this.executeWithShellIntegration(); + } + } else if (!ptyProcess) { + void this.finalize(undefined, ExecutionError.createAbortError()); + } + } catch (error) { + this.cleanupAfterInitializationFailure(); + throw error; + } + TerminalJob.onDidCreateEmitter.fire(this); 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(); - - // Check if already aborted - if (this.config.abortSignal?.aborted) { - reject(abortError); - return; - } + 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, + ); + } - // Set up abort listener - const abortListener = () => { - logger.info(`Command execution aborted: ${this.config.command}`); - this.stopRequested = true; - this.terminal.dispose(); - reject(abortError); - }; + static list(): readonly TerminalJob[] { + return Array.from(TerminalJob.jobs.values()); + } - this.config.abortSignal?.addEventListener("abort", abortListener, { - once: true, - }); + show(): void { + if (this.finished || this.stopRequested) return; + if (this.ptyProcess && !this.terminal) { + this.createPtyTerminalView(); + } + this.terminal?.show(false); + this.setVisible(true); + } - // Clean up timeout if promise chain is resolved elsewhere - // This is a fallback cleanup mechanism - const cleanup = () => { - this.config.abortSignal?.removeEventListener("abort", abortListener); - }; + 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(); + } - // Store cleanup function for potential use in dispose - this.disposables.push({ - dispose: cleanup, - }); - }); + closePtyProcess(): void { + if (!this.ptyProcess) return; + this.hide(); + this.requestStop("close requested"); } - /** - * 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; + kill(): void { + this.requestStop("kill requested"); + } - await this.outputWriter.append(completeText); - this.outputManager.addChunk(completeText); - }; + 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}"`); + } - for await (const chunk of outputStream) { - await appendPlainText(sanitizer.write(chunk)); + 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); } - 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; + // This listener is registered before PtyTerminal's close listener so a + // process-driven terminal close is not mistaken for a user action. + this.disposables.push( + ptyProcess.onExit(() => { + this.ptyExited = true; + }), + ); + + this.disposables.push( + 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); + }), + ); } - /** - * Kills the terminal job. - */ - kill(): void { - this.stopRequested = true; - this.terminal.dispose(); + private createPtyTerminalView(): void { + if (!this.ptyProcess || this.terminal) return; + const ptyTerminal = new PtyTerminal(this.ptyProcess, () => { + this.detachPtyTerminalView(ptyTerminal); + }); + this.ptyTerminal = ptyTerminal; + 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; + } } - /** - * Dispose of the terminal and clean up resources - */ - dispose(): void { - if (this.disposed) { + private detachPtyTerminalView(ptyTerminal?: PtyTerminal): void { + if ( + this.finished || + this.ptyExited || + (ptyTerminal && ptyTerminal !== this.ptyTerminal) + ) { return; } - this.disposed = true; + const attachedPtyTerminal = ptyTerminal ?? this.ptyTerminal; + this.terminal = undefined; + this.ptyTerminal = undefined; + attachedPtyTerminal?.dispose(); + this.setVisible(false); + } - TerminalJob.jobs.delete(this.id); - TerminalJob.onDidDisposeEmitter.fire(this); + private setVisible(isVisible: boolean): void { + if (this.terminalVisibility.value === isVisible) return; + this.terminalVisibility.value = isVisible; + TerminalJob.onDidChangeVisibilityEmitter.fire(this); + } + + 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, + }); + } - this.closeListener?.dispose(); - this.closeListener = undefined; + private initializeLifecycle(): void { + this.disposables.push( + vscode.window.onDidCloseTerminal((terminal) => { + if (terminal !== this.terminal || this.finished) return; + if (this.ptyProcess) { + if (!this.ptyExited) this.detachPtyTerminalView(); + return; + } - this.cleanupExecution(); + this.stopRequested = true; + this.terminalCloseError = ExecutionError.create( + "Background job finished as user closed terminal.", + ); + for (const reject of this.terminalCloseRejectors) { + reject(this.terminalCloseError); + } + this.terminalCloseRejectors.clear(); + }), + ); - logger.debug(`Disposed terminal job "${this.config.name}"`); + 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), + }); + } } - /** - * Wait for shell integration to become available - */ - private async waitForShellIntegration( - timeoutMs = 15000, - ): Promise { - if (this.terminal.shellIntegration) { - this.shellIntegration = this.terminal.shellIntegration; - return this.shellIntegration; + 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); + } - return new Promise((resolve, reject) => { - // Set up timeout + private async processShellOutput( + output: AsyncIterable, + ): Promise { + for await (const chunk of output) { + this.enqueueRawOutput(chunk); + } + } + + private waitForShellIntegration( + timeoutMs = 15_000, + ): Promise { + if (this.terminal?.shellIntegration) { + return Promise.resolve(this.terminal.shellIntegration); + } + 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 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; + 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"); + }) + .finally(() => { + this.pendingOutputCharacters = Math.max( + 0, + this.pendingOutputCharacters - text.length, + ); + this.updatePtyOutputFlowControl(); + }); } - private async finish(error?: ExecutionError): Promise { + 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( + 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(); + this.terminal?.dispose(); + this.terminal = undefined; + this.ptyTerminal?.dispose(); + this.ptyTerminal = undefined; + this.setVisible(false); + + let executionError = initialError ?? this.persistenceError; + if ( + exitCode !== undefined && + exitCode !== 0 && + !this.stopRequested && + !executionError + ) { + 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 +598,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.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(); + this.ptyTerminal?.dispose(); + if (this.outputWriter) { + void this.outputQueue + .finally(() => this.outputWriter.close()) + .catch(() => {}); } + this.ptyProcess?.kill("SIGKILL"); } } diff --git a/packages/vscode/src/integrations/terminal/terminal-state.ts b/packages/vscode/src/integrations/terminal/terminal-state.ts index 41d04f7332..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,23 +56,48 @@ 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(); } 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 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.isFinished ? job : undefined; } /** @@ -87,7 +113,11 @@ 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), + ); this.disposables.push( TerminalJob.onDidFinish((event) => { void this.taskDataStore.addBackgroundJobNotification( @@ -110,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) { @@ -211,26 +244,49 @@ 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[] { - 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..bc0813fe2c 100644 --- a/packages/vscode/src/integrations/webview/vscode-host-impl.ts +++ b/packages/vscode/src/integrations/webview/vscode-host-impl.ts @@ -466,6 +466,21 @@ export class VSCodeHostImpl implements VSCodeHostApi, vscode.Disposable { }; }; + 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( this.taskStateStore.getBackgroundJobNotificationsSignal(taskId), 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 }; }