Part of duplicate code analysis: #2825

Summary

Three locations in the server package extract the MCP session ID from a context.Context using different — and inconsistent — approaches. One of the three uses an unchecked type assertion that will panic if the session ID is absent or of the wrong type.

Duplication Details

Pattern: Session ID context extraction — three divergent implementations

• Severity: High
• Occurrences: 3 locations
• Locations:
  • internal/server/session.go ~lines 37–43 — safe, uses ok check
  • internal/server/unified.go ~lines 245–250 — partially safe, nil check then forced cast
  • internal/server/routed.go ~line 91 — unsafe, bare forced type assertion

Variant 1 — safe (session.go):

if sessionID, ok := ctx.Value(SessionIDContextKey).(string); ok && sessionID != "" {
    logSession.Printf("Extracted session ID: %s", auth.TruncateSessionID(sessionID))
    return sessionID
}
return "default"

Variant 2 — partially safe (unified.go):

sessionID := g.ctx.Value(SessionIDContextKey)
if sessionID == nil {
    sessionID = "default"
}
return executeBackendToolCall(g.ctx, g.server.launcher, g.serverID, sessionID.(string), ...)
// ⚠️ sessionID.(string) panics if context value is non-nil but not a string

Variant 3 — unsafe (routed.go):

sessionID := r.Context().Value(SessionIDContextKey).(string)
// ⚠️ Panics immediately if SessionIDContextKey is absent or has wrong type

Impact Analysis

• Maintainability: Three different extraction patterns create confusion about which is canonical.
• Bug Risk: High — routed.go:91 will cause a runtime panic in any code path where the session ID is not already set (e.g. an early middleware rejection path that somehow reaches the handler).
• Code Bloat: Minor (30–40 lines total), but correctness is the bigger concern.

Refactoring Recommendations

1. Add a canonical helper to session.go:

   // SessionIDFromContext returns the MCP session ID stored in ctx, or fallback
   // if the context contains no session ID. This is the only place in the server
   // package that should read SessionIDContextKey directly.
   func SessionIDFromContext(ctx context.Context) string {
       if id, ok := ctx.Value(SessionIDContextKey).(string); ok && id != "" {
           return id
       }
       return "default"
   }

2. Replace all three sites with SessionIDFromContext(ctx) / SessionIDFromContext(r.Context()).

3. Fix the latent panic in routed.go:91 immediately, even before the full refactor:

   // Before (panics if absent)
   sessionID := r.Context().Value(SessionIDContextKey).(string)
   
   // After (safe)
   sessionID := SessionIDFromContext(r.Context())

   Estimated effort: < 30 minutes for the panic fix; ~1 hour for the full refactor.

Implementation Checklist

• Add SessionIDFromContext helper to internal/server/session.go
• Replace unsafe assertion in routed.go:91
• Replace partially-safe pattern in unified.go:245–250
• Replace safe-but-inline pattern in session.go:37–43
• Run unit tests to confirm no regression

Parent Issue

See parent analysis report: #2825
Related to #2825

Generated by Duplicate Code Detector · ◷

• expires on Apr 6, 2026, 6:10 AM UTC