From d33fb18718f7211d42f6e2c26e2eb5dd580a80a8 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sat, 29 Aug 2026 12:31:29 +0200 Subject: [PATCH] fix(config): boot with duplicate site meters demoted instead of crash-looping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A config with two is_site_meter: true drivers crash-looped Core at boot since #844, before the boot-phase HTTP listener binds — the operator loses the very UI they would fix the config with. Field incident 2026-08-29: a driver install on v1.15.0 left two site meters, and the update to v2.3.0 took the box dark until SSH. Parse now keeps the first declared site meter — the same driver older versions silently dispatched against — clears the flag on the rest, and records a LoadWarning that main logs at ERROR so it reaches the log ring and the help report. Validate keeps rejecting the ambiguity, so Settings save and bootstrap stay strict; only a file already on disk is repaired. Closes #988. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01C7xyXX1HYi2i1aax8EsCFx --- .changeset/boot-with-duplicate-site-meters.md | 5 ++ go/cmd/ftw/main.go | 5 ++ go/internal/config/config.go | 39 ++++++++++++++ go/internal/config/validate_site_test.go | 52 ++++++++++++++++--- 4 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 .changeset/boot-with-duplicate-site-meters.md diff --git a/.changeset/boot-with-duplicate-site-meters.md b/.changeset/boot-with-duplicate-site-meters.md new file mode 100644 index 000000000..44849908b --- /dev/null +++ b/.changeset/boot-with-duplicate-site-meters.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +A config with two `is_site_meter: true` drivers no longer stops the box from starting. The box boots with the first declared driver as the site meter — the same one older versions silently used — ignores the flag on the rest, and logs a clear error naming both drivers so the mistake is visible in the log and the help report. Saving such a config from Settings is still rejected. A driver install that accidentally added a second site meter used to crash-loop the box before the web UI came up, leaving SSH as the only way back in. diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index 92bf2ca7c..2954b4cfd 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -397,6 +397,11 @@ func main() { os.Exit(1) } slog.Info("config loaded", "site", cfg.Site.Name, "drivers", len(cfg.Drivers)) + // Repaired-but-wrong config: ERROR so it reaches the log ring and the + // support report, without stopping a boot the repair made safe. + for _, w := range cfg.LoadWarnings { + slog.Error(w) + } // ---- Open persistent state (SQLite) ---- statePath := "state.db" diff --git a/go/internal/config/config.go b/go/internal/config/config.go index 3aae51e36..a38d06d78 100644 --- a/go/internal/config/config.go +++ b/go/internal/config/config.go @@ -42,6 +42,13 @@ type Config struct { FleetPing *FleetPing `yaml:"fleet_ping,omitempty" json:"fleet_ping,omitempty"` Nova *Nova `yaml:"nova,omitempty" json:"nova,omitempty"` DeviceRepository *DeviceRepository `yaml:"device_repository,omitempty" json:"device_repository,omitempty"` + + // LoadWarnings collects recoverable problems Parse repaired instead of + // refusing the file: an on-disk config an older version accepted must + // still boot, or the operator loses the UI they would fix it with. The + // write path (Settings save, bootstrap) never populates this — it calls + // Validate directly and stays strict. Never serialized. + LoadWarnings []string `yaml:"-" json:"-"` } // AppLink controls the outbound connection the FTW app reaches this box @@ -1312,6 +1319,7 @@ func Parse(data []byte, baseDir string) (*Config, error) { c.AppLink = &AppLink{Enabled: false} } applyDefaults(&c) + c.demoteExtraSiteMeters() if err := c.Validate(); err != nil { return nil, err } @@ -1319,6 +1327,37 @@ func Parse(data []byte, baseDir string) (*Config, error) { return &c, nil } +// demoteExtraSiteMeters keeps the first declared is_site_meter driver and +// clears the flag on the rest, recording a LoadWarning per demotion. +// +// Load-time only. Duplicate site meters are an operator mistake Validate +// rejects on the write path, but a file already on disk was often written +// by an older version that silently used the first match +// (SiteMeterDriver's order) — refusing it at boot crash-loops the process +// before the HTTP listener binds, and the operator loses the very UI they +// would fix the config with (field incident 2026-08-29: a driver install +// on v1.15.0 left two site meters; the box updated to v2.3.0 and went +// dark until SSH). Demoting reproduces exactly what the older version +// dispatched against, and the warning makes the ambiguity visible where +// silence caused it to be missed. +func (c *Config) demoteExtraSiteMeters() { + kept := "" + for i := range c.Drivers { + d := &c.Drivers[i] + if !d.IsSiteMeter { + continue + } + if kept == "" { + kept = d.Name + continue + } + d.IsSiteMeter = false + c.LoadWarnings = append(c.LoadWarnings, fmt.Sprintf( + "config: drivers %q and %q both set is_site_meter: true; keeping %q as the site meter and ignoring the flag on %q — fix config.yaml so exactly one driver has it", + kept, d.Name, kept, d.Name)) + } +} + func topLevelYAMLNull(doc *yaml.Node, key string) bool { if doc == nil || doc.Kind != yaml.DocumentNode || len(doc.Content) != 1 { return false diff --git a/go/internal/config/validate_site_test.go b/go/internal/config/validate_site_test.go index 259d4f67e..f79adc8b1 100644 --- a/go/internal/config/validate_site_test.go +++ b/go/internal/config/validate_site_test.go @@ -8,13 +8,18 @@ import ( ) func loadSiteValidationConfig(t *testing.T, yaml string) error { + t.Helper() + _, err := loadSiteValidationConfigFull(t, yaml) + return err +} + +func loadSiteValidationConfigFull(t *testing.T, yaml string) (*Config, error) { t.Helper() path := filepath.Join(t.TempDir(), "config.yaml") if err := os.WriteFile(path, []byte(yaml), 0o600); err != nil { t.Fatal(err) } - _, err := Load(path) - return err + return Load(path) } func TestLoadRejectsMoreThanThreeFusePhases(t *testing.T) { @@ -47,7 +52,14 @@ func meterDriver(name string, siteMeter bool) Driver { } } -func TestLoadRejectsDuplicateSiteMeters(t *testing.T) { +// Load repairs duplicate site meters instead of refusing the file: a +// config an older version booted with (first match won silently) must +// keep booting after an update, or the operator loses the UI they would +// fix it with. Field incident 2026-08-29: a driver install on v1.15.0 +// left two is_site_meter drivers, and the v2.3.0 update crash-looped the +// box before the HTTP listener bound. The write path stays strict — see +// TestValidateRejectsDuplicateSiteMeters. +func TestLoadDemotesDuplicateSiteMetersAndWarns(t *testing.T) { yaml := strings.Replace(minimalYAML, "api:\n", ` - name: second-meter lua: drivers/second-meter.lua @@ -57,9 +69,37 @@ func TestLoadRejectsDuplicateSiteMeters(t *testing.T) { host: 192.168.1.154 api: `, 1) - err := loadSiteValidationConfig(t, yaml) - if err == nil || err.Error() != "exactly one driver may set is_site_meter: true (found 2)" { - t.Errorf("Load error = %v, want duplicate site meter validation error", err) + cfg, err := loadSiteValidationConfigFull(t, yaml) + if err != nil { + t.Fatalf("Load with duplicate site meters: %v", err) + } + if got := cfg.SiteMeterDriver(); got != "ferroamp" { + t.Errorf("site meter = %q, want the first declared %q", got, "ferroamp") + } + for _, d := range cfg.Drivers { + if d.Name == "second-meter" && d.IsSiteMeter { + t.Error("second-meter kept is_site_meter after load") + } + } + if len(cfg.LoadWarnings) != 1 || + !strings.Contains(cfg.LoadWarnings[0], `"ferroamp"`) || + !strings.Contains(cfg.LoadWarnings[0], `"second-meter"`) { + t.Errorf("LoadWarnings = %q, want one warning naming both drivers", cfg.LoadWarnings) + } +} + +// The write path (Settings save, bootstrap POST /api/config) calls +// Validate directly and must keep rejecting the ambiguity — the operator +// is present to fix it before it persists. +func TestValidateRejectsDuplicateSiteMeters(t *testing.T) { + c := &Config{ + Site: Site{SmoothingAlpha: 0.3}, + Fuse: Fuse{MaxAmps: 16, Phases: 3, Voltage: 230}, + Drivers: []Driver{meterDriver("a", true), meterDriver("b", true)}, + } + if err := c.Validate(); err == nil || + err.Error() != "exactly one driver may set is_site_meter: true (found 2)" { + t.Errorf("Validate error = %v, want duplicate site meter validation error", err) } }