Fix vendored re-runs failing on unserved upgrades (#1235)

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

@@ -117,14 +117,15 @@ pub struct VendorArgs {
 fn refusal_is_benign(code: &str) -> bool {
     matches!(code, "vendor_unsupported_ecosystem" | "already_vendored")
         // An older vendored patch kept in force (see [`keep_older_vendored_patch`]).
-        || matches!(code, vendor::VENDOR_PREBUILT_PENDING | vendor::VENDOR_PREBUILT_UNAVAILABLE)
+        || vendor::is_unserved(code)
         || socket_patch_core::vendor::jvm::sbt_gate::SKIP_CODES.contains(&code)
 }
 
-/// #954: the patch service has no artifact for `uuid` yet (still building)
-/// or at all (`build_failed`, `not_found`, …) — the backend's failed `Done`
-/// carries [`vendor::VENDOR_PREBUILT_PENDING`] /
-/// [`vendor::VENDOR_PREBUILT_UNAVAILABLE`] — while the ledger already holds
+/// #954, #1235: the patch service has no artifact for `uuid` yet (still
+/// building) or at all (`build_failed`, `not_found`, …) — the backend's
+/// refusal carries [`vendor::VENDOR_PREBUILT_PENDING`] /
+/// [`vendor::VENDOR_PREBUILT_UNAVAILABLE`] as its code (an npm-family failed
+/// `Done` as a warning) — while the ledger already holds
 /// `purl` vendored at another patch. The backend touched nothing, so that
 /// older vendoring is still in force: like hosted mode, which keeps its pin
 /// and skips the upgrade, the package is a benign skip under the unserved
@@ -133,8 +134,9 @@ fn refusal_is_benign(code: &str) -> bool {
 /// (the [`Discovery::vendor_entry_live`] verdict `vendor --check` and `vex`
 /// use) is in force: once a relock dropped its `.socket/vendor/` reference
 /// the package is patched in neither mode, so the unserved upgrade stays a
-/// failure. Any other outcome passes through, a failure minus the unserved
-/// marker (its error already says it).
+/// failure. Any other outcome passes through: an unserved refusal as the
+/// `vendor_prebuilt_required` failure it always was, a failed `Done` minus
+/// the unserved marker (its error already says it).
 ///
 /// [`Discovery::vendor_entry_live`]: socket_patch_core::vex::discover::Discovery::vendor_entry_live
 async fn keep_older_vendored_patch(
@@ -144,49 +146,57 @@ async fn keep_older_vendored_patch(
     uuid: &str,
     common: &GlobalArgs,
 ) -> Option<VendorOutcome> {
-    let Some(VendorOutcome::Done {
-        result,
-        entry,
-        mut warnings,
-    }) = outcome
-    else {
-        return outcome;
-    };
-    if !result.success {
-        if let Some(i) = warnings.iter().position(|w| {
-            matches!(
-                w.code,
-                vendor::VENDOR_PREBUILT_PENDING | vendor::VENDOR_PREBUILT_UNAVAILABLE
-            )
-        }) {
-            let unserved = warnings.remove(i);
-            if let Some(kept) = lookup_entry(&state.entries, purl).filter(|e| e.uuid != uuid) {
-                let root = common.project_root();
-                if !crate::commands::discover_wiring(common, &root)
-                    .await
-                    .vendor_entry_live(&root, kept)
-                    .await
-                {
-                    return Some(VendorOutcome::Done {
-                        result,
-                        entry,
-                        warnings,
-                    });
-                }
-                return Some(VendorOutcome::Refused {
-                    code: unserved.code,
-                    detail: format!(
-                        "kept the vendored patch {}: {} for patch {uuid}",
-                        kept.uuid, unserved.detail
-                    ),
+    let (code, detail, failed_done) = match outcome {
+        Some(VendorOutcome::Refused { code, detail }) if vendor::is_unserved(code) => {
+            (code, detail, None)
+        }
+        Some(VendorOutcome::Done {
+            result,
+            entry,
+            mut warnings,
+        }) if !result.success => {
+            let Some(i) = warnings.iter().position(|w| vendor::is_unserved(w.code)) else {
+                return Some(VendorOutcome::Done {
+                    result,
+                    entry,
+                    warnings,
                 });
-            }
+            };
+            let unserved = warnings.remove(i);
+            (
+                unserved.code,
+                unserved.detail,
+                Some((result, entry, warnings)),
+            )
+        }
+        other => return other,
+    };
+    if let Some(kept) = lookup_entry(&state.entries, purl).filter(|e| e.uuid != uuid) {
+        let root = common.project_root();
+        if crate::commands::discover_wiring(common, &root)
+            .await
+            .vendor_entry_live(&root, kept)
+            .await
+        {
+            return Some(VendorOutcome::Refused {
+                code,
+                detail: format!(
+                    "kept the vendored patch {}: {detail} for patch {uuid}",
+                    kept.uuid
+                ),
+            });
         }
     }
-    Some(VendorOutcome::Done {
-        result,
-        entry,
-        warnings,
+    Some(match failed_done {
+        Some((result, entry, warnings)) => VendorOutcome::Done {
+            result,
+            entry,
+            warnings,
+        },
+        None => VendorOutcome::Refused {
+            code: vendor::VENDOR_PREBUILT_REQUIRED,
+            detail,
+        },
     })
 }
 
@@ -299,6 +309,10 @@ async fn takeover_dry_refusal(
     .await;
     drop(probe);
     match outcome {
+        // A takeover has no older vendored patch to keep in force.
+        Some(VendorOutcome::Refused { code, detail }) if vendor::is_unserved(code) => {
+            Some((vendor::VENDOR_PREBUILT_REQUIRED, detail))
+        }
         Some(VendorOutcome::Refused { code, detail }) if !refusal_is_benign(code) => {
             Some((code, detail))
         }

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

@@ -3514,13 +3514,25 @@ fn gem_fixture() -> GemFixture {
 /// [`GEM_PURL_QUALIFIED`]); the batch response's outer package purl stays the
 /// bare purl the crawler requested, mirroring production.
 async fn mount_gem_patch_api(mock: &wiremock::MockServer, patch_purl: &str) {
+    mount_gem_patch_api_at(mock, patch_purl, GEM_UUID, None).await;
+}
+
+/// [`mount_gem_patch_api`] serving patch `uuid`. With `unserved` set, the
+/// patch service answers that status (`pending_build`, `build_failed`, …)
+/// for the patch instead of serving its prebuilt `.gem`.
+async fn mount_gem_patch_api_at(
+    mock: &wiremock::MockServer,
+    patch_purl: &str,
+    uuid: &str,
+    unserved: Option<&str>,
+) {
     use base64::Engine as _;
-    use wiremock::matchers::{method, path};
+    use wiremock::matchers::{method, path, path_regex};
     use wiremock::{Mock, ResponseTemplate};
 
     let fx = gem_fixture();
     let record: socket_patch_core::manifest::schema::PatchRecord = serde_json::from_value(json!({
-        "uuid": GEM_UUID, "exportedAt": "2026-01-01T00:00:00Z",
+        "uuid": uuid, "exportedAt": "2026-01-01T00:00:00Z",
         "files": { "lib/demo_gem.rb": { "beforeHash": compute_git_sha256_from_bytes(GEM_ORIG), "afterHash": compute_git_sha256_from_bytes(GEM_PATCHED) } },
         "vulnerabilities": {}, "description": "gem vendor patch", "license": "MIT", "tier": "free"
     })).unwrap();
@@ -3532,14 +3544,29 @@ async fn mount_gem_patch_api(mock: &wiremock::MockServer, patch_purl: &str) {
         blobs_path: fx.root(),
         mem_blobs: Some(&blobs),
     };
-    prebuilt_common::mount_record(
-        mock,
-        patch_purl,
-        &record,
-        fx.installed_lib().parent().unwrap().parent().unwrap(),
-        &sources,
-    )
-    .await;
+    match unserved {
+        None => {
+            prebuilt_common::mount_record(
+                mock,
+                patch_purl,
+                &record,
+                fx.installed_lib().parent().unwrap().parent().unwrap(),
+                &sources,
+            )
+            .await
+        }
+        Some(status) => {
+            Mock::given(method("POST"))
+                .and(path_regex(r"/(patch|patches)/package$"))
+                .respond_with(
+                    ResponseTemplate::new(200)
+                        .set_body_json(json!({ "results": { uuid: { "status": status } } })),
+                )
+                .with_priority(1)
+                .mount(mock)
+                .await
+        }
+    }
     const ORG_SLUG: &str = "test-org";
     /// The exact percent-encoded by-package path segment for the BARE purl —
     /// the spelling the crawler synthesizes and the CLI queries with (the
@@ -3555,7 +3582,7 @@ async fn mount_gem_patch_api(mock: &wiremock::MockServer, patch_purl: &str) {
             "packages": [{
                 "purl": GEM_PURL,
                 "patches": [{
-                    "uuid": GEM_UUID,
+                    "uuid": uuid,
                     "purl": patch_purl,
                     "tier": "free",
                     "cveIds": ["CVE-2026-0002"],
@@ -3574,7 +3601,7 @@ async fn mount_gem_patch_api(mock: &wiremock::MockServer, patch_purl: &str) {
         )))
         .respond_with(ResponseTemplate::new(200).set_body_json(json!({
             "patches": [{
-                "uuid": GEM_UUID,
+                "uuid": uuid,
                 "purl": patch_purl,
                 "publishedAt": "2026-01-01T00:00:00Z",
                 "description": "gem vendor patch",
@@ -3587,9 +3614,9 @@ async fn mount_gem_patch_api(mock: &wiremock::MockServer, patch_purl: &str) {
         .mount(mock)
         .await;
     Mock::given(method("GET"))
-        .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/view/{GEM_UUID}")))
+        .and(path(format!("/v0/orgs/{ORG_SLUG}/patches/view/{uuid}")))
         .respond_with(ResponseTemplate::new(200).set_body_json(json!({
-            "uuid": GEM_UUID,
+            "uuid": uuid,
             "purl": patch_purl,
             "publishedAt": "2026-01-01T00:00:00Z",
             "files": {
@@ -3674,6 +3701,101 @@ async fn scan_vendor_gem_refuses_a_gitignored_ledger() {
     }
 }
 
+/// A gem `scan --mode vendored` run at patch A, then the bytes of the
+/// files that wiring owns: Gemfile, lock, ledger and the vendored lib.
+async fn gem_vendored_at_a() -> (GemFixture, [Vec<u8>; 4]) {
+    let mock = wiremock::MockServer::start().await;
+    mount_gem_patch_api(&mock, GEM_PURL).await;
+    let fx = gem_fixture();
+    let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]);
+    assert_eq!(code, 0, "vendoring patch A must succeed: {env:#}");
+    let wired = gem_wired_bytes(&fx);
+    (fx, wired)
+}
+
+fn gem_wired_bytes(fx: &GemFixture) -> [Vec<u8>; 4] {
+    [
+        std::fs::read(fx.gemfile_path()).unwrap(),
+        std::fs::read(fx.lock_path()).unwrap(),
+        std::fs::read(fx.state_path()).unwrap(),
+        std::fs::read(fx.vendored_lib()).unwrap(),
+    ]
+}
+
+/// The superseding patch of the demo gem in the #1235 tests.
+const GEM_UUID2: &str = "36363636-3636-4636-8636-363636363636";
+
+/// #1235: the #954 skip holds for every vendored backend, not only the npm
+/// family. A gem vendored at patch A, re-scanned once the API offers a
+/// superseding patch B whose prebuilt `.gem` the service is still building
+/// (`pending_build`) or cannot serve (`build_failed`, `not_found`), keeps A
+/// in force: one `skipped` event under the unserved code naming both
+/// patches, exit 0, and the Gemfile, lock, ledger and artifact untouched.
+#[tokio::test]
+async fn scan_vendor_gem_keeps_the_older_patch_while_the_superseding_one_is_unserved() {
+    for (status, code) in [
+        ("pending_build", "vendor_prebuilt_pending"),
+        ("build_failed", "vendor_prebuilt_unavailable"),
+        ("not_found", "vendor_prebuilt_unavailable"),
+    ] {
+        let (fx, wired) = gem_vendored_at_a().await;
+        let mock = wiremock::MockServer::start().await;
+        mount_gem_patch_api_at(&mock, GEM_PURL, GEM_UUID2, Some(status)).await;
+        let (exit, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]);
+        assert_eq!(exit, 0, "{status}: the older patch is kept: {env:#}");
+        let skipped = find_event(&env, "skipped", Some(code));
+        assert_eq!(skipped["purl"], GEM_PURL, "{status}: {skipped}");
+        assert!(
+            skipped.to_string().contains(GEM_UUID) && skipped.to_string().contains(GEM_UUID2),
+            "{status}: names the kept and the unserved patch: {skipped}"
+        );
+        assert_eq!(env["summary"]["failed"], 0, "{status}: {env:#}");
+        assert_eq!(gem_wired_bytes(&fx), wired, "{status}: wiring untouched");
+    }
+}
+
+/// The #1235 skip keeps only an older vendoring that is still wired: once
+/// the Gemfile pair no longer points at `.socket/vendor/` the gem is patched
+/// in neither mode, so the unserved upgrade still fails with the stable
+/// `vendor_prebuilt_required` code, and no unserved marker leaks out.
+#[tokio::test]
+async fn scan_vendor_gem_unserved_upgrade_fails_when_the_old_patch_is_unwired() {
+    let (fx, _) = gem_vendored_at_a().await;
+    std::fs::write(fx.gemfile_path(), GEM_GEMFILE).unwrap();
+    std::fs::write(fx.lock_path(), GEM_LOCK).unwrap();
+    let mock = wiremock::MockServer::start().await;
+    mount_gem_patch_api_at(&mock, GEM_PURL, GEM_UUID2, Some("pending_build")).await;
+    let (exit, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]);
+    assert_eq!(exit, 1, "an unwired older patch is not kept: {env:#}");
+    let failed = find_event(&env, "failed", Some("vendor_prebuilt_required"));
+    assert_eq!(failed["purl"], GEM_PURL);
+    assert!(failed.to_string().contains("still building"), "{failed}");
+    assert!(
+        events(&env)
+            .iter()
+            .all(|e| e["errorCode"] != "vendor_prebuilt_pending"),
+        "no unserved marker leaks out of a real failure: {env:#}"
+    );
+}
+
+/// A first gem vendor whose `.gem` is still building has no older patch to
+/// keep: it fails with the stable `vendor_prebuilt_required` code (#1235).
+#[tokio::test]
+async fn scan_vendor_gem_unserved_first_vendor_still_fails() {
+    let fx = gem_fixture();
+    let mock = wiremock::MockServer::start().await;
+    mount_gem_patch_api_at(&mock, GEM_PURL, GEM_UUID, Some("pending_build")).await;
+    let (exit, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]);
+    assert_eq!(exit, 1, "{env:#}");
+    let failed = find_event(&env, "failed", Some("vendor_prebuilt_required"));
+    assert!(failed.to_string().contains("still building"), "{failed}");
+    assert_eq!(
+        std::fs::read_to_string(fx.gemfile_path()).unwrap(),
+        GEM_GEMFILE
+    );
+    assert_eq!(std::fs::read_to_string(fx.lock_path()).unwrap(), GEM_LOCK);
+}
+
 /// `scan --mode vendored` end to end on a gem project: discover → download →
 /// vendor lands the gem pair edit (Gemfile pin + `path:`, lock PATH section
 /// + `(= …)!` DEPENDENCIES pin) and the patched artifact dir, keyed in the

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

@@ -4247,7 +4247,7 @@ mod tests {
             )),
         )
         .await;
-        let detail = expect_refused(outcome, "vendor_prebuilt_required");
+        let detail = expect_refused(outcome, "vendor_prebuilt_pending");
         assert!(detail.contains("still building"), "{detail}");
         assert!(!root.join(".socket/vendor").exists(), "no vendor debris");
         assert!(!root.join(".cargo").exists(), "nothing wired");
@@ -4286,7 +4286,7 @@ mod tests {
             )),
         )
         .await;
-        let detail = expect_refused(outcome, "vendor_prebuilt_required");
+        let detail = expect_refused(outcome, "vendor_prebuilt_unavailable");
         assert!(
             detail.contains("prebuilt crate unavailable: not_found"),
             "{detail}"

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

@@ -3000,7 +3000,7 @@ mod tests {
             )
             .await,
         );
-        assert_eq!(code, "vendor_prebuilt_required");
+        assert_eq!(code, "vendor_prebuilt_pending");
         assert!(detail.contains("still building"), "{detail}");
         assert!(!root.join(".socket").exists(), "nothing written");
         assert_eq!(
@@ -3053,7 +3053,7 @@ mod tests {
             )
             .await,
         );
-        assert_eq!(code, "vendor_prebuilt_required");
+        assert_eq!(code, "vendor_prebuilt_unavailable");
         assert!(detail.contains("unavailable"), "{detail}");
         assert!(!root.join(".socket").exists(), "nothing written");
     }

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

@@ -6697,7 +6697,7 @@ mod tests {
 
         let (code, detail) =
             unwrap_refused(run_vendor_service(&root, &blobs, &installed, &record, &cfg).await);
-        assert_eq!(code, "vendor_prebuilt_required");
+        assert_eq!(code, "vendor_prebuilt_pending");
         assert!(detail.contains("still building"), "{detail}");
         assert!(!root.join(".socket").exists());
         assert_eq!(
@@ -6734,7 +6734,7 @@ mod tests {
 
         let (code, detail) =
             unwrap_refused(run_vendor_service(&root, &blobs, &installed, &record, &cfg).await);
-        assert_eq!(code, "vendor_prebuilt_required");
+        assert_eq!(code, "vendor_prebuilt_unavailable");
         assert!(detail.contains("unavailable"), "{detail}");
         assert!(!root.join(".socket").exists());
     }