From b557015b88bb9137fc304e144923520bfdefd7df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 23:47:12 +0000 Subject: [PATCH 1/3] Initial plan From d19198b03ebf35e766b0e30d30e8d01c789dffe0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 23:51:28 +0000 Subject: [PATCH 2/3] fix pagination cursor cycle handling and add sdk canary coverage Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4d4e8792-d37a-4028-8eaa-15752f44b0d7 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/mcp/connection.go | 5 +++++ internal/mcp/connection_test.go | 31 +++++++++++++++++++++++++++++-- internal/mcp/http_transport.go | 6 +++++- internal/server/routed_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/internal/mcp/connection.go b/internal/mcp/connection.go index d1871aa5..74efaf8f 100644 --- a/internal/mcp/connection.go +++ b/internal/mcp/connection.go @@ -610,10 +610,15 @@ func paginateAll[T any]( logConn.Printf("list%s: received page of %d %s from serverID=%s", itemKind, len(first.Items), itemKind, serverID) cursor := first.NextCursor + seenCursors := make(map[string]struct{}) for pageCount := 1; cursor != ""; pageCount++ { if pageCount >= paginateAllMaxPages { return nil, fmt.Errorf("list%s: backend serverID=%s returned more than %d pages; aborting to prevent unbounded memory growth", itemKind, serverID, paginateAllMaxPages) } + if _, seen := seenCursors[cursor]; seen { + return nil, fmt.Errorf("list%s: backend serverID=%s returned cyclical cursor %q", itemKind, serverID, cursor) + } + seenCursors[cursor] = struct{}{} page, err := fetch(cursor) if err != nil { return nil, err diff --git a/internal/mcp/connection_test.go b/internal/mcp/connection_test.go index 31c671d1..5922ab84 100644 --- a/internal/mcp/connection_test.go +++ b/internal/mcp/connection_test.go @@ -978,11 +978,15 @@ func TestPaginateAll(t *testing.T) { }) t.Run("exceeding max pages returns error", func(t *testing.T) { - // Each call returns a cursor so the loop never ends naturally. + // Each call returns a unique cursor so the loop never ends naturally. callCount := 0 _, err := paginateAll("server1", "tools", func(cursor string) (paginatedPage[string], error) { callCount++ - return paginatedPage[string]{Items: []string{"x"}, NextCursor: "next"}, nil + nextCursor := cursor + "next" + if nextCursor == "" { + nextCursor = "next" + } + return paginatedPage[string]{Items: []string{"x"}, NextCursor: nextCursor}, nil }) require.Error(t, err) assert.Contains(t, err.Error(), "more than") @@ -990,4 +994,27 @@ func TestPaginateAll(t *testing.T) { // Must stop at the page limit, not run forever. assert.Equal(t, paginateAllMaxPages, callCount) }) + + t.Run("cyclical cursor returns error", func(t *testing.T) { + callCount := 0 + _, err := paginateAll("server1", "tools", func(cursor string) (paginatedPage[string], error) { + callCount++ + switch cursor { + case "": + return paginatedPage[string]{Items: []string{"a"}, NextCursor: "page2"}, nil + case "page2": + return paginatedPage[string]{Items: []string{"b"}, NextCursor: "page3"}, nil + case "page3": + return paginatedPage[string]{Items: []string{"c"}, NextCursor: "page2"}, nil + default: + return paginatedPage[string]{Items: nil, NextCursor: ""}, nil + } + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "cyclical cursor") + assert.Contains(t, err.Error(), "page2") + // Initial page + 2 unique cursor fetches, then cycle detected before another fetch. + assert.Equal(t, 3, callCount) + }) } diff --git a/internal/mcp/http_transport.go b/internal/mcp/http_transport.go index 68a6ebc3..4d0ba07a 100644 --- a/internal/mcp/http_transport.go +++ b/internal/mcp/http_transport.go @@ -33,7 +33,9 @@ const ( HTTPTransportPlainJSON HTTPTransportType = "plain-json" ) -// MCPProtocolVersion is the MCP protocol version used in initialization requests. +// MCPProtocolVersion is the MCP protocol version used only by the plain JSON-RPC +// fallback path in this package. Streamable and SSE transports are SDK-managed +// and negotiate protocol versions internally. const MCPProtocolVersion = "2025-11-25" // requestIDCounter is used to generate unique request IDs for HTTP requests @@ -78,6 +80,8 @@ func isSessionNotFoundError(err error) bool { if errors.Is(err, sdk.ErrSessionMissing) { return true } + // Plain JSON-RPC fallback requests bypass SDK session types, so they cannot + // return sdk.ErrSessionMissing and are matched by backend error text instead. return strings.Contains(strings.ToLower(err.Error()), "session not found") } diff --git a/internal/server/routed_test.go b/internal/server/routed_test.go index c7a7ad4f..0fb826f4 100644 --- a/internal/server/routed_test.go +++ b/internal/server/routed_test.go @@ -703,6 +703,24 @@ func TestRegisterToolWithoutValidation(t *testing.T) { }, }, handler) + // This canary verifies the key behavior relied on by registerToolWithoutValidation: + // tool calls are not rejected by SDK argument-value validation. + var strictHandlerCalled bool + registerToolWithoutValidation(server, &sdk.Tool{ + Name: "strict_tool", + Description: "A strict-schema tool", + InputSchema: map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "count": map[string]interface{}{"type": "integer"}, + }, + "required": []interface{}{"count"}, + }, + }, func(ctx context.Context, req *sdk.CallToolRequest, state interface{}) (*sdk.CallToolResult, interface{}, error) { + strictHandlerCalled = true + return &sdk.CallToolResult{IsError: false}, nil, nil + }) + // Use in-memory transports to connect a client to the server and invoke the tool serverTransport, clientTransport := sdk.NewInMemoryTransports() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -721,6 +739,16 @@ func TestRegisterToolWithoutValidation(t *testing.T) { require.NoError(err) assert.False(result.IsError) assert.True(handlerCalled, "Handler should have been called") + + // Provide an intentionally invalid value for the strict schema ("count" must be integer). + // If SDK starts validating argument values on this registration path, this call will fail. + strictResult, err := clientSession.CallTool(ctx, &sdk.CallToolParams{ + Name: "strict_tool", + Arguments: map[string]interface{}{"count": "not-an-integer"}, + }) + require.NoError(err) + assert.False(strictResult.IsError) + assert.True(strictHandlerCalled, "Strict handler should be called even with schema-invalid arguments") } // TestCreateHTTPServerForRoutedMode_OAuth tests OAuth discovery endpoint in routed mode From 798dc9ae0b7d3d217797dda670baf28fc5407894 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 23:54:29 +0000 Subject: [PATCH 3/3] test: address validation feedback on go-sdk canary changes Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/4d4e8792-d37a-4028-8eaa-15752f44b0d7 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/mcp/connection_test.go | 6 +++--- internal/server/routed_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mcp/connection_test.go b/internal/mcp/connection_test.go index 5922ab84..5b4fa8dc 100644 --- a/internal/mcp/connection_test.go +++ b/internal/mcp/connection_test.go @@ -982,9 +982,9 @@ func TestPaginateAll(t *testing.T) { callCount := 0 _, err := paginateAll("server1", "tools", func(cursor string) (paginatedPage[string], error) { callCount++ - nextCursor := cursor + "next" - if nextCursor == "" { - nextCursor = "next" + nextCursor := "next" + if cursor != "" { + nextCursor = cursor + "next" } return paginatedPage[string]{Items: []string{"x"}, NextCursor: nextCursor}, nil }) diff --git a/internal/server/routed_test.go b/internal/server/routed_test.go index 0fb826f4..ed28bd74 100644 --- a/internal/server/routed_test.go +++ b/internal/server/routed_test.go @@ -718,7 +718,7 @@ func TestRegisterToolWithoutValidation(t *testing.T) { }, }, func(ctx context.Context, req *sdk.CallToolRequest, state interface{}) (*sdk.CallToolResult, interface{}, error) { strictHandlerCalled = true - return &sdk.CallToolResult{IsError: false}, nil, nil + return &sdk.CallToolResult{IsError: false}, state, nil }) // Use in-memory transports to connect a client to the server and invoke the tool