/ internal / skills / loader_test.go
loader_test.go
  1  package skills
  2  
  3  import (
  4  	"os"
  5  	"path/filepath"
  6  	"strings"
  7  	"testing"
  8  )
  9  
 10  func createSkillDir(t *testing.T, base, name, content string) {
 11  	t.Helper()
 12  	dir := filepath.Join(base, name)
 13  	if err := os.MkdirAll(dir, 0o755); err != nil {
 14  		t.Fatal(err)
 15  	}
 16  	if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644); err != nil {
 17  		t.Fatal(err)
 18  	}
 19  }
 20  
 21  func TestLoadSkills_BasicParsing(t *testing.T) {
 22  	tmp := t.TempDir()
 23  	createSkillDir(t, tmp, "pdf", "---\nname: pdf\ndescription: Extract text from PDFs\nlicense: MIT\n---\n\n# PDF Processing\n\nUse pypdf to extract text.\n")
 24  
 25  	skills, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
 26  	if err != nil {
 27  		t.Fatalf("unexpected error: %v", err)
 28  	}
 29  	if len(skills) != 1 {
 30  		t.Fatalf("expected 1 skill, got %d", len(skills))
 31  	}
 32  	s := skills[0]
 33  	if s.Name != "pdf" {
 34  		t.Errorf("name = %q, want pdf", s.Name)
 35  	}
 36  	if s.Description != "Extract text from PDFs" {
 37  		t.Errorf("description = %q", s.Description)
 38  	}
 39  	if s.License != "MIT" {
 40  		t.Errorf("license = %q", s.License)
 41  	}
 42  	if !strings.Contains(s.Prompt, "# PDF Processing") {
 43  		t.Errorf("prompt missing body")
 44  	}
 45  	if s.Source != "global" {
 46  		t.Errorf("source = %q", s.Source)
 47  	}
 48  	if s.Dir != filepath.Join(tmp, "pdf") {
 49  		t.Errorf("dir = %q", s.Dir)
 50  	}
 51  }
 52  
 53  func TestLoadSkills_PriorityDedup(t *testing.T) {
 54  	agentDir := t.TempDir()
 55  	globalDir := t.TempDir()
 56  	createSkillDir(t, agentDir, "pdf", "---\nname: pdf\ndescription: Agent PDF\n---\nAgent version.")
 57  	createSkillDir(t, globalDir, "pdf", "---\nname: pdf\ndescription: Global PDF\n---\nGlobal version.")
 58  	createSkillDir(t, globalDir, "xlsx", "---\nname: xlsx\ndescription: Spreadsheet\n---\nXLSX.")
 59  
 60  	skills, err := LoadSkills(
 61  		SkillSource{Dir: agentDir, Source: "agent:mybot"},
 62  		SkillSource{Dir: globalDir, Source: "global"},
 63  	)
 64  	if err != nil {
 65  		t.Fatalf("error: %v", err)
 66  	}
 67  	if len(skills) != 2 {
 68  		t.Fatalf("expected 2, got %d", len(skills))
 69  	}
 70  	var pdf *Skill
 71  	for _, s := range skills {
 72  		if s.Name == "pdf" {
 73  			pdf = s
 74  		}
 75  	}
 76  	if pdf == nil {
 77  		t.Fatal("pdf not found")
 78  	}
 79  	if pdf.Source != "agent:mybot" {
 80  		t.Errorf("pdf source = %q", pdf.Source)
 81  	}
 82  	if !strings.Contains(pdf.Prompt, "Agent version") {
 83  		t.Error("agent pdf should win")
 84  	}
 85  }
 86  
 87  // TestLoadSkills_NameDifferentFromDir_Loads confirms the name/slug decoupling:
 88  // when frontmatter.name differs from the directory basename, the skill still
 89  // loads. This matches the openclaw/clawhub contract (slug is always derived
 90  // from the directory, name is a free-form label from frontmatter). Regression
 91  // target: ClawHub's xiaohongshu-mcp-skills package which ships with
 92  // `name: xiaohongshu` but installs under slug `xiaohongshu-mcp-skills`.
 93  func TestLoadSkills_NameDifferentFromDir_Loads(t *testing.T) {
 94  	tmp := t.TempDir()
 95  	createSkillDir(t, tmp, "xiaohongshu-mcp-skills", "---\nname: xiaohongshu\ndescription: Mismatch is fine\n---\nBody.")
 96  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
 97  	if err != nil {
 98  		t.Fatalf("LoadSkills should not error: %v", err)
 99  	}
100  	if len(loaded) != 1 {
101  		t.Fatalf("expected the skill to load, got %d skills", len(loaded))
102  	}
103  	if loaded[0].Name != "xiaohongshu" {
104  		t.Errorf("Name should come from frontmatter, got %q", loaded[0].Name)
105  	}
106  	if loaded[0].Slug != "xiaohongshu-mcp-skills" {
107  		t.Errorf("Slug should come from directory name, got %q", loaded[0].Slug)
108  	}
109  }
110  
111  // TestLoadSkills_FailOpenWithGoodAndBad guards the central fail-open contract:
112  // when one skill in a source is malformed (e.g. broken YAML frontmatter), the
113  // loader must skip it and still return every other valid skill in the same
114  // source. Regression target: a real user environment where one skill with a
115  // nested-map metadata block silently broke ALL global skill loading.
116  func TestLoadSkills_FailOpenWithGoodAndBad(t *testing.T) {
117  	tmp := t.TempDir()
118  
119  	// Valid skill — must survive.
120  	createSkillDir(t, tmp, "pdf", "---\nname: pdf\ndescription: Read PDFs\n---\n# PDF body")
121  
122  	// Broken skill — frontmatter parses fine but the description field is a
123  	// map instead of a string, mirroring the real-world failure observed in
124  	// the user's environment ("cannot unmarshal !!map into string"). This
125  	// MUST be skipped, not fatal.
126  	createSkillDir(t, tmp, "broken", "---\nname: broken\ndescription:\n  nested: oops\n---\n# body")
127  
128  	// Another valid skill loaded after the broken one in alphabetical order
129  	// (sorted by directory name): "pdf" comes before "broken"? No — "broken"
130  	// sorts before "pdf". So this confirms the loader recovers and continues
131  	// past a mid-stream failure rather than aborting.
132  	createSkillDir(t, tmp, "zebra", "---\nname: zebra\ndescription: Stripes\n---\n# zebra body")
133  
134  	skills, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
135  	if err != nil {
136  		t.Fatalf("LoadSkills must not error when only some skills are bad: %v", err)
137  	}
138  	if len(skills) != 2 {
139  		t.Fatalf("expected 2 valid skills (pdf, zebra) — broken should be skipped — got %d", len(skills))
140  	}
141  	got := map[string]bool{}
142  	for _, s := range skills {
143  		got[s.Name] = true
144  	}
145  	if !got["pdf"] || !got["zebra"] {
146  		t.Errorf("expected both pdf and zebra to load, got %v", got)
147  	}
148  	if got["broken"] {
149  		t.Error("broken skill should have been skipped, but it was loaded")
150  	}
151  }
152  
153  // TestLoadSkills_BrokenSkillDoesNotShadowLowerSource locks in the part of the
154  // fail-open fix that intentionally does NOT mark a broken skill as "seen".
155  // If a broken global skill shadowed a working bundled skill of the same name,
156  // users would be silently downgraded; instead, the bundled version must take
157  // over when the higher-priority source is malformed.
158  func TestLoadSkills_BrokenSkillDoesNotShadowLowerSource(t *testing.T) {
159  	highPrio := t.TempDir()
160  	lowPrio := t.TempDir()
161  
162  	// Higher-priority source has a broken `pdf` skill (missing required
163  	// description field, so the loader rejects it).
164  	createSkillDir(t, highPrio, "pdf", "---\nname: pdf\n---\n# broken: no description")
165  
166  	// Lower-priority source has a valid `pdf` skill.
167  	createSkillDir(t, lowPrio, "pdf", "---\nname: pdf\ndescription: Real PDF\n---\n# Real PDF")
168  
169  	skills, err := LoadSkills(
170  		SkillSource{Dir: highPrio, Source: "global"},
171  		SkillSource{Dir: lowPrio, Source: "bundled"},
172  	)
173  	if err != nil {
174  		t.Fatalf("LoadSkills returned error: %v", err)
175  	}
176  	if len(skills) != 1 {
177  		t.Fatalf("expected the bundled pdf to take over, got %d skills", len(skills))
178  	}
179  	if skills[0].Name != "pdf" {
180  		t.Errorf("expected name=pdf, got %q", skills[0].Name)
181  	}
182  	if skills[0].Description != "Real PDF" {
183  		t.Errorf("expected the bundled (working) skill to win, got description=%q", skills[0].Description)
184  	}
185  	if skills[0].Source != "bundled" {
186  		t.Errorf("expected source=bundled (broken global was skipped), got %q", skills[0].Source)
187  	}
188  }
189  
190  func TestLoadSkills_LegacyYAML(t *testing.T) {
191  	tmp := t.TempDir()
192  	os.WriteFile(filepath.Join(tmp, "old.yaml"), []byte("name: old"), 0o644)
193  	skills, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
194  	if err != nil {
195  		t.Fatalf("error: %v", err)
196  	}
197  	if len(skills) != 0 {
198  		t.Errorf("expected 0, got %d", len(skills))
199  	}
200  }
201  
202  func TestLoadSkills_EmptyDir(t *testing.T) {
203  	tmp := t.TempDir()
204  	skills, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
205  	if err != nil {
206  		t.Fatalf("error: %v", err)
207  	}
208  	if len(skills) != 0 {
209  		t.Errorf("expected 0, got %d", len(skills))
210  	}
211  }
212  
213  func TestLoadSkills_NonexistentDir(t *testing.T) {
214  	skills, err := LoadSkills(SkillSource{Dir: "/nonexistent", Source: "global"})
215  	if err != nil {
216  		t.Fatalf("error: %v", err)
217  	}
218  	if skills != nil {
219  		t.Errorf("expected nil")
220  	}
221  }
222  
223  func TestLoadSkills_Integration(t *testing.T) {
224  	agentDir := t.TempDir()
225  	globalDir := t.TempDir()
226  
227  	// Agent skill shadows global
228  	createSkillDir(t, agentDir, "pdf", "---\nname: pdf\ndescription: Agent PDF\n---\n# Agent PDF Guide")
229  	// Global skills
230  	createSkillDir(t, globalDir, "pdf", "---\nname: pdf\ndescription: Global PDF\n---\n# Global PDF Guide")
231  	createSkillDir(t, globalDir, "xlsx", "---\nname: xlsx\ndescription: Spreadsheet processing\n---\n# XLSX Guide")
232  
233  	loaded, err := LoadSkills(
234  		SkillSource{Dir: agentDir, Source: "agent:test"},
235  		SkillSource{Dir: globalDir, Source: "global"},
236  	)
237  	if err != nil {
238  		t.Fatalf("load error: %v", err)
239  	}
240  	if len(loaded) != 2 {
241  		t.Fatalf("expected 2 skills (deduped), got %d", len(loaded))
242  	}
243  
244  	var pdf, xlsx *Skill
245  	for _, s := range loaded {
246  		switch s.Name {
247  		case "pdf":
248  			pdf = s
249  		case "xlsx":
250  			xlsx = s
251  		}
252  	}
253  
254  	// Agent pdf shadows global
255  	if pdf == nil {
256  		t.Fatal("pdf not found")
257  	}
258  	if pdf.Source != "agent:test" {
259  		t.Errorf("pdf source = %q, want agent:test", pdf.Source)
260  	}
261  	if !strings.Contains(pdf.Prompt, "Agent PDF Guide") {
262  		t.Error("agent pdf should shadow global")
263  	}
264  
265  	// Global xlsx loaded
266  	if xlsx == nil {
267  		t.Fatal("xlsx not found")
268  	}
269  	if xlsx.Source != "global" {
270  		t.Errorf("xlsx source = %q, want global", xlsx.Source)
271  	}
272  
273  	// Sorted order
274  	if loaded[0].Name != "pdf" || loaded[1].Name != "xlsx" {
275  		t.Errorf("expected [pdf, xlsx], got [%s, %s]", loaded[0].Name, loaded[1].Name)
276  	}
277  }
278  
279  func TestLoadSkills_Sorted(t *testing.T) {
280  	tmp := t.TempDir()
281  	createSkillDir(t, tmp, "zebra", "---\nname: zebra\ndescription: Z\n---\nZ")
282  	createSkillDir(t, tmp, "alpha", "---\nname: alpha\ndescription: A\n---\nA")
283  	skills, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
284  	if err != nil {
285  		t.Fatalf("error: %v", err)
286  	}
287  	if len(skills) != 2 {
288  		t.Fatalf("expected 2, got %d", len(skills))
289  	}
290  	if skills[0].Name != "alpha" {
291  		t.Errorf("expected alpha first, got %s", skills[0].Name)
292  	}
293  }
294  
295  func TestLoadSkills_InstallProvenance(t *testing.T) {
296  	globalDir := t.TempDir()
297  	bundledDir := t.TempDir()
298  
299  	createSkillDir(t, globalDir, "local-skill", "---\nname: local-skill\ndescription: Local\n---\nlocal")
300  	createSkillDir(t, globalDir, "market-skill", "---\nname: market-skill\ndescription: Market\n---\nmarket")
301  	createSkillDir(t, bundledDir, "bundled-skill", "---\nname: bundled-skill\ndescription: Bundled\n---\nbundled")
302  
303  	if err := writeMarketplaceProvenance(filepath.Join(globalDir, "market-skill"), "market-skill"); err != nil {
304  		t.Fatalf("write provenance: %v", err)
305  	}
306  
307  	loaded, err := LoadSkills(
308  		SkillSource{Dir: globalDir, Source: SourceGlobal},
309  		SkillSource{Dir: bundledDir, Source: SourceBundled},
310  	)
311  	if err != nil {
312  		t.Fatalf("load error: %v", err)
313  	}
314  
315  	got := make(map[string]*Skill, len(loaded))
316  	for _, skill := range loaded {
317  		got[skill.Name] = skill
318  	}
319  
320  	if got["local-skill"].InstallSource != InstallSourceLocal {
321  		t.Errorf("local-skill install source = %q, want %q", got["local-skill"].InstallSource, InstallSourceLocal)
322  	}
323  	if got["local-skill"].MarketplaceSlug != "" {
324  		t.Errorf("local-skill marketplace slug = %q, want empty", got["local-skill"].MarketplaceSlug)
325  	}
326  
327  	if got["market-skill"].InstallSource != InstallSourceMarketplace {
328  		t.Errorf("market-skill install source = %q, want %q", got["market-skill"].InstallSource, InstallSourceMarketplace)
329  	}
330  	if got["market-skill"].MarketplaceSlug != "market-skill" {
331  		t.Errorf("market-skill marketplace slug = %q, want market-skill", got["market-skill"].MarketplaceSlug)
332  	}
333  
334  	if got["bundled-skill"].InstallSource != InstallSourceBundled {
335  		t.Errorf("bundled-skill install source = %q, want %q", got["bundled-skill"].InstallSource, InstallSourceBundled)
336  	}
337  }
338  
339  func TestLoadSkills_StickyInstructions_OptIn(t *testing.T) {
340  	tmp := t.TempDir()
341  	body := "---\nname: policy\ndescription: Policy skill\nsticky-instructions: true\n---\n\n# Policy\n\nRoute all platform operations through http://localhost:7533 — never edit ~/.shannon files directly.\n\nMore detail in later sections."
342  	createSkillDir(t, tmp, "policy", body)
343  
344  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
345  	if err != nil {
346  		t.Fatalf("LoadSkills: %v", err)
347  	}
348  	if len(loaded) != 1 {
349  		t.Fatalf("expected 1 skill, got %d", len(loaded))
350  	}
351  	s := loaded[0]
352  	if !s.StickyInstructions {
353  		t.Errorf("StickyInstructions = false, want true")
354  	}
355  	if s.StickySnippet == "" {
356  		t.Error("StickySnippet is empty, expected first-paragraph extraction")
357  	}
358  	if strings.HasPrefix(s.StickySnippet, "#") {
359  		t.Errorf("snippet should not start with an ATX heading: %q", s.StickySnippet)
360  	}
361  	if !strings.Contains(s.StickySnippet, "Route all platform operations") {
362  		t.Errorf("snippet missing expected body content: %q", s.StickySnippet)
363  	}
364  	if strings.Contains(s.StickySnippet, "\n") {
365  		t.Errorf("snippet should be single-line (newlines collapsed), got %q", s.StickySnippet)
366  	}
367  }
368  
369  func TestLoadSkills_StickyInstructions_DefaultFalse(t *testing.T) {
370  	tmp := t.TempDir()
371  	createSkillDir(t, tmp, "plain", "---\nname: plain\ndescription: Plain skill\n---\n# Heading\n\nBody text.")
372  
373  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
374  	if err != nil {
375  		t.Fatalf("LoadSkills: %v", err)
376  	}
377  	if len(loaded) != 1 {
378  		t.Fatalf("expected 1, got %d", len(loaded))
379  	}
380  	if loaded[0].StickyInstructions {
381  		t.Error("StickyInstructions should default to false when frontmatter omits it")
382  	}
383  }
384  
385  func TestLoadSkills_Hidden_OptIn(t *testing.T) {
386  	tmp := t.TempDir()
387  	createSkillDir(t, tmp, "kocoro", "---\nname: kocoro\ndescription: Policy skill\nhidden: true\n---\n# Body")
388  
389  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
390  	if err != nil {
391  		t.Fatalf("LoadSkills: %v", err)
392  	}
393  	if len(loaded) != 1 {
394  		t.Fatalf("expected 1 skill, got %d", len(loaded))
395  	}
396  	if !loaded[0].Hidden {
397  		t.Error("Hidden = false, want true when frontmatter sets hidden: true")
398  	}
399  	if !loaded[0].ToMeta().Hidden {
400  		t.Error("SkillMeta.Hidden = false, want true — ToMeta must propagate the flag")
401  	}
402  }
403  
404  func TestLoadSkills_Hidden_DefaultFalse(t *testing.T) {
405  	tmp := t.TempDir()
406  	createSkillDir(t, tmp, "plain", "---\nname: plain\ndescription: Plain skill\n---\n# Body")
407  
408  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
409  	if err != nil {
410  		t.Fatalf("LoadSkills: %v", err)
411  	}
412  	if len(loaded) != 1 {
413  		t.Fatalf("expected 1, got %d", len(loaded))
414  	}
415  	if loaded[0].Hidden {
416  		t.Error("Hidden should default to false when frontmatter omits it")
417  	}
418  	if loaded[0].ToMeta().Hidden {
419  		t.Error("SkillMeta.Hidden should default to false")
420  	}
421  }
422  
423  func TestLoadSkills_StickySnippet_TruncatedTo400(t *testing.T) {
424  	tmp := t.TempDir()
425  	// Build a long first paragraph (>400 chars) to exercise the cap.
426  	long := strings.Repeat("abcdefghij ", 60) // 660 chars
427  	body := "---\nname: long\ndescription: Long skill\nsticky-instructions: true\n---\n\n" + long
428  	createSkillDir(t, tmp, "long", body)
429  
430  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
431  	if err != nil {
432  		t.Fatalf("LoadSkills: %v", err)
433  	}
434  	if len(loaded) != 1 {
435  		t.Fatalf("expected 1, got %d", len(loaded))
436  	}
437  	runes := []rune(loaded[0].StickySnippet)
438  	if len(runes) > stickySnippetMaxChars {
439  		t.Errorf("snippet len=%d, want <= %d", len(runes), stickySnippetMaxChars)
440  	}
441  	if len(runes) < 10 {
442  		t.Errorf("snippet too short: %q", loaded[0].StickySnippet)
443  	}
444  	if !strings.HasSuffix(loaded[0].StickySnippet, "...") {
445  		t.Errorf("truncated snippet should end with '...', got %q", loaded[0].StickySnippet)
446  	}
447  }
448  
449  func TestLoadSkills_StickySnippet_FallsBackToDescription(t *testing.T) {
450  	tmp := t.TempDir()
451  	// Frontmatter-only SKILL.md — body is empty, so snippet should fall back.
452  	createSkillDir(t, tmp, "empty", "---\nname: empty\ndescription: Fallback description text\nsticky-instructions: true\n---\n")
453  
454  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
455  	if err != nil {
456  		t.Fatalf("LoadSkills: %v", err)
457  	}
458  	if len(loaded) != 1 {
459  		t.Fatalf("expected 1 skill, got %d", len(loaded))
460  	}
461  	if loaded[0].StickySnippet != "Fallback description text" {
462  		t.Errorf("expected description fallback, got %q", loaded[0].StickySnippet)
463  	}
464  }
465  
466  func TestExtractStickySnippet_SkipsHeadings(t *testing.T) {
467  	body := "# Title\n\n## Sub\n\nThe actual first guidance paragraph."
468  	got := extractStickySnippet(body)
469  	if got != "The actual first guidance paragraph." {
470  		t.Errorf("extractStickySnippet = %q", got)
471  	}
472  }
473  
474  func TestExtractStickySnippet_CollapsesNewlines(t *testing.T) {
475  	body := "Line one\nline two\nline three."
476  	got := extractStickySnippet(body)
477  	if got != "Line one line two line three." {
478  		t.Errorf("extractStickySnippet = %q", got)
479  	}
480  }
481  
482  // TestExtractStickySnippet_PrefersImperativeParagraph locks in the core
483  // reviewer-flagged bug: when a SKILL.md body has a bland intro paragraph
484  // followed by the actual policy ("ALL platform operations MUST go through
485  // ..."), the extractor must prefer the policy paragraph, not the intro.
486  func TestExtractStickySnippet_PrefersImperativeParagraph(t *testing.T) {
487  	body := "You help users manage the platform.\n\nALL platform operations go through the daemon HTTP API. Never edit config files directly.\n\nMore detail later."
488  	got := extractStickySnippet(body)
489  	if !strings.Contains(got, "ALL platform operations") {
490  		t.Errorf("expected imperative paragraph, got %q", got)
491  	}
492  	if strings.Contains(got, "You help users") {
493  		t.Errorf("should NOT select the bland intro paragraph, got %q", got)
494  	}
495  }
496  
497  func TestExtractStickySnippet_ImperativeMarkers(t *testing.T) {
498  	tests := []struct {
499  		name string
500  		body string
501  		want string // substring that must be in the returned snippet
502  	}{
503  		{"MUST", "Intro paragraph.\n\nYou MUST do X before Y.", "MUST do X"},
504  		{"NEVER caps", "Intro.\n\nNEVER run destructive commands.", "NEVER run"},
505  		{"DO NOT", "Intro.\n\nDO NOT bypass validation.", "DO NOT bypass"},
506  		{"DON'T", "Intro.\n\nDON'T touch prod.", "DON'T touch"},
507  		{"Never sentence-start", "Intro.\n\nNever commit without review.", "Never commit"},
508  		{"Use the ...", "Intro.\n\nUse the http tool for every operation.", "Use the http tool"},
509  		{"ZH 必须", "中性描述。\n\n必须通过 API 操作,不要直接改文件。", "必须通过"},
510  		{"ZH 绝不", "介绍段落。\n\n绝不要直接写 ~/.shannon 文件。", "绝不要"},
511  		{"JA 必ず", "汎用説明。\n\n必ずAPIを使用してください。", "必ずAPI"},
512  	}
513  	for _, tt := range tests {
514  		t.Run(tt.name, func(t *testing.T) {
515  			got := extractStickySnippet(tt.body)
516  			if !strings.Contains(got, tt.want) {
517  				t.Errorf("extractStickySnippet missed imperative paragraph, got %q (want substring %q)", got, tt.want)
518  			}
519  		})
520  	}
521  }
522  
523  func TestLoadSkills_StickySnippet_ExplicitOverride(t *testing.T) {
524  	tmp := t.TempDir()
525  	// Body has a clear imperative paragraph that SHOULD win via heuristic,
526  	// but the frontmatter explicitly pins a different snippet — override wins.
527  	body := "---\nname: over\ndescription: Override test\nsticky-instructions: true\nsticky-snippet: \"Explicit: use http tool only.\"\n---\n\nIntro.\n\nMUST not appear in snippet because override is set."
528  	createSkillDir(t, tmp, "over", body)
529  
530  	loaded, err := LoadSkills(SkillSource{Dir: tmp, Source: "global"})
531  	if err != nil {
532  		t.Fatalf("LoadSkills: %v", err)
533  	}
534  	if len(loaded) != 1 {
535  		t.Fatalf("expected 1, got %d", len(loaded))
536  	}
537  	if loaded[0].StickySnippet != "Explicit: use http tool only." {
538  		t.Errorf("expected explicit override, got %q", loaded[0].StickySnippet)
539  	}
540  }
541  
542  // TestLoadSkills_Kocoro_NotSticky asserts kocoro is intentionally NOT sticky.
543  // Sticky reminders re-inject a snippet at the start of every turn; for a
544  // task-driven skill like kocoro that body competes with the use_skill tool
545  // result for model attention, prompting the model to re-call use_skill to
546  // reconcile and tripping ConsecutiveDuplicate on turn 3. The opt-in was
547  // removed; this test pins the decision so future edits don't silently
548  // re-enable it.
549  func TestLoadSkills_Kocoro_NotSticky(t *testing.T) {
550  	wd, err := os.Getwd()
551  	if err != nil {
552  		t.Fatalf("getwd: %v", err)
553  	}
554  	bundledDir := filepath.Join(wd, "bundled", "skills")
555  	loaded, err := LoadSkills(SkillSource{Dir: bundledDir, Source: "bundled"})
556  	if err != nil {
557  		t.Fatalf("LoadSkills bundled: %v", err)
558  	}
559  	var kocoro *Skill
560  	for _, s := range loaded {
561  		if s.Name == "kocoro" {
562  			kocoro = s
563  			break
564  		}
565  	}
566  	if kocoro == nil {
567  		t.Fatalf("kocoro skill not found in bundled; loaded=%d", len(loaded))
568  	}
569  	if kocoro.StickyInstructions {
570  		t.Fatal("kocoro must NOT be sticky — re-injection competes with use_skill body and triggers ConsecutiveDuplicate loop")
571  	}
572  }
573  
574  func TestWriteGlobalSkillClearsMarketplaceProvenance(t *testing.T) {
575  	shannonDir := t.TempDir()
576  	skillDir := filepath.Join(shannonDir, "skills", "ontology")
577  
578  	if err := os.MkdirAll(skillDir, 0o755); err != nil {
579  		t.Fatalf("mkdir: %v", err)
580  	}
581  	if err := writeMarketplaceProvenance(skillDir, "ontology"); err != nil {
582  		t.Fatalf("write provenance: %v", err)
583  	}
584  
585  	err := WriteGlobalSkill(shannonDir, &Skill{
586  		Name:        "ontology",
587  		Description: "Local replacement",
588  		Prompt:      "# local body",
589  	})
590  	if err != nil {
591  		t.Fatalf("WriteGlobalSkill: %v", err)
592  	}
593  
594  	if _, err := os.Stat(filepath.Join(skillDir, marketplaceProvenanceFile)); !os.IsNotExist(err) {
595  		t.Fatalf("provenance marker should be removed, stat err = %v", err)
596  	}
597  
598  	loaded, err := LoadSkills(SkillSource{Dir: filepath.Join(shannonDir, "skills"), Source: SourceGlobal})
599  	if err != nil {
600  		t.Fatalf("LoadSkills: %v", err)
601  	}
602  	if len(loaded) != 1 {
603  		t.Fatalf("expected 1 skill, got %d", len(loaded))
604  	}
605  	if loaded[0].InstallSource != InstallSourceLocal {
606  		t.Errorf("install source = %q, want %q", loaded[0].InstallSource, InstallSourceLocal)
607  	}
608  	if loaded[0].MarketplaceSlug != "" {
609  		t.Errorf("marketplace slug = %q, want empty", loaded[0].MarketplaceSlug)
610  	}
611  }
612  
613  // TestWriteGlobalSkill_RoundTripsSticky guards the reviewer-flagged
614  // persistence gap: WriteGlobalSkill must preserve sticky-instructions and
615  // the author-pinned sticky-snippet across a write→load cycle. Otherwise
616  // global skills silently lose sticky config on save.
617  func TestWriteGlobalSkill_RoundTripsSticky(t *testing.T) {
618  	shannonDir := t.TempDir()
619  
620  	// Case A: sticky-instructions=true + author-pinned sticky-snippet override.
621  	t.Run("explicit snippet preserved", func(t *testing.T) {
622  		err := WriteGlobalSkill(shannonDir, &Skill{
623  			Name:                  "policy-a",
624  			Description:           "A",
625  			Prompt:                "# Policy\n\nBland intro here.\n\nALL ops go through http://localhost.",
626  			StickyInstructions:    true,
627  			StickySnippetOverride: "Use the http tool for every platform op.",
628  		})
629  		if err != nil {
630  			t.Fatalf("WriteGlobalSkill: %v", err)
631  		}
632  		loaded, err := LoadSkills(SkillSource{Dir: filepath.Join(shannonDir, "skills"), Source: SourceGlobal})
633  		if err != nil {
634  			t.Fatalf("LoadSkills: %v", err)
635  		}
636  		var s *Skill
637  		for _, x := range loaded {
638  			if x.Name == "policy-a" {
639  				s = x
640  				break
641  			}
642  		}
643  		if s == nil {
644  			t.Fatal("policy-a not reloaded")
645  		}
646  		if !s.StickyInstructions {
647  			t.Error("StickyInstructions dropped on round-trip")
648  		}
649  		if s.StickySnippetOverride != "Use the http tool for every platform op." {
650  			t.Errorf("StickySnippetOverride lost: %q", s.StickySnippetOverride)
651  		}
652  		if s.StickySnippet != "Use the http tool for every platform op." {
653  			t.Errorf("resolved StickySnippet != override: %q", s.StickySnippet)
654  		}
655  	})
656  
657  	// Case B: sticky-instructions=true but NO explicit override. Save must
658  	// NOT freeze the heuristic result into the file; on reload, the
659  	// heuristic must run again and still pick the imperative paragraph.
660  	t.Run("heuristic snippet not frozen", func(t *testing.T) {
661  		err := WriteGlobalSkill(shannonDir, &Skill{
662  			Name:                  "policy-b",
663  			Description:           "B",
664  			Prompt:                "# Policy\n\nBland intro here.\n\nNEVER edit config.yaml directly.",
665  			StickyInstructions:    true,
666  			StickySnippet:         "bland intro here.", // would-be frozen value
667  			StickySnippetOverride: "",                  // NOT explicit — should be dropped
668  		})
669  		if err != nil {
670  			t.Fatalf("WriteGlobalSkill: %v", err)
671  		}
672  		raw, err := os.ReadFile(filepath.Join(shannonDir, "skills", "policy-b", "SKILL.md"))
673  		if err != nil {
674  			t.Fatalf("read: %v", err)
675  		}
676  		if strings.Contains(string(raw), "sticky-snippet:") {
677  			t.Errorf("WriteGlobalSkill froze heuristic snippet into SKILL.md:\n%s", raw)
678  		}
679  		if !strings.Contains(string(raw), "sticky-instructions: true") {
680  			t.Errorf("WriteGlobalSkill dropped sticky-instructions flag:\n%s", raw)
681  		}
682  
683  		loaded, _ := LoadSkills(SkillSource{Dir: filepath.Join(shannonDir, "skills"), Source: SourceGlobal})
684  		var s *Skill
685  		for _, x := range loaded {
686  			if x.Name == "policy-b" {
687  				s = x
688  				break
689  			}
690  		}
691  		if s == nil {
692  			t.Fatal("policy-b not reloaded")
693  		}
694  		// Heuristic should run on reload and pick the NEVER paragraph.
695  		if !strings.Contains(s.StickySnippet, "NEVER edit") {
696  			t.Errorf("heuristic re-extraction failed, got %q", s.StickySnippet)
697  		}
698  	})
699  
700  	// Case C: sticky-instructions=false + no override → neither field
701  	// appears in the written frontmatter (no noisy `false` line).
702  	t.Run("disabled sticky omitted", func(t *testing.T) {
703  		err := WriteGlobalSkill(shannonDir, &Skill{
704  			Name:        "plain",
705  			Description: "Plain",
706  			Prompt:      "# plain\n\nhello",
707  		})
708  		if err != nil {
709  			t.Fatalf("WriteGlobalSkill: %v", err)
710  		}
711  		raw, err := os.ReadFile(filepath.Join(shannonDir, "skills", "plain", "SKILL.md"))
712  		if err != nil {
713  			t.Fatalf("read: %v", err)
714  		}
715  		if strings.Contains(string(raw), "sticky-instructions:") {
716  			t.Errorf("plain skill gained noisy sticky-instructions: line:\n%s", raw)
717  		}
718  		if strings.Contains(string(raw), "sticky-snippet:") {
719  			t.Errorf("plain skill gained noisy sticky-snippet: line:\n%s", raw)
720  		}
721  	})
722  
723  	// Case D: hidden=true must survive the write→load cycle. Without this,
724  	// PUT /skills/{slug} on a hidden skill would silently strip hidden:true
725  	// from disk, making the skill reappear in the list on next reload.
726  	t.Run("hidden flag preserved", func(t *testing.T) {
727  		err := WriteGlobalSkill(shannonDir, &Skill{
728  			Name:        "policy-hidden",
729  			Description: "Hidden policy",
730  			Prompt:      "# Policy\n\nbody",
731  			Hidden:      true,
732  		})
733  		if err != nil {
734  			t.Fatalf("WriteGlobalSkill: %v", err)
735  		}
736  		raw, err := os.ReadFile(filepath.Join(shannonDir, "skills", "policy-hidden", "SKILL.md"))
737  		if err != nil {
738  			t.Fatalf("read: %v", err)
739  		}
740  		if !strings.Contains(string(raw), "hidden: true") {
741  			t.Errorf("WriteGlobalSkill dropped hidden flag:\n%s", raw)
742  		}
743  		loaded, _ := LoadSkills(SkillSource{Dir: filepath.Join(shannonDir, "skills"), Source: SourceGlobal})
744  		var s *Skill
745  		for _, x := range loaded {
746  			if x.Name == "policy-hidden" {
747  				s = x
748  				break
749  			}
750  		}
751  		if s == nil {
752  			t.Fatal("policy-hidden not reloaded")
753  		}
754  		if !s.Hidden {
755  			t.Error("Hidden flag lost on round-trip")
756  		}
757  	})
758  
759  	// Case E: hidden omitted or false → no noisy `hidden: false` line in
760  	// the written frontmatter, consistent with Case C's treatment of
761  	// sticky-instructions.
762  	t.Run("hidden false omitted", func(t *testing.T) {
763  		err := WriteGlobalSkill(shannonDir, &Skill{
764  			Name:        "policy-visible",
765  			Description: "Visible policy",
766  			Prompt:      "# Policy\n\nbody",
767  		})
768  		if err != nil {
769  			t.Fatalf("WriteGlobalSkill: %v", err)
770  		}
771  		raw, err := os.ReadFile(filepath.Join(shannonDir, "skills", "policy-visible", "SKILL.md"))
772  		if err != nil {
773  			t.Fatalf("read: %v", err)
774  		}
775  		if strings.Contains(string(raw), "hidden:") {
776  			t.Errorf("visible skill gained noisy hidden: line:\n%s", raw)
777  		}
778  	})
779  }