[imbenrabi]

Requested by Ben · project thread

Stack and merge order

This is the base PR (B1) of a stack. Merge order:

1. This PR into main
2. Stacked on this branch (any order, each retargets to main after this merges): T1 chore(deps): update vite-plugin-dts to ^5.1.2 (stacked on #35) #36 vite-plugin-dts 5 (sprintf/minimatch advisory), T2 ci: run CI and release on Node 26 (stacked on #35) #37 CI/release on Node 26 (vitest 5), T3 JSON Schema tools via fromJsonSchema() (separate thread)
3. R1 release PR 0.9.0 (version + changelog), then Ben tags v0.9.0

Nothing here is tagged or published.

What changed

• @modelcontextprotocol/sdk → @modelcontextprotocol/server ^2.4.0 + @modelcontextprotocol/node ^2.1.2; zod ^3.25 → ^4.6.7; @modelcontextprotocol/client is a dev dependency for tests. SDK v2 cannot read Zod 3 schemas, so the two cannot be split.
• Ran the official @modelcontextprotocol/codemod v1-to-v2, then cleared its one @mcp-codemod-error marker.
• FastifyTransport and PermissionAwareFastifyTransport use NodeStreamableHTTPServerTransport. Session handling, the readiness gate, originValidation.ts and the per-client bundle model are unchanged (2025-era session path, no createMcpHandler).
• DynamicToolManager.toRegistrableInputSchema(): Zod 4 schema → as is; raw shape of Zod 4 values (incl. {}) → wrapped with toolception's own z.object(); Zod 3 schema → E_VALIDATION naming the tool, nothing registered; anything else → JSON Schema passthrough, now z.looseObject({}) (T3 changes this).
• http.utils.ts: ZodError.errors → .issues. vite.config.ts externals follow the new packages (otherwise the SDK would be bundled into dist).
• README, examples, smoke demos: new imports; capabilities now passed as the 2nd McpServer argument (the old position is a type error in v2).
• Intent layer (root, core, http, types nodes) and CHANGELOG [Unreleased] (breaking changes and migration).

Must-have: multiple clients, each with its own toolset state

New tests/e2e/multiClient.e2e.test.ts (8 tests) covers, concurrently connected clients:

• DYNAMIC: A enables/disables, B's list unchanged, and the reverse
• a disabled tool is removed and uncallable for that client only; the other client can still call it
• tools/list_changed only reaches the client whose state changed
• same client id reconnects and keeps its toolsets; another client id cannot use A's session (POST 400, DELETE 404)
• one McpServer created per client id
• session-context token of client A never visible to client B (DYNAMIC); STATIC bundles per client; permission-based clients with different permissions
• tests/dependencies.test.ts: exactly one copy of zod and @modelcontextprotocol/server in the lockfile

Behaviour changes to know about

• Calling a tool that is not (or no longer) registered now rejects with a JSON-RPC error ("Tool … not found") instead of returning isError: true (SDK v2). Three existing tests re-baselined.
• Disable of N tools still sends N+1 tools/list_changed; enable of one tool sends 2 (SDK + toolception). Checked on v2 with and without declared capabilities (listChanged is advertised either way).
• VALIDATION_ERROR details of custom endpoints now carry Zod 4 issue objects (listed in the changelog).

Checks (Node 26.13.0)

• npm run typecheck ✔, npm run build ✔ (dist imports @modelcontextprotocol/server/node, not bundled), npm run test:run ✔ 411 tests (was 395)
• Examples and smoke demos type-check; server-demo + client-demo smoke ✔ (DYNAMIC)
• Not run: STATIC and permission smoke demos by hand (covered by the e2e tests)

Advisories

• Before: 10 findings (2 high: zod, body-parser). After npm audit fix on the new tree: 4 moderate, all the dev-only minimatch chain via vite-plugin-dts 4 (cleared by T1, chore(deps): update vite-plugin-dts to ^5.1.2 (stacked on #35) #36). npm audit --audit-level=critical passes.
• zod high (GHSA-mrhx-c7xc-m3w5) cleared by 4.6.7; body-parser, qs, ip-address left the tree with the v1 SDK's Express deps; fastify 5.12.8, fast-uri 3.1.9.

Review

TS review (pipeline-ts-reviewer) round 1 COMMENT, round 2 APPROVE; see the PR comments. peerDependencies for @modelcontextprotocol/server and zod is deliberately not part of this PR (todo e3).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VmUXGSYMuTcM7fquxzRook

[imbenrabi]

TS review: pipeline-ts-reviewer (feature-pipeline)

Verdict (round 1): COMMENT. No CRITICAL or HIGH findings. Static review of the PR head diff against main (lockfile excluded) plus the installed SDK 2.4.0 / node 2.1.2 / zod 4.6.7 sources. The reviewer traced toRegistrableInputSchema for every schema shape, the rename to NodeStreamableHTTPServerTransport, and checked that the multi-client tests would fail if state leaked (shared server/orchestrator, shared sessions map, one bundle per request, bundle recreated on reconnect, shared server in permission mode).

#	Sev	Location	Finding
1	MEDIUM	package.json dependencies, README.md:82-86	@modelcontextprotocol/server and zod are regular dependencies although consumers must share the same copy; only this repo's lockfile is guarded against duplicates. Consider peerDependencies.
2	LOW	tests/smoke-e2e/permission-config-server-demo.ts:87, permission-header-server-demo.ts:89	Codemod left the ; inside the trailing comment (}) // STATIC mode, no listChanged;); works only through ASI.
3	LOW	src/http/FastifyTransport.ts:403, src/permissions/PermissionAwareFastifyTransport.ts:184	TSDoc quotes the old "Already connected" message; v2 throws SdkError AlreadyConnected with different wording.
4	LOW	src/http/http.utils.ts:300, CHANGELOG.md	VALIDATION_ERROR details now carries Zod 4 issue objects (different codes/messages); not listed under Breaking.
5	LOW	src/core/DynamicToolManager.ts:398,406	isZod4Type / isZod3Type have @param/@returns but no summary sentence.
6	LOW	tests/e2e/multiClient.e2e.test.ts:67	settle() is a fixed 100 ms sleep; poll for the positive notification instead.
7	LOW	tests/dependencies.test.ts:25	Asserts a single copy of the SDK-internal @modelcontextprotocol/core; brittle if the SDK reshuffles packages. Reads the lockfile, not the installed tree.
8	LOW	tests/dynamicToolManager.test.ts (Zod 3 refusal)	"Registers nothing" uses a one-tool toolset, so rollback of already registered tools is not exercised.

{
  "recommendation": "COMMENT",
  "summary": "The SDK v2 and Zod 4 migration is correct and well documented, and the multi-client e2e tests do detect state leakage; only MEDIUM and LOW issues remain.",
  "findings": [
    { "severity": "MEDIUM", "file": "package.json", "line": null, "issue": "@modelcontextprotocol/server and zod are regular dependencies; consider peerDependencies." },
    { "severity": "LOW", "file": "tests/smoke-e2e/permission-config-server-demo.ts", "line": 87, "issue": "Statement terminator left inside trailing comment (also permission-header-server-demo.ts:89)." },
    { "severity": "LOW", "file": "src/http/FastifyTransport.ts", "line": 403, "issue": "TSDoc quotes the old 'Already connected' error text (also PermissionAwareFastifyTransport.ts:184)." },
    { "severity": "LOW", "file": "src/http/http.utils.ts", "line": 300, "issue": "VALIDATION_ERROR details shape changes under Zod 4; not in changelog." },
    { "severity": "LOW", "file": "src/core/DynamicToolManager.ts", "line": 398, "issue": "isZod4Type/isZod3Type lack a TSDoc summary sentence." },
    { "severity": "LOW", "file": "tests/e2e/multiClient.e2e.test.ts", "line": 67, "issue": "Fixed 100 ms sleep in the list_changed isolation test." },
    { "severity": "LOW", "file": "tests/dependencies.test.ts", "line": 25, "issue": "Single-copy assertion on SDK-internal @modelcontextprotocol/core is brittle." },
    { "severity": "LOW", "file": "tests/dynamicToolManager.test.ts", "line": null, "issue": "Zod 3 refusal test does not exercise rollback of already registered tools." }
  ]
}

---

Generated by Claude Code

[imbenrabi]

Fix mapping for the TS review (round 1)

Fixes are in ac12cac. Numbers refer to the findings table in the review comment.

#	Finding	Resolution
1	MEDIUM: make @modelcontextprotocol/server and zod peer dependencies	Left unchanged, deliberately. Moving them to peerDependencies is todo item e3, which Ben scoped out of this stack (the plan says so explicitly). It is a separate decision with its own install-experience tradeoffs. The README already tells consumers to install the same versions, and the lockfile test guards this repo's tree.
2	Terminator inside trailing comment	Fixed: ; now precedes the comment in both permission server demos.
3	TSDoc quotes old "Already connected" text	Fixed: both transports name the AlreadyConnected error.
4	VALIDATION_ERROR details shape not in changelog	Fixed: new Breaking bullet in CHANGELOG.md [Unreleased].
5	Type guards lack a TSDoc summary	Fixed: summary sentences on isZod4Type and isZod3Type.
6	Fixed 100 ms sleep in the list_changed test	Fixed: waitFor() polls until client A is notified (3 s timeout), then asserts B's count is unchanged.
7	Single-copy check includes SDK-internal core	Fixed: only zod and @modelcontextprotocol/server are asserted. It still reads the lockfile; that is what npm ci installs, so I kept it.
8	Zod 3 refusal test does not exercise rollback	Fixed: the toolset now has a valid first tool and the Zod 3 tool second; the test asserts nothing stays listed.

Re-run: typecheck ✔, 411 tests ✔ (three consecutive runs, Node 26). Re-review follows.

---

Generated by Claude Code

[imbenrabi]

TS review: pipeline-ts-reviewer (feature-pipeline), round 2

Verdict: APPROVE. All 8 round-1 resolutions in ac12cac were verified as correct and complete (including that the Zod 3 refusal test now genuinely exercises rollback of an earlier tool). The delta touches comments, tests and the changelog only, with no runtime source change and no new CRITICAL/HIGH findings. The reviewer did not re-raise the peerDependencies item (left as is on purpose, todo e3).

Sev	Location	Finding
LOW	tests/e2e/multiClient.e2e.test.ts, list_changed isolation test	Removing the initial settle() also removed the implicit wait for the client's standalone SSE stream to open; a notification sent earlier would be dropped and waitFor would time out (fail-loud, unlikely on localhost). Optional: await a round trip on A first.

Resolution of the LOW (231a26c, test-only, one block): both clients now do a listTools round trip before the counts are taken, so their SSE streams are open. The multi-client file passed three times in a row, and the full suite passes (411 tests, Node 26).

{
  "recommendation": "APPROVE",
  "summary": "All 8 round-1 resolutions are correct and complete; the delta adds no CRITICAL/HIGH issues, only a LOW SSE-timing note in a test (since addressed in 231a26c).",
  "findings": [
    { "severity": "LOW", "file": "tests/e2e/multiClient.e2e.test.ts", "line": null, "issue": "Removing the initial settle() drops the implicit wait for the standalone SSE stream to open before enable_toolset fires." }
  ]
}

---

Generated by Claude Code