-
Notifications
You must be signed in to change notification settings - Fork 13
Description
Part of duplicate code analysis: #1388
Summary
In internal/mcp/connection.go, three SDK list-operation wrapper methods (listTools, listResources, listPrompts) are structurally identical: each calls requireSession(), invokes the corresponding SDK list method with an empty params struct, and calls marshalToResponse(result). This 7-line pattern is repeated three times.
Duplication Details
Pattern: SDK no-param list method wrappers
- Severity: Low
- Occurrences: 3 instances
- Location:
internal/mcp/connection.go(lines 377–459)
// listTools (lines 377–387)
func (c *Connection) listTools() (*Response, error) {
if err := c.requireSession(); err != nil {
return nil, err
}
result, err := c.session.ListTools(c.ctx, &sdk.ListToolsParams{})
if err != nil {
return nil, err
}
return marshalToResponse(result)
}
// listResources (lines 417–427) — structurally identical
func (c *Connection) listResources() (*Response, error) {
if err := c.requireSession(); err != nil {
return nil, err
}
result, err := c.session.ListResources(c.ctx, &sdk.ListResourcesParams{})
if err != nil {
return nil, err
}
return marshalToResponse(result)
}
// listPrompts (lines 450–459) — structurally identical
func (c *Connection) listPrompts() (*Response, error) {
if err := c.requireSession(); err != nil {
return nil, err
}
result, err := c.session.ListPrompts(c.ctx, &sdk.ListPromptsParams{})
if err != nil {
return nil, err
}
return marshalToResponse(result)
}All three functions follow the exact same structure; only the SDK method name and params type differ.
Impact Analysis
- Maintainability: Low risk currently. The pattern is simple and the functions are short. However, if session handling or result marshaling changes, all three functions need to be updated consistently.
- Bug Risk: Low — the functions are short enough that drift is unlikely.
- Code Bloat: ~21 lines that could be reduced to a generic helper plus 3 one-liners.
Refactoring Recommendations
-
Extract a generic
callListMethodhelper using a function parameter for the SDK call:// callListMethod is a generic helper for no-parameter SDK list operations. func (c *Connection) callListMethod(call func() (interface{}, error)) (*Response, error) { if err := c.requireSession(); err != nil { return nil, err } result, err := call() if err != nil { return nil, err } return marshalToResponse(result) } func (c *Connection) listTools() (*Response, error) { return c.callListMethod(func() (interface{}, error) { return c.session.ListTools(c.ctx, &sdk.ListToolsParams{}) }) } // ... same for listResources and listPrompts
Estimated effort: ~30 minutes. Benefits: single place to update list operation error handling.
Note: Go generics or a
func() (interface{}, error)adapter can be used. The benefit is modest given the simplicity of the pattern; weigh against reduced readability before implementing.
Implementation Checklist
- Evaluate whether the helper improves or reduces readability in context
- If proceeding: add
callListMethodhelper tointernal/mcp/connection.go - Refactor
listTools,listResources,listPromptsto use the helper - Verify existing tests still pass
- Run
make agent-finished
Parent Issue
See parent analysis report: #1388
Related to #1388
Generated by Duplicate Code Detector
- expires on Mar 4, 2026, 3:01 AM UTC