diff --git a/crates/socket-patch-cli/src/commands/bun_preflight.rs b/crates/socket-patch-cli/src/commands/bun_preflight.rs index fb8f30bde..733ef9311 100644 --- a/crates/socket-patch-cli/src/commands/bun_preflight.rs +++ b/crates/socket-patch-cli/src/commands/bun_preflight.rs @@ -1,17 +1,12 @@ -//! The Bun vendored-mode preflight shared by EVERY path that feeds the -//! vendor engine: `scan --mode vendored` (its in-memory download phase), -//! `get … --mode vendored` -//! (search and uuid paths), their `--dry-run` previews, and the `vendor` command's engine -//! loop itself ([`crate::commands::vendor::vendor_records`], where it runs -//! BEFORE the hosted→vendored takeover reverts anything). +//! The Bun vendored-mode preflight shared by every path that feeds the +//! vendor engine: `scan --mode vendored`, `get … --mode vendored`, their +//! `--dry-run` previews, and the `vendor` command itself (BEFORE the +//! hosted→vendored takeover reverts anything). //! -//! One read-only [`preflight_vendor`] per run, evaluated before any -//! `/patches/view/` fetch and before any write, so an incompatible Bun -//! project (malformed binary `bun.lockb`, an unreadable lock, an -//! unsupported `lockfileVersion`, a pre-version-2 `workspace:` lock) never -//! has a patch downloaded on its behalf — let alone recorded in the -//! manifest, or its live hosted redirect stripped — and every entry point -//! reports the SAME vendor code the engine would have emitted. +//! It runs [`preflight_vendor`] once, before any `/patches/view/` fetch and +//! before any write, so an incompatible Bun project never has a patch +//! downloaded, recorded, or its hosted redirect stripped, and every entry +//! point reports the same vendor code the engine would have emitted. //! //! [`preflight_vendor`]: socket_patch_core::vendor::bun_lock::preflight_vendor @@ -23,58 +18,50 @@ use socket_patch_core::crawlers::Ecosystem; use socket_patch_core::vendor::load_state; use socket_patch_core::vendor::state::VendorEntry; -/// The vendor ledger as the preflight consumes it: the caller's own -/// `load_state` outcome, so an UNREADABLE ledger is a fact the refusal can -/// report instead of a silently-emptied exemption set. +/// The caller's own `load_state` outcome, so an unreadable ledger is a fact +/// the refusal can report instead of a silently-emptied exemption set. pub(crate) type LedgerLoad<'a> = Result<&'a HashMap, &'a std::io::Error>; /// The outcome of the preflight when the project is refused. /// -/// `exempt` holds the selected purls the refusal must NOT pre-empt — they -/// flow through to the engine, which lets them in exactly as on a non-Bun -/// project. A purl is exempt only when `bun.lock` wires every instance of -/// its `name@version` to one of our `.socket/vendor/npm/` tuples, at any -/// UUID ([`wired_instances_all_ours`]). This matches the engine's workspace -/// gate: updating an already-local tuple introduces no new relative path, -/// so in-sync runs, superseding patches and repairs remain supported. -/// +/// `exempt` holds the selected purls the refusal must NOT pre-empt. A purl +/// is exempt only when `bun.lock` wires every instance of its +/// `name@version` to one of our `.socket/vendor/npm/` tuples, at any UUID +/// ([`wired_instances_all_ours`]). This matches the engine's workspace +/// gate: updating an already-local tuple introduces no new relative path. /// A matching ledger UUID alone is insufficient: `rollback --preserve-state` /// retains the entry after removing its wiring. /// /// An unreadable ledger exempts nothing (fail closed) and the refusal -/// itself becomes `vendor_state_unreadable` with the io/parse detail: -/// the vendor step does NOT reliably report the corrupt ledger itself -/// (`get --mode vendored` returns before it runs, and the -/// scan/search paths reach it only when the manifest already holds another -/// vendorable record), so the one refusal this run emits has to name the -/// real problem rather than send the operator off to re-lock bun.lock. +/// becomes `vendor_state_unreadable`: the vendor step does not reliably +/// report the corrupt ledger itself (`get --mode vendored` returns +/// before it runs), so this refusal has to name the real problem rather +/// than send the operator off to re-lock bun.lock. /// /// [`wired_instances_all_ours`]: socket_patch_core::vendor::bun_lock::wired_instances_all_ours pub(crate) struct BunVendorRefusal { - /// The stable vendor error code (`vendor_bun_lockb_invalid`, - /// `vendor_lockfile_missing`, `vendor_lockfile_version_unsupported`, - /// `vendor_bun_workspace_unsupported`) — the same string the vendor - /// engine would have emitted as a `failed` event — or - /// `vendor_state_unreadable` when the ledger could not be read. + /// The vendor error code the engine would have emitted, or + /// `vendor_state_unreadable`. pub(crate) code: &'static str, - /// The engine's (or the ledger loader's) human-readable detail, relayed - /// verbatim. pub(crate) detail: String, exempt: HashSet, } impl BunVendorRefusal { - /// Whether the refusal applies to `purl`: npm-family only (no other - /// ecosystem's backend consults `bun.lock`), minus the already-vendored - /// exemption. + /// npm-family only (no other ecosystem's backend consults `bun.lock`), + /// minus the already-vendored exemption. pub(crate) fn applies_to(&self, purl: &str) -> bool { - Ecosystem::from_purl(purl) == Some(Ecosystem::Npm) && !self.exempt.contains(purl) + is_npm(purl) && !self.exempt.contains(purl) } } -/// Run the Bun preflight once for `selected` — only when it holds at least -/// one npm purl, since nothing else can be affected — loading the vendor -/// ledger at `cwd` to detect corruption. `None` means nothing to refuse. +fn is_npm(purl: &str) -> bool { + Ecosystem::from_purl(purl) == Some(Ecosystem::Npm) +} + +/// The preflight for `selected`, loading the vendor ledger only once the +/// project is known to refuse, so an accepted project never touches +/// `state.json`. pub(crate) async fn bun_vendor_preflight( cwd: &Path, selected: &[PatchSearchResult], @@ -82,9 +69,7 @@ pub(crate) async fn bun_vendor_preflight( preflight_pairs(cwd, &selection_pairs(selected), None).await } -/// [`bun_vendor_preflight`] for callers that already loaded the ledger (the -/// detached download phase, the dry-run preview), handed the load outcome -/// so an unreadable ledger reports as such. +/// [`bun_vendor_preflight`] with the caller's ledger load. pub(crate) async fn bun_vendor_preflight_with_ledger( cwd: &Path, selected: &[PatchSearchResult], @@ -93,10 +78,8 @@ pub(crate) async fn bun_vendor_preflight_with_ledger( preflight_pairs(cwd, &selection_pairs(selected), Some(ledger)).await } -/// The preflight over bare `(purl, uuid)` pairs — the `vendor` command's -/// view of its selection (manifest records, not search results) — with a -/// caller-loaded ledger. `None` when no npm purl is selected or the -/// project is accepted. +/// The preflight over bare `(purl, uuid)` pairs (the `vendor` command's +/// manifest records) with the caller's ledger load. pub(crate) async fn bun_vendor_preflight_pairs( cwd: &Path, pairs: &[(&str, &str)], @@ -105,19 +88,15 @@ pub(crate) async fn bun_vendor_preflight_pairs( preflight_pairs(cwd, pairs, Some(ledger)).await } -/// The one preflight every entry point above funnels into. `ledger` is the -/// caller's own load when it has one; `None` loads the ledger here — and -/// only once the project is known to refuse, so an accepted project never -/// touches `state.json` (the vendor step owns it). +/// `ledger` is the caller's own load, or `None` to load it here once the +/// project is known to refuse. `None` when no npm purl is selected, a vlt +/// lock routes the project away from Bun, or the project is accepted. async fn preflight_pairs( cwd: &Path, pairs: &[(&str, &str)], ledger: Option>, ) -> Option { - if !pairs - .iter() - .any(|(purl, _)| Ecosystem::from_purl(purl) == Some(Ecosystem::Npm)) - { + if !pairs.iter().any(|(purl, _)| is_npm(purl)) { return None; } // vlt wins the router (DESIGN D2): a Bun lock beside `vlt-lock.json` @@ -139,58 +118,38 @@ async fn preflight_pairs( loaded.as_ref().map(|s| &s.entries) } }; - Some(refusal_with_exemptions(cwd, code, detail, pairs, ledger).await) -} - -fn selection_pairs(selected: &[PatchSearchResult]) -> Vec<(&str, &str)> { - selected - .iter() - .map(|s| (s.purl.as_str(), s.uuid.as_str())) - .collect() -} - -/// Turn the engine's project-level refusal into the per-purl verdict: the -/// live-lock exemption described on [`BunVendorRefusal`], or the -/// `vendor_state_unreadable` refusal when the ledger cannot be read. -async fn refusal_with_exemptions( - cwd: &Path, - code: &'static str, - detail: String, - pairs: &[(&str, &str)], - ledger: LedgerLoad<'_>, -) -> BunVendorRefusal { if let Err(e) = ledger { - return BunVendorRefusal { + return Some(BunVendorRefusal { code: "vendor_state_unreadable", detail: e.to_string(), exempt: HashSet::new(), - }; + }); } - // The lock-derived exemption exists only for the workspace gate: every - // other preflight code means bun.lock could not be read or parsed, so - // nothing in it can be ours and re-reading it per purl would be wasted - // (guarded, but still) I/O. - let lock_parsed = code == "vendor_bun_workspace_unsupported"; + // The exemption exists only for the workspace gate: every other code + // means bun.lock could not be read or parsed, so nothing in it is ours. let mut exempt = HashSet::new(); - for (purl, _) in pairs { - if Ecosystem::from_purl(purl) != Some(Ecosystem::Npm) { - continue; - } - // A preserved ledger can outlive its wiring (rollback --preserve-state). - // Only live lock tuples prove the engine can skip the workspace gate. - let lock_all_ours = lock_parsed - && socket_patch_core::vendor::bun_lock::wired_instances_all_ours(cwd, purl) + if code == "vendor_bun_workspace_unsupported" { + for (purl, _) in pairs.iter().filter(|(purl, _)| is_npm(purl)) { + if socket_patch_core::vendor::bun_lock::wired_instances_all_ours(cwd, purl) .await - .unwrap_or(false); - if lock_all_ours { - exempt.insert((*purl).to_string()); + .unwrap_or(false) + { + exempt.insert((*purl).to_string()); + } } } - BunVendorRefusal { + Some(BunVendorRefusal { code, detail, exempt, - } + }) +} + +fn selection_pairs(selected: &[PatchSearchResult]) -> Vec<(&str, &str)> { + selected + .iter() + .map(|s| (s.purl.as_str(), s.uuid.as_str())) + .collect() } #[cfg(test)] diff --git a/crates/socket-patch-cli/src/commands/composer_hints.rs b/crates/socket-patch-cli/src/commands/composer_hints.rs index d4637c423..cbc59f33b 100644 --- a/crates/socket-patch-cli/src/commands/composer_hints.rs +++ b/crates/socket-patch-cli/src/commands/composer_hints.rs @@ -13,7 +13,7 @@ //! `$COMPOSER_HOME/config.json` `vendor-dir`, else `vendor`): a hint //! naming `vendor/` in a project that relocated it sends the user to delete //! a path that does not exist, and `composer install` then leaves the -//! package unpatched (#658). +//! package unpatched. use std::path::Path; @@ -187,25 +187,30 @@ mod tests { #[test] fn vendored_hints_name_every_package_dir() { assert!(vendored_reinstall_hints(&[], "vendor").is_empty()); - let hints = vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()], "vendor"); - assert_eq!(hints.len(), 2); - assert!( - hints[0].starts_with("Run `composer install`"), - "{}", - hints[0] - ); - assert!( - hints[1].contains("remove vendor/a/b, vendor/c/d first"), - "{}", - hints[1] + assert_eq!( + vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()], "vendor"), + vec![ + "Run `composer install` to update vendor/ — vendoring rewires composer.lock \ + only, so the installed vendor/ tree keeps the unpatched bytes until Composer \ + reinstalls it." + .to_string(), + "Composer 1 does not reinstall a locked package whose dist changed: remove \ + vendor/a/b, vendor/c/d first, then run `composer install`." + .to_string(), + ] ); } #[test] fn hosted_hint_needs_a_rewritten_composer_lock() { assert!(hosted_reinstall_hint(&["package-lock.json".to_string()], &[], "vendor").is_none()); - let hint = hosted_reinstall_hint(&["composer.lock".to_string()], &[], "vendor").unwrap(); - assert!(hint.contains("composer install") && hint.contains("Composer 1")); + assert_eq!( + hosted_reinstall_hint(&["composer.lock".to_string()], &[], "vendor").as_deref(), + Some( + " (e.g. `composer install`; on Composer 1 first remove the patched packages' \ + vendor// directories)" + ) + ); } fn dist_edit(key: &str, original: &str, new: &str) -> FileEdit { @@ -239,42 +244,57 @@ mod tests { &format!(r#"{OLD_DIST}, "extra": {{"source": {{}}}}, "source": {{"type": "git"}}"#), &format!(r#"{NEW_DIST}, "extra": {{"source": {{}}}}"#), ); + // An edit that drops `source` names no dir beyond the Composer 1 note. + let composer1_only = |dir: &str| { + format!( + " (e.g. `composer install`; on Composer 1 first remove the patched packages' \ + {dir}// directories)" + ) + }; for edits in [vec![with_source.clone()], vec![source_after_extra]] { - let hint = hosted_reinstall_hint(&files, &edits, "vendor").unwrap(); - assert!(hint.contains("on Composer 1 first remove"), "{hint}"); + assert_eq!( + hosted_reinstall_hint(&files, &edits, "vendor"), + Some(composer1_only("vendor")) + ); } + assert_eq!( + hosted_reinstall_hint(&files, &[], "lib"), + Some(composer1_only("lib")) + ); let dist_only = dist_edit("Acme/Lib", OLD_DIST, NEW_DIST); - let hint = - hosted_reinstall_hint(&files, &[with_source.clone(), dist_only.clone()], "vendor") - .unwrap(); - assert!(hint.contains("first remove vendor/acme/lib —"), "{hint}"); - assert!(!hint.contains("vendor/psr/log"), "{hint}"); - assert!(hint.contains("on Composer 1"), "{hint}"); - - // A relocated vendor dir is the one named (#658). - let hint = hosted_reinstall_hint(&files, &[with_source, dist_only], "lib").unwrap(); - assert!(hint.contains("first remove lib/acme/lib —"), "{hint}"); - assert!( - hint.contains("patched package's lib//"), - "{hint}" + let removing = |dir: &str| { + format!( + " (e.g. `composer install`; first remove {dir}/acme/lib — Composer does not \ + reinstall a package whose lock entry has no `source` when only its dist url \ + changes — and on Composer 1 every other patched package's \ + {dir}// directory)" + ) + }; + let edits = [with_source, dist_only]; + assert_eq!( + hosted_reinstall_hint(&files, &edits, "vendor"), + Some(removing("vendor")) ); - assert!(!hint.contains("vendor/"), "{hint}"); - let hint = hosted_reinstall_hint(&files, &[], "lib").unwrap(); - assert!( - hint.contains("remove the patched packages' lib//"), - "{hint}" + // A relocated vendor dir is the one named. + assert_eq!( + hosted_reinstall_hint(&files, &edits, "lib"), + Some(removing("lib")) ); } - /// #658: with `config.vendor-dir: lib` the vendored hints name + /// With `config.vendor-dir: lib` the vendored hints name /// `lib//`, the directory Composer installed into. #[test] fn vendored_hints_name_a_relocated_vendor_dir() { let hints = vendored_reinstall_hints(&["acme/tool".to_string()], "lib"); - assert!(hints[0].contains("update lib/"), "{}", hints[0]); assert!( - hints[1].contains("remove lib/acme/tool first"), + hints[0].starts_with("Run `composer install` to update lib/ —"), + "{}", + hints[0] + ); + assert!( + hints[1].ends_with(": remove lib/acme/tool first, then run `composer install`."), "{}", hints[1] ); diff --git a/crates/socket-patch-cli/src/commands/context.rs b/crates/socket-patch-cli/src/commands/context.rs index d46ebca65..b9b2b1b93 100644 --- a/crates/socket-patch-cli/src/commands/context.rs +++ b/crates/socket-patch-cli/src/commands/context.rs @@ -1,9 +1,9 @@ //! The project a command reads, loaded lazily and at most once per run: //! the patch stores ([`LoadedLedgers`]: manifest + both ledgers), the lock //! set (the lockfile inventory and its refused npm layouts) and the -//! lockfile wiring discovery. `scan`, `vendor`, `vex`, `list` and `get` -//! read these through one [`ProjectContext`] instead of each re-loading -//! and re-merging them its own way. +//! lockfile wiring discovery. `scan`, `list`, `get` and `apply` read these +//! through one [`ProjectContext`] instead of each re-loading and re-merging +//! them its own way. //! //! The lock set and the discovery read `--cwd` through one //! [`DiskSnapshot`], so each lock and config file is read once and both see @@ -46,8 +46,8 @@ pub(crate) struct ProjectContext<'a> { } impl<'a> ProjectContext<'a> { - /// Every command's context: the ledgers always load from the manifest's - /// project, so `--manifest-path` never mixes two projects' state (#745). + /// The ledgers always load from the manifest's project, so + /// `--manifest-path` never mixes two projects' state. pub(crate) fn new(common: &'a GlobalArgs) -> Self { Self { common, @@ -59,7 +59,6 @@ impl<'a> ProjectContext<'a> { } } - /// The three stores, each with its own load outcome. pub(crate) async fn loaded(&self) -> &LoadedLedgers { self.ledgers .get_or_init(|| async { @@ -73,7 +72,6 @@ impl<'a> ProjectContext<'a> { self.loaded().await.view() } - /// The lockfile inventory of `--cwd`. pub(crate) async fn locks(&self) -> &LockSet { self.locks .get_or_init(|| async { @@ -90,7 +88,7 @@ impl<'a> ProjectContext<'a> { .await } - /// The lockfile wiring discovery of `--cwd` ([`super::discover_wiring`]). + /// The lockfile wiring discovery of `--cwd` ([`super::discover_wiring_in`]). pub(crate) async fn discovery(&self) -> &Discovery { &self.recorded_discovery().await.0 } diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index 2c91ab644..6e946e6da 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -1,11 +1,8 @@ -//! Shared patch-source staging for the mutating commands (`apply`, `vendor`). -//! -//! Resolves where the patch pipeline should read per-file blobs from, -//! downloading what's missing into a transient overlay tempdir. The -//! persistent `.socket/blobs` cache is only ever *read* — -//! downloads land in the tempdir and are discarded when it drops (filling the -//! cache is `repair`'s job, keeping these commands read-only against -//! `.socket/`). +//! Patch-source staging for `apply`: where the patch pipeline reads +//! per-file blobs from, downloading what's missing into a transient overlay +//! tempdir. The persistent `.socket/blobs` cache is only ever read; +//! downloads are discarded when the tempdir drops (filling the cache is +//! `repair`'s job, keeping `apply` read-only against `.socket/`). use std::collections::HashSet; use std::path::{Path, PathBuf}; @@ -48,31 +45,6 @@ pub(crate) enum StageOutcome { Unavailable, } -/// The disk stager's remedy: `repair` fills the persistent `.socket/` -/// cache `apply` reads from. -const APPLY_OFFLINE_REMEDY: &str = "Run `socket-patch repair` to download missing artifacts."; - -/// Shared offline diagnostic: patches with no usable local source while -/// `--offline` is set (first five PURLs, then the caller's `remedy` line). -/// Prints even under `--silent` (errors only, NEVER nothing — an exit-1 -/// run with zero output is undiagnosable); `--json` mutes stderr and the -/// caller's envelope is the machine channel instead. -fn report_offline_missing(common: &GlobalArgs, purls: &[&str], remedy: &str) { - if common.json { - return; - } - let n = purls.len(); - let (count, verb) = ( - plural(n, "patch", "patches"), - if n == 1 { "has" } else { "have" }, - ); - eprintln!("Error: {count} {verb} no local source and --offline is set:"); - for line in format_purl_list(purls, 5) { - eprintln!("{line}"); - } - eprintln!("{remedy}"); -} - /// ` - ` for the first `max` purls, then ` ... and N more`. fn format_purl_list(purls: &[&str], max: usize) -> Vec { let mut lines: Vec = purls.iter().take(max).map(|p| format!(" - {p}")).collect(); @@ -127,13 +99,8 @@ fn format_fetch_failures(result: &FetchMissingBlobsResult) -> Vec { lines } -/// The disk stager's status line while it downloads what `.socket/` lacks. -const DOWNLOADING_ARTIFACTS: &str = "Downloading missing patch artifacts..."; - -/// The manifest PURLs with no usable local source: some file the patch -/// touches has no `after_hash` blob in `missing_blobs`' directory. Shared by -/// the offline gate (probed against `.socket/`) and the post-download gate -/// (probed against the staged overlay). +/// The manifest PURLs with no usable local source: the `after_hash` of +/// some file the patch touches is in `missing_blobs` (absent on disk). fn patches_without_source<'m>( manifest: &'m PatchManifest, missing_blobs: &HashSet, @@ -179,11 +146,9 @@ async fn overlay_dir(src: &Path, dst: &Path) { /// Resolve patch sources for `manifest`: read straight from `.socket/` when /// every blob needed is cached (or `--offline`), else stage an overlay -/// tempdir and fetch the missing blobs through `client` (the run's one API -/// client — building another here repeated its advisory and org-slug -/// resolution). `Err` is a hard setup failure (tempdir creation); -/// `Ok(Unavailable)` is the soft "cannot proceed" path with diagnostics -/// already printed. +/// tempdir and fetch the missing blobs through `client`. `Err` is a hard +/// setup failure (tempdir creation); `Ok(Unavailable)` is the soft "cannot +/// proceed" path with diagnostics already printed. pub(crate) async fn stage_patch_sources( common: &GlobalArgs, manifest: &PatchManifest, @@ -193,17 +158,26 @@ pub(crate) async fn stage_patch_sources( let quiet = common.silent || common.json; let socket_blobs_path = socket_dir.join("blobs"); - // Read-only probe of what is already on disk. let missing_blobs = get_missing_blobs(manifest, &socket_blobs_path).await; if common.offline { - // Offline: bail only if some patch has no usable local source. - // Note: with `--force`, the patch pipeline can short-circuit - // verification on its own; we still surface the no-source - // diagnosis so the user runs `repair` before retrying. let no_source_purls = patches_without_source(manifest, &missing_blobs); if !no_source_purls.is_empty() { - report_offline_missing(common, &no_source_purls, APPLY_OFFLINE_REMEDY); + // Prints even under `--silent`: an exit-1 run with zero output is + // undiagnosable. `--json` mutes stderr; the caller's envelope is + // the machine channel. + if !common.json { + let n = no_source_purls.len(); + let verb = if n == 1 { "has" } else { "have" }; + eprintln!( + "Error: {} {verb} no local source and --offline is set:", + plural(n, "patch", "patches") + ); + for line in format_purl_list(&no_source_purls, 5) { + eprintln!("{line}"); + } + eprintln!("Run `socket-patch repair` to download missing artifacts."); + } return Ok(StageOutcome::Unavailable); } } @@ -215,10 +189,8 @@ pub(crate) async fn stage_patch_sources( })); } - // Stage a transient overlay tempdir that hardlinks every existing - // `.socket/blobs` entry and receives fresh downloads. The pipeline reads - // exclusively from the tempdir; `.socket/` is never mutated. Dropping - // `StagedSources` removes the directory and any downloaded bytes. + // The pipeline reads only from the overlay; dropping `StagedSources` + // removes it and any downloaded bytes. let stage = tempfile::tempdir().map_err(|e| e.to_string())?; let staged = StagedSources { blobs: stage.path().join("blobs"), @@ -229,10 +201,8 @@ pub(crate) async fn stage_patch_sources( .map_err(|e| e.to_string())?; overlay_dir(&socket_blobs_path, &staged.blobs).await; - // Progress: a transient status line on stderr (stdout is data); the - // result lines below are what stays on screen. let mut status = StatusLine::stderr(common.json, common.silent); - status.set(DOWNLOADING_ARTIFACTS); + status.set("Downloading missing patch artifacts..."); let fetch_result = fetch_missing_blobs(manifest, &staged.blobs, client, None).await; status.finish(); if !quiet { @@ -247,9 +217,8 @@ pub(crate) async fn stage_patch_sources( if fetch_result.failed > 0 { let missing_blobs = get_missing_blobs(manifest, &staged.blobs).await; if !patches_without_source(manifest, &missing_blobs).is_empty() { - // An error, not progress chatter: prints even under --silent - // (same rule as report_offline_missing above), with the - // per-blob reasons the quiet summary above held back. + // An error, so it prints even under --silent, with the per-blob + // reasons the quiet summary above held back. if !common.json { if quiet { for line in format_fetch_failures(&fetch_result) { @@ -267,14 +236,6 @@ pub(crate) async fn stage_patch_sources( Ok(StageOutcome::Ready(staged)) } -/// In-memory staged sources for the VENDOR flows. -/// -/// Existing `.socket/` artifacts are read in place (never copied, never -/// rewritten); patch content that is missing locally is fetched into -/// MEMORY via the patch view endpoint — vendoring writes no -/// `.socket/blobs` entries and no temporary files. The committed -/// `.socket/vendor/` artifact is the patch; nothing else should land on -/// disk. #[cfg(test)] mod tests { use super::*; @@ -320,9 +281,8 @@ mod tests { } } - /// A network-free client for the offline arms (never used: they return - /// before any fetch), built directly so no ambient token or socket-cli - /// config can leak into a unit test. + /// A client for the arms that return before any fetch, built directly + /// so no ambient token or socket-cli config can leak into a unit test. fn offline_client() -> ApiClient { ApiClient::new(socket_patch_core::api::client::ApiClientOptions { api_url: "http://127.0.0.1:1".to_string(), @@ -338,8 +298,8 @@ mod tests { .0 } - /// Everything cached → read `.socket/` in place: no overlay tempdir, and - /// the returned paths are the persistent cache dirs themselves. + /// Everything cached, online or offline → read `.socket/` in place: no + /// overlay tempdir, and the blobs path is the persistent cache itself. #[tokio::test] async fn stage_reads_socket_dir_in_place_when_fully_cached() { let tmp = tempfile::tempdir().unwrap(); @@ -347,19 +307,25 @@ mod tests { std::fs::create_dir_all(socket_dir.join("blobs")).unwrap(); std::fs::write(socket_dir.join("blobs").join(HASH), b"patched").unwrap(); - let outcome = stage_patch_sources( - &offline_args(), - &manifest_with_one_patch(), - &socket_dir, - &offline_client(), - ) - .await - .expect("no hard failure"); - let StageOutcome::Ready(staged) = outcome else { - panic!("fully-cached staging must be Ready"); - }; - assert!(staged._stage.is_none(), "no overlay when nothing to fetch"); - assert_eq!(staged.blobs, socket_dir.join("blobs")); + for offline in [true, false] { + let args = GlobalArgs { + offline, + ..offline_args() + }; + let outcome = stage_patch_sources( + &args, + &manifest_with_one_patch(), + &socket_dir, + &offline_client(), + ) + .await + .expect("no hard failure"); + let StageOutcome::Ready(staged) = outcome else { + panic!("fully-cached staging must be Ready (offline: {offline})"); + }; + assert!(staged._stage.is_none(), "no overlay when nothing to fetch"); + assert_eq!(staged.blobs, socket_dir.join("blobs")); + } } /// Offline with no usable source → Unavailable, and the read-only @@ -387,34 +353,6 @@ mod tests { ); } - /// A leftover `.socket/diffs/.tar.gz` is not a source: v5 reads - /// only per-file blobs, so offline staging with the blob missing is - /// Unavailable even when the old diff archive is on disk. - #[tokio::test] - async fn stage_offline_ignores_obsolete_diff_archive() { - let tmp = tempfile::tempdir().unwrap(); - let socket_dir = tmp.path().join(".socket"); - std::fs::create_dir_all(socket_dir.join("diffs")).unwrap(); - std::fs::write( - socket_dir.join("diffs").join(format!("{UUID}.tar.gz")), - b"x", - ) - .unwrap(); - - let outcome = stage_patch_sources( - &offline_args(), - &manifest_with_one_patch(), - &socket_dir, - &offline_client(), - ) - .await - .expect("no hard failure"); - assert!( - matches!(outcome, StageOutcome::Unavailable), - "an obsolete diff archive must not cover the patch" - ); - } - /// GlobalArgs wired to a guaranteed-unreachable API endpoint: explicit /// token + org overrides keep client construction network-free, and the /// URL points at a port that was just bound and released, so every fetch @@ -434,34 +372,6 @@ mod tests { } } - /// A leftover legacy `.socket/packages/.tar.gz` is not a source: - /// nothing reads it, so it must not mask failed downloads. - #[tokio::test] - async fn stage_online_fetch_failure_ignores_legacy_package_archive() { - let tmp = tempfile::tempdir().unwrap(); - let socket_dir = tmp.path().join(".socket"); - std::fs::create_dir_all(socket_dir.join("packages")).unwrap(); - std::fs::write( - socket_dir.join("packages").join(format!("{UUID}.tar.gz")), - b"x", - ) - .unwrap(); - - let args = dead_endpoint_args(); - let outcome = stage_patch_sources( - &args, - &manifest_with_one_patch(), - &socket_dir, - &dead_endpoint_client(&args).await, - ) - .await - .expect("no hard failure"); - assert!( - matches!(outcome, StageOutcome::Unavailable), - "a legacy package archive must not cover the patch" - ); - } - /// Overshoot guard for the per-patch coverage gate: with no local source /// at all, failed downloads must still yield Unavailable. #[tokio::test] diff --git a/crates/socket-patch-cli/tests/apply/apply_silent_staging_errors.rs b/crates/socket-patch-cli/tests/apply/apply_silent_staging_errors.rs index ffacb6acf..e08a1c0b5 100644 --- a/crates/socket-patch-cli/tests/apply/apply_silent_staging_errors.rs +++ b/crates/socket-patch-cli/tests/apply/apply_silent_staging_errors.rs @@ -7,7 +7,7 @@ //! //! 1. `apply --silent --offline` with a manifest patch that has no local //! blob/diff/package source exited 1 with zero bytes on stdout+stderr -//! (`report_offline_missing` returned early on `silent || json`). +//! (the offline report returned early on `silent || json`). //! 2. Online `apply --silent` whose downloads all fail (dead endpoint) //! exited 1 with zero output (the "Some artifacts could not be //! downloaded" line was gated on `!quiet`, i.e. muted by `--silent`).