diff --git a/src/acp/terminal-manager.ts b/src/acp/terminal-manager.ts index e22bf9ba..f19dff8e 100644 --- a/src/acp/terminal-manager.ts +++ b/src/acp/terminal-manager.ts @@ -15,7 +15,7 @@ import type { } from "@agentclientprotocol/sdk"; import { PermissionDeniedError, PermissionPromptUnavailableError } from "../errors.js"; import { promptForPermission } from "../permission-prompt.js"; -import { buildSpawnCommandOptions } from "../spawn-command-options.js"; +import { buildShellExec, buildSpawnCommandOptions } from "../spawn-command-options.js"; import type { ClientOperation, NonInteractivePermissionPolicy, PermissionMode } from "../types.js"; const DEFAULT_TERMINAL_OUTPUT_LIMIT_BYTES = 64 * 1024; @@ -190,10 +190,11 @@ export class TerminalManager { 0, Math.round(params.outputByteLimit ?? DEFAULT_TERMINAL_OUTPUT_LIMIT_BYTES), ); + const exec = buildShellExec(params.command, params.args); const proc = spawn( - params.command, - params.args ?? [], - buildTerminalSpawnOptions(params.command, params.cwd ?? this.cwd, params.env), + exec.command, + exec.args, + buildTerminalSpawnOptions(exec.command, params.cwd ?? this.cwd, params.env), ); await waitForSpawn(proc); diff --git a/src/spawn-command-options.ts b/src/spawn-command-options.ts index 27bb516f..0e354529 100644 --- a/src/spawn-command-options.ts +++ b/src/spawn-command-options.ts @@ -74,3 +74,41 @@ export function buildSpawnCommandOptions( shell: true, }; } + +export type ShellExec = { + command: string; + args: string[]; +}; + +/** + * Resolve a `terminal/create` request into an actual `spawn()` argv pair. + * + * The ACP `CreateTerminalRequest` schema models `command` as the executable + * and `args` as its argument vector, mirroring `child_process.spawn(cmd, args)`. + * In practice, several agents (Claude Code, Codex-style wrappers, and others) + * place a full shell command line in the `command` field and leave `args` + * empty. Other ACP clients — notably Zed via its `ShellBuilder` — handle this + * by always running the request through the system shell, so those agents work + * out of the box. Spawning the unsplit string directly fails with `ENOENT` + * because the shell line is treated as an executable name. + * + * To match the de facto behavior of other ACP clients, when `args` is empty we + * route the command through `/bin/sh -c ` on Unix or `cmd.exe /C + * ` on Windows. When `args` is non-empty we honor the literal ACP + * contract and spawn `command` with the provided argv unchanged. This keeps + * well-behaved agents on the original direct-spawn fast path while letting + * "raw shell line" agents work transparently. + */ +export function buildShellExec( + command: string, + args: string[] | undefined, + platform: NodeJS.Platform = process.platform, +): ShellExec { + if (args && args.length > 0) { + return { command, args }; + } + if (platform === "win32") { + return { command: "cmd.exe", args: ["/d", "/s", "/c", command] }; + } + return { command: "/bin/sh", args: ["-c", command] }; +} diff --git a/test/spawn-options.test.ts b/test/spawn-options.test.ts index 5da9ad57..239efc4a 100644 --- a/test/spawn-options.test.ts +++ b/test/spawn-options.test.ts @@ -8,6 +8,7 @@ import { resolveAgentSessionCwd } from "../src/acp/client-process.js"; import { buildAgentSpawnOptions, buildSpawnCommandOptions } from "../src/acp/client.js"; import { buildTerminalSpawnOptions } from "../src/acp/terminal-manager.js"; import { buildQueueOwnerSpawnOptions } from "../src/cli/session/queue-owner-process.js"; +import { buildShellExec } from "../src/spawn-command-options.js"; test("buildAgentSpawnOptions hides Windows console windows and preserves auth env", () => { const options = buildAgentSpawnOptions("/tmp/acpx-agent", { @@ -130,6 +131,38 @@ test("buildSpawnCommandOptions keeps shell disabled for non-batch commands", asy } }); +test("buildShellExec passes through command and args verbatim when args are non-empty", () => { + const linuxExec = buildShellExec("node", ["-e", "console.log('ok')"], "linux"); + assert.deepEqual(linuxExec, { command: "node", args: ["-e", "console.log('ok')"] }); + + const windowsExec = buildShellExec("npx", ["create-foo"], "win32"); + assert.deepEqual(windowsExec, { command: "npx", args: ["create-foo"] }); +}); + +test("buildShellExec wraps command in /bin/sh -c on Unix when args are missing or empty", () => { + const fromUndefined = buildShellExec("ls -la /Users/foo/", undefined, "linux"); + assert.deepEqual(fromUndefined, { command: "/bin/sh", args: ["-c", "ls -la /Users/foo/"] }); + + const fromEmpty = buildShellExec("echo hello | tr a-z A-Z", [], "darwin"); + assert.deepEqual(fromEmpty, { + command: "/bin/sh", + args: ["-c", "echo hello | tr a-z A-Z"], + }); +}); + +test("buildShellExec wraps command in cmd.exe /d /s /c on Windows when args are empty", () => { + const exec = buildShellExec("dir C:\\Users", undefined, "win32"); + assert.deepEqual(exec, { command: "cmd.exe", args: ["/d", "/s", "/c", "dir C:\\Users"] }); +}); + +test("buildShellExec preserves bare commands without args by routing through the shell", () => { + // A bare token like "ls" still goes through the shell so PATH lookup, + // aliases, and shell builtins behave consistently regardless of whether + // an agent splits args or not. + const exec = buildShellExec("ls", undefined, "linux"); + assert.deepEqual(exec, { command: "/bin/sh", args: ["-c", "ls"] }); +}); + test("resolveAgentSessionCwd translates WSL cwd for Windows exe agents", async () => { let capturedCwd: string | undefined; const inputCwd = "/home/user/project"; diff --git a/test/terminal.test.ts b/test/terminal.test.ts index c84d5727..7d771497 100644 --- a/test/terminal.test.ts +++ b/test/terminal.test.ts @@ -133,3 +133,83 @@ test("terminal manager fails when prompt is unavailable and policy is fail", asy await fs.rm(tmp, { recursive: true, force: true }); } }); + +test("terminal manager runs an unsplit shell command line passed in the command field", async (t) => { + if (process.platform === "win32") { + t.skip("POSIX-shell behavior; Windows path covered elsewhere"); + return; + } + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-terminal-test-")); + try { + const manager = new TerminalManager({ + cwd: tmp, + permissionMode: "approve-all", + }); + + // The whole shell line lives in `command`; `args` is omitted on purpose, + // mirroring the request shape used by Claude Code, codebuddy, and other + // agents that don't pre-split their Bash tool input. + const created = await manager.createTerminal({ + sessionId: "session-1", + command: "echo hello-from-shell", + }); + + const waitResult = await manager.waitForTerminalExit({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + assert.equal(waitResult.exitCode, 0); + + const outputResult = await manager.terminalOutput({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + assert.match(outputResult.output, /hello-from-shell/); + + await manager.releaseTerminal({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + } finally { + await fs.rm(tmp, { recursive: true, force: true }); + } +}); + +test("terminal manager honors shell metacharacters in an unsplit command", async (t) => { + if (process.platform === "win32") { + t.skip("POSIX-shell behavior; Windows path covered elsewhere"); + return; + } + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "acpx-terminal-test-")); + try { + const manager = new TerminalManager({ + cwd: tmp, + permissionMode: "approve-all", + }); + + const created = await manager.createTerminal({ + sessionId: "session-1", + command: "printf 'one\\ntwo\\nthree\\n' | wc -l", + }); + + const waitResult = await manager.waitForTerminalExit({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + assert.equal(waitResult.exitCode, 0); + + const outputResult = await manager.terminalOutput({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + // wc -l prints the count; tolerate leading whitespace from BSD/GNU wc. + assert.match(outputResult.output, /^\s*3\b/); + + await manager.releaseTerminal({ + sessionId: "session-1", + terminalId: created.terminalId, + }); + } finally { + await fs.rm(tmp, { recursive: true, force: true }); + } +});