fix(drivers): recover to the release's driver over a kept selection - #1434
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6e7d422a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| recovered, recoveryErr = s.deps.DriverRepository.Rollback(activated.LogicalPath) | ||
| if recoveryErr == nil { | ||
| _, recoveryErr = s.restartManagedDriversExpected(context.Background(), recovered, restartState.ExpectedIdentities) |
There was a problem hiding this comment.
Recover failed activations when the driver path changed
When activating a retained version whose manifest renamed the driver file, the original remains active under its old logical path, so the newly activated path has no PreviousInstalledPath. If its runtime restart then fails, this Rollback(activated.LogicalPath) returns driver has no previous managed artifact and no configuration/runtime restoration occurs, leaving the device stopped. Filename changes are an explicitly supported switching flow, so this case needs to deactivate the failed path and restore/restart the captured original, as the install recovery path already does.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
| if _, err := os.Stat(activePath); artifact.InstalledPath == "" || err != nil { | ||
| var err error | ||
| targetPath, err = s.bundledDriverFor(artifact.DriverID, artifact.LogicalPath) |
There was a problem hiding this comment.
Treat only superseded selections as bundled
If the effective symlink is absent or unreadable for any reason other than intentional supersession—for example, rebuildEffective logged and swallowed a disk/permission error, or the retained artifact disappeared after validation—this branch silently starts the bundled driver. Fresh telemetry can then make the install/activate request return 200 with the requested managed artifact even though a different bundled version is running and the persisted config now points to it. Use the repository's semantic Runs result to detect supersession, and surface unexpected Stat failures so request acceptance and measured effect remain distinct.
AGENTS.md reference: AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
Problem
Since #1428, a release's newer copy can overtake a kept driver selection: the selection stays in
active, but no file for it is ineffective, and the release's own driver runs. The home box has this state now (Easee: kept 1.3.3, release 1.3.4 runs). The Codex review of #1422..#1433 found that two places still assumed the kept selection runs, and checking the fix found a third:Rollbackrestores the kept selection. Recovery then restartseffective/<file>, which does not exist:previous artifact reactivated but did not recover: … no such file or directory./activatecalledActivateInstalled(original). Coming back from a newer running version, this pinned the kept 1.3.3 as the owner's choice, so it ran over the release's 1.3.4 without anyone choosing it.Change
restartManagedDriversExpectedtargets the release's copy when no managed file runs at the path. This rule already applied afteruse_bundled; it now also covers an overtaken selection./activaterecovery usesRollback. Like the install recovery, it restores the selection and the choice together./versionsnames the overtaken selection assuperseded_version, taken from the manager's ownrunsrule (Manager.Runs). The list offers that version like any version on disk. Choosing it pins it, as before.Evidence
TestFailedTrialOverASupersededSelectionReturnsToTheRelease:On master the test fails at the install step with the missing-file error. With only the old activate recovery restored, it fails because 1.0.3 runs instead of 1.0.4.
Web:
a kept selection the release has overtaken can be chosen again(37/37 pass).go test ./internal/api ./internal/driverrepoandgo vetpass.The web change is one button on an existing row type. It needs a look in a browser: on the home box, Easee's kept v1.3.3 should show "on disk" and "Use this" under Settings › Devices › Versions.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MuerPFZFG88kgu8sWVHeq7