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
Parent Issue
See parent analysis report: #2983
Related to #2983
Generated by Duplicate Code Detector · ◷
Part of duplicate code analysis: #2983
Summary
Consumer code in
internal/server/unified.goandinternal/launcher/launcher.gore-implements config field defaults that the config loading layer already guarantees viaapplyDefaults()(called fromLoadFromFile/LoadFromStdin) andapplyGatewayDefaults(). 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
internal/server/unified.go(lines 113–130) — 3 blocks forPayloadDir,PayloadPathPrefix,PayloadSizeThresholdinternal/launcher/launcher.go(lines 58–65) — 1 block forStartupTimeoutinternal/server/unified.go(lines 113–130):internal/launcher/launcher.go(lines 58–65):What the config layer already guarantees:
internal/config/config_payload.goregisters aDefaultsSetterviainit():internal/config/config_core.go'sapplyGatewayDefaults()sets:Both
LoadFromFileandLoadFromStdinensurecfg.Gatewayis never nil before callingapplyDefaults()/applyGatewayDefaults(). So after config loading:cfg.Gatewayis always non-nilcfg.Gateway.PayloadDiris alwaysDefaultPayloadDiror user-specifiedcfg.Gateway.PayloadSizeThresholdis alwaysDefaultPayloadSizeThresholdor user-specifiedcfg.Gateway.StartupTimeoutis alwaysDefaultStartupTimeoutor user-specifiedImpact Analysis
config_payload.go, the consumer files must be manually updated or they silently use stale logic. There is no compile-time enforcement.PayloadPathPrefixinunified.gohas no corresponding default in the config package — it defaults to""in both places but would silently stay""even if a default were added later.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.Gatewayis guaranteed non-nil and fields have defaults applied:Option B: Add accessor methods to Config (more robust)
Add accessor methods to
*Configthat encapsulate nil-safety and return defaults:This makes the contract explicit and is usable from any consumer without knowledge of the loading lifecycle.
Option C: Move
PayloadPathPrefixintoconfig_payload.godefaultsPayloadPathPrefixcurrently has no registered default — add it alongsidePayloadDirandPayloadSizeThresholdfor 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
NewUnified()andlauncher.New()pass a config loaded viaLoadFromFileorLoadFromStdinPayloadPathPrefixdefault toconfig_payload.go(currently missing)internal/server/unified.golines 113–130internal/launcher/launcher.golines 58–65NewUnifiedreadsPayloadDir/PayloadSizeThresholddirectly from configmake agent-finishedParent Issue
See parent analysis report: #2983
Related to #2983