[Mikola Lysenko (mikolalysenko)]

Problem

CI is healthy this run. The merge queue is empty, the last 30 merge_group runs passed, main's push and nightly runs are green, and nothing is queued. So this run takes a priority-4 robustness target from #1919: unit tests that pass only because CI's environment is clean.

detect_npm_pkg_manager reads YARN_NODE_LINKER, YARN_RC_FILENAME and ~/.yarnrc.yml whenever a PnP loader file exists. I reproduced the failures on origin/main (c09cb0e):

env	cargo test -p socket-patch-core --lib crawlers::
clean	798 passed
HOME with .yarnrc.yml → nodeLinker: node-modules	13 failed (12 pkg_managers, 1 pnpm_layout)
YARN_NODE_LINKER=node-modules	15 failed (the same 13, plus 2 npm_crawler)

A runner image or a contributor with a global yarn nodeLinker gets red tests on a clean checkout.

Root cause

The tests call the public detector, and it builds YarnEnv::for_project from the process environment and the home folder.

Fix

• Split out detect_npm_pkg_manager_with(project_root, yarn_linker) and a private live_pnp_marker_in. The nodeLinker lookup is now passed in as a closure. In production the public functions pass the same yarn_node_linker(scope, root) as before, so behaviour doesn't change.
• YarnPnpLoader::detect gets the same split (detect_with).
• The test modules use #[cfg(test)] hermetic wrappers over YarnEnv::repository_only(). Project-local .yarnrc.yml files are still read.
• I injected the env instead of using EnvGuard plus #[serial] on purpose. Changing process env while other tests run in parallel is itself a source of flakes.
• New test public_detectors_match_the_hermetic_ones_without_a_loader keeps the public entry points covered by unit tests.

Proof

After the change:

• clean: 799 passed
• hostile HOME/.yarnrc.yml: 799 passed (was 13 failed)
• YARN_NODE_LINKER=node-modules: 796 passed, 2 failed. Both are npm_crawler tests that go through the full crawler; they are listed below.
• cargo clippy --locked -p socket-patch-core --all-features -- -D warnings (CI's clippy command) is clean, and rustfmt --check passes on the touched files.

No tests were removed or weakened. Every changed test asserts the same thing on the same fixtures.

Left for follow-ups (#1919)

• npm_crawler::global_scope_reads_a_yarn_store_configured_above_the_repository and test_yarn4_pnpm_linker_relocated_store_transitive_packages_are_found still fail under YARN_NODE_LINKER. They go through NpmCrawler, and fixing them needs the env threaded through the crawler.
• The 5 python_crawler tests that fail under an activated VIRTUAL_ENV.

Refs #1919

/code-review high (burn-down agent)

Fixed in 2abfc1e:

• The hermetic helper still walked every ancestor's .yarnrc.yml. Outside a git checkout that walk has no bound, so a nodeLinker set above TMPDIR still failed 15 pkg_managers tests. The helper now reads only the project's own rc file, and hermetic_detector_ignores_an_rc_file_above_the_project covers it. With the hostile setup, 3 tests still fail. They are older tests that call yarn_node_linker_in directly, and this PR doesn't touch them.
• The public-vs-hermetic test now also asserts that the linker lookup is never called when there is no loader.

Not fixed here, deliberately:

• Other modules' tests (npm_crawler, shared_store, lock_inventory, npm_flavor, refusals, yarn_berry_lock) still reach the host yarn env through yarn_node_linker. That is the Crawler unit tests read the developer's real HOME, YARN_NODE_LINKER and VIRTUAL_ENV: 12 pkg_managers/pnpm_layout tests fail under a user nodeLinker setting #1919 follow-up. The better long-term fix is one test-time override in YarnEnv, not more *_hermetic twins.
• YarnPnpLoader::detect hard-codes ReadScope::Workspace (vex.rs). That predates this PR and is unchanged.
• The hermetic helper ignores .env.yarn injection. No test that uses it has an env file.

---

Generated by Claude Code

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

PR Summary

Low Risk
Refactor preserves production wiring; changes are mainly test isolation and dependency injection with no new runtime paths.

Overview
Unit tests no longer depend on the host’s Yarn setup (YARN_NODE_LINKER, ~/.yarnrc.yml, ancestor rc files). Production behavior is unchanged: detect_npm_pkg_manager and live_pnp_marker delegate to internal *_with / live_pnp_marker_in helpers that take an injectable nodeLinker closure (still wired to yarn_node_linker at the public API).

Tests use detect_npm_pkg_manager_hermetic and YarnPnpLoader::detect_hermetic, which resolve linker only from the project’s own .yarnrc.yml via YarnEnv::repository_only(). Existing pkg_managers and pnpm_layout assertions are switched to these entry points.

New coverage confirms public vs hermetic agreement when no PnP loader exists (linker lookup not invoked), and that hermetic detection ignores rc files above the temp project root.

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

[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 2abfc1e. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 2abfc1e.

• CI: 50/50 checks done, none failed (31 success, 19 skipped by path filters). No main-wide failures.
• Bugbot: reviewed 2abfc1e and found nothing.
• /code-review high: one real gap, fixed in 2abfc1e. The hermetic helper still walked ancestor .yarnrc.yml files, and outside a git checkout that walk has no bound. It now reads only the project's own rc file, and there is a regression test for it. The other findings are scope follow-ups for Crawler unit tests read the developer's real HOME, YARN_NODE_LINKER and VIRTUAL_ENV: 12 pkg_managers/pnpm_layout tests fail under a user nodeLinker setting #1919 or predate this PR. They are listed in the PR description.
• Worth a reviewer's look: whether #[cfg(test)] *_hermetic twins are the right shape, or whether a single test-time override in YarnEnv would be better as the follow-up.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Unit tests for the npm package-manager detector no longer pick up the host's yarn setup (YARN_NODE_LINKER, ~/.yarnrc.yml, rc files above the project). The public detectors now hand the nodeLinker lookup to internal *_with helpers. Production still passes the same yarn_node_linker(scope, root), and tests pass a #[cfg(test)] lookup that reads only the project's own .yarnrc.yml. Test-only otherwise. Every changed test asserts the same thing on the same fixtures.

Risk: low. The production paths are a straight delegation with the same closure, and the new code is #[cfg(test)] only. The one behavioural nuance is that YarnPnpLoader::detect now computes the manager before calling detect_with, which is the same order as before.

Look here

• pkg_managers.rs:98: public detect_npm_pkg_manager delegates to _with
• pkg_managers.rs:541: live_pnp_marker_in, where the linker is injected
• pkg_managers.rs:632: YarnPnpLoader::detect_with
• pkg_managers.rs:685: the hermetic test helper (reads only <project>/.yarnrc.yml)
• pkg_managers.rs:711: new tests covering public vs hermetic behaviour and an rc file above the project

Verified

• Read the full diff (2 files, +168/−60). No debug code or scope creep, and CHANGELOG.md is untouched.
• cargo test -p socket-patch-core --lib crawlers::pkg_managers passes: 51/51 clean, 51/51 with HOME containing .yarnrc.yml → nodeLinker: node-modules, and 51/51 with YARN_NODE_LINKER=node-modules. crawlers::pnpm_layout passes 4/4 under the hostile HOME.
• CI on 2abfc1e: ci-ok and clippy green (burn-down reports 50/50 done, 0 failed). Bugbot reviewed 2abfc1e and found nothing. No open review threads.

Changes I made. None.

Open questions (not blocking): should the follow-up for #1919 replace the *_hermetic twins with a single test-time override in YarnEnv? The PR description already proposes that, and npm_crawler and python_crawler are left for it.

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

---

Generated by Claude Code