[Mikola Lysenko (mikolalysenko)]

Fixes #2023.

Problem

crawlers::nuget_crawler::tests::solution_roots_and_unrestored_projects_keep_the_walk fails every time on a machine with any package in ~/.nuget/packages. It passes in CI only because the runners' NuGet cache is empty. Issue #2023 has the repro.

While reproducing it I found a second test with the same dependence, an_uncovered_sub_project_keeps_the_walk. That one is also order-dependent: neither test was #[serial], so both ran alongside the serial tests in the same module that point NUGET_PACKAGES and HOME at their own warm folders. With one host package planted (~/.nuget/packages/zz.hostpkg/9.9.9), 30 loops of cargo test -p socket-patch-core --lib nuget_crawler on the partial fix gave:

• 15 failures of solution_roots_… (left had an extra pkg:nuget/zz.hostpkg@9.9.9)
• 8 failures of an_uncovered_…, which passed only in runs where a concurrent serial test happened to have NUGET_PACKAGES set

Root cause

An unscoped project-mode crawl (a solution root, or a project with no parsed restore) always adds the global packages folder NuGet would restore into as search root 3 (crawl_roots, via global_packages_folder → nuget_home()). The tests built their packages in a temp shared-cache but left that folder pointing at the process env and home, which belong to the host and to whichever serial test is running at the time.

Fix (test-only)

• New `crawl_local(cwd)` test helper. For the duration of the crawl it sets `NUGET_PACKAGES`, under `crate::test_env::EnvGuard`, to a tempdir holding exactly one package, `global.only@1.0.0`. All six project-mode crawl tests use it and are now `#[serial]`. The two unscoped-walk tests must list `global.only`, which shows root 3 is walked. The four scoped tests must leave it out, which shows scoping excludes the global folder; before this PR they passed only because `seen` dedups by purl. Mutation check: with root 3 dropped from `crawl_roots`, 3 tests fail.
• `test_nuget_home_env_var` and `test_nuget_home_empty_env_var_falls_back_to_default` move from bare `set_var`/`remove_var` onto `EnvGuard`, so a failed assertion can't leak `NUGET_PACKAGES` into later tests.

The crawler itself doesn't change. The unscoped walk the tests pin (#427 / #1183) stays the same.

Not fixed here, left as follow-ups from `/code-review high`:

• `NugetEnv` still hand-rolls a restore-on-drop that duplicates `EnvGuard`.
• `tests/crawler_nuget_e2e.rs` still saves and restores `NUGET_PACKAGES` by hand around an `unwrap()`.
• `crawl_local` doesn't isolate the user-level NuGet.Config. No current walk test depends on it.

Proof

setup	main	this PR
host ~/.nuget/packages holds zz.hostpkg/9.9.9 and newtonsoft.json/13.0.3	solution_roots_… FAILED every run, an_uncovered_… intermittent	100/100 module runs green (host also holding `serilog/3.1.1`)
no ~/.nuget (CI-like)	150/150 green	150/150 + 50/50 green

So in CI this is latent: no failure reproduced in 150 CI-like runs. The fix makes the tests hermetic for dev machines and removes the cross-test env race. cargo clippy -p socket-patch-core --all-features -D warnings (CI's command) is clean, and rustfmt shows no diff on the touched file. With --all-targets, my local clippy reports 39 lints that main has too; none are in this file.

Tests moved or removed

None. Every test still runs where it ran before (test jobs, PR and merge queue).

https://claude.ai/code/session_011f9aeRytjzpRh8AHn2i5Qh

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit d089d69. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: d089d69f
• CI: 50/50 check runs green (31 success, 19 skipped by path filters); mergeable, no CHANGELOG.md change.
• Bugbot: reviewed d089d69f, no findings; no unresolved review threads.
• Reviewer note: test-only change to crates/socket-patch-core/src/crawlers/nuget_crawler.rs (hermetic NUGET_PACKAGES + #[serial] for project-mode crawl tests). The PR body lists three follow-ups left out of scope.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A test-only fix for #2023. The project-mode NuGet crawl tests now point NUGET_PACKAGES (search root 3, the global packages folder) at a tempdir holding one known package, global.only@1.0.0, and run #[serial]. So they no longer read the developer's ~/.nuget/packages or race other tests that change NUGET_PACKAGES/HOME. The production crawler is unchanged.

Risk: low. Only the #[cfg(test)] module of one file changes. #[serial] on six more tests adds well under a second to the run.

Look here

• crawl_local helper: seeds the fake global folder and sets the env through EnvGuard
• global_packages_folder: NUGET_PACKAGES takes precedence over NuGet.Config, so a user-level config can't get around the helper
• unscoped walk tests: they now expect GLOBAL_ONLY, which proves root 3 is walked
• scoped tests and project_local_packages_folder_is_still_walked: they must leave GLOBAL_ONLY out, which proves scoping excludes root 3
• nuget_home env tests: moved onto EnvGuard, so a failed assert can't leak the var

Verified

• I planted a host package at ~/.nuget/packages/zz.hostpkg/9.9.9. With it, cargo test -p socket-patch-core --lib nuget_crawler on this head passed 56/56 three times out of three. On main's version of the file, the same run fails both solution_roots_and_unrestored_projects_keep_the_walk and an_uncovered_sub_project_keeps_the_walk, so the tests do catch the bug.
• CI: ci-ok and clippy are green on d089d69; the burn-down counted 50/50 check runs. Bugbot found nothing on d089d69, and no review threads are unresolved.
• Mergeable. CHANGELOG.md is untouched. There's no scope creep, and the three follow-ups listed in the body are reasonable to leave out.

Changes I made: none.

Open questions: none.

Auto-merge is armed, so approving sends this PR straight to the merge queue.

---

Generated by Claude Code