From 68426124c5333389fb4ea7776cfc09978da2215c Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 13 Apr 2026 14:28:24 -0700 Subject: [PATCH] fix: recover from invalid default workspace paths (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: recover from bad default workspace paths (cherry picked from commit 789d7537a325d1c7d3aa03c387918dddd2d0897d) * fix: recover from invalid default workspace paths — 7 tests, CHANGELOG (#366) - tests/test_default_workspace_fallback.py: 5 additional tests (dedup, RuntimeError, env var priority, mkdir on missing dir, unwritable path) - CHANGELOG.md: v0.50.18 entry; 922 tests (up from 915) --------- Co-authored-by: Jordan SkyLF Co-authored-by: Nathan Esquenazi --- CHANGELOG.md | 7 ++ api/config.py | 91 ++++++++++++++++---- tests/test_default_workspace_fallback.py | 103 +++++++++++++++++++++++ 3 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 tests/test_default_workspace_fallback.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c8b42..df60a8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ --- +## [v0.50.18] Recover from invalid default workspace paths (PR #366) + +- **WebUI no longer breaks when the configured default workspace is unavailable** (`api/config.py`): The workspace resolution path was refactored into three composable functions — `_workspace_candidates()`, `_ensure_workspace_dir()`, and `resolve_default_workspace()`. When the configured workspace (from env var, settings file, or passed path) cannot be created or accessed, the server falls back through an ordered priority list: `HERMES_WEBUI_DEFAULT_WORKSPACE` env var → `~/workspace` (if exists) → `~/work` (if exists) → `~/workspace` (create it) → `STATE_DIR/workspace`. +- **`save_settings()` now validates and corrects the workspace path** (`api/config.py`): If a client posts an invalid or inaccessible `default_workspace`, the saved value is corrected to the nearest valid fallback rather than persisting an unusable path. +- **Startup normalizes stale workspace paths** (`api/config.py`): If the settings file stores a workspace that no longer exists, the server rewrites it with the resolved fallback on startup so the problem self-heals. + - 7 tests in `tests/test_default_workspace_fallback.py` (2 from PR + 5 added during review: fallback creation, RuntimeError on all-fail, deduplication, env var priority, unwritable path returns False); 922 tests total (up from 915) + ## [v0.50.17] Docker: pre-install uv at build time + fix workspace permissions (fixes #357) - **Docker containers no longer need internet access at startup** (`Dockerfile`): `uv` is now installed at image build time via `RUN curl -LsSf https://astral.sh/uv/install.sh | env UV_INSTALL_DIR=/usr/local/bin sh` (run as root, so `uv` lands in `/usr/local/bin` — accessible to all users). The init script skips the download if `uv` is already on PATH (`command -v uv`), and falls back to downloading with a proper `error_exit` if it isn't. This fixes startup failures in air-gapped, firewalled, or isolated Docker networks where `github.com` is unreachable at runtime. diff --git a/api/config.py b/api/config.py index 6d793fb..d351fc3 100644 --- a/api/config.py +++ b/api/config.py @@ -209,21 +209,70 @@ cfg = _cfg_cache # alias for backward compat with existing references # ── Default workspace discovery ─────────────────────────────────────────────── +def _workspace_candidates(raw: str | Path | None = None) -> list[Path]: + """Return ordered candidate workspace paths, de-duplicated.""" + candidates: list[Path] = [] + + def add(candidate: str | Path | None) -> None: + if candidate in (None, ""): + return + try: + path = Path(candidate).expanduser().resolve() + except Exception: + return + if path not in candidates: + candidates.append(path) + + add(raw) + if os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): + add(os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE")) + + home_workspace = HOME / "workspace" + home_work = HOME / "work" + if home_workspace.exists(): + add(home_workspace) + if home_work.exists(): + add(home_work) + + add(home_workspace) + add(STATE_DIR / "workspace") + return candidates + + + +def _ensure_workspace_dir(path: Path) -> bool: + """Best-effort check that a workspace directory exists and is writable.""" + try: + path = path.expanduser().resolve() + path.mkdir(parents=True, exist_ok=True) + return path.is_dir() and os.access(path, os.R_OK | os.W_OK | os.X_OK) + except Exception: + return False + + + +def resolve_default_workspace(raw: str | Path | None = None) -> Path: + """Return the first usable workspace path, creating it when possible.""" + for candidate in _workspace_candidates(raw): + if _ensure_workspace_dir(candidate): + return candidate + raise RuntimeError( + "Could not create or access any usable workspace directory. " + "Set HERMES_WEBUI_DEFAULT_WORKSPACE to a writable path." + ) + + + def _discover_default_workspace() -> Path: """ Resolve the default workspace in order: 1. HERMES_WEBUI_DEFAULT_WORKSPACE env var - 2. ~/workspace (common Hermes convention) - 3. STATE_DIR / workspace (isolated fallback) + 2. ~/workspace if it already exists + 3. ~/work if it already exists + 4. ~/workspace (create if needed) + 5. STATE_DIR / workspace """ - if os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): - return Path(os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE")).expanduser().resolve() - - common = HOME / "workspace" - if common.exists(): - return common.resolve() - - return (STATE_DIR / "workspace").resolve() + return resolve_default_workspace() DEFAULT_WORKSPACE = _discover_default_workspace() @@ -1080,6 +1129,10 @@ def save_settings(settings: dict) -> dict: if k in _SETTINGS_BOOL_KEYS: v = bool(v) current[k] = v + + current["default_workspace"] = str( + resolve_default_workspace(current.get("default_workspace")) + ) SETTINGS_FILE.write_text( json.dumps(current, ensure_ascii=False, indent=2), encoding="utf-8", @@ -1089,7 +1142,7 @@ def save_settings(settings: dict) -> dict: if "default_model" in current: DEFAULT_MODEL = current["default_model"] if "default_workspace" in current: - DEFAULT_WORKSPACE = Path(current["default_workspace"]).expanduser().resolve() + DEFAULT_WORKSPACE = resolve_default_workspace(current["default_workspace"]) return current @@ -1098,10 +1151,18 @@ _startup_settings = load_settings() if SETTINGS_FILE.exists(): if _startup_settings.get("default_model"): DEFAULT_MODEL = _startup_settings["default_model"] - if _startup_settings.get("default_workspace"): - DEFAULT_WORKSPACE = ( - Path(_startup_settings["default_workspace"]).expanduser().resolve() - ) + DEFAULT_WORKSPACE = resolve_default_workspace( + _startup_settings.get("default_workspace") + ) + if _startup_settings.get("default_workspace") != str(DEFAULT_WORKSPACE): + _startup_settings["default_workspace"] = str(DEFAULT_WORKSPACE) + try: + SETTINGS_FILE.write_text( + json.dumps(_startup_settings, ensure_ascii=False, indent=2), + encoding="utf-8", + ) + except Exception: + pass # ── SESSIONS in-memory cache (LRU OrderedDict) ─────────────────────────────── SESSIONS: collections.OrderedDict = collections.OrderedDict() diff --git a/tests/test_default_workspace_fallback.py b/tests/test_default_workspace_fallback.py new file mode 100644 index 0000000..2bb3329 --- /dev/null +++ b/tests/test_default_workspace_fallback.py @@ -0,0 +1,103 @@ +import json +from pathlib import Path + +import api.config as config + + +def test_resolve_default_workspace_falls_back_to_existing_home_work(monkeypatch, tmp_path): + preferred = tmp_path / "work" + preferred.mkdir() + state_dir = tmp_path / "state" + + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + + resolved = config.resolve_default_workspace("/definitely/not/usable") + + assert resolved == preferred.resolve() + + + +def test_save_settings_rewrites_bad_default_workspace_to_fallback(monkeypatch, tmp_path): + preferred = tmp_path / "work" + preferred.mkdir() + state_dir = tmp_path / "state" + settings_file = tmp_path / "settings.json" + + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + monkeypatch.setattr(config, "SETTINGS_FILE", settings_file) + monkeypatch.setattr(config, "DEFAULT_WORKSPACE", preferred) + + saved = config.save_settings({"default_workspace": "/definitely/not/usable"}) + on_disk = json.loads(settings_file.read_text(encoding="utf-8")) + + assert saved["default_workspace"] == str(preferred.resolve()) + assert on_disk["default_workspace"] == str(preferred.resolve()) + + +def test_resolve_default_workspace_creates_home_workspace_when_missing(monkeypatch, tmp_path): + """When no preferred dir exists, resolve falls back to creating ~/workspace.""" + state_dir = tmp_path / "state" + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + # Neither ~/work nor ~/workspace exists yet + resolved = config.resolve_default_workspace(None) + assert resolved == (tmp_path / "workspace").resolve() + assert resolved.is_dir() + + +def test_resolve_default_workspace_raises_when_all_candidates_fail(monkeypatch, tmp_path): + """RuntimeError is raised when every candidate is unwritable.""" + import stat, pytest + # Make tmp_path read-only so mkdir inside it fails + tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR) + state_dir = tmp_path / "state" + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + monkeypatch.delenv("HERMES_WEBUI_DEFAULT_WORKSPACE", raising=False) + try: + with pytest.raises(RuntimeError, match="Could not create or access"): + config.resolve_default_workspace(None) + finally: + tmp_path.chmod(stat.S_IRWXU) # restore for cleanup + + +def test_workspace_candidates_deduplicates_home_workspace(monkeypatch, tmp_path): + """~/workspace must appear at most once in the candidates list even if it exists.""" + ws = tmp_path / "workspace" + ws.mkdir() + state_dir = tmp_path / "state" + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + monkeypatch.delenv("HERMES_WEBUI_DEFAULT_WORKSPACE", raising=False) + candidates = config._workspace_candidates(None) + paths = [str(p) for p in candidates] + assert paths.count(str(ws.resolve())) <= 1, "~/workspace must not appear twice" + + +def test_env_var_workspace_takes_priority_over_passed_raw(monkeypatch, tmp_path): + """HERMES_WEBUI_DEFAULT_WORKSPACE env var overrides a None raw arg but not a valid one.""" + env_ws = tmp_path / "env_workspace" + env_ws.mkdir() + state_dir = tmp_path / "state" + monkeypatch.setattr(config, "HOME", tmp_path) + monkeypatch.setattr(config, "STATE_DIR", state_dir) + monkeypatch.setenv("HERMES_WEBUI_DEFAULT_WORKSPACE", str(env_ws)) + # When raw is None, env var should be used + resolved = config.resolve_default_workspace(None) + assert resolved == env_ws.resolve() + + +def test_ensure_workspace_dir_returns_false_for_unwritable_path(monkeypatch, tmp_path): + """_ensure_workspace_dir returns False for a path that can't be created.""" + import stat + # Make parent read-only so mkdir fails + parent = tmp_path / "ro_parent" + parent.mkdir() + parent.chmod(stat.S_IRUSR | stat.S_IXUSR) + try: + result = config._ensure_workspace_dir(parent / "child") + assert result is False + finally: + parent.chmod(stat.S_IRWXU)