[shhhhhhjokerist]

Fixes #3182

Per CONTRIBUTING.md ("Before starting work, comment on the issue so we can assign it to you"), I asked for assignment in #3182 (comment); the issue is still unassigned, so I'm opening this small, test-covered fix now and will hand it over if a maintainer prefers to assign it to someone else.

Root cause

packages/core-internal/src/shared/uriTemplate.ts only knew the reserved/label/path/query operators (+ # . / ? &):

• getOperator (line 91) did not list ;, so {;x,y} was parsed as a simple expression whose variable names were literally ;x and ;y. Every variable lookup then missed and the whole expression collapsed to the remaining literal text ('' or a bare number), so path-style parameters could not be produced.
• Neither expandPart nor partToRegExp had a ; branch, so ;name=value segments could not be matched back into variables either.
• A list with zero members was treated as a defined value everywhere, so {/tags}, {?tags}, {.tags} and {#tags} expanded to /, ?tags=, . and # respectively — separators with nothing behind them.

RFC 6570 §2.3 says a list value with zero members is considered undefined, and §3.2.7 says a path-style parameter appends = only when its value is not empty:

{;who}         → ;who=fred
{;empty}       → ;empty
{;x,y,undef}   → ;x=1024;y=768
{;list}        → ;list=red,green,blue
{;list*}       → ;list=red;list=green;list=blue

Fix

• Add ; to the operator list (getOperator).
• Expand ; as ;name=value for each defined variable, ;name when the value is the empty string, values pct-encoded against the unreserved set (so ;, = and / stay literal separators), and ;name=value per member for exploded lists.
• Treat a zero-member list as undefined (isUndefinedValue), so it contributes nothing under every operator instead of leaving a dangling separator or an empty name= pair. A defined empty string is unaffected: {;empty} → ;empty, {?empty} → ?empty= (§3.2.8).
• Teach match to parse ;name=value and ;name segments back into variables; a variable that was omitted or empty matches as ''.

Diff is confined to uriTemplate.ts plus tests and a patch changeset for @modelcontextprotocol/core-internal and @modelcontextprotocol/server.

Tests

Run against packages/core-internal/test/shared/uriTemplate.test.ts. Added cases cover {;x,y}, the §3.2.7 examples (;who=fred, ;half=50%25, ;empty, ;v=6;empty;who=fred, {;v,bar,who}, {;x,y,empty}, {;x,y,undef}, {;list}, {;list*}), a literal-prefix template (X{;color,year} → X;color=red;year=2012), pct-encoding (;v=a%20b, ;v=a%3Bb%3Dc), matching (;x=1, ;x=1;y=2, X;color=red;year=2012, ;empty), negative matches (?x=1, ;y=1), and empty-list expansion across all eight operators.

RED on untouched main:

Test Files  1 failed (1)
Tests       10 failed | 38 passed (48)

GREEN after the fix:

Test Files  1 passed (1)
Tests       48 passed (48)

Checks run locally

• pnpm -r typecheck — pass
• pnpm -r lint — pass (ESLint + Prettier across every package)
• pnpm sync:snippets --check and pnpm check:skills — pass
• full @modelcontextprotocol/core-internal suite — 1863 passed (1863) (includes the schema-twin conformance tests)

Risk

Contained to UriTemplate. {;...} templates previously expanded to junk (the operator was swallowed into variable names), so no caller can be relying on the old output for ;; empty lists previously produced a bare separator, which callers do not depend on either. match keeps the existing lenient behaviour for literal prefixes and now also understands ;. Exploded ; lists are supported in expansion; matching covers the non-exploded ;name=value / ;name forms the issue reports.

Known limitation

An exploded ; list with exactly one member expands to ;list=red and matches back as the string 'red' rather than ['red']; a zero-member list expands to nothing and therefore does not match. Both follow from the existing string | string[] Variables type and the + in the repeated pattern, and are recorded in the changeset so nobody discovers them from a runtime error.

[changeset-bot]

🦋 Changeset detected

Latest commit: 155ae81

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/client	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	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]

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: 155ae81

[media-ctrl]

Thanks, the parse-side reading matches what I hit from the source, and sharing one isUndefinedValue between the expand and match sites is the right shape for the empty-list case, since the two directions disagreeing is the part that surprises people.

Four rows from the Section 3.2.7 table are not covered by the committed tests yet, and one of them exposes a match-direction gap:

{;list*}    ;list=red;list=green;list=blue
{;keys}     ;keys=semi,%3B,dot,.,comma,%2C
{;keys*}    ;semi=%3B;dot=.;comma=%2C
{;hello:5}  ;hello=Hello

{;list*} expands correctly with the new branch, but match cannot round-trip it. partToRegExp builds ';' + name + '(?:=([^;]*))?' once per name, so a uri of ;list=red;list=green;list=blue fails against the anchored pattern added at the end of match, because the first capture stops at the second semicolon and the rest of the string is left over. A repeated group such as (?:;name=([^;]*))+ collected into an array when part.exploded is true would close the round trip. The form-style ? arm has the same shape, so if you are already in that file it may be worth checking whether exploded lists round-trip there as well.

The two keys rows are outside the current type rather than broken by your patch: Variables is Record<string, string | string[]>, so an associative value has no representation, and value.toString() in the new branch would produce [object Object]. Leaving those rows out of this change looks right unless you want to widen the type in the same PR.

{;hello:5} is a parser gap and not yours. parse records only name, operator, names and exploded, so a prefix modifier stays inside the name, and it fails for every operator today. It should not block #3183.

One detail that deserves a code comment: match[i + 1] ?? '' is what makes ;empty return { empty: '' } instead of undefined, since the = group is optional in your pattern. That is the correct behaviour, but a reader cannot see why without the note.

If it keeps #3183 focused, I am glad to send a follow-up PR with the extra table rows and the exploded-list match test rather than pushing onto your branch.

[shhhhhhjokerist]

Thanks for the review — all three points are addressed in 5ec4821 (a new commit on top, no force-push):

1. Exploded ; round-trip. partToRegExp now emits ((?:;name(?:=[^;]*)?)+) for exploded === true. One caveat on the suggested shape: in JavaScript a repeated capture group keeps only its last iteration, so (?:;list=([^;]*))+ alone can't yield the member array. The pattern therefore captures the whole ;list=red;list=green;list=blue run, and match splits it on ; to rebuild ['red', 'green', 'blue']; a single ;list=red still comes back as the string 'red'. Non-exploded {;list} → ;list=red,green,blue now splits on , as well (expansion pct-encodes values, so a literal ;name= value can never contain a bare , or ;).
2. Missing §3.2.7 row {;list*} → ;list=red;list=green;list=blue is now an expand test, alongside round-trip tests for {;list*} and {;list}. {;keys} / {;keys*} stay skipped: Variables is Record<string, string | string[]>, so associative values have no representation in this API (commented in the test). {;hello:5} also stays out — prefix modifiers (:5) are not parsed at all in parse(), a pre-existing gap independent of ;.
3. Comment on match[i + 1] ?? '' added: the = group is optional, which is exactly how the bare ;empty that expand emits matches as '' instead of dropping the variable.

Gates on 5ec4821:

• pnpm test test/shared/uriTemplate.test.ts — 50/50. RED first: 2 failed | 48 passed (50), with expected null to deeply equal { list: [ 'red', 'green', 'blue' ] } (exploded) and expected { list: 'red,green,blue' } to deeply equal { list: [ 'red', 'green', 'blue' ] } (non-exploded).
• full @modelcontextprotocol/core-internal suite — 1865/1865.
• pnpm -r typecheck, pnpm lint:all, pnpm sync:snippets --check — all clean.

I don't see anything else left in this PR, but if you'd still rather take the keys / prefix-modifier rows as your own follow-up, that's a clean split — happy to either way.

[media-ctrl]

Read 5ec4821. All three points are closed as described, and the repeated-capture reasoning is correct: (?:;list=([^;]*))+ cannot yield the member array in JavaScript, so capturing the run and splitting it is the right workaround. Two notes, one of which I would rather see recorded than fixed here.

1. Single-member asymmetry. {;list*} with ['red'] expands to ;list=red and matches back as the string 'red', not ['red']. The new test documents it, which is the right call. But Variables is string | string[], so a caller doing result.list.map(...) gets a runtime error on the one-member case. One line in the changeset or PR body marking it a known limitation would save someone discovering it in production.

2. Empty list. expand treats a zero-member list as undefined (RFC 6570 section 2.3) and drops the segment, while the + in ((?:;name(?:=[^;]*)?)+) requires at least one segment. So {;list*} with [] expands to '' and then fails to match. That gap is library-wide, {var} with ([^/,]+) behaves the same way, so it is not this PR's business. Flagging only so the new repeated pattern is not mistaken for a fix of it.

3. splitOnComma on the non-exploded branch is safe as written: expansion runs through encodeURIComponent, so a literal , can never appear inside a member value.

No objection from me on the change.

[shhhhhhjokerist]

Recorded the single-member asymmetry as a known limitation in the changeset (155ae81, appended to this PR so the change is still reviewable as one patch set) and in the PR body; the zero-member-list gap stays documented as library-wide rather than something this PR claims to fix. Thanks for the careful read.