Skip to content

fix(plugins): keep updated plugin marked Installed instead of reoffering Update#1626

Merged
datlechin merged 1 commit into
mainfrom
fix/plugin-update-stale-version
Jun 8, 2026
Merged

fix(plugins): keep updated plugin marked Installed instead of reoffering Update#1626
datlechin merged 1 commit into
mainfrom
fix/plugin-update-stale-version

Conversation

@datlechin

Copy link
Copy Markdown
Member

Problem

After updating a plugin in Settings, the row shows the green Installed state, then a few seconds later (often noticed when you click to another plugin and back) it shows the Update button again. The plugin never settles into a stable Installed state.

Root cause

The revert is not triggered by navigation, it is a stale version read plus a timer.

PluginInstaller.atomicReplace swaps the plugin bundle on disk at the same URL. PluginManager.registerBundle then read the new version from bundle.infoDictionary["CFBundleShortVersionString"]. But Bundle(url:) returns a process-cached CFBundle whose info dictionary was populated at launch-time discovery with the old Info.plist, and CFBundle never re-reads it. So the new PluginEntry.version equalled the old version and hasUpdate(for:) stayed true. On success the install tracker shows a green checkmark for 3 seconds, then its timer clears the tracker entry, and because hasUpdate was still true the row fell back to the Update button.

Changes

  • PluginManager: read CFBundleShortVersionString straight from Contents/Info.plist on disk via a new bundleShortVersion(at:), bypassing the stale CFBundle cache. hasUpdate now turns false after an update completes.
  • PluginManager+Install: drop the duplicate completeInstall in updateFromRegistry (the coordinator already calls it; the reconciliation path never opened a tracker entry). Stops two 3s timers racing.
  • BrowsePluginsView: restructure rowStatusBadge so the tracker phase is one switch checked first (was duplicated and nested under hasUpdate, making the success flash unreachable once the version was correct). hasUpdate(for:) now delegates to the canonical PluginManager.registryUpdate(for:), which adds the user-installed and non-theme guards the local copy lacked.

Tests

New PluginBundleVersionTests seeds the CFBundle cache, rewrites Info.plist in place, then asserts bundleShortVersion(at:) returns the new version while Bundle(url:).infoDictionary still returns the stale one, covering both the bug and the fix.

No UI test: the install/update flow needs a live registry download and is not deterministic.

@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: 9e78cefbae

ℹ️ 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".

let info = bundle.infoDictionary ?? [:]
let version: String
if let declared = info["CFBundleShortVersionString"] as? String {
if let declared = Self.bundleShortVersion(at: url) {

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 Don’t mark a cached bundle as the new version

When updating a plugin that has already been discovered or loaded at this same URL, loadPluginAsync still obtains and loads the Bundle before replaceExistingPlugin unloads the existing entry, so the bundle passed here can be the cached CFBundle object from the previous install. Reading only CFBundleShortVersionString from the replaced Info.plist then creates a PluginEntry that advertises the registry version while its principal class/capabilities can still come from the cached old bundle, causing registryUpdate(for:) to stop offering the update even though the running plugin code was not refreshed. Please avoid mixing the disk version with a cached Bundle (or unload/avoid the cached bundle before loading the replacement).

Useful? React with 👍 / 👎.

@datlechin datlechin merged commit 63c1d19 into main Jun 8, 2026
4 checks passed
@datlechin datlechin deleted the fix/plugin-update-stale-version branch June 8, 2026 12:02
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