fix(local): recover when persistent_shell cwd is deleted (#17558)

When a tool call deletes its own working directory (`cd /tmp/foo &&
rm -rf /tmp/foo`), the next `subprocess.Popen(args, cwd=self.cwd)` raised
`FileNotFoundError: [Errno 2]` before bash even started — every subsequent
terminal/file-tool call hit the same wedge until the gateway restarted.

Fix in `LocalEnvironment._run_bash`: before handing `self.cwd` to Popen,
resolve a safe alternative when the path is gone (walk up to the nearest
existing ancestor, falling back to `tempfile.gettempdir()` only as a last
resort). Log a warning so the recovery is visible — not silent — and
update `self.cwd` so the next call doesn't repeat the message.

Defense in depth in `LocalEnvironment._update_cwd`: only adopt the new
cwd when it still exists as a directory. `pwd -P` from a deleted cwd can
leave a stale value in the marker file; refusing to store a missing path
keeps `self.cwd` valid by construction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
briandevans 2026-04-29 10:16:22 -07:00 committed by kshitij
parent b8fb9270c4
commit 9644b8ae67
2 changed files with 199 additions and 2 deletions

View file

@ -0,0 +1,151 @@
"""Tests for LocalEnvironment recovery when ``self.cwd`` is deleted.
When a tool call inside the persistent terminal session ``rm -rf``'s its own
working directory, the next ``subprocess.Popen(..., cwd=self.cwd)`` would
otherwise raise ``FileNotFoundError`` before bash starts, wedging every
subsequent terminal/file-tool call until the gateway restarts.
Regression coverage for https://github.com/NousResearch/hermes-agent/issues/17558.
"""
import os
import shutil
import tempfile
import threading
from unittest.mock import MagicMock, patch
from tools.environments.local import (
LocalEnvironment,
_resolve_safe_cwd,
)
class TestResolveSafeCwd:
"""Pure-function unit tests for the recovery helper."""
def test_returns_cwd_when_directory_exists(self, tmp_path):
path = str(tmp_path)
assert _resolve_safe_cwd(path) == path
def test_walks_up_to_first_existing_ancestor(self, tmp_path):
nested = tmp_path / "child" / "grandchild"
nested.mkdir(parents=True)
deleted = str(nested)
shutil.rmtree(tmp_path / "child")
# The deepest existing ancestor on the path is tmp_path itself.
assert _resolve_safe_cwd(deleted) == str(tmp_path)
def test_falls_back_when_path_is_empty(self):
assert _resolve_safe_cwd("") == tempfile.gettempdir()
def test_returns_tempdir_when_nothing_on_path_exists(self, monkeypatch):
monkeypatch.setattr(os.path, "isdir", lambda p: False)
assert _resolve_safe_cwd("/no/such/dir") == tempfile.gettempdir()
def _fake_interrupt():
return threading.Event()
def _make_fake_popen(captured: dict):
def fake_popen(cmd, **kwargs):
captured["cwd"] = kwargs.get("cwd")
captured["env"] = kwargs.get("env", {})
proc = MagicMock()
proc.poll.return_value = 0
proc.returncode = 0
proc.stdout = MagicMock(
__iter__=lambda s: iter([]),
__next__=lambda s: (_ for _ in ()).throw(StopIteration),
)
proc.stdin = MagicMock()
return proc
return fake_popen
class TestRunBashCwdRecovery:
"""End-to-end recovery: deleted ``self.cwd`` must not crash Popen."""
def test_recovers_when_cwd_deleted_after_init(self, tmp_path, caplog):
"""Reproduces the wedge from #17558: cwd was valid when the
snapshot was taken, but a subsequent command deleted it before the
next ``Popen``."""
wedged = tmp_path / "wedge-repro"
wedged.mkdir()
with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None):
env = LocalEnvironment(cwd=str(wedged), timeout=10)
# The previous tool call deleted the working directory.
shutil.rmtree(wedged)
assert env.cwd == str(wedged) and not os.path.isdir(env.cwd)
captured = {}
with patch("tools.environments.local._find_bash", return_value="/bin/bash"), \
patch("subprocess.Popen", side_effect=_make_fake_popen(captured)), \
patch("tools.terminal_tool._interrupt_event", _fake_interrupt()), \
caplog.at_level("WARNING", logger="tools.environments.local"):
env.execute("echo hello")
# Popen must have been handed a real, existing directory.
assert captured["cwd"] == str(tmp_path)
assert os.path.isdir(captured["cwd"])
# ``self.cwd`` is updated so the next call doesn't re-warn.
assert env.cwd == str(tmp_path)
# The warning surfaces the wedge so it isn't silently masked.
assert any("missing on disk" in rec.message for rec in caplog.records)
def test_no_warning_when_cwd_still_exists(self, tmp_path, caplog):
with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None):
env = LocalEnvironment(cwd=str(tmp_path), timeout=10)
captured = {}
with patch("tools.environments.local._find_bash", return_value="/bin/bash"), \
patch("subprocess.Popen", side_effect=_make_fake_popen(captured)), \
patch("tools.terminal_tool._interrupt_event", _fake_interrupt()), \
caplog.at_level("WARNING", logger="tools.environments.local"):
env.execute("echo hello")
assert captured["cwd"] == str(tmp_path)
assert env.cwd == str(tmp_path)
assert not any("missing on disk" in rec.message for rec in caplog.records)
class TestUpdateCwdRejectsMissingPaths:
"""``_update_cwd`` must not propagate a deleted path back into ``self.cwd``."""
def test_skips_assignment_when_marker_path_missing(self, tmp_path):
original = tmp_path / "starting"
original.mkdir()
with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None):
env = LocalEnvironment(cwd=str(original), timeout=10)
# Simulate the stale-marker case: the prior command's ``pwd -P`` left
# a path in the cwd file, but that path has since been deleted.
deleted = tmp_path / "wedge-repro"
with open(env._cwd_file, "w") as f:
f.write(str(deleted))
env._update_cwd({"output": "", "returncode": 0})
assert env.cwd == str(original)
def test_accepts_assignment_when_marker_path_exists(self, tmp_path):
original = tmp_path / "starting"
original.mkdir()
new_dir = tmp_path / "next"
new_dir.mkdir()
with patch.object(LocalEnvironment, "init_session", autospec=True, return_value=None):
env = LocalEnvironment(cwd=str(original), timeout=10)
with open(env._cwd_file, "w") as f:
f.write(str(new_dir))
env._update_cwd({"output": "", "returncode": 0})
assert env.cwd == str(new_dir)