fix(unify): enforce atomic workspace-member cohorts for non-default

loadingalias · loadingalias · commit 5e588ec11c29 · 2026-02-16T22:20:07.000-05:00
members to prevent Tokio split-source graphs
diff --git a/src/cargo/unify_analyzer.rs b/src/cargo/unify_analyzer.rs
@@ -19,6 +19,7 @@ use crate::error::{RailResult, ResultExt};
 use crate::progress;
 use crate::workspace::WorkspaceContext;
 use rustc_hash::{FxHashMap, FxHashSet};
+use semver::Version;
 use semver::VersionReq;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
@@ -294,6 +295,7 @@ impl UnifyAnalyzer {
     // Pre-compute workspace member metadata for reuse.
     let mut workspace_member_names: FxHashSet<Arc<str>> = FxHashSet::default();
     let mut workspace_member_paths: FxHashMap<Arc<str>, PathBuf> = FxHashMap::default();
+    let mut workspace_member_versions: FxHashMap<Arc<str>, Version> = FxHashMap::default();
 
     // Build member_paths mapping from metadata (manifest file paths)
     for pkg in self.metadata.workspace_packages() {
@@ -304,6 +306,7 @@ impl UnifyAnalyzer {
         Arc::clone(&member_name),
         self.normalize_workspace_member_path(&manifest_path),
       );
+      workspace_member_versions.insert(Arc::clone(&member_name), pkg.version.clone());
       member_paths.insert(member_name, manifest_path);
     }
 
@@ -384,30 +387,40 @@ impl UnifyAnalyzer {
       // Get version from metadata - but ONLY from direct dependencies of workspace members
       // This uses direct_dep_versions() which filters out transitive dependencies
       let versions = self.metadata.direct_dep_versions(&dep_key.name);
-      if versions.is_empty() {
-        continue; // Not a direct dependency of any workspace member
-      }
 
-      // Get unique versions across all targets
-      let unique_versions: FxHashSet<_> = versions.values().collect();
-
-      // Always use highest version (cargo's resolver already picks highest compatible)
-      // When targets resolve to different versions, we unify to highest
-      let version = match unique_versions.iter().max() {
-        Some(max) if unique_versions.len() > 1 => {
-          // Multiple versions found across targets - use highest
-          // This is the "silent win" - we unify duplicates automatically
-          let mut versions_found: Vec<Arc<str>> = unique_versions.iter().map(|v| Arc::from(v.to_string())).collect();
-          versions_found.sort();
-          duplicates_cleaned.push(DuplicateCleanup {
-            dep_name: Arc::clone(&dep_key.name),
-            versions_found,
-            selected_version: Arc::from(max.to_string()),
-          });
-          max
+      // Workspace-member deps must be unifyable even when Cargo metadata excludes
+      // non-default members from resolved direct-dependency traversal.
+      let version = if versions.is_empty() {
+        if workspace_member_names.contains(&dep_key.name) {
+          match workspace_member_versions.get(&dep_key.name) {
+            Some(v) => v,
+            None => continue,
+          }
+        } else {
+          continue; // Not a direct dependency of any workspace member
+        }
+      } else {
+        // Get unique versions across all targets
+        let unique_versions: FxHashSet<_> = versions.values().collect();
+
+        // Always use highest version (cargo's resolver already picks highest compatible)
+        // When targets resolve to different versions, we unify to highest
+        match unique_versions.iter().max() {
+          Some(max) if unique_versions.len() > 1 => {
+            // Multiple versions found across targets - use highest
+            // This is the "silent win" - we unify duplicates automatically
+            let mut versions_found: Vec<Arc<str>> = unique_versions.iter().map(|v| Arc::from(v.to_string())).collect();
+            versions_found.sort();
+            duplicates_cleaned.push(DuplicateCleanup {
+              dep_name: Arc::clone(&dep_key.name),
+              versions_found,
+              selected_version: Arc::from(max.to_string()),
+            });
+            *max
+          }
+          Some(version) => *version,
+          None => continue, // Empty set - shouldn't happen given versions check above
         }
-        Some(version) => version,
-        None => continue, // Empty set - shouldn't happen given versions check above
       };
 
       // Construct version requirement - preserve exact pins if configured
diff --git a/tests/integration/test_unify.rs b/tests/integration/test_unify.rs
@@ -1017,6 +1017,76 @@ exclude = ["tokio"]
   Ok(())
 }
 
+#[test]
+fn test_unify_workspace_member_cohort_with_non_default_members_still_unifies() -> Result<()> {
+  let workspace = TestWorkspace::new_named("non-default-member-cohort")?;
+
+  // default member: excluded from the member-dependency cohort
+  workspace.add_crate("root-default", "0.1.0", &[])?;
+
+  // non-default member cohort:
+  // edge-a -> edge-b -> edge-c
+  workspace.add_crate(
+    "edge-a",
+    "0.1.0",
+    &[("edge-b", r#"{ path = "../edge-b", version = "0.1.0" }"#)],
+  )?;
+  workspace.add_crate(
+    "edge-b",
+    "0.1.0",
+    &[("edge-c", r#"{ path = "../edge-c", version = "0.1.0" }"#)],
+  )?;
+  workspace.add_crate("edge-c", "0.1.0", &[])?;
+
+  // Restrict default-members so edge-* crates are often absent from resolved metadata's
+  // direct-dependency traversal. Cohort logic must still unify edge-b/edge-c atomically.
+  let root_manifest = workspace.path.join("Cargo.toml");
+  let mut root = std::fs::read_to_string(&root_manifest)?;
+  root = root.replace(
+    "resolver = \"2\"\n",
+    "resolver = \"2\"\ndefault-members = [\"crates/root-default\"]\n",
+  );
+  std::fs::write(&root_manifest, root)?;
+
+  workspace.commit("Add non-default workspace member cohort")?;
+
+  let output = run_cargo_rail(&workspace.path, &["rail", "unify"])?;
+  assert!(
+    output.status.success(),
+    "unify should succeed.\nstdout:\n{}\nstderr:\n{}",
+    String::from_utf8_lossy(&output.stdout),
+    String::from_utf8_lossy(&output.stderr)
+  );
+
+  let workspace_toml = std::fs::read_to_string(workspace.path.join("Cargo.toml"))?;
+  assert!(
+    workspace_toml.contains("edge-b = {") && workspace_toml.contains("path = \"crates/edge-b\""),
+    "edge-b should be unified as workspace member despite non-default-members metadata limits.\nCargo.toml:\n{}",
+    workspace_toml
+  );
+  assert!(
+    workspace_toml.contains("edge-c = {") && workspace_toml.contains("path = \"crates/edge-c\""),
+    "edge-c should be unified as workspace member despite non-default-members metadata limits.\nCargo.toml:\n{}",
+    workspace_toml
+  );
+
+  let edge_a_toml = std::fs::read_to_string(workspace.path.join("crates/edge-a/Cargo.toml"))?;
+  assert!(
+    edge_a_toml.contains("edge-b = { workspace = true"),
+    "edge-a should inherit edge-b from workspace.\nCargo.toml:\n{}",
+    edge_a_toml
+  );
+
+  let edge_b_toml = std::fs::read_to_string(workspace.path.join("crates/edge-b/Cargo.toml"))?;
+  assert!(
+    edge_b_toml.contains("edge-c = { workspace = true"),
+    "edge-b should inherit edge-c from workspace.\nCargo.toml:\n{}",
+    edge_b_toml
+  );
+
+  Ok(())
+}
+
 // Config Options: include, pin_transitives, include_renamed
 
 /// Test include config forces specific dependencies to be included
