git.iko.soy/crush
on main help sign in
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.

commitb8989b964abf98e0e09b4c4c2f2af49ccc608f80 authorKieran Klukas <kieran@dunkirk.sh> dateMon 11 May 2026, 10:42 (5 months ago) parent8e7451f
2 files changed+9−29
compare with main
unifiedsplit
internal/skills/skills.go−2 internal/skills/skills_test.go+9−27
internal/skills/skills.go−2
@@ -289,8 +289,6 @@ func DiscoverWithStates(paths []string) ([]*Skill, []*SkillState) {
289289 return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name))
290290 })
291291
292− // Publish states as-is; the coordinator will merge and re-sort them later.
293− broker.Publish(pubsub.UpdatedEvent, Event{States: states})
294292 return skills, states
295293}
296294
internal/skills/skills_test.go+9−27
@@ -1,7 +1,6 @@
11package skills
22
33import (
4− "context"
54 "os"
65 "path/filepath"
76 "strings"
@@ -216,7 +215,8 @@ func TestSkillValidate(t *testing.T) {
216215}
217216
218217func TestDiscover(t *testing.T) {
219− // Not parallel: shares global broker with other Discover tests.
218+ t.Parallel()
219+
220220 tmpDir := t.TempDir()
221221
222222 // Create valid skill 1.
@@ -248,14 +248,8 @@ description: Name doesn't match directory.
248248---
249249`), 0o644))
250250
251− ctx, cancel := context.WithCancel(context.Background())
252− defer cancel()
253− ch := SubscribeEvents(ctx)
254−
255− skills := Discover([]string{tmpDir})
251+ skills, states := DiscoverWithStates([]string{tmpDir})
256252
257− evt := <-ch
258− states := evt.Payload.States
259253 var normalCount int
260254 var errorCount int
261255 var hasInvalidDir bool
@@ -285,32 +279,20 @@ description: Name doesn't match directory.
285279}
286280
287281func TestDiscoverEmptyDir(t *testing.T) {
288− // Not parallel: shares global broker with other Discover tests.
282+ t.Parallel()
289283
290284 tmpDir := t.TempDir()
291285
292− ctx, cancel := context.WithCancel(context.Background())
293− defer cancel()
294− ch := SubscribeEvents(ctx)
295−
296− skills := Discover([]string{tmpDir})
297−
298− evt := <-ch
299− require.Empty(t, evt.Payload.States)
286+ skills, states := DiscoverWithStates([]string{tmpDir})
287+ require.Empty(t, states)
300288 require.Empty(t, skills)
301289}
302290
303291func TestDiscoverMissingPath(t *testing.T) {
304− // Not parallel: shares global broker with other Discover tests.
305−
306− ctx, cancel := context.WithCancel(context.Background())
307− defer cancel()
308− ch := SubscribeEvents(ctx)
309−
310− skills := Discover([]string{filepath.Join(t.TempDir(), "missing")})
292+ t.Parallel()
311293
312− evt := <-ch
313− require.Empty(t, evt.Payload.States)
294+ skills, states := DiscoverWithStates([]string{filepath.Join(t.TempDir(), "missing")})
295+ require.Empty(t, states)
314296 require.Empty(t, skills)
315297}
316298