Part of duplicate code analysis: #1283

Summary

The sdk.NewStreamableHTTPHandler callback used to establish HTTP sessions is nearly identical in transport.go (unified mode) and routed.go (routed mode). Both callbacks perform the same sequence of operations — extract+validate session, log the connection, log the request body, inject context — with only cosmetic differences.

Duplication Details

Pattern: StreamableHTTP session-setup callback

• Severity: Medium
• Occurrences: 2
• Locations:
  • internal/server/transport.go (lines 85–108)
  • internal/server/routed.go (lines 110–130)

transport.go (unified):

sessionID := extractAndValidateSession(r)
if sessionID == "" {
    return nil
}
logger.LogInfo("client", "MCP connection established, remote=%s, method=%s, path=%s, session=%s", ...)
log.Printf("=== NEW STREAMABLE HTTP CONNECTION ===")
log.Printf("[%s] %s %s", r.RemoteAddr, r.Method, r.URL.Path)
log.Printf("Authorization (Session ID): %s", sanitize.TruncateSecret(sessionID))
log.Printf("DEBUG: About to check request body, Method=%s, Body!=nil: %v", ...)
logHTTPRequestBody(r, sessionID, "")
*r = *injectSessionContext(r, sessionID, "")
log.Printf("✓ Injected session ID into context")
log.Printf("==========================\n")
return unifiedServer.server

routed.go (routed):

sessionID := extractAndValidateSession(r)
if sessionID == "" {
    return nil
}
logger.LogInfo("client", "New MCP client connection, remote=%s, method=%s, path=%s, backend=%s, session=%s", ...)
log.Printf("=== NEW STREAMABLE HTTP CONNECTION (ROUTED) ===")
log.Printf("[%s] %s %s", r.RemoteAddr, r.Method, r.URL.Path)
log.Printf("Backend: %s", backendID)
log.Printf("Authorization (Session ID): %s", sessionID)
logHTTPRequestBody(r, sessionID, backendID)
*r = *injectSessionContext(r, sessionID, backendID)
log.Printf("✓ Injected session ID and backend ID into context")
log.Printf("===================================\n")
return serverCache.getOrCreate(...)

The only differences are: the log message text, whether backendID is passed, and the return value. The structural logic is identical.

Impact Analysis

• Maintainability: Any change to how sessions are logged (e.g. adding a trace ID, changing log formats) must be applied in both places. This has already led to minor inconsistencies (e.g. sanitize.TruncateSecret used in one but not the other).
• Bug Risk: If validation logic in extractAndValidateSession is changed and callers are updated separately, the two callbacks could diverge silently.
• Code Bloat: Minor (~15 lines duplicated).

Refactoring Recommendations

1. Extract a setupSession helper in http_helpers.go:

   // setupSession handles the common session establishment logic for both unified and routed modes.
   // It extracts and validates the session, logs the connection, and injects context.
   // Returns the modified request and session ID, or empty string on failure.
   func setupSession(r *http.Request, backendID string) (*http.Request, string) {
       sessionID := extractAndValidateSession(r)
       if sessionID == "" {
           return r, ""
       }
       if backendID != "" {
           logger.LogInfo("client", "New MCP client connection, remote=%s, ..., backend=%s, session=%s", ...)
       } else {
           logger.LogInfo("client", "MCP connection established, remote=%s, ..., session=%s", ...)
       }
       logHTTPRequestBody(r, sessionID, backendID)
       r = injectSessionContext(r, sessionID, backendID)
       return r, sessionID
   }

   Then callers become:

   r, sessionID := setupSession(r, backendID) // or "" for unified
   if sessionID == "" { return nil }

2. Estimated effort: ~30 minutes

3. Benefits: Single place to update session logging format; consistent use of sanitize.TruncateSecret; cleaner callbacks

Implementation Checklist

• Review duplication findings
• Extract setupSession helper into internal/server/http_helpers.go
• Update transport.go and routed.go callbacks to call the helper
• Update tests if applicable
• Verify no functionality broken

Parent Issue

See parent analysis report: #1283