fix(mcp): surface parameterized custom resources as MCP tools#1602
Merged
Conversation
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 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e 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 <noreply@anthropic.com>
…s 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 <noreply@anthropic.com>
…rd 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 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for parameterised routes (e.g., /widget/:id) in the MCP application tools builder, ensuring that parameterised resources are correctly enumerated and registered. The feedback identifies a critical issue in the isSimpleIdRoute helper function, which incorrectly returns true for routes where :id is not the final segment (such as widget/:id/action). This would lead to incorrect behavior in the generated verb handlers. The reviewer has provided a code suggestion to ensure :id is the final segment and recommended adding a corresponding test case to verify this behavior.
Contributor
|
Reviewed; no blockers found. Prior blocker (banned |
…routes 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) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
dawsontoth
approved these changes
Jul 6, 2026
Contributor
|
try rebasing |
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Custom resources on a parameterised path (
static path = '/widget/:id') are stored byResources.setParamRoutein aparamRoutesside-array and never inserted into the base resourcesMap. MCP'sbuildApplicationToolsiterated only the Map, so these resources produced zero MCP tools — even though OpenAPI (which already iteratesparamRoutes) exposed them. This enumeratesparamRoutesfor MCP tool building so the tool surface matches the REST surface.What to look at
The enumeration is deliberately restricted to what the generated verb handlers can actually bind, so we never advertise a tool that would dispatch to the wrong target:
:idroutes →get/update/patch/deletetools. These handlers bindtarget.id = args.idcorrectly.create/searchare suppressed:makeCreateHandlerforcestarget.isCollection = trueand never bindsid, andmakeSearchHandleris collection-scoped — neither matches "the record named by:id"./files/*rest) → no generated verb tools yet (nothing binds the extra segments), but author-definedmcpTools/mcpPromptsstill register, since they carry their own schemas and handler methods.liveResource()(live-registry dispatch, added in Consolidate row-level allowRead enforcement — apply it on all read paths, not just the single-record point-read #1487) now also consultsparamRoutes, so param-route tools don't silently fall back to the registration-time class.Regression coverage in
unitTests/components/mcp/tools/application-paramroutes.test.js(get present; create/search absent on:id; custom tools present + generated verbs absent on wildcard). Full MCP tools suite green (94 passing).Provenance
The core enumeration fix was authored by @dawsontoth as part of the discussion on #1503 (RFC 0001) — extracted here so it can land independently of the RFC. The binding-mode restrictions (single-
:idscoping,create/searchsuppression, custom-tool preservation on wildcards) were added during extraction in response to a Codex cross-model review that converged over four rounds.Cross-model review: Codex — clean (no actionable correctness issues) after the changes above. Gemini/
agytimed out (twice) and did not produce a review.Prepared by an AI agent (Claude, Opus 4.8). Not the original commit author — see per-commit
Co-Authored-Bytrailers.🤖 Generated with Claude Code