mirror of
https://github.com/usestrix/strix.git
synced 2026-08-28 05:25:00 +00:00
fix(config): drop stale sibling aliases and keep stored value types on persist
Address Greptile review on #1131: - When an env var sets one alias of a field, remove the field's other aliases from the merged block. Otherwise a stale stored LLM_API_KEY would shadow a freshly persisted OPENAI_API_KEY on the next load, since _read_json_overrides() resolves aliases in priority order. - Preserve stored env values verbatim instead of coercing through str(), so structured values written by newer versions (e.g. a JSON object of headers) survive the rewrite. Adds regression tests for stale sibling-alias removal and non-string value preservation.
This commit is contained in:
parent
071466a9cf
commit
67798f6a2c
2 changed files with 53 additions and 8 deletions
|
|
@ -66,26 +66,34 @@ def persist_current() -> None:
|
|||
target = _override or _DEFAULT_PATH
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
env_block: dict[str, str] = _read_existing_env_block(target)
|
||||
env_block: dict[str, Any] = _read_existing_env_block(target)
|
||||
for sub_name in 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)]
|
||||
for alias in aliases:
|
||||
value = os.environ.get(alias)
|
||||
if value:
|
||||
env_block[alias.upper()] = value
|
||||
# Drop sibling aliases of this field so a stale stored
|
||||
# value (e.g. LLM_API_KEY) can't shadow the fresh one on
|
||||
# the next load.
|
||||
for key in [k for k in env_block if k.upper() in aliases]:
|
||||
del env_block[key]
|
||||
env_block[alias] = value
|
||||
break
|
||||
|
||||
write_secret_text(target, json.dumps({"env": env_block}, indent=2))
|
||||
|
||||
|
||||
def _read_existing_env_block(path: Path) -> dict[str, str]:
|
||||
def _read_existing_env_block(path: Path) -> dict[str, Any]:
|
||||
"""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.
|
||||
Values are preserved verbatim so structured entries written by newer
|
||||
versions survive the rewrite. 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 {}
|
||||
|
|
@ -96,7 +104,7 @@ def _read_existing_env_block(path: Path) -> dict[str, str]:
|
|||
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()}
|
||||
return {str(k): v for k, v in env_block.items()}
|
||||
|
||||
|
||||
def _aliases_for(finfo: FieldInfo) -> list[str]:
|
||||
|
|
|
|||
|
|
@ -266,3 +266,40 @@ def test_persist_current_keeps_unknown_stored_keys(tmp_path: Path) -> None:
|
|||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored["FUTURE_OPTION"] == "on"
|
||||
|
||||
|
||||
def test_persist_current_removes_stale_sibling_alias(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
# api_key resolves from AliasChoices("LLM_API_KEY", "OPENAI_API_KEY"). If the
|
||||
# file holds the first alias while the env sets the second, keeping both would
|
||||
# let the stale LLM_API_KEY win on the next load. It must be dropped.
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-new")
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"LLM_API_KEY": "sk-old"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored == {"OPENAI_API_KEY": "sk-new"}
|
||||
|
||||
|
||||
def test_persist_current_preserves_non_string_stored_values(tmp_path: Path) -> None:
|
||||
# Structured values written by newer versions must survive verbatim instead
|
||||
# of being flattened through str().
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"FUTURE_HEADERS": {"X-Team": "sec"}, "FUTURE_RETRIES": 3}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
stored = json.loads(target.read_text(encoding="utf-8"))["env"]
|
||||
assert stored["FUTURE_HEADERS"] == {"X-Team": "sec"}
|
||||
assert stored["FUTURE_RETRIES"] == 3
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue