Repository navigation
Sweep scan/hosted.rs (helpers and tests region) - #1897
Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
The pnpm trust, npm allow-remote and pnpm lock-sniff tests checked core functions through a cfg(test) re-export in the CLI. They now sit beside the functions they test, in hosted/guidance.rs and formats/pnpm/mod.rs, and the re-export is gone. Assisted-by: Claude Code:claude-opus-5-5
- Inline emit_json_error into run_redirect, its only caller. scan calls run_redirect only under --json, so it now takes the Envelope itself, and the human 'No changes made.' arm (unreachable) is gone. - Delete the test-only installed_stale_positive_evidence wrapper; its test calls core judge_installed_record directly. - Flatten the gem probe's vendor/cache fold and key cache_covered on the judged purl instead of looking it up again in the candidates. - Move the SBT_OWNED_FILE_UNREADABLE import to the top. - Delete the duplicate uuid-keyed gem probe test, two test banners and the human-output banner, and trim oversized private docs and bare issue references. Assisted-by: Claude Code:claude-opus-5-5
The test's doc says a \/-escaped hosted URL counts as a prior splice, but it only checked the raw and percent-encoded spellings. It now checks the escaped one too, and fails if artifact_url_spellings drops it. Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 30, 2026 09:31
Collaborator
Author
|
BugBot review Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
pushed a commit
that referenced
this pull request
Oct 30, 2026
Assisted-by: Claude Code:claude-opus-5-5
There was a problem hiding this comment.
✅ 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)
pushed a commit
that referenced
this pull request
Oct 30, 2026
Assisted-by: Claude Code:claude-opus-5-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-linerun_redirect_selected_untracked, which is T24's structural work (part of #894), so the row is recorded as partial.Changes
hosted::guidancesat in the CLI behind a#[cfg(test)] pub(crate) usere-export of 17 core items. They now live inhosted/guidance.rs, and the re-export is deleted. The two pnpm lock-sniff tests went beside their functions informats/pnpm/mod.rsand were renamed for those functions.emit_json_errorintorun_redirect, its only caller.scancallsrun_redirectonly insideif args.common.json, sorun_redirectnow takes theEnvelopeitself. The unreachable human "No changes made." arm is deleted.installed_stale_positive_evidence. Its test calls corejudge_installed_record(..).stale_evidencethrough a local helper. Renamed tostale_evidence_requires_readable_mismatched_bytes.if … else None, andcache_coveredis keyed on the judged purl instead of looked up again incandidates.gem_stale_probe_folds_committed_vendor_cache_into_the_remedyandgem_stale_probe_follows_the_configured_bundle_cache_path.gem_stale_probe_record_lookup_is_uuid_keyed. It had the same fixture and assertion asgem_stale_probe_matches_records_by_uuid_not_purl_key.pnpm_lock_carries_hosted_redirect_detects_prior_run_splicesnow asserts the\/-escaped spelling its doc names.artifact_url_spellingsreturn the raw URL twice fails it.run_redirect_selected,gem_stale_install_warnings,acquire_hosted_lock,run_redirect's parameters) and bare issue references. The why comments stay.SBT_OWNED_FILE_UNREADABLEimport to the top of the file.Size
git diff --stat: 4 files, +696 / −847.scan/hosted.rs+84 / −231,scan/mod.rs±1.scan/hosted.rs+20 / −615, coreguidance.rs+543, coreformats/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)orcfg(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 highfound no bugs in the diff.Review findings not fixed here
All of these predate this PR or are outside a sweep.
vendor/cachearchive twice.{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_fileis a blocking stat inside the async probe. It predates this PR.npm_overridetest 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_selectedstill takesOption<Envelope>keyed oncommon.json. Changing it touchesget.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.rswithout changing hosted redirect behavior: large comment/doc trims, removal of theemit_json_errorhelper (inlined into JSON-onlyrun_redirect, which now takes anEnvelopedirectly), and a small flatten of the gem stale-install probe’s vendor/cache fold (cache_coveredkeyed 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_evidencewrapper, redundant uuid lookup test), andpnpm_lock_carries_hosted_redirectnow asserts\/-escaped URLs as documented.Reviewed by Cursor Bugbot for commit 8a0af7a. Configure here.
Generated by Claude Code