Jason Cat f623e64e07
fix(acp): resolve a bare agent command through PATH before spawning (#6598)
* fix(acp): resolve a bare agent command through PATH before spawning

`invoke_acp_agent` handed `acp_agents.<name>.command` straight to the ACP
SDK, which spawns it with `asyncio.create_subprocess_exec`. That API applies
no `PATHEXT` lookup on Windows, so a bare npm launcher (`npx`, `mcode`)
raised `FileNotFoundError` although its `.cmd` shim was on `PATH`. The
documented `command: npx` and `command: mcode` shapes could not start, and
the tool reported "was not found on PATH" for an installed launcher.

Resolve the configured command with `shutil.which` before spawning — the way
the MCP Python SDK normalizes stdio server commands — and keep the configured
value when nothing matches, so the existing install guidance still fires.

Fixes #6597

* fix(acp): resolve the ACP agent command off the event loop

`_resolve_agent_command` calls `shutil.which`, which stats the filesystem.
Calling it inline in the async `_invoke` tripped the strict BlockBuster gate
(`BlockingError: Blocking call to os.access`) for every command that resolves,
and would block the Gateway event loop in production.

Offload the lookup with `asyncio.to_thread`, matching the neighbouring
`_get_work_dir`, `_build_acp_mcp_servers`, and `_format_invocation_error`
calls.

The blocking-io anchor configured `command="test-agent"`, a name that resolves
nowhere, so `shutil.which` short-circuited before `os.access` and the gate
never covered the new call. Point it at `sys.executable` so the lookup is
really exercised, and cover the unresolvable-guidance branch end-to-end with a
new test.

Refs #6597

* fix(acp): keep the configured command in ACP remediation text

Review follow-ups on the PATH resolution:

- Only bare names are resolved. A configured path is interpreted by the spawn
  against the per-thread ACP workspace, so resolving it against the Gateway's
  working directory could pick a different file or hand the spawn a path that
  then fails.
- `_format_invocation_error` receives `agent_config.command` again. A command
  can resolve on PATH and still fail to launch (a broken shebang, for example),
  and reporting the resolved path made "was not found on PATH" false and
  dropped the `codex-acp` hint.
- The blocking-io anchor uses a bare name backed by a shim on a controlled PATH
  instead of `sys.executable`, which the bare-name scoping no longer resolves.

Tests: the pass-through assertion in
`test_invoke_acp_agent_uses_fixed_acp_workspace` pins PATH, and new coverage
covers the bare-name scoping and the spawn-failure message.

Refs #6597

* docs(acp): correct the path-resolution rationale

A configured path is resolved by the spawn in its own process, so the
pass-through reason must not claim it is resolved against the ACP workspace:
that only holds on POSIX, while Windows resolves a relative executable against
the calling process's directory. State the platform-neutral reason instead.

Refs #6597

* fix(acp): resolve the launcher against the agent's own PATH

The ACP SDK merges acp_agents.<name>.env over its inherited environment at spawn time, so a configured PATH, not the Gateway's, decides which launcher the agent would find. Resolve a bare command against that effective PATH, and use the Gateway's only when the agent does not override it.

Return a successful lookup as an absolute path: a relative PATH entry makes shutil.which yield a result relative to the Gateway's cwd, while the spawn runs with cwd set to the ACP workspace, where that path resolves elsewhere or not at all.

Regressions: a real-subprocess test with same-named launchers in the Gateway and agent PATHs (asserting which one ran), and a real-subprocess test whose PATH holds a relative directory while the ACP workspace is elsewhere. Both are red on the pre-fix code.
2026-10-10 19:23:39 +08:00
..