[Mikola Lysenko (mikolalysenko)]

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

Sweep of crates/socket-patch-cli/src/commands/scan/hosted.rs, from its coverage row in the architecture ledger (discussion #1830). It covers everything except the 1,233-line run_redirect_selected_untracked, which is T24's structural work (part of #894), so the row is recorded as partial.

Changes

• Tests moved to the module they test. 13 tests of core hosted::guidance sat in the CLI behind a #[cfg(test)] pub(crate) use re-export of 17 core items. They now live in hosted/guidance.rs, and the re-export is deleted. The two pnpm lock-sniff tests went beside their functions in formats/pnpm/mod.rs and were renamed for those functions.
• Inlined emit_json_error into run_redirect, its only caller. scan calls run_redirect only inside if args.common.json, so run_redirect now takes the Envelope itself. The unreachable human "No changes made." arm is deleted.
• Deleted the test-only wrapper installed_stale_positive_evidence. Its test calls core judge_installed_record(..).stale_evidence through a local helper. Renamed to stale_evidence_requires_readable_mismatched_bytes.
• Flattened the gem probe's vendor/cache fold (pass 2): one if … else None, and cache_covered is keyed on the judged purl instead of looked up again in candidates.
  • Red→green: dropping the insert fails gem_stale_probe_folds_committed_vendor_cache_into_the_remedy and gem_stale_probe_follows_the_configured_bundle_cache_path.
• Deleted the duplicate test gem_stale_probe_record_lookup_is_uuid_keyed. It had the same fixture and assertion as gem_stale_probe_matches_records_by_uuid_not_purl_key.
• Fixed one test that claimed more than it checked. pnpm_lock_carries_hosted_redirect_detects_prior_run_splices now asserts the \/-escaped spelling its doc names.
  • Red→green: making artifact_url_spellings return the raw URL twice fails it.
• About 120 comment lines removed: the human-output banner, 2 test banners, oversized private docs (run_redirect_selected, gem_stale_install_warnings, acquire_hosted_lock, run_redirect's parameters) and bare issue references. The why comments stay.
• Moved the SBT_OWNED_FILE_UNREADABLE import to the top of the file.

Size

git diff --stat: 4 files, +696 / −847.

• Production: scan/hosted.rs +84 / −231, scan/mod.rs ±1.
• Tests: scan/hosted.rs +20 / −615, core guidance.rs +543, core formats/pnpm/mod.rs +48. Most of that is the move.

Behavior change

None. The deleted arm could not run.

Risk

Low. The production edits are an inline, a dead branch and a loop flattening covered by the gem probe tests. No cfg(windows) or cfg(unix) code changed.

Verification

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --lib (guidance, formats::pnpm): pass. cargo test -p socket-patch-cli --lib scan::hosted: 48 pass.
• cargo test -p socket-patch-cli --test scan --test in_process_redirect: 149 pass, 3 fail. The 3 failures are chmod-based and fail only because the sandbox runs as root; they pass in CI: partial_lockfile_write_failure_exits_1_and_writes_no_ledger, redirect_json_mode_write_failures_emit_error_envelope, vlt::scan_redirect_vlt_heal_invalidation_failure_warns.
• /code-review high found no bugs in the diff.

Review findings not fixed here

All of these predate this PR or are outside a sweep.

• Gem probe pass 3 matches by purl string. Two spellings of one gem sharing an installed dir can warn about the same vendor/cache archive twice.
• Pass 3 builds the cache name without the platform suffix. It uses {name}-{version}.gem, so a committed stale platform-gem archive with no installed dir is missed. Both gem probe findings change behavior, so they want an issue.
• Path::is_file is a blocking stat inside the async probe. It predates this PR.
• The npm_override test fixture now has three copies (the CLI, guidance.rs, redirect/mod.rs). A shared core test builder belongs with A37/Share one scoped env-var test guard: EnvGuard is copy-pasted 11 times across core unit and integration tests #1889-style test-support work.
• run_redirect_selected still takes Option<Envelope> keyed on common.json. Changing it touches get.rs (held by Fix Go patch delivery, audits, and reinstalls #1698) and the T24 body.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeR2NHGsMnustoUpVWS

---

Note

Low Risk
Production edits are refactors and comment removal in hosted scan/redirect paths; behavior is unchanged aside from removing unreachable human-output branches, with existing gem probe tests covering the loop change.

Overview
This PR cleans up scan/hosted.rs without changing hosted redirect behavior: large comment/doc trims, removal of the emit_json_error helper (inlined into JSON-only run_redirect, which now takes an Envelope directly), and a small flatten of the gem stale-install probe’s vendor/cache fold (cache_covered keyed by judged purl).

Tests move next to the code they exercise: pnpm/npm trust and heal-on-rerun coverage leaves the CLI re-export shim and lands in socket-patch-core (hosted/guidance.rs, formats/pnpm/mod.rs); the CLI keeps integration-style hosted tests. Duplicates drop (installed_stale_positive_evidence wrapper, redundant uuid lookup test), and pnpm_lock_carries_hosted_redirect now asserts \/-escaped URLs as documented.

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

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent) at 8a0af7a.

• CI: 32/32 run checks green (50 skipped). Mergeable, no conflicts.
• Bugbot: reviewed 8a0af7a, no findings; no unresolved threads.
• Reviewer note: refactor sweep of scan/hosted.rs helpers and tests; it also touches formats/pnpm/mod.rs and hosted/guidance.rs.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: This is a behavior-preserving sweep of scan/hosted.rs. It moves 15 tests of core hosted::guidance and the pnpm lock-sniff helpers into core and drops the CLI's #[cfg(test)] re-export block. It also inlines emit_json_error into the JSON-only run_redirect, which now takes an Envelope instead of an Option. Finally, it flattens the gem vendor/cache fold, removes a duplicate test and trims about 120 comment lines.

Risk: low.

• The removed human-output arm of run_redirect was unreachable: it is only called under if args.common.json, with a value that was always Some.
• The flattened gem fold gives the same contains results.
• Every moved test keeps all its assertions; I compared the removed and added test lines as sorted sets.
• The deleted gem test was a duplicate of gem_stale_probe_matches_records_by_uuid_not_purl_key.

Look here

• hosted.rs:494: the new run_redirect signature
• hosted.rs:538: the inlined JSON error emit
• hosted.rs:398: the flattened vendor/cache fold
• guidance.rs:1022: the new assertion for the \/-escaped URL spelling

Verified: I read the full diff and checked the moved tests one by one. There is no unrelated edit, and CHANGELOG.md is untouched. CI is 82/82 green, and Bugbot passed.

Changes I made: none.

Open questions (non-blocking): a few trimmed doc comments carried useful detail and could each get one line back:

• the acquire_hosted_lock guard unlinks the lock and prunes an empty .socket/, and maps failures to the lock_held/lock_io codes;
• the candidate filter is skipped on --dry-run;
• a --json caller of run_redirect_selected that passes None gets a bare envelope.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] On the three non-blocking doc questions in the final review: I'm leaving them as they are, to avoid a push that would rerun CI and reset the review.

• acquire_hosted_lock: the lock guard's unlink-and-prune behavior and the lock_held/lock_io mapping are documented on lock_cli, the helpers it calls. T09 (Acquire apply.lock through one lock_cli helper that takes &GlobalArgs; the sequence is open-coded 9 times and renders lock_held 3 ways #1858) will fold this function into the shared lock helper anyway.
• --dry-run: the dry-run skip of the gem probe is decided and commented where the probe is called. The candidate filter itself doesn't depend on dry-run.
• Some with --json: run_redirect_selected's doc still says scan_result must be Some exactly when common.json is set.

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 8a0af7aca2a522775bd59f44ef23321474b54fed. Checked hosted JSON-only caller contract, stale gem judgment/caches and equivalent event/error behavior. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.