From 8676913c88c6edc6412ed4b374fb35cb20dd8ad8 Mon Sep 17 00:00:00 2001
From: ddonaldson130 <drewdonaldson@gmail.com>
Date: Mon, 6 Jul 2026 22:18:27 -0400
Subject: [PATCH] fix(plugins): stop /plugins/:id from shadowing
 /plugins/registry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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').
---
 .../__tests__/plugin-routes.routes.test.ts    | 36 +++++++++++++++++++
 packages/dashboard/src/routes.ts              | 12 ++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/packages/dashboard/src/__tests__/plugin-routes.routes.test.ts b/packages/dashboard/src/__tests__/plugin-routes.routes.test.ts
index 03b70a5ac0..fc70af31a9 100644
--- a/packages/dashboard/src/__tests__/plugin-routes.routes.test.ts
+++ b/packages/dashboard/src/__tests__/plugin-routes.routes.test.ts
@@ -305,6 +305,42 @@ describe("GET /plugins/:id", () => {
   });
 });
 
+describe("GET /plugins/registry (route-shadowing regression)", () => {
+  let store: TaskStore;
+  let pluginStore: PluginStore;
+
+  beforeEach(() => {
+    pluginStore = createMockPluginStore();
+    store = createMockTaskStore({
+      getPluginStore: vi.fn().mockReturnValue(pluginStore),
+    });
+  });
+
+  function buildApp() {
+    const app = express();
+    app.use(express.json());
+    app.use("/api", createApiRoutes(store, {
+      pluginStore,
+      pluginLoader: createMockPluginLoader(),
+    }));
+    return app;
+  }
+
+  // Regression: the generic "GET /plugins/:id" route is registered before the
+  // plugin sub-router that owns "GET /plugins/registry". Without the pass-through
+  // guard, "/api/plugins/registry" matched ":id" (id === "registry"), called
+  // pluginStore.getPlugin("registry") and failed with 'Plugin "registry" not found'.
+  it("serves the registry listing instead of being shadowed by /plugins/:id", async () => {
+    const res = await GET(buildApp(), "/api/plugins/registry");
+
+    expect(res.status).toBe(200);
+    expect(res.body).toHaveProperty("plugins");
+    expect(Array.isArray(res.body.plugins)).toBe(true);
+    // The ":id" handler must NOT have been consulted for the literal "registry".
+    expect(pluginStore.getPlugin as ReturnType<typeof vi.fn>).not.toHaveBeenCalledWith("registry");
+  });
+});
+
 describe("GET /plugins/:id/settings", () => {
   let store: TaskStore;
   let pluginStore: PluginStore;
diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts
index 442ad265c5..46b5540ee9 100644
--- a/packages/dashboard/src/routes.ts
+++ b/packages/dashboard/src/routes.ts
@@ -3468,7 +3468,17 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
    * Get a single plugin by ID.
    * Query: { projectId?: string }
    */
-  router.get("/plugins/:id", async (req: Request, res: Response) => {
+  router.get("/plugins/:id", async (req: Request, res: Response, next: NextFunction) => {
+    // "registry" is a static sub-route (GET /plugins/registry) owned by the
+    // plugin sub-router mounted further below. Because this generic ":id" route
+    // is registered first, Express would otherwise match it for the literal
+    // path "/plugins/registry" (id === "registry") and throw
+    // 'Plugin "registry" not found', shadowing the real registry handler.
+    // Fall through so the mounted sub-router can serve the registry listing.
+    if (req.params.id === "registry") {
+      next();
+      return;
+    }
     const { store: scopedStore } = await getProjectContext(req);
     const pluginStore = scopedStore.getPluginStore();
     const id = req.params.id as string;