[Mikola Lysenko (mikolalysenko)]

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

This PR sweeps crates/socket-patch-cli/src/commands/scan/discovery.rs. The file's coverage row was audited at c8e26f6. The work is for the architecture refactor routine (discussion #1830, Part 4 "structure"). It also folds in T18 because the fix stays inside this file.

Fixes #1859

Changes

• Re-export shims deleted (2):
  • discovery::unsupported_layout_warnings is gone. scan/mod.rs and get.rs now import it from socket_patch_core::vendor::lock_inventory.
  • discovery::merge_ledger_records_for_updates is gone. scan/mod.rs now imports it from socket_patch_core::ledgers.
• Wrappers inlined (2):
  • candidate_supersedes → ranking::batch_supersedes.
  • severity_order(&str) → core ranking::severity_order(Some(s)), at its 4 call sites in scan/mod.rs and render.rs.
• Duplicate removed (scan's nested-project remedy quotes paths POSIX-style on Windows: discovery.rs keeps its own drifted shell_word beside ui::shell_word #1859): the private shell_word is deleted. The nested_project_skipped remedy now quotes through crate::ui::shell_word.
• Comments: about 20 lines removed (restating doc comments on UpdateInfo, detect_updates and collect_vuln_ids, the stale "severity helpers" in the module doc, and 8 test banners). The mod.rs comment over the discovery re-exports now names what they export.
• Tests:
  • Deleted (5): the severity_order tests (critical/case/known/moderate/unknown). They tested a forwarding wrapper, and core's severity_ladder_is_* and moderate_is_the_medium_tier already cover the same cases.
  • Moved to core api/ranking.rs (4 → 3): the candidate_supersedes tests, rewritten onto core's batch/batch_multi helpers as batch_supersedes_on_advisory_count_at_equal_severity, batch_supersedes_on_severity_before_advisory_count and batch_supersedes_ignores_cve_aliases_beside_a_ghsa.
  • Moved to core lock_inventory/tests.rs: the unsupported_layout_warnings test. It now asserts all three arms (both known codes renamed and rephrased, an unknown code forwarded verbatim). Red→green: forwarding an empty detail for unknown codes fails it.
  • Moved to ui/text.rs: the shell_word test, which ui::shell_word had no test of its own for. It now pins % and the Windows forms too. Red→green: adding % to the plain set fails it.
  • collect_vuln_ids_dedups_across_patches moved next to its siblings, rebuilt on patch_with, and now also asserts all.
  • The single-use info_with_advisories helper is deleted; its last test builds on patch_with.

Size

git diff --stat: 7 files, +127 / −283.

• Production: +25 / −63 (net −38).
• Tests: +102 / −220 (net −118).

Behavior change

Only the nested_project_skipped remedy text changes, as #1859 asks:

• POSIX: a PATH containing % is now single-quoted.
• Windows: the PATH is double-quoted instead of single-quoted.

The PATH is already /-normalized, so backslash handling does not come up. Every other output, JSON field, exit code and error code is unchanged.

Review notes (/code-review high)

Fixed: the stale re-export comment and the leftover info_with_advisories helper.

Not fixed here (they belong to ui::shell_word, which every other pasteable command already uses, and which this PR does not change):

• cmd still expands %VAR% inside double quotes.
• An embedded " is not escaped on Windows.

The remedy now matches the rest of the CLI, and fixing those for every caller is a separate change. The per-comparison lowercase allocation in core severity_order existed before this PR.

Verification

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --lib -- api::ranking vendor::lock_inventory::tests: 142 passed.
• cargo test -p socket-patch-cli --lib --bins -- scan:: ui::text: 280 passed.
• cargo test -p socket-patch-cli --test scan -- nested_project: 7 passed. The remedy parser in scan_nested_project_e2e still parses the remedy.
• CI: 82 checks green.
• Bugbot: no issues.

The Windows branch of the moved shell_word test only runs on Windows CI.

Risk

Low. Mostly deletions and import moves.

get.rs changes by one line (L911). #1698 also touches get.rs, but only at L3540, so the two should merge without conflict.

https://claude.ai/code/session_011f9aeQJdZ8UcVnhg3sXBcn

---

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 ff4b1ae. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review at ff4b1aed.

• CI: 82/82 check runs green on this head; mergeable with no conflicts; no CHANGELOG.md change.
• Bugbot: reviewed ff4b1ae, no issues found; no unresolved review threads.
• Reviewer focus: behavior-preserving sweep of scan/discovery.rs plus touches in get.rs, scan/render.rs, api/ranking.rs.
• Slack announcement not sent this run (Slack send tool unavailable); next run should retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (head ff4b1ae)

What it does. This is a cleanup of scan/discovery.rs. It deletes two re-export shims, two single-caller wrappers and a private duplicate of shell_word, so callers now use the core and ui functions directly. The tests move next to the code they cover. JSON and table output do not change.

Risk: low. The code changes are mechanical removals of forwarding functions. The one visible change is how the nested-project remedy command quotes its path, which now goes through the shared ui::shell_word.

Look here

• scan/discovery.rs:678: the remedy quoting now uses ui::shell_word
• scan/discovery.rs:484: the supersede check, inlined to ranking::batch_supersedes
• scan/render.rs:376: calls core severity_order directly, with the same ordering and tie-break
• ui/text.rs:184: shell_word and its new test
• api/ranking.rs:988: the moved ranking tests

Verified

• Every removed item is either an exact forwarder or the same as its core equivalent. severity_order(&str) matches core severity_order(Some(s)), and all 4 call sites keep the same min_by_key order and label tie-break.
• The moved tests match the old cases one for one. The unsupported_layout_warnings test now covers all three arms.
• cargo test -p socket-patch-cli --lib -- scan::discovery ui::text: 65 passed.
• CI is green on the head (ci-ok, clippy). There are no unresolved threads. CHANGELOG.md is untouched.

Changes I made: none.

Open questions (non-blocking)

• On Windows, shell_word wraps a path in double quotes, and PowerShell expands $ inside double quotes. A nested project under a directory with $ in its name (for example C:/work/$tmp/...) used to get a literal single-quoted remedy; now the pasted command would point somewhere else. Every other CLI caller of shell_word already behaves this way, so this is a follow-up for shell_word, not this PR.

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

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed ff4b1aed3d00f364160b22b15d987be46278dcf2. Checked scan discovery helper consolidation, severity ordering and layout-warning mappings. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.