[duplicate-code] Duplicate Code Pattern: Delegation Control-Plane Handler Wiring in server and proxy #12943

Part of duplicate code analysis: #12942

Summary

internal/server and internal/proxy each define their own delegation state and their own thin ControlHandler()/handleDelegationControl() pair that adapts that state into delegation.HandleControl (the shared handler added in commit fa3123e / PR #12923). The two adapters are structurally identical: gate the private control path, forward to a package-specific handler, and build a delegation.ControlDeps{Store, Capability, StatePath} from three near-identical fields.

Duplication Details

Pattern: Duplicated control-plane adapter methods

• Severity: Medium
• Occurrences: 2 (one per package)
• Locations:
  • internal/server/delegation.go (lines 170-189): (us *UnifiedServer) ControlHandler() and (us *UnifiedServer) handleDelegationControl(...)
  • internal/proxy/delegation.go (lines 40-63): (h *proxyHandler) handleDelegationControl(...) and (s *Server) ControlHandler()
• Code Sample (server variant):

  func (us *UnifiedServer) ControlHandler() http.Handler {
      return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
          if !us.delegationEnabled() || !strings.HasPrefix(r.URL.Path, delegation.ControlPathPrefix) {
              http.NotFound(w, r)
              return
          }
          us.handleDelegationControl(w, r)
      })
  }
  
  func (us *UnifiedServer) handleDelegationControl(w http.ResponseWriter, r *http.Request) {
      delegation.HandleControl(w, r, delegation.ControlDeps{
          Store:      us.delegation.Store,
          Capability: us.delegation.Capability,
          StatePath:  us.delegation.StatePath,
      }, logServerDelegation.Printf)
  }

  Proxy variant (internal/proxy/delegation.go):

  func (h *proxyHandler) handleDelegationControl(w http.ResponseWriter, r *http.Request) {
      delegation.HandleControl(w, r, delegation.ControlDeps{
          Store:      h.server.delegation.store,
          Capability: h.server.delegation.capability,
          StatePath:  h.server.delegation.statePath,
      }, logDelegation.Printf)
  }
  
  func (s *Server) ControlHandler() http.Handler {
      handler := &proxyHandler{server: s, CachedTracer: tracing.CachedTracer{Tracer: tracing.Tracer()}}
      return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
          if s.delegation == nil || !strings.HasPrefix(r.URL.Path, delegationControlPath) {
              http.NotFound(w, r)
              return
          }
          handler.handleDelegationControl(w, r)
      })
  }

The only real differences are: the receiver type (*UnifiedServer vs *Server/*proxyHandler), the field names on the private delegation-state struct (Store/Capability/StatePath vs store/capability/statePath), and which package-local logger is passed in (logServerDelegation.Printf vs logDelegation.Printf).

Impact Analysis

• Maintainability: Any future change to the control-plane wiring (e.g. adding a metrics hook, changing the not-found behavior, or adding request size limits before dispatch) has to be applied identically in both packages, and nothing enforces that today.
• Bug Risk: Low but non-zero — it would be easy for a future edit to touch only one of the two copies (as already nearly happened with the review feedback fixed by PR Fix delegation control success logging #12923, which centralized the handler body but left the wiring duplicated).
• Code Bloat: ~35 lines duplicated in structure (not byte-identical, but the same shape) across two files.

Refactoring Recommendations

1. Extract a small shared helper in internal/delegation that both packages call, e.g.:

   // internal/delegation/control_handler.go
   func NewControlHTTPHandler(deps ControlDeps, logf ControlLogger) http.Handler {
       return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
           if deps.Store == nil || !strings.HasPrefix(r.URL.Path, ControlPathPrefix) {
               http.NotFound(w, r)
               return
           }
           HandleControl(w, r, deps, logf)
       })
   }

   Then internal/server and internal/proxy would each reduce their ControlHandler() to a one-line call that builds ControlDeps from their own state and passes their own logger — eliminating the duplicated http.HandlerFunc wrapper and not-found gating in both places.

   • Estimated effort: ~30 minutes, low risk (pure refactor, existing tests in internal/proxy/delegation_control_test.go and internal/server/delegation_test.go should cover behavior unchanged)
   • Benefits: single source of truth for "is delegation enabled + does path match" gating logic; new callers (if any) get it for free.

2. Optionally normalize the two private delegation-state structs (internal/server's use of *delegation.RuntimeConfig directly vs internal/proxy's bespoke delegationState with lowercase fields) so both packages pass the same shape into ControlDeps construction, removing the need for the wrapper struct in internal/proxy/delegation.go entirely.

Implementation Checklist

• Review duplication findings
• Prioritize refactoring tasks
• Create refactoring plan
• Implement changes
• Update tests
• Verify no functionality broken

Parent Issue

See parent analysis report: #12942
Related to #12942

Generated by Duplicate Code Detector · copilot · auto · 120.4 AIC · ⊞ 12.2K · ◷

• expires on Sep 18, 2026, 2:46 AM UTC