From 030e8a82b5345e5ecb35e52ca5b11f0283ab4bf2 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Thu, 12 Jun 2025 14:46:56 +1000 Subject: [PATCH] feat/fix: don't stop cli starting if MCPs don't load (#2860) --- crates/goose-cli/src/cli.rs | 3 + crates/goose-cli/src/commands/bench.rs | 1 + crates/goose-cli/src/session/builder.rs | 280 ++++++++++++++++++++++-- 3 files changed, 271 insertions(+), 13 deletions(-) diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index e4239c64..5832330b 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -632,6 +632,7 @@ pub async fn cli() -> Result<()> { additional_system_prompt: None, debug, max_tool_repetitions, + interactive: true, // Session command is always interactive }) .await; setup_logging( @@ -740,6 +741,7 @@ pub async fn cli() -> Result<()> { additional_system_prompt: input_config.additional_system_prompt, debug, max_tool_repetitions, + interactive, // Use the interactive flag from the Run command }) .await; @@ -854,6 +856,7 @@ pub async fn cli() -> Result<()> { additional_system_prompt: None, debug: false, max_tool_repetitions: None, + interactive: true, // Default case is always interactive }) .await; setup_logging( diff --git a/crates/goose-cli/src/commands/bench.rs b/crates/goose-cli/src/commands/bench.rs index ee9514e2..1d248833 100644 --- a/crates/goose-cli/src/commands/bench.rs +++ b/crates/goose-cli/src/commands/bench.rs @@ -42,6 +42,7 @@ pub async fn agent_generator( additional_system_prompt: None, debug: false, max_tool_repetitions: None, + interactive: false, // Benchmarking is non-interactive }) .await; diff --git a/crates/goose-cli/src/session/builder.rs b/crates/goose-cli/src/session/builder.rs index 1190220b..3c314b74 100644 --- a/crates/goose-cli/src/session/builder.rs +++ b/crates/goose-cli/src/session/builder.rs @@ -38,6 +38,102 @@ pub struct SessionBuilderConfig { pub debug: bool, /// Maximum number of consecutive identical tool calls allowed pub max_tool_repetitions: Option, + /// Whether this session will be used interactively (affects debugging prompts) + pub interactive: bool, +} + +/// Offers to help debug an extension failure by creating a minimal debugging session +async fn offer_extension_debugging_help( + extension_name: &str, + error_message: &str, + provider: Arc, + interactive: bool, +) -> Result<(), anyhow::Error> { + // Only offer debugging help in interactive mode + if !interactive { + return Ok(()); + } + + let help_prompt = format!( + "Would you like me to help debug the '{}' extension failure?", + extension_name + ); + + let should_help = match cliclack::confirm(help_prompt) + .initial_value(false) + .interact() + { + Ok(choice) => choice, + Err(e) => { + if e.kind() == std::io::ErrorKind::Interrupted { + return Ok(()); + } else { + return Err(e.into()); + } + } + }; + + if !should_help { + return Ok(()); + } + + println!("{}", style("🔧 Starting debugging session...").cyan()); + + // Create a debugging prompt with context about the extension failure + let debug_prompt = format!( + "I'm having trouble starting an extension called '{}'. Here's the error I encountered:\n\n{}\n\nCan you help me diagnose what might be wrong and suggest how to fix it? Please consider common issues like:\n- Missing dependencies or tools\n- Configuration problems\n- Network connectivity (for remote extensions)\n- Permission issues\n- Path or environment variable problems", + extension_name, + error_message + ); + + // Create a minimal agent for debugging + let debug_agent = Agent::new(); + debug_agent.update_provider(provider).await?; + + // Add the developer extension if available to help with debugging + if let Ok(extensions) = ExtensionConfigManager::get_all() { + for ext_wrapper in extensions { + if ext_wrapper.enabled && ext_wrapper.config.name() == "developer" { + if let Err(e) = debug_agent.add_extension(ext_wrapper.config).await { + // If we can't add developer extension, continue without it + eprintln!( + "Note: Could not load developer extension for debugging: {}", + e + ); + } + break; + } + } + } + + // Create a temporary session file for this debugging session + let temp_session_file = + std::env::temp_dir().join(format!("goose_debug_extension_{}.jsonl", extension_name)); + + // Create the debugging session + let mut debug_session = Session::new(debug_agent, temp_session_file.clone(), false); + + // Process the debugging request + println!("{}", style("Analyzing the extension failure...").yellow()); + match debug_session.headless(debug_prompt).await { + Ok(_) => { + println!( + "{}", + style("✅ Debugging session completed. Check the suggestions above.").green() + ); + } + Err(e) => { + eprintln!( + "{}", + style(format!("❌ Debugging session failed: {}", e)).red() + ); + } + } + + // Clean up the temporary session file + let _ = std::fs::remove_file(temp_session_file); + + Ok(()) } pub async fn build_session(session_config: SessionBuilderConfig) -> Session { @@ -180,12 +276,35 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session { ExtensionError::Transport(McpClientError::StdioProcessError(inner)) => inner, _ => e.to_string(), }; - eprintln!("Failed to start extension: {}, {:?}", extension.name(), err); eprintln!( - "Please check extension configuration for {}.", - extension.name() + "{}", + style(format!( + "Warning: Failed to start extension '{}': {}", + extension.name(), + err + )) + .yellow() ); - process::exit(1); + eprintln!( + "{}", + style(format!( + "Continuing without extension '{}'", + extension.name() + )) + .yellow() + ); + + // Offer debugging help + if let Err(debug_err) = offer_extension_debugging_help( + &extension.name(), + &err, + Arc::clone(&provider_for_display), + session_config.interactive, + ) + .await + { + eprintln!("Note: Could not start debugging session: {}", debug_err); + } } } @@ -194,25 +313,99 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session { // Add extensions if provided for extension_str in session_config.extensions { - if let Err(e) = session.add_extension(extension_str).await { - eprintln!("Failed to start extension: {}", e); - process::exit(1); + if let Err(e) = session.add_extension(extension_str.clone()).await { + eprintln!( + "{}", + style(format!( + "Warning: Failed to start extension '{}': {}", + extension_str, e + )) + .yellow() + ); + eprintln!( + "{}", + style(format!("Continuing without extension '{}'", extension_str)).yellow() + ); + + // Offer debugging help + if let Err(debug_err) = offer_extension_debugging_help( + &extension_str, + &e.to_string(), + Arc::clone(&provider_for_display), + session_config.interactive, + ) + .await + { + eprintln!("Note: Could not start debugging session: {}", debug_err); + } } } // Add remote extensions if provided for extension_str in session_config.remote_extensions { - if let Err(e) = session.add_remote_extension(extension_str).await { - eprintln!("Failed to start extension: {}", e); - process::exit(1); + if let Err(e) = session.add_remote_extension(extension_str.clone()).await { + eprintln!( + "{}", + style(format!( + "Warning: Failed to start remote extension '{}': {}", + extension_str, e + )) + .yellow() + ); + eprintln!( + "{}", + style(format!( + "Continuing without remote extension '{}'", + extension_str + )) + .yellow() + ); + + // Offer debugging help + if let Err(debug_err) = offer_extension_debugging_help( + &extension_str, + &e.to_string(), + Arc::clone(&provider_for_display), + session_config.interactive, + ) + .await + { + eprintln!("Note: Could not start debugging session: {}", debug_err); + } } } // Add builtin extensions for builtin in session_config.builtins { - if let Err(e) = session.add_builtin(builtin).await { - eprintln!("Failed to start builtin extension: {}", e); - process::exit(1); + if let Err(e) = session.add_builtin(builtin.clone()).await { + eprintln!( + "{}", + style(format!( + "Warning: Failed to start builtin extension '{}': {}", + builtin, e + )) + .yellow() + ); + eprintln!( + "{}", + style(format!( + "Continuing without builtin extension '{}'", + builtin + )) + .yellow() + ); + + // Offer debugging help + if let Err(debug_err) = offer_extension_debugging_help( + &builtin, + &e.to_string(), + Arc::clone(&provider_for_display), + session_config.interactive, + ) + .await + { + eprintln!("Note: Could not start debugging session: {}", debug_err); + } } } @@ -243,3 +436,64 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session { ); session } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_session_builder_config_creation() { + let config = SessionBuilderConfig { + identifier: Some(Identifier::Name("test".to_string())), + resume: false, + no_session: false, + extensions: vec!["echo test".to_string()], + remote_extensions: vec!["http://example.com".to_string()], + builtins: vec!["developer".to_string()], + extensions_override: None, + additional_system_prompt: Some("Test prompt".to_string()), + debug: true, + max_tool_repetitions: Some(5), + interactive: true, + }; + + assert_eq!(config.extensions.len(), 1); + assert_eq!(config.remote_extensions.len(), 1); + assert_eq!(config.builtins.len(), 1); + assert!(config.debug); + assert_eq!(config.max_tool_repetitions, Some(5)); + assert!(config.interactive); + } + + #[test] + fn test_session_builder_config_default() { + let config = SessionBuilderConfig::default(); + + assert!(config.identifier.is_none()); + assert!(!config.resume); + assert!(!config.no_session); + assert!(config.extensions.is_empty()); + assert!(config.remote_extensions.is_empty()); + assert!(config.builtins.is_empty()); + assert!(config.extensions_override.is_none()); + assert!(config.additional_system_prompt.is_none()); + assert!(!config.debug); + assert!(config.max_tool_repetitions.is_none()); + assert!(!config.interactive); + } + + #[tokio::test] + async fn test_offer_extension_debugging_help_function_exists() { + // This test just verifies the function compiles and can be called + // We can't easily test the interactive parts without mocking + + // We can't actually test the full function without a real provider and user interaction + // But we can at least verify it compiles and the function signature is correct + let extension_name = "test-extension"; + let error_message = "test error"; + + // This test mainly serves as a compilation check + assert_eq!(extension_name, "test-extension"); + assert_eq!(error_message, "test error"); + } +}