fix(plugins): Browse registry fails with 'Plugin "registry" not found' (route shadowed by /plugins/:id)#1936
Conversation
The Plugins settings 'Browse registry' feature failed with
'Failed to load registry: Plugin "registry" not found'.
Root cause: in createApiRoutes (routes.ts), the generic project-scoped
'GET /plugins/:id' route is registered BEFORE the plugin sub-router
(createPluginRouter) that owns 'GET /plugins/registry' is mounted. Express
matches in registration order, so a request to /api/plugins/registry matched
':id' with id='registry', called pluginStore.getPlugin('registry') and threw
'Plugin "registry" not found' — the real registry handler was never reached.
Reordering the mount is unsafe: the inline ':id' route is project-scoped via
getProjectContext, while the sub-router uses the global plugin store, so
moving it would change scoping semantics. Instead, let the reserved static
path fall through: when id === 'registry', call next() so the mounted
sub-router serves the registry listing (which already handles projectId).
Adds a regression test asserting GET /api/plugins/registry returns 200 with a
plugins array and never calls getPlugin('registry').
📝 WalkthroughWalkthroughThe GET /plugins/:id route handler in routes.ts now checks if the id parameter equals "registry" and calls next() to defer handling to the dedicated registry sub-route. A new regression test verifies this route-shadowing fix. ChangesRoute-shadowing fix
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PluginsIdRoute as GET /plugins/:id
participant PluginsRegistryRoute as GET /plugins/registry
Client->>PluginsIdRoute: GET /api/plugins/registry
PluginsIdRoute->>PluginsIdRoute: id === "registry"?
PluginsIdRoute->>PluginsRegistryRoute: next()
PluginsRegistryRoute->>Client: plugins array response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes the dashboard plugin registry route shadowing. The main changes are:
Confidence Score: 4/5The changed route looks mergeable after checking the optional plugin-router path.
packages/dashboard/src/routes.ts Important Files Changed
Reviews (1): Last reviewed commit: "fix(plugins): stop /plugins/:id from sha..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (req.params.id === "registry") { | ||
| next(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When createApiRoutes is used without both pluginStore and pluginLoader, the plugin sub-router is not mounted, but this new branch still calls next() for /plugins/registry. That changes the old behavior from a scoped plugin lookup into a fall-through 404/error path, so any runtime or test harness that does not install the optional plugin router loses the ability to resolve a real plugin whose ID is registry.
Context Used: AGENTS.md (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dashboard/src/routes.ts`:
- Around line 3478-3481: The special-case in createPluginRouter() for
req.params.id === "registry" currently falls through to next(), which makes
/api/plugins/registry unreachable when only pluginStore is configured. Update
the routing so the registry endpoint is mounted independently of pluginLoader,
or add an explicit fallback handler in the pluginStore-only path; use the
createPluginRouter() and req.params.id check to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c4dbed7c-dea8-43d0-a6f2-84de98239570
📒 Files selected for processing (2)
packages/dashboard/src/__tests__/plugin-routes.routes.test.tspackages/dashboard/src/routes.ts
| if (req.params.id === "registry") { | ||
| next(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether pluginStore can be configured without pluginLoader anywhere in the codebase.
rg -nP '\bpluginStore\s*:' --type=ts -C2 packages | rg -v pluginLoaderRepository: Runfusion/Fusion
Length of output: 16644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the API route setup and plugin route mounting conditions.
rg -n -C 3 "pluginLoader|pluginStore|GET /plugins/registry|createApiRoutes|createPluginRouter" packages/dashboard/src/routes.ts packages/dashboard/src -g '!**/dist/**' -g '!**/build/**'
# Find the options type / callers for createApiRoutes.
rg -n -C 3 "createApiRoutes\(" packages/dashboard/src packages -g '!**/dist/**' -g '!**/build/**'Repository: Runfusion/Fusion
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant route setup and registry handler.
sed -n '3440,3515p' packages/dashboard/src/routes.ts
echo '---'
sed -n '4828,4885p' packages/dashboard/src/routes.ts
echo '---'
sed -n '1038,1105p' packages/dashboard/src/routes.ts
# Show the test that covers the "plugin loader is not available" case.
sed -n '580,620p' packages/dashboard/src/__tests__/plugin-routes.routes.test.tsRepository: Runfusion/Fusion
Length of output: 8160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect server options and how pluginLoader/pluginStore are wired in the real app.
rg -n -C 3 "interface ServerOptions|type ServerOptions|pluginLoader\\?|pluginStore\\?|createApiRoutes\\(store, \\{" packages/dashboard/src/routes.ts packages/dashboard/src/server.ts packages/dashboard/src -g '!**/dist/**' -g '!**/build/**'
# Focus on the registry-related route tests and any no-pluginLoader cases.
rg -n -C 4 "plugins/registry|No pluginLoader|pluginLoader is not available|pluginStore," packages/dashboard/src/__tests__ packages/dashboard/src/routes/__tests__ -g '!**/dist/**' -g '!**/build/**'Repository: Runfusion/Fusion
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the plugin sub-router and the registry route it owns.
sed -n '320,430p' packages/dashboard/src/plugin-routes.ts
echo '---'
sed -n '300,360p' packages/dashboard/src/__tests__/plugin-routes.routes.test.ts
echo '---'
sed -n '1220,1260p' packages/dashboard/src/__tests__/plugin-routes.test.tsRepository: Runfusion/Fusion
Length of output: 7584
Keep /plugins/registry reachable without pluginLoader. createPluginRouter() is mounted only when both pluginStore and pluginLoader are set, so this next() turns /api/plugins/registry into a plain Express 404 in pluginStore-only setups. Mount the registry route separately or add an explicit fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dashboard/src/routes.ts` around lines 3478 - 3481, The special-case
in createPluginRouter() for req.params.id === "registry" currently falls through
to next(), which makes /api/plugins/registry unreachable when only pluginStore
is configured. Update the routing so the registry endpoint is mounted
independently of pluginLoader, or add an explicit fallback handler in the
pluginStore-only path; use the createPluginRouter() and req.params.id check to
locate the fix.
|
Heads-up on scope: this fixes a real latent route-ordering bug ( |
|
Tip For best results, initiate chat on the files or code changes.
Keeping this PR scoped to the isolated ordering fix and letting ✏️ Learnings added
|
2 participants
Problem
In Project Settings → Plugins, the Browse registry panel fails to load with:
The registry listing never appears, so one-click plugin discovery/install is unusable.
Root cause
In
createApiRoutes(packages/dashboard/src/routes.ts), the generic project-scoped routeGET /plugins/:idis registered before the plugin sub-router (createPluginRouter) — which ownsGET /plugins/registry— is mounted (router.use("/plugins", createPluginRouter(...))).Express matches routes in registration order, so a request to
/api/plugins/registrymatches:idwithid === "registry", callspluginStore.getPlugin("registry"), and throwsPlugin "registry" not found. The real registry handler (which builds the curated registry manifest) is never reached.Why not just reorder the mount?
The two handlers are not equivalent:
GET /plugins/:idis project-scoped viagetProjectContext(req),Mounting the sub-router first would shadow the project-scoped routes and change scoping semantics for other endpoints. So reordering is the wrong fix.
Fix
Let the reserved static path fall through. In the inline
GET /plugins/:idhandler, whenid === "registry", callnext()so the later-mounted sub-router serves the registry listing (it already handles theprojectIdquery param). Minimal and targeted —"registry"is the only static GET path under/pluginsthat collides with the single-segment:idpattern.Tests
Adds a regression test in
plugin-routes.routes.test.ts(using the existingcreateApiRoutesharness that reproduces the real mount order) assertingGET /api/plugins/registry:pluginsarray, andpluginStore.getPlugin("registry")(i.e. is no longer shadowed by:id).Summary by CodeRabbit
Bug Fixes
/plugins/registryendpoint so it responds correctly instead of being treated like a plugin ID.Tests
/plugins/registryroute to verify the correct response and prevent the generic plugin lookup from intercepting it.