Add strategy pattern for Ruby detection.

crates/rb-cli/src/commands/environment.rs

@@ -424,7 +424,7 @@ mod tests {
 
         let bundler_sandbox = BundlerSandbox::new()?;
         let project_dir = bundler_sandbox.add_bundler_project("test-app", true)?;
-        let bundler_runtime = BundlerRuntime::new(&project_dir);
+        let bundler_runtime = BundlerRuntime::new(&project_dir, ruby.version.clone());
 
         // Use sandboxed gem directory instead of real home directory
         let gem_runtime = GemRuntime::for_base_dir(&ruby_sandbox.gem_base_dir(), &ruby.version);

crates/rb-cli/src/commands/exec.rs

@@ -237,8 +237,17 @@ mod tests {
 
         // Test that standard variables are present
         assert!(env_vars.contains_key("PATH"));
-        assert!(env_vars.contains_key("GEM_HOME"));
-        assert!(env_vars.contains_key("GEM_PATH"));
+
+        // IMPORTANT: When bundler context is detected, GEM_HOME and GEM_PATH should NOT be set
+        // This is bundler isolation - only bundled gems are available
+        assert!(
+            !env_vars.contains_key("GEM_HOME"),
+            "GEM_HOME should NOT be set in bundler context (isolation)"
+        );
+        assert!(
+            !env_vars.contains_key("GEM_PATH"),
+            "GEM_PATH should NOT be set in bundler context (isolation)"
+        );
 
         // Test that bundler variables are set when bundler project is detected
         assert!(env_vars.contains_key("BUNDLE_GEMFILE"));

crates/rb-cli/src/config/locator.rs

@@ -1,6 +1,20 @@
 use log::debug;
 use std::path::PathBuf;
 
+/// Trait for reading environment variables - allows mocking in tests
+pub trait EnvReader {
+    fn var(&self, key: &str) -> Result<String, std::env::VarError>;
+}
+
+/// Production implementation using std::env
+pub struct StdEnvReader;
+
+impl EnvReader for StdEnvReader {
+    fn var(&self, key: &str) -> Result<String, std::env::VarError> {
+        std::env::var(key)
+    }
+}
+
 /// Locate the configuration file following XDG Base Directory specification
 ///
 /// Supports both rb.kdl and rb.toml (preferring .kdl)
@@ -13,6 +27,14 @@ use std::path::PathBuf;
 /// 5. %APPDATA%/rb/rb.kdl or rb.toml (Windows)
 /// 6. ~/.rb.kdl or ~/.rb.toml (cross-platform fallback)
 pub fn locate_config_file(override_path: Option<PathBuf>) -> Option<PathBuf> {
+    locate_config_file_with_env(override_path, &StdEnvReader)
+}
+
+/// Internal function that accepts an environment reader for testing
+fn locate_config_file_with_env(
+    override_path: Option<PathBuf>,
+    env: &dyn EnvReader,
+) -> Option<PathBuf> {
     debug!("Searching for configuration file...");
 
     // 1. Check for explicit override first
@@ -25,7 +47,7 @@ pub fn locate_config_file(override_path: Option<PathBuf>) -> Option<PathBuf> {
     }
 
     // 2. Check RB_CONFIG environment variable
-    if let Ok(rb_config) = std::env::var("RB_CONFIG") {
+    if let Ok(rb_config) = env.var("RB_CONFIG") {
         let config_path = PathBuf::from(rb_config);
         debug!("  Checking RB_CONFIG env var: {}", config_path.display());
         if config_path.exists() {
@@ -35,7 +57,7 @@ pub fn locate_config_file(override_path: Option<PathBuf>) -> Option<PathBuf> {
     }
 
     // 3. Try XDG_CONFIG_HOME (Unix/Linux)
-    if let Ok(xdg_config) = std::env::var("XDG_CONFIG_HOME") {
+    if let Ok(xdg_config) = env.var("XDG_CONFIG_HOME") {
         let base_path = PathBuf::from(xdg_config).join("rb");
         // Try .kdl first, then .toml
         for ext in &["rb.kdl", "rb.toml"] {
@@ -98,6 +120,34 @@ pub fn locate_config_file(override_path: Option<PathBuf>) -> Option<PathBuf> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::collections::HashMap;
+
+    /// Mock environment reader for testing without global state mutation
+    struct MockEnvReader {
+        vars: HashMap<String, String>,
+    }
+
+    impl MockEnvReader {
+        fn new() -> Self {
+            Self {
+                vars: HashMap::new(),
+            }
+        }
+
+        fn with_var(mut self, key: impl Into<String>, value: impl Into<String>) -> Self {
+            self.vars.insert(key.into(), value.into());
+            self
+        }
+    }
+
+    impl EnvReader for MockEnvReader {
+        fn var(&self, key: &str) -> Result<String, std::env::VarError> {
+            self.vars
+                .get(key)
+                .cloned()
+                .ok_or(std::env::VarError::NotPresent)
+        }
+    }
 
     #[test]
     fn test_locate_config_file_returns_option() {
@@ -127,24 +177,44 @@ mod tests {
     fn test_locate_config_file_with_env_var() {
         use std::fs;
         let temp_dir = std::env::temp_dir();
-        let config_path = temp_dir.join("test_rb_env.toml");
+        let config_path = temp_dir.join("test_rb_env_mock.toml");
 
         // Create a temporary config file
         fs::write(&config_path, "# test config").expect("Failed to write test config");
 
-        // Set environment variable (unsafe but required for testing)
-        unsafe {
-            std::env::set_var("RB_CONFIG", &config_path);
-        }
+        // Use mock environment - no global state mutation!
+        let mock_env =
+            MockEnvReader::new().with_var("RB_CONFIG", config_path.to_string_lossy().to_string());
 
         // Should return the env var path
-        let result = locate_config_file(None);
+        let result = locate_config_file_with_env(None, &mock_env);
         assert_eq!(result, Some(config_path.clone()));
 
         // Cleanup
-        unsafe {
-            std::env::remove_var("RB_CONFIG");
-        }
         let _ = fs::remove_file(&config_path);
     }
+
+    #[test]
+    fn test_locate_config_file_with_xdg_config_home() {
+        use std::fs;
+        let temp_dir = std::env::temp_dir();
+        let xdg_base = temp_dir.join("test_xdg_config");
+        let rb_dir = xdg_base.join("rb");
+        let config_path = rb_dir.join("rb.toml");
+
+        // Create directory structure
+        fs::create_dir_all(&rb_dir).expect("Failed to create test directory");
+        fs::write(&config_path, "# test config").expect("Failed to write test config");
+
+        // Use mock environment
+        let mock_env = MockEnvReader::new()
+            .with_var("XDG_CONFIG_HOME", xdg_base.to_string_lossy().to_string());
+
+        // Should return the XDG config path
+        let result = locate_config_file_with_env(None, &mock_env);
+        assert_eq!(result, Some(config_path.clone()));
+
+        // Cleanup
+        let _ = fs::remove_dir_all(&xdg_base);
+    }
 }

crates/rb-cli/src/discovery.rs

@@ -45,13 +45,10 @@ impl DiscoveryContext {
 
         // Step 2: Detect bundler environment
         debug!("Detecting bundler environment");
-        let bundler_environment = match BundlerRuntimeDetector::discover(&current_dir) {
-            Ok(Some(bundler)) => {
-                debug!(
-                    "Bundler environment detected at: {}",
-                    bundler.root.display()
-                );
-                Some(bundler)
+        let bundler_root = match BundlerRuntimeDetector::discover(&current_dir) {
+            Ok(Some(root)) => {
+                debug!("Bundler environment detected at: {}", root.display());
+                Some(root)
             }
             Ok(None) => {
                 debug!("No bundler environment detected");
@@ -64,8 +61,10 @@ impl DiscoveryContext {
         };
 
         // Step 3: Determine required Ruby version
-        let required_ruby_version = if let Some(bundler) = &bundler_environment {
-            match bundler.ruby_version() {
+        let required_ruby_version = if bundler_root.is_some() {
+            use rb_core::ruby::CompositeDetector;
+            let detector = CompositeDetector::bundler();
+            match detector.detect(&current_dir) {
                 Some(version) => {
                     debug!("Bundler environment specifies Ruby version: {}", version);
                     Some(version)
@@ -86,7 +85,19 @@ impl DiscoveryContext {
             &required_ruby_version,
         );
 
-        // Step 5: Create butler runtime if we have a selected Ruby
+        // Step 5: Create bundler runtime with selected Ruby version (if bundler detected)
+        let bundler_environment = if let Some(ref root) = bundler_root {
+            if let Some(ref ruby) = selected_ruby {
+                Some(BundlerRuntime::new(root, ruby.version.clone()))
+            } else {
+                // No suitable Ruby found - create with temp version for display purposes
+                Some(BundlerRuntime::new(root, Version::new(0, 0, 0)))
+            }
+        } else {
+            None
+        };
+
+        // Step 6: Create butler runtime if we have a selected Ruby
         let butler_runtime = if let Some(ruby) = &selected_ruby {
             match ruby.infer_gem_runtime() {
                 Ok(gem_runtime) => {

crates/rb-cli/src/lib.rs

@@ -227,11 +227,36 @@ mod tests {
     #[test]
     fn test_create_ruby_context_with_sandbox() {
         let sandbox = RubySandbox::new().expect("Failed to create sandbox");
-        sandbox
+        let ruby_dir = sandbox
             .add_ruby_dir("3.2.5")
             .expect("Failed to create ruby-3.2.5");
 
-        let result = create_ruby_context(Some(sandbox.root().to_path_buf()), None);
+        // Create Ruby executable so it can be discovered
+        std::fs::create_dir_all(ruby_dir.join("bin")).expect("Failed to create bin dir");
+        let ruby_exe = ruby_dir.join("bin").join("ruby");
+        std::fs::write(&ruby_exe, "#!/bin/sh\necho ruby").expect("Failed to write ruby exe");
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            std::fs::set_permissions(&ruby_exe, std::fs::Permissions::from_mode(0o755))
+                .expect("Failed to set permissions");
+        }
+
+        // Create gem directories so gem runtime is inferred
+        let gem_base = sandbox.gem_base_dir();
+        let gem_dir = gem_base.join("3.2.5");
+        std::fs::create_dir_all(&gem_dir).expect("Failed to create gem dir");
+
+        // Use the internal method that accepts current_dir to avoid global state
+        use rb_core::butler::ButlerRuntime;
+        let result = ButlerRuntime::discover_and_compose_with_current_dir(
+            sandbox.root().to_path_buf(),
+            None,
+            None,
+            false,
+            sandbox.root().to_path_buf(), // Current dir = sandbox root
+        )
+        .expect("Failed to create ButlerRuntime");
 
         // Should successfully create a ButlerRuntime
         let current_path = std::env::var("PATH").ok();

crates/rb-core/src/bundler/detector.rs

@@ -1,14 +1,13 @@
 use log::{debug, info};
-use std::path::Path;
-
-use super::BundlerRuntime;
+use std::path::{Path, PathBuf};
 
 pub struct BundlerRuntimeDetector;
 
 impl BundlerRuntimeDetector {
-    /// Discover a BundlerRuntime by searching for Gemfile in the current directory
+    /// Discover a Bundler project by searching for Gemfile in the current directory
     /// and walking up the directory tree until one is found or we reach the root.
-    pub fn discover(start_dir: &Path) -> std::io::Result<Option<BundlerRuntime>> {
+    /// Returns the root directory containing the Gemfile.
+    pub fn discover(start_dir: &Path) -> std::io::Result<Option<PathBuf>> {
         debug!(
             "Starting Bundler discovery from directory: {}",
             start_dir.display()
@@ -22,9 +21,8 @@ impl BundlerRuntimeDetector {
 
             if gemfile_path.exists() && gemfile_path.is_file() {
                 info!("Found Gemfile at: {}", gemfile_path.display());
-                let bundler_runtime = BundlerRuntime::new(&current_dir);
-                debug!("Created BundlerRuntime for root: {}", current_dir.display());
-                return Ok(Some(bundler_runtime));
+                debug!("Returning bundler root: {}", current_dir.display());
+                return Ok(Some(current_dir));
             } else {
                 debug!("No Gemfile found in: {}", current_dir.display());
             }
@@ -50,7 +48,7 @@ impl BundlerRuntimeDetector {
     }
 
     /// Convenience method to discover from current working directory
-    pub fn discover_from_cwd() -> std::io::Result<Option<BundlerRuntime>> {
+    pub fn discover_from_cwd() -> std::io::Result<Option<PathBuf>> {
         let cwd = std::env::current_dir()?;
         debug!(
             "Discovering Bundler runtime from current working directory: {}",
@@ -64,7 +62,6 @@ impl BundlerRuntimeDetector {
 mod tests {
     use super::*;
     use rb_tests::BundlerSandbox;
-    use semver;
     use std::io;
 
     #[test]
@@ -75,9 +72,9 @@ mod tests {
         let result = BundlerRuntimeDetector::discover(&project_dir)?;
 
         assert!(result.is_some());
-        let bundler_runtime = result.unwrap();
-        assert_eq!(bundler_runtime.root, project_dir);
-        assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile"));
+        let bundler_root = result.unwrap();
+        assert_eq!(bundler_root, project_dir);
+        assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile"));
 
         Ok(())
     }
@@ -95,9 +92,9 @@ mod tests {
         let result = BundlerRuntimeDetector::discover(&sub_dir)?;
 
         assert!(result.is_some());
-        let bundler_runtime = result.unwrap();
-        assert_eq!(bundler_runtime.root, project_dir);
-        assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile"));
+        let bundler_root = result.unwrap();
+        assert_eq!(bundler_root, project_dir);
+        assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile"));
 
         Ok(())
     }
@@ -125,9 +122,9 @@ mod tests {
         let result = BundlerRuntimeDetector::discover(&deep_dir)?;
 
         assert!(result.is_some());
-        let bundler_runtime = result.unwrap();
-        assert_eq!(bundler_runtime.root, subproject);
-        assert_eq!(bundler_runtime.gemfile_path(), subproject.join("Gemfile"));
+        let bundler_root = result.unwrap();
+        assert_eq!(bundler_root, subproject);
+        assert_eq!(bundler_root.join("Gemfile"), subproject.join("Gemfile"));
 
         Ok(())
     }
@@ -147,41 +144,9 @@ mod tests {
         let result = BundlerRuntimeDetector::discover(&deep_dir)?;
 
         assert!(result.is_some());
-        let bundler_runtime = result.unwrap();
-        assert_eq!(bundler_runtime.root, project_dir);
-        assert_eq!(bundler_runtime.gemfile_path(), project_dir.join("Gemfile"));
-
-        Ok(())
-    }
-
-    #[test]
-    fn discover_detects_ruby_version_from_project() -> io::Result<()> {
-        let sandbox = BundlerSandbox::new()?;
-        let project_dir = sandbox.add_dir("ruby-version-app")?;
-
-        // Create Gemfile with ruby version
-        let gemfile_content = r#"source 'https://rubygems.org'
-
-ruby '3.2.1'
-
-gem 'rails'
-"#;
-        sandbox.add_file(
-            format!(
-                "{}/Gemfile",
-                project_dir.file_name().unwrap().to_str().unwrap()
-            ),
-            gemfile_content,
-        )?;
-
-        let result = BundlerRuntimeDetector::discover(&project_dir)?;
-
-        assert!(result.is_some());
-        let bundler_runtime = result.unwrap();
-        assert_eq!(
-            bundler_runtime.ruby_version(),
-            Some(semver::Version::parse("3.2.1").unwrap())
-        );
+        let bundler_root = result.unwrap();
+        assert_eq!(bundler_root, project_dir);
+        assert_eq!(bundler_root.join("Gemfile"), project_dir.join("Gemfile"));
 
         Ok(())
     }