Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 26, 2025

MCP server tool handlers returned generic errors wrapped in mcp.CallToolResult. This prevented MCP clients from distinguishing between parameter validation errors and internal failures.

Changes

Error Infrastructure

  • Added mcpErrorData() helper to marshal error context to json.RawMessage
  • Replaced CallToolResult error returns with jsonrpc.Error using standard codes

Error Classification

  • InvalidParams (-32602): Missing required parameters, conflicting flags, invalid jq filters
  • InternalError (-32603): Command execution failures, Docker preparation, context cancellation

Error Context

  • Structured Data field includes error details, command output, and user-provided input for debugging
  • Error messages are clear and actionable
  • Context includes error messages, output, filter expressions, and run IDs

Example

Before:

return &mcp.CallToolResult{
    Content: []mcp.Content{
        &mcp.TextContent{Text: fmt.Sprintf("Error: %v\nOutput: %s", err, output)},
    },
}, nil, nil

After:

return nil, nil, &jsonrpc.Error{
    Code:    jsonrpc.CodeInternalError,
    Message: "failed to compile workflows",
    Data:    mcpErrorData(map[string]any{"error": err.Error(), "output": string(output)}),
}

Parameter validation with context:

return nil, nil, &jsonrpc.Error{
    Code:    jsonrpc.CodeInvalidParams,
    Message: "invalid jq filter expression",
    Data:    mcpErrorData(map[string]any{"error": jqErr.Error(), "filter": args.JqFilter}),
}

Testing

Added mcp_server_error_codes_test.go with integration tests covering:

  • Parameter validation failures
  • Internal execution failures
  • Error message propagation through MCP SDK
Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Implement standardized MCP error codes</issue_title>
<issue_description>## Objective

Update tool error handling to use standardized MCP error codes from v1.2.0's jsonrpc.Error constants for better client compatibility.

Context

v1.2.0 exposes standardized error codes via jsonrpc.Error sentinels. Currently, tools return generic errors. Using standardized codes enables MCP clients to handle errors more intelligently.

Approach

  1. Review current error handling in tool handlers in pkg/cli/mcp_server.go
  2. Identify error types (invalid params, internal error, method not found, etc.)
  3. Replace generic errors with jsonrpc.Error constants:
    // Example
    return nil, jsonrpc.NewError(jsonrpc.InvalidParams, "workflow not found", nil)
  4. Update error messages to be clear and actionable
  5. Add tests for error scenarios

Files to Modify

  • pkg/cli/mcp_server.go - Update error returns in tool handlers
  • pkg/cli/mcp_server_test.go - Add tests for error codes

Acceptance Criteria

AI generated by Plan Command for discussion #7710

Comments on the Issue (you are @copilot in this section)


💡 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.

Copilot AI changed the title [WIP] Update tool error handling to use standardized MCP error codes Implement standardized MCP error codes using jsonrpc.Error Dec 26, 2025
Copilot AI requested a review from mnkiefer December 26, 2025 14:21
return nil, nil, &jsonrpc.Error{
Code: jsonrpc.CodeInternalError,
Message: "request cancelled",
Data: mcpErrorData(ctx.Err().Error()),
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot you cannot leak internal error data for security reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7c8a907. Removed internal error data from all error responses:

  • No command output exposed
  • No internal error messages exposed
  • Only user-provided input (jq filters, run_id) included in Data field

All error code tests still passing.

- Remove command output from error Data fields
- Remove internal error messages from error Data fields
- Keep only user-provided input (jq filters, run_id) in Data
- Prevents leaking sensitive information like tokens, paths, system details

Co-authored-by: pelikhan <[email protected]>
@pelikhan
Copy link
Contributor

@copilot revert last commit

…ity"

This reverts commit 7c8a907.

Restoring internal error data in MCP error responses as requested.

Co-authored-by: pelikhan <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 26, 2025

@copilot revert last commit

Reverted in 754db11. The security fix has been undone and internal error data (command output, error messages) is now included in error responses again.

@pelikhan pelikhan marked this pull request as ready for review December 26, 2025 16:00
@pelikhan pelikhan merged commit eb7c188 into main Dec 26, 2025
4 checks passed
@pelikhan pelikhan deleted the copilot/implement-standardized-mcp-errors branch December 26, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[plan] Implement standardized MCP error codes

3 participants