From d6a8d9dcab92059ce493604dea5e979f4bb8de18 Mon Sep 17 00:00:00 2001 From: Gille <4317663+helix4u@users.noreply.github.com> Date: Sun, 14 Jun 2026 22:32:53 -0600 Subject: [PATCH 1/4] fix(tools): respect session cwd in file tools --- .../tools/test_file_tools_container_config.py | 34 ++++++++++++--- tests/tools/test_file_tools_cwd_resolution.py | 24 ++++++++++- tools/file_tools.py | 43 ++++++++++++++++++- 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/tests/tools/test_file_tools_container_config.py b/tests/tools/test_file_tools_container_config.py index 54c3a60919fd..f8a79a37e4ed 100644 --- a/tests/tools/test_file_tools_container_config.py +++ b/tests/tools/test_file_tools_container_config.py @@ -27,7 +27,7 @@ def _make_env_config(**overrides): class TestFileToolsContainerConfig: - def _run(self, env_config, task_id): + def _run(self, env_config, task_id, task_env_overrides=None): captured = {} mock_env = MagicMock() @@ -35,31 +35,51 @@ class TestFileToolsContainerConfig: captured.update(kwargs) return mock_env - with patch("tools.terminal_tool._get_env_config", return_value=env_config), patch("tools.terminal_tool._task_env_overrides", {}), patch("tools.terminal_tool._active_environments", {}), patch("tools.terminal_tool._creation_locks", {}), patch("tools.terminal_tool._creation_locks_lock", __import__("threading").Lock()), patch("tools.terminal_tool._create_environment", side_effect=fake_create_env), patch("tools.terminal_tool._start_cleanup_thread"), patch("tools.terminal_tool._check_disk_usage_warning"), patch("tools.file_tools._file_ops_cache", {}), patch("tools.file_tools._file_ops_lock", __import__("threading").Lock()): + with patch("tools.terminal_tool._get_env_config", return_value=env_config), \ + patch("tools.terminal_tool._task_env_overrides", task_env_overrides or {}), \ + patch("tools.terminal_tool._active_environments", {}), \ + patch("tools.terminal_tool._creation_locks", {}), \ + patch("tools.terminal_tool._creation_locks_lock", __import__("threading").Lock()), \ + patch("tools.terminal_tool._create_environment", side_effect=fake_create_env), \ + patch("tools.terminal_tool._start_cleanup_thread"), \ + patch("tools.terminal_tool._check_disk_usage_warning"), \ + patch("tools.file_tools._file_ops_cache", {}), \ + patch("tools.file_tools._file_ops_lock", __import__("threading").Lock()): file_tools._get_file_ops(task_id) - return captured.get("container_config", {}) + return captured def test_docker_mount_cwd_to_workspace_passed(self): """docker_mount_cwd_to_workspace is forwarded to container_config.""" - cc = self._run(_make_env_config(docker_mount_cwd_to_workspace=True), "t1") + cc = self._run(_make_env_config(docker_mount_cwd_to_workspace=True), "t1").get("container_config", {}) assert cc.get("docker_mount_cwd_to_workspace") is True def test_docker_forward_env_passed(self): """docker_forward_env is forwarded to container_config.""" - cc = self._run(_make_env_config(docker_forward_env=["MY_SECRET"]), "t2") + cc = self._run(_make_env_config(docker_forward_env=["MY_SECRET"]), "t2").get("container_config", {}) assert cc.get("docker_forward_env") == ["MY_SECRET"] def test_docker_mount_cwd_defaults_to_false(self): """docker_mount_cwd_to_workspace defaults to False when absent from config.""" cfg = _make_env_config() del cfg["docker_mount_cwd_to_workspace"] - cc = self._run(cfg, "t3") + cc = self._run(cfg, "t3").get("container_config", {}) assert cc.get("docker_mount_cwd_to_workspace") is False def test_docker_forward_env_defaults_to_empty_list(self): """docker_forward_env defaults to [] when absent from config.""" cfg = _make_env_config() del cfg["docker_forward_env"] - cc = self._run(cfg, "t4") + cc = self._run(cfg, "t4").get("container_config", {}) assert cc.get("docker_forward_env") == [] + + def test_cwd_only_raw_task_override_reaches_file_environment(self): + """CWD-only task overrides collapse to default but must keep their cwd.""" + captured = self._run( + _make_env_config(env_type="local", cwd="/config-cwd"), + "desktop-session-cwd", + task_env_overrides={"desktop-session-cwd": {"cwd": "/workspace/session"}}, + ) + + assert captured["task_id"] == "default" + assert captured["cwd"] == "/workspace/session" diff --git a/tests/tools/test_file_tools_cwd_resolution.py b/tests/tools/test_file_tools_cwd_resolution.py index cad7f66f91d2..2e8356325ed7 100644 --- a/tests/tools/test_file_tools_cwd_resolution.py +++ b/tests/tools/test_file_tools_cwd_resolution.py @@ -21,6 +21,7 @@ from pathlib import Path import pytest import tools.file_tools as ft +import tools.terminal_tool as terminal_tool @pytest.fixture @@ -218,6 +219,28 @@ def test_absolute_terminal_cwd_anchors_with_empty_registry(_isolated_cwd, monkey assert not str(resolved).startswith(str(decoy)) +def test_registered_task_cwd_override_anchors_before_terminal_env_exists(_isolated_cwd, monkeypatch): + """TUI/Desktop sessions register cwd by raw session key before tools run. + + CWD-only overrides collapse to the shared terminal environment key, but the + file resolver must still read the raw task/session override before falling + back to TERMINAL_CWD or the process cwd. + """ + workspace, decoy = _isolated_cwd + task_id = "desktop-session-cwd" + monkeypatch.setattr(ft, "_get_live_tracking_cwd", lambda task_id="default": None) + monkeypatch.delenv("TERMINAL_CWD", raising=False) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + + terminal_tool.register_task_env_overrides(task_id, {"cwd": str(workspace)}) + + resolved = ft._resolve_path_for_task("target.py", task_id=task_id) + + assert terminal_tool._resolve_container_task_id(task_id) == "default" + assert resolved == (workspace / "target.py") + assert not str(resolved).startswith(str(decoy)) + + def test_warning_fires_from_terminal_cwd_when_registry_empty(_isolated_cwd, monkeypatch): """Divergence warning must fire even before any terminal command runs. @@ -291,4 +314,3 @@ def test_patch_reports_resolved_absolute_path(_isolated_cwd, monkeypatch): assert "WORKSPACE_PATCHED" in (workspace / "target.py").read_text() # And the decoy copy is untouched. assert (decoy / "target.py").read_text() == "DECOY_ORIGINAL\n" - diff --git a/tools/file_tools.py b/tools/file_tools.py index c0b2fd066287..ad73e6ad7b78 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -113,6 +113,37 @@ def _configured_terminal_cwd() -> str | None: return expanded +def _registered_task_cwd_override(task_id: str = "default") -> str | None: + """Return a registered cwd override for the raw task id, when available. + + ``terminal_tool`` intentionally collapses CWD-only task overrides to the + shared ``"default"`` environment so TUI/dashboard/ACP sessions do not spin + up isolated sandboxes just because they have different workspaces. The cwd + value itself is still keyed by the raw session/task id, so file tools must + read that raw override before falling back to the collapsed container key. + """ + try: + from tools.terminal_tool import _resolve_container_task_id, _task_env_overrides + + raw_task_id = task_id or "default" + container_key = _resolve_container_task_id(raw_task_id) + overrides = ( + _task_env_overrides.get(raw_task_id) + or _task_env_overrides.get(container_key) + or {} + ) + except Exception: + return None + + raw_cwd = str(overrides.get("cwd") or "").strip() + if raw_cwd.lower() in _TERMINAL_CWD_SENTINELS: + return None + expanded = os.path.expanduser(raw_cwd) + if not os.path.isabs(expanded): + return None + return expanded + + def _get_live_tracking_cwd(task_id: str = "default") -> str | None: """Return the task's live terminal cwd for bookkeeping when available.""" try: @@ -159,6 +190,9 @@ def _authoritative_workspace_root(task_id: str = "default") -> str | None: live = _get_live_tracking_cwd(task_id) if live: return live + registered = _registered_task_cwd_override(task_id) + if registered: + return registered return _configured_terminal_cwd() @@ -625,7 +659,8 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: ) import time - task_id = _resolve_container_task_id(task_id) + raw_task_id = task_id or "default" + task_id = _resolve_container_task_id(raw_task_id) # Fast path: check cache -- but also verify the underlying environment # is still alive (it may have been killed by the cleanup thread). @@ -662,7 +697,11 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: config = _get_env_config() env_type = config["env_type"] - overrides = _task_env_overrides.get(task_id, {}) + overrides = ( + _task_env_overrides.get(raw_task_id) + or _task_env_overrides.get(task_id) + or {} + ) if env_type == "docker": image = overrides.get("docker_image") or config["docker_image"] From ddf7c7af811394f6673cf43ae50bdbd016c72c6c Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:02:17 +0530 Subject: [PATCH 2/4] refactor(tools): consolidate task-override lookup into one helper The raw-key-first-then-collapsed override lookup was hand-rolled in three places with subtly different spellings: terminal_tool's command setup, and both file_tools._registered_task_cwd_override and _get_file_ops. Since that exact raw-vs-collapsed invariant is what the session-cwd fix depends on, keeping three copies invites the drift that caused the original bug. Add terminal_tool.resolve_task_overrides(task_id) as the single source and route all three sites through it. Behaviour is unchanged (verified byte-equivalent across raw/collapsed/isolation/None/subagent inputs). --- tools/file_tools.py | 18 ++++-------------- tools/terminal_tool.py | 40 ++++++++++++++++++++++++++-------------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/tools/file_tools.py b/tools/file_tools.py index ad73e6ad7b78..50b4b7bb8567 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -123,15 +123,9 @@ def _registered_task_cwd_override(task_id: str = "default") -> str | None: read that raw override before falling back to the collapsed container key. """ try: - from tools.terminal_tool import _resolve_container_task_id, _task_env_overrides + from tools.terminal_tool import resolve_task_overrides - raw_task_id = task_id or "default" - container_key = _resolve_container_task_id(raw_task_id) - overrides = ( - _task_env_overrides.get(raw_task_id) - or _task_env_overrides.get(container_key) - or {} - ) + overrides = resolve_task_overrides(task_id) except Exception: return None @@ -693,15 +687,11 @@ def _get_file_ops(task_id: str = "default") -> ShellFileOperations: terminal_env = None if terminal_env is None: - from tools.terminal_tool import _task_env_overrides + from tools.terminal_tool import resolve_task_overrides config = _get_env_config() env_type = config["env_type"] - overrides = ( - _task_env_overrides.get(raw_task_id) - or _task_env_overrides.get(task_id) - or {} - ) + overrides = resolve_task_overrides(raw_task_id) if env_type == "docker": image = overrides.get("docker_image") or config["docker_image"] diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index f20f2abcbb50..71907a3a3ccb 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -1034,6 +1034,26 @@ def _resolve_container_task_id(task_id: Optional[str]) -> str: return "default" +def resolve_task_overrides(task_id: Optional[str]) -> Dict[str, Any]: + """Return the env overrides for *task_id*, raw key first then collapsed. + + ``register_task_env_overrides`` writes under the *raw* task/session id, but + a CWD-only override collapses (:func:`_resolve_container_task_id`) to the + shared ``"default"`` container so per-session surfaces (ACP/gateway/ + dashboard) don't each spin up their own sandbox. Callers that need the + override (terminal command setup, file-tool cwd resolution) must therefore + read the raw id FIRST and only fall back to the collapsed container id, or + the originating session's override is silently dropped. This is the single + source of that lookup so the terminal and file layers can't drift apart. + """ + raw = task_id or "default" + return ( + _task_env_overrides.get(raw) + or _task_env_overrides.get(_resolve_container_task_id(raw)) + or {} + ) + + # Configuration from environment variables def _parse_env_var(name: str, default: str, converter: Any = int, type_label: str = "integer"): @@ -1885,20 +1905,12 @@ def terminal_tool( effective_task_id = _resolve_container_task_id(task_id) # Check per-task overrides (set by environments like TerminalBench2Env) - # before falling back to global env var config. - # - # Overrides are keyed by the *raw* task_id (that's the key - # ``register_task_env_overrides`` writes under), NOT by the collapsed - # container id. A CWD-only override collapses ``effective_task_id`` to - # ``"default"`` for container sharing, but its cwd must still be read - # back here under the originating task_id, or the override is silently - # dropped. Fall back to the collapsed id so isolation-keyed RL/benchmark - # overrides (registered under an id that equals their container id) keep - # resolving as before. - overrides = ( - (_task_env_overrides.get(task_id) if task_id else None) - or _task_env_overrides.get(effective_task_id, {}) - ) + # before falling back to global env var config. ``resolve_task_overrides`` + # reads the raw task id first then the collapsed container id, so a + # CWD-only override (which collapses ``effective_task_id`` to + # ``"default"``) is still found under its originating session id while + # isolation-keyed RL/benchmark overrides keep resolving as before. + overrides = resolve_task_overrides(task_id) # Select image based on env type, with per-task override support if env_type == "docker": From b0c99c12ddc7b6f4afcce5f776347bdf40c6e91c Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:02:54 +0530 Subject: [PATCH 3/4] docs(tools): document registered-cwd step in resolver docstrings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session-cwd fix inserted a registered task/session cwd override step between the live-cwd and $TERMINAL_CWD fallbacks, but three docstrings still described the old two-step order — _resolve_base_dir's numbered list was outright wrong. Update _authoritative_workspace_root, _resolve_base_dir, and _path_resolution_warning to reflect the actual four-step resolution order. No behaviour change. --- tools/file_tools.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tools/file_tools.py b/tools/file_tools.py index 50b4b7bb8567..47b039b46fca 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -174,8 +174,10 @@ def _authoritative_workspace_root(task_id: str = "default") -> str | None: 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 sentinel-free absolute ``$TERMINAL_CWD``. This is what lets - a worktree session warn about (and resolve into) the worktree from the very + 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. Returns ``None`` only when there is genuinely no reliable anchor, in which @@ -196,10 +198,12 @@ def _resolve_base_dir(task_id: str = "default") -> Path: Resolution order: 1. The task's live terminal cwd (the directory the agent is actually working in — e.g. a git worktree). Authoritative when known. - 2. A sentinel-free, absolute ``$TERMINAL_CWD`` (the worktree path set by + 2. A registered task/session cwd override (TUI/Desktop/ACP sessions + register a raw-keyed workspace cwd before any terminal command runs). + 3. A sentinel-free, absolute ``$TERMINAL_CWD`` (the worktree path set by ``cli.py``/``main.py`` for ``-w`` sessions). Used even before any terminal command has populated the live cwd registry. - 3. The process cwd. + 4. The process cwd. The returned base is ALWAYS absolute. This is the core invariant that prevents the worktree-cwd divergence bug: a relative or sentinel @@ -246,9 +250,10 @@ def _path_resolution_warning(filepath: str, resolved: Path, task_id: str = "defa target. ``None`` when the path is absolute, the base is unknown, or the resolved path is correctly under the workspace root. - The workspace root is the live terminal cwd when known, else a sentinel-free - absolute ``$TERMINAL_CWD`` — so a worktree session whose terminal registry - is still empty (no ``cd`` run yet) is warned on the very first write. + The workspace root is the live terminal cwd when known, else a registered + task/session cwd override, else a sentinel-free absolute ``$TERMINAL_CWD`` + — so a worktree or Desktop session whose terminal registry is still empty + (no ``cd`` run yet) is warned on the very first write. """ try: if Path(filepath).expanduser().is_absolute(): From 8fce54499fd8758a9b793b4b3c24b0dbfff14c3e Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:03:41 +0530 Subject: [PATCH 4/4] refactor(tools): extract shared sentinel-free abs cwd validator _configured_terminal_cwd and _registered_task_cwd_override carried a byte-identical sentinel + expanduser + isabs validation tail. Extract it into _sentinel_free_abs_cwd(raw) so the relative/sentinel rejection rule lives in one place. Behaviour unchanged (the str() coercion the override path relied on is preserved in the helper). --- tools/file_tools.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tools/file_tools.py b/tools/file_tools.py index 47b039b46fca..0eb7b2cb174a 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -96,6 +96,23 @@ def _resolve_path(filepath: str, task_id: str = "default") -> Path: _TERMINAL_CWD_SENTINELS = frozenset({"", ".", "./", "auto", "cwd"}) +def _sentinel_free_abs_cwd(raw: str | None) -> str | None: + """Normalize a cwd candidate to an absolute, sentinel-free anchor. + + Returns the expanded path only when *raw* is non-empty, not a sentinel (see + ``_TERMINAL_CWD_SENTINELS``), and absolute. A relative anchor is meaningless + without knowing which cwd it is relative to — exactly the ambiguity that + misroutes worktree edits — so relative/sentinel/empty values yield ``None``. + """ + raw = str(raw or "").strip() + if raw.lower() in _TERMINAL_CWD_SENTINELS: + return None + expanded = os.path.expanduser(raw) + if not os.path.isabs(expanded): + return None + return expanded + + def _configured_terminal_cwd() -> str | None: """Return ``$TERMINAL_CWD`` only when it names a real directory anchor. @@ -104,13 +121,7 @@ def _configured_terminal_cwd() -> str | None: relative to, which is exactly the ambiguity that misroutes worktree edits. Only an absolute, sentinel-free value is honored. """ - raw = (os.environ.get("TERMINAL_CWD") or "").strip() - if raw.lower() in _TERMINAL_CWD_SENTINELS: - return None - expanded = os.path.expanduser(raw) - if not os.path.isabs(expanded): - return None - return expanded + return _sentinel_free_abs_cwd(os.environ.get("TERMINAL_CWD")) def _registered_task_cwd_override(task_id: str = "default") -> str | None: @@ -129,13 +140,7 @@ def _registered_task_cwd_override(task_id: str = "default") -> str | None: except Exception: return None - raw_cwd = str(overrides.get("cwd") or "").strip() - if raw_cwd.lower() in _TERMINAL_CWD_SENTINELS: - return None - expanded = os.path.expanduser(raw_cwd) - if not os.path.isabs(expanded): - return None - return expanded + return _sentinel_free_abs_cwd(overrides.get("cwd")) def _get_live_tracking_cwd(task_id: str = "default") -> str | None: