diff --git a/strix/config/loader.py b/strix/config/loader.py index fbcde898..969d67f8 100644 --- a/strix/config/loader.py +++ b/strix/config/loader.py @@ -54,21 +54,32 @@ def apply_config_override(path: Path) -> None: def persist_current() -> None: - """Write currently-set env vars to the active config file (0o600).""" + """Merge currently-set env vars into the active config file (0o600). + + Values already in the file survive when their env var is unset, so a + run that gets its settings from the file does not erase them. An env + var set to the empty string clears the field from the file. + """ s = load_settings() target = _override or _DEFAULT_PATH target.parent.mkdir(parents=True, exist_ok=True) - env_block: dict[str, str] = {} - for sub_name in s.model_fields: + env_block = _read_env_block(target) + for sub_name in type(s).model_fields: sub_model = getattr(s, sub_name) if not isinstance(sub_model, BaseModel): continue for finfo in type(sub_model).model_fields.values(): - for alias in _aliases_for(finfo): - value = os.environ.get(alias.upper()) + aliases = [alias.upper() for alias in _aliases_for(finfo)] + set_aliases = [alias for alias in aliases if os.environ.get(alias) is not None] + if not set_aliases: + continue + for alias in aliases: + env_block.pop(alias, None) + for alias in set_aliases: + value = os.environ[alias] if value: - env_block[alias.upper()] = value + env_block[alias] = value break write_secret_text(target, json.dumps({"env": env_block}, indent=2)) @@ -93,17 +104,9 @@ def _read_json_overrides(path: Path) -> dict[str, dict[str, Any]]: Only includes keys whose env var is NOT already set, so env always wins over the persisted file. """ - if not path.exists(): + env_block_upper = _read_env_block(path) + if not env_block_upper: 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 {} - - env_block_upper = {str(k).upper(): v for k, v in env_block.items()} env_present = {k.upper() for k in os.environ} nested: dict[str, dict[str, Any]] = {} @@ -123,3 +126,17 @@ def _read_json_overrides(path: Path) -> dict[str, dict[str, Any]]: if sub_data: nested[sub_name] = sub_data return nested + + +def _read_env_block(path: Path) -> dict[str, Any]: + """Return the ``env`` block stored in ``path`` with upper-cased keys, or ``{}``.""" + 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).upper(): v for k, v in env_block.items()} diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index e83ab119..be975f0e 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -208,6 +208,82 @@ def test_persist_current_writes_env_block(tmp_path: Path, monkeypatch: pytest.Mo } +def test_persist_current_keeps_file_values_when_env_is_unset(tmp_path: Path) -> None: + target = tmp_path / "cli-config.json" + target.write_text( + json.dumps({"env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"}}), + encoding="utf-8", + ) + loader.apply_config_override(target) + assert loader.load_settings().llm.model == "file-model" + + loader.persist_current() + + assert json.loads(target.read_text(encoding="utf-8")) == { + "env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"} + } + + +def test_persist_current_env_overrides_file_value( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "cli-config.json" + target.write_text( + json.dumps({"env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"}}), + encoding="utf-8", + ) + loader.apply_config_override(target) + monkeypatch.setenv("STRIX_LLM", "env-model") + + loader.persist_current() + + assert json.loads(target.read_text(encoding="utf-8")) == { + "env": {"STRIX_LLM": "env-model", "LLM_API_KEY": "file-key"} + } + + +def test_persist_current_env_alias_replaces_other_alias_in_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "cli-config.json" + target.write_text(json.dumps({"env": {"OPENAI_API_KEY": "old-key"}}), encoding="utf-8") + loader.apply_config_override(target) + monkeypatch.setenv("LLM_API_KEY", "new-key") + + loader.persist_current() + + assert json.loads(target.read_text(encoding="utf-8")) == {"env": {"LLM_API_KEY": "new-key"}} + + +def test_persist_current_empty_env_clears_file_value( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "cli-config.json" + target.write_text( + json.dumps({"env": {"STRIX_LLM": "file-model", "PERPLEXITY_API_KEY": "pplx"}}), + encoding="utf-8", + ) + loader.apply_config_override(target) + monkeypatch.setenv("PERPLEXITY_API_KEY", "") + + loader.persist_current() + + assert json.loads(target.read_text(encoding="utf-8")) == {"env": {"STRIX_LLM": "file-model"}} + + +def test_persist_current_replaces_corrupt_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "cli-config.json" + target.write_text("{not json", encoding="utf-8") + loader.apply_config_override(target) + monkeypatch.setenv("STRIX_LLM", "env-model") + + loader.persist_current() + + assert json.loads(target.read_text(encoding="utf-8")) == {"env": {"STRIX_LLM": "env-model"}} + + def test_persist_current_sets_0600_mode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("STRIX_LLM", "persisted-model") target = tmp_path / "cli-config.json"