Fix berry mode takeover reverting before gates (#468, #369)

crates/socket-patch-cli/src/commands/scan/hosted.rs

@@ -1637,25 +1637,32 @@ async fn vendored_takeover(
         None
     };
     // The takeover refusal (if any) for one candidate: bun gates every
-    // npm purl, berry and vlt only their own vendored entries. A refused
-    // purl is never dispatched (see the loop), so its wiring is not a
-    // write target here.
+    // npm purl, berry and vlt only their own vendored entries. Berry also
+    // runs the rewriter's per-dep grant gate (a grant without the berry
+    // cache checksum is skipped by the rewriter, so reverting first would
+    // leave the package in neither mode). A refused purl is never
+    // dispatched (see the loop), so its wiring is not a write target here.
     let takeover_refusal = |c: &Candidate,
                             entry: Option<&socket_patch_core::vendor::VendorEntry>|
-     -> Option<&socket_patch_core::patch::redirect::RewriteWarning> {
+     -> Option<socket_patch_core::patch::redirect::RewriteWarning> {
         if !c.purl.starts_with("pkg:npm/") {
             return None;
         }
+        let berry = entry.is_some_and(berry_entry);
         bun_takeover_refusal
-            .as_ref()
+            .clone()
+            .or_else(|| berry_takeover_refusal.clone().filter(|_| berry))
             .or_else(|| {
-                berry_takeover_refusal
-                    .as_ref()
-                    .filter(|_| entry.is_some_and(berry_entry))
+                berry
+                    .then(|| {
+                        socket_patch_core::patch::redirect::preflight_yarn_berry_hosted_dep(&c.dep)
+                            .err()
+                    })
+                    .flatten()
             })
             .or_else(|| {
                 vlt_takeover_refusal
-                    .as_ref()
+                    .clone()
                     .filter(|_| entry.is_some_and(vlt_entry))
             })
     };
@@ -1684,8 +1691,10 @@ async fn vendored_takeover(
         if let Some(entry) = ledger_entry {
             if let Some(warning) = takeover_refusal(candidate, Some(entry)) {
                 refused.push(purl.clone());
-                if !out.pre_warnings.iter().any(|w| w["code"] == warning.code) {
-                    out.pre_warnings.push(serde_json::json!(warning));
+                // Project-level refusals repeat per purl; report each once.
+                let warning = serde_json::json!(warning);
+                if !out.pre_warnings.contains(&warning) {
+                    out.pre_warnings.push(warning);
                 }
                 continue;
             }
@@ -1827,8 +1836,8 @@ async fn vendored_takeover(
     for purl in &refused {
         if let Some((c, entry)) = takeover.iter().find(|(c, _)| &c.purl == purl) {
             let reason = takeover_refusal(c, entry.as_ref())
-                .map_or("vendored_revert_failed", |w| w.code.as_str());
-            skipped.push(SkippedPatch::new(purl, &c.dep.patch_uuid, reason));
+                .map_or_else(|| "vendored_revert_failed".to_string(), |w| w.code);
+            skipped.push(SkippedPatch::new(purl, &c.dep.patch_uuid, &reason));
         }
     }
     // Purls leaving the rewrite set: refused takeovers, plus the dry-run

crates/socket-patch-cli/src/commands/vendor.rs

@@ -2347,17 +2347,37 @@ pub(crate) async fn vendor_records_reusing(
             // whose upstream entry cannot be restored is REFUSED; the cargo
             // backend's `hosted_redirect_live` guard backstops the rest.
             if let Some(pin) = hosted_pin_of(candidate) {
+                let origins = crate::commands::rollback::patch_server_origins(common);
+                let restore_opts = socket_patch_core::patch::redirect::upstream::RestoreOptions {
+                    dry_run: common.dry_run,
+                    offline: common.offline,
+                    patch_server_origins: origins.clone(),
+                    bun_lockb: true,
+                };
                 // The refusal the berry backend would raise after the
                 // restore, raised HERE instead — the same `failed` event,
                 // code and detail, in the dry run and the wet run alike —
                 // so the hosted wiring stays untouched.
                 if candidate.starts_with("pkg:npm/") {
-                    let refusal = berry_takeover_refusal
+                    let project = berry_takeover_refusal
                         .get_or_init(|| {
                             socket_patch_core::vendor::yarn_berry_vendor_preflight(&common.cwd)
                         })
-                        .await;
-                    if let Some((code, detail)) = refusal {
+                        .await
+                        .clone();
+                    let refusal = match project {
+                        Some(refusal) => Some(refusal),
+                        None => {
+                            socket_patch_core::vendor::yarn_berry_vendor_target_preflight(
+                                &common.cwd,
+                                candidate,
+                                pin,
+                                &restore_opts,
+                            )
+                            .await
+                        }
+                    };
+                    if let Some((code, detail)) = &refusal {
                         has_errors = true;
                         env.record(
                             PatchEvent::new(PatchAction::Failed, candidate.clone())
@@ -2367,7 +2387,6 @@ pub(crate) async fn vendor_records_reusing(
                         continue;
                     }
                 }
-                let origins = crate::commands::rollback::patch_server_origins(common);
                 let vlt_lock = socket_patch_core::utils::fs::read_regular_to_string(
                     &common
                         .cwd
@@ -2388,12 +2407,7 @@ pub(crate) async fn vendor_records_reusing(
                 let restore = socket_patch_core::patch::redirect::upstream::restore_upstream(
                     &common.cwd,
                     std::slice::from_ref(pin),
-                    &socket_patch_core::patch::redirect::upstream::RestoreOptions {
-                        dry_run: common.dry_run,
-                        offline: common.offline,
-                        patch_server_origins: origins,
-                        bun_lockb: true,
-                    },
+                    &restore_opts,
                 )
                 .await;
                 let refusal = restore

crates/socket-patch-cli/tests/in_process_vendor.rs

@@ -1031,6 +1031,13 @@ async fn berry_mixed_line_endings_fail_closed_with_code() {
 /// reference carrying the yarn-berry-zip checksum, the patch view) for the
 /// berry takeover legs. Returns the hosted tarball URL.
 async fn mount_berry_hosted_api(server: &wiremock::MockServer) -> String {
+    mount_berry_hosted_api_opts(server, true).await
+}
+
+/// [`mount_berry_hosted_api`], with the grant's `yarn-berry-zip` artifact
+/// (the `yarnBerry10c0` cache checksum) present or not. Vendored mode only
+/// uses the `tarball` artifact, so a vendorable grant can lack it.
+async fn mount_berry_hosted_api_opts(server: &wiremock::MockServer, berry_zip: bool) -> String {
     use wiremock::matchers::{method, path, path_regex};
     use wiremock::{Mock, ResponseTemplate};
     let org = "test-org";
@@ -1062,17 +1069,18 @@ async fn mount_berry_hosted_api(server: &wiremock::MockServer) -> String {
         })))
         .mount(server)
         .await;
+    let mut artifacts = vec![json!({ "kind": "tarball", "url": hosted_url,
+                                    "integrity": { "sha512": "sha512-unused-by-berry==" } })];
+    if berry_zip {
+        artifacts.push(json!({ "kind": "yarn-berry-zip", "url": hosted_url,
+            "integrity": { "yarnBerry10c0": format!("10c0/{}", "7".repeat(128)) } }));
+    }
     Mock::given(method("POST"))
         .and(path(format!("/v0/orgs/{org}/patches/package")))
         .respond_with(ResponseTemplate::new(200).set_body_json(json!({
             "results": { UUID: {
                 "status": "granted", "url": hosted_url, "purl": PURL,
-                "artifacts": [
-                    { "kind": "tarball", "url": hosted_url,
-                      "integrity": { "sha512": "sha512-unused-by-berry==" } },
-                    { "kind": "yarn-berry-zip", "url": hosted_url,
-                      "integrity": { "yarnBerry10c0": format!("10c0/{}", "7".repeat(128)) } }
-                ],
+                "artifacts": artifacts,
                 "registryOverride": null
             }}
         })))
@@ -1588,6 +1596,161 @@ async fn berry_takeovers_refuse_before_reverting_the_old_mode() {
     }
 }
 
+/// #468: a vendored→hosted takeover whose grant has no `yarnBerry10c0`
+/// cache checksum (vendored mode never needs it) must keep the package
+/// vendored. The berry rewriter skips such a dep with
+/// `redirect_yarn_berry_missing_checksum`; reverting the vendored wiring
+/// first left it patched in neither mode while the run exited 0 announcing
+/// "now fully hosted".
+#[tokio::test]
+async fn berry_vendored_to_hosted_takeover_keeps_vendored_without_berry_checksum() {
+    let server = wiremock::MockServer::start().await;
+    mount_berry_hosted_api_opts(&server, false).await;
+    let code = "redirect_yarn_berry_missing_checksum";
+    for dry in [true, false] {
+        let ctx = format!("dry={dry}");
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path();
+        stage_berry_project(root, BERRY_WIN_PKG, &berry_win_lock());
+        let (exit, env) = vendor_cli(root, &[]);
+        assert_eq!(exit, 0, "{ctx}: vendor: {env:#}");
+        let before = berry_wiring_snapshot(root);
+        let extra: &[&str] = if dry { &["--dry-run"] } else { &[] };
+        let (_, env) = hosted_scan_cli_with(root, &server.uri(), extra);
+        let text = env.to_string();
+        assert!(text.contains(code), "{ctx}: refused with {code}: {env:#}");
+        for announced in [
+            "redirect_takeover_reverted_vendored",
+            "redirect_would_revert_vendored",
+        ] {
+            assert!(
+                !text.contains(announced),
+                "{ctx}: no takeover ({announced}): {env:#}"
+            );
+        }
+        assert_eq!(env["redirect"]["redirected"], 0, "{ctx}: {env:#}");
+        let skipped = env["redirect"]["skipped"]
+            .as_array()
+            .cloned()
+            .unwrap_or_default();
+        assert!(
+            skipped
+                .iter()
+                .any(|s| s["purl"] == PURL && s["reason"] == code),
+            "{ctx}: the purl is skipped with the gate's code: {env:#}"
+        );
+        assert_eq!(
+            berry_wiring_snapshot(root),
+            before,
+            "{ctx}: the vendored wiring, ledger and artifact stay byte-identical"
+        );
+    }
+}
+
+/// #369: a hosted→vendored takeover must run the berry backend's
+/// per-package gates (another locked version of the name, a user-authored
+/// `resolutions` override) BEFORE restoring the upstream registry entry.
+/// Restoring first left the package patched in neither mode: the hosted
+/// redirect was gone and vendoring then refused with
+/// `vendor_override_conflict`.
+#[tokio::test]
+async fn berry_hosted_to_vendored_takeover_runs_package_gates_first() {
+    let server = wiremock::MockServer::start().await;
+    mount_berry_hosted_api(&server).await;
+    // The upstream entry the takeover's restore reads: the gates are
+    // evaluated on the restored files, so the restore itself must succeed.
+    mount_npm_registry(
+        &server,
+        "left-pad",
+        "1.3.0",
+        npm_tgz("left-pad", "1.3.0", ORIG_INDEX),
+    )
+    .await;
+    type Break = fn(&Path);
+    // A workspace member's lock entry for another version of the name: a
+    // name-keyed `resolutions` entry would move it too.
+    let other_version: Break = |root| {
+        let lock = std::fs::read_to_string(root.join("yarn.lock")).unwrap();
+        let extra = format!(
+            "\n\"left-pad@npm:1.1.3\":\n  version: 1.1.3\n  \
+             resolution: \"left-pad@npm:1.1.3\"\n  checksum: 10c0/{}\n  \
+             languageName: node\n  linkType: hard\n",
+            "5".repeat(128)
+        );
+        std::fs::write(root.join("yarn.lock"), lock + &extra).unwrap();
+    };
+    // A user-authored range override for the name, merged into any
+    // `resolutions` table the hosted wiring already wrote.
+    let user_resolution: Break = |root| {
+        let pkg = std::fs::read_to_string(root.join("package.json")).unwrap();
+        let mut pkg: Value = serde_json::from_str(&pkg).unwrap();
+        let table = pkg
+            .as_object_mut()
+            .unwrap()
+            .entry("resolutions")
+            .or_insert_with(|| json!({}));
+        table
+            .as_object_mut()
+            .unwrap()
+            .insert("left-pad".into(), json!("^1.0.0"));
+        let text = serde_json::to_string_pretty(&pkg).unwrap() + "\n";
+        std::fs::write(root.join("package.json"), text).unwrap();
+    };
+    for (label, breakage) in [
+        ("other locked version", other_version),
+        ("user resolutions", user_resolution),
+    ] {
+        for dry in [true, false] {
+            let ctx = format!("{label} dry={dry}");
+            let tmp = tempfile::tempdir().unwrap();
+            let root = tmp.path();
+            stage_berry_project(root, BERRY_WIN_PKG, &berry_win_lock());
+            let (exit, env) = hosted_scan_cli_with(root, &server.uri(), &[]);
+            assert_eq!(exit, 0, "{ctx}: hosted scan: {env:#}");
+            assert_eq!(env["redirect"]["redirected"], 1, "{ctx}: {env:#}");
+            breakage(root);
+            let before = berry_wiring_snapshot(root);
+            // The hosted pin's origin must count as the patch server, or
+            // the vendor run never sees it as a takeover.
+            let uri = server.uri();
+            let mut args = vec![
+                "vendor",
+                "--json",
+                "--cwd",
+                root.to_str().unwrap(),
+                "--patch-server-url",
+                &uri,
+            ];
+            if dry {
+                args.push("--dry-run");
+            }
+            let env = online_env(&uri, &uri);
+            let env: Vec<(&str, &str)> = env.iter().map(|(k, v)| (*k, v.as_str())).collect();
+            let (exit, stdout, stderr) = run_cli(root, &args, &env);
+            let text = format!("{stdout}\n{stderr}");
+            assert_ne!(exit, 0, "{ctx}: the refusal fails the run: {text}");
+            assert!(
+                text.contains("vendor_override_conflict"),
+                "{ctx}: refused with the gate's code: {text}"
+            );
+            for announced in [
+                "vendor_takeover_reverted_redirect",
+                "vendor_would_revert_redirect",
+            ] {
+                assert!(
+                    !text.contains(announced),
+                    "{ctx}: no takeover ({announced}): {text}"
+                );
+            }
+            assert_eq!(
+                berry_wiring_snapshot(root),
+                before,
+                "{ctx}: the hosted lock edits stay byte-identical"
+            );
+        }
+    }
+}
+
 // ─────────────────────────────────────────────────────────────────────
 // 9. offline with no local source
 // ─────────────────────────────────────────────────────────────────────

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

@@ -3233,6 +3233,32 @@ fn berry_cache_key(content: &str) -> Option<String> {
 /// compares the file with its own majority-normalized re-render and fails
 /// (YN0028), while a plain install rewrites every minority line — so it is
 /// refused untouched, `yarn install` normalizes it first.
+/// The grant prerequisite for creating a new yarn berry hosted pin: a dep
+/// whose grant carries no `yarnBerry10c0` cache checksum cannot be redirected
+/// (berry verifies the converted cache zip, and only the service can compute
+/// that checksum).
+///
+/// Exposed for the vendored→hosted mode takeover, like
+/// [`preflight_yarn_berry_hosted`]: vendored mode only uses the `tarball`
+/// artifact, so a vendorable patch can lack the berry checksum, and the
+/// takeover must keep such a package vendored instead of reverting it and
+/// then skipping the redirect.
+/// Keep this unconditional gate at the takeover boundary: a lock-aware
+/// rewriter may retain an already complete pin's stored checksum.
+pub fn preflight_yarn_berry_hosted_dep(dep: &DepOverride) -> Result<(), RewriteWarning> {
+    if dep.integrity.yarn_berry10c0.is_some() {
+        return Ok(());
+    }
+    Err(RewriteWarning {
+        code: "redirect_yarn_berry_missing_checksum".into(),
+        detail: format!(
+            "{}@{} has no yarnBerry10c0 cache checksum",
+            full_name(dep),
+            dep.version
+        ),
+    })
+}
+
 pub fn preflight_yarn_berry_hosted(lock: &str, yarnrc: Option<&str>) -> Result<(), RewriteWarning> {
     if !is_berry_lock(lock) {
         return Ok(());

crates/socket-patch-core/src/patch/redirect/upstream/mod.rs

@@ -337,6 +337,10 @@ pub struct RestoreOutcome {
     pub reverted_files: Vec<String>,
     /// Advisory `(code, detail)` pairs.
     pub warnings: Vec<(&'static str, String)>,
+    /// The new text of each root-relative text file the restore rewrote (or
+    /// would, on a dry run); `None` for a file it removed. Lets a caller
+    /// evaluate the restored project before anything is written.
+    pub staged_text: BTreeMap<String, Option<String>>,
     /// A write failure after every pin resolved: some files may have
     /// landed. `None` on a clean flush (and always on a dry run).
     pub flush_error: Option<String>,
@@ -649,6 +653,7 @@ pub async fn restore_upstream(
         pins: pins_out,
         reverted_files: reverted_files.into_iter().collect(),
         warnings: result.warnings,
+        staged_text: changed,
         flush_error,
     }
 }

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

@@ -128,7 +128,7 @@ pub use verify::{
 };
 // The hosted→vendored takeover refuses a berry project the backend would
 // refuse BEFORE it reverts the hosted redirect.
-pub use yarn_berry_lock::yarn_berry_vendor_preflight;
+pub use yarn_berry_lock::{yarn_berry_vendor_preflight, yarn_berry_vendor_target_preflight};
 
 use std::collections::{HashMap, HashSet};
 use std::path::Path;