diff --git a/strix/config/loader.py b/strix/config/loader.py index fbcde898..1b047db1 100644 --- a/strix/config/loader.py +++ b/strix/config/loader.py @@ -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] = [] diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index e83ab119..023f9e78 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -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"