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() == ""