diff --git a/src/agent_tools/subprocess_tools.py b/src/agent_tools/subprocess_tools.py index 15041c76e..1c407b112 100644 --- a/src/agent_tools/subprocess_tools.py +++ b/src/agent_tools/subprocess_tools.py @@ -6,6 +6,7 @@ import sys import time import collections from typing import Optional, Callable, Awaitable, Tuple, Dict +from core.platform_compat import IS_WINDOWS, find_bash from src.constants import MAX_OUTPUT_CHARS DEFAULT_BASH_TIMEOUT = 60 * 60 # 1 hour @@ -16,6 +17,27 @@ PROGRESS_TAIL_LINES = 12 TMUX_CAPTURE_LINES = 2000 +async def _create_bash_subprocess(command: str, **kwargs): + """Start the agent shell with Bash semantics on every supported OS. + + ``asyncio.create_subprocess_shell`` delegates to ``cmd.exe`` on native + Windows. That contradicts the Bash tool contract and makes POSIX commands + such as ``pwd``, ``ls -la``, and ``cat`` unreliable even when the launcher + has found Git Bash. Pass the selected workspace as a structural ``cwd`` + argument; Git Bash inherits that native Windows directory and exposes it + using its normal ``/c/...`` representation. + """ + if IS_WINDOWS: + bash = find_bash() + if not bash: + raise RuntimeError( + "Git Bash is required for the Bash tool on Windows; " + "install Git for Windows and restart Odysseus" + ) + return await asyncio.create_subprocess_exec(bash, "-c", command, **kwargs) + return await asyncio.create_subprocess_shell(command, **kwargs) + + def _tmux_session_name(session_id: Optional[str]) -> str: raw = re.sub(r"[^A-Za-z0-9_.-]+", "-", str(session_id or "default")).strip("-") return f"ody-agent-{raw[:80] or 'default'}" @@ -280,7 +302,10 @@ class BashTool: progress_cb = ctx.get("progress_cb") _subproc_env = ctx.get("subproc_env") session_id = ctx.get("session_id") - if session_id and shutil.which("tmux"): + # tmux is a POSIX persistence path. A stray MSYS/Cygwin tmux.exe on + # native Windows must not bypass the Git Bash launcher below: the tmux + # setup hard-codes /bin/bash and cannot safely consume a native cwd. + if session_id and not IS_WINDOWS and shutil.which("tmux"): stdout, stderr, rc, timed_out = await _run_tmux_bash( content, session_id=str(session_id), @@ -307,13 +332,16 @@ class BashTool: "tmux_session": _tmux_session_name(str(session_id)), } - proc = await asyncio.create_subprocess_shell( - content, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - env=_subproc_env, - cwd=agent_cwd(), - ) + try: + proc = await _create_bash_subprocess( + content, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + env=_subproc_env, + cwd=agent_cwd(), + ) + except RuntimeError as e: + return {"error": f"bash: {e}", "exit_code": 1} stdout, stderr, rc, timed_out = await _run_subprocess_streaming( proc, timeout=DEFAULT_BASH_TIMEOUT, diff --git a/tests/test_agent_bash_windows.py b/tests/test_agent_bash_windows.py new file mode 100644 index 000000000..888c906b3 --- /dev/null +++ b/tests/test_agent_bash_windows.py @@ -0,0 +1,128 @@ +"""Windows execution contract for the agent Bash tool.""" + +import pytest + +from src.agent_tools import subprocess_tools + + +@pytest.mark.asyncio +async def test_windows_bash_uses_git_bash_with_structural_cwd(monkeypatch): + captured = {} + bash = r"C:\Program Files\Git\bin\bash.exe" + workspace = r"D:\Workspaces\Project with spaces" + process = object() + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr(subprocess_tools, "find_bash", lambda: bash) + + async def fake_exec(*argv, **kwargs): + captured["argv"] = argv + captured["kwargs"] = kwargs + return process + + async def fail_shell(*_args, **_kwargs): + pytest.fail("native Windows Bash must not execute through cmd.exe") + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fake_exec) + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_shell", fail_shell) + + result = await subprocess_tools._create_bash_subprocess( + "pwd; cat package.json", + cwd=workspace, + env={"HOME": r"C:\Odysseus\data"}, + ) + + assert result is process + assert captured["argv"] == (bash, "-c", "pwd; cat package.json") + assert captured["kwargs"]["cwd"] == workspace + + +@pytest.mark.asyncio +async def test_windows_bash_without_git_bash_fails_clearly(monkeypatch): + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr(subprocess_tools, "find_bash", lambda: None) + + async def fail_spawn(*_args, **_kwargs): + pytest.fail("no subprocess should start without Git Bash") + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fail_spawn) + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_shell", fail_spawn) + + with pytest.raises(RuntimeError, match="Git Bash is required"): + await subprocess_tools._create_bash_subprocess("pwd", cwd=r"C:\Work") + + +@pytest.mark.asyncio +async def test_bash_tool_returns_install_hint_when_git_bash_is_missing(monkeypatch): + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr(subprocess_tools, "find_bash", lambda: None) + + result = await subprocess_tools.BashTool().execute( + "pwd", + {"subproc_env": {}, "session_id": None}, + ) + + assert result["exit_code"] == 1 + assert "install Git for Windows" in result["error"] + + +@pytest.mark.asyncio +async def test_windows_bash_does_not_use_a_stray_tmux_executable(monkeypatch): + captured = {} + workspace = r"D:\Workspaces\Project with spaces" + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", True) + monkeypatch.setattr( + subprocess_tools.shutil, + "which", + lambda name: r"C:\msys64\usr\bin\tmux.exe", + ) + monkeypatch.setattr("src.tool_execution.agent_cwd", lambda: workspace) + + async def fail_tmux(*_args, **_kwargs): + pytest.fail("native Windows must not enter the POSIX tmux path") + + async def fake_create(command, **kwargs): + captured["command"] = command + captured["kwargs"] = kwargs + return object() + + async def fake_stream(_process, **_kwargs): + return "ok", "", 0, False + + monkeypatch.setattr(subprocess_tools, "_run_tmux_bash", fail_tmux) + monkeypatch.setattr(subprocess_tools, "_create_bash_subprocess", fake_create) + monkeypatch.setattr(subprocess_tools, "_run_subprocess_streaming", fake_stream) + + result = await subprocess_tools.BashTool().execute( + "pwd", + {"subproc_env": {}, "session_id": "chat-1"}, + ) + + assert result == {"output": "ok", "exit_code": 0} + assert captured["command"] == "pwd" + assert captured["kwargs"]["cwd"] == workspace + + +@pytest.mark.asyncio +async def test_posix_bash_keeps_existing_shell_path(monkeypatch): + captured = {} + process = object() + + monkeypatch.setattr(subprocess_tools, "IS_WINDOWS", False) + + async def fake_shell(command, **kwargs): + captured["command"] = command + captured["kwargs"] = kwargs + return process + + async def fail_exec(*_args, **_kwargs): + pytest.fail("POSIX behavior must continue through create_subprocess_shell") + + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_shell", fake_shell) + monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_exec", fail_exec) + + result = await subprocess_tools._create_bash_subprocess("pwd", cwd="/tmp/work") + + assert result is process + assert captured == {"command": "pwd", "kwargs": {"cwd": "/tmp/work"}}