Prevent MCP tool errors from exposing internals#6607
Merged
IMax153 merged 2 commits intoJul 25, 2026
Conversation
🦋 Changeset detectedLatest commit: b86a941 The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMCP toolkit invocation now converts validation, declared, AI, and defect failures into controlled tool error responses. Tests add reusable client and toolkit fixtures covering these outcomes, optional parameters, unknown tools, and suppression of internal error details. A patch changeset documents the behavior. ChangesMCP tool error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant ToolHandler
MCPClient->>MCPServer: Call toolkit tool
MCPServer->>ToolHandler: Execute with decoded arguments
ToolHandler-->>MCPServer: Success or classified failure
MCPServer-->>MCPClient: CallToolResult with structured or controlled error text
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/effect/src/unstable/ai/McpServer.ts`:
- Around line 748-756: Move Effect.tapCause(Effect.log) ahead of
Effect.catchCause in the tool execution effect chain, while keeping the existing
catch handlers and toolErrorResult behavior unchanged. This ensures all causes,
including defects and interruptions, are logged before catchCause converts
failures into successes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 432aa03f-f74c-4fa9-b5b4-d66750a46785
📒 Files selected for processing (3)
.changeset/quiet-tools-smile.mdpackages/effect/src/unstable/ai/McpServer.tspackages/effect/test/unstable/ai/McpServer.test.ts
github-project-automation
Bot
moved this from Discussion Ongoing
to Waiting on Author
in PR Backlog
Contributor
Bundle Size Analysis
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type
Description
Summary
McpServer.registerToolkitpreviously converted every failed tool execution into an MCPCallToolResultwhose text wasCause.pretty(cause). That diagnostic renderer includes stack traces, nested Cause details, Effect implementation frames, and absolute source paths, so server internals crossed the MCP protocol boundary.This change sanitizes registered-tool failure responses while preserving the validation feedback a MCP client needs to correct invalid arguments and retry.
Why
Tool-originated errors belong in
CallToolResultso an LLM can observe the failure, correct its arguments, and retry. Server diagnostic representations do not belong in that result. In particular,Cause.prettyis suitable for logs and debugging, not client-visible protocol payloads.For an optional string field, passing
nullshould continue to produce actionable feedback such as:It should not append an Effect error type, stack trace, nested
[cause]rendering, or local source paths.Behavior
CallToolResultToolParameterValidationErrorisError: truewithreason.messageErrorisError: truewithError.messageAiErrorisError: truewith a generic internal-error messageisError: truewith a generic internal-error messageisError: truewith a generic internal-error messageRelated
Original Effect-smol PR: Effect-TS/effect-smol#2573
Summary by CodeRabbit