Repository navigation
Conversation
A plugin's setup() is JavaScript at runtime, so it may return anything. The teardown that runs when the previous plugin generation is released checked the returned value for truthiness and then called it, so a plugin whose setup() returned an object aborted the whole reload with "TypeError: cleanup is not a function". Only a real function counts as a cleanup callback now. Regression test in test/plugin/supervisor-reload.test.ts drives a reload against a plugin whose setup() returns an object and asserts the new generation replaces the old one. Red/green checked: the previous truthiness check fails that case, the fix passes it. Verification: `bun test test/plugin/supervisor-reload.test.ts` is 9/9; `bun test test/plugin/` is 396/397 with the single failure being a pre-existing timeout in module.test.ts that reproduces on the base commit; `bun typecheck` clean in packages/core and packages/plugin.
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I ran the #57719 reproduction from source on both checkouts. On the base, with a plugin whose setup() returns an object, opencode reload fails with TypeError: cleanup is not a function (adapter.ts:621). With this PR every reload prints Configuration reloaded. The new test in supervisor-reload.test.ts fails on the base and passes here. The rest of that file shows one failure, which also fails on the base. Type checks are clean in packages/plugin and packages/core.
Public API: unchanged. A non-function value returned from setup() is now ignored instead of called as a cleanup function, so existing plugins are unaffected.
No concerns. The fix is in the right place (the adapter, where the cleanup is called) and complements #57096, which handles a cleanup that throws.
Related: #57089
|
Closing for now to free an open-PR slot — the branch stays put and I'll reopen if the teardown guard still needs a home after the current cleanup cycle. |
Issue for this PR
Closes #57719
Type of change
What does this PR do?
A plugin's
setup()runs as JavaScript, so it may return anything. The teardown that runs when the previous plugin generation is released only checked the returned value for truthiness and then called it:A plugin whose
setup()returns an object therefore hitscleanup()and the reload dies withTypeError: o is not a function, which surfaces asopencode reloadfailing against the background service.Only a real function counts as a cleanup callback now, so a plugin that returns something else is simply left without a teardown hook.
This is a different layer from #57096: that PR isolates a cleanup that throws so one plugin cannot abort the whole activation, while this one avoids calling a value that was never a cleanup function in the first place. The two are complementary — a plugin can still return a well-formed cleanup that throws, and #57096 covers that.
How did you verify your code works?
packages/core/test/plugin/supervisor-reload.test.tsloads a local plugin whosesetup()returns an object, then triggers a reload and asserts the new generation replaces the old one instead of the reload dying. Red/green checked: the previous truthiness check fails that case, the fix passes it.bun test test/plugin/supervisor-reload.test.tsis 9/9.bun test test/plugin/is 396/397, with the single failure being a pre-existing timeout inmodule.test.tsthat reproduces on the base commit.bun typecheckclean inpackages/coreandpackages/plugin.Screenshots / recordings
N/A, not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #57089