Skip to content

Commit fa2001b

Browse files
committed
refactor: sweep scan rollout/withdrawn (#1985)
Assisted-by: Claude Code:claude-opus-5-5
1 parent 57fe087 commit fa2001b

3 files changed

Lines changed: 53 additions & 9 deletions

File tree

‎coverage/files.tsv‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs 721 audit-cli-tests
2525
crates/socket-patch-cli/src/commands/scan/mod.rs 5264 audit-cli-tests 5ca5623 2026-10-30 - run_scan L1725-3588 = 1,864 lines (C11), discover_selected twice L2818/L2850; inline crawl_scope L883, overlap_from_states L967, select_accessible L735, is_info_note L1228, report_policy_error L1711; selection_args L651 no-op; takeover block L926-1123 -> hosted.rs; gradle_scan L1194-1387 -> discovery; T20 x4; ~150 comment lines; 7 weak tests (3 can't-fail takeover_detail)
2626
crates/socket-patch-cli/src/commands/scan/policy.rs 1051 audit-cli-tests 5ca5623 2026-10-30 - inline admit_crawled L497; drop getters judges_nested_roots L264, root_excluded L462, writes_allowed L471; 5 FilteredEntry literals -> ctor; lock_markers L105 (T31); select L556 (T15); policy_bypass_warnings L803 reloads policy, get-only, purl unsanitized L858, belongs in core policy; Warning: vs Warning ({code}) (T34); ~10 comment lines; L988 .contains
2727
crates/socket-patch-cli/src/commands/scan/render.rs 1036 audit-cli-tests 5ca5623 2026-10-30 - highest_severity L396 (T10, raw API label); vuln_label L385 (T28); patch_block L437 prints API text unfiltered (T23); get format_dry_run L619 = dry_run_line; inline quoted_list L201, sorted_vulns L371; *_COL consts needlessly pub(super); 4 test banners; ~8 comment lines; weak tests L856 (.contains), highest_severity_picks_worst copies core
28-
crates/socket-patch-cli/src/commands/scan/rollout.rs 860 audit-cli-tests cbbbdb9 2026-11-01 - fold human_lines+HumanContext into human; inline short_name (use purl helper); Gate::new/with_prior/with_references -> struct literal (mod.rs L3262, hosted.rs L692); move Gate::may_admit stage predicate to core Stage; move 11 core-only tests (L370-L859) to core rollout/stage.rs, which has none
29-
crates/socket-patch-cli/src/commands/scan/rollout_args.rs 126 audit-cli-tests cbbbdb9 2026-11-01 - drop pub use re-exports (import RolloutCarry from core at mod.rs L1679); parses_numbers_and_none duplicates tests/cli_parse_scan.rs; delete cli_parse_scan::max_new_patches_env_is_read_at_run_time (subset of flag_beats_env_beats_file)
28+
crates/socket-patch-cli/src/commands/scan/rollout.rs 860 audit-cli-tests cbbbdb9 2026-11-01 PR #1985 fold human_lines+HumanContext into human; inline short_name (use purl helper); Gate::new/with_prior/with_references -> struct literal (mod.rs L3262, hosted.rs L692); move Gate::may_admit stage predicate to core Stage; move 11 core-only tests (L370-L859) to core rollout/stage.rs, which has none
29+
crates/socket-patch-cli/src/commands/scan/rollout_args.rs 126 audit-cli-tests cbbbdb9 2026-11-01 PR #1985 drop pub use re-exports (import RolloutCarry from core at mod.rs L1679); parses_numbers_and_none duplicates tests/cli_parse_scan.rs; delete cli_parse_scan::max_new_patches_env_is_read_at_run_time (subset of flag_beats_env_beats_file)
3030
crates/socket-patch-cli/src/commands/scan/socket_yml_args.rs 58 audit-cli-tests cbbbdb9 2026-11-01 - value_parser should return Option<u8>: today min_severity is parsed, discarded, stored as String and re-parsed in overrides (L42 error prefix unreachable); read env via injected Option<&str> like RolloutArgs::resolve
3131
crates/socket-patch-cli/src/commands/scan/vendor_flow.rs 2099 audit-cli-tests cbbbdb9 2026-11-01 - preview_vendor takes &GlobalArgs, computes takeover refusals+origins (3 callers repeat); assert on PreviewVerdict, delete test-only preview_vendor_json; inline ledger_key_for, vendor_under_lock, format_nothing_vendored; dedupe 4 boxed_* Windows docs; fix stale L347, L1020, L1700 docs; print_refusals writes [warning] to stdout; see T35, T09, T25
32-
crates/socket-patch-cli/src/commands/scan/withdrawn.rs 350 audit-cli-tests cbbbdb9 2026-11-01 - warn() prints under --silent but leg warnings (L272) do not: pick one; store RunWarning not (String,String); inline is_gone; drop #1606/#1607 history in module doc; delete only_withdrawn_and_not_found_are_gone (restates matches!); see T39, T09
32+
crates/socket-patch-cli/src/commands/scan/withdrawn.rs 350 audit-cli-tests cbbbdb9 2026-11-01 PR #1985 warn() prints under --silent but leg warnings (L272) do not: pick one; store RunWarning not (String,String); inline is_gone; drop #1606/#1607 history in module doc; delete only_withdrawn_and_not_found_are_gone (restates matches!); see T39, T09
3333
crates/socket-patch-cli/src/commands/update.rs 613 audit-cli-tests cbbbdb9 2026-11-01 PR #1929 UPDATE_TARGET private; pin_version as semver::Version, delete unreachable Err arm L257-L265; parse_version_pin reuse core parse_release_tag (CLI accepts vv3.4.0); channel_id -> core InstallChannel; inline download_status; trim note_warning doc; RunWarning::new; delete checking_latest_status_line + sentence_case asserts; ~20 comment lines
3434
crates/socket-patch-cli/src/commands/user_patches.rs 188 audit-cli-tests cbbbdb9 2026-11-01 PR #1929 evidence/skip_protected/partition repeat one loop; return RunWarning not tuples; refuse reuse lock_cli::error_envelope; DETECTED prefix list belongs in core UserPatchIndex; see T38
3535
crates/socket-patch-cli/src/commands/vendor.rs 7937 audit-cli-tests cbbbdb9 2026-11-01 - run_eject 554 lines, run 306, run_check 210; 10 RunWarning literals; refuse_candidate helper for 10 Failed+report copies; eject envelope preamble x4; misplaced docs L558, L3446; dead dry-run VEX L2319; sweep_stale_artifact let _ then Removed; inline gc_note; merge 2 FIFO tests; move core/envelope/orphan tests out; 2 vacuous if-let tests; ~70 comment lines; see T35-T38,T40,T41
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
[agent] 2026-11-02: architecture refactor (sweep)
2+
3+
`main` @ `cbbbdb9`, unchanged since the last run. The previous entry was structural (#1978), so this run is a sweep. None of my 26 open PRs has merged or closed, and the discussion has no maintainer steering.
4+
5+
I took `scan/{rollout,rollout_args,socket_yml_args,withdrawn}.rs`, the next free audited rows. No open PR touches them; I checked by diffing all 70 open heads locally. The callers I changed in `scan/mod.rs` and `scan/hosted.rs` are held by #1897, #1884, #1856, #1930, #1977, #1883 and #1903, and the branch merges cleanly with all seven (checked with `git merge-tree`).
6+
7+
**PR:** [#1985](https://github.com/SocketDev/socket-patch/pull/1985), ready for review, with Bugbot triggered. CI is green on all 82 checks, and Bugbot found no issues.
8+
9+
### Per file
10+
- **`rollout.rs`:**
11+
- 2 fns inlined (`human_lines`, `short_name`) and `HumanContext` deleted.
12+
- 3 builder methods deleted (`Gate::new`/`with_prior`/`with_references`), so the callers use a struct literal.
13+
- `Gate::may_admit` moved to core `Stage::may_admit`.
14+
- 11 core-only tests moved to core `rollout/stage.rs`.
15+
- Net prod −63, test −316.
16+
- **`rollout_args.rs`:** 2 re-exports dropped and 1 duplicate parse test deleted. Net −17.
17+
- **`withdrawn.rs`:**
18+
- Warnings are stored as `RunWarning`.
19+
- 3 lines of bare issue numbers removed from the module doc.
20+
- `is_gone` is kept, with a why-line.
21+
- Net −2.
22+
- **`socket_yml_args.rs`:** unchanged. Its noted type change reaches `cli_parse_scan.rs` (#1978) and `hosted_bundle.rs` (#1864), so the `swept` cell stays `-`.
23+
- **Core `rollout/stage.rs`:** gains `may_admit` (+11) and its first tests (+387), one of them new.
24+
- **Total:** +440 / −437. Production is about −54.
25+
26+
### Evidence
27+
- When I removed the incomplete-lookup guard from `Stage::may_admit`, the new `a_spent_budget_admits_only_packages_an_earlier_directory_admitted` test turned red.
28+
- Clippy (workspace, all features) is clean.
29+
- CLI lib passes 927. Core lib passes 7,006, and the 4 failures are the known root-sandbox ones.
30+
- The `in_process_scan_withdrawn`, `scan_rollout_e2e` and `hosted_memory_rollout` suites pass. `in_process_redirect` passes except its 3 root-only chmod tests.
31+
- `/code-review high` raised 7 findings. 3 are fixed:
32+
- `is_gone`'s test was restored, because `build_failed` keeps a pin here but core's vendor fetch treats it as unavailable;
33+
- `may_admit` now states its guard once;
34+
- `sel_purl` is built once per candidate.
35+
36+
The PR gives a reason for each of the other 4.
37+
38+
### Merges recorded
39+
None.
40+
41+
### Lessons
42+
- A status-table test that "restates `matches!`" is still worth keeping when another module classifies the same statuses differently. Grep the status strings across the crates before you delete one.
43+
- Before you edit a caller in a file that open PRs hold, run `git merge-tree --write-tree HEAD pr/<n>` against each of those PRs. It is cheap and settles whether the edit conflicts.

‎register/90-refactor.md‎

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

44
#### In flight
55
| PR | Kind | Scope | State |
@@ -30,6 +30,7 @@ _Last updated 2026-11-02T09:20Z · main @ cbbbdb9_
3030
| #1972 | structural | T60 #1968 slice 1: `tests/docker_common` + 15 `docker_e2e_*` suites (test-only, +260/−1,113) | ready; green |
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 |
33+
| #1985 | sweep | `scan/{rollout,rollout_args,withdrawn}.rs` (+ `Stage::may_admit` and 11 tests moved to core `rollout/stage.rs`) | ready; green |
3334

3435
#### Merged
3536
_None._
@@ -38,25 +39,25 @@ _None._
3839
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).
3940
A26 #1867, A46 #1917, A44 #1918 score ~4.
4041

41-
Next sweep files, in order: `scan/{rollout,rollout_args,socket_yml_args,withdrawn}.rs` (free); `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: `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).
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 (#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).
4243

4344
#### Notes
4445
- 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@`.
45-
- Check every "redundant filter" note against all consumers and derived `GlobalArgs` (the `args.rs` proxy filter, `select_one`'s `quiet()`).
46+
- Check "redundant filter" notes against all consumers and derived `GlobalArgs`.
4647
- Clap field doc comments are `--help` output: leave them.
4748
- `--cwd` defaults to `.`: absolutize before walking `ancestors()`; test via chdir under `#[serial]`.
4849
- `vendor/cargo_config.rs` (~L165-L190) has a third `.cargo/config` pick: fold into `config_layer_paths` when swept.
4950
- Build with `CARGO_PROFILE_DEV_DEBUG=0 CARGO_INCREMENTAL=0` (disk). CI's clippy has no `--tests`.
5051
- Root-sandbox failures that pass in CI: 2 chmod tests in `tests/repair`, and core `copy_tree` symlinked-root, `vlt_heal` unremovable-lock, `pypi_poetry`/`pypi_requirements` wire-failure tests.; `in_process_redirect`'s 3 chmod tests
51-
- The proxy refuses branch deletions: name branches right first time (`sweep-scan-gc-policy` is stale).
52+
- The proxy refuses branch deletions: name branches right first time.
5253
- `run_redirect_selected` still takes `Option<Envelope>` keyed on `common.json`; making it a value touches `get.rs` (#1698) and T24.
5354
- Making a `pub` core item private can leave rustdoc links to it; unlink in the same commit.
5455
- 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).
5556
- Left from #1929: `UpdateArgs.pin_version` as `semver::Version` waits on `lib.rs` (#1842); `sweep_orphan_vendor_dirs` forwarder on `vendor.rs` (#1698).
56-
- Left from #1936: `vendor/gem.rs` `plan_gemfile_edit`, `vex/discover/gem.rs` (~L252) and `redirect/upstream/gem.rs` (~L661) still hand-scan `=begin`; route them through `manifest::code_lines` (the pin writer refuses a Gemfile whose duplicate `gem` line sits in `=begin`). `code_lines`'s heredoc heuristic reads `x <<FLAG` (a shift) as a heredoc and closes a plain `<<ID` at an indented terminator.
57+
- 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.
5758
- 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.
5859
- 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`).
59-
- Left from #1955: PTY watchdog SIGHUPs; fixed 10 s prompt waits.
60+
- Left from #1955: PTY watchdog SIGHUPs, fixed 10 s prompt waits.
6061
- 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).
6162
- 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.
6263
- 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.

0 commit comments

Comments
 (0)