diff --git a/src/extensions/discovery.rs b/src/extensions/discovery.rs index 51123dc3..04cb366b 100644 --- a/src/extensions/discovery.rs +++ b/src/extensions/discovery.rs @@ -104,6 +104,7 @@ impl OnlineDiscovery { source: ExtensionSource::McpUrl { url: url.to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }) } else { @@ -178,6 +179,7 @@ impl OnlineDiscovery { .unwrap_or_else(|| format!("MCP server from GitHub: {}", item.full_name)), keywords: item.topics, source: ExtensionSource::Discovered { url }, + fallback_source: None, auth_hint: AuthHint::Dcr, }) }) diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 884b9706..52814340 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -545,10 +545,41 @@ impl ExtensionManager { async fn install_from_entry( &self, entry: &RegistryEntry, + ) -> Result { + let primary_result = self.try_install_from_source(entry, &entry.source).await; + match fallback_decision(&primary_result, &entry.fallback_source) { + FallbackDecision::Return => primary_result, + FallbackDecision::TryFallback => { + let primary_err = primary_result.unwrap_err(); + let fallback = entry.fallback_source.as_ref().unwrap(); + tracing::info!( + extension = %entry.name, + primary_error = %primary_err, + "Primary install failed, trying fallback source" + ); + self.try_install_from_source(entry, fallback) + .await + .map_err(|fallback_err| { + tracing::error!( + extension = %entry.name, + fallback_error = %fallback_err, + "Fallback install also failed" + ); + combine_install_errors(&primary_err, fallback_err) + }) + } + } + } + + /// Attempt to install an extension using a specific source. + async fn try_install_from_source( + &self, + entry: &RegistryEntry, + source: &ExtensionSource, ) -> Result { match entry.kind { ExtensionKind::McpServer => { - let url = match &entry.source { + let url = match source { ExtensionSource::McpUrl { url } => url.clone(), ExtensionSource::Discovered { url } => url.clone(), _ => { @@ -559,7 +590,7 @@ impl ExtensionManager { }; self.install_mcp_from_url(&entry.name, &url).await } - ExtensionKind::WasmTool => match &entry.source { + ExtensionKind::WasmTool => match source { ExtensionSource::WasmDownload { wasm_url, capabilities_url, @@ -586,10 +617,10 @@ impl ExtensionManager { .await } _ => Err(ExtensionError::InstallFailed( - "WASM tool entry has no download URL".to_string(), + "WASM tool entry has no download URL or build info".to_string(), )), }, - ExtensionKind::WasmChannel => match &entry.source { + ExtensionKind::WasmChannel => match source { ExtensionSource::WasmDownload { wasm_url, capabilities_url, @@ -616,7 +647,7 @@ impl ExtensionManager { .await } _ => Err(ExtensionError::InstallFailed( - "WASM channel entry has no download URL".to_string(), + "WASM channel entry has no download URL or build info".to_string(), )), }, } @@ -2228,10 +2259,56 @@ fn infer_kind_from_url(url: &str) -> ExtensionKind { } } +/// Decision from `fallback_decision`: should we try the fallback source or +/// return the primary result as-is? +enum FallbackDecision { + /// Return the primary result directly (success or non-retriable error). + Return, + /// Primary failed with a retriable error and a fallback source is available. + TryFallback, +} + +/// Decide whether to attempt a fallback install based on the primary result +/// and the availability of a fallback source. +fn fallback_decision( + primary_result: &Result, + fallback_source: &Option>, +) -> FallbackDecision { + match (primary_result, fallback_source) { + // Success — no fallback needed + (Ok(_), _) => FallbackDecision::Return, + // AlreadyInstalled — don't try building from source + (Err(ExtensionError::AlreadyInstalled(_)), _) => FallbackDecision::Return, + // Failed with a fallback available — try it + (Err(_), Some(_)) => FallbackDecision::TryFallback, + // Failed with no fallback — return the error + (Err(_), None) => FallbackDecision::Return, + } +} + +/// Combine primary and fallback errors into a single error. +/// +/// Preserves `AlreadyInstalled` from the fallback directly; otherwise wraps +/// both error messages into `ExtensionError::Other`. +fn combine_install_errors( + primary_err: &ExtensionError, + fallback_err: ExtensionError, +) -> ExtensionError { + if matches!(fallback_err, ExtensionError::AlreadyInstalled(_)) { + return fallback_err; + } + ExtensionError::Other(format!( + "Primary install failed: {}; fallback install also failed: {}", + primary_err, fallback_err + )) +} + #[cfg(test)] mod tests { - use crate::extensions::ExtensionKind; - use crate::extensions::manager::infer_kind_from_url; + use crate::extensions::manager::{ + FallbackDecision, combine_install_errors, fallback_decision, infer_kind_from_url, + }; + use crate::extensions::{ExtensionError, ExtensionKind, ExtensionSource, InstallResult}; #[test] fn test_infer_kind_from_url() { @@ -2252,4 +2329,86 @@ mod tests { ExtensionKind::McpServer ); } + + // ---- fallback install logic tests ---- + + fn make_ok_result() -> Result { + Ok(InstallResult { + name: "test".to_string(), + kind: ExtensionKind::WasmTool, + message: "Installed".to_string(), + }) + } + + fn make_fallback_source() -> Option> { + Some(Box::new(ExtensionSource::WasmBuildable { + repo_url: "tools-src/test".to_string(), + build_dir: Some("tools-src/test".to_string()), + crate_name: Some("test-tool".to_string()), + })) + } + + #[test] + fn test_fallback_decision_success_returns_directly() { + let result = make_ok_result(); + let fallback = make_fallback_source(); + assert!(matches!( + fallback_decision(&result, &fallback), + FallbackDecision::Return + )); + } + + #[test] + fn test_fallback_decision_already_installed_skips_fallback() { + let result: Result = + Err(ExtensionError::AlreadyInstalled("test".to_string())); + let fallback = make_fallback_source(); + assert!(matches!( + fallback_decision(&result, &fallback), + FallbackDecision::Return + )); + } + + #[test] + fn test_fallback_decision_download_failed_triggers_fallback() { + let result: Result = + Err(ExtensionError::DownloadFailed("404 Not Found".to_string())); + let fallback = make_fallback_source(); + assert!(matches!( + fallback_decision(&result, &fallback), + FallbackDecision::TryFallback + )); + } + + #[test] + fn test_fallback_decision_error_without_fallback_returns() { + let result: Result = + Err(ExtensionError::DownloadFailed("404 Not Found".to_string())); + let fallback = None; + assert!(matches!( + fallback_decision(&result, &fallback), + FallbackDecision::Return + )); + } + + #[test] + fn test_combine_errors_includes_both_messages() { + let primary = ExtensionError::DownloadFailed("404 Not Found".to_string()); + let fallback = ExtensionError::InstallFailed("cargo not found".to_string()); + let combined = combine_install_errors(&primary, fallback); + let msg = combined.to_string(); + assert!(msg.contains("404 Not Found"), "missing primary: {msg}"); + assert!(msg.contains("cargo not found"), "missing fallback: {msg}"); + } + + #[test] + fn test_combine_errors_forwards_already_installed_from_fallback() { + let primary = ExtensionError::DownloadFailed("404".to_string()); + let fallback = ExtensionError::AlreadyInstalled("test".to_string()); + let combined = combine_install_errors(&primary, fallback); + assert!( + matches!(combined, ExtensionError::AlreadyInstalled(ref name) if name == "test"), + "Expected AlreadyInstalled, got: {combined:?}" + ); + } } diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index d9b36291..cb45ed02 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -64,6 +64,9 @@ pub struct RegistryEntry { pub keywords: Vec, /// Where to get this extension. pub source: ExtensionSource, + /// Fallback source when the primary source fails (e.g., download 404 → build from source). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub fallback_source: Option>, /// How authentication works. pub auth_hint: AuthHint, } diff --git a/src/extensions/registry.rs b/src/extensions/registry.rs index 5f925427..95b96bd1 100644 --- a/src/extensions/registry.rs +++ b/src/extensions/registry.rs @@ -208,6 +208,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.notion.com/mcp".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -227,6 +228,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.linear.app".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -245,6 +247,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.google.com/calendar".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -263,6 +266,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.google.com/drive".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -282,6 +286,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.github.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -301,6 +306,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.slack.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -320,6 +326,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.sentry.dev/sse".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -339,6 +346,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.stripe.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -358,6 +366,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.cloudflare.com/sse".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -375,6 +384,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.asana.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, RegistryEntry { @@ -393,6 +403,7 @@ fn builtin_entries() -> Vec { source: ExtensionSource::McpUrl { url: "https://mcp.intercom.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }, // WASM channels (telegram, slack, discord, whatsapp) come from the embedded @@ -417,6 +428,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }; @@ -439,6 +451,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }; @@ -461,6 +474,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }; @@ -483,6 +497,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }; @@ -546,6 +561,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://custom.example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }; @@ -571,6 +587,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://example.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::None, }; @@ -595,6 +612,7 @@ mod tests { build_dir: Some("channels-src/telegram".to_string()), crate_name: Some("telegram-channel".to_string()), }, + fallback_source: None, auth_hint: AuthHint::CapabilitiesAuth, }, // This shares a name with the builtin slack-mcp but has a different kind, so both should appear @@ -609,6 +627,7 @@ mod tests { build_dir: Some("tools-src/slack".to_string()), crate_name: Some("slack-tool".to_string()), }, + fallback_source: None, auth_hint: AuthHint::CapabilitiesAuth, }, ]; @@ -644,6 +663,7 @@ mod tests { source: ExtensionSource::McpUrl { url: "https://other.slack.com".to_string(), }, + fallback_source: None, auth_hint: AuthHint::Dcr, }]; diff --git a/src/registry/manifest.rs b/src/registry/manifest.rs index d5b3fedf..de84aa31 100644 --- a/src/registry/manifest.rs +++ b/src/registry/manifest.rs @@ -154,26 +154,29 @@ impl ExtensionManifest { /// Convert this manifest into a [`RegistryEntry`] for use with the in-chat /// extension discovery system. pub fn to_registry_entry(&self) -> RegistryEntry { - // Prefer pre-built artifact download when a URL is available - let source = if let Some(artifact) = self.artifacts.get("wasm32-wasip2") { + let buildable = ExtensionSource::WasmBuildable { + repo_url: self.source.dir.clone(), + build_dir: Some(self.source.dir.clone()), + crate_name: Some(self.source.crate_name.clone()), + }; + + // Prefer pre-built artifact download when a URL is available, + // with build-from-source as fallback in case the download fails (e.g., 404). + let (source, fallback_source) = if let Some(artifact) = self.artifacts.get("wasm32-wasip2") + { if let Some(ref url) = artifact.url { - ExtensionSource::WasmDownload { - wasm_url: url.clone(), - capabilities_url: artifact.capabilities_url.clone(), - } + ( + ExtensionSource::WasmDownload { + wasm_url: url.clone(), + capabilities_url: artifact.capabilities_url.clone(), + }, + Some(Box::new(buildable)), + ) } else { - ExtensionSource::WasmBuildable { - repo_url: self.source.dir.clone(), - build_dir: Some(self.source.dir.clone()), - crate_name: Some(self.source.crate_name.clone()), - } + (buildable, None) } } else { - ExtensionSource::WasmBuildable { - repo_url: self.source.dir.clone(), - build_dir: Some(self.source.dir.clone()), - crate_name: Some(self.source.crate_name.clone()), - } + (buildable, None) }; let auth_hint = match self.auth_summary.as_ref().and_then(|a| a.method.as_deref()) { @@ -190,6 +193,7 @@ impl ExtensionManifest { description: self.description.clone(), keywords: self.keywords.clone(), source, + fallback_source, auth_hint, } } @@ -292,4 +296,123 @@ mod tests { assert_eq!(ManifestKind::Tool.to_string(), "tool"); assert_eq!(ManifestKind::Channel.to_string(), "channel"); } + + /// When a manifest has a download URL in artifacts, to_registry_entry() + /// should set WasmDownload as primary source and WasmBuildable as fallback. + #[test] + fn test_manifest_with_download_url_has_buildable_fallback() { + let json = r#"{ + "name": "gmail", + "display_name": "Gmail", + "kind": "tool", + "version": "0.1.0", + "description": "Gmail tool", + "keywords": ["email"], + "source": { + "dir": "tools-src/gmail", + "capabilities": "gmail-tool.capabilities.json", + "crate_name": "gmail-tool" + }, + "artifacts": { + "wasm32-wasip2": { + "url": "https://github.com/nearai/ironclaw/releases/latest/download/gmail-wasm32-wasip2.tar.gz", + "sha256": null + } + }, + "tags": ["default"] + }"#; + + let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest"); + let entry = manifest.to_registry_entry(); + + // Primary source should be WasmDownload + assert!( + matches!(&entry.source, ExtensionSource::WasmDownload { .. }), + "Primary source should be WasmDownload, got {:?}", + entry.source + ); + + // Fallback should be WasmBuildable with the source dir info + let fallback = entry + .fallback_source + .as_ref() + .expect("Should have fallback_source when download URL is set"); + match fallback.as_ref() { + ExtensionSource::WasmBuildable { + build_dir, + crate_name, + .. + } => { + assert_eq!(build_dir.as_deref(), Some("tools-src/gmail")); + assert_eq!(crate_name.as_deref(), Some("gmail-tool")); + } + other => panic!("Fallback should be WasmBuildable, got {:?}", other), + } + } + + /// When a manifest has null URL in artifacts, the primary source should be + /// WasmBuildable with no fallback. + #[test] + fn test_manifest_with_null_url_no_fallback() { + let json = r#"{ + "name": "slack", + "display_name": "Slack", + "kind": "tool", + "version": "0.1.0", + "description": "Slack tool", + "keywords": [], + "source": { + "dir": "tools-src/slack", + "capabilities": "slack-tool.capabilities.json", + "crate_name": "slack-tool" + }, + "artifacts": { + "wasm32-wasip2": { "url": null, "sha256": null } + }, + "tags": [] + }"#; + + let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest"); + let entry = manifest.to_registry_entry(); + + assert!( + matches!(&entry.source, ExtensionSource::WasmBuildable { .. }), + "Should use WasmBuildable when URL is null" + ); + assert!( + entry.fallback_source.is_none(), + "Should have no fallback when already using WasmBuildable" + ); + } + + /// When a manifest has no artifacts section, should use WasmBuildable with no fallback. + #[test] + fn test_manifest_no_artifacts_no_fallback() { + let json = r#"{ + "name": "custom", + "display_name": "Custom", + "kind": "tool", + "version": "0.1.0", + "description": "Custom tool", + "keywords": [], + "source": { + "dir": "tools-src/custom", + "capabilities": "custom.capabilities.json", + "crate_name": "custom-tool" + }, + "tags": [] + }"#; + + let manifest: ExtensionManifest = serde_json::from_str(json).expect("parse manifest"); + let entry = manifest.to_registry_entry(); + + assert!( + matches!(&entry.source, ExtensionSource::WasmBuildable { .. }), + "Should use WasmBuildable when no artifacts" + ); + assert!( + entry.fallback_source.is_none(), + "Should have no fallback when already using WasmBuildable" + ); + } }