fix(drivers): recover to the release's driver over a kept selection

.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.

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 {

go/internal/api/api_device_repository_test.go

@@ -497,7 +497,12 @@ func TestDriverCatalogNamesTheDriversThatRunEachFile(t *testing.T) {
 // drivers/<filename>.
 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")
+	}
+}

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.

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…" });
+});

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: ""
       });
     });