diff --git a/api/routes.py b/api/routes.py index 6905daba..36910521 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7533,6 +7533,7 @@ from api.workspace import ( load_workspaces, save_workspaces, get_last_workspace, + get_profile_default_workspace, set_last_workspace, git_info_for_workspace, authorize_escape_target, @@ -11264,14 +11265,14 @@ def handle_get(handler, parsed) -> bool: active_profile_name = get_active_profile_name() # Resolve the ACTIVE PROFILE's configured workspace so a cold boot with a # profile cookie shows the right composer workspace chip on a blank - # new-chat page (#5169). get_last_workspace() is already profile-scoped - # (server.py sets the per-request profile from the hermes_profile cookie - # before this handler runs) and resolves in the same priority order as - # POST /api/profile/switch. Fail open: a resolution error must never 500 - # this endpoint (boot depends on it) — the chip just falls back to the - # global default as before. + # new-chat page (#5169). Use get_profile_default_workspace() (NOT + # get_last_workspace) so a named profile without its own last_workspace.txt + # resolves to its config.yaml workspace/terminal.cwd rather than leaking the + # GLOBAL last-workspace file (the #5169 regression Codex flagged). It is + # profile-scoped via the per-request hermes_profile cookie set in server.py. + # Fail open: a resolution error must never 500 this boot-critical endpoint. try: - _profile_default_workspace = get_last_workspace() + _profile_default_workspace = get_profile_default_workspace() except Exception: logger.debug("Failed to resolve profile default workspace for /api/profile/active", exc_info=True) _profile_default_workspace = None diff --git a/api/workspace.py b/api/workspace.py index f3a82693..b56979ad 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -373,6 +373,45 @@ def save_workspaces(workspaces: list) -> None: ws_file.write_text(json.dumps(workspaces, ensure_ascii=False, indent=2), encoding='utf-8') +def get_profile_default_workspace() -> str: + """Resolve the ACTIVE PROFILE's default workspace, never the global file. + + Like get_last_workspace() but WITHOUT the global ``_GLOBAL_LW_FILE`` + fallback: for a named profile that has not yet written its own + profile-scoped ``last_workspace.txt``, that global fallback would leak the + *global* last-workspace instead of the profile's configured workspace — + which is exactly the #5169 bug (the composer chip on a blank new-chat page + showing the wrong/global workspace for a named profile). Used by + ``GET /api/profile/active`` so a cold boot under a profile cookie reflects + the profile's own configured working directory. + + Priority: profile-scoped ``last_workspace.txt`` -> profile ``config.yaml`` + ``workspace``/``default_workspace`` -> ``terminal.cwd`` -> process default. + """ + remote_cwd = _remote_terminal_cwd() + + def _valid(raw: str) -> str | None: + if not raw: + return None + if remote_cwd: + if _remote_terminal_workspace_candidate(raw) is not None: + return raw + return None + if Path(raw).is_dir(): + return raw + return None + + lw_file = _last_workspace_file() + if lw_file.exists(): + try: + p = _valid(lw_file.read_text(encoding='utf-8').strip()) + if p: + return p + except Exception: + logger.debug("Failed to read profile last workspace from %s", lw_file) + return _profile_default_workspace() + + def get_last_workspace() -> str: remote_cwd = _remote_terminal_cwd() diff --git a/tests/test_issue5169_profile_active_default_workspace.py b/tests/test_issue5169_profile_active_default_workspace.py index 48ab2116..eb9ab3ee 100644 --- a/tests/test_issue5169_profile_active_default_workspace.py +++ b/tests/test_issue5169_profile_active_default_workspace.py @@ -57,7 +57,7 @@ def test_profile_active_includes_default_workspace_from_resolver(monkeypatch): monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: "/home/u/.hermes/profiles/work") monkeypatch.setattr(profiles, "_is_root_profile", lambda name: name in ("default", "")) # The resolver is the single source of truth for the workspace value. - monkeypatch.setattr(routes, "get_last_workspace", lambda: "/srv/projects/work") + monkeypatch.setattr(routes, "get_profile_default_workspace", lambda: "/srv/projects/work") routes.handle_get(SimpleNamespace(), urlparse("/api/profile/active")) @@ -67,7 +67,7 @@ def test_profile_active_includes_default_workspace_from_resolver(monkeypatch): assert payload["is_default"] is False assert payload["default_workspace"] == "/srv/projects/work", ( "GET /api/profile/active must surface the profile-scoped workspace from " - "get_last_workspace() so the blank new-chat composer chip shows it (#5169)" + "get_profile_default_workspace() so the blank new-chat composer chip shows it (#5169)" ) @@ -159,3 +159,56 @@ def test_profile_active_default_workspace_falls_back_to_config_workspace(monkeyp "default_workspace must fall back to config.yaml `workspace` when no " "last_workspace.txt exists (mirrors POST /api/profile/switch priority, #5169)" ) + + +def test_profile_active_default_workspace_ignores_global_last_workspace(monkeypatch, tmp_path): + """The #5169 regression Codex flagged: a GLOBAL last_workspace.txt must NOT + leak into a named profile's default_workspace. + + get_last_workspace() falls back to the global _GLOBAL_LW_FILE before reaching + the profile config.yaml — so a named profile with NO own last_workspace.txt + but WITH a config.yaml `workspace` would have returned the global path, + re-introducing exactly the wrong-workspace bug. /api/profile/active now uses + get_profile_default_workspace(), which skips the global file: profile-scoped + last_workspace.txt -> config.yaml -> terminal.cwd -> default. + """ + base_home = tmp_path / ".hermes" + profile_home = base_home / "profiles" / "work" + (profile_home / "webui_state").mkdir(parents=True, exist_ok=True) + + # A GLOBAL last_workspace.txt pointing somewhere the named profile must NOT use. + global_ws = tmp_path / "global-workspace" + global_ws.mkdir(parents=True, exist_ok=True) + global_lw = tmp_path / "global-last_workspace.txt" + global_lw.write_text(str(global_ws.resolve()), encoding="utf-8") + + # The named profile's OWN configured workspace (config.yaml), which must win. + cfg_workspace = tmp_path / "work-config-workspace" + cfg_workspace.mkdir(parents=True, exist_ok=True) + resolved_cfg_ws = str(cfg_workspace.resolve()) + + captured = _capture_j(monkeypatch) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base_home) + monkeypatch.delenv("HERMES_WEBUI_ISOLATED_PROFILE", raising=False) + monkeypatch.setattr(profiles, "_INITIAL_ISOLATED_PROFILE_OPT_IN", "") + monkeypatch.setattr(workspace, "_remote_terminal_cwd", lambda: None) + # The global last-workspace file DOES exist (and is valid) — the named-profile + # resolver must still ignore it. + monkeypatch.setattr(workspace, "_GLOBAL_LW_FILE", global_lw) + # No profile-scoped last_workspace.txt on disk -> resolver consults config.yaml. + monkeypatch.setattr(config_mod, "get_config", lambda: {"workspace": resolved_cfg_ws}) + + profiles.set_request_profile("work") + try: + routes.handle_get(SimpleNamespace(), urlparse("/api/profile/active")) + finally: + profiles.clear_request_profile() + + payload = captured["payload"] + assert payload["default_workspace"] == resolved_cfg_ws, ( + "default_workspace must resolve from the named profile's config.yaml workspace, " + "NOT the global last_workspace.txt — the #5169 regression guard" + ) + assert payload["default_workspace"] != str(global_ws.resolve()), ( + "the global last-workspace path must never leak into a named profile's default_workspace" + ) diff --git a/tests/test_profile_default_workspace_823.py b/tests/test_profile_default_workspace_823.py index 59f6f940..f72150d8 100644 --- a/tests/test_profile_default_workspace_823.py +++ b/tests/test_profile_default_workspace_823.py @@ -146,7 +146,7 @@ class TestProfileActiveDefaultWorkspaceApi: monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "work") monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: "/home/.hermes/profiles/work") monkeypatch.setattr(profiles, "_is_root_profile", lambda _n: False) - monkeypatch.setattr(routes, "get_last_workspace", lambda: expected_ws) + monkeypatch.setattr(routes, "get_profile_default_workspace", lambda: expected_ws) monkeypatch.setattr( routes, "j",