Finish vendored reverts through one shared step with a keep policy (#989) (#1245)

* Start refactor for #989

Assisted-by: Claude Code:claude-opus-5-5

* Finish vendored reverts through one shared step

Every vendored backend copied the same revert finish sequence: return
on a dry run, keep the artifact when a record drifted, return under
--preserve-state, then delete the uuid dir and prune empty vendor
levels. vendor::revert::finish now owns that sequence, and each
backend names its keep rule as a KeepPolicy (OnDrift,
OnDriftWhileReferenced, NpmFamily) instead of re-deciding it.

gem, composer, Maven legacy, NuGet and pnpm move onto the helper and
their copies are deleted. Revert output, warnings, error messages and
exit codes are unchanged.

Refs #989

Assisted-by: Claude Code:claude-opus-5-5

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mikolalysenko

crates/socket-patch-core/src/vendor/composer_lock.rs

@@ -44,7 +44,6 @@ use std::sync::Arc;
 
 use serde_json::{json, Map, Value};
 
-use crate::constants::SOCKET_DIR;
 use crate::manifest::schema::PatchRecord;
 use crate::patch::apply::PatchSources;
 use crate::patch::copy_tree::remove_tree;
@@ -53,16 +52,16 @@ use crate::utils::composer_version::composer_versions_equivalent;
 use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string};
 use crate::utils::line_endings::LineEndings;
 use crate::utils::purl::{build_composer_purl, parse_composer_purl};
-use crate::utils::socket_dir::remove_tree_and_prune;
 
 use super::common::{
-    already_patched_result, any_live_file_references, copy_matches_after_hashes, done,
-    inventory_or_warn, prune_empty_vendor_levels, refused, serialize_json,
-    service_offline_conflict, stage_dir_for, swap_stage_into_place, synthesized_result,
+    already_patched_result, copy_matches_after_hashes, done, inventory_or_warn,
+    prune_empty_vendor_levels, refused, serialize_json, service_offline_conflict, stage_dir_for,
+    swap_stage_into_place, synthesized_result,
 };
 use super::parse_memo::ParseMemo;
 use super::path::{parse_vendor_path, vendor_uuid_dir_rel};
 use super::registry_fetch::{extract_on_blocking_pool, extract_zip};
+use super::revert::{self, KeepPolicy};
 use super::service_fetch::{
     claim_prestaged, fetch_verified_archive, ServiceAttempt, ServicePolicy, ServiceTerminal,
 };
@@ -528,7 +527,6 @@ pub async fn revert_composer_opts(
             entry.uuid
         ));
     };
-    let uuid_dir = project_root.join(&uuid_dir_rel);
     let lock_path = project_root.join(COMPOSER_LOCK);
     let mut warnings = Vec::new();
 
@@ -584,32 +582,18 @@ pub async fn revert_composer_opts(
         }
     }
 
-    let mut outcome = RevertOutcome {
+    let outcome = RevertOutcome {
         kept_artifact: false,
         success: true,
         warnings,
         error: None,
     };
-    if !dry_run {
-        if outcome.drift_skipped()
-            && any_live_file_references(project_root, &[COMPOSER_LOCK], &uuid_dir_rel).await
-        {
-            // Drift-keep (see the fn doc): never delete a uuid dir the live
-            // lock still routes composer at.
-            outcome.keep_artifact(&uuid_dir_rel);
-        } else if !keep_artifact {
-            // `--preserve-state` (`keep_artifact`) skips only this deletion:
-            // the artifact dir stays behind and the caller keeps the ledger
-            // entry. The last composer entry leaves `.socket/vendor/composer/`
-            // (and `.socket/vendor/`) empty: the shared helper prunes them so
-            // a reverted project carries no vendor residue (non-recursive:
-            // siblings keep them).
-            if let Err(e) = remove_tree_and_prune(&uuid_dir, &project_root.join(SOCKET_DIR)).await {
-                outcome.success = false;
-                outcome.error = Some(format!("failed to remove {}: {e}", uuid_dir.display()));
-                return outcome;
-            }
-        }
+    // Drift-keep (see the fn doc): never delete a uuid dir the live lock
+    // still routes composer at.
+    let policy = KeepPolicy::OnDriftWhileReferenced(&[COMPOSER_LOCK]);
+    let mut outcome = revert::finish(outcome, project_root, &uuid_dir_rel, opts, policy).await;
+    if outcome.error.is_some() {
+        return outcome;
     }
 
     outcome.warnings.push(VendorWarning::new(

crates/socket-patch-core/src/vendor/gem.rs

@@ -55,7 +55,6 @@ use std::path::{Path, PathBuf};
 
 use serde_json::Value;
 
-use crate::constants::SOCKET_DIR;
 use crate::formats::gem::gemfile;
 use crate::manifest::schema::PatchRecord;
 use crate::patch::apply::{ApplyResult, PatchSources};
@@ -64,7 +63,6 @@ use crate::patch::path_safety::is_safe_single_segment;
 use crate::patch::redirect::gem_line_tail_blocks_edit;
 use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string};
 use crate::utils::purl::{build_gem_purl, parse_gem_purl, purl_qualifier};
-use crate::utils::socket_dir::remove_tree_and_prune;
 
 use super::common::{
     already_patched_result, copy_matches_after_hashes, done, failed_result, inventory_or_warn,
@@ -73,6 +71,7 @@ use super::common::{
 };
 use super::path::{parse_vendor_path, vendor_uuid_dir_rel};
 use super::registry_fetch::{extract_gem_data, extract_on_blocking_pool};
+use super::revert::{self, KeepPolicy};
 use super::service_fetch::{
     claim_prestaged, fetch_verified_archive, fetch_verified_secondary, SecondaryArtifactResult,
     ServiceAttempt, ServicePolicy, ServiceTerminal,
@@ -1207,10 +1206,7 @@ pub async fn revert_gem_opts(
     project_root: &Path,
     opts: RevertOpts,
 ) -> RevertOutcome {
-    let RevertOpts {
-        dry_run,
-        keep_artifact,
-    } = opts;
+    let dry_run = opts.dry_run;
     // SECURITY: state.json is committed and tamper-able; the uuid keys the
     // directory we are about to delete. Anything but the canonical uuid
     // grammar is rejected fail-closed before any disk access.
@@ -1220,7 +1216,6 @@ pub async fn revert_gem_opts(
             entry.uuid
         ));
     };
-    let uuid_dir = project_root.join(&uuid_dir_rel);
     let mut warnings = Vec::new();
 
     // Fail-closed guard: an entry with NO wiring records (one an older
@@ -1297,36 +1292,22 @@ pub async fn revert_gem_opts(
         }
     }
 
-    let mut outcome = RevertOutcome {
+    let outcome = RevertOutcome {
         kept_artifact: false,
         success: true,
         warnings,
         error: None,
     };
-    if dry_run {
-        return outcome;
-    }
     // Drift-keep (see the fn doc): never delete a copy dir a left-alone
     // record may still reference.
-    if outcome.drift_skipped() {
-        outcome.keep_artifact(&uuid_dir_rel);
-        return outcome;
-    }
-    // `--preserve-state` (`keep_artifact`): the artifact dir stays behind
-    // (and the caller keeps the ledger entry), so only the deletion is
-    // skipped.
-    if keep_artifact {
-        return outcome;
-    }
-    // The last gem entry leaves `.socket/vendor/gem/` (and `.socket/vendor/`)
-    // empty: the shared helper prunes them so a reverted project carries no
-    // vendor residue (non-recursive: siblings keep them).
-    if let Err(e) = remove_tree_and_prune(&uuid_dir, &project_root.join(SOCKET_DIR)).await {
-        outcome.success = false;
-        outcome.error = Some(format!("failed to remove {}: {e}", uuid_dir.display()));
-        return outcome;
-    }
-    outcome
+    revert::finish(
+        outcome,
+        project_root,
+        &uuid_dir_rel,
+        opts,
+        KeepPolicy::OnDrift,
+    )
+    .await
 }
 
 // ── Gemfile editing ──────────────────────────────────────────────────────────

crates/socket-patch-core/src/vendor/maven_repo.rs

@@ -16,22 +16,21 @@ use serde_json::Value;
 use sha1::Sha1;
 use sha2::{Digest as _, Sha256};
 
-use crate::constants::SOCKET_DIR;
 use crate::manifest::schema::PatchRecord;
 use crate::patch::apply::PatchSources;
 use crate::utils::fs::{
     atomic_write_artifact, atomic_write_bytes_preserving_mode, read_regular_to_bytes,
     read_regular_to_string,
 };
 use crate::utils::purl::{build_maven_purl, parse_maven_purl};
-use crate::utils::socket_dir::remove_tree_and_prune;
 use crate::vendor::jvm::layout;
 
 use super::common::{
-    already_patched_result, any_live_file_references, done, failed_result, refused,
-    synthesized_result, zip_bytes_match_after_hashes,
+    already_patched_result, done, failed_result, refused, synthesized_result,
+    zip_bytes_match_after_hashes,
 };
 use super::path::vendor_uuid_dir_rel;
+use super::revert::{self, KeepPolicy};
 use super::service_fetch::{service_archive_copy, ServiceCopy};
 use super::state::{VendorArtifact, VendorEntry, WiringAction, WiringRecord};
 use super::{RevertOpts, RevertOutcome, VendorOutcome, VendorServiceConfig, VendorWarning};
@@ -222,10 +221,7 @@ pub async fn revert_maven_opts(
     project_root: &Path,
     opts: RevertOpts,
 ) -> RevertOutcome {
-    let RevertOpts {
-        dry_run,
-        keep_artifact,
-    } = opts;
+    let dry_run = opts.dry_run;
     // Routed only when EVERY record is a JVM kind; the JVM revert validates
     // the uuid, the coordinates and each recorded path before any disk
     // access (state.json is tamper-able).
@@ -241,7 +237,6 @@ pub async fn revert_maven_opts(
             entry.uuid
         ));
     };
-    let uuid_dir = project_root.join(&uuid_dir_rel);
     let mut warnings = Vec::new();
 
     // One wiring record today; reverse-order iteration keeps parity with the
@@ -280,38 +275,16 @@ pub async fn revert_maven_opts(
         }
     }
 
-    let mut outcome = RevertOutcome {
+    let outcome = RevertOutcome {
         kept_artifact: false,
         success: true,
         warnings,
         error: None,
     };
-    if dry_run {
-        return outcome;
-    }
     // Drift-keep (see the fn doc): never delete a uuid dir the live pom
     // still routes Maven at.
-    if outcome.drift_skipped()
-        && any_live_file_references(project_root, &[PROJECT_POM], &uuid_dir_rel).await
-    {
-        outcome.keep_artifact(&uuid_dir_rel);
-        return outcome;
-    }
-    // `--preserve-state` (`keep_artifact`): the artifact dir stays behind
-    // (and the caller keeps the ledger entry), so only the deletion is
-    // skipped.
-    if keep_artifact {
-        return outcome;
-    }
-    // The last maven entry leaves `.socket/vendor/maven/` (and
-    // `.socket/vendor/`) empty: the shared helper prunes them so a
-    // reverted project carries no vendor residue (non-recursive:
-    // siblings keep them).
-    if let Err(e) = remove_tree_and_prune(&uuid_dir, &project_root.join(SOCKET_DIR)).await {
-        outcome.success = false;
-        outcome.error = Some(format!("failed to remove {}: {e}", uuid_dir.display()));
-    }
-    outcome
+    let policy = KeepPolicy::OnDriftWhileReferenced(&[PROJECT_POM]);
+    revert::finish(outcome, project_root, &uuid_dir_rel, opts, policy).await
 }
 
 // ── v5 JVM backend ─────────────────────────────────────────────────

crates/socket-patch-core/src/vendor/mod.rs

@@ -95,6 +95,7 @@ mod pypi_wheel;
 pub mod redownload;
 pub mod registry_fetch;
 pub(crate) mod reuse;
+pub(crate) mod revert;
 pub(crate) mod service_fetch;
 pub mod source;
 #[cfg(any(test, feature = "test-fixtures"))]

crates/socket-patch-core/src/vendor/nuget_feed.rs

@@ -4,7 +4,6 @@ use std::sync::Arc;
 use crate::utils::digest::sha512_base64_of;
 use serde_json::Value;
 
-use crate::constants::SOCKET_DIR;
 use crate::manifest::schema::PatchRecord;
 use crate::patch::apply::{ApplyResult, PatchSources};
 use crate::patch::copy_tree::remove_tree;
@@ -13,14 +12,14 @@ use crate::utils::fs::{
     atomic_write_artifact, atomic_write_bytes_preserving_mode, read_regular_to_string,
 };
 use crate::utils::purl::{build_nuget_purl, parse_nuget_purl};
-use crate::utils::socket_dir::remove_tree_and_prune;
 
 use super::common::{
-    already_patched_result, any_live_file_references, done, prune_empty_vendor_levels,
-    read_zip_artifact, refused, synthesized_result, zip_bytes_match_after_hashes,
+    already_patched_result, done, prune_empty_vendor_levels, read_zip_artifact, refused,
+    synthesized_result, zip_bytes_match_after_hashes,
 };
 use super::parse_memo::ParseMemo;
 use super::path::vendor_uuid_dir_rel;
+use super::revert::{self, KeepPolicy};
 use super::service_fetch::{service_archive_copy, ServiceCopy};
 use super::state::{
     write_marker_or_warn, VendorArtifact, VendorEntry, VendorMarker, WiringAction, WiringRecord,
@@ -708,10 +707,7 @@ pub async fn revert_nuget_opts(
     project_root: &Path,
     opts: RevertOpts,
 ) -> RevertOutcome {
-    let RevertOpts {
-        dry_run,
-        keep_artifact,
-    } = opts;
+    let dry_run = opts.dry_run;
     // SECURITY: state.json is committed and tamper-able; the uuid keys the
     // directory we are about to delete. Anything but the canonical uuid
     // grammar is rejected fail-closed before any disk access.
@@ -721,7 +717,6 @@ pub async fn revert_nuget_opts(
             entry.uuid
         ));
     };
-    let uuid_dir = project_root.join(&uuid_dir_rel);
     let mut warnings = Vec::new();
 
     // Reverse application order: lock pin, then the (no-op) mapping audit
@@ -766,15 +761,12 @@ pub async fn revert_nuget_opts(
         }
     }
 
-    let mut outcome = RevertOutcome {
+    let outcome = RevertOutcome {
         kept_artifact: false,
         success: true,
         warnings,
         error: None,
     };
-    if dry_run {
-        return outcome;
-    }
     // Drift-keep (see the fn doc): never delete a uuid dir a live wiring
     // file still routes NuGet at. Only the root-level basenames vendor
     // records are probed (a tampered `../` path is never read).
@@ -786,27 +778,8 @@ pub async fn revert_nuget_opts(
         .collect();
     wired_files.sort_unstable();
     wired_files.dedup();
-    if outcome.drift_skipped()
-        && any_live_file_references(project_root, &wired_files, &uuid_dir_rel).await
-    {
-        outcome.keep_artifact(&uuid_dir_rel);
-        return outcome;
-    }
-    // `--preserve-state` (`keep_artifact`): the artifact dir stays behind
-    // (and the caller keeps the ledger entry), so only the deletion is
-    // skipped.
-    if keep_artifact {
-        return outcome;
-    }
-    // The last nuget entry leaves `.socket/vendor/nuget/` (and
-    // `.socket/vendor/`) empty: the shared helper prunes them so a
-    // reverted project carries no vendor residue (non-recursive:
-    // siblings keep them).
-    if let Err(e) = remove_tree_and_prune(&uuid_dir, &project_root.join(SOCKET_DIR)).await {
-        outcome.success = false;
-        outcome.error = Some(format!("failed to remove {}: {e}", uuid_dir.display()));
-    }
-    outcome
+    let policy = KeepPolicy::OnDriftWhileReferenced(&wired_files);
+    revert::finish(outcome, project_root, &uuid_dir_rel, opts, policy).await
 }
 
 // ── materialisation (service download) ─────────────────────────