fix(get,scan): retain patch-added new files; guard empty patch records

[Mikola Lysenko (mikolalysenko)]

Problem

fn files_with_both_hashes(patch) in crates/socket-patch-cli/src/commands/get.rs kept only patch files carrying BOTH beforeHash+afterHash, silently dropping every net-new file a patch ADDS (a file with afterHash but no beforeHash). It backed the scan/download/vendor record builders, while the by-uuid apply path save_and_apply_patch already TOLERATED new files via an empty-beforeHash sentinel.

So get <uuid> patched correctly, but scan / apply / vendor dropped added files:

• P0 (real prod): the only free cargo patch, cf2e6f58 for pkg:cargo/traitobject@0.1.1, is a whole-crate export where ALL 9 files lack beforeHash. The manifest recorded files:{}, yet scan --mode vendored, apply, and vendor all printed applied:1 / success while writing NOTHING — telling the user to commit a .socket/vendor/ directory that never got created.
• P1 (real prod): gem patch 2535d43d for activestorage@7.0.2.2 ships a genuinely-new lib/rubygems_plugin.rb (a CVE-2022-21831 runtime guard) with no beforeHash → dropped by agent-mode and --vendor-source build, silently omitting a protection file the patch author intended.

Repro (traitobject): fresh cargo new with traitobject="=0.1.1", isolated CARGO_HOME, SOCKET_NO_CONFIG=true, scan --json --yes --mode vendored → observed applied:1 with nothing written; expected either the files recorded/vendored, or a loud failure.

Fix

Two parts, both in crates/socket-patch-cli/src/commands/get.rs:

1. Retain new-file entries. New shared helper files_for_manifest (get.rs:384) keeps every file with an afterHash, recording missing beforeHash as the empty-string sentinel — the exact convention save_and_apply_patch already relied on. All record builders now use it: record_from_patch_response (:405), download_patch_records (:938), download_and_apply_patches (:1226), and save_and_apply_patch's inline loop (:1928, refactored to the shared helper). files_with_both_hashes (:353) is retained ONLY for installed-variant matching in filter_to_installed_releases (:800), where new files are non-discriminating anyway (select_installed_variants filters on a non-empty beforeHash).

2. Guardrail. If a built record's files map ends up EMPTY, it is now surfaced as a failure instead of a success: download_and_apply_patches and download_patch_records push a failed action and increment the failed count (degrading status/exit via run_outcome), and save_and_apply_patch reports a loud error and returns exit 1. A defective all-files-dropped patch can never again be counted as applied:1.

Test

Three hermetic unit tests in get.rs's #[cfg(test)] module:

• files_for_manifest_retains_new_file_without_before_hash — a patch adding lib/rubygems_plugin.rb (afterHash, no beforeHash) is retained with an empty-beforeHash sentinel; the strict helper still drops it.
• files_for_manifest_keeps_all_new_file_whole_crate_export — the P0 cargo case: 9 whole-crate-export files all retained (old rule produced files:{}).
• build_patch_record_from_new_files_is_not_empty — record_from_patch_response retains new files (RED before the fix), and a no-afterHash patch yields the empty map the guardrail rejects.

Verified RED→GREEN: reverting record_from_patch_response to files_with_both_hashes makes build_patch_record_from_new_files_is_not_empty fail; with the fix all 56 commands::get::tests pass. cargo build -p socket-patch-cli is clean.

Scope

Focused on the CLI record-builder + guardrail. Does not touch the core apply engine (already new-file-tolerant) or blob download. The other half of the gem-new-file finding (service-side export) is out of scope. Kills sweep findings: cargo-scan-apply-vendor-silent-noop, scan-vendored-false-applied-nothing-written, download-flow-drops-new-files, gem-new-file-dropped-build-weaker-than-service (CLI-drop half), and the K4 axios "works via get, breaks via scan" class.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes how scan/apply/vendor build manifest records and exit/status for defective patches—user-visible behavior improves but wrong patches now fail instead of silent no-ops; core apply engine unchanged.

Overview
Aligns scan, apply, and vendor with get <uuid> by introducing shared files_for_manifest, which records every file with an afterHash—including net-new files that only have an afterHash—using an empty-string beforeHash sentinel. The previous files_with_both_hashes rule dropped those files, so whole-crate cargo exports and gem runtime-guard additions could end up with files:{} while JSON still reported applied:1 and nothing was written.

files_with_both_hashes is only used for installed-release variant matching in filter_to_installed_releases; all manifest record paths now call files_for_manifest (record_from_patch_response, download/apply, vendor download, and save_and_apply_patch).

Guardrail: if a patch yields no recordable files, flows treat it as failed (not skipped/applied)—vendor download, download_and_apply_patches, and save_and_apply_patch error out or increment failure counts so empty patches cannot masquerade as successful protection.

Unit tests cover new-file retention, whole-crate export (9 files), and non-empty records from record_from_patch_response.

^{Reviewed by Cursor Bugbot for commit 32bb21d. Configure here.}