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 }