fix(api): apply a POSTed config through the same path as a file edit - #761
Conversation
POST /api/config hot-applied a hand-picked subset of control fields and swapped the shared config pointer itself, which left the configreload watcher diffing new against new — everything the handler didn't copy, starting with a first-time site-meter designation, never reached the running controller until restart. configreload.Apply is now the one apply path; the handler calls it with main.go's applier closure (Deps.ConfigApplier), the same closure the watcher runs. Fixes #760. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
miravoss26
left a comment
There was a problem hiding this comment.
Collapses the two config-apply paths into one exported configreload.Apply, so a config saved through POST /api/config runs the same body as a file edit. The #760 root cause is clearly addressed: the handler used to hot-apply a hand-picked subset and swap the shared pointer itself, which left the watcher diffing new against new, so a first-time site-meter set never reached the controller until a restart.
Correctness. The big applyConfigChange closure in main.go is a pure lift out of the configreload.New(...) call to a named var, handed to both the watcher and Deps.ConfigApplier, no body change (and TestWatcherUpdatesSiteMeterDriverOnReload staying green backs that). The control-field diff + pointer swap moving from Watcher.reload() into Apply(...) is a faithful receiver-to-params rename, and the "config reload: applied" log stays on the watcher path only, so the API path doesn't double-log. The nil-ConfigApplier fallback (bare Registry.Reload) matches the old handler floor, and in production the applier's reg.Reload(config.WithBatterySoCBounds(...)) means the API path now gets the SoC-bounded reload the old bare Registry.Reload(newCfg.Drivers) skipped, a real latent gap closed. Regression test asserts the meter reaches Ctrl.SiteMeterDriver and the applier sees the pre-POST snapshot as old.
One conscious-choice nod, not a bug: the API path's reload now runs under main.go's long-lived ctx (captured in the closure) instead of the request's r.Context(). That's arguably better (a reload won't cancel if the client disconnects mid-save), just worth being deliberate about.
Security. No secrets in the diff; EV/CalDAV passwords stay state.db-only and unlogged; no new deps, no new network destinations, no authz change. Test host is TEST-NET-1 (192.0.2.10).
Safe to merge from my read: CI green (go test + vet, full stack, core all pass), mergeable, no unresolved threads. ftw isn't on my auto-merge allowlist, so the merge is yours.
Fixes #760.
The bug
handlePostConfighot-applied a hand-picked subset of control fields (grid target, tolerance, slew rate, …), reloaded the registry, and swapped the shared config pointer itself "for snappiness". That pointer swap is what blinded the config watcher: its diff snapshots "old" from the same shared pointer, so by the time the fsnotify event for the API-saved file arrived, old == new and every diff — including the site-meter swap whose comment literally predicts "grid_w pegs at 0" — was a no-op.Consequence: setting
is_site_meterfor the first time through the API (the setup wizard's and Settings' normal path) never reached the running controller./api/statustook the deliberateSiteMeterDriver == ""→grid_w = 0branch,load_winflated to ≈ |pv_w|, grid energy slots stayed at zero, and dispatch had no site boundary — until a process restart. Reproduced on a fresh Pi-image install (details in #760). The handler's partial apply also skipped everything else only the watcher path did:WithBatterySoCBoundson the registry reload, capacities, inverter groups, fuse hot-reload, loadpoint fuse clamp, and the mpc/loadmodel site-meter sync.The fix
One apply path. The watcher's apply body moves into exported
configreload.Apply(cfgMu, cfg, ctrlMu, ctrl, newCfg, applier)— snapshot old, diff-apply control fields, swap the pointer, run the applier with (new, old). The watcher calls it afterconfig.Load;handlePostConfigcalls it directly with the config it just saved, passing main.go's applier closure through the newDeps.ConfigApplier. The closure is now a named variable in main.go, handed to bothconfigreload.Newand the API deps — no behavior change on the watcher path (its tests, includingTestWatcherUpdatesSiteMeterDriverOnReload, pass unchanged). A nilConfigApplier(tests, minimal embeddings) still applies control fields and falls back to a bare registry reload, matching the old handler's floor.Tests
TestPostConfigFirstSiteMeterReachesControl— the regression: POST a config that sets the first site meter, assert it reachesCtrl.SiteMeterDriverand the shared config.TestPostConfigRunsTheSharedApplierWithOldSnapshot— the applier runs on the API path and receives the pre-POST snapshot as old.TestApplyFirstSiteMeterWithoutAWatcher—configreload.Applyworks with no watcher constructed.make verifyclean. Live verification of the symptom and the restart workaround is documented in #760.Coordination
#741 touches
go/internal/api/api.goin the driver-control handlers — different functions, no overlapping hunks. #746/#731/#728 touchgo/cmd/ftw/main.goin the container/mDNS areas; this PR's main.go diff is confined to naming the existing applier closure and one added deps field.🤖 Generated with Claude Code