From 0e4f146918899c9fad0923c9ba41f33454a7a2d9 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Sat, 4 Jul 2026 15:20:26 -0700 Subject: [PATCH] fix(config): make env vars win over persisted JSON across all aliases _read_json_overrides is documented to let env vars outrank the persisted cli-config.json, but it decided per-alias and broke on the first alias found in either env or the file. When a multi-alias field (e.g. api_key via LLM_API_KEY/OPENAI_API_KEY) was set in the env under one alias but stored in the file under another, the stale file value was surfaced as an init kwarg and overrode the live env var. A lowercase env var was also missed (settings use case_sensitive=False). Decide whether a field is already set in the environment by checking all of its aliases case-insensitively before consulting the file. Add regression tests for the cross-alias and case-insensitive cases. Closes #688 --- strix/config/loader.py | 13 +++++++------ tests/test_config_loader.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/strix/config/loader.py b/strix/config/loader.py index 9d9911558..5b940760f 100644 --- a/strix/config/loader.py +++ b/strix/config/loader.py @@ -106,6 +106,7 @@ def _read_json_overrides(path: Path) -> dict[str, dict[str, Any]]: 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]] = {} for sub_name, sub_finfo in Settings.model_fields.items(): @@ -114,12 +115,12 @@ def _read_json_overrides(path: Path) -> dict[str, dict[str, Any]]: continue sub_data: dict[str, Any] = {} for fname, finfo in sub_cls.model_fields.items(): - for alias in _aliases_for(finfo): - key = alias.upper() - if key in os.environ: - break # env wins; skip JSON for this field - if key in env_block_upper: - sub_data[fname] = env_block_upper[key] + aliases = [alias.upper() for alias in _aliases_for(finfo)] + if any(alias in env_present for alias in aliases): + continue # env wins under some alias; skip the JSON file for this field + for alias in aliases: + if alias in env_block_upper: + sub_data[fname] = env_block_upper[alias] break if sub_data: nested[sub_name] = sub_data diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index 247c41ffe..56e644f3d 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -89,6 +89,36 @@ def test_read_json_overrides_skips_keys_already_in_environ( assert loader._read_json_overrides(path) == {} +def test_read_json_overrides_env_wins_across_field_aliases( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # api_key resolves from AliasChoices("LLM_API_KEY", "OPENAI_API_KEY"). The env + # sets one alias while the persisted file holds another. Env must still win, so + # the stale file value must not be surfaced as an init kwarg (which outranks env). + monkeypatch.setenv("OPENAI_API_KEY", "sk-env") + path = tmp_path / "cli-config.json" + path.write_text(json.dumps({"env": {"LLM_API_KEY": "sk-file"}}), encoding="utf-8") + assert loader._read_json_overrides(path) == {} + + +def test_read_json_overrides_env_wins_case_insensitively( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # Settings use case_sensitive=False, so a lowercase env var also counts as set. + monkeypatch.setenv("strix_llm", "from-env") + path = tmp_path / "cli-config.json" + path.write_text(json.dumps({"env": {"STRIX_LLM": "from-file"}}), encoding="utf-8") + assert loader._read_json_overrides(path) == {} + + +def test_read_json_overrides_uses_json_when_no_alias_in_environ(tmp_path: Path) -> None: + # No alias of api_key is set in the environment -> the file value is used, even + # when it is stored under a non-first alias. + path = tmp_path / "cli-config.json" + path.write_text(json.dumps({"env": {"OPENAI_API_KEY": "sk-file"}}), encoding="utf-8") + assert loader._read_json_overrides(path) == {"llm": {"api_key": "sk-file"}} + + # --------------------------------------------------------------------------- # # _aliases_for # --------------------------------------------------------------------------- #