fix(mcp): preserve errors in schema-valid clients - #54
Conversation
A tool's outputSchema describes successful structuredContent. Clients
that validate structuredContent against it reject the error payload
({ error: ... } against a { data: ... } schema), replacing the
actionable error with a validation failure. Omit structuredContent on
error results; the full JSON error payload stays in the text content,
which is what MCP clients fall back to.
Signed-off-by: Jose Francisco 'Kiko' Verdú Gambín <2096101+Kikobeats@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughMCP tool responses now omit ChangesMCP error response shape
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change preserves successful structured responses and keeps complete error details in text content without identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for CI Build 34746554383Warning No base build found for commit Coverage: 79.515%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
2 participants
Problem
Since #47, every tool declares an
outputSchemadescribing its successfulstructuredContent({ data: ... }). Error results, however, were also sent asstructuredContent({ error: ... }).The MCP SDK skips output validation server-side when
isErroris set, but a spec-strict client can validate anystructuredContentit receives against the advertised schema.{ error: ... }never matches{ data: ... }, so those clients replace the actionable error payload (message,code,reason,hint,upgrade) with a schema-validation failure, hiding the very guidance the error taxonomy (#39) exists to provide.Fix
Omit
structuredContenton error results and keep the full JSON error payload in the text content, which is what MCP clients fall back to when nostructuredContentis present. One-line behavioral change in the single choke point (toToolResponseinmicrolink-client.js), through which every tool error flows:structuredContent: { data: ... }+ text mirror).isError: true+ text content carrying the identical JSON payload, nostructuredContent.Tests
Updated the error envelope assertions (
errors.test.js,tools.test.js,output-schemas.test.js) to assert the new contract:structuredContentabsent on errors, payload intact in text content. MCP suite 103/103 (8 live tests skipped as usual),standardlint green.Note
Low Risk
Single response-shaping change with broad test updates; success paths unchanged and error payloads remain in text content.
Overview
MCP tool errors no longer include
structuredContent, so strict clients that validate responses against each tool’s success-onlyoutputSchema({ data: ... }) still see the full actionable JSON (codes, hints, upgrade guidance) in text content instead of a schema failure masking it.The change is centralized in
toToolResponse: successes still returnstructuredContent: { data: ... }plus a text mirror; errors setisError: trueand put the same JSON payload only incontent[0].text.Tests across
errors.test.js,tools.test.js, andoutput-schemas.test.jsnow assertstructuredContentis absent on errors and read assertions from parsed text content.Reviewed by Cursor Bugbot for commit b77c021. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
structuredContent, preserving compatibility with clients that validate successful response schemas.Tests