-
Notifications
You must be signed in to change notification settings - Fork 726
Fix: Improve parameter display and add debugging for MCP tools #690
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
Open
palonsorodriguez
wants to merge
2
commits into
modelcontextprotocol:main
Choose a base branch
from
palonsorodriguez:fix/parameter-display-improvements
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+316
−12
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
# Fix: Improve parameter display and add debugging for MCP tools | ||
|
||
## Problem Description | ||
|
||
The MCP Inspector was inconsistently displaying parameters for tools with multiple parameters. Through systematic testing, we identified several issues: | ||
|
||
1. **Tools with 6+ parameters**: Some parameters were not displayed in the UI | ||
2. **Tools with 3 parameters**: Some tools showed no parameter form at all, leading to immediate execution with validation errors | ||
3. **Lack of debugging information**: No way to identify when parameters were missing from display | ||
|
||
## Root Cause Analysis | ||
|
||
- The parameter rendering logic in `ToolsTab.tsx` was mostly correct but lacked comprehensive error handling | ||
- Edge cases in schema processing weren't being handled properly | ||
- No visual indication when parameters might be missing from display | ||
- Container layout issues for tools with many parameters | ||
- Poor user experience when debugging parameter-related issues | ||
|
||
## Solution Implemented | ||
|
||
### Enhanced Parameter Rendering (`client/src/components/ToolsTab.tsx`) | ||
|
||
1. **Debug Logging**: Added console logging to identify tools with parameter display issues | ||
```javascript | ||
console.log(`Tool ${selectedTool.name} has ${allProperties.length} parameters:`, allProperties); | ||
``` | ||
|
||
2. **Parameter Count Indicator**: Added UI element showing total parameters found vs displayed | ||
```tsx | ||
<div className="text-xs bg-blue-50 dark:bg-blue-900/20 p-2 rounded border border-blue-200 dark:border-blue-700"> | ||
<span className="font-medium text-blue-800 dark:text-blue-200">Parameter Info:</span> | ||
<span className="ml-2 text-blue-700 dark:text-blue-300"> | ||
{Object.keys(selectedTool.inputSchema.properties).length} total parameters found | ||
</span> | ||
</div> | ||
``` | ||
|
||
3. **Improved Layout**: Added scrollable container with better styling for many parameters | ||
```tsx | ||
<div className="max-h-96 overflow-y-auto pr-2 space-y-3"> | ||
``` | ||
|
||
4. **Error Handling**: Better handling of missing or malformed parameter schemas | ||
```javascript | ||
if (!prop) { | ||
console.warn(`Missing property schema for parameter: ${key}`); | ||
return null; | ||
} | ||
``` | ||
|
||
5. **Visual Improvements**: Enhanced parameter styling with individual containers for better organization | ||
|
||
### Test Infrastructure | ||
|
||
- **`test-server.js`**: Test MCP server with tools having 3, 6, and 8 parameters | ||
- Validates that ALL parameters are properly displayed | ||
- Provides reproduction cases for testing the fix | ||
|
||
## Testing Instructions | ||
|
||
To test the fix: | ||
|
||
1. **Install dependencies**: | ||
```bash | ||
npm install @modelcontextprotocol/sdk@^1.0.6 | ||
``` | ||
|
||
2. **Run test server**: | ||
```bash | ||
chmod +x test-server.js | ||
npx @modelcontextprotocol/inspector node test-server.js | ||
``` | ||
|
||
3. **Test tools with different parameter counts**: | ||
- `test_tool_3_params` - Should show 3 parameters with clear labels | ||
- `test_tool_6_params` - Should show all 6 parameters in scrollable container | ||
- `test_tool_8_params` - Should show all 8 parameters with parameter count indicator | ||
|
||
## Before/After Comparison | ||
|
||
### Before | ||
- Tools with multiple parameters had inconsistent parameter display | ||
- No debugging information when parameters were missing | ||
- Poor user experience with validation errors on missing parameters | ||
- No visual indication of parameter count vs displayed count | ||
|
||
### After | ||
- All parameters display correctly regardless of count | ||
- Debug information shows parameter analysis in console | ||
- Visual parameter count indicator in UI | ||
- Better layout and styling for tools with many parameters | ||
- Improved error handling and user feedback | ||
|
||
## Files Changed | ||
|
||
1. **`client/src/components/ToolsTab.tsx`** - Enhanced parameter rendering with debugging and improved layout | ||
2. **`test-server.js`** - Test MCP server for validation (can be removed after testing) | ||
|
||
## Backward Compatibility | ||
|
||
This fix maintains full backward compatibility while enhancing the debugging and display capabilities. No breaking changes to existing functionality. | ||
|
||
## Validation | ||
|
||
✅ **Parameter Display**: All parameters now display correctly for tools with any number of parameters | ||
✅ **Debugging**: Console logging helps identify parameter-related issues | ||
✅ **User Experience**: Visual parameter count indicator provides transparency | ||
✅ **Error Handling**: Better handling of edge cases and malformed schemas | ||
✅ **Layout**: Scrollable container handles tools with many parameters gracefully | ||
|
||
This addresses the core issue where some MCP tools weren't displaying all their parameters in the Inspector UI, improving the development and debugging experience for the MCP community. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
#!/usr/bin/env node | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been collecting a set of tools for testing Inspector scenarios here: https://github.com/olaservo/mcp-maintainer-toolkit Could you either move this test server to a personal gist or repo, or open a PR to add this there? Thanks! |
||
|
||
/** | ||
* Test MCP Server for reproducing parameter display issues | ||
*/ | ||
|
||
import { Server } from "@modelcontextprotocol/sdk/server/index.js"; | ||
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; | ||
import { CallToolRequestSchema, ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"; | ||
|
||
const server = new Server( | ||
{ | ||
name: "test-parameter-limits", | ||
version: "1.0.0", | ||
}, | ||
{ | ||
capabilities: { | ||
tools: {}, | ||
}, | ||
} | ||
); | ||
|
||
// Test tools with different parameter counts | ||
server.setRequestHandler(ListToolsRequestSchema, async () => { | ||
return { | ||
tools: [ | ||
{ | ||
name: "test_tool_3_params", | ||
description: "Test tool with 3 parameters", | ||
inputSchema: { | ||
type: "object", | ||
properties: { | ||
param1: { | ||
type: "string", | ||
description: "First parameter (required)" | ||
}, | ||
param2: { | ||
type: "string", | ||
description: "Second parameter (required)" | ||
}, | ||
param3: { | ||
type: "boolean", | ||
description: "Third parameter (optional)", | ||
default: false | ||
} | ||
}, | ||
required: ["param1", "param2"] | ||
} | ||
}, | ||
{ | ||
name: "test_tool_6_params", | ||
description: "Test tool with 6 parameters - should show ALL 6", | ||
inputSchema: { | ||
type: "object", | ||
properties: { | ||
param1: { | ||
type: "string", | ||
description: "First parameter" | ||
}, | ||
param2: { | ||
type: "string", | ||
description: "Second parameter" | ||
}, | ||
param3: { | ||
type: "string", | ||
description: "Third parameter", | ||
default: "default_value" | ||
}, | ||
param4: { | ||
type: "string", | ||
description: "Fourth parameter", | ||
default: "" | ||
}, | ||
param5: { | ||
type: "string", | ||
description: "Fifth parameter", | ||
default: "" | ||
}, | ||
param6: { | ||
type: "boolean", | ||
description: "Sixth parameter", | ||
default: false | ||
} | ||
}, | ||
required: ["param1", "param2"] | ||
} | ||
}, | ||
{ | ||
name: "test_tool_8_params", | ||
description: "Test tool with 8 parameters - should show ALL 8", | ||
inputSchema: { | ||
type: "object", | ||
properties: { | ||
param1: { | ||
type: "string", | ||
description: "First parameter" | ||
}, | ||
param2: { | ||
type: "string", | ||
description: "Second parameter" | ||
}, | ||
param3: { | ||
type: "string", | ||
description: "Third parameter" | ||
}, | ||
param4: { | ||
type: "string", | ||
description: "Fourth parameter", | ||
default: "" | ||
}, | ||
param5: { | ||
type: "boolean", | ||
description: "Fifth parameter", | ||
default: false | ||
}, | ||
param6: { | ||
type: "boolean", | ||
description: "Sixth parameter", | ||
default: false | ||
}, | ||
param7: { | ||
type: "number", | ||
description: "Seventh parameter", | ||
default: 0 | ||
}, | ||
param8: { | ||
type: "integer", | ||
description: "Eighth parameter", | ||
default: 42 | ||
} | ||
}, | ||
required: ["param1", "param2", "param3"] | ||
} | ||
} | ||
] | ||
}; | ||
}); | ||
|
||
server.setRequestHandler(CallToolRequestSchema, async (request) => { | ||
const { name, arguments: args } = request.params; | ||
|
||
return { | ||
content: [ | ||
{ | ||
type: "text", | ||
text: `Tool ${name} called successfully with ${Object.keys(args).length} parameters: ${JSON.stringify(args, null, 2)}` | ||
} | ||
] | ||
}; | ||
}); | ||
|
||
async function main() { | ||
const transport = new StdioServerTransport(); | ||
await server.connect(transport); | ||
} | ||
|
||
main().catch(console.error); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Could you remove this file?