From e03c197cdfbb1a5941aaec9cb9faddfb8ecaed74 Mon Sep 17 00:00:00 2001 From: dobby-d-elf Date: Tue, 12 May 2026 05:52:16 -0600 Subject: [PATCH 1/2] fix: recover from stale deleted workspaces --- api/routes.py | 41 ++++++++++++++- api/workspace.py | 34 ++++++++---- tests/test_workspace_stale_recovery.py | 72 ++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 tests/test_workspace_stale_recovery.py diff --git a/api/routes.py b/api/routes.py index efae299c..814db72f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -6967,7 +6967,7 @@ def _handle_chat_start(handler, body, diag=None): attachments = _normalize_chat_attachments(body.get("attachments") or [])[:20] diag.stage("resolve_workspace") if diag else None try: - workspace = str(resolve_trusted_workspace(body.get("workspace") or s.workspace)) + workspace = _resolve_chat_workspace_with_recovery(s, body.get("workspace")) except ValueError as e: return bad(handler, str(e)) requested_model = body.get("model") or s.model @@ -7000,6 +7000,45 @@ def _handle_chat_start(handler, body, diag=None): +def _resolve_chat_workspace_with_recovery(s, requested_workspace) -> str: + """Resolve a chat workspace, recovering stale implicit session paths. + + If the browser explicitly sent a workspace, preserve the existing strict + validation behaviour and surface any error to the user. + + If the browser omitted ``workspace`` and the session's stored workspace now + points at a deleted directory (common after old test workspaces are cleaned + up), fall back to the current last/default workspace and persist the repair + so the chat becomes usable again. + """ + explicit = requested_workspace not in (None, "") + candidate = requested_workspace if explicit else getattr(s, "workspace", None) + try: + return str(resolve_trusted_workspace(candidate)) + except ValueError: + if explicit: + raise + fallback = str(resolve_trusted_workspace(get_last_workspace())) + stale = str(candidate or "").strip() + if stale and fallback != stale: + logger.warning( + "Recovered stale session workspace for %s: %s -> %s", + getattr(s, "session_id", "unknown"), + stale, + fallback, + ) + s.workspace = fallback + try: + s.save() + except Exception: + logger.debug( + "Failed to persist recovered workspace for session %s", + getattr(s, "session_id", "unknown"), + ) + return fallback + raise + + def _normalize_chat_attachments(raw_attachments): """Normalize attachment payloads from the browser. diff --git a/api/workspace.py b/api/workspace.py index 5ec8ec9e..70226dd0 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -20,11 +20,26 @@ logger = logging.getLogger(__name__) from api.config import ( WORKSPACES_FILE as _GLOBAL_WS_FILE, LAST_WORKSPACE_FILE as _GLOBAL_LW_FILE, - DEFAULT_WORKSPACE as _BOOT_DEFAULT_WORKSPACE, MAX_FILE_BYTES, IMAGE_EXTS, MD_EXTS ) +def _current_default_workspace() -> Path: + """Return the live default workspace from api.config. + + ``api.config.DEFAULT_WORKSPACE`` is mutable at runtime (for example after + ``save_settings()``). Importing it once into this module bakes in a stale + snapshot that can diverge from the actual current default and leak deleted + test workspaces back into live sessions. + """ + try: + from api import config as _config + + return Path(_config.DEFAULT_WORKSPACE).expanduser().resolve() + except Exception: + return Path.home().expanduser().resolve() + + # ── Profile-aware path resolution ─────────────────────────────────────────── def _profile_state_dir() -> Path: @@ -64,7 +79,7 @@ def _profile_default_workspace() -> str: 2. 'default_workspace' — alternate explicit key 3. 'terminal.cwd' — hermes-agent terminal working dir (most common) - Falls back to the boot-time DEFAULT_WORKSPACE constant. + Falls back to the live DEFAULT_WORKSPACE from api.config. """ try: from api.config import get_config @@ -86,7 +101,7 @@ def _profile_default_workspace() -> str: return str(p) except (ImportError, Exception): logger.debug("Failed to load profile default workspace config") - return str(_BOOT_DEFAULT_WORKSPACE) + return str(_current_default_workspace()) # ── Public API ────────────────────────────────────────────────────────────── @@ -427,7 +442,7 @@ def _trusted_workspace_roots() -> list[Path]: roots.append(p) add(Path.home()) - add(_BOOT_DEFAULT_WORKSPACE) + add(_current_default_workspace()) for w in load_workspaces(): add(w.get("path")) roots.sort(key=lambda p: len(str(p))) @@ -536,11 +551,10 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path: /boot, /proc, /sys, /dev, /root on Linux/macOS; Windows system dirs). This prevents even admin-saved workspaces from pointing at OS internals. - None/empty path falls back to the boot-time DEFAULT_WORKSPACE, which is always - trusted (it was validated at server startup). + None/empty path falls back to the current DEFAULT_WORKSPACE. """ if path in (None, ""): - return Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve() + return _current_default_workspace() candidate = Path(path).expanduser().resolve() @@ -571,14 +585,14 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path: except Exception: pass - # (C) Trusted if it is equal to or under the boot-time DEFAULT_WORKSPACE. + # (C) Trusted if it is equal to or under the current DEFAULT_WORKSPACE. # In Docker deployments HERMES_WEBUI_DEFAULT_WORKSPACE is often set to a # volume mount outside the user's home (e.g. /data/workspace). That path # was already validated at server startup, so any sub-path of it is safe # without requiring the user to add it to the workspace list manually. try: - boot_default = Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve() - candidate.relative_to(boot_default) + current_default = _current_default_workspace() + candidate.relative_to(current_default) return candidate except ValueError: pass diff --git a/tests/test_workspace_stale_recovery.py b/tests/test_workspace_stale_recovery.py new file mode 100644 index 00000000..4206c434 --- /dev/null +++ b/tests/test_workspace_stale_recovery.py @@ -0,0 +1,72 @@ +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from api import config as api_config +from api import routes, workspace + + +def test_profile_default_workspace_uses_live_config_default(monkeypatch, tmp_path): + live_default = tmp_path / "live-default" + live_default.mkdir() + + monkeypatch.setattr(api_config, "DEFAULT_WORKSPACE", live_default) + monkeypatch.setattr(api_config, "get_config", lambda: {}) + + assert workspace._profile_default_workspace() == str(live_default.resolve()) + assert workspace.resolve_trusted_workspace(None) == live_default.resolve() + + +def test_resolve_chat_workspace_with_recovery_repairs_missing_implicit_workspace(monkeypatch, tmp_path): + fallback = tmp_path / "fallback" + fallback.mkdir() + stale = tmp_path / "deleted-workspace" + + def fake_resolve(value): + if value == str(stale): + raise ValueError(f"Path does not exist: {stale}") + return Path(value).resolve() + + saved = {"count": 0} + + def fake_save(): + saved["count"] += 1 + + session = SimpleNamespace(session_id="sess-1", workspace=str(stale), save=fake_save) + + monkeypatch.setattr(routes, "resolve_trusted_workspace", fake_resolve) + monkeypatch.setattr(routes, "get_last_workspace", lambda: str(fallback)) + + resolved = routes._resolve_chat_workspace_with_recovery(session, None) + + assert resolved == str(fallback.resolve()) + assert session.workspace == str(fallback.resolve()) + assert saved["count"] == 1 + + +def test_resolve_chat_workspace_with_recovery_preserves_explicit_errors(monkeypatch, tmp_path): + fallback = tmp_path / "fallback" + fallback.mkdir() + stale = tmp_path / "deleted-workspace" + + def fake_resolve(value): + if value == str(stale): + raise ValueError(f"Path does not exist: {stale}") + return Path(value).resolve() + + saved = {"count": 0} + + def fake_save(): + saved["count"] += 1 + + session = SimpleNamespace(session_id="sess-2", workspace=str(fallback), save=fake_save) + + monkeypatch.setattr(routes, "resolve_trusted_workspace", fake_resolve) + monkeypatch.setattr(routes, "get_last_workspace", lambda: str(fallback)) + + with pytest.raises(ValueError, match="Path does not exist"): + routes._resolve_chat_workspace_with_recovery(session, str(stale)) + + assert session.workspace == str(fallback) + assert saved["count"] == 0 From 516d942d6a816d0e9857d5cfe79f5559cf0ba005 Mon Sep 17 00:00:00 2001 From: dobby-d-elf Date: Tue, 12 May 2026 06:28:35 -0600 Subject: [PATCH 2/2] refactor: reduce stale workspace recovery fix --- api/routes.py | 37 ++++++-------------------- api/workspace.py | 37 ++++++++++---------------- tests/test_workspace_stale_recovery.py | 1 - 3 files changed, 22 insertions(+), 53 deletions(-) diff --git a/api/routes.py b/api/routes.py index 814db72f..b28bd2f4 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7001,16 +7001,7 @@ def _handle_chat_start(handler, body, diag=None): def _resolve_chat_workspace_with_recovery(s, requested_workspace) -> str: - """Resolve a chat workspace, recovering stale implicit session paths. - - If the browser explicitly sent a workspace, preserve the existing strict - validation behaviour and surface any error to the user. - - If the browser omitted ``workspace`` and the session's stored workspace now - points at a deleted directory (common after old test workspaces are cleaned - up), fall back to the current last/default workspace and persist the repair - so the chat becomes usable again. - """ + """Recover stale implicit session workspaces without hiding explicit errors.""" explicit = requested_workspace not in (None, "") candidate = requested_workspace if explicit else getattr(s, "workspace", None) try: @@ -7018,25 +7009,13 @@ def _resolve_chat_workspace_with_recovery(s, requested_workspace) -> str: except ValueError: if explicit: raise - fallback = str(resolve_trusted_workspace(get_last_workspace())) - stale = str(candidate or "").strip() - if stale and fallback != stale: - logger.warning( - "Recovered stale session workspace for %s: %s -> %s", - getattr(s, "session_id", "unknown"), - stale, - fallback, - ) - s.workspace = fallback - try: - s.save() - except Exception: - logger.debug( - "Failed to persist recovered workspace for session %s", - getattr(s, "session_id", "unknown"), - ) - return fallback - raise + fallback = str(resolve_trusted_workspace(get_last_workspace())) + s.workspace = fallback + try: + s.save() + except Exception: + pass + return fallback def _normalize_chat_attachments(raw_attachments): diff --git a/api/workspace.py b/api/workspace.py index 70226dd0..97c768f6 100644 --- a/api/workspace.py +++ b/api/workspace.py @@ -20,26 +20,11 @@ logger = logging.getLogger(__name__) from api.config import ( WORKSPACES_FILE as _GLOBAL_WS_FILE, LAST_WORKSPACE_FILE as _GLOBAL_LW_FILE, + DEFAULT_WORKSPACE as _BOOT_DEFAULT_WORKSPACE, MAX_FILE_BYTES, IMAGE_EXTS, MD_EXTS ) -def _current_default_workspace() -> Path: - """Return the live default workspace from api.config. - - ``api.config.DEFAULT_WORKSPACE`` is mutable at runtime (for example after - ``save_settings()``). Importing it once into this module bakes in a stale - snapshot that can diverge from the actual current default and leak deleted - test workspaces back into live sessions. - """ - try: - from api import config as _config - - return Path(_config.DEFAULT_WORKSPACE).expanduser().resolve() - except Exception: - return Path.home().expanduser().resolve() - - # ── Profile-aware path resolution ─────────────────────────────────────────── def _profile_state_dir() -> Path: @@ -101,7 +86,12 @@ def _profile_default_workspace() -> str: return str(p) except (ImportError, Exception): logger.debug("Failed to load profile default workspace config") - return str(_current_default_workspace()) + try: + from api.config import DEFAULT_WORKSPACE as _LIVE_DEFAULT_WORKSPACE + + return str(Path(_LIVE_DEFAULT_WORKSPACE).expanduser().resolve()) + except Exception: + return str(Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve()) # ── Public API ────────────────────────────────────────────────────────────── @@ -442,7 +432,7 @@ def _trusted_workspace_roots() -> list[Path]: roots.append(p) add(Path.home()) - add(_current_default_workspace()) + add(_BOOT_DEFAULT_WORKSPACE) for w in load_workspaces(): add(w.get("path")) roots.sort(key=lambda p: len(str(p))) @@ -551,10 +541,11 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path: /boot, /proc, /sys, /dev, /root on Linux/macOS; Windows system dirs). This prevents even admin-saved workspaces from pointing at OS internals. - None/empty path falls back to the current DEFAULT_WORKSPACE. + None/empty path falls back to the boot-time DEFAULT_WORKSPACE, which is always + trusted (it was validated at server startup). """ if path in (None, ""): - return _current_default_workspace() + return Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve() candidate = Path(path).expanduser().resolve() @@ -585,14 +576,14 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path: except Exception: pass - # (C) Trusted if it is equal to or under the current DEFAULT_WORKSPACE. + # (C) Trusted if it is equal to or under the boot-time DEFAULT_WORKSPACE. # In Docker deployments HERMES_WEBUI_DEFAULT_WORKSPACE is often set to a # volume mount outside the user's home (e.g. /data/workspace). That path # was already validated at server startup, so any sub-path of it is safe # without requiring the user to add it to the workspace list manually. try: - current_default = _current_default_workspace() - candidate.relative_to(current_default) + boot_default = Path(_BOOT_DEFAULT_WORKSPACE).expanduser().resolve() + candidate.relative_to(boot_default) return candidate except ValueError: pass diff --git a/tests/test_workspace_stale_recovery.py b/tests/test_workspace_stale_recovery.py index 4206c434..31141525 100644 --- a/tests/test_workspace_stale_recovery.py +++ b/tests/test_workspace_stale_recovery.py @@ -15,7 +15,6 @@ def test_profile_default_workspace_uses_live_config_default(monkeypatch, tmp_pat monkeypatch.setattr(api_config, "get_config", lambda: {}) assert workspace._profile_default_workspace() == str(live_default.resolve()) - assert workspace.resolve_trusted_workspace(None) == live_default.resolve() def test_resolve_chat_workspace_with_recovery_repairs_missing_implicit_workspace(monkeypatch, tmp_path):