Skip to content

Sweep scan/hosted nuget.rs and takeover.rs - #1903

Merged
Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
arch-refactor/sweep-scan-hosted-nuget-takeover
Nov 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 3 commits into
mainfrom
arch-refactor/sweep-scan-hosted-nuget-takeover

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

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

Architecture refactor sweep of crates/socket-patch-cli/src/commands/scan/hosted/{nuget,takeover}.rs, coverage rows 22 and 24 of the ledger (python.rs is held by #1883). It follows the audit notes and Part 4 (structure) of the living document.

Changes

takeover.rs

nuget.rs: dropped the bare (#352) from the module doc.

Tests: none changed. The existing explain and names_package unit tests still apply. classic_takeover_keeps_vendored_alias_copy_patched (mode_migration_npm) guards unwires_alias_copy: breaking its yarn.lock match turned that test red.

Diff: +65 / −94, all production code in takeover.rs and nuget.rs. No test lines changed.

Behavior change

None. Output bytes, codes and skip reasons are the same.

Risk

Low. This is a local inlining in one CLI module, with no cfg(windows)/cfg(unix) code touched.

Verification

  • cargo clippy -p socket-patch-cli --all-features -- -D warnings passes.
  • The socket-patch-cli lib tests pass (940).
  • These suites pass: scan (247), mode_migration_{cargo,composer,npm,pypi,vlt,bun}, in_process_vendor, and in_process_vendor_{pnpm,bun}_takeover.
  • /code-review high ran. I fixed the dead conjunct, the stale comment and the repeated code literal. I kept unwires_alias_copy named, because inlining it nested 5 closures.

Not fixed here (pre-existing; each changes output)

  • The residual-refusal pin split doesn't decode the purl. It splits by hand without percent-decoding, so pkg:pypi/six@1.16.0%2Bsocket.1 would advise six==1.16.0%2Bsocket.1. purl_parts decodes, but switching to it changes the message bytes. A14 (Tracking: build and classify purls through one validated utils::purl API #748).
  • The takeover and vendor --revert classify a residual keep differently. residual_only_keep doesn't require kept_artifact, while core RevertOutcome::kept_for_residual_reference does. A wet revert always sets it, but a dry-run revert emits the residual warning without it.
  • No test asserts the residual refusal's detail text. I mutated that string and every suite still passed.
  • Bare issue references: the review suggested keeping them. The refactor procedure deletes bare issue numbers, and the "why" text around them stays.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeNy5nnxWTLcart5Joa


Note

Low Risk
Local CLI refactor in hosted scan takeover logic with explicit parity on output bytes; no auth, data, or cross-platform paths touched.

Overview
Refactors takeover.rs and trims nuget.rs module docs as an audit-driven structure sweep, with no intended behavior change to warning codes, skip reasons, or JSON output.

In Takeover::stage, vendored-revert refusal handling is inlined (replacing revert_keeps_wiring, residual_takeover_warning, and drifted_takeover_warning) and the keep-wiring check is simplified to kept_artifact || drift_skipped() after dropping a dead residual-warning conjunct. Repeated "redirect_vendored_revert_failed" strings become a REVERT_FAILED const; residual lookup uses RESIDUAL_REFERENCE_CODE. KEPT_VENDORED is now module-private.

Smaller cleanups: cargo ledger-less probing uses purl_parts + type filter instead of a pkg:cargo/ prefix chain; unwires_alias_copy takes &BTreeSet and unpinned uses is_some_and; bare issue references are removed from comments. nuget.rs drops (#352) from its module doc.

Reviewed by Cursor Bugbot for commit 3c9459a. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Nov 1, 2026
Inline revert_keeps_wiring, residual_takeover_warning,
drifted_takeover_warning and unwires_alias_copy into their only
callers in Takeover::stage and Takeover::unpinned. The residual-ref
check now uses vendor::RESIDUAL_REFERENCE_CODE instead of a copy of
the literal, KEPT_VENDORED is private, and the cargo probe filters
purl_parts by type instead of a starts_with + then + flatten chain.
Bare issue references are dropped from comments here and in nuget.rs.

Output bytes, codes and skip reasons are unchanged.

Assisted-by: Claude Code:claude-opus-5-5
Drop the residual-reference conjunct from the keep-wiring branch: it
is reached only when residual_only_keep returned None, so a residual
warning there implies drift_skipped, which the branch already checks.
Fix that branch's comment to cover kept_artifact, keep the yarn alias
rule as the named unwires_alias_copy predicate, and name the repeated
refusal code REVERT_FAILED.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 1, 2026 01:11
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 1, 2026
Assisted-by: Claude Code:claude-opus-5-5

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 3c9459a. Configure here.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 1, 2026
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 3c9459a.

  • CI: all PR checks green on 3c9459a (32 success, 50 skipped); mergeable, no conflicts.
  • Bugbot: reviewed 3c9459a, no findings; no unresolved review threads.
  • Reviewer note: local cleanups in scan/hosted/nuget.rs and scan/hosted/takeover.rs only.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: Behavior-neutral cleanup of scan/hosted/takeover.rs (plus a one-line doc change in nuget.rs). It inlines three single-caller warning helpers into Takeover::stage, replaces repeated code literals with REVERT_FAILED and RESIDUAL_REFERENCE_CODE, drops one dead conjunct, simplifies the cargo purl probe and unwires_alias_copy's Option parameter, and removes bare issue numbers from comments.

Risk: low. Warning codes and detail strings are byte-identical, and no platform-specific code changed.

Look here:

  • crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs:281:`` the keep-wiring branch without the residual conjunct. It's dead: residual_only_keep returns `None` only when drift was skipped or no residual warning exists, so a residual-only outcome never reaches this branch.
  • crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs:263:```` the inlined residual refusal, whose text matches the deleted helper
  • crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs:146:````` purl_parts(purl).filter(ty == "cargo") is equivalent to the old `starts_with("pkg:cargo/")` check, because `purl_parts` doesn't normalize the type
  • crates/socket-patch-cli/src/commands/scan/hosted/takeover.rs:342:`````` unpinned now unwraps the `Option` with `is_some_and`, with the same result for `None`

Verified: Read the full diff and compared each inlined string against the deleted helpers. rustfmt --check passes on the head file. The PR's CI run is green (ci-ok and clippy pass). Bugbot found no issues on this head. CHANGELOG.md is untouched.

Changes I made: none.

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


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[agent] Reviewed 3c9459acd531302fcfc6b55219c9dab7770c7a12. Checked takeover revert refusal equivalence, residual wiring and alias-copy protection. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 362bde4 Nov 2, 2026
82 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the arch-refactor/sweep-scan-hosted-nuget-takeover branch November 2, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants