diff --git a/meshai/config_loader.py b/meshai/config_loader.py index 9de07f0..169d99e 100644 --- a/meshai/config_loader.py +++ b/meshai/config_loader.py @@ -160,6 +160,104 @@ def _load_yaml_with_includes(file_path: Path) -> dict: _loading_files.discard(file_path) +# ---- v0.6-tail-4: !include-preserving load/dump for save_section ---------- +# +# save_section() needs to re-read target_path off disk so it can preserve +# secret-ref placeholders and existing keys for sections that share a file. +# When target_path is config.yaml (the orchestrator) the file contains +# !include directives for OTHER sections; plain yaml.safe_load can't parse +# those and the whole save fails. We can't use _load_yaml_with_includes +# because that would substitute the included files in, and then the +# subsequent yaml.dump would flatten them onto disk -- losing the +# multi-file layout permanently the first time anyone PUTs an inline +# section like `bot`. Instead we read with a loader that returns an +# Include() placeholder for each !include node, and dump with a dumper +# that re-emits Include(path) as `!include path`. The round-trip is +# byte-stable for the include directives, and the non-include sections +# (which is everything save_section actually mutates) just round-trip as +# plain dict/list. + + +class Include: + """Placeholder preserving an !include directive across read/write.""" + + __slots__ = ("path",) + + def __init__(self, path: str): + self.path = path + + def __repr__(self) -> str: + return f"Include({self.path!r})" + + def __eq__(self, other) -> bool: + return isinstance(other, Include) and self.path == other.path + + def __hash__(self) -> int: + return hash(("Include", self.path)) + + +def _make_preserve_loader(): + """SafeLoader subclass that returns Include() for !include scalars. + + Unlike _make_include_loader, this does NOT recurse into the + referenced file -- it keeps the directive intact so a subsequent + dump can emit it back to disk verbatim. + """ + + class PreserveLoader(yaml.SafeLoader): + pass + + def construct_include(loader: PreserveLoader, node: yaml.Node) -> Include: + return Include(loader.construct_scalar(node)) + + PreserveLoader.add_constructor("!include", construct_include) + return PreserveLoader + + +def _make_preserve_dumper(): + """SafeDumper subclass that renders Include() back as `!include path`.""" + + class PreserveDumper(yaml.SafeDumper): + pass + + def represent_include(dumper: PreserveDumper, data: Include): + # style="" forces plain (unquoted) scalar so the output matches + # the prod on-disk convention `!include foo.yaml` rather than + # PyYAML's auto-picked `!include 'foo.yaml'`. The runtime loader + # parses both, but we want the round-trip to be byte-stable. + return dumper.represent_scalar("!include", data.path, style="") + + PreserveDumper.add_representer(Include, represent_include) + return PreserveDumper + + +def _load_yaml_preserve(file_path: Path): + """Read a YAML file, keeping !include nodes as Include placeholders. + + Returns {} for missing files (matches the runtime loader's contract). + """ + if not Path(file_path).exists(): + return {} + Loader = _make_preserve_loader() + with open(file_path, "r") as f: + return yaml.load(f, Loader=Loader) or {} + + +def _dump_yaml_preserve(data, file_path: Path) -> None: + """Write a YAML file, re-emitting Include() as `!include path`.""" + Dumper = _make_preserve_dumper() + with open(file_path, "w") as f: + yaml.dump( + data, + f, + Dumper=Dumper, + default_flow_style=False, + sort_keys=False, + allow_unicode=True, + ) + + + # ============================================================================= # ENVIRONMENT VARIABLE INTERPOLATION # ============================================================================= @@ -587,8 +685,10 @@ def save_section( _raw_on_disk = {} if target_path.exists(): try: - with open(target_path) as _rf: - _raw_on_disk = yaml.safe_load(_rf) or {} + # v0.6-tail-4: read with Include() preservation so config.yaml + # (which has !include directives for sibling sections) parses + # without choking. Plain yaml.safe_load used to die here. + _raw_on_disk = _load_yaml_preserve(target_path) or {} except Exception: _raw_on_disk = {} if target_file in ("meshtastic.yaml", "config.yaml") and isinstance(_raw_on_disk, dict): @@ -680,10 +780,10 @@ def save_section( data = check_secrets(data) domain_data, local_updates = _extract_local_fields(section_name, data) - # Load existing target file + # Load existing target file (v0.6-tail-4: preserve !include directives + # for inline-section saves to config.yaml; safe_load would crash). if target_path.exists(): - with open(target_path, "r") as f: - existing = yaml.safe_load(f) or {} + existing = _load_yaml_preserve(target_path) or {} else: existing = {} @@ -697,9 +797,10 @@ def save_section( # For dedicated files, the whole file IS the section existing = domain_data - # Write domain file - with open(target_path, "w") as f: - yaml.dump(existing, f, default_flow_style=False, sort_keys=False, allow_unicode=True) + # Write domain file (v0.6-tail-4: preserve dumper re-emits Include() + # placeholders as `!include path` so multi-file layouts survive the + # round-trip. Plain yaml.dump would crash on Include objects.) + _dump_yaml_preserve(existing, target_path) files_written.append(str(target_path)) _logger.info(f"Saved {section_name} to {target_path}") diff --git a/tests/test_include_roundtrip.py b/tests/test_include_roundtrip.py new file mode 100644 index 0000000..3517a3d --- /dev/null +++ b/tests/test_include_roundtrip.py @@ -0,0 +1,240 @@ +"""v0.6-tail-4: !include YAML round-trip preservation tests. + +Verifies: + - The runtime loader (_load_yaml_with_includes) still substitutes + !include contents (read-path behavior unchanged). + - The preserve loader (_load_yaml_preserve) returns Include() sentinels + instead of substituting. + - The preserve dumper re-emits Include() as `!include path`. + - A read-then-write-then-re-read round-trip through the preserve helpers + yields identical effective config. + - save_section() on a section whose target file uses !include for + sibling sections succeeds (the PUT /api/config/bot 500 from v0.6-tail-3 + is closed). +""" +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + + +# --------------------------------------------------------------------------- +# Read path: runtime !include substitution still works (unchanged behavior). +# --------------------------------------------------------------------------- + + +def _write(p: Path, body: str) -> Path: + p.write_text(body) + return p + + +def test_runtime_load_substitutes_include(tmp_path): + from meshai.config_loader import _load_yaml_with_includes + + _write(tmp_path / "child.yaml", "value: 42\nname: child\n") + root = _write( + tmp_path / "config.yaml", + "top: 1\nnested: !include child.yaml\n", + ) + + data = _load_yaml_with_includes(root) + # The !include directive must be substituted in at runtime so the + # Config dataclass sees real values, not placeholders. + assert data["top"] == 1 + assert data["nested"] == {"value": 42, "name": "child"} + + +# --------------------------------------------------------------------------- +# Preserve path: !include round-trips as a sentinel. +# --------------------------------------------------------------------------- + + +def test_preserve_load_returns_include_sentinel(tmp_path): + from meshai.config_loader import Include, _load_yaml_preserve + + _write(tmp_path / "child.yaml", "value: 42\n") + root = _write( + tmp_path / "config.yaml", + "top: 1\nnested: !include child.yaml\nlater: hi\n", + ) + + data = _load_yaml_preserve(root) + assert data["top"] == 1 + # The !include node must NOT be substituted -- the placeholder is + # what makes round-trip possible. + assert isinstance(data["nested"], Include) + assert data["nested"].path == "child.yaml" + assert data["later"] == "hi" + + +def test_preserve_dump_reemits_include_directive(tmp_path): + from meshai.config_loader import ( + Include, + _dump_yaml_preserve, + _load_yaml_preserve, + ) + + data = { + "bot": {"name": "AIDA"}, + "domain": Include("env_feeds.yaml"), + } + out = tmp_path / "out.yaml" + _dump_yaml_preserve(data, out) + raw = out.read_text() + # The directive must come back as text, not flattened to a dict or + # serialized as some Python repr. PyYAML may auto-quote the scalar + # (`!include 'env_feeds.yaml'`); both are equivalent at parse time. + assert ("!include env_feeds.yaml" in raw + or "!include 'env_feeds.yaml'" in raw) + + # And the round-trip back via _load_yaml_preserve must give us the + # same Include placeholder. + reread = _load_yaml_preserve(out) + assert reread["bot"] == {"name": "AIDA"} + assert isinstance(reread["domain"], Include) + assert reread["domain"].path == "env_feeds.yaml" + + +def test_roundtrip_identical(tmp_path): + """read -> write -> read produces the same data structure.""" + from meshai.config_loader import _dump_yaml_preserve, _load_yaml_preserve + + _write(tmp_path / "leaf.yaml", "x: 1\ny: 2\n") + root = _write( + tmp_path / "config.yaml", + ( + "timezone: America/Boise\n" + "bot:\n" + " name: AIDA\n" + " respond_to_dms: true\n" + "domain: !include leaf.yaml\n" + ), + ) + + first = _load_yaml_preserve(root) + out = tmp_path / "rewrite.yaml" + _dump_yaml_preserve(first, out) + second = _load_yaml_preserve(out) + assert first == second + + +# --------------------------------------------------------------------------- +# Integration: save_section() on config.yaml that uses !include succeeds. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def prod_shaped_config(tmp_path): + """Build a /data/config-shaped tree with config.yaml using !include.""" + config_dir = tmp_path / "config" + config_dir.mkdir() + + # Sibling files that config.yaml !include's. + (config_dir / "meshtastic.yaml").write_text( + "connection:\n tcp_host: 1.2.3.4\n port: 4403\n" + "commands:\n enabled: true\n prefix: '!'\n" + ) + (config_dir / "env_feeds.yaml").write_text( + "usgs:\n enabled: true\n feed_source: central\n" + ) + (config_dir / "llm.yaml").write_text( + "backend: google\n \n# placeholder\n" + ) + (config_dir / "config.yaml").write_text( + "timezone: America/Boise\n" + "bot:\n" + " respond_to_dms: true\n" + " filter_bbs_protocols: true\n" + " name: AIDA\n" + "response:\n" + " delay_min: 1.5\n" + " delay_max: 2.5\n" + "meshtastic: !include meshtastic.yaml\n" + "environmental: !include env_feeds.yaml\n" + "llm: !include llm.yaml\n" + ) + return config_dir + + +def test_save_section_inline_does_not_crash_with_includes(prod_shaped_config): + """PUT /api/config/bot -> save_section('bot', ...) used to die with + 'could not determine a constructor for the tag !include'. After + v0.6-tail-4 it must succeed and leave the include directives intact.""" + from meshai.config_loader import save_section + + result = save_section( + "bot", + {"respond_to_dms": False, "filter_bbs_protocols": True, "name": "AIDA"}, + config_dir=prod_shaped_config, + ) + assert result["saved"] is True + assert result["rejected_secrets"] == [] + + # The on-disk config.yaml must still contain the !include directives + # for the sibling sections (meshtastic, environmental, llm). PyYAML + # may emit them quoted; both forms are valid. + written = (prod_shaped_config / "config.yaml").read_text() + for name in ("meshtastic.yaml", "env_feeds.yaml", "llm.yaml"): + assert (f"!include {name}" in written + or f"!include '{name}'" in written), ( + f"missing !include for {name}; got: {written!r}" + ) + # And the change to bot.respond_to_dms must have actually persisted. + assert "respond_to_dms: false" in written + + +def test_save_section_inline_then_runtime_reload_sees_change(prod_shaped_config): + """Round-trip integrity: after save_section() the runtime loader + must still resolve the !include directives AND see the saved + change to the inline section.""" + from meshai.config_loader import _load_yaml_with_includes, save_section + + save_section( + "response", + {"delay_min": 0.5, "delay_max": 1.0}, + config_dir=prod_shaped_config, + ) + + runtime = _load_yaml_with_includes(prod_shaped_config / "config.yaml") + # The save_section change took effect. + assert runtime["response"]["delay_min"] == 0.5 + # The includes still resolve at runtime. + assert runtime["meshtastic"]["connection"]["tcp_host"] == "1.2.3.4" + assert runtime["environmental"]["usgs"]["feed_source"] == "central" + + +def test_save_section_dedicated_file_unchanged(prod_shaped_config): + """save_section() on a section whose target_file is a !include'd + file (e.g. environmental -> env_feeds.yaml) must not be affected + by this patch -- those files don't use !include themselves.""" + from meshai.config_loader import save_section + + result = save_section( + "environmental", + {"usgs": {"enabled": False, "feed_source": "central"}}, + config_dir=prod_shaped_config, + ) + assert result["saved"] is True + + written = (prod_shaped_config / "env_feeds.yaml").read_text() + assert "enabled: false" in written + # config.yaml is untouched -- !include directives still there. + main = (prod_shaped_config / "config.yaml").read_text() + assert "!include env_feeds.yaml" in main + + +# --------------------------------------------------------------------------- +# Defensive: cycle detection still trips for the runtime loader. +# --------------------------------------------------------------------------- + + +def test_runtime_loader_detects_cycles(tmp_path): + from meshai.config_loader import _load_yaml_with_includes + + _write(tmp_path / "a.yaml", "child: !include b.yaml\n") + _write(tmp_path / "b.yaml", "child: !include a.yaml\n") + + with pytest.raises(yaml.YAMLError, match="Circular include"): + _load_yaml_with_includes(tmp_path / "a.yaml")