mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
fix(config): preserve stored config keys not present in env during persist
persist_current() rebuilt the cli-config.json "env" block solely from os.environ and overwrote the file. Keys loaded from the file itself (most commonly STRIX_LLM) are never re-injected into os.environ, so a single run in a bare environment (fresh terminal, cron, CI, IDE runner without exported vars) silently wiped them. The next launch then failed with "no model configured". Merge instead of overwrite: read the existing "env" block first, then overlay current env vars. Env vars still win on conflicts, stored-only and schema-unknown keys are preserved, and missing/corrupt files behave as before. The 0o600 write path is unchanged. Adds regression tests covering stored-only keys, env-over-file precedence, and preservation of unknown stored keys.
This commit is contained in:
parent
6f88b7d7d5
commit
071466a9cf
2 changed files with 77 additions and 2 deletions
|
|
@ -54,12 +54,19 @@ def apply_config_override(path: Path) -> None:
|
|||
|
||||
|
||||
def persist_current() -> None:
|
||||
"""Write currently-set env vars to the active config file (0o600)."""
|
||||
"""Write currently-set env vars to the active config file (0o600).
|
||||
|
||||
The existing ``"env"`` block on disk is merged in first so keys that are
|
||||
not present in the current process environment (e.g. loaded only from the
|
||||
file itself, or written by a newer version) are preserved. Current env
|
||||
vars still win on conflicts. If the file is missing or contains invalid
|
||||
JSON, behavior is unchanged: only the env-derived block is written.
|
||||
"""
|
||||
s = load_settings()
|
||||
target = _override or _DEFAULT_PATH
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
env_block: dict[str, str] = {}
|
||||
env_block: dict[str, str] = _read_existing_env_block(target)
|
||||
for sub_name in s.model_fields:
|
||||
sub_model = getattr(s, sub_name)
|
||||
if not isinstance(sub_model, BaseModel):
|
||||
|
|
@ -74,6 +81,24 @@ def persist_current() -> None:
|
|||
write_secret_text(target, json.dumps({"env": env_block}, indent=2))
|
||||
|
||||
|
||||
def _read_existing_env_block(path: Path) -> dict[str, str]:
|
||||
"""Return the ``"env"`` block already stored in ``path``.
|
||||
|
||||
Missing files, invalid JSON, and non-dict ``"env"`` values all yield an
|
||||
empty dict so persistence degrades to the previous overwrite behavior.
|
||||
"""
|
||||
if not path.exists():
|
||||
return {}
|
||||
try:
|
||||
data = json.loads(path.read_text(encoding="utf-8"))
|
||||
except (json.JSONDecodeError, OSError):
|
||||
return {}
|
||||
env_block = data.get("env", {}) if isinstance(data, dict) else {}
|
||||
if not isinstance(env_block, dict):
|
||||
return {}
|
||||
return {str(k): str(v) for k, v in env_block.items()}
|
||||
|
||||
|
||||
def _aliases_for(finfo: FieldInfo) -> list[str]:
|
||||
"""Collect every env-var name that should populate ``finfo``."""
|
||||
aliases: list[str] = []
|
||||
|
|
|
|||
|
|
@ -216,3 +216,53 @@ def test_persist_current_sets_0600_mode(tmp_path: Path, monkeypatch: pytest.Monk
|
|||
loader.persist_current()
|
||||
|
||||
assert target.stat().st_mode & 0o777 == 0o600
|
||||
|
||||
|
||||
def test_persist_current_preserves_file_only_keys(tmp_path: Path) -> None:
|
||||
# Keys stored in the file but absent from the current environment (e.g.
|
||||
# STRIX_LLM in a bare shell/cron/CI run) must survive a persist.
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"STRIX_LLM": "file-model", "PERPLEXITY_API_KEY": "pk"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored["STRIX_LLM"] == "file-model"
|
||||
assert stored["PERPLEXITY_API_KEY"] == "pk"
|
||||
|
||||
|
||||
def test_persist_current_env_var_overrides_stored_value(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setenv("STRIX_LLM", "env-model")
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"STRIX_LLM": "file-model"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored["STRIX_LLM"] == "env-model"
|
||||
|
||||
|
||||
def test_persist_current_keeps_unknown_stored_keys(tmp_path: Path) -> None:
|
||||
# Keys outside the current schema (written by a newer version) must not
|
||||
# be dropped by an older binary rewriting the file.
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"STRIX_LLM": "file-model", "FUTURE_OPTION": "on"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored["FUTURE_OPTION"] == "on"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue