mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
fix: init WASM runtime eagerly regardless of tools directory existence (#401)
* fix: init WASM runtime eagerly regardless of tools directory existence The WASM tool runtime was only created at startup when both `wasm.enabled` and `wasm.tools_dir.exists()` were true. This meant that if the tools directory didn't exist yet (e.g. fresh deploy with `--no-onboard`), the runtime was set to None and passed to the ExtensionManager. Extensions installed later via the web UI would then fail with "WASM runtime not available" because the runtime could not be retroactively created. The Wasmtime engine initialization has no dependency on the tools directory — it only configures the compiler and starts an epoch ticker thread. The directory is only needed later when loading .wasm modules. Remove the directory check so the runtime is available for post-startup extension activation. Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add regression tests for WASM runtime eager init - runtime.rs: test_runtime_creation_without_tools_dir confirms the Wasmtime engine initialises without a tools directory on disk - manager.rs: test_activate_wasm_tool_with_runtime_passes_runtime_check verifies activation gets past the runtime check when a runtime is provided (fails on missing file, not missing runtime) - manager.rs: test_activate_wasm_tool_without_runtime_fails_with_runtime_error verifies the original error when no runtime is available Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: use idiomatic Result-to-Option conversion for WASM runtime init Address PR review feedback: replace match block with .map(Arc::new).map_err(|e| warn!(...)).ok() chain. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: fix formatting in extension manager tests Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
3362081192
commit
7b883a02c0
+11
-13
@@ -402,19 +402,17 @@ impl AppBuilder {
|
||||
|
||||
let mcp_session_manager = Arc::new(McpSessionManager::new());
|
||||
|
||||
// Create WASM tool runtime
|
||||
let wasm_tool_runtime: Option<Arc<WasmToolRuntime>> =
|
||||
if self.config.wasm.enabled && self.config.wasm.tools_dir.exists() {
|
||||
match WasmToolRuntime::new(self.config.wasm.to_runtime_config()) {
|
||||
Ok(runtime) => Some(Arc::new(runtime)),
|
||||
Err(e) => {
|
||||
tracing::warn!("Failed to initialize WASM runtime: {}", e);
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
// Create WASM tool runtime eagerly so extensions installed after startup
|
||||
// (e.g. via the web UI) can still be activated. The tools directory is only
|
||||
// needed when loading modules, not for engine initialisation.
|
||||
let wasm_tool_runtime: Option<Arc<WasmToolRuntime>> = if self.config.wasm.enabled {
|
||||
WasmToolRuntime::new(self.config.wasm.to_runtime_config())
|
||||
.map(Arc::new)
|
||||
.map_err(|e| tracing::warn!("Failed to initialize WASM runtime: {}", e))
|
||||
.ok()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Load WASM tools and MCP servers concurrently
|
||||
let wasm_tools_future = {
|
||||
|
||||
@@ -2602,6 +2602,8 @@ fn combine_install_errors(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::extensions::manager::{
|
||||
FallbackDecision, combine_install_errors, fallback_decision, infer_kind_from_url,
|
||||
};
|
||||
@@ -2780,6 +2782,86 @@ mod tests {
|
||||
assert!(!channel_wasm.exists());
|
||||
}
|
||||
|
||||
// === WASM runtime availability tests ===
|
||||
//
|
||||
// Regression tests for a bug where the WASM runtime was only created at
|
||||
// startup when the tools directory already existed. Extensions installed
|
||||
// after startup (e.g. via the web UI) would fail with "WASM runtime not
|
||||
// available" because the ExtensionManager had `wasm_tool_runtime: None`.
|
||||
|
||||
/// Build a minimal ExtensionManager suitable for unit tests.
|
||||
fn make_test_manager(
|
||||
wasm_runtime: Option<Arc<crate::tools::wasm::WasmToolRuntime>>,
|
||||
tools_dir: std::path::PathBuf,
|
||||
) -> crate::extensions::manager::ExtensionManager {
|
||||
use crate::secrets::{InMemorySecretsStore, SecretsCrypto};
|
||||
use crate::tools::mcp::session::McpSessionManager;
|
||||
|
||||
let key = secrecy::SecretString::from(crate::secrets::keychain::generate_master_key_hex());
|
||||
let crypto = Arc::new(SecretsCrypto::new(key).expect("crypto"));
|
||||
let secrets: Arc<dyn crate::secrets::SecretsStore + Send + Sync> =
|
||||
Arc::new(InMemorySecretsStore::new(crypto));
|
||||
let tools = Arc::new(crate::tools::ToolRegistry::new());
|
||||
let mcp = Arc::new(McpSessionManager::new());
|
||||
|
||||
crate::extensions::manager::ExtensionManager::new(
|
||||
mcp,
|
||||
secrets,
|
||||
tools,
|
||||
None, // hooks
|
||||
wasm_runtime,
|
||||
tools_dir.clone(),
|
||||
tools_dir, // channels dir (unused here)
|
||||
None, // tunnel_url
|
||||
"test".to_string(),
|
||||
None, // db
|
||||
vec![],
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_activate_wasm_tool_with_runtime_passes_runtime_check() {
|
||||
// When the ExtensionManager has a WASM runtime, activation should get
|
||||
// past the "WASM runtime not available" check. It will still fail
|
||||
// because no .wasm file exists on disk — but the error message should
|
||||
// be "not found", NOT "WASM runtime not available".
|
||||
let dir = tempfile::tempdir().expect("temp dir");
|
||||
let config = crate::tools::wasm::WasmRuntimeConfig::for_testing();
|
||||
let runtime = Arc::new(crate::tools::wasm::WasmToolRuntime::new(config).expect("runtime"));
|
||||
let mgr = make_test_manager(Some(runtime), dir.path().to_path_buf());
|
||||
|
||||
let err = mgr.activate("nonexistent").await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
!msg.contains("WASM runtime not available"),
|
||||
"Should not fail on runtime check, got: {msg}"
|
||||
);
|
||||
assert!(
|
||||
msg.contains("not found")
|
||||
|| msg.contains("not installed")
|
||||
|| msg.contains("Not installed"),
|
||||
"Should fail on missing file, got: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_activate_wasm_tool_without_runtime_fails_with_runtime_error() {
|
||||
// When the ExtensionManager has no WASM runtime (None), activation
|
||||
// must fail with the "WASM runtime not available" message.
|
||||
let dir = tempfile::tempdir().expect("temp dir");
|
||||
// Write a fake .wasm file so we don't fail on "not found" first.
|
||||
std::fs::write(dir.path().join("fake.wasm"), b"not-a-real-wasm").unwrap();
|
||||
|
||||
let mgr = make_test_manager(None, dir.path().to_path_buf());
|
||||
|
||||
let err = mgr.activate("fake").await.unwrap_err();
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
msg.contains("WASM runtime not available"),
|
||||
"Expected runtime not available error, got: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_capabilities_files_also_separate() {
|
||||
// capabilities.json files for tools and channels should also be separate.
|
||||
|
||||
@@ -347,4 +347,18 @@ mod tests {
|
||||
assert_eq!(limits.memory_bytes, 5 * 1024 * 1024);
|
||||
assert_eq!(limits.fuel, 500_000);
|
||||
}
|
||||
|
||||
/// The WASM runtime (Wasmtime engine) must initialise successfully even
|
||||
/// when no tools directory exists on disk. The engine only configures the
|
||||
/// compiler and epoch ticker — loading modules from a directory is a
|
||||
/// separate step. Regression test for a bug where the runtime was gated
|
||||
/// on `tools_dir.exists()`, causing extensions installed after startup
|
||||
/// (e.g. via the web UI) to fail with "WASM runtime not available".
|
||||
#[test]
|
||||
fn test_runtime_creation_without_tools_dir() {
|
||||
let config = WasmRuntimeConfig::for_testing();
|
||||
// Runtime should succeed even though no tools directory exists.
|
||||
let runtime = WasmToolRuntime::new(config).expect("runtime should init without tools dir");
|
||||
assert!(runtime.config().fuel_config.enabled);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user