From 277a37f153185763288c37a1f095aaeb504e7e1a Mon Sep 17 00:00:00 2001 From: Ramnath0521 Date: Sun, 6 Sep 2026 02:06:13 +0530 Subject: [PATCH] fix: name the Windows command interpreter in the Runtime prompt section TurnRunner built the Runtime prompt block with os.environ.get("SHELL", ""). SHELL is a POSIX variable that Windows never sets, and system_prompt.j2 renders the field unconditionally, so every Windows turn told the model: ## Runtime - OS: Windows - Shell: An empty declared field is worse than an absent one: the model is told a shell exists, given nothing to identify it, and falls back to POSIX idioms that then fail on the Windows command runner. process_tree.default_command_shell() names the interpreter that actually runs agent commands, and create_owned_subprocess_shell now calls it for its Windows branch instead of reading COMSPEC separately, so the value reported to the model and the value handed to CreateProcess cannot drift. POSIX behaviour is unchanged. Refs #941 Co-Authored-By: Claude Opus 5 --- src/opensquilla/engine/runtime.py | 4 ++-- src/opensquilla/process_tree.py | 18 +++++++++++++-- tests/test_process_tree.py | 38 +++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/opensquilla/engine/runtime.py b/src/opensquilla/engine/runtime.py index f850b790b..99e310809 100644 --- a/src/opensquilla/engine/runtime.py +++ b/src/opensquilla/engine/runtime.py @@ -228,7 +228,7 @@ from opensquilla.observability.trace import TraceContext, TraceEvent, write_trace_event from opensquilla.observability.turn_call_log import TurnCallLogger, is_turn_call_log_enabled from opensquilla.paths import media_root_from_config -from opensquilla.process_tree import task_process_scope +from opensquilla.process_tree import default_command_shell, task_process_scope from opensquilla.provider import ( ErrorEvent as ProviderErrorEvent, ) @@ -8680,7 +8680,7 @@ def _assemble_prompt( if restricted_tool_boundary else { "os": os_name, - "shell": os.environ.get("SHELL", ""), + "shell": default_command_shell(), "workspace_dir": str(workspace_dir or bootstrap_workspace_dir), } ) diff --git a/src/opensquilla/process_tree.py b/src/opensquilla/process_tree.py index c2f6b84f0..67c82f1c0 100644 --- a/src/opensquilla/process_tree.py +++ b/src/opensquilla/process_tree.py @@ -2269,10 +2269,23 @@ async def create_owned_subprocess_exec(*argv: str, **kwargs: Any) -> Any: raise ProcessTreeOwnershipError(f"unsupported process-tree platform: {os.name}") +def default_command_shell() -> str: + """Name the interpreter that runs agent shell commands on this host. + + Windows never sets ``SHELL``, so reading it there yields an empty string. + ``create_owned_subprocess_shell`` execs ``COMSPEC`` on Windows, which makes + that the honest answer. POSIX keeps reporting the login shell it always has. + """ + if os.name == "nt": + return os.environ.get("COMSPEC", "cmd.exe") + return os.environ.get("SHELL", "") + + async def create_owned_subprocess_shell(command: str, **kwargs: Any) -> Any: if os.name == "nt": - comspec = os.environ.get("COMSPEC", "cmd.exe") - return await create_owned_subprocess_exec(comspec, "/d", "/s", "/c", command, **kwargs) + return await create_owned_subprocess_exec( + default_command_shell(), "/d", "/s", "/c", command, **kwargs + ) if os.name != "posix": raise ProcessTreeOwnershipError(f"unsupported process-tree platform: {os.name}") child_kwargs = dict(kwargs) @@ -3001,6 +3014,7 @@ async def _wait_direct_process(process: Any, timeout: float) -> bool: "create_owned_popen", "create_owned_subprocess_exec", "create_owned_subprocess_shell", + "default_command_shell", "main", "reconcile_persisted_processes", "task_process_scope", diff --git a/tests/test_process_tree.py b/tests/test_process_tree.py index 3fe6e39a3..9a68999f5 100644 --- a/tests/test_process_tree.py +++ b/tests/test_process_tree.py @@ -3060,3 +3060,41 @@ async def test_windows_job_kills_descendant_after_direct_leader_exits(tmp_path) assert kernel32.WaitForSingleObject(child_handle, 5000) == 0 finally: kernel32.CloseHandle(child_handle) + + +def test_default_command_shell_reports_comspec_on_windows(monkeypatch) -> None: + """On Windows the reported shell must name the interpreter that actually runs + commands. create_owned_subprocess_shell execs COMSPEC there, so SHELL -- + which Windows never sets -- left the field empty.""" + monkeypatch.setattr(os, "name", "nt") + monkeypatch.delenv("SHELL", raising=False) + monkeypatch.setenv("COMSPEC", r"C:\WINDOWS\system32\cmd.exe") + + assert process_tree.default_command_shell() == r"C:\WINDOWS\system32\cmd.exe" + + +def test_default_command_shell_falls_back_when_comspec_missing(monkeypatch) -> None: + """COMSPEC is effectively always set on Windows, but the field must never be + empty even if it is not.""" + monkeypatch.setattr(os, "name", "nt") + monkeypatch.delenv("SHELL", raising=False) + monkeypatch.delenv("COMSPEC", raising=False) + + assert process_tree.default_command_shell() == "cmd.exe" + + +def test_default_command_shell_keeps_posix_shell(monkeypatch) -> None: + """Control: POSIX behaviour is unchanged -- still the login shell.""" + monkeypatch.setattr(os, "name", "posix") + monkeypatch.setenv("SHELL", "/bin/zsh") + + assert process_tree.default_command_shell() == "/bin/zsh" + + +def test_default_command_shell_is_empty_when_posix_shell_unset(monkeypatch) -> None: + """Control: the POSIX empty case is pre-existing behaviour, not a regression + this change is expected to fix.""" + monkeypatch.setattr(os, "name", "posix") + monkeypatch.delenv("SHELL", raising=False) + + assert process_tree.default_command_shell() == ""