fix(acp): resolve an absolute Windows agent command before spawning (#6656)

* fix(acp): resolve an absolute Windows agent command before spawning

A command configured as an absolute path without an extension -- npm's global
shim layout, e.g. C:\npm\mcode beside mcode.cmd -- was handed to the ACP SDK
unchanged and always failed: create_subprocess_exec applies no PATHEXT, so it
ran npm's extensionless shell script and died with WinError 193 (or raised
FileNotFoundError when only the .cmd exists). The separator guard from #6598
was too broad: only relative paths need to stay with the spawn, because their
meaning is owned by the ACP-workspace cwd, while an absolute path does not
depend on the cwd.

Absolute paths now go through the same shutil.which lookup as bare names: the
lookup searches the command's own directory and probes PATHEXT, and the
configured value is kept when nothing matches so the not-found remediation
still fires. Relative paths (bin/agent, ./bin/agent) still go to the spawn
unchanged.

Docs: README.md and backend/packages/harness/deerflow/tools/AGENTS.md now
describe the bare/absolute vs relative split.

AI disclosure: this change was drafted with AI assistance at my request, under
my direction and review.

* fix(acp): keep already-absolute lookup results unchanged

Review feedback on #6656 (willem-bd): os.path.abspath collapsed a `..` that
follows a symlink, so an absolute POSIX command like
`/srv/current/../shared/agent` (with `current -> releases/v1`) was rewritten to
`/srv/shared/agent` -- a path that either does not exist or names a different
launcher. The anchoring to the Gateway cwd exists only for relative PATH
lookup results, where shutil.which cannot know the cwd the spawn will use; an
already-absolute result needs no rewrite.

Regressions: a platform-independent stub-lookup test pinning the literal
parent-segment path, and a POSIX-only test that builds the symlink layout and
asserts the configured command is returned unchanged.

README.md

@@ -372,7 +372,7 @@ streams simulate chunks from a non-streaming response, while no-tool streams sta
    - `CLAUDE_CODE_OAUTH_TOKEN_FILE_DESCRIPTOR` accepts a UTF-8 token handoff and reuses it for later model instances in the same process. Undecodable handoffs are skipped so Claude Code can still try its override or default credentials file.
    - CLI credential JSON files accept UTF-8 with or without a BOM, independently of the host locale. `make doctor` accepts the same files when checking CLI authentication. Invalid text encoding is treated as an unreadable source; Claude Code can still try its default file after an invalid override.
    - ACP agent entries are separate from model providers — if you configure `acp_agents.codex`, point it at a Codex ACP adapter such as `npx -y @zed-industries/codex-acp`
-   - A bare ACP agent `command` is resolved before the agent starts, so npm-installed launchers work on Windows too: `npx` or `mcode` is spawned as its `npx.cmd`/`mcode.cmd` shim instead of failing with a command-not-found error. The lookup uses the `PATH` the agent subprocess will actually see (`acp_agents.<name>.env.PATH` when set, otherwise the Gateway's) and hands the spawn an absolute path, so a relative `PATH` entry cannot be re-interpreted inside the agent's own workspace. A `command` containing a path separator is used as configured, and a bare name that cannot be resolved is still reported with the install guidance below.
+   - A bare or absolute ACP agent `command` is resolved before the agent starts, so npm-installed launchers work on Windows too: `npx` or `mcode` is spawned as its `npx.cmd`/`mcode.cmd` shim instead of failing with a command-not-found error, and an absolute path such as `C:\Users\me\AppData\Roaming\npm\mcode` resolves to the `mcode.cmd` shim beside npm's extensionless shell script instead of failing with `WinError 193`. The lookup uses the `PATH` the agent subprocess will actually see (`acp_agents.<name>.env.PATH` when set, otherwise the Gateway's) and hands the spawn an absolute path, so a relative `PATH` entry cannot be re-interpreted inside the agent's own workspace. A relative `command` (for example `bin/mcode`) is used as configured, and a command that cannot be resolved is still reported with the install guidance below.
    - Each ACP agent's `timeout_seconds` (default: 1800) is one shared budget for initialization, session creation, and the prompt, starting after the subprocess launches. On timeout, DeerFlow aborts the invocation and closes the subprocess before returning an error. Workspace/MCP preparation and subprocess cleanup are outside this budget. A `TimeoutError` raised by the SDK before this deadline expires retains its own error message.
    - MiniMax Code speaks ACP directly. Install and authenticate it, then add it as an ACP agent:
 

backend/packages/harness/deerflow/tools/AGENTS.md

@@ -108,7 +108,7 @@ E2B output sync records remote file versions and actual host file metadata in a
 - `timeout_seconds` is one deadline shared by initialization, session creation, and prompt after process launch. Keep the SDK process context outside the timeout so teardown finishes before the tool returns; workspace/MCP preparation and teardown are outside the protocol budget. Real-process regressions in `tests/test_invoke_acp_agent_tool.py` cover every hung phase, the shared budget, success, and caller cancellation. Only report budget expiry when the timeout context's `expired()` is true; otherwise propagate SDK `TimeoutError` to `_format_invocation_error`.
 - ACP results collect only `agent_message_chunk` text. Thought chunks remain internal and must not be concatenated into the tool result
 - Missing ACP executables now return an actionable error message instead of a raw `[Errno 2]`
-- A bare configured `command` is resolved before the spawn (`_resolve_agent_command`), off the event loop because the lookup stats the filesystem. The ACP SDK's stdio transport calls `asyncio.create_subprocess_exec`, which performs no `PATHEXT` lookup on Windows, so a bare npm launcher (`npx`, `mcode`) would raise `FileNotFoundError` although its `.cmd` shim is on `PATH`. Resolution consults the `PATH` the agent subprocess will actually see (`acp_agents.<name>.env.PATH` when the agent config sets one, the Gateway's otherwise) and returns an absolute path, so a relative `PATH` entry cannot be re-interpreted inside the ACP workspace. A `command` containing a path separator goes to the spawn unchanged, so the spawn — not `shutil.which` — decides what it refers to, and `_format_invocation_error` always receives the configured value so its remediation still matches `config.yaml`
+- A configured `command` that is a bare name or an absolute path is resolved before the spawn (`_resolve_agent_command`), off the event loop because the lookup stats the filesystem. The ACP SDK's stdio transport calls `asyncio.create_subprocess_exec`, which performs no `PATHEXT` lookup on Windows, so a bare npm launcher (`npx`, `mcode`) would raise `FileNotFoundError`, and an absolute path without an extension (`C:\npm\mcode`, npm's shell script beside its `.cmd` shim) would die with `WinError 193`, although the shim exists. Resolution consults the `PATH` the agent subprocess will actually see (`acp_agents.<name>.env.PATH` when the agent config sets one, the Gateway's otherwise) and returns an absolute path, so a relative `PATH` entry cannot be re-interpreted inside the ACP workspace; an absolute command is resolved inside its own directory. A relative `command` goes to the spawn unchanged, so the spawn — not `shutil.which` — decides what it refers to, and `_format_invocation_error` always receives the configured value so its remediation still matches `config.yaml`
 - Each ACP agent uses a per-thread workspace at `{base_dir}/users/{user_id}/threads/{thread_id}/acp-workspace/`. The workspace is accessible to the lead agent via the virtual path `/mnt/acp-workspace/` (read-only). In docker sandbox mode, the directory is volume-mounted into the container at `/mnt/acp-workspace` (read-only); in local sandbox mode, path translation is handled by `tools.py`
 
 Ordinary `task` results forward bounded `artifact.knowledge_sources` records

backend/packages/harness/deerflow/tools/builtins/invoke_acp_agent_tool.py

@@ -143,18 +143,22 @@ def _agent_path(env: dict[str, str] | None) -> str | None:
 
 
 def _resolve_agent_command(command: str, path: str | None = None) -> str:
-    """Return the host path to spawn for a configured ACP agent command.
+    r"""Return the host path to spawn for a configured ACP agent command.
 
     ``asyncio.create_subprocess_exec``, which the ACP SDK's stdio transport
     uses, does not apply ``PATHEXT`` on Windows: a bare ``npx`` or ``mcode``
     raises ``FileNotFoundError`` even though the npm shim (``npx.cmd``) is on
-    ``PATH``. Resolve the name the way the MCP Python SDK normalizes stdio
-    commands, and keep the configured value when nothing matches so the
-    not-found remediation still fires.
+    ``PATH``, and an absolute ``C:\npm\mcode`` spawns npm's extensionless shell
+    script instead of ``mcode.CMD`` and dies with ``WinError 193``. Resolve
+    the command the way the MCP Python SDK normalizes stdio commands, and keep
+    the configured value when nothing matches so the not-found remediation
+    still fires.
 
-    Only bare names are resolved. A configured path is handed to the spawn
-    unchanged, so the spawn -- not ``shutil.which`` -- decides what it refers
-    to.
+    Bare names and absolute paths are resolved. A relative configured path is
+    handed to the spawn unchanged: it refers to the ACP workspace the spawn
+    runs in, not to the Gateway's cwd. An absolute path keeps its own
+    directory -- ``shutil.which`` searches that directory and probes
+    ``PATHEXT`` there.
 
     ``path`` is the ``PATH`` the agent subprocess will actually see (see
     ``_agent_path``); the Gateway's own is only the fallback. A successful
@@ -163,16 +167,22 @@ def _resolve_agent_command(command: str, path: str | None = None) -> str:
     ``cwd`` set to the ACP workspace, where that path resolves elsewhere -- or
     not at all.
 
+    An already-absolute result is returned unchanged: a lexical ``abspath``
+    would collapse a ``..`` that follows a symlink, changing which file the
+    spawn actually runs.
+
     The lookup stats the filesystem (``shutil.which`` -> ``os.access``), so
     callers must run it off the event loop.
     """
-    if os.path.dirname(command):
+    if os.path.dirname(command) and not os.path.isabs(command):
         return command
     try:
         resolved = shutil.which(command, path=path)
     except OSError:
         return command
-    return os.path.abspath(resolved) if resolved else command
+    if not resolved:
+        return command
+    return resolved if os.path.isabs(resolved) else os.path.abspath(resolved)
 
 
 def _format_invocation_error(agent: str, cmd: str, exc: Exception) -> str:

backend/tests/test_invoke_acp_agent_tool.py

@@ -1061,6 +1061,53 @@ async def test_invoke_acp_agent_spawns_the_path_resolved_command(monkeypatch, tm
     assert result == "(no response)"
 
 
+@pytest.mark.anyio
+async def test_invoke_acp_agent_spawns_an_absolute_command_without_an_extension(monkeypatch, tmp_path):
+    """An absolute command has to reach its ``PATHEXT`` shim before the spawn.
+
+    npm installs ``mcode``, ``mcode.cmd``, and ``mcode.ps1`` side by side; a
+    configured absolute path without the extension would spawn the shell
+    script and fail with ``WinError 193``. The resolver has to pick the
+    ``.cmd`` shim the same way it does for a bare name.
+    """
+    import acp as acp_module
+
+    from deerflow.config import paths as paths_module
+
+    monkeypatch.setattr(paths_module, "get_paths", lambda: paths_module.Paths(base_dir=tmp_path))
+    monkeypatch.setattr(
+        "deerflow.config.extensions_config.ExtensionsConfig.from_file",
+        classmethod(lambda cls: ExtensionsConfig(mcp_servers={}, skills={})),
+    )
+    script_path = tmp_path / "test_acp_agent.py"
+    script_path.write_text(_ACP_AGENT_SCRIPT, encoding="utf-8")
+    phase_path = tmp_path / "phase.txt"
+
+    shim_name = "deerflow-acp-probe"
+    shim = _write_launcher_shim(tmp_path / "npm", shim_name, f'"{sys.executable}" "{script_path}" "" 0 "{phase_path}"')
+    if sys.platform == "win32":
+        # npm also drops an extensionless shell script next to the .cmd shim.
+        (shim.parent / shim_name).write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
+
+    captured: dict[str, object] = {}
+    real_spawn_agent_process = acp_module.spawn_agent_process
+
+    @contextlib.asynccontextmanager
+    async def _spying_spawn_agent_process(client, cmd, *args, env=None, cwd=None):
+        captured["cmd"] = cmd
+        async with real_spawn_agent_process(client, cmd, *args, env=env, cwd=cwd) as (conn, proc):
+            yield conn, proc
+
+    monkeypatch.setattr(acp_module, "spawn_agent_process", _spying_spawn_agent_process)
+
+    configured = str(shim.parent / shim_name)
+    tool = build_invoke_acp_agent_tool({"probe": ACPAgentConfig(command=configured, description="Probe", timeout_seconds=30)})
+    result = await asyncio.wait_for(tool.coroutine(agent="probe", prompt="do work"), timeout=20)
+
+    assert Path(captured["cmd"]) == shim
+    assert result == "(no response)"
+
+
 @pytest.mark.anyio
 async def test_invoke_acp_agent_reports_guidance_for_an_unresolvable_command(monkeypatch, tmp_path):
     """A command PATH cannot resolve still reports the configured name.
@@ -1092,8 +1139,8 @@ def test_agent_path_reads_the_configured_override_case_insensitively():
     assert _agent_path({"Path": "/agent-bin"}) == "/agent-bin"
 
 
-def test_resolve_agent_command_only_resolves_bare_names(monkeypatch, tmp_path):
-    """A configured path is left to the spawn, which runs in the ACP workspace."""
+def test_resolve_agent_command_leaves_relative_paths_to_the_spawn(monkeypatch, tmp_path):
+    """A relative configured path is left to the spawn, which runs in the ACP workspace."""
     resolved_dir = tmp_path / "resolved"
     monkeypatch.setattr(
         "deerflow.tools.builtins.invoke_acp_agent_tool.shutil.which",
@@ -1105,6 +1152,90 @@ def test_resolve_agent_command_only_resolves_bare_names(monkeypatch, tmp_path):
     assert _resolve_agent_command("./bin/agent") == "./bin/agent"
 
 
+def test_resolve_agent_command_resolves_absolute_paths_through_the_lookup(monkeypatch, tmp_path):
+    """An absolute command keeps its directory but still goes through the lookup."""
+    seen: list[tuple[str, str | None]] = []
+
+    def _which(command, path=None):
+        seen.append((command, path))
+        return command + ".CMD"
+
+    monkeypatch.setattr("deerflow.tools.builtins.invoke_acp_agent_tool.shutil.which", _which)
+
+    absolute = str(tmp_path / "npm" / "mcode")
+    assert _resolve_agent_command(absolute, str(tmp_path / "agent-bin")) == absolute + ".CMD"
+    assert seen == [(absolute, str(tmp_path / "agent-bin"))]
+
+
+def test_resolve_agent_command_keeps_an_absolute_lookup_result_as_returned(monkeypatch, tmp_path):
+    """An already-absolute lookup result must not be lexically rewritten.
+
+    ``os.path.abspath`` collapses a ``..`` that the kernel resolves after a
+    symlink, which changes the file the spawn runs; the anchoring exists for
+    relative ``PATH`` results only.
+    """
+    literal = str(tmp_path / "current" / ".." / "shared" / "agent")
+    monkeypatch.setattr(
+        "deerflow.tools.builtins.invoke_acp_agent_tool.shutil.which",
+        lambda command, path=None: command,
+    )
+
+    assert _resolve_agent_command(literal) == literal
+
+
+@pytest.mark.skipif(sys.platform != "win32", reason="PATHEXT sibling resolution is Windows-only")
+def test_resolve_agent_command_resolves_a_windows_path_without_an_extension(tmp_path):
+    """npm's extensionless sh script and its ``.cmd`` shim sit side by side.
+
+    ``asyncio.create_subprocess_exec`` does not probe ``PATHEXT``, so spawning
+    the configured path runs the shell script and fails with ``WinError 193``;
+    the lookup has to pick the ``.cmd`` shim instead.
+    """
+    npm_bin = tmp_path / "npm"
+    npm_bin.mkdir()
+    extensionless = npm_bin / "mcode"
+    extensionless.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
+    (npm_bin / "mcode.cmd").write_text("@echo off\r\necho ran\r\n", encoding="ascii")
+
+    resolved = _resolve_agent_command(str(extensionless))
+
+    assert Path(resolved) == npm_bin / "mcode.CMD"
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="POSIX has no PATHEXT indirection to resolve")
+def test_resolve_agent_command_keeps_an_absolute_posix_command(tmp_path):
+    """On POSIX the absolute command itself is the executable; resolution keeps it."""
+    tool = tmp_path / "bin" / "mcode"
+    tool.parent.mkdir()
+    tool.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
+    tool.chmod(0o755)
+
+    assert _resolve_agent_command(str(tool)) == str(tool)
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="symlink-plus-parent-segment resolution is POSIX-specific")
+def test_resolve_agent_command_keeps_a_posix_symlink_that_precedes_a_parent_segment(tmp_path):
+    """An absolute command is returned as configured, not lexically rewritten.
+
+    With ``current -> releases/v1``, ``current/../shared/agent`` resolves to
+    ``releases/shared/agent`` for the kernel, while a lexical ``abspath``
+    would collapse it to the nonexistent ``shared/agent`` beside the symlink
+    or to a different file that happens to live there.
+    """
+    release = tmp_path / "releases" / "v1"
+    release.mkdir(parents=True)
+    tool = tmp_path / "releases" / "shared" / "agent"
+    tool.parent.mkdir(parents=True)
+    tool.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8")
+    tool.chmod(0o755)
+    current = tmp_path / "current"
+    current.symlink_to(release, target_is_directory=True)
+
+    configured = str(current / ".." / "shared" / "agent")
+
+    assert _resolve_agent_command(configured) == configured
+
+
 def test_resolve_agent_command_hands_the_effective_path_to_the_lookup(monkeypatch, tmp_path):
     """The agent's own PATH, not the Gateway's, is what the lookup consults."""
     seen: list[str | None] = []