[Mikola Lysenko (mikolalysenko)]

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

Sweep of crates/socket-patch-cli/src/commands/{update,user_patches,vendor_orphans,vlt_heal}.rs, following their coverage/files.tsv audit notes (architecture refactor routine; living document in discussion #1830, Part 4 "structure"). All four files were audited and free of open PRs; the structural candidates scoring 6 or more are all still blocked by open PRs.

Changes

• update.rs
  • Inlined download_status and the CHECKING_LATEST const into their one call site each.
  • Moved channel_id to core as InstallChannel::id, beside channel_label, and pinned its telemetry names with a table test. Shown red→green by mutating Pypi => "pypi".
  • note_warning builds its warning with RunWarning::new, and its 9-line doc is cut to 2.
  • Deleted checking_latest_status_line, which compared the const to its own literal, and the sentence_case asserts that ui/text.rs already covers.
• user_patches.rs: partition, evidence and skip_protected each open-coded the same selected-ecosystem + conflicts check. They now share one private protecting().
• vendor_orphans.rs
  • The wired set borrows (&str, &str) keys instead of cloning two strings per candidate.
  • Removed bare issue refs (Vendored rollback and remove leave an unledgered .socket/vendor/npm/<uuid>/ artifact dir behind (e.g. the losing side of a merge), while vendor --revert sweeps it, and nothing afterwards reports it #1610 ×2, Vendored yarn classic: after a git merge resolves .socket/vendor/state.json to one side, vendor --check and rollback exit 0 while the other side's package stays wired, and rollback deletes the ledger, stranding it #991) and trimmed the sweep's doc where it restated the struct.
  • Fixed the still-wired test doc, which claimed the advisory names a lockfile restore. The test now pins the whole advisory with assert_eq! instead of three .contains checks.
• vlt_heal.rs
  • Inlined cleanup_disabled.
  • Deleted the healed_store fallback at the end of heal_after_rewrite. It was dead: healed needs a heal target, targets come only from final_lock, and the bundled-copy block above it sets healed_store whenever final_lock is Some. A debug_assert! now states the invariant.
  • Removed bare refs (vlt hosted and vendored vex attests not_affected while a bundled copy of the same name@version in node_modules/.vlt stays unpatched (the #325 fix covers npm locks only) #471 ×2, Support vlt in hosted, vendored and agent modes #269) and a full std::collections path.

git diff --stat: 5 files, +80 / −116. That is about −27 production and −9 test lines.

Not done from the notes, and why

• update.rs pin_version as semver::Version (which would delete the unreachable Err arm): lib.rs tests compare it as a string, and lib.rs is held by Sweep cli args.rs: inline, dedupe tests, trim docs #1842.
• parse_version_pin → core parse_release_tag: it changes behavior (vv3.4.0 is accepted today).
• user_patches returning RunWarning and refuse reusing lock_cli::error_envelope: the callers are in get.rs, apply.rs and vendor.rs (Fix Go patch delivery, audits, and reinstalls #1698) and in lock_cli.rs (Sweep hosted_unwind, lock_cli, list, commands/mod (+ rollback dead branch) #1864). The cross-file gate loop is T38 (The project-patch (user_patches) gate loop is written five times, and scan's and vendor's copies have drifted #1911).
• vendor_orphans: the sweep_orphan_vendor_dirs forwarder stays, because its callers are in vendor.rs (Fix Go patch delivery, audits, and reinstalls #1698).
• vlt_heal: vlt_vendored_uuids stays as a named helper (see review below). The swallowed load_state error and rollback_heal's .ok() change output, so they are left for an issue. advisory_variants_follow_severity_order is kept: it is the only test of the precedence between stale_left, undeterminable and invalidated, since the optional-copies test sets one at a time.

/code-review high

No correctness findings. Of its 6 low-severity notes, I applied 3: kept vlt_vendored_uuids named, added the debug_assert!, and added the InstallChannel::id table test. I skipped the other 3:

• Status-line strings now untested. The deleted tests only restated a const or a one-line format!.
• Keep the !contains("git checkout") checks next to the exact match. The exact match already pins their absence.
• id as a method vs channel_label as a free fn. This is style; making channel_label a method would touch its other callers.

Verification

• cargo clippy --workspace --all-features -- -D warnings: clean.
• socket-patch-cli --lib (938 tests) and core update:: tests: pass.
• Integration suites self_update_e2e, self_update_failures_e2e, update, user_patches, in_process_vendor, in_process_rollback_vendored, in_process_rollback_hosted, in_process_vendor_bun_takeover, mode_migration_vlt and in_process_remove_repair_lifecycle: pass.
• In in_process_redirect, 149 pass. 3 chmod-based tests fail identically on main in the root sandbox: partial_lockfile_write_failure_exits_1_and_writes_no_ledger, redirect_json_mode_write_failures_emit_error_envelope and vlt::scan_redirect_vlt_heal_invalidation_failure_warns.

Behavior change: none. Output bytes, JSON, exit codes and telemetry values are identical. No cfg(windows)/cfg(unix) code was touched.

Risk: low.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aePD791iNBnbWkcoVVH

---

Note

Low Risk
Refactor-only changes across self-update, patch gating, vendor cleanup, and VLT heal with no intended behavior change.

Overview
Architecture sweep across four CLI command modules (plus core channel telemetry): deduplication and colocation, not feature work.

Self-update (update.rs + core): Telemetry install-channel IDs move from a CLI-local channel_id to InstallChannel::id() in core, with a table test locking stable names. One-off helpers for status strings are inlined at their single call sites; note_warning builds advisories via RunWarning::new. Redundant unit tests that only echoed literals or duplicated sentence_case coverage are removed.

User patch gate (user_patches.rs): protecting() centralizes the “selected ecosystem + project patch conflicts” check used by partition, evidence, and skip_protected.

Vendor orphan sweep (vendor_orphans.rs): Lockfile wiring lookup uses (&str, &str) keys instead of cloning eco/uuid per candidate. Docs lose stale issue references; the still-wired advisory test pins the full message with assert_eq!.

VLT heal (vlt_heal.rs): cleanup_disabled is inlined; a dead healed_store fallback after rewrite heal is dropped in favor of a debug_assert! on the existing invariant. Minor comment/type cleanup.

Intended to be behavior-neutral (same user-visible output, JSON, exit codes, telemetry).

^{Reviewed by Cursor Bugbot for commit 94d90fd. 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 94d90fd. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at 94d90fd.

• CI: 82/82 green on 94d90fd (32 success, 50 skipped); mergeable, no conflicts.
• Bugbot: reviewed 94d90fd, no findings.
• Reviewer note: refactor sweep of update, user_patches, vendor_orphans, vlt_heal; it also adds code to socket-patch-core/src/update/channel.rs (+29). Check that the logic moved out of update.rs behaves the same.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: This is a cleanup refactor with no change in behaviour. It moves channel_id into core as InstallChannel::id(). In user_patches, one protecting() gate replaces three copies of the same check. The wired set in vendor_orphans now borrows strings instead of cloning them. A few one-off helpers and constants in update.rs and vlt_heal.rs are inlined. User-facing strings, telemetry ids, stdout/stderr routing and the JSON output shape are all unchanged.

Risk: low. The only change that affects runtime is in vlt_heal: a release-mode fallback becomes a debug_assert!. The fallback could never run. healed = true is set only when owned is non-empty, which needs final_lock to be Some, and that branch always sets healed_store.

Look here:

• vlt_heal.rs:423: the fallback replaced by debug_assert! (see lines 368 and 394 for the invariant).
• user_patches.rs:38: protecting() and its three callers. All three keep the same order: ecosystem check first, then conflicts.
• channel.rs:530: InstallChannel::id(). It returns the same strings as before and has a new test.
• vendor_orphans.rs:71: wired is now HashSet<(&str, &str)>.
• update.rs:159: note_warning now builds its warning with RunWarning::new.

Verified:

• Read the full diff, looking for semantic drift.
• Core update::channel tests: 25/25 pass.
• CLI unit tests for update, vendor_orphans and vlt_heal pass.
• Clippy reports nothing on the touched files.
• CI: ci-ok and clippy are green on the head.
• Bugbot: success.
• CHANGELOG.md is untouched and there's no debug code left in.

Changes I made: none.

Notes (non-blocking):

• The update.rs tests for the "Checking…" and "Downloading…" status strings were deleted with the helpers they tested, so those strings no longer have a test.
• The orphan test now asserts the exact UNLEDGERED_WIRING_REMEDY text. That is stricter, but any wording change will break it.

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

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 94d90fd136486da46b241269164735253dbdee36. Checked telemetry channel identifiers, protected user patches, orphan retention and vlt heal invariants. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.