Skip to content

fix(plugins): quarantine stuck plugins without deleting them#2160

Merged
bajrangCoder merged 10 commits into
Acode-Foundation:mainfrom
bajrangCoder:fix/plugin-timeout-quarantine
Jun 7, 2026
Merged

fix(plugins): quarantine stuck plugins without deleting them#2160
bajrangCoder merged 10 commits into
Acode-Foundation:mainfrom
bajrangCoder:fix/plugin-timeout-quarantine

Conversation

@bajrangCoder
Copy link
Copy Markdown
Member

@bajrangCoder bajrangCoder commented Jun 6, 2026

Summary

Fixes plugin disappearance caused by startup load failures/timeouts deleting installed plugin folders.

New Behavior

  • Plugin files are no longer deleted when plugin loading fails.
  • A plugin that throws a real load error is marked broken and persisted as disabled via pluginsDisabled.
  • A plugin that exceeds the normal load timeout is treated as slow first:
    • after 15s, app startup continues
    • the plugin continues loading in the background
    • if it finishes successfully, it is marked loaded and the timeout state is cleared
  • If a timed-out plugin still has not finished after 60s, it is treated as stuck and persisted as disabled.
  • Disabled/quarantined plugins will not be retried on every app restart, but users still keep the installed plugin files and can manually re-enable or uninstall them.

Why

Previously, the startup plugin loader treated slow plugins and broken plugins the same. Any failed/timed-out plugin was added to failedPlugins, then cleanupFailedPlugins() deleted its directory from PLUGIN_DIR.

That could make plugins appear to disappear permanently, especially on slower devices where valid plugins may exceed the startup timeout.

Fixes: #2010

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 6, 2026

Greptile Summary

This PR replaces the old "delete on failure" plugin cleanup strategy with a quarantine system that preserves plugin files. A slow plugin is now given 15 s to start before startup continues, then an additional 45 s to load in the background before being persisted to pluginsDisabled. Real errors immediately mark the plugin broken without deleting its directory.

  • Timeout-vs-error disambiguation: A typed PluginLoadTimeoutError class replaces the old string-matched sentinel, and a queue (pluginDisabledUpdateQueue) serializes all settings.update writes, addressing both previously flagged races.
  • Theme-plugin quarantine fix: The loadOnlyTheme filter now includes enabledMap[pluginId] !== true, so theme plugins quarantined in a prior session are no longer retried on every boot.
  • installPlugin.js: Swaps loadPlugin for the new exported loadPluginWithTimeout, so dependency loads during installation participate in the same timeout/quarantine logic.

Confidence Score: 4/5

Safe to merge with awareness of one edge case: when a plugin fails to load during installation, the quarantine entry is written to pluginsDisabled before installPlugin.js deletes the directory, leaving an orphaned settings key for a plugin that no longer exists.

All three issues flagged in prior reviews are addressed — the timeout/error distinction now uses a typed PluginLoadTimeoutError class, the settings.update race is resolved via a serialized pluginDisabledUpdateQueue, and the missing disabled-check for theme plugins in loadOnlyTheme is now present. The one remaining gap is the orphaned pluginsDisabled entry that persists when a plugin load fails during installation and installPlugin.js subsequently deletes the directory; it is self-correcting on a successful reinstall and harmless on future startups, but it represents real state drift between the filesystem and stored settings.

src/lib/loadPlugins.js — the interaction between markPluginBroken (called inside the pluginLoadPromise .catch chain) and installPlugin.js's directory-cleanup catch block is worth a second look.

Important Files Changed

Filename Overview
src/lib/loadPlugins.js Core quarantine logic rewrite: adds PluginLoadTimeoutError, loadPluginWithTimeout, markPluginLoaded/Broken/TimedOut, and a serialized updatePluginDisabled queue. The theme-plugin disabled-check omission is fixed. One subtle gap: when loadPluginWithTimeout is called from installPlugin.js and a real (non-timeout) error fires, markPluginBroken writes a pluginsDisabled entry before installPlugin.js deletes the directory, leaving an orphaned settings key.
src/lib/installPlugin.js Minimal change: import swapped from loadPlugin to loadPluginWithTimeout. The timeout/quarantine path now affects the install flow — a plugin that times out during install is silently treated as loading-in-background rather than failing the install, which is intentional per the PR description.

Sequence Diagram

sequenceDiagram
    participant S as loadPlugins / installPlugin
    participant LPT as loadPluginWithTimeout
    participant LP as loadPlugin (plugin code)
    participant MT as markPluginTimedOut (45s timer)
    participant Settings as pluginsDisabled (settings)

    S->>LPT: loadPluginWithTimeout(pluginId, justInstalled)
    LPT->>LP: loadPlugin() [background promise]
    LPT->>LPT: Promise.race(pluginLoadPromise, 15s timeout)

    alt Plugin loads within 15s
        LP-->>LPT: resolve
        LPT->>LPT: markPluginLoaded → LOADED_PLUGINS.add
        LPT-->>S: return true
    else Real error within 15s
        LP-->>LPT: reject(error)
        LPT->>Settings: "markPluginBroken → pluginsDisabled[id]=true"
        LPT-->>S: throw error
    else Timeout at 15s
        LPT->>MT: markPluginTimedOut(pluginId, pluginState)
        Note over MT: BROKEN_PLUGINS set in-memory, 45s timer starts
        LPT-->>S: return false (startup continues)
        par Plugin loads in background (15s-60s)
            LP-->>LPT: resolve
            LPT->>LPT: markPluginLoaded
            Note over Settings: If AUTO_DISABLED: pluginsDisabled[id] deleted
        and 60s timer fires (plugin still stuck)
            MT->>Settings: "markPluginBroken → pluginsDisabled[id]=true"
        end
    end
Loading

Reviews (6): Last reviewed commit: "chore: fmt" | Re-trigger Greptile

Comment thread src/lib/loadPlugins.js
Comment thread src/lib/loadPlugins.js
@bajrangCoder

This comment was marked as outdated.

@bajrangCoder

This comment was marked as outdated.

@UnschooledGamer UnschooledGamer added the CI: RUN ON-DEMAND PREVIEW RELEASES Triggers an on-demand preview build for this pull request via CI workflow. label Jun 6, 2026
@github-actions github-actions Bot removed the CI: RUN ON-DEMAND PREVIEW RELEASES Triggers an on-demand preview build for this pull request via CI workflow. label Jun 6, 2026
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 6, 2026

Preview Release for this, has been built.

Click here to view that github actions build

@bajrangCoder

This comment was marked as outdated.

@bajrangCoder

This comment was marked as outdated.

This commit fixes `updatePluginDisabled` that skipped its no-op guard when disabled=false and the key was absent, producing a redundant `settings.update`
Added error logging for plugin timeout and state updates.
@UnschooledGamer

This comment was marked as outdated.

@bajrangCoder bajrangCoder merged commit 439d43f into Acode-Foundation:main Jun 7, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in The Code Board - Acode Jun 7, 2026
@bajrangCoder bajrangCoder deleted the fix/plugin-timeout-quarantine branch June 7, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Plugins sometimes disappear

2 participants