Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/acp/terminal-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
38 changes: 38 additions & 0 deletions src/spawn-command-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <command>` on Unix or `cmd.exe /C
* <command>` 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] };
}
33 changes: 33 additions & 0 deletions test/spawn-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down Expand Up @@ -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";
Expand Down
80 changes: 80 additions & 0 deletions test/terminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
});