Skip to content

Commit 5af7a1e

Browse files
committed
Make scan exit alike in human and JSON output
A scan whose detail queries all succeeded but returned no records exited 1 in human output ("could not fetch patch details") and 0 with --json, so CI and a developer saw different results for the same API state. Human --dry-run --prune also never previewed the GC that the --json arm reports, and hosted --json passed the raw --prune flag where the human arm used the policy-gated one. Only a real fetch failure (every query errored) now fails either arm; the human dry run previews the GC; hosted mode gets the gated prune in both arms (#1062). Assisted-by: Claude Code:claude-opus-5-5
1 parent 7c2eec7 commit 5af7a1e

4 files changed

Lines changed: 47 additions & 29 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,9 @@ pub(super) async fn run_redirect(
554554
stage: &mut super::rollout::Stage,
555555
// Scan's pre-redirect lockfile discovery (see `rollout::Gate::prior`).
556556
prior: Option<super::rollout::Prior<'_>>,
557+
// `--prune` / `--sync`, gated by the policy exactly as the human arm
558+
// gates it (`patches.enabled: false` writes nothing, the GC included).
559+
prune: bool,
557560
) -> i32 {
558561
// Same discovery/selection as agent and vendored mode.
559562
let discovered = match discover_selected(
@@ -606,7 +609,7 @@ pub(super) async fn run_redirect(
606609
run_redirect_selected(
607610
&args.common,
608611
&args.vex,
609-
args.prune || args.sync,
612+
prune,
610613
api_client,
611614
&pairs,
612615
scan_result,

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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,6 @@ async fn discover_selected(
489489
// Some queries failed, some succeeded: a `--json` run has no stderr
490490
// warning (`warn` is human-only), so each failed package becomes a
491491
// run-level `warnings[]` entry — never a silent drop from the envelope.
492-
let fetched = all_search_results.len();
493492
let offers = select_accessible(all_search_results, can_access_paid_patches, policy);
494493
if let Some(result) = json_warnings {
495494
for (purl, e) in &failures {
@@ -503,18 +502,15 @@ async fn discover_selected(
503502
}
504503
Ok(Discovered {
505504
offers,
506-
fetched,
507505
failed: failures,
508506
})
509507
}
510508

511-
/// [`discover_selected`]'s result: the offers, how many records came back
512-
/// (before the tier filter), and each failed detail query as `(purl,
513-
/// error)` (a failure for a package with no recorded patch makes a capped
514-
/// run's data incomplete).
509+
/// [`discover_selected`]'s result: the offers and each failed detail
510+
/// query as `(purl, error)` (a failure for a package with no recorded
511+
/// patch makes a capped run's data incomplete).
515512
struct Discovered {
516513
offers: rollout::Offers,
517-
fetched: usize,
518514
failed: Vec<(String, String)>,
519515
}
520516

@@ -2544,6 +2540,7 @@ async fn run_scan(
25442540
batch_error_count > 0,
25452541
&mut stage,
25462542
prior_discovery,
2543+
prune,
25472544
)
25482545
.await;
25492546
}
@@ -2843,10 +2840,11 @@ async fn run_scan(
28432840

28442841
// The by-package records every arm selects from, fetched before the
28452842
// table so its `[UPDATE]` markers are the same UPGRADE rows the
2846-
// selection acts on (§5.1). Discovery said these packages HAVE
2847-
// patches, so an empty merged set is a fetch failure.
2848-
// A failed discovery still prints the table first; its exit code is
2849-
// returned below it.
2843+
// selection acts on (§5.1). Only `discover_selected`'s own `Err`
2844+
// (every query failed) is a fetch failure: queries that succeed with
2845+
// no records leave nothing to select, in every arm and in `--json`
2846+
// alike (#1062). A failed discovery still prints the table first; its
2847+
// exit code is returned below it.
28502848
let mut discovery_failure: Option<i32> = None;
28512849
let rows: Vec<rollout::Row> = if downloadable_count == 0 {
28522850
Vec::new()
@@ -2864,13 +2862,6 @@ async fn run_scan(
28642862
)
28652863
.await
28662864
{
2867-
// The agent / vendored / report-only arms need records to show:
2868-
// an empty merged set is a fetch failure there.
2869-
Ok(discovered) if !hosted && discovered.fetched == 0 => {
2870-
eprintln!("{}", render::fetch_details_failed(&discovered.failed));
2871-
discovery_failure = Some(1);
2872-
Vec::new()
2873-
}
28742865
Ok(discovered) => {
28752866
let rows = classified_rows(
28762867
&mut stage,
@@ -3228,6 +3219,19 @@ async fn run_scan(
32283219
}
32293220
}
32303221
print_rollout_human(&stage, true, silent);
3222+
// `finish_human` previews the agent / report-only GC; vendored
3223+
// runs its own GC, so its dry run previews it here, as the
3224+
// `--json` arm reports it in `gc`.
3225+
if prune && vendor {
3226+
gc::run_human_gc(
3227+
&args.common,
3228+
&manifest_path,
3229+
&socket_dir,
3230+
&scanned_purls,
3231+
&vendored_purls,
3232+
)
3233+
.await;
3234+
}
32313235
return finish_human(0).await;
32323236
}
32333237

‎crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,13 +2852,18 @@ async fn scan_empty_detail_results_exit_alike_in_human_and_json() {
28522852
}
28532853
}
28542854

2855-
/// #1062: a human `--dry-run --prune` previews the GC the `--json` arm
2856-
/// previews (`gc` block), instead of skipping it.
2855+
/// #1062: a human `--dry-run --prune` previews the GC (once) in every
2856+
/// mode the `--json` arm previews it (`gc` block); vendored skipped it.
28572857
#[tokio::test]
28582858
async fn scan_dry_run_prune_previews_gc_in_human_and_json() {
28592859
let purl = "pkg:npm/minimist@1.2.2";
28602860
let stale = "pkg:npm/left-pad@1.3.0";
2861-
for mode in [&["--mode", "agent"][..], &["--prune"][..]] {
2861+
// Agent, vendored, and report-only (no mode; `--prune` below).
2862+
for mode in [
2863+
&["--mode", "agent"][..],
2864+
&["--mode", "vendored"][..],
2865+
&[][..],
2866+
] {
28622867
let mock = MockServer::start().await;
28632868
mount_batch_one(&mock, purl, UUID, "free", &[], false).await;
28642869
mount_by_package(&mock, purl, UUID, serde_json::json!({})).await;
@@ -2887,9 +2892,12 @@ async fn scan_dry_run_prune_previews_gc_in_human_and_json() {
28872892
"{extra:?}: {v}"
28882893
);
28892894
} else {
2890-
assert!(
2891-
stdout.contains("[dry-run] GC would prune 1 manifest entry"),
2892-
"{extra:?}: the human dry run previews the GC; stdout={stdout}"
2895+
assert_eq!(
2896+
stdout
2897+
.matches("[dry-run] GC would prune 1 manifest entry")
2898+
.count(),
2899+
1,
2900+
"{extra:?}: the human dry run previews the GC once; stdout={stdout}"
28932901
);
28942902
}
28952903
assert_eq!(

‎crates/socket-patch-cli/tests/in_process_scan.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -930,10 +930,13 @@ async fn scan_non_json_with_patches_prints_table() {
930930

931931
let code = run_scrubbed(args).await;
932932
// Non-JSON path: discovery → batch query → render table → fetch
933-
// per-package details. We only mount the batch mock, so detail-fetch
934-
// 404s and scan exits 1 ("Error: could not fetch patch details"). That exit is
935-
// deterministic given these mocks.
936-
assert_eq!(code, 1, "missing detail mock → detail fetch fails → exit 1");
933+
// per-package details. We only mount the batch mock, so the detail
934+
// query 404s, which the client reads as "no records": nothing to
935+
// select, a clean exit 0, exactly like the `--json` arm (#1062).
936+
assert_eq!(
937+
code, 0,
938+
"an empty detail result is no patch to apply, not a failure"
939+
);
937940
// Prove the table-rendering path actually ran against real discovered
938941
// data: the batch endpoint was queried with the package, and the path
939942
// proceeded to the per-package detail fetch (i.e. it had a row to print).

0 commit comments

Comments
 (0)