From 293d2129c473766ff60087f9054dbd0d435adcad Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 14:22:22 +0000 Subject: [PATCH 1/3] feat(scanner): Skip testdata directories by default Fixture repositories under testdata are not the project's source. Scanning them inflates file counts, and once a fixture is written in a language the file-level edge model does not cover, it changes the project's reported coverage: a three-file Swift fixture is enough to make an entire Go project report partial. Go's own toolchain ignores the directory for the same reason, so this matches the convention users already expect, alongside vendor and node_modules. A fixture is still scannable when it is itself the scan root, which is how fixture tests use them; only testdata encountered during a walk is skipped. Relates to #172 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo --- scanner/ignoretestdata_test.go | 63 ++++++++++++++++++++++++++++++++++ scanner/walker.go | 10 ++++-- 2 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 scanner/ignoretestdata_test.go diff --git a/scanner/ignoretestdata_test.go b/scanner/ignoretestdata_test.go new file mode 100644 index 0000000..94ce9b3 --- /dev/null +++ b/scanner/ignoretestdata_test.go @@ -0,0 +1,63 @@ +package scanner + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// Fixture repositories under testdata are not the project's source. Scanning +// them inflates file counts and lets a fixture's language change the project's +// reported coverage — a Swift fixture made the whole project report partial. +// Go's own toolchain ignores the directory for the same reason. +func TestScanSkipsTestdataDirectories(t *testing.T) { + root := t.TempDir() + write := func(rel, body string) { + t.Helper() + path := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + write("main.go", "package main\n\nfunc main() {}\n") + write("testdata/fixture/main.go", "package main\n\nfunc main() {}\n") + write("internal/testdata/nested/app.go", "package nested\n") + + files, err := ScanFiles(context.Background(), root, NewGitIgnoreCache(root), nil, nil) + if err != nil { + t.Fatalf("scan: %v", err) + } + for _, file := range files { + if file.Path != "main.go" { + t.Errorf("scan returned %q, want only main.go — testdata must be skipped at any depth", file.Path) + } + } + if len(files) != 1 { + t.Fatalf("scan returned %d files, want 1", len(files)) + } +} + +// A fixture is still scannable when it is itself the root, which is how the +// fixture tests in this package use them. +func TestTestdataFixtureScansWhenItIsTheRoot(t *testing.T) { + root := t.TempDir() + fixture := filepath.Join(root, "testdata", "fixture") + if err := os.MkdirAll(fixture, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(fixture, "app.go"), []byte("package app\n"), 0o644); err != nil { + t.Fatal(err) + } + + files, err := ScanFiles(context.Background(), fixture, NewGitIgnoreCache(fixture), nil, nil) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(files) != 1 || files[0].Path != "app.go" { + t.Fatalf("scan of the fixture root returned %v, want [app.go]", files) + } +} diff --git a/scanner/walker.go b/scanner/walker.go index 5c4d7a8..8bae2ac 100644 --- a/scanner/walker.go +++ b/scanner/walker.go @@ -113,9 +113,13 @@ func (c *GitIgnoreCache) ShouldIgnore(absPath string) bool { // IgnoredDirs are directories to skip during scanning var IgnoredDirs = map[string]bool{ - ".git": true, - "node_modules": true, - "vendor": true, + ".git": true, + "node_modules": true, + "vendor": true, + // Go's own toolchain ignores testdata, and fixture repositories inside it + // are not the project's source: scanning them inflates file counts and + // lets a fixture's language change the project's reported coverage. + "testdata": true, "Pods": true, "build": true, "DerivedData": true, From d0aa22ddd2ad790a1786ae96fe9ea416cd7256fa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 14:29:36 +0000 Subject: [PATCH 2/3] fix: Port the cargo-timeout and readiness fixes so CI can go green MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two failures on this PR, neither from the testdata exclusion — scanner passed on both legs. codemap/mcp: the cargo-metadata deadline failed the whole graph build instead of falling back. Ported from @reneleonhardt's #171. codemap root: TestRunWatchModeRunDaemonAndWatchStart hit "reading daemon readiness: unexpected end of JSON input" — the same window #177 fixes, in a different test than the one that led me to open it. waitWatchReadiness gave up on the first unparseable read of a file still being written. Both no-op once main carries them. Relates to #172 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo --- main.go | 23 +++++++++----- scanner/rustcargo.go | 11 +++++-- watch_readiness_test.go | 66 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 watch_readiness_test.go diff --git a/main.go b/main.go index e14ae1b..27f8d0c 100644 --- a/main.go +++ b/main.go @@ -1153,22 +1153,31 @@ func publishWatchReadiness(path string, readinessErr error) error { func waitWatchReadiness(path string, timeout time.Duration) error { deadline := time.Now().Add(timeout) + // A readiness file that does not parse is a file still being written, not + // a daemon that failed: publishWatchReadiness renames its payload into + // place atomically, but nothing guarantees every writer does, and treating + // the first unparseable read as fatal reported a startup failure for a + // daemon that was starting fine. Keep the last parse error so a file that + // never becomes valid says why, rather than only that it timed out. + var lastParseErr error for { data, err := os.ReadFile(path) if err == nil { var status watchReadiness - if err := json.Unmarshal(data, &status); err != nil { - return fmt.Errorf("reading daemon readiness: %w", err) - } - if status.Error != "" { + if parseErr := json.Unmarshal(data, &status); parseErr != nil { + lastParseErr = parseErr + } else if status.Error != "" { return errors.New(status.Error) + } else { + return nil } - return nil - } - if !errors.Is(err, os.ErrNotExist) { + } else if !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("reading daemon readiness: %w", err) } if time.Now().After(deadline) { + if lastParseErr != nil { + return fmt.Errorf("reading daemon readiness: %w", lastParseErr) + } return fmt.Errorf("daemon readiness timed out after %s", timeout) } time.Sleep(10 * time.Millisecond) diff --git a/scanner/rustcargo.go b/scanner/rustcargo.go index 9f08133..90db77d 100644 --- a/scanner/rustcargo.go +++ b/scanner/rustcargo.go @@ -100,6 +100,10 @@ func parseCargoMetadata(data []byte) (cargoMetadata, error) { } func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAnalysis, files []FileInfo, loader cargoMetadataLoader) (*rustWorkspaceIndex, *ScanSourceOutcome, error) { + return buildRustWorkspaceIndexWithTimeout(ctx, root, analyses, files, loader, cargoMetadataTimeout) +} + +func buildRustWorkspaceIndexWithTimeout(ctx context.Context, root string, analyses []FileAnalysis, files []FileInfo, loader cargoMetadataLoader, metadataTimeout time.Duration) (*rustWorkspaceIndex, *ScanSourceOutcome, error) { index := buildRustFallbackWorkspaceIndex(root, analyses) manifestPaths, err := discoverCargoManifests(ctx, root, files) if err != nil { @@ -115,7 +119,7 @@ func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAn outcome := cargoMetadataOutcome(0, len(manifestPaths)) return index, &outcome, nil } - ctx, cancel := context.WithTimeout(ctx, cargoMetadataTimeout) + metadataCtx, cancel := context.WithTimeout(ctx, metadataTimeout) defer cancel() packagesByRoot := make(map[string]rustPackage, len(index.packages)) @@ -129,11 +133,14 @@ func buildRustWorkspaceIndex(ctx context.Context, root string, analyses []FileAn if err := ctx.Err(); err != nil { return nil, nil, err } + if metadataCtx.Err() != nil { + break + } manifestPath = filepath.Clean(manifestPath) if handledManifests[manifestPath] { continue } - data, err := loader(ctx, manifestPath) + data, err := loader(metadataCtx, manifestPath) if err != nil { continue } diff --git a/watch_readiness_test.go b/watch_readiness_test.go new file mode 100644 index 0000000..e2a10cb --- /dev/null +++ b/watch_readiness_test.go @@ -0,0 +1,66 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// A readiness file caught mid-write reads as zero or partial bytes. Treating +// that as fatal reported a startup failure for a daemon that was starting +// fine, and made TestRunWatchStartWaitsForChildReadinessFailure fail whenever +// the machine was loaded enough to land in the window. +func TestWaitWatchReadinessWaitsThroughPartialWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, nil, 0o644); err != nil { + t.Fatal(err) + } + go func() { + time.Sleep(50 * time.Millisecond) + _ = os.WriteFile(path, []byte(`{"error":"claim rejected"}`), 0o644) + }() + + err := waitWatchReadiness(path, 5*time.Second) + if err == nil || !strings.Contains(err.Error(), "claim rejected") { + t.Fatalf("waitWatchReadiness() = %v, want the daemon's own error once the file is complete", err) + } +} + +func TestWaitWatchReadinessSucceedsAfterPartialWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, []byte(`{"err`), 0o644); err != nil { + t.Fatal(err) + } + go func() { + time.Sleep(50 * time.Millisecond) + _ = os.WriteFile(path, []byte(`{}`), 0o644) + }() + + if err := waitWatchReadiness(path, 5*time.Second); err != nil { + t.Fatalf("waitWatchReadiness() = %v, want success once the file is complete", err) + } +} + +// A file that never becomes valid still has to say why, rather than reporting +// only that it timed out. +func TestWaitWatchReadinessReportsPersistentGarbage(t *testing.T) { + path := filepath.Join(t.TempDir(), "ready.json") + if err := os.WriteFile(path, []byte("not json at all"), 0o644); err != nil { + t.Fatal(err) + } + err := waitWatchReadiness(path, 200*time.Millisecond) + if err == nil || !strings.Contains(err.Error(), "reading daemon readiness") { + t.Fatalf("waitWatchReadiness() = %v, want the parse failure surfaced", err) + } +} + +// A missing file must still time out rather than hang. +func TestWaitWatchReadinessTimesOutWhenAbsent(t *testing.T) { + path := filepath.Join(t.TempDir(), "never-written.json") + err := waitWatchReadiness(path, 100*time.Millisecond) + if err == nil || !strings.Contains(err.Error(), "timed out") { + t.Fatalf("waitWatchReadiness() = %v, want a timeout", err) + } +} From 5b4dd0d88d6f6608a6fcd6ecc16e31eb705c5d6a Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Fri, 4 Sep 2026 10:47:42 -0400 Subject: [PATCH 3/3] fix(scanner): Never apply the ignore list to the scan root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IgnoredDirs is matched on a directory's base name, and the walk root is matched along with everything under it. So `codemap testdata/`, or `codemap .` from inside vendor/ or node_modules/, returned Files: 0 — no error, no message, just an empty result for the directory the user explicitly named. Adding testdata to the list widened the set of paths that hit this. Exempt the walk root from the fast-path check in ScanFiles, and from the same check in ReadExternalDeps so a manifest at that root is still read. Nested directories with those names are still skipped, at any depth. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t --- scanner/deps.go | 4 ++- scanner/ignoretestdata_test.go | 50 ++++++++++++++++++++++++++++++++++ scanner/walker.go | 7 +++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/scanner/deps.go b/scanner/deps.go index 15e5956..a63a6e4 100644 --- a/scanner/deps.go +++ b/scanner/deps.go @@ -36,7 +36,9 @@ func ReadExternalDeps(ctx context.Context, root string, manifestByteBudget int64 return nil } if info.IsDir() { - if IgnoredDirs[info.Name()] { + // The walk root is what the user asked for, so it is never one of + // the hardcoded ignored directories. + if IgnoredDirs[info.Name()] && path != root { return filepath.SkipDir } return nil diff --git a/scanner/ignoretestdata_test.go b/scanner/ignoretestdata_test.go index 94ce9b3..cdc475f 100644 --- a/scanner/ignoretestdata_test.go +++ b/scanner/ignoretestdata_test.go @@ -41,6 +41,56 @@ func TestScanSkipsTestdataDirectories(t *testing.T) { } } +// The hardcoded ignore list must never apply to the directory the user asked +// for. `codemap testdata/`, or `cd vendor && codemap .`, previously matched the +// root's own base name and returned Files: 0 with no error and no explanation. +func TestIgnoredDirNameScansWhenItIsTheRoot(t *testing.T) { + for _, name := range []string{"testdata", "vendor", "node_modules"} { + t.Run(name, func(t *testing.T) { + root := filepath.Join(t.TempDir(), name) + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "app.go"), []byte("package app\n"), 0o644); err != nil { + t.Fatal(err) + } + + files, err := ScanFiles(context.Background(), root, NewGitIgnoreCache(root), nil, nil) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(files) != 1 || files[0].Path != "app.go" { + t.Fatalf("scan of %s/ as the root returned %v, want [app.go]", name, files) + } + }) + } +} + +// Nested ignored directories are still skipped when the root is itself named +// like one: the exemption covers the root only, not every directory sharing +// its name. +func TestIgnoredDirRootStillSkipsNestedIgnoredDirs(t *testing.T) { + root := filepath.Join(t.TempDir(), "testdata") + nested := filepath.Join(root, "fixture", "testdata") + if err := os.MkdirAll(nested, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "app.go"), []byte("package app\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(nested, "deep.go"), []byte("package deep\n"), 0o644); err != nil { + t.Fatal(err) + } + + files, err := ScanFiles(context.Background(), root, NewGitIgnoreCache(root), nil, nil) + if err != nil { + t.Fatalf("scan: %v", err) + } + if len(files) != 1 || files[0].Path != "app.go" { + t.Fatalf("scan returned %v, want only [app.go] — nested testdata must still be skipped", files) + } +} + // A fixture is still scannable when it is itself the root, which is how the // fixture tests in this package use them. func TestTestdataFixtureScansWhenItIsTheRoot(t *testing.T) { diff --git a/scanner/walker.go b/scanner/walker.go index 8bae2ac..0b17c0a 100644 --- a/scanner/walker.go +++ b/scanner/walker.go @@ -251,8 +251,11 @@ func ScanFiles(ctx context.Context, root string, cache *GitIgnoreCache, only []s name := info.Name() - // Fast path: skip hardcoded ignored dirs/files - if IgnoredDirs[name] { + // Fast path: skip hardcoded ignored dirs/files. Never applied to the + // walk root itself: a user who runs `codemap testdata/` or scans from + // inside vendor/ asked for exactly that tree, and skipping it here + // returns Files: 0 with no explanation. + if IgnoredDirs[name] && path != absRoot { if info.IsDir() { return filepath.SkipDir }