Skip to content

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

Merged
frahlg merged 1 commit into
masterfrom
fix/superseded-selection-recovery
Sep 25, 2026
Merged

frahlg merged 1 commit into
masterfrom
fix/superseded-selection-recovery

Conversation

@frahlg

@frahlg frahlg commented Sep 25, 2026

Copy link
Copy Markdown
Member

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 in effective, 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:

  1. Failed trial leaves the device stopped (P1). Install another version, the restart fails, and Rollback restores the kept selection. Recovery then restarts effective/<file>, which does not exist: previous artifact reactivated but did not recover: … no such file or directory.
  2. Failed switch runs the kept older version. The recovery in /activate called ActivateInstalled(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.
  3. The kept version could not be chosen again (P2). The Versions list showed the kept row as "selected", with no "Use this".

Change

  • restartManagedDriversExpected targets the release's copy when no managed file runs at the path. This rule already applied after use_bundled; it now also covers an overtaken selection.
  • The /activate recovery uses Rollback. Like the install recovery, it restores the selection and the choice together.
  • /versions names the overtaken selection as superseded_version, taken from the manager's own runs rule (Manager.Runs). The list offers that version like any version on disk. Choosing it pins it, as before.

Evidence

  • TestFailedTrialOverASupersededSelectionReturnsToTheRelease:

    1. Runs a real signed install and a Lua runtime.
    2. Replaces the release copy 1.0.2 with 1.0.4, which overtakes the kept 1.0.3.
    3. Fails an install, then fails an activate. After each, the device must emit the release's readings, keep 1.0.3 unpinned and have no managed file running.
    4. Choosing 1.0.3 afterwards must run it and pin it.

    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/driverrepo and go vet pass.

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

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
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T09:08:20.894102Z d6e7d42 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +377 to 379
recovered, recoveryErr = s.deps.DriverRepository.Rollback(activated.LogicalPath)
if recoveryErr == nil {
_, recoveryErr = s.restartManagedDriversExpected(context.Background(), recovered, restartState.ExpectedIdentities)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +541 to 543
if _, err := os.Stat(activePath); artifact.InstalledPath == "" || err != nil {
var err error
targetPath, err = s.bundledDriverFor(artifact.DriverID, artifact.LogicalPath)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@frahlg
frahlg merged commit 9044826 into master Sep 25, 2026
14 checks passed
@frahlg
frahlg deleted the fix/superseded-selection-recovery branch September 25, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant