manager_test.go

  1package skills
  2
  3import (
  4	"context"
  5	"errors"
  6	"os"
  7	"path/filepath"
  8	"testing"
  9	"time"
 10
 11	"github.com/stretchr/testify/require"
 12)
 13
 14func TestManager_NoGlobalMirrorByDefault(t *testing.T) {
 15	// Not parallel - touches package-level cache.
 16	prev := GetLatestStates()
 17	t.Cleanup(func() { SetLatestStates(prev) })
 18
 19	SetLatestStates(nil)
 20
 21	mgrA := NewManager(nil, nil, []*SkillState{{Name: "a", State: StateNormal}})
 22	mgrB := NewManager(nil, nil, []*SkillState{{Name: "b", State: StateNormal}})
 23
 24	mgrA.PublishStates(mgrA.States())
 25	mgrB.PublishStates(mgrB.States())
 26
 27	// Without WithGlobalMirror, the package-level cache must not be
 28	// touched by manager construction or PublishStates calls.
 29	require.Nil(t, GetLatestStates(), "package global must remain untouched")
 30	require.Equal(t, "a", mgrA.States()[0].Name)
 31	require.Equal(t, "b", mgrB.States()[0].Name)
 32}
 33
 34func TestManager_GlobalMirror(t *testing.T) {
 35	// Not parallel - touches package-level cache.
 36	prev := GetLatestStates()
 37	t.Cleanup(func() { SetLatestStates(prev) })
 38
 39	SetLatestStates(nil)
 40
 41	mgr := NewManager(nil, nil, []*SkillState{{Name: "x", State: StateNormal}}, WithGlobalMirror())
 42
 43	got := GetLatestStates()
 44	require.Len(t, got, 1)
 45	require.Equal(t, "x", got[0].Name)
 46
 47	// PublishStates with mirror enabled forwards to the global cache.
 48	mgr.SetLatestStates([]*SkillState{{Name: "y", State: StateNormal}})
 49	got = GetLatestStates()
 50	require.Len(t, got, 1)
 51	require.Equal(t, "y", got[0].Name)
 52}
 53
 54func TestManager_PublishStatesUpdatesCache(t *testing.T) {
 55	// Not parallel - exercises WithGlobalMirror, which touches the
 56	// package-level cache.
 57	prev := GetLatestStates()
 58	t.Cleanup(func() { SetLatestStates(prev) })
 59
 60	SetLatestStates(nil)
 61
 62	mgr := NewManager(nil, nil, []*SkillState{{Name: "old"}}, WithGlobalMirror())
 63	t.Cleanup(mgr.Shutdown)
 64
 65	// PublishStates must update every observable snapshot, not just the
 66	// SSE subscribers: Manager.States() (used by workspaceToProto on
 67	// the backend) and skills.GetLatestStates() (read by the TUI on the
 68	// client process and in local mode) must reflect the new value.
 69	mgr.PublishStates([]*SkillState{{Name: "new"}})
 70
 71	got := mgr.States()
 72	require.Len(t, got, 1)
 73	require.Equal(t, "new", got[0].Name)
 74
 75	cached := GetLatestStates()
 76	require.Len(t, cached, 1)
 77	require.Equal(t, "new", cached[0].Name)
 78}
 79
 80func TestManager_SubscribeReceivesPublishedStates(t *testing.T) {
 81	t.Parallel()
 82
 83	mgr := NewManager(nil, nil, nil)
 84	t.Cleanup(mgr.Shutdown)
 85
 86	ctx, cancel := context.WithCancel(context.Background())
 87	t.Cleanup(cancel)
 88	ch := mgr.SubscribeEvents(ctx)
 89
 90	want := []*SkillState{{Name: "k", State: StateNormal}}
 91	go mgr.PublishStates(want)
 92
 93	select {
 94	case ev := <-ch:
 95		require.Equal(t, "k", ev.Payload.States[0].Name)
 96	case <-time.After(2 * time.Second):
 97		t.Fatal("timed out waiting for manager event")
 98	}
 99}
100
101func TestManager_ConcurrentWorkspacesAreIsolated(t *testing.T) {
102	t.Parallel()
103
104	// Two managers without WithGlobalMirror should not see each other's
105	// events; this models the multi-workspace backend.
106	mgrA := NewManager(nil, nil, nil)
107	mgrB := NewManager(nil, nil, nil)
108	t.Cleanup(mgrA.Shutdown)
109	t.Cleanup(mgrB.Shutdown)
110
111	ctx, cancel := context.WithCancel(context.Background())
112	t.Cleanup(cancel)
113	chA := mgrA.SubscribeEvents(ctx)
114	chB := mgrB.SubscribeEvents(ctx)
115
116	go mgrA.PublishStates([]*SkillState{{Name: "from-a"}})
117
118	select {
119	case ev := <-chA:
120		require.Equal(t, "from-a", ev.Payload.States[0].Name)
121	case <-time.After(2 * time.Second):
122		t.Fatal("workspace A never received its own event")
123	}
124
125	select {
126	case ev := <-chB:
127		t.Fatalf("workspace B received workspace A's event: %v", ev)
128	case <-time.After(100 * time.Millisecond):
129		// Expected — B's stream is isolated.
130	}
131}
132
133func TestDiscoverFromConfig(t *testing.T) {
134	t.Parallel()
135
136	tmp := t.TempDir()
137	skillDir := filepath.Join(tmp, "custom-skill")
138	require.NoError(t, os.MkdirAll(skillDir, 0o755))
139	require.NoError(t, os.WriteFile(
140		filepath.Join(skillDir, SkillFileName),
141		[]byte("---\nname: custom-skill\ndescription: A custom skill for tests.\n---\nDo a thing.\n"),
142		0o644,
143	))
144
145	allSkills, activeSkills, states := DiscoverFromConfig(DiscoveryConfig{
146		SkillsPaths:    []string{tmp},
147		DisabledSkills: nil,
148	})
149
150	// Builtins plus our one custom skill.
151	require.NotEmpty(t, allSkills)
152	require.NotEmpty(t, activeSkills)
153	require.GreaterOrEqual(t, len(allSkills), 2)
154	require.GreaterOrEqual(t, len(activeSkills), 2)
155
156	// The custom skill is present with full Instructions populated, so
157	// the coordinator can render system prompts without re-walking the
158	// filesystem.
159	var custom *Skill
160	for _, s := range allSkills {
161		if s.Name == "custom-skill" {
162			custom = s
163			break
164		}
165	}
166	require.NotNil(t, custom)
167	require.NotEmpty(t, custom.Instructions, "DiscoverFromConfig must return Skill.Instructions")
168
169	// State snapshot includes the custom skill too.
170	foundCustom := false
171	for _, s := range states {
172		if s.Name == "custom-skill" {
173			foundCustom = true
174			require.Equal(t, StateNormal, s.State)
175		}
176	}
177	require.True(t, foundCustom, "states slice should include the custom skill")
178}
179
180func TestDiscoverFromConfig_DisabledFiltered(t *testing.T) {
181	t.Parallel()
182
183	tmp := t.TempDir()
184	skillDir := filepath.Join(tmp, "off-skill")
185	require.NoError(t, os.MkdirAll(skillDir, 0o755))
186	require.NoError(t, os.WriteFile(
187		filepath.Join(skillDir, SkillFileName),
188		[]byte("---\nname: off-skill\ndescription: Should be filtered.\n---\nx\n"),
189		0o644,
190	))
191
192	allSkills, activeSkills, states := DiscoverFromConfig(DiscoveryConfig{
193		SkillsPaths:    []string{tmp},
194		DisabledSkills: []string{"off-skill"},
195	})
196
197	// All discovered: yes; active: no.
198	hasInAll := false
199	for _, s := range allSkills {
200		if s.Name == "off-skill" {
201			hasInAll = true
202		}
203	}
204	require.True(t, hasInAll, "DisabledSkills must not be removed from allSkills")
205
206	for _, s := range activeSkills {
207		require.NotEqual(t, "off-skill", s.Name, "DisabledSkills must be removed from activeSkills")
208	}
209
210	// State snapshot still carries discovered entries (UI re-applies filter).
211	hasInStates := false
212	for _, s := range states {
213		if s.Name == "off-skill" {
214			hasInStates = true
215		}
216	}
217	require.True(t, hasInStates)
218}
219
220func TestDiscoverFromConfig_Resolver(t *testing.T) {
221	t.Parallel()
222
223	tmp := t.TempDir()
224	skillDir := filepath.Join(tmp, "envvar-skill")
225	require.NoError(t, os.MkdirAll(skillDir, 0o755))
226	require.NoError(t, os.WriteFile(
227		filepath.Join(skillDir, SkillFileName),
228		[]byte("---\nname: envvar-skill\ndescription: Env-resolved.\n---\nx\n"),
229		0o644,
230	))
231
232	allSkills, _, _ := DiscoverFromConfig(DiscoveryConfig{
233		SkillsPaths: []string{"$CUSTOM_SKILLS_DIR"},
234		Resolver: func(s string) (string, error) {
235			if s == "$CUSTOM_SKILLS_DIR" {
236				return tmp, nil
237			}
238			return s, errors.New("unknown")
239		},
240	})
241
242	found := false
243	for _, s := range allSkills {
244		if s.Name == "envvar-skill" {
245			found = true
246		}
247	}
248	require.True(t, found, "DiscoverFromConfig must expand $VAR via Resolver")
249}