From 34f01c0b58c29993cdecd3ad90163882034e638f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o?= Date: Thu, 1 Oct 2026 16:03:03 +0200 Subject: [PATCH] fix(agent): capture Windows Bash output and pass the subprocess env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows branch of `_create_bash_subprocess` spawned Git Bash with neither pipes nor the env it was handed. `proc.stdout` and `proc.stderr` came back `None`, so `_run_subprocess_streaming`'s reader returned immediately and the Bash tool reported `"(no output)"` alongside the real exit code — while the child inherited the server's own stdout/stderr and wrote agent command output into the console and the launchd/Docker logs. The `env` parameter was accepted and never used, so `PATH`, `VIRTUAL_ENV`, `HOME`, `TMPDIR` and the configured import paths carried in `ctx["subproc_env"]` never reached the child on Windows, even though every POSIX path applies them. Spawn it the way the POSIX path at `:688` already does: `stdin=DEVNULL`, `stdout=PIPE`, `stderr=PIPE`, `env=env`. `website/configuration-reference.md` is generated from source line numbers, so the four added lines shift one entry; regenerated with `scripts/generate_env_reference.py`. --- src/agent_tools/subprocess_tools.py | 4 ++ tests/test_agent_bash_windows.py | 78 +++++++++++++++++++++++++++++ website/configuration-reference.md | 2 +- 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/src/agent_tools/subprocess_tools.py b/src/agent_tools/subprocess_tools.py index 7537c82b5..b4b8b5405 100644 --- a/src/agent_tools/subprocess_tools.py +++ b/src/agent_tools/subprocess_tools.py @@ -117,6 +117,10 @@ async def _create_bash_subprocess( bash, "-c", str(command or ""), + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + env=env, cwd=cwd, ) kwargs = {"cwd": cwd} if cwd is not None else {} diff --git a/tests/test_agent_bash_windows.py b/tests/test_agent_bash_windows.py index 0145b4b28..fabcee974 100644 --- a/tests/test_agent_bash_windows.py +++ b/tests/test_agent_bash_windows.py @@ -1,5 +1,7 @@ """Windows execution contract for the agent Bash tool.""" +import asyncio + import pytest from types import SimpleNamespace @@ -38,6 +40,82 @@ async def test_windows_bash_uses_git_bash_with_structural_cwd(monkeypatch): assert captured["kwargs"]["cwd"] == workspace +@pytest.mark.asyncio +async def test_windows_bash_captures_output_instead_of_inheriting_server_handles(monkeypatch): + captured = {} + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr( + subprocess_tools, "find_bash", lambda: r"C:\Program Files\Git\bin\bash.exe" + ) + + async def fake_exec(*_argv, **kwargs): + captured.update(kwargs) + return object() + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fake_exec) + + await subprocess_tools._create_bash_subprocess("pwd", cwd=r"C:\Work") + + assert captured["stdout"] == asyncio.subprocess.PIPE + assert captured["stderr"] == asyncio.subprocess.PIPE + assert captured["stdin"] == asyncio.subprocess.DEVNULL + + +@pytest.mark.asyncio +async def test_windows_bash_applies_the_subprocess_env(monkeypatch): + captured = {} + env = {"PATH": r"C:\Odysseus\venv\Scripts", "HOME": r"C:\Odysseus\data"} + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr( + subprocess_tools, "find_bash", lambda: r"C:\Program Files\Git\bin\bash.exe" + ) + + async def fake_exec(*_argv, **kwargs): + captured.update(kwargs) + return object() + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fake_exec) + + await subprocess_tools._create_bash_subprocess("pwd", cwd=r"C:\Work", env=env) + + assert captured["env"] == env + + +@pytest.mark.asyncio +async def test_windows_bash_tool_passes_ctx_env_through_to_the_child(monkeypatch): + captured = {} + env = {"PATH": r"C:\Odysseus\venv\Scripts", "VIRTUAL_ENV": r"C:\Odysseus\venv"} + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr( + subprocess_tools, "find_bash", lambda: r"C:\Program Files\Git\bin\bash.exe" + ) + monkeypatch.setattr("src.tool_execution.agent_cwd", lambda: r"D:\Workspaces\Project") + + async def fake_exec(*argv, **kwargs): + captured["argv"] = argv + captured["kwargs"] = kwargs + return SimpleNamespace(pid=4242) + + async def fake_stream(_process, **_kwargs): + return "ok", "", 0, False + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fake_exec) + monkeypatch.setattr(subprocess_tools, "_run_subprocess_streaming", fake_stream) + + result = await subprocess_tools.BashTool().execute( + "pwd", + {"subproc_env": env, "session_id": "chat-1"}, + ) + + assert result == {"output": "ok", "exit_code": 0} + assert captured["kwargs"]["env"] == env + assert captured["kwargs"]["stdout"] == asyncio.subprocess.PIPE + assert captured["kwargs"]["stderr"] == asyncio.subprocess.PIPE + + @pytest.mark.asyncio async def test_windows_bash_without_git_bash_fails_clearly(monkeypatch): monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) diff --git a/website/configuration-reference.md b/website/configuration-reference.md index fc2a3bdeb..ba0541539 100644 --- a/website/configuration-reference.md +++ b/website/configuration-reference.md @@ -75,7 +75,7 @@ The source tree reads **108** `ODYSSEUS_*` variables: 78 an operator may want to | `ODYSSEUS_MAX_VISUAL_EVIDENCE_FRAMES` | `'3'` | `src/agent_loop.py:15361` | How many video frames one tool result may contribute. Clamped to 1-8. | | `ODYSSEUS_MAX_VISUAL_EVIDENCE_IMAGES` | `'1'` | `src/agent_loop.py:15329` | How many images one tool result may contribute to the model turn. Clamped to 1-8. | | `ODYSSEUS_MCP_ALLOWED_COMMANDS` | `''` | `src/agent_tools/admin_tools.py:140` | Security-relevant. Comma-separated allowlist of MCP launcher basenames the agent may start. Empty by default, and the deny list still wins. | -| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:927` | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. | +| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:931` | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. | | `ODYSSEUS_SCRIPT_HOST` | `'localhost'` | `src/builtin_actions.py:919` | Default host for the run-script action. `localhost`, `127.0.0.1`, `local` and empty run locally; any other value runs over SSH. | | `ODYSSEUS_TOOL_APPROVAL_GATE` | `'0'` | `src/tool_capabilities.py:645` | Security-relevant. Truthy makes tool calls pass through the approval gate. Off by default. |