Remove redundant annotation-based reconcile triggers in config controllers

[ChrisJBurns]

Summary

The MCPExternalAuthConfig and ToolConfig controllers annotate MCPServer objects to trigger reconciliation when a referenced config changes. However, the MCPServer controller already has direct watches on these config types, making the annotation updates redundant and causing double reconciliation.

Mutating another controller's primary resource from a config controller is an anti-pattern in controller-runtime that creates implicit coupling and can cause update conflicts.

Severity: SHOULD FIX
Area: Controller Design
Breaking: No

Location

• cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go:172-186
• cmd/thv-operator/controllers/toolconfig_controller.go:158-170

Problem

// In MCPExternalAuthConfig controller:
annotations["toolhive.stacklok.com/auth-config-updated"] = time.Now().Format(...)
mcpServer.SetAnnotations(annotations)
r.Update(ctx, &mcpServer)

The MCPServer controller already has direct watches on these config types via Watches() or handler.EnqueueRequestsFromMapFunc. The annotation update triggers a second reconciliation on top of the one already triggered by the watch.

Impact

• Double reconciliation on every config change (once from watch, once from annotation update)
• Update conflicts if the MCPServer controller and config controller both try to update the MCPServer simultaneously
• Anti-pattern: config controllers should not mutate MCPServer objects
• Annotation timestamps prevent idempotent reconciliation (always triggers a diff)

Recommended Fix

1. Remove the annotation-based trigger logic from MCPExternalAuthConfig and ToolConfig controllers.
2. Verify that existing watch handlers on MCPServer controller correctly enqueue reconciliation when referenced configs change.
3. If watch handlers are missing, add them instead of using annotations.