In-session module-plugin upgrade orphans the old instance — stale audio graph keeps playing (stems)

[topkoa]

Summary

The plugin loader supports upgrading a plugin within a running session — but for
scriptType: "module" plugins that's unsound: an ES module can't be un-executed. The old
module instance survives the "upgrade" with all its live state. For a stateful audio plugin
(stems), that means an orphaned audio graph that keeps playing alongside the new
instance's graph.

Mechanism

static/js/plugin-loader.js (~645–682):

• loadedScripts.get(plugin.id) !== wantedVersion → on a version change it calls
  _removePluginScriptTags(plugin.id) and injects screen.js?v=<newVersion> (cache-bust is
  deliberate: "so a plugin upgrade within the same browser session fetches the new screen.js").
• Removing a <script type="module"> tag does not tear down the already-evaluated module.
  The old instance's module state, event listeners, capability registrations, and Web Audio
  nodes all stay live.

For the stems plugin specifically (plugins/stems, scriptType: "module"):

• Both instances observe the playback lifecycle → both build audio graphs on the next
  song (double audio, initially sample-aligned so it's easy to miss).
• Only the newest instance installs the #audio shims, so transport commands (seek/pause/
  rate) reach only it. After a seek, the orphan keeps playing from the pre-seek position —
  audibly desynced vocals/instruments while the on-screen clock, notes, and lyrics follow the
  live instance.

Any stateful module plugin is affected (audio graphs, MutationObservers, WS handlers,
window.playSong wraps); audio is just the loudest failure mode.

When it triggers

Any in-session version change picked up by _refreshPluginsSoon / loadPlugins(): Plugin
Manager updates, and in dev, a git pull in a plugin dir while the app is running (this repo
bumped stems to 0.9.0 today — a running session would have orphaned 0.8.2).

Possibly related field report (unconfirmed link)

Persistent post-fast-forward lyric/vocal desync observed on one song ("Marks Of The Evil
One", zip pack). Pack data was verified internally consistent (lyrics ↔ vocals stem ↔ full
mix cross-correlated within ~9 ms across the whole song), and the lyric renderer is
time-faithful — the symptom matches the orphan mechanism (audio from the wrong position
after seek while the clock is right). However the report claims it survived a full app
restart, which an orphan should not — so this issue is filed for the loader hole on its own
merits; the field bug may have an additional cause. Repro attempts ongoing.

Suggested directions

• Minimum: make module-plugin upgrade not silently unsound — e.g. require a
  window.__<plugin>Instance.destroy()-style handshake (stem_mixer already implements this
  pattern) before injecting the new version, and skip the upgrade (with a "reload required"
  notice) when the plugin doesn't expose one.
• Alternatively: on any module-plugin version change, prompt/force a renderer reload instead
  of hot-swapping.
• The plugin-runtime-idempotent.v1 standard could grow an explicit teardown contract for
  module plugins (the module-cache idempotency it relies on only covers same-version
  re-injection, not upgrades).

🤖 Generated with Claude Code

[topkoa]

Status update after re-checking against main @ 8297afc (2026-07-23). The hole is still present — no loader change since #897 (7565886), and no PR references this issue — but two parts of the report need correcting, and one supporting fact is worth adding.

Field report: resolved, and it was unrelated

The "Marks Of The Evil One" post-fast-forward desync is closed out: the pack's lyrics.json had been scrambled by the stem_splitter re-align job (per-word deltas −72s..+3.9s vs. the original — it anchored lyrics to non-lyric vocal-stem energy). Restoring the original pack fixed sync on main. Filed as feedback-plugin-stem-splitter#27. That also explains why the symptom survived an app restart, which an orphan never could. So: strike "repro attempts ongoing" — this issue now stands purely on the loader hole, as filed.

Trigger correction: Plugin Manager updates do NOT hot-swap today

The "When it triggers" section overstates the surface. updatePlugin() (static/js/plugin-loader.js ~898) POSTs the update and shows "Updated — restart to apply" — it never calls loadPlugins(), and the Plugin Manager screen only refreshes its own list. After startup completes, nothing re-runs loadPlugins() spontaneously: the SSE stream closes on terminal status and the poll fallback is bounded.

Realistic triggers as of today:

• a version change landing (e.g. dev git pull) while a startup-window refetch is still live (plugin-registered / plugin-error SSE events, or the 60-min poll fallback);
• the DOM-wiped reinstall path (~341–347), which re-evaluates a module at the same version — nominally plugin-runtime-idempotent.v1 territory, but the same double-instance shape;
• any future hot-update flow — which is exactly what fix(plugins): make a module plugin actually re-evaluate on reload (#879) #897's generation-token machinery was built to enable, so the hole gets promoted from landmine to daily bug the moment such a flow ships.

Net: lower present-day severity than the original text implies, but the unsoundness is load-bearing for #897's intended direction.

Precedent for the failure class, in the loader's own comments

loadPlugins() (~319–329) documents a prior incident: a mid-session re-evaluation duplicated the desktop audio_engine's native signal chain ("its init re-ran against the surviving engine chain"). Same class as this report — second instance initializing against a live predecessor — so this isn't hypothetical; it has shipped a bug before. The same block also concedes the core fact: an uninstalled plugin's script is "already-evaluated, un-unloadable".

Sketch of the intended fix

Planned as two PRs:

1. Loader — never hot-swap a live module plugin unless it can be torn down. New window.feedBack.pluginRuntimes.register(id, { destroy }); on a version change for an already-evaluated module, the loader awaits destroy() (bounded) before injecting, and with no hook registered it skips the swap — old version keeps running with its DOM/CSS preserved (needs explicit guards: the alreadyHydrated version-equality check and the versioned style reconciliation would otherwise strip the running instance) — and emits a one-shot plugin:reload-required + console warning, reusing the existing "restart to apply" language. Classic-script upgrades and same-version idempotency stay out of scope.
2. stems — register the hook: existing teardown() (already generation-bumps and aborts in-flight loads) plus closing the audio graph and removing the #audio shims / playSong wrap.