Skip to content

fix(client): compile output validators lazily for the called tool - #2998

Open
epistemedeus wants to merge 3 commits into
modelcontextprotocol:mainfrom
epistemedeus:codex/mcp-cold-call-root-142710
Open

epistemedeus wants to merge 3 commits into
modelcontextprotocol:mainfrom
epistemedeus:codex/mcp-cold-call-root-142710

Conversation

@epistemedeus

Copy link
Copy Markdown

Summary

  • Compile only the called tool's output-schema validator rather than every advertised schema during tools/list.
  • Preserve stamp-bound invalidation, cached/uncached Client behavior, explicit toolDefinition, and useful validation errors for the called tool.
  • Memoize undefined validators for catalogued tools, but do not retain arbitrary caller names absent from the catalog.
  • Add focused cold-call, cache/invalidation, refusal, and local stdio/Streamable HTTP coverage plus a changeset.

Fixes #2997.

Verification

On an actual remote Node 22.14.0 / pnpm 10.26.1 / Vitest 4.1.2 checkout: 22 focused tests and 1,019 full client tests pass; client typecheck, lint, build, and git diff --check pass. The extra absent-name cardinality regression failed before its shared-cache correction (21 pass / 1 fail).

A freshly deployed client calling a local 26-tool fixture observes zero output-validator compiles at list time and one for the first called tool across stdio and Streamable HTTP. Repeated stdio call adds no compile; invalid called schemas refuse before server execution; changed catalog rejects obsolete output; an explicit tool definition uses one compile and zero lists. These are local mechanism checks, not a reproduction of the reporter's 139-tool latency or a provider-performance claim.

This change addresses the Client list/call compile strategy. It is separate from #2950's provider-level AJV validator reuse and #2976's v1.x failed-refresh handling.

@epistemedeus
epistemedeus requested a review from a team as a code owner October 10, 2026 07:42
@changeset-bot

changeset-bot Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cccc109

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

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/client Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/core-internal 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

pkg-pr-new Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: cccc109

@claude claude Bot added the v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes label Oct 10, 2026

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 1fce22a1, focusing on stamp invalidation and negative-result caching. The implementation keeps the complete decoded catalog but compiles only the requested name, bounds memoization to catalogued tools, and replaces the validator map whenever the backing stamp changes. That preserves refresh/list-change/reconnect isolation while preventing an uncalled invalid schema from blocking a valid tool.

The focused cold-call/invalidation/transport coverage is present, git diff --check passes, and the hosted Node 20/22/24, e2e, conformance, runtime, build, examples, and package checks are green. I did not find a distinct correctness issue in this head.

@N0zoM1z0

Copy link
Copy Markdown

Verified this against REA's current 170-tool production catalog (morluto/rea@21650455a9c90a26e725c3505a3c17054cc4516c), using real stdio servers and SDK Client.callTool() without per-call toolDefinition. The released client 2.3.1 still exhibits the cold preparation cost; this PR removes it in the tested workflow.

I compared the exact published cache-owner source recompiled unchanged with the exact source at 1fce22a12ab91fb890aa7255f5448e1e1ff0b4a1. These were private owning-module experimental builds, not full upstream builds or published packages. Both CJS and ESM export regions were compiled identically; the actual loaded CJS entry and SHA were checked per trial. All other release bytes, including the standard AjvJsonSchemaValidator implementation, stayed identical. Its counting delegates forwarded all compilation and validation unchanged. The unmodified published CJS control and recompiled baseline had matching preparation/compilation counts and complete analysis/retained-view responses.

Five pairs, ordered 3 AB / 2 BA, on Node 24.18.0 with two CPUs and a 768 MiB Node heap:

First binary_session after full discovery Baseline PR source
Median call wall time 6,286.4 ms 723.6 ms
Output-validator preparations 170 1
Actual Ajv compilations 148 1
Median measured compilation time 5,508.0 ms 126.2 ms

The median within-pair call reduction was 5,533.5 ms (median paired fraction 88.0%). The catalog stayed complete and identical: 170 tools, 1,954,693 JSON bytes; all 340 advertised input/output schemas passed Ajv2020 meta-validation. Every trial exercised explicit same-catalog refresh, reconnect, retained JavaScript analysis summary/page, ping and close. Refresh/reconnect prepared only the called validator in the candidate; the provider's existing schema-ID cache avoided recompilation in both variants. Complete analysis/view bodies, Evidence IDs, metadata and text/structured parity matched the published baseline. A separately labeled malformed-wire-response injection was rejected by the actual client, followed by successful recovery.

I also used Flameox 0.2.9 on separate native V8 client profiles, restricted to the recorded first-call interval. Complete rankings confirmed Ajv compiler hotspots: addNames self sample hits 689 → 4, compileSchema 498 → 8. Original profiles, phase derivation and digests are preserved locally. These captures are excluded from the five timing pairs; sample hits are not CPU utilization, allocation or leak measurements. This verifies a cold client-call improvement, not whole-analysis throughput or changed-schema/list-changed coverage beyond your existing tests. REA's product dependency remains the published version pending the upstream merge and release.

Copy link
Copy Markdown
Author

@N0zoM1z0 Thanks for checking this against a real 170-tool catalog and keeping the owning-module experiment distinct from the published SDK. Would you be comfortable sharing a sanitized catalog snapshot or measurement harness that we could adapt into a maintained regression? No private traces or customer data needed. The existing changed-schema/list-changed tests and upstream CI remain separate evidence; your product-dependency and release distinction is helpful.

@N0zoM1z0

Copy link
Copy Markdown

Thanks. I shared REA’s public 170-tool catalog and a runnable stdio test in #3001. It reports cold validator preparation and compilation counts, and checks invalid-output rejection, recovery, and cache reuse.

@astrogilda

Copy link
Copy Markdown

@epistemedeus, thanks for adding tests for the selected tool. The fixture records provider getValidator requests delegated to AjvJsonSchemaValidator at base and head on Node 20.20.2.

# Node >=20; pnpm 10.26.1
set -e
git clone --filter=blob:none https://github.com/modelcontextprotocol/typescript-sdk.git
cd typescript-sdk
git fetch origin b022522089a0c8b632595c6e7b536453945ed5a9 pull/2998/head
mkdir -p lab-cases/mcp2998
fixture_source=https://raw.githubusercontent.com/probityai/agent-evidence-atlas/ece6bded7399177202a69efe90f5bbf80a264f6a/lab-cases/mcp2998
for file in consumer.test.mjs server.mjs; do
  curl --fail --location "$fixture_source/$file" -o "lab-cases/mcp2998/$file"
done
for revision in base head; do
  ref=b022522089a0c8b632595c6e7b536453945ed5a9
  if [ "$revision" = head ]; then
    ref=1fce22a12ab91fb890aa7255f5448e1e1ff0b4a1
  fi
  git checkout --detach "$ref"
  pnpm --filter @modelcontextprotocol/client... install --frozen-lockfile
  pnpm --filter @modelcontextprotocol/client... run build
  CALLED_TOOL_REVISION="$revision" node --test lab-cases/mcp2998/consumer.test.mjs
done

Both accepted pick with an unusable uncalled tool schema: first-call requests fell from 2 on base to 1 on head, with 0 more on repeat. Base catches compilation errors per tool, and this measurement excludes AJV compilation, latency, and whole-catalog validation.

Preserve the public 170-tool catalog fixture and deterministic lazy-compile, refusal, recovery, invalidation and portable stdio-exit regressions. Timing benchmark remains optional and environment-specific.

Signed-off-by: Lucian Constantinescu <epistemedeus@gmail.com>

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the single-commit delta at cccc109. The production-size fixture now exercises the same lazy-cache boundaries against 170 tools: one called validator, rejection and recovery, list-change/manual-refresh invalidation, configured-engine reuse, and stdio child shutdown. The timing path stays explicitly optional, while the assertions are count/body based. Hosted Node 20/22/24, e2e, conformance, runtime, build, examples, and package checks are green. I did not find a new correctness issue in this head.

@astrogilda

Copy link
Copy Markdown

@epistemedeus, I replayed the unchanged Atlas fixture at SDK head cccc109 on Node 20.20.2: six cases pass at both pinned revisions, alongside 31 current-head upstream tests.

Both revisions already accept the valid call with an unusable uncalled schema; first-call provider getValidator requests fall from 2 to 1, with 0 more on repeat. The report retains the revisions, artifact hashes and TAP extracts. These counts exclude Ajv compilations, timing and whole-catalog measurements.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClientResponseCache compiles every cached output schema before the first ordinary callTool

4 participants