From 2f35ea2e42c3875f79aa43480ff03ee58f53bac2 Mon Sep 17 00:00:00 2001 From: darken Date: Fri, 19 Jun 2026 18:08:54 +0200 Subject: [PATCH] fix(aggregators): distinguish restored-but-disabled adapters from unconfigured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A restored identity is seeded disabled, so it reports not_installed with configured=true and rendered as "Not set up" — identical to a never-touched adapter, making a successful restore look like a no-op. Map that state to a distinct "Ready to enable" badge, and have the restore summary note that identities were restored and need enabling to resume feeding. Also trim the aggregator backup section to {kind, schema_version, aggregators}, dropping the RPC envelope fields (result, protocol_version) that leaked in via verbatim embedding. Mirrors the Wi-Fi reshape; the lenient importer keeps older backups loadable. --- internal/server/backup_handlers.go | 22 ++++++++++- internal/server/backup_handlers_test.go | 52 +++++++++++++++++++++++++ web/assets/app.js | 19 ++++++--- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/internal/server/backup_handlers.go b/internal/server/backup_handlers.go index 77a15ff..220c6f0 100644 --- a/internal/server/backup_handlers.go +++ b/internal/server/backup_handlers.go @@ -238,7 +238,21 @@ func (s *Server) exportAggregatorsSection(ctx context.Context) (json.RawMessage, probe.Kind != "aggregator-backup" || probe.SchemaVersion != 1 || len(probe.Aggregators) == 0 { return nil, fmt.Errorf("aggregator export: not a valid backup payload (kind=%q schema=%d)", probe.Kind, probe.SchemaVersion) } - return json.RawMessage(body), nil + // Reshape to just the fields the section needs: {kind, schema_version, + // aggregators}. The helper's reply also carries the generic RPC envelope + // fields (result, protocol_version), which mean nothing once the payload is + // at rest in a backup file. Trimming mirrors exportWifiSection; the importer + // is lenient about extra keys, so this is backward-compatible with older + // backups that still embed the full envelope. + clean, err := json.Marshal(struct { + Kind string `json:"kind"` + SchemaVersion int `json:"schema_version"` + Aggregators json.RawMessage `json:"aggregators"` + }{probe.Kind, probe.SchemaVersion, probe.Aggregators}) + if err != nil { + return nil, fmt.Errorf("aggregator export: reshape: %w", err) + } + return json.RawMessage(clean), nil } // exportWifiSection reshapes the helper's export envelope into the section the @@ -555,7 +569,11 @@ func (s *Server) restoreAggregators(ctx context.Context, env combinedBackupEnvel return "failed", "import error" } if status == http.StatusOK || status == http.StatusAccepted { - return "applied", "" + // Import seeds each identity disabled; the operator re-enables to resume + // feeding (and trigger the vendor install). The checklist renders this as + // "restored — ", so a restore that leaves adapters showing + // "Ready to enable" doesn't look like a no-op. + return "applied", "enable each adapter to resume feeding" } // The helper returns the same `rejected` code for an empty backup, an // unknown adapter, AND an adapter that is currently enabled. Only the last diff --git a/internal/server/backup_handlers_test.go b/internal/server/backup_handlers_test.go index f65d3d1..d2a6e86 100644 --- a/internal/server/backup_handlers_test.go +++ b/internal/server/backup_handlers_test.go @@ -187,6 +187,16 @@ func TestBackupExport_AssemblesAllSections(t *testing.T) { if strings.Contains(string(env.Sections["wifi"]), "status") { t.Errorf("wifi section should not carry status: %s", env.Sections["wifi"]) } + // Aggregators section reshaped to {kind, schema_version, aggregators} — the + // helper's RPC envelope fields (result, protocol_version) mean nothing at + // rest and must be stripped, mirroring the Wi-Fi reshape above. + agg := string(env.Sections["aggregators"]) + if strings.Contains(agg, "protocol_version") || strings.Contains(agg, `"result"`) { + t.Errorf("aggregators section should not carry RPC envelope fields: %s", agg) + } + if !strings.Contains(agg, "aggregator-backup") { + t.Errorf("aggregators section missing kind: %s", agg) + } } func TestBackupExport_OmitsIdentityWhenUnclaimed(t *testing.T) { @@ -437,8 +447,13 @@ func TestBackupRestore_EmptyAggregatorsSkippedNotFailed(t *testing.T) { }{ // The real empty export — must skip without touching the helper. {"valid empty set", testAggExportEnvelope, rejectEmpty, "skipped", false}, + // The reshaped export drops result/protocol_version; a trimmed empty set + // must still short-circuit to a skip exactly like the full envelope. + {"trimmed empty set", `{"kind":"aggregator-backup","schema_version":1,"aggregators":{}}`, rejectEmpty, "skipped", false}, // A populated backup goes through the helper and applies. {"populated", testAggImportPopulated, ok, "applied", true}, + // A trimmed populated export (new shape) must apply just the same. + {"trimmed populated", `{"kind":"aggregator-backup","schema_version":1,"aggregators":{"fr24":{"mlat_enabled":false,"fields":{"sharing_key":"deadbeef"}}}}`, ok, "applied", true}, // Malformed / ambiguous shapes must NOT be mistaken for empty: they // reach the helper rather than being silently skipped. {"aggregators null", `{"kind":"aggregator-backup","schema_version":1,"aggregators":null}`, rejectEmpty, "failed", true}, @@ -472,6 +487,43 @@ func TestBackupRestore_EmptyAggregatorsSkippedNotFailed(t *testing.T) { } } +// A successful aggregator restore seeds identities disabled, so the checklist +// must carry a non-empty reason telling the operator to re-enable them — +// otherwise the row reads a bare "restored" and the restore looks like a no-op. +func TestBackupRestore_AggregatorsAppliedReasonGuidesEnable(t *testing.T) { + h := newWriteHarness(t) + h.stdinResultFor = func(argv []string) wexec.Result { + if argvHas(argv, "apl-aggregator", "import") { + return wexec.Result{Stdout: []byte(`{"protocol_version":1,"result":"ok"}`)} + } + return okStdin(argv) + } + sections := fullRestoreSections(t) + r := postJSON(t, h.client, h.ts.URL+"/api/backup/restore", combinedBody(sections)) + defer r.Body.Close() + + var reason string + found := false + for _, line := range bytes.Split(bytes.TrimSpace(readBody(t, r)), []byte("\n")) { + if len(bytes.TrimSpace(line)) == 0 { + continue + } + var ev restoreEvent + if err := json.Unmarshal(line, &ev); err != nil { + t.Fatalf("bad ndjson line %q: %v", line, err) + } + if ev.Type == "section" && ev.Section == "aggregators" { + reason, found = ev.Reason, true + } + } + if !found { + t.Fatal("no aggregators section event in restore stream") + } + if reason == "" { + t.Error("aggregators applied reason is empty; operator gets no enable guidance") + } +} + func TestBackupRestore_RejectsOverCostPHC(t *testing.T) { h := newWriteHarness(t) h.stdinResultFor = okStdin diff --git a/web/assets/app.js b/web/assets/app.js index 87bbb23..7384d8e 100644 --- a/web/assets/app.js +++ b/web/assets/app.js @@ -3677,6 +3677,7 @@ installing: ["Installing…", "warn"], stopped: ["Off", "na"], not_installed: ["Not set up", "na"], + configured_off: ["Ready to enable", "na"], failed: ["Setup failed", "err"], decoder_unavailable: ["Decoder not ready", "warn"], network_unavailable: ["Network unavailable", "warn"], @@ -3695,7 +3696,13 @@ function aggDisplayState(a) { const s = a.state; if (s === "installing" || s === "removing" || s === "applying" || s === "failed") return s; - return a.external_install ? "unmanaged" : s; + if (a.external_install) return "unmanaged"; + // A restored (or otherwise saved-but-disabled) identity reports + // not_installed with configured=true: credentials are on disk, the vendor + // binary just isn't enabled yet. Distinguish it from a never-touched + // adapter so a restore doesn't read as "Not set up" / a no-op. + if (s === "not_installed" && a.configured) return "configured_off"; + return s; } // buildAdapterTile renders one configured/active adapter as a nav tile that @@ -3884,7 +3891,7 @@ const r = rows[key]; if (!r) return; let sev = "na", text = status; - if (status === "applied") { sev = "ok"; text = "restored"; } + if (status === "applied") { sev = "ok"; text = "restored" + (reason ? " — " + reason : ""); } else if (status === "skipped") { sev = "na"; text = "skipped" + (reason ? " — " + reason : ""); } else if (status === "failed") { sev = "err"; text = "failed" + (reason ? " — " + reason : ""); } r.dot.className = "wc-tile__dot wc-tile__dot--" + sev; @@ -4397,7 +4404,7 @@ actions.appendChild(viewLogs); render(el("section", { class: "wc-card" }, - aggDetailHead("Flightradar24", a.state), + aggDetailHead("Flightradar24", aggDisplayState(a)), el("p", { class: "muted" }, a.enabled ? "Feeding Flightradar24." : "Set up but not feeding right now."), buildAggStatusBlock(a), inlineErr, @@ -4486,7 +4493,7 @@ recheck(); render(el("section", { class: "wc-card" }, - aggDetailHead("Flightradar24", a.state), + aggDetailHead("Flightradar24", aggDisplayState(a)), el("p", { class: "muted" }, "Send your receiver's data to Flightradar24."), buildAggStatusBlock(a), form, @@ -4571,7 +4578,7 @@ actions.appendChild(viewLogs); render(el("section", { class: "wc-card" }, - aggDetailHead("FlightAware", a.state), + aggDetailHead("FlightAware", aggDisplayState(a)), el("p", { class: "muted" }, a.enabled ? "Feeding FlightAware." : "Set up but not feeding right now."), buildAggStatusBlock(a), inlineErr, @@ -4629,7 +4636,7 @@ recheck(); render(el("section", { class: "wc-card" }, - aggDetailHead("FlightAware", a.state), + aggDetailHead("FlightAware", aggDisplayState(a)), el("p", { class: "muted" }, "Send your receiver's data to FlightAware."), buildAggStatusBlock(a), form,