-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: MCP UI not rendering due to CallToolResult structure change #6143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The tool result value changed from an array of Content to an object with a content property (CallToolResult from rmcp). This broke MCP UI rendering because the code expected toolResult.value to be an array directly. - Add typed interfaces for ToolResultData matching rmcp::model::CallToolResult - Create getToolResultContent helper that extracts content and filters by audience - Simplify toolResults assignment using the new helper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical bug where MCP UI resources were not rendering due to a structural change in tool results. The ToolResult<CallToolResult> structure changed from having value as a Content[] array directly to value being a CallToolResult object containing a content array field.
- Updated TypeScript interfaces to match the new
rmcp::model::CallToolResultstructure with camelCase serialization - Created a new
getToolResultContenthelper function that correctly extracts content from the nested structure and applies audience filtering - Simplified tool results assignment in two locations to use the new helper
|
This regression was introduced by #6074 which changed the tool result structure to preserve full CallToolResult metadata. |
DOsinga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice. think it can be simplified a bit
| content: Content[]; | ||
| structuredContent?: unknown; | ||
| isError?: boolean; | ||
| _meta?: unknown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should use the version we get from the api, not declare your own. I'm confused though since the API version doesn't seem to use camelCase?
| interface ToolResultError { | ||
| status: 'error'; | ||
| error: string; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't bother with these since your doing a bunch of casting anyway
|
|
||
| type ToolResultData = ToolResultSuccess | ToolResultError; | ||
|
|
||
| function getToolResultContent(toolResult: Record<string, unknown>): Content[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if you drop all the types, you can just go with
function getToolResultContent(toolResult: Record<string, unknown>): Content[] {
if (toolResult.status !== 'success') {
return [];
}
const value = toolResult.value as CallToolResponse;
return value.content.filter((item) => {
const annotations = (item as { annotations?: { audience?: string[] } }).annotations;
return !annotations?.audience || annotations.audience.includes('user');
});
}
| loadingStatus === 'success' && Array.isArray(toolResponse?.toolResult.value) | ||
| ? toolResponse!.toolResult.value.filter((item) => { | ||
| const audience = item.annotations?.audience as string[] | undefined; | ||
| return !audience || audience.includes('user'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good clean-up here
…interfaces Updated the getToolResultContent function to directly access the toolResult object, simplifying the logic. Removed unused TypeScript interfaces related to CallToolResultValue and ToolResultData, as they are no longer necessary with the new structure.
|
feedback addressed, thanks! |
| const annotations = (item as { annotations?: { audience?: string[] } }).annotations; | ||
| return !annotations?.audience || annotations.audience.includes('user'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is fine, but casting the value to CallToolResponse from the api would be even better and avoid this sort of problem in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
happy to address this, no prob!
Modified the ToolCallWithResponse component to import and utilize the CallToolResponse type for better type safety. Adjusted the getToolResultContent function to reflect this change, ensuring it correctly processes the tool result structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
* main: fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077)
* origin/main: (57 commits) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) perf: Avoid repeated MCP queries during streaming responses (#6138) Fix YAML serialization for recipes with special characters (#5796) Add more posthog analytics (privacy aware) (#6122) docs: add Sugar MCP server to extensions registry (#6077) ...
…icing * 'main' of github.com:block/goose: (35 commits) docs: skills (#6062) fix: add conditional configuration for GOOSE_BIN_DIR in PATH (#5940) Update dependencies to help in Fedora packaging (#5835) fix: make goose reviewer less bad (#6154) docs: create/edit recipe button (#6145) fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035) fix: we don't need to warn about tool count when in code mode (#6149) deps: upgrade agent-client-protocol to 0.9.0 (#6109) fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966) More slash commands (#5858) fix: MCP UI not rendering due to CallToolResult structure change (#6143) fix: display shell output as static text instead of spinner (#6041) fix : Custom providers with empty API keys show as configured in desktop (#6105) Add .agents/skills and ~/.config/agent/skills to skills discovery paths (#6139) fix: use instructions for system prompt and prompt for user message in subagents (#6121) Fix compaction loop for small models or large input (#5803) feat: Centralize theme management with ThemeContext (#6137) OpenRouter & Xai streaming (#5873) fix: resolve mcp-hermit cleanup path expansion issue (#5953) feat: add goose PR reviewer workflow (#6124) ...
Summary
MCP UI was not rendering on main because the tool result structure changed from an array to an object.
Problem
The
toolResult.valuechanged from:Content[](array directly){ content: Content[], structuredContent?, isError?, _meta? }(CallToolResult object)The existing code expected
toolResult.valueto be an array, sogetToolResultValuereturnednulland MCP UI resources were never rendered.This regression was introduced by #6074 which changed the tool result structure to preserve full CallToolResult metadata.
Solution
ToolResultDatamatchingrmcp::model::CallToolResult(camelCase serialization)getToolResultContenthelper that:contentarray from the new structuretoolResultsassignment using the new helperTesting
Tested with diagnostic session logs comparing 1.16 (working) vs main (broken) to verify the fix correctly extracts content from the new structure.