fix(core): restore the v1 content default for tools/call results on the legacy era#2456
fix(core): restore the v1 content default for tools/call results on the legacy era#2456felixweinberger wants to merge 4 commits into
Conversation
…he legacy era
Deployed servers return structuredContent-only (and otherwise content-less)
tool results — SDK v1 accepted that shape for years via content.default([]),
and removing the default turned every such call into an INVALID_RESULT error
before application code could run, with no way to opt out downstream.
Restore the default on the neutral schema and the 2025 era. The
silent-empty-success hazard that motivated the removal is closed at its
actual source instead of at every legacy server's expense:
- the 2025 codec's strip arm rejects a stripped foreign resultType body with
no explicit content (an input_required round can never surface as a hollow
success), and likewise rejects a bare task-shaped body — modern task
vocabulary is not a legacy server omitting content;
- the 2026-era wire schemas stay strict: revision-2026-07-28 servers have no
legacy excuse.
Authoring is unchanged — the TypeScript surface requires content, and a
structured-only handler result now reaches the wire spec-valid as
content: [] (v1 behavior) instead of erroring.
Driven end-to-end over HTTP: a live tools/call answering
{"structuredContent": ...} with no content resolves with content: [].
🦋 Changeset detectedLatest commit: e4dc30c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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}
Review + CI caught the first cut over-reaching: the codec-level bare-task guard fired before explicit-schema validation, breaking the documented 2025 task-interop overload (task IS this era's wire vocabulary), and the guard enumeration missed the other input_required members. Move the protection to where it belongs: the registry's tools/call entry is now a wire-seam schema that refuses to default content for a body carrying another result family's vocabulary (task, inputRequests, requestState — the era is frozen, so the list is complete), then pipes into the plain schema. The explicit-schema task-interop overload never consults that map and is pinned working by a new test; the strip-on-lift resultType guard stays in the codec where the strip knowledge lives. Server-side authoring is now era-independent: a content-less handler result is normalized to content: [] before era validation on every leg — previously the flipped test only covered the legacy leg while a 2026-negotiated connection still rejected the same handler with -32602. Also updates the stale registry/typedMapAlignment/server-test comments that described the removed strict-schema mechanism, and pins the wire-seam wrapper by reference in registryPins.
The sniffer relied on the neutral result union rejecting input_required bodies by content strictness; with the legacy-era content default restored that shape parses, so the era-gate is asserted by vocabulary instead — which states the actual rule.
… all husk guarding at the wire seam Review follow-ups: - The server-side authoring normalization excluded only task-shaped bodies, so a handler result carrying inputRequests/requestState (without the resultType discriminator) was normalized to content: [] and shipped as a silent empty success where it was previously a loud -32602. The wire-seam vocabulary list is now a shared constant (TOOL_RESULT_FOREIGN_FAMILY_KEYS) used by both the registry guard and the normalization, with a test. - The 2025 codec's strip-arm content check ran in the shared decode hop, so it also fired for explicit-schema callers — an interop CreateTaskResult body that additionally stamped resultType was rejected before the caller's schema ran, contradicting the interop guarantee. Deleted: the strip is a pass-through again and the wire-seam schema owns all husk guarding (the input_required fixture carries family vocabulary, so V-1 still holds; a husk that was ONLY a resultType key defaults like any payload-free body, v1 parity). New test pins explicit-schema interop with a stamped resultType. - support-2026-07-28.md no longer lists 'content required' among the v1-affecting tightenings; the dangling half-rewritten comment in typedMapAlignment is completed and its title updated.
| --- | ||
| '@modelcontextprotocol/core-internal': patch | ||
| '@modelcontextprotocol/server': patch | ||
| --- |
There was a problem hiding this comment.
🔴 The changeset only bumps @modelcontextprotocol/core-internal (private, consumed by the published packages only as a devDependency and bundled at build time) and @modelcontextprotocol/server, so @modelcontextprotocol/client — the package whose inbound decode path (restored content.default([]), CallToolResultWireSchema, the rev2025 codec guard) is the very fix this PR exists to ship — is not guaranteed a release and won't credit the change in its changelog (the changeset-bot confirms only 2 packages release). Add an explicit '@modelcontextprotocol/client': patch entry (and arguably '@modelcontextprotocol/core', whose published CallToolResultSchema parse behavior also changed), matching the repo convention of naming client whenever bundled client behavior changes.
Extended reasoning...
What the bug is
The changeset .changeset/calltoolresult-content-default.md names only @modelcontextprotocol/core-internal (patch) and @modelcontextprotocol/server (patch). But core-internal is private: true and is consumed by the published packages only as a devDependency (workspace:^ in packages/client/package.json, packages/core/package.json, and packages/server/package.json); the published packages bundle it at build time (packages/client/tsdown.config.ts lists noExternal: ['@modelcontextprotocol/core-internal', ...]). Changesets' updateInternalDependencies: 'patch' (see .changeset/config.json, with fixed: [] and linked: []) cascades bumps only through dependencies/peerDependencies, never devDependencies — so bumping core-internal does not cause client (or core) to release. The changeset-bot comment on this PR confirms it: only 2 packages will be released.
Why this matters for this specific PR
The primary consumer of the change is the client inbound decode path: the restored content.default([]) in the neutral schemas and 2025-era wire schemas, the new CallToolResultWireSchema registry entry, and the rev2025 codec strip-arm guard are all bundled into the published @modelcontextprotocol/client artifact. The PR's own motivation — "a large production host", "previously-working third-party connectors turned into hard tool-call failures" — is a client calling deployed servers. The server-side authoring normalization is secondary (and server is correctly listed for it); the package that actually needs to ship for the stated problem to be fixed is client.
Step-by-step proof
- This PR merges with the changeset as-is.
changesetsrecords patch intents forcore-internalandserveronly. - On the next
changeset versionrun,core-internal(private) andserverare bumped.clientandcoreare not bumped by this changeset, because their only relationship tocore-internalis a devDependency, which the cascade rule ignores. changeset publishreleases a new@modelcontextprotocol/servercontaining the fix, but no new@modelcontextprotocol/clientis published on account of this change — the very host application in the PR's motivation cannot pick up the client-side tolerance until some unrelated changeset happens to bumpclient.- Even in the case where
clientdoes get bumped in the same cycle by an unrelated pending changeset (e.g.cross-bundle-error-instanceof.md,cjs-ajv-validator-subpath.md), the fixed code rides along only incidentally and the client CHANGELOG never documents this parse-behavior change (an empty/structured-onlytools/callresult now succeeds withcontent: []instead ofINVALID_RESULT) — a behavior change downstream users would want to see attributed.
Why nothing else prevents it
There is no tooling that flags "a bundled-behavior change with no changeset entry for the bundling package" — the repo relies on convention. That convention is visible in the sibling pending changesets: cross-bundle-error-instanceof.md, silent-validators-wave.md, cjs-ajv-validator-subpath.md, and probe-window-handler-restore.md all name @modelcontextprotocol/client explicitly whenever client-shipped behavior changes; this is the only changeset that names core-internal at all. The partial mitigation (other pending changesets currently bump client) is timing-dependent: a Version Packages PR that consumes those changesets before this PR merges leaves a subsequent release shipping the fix in server but not client.
How to fix
Add '@modelcontextprotocol/client': patch to the changeset frontmatter (and consider '@modelcontextprotocol/core': patch, since its published CallToolResultSchema parse behavior changed — an empty object now parses with content: []). Optionally drop the core-internal line to match the convention used by every other changeset in the repo; either way, the client entry is the one that guarantees the fix ships and is documented where users will look for it.
| // (InputRequiredResultSchema lives alongside, never widening it). | ||
| // Era-gated: only cells wired for the modern era opt in, so an | ||
| // input_required on a 2025-era cell's wire is still flagged. | ||
| if (party === 'server' && opts.allowInputRequiredResults === true && isInputRequiredResult(result)) return; | ||
| if (party === 'server' && isInputRequiredResult(result)) { | ||
| if (opts.allowInputRequiredResults === true) return; | ||
| // Explicit flag: the neutral union no longer rejects this shape | ||
| // by strictness (CallToolResult.content defaults on the legacy | ||
| // era), so the era-gating is asserted here, by vocabulary. | ||
| fail(party, `input_required result on a cell not opted into multi-round-trip`, msg); | ||
| } | ||
| const r = schemas.result.safeParse(result); | ||
| if (!r.success) { | ||
| // A result for a vendor-extension request legitimately won't match the spec union. |
There was a problem hiding this comment.
🟡 The wire sniffer's server-result conformance check (schemas.result.safeParse(result)) validates against the neutral ServerResultSchema union, and with content.default([]) restored on the neutral CallToolResultSchema, essentially any object-shaped result body now parses via the CallToolResult member — so the shapes the sniffer used to flag on legacy cells (task-shaped CreateTaskResult, a ListToolsResult missing tools, a GetPromptResult missing messages, etc.) silently pass; only input_required got a compensating explicit vocabulary check in this hunk. Consider a compensating check for the other shapes (e.g. an analogous task-vocabulary check, or parsing server results against the era wire schemas / registry entries instead of the widened neutral union), or at least a comment acknowledging that the general conformance assertion is now largely vacuous.
Extended reasoning...
What changed and why the check is now vacuous
assertWireMessage in test/e2e/helpers/wire-sniffer.ts validates every server result body with schemas.result.safeParse(result), where schemas.result for the server party is the neutral ServerResultSchema union. That union includes the neutral CallToolResultSchema, which is a loose ResultSchema.extend whose only required member was content. This PR restores content: z.array(ContentBlockSchema).default([]) on that neutral schema (packages/core-internal/src/types/schemas.ts). A Zod union succeeds if any member succeeds — so after this PR, essentially any plain-object result body satisfies the union via the CallToolResult member: content defaults to [], structuredContent/isError are optional, and unknown siblings pass through the loose base.
What the sniffer used to catch
Pre-PR, a body with no content that matched no other union member failed every member and the sniffer raised 'result does not conform to any spec result'. Concretely, on a legacy (2025-era) cell the sniffer would flag a server that put any of these on the wire:
- a task-shaped
CreateTaskResultbody ({ task: {...} }), - an
input_requiredbody, - a malformed
ListToolsResultmissingtools, - a
GetPromptResultmissingmessages.
Step-by-step, post-PR
- A server on a legacy cell answers
prompts/getwith{ description: 'oops' }(nomessages). - The sniffer runs
ServerResultSchema.safeParse({ description: 'oops' }). GetPromptResultSchemarejects (missingmessages), but the union keeps trying members and reachesCallToolResultSchema:contentis absent → defaults to[];descriptionsurvives as a loose sibling → parse succeeds.r.successis true, so the'result does not conform to any spec result'failure never fires. Pre-PR, step 3 failed (content was required) and the sniffer flagged the message.
The same walk-through applies to a task-shaped body, a tool-less ListToolsResult, or any other content-less spec-nonconforming result.
Why the diff only partially compensates
The hunk in this PR shows the author understood the union no longer rejects by strictness — the new comment says exactly that ('the neutral union no longer rejects this shape by strictness') — and it adds an explicit vocabulary check for one shape: an isInputRequiredResult body on a cell not opted into multi-round-trip now fails explicitly. But that guard covers only the input_required shape. Every other body the union used to reject is now silently accepted, so the general server-result spec-conformance assertion the sniffer exists for is largely defanged, without a comment or compensating check for the remaining shapes.
Impact and how to fix
This is e2e test-infrastructure coverage only: no runtime/shipped behavior changes, the SDK server constructs conforming results itself, and the individual e2e stories still assert their own results — so nothing concretely breaks by merging as-is. Options to restore the coverage: (a) add an analogous vocabulary check for task-shaped bodies (mirroring the OTHER_RESULT_FAMILY_KEYS guard the PR adds at the wire seam), (b) parse server results against the era wire schemas / registry entries (getResultSchema) instead of the widened neutral union, or (c) at minimum add a comment on the schemas.result.safeParse call acknowledging that the conformance assertion is now largely vacuous for content-less bodies, so future readers don't rely on it.
| // normalized — they must fail era validation loudly, never | ||
| // masquerade as an empty tool result. | ||
| const normalizedResult = | ||
| result !== null && |
There was a problem hiding this comment.
🟡 The normalization predicate at server.ts:534-537 does not exclude arrays, so a dynamic/JS handler that returns the content array directly (e.g. return [{type:'text',text:'hi'}]) gets spread into {"0": {...}, content: []} and ships as a silent empty-content success — where pre-PR the same mistake failed loudly with -32602. This is a separate trigger from the OTHER_RESULT_FAMILY_KEYS gap already noted at this site; adding !Array.isArray(result) to the predicate (or reusing an isPlainObject-style check, as the codec and the registry guard already do) closes it.
Extended reasoning...
What the bug is
The new v1-parity normalization in Server._wrapHandler (packages/server/src/server/server.ts:534-537) guards on result !== null && typeof result === 'object' && !('content' in result) && !('task' in result). Unlike the codec's own isPlainObject helper (rev2025-11-25/codec.ts) and the registry's CallToolResultWireSchema superRefine — both of which explicitly exclude arrays via Array.isArray — this predicate does not exclude arrays. An array is typeof 'object', is not null, and has no own or inherited content/task property, so it satisfies all four conditions.
The code path that triggers it
A JS/dynamic tools/call handler returns the content array directly instead of the result object — e.g. return [{ type: 'text', text: 'hi' }]. This is a plausible authoring mistake precisely in the population this normalization exists to serve (the TypeScript surface prevents it, but the whole point of the affordance is dynamic/JS callers). isInputRequiredResult (guards.ts:103-107) returns false for arrays, so the input-required seam does not intercept it, and the result reaches the normalization step, which produces { ...arr, content: [] } — an object with numeric-string keys plus an empty content array.
Why the existing guards don't catch it
On the 2025 era, CallToolResultWireSchema's superRefine returns immediately because 'content' in value is now true (the spread destroyed the array-ness and injected content: []), and the piped CallToolResultSchema is a loose ResultSchema.extend — content: [] parses and the numeric-string keys pass through as loose siblings. On the 2026 era, the dispatch schema (liftedResult) only requires content, which is present as []. So the wire carries {"0": {...}, content: []} as a successful tools/call response.
Step-by-step proof
- Register
server.setRequestHandler('tools/call', () => [{ type: 'text', text: 'hi' }] as any)on a legacy-negotiatedServer. isInputRequiredResult(result)→ false (arrays are rejected by the guard), so the seam is skipped.- Normalization predicate:
result !== null✓,typeof result === 'object'✓ (arrays),!('content' in result)✓,!('task' in result)✓ →normalizedResult = { ...[{type:'text',text:'hi'}], content: [] }={ '0': { type: 'text', text: 'hi' }, content: [] }. codec.validateResult('tools/call', normalizedResult): the wire-seam superRefine sees'content' in valueand returns early; the piped looseCallToolResultSchemaacceptscontent: []and keeps'0'as a loose sibling — validation succeeds.- The client receives
{ content: [], '0': {...} }— a hollow success with the author's actual content silently dropped.
Impact and how to fix
Before this PR, the raw array went straight to codec.validateResult and Zod's object schemas reject arrays (invalid_type: expected object, received array), so the author saw a loud -32602 Invalid tools/call result immediately. After this PR, the same authoring mistake becomes a silent empty-content success — exactly the loud-error → hollow-success class the PR states it preserves elsewhere. Note this is genuinely distinct from the earlier inline comment about OTHER_RESULT_FAMILY_KEYS at the same site: excluding those keys does not exclude arrays. The fix is one term — add !Array.isArray(result) to the predicate at server.ts:535 (or reuse an isPlainObject-style check to mirror the codec and registry guards), so array-shaped results keep failing loudly at validation. Severity is a nit because the trigger requires a mis-authored dynamic/JS handler and correct code is unaffected, but tightening the guard while this normalization is fresh (and already being adjusted for the sibling review comment) is cheap.
| // v1-parity authoring affordance, era-independent: a handler | ||
| // result without content (typically structuredContent-only from | ||
| // dynamic/JS callers — the TypeScript surface requires content) | ||
| // is normalized to content: [] before era validation, so the | ||
| // wire is spec-valid on every leg. Bodies carrying another | ||
| // result family's vocabulary (task / input_required) are not | ||
| // normalized — they must fail era validation loudly, never | ||
| // masquerade as an empty tool result. | ||
| const normalizedResult = | ||
| result !== null && | ||
| typeof result === 'object' && | ||
| !('content' in result) && | ||
| !TOOL_RESULT_FOREIGN_FAMILY_KEYS.some(key => key in result) | ||
| ? { ...result, content: [] } | ||
| : result; | ||
|
|
||
| const validationResult = codec.validateResult('tools/call', normalizedResult); |
There was a problem hiding this comment.
🟡 A handler result that carries a foreign-family key alongside an explicit content: undefined (e.g. a proxying handler forwarding { task: r.task, content: r.content } where r.content is absent) bypasses both the normalization's foreign-family exclusion here and the wire-seam superRefine, because both key on property existence ('content' in ...) while Zod's restored .default([]) fires on an undefined value — so the body ships as a hollow { content: [], task: {...} } success where pre-PR it failed loudly with -32602. Treating content === undefined the same as an absent key in both checks (or deleting an undefined content key before the guards run) closes this last presence-vs-undefined gap; server-authoring seam only, since JSON can't carry undefined.
Extended reasoning...
What the bug is
Both new hollow-success guards this PR adds key on property EXISTENCE, while the restored .default([]) keys on the VALUE being undefined:
- The server-side normalization predicate at
packages/server/src/server/server.ts:534-542uses!('content' in result) && !TOOL_RESULT_FOREIGN_FAMILY_KEYS.some(key => key in result). When a handler result has an owncontentkey whose value isundefined,'content' in resultistrue, so the whole predicate short-circuits and the foreign-family exclusion is never consulted — the body passes through un-normalized. - The wire-seam guard,
CallToolResultWireSchema.superRefineinpackages/core-internal/src/wire/rev2025-11-25/registry.ts, returns early on'content' in value, so the same body skips theTOOL_RESULT_FOREIGN_FAMILY_KEYSrejection there too. - The piped 2025-era
CallToolResultSchema(rev2025-11-25/schemas.ts) now hascontent: z.array(ContentBlockSchema).default([]), and Zod applies a.default()whenever the field's input value isundefined— an explicitly-undefinedkey included — so the parse succeeds and materializescontent: [], with thetask/inputRequests/requestStatesibling surviving via the loose base.
The code path that triggers it
On a legacy-era connection, a dynamic/JS tools/call handler proxies a downstream body: server.setRequestHandler('tools/call', () => ({ task: downstream.task, content: downstream.content })) where downstream.content is absent. isInputRequiredResult() is false (no resultType), so the input-required seam is skipped and the result reaches the normalization step.
Step-by-step proof
- Handler returns
{ task: { taskId: 't-1', status: 'working', ... }, content: undefined }. - Normalization predicate (server.ts:536-542):
'content' in result→true→ predicate is false → body passed through unchanged; the foreign-family exclusion never runs. codec.validateResult('tools/call', body)resolves toCallToolResultWireSchema; itssuperRefinesees'content' in value→ returns immediately without checkingTOOL_RESULT_FOREIGN_FAMILY_KEYS.- The pipe hands the body to
CallToolResultSchema;content: undefinedtriggers.default([]),taskpasses through as a loose sibling → parse succeeds. - The wire carries
{ content: [], task: {...} }as a successfultools/callresponse — a hollow empty success carrying another result family's vocabulary, which is exactly the invariant the PR states it preserves ("another result family cannot default into an empty tools/call success"). The same holds forinputRequests/requestStatesiblings with an explicitundefinedcontent.
Why existing safeguards don't catch it
Pre-PR, content had no default, so content: undefined failed the required-array check and the handler got a loud -32602 Invalid tools/call result; post-PR the same authoring mistake becomes a silent success. The TypeScript surface only protects typed callers — the affordance explicitly targets dynamic/JS callers. JSON round-trips cannot produce undefined, so the inbound/client decode path is unaffected; this is a server-authoring-seam gap only, which is why it's a nit.
Distinct from the already-addressed findings at this site
The family-key exclusion (fixed in e4dc30c) and the open Array.isArray comment both concern the predicate's coverage when content is ABSENT. This is the presence-vs-undefined blind spot shared by both the normalization and the wire-seam superRefine — a separate trigger not covered by either fix.
How to fix
Treat content === undefined the same as an absent key in both checks — e.g. use (result as { content?: unknown }).content === undefined instead of !('content' in result) in the normalization predicate, and the analogous change in the superRefine's early-return — or delete an undefined content key before the guards run. A one-line change in each place, cheap to close while the seam is fresh.
| import * as z from 'zod/v4'; | ||
|
|
||
| import type { NotificationMethod, NotificationTypeMap, RequestMethod, RequestTypeMap, ResultTypeMap } from '../../types/types'; | ||
| import { TOOL_RESULT_FOREIGN_FAMILY_KEYS } from '../codec'; |
There was a problem hiding this comment.
🟡 The new value import of TOOL_RESULT_FOREIGN_FAMILY_KEYS from ../codec closes a runtime cycle (wire/codec.ts → rev2025-11-25/codec.ts → registry.ts → wire/codec.ts) that the wire layer's own documentation in wire/textFallback.ts explicitly forbids: any module graph that evaluates the rev2025 codec before wire/codec.ts (e.g. the import order in test/wire/legacyWrap.test.ts) builds ALL_CODECS while rev2025Codec is uninitialized — a TDZ ReferenceError under native ESM, or a silent [undefined, rev2026Codec] under transform-based runners that corrupts isSpecRequestMethod/isSpecNotificationMethod. Nothing breaks today (production always reaches wire/codec.ts first and the affected test graphs never call those functions), but the fix is trivial: define the constant in a leaf module (e.g. wire/toolResultFamilies.ts) and import it from codec.ts, this registry, and the barrel.
Extended reasoning...
What the bug is
Before this PR, every import of ../codec from the rev2025-11-25 modules was import type (erased at runtime), so the wire layer had no runtime cycle. Commit e4dc30c makes registry.ts VALUE-import TOOL_RESULT_FOREIGN_FAMILY_KEYS from ../codec (registry.ts:24-27), while wire/codec.ts already value-imports rev2025Codec from rev2025-11-25/codec.ts, which value-imports ./registry. That closes the runtime cycle wire/codec.ts → rev2025-11-25/codec.ts → registry.ts → wire/codec.ts.
Why this violates a rule the codebase itself states
wire/textFallback.ts (untouched by the PR) documents exactly this hazard as the reason it must remain a leaf: it "must NOT import from ./codec.js (which value-imports the rev codecs at top level — that would make a runtime cycle and a TDZ hazard for entries that evaluate a rev codec module first)." This PR introduces precisely that cycle one module over — through the registry instead of textFallback.
The concrete order-sensitive trigger
wire/codec.ts ends with a module-level eager read: const ALL_CODECS = [rev2025Codec, rev2026Codec]. If any module graph evaluates rev2025-11-25/codec.ts before wire/codec.ts, the chain rev2025 codec → registry → ../codec runs wire/codec.ts's body while rev2025-11-25/codec.ts is still mid-evaluation, so the rev2025Codec binding is uninitialized when ALL_CODECS is built. Rev-codec-first entry points already exist in the repo: test/wire/legacyWrap.test.ts imports rev2025-11-25/legacyWrap then rev2025-11-25/codec with nothing pulling wire/codec.ts first, and test/wire/encodeContract.test.ts has the same shape.
Step-by-step proof (empirically reproduced by a verifier)
- Create a test that imports
src/wire/rev2025-11-25/codec.tsbeforesrc/wire/codec.ts(the same order aslegacyWrap.test.ts). - ESM evaluation: rev2025 codec starts → registry starts → registry imports
../codec→wire/codec.tsruns to completion, includingconst ALL_CODECS = [rev2025Codec, rev2026Codec]— butrev2025Codechas not been initialized yet. - Under vitest's transform-based module handling,
ALL_CODECSsilently becomes[undefined, rev2026Codec]; callingisSpecRequestMethod('tools/call')then throwsTypeError: Cannot read properties of undefined (reading 'hasRequestMethod')atwire/codec.ts:331. Under native ESM semantics the same order is a load-time TDZReferenceError.
Why nothing catches it today (and why this is a nit, not a blocker)
The published bundles and the core-internal barrel always reach wire/codec.ts before the rev2025 modules (codec.ts is the only production importer of the rev2025 codec, and the layering-invariants suite forbids other src files from importing rev modules), so every shipped code path evaluates in a safe order. The registry only reads the constant lazily inside the superRefine callback, so the reverse direction never faults. The existing rev-codec-first tests (legacyWrap.test.ts, encodeContract.test.ts) pass because they never call isSpecRequestMethod/isSpecNotificationMethod. So merging as-is breaks nothing observable — this is latent, order-dependent fragility rather than a live failure, but it makes the correctness of the whole wire layer's spec-method-universe functions depend on which module happens to be imported first, in a layer whose own comments treat that as a hard rule.
How to fix
Define TOOL_RESULT_FOREIGN_FAMILY_KEYS in a leaf module (e.g. a small wire/toolResultFamilies.ts, or alongside wire/textFallback.ts) and import it from wire/codec.ts (re-exporting it there so the barrel export and server.ts import are unchanged), rev2025-11-25/registry.ts, and anywhere else that needs it. Alternatively, define it in the rev2025 registry/schemas layer and re-export from codec.ts. Either preserves the shared-constant intent (the vocabulary can't drift between the wire-seam guard and the server normalization) without the cycle.
| reaches the wire. One sharpening remains: a content-less body carrying another | ||
| result family's vocabulary (a task handle, an `input_required` round, or a stripped | ||
| foreign `resultType`) is still rejected loudly — tolerance never turns a different | ||
| result kind into a silent empty success. |
There was a problem hiding this comment.
🟡 The bullet at docs/migration/upgrade-to-v2.md:1681-1684 lists 'a stripped foreign resultType' among the content-less shapes that are 'still rejected loudly', but the final commit (e4dc30c) deleted the 2025 codec's content guard, so a body whose only foreign material is resultType (e.g. {resultType: 'complete', structuredContent: {...}}) is now stripped and defaults to content: []. Drop or reword that item so the enumeration matches the shipped guard, which only rejects bodies carrying task/inputRequests/requestState.
Extended reasoning...
What the guide claims vs. what the code ships
The rewritten "Wire tightening (every era)" bullet in docs/migration/upgrade-to-v2.md (lines 1681-1684) says: "One sharpening remains: a content-less body carrying another result family's vocabulary (a task handle, an input_required round, or a stripped foreign resultType) is still rejected loudly." The third item — the stripped foreign resultType — was accurate for the intermediate commit 4f00b91, which added a guard in rev2025Codec.decodeResult rejecting a stripped body with no remaining content. The final commit e4dc30c deleted that guard (in response to the explicit-schema task-interop review comment), so the strip is a pass-through again and the sentence no longer describes the shipped behavior.
The code path
After e4dc30c, husk guarding lives entirely in the wire-seam schema: CallToolResultWireSchema in packages/core-internal/src/wire/rev2025-11-25/registry.ts, whose superRefine rejects a content-less body only when it carries one of TOOL_RESULT_FOREIGN_FAMILY_KEYS = ['task', 'inputRequests', 'requestState']. resultType is not (and cannot be) in that list — the codec strips it before the schema ever runs. The codec's own strip comment states the resulting behavior plainly: "A husk that was ONLY a resultType key defaults to an empty success — v1 parity for a payload-free body."
Step-by-step proof
- A 2025-era peer answers a plain-path
tools/callwith{resultType: 'complete', structuredContent: {ok: true}}(or a bare{resultType: 'input_required'}with noinputRequests/requestState). rev2025Codec.decodeResultseesresultType, strips it, and returnskind: 'complete'with the husk — no guard fires (the 4f00b91 guard that would have rejected this is gone).- The husk reaches
CallToolResultWireSchema. ItssuperRefinescans fortask/inputRequests/requestState, finds none, and passes through to the pipedCallToolResultSchema, wherecontentdefaults to[]. - The call resolves with
content: []— exactly the outcome the bullet says can never happen ("tolerance never turns a different result kind into a silent empty success").
Why nothing else catches it
The changeset (.changeset/calltoolresult-content-default.md) was updated for e4dc30c and correctly names only task/inputRequests/requestState; only this migration-guide bullet, last touched in 4f00b91, kept the earlier wording. So the guide now contradicts both the implementation and the codec's strip comment.
Impact and fix
Documentation accuracy only — no runtime behavior is affected, and the resulting behavior is arguably the intended v1 parity for a payload-free body. The fix is a one-phrase edit: drop "or a stripped foreign resultType" from the enumeration (or reword to say a stripped foreign resultType is tolerated like any other payload-free body, and only the family-vocabulary keys task/inputRequests/requestState are rejected).
1 participant
Restores
CallToolResult.content's.default([])parse tolerance on the neutral schema and the 2025 era, closing the silent-empty-success hazard at its source (codec vocabulary guards + strict 2026-era schemas) instead of at every legacy server's expense.Motivation and Context
Deployed servers return
structuredContent-only (and otherwise content-less) tool results — SDK v1 accepted that shape for years. After the v2 wire tightening, every such call fails inside the SDK withINVALID_RESULTbefore application code runs, and a host application has no way to restore tolerance downstream. Migrating a large production host surfaced this immediately: previously-working third-party connectors turned into hard tool-call failures.The original strictness had a real purpose — a task-shaped body must never parse as a silent
{content: []}success. That invariant is preserved, just enforced where the knowledge lives:resultTypebody or a bare task-shaped body that carries no explicitcontent;Authoring is unchanged: the TypeScript surface still requires
content, and a structured-only handler result now reaches the wire spec-valid ascontent: [](v1 behavior) instead of erroring.How Has This Been Tested?
contentDefaultParity.test.ts): structured-only and empty results resolve withcontent: []; a task-shaped body without content still fails loudly.schemaBoundaryPins,specTypeSchema,types.test,eraGates,rawResultTypeFirst, server handler-result validation).tools/callanswering{"structuredContent": …}with nocontentresolves withcontent: [].Breaking Changes
None — this widens accepted inputs back to v1 behavior. (Results that previously failed with
INVALID_RESULTnow succeed with an emptycontent.)Types of changes
Checklist
Additional context
The migration guide's "Wire tightening (every era)" section is updated to describe the restored tolerance and the remaining 2025-era sharpening (stripped foreign bodies still require explicit
content). Changeset: patch on@modelcontextprotocol/core-internal.