Pre-validate workflow names in MCP logs tool#15221
Conversation
- Add validateWorkflowName() function in mcp_server.go - Validate workflow name before executing logs command - Return structured MCP error with suggestions on invalid workflow - Add unit test TestMCPValidateWorkflowName - Import sliceutil package for workflow name checking Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Change suggestion from "Run 'gh aw status'" to "Use the 'status' tool" to reference the MCP status tool available in the server. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves error handling in the MCP server's logs tool by adding pre-validation for workflow names, providing user-friendly error messages with actionable suggestions instead of generic CLI exit errors when invalid workflow names are provided.
Changes:
- Added
validateWorkflowName()function to check workflow existence before command execution - Integrated validation into the logs tool handler with structured error responses
- Added comprehensive unit tests covering valid and invalid workflow name scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cli/mcp_server.go | Implements workflow name validation logic and integrates it into the logs tool handler |
| pkg/cli/mcp_server_workflow_validation_test.go | Adds unit tests for workflow name validation covering empty names, non-existent workflows, and error message content |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mcpLog.Printf("Validating workflow name: %s", workflowName) | ||
|
|
There was a problem hiding this comment.
Consider using a debug-level log or removing this log statement. This validation is called for every logs tool invocation and will create noise in production logs.
| mcpLog.Printf("Validating workflow name: %s", workflowName) |
| resolvedName, err := workflow.ResolveWorkflowName(workflowName) | ||
| if err == nil { | ||
| mcpLog.Printf("Workflow name resolved successfully: %s -> %s", workflowName, resolvedName) |
There was a problem hiding this comment.
The resolved workflow name is logged but not used. Either remove the log statement on line 180 or use the resolvedName for further validation if needed.
| resolvedName, err := workflow.ResolveWorkflowName(workflowName) | |
| if err == nil { | |
| mcpLog.Printf("Workflow name resolved successfully: %s -> %s", workflowName, resolvedName) | |
| if _, err := workflow.ResolveWorkflowName(workflowName); err == nil { | |
| mcpLog.Printf("Workflow name resolved successfully: %s", workflowName) |
Summary: Improve error messaging for non-existent workflows in logs command ✅
Adds workflow name validation to the MCP server's logs tool to provide user-friendly error messages with suggestions when an invalid workflow name is provided, instead of generic CLI exit errors.
Changes
1. Added
validateWorkflowName()function inpkg/cli/mcp_server.goworkflow.ResolveWorkflowName()getAgenticWorkflowNames()suggestWorkflowNames()2. Integrated validation in logs tool handler
InvalidParamscode on validation failureworkflow_nameanderror_type3. Added comprehensive unit tests
TestMCPValidateWorkflowNametests 4 scenarios:Error Message Improvement
❌ Before: Generic MCP error with exit status 1
✅ After: Structured error with actionable suggestions
Benefits
Testing & Validation
Files Changed
pkg/cli/mcp_server.go(+57 lines)pkg/cli/mcp_server_workflow_validation_test.go(+57 lines)Total: 114 lines added across 2 files
Next Steps
This pattern can be applied to the audit command as mentioned in the issue for consistent error messaging across all MCP tools.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.