mirror of
https://github.com/zvx-echo6/meshai.git
synced 2026-06-11 01:14:45 +02:00
fix(v0.6-tail-4): register !include YAML tag constructor in config loader -- closes prod PUT 500
Pre-existing issue surfaced by v0.6-tail-3: prod config at
/data/config/config.yaml:82 uses !include to compose from separate
files, but the loader had no constructor registered so PUT
/api/config/<section> returned 500 with could-not-determine-constructor
when the section save path round-tripped YAML. This adds the !include
constructor (read path) + preserves the include structure on write so
multi-file config layouts work end-to-end via the GUI. The runtime
config behavior is unchanged; this only fixes the PUT-and-round-trip
case.
Implementation note: the read-only runtime path
(_load_yaml_with_includes) already had a working !include constructor
that recursively substitutes content. The bug was specifically in
save_section() -- it used plain yaml.safe_load() to re-read the target
file off disk for secret-ref preservation and for in-place section
updates. When target_file == "config.yaml" that file contains !include
directives for OTHER sections, and safe_load died on them.
Adding a third constructor that substitutes !include on save would
have flattened the multi-file layout to a single file the first time
anyone PUT an inline section. Instead this commit adds a preserve-mode
loader/dumper pair:
- _load_yaml_preserve() returns an Include("path") sentinel for each
!include node instead of recursing into the referenced file.
- _dump_yaml_preserve() re-emits Include("path") back to disk as
`!include path`. (PyYAML auto-quotes when the scalar contains a
period, so the on-disk form is `!include 'foo.yaml'`; both forms
are equivalent at parse time.)
- save_section()'s three yaml-touching sites (the secret-ref raw
read, the existing-target read, and the final dump) now use these
helpers. Local.yaml stays on yaml.safe_load/dump because local.yaml
never contains !include.
The runtime loader is untouched, so boot-time config still substitutes
includes and Config dataclasses see real values. Only the GUI's
section-save path round-trips through the preserve helpers.
Tests (tests/test_include_roundtrip.py, 8 cases):
- Runtime loader still substitutes !include content (regression guard)
- Preserve loader returns Include() sentinels
- Preserve dumper re-emits `!include path` (tolerant of PyYAML
auto-quoting)
- Read -> write -> read identity through the preserve helpers
- save_section('bot', ...) on a config.yaml that uses !include for
sibling sections succeeds AND leaves the includes intact on disk
(this is the exact prod PUT 500 case from v0.6-tail-3)
- After save_section, the runtime loader re-resolves all !include
files AND sees the saved change to the inline section
- save_section on a dedicated file (env_feeds.yaml) writes only that
file; config.yaml's !include directives are untouched
- Runtime cycle detection still trips on A!include->B!include->A
Live verification on CT108 after rebuild:
PUT /api/config/bot {"name":"AIDA","owner":"Malice","respond_to_dms":true,"filter_bbs_protocols":true}
-> HTTP 200 {"saved":true,"restart_required":false,"changed_keys":[]}
/data/config/config.yaml retains all 7 !include directives
(meshtastic, mesh_sources, mesh_intelligence, environmental,
notifications, llm, dashboard)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f89e9c11fb
commit
3351e7b444
2 changed files with 349 additions and 8 deletions
|
|
@ -160,6 +160,104 @@ def _load_yaml_with_includes(file_path: Path) -> dict:
|
||||||
_loading_files.discard(file_path)
|
_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
|
# ENVIRONMENT VARIABLE INTERPOLATION
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
|
|
@ -587,8 +685,10 @@ def save_section(
|
||||||
_raw_on_disk = {}
|
_raw_on_disk = {}
|
||||||
if target_path.exists():
|
if target_path.exists():
|
||||||
try:
|
try:
|
||||||
with open(target_path) as _rf:
|
# v0.6-tail-4: read with Include() preservation so config.yaml
|
||||||
_raw_on_disk = yaml.safe_load(_rf) or {}
|
# (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:
|
except Exception:
|
||||||
_raw_on_disk = {}
|
_raw_on_disk = {}
|
||||||
if target_file in ("meshtastic.yaml", "config.yaml") and isinstance(_raw_on_disk, dict):
|
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)
|
data = check_secrets(data)
|
||||||
domain_data, local_updates = _extract_local_fields(section_name, 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():
|
if target_path.exists():
|
||||||
with open(target_path, "r") as f:
|
existing = _load_yaml_preserve(target_path) or {}
|
||||||
existing = yaml.safe_load(f) or {}
|
|
||||||
else:
|
else:
|
||||||
existing = {}
|
existing = {}
|
||||||
|
|
||||||
|
|
@ -697,9 +797,10 @@ def save_section(
|
||||||
# For dedicated files, the whole file IS the section
|
# For dedicated files, the whole file IS the section
|
||||||
existing = domain_data
|
existing = domain_data
|
||||||
|
|
||||||
# Write domain file
|
# Write domain file (v0.6-tail-4: preserve dumper re-emits Include()
|
||||||
with open(target_path, "w") as f:
|
# placeholders as `!include path` so multi-file layouts survive the
|
||||||
yaml.dump(existing, f, default_flow_style=False, sort_keys=False, allow_unicode=True)
|
# round-trip. Plain yaml.dump would crash on Include objects.)
|
||||||
|
_dump_yaml_preserve(existing, target_path)
|
||||||
files_written.append(str(target_path))
|
files_written.append(str(target_path))
|
||||||
_logger.info(f"Saved {section_name} to {target_path}")
|
_logger.info(f"Saved {section_name} to {target_path}")
|
||||||
|
|
||||||
|
|
|
||||||
240
tests/test_include_roundtrip.py
Normal file
240
tests/test_include_roundtrip.py
Normal file
|
|
@ -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")
|
||||||
Loading…
Add table
Add a link
Reference in a new issue