-
Notifications
You must be signed in to change notification settings - Fork 248
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
Do not remove form on error in tool call tab. #241
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.
It is definitely worthwhile that the form doesn't get wiped out, so that you can try again 👍.
However, I question whether we need it to remain persistent. We have a toast that pops with the error, then goes away. The persistent one remains even when you subsequently make changes to the form.
Both displayed when the error happens.

Persistent error not removed after amending form

Maybe the answer is just to not display the error at all in this component and let the toast handle it? I suspect its presence in this component may be a holdover from pre-toast times, but not sure. But I don't see the need for it at all now.
Agree with you, I think that leaving the toast without the Alert is just fine :) |
This reverts commit c48670f.
Updated! @cliffhall |
@NicolasMontone Doh, but you need to run Prettier to get past the build. |
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.
LGTM! 👍
Now if you have an error in you MCP tool call, do not remove the form
Before:
After:
Screen.Recording.2025-04-01.at.10.55.34.mov
Motivation and Context
Before, on each error, you need to select another tool and then come back to the same one, now, you can see the error and continue testing.
How Has This Been Tested?
Yep, I'm building my own MCP, and I made the change to continue my integration, I tested it with another MCP.
Breaking Changes
Nope
Types of changes
Checklist
Additional context
Nope