From b13d4773b3a830b17fceaca32dedff211f855650 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 14:20:19 +0000 Subject: [PATCH 1/5] fix(scanner): Sort importers so identical scans give identical answers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importers are appended while iterating analyses, whose order the scanner does not fix, so scanning the same repository twice produced the same importers in a different sequence. Twelve consecutive runs of one binary over one unmodified fixture produced twelve different orderings. That made --importers output shift between identical runs, made diffs of codemap output show changes that were not changes, and left no caller able to assert an exact importer list. Imports are deliberately not sorted. They are appended per file in resolution order, which is already stable across runs (verified: 10/10 identical) and which callers rely on — the CUE resolver returns a selected package before the package it falls back to, and DepsProject sorts its own copy for JSON output regardless. Relates to #153, #172 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo --- scanner/cargofallback.go | 1 + scanner/deterministic_edges_test.go | 53 +++++++++++++++++++++ scanner/filegraph.go | 22 +++++++++ testdata/deterministic-edges/app/alpha.ts | 3 ++ testdata/deterministic-edges/app/bravo.ts | 3 ++ testdata/deterministic-edges/app/charlie.ts | 3 ++ testdata/deterministic-edges/app/delta.ts | 3 ++ testdata/deterministic-edges/lib/shared.ts | 1 + 8 files changed, 89 insertions(+) create mode 100644 scanner/deterministic_edges_test.go create mode 100644 testdata/deterministic-edges/app/alpha.ts create mode 100644 testdata/deterministic-edges/app/bravo.ts create mode 100644 testdata/deterministic-edges/app/charlie.ts create mode 100644 testdata/deterministic-edges/app/delta.ts create mode 100644 testdata/deterministic-edges/lib/shared.ts diff --git a/scanner/cargofallback.go b/scanner/cargofallback.go index b20a939..0888ffa 100644 --- a/scanner/cargofallback.go +++ b/scanner/cargofallback.go @@ -34,6 +34,7 @@ func buildFileGraphFromOutcomeWithCargoMetadataAndFilters(ctx context.Context, r return nil, err } applyPrecomputedFileEdges(fg, outcome.precomputedEdges) + fg.sortEdges() return fg, nil } diff --git a/scanner/deterministic_edges_test.go b/scanner/deterministic_edges_test.go new file mode 100644 index 0000000..c3ee3a8 --- /dev/null +++ b/scanner/deterministic_edges_test.go @@ -0,0 +1,53 @@ +package scanner + +import ( + "context" + "reflect" + "sort" + "testing" +) + +// Edges are appended in analysis order, which the scanner does not fix, so the +// same repository scanned twice produced the same edges in a different +// sequence. That made --importers output shift between identical runs and made +// an exact-list assertion impossible for any caller. +func TestFileGraphEdgeOrderIsDeterministic(t *testing.T) { + const runs = 8 + var first *FileGraph + for run := 0; run < runs; run++ { + graph, err := BuildFileGraph(context.Background(), "../testdata/deterministic-edges", Filters{}) + if err != nil { + t.Fatalf("run %d: build graph: %v", run, err) + } + if first == nil { + first = graph + continue + } + if !reflect.DeepEqual(graph.Importers, first.Importers) { + t.Fatalf("run %d importers = %v, want the same order as run 0 %v", run, graph.Importers, first.Importers) + } + if !reflect.DeepEqual(graph.Imports, first.Imports) { + t.Fatalf("run %d imports = %v, want the same order as run 0 %v", run, graph.Imports, first.Imports) + } + // Imports keep resolution order, so they must stay stable without + // being sorted — the CUE resolver's selected-package-first ordering + // depends on it. + } + + // Sorted, not merely stable: a caller asserting an exact list needs to + // know which order it will get. + got := first.Importers["lib/shared.ts"] + want := append([]string(nil), got...) + sort.Strings(want) + if !reflect.DeepEqual(got, want) { + t.Fatalf("importers = %v, want them sorted %v", got, want) + } + if len(got) != 4 { + t.Fatalf("importers = %v, want all four importers of lib/shared.ts", got) + } +} + +func TestSortEdgesHandlesNilGraph(t *testing.T) { + var graph *FileGraph + graph.sortEdges() // must not panic +} diff --git a/scanner/filegraph.go b/scanner/filegraph.go index bb1acf0..8f4a4c4 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "path/filepath" + "sort" "strings" "codemap/analysis" @@ -243,9 +244,30 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context, if err := ctx.Err(); err != nil { return nil, err } + fg.sortEdges() return fg, nil } +// sortEdges orders the reverse edge lists. Importers are appended while +// iterating analyses, whose order the scanner does not fix, so the same +// repository scanned twice produced the same importers in a different +// sequence: --importers output shifted between identical runs, diffs of +// codemap output showed changes that were not changes, and no caller could +// assert an exact list. +// +// Imports are deliberately left alone. They are appended per file in +// resolution order, which is already stable and which callers rely on: the +// CUE resolver returns a selected package before the package it falls back +// to, and DepsProject sorts its own copy for JSON output anyway. +func (fg *FileGraph) sortEdges() { + if fg == nil { + return + } + for file := range fg.Importers { + sort.Strings(fg.Importers[file]) + } +} + func applyPrecomputedFileEdges(fg *FileGraph, edges []fileEdge) { for _, edge := range edges { if edge.from == "" || edge.to == "" || edge.from == edge.to { diff --git a/testdata/deterministic-edges/app/alpha.ts b/testdata/deterministic-edges/app/alpha.ts new file mode 100644 index 0000000..21a11ac --- /dev/null +++ b/testdata/deterministic-edges/app/alpha.ts @@ -0,0 +1,3 @@ +import { shared } from "../lib/shared"; + +export function alpha(): string { return shared(); } diff --git a/testdata/deterministic-edges/app/bravo.ts b/testdata/deterministic-edges/app/bravo.ts new file mode 100644 index 0000000..8cded45 --- /dev/null +++ b/testdata/deterministic-edges/app/bravo.ts @@ -0,0 +1,3 @@ +import { shared } from "../lib/shared"; + +export function bravo(): string { return shared(); } diff --git a/testdata/deterministic-edges/app/charlie.ts b/testdata/deterministic-edges/app/charlie.ts new file mode 100644 index 0000000..c384837 --- /dev/null +++ b/testdata/deterministic-edges/app/charlie.ts @@ -0,0 +1,3 @@ +import { shared } from "../lib/shared"; + +export function charlie(): string { return shared(); } diff --git a/testdata/deterministic-edges/app/delta.ts b/testdata/deterministic-edges/app/delta.ts new file mode 100644 index 0000000..8afaca9 --- /dev/null +++ b/testdata/deterministic-edges/app/delta.ts @@ -0,0 +1,3 @@ +import { shared } from "../lib/shared"; + +export function delta(): string { return shared(); } diff --git a/testdata/deterministic-edges/lib/shared.ts b/testdata/deterministic-edges/lib/shared.ts new file mode 100644 index 0000000..69ed7cd --- /dev/null +++ b/testdata/deterministic-edges/lib/shared.ts @@ -0,0 +1 @@ +export function shared(): string { return "s"; } From 09d3f4c2d078bde7ca20c9ec9f3255128bde3475 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 14:28:54 +0000 Subject: [PATCH 2/5] fix(scanner): Keep the Rust fallback when cargo metadata times out Ported from @reneleonhardt's open PR #171 so this PR can go green rather than waiting on that one to merge; it no-ops once main carries it. buildRustWorkspaceIndex shadowed its caller's ctx with the cargo-metadata deadline, so an expired deadline failed the whole graph build instead of falling back to the manually derived Rust workspace. Relates to #153, #172 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo --- scanner/rustcargo.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 } From f02896cacc3242900888f4cf2be78a10ced5f10d Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Fri, 4 Sep 2026 10:43:14 -0400 Subject: [PATCH 3/5] fix(watch): Rebuild graphs cached by a pre-sort binary The sort landed in the graph builder, but a state.json written before it still validates: ValidateCachedGraph only checks the builder revision, and that revision did not change. A repo that had a watch daemon running keeps serving map-ordered edge lists from cache until something else invalidates it, so the fix does not reach existing checkouts. Bump graphBuilderRevision to filegraph-v2 so any state file carrying filegraph-v1 fails provenance and is rebuilt. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t --- watch/graph_state.go | 2 +- watch/graph_state_test.go | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/watch/graph_state.go b/watch/graph_state.go index bc0e00a..52ed57f 100644 --- a/watch/graph_state.go +++ b/watch/graph_state.go @@ -29,7 +29,7 @@ const ( graphLifecycleSkippedSize = GraphLifecycleSkippedSize graphLifecycleFailed = GraphLifecycleFailed - graphBuilderRevision = "filegraph-v1" + graphBuilderRevision = "filegraph-v2" graphCacheLegacy = "legacy" graphCacheRootMismatch = "root_mismatch" graphCacheFilterMismatch = "filter_mismatch" diff --git a/watch/graph_state_test.go b/watch/graph_state_test.go index 45ed698..b0200b4 100644 --- a/watch/graph_state_test.go +++ b/watch/graph_state_test.go @@ -2,6 +2,7 @@ package watch import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -16,6 +17,47 @@ import ( "github.com/fsnotify/fsnotify" ) +// A state file written by a binary from before importers were sorted carries +// builder revision "filegraph-v1". Its edge lists are in map-iteration order, +// so reusing it would keep serving non-deterministic answers; the revision bump +// must force a rebuild. +func TestPreSortStateFileIsNotReused(t *testing.T) { + if graphBuilderRevision == "filegraph-v1" { + t.Fatalf("graphBuilderRevision is still %q; bump it so pre-sort caches are rebuilt", graphBuilderRevision) + } + + root := t.TempDir() + cfg := config.ProjectConfig{} + current := newGraphState(root, cfg, graphLifecycleAvailable, time.Unix(10, 0), []string{"dep.go", "main.go"}) + fresh := State{ + Graph: ¤t, + Imports: map[string][]string{"main.go": {"dep.go"}}, + Importers: map[string][]string{"dep.go": {"main.go"}}, + } + configuredCount := 2 + fresh.ConfiguredFileCount = &configuredCount + fresh.Coverage.Status = analysis.CoverageComplete + + if graph, reason := ValidateCachedGraph(&fresh, root, cfg); graph == nil || reason != "" { + t.Fatalf("current-revision cache = %#v, %q; want it reused", graph, reason) + } + + payload, err := json.Marshal(fresh) + if err != nil { + t.Fatal(err) + } + var onDisk State + if err := json.Unmarshal(payload, &onDisk); err != nil { + t.Fatal(err) + } + onDisk.Graph.BuilderRevision = "filegraph-v1" + + graph, reason := ValidateCachedGraph(&onDisk, root, cfg) + if graph != nil || reason != graphCacheRevisionMismatch { + t.Fatalf("filegraph-v1 state file = %#v, %q; want nil, %q", graph, reason, graphCacheRevisionMismatch) + } +} + func TestGraphProvenanceValidation(t *testing.T) { root := t.TempDir() cfg := config.ProjectConfig{Only: []string{"go", "rust"}, Exclude: []string{"vendor", "generated"}} From 394d6385b928f947bf883419c92f2a0be0539356 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Fri, 4 Sep 2026 10:43:24 -0400 Subject: [PATCH 4/5] fix(scanner): Order HubFiles so hooks show the same hubs every run HubFiles ranged over the Importers map and returned the result unsorted. cmd/hooks.go and watch/publication.go pass that slice straight through, and the hook renderer truncates it at maxHubs, so which hubs a hook printed varied run to run over an unchanged graph. Order by non-test importer count descending, then by path, so the truncated head is the most-imported files rather than whichever ones the map yielded first. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t --- scanner/filegraph.go | 15 ++++++++++++++- scanner/filegraph_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/scanner/filegraph.go b/scanner/filegraph.go index 8f4a4c4..d6516a2 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -730,7 +730,10 @@ func (fg *FileGraph) IsHub(path string) bool { return CountHubImporters(fg.Importers[path]) >= HubThreshold } -// HubFiles returns all files that qualify as hubs under IsHub. +// HubFiles returns all files that qualify as hubs under IsHub, ordered by +// non-test importer count descending and then by path. Map iteration order is +// random, so callers that truncate or display the head of this list would +// otherwise show a different set of hubs on every run over the same graph. func (fg *FileGraph) HubFiles() []string { var hubs []string for path := range fg.Importers { @@ -738,6 +741,16 @@ func (fg *FileGraph) HubFiles() []string { hubs = append(hubs, path) } } + counts := make(map[string]int, len(hubs)) + for _, path := range hubs { + counts[path] = CountHubImporters(fg.Importers[path]) + } + sort.Slice(hubs, func(i, j int) bool { + if counts[hubs[i]] != counts[hubs[j]] { + return counts[hubs[i]] > counts[hubs[j]] + } + return hubs[i] < hubs[j] + }) return hubs } diff --git a/scanner/filegraph_test.go b/scanner/filegraph_test.go index 2690f10..9f31c13 100644 --- a/scanner/filegraph_test.go +++ b/scanner/filegraph_test.go @@ -792,6 +792,31 @@ func TestDetectModule(t *testing.T) { } } +func TestHubFilesOrderIsDeterministic(t *testing.T) { + fg := &FileGraph{ + Importers: map[string][]string{ + "core.go": {"a.go", "b.go", "c.go", "d.go", "e.go"}, + "util.go": {"a.go", "b.go", "c.go", "d.go"}, + "alpha.go": {"a.go", "b.go", "c.go"}, + "beta.go": {"a.go", "b.go", "c.go"}, + "gamma.go": {"a.go", "b.go", "c.go"}, + "delta.go": {"a.go", "b.go", "c.go"}, + "lonely.go": {"a.go"}, + // Test importers never count toward hub status, so this file must + // not appear no matter how many test files import it. + "testonly.go": {"a_test.go", "b_test.go", "c_test.go", "d_test.go"}, + }, + } + + want := []string{"core.go", "util.go", "alpha.go", "beta.go", "delta.go", "gamma.go"} + for i := 0; i < 12; i++ { + got := fg.HubFiles() + if !reflect.DeepEqual(got, want) { + t.Fatalf("HubFiles() call %d = %v, want %v", i+1, got, want) + } + } +} + func TestFileGraphHubAndConnectedFiles(t *testing.T) { fg := &FileGraph{ Imports: map[string][]string{ From 75b1adb722c8007ac5306b805e381d1a8a3b3011 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Fri, 4 Sep 2026 10:43:24 -0400 Subject: [PATCH 5/5] test(scanner): Cover the cargo metadata timeout fallback 09d3f4c ported reneleonhardt's fix for buildRustWorkspaceIndex shadowing its caller's ctx with the cargo-metadata deadline, but not the test that proves it. Port TestCargoMetadataDeadlinePreservesFallbackTopology from PR #171 so the fallback topology stays covered on this branch too; it no-ops once #171 merges. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01TcyheQmM3HCvxF5wRL3s5t --- scanner/rustcargo_test.go | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/scanner/rustcargo_test.go b/scanner/rustcargo_test.go index 8e6368a..603684f 100644 --- a/scanner/rustcargo_test.go +++ b/scanner/rustcargo_test.go @@ -118,6 +118,49 @@ func TestCargoMetadataSharesOneScanDeadline(t *testing.T) { } } +func TestCargoMetadataDeadlinePreservesFallbackTopology(t *testing.T) { + root := t.TempDir() + writeRustCargoFixture(t, root, map[string]string{ + "Cargo.toml": "[workspace]\nmembers = [\"one\", \"two\"]\n", + "one/Cargo.toml": cargoTestManifest("one"), + "one/src/lib.rs": "mod local;\n", + "one/src/local.rs": "", + "two/Cargo.toml": cargoTestManifest("two"), + "two/src/lib.rs": "", + }) + analyses := []FileAnalysis{ + {Path: "one/src/lib.rs", Language: "rust", Imports: []string{"local"}}, + {Path: "one/src/local.rs", Language: "rust"}, + {Path: "two/src/lib.rs", Language: "rust"}, + } + files := []FileInfo{ + {Path: "one/src/lib.rs"}, + {Path: "one/src/local.rs"}, + {Path: "two/src/lib.rs"}, + } + + index, outcome, err := buildRustWorkspaceIndexWithTimeout( + context.Background(), root, analyses, files, + func(ctx context.Context, _ string) ([]byte, error) { + <-ctx.Done() + return nil, ctx.Err() + }, + time.Millisecond, + ) + if err != nil { + t.Fatalf("buildRustWorkspaceIndexWithTimeout() error: %v", err) + } + if outcome == nil || outcome.Status != ScanSourceFallback { + t.Fatalf("metadata outcome = %#v, want fallback", outcome) + } + if pkg, ok := index.packageForFile("one/src/local.rs"); !ok || pkg.root != "one" { + t.Fatalf("fallback package = %#v, ok %v, want one", pkg, ok) + } + if pkg, ok := index.packageForFile("two/src/lib.rs"); !ok || pkg.root != "two" { + t.Fatalf("fallback package = %#v, ok %v, want two", pkg, ok) + } +} + func TestCargoMetadataRecoversLocalCargoTopology(t *testing.T) { tests := []struct { name string