-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(mcp): improve mcp with McpExecution, make UX like terminal execute #3485
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
|
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.
Typo: The component name CodeAccordian is likely a misspelling. The standard spelling is CodeAccordion. Consider renaming it for consistency and clarity.
|
@LousyBook94 just push my pr, can you check this ? |
|
@samhvw8 woah that looks better than i imagined :] |
|
@samhvw8 you should probably link this pr to the issue too |
|
@LousyBook94 already linked you can check above pr description
thanks for your review, can you try it local ? @cte @sachasayan if you have time can you review it for me ? |
6a1191e to
f28846d
Compare
* protos report bug * changeset
78d4ee9 to
98b091b
Compare
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 noticed there are 4 separate for loops here. Should we consider combining these to optimize performance?
The MCP processing loops could potentially be merged, and we might be able to build the result array directly instead of creating intermediate arrays.
What do you think about refactoring this to use fewer loops?
|
Hey @samhvw8 this is pretty cool, I am curious about the UI, do you think we should keep these new styles or make the styles match our current ones for consistency? |
Ok let me do that |
yeah, thanks for help me to complete this PR <3 |
bfce0df to
86a95f0
Compare
9df793d to
53a90aa
Compare
daniel-lxs
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.
Hey @samhvw8, Thank you for fixing the test due to the new translations keys I added.
LGTM
- Add McpExecutionStatus type with discriminated union for execution states - Implement McpExecution component with live status updates and collapsible output - Replace inline MCP UI in ChatRow with dedicated McpExecution component - Add comprehensive test coverage for useMcpToolTool - Enhance CodeAccordian to support custom headers - Improve combineCommandSequences to handle MCP execution flows
- Reduced from 3 separate loops to 1 main processing loop - Improved time complexity from O(n²) to O(n) - Reduced total passes through array from 4 to 2 - Better memory access patterns and cache utilization
…lid JSON arguments
|
@daniel-lxs i just sync with latest change, can you check it again |
…name in approval section
daniel-lxs
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.


Implements a new UI component and status tracking system for MCP tool executions, providing users with feedback on tool execution progress and results.
This change improves the user experience by providing clear visual feedback during MCP tool execution, similar to the existing command execution experience.
Related GitHub Issue
Closes: #2145
Description
Test Procedure
Type of Change
srcor test files.Pre-Submission Checklist
npm run lint).console.log) has been removed.npm test).mainbranch.npm run changesetif this PR includes user-facing changes or dependency updates.Screenshots / Videos
MCP Execution Viewer - Watch Video
Documentation Updates
Additional Notes
Important
Enhance MCP tool execution feedback with new UI components, status tracking, and improved message handling.
McpExecutioncomponent inMcpExecution.tsxfor displaying MCP tool execution status and results.ChatRowinChatRow.tsxto useMcpExecutionfor better visualization.McpExecutionStatusschema inmcp.tswith states: started, output, completed, error.useMcpToolToolinuseMcpToolTool.tsto send execution status updates.combineCommandSequencesincombineCommandSequences.tsto combineuse_mcp_serverandmcp_server_responsemessages.useMcpToolToolinuseMcpToolTool.test.tsto cover parameter validation, partial requests, successful execution, and error handling.combineCommandSequencesincombineCommandSequences.test.tsto handle MCP server responses.CodeAccordianinCodeAccordian.tsxto support custom headers.This description was created by
for a98d119c7f4ec99671109b0d9caab741e684afab. You can customize this summary. It will automatically update as commits are pushed.