mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(file-ops): preserve CWD across terminal environment re-creation (#26211)
Root cause: when the terminal environment (`_active_environments` entry) is cleaned up and re-created during a long conversation, the new environment always starts with the default config CWD (typically `~/.hermes/hermes-agent`) instead of preserving the user's last-known working directory. Subsequent relative-path writes (`write_file`, `execute_code`, shell commands) silently land in the default CWD, making files appear to be "created but absent." Fix: add `_last_known_cwd` dict that preserves the old environment's CWD before the stale cache entry is invalidated. When a new environment is created for the same task_id, we check `_last_known_cwd` first and use the preserved CWD instead of the config default. Changes: - tools/file_tools.py: add `_last_known_cwd` dict, save CWD before stale cache invalidation, restore CWD on env recreation - tests/tools/test_file_tools.py: add `TestLastKnownCwd` with 2 tests verifying CWD preservation and fallback behavior Fixes #26211
This commit is contained in:
parent
926a1b915d
commit
adeba1d7a8
2 changed files with 114 additions and 2 deletions
|
|
@ -513,3 +513,105 @@ class TestPatchSchemaShape:
|
|||
params = PATCH_SCHEMA["parameters"]
|
||||
assert params["required"] == ["mode"]
|
||||
assert "anyOf" not in params and "oneOf" not in params
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _last_known_cwd tests (#26211: silent file creation failure in long conversations)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestLastKnownCwd:
|
||||
"""
|
||||
When the terminal environment is cleaned up and re-created during a long
|
||||
conversation, _last_known_cwd preserves the old environment's CWD so
|
||||
subsequent file writes with relative paths land in the right directory.
|
||||
|
||||
Regression guard for issue #26211.
|
||||
"""
|
||||
|
||||
@patch("tools.terminal_tool._active_environments", new_callable=dict)
|
||||
@patch("tools.file_tools._file_ops_cache", new_callable=dict)
|
||||
@patch("tools.terminal_tool._get_env_config")
|
||||
@patch("tools.terminal_tool._create_environment")
|
||||
def test_last_known_cwd_preserved_across_env_recreation(
|
||||
self, mock_create_env, mock_config, mock_cache, mock_active
|
||||
):
|
||||
from tools.file_tools import _get_file_ops, _last_known_cwd
|
||||
|
||||
# Setup: create a mock env with a known CWD
|
||||
mock_env = MagicMock()
|
||||
mock_env.cwd = "/Users/user/project"
|
||||
mock_create_env.return_value = mock_env
|
||||
mock_config.return_value = {
|
||||
"env_type": "local",
|
||||
"cwd": "/default/path",
|
||||
"timeout": 30,
|
||||
}
|
||||
|
||||
task_id = "default"
|
||||
|
||||
# Preset _last_known_cwd to simulate a previous env's CWD
|
||||
_last_known_cwd[task_id] = "/Users/user/project"
|
||||
|
||||
# Call _get_file_ops - should use _last_known_cwd for the new env
|
||||
result = _get_file_ops(task_id)
|
||||
|
||||
# Verify the env was created with the saved CWD, not the default
|
||||
create_call = mock_create_env.call_args
|
||||
assert create_call is not None, "_create_environment was not called"
|
||||
|
||||
# Find cwd in the kwargs
|
||||
kwargs = create_call.kwargs if create_call.kwargs else {}
|
||||
# cwd is passed as positional or keyword
|
||||
cwd_passed = kwargs.get("cwd", None)
|
||||
if cwd_passed is None:
|
||||
# Try positional args
|
||||
args = create_call.args if create_call.args else []
|
||||
# Position: (env_type, image, cwd, timeout, ...)
|
||||
if len(args) >= 3:
|
||||
cwd_passed = args[2]
|
||||
|
||||
assert cwd_passed == "/Users/user/project", \
|
||||
f"Expected cwd='/Users/user/project', got {cwd_passed!r}"
|
||||
|
||||
# Cleanup
|
||||
_last_known_cwd.pop(task_id, None)
|
||||
|
||||
@patch("tools.terminal_tool._active_environments", new_callable=dict)
|
||||
@patch("tools.file_tools._file_ops_cache", new_callable=dict)
|
||||
@patch("tools.terminal_tool._get_env_config")
|
||||
@patch("tools.terminal_tool._create_environment")
|
||||
def test_last_known_cwd_falls_back_to_config_default_when_not_set(
|
||||
self, mock_create_env, mock_config, mock_cache, mock_active
|
||||
):
|
||||
from tools.file_tools import _get_file_ops, _last_known_cwd
|
||||
|
||||
mock_env = MagicMock()
|
||||
mock_env.cwd = "/default/path"
|
||||
mock_create_env.return_value = mock_env
|
||||
mock_config.return_value = {
|
||||
"env_type": "local",
|
||||
"cwd": "/config/default/path",
|
||||
"timeout": 30,
|
||||
}
|
||||
|
||||
# _get_file_ops resolves to "default"
|
||||
task_id = "default"
|
||||
|
||||
# Ensure _last_known_cwd is empty for this task
|
||||
_last_known_cwd.pop(task_id, None)
|
||||
|
||||
result = _get_file_ops(task_id)
|
||||
|
||||
create_call = mock_create_env.call_args
|
||||
assert create_call is not None, "_create_environment was not called"
|
||||
|
||||
kwargs = create_call.kwargs if create_call.kwargs else {}
|
||||
cwd_passed = kwargs.get("cwd", None)
|
||||
if cwd_passed is None:
|
||||
args = create_call.args if create_call.args else []
|
||||
if len(args) >= 3:
|
||||
cwd_passed = args[2]
|
||||
|
||||
# Should fall back to config default
|
||||
assert cwd_passed == "/config/default/path", \
|
||||
f"Expected cwd='/config/default/path', got {cwd_passed!r}"
|
||||
|
|
|
|||
|
|
@ -555,6 +555,10 @@ def _is_expected_write_exception(exc: Exception) -> bool:
|
|||
|
||||
_file_ops_lock = threading.Lock()
|
||||
_file_ops_cache: dict = {}
|
||||
# Per-task last-known CWD — preserved across env re-creation so
|
||||
# relative-path file writes land in the right directory after the
|
||||
# terminal environment is cleaned up and rebuilt (root cause of #26211).
|
||||
_last_known_cwd: dict = {}
|
||||
|
||||
# Track files read per task to detect re-read loops and deduplicate reads.
|
||||
# Per task_id we store:
|
||||
|
|
@ -789,7 +793,13 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations:
|
|||
_last_activity[task_id] = time.time()
|
||||
return cached
|
||||
else:
|
||||
# Environment was cleaned up -- invalidate stale cache entry
|
||||
# Environment was cleaned up -- preserve the old cwd before
|
||||
# invalidating the stale cache entry (fixes #26211: silent
|
||||
# file-creation failures in long-running conversations).
|
||||
old_cwd = getattr(cached, "cwd", None)
|
||||
if old_cwd:
|
||||
with _file_ops_lock:
|
||||
_last_known_cwd[task_id] = old_cwd
|
||||
with _file_ops_lock:
|
||||
_file_ops_cache.pop(task_id, None)
|
||||
|
||||
|
|
@ -827,7 +837,7 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations:
|
|||
else:
|
||||
image = ""
|
||||
|
||||
cwd = overrides.get("cwd") or config["cwd"]
|
||||
cwd = overrides.get("cwd") or _last_known_cwd.get(task_id) or config["cwd"]
|
||||
logger.info("Creating new %s environment for task %s...", env_type, task_id[:8])
|
||||
|
||||
container_config = None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue