Skip to content

Commit cc1064d

Browse files
authored
Merge pull request #64 from agent-ecosystem/fix-orphan-check-on-windows
Normalize filepath separator to fix Windows-orphan check bug
2 parents 935f2bf + 242855c commit cc1064d

11 files changed

Lines changed: 176 additions & 23 deletions

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ jobs:
1919
- uses: golangci/golangci-lint-action@v7
2020

2121
test:
22-
runs-on: ubuntu-latest
22+
strategy:
23+
matrix:
24+
os: [ubuntu-latest, windows-latest]
25+
runs-on: ${{ matrix.os }}
2326
steps:
2427
- uses: actions/checkout@v6
2528

CHANGELOG.md

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [1.5.4]
9+
10+
### Fixed
11+
12+
- Fix false-positive orphan warnings on Windows: file paths from the filesystem
13+
are now normalized to forward slashes before comparing against markdown
14+
references ([#63]).
15+
- Fix code block detection on Windows: fenced code block regexes now handle
16+
CRLF line endings, fixing zero code-block counts when files are checked out
17+
with Windows-style line endings.
18+
- Fix backslash paths in token count keys, result messages, and GitHub Actions
19+
annotations on Windows.
20+
- Add Windows (`windows-latest`) to CI test matrix.
21+
22+
## [1.5.3]
23+
24+
### Fixed
25+
26+
- Contamination warnings now only fire when multiple application programming
27+
languages are detected. Skills containing only auxiliary languages (shell,
28+
config formats) no longer trigger false positives ([#60], [#62]).
29+
830
## [1.5.2]
931

1032
### Fixed
@@ -167,17 +189,28 @@ First stable release. Includes the complete CLI and importable library packages.
167189
- `types` — shared data types (`Report`, `Result`, `Level`, etc.)
168190
- `judge.LLMClient` interface for custom LLM providers
169191

192+
[1.5.4]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.3...v1.5.4
193+
[1.5.3]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.2...v1.5.3
194+
[1.5.2]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.1...v1.5.2
195+
[1.5.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.5.0...v1.5.1
196+
[1.5.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.4.0...v1.5.0
170197
[1.4.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.3.1...v1.4.0
171198
[1.3.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.3.0...v1.3.1
172199
[1.3.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.2.1...v1.3.0
173200
[1.2.1]: https://github.com/agent-ecosystem/skill-validator/compare/v1.2.0...v1.2.1
174201
[1.2.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.1.0...v1.2.0
175202
[1.1.0]: https://github.com/agent-ecosystem/skill-validator/compare/v1.0.0...v1.1.0
203+
[#23]: https://github.com/agent-ecosystem/skill-validator/issues/23
204+
[#26]: https://github.com/agent-ecosystem/skill-validator/issues/26
205+
[#27]: https://github.com/agent-ecosystem/skill-validator/issues/27
176206
[#33]: https://github.com/agent-ecosystem/skill-validator/issues/33
177207
[#34]: https://github.com/agent-ecosystem/skill-validator/pull/34
178208
[#35]: https://github.com/agent-ecosystem/skill-validator/pull/35
179209
[#37]: https://github.com/agent-ecosystem/skill-validator/pull/37
180-
[#26]: https://github.com/agent-ecosystem/skill-validator/issues/26
181-
[#23]: https://github.com/agent-ecosystem/skill-validator/issues/23
182-
[#27]: https://github.com/agent-ecosystem/skill-validator/issues/27
183210
[#39]: https://github.com/agent-ecosystem/skill-validator/issues/39
211+
[#43]: https://github.com/agent-ecosystem/skill-validator/issues/43
212+
[#44]: https://github.com/agent-ecosystem/skill-validator/pull/44
213+
[#45]: https://github.com/agent-ecosystem/skill-validator/issues/45
214+
[#60]: https://github.com/agent-ecosystem/skill-validator/issues/60
215+
[#62]: https://github.com/agent-ecosystem/skill-validator/pull/62
216+
[#63]: https://github.com/agent-ecosystem/skill-validator/issues/63

cmd/root.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/agent-ecosystem/skill-validator/types"
1212
)
1313

14-
const version = "v1.5.3"
14+
const version = "v1.5.4"
1515

1616
var (
1717
outputFormat string

judge/judge_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,9 @@ func TestLatestByFile_Empty(t *testing.T) {
641641

642642
func TestCacheDir(t *testing.T) {
643643
dir := CacheDir("/path/to/skill")
644-
if dir != "/path/to/skill/.score_cache" {
645-
t.Errorf("got %s, want /path/to/skill/.score_cache", dir)
644+
want := filepath.Join("/path/to/skill", ".score_cache")
645+
if dir != want {
646+
t.Errorf("got %s, want %s", dir, want)
646647
}
647648
}
648649

report/annotations.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func formatAnnotation(skillDir string, res types.Result, workDir string) string
4949
if err != nil {
5050
relPath = absPath // fall back to absolute if Rel fails
5151
}
52-
params = fmt.Sprintf(" file=%s", relPath)
52+
params = fmt.Sprintf(" file=%s", filepath.ToSlash(relPath))
5353
if res.Line > 0 {
5454
params += fmt.Sprintf(",line=%d", res.Line)
5555
}

structure/markdown.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func CheckMarkdown(dir, body string) []types.Result {
3939
if err != nil {
4040
continue
4141
}
42-
relPath := filepath.Join("references", entry.Name())
42+
relPath := "references/" + entry.Name()
4343
if line, ok := FindUnclosedFence(string(data)); ok {
4444
results = append(results, ctx.ErrorAtLinef(relPath, line,
4545
"%s has an unclosed code fence starting at line %d — this may cause agents to misinterpret everything after it as code", relPath, line))

structure/orphans.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func inventoryFiles(dir string) []string {
204204
return nil
205205
}
206206
rel, _ := filepath.Rel(dir, path)
207-
files = append(files, rel)
207+
files = append(files, filepath.ToSlash(rel))
208208
return nil
209209
})
210210
if err != nil {
@@ -216,7 +216,7 @@ func inventoryFiles(dir string) []string {
216216

217217
// filesInDir returns inventory entries that start with the given directory prefix.
218218
func filesInDir(inventory []string, dir string) []string {
219-
prefix := dir + string(filepath.Separator)
219+
prefix := dir + "/"
220220
var out []string
221221
for _, f := range inventory {
222222
if strings.HasPrefix(f, prefix) {
@@ -239,8 +239,10 @@ func containsReference(text, sourceDir, relPath string) bool {
239239
// path relative to that directory appears in the text.
240240
if sourceDir != "" {
241241
rel, err := filepath.Rel(sourceDir, relPath)
242-
if err == nil && !strings.HasPrefix(rel, "..") && strings.Contains(text, rel) {
243-
return true
242+
if err == nil && !strings.HasPrefix(rel, "..") {
243+
if strings.Contains(text, filepath.ToSlash(rel)) {
244+
return true
245+
}
244246
}
245247
}
246248
return false
@@ -313,10 +315,10 @@ func pythonImportReaches(text, source, relPath string) bool {
313315
}
314316

315317
// Convert dotted module path to file path: helpers.merge_runs → helpers/merge_runs
316-
modulePath := strings.ReplaceAll(module, ".", string(filepath.Separator))
318+
modulePath := strings.ReplaceAll(module, ".", "/")
317319

318320
// Try resolving as a .py file relative to the source directory.
319-
candidate := filepath.Join(resolveDir, modulePath+".py")
321+
candidate := filepath.ToSlash(filepath.Join(resolveDir, modulePath+".py"))
320322
if candidate == relPath {
321323
return true
322324
}
@@ -350,8 +352,8 @@ func pythonPackageInits(text, source, dir string) []string {
350352
continue
351353
}
352354

353-
modulePath := strings.ReplaceAll(module, ".", string(filepath.Separator))
354-
initPath := filepath.Join(resolveDir, modulePath, "__init__.py")
355+
modulePath := strings.ReplaceAll(module, ".", "/")
356+
initPath := filepath.ToSlash(filepath.Join(resolveDir, modulePath, "__init__.py"))
355357

356358
// Check if the __init__.py actually exists on disk.
357359
if _, err := os.Stat(filepath.Join(dir, initPath)); err == nil {

structure/orphans_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,56 @@ func TestCheckOrphanFiles(t *testing.T) {
378378
})
379379
}
380380

381+
func TestContainsReference(t *testing.T) {
382+
t.Run("forward-slash path matches markdown text", func(t *testing.T) {
383+
// Inventory paths are normalized to forward slashes; markdown uses forward slashes.
384+
if !containsReference("See references/other.md for details.", "", "references/other.md") {
385+
t.Error("expected forward-slash inventory path to match forward-slash markdown reference")
386+
}
387+
})
388+
389+
t.Run("relative path from subdirectory matches", func(t *testing.T) {
390+
// Source is in references/, relPath is references/images/diagram.png,
391+
// so the relative path is images/diagram.png.
392+
text := "See ![diagram](images/diagram.png)."
393+
if !containsReference(text, "references", "references/images/diagram.png") {
394+
t.Error("expected relative path from subdirectory to match")
395+
}
396+
})
397+
}
398+
399+
func TestFilesInDir(t *testing.T) {
400+
t.Run("forward-slash inventory matches directory prefix", func(t *testing.T) {
401+
inventory := []string{
402+
"references/guide.md",
403+
"references/other.md",
404+
"scripts/setup.sh",
405+
}
406+
got := filesInDir(inventory, "references")
407+
if len(got) != 2 {
408+
t.Errorf("expected 2 files in references/, got %d: %v", len(got), got)
409+
}
410+
})
411+
}
412+
413+
func TestPythonImportReaches(t *testing.T) {
414+
t.Run("dotted import resolves with forward slashes", func(t *testing.T) {
415+
// "from helpers.merge_runs import merge" in scripts/main.py should
416+
// resolve to scripts/helpers/merge_runs.py (forward slashes).
417+
text := "from helpers.merge_runs import merge"
418+
if !pythonImportReaches(text, "scripts/main.py", "scripts/helpers/merge_runs.py") {
419+
t.Error("expected dotted Python import to resolve to forward-slash path")
420+
}
421+
})
422+
423+
t.Run("relative import resolves with forward slashes", func(t *testing.T) {
424+
text := "from .utils import helper"
425+
if !pythonImportReaches(text, "scripts/pkg/main.py", "scripts/pkg/utils.py") {
426+
t.Error("expected relative Python import to resolve to forward-slash path")
427+
}
428+
})
429+
}
430+
381431
func TestCheckFlatOrphanFiles(t *testing.T) {
382432
t.Run("all root files referenced", func(t *testing.T) {
383433
dir := t.TempDir()

structure/orphans_windows_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//go:build windows
2+
3+
package structure
4+
5+
import (
6+
"strings"
7+
"testing"
8+
9+
"github.com/agent-ecosystem/skill-validator/types"
10+
)
11+
12+
func TestInventoryFilesUsesForwardSlashes(t *testing.T) {
13+
dir := t.TempDir()
14+
writeFile(t, dir, "references/guide.md", "guide content")
15+
writeFile(t, dir, "references/images/diagram.png", "fake image")
16+
writeFile(t, dir, "scripts/setup.sh", "#!/bin/bash")
17+
18+
inventory := inventoryFiles(dir)
19+
for _, rel := range inventory {
20+
if strings.Contains(rel, `\`) {
21+
t.Errorf("inventory path contains backslash: %s", rel)
22+
}
23+
}
24+
}
25+
26+
func TestOrphanCheckWithForwardSlashReferences(t *testing.T) {
27+
// This is the exact scenario from issue #63: skill author writes
28+
// forward-slash paths in SKILL.md, which is the cross-platform convention.
29+
// On Windows, filepath.WalkDir returns backslash paths, so the orphan
30+
// checker must normalize before comparing.
31+
dir := t.TempDir()
32+
writeFile(t, dir, "references/other.md", "reference content")
33+
34+
body := "See references/other.md."
35+
results := CheckOrphanFiles(dir, body, Options{})
36+
37+
requireResult(t, results, types.Pass, "all files in references/ are referenced")
38+
requireNoLevel(t, results, types.Warning)
39+
}
40+
41+
func TestOrphanCheckNestedForwardSlashReference(t *testing.T) {
42+
dir := t.TempDir()
43+
writeFile(t, dir, "references/guide.md", "See ![diagram](images/diagram.png).")
44+
writeFile(t, dir, "references/images/diagram.png", "fake image")
45+
46+
body := "Read the [guide](references/guide.md)."
47+
results := CheckOrphanFiles(dir, body, Options{})
48+
49+
requireNoResultContaining(t, results, types.Warning, "diagram.png")
50+
requireResult(t, results, types.Pass, "all files in references/ are referenced")
51+
}
52+
53+
func TestPythonImportResolvesOnWindows(t *testing.T) {
54+
dir := t.TempDir()
55+
writeFile(t, dir, "scripts/main.py", "from helpers.merge_runs import merge\nmerge()")
56+
writeFile(t, dir, "scripts/helpers/__init__.py", "")
57+
writeFile(t, dir, "scripts/helpers/merge_runs.py", "def merge(): pass")
58+
59+
body := "Run scripts/main.py to start."
60+
results := CheckOrphanFiles(dir, body, Options{})
61+
62+
requireNoResultContaining(t, results, types.Warning, "merge_runs.py")
63+
requireResult(t, results, types.Pass, "all files in scripts/ are referenced")
64+
}

structure/tokens.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ func CheckTokens(dir, body string, opts Options) ([]types.Result, []types.TokenC
8181
path := filepath.Join(refsDir, entry.Name())
8282
data, err := os.ReadFile(path)
8383
if err != nil {
84-
relPath := filepath.Join("references", entry.Name())
84+
relPath := "references/" + entry.Name()
8585
results = append(results, ctx.WarnFilef(relPath, "could not read %s: %v", relPath, err))
8686
continue
8787
}
8888
tokens, _, _ := enc.Encode(string(data))
8989
fileTokens := len(tokens)
90-
relPath := filepath.Join("references", entry.Name())
90+
relPath := "references/" + entry.Name()
9191
counts = append(counts, types.TokenCount{
9292
File: relPath,
9393
Tokens: fileTokens,
@@ -251,7 +251,7 @@ func countAssetFiles(dir string, enc tokenizer.Codec) []types.TokenCount {
251251
}
252252
rel, _ := filepath.Rel(dir, path)
253253
tokens, _, _ := enc.Encode(string(data))
254-
counts = append(counts, types.TokenCount{File: rel, Tokens: len(tokens)})
254+
counts = append(counts, types.TokenCount{File: filepath.ToSlash(rel), Tokens: len(tokens)})
255255
return nil
256256
})
257257

@@ -323,7 +323,7 @@ func countFilesInDir(rootDir, dirName string, enc tokenizer.Codec) []types.Token
323323
}
324324
rel, _ := filepath.Rel(rootDir, path)
325325
tokens, _, _ := enc.Encode(string(data))
326-
counts = append(counts, types.TokenCount{File: rel, Tokens: len(tokens)})
326+
counts = append(counts, types.TokenCount{File: filepath.ToSlash(rel), Tokens: len(tokens)})
327327
return nil
328328
})
329329

0 commit comments

Comments
 (0)