Part of duplicate code analysis: #2983

Summary

Consumer code in internal/server/unified.go and internal/launcher/launcher.go re-implements config field defaults that the config loading layer already guarantees via applyDefaults() (called from LoadFromFile/LoadFromStdin) and applyGatewayDefaults(). This is a copy-of-defaults pattern: the same constant is used as a fallback in both the config package and the consuming package.

Duplication Details

Pattern: Consumer code re-implementing config defaults

• Severity: Medium
• Occurrences: 4 nil-guard + fallback blocks across 2 files
• Locations:
  • internal/server/unified.go (lines 113–130) — 3 blocks for PayloadDir, PayloadPathPrefix, PayloadSizeThreshold
  • internal/launcher/launcher.go (lines 58–65) — 1 block for StartupTimeout

internal/server/unified.go (lines 113–130):

// Get payload directory from config, with fallback to default
payloadDir := config.DefaultPayloadDir
if cfg.Gateway != nil && cfg.Gateway.PayloadDir != "" {
    payloadDir = cfg.Gateway.PayloadDir
}

// Get payload path prefix from config (empty by default)
payloadPathPrefix := ""
if cfg.Gateway != nil && cfg.Gateway.PayloadPathPrefix != "" {
    payloadPathPrefix = cfg.Gateway.PayloadPathPrefix
}

// Get payload size threshold from config, with fallback to default
payloadSizeThreshold := config.DefaultPayloadSizeThreshold
if cfg.Gateway != nil && cfg.Gateway.PayloadSizeThreshold > 0 {
    payloadSizeThreshold = cfg.Gateway.PayloadSizeThreshold
}

internal/launcher/launcher.go (lines 58–65):

startupTimeout := time.Duration(config.DefaultStartupTimeout) * time.Second
if cfg.Gateway != nil && cfg.Gateway.StartupTimeout > 0 {
    startupTimeout = time.Duration(cfg.Gateway.StartupTimeout) * time.Second
    logLauncher.Printf("Using configured startup timeout: %v", startupTimeout)
} else {
    logLauncher.Printf("Using default startup timeout: %v", startupTimeout)
}

What the config layer already guarantees:

internal/config/config_payload.go registers a DefaultsSetter via init():

RegisterDefaults(func(cfg *Config) {
    if cfg.Gateway != nil {
        if cfg.Gateway.PayloadDir == "" {
            cfg.Gateway.PayloadDir = DefaultPayloadDir
        }
        if cfg.Gateway.PayloadSizeThreshold == 0 {
            cfg.Gateway.PayloadSizeThreshold = DefaultPayloadSizeThreshold
        }
    }
})

internal/config/config_core.go's applyGatewayDefaults() sets:

if cfg.StartupTimeout == 0 {
    cfg.StartupTimeout = DefaultStartupTimeout
}

Both LoadFromFile and LoadFromStdin ensure cfg.Gateway is never nil before calling applyDefaults() / applyGatewayDefaults(). So after config loading:

• cfg.Gateway is always non-nil
• cfg.Gateway.PayloadDir is always DefaultPayloadDir or user-specified
• cfg.Gateway.PayloadSizeThreshold is always DefaultPayloadSizeThreshold or user-specified
• cfg.Gateway.StartupTimeout is always DefaultStartupTimeout or user-specified

Impact Analysis

• Maintainability: If a new config field gets a default in config_payload.go, the consumer files must be manually updated or they silently use stale logic. There is no compile-time enforcement.
• Bug Risk: Low currently (defaults match), but divergence is possible. The PayloadPathPrefix in unified.go has no corresponding default in the config package — it defaults to "" in both places but would silently stay "" even if a default were added later.
• Code Bloat: ~20 lines of redundant defensive code that guards against a condition (cfg.Gateway == nil) that the config loading layer already prevents.

Refactoring Recommendations

Option A: Direct field access (minimal change)

Remove the nil-guards and fallback assignments. After config loading, cfg.Gateway is guaranteed non-nil and fields have defaults applied:

// unified.go — replace lines 113–130 with:
payloadDir := cfg.Gateway.PayloadDir
payloadPathPrefix := cfg.Gateway.PayloadPathPrefix
payloadSizeThreshold := cfg.Gateway.PayloadSizeThreshold

// launcher.go — replace lines 58–65 with:
startupTimeout := time.Duration(cfg.Gateway.StartupTimeout) * time.Second
logLauncher.Printf("Using startup timeout: %v", startupTimeout)

Option B: Add accessor methods to Config (more robust)

Add accessor methods to *Config that encapsulate nil-safety and return defaults:

func (c *Config) GetPayloadDir() string { ... }
func (c *Config) GetStartupTimeout() time.Duration { ... }

This makes the contract explicit and is usable from any consumer without knowledge of the loading lifecycle.

Option C: Move PayloadPathPrefix into config_payload.go defaults

PayloadPathPrefix currently has no registered default — add it alongside PayloadDir and PayloadSizeThreshold for consistency.

Estimated effort: 1–2 hours
Recommended approach: Option A with Option C for PayloadPathPrefix, then follow up with Option B if more callers emerge.

Implementation Checklist

• Verify that all call sites of NewUnified() and launcher.New() pass a config loaded via LoadFromFile or LoadFromStdin
• Add PayloadPathPrefix default to config_payload.go (currently missing)
• Remove redundant nil-guards in internal/server/unified.go lines 113–130
• Remove redundant fallback in internal/launcher/launcher.go lines 58–65
• Update or add tests asserting that NewUnified reads PayloadDir/PayloadSizeThreshold directly from config
• Verify no functionality broken with make agent-finished

Parent Issue

See parent analysis report: #2983
Related to #2983

Generated by Duplicate Code Detector · ◷

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