Skip to content

Refactor duplicated MCP handler config setup in transport and routed server paths#6753

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-buildmcp-handler-setup
May 30, 2026
Merged

Refactor duplicated MCP handler config setup in transport and routed server paths#6753
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-buildmcp-handler-setup

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

CreateHTTPServerForMCP and CreateHTTPServerForRoutedMode were each assembling nearly identical mcpHandlerConfig values before calling buildMCPHandler, creating a small but recurring maintenance seam. This change centralizes that shared config construction while preserving the per-mode server-factory behavior.

  • Shared config construction

    • Added buildDefaultHandlerConfig(...) in internal/server/middleware.go to build the common mcpHandlerConfig fields (sessionTimeout, unifiedServer) plus caller-provided options (handlerLog, logTag, apiKey, hmacSecret).
    • Introduced defaultHandlerConfigOptions so string fields are passed by name instead of positional arguments.
  • Call-site deduplication

    • Updated internal/server/transport.go to use the helper for unified mode (logTag: "unified").
    • Updated internal/server/routed.go to use the helper inside the backend loop (logTag: "routed:"+backendID).
  • Focused coverage

    • Added TestBuildDefaultHandlerConfig in internal/server/middleware_test.go to validate field mapping and keep future config-field additions centralized.
finalHandler := buildMCPHandler(serverFactory, buildDefaultHandlerConfig(
    unifiedServer,
    sessionTimeout,
    defaultHandlerConfigOptions{
        handlerLog: logTransport,
        logTag:     "unified",
        apiKey:     apiKey,
        hmacSecret: hmacSecret,
    },
))

Copilot AI changed the title [WIP] Refactor duplicate buildMCPHandler setup in transport.go and routed.go Refactor duplicated MCP handler config setup in transport and routed server paths May 30, 2026
Copilot finished work on behalf of lpcox May 30, 2026 14:00
Copilot AI requested a review from lpcox May 30, 2026 14:00
@lpcox lpcox marked this pull request as ready for review May 30, 2026 14:01
Copilot AI review requested due to automatic review settings May 30, 2026 14:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 server setup by centralizing construction of the common mcpHandlerConfig used by both unified (transport) and routed server modes, reducing duplicated config wiring in multiple call paths.

Changes:

  • Added buildDefaultHandlerConfig(...) to construct shared mcpHandlerConfig fields in one place.
  • Updated unified and routed server creation paths to use the new helper.
  • Added a focused unit test to validate config field mapping.
Show a summary per file
File Description
internal/server/transport.go Replaces inline mcpHandlerConfig{...} construction with buildDefaultHandlerConfig(...) for unified mode.
internal/server/routed.go Replaces inline mcpHandlerConfig{...} construction with buildDefaultHandlerConfig(...) per backend in routed mode.
internal/server/middleware.go Introduces the shared buildDefaultHandlerConfig(...) helper that returns an mcpHandlerConfig.
internal/server/middleware_test.go Adds TestBuildDefaultHandlerConfig to assert field mapping from helper inputs to config fields.

Copilot's findings

Tip

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

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

Comment thread internal/server/middleware.go Outdated
Comment on lines +34 to +42
func buildDefaultHandlerConfig(unifiedServer *UnifiedServer, apiKey, hmacSecret string, sessionTimeout time.Duration, handlerLog *logger.Logger, logTag string) mcpHandlerConfig {
return mcpHandlerConfig{
handlerLog: handlerLog,
sessionTimeout: sessionTimeout,
logTag: logTag,
unifiedServer: unifiedServer,
apiKey: apiKey,
hmacSecret: hmacSecret,
}
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 30, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 30, 2026

@copilot address review feedback

Addressed in commit 5f10b06 by changing buildDefaultHandlerConfig to take a named options struct (removing positional string arguments), and updating both call sites plus the middleware test.

Copilot finished work on behalf of lpcox May 30, 2026 14:10
@lpcox lpcox merged commit 2d00487 into main May 30, 2026
22 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-buildmcp-handler-setup branch May 30, 2026 14:15
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: buildMCPHandler Setup in transport.go and routed.go

3 participants