From bfc19a30687872aaf4b668cb7412528ef07c1388 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 26 Jun 2026 12:21:00 -0400 Subject: [PATCH 1/6] fix(mcp): enumerate parameterised routes when building application tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A custom resource on a parameterised path (e.g. `static path = '/widget/:id'`) is stored by Resources.setParamRoute in the `paramRoutes` side-array and never inserted into the base Map. buildApplicationTools iterated only the Map, so such resources produced ZERO MCP tools — even though OpenAPI (which iterates paramRoutes) exposed them. Enumerate `resources.paramRoutes` too, so the MCP tool surface matches the REST surface. The common single-`:id` case binds via `target.id` in the existing verb handlers; richer multi-segment/named-wildcard binding can layer on later. Also fix `liveResource()` (added by #1487 for live-registry dispatch so a component's row-level `allow*` overrides stay in effect) to check `paramRoutes` as well as the base Map — otherwise every param-route tool silently fell back to its registration-time class reference, since a plain Map lookup always misses for paths that live in the side array. Caught by cross-model review (Codex + Gemini independently flagged it); added a regression test that swaps the live registry entry after registration and asserts the tool dispatches on the replacement. Regression tests: unitTests/components/mcp/tools/application-paramroutes.test.js. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Sonnet 5 --- components/mcp/tools/application.ts | 38 +++++- .../mcp/tools/application-paramroutes.test.js | 109 ++++++++++++++++++ 2 files changed, 141 insertions(+), 6 deletions(-) create mode 100644 unitTests/components/mcp/tools/application-paramroutes.test.js diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index 03e19f17ce..f06b40865c 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -133,7 +133,12 @@ interface ToolAnnotationsLike { openWorldHint?: boolean; } -type ResourcesRegistry = Map; +/** A compiled parameterised route (e.g. `/widget/:id`), stored outside the base Map. */ +interface ParamRouteEntry { + pattern: string; + entry: ResourceRegistryEntry; +} +type ResourcesRegistry = Map & { paramRoutes?: ParamRouteEntry[] }; type RequestTargetCtor = new () => Record & { conditions?: unknown[]; operator?: string; @@ -198,9 +203,15 @@ function loadRequestTarget(): RequestTargetCtor | undefined { * * Falls back to the registration-time class only if the registry lookup fails * (e.g. the entry was removed), so a stale tool still dispatches *something*. + * + * Parameterised routes (e.g. `/widget/:id`) live outside the base Map in + * `paramRoutes`, so a plain `Map.get(path)` always misses for them — checked + * there too, or the live-dispatch guarantee above silently doesn't apply to + * param-route resources. */ function liveResource(path: string, fallback: ResourceClassLike): ResourceClassLike { - const entry = loadResources()?.get(path); + const registry = loadResources(); + const entry = registry?.get(path) ?? registry?.paramRoutes?.find((route) => route.pattern === path)?.entry; return (entry?.Resource as ResourceClassLike | undefined) ?? fallback; } @@ -933,21 +944,23 @@ function buildApplicationTools(resources: ResourcesRegistry): void { const claimedSuffixes = new Set(); let toolsRegistered = 0; let resourcesConsidered = 0; - for (const [path, entry] of resources) { + + const considerEntry = (path: string, entry: ResourceRegistryEntry | undefined): void => { + if (!entry) return; resourcesConsidered++; - if (!shouldEnumerate(entry)) continue; + if (!shouldEnumerate(entry)) return; const ResourceClass = entry.Resource; // @hidden type-level: suppress the Resource from MCP tool listing entirely. // Data remains accessible via direct query/RBAC; only descriptive surfaces drop it. if (ResourceClass?.hidden === true) { harperLogger.trace(`MCP application: '/${path}' suppressed from tool listing (@hidden)`); - continue; + return; } const verbs = detectVerbs(ResourceClass); const hasVerbs = verbs.get || verbs.search || verbs.create || verbs.updatePut || verbs.updatePatch || verbs.delete; const hasCustomTools = Array.isArray(ResourceClass?.mcpTools) && ResourceClass.mcpTools.length > 0; const hasCustomPrompts = Array.isArray(ResourceClass?.mcpPrompts) && ResourceClass.mcpPrompts.length > 0; - if (!hasVerbs && !hasCustomTools && !hasCustomPrompts) continue; + if (!hasVerbs && !hasCustomTools && !hasCustomPrompts) return; const databaseName = ResourceClass?.databaseName; const tableName = ResourceClass?.tableName; const suffix = uniqueSuffix(path, databaseName, claimedSuffixes); @@ -966,7 +979,20 @@ function buildApplicationTools(resources: ResourcesRegistry): void { } toolsRegistered += registerCustomMcpTools(ResourceClass, path); registerCustomMcpPrompts(ResourceClass, path); + }; + + for (const [path, entry] of resources) considerEntry(path, entry); + + // Parameterised routes (e.g. `/widget/:id`) live OUTSIDE the base Map: Resources.setParamRoute + // stores them in `paramRoutes` and returns before the Map insert, so the loop above never sees + // them. Without this, a custom resource declaring `static path = '/widget/:id'` produces ZERO MCP + // tools — even though it appears in the OpenAPI document, which already iterates `paramRoutes`. + // Enumerate them so the tool surface matches the REST surface. The common single-`:id` case binds + // via `target.id` in the verb handlers; richer multi-segment/named-wildcard binding can layer on later. + for (const route of resources.paramRoutes ?? []) { + considerEntry(route.pattern, route.entry); } + harperLogger.info( `MCP application profile: considered ${resourcesConsidered} resource(s), registered ${toolsRegistered} tool(s)` ); diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js new file mode 100644 index 0000000000..1ce43e964c --- /dev/null +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -0,0 +1,109 @@ +// Regression test for param-route enumeration: a custom resource declaring a +// parameterised path (e.g. `static path = '/widget/:id'`) is stored by +// Resources.setParamRoute in the `paramRoutes` side-array and never inserted +// into the base Map. MCP's buildApplicationTools previously iterated only the +// Map, so such resources produced ZERO tools — while OpenAPI (which iterates +// paramRoutes) did expose them. buildApplicationTools now enumerates paramRoutes +// too, so the tool surface matches the REST surface. + +const assert = require('node:assert/strict'); +const { + registerApplicationTools, + _setResourcesForTest, + _setRequestTargetForTest, + _resetApplicationToolsRegisteredForTest, +} = require('#src/components/mcp/tools/application'); +const { listTools, getTool, _resetRegistryForTest } = require('#src/components/mcp/toolRegistry'); +const { _resetPromptRegistryForTest } = require('#src/components/mcp/promptRegistry'); + +const SUPER = { username: 'admin', role: { permission: { super_user: true } } }; + +class FakeRequestTarget {} + +// A custom (non-table) Resource registered at a parameterised path. +function makeParamResource() { + class Widget {} + Widget.prototype.get = function () {}; // read verb present + Widget.prototype.post = function () {}; // create verb present + Widget.get = async (target) => ({ id: target.id, name: 'sample' }); + Widget.post = async (_target, data) => ({ created: true, ...data }); + return Widget; +} + +// A registry (Map) with a `paramRoutes` side-array, mirroring resources/Resources.ts. +function makeRegistryWithParamRoute(pattern, Resource, exportTypes) { + const m = new Map(); // intentionally no static Map entries + m.paramRoutes = [{ pattern, entry: { path: pattern, Resource, exportTypes, hasSubPaths: false, relativeURL: '' } }]; + return m; +} + +function listSuperToolNames() { + const { tools } = listTools({ user: SUPER, profile: 'application', sessionId: 's', limit: 200 }); + return tools.map((t) => t.name); +} + +describe('mcp/tools/application — parameterised routes', () => { + beforeEach(() => { + _resetRegistryForTest(); + _resetPromptRegistryForTest(); + _setRequestTargetForTest(FakeRequestTarget); + }); + + afterEach(() => { + _resetRegistryForTest(); + _resetPromptRegistryForTest(); + _setResourcesForTest(undefined); + _setRequestTargetForTest(undefined); + _resetApplicationToolsRegisteredForTest(); + }); + + it('registers tools for a custom resource on a parameterised path (/widget/:id)', () => { + _setResourcesForTest(makeRegistryWithParamRoute('widget/:id', makeParamResource())); + registerApplicationTools(); + const names = listSuperToolNames(); + assert.ok(names.length > 0, `expected tools for the parameterised resource, got none`); + assert.ok( + names.some((n) => n.startsWith('get_')), + `expected a get_* tool, got ${JSON.stringify(names)}` + ); + assert.ok( + names.some((n) => n.startsWith('create_')), + `expected a create_* tool, got ${JSON.stringify(names)}` + ); + }); + + it('respects mcp:false on a parameterised route', () => { + _setResourcesForTest(makeRegistryWithParamRoute('secret/:id', makeParamResource(), { mcp: false })); + registerApplicationTools(); + assert.equal(listSuperToolNames().length, 0, 'mcp:false should suppress the param-route resource'); + }); + + it('registers nothing when there are no resources at all', () => { + const empty = new Map(); + empty.paramRoutes = []; + _setResourcesForTest(empty); + registerApplicationTools(); + assert.equal(listSuperToolNames().length, 0); + }); + + it('dispatches on the live paramRoutes entry, not the class captured at registration', async () => { + // Regression guard: buildApplicationTools binds verb handlers to `liveResource(path, ...)`, + // which re-reads the registry at call time. For a param route, that lookup must consult + // `paramRoutes` (not just the base Map) or it silently falls back to the stale captured + // class — the same live-dispatch guarantee static routes rely on for per-record RBAC. + const registry = makeRegistryWithParamRoute('widget/:id', makeParamResource()); + _setResourcesForTest(registry); + registerApplicationTools(); + + class Replacement {} + Replacement.get = async (target) => ({ id: target.id, name: 'from-replacement' }); + registry.paramRoutes[0].entry.Resource = Replacement; + + const result = await getTool('get_widget_id').handler({ id: '1' }, { user: SUPER }); + assert.equal( + result.structuredContent?.name, + 'from-replacement', + 'expected the tool to dispatch on the live registry entry after it was swapped' + ); + }); +}); From 624c3399806f8eeb9306258c368d72049a19e72e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Sat, 4 Jul 2026 14:33:11 -0600 Subject: [PATCH 2/6] fix(mcp): restrict param-route enumeration to the single-:id shape the handlers bind The verb handlers only bind `target.id = args.id`. A route with a differently-named param (:widgetId), more than one param, or a *wildcard segment would advertise an id-only tool that silently drops the other segment(s) the Resource actually needs. Gate enumeration on isSimpleIdRoute so those shapes are skipped until richer path-param binding lands, instead of shipping a broken tool. Co-Authored-By: Claude Sonnet 5 --- components/mcp/tools/application.ts | 30 +++++++++++++++++-- .../mcp/tools/application-paramroutes.test.js | 15 ++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index f06b40865c..b3c8acdb8c 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -133,6 +133,24 @@ interface ToolAnnotationsLike { openWorldHint?: boolean; } +/** + * True when `pattern`'s only dynamic segment is a single `:id` param (e.g. `widget/:id`). + * That is the ONLY shape the verb handlers below bind — `target.id = args.id` — so a + * differently-named param (`:widgetId`), more than one param, or a `*wildcard` segment + * would advertise an `id` input the handler never actually threads onto the real segment(s). + */ +function isSimpleIdRoute(pattern: string): boolean { + let paramCount = 0; + for (const segment of pattern.split('/')) { + if (segment.startsWith('*')) return false; + if (segment.startsWith(':')) { + if (segment !== ':id') return false; + paramCount++; + } + } + return paramCount === 1; +} + /** A compiled parameterised route (e.g. `/widget/:id`), stored outside the base Map. */ interface ParamRouteEntry { pattern: string; @@ -987,9 +1005,17 @@ function buildApplicationTools(resources: ResourcesRegistry): void { // stores them in `paramRoutes` and returns before the Map insert, so the loop above never sees // them. Without this, a custom resource declaring `static path = '/widget/:id'` produces ZERO MCP // tools — even though it appears in the OpenAPI document, which already iterates `paramRoutes`. - // Enumerate them so the tool surface matches the REST surface. The common single-`:id` case binds - // via `target.id` in the verb handlers; richer multi-segment/named-wildcard binding can layer on later. + // Enumerate them so the tool surface matches the REST surface — but only the single-`:id` shape + // the verb handlers actually bind (see isSimpleIdRoute); richer multi-segment/named-wildcard + // binding is excluded here until that binding lands, rather than advertising a tool that silently + // drops the other segment(s). for (const route of resources.paramRoutes ?? []) { + if (!isSimpleIdRoute(route.pattern)) { + harperLogger.trace( + `MCP application: '/${route.pattern}' skipped — richer param binding (multi-segment/named-wildcard) not yet supported` + ); + continue; + } considerEntry(route.pattern, route.entry); } diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js index 1ce43e964c..486614bb75 100644 --- a/unitTests/components/mcp/tools/application-paramroutes.test.js +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -106,4 +106,19 @@ describe('mcp/tools/application — parameterised routes', () => { 'expected the tool to dispatch on the live registry entry after it was swapped' ); }); + + // The verb handlers only bind `target.id = args.id`. A route with a differently-named + // param, more than one param, or a `*wildcard` segment would advertise an `id`-only tool + // that silently drops the other segment(s) — so those shapes must be skipped, not exposed. + for (const pattern of ['widget/:id/action/:action', 'widget/:widgetId', 'files/*rest', 'files/*']) { + it(`skips a parameterised route the verb handlers cannot fully bind (${pattern})`, () => { + _setResourcesForTest(makeRegistryWithParamRoute(pattern, makeParamResource())); + registerApplicationTools(); + assert.equal( + listSuperToolNames().length, + 0, + `expected no tools for an unsupported param shape, got ${JSON.stringify(listSuperToolNames())}` + ); + }); + } }); From c8cf4c1b01ea5cd160de5fcf2b02aa774f30a34e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Sat, 4 Jul 2026 14:40:44 -0600 Subject: [PATCH 3/6] fix(mcp): don't advertise create_*/search_* for param routes; handlers can't bind the record id makeCreateHandler forces target.isCollection = true and never sets target.id; makeSearchHandler is collection-scoped by design. Neither matches a :id-scoped record, so an MCP client calling create_*/search_* on a param route would silently run against the wrong target (a fresh collection insert, or a collection scan) instead of the selected record. Restrict param-route enumeration to get/update/patch/delete, which all correctly bind target.id = args.id. Co-Authored-By: Claude Sonnet 5 --- components/mcp/tools/application.ts | 13 +++++++++++-- .../mcp/tools/application-paramroutes.test.js | 18 ++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index b3c8acdb8c..43d23405ef 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -963,7 +963,7 @@ function buildApplicationTools(resources: ResourcesRegistry): void { let toolsRegistered = 0; let resourcesConsidered = 0; - const considerEntry = (path: string, entry: ResourceRegistryEntry | undefined): void => { + const considerEntry = (path: string, entry: ResourceRegistryEntry | undefined, isParamRoute = false): void => { if (!entry) return; resourcesConsidered++; if (!shouldEnumerate(entry)) return; @@ -975,6 +975,15 @@ function buildApplicationTools(resources: ResourcesRegistry): void { return; } const verbs = detectVerbs(ResourceClass); + if (isParamRoute) { + // A `:id`-scoped route is record-scoped, but `makeCreateHandler` forces + // `target.isCollection = true` (never binds `target.id`) and `makeSearchHandler` + // is collection-scoped by design — neither matches "the record named by :id", so + // don't advertise them here. get/update/patch/delete all correctly bind + // `target.id = args.id` and are unaffected. + verbs.search = false; + verbs.create = false; + } const hasVerbs = verbs.get || verbs.search || verbs.create || verbs.updatePut || verbs.updatePatch || verbs.delete; const hasCustomTools = Array.isArray(ResourceClass?.mcpTools) && ResourceClass.mcpTools.length > 0; const hasCustomPrompts = Array.isArray(ResourceClass?.mcpPrompts) && ResourceClass.mcpPrompts.length > 0; @@ -1016,7 +1025,7 @@ function buildApplicationTools(resources: ResourcesRegistry): void { ); continue; } - considerEntry(route.pattern, route.entry); + considerEntry(route.pattern, route.entry, true); } harperLogger.info( diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js index 486614bb75..21fb55db4c 100644 --- a/unitTests/components/mcp/tools/application-paramroutes.test.js +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -66,9 +66,23 @@ describe('mcp/tools/application — parameterised routes', () => { names.some((n) => n.startsWith('get_')), `expected a get_* tool, got ${JSON.stringify(names)}` ); + }); + + it('does not register create_*/search_* for a param route (their handlers cannot bind the record id)', () => { + // makeCreateHandler forces `target.isCollection = true` and never sets `target.id`; + // makeSearchHandler is collection-scoped. Neither matches "the record named by :id", + // so a param-route resource with `post`/`search` should still get its `get_*` tool but + // not `create_*`/`search_*`. + _setResourcesForTest(makeRegistryWithParamRoute('widget/:id', makeParamResource())); + registerApplicationTools(); + const names = listSuperToolNames(); + assert.ok( + names.some((n) => n.startsWith('get_')), + `expected a get_* tool, got ${JSON.stringify(names)}` + ); assert.ok( - names.some((n) => n.startsWith('create_')), - `expected a create_* tool, got ${JSON.stringify(names)}` + !names.some((n) => n.startsWith('create_') || n.startsWith('search_')), + `expected no create_*/search_* tool for a param route, got ${JSON.stringify(names)}` ); }); From 3a595a8f9ae496678ba1919f6402e04c43b35773 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Sat, 4 Jul 2026 16:08:00 -0600 Subject: [PATCH 4/6] fix(mcp): register custom mcpTools/mcpPrompts on multi-segment/wildcard param routes The simple-:id gate suppressed considerEntry entirely for richer route shapes, hiding author-defined mcpTools/mcpPrompts that carry their own schemas and handler methods and don't depend on generated verb binding. Suppress only the generated verbs ('none' binding mode); custom surfaces register on every route shape. Co-Authored-By: Claude Fable 5 --- components/mcp/tools/application.ts | 41 +++++++++++-------- .../mcp/tools/application-paramroutes.test.js | 26 ++++++++++++ 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index 43d23405ef..6fd467f6af 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -963,7 +963,19 @@ function buildApplicationTools(resources: ResourcesRegistry): void { let toolsRegistered = 0; let resourcesConsidered = 0; - const considerEntry = (path: string, entry: ResourceRegistryEntry | undefined, isParamRoute = false): void => { + // How much of a param route the GENERATED verb handlers can bind: + // 'id' — single-`:id` routes: get/update/patch/delete bind `target.id = args.id`, + // but makeCreateHandler forces `target.isCollection = true` (never binds id) + // and makeSearchHandler is collection-scoped, so create/search are dropped. + // 'none' — multi-segment/named-wildcard routes: no generated handler can bind the + // segments yet, so ALL generated verbs are dropped. Author-defined + // mcpTools/mcpPrompts carry their own schemas and handler methods, so they + // register regardless of binding mode. + const considerEntry = ( + path: string, + entry: ResourceRegistryEntry | undefined, + paramBinding?: 'id' | 'none' + ): void => { if (!entry) return; resourcesConsidered++; if (!shouldEnumerate(entry)) return; @@ -975,14 +987,14 @@ function buildApplicationTools(resources: ResourcesRegistry): void { return; } const verbs = detectVerbs(ResourceClass); - if (isParamRoute) { - // A `:id`-scoped route is record-scoped, but `makeCreateHandler` forces - // `target.isCollection = true` (never binds `target.id`) and `makeSearchHandler` - // is collection-scoped by design — neither matches "the record named by :id", so - // don't advertise them here. get/update/patch/delete all correctly bind - // `target.id = args.id` and are unaffected. + if (paramBinding === 'id') { verbs.search = false; verbs.create = false; + } else if (paramBinding === 'none') { + harperLogger.trace( + `MCP application: '/${path}' generated verb tools skipped — multi-segment/named-wildcard binding not yet supported` + ); + verbs.get = verbs.search = verbs.create = verbs.updatePut = verbs.updatePatch = verbs.delete = false; } const hasVerbs = verbs.get || verbs.search || verbs.create || verbs.updatePut || verbs.updatePatch || verbs.delete; const hasCustomTools = Array.isArray(ResourceClass?.mcpTools) && ResourceClass.mcpTools.length > 0; @@ -1014,18 +1026,11 @@ function buildApplicationTools(resources: ResourcesRegistry): void { // stores them in `paramRoutes` and returns before the Map insert, so the loop above never sees // them. Without this, a custom resource declaring `static path = '/widget/:id'` produces ZERO MCP // tools — even though it appears in the OpenAPI document, which already iterates `paramRoutes`. - // Enumerate them so the tool surface matches the REST surface — but only the single-`:id` shape - // the verb handlers actually bind (see isSimpleIdRoute); richer multi-segment/named-wildcard - // binding is excluded here until that binding lands, rather than advertising a tool that silently - // drops the other segment(s). + // Enumerate them so the tool surface matches the REST surface; the binding mode restricts the + // GENERATED verb tools to what their handlers actually bind (see considerEntry), while custom + // mcpTools/mcpPrompts register on every route shape. for (const route of resources.paramRoutes ?? []) { - if (!isSimpleIdRoute(route.pattern)) { - harperLogger.trace( - `MCP application: '/${route.pattern}' skipped — richer param binding (multi-segment/named-wildcard) not yet supported` - ); - continue; - } - considerEntry(route.pattern, route.entry, true); + considerEntry(route.pattern, route.entry, isSimpleIdRoute(route.pattern) ? 'id' : 'none'); } harperLogger.info( diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js index 21fb55db4c..3ad616614d 100644 --- a/unitTests/components/mcp/tools/application-paramroutes.test.js +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -68,6 +68,32 @@ describe('mcp/tools/application — parameterised routes', () => { ); }); + it('keeps custom mcpTools on multi-segment/wildcard routes while suppressing generated verbs', () => { + // Generated verb handlers cannot bind multi-segment/wildcard params yet, but + // author-defined mcpTools carry their own schemas and handler methods — a + // `files/*rest` resource with explicit tools must not become invisible. + class Files {} + Files.prototype.get = function () {}; + Files.get = async () => ({}); + Files.prototype.listVersions = function () {}; + Files.mcpTools = [ + { + name: 'list_file_versions', + method: 'listVersions', + description: 'List versions of a file', + inputSchema: { type: 'object', properties: { path: { type: 'string' } }, required: ['path'] }, + }, + ]; + _setResourcesForTest(makeRegistryWithParamRoute('files/*rest', Files)); + registerApplicationTools(); + const names = listSuperToolNames(); + assert.ok(names.includes('list_file_versions'), `expected the custom tool, got ${JSON.stringify(names)}`); + assert.ok( + !names.some((n) => n.startsWith('get_')), + `expected no generated verb tools on a wildcard route, got ${JSON.stringify(names)}` + ); + }); + it('does not register create_*/search_* for a param route (their handlers cannot bind the record id)', () => { // makeCreateHandler forces `target.isCollection = true` and never sets `target.id`; // makeSearchHandler is collection-scoped. Neither matches "the record named by :id", From d74c6a2107c2baad6955701a890bf504c12d1f3c Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 6 Jul 2026 10:44:32 -0600 Subject: [PATCH 5/6] fix(mcp): require :id to be the trailing segment for single-id param routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isSimpleIdRoute treated `widget/:id/action` as a bindable single-:id route (one :id, no wildcard), but the verb handlers bind `target.id = args.id` treating :id as the record itself — for a sub-resource route that silently drops the trailing `/action` segment and dispatches to the wrong target. Require :id to be the final segment so such routes fall through to the custom-tool-only path instead of advertising bogus get/update/patch/delete. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/mcp/tools/application.ts | 14 +++++++++----- .../mcp/tools/application-paramroutes.test.js | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index 6fd467f6af..388ccd309e 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -134,14 +134,18 @@ interface ToolAnnotationsLike { } /** - * True when `pattern`'s only dynamic segment is a single `:id` param (e.g. `widget/:id`). - * That is the ONLY shape the verb handlers below bind — `target.id = args.id` — so a - * differently-named param (`:widgetId`), more than one param, or a `*wildcard` segment - * would advertise an `id` input the handler never actually threads onto the real segment(s). + * True when `pattern`'s only dynamic segment is a single trailing `:id` param (e.g. `widget/:id`). + * That is the ONLY shape the verb handlers below bind — `target.id = args.id`, treating `:id` as + * the record itself — so a differently-named param (`:widgetId`), more than one param, a `*wildcard` + * segment, or an `:id` that is NOT the final segment (`widget/:id/action`, where the route names a + * sub-resource, not the widget) would advertise an `id` input the handler never actually threads + * onto the real segment(s). */ function isSimpleIdRoute(pattern: string): boolean { + const segments = pattern.split('/'); + if (segments[segments.length - 1] !== ':id') return false; let paramCount = 0; - for (const segment of pattern.split('/')) { + for (const segment of segments) { if (segment.startsWith('*')) return false; if (segment.startsWith(':')) { if (segment !== ':id') return false; diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js index 3ad616614d..9e84b1606b 100644 --- a/unitTests/components/mcp/tools/application-paramroutes.test.js +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -147,10 +147,18 @@ describe('mcp/tools/application — parameterised routes', () => { ); }); - // The verb handlers only bind `target.id = args.id`. A route with a differently-named - // param, more than one param, or a `*wildcard` segment would advertise an `id`-only tool - // that silently drops the other segment(s) — so those shapes must be skipped, not exposed. - for (const pattern of ['widget/:id/action/:action', 'widget/:widgetId', 'files/*rest', 'files/*']) { + // The verb handlers only bind `target.id = args.id`, treating `:id` as the record itself. + // A route with a differently-named param, more than one param, a `*wildcard` segment, or an + // `:id` that is not the final segment (`widget/:id/action` names a sub-resource, not the + // widget) would advertise an `id`-only tool that silently drops the other segment(s) — so + // those shapes must be skipped, not exposed. + for (const pattern of [ + 'widget/:id/action/:action', + 'widget/:id/action', + 'widget/:widgetId', + 'files/*rest', + 'files/*', + ]) { it(`skips a parameterised route the verb handlers cannot fully bind (${pattern})`, () => { _setResourcesForTest(makeRegistryWithParamRoute(pattern, makeParamResource())); registerApplicationTools(); From 85c7ee8bfc4df1e353b4f115d8a25ad9c5bfade9 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 6 Jul 2026 15:02:08 -0600 Subject: [PATCH 6/6] Update unitTests/components/mcp/tools/application-paramroutes.test.js Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- unitTests/components/mcp/tools/application-paramroutes.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unitTests/components/mcp/tools/application-paramroutes.test.js b/unitTests/components/mcp/tools/application-paramroutes.test.js index 9e84b1606b..f9638e9565 100644 --- a/unitTests/components/mcp/tools/application-paramroutes.test.js +++ b/unitTests/components/mcp/tools/application-paramroutes.test.js @@ -6,7 +6,7 @@ // paramRoutes) did expose them. buildApplicationTools now enumerates paramRoutes // too, so the tool surface matches the REST surface. -const assert = require('node:assert/strict'); +const assert = require('node:assert'); const { registerApplicationTools, _setResourcesForTest,