GeezerVison

BugBot review

Generated by Claude Code

[agent] Two red checks on 3033eee, neither in code this PR touches:

• e2e (ubuntu-latest, e2e_redirect_maven_build, 3.8.9) died in the fixture warm-up against Maven Central, before socket-patch ran: Could not find artifact org.apache.maven.plugins:maven-dependency-plugin:jar:3.6.1 in central. The diff doesn't touch Maven, and Maven discovery doesn't use the probe runner. I'll re-run it once when the CI run completes.
• Pipenv matrix (macos-latest, …): one leg (2018.11.26 direct hosted, rollbackRestoresLockBytes) failed after PyPI request transport failed retries. The same leg passed on b29d415, and the only change since is behind cfg!(windows) plus a clippy lint, so macOS runs identical code. That's the transport-blip class Fix Pipenv matrix flake on 503/DNS transport blips #419 targets. I've re-run the failed job once.

If either fails again on re-run, I'll treat it as real and root-cause it.

Generated by Claude Code

BugBot review

Generated by Claude Code

BugBot review

Generated by Claude Code

[burn-down agent] Ready for review on 059b07b: mergeable, 0 commits behind main.

• CI: 405/405 check runs green on 059b07b (6 skipped by workflow conditions, none failing).
• Bugbot: reviewed 059b07b, no new issues (earlier runs on 3033eee/a5ef5a0 are stale); 0 unresolved review threads.
• Reviewer focus: the PATHEXT-aware tool resolution in crates/socket-patch-core/src/utils/process.rs and how the Composer/npm global probes now spawn resolved tools instead of bare names from the project cwd; new e2e coverage in tests/global_probe_spawn_e2e.rs.

Note: Slack announcement could not be sent this run (no Slack send tool available in the agent session); next run will retry.

Generated by Claude Code

bugbot run

Generated by Claude Code

bugbot run

Generated by Claude Code

e2e (ubuntu-latest, mode_migration_vlt, 1.0.0-rc.14, …) failed on 465db05 before any test ran, and the cause isn't in this PR. Downloading the e2e-bin-ubuntu-latest artifact (146 MB) took about 1.5 minutes and left target/e2e-bin/ empty, so the next step failed with cp: cannot stat 'target/e2e-bin/socket-patch'. This PR doesn't touch the workflow or the artifact upload. I'll re-run the job once when the rest of CI run 36870777682 has finished; GitHub refuses a re-run while the run is still going.

Generated by Claude Code

bugbot run

Generated by Claude Code

Burn-down agent check: ready for review at d60a204e.

• CI: 97/97 non-skipped checks green on d60a204 (3 skipped)
• Bugbot reviewed d60a204 and found no new issues. No review threads are open.
• Reviewers: the last push was a merge of main to resolve a conflict; the fix itself didn't change since it was last reviewed.

Generated by Claude Code

bugbot run

Generated by Claude Code

Burn-down agent: ready for review again at 0958e11 (0958e11aa1be45852ede833dae021232dbfdf521).

• Merged main to clear a conflict. Only CHANGELOG.md conflicted, and both entries are kept. No code changes.
• CI: all non-skipped checks green on 0958e11.
• Bugbot: reviewed 0958e11, no new findings; all review threads resolved.

Generated by Claude Code

Reviewed 0958e11aa1be. Changes needed before merge.

[P1] Preserve absolute-PATH filtering in the Windows fallback — crates/socket-patch-core/src/utils/process.rs:207.

When resolve_tool finds no safe executable, None if cfg!(windows) => PathBuf::from(bin) hands the bare name back to Command. Rust 1.93.1's Windows search implementation skips empty PATH entries, but still searches relative entries such as .. It resolves the executable in the parent process before applying the child's current_dir.

Consequently, on Windows with . on PATH and no installed Yarn on an absolute PATH entry, a yarn.exe planted in the scanned project is still executed by the global probe. The neutral child cwd does not prevent it. Please preserve the filtered lookup when supporting App Execution Aliases—for example, probe only absolute candidates—and add a Windows case with a planted executable and no safe matching tool.

Validation: cargo test --locked -p socket-patch-core --test global_probe_spawn_e2e: 6 passed on macOS. The Windows finding is source-traced against the pinned Rust implementation and independently cross-checked; I did not execute the Windows reproduction locally.

Thanks, the [P1] is right. The old comment saying std never searches the cwd on Windows was wrong, because std still walks relative PATH entries such as .. Fixed in 96097f3, and main is merged in at dce7d1a:

• The Windows fallback no longer hands the bare name to std. When resolve_tool finds nothing, the new resolve_app_alias_with looks for <name>.exe on absolute PATH entries only. It uses symlink_metadata, so App Execution Alias reparse points are still found, but nothing reachable only through ., an empty entry or a relative directory can be chosen. If there's no match, the probe returns None.
• New Windows e2e test, global_probe_never_runs_an_executable_planted_in_the_project. It copies cmd.exe into the project as npm.exe, sets PATH to . only, chdirs into the project, and asserts get_npm_global_prefix() is an error. With the old fallback, std would find .\npm.exe, run it, and return the cmd banner as the "prefix".
• New unit test, resolve_app_alias_skips_relative_entries, which runs on every OS.

I couldn't run or type-check the Windows test here: there's no Windows host, and cross-compiling ring needs MinGW. So test (windows-latest) on this push is its first real run, and I'll watch it. Clippy and the Linux suites pass. I've also corrected the PR description.

Generated by Claude Code

test (ubuntu-latest) failed on dce7d1a in one library test: crawlers::npm_crawler::oracle::tests::walk_without_a_pool_matches_the_sequential_oracle (no pool, seed 12: find_by_purls differs under …/proj/node_modules). The two sides disagree on how far a .pnpm symlink cycle is followed (.pnpm/@[email protected]+u+r@abc1/node_modules/.pnpm/…).

I don't think this PR caused it:

• This branch only changes npm_crawler.rs to swap SystemCommandRunner for GlobalProbeRunner in the four get_*_global_prefix helpers. The crawl, find_by_purls and the store walk the oracle compares are untouched.
• dce7d1a is the first time this branch includes the npm crawler changes from main (Fix npm crawler missing relocated dependency stores (#359, #362) #365 and later).
• The test passed 6 times out of 6 locally on this exact commit (cargo test -p socket-patch-core --lib crawlers::npm_crawler::oracle).
• Every recent main CI run was cancelled by the next push, so main hasn't shown whether it passes there.

No fix exists for it yet. I'll re-run the failed job once this CI run finishes (GitHub refuses a re-run while the run is still going). If it fails again, I'll treat it as a real nondeterminism in the crawler-vs-oracle walk and look into it.

Generated by Claude Code

Follow-up reviewed dce7d1af663ab9e89f2c1b35fffc7916fe81edc0. The earlier P1 is fixed; code review is clear, pending CI clearance.

The Windows alias fallback now considers only absolute PATH entries and passes the selected absolute .exe path to Command. It no longer restores bare-name lookup, so Rust cannot select a project-planted executable through . or another relative PATH entry. The new Windows regression covers the original trigger. No new actionable issues found in this delta.

Validation: cargo test --locked -p socket-patch-core --lib utils::process 15 passed; cargo test --locked -p socket-patch-core --test global_probe_spawn_e2e 6 passed on macOS. I also checked the absolute-path branch of the pinned Rust Windows implementation. The Windows-only executable regression and actual Store alias launch were not run locally.

The final main-merge commit leaves the process implementation and probe tests identical to tested 96097f38; no additional PR-specific change was introduced.
The final head currently has a failing test (ubuntu-latest) check: walk_without_a_pool_matches_the_sequential_oracle differs on encoded scoped PURLs for seed 12. This is outside the process-execution change reviewed here, but the check still needs to pass before merging.

bugbot run

Generated by Claude Code

[burn-down agent] Re-verified Ready for review at dce7d1af663ab9e89f2c1b35fffc7916fe81edc0.

• CI: 400/400 non-skipped checks green on the head (6 skipped). The earlier test (ubuntu-latest) npm-crawler oracle failure passed on re-run. That test is outside this PR's diff.
• Bugbot: reviewed dce7d1a, no new issues. The earlier composer APPDATA isolation finding was fixed in 465db05 and resolved.
• Mergeable: yes, no conflicts with current main. It is only waiting on approval.
• For reviewers: the Windows alias fallback now resolves only absolute PATH entries to an absolute .exe. The Windows-only regression test runs in CI only.

Slack announcement not sent: the Slack connector in this session has no send-message tool.

Generated by Claude Code

Re-reviewed dce7d1af663ab9e89f2c1b35fffc7916fe81edc0. The earlier Ubuntu CI blocker has cleared; no additional code change is needed.

The Ubuntu test job now passes on this same head, as do the Windows and macOS test jobs. The Windows absolute-PATH security fix remains unchanged, and the branch merges cleanly with current main (73b17db5). No new actionable defect found.

Fresh local validation: 4 oracle tests, 15 process tests, and 6 global-probe integration tests pass on macOS, including the oracle test that previously failed in Ubuntu CI. The old failure cleared on a same-SHA CI rerun; the local replay also passed.