Skip to content

Conversation

@dishaprakash
Copy link
Contributor

@dishaprakash dishaprakash commented Dec 18, 2025

This is PR no. 5 - Adding support for MCP protocol in core SDK
PR no.4 - #132
PR no.3 - #131
PR no.2 - #128
PR no.1 - #126

Key Changes

  • Protocol Definitions: Added Protocol type and constants
  • Functional Options: Implemented WithProtocol to allow configuring the client version, including validation to prevent duplicate assignment.
  • Factory Logic: Updated NewToolboxClient to instantiate the correct underlying transport (mcp20250618, mcp20250326, mcp20241105, toolbox) based on the configuration.
  • Struct Updates: Added protocol and transport fields to ToolboxClient to store state and configuration.
  • Testing:
    • Added unit tests for WithProtocol (verifying configuration and error handling).
    • Added unit tests for GetSupportedMcpVersions.
    • Added e2e_mcp_test.go to parameterize existing E2E tests across all supported MCP protocol versions using a table-driven approach.

@dishaprakash dishaprakash requested a review from a team as a code owner December 18, 2025 20:08
@twishabansal twishabansal added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Dec 24, 2025
@dishaprakash dishaprakash force-pushed the add-mcp-v20250618 branch 2 times, most recently from b9a60a8 to d439c55 Compare January 1, 2026 19:08
func TestE2E_ContextHandling(t *testing.T) {
newClient := func(t *testing.T) *core.ToolboxClient {
client, err := core.NewToolboxClient("http://localhost:5000")
client, err := getNewToolboxClient()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a breaking change for the users?

Copy link
Contributor Author

@dishaprakash dishaprakash Jan 6, 2026

Choose a reason for hiding this comment

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

Similar to Python, if users want to continue the usage of the native Toolbox REST protocol then yes. But we are defaulting to the latest MCP version.
Since this E2E test is for the Toolbox Protocol, we have to use the functional option to mention the protocol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants