From b8989b964abf98e0e09b4c4c2f2af49ccc608f80 Mon Sep 17 00:00:00 2001
From: Kieran Klukas <kieran@dunkirk.sh>
Date: Mon, 11 May 2026 10:42:43 -0400
Subject: [PATCH] refactor(skills): make coordinator the sole skill discovery
 publisher

Remove the auto-publish from DiscoverWithStates so that
discoverSkills in the coordinator is the only code path that
emits skill discovery events. This eliminates the double-publish
where DiscoverWithStates would emit user-only states, followed
by the coordinator emitting the merged builtin+user states.

Update the Discover tests to call DiscoverWithStates directly
and assert on the returned states slice, rather than subscribing
to the pubsub broker. This also lets those tests run in parallel.
---
 internal/skills/skills.go      |  2 --
 internal/skills/skills_test.go | 36 +++++++++-------------------------
 2 files changed, 9 insertions(+), 29 deletions(-)

diff --git a/internal/skills/skills.go b/internal/skills/skills.go
index 4b9d32d69038c11a315c54e41bd0ce01ac87391f..7071ba61820158a5324a48fbb5faf5cc89bd5f38 100644
--- a/internal/skills/skills.go
+++ b/internal/skills/skills.go
@@ -289,8 +289,6 @@ func DiscoverWithStates(paths []string) ([]*Skill, []*SkillState) {
 		return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
 	})
 
-	// Publish states as-is; the coordinator will merge and re-sort them later.
-	broker.Publish(pubsub.UpdatedEvent, Event{States: states})
 	return skills, states
 }
 
diff --git a/internal/skills/skills_test.go b/internal/skills/skills_test.go
index 34a70c40a2e1479a7c86f3b86e4e02a2e551bf23..f11a48c37f58fe727da97a90939efdedd0189250 100644
--- a/internal/skills/skills_test.go
+++ b/internal/skills/skills_test.go
@@ -1,7 +1,6 @@
 package skills
 
 import (
-	"context"
 	"os"
 	"path/filepath"
 	"strings"
@@ -216,7 +215,8 @@ func TestSkillValidate(t *testing.T) {
 }
 
 func TestDiscover(t *testing.T) {
-	// Not parallel: shares global broker with other Discover tests.
+	t.Parallel()
+
 	tmpDir := t.TempDir()
 
 	// Create valid skill 1.
@@ -248,14 +248,8 @@ description: Name doesn't match directory.
 ---
 `), 0o644))
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	ch := SubscribeEvents(ctx)
-
-	skills := Discover([]string{tmpDir})
+	skills, states := DiscoverWithStates([]string{tmpDir})
 
-	evt := <-ch
-	states := evt.Payload.States
 	var normalCount int
 	var errorCount int
 	var hasInvalidDir bool
@@ -285,32 +279,20 @@ description: Name doesn't match directory.
 }
 
 func TestDiscoverEmptyDir(t *testing.T) {
-	// Not parallel: shares global broker with other Discover tests.
+	t.Parallel()
 
 	tmpDir := t.TempDir()
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	ch := SubscribeEvents(ctx)
-
-	skills := Discover([]string{tmpDir})
-
-	evt := <-ch
-	require.Empty(t, evt.Payload.States)
+	skills, states := DiscoverWithStates([]string{tmpDir})
+	require.Empty(t, states)
 	require.Empty(t, skills)
 }
 
 func TestDiscoverMissingPath(t *testing.T) {
-	// Not parallel: shares global broker with other Discover tests.
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	ch := SubscribeEvents(ctx)
-
-	skills := Discover([]string{filepath.Join(t.TempDir(), "missing")})
+	t.Parallel()
 
-	evt := <-ch
-	require.Empty(t, evt.Payload.States)
+	skills, states := DiscoverWithStates([]string{filepath.Join(t.TempDir(), "missing")})
+	require.Empty(t, states)
 	require.Empty(t, skills)
 }