Skip to content

refactor(mcp): extract withReconnectLock to eliminate reconnect boilerplate#7103

Merged
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-reconnect-boilerplate
Jun 6, 2026
Merged

refactor(mcp): extract withReconnectLock to eliminate reconnect boilerplate#7103
lpcox merged 2 commits into
mainfrom
copilot/duplicate-code-reconnect-boilerplate

Conversation

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

reconnectPlainJSON and reconnectSDKTransport shared identical lock/log/error-wrap scaffolding, requiring any change to the reconnect lifecycle to be applied in two places.

Changes

  • New withReconnectLock helper (connection.go) — acquires the session write lock, emits the reconnect log, calls logReconnectStart/logReconnectResult, and wraps errors uniformly; accepts the transport-specific logic as a func() error closure
  • reconnectPlainJSON — reduced to a withReconnectLock("plain JSON-RPC", ...) call containing only the session-init and ID assignment logic
  • reconnectSDKTransport — reduced to a withReconnectLock(fmt.Sprintf("SDK transport (type=%s)", ...), ...) call containing only transport construction and session connect logic; the default branch now also flows through the shared epilogue (logging + wrapping), which is a minor improvement
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
}

…t boilerplate

Both reconnectPlainJSON and reconnectSDKTransport shared identical preamble
(acquire write lock, log reconnect attempt, call logReconnectStart) and epilogue
(call logReconnectResult, wrap error with "session reconnect failed: %w").

Extract withReconnectLock(transportName string, reconnect func() error) error that
handles the lifecycle uniformly. Each reconnect function now passes only its unique
transport logic as a closure, making the reconnect contract compiler-enforced and
reducing the risk of drift between the two paths.

Resolves the duplicate-code issue raised in gh-aw-mcpg#7078.
Copilot AI changed the title [WIP] Refactor duplicate reconnect functions for MCP connection refactor(mcp): extract withReconnectLock to eliminate reconnect boilerplate Jun 6, 2026
Copilot finished work on behalf of lpcox June 6, 2026 16:21
Copilot AI requested a review from lpcox June 6, 2026 16:21
@lpcox lpcox marked this pull request as ready for review June 6, 2026 16:26
Copilot AI review requested due to automatic review settings June 6, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 HTTP session reconnection logic by extracting the shared session-locking + reconnect-logging + error-wrapping scaffolding into a single helper, reducing duplicated reconnect boilerplate and ensuring consistent reconnect telemetry/error handling across transports.

Changes:

  • Added (*Connection).withReconnectLock(...) to centralize session write-locking, reconnect start/result logging, and uniform error wrapping.
  • Simplified reconnectPlainJSON to only perform HTTP session initialization + session ID assignment inside the helper closure.
  • Simplified reconnectSDKTransport to only perform SDK transport construction + connect logic inside the helper closure, and ensured the default/unsupported transport branch now gets consistent logging/wrapping.
Show a summary per file
File Description
internal/mcp/connection.go Extracts shared reconnect lock/log/error epilogue into withReconnectLock and rewrites both reconnect paths to use it.

Copilot's findings

Tip

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

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

@lpcox lpcox merged commit 52d5ec0 into main Jun 6, 2026
29 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-reconnect-boilerplate branch June 6, 2026 16:34
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 Connection Reconnect Boilerplate

3 participants