mirror of
https://github.com/usestrix/strix.git
synced 2026-09-09 22:31:07 +00:00
fix(config): drop the stored LLM connection when a linked env var changes
A new STRIX_LLM, LLM_API_KEY, or LLM_API_BASE exported in the shell must not be combined with the key, base, or model still stored in cli-config.json. Restore the pre-refactor rule: when any linked LLM connection var differs from the stored value, discard the whole stored connection before loading and before persisting. Unrelated stored settings are kept.
This commit is contained in:
parent
ce0db30252
commit
3e88e498b9
3 changed files with 136 additions and 7 deletions
|
|
@ -10,11 +10,13 @@ from typing import TYPE_CHECKING, Any
|
|||
|
||||
from pydantic import AliasChoices, BaseModel
|
||||
|
||||
from strix.config.settings import Settings
|
||||
from strix.config.settings import LlmSettings, Settings
|
||||
from strix.utils.secret_files import write_secret_text
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Mapping
|
||||
|
||||
from pydantic.fields import FieldInfo
|
||||
|
||||
|
||||
|
|
@ -25,6 +27,11 @@ _DEFAULT_PATH: Path = Path.home() / ".strix" / "cli-config.json"
|
|||
_override: Path | None = None
|
||||
_cached: Settings | None = None
|
||||
|
||||
# Model, API key, and API base describe one provider connection. When the shell
|
||||
# changes any of them, the stored values of the others no longer belong together
|
||||
# and are dropped rather than mixed with the new value.
|
||||
_LINKED_LLM_FIELDS = ("model", "api_key", "api_base")
|
||||
|
||||
|
||||
def load_settings() -> Settings:
|
||||
"""Resolve settings from env + JSON file + defaults. Memoized.
|
||||
|
|
@ -58,13 +65,14 @@ def persist_current() -> None:
|
|||
|
||||
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.
|
||||
var set to the empty string clears the field from the file. A change to
|
||||
any linked LLM connection var drops the whole stored connection first.
|
||||
"""
|
||||
s = load_settings()
|
||||
target = _override or _DEFAULT_PATH
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
env_block = _read_env_block(target)
|
||||
env_block = _drop_stale_llm_connection(_read_env_block(target))
|
||||
for sub_name in type(s).model_fields:
|
||||
sub_model = getattr(s, sub_name)
|
||||
if not isinstance(sub_model, BaseModel):
|
||||
|
|
@ -104,7 +112,7 @@ 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.
|
||||
"""
|
||||
env_block_upper = _read_env_block(path)
|
||||
env_block_upper = _drop_stale_llm_connection(_read_env_block(path))
|
||||
if not env_block_upper:
|
||||
return {}
|
||||
env_present = {k.upper() for k in os.environ}
|
||||
|
|
@ -128,6 +136,27 @@ def _read_json_overrides(path: Path) -> dict[str, dict[str, Any]]:
|
|||
return nested
|
||||
|
||||
|
||||
def _first_alias_value(aliases: list[str], source: Mapping[str, Any]) -> Any | None:
|
||||
return next((source[alias] for alias in aliases if alias in source), None)
|
||||
|
||||
|
||||
def _drop_stale_llm_connection(env_block: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Remove every linked LLM var from ``env_block`` if the shell changed any of them."""
|
||||
linked_aliases = [
|
||||
[alias.upper() for alias in _aliases_for(LlmSettings.model_fields[name])]
|
||||
for name in _LINKED_LLM_FIELDS
|
||||
]
|
||||
changed = any(
|
||||
(env_value := _first_alias_value(aliases, os.environ)) is not None
|
||||
and env_value != _first_alias_value(aliases, env_block)
|
||||
for aliases in linked_aliases
|
||||
)
|
||||
if not changed:
|
||||
return env_block
|
||||
stale = {alias for aliases in linked_aliases for alias in aliases}
|
||||
return {k: v for k, v in env_block.items() if k not in stale}
|
||||
|
||||
|
||||
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():
|
||||
|
|
|
|||
|
|
@ -229,16 +229,109 @@ def test_persist_current_env_overrides_file_value(
|
|||
) -> None:
|
||||
target = tmp_path / "cli-config.json"
|
||||
target.write_text(
|
||||
json.dumps({"env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"}}),
|
||||
json.dumps({"env": {"STRIX_LLM": "file-model", "PERPLEXITY_API_KEY": "file-pplx"}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
monkeypatch.setenv("PERPLEXITY_API_KEY", "env-pplx")
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {
|
||||
"env": {"STRIX_LLM": "file-model", "PERPLEXITY_API_KEY": "env-pplx"}
|
||||
}
|
||||
|
||||
|
||||
def test_linked_llm_model_change_drops_stored_key_and_base(
|
||||
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",
|
||||
"LLM_API_BASE": "http://file-base",
|
||||
"PERPLEXITY_API_KEY": "pplx",
|
||||
}
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
loader.apply_config_override(target)
|
||||
monkeypatch.setenv("STRIX_LLM", "env-model")
|
||||
|
||||
llm = loader.load_settings().llm
|
||||
assert llm.model == "env-model"
|
||||
assert llm.api_key is None
|
||||
assert llm.api_base is None
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {
|
||||
"env": {"STRIX_LLM": "env-model", "LLM_API_KEY": "file-key"}
|
||||
"env": {"STRIX_LLM": "env-model", "PERPLEXITY_API_KEY": "pplx"}
|
||||
}
|
||||
|
||||
|
||||
def test_linked_llm_key_change_drops_stored_model(
|
||||
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("LLM_API_KEY", "new-key")
|
||||
|
||||
assert loader.load_settings().llm.model is None
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {"env": {"LLM_API_KEY": "new-key"}}
|
||||
|
||||
|
||||
def test_linked_llm_secondary_alias_in_env_is_not_a_change(
|
||||
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("LLM_API_KEY", "file-key")
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "unrelated-global-key")
|
||||
|
||||
llm = loader.load_settings().llm
|
||||
assert llm.model == "file-model"
|
||||
assert llm.api_key == "file-key"
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {
|
||||
"env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"}
|
||||
}
|
||||
|
||||
|
||||
def test_linked_llm_unchanged_env_keeps_stored_key(
|
||||
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", "file-model")
|
||||
|
||||
assert loader.load_settings().llm.api_key == "file-key"
|
||||
|
||||
loader.persist_current()
|
||||
|
||||
assert json.loads(target.read_text(encoding="utf-8")) == {
|
||||
"env": {"STRIX_LLM": "file-model", "LLM_API_KEY": "file-key"}
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -114,7 +114,14 @@ def test_config_file_loads_dedupe_model(
|
|||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
for key in ("STRIX_LLM", "STRIX_DEDUPE_MODEL", "STRIX_DEDUPE_REASONING_EFFORT"):
|
||||
for key in (
|
||||
"STRIX_LLM",
|
||||
"LLM_API_KEY",
|
||||
"OPENAI_API_KEY",
|
||||
"LLM_API_BASE",
|
||||
"STRIX_DEDUPE_MODEL",
|
||||
"STRIX_DEDUPE_REASONING_EFFORT",
|
||||
):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
path = tmp_path / "config.json"
|
||||
path.write_text(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue