Repository navigation
fix(core): keep plugin swaps going when a cleanup fails - #63548
Conversation
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Issue for this PR
Closes #49982
Type of change
What does this PR do?
When a plugin is removed or replaced,
Plugin.activatecloses 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 withAgentNotFoundError, andplugin.liststill reported everything as active. The only trace wasfailed 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) threwTypeError: o is not a functionon every unload.Two changes:
Plugin.activatecloses each old slot on its own and logsfailed to clean up pluginwith the plugin id when a finalizer fails, so the swap finishes and the later plugins are registered again.How did you verify your code works?
test/plugin.test.tsandtest/plugin/: 621 pass.Screenshots / recordings
N/A, not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Fixes #57089