fix(mcp): send content or structuredContent to the model, never both - #3234
fix(mcp): send content or structuredContent to the model, never both#3234xpzouying wants to merge 7 commits into
Conversation
… no usable text Servers that follow the MCP spec's backwards-compatibility SHOULD return the same JSON both as a TextContent block and as structuredContent. Forwarding both to the model sent the same data twice. structuredContent now rides the mcp-structured-result block only when the content blocks carry no usable text; _meta still always passes through.
🦋 Changeset detectedLatest commit: c7814b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40bfd62987
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| const structuredExtras: Record<string, unknown> = {}; | ||
| if (result.structuredContent !== undefined) { | ||
| if (result.structuredContent !== undefined && !hasUsableText) { |
There was a problem hiding this comment.
Any non-empty text part suppresses structuredContent, even though it is not evidence that the text serializes the structured value. When an MCP server returns a human-readable status or summary such as Found 3 results together with structured fields containing the actual result records, this drops the only copy of those records before they reach the model; the v2 mirror has the same predicate. Compare against a serialization of the structured value, or retain structuredContent unless duplication can be established.
Useful? React with 👍 / 👎.
…erver Round-trip four server result shapes (dual-emit, structuredContent-only, prose+structured, vendor _meta) through StdioMcpClient and the output pipeline, so the fallback behaviour is checked against real protocol bytes instead of hand-built result objects.
…ation The earlier has-usable-text gate also suppressed the structured payload when content was a lossy human summary — the primary case from MoonshotAI#2554 (list_projects returning 'N item(s)' while the items live in structuredContent). Skip the structured block only when a content text block parses to the same JSON value (semantic compare, key order and formatting insensitive); summaries and structured-only results still pass through.
xpzouying
changed the title
…y cover it Replace the verbatim-serialization comparison with a size heuristic: well-behaved servers render the same data into content (the spec's dual-emit, or a faithful human reorganisation), and either way the text measures at roughly the same size as the payload, so forwarding it would double the information. Append the structured payload only when content carries no usable text, or when the payload is more than twice the text size — the signature of a lossy summary. Verified against a live video- editor MCP whose tools all measure a json/text ratio of 1.2-1.9.
xpzouying
changed the title
Final policy: content and structuredContent are alternatives. content wins whenever it carries anything usable (a media block or non-whitespace text); structuredContent fills in only for an empty content array. There is no reliable signal that the structured payload is richer than what the server already rendered into content, so no size or structure heuristic is attempted. _meta still always passes through.
xpzouying
changed the title
The block carries structuredContent and/or _meta; the old mcp-structured-result name was inaccurate whenever it is a pure _meta carrier.
1 participant
Related Issue
Follow-up to #2596 (which resolved #2554), addressing user feedback that MCP tool results reached the model twice.
Problem
#2596 made the MCP
structuredContentand_metafields visible to the model by appending them as a serialized<mcp-result-extras>text block. But servers already render their data intocontent— as the spec's verbatim-JSON fallback (Google Workspace style) or as a human-readable reorganisation — so the model received the same information twice.What changed
contentandstructuredContentbecome alternatives — never both forwarded:contentwins whenever it carries anything usable (a media block or non-whitespace text);structuredContentfills in only when the content array is effectively empty (structuredContent-only servers keep working, preserving the fix(mcp): pass structuredContent and _meta through to model-visible tool output #2596 fix);_meta(vendor-namespaced keys, protocol-reserved keys still filtered) is unchanged and always passes through.Why not a smarter heuristic: there is no reliable signal that the structured payload is richer than what the server rendered into content — semantic equality misses faithful human reorganisations, and size ratios misjudge both directions. Rather than guess, the rule is absolute. The accepted trade-off: servers that put a lossy summary in content while keeping the real payload only in structuredContent will have that payload suppressed — such servers should render the data into content (the spec's SHOULD).
Applied identically to the v1 (
packages/agent-core/src/mcp/output.ts) and v2 (packages/agent-core-v2/src/agent/mcp/output.ts) output pipelines.Verification
_metafiltering.structured-content-stdio-server.mjsfixture) and push five result shapes throughStdioMcpClient→toMcpToolResult→mcpResultToExecutableOutput:dual_emit/faithful_rendering/prose_plus_structured→ model receives content only, no structured block;structured_only→ payload still surfaced;meta_vendor→ vendor_metapasses through.Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.