[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #1979 (T64, slice 1). Living document: Part 5, tests.

Why

crates/socket-patch-cli/tests carried ~38 hand-written recursive directory copiers and no shared one. The four hermetic scan suites also each defined a byte-identical mock public proxy. This PR adds one shared copy of each and deletes the duplicates in the files no open PR holds.

Changes (test-only)

• New tests/common/fs_tree.rs holds copy_tree, reached as common::fs_tree::copy_tree. Modules whose includers don't load common pull it in with #[path = "../common/fs_tree.rs"] mod fs_tree;, the same pattern as oracle.rs.
• 28 copies deleted:
  • redirect suites: e2e_redirect_{bun,cargo,pnpm,yarn_berry}_build, e2e_redirect_cargo_shapes;
  • vendor suites: e2e_vendor_{bun,cargo,gem,pnpm,pypi,yarn_berry,yarn_classic}_build, e2e_vendor_yarn_classic_dev_flow;
  • yarn4 suites: e2e_yarn4_{pnpm_linker,workspaces}_build;
  • mode migration: mode_migration_{bun,cargo,npm};
  • others: e2e_nuget_dotnet_build, vendor_jvm_cli, in_process_redirect_pdm, e2e_vex_build/poetry;
  • shared modules: composer_e2e_common, maven_build_common, vlt_vendor_common, yarn_berry_common, prebuilt_common, vex_e2e_common/uv.
• Callers of composer_e2e_common::copy_dir_recursive updated, including one line in docker_e2e_vendor_composer.rs. That file is held by Share one docker test harness (#1968, first slice) #1972, and git merge-tree shows no conflict.
• Missing-source handling. The two copies that silently skipped a missing source (uv.rs and pdm) are replaced by an is_dir() guard at each of their 5 call sites. The shared helper stays strict, so a missing fixture still panics.
• New tests/common/scan_proxy.rs holds start_proxy and assert_proxy_served_scans, moved verbatim from e2e_{cargo,composer,maven,nuget}. Those 4 copies are deleted.
• Diff: +218 / −589 across 39 files, all of them test files.

Not in this slice

These copies are left because open PRs hold their files, or because they are real variants:

• copies in held files:
  • e2e_vendored_production (Fix Go patch delivery, audits, and reinstalls #1698, Scrub color env in hermetic test children #1964);
  • e2e_vendor_golang_build (Fix Go patch delivery, audits, and reinstalls #1698);
  • e2e_bun_lockb (Fix bun.lockb nightly e2e reaching the live blob API #1905);
  • e2e_redirect_gem_build (Fix hosted gem block mixing CRLF Gemfile endings (#1831) #1841);
  • e2e_redirect_yarn_classic_build (Fix yarn classic copy identity read two ways (#1951) #1984);
  • e2e_redirect_npm_build, e2e_vendor_npm_build and the npm_e2e_common/manifestless copy (Fix CRLF bin e2e failing under npm 11 on macOS #1896, Fix npm overrides to a URL treated as registry (#1965) #1966, Fix npm v2 revert half-reverting the lock (#1937) #1938);
  • docker_e2e_vendor_{maven,nuget} (Share one docker test harness (#1968, first slice) #1972).
• real variants: the skip-list copy_tree in vex_pypi_real_common and vex_pipenv_pip_real, vex_pipenv_pip_steps (which skips dangling links), golang_e2e_matrix (which makes copies writable), e2e_gradle_agent_build (walkdir), and e2e_embedded_vex (one level only).
• hosted_pin_count, which overlaps e2e_hosted_production and e2e_vendored_production are a near-fork: 11 identical helpers, a copied PURL/UUID catalog and hand-rolled VEX checkers beside vex_e2e_common #1983.

Evidence

• CI is green on all 82 checks, and Bugbot found no issues.
• cargo check -p socket-patch-cli --tests --all-features is clean. The 3 warnings left (binary, find_event unused) are in files this PR doesn't touch.
• cargo clippy --workspace --all-features -- -D warnings is clean.
• 16 suites were run and pass (444 tests): e2e_{cargo,composer,maven,nuget}, e2e_redirect_cargo_build, e2e_redirect_cargo_shapes, e2e_redirect_yarn_berry_build, e2e_vendor_cargo_build, e2e_vendor_yarn_berry_build, e2e_yarn4_workspaces_build, in_process_redirect_pdm, mode_migration_{bun,cargo,npm} and vendor_jvm_cli.
• Red→green: when the shared copy_tree was changed to skip files, mode_migration_cargo failed 5 tests.
• Pre-existing gap: when the proxy mount path was changed to the wrong one, e2e_cargo stayed green. assert_proxy_served_scans counts every request, 404s included. The code moved verbatim, so this PR doesn't change that; I'll note it for the audit.

/code-review high

It raised 7 findings.

• Fixed:
  • The shared copy followed directory symlinks (path().is_dir()), so a link cycle could recurse forever. It now uses file_type(), as 27 of the 28 copies did.
  • The extra stat per entry goes away with that change.
  • The module doc claimed a re-export that doesn't exist.
• Not fixed:
  • The consolidation is incomplete: those files are held by the open PRs listed above.
  • No shared_helper_copies guard for copy_tree yet: it would fail on the held copies. Add it in the slice that removes them.
  • duplicate_mod on common/mod.rs's new submodules only fires under clippy --tests, which CI doesn't run. It comes from vlt_e2e_common re-including all of common, which already duplicates every common submodule.
  • No copy_tree_if_present helper: keeping the helper strict keeps missing fixtures loud. The 5 guards are explicit where a hosted flow commits no .socket/.

Behavior change

None. No assertions change. Only yarn_berry_common's copy followed directory symlinks, and its two uses (.socket, the yarn zip cache) contain none.

Risk

Low: test-only. No cfg(windows)/cfg(unix) logic changed. e2e_vex_build's include is cfg(unix), the same as its only user, poetry.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeP5DtGe4XN7J3Vprc7

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

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 ef8e862. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review.

• Head: ef8e862bf6baa77229d630a795f9b24b2cdbd166, no conflicts with main (cbbbdb9).
• CI: all 82 check runs on the head are success/skipped (ci-ok green). No main-wide failures.
• Bugbot: reviewed ef8e862, no findings.
• Reviewer focus: test-only change. The shared tests/common/fs_tree.rs::copy_tree is strict (a missing source panics); the 5 call sites that used to skip a missing source now have explicit is_dir() guards.
• Slack announcement not sent this run (Slack send unavailable); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Test-only refactor. It replaces 28 hand-written recursive directory copiers in crates/socket-patch-cli/tests with one shared common::fs_tree::copy_tree. It also moves the byte-identical mock public proxy of the 4 hermetic scan suites into common::scan_proxy. +218/−589 across 39 test files, and no production code changes.

Risk: low. Only test code changes. The one behaviour change is deliberate: yarn_berry_common::copy_dir used to follow directory symlinks and now panics on them, like the other 27 copiers. It only copies .socket and the yarn zip cache, which hold no directory links. The two copiers that skipped a missing source silently (uv.rs, pdm) now have an is_dir() guard at each call site.

Look here:

• common/fs_tree.rs:14: the shared copier. It uses DirEntry::file_type(), which doesn't follow links, so a file symlink is copied as the file and a directory symlink panics.
• common/scan_proxy.rs:9: the moved proxy and hermeticity guard.
• yarn_berry_common/mod.rs:71: the only copier whose symlink semantics changed.
• common/mod.rs:54: the new submodules.

Verified:

• I read the full diff. None of the removed copiers recreated symlinks or skipped paths: the removed code has no read_link, is_symlink or skip-list.
• CHANGELOG.md is untouched.
• ci-ok and clippy are green on the head, and the PR reports 82/82 checks green. Bugbot found no issues. No review threads are unresolved.
• I ran cargo clippy -p socket-patch-cli --all-targets -- -D warnings locally. It fails on the same 93 test targets as main (pre-existing needless_borrow/duplicate_mod lints that CI doesn't enforce on tests). The branch adds 2 more duplicate_mod hits in e2e_{hosted,vendored}_production, which load common twice. The PR introduces no compile errors.

Changes I made: none.

Open questions: none.

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

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed ef8e862bf6baa77229d630a795f9b24b2cdbd166. Checked shared fixture copy semantics, optional-source guards, imports and proxy-helper consolidation across migrated suites. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.