fix(acp): wrap shell command in bash -c for terminal/create

[kevintruong]

Summary

When kimi acp is connected to a client that advertises terminal capability, the Terminal ACP wrapper at src/kimi_cli/acp/tools.py replaces the local Shell tool and forwards every shell-tool call to the client over terminal/create. It currently does this:

# kimi_cli/acp/tools.py — current
resp = await self._acp_conn.create_terminal(
    command=params.command,                    # e.g. "git rev-parse --short HEAD"
    session_id=self._acp_session_id,
    output_byte_limit=builder.max_chars,
)                                               # `args=` not passed

The ACP CreateTerminalRequest spec defines command: string as a program path and args: string[] as a separate argv vector. Conformant clients run the request as the equivalent of Command::new(req.command).args(req.args). So when this wrapper packs the LLM's full shell line into command and omits args, the client tries to spawn a binary literally named <full shell line> and fails with No such file or directory.

This affects every multi-token, quoted, piped, or redirected command — i.e. essentially every real-world shell invocation. Single-token commands like pwd happen to work by coincidence because command="pwd" resolves to a real binary path; everything else fails on the first attempt.

Root cause: asymmetry with the local Shell tool

The local Shell tool at src/kimi_cli/tools/shell/__init__.py already does the right thing:

def _shell_args(self, command: str) -> tuple[str, ...]:
    if self._is_powershell:
        return (str(self._shell_path), "-command", command)
    return (str(self._shell_path), "-c", command)

# called as:
process = await kaos.exec(*self._shell_args(command), ...)

So locally, kimi exec()s bash -c <full shell line> (or pwsh -command …), and the user's host shell parses pipes, quotes, redirects, &&, ;, etc. The ACP wrapper diverged from this path — same LLM input, same Shell tool name and schema, but a different (broken) execution model.

Fix

Have the ACP Terminal reuse the local Shell's argv-build logic, then split program/argv into the spec-compliant fields:

# kimi_cli/acp/tools.py — fixed
argv = self._shell_tool.shell_argv(params.command)
resp = await self._acp_conn.create_terminal(
    command=argv[0],
    args=list(argv[1:]),
    session_id=self._acp_session_id,
    output_byte_limit=builder.max_chars,
)

Now the client sees Command::new("/bin/bash").args(["-c", "<shell-line>"]) — semantically identical to the local Shell tool. Pipes, quotes, redirects, &&, ; all work the same way they do under kimi interactive.

To make the seam explicit and avoid reaching across private attributes, the existing _shell_args is renamed to public shell_argv (with a back-compat wrapper method retained for one cycle). The wrapper preserves dynamic dispatch — a subclass overriding shell_argv will be honored via the legacy _shell_args call too.

Empirical reproduction

Tested against an external ACP client that follows the spec strictly (Command::new(req.command).args(req.args)):

Shell command	Before	After
pwd	✅ works (single token = valid binary)	✅ works
echo hello	❌ "No such file or directory"	✅ works
git rev-parse --short HEAD	❌ same	✅ works
python3 -c "print(7*8)"	❌ same	✅ works
cat README.md | head -5	❌ same (and pipes!)	✅ works

Pre-fix: ~3 retries per shell call, all fail, kimi reports "Internal error". Post-fix: same call shape that already worked under kimi interactive now works identically over ACP.

Reproduction recipe (for maintainers)

A minimal Python ACP client that demonstrates the bug pre-fix and confirms the fix post-fix:

# probe_kimi_acp.py — reproduction harness, no extra deps beyond stdlib
import json, subprocess, threading, uuid
proc = subprocess.Popen(["kimi", "-y", "--afk", "acp"],
    stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL,
    text=True, bufsize=1)
def send(o): proc.stdin.write(json.dumps(o)+"\n"); proc.stdin.flush()
def reader():
    for line in proc.stdout:
        m = json.loads(line)
        if m.get("method") == "terminal/create":
            print("PAYLOAD:", json.dumps(m["params"]))
            send({"jsonrpc":"2.0","id":m["id"],
                "error":{"code":-32601,"message":"unimplemented (probe)"}})
        elif m.get("method") == "session/request_permission":
            send({"jsonrpc":"2.0","id":m["id"],
                "result":{"outcome":{"outcome":"selected","optionId":"approve_for_session"}}})
threading.Thread(target=reader, daemon=True).start()
send({"jsonrpc":"2.0","id":0,"method":"initialize",
    "params":{"protocolVersion":1,"clientInfo":{"name":"probe","version":"0"},
        "clientCapabilities":{"terminal":True,"fs":{"readTextFile":True,"writeTextFile":True}}}})
sid = str(uuid.uuid4())
send({"jsonrpc":"2.0","id":1,"method":"session/new",
    "params":{"sessionId":sid,"cwd":".","mcpServers":[]}})
send({"jsonrpc":"2.0","id":2,"method":"session/prompt",
    "params":{"sessionId":sid,"prompt":[{"type":"text",
        "text":'Use the shell tool to run "git rev-parse --short HEAD".'}]}})
import time; time.sleep(60)

Pre-fix output: PAYLOAD: {"command": "git rev-parse --short HEAD", ...} — bare shell line, no args.
Post-fix output: PAYLOAD: {"command": "/bin/bash", "args": ["-c", "git rev-parse --short HEAD"], ...} — proper program/argv split.

Test plan

• pytest tests/tools/test_shell_bash.py — 20 passed (no regression in local Shell behavior)
• pytest tests/tools/test_shell_powershell.py — local Shell PowerShell branch unchanged
• pytest tests/acp/ — 21 existing + 7 new passed
• pytest tests/acp/test_terminal_wrapper.py — 7 new tests:
  • 5 parametrized POSIX cases (single-token, two-token, multi-token-with-args, quoted, piped) verify the wrap is applied uniformly
  • test_shell_argv_helper_matches_local_run asserts ACP and local paths build identical argv (so they can't drift silently)
  • test_terminal_wraps_powershell_branch_argv covers the -command branch end-to-end via Terminal.__call__ (monkey-patches _is_powershell so it runs on every platform without needing a real pwsh)
• Manual end-to-end probe against an external spec-compliant ACP client (recipe above): every command shape that previously failed now executes and returns expected stdout

Risks / out of scope

• Behavior change for clients with workarounds: any client implementing a sh -c req.command workaround will now see proper argv split. Net effect is strictly safer: the workaround was a security hazard (turns ACP's per-program permission model into shell-string semantics). Note that double-wrapping is benign — Command::new("/bin/bash").args(["-c", "<line>"]) invoked under a sh -c workaround becomes sh -c "/bin/bash -c '<line>'", which still evaluates correctly because bash -c is itself idempotent under reentry. No quoting hazards introduced.
• PowerShell branch: handled by the same shell_argv (returns (<pwsh>, "-command", cmd)); now covered end-to-end through Terminal.__call__ by test_terminal_wraps_powershell_branch_argv.
• Backwards compat: _shell_args retained as a wrapper method (not a static alias) so dynamic dispatch is preserved if a subclass overrides shell_argv. Can be removed in a future cycle once any internal callers migrate.
• Working-directory and env: unchanged — the existing create_terminal call already accepts cwd and env kwargs from the ACP schema; this PR doesn't touch those code paths.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e4c64eb06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove module-wide Windows skip from terminal wrapper tests

The module-level pytestmark = pytest.mark.skipif(platform.system() == "Windows", ...) skips all tests in this file on Windows, including test_terminal_wraps_powershell_branch_argv; that means the new ACP PowerShell wrapping path is never exercised in Windows CI despite the test comments claiming cross-platform coverage. A regression in the Windows -command branch would therefore ship undetected.

Useful? React with 👍 / 👎.

[kevintruong]

@claude review