mirror of
https://github.com/usestrix/strix.git
synced 2026-09-09 22:31:07 +00:00
fix(config): merge env into cli-config.json instead of overwriting it
persist_current rewrote the config file with only the env vars set in the shell, so a run whose STRIX_LLM or LLM_API_KEY came from the file erased them and the next launch failed with MISSING REQUIRED ENVIRONMENT VARIABLES. Start from the stored env block, let a set env var override or replace the aliases of its field, and let an empty env var clear it.
This commit is contained in:
parent
941c960650
commit
ce0db30252
2 changed files with 109 additions and 16 deletions
|
|
@ -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()}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue