Skip to content

[v1.x] fix: support optional {name?} segments in UriTemplate - #2824

Draft
errmakov wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
errmakov:fix/uri-template-optional-677-v1.x
Draft

errmakov wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
errmakov:fix/uri-template-optional-677-v1.x

Conversation

@errmakov

Copy link
Copy Markdown

Fixes #677 on v1.x. Backport of #2823.

Problem

A trailing ? on a template variable (scheme://path/{required}/{optional?}) is kept as part of the variable's name. variableNames reports optional?, expand({ required: 'foo', optional: 'bar' }) gives scheme://path/foo/, match('scheme://path/foo/bar') returns the key 'optional?', and match('scheme://path/foo') returns null. So a ResourceTemplate with an optional segment can't be read with or without that segment.

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

Change

Same change as #2823, in 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?}.
  • match() wraps optional parts, and {/name} and {.name} parts, in a non-capturing optional group together with the / an optional part owns. Unmatched groups add no key.

The only conflict with the main patch was the result loop, which keeps v1.x's indexed for loop.

Tests

Eight cases in a new optional variables block in test/shared/uriTemplate.test.ts, identical to #2823.

  • npm test: 1655 passed
  • npm run typecheck: pass
  • eslint and prettier --check on the changed files: clean

🤖 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.

v1.x backport of the main-branch fix. 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: 1e16063

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

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

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2824

commit: 1e16063

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.

1 participant