semver checking for multi-world wit packages

crates/wasm-pkg-client/src/decoded_component.rs

@@ -88,34 +88,47 @@ impl DecodedComponent {
             (other, self)
         };
 
-        let (prev_resolve, prev_world) = extract_resolve_and_world_id(&newer.decoded_wasm)?;
-        let (new_resolve, new_world) = extract_resolve_and_world_id(&older.decoded_wasm)?;
-
-        // Merge resolves, remap merged resolve, check for incompatibility
-        let mut merged = prev_resolve.clone();
-        let new_world = match merged
-            .merge(new_resolve.clone())
-            .map(|remap| remap.map_world(new_world, wit_parser::Span::default()))
-        {
-            Ok(Ok(w)) => w,
-            Ok(Err(e)) => {
-                return Err(Error::InvalidComponent(anyhow::format_err!(
-                    "failed to remap merged worlds: {}",
-                    e.kind()
-                )));
-            }
-            Err(e) => {
-                return Err(Error::InvalidComponent(e));
-            }
-        };
+        let (prev_resolve, prev_worlds) = extract_resolve_and_worlds(&newer.decoded_wasm);
+        let (new_resolve, new_worlds) = extract_resolve_and_worlds(&older.decoded_wasm);
+
+        for (name, new_world) in new_worlds {
+            let Some(&prev_world) = prev_worlds.get(&name) else {
+                // World removal is considered a breaking change
+                return Err(Error::SemverIncompatible {
+                    previous: older.version.clone(),
+                    new: newer.version.clone(),
+                    source: anyhow::anyhow!("world `{name}` was removed"),
+                });
+            };
+
+            // Merge resolves, remap merged resolve, check for incompatibility
+            let mut merged = prev_resolve.clone();
+            let new_world = match merged
+                .merge(new_resolve.clone())
+                .map(|remap| remap.map_world(new_world, wit_parser::Span::default()))
+            {
+                Ok(Ok(w)) => w,
+                Ok(Err(e)) => {
+                    return Err(Error::InvalidComponent(anyhow::format_err!(
+                        "failed to remap merged worlds: {}",
+                        e.kind()
+                    )));
+                }
+                Err(e) => {
+                    return Err(Error::InvalidComponent(e));
+                }
+            };
+
+            wit_component::semver_check(merged, prev_world, new_world).map_err(|e| {
+                Error::SemverIncompatible {
+                    previous: older.version.clone(),
+                    new: newer.version.clone(),
+                    source: e.context(format!("world `{name}`")),
+                }
+            })?;
+        }
 
-        wit_component::semver_check(merged, prev_world, new_world).map_err(|e| {
-            Error::SemverIncompatible {
-                previous: older.version.clone(),
-                new: newer.version.clone(),
-                source: e,
-            }
-        })
+        Ok(())
     }
 }
 
@@ -208,20 +221,26 @@ fn extract_package_version(decoded: &DecodedWasm) -> Result<(PackageRef, Version
     Ok((package, version))
 }
 
-/// Borrow the inner `wit_parser::Resolve` and resolve a concrete `WorldId`.
-/// For a decoded component the world is fixed; for a WIT package we ask
-/// `Resolve::select_world` to pick one — deferred until needed so a
-/// multi-world WIT package can publish its first version unambiguously.
-fn extract_resolve_and_world_id(
+/// Borrow the inner `Resolve` and build an index of its worlds. The world
+/// names are borrowed from the `Resolve`, which outlives the returned index.
+fn extract_resolve_and_worlds(
     decoded: &DecodedWasm,
-) -> Result<(&wit_parser::Resolve, wit_parser::WorldId), Error> {
+) -> (
+    &wit_parser::Resolve,
+    std::collections::HashMap<&str, wit_parser::WorldId>,
+) {
     match decoded {
-        DecodedWasm::Component(resolve, world_id) => Ok((resolve, *world_id)),
+        DecodedWasm::Component(resolve, world_id) => {
+            let name = resolve.worlds[*world_id].name.as_str();
+            (resolve, std::iter::once((name, *world_id)).collect())
+        }
         DecodedWasm::WitPackage(resolve, pkg) => {
-            let world_id = resolve
-                .select_world(&[*pkg], None)
-                .map_err(Error::InvalidPackage)?;
-            Ok((resolve, world_id))
+            let worlds = resolve.packages[*pkg]
+                .worlds
+                .iter()
+                .map(|(name, id)| (name.as_str(), *id))
+                .collect();
+            (resolve, worlds)
         }
     }
 }

crates/wasm-pkg-client/tests/publish_semver_check.rs

@@ -215,3 +215,112 @@ async fn publish_semver_check(
         _ => panic!("expectation mismatch\n  expected: {expected:?}\n  actual:   {result:?}",),
     }
 }
+
+// ---------------------------------------------------------------------------
+// Multi-world packages (issue: semver_check assumed at most one world).
+//
+// A WIT package may declare more than one world. Semver checking must compare
+// worlds pairwise by name: shared worlds must stay compatible, adding a world
+// is additive (OK), and removing a world within the same compat range is
+// breaking (strict policy).
+// ---------------------------------------------------------------------------
+
+/// Which worlds a multi-world fixture declares, and the body of each.
+struct MultiWorld {
+    /// `alpha` world body, if present.
+    alpha: Option<WorldDiff>,
+    /// `beta` world body, if present.
+    beta: Option<WorldDiff>,
+}
+
+fn multiworld_wit(package: &str, version: &str, worlds: &MultiWorld) -> String {
+    let mut out = format!("\npackage {NAMESPACE}:{package}@{version};\n");
+    if let Some(diff) = worlds.alpha {
+        out.push_str(&format!("\nworld alpha {{\n    {diff}\n}}\n"));
+    }
+    if let Some(diff) = worlds.beta {
+        out.push_str(&format!("\nworld beta {{\n    {diff}\n}}\n"));
+    }
+    out
+}
+
+#[rstest]
+// Both worlds compatible across a minor bump within a major -> OK.
+#[case::multiworld_all_compatible(
+    "mw-all-compat",
+    ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }),
+    ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::AddExtra) }),
+    Ok(())
+)]
+// One shared world (beta) breaks across a minor bump -> incompatible.
+#[case::multiworld_one_incompatible(
+    "mw-one-incompat",
+    ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }),
+    ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::ChangeBase) }),
+    Err(semver_incompatible("1.2.0", "1.3.0"))
+)]
+// Adding a world across a minor bump is additive -> OK.
+#[case::multiworld_added_world(
+    "mw-added",
+    ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: None }),
+    ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: Some(WorldDiff::AddBase) }),
+    Ok(())
+)]
+// Removing a world within the same major is breaking (strict policy) -> incompatible.
+#[case::multiworld_removed_world_same_major(
+    "mw-removed",
+    ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }),
+    ("1.3.0", MultiWorld { alpha: Some(WorldDiff::AddExtra), beta: None }),
+    Err(semver_incompatible("1.2.0", "1.3.0"))
+)]
+// Removing a world across a major boundary is allowed: the versions fall in
+// different compatibility ranges, so `semver_check` never compares them.
+#[case::multiworld_removed_world_across_major(
+    "mw-removed-cross-major",
+    ("1.2.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: Some(WorldDiff::AddBase) }),
+    ("2.0.0", MultiWorld { alpha: Some(WorldDiff::AddBase), beta: None }),
+    Ok(())
+)]
+#[tokio::test]
+async fn publish_semver_check_multiworld(
+    #[case] package: &str,
+    #[case] initial: (&str, MultiWorld),
+    #[case] candidate: (&str, MultiWorld),
+    #[case] expected: Result<(), Error>,
+) {
+    let tmp = TempDir::new().unwrap();
+    let client = make_client(tmp.path());
+
+    let (init_version, init_worlds) = initial;
+    publish(
+        &client,
+        wit_to_wasm(&multiworld_wit(package, init_version, &init_worlds)),
+        Default::default(),
+    )
+    .await
+    .unwrap_or_else(|e| panic!("seeding {init_version} failed: {e:?}"));
+
+    let (cand_version, cand_worlds) = candidate;
+    let result = publish(
+        &client,
+        wit_to_wasm(&multiworld_wit(package, cand_version, &cand_worlds)),
+        Default::default(),
+    )
+    .await;
+
+    match (&expected, &result) {
+        (Ok(()), Ok(())) => {}
+        (
+            Err(Error::SemverIncompatible {
+                previous: exp_prev,
+                new: exp_new,
+                ..
+            }),
+            Err(Error::SemverIncompatible { previous, new, .. }),
+        ) => {
+            assert_eq!(previous, exp_prev, "previous version mismatch");
+            assert_eq!(new, exp_new, "new version mismatch");
+        }
+        _ => panic!("expectation mismatch\n  expected: {expected:?}\n  actual:   {result:?}",),
+    }
+}