Skip to content

fix(core): keep plugin swaps going when a cleanup fails - #63548

Merged
neriousy merged 2 commits into
devfrom
plugin-teardown-isolation
Nov 12, 2026
Merged

neriousy merged 2 commits into
devfrom
plugin-teardown-isolation

Conversation

@neriousy

@neriousy neriousy commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

Issue for this PR

Closes #49982

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

When a plugin is removed or replaced, Plugin.activate closes the old slots in reverse order before loading the new ones. A finalizer that dies aborted that loop. By then the config agent and command plugins, which are last in activation order, were already closed, and the new generation never loaded. Custom agents and commands disappeared until the next reload or a restart, sessions failed with AgentNotFoundError, and plugin.list still reported everything as active. The only trace was failed to reload plugins.

The finalizer that died in the reports is the promise adapter's: it called whatever setup() returned as a cleanup as long as it was truthy, so a plugin returning an object (a v1-style hooks object) threw TypeError: o is not a function on every unload.

Two changes:

  • Plugin.activate closes each old slot on its own and logs failed to clean up plugin with the plugin id when a finalizer fails, so the swap finishes and the later plugins are registered again.
  • The promise adapter only calls a cleanup that is a function.

How did you verify your code works?

  • Reproduced locally with a plugin whose cleanup dies followed by a later builtin that registers a command: before the fix, removing the plugin left the command list empty; after it, the command is registered again and the inventory is correct.
  • test/plugin.test.ts and test/plugin/: 621 pass.

Screenshots / recordings

N/A, not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Added by the opencode bot (AI-generated).

Fixes #57089

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reproduced the bug on the base and it's gone with this PR. I ran the #49982 repro script against opencode serve from source in both checkouts. On the base, removing a plugin whose setup returns {} logged failed to reload plugins … cleanup is not a function and dropped the custom review agent and the gcmd/p1–p3 commands in both locations. With this PR they all stay and nothing is logged. The two new tests fail on the base and pass here; test/plugin.test.ts plus test/plugin/ give 621 passing. I couldn't run the typecheck scripts here because they call bun check.

The per-slot catchCause matches how disabled plugins are already cleaned up, and the typeof check in the adapter is the right boundary fix. No concerns.

Public API is unchanged.

Fixes #57089

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new commit only removes the two regression tests. Since the source changes are the same as at 59e1525e, my earlier verification still applies. With the #49982 repro script from source, the base dropped the custom agents and commands, and this PR keeps them. The plugin tests pass at the new head (627 passing).

The public API is unchanged. No concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant