FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(core): match multi-variable URI templates like `{a,b}` (#2166) by NishchayMahor · Pull Request #2170 · modelcontextprotocol/typescript-sdk · GitHub

/ typescript-sdk Public

fix(core): match multi-variable URI templates like {a,b} (#2166) - #2170

Open
NishchayMahor wants to merge 1 commit into
modelcontextprotocol:mainfrom
NishchayMahor:fix/uri-template-multi-variable-2166
ConversationCommits (1)ChecksFiles changed
Open

fix(core): match multi-variable URI templates like {a,b} (#2166)#2170
NishchayMahor wants to merge 1 commit into
modelcontextprotocol:mainfrom
NishchayMahor:fix/uri-template-multi-variable-2166

Conversation

NishchayMahor commented May 28, 2026

Copy link
Copy Markdown

Closes #2166.

What's wrong

UriTemplate.expand already supports multi-variable expansions like {userId,format} and produces values joined by commas. UriTemplate.match only emitted a single regex capture per part and assigned the entire captured run to part.name, so everything past the first variable was silently dropped. Any resource registered with a template such as data://users/{userId,format} never routed because match() returned null.

Minimal repro:

const tpl = new UriTemplate('/users/{userId,format}');
tpl.match('/users/42,json');
// before: null
// after:  { userId: '42', format: 'json' }

The fix

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

  • In partToRegExp, when part.names.length > 1 and the operator is not ? / &, emit one capture per name with a literal comma between captures, mirroring the comma join expandPart already does.
  • The first capture still gets the operator's literal prefix where it has one (/, \\., #), so path / label / fragment operators keep matching like they did for the single-variable case.

? and & (form-style query strings) were already handled by the earlier branch and are untouched.

Tests

packages/core/test/shared/uriTemplate.test.ts:

  • match a {userId,format} template against 42,json
  • round-trip expand and match through data://users/{userId,format}
  • multi-variable expression behind the path operator ({/userId,format})
Test Files  1 passed
Tests       3 passed | 552 skipped (555 total in the package)

pnpm --filter @modelcontextprotocol/core lint and typecheck clean. Lefthook pre-push (typecheck, build, lint) all green.

NishchayMahor requested a review from a team as a code owner May 28, 2026 20:04

changeset-bot Bot commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0c20a9b

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

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/core-internal Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/codemod 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 Bot commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: 0c20a9b

NishchayMahor commented May 29, 2026

Copy link
Copy Markdown
Author

Heads up on the red test (22) check: the only failure is test/server/cloudflareWorkers.test.ts > should handle MCP requests, which fails with Network connection lost inside miniflare's local Workers sim. 422 of 423 tests pass, including the three new URI-template regression tests in this PR. Looks unrelated to the multi-variable regex change here — happy to re-run if a maintainer wants a fresh CI cycle.

NishchayMahor force-pushed the fix/uri-template-multi-variable-2166 branch from 17e6435 to e7f131e Compare June 1, 2026 20:45
fix(core): match multi-variable URI templates like {a,b} (modelcont…
0c20a9b 
…extprotocol#2166)

`UriTemplate.expand` already joins multi-name expansions with commas,
but `match` only emitted one regex capture per part and assigned the
whole captured run to the first name. Anything past the first variable
silently never matched and resources registered against templates like
`data://users/{userId,format}` were unreachable.

Emit one capture per name with literal commas between them in
`partToRegExp`, mirroring what `expandPart` produces, so round
tripping through expand and match recovers the original variables.
Path / label / fragment operators get their existing literal prefix
on the first capture; the bare and reserved cases just sit at the
current position.
NishchayMahor force-pushed the fix/uri-template-multi-variable-2166 branch from e7f131e to 0c20a9b Compare August 17, 2026 08:28

NishchayMahor commented Aug 17, 2026

Copy link
Copy Markdown
Author

Rebased onto 75dc7ea6. The conflict was purely the package move in #2354 — shared/uriTemplate.ts went from packages/core to packages/core-internal, and partToRegExp is unchanged there, so the fix reapplied as-is at the new path. The changeset now names core-internal and server (core-internal is private), and the two commits are squashed into one.

The bug is still live on main: for the bare operator partToRegExp pushes a single capture named part.name, and its pattern ([^/,]+) excludes the comma that expand itself writes between multi-name values, so match returns null rather than a partial result.

template match(...) input before after
/users/{userId,format} /users/42,json null { userId: '42', format: 'json' }
{/userId,format} /42,json null { userId: '42', format: 'json' }
{.a,b} .x,y null { a: 'x', b: 'y' }
{#a,b} #x,y { a: '#x,y' } { a: 'x', b: 'y' }
{+a,b} x,y { a: 'x,y' } { a: 'x', b: 'y' }
{?a,b} ?a=x&b=y { a: 'x', b: 'y' } unchanged
/users/{userId} /users/42 { userId: '42' } unchanged

Worth noting the reserved and fragment forms fail differently: (.+) is greedy, so they did match, but dumped the whole run into the first name — {+a,b} gave { a: 'x,y' } and {#a,b} gave { a: '#x,y' }, keeping the # in the value. Those are wrong values rather than a null, which is the quieter of the two failures.

Single-name parts and the ?/& query forms are unaffected — those already emitted one capture per name, and that early return sits above the new branch.

pnpm run check:all and pnpm run build:all are clean; pnpm -r --filter '!@modelcontextprotocol/test-e2e' test gives 4106 passing across 210 files on Node 24. I did not run the conformance, e2e or bun/deno legs locally, so I'm relying on CI for those.

One thing worth flagging rather than leaving for you to find: #2216 and #2218 fix the same issue. #2218 is still against the old packages/core path. #2216 is rebased and current, but scopes its branch to case '':, so it fixes {a,b} and not {/a,b}, {.a,b} or {#a,b} — the {/userId,format} case in the table above is the difference. Happy to close this in favour of #2216 if you'd rather take the smaller diff; I'd just suggest the extra operators get covered somewhere, since expand comma-joins for all of them.

MahathirMohammadShuvo mentioned this pull request Aug 27, 2026
Open
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UriTemplate.match() returns null for multi-variable path expressions like {userId,format}

1 participant

Back | FazBrowse Home | New Git URL