diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index 9e1c7c1a0..0a17d73d4 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, test, vi } from "vitest"; +import { type ChildProcess } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { afterEach, describe, expect, test, vi } from "vitest"; import type { PermissionOption, PromptResponse, @@ -23,6 +25,7 @@ import { transformPiModels } from "./pi-direct-agent.js"; import type { AgentStreamEvent } from "../agent-sdk-types.js"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import { asInternals } from "../../test-utils/class-mocks.js"; +import * as spawnUtils from "../../../utils/spawn.js"; interface ACPSessionInternals { sessionId: string | null; @@ -114,6 +117,14 @@ function createSessionWithConfig( ); } +function createTerminalChildStub(): ChildProcess { + const child = new EventEmitter() as ChildProcess; + child.stdout = new EventEmitter() as ChildProcess["stdout"]; + child.stderr = new EventEmitter() as ChildProcess["stderr"]; + child.kill = vi.fn(() => true) as ChildProcess["kill"]; + return child; +} + function selectConfigOption( category: "mode" | "model" | "thought_level", values: string[], @@ -303,6 +314,78 @@ describe("createLoggedNdJsonStream", () => { }); }); +describe("ACPAgentSession terminal tools", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test("runs single-string terminal commands through the platform shell", async () => { + const child = createTerminalChildStub(); + const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child); + const session = createSession(); + const shell = spawnUtils.platformShell(); + + await session.createTerminal({ + sessionId: "session-1", + command: "git -C /repo status --short", + cwd: "/repo", + }); + + expect(spawn).toHaveBeenCalledWith( + shell.command, + [...shell.flag, "git -C /repo status --short"], + expect.objectContaining({ cwd: "/repo" }), + ); + }); + + test("preserves explicit terminal argv", async () => { + const child = createTerminalChildStub(); + const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child); + const session = createSession(); + + await session.createTerminal({ + sessionId: "session-1", + command: "git", + args: ["status", "--short"], + cwd: "/repo", + }); + + expect(spawn).toHaveBeenCalledWith( + "git", + ["status", "--short"], + expect.objectContaining({ cwd: "/repo" }), + ); + }); + + test("surfaces spawn errors through terminal output and waitForTerminalExit", async () => { + const child = createTerminalChildStub(); + vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child); + const session = createSession(); + + const terminal = await session.createTerminal({ + sessionId: "session-1", + command: "missing-command", + }); + child.emit("error", new Error("spawn missing-command ENOENT")); + + await expect( + session.waitForTerminalExit({ + sessionId: "session-1", + terminalId: terminal.terminalId, + }), + ).rejects.toThrow("spawn missing-command ENOENT"); + await expect( + session.terminalOutput({ + sessionId: "session-1", + terminalId: terminal.terminalId, + }), + ).resolves.toMatchObject({ + output: "spawn missing-command ENOENT\n", + truncated: false, + }); + }); +}); + describe("mapACPUsage", () => { test("maps ACP usage fields into Paseo usage", () => { expect( diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 236577955..241e35c00 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -92,7 +92,7 @@ import { import { renderPromptAttachmentAsText } from "../prompt-attachments.js"; import { appendOrReplaceGrowingAssistantMessage, runProviderTurn } from "./provider-runner.js"; import { findExecutable } from "../../../utils/executable.js"; -import { spawnProcess } from "../../../utils/spawn.js"; +import { platformShell, spawnProcess } from "../../../utils/spawn.js"; function assertChildWithPipes( child: ChildProcess, @@ -106,6 +106,22 @@ function isRecord(value: unknown): value is Record { return value != null && typeof value === "object" && !Array.isArray(value); } +function resolveTerminalCommand( + command: string, + args?: string[], +): { command: string; args: string[] } { + if (args && args.length > 0) { + return { command, args }; + } + + if (!/\s/.test(command.trim())) { + return { command, args: [] }; + } + + const shell = platformShell(); + return { command: shell.command, args: [...shell.flag, command] }; +} + const DEFAULT_ACP_CAPABILITIES: AgentCapabilityFlags = { supportsStreaming: true, supportsSessionPersistence: true, @@ -1464,7 +1480,8 @@ export class ACPAgentSession implements AgentSession, ACPClient { const env = Object.fromEntries( (params.env ?? []).map((entry: EnvVariable) => [entry.name, entry.value]), ); - const child = spawnProcess(params.command, params.args ?? [], { + const terminalCommand = resolveTerminalCommand(params.command, params.args); + const child = spawnProcess(terminalCommand.command, terminalCommand.args, { cwd: params.cwd ?? this.config.cwd, ...createProviderEnvSpec({ runtimeSettings: this.runtimeSettings, @@ -1479,6 +1496,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { resolveExit = resolve; rejectExit = reject; }); + waitForExit.catch(() => undefined); const entry: TerminalEntry = { id: terminalId, @@ -1498,9 +1516,11 @@ export class ACPAgentSession implements AgentSession, ACPClient { child.stderr!.on("data", (chunk: Buffer | string) => appendTerminalOutput(entry, chunk.toString()), ); - child.once("error", (error) => - rejectExit(error instanceof Error ? error : new Error(String(error))), - ); + child.once("error", (error) => { + const spawnError = error instanceof Error ? error : new Error(String(error)); + appendTerminalOutput(entry, `${spawnError.message}\n`); + rejectExit(spawnError); + }); child.once("exit", (code, signal) => { const exit = { exitCode: code, signal }; entry.exit = exit;