diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ca82c1..afb1446 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,10 @@ jobs: - uses: golangci/golangci-lint-action@v7 test: - runs-on: ubuntu-latest + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 diff --git a/CHANGELOG.md b/CHANGELOG.md index e31c377..8921e3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.5.4] + +### Fixed + +- Fix false-positive orphan warnings on Windows: file paths from the filesystem + are now normalized to forward slashes before comparing against markdown + references ([#63]). +- Fix code block detection on Windows: fenced code block regexes now handle + CRLF line endings, fixing zero code-block counts when files are checked out + with Windows-style line endings. +- Fix backslash paths in token count keys, result messages, and GitHub Actions + annotations on Windows. +- Add Windows (`windows-latest`) to CI test matrix. + +## [1.5.3] + +### Fixed + +- Contamination warnings now only fire when multiple application programming + languages are detected. Skills containing only auxiliary languages (shell, + config formats) no longer trigger false positives ([#60], [#62]). + ## [1.5.2] ### Fixed @@ -167,17 +189,28 @@ First stable release. Includes the complete CLI and importable library packages. - `types` — shared data types (`Report`, `Result`, `Level`, etc.) - `judge.LLMClient` interface for custom LLM providers +[1.5.4]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.3...v1.5.4 +[1.5.3]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.2...v1.5.3 +[1.5.2]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.1...v1.5.2 +[1.5.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.0...v1.5.1 +[1.5.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.4.0...v1.5.0 [1.4.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.3.1...v1.4.0 [1.3.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.3.0...v1.3.1 [1.3.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.2.1...v1.3.0 [1.2.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.2.0...v1.2.1 [1.2.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.0.0...v1.1.0 +[#23]: https://github.com/agent-ecosystem/skill-validator/issues/23 +[#26]: https://github.com/agent-ecosystem/skill-validator/issues/26 +[#27]: https://github.com/agent-ecosystem/skill-validator/issues/27 [#33]: https://github.com/agent-ecosystem/skill-validator/issues/33 [#34]: https://github.com/agent-ecosystem/skill-validator/pull/34 [#35]: https://github.com/agent-ecosystem/skill-validator/pull/35 [#37]: https://github.com/agent-ecosystem/skill-validator/pull/37 -[#26]: https://github.com/agent-ecosystem/skill-validator/issues/26 -[#23]: https://github.com/agent-ecosystem/skill-validator/issues/23 -[#27]: https://github.com/agent-ecosystem/skill-validator/issues/27 [#39]: https://github.com/agent-ecosystem/skill-validator/issues/39 +[#43]: https://github.com/agent-ecosystem/skill-validator/issues/43 +[#44]: https://github.com/agent-ecosystem/skill-validator/pull/44 +[#45]: https://github.com/agent-ecosystem/skill-validator/issues/45 +[#60]: https://github.com/agent-ecosystem/skill-validator/issues/60 +[#62]: https://github.com/agent-ecosystem/skill-validator/pull/62 +[#63]: https://github.com/agent-ecosystem/skill-validator/issues/63 diff --git a/cmd/root.go b/cmd/root.go index 613481b..38fe9c2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -11,7 +11,7 @@ import ( "github.com/agent-ecosystem/skill-validator/types" ) -const version = "v1.5.3" +const version = "v1.5.4" var ( outputFormat string diff --git a/judge/judge_test.go b/judge/judge_test.go index 054ee64..3521a15 100644 --- a/judge/judge_test.go +++ b/judge/judge_test.go @@ -641,8 +641,9 @@ func TestLatestByFile_Empty(t *testing.T) { func TestCacheDir(t *testing.T) { dir := CacheDir("/path/to/skill") - if dir != "/path/to/skill/.score_cache" { - t.Errorf("got %s, want /path/to/skill/.score_cache", dir) + want := filepath.Join("/path/to/skill", ".score_cache") + if dir != want { + t.Errorf("got %s, want %s", dir, want) } } diff --git a/report/annotations.go b/report/annotations.go index 21ed843..1b5697c 100644 --- a/report/annotations.go +++ b/report/annotations.go @@ -49,7 +49,7 @@ func formatAnnotation(skillDir string, res types.Result, workDir string) string if err != nil { relPath = absPath // fall back to absolute if Rel fails } - params = fmt.Sprintf(" file=%s", relPath) + params = fmt.Sprintf(" file=%s", filepath.ToSlash(relPath)) if res.Line > 0 { params += fmt.Sprintf(",line=%d", res.Line) } diff --git a/structure/markdown.go b/structure/markdown.go index b247704..98d55c9 100644 --- a/structure/markdown.go +++ b/structure/markdown.go @@ -39,7 +39,7 @@ func CheckMarkdown(dir, body string) []types.Result { if err != nil { continue } - relPath := filepath.Join("references", entry.Name()) + relPath := "references/" + entry.Name() if line, ok := FindUnclosedFence(string(data)); ok { results = append(results, ctx.ErrorAtLinef(relPath, line, "%s has an unclosed code fence starting at line %d — this may cause agents to misinterpret everything after it as code", relPath, line)) diff --git a/structure/orphans.go b/structure/orphans.go index 4e42e2f..1f8b579 100644 --- a/structure/orphans.go +++ b/structure/orphans.go @@ -204,7 +204,7 @@ func inventoryFiles(dir string) []string { return nil } rel, _ := filepath.Rel(dir, path) - files = append(files, rel) + files = append(files, filepath.ToSlash(rel)) return nil }) if err != nil { @@ -216,7 +216,7 @@ func inventoryFiles(dir string) []string { // filesInDir returns inventory entries that start with the given directory prefix. func filesInDir(inventory []string, dir string) []string { - prefix := dir + string(filepath.Separator) + prefix := dir + "/" var out []string for _, f := range inventory { if strings.HasPrefix(f, prefix) { @@ -239,8 +239,10 @@ func containsReference(text, sourceDir, relPath string) bool { // path relative to that directory appears in the text. if sourceDir != "" { rel, err := filepath.Rel(sourceDir, relPath) - if err == nil && !strings.HasPrefix(rel, "..") && strings.Contains(text, rel) { - return true + if err == nil && !strings.HasPrefix(rel, "..") { + if strings.Contains(text, filepath.ToSlash(rel)) { + return true + } } } return false @@ -313,10 +315,10 @@ func pythonImportReaches(text, source, relPath string) bool { } // Convert dotted module path to file path: helpers.merge_runs → helpers/merge_runs - modulePath := strings.ReplaceAll(module, ".", string(filepath.Separator)) + modulePath := strings.ReplaceAll(module, ".", "/") // Try resolving as a .py file relative to the source directory. - candidate := filepath.Join(resolveDir, modulePath+".py") + candidate := filepath.ToSlash(filepath.Join(resolveDir, modulePath+".py")) if candidate == relPath { return true } @@ -350,8 +352,8 @@ func pythonPackageInits(text, source, dir string) []string { continue } - modulePath := strings.ReplaceAll(module, ".", string(filepath.Separator)) - initPath := filepath.Join(resolveDir, modulePath, "__init__.py") + modulePath := strings.ReplaceAll(module, ".", "/") + initPath := filepath.ToSlash(filepath.Join(resolveDir, modulePath, "__init__.py")) // Check if the __init__.py actually exists on disk. if _, err := os.Stat(filepath.Join(dir, initPath)); err == nil { diff --git a/structure/orphans_test.go b/structure/orphans_test.go index 7af2d32..1719bdb 100644 --- a/structure/orphans_test.go +++ b/structure/orphans_test.go @@ -378,6 +378,56 @@ func TestCheckOrphanFiles(t *testing.T) { }) } +func TestContainsReference(t *testing.T) { + t.Run("forward-slash path matches markdown text", func(t *testing.T) { + // Inventory paths are normalized to forward slashes; markdown uses forward slashes. + if !containsReference("See references/other.md for details.", "", "references/other.md") { + t.Error("expected forward-slash inventory path to match forward-slash markdown reference") + } + }) + + t.Run("relative path from subdirectory matches", func(t *testing.T) { + // Source is in references/, relPath is references/images/diagram.png, + // so the relative path is images/diagram.png. + text := "See ![diagram](images/diagram.png)." + if !containsReference(text, "references", "references/images/diagram.png") { + t.Error("expected relative path from subdirectory to match") + } + }) +} + +func TestFilesInDir(t *testing.T) { + t.Run("forward-slash inventory matches directory prefix", func(t *testing.T) { + inventory := []string{ + "references/guide.md", + "references/other.md", + "scripts/setup.sh", + } + got := filesInDir(inventory, "references") + if len(got) != 2 { + t.Errorf("expected 2 files in references/, got %d: %v", len(got), got) + } + }) +} + +func TestPythonImportReaches(t *testing.T) { + t.Run("dotted import resolves with forward slashes", func(t *testing.T) { + // "from helpers.merge_runs import merge" in scripts/main.py should + // resolve to scripts/helpers/merge_runs.py (forward slashes). + text := "from helpers.merge_runs import merge" + if !pythonImportReaches(text, "scripts/main.py", "scripts/helpers/merge_runs.py") { + t.Error("expected dotted Python import to resolve to forward-slash path") + } + }) + + t.Run("relative import resolves with forward slashes", func(t *testing.T) { + text := "from .utils import helper" + if !pythonImportReaches(text, "scripts/pkg/main.py", "scripts/pkg/utils.py") { + t.Error("expected relative Python import to resolve to forward-slash path") + } + }) +} + func TestCheckFlatOrphanFiles(t *testing.T) { t.Run("all root files referenced", func(t *testing.T) { dir := t.TempDir() diff --git a/structure/orphans_windows_test.go b/structure/orphans_windows_test.go new file mode 100644 index 0000000..980d154 --- /dev/null +++ b/structure/orphans_windows_test.go @@ -0,0 +1,64 @@ +//go:build windows + +package structure + +import ( + "strings" + "testing" + + "github.com/agent-ecosystem/skill-validator/types" +) + +func TestInventoryFilesUsesForwardSlashes(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "guide content") + writeFile(t, dir, "references/images/diagram.png", "fake image") + writeFile(t, dir, "scripts/setup.sh", "#!/bin/bash") + + inventory := inventoryFiles(dir) + for _, rel := range inventory { + if strings.Contains(rel, `\`) { + t.Errorf("inventory path contains backslash: %s", rel) + } + } +} + +func TestOrphanCheckWithForwardSlashReferences(t *testing.T) { + // This is the exact scenario from issue #63: skill author writes + // forward-slash paths in SKILL.md, which is the cross-platform convention. + // On Windows, filepath.WalkDir returns backslash paths, so the orphan + // checker must normalize before comparing. + dir := t.TempDir() + writeFile(t, dir, "references/other.md", "reference content") + + body := "See references/other.md." + results := CheckOrphanFiles(dir, body, Options{}) + + requireResult(t, results, types.Pass, "all files in references/ are referenced") + requireNoLevel(t, results, types.Warning) +} + +func TestOrphanCheckNestedForwardSlashReference(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "references/guide.md", "See ![diagram](images/diagram.png).") + writeFile(t, dir, "references/images/diagram.png", "fake image") + + body := "Read the [guide](references/guide.md)." + results := CheckOrphanFiles(dir, body, Options{}) + + requireNoResultContaining(t, results, types.Warning, "diagram.png") + requireResult(t, results, types.Pass, "all files in references/ are referenced") +} + +func TestPythonImportResolvesOnWindows(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "scripts/main.py", "from helpers.merge_runs import merge\nmerge()") + writeFile(t, dir, "scripts/helpers/__init__.py", "") + writeFile(t, dir, "scripts/helpers/merge_runs.py", "def merge(): pass") + + body := "Run scripts/main.py to start." + results := CheckOrphanFiles(dir, body, Options{}) + + requireNoResultContaining(t, results, types.Warning, "merge_runs.py") + requireResult(t, results, types.Pass, "all files in scripts/ are referenced") +} diff --git a/structure/tokens.go b/structure/tokens.go index 4ad86ac..026b985 100644 --- a/structure/tokens.go +++ b/structure/tokens.go @@ -81,13 +81,13 @@ func CheckTokens(dir, body string, opts Options) ([]types.Result, []types.TokenC path := filepath.Join(refsDir, entry.Name()) data, err := os.ReadFile(path) if err != nil { - relPath := filepath.Join("references", entry.Name()) + relPath := "references/" + entry.Name() results = append(results, ctx.WarnFilef(relPath, "could not read %s: %v", relPath, err)) continue } tokens, _, _ := enc.Encode(string(data)) fileTokens := len(tokens) - relPath := filepath.Join("references", entry.Name()) + relPath := "references/" + entry.Name() counts = append(counts, types.TokenCount{ File: relPath, Tokens: fileTokens, @@ -251,7 +251,7 @@ func countAssetFiles(dir string, enc tokenizer.Codec) []types.TokenCount { } rel, _ := filepath.Rel(dir, path) tokens, _, _ := enc.Encode(string(data)) - counts = append(counts, types.TokenCount{File: rel, Tokens: len(tokens)}) + counts = append(counts, types.TokenCount{File: filepath.ToSlash(rel), Tokens: len(tokens)}) return nil }) @@ -323,7 +323,7 @@ func countFilesInDir(rootDir, dirName string, enc tokenizer.Codec) []types.Token } rel, _ := filepath.Rel(rootDir, path) tokens, _, _ := enc.Encode(string(data)) - counts = append(counts, types.TokenCount{File: rel, Tokens: len(tokens)}) + counts = append(counts, types.TokenCount{File: filepath.ToSlash(rel), Tokens: len(tokens)}) return nil }) diff --git a/util/regex.go b/util/regex.go index da74066..fdb9e9a 100644 --- a/util/regex.go +++ b/util/regex.go @@ -3,10 +3,10 @@ package util import "regexp" // CodeBlockStrip removes fenced code blocks (backtick and tilde) from markdown. -var CodeBlockStrip = regexp.MustCompile("(?s)(?:```|~~~)[\\w]*\\n.*?(?:```|~~~)") +var CodeBlockStrip = regexp.MustCompile("(?s)(?:```|~~~)[\\w]*\\r?\\n.*?(?:```|~~~)") // InlineCodeStrip removes inline code spans from markdown. var InlineCodeStrip = regexp.MustCompile("`[^`]+`") // CodeBlockPattern extracts fenced code block bodies (capture group 1) from markdown. -var CodeBlockPattern = regexp.MustCompile("(?s)(?:```|~~~)[\\w]*\\n(.*?)(?:```|~~~)") +var CodeBlockPattern = regexp.MustCompile("(?s)(?:```|~~~)[\\w]*\\r?\\n(.*?)(?:```|~~~)")