refactor(config): resolve env vars via in-place Pydantic walk

Replace the dump→resolve→model_validate roundtrip with a recursive walk
that substitutes ${VAR} in string values directly on BaseModel /
__pydantic_extra__ / dict / list nodes. Identity is preserved on any
subtree with no references, so the original Config instance is returned
unchanged when nothing needs resolving.

Side effects:
- exclude=True fields (e.g. DreamConfig.cron) now survive even when
  other fields in the same config contain ${VAR} references, closing
  the edge case left open by the previous fast-path-only fix.
- _has_env_refs is dropped (the walker short-circuits naturally).
- Added a regression test pairing cron with a resolved providers.groq
  api_key to lock the coexistence case.

Made-with: Cursor
This commit is contained in:
Xubin Ren 2026-04-22 14:27:33 +00:00 committed by Xubin Ren
parent c9a21d96d8
commit c1e7aa5504
2 changed files with 61 additions and 16 deletions

View File

@ -4,9 +4,11 @@ import json
import os import os
import re import re
from pathlib import Path from pathlib import Path
from typing import Any
import pydantic import pydantic
from loguru import logger from loguru import logger
from pydantic import BaseModel
from nanobot.config.schema import Config from nanobot.config.schema import Config
@ -82,32 +84,50 @@ _ENV_REF_PATTERN = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
def resolve_config_env_vars(config: Config) -> Config: def resolve_config_env_vars(config: Config) -> Config:
"""Return a copy of *config* with ``${VAR}`` env-var references resolved. """Return *config* with ``${VAR}`` env-var references resolved.
Only string values are affected; other types pass through unchanged. Walks in place so fields declared with ``exclude=True`` (e.g.
Raises :class:`ValueError` if a referenced variable is not set. ``DreamConfig.cron``) survive; returns the same instance when no
references are present. Raises ``ValueError`` if a referenced
variable is not set.
""" """
data = config.model_dump(mode="json", by_alias=True) return _resolve_in_place(config)
if not _has_env_refs(data):
# Skip the dump→revalidate roundtrip so fields with ``exclude=True`` survive.
return config
data = _resolve_env_vars(data)
return Config.model_validate(data)
def _has_env_refs(obj: object) -> bool: def _resolve_in_place(obj: Any) -> Any:
"""Return True if any string value contains a ``${VAR}`` reference."""
if isinstance(obj, str): if isinstance(obj, str):
return bool(_ENV_REF_PATTERN.search(obj)) new = _ENV_REF_PATTERN.sub(_env_replace, obj)
return new if new != obj else obj
if isinstance(obj, BaseModel):
updates: dict[str, Any] = {}
for name in type(obj).model_fields:
old = getattr(obj, name)
new = _resolve_in_place(old)
if new is not old:
updates[name] = new
extras = obj.__pydantic_extra__
new_extras: dict[str, Any] | None = None
if extras:
resolved = {k: _resolve_in_place(v) for k, v in extras.items()}
if any(resolved[k] is not extras[k] for k in extras):
new_extras = resolved
if not updates and new_extras is None:
return obj
copy = obj.model_copy(update=updates) if updates else obj.model_copy()
if new_extras is not None:
copy.__pydantic_extra__ = new_extras
return copy
if isinstance(obj, dict): if isinstance(obj, dict):
return any(_has_env_refs(v) for v in obj.values()) resolved = {k: _resolve_in_place(v) for k, v in obj.items()}
return resolved if any(resolved[k] is not obj[k] for k in obj) else obj
if isinstance(obj, list): if isinstance(obj, list):
return any(_has_env_refs(v) for v in obj) resolved = [_resolve_in_place(v) for v in obj]
return False return resolved if any(nv is not ov for nv, ov in zip(resolved, obj)) else obj
return obj
def _resolve_env_vars(obj: object) -> object: def _resolve_env_vars(obj: object) -> object:
"""Recursively resolve ``${VAR}`` patterns in string values.""" """Recursively resolve ``${VAR}`` patterns in plain strings/dicts/lists."""
if isinstance(obj, str): if isinstance(obj, str):
return _ENV_REF_PATTERN.sub(_env_replace, obj) return _ENV_REF_PATTERN.sub(_env_replace, obj)
if isinstance(obj, dict): if isinstance(obj, dict):

View File

@ -102,3 +102,28 @@ class TestResolveConfig:
assert resolved.agents.defaults.dream.describe_schedule() == ( assert resolved.agents.defaults.dream.describe_schedule() == (
"cron 5 11 * * * (legacy)" "cron 5 11 * * * (legacy)"
) )
def test_preserves_excluded_fields_with_env_refs(self, tmp_path, monkeypatch):
"""Excluded fields must also survive when the config contains
``${VAR}`` refs elsewhere. An in-place walk preserves the legacy
``cron`` override even as unrelated string fields are substituted."""
monkeypatch.setenv("TEST_API_KEY", "resolved-key")
config_path = tmp_path / "config.json"
config_path.write_text(
json.dumps(
{
"agents": {"defaults": {"dream": {"cron": "5 11 * * *"}}},
"providers": {"groq": {"apiKey": "${TEST_API_KEY}"}},
}
),
encoding="utf-8",
)
raw = load_config(config_path)
resolved = resolve_config_env_vars(raw)
assert resolved.providers.groq.api_key == "resolved-key"
assert resolved.agents.defaults.dream.cron == "5 11 * * *"
assert resolved.agents.defaults.dream.describe_schedule() == (
"cron 5 11 * * * (legacy)"
)