[duplicate-code] Duplicate Code Pattern: Hardcoded 30s connect timeout default duplicated in connection.go

[github-actions]

Part of duplicate code analysis: #3931

Summary

The 30-second connect timeout default is applied inline in two separate locations inside internal/mcp/connection.go, while the canonical value DefaultConnectTimeout = 30 already exists in internal/config/config_core.go. The two inline copies also use slightly different guard conditions (<= 0 vs == 0), creating a latent inconsistency.

Duplication Details

Pattern: Hardcoded 30s default for connect timeout, duplicated in connection.go

• Severity: Low
• Occurrences: 3 (2 inline duplicates + 1 config constant that should be the single source of truth)
• Locations:
  • internal/config/config_core.go line 44 — canonical constant: DefaultConnectTimeout = 30
  • internal/mcp/connection.go lines 200–203 — NewHTTPConnection: if connectTimeout <= 0 { connectTimeout = 30 * time.Second }
  • internal/mcp/connection.go lines 381–384 — reconnectSDKTransport: if timeout == 0 { timeout = 30 * time.Second }
• Code Sample:

  // In NewHTTPConnection (line 201)
  if connectTimeout <= 0 {
      connectTimeout = 30 * time.Second
  }
  
  // In reconnectSDKTransport (line 382)
  if timeout == 0 {
      timeout = 30 * time.Second
  }

  Note: the guards differ (<= 0 vs == 0). The reconnectSDKTransport guard will not catch negative values.

Impact Analysis

• Maintainability: Changing the default connect timeout requires edits in 3 files — config_core.go and two locations in connection.go
• Bug Risk: The inconsistent guards (<= 0 vs == 0) mean a negative connectTimeout value passed to reconnectSDKTransport would be used as-is rather than replaced with the 30-second default, potentially causing unexpected behavior
• Code Bloat: Minor

Refactoring Recommendations

1. Import and use config.DefaultConnectTimeout in connection.go:

   import "github.com/github/gh-aw-mcpg/internal/config"
   
   // In NewHTTPConnection:
   if connectTimeout <= 0 {
       connectTimeout = time.Duration(config.DefaultConnectTimeout) * time.Second
   }
   
   // In reconnectSDKTransport (fix guard to <= 0 for consistency):
   if c.connectTimeout <= 0 {
       timeout = time.Duration(config.DefaultConnectTimeout) * time.Second
   } else {
       timeout = c.connectTimeout
   }

2. Alternative: Define a package-level constant in internal/mcp that imports the value, if a circular import would result:

   const defaultConnectTimeout = 30 * time.Second

3. Estimated effort: ~15 minutes

4. Benefits: Single source of truth for the default timeout value; consistent <= 0 guard prevents edge case with negative values

Implementation Checklist

• Review duplication findings
• Verify no import cycle between mcp and config packages
• Update both locations in internal/mcp/connection.go to use the constant
• Fix the inconsistent guard (== 0 → <= 0) in reconnectSDKTransport
• Run make test to verify no functionality broken

Parent Issue

See parent analysis report: #3931
Related to #3931

Generated by Duplicate Code Detector · ● 1.4M · ◷

• expires on Apr 23, 2026, 6:12 AM UTC