mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
refactor(file-tools): resolve paths against per-session cwd records (step 2)
Flips the read side of the cwd rearchitecture onto the _session_cwd
store introduced in the previous commit.
_authoritative_workspace_root now resolves:
1. the session's own cwd record (get_session_cwd) — per-session by
construction, so one session's cd can never leak into another
session's file resolution, with no ownership heuristics at all
2. registered override (fallback for cleared/never-written records)
3. legacy shared-env live cwd + preserved anchor (transition-only,
for commands that ran before this code loaded)
4. sentinel-free absolute TERMINAL_CWD
delegate_task children get their record seeded from the parent's at
spawn: they keep starting in the parent's directory (current behavior)
but their subsequent cds stay isolated in their own record instead of
bleeding back through the shared env.
The wrong-worktree leak class is now solved structurally on this path —
there is no shared cwd for sessions to inherit. The legacy env-side
tracking (cwd_owner, _live_cwd_if_owned, _last_known_cwd) remains only
as a transition fallback and is deleted in the next step.
This commit is contained in:
parent
be2a1290de
commit
5461e0e098
3 changed files with 103 additions and 10 deletions
|
|
@ -112,3 +112,65 @@ class TestPostCommandDualWrite:
|
|||
result = self._run(monkeypatch, "sess-a", FakeEnv())
|
||||
assert result["exit_code"] == 0
|
||||
assert tt.get_session_cwd("sess-a") is None
|
||||
|
||||
|
||||
class TestFileToolsReadTheRecord:
|
||||
"""Step 2: file-tool path resolution prefers the session's own record."""
|
||||
|
||||
def test_two_sessions_resolve_into_their_own_recorded_cwds(self, tmp_path, monkeypatch):
|
||||
import tools.file_tools as ft
|
||||
|
||||
wt_a = tmp_path / "wt_a"
|
||||
wt_b = tmp_path / "wt_b"
|
||||
for d in (wt_a, wt_b):
|
||||
d.mkdir()
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.delenv("TERMINAL_CWD", raising=False)
|
||||
monkeypatch.setattr(ft, "_file_ops_cache", {})
|
||||
monkeypatch.setattr(ft, "_last_known_cwd", {})
|
||||
monkeypatch.setattr(tt, "_active_environments", {})
|
||||
|
||||
# Each session ran commands that recorded its own cwd. No env alive,
|
||||
# no ownership metadata, no registered overrides — just the records.
|
||||
tt.record_session_cwd("sess-a", str(wt_a))
|
||||
tt.record_session_cwd("sess-b", str(wt_b))
|
||||
|
||||
assert ft._resolve_path_for_task("f.py", task_id="sess-a") == (wt_a / "f.py")
|
||||
assert ft._resolve_path_for_task("f.py", task_id="sess-b") == (wt_b / "f.py")
|
||||
|
||||
def test_record_beats_foreign_env_cwd_without_ownership_metadata(self, tmp_path, monkeypatch):
|
||||
"""The leak-A scenario, solved structurally: no cwd_owner consulted."""
|
||||
import tools.file_tools as ft
|
||||
|
||||
wt_a = tmp_path / "wt_a"
|
||||
wt_b = tmp_path / "wt_b"
|
||||
for d in (wt_a, wt_b):
|
||||
d.mkdir()
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.delenv("TERMINAL_CWD", raising=False)
|
||||
monkeypatch.setattr(ft, "_file_ops_cache", {})
|
||||
monkeypatch.setattr(ft, "_last_known_cwd", {})
|
||||
|
||||
class _Env:
|
||||
cwd = str(wt_b)
|
||||
cwd_owner = "" # unowned — the case the legacy guard let through
|
||||
|
||||
monkeypatch.setattr(tt, "_active_environments", {"default": _Env()})
|
||||
tt.record_session_cwd("sess-a", str(wt_a))
|
||||
|
||||
resolved = ft._resolve_path_for_task("f.py", task_id="sess-a")
|
||||
assert resolved == (wt_a / "f.py")
|
||||
assert not str(resolved).startswith(str(wt_b))
|
||||
|
||||
|
||||
class TestDelegateSeedsChildRecord:
|
||||
def test_child_record_seeded_from_parent_then_isolated(self):
|
||||
tt.record_session_cwd("parent-task", "/parent/worktree")
|
||||
# what delegate_tool does at spawn:
|
||||
tt.record_session_cwd("child-1", tt.get_session_cwd("parent-task"))
|
||||
|
||||
assert tt.get_session_cwd("child-1") == "/parent/worktree"
|
||||
# child cds somewhere; parent record must be untouched.
|
||||
tt.record_session_cwd("child-1", "/child/scratch")
|
||||
assert tt.get_session_cwd("parent-task") == "/parent/worktree"
|
||||
assert tt.get_session_cwd("child-1") == "/child/scratch"
|
||||
|
|
|
|||
|
|
@ -1906,6 +1906,18 @@ def _run_single_child(
|
|||
|
||||
child_task_id = _subagent_id or f"subagent-{task_index}-{_uuid.uuid4().hex[:8]}"
|
||||
parent_task_id = getattr(parent_agent, "_current_task_id", None)
|
||||
# Seed the child's session-cwd record from the parent's (cwd rearch):
|
||||
# children share the parent's container, and today they inherit the
|
||||
# parent's live env.cwd implicitly. Seeding at spawn preserves that
|
||||
# starting directory while keeping the child's subsequent `cd`s
|
||||
# isolated in its own record (a child's cd no longer bleeds back into
|
||||
# the parent once readers flip to the record store).
|
||||
try:
|
||||
from tools.terminal_tool import get_session_cwd, record_session_cwd
|
||||
|
||||
record_session_cwd(child_task_id, get_session_cwd(parent_task_id))
|
||||
except Exception as e:
|
||||
logger.debug("Child cwd seed failed: %s", e)
|
||||
wall_start = time.time()
|
||||
parent_reads_snapshot = (
|
||||
list(file_state.known_reads(parent_task_id)) if parent_task_id else []
|
||||
|
|
|
|||
|
|
@ -333,20 +333,36 @@ def _get_live_tracking_cwd(task_id: str = "default") -> str | None:
|
|||
def _authoritative_workspace_root(task_id: str = "default") -> str | None:
|
||||
"""Best-effort absolute workspace root for divergence checks.
|
||||
|
||||
Prefers the live terminal cwd (the directory the agent is actually working
|
||||
in). When no terminal command has run yet — so the live registry is empty —
|
||||
falls back to a registered task/session cwd override (TUI/Desktop/ACP
|
||||
sessions register a raw-keyed cwd before any tool runs), then to a
|
||||
sentinel-free absolute ``$TERMINAL_CWD``. This is what lets a worktree or
|
||||
Desktop session warn about (and resolve into) its workspace from the very
|
||||
first ``write_file``/``patch``, before any ``cd`` has populated the live cwd.
|
||||
Resolution (cwd rearch, step 2):
|
||||
|
||||
1. The session's own cwd RECORD (``terminal_tool.get_session_cwd``) —
|
||||
written on every completed terminal command and seeded by workspace
|
||||
registration, keyed by the raw session id. Because the record is
|
||||
per-session, one session's ``cd`` can never leak into another
|
||||
session's resolution — the property the legacy env-side tracking
|
||||
(shared ``env.cwd`` + ownership stamping) could not guarantee.
|
||||
2. A registered task/session cwd override (TUI/Desktop/ACP sessions
|
||||
register a raw-keyed cwd before any tool runs). Normally already
|
||||
mirrored into the record at registration; kept as a direct fallback
|
||||
so a cleared/never-written record still resolves the workspace.
|
||||
3. Legacy shared-env live cwd + preserved anchor (transition-only:
|
||||
single-session flows whose commands ran before this code loaded).
|
||||
4. A sentinel-free absolute ``$TERMINAL_CWD``.
|
||||
|
||||
Returns ``None`` only when there is genuinely no reliable anchor, in which
|
||||
case callers fall back to the process cwd.
|
||||
"""
|
||||
live = _get_live_tracking_cwd(task_id)
|
||||
if live:
|
||||
return live
|
||||
try:
|
||||
from tools.terminal_tool import get_session_cwd
|
||||
|
||||
recorded = get_session_cwd(task_id)
|
||||
except Exception:
|
||||
recorded = None
|
||||
if recorded:
|
||||
# Keep the legacy mirror warm for the transition (readers of
|
||||
# _last_known_cwd still exist until step 4 deletes them).
|
||||
_get_live_tracking_cwd(task_id)
|
||||
return recorded
|
||||
# A session-specific registered override (TUI/Desktop/ACP workspace cwd)
|
||||
# is more authoritative than the shared last-known anchor: it is keyed by
|
||||
# the raw session id, so when two worktree sessions share the single
|
||||
|
|
@ -356,6 +372,9 @@ def _authoritative_workspace_root(task_id: str = "default") -> str | None:
|
|||
registered = _registered_task_cwd_override(task_id)
|
||||
if registered:
|
||||
return registered
|
||||
live = _get_live_tracking_cwd(task_id)
|
||||
if live:
|
||||
return live
|
||||
# When the terminal env was cleaned up mid-conversation, the live cwd is
|
||||
# gone but the directory the agent navigated to is still recorded in the
|
||||
# durable _last_known_cwd registry. Prefer it over the config/process
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue