fix(terminal): ignore stale env.cwd from a different session's cd
The terminal environment is shared process-globally (collapsed to the default key), so env.cwd tracks the LAST session that ran a command. _resolve_command_cwd() trusted env.cwd unconditionally — no ownership check — so when session A left env.cwd pointing at A's checkout, session B's first terminal command inherited A's stale cwd and ran in the wrong workspace. The file tools already solved this exact shared-env problem with _live_cwd_if_owned() checking env.cwd_owner. The terminal tool never got the same guard. Fix: capture env.cwd_owner BEFORE the current session claims it, and pass it as prev_owner to _resolve_command_cwd. When the previous owner was a different session, env.cwd is stale — fall through to default_cwd (the config/override cwd for this session) instead. Once the session has claimed the env, subsequent calls in the same session still trust env.cwd so in-session state survives.
This commit is contained in:
parent
271a9d8ec6
commit
7e84d2b5a4
2 changed files with 107 additions and 0 deletions
|
|
@ -223,6 +223,89 @@ def test_registering_non_cwd_override_leaves_live_env_cwd_untouched(monkeypatch)
|
|||
assert fake_env.cwd == "/workspace/keep"
|
||||
|
||||
|
||||
def test_stale_env_cwd_from_different_session_is_ignored(monkeypatch):
|
||||
"""A different session's `cd` left env.cwd pointing at its checkout.
|
||||
|
||||
The terminal env is shared (collapsed to "default"), so env.cwd tracks the
|
||||
LAST session that ran a command. When session B claims the env after
|
||||
session A left it in A's worktree, the first command must NOT run in A's
|
||||
leftover cwd — it must fall through to the config/override cwd (this
|
||||
session's own workspace).
|
||||
"""
|
||||
calls = []
|
||||
|
||||
class FakeEnv:
|
||||
env = {}
|
||||
cwd = "/home/user/src/hermes-desktop-tipc/apps/desktop"
|
||||
cwd_owner = "session-A-key"
|
||||
|
||||
def execute(self, command, **kwargs):
|
||||
calls.append((command, kwargs))
|
||||
return {"output": "ok", "returncode": 0}
|
||||
|
||||
task_id = "session-B"
|
||||
monkeypatch.setattr(terminal_tool, "_active_environments", {"default": FakeEnv()})
|
||||
monkeypatch.setattr(terminal_tool, "_last_activity", {})
|
||||
monkeypatch.setattr(terminal_tool, "_task_env_overrides", {})
|
||||
monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/home/user/src/hermes-agent"))
|
||||
monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None)
|
||||
monkeypatch.setattr(terminal_tool, "_resolve_container_task_id", lambda value: "default")
|
||||
monkeypatch.setattr(
|
||||
terminal_tool,
|
||||
"_check_all_guards",
|
||||
lambda command, env_type, **kwargs: {"approved": True},
|
||||
)
|
||||
|
||||
result = json.loads(terminal_tool.terminal_tool(command="pwd", task_id=task_id))
|
||||
|
||||
assert result["exit_code"] == 0
|
||||
# The command must run in the config cwd (hermes-agent), NOT the stale
|
||||
# env.cwd left by session A (hermes-desktop-tipc).
|
||||
assert calls == [("pwd", {"timeout": 60, "cwd": "/home/user/src/hermes-agent"})]
|
||||
|
||||
|
||||
def test_same_session_env_cwd_is_trusted_after_first_claim(monkeypatch):
|
||||
"""Once a session has claimed the env, subsequent commands trust env.cwd.
|
||||
|
||||
The prev_owner check only rejects env.cwd when a DIFFERENT session owned it
|
||||
before this call. After the first command (which claims ownership),
|
||||
subsequent calls in the same session should trust the live env.cwd so that
|
||||
in-session `cd` state survives.
|
||||
"""
|
||||
calls = []
|
||||
|
||||
class FakeEnv:
|
||||
env = {}
|
||||
cwd = "/workspace/deep"
|
||||
cwd_owner = "session-X"
|
||||
|
||||
def execute(self, command, **kwargs):
|
||||
calls.append((command, kwargs))
|
||||
return {"output": "ok", "returncode": 0}
|
||||
|
||||
env = FakeEnv()
|
||||
task_id = "session-X"
|
||||
monkeypatch.setattr(terminal_tool, "_active_environments", {"default": env})
|
||||
monkeypatch.setattr(terminal_tool, "_last_activity", {})
|
||||
monkeypatch.setattr(terminal_tool, "_task_env_overrides", {})
|
||||
monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/workspace/config"))
|
||||
monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None)
|
||||
monkeypatch.setattr(terminal_tool, "_resolve_container_task_id", lambda value: "default")
|
||||
monkeypatch.setattr(
|
||||
terminal_tool,
|
||||
"_check_all_guards",
|
||||
lambda command, env_type, **kwargs: {"approved": True},
|
||||
)
|
||||
|
||||
# First call: env was owned by "session-X" (same session_key since
|
||||
# get_current_session_key falls back to task_id). prev_owner == current
|
||||
# session, so env.cwd is trusted.
|
||||
result = json.loads(terminal_tool.terminal_tool(command="pwd", task_id=task_id))
|
||||
|
||||
assert result["exit_code"] == 0
|
||||
assert calls == [("pwd", {"timeout": 60, "cwd": "/workspace/deep"})]
|
||||
|
||||
|
||||
def test_safe_getcwd_returns_real_cwd(monkeypatch):
|
||||
monkeypatch.setattr(terminal_tool.os, "getcwd", lambda: "/home/user/project")
|
||||
assert terminal_tool._safe_getcwd() == "/home/user/project"
|
||||
|
|
|
|||
|
|
@ -1988,6 +1988,7 @@ def _resolve_command_cwd(
|
|||
workdir: Optional[str],
|
||||
env: Any,
|
||||
default_cwd: str,
|
||||
prev_owner: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Return the cwd for a command, preferring the live session cwd.
|
||||
|
||||
|
|
@ -1996,12 +1997,29 @@ def _resolve_command_cwd(
|
|||
new directory in ``env.cwd``, but foreground/background calls kept forcing
|
||||
the old cwd back through ``env.execute(..., cwd=...)``. Explicit
|
||||
``workdir=`` must still override everything.
|
||||
|
||||
When ``prev_owner`` is provided and differs from the current session,
|
||||
``env.cwd`` was mutated by a *different* session's ``cd`` and must NOT be
|
||||
trusted — fall through to ``default_cwd`` (the config/override cwd) so
|
||||
the command runs in this session's own workspace, not the previous
|
||||
session's leftover checkout. This mirrors the ``_live_cwd_if_owned``
|
||||
guard file_tools uses for the same shared-env problem.
|
||||
"""
|
||||
if workdir:
|
||||
return workdir
|
||||
|
||||
live_cwd = getattr(env, "cwd", None)
|
||||
if isinstance(live_cwd, str) and live_cwd.strip():
|
||||
# The env is shared (collapsed to "default"); its cwd tracks the LAST
|
||||
# session that ran a command. If a different session owned the env
|
||||
# before this call claimed it, env.cwd is that session's leftover `cd`
|
||||
# — not ours. Don't use it.
|
||||
if prev_owner is not None:
|
||||
session_key = getattr(env, "cwd_owner", "")
|
||||
# cwd_owner was already overwritten to the current session at the
|
||||
# call site, so compare against the captured previous owner.
|
||||
if prev_owner and prev_owner != "default" and session_key != prev_owner:
|
||||
return default_cwd
|
||||
return live_cwd
|
||||
|
||||
return default_cwd
|
||||
|
|
@ -2354,6 +2372,10 @@ def terminal_tool(
|
|||
from tools.approval import get_current_session_key
|
||||
|
||||
session_key = get_current_session_key(default="") or (task_id or "")
|
||||
# Capture the env's previous owner BEFORE claiming it — _resolve_command_cwd
|
||||
# needs to know whether env.cwd was left by a *different* session's `cd`
|
||||
# (in which case it's stale for this session and must be ignored).
|
||||
prev_cwd_owner = getattr(env, "cwd_owner", "") or ""
|
||||
try:
|
||||
env.cwd_owner = session_key
|
||||
except Exception:
|
||||
|
|
@ -2369,6 +2391,7 @@ def terminal_tool(
|
|||
workdir=workdir,
|
||||
env=env,
|
||||
default_cwd=cwd,
|
||||
prev_owner=prev_cwd_owner,
|
||||
)
|
||||
try:
|
||||
if env_type == "local":
|
||||
|
|
@ -2629,6 +2652,7 @@ def terminal_tool(
|
|||
workdir=workdir,
|
||||
env=env,
|
||||
default_cwd=cwd,
|
||||
prev_owner=prev_cwd_owner,
|
||||
)
|
||||
execute_kwargs = {
|
||||
"timeout": effective_timeout,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue