-
Notifications
You must be signed in to change notification settings - Fork 7
Open
Description
🔍 Duplicate Code Pattern: Session-Aware Logging Branching
Part of duplicate code analysis: #929
Summary
Multiple logging functions in internal/launcher/log_helpers.go contain identical if/else branching logic to handle sessionID-aware logging. This pattern appears in 3 functions with nearly identical structure, leading to ~45 lines of duplicated conditional logic.
Duplication Details
Pattern: Conditional Session Logging
-
Severity: High
-
Occurrences: 3 functions
-
Locations:
internal/launcher/log_helpers.golines 31-44 (logLaunchStart)internal/launcher/log_helpers.golines 95-106 (logTimeoutError)internal/launcher/log_helpers.golines 109-119 (logLaunchSuccess)
-
Code Sample (from
logLaunchStart):func (l *Launcher) logLaunchStart(serverID, sessionID string, serverCfg *config.ServerConfig, isDirectCommand bool) { if sessionID != "" { logger.LogInfoWithServer(serverID, "backend", "Launching MCP backend server for session: server=%s, session=%s, command=%s, args=%v", serverID, sessionID, serverCfg.Command, sanitize.SanitizeArgs(serverCfg.Args)) log.Printf("[LAUNCHER] Starting MCP server for session: %s (session: %s)", serverID, sessionID) logLauncher.Printf("Launching new session server: serverID=%s, sessionID=%s, command=%s", serverID, sessionID, serverCfg.Command) } else { logger.LogInfoWithServer(serverID, "backend", "Launching MCP backend server: %s, command=%s, args=%v", serverID, serverCfg.Command, sanitize.SanitizeArgs(serverCfg.Args)) log.Printf("[LAUNCHER] Starting MCP server: %s", serverID) logLauncher.Printf("Launching new server: serverID=%s, command=%s, inContainer=%v, isDirectCommand=%v", serverID, serverCfg.Command, l.runningInContainer, isDirectCommand) } log.Printf("[LAUNCHER] Command: %s", serverCfg.Command) log.Printf("[LAUNCHER] Args: %v", sanitize.SanitizeArgs(serverCfg.Args)) }
-
Similar Pattern (from
logLaunchSuccess):func (l *Launcher) logLaunchSuccess(serverID, sessionID string) { if sessionID != "" { logger.LogInfoWithServer(serverID, "backend", "Successfully launched MCP backend server for session: server=%s, session=%s", serverID, sessionID) log.Printf("[LAUNCHER] Successfully launched: %s (session: %s)", serverID, sessionID) logLauncher.Printf("Session connection established: serverID=%s, sessionID=%s", serverID, sessionID) } else { logger.LogInfoWithServer(serverID, "backend", "Successfully launched MCP backend server: %s", serverID) log.Printf("[LAUNCHER] Successfully launched: %s", serverID) logLauncher.Printf("Connection established: serverID=%s", serverID) } }
Impact Analysis
- Maintainability: High impact - Changes to session handling must be replicated across 3 functions, increasing risk of inconsistent behavior
- Bug Risk: Medium-High - If one function is updated for session handling but others aren't, logging behavior becomes inconsistent
- Code Bloat: ~45 lines of duplicated branching logic
- Testing Complexity: Each function requires separate test cases for session vs non-session scenarios
Refactoring Recommendations
Option 1: Extract Session-Aware Logging Helper (Recommended)
- Extract common pattern to:
internal/launcher/session_logger.go - Estimated effort: 2-3 hours
- Benefits:
- Single source of truth for session-aware logging
- Consistent logging format across all launcher operations
- Easier to modify logging behavior
- Implementation:
// sessionLogger wraps triple logging with session awareness type sessionLogger struct { serverID string sessionID string } func (sl *sessionLogger) logInfo(category, fileMsg, stdoutMsg, debugMsg string, args ...interface{}) { if sl.sessionID != "" { logger.LogInfoWithServer(sl.serverID, category, fileMsg+" for session: session="+sl.sessionID, args...) log.Printf(stdoutMsg+" (session: "+sl.sessionID+")", args...) logLauncher.Printf(debugMsg+", sessionID="+sl.sessionID, args...) } else { logger.LogInfoWithServer(sl.serverID, category, fileMsg, args...) log.Printf(stdoutMsg, args...) logLauncher.Printf(debugMsg, args...) } }
Option 2: Use sessionSuffix() More Consistently
- Leverage existing
sessionSuffix()helper more broadly - Estimated effort: 1-2 hours
- Benefits: Simpler, less refactoring needed
- Trade-off: Still requires manual if/else in some cases
Implementation Checklist
- Review duplication findings with team
- Decide on refactoring approach (Option 1 vs Option 2)
- Create session-aware logging helper utility
- Refactor
logLaunchStart(),logTimeoutError(),logLaunchSuccess() - Update tests to cover new utility
- Verify no functionality broken
- Consider extending pattern to other launcher functions
Parent Issue
See parent analysis report: #929
Related to #929
AI generated by Duplicate Code Detector
- expires on Feb 21, 2026, 10:12 AM UTC
Reactions are currently unavailable