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.go lines 497–523 (callTool)
  • internal/mcp/connection.go lines 536–557 (readResource)
  • internal/mcp/connection.go lines 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, &params); 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

1. Introduce callParamMethod helper analogous to the existing callListMethod

   • Signature: func (c *Connection) callParamMethod(params interface{}, fn func(interface{}) (interface{}, error)) (*Response, error)
   • Handles: requireSession, unmarshalParams, error checks, and marshalToResponse
   • 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

2. 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, getPrompt to use the helper
• Verify all three unit/integration tests still pass
• Check callTool's extra nil guard for Arguments is 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