fix(config): preserve excluded fields in resolve_config_env_vars

`resolve_config_env_vars` unconditionally dumped the config via
`model_dump(mode="json")` and revalidated it, which silently dropped
any field declared with `exclude=True` (e.g. `DreamConfig.cron` —
introduced by the Dream rename refactor in #2717). Result:
`agents.defaults.dream.cron` was never honored at runtime — the gateway
always fell back to the default `every 2h` schedule even when `cron`
was set in config.json.

Fix: skip the roundtrip entirely when the config has no `${VAR}`
references. Env-var interpolation still works unchanged when refs
exist; the legacy `cron` override now survives the common case of
fully-resolved config.

Regression test covers the bug path.
This commit is contained in:
Saimon Ventura 2026-04-22 11:07:27 +01:00 committed by Xubin Ren
parent 239e91a4d6
commit c9a21d96d8
2 changed files with 40 additions and 1 deletions

View File

@ -78,6 +78,9 @@ def save_config(config: Config, config_path: Path | None = None) -> None:
json.dump(data, f, indent=2, ensure_ascii=False)
_ENV_REF_PATTERN = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
def resolve_config_env_vars(config: Config) -> Config:
"""Return a copy of *config* with ``${VAR}`` env-var references resolved.
@ -85,14 +88,28 @@ def resolve_config_env_vars(config: Config) -> Config:
Raises :class:`ValueError` if a referenced variable is not set.
"""
data = config.model_dump(mode="json", by_alias=True)
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:
"""Return True if any string value contains a ``${VAR}`` reference."""
if isinstance(obj, str):
return bool(_ENV_REF_PATTERN.search(obj))
if isinstance(obj, dict):
return any(_has_env_refs(v) for v in obj.values())
if isinstance(obj, list):
return any(_has_env_refs(v) for v in obj)
return False
def _resolve_env_vars(obj: object) -> object:
"""Recursively resolve ``${VAR}`` patterns in string values."""
if isinstance(obj, str):
return re.sub(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}", _env_replace, obj)
return _ENV_REF_PATTERN.sub(_env_replace, obj)
if isinstance(obj, dict):
return {k: _resolve_env_vars(v) for k, v in obj.items()}
if isinstance(obj, list):

View File

@ -80,3 +80,25 @@ class TestResolveConfig:
saved = json.loads(config_path.read_text(encoding="utf-8"))
assert saved["channels"]["telegram"]["token"] == "${MY_TOKEN}"
def test_preserves_excluded_fields_when_no_env_refs(self, tmp_path):
"""Regression: fields with ``exclude=True`` (e.g. DreamConfig.cron)
must survive ``resolve_config_env_vars`` when the config has no
``${VAR}`` references. Previously the unconditional dumprevalidate
roundtrip silently dropped them."""
config_path = tmp_path / "config.json"
config_path.write_text(
json.dumps(
{"agents": {"defaults": {"dream": {"cron": "5 11 * * *"}}}}
),
encoding="utf-8",
)
raw = load_config(config_path)
assert raw.agents.defaults.dream.cron == "5 11 * * *"
resolved = resolve_config_env_vars(raw)
assert resolved.agents.defaults.dream.cron == "5 11 * * *"
assert resolved.agents.defaults.dream.describe_schedule() == (
"cron 5 11 * * * (legacy)"
)