[duplicate-code] Duplicate Code Pattern: Guard Policy Config Resolution in UnifiedServer

[github-actions]

Part of duplicate code analysis: #1948

Summary

resolveGuardPolicy and resolveWriteSinkPolicy in internal/server/unified.go share the same 4-step config navigation pattern: global override check → nil guard → server config lookup → per-server policy parse. The structural duplication means any change to how config is traversed must be applied in both methods.

Duplication Details

Pattern: global-override → cfg-nil-guard → serverCfg-lookup → parseServerGuardPolicy

• Severity: Medium
• Occurrences: 2 functions
• Locations:
  • internal/server/unified.go lines 1075–1117 (resolveGuardPolicy, 43 lines)
  • internal/server/unified.go lines 1119–1141 (resolveWriteSinkPolicy, 23 lines)

Shared skeleton:

// Step 1 – global override
if us.cfg != nil && us.cfg.GuardPolicy != nil {
    // (validate and) return global policy field
}
// Step 2 – nil guard
if us.cfg == nil {
    return (zero-value)
}
// Step 3 – server config lookup
serverCfg, ok := us.cfg.Servers[serverID]
if !ok || serverCfg == nil {
    return (zero-value)
}
// Step 4 – per-server policy
policy, err := parseServerGuardPolicy(serverID, serverCfg.GuardPolicies)
// ... extract and return relevant field

Impact Analysis

• Maintainability: Adding a new config source (e.g., environment-variable override, org-level policy) requires updating both functions independently
• Bug Risk: Medium — the nil-check ordering and config traversal logic could diverge silently
• Code Bloat: ~25 lines of structurally identical navigation scaffolding

Refactoring Recommendations

1. Extract resolveServerPolicy helper that returns the full *config.GuardPolicy from the 4-step traversal:

   func (us *UnifiedServer) resolveServerPolicy(serverID string) (*config.GuardPolicy, string, error)

   • resolveGuardPolicy calls this and returns the full policy + source
   • resolveWriteSinkPolicy calls this and returns only policy.WriteSink
   • Estimated effort: 1 hour
   • Benefits: single traversal path; easier to add new policy sources

2. Add validation call — currently resolveWriteSinkPolicy skips ValidateGuardPolicy before returning. Unifying the traversal would naturally expose this inconsistency for a fix.

Implementation Checklist

• Review duplication findings
• Extract shared config-traversal logic into a single helper
• Ensure resolveGuardPolicy still returns source string for audit logging
• Add ValidateGuardPolicy call to resolveWriteSinkPolicy path (currently absent)
• Run full test suite to verify guard policy resolution tests pass

Parent Issue

See parent analysis report: #1948
Related to #1948

Generated by Duplicate Code Detector · ◷

• expires on Mar 22, 2026, 3:06 AM UTC