fix(config): make env vars win over persisted JSON across all aliases (#689)
_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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
Reference in New Issue
Block a user