mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-31 08:39:24 +00:00
fix: fall back to build-from-source when extension download fails (#312)
* fix: fall back to build-from-source when extension download fails Extension manifests hardcode GitHub release URLs for WASM artifacts, but these artifacts are not yet published to any release. This causes all WASM extension installs to fail with HTTP 404. Add a fallback_source field to RegistryEntry so that when the primary WasmDownload source fails (e.g., 404), the installer automatically falls back to WasmBuildable (build from source). The manifest conversion now populates this fallback whenever a download URL is set. Fixes nearai/ironclaw#298 Co-Authored-By: Claude Opus 4.6 <[email protected]> * Address Copilot/Gemini review feedback - Skip fallback for AlreadyInstalled errors (Gemini) - Include both primary and fallback errors in combined message (Copilot) - Fix comment to match broader behavior (any error, not just download) (Copilot) Co-Authored-By: Claude Opus 4.6 <[email protected]> * Address serrrfirat review feedback - Forward AlreadyInstalled from fallback directly instead of wrapping in ExtensionError::Other (defensive, prevents misleading error message) Co-Authored-By: Claude Opus 4.6 <[email protected]> * Add unit tests for fallback install logic Extract fallback_decision() and combine_install_errors() from install_from_entry() to enable direct unit testing without requiring a full ExtensionManager setup. Tests cover: - Primary success returns directly (no fallback attempted) - AlreadyInstalled short-circuits (no fallback attempted) - Download failure with fallback available triggers fallback - Error without fallback source returns primary error - Both-fail produces combined error with both messages - AlreadyInstalled from fallback is forwarded directly Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: firat.sertgoz <[email protected]>
This commit is contained in:
Bowen Wang
co-authored by
Claude Opus 4.6
firat.sertgoz
Claude Opus 4.6
firat.sertgoz
parent
ebb4ce95e3
commit
f4ba85ffa2
+139
-16
@@ -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"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user