fix: address lint and data race in annotation overrides

- httpapi: remove redundant nil check before len (S1009) and simplify restartRequired assignment (QF1007)
- config: remove unused helpers annotationsEqual/boolPtrEqual
- server: remove unused test helpers servedHints/boolVal
- server: fix UpdateServer data race by copy-on-write for runtime Config snapshot (mirrors configWithAppendedServer pattern)

Author: AGI Developer

internal/config/annotation_overrides.go

@@ -61,43 +61,6 @@ func EffectiveAnnotationsForTool(overrides map[string]*ToolAnnotations, toolName
 	return base
 }
 
-// annotationsEqual reports value equality for two ToolAnnotations, comparing
-// Title and dereferenced *bool hints (nil == nil). Pointer identity is ignored.
-func annotationsEqual(a, b *ToolAnnotations) bool {
-	if a == nil && b == nil {
-		return true
-	}
-	if a == nil || b == nil {
-		return false
-	}
-	if a.Title != b.Title {
-		return false
-	}
-	if !boolPtrEqual(a.ReadOnlyHint, b.ReadOnlyHint) {
-		return false
-	}
-	if !boolPtrEqual(a.DestructiveHint, b.DestructiveHint) {
-		return false
-	}
-	if !boolPtrEqual(a.IdempotentHint, b.IdempotentHint) {
-		return false
-	}
-	if !boolPtrEqual(a.OpenWorldHint, b.OpenWorldHint) {
-		return false
-	}
-	return true
-}
-
-func boolPtrEqual(a, b *bool) bool {
-	if a == nil && b == nil {
-		return true
-	}
-	if a == nil || b == nil {
-		return false
-	}
-	return *a == *b
-}
-
 // CloneAnnotationOverrides deep-copies an annotation_overrides map (values and
 // their *bool hints) so hot-reload snapshots never alias the caller's map.
 // Nil in, nil out; nil values are preserved as nil.

internal/httpapi/server.go

@@ -2655,7 +2655,7 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
 	// Validate annotation overrides via ValidateDetailed (write gate) before
 	// persisting. Reuses the PATCH pattern: temporary config with the new
 	// server appended, filtered to annotation_overrides errors.
-	if serverConfig.AnnotationOverrides != nil && len(serverConfig.AnnotationOverrides) > 0 {
+	if len(serverConfig.AnnotationOverrides) > 0 {
 		cfg, err := s.controller.GetConfig()
 		if err != nil {
 			s.writeError(w, r, http.StatusInternalServerError, fmt.Sprintf("Failed to load current config for annotation validation: %v", err))
@@ -2705,7 +2705,7 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
 	}
 
 	logger.Infow("Server added successfully", "server", req.Name, "quarantined", quarantined)
-	if s.auditSink != nil && serverConfig.AnnotationOverrides != nil && len(serverConfig.AnnotationOverrides) > 0 {
+	if s.auditSink != nil && len(serverConfig.AnnotationOverrides) > 0 {
 		afterMap := httpAnnotationOverridesToAuditMap(serverConfig.AnnotationOverrides)
 		if len(afterMap) > 0 {
 			if line, lerr := audit.NewConfigChange(audit.ConfigChangeInput{
@@ -3249,10 +3249,7 @@ func (s *Server) handlePatchServer(w http.ResponseWriter, r *http.Request) {
 			}
 		}
 	}
-	restartRequired := true
-	if onlyAnnotationOverrideHot {
-		restartRequired = false
-	}
+	restartRequired := !onlyAnnotationOverrideHot
 	s.writeSuccess(w, map[string]interface{}{
 		"message":          fmt.Sprintf("Server '%s' updated successfully", serverName),
 		"restart_required": restartRequired,

internal/server/annotation_override_immediate_refresh_test.go

@@ -34,37 +34,8 @@ import (
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime"
 )
 
-// servedHints returns the (destructiveHint, readOnlyHint) the production GET
-// path would serve for the named tool (Runtime.GetServerTools → StateView
-// snapshot). The snapshot carries *config.ToolAnnotations, not JSON maps.
-func servedHints(t *testing.T, rt *runtime.Runtime, server, tool string) (destructive, readOnly *bool) {
-	t.Helper()
-	tools, err := rt.GetServerTools(server)
-	require.NoError(t, err)
-	for _, tm := range tools {
-		if tm["name"] == tool {
-			ann, ok := tm["annotations"].(*config.ToolAnnotations)
-			require.True(t, ok, "served annotations must be *config.ToolAnnotations, got %T", tm["annotations"])
-			if ann == nil {
-				return nil, nil
-			}
-			return ann.DestructiveHint, ann.ReadOnlyHint
-		}
-	}
-	t.Fatalf("tool %q not served for server %q", tool, server)
-	return nil, nil
-}
-
-func boolVal(b *bool) interface{} {
-	if b == nil {
-		return nil
-	}
-	return *b
-}
-
 // servedAnnotations is the map form the existing test body expects (keys
-// "destructiveHint"/"readOnlyHint" etc). Kept for the test assertions that
-// compare via map lookup; built from the same StateView snapshot as servedHints.
+// "destructiveHint"/"readOnlyHint" etc) for assertions that compare via map lookup.
 func servedAnnotations(t *testing.T, rt *runtime.Runtime, server, tool string) map[string]interface{} {
 	t.Helper()
 	tools, err := rt.GetServerTools(server)

internal/server/server.go

@@ -2282,16 +2282,19 @@ func (s *Server) PatchServer(ctx context.Context, serverName string, resolve fun
 		}
 	}
 
-	// Update runtime config
-	currentConfig := s.runtime.Config()
-	if currentConfig != nil {
-		for i, sc := range currentConfig.Servers {
+	// Update runtime config — copy-on-write: Config() is the published
+	// snapshot that DiscoverAndIndexTools and others range over without a
+	// lock, so mutating its Servers slice in place is a DATA RACE.
+	if currentConfig := s.runtime.Config(); currentConfig != nil {
+		updated := *currentConfig
+		updated.Servers = append([]*config.ServerConfig(nil), currentConfig.Servers...)
+		for i, sc := range updated.Servers {
 			if sc.Name == serverName {
-				currentConfig.Servers[i] = existing
+				updated.Servers[i] = existing
 				break
 			}
 		}
-		s.runtime.UpdateConfig(currentConfig, "")
+		s.runtime.UpdateConfig(&updated, "")
 	}
 
 	// Push the refreshed config to the live manager client so hot-reloadable