cargo-rail: added 'split/sync' safety rails; defrred 'git centralization'; removed the four unwrap()/expect() calls from the production code; fixed the 'process::exit()' in the lib code.

Author: loadingalias

.gitignore

@@ -19,6 +19,7 @@ Thumbs.db
 rules.md
 release.md
 todo.md
+PLAN-centralize-git.md
 **/demo.cast
 distribution/
 backlog/

src/cargo/manifest_writer.rs

@@ -327,12 +327,8 @@ impl ManifestWriter {
 
     // Update the dependency entry
     // If it's a simple string like `serde = "1.0"`, we need to convert it to a table
-    if dep_item.is_str() {
+    if let Some(version) = dep_item.as_str() {
       // Convert simple string (e.g., `dep = "1.0"`) to inline table with features
-      let version = dep_item
-        .as_str()
-        .expect("dep_item.is_str() was true but as_str() returned None")
-        .to_string();
       let mut inline_table = toml_edit::InlineTable::new();
       inline_table.insert("version", toml_edit::Value::from(version));
       inline_table.insert("features", manifest_ops::build_feature_array(&existing_features));

src/commands/config.rs

@@ -170,7 +170,7 @@ pub fn run_config_validate_standalone(
           "{}",
           serde_json::to_string_pretty(&result).map_err(|e| RailError::message(e.to_string()))?
         );
-        std::process::exit(2);
+        return Err(RailError::ExitWithCode { code: 2 });
       } else {
         println!("no configuration file found");
         println!("\nhelp: run 'cargo rail init' to create one");
@@ -315,7 +315,7 @@ pub fn run_config_validate_standalone(
   if valid {
     Ok(())
   } else if json {
-    std::process::exit(2);
+    Err(RailError::ExitWithCode { code: 2 })
   } else {
     Err(RailError::message("configuration validation failed"))
   }
@@ -501,7 +501,7 @@ pub fn run_config_sync(workspace_root: &Path, check: bool, format: OutputFormat)
 
     if has_changes {
       // Exit with code 1 to indicate changes are needed
-      std::process::exit(1);
+      return Err(RailError::CheckHasPendingChanges);
     }
   } else {
     // Apply mode

src/commands/test.rs

@@ -147,7 +147,9 @@ pub fn run_test(ctx: &WorkspaceContext, config: TestConfig) -> RailResult<()> {
     .map_err(|e| crate::error::RailError::message(format!("{} failed: {}", runner.name(), e)))?;
 
   if !status.success() {
-    std::process::exit(status.code().unwrap_or(1));
+    return Err(crate::error::RailError::ExitWithCode {
+      code: status.code().unwrap_or(1),
+    });
   }
 
   println!("\nall tests passed");

src/commands/unify.rs

@@ -393,7 +393,10 @@ pub fn run_unify_apply(
       // Find a suitable member's path
       let first_member = &members[0];
       if let Some(pkg) = ctx.cargo.get_package(first_member) {
-        let member_path = pkg.manifest_path.parent().unwrap();
+        let member_path = pkg
+          .manifest_path
+          .parent()
+          .ok_or_else(|| RailError::message(format!("Invalid manifest path: {}", pkg.manifest_path)))?;
         let relative_path = member_path.strip_prefix(ctx.workspace_root()).unwrap_or(member_path);
         progress!(
           "  note: using '{}' as transitive host (virtual workspace detected)",

src/error.rs

@@ -16,17 +16,24 @@ use std::path::PathBuf;
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ExitCode {
   /// Success - everything is good
-  Success = 0,
+  Success,
   /// Check failed - changes would be made (use in --check mode)
-  CheckFailed = 1,
+  CheckFailed,
   /// Error occurred (user or system error)
-  Error = 2,
+  Error,
+  /// Custom exit code (e.g., propagated from subprocess)
+  Custom(i32),
 }
 
 impl ExitCode {
   /// Convert to i32 for process exit
   pub fn as_i32(self) -> i32 {
-    self as i32
+    match self {
+      ExitCode::Success => 0,
+      ExitCode::CheckFailed => 1,
+      ExitCode::Error => 2,
+      ExitCode::Custom(code) => code,
+    }
   }
 }
 
@@ -57,6 +64,17 @@ pub enum RailError {
   /// Used by --check commands to signal that changes would be made.
   /// This is not a failure, but CI should treat it as "action needed".
   CheckHasPendingChanges,
+
+  /// Exit with specific code (no error message printed)
+  ///
+  /// Used for:
+  /// - Propagating subprocess exit codes (e.g., cargo test failures)
+  /// - Silent exits after JSON output has been written
+  /// - Any case where we need a specific exit code without error output
+  ExitWithCode {
+    /// The exit code to use
+    code: i32,
+  },
 }
 
 impl RailError {
@@ -95,6 +113,7 @@ impl RailError {
   pub fn exit_code(&self) -> ExitCode {
     match self {
       RailError::CheckHasPendingChanges => ExitCode::CheckFailed,
+      RailError::ExitWithCode { code } => ExitCode::Custom(*code),
       _ => ExitCode::Error,
     }
   }
@@ -123,7 +142,8 @@ impl fmt::Display for RailError {
         }
         Ok(())
       }
-      RailError::CheckHasPendingChanges => Ok(()), // Silent - no message
+      RailError::CheckHasPendingChanges => Ok(()), // Silent - CI signal
+      RailError::ExitWithCode { .. } => Ok(()),    // Silent - exit code only
     }
   }
 }
@@ -463,9 +483,13 @@ struct JsonError {
 ///
 /// In JSON mode, outputs a structured JSON error object instead of text.
 pub fn print_error(error: &RailError) {
-  // CheckHasPendingChanges is not an error - it's a signal for CI
-  // Don't print "error:" prefix for it
-  if matches!(error, RailError::CheckHasPendingChanges) {
+  // These are not errors to display - they're exit code signals
+  // CheckHasPendingChanges: CI signal for "changes would be made"
+  // ExitWithCode: subprocess errors or silent exits after JSON output
+  if matches!(
+    error,
+    RailError::CheckHasPendingChanges | RailError::ExitWithCode { .. }
+  ) {
     return;
   }
 

src/git/ops.rs

@@ -323,6 +323,12 @@ impl SystemGit {
     Ok(())
   }
 
+  /// Check if a local branch exists
+  pub fn branch_exists(&self, branch_name: &str) -> RailResult<bool> {
+    let ref_name = format!("refs/heads/{}", branch_name);
+    Ok(self.run_git_check(&["show-ref", "--verify", "--quiet", &ref_name]))
+  }
+
   /// Create and checkout a branch
   pub fn create_and_checkout_branch(&self, branch_name: &str) -> RailResult<()> {
     self.create_branch(branch_name)?;

src/release/publisher.rs

@@ -358,9 +358,13 @@ impl<'a> ReleasePublisher<'a> {
 
   /// Create GitHub release using gh CLI
   fn create_github_release(&self, plan: &CrateReleasePlan) -> RailResult<()> {
-    let check = Command::new("gh").args(["--version"]).output();
+    let gh_available = Command::new("gh")
+      .args(["--version"])
+      .output()
+      .map(|o| o.status.success())
+      .unwrap_or(false);
 
-    if check.is_err() || !check.unwrap().status.success() {
+    if !gh_available {
       eprintln!("  skipped github release (gh CLI not found)");
       return Ok(());
     }

src/release/version.rs

@@ -89,7 +89,7 @@ impl BumpType {
 
         if current.pre.is_empty() {
           // No prerelease - add rc.1
-          next.pre = semver::Prerelease::new("rc.1").unwrap();
+          next.pre = semver::Prerelease::new("rc.1").expect("BUG: 'rc.1' is a valid semver prerelease identifier");
         } else {
           // Increment the numeric part of the prerelease
           // Supports: rc.1 -> rc.2, alpha.3 -> alpha.4, beta.0 -> beta.1

src/split/engine.rs

@@ -408,7 +408,7 @@ impl<'a> SplitEngine<'a> {
     Ok(output.status.success() && !output.stdout.is_empty())
   }
 
-  /// Execute a split operation (ONE-TIME ONLY - use sync for updates)
+  /// Execute a split operation (idempotent - re-runs sync new commits only)
   pub fn split(&self, config: &SplitConfig) -> RailResult<()> {
     println!("🚂 Splitting crate: {}", config.crate_name);
     println!("   Mode: {:?}", config.mode);
@@ -424,10 +424,14 @@ impl<'a> SplitEngine<'a> {
       println!("   Exclude patterns: {} configured", config.exclude.len());
     }
 
-    // Check if remote already exists - if so, error with helpful message
+    // Check if target repo already exists (for idempotency)
+    let target_exists = config.target_repo_path.join(".git").exists();
+
+    // Check if remote already exists - warn but allow re-run for idempotency
     if let Some(ref remote_url) = config.remote_url {
       let remote_exists = self.check_remote_exists(remote_url)?;
-      if remote_exists {
+      if remote_exists && !target_exists {
+        // Remote exists but no local target - user probably wants to use sync instead
         return Err(RailError::with_help(
           format!("Split already exists at {}", remote_url),
           format!(
@@ -438,9 +442,10 @@ impl<'a> SplitEngine<'a> {
           ),
         ));
       }
+      // If both remote and target exist, we'll check mappings below for idempotency
     }
 
-    // Create fresh target repo
+    // Create or reuse target repo
     self.ensure_target_repo(&config.target_repo_path)?;
 
     // Discover workspace-level auxiliary files from workspace
@@ -461,13 +466,34 @@ impl<'a> SplitEngine<'a> {
       );
     }
 
-    // Create mapping store
+    // Create mapping store and load existing mappings (from both workspace and target)
     let mut mapping_store = MappingStore::new(config.crate_name.clone());
     mapping_store.load(self.ctx.workspace_root())?;
+    if target_exists {
+      mapping_store.load(&config.target_repo_path)?;
+    }
 
     // Walk filtered history to find commits touching the crate
     let filtered_commits = self.walk_filtered_history(&config.crate_paths)?;
 
+    // Count how many commits are already mapped (for idempotency)
+    let already_mapped_count = filtered_commits
+      .iter()
+      .filter(|c| mapping_store.has_mapping(&c.sha))
+      .count();
+
+    if already_mapped_count > 0 {
+      println!("   Found {} commits already split (will skip)", already_mapped_count);
+    }
+
+    // Check if all commits are already mapped - nothing to do
+    if already_mapped_count == filtered_commits.len() && !filtered_commits.is_empty() {
+      println!("\n✅ Split already up-to-date!");
+      println!("   All {} commits have been split previously.", filtered_commits.len());
+      println!("   Target repo: {}", config.target_repo_path.display());
+      return Ok(());
+    }
+
     if filtered_commits.is_empty() {
       println!("   No commits found that touch the crate paths");
       println!("   Falling back to current state copy...");
@@ -505,10 +531,31 @@ impl<'a> SplitEngine<'a> {
       let mut last_recreated_sha: Option<String> = None;
 
       let mut skipped_commits = 0usize;
+      let mut skipped_already_mapped = 0usize;
+
+      // For incremental splits, find the last mapped commit's SHA in target repo
+      // to use as parent for new commits
+      if target_exists && already_mapped_count > 0 {
+        // Find the most recent mapped commit and use its target SHA as last_recreated_sha
+        for commit in filtered_commits.iter().rev() {
+          if let Ok(Some(target_sha)) = mapping_store.get_mapping(&commit.sha) {
+            last_recreated_sha = Some(target_sha);
+            break;
+          }
+        }
+      }
 
       for (idx, commit) in filtered_commits.iter().enumerate() {
-        if (idx + 1) % 10 == 0 || idx + 1 == filtered_commits.len() {
-          println!("   Progress: {}/{} commits", idx + 1, filtered_commits.len());
+        // Skip already-mapped commits (idempotency)
+        if mapping_store.has_mapping(&commit.sha) {
+          skipped_already_mapped += 1;
+          continue;
+        }
+
+        let new_count = idx + 1 - skipped_already_mapped;
+        let total_new = filtered_commits.len() - already_mapped_count;
+        if new_count.is_multiple_of(10) || new_count == total_new {
+          println!("   Progress: {}/{} new commits", new_count, total_new);
         }
 
         // Use prefetched files if available
@@ -539,11 +586,19 @@ impl<'a> SplitEngine<'a> {
         last_recreated_sha = Some(new_sha);
       }
 
-      if skipped_commits > 0 {
-        println!(
-          "   Skipped {} commits where path didn't exist (dirty history)",
-          skipped_commits
-        );
+      if skipped_commits > 0 || skipped_already_mapped > 0 {
+        if skipped_commits > 0 {
+          println!(
+            "   Skipped {} commits where path didn't exist (dirty history)",
+            skipped_commits
+          );
+        }
+        if skipped_already_mapped > 0 {
+          println!(
+            "   Skipped {} commits already split (idempotent)",
+            skipped_already_mapped
+          );
+        }
       }
 
       // Create workspace Cargo.toml if in workspace mode