feat(dashboard): implement remote contributor registration and management with auto-filtering for local services

Author: juicycleff

extensions/dashboard/contributor/manifest.go

@@ -1,6 +1,10 @@
 package contributor
 
-import "github.com/a-h/templ"
+import (
+	"encoding/json"
+
+	"github.com/a-h/templ"
+)
 
 // Manifest describes a contributor's capabilities, navigation, widgets, settings, and more.
 type Manifest struct {
@@ -188,3 +192,25 @@ type AuthPageDef struct {
 	Icon     string `json:"icon"`     // optional icon name
 	Priority int    `json:"priority"` // sort order
 }
+
+// ManifestsEqual compares two manifests for change detection (used by remote
+// refresh paths). Implemented via JSON serialisation to avoid a reflect-deep
+// compare over templ.Component fields, which are nil on remote-fetched
+// manifests anyway and would compare unequally otherwise.
+func ManifestsEqual(a, b *Manifest) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+
+	ab, err := json.Marshal(a)
+	if err != nil {
+		return false
+	}
+
+	bb, err := json.Marshal(b)
+	if err != nil {
+		return false
+	}
+
+	return string(ab) == string(bb)
+}

extensions/dashboard/discovery/integration.go

@@ -2,7 +2,6 @@ package discovery
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"sync"
 	"time"
@@ -69,11 +68,19 @@ type Integration struct {
 	proxyTimeout time.Duration
 	logger       forge.Logger
 
+	// localServiceIDs holds service IDs that belong to the host process. Any
+	// instance whose ID matches is filtered out before registration so the
+	// dashboard never tries to fetch a contributor manifest from itself
+	// (memory-backed discovery surfaces the host's own service registration
+	// alongside any peers it learns about).
+	localServiceIDs map[string]struct{}
+
 	// tracked keeps track of remote contributors we've registered
 	tracked map[string]string // service ID → contributor name
 
-	stopCh chan struct{}
-	wg     sync.WaitGroup
+	stopOnce sync.Once
+	stopCh   chan struct{}
+	wg       sync.WaitGroup
 }
 
 // NewIntegration creates a new discovery integration.
@@ -97,8 +104,30 @@ func NewIntegration(
 	}
 }
 
+// LocalServiceIDProvider is the optional interface a discovery service
+// implements when it can report the host process's own service ID. Used so
+// memory-backed discovery doesn't make the dashboard try to fetch a
+// contributor manifest from itself.
+type LocalServiceIDProvider interface {
+	LocalServiceID() string
+}
+
 // Start begins polling the discovery service for dashboard contributors.
 func (i *Integration) Start(ctx context.Context) {
+	// Auto-filter the host's own service ID when the discovery service can
+	// report it. Memory-backed discovery surfaces the local registration in
+	// every poll; without this guard the dashboard would attempt to fetch a
+	// contributor manifest from itself.
+	if provider, ok := i.discovery.(LocalServiceIDProvider); ok {
+		if id := provider.LocalServiceID(); id != "" {
+			i.IgnoreLocalService(id)
+
+			i.logger.Debug("discovery integration: filtering local service ID",
+				forge.F("local_service_id", id),
+			)
+		}
+	}
+
 	i.wg.Add(1)
 
 	go i.pollLoop(ctx)
@@ -109,12 +138,34 @@ func (i *Integration) Start(ctx context.Context) {
 	)
 }
 
-// Stop stops the discovery integration.
+// Stop stops the discovery integration. Safe to call multiple times — the
+// dashboard extension's Stop() can fire more than once (forge calls Stop on
+// the extension and the DI container Stop walks the same instance again).
 func (i *Integration) Stop() {
-	close(i.stopCh)
-	i.wg.Wait()
+	i.stopOnce.Do(func() {
+		close(i.stopCh)
+		i.wg.Wait()
+
+		i.logger.Info("discovery integration stopped")
+	})
+}
+
+// IgnoreLocalService instructs the integration to skip a service ID when
+// scanning discovery. This is used to filter the host's own self-registration
+// out of dashboard contributor candidates. Must be called before Start.
+func (i *Integration) IgnoreLocalService(serviceID string) {
+	if serviceID == "" {
+		return
+	}
+
+	i.mu.Lock()
+	defer i.mu.Unlock()
+
+	if i.localServiceIDs == nil {
+		i.localServiceIDs = make(map[string]struct{})
+	}
 
-	i.logger.Info("discovery integration stopped")
+	i.localServiceIDs[serviceID] = struct{}{}
 }
 
 // pollLoop periodically checks discovery for services with the dashboard tag.
@@ -165,6 +216,14 @@ func (i *Integration) reconcile(ctx context.Context) {
 				continue
 			}
 
+			i.mu.Lock()
+			_, isLocal := i.localServiceIDs[inst.ID]
+			i.mu.Unlock()
+
+			if isLocal {
+				continue
+			}
+
 			foundIDs[inst.ID] = true
 
 			i.mu.Lock()
@@ -269,7 +328,7 @@ func (i *Integration) refreshRemoteService(ctx context.Context, inst *ServiceIns
 	}
 
 	current, ok := i.registry.GetManifest(contribName)
-	if ok && manifestsEqual(current, manifest) {
+	if ok && contributor.ManifestsEqual(current, manifest) {
 		return
 	}
 
@@ -303,26 +362,6 @@ func (i *Integration) refreshRemoteService(ctx context.Context, inst *ServiceIns
 	)
 }
 
-// manifestsEqual compares two manifests via JSON serialisation. This avoids a
-// deep reflect equality check over templ.Component fields (which are nil on
-// remote-fetched manifests anyway).
-func manifestsEqual(a, b *contributor.Manifest) bool {
-	if a == nil || b == nil {
-		return a == b
-	}
-
-	ab, err := json.Marshal(a)
-	if err != nil {
-		return false
-	}
-
-	bb, err := json.Marshal(b)
-	if err != nil {
-		return false
-	}
-
-	return string(ab) == string(bb)
-}
 
 // TrackedCount returns the number of tracked remote contributors.
 func (i *Integration) TrackedCount() int {

extensions/dashboard/discovery/integration_test.go

@@ -20,10 +20,22 @@ import (
 )
 
 // fakeDiscovery is a deterministic DiscoveryService used to drive reconcile.
+// When localID is set the fake also implements LocalServiceIDProvider so the
+// auto-filter path in Start can be exercised.
 type fakeDiscovery struct {
 	mu        sync.Mutex
 	services  []string
 	instances map[string][]*ServiceInstance // keyed by service name
+	localID   string
+}
+
+// LocalServiceID makes fakeDiscovery satisfy LocalServiceIDProvider when
+// localID is non-empty.
+func (f *fakeDiscovery) LocalServiceID() string {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	return f.localID
 }
 
 func newFakeDiscovery() *fakeDiscovery {
@@ -359,24 +371,79 @@ func TestIntegration_SkipsServicesWithoutTag(t *testing.T) {
 	}
 }
 
+func TestIntegration_AutoFiltersLocalServiceID(t *testing.T) {
+	manifest := &contributor.Manifest{Name: "authsome", Layout: "extension"}
+	ms := newManifestServer(t, manifest)
+
+	registry := contributor.NewContributorRegistry("/dashboard")
+
+	disc := newFakeDiscovery()
+	disc.localID = "Portal-self-1"
+	disc.set("Portal", instanceFor(t, ms, "Portal-self-1", true))
+	disc.set("identity", instanceFor(t, ms, "identity-1", true))
+
+	integ := NewIntegration(
+		disc,
+		registry,
+		"forge-dashboard-contributor",
+		60*time.Second,
+		2*time.Second,
+		forge.NewNoopLogger(),
+	)
+
+	// Mimic production wiring — Start() reads LocalServiceIDProvider and
+	// calls IgnoreLocalService internally. Drive that path here, then
+	// reconcile.
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	integ.Start(ctx)
+	defer integ.Stop()
+
+	// Force a reconcile pass; IgnoreLocalService was set inside Start.
+	integ.reconcile(ctx)
+
+	if _, ok := registry.GetManifest("authsome"); !ok {
+		t.Fatalf("identity should still be registered (only Portal-self-1 is filtered)")
+	}
+
+	// The Portal-self-1 service used the same manifest server, so if the
+	// filter weren't active, the registry would have a contributor named
+	// "authsome" registered against Portal's service ID. Since the manifest
+	// shares one name across services, we instead assert via the tracked
+	// map that Portal-self-1 is NOT being polled.
+	integ.mu.Lock()
+	_, portalTracked := integ.tracked["Portal-self-1"]
+	_, identityTracked := integ.tracked["identity-1"]
+	integ.mu.Unlock()
+
+	if portalTracked {
+		t.Errorf("Portal-self-1 was tracked despite local-service filter")
+	}
+
+	if !identityTracked {
+		t.Errorf("identity-1 should be tracked")
+	}
+}
+
 func TestManifestsEqual_DiffersOnNavChange(t *testing.T) {
 	a := &contributor.Manifest{Name: "x", Nav: []contributor.NavItem{{Path: "/"}}}
 	b := &contributor.Manifest{Name: "x", Nav: []contributor.NavItem{{Path: "/"}}}
 	c := &contributor.Manifest{Name: "x", Nav: []contributor.NavItem{{Path: "/"}, {Path: "/users"}}}
 
-	if !manifestsEqual(a, b) {
+	if !contributor.ManifestsEqual(a, b) {
 		t.Errorf("identical manifests should be equal")
 	}
 
-	if manifestsEqual(a, c) {
+	if contributor.ManifestsEqual(a, c) {
 		t.Errorf("manifests with different nav should not be equal")
 	}
 
-	if !manifestsEqual(nil, nil) {
+	if !contributor.ManifestsEqual(nil, nil) {
 		t.Errorf("nil/nil should be equal")
 	}
 
-	if manifestsEqual(a, nil) {
+	if contributor.ManifestsEqual(a, nil) {
 		t.Errorf("nil/non-nil should not be equal")
 	}
 }