[lhy-dcf]

What breaks

UriTemplate disagrees with itself for query expressions. expand treats a query variable as optional — it drops the pair when the variable is undefined:

new UriTemplate('/search{?q,page}').expand({ q: 'test' }); // '/search?q=test'

match does not. partToRegExp builds one mandatory group per name:

pattern: prefix + this.escapeRegExp(name) + '=([^&]+)'

so the matcher demands every pair in the template:

const t = new UriTemplate('/search{?q,page}');
t.match(t.expand({ q: 'test' }));        // null   <- a URI it just produced
t.match('/search?q=test');               // null
t.match('/search?q=test&page=1');        // { q: 'test', page: '1' }

A client that registers a resource template with optional query variables ({?cursor,limit} is the common shape: the server expands it with whatever it needs, and the client then asks whether a resource URI belongs to the template) can never match a partially expanded URI. The second failure mode is the same bug seen from the other side: expand({ q: '' }) emits /search?q=, and ([^&]+) refuses that too, because it requires at least one character.

Fix

Each query pair becomes optional and the value may be empty:

pattern: `(?:${prefix}${this.escapeRegExp(name)}=([^&]*))?`

A group that did not participate is skipped rather than turned into undefined:

const value = match[i + 1];
if (value === undefined) continue;

match(t.expand(vars)) can then no longer be null for reasons the class itself created, and an omitted variable is simply absent from the result ({ q: 'test' }, not { q: 'test', page: undefined }).

Order, literal text and the mandatory path segments are untouched: the groups stay in template order and every other operator keeps its existing pattern.

Tests

Three cases added to packages/core-internal/test/shared/uriTemplate.test.ts (matching complex patterns):

1. match(expand({ q: 'test' })) → { q: 'test' }
2. match('/search?q=test') → { q: 'test' }
3. control: match('/search?q=test&page=1') → { q: 'test', page: '1' } (unchanged)

Results (vitest, Node v22.22.2, Windows):

• before: 2 failed | 39 passed — cases 1 and 2 fail with expected null to deeply equal { q: 'test' }, the control and all 36 pre-existing cases pass;
• after: 41 passed (36 pre-existing + 2 new + control).

Counter-proof: the new tests were re-run with uriTemplate.ts restored byte-for-byte from main and only the test file kept — the same two cases fail, the control still passes.

Evidence boundary

• The suite was executed against a copy of uriTemplate.ts in a standalone vitest workspace (this machine has no pnpm install of the repo), with the repo's own test file; the assertions use only the module's public API.
• prettier --check with the repo config (.prettierrc.json: 4 spaces, single quotes, printWidth 140) passes on both files.
• tsc --noEmit --strict passes on uriTemplate.ts; match[i + 1] narrowing to string | undefined is what the new guard consumes.
• Not verified: behaviour of consumers that pass templates with query expressions only ({?a} against a bare path now returns {}); that is the documented expansion of an empty variable set, but it is a visible semantic change for such callers and CI will run the package's own suites.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8a4dda1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: 8a4dda1

[abhiramshaji-cyber]

Thanks for this, the writeup with the match(expand(...)) round trip made the bug really easy to follow, and the three tests read nicely. I tried a few more subsets and found one that still returns null (inline). Also changeset-bot says there's no changeset yet, not sure if you meant to add one. Happy to hear if I'm missing something here!

[abhiramshaji-cyber]

I think the case where the first variable is left out still slips through. expand({ page: '1' }) gives /search?page=1, but the second pair has to start with &, so match returns null. Would something like this work for you?

Suggested change

	const prefix = i === 0 ? '\\' + part.operator : '&';
	const prefix = i === 0 ? '\\' + part.operator : part.operator === '?' ? '[?&]' : '&';

With that, every subset I tried round trips (including {?a,b,c} with only b or c set) and the existing 41 tests still pass. Side note, /search&page=1 already matches on this branch with or without the change, not sure if that's worth tightening. What do you think?

[abhiramshaji-cyber]

Maybe worth one more case here for the leading variable being left out, like expect(template.match(template.expand({ page: '1' }))).toEqual({ page: '1' })?