Skip to content

Commit 76434fd

Browse files
committed
refactor: T64 slice 1 in #1988
Assisted-by: Claude Code:claude-opus-5-5
1 parent c392641 commit 76434fd

3 files changed

Lines changed: 44 additions & 9 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
[agent] 2026-11-02: architecture refactor (structural)
2+
3+
`main` @ `cbbbdb9`, unchanged since the last run. The previous entry was a sweep (#1985), so this run did structural work. None of my 27 open PRs has merged or closed, and the discussion has no maintainer steering.
4+
5+
I chose **T64 [#1979](https://github.com/SocketDev/socket-patch/issues/1979)**. It scores about 11: D 4 (28 copiers plus 4 proxy copies, about −370 net), C 2 and risk L, because it is test-only. T68 #1983 was skipped because #1698, #1946 and #1964 hold both of its files. T65 #1980 was skipped because #1698 holds all of its Go suites. I checked by diffing all 72 open heads locally.
6+
7+
**PR:** [#1988](https://github.com/SocketDev/socket-patch/pull/1988), ready for review, with Bugbot triggered. CI is green on all 82 checks, and Bugbot found no issues.
8+
9+
### What changed (test-only, +218 / −589, 39 files)
10+
- New `tests/common/fs_tree.rs` with one `copy_tree`. Modules whose includers don't load `common` pull it in by `#[path]`, as with `oracle.rs`.
11+
- 28 copiers deleted: 20 suites and 8 shared modules, among them `prebuilt_common`, `yarn_berry_common`, `composer_e2e_common` and `vex_e2e_common/uv`.
12+
- The two copies that skipped a missing source became an `is_dir()` guard at their 5 call sites.
13+
- New `tests/common/scan_proxy.rs`: the byte-identical `start_proxy`/`assert_proxy_served_scans` from `e2e_{cargo,composer,maven,nuget}` moved there.
14+
- One caller line in `docker_e2e_vendor_composer.rs` (held by #1972) changed; `git merge-tree` shows it merges cleanly.
15+
16+
### Evidence
17+
- `cargo check --tests --all-features` and workspace clippy are clean.
18+
- 16 suites pass (444 tests).
19+
- When `copy_tree` was made to skip files, `mode_migration_cargo` turned red with 5 failures.
20+
- `/code-review high` raised 7 findings. 3 are fixed: the shared copy followed directory symlinks, which risked unbounded recursion on a link cycle, so it is back to `file_type()`; the extra stat went with that; and a stale doc claim. The PR gives a reason for each of the other 4.
21+
22+
### Left over
23+
- Copies in files held by 9 open PRs, plus the real variants (skip-list, chmod, `walkdir`, one-level).
24+
- `hosted_pin_count`, which goes with #1983.
25+
- A `shared_helper_copies` guard once the last copy is gone.
26+
27+
### Merges recorded
28+
None.
29+
30+
### Lessons
31+
- `assert_proxy_served_scans` counts every request, 404s included, so mounting the proxy on a wrong path stays green (executed). It should count only `POST /patch/batch` hits.
32+
- A `#[path]` inside a non-`mod.rs` submodule file such as `e2e_vex_build/poetry.rs` resolves under `poetry/`. Declare the module in `main.rs` and `use crate::` it.
33+
- When a shared helper replaces copies that disagree on symlinks, pick the majority's non-following `file_type()`. Following symlinks only looks more permissive; it can recurse forever on a link cycle.

‎register/20-audit-cli-tests.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ _Last updated 2026-11-02T09:51Z · main @ cbbbdb9 · owner: audit-cli-tests_
6666
| T61 | 3 | `prebuilt_common::prepare_command` filters `--offline` out of vendor/repair args; 49 `"--offline"` calls in 16 suites run online, and comments claim offline runs. | #1969 | filed #1969 |
6767
| T62 | 3 | 8 of 9 docker agent suites serve a zero `beforeHash` and `apply --force`, so only the force path is proven (gem's doc says so); port gem's real-hash probe. | #1970 | filed #1970 |
6868
| T63 | 3 | Docker vendor capstones: `assert_vex_attested_from_host` ×4 checks only `(vendored)` beside `assert_attested`; `render` ×5; stage-3 bash ×5; 4 manifest-less VEX mechanisms. | | to verify |
69-
| T64 | 3 | Test fs/scan plumbing copied: 38 recursive `copy_tree`/`copy_dir_recursive` (20 identical), `start_proxy`+`assert_proxy_served_scans` ×4, `hosted_pin_count` ×8 beside `envelope::hosted_pins`. | #1979 | filed #1979 |
69+
| T64 | 3 | Test fs/scan plumbing copied: 38 recursive `copy_tree`/`copy_dir_recursive` (20 identical), `start_proxy`+`assert_proxy_served_scans` ×4, `hosted_pin_count` ×8 beside `envelope::hosted_pins`. | #1979, #1988 | in PR #1988; slice 1 |
7070
| T65 | 3 | Go e2e harness copied across 5 suites beside `golang_e2e_matrix`: `go()` ×4 (drifted on `GOFLAGS`/`GOWORK`), file-proxy publish+zip ×5, `ChmodGuard` ×2 (md5-identical), `harvest_sums` ×2, grant/sumdb mocks ×3, gopatch literal ×6 files. | #1980 | filed #1980 |
7171
| T66 | 3 | `hermetic` has no Go `Extra`: children inherit `GOFLAGS`/`GOENV`/`GOWORK`/`GOPATH`/`GOMODCACHE`; Go suites `set_var` unrestored; hosted-production golang/deno tests can't fail. | #1981 | filed #1981 |
7272
| T67 | 3 | Gradle `--global-prefix` tests pass without the flag: `isolate_cli`/`socket()` always set `GRADLE_USER_HOME`, which silences `gradle_user_home_differs` (executed); no test asserts the warning fires. | #1982 | filed #1982 |

‎register/90-refactor.md‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
### Refactor routine
2-
_Last updated 2026-11-02T11:35Z · main @ cbbbdb9_
2+
_Last updated 2026-11-02T13:35Z · main @ cbbbdb9_
33

44
#### In flight
55
| PR | Kind | Scope | State |
@@ -31,15 +31,16 @@ _Last updated 2026-11-02T11:35Z · main @ cbbbdb9_
3131
| #1975 | sweep | core `api/{blob_fetcher,date,retry,types}.rs` | ready; green |
3232
| #1978 | structural | T56 #1956: 9 `cli_parse_*`/`cli_global_args` suites onto `hermetic::try_parse` + new `common/clap_parse.rs` (test-only, +418/−1,471) | ready; green |
3333
| #1985 | sweep | `scan/{rollout,rollout_args,withdrawn}.rs` (+ `Stage::may_admit` and 11 tests moved to core `rollout/stage.rs`) | ready; green |
34+
| #1988 | structural | T64 #1979 slice 1: `tests/common/{fs_tree,scan_proxy}.rs`; 28 copiers + 4 proxy copies deleted (test-only, +218/−589) | ready; green |
3435

3536
#### Merged
3637
_None._
3738

3839
#### Queue
39-
Structural, scored `3·B + 2·U + 2·D + 2·C − risk`. Nothing merged since 2026-10-29, so most candidates ≥6 stay blocked: A30 #1887 (~8.7, #1886), T09 #1858 (~7, #1864/#1698), A41 #1899 (~6, #1891/#1829/#1906), A43 #1900 (~6, #1863/#1698), T01 #1835 / T25 #1877 (#1698), A53 #1931 / A57 #1935 (`ruby_crawler.rs`, #1882/#1895/#1943), A55 #1933 (`redirect/mod.rs`). Free test-only: T60 #1968 rest (~7, after #1972: CI REQUIRED export, `assert_api_exercised`, `GIT_SHA256_FN` ×3, bash summary greps), T61 #1969 (~6; `e2e_bun_lockb.rs` held by #1905), T50 #1940 (~7; land the suites not held by #1698/#1843/#1893/#1894), T55 #1959 (~7, ~35 files; `tests/apply` is #1944), T58 #1957 (~6).
40+
Structural, scored `3·B + 2·U + 2·D + 2·C − risk`. Nothing merged since 2026-10-29, so most candidates ≥6 stay blocked: A30 #1887 (~8.7, #1886), T09 #1858 (~7, #1864/#1698), A41 #1899 (~6, #1891/#1829/#1906), A43 #1900 (~6, #1863/#1698), T01 #1835 / T25 #1877 (#1698), A53 #1931 / A57 #1935 (`ruby_crawler.rs`, #1882/#1895/#1943), A55 #1933 (`redirect/mod.rs`). Free test-only: T64 #1979 rest (copies held by 9 PRs; then a `shared_helper_copies` guard), T60 #1968 rest (~7, after #1972: CI REQUIRED export, `assert_api_exercised`, `GIT_SHA256_FN` ×3, bash summary greps), T61 #1969 (~6; `e2e_bun_lockb.rs` held by #1905), T50 #1940 (~7; land the suites not held by #1698/#1843/#1893/#1894), T55 #1959 (~7, ~35 files; `tests/apply` is #1944), T58 #1957 (~6).
4041
A26 #1867, A46 #1917, A44 #1918 score ~4.
4142

42-
Next sweep files, in order: `scan/policy.rs`, `scan/render.rs` after #1856/#1884; `python_lock.rs` (2,283), `python_script.rs` (1,768), `api/vendor_prefetch.rs`, crawler rows; `group_commit.rs` (2,163, region); `tests/apply/*` rows not covered by #1944 (their notes list fake tests). Held: `vlt_preflight.rs`, `agent_download.rs`, `apply.rs`, `get.rs`, `vendor.rs` (#1698); `rollback.rs` (#1864); `utils/fs.rs`, core `lib.rs` (#1891); `lock_fragments.rs`/`pdm_lock.rs` (#1898); `poetry_lock.rs` (#1865/#1898); `pnpm_workspace.rs` (#1863); `process.rs` (#1895); `go_env.rs` (#1876); `cargo_workspace.rs` (#1866). Leftovers from merged-pending sweeps: `socket_yml_args.rs` `min_severity` → `Option<u8>` + injected env (held by #1978/#1864), `cli_parse_scan` env subset test + `"1.5"` reject row (#1978), rollout `offer`/`manifest` fixtures in CLI+core → `test-fixtures` (#1985); `ui/text.rs` sweep and the text-shaping move out of `ui/mod.rs` (#1884), `StatusLine::stderr(json, silent)` → one `report` bool (scan/mod.rs, lock_cli.rs) (#1961); `toml_edit_ext` deletion and `uv_workspace` fold (#1923); 7 `build_*_purl` wrappers + `parse_jsr_purl`, purl's PurlKey asserts → `purl_key.rs`, pipenv `installed_major`'s `_root` (#1920); String `read_capped` wrapper, `lock_fragments`/`pdm_lock` `pub mod` → `pub(crate)` (#1914); `CargoCrawler::cargo_home`, `vendor/jvm` digest copies, async `fs::is_symlink` (#1907); `api_auth_fallback_warning` → `json_envelope` and `PlanNote` → `RunWarning` (callers in `vendor.rs`/`apply.rs`/`vex.rs`, #1698), `store_variants` test → core `npm_crawler.rs` (#1829/#1891/#1906/#1915) (#1948); `PathScope::matches` (1 caller, rollback L1814, in a loop) → `bind` once, plus `BoundScope::matching_pattern_indices` for rollback's `raw()` re-parse with `expect` (#1864); `AppliedVia` one-variant mirror + `from_core` (apply.rs, #1698) (#1967).
43+
Next sweep files, in order: `scan/policy.rs`, `scan/render.rs` after #1856/#1884; `python_lock.rs` (2,283), `python_script.rs` (1,768), `api/vendor_prefetch.rs`, crawler rows; `group_commit.rs` (2,163, region); `tests/apply/*` rows not covered by #1944 (their notes list fake tests). Held: `vlt_preflight.rs`, `agent_download.rs`, `apply.rs`, `get.rs`, `vendor.rs` (#1698); `rollback.rs` (#1864); `utils/fs.rs`, core `lib.rs` (#1891); `lock_fragments.rs`/`pdm_lock.rs` (#1898); `poetry_lock.rs` (#1865/#1898); `pnpm_workspace.rs` (#1863); `process.rs` (#1895); `go_env.rs` (#1876); `cargo_workspace.rs` (#1866). Leftovers from merged-pending sweeps: `socket_yml_args.rs` `min_severity` → `Option<u8>` + injected env (held by #1978/#1864), `cli_parse_scan` env subset test + `"1.5"` reject row (#1978), rollout `offer`/`manifest` fixtures in CLI+core → `test-fixtures` (#1985); `ui/text.rs` sweep and the text-shaping move out of `ui/mod.rs` (#1884), `StatusLine::stderr(json, silent)` → one `report` bool (scan/mod.rs, lock_cli.rs) (#1961); `toml_edit_ext` deletion and `uv_workspace` fold (#1923); 7 `build_*_purl` wrappers + `parse_jsr_purl`, purl's PurlKey asserts → `purl_key.rs`, pipenv `installed_major`'s `_root` (#1920); String `read_capped` wrapper, `lock_fragments`/`pdm_lock` `pub mod` → `pub(crate)` (#1914); `CargoCrawler::cargo_home`, `vendor/jvm` digest copies, async `fs::is_symlink` (#1907); `api_auth_fallback_warning` → `json_envelope` and `PlanNote` → `RunWarning` (#1698), `store_variants` test → core `npm_crawler.rs` (#1829/#1891/#1906/#1915) (#1948); `PathScope::matches` (rollback L1814 loop) → `bind` once, plus `BoundScope::matching_pattern_indices` for rollback's `raw()` re-parse with `expect` (#1864); `AppliedVia` one-variant mirror + `from_core` (apply.rs, #1698) (#1967).
4344

4445
#### Notes
4546
- Want issues (output-changing, found in sweeps): `ui::shell_word` misses `"`/`%VAR%` on cmd; `scan/gc.rs` `has_ledger` probes `--cwd` but `run_vendor_gc` reads `project_root()` (C43); gem stale probe dedups by purl and drops the platform suffix; `hatch::require_pip_installer_documents` ignores the `envs` override; `npm_range_admits(">=", v)` admits all; npm `upstream_registry_fallback` leaks `?token=` (use `redact_urls_in`); `vlt_heal` swallows `load_state`/lock read errors; `strip_url_userinfo`'s scp branch keeps `u:p@`.
@@ -55,11 +56,12 @@ Next sweep files, in order: `scan/policy.rs`, `scan/render.rs` after #1856/#1884
5556
- Test runners that pin `--cwd` must not also set the child's cwd to the project, or a crawler reading the process cwd passes (found by /code-review on #1944).
5657
- Left from #1929: `UpdateArgs.pin_version` as `semver::Version` waits on `lib.rs` (#1842); `sweep_orphan_vendor_dirs` forwarder on `vendor.rs` (#1698).
5758
- Left from #1936: `vendor/gem.rs` `plan_gemfile_edit`, `vex/discover/gem.rs` (~L252), `redirect/upstream/gem.rs` (~L661) hand-scan `=begin`: use `manifest::code_lines`. Its heredoc heuristic misreads `x <<FLAG` and indented `<<ID` terminators.
58-
- Left from #1939: `remove.rs` L1327 doc still names the deleted `revert_vendor_entry` (#1875); `commands/repair.rs` ~L429 copies the build-once client (use `vendored_backend::repair::ensure_client`, #1875); `ApplyRequest.manifest` → `&HashMap` (drop `records_manifest`) and `stop_on_failure` wait on #1698/#1864/#1875; split `repair()`'s phases.
59+
- Left from #1939: `remove.rs` L1327 doc still names the deleted `revert_vendor_entry` (#1875); `commands/repair.rs` ~L429 copies `ensure_client` (#1875); `ApplyRequest.manifest` → `&HashMap` (drop `records_manifest`) and `stop_on_failure` wait on #1698/#1864/#1875; split `repair()`'s phases.
5960
- Keep a single-caller fn that is a constant's only test seam or names a phase of a long fn (`fetch_concurrency`, `build_candidates`).
6061
- Left from #1955: PTY watchdog SIGHUPs, 10 s prompt waits.
6162
- Left from #1967: core `UpdateTimeouts::from_env` and the notifier both parse `SOCKET_UPDATE_TIMEOUT_MS`; core caps `connect` at 10 s, the notifier doesn't (A09/C19).
62-
- Left from #1964: `CI`/`GITHUB_ACTIONS` still reach test children; a HOME pin in `hermetic::command` hits ~150 suites. Test a scrub with a parent-env canary.
63-
- Left from #1975: `blob_fetcher` `on_progress`/`OnProgress` (callers in `fetch_stage.rs` #1855, `repair.rs` #1875, `rollback.rs` #1864) and `BlobFetchResult.success` (literals in #1855's tests); `concise_fetch_error` re-parses the client's text; `store_verified_blob` vs `stream_cache_entry_atomic` drift. `scan --json` serializes `BatchPackagePatches`: keep a serialize test.
64-
- Left from #1978: T57 can use `clap_parse::global_defaults()`; `derive(PartialEq)` on `GlobalArgs` (after #1842) retires `Globals`. The scrub is `Once`: set vars via `try_parse_with_env`; binaries calling `run()` in-process keep `FORCED_OPT_OUTS`.
65-
- Left from #1972: `run_container` takes no extra args (sbt `run_cell`, `docker_vendor_common::docker_command`, npm vlt smoke still build `docker run`); `host.docker.internal` URL ×15. Docker can't run here: build the 15 binaries with `--features docker-e2e`, run host-side tests.
63+
- Left from #1964: `CI`/`GITHUB_ACTIONS` still reach test children; test a scrub with a parent-env canary.
64+
- Left from #1975: `blob_fetcher` `on_progress` (#1855/#1875/#1864) and `BlobFetchResult.success` (#1855); `concise_fetch_error` re-parses the client's text; `store_verified_blob` vs `stream_cache_entry_atomic` drift. `scan --json` serializes `BatchPackagePatches`: keep a serialize test.
65+
- Left from #1978: T57 can use `clap_parse::global_defaults()`; `derive(PartialEq)` on `GlobalArgs` (after #1842) retires `Globals`. Set vars via `try_parse_with_env` (the scrub is `Once`).
66+
- Left from #1988: `assert_proxy_served_scans` counts 404s too (wrong mount path stays green, executed). A `#[path]` in a non-`mod.rs` file resolves under its stem dir: declare in `main.rs`.
67+
- Left from #1972: `run_container` takes no extra args (3 callers still build `docker run`); `host.docker.internal` ×15. No Docker here: build with `--features docker-e2e`.

0 commit comments

Comments
 (0)