Skip to content

fix(scanner): Windows residue of #214: workspace roots, gitignore probes, cargo edges as slash keys; tests spell keys with slashes - #219

Merged
JordanCoin merged 3 commits into
mainfrom
fix/214-windows-residue
Oct 11, 2026
Merged

JordanCoin merged 3 commits into
mainfrom
fix/214-windows-residue

Conversation

@JordanCoin

@JordanCoin JordanCoin commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Residue of #214 after #218 merged. The informational Windows cell on #218's run had 81 top-level failing tests (down from 124). This PR takes it to 10, in two iterations read from the Windows cell.

Before (81), first assertion per test, grouped by cause:

group cause before after
1 resolver builds a native probe or root and compares it to a slash key (product bug: wrong answer on Windows) 31 7
2 test expectation spelled with filepath.Join or filepath.FromSlash literals, i.e. native separators 25 2
3 temp-dir or absolute-path assumptions (/abs, /etc, /setup/root, Rel against C:\...) 6 0 (skipped by name)
4 Unix tool, shell or binary name (codemap without .exe for a second binary, rename-while-open) 5 0 (skipped by name)
5 console / ANSI colour on a non-TTY 2 0 (skipped by name)
6 other (hub warnings in hooks, doctor root, topology, plugin install, burst timing) 12 1 (burst test timing, also flaky on Linux: #215 follow-up)

Product fixes (group 1), all at chokepoints:

  • cleanRepoPath (JS workspace roots) and pathContains are slash-form; pathWithin likewise.
  • Dart package roots via path.Dir.
  • CUE and Go-fallback file keys via keyPath; the Go fallback's nested-repo prefix check splits on /.
  • Nested .gitignore probes go to go-gitignore as slash paths (on Windows ignored files were scanned and visible ones dropped); nested-repo paths are slash keys.
  • Cargo-metadata fallback edges via keyPath.
  • Rust resolver: every repo-relative path goes through keyPath / path.Join / path.Dir; only calls that touch the disk keep filepath. Crate-rooted, super::, literal #[path] and askama template lookups resolved to nothing on Windows before.

Test side (group 2): 123 all-literal filepath.Join calls and 115 filepath.FromSlash wrappers on expectations in scanner tests are slash literals now; fixture creation that joins a temp root is unchanged; rewrites outside scanner were reverted where a native path is the right expectation (config path, topology repo path, git exclude path).

Skips (groups 3-5 and the non-resolver part of 6): 24 tests outside the resolvers now t.Skip("windows: <cause>, tracked in #214"), so the residue is explicit in the cell rather than a wall of red. Not skipped: anything in scanner that is a product bug.

Remaining 10 on the Windows cell (run 38175032578):

  • Cargo fallback cluster (5): TestCargoFallbackAcceptsDevDependencyFromLibraryTarget, TestScanForGraphOutcomeCargoFallbackResolvesRelativeRoot (metadata reloads = 1, want 0), TestScanForGraphOutcomeCombinesGoAndCargoFallbacks, TestBuildFileGraphUsesCargoFallbackOnce, TestBuildFileGraphFromFallbackOutcomePreservesCargoEdgesWithoutReload: the cargo-metadata loader keys packages by absolute manifest path (handledManifests, rustPackageFromCargoMetadata), where the fixture passes /-form absolute paths that Windows does not treat as the same manifest. Product bug, needs a Windows-side reproduction to fix without guessing.
  • TestBuildFileGraphResolvesNestedCUEPackageFromModule (1): nested cue.mod module root detection; the single-module cases pass.
  • TestDepsBuildFileIndex, TestValidLocalTargetAndManifestLocalPath (2): test-side, assert a byDir key and an absolute temp root built with native joins.
  • TestBuildFileGraphFromOutcomeDedupesFallbackCargoSource (1): same cargo cluster.
  • TestCheckoutStormCoalescesIntoOneBatch (1): timing; 27 git subprocesses vs ≤17; it also failed once on ubuntu 1.26 on this PR's first run, so it is not Windows-specific.

New test: TestPathContainsAcceptsNativeProbes (native probes via filepath.FromSlash; passes on macOS before and after, failed on Windows before).

Mitigation for no local Windows: go build ./... && go test ./... && gofmt -l . clean on macOS; GOOS=windows go build ./... and GOOS=windows go test -c for scanner, main, mcp, cmd, watch, config, render, topology compile clean. Both iterations were read from the informational Windows cell.

Refs #214

🤖 Generated with Claude Code

https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T

…lash keys; tests spell keys with slashes

Residue of #214 after #218. Graph keys are forward-slashed on every OS,
but the JS and Dart workspace resolvers still derived package roots with
filepath.Dir and compared them to keys with the native separator, so on
Windows pathContains was false for every member file and workspace
imports resolved to nothing. Nested .gitignore matching handed
go-gitignore a native relative path, which never matched a slash
pattern, so ignored files were scanned and visible ones dropped. The
cargo-metadata fallback emitted native edges that no slash key matched.

cleanRepoPath, the CUE and Go fallback file keys, nested-repo paths and
cargo edges now go through keyPath; pathContains splits on "/".

Test side: 123 filepath.Join calls whose arguments were all string
literals were graph keys written the long way; they are slash literals
now. Fixture creation that joins a temp root is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T
Copilot AI balanced review requested due to automatic review settings October 11, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ctations as slash keys; skip the residue outside the resolvers by name

The Rust resolver kept every repo-relative path in native form through
filepath.Clean/Join/Dir and compared it to slash graph keys, so on
Windows crate-rooted, super:: and literal-path modules resolved to
nothing. Repo-relative paths now go through keyPath and path.Join; only
calls that touch the disk keep filepath. Same for the Go fallback's
nested-repo check.

Test side: filepath.FromSlash wrappers on expectations in scanner tests
were spelling graph keys in native form; removed. The literal rewrite
from the previous commit is reverted outside scanner, where native
paths are the right expectation (config path, topology repo path,
git exclude path).

Twenty-four tests outside the resolvers now skip on Windows with the
cause named and "tracked in #214", so the residue is explicit in the
informational cell instead of a wall of red.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T
JordanCoin added a commit that referenced this pull request Oct 11, 2026
…st asserts exact counts on a synthetic clock (#221)

TestCheckoutStormCoalescesIntoOneBatch fed 500 events in 200 ms against
250 ms wall-clock windows and asserted at most 17 git calls; on a loaded
CI runner the storm split into two bursts and 27 were observed (#219's
ubuntu 1.26 run). The detection logic moves out of the event loop into
burstDetector, whose observe, enqueue and take all take `now`, so the loop
passes time.Now() and tests pass a fixed clock. The exact counts (16
per-file, then one batch of 292 files; git, topology and refresh each
exactly once) are asserted on the synthetic clock, and the one test that
still goes through the real loop asserts only that batching happened,
with the bound derived from burstThreshold.

Refs #201


Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@JordanCoin
JordanCoin merged commit 30b89d7 into main Oct 11, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants