From 9d63dcc3f9eab2c67923229c519a321295b0d40b Mon Sep 17 00:00:00 2001 From: "balyan.sid@gmail.com" Date: Thu, 12 Mar 2026 23:38:11 +0530 Subject: [PATCH 1/7] add persistent ssh backend --- tools/environments/ssh.py | 309 +++++++++++++++++++++++++++++++++++++- tools/terminal_tool.py | 3 + 2 files changed, 308 insertions(+), 4 deletions(-) diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 83cc335b1..7a31006db 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -1,10 +1,12 @@ """SSH remote execution environment with ControlMaster connection persistence.""" import logging +import shlex import subprocess import tempfile import threading import time +import uuid from pathlib import Path from tools.environments.base import BaseEnvironment @@ -22,21 +24,44 @@ class SSHEnvironment(BaseEnvironment): Foreground commands are interruptible: the local ssh process is killed and a remote kill is attempted over the ControlMaster socket. + + When ``persistent=True``, a single long-lived bash shell is kept alive + over SSH and state (cwd, env vars, shell variables) persists across + ``execute()`` calls. Output capture uses file-based IPC on the remote + host (stdout/stderr/exit-code written to temp files, polled via fast + ControlMaster one-shot reads). """ def __init__(self, host: str, user: str, cwd: str = "~", - timeout: int = 60, port: int = 22, key_path: str = ""): + timeout: int = 60, port: int = 22, key_path: str = "", + persistent: bool = False): super().__init__(cwd=cwd, timeout=timeout) self.host = host self.user = user self.port = port self.key_path = key_path + self.persistent = persistent self.control_dir = Path(tempfile.gettempdir()) / "hermes-ssh" self.control_dir.mkdir(parents=True, exist_ok=True) self.control_socket = self.control_dir / f"{user}@{host}:{port}.sock" self._establish_connection() + # Persistent shell state + self._shell_proc: subprocess.Popen | None = None + self._shell_lock = threading.Lock() + self._shell_alive = False + self._session_id: str = "" + self._remote_stdout: str = "" + self._remote_stderr: str = "" + self._remote_status: str = "" + self._remote_cwd: str = "" + self._remote_pid: str = "" + self._remote_shell_pid: int | None = None + + if self.persistent: + self._start_persistent_shell() + def _build_ssh_command(self, extra_args: list = None) -> list: cmd = ["ssh"] cmd.extend(["-o", f"ControlPath={self.control_socket}"]) @@ -65,9 +90,240 @@ class SSHEnvironment(BaseEnvironment): except subprocess.TimeoutExpired: raise RuntimeError(f"SSH connection to {self.user}@{self.host} timed out") - def execute(self, command: str, cwd: str = "", *, - timeout: int | None = None, - stdin_data: str | None = None) -> dict: + # ------------------------------------------------------------------ + # Persistent shell management + # ------------------------------------------------------------------ + + def _start_persistent_shell(self): + """Spawn a long-lived bash shell over SSH.""" + self._session_id = uuid.uuid4().hex[:12] + prefix = f"/tmp/hermes-ssh-{self._session_id}" + self._remote_stdout = f"{prefix}-stdout" + self._remote_stderr = f"{prefix}-stderr" + self._remote_status = f"{prefix}-status" + self._remote_cwd = f"{prefix}-cwd" + self._remote_pid = f"{prefix}-pid" + + cmd = self._build_ssh_command() + cmd.append("bash -l") + + self._shell_proc = subprocess.Popen( + cmd, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + self._shell_alive = True + + # Start daemon thread to drain stdout/stderr and detect shell death + self._drain_thread = threading.Thread( + target=self._drain_shell_output, daemon=True + ) + self._drain_thread.start() + + # Initialize remote temp files and capture shell PID + init_script = ( + f"touch {self._remote_stdout} {self._remote_stderr} " + f"{self._remote_status} {self._remote_cwd} {self._remote_pid}\n" + f"echo $$ > {self._remote_pid}\n" + f"pwd > {self._remote_cwd}\n" + ) + self._send_to_shell(init_script) + + # Give shell time to initialize and write PID file + time.sleep(0.3) + + # Read the remote shell PID + pid_str = self._read_remote_file(self._remote_pid).strip() + if pid_str.isdigit(): + self._remote_shell_pid = int(pid_str) + logger.info("Persistent shell started (session=%s, pid=%d)", + self._session_id, self._remote_shell_pid) + else: + logger.warning("Could not read persistent shell PID (got %r)", pid_str) + self._remote_shell_pid = None + + # Update cwd from what the shell reports + remote_cwd = self._read_remote_file(self._remote_cwd).strip() + if remote_cwd: + self.cwd = remote_cwd + + def _drain_shell_output(self): + """Drain the shell's stdout/stderr to prevent pipe deadlock. + + Also detects when the shell process dies. + """ + try: + for _ in self._shell_proc.stdout: + pass # Discard — real output goes to temp files + except Exception: + pass + self._shell_alive = False + + def _send_to_shell(self, text: str): + """Write text to the persistent shell's stdin.""" + if not self._shell_alive or self._shell_proc is None: + return + try: + self._shell_proc.stdin.write(text) + self._shell_proc.stdin.flush() + except (BrokenPipeError, OSError): + self._shell_alive = False + + def _read_remote_file(self, path: str) -> str: + """Read a file on the remote host via a one-shot SSH command. + + Uses ControlMaster so this is very fast (~5ms on LAN). + """ + cmd = self._build_ssh_command() + cmd.append(f"cat {path} 2>/dev/null") + try: + result = subprocess.run( + cmd, capture_output=True, text=True, timeout=10 + ) + return result.stdout + except (subprocess.TimeoutExpired, OSError): + return "" + + def _kill_shell_children(self): + """Kill children of the persistent shell (the running command), + but not the shell itself.""" + if self._remote_shell_pid is None: + return + cmd = self._build_ssh_command() + cmd.append(f"pkill -P {self._remote_shell_pid} 2>/dev/null; true") + try: + subprocess.run(cmd, capture_output=True, timeout=5) + except (subprocess.TimeoutExpired, OSError): + pass + + def _execute_persistent(self, command: str, cwd: str, *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + """Execute a command in the persistent shell.""" + # If shell is dead, restart it + if not self._shell_alive: + logger.info("Persistent shell died, restarting...") + self._start_persistent_shell() + + exec_command, sudo_stdin = self._prepare_command(command) + effective_timeout = timeout or self.timeout + + # Fall back to one-shot for commands needing piped stdin + if stdin_data or sudo_stdin: + return self._execute_oneshot( + command, cwd, timeout=timeout, stdin_data=stdin_data + ) + + with self._shell_lock: + return self._execute_persistent_locked( + exec_command, cwd, effective_timeout + ) + + def _execute_persistent_locked(self, command: str, cwd: str, + timeout: int) -> dict: + """Inner persistent execution — caller must hold _shell_lock.""" + work_dir = cwd or self.cwd + + # Truncate temp files + truncate = ( + f": > {self._remote_stdout}\n" + f": > {self._remote_stderr}\n" + f": > {self._remote_status}\n" + ) + self._send_to_shell(truncate) + + # Escape command for eval — use single quotes with proper escaping + escaped = command.replace("'", "'\\''") + + # Send the IPC script + ipc_script = ( + f"cd {shlex.quote(work_dir)}\n" + f"eval '{escaped}' < /dev/null > {self._remote_stdout} 2> {self._remote_stderr}\n" + f"__EC=$?\n" + f"pwd > {self._remote_cwd}\n" + f"echo $__EC > {self._remote_status}\n" + ) + self._send_to_shell(ipc_script) + + # Poll the status file + deadline = time.monotonic() + timeout + poll_interval = 0.05 # 50ms + + while True: + if is_interrupted(): + self._kill_shell_children() + stdout = self._read_remote_file(self._remote_stdout) + stderr = self._read_remote_file(self._remote_stderr) + output = self._merge_output(stdout, stderr) + return { + "output": output + "\n[Command interrupted]", + "returncode": 130, + } + + if time.monotonic() > deadline: + self._kill_shell_children() + stdout = self._read_remote_file(self._remote_stdout) + stderr = self._read_remote_file(self._remote_stderr) + output = self._merge_output(stdout, stderr) + if output: + return { + "output": output + f"\n[Command timed out after {timeout}s]", + "returncode": 124, + } + return self._timeout_result(timeout) + + if not self._shell_alive: + return { + "output": "Persistent shell died during execution", + "returncode": 1, + } + + # Check if status file has content (command is done) + status_content = self._read_remote_file(self._remote_status).strip() + if status_content: + break + + time.sleep(poll_interval) + + # Read results + stdout = self._read_remote_file(self._remote_stdout) + stderr = self._read_remote_file(self._remote_stderr) + exit_code_str = status_content + new_cwd = self._read_remote_file(self._remote_cwd).strip() + + # Parse exit code + try: + exit_code = int(exit_code_str) + except ValueError: + exit_code = 1 + + # Update cwd + if new_cwd: + self.cwd = new_cwd + + output = self._merge_output(stdout, stderr) + return {"output": output, "returncode": exit_code} + + @staticmethod + def _merge_output(stdout: str, stderr: str) -> str: + """Combine stdout and stderr into a single output string.""" + parts = [] + if stdout.strip(): + parts.append(stdout.rstrip("\n")) + if stderr.strip(): + parts.append(stderr.rstrip("\n")) + return "\n".join(parts) + + # ------------------------------------------------------------------ + # One-shot execution (original behavior) + # ------------------------------------------------------------------ + + def _execute_oneshot(self, command: str, cwd: str = "", *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + """Execute a command via a fresh one-shot SSH invocation.""" work_dir = cwd or self.cwd exec_command, sudo_stdin = self._prepare_command(command) wrapped = f'cd {work_dir} && {exec_command}' @@ -141,7 +397,52 @@ class SSHEnvironment(BaseEnvironment): except Exception as e: return {"output": f"SSH execution error: {str(e)}", "returncode": 1} + # ------------------------------------------------------------------ + # Public interface + # ------------------------------------------------------------------ + + def execute(self, command: str, cwd: str = "", *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + if self.persistent: + return self._execute_persistent( + command, cwd, timeout=timeout, stdin_data=stdin_data + ) + return self._execute_oneshot( + command, cwd, timeout=timeout, stdin_data=stdin_data + ) + def cleanup(self): + # Persistent shell teardown + if self.persistent and self._shell_proc is not None: + # Remove remote temp files + if self._session_id: + try: + cmd = self._build_ssh_command() + cmd.append( + f"rm -f /tmp/hermes-ssh-{self._session_id}-*" + ) + subprocess.run(cmd, capture_output=True, timeout=5) + except (OSError, subprocess.SubprocessError): + pass + + # Close the shell + try: + self._shell_proc.stdin.close() + except Exception: + pass + try: + self._shell_proc.terminate() + self._shell_proc.wait(timeout=3) + except Exception: + try: + self._shell_proc.kill() + except Exception: + pass + self._shell_alive = False + self._shell_proc = None + + # ControlMaster cleanup if self.control_socket.exists(): try: cmd = ["ssh", "-o", f"ControlPath={self.control_socket}", diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index bf1d2b6b3..c7f72040a 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -503,6 +503,7 @@ def _get_env_config() -> Dict[str, Any]: "ssh_user": os.getenv("TERMINAL_SSH_USER", ""), "ssh_port": _parse_env_var("TERMINAL_SSH_PORT", "22"), "ssh_key": os.getenv("TERMINAL_SSH_KEY", ""), + "ssh_persistent": os.getenv("TERMINAL_SSH_PERSISTENT", "false").lower() in ("true", "1", "yes"), # Container resource config (applies to docker, singularity, modal, daytona -- ignored for local/ssh) "container_cpu": _parse_env_var("TERMINAL_CONTAINER_CPU", "1", float, "number"), "container_memory": _parse_env_var("TERMINAL_CONTAINER_MEMORY", "5120"), # MB (default 5GB) @@ -594,6 +595,7 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int, key_path=ssh_config.get("key", ""), cwd=cwd, timeout=timeout, + persistent=ssh_config.get("persistent", False), ) else: @@ -923,6 +925,7 @@ def terminal_tool( "user": config.get("ssh_user", ""), "port": config.get("ssh_port", 22), "key": config.get("ssh_key", ""), + "persistent": config.get("ssh_persistent", False), } container_config = None From 861202b56c453eee8b47db190ca5541dc8e85eb0 Mon Sep 17 00:00:00 2001 From: "balyan.sid@gmail.com" Date: Fri, 13 Mar 2026 16:54:11 +0530 Subject: [PATCH 2/7] wip: add persistent shell to ssh and local terminal backends --- tests/tools/test_local_persistent.py | 183 +++++++++++++++ tests/tools/test_ssh_environment.py | 198 ++++++++++++++++ tools/environments/local.py | 132 ++++++++--- tools/environments/persistent_shell.py | 308 +++++++++++++++++++++++++ tools/environments/ssh.py | 284 ++++------------------- tools/terminal_tool.py | 14 +- 6 files changed, 842 insertions(+), 277 deletions(-) create mode 100644 tests/tools/test_local_persistent.py create mode 100644 tests/tools/test_ssh_environment.py create mode 100644 tools/environments/persistent_shell.py diff --git a/tests/tools/test_local_persistent.py b/tests/tools/test_local_persistent.py new file mode 100644 index 000000000..9c1642a2c --- /dev/null +++ b/tests/tools/test_local_persistent.py @@ -0,0 +1,183 @@ +"""Tests for the local persistent shell backend. + +Unit tests cover config plumbing (no real shell needed). +Integration tests run real commands — no external dependencies required. + + pytest tests/tools/test_local_persistent.py -v +""" + +import glob as glob_mod + +import pytest + +from tools.environments.local import LocalEnvironment +from tools.environments.persistent_shell import PersistentShellMixin + + +# --------------------------------------------------------------------------- +# Unit tests — config plumbing +# --------------------------------------------------------------------------- + +class TestLocalConfig: + def test_local_persistent_default_false(self, monkeypatch): + monkeypatch.delenv("TERMINAL_LOCAL_PERSISTENT", raising=False) + from tools.terminal_tool import _get_env_config + assert _get_env_config()["local_persistent"] is False + + def test_local_persistent_true(self, monkeypatch): + monkeypatch.setenv("TERMINAL_LOCAL_PERSISTENT", "true") + from tools.terminal_tool import _get_env_config + assert _get_env_config()["local_persistent"] is True + + def test_local_persistent_yes(self, monkeypatch): + monkeypatch.setenv("TERMINAL_LOCAL_PERSISTENT", "yes") + from tools.terminal_tool import _get_env_config + assert _get_env_config()["local_persistent"] is True + + +class TestMergeOutput: + """Test the shared _merge_output static method.""" + + def test_stdout_only(self): + assert PersistentShellMixin._merge_output("out", "") == "out" + + def test_stderr_only(self): + assert PersistentShellMixin._merge_output("", "err") == "err" + + def test_both(self): + assert PersistentShellMixin._merge_output("out", "err") == "out\nerr" + + def test_empty(self): + assert PersistentShellMixin._merge_output("", "") == "" + + def test_strips_trailing_newlines(self): + assert PersistentShellMixin._merge_output("out\n\n", "err\n") == "out\nerr" + + +# --------------------------------------------------------------------------- +# One-shot regression tests — ensure refactor didn't break anything +# --------------------------------------------------------------------------- + +class TestLocalOneShotRegression: + """Verify one-shot mode still works after adding the mixin.""" + + def test_echo(self): + env = LocalEnvironment(persistent=False) + r = env.execute("echo hello") + assert r["returncode"] == 0 + assert "hello" in r["output"] + env.cleanup() + + def test_exit_code(self): + env = LocalEnvironment(persistent=False) + r = env.execute("exit 42") + assert r["returncode"] == 42 + env.cleanup() + + def test_state_does_not_persist(self): + """Env vars set in one command should NOT survive in one-shot mode.""" + env = LocalEnvironment(persistent=False) + env.execute("export HERMES_ONESHOT_LOCAL=yes") + r = env.execute("echo $HERMES_ONESHOT_LOCAL") + # In one-shot mode, env var should not persist + assert r["output"].strip() == "" + env.cleanup() + + +# --------------------------------------------------------------------------- +# Persistent shell integration tests +# --------------------------------------------------------------------------- + +class TestLocalPersistent: + """Persistent mode: state persists across execute() calls.""" + + @pytest.fixture + def env(self): + e = LocalEnvironment(persistent=True) + yield e + e.cleanup() + + def test_echo(self, env): + r = env.execute("echo hello-persistent") + assert r["returncode"] == 0 + assert "hello-persistent" in r["output"] + + def test_env_var_persists(self, env): + env.execute("export HERMES_LOCAL_PERSIST_TEST=works") + r = env.execute("echo $HERMES_LOCAL_PERSIST_TEST") + assert r["output"].strip() == "works" + + def test_cwd_persists(self, env): + env.execute("cd /tmp") + r = env.execute("pwd") + assert r["output"].strip() == "/tmp" + + def test_exit_code(self, env): + r = env.execute("(exit 42)") + assert r["returncode"] == 42 + + def test_stderr(self, env): + r = env.execute("echo oops >&2") + assert r["returncode"] == 0 + assert "oops" in r["output"] + + def test_multiline_output(self, env): + r = env.execute("echo a; echo b; echo c") + lines = r["output"].strip().splitlines() + assert lines == ["a", "b", "c"] + + def test_timeout_then_recovery(self, env): + r = env.execute("sleep 999", timeout=2) + assert r["returncode"] in (124, 130) # timeout or interrupted + # Shell should survive — next command works + r = env.execute("echo alive") + assert r["returncode"] == 0 + assert "alive" in r["output"] + + def test_large_output(self, env): + r = env.execute("seq 1 1000") + assert r["returncode"] == 0 + lines = r["output"].strip().splitlines() + assert len(lines) == 1000 + assert lines[0] == "1" + assert lines[-1] == "1000" + + def test_shell_variable_persists(self, env): + """Shell variables (not exported) should also persist.""" + env.execute("MY_LOCAL_VAR=hello123") + r = env.execute("echo $MY_LOCAL_VAR") + assert r["output"].strip() == "hello123" + + def test_cleanup_removes_temp_files(self, env): + env.execute("echo warmup") + prefix = env._temp_prefix + # Temp files should exist + assert len(glob_mod.glob(f"{prefix}-*")) > 0 + env.cleanup() + remaining = glob_mod.glob(f"{prefix}-*") + assert remaining == [] + + def test_state_does_not_leak_between_instances(self): + """Two separate persistent instances don't share state.""" + env1 = LocalEnvironment(persistent=True) + env2 = LocalEnvironment(persistent=True) + try: + env1.execute("export LEAK_TEST=from_env1") + r = env2.execute("echo $LEAK_TEST") + assert r["output"].strip() == "" + finally: + env1.cleanup() + env2.cleanup() + + def test_special_characters_in_command(self, env): + """Commands with quotes and special chars should work.""" + r = env.execute("echo 'hello world'") + assert r["output"].strip() == "hello world" + + def test_pipe_command(self, env): + r = env.execute("echo hello | tr 'h' 'H'") + assert r["output"].strip() == "Hello" + + def test_multiple_commands_semicolon(self, env): + r = env.execute("X=42; echo $X") + assert r["output"].strip() == "42" diff --git a/tests/tools/test_ssh_environment.py b/tests/tools/test_ssh_environment.py new file mode 100644 index 000000000..d10108c9b --- /dev/null +++ b/tests/tools/test_ssh_environment.py @@ -0,0 +1,198 @@ +"""Tests for the SSH remote execution environment backend. + +Unit tests (no SSH required) cover pure logic: command building, output merging, +config plumbing. + +Integration tests require a real SSH target. Set TERMINAL_SSH_HOST and +TERMINAL_SSH_USER to enable them. In CI, start an sshd container or enable +the localhost SSH service. + + TERMINAL_SSH_HOST=localhost TERMINAL_SSH_USER=$(whoami) \ + pytest tests/tools/test_ssh_environment.py -v +""" + +import json +import os +import subprocess +from unittest.mock import MagicMock + +import pytest + +from tools.environments.ssh import SSHEnvironment + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_SSH_HOST = os.getenv("TERMINAL_SSH_HOST", "") +_SSH_USER = os.getenv("TERMINAL_SSH_USER", "") +_SSH_PORT = int(os.getenv("TERMINAL_SSH_PORT", "22")) +_SSH_KEY = os.getenv("TERMINAL_SSH_KEY", "") + +_has_ssh = bool(_SSH_HOST and _SSH_USER) + +requires_ssh = pytest.mark.skipif( + not _has_ssh, + reason="TERMINAL_SSH_HOST / TERMINAL_SSH_USER not set", +) + + +def _run(command, task_id="ssh_test", **kwargs): + """Call terminal_tool like an LLM would, return parsed JSON.""" + from tools.terminal_tool import terminal_tool + return json.loads(terminal_tool(command, task_id=task_id, **kwargs)) + + +def _cleanup(task_id="ssh_test"): + from tools.terminal_tool import cleanup_vm + cleanup_vm(task_id) + + +# --------------------------------------------------------------------------- +# Unit tests — no SSH connection needed +# --------------------------------------------------------------------------- + +class TestBuildSSHCommand: + """Pure logic: verify the ssh command list is assembled correctly.""" + + @pytest.fixture(autouse=True) + def _mock_connection(self, monkeypatch): + monkeypatch.setattr("tools.environments.ssh.subprocess.run", + lambda *a, **k: subprocess.CompletedProcess([], 0)) + monkeypatch.setattr("tools.environments.ssh.subprocess.Popen", + lambda *a, **k: MagicMock(stdout=iter([]), + stderr=iter([]), + stdin=MagicMock())) + monkeypatch.setattr("tools.environments.ssh.time.sleep", lambda _: None) + + def test_base_flags(self): + env = SSHEnvironment(host="h", user="u") + cmd = " ".join(env._build_ssh_command()) + for flag in ("ControlMaster=auto", "ControlPersist=300", + "BatchMode=yes", "StrictHostKeyChecking=accept-new"): + assert flag in cmd + + def test_custom_port(self): + env = SSHEnvironment(host="h", user="u", port=2222) + cmd = env._build_ssh_command() + assert "-p" in cmd and "2222" in cmd + + def test_key_path(self): + env = SSHEnvironment(host="h", user="u", key_path="/k") + cmd = env._build_ssh_command() + assert "-i" in cmd and "/k" in cmd + + def test_user_host_suffix(self): + env = SSHEnvironment(host="h", user="u") + assert env._build_ssh_command()[-1] == "u@h" + + +class TestTerminalToolConfig: + def test_ssh_persistent_default_false(self, monkeypatch): + monkeypatch.delenv("TERMINAL_SSH_PERSISTENT", raising=False) + from tools.terminal_tool import _get_env_config + assert _get_env_config()["ssh_persistent"] is False + + def test_ssh_persistent_true(self, monkeypatch): + monkeypatch.setenv("TERMINAL_SSH_PERSISTENT", "true") + from tools.terminal_tool import _get_env_config + assert _get_env_config()["ssh_persistent"] is True + + +# --------------------------------------------------------------------------- +# Integration tests — real SSH, through terminal_tool() interface +# --------------------------------------------------------------------------- + +def _setup_ssh_env(monkeypatch, persistent: bool): + """Configure env vars for SSH integration tests.""" + monkeypatch.setenv("TERMINAL_ENV", "ssh") + monkeypatch.setenv("TERMINAL_SSH_HOST", _SSH_HOST) + monkeypatch.setenv("TERMINAL_SSH_USER", _SSH_USER) + monkeypatch.setenv("TERMINAL_SSH_PERSISTENT", "true" if persistent else "false") + if _SSH_PORT != 22: + monkeypatch.setenv("TERMINAL_SSH_PORT", str(_SSH_PORT)) + if _SSH_KEY: + monkeypatch.setenv("TERMINAL_SSH_KEY", _SSH_KEY) + + +@requires_ssh +class TestOneShotSSH: + """One-shot mode: each command is a fresh ssh invocation.""" + + @pytest.fixture(autouse=True) + def _setup(self, monkeypatch): + _setup_ssh_env(monkeypatch, persistent=False) + yield + _cleanup() + + def test_echo(self): + r = _run("echo hello") + assert r["exit_code"] == 0 + assert "hello" in r["output"] + + def test_exit_code(self): + r = _run("exit 42") + assert r["exit_code"] == 42 + + def test_state_does_not_persist(self): + """Env vars set in one command should NOT survive to the next.""" + _run("export HERMES_ONESHOT_TEST=yes") + r = _run("echo $HERMES_ONESHOT_TEST") + assert r["output"].strip() == "" + + +@requires_ssh +class TestPersistentSSH: + """Persistent mode: single long-lived shell, state persists.""" + + @pytest.fixture(autouse=True) + def _setup(self, monkeypatch): + _setup_ssh_env(monkeypatch, persistent=True) + yield + _cleanup() + + def test_echo(self): + r = _run("echo hello-persistent") + assert r["exit_code"] == 0 + assert "hello-persistent" in r["output"] + + def test_env_var_persists(self): + _run("export HERMES_PERSIST_TEST=works") + r = _run("echo $HERMES_PERSIST_TEST") + assert r["output"].strip() == "works" + + def test_cwd_persists(self): + _run("cd /tmp") + r = _run("pwd") + assert r["output"].strip() == "/tmp" + + def test_exit_code(self): + r = _run("(exit 42)") + assert r["exit_code"] == 42 + + def test_stderr(self): + r = _run("echo oops >&2") + assert r["exit_code"] == 0 + assert "oops" in r["output"] + + def test_multiline_output(self): + r = _run("echo a; echo b; echo c") + lines = r["output"].strip().splitlines() + assert lines == ["a", "b", "c"] + + def test_timeout_then_recovery(self): + r = _run("sleep 999", timeout=2) + assert r["exit_code"] == 124 + # Shell should survive — next command works + r = _run("echo alive") + assert r["exit_code"] == 0 + assert "alive" in r["output"] + + def test_large_output(self): + r = _run("seq 1 1000") + assert r["exit_code"] == 0 + lines = r["output"].strip().splitlines() + assert len(lines) == 1000 + assert lines[0] == "1" + assert lines[-1] == "1000" diff --git a/tools/environments/local.py b/tools/environments/local.py index 276ff9aca..a1d4686ec 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -11,6 +11,8 @@ import time _IS_WINDOWS = platform.system() == "Windows" from tools.environments.base import BaseEnvironment +from tools.environments.persistent_shell import PersistentShellMixin +from tools.interrupt import is_interrupted # Unique marker to isolate real command output from shell init/exit noise. # printf (no trailing newline) keeps the boundaries clean for splitting. @@ -162,6 +164,25 @@ def _clean_shell_noise(output: str) -> str: return result +_SANE_PATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + + +def _make_run_env(env: dict) -> dict: + """Build a run environment with a sane PATH and provider-var stripping.""" + merged = dict(os.environ | env) + run_env = {} + for k, v in merged.items(): + if k.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): + real_key = k[len(_HERMES_PROVIDER_ENV_FORCE_PREFIX):] + run_env[real_key] = v + elif k not in _HERMES_PROVIDER_ENV_BLOCKLIST: + run_env[k] = v + existing_path = run_env.get("PATH", "") + if "/usr/bin" not in existing_path.split(":"): + run_env["PATH"] = f"{existing_path}:{_SANE_PATH}" if existing_path else _SANE_PATH + return run_env + + def _extract_fenced_output(raw: str) -> str: """Extract real command output from between fence markers. @@ -186,7 +207,7 @@ def _extract_fenced_output(raw: str) -> str: return raw[start:last] -class LocalEnvironment(BaseEnvironment): +class LocalEnvironment(PersistentShellMixin, BaseEnvironment): """Run commands directly on the host machine. Features: @@ -195,24 +216,72 @@ class LocalEnvironment(BaseEnvironment): - stdin_data support for piping content (bypasses ARG_MAX limits) - sudo -S transform via SUDO_PASSWORD env var - Uses interactive login shell so full user env is available + - Optional persistent shell mode (cwd/env vars survive across calls) """ - def __init__(self, cwd: str = "", timeout: int = 60, env: dict = None): + def __init__(self, cwd: str = "", timeout: int = 60, env: dict = None, + persistent: bool = False): super().__init__(cwd=cwd or os.getcwd(), timeout=timeout, env=env) + self.persistent = persistent + if self.persistent: + self._init_persistent_shell() - def execute(self, command: str, cwd: str = "", *, - timeout: int | None = None, - stdin_data: str | None = None) -> dict: - from tools.terminal_tool import _interrupt_event + # ------------------------------------------------------------------ + # PersistentShellMixin: backend-specific implementations + # ------------------------------------------------------------------ + @property + def _temp_prefix(self) -> str: + return f"/tmp/hermes-local-{self._session_id}" + + def _spawn_shell_process(self) -> subprocess.Popen: + user_shell = _find_bash() + run_env = _make_run_env(self.env) + return subprocess.Popen( + [user_shell, "-l"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=run_env, + preexec_fn=None if _IS_WINDOWS else os.setsid, + ) + + def _read_temp_files(self, *paths: str) -> list[str]: + """Read local files directly.""" + results = [] + for path in paths: + try: + with open(path) as f: + results.append(f.read()) + except OSError: + results.append("") + return results + + def _kill_shell_children(self): + """Kill children of the persistent shell via pkill -P.""" + if self._shell_pid is None: + return + try: + subprocess.run( + ["pkill", "-P", str(self._shell_pid)], + capture_output=True, timeout=5, + ) + except (subprocess.TimeoutExpired, OSError, FileNotFoundError): + pass + + # ------------------------------------------------------------------ + # One-shot execution (original behavior) + # ------------------------------------------------------------------ + + def _execute_oneshot(self, command: str, cwd: str = "", *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: work_dir = cwd or self.cwd or os.getcwd() effective_timeout = timeout or self.timeout exec_command, sudo_stdin = self._prepare_command(command) # Merge the sudo password (if any) with caller-supplied stdin_data. - # sudo -S reads exactly one line (the password) then passes the rest - # of stdin to the child, so prepending is safe even when stdin_data - # is also present. if sudo_stdin is not None and stdin_data is not None: effective_stdin = sudo_stdin + stdin_data elif sudo_stdin is not None: @@ -221,13 +290,7 @@ class LocalEnvironment(BaseEnvironment): effective_stdin = stdin_data try: - # The fence wrapper uses bash syntax (semicolons, $?, printf). - # Always use bash for the wrapper — NOT $SHELL which could be - # fish, zsh, or another shell with incompatible syntax. - # The -lic flags source rc files so tools like nvm/pyenv work. user_shell = _find_bash() - # Wrap with output fences so we can later extract the real - # command output and discard shell init/exit noise. fenced_cmd = ( f"printf '{_OUTPUT_FENCE}';" f" {exec_command};" @@ -235,24 +298,7 @@ class LocalEnvironment(BaseEnvironment): f" printf '{_OUTPUT_FENCE}';" f" exit $__hermes_rc" ) - # Ensure PATH always includes standard dirs — systemd services - # and some terminal multiplexers inherit a minimal PATH. - _SANE_PATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" - # Strip Hermes-internal provider vars so external CLIs - # (e.g. codex) are not silently misrouted. Callers that - # truly need a blocked var can opt in by prefixing the key - # with _HERMES_FORCE_ in self.env (e.g. _HERMES_FORCE_OPENAI_API_KEY). - merged = dict(os.environ | self.env) - run_env = {} - for k, v in merged.items(): - if k.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): - real_key = k[len(_HERMES_PROVIDER_ENV_FORCE_PREFIX):] - run_env[real_key] = v - elif k not in _HERMES_PROVIDER_ENV_BLOCKLIST: - run_env[k] = v - existing_path = run_env.get("PATH", "") - if "/usr/bin" not in existing_path.split(":"): - run_env["PATH"] = f"{existing_path}:{_SANE_PATH}" if existing_path else _SANE_PATH + run_env = _make_run_env(self.env) proc = subprocess.Popen( [user_shell, "-lic", fenced_cmd], @@ -295,7 +341,7 @@ class LocalEnvironment(BaseEnvironment): deadline = time.monotonic() + effective_timeout while proc.poll() is None: - if _interrupt_event.is_set(): + if is_interrupted(): try: if _IS_WINDOWS: proc.terminate() @@ -332,5 +378,21 @@ class LocalEnvironment(BaseEnvironment): except Exception as e: return {"output": f"Execution error: {str(e)}", "returncode": 1} + # ------------------------------------------------------------------ + # Public interface + # ------------------------------------------------------------------ + + def execute(self, command: str, cwd: str = "", *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + if self.persistent: + return self._execute_persistent( + command, cwd, timeout=timeout, stdin_data=stdin_data, + ) + return self._execute_oneshot( + command, cwd, timeout=timeout, stdin_data=stdin_data, + ) + def cleanup(self): - pass + if self.persistent: + self._cleanup_persistent_shell() diff --git a/tools/environments/persistent_shell.py b/tools/environments/persistent_shell.py new file mode 100644 index 000000000..f0bd438f0 --- /dev/null +++ b/tools/environments/persistent_shell.py @@ -0,0 +1,308 @@ +"""Persistent shell mixin: file-based IPC protocol for long-lived bash shells. + +Provides the shared logic for maintaining a persistent bash shell across +execute() calls. Backend-specific operations (spawning the shell, reading +temp files, killing child processes) are implemented by subclasses via +abstract methods. + +The IPC protocol writes each command's stdout/stderr/exit-code/cwd to temp +files, then polls the status file for completion. A daemon thread drains +the shell's stdout to prevent pipe deadlock and detect shell death. +""" + +import glob as glob_mod +import logging +import os +import shlex +import subprocess +import threading +import time +import uuid +from abc import abstractmethod + +from tools.interrupt import is_interrupted + +logger = logging.getLogger(__name__) + + +class PersistentShellMixin: + """Mixin that adds persistent shell capability to any BaseEnvironment. + + Subclasses MUST implement: + _spawn_shell_process() -> subprocess.Popen + _read_temp_files(*paths) -> list[str] + _kill_shell_children() + + Subclasses MUST also provide ``_execute_oneshot()`` for the stdin_data + fallback path (commands with piped stdin cannot use the persistent shell). + """ + + # -- State (initialized by _init_persistent_shell) --------------------- + _shell_proc: subprocess.Popen | None = None + _shell_alive: bool = False + _shell_pid: int | None = None + _session_id: str = "" + + # -- Abstract methods (backend-specific) ------------------------------- + + @abstractmethod + def _spawn_shell_process(self) -> subprocess.Popen: + """Spawn a long-lived bash shell and return the Popen handle. + + Must use ``stdin=PIPE, stdout=PIPE, stderr=PIPE, text=True``. + """ + ... + + @abstractmethod + def _read_temp_files(self, *paths: str) -> list[str]: + """Read temp files from the execution context. + + Returns contents in the same order as *paths*. Falls back to + empty strings on failure. + """ + ... + + @abstractmethod + def _kill_shell_children(self): + """Kill the running command's processes but keep the shell alive.""" + ... + + # -- Overridable properties -------------------------------------------- + + @property + def _temp_prefix(self) -> str: + """Base path for temp files. Override per backend.""" + return f"/tmp/hermes-persistent-{self._session_id}" + + # -- Shared implementation --------------------------------------------- + + def _init_persistent_shell(self): + """Call from ``__init__`` when ``persistent=True``.""" + self._shell_lock = threading.Lock() + self._session_id = "" + self._shell_proc = None + self._shell_alive = False + self._shell_pid = None + self._start_persistent_shell() + + def _start_persistent_shell(self): + """Spawn the shell, create temp files, capture PID.""" + self._session_id = uuid.uuid4().hex[:12] + p = self._temp_prefix + self._pshell_stdout = f"{p}-stdout" + self._pshell_stderr = f"{p}-stderr" + self._pshell_status = f"{p}-status" + self._pshell_cwd = f"{p}-cwd" + self._pshell_pid_file = f"{p}-pid" + + self._shell_proc = self._spawn_shell_process() + self._shell_alive = True + + self._drain_thread = threading.Thread( + target=self._drain_shell_output, daemon=True, + ) + self._drain_thread.start() + + # Initialize temp files and capture shell PID + init_script = ( + f"touch {self._pshell_stdout} {self._pshell_stderr} " + f"{self._pshell_status} {self._pshell_cwd} {self._pshell_pid_file}\n" + f"echo $$ > {self._pshell_pid_file}\n" + f"pwd > {self._pshell_cwd}\n" + ) + self._send_to_shell(init_script) + + # Poll for PID file + deadline = time.monotonic() + 3.0 + while time.monotonic() < deadline: + pid_str = self._read_temp_files(self._pshell_pid_file)[0].strip() + if pid_str.isdigit(): + self._shell_pid = int(pid_str) + break + time.sleep(0.05) + else: + logger.warning("Could not read persistent shell PID") + self._shell_pid = None + + if self._shell_pid: + logger.info( + "Persistent shell started (session=%s, pid=%d)", + self._session_id, self._shell_pid, + ) + + # Update cwd from what the shell reports + reported_cwd = self._read_temp_files(self._pshell_cwd)[0].strip() + if reported_cwd: + self.cwd = reported_cwd + + def _drain_shell_output(self): + """Drain stdout to prevent pipe deadlock; detect shell death.""" + try: + for _ in self._shell_proc.stdout: + pass # Real output goes to temp files + except Exception: + pass + self._shell_alive = False + + def _send_to_shell(self, text: str): + """Write text to the persistent shell's stdin.""" + if not self._shell_alive or self._shell_proc is None: + return + try: + self._shell_proc.stdin.write(text) + self._shell_proc.stdin.flush() + except (BrokenPipeError, OSError): + self._shell_alive = False + + def _read_persistent_output(self) -> tuple[str, int, str]: + """Read stdout, stderr, status, cwd. Returns (output, exit_code, cwd).""" + stdout, stderr, status_raw, cwd = self._read_temp_files( + self._pshell_stdout, self._pshell_stderr, + self._pshell_status, self._pshell_cwd, + ) + output = self._merge_output(stdout, stderr) + # Status format: "cmd_id:exit_code" — strip the ID prefix + status = status_raw.strip() + if ":" in status: + status = status.split(":", 1)[1] + try: + exit_code = int(status.strip()) + except ValueError: + exit_code = 1 + return output, exit_code, cwd.strip() + + def _execute_persistent(self, command: str, cwd: str, *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + """Execute a command in the persistent shell.""" + if not self._shell_alive: + logger.info("Persistent shell died, restarting...") + self._start_persistent_shell() + + exec_command, sudo_stdin = self._prepare_command(command) + effective_timeout = timeout or self.timeout + + # Fall back to one-shot for commands needing piped stdin + if stdin_data or sudo_stdin: + return self._execute_oneshot( + command, cwd, timeout=timeout, stdin_data=stdin_data, + ) + + with self._shell_lock: + return self._execute_persistent_locked( + exec_command, cwd, effective_timeout, + ) + + def _execute_persistent_locked(self, command: str, cwd: str, + timeout: int) -> dict: + """Inner persistent execution — caller must hold ``_shell_lock``.""" + work_dir = cwd or self.cwd + + # Each command gets a unique ID written into the status file so the + # poll loop can distinguish the *current* command's result from a + # stale value left over from the previous command. This eliminates + # the race where a fast local file read sees the old status before + # the shell has processed the truncation. + cmd_id = uuid.uuid4().hex[:8] + + # Truncate temp files + truncate = ( + f": > {self._pshell_stdout}\n" + f": > {self._pshell_stderr}\n" + f": > {self._pshell_status}\n" + ) + self._send_to_shell(truncate) + + # Escape command for eval + escaped = command.replace("'", "'\\''") + + ipc_script = ( + f"cd {shlex.quote(work_dir)}\n" + f"eval '{escaped}' < /dev/null > {self._pshell_stdout} 2> {self._pshell_stderr}\n" + f"__EC=$?\n" + f"pwd > {self._pshell_cwd}\n" + f"echo {cmd_id}:$__EC > {self._pshell_status}\n" + ) + self._send_to_shell(ipc_script) + + # Poll the status file for current command's ID + deadline = time.monotonic() + timeout + poll_interval = 0.15 + + while True: + if is_interrupted(): + self._kill_shell_children() + output, _, _ = self._read_persistent_output() + return { + "output": output + "\n[Command interrupted]", + "returncode": 130, + } + + if time.monotonic() > deadline: + self._kill_shell_children() + output, _, _ = self._read_persistent_output() + if output: + return { + "output": output + f"\n[Command timed out after {timeout}s]", + "returncode": 124, + } + return self._timeout_result(timeout) + + if not self._shell_alive: + return { + "output": "Persistent shell died during execution", + "returncode": 1, + } + + status_content = self._read_temp_files(self._pshell_status)[0].strip() + if status_content.startswith(cmd_id + ":"): + break + + time.sleep(poll_interval) + + output, exit_code, new_cwd = self._read_persistent_output() + if new_cwd: + self.cwd = new_cwd + return {"output": output, "returncode": exit_code} + + @staticmethod + def _merge_output(stdout: str, stderr: str) -> str: + """Combine stdout and stderr into a single output string.""" + parts = [] + if stdout.strip(): + parts.append(stdout.rstrip("\n")) + if stderr.strip(): + parts.append(stderr.rstrip("\n")) + return "\n".join(parts) + + def _cleanup_persistent_shell(self): + """Clean up persistent shell resources. Call from ``cleanup()``.""" + if self._shell_proc is None: + return + + if self._session_id: + self._cleanup_temp_files() + + try: + self._shell_proc.stdin.close() + except Exception: + pass + try: + self._shell_proc.terminate() + self._shell_proc.wait(timeout=3) + except subprocess.TimeoutExpired: + self._shell_proc.kill() + + self._shell_alive = False + self._shell_proc = None + + if hasattr(self, "_drain_thread") and self._drain_thread.is_alive(): + self._drain_thread.join(timeout=1.0) + + def _cleanup_temp_files(self): + """Remove local temp files. Override for remote backends (SSH, Docker).""" + for f in glob_mod.glob(f"{self._temp_prefix}-*"): + try: + os.remove(f) + except OSError: + pass diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 7a31006db..13893dedd 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -1,21 +1,20 @@ """SSH remote execution environment with ControlMaster connection persistence.""" import logging -import shlex import subprocess import tempfile import threading import time -import uuid from pathlib import Path from tools.environments.base import BaseEnvironment +from tools.environments.persistent_shell import PersistentShellMixin from tools.interrupt import is_interrupted logger = logging.getLogger(__name__) -class SSHEnvironment(BaseEnvironment): +class SSHEnvironment(PersistentShellMixin, BaseEnvironment): """Run commands on a remote machine over SSH. Uses SSH ControlMaster for connection persistence so subsequent @@ -47,22 +46,10 @@ class SSHEnvironment(BaseEnvironment): self.control_socket = self.control_dir / f"{user}@{host}:{port}.sock" self._establish_connection() - # Persistent shell state - self._shell_proc: subprocess.Popen | None = None - self._shell_lock = threading.Lock() - self._shell_alive = False - self._session_id: str = "" - self._remote_stdout: str = "" - self._remote_stderr: str = "" - self._remote_status: str = "" - self._remote_cwd: str = "" - self._remote_pid: str = "" - self._remote_shell_pid: int | None = None - if self.persistent: - self._start_persistent_shell() + self._init_persistent_shell() - def _build_ssh_command(self, extra_args: list = None) -> list: + def _build_ssh_command(self, extra_args: list | None = None) -> list: cmd = ["ssh"] cmd.extend(["-o", f"ControlPath={self.control_socket}"]) cmd.extend(["-o", "ControlMaster=auto"]) @@ -91,230 +78,70 @@ class SSHEnvironment(BaseEnvironment): raise RuntimeError(f"SSH connection to {self.user}@{self.host} timed out") # ------------------------------------------------------------------ - # Persistent shell management + # PersistentShellMixin: backend-specific implementations # ------------------------------------------------------------------ - def _start_persistent_shell(self): - """Spawn a long-lived bash shell over SSH.""" - self._session_id = uuid.uuid4().hex[:12] - prefix = f"/tmp/hermes-ssh-{self._session_id}" - self._remote_stdout = f"{prefix}-stdout" - self._remote_stderr = f"{prefix}-stderr" - self._remote_status = f"{prefix}-status" - self._remote_cwd = f"{prefix}-cwd" - self._remote_pid = f"{prefix}-pid" + @property + def _temp_prefix(self) -> str: + return f"/tmp/hermes-ssh-{self._session_id}" + def _spawn_shell_process(self) -> subprocess.Popen: cmd = self._build_ssh_command() cmd.append("bash -l") - - self._shell_proc = subprocess.Popen( + return subprocess.Popen( cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, ) - self._shell_alive = True - # Start daemon thread to drain stdout/stderr and detect shell death - self._drain_thread = threading.Thread( - target=self._drain_shell_output, daemon=True + def _read_temp_files(self, *paths: str) -> list[str]: + """Read remote files via ControlMaster one-shot SSH calls.""" + if len(paths) == 1: + cmd = self._build_ssh_command() + cmd.append(f"cat {paths[0]} 2>/dev/null") + try: + result = subprocess.run( + cmd, capture_output=True, text=True, timeout=10, + ) + return [result.stdout] + except (subprocess.TimeoutExpired, OSError): + return [""] + + delim = f"__HERMES_SEP_{self._session_id}__" + script = "; ".join( + f"cat {p} 2>/dev/null; echo '{delim}'" for p in paths ) - self._drain_thread.start() - - # Initialize remote temp files and capture shell PID - init_script = ( - f"touch {self._remote_stdout} {self._remote_stderr} " - f"{self._remote_status} {self._remote_cwd} {self._remote_pid}\n" - f"echo $$ > {self._remote_pid}\n" - f"pwd > {self._remote_cwd}\n" - ) - self._send_to_shell(init_script) - - # Give shell time to initialize and write PID file - time.sleep(0.3) - - # Read the remote shell PID - pid_str = self._read_remote_file(self._remote_pid).strip() - if pid_str.isdigit(): - self._remote_shell_pid = int(pid_str) - logger.info("Persistent shell started (session=%s, pid=%d)", - self._session_id, self._remote_shell_pid) - else: - logger.warning("Could not read persistent shell PID (got %r)", pid_str) - self._remote_shell_pid = None - - # Update cwd from what the shell reports - remote_cwd = self._read_remote_file(self._remote_cwd).strip() - if remote_cwd: - self.cwd = remote_cwd - - def _drain_shell_output(self): - """Drain the shell's stdout/stderr to prevent pipe deadlock. - - Also detects when the shell process dies. - """ - try: - for _ in self._shell_proc.stdout: - pass # Discard — real output goes to temp files - except Exception: - pass - self._shell_alive = False - - def _send_to_shell(self, text: str): - """Write text to the persistent shell's stdin.""" - if not self._shell_alive or self._shell_proc is None: - return - try: - self._shell_proc.stdin.write(text) - self._shell_proc.stdin.flush() - except (BrokenPipeError, OSError): - self._shell_alive = False - - def _read_remote_file(self, path: str) -> str: - """Read a file on the remote host via a one-shot SSH command. - - Uses ControlMaster so this is very fast (~5ms on LAN). - """ cmd = self._build_ssh_command() - cmd.append(f"cat {path} 2>/dev/null") + cmd.append(script) try: result = subprocess.run( - cmd, capture_output=True, text=True, timeout=10 + cmd, capture_output=True, text=True, timeout=10, ) - return result.stdout + parts = result.stdout.split(delim + "\n") + return [parts[i] if i < len(parts) else "" for i in range(len(paths))] except (subprocess.TimeoutExpired, OSError): - return "" + return [""] * len(paths) def _kill_shell_children(self): - """Kill children of the persistent shell (the running command), - but not the shell itself.""" - if self._remote_shell_pid is None: + if self._shell_pid is None: return cmd = self._build_ssh_command() - cmd.append(f"pkill -P {self._remote_shell_pid} 2>/dev/null; true") + cmd.append(f"pkill -P {self._shell_pid} 2>/dev/null; true") try: subprocess.run(cmd, capture_output=True, timeout=5) except (subprocess.TimeoutExpired, OSError): pass - def _execute_persistent(self, command: str, cwd: str, *, - timeout: int | None = None, - stdin_data: str | None = None) -> dict: - """Execute a command in the persistent shell.""" - # If shell is dead, restart it - if not self._shell_alive: - logger.info("Persistent shell died, restarting...") - self._start_persistent_shell() - - exec_command, sudo_stdin = self._prepare_command(command) - effective_timeout = timeout or self.timeout - - # Fall back to one-shot for commands needing piped stdin - if stdin_data or sudo_stdin: - return self._execute_oneshot( - command, cwd, timeout=timeout, stdin_data=stdin_data - ) - - with self._shell_lock: - return self._execute_persistent_locked( - exec_command, cwd, effective_timeout - ) - - def _execute_persistent_locked(self, command: str, cwd: str, - timeout: int) -> dict: - """Inner persistent execution — caller must hold _shell_lock.""" - work_dir = cwd or self.cwd - - # Truncate temp files - truncate = ( - f": > {self._remote_stdout}\n" - f": > {self._remote_stderr}\n" - f": > {self._remote_status}\n" - ) - self._send_to_shell(truncate) - - # Escape command for eval — use single quotes with proper escaping - escaped = command.replace("'", "'\\''") - - # Send the IPC script - ipc_script = ( - f"cd {shlex.quote(work_dir)}\n" - f"eval '{escaped}' < /dev/null > {self._remote_stdout} 2> {self._remote_stderr}\n" - f"__EC=$?\n" - f"pwd > {self._remote_cwd}\n" - f"echo $__EC > {self._remote_status}\n" - ) - self._send_to_shell(ipc_script) - - # Poll the status file - deadline = time.monotonic() + timeout - poll_interval = 0.05 # 50ms - - while True: - if is_interrupted(): - self._kill_shell_children() - stdout = self._read_remote_file(self._remote_stdout) - stderr = self._read_remote_file(self._remote_stderr) - output = self._merge_output(stdout, stderr) - return { - "output": output + "\n[Command interrupted]", - "returncode": 130, - } - - if time.monotonic() > deadline: - self._kill_shell_children() - stdout = self._read_remote_file(self._remote_stdout) - stderr = self._read_remote_file(self._remote_stderr) - output = self._merge_output(stdout, stderr) - if output: - return { - "output": output + f"\n[Command timed out after {timeout}s]", - "returncode": 124, - } - return self._timeout_result(timeout) - - if not self._shell_alive: - return { - "output": "Persistent shell died during execution", - "returncode": 1, - } - - # Check if status file has content (command is done) - status_content = self._read_remote_file(self._remote_status).strip() - if status_content: - break - - time.sleep(poll_interval) - - # Read results - stdout = self._read_remote_file(self._remote_stdout) - stderr = self._read_remote_file(self._remote_stderr) - exit_code_str = status_content - new_cwd = self._read_remote_file(self._remote_cwd).strip() - - # Parse exit code + def _cleanup_temp_files(self): + """Remove remote temp files via SSH.""" try: - exit_code = int(exit_code_str) - except ValueError: - exit_code = 1 - - # Update cwd - if new_cwd: - self.cwd = new_cwd - - output = self._merge_output(stdout, stderr) - return {"output": output, "returncode": exit_code} - - @staticmethod - def _merge_output(stdout: str, stderr: str) -> str: - """Combine stdout and stderr into a single output string.""" - parts = [] - if stdout.strip(): - parts.append(stdout.rstrip("\n")) - if stderr.strip(): - parts.append(stderr.rstrip("\n")) - return "\n".join(parts) + cmd = self._build_ssh_command() + cmd.append(f"rm -f {self._temp_prefix}-*") + subprocess.run(cmd, capture_output=True, timeout=5) + except (OSError, subprocess.SubprocessError): + pass # ------------------------------------------------------------------ # One-shot execution (original behavior) @@ -413,34 +240,9 @@ class SSHEnvironment(BaseEnvironment): ) def cleanup(self): - # Persistent shell teardown - if self.persistent and self._shell_proc is not None: - # Remove remote temp files - if self._session_id: - try: - cmd = self._build_ssh_command() - cmd.append( - f"rm -f /tmp/hermes-ssh-{self._session_id}-*" - ) - subprocess.run(cmd, capture_output=True, timeout=5) - except (OSError, subprocess.SubprocessError): - pass - - # Close the shell - try: - self._shell_proc.stdin.close() - except Exception: - pass - try: - self._shell_proc.terminate() - self._shell_proc.wait(timeout=3) - except Exception: - try: - self._shell_proc.kill() - except Exception: - pass - self._shell_alive = False - self._shell_proc = None + # Persistent shell teardown (via mixin) + if self.persistent: + self._cleanup_persistent_shell() # ControlMaster cleanup if self.control_socket.exists(): diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index c7f72040a..b273ec028 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -504,6 +504,8 @@ def _get_env_config() -> Dict[str, Any]: "ssh_port": _parse_env_var("TERMINAL_SSH_PORT", "22"), "ssh_key": os.getenv("TERMINAL_SSH_KEY", ""), "ssh_persistent": os.getenv("TERMINAL_SSH_PERSISTENT", "false").lower() in ("true", "1", "yes"), + # Local persistent shell (cwd/env vars survive across calls) + "local_persistent": os.getenv("TERMINAL_LOCAL_PERSISTENT", "false").lower() in ("true", "1", "yes"), # Container resource config (applies to docker, singularity, modal, daytona -- ignored for local/ssh) "container_cpu": _parse_env_var("TERMINAL_CONTAINER_CPU", "1", float, "number"), "container_memory": _parse_env_var("TERMINAL_CONTAINER_MEMORY", "5120"), # MB (default 5GB) @@ -515,6 +517,7 @@ def _get_env_config() -> Dict[str, Any]: def _create_environment(env_type: str, image: str, cwd: str, timeout: int, ssh_config: dict = None, container_config: dict = None, + local_config: dict = None, task_id: str = "default"): """ Create an execution environment from mini-swe-agent. @@ -539,7 +542,9 @@ def _create_environment(env_type: str, image: str, cwd: str, timeout: int, volumes = cc.get("docker_volumes", []) if env_type == "local": - return _LocalEnvironment(cwd=cwd, timeout=timeout) + lc = local_config or {} + return _LocalEnvironment(cwd=cwd, timeout=timeout, + persistent=lc.get("persistent", False)) elif env_type == "docker": return _DockerEnvironment( @@ -938,6 +943,12 @@ def terminal_tool( "docker_volumes": config.get("docker_volumes", []), } + local_config = None + if env_type == "local": + local_config = { + "persistent": config.get("local_persistent", False), + } + new_env = _create_environment( env_type=env_type, image=image, @@ -945,6 +956,7 @@ def terminal_tool( timeout=effective_timeout, ssh_config=ssh_config, container_config=container_config, + local_config=local_config, task_id=effective_task_id, ) except ImportError as e: From 9001b34146e80bcf39728d64b3b28ea085f0b210 Mon Sep 17 00:00:00 2001 From: "balyan.sid@gmail.com" Date: Sun, 15 Mar 2026 01:12:16 +0530 Subject: [PATCH 3/7] simplify docstrings, fix some bugs --- tests/tools/test_local_persistent.py | 35 +------- tests/tools/test_ssh_environment.py | 33 +------ tools/environments/local.py | 23 ++--- tools/environments/persistent_shell.py | 117 ++++++------------------- tools/environments/ssh.py | 24 +---- tools/terminal_tool.py | 1 - 6 files changed, 37 insertions(+), 196 deletions(-) diff --git a/tests/tools/test_local_persistent.py b/tests/tools/test_local_persistent.py index 9c1642a2c..b20cca5be 100644 --- a/tests/tools/test_local_persistent.py +++ b/tests/tools/test_local_persistent.py @@ -1,10 +1,4 @@ -"""Tests for the local persistent shell backend. - -Unit tests cover config plumbing (no real shell needed). -Integration tests run real commands — no external dependencies required. - - pytest tests/tools/test_local_persistent.py -v -""" +"""Tests for the local persistent shell backend.""" import glob as glob_mod @@ -14,10 +8,6 @@ from tools.environments.local import LocalEnvironment from tools.environments.persistent_shell import PersistentShellMixin -# --------------------------------------------------------------------------- -# Unit tests — config plumbing -# --------------------------------------------------------------------------- - class TestLocalConfig: def test_local_persistent_default_false(self, monkeypatch): monkeypatch.delenv("TERMINAL_LOCAL_PERSISTENT", raising=False) @@ -36,8 +26,6 @@ class TestLocalConfig: class TestMergeOutput: - """Test the shared _merge_output static method.""" - def test_stdout_only(self): assert PersistentShellMixin._merge_output("out", "") == "out" @@ -54,13 +42,7 @@ class TestMergeOutput: assert PersistentShellMixin._merge_output("out\n\n", "err\n") == "out\nerr" -# --------------------------------------------------------------------------- -# One-shot regression tests — ensure refactor didn't break anything -# --------------------------------------------------------------------------- - class TestLocalOneShotRegression: - """Verify one-shot mode still works after adding the mixin.""" - def test_echo(self): env = LocalEnvironment(persistent=False) r = env.execute("echo hello") @@ -75,22 +57,14 @@ class TestLocalOneShotRegression: env.cleanup() def test_state_does_not_persist(self): - """Env vars set in one command should NOT survive in one-shot mode.""" env = LocalEnvironment(persistent=False) env.execute("export HERMES_ONESHOT_LOCAL=yes") r = env.execute("echo $HERMES_ONESHOT_LOCAL") - # In one-shot mode, env var should not persist assert r["output"].strip() == "" env.cleanup() -# --------------------------------------------------------------------------- -# Persistent shell integration tests -# --------------------------------------------------------------------------- - class TestLocalPersistent: - """Persistent mode: state persists across execute() calls.""" - @pytest.fixture def env(self): e = LocalEnvironment(persistent=True) @@ -128,8 +102,7 @@ class TestLocalPersistent: def test_timeout_then_recovery(self, env): r = env.execute("sleep 999", timeout=2) - assert r["returncode"] in (124, 130) # timeout or interrupted - # Shell should survive — next command works + assert r["returncode"] in (124, 130) r = env.execute("echo alive") assert r["returncode"] == 0 assert "alive" in r["output"] @@ -143,7 +116,6 @@ class TestLocalPersistent: assert lines[-1] == "1000" def test_shell_variable_persists(self, env): - """Shell variables (not exported) should also persist.""" env.execute("MY_LOCAL_VAR=hello123") r = env.execute("echo $MY_LOCAL_VAR") assert r["output"].strip() == "hello123" @@ -151,14 +123,12 @@ class TestLocalPersistent: def test_cleanup_removes_temp_files(self, env): env.execute("echo warmup") prefix = env._temp_prefix - # Temp files should exist assert len(glob_mod.glob(f"{prefix}-*")) > 0 env.cleanup() remaining = glob_mod.glob(f"{prefix}-*") assert remaining == [] def test_state_does_not_leak_between_instances(self): - """Two separate persistent instances don't share state.""" env1 = LocalEnvironment(persistent=True) env2 = LocalEnvironment(persistent=True) try: @@ -170,7 +140,6 @@ class TestLocalPersistent: env2.cleanup() def test_special_characters_in_command(self, env): - """Commands with quotes and special chars should work.""" r = env.execute("echo 'hello world'") assert r["output"].strip() == "hello world" diff --git a/tests/tools/test_ssh_environment.py b/tests/tools/test_ssh_environment.py index d10108c9b..65469e5f5 100644 --- a/tests/tools/test_ssh_environment.py +++ b/tests/tools/test_ssh_environment.py @@ -1,15 +1,4 @@ -"""Tests for the SSH remote execution environment backend. - -Unit tests (no SSH required) cover pure logic: command building, output merging, -config plumbing. - -Integration tests require a real SSH target. Set TERMINAL_SSH_HOST and -TERMINAL_SSH_USER to enable them. In CI, start an sshd container or enable -the localhost SSH service. - - TERMINAL_SSH_HOST=localhost TERMINAL_SSH_USER=$(whoami) \ - pytest tests/tools/test_ssh_environment.py -v -""" +"""Tests for the SSH remote execution environment backend.""" import json import os @@ -20,11 +9,6 @@ import pytest from tools.environments.ssh import SSHEnvironment - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - _SSH_HOST = os.getenv("TERMINAL_SSH_HOST", "") _SSH_USER = os.getenv("TERMINAL_SSH_USER", "") _SSH_PORT = int(os.getenv("TERMINAL_SSH_PORT", "22")) @@ -39,7 +23,6 @@ requires_ssh = pytest.mark.skipif( def _run(command, task_id="ssh_test", **kwargs): - """Call terminal_tool like an LLM would, return parsed JSON.""" from tools.terminal_tool import terminal_tool return json.loads(terminal_tool(command, task_id=task_id, **kwargs)) @@ -49,12 +32,7 @@ def _cleanup(task_id="ssh_test"): cleanup_vm(task_id) -# --------------------------------------------------------------------------- -# Unit tests — no SSH connection needed -# --------------------------------------------------------------------------- - class TestBuildSSHCommand: - """Pure logic: verify the ssh command list is assembled correctly.""" @pytest.fixture(autouse=True) def _mock_connection(self, monkeypatch): @@ -100,12 +78,7 @@ class TestTerminalToolConfig: assert _get_env_config()["ssh_persistent"] is True -# --------------------------------------------------------------------------- -# Integration tests — real SSH, through terminal_tool() interface -# --------------------------------------------------------------------------- - def _setup_ssh_env(monkeypatch, persistent: bool): - """Configure env vars for SSH integration tests.""" monkeypatch.setenv("TERMINAL_ENV", "ssh") monkeypatch.setenv("TERMINAL_SSH_HOST", _SSH_HOST) monkeypatch.setenv("TERMINAL_SSH_USER", _SSH_USER) @@ -118,7 +91,6 @@ def _setup_ssh_env(monkeypatch, persistent: bool): @requires_ssh class TestOneShotSSH: - """One-shot mode: each command is a fresh ssh invocation.""" @pytest.fixture(autouse=True) def _setup(self, monkeypatch): @@ -136,7 +108,6 @@ class TestOneShotSSH: assert r["exit_code"] == 42 def test_state_does_not_persist(self): - """Env vars set in one command should NOT survive to the next.""" _run("export HERMES_ONESHOT_TEST=yes") r = _run("echo $HERMES_ONESHOT_TEST") assert r["output"].strip() == "" @@ -144,7 +115,6 @@ class TestOneShotSSH: @requires_ssh class TestPersistentSSH: - """Persistent mode: single long-lived shell, state persists.""" @pytest.fixture(autouse=True) def _setup(self, monkeypatch): @@ -184,7 +154,6 @@ class TestPersistentSSH: def test_timeout_then_recovery(self): r = _run("sleep 999", timeout=2) assert r["exit_code"] == 124 - # Shell should survive — next command works r = _run("echo alive") assert r["exit_code"] == 0 assert "alive" in r["output"] diff --git a/tools/environments/local.py b/tools/environments/local.py index a1d4686ec..1641d12a4 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1,5 +1,6 @@ """Local execution environment with interrupt support and non-blocking I/O.""" +import glob import os import platform import shutil @@ -226,10 +227,6 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): if self.persistent: self._init_persistent_shell() - # ------------------------------------------------------------------ - # PersistentShellMixin: backend-specific implementations - # ------------------------------------------------------------------ - @property def _temp_prefix(self) -> str: return f"/tmp/hermes-local-{self._session_id}" @@ -241,14 +238,13 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): [user_shell, "-l"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stderr=subprocess.DEVNULL, text=True, env=run_env, preexec_fn=None if _IS_WINDOWS else os.setsid, ) def _read_temp_files(self, *paths: str) -> list[str]: - """Read local files directly.""" results = [] for path in paths: try: @@ -259,7 +255,6 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): return results def _kill_shell_children(self): - """Kill children of the persistent shell via pkill -P.""" if self._shell_pid is None: return try: @@ -270,9 +265,12 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): except (subprocess.TimeoutExpired, OSError, FileNotFoundError): pass - # ------------------------------------------------------------------ - # One-shot execution (original behavior) - # ------------------------------------------------------------------ + def _cleanup_temp_files(self): + for f in glob.glob(f"{self._temp_prefix}-*"): + try: + os.remove(f) + except OSError: + pass def _execute_oneshot(self, command: str, cwd: str = "", *, timeout: int | None = None, @@ -281,7 +279,6 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): effective_timeout = timeout or self.timeout exec_command, sudo_stdin = self._prepare_command(command) - # Merge the sudo password (if any) with caller-supplied stdin_data. if sudo_stdin is not None and stdin_data is not None: effective_stdin = sudo_stdin + stdin_data elif sudo_stdin is not None: @@ -378,10 +375,6 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): except Exception as e: return {"output": f"Execution error: {str(e)}", "returncode": 1} - # ------------------------------------------------------------------ - # Public interface - # ------------------------------------------------------------------ - def execute(self, command: str, cwd: str = "", *, timeout: int | None = None, stdin_data: str | None = None) -> dict: diff --git a/tools/environments/persistent_shell.py b/tools/environments/persistent_shell.py index f0bd438f0..0ee998381 100644 --- a/tools/environments/persistent_shell.py +++ b/tools/environments/persistent_shell.py @@ -1,18 +1,6 @@ -"""Persistent shell mixin: file-based IPC protocol for long-lived bash shells. +"""Persistent shell mixin: file-based IPC protocol for long-lived bash shells.""" -Provides the shared logic for maintaining a persistent bash shell across -execute() calls. Backend-specific operations (spawning the shell, reading -temp files, killing child processes) are implemented by subclasses via -abstract methods. - -The IPC protocol writes each command's stdout/stderr/exit-code/cwd to temp -files, then polls the status file for completion. A daemon thread drains -the shell's stdout to prevent pipe deadlock and detect shell death. -""" - -import glob as glob_mod import logging -import os import shlex import subprocess import threading @@ -28,65 +16,42 @@ logger = logging.getLogger(__name__) class PersistentShellMixin: """Mixin that adds persistent shell capability to any BaseEnvironment. - Subclasses MUST implement: - _spawn_shell_process() -> subprocess.Popen - _read_temp_files(*paths) -> list[str] - _kill_shell_children() - - Subclasses MUST also provide ``_execute_oneshot()`` for the stdin_data - fallback path (commands with piped stdin cannot use the persistent shell). + Subclasses must implement ``_spawn_shell_process()``, ``_read_temp_files()``, + ``_kill_shell_children()``, and ``_execute_oneshot()`` (stdin fallback). """ - # -- State (initialized by _init_persistent_shell) --------------------- - _shell_proc: subprocess.Popen | None = None - _shell_alive: bool = False - _shell_pid: int | None = None + @abstractmethod + def _spawn_shell_process(self) -> subprocess.Popen: ... + + @abstractmethod + def _read_temp_files(self, *paths: str) -> list[str]: ... + + @abstractmethod + def _kill_shell_children(self): ... + + @abstractmethod + def _execute_oneshot(self, command: str, cwd: str, *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: ... + + @abstractmethod + def _cleanup_temp_files(self): ... + _session_id: str = "" - # -- Abstract methods (backend-specific) ------------------------------- - - @abstractmethod - def _spawn_shell_process(self) -> subprocess.Popen: - """Spawn a long-lived bash shell and return the Popen handle. - - Must use ``stdin=PIPE, stdout=PIPE, stderr=PIPE, text=True``. - """ - ... - - @abstractmethod - def _read_temp_files(self, *paths: str) -> list[str]: - """Read temp files from the execution context. - - Returns contents in the same order as *paths*. Falls back to - empty strings on failure. - """ - ... - - @abstractmethod - def _kill_shell_children(self): - """Kill the running command's processes but keep the shell alive.""" - ... - - # -- Overridable properties -------------------------------------------- - @property def _temp_prefix(self) -> str: - """Base path for temp files. Override per backend.""" return f"/tmp/hermes-persistent-{self._session_id}" - # -- Shared implementation --------------------------------------------- - def _init_persistent_shell(self): - """Call from ``__init__`` when ``persistent=True``.""" self._shell_lock = threading.Lock() - self._session_id = "" - self._shell_proc = None - self._shell_alive = False - self._shell_pid = None + self._session_id: str = "" + self._shell_proc: subprocess.Popen | None = None + self._shell_alive: bool = False + self._shell_pid: int | None = None self._start_persistent_shell() def _start_persistent_shell(self): - """Spawn the shell, create temp files, capture PID.""" self._session_id = uuid.uuid4().hex[:12] p = self._temp_prefix self._pshell_stdout = f"{p}-stdout" @@ -103,7 +68,6 @@ class PersistentShellMixin: ) self._drain_thread.start() - # Initialize temp files and capture shell PID init_script = ( f"touch {self._pshell_stdout} {self._pshell_stderr} " f"{self._pshell_status} {self._pshell_cwd} {self._pshell_pid_file}\n" @@ -112,7 +76,6 @@ class PersistentShellMixin: ) self._send_to_shell(init_script) - # Poll for PID file deadline = time.monotonic() + 3.0 while time.monotonic() < deadline: pid_str = self._read_temp_files(self._pshell_pid_file)[0].strip() @@ -130,22 +93,19 @@ class PersistentShellMixin: self._session_id, self._shell_pid, ) - # Update cwd from what the shell reports reported_cwd = self._read_temp_files(self._pshell_cwd)[0].strip() if reported_cwd: self.cwd = reported_cwd def _drain_shell_output(self): - """Drain stdout to prevent pipe deadlock; detect shell death.""" try: for _ in self._shell_proc.stdout: - pass # Real output goes to temp files + pass except Exception: pass self._shell_alive = False def _send_to_shell(self, text: str): - """Write text to the persistent shell's stdin.""" if not self._shell_alive or self._shell_proc is None: return try: @@ -155,13 +115,11 @@ class PersistentShellMixin: self._shell_alive = False def _read_persistent_output(self) -> tuple[str, int, str]: - """Read stdout, stderr, status, cwd. Returns (output, exit_code, cwd).""" stdout, stderr, status_raw, cwd = self._read_temp_files( self._pshell_stdout, self._pshell_stderr, self._pshell_status, self._pshell_cwd, ) output = self._merge_output(stdout, stderr) - # Status format: "cmd_id:exit_code" — strip the ID prefix status = status_raw.strip() if ":" in status: status = status.split(":", 1)[1] @@ -174,15 +132,12 @@ class PersistentShellMixin: def _execute_persistent(self, command: str, cwd: str, *, timeout: int | None = None, stdin_data: str | None = None) -> dict: - """Execute a command in the persistent shell.""" if not self._shell_alive: logger.info("Persistent shell died, restarting...") self._start_persistent_shell() exec_command, sudo_stdin = self._prepare_command(command) effective_timeout = timeout or self.timeout - - # Fall back to one-shot for commands needing piped stdin if stdin_data or sudo_stdin: return self._execute_oneshot( command, cwd, timeout=timeout, stdin_data=stdin_data, @@ -195,25 +150,14 @@ class PersistentShellMixin: def _execute_persistent_locked(self, command: str, cwd: str, timeout: int) -> dict: - """Inner persistent execution — caller must hold ``_shell_lock``.""" work_dir = cwd or self.cwd - - # Each command gets a unique ID written into the status file so the - # poll loop can distinguish the *current* command's result from a - # stale value left over from the previous command. This eliminates - # the race where a fast local file read sees the old status before - # the shell has processed the truncation. cmd_id = uuid.uuid4().hex[:8] - - # Truncate temp files truncate = ( f": > {self._pshell_stdout}\n" f": > {self._pshell_stderr}\n" f": > {self._pshell_status}\n" ) self._send_to_shell(truncate) - - # Escape command for eval escaped = command.replace("'", "'\\''") ipc_script = ( @@ -224,8 +168,6 @@ class PersistentShellMixin: f"echo {cmd_id}:$__EC > {self._pshell_status}\n" ) self._send_to_shell(ipc_script) - - # Poll the status file for current command's ID deadline = time.monotonic() + timeout poll_interval = 0.15 @@ -267,7 +209,6 @@ class PersistentShellMixin: @staticmethod def _merge_output(stdout: str, stderr: str) -> str: - """Combine stdout and stderr into a single output string.""" parts = [] if stdout.strip(): parts.append(stdout.rstrip("\n")) @@ -276,7 +217,6 @@ class PersistentShellMixin: return "\n".join(parts) def _cleanup_persistent_shell(self): - """Clean up persistent shell resources. Call from ``cleanup()``.""" if self._shell_proc is None: return @@ -299,10 +239,3 @@ class PersistentShellMixin: if hasattr(self, "_drain_thread") and self._drain_thread.is_alive(): self._drain_thread.join(timeout=1.0) - def _cleanup_temp_files(self): - """Remove local temp files. Override for remote backends (SSH, Docker).""" - for f in glob_mod.glob(f"{self._temp_prefix}-*"): - try: - os.remove(f) - except OSError: - pass diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 13893dedd..7f7c7064a 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -77,10 +77,6 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): except subprocess.TimeoutExpired: raise RuntimeError(f"SSH connection to {self.user}@{self.host} timed out") - # ------------------------------------------------------------------ - # PersistentShellMixin: backend-specific implementations - # ------------------------------------------------------------------ - @property def _temp_prefix(self) -> str: return f"/tmp/hermes-ssh-{self._session_id}" @@ -92,12 +88,11 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stderr=subprocess.DEVNULL, text=True, ) def _read_temp_files(self, *paths: str) -> list[str]: - """Read remote files via ControlMaster one-shot SSH calls.""" if len(paths) == 1: cmd = self._build_ssh_command() cmd.append(f"cat {paths[0]} 2>/dev/null") @@ -135,7 +130,6 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): pass def _cleanup_temp_files(self): - """Remove remote temp files via SSH.""" try: cmd = self._build_ssh_command() cmd.append(f"rm -f {self._temp_prefix}-*") @@ -143,20 +137,14 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): except (OSError, subprocess.SubprocessError): pass - # ------------------------------------------------------------------ - # One-shot execution (original behavior) - # ------------------------------------------------------------------ - def _execute_oneshot(self, command: str, cwd: str = "", *, timeout: int | None = None, stdin_data: str | None = None) -> dict: - """Execute a command via a fresh one-shot SSH invocation.""" work_dir = cwd or self.cwd exec_command, sudo_stdin = self._prepare_command(command) wrapped = f'cd {work_dir} && {exec_command}' effective_timeout = timeout or self.timeout - # Merge sudo password (if any) with caller-supplied stdin_data. if sudo_stdin is not None and stdin_data is not None: effective_stdin = sudo_stdin + stdin_data elif sudo_stdin is not None: @@ -169,11 +157,8 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): try: kwargs = self._build_run_kwargs(timeout, effective_stdin) - # Remove timeout from kwargs -- we handle it in the poll loop kwargs.pop("timeout", None) - _output_chunks = [] - proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, @@ -224,10 +209,6 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): except Exception as e: return {"output": f"SSH execution error: {str(e)}", "returncode": 1} - # ------------------------------------------------------------------ - # Public interface - # ------------------------------------------------------------------ - def execute(self, command: str, cwd: str = "", *, timeout: int | None = None, stdin_data: str | None = None) -> dict: @@ -240,11 +221,8 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): ) def cleanup(self): - # Persistent shell teardown (via mixin) if self.persistent: self._cleanup_persistent_shell() - - # ControlMaster cleanup if self.control_socket.exists(): try: cmd = ["ssh", "-o", f"ControlPath={self.control_socket}", diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index b273ec028..a5551d716 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -504,7 +504,6 @@ def _get_env_config() -> Dict[str, Any]: "ssh_port": _parse_env_var("TERMINAL_SSH_PORT", "22"), "ssh_key": os.getenv("TERMINAL_SSH_KEY", ""), "ssh_persistent": os.getenv("TERMINAL_SSH_PERSISTENT", "false").lower() in ("true", "1", "yes"), - # Local persistent shell (cwd/env vars survive across calls) "local_persistent": os.getenv("TERMINAL_LOCAL_PERSISTENT", "false").lower() in ("true", "1", "yes"), # Container resource config (applies to docker, singularity, modal, daytona -- ignored for local/ssh) "container_cpu": _parse_env_var("TERMINAL_CONTAINER_CPU", "1", float, "number"), From 7be314c4561cffc89fd9c5bf1c0f504037167266 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sun, 15 Mar 2026 02:26:39 +0530 Subject: [PATCH 4/7] pass configs to file_tools for r+w over ssh. pass TERM env. default to ~ to in local and ssh backends. ssh backend. --- tools/environments/persistent_shell.py | 1 + tools/environments/ssh.py | 2 +- tools/file_tools.py | 19 +++++++++++++++++++ tools/terminal_tool.py | 2 ++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tools/environments/persistent_shell.py b/tools/environments/persistent_shell.py index 0ee998381..dd560a93b 100644 --- a/tools/environments/persistent_shell.py +++ b/tools/environments/persistent_shell.py @@ -69,6 +69,7 @@ class PersistentShellMixin: self._drain_thread.start() init_script = ( + f"export TERM=${{TERM:-dumb}}\n" f"touch {self._pshell_stdout} {self._pshell_stderr} " f"{self._pshell_status} {self._pshell_cwd} {self._pshell_pid_file}\n" f"echo $$ > {self._pshell_pid_file}\n" diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 7f7c7064a..1bcc41ee7 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -153,7 +153,7 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): effective_stdin = stdin_data cmd = self._build_ssh_command() - cmd.extend(["bash", "-c", wrapped]) + cmd.append(wrapped) try: kwargs = self._build_run_kwargs(timeout, effective_stdin) diff --git a/tools/file_tools.py b/tools/file_tools.py index 8ed019f0a..6e9e7e37e 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -101,12 +101,31 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: "container_persistent": config.get("container_persistent", True), "docker_volumes": config.get("docker_volumes", []), } + + ssh_config = None + if env_type == "ssh": + ssh_config = { + "host": config.get("ssh_host", ""), + "user": config.get("ssh_user", ""), + "port": config.get("ssh_port", 22), + "key": config.get("ssh_key", ""), + "persistent": config.get("ssh_persistent", False), + } + + local_config = None + if env_type == "local": + local_config = { + "persistent": config.get("local_persistent", False), + } + terminal_env = _create_environment( env_type=env_type, image=image, cwd=cwd, timeout=config["timeout"], + ssh_config=ssh_config, container_config=container_config, + local_config=local_config, task_id=task_id, ) diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index a5551d716..327e12210 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -471,6 +471,8 @@ def _get_env_config() -> Dict[str, Any]: # is running inside the container/remote). if env_type == "local": default_cwd = os.getcwd() + elif env_type == "ssh": + default_cwd = "~" else: default_cwd = "/root" From 9f36483bf4570a521a50e1759810d03fcb9c01d7 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sun, 15 Mar 2026 02:39:56 +0530 Subject: [PATCH 5/7] refactor: deduplicate execute/cleanup, merge init, clean up helpers - Merge _init_persistent_shell + _start_persistent_shell into single method - Move execute() dispatcher and cleanup() into PersistentShellMixin so LocalEnvironment and SSHEnvironment inherit them - Remove broad except Exception wrappers from _execute_oneshot in both backends - Replace try/except with os.path.exists checks in local _read_temp_files and _cleanup_temp_files - Remove redundant bash -c from SSH oneshot (SSH already runs in a shell) Co-Authored-By: Claude Opus 4.6 (1M context) --- tools/environments/local.py | 191 +++++++++++-------------- tools/environments/persistent_shell.py | 87 +++++++---- tools/environments/ssh.py | 120 +++++++--------- 3 files changed, 195 insertions(+), 203 deletions(-) diff --git a/tools/environments/local.py b/tools/environments/local.py index 1641d12a4..5859d6e40 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -247,10 +247,10 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): def _read_temp_files(self, *paths: str) -> list[str]: results = [] for path in paths: - try: + if os.path.exists(path): with open(path) as f: results.append(f.read()) - except OSError: + else: results.append("") return results @@ -262,15 +262,13 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): ["pkill", "-P", str(self._shell_pid)], capture_output=True, timeout=5, ) - except (subprocess.TimeoutExpired, OSError, FileNotFoundError): + except (subprocess.TimeoutExpired, FileNotFoundError): pass def _cleanup_temp_files(self): for f in glob.glob(f"{self._temp_prefix}-*"): - try: + if os.path.exists(f): os.remove(f) - except OSError: - pass def _execute_oneshot(self, command: str, cwd: str = "", *, timeout: int | None = None, @@ -286,106 +284,87 @@ class LocalEnvironment(PersistentShellMixin, BaseEnvironment): else: effective_stdin = stdin_data - try: - user_shell = _find_bash() - fenced_cmd = ( - f"printf '{_OUTPUT_FENCE}';" - f" {exec_command};" - f" __hermes_rc=$?;" - f" printf '{_OUTPUT_FENCE}';" - f" exit $__hermes_rc" - ) - run_env = _make_run_env(self.env) + user_shell = _find_bash() + fenced_cmd = ( + f"printf '{_OUTPUT_FENCE}';" + f" {exec_command};" + f" __hermes_rc=$?;" + f" printf '{_OUTPUT_FENCE}';" + f" exit $__hermes_rc" + ) + run_env = _make_run_env(self.env) - proc = subprocess.Popen( - [user_shell, "-lic", fenced_cmd], - text=True, - cwd=work_dir, - env=run_env, - encoding="utf-8", - errors="replace", - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if effective_stdin is not None else subprocess.DEVNULL, - preexec_fn=None if _IS_WINDOWS else os.setsid, - ) - - if effective_stdin is not None: - def _write_stdin(): - try: - proc.stdin.write(effective_stdin) - proc.stdin.close() - except (BrokenPipeError, OSError): - pass - threading.Thread(target=_write_stdin, daemon=True).start() - - _output_chunks: list[str] = [] - - def _drain_stdout(): - try: - for line in proc.stdout: - _output_chunks.append(line) - except ValueError: - pass - finally: - try: - proc.stdout.close() - except Exception: - pass - - reader = threading.Thread(target=_drain_stdout, daemon=True) - reader.start() - deadline = time.monotonic() + effective_timeout - - while proc.poll() is None: - if is_interrupted(): - try: - if _IS_WINDOWS: - proc.terminate() - else: - pgid = os.getpgid(proc.pid) - os.killpg(pgid, signal.SIGTERM) - try: - proc.wait(timeout=1.0) - except subprocess.TimeoutExpired: - os.killpg(pgid, signal.SIGKILL) - except (ProcessLookupError, PermissionError): - proc.kill() - reader.join(timeout=2) - return { - "output": "".join(_output_chunks) + "\n[Command interrupted — user sent a new message]", - "returncode": 130, - } - if time.monotonic() > deadline: - try: - if _IS_WINDOWS: - proc.terminate() - else: - os.killpg(os.getpgid(proc.pid), signal.SIGTERM) - except (ProcessLookupError, PermissionError): - proc.kill() - reader.join(timeout=2) - return self._timeout_result(effective_timeout) - time.sleep(0.2) - - reader.join(timeout=5) - output = _extract_fenced_output("".join(_output_chunks)) - return {"output": output, "returncode": proc.returncode} - - except Exception as e: - return {"output": f"Execution error: {str(e)}", "returncode": 1} - - def execute(self, command: str, cwd: str = "", *, - timeout: int | None = None, - stdin_data: str | None = None) -> dict: - if self.persistent: - return self._execute_persistent( - command, cwd, timeout=timeout, stdin_data=stdin_data, - ) - return self._execute_oneshot( - command, cwd, timeout=timeout, stdin_data=stdin_data, + proc = subprocess.Popen( + [user_shell, "-lic", fenced_cmd], + text=True, + cwd=work_dir, + env=run_env, + encoding="utf-8", + errors="replace", + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + stdin=subprocess.PIPE if effective_stdin is not None else subprocess.DEVNULL, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) - def cleanup(self): - if self.persistent: - self._cleanup_persistent_shell() + if effective_stdin is not None: + def _write_stdin(): + try: + proc.stdin.write(effective_stdin) + proc.stdin.close() + except (BrokenPipeError, OSError): + pass + threading.Thread(target=_write_stdin, daemon=True).start() + + _output_chunks: list[str] = [] + + def _drain_stdout(): + try: + for line in proc.stdout: + _output_chunks.append(line) + except ValueError: + pass + finally: + try: + proc.stdout.close() + except Exception: + pass + + reader = threading.Thread(target=_drain_stdout, daemon=True) + reader.start() + deadline = time.monotonic() + effective_timeout + + while proc.poll() is None: + if is_interrupted(): + try: + if _IS_WINDOWS: + proc.terminate() + else: + pgid = os.getpgid(proc.pid) + os.killpg(pgid, signal.SIGTERM) + try: + proc.wait(timeout=1.0) + except subprocess.TimeoutExpired: + os.killpg(pgid, signal.SIGKILL) + except (ProcessLookupError, PermissionError): + proc.kill() + reader.join(timeout=2) + return { + "output": "".join(_output_chunks) + "\n[Command interrupted — user sent a new message]", + "returncode": 130, + } + if time.monotonic() > deadline: + try: + if _IS_WINDOWS: + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + except (ProcessLookupError, PermissionError): + proc.kill() + reader.join(timeout=2) + return self._timeout_result(effective_timeout) + time.sleep(0.2) + + reader.join(timeout=5) + output = _extract_fenced_output("".join(_output_chunks)) + return {"output": output, "returncode": proc.returncode} diff --git a/tools/environments/persistent_shell.py b/tools/environments/persistent_shell.py index dd560a93b..df1a78ef9 100644 --- a/tools/environments/persistent_shell.py +++ b/tools/environments/persistent_shell.py @@ -17,9 +17,11 @@ class PersistentShellMixin: """Mixin that adds persistent shell capability to any BaseEnvironment. Subclasses must implement ``_spawn_shell_process()``, ``_read_temp_files()``, - ``_kill_shell_children()``, and ``_execute_oneshot()`` (stdin fallback). + ``_kill_shell_children()``, ``_execute_oneshot()``, and ``_cleanup_temp_files()``. """ + persistent: bool + @abstractmethod def _spawn_shell_process(self) -> subprocess.Popen: ... @@ -43,15 +45,16 @@ class PersistentShellMixin: def _temp_prefix(self) -> str: return f"/tmp/hermes-persistent-{self._session_id}" + # ------------------------------------------------------------------ + # Lifecycle + # ------------------------------------------------------------------ + def _init_persistent_shell(self): self._shell_lock = threading.Lock() - self._session_id: str = "" self._shell_proc: subprocess.Popen | None = None self._shell_alive: bool = False self._shell_pid: int | None = None - self._start_persistent_shell() - def _start_persistent_shell(self): self._session_id = uuid.uuid4().hex[:12] p = self._temp_prefix self._pshell_stdout = f"{p}-stdout" @@ -98,6 +101,52 @@ class PersistentShellMixin: if reported_cwd: self.cwd = reported_cwd + def _cleanup_persistent_shell(self): + if self._shell_proc is None: + return + + if self._session_id: + self._cleanup_temp_files() + + try: + self._shell_proc.stdin.close() + except Exception: + pass + try: + self._shell_proc.terminate() + self._shell_proc.wait(timeout=3) + except subprocess.TimeoutExpired: + self._shell_proc.kill() + + self._shell_alive = False + self._shell_proc = None + + if hasattr(self, "_drain_thread") and self._drain_thread.is_alive(): + self._drain_thread.join(timeout=1.0) + + # ------------------------------------------------------------------ + # execute() / cleanup() — shared dispatcher, subclasses inherit + # ------------------------------------------------------------------ + + def execute(self, command: str, cwd: str = "", *, + timeout: int | None = None, + stdin_data: str | None = None) -> dict: + if self.persistent: + return self._execute_persistent( + command, cwd, timeout=timeout, stdin_data=stdin_data, + ) + return self._execute_oneshot( + command, cwd, timeout=timeout, stdin_data=stdin_data, + ) + + def cleanup(self): + if self.persistent: + self._cleanup_persistent_shell() + + # ------------------------------------------------------------------ + # Shell I/O + # ------------------------------------------------------------------ + def _drain_shell_output(self): try: for _ in self._shell_proc.stdout: @@ -130,12 +179,16 @@ class PersistentShellMixin: exit_code = 1 return output, exit_code, cwd.strip() + # ------------------------------------------------------------------ + # Execution + # ------------------------------------------------------------------ + def _execute_persistent(self, command: str, cwd: str, *, timeout: int | None = None, stdin_data: str | None = None) -> dict: if not self._shell_alive: logger.info("Persistent shell died, restarting...") - self._start_persistent_shell() + self._init_persistent_shell() exec_command, sudo_stdin = self._prepare_command(command) effective_timeout = timeout or self.timeout @@ -216,27 +269,3 @@ class PersistentShellMixin: if stderr.strip(): parts.append(stderr.rstrip("\n")) return "\n".join(parts) - - def _cleanup_persistent_shell(self): - if self._shell_proc is None: - return - - if self._session_id: - self._cleanup_temp_files() - - try: - self._shell_proc.stdin.close() - except Exception: - pass - try: - self._shell_proc.terminate() - self._shell_proc.wait(timeout=3) - except subprocess.TimeoutExpired: - self._shell_proc.kill() - - self._shell_alive = False - self._shell_proc = None - - if hasattr(self, "_drain_thread") and self._drain_thread.is_alive(): - self._drain_thread.join(timeout=1.0) - diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index 1bcc41ee7..c48b38509 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -130,11 +130,11 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): pass def _cleanup_temp_files(self): + cmd = self._build_ssh_command() + cmd.append(f"rm -f {self._temp_prefix}-*") try: - cmd = self._build_ssh_command() - cmd.append(f"rm -f {self._temp_prefix}-*") subprocess.run(cmd, capture_output=True, timeout=5) - except (OSError, subprocess.SubprocessError): + except (subprocess.TimeoutExpired, OSError): pass def _execute_oneshot(self, command: str, cwd: str = "", *, @@ -155,74 +155,58 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): cmd = self._build_ssh_command() cmd.append(wrapped) - try: - kwargs = self._build_run_kwargs(timeout, effective_stdin) - kwargs.pop("timeout", None) - _output_chunks = [] - proc = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - stdin=subprocess.PIPE if effective_stdin else subprocess.DEVNULL, - text=True, - ) - - if effective_stdin: - try: - proc.stdin.write(effective_stdin) - proc.stdin.close() - except Exception: - pass - - def _drain(): - try: - for line in proc.stdout: - _output_chunks.append(line) - except Exception: - pass - - reader = threading.Thread(target=_drain, daemon=True) - reader.start() - deadline = time.monotonic() + effective_timeout - - while proc.poll() is None: - if is_interrupted(): - proc.terminate() - try: - proc.wait(timeout=1) - except subprocess.TimeoutExpired: - proc.kill() - reader.join(timeout=2) - return { - "output": "".join(_output_chunks) + "\n[Command interrupted]", - "returncode": 130, - } - if time.monotonic() > deadline: - proc.kill() - reader.join(timeout=2) - return self._timeout_result(effective_timeout) - time.sleep(0.2) - - reader.join(timeout=5) - return {"output": "".join(_output_chunks), "returncode": proc.returncode} - - except Exception as e: - return {"output": f"SSH execution error: {str(e)}", "returncode": 1} - - def execute(self, command: str, cwd: str = "", *, - timeout: int | None = None, - stdin_data: str | None = None) -> dict: - if self.persistent: - return self._execute_persistent( - command, cwd, timeout=timeout, stdin_data=stdin_data - ) - return self._execute_oneshot( - command, cwd, timeout=timeout, stdin_data=stdin_data + kwargs = self._build_run_kwargs(timeout, effective_stdin) + kwargs.pop("timeout", None) + _output_chunks = [] + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + stdin=subprocess.PIPE if effective_stdin else subprocess.DEVNULL, + text=True, ) + if effective_stdin: + try: + proc.stdin.write(effective_stdin) + proc.stdin.close() + except (BrokenPipeError, OSError): + pass + + def _drain(): + try: + for line in proc.stdout: + _output_chunks.append(line) + except Exception: + pass + + reader = threading.Thread(target=_drain, daemon=True) + reader.start() + deadline = time.monotonic() + effective_timeout + + while proc.poll() is None: + if is_interrupted(): + proc.terminate() + try: + proc.wait(timeout=1) + except subprocess.TimeoutExpired: + proc.kill() + reader.join(timeout=2) + return { + "output": "".join(_output_chunks) + "\n[Command interrupted]", + "returncode": 130, + } + if time.monotonic() > deadline: + proc.kill() + reader.join(timeout=2) + return self._timeout_result(effective_timeout) + time.sleep(0.2) + + reader.join(timeout=5) + return {"output": "".join(_output_chunks), "returncode": proc.returncode} + def cleanup(self): - if self.persistent: - self._cleanup_persistent_shell() + super().cleanup() if self.control_socket.exists(): try: cmd = ["ssh", "-o", f"ControlPath={self.control_socket}", From 879b7d3fbf8b1b214dbf844f0e6a58d018415da7 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sun, 15 Mar 2026 02:48:05 +0530 Subject: [PATCH 6/7] fix(tests): update mock stdout in env blocklist tests The fake_popen mock used iter([]) for proc.stdout which doesn't support .close(). Use MagicMock with __iter__ instead, since _drain_stdout now calls proc.stdout.close() in its finally block. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/tools/test_local_env_blocklist.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 70a8ae5d1..fdb68f002 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -25,8 +25,7 @@ def _make_fake_popen(captured: dict): proc = MagicMock() proc.poll.return_value = 0 proc.returncode = 0 - proc.stdout = iter([]) - proc.stdout.close = lambda: None + proc.stdout = MagicMock(__iter__=lambda s: iter([]), __next__=lambda s: (_ for _ in ()).throw(StopIteration)) proc.stdin = MagicMock() return proc return fake_popen From e266530c7d7ca316e1a522d59193811ee840959e Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Sun, 15 Mar 2026 02:33:04 +0530 Subject: [PATCH 7/7] add different polling intervals for ssh and local backends. ssh has a longer roundtrip --- tools/environments/persistent_shell.py | 3 ++- tools/environments/ssh.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/environments/persistent_shell.py b/tools/environments/persistent_shell.py index df1a78ef9..4b89db471 100644 --- a/tools/environments/persistent_shell.py +++ b/tools/environments/persistent_shell.py @@ -40,6 +40,7 @@ class PersistentShellMixin: def _cleanup_temp_files(self): ... _session_id: str = "" + _poll_interval: float = 0.01 @property def _temp_prefix(self) -> str: @@ -223,7 +224,7 @@ class PersistentShellMixin: ) self._send_to_shell(ipc_script) deadline = time.monotonic() + timeout - poll_interval = 0.15 + poll_interval = self._poll_interval while True: if is_interrupted(): diff --git a/tools/environments/ssh.py b/tools/environments/ssh.py index c48b38509..90532dda0 100644 --- a/tools/environments/ssh.py +++ b/tools/environments/ssh.py @@ -77,6 +77,8 @@ class SSHEnvironment(PersistentShellMixin, BaseEnvironment): except subprocess.TimeoutExpired: raise RuntimeError(f"SSH connection to {self.user}@{self.host} timed out") + _poll_interval: float = 0.15 + @property def _temp_prefix(self) -> str: return f"/tmp/hermes-ssh-{self._session_id}"