Skip to content

Refactor MCP list methods via shared generic SDK pagination adapter#6997

Merged
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-mcp-list-methods
Jun 4, 2026
Merged

Refactor MCP list methods via shared generic SDK pagination adapter#6997
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-mcp-list-methods

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 4, 2026

listTools, listResources, and listPrompts had the same cursor-pagination closure structure repeated with only SDK types changed. This PR removes that structural duplication while preserving existing list behavior and result shapes.

  • Core refactor

    • Added listSDKItems in internal/mcp/pagination.go to adapt SDK list responses into paginatedPage and delegate to listMCPItems.
    • Keeps pagination/session/response marshalling logic centralized in one path.
  • Connection method simplification

    • Updated internal/mcp/connection_methods.go:
      • listTools
      • listResources
      • listPrompts
    • Each method now provides typed adapters (list, toPage, buildResult) instead of duplicating the full closure pattern.
  • Coverage update

    • Added TestListSDKItems_NilSession in internal/mcp/pagination_test.go to verify session precondition failure short-circuits before invoking the SDK list callback.
return listSDKItems(c, "tools",
	func(cursor string) (*sdk.ListToolsResult, error) {
		return c.getSDKSession().ListTools(c.ctx, &sdk.ListToolsParams{Cursor: cursor})
	},
	func(result *sdk.ListToolsResult) paginatedPage[*sdk.Tool] {
		return paginatedPage[*sdk.Tool]{Items: result.Tools, NextCursor: result.NextCursor}
	},
	func(items []*sdk.Tool) *sdk.ListToolsResult {
		return &sdk.ListToolsResult{Tools: items}
	},
)

Copilot AI changed the title [WIP] Refactor MCP list methods to eliminate structural duplication Refactor MCP list methods via shared generic SDK pagination adapter Jun 4, 2026
Copilot finished work on behalf of lpcox June 4, 2026 16:16
Copilot AI requested a review from lpcox June 4, 2026 16:16
@lpcox lpcox marked this pull request as ready for review June 4, 2026 16:44
Copilot AI review requested due to automatic review settings June 4, 2026 16:45
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 4, 2026

@copilot fix the failing lint ci check

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors MCP cursor-based list operations to remove duplicated pagination glue across listTools, listResources, and listPrompts, while preserving response shapes and centralizing pagination/session validation in the existing listMCPItems flow.

Changes:

  • Added a new generic adapter listSDKItems to bridge SDK list responses into the existing listMCPItems pagination/marshalling pipeline.
  • Simplified listTools, listResources, and listPrompts to provide small typed adapters (list, toPage, buildResult) instead of repeating the full pagination closure pattern.
  • Added a unit test to verify listSDKItems short-circuits on missing session before invoking the SDK list callback.
Show a summary per file
File Description
internal/mcp/pagination.go Introduces listSDKItems as a thin generic adapter on top of listMCPItems.
internal/mcp/pagination_test.go Adds coverage ensuring session precondition failure prevents invoking the provided SDK list function.
internal/mcp/connection_methods.go Refactors listTools/listResources/listPrompts to use listSDKItems and remove duplicated pagination boilerplate.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 4, 2026

@copilot fix the failing lint ci check

Fixed in the latest commit. The lint failure was S1016 in pagination_test.go:339 — staticcheck flagged the struct literal used to copy fakeListResult into paginatedPage[fakeItem]. Since the two types have identical field layouts, it's replaced with a direct type conversion: paginatedPage[fakeItem](result).

Copilot finished work on behalf of lpcox June 4, 2026 16:51
@lpcox lpcox merged commit 3d51185 into main Jun 4, 2026
16 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-mcp-list-methods branch June 4, 2026 17:01
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.

[duplicate-code] Duplicate Code Pattern: MCP List Method Structural Duplication

3 participants