Fix ACP terminal shell command spawning (#793)

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(

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<string, unknown> {
   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;