Skip to content

Commit c8f261a

Browse files
committed
refactor: T51 in PR #1944
Assisted-by: Claude Code:claude-opus-5-5
1 parent 8fad7aa commit c8f261a

3 files changed

Lines changed: 36 additions & 11 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[agent] 2026-11-01: architecture refactor (structural)
2+
3+
`main` @ `cbbbdb9`. The previous entry was a sweep (#1939), so this run looked for structural work. Nothing has merged since 2026-10-29, and the older candidates scoring 6 or more are still held by open PRs. The audit filed T50-T52 ([#1940](https://github.com/SocketDev/socket-patch/issues/1940)-[#1942](https://github.com/SocketDev/socket-patch/issues/1942)) an hour before this run. **T51 [#1941](https://github.com/SocketDev/socket-patch/issues/1941)** scored about 12 (D: 9 duplicate fixture sets and about 3 for net deleted lines, C2, risk L). Its files are free (the only open PR touching `tests/apply/` is #1855, on `apply_silent_staging_errors.rs`), so I claimed it. There was no maintainer steering, and no PR of mine merged or closed.
4+
5+
**PR:** [#1944](https://github.com/SocketDev/socket-patch/pull/1944), ready for review. Bugbot has been triggered and CI is running. `/code-review high` ran first: I fixed 9 of its 10 findings and gave the reason for the other one in the PR.
6+
7+
### What changed (test-only)
8+
- **11 suites in the `apply` binary** now build fixtures through `apply_fixture`:
9+
- helpers added: `hashed_file`, `index_js_patches`, `link_dir` and `run_gem`;
10+
- `stage_blob` now writes through `common::write_blob`.
11+
- **`common/envelope.rs`** gains `event_for`, which replaces 4 private `event` fns.
12+
- **Deleted:** about 25 local helpers (manifest, blob, package and gem writers; runners; `dir_hash`/`collect_files`) and 13 redundant telemetry env pairs.
13+
- **gem_stale_bundler_plugin** now gets the `BUNDLE_*` scrub.
14+
- **Moved:** the fallback-home human test now sits beside its `--json` twin.
15+
- **Tests:** 2 invariant tests compare `.socket/` snapshots (shown red by mutating the sentinel after the run); 157 before and after.
16+
- **Total:** +410 / −1,045, with 0 production lines.
17+
18+
### Behavior
19+
None in the product.
20+
21+
### Merges recorded
22+
None.
23+
24+
### Lessons
25+
- **A shared runner must keep the cwd semantics of the runners it replaces.** Runners that pinned `--cwd` deliberately left the child in the runner's directory. /code-review caught that the new `run_gem` set the cwd to the project.
26+
- **Step 3 of #1941 is out of reach for one binary.** It asked for a single manifest writer, but `common::write_minimal_manifest` has 100+ callers in other binaries, so it waits for T55.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ _Last updated 2026-11-01T15:52Z · main @ cbbbdb9 · owner: audit-cli-tests_
5353
| T48 | 3 | vex_consumed re-derives core rules: Go hosted-module uuid suffix (any `<uuid>/…` vs core `/vN`), Maven `-socket.{hex8}` without lowercasing/dash strip (vex_consumed L418, also core redirect/gradle.rs L192), cargo registry prefix ×3; `PRODUCT_MANIFESTS` (vex.rs L1858) copies core product.rs L84 minus csproj/gemspec hints. Child of #855. | | to verify |
5454
| T49 | 3 | vlt naming is crossed: CLI `vlt_preflight.rs` is the vendored preflight, core `redirect::vlt_preflight` the hosted probe, and the CLI hosted wrapper `artifact_preflight` lives in `vlt_heal.rs`; `redirect_vlt_reinstall_required` returns in 3 shapes (Value, tuple, Option). Part of T27/T05. | | to verify |
5555
| T50 | 3 | Toolchain e2e gate `required()`/`skip()` hand-written per suite (15 / 8 defs); drifted: `=0` required for golang/sbt/npm/nuget/deno, not cargo/uv/bundler; docker accepts only `"1"`. | #1940 | filed #1940 |
56-
| T51 | 3 | 9 `tests/apply` suites re-write `apply_fixture`'s manifest/blob/package/gem/event helpers in the same binary; `apply_fixture` and `common::write_minimal_manifest` are two writers; gem_stale_bundler_plugin skips the `BUNDLE_*` scrub. | #1941 | filed #1941 |
56+
| T51 | 3 | 9 `tests/apply` suites re-write `apply_fixture`'s manifest/blob/package/gem/event helpers in the same binary; `apply_fixture` and `common::write_minimal_manifest` are two writers; gem_stale_bundler_plugin skips the `BUNDLE_*` scrub. | #1941 | in PR #1944 |
5757
| T52 | 3 | PTY runner (openpty, spawn, feed, collect) copied 7× beside `common/pty_io.rs`; only 2 copies set `TERM`. | #1942 | filed #1942 |
5858
| T53 | 3 | `hermetic::command` removes `SOCKET_PROXY_URL`, so a fetch with no mock reaches the live public proxy: `apply_invariants` L355 does (executed). | | to verify |
5959
| T54 | 3 | The `--json` reader ratchet (`envelope_helper_copies`) is name-based: misses 7 `fn envelope(` parsers and private `json_stdout`/`run_json`/`dupvuln_events`; stale allowlist entries pass; its `test_sources` is a byte copy of `shared_helper_copies`'. Part of C32. | #1089 | to verify |

‎register/90-refactor.md‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
### Refactor routine
2-
_Last updated 2026-11-01T15:15Z · main @ cbbbdb9_
2+
_Last updated 2026-11-01T17:15Z · main @ cbbbdb9_
33

44
#### In flight
55
| PR | Kind | Scope | State |
@@ -21,21 +21,19 @@ _Last updated 2026-11-01T15:15Z · main @ cbbbdb9_
2121
| #1929 | sweep | `cli/src/commands/{update,user_patches,vendor_orphans,vlt_heal}.rs` (+ `InstallChannel::id` in core `update/channel.rs`) | ready; CI (82 checks) and Bugbot green |
2222
| #1936 | structural | A54 #1932: `formats/gem/{gemspec,manifest}.rs`, `vex/product.rs` (`for_each_code_line` → `code_lines`) | ready; CI (82 checks) and Bugbot green |
2323
| #1939 | sweep | `cli/src/commands/vendored_backend/{mod,repair}.rs` | ready; CI (82 checks) and Bugbot green |
24+
| #1944 | structural | T51 #1941: 11 `tests/apply` suites + `common/envelope.rs` onto `apply_fixture` (test-only, −635 net) | ready; CI and Bugbot pending |
2425

2526
#### Merged
2627
_None._
2728

2829
#### Queue
29-
Structural candidates, scored `3·B + 2·U + 2·D + 2·C − risk`. As of 2026-11-01T13Z, the audit's A53-A57 (#1931-#1935) are new: A54 is in #1936; A53 #1931 (~9) and A57 #1935 wait on `ruby_crawler.rs` (#1882, #1895); A55 #1933 (~11, risk M) waits on `redirect/mod.rs` (#1841, #1843, #1866, #1885, #1886, #1893); A56 #1934 is a fixer bug (free, ~5). Every older one scoring 6 or more is blocked by an open PR (nothing merged since 2026-10-29); T38-T45 (#1908-#1912, #1924-#1927) touch `scan/mod.rs`, `vendor.rs`, `rollback.rs` or `remove.rs`, all held. A46 #1917, A44 #1918 score ~4:
30-
1. A30 [#1887](https://github.com/SocketDev/socket-patch/issues/1887), Maven `${…}` interpolation ×5: **~8.7** (blocked by #1886: `maven_scope.rs`, `maven_reactor.rs`).
31-
2. T09 [#1858](https://github.com/SocketDev/socket-patch/issues/1858), lock acquire open-coded 9×: **~7** (blocked by #1864 `lock_cli.rs`, #1698 `vendor.rs`/`get.rs`/`agent_download.rs`).
32-
3. A41 [#1899](https://github.com/SocketDev/socket-patch/issues/1899), npm store-child classification ×3: D2 C2, risk M, **~6** (blocked by #1891, #1829 and #1906: `npm_crawler.rs`).
33-
4. A43 [#1900](https://github.com/SocketDev/socket-patch/issues/1900), package.json `workspaces` read 5×: **~6** (blocked by #1863 `governing_root.rs`, #1698 `vendor/mod.rs`).
34-
5. T01 [#1835](https://github.com/SocketDev/socket-patch/issues/1835) (~6.4) and T25 [#1877](https://github.com/SocketDev/socket-patch/issues/1877) (~6): blocked by #1698.
30+
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 candidates from the audit's T50-T55:
31+
1. T52 [#1942](https://github.com/SocketDev/socket-patch/issues/1942), PTY runner ×7, only 2 pin TERM: **~7** (check PTY suite files against open PRs).
32+
2. T50 [#1940](https://github.com/SocketDev/socket-patch/issues/1940), toolchain e2e gate ×10 drifted: **~7**, but e2e_golang* are held by #1698/#1843/#1893/#1894: land the free suites' slice.
33+
3. T55 (no issue yet), test helpers across binaries (`stderr_chatter` 5×, dir snapshot ~8×): ~6 once filed; includes a `run_bin_with_env` `extras` param so `apply_fixture::run_gem` stops copying it.
34+
A26 #1867, A46 #1917, A44 #1918 score ~4.
3535

36-
A05/A32 are the fixer's (#1898/#1895); A42 #1901 and A45 #1902 likewise. A26 [#1867](https://github.com/SocketDev/socket-patch/issues/1867) scores 4 (files free).
37-
38-
The next run is structural if a candidate scoring 6 or more has free files, else a sweep. Next sweep files, in order: `vlt_preflight.rs` (callers in `vendor.rs`, #1698), `vex_consumed.rs`, `vex_sources.rs` (free; check callers), `scan/policy.rs` and `scan/render.rs` once #1856/#1884 merge; core `utils/lock_fragments.rs` (wait on #1898); next free: `python_lock.rs` (2,283), `python_script.rs` (1,768), then `api/{blob_fetcher,date,retry,types}.rs` and the crawler rows. `purl_key_nuget_vendor_tests.rs` waits on #1907 (`purl_key.rs` mounts it); scan `policy`/`rollout`/`rollout_args`/`socket_yml_args`/`withdrawn` have free files but their callers sit in `scan/mod.rs` (#1856, #1884, #1897). Left from #1923: delete `toml_edit_ext` (move `ensure_table`/`has_table` beside their one caller each; `utils/mod.rs` held by #1914, `vendor/pypi.rs` by the pypi PRs) and fold `uv_workspace`'s two-call pattern (`governing_root.rs` #1863, `vendor/pypi.rs`). `process.rs` waits on #1895, `pdm_lock.rs` on #1898, `poetry_lock.rs` on #1865/#1898, `pnpm_workspace.rs` on #1863. Left from #1920: inline the 7 `build_*_purl` wrappers and `parse_jsr_purl` (callers held by #1882/#1886/#1891/#1698), move purl's PurlKey asserts to `purl_key.rs` (#1907), drop pipenv `installed_major`'s `_root` (callers in #1897 and the `vendor/pypi.rs` PRs). Waiting on PRs: `utils/fs.rs` and core `lib.rs` (#1891), `go_env.rs`/`go_installed_tools.rs` (#1876), `cargo_workspace.rs` (#1866), `cargo_build_cache.rs` (#1849), `group_commit.rs` is free but 2,163 lines (sweep a region). Skip `agent_download.rs`, `apply.rs`, `get.rs`, `vendor.rs` while #1698 is open, `rollback.rs` while #1864 is open. Left from #1914: delete the String `read_capped` wrapper (callers in `maven_repo.rs`, held by #1886/#1891; or rename `read_capped_typed`, touching `api/client.rs`, held by #1870/#1891); `lock_fragments`/`pdm_lock` `pub mod` → `pub(crate)` waits on #1898. Left from #1907: `CargoCrawler::cargo_home` forwarder (#1882), digest's `vendor/jvm` copies (#1886), async `fs::is_symlink` (#1891).
36+
Next sweep files, in order: `vex_consumed.rs`, `vex_sources.rs` (free; check callers); `scan/policy.rs`, `scan/render.rs` after #1856/#1884; `python_lock.rs` (2,283), `python_script.rs` (1,768), `api/{blob_fetcher,date,retry,types}.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: `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).
3937

4038
#### Notes
4139
- First-thread PRs: `review/2026-10-v1/register/90-refactor.md`.
@@ -56,6 +54,7 @@ The next run is structural if a candidate scoring 6 or more has free files, else
5654
- `hatch::require_pip_installer_documents` ignores the hatch.toml `envs` override (a shadowed pyproject `installer = "uv"` still refuses): output-changing, wants an issue.
5755
- `npm_range_admits(">=", v)` (an operator with no version) reads as `*` and admits everything, where npm admits nothing: output-changing, wants an issue (found by /code-review on #1920).
5856
- Commit *every* edit before a mutation test.
57+
- 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).
5958
- Wants an issue: npm `upstream_registry_fallback` (`redirect/upstream/npm.rs` ~L158) strips only userinfo, so a registry `?token=` reaches output and `--json` (fix: `redact_urls_in`, output-changing).
6059
- Wants an issue: `vlt_heal::artifact_preflight` swallows a `load_state` error and `rollback_heal` drops a lock read error with `.ok()` (output-changing).
6160
- Left from #1929: `UpdateArgs.pin_version` as `semver::Version` waits on `lib.rs` (#1842); `sweep_orphan_vendor_dirs` forwarder on `vendor.rs` (#1698).

0 commit comments

Comments
 (0)