From d6e7d422a80e16e514d75248eb84be8f5d7e0f4c Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Fri, 25 Sep 2026 11:04:48 +0200 Subject: [PATCH] fix(drivers): recover to the release's driver over a kept selection When a release's newer copy overtakes a kept driver selection, the selection stays but does not run. Three places still assumed it ran: - A failed trial restarted the kept selection's file in the effective directory, which no longer exists, so the device stayed stopped. The restart now falls back to the release's copy when no managed file runs at the path. - A failed switch recovered with ActivateInstalled, which pinned the kept older version and ran it over the release's copy. It now uses Rollback, which restores the selection and the choice together. - The Versions list marked the kept selection as selected and hid its "Use this". /versions now names it as superseded_version, and the list offers it like any version on disk. Found by the Codex review of #1422..#1433. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7 --- .changeset/superseded-selection-recovery.md | 9 +++ go/internal/api/api_device_repository.go | 24 ++++-- go/internal/api/api_device_repository_test.go | 80 ++++++++++++++++++- go/internal/driverrepo/manager.go | 10 +++ web/driver-versions.test.mjs | 17 ++++ web/settings/tabs/devices.js | 14 +++- 6 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 .changeset/superseded-selection-recovery.md diff --git a/.changeset/superseded-selection-recovery.md b/.changeset/superseded-selection-recovery.md new file mode 100644 index 000000000..7abb47e6a --- /dev/null +++ b/.changeset/superseded-selection-recovery.md @@ -0,0 +1,9 @@ +--- +"ftw": patch +--- + +When a release's newer driver runs instead of a kept older selection, a failed +trial of another version now returns the device to the release's driver. Before, +the recovery restarted a file that no longer ran and left the device stopped, or +made the kept older version run. The kept version now offers "Use this" under +Settings › Devices, so the owner can go back to it. diff --git a/go/internal/api/api_device_repository.go b/go/internal/api/api_device_repository.go index 6abfae2fb..f829e33ed 100644 --- a/go/internal/api/api_device_repository.go +++ b/go/internal/api/api_device_repository.go @@ -303,19 +303,27 @@ func (s *Server) handleDeviceRepositoryVersions(w http.ResponseWriter, r *http.R logicalPath = candidate.Driver.Path } } - release, chosen := "", "" + // A selection the release's newer copy has overtaken is kept but does + // not run, so the picker offers it like any version on disk. + release, chosen, superseded := "", "", "" if logicalPath != "" { release = s.deps.DriverRepository.ReleaseVersion(logicalPath) for _, installed := range versions { - if installed.Active && s.deps.DriverRepository.Chosen(logicalPath, installed.Version) { + if !installed.Active { + continue + } + if s.deps.DriverRepository.Chosen(logicalPath, installed.Version) { chosen = installed.Version } + if !s.deps.DriverRepository.Runs(installed) { + superseded = installed.Version + } } } writeJSON(w, 200, map[string]any{ "driver_id": r.PathValue("id"), "installed": versions, "available": available, "logical_path": logicalPath, "release_version": release, "chosen_version": chosen, - "release_source": s.bundledSource(), + "superseded_version": superseded, "release_source": s.bundledSource(), }) } @@ -362,8 +370,11 @@ func (s *Server) handleDeviceRepositoryActivate(w http.ResponseWriter, r *http.R if restartErr != nil { recoveryErr := error(nil) if original != nil { + // A rollback restores the selection and the choice that went + // with it. Activating the original again would pin a kept + // version the release had overtaken, and run it instead. var recovered state.DriverRepoInstall - recovered, recoveryErr = s.deps.DriverRepository.ActivateInstalled(driverID, original.Version, original.SHA256) + recovered, recoveryErr = s.deps.DriverRepository.Rollback(activated.LogicalPath) if recoveryErr == nil { _, recoveryErr = s.restartManagedDriversExpected(context.Background(), recovered, restartState.ExpectedIdentities) } @@ -524,7 +535,10 @@ func (s *Server) restartManagedDriversExpected(ctx context.Context, artifact sta rel := filepath.FromSlash(strings.TrimPrefix(artifact.LogicalPath, "drivers/")) activePath := filepath.Join(s.managedDriverDir(), rel) targetPath := activePath - if artifact.InstalledPath == "" { + // No managed file runs at this path either when nothing is selected + // (UseBundled) or when the release's newer copy overtakes the selection; + // the release's own file runs then. + if _, err := os.Stat(activePath); artifact.InstalledPath == "" || err != nil { var err error targetPath, err = s.bundledDriverFor(artifact.DriverID, artifact.LogicalPath) if err != nil { diff --git a/go/internal/api/api_device_repository_test.go b/go/internal/api/api_device_repository_test.go index 57d026564..87530bce0 100644 --- a/go/internal/api/api_device_repository_test.go +++ b/go/internal/api/api_device_repository_test.go @@ -497,7 +497,12 @@ func TestDriverCatalogNamesTheDriversThatRunEachFile(t *testing.T) { // drivers/. func (f *driverUpdateFixture) publishAs(id, filename, version string) { f.t.Helper() - source := []byte(strings.Replace(string(updateDriverLua(version, "P1-123", `host.emit("meter", {w=103})`)), + f.publishSourceAs(id, filename, version, "P1-123", `host.emit("meter", {w=103})`) +} + +func (f *driverUpdateFixture) publishSourceAs(id, filename, version, serial, poll string) { + f.t.Helper() + source := []byte(strings.Replace(string(updateDriverLua(version, serial, poll)), `id = "esphome-dsmr"`, `id = "`+id+`"`, 1)) f.mu.Lock() f.lua = source @@ -633,3 +638,76 @@ func TestVersionsShowTheReleaseAndTheOwnersChoice(t *testing.T) { } } } + +// A selection a newer release has overtaken is kept but does not run. A +// failed trial of another version must come back to what ran -- the +// release's copy -- and must not make the kept selection run instead. +func TestFailedTrialOverASupersededSelectionReturnsToTheRelease(t *testing.T) { + f := newDriverUpdateFixture(t, "running") + release := func(version string, watts int) { + t.Helper() + source := strings.Replace(string(updateDriverLua(version, "P1-123", fmt.Sprintf(`host.emit("meter", {w=%d})`, watts))), + `id = "esphome-dsmr"`, `id = "esphome_dsmr"`, 1) + if err := os.WriteFile(f.bundled, []byte(source), 0o644); err != nil { + t.Fatal(err) + } + } + release("1.0.2", 102) + f.publishAs("esphome_dsmr", "esphome_dsmr.lua", "1.0.3") + f.requestFor("esphome_dsmr", "install", `{"repository_id":"test"}`, 200) + f.reading(103) + + // A Core update brings 1.0.4: the 1.0.3 selection stays but no longer runs. + release("1.0.4", 104) + f.s.deps.DriverRepository.ApplyBundled() + f.s.deps.Cfg.Drivers[0].Lua = f.bundled + if err := f.s.deps.Registry.Restart(context.Background(), f.s.deps.Cfg.Drivers[0]); err != nil { + t.Fatal(err) + } + f.reading(104) + w := httptest.NewRecorder() + f.s.Handler().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/api/device_repository/drivers/esphome_dsmr/versions", nil)) + var versions struct { + Superseded string `json:"superseded_version"` + } + if w.Code != 200 || json.Unmarshal(w.Body.Bytes(), &versions) != nil || versions.Superseded != "1.0.3" { + t.Fatalf("versions: HTTP %d %s; want the kept 1.0.3 named as overtaken", w.Code, w.Body.String()) + } + + stillTheRelease := func(action string) { + t.Helper() + f.reading(104) + if got := f.s.activeManagedDriverVersion("esphome_dsmr"); got != "1.0.3" { + t.Fatalf("after the failed %s the kept selection is %q, want 1.0.3", action, got) + } + if f.s.deps.DriverRepository.Chosen("drivers/esphome_dsmr.lua", "1.0.3") { + t.Fatalf("the failed %s turned the kept 1.0.3 into a choice that runs over the release", action) + } + if _, err := os.Stat(filepath.Join(f.s.managedDriverDir(), "esphome_dsmr.lua")); !os.IsNotExist(err) { + t.Fatalf("after the failed %s a managed file runs instead of the release's (stat err %v)", action, err) + } + } + + // A version for another meter fails its identity check. + f.publishSourceAs("esphome_dsmr", "esphome_dsmr.lua", "1.0.5", "OTHER-METER", `host.emit("meter", {w=105})`) + response := f.requestFor("esphome_dsmr", "install", `{"repository_id":"test"}`, 502) + if msg, _ := response["error"].(string); strings.Contains(msg, "rollback failed") { + t.Fatalf("install recovery failed: %s", msg) + } + stillTheRelease("install") + + // The failed version is on disk now; switching to it fails the same way. + response = f.requestFor("esphome_dsmr", "activate", `{"version":"1.0.5"}`, 502) + if msg, _ := response["error"].(string); strings.Contains(msg, "recovery failed") { + t.Fatalf("activate recovery failed: %s", msg) + } + stillTheRelease("activate") + + // The owner can still go back to the kept version on purpose, and it + // then stays as a choice over the release's newer copy. + f.requestFor("esphome_dsmr", "activate", `{"version":"1.0.3"}`, 200) + f.reading(103) + if !f.s.deps.DriverRepository.Chosen("drivers/esphome_dsmr.lua", "1.0.3") { + t.Fatal("going back to the kept 1.0.3 was not recorded as the owner's choice") + } +} diff --git a/go/internal/driverrepo/manager.go b/go/internal/driverrepo/manager.go index 0edbbdf27..127ea11c8 100644 --- a/go/internal/driverrepo/manager.go +++ b/go/internal/driverrepo/manager.go @@ -960,6 +960,16 @@ func (m *Manager) Chosen(logicalPath, version string) bool { return pinned == version } +// Runs reports whether a managed selection runs, rather than the release's +// newer copy at its path. A selection that does not run is kept all the same. +func (m *Manager) Runs(installed state.DriverRepoInstall) bool { + if m.store == nil { + return true + } + ok, _ := m.runs(installed) + return ok +} + // pinKey holds the version of a managed selection the owner chose over a // newer one that was running. previousPinKey keeps the value it replaced, so // undoing an activation also undoes its effect on the choice. diff --git a/web/driver-versions.test.mjs b/web/driver-versions.test.mjs index cc7d06350..511ed7e88 100644 --- a/web/driver-versions.test.mjs +++ b/web/driver-versions.test.mjs @@ -678,3 +678,20 @@ test("a beta outage is shown but the list still redraws", async () => { assert.match(textOf(panel), /Checked, but beta channel: connection refused/); assert.ok(rowOf(panel, "v1.1.1"), "the stable rows are drawn again"); }); + +test("a kept selection the release has overtaken can be chosen again", async () => { + const { api, calls } = load(); + const panel = element("div"); + api.render(panel, "ferroamp", { ...PAYLOAD, release_version: "1.0.2", superseded_version: "1.0.0" }, { + runningVersion: "1.0.2", runningSource: "bundled", logicalPath: "drivers/ferroamp.lua", + }); + + assert.match(textOf(rowOf(panel, "release")), /running now · this release/); + const kept = rowOf(panel, "v1.0.0"); + assert.doesNotMatch(textOf(kept), /selected/, "the release's copy runs, not the kept 1.0.0"); + assert.match(textOf(kept), /on disk/); + buttonsOf(kept)[0].click(); + await settle(); + assert.equal(calls[0].path, "/api/device_repository/drivers/ferroamp/activate"); + assert.deepEqual(calls[0].body, { version: "1.0.0", sha256: "aa11…" }); +}); diff --git a/web/settings/tabs/devices.js b/web/settings/tabs/devices.js index 481990e15..fdd5a97be 100644 --- a/web/settings/tabs/devices.js +++ b/web/settings/tabs/devices.js @@ -213,6 +213,12 @@ var available = (body && body.available) || []; var release = (body && body.release_version) || ""; var chosen = (body && body.chosen_version) || ""; + // A selection the release's newer copy has overtaken is kept but does not + // run. It is offered like any version on disk, so the owner can go back. + var superseded = (body && body.superseded_version) || ""; + function selected(item, version) { + return !!(item && item.active && version !== superseded); + } var rows = []; var seen = {}; @@ -242,8 +248,8 @@ channel: candidate.channel === "beta" ? "beta" : "stable", changes: changesURL(candidate.repository, driver.source_commit, driver.filename), downloaded: !!onDisk, - active: !!(onDisk && onDisk.active), - chosen: !!(onDisk && onDisk.active && chosen && driver.version === chosen), + active: selected(onDisk, driver.version), + chosen: !!(selected(onDisk, driver.version) && chosen && driver.version === chosen), verification: verificationLabel((driver.metadata || {}).verification_status) }); }); @@ -260,8 +266,8 @@ channel: "", changes: "", downloaded: true, - active: !!item.active, - chosen: !!(item.active && chosen && item.version === chosen), + active: selected(item, item.version), + chosen: !!(selected(item, item.version) && chosen && item.version === chosen), verification: "" }); });