[Mikola Lysenko (mikolalysenko)]

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

Sweep of crates/socket-patch-cli/src/commands/scan/{rollout,rollout_args,withdrawn}.rs. It applies the audit's coverage notes for those rows (arch-audit ledger, Part 4 tally and Part 5 "Misplaced: 11 scan/rollout.rs tests"). socket_yml_args.rs is left alone: its noted type change (min_severity: Option<String> → parsed) reaches tests/cli_parse_scan.rs (#1978) and hosted_bundle.rs (#1864).

Changes

scan/rollout.rs

• human_lines and HumanContext (one caller, human) are folded into human.
• short_name (one caller) is inlined.
• Gate::new/with_prior/with_references are deleted. They were builder methods with no invariant, so the two callers (scan/mod.rs, scan/hosted.rs) now build Gate with a struct literal.
• Gate::may_admit is deleted. Its stage-only predicate is now core Stage::may_admit, with the incomplete-lookup guard stated once. The hosted.rs caller calls gate.stage.may_admit and builds each candidate's purl once.
• Test move: 11 tests exercised only core rollout::stage (classify, mark_pinned, offers_from_results, lookup_incomplete, Stage::warnings, the lockless-pin regression). They now live in core rollout/stage.rs, which had no tests, and are otherwise unchanged. The tests that check the CLI's human() lines and updates[] stay.

scan/rollout_args.rs

• The pub(crate) use RolloutCarry and pub use MAX_NEW_PATCHES_ENV re-exports are dropped, and mod.rs names rollout::RolloutCarry.
• parses_numbers_and_none is deleted, because tests/cli_parse_scan.rs max_new_patches_takes_a_count_or_none and max_new_patches_rejects_malformed_values cover the same inputs through clap.

scan/withdrawn.rs

• Warnings are stored as RunWarning instead of (String, String) and handed to the envelope as they are.
• The bare #1606/#1607 references in the module doc are removed.
• is_gone and its status test are kept (see review below), with a doc line on why build_failed keeps the pin.

New test: a_spent_budget_admits_only_packages_an_earlier_directory_admitted covers Stage::may_admit. I removed the incomplete-lookup guard as a mutation, and the test failed at the "a failed lookup admits nothing" assert. It passes again on the real code.

Size

6 files, +440 / −437.

• Production: about −54 (rollout −63, rollout_args −3, withdrawn −2, core stage +11, callers +3).
• Tests: about +57. 316 lines left the CLI and 387 arrived in core: the 11 moved tests, their offer/manifest fixture builders (now in both crates) and the new test.

Behavior change

None. The output bytes, JSON and exit codes are unchanged.

Verification

• cargo clippy --workspace --all-features -- -D warnings is clean.
• cargo test -p socket-patch-cli --lib: 927 passed.
• cargo test -p socket-patch-core --lib: 7,006 passed. The 4 failures are the known root-sandbox ones (copy_tree symlinked root, vlt_heal unremovable lock, pypi_poetry/pypi_requirements wire failure); they pass in CI.
• in_process_scan_withdrawn (13), scan_rollout_e2e (38), hosted_memory_rollout (8) and in_process_redirect (149 of 152) pass. The 3 in_process_redirect failures are the known root-sandbox chmod tests.
• The branch merges cleanly with the open PRs that touch scan/mod.rs or scan/hosted.rs (Sweep scan/hosted.rs (helpers and tests region) #1897, Sweep scan/discovery.rs #1884, Fix vendored dry-run counting in-sync patches (#1850) #1856, Fix vendored planning ignoring PyPI refusals (#1908) #1930, Fix npm lock-entry refusals missed by vendored preview (#1976) #1977, Fix Pipenv stale-install remedy built twice (#1878) #1883, Sweep scan/hosted nuget.rs and takeover.rs #1903), checked with git merge-tree.

/code-review high

7 findings. 3 are fixed:

• Deleting is_gone's test lost the only check that build_failed keeps a pin, which core's vendor fetch treats the other way. I restored it.
• may_admit repeated may_admit_new's guard. It now states the guard once.
• sel_purl was built twice per candidate. It is now built once.

4 are not changed:

• The hard-coded SOCKET_MAX_NEW_PATCHES in human() was already there before this PR and is output text.
• The offer/manifest fixture builders now exist in both crates. Moving them to core's test-fixtures feature is wider than this sweep.
• Pushing into env.warnings directly matches what vendor.rs already does (Envelope::warn is a plain push).
• The inlined short_name keeps the exact former expression. Its golang behavior is unchanged and was already there before.

Risk: low. The production edits are mechanical, and the moved tests are byte-identical apart from crate:: paths.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeQgEa6vYRbZ6T9GcRp

---

Note

Low Risk
Mechanical refactor and test relocation around gradual-rollout admission and messaging; behavior is explicitly unchanged and guarded by existing e2e and unit tests.

Overview
This PR refactors scan rollout, rollout args, and withdrawn handling without changing CLI output, JSON, or exit codes.

Rollout: Gate is built with struct literals instead of new/with_* builders. Per-package admission moves from Gate::may_admit to Stage::may_admit in socket-patch-core, with the incomplete-lookup guard defined once; hosted lock eligibility calls gate.stage.may_admit and computes each candidate purl once. Rollout human messaging drops HumanContext/human_lines in favor of a single human() that reads Stage directly. RolloutCarry is imported from scan::rollout rather than rollout_args.

Tests: Eleven rollout tests that only exercised core (classify, pins, lockless-pin cap regression, etc.) move from scan/rollout.rs to rollout/stage.rs, plus a new test for Stage::may_admit under a spent budget. CLI-only tests for human()/updates[] stay; redundant parser tests are removed where clap integration tests already cover them.

Withdrawn: Sweep warnings use RunWarning end-to-end instead of (code, detail) tuples; module docs are trimmed (including explicit issue refs).

^{Reviewed by Cursor Bugbot for commit 010e80b. Configure here.}

---

Generated by Claude Code

[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 010e80b. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at 010e80b.

• CI: 32/32 checks green on 010e80b (50 skipped); mergeable, no conflicts.
• Bugbot: reviewed 010e80b, no findings; no open review threads.
• Reviewer note: refactor sweep of scan/{rollout,rollout_args,withdrawn}.rs plus Stage::may_admit in core; the last commit keeps is_gone and simplifies may_admit per review.
• Slack announcement not sent this run (Slack send unavailable to the agent); a later run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. A cleanup sweep of scan/rollout.rs, rollout_args.rs and withdrawn.rs. The CLI's Gate::may_admit becomes Stage::may_admit on the existing core rollout/stage.rs, and 11 unit tests move there with it. The CLI's Gate builder methods become struct literals, the human_lines/HumanContext/short_name helpers fold into human(), and withdrawn.rs stores RunWarning values directly instead of (String, String) pairs. No behavior change intended.

Risk: low. I checked each logic change by hand. Old may_admit_new() || (!X && admitted) reduces to the new !X && (budget≠0 || admitted). The swapped || operands in hosted.rs have no side effects. The inlined human() computes the same values. warnings.extend matches the old warn() loop. The moved tests match the originals line for line except for import paths.

Look here

• stage.rs:364: may_admit, the only real logic move
• hosted.rs:1058: call site (operands reordered, sel_purl computed once)
• rollout.rs:114: inlined human()
• withdrawn.rs:117: warnings now typed
• rollout_args.rs:1:`` MAX_NEW_PATCHES_ENV/`RolloutCarry` re-exports dropped (nothing in the repo uses them)

Verified

• Read the full diff. No CHANGELOG.md change and no debug leftovers. Test counts: CLI rollout.rs 16→5, core stage.rs 0→12 (11 moved, 1 new).
• cargo test -p socket-patch-core --lib rollout: 30 passed. cargo test -p socket-patch-cli --lib scan::: 262 passed.
• CI on 010e80b: ci-ok and clippy green (32 success, 50 skipped by path filters, 0 failed). Bugbot passed. No unresolved review threads.

Changes I made: none.

Open questions (non-blocking): rollout_args::tests::parses_numbers_and_none was deleted, not moved. tests/cli_parse_scan.rs covers most of it, but the "1.5" rejection case is no longer tested anywhere.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] On the open question about the "1.5" case from the deleted parses_numbers_and_none: I'm leaving it out of this PR and not pushing. A push would re-run CI on a PR that is already armed for merge.

• parse_max_new_patches accepts a value only if str::parse::<u32> accepts it, so "1.5" is rejected the same way as "five", which tests/cli_parse_scan.rs::max_new_patches_rejects_malformed_values still covers.
• The natural place for a "1.5" row is that test's list, but cli_parse_scan.rs is being rewritten by test(cli): parse suites onto hermetic::try_parse (T56) #1978. I've logged it as a follow-up to add once test(cli): parse suites onto hermetic::try_parse (T56) #1978 lands.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 010e80bc2318a066b3df1708e1f65bedf4705b32. Checked rollout admission equivalence, incomplete-lookup gates, shared budgets and withdrawn-warning recording. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.