Skip to content

fix: Validate JSON parameters before tool execution #528

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.

Sign up for GitHub

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

Jump to bottom
Merged
olaservo merged 13 commits into from
Sep 28, 2025
Merged

fix: Validate JSON parameters before tool execution #528

olaservo merged 13 commits into from
Sep 28, 2025

Conversation

@nandsha
Copy link
Contributor

@nandsha nandsha commented Jun 18, 2025 •
edited
Loading

Summary

Motivation and Context

Prevents execution of tools with invalid JSON parameters by validating JSON syntax before tool execution and showing error UI for invalid input.

How Has This Been Tested?

Yes

mcp-inspector-fix-for-issue-498.mov

Breaking Changes

No breaking changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@nandsha nandsha changed the title Add JSON validation to tool execution fix: Validate JSON parameters before tool execution Jun 18, 2025
@nandsha nandsha marked this pull request as ready for review June 18, 2025 16:48
@nandsha
Add JSON validation to tool execution
d65ed01 
Prevents execution of tools with invalid JSON parameters by validating JSON
syntax before tool execution and showing error UI for invalid input.

- Add validateJson method to DynamicJsonForm via forwardRef
- Add validation check in ToolsTab before tool execution
- Reuse existing JsonEditor error display for consistent UX
@nandsha nandsha force-pushed the fix/validate-json-on-tool-execution branch from c524a16 to d65ed01 Compare June 26, 2025 13:17
@olaservo olaservo added the tools Issues related to testing tools label Jul 7, 2025
@olaservo
Copy link
Member

olaservo commented Jul 7, 2025

Hi @nandsha thanks for this PR! Just wanted to give a heads up: we recently merged in a fairly large set of changes to the JSON schema handling behind the scenes, so I'm taking a little time to go through regression scenarios from other past issues before merging in more large changes to tool input validation. Will dig into this one next.

nandsha added 2 commits July 9, 2025 23:38
@nandsha
Resolve merge conflict in DynamicJsonForm.tsx
20d9480 
Integrate JSON validation with copy JSON functionality:
- Maintain forwardRef structure for validation exposure
- Combine copy button with validation method
- Both features working together without conflicts
@nandsha
Merge branch 'main' into fix/validate-json-on-tool-execution
bd7c753
@nandsha
Copy link
Contributor Author

nandsha commented Jul 9, 2025 •
edited
Loading

Resolved merge conflicts and tested successfully. Thanks @olaservo

olaservo added 3 commits August 5, 2025 05:48
@olaservo
Resolve merge conflicts in DynamicJsonForm.tsx and ToolsTab.tsx
8ce1bbd 
- Merged JSON validation functionality from PR branch with enhanced form features from main
- Preserved validateJson method and DynamicJsonFormRef interface for tool validation
- Added support for oneOf schemas, enum dropdowns, and enhanced string field types
- Integrated meta display functionality and resource content props
- Maintained JSON validation before tool execution to prevent invalid data submission
@olaservo
Add tests
80a535e
@olaservo
Fix formatting
ad10912
@HQidea
Copy link

HQidea commented Sep 18, 2025

Any progress?

@nandsha
Merge branch 'main' into fix/validate-json-on-tool-execution
119922b
@nandsha
Add real-time JSON validation and prevent tool execution on invalid i…
8d149b2 
…nput
@nandsha
Fix ESLint issues in DynamicJsonForm.tsx
a3445c9 
- Remove unnecessary escape character in regex pattern
- Add missing schema dependency to useCallback hook
@nandsha
Add PR demo video showing JSON validation improvements
9903c83 
- Demonstrates real-time JSON validation feedback
- Shows tool execution disabled on invalid JSON
- Displays improved error messages and auto-reset functionality
- File optimized for GitHub (709KB MP4 format)
@nandsha
Copy link
Contributor Author

nandsha commented Sep 23, 2025 •
edited
Loading

Hi @olaservo ,

Sorry about the radio silence over the past couple of months.

I have now fetched the latest from upstream, resolved merge conflicts, and have further enhanced JSON Validation by adding real-time validation improvements that build on the existing JSON validation.

  • Tool execution button disables when validation errors are present
  • Shows descriptive JSON parsing errors instead of silent failures
  • Malformed JSON automatically resets to the result to default values to prevent confusion
  • This ensures users can't execute tools with invalid JSON and always receive clear validation feedback
  • The changes were tested against a fork of "everything" server, by adding a purpose-built echoJSON tool (Refer nandsha/servers@1d85b74)

Screen recording:

mcp-inspector-fix-for-issue-498-v2.mp4

Please review whenever you could next. Thanks!

olaservo
olaservo approved these changes Sep 28, 2025
View reviewed changes
Copy link
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this, and my bad for not reviewing it sooner!

@olaservo olaservo merged commit ce4ac9e into modelcontextprotocol:main Sep 28, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olaservo olaservo olaservo approved these changes

Assignees

No one assigned

Labels

tools Issues related to testing tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nandsha @olaservo @HQidea