test: fix recipe and audio tests to avoid side effects (#6231)

Signed-off-by: Adrian Cole <adrian@tetrate.io>

Cargo.lock

@@ -2453,6 +2453,12 @@ version = "0.1.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c34f04666d835ff5d62e058c3995147c06f42fe86ff053337632bca83e42702d"
 
+[[package]]
+name = "env-lock"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2afbd3cff67810192b5bdb76b77feff72775314d41195c1d779bc92e0e83285d"
+
 [[package]]
 name = "env_filter"
 version = "0.1.3"
@@ -3090,6 +3096,7 @@ dependencies = [
  "dashmap",
  "dirs 5.0.1",
  "dotenvy",
+ "env-lock",
  "etcetera 0.11.0",
  "fs2",
  "futures",
@@ -3308,6 +3315,7 @@ dependencies = [
  "chrono",
  "clap",
  "config",
+ "env-lock",
  "fs2",
  "futures",
  "goose",

crates/goose-cli/src/commands/recipe.rs

@@ -45,39 +45,55 @@ pub fn handle_deeplink(recipe_name: &str, params: &[String]) -> Result<String> {
 }
 
 pub fn handle_open(recipe_name: &str, params: &[String]) -> Result<()> {
-    // Generate the deeplink using the helper function (no printing)
+    handle_open_with(
-    // This reuses all the validation and encoding logic
+        recipe_name,
+        params,
+        |url| open::that(url),
+        &mut std::io::stdout(),
+    )
+}
+
+fn handle_open_with<F, W>(
+    recipe_name: &str,
+    params: &[String],
+    opener: F,
+    out: &mut W,
+) -> Result<()>
+where
+    F: FnOnce(&str) -> std::io::Result<()>,
+    W: std::io::Write,
+{
     let params_map = parse_params(params)?;
     match generate_deeplink(recipe_name, params_map) {
-        Ok((deeplink_url, recipe)) => {
+        Ok((deeplink_url, recipe)) => match opener(&deeplink_url) {
-            // Attempt to open the deeplink
+            Ok(_) => {
-            match open::that(&deeplink_url) {
+                writeln!(
-                Ok(_) => {
+                    out,
-                    println!(
+                    "{} Opened recipe '{}' in Goose Desktop",
-                        "{} Opened recipe '{}' in Goose Desktop",
+                    style("✓").green().bold(),
-                        style("✓").green().bold(),
+                    recipe.title
-                        recipe.title
+                )?;
-                    );
+                Ok(())
-                    Ok(())
-                }
-                Err(err) => {
-                    println!(
-                        "{} Failed to open recipe in Goose Desktop: {}",
-                        style("✗").red().bold(),
-                        err
-                    );
-                    println!("Generated deeplink: {}", deeplink_url);
-                    println!("You can manually copy and open the URL above, or ensure Goose Desktop is installed.");
-                    Err(anyhow::anyhow!("Failed to open recipe: {}", err))
-                }
             }
-        }
+            Err(err) => {
+                writeln!(
+                    out,
+                    "{} Failed to open recipe in Goose Desktop: {}",
+                    style("✗").red().bold(),
+                    err
+                )?;
+                writeln!(out, "Generated deeplink: {}", deeplink_url)?;
+                writeln!(out, "You can manually copy and open the URL above, or ensure Goose Desktop is installed.")?;
+                Err(anyhow::anyhow!("Failed to open recipe: {}", err))
+            }
+        },
         Err(err) => {
-            println!(
+            writeln!(
+                out,
                 "{} Failed to encode recipe: {}",
                 style("✗").red().bold(),
                 err
-            );
+            )?;
             Err(err)
         }
     }
@@ -250,47 +266,82 @@ instructions: "Test instructions"
         assert!(result.is_err());
     }
 
+    fn run_handle_open(
+        recipe_path: &str,
+        params: &[String],
+        opener_result: std::io::Result<()>,
+    ) -> (Result<()>, String, String) {
+        let captured_url = std::cell::RefCell::new(String::new());
+        let mut out = Vec::new();
+        let result = handle_open_with(
+            recipe_path,
+            params,
+            |url| {
+                *captured_url.borrow_mut() = url.to_string();
+                opener_result
+            },
+            &mut out,
+        );
+        let output = String::from_utf8(out).unwrap();
+        (result, captured_url.into_inner(), output)
+    }
+
     #[test]
     fn test_handle_open_recipe() {
-        let temp_dir = TempDir::new().expect("Failed to create temp directory");
+        let temp_dir = TempDir::new().unwrap();
         let recipe_path =
             create_test_recipe_file(&temp_dir, "test_recipe.yaml", VALID_RECIPE_CONTENT);
 
-        // Test handle_open - should attempt to open but may fail (that's expected in test environment)
+        let (expected_url, _) = generate_deeplink(&recipe_path, HashMap::new()).unwrap();
-        // We just want to ensure it doesn't panic and handles the error gracefully
+        let (result, captured_url, _) = run_handle_open(&recipe_path, &[], Ok(()));
-        let result = handle_open(&recipe_path, &[]);
+
-        // The result may be Ok or Err depending on whether the system can open the URL
+        assert!(result.is_ok());
-        // In a test environment, it will likely fail to open, but that's fine
+        assert_eq!(captured_url, expected_url);
-        // We're mainly testing that the function doesn't panic and processes the recipe correctly
-        match result {
-            Ok(_) => {
-                // Successfully opened (unlikely in test environment)
-            }
-            Err(_) => {
-                // Failed to open (expected in test environment) - this is fine
-            }
-        }
     }
 
     #[test]
     fn test_handle_open_with_parameters() {
-        let temp_dir = TempDir::new().expect("Failed to create temp directory");
+        let temp_dir = TempDir::new().unwrap();
         let recipe_path =
             create_test_recipe_file(&temp_dir, "test_recipe.yaml", VALID_RECIPE_CONTENT);
 
+        let (base_url, _) = generate_deeplink(&recipe_path, HashMap::new()).unwrap();
+
         let params = vec!["name=Alice".to_string(), "role=developer".to_string()];
-        let result = handle_open(&recipe_path, &params);
+        let (result, captured_url, _) = run_handle_open(&recipe_path, &params, Ok(()));
-        // The result may be Ok or Err depending on whether the system can open the URL
+
-        // In a test environment, it will likely fail to open, but that's fine
+        assert!(result.is_ok());
-        // We're mainly testing that the function processes parameters correctly and doesn't panic
+        assert!(captured_url.starts_with(&base_url));
-        match result {
+        assert!(captured_url.contains("&name=Alice"));
-            Ok(_) => {
+        assert!(captured_url.contains("&role=developer"));
-                // Successfully opened (unlikely in test environment)
+    }
-            }
+
-            Err(_) => {
+    #[test]
-                // Failed to open (expected in test environment) - this is fine
+    fn test_handle_open_opener_fails() {
-            }
+        let temp_dir = TempDir::new().unwrap();
-        }
+        let recipe_path =
+            create_test_recipe_file(&temp_dir, "test_recipe.yaml", VALID_RECIPE_CONTENT);
+
+        let (expected_url, _) = generate_deeplink(&recipe_path, HashMap::new()).unwrap();
+        let opener_err = std::io::Error::new(std::io::ErrorKind::NotFound, "desktop not found");
+        let (result, _, output) = run_handle_open(&recipe_path, &[], Err(opener_err));
+
+        assert!(result.is_err());
+        assert!(output.contains("Failed to open recipe in Goose Desktop"));
+        assert!(output.contains("desktop not found"));
+        assert!(output.contains(&expected_url));
+    }
+
+    #[test]
+    fn test_handle_open_invalid_recipe() {
+        let temp_dir = TempDir::new().unwrap();
+        let recipe_path =
+            create_test_recipe_file(&temp_dir, "invalid.yaml", INVALID_RECIPE_CONTENT);
+
+        let (result, _, output) = run_handle_open(&recipe_path, &[], Ok(()));
+
+        assert!(result.is_err());
+        assert!(output.contains("Failed to encode recipe"));
     }
 
     #[test]

crates/goose-server/Cargo.toml

@@ -62,3 +62,4 @@ path = "src/bin/generate_schema.rs"
 tower = "0.5"
 async-trait = "0.1.89"
 tempfile = "3.15.0"
+env-lock = "1.0.1"

crates/goose-server/src/routes/audio.rs

@@ -395,6 +395,8 @@ mod tests {
 
     #[tokio::test(flavor = "multi_thread")]
     async fn test_transcribe_endpoint_requires_auth() {
+        let _guard = env_lock::lock_env([("OPENAI_API_KEY", Some("fake-openai-no-keyring"))]);
+
         let state = AppState::new().await.unwrap();
         let app = routes(state);
         // Test without auth header

crates/goose/Cargo.toml

@@ -132,6 +132,7 @@ temp-env = "0.3.6"
 dotenvy = "0.15.7"
 ctor = "0.2.9"
 test-case = "3.3"
+env-lock = "1.0.1"
 
 [[example]]
 name = "agent"

crates/goose/src/providers/base.rs

@@ -338,6 +338,9 @@ pub trait LeadWorkerProviderTrait {
 
     /// Get the currently active model name
     fn get_active_model(&self) -> String;
+
+    /// Get (lead_turns, failure_threshold, fallback_turns)
+    fn get_settings(&self) -> (usize, usize, usize);
 }
 
 /// Base trait for AI providers (OpenAI, Anthropic, etc)

crates/goose/src/providers/factory.rs

@@ -222,14 +222,10 @@ fn create_worker_model_config(default_model: &ModelConfig) -> Result<ModelConfig
 
     let global_config = crate::config::Config::global();
 
-    if let Ok(limit_str) = global_config.get_param::<String>("GOOSE_WORKER_CONTEXT_LIMIT") {
+    if let Ok(limit) = global_config.get_param::<usize>("GOOSE_WORKER_CONTEXT_LIMIT") {
-        if let Ok(limit) = limit_str.parse::<usize>() {
+        worker_config = worker_config.with_context_limit(Some(limit));
-            worker_config = worker_config.with_context_limit(Some(limit));
+    } else if let Ok(limit) = global_config.get_param::<usize>("GOOSE_CONTEXT_LIMIT") {
-        }
+        worker_config = worker_config.with_context_limit(Some(limit));
-    } else if let Ok(limit_str) = global_config.get_param::<String>("GOOSE_CONTEXT_LIMIT") {
-        if let Ok(limit) = limit_str.parse::<usize>() {
-            worker_config = worker_config.with_context_limit(Some(limit));
-        }
     }
 
     Ok(worker_config)
@@ -238,148 +234,76 @@ fn create_worker_model_config(default_model: &ModelConfig) -> Result<ModelConfig
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::env;
-
-    struct EnvVarGuard {
-        vars: Vec<(String, Option<String>)>,
-    }
-
-    impl EnvVarGuard {
-        fn new(vars: &[&str]) -> Self {
-            let saved_vars = vars
-                .iter()
-                .map(|&var| (var.to_string(), env::var(var).ok()))
-                .collect();
-
-            for &var in vars {
-                env::remove_var(var);
-            }
-
-            Self { vars: saved_vars }
-        }
-
-        fn set(&self, key: &str, value: &str) {
-            env::set_var(key, value);
-        }
-    }
-
-    impl Drop for EnvVarGuard {
-        fn drop(&mut self) {
-            for (key, value) in &self.vars {
-                match value {
-                    Some(val) => env::set_var(key, val),
-                    None => env::remove_var(key),
-                }
-            }
-        }
-    }
 
+    #[test_case::test_case(None, None, None, DEFAULT_LEAD_TURNS, DEFAULT_FAILURE_THRESHOLD, DEFAULT_FALLBACK_TURNS ; "defaults")]
+    #[test_case::test_case(Some("7"), Some("4"), Some("3"), 7, 4, 3 ; "custom")]
     #[tokio::test]
-    async fn test_create_lead_worker_provider() {
+    async fn test_create_lead_worker_provider(
-        // Both API keys needed: openai for worker, anthropic for lead (GOOSE_LEAD_PROVIDER=anthropic)
+        lead_turns: Option<&str>,
-        let _guard = EnvVarGuard::new(&[
+        failure_threshold: Option<&str>,
-            "GOOSE_LEAD_MODEL",
+        fallback_turns: Option<&str>,
-            "GOOSE_LEAD_PROVIDER",
+        expected_turns: usize,
-            "GOOSE_LEAD_TURNS",
+        expected_failure: usize,
-            "OPENAI_API_KEY",
+        expected_fallback: usize,
-            "ANTHROPIC_API_KEY",
+    ) {
+        let _guard = env_lock::lock_env([
+            ("GOOSE_LEAD_MODEL", Some("gpt-4o")),
+            ("GOOSE_LEAD_PROVIDER", None),
+            ("GOOSE_LEAD_TURNS", lead_turns),
+            ("GOOSE_LEAD_FAILURE_THRESHOLD", failure_threshold),
+            ("GOOSE_LEAD_FALLBACK_TURNS", fallback_turns),
+            ("OPENAI_API_KEY", Some("fake-openai-no-keyring")),
         ]);
 
-        _guard.set("OPENAI_API_KEY", "fake-openai-no-keyring");
+        let provider = create("openai", ModelConfig::new_or_fail("gpt-4o-mini"))
-        _guard.set("ANTHROPIC_API_KEY", "fake-anthropic-no-keyring");
+            .await
-        _guard.set("GOOSE_LEAD_MODEL", "gpt-4o");
+            .unwrap();
-
+        let lw = provider.as_lead_worker().unwrap();
-        let gpt4mini_config = ModelConfig::new_or_fail("gpt-4o-mini");
+        let (lead, worker) = lw.get_model_info();
-        let result = create("openai", gpt4mini_config.clone()).await;
+        assert_eq!(lead, "gpt-4o");
-
+        assert_eq!(worker, "gpt-4o-mini");
-        match result {
+        assert_eq!(
-            Ok(_) => {}
+            lw.get_settings(),
-            Err(error) => {
+            (expected_turns, expected_failure, expected_fallback)
-                let error_msg = error.to_string();
+        );
-                assert!(error_msg.contains("OPENAI_API_KEY") || error_msg.contains("secret"));
-            }
-        }
-
-        _guard.set("GOOSE_LEAD_PROVIDER", "anthropic");
-        _guard.set("GOOSE_LEAD_TURNS", "5");
-
-        let _result = create("openai", gpt4mini_config).await;
-    }
-
-    #[tokio::test]
-    async fn test_lead_model_env_vars_with_defaults() {
-        let _guard = EnvVarGuard::new(&[
-            "GOOSE_LEAD_MODEL",
-            "GOOSE_LEAD_PROVIDER",
-            "GOOSE_LEAD_TURNS",
-            "GOOSE_LEAD_FAILURE_THRESHOLD",
-            "GOOSE_LEAD_FALLBACK_TURNS",
-            "OPENAI_API_KEY",
-        ]);
-
-        _guard.set("OPENAI_API_KEY", "fake-openai-no-keyring");
-        _guard.set("GOOSE_LEAD_MODEL", "grok-3");
-
-        let result = create("openai", ModelConfig::new_or_fail("gpt-4o-mini")).await;
-
-        match result {
-            Ok(_) => {}
-            Err(error) => {
-                let error_msg = error.to_string();
-                assert!(error_msg.contains("OPENAI_API_KEY") || error_msg.contains("secret"));
-            }
-        }
-
-        _guard.set("GOOSE_LEAD_TURNS", "7");
-        _guard.set("GOOSE_LEAD_FAILURE_THRESHOLD", "4");
-        _guard.set("GOOSE_LEAD_FALLBACK_TURNS", "3");
-
-        let _result = create("openai", ModelConfig::new_or_fail("gpt-4o-mini"));
     }
 
     #[tokio::test]
     async fn test_create_regular_provider_without_lead_config() {
-        let _guard = EnvVarGuard::new(&[
+        let _guard = env_lock::lock_env([
-            "GOOSE_LEAD_MODEL",
+            ("GOOSE_LEAD_MODEL", None),
-            "GOOSE_LEAD_PROVIDER",
+            ("GOOSE_LEAD_PROVIDER", None),
-            "GOOSE_LEAD_TURNS",
+            ("GOOSE_LEAD_TURNS", None),
-            "GOOSE_LEAD_FAILURE_THRESHOLD",
+            ("GOOSE_LEAD_FAILURE_THRESHOLD", None),
-            "GOOSE_LEAD_FALLBACK_TURNS",
+            ("GOOSE_LEAD_FALLBACK_TURNS", None),
-            "OPENAI_API_KEY",
+            ("OPENAI_API_KEY", Some("fake-openai-no-keyring")),
         ]);
 
-        _guard.set("OPENAI_API_KEY", "fake-openai-no-keyring");
+        let provider = create("openai", ModelConfig::new_or_fail("gpt-4o-mini"))
-        let result = create("openai", ModelConfig::new_or_fail("gpt-4o-mini")).await;
+            .await
-
+            .unwrap();
-        match result {
+        assert!(provider.as_lead_worker().is_none());
-            Ok(_) => {}
+        assert_eq!(provider.get_model_config().model_name, "gpt-4o-mini");
-            Err(error) => {
-                let error_msg = error.to_string();
-                assert!(error_msg.contains("OPENAI_API_KEY") || error_msg.contains("secret"));
-            }
-        }
     }
 
-    #[test]
+    #[test_case::test_case(None, None, 16_000 ; "no overrides uses default")]
-    fn test_worker_model_preserves_original_context_limit() {
+    #[test_case::test_case(Some("32000"), None, 32_000 ; "worker limit overrides default")]
-        let _guard = EnvVarGuard::new(&[
+    #[test_case::test_case(Some("32000"), Some("64000"), 32_000 ; "worker limit takes priority over global")]
-            "GOOSE_LEAD_MODEL",
+    fn test_worker_model_context_limit(
-            "GOOSE_WORKER_CONTEXT_LIMIT",
+        worker_limit: Option<&str>,
-            "GOOSE_CONTEXT_LIMIT",
+        global_limit: Option<&str>,
+        expected_limit: usize,
+    ) {
+        let _guard = env_lock::lock_env([
+            ("GOOSE_WORKER_CONTEXT_LIMIT", worker_limit),
+            ("GOOSE_CONTEXT_LIMIT", global_limit),
         ]);
 
-        _guard.set("GOOSE_LEAD_MODEL", "gpt-4o");
-
         let default_model =
             ModelConfig::new_or_fail("gpt-3.5-turbo").with_context_limit(Some(16_000));
 
-        let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o");
+        let result = create_worker_model_config(&default_model).unwrap();
-
+        assert_eq!(result.context_limit, Some(expected_limit));
-        _guard.set("GOOSE_WORKER_CONTEXT_LIMIT", "32000");
-        let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o");
-
-        _guard.set("GOOSE_CONTEXT_LIMIT", "64000");
-        let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o");
     }
 
     #[tokio::test]

crates/goose/src/providers/lead_worker.rs

@@ -303,6 +303,15 @@ impl LeadWorkerProviderTrait for LeadWorkerProvider {
             self.lead_provider.get_model_config().model_name
         })
     }
+
+    /// Get (lead_turns, failure_threshold, fallback_turns)
+    fn get_settings(&self) -> (usize, usize, usize) {
+        (
+            self.lead_turns,
+            self.max_failures_before_fallback,
+            self.fallback_turns,
+        )
+    }
 }
 
 #[async_trait]