fix(prompts): Allow MCP prompts with no arguments #3018
Conversation
…dummy args - Fix argument processing logic in tool_manager.rs to handle prompts with optional/no arguments - Add validation for required arguments before sending to MCP server - Add MissingRequiredArguments error type for better error messages - Update error handling in prompts.rs to display helpful messages - Add comprehensive tests to verify the fix works correctly Fixes issue where prompts like 'simple-greeting' with no required arguments would fail with 'Missing required arguments' error when called as @simple-greeting without any parameters. 🤖 Assisted by Amazon Q Developer
|
Thanks for doing this, James. I have put up a PR for fixing tests that were invalidated from type changes that came with the upgrade of rmcp: #3037 |
|
I just pulled latest main and I still see the bug: Latest main branch: With my change: |
| // Validate required arguments before processing | ||
| if let Some(schema) = &prompt_get.arguments { | ||
| let required_args: Vec<_> = schema.iter().filter(|arg| arg.required == Some(true)).collect(); | ||
|
|
||
| let provided_count = arguments.as_ref().map_or(0, |args| args.len()); | ||
|
|
||
| if !required_args.is_empty() && provided_count < required_args.len() { | ||
| return Err(GetPromptError::MissingRequiredArguments { | ||
| prompt_name: prompt_name.clone(), | ||
| required_args: required_args.iter().map(|arg| arg.name.clone()).collect(), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
We don't need this. We should just let the error happen where the source of truth is, the servers.
| } | ||
|
|
||
| #[test] | ||
| fn test_argument_processing_logic() { |
There was a problem hiding this comment.
These tests are not really testing anything in the code path introduced.
If we want to validate that the logic introduced to derive the arguments, we might want to put them in a function and #[inline] them.
- Remove client-side validation logic per reviewer feedback - Extract argument processing logic to inline function for better testability - Replace integration tests with focused unit tests for process_prompt_arguments - Let MCP servers handle their own argument validation as source of truth 🤖 Assisted by Amazon Q Developer
Issue #, if available: n/a
Description of changes:
Allow MCP prompts with no arguments
Fixes issue where prompts like 'simple-greeting' with no required arguments would fail with 'Missing required arguments' error when called as @simple-greeting without any parameters.
🤖 Assisted by Amazon Q Developer
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.