Skip to content

fix(core-internal): support optional {name?} segments in UriTemplate - #2823

Draft
errmakov wants to merge 1 commit into
modelcontextprotocol:mainfrom
errmakov:fix/uri-template-optional-677
Draft

errmakov wants to merge 1 commit into
modelcontextprotocol:mainfrom
errmakov:fix/uri-template-optional-677

Conversation

@errmakov

@errmakov errmakov commented Sep 17, 2026

Copy link
Copy Markdown

Fixes #677.

Problem

A trailing ? on a template variable (scheme://path/{required}/{optional?}) is kept as part of the variable's name, so a ResourceTemplate with an optional segment does not work:

Call Before After
variableNames ['required', 'optional?'] ['required', 'optional']
expand({ required: 'foo', optional: 'bar' }) scheme://path/foo/ scheme://path/foo/bar
expand({ required: 'foo' }) scheme://path/foo/ scheme://path/foo
match('scheme://path/foo/bar') { required: 'foo', 'optional?': 'bar' } { required: 'foo', optional: 'bar' }
match('scheme://path/foo') null { required: 'foo' }

The RFC 6570 forms {/name} and {.name} have the same gap in match(). expand({ required: 'foo' }) on scheme://path/{required}{/optional} gives scheme://path/foo, but match() on that URI returns null.

Change

packages/core-internal/src/shared/uriTemplate.ts:

  • Parsing marks an expression ending in ? as optional and strips the ? from the name.
  • expand() drops the / before an absent {name?}, so the segment and its separator go together.
  • match() puts an optional part, together with the / it owns, inside a non-capturing optional group. {/name} and {.name} parts are also optional, since their expansion is empty when the variable is undefined. Unmatched groups add no key to the result.
  • The four copies of the inline part shape are now one TemplatePart type, so the new optional field is declared once.

Out of scope: comma-separated expressions like {a,b?}. Multi-variable matching is being handled in other open PRs, and this change doesn't touch it.

McpServer only checks that match() returns a truthy value (mcp.ts:192, :555), and completion lookups use variableNames. Both now see optional, so a readCallback or completer for the optional variable works without special-casing the literal optional?.

Tests

Eight cases in a new optional variables block in packages/core-internal/test/shared/uriTemplate.test.ts. Seven fail on main before the change. The eighth, a URI missing a required segment still returning null, guards against over-matching.

  • pnpm --filter @modelcontextprotocol/core-internal test: 1465 passed
  • pnpm --filter @modelcontextprotocol/server test: 509 passed
  • pnpm -r build, pnpm -r typecheck, pnpm sync:snippets --check: pass
  • pnpm -r lint: the only errors are import/no-extraneous-dependencies in test/helpers, test/conformance and test/e2e, in files this PR doesn't touch. They appear only because my local worktree lives under a dot-directory (.claude/worktrees/…), and minimatch's **/test/** doesn't cross dot-directories. core-internal lint and Prettier are clean.

v1.x backport: #2824

🤖 Generated with Claude Code

A trailing ? on a template variable was kept as part of its name, so
expand() dropped the value, match() returned the key 'name?', and a URI
without the optional segment did not match at all. Strip the marker,
omit the segment and its leading / when the value is absent, and let
match() accept the URI either way.

{/name} and {.name} had the same gap in match(): the URI expand()
produces for an undefined variable returned null.

Fixes modelcontextprotocol#677

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 188c6ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/core-internal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 17, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2823

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2823

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2823

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2823

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2823

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2823

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2823

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2823

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2823

commit: 188c6ae

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ResourceTemplate fails when user selects <Empty> for optional parameter

1 participant