From 77748ff104ebafe7401a7c7f468a754ac7df8606 Mon Sep 17 00:00:00 2001 From: Matt Johnson Date: Fri, 17 Jul 2026 07:20:48 +0000 Subject: [PATCH] fix(config): merge partial PUT bodies instead of resetting to defaults Saving the "Auto-advert interval" dropdown on the MeshCore Companion page took BOTH radios offline on 2026-07-17 06:46:52. One click, full outage. The page PUT a single-key body to /api/config/connection: {"meshcore_advert_interval_seconds": 10800} _dict_to_dataclass() builds kwargs only from the keys present in the body and lets `cls(**kwargs)` default the rest, so every OMITTED field was reset to its dataclass default and written to disk: type: tcp -> serial (Meshtastic offline) tcp_host: 192.168.1.100 -> (LOCAL_FIELDS, see below) tcp_port: 4404 -> 4403 (wrong meshmonitor vnode) meshcore_host: 192.168.1.253 -> '' (MeshCore off; blank = off) meshcore_conn_type: serial -> tcp (wrong transport) meshcore_serial_port: /dev/meshcore-rak -> '' (RAK radio lost) It was silent twice over. `connection` is restart-required, so the running process kept the good in-memory config while the file sat gutted, waiting for any restart to detonate. And save_section() writes the domain file FIRST and local.yaml SECOND: meshtastic.yaml hit the disk already gutted, then the local.yaml write (which owns connection.tcp_host via LOCAL_FIELDS) died on `[Errno 13] Permission denied` -- so tcp_host landed in neither file, and the 500 that would have named the cause was swallowed by the UI. The operator saw nothing happen. This was never one page's bug: PUT /api/config/{section} was destructive on a partial payload for EVERY section. Other callers only survive because they happen to spread the full object first. Fixes, in depth: * Route (the durable fix): merge the body over the CURRENT live section before coercing, so omitted keys keep their live values while present keys -- including '' / False / [] -- still apply. The base is the live config, the same values GET serves, so a partial PUT now lands exactly where a full-object PUT from that same GET would. Full-object callers are unaffected. Fixed at the HTTP boundary, not in _dict_to_dataclass(): absent-key-means-default is CORRECT at config-load time, where a file legitimately omits fields it does not override. * Nested semantics keyed off the dataclass schema, not "is it a dict": nested dataclass fields DEEP-MERGE (a partial region_routes must not drop sibling cells), while bare dict/list fields REPLACE at the key (cells, toggles, destinations, rules are dynamic maps -- deep-merging them would resurrect deleted keys and make deletion impossible, the mirror image of the bug being fixed). * Page: send the full connection object like every other caller does. * Errors are visible: the save handler no longer swallows the exception, and updateConfig() surfaces the server's `detail` rather than a bare "API error: 500", which is what hid Permission denied from the operator. * Default advert interval 10800 -> 86400 (24h). 3h is far too frequent a default for a public mesh; the UI "(default)" label moves to match. Tests: tests/test_config_partial_save_merge.py reproduces the outage with the exact payload, and pins merge semantics across connection AND notifications, intentional clearing, deep-merge, and map-deletion. Co-Authored-By: Claude Opus 4.8 (1M context) --- work/dashboard-frontend/src/lib/api.ts | 19 +- .../src/pages/MeshCoreCompanion.tsx | 40 ++- work/meshai/config.py | 2 +- work/meshai/dashboard/api/config_routes.py | 78 ++++- work/tests/test_config_partial_save_merge.py | 308 ++++++++++++++++++ work/tests/test_mesh_send_api.py | 7 +- 6 files changed, 442 insertions(+), 12 deletions(-) create mode 100644 work/tests/test_config_partial_save_merge.py diff --git a/work/dashboard-frontend/src/lib/api.ts b/work/dashboard-frontend/src/lib/api.ts index b24d85e..f053979 100644 --- a/work/dashboard-frontend/src/lib/api.ts +++ b/work/dashboard-frontend/src/lib/api.ts @@ -297,7 +297,24 @@ export async function updateConfig( body: JSON.stringify(data), }) if (!response.ok) { - throw new Error(`API error: ${response.status} ${response.statusText}`) + // Surface the server's `detail` when there is one. A bare + // "API error: 500 Internal Server Error" hid the actual cause of the + // 2026-07-17 outage from the operator -- the real message was + // "[Errno 13] Permission denied: '/data/config/local.yaml'", which would + // have named the problem outright. + let detail = '' + try { + const body = await response.json() as { detail?: unknown } + if (typeof body?.detail === 'string') detail = body.detail + else if (body?.detail != null) detail = JSON.stringify(body.detail) + } catch { + // non-JSON error body — fall back to the status line + } + throw new Error( + detail + ? `${detail} (${response.status})` + : `API error: ${response.status} ${response.statusText}` + ) } return response.json() } diff --git a/work/dashboard-frontend/src/pages/MeshCoreCompanion.tsx b/work/dashboard-frontend/src/pages/MeshCoreCompanion.tsx index bd9dd4d..61cca20 100644 --- a/work/dashboard-frontend/src/pages/MeshCoreCompanion.tsx +++ b/work/dashboard-frontend/src/pages/MeshCoreCompanion.tsx @@ -61,9 +61,14 @@ export default function MeshCoreCompanion() { // Auto-advert control state — interval in hours (0 = disabled) // Loaded from connection config; editable in-page and PUTted back. - const [advertIntervalHours, setAdvertIntervalHours] = useState(3) + const [advertIntervalHours, setAdvertIntervalHours] = useState(24) const [advertIntervalSaving, setAdvertIntervalSaving] = useState(false) const [advertIntervalSaved, setAdvertIntervalSaved] = useState(false) + const [advertIntervalError, setAdvertIntervalError] = useState(null) + // The FULL connection section as fetched. Saving spreads this so the PUT + // carries every field, matching every other updateConfig('connection', ...) + // caller. See handleSaveAdvertInterval. + const [connConfig, setConnConfig] = useState | null>(null) useEffect(() => { document.title = 'Companion & Channels - MeshAI' @@ -101,6 +106,7 @@ export default function MeshCoreCompanion() { const resp = await fetch('/api/config/connection') if (resp.ok) { const data = await resp.json() as Record + setConnConfig(data) const sec = data['meshcore_advert_interval_seconds'] if (typeof sec === 'number') { setAdvertIntervalHours(sec > 0 ? sec / 3600 : 0) @@ -140,17 +146,32 @@ export default function MeshCoreCompanion() { const handleSaveAdvertInterval = useCallback(async () => { setAdvertIntervalSaving(true) setAdvertIntervalSaved(false) + setAdvertIntervalError(null) try { const seconds = Math.round(advertIntervalHours * 3600) - await updateConfig('connection', { meshcore_advert_interval_seconds: seconds }) + // Spread the full fetched section, don't PUT a lone key. On 2026-07-17 a + // single-key body here reset every OMITTED connection field to its + // dataclass default (type -> serial, meshcore_host -> '', ...) and took + // both radios offline. The route now merges partial bodies server-side, + // but this page still sends the whole object like every other caller: + // belt and braces, and it keeps the PUT's meaning explicit. + const current = connConfig ?? {} + await updateConfig('connection', { + ...current, + meshcore_advert_interval_seconds: seconds, + }) + setConnConfig({ ...current, meshcore_advert_interval_seconds: seconds }) setAdvertIntervalSaved(true) setTimeout(() => setAdvertIntervalSaved(false), 2000) - } catch { - // keep saving=false, let UI show failure implicitly + } catch (err) { + // A failed save MUST be visible. This handler used to swallow the error + // and "let the UI show failure implicitly" -- it showed nothing at all, + // so the operator saw a silent no-op while the write had already failed. + setAdvertIntervalError(err instanceof Error ? err.message : 'Save failed') } finally { setAdvertIntervalSaving(false) } - }, [advertIntervalHours]) + }, [advertIntervalHours, connConfig]) const handleCopyKey = useCallback(async (key: string) => { try { @@ -343,10 +364,10 @@ export default function MeshCoreCompanion() { > - + - +