refactor(ssh): drop probe-only bookkeeping the probe never reads
- probe_only is consumed inside __init__ only; no instance attribute. - _sync_manager is assigned only on the probe branch, after the connection is up, so a normal SSHEnvironment whose constructor fails still behaves exactly as before (its __del__ cleanup does not reach the shared socket teardown). - _remote_home has no reader on the probe path; not assigned. - prompt_builder passes probe_only=True unconditionally: which backends honor it is the builder table's decision, not a caller-side env_type check. - Tests trimmed to the probe_only contract: the cleanup-raises case was green on main (pre-existing try/except), the socket-name length and double-cleanup assertions covered pre-existing behaviour.
This commit is contained in:
@@ -876,10 +876,10 @@ def _run_backend_probe(env_type: str, terminal_tool) -> str:
|
||||
container_config=(_container_config_from_config(config)
|
||||
if terminal_tool._is_container_backend(env_type) else None),
|
||||
task_id="prompt-backend-probe", host_cwd=config.get("host_cwd"),
|
||||
# ssh: an isolated ControlMaster socket and no remote dir setup / file sync / snapshot —
|
||||
# a normal SSHEnvironment would upload the whole ~/.hermes tree just to run `uname`, and
|
||||
# its later __del__ would sync_back() and close the master shared with the agent's own env.
|
||||
probe_only=env_type == "ssh",
|
||||
# Only ssh honors this: an isolated ControlMaster socket and no remote dir setup / file sync /
|
||||
# snapshot. A normal SSHEnvironment would upload the whole ~/.hermes tree just to run `uname`,
|
||||
# and its later __del__ would sync_back() and close the master shared with the agent's own env.
|
||||
probe_only=True,
|
||||
)
|
||||
try:
|
||||
result = env.execute(_BACKEND_PROBE_CMD, timeout=4)
|
||||
|
||||
@@ -970,27 +970,6 @@ class TestEnvironmentHints:
|
||||
assert created["probe_only"] is True
|
||||
assert calls == ["cleanup"]
|
||||
|
||||
def test_probe_remote_backend_ssh_cleanup_error_keeps_result(self, monkeypatch):
|
||||
"""A failing teardown of the throwaway probe connection must not discard the
|
||||
metadata the probe already collected."""
|
||||
import agent.prompt_builder as _pb
|
||||
|
||||
monkeypatch.setenv("TERMINAL_ENV", "ssh")
|
||||
_pb._clear_backend_probe_cache()
|
||||
|
||||
class _Env:
|
||||
def execute(self, cmd, timeout=None):
|
||||
return {"returncode": 0, "output": "os=Linux\nkernel=6.8.0\nhome=/h\ncwd=/h\nuser=u\n"}
|
||||
|
||||
def cleanup(self):
|
||||
raise RuntimeError("cleanup failed")
|
||||
|
||||
import tools.terminal_tool_backends as _tt
|
||||
monkeypatch.setattr(_tt, "_create_environment", lambda **kw: _Env())
|
||||
|
||||
assert "Linux 6.8.0" in _pb._probe_remote_backend("ssh")
|
||||
|
||||
|
||||
def test_environment_hint_from_env_var_is_appended(self, monkeypatch):
|
||||
"""HERMES_ENVIRONMENT_HINT lets an embedder describe the runtime env."""
|
||||
import agent.prompt_builder as _pb
|
||||
|
||||
@@ -261,7 +261,6 @@ class TestSSHProbeOnly:
|
||||
_mock_ssh_runtime["_ensure_remote_dirs"].assert_not_called()
|
||||
_mock_ssh_runtime["sync_factory"].assert_not_called()
|
||||
_mock_ssh_runtime["init_session"].assert_not_called()
|
||||
assert env._sync_manager is None
|
||||
|
||||
def test_probe_only_control_socket_is_isolated(self, monkeypatch, _mock_ssh_runtime):
|
||||
control_exit_calls = []
|
||||
@@ -283,12 +282,10 @@ class TestSSHProbeOnly:
|
||||
normal.control_socket.touch()
|
||||
first_probe.control_socket.touch()
|
||||
first_probe.cleanup()
|
||||
first_probe.cleanup()
|
||||
|
||||
assert normal.control_socket.exists()
|
||||
assert not first_probe.control_socket.exists()
|
||||
assert len(control_exit_calls) == 1
|
||||
normal.cleanup()
|
||||
|
||||
|
||||
def _setup_ssh_env(monkeypatch, persistent: bool):
|
||||
|
||||
@@ -48,7 +48,6 @@ class SSHEnvironment(BaseEnvironment):
|
||||
Spawn-per-call: every execute() spawns a fresh ``ssh ... bash -c`` process.
|
||||
Session snapshot preserves env vars across calls; CWD persists via in-band
|
||||
stdout markers. Uses SSH ControlMaster for connection reuse.
|
||||
Probe-only instances use an isolated connection without sync or session state.
|
||||
"""
|
||||
|
||||
# Passthrough values are re-forwarded on every command (see _run_bash), so like docker/local
|
||||
@@ -60,8 +59,6 @@ class SSHEnvironment(BaseEnvironment):
|
||||
probe_only: bool = False):
|
||||
super().__init__(cwd=cwd, timeout=timeout)
|
||||
self.host, self.user, self.port, self.key_path = host, user, port, key_path
|
||||
self._probe_only = probe_only
|
||||
self._sync_manager = None
|
||||
self.control_dir = Path(tempfile.gettempdir()) / "hermes-ssh"
|
||||
self.control_dir.mkdir(parents=True, exist_ok=True)
|
||||
# Short, deterministic socket name: the path must stay under macOS's 104-byte sun_path
|
||||
@@ -69,16 +66,15 @@ class SSHEnvironment(BaseEnvironment):
|
||||
# stability across reconnects keeps ControlMaster reuse working. A probe gets its own
|
||||
# per-instance socket so its cleanup() can never close the agent's shared master.
|
||||
socket_key = f"{user}@{host}:{port}"
|
||||
if self._probe_only:
|
||||
if probe_only:
|
||||
socket_key = f"{socket_key}:probe:{self._session_id}"
|
||||
_socket_id = hashlib.sha256(socket_key.encode()).hexdigest()[:16]
|
||||
self.control_socket = self.control_dir / f"{_socket_id}.sock"
|
||||
_ensure_ssh_available()
|
||||
self._establish_connection()
|
||||
if self._probe_only:
|
||||
self._remote_home = ""
|
||||
if probe_only:
|
||||
self._sync_manager = None
|
||||
return
|
||||
|
||||
self._remote_home = self._detect_remote_home()
|
||||
self._ensure_remote_dirs()
|
||||
self._sync_manager = FileSyncManager(
|
||||
|
||||
Reference in New Issue
Block a user