[duplicate-code] Duplicate Code Pattern: Server Launch Sequences in launcher.go

[github-actions]

🔍 Duplicate Code Pattern: Duplicated Server Launch Sequences

Part of duplicate code analysis: #555

Summary

The internal/launcher/launcher.go file contains two nearly identical server launch sequences (~90 lines each): one for shared/stateless connections (GetOrLaunch) and one for session-aware connections (GetOrLaunchForSession). These functions share approximately 80% identical code, including error handling, logging, timeout logic, and security warnings.

Duplication Details

Pattern: Duplicate Launch Logic with Minor Variations

• Severity: High
• Occurrences: 2 instances in the same file
• Locations:
  • internal/launcher/launcher.go (lines 130-233) - GetOrLaunch function
  • internal/launcher/launcher.go (lines 280-370) - GetOrLaunchForSession function
• Duplicated Lines: ~90 lines per function, ~70 lines identical

Code Sample - Shared Error Handling Block

Both functions contain this identical error handling pattern:

// From GetOrLaunch (lines 193-213)
log.Printf("[LAUNCHER] ❌ FAILED to launch server '%s'", serverID)
log.Printf("[LAUNCHER] Error: %v", err)
log.Printf("[LAUNCHER] Debug Information:")
log.Printf("[LAUNCHER]   - Command: %s", serverCfg.Command)
log.Printf("[LAUNCHER]   - Args: %v", serverCfg.Args)
log.Printf("[LAUNCHER]   - Env vars: %v", sanitize.TruncateSecretMap(serverCfg.Env))
log.Printf("[LAUNCHER]   - Running in container: %v", l.runningInContainer)
log.Printf("[LAUNCHER]   - Is direct command: %v", isDirectCommand)
log.Printf("[LAUNCHER]   - Startup timeout: %v", l.startupTimeout)

if isDirectCommand && l.runningInContainer {
    log.Printf("[LAUNCHER] ⚠️  Possible causes:")
    log.Printf("[LAUNCHER]   - Command '%s' may not be installed in the gateway container", serverCfg.Command)
    log.Printf("[LAUNCHER]   - Consider using 'container' config instead of 'command'")
    log.Printf("[LAUNCHER]   - Or add '%s' to the gateway's Dockerfile", serverCfg.Command)
} else if isDirectCommand {
    log.Printf("[LAUNCHER] ⚠️  Possible causes:")
    log.Printf("[LAUNCHER]   - Command '%s' may not be in PATH", serverCfg.Command)
    log.Printf("[LAUNCHER]   - Check if '%s' is installed: which %s", serverCfg.Command, serverCfg.Command)
    log.Printf("[LAUNCHER]   - Verify file permissions and execute bit")
}

// Same block repeated in GetOrLaunchForSession (lines 339-347)

Other Duplicated Sections

1. Security Warning Block (lines 132-134 and 284-286):

   log.Printf("[LAUNCHER] ⚠️  WARNING: Server '%s' uses direct command execution inside a container", serverID)
   log.Printf("[LAUNCHER] ⚠️  Security Notice: Command '%s' will execute with the same privileges as the gateway", serverCfg.Command)
   log.Printf("[LAUNCHER] ⚠️  Consider using 'container' field instead for better isolation")

2. Environment Variable Passthrough Check (lines 146-164 and 297-313):

   • Nearly identical 18-line blocks checking for -e flag and validating environment variables

3. Startup Logging (lines 139-141 and 291-293):

   log.Printf("[LAUNCHER] Starting MCP server: %s", serverID)
   log.Printf("[LAUNCHER] Command: %s", serverCfg.Command)
   log.Printf("[LAUNCHER] Args: %v", sanitize.SanitizeArgs(serverCfg.Args))

4. Timeout Error Handling (lines 228-230 and 366-368):

   log.Printf("[LAUNCHER] ❌ Server startup timed out after %v", l.startupTimeout)
   log.Printf("[LAUNCHER] ⚠️  The server may be hanging or taking too long to initialize")
   log.Printf("[LAUNCHER] ⚠️  Consider increasing 'startupTimeout' in gateway config if server needs more time")

Impact Analysis

• Maintainability: When bugs are found in the launch logic, fixes must be applied to both functions. Recent commit 39f8c61 already had to fix test failures in launcher - this duplication increases the risk of incomplete fixes.
• Bug Risk: High potential for divergence between the two implementations if updates are made to only one function
• Code Bloat: ~90 lines duplicated = 180 lines total when could be ~110 with proper extraction
• Testing Overhead: Every edge case (timeout, errors, env vars, security warnings) must be tested twice

Refactoring Recommendations

1. Extract Common Launch Logic into Private Helper Function

Create a helper function that handles the shared launch sequence:

// launchServerWithConfig handles the common server launch logic for both shared and session-aware connections
func (l *Launcher) launchServerWithConfig(
    ctx context.Context,
    serverID string,
    sessionID string, // empty string for shared connections
    serverCfg *config.ServerConfig,
) (*mcp.Connection, error) {
    // Consolidated logic:
    // - Security warnings
    // - Environment variable checks
    // - Startup logging
    // - Connection creation with timeout
    // - Error handling with diagnostics
    // - Success logging
}

Estimated effort: 2-3 hours
Benefits:

• Single source of truth for launch logic
• Easier to add new features (only one place to modify)
• Reduced testing burden
• Better error handling consistency

2. Extract Error Logging Helper

Create a dedicated error diagnostics function:

// logLaunchError logs detailed error information for failed server launches
func (l *Launcher) logLaunchError(
    serverID string,
    sessionID string,
    err error,
    serverCfg *config.ServerConfig,
    isDirectCommand bool,
) {
    // Consolidated error logging with diagnostics
}

Estimated effort: 1 hour
Benefits:

• Consistent error messages
• Easier to enhance diagnostics in one place

3. Extract Environment Variable Validation

// logEnvPassthrough checks and logs environment variable passthrough status
func (l *Launcher) logEnvPassthrough(args []string) {
    // Consolidated env var checking logic
}

Estimated effort: 30 minutes
Benefits:

• Reusable across other launcher functions
• Easier to modify passthrough logic

Implementation Checklist

• Review duplication findings with team
• Decide on refactoring approach (incremental vs. full rewrite)
• Create helper function launchServerWithConfig
• Refactor GetOrLaunch to use helper
• Refactor GetOrLaunchForSession to use helper
• Update unit tests to cover helper function
• Run make agent-finished to verify no regressions
• Update integration tests if needed

Parent Issue

See parent analysis report: #555
Related to #555

AI generated by Duplicate Code Detector