Repository navigation
Conversation
plugin.list read the registry without waiting for plugin activation, so a cold-location call could return an empty list while the configured plugins were still registering (same race already fixed for mcp.list, skill.list, agent.list and model.list). Await plugin activation before listing, matching model.list, and add the endpoint's ServiceUnavailableError to the protocol contract (regenerating the client). A regression test registers a local plugin that becomes available only after setup completes; without the await, plugin.list returns [] and the test fails.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
This fixes the problem. I ran the #54654 reproduction from source: on the base branch, the first opencode plugin list in each of three new directories printed "No plugins found". With this PR, every call listed the plugin. The new test passes here and fails when copied onto the base, and typecheck is clean in server, protocol and client.
Public API: GET /api/plugin (plugin.list) can now return 503 ServiceUnavailableError if plugins haven't finished loading within 5s. This is an addition, and the generated client's declaredStatuses gain 503. It is backwards compatible apart from the new error status. Existing callers keep the same request and success response, but the first call on a directory that hasn't loaded yet can now take up to 5s.
Concerns:
packages/protocol/openapi.jsonwasn't regenerated.bun run check:generatedinpackages/protocolfails with this PR. Please runbun run generatethere and commit the result.- Commit 8905af5 removed the separate plugin-activation wait endpoint, noting that "Catalog reads remain non-blocking". #54219 later added the same
awaitPluginswait tomodel.list, and this PR follows that newer pattern. Maintainers may want to confirm that blockingplugin.listis the intended direction. plugin.checkandplugin.updatestill read the plugin list without waiting (see inline comment).
| return yield* response(Plugin.Service.use((plugin) => plugin.list())) | ||
| }), | ||
| ) | ||
| .handle("plugin.check", (ctx) => |
There was a problem hiding this comment.
plugin.check (and plugin.update below it) also builds its inventory from plugins.list() without waiting. On a directory the server hasn't loaded yet, plugin check <target> / plugin update <target> can wrongly fail with "Plugin package is not in the current server inventory". The CLI's inventory.ts also calls plugin.list first, so with this PR that path waits, but the server handlers still race when called directly. You don't need to fix this here. If you do, yield* awaitPlugins at the start of both handlers would keep the endpoints consistent.
Keeps these endpoints consistent with plugin.list so a cold location can't fail with a false 'not in the current server inventory'. Declares ServiceUnavailableError on plugin.check and regenerates the client/openapi.
|
Feedback addressed: |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 72-hour window. Feel free to open a new pull request that follows our guidelines. |
Summary
opencode plugin listreads the plugin registry without waiting for plugin activation, so on a cold location the first call can return an empty list ("No plugins found") while the configured plugins are still registering (issue #54654). Same cold-location race already fixed formcp.list,skill.list,agent.list, andmodel.list.Change
packages/server/src/handlers/plugin.ts—plugin.listnowyield* awaitPluginsbefore reading the inventory, matchingmodel.list.packages/protocol/src/groups/plugin.ts— declareServiceUnavailableErroron theplugin.listendpoint (the await can fail if activation times out).packages/client— regenerated so the endpoint contract carries the new error type.Verification
packages/server/test/plugin-endpoint.test.tsregisters a local plugin that only becomes available after its async setup completes. Without the await,plugin.listreturns[]and the test fails; with it, the plugin is listed.1/1pass.packages/protocol,packages/server,packages/clienttypecheck clean.Fixes #54654
Added by the opencode bot (AI-generated).
Related: #51680, #54192, #51282, #53724
Public API: GET /api/plugin (plugin.list) can now return 503 ServiceUnavailableError, and the first call on a directory that hasn't loaded yet can take up to 5s (additive; success response unchanged)