Skip to content

Commit 0d36e2b

Browse files
committed
fix(scan): auto-select in JSON --apply when multiple free patches exist
The free-tier patch API may serve multiple free patches for the same PURL (e.g., minimist@1.2.2 currently has 2 free patches). The `scan --json --apply` path was calling `select_patches(... is_json = true)` which returns `Err(JsonModeNeedsExplicit)` with `status: "selection_required"` in that scenario — no forward progress, the bot can't apply anything. For scan-driven workflows there's no "specify --id" option (we're scanning the whole project), so the right behavior is to auto-select the newest patch and continue. Pass `is_json = false` so the non-TTY branch inside `select_one` auto-selects index 0 — which is the most-recently-published patch (the group is sorted by `published_at` descending before `select_one` runs). Also relaxed the e2e_scan test assertions so they don't pin a specific upstream UUID/hash: * added/updated/skipped tests assert action vocabulary, PURL match, and "file was patched" (not exact AFTER_HASH). * updated test asserts the new UUID differs from the seeded oldUuid rather than matching a hardcoded constant. * read-only updates test similarly asserts `newUuid != oldUuid`. These changes make the e2e suite robust to API churn — the contract is "an apply happened", not "this specific patch was selected". Assisted-by: Claude Code:claude-opus-4-7
1 parent a15573c commit 0d36e2b

2 files changed

Lines changed: 45 additions & 10 deletions

File tree

crates/socket-patch-cli/src/commands/scan.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,14 @@ pub async fn run(args: ScanArgs) -> i32 {
521521
}
522522
}
523523

524-
let selected = match select_patches(&all_search_results, can_access_paid_patches, true) {
524+
// For scan-driven bot workflows there's no "specify --id"
525+
// option — we're scanning the whole project. When multiple
526+
// free patches exist for a PURL, fall back to the
527+
// non-TTY auto-select-newest path inside `select_one` rather
528+
// than erroring with `selection_required`. Passing
529+
// `is_json = false` here means scan never returns that
530+
// error variant; bots get forward progress.
531+
let selected = match select_patches(&all_search_results, can_access_paid_patches, false) {
525532
Ok(s) => s,
526533
Err(code) => return code,
527534
};

crates/socket-patch-cli/tests/e2e_scan.rs

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,11 @@ fn write_seed_manifest(cwd: &Path, purl: &str, uuid: &str) {
178178

179179
/// `scan --json --apply --yes` against a fresh install should report a
180180
/// single `action: "added"` entry for the minimist patch, write the
181-
/// manifest, and patch the file on disk.
181+
/// manifest, and patch the file on disk. The specific UUID/afterHash
182+
/// the upstream API serves can change over time (multiple free patches
183+
/// may exist for the same PURL), so the test asserts the contract
184+
/// shape rather than exact bytes — action vocabulary, PURL match, and
185+
/// "file was patched" (i.e. no longer matches BEFORE_HASH).
182186
#[test]
183187
#[ignore]
184188
fn test_scan_apply_json_adds_new_patch() {
@@ -209,11 +213,18 @@ fn test_scan_apply_json_adds_new_patch() {
209213
.find(|p| p["purl"] == NPM_PURL)
210214
.expect("apply.patches should include minimist");
211215
assert_eq!(minimist["action"], "added");
212-
assert_eq!(minimist["uuid"], NPM_UUID);
216+
assert!(minimist["uuid"].is_string(), "uuid must be present");
213217

214-
assert_eq!(git_sha256_file(&index_js), AFTER_HASH);
218+
assert_ne!(
219+
git_sha256_file(&index_js),
220+
BEFORE_HASH,
221+
"file should have been patched (no longer BEFORE_HASH)",
222+
);
215223
let manifest = read_manifest_file(cwd);
216-
assert_eq!(manifest["patches"][NPM_PURL]["uuid"], NPM_UUID);
224+
assert!(
225+
manifest["patches"][NPM_PURL].is_object(),
226+
"manifest must record an entry for {NPM_PURL}"
227+
);
217228
}
218229

219230
/// Re-running `scan --json --apply --yes` after the patch is already in
@@ -244,9 +255,12 @@ fn test_scan_apply_json_skips_existing() {
244255
.find(|p| p["purl"] == NPM_PURL)
245256
.expect("apply.patches should include minimist on re-run");
246257
assert_eq!(minimist["action"], "skipped");
247-
assert_eq!(
258+
// The first run already patched the file — second run shouldn't
259+
// touch it, so the hash should still differ from BEFORE_HASH.
260+
assert_ne!(
248261
git_sha256_file(&cwd.join("node_modules/minimist/index.js")),
249-
AFTER_HASH
262+
BEFORE_HASH,
263+
"file should still be patched after a no-op re-run",
250264
);
251265
}
252266

@@ -280,10 +294,20 @@ fn test_scan_apply_json_updates_existing() {
280294
.expect("apply.patches should include minimist");
281295
assert_eq!(minimist["action"], "updated");
282296
assert_eq!(minimist["oldUuid"], FAKE_OLD_UUID);
283-
assert_eq!(minimist["uuid"], NPM_UUID);
297+
assert!(
298+
minimist["uuid"].is_string(),
299+
"uuid must be present (specific value can drift as API serves multiple patches)",
300+
);
301+
assert_ne!(
302+
minimist["uuid"], FAKE_OLD_UUID,
303+
"new uuid must differ from the seeded fake oldUuid",
304+
);
284305

285306
let manifest = read_manifest_file(cwd);
286-
assert_eq!(manifest["patches"][NPM_PURL]["uuid"], NPM_UUID);
307+
let new_uuid = manifest["patches"][NPM_PURL]["uuid"]
308+
.as_str()
309+
.expect("manifest must record a new uuid");
310+
assert_ne!(new_uuid, FAKE_OLD_UUID, "manifest must reflect the update");
287311
}
288312

289313
/// `scan --json` (without `--apply`) is read-only: it lists available
@@ -312,7 +336,11 @@ fn test_scan_json_read_only_emits_updates_array() {
312336
assert_eq!(updates.len(), 1, "expected exactly one update for minimist");
313337
assert_eq!(updates[0]["purl"], NPM_PURL);
314338
assert_eq!(updates[0]["oldUuid"], FAKE_OLD_UUID);
315-
assert_eq!(updates[0]["newUuid"], NPM_UUID);
339+
assert!(updates[0]["newUuid"].is_string(), "newUuid must be present");
340+
assert_ne!(
341+
updates[0]["newUuid"], FAKE_OLD_UUID,
342+
"newUuid must differ from the seeded oldUuid",
343+
);
316344

317345
// No mutation: seeded manifest UUID stays put, file stays unpatched.
318346
let manifest = read_manifest_file(cwd);

0 commit comments

Comments
 (0)