Update toast error message for MCP tool connection#1920
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
1 Key Finding | Risk: Low
💭 Consider (1) 💭
- 💭 Consider:
page.client.tsx:568Whether to include both valid user actions in error guidance
💡 APPROVE WITH SUGGESTIONS
Summary: This is a straightforward copy change to an error toast message. The change itself is valid — both the old and new suggested actions are things users can actually do in the UI. The only consideration is whether the new message should mention both options (removing existing connection OR connecting to a new node) for complete guidance. Feel free to merge as-is if the product intent is to steer users toward creating new MCP nodes.
Discarded (0)
No findings discarded.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Direct assessment | 1 | 0 | 0 | 0 | 1 | 0 | 0 |
| Total | 1 | 0 | 0 | 0 | 1 | 0 | 0 |
Note: Given the trivial scope (1-line copy change), full subagent dispatch was skipped in favor of direct orchestrator assessment.
| if (targetNode && targetNode.type === NodeType.MCP) { | ||
| if (edges.some((edge) => edge.target === targetNode.id)) { | ||
| toast.error('This MCP tool is already connected. Remove the existing connection first.'); | ||
| toast.error('This MCP tool is already connected. Connect to a new MCP server node.'); |
There was a problem hiding this comment.
💭 CONSIDER: Completeness of user guidance
Issue: Both actions are valid — users can either remove the existing connection OR connect to a new MCP server node. The new message drops the first option.
Why: The old message guided users to the existing edge editor delete functionality, while the new message only mentions creating a new node. Users who want to re-wire an existing MCP node (rather than add another one) might be confused about how to proceed.
Fix: Consider whether to include both options, or whether the UX intent is to deliberately steer users toward adding new nodes:
toast.error('This MCP tool is already connected. Remove the existing connection first, or connect to a new MCP server node.');Or keep the new message if the product intent is to encourage creating new MCP nodes over modifying existing connections.
2 participants
No description provided.