diff --git a/crates/socket-patch-cli/src/commands/hosted_bundle.rs b/crates/socket-patch-cli/src/commands/hosted_bundle.rs index 8887adb8e..56542b73e 100644 --- a/crates/socket-patch-cli/src/commands/hosted_bundle.rs +++ b/crates/socket-patch-cli/src/commands/hosted_bundle.rs @@ -10,7 +10,7 @@ //! Stdin: `{"files": {path: text}, "binaryFiles"?: {path: base64}, //! "presentOnly"?: [path], "symlinks"?: [path], "projectRoots"?: [dir], //! "pipenvMajor"?: n, "batchSize"?: n, "noSocketYml"?: bool, -//! "minSeverity"?: severity, "policyPaths"?: [path], +//! "minSeverity"?: severity, "policyPaths"?: [path], "policySha256"?: hex, //! "maxNewPatches"?: n | "none", "maxNewPatchesCap"?: n, //! "inFlightPatches"?: [purl]}`. Stdout: the engine result //! (`HostedScanResult`, binary contents base64), or @@ -122,7 +122,7 @@ pub async fn run(args: HostedBundleArgs) -> i32 { print_error("invalid_bundle", &format!("cannot read stdin: {e}")); return 2; } - let bundle: Bundle = match serde_json::from_str(&raw) { + let mut bundle: Bundle = match serde_json::from_str(&raw) { Ok(bundle) => bundle, Err(e) => { print_error("invalid_bundle", &e.to_string()); @@ -137,14 +137,14 @@ pub async fn run(args: HostedBundleArgs) -> i32 { pipenv_major: bundle.pipenv_major, trust_lockfile_config: Some(!common.no_trust_lockfile_config), npm_allow_remote_config: Some(!common.no_npm_allow_remote_config), - project_roots: bundle.project_roots.clone(), + project_roots: bundle.project_roots.take(), no_socket_yml: bundle.no_socket_yml, - min_severity: bundle.min_severity.clone(), - policy_paths: bundle.policy_paths.clone(), - policy_sha256: bundle.policy_sha256.clone(), + min_severity: bundle.min_severity.take(), + policy_paths: bundle.policy_paths.take(), + policy_sha256: bundle.policy_sha256.take(), max_new_patches: bundle.max_new_patches, max_new_patches_cap: bundle.max_new_patches_cap, - in_flight_patches: bundle.in_flight_patches.clone(), + in_flight_patches: bundle.in_flight_patches.take(), ..HostedScanOptions::default() }; let input = match build_input(bundle, options) { diff --git a/crates/socket-patch-cli/src/commands/hosted_unwind.rs b/crates/socket-patch-cli/src/commands/hosted_unwind.rs index 529e61881..73e04b282 100644 --- a/crates/socket-patch-cli/src/commands/hosted_unwind.rs +++ b/crates/socket-patch-cli/src/commands/hosted_unwind.rs @@ -1,7 +1,7 @@ //! The hosted unwind leg: restore in-scope hosted pins to their default //! upstream registry entries. A helper module, not a command: `rollback`, -//! `remove` and `vendor` (a pre-v5 vendored-over-hosted revert) all run it -//! without importing each other. +//! `remove`, scan's withdrawn-patch restore and `vendor` (a pre-v5 +//! vendored-over-hosted revert) all run it without importing each other. use socket_patch_core::patch::redirect::upstream::HostedPin; @@ -12,8 +12,6 @@ use crate::args::GlobalArgs; pub(crate) struct HostedLegOutcome { pub(crate) reverted: Vec, pub(crate) failed: Vec<(String, String)>, - /// Scoped targets whose ecosystem has no per-purl hosted revert. - pub(crate) unsupported: Vec, pub(crate) warnings: Vec<(String, String)>, pub(crate) edited_files: std::collections::BTreeSet, } @@ -42,7 +40,7 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H use socket_patch_core::patch::redirect::upstream::{ restore_upstream, PinStatus, RestoreOptions, }; - // The pins are the manifest's project's (#745): restore them there. + // The pins are the manifest's project's: restore them there. let rooted = common.at_project_root(); let common: &GlobalArgs = &rooted; @@ -76,7 +74,11 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H // rebuilt registry record is not byte-exact for every lock. bun_lockb: false, }; - let live_before = socket_patch_core::vendor::bun_lock::live_user_patch_keys(&common.cwd).await; + let live_before = if common.dry_run { + Default::default() + } else { + socket_patch_core::vendor::bun_lock::live_user_patch_keys(&common.cwd).await + }; let outcome = restore_upstream(&common.cwd, pins, &opts).await; for pin in &outcome.pins { match &pin.status { @@ -115,7 +117,7 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H out.edited_files .extend(outcome.reverted_files.iter().cloned()); // Bun's hoisted linker keeps the patched copies of the pins it no - // longer pins (#764): say so, with the install that does reinstall. + // longer pins: say so, with the install that does reinstall. // A dry run says it too, so the preview's reinstall note names // `bun install --force` like the real run's. if outcome.flush_error.is_none() { @@ -133,7 +135,7 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H )); } // A `bun patch` keyed by a hosted URL the restore took out of the - // lock stops applying (#1019). + // lock stops applying. if !common.dry_run { let live_after = bun_lock::live_user_patch_keys(&common.cwd).await; for key in live_before.difference(&live_after) { @@ -143,11 +145,9 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H )); } } - } - // pip, PDM, uv and Pipenv keep a same-version install through a plain - // install or sync (#477, #1953): name the reinstall that restores the - // upstream bytes. - if outcome.flush_error.is_none() { + // pip, PDM, uv and Pipenv keep a same-version install through a + // plain install or sync: name the reinstall that restores the + // upstream bytes. use crate::commands::pypi_reinstall::{advisory, Tool, Unwound, HOSTED_CODE}; let unwound: Vec = outcome .restored() diff --git a/crates/socket-patch-cli/src/commands/list.rs b/crates/socket-patch-cli/src/commands/list.rs index c5969455f..64821aac9 100644 --- a/crates/socket-patch-cli/src/commands/list.rs +++ b/crates/socket-patch-cli/src/commands/list.rs @@ -1,4 +1,5 @@ use clap::Args; +use socket_patch_core::ledgers::Store; use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; use socket_patch_core::patch::redirect::upstream::HostedPin; use socket_patch_core::patch::redirect::RedirectState; @@ -16,20 +17,12 @@ pub struct ListArgs { pub common: GlobalArgs, } -// Where a listed record lives (see `socket_patch_core::ledgers`): the -// manifest (agent mode); the hosted redirect ledger, where `scan --mode -// hosted` recorded its patches; or the vendor ledger, where vendored mode -// keeps every `scan`/`get --mode vendored` patch as a `detached` entry's -// embedded record (a standalone `vendor` entry's fallback copy lists too -// once no manifest entry covers it — the checkout `vex` attests from it). -use socket_patch_core::ledgers::Store as Source; - /// The display order of one purl's copies: manifest, hosted, vendored. -fn display_rank(source: Source) -> u8 { +fn display_rank(source: Store) -> u8 { match source { - Source::Manifest => 0, - Source::Hosted => 1, - Source::Vendored => 2, + Store::Manifest => 0, + Store::Hosted => 1, + Store::Vendored => 2, } } @@ -38,10 +31,10 @@ fn display_rank(source: Source) -> u8 { /// `HOSTED_MODE_LABEL`'s docs) — or `None` for a manifest entry or a hosted /// pin (which has no ledger; see [`ListEntry::lockfiles`]). Shared by the /// JSON `details` and the human `Mode:` line. -fn ledger_label(source: Source) -> Option<(&'static str, &'static str)> { +fn ledger_label(source: Store) -> Option<(&'static str, &'static str)> { match source { - Source::Manifest | Source::Hosted => None, - Source::Vendored => Some((crate::commands::VENDORED_MODE_LABEL, VENDOR_STATE_REL)), + Store::Manifest | Store::Hosted => None, + Store::Vendored => Some((crate::commands::VENDORED_MODE_LABEL, VENDOR_STATE_REL)), } } @@ -49,7 +42,7 @@ fn ledger_label(source: Source) -> Option<(&'static str, &'static str)> { struct ListEntry<'a> { purl: &'a str, record: &'a PatchRecord, - source: Source, + source: Store, /// The lockfiles wiring a hosted pin (empty for the other sources). lockfiles: &'a [String], } @@ -78,15 +71,7 @@ impl HostedListing { .find(|(k, r)| PurlKey::same(k, &pin.purl) && r.uuid == pin.uuid) .map(|(_, r)| r.clone()) }) - .unwrap_or_else(|| PatchRecord { - uuid: pin.uuid.clone(), - exported_at: String::new(), - files: Default::default(), - vulnerabilities: Default::default(), - description: String::new(), - license: String::new(), - tier: String::new(), - }); + .unwrap_or_else(|| socket_patch_core::ledgers::uuid_only_record(&pin.uuid)); HostedListing { purl: pin.purl.clone(), record, @@ -129,7 +114,7 @@ fn combined_entries<'a>( entries.extend(hosted.iter().map(|h| ListEntry { purl: &h.purl, record: &h.record, - source: Source::Hosted, + source: Store::Hosted, lockfiles: &h.lockfiles, })); entries.sort_by(|a, b| { @@ -195,7 +180,7 @@ fn build_list_envelope(entries: &[ListEntry<'_>]) -> Envelope { details["mode"] = serde_json::json!(mode); details["ledger"] = serde_json::json!(ledger); } - if entry.source == Source::Hosted { + if entry.source == Store::Hosted { details["mode"] = serde_json::json!(crate::commands::HOSTED_MODE_LABEL); details["lockfiles"] = serde_json::json!(entry.lockfiles); } @@ -264,7 +249,7 @@ fn format_entry(entry: &ListEntry<'_>, color: bool) -> String { // Same labeling rule as the JSON details. lines.push(format!(" Mode: {mode} (recorded in {ledger})")); } - if entry.source == Source::Hosted { + if entry.source == Store::Hosted { lines.push(format!( " Mode: {} (wired in {})", crate::commands::HOSTED_MODE_LABEL, @@ -334,11 +319,6 @@ pub async fn run(args: ListArgs) -> i32 { apply_env_toggles(&args.common); let manifest_path = args.common.resolved_manifest_path(); - // `read_manifest` is the single source of truth for the three error - // states: `Ok(None)` (file absent), `Err(InvalidData)` (present but - // unparseable), and any other `Err` (genuine I/O failure). No stat - // pre-check: it would report any stat failure as `manifest_not_found` - // and open a TOCTOU window. // One load of the three stores, all from the SAME project as the // manifest (see below); each keeps list's own posture. let ctx = crate::commands::context::ProjectContext::new(&args.common); @@ -370,10 +350,10 @@ pub async fn run(args: ListArgs) -> i32 { Ok(state) => state.as_ref(), Err(corrupt) => { if args.common.json { - warnings.push(RunWarning { - code: "redirect_ledger_corrupt".to_string(), - detail: corrupt.to_string(), - }); + warnings.push(RunWarning::new( + "redirect_ledger_corrupt", + corrupt.to_string(), + )); } else if !args.common.silent { eprintln!("Warning: {corrupt}"); } @@ -387,10 +367,7 @@ pub async fn run(args: ListArgs) -> i32 { let contested = inventory.contested_refusal(); if let Some(detail) = &contested { if args.common.json { - warnings.push(RunWarning { - code: "hosted_wiring_contested".to_string(), - detail: detail.clone(), - }); + warnings.push(RunWarning::new("hosted_wiring_contested", detail.clone())); } else if !args.common.silent { eprintln!("Warning: {}", crate::ui::sentence_case(detail)); } @@ -426,15 +403,24 @@ pub async fn run(args: ListArgs) -> i32 { ) }; if args.common.json { - warnings.push(RunWarning { - code: "hosted_resolution_orphaned".to_string(), - detail, - }); + warnings.push(RunWarning::new("hosted_resolution_orphaned", detail)); } else if !args.common.silent { eprintln!("Warning: {}", crate::ui::sentence_case(&detail)); } } - let vendor_state = crate::commands::vendor_state_lenient(&loaded.vendor, args.common.silent); + // A broken vendor ledger degrades to nothing-to-list here (every path + // that writes or attests from it fails closed instead), but says so. + let vendor_state = match &loaded.vendor { + Ok(state) => Some(state), + Err(e) => { + if !args.common.silent { + eprintln!( + "Warning: unreadable vendor ledger ({e}); its vendored patches are not listed" + ); + } + None + } + }; // `combined_entries` folds only real records in (a record-less legacy // vendor entry asserts no patch), so entry emptiness is the whole exit @@ -799,10 +785,6 @@ mod tests { assert_eq!(v["events"][0]["details"]["mode"], "hosted"); } - /// A vendor-ledger entry: `detached` with the embedded record when - /// `record` is given (the manifest-free vendored posture), a legacy - /// manifest-tracked entry (no record of its own) otherwise. Built from - /// the on-disk JSON shape so the fixture follows the ledger schema. fn as_state(entries: &HashMap) -> VendorState { VendorState { entries: entries.clone(), @@ -810,6 +792,10 @@ mod tests { } } + /// A vendor-ledger entry: `detached` with the embedded record when + /// `record` is given (the manifest-free vendored posture), a legacy + /// manifest-tracked entry (no record of its own) otherwise. Built from + /// the on-disk JSON shape so the fixture follows the ledger schema. fn vendor_entry(purl: &str, record: Option) -> VendorEntry { serde_json::from_value(serde_json::json!({ "ecosystem": "npm", @@ -877,7 +863,7 @@ mod tests { ("pkg:npm/minimist@1.2.2", "vendored"), ("pkg:npm/zzz-vendored@1.0.0", "vendored"), ], - "purl-sorted, manifest < hosted < vendored on a tie, record-less entries skipped: {v}" + "purl-sorted, manifest < hosted < vendored on a tie, record-less entries skipped: {v}" ); let events = v["events"].as_array().unwrap(); assert_eq!( @@ -982,15 +968,6 @@ mod tests { ); } - #[test] - fn ordering_is_deterministic_across_builds() { - // Two independent builds of the same manifest must be byte-identical. - let manifest = multi_entry_manifest(); - let a = manifest_envelope(&manifest).to_pretty_json(); - let b = manifest_envelope(&manifest).to_pretty_json(); - assert_eq!(a, b); - } - fn record_with(description: &str, license: &str, severity: &str, summary: &str) -> PatchRecord { let mut rec = sample_manifest().patches["pkg:npm/minimist@1.2.2"].clone(); rec.description = description.to_string(); @@ -1005,7 +982,7 @@ mod tests { ListEntry { purl, record, - source: Source::Manifest, + source: Store::Manifest, lockfiles: &[], } } diff --git a/crates/socket-patch-cli/src/commands/lock_cli.rs b/crates/socket-patch-cli/src/commands/lock_cli.rs index caf399a9d..d0e44bd2f 100644 --- a/crates/socket-patch-cli/src/commands/lock_cli.rs +++ b/crates/socket-patch-cli/src/commands/lock_cli.rs @@ -1,15 +1,6 @@ -//! Envelope-aware wrapper around the -//! `socket_patch_core::patch::apply_lock` advisory lock. -//! -//! Mutating subcommands (`apply`, `rollback`, `repair`, `remove`, -//! `vendor`) all need the same shape: acquire the lock at the top of -//! `run`, on contention emit a JSON envelope with `error.code: -//! "lock_held"` (status "error") (or stderr in human mode) and exit 1. This module -//! centralises that emission so the call sites stay one line each. -//! -//! The lock itself is in `socket-patch-core` (cross-crate, also used -//! by tests). This module is the CLI-side glue that knows how to -//! render the failure through the shared [`crate::json_envelope`]. +//! CLI rendering of the `socket_patch_core::patch::apply_lock` advisory +//! lock: acquire it with a waiting status line, and report a failure as a +//! `lock_held` / `lock_io` JSON envelope or an `Error:` line on stderr. use std::path::Path; use std::time::Duration; @@ -21,30 +12,9 @@ use crate::json_envelope::{Command, Envelope, EnvelopeError}; /// Try to acquire `/apply.lock` and return the guard, or /// emit a failure envelope and a non-zero exit code. /// -/// `command` selects the envelope's `command` field so downstream -/// consumers see `apply` / `rollback` / `repair` / `remove` rather -/// than a generic "lock failed". `dry_run` is plumbed through to the -/// envelope's `dry_run` field for the (rare) case where lock -/// contention happens during a dry-run apply. -/// -/// `timeout = Duration::ZERO` keeps the historical non-blocking -/// try-once shape. Positive values wait with a 100 ms backoff — -/// see `socket_patch_core::patch::apply_lock::acquire`. -/// -/// The lock's whole lifecycle lives in the core guard: `acquire` -/// creates a missing `.socket/` itself, and the returned guard's drop -/// unlinks `apply.lock` while still holding the lock, releases it, and -/// prunes an otherwise-empty `.socket/` — so no command leaves a lock -/// file (or a bare `.socket/`) behind, and this wrapper never has to -/// touch the file. An interrupted run removes the file too: the -/// `interrupt` handlers (SIGINT/SIGTERM/SIGHUP, Windows console ctrl) -/// clean up the held lock before the signal ends the process. Only an -/// uncatchable kill (SIGKILL, power loss) can leave it, and such a -/// leftover never contends: the kernel released the dead holder's -/// advisory lock along with its file handle, so the acquire reclaims the -/// file in place and removes it on exit. `Held` therefore always means a -/// *live* process. socket-patch never keeps a persistent lock file in the -/// project; one that is ever needed lives outside it. +/// `Duration::ZERO` tries once; a positive `timeout` waits for the lock. +/// The core guard owns the lock file's whole lifecycle (see +/// `socket_patch_core::patch::apply_lock`). pub(crate) fn acquire_or_emit( socket_dir: &Path, command: Command, @@ -52,15 +22,22 @@ pub(crate) fn acquire_or_emit( dry_run: bool, timeout: Duration, ) -> Result { - match acquire_with_status(socket_dir, timeout) { - Ok(guard) => Ok(guard), - Err(err) => { - let hint = failure_hint(&err, &socket_dir.join("apply.lock"), timeout); - let (code, message) = lock_failure(&err, timeout); - emit(command, json, dry_run, code, &message, &hint); - Err(1) - } + let err = match acquire_with_status(socket_dir, timeout) { + Ok(guard) => return Ok(guard), + Err(err) => err, + }; + let (code, message) = lock_failure(&err, timeout); + if json { + println!( + "{}", + error_envelope(command, dry_run, code, &message).to_pretty_json() + ); + } else { + // Errors print even under --silent ("errors only", CLI_CONTRACT.md), + // and the hint is part of the error report. + eprint!("{}", format_lock_error(socket_dir, &err, timeout)); } + Err(1) } /// [`acquire`] with the contended-wait status line: a `--lock-timeout 30` @@ -88,7 +65,19 @@ pub(crate) fn acquire_with_status( /// useful advice). fn failure_hint(err: &LockError, lock_path: &Path, timeout: Duration) -> String { match err { - LockError::Held => held_hint(lock_path, timeout), + // `Held` always means a live process (a leftover file never + // contends), so the only honest advice is to wait. + LockError::Held => { + let how = if timeout > Duration::ZERO { + "retry with a longer --lock-timeout" + } else { + "pass --lock-timeout to wait for it automatically" + }; + format!( + "Wait for it to finish, or {how}. (Lock file: {})", + lock_path.display() + ) + } LockError::Io { path, source } if source.kind() == std::io::ErrorKind::PermissionDenied => { format!( "Check that {} is writable.", @@ -125,31 +114,17 @@ fn waiting_message(lock_path: &Path, timeout: Duration) -> String { pub(crate) const HELD_RETRY_HINT: &str = "Wait for it to finish, or retry with --lock-timeout to wait for the lock."; -/// Remediation printed under a human-mode `lock_held` error. `Held` -/// always means a live process (leftover files never contend), so the -/// only honest advice is to wait; how depends on whether this run already -/// waited. -fn held_hint(lock_path: &Path, timeout: Duration) -> String { - let how = if timeout > Duration::ZERO { - "retry with a longer --lock-timeout" - } else { - "pass --lock-timeout to wait for it automatically" - }; - format!( - "Wait for it to finish, or {how}. (Lock file: {})", - lock_path.display() - ) -} - -/// The one `LockError` → (`errorCode`, message) mapping every lock -/// site renders: `Held` → `lock_held` with the wait budget spelled out -/// by [`held_message`], `Io` → `lock_io` naming the path and the OS -/// error. Callers that build their own envelope (the scan/vendor step, -/// GC, hosted) use this rather than re-deriving the strings, so the -/// contention text and the waited clause cannot drift between commands. +/// The one `LockError` → (`errorCode`, message) mapping every lock site +/// renders, so the error message cannot drift between commands. pub(crate) fn lock_failure(err: &LockError, timeout: Duration) -> (&'static str, String) { + const HELD: &str = "another socket-patch process is operating in this directory"; match err { - LockError::Held => ("lock_held", held_message(timeout)), + // A zero budget never waited, so it claims no wait. + LockError::Held if timeout > Duration::ZERO => ( + "lock_held", + format!("{HELD} (waited {})", fmt_duration(timeout)), + ), + LockError::Held => ("lock_held", HELD.to_string()), LockError::Io { path, source } => ( "lock_io", format!("failed to open lock file at {}: {}", path.display(), source), @@ -157,24 +132,8 @@ pub(crate) fn lock_failure(err: &LockError, timeout: Duration) -> (&'static str, } } -/// Human-readable description of a `lock_held` contention for the given -/// wait budget. A zero budget means the historical non-blocking -/// try-once, so we omit the "(waited …)" clause entirely. -fn held_message(timeout: Duration) -> String { - if timeout > Duration::ZERO { - format!( - "another socket-patch process is operating in this directory (waited {})", - fmt_duration(timeout) - ) - } else { - "another socket-patch process is operating in this directory".to_string() - } -} - -/// Format a wait budget for humans. Whole seconds read naturally -/// (`5s`); sub-second budgets — reachable through the library API even -/// though the CLI only ever passes whole seconds — render as -/// milliseconds rather than truncating to a misleading `0s`. +/// A wait budget for humans: whole seconds as `5s`, a sub-second budget +/// (tests pass them) as milliseconds rather than a misleading `0s`. fn fmt_duration(d: Duration) -> String { if d.subsec_nanos() == 0 { format!("{}s", d.as_secs()) @@ -183,11 +142,8 @@ fn fmt_duration(d: Duration) -> String { } } -/// Build the top-level error envelope emitted in `--json` mode when a -/// command fails before doing real work (lock acquisition here; `repair` -/// reuses it for its early error exits). Split out from [`emit`] so the -/// serialized shape (status / error.code / command / dryRun) is -/// unit-testable without capturing stdout. +/// The `--json` error envelope of a command that fails before doing real +/// work (lock acquisition here; `repair` and `update` reuse it). pub(crate) fn error_envelope( command: Command, dry_run: bool, @@ -200,21 +156,6 @@ pub(crate) fn error_envelope( env } -fn emit(command: Command, json: bool, dry_run: bool, code: &str, message: &str, hint: &str) { - if json { - println!( - "{}", - error_envelope(command, dry_run, code, message).to_pretty_json() - ); - } else { - // Errors print even under --silent ("errors only", never "nothing" - // — CLI_CONTRACT.md): exit 1 with no message would be - // undiagnosable. The remediation hint is part of the error report, - // not informational chatter, so it prints with the error. - eprint!("{}", format_human_error(message, hint)); - } -} - /// The human-mode error report: `Error: ` (first letter /// capitalized; the envelope keeps the message verbatim), then the /// indented hint when there is one. @@ -266,9 +207,7 @@ mod tests { assert!(!socket.exists(), "empty .socket/ must be pruned on release"); } - /// A file squatting where `.socket/` should be still surfaces as - /// `lock_io` / exit 1 — the acquire-mkdirs change did not turn - /// genuine faults into silent successes. + /// A file squatting where `.socket/` should be is still a failure. #[test] fn acquire_or_emit_returns_one_when_socket_dir_is_a_file() { let dir = tempfile::tempdir().unwrap(); @@ -279,10 +218,7 @@ mod tests { assert_eq!(code, 1); } - /// Positive timeout waits then errors `lock_held` — confirms the - /// budget is plumbed through to `acquire`. Mirrors the - /// `apply_lock::tests::timeout_held` shape so a regression in - /// either layer surfaces here. + /// A positive timeout waits before it gives up. #[test] fn acquire_or_emit_honors_lock_timeout() { let dir = tempfile::tempdir().unwrap(); @@ -309,10 +245,7 @@ mod tests { /// A leftover lock file from a crashed run never contends — the /// kernel released the dead holder's advisory lock along with its /// file handle, so a plain acquire reclaims the file in place and - /// the guard's drop removes it. This is the fact that made - /// `--break-lock` redundant (and, with it, the `unlock` - /// subcommand): there is no stale-lock state a user ever needs to - /// clear before running a mutating command. + /// the guard's drop removes it. #[test] fn acquire_or_emit_reclaims_stale_leftover_file() { let dir = tempfile::tempdir().unwrap(); @@ -336,8 +269,7 @@ mod tests { ); } - /// Regression guard carried over from the `--break-lock` era: the - /// wrapper must never open a window in which a competitor can be + /// The wrapper must never open a window in which a competitor can be /// robbed of a lock it legitimately acquired: a competitor that /// flocked (or had merely *opened*) the file before an unlink keeps a /// valid lock on the orphaned inode while a re-acquire locks a fresh @@ -504,21 +436,6 @@ mod tests { assert_eq!(format_human_error("", ""), "Error: \n"); } - #[test] - fn held_hint_depends_on_whether_we_already_waited() { - let lock = Path::new("proj/.socket/apply.lock"); - assert_eq!( - held_hint(lock, Duration::ZERO), - "Wait for it to finish, or pass --lock-timeout to wait for it \ - automatically. (Lock file: proj/.socket/apply.lock)" - ); - assert_eq!( - held_hint(lock, Duration::from_secs(2)), - "Wait for it to finish, or retry with a longer --lock-timeout. \ - (Lock file: proj/.socket/apply.lock)" - ); - } - #[test] fn waiting_message_names_lock_and_budget() { assert_eq!( @@ -531,38 +448,17 @@ mod tests { ); } - /// Whole-second budgets read naturally in the contention message. + /// A sub-second budget renders as milliseconds, not `0s`, and a zero + /// budget claims no wait. #[test] - fn held_message_reports_whole_seconds() { + fn lock_failure_renders_the_waited_budget() { assert_eq!( - held_message(Duration::from_secs(5)), - "another socket-patch process is operating in this directory (waited 5s)" - ); - } - - /// Regression: `timeout.as_secs()` truncated a 250ms budget to - /// `(waited 0s)`, which read as "we didn't wait at all". Sub-second - /// budgets now surface as milliseconds. The 250ms budget mirrors - /// `acquire_or_emit_honors_lock_timeout`, so the message stays - /// honest for the exact value that test exercises. - #[test] - fn held_message_does_not_truncate_sub_second_to_zero() { - let msg = held_message(Duration::from_millis(250)); - assert!(msg.contains("250ms"), "expected ms rendering, got: {msg}"); - assert!( - !msg.contains("0s"), - "sub-second budget must not collapse to 0s: {msg}" + lock_failure(&LockError::Held, Duration::from_millis(250)).1, + "another socket-patch process is operating in this directory (waited 250ms)" ); - } - - /// A zero budget is the non-blocking try-once shape — no "(waited …)" - /// clause, since we never actually waited. - #[test] - fn held_message_zero_timeout_omits_waited_clause() { - let msg = held_message(Duration::ZERO); - assert!( - !msg.contains("waited"), - "zero budget should not claim a wait: {msg}" + assert_eq!( + lock_failure(&LockError::Held, Duration::ZERO).1, + "another socket-patch process is operating in this directory" ); } @@ -582,9 +478,8 @@ mod tests { assert_eq!(v["events"].as_array().unwrap().len(), 0); } - /// `dry_run` and `command` are plumbed through to the envelope so a - /// contention during a dry-run apply/rollback is still reported as - /// a dry run. Covers the other two reason tags too. + /// `dry_run` and `command` reach the envelope, so a contention during + /// a dry-run rollback is still reported as a dry run. #[test] fn error_envelope_propagates_dry_run_and_command() { let env = error_envelope(Command::Rollback, true, "lock_io", "open failed"); diff --git a/crates/socket-patch-cli/src/commands/mod.rs b/crates/socket-patch-cli/src/commands/mod.rs index 27d5fd9ff..99a173774 100644 --- a/crates/socket-patch-cli/src/commands/mod.rs +++ b/crates/socket-patch-cli/src/commands/mod.rs @@ -15,6 +15,7 @@ pub mod repair; pub mod rollback; pub mod scan; pub mod update; +pub(crate) mod user_patches; pub mod vendor; pub(crate) mod vendor_orphans; pub(crate) mod vendored_backend; @@ -384,44 +385,14 @@ pub(crate) fn hosted_state_from_pins( ) -> socket_patch_core::patch::redirect::RedirectState { let mut state = socket_patch_core::patch::redirect::RedirectState::new(); for pin in pins { - state.records.entry(pin.purl.clone()).or_insert_with(|| { - socket_patch_core::manifest::schema::PatchRecord { - uuid: pin.uuid.clone(), - exported_at: String::new(), - files: Default::default(), - vulnerabilities: Default::default(), - description: String::new(), - license: String::new(), - tier: String::new(), - } - }); + state + .records + .entry(pin.purl.clone()) + .or_insert_with(|| socket_patch_core::ledgers::uuid_only_record(&pin.uuid)); } state } -/// Read-only lenient view of a loaded vendor ledger (`.socket/vendor/state.json`): -/// missing → an empty ledger; malformed/unreadable → `None` with the -/// problem surfaced on stderr unless `silent`. A read-only -/// consumer (`list`) degrades a broken ledger to nothing-to-consult but -/// must say so, while every path that writes or attests from it fails -/// closed instead. -pub(crate) fn vendor_state_lenient( - loaded: &std::io::Result, - silent: bool, -) -> Option<&socket_patch_core::vendor::state::VendorState> { - match loaded { - Ok(state) => Some(state), - Err(e) => { - if !silent { - eprintln!( - "Warning: unreadable vendor ledger ({e}); its vendored patches are not listed" - ); - } - None - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -485,6 +456,29 @@ mod tests { .unwrap(); } + /// The usage error for agent-mode patches beside a vendor ledger in + /// the project at `root`. + fn agent_and_vendored(root: &Path) -> String { + format!( + "{} holds both agent-mode patches ({}) and vendored patches \ + (.socket/vendor/state.json): pass --mode agent, --mode vendored or \ + --mode hosted to choose the mode this run uses", + root.display(), + root.join(".socket/manifest.json").display(), + ) + } + + /// The usage error for hosted pins (`named`) beside a vendor ledger in + /// the project at `root`. + fn hosted_and_vendored(root: &Path, named: &str) -> String { + format!( + "{} holds both hosted pins in its dependency files ({named}) and vendored \ + patches (.socket/vendor/state.json): pass --mode hosted, --mode vendored \ + or --mode agent to choose the mode this run uses", + root.display(), + ) + } + fn args(cwd: &Path, manifest_path: &str) -> crate::args::GlobalArgs { crate::args::GlobalArgs { cwd: PathBuf::from(cwd), @@ -513,8 +507,7 @@ mod tests { write_ledger(tmp.path()).await; write_manifest(tmp.path(), &[LEDGER_PURL, "pkg:npm/is-odd@3.0.1"]); let mode = mode_from_project_state(&args(tmp.path(), ".socket/manifest.json")).await; - let err = mode.expect_err("agent and vendored state together"); - assert!(err.contains("--mode"), "{err}"); + assert_eq!(mode, Err(agent_and_vendored(tmp.path()))); } /// A malformed manifest is agent state: a bare run must not take the @@ -532,7 +525,10 @@ mod tests { // Beside a vendor ledger it is ambiguous, not silently vendored. write_ledger(tmp.path()).await; - assert!(mode_from_project_state(&common).await.is_err()); + assert_eq!( + mode_from_project_state(&common).await, + Err(agent_and_vendored(tmp.path())) + ); } /// A `package-lock.json` at `root` pinning `name@version` to Socket's @@ -571,9 +567,10 @@ mod tests { write_ledger(tmp.path()).await; write_hosted_lock(tmp.path(), "minimist", "1.2.2"); let mode = mode_from_project_state(&args(tmp.path(), ".socket/manifest.json")).await; - let err = mode.expect_err("vendored and hosted state together"); - assert!(err.contains("hosted"), "{err}"); - assert!(err.contains("--mode"), "{err}"); + assert_eq!( + mode, + Err(hosted_and_vendored(tmp.path(), "pkg:npm/minimist@1.2.2")) + ); } /// A hosted pin over the very package the ledger holds is still a @@ -584,7 +581,10 @@ mod tests { write_ledger(tmp.path()).await; write_hosted_lock(tmp.path(), "left-pad", "1.3.0"); let mode = mode_from_project_state(&args(tmp.path(), ".socket/manifest.json")).await; - assert!(mode.is_err(), "{mode:?}"); + assert_eq!( + mode, + Err(hosted_and_vendored(tmp.path(), "pkg:npm/left-pad@1.3.0")) + ); } /// A lockless hosted pin (a nuget source no lock records yet) is @@ -672,4 +672,3 @@ mod tests { assert_eq!(mode, Ok(scan::ScanMode::Vendored)); } } -pub(crate) mod user_patches; diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 3299a891c..6d9a3c040 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1256,16 +1256,6 @@ fn build_rollback_envelope(report: &RollbackReport<'_>) -> Envelope { }; env.record(event.with_details(hosted())); } - for purl in &leg.unsupported { - env.record( - PatchEvent::new(PatchAction::Failed, purl.clone()) - .with_error( - "hosted_unsupported", - "this ecosystem has no per-package hosted restore", - ) - .with_details(hosted()), - ); - } if let Some(refusal) = report.contested { env.record( PatchEvent::artifact(PatchAction::Failed) @@ -2332,13 +2322,12 @@ pub async fn run(args: RollbackArgs) -> i32 { // Not-installed entries never flip the exit code (see // `RollbackOutcome`). Everything that leaves the system still // patched DOES: agent failures, vendored drift-keeps and - // failures, hosted refusals/unsupported targets, corrupt + // failures, hosted refusals, corrupt // ledgers, and a failed manifest write. let success = agent_success && vendored_leg.kept.is_empty() && vendored_leg.failed.is_empty() && hosted_leg.failed.is_empty() - && hosted_leg.unsupported.is_empty() && contested.is_none() && !vendor_corrupt && manifest_write_failed.is_none();