Palash DebnathandClaude Opus 4.8
1cfda2f44e
* feat(settings): configurable models directory (#64) Let users pick where model weights download (the HuggingFace / Torch cache) instead of being pinned to ~/.cache/huggingface — useful when the system drive is small or slow. Backend: - core/user_env.py: durable per-user env file (~/.config/omnivoice/env) helper with upsert/unset that preserves other keys and writes 0600. main.py already loads this at startup before importing torch/HF, so the value takes effect on the next launch. Path resolves at call time via an OMNIVOICE_ENV_FILE override so it's robust to module re-import in tests. - settings.py: GET/PUT /api/settings/storage/models-dir — validates the dir is writable (mkdir + write-probe → 400 if not), persists the choice, and writes OMNIVOICE_CACHE_DIR to the durable env. Empty path clears → reverts to default. Returns restart_required since an in-use cache can't be safely moved mid-process. Loopback-gated like the other settings. Frontend: - StoragePanel: Models tab panel to view/set/reset the directory, shows effective vs configured vs default + a restart note. Cross-platform default parity preserved (default cache path is the HF default on every OS); local-first (no network); backward-compatible (absent setting → existing behavior). No version bump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#64): harden models-dir input + clear CodeQL hygiene flags - settings.py: reject control/NUL chars in the path with a 400 before any filesystem call (an embedded NUL otherwise raised ValueError → 500). Also serves as the explicit input-validation barrier for the user-chosen path (loopback-gated same-user local file picker — no cross-privilege boundary). - test_user_env.py: use `with open(...)` so the file is closed and the assert has no side effects. - user_env.py: comment the best-effort chmod except clause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(#64): single source of truth for models dir + review fixes Address CodeRabbit + Greptile review on PR #149: - P1 (both bots): the settings_store copy of the models dir was only ever read by this GET endpoint, so it was a redundant cache that could diverge from the durable env file (the value main.py actually reads). Drop it — the per-user env file (OMNIVOICE_CACHE_DIR) is now the single source of truth: PUT writes it, GET reads it back. No divergence possible. - XDG-aware default (CodeRabbit): _default_models_dir now honors XDG_CACHE_HOME, matching huggingface_hub's real default on Linux. - Atomic 0600 write (Greptile, security): user_env writes via an os.open opener that creates the file 0600 from the start — no world-readable window before chmod for a file that can hold HF_TOKEN. - _read_lines only swallows FileNotFoundError; other OSErrors propagate so an upsert can't silently drop existing keys on a transient read failure. - Guard makedirs("") when the env path is a bare filename (no parent). - Best-effort write-probe cleanup in a finally; raise ... from e. - a11y: label the models-dir input via aria-labelledby/aria-describedby. - OS-neutral unwritable-dir test (mock makedirs) instead of Unix-only /dev/null path semantics. 12 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
65 lines
2.3 KiB
Python
65 lines
2.3 KiB
Python
"""plan-01 follow-up / #64 — durable per-user env file helper.
|
|
|
|
Backs the configurable models directory: the Settings endpoint writes
|
|
OMNIVOICE_CACHE_DIR into ~/.config/omnivoice/env, which main.py loads at startup
|
|
(→ HF_HOME / HF_HUB_CACHE / TORCH_HOME). The helper must upsert one key without
|
|
clobbering others (e.g. a persisted HF_TOKEN) and store the file 0600.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
import stat
|
|
import sys
|
|
|
|
from core import user_env
|
|
|
|
|
|
def test_set_creates_and_upserts(tmp_path):
|
|
p = str(tmp_path / "env")
|
|
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/data/models", path=p)
|
|
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR", path=p) == "/data/models"
|
|
# upsert: change value, do not duplicate the key
|
|
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/other/models", path=p)
|
|
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR", path=p) == "/other/models"
|
|
with open(p) as f:
|
|
assert f.read().count("OMNIVOICE_CACHE_DIR=") == 1
|
|
|
|
|
|
def test_preserves_other_keys(tmp_path):
|
|
p = tmp_path / "env"
|
|
p.write_text("HF_TOKEN=hf_abc123\nFOO=bar\n")
|
|
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/m", path=str(p))
|
|
txt = p.read_text()
|
|
assert "HF_TOKEN=hf_abc123" in txt
|
|
assert "FOO=bar" in txt
|
|
assert "OMNIVOICE_CACHE_DIR=/m" in txt
|
|
|
|
|
|
def test_unset_removes_only_that_key(tmp_path):
|
|
p = tmp_path / "env"
|
|
p.write_text("HF_TOKEN=hf_x\nOMNIVOICE_CACHE_DIR=/m\n")
|
|
user_env.unset_user_env("OMNIVOICE_CACHE_DIR", path=str(p))
|
|
txt = p.read_text()
|
|
assert "OMNIVOICE_CACHE_DIR" not in txt
|
|
assert "HF_TOKEN=hf_x" in txt
|
|
|
|
|
|
def test_get_missing_returns_none(tmp_path):
|
|
assert user_env.get_user_env("NOPE", path=str(tmp_path / "env")) is None
|
|
|
|
|
|
def test_set_with_bare_filename_no_parent(tmp_path, monkeypatch):
|
|
# A path with no directory component (e.g. OMNIVOICE_ENV_FILE=env) must not
|
|
# blow up: os.makedirs("") raises, so the helper has to skip the mkdir.
|
|
monkeypatch.chdir(tmp_path)
|
|
user_env.set_user_env("K", "v", path="envfile")
|
|
assert user_env.get_user_env("K", path="envfile") == "v"
|
|
|
|
|
|
def test_file_is_0600(tmp_path):
|
|
if sys.platform == "win32":
|
|
return # POSIX perms not meaningful on Windows
|
|
p = tmp_path / "env"
|
|
user_env.set_user_env("K", "v", path=str(p))
|
|
assert stat.S_IMODE(os.stat(p).st_mode) == 0o600
|