From dd7ecc6282aba5e77a2230f5465cc3c9a1bbd5ad Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 14:57:46 +0000 Subject: [PATCH 1/3] Start sweep of vendored_backend Assisted-by: Claude Code:claude-opus-5-5 From 6df8ff798a334008e5e49308d42a0836625f344f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 15:04:33 +0000 Subject: [PATCH 2/3] Sweep vendored_backend mod.rs and repair.rs Inline revert_vendor_entry and its result struct into VendoredBackend::revert, and give VendorRevertStep one unwired() predicate for the two lists that spelled it out. In repair.rs, inline workspace_copy_issue, rebuild_reason_label and format_ledger_missing, drop Candidate's Clone and its copy of entry.detached, and build the lazy API client in one place, which removes an expect. Merge the ui_format_tests module into tests, rename the FIFO test after what it checks, and cut bare issue numbers from comments. Behavior is unchanged. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/vendored_backend/mod.rs | 153 ++++++++---------- .../src/commands/vendored_backend/repair.rs | 125 ++++++-------- 2 files changed, 112 insertions(+), 166 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs index a00d1e93c..9b16e4165 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs @@ -47,7 +47,7 @@ pub(crate) fn tag_event_mode(events: &mut [crate::json_envelope::PatchEvent], mo pub(crate) struct VendoredBackend<'a> { /// The run's args re-rooted at the manifest's project /// ([`GlobalArgs::at_project_root`]): the ledger, its artifacts and the - /// lockfile wiring they record always belong to one project (#745). + /// lockfile wiring they record always belong to one project. pub(crate) common: std::borrow::Cow<'a, GlobalArgs>, pub(crate) service: Option<&'a VendorServiceConfig>, } @@ -143,46 +143,64 @@ impl<'a> VendoredBackend<'a> { ) -> Vec { let mut out = Vec::with_capacity(keys.len()); for key in keys { - // Captured before a clean revert drops the entry. - let flavor = state.entries.get(key).and_then(|e| e.flavor.clone()); - let gem = state - .entries - .get(key) - .map(|e| (e.ecosystem.clone(), e.base_purl.clone())); - let mut result = revert_vendor_entry(&self.common.cwd, key, state, opts).await; + let Some(entry) = state.entries.get(key).cloned() else { + out.push(RevertedEntry { + key: key.clone(), + flavor: None, + warnings: Vec::new(), + step: VendorRevertStep::Missing, + }); + continue; + }; + let flavor = entry.flavor.clone(); + let root = &self.common.cwd; + let outcome = dispatch_revert_one_opts(&entry, root, opts).await; + let step = if !outcome.success { + VendorRevertStep::Failed(outcome.error.unwrap_or_else(|| "unknown error".into())) + } else if outcome.kept_artifact { + VendorRevertStep::Kept(if outcome.kept_for_residual_reference() { + KeepCause::Reference + } else { + KeepCause::Drift + }) + } else if opts.dry_run { + VendorRevertStep::WouldRevert + } else if opts.keep_artifact { + VendorRevertStep::Preserved + } else { + state.entries.remove(key); + match save_state(root, state).await { + Ok(()) => VendorRevertStep::Reverted, + Err(e) => VendorRevertStep::LedgerWriteFailed(e.to_string()), + } + }; + let mut warnings = outcome.warnings; let hard_failure = matches!( - result.step, + step, VendorRevertStep::Failed(_) | VendorRevertStep::LedgerWriteFailed(_) ); + let dry = matches!(step, VendorRevertStep::WouldRevert); // The wiring is back on the registry while the installed tree // may still hold the vendored build, which PDM, uv and Pipenv - // keep through a plain sync (#477): name the reinstall. - let unwired = matches!( - result.step, - VendorRevertStep::WouldRevert - | VendorRevertStep::Reverted - | VendorRevertStep::Preserved - | VendorRevertStep::LedgerWriteFailed(_) - ); - if unwired { + // keep through a plain sync: name the reinstall. + if dry || step.unwired() { crate::commands::pypi_reinstall::push_vendor_advisory( - &self.common.cwd, + root, key, flavor.as_deref(), - &mut result.warnings, + &mut warnings, ) .await; // A dry run reads the still-vendored lock, whose bare // CHECKSUMS entry can't judge the archive: the wet run // names it. - let dry = matches!(result.step, VendorRevertStep::WouldRevert); - if let Some((ecosystem, purl)) = gem.as_ref().filter(|_| !dry) { + if !dry { push_gem_stale_cache( - &self.common.cwd, - ecosystem, - purl, + root, + &entry.ecosystem, + &entry.base_purl, self.common.offline, - &mut result.warnings, + &mut warnings, ) .await; } @@ -190,8 +208,8 @@ impl<'a> VendoredBackend<'a> { out.push(RevertedEntry { key: key.clone(), flavor, - warnings: result.warnings, - step: result.step, + warnings, + step, }); if stop_on_failure && hard_failure { break; @@ -202,7 +220,7 @@ impl<'a> VendoredBackend<'a> { /// Dropping a vendored Go `replace` leaves a committed /// vendor/modules.txt still recording it, which breaks every default - /// build until `go mod vendor` runs (#343, #1777). The undo commands + /// build until `go mod vendor` runs. The undo commands /// call this after [`Self::revert`]: it audits the Go consumer state /// once and returns the issues of the Go modules `reverted` unwired, /// each keyed by the ledger key of its entry. Not part of `revert` @@ -214,14 +232,7 @@ impl<'a> VendoredBackend<'a> { ) -> Vec<(String, VendorWarning)> { let unwired: Vec<(&str, String)> = reverted .iter() - .filter(|e| { - matches!( - e.step, - VendorRevertStep::Reverted - | VendorRevertStep::Preserved - | VendorRevertStep::LedgerWriteFailed(_) - ) - }) + .filter(|e| e.step.unwired()) .filter_map(|e| { socket_patch_core::utils::purl::parse_golang_purl(&e.key) .map(|(module, _)| (e.key.as_str(), module.into_owned())) @@ -245,11 +256,10 @@ impl<'a> VendoredBackend<'a> { } /// Name a patched `-.gem` left in Bundler's cache dir once -/// the vendored ledger entry of `ecosystem` for `base_purl` is unwired -/// (#1594): a `bundle cache` taken while a hosted pin was live keeps it -/// there through the takeover, and bundler installs from it again once the -/// lock is back on upstream. The hosted unwind raises the same warning -/// (#1260). +/// the vendored ledger entry of `ecosystem` for `base_purl` is unwired: a +/// `bundle cache` taken while a hosted pin was live keeps it there through +/// the takeover, and bundler installs from it again once the lock is back +/// on upstream. The hosted unwind raises the same warning. pub(crate) async fn push_gem_stale_cache( root: &Path, ecosystem: &str, @@ -310,6 +320,18 @@ pub(crate) enum VendorRevertStep { LedgerWriteFailed(String), } +impl VendorRevertStep { + /// The wiring is back on the registry (a dry run's `WouldRevert` is not). + fn unwired(&self) -> bool { + matches!( + self, + VendorRevertStep::Reverted + | VendorRevertStep::Preserved + | VendorRevertStep::LedgerWriteFailed(_) + ) + } +} + /// Why a vendored revert kept the artifact and ledger entry. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub(crate) enum KeepCause { @@ -318,7 +340,7 @@ pub(crate) enum KeepCause { Drift, /// The recorded wiring was restored, but another project file (an /// exported requirements file) still installs from the artifact - /// (`vendor_revert_residual_reference`, #1184). Nothing drifted: the + /// (`vendor_revert_residual_reference`). Nothing drifted: the /// way out is to point that file back at the registry release. Reference, } @@ -350,51 +372,6 @@ impl KeepCause { } } -pub(crate) struct VendorRevertResult { - pub(crate) warnings: Vec, - pub(crate) step: VendorRevertStep, -} - -/// Revert the vendored ledger entry `key` (see [`VendorRevertStep`]) of -/// the project at `root` (its ledger, artifacts and lockfile wiring). -pub(crate) async fn revert_vendor_entry( - root: &Path, - key: &str, - state: &mut VendorState, - opts: RevertOpts, -) -> VendorRevertResult { - let Some(entry) = state.entries.get(key).cloned() else { - return VendorRevertResult { - warnings: Vec::new(), - step: VendorRevertStep::Missing, - }; - }; - let outcome = dispatch_revert_one_opts(&entry, root, opts).await; - let step = if !outcome.success { - VendorRevertStep::Failed(outcome.error.unwrap_or_else(|| "unknown error".into())) - } else if outcome.kept_artifact { - VendorRevertStep::Kept(if outcome.kept_for_residual_reference() { - KeepCause::Reference - } else { - KeepCause::Drift - }) - } else if opts.dry_run { - VendorRevertStep::WouldRevert - } else if opts.keep_artifact { - VendorRevertStep::Preserved - } else { - state.entries.remove(key); - match save_state(root, state).await { - Ok(()) => VendorRevertStep::Reverted, - Err(e) => VendorRevertStep::LedgerWriteFailed(e.to_string()), - } - }; - VendorRevertResult { - warnings: outcome.warnings, - step, - } -} - /// A patch-record map as the manifest view [`ApplyRequest::manifest`] /// takes (no `setup` block — vendoring never reads it). pub(crate) fn records_manifest(patches: HashMap) -> PatchManifest { diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs index bb40e02a1..61930a7e5 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs @@ -23,12 +23,10 @@ use crate::json_envelope::{Envelope, PatchAction, PatchEvent}; use crate::ui::plural; /// One broken vendored unit queued for re-vendoring. -#[derive(Clone)] struct Candidate { purl: String, entry: VendorEntry, record: PatchRecord, - detached: bool, reason: &'static str, } @@ -171,23 +169,19 @@ fn format_redownload_preview(items: &[(String, &str, &str)]) -> Vec { plural(items.len(), "vendored artifact", "vendored artifacts") )]; lines.extend(items.iter().map(|(purl, reason, path)| { - format!(" - {purl} ({}: {path})", rebuild_reason_label(reason)) + let label = match *reason { + "vendor_artifact_missing" => "missing", + "vendor_artifact_corrupt" => "corrupt", + other => other, + }; + format!(" - {purl} ({label}: {path})") })); lines } -/// Plain words for a rebuild candidate's reason code. -fn rebuild_reason_label(code: &str) -> &str { - match code { - "vendor_artifact_missing" => "missing", - "vendor_artifact_corrupt" => "corrupt", - other => other, - } -} - /// The [`scan_vendor_references`] triples whose `(ecosystem, uuid)` no /// entry of `state` owns: lockfile wiring the ledger lost (state.json -/// deleted, or a git merge kept one side of it, #991). `vendor --check` +/// deleted, or a git merge kept one side of it). `vendor --check` /// fails on them and an unscoped `rollback` refuses them. pub(crate) async fn unledgered_vendor_references( project_root: &Path, @@ -205,18 +199,6 @@ pub(crate) async fn unledgered_vendor_references( .collect() } -/// The `vendor_ledger_missing` detail for a lockfile reference no ledger -/// entry owns. -fn format_ledger_missing(eco: &str, uuid: &str) -> String { - format!( - "a lockfile references .socket/vendor/{eco}/{uuid}/ but the vendor ledger \ - (.socket/vendor/state.json) has no entry for it, and repair does not rebuild the \ - ledger from lockfiles; restore state.json from version control and re-run \ - `socket-patch repair`, or restore the lockfile (`git checkout -- `) and \ - re-vendor" - ) -} - /// Recover artifacts left aside by older releases when a repair was interrupted. async fn restore_orphaned_pre_rebuild_dirs(common: &GlobalArgs) { const SUFFIX: &str = ".pre-rebuild"; @@ -440,11 +422,11 @@ impl VendoredBackend<'_> { }; let health = check_vendored_artifact(&common.cwd, &entry, &record).await; // An npm-family `/.gitignore` and `.gitattributes` are not - // part of the artifact: a missing, edited or pre-#1541 one is + // part of the artifact: a missing, edited or older one is // simply rewritten, and git must still commit the artifact with - // them (#1581). A broken artifact gets both after its redownload. + // them. A broken artifact gets both after its redownload. if !common.dry_run - && (health == ArtifactHealth::Healthy || workspace_copy_issue(&health)) + && (health == ArtifactHealth::Healthy || health.is_workspace_copy_issue()) { if let Err(e) = vendor::redownload::restore_npm_uuid_metadata(&entry, &common.cwd).await @@ -462,7 +444,7 @@ impl VendoredBackend<'_> { continue; } } - if health == ArtifactHealth::Healthy || workspace_copy_issue(&health) { + if health == ArtifactHealth::Healthy || health.is_workspace_copy_issue() { let mut healed = entry.clone(); match repair_workspace_copies(&common.cwd, &mut healed, common.dry_run).await { Ok(true) => { @@ -511,14 +493,14 @@ impl VendoredBackend<'_> { continue; } } - if workspace_copy_issue(&health) { + if health.is_workspace_copy_issue() { continue; } } match health { ArtifactHealth::Healthy => { // A yarn classic block a pre-v5 vendor left naming a - // dependency with no lock block (#1379): the artifact + // dependency with no lock block: the artifact // is fine, but repair can't resolve the descriptor. if entry.ecosystem == "npm" { if let Some(detail) = @@ -552,7 +534,7 @@ impl VendoredBackend<'_> { } } // A stray file in the tree (a Finder `.DS_Store`) fails - // the Gradle build (#656); the index says what belongs + // the Gradle build; the index says what belongs // there, so it goes offline too. if vendor::jvm::apply::is_jvm_entry(&entry) { let indexed = vendor::jvm::apply::recorded_tree_files(&entry); @@ -680,12 +662,10 @@ impl VendoredBackend<'_> { } else { "vendor_artifact_corrupt" }; - let detached = entry.detached; candidates.push(Candidate { purl: purl.clone(), entry, record, - detached, reason, }); } @@ -706,7 +686,13 @@ impl VendoredBackend<'_> { } // No ledger entry means no purl to name: the event carries the // uuid and the referenced path instead. - let detail = format_ledger_missing(eco, uuid); + let detail = format!( + "a lockfile references .socket/vendor/{eco}/{uuid}/ but the vendor ledger \ + (.socket/vendor/state.json) has no entry for it, and repair does not rebuild \ + the ledger from lockfiles; restore state.json from version control and \ + re-run `socket-patch repair`, or restore the lockfile \ + (`git checkout -- `) and re-vendor" + ); if !common.json { eprintln!("Error: Cannot repair vendored artifact {path}: {detail}"); } @@ -765,12 +751,8 @@ impl VendoredBackend<'_> { ); } - if api_client.is_none() && !common.offline { - *api_client = Some( - get_api_client_with_overrides(common.api_client_overrides()) - .await - .0, - ); + if !common.offline { + ensure_client(common, api_client).await; } let use_public_proxy = api_client .as_ref() @@ -797,7 +779,7 @@ impl VendoredBackend<'_> { &mut state, &candidate.purl, candidate.entry.clone(), - candidate.detached, + candidate.entry.detached, &candidate.record, ) .await @@ -871,10 +853,6 @@ async fn npm_artifact_committable( } } -fn workspace_copy_issue(health: &ArtifactHealth) -> bool { - health.is_workspace_copy_issue() -} - /// Preserve package originals while adopting/rebuilding every member-relative /// copy from a canonical tarball whose whole-file fingerprint is trusted. async fn repair_workspace_copies( @@ -904,12 +882,22 @@ async fn repair_workspace_copies( Ok(changed) } +/// The phase's one API client, built on first need: construction prints +/// the token-shape stderr advisory, so it must happen at most once, and +/// not at all in a run with nothing to fetch. +async fn ensure_client<'c>(common: &GlobalArgs, slot: &'c mut Option) -> &'c ApiClient { + if slot.is_none() { + *slot = Some( + get_api_client_with_overrides(common.api_client_overrides()) + .await + .0, + ); + } + slot.as_ref().unwrap() +} + /// Fetch one patch view by uuid (proxy-aware) and shape it as a manifest -/// record; `None` offline or on any API failure. `client_cache` holds the -/// one API client the whole vendored-artifact phase shares — construction -/// re-prints the token-shape stderr advisory, so N uuid lookups must not -/// print it N times. Built lazily: a run with nothing to look up never -/// constructs (or warns) at all. +/// record; `None` offline or on any API failure. async fn fetch_record_by_uuid( common: &GlobalArgs, client_cache: &mut Option, @@ -918,16 +906,7 @@ async fn fetch_record_by_uuid( if common.offline { return None; } - if client_cache.is_none() { - *client_cache = Some( - get_api_client_with_overrides(common.api_client_overrides()) - .await - .0, - ); - } - let client = client_cache - .as_ref() - .expect("client_cache was just initialized above"); + let client = ensure_client(common, client_cache).await; let patch = client.fetch_patch(uuid).await.ok()??; Some(socket_patch_core::manifest::records::record_from_patch_response(&patch)) } @@ -1018,12 +997,11 @@ mod tests { /// A FIFO under a wiring-file name (here the paired `