fix: reliable network tests and improved tool error messages (#626)

* fix(wasm): use per-engine cache dirs on Windows to avoid file lock error (#448)

On Windows, multiple wasmtime Engine instances sharing the default
compilation cache directory hit OS error 33 (ERROR_LOCK_VIOLATION)
because Windows holds exclusive file locks on memory-mapped cache
files. This is especially triggered when the Telegram channel WASM
module is loaded at startup and then hot-activated via the Extensions
UI.

Fix by giving each engine its own cache subdirectory on Windows
(~/.cache/ironclaw/wasmtime-tools/ and wasmtime-channels/). On
Unix the shared default cache continues to work as before.

Also adds Windows CI jobs (cargo check + clippy across all feature
flag combinations) to catch Windows-specific issues going forward.

Closes #448

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: silence Windows clippy warnings for platform-gated code

Gate PathBuf import behind #[cfg(unix)] in container.rs (only used
in Unix socket path), suppress unused_mut on conflicts Vec in
channels.rs (mutations are platform-gated), and add cfg gates on
keychain constants and hex_to_bytes that are only used on macOS/Linux.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: escape directory path in TOML cache config to prevent injection

Use double-quoted TOML strings with backslash and double-quote
escaping for the cache directory path, preventing breakage or
injection when paths contain special characters (e.g. single
quotes on Unix, backslashes on Windows).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve cargo fmt formatting errors

Fix import ordering in container.rs and line wrapping in runtime.rs
to pass the CI formatting check.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(ci): restore Path import for all platforms, keep PathBuf unix-only

Path is used in non-cfg-gated functions (lines 148, 244) so it must
be available on all platforms. Only PathBuf is unix-specific.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use RFC 5737 TEST-NET-1 IPs for reliable network failure tests

Replace localhost/loopback addresses with 192.0.2.1 (TEST-NET-1) in
network failure tests so they work consistently behind HTTP proxies.
Tighten the catalog.rs error assertion to avoid matching any string
containing "error".

Closes #444 (takeover from hobostay)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: include tool name in error messages sent to LLM

Format tool errors as "Tool '<name>' failed: <reason>" instead of the
bare "Error: <reason>" so the LLM can identify which tool failed and
reason about alternatives. Does not short-circuit the agent loop --
errors still flow back to the LLM for reasoning.

Closes #487 (takeover from lustsazeus-lab, PR #530)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: resolve cargo fmt formatting in dispatcher

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Zaki Manian
2026-03-07 05:54:12 +00:00
committed by GitHub
co-authored by Claude Opus 4.6
parent ae89a52ac2
commit 4ac78a5b1f
3 changed files with 39 additions and 6 deletions
+22 -1
View File
@@ -708,7 +708,7 @@ impl Agent {
sanitized.was_modified,
)
}
Err(e) => format!("Error: {}", e),
Err(e) => format!("Tool '{}' failed: {}", tc.name, e),
};
context_messages.push(ChatMessage::tool_result(
@@ -2028,4 +2028,25 @@ mod tests {
let result = super::strip_internal_tool_call_text(input);
assert_eq!(result, input);
}
#[test]
fn test_tool_error_format_includes_tool_name() {
// Regression test for issue #487: tool errors sent to the LLM should
// include the tool name so the model can reason about which tool failed
// and try alternatives.
let tool_name = "http";
let err = crate::error::ToolError::ExecutionFailed {
name: tool_name.to_string(),
reason: "connection refused".to_string(),
};
let formatted = format!("Tool '{}' failed: {}", tool_name, err);
assert!(
formatted.contains("Tool 'http' failed:"),
"Error should identify the tool by name, got: {formatted}"
);
assert!(
formatted.contains("connection refused"),
"Error should include the underlying reason, got: {formatted}"
);
}
}
+11 -3
View File
@@ -457,12 +457,20 @@ mod tests {
#[tokio::test]
async fn test_search_returns_error_on_network_failure() {
// Point at an invalid URL to trigger a network error
let catalog = SkillCatalog::with_url("http://127.0.0.1:1");
// Use RFC 5737 TEST-NET-1 (192.0.2.0/24) for reliable failure even behind proxies.
let catalog = SkillCatalog::with_url("http://192.0.2.1:9999");
let outcome = catalog.search("test").await;
assert!(outcome.results.is_empty());
assert!(outcome.error.is_some());
assert!(outcome.error.unwrap().contains("Registry unreachable"));
let error = outcome.error.unwrap();
assert!(
error.contains("Registry unreachable")
|| error.contains("connect")
|| error.contains("502")
|| error.contains("503")
|| error.contains("504"),
"Expected connection or gateway error, got: {error}",
);
}
#[tokio::test]
+6 -2
View File
@@ -214,12 +214,16 @@ mod tests {
#[tokio::test]
async fn health_with_unreachable_url_is_false() {
// Use RFC 5737 TEST-NET-1 (192.0.2.0/24) for reliable failure even behind proxies.
let tunnel = CustomTunnel::new(
"sleep 1".into(),
Some("http://127.0.0.1:9/healthz".into()),
Some("http://192.0.2.1:9999/healthz".into()),
None,
);
assert!(!tunnel.health_check().await);
assert!(
!tunnel.health_check().await,
"Health check should fail for unreachable URL"
);
}
#[test]