sdsrss/code-graph-mcp/windows-compat

Windows compatibility (code-graph-mcp)

Three defects reached a Windows user from one root cause: two spellings of the same path compared unequal. CI already ran windows-latest and caught none of them, because nothing asserted on path spellings at all. Closing that gap is what this skill is for.

The invariant everything hangs off

indexer::merkle::normalize_rel_path stores every indexed path as a `/`-separated relative path, rewriting separators only under #[cfg(windows)]. So any path that becomes a lookup key, a dedup key, or stdout must land in exactly that spelling.

Almost every defect below is one producer handing over a different spelling of the same file. The producer table and the full catalogue are in references/failure-modes.md — read it before changing path-handling code. It is short and every entry is a real defect with its shipped fix.

The doctrine: platform is a parameter, not a cfg!

This is the single rule that makes everything else testable.

// NO — the Windows branch only runs on Windows, so the Linux and macOS legs
// prove nothing, and a Windows-only CI failure is expensive to debug.
fn normalize(path: &str) -> String {
    if cfg!(windows) { path.replace('\\', "/") } else { path.into() }
}
 
// YES — one thin cfg! entry point, all logic in a function taking the platform.
fn normalize(path: &str) -> String {
    normalize_on(path, cfg!(windows))
}
fn normalize_on(path: &str, backslash_is_sep: bool) -> String { ... }

Then assert both values from any platform:

assert_eq!(normalize_on(r"src\a.rs", true), "src/a.rs");   // Windows behaviour
assert_eq!(normalize_on(r"src\a.rs", false), r"src\a.rs"); // `\` is a legal Unix filename char

Live examples: merkle.rs::normalize_rel_str_on (the crate's single separator-rewriting implementation — everything else delegates to it), cli.rs::normalize_path_display_on, cli.rs::relativize_path_on.

Skip dunce / path-slash here. They do the right thing but decide internally on cfg(windows), forfeiting exactly the property that makes the Linux leg useful.

Before trusting a green suite, classify the fix

Where the correct Unix behaviour is the identity transform, no assertion can go red on a Unix leg — reverting the fix changes nothing observable there. That is not a weak test, it is a Windows-observable fix. Handle it by asserting the relation (produced key == merkle::normalize_rel_path(same input)), so the windows-latest leg carries a real regression test, and by making sure the decision logic it delegates to is _on-tested, which every leg does run. D6 in the reference is the worked example.

Workflow

1. Scan

bash docs/skills/windows-compat/scripts/audit.sh src

Advisory, not a gate: exit 1 means "read these", not "these are bugs". The patterns are deliberately tuned so a hit is worth a human read — worktree_main_root is a correct site it flags, and it is worth reading as the reference for the length-preserving pattern.

2. For each hit, answer three questions

1. Where does this path come from, and where does it go? Check the producer

table in the reference. A path from git (always /) compared against one from rg (native separators) is defect D1.

1. Does it become an index lookup key or reach stdout? Then it must match

merkle::normalize_rel_path's spelling exactly. Rather than hand-tracing callers, use this repo's own index — code-graph-mcp impact <fn> answers "what does this reach" in one call, and it sees the whole repo rather than the files you happen to have open.

1. Is it an argv operand that can grow with repo size? Then bound it in

bytes and batch. A count cap is not a length bound (D3).

3. Fix behind a platform parameter, then test both values

New path logic goes in a _on(…, windows: bool) function per the doctrine above. A regression test that only runs on Windows is worth far less than the same assertion running on every leg.

4. Prove the test has teeth

A passing test proves nothing until its failure has been observed. Break the fix deliberately, watch the new test go red, restore, watch it pass:

# e.g. make normalize_path_display_on ignore its flag, then:
cargo test --lib normalize_path_display_leaves_unix_backslash

Mutate the whole condition, not one operand — if false && a || b still matches via b, because && binds tighter than ||. A mutation that leaves the test green has told you nothing about the test.

This step was decisive twice on the shipped fixes: the first affected test passed even with the new classification legs disabled (an unrelated leg matched it), and once mutated correctly, the output reproduced the bug report symptom-for-symptom.

When changing a path predicate that has mirrors

Enumerate the mirrors before you edit. Do not trust a count — including this one. domain::is_test_path is reimplemented across several sites in two languages and a SQL dialect; domain.rs carries a "Five sites must agree" note, and the inventory doc it points at (feedback_test_classifier_dual_sources.md) no longer exists, so the code comments are now the only record and they may themselves be stale.

Find them yourself, every time:

# Search the WHOLE repo. Scoping this to src/ is how you miss mirrors — the
# Python ports under scripts/ and the JS one under claude-plugin/ are real
# must-agree sites, and an earlier draft of this command scoped away the former.
rg -l 'is_test_path|isTestPath|is_test_node_sql|TEST_SOURCE_FILTER'

Ports often announce themselves in a docstring ("""Port of src/domain.rs..."""), so also try rg -l 'Port of src/domain' and the equivalent for whatever predicate you are changing.

Then separate consumers (call the predicate — nothing to do) from reimplementations (their own copy of the rules — these are the mirrors). code-graph-mcp callgraph is_test_path settles which is which faster than reading each file.

Then decide per site, because some divergence is deliberate. At the time of writing: the Rust and SQL legs of is_test_node_sql must agree, but PROD_SOURCE_FILTER_AND / TEST_SOURCE_FILTER_OR are documented as intentionally narrower, and the closure in indexer::pipeline::resolve::refine_ambiguous_targets is deliberately broader. Blindly unifying all of them is its own bug. Read each site's comment first; if it claims to be intentionally different, leave it and say so in your summary.

The one that is easiest to miss sits outside src/ entirely: claude-plugin/scripts/pr-impact-comment.js::isTestPath, whose test is named isTestPath mirrors domain::is_test_path patterns — an explicit parity contract that a Rust-only change silently breaks.

For the legs that must agree, generate both from the shared constants (PASCAL_TEST_EXTS / PASCAL_TEST_STEMS / INFIX_TEST_EXTS) rather than transcribing, and extend test_is_test_node_sql_matches_rust, which runs the emitted SQL against in-memory SQLite to prove agreement. Watch GLOB (case-sensitive, _ literal) vs LIKE (case-insensitive, _ wildcard) and pick per leg to match the Rust semantics. Add near-miss negatives, not just positives — src/latest.cs, src/protest/api.cs and src/testing/api.cs must stay classified as production code.

Verifying without a Windows machine

• Drive both branches of every _on function from unit tests (works anywhere).
• Drive size/limit logic through a test seam instead of materializing the real

limit: CODE_GRAPH_RG_ARGV_BUDGET makes the 32 KB batching path reachable with a 40-byte budget (tests/cli_e2e.rs::test_cli_grep_supplement_batches_across_argv_budget).

• Leave genuinely OS-gated assertions to the windows-latest leg and keep them

few — anything expressible as pure string logic belongs in an _on test.

Done when

• Every audit hit is either fixed or consciously accepted as a correct site.
• New path logic takes the platform as a parameter, with both values asserted.
• At least one mutation was observed going red.
• Predicates with a SQL mirror were regenerated from shared constants, with

near-miss negatives added.