diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fa347fbb9..f6b78a91d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,9 @@ ### Fixed - Improved reliability of edits when file snapshots share identical 16-bit hash tags +### Fixed + +- Fixed ACP `terminal/create` sending the bash tool's full shell line in `command` with no `args`, which broke spec-conformant clients that spawn `command`+`args` directly (no implicit shell) — any command containing a space, pipe, `&&`, redirect, or `$(...)` failed with `ENOENT` and the agent silently degraded to read-only tools. The bash tool now wraps the shell line before calling `clientBridge.createTerminal`, reusing the same shell binary + args the local `bash-executor` resolves via `settings.getShellConfig()` (Git Bash / `bash.exe` on Windows, `$SHELL` with `sh` fallback on POSIX) so bash semantics — `$VAR`, `$(...)`, `source`, POSIX quoting, `-l` — are preserved on both platforms. ([#4333](https://github.com/can1357/oh-my-pi/issues/4333)) ## [16.3.2] - 2026-07-02 diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index df3d14f12..291206920 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -46,6 +46,33 @@ export const BASH_DEFAULT_PREVIEW_LINES = 10; const BASH_ENV_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/; const DEFAULT_AUTO_BACKGROUND_THRESHOLD_MS = 60_000; +/** + * Shape a shell command line for an ACP-conformant `terminal/create` request. + * + * ACP's `command` field is documented as the executable and `args` as its + * argv tail (see https://agentclientprotocol.com/protocol/v1/terminals), so a + * spec-conformant client `spawn(command, args)`s them directly — no implicit + * shell. A raw `bash` tool line ("git status && echo x | head") therefore has + * to be wrapped in an explicit shell invocation, otherwise the client tries + * to spawn the whole line as argv[0] and fails with `ENOENT` for anything + * containing a space, pipe, `&&`, redirect, or `$(...)`. + * + * The wrap reuses the same shell binary + args the local `bash-executor` would + * pick via `settings.getShellConfig()` — Git Bash / `bash.exe` on Windows, + * `$SHELL` (bash/zsh) with the `sh` fallback on POSIX — so the ACP path + * preserves `bash` tool semantics (`$VAR`, `$(...)`, `source`, POSIX quoting, + * `-l`) instead of dropping to `cmd.exe` on Windows. The agent host's shell + * path is used as a proxy for the client's, matching the near-universal + * ACP deployment shape of an editor spawning omp as a co-hosted subprocess. + */ +export function wrapShellLineForClientTerminal( + line: string, + shellConfig: { shell: string; args: string[]; prefix?: string | undefined }, +): { command: string; args: string[] } { + const finalLine = shellConfig.prefix ? `${shellConfig.prefix} ${line}` : line; + return { command: shellConfig.shell, args: [...shellConfig.args, finalLine] }; +} + /** * Bash patterns flagged as safety critical for approval policy. * @@ -842,8 +869,10 @@ export class BashTool implements AgentTool ({ name, value: value as string })) diff --git a/packages/coding-agent/test/bash-acp-terminal.test.ts b/packages/coding-agent/test/bash-acp-terminal.test.ts index 50fd110ac..ff6490bf7 100644 --- a/packages/coding-agent/test/bash-acp-terminal.test.ts +++ b/packages/coding-agent/test/bash-acp-terminal.test.ts @@ -24,6 +24,13 @@ function makeSession(bridge: ClientBridge): ToolSession { getBashInterceptorRules() { return []; }, + getShellConfig() { + // Fixed bash shell keeps the wrap assertions cross-platform: the fix + // must reuse the resolved shell (Git Bash on Windows, `$SHELL` on + // POSIX) instead of collapsing to `cmd.exe` — that's the contract + // this test defends. + return { shell: "/bin/bash", args: ["-l", "-c"], env: {}, prefix: undefined }; + }, }, getClientBridge: () => bridge, } as unknown as ToolSession; @@ -60,10 +67,14 @@ describe("BashTool ACP terminal routing", () => { updates.push(update as { details?: { terminalId?: string } }); }); - // createTerminal must be called with the expanded command + // createTerminal must send the resolved bash shell + `-l -c ` so + // spec-conformant ACP clients that spawn `command` directly (no implicit + // shell) still get bash semantics — never `cmd.exe`, which would break + // `$VAR`, `$(...)`, `source`, and POSIX quoting on Windows. expect(createSpy).toHaveBeenCalledTimes(1); const params = createSpy.mock.calls[0]![0]; - expect(params.command).toBe("echo hi"); + expect(params.command).toBe("/bin/bash"); + expect(params.args).toEqual(["-l", "-c", "echo hi"]); // The first onUpdate must carry the terminalId so the editor can embed it expect(updates.length).toBeGreaterThanOrEqual(1); @@ -80,6 +91,36 @@ describe("BashTool ACP terminal routing", () => { expect(releaseSpy).toHaveBeenCalledTimes(1); }); + it("wraps shell metacharacters into args instead of packing them into command", async () => { + // Regression for #4333: a bash line with `&&`, pipes, or spaces must not + // be sent as raw `command` (spec-conformant ACP clients spawn command+args + // directly and would ENOENT the whole line as argv[0]). + const handle: ClientBridgeTerminalHandle = { + terminalId: "term-shell-wrap", + waitForExit: async () => ({ exitCode: 0, signal: null }), + currentOutput: async () => ({ output: "", truncated: false }), + kill: async () => {}, + release: async () => {}, + }; + const bridge: ClientBridge = { + capabilities: { terminal: true }, + createTerminal: async () => handle, + }; + const createSpy = spyOn(bridge, "createTerminal"); + + const line = "git status && echo x | head"; + const tool = new BashTool(makeSession(bridge)); + await tool.execute("call-shell-wrap", { command: line }); + + expect(createSpy).toHaveBeenCalledTimes(1); + const params = createSpy.mock.calls[0]![0]; + expect(params.command).toBe("/bin/bash"); + expect(params.args).toEqual(["-l", "-c", line]); + // `args` must actually be present — the bug was omitting it entirely. + expect(params.args).toBeDefined(); + expect(params.args?.length).toBeGreaterThan(0); + }); + it("releases the client terminal when final output retrieval fails", async () => { const handle: ClientBridgeTerminalHandle = { terminalId: "term-output-failure",