[duplicate-code] Duplicate Code Pattern: MCP Connection Reconnect Boilerplate

[github-actions]

🔍 Duplicate Code Pattern: MCP Connection Reconnect Boilerplate

Part of duplicate code analysis: #7078

Summary

internal/mcp/connection.go contains two reconnect functions — reconnectPlainJSON and reconnectSDKTransport — that share identical structural boilerplate at the start and end of each function body. The unique logic is sandwiched between identical preamble and epilogue code.

Duplication Details

Pattern: Shared lock/log/error preamble and epilogue across two reconnect methods

• Severity: Medium
• Occurrences: 2 functions in the same file
• Locations:
  • internal/mcp/connection.go — reconnectPlainJSON (approx. lines 321–336)
  • internal/mcp/connection.go — reconnectSDKTransport (approx. lines 341–396)
• Duplicated lines per function: ~7 structural lines (lock/unlock, log helpers, error return shape)

Code Sample — identical preamble in both functions:

c.sessionMu.Lock()
defer c.sessionMu.Unlock()
logConn.Printf("Session expired, reconnecting ... for serverID=%s", c.serverID)
c.logReconnectStart()

Code Sample — identical epilogue in both functions:

c.logReconnectResult(err)
if err != nil {
    return fmt.Errorf("session reconnect failed: %w", err)
}

Impact Analysis

• Maintainability: Any change to the reconnect locking strategy, logging call, or error message must be applied to both functions manually.
• Bug Risk: A future engineer fixing the error message in reconnectSDKTransport could easily miss reconnectPlainJSON (or vice versa), leading to inconsistent behaviour.
• Code Bloat: Minor — ~7 lines × 2 = 14 extra lines in a single file.

Refactoring Recommendations

1. Extract a withReconnectLock helper that accepts the transport-specific logic as a func() error closure

   // Suggested addition in connection.go
   func (c *Connection) withReconnectLock(transportName string, reconnect func() error) error {
       c.sessionMu.Lock()
       defer c.sessionMu.Unlock()
       logConn.Printf("Session expired, reconnecting %s for serverID=%s", transportName, c.serverID)
       c.logReconnectStart()
       err := reconnect()
       c.logReconnectResult(err)
       if err != nil {
           return fmt.Errorf("session reconnect failed: %w", err)
       }
       return nil
   }

   Then the two existing functions become:

   func (c *Connection) reconnectPlainJSON() error {
       return c.withReconnectLock("plain JSON-RPC", func() error {
           sessionID, err := c.initializeHTTPSession()
           if err != nil { return err }
           c.httpSessionID = sessionID
           return nil
       })
   }

   • Extract to: internal/mcp/connection.go (same file, new unexported method)
   • Estimated effort: 30–60 minutes
   • Benefits: Single place to modify reconnect lifecycle; compiler enforces the pattern

Implementation Checklist

• Review both reconnectPlainJSON and reconnectSDKTransport in full
• Implement withReconnectLock helper
• Refactor both reconnect functions to use the helper
• Run make test to verify no regression
• Verify reconnect logging output is unchanged

Parent Issue

See parent analysis report: #7078
Related to #7078

Generated by Duplicate Code Detector · sonnet46 6.1M · ◷

• expires on Jun 13, 2026, 3:43 AM UTC