Skip to content

Commit 4da7bd1

Browse files
committed
Repair downloads created files' blobs
A default (diff-mode) `repair` downloaded only diff archives, but a diff has no delta for a file the patch creates. After such a repair `apply --offline` still could not apply a patch that creates files. In diff mode, repair now also downloads the blobs of created files (and lists them under `--offline` and `--dry-run`), reported as their own blob download. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
1 parent 07996b5 commit 4da7bd1

2 files changed

Lines changed: 178 additions & 78 deletions

File tree

‎crates/socket-patch-cli/src/commands/repair.rs‎

Lines changed: 157 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::path::Path;
1414
use std::time::Duration;
1515

1616
use crate::args::{apply_env_toggles, parse_bool_flag, GlobalArgs};
17+
use crate::commands::fetch_stage::files_diffs_cannot_cover;
1718
use crate::commands::lock_cli::{acquire_or_emit, error_envelope};
1819
use crate::commands::rollback::{sweep_failure, sweep_unused_artifacts};
1920
use crate::json_envelope::{Command, Envelope, PatchAction, PatchEvent, Status};
@@ -341,6 +342,100 @@ fn format_final_line(
341342
}
342343
}
343344

345+
/// The `.socket/` source directories a download pass writes into.
346+
struct SourcePaths<'a> {
347+
blobs: &'a Path,
348+
packages: &'a Path,
349+
diffs: &'a Path,
350+
}
351+
352+
/// What one download pass did: how many artifacts were missing, and how
353+
/// many of them it downloaded or failed to.
354+
#[derive(Default)]
355+
struct DownloadPass {
356+
missing: usize,
357+
downloaded: usize,
358+
failed: usize,
359+
}
360+
361+
/// Step 1's pass over `missing` (non-empty), the `mode` artifacts `m`
362+
/// references: the `--offline` warning, the `--dry-run` preview, or the
363+
/// download and its result lines.
364+
async fn download_pass(
365+
args: &RepairArgs,
366+
client: &mut Option<ApiClient>,
367+
m: &socket_patch_core::manifest::schema::PatchManifest,
368+
missing: &[String],
369+
mode: DownloadMode,
370+
paths: &SourcePaths<'_>,
371+
) -> DownloadPass {
372+
let quiet = args.common.json || args.common.silent;
373+
let noun = mode.noun();
374+
let mut pass = DownloadPass {
375+
missing: missing.len(),
376+
..DownloadPass::default()
377+
};
378+
if args.common.offline {
379+
if !quiet {
380+
eprintln!("{}", format_offline_warning(missing, noun));
381+
}
382+
return pass;
383+
}
384+
if !quiet {
385+
println!("{}", format_found_missing(missing.len(), noun));
386+
}
387+
if args.common.dry_run {
388+
if !quiet {
389+
println!();
390+
println!("Would download:");
391+
for line in format_id_list(missing, noun, DRY_RUN_LIST_CAP) {
392+
println!("{line}");
393+
}
394+
}
395+
return pass;
396+
}
397+
let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent);
398+
status.set(format!("Downloading {}...", noun.count(missing.len())));
399+
if client.is_none() {
400+
*client = Some(
401+
get_api_client_with_overrides(args.common.api_client_overrides())
402+
.await
403+
.0,
404+
);
405+
}
406+
let client = client.as_ref().expect("client built just above");
407+
let sources = PatchSources {
408+
blobs_path: paths.blobs,
409+
packages_path: Some(paths.packages),
410+
diffs_path: Some(paths.diffs),
411+
mem_blobs: None,
412+
};
413+
let fetch_result = fetch_missing_sources(m, &sources, mode, client, None).await;
414+
status.finish();
415+
pass.downloaded = fetch_result.downloaded;
416+
pass.failed = fetch_result.failed;
417+
if !quiet {
418+
for line in format_fetch_successes(&fetch_result, noun) {
419+
println!("{line}");
420+
}
421+
}
422+
// Failures are error output: stderr, and not muted by `--silent`
423+
// (`--json` runs carry them in the envelope).
424+
if !args.common.json {
425+
for (i, line) in format_fetch_failures(&fetch_result, noun)
426+
.iter()
427+
.enumerate()
428+
{
429+
if i == 0 {
430+
eprintln!("Error: {line}");
431+
} else {
432+
eprintln!("{line}");
433+
}
434+
}
435+
}
436+
pass
437+
}
438+
344439
/// Whether an API token will be found, mirroring the client's chain: the
345440
/// `--api-token` flag (clap also maps SOCKET_API_TOKEN into it), then —
346441
/// unless `SOCKET_NO_API_TOKEN` vetoes ambient tokens — the env var and the
@@ -454,86 +549,48 @@ async fn repair_inner(
454549
.into_iter()
455550
.collect(),
456551
};
457-
let missing_count = missing_artifacts.len();
458552
let noun = download_mode.noun();
553+
let paths = SourcePaths {
554+
blobs: &blobs_path,
555+
packages: &packages_path,
556+
diffs: &diffs_path,
557+
};
459558
// Whether stdout already carries a line, so the blank separators
460559
// between sections never open the output (the offline warning goes
461560
// to stderr).
462-
let mut stdout_started = true;
463-
464-
if missing_artifacts.is_empty() {
465-
if !quiet {
466-
println!("{}", format_nothing_missing(manifest.as_ref(), noun));
561+
let mut stdout_started = !args.common.offline || missing_artifacts.is_empty();
562+
let primary = match scoped_manifest.as_ref() {
563+
Some(m) if !missing_artifacts.is_empty() => {
564+
download_pass(args, client, m, &missing_artifacts, download_mode, &paths).await
467565
}
468-
} else if args.common.offline {
469-
if !quiet {
470-
eprintln!("{}", format_offline_warning(&missing_artifacts, noun));
471-
}
472-
stdout_started = false;
473-
} else {
474-
if !quiet {
475-
println!("{}", format_found_missing(missing_artifacts.len(), noun));
476-
}
477-
478-
if args.common.dry_run {
566+
_ => {
479567
if !quiet {
480-
println!();
481-
println!("Would download:");
482-
for line in format_id_list(&missing_artifacts, noun, DRY_RUN_LIST_CAP) {
483-
println!("{line}");
484-
}
485-
}
486-
} else {
487-
let mut status = crate::ui::StatusLine::stderr(args.common.json, args.common.silent);
488-
status.set(format!(
489-
"Downloading {}...",
490-
noun.count(missing_artifacts.len())
491-
));
492-
if client.is_none() {
493-
*client = Some(
494-
get_api_client_with_overrides(args.common.api_client_overrides())
495-
.await
496-
.0,
497-
);
568+
println!("{}", format_nothing_missing(manifest.as_ref(), noun));
498569
}
499-
let client = client.as_ref().expect("client built just above");
500-
let sources = PatchSources {
501-
blobs_path: &blobs_path,
502-
packages_path: Some(&packages_path),
503-
diffs_path: Some(&diffs_path),
504-
mem_blobs: None,
505-
};
506-
// Step 1 only runs with a manifest (missing_artifacts is
507-
// empty otherwise), so the expect is unreachable.
508-
let m = scoped_manifest
509-
.as_ref()
510-
.expect("step 1 requires a manifest");
511-
let fetch_result =
512-
fetch_missing_sources(m, &sources, download_mode, client, None).await;
513-
status.finish();
514-
downloaded_count = fetch_result.downloaded;
515-
download_failed_count = fetch_result.failed;
516-
if !quiet {
517-
for line in format_fetch_successes(&fetch_result, noun) {
518-
println!("{line}");
519-
}
520-
}
521-
// Failures are error output: stderr, and not muted by
522-
// `--silent` (`--json` runs carry them in the envelope).
523-
if !args.common.json {
524-
for (i, line) in format_fetch_failures(&fetch_result, noun)
525-
.iter()
526-
.enumerate()
527-
{
528-
if i == 0 {
529-
eprintln!("Error: {line}");
530-
} else {
531-
eprintln!("{line}");
532-
}
533-
}
570+
DownloadPass::default()
571+
}
572+
};
573+
// A diff archive has no delta for a file the patch creates, so in diff
574+
// mode that file's blob is downloaded too: without it, a later
575+
// `apply --offline` cannot apply the patch.
576+
let created = match (&scoped_manifest, download_mode) {
577+
(Some(m), DownloadMode::Diff) => {
578+
let created = files_diffs_cannot_cover(m);
579+
let missing: Vec<String> = get_missing_blobs(&created, &blobs_path)
580+
.await
581+
.into_iter()
582+
.collect();
583+
if missing.is_empty() {
584+
DownloadPass::default()
585+
} else {
586+
download_pass(args, client, &created, &missing, DownloadMode::File, &paths).await
534587
}
535588
}
536-
}
589+
_ => DownloadPass::default(),
590+
};
591+
let missing_count = primary.missing;
592+
downloaded_count += primary.downloaded + created.downloaded;
593+
download_failed_count += primary.failed;
537594

538595
// Step 1.5: vendored artifacts — health-check the ledger (and any
539596
// lockfile vendor references with no ledger coverage) and rebuild
@@ -613,13 +670,13 @@ async fn repair_inner(
613670
// so a piped stdout never ends in a stray blank line when the
614671
// line itself goes to stderr.
615672
let other_failure = matches!(env.status, Status::PartialFailure | Status::Error);
616-
let line = format_final_line(
617-
download_failed_count,
618-
other_failure,
619-
noun,
620-
args.common.dry_run,
621-
);
622-
if download_failed_count > 0 || other_failure {
673+
let (failed, failed_noun) = if download_failed_count > 0 {
674+
(download_failed_count, noun)
675+
} else {
676+
(created.failed, BLOB)
677+
};
678+
let line = format_final_line(failed, other_failure, failed_noun, args.common.dry_run);
679+
if failed > 0 || other_failure {
623680
if stdout_started {
624681
eprintln!();
625682
}
@@ -660,6 +717,28 @@ async fn repair_inner(
660717
));
661718
env.mark_partial_failure();
662719
}
720+
if created.downloaded > 0
721+
|| (!args.common.offline && args.common.dry_run && created.missing > 0)
722+
{
723+
let (action, count) = if args.common.dry_run {
724+
(PatchAction::Verified, created.missing)
725+
} else {
726+
(PatchAction::Downloaded, created.downloaded)
727+
};
728+
env.record(
729+
PatchEvent::artifact(action).with_details(serde_json::json!({
730+
"count": count,
731+
"mode": DownloadMode::File.as_tag(),
732+
})),
733+
);
734+
}
735+
if created.failed > 0 {
736+
env.record(PatchEvent::artifact(PatchAction::Failed).with_error(
737+
"download_failed",
738+
format!("{} failed to download", BLOB.count(created.failed)),
739+
));
740+
env.mark_partial_failure();
741+
}
663742
if blobs_cleaned > 0 {
664743
let cleanup_action = if args.common.dry_run {
665744
PatchAction::Verified

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,3 +237,24 @@ async fn default_repair_downloads_the_created_files_blob_for_offline_apply() {
237237
assert_eq!(code, 0, "apply: stdout={stdout}\nstderr={stderr}");
238238
assert_fully_patched(&pkg);
239239
}
240+
241+
#[test]
242+
fn offline_repair_names_the_created_files_missing_blob() {
243+
let tmp = tempfile::tempdir().unwrap();
244+
seed_project(tmp.path());
245+
seed_cached_diff_archive(tmp.path());
246+
247+
let (code, stdout, stderr) = run_cli(tmp.path(), &["repair", "--offline"], None);
248+
249+
assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}");
250+
assert!(
251+
stdout.contains("All diff archives are present locally."),
252+
"stdout={stdout}"
253+
);
254+
let short: String = git_sha256(CREATED).chars().take(12).collect();
255+
assert!(
256+
stderr.contains("Warning: 1 blob is missing (offline mode - not downloading):")
257+
&& stderr.contains(&short),
258+
"the created file's blob is still missing; stderr={stderr}"
259+
);
260+
}

0 commit comments

Comments
 (0)