diff --git a/tests/agent/test_file_safety_credentials.py b/tests/agent/test_file_safety_credentials.py index 4872a1f0d8..099ccc4184 100644 --- a/tests/agent/test_file_safety_credentials.py +++ b/tests/agent/test_file_safety_credentials.py @@ -144,6 +144,7 @@ def test_read_file_tool_blocks_relative_path_under_terminal_cwd( import json import tools.file_tools as ft + import tools.terminal_tool as terminal_tool _create(fake_home, "auth.json") # Force the file_tools resolver to anchor relative paths at HERMES_HOME @@ -151,7 +152,7 @@ def test_read_file_tool_blocks_relative_path_under_terminal_cwd( monkeypatch.setenv("TERMINAL_CWD", str(fake_home)) monkeypatch.chdir(tmp_path) monkeypatch.setattr( - ft, "_get_live_tracking_cwd", lambda task_id="default": None + terminal_tool, "_session_cwd", {} ) out = json.loads(ft.read_file_tool("auth.json")) @@ -166,6 +167,7 @@ def test_read_file_tool_blocks_nested_google_oauth_path( import json import tools.file_tools as ft + import tools.terminal_tool as terminal_tool oauth = _create(fake_home, Path("auth") / "google_oauth.json") oauth.write_text( @@ -180,7 +182,7 @@ def test_read_file_tool_blocks_nested_google_oauth_path( ) monkeypatch.chdir(tmp_path) monkeypatch.setattr( - ft, "_get_live_tracking_cwd", lambda task_id="default": None + terminal_tool, "_session_cwd", {} ) out = json.loads(ft.read_file_tool(str(oauth), task_id="google-oauth-test")) @@ -195,6 +197,7 @@ def test_search_tool_blocks_direct_auth_json_path(fake_home, monkeypatch): import json import tools.file_tools as ft + import tools.terminal_tool as terminal_tool auth = _create(fake_home, "auth.json") auth.write_text("SEARCH_DIRECT_AUTH_SECRET", encoding="utf-8") @@ -223,6 +226,7 @@ def test_search_tool_filters_credential_results(fake_home, tmp_path, monkeypatch from tools.file_operations import SearchMatch, SearchResult import tools.file_tools as ft + import tools.terminal_tool as terminal_tool auth = _create(fake_home, "auth.json") token = _create(fake_home, Path("mcp-tokens") / "provider.json") @@ -256,7 +260,7 @@ def test_search_tool_filters_credential_results(fake_home, tmp_path, monkeypatch monkeypatch.chdir(tmp_path) monkeypatch.setattr(ft, "_get_file_ops", lambda task_id="default": FakeFileOps()) monkeypatch.setattr( - ft, "_get_live_tracking_cwd", lambda task_id="default": None + terminal_tool, "_session_cwd", {} ) search_response = ft.search_tool( diff --git a/tests/tools/test_container_cwd_sanitize.py b/tests/tools/test_container_cwd_sanitize.py index 00a155e8bb..dc85251d07 100644 --- a/tests/tools/test_container_cwd_sanitize.py +++ b/tests/tools/test_container_cwd_sanitize.py @@ -211,7 +211,7 @@ class TestFileOpsCwdSanitizedAtCallSite: monkeypatch.setattr(tt, "_active_environments", {}) monkeypatch.setattr(tt, "_last_activity", {}) monkeypatch.setattr(ft, "_file_ops_cache", {}) - monkeypatch.setattr(ft, "_last_known_cwd", {}) + monkeypatch.setattr(tt, "_session_cwd", {}) task_id = "sess-fileops-host-cwd" tt.register_task_env_overrides(task_id, {"cwd": override_cwd}) diff --git a/tests/tools/test_file_staleness.py b/tests/tools/test_file_staleness.py index dc5a1e7bd7..31caf1d795 100644 --- a/tests/tools/test_file_staleness.py +++ b/tests/tools/test_file_staleness.py @@ -150,8 +150,8 @@ class TestStalenessCheck(unittest.TestCase): self.assertNotIn("_warning", result) @patch("tools.file_tools._get_file_ops") - def test_relative_path_uses_live_cwd_for_staleness_tracking(self, mock_ops): - """Relative-path stale tracking must follow the live terminal cwd.""" + def test_relative_path_uses_recorded_session_cwd_for_staleness_tracking(self, mock_ops): + """Relative-path stale tracking must follow the session's recorded cwd.""" start_dir = os.path.join(self._tmpdir, "start") live_dir = os.path.join(self._tmpdir, "worktree") os.makedirs(start_dir, exist_ok=True) @@ -165,15 +165,12 @@ class TestStalenessCheck(unittest.TestCase): f.write("live copy\n") fake_ops = _make_fake_ops("live copy\n", 10) - fake_ops.env = SimpleNamespace(cwd=live_dir) - fake_ops.cwd = start_dir mock_ops.return_value = fake_ops - from tools import file_tools + from tools import terminal_tool - with file_tools._file_ops_lock: - previous = file_tools._file_ops_cache.get("live_task") - file_tools._file_ops_cache["live_task"] = fake_ops + # The session cd'd into the worktree (recorded by the completed command). + terminal_tool.record_session_cwd("live_task", live_dir) try: with patch.dict(os.environ, {"TERMINAL_CWD": start_dir}, clear=False): @@ -187,11 +184,7 @@ class TestStalenessCheck(unittest.TestCase): write_file_tool("shared.txt", "replacement", task_id="live_task") ) finally: - with file_tools._file_ops_lock: - if previous is None: - file_tools._file_ops_cache.pop("live_task", None) - else: - file_tools._file_ops_cache["live_task"] = previous + terminal_tool.clear_session_cwd("live_task") self.assertIn("_warning", result) self.assertIn("modified since you last read", result["_warning"]) diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py index f57d528210..cbb0bd55cc 100644 --- a/tests/tools/test_file_tools.py +++ b/tests/tools/test_file_tools.py @@ -677,13 +677,16 @@ class TestPatchSchemaShape: # --------------------------------------------------------------------------- -# _last_known_cwd tests (#26211: silent file creation failure in long conversations) +# Session-cwd persistence across env recreation (#26211: silent file creation +# failure in long conversations). The durable anchor is the per-session cwd +# record in terminal_tool; env cleanup cannot lose it because it never lived +# on the env. # --------------------------------------------------------------------------- -class TestLastKnownCwd: +class TestSessionCwdSurvivesEnvRecreation: """ When the terminal environment is cleaned up and re-created during a long - conversation, _last_known_cwd preserves the old environment's CWD so + conversation, the session's cwd record preserves the working directory so subsequent file writes with relative paths land in the right directory. Regression guard for issue #26211. @@ -693,12 +696,12 @@ class TestLastKnownCwd: @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( + def test_recorded_cwd_used_for_recreated_env( self, mock_create_env, mock_config, mock_cache, mock_active ): - from tools.file_tools import _get_file_ops, _last_known_cwd + import tools.terminal_tool as tt + from tools.file_tools import _get_file_ops - # 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 @@ -709,42 +712,35 @@ class TestLastKnownCwd: } task_id = "default" + # The session's record holds the directory (written by the last + # completed terminal command before the env was cleaned up). + tt.record_session_cwd(task_id, "/Users/user/project") + try: + _get_file_ops(task_id) - # Preset _last_known_cwd to simulate a previous env's CWD - _last_known_cwd[task_id] = "/Users/user/project" + 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] - # Call _get_file_ops - should use _last_known_cwd for the new env - result = _get_file_ops(task_id) + assert cwd_passed == "/Users/user/project", \ + f"Expected cwd='/Users/user/project', got {cwd_passed!r}" + finally: + tt.clear_session_cwd(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( + def test_falls_back_to_config_default_when_no_record( self, mock_create_env, mock_config, mock_cache, mock_active ): - from tools.file_tools import _get_file_ops, _last_known_cwd + import tools.terminal_tool as tt + from tools.file_tools import _get_file_ops mock_env = MagicMock() mock_env.cwd = "/default/path" @@ -755,94 +751,8 @@ class TestLastKnownCwd: "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}" - - @patch("tools.terminal_tool._active_environments", new_callable=dict) - @patch("tools.file_tools._file_ops_cache", new_callable=dict) - def test_live_cwd_read_mirrors_into_last_known_cwd(self, mock_cache, mock_active): - """Belt-and-suspenders (#26211): every successful live-cwd read records - the cwd in _last_known_cwd, so the durable anchor doesn't depend on the - cleanup-detection branch of _get_file_ops firing.""" - from tools.file_tools import _get_live_tracking_cwd, _last_known_cwd - - task_id = "default" - _last_known_cwd.pop(task_id, None) - - cached = MagicMock() - cached.env = MagicMock() - cached.env.cwd = "/Users/user/project" - cached.env.cwd_owner = "default" - mock_cache[task_id] = cached - - live = _get_live_tracking_cwd(task_id) - - assert live == "/Users/user/project" - # The read mirrored the live cwd into the durable registry. - assert _last_known_cwd.get(task_id) == "/Users/user/project" - _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_mirrored_cwd_survives_when_cache_already_cleared( - self, mock_create_env, mock_config, mock_cache, mock_active - ): - """The original save-old-cwd path only fires when _file_ops_cache still - holds the stale entry. If the cleanup thread popped BOTH dicts first, - _get_file_ops sees cached=None and never saves — but the proactive - mirror from an earlier live read already populated _last_known_cwd, so - the rebuilt env still restores the user's directory.""" - from tools.file_tools import ( - _get_file_ops, _get_live_tracking_cwd, _last_known_cwd, - ) - - task_id = "default" - _last_known_cwd.pop(task_id, None) - - # 1) Env is alive and the agent has cd'd into the project. A live read - # (happens on every relative-path resolution) mirrors the cwd. - cached = MagicMock() - cached.env = MagicMock() - cached.env.cwd = "/Users/user/project" - cached.env.cwd_owner = "default" - mock_cache[task_id] = cached - assert _get_live_tracking_cwd(task_id) == "/Users/user/project" - assert _last_known_cwd.get(task_id) == "/Users/user/project" - - # 2) Cleanup thread kills the env AND clears the cache before the next - # file write — so _get_file_ops' save-old-cwd branch never runs. - mock_cache.pop(task_id, None) - mock_active.clear() - - mock_env = MagicMock() - mock_env.cwd = "/Users/user/project" - mock_create_env.return_value = mock_env - mock_config.return_value = { - "env_type": "local", - "cwd": "/config/default/path", - "timeout": 30, - } + tt.clear_session_cwd(task_id) _get_file_ops(task_id) @@ -855,10 +765,57 @@ class TestLastKnownCwd: if len(args) >= 3: cwd_passed = args[2] - # Rebuilt env restored the mirrored cwd, NOT the config default. - assert cwd_passed == "/Users/user/project", \ - f"Expected restored cwd='/Users/user/project', got {cwd_passed!r}" - _last_known_cwd.pop(task_id, None) + assert cwd_passed == "/config/default/path", \ + f"Expected cwd='/config/default/path', got {cwd_passed!r}" + + @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_stale_cache_cwd_rescued_into_record_on_cleanup_detection( + self, mock_create_env, mock_config, mock_cache, mock_active + ): + """If the env died but the file-ops cache entry survived, its cwd is + rescued into the session record before the cache entry is dropped — + the recreated env starts where the user left off.""" + import tools.terminal_tool as tt + from tools.file_tools import _get_file_ops + + task_id = "default" + tt.clear_session_cwd(task_id) + + # Stale cache entry: env was cleaned up, cache still holds the old cwd. + cached = MagicMock() + cached.env = None + cached.cwd = "/Users/user/project" + mock_cache[task_id] = cached + + mock_env = MagicMock() + mock_env.cwd = "/Users/user/project" + mock_create_env.return_value = mock_env + mock_config.return_value = { + "env_type": "local", + "cwd": "/config/default/path", + "timeout": 30, + } + + try: + _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] + + # Rebuilt env restored the rescued cwd, NOT the config default. + assert cwd_passed == "/Users/user/project", \ + f"Expected restored cwd='/Users/user/project', got {cwd_passed!r}" + finally: + tt.clear_session_cwd(task_id) class TestSilentFileMisplacementE2E: @@ -868,8 +825,8 @@ class TestSilentFileMisplacementE2E: agent cd's into a project, the cleanup thread kills the env, and a later relative-path write must land in the project dir (not the config default). Mocks miss this because resolution (_resolve_path_for_task) runs BEFORE - _get_file_ops rebuilds the env — only the durable _last_known_cwd fallback - in _authoritative_workspace_root makes the resolved path correct. + _get_file_ops rebuilds the env — only the durable session-cwd record + makes the resolved path correct. """ def test_relative_write_after_env_cleanup_lands_in_user_cwd(self, tmp_path, monkeypatch): @@ -889,13 +846,13 @@ class TestSilentFileMisplacementE2E: ) task_id = "default" - ft._last_known_cwd.pop(task_id, None) + tt.clear_session_cwd(task_id) - # 1) Env alive; agent has cd'd into the project. A relative write - # while alive mirrors the live cwd into the durable registry. + # 1) Env alive; agent has cd'd into the project (the completed command + # recorded the session cwd — simulate that write here). fo = ft._get_file_ops(task_id) fo.env.cwd = str(project) - fo.env.cwd_owner = "default" + tt.record_session_cwd(task_id, str(project)) ft.write_file_tool("alive.txt", "1\n", task_id) assert (project / "alive.txt").exists() @@ -913,4 +870,4 @@ class TestSilentFileMisplacementE2E: assert not (config_default / "report.txt").exists(), \ "file silently misplaced into config default (the #26211 bug)" - ft._last_known_cwd.pop(task_id, None) + tt.clear_session_cwd(task_id) diff --git a/tests/tools/test_file_tools_cwd_resolution.py b/tests/tools/test_file_tools_cwd_resolution.py index 530fd9690c..c333cc7d80 100644 --- a/tests/tools/test_file_tools_cwd_resolution.py +++ b/tests/tools/test_file_tools_cwd_resolution.py @@ -36,8 +36,8 @@ def _isolated_cwd(tmp_path, monkeypatch): # Process cwd = decoy, analogous to "main repo" while the terminal is in # the worktree. monkeypatch.chdir(decoy) - # No live-terminal-cwd tracking recorded yet (fresh-session condition). - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + # No session cwd recorded yet (fresh-session condition). + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) return workspace, decoy @@ -72,7 +72,7 @@ def test_live_tracking_cwd_wins_over_relative_terminal_cwd(_isolated_cwd, monkey """ workspace, decoy = _isolated_cwd monkeypatch.setenv("TERMINAL_CWD", ".") - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + terminal_tool.record_session_cwd("default", str(workspace)) resolved = ft._resolve_path_for_task("target.py", task_id="default") @@ -137,7 +137,7 @@ def test_container_relative_path_keeps_container_cwd_symlink(tmp_path, monkeypat container_mount.symlink_to(host_project, target_is_directory=True) monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: {"env_type": "docker"}) monkeypatch.setattr(terminal_tool, "_active_environments", {}) - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(container_mount)) + terminal_tool.record_session_cwd("default", str(container_mount)) resolved = ft._resolve_path_for_task("oilsands-sim/README.md", task_id="default") @@ -185,9 +185,9 @@ def test_warning_fires_when_relative_path_escapes_workspace(_isolated_cwd, monke """Relative path resolving outside the live workspace must warn.""" workspace, decoy = _isolated_cwd # Live cwd = workspace, but the relative path resolves to decoy (process cwd) - # because TERMINAL_CWD is the poison '.'. Simulate by pointing live tracking - # at workspace while the resolved path is under decoy. - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + # because TERMINAL_CWD is the poison '.'. Simulate by recording workspace + # as the session cwd while the resolved path is under decoy. + terminal_tool.record_session_cwd("default", str(workspace)) resolved_in_decoy = decoy / "target.py" warn = ft._path_resolution_warning("target.py", resolved_in_decoy, task_id="default") @@ -200,7 +200,7 @@ def test_warning_fires_when_relative_path_escapes_workspace(_isolated_cwd, monke def test_no_warning_when_relative_path_inside_workspace(_isolated_cwd, monkeypatch): workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + terminal_tool.record_session_cwd("default", str(workspace)) resolved_in_workspace = workspace / "target.py" warn = ft._path_resolution_warning("target.py", resolved_in_workspace, task_id="default") @@ -210,7 +210,7 @@ def test_no_warning_when_relative_path_inside_workspace(_isolated_cwd, monkeypat def test_no_warning_for_absolute_input(_isolated_cwd, monkeypatch): workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + terminal_tool.record_session_cwd("default", str(workspace)) warn = ft._path_resolution_warning(str(decoy / "target.py"), decoy / "target.py", task_id="default") @@ -219,7 +219,7 @@ def test_no_warning_for_absolute_input(_isolated_cwd, monkeypatch): def test_no_warning_when_no_live_cwd(_isolated_cwd, monkeypatch): workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.delenv("TERMINAL_CWD", raising=False) warn = ft._path_resolution_warning("target.py", decoy / "target.py", task_id="default") @@ -244,7 +244,7 @@ def test_sentinel_terminal_cwd_is_treated_as_unset(_isolated_cwd, monkeypatch, s never resolved as a literal relative directory. """ workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setenv("TERMINAL_CWD", sentinel) assert ft._configured_terminal_cwd() is None @@ -261,7 +261,7 @@ def test_relative_nonsentinel_terminal_cwd_rejected(_isolated_cwd, monkeypatch): be joined onto it as a literal subdir. """ workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setenv("TERMINAL_CWD", "some/rel/path") assert ft._configured_terminal_cwd() is None @@ -277,7 +277,7 @@ def test_absolute_terminal_cwd_anchors_with_empty_registry(_isolated_cwd, monkey worktree — not the process cwd (main repo). """ workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setenv("TERMINAL_CWD", str(workspace)) resolved = ft._resolve_path_for_task("target.py", task_id="default") @@ -295,7 +295,7 @@ def test_registered_task_cwd_override_anchors_before_terminal_env_exists(_isolat """ workspace, decoy = _isolated_cwd task_id = "desktop-session-cwd" - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.delenv("TERMINAL_CWD", raising=False) monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) @@ -317,7 +317,7 @@ def test_warning_fires_from_terminal_cwd_when_registry_empty(_isolated_cwd, monk worktree is flagged on the very first write. """ workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setenv("TERMINAL_CWD", str(workspace)) # Relative path that escapes the worktree into the decoy/main checkout. @@ -336,8 +336,8 @@ def test_live_cwd_still_wins_over_absolute_terminal_cwd(_isolated_cwd, monkeypat workspace, decoy = _isolated_cwd other = decoy.parent / "other" other.mkdir() - # Live cwd = workspace; TERMINAL_CWD points elsewhere — live must win. - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + # Recorded session cwd = workspace; TERMINAL_CWD points elsewhere — record wins. + terminal_tool.record_session_cwd("default", str(workspace)) monkeypatch.setenv("TERMINAL_CWD", str(other)) resolved = ft._resolve_path_for_task("target.py", task_id="default") @@ -351,7 +351,7 @@ def test_live_cwd_still_wins_over_absolute_terminal_cwd(_isolated_cwd, monkeypat def test_write_file_reports_resolved_absolute_path(_isolated_cwd, monkeypatch): """write_file_tool must put the absolute on-disk path in files_modified.""" workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + terminal_tool.record_session_cwd("t1", str(workspace)) import json out = json.loads(ft.write_file_tool("newfile.txt", "hello\n", task_id="t1")) @@ -365,7 +365,7 @@ def test_write_file_reports_resolved_absolute_path(_isolated_cwd, monkeypatch): def test_patch_reports_resolved_absolute_path(_isolated_cwd, monkeypatch): """patch_tool (replace mode) must put the absolute on-disk path in files_modified.""" workspace, decoy = _isolated_cwd - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": str(workspace)) + terminal_tool.record_session_cwd("t1", str(workspace)) import json out = json.loads(ft.patch_tool( @@ -383,24 +383,16 @@ def test_patch_reports_resolved_absolute_path(_isolated_cwd, monkeypatch): assert (decoy / "target.py").read_text() == "DECOY_ORIGINAL\n" -# ── Fix D: shared terminal env must not leak its cwd across worktree sessions ─ -# (June 2026: two desktop sessions, each on its own worktree, share the single -# "default" terminal environment. Its `cwd` tracks whichever session ran the -# last command, so a file edit from the OTHER session resolved against that -# foreign cwd and silently landed in the wrong worktree. terminal_tool now -# stamps env.cwd_owner with the driving session; file tools trust the shared -# env's live cwd only when the resolving session owns it.) - - -class _FakeOwnedEnv: - def __init__(self, cwd: str, cwd_owner: str): - self.cwd = cwd - self.cwd_owner = cwd_owner +# ── Cross-session isolation: one session's cwd never leaks into another ────── +# (June 2026 bug class: two desktop sessions, each on its own worktree, shared +# the single "default" terminal environment and could inherit each other's cwd. +# The per-session record store solves this structurally: each session's cd +# state lives in its own record, keyed by the raw session id.) @pytest.fixture def _two_worktree_sessions(tmp_path, monkeypatch): - """Two worktree sessions sharing one terminal env owned by session B.""" + """Two worktree sessions: B has cd'd (record), both registered overrides.""" wt_a = tmp_path / "wt_a" wt_b = tmp_path / "wt_b" main = tmp_path / "main" @@ -410,78 +402,60 @@ def _two_worktree_sessions(tmp_path, monkeypatch): monkeypatch.chdir(main) monkeypatch.delenv("TERMINAL_CWD", raising=False) monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setattr(ft, "_file_ops_cache", {}) - # Both sessions register their worktree cwd (TUI/desktop registration path). + # Both sessions register their worktree cwd (TUI/desktop registration path; + # registration seeds each session's record). terminal_tool.register_task_env_overrides("sess-a", {"cwd": str(wt_a)}) terminal_tool.register_task_env_overrides("sess-b", {"cwd": str(wt_b)}) - # The shared "default" env: session B ran the last command, so its live cwd - # is wt_b and B owns it. + # Session B ran the last command; the shared env's live cwd is wt_b but + # only B's RECORD carries it. monkeypatch.setattr( terminal_tool, "_active_environments", - {"default": _FakeOwnedEnv(str(wt_b), "sess-b")}, + {"default": _FakeEnv(str(wt_b))}, ) return wt_a, wt_b, main -def test_live_cwd_ignored_for_non_owning_session(_two_worktree_sessions): - wt_a, wt_b, _main = _two_worktree_sessions - # Owner sees the live cwd; the other session must NOT inherit it. - assert ft._get_live_tracking_cwd("sess-b") == str(wt_b) - assert ft._get_live_tracking_cwd("sess-a") is None +class _FakeEnv: + def __init__(self, cwd: str): + self.cwd = cwd def test_resolution_routes_to_resolving_sessions_worktree(_two_worktree_sessions): """The wrong-worktree fix: A resolves into wt_a, not the shared env's wt_b.""" wt_a, wt_b, _main = _two_worktree_sessions - # Session A does not own the shared env → falls back to its own registered - # worktree cwd instead of B's live cwd. resolved_a = ft._resolve_path_for_task("target.py", task_id="sess-a") assert resolved_a == (wt_a / "target.py") assert not str(resolved_a).startswith(str(wt_b)) -def test_owning_session_still_resolves_against_live_cwd(_two_worktree_sessions): - """No regression: the owner keeps resolving against the live cwd.""" +def test_session_with_cd_record_resolves_against_it(_two_worktree_sessions): + """B's record (its own cd state) is authoritative for B.""" wt_a, wt_b, _main = _two_worktree_sessions resolved_b = ft._resolve_path_for_task("target.py", task_id="sess-b") assert resolved_b == (wt_b / "target.py") assert not str(resolved_b).startswith(str(wt_a)) -def test_unknown_owner_keeps_prior_single_session_behavior(tmp_path, monkeypatch): - """An env with no owner (CLI / legacy) still yields its live cwd.""" - ws = tmp_path / "ws" - ws.mkdir() - monkeypatch.setattr(ft, "_file_ops_cache", {}) - monkeypatch.setattr( - terminal_tool, - "_active_environments", - {"default": _FakeOwnedEnv(str(ws), "")}, - ) - assert ft._get_live_tracking_cwd("default") == str(ws) - assert ft._get_live_tracking_cwd("any-session") == str(ws) +def test_sessions_cd_updates_only_its_own_resolution(_two_worktree_sessions, tmp_path): + """B cd's elsewhere → B's resolution follows, A's is untouched.""" + wt_a, wt_b, _main = _two_worktree_sessions + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + terminal_tool.record_session_cwd("sess-b", str(elsewhere)) + + assert ft._resolve_path_for_task("f.py", task_id="sess-b") == (elsewhere / "f.py") + assert ft._resolve_path_for_task("f.py", task_id="sess-a") == (wt_a / "f.py") -def test_preserved_cwd_does_not_override_non_owning_sessions_worktree( +def test_unregistered_session_never_inherits_another_sessions_record( _two_worktree_sessions, monkeypatch ): - """#26211 belt-and-suspenders must not break worktree isolation. - - The owner (session B) doing an owned live read mirrors wt_b into the shared - _last_known_cwd['default'] registry. Session A — which does NOT own the env - but HAS its own registered worktree (wt_a) — must still resolve into wt_a, - not inherit B's preserved cwd through the shared-container key. The - session-specific registered override must beat the durable shared anchor. - """ - wt_a, wt_b, _main = _two_worktree_sessions - monkeypatch.setattr(ft, "_last_known_cwd", {}) - - # Owner B resolves first — this mirrors wt_b into _last_known_cwd['default']. - assert ft._resolve_path_for_task("target.py", task_id="sess-b") == (wt_b / "target.py") - assert ft._last_known_cwd.get("default") == str(wt_b) - - # A still routes to its own registered worktree despite the shared anchor. - resolved_a = ft._resolve_path_for_task("target.py", task_id="sess-a") - assert resolved_a == (wt_a / "target.py") - assert not str(resolved_a).startswith(str(wt_b)) + """Session C: no record, no override. Must NOT inherit A's or B's cwd.""" + wt_a, wt_b, main = _two_worktree_sessions + resolved = ft._resolve_path_for_task("target.py", task_id="sess-c") + assert not str(resolved).startswith(str(wt_a)) + assert not str(resolved).startswith(str(wt_b)) + assert resolved == (main / "target.py").resolve() diff --git a/tests/tools/test_file_tools_tilde_profile.py b/tests/tools/test_file_tools_tilde_profile.py index fc3dadef45..003e95b797 100644 --- a/tests/tools/test_file_tools_tilde_profile.py +++ b/tests/tools/test_file_tools_tilde_profile.py @@ -21,6 +21,7 @@ from unittest.mock import patch import pytest import tools.file_tools as ft +import tools.terminal_tool as terminal_tool # --------------------------------------------------------------------------- @@ -82,7 +83,7 @@ class TestResolvePathUsesProfileHome: process_home.mkdir() monkeypatch.setenv("HOME", str(process_home)) - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) with patch("hermes_constants.get_subprocess_home", return_value=str(profile_home)): resolved = ft._resolve_path_for_task("~/test_file.txt", task_id="test") @@ -98,7 +99,7 @@ class TestResolvePathUsesProfileHome: process_home.mkdir() monkeypatch.setenv("HOME", str(process_home)) - monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) with patch("hermes_constants.get_subprocess_home", return_value=str(profile_home)): # _resolve_base_dir uses the workspace root from config; if it contains ~, diff --git a/tests/tools/test_resolve_path.py b/tests/tools/test_resolve_path.py index 4c9cbe30ab..e37ca85893 100644 --- a/tests/tools/test_resolve_path.py +++ b/tests/tools/test_resolve_path.py @@ -52,33 +52,23 @@ class TestResolvePath: assert ".." not in str(result) assert result == (tmp_path / "b" / "file.txt") - def test_relative_path_prefers_live_file_ops_cwd(self, monkeypatch, tmp_path): - """Live env.cwd must win after the terminal session changes directory.""" + def test_relative_path_prefers_recorded_session_cwd(self, monkeypatch, tmp_path): + """The session's recorded cwd must win after the terminal changes directory.""" start_dir = tmp_path / "start" live_dir = tmp_path / "worktree" start_dir.mkdir() live_dir.mkdir() monkeypatch.setenv("TERMINAL_CWD", str(start_dir)) - from tools import file_tools + from tools import file_tools, terminal_tool task_id = "live-cwd" - fake_ops = SimpleNamespace( - env=SimpleNamespace(cwd=str(live_dir)), - cwd=str(start_dir), - ) - - with file_tools._file_ops_lock: - previous = file_tools._file_ops_cache.get(task_id) - file_tools._file_ops_cache[task_id] = fake_ops + # The session's completed `cd` recorded the new directory. + terminal_tool.record_session_cwd(task_id, str(live_dir)) try: result = file_tools._resolve_path("nested/file.txt", task_id=task_id) finally: - with file_tools._file_ops_lock: - if previous is None: - file_tools._file_ops_cache.pop(task_id, None) - else: - file_tools._file_ops_cache[task_id] = previous + terminal_tool.clear_session_cwd(task_id) assert result == live_dir / "nested" / "file.txt" diff --git a/tests/tools/test_session_cwd_store.py b/tests/tools/test_session_cwd_store.py index 6db7f10b13..3cf8fdc7b6 100644 --- a/tests/tools/test_session_cwd_store.py +++ b/tests/tools/test_session_cwd_store.py @@ -127,11 +127,10 @@ class TestFileToolsReadTheRecord: 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. + # no registered overrides — just the records. tt.record_session_cwd("sess-a", str(wt_a)) tt.record_session_cwd("sess-b", str(wt_b)) @@ -139,7 +138,8 @@ class TestFileToolsReadTheRecord: 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.""" + """The leak-A scenario, solved structurally: the shared env's cwd is + never consulted for path resolution — only the session's own record.""" import tools.file_tools as ft wt_a = tmp_path / "wt_a" @@ -149,11 +149,9 @@ class TestFileToolsReadTheRecord: 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 + cwd = str(wt_b) # another session's leftover cd on the shared env monkeypatch.setattr(tt, "_active_environments", {"default": _Env()}) tt.record_session_cwd("sess-a", str(wt_a)) @@ -177,19 +175,13 @@ class TestDelegateSeedsChildRecord: class TestCommandCwdReadsTheRecord: - """Step 3: _resolve_command_cwd prefers the session's own record.""" - - def test_record_beats_foreign_env_cwd_for_commands(self): - class _Env: - cwd = "/other/sessions/worktree" - cwd_owner = "" # unowned shared env — the leak-A shape + """_resolve_command_cwd: workdir > session record > default. Nothing else.""" + def test_record_beats_default(self): tt.record_session_cwd("sess-a", "/my/worktree") resolved = tt._resolve_command_cwd( workdir=None, - env=_Env(), default_cwd="/config/default", - prev_owner="", session_key="sess-a", ) assert resolved == "/my/worktree" @@ -198,31 +190,23 @@ class TestCommandCwdReadsTheRecord: tt.record_session_cwd("sess-a", "/my/worktree") resolved = tt._resolve_command_cwd( workdir="/explicit/place", - env=None, default_cwd="/config/default", session_key="sess-a", ) assert resolved == "/explicit/place" - def test_no_record_falls_back_to_legacy_env_cwd(self): - """Transition path: session with no record keeps prior behavior.""" - class _Env: - cwd = "/live/dir" - cwd_owner = "sess-a" - + def test_no_record_falls_back_to_default(self): resolved = tt._resolve_command_cwd( workdir=None, - env=_Env(), default_cwd="/config/default", - prev_owner="sess-a", session_key="sess-a", ) - assert resolved == "/live/dir" + assert resolved == "/config/default" - def test_no_record_no_env_falls_back_to_default(self): + def test_other_sessions_record_is_not_consulted(self): + tt.record_session_cwd("sess-b", "/other/worktree") resolved = tt._resolve_command_cwd( workdir=None, - env=None, default_cwd="/config/default", session_key="sess-a", ) diff --git a/tests/tools/test_terminal_task_cwd.py b/tests/tools/test_terminal_task_cwd.py index fffe3c0bbe..fc02aa6c7d 100644 --- a/tests/tools/test_terminal_task_cwd.py +++ b/tests/tools/test_terminal_task_cwd.py @@ -76,8 +76,8 @@ def test_explicit_workdir_still_wins_over_registered_task_cwd(monkeypatch): assert calls == [{"timeout": 60, "cwd": "/explicit/workdir", "bounded_capture": True}] -def test_foreground_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch): - """A prior `cd` updates env.cwd; terminal_tool must honor that live cwd.""" +def test_foreground_command_prefers_recorded_session_cwd_over_init_time_cwd(monkeypatch): + """A prior `cd` records the session cwd; terminal_tool must honor it.""" calls = [] class FakeEnv: @@ -91,6 +91,7 @@ def test_foreground_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) task_id = "session-live-cwd" monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: FakeEnv()}) monkeypatch.setattr(terminal_tool, "_last_activity", {}) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setattr(terminal_tool, "_task_env_overrides", {task_id: {"cwd": "/workspace/init"}}) monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/workspace/init")) monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None) @@ -100,6 +101,8 @@ def test_foreground_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) "_check_all_guards", lambda command, env_type, **kwargs: {"approved": True}, ) + # The prior command's completed `cd` recorded the session cwd. + terminal_tool.record_session_cwd(task_id, "/workspace/live") result = json.loads(terminal_tool.terminal_tool(command="pwd", task_id=task_id)) @@ -107,8 +110,8 @@ def test_foreground_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) assert calls == [("pwd", {"timeout": 60, "cwd": "/workspace/live", "bounded_capture": True})] -def test_background_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch): - """Background process launches must also use the live session cwd.""" +def test_background_command_prefers_recorded_session_cwd_over_init_time_cwd(monkeypatch): + """Background process launches must also use the recorded session cwd.""" class FakeEnv: env = {} @@ -129,6 +132,7 @@ def test_background_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) task_id = "session-live-cwd-bg" monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: FakeEnv()}) monkeypatch.setattr(terminal_tool, "_last_activity", {}) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) monkeypatch.setattr(terminal_tool, "_task_env_overrides", {task_id: {"cwd": "/workspace/init"}}) monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/workspace/init")) monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None) @@ -139,6 +143,7 @@ def test_background_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) lambda command, env_type, **kwargs: {"approved": True}, ) monkeypatch.setattr(process_registry_mod, "process_registry", registry) + terminal_tool.record_session_cwd(task_id, "/workspace/live") result = json.loads( terminal_tool.terminal_tool( @@ -162,15 +167,13 @@ def test_background_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch) }] -def test_registering_cwd_override_updates_live_env_cwd(monkeypatch): +def test_registering_cwd_override_updates_session_record(monkeypatch): """An ACP ``update_cwd`` (re-)registered mid-session must win over a - previously ``cd``-ed live ``env.cwd``. + previously ``cd``-ed session cwd. - Preferring live ``env.cwd`` (so session-local ``cd`` survives) means a - freshly registered ``cwd`` override would otherwise sit *below* the - already-set ``env.cwd`` and be silently ignored. ``register_task_env_overrides`` - syncs the new cwd onto the live cached env so an explicit ACP project-root - change takes effect, as the editor client expects. + Registration writes the session record directly, so an explicit ACP + project-root change takes effect on the next command, as the editor + client expects. """ class FakeEnv: @@ -181,15 +184,18 @@ def test_registering_cwd_override_updates_live_env_cwd(monkeypatch): fake_env = FakeEnv() monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: fake_env}) monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + monkeypatch.setattr(terminal_tool, "_session_cwd", {}) + # The session had cd'd somewhere before the editor switched project roots. + terminal_tool.record_session_cwd(task_id, "/workspace/old") terminal_tool.register_task_env_overrides(task_id, {"cwd": "/workspace/new"}) - # The live env now reflects the editor's new project root. + # The live env mirror still updates (legacy env seeding) … assert fake_env.cwd == "/workspace/new" - - # A subsequent command resolves to the new cwd (env.cwd precedence). + # … and the session record — what commands actually resolve against — too. + assert terminal_tool.get_session_cwd(task_id) == "/workspace/new" assert terminal_tool._resolve_command_cwd( - workdir=None, env=fake_env, default_cwd="/workspace/config" + workdir=None, default_cwd="/workspace/config", session_key=task_id ) == "/workspace/new" @@ -264,20 +270,14 @@ def test_stale_env_cwd_from_different_session_is_ignored(monkeypatch): assert calls == [("pwd", {"timeout": 60, "cwd": "/home/user/src/hermes-agent", "bounded_capture": True})] -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. - """ +def test_same_session_recorded_cwd_survives_across_commands(monkeypatch): + """In-session `cd` state survives: the record written by one command is + used by the next command in the same session.""" calls = [] class FakeEnv: env = {} cwd = "/workspace/deep" - cwd_owner = "session-X" def execute(self, command, **kwargs): calls.append((command, kwargs)) @@ -288,6 +288,7 @@ def test_same_session_env_cwd_is_trusted_after_first_claim(monkeypatch): 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, "_session_cwd", {}) 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") @@ -297,13 +298,17 @@ def test_same_session_env_cwd_is_trusted_after_first_claim(monkeypatch): 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. + # First command runs in the config cwd (no record yet) and afterwards + # mirrors the env's post-command cwd into the session record. 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", "bounded_capture": True})] + assert calls[0] == ("pwd", {"timeout": 60, "cwd": "/workspace/config", "bounded_capture": True}) + assert terminal_tool.get_session_cwd(task_id) == "/workspace/deep" + + # Second command in the same session trusts the record. + result = json.loads(terminal_tool.terminal_tool(command="pwd", task_id=task_id)) + assert result["exit_code"] == 0 + assert calls[1] == ("pwd", {"timeout": 60, "cwd": "/workspace/deep", "bounded_capture": True}) def test_safe_getcwd_returns_real_cwd(monkeypatch): diff --git a/tools/file_tools.py b/tools/file_tools.py index 34dd4aabf2..15ddd0e365 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -269,85 +269,22 @@ def _registered_task_cwd_override(task_id: str = "default") -> str | None: return _sentinel_free_abs_cwd(overrides.get("cwd")) -def _live_cwd_if_owned(env, task_id: str) -> str | None: - """The env's live cwd, but only when THIS session owns it. - - The terminal env is shared (collapsed to the ``"default"`` container), so its - ``cwd`` tracks the LAST session that ran a command. With two worktree - sessions open, trusting it blindly routes one session's edits into the other - session's checkout (the wrong-worktree-patch bug). ``terminal_tool`` stamps - ``env.cwd_owner`` with the session that last drove the env; return its cwd - only when that owner matches the resolving session, else ``None`` so the - caller falls through to this session's own registered cwd override. Unknown - owner / ``default`` keys keep the prior behavior (single-session / CLI). - """ - if env is None: - return None - live = getattr(env, "cwd", None) - if not live: - return None - owner = str(getattr(env, "cwd_owner", "") or "") - tid = str(task_id or "") - if owner and tid and owner != "default" and tid != "default" and owner != tid: - return None - return live - - -def _get_live_tracking_cwd(task_id: str = "default") -> str | None: - """Return the task's live terminal cwd for bookkeeping when available.""" - try: - from tools.terminal_tool import _resolve_container_task_id - container_key = _resolve_container_task_id(task_id) - except Exception: - container_key = task_id - - with _file_ops_lock: - cached = _file_ops_cache.get(container_key) or _file_ops_cache.get(task_id) - if cached is not None: - env = getattr(cached, "env", None) - live_cwd = _live_cwd_if_owned(env, task_id) - if live_cwd: - _remember_last_known_cwd(container_key, live_cwd) - return live_cwd - # Legacy: a cache entry carrying its own cwd with no env to own it. - if env is None and getattr(cached, "cwd", None): - legacy_cwd = getattr(cached, "cwd", None) - _remember_last_known_cwd(container_key, legacy_cwd) - return legacy_cwd - - try: - from tools.terminal_tool import _active_environments, _env_lock - - with _env_lock: - env = _active_environments.get(container_key) or _active_environments.get(task_id) - live_cwd = _live_cwd_if_owned(env, task_id) - if live_cwd: - _remember_last_known_cwd(container_key, live_cwd) - return live_cwd - except Exception: - pass - - return None - - def _authoritative_workspace_root(task_id: str = "default") -> str | None: """Best-effort absolute workspace root for divergence checks. - Resolution (cwd rearch, step 2): + Resolution: 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. + session's resolution. 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``. + 3. A sentinel-free absolute ``$TERMINAL_CWD`` (the worktree path set by + ``cli.py``/``main.py`` for ``-w`` sessions). Returns ``None`` only when there is genuinely no reliable anchor, in which case callers fall back to the process cwd. @@ -359,32 +296,10 @@ def _authoritative_workspace_root(task_id: str = "default") -> str | None: 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 - # "default" terminal env, a NON-owning session must resolve against its OWN - # registered worktree — never the other session's leftover cwd. (Checked - # before _last_known_cwd, which is keyed by the shared container id.) 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 - # fallback so a relative-path write resolved BEFORE the env is rebuilt - # still lands in the user's directory (root cause of #26211: write happens - # via _resolve_path_for_task -> here, which runs before _get_file_ops - # rebuilds the env). Keyed by the resolved container id, same as the save. - preserved = _last_known_cwd_for(task_id) - if preserved: - return preserved return _configured_terminal_cwd() @@ -813,45 +728,6 @@ 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 = {} - - -def _remember_last_known_cwd(task_id: str, cwd: str | None) -> None: - """Mirror a live terminal cwd into the durable ``_last_known_cwd`` registry. - - Belt-and-suspenders for #26211: the cleanup thread can pop BOTH - ``_file_ops_cache`` and ``_active_environments`` before ``_get_file_ops`` - reaches its stale-cache detection branch, in which case the old cwd is - never saved and the rebuilt env falls back to the config default — exactly - the silent-misplacement bug. By recording the cwd on every successful live - read (which happens on every relative-path file resolution while the env is - alive), the durable anchor no longer depends on the cleanup-detection - branch firing, so it survives recreation regardless of pop ordering. - """ - if not cwd: - return - with _file_ops_lock: - if _last_known_cwd.get(task_id) != cwd: - _last_known_cwd[task_id] = cwd - - -def _last_known_cwd_for(task_id: str = "default") -> str | None: - """Read the durable last-known cwd for *task_id*, container-key aware. - - The registry is keyed by the resolved container id (the same key used by - the save sites in ``_get_file_ops`` / ``_get_live_tracking_cwd``), so look - up the resolved key first and fall back to the raw task id. - """ - try: - from tools.terminal_tool import _resolve_container_task_id - container_key = _resolve_container_task_id(task_id) - except Exception: - container_key = task_id - with _file_ops_lock: - return _last_known_cwd.get(container_key) or _last_known_cwd.get(task_id) # Track files read per task to detect re-read loops and deduplicate reads. # Per task_id we store: @@ -1088,13 +964,18 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: _last_activity[task_id] = time.time() return cached else: - # Environment was cleaned up -- preserve the old cwd before - # invalidating the stale cache entry (fixes #26211: silent - # file-creation failures in long-running conversations). + # Environment was cleaned up -- preserve the old cwd in the + # session record before invalidating the stale cache entry + # (fixes #26211: silent file-creation failures in long-running + # conversations). Usually a no-op: every completed command + # already recorded its cwd. old_cwd = getattr(cached, "cwd", None) if old_cwd: - with _file_ops_lock: - _last_known_cwd[task_id] = old_cwd + try: + from tools.terminal_tool import record_session_cwd + record_session_cwd(raw_task_id, old_cwd) + except Exception: + pass with _file_ops_lock: _file_ops_cache.pop(task_id, None) @@ -1132,7 +1013,12 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: else: image = "" - cwd = overrides.get("cwd") or _last_known_cwd.get(task_id) or config["cwd"] + try: + from tools.terminal_tool import get_session_cwd + recorded_cwd = get_session_cwd(raw_task_id) + except Exception: + recorded_cwd = None + cwd = overrides.get("cwd") or recorded_cwd or config["cwd"] # Re-apply the container cwd guard that _get_env_config() already # ran on config["cwd"] (see #50636). A per-task cwd override # registered by the gateway/TUI/ACP for workspace tracking is a diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index d7a64dc9ce..3eb32af274 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -1142,17 +1142,13 @@ def register_task_env_overrides(task_id: str, overrides: Dict[str, Any]): # If a live environment already exists for this task, a freshly registered # ``cwd`` override (e.g. the ACP client switching the editor's project root - # mid-session via ``session/load`` / ``session/resume``) must take effect on - # the cached env too. ``terminal_tool`` resolves the per-command cwd as - # ``workdir > env.cwd > config/override cwd`` so that ordinary in-session - # ``cd`` state is preserved; without syncing here the override would sit - # below the (already-set) ``env.cwd`` and be silently ignored once any - # command has run. Pushing it onto the live env keeps ``cd`` tracking intact - # while letting an explicit ACP cwd change win, as the client expects. + # mid-session via ``session/load`` / ``session/resume``) must take effect + # immediately. The session record is what commands resolve against; + # the live env's cwd is also updated so env-side seeding stays consistent. new_cwd = overrides.get("cwd") if isinstance(new_cwd, str) and new_cwd.strip(): - # Dual-write (cwd rearch step 1): a registered workspace cwd IS the - # session's working directory until a `cd` changes it. + # A registered workspace cwd IS the session's working directory until + # a `cd` changes it. record_session_cwd(task_id, new_cwd) # The live env is cached under the raw task_id for per-session surfaces # (ACP/gateway/dashboard) and under the collapsed container id for @@ -2042,49 +2038,21 @@ def _resolve_notification_flag_conflict( def _resolve_command_cwd( *, workdir: Optional[str], - env: Any, default_cwd: str, - prev_owner: Optional[str] = None, session_key: Optional[str] = None, ) -> str: """Return the cwd for a command. Explicit ``workdir=`` overrides everything. - Resolution (cwd rearch, step 3): - - 1. ``workdir`` — the caller said exactly where to run. - 2. The session's own cwd RECORD (``get_session_cwd(session_key)``) — - written after every completed command for this session, so it IS the - session's ``cd`` state, with no shared-env ambiguity: another - session's ``cd`` lands in another record and can't affect us. - 3. Transition-only: the shared env's live cwd, gated by the legacy - ownership guard. Only reachable for a session with no record yet - (no command completed since this code loaded). When ``prev_owner`` - differs from the current session, ``env.cwd`` is a different - session's leftover ``cd`` and falls through to ``default_cwd``. - 4. ``default_cwd`` (config/override cwd). + Otherwise the session's own cwd RECORD (``get_session_cwd``) wins — it is + written after every completed command for this session, so it IS the + session's ``cd`` state, with no shared-env ambiguity: another session's + ``cd`` lands in another record and can't affect us. A session with no + record yet (first command) runs in ``default_cwd`` (config/override cwd), + which is also what seeds a fresh environment. """ if workdir: return workdir - - recorded = get_session_cwd(session_key) - if recorded: - return recorded - - 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: - owner_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 owner_key != prev_owner: - return default_cwd - return live_cwd - - return default_cwd + return get_session_cwd(session_key) or default_cwd def terminal_tool( @@ -2173,7 +2141,7 @@ def terminal_tool( else: image = "" - cwd = overrides.get("cwd") or config["cwd"] + cwd = overrides.get("cwd") or get_session_cwd(task_id) or config["cwd"] # A per-task cwd override (registered by the gateway/TUI for workspace # tracking, or by RL/benchmark envs) wins over config["cwd"] — but # config["cwd"] was already sanitized for container backends in @@ -2435,24 +2403,13 @@ def terminal_tool( "EOF." ) - # Claim the (shared "default") terminal env for the session driving this - # command. File tools read env.cwd_owner to decide whether the env's live - # cwd is THIS session's `cd` or a different worktree session's — without - # it, two open worktree sessions sharing the env route each other's edits - # to the wrong checkout. get_current_session_key()'s contextvar doesn't - # cross tool-worker threads, so fall back to the raw task_id (which IS the - # session_key for the top-level agent) — a stable, thread-safe anchor. + # The session key that drives cwd records: get_current_session_key()'s + # contextvar doesn't cross tool-worker threads, so fall back to the raw + # task_id (which IS the session_key for the top-level agent) — a + # stable, thread-safe anchor. 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: - pass if background: # Spawn a tracked background process via the process registry. @@ -2462,9 +2419,7 @@ def terminal_tool( effective_cwd = _resolve_command_cwd( workdir=workdir, - env=env, default_cwd=cwd, - prev_owner=prev_cwd_owner, session_key=session_key, ) try: @@ -2724,9 +2679,7 @@ def terminal_tool( try: command_cwd = _resolve_command_cwd( workdir=workdir, - env=env, default_cwd=cwd, - prev_owner=prev_cwd_owner, session_key=session_key, ) execute_kwargs = {