cargo-rail: cleaning up dead_code blocks; fixing CI for Windows ARM64 runner (wrong label) and Windows escaping in TOML parsing.

Author: loadingalias

.github/workflows/commit.yaml

@@ -47,16 +47,16 @@ jobs:
             runner: windows-latest
             cache-key: commit-windows-x64
 
-          # Windows ARM64 (GitHub Teams/Enterprise)
+          # Windows ARM64 (GitHub Free in public repos)
           - name: aarch64-pc-windows-msvc
-            runner: windows-11-arm64
+            runner: windows-11-arm
             cache-key: commit-windows-arm64
 
           # macOS tested locally (not in CI)
 
     steps:
       - name: Checkout
-        uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8  # v5.0.0
+        uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
 
       - name: Setup
         uses: ./.github/actions/setup
@@ -74,7 +74,7 @@ jobs:
 
       - name: Upload Test Results (on failure)
         if: failure()
-        uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4  # v5.0.0
+        uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
         with:
           name: test-results-${{ matrix.target.name }}
           path: target/nextest/

TODO.md

@@ -55,58 +55,53 @@ Build status: ✅ 115 tests passing, zero warnings, cargo deny/audit passing
 
 #### Minor Optimizations (MEDIUM - 20-50ms)
 
-- [ ] Use HashSet for resolved_files membership test (sync.rs:305) - *10 min*
-  - Currently Vec for `.any()` = O(n), should be HashSet = O(1)
-
-- [ ] Batch path deduplication in walk_filtered_history (split.rs:76-101) - *30 min*
-  - Currently: `get_commits_touching_path()` per crate_path separately
-  - Use single git command for all paths
-
-- [ ] Share metadata/transform in parallel split (split.rs:237-261) - *30 min*
-  - Wrap in Arc instead of creating per thread
-
-- [ ] Simplify git status check (split.rs:420-425) - *10 min*
-  - Remove redundant `status --porcelain`, just run `add -A`
+- [x] Use HashSet for resolved_files membership test - O(1) lookups
+- [x] Batch path deduplication in walk_filtered_history - single git command
+- [x] Remove unused metadata field in SyncEngine - eliminated unnecessary clone
+- [x] Simplify git status check - cleaner logic with diff --cached
 
 ### Critical: Code Cleanup (2 hours)
 
 #### DRY Violations (CRITICAL)
 
-- [ ] Extract path normalization helper - *15 min*
-  - Pattern: `strip_prefix().unwrap_or()` appears 5+ times
-  - system_git_ops.rs:53-57, 105-109, 136-140; transform.rs:35-39
-  - Create `SystemGit::normalize_path()`
-
-- [ ] Extract Cargo.toml transformation helper - *30 min*
-  - Identical 11-line block in split.rs:611-622 and 654-664
-  - Create `apply_manifest_transform()`
+- [x] Extract path normalization helper - SystemGit::normalize_path()
+- [x] Extract Cargo.toml transformation helper - Splitter::apply_manifest_transform()
 
 #### Dead Code Suppressions (CRITICAL)
 
-- [ ] Replace module-level `#![allow(dead_code)]` with item-level - *30 min*
-  - Files: sync.rs, config.rs, transform.rs, files.rs, metadata.rs
-  - Currently masks accidental dead code
-  - Keep: plan.rs (documented for future CI/automation)
-  - Keep: All function-level annotations (legitimate convenience APIs)
+- [x] Replace module-level `#![allow(dead_code)]` with item-level
+  - Removed from: sync.rs, config.rs, transform.rs, files.rs, metadata.rs
+  - Added item-level annotations with documentation for legitimate API methods
+  - Kept: plan.rs (documented for future CI/automation)
 
 #### TODO Comments Cleanup (CRITICAL)
 
-- [ ] Resolve sync.rs:139 combined mode TODO - *30 min*
-  - Either implement or remove combined mode support
-  - Currently blocks entire file with dead_code suppression
+- [x] Resolve sync.rs:139 combined mode TODO - now uses get_commits_touching_paths()
+- [x] Address release.rs:66 skip_sync TODO - documented deferral to post-v1
 
-- [ ] Address release.rs:66 skip_sync TODO - *15 min*
-  - Implement or document deferral
+**Week 1 Complete**: All performance optimizations and code cleanup done
 
-**Week 1 Performance Complete**: All critical performance optimizations done
+Performance improvements:
 
 - Reverse mapping index: O(1) lookups (100-500ms saved per sync)
 - Conflict detection cache: Single git call vs N calls (100ms-1s saved)
 - get_changed_files() cache: Eliminated duplicate parsing (10-50ms saved)
 - Arc<SecurityConfig>: Cheap clones in parallel (eliminates N deep clones)
 - Mapping load cache: Eliminated redundant subprocess calls (50ms saved)
-- Type complexity warning: Fixed with ConflictResolutionResult type alias
-- Dead code warning: Fixed with proper documentation + #[allow(dead_code)]
+- HashSet for resolved_files: O(1) vs O(n) membership testing
+- Batched path operations: Single git command for multiple paths
+- Removed unused metadata field: Eliminated unnecessary WorkspaceMetadata clone
+
+Code quality improvements:
+
+- Extracted SystemGit::normalize_path() helper (5 duplicates removed)
+- Extracted Splitter::apply_manifest_transform() helper (3 duplicates removed)
+- Replaced all module-level #![allow(dead_code)] with item-level annotations
+- Resolved sync.rs combined mode TODO (now handles both single and combined modes)
+- Documented skip_sync deferral in release.rs
+- Simplified git status check logic
+- Zero compiler warnings
+- All 115 tests passing
 
 ---
 

src/cargo/files.rs

@@ -1,5 +1,3 @@
-#![allow(dead_code)]
-
 use crate::core::error::{RailResult, ResultExt};
 use crate::ui::progress::FileProgress;
 use std::fs;

src/cargo/metadata.rs

@@ -1,5 +1,3 @@
-#![allow(dead_code)]
-
 use crate::core::error::RailResult;
 use cargo_metadata::{MetadataCommand, Package};
 use std::path::Path;
@@ -22,6 +20,8 @@ impl WorkspaceMetadata {
     self.metadata.workspace_packages()
   }
 
+  /// Get package by name - public API for external use
+  #[allow(dead_code)]
   pub fn get_package(&self, name: &str) -> Option<&Package> {
     self
       .metadata
@@ -34,7 +34,8 @@ impl WorkspaceMetadata {
     self.metadata.workspace_root.as_std_path()
   }
 
-  /// Get raw JSON string for external tools
+  /// Get raw JSON string for external tools - public API for debugging/logging
+  #[allow(dead_code)]
   pub fn to_json_string(&self) -> RailResult<String> {
     serde_json::to_string(&self.metadata)
       .map_err(|e| crate::core::error::RailError::message(format!("Failed to serialize metadata: {}", e)))

src/cargo/transform.rs

@@ -1,14 +1,16 @@
-#![allow(dead_code)]
-
 use crate::cargo::metadata::WorkspaceMetadata;
 use crate::core::error::{RailError, RailResult, ResultExt};
 use std::collections::HashMap;
 use std::path::PathBuf;
 use toml_edit::{DocumentMut, Item, Value};
 
-/// Context for transforming Cargo.toml
+/// Context for transformation operations
 pub struct TransformContext {
+  /// Crate name being transformed - public API field for future use
+  #[allow(dead_code)]
   pub crate_name: String,
+  /// Workspace root path - public API field for future use
+  #[allow(dead_code)]
   pub workspace_root: PathBuf,
 }
 

src/commands/release.rs

@@ -63,7 +63,8 @@ pub fn run_release_plan(name: Option<String>, all: bool, json: bool) -> RailResu
 pub fn run_release_apply(
   name: String,
   dry_run: bool,
-  _skip_sync: bool, // TODO: Implement split sync
+  // Skip syncing to split repos - deferred to post-v1 (see TODO.md Post-v1: Auto-sync to split repos)
+  #[allow(unused_variables)] skip_sync: bool,
 ) -> RailResult<()> {
   let workspace_root = env::current_dir()?;
 

src/commands/split.rs

@@ -22,7 +22,13 @@ fn prompt_for_confirmation(message: &str) -> RailResult<bool> {
 }
 
 /// Run the split command
-pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool) -> RailResult<()> {
+pub fn run_split(
+  crate_name: Option<String>,
+  all: bool,
+  remote: Option<String>,
+  apply: bool,
+  json: bool,
+) -> RailResult<()> {
   let current_dir = env::current_dir()?;
 
   // Load configuration
@@ -35,8 +41,8 @@ pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool)
   let config = RailConfig::load(&current_dir)?;
   println!("📦 Loaded configuration from .rail/config.toml");
 
-  // Determine which crates to split (need this to check if they're all local)
-  let crates_to_split_check: Vec<_> = if all {
+  // Determine which crates to split
+  let mut crates_to_split_check: Vec<_> = if all {
     config.splits.clone()
   } else if let Some(ref name) = crate_name {
     let split_config = config
@@ -52,6 +58,13 @@ pub fn run_split(crate_name: Option<String>, all: bool, apply: bool, json: bool)
     ));
   };
 
+  // Apply remote override if provided (before all_local check)
+  if let Some(ref remote_override) = remote {
+    for split_config in &mut crates_to_split_check {
+      split_config.remote = remote_override.clone();
+    }
+  }
+
   // Check if all remotes are local paths (skip SSH checks for local testing)
   let all_local = crates_to_split_check
     .iter()

src/commands/sync.rs

@@ -9,16 +9,60 @@ use crate::core::sync::{SyncConfig, SyncDirection, SyncEngine, SyncResult};
 use crate::ui::progress::{FileProgress, MultiProgress};
 use rayon::prelude::*;
 
+/// Sync command parameters
+pub struct SyncParams {
+  pub crate_name: Option<String>,
+  pub all: bool,
+  pub remote: Option<String>,
+  pub from_remote: bool,
+  pub to_remote: bool,
+  pub strategy_str: String,
+  pub no_protected_branches: bool,
+  pub apply: bool,
+  pub json: bool,
+}
+
 /// Run the sync command
+#[allow(clippy::too_many_arguments)]
 pub fn run_sync(
   crate_name: Option<String>,
   all: bool,
+  remote: Option<String>,
   from_remote: bool,
   to_remote: bool,
   strategy_str: String,
+  no_protected_branches: bool,
   apply: bool,
   json: bool,
 ) -> RailResult<()> {
+  // Convert to struct for internal use
+  let params = SyncParams {
+    crate_name,
+    all,
+    remote,
+    from_remote,
+    to_remote,
+    strategy_str,
+    no_protected_branches,
+    apply,
+    json,
+  };
+  run_sync_impl(params)
+}
+
+/// Internal implementation of sync command
+fn run_sync_impl(params: SyncParams) -> RailResult<()> {
+  let SyncParams {
+    crate_name,
+    all,
+    remote,
+    from_remote,
+    to_remote,
+    strategy_str,
+    no_protected_branches,
+    apply,
+    json,
+  } = params;
   // Parse conflict strategy
   let strategy = ConflictStrategy::from_str(&strategy_str)?;
   let current_dir = env::current_dir()?;
@@ -30,11 +74,17 @@ pub fn run_sync(
     }));
   }
 
-  let config = RailConfig::load(&current_dir)?;
+  let mut config = RailConfig::load(&current_dir)?;
+
+  // Apply CLI overrides to security config if provided
+  if no_protected_branches {
+    config.security.protected_branches.clear();
+  }
+
   println!("📦 Loaded configuration from .rail/config.toml");
 
-  // Determine which crates to sync (need this to check if they're all local)
-  let crates_to_sync_check: Vec<_> = if all {
+  // Determine which crates to sync
+  let mut crates_to_sync_check: Vec<_> = if all {
     config.splits.clone()
   } else if let Some(ref name) = crate_name {
     let split_config = config
@@ -50,6 +100,13 @@ pub fn run_sync(
     ));
   };
 
+  // Apply remote override if provided (before all_local check)
+  if let Some(ref remote_override) = remote {
+    for split_config in &mut crates_to_sync_check {
+      split_config.remote = remote_override.clone();
+    }
+  }
+
   // Check if all remotes are local paths (skip SSH checks for local testing)
   let all_local = crates_to_sync_check
     .iter()

src/core/config.rs

@@ -1,5 +1,3 @@
-#![allow(dead_code)]
-
 use crate::core::error::{ConfigError, RailError, RailResult, ResultExt};
 use serde::{Deserialize, Serialize};
 use std::fs;
@@ -147,7 +145,8 @@ impl PolicyConfig {
     Ok(())
   }
 
-  /// Check if policy is enabled (any field is set)
+  /// Check if policy is enabled (any field is set) - public API for conditional logic
+  #[allow(dead_code)]
   pub fn is_enabled(&self) -> bool {
     self.resolver.is_some()
       || self.msrv.is_some()