-
Notifications
You must be signed in to change notification settings - Fork 16
Closed
Description
Part of duplicate code analysis: #1948
Summary
Three functions in internal/mcp/connection.go — callTool, readResource, and getPrompt — independently implement an identical 6-step dispatch pattern. A callListMethod helper already exists to abstract the parameterless list operations (lines 475-484), but no equivalent helper exists for parameterized SDK calls.
Duplication Details
Pattern: requireSession → unmarshalParams → log → SDK call → error check → marshalToResponse
- Severity: Medium
- Occurrences: 3 functions
- Locations:
internal/mcp/connection.golines 497–523 (callTool)internal/mcp/connection.golines 536–557 (readResource)internal/mcp/connection.golines 569–592 (getPrompt)
Shared structure (all three functions):
if err := c.requireSession(); err != nil {
return nil, err
}
var params struct { /* unique per function */ }
if err := unmarshalParams(rawParams, ¶ms); err != nil {
return nil, err
}
logConn.Printf("...", /* unique per function */)
result, err := c.session.SDKMETHOD(c.ctx, &sdk.PARAMS{/* unique fields */})
if err != nil {
return nil, err
}
return marshalToResponse(result)Impact Analysis
- Maintainability: Any change to the dispatch flow (e.g., adding tracing, changing error wrapping, new protocol requirement) must be applied three times manually
- Bug Risk: High — if a bug is found in the error-handling path of one function, developers may miss the other two copies
- Code Bloat: ~60 lines of near-identical scaffolding across three functions
Refactoring Recommendations
-
Introduce
callParamMethodhelper analogous to the existingcallListMethod- Signature:
func (c *Connection) callParamMethod(params interface{}, fn func(interface{}) (interface{}, error)) (*Response, error) - Handles:
requireSession,unmarshalParams, error checks, andmarshalToResponse - Each of the three callers provides only the unique unmarshal target and SDK call
- Estimated effort: 1–2 hours
- Benefits: single point of change for protocol updates; removes ~40 lines of scaffolding
- Signature:
-
Alternative: Use Go generics
callTypedMethod[P any]to unmarshal directly into the typed struct inside the helper, eliminating even the per-caller unmarshal boilerplate
Implementation Checklist
- Review duplication findings
- Design
callParamMethod(or generic equivalent) signature - Refactor
callTool,readResource,getPromptto use the helper - Verify all three unit/integration tests still pass
- Check
callTool's extranilguard forArgumentsis preserved
Parent Issue
See parent analysis report: #1948
Related to #1948
Generated by Duplicate Code Detector · ◷
- expires on Mar 22, 2026, 3:06 AM UTC
Reactions are currently unavailable
Metadata
Metadata
Assignees
Type
Fields
Give feedbackNo fields configured for issues without a type.