diff --git a/README.md b/README.md index cb30b2b..c1739e0 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,8 @@ Spec compliance is table stakes. `skill-validator` goes further: it checks that - [Examples](#examples) - [What it checks & why](#what-it-checks) - [Structure validation](#structure-validation-validate-structure) + - [Flat skill layouts](#flat-skill-layouts) + - [Allowing non-standard directories](#allowing-non-standard-directories) - [Link validation](#link-validation-validate-links) - [Content analysis](#content-analysis-analyze-content) - [Contamination analysis](#contamination-analysis-analyze-contamination) @@ -185,9 +187,18 @@ skill-validator validate structure --skip-orphans skill-validator validate structure --strict skill-validator validate structure --allow-extra-frontmatter skill-validator validate structure --allow-flat-layouts +skill-validator validate structure --allow-dirs=evals,testing ``` -Checks spec compliance: directory structure, frontmatter fields, token limits, skill ratio, code fence integrity, internal link validity, and orphan file detection. Use `--skip-orphans` to suppress warnings about unreferenced files in `scripts/`, `references/`, and `assets/`. Use `--strict` to treat warnings as errors (exit 1 instead of 2). Use `--allow-extra-frontmatter` to suppress warnings for frontmatter fields not defined in the spec (e.g. `user-invokable`). Standard frontmatter fields are still fully validated. Use `--allow-flat-layouts` to allow supplemental files alongside SKILL.md at the skill root without warnings (see [Flat skill layouts](#flat-skill-layouts)). +Checks spec compliance: directory structure, frontmatter fields, token limits, skill ratio, code fence integrity, internal link validity, and orphan file detection. + +| Flag | Effect | +|---|---| +| `--strict` | Treat warnings as errors (exit 1 instead of 2) | +| `--skip-orphans` | Suppress warnings about unreferenced files in `scripts/`, `references/`, and `assets/` | +| `--allow-extra-frontmatter` | Suppress warnings for non-spec frontmatter fields (e.g. `user-invokable`). Standard fields are still fully validated | +| `--allow-flat-layouts` | Allow files at the skill root without warnings (see [Flat skill layouts](#flat-skill-layouts)) | +| `--allow-dirs=evals,testing` | Accept specific non-standard directories without warnings (see [Allowing non-standard directories](#allowing-non-standard-directories)) | ``` Validating skill: my-skill/ @@ -284,9 +295,21 @@ skill-validator check --skip-orphans skill-validator check --strict skill-validator check --allow-extra-frontmatter skill-validator check --allow-flat-layouts +skill-validator check --allow-dirs=evals,testing ``` -Runs all checks (structure + links + content + contamination). Use `--only` or `--skip` to select specific check groups. The flags are mutually exclusive. Use `--per-file` to see per-file reference analysis alongside the aggregate. Use `--skip-orphans` to suppress orphan file warnings in the structure check. Use `--strict` to treat warnings as errors (exit 1 instead of 2). Use `--allow-extra-frontmatter` to suppress warnings for non-spec frontmatter fields. Use `--allow-flat-layouts` to allow supplemental files at the skill root without warnings (see [Flat skill layouts](#flat-skill-layouts)). +Runs all checks (structure + links + content + contamination). + +| Flag | Effect | +|---|---| +| `--only` | Comma-separated list of check groups to run (mutually exclusive with `--skip`) | +| `--skip` | Comma-separated list of check groups to skip (mutually exclusive with `--only`) | +| `--per-file` | Show per-file reference analysis alongside the aggregate | +| `--strict` | Treat warnings as errors (exit 1 instead of 2) | +| `--skip-orphans` | Suppress orphan file warnings in the structure check | +| `--allow-extra-frontmatter` | Suppress warnings for non-spec frontmatter fields | +| `--allow-flat-layouts` | Allow files at the skill root without warnings (see [Flat skill layouts](#flat-skill-layouts)) | +| `--allow-dirs=evals,testing` | Accept specific non-standard directories without warnings (see [Allowing non-standard directories](#allowing-non-standard-directories)) | Valid check groups: `structure`, `links`, `content`, `contamination`. @@ -709,6 +732,27 @@ skill-validator check --allow-flat-layouts my-skill/ > [!NOTE] > The standard directory structure remains the recommended approach for maximum portability across agent platforms. Use `--allow-flat-layouts` when a flat layout better fits your workflow, with the understanding that some platforms may not discover files outside the recognized directories. +**Allowing non-standard directories** + +The spec defines three recognized directories (`scripts/`, `references/`, `assets/`). Any other directory at the skill root produces a warning. This relates to cross-platform skill file loading considerations described in [agent-ecosystem/agent-skill-implementation](https://github.com/agent-ecosystem/agent-skill-implementation). + +Some development workflows use additional directories that may produce unexpected behavior across agent platforms. For example, the [evaluating-skills guide](https://agentskills.io/skill-creation/evaluating-skills) recommends an `evals/` directory for evaluation test cases, and teams may keep integration test fixtures in a `testing/` directory. If you are not distributing cross-platform skills and want to suppress warnings for specific directories that you know your preferred agent platform supports, use the `--allow-dirs` flag to suppress warnings for specific directories by name: + +``` +skill-validator validate structure --allow-dirs=evals my-skill/ +skill-validator check --allow-dirs=evals,testing my-skill/ +``` + +The flag accepts a comma-separated list or can be repeated (`--allow-dirs=evals --allow-dirs=testing`). Allowed directories differ from recognized directories in two ways: + +1. **Exempt from deep-nesting checks**: The validator can't know the expected internal structure of arbitrary directories, so subdirectories like `evals/files/` won't trigger nesting warnings. +2. **Skipped for orphan detection**: Since the validator doesn't know how these directories are used, it skips orphan file checks for them and emits an informational note instead. + +Directories not in the allow list still produce the standard warning with file counts and suggestions. If an allowed directory name matches a recognized directory (e.g., `--allow-dirs=scripts`), it's silently accepted with no change in behavior. + +> [!NOTE] +> Allowing a directory suppresses validator warnings but does not change how agent platforms handle the directory. Files in non-standard directories may not be discovered during skill activation, or may load into agent context unexpectedly. If you're distributing skills across platforms, consider whether those files belong in `references/` or `assets/` instead. + ### Link validation (`validate links`) - Checks external (HTTP/HTTPS) links only -- internal (relative) links are validated by `validate structure` diff --git a/cmd/check.go b/cmd/check.go index e027fc5..9fe4167 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -20,6 +20,7 @@ var ( strictCheck bool checkAllowExtraFrontmatter bool checkAllowFlatLayouts bool + checkAllowDirs []string ) var checkCmd = &cobra.Command{ @@ -41,6 +42,8 @@ func init() { "suppress warnings for non-spec frontmatter fields") checkCmd.Flags().BoolVar(&checkAllowFlatLayouts, "allow-flat-layouts", false, "allow files at the skill root without warnings and treat them as standard content for token counting") + checkCmd.Flags().StringSliceVar(&checkAllowDirs, "allow-dirs", nil, + "comma-separated list of directory names to accept without warnings (e.g. --allow-dirs=evals,testing)") rootCmd.AddCommand(checkCmd) } @@ -72,6 +75,7 @@ func runCheck(cmd *cobra.Command, args []string) error { SkipOrphans: checkSkipOrphans, AllowExtraFrontmatter: checkAllowExtraFrontmatter, AllowFlatLayouts: checkAllowFlatLayouts, + AllowDirs: checkAllowDirs, }, } eopts := exitOpts{strict: strictCheck} diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index cb07532..f1320e0 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -551,6 +551,121 @@ func TestValidateCommand_FlatSkill_OrphanDetection(t *testing.T) { } } +func TestValidateCommand_AllowedDirsSkill_WithoutFlag(t *testing.T) { + dir := fixtureDir(t, "allowed-dirs-skill") + + r := structure.Validate(dir, structure.Options{}) + // Without --allow-dirs, evals/ and testing/ should produce warnings + hasEvalsWarning := false + hasTestingWarning := false + for _, res := range r.Results { + if res.Level == types.Warning && strings.Contains(res.Message, "unknown directory: evals/") { + hasEvalsWarning = true + } + if res.Level == types.Warning && strings.Contains(res.Message, "unknown directory: testing/") { + hasTestingWarning = true + } + } + if !hasEvalsWarning { + t.Error("expected warning for evals/ without --allow-dirs") + } + if !hasTestingWarning { + t.Error("expected warning for testing/ without --allow-dirs") + } +} + +func TestValidateCommand_AllowedDirsSkill_WithFlag(t *testing.T) { + dir := fixtureDir(t, "allowed-dirs-skill") + + r := structure.Validate(dir, structure.Options{AllowDirs: []string{"evals", "testing"}}) + + // Should pass with no errors + if r.Errors != 0 { + t.Errorf("expected 0 errors, got %d", r.Errors) + for _, res := range r.Results { + if res.Level == types.Error { + t.Logf(" error: %s: %s", res.Category, res.Message) + } + } + } + + // No warnings about evals/ or testing/ + for _, res := range r.Results { + if res.Level == types.Warning && + (strings.Contains(res.Message, "evals/") || strings.Contains(res.Message, "testing/")) { + t.Errorf("unexpected warning with --allow-dirs: %s", res.Message) + } + } + + // No deep nesting warning for evals/files/ + for _, res := range r.Results { + if res.Level == types.Warning && strings.Contains(res.Message, "deep nesting") { + t.Errorf("unexpected deep nesting warning for allowed dir: %s", res.Message) + } + } + + // Should have info notes for orphan detection skipping + hasEvalsInfo := false + hasTestingInfo := false + for _, res := range r.Results { + if res.Level == types.Info && strings.Contains(res.Message, "evals/ skipped for orphan detection") { + hasEvalsInfo = true + } + if res.Level == types.Info && strings.Contains(res.Message, "testing/ skipped for orphan detection") { + hasTestingInfo = true + } + } + if !hasEvalsInfo { + t.Error("expected info note about evals/ being skipped for orphan detection") + } + if !hasTestingInfo { + t.Error("expected info note about testing/ being skipped for orphan detection") + } + + // Recognized dirs should still have orphan pass results + hasReferencesPass := false + hasScriptsPass := false + for _, res := range r.Results { + if res.Level == types.Pass && strings.Contains(res.Message, "all files in references/ are referenced") { + hasReferencesPass = true + } + if res.Level == types.Pass && strings.Contains(res.Message, "all files in scripts/ are referenced") { + hasScriptsPass = true + } + } + if !hasReferencesPass { + t.Error("expected pass for references/ orphan check") + } + if !hasScriptsPass { + t.Error("expected pass for scripts/ orphan check") + } +} + +func TestValidateCommand_AllowedDirsSkill_PartialAllow(t *testing.T) { + dir := fixtureDir(t, "allowed-dirs-skill") + + // Only allow evals, not testing + r := structure.Validate(dir, structure.Options{AllowDirs: []string{"evals"}}) + + // evals/ should not warn + for _, res := range r.Results { + if res.Level == types.Warning && strings.Contains(res.Message, "unknown directory: evals/") { + t.Error("unexpected warning for evals/ with --allow-dirs=evals") + } + } + + // testing/ should still warn + hasTestingWarning := false + for _, res := range r.Results { + if res.Level == types.Warning && strings.Contains(res.Message, "unknown directory: testing/") { + hasTestingWarning = true + } + } + if !hasTestingWarning { + t.Error("expected warning for testing/ when not in --allow-dirs") + } +} + func TestDetectAndResolve_NoSkill(t *testing.T) { dir := t.TempDir() _, _, _, err := detectAndResolve([]string{dir}) diff --git a/cmd/validate_structure.go b/cmd/validate_structure.go index 63ee6e7..a1711ee 100644 --- a/cmd/validate_structure.go +++ b/cmd/validate_structure.go @@ -12,6 +12,7 @@ var ( strictStructure bool structAllowExtraFrontmatter bool structAllowFlatLayouts bool + structAllowDirs []string ) var validateStructureCmd = &cobra.Command{ @@ -30,6 +31,8 @@ func init() { "suppress warnings for non-spec frontmatter fields") validateStructureCmd.Flags().BoolVar(&structAllowFlatLayouts, "allow-flat-layouts", false, "allow files at the skill root without warnings and treat them as standard content for token counting") + validateStructureCmd.Flags().StringSliceVar(&structAllowDirs, "allow-dirs", nil, + "comma-separated list of directory names to accept without warnings (e.g. --allow-dirs=evals,testing)") validateCmd.AddCommand(validateStructureCmd) } @@ -43,6 +46,7 @@ func runValidateStructure(cmd *cobra.Command, args []string) error { SkipOrphans: skipOrphans, AllowExtraFrontmatter: structAllowExtraFrontmatter, AllowFlatLayouts: structAllowFlatLayouts, + AllowDirs: structAllowDirs, } eopts := exitOpts{strict: strictStructure} diff --git a/structure/checks.go b/structure/checks.go index d4eefce..779446b 100644 --- a/structure/checks.go +++ b/structure/checks.go @@ -42,10 +42,18 @@ var knownExtraneousFiles = map[string]string{ // CheckStructure validates the directory layout of a skill package. It checks // for the required SKILL.md file, flags unrecognized directories and extraneous // root files, and warns about deep nesting in recognized directories. +// Directories listed in opts.AllowDirs are accepted without warning and are +// exempt from deep-nesting checks. func CheckStructure(dir string, opts Options) []types.Result { ctx := types.ResultContext{Category: "Structure"} var results []types.Result + // Build a set of user-allowed directories. + allowedDirs := make(map[string]bool, len(opts.AllowDirs)) + for _, d := range opts.AllowDirs { + allowedDirs[d] = true + } + // Check SKILL.md exists skillPath := filepath.Join(dir, "SKILL.md") if _, err := os.Stat(skillPath); os.IsNotExist(err) { @@ -72,7 +80,7 @@ func CheckStructure(dir string, opts Options) []types.Result { } continue } - if !recognizedDirs[name] { + if !recognizedDirs[name] && !allowedDirs[name] { msg := fmt.Sprintf("unknown directory: %s/", name) if subEntries, err := os.ReadDir(filepath.Join(dir, name)); err == nil { fileCount := 0 @@ -93,7 +101,9 @@ func CheckStructure(dir string, opts Options) []types.Result { } } - // Check for deep nesting in recognized directories + // Check for deep nesting in recognized directories only. + // Allowed directories are exempt because the validator cannot know + // their expected internal structure. for dirName := range recognizedDirs { subdir := filepath.Join(dir, dirName) if _, err := os.Stat(subdir); os.IsNotExist(err) { diff --git a/structure/checks_test.go b/structure/checks_test.go index 7acaf8d..71ddc50 100644 --- a/structure/checks_test.go +++ b/structure/checks_test.go @@ -205,4 +205,96 @@ func TestCheckStructure(t *testing.T) { results := CheckStructure(dir, Options{AllowFlatLayouts: true}) requireResultContaining(t, results, types.Warning, "unknown directory: extras/") }) + + t.Run("allow-dirs suppresses warning for allowed directory", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + results := CheckStructure(dir, Options{AllowDirs: []string{"evals"}}) + requireResult(t, results, types.Pass, "SKILL.md found") + requireNoLevel(t, results, types.Warning) + }) + + t.Run("allow-dirs with multiple directories", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "testing/test1.md", "test content") + results := CheckStructure(dir, Options{AllowDirs: []string{"evals", "testing"}}) + requireResult(t, results, types.Pass, "SKILL.md found") + requireNoLevel(t, results, types.Warning) + }) + + t.Run("allow-dirs partial allows still warn for non-allowed dirs", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "extras/file.md", "content") + results := CheckStructure(dir, Options{AllowDirs: []string{"evals"}}) + requireNoResultContaining(t, results, types.Warning, "evals/") + requireResultContaining(t, results, types.Warning, "unknown directory: extras/") + }) + + t.Run("allow-dirs silently accepts already-recognized directory", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "scripts/setup.sh", "#!/bin/bash") + results := CheckStructure(dir, Options{AllowDirs: []string{"scripts"}}) + requireResult(t, results, types.Pass, "SKILL.md found") + requireNoLevel(t, results, types.Warning) + }) + + t.Run("allow-dirs exempt from deep nesting checks", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/files/test1.txt", "test input") + results := CheckStructure(dir, Options{AllowDirs: []string{"evals"}}) + requireResult(t, results, types.Pass, "SKILL.md found") + requireNoResultContaining(t, results, types.Warning, "deep nesting") + }) + + t.Run("allow-dirs does not exempt recognized dirs from deep nesting", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/files/test1.txt", "test input") + if err := os.MkdirAll(filepath.Join(dir, "references", "subdir"), 0o755); err != nil { + t.Fatal(err) + } + results := CheckStructure(dir, Options{AllowDirs: []string{"evals"}}) + requireNoResultContaining(t, results, types.Warning, "evals/") + requireResult(t, results, types.Warning, "deep nesting detected: references/subdir/") + }) + + t.Run("allow-dirs with allow-flat-layouts", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "README.md", "readme") + writeFile(t, dir, "notes.txt", "notes") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + results := CheckStructure(dir, Options{AllowFlatLayouts: true, AllowDirs: []string{"evals"}}) + requireResult(t, results, types.Pass, "SKILL.md found") + requireNoLevel(t, results, types.Warning) + }) + + t.Run("allow-dirs with allow-flat-layouts still warns on non-allowed dirs", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "README.md", "readme") + writeFile(t, dir, "extras/file.md", "content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + results := CheckStructure(dir, Options{AllowFlatLayouts: true, AllowDirs: []string{"evals"}}) + requireNoLevel(t, results, types.Error) + requireNoResultContaining(t, results, types.Warning, "README.md") + requireNoResultContaining(t, results, types.Warning, "evals/") + requireResultContaining(t, results, types.Warning, "unknown directory: extras/") + }) + + t.Run("allow-dirs hint still shown for non-allowed unknown dirs", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "extras/file.md", "content") + results := CheckStructure(dir, Options{AllowDirs: []string{"evals"}}) + requireResultContaining(t, results, types.Warning, "should this be references/ or assets/?") + }) } diff --git a/structure/orphans.go b/structure/orphans.go index 12df831..4e42e2f 100644 --- a/structure/orphans.go +++ b/structure/orphans.go @@ -22,13 +22,29 @@ type queueItem struct { // CheckOrphanFiles walks scripts/, references/, and assets/ to find files // that are never referenced (directly or transitively) from SKILL.md. -func CheckOrphanFiles(dir, body string) []types.Result { +// Directories listed in opts.AllowDirs are skipped with an informational note, +// since the validator cannot know their expected reference patterns. +func CheckOrphanFiles(dir, body string, opts Options) []types.Result { ctx := types.ResultContext{Category: "Structure"} + var results []types.Result + + // Emit informational notes for allowed directories that exist on disk. + // These are skipped for orphan detection because the validator cannot + // know their expected reference patterns. + for _, ad := range opts.AllowDirs { + if recognizedDirs[ad] { + continue // already covered by normal orphan detection + } + if _, err := os.Stat(filepath.Join(dir, ad)); err == nil { + results = append(results, ctx.Infof( + "%s/ skipped for orphan detection (allowed via --allow-dirs)", ad)) + } + } // Inventory: collect all files in recognized directories. inventory := inventoryFiles(dir) if len(inventory) == 0 { - return nil + return results } // Collect root-level text files (excluding SKILL.md) that can serve as @@ -115,8 +131,6 @@ func CheckOrphanFiles(dir, body string) []types.Result { } // Build results per directory. - var results []types.Result - for _, d := range orderedRecognizedDirs { dirFiles := filesInDir(inventory, d) if len(dirFiles) == 0 { diff --git a/structure/orphans_test.go b/structure/orphans_test.go index 40351d4..7af2d32 100644 --- a/structure/orphans_test.go +++ b/structure/orphans_test.go @@ -14,7 +14,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "assets/logo.png", "fake image") body := "See references/guide.md and scripts/setup.sh and assets/logo.png" - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResult(t, results, types.Pass, "all files in scripts/ are referenced") requireResult(t, results, types.Pass, "all files in references/ are referenced") @@ -28,7 +28,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "references/unused.md", "unused content") body := "See references/guide.md for details." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResultContaining(t, results, types.Warning, "potentially unreferenced file: references/unused.md") }) @@ -38,7 +38,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/setup.sh", "#!/bin/bash") body := "No references to scripts here." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResultContaining(t, results, types.Warning, "potentially unreferenced file: scripts/setup.sh") }) @@ -46,7 +46,7 @@ func TestCheckOrphanFiles(t *testing.T) { t.Run("empty directories produce no results", func(t *testing.T) { dir := t.TempDir() // No files at all - results := CheckOrphanFiles(dir, "some body") + results := CheckOrphanFiles(dir, "some body", Options{}) if len(results) != 0 { t.Errorf("expected 0 results for empty dirs, got %d", len(results)) } @@ -56,7 +56,7 @@ func TestCheckOrphanFiles(t *testing.T) { dir := t.TempDir() writeFile(t, dir, "other/file.md", "content") - results := CheckOrphanFiles(dir, "some body") + results := CheckOrphanFiles(dir, "some body", Options{}) if len(results) != 0 { t.Errorf("expected 0 results for unrecognized dirs, got %d", len(results)) } @@ -68,7 +68,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "references/secret.md", "secret content") body := "See assets/logo.png for the logo." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // logo.png is reached (referenced from body) but not scanned for further refs // so references/secret.md should be an orphan @@ -84,7 +84,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "references/images/diagram.png", "fake image") body := "Read the [guide](references/guide.md)." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // The image should be reached (indirectly via guide.md), not flagged as orphan requireNoResultContaining(t, results, types.Warning, "references/images/diagram.png") @@ -98,7 +98,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/fill_form.py", "#!/usr/bin/env python3") body := "For form filling, read FORMS.md and follow its instructions." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "scripts/fill_form.py") requireResult(t, results, types.Pass, "all files in scripts/ are referenced") @@ -111,7 +111,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/validate.js", "// validator") body := "See package.json for available commands. Run `npm run validate` to check." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // package.json is mentioned so it gets scanned, finding scripts/validate.js requireNoResultContaining(t, results, types.Warning, "scripts/validate.js") @@ -124,7 +124,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/validate.js", "// validator") body := "Run `npm run validate` to check your component." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // package.json is not mentioned, so scripts/validate.js stays orphaned requireResultContaining(t, results, types.Warning, "potentially unreferenced file: scripts/validate.js") @@ -137,7 +137,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/fill_form.py", "#!/usr/bin/env python3") body := "For form filling, read FORMS.md and follow its instructions." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "scripts/fill_form.py") requireResult(t, results, types.Pass, "all files in scripts/ are referenced") @@ -148,7 +148,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/check_fields.py", "#!/usr/bin/env python3") body := "Run `python scripts/check_fields ` to check." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResultContaining(t, results, types.Warning, "file scripts/check_fields.py is referenced without its extension (as scripts/check_fields in SKILL.md) — include the .py extension so agents can reliably locate the file") @@ -162,7 +162,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/check_fields.py", "#!/usr/bin/env python3") body := "For form filling, read forms.md." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResultContaining(t, results, types.Warning, "file scripts/check_fields.py is referenced without its extension (as scripts/check_fields in forms.md)") @@ -174,7 +174,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/run.py", "#!/usr/bin/env python3") body := "Run scripts/run.py to start." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "__init__.py") requireNoResultContaining(t, results, types.Info, "__init__.py") @@ -187,7 +187,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/run.py", "#!/usr/bin/env python3") body := "No references here." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "__init__.py") requireResultContaining(t, results, types.Warning, "potentially unreferenced file: scripts/run.py") @@ -199,7 +199,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/pkg/helpers.py", "# helpers") body := "No references here." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "__init__.py") requireResultContaining(t, results, types.Warning, "scripts/pkg/helpers.py") @@ -211,7 +211,7 @@ func TestCheckOrphanFiles(t *testing.T) { // Body references with full extension — should get normal treatment, not the extension warning body := "Run scripts/setup.sh to configure." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResult(t, results, types.Pass, "all files in scripts/ are referenced") requireNoResultContaining(t, results, types.Warning, "referenced without its extension") @@ -224,7 +224,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/secret.sh", "#!/bin/bash") body := "This skill has no special setup." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // notes.md is never mentioned, so it shouldn't be scanned, and the script stays orphaned requireResultContaining(t, results, types.Warning, "potentially unreferenced file: scripts/secret.sh") @@ -237,7 +237,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/helpers.py", "def merge(): pass") body := "Run scripts/main.py to start." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "scripts/helpers.py") requireResult(t, results, types.Pass, "all files in scripts/ are referenced") @@ -250,7 +250,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/helpers/merge_runs.py", "def merge(): pass") body := "Run scripts/main.py to start." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "scripts/helpers/merge_runs.py") requireResult(t, results, types.Pass, "all files in scripts/ are referenced") @@ -263,7 +263,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/pkg/utils.py", "def helper(): pass") body := "Run scripts/pkg/main.py to start." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireNoResultContaining(t, results, types.Warning, "scripts/pkg/utils.py") requireResult(t, results, types.Pass, "all files in scripts/ are referenced") @@ -275,7 +275,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/data_loader.sh", "#!/bin/bash") body := "Run scripts/main.py to start." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // .sh file should not be resolved by Python imports; it's matched // via the extensionless fallback since "data_loader" appears in the text @@ -293,7 +293,7 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "scripts/validators/extra.py", "class ExtraValidator: pass") body := "Run scripts/pack.py to package." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) // base.py should be reached via: pack.py → __init__.py → .base requireNoResultContaining(t, results, types.Warning, "scripts/validators/base.py") @@ -308,12 +308,74 @@ func TestCheckOrphanFiles(t *testing.T) { writeFile(t, dir, "assets/unused3.png", "content") body := "No references to any files." - results := CheckOrphanFiles(dir, body) + results := CheckOrphanFiles(dir, body, Options{}) requireResultContaining(t, results, types.Warning, "potentially unreferenced file: references/unused1.md") requireResultContaining(t, results, types.Warning, "potentially unreferenced file: scripts/unused2.sh") requireResultContaining(t, results, types.Warning, "potentially unreferenced file: assets/unused3.png") }) + + t.Run("allow-dirs emits info note for existing allowed directory", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "guide content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + + body := "See references/guide.md for details." + results := CheckOrphanFiles(dir, body, Options{AllowDirs: []string{"evals"}}) + + requireResultContaining(t, results, types.Info, "evals/ skipped for orphan detection (allowed via --allow-dirs)") + requireResult(t, results, types.Pass, "all files in references/ are referenced") + }) + + t.Run("allow-dirs no info note for nonexistent allowed directory", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "guide content") + + body := "See references/guide.md for details." + results := CheckOrphanFiles(dir, body, Options{AllowDirs: []string{"evals"}}) + + requireNoResultContaining(t, results, types.Info, "evals/") + }) + + t.Run("allow-dirs skips recognized dir in info notes", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "guide content") + + body := "See references/guide.md for details." + results := CheckOrphanFiles(dir, body, Options{AllowDirs: []string{"references"}}) + + // "references" is already a recognized dir, so no info note + requireNoResultContaining(t, results, types.Info, "references/") + requireResult(t, results, types.Pass, "all files in references/ are referenced") + }) + + t.Run("allow-dirs with skip-orphans does not conflict", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "SKILL.md", "---\nname: test\n---\nbody") + writeFile(t, dir, "references/guide.md", "guide content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + + // When SkipOrphans is true, CheckOrphanFiles is never called (handled in Validate). + // But if called directly, it should still work. + results := CheckOrphanFiles(dir, "body", Options{ + AllowDirs: []string{"evals"}, + }) + requireResultContaining(t, results, types.Info, "evals/ skipped for orphan detection") + }) + + t.Run("allow-dirs with multiple allowed dirs", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "guide content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "testing/test1.md", "test content") + + body := "See references/guide.md for details." + results := CheckOrphanFiles(dir, body, Options{AllowDirs: []string{"evals", "testing"}}) + + requireResultContaining(t, results, types.Info, "evals/ skipped for orphan detection") + requireResultContaining(t, results, types.Info, "testing/ skipped for orphan detection") + requireResult(t, results, types.Pass, "all files in references/ are referenced") + }) } func TestCheckFlatOrphanFiles(t *testing.T) { diff --git a/structure/validate.go b/structure/validate.go index 2e9a8e3..b88f394 100644 --- a/structure/validate.go +++ b/structure/validate.go @@ -14,6 +14,7 @@ type Options struct { SkipOrphans bool AllowExtraFrontmatter bool AllowFlatLayouts bool + AllowDirs []string } // ValidateMulti validates each directory and returns an aggregated report. @@ -78,7 +79,7 @@ func Validate(dir string, opts Options) *types.Report { // Orphan file checks (files in recognized dirs that are never referenced) if !opts.SkipOrphans { - report.Results = append(report.Results, CheckOrphanFiles(dir, s.Body)...) + report.Results = append(report.Results, CheckOrphanFiles(dir, s.Body, opts)...) if opts.AllowFlatLayouts { report.Results = append(report.Results, CheckFlatOrphanFiles(dir, s.Body)...) } diff --git a/structure/validate_test.go b/structure/validate_test.go index 46b6b96..0a90aae 100644 --- a/structure/validate_test.go +++ b/structure/validate_test.go @@ -167,6 +167,110 @@ func TestValidate(t *testing.T) { } requireResultContaining(t, report.Results, types.Error, "parsing frontmatter YAML") }) + + t.Run("allow-dirs suppresses unknown dir warning in full validation", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: A valid skill\n---\n# Body\n") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + report := Validate(dir, Options{AllowDirs: []string{"evals"}}) + if report.Errors != 0 { + t.Errorf("expected 0 errors, got %d", report.Errors) + } + requireNoResultContaining(t, report.Results, types.Warning, "evals/") + }) + + t.Run("allow-dirs with skip-orphans", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: A valid skill\n---\n# Body\n") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "references/unused.md", "unused content") + report := Validate(dir, Options{AllowDirs: []string{"evals"}, SkipOrphans: true}) + if report.Errors != 0 { + t.Errorf("expected 0 errors, got %d", report.Errors) + } + // No unknown dir warning for evals + requireNoResultContaining(t, report.Results, types.Warning, "evals/") + // Orphan check skipped entirely, so no orphan warning for unused.md + requireNoResultContaining(t, report.Results, types.Warning, "unreferenced") + // No info note about allowed dirs since orphan detection was skipped + requireNoResultContaining(t, report.Results, types.Info, "skipped for orphan detection") + }) + + t.Run("allow-dirs with allow-extra-frontmatter", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: desc\ncustom_field: value\n---\n# Body\n") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + report := Validate(dir, Options{ + AllowDirs: []string{"evals"}, + AllowExtraFrontmatter: true, + }) + if report.Errors != 0 { + t.Errorf("expected 0 errors, got %d", report.Errors) + } + requireNoResultContaining(t, report.Results, types.Warning, "evals/") + requireNoResultContaining(t, report.Results, types.Warning, "custom_field") + }) + + t.Run("allow-dirs with allow-flat-layouts full integration", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: desc\n---\n# Body\nSee guide.md.\n") + writeFile(t, dir, "guide.md", "Guide content.") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + report := Validate(dir, Options{ + AllowDirs: []string{"evals"}, + AllowFlatLayouts: true, + }) + if report.Errors != 0 { + t.Errorf("expected 0 errors, got %d", report.Errors) + } + if report.Warnings != 0 { + t.Errorf("expected 0 warnings, got %d", report.Warnings) + for _, r := range report.Results { + if r.Level == types.Warning { + t.Logf(" warning: %s: %s", r.Category, r.Message) + } + } + } + }) + + t.Run("allow-dirs emits info note during orphan detection", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: desc\n---\n# Body\nSee references/guide.md.\n") + writeFile(t, dir, "references/guide.md", "guide content") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + report := Validate(dir, Options{AllowDirs: []string{"evals"}}) + requireResultContaining(t, report.Results, types.Info, "evals/ skipped for orphan detection") + }) + + t.Run("allow-dirs with all flags combined", func(t *testing.T) { + dir := t.TempDir() + writeSkill(t, dir, "---\nname: "+dirName(dir)+"\ndescription: desc\ncustom: val\n---\n# Body\nSee helper.md.\n") + writeFile(t, dir, "helper.md", "Helper content.") + writeFile(t, dir, "evals/evals.json", `{"tests": []}`) + writeFile(t, dir, "testing/test1.md", "test content") + report := Validate(dir, Options{ + AllowDirs: []string{"evals", "testing"}, + AllowExtraFrontmatter: true, + AllowFlatLayouts: true, + SkipOrphans: true, + }) + if report.Errors != 0 { + t.Errorf("expected 0 errors, got %d", report.Errors) + for _, r := range report.Results { + if r.Level == types.Error { + t.Logf(" error: %s: %s", r.Category, r.Message) + } + } + } + if report.Warnings != 0 { + t.Errorf("expected 0 warnings, got %d", report.Warnings) + for _, r := range report.Results { + if r.Level == types.Warning { + t.Logf(" warning: %s: %s", r.Category, r.Message) + } + } + } + }) } func TestValidateMulti(t *testing.T) { diff --git a/testdata/allowed-dirs-skill/SKILL.md b/testdata/allowed-dirs-skill/SKILL.md new file mode 100644 index 0000000..ed65faa --- /dev/null +++ b/testdata/allowed-dirs-skill/SKILL.md @@ -0,0 +1,28 @@ +--- +name: allowed-dirs-skill +description: A skill with development directories that require --allow-dirs. +license: MIT +compatibility: Works with all major LLM providers. +metadata: + author: test + version: "1.0" +allowed-tools: Bash, Read, Write +--- +# Allowed Dirs Skill + +This skill demonstrates using non-standard directories alongside the +standard skill structure. The `evals/` and `testing/` directories are +development artifacts that require `--allow-dirs` to suppress warnings. + +## Usage + +Follow the instructions in the [reference guide](references/guide.md) +to get started. + +Run scripts/validate.sh to check your work. + +## Notes + +The evals/ directory contains evaluation test cases and the testing/ +directory contains integration test fixtures. Neither is part of the +standard skill structure, but both are useful during development. diff --git a/testdata/allowed-dirs-skill/evals/evals.json b/testdata/allowed-dirs-skill/evals/evals.json new file mode 100644 index 0000000..ad6c992 --- /dev/null +++ b/testdata/allowed-dirs-skill/evals/evals.json @@ -0,0 +1,9 @@ +{ + "tests": [ + { + "name": "basic_usage", + "prompt": "Show me how to use this skill", + "expected": "Follow the reference guide" + } + ] +} diff --git a/testdata/allowed-dirs-skill/evals/files/sample_input.txt b/testdata/allowed-dirs-skill/evals/files/sample_input.txt new file mode 100644 index 0000000..1432c7c --- /dev/null +++ b/testdata/allowed-dirs-skill/evals/files/sample_input.txt @@ -0,0 +1 @@ +Sample input for evaluation test cases. diff --git a/testdata/allowed-dirs-skill/references/guide.md b/testdata/allowed-dirs-skill/references/guide.md new file mode 100644 index 0000000..8f7e862 --- /dev/null +++ b/testdata/allowed-dirs-skill/references/guide.md @@ -0,0 +1,7 @@ +# Guide + +This is the reference guide for the allowed-dirs-skill. + +## Getting Started + +Follow these steps to set up the skill. diff --git a/testdata/allowed-dirs-skill/scripts/validate.sh b/testdata/allowed-dirs-skill/scripts/validate.sh new file mode 100644 index 0000000..793dd2d --- /dev/null +++ b/testdata/allowed-dirs-skill/scripts/validate.sh @@ -0,0 +1,2 @@ +#!/bin/bash +echo "Validating skill..." diff --git a/testdata/allowed-dirs-skill/testing/fixtures.json b/testdata/allowed-dirs-skill/testing/fixtures.json new file mode 100644 index 0000000..45bf173 --- /dev/null +++ b/testdata/allowed-dirs-skill/testing/fixtures.json @@ -0,0 +1,5 @@ +{ + "fixtures": [ + {"name": "happy_path", "input": "valid input", "expected_output": "success"} + ] +}