fix: recover from invalid default workspace paths (#366)
* 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 <jordan@skylinkfiber.net> Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
|
||||
103
tests/test_default_workspace_fallback.py
Normal file
103
tests/test_default_workspace_fallback.py
Normal file
@@ -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)
|
||||
Reference in New Issue
Block a user