Repository navigation
chore(deps): build the bundled ajv with fast-uri 3.1.9 - #3108
Conversation
Fixes #3101. **Before:** the `dist` of `@modelcontextprotocol/client` and `@modelcontextprotocol/server` inlines ajv with fast-uri 3.1.8. `pnpm audit` lists 4 fast-uri advisories, and a consumer cannot override an inlined copy. **After:** the lockfile resolves fast-uri 3.1.9, so both bundles inline it. `pnpm audit` lists none. No install requirement changes. 3.1.9 is now older than the 7 days that `minimumReleaseAge` asks for. **Visible change:** only unusual `$id` and `$ref` values behave differently; the changeset lists them. In a comparison of ajv with 3.1.8 and with 3.1.9 over 25,974 schemas (111 `$id` / `$ref` values, draft-07 and 2020-12), plain names, `#/$defs/...` references, ordinary URLs and URNs gave the same result. An install with its own ajv, v1.x included, already resolves 3.1.9. Same shape as #3083: lockfile, changeset, and the version in the type shim's comment (the `URIComponent` type is the same in both versions).
🦋 Changeset detectedLatest commit: 122503e The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| dependencies: | ||
| fast-deep-equal: 3.1.3 | ||
| fast-uri: 3.1.8 | ||
| fast-uri: 3.1.9 |
There was a problem hiding this comment.
🟡 (optional) Client users lose a remote tool entirely after a patch upgrade when its outputSchema has a $id/$ref that fast-uri 3.1.9 newly rejects. The compile throw is caught at packages/client/src/client/client.ts:1883, but assertCompiled at client.ts:2436 then rejects every callTool for that tool before the request is sent. The schema belongs to a third-party server, so the client user cannot repair it. Fix: the changeset must name this outcome (the InvalidParams rejection and its message) for each affected $id/$ref form, and a test should pin it.
Why this was flagged
If any remote server lists a tool whose outputSchema has a $id/$ref with a percent-encoded character before the first colon, {/} in an http(s)/ws(s) host, or a $ref of only dot segments, the bundled ajv at pnpm-lock.yaml:8821 now throws on compile. _compileOutputValidator (packages/client/src/client/client.ts:1879-1886) turns that into {ok:false, compileError}. assertCompiled (client.ts:2432-2438) then throws Tool '<name>' has an invalid outputSchema: ... before tools/call is sent, on every call. On the base branch the same schema compiled with fast-uri 3.1.8 and the tool ran. The try/catch the dismissal cites only keeps other tools working; its own action is the failure for this tool. The changeset at .changeset/bundled-fast-uri-3-1-9.md:6 says only that a schema "no longer compiles" and ships as a patch. I could not open fast-uri 3.1.9 offline; the rejected forms are taken from the changeset itself. Remedy: state the callTool outcome in the changeset and cover it with a test.
Verification: Triggering condition: a Node client using the default validator has listed the tools (or passes toolDefinition) and a remote server's tool outputSchema carries one of the $id/$ref forms that no longer compile. packages/client/src/client/client.ts:1881-1885 turns any throw into { ok: false, compileError: error }. So every callTool for that tool is rejected without reaching the wire.
Finding ID: f-4756b12c
| '@modelcontextprotocol/server': patch | ||
| --- | ||
|
|
||
| The bundled Ajv validator is now built with fast-uri 3.1.9 (was 3.1.8). A schema no longer compiles when a `$id` or `$ref` has a percent-encoded character before its first colon (`%61:b`), or `{` or `}` in the host of an `http`, `https`, `ws` or `wss` URL (`http://{host}/a`). Relative values keep their leading `..` segments: a `$ref` to a `$id` such as `../a` now resolves, and a `$ref` of only dot segments (`.`, `../`) no longer resolves to the root schema. Plain names, `#/$defs/...` references, other URLs and URNs are not affected. |
There was a problem hiding this comment.
🟡 nit (optional): Users are told of four $id/$ref behaviour changes that no test in the repo checks, so the changeset wording cannot be confirmed or kept true. The diff changes only the lockfile, a comment and the changeset; packages/core-internal/test/validators/validators.test.ts has one $id case (https://example.com/schemas/test, line 328). Fix: add validator tests that compile schemas with the listed values (%61:b, http://{host}/a, a $ref to $id ../a, a $ref of . or ../) and assert the stated result for each. [also at: .changeset/bundled-fast-uri-3-1-9.md:6 - nit: CLAUDE.md principle 7 ("Test what users see") asks for tests of user-visible behaviour: this changeset announces four $id/$ref behaviour changes in the bundled Ajv validator (%61:b and http://{host}/a no longer compile, a $ref to ../a now resolves, a dot-segment-only $ref no longer resolves to the root), and the diff adds no test for any of them.]
Why this was flagged
No runtime failure; this is the check CLAUDE.md principle 7 asks for. The trigger is a schema whose $id or $ref is one of the values named in .changeset/bundled-fast-uri-3-1-9.md:6, reaching the bundled Ajv through engine.compile(schema) at packages/core-internal/src/validators/ajvProvider.ts:135. The diff changes what that call does for those values (pnpm-lock.yaml:8821 moves ajv@ 8.18.0 to fast-uri 3.1.9) but adds no test, and a repo-wide search finds no test using %61, {host} or a dot-segment $id/$ref. On the base branch these schemas compiled (or resolved differently) and that was equally untested, but the base made no published claim about them. After merging, the released changelog states exact outcomes that nothing in CI verifies, and the next fast-uri bump can change them silently. No safeguard applies: node_modules is absent from the checkout, so the claims cannot be checked against the 3.1.9 source in review either.
Verification: The diff touches only .changeset/bundled-fast-uri-3-1-9.md, a one-line comment in packages/core-internal/src/validators/fastUriShim.d.ts, and pnpm-lock.yaml. No test file is added or changed. packages/core-internal/test/validators/validators.test.ts:326-334 is that file's only $id case ($id: 'https://example.com/schemas/test').
Finding ID: f-03139411
| * Runtime code is unaffected (this is a `.d.ts`; the path mapping is dts-only). | ||
| */ | ||
| // Field-for-field copy of fast-uri@3.1.8's URIComponent (types/index.d.ts). | ||
| // Field-for-field copy of fast-uri@3.1.9's URIComponent (types/index.d.ts). |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: nit: CLAUDE.md principle 8 asks for one-line inline comments: the line this diff edits in fastUriShim.d.ts is the first of a two-line // comment (lines 14-15: the "Field-for-field copy of fast-uri@ 3.1.9's URIComponent" line plus "Keep in sync when bumping fast-uri/ajv."). The two-line shape predates this PR; the diff only changes the version number. Fix: since the line is being touched anyway, fold both into one line, e.g. "Copy of fast-uri@ 3.1.9's URIComponent (types/index.d.ts); keep in sync when bumping fast-uri/ajv."
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Nothing fails at runtime. The instruction guards against inline comments growing past one line. This is a pre-existing layout nit: the comment was already two lines at the base commit, and the diff re-touches line 14 only to change 3.1.8 to 3.1.9, so the violation is unchanged in substance. The consequence is cosmetic. The 12-line header JSDoc above it is on a non-public file as well, but the diff does not touch it, so it is not part of this finding.
Verification: Instruction: root CLAUDE.md at base b415e60, principle 8, verbatim as quoted: "Brief comments — Keep inline comments to one line and JSDoc to the public API; the reasoning goes in the commit message or PR description." It is a root-level convention, so it binds packages/core-internal/src/validators/fastUriShim.d.ts. Code: the diff changes line 14 ("// Field-for-field copy of fast-uri@ 3.1.9's URIComponent (types/index.d.ts).") which is the first line of a two-line // inline comment; line 15 ("// Keep in sync when bumping fast-uri/ajv.") is untouched.
Finding ID: f-8ddc0afd
claude
Bot
added
the
v2
Requested by Felix Weinberger · Slack thread
Fixes #3101.
Before: the
distof@modelcontextprotocol/clientand@modelcontextprotocol/serverinlines ajv with fast-uri 3.1.8.pnpm auditlists 4 fast-uri advisories, and a consumer cannot override an inlined copy.After: the lockfile resolves fast-uri 3.1.9, so both bundles inline it.
pnpm auditlists none. No install requirement changes. 3.1.9 is now older than the 7 days thatminimumReleaseAgeasks for.Visible change: only unusual
$idand$refvalues behave differently; the changeset lists them. In a comparison of ajv with 3.1.8 and with 3.1.9 over 25,974 schemas (111$id/$refvalues, draft-07 and 2020-12), plain names,#/$defs/...references, ordinary URLs and URNs gave the same result. An install with its own ajv, v1.x included, already resolves 3.1.9.Same shape as #3083: lockfile, changeset, and the version in the type shim's comment (the
URIComponenttype is the same in both versions).Generated by Claude Code