wip clean up

Mikola Lysenko (mikolalysenko) · Jul 7, 2026 · Jul 2, 2026 · Jul 2, 2026 · Jul 2, 2026 · Jul 2, 2026
commit 05d8d6663595a5ecb7a126ea3e971853a8e66415
diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs
@@ -659,7 +659,7 @@ async fn synthesize_go_patches(
     use socket_patch_core::patch::go_mod_edit::{
         read_replace_entries, ReplaceOwner, GO_PATCHES_DIR,
     };
-    use socket_patch_core::patch::go_redirect::are_safe_redirect_coords;
+    use socket_patch_core::patch::go_redirect::{are_safe_redirect_coords, copy_dir_for};
     use socket_patch_core::utils::purl::build_golang_purl;
 
     let mut go_patches = HashMap::new();
@@ -688,10 +688,7 @@ async fn synthesize_go_patches(
         }
         go_patches.insert(
             purl,
-            common
-                .cwd
-                .join(GO_PATCHES_DIR)
-                .join(format!("{}@{version}", entry.module)),
+            copy_dir_for(&common.cwd, GO_PATCHES_DIR, &entry.module, version),
         );
     }
     go_patches

diff --git a/crates/socket-patch-cli/tests/e2e_safety_cow.rs b/crates/socket-patch-cli/tests/e2e_safety_cow.rs
@@ -125,7 +125,7 @@ fn assert_applied(env: &serde_json::Value, purl: &str, expected_paths: &[&str])
 /// Assert no patch-time temp files leaked into `pkg_dir`.
 ///
 /// Two distinct stagers write into the package directory:
-///   * the atomic writer (`apply::write_atomic`) stages `.socket-stage-*`,
+///   * the atomic writer (`utils::fs::atomic_write_bytes`) stages `.socket-stage-*`,
 ///   * **CoW** (`cow::write_via_stage_rename`, the hardlink and symlink
 ///     branches) stages `.socket-cow-*`.
 ///

diff --git a/crates/socket-patch-cli/tests/e2e_safety_internals.rs b/crates/socket-patch-cli/tests/e2e_safety_internals.rs
@@ -17,8 +17,6 @@
 //! No network. No toolchain. Unix-gated for the chmod-based test;
 //! the rest are portable.
 
-use std::collections::HashMap;
-
 use socket_patch_core::patch::cow::{break_hardlink_if_needed, CowAction};
 use socket_patch_core::patch::sidecars::dispatch_fixup;
 
@@ -44,7 +42,7 @@ use socket_patch_core::patch::sidecars::dispatch_fixup;
 #[tokio::test]
 async fn dispatch_fixup_empty_patched_returns_none() {
     let tmp = tempfile::tempdir().unwrap();
-    let out = dispatch_fixup("pkg:pypi/requests@2.28.0", tmp.path(), &[], &HashMap::new())
+    let out = dispatch_fixup("pkg:pypi/requests@2.28.0", tmp.path(), &[])
         .await
         .unwrap();
     assert!(
@@ -63,7 +61,6 @@ async fn dispatch_fixup_unknown_ecosystem_returns_none() {
         "pkg:totally-not-an-ecosystem/x@1",
         tmp.path(),
         &["x".to_string()],
-        &HashMap::new(),
     )
     .await
     .unwrap();
@@ -107,7 +104,6 @@ async fn dispatch_fixup_cargo_sha256_file_failure_arm() {
         "pkg:cargo/anything@1.0.0",
         pkg,
         &["package/missing-on-disk.txt".to_string()],
-        &HashMap::new(),
     )
     .await;
 
@@ -153,7 +149,6 @@ async fn dispatch_fixup_nuget_with_nonexistent_pkg_path() {
         "pkg:nuget/Anything@1.0.0",
         &absent,
         &["package/file.txt".to_string()],
-        &HashMap::new(),
     )
     .await
     .unwrap();

diff --git a/crates/socket-patch-core/src/package_json/find.rs b/crates/socket-patch-core/src/package_json/find.rs
@@ -72,16 +72,13 @@ pub async fn find_package_json_files(start_path: &Path) -> PackageJsonFindResult
             if root_exists {
                 let mut nested = Vec::new();
                 search_recursive(start_path, 0, 5, &mut nested).await;
-                results.extend(
-                    nested
-                        .into_iter()
-                        .filter(|p| *p != root_package_json)
-                        .map(|path| PackageJsonLocation {
-                            path,
-                            is_root: false,
-                            is_workspace: false,
-                        }),
-                );
+                results.extend(nested.into_iter().filter(|p| *p != root_package_json).map(
+                    |path| PackageJsonLocation {
+                        path,
+                        is_root: false,
+                        is_workspace: false,
+                    },
+                ));
             }
         }
         _ => {

diff --git a/crates/socket-patch-core/src/patch/apply.rs b/crates/socket-patch-core/src/patch/apply.rs
@@ -45,14 +45,13 @@ pub struct VerifyResult {
 /// content or fails, never silently corrupted. What tolerance can do is
 /// discard local modifications to the dependency file, which is why
 /// `Strict` exists.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum MismatchPolicy {
     /// DEFAULT: a beforeHash mismatch is overwritten with the verified
     /// patched content and surfaced as a warning (the promoted
     /// [`VerifyResult`] keeps `expected_hash`/`current_hash`, which is
     /// how callers detect and report it). A MISSING pre-existing file is
     /// still a hard error.
-    #[default]
     Warn,
     /// A beforeHash mismatch is a hard error (`--strict`).
     Strict,
@@ -103,9 +102,10 @@ pub struct PatchSources<'a> {
 
 impl<'a> PatchSources<'a> {
     /// Construct a `PatchSources` that only knows about the legacy
-    /// per-file blob directory. Convenient for tests and existing call
-    /// sites that have not been upgraded.
-    pub fn blobs_only(blobs_path: &'a Path) -> Self {
+    /// per-file blob directory. All remaining callers are same-crate
+    /// tests, hence the `cfg(test)` gate.
+    #[cfg(test)]
+    pub(crate) fn blobs_only(blobs_path: &'a Path) -> Self {
         Self {
             blobs_path,
             packages_path: None,
@@ -141,7 +141,7 @@ pub struct ApplyResult {
 /// Normalize file path by removing the "package/" prefix if present.
 /// Patch files come from the API with paths like "package/lib/file.js"
 /// but we need relative paths like "lib/file.js" for the actual package directory.
-pub fn normalize_file_path(file_name: &str) -> &str {
+pub(crate) fn normalize_file_path(file_name: &str) -> &str {
     const PACKAGE_PREFIX: &str = "package/";
     if let Some(stripped) = file_name.strip_prefix(PACKAGE_PREFIX) {
         stripped
@@ -160,7 +160,7 @@ pub fn normalize_file_path(file_name: &str) -> &str {
 /// execution) via `pkg_path.join(key)` — `Path::join` discards the base on an
 /// absolute key, and `..` components walk out. We reject anything that isn't a
 /// plain relative path (no absolute/root/prefix components, no `..`, no NUL).
-pub fn is_safe_relative_subpath(normalized: &str) -> bool {
+pub(crate) fn is_safe_relative_subpath(normalized: &str) -> bool {
     use std::path::Component;
     if normalized.is_empty() || normalized.contains('\0') {
         return false;
@@ -373,7 +373,7 @@ pub async fn select_installed_variants(
 /// set on new files to honor the read-only-by-default policy.
 ///
 /// Writes the patched content and verifies the resulting hash.
-pub async fn apply_file_patch(
+pub(crate) async fn apply_file_patch(
     pkg_path: &Path,
     file_name: &str,
     patched_content: &[u8],
@@ -447,9 +447,10 @@ pub async fn apply_file_patch(
     // before we mutate. No-op on regular private files (single
     // syscall). See `patch::cow`.
     //
-    // Atomic write: stage in the parent directory, fsync, rename onto
-    // the target. POSIX `rename(2)` is atomic — observers see either
-    // the old bytes or the new bytes, never a truncated half-write.
+    // Atomic write (`utils::fs::atomic_write_bytes`): stage in the
+    // parent directory, fsync, rename onto the target. POSIX
+    // `rename(2)` is atomic — observers see either the old bytes or
+    // the new bytes, never a truncated half-write.
     //
     // The stage file is created with the user's umask defaults
     // (typically 0o644) — that's how we sidestep the "existing file
@@ -462,7 +463,7 @@ pub async fn apply_file_patch(
     // restored — even if a step errors — before the failure propagates.
     let write_result = async {
         break_hardlink_if_needed(&filepath).await?;
-        write_atomic(&filepath, patched_content).await
+        crate::utils::fs::atomic_write_bytes(&filepath, patched_content).await
     }
     .await;
     dir_guard.restore().await;
@@ -553,14 +554,6 @@ async fn nearest_existing_ancestor(path: &Path) -> Option<&Path> {
     None
 }
 
-/// Write `content` to `target` atomically via stage + rename, so a patched
-/// package file is only ever seen as the complete old or complete new
-/// bytes. Delegates to the crate-wide hardened writer (permissions/chown
-/// are restored separately by [`restore_file_permissions`]).
-async fn write_atomic(target: &Path, content: &[u8]) -> std::io::Result<()> {
-    crate::utils::fs::atomic_write_bytes(target, content).await
-}
-
 /// Restore the post-write permission state on `filepath`.
 ///
 /// * `pre_patch` = `Some(meta)` → the file existed before the patch;
@@ -686,7 +679,6 @@ pub async fn apply_package_patch(
         // disk write — NOT skippable by `--force`, since a path escape is never
         // a legitimate patch target.
         if !is_safe_relative_subpath(normalize_file_path(file_name)) {
-            result.success = false;
             result.error = Some(format!(
                 "Refusing patch with unsafe file path (escapes package directory): {file_name}"
             ));
@@ -879,9 +871,7 @@ pub async fn apply_package_patch(
     // consumers see a uniform shape regardless of whether the
     // fixup succeeded, was advisory-only, or raised an error.
     if !result.files_patched.is_empty() {
-        use crate::patch::sidecars::{
-            dispatch_fixup, SidecarAdvisory, SidecarAdvisoryCode, SidecarRecord, SidecarSeverity,
-        };
+        use crate::patch::sidecars::{dispatch_fixup, fixup_failed_record};
         // Include files verified `AlreadyPatched` alongside the ones
         // written this run: a previous apply that failed partway left
         // them patched on disk but returned before this boundary, so
@@ -901,23 +891,14 @@ pub async fn apply_package_patch(
                     .map(|v| v.file.clone()),
             )
             .collect();
-        match dispatch_fixup(package_key, pkg_path, &fixup_files, files).await {
+        match dispatch_fixup(package_key, pkg_path, &fixup_files).await {
             Ok(Some(record)) => result.sidecar = Some(record),
             Ok(None) => {}
             Err(e) => {
-                let ecosystem = crate::crawlers::Ecosystem::from_purl(package_key)
-                    .map(|eco| eco.cli_name().to_string())
-                    .unwrap_or_else(|| "unknown".to_string());
-                result.sidecar = Some(SidecarRecord {
-                    purl: package_key.to_string(),
-                    ecosystem,
-                    files: Vec::new(),
-                    advisory: Some(SidecarAdvisory {
-                        code: SidecarAdvisoryCode::SidecarFixupFailed,
-                        severity: SidecarSeverity::Error,
-                        message: format!("sidecar fixup failed (patch still applied): {}", e),
-                    }),
-                });
+                result.sidecar = Some(fixup_failed_record(
+                    package_key,
+                    format!("sidecar fixup failed (patch still applied): {}", e),
+                ));
             }
         }
     }

diff --git a/crates/socket-patch-core/src/patch/bun_lock_text.rs b/crates/socket-patch-core/src/patch/bun_lock_text.rs
@@ -11,7 +11,7 @@
 
 /// The only text-lockfile version the surgery has byte-exact fixtures for
 /// (bun 1.3.x; spike pinned 1.3.14).
-pub(crate) const SUPPORTED_LOCK_VERSION: u64 = 1;
+const SUPPORTED_LOCK_VERSION: u64 = 1;
 
 /// One parsed single-line packages entry.
 pub(crate) struct BunEntry {
@@ -134,7 +134,7 @@ pub(crate) fn parse_entry_line(line: &str) -> Result<BunEntry, String> {
 
 /// Byte index one past the closing quote of the JSON string at the start of
 /// `s` (escape-aware).
-pub(crate) fn scan_json_string(s: &str) -> Result<usize, String> {
+fn scan_json_string(s: &str) -> Result<usize, String> {
     let bytes = s.as_bytes();
     if bytes.first() != Some(&b'"') {
         return Err("expected a quoted key".to_string());
@@ -152,13 +152,13 @@ pub(crate) fn scan_json_string(s: &str) -> Result<usize, String> {
 
 /// Byte index one past the `]` matching the `[` at the start of `s`
 /// (string- and nesting-aware).
-pub(crate) fn scan_balanced_array(s: &str) -> Result<usize, String> {
+fn scan_balanced_array(s: &str) -> Result<usize, String> {
     let bytes = s.as_bytes();
     let mut depth = 0usize;
     let mut i = 0;
     while i < bytes.len() {
         match bytes[i] {
-            b'"' => i += scan_json_string(&s[i..]).map_err(|e| e.to_string())? - 1,
+            b'"' => i += scan_json_string(&s[i..])? - 1,
             b'[' | b'{' => depth += 1,
             b']' | b'}' => {
                 depth -= 1;
@@ -175,7 +175,7 @@ pub(crate) fn scan_balanced_array(s: &str) -> Result<usize, String> {
 
 /// Split the tuple interior at top-level commas into verbatim trimmed
 /// element substrings.
-pub(crate) fn split_top_level(interior: &str) -> Result<Vec<String>, String> {
+fn split_top_level(interior: &str) -> Result<Vec<String>, String> {
     let bytes = interior.as_bytes();
     let mut elems = Vec::new();
     let mut depth = 0usize;

diff --git a/crates/socket-patch-core/src/patch/copy_tree.rs b/crates/socket-patch-core/src/patch/copy_tree.rs
@@ -1,10 +1,8 @@
-//! Shared tree-copy helpers for the project-local Go `replace`-redirect backend
-//! ([`crate::patch::go_redirect`]). It materialises a project-local **patched
-//! copy** of a module by copying its pristine source out of the read-only,
-//! checksum-verified module cache into a writable dir under `.socket/`, then
-//! patching the copy in place.
-//!
-//! Only compiled when the Go redirect backend is enabled (gated in `mod.rs`).
+//! Shared tree-copy helpers used by the Go `replace`-redirect backend
+//! ([`crate::patch::go_redirect`]) and the vendor backends. They materialise a
+//! project-local **patched copy** of a package by copying its pristine source
+//! out of a read-only registry/module cache into a writable dir under
+//! `.socket/`, then patching the copy in place.
 
 use std::path::Path;
 
@@ -60,12 +58,12 @@ pub(crate) async fn fresh_copy(
         Ok(())
     })
     .await
-    .map_err(|e| std::io::Error::other(e.to_string()))?
+    .map_err(to_io)?
 }
 
 /// Recursively remove a tree, retrying once after relaxing perms (a previously
 /// patched copy may carry read-only file modes copied from the registry/cache).
-pub(crate) fn force_remove_dir_all(dir: &Path) -> std::io::Result<()> {
+fn force_remove_dir_all(dir: &Path) -> std::io::Result<()> {
     match std::fs::remove_dir_all(dir) {
         Ok(()) => Ok(()),
         Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
@@ -107,7 +105,7 @@ pub async fn remove_tree(dir: &Path) -> std::io::Result<()> {
     let dir = dir.to_path_buf();
     tokio::task::spawn_blocking(move || force_remove_dir_all(&dir))
         .await
-        .map_err(|e| std::io::Error::other(e.to_string()))?
+        .map_err(to_io)?
 }
 
 #[cfg(test)]
@@ -273,8 +271,8 @@ mod tests {
     /// Regression: the perm-relax retry in [`force_remove_dir_all`] must not
     /// chmod *through* a symlink. `set_permissions` follows links, so a symlink
     /// entry would silently mutate its target's mode — which can live outside
-    /// the tree. (Copy trees are symlink-free today, but this is a general
-    /// pub(crate) helper and the safety property must hold regardless.)
+    /// the tree. (Copy trees are symlink-free today, but [`remove_tree`] is a
+    /// general pub helper and the safety property must hold regardless.)
     #[cfg(unix)]
     #[tokio::test]
     async fn relax_loop_must_not_chmod_external_symlink_target() {

diff --git a/crates/socket-patch-core/src/patch/cow.rs b/crates/socket-patch-core/src/patch/cow.rs
@@ -25,7 +25,7 @@
 //! `GetFileInformationByHandle` via `windows-sys` for full Windows
 //! parity.
 
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 /// Outcome of [`break_hardlink_if_needed`].
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -79,7 +79,8 @@ pub async fn break_hardlink_if_needed(path: &Path) -> std::io::Result<CowAction>
         // target, a crash), the original would be gone with nothing to
         // roll back to. The rename-over-symlink is a single atomic
         // step — on any failure `path` still holds the original link.
-        // This mirrors the hardlink branch below and `write_atomic`.
+        // This mirrors the hardlink branch below and the apply path's
+        // `utils::fs::atomic_write_bytes`.
         write_via_stage_rename(path, &target_bytes).await?;
         return Ok(CowAction::BrokeSymlink);
     }
@@ -110,14 +111,11 @@ pub async fn break_hardlink_if_needed(path: &Path) -> std::io::Result<CowAction>
 /// `path`. Cross-FS-safe because the stage lives in the same
 /// directory as the target, so `rename(2)` is intra-filesystem.
 async fn write_via_stage_rename(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
-    // Preconditions: cow callers always pass a real file path
-    // inside a package directory, so `path.parent()` and
-    // `path.file_name()` are guaranteed `Some`. The previous
-    // `unwrap_or_else` defaults only fired on `path == "/"`,
-    // which cow can never reach (lstat on "/" returns a directory,
-    // and the hardlink branch's `read("/")` errors out long
-    // before we get here). Using `.expect()` documents the
-    // invariant and eliminates the dead defensive default.
+    // Cow callers always pass a real file path inside a package
+    // directory, so `path.parent()` and `path.file_name()` are
+    // guaranteed `Some`: the only counterexample, `path == "/"`,
+    // is unreachable (lstat on "/" reports a directory, and the
+    // hardlink branch's `read("/")` errors long before we get here).
     let parent = path
         .parent()
         .expect("cow stage path always has a parent — callers pass package-internal files");
@@ -127,13 +125,13 @@ async fn write_via_stage_rename(path: &Path, bytes: &[u8]) -> std::io::Result<()
     // but defense in depth.)
     let stem = path
         .file_name()
-        .map(|n| n.to_string_lossy().into_owned())
-        .expect("cow stage path always has a file_name — callers pass package-internal files");
-    let stage: PathBuf = parent.join(format!(".socket-cow-{}-{}", stem, uuid::Uuid::new_v4()));
+        .expect("cow stage path always has a file_name — callers pass package-internal files")
+        .to_string_lossy();
+    let stage = parent.join(format!(".socket-cow-{}-{}", stem, uuid::Uuid::new_v4()));
     // Stage write. If this fails *after* creating the file (e.g. a
     // mid-write ENOSPC), the partial stage would otherwise leak as a
     // `.socket-cow-*` turd, so clean it up before propagating — same
-    // discipline as `apply::write_atomic`'s write arm.
+    // discipline as `utils::fs::atomic_write_bytes`'s write arm.
     if let Err(e) = tokio::fs::write(&stage, bytes).await {
         let _ = tokio::fs::remove_file(&stage).await;
         return Err(e);