Files
optimclaw/tests/tool_schema_validation.rs
06c84a5c77 test: add 26 tests for multi-thread safety, db CRUD, concurrency, errors (#442)
* fix: use std::sync::RwLock in MessageTool to avoid runtime panic

The `requires_approval` method is synchronous but was using
`tokio::sync::RwLock` with `.await` which requires blocking the
runtime. This caused a panic:
"Cannot block the current thread from within a runtime"

Changes:
- Replace `tokio::sync::RwLock` with `std::sync::RwLock` for
  `default_channel` and `default_target` fields
- Use `unwrap_or_else(|e| e.into_inner())` to gracefully handle
  poisoned locks (recovers instead of panicking)
- Update all usages from `.read().await` to `.read().unwrap_or_else()`

The locks are short-held (just cloning strings), making std::sync::RwLock
appropriate for sync methods called from async contexts.

Fixes: "Cannot block the current thread from within a runtime" panic
when the LLM tries to send a message via the message tool.

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

* test: comprehensive testing improvements and fix MessageTool blocking_read panic

Fix tokio::sync::RwLock::blocking_read() panic in MessageTool::requires_approval()
under multi-threaded tokio runtimes by switching to std::sync::RwLock with poison
recovery. Add 26 new tests across 4 tiers:

Tier 1 - Multi-thread runtime safety:
- Fix MessageTool to use std::sync::RwLock instead of tokio::sync::RwLock
- 4 multi-thread tests for MessageTool::requires_approval() scenarios
- 1 multi-thread test for HttpTool credential-dependent approval
- 1 structural test exercising all core tool sync trait methods under multi-thread runtime

Tier 2 - Database CRUD coverage:
- Settings lifecycle (CRUD, bulk ops)
- Tool failure tracking (record, broken list, repair)
- Routine lifecycle (create, get, list, update, delete, runs)
- LLM call recording
- Sandbox job lifecycle (create, get, update, list, mode)
- Job events (save, list, limit)
- Estimation snapshot round-trip

Tier 3 - Concurrency:
- ToolRegistry concurrent register + read under 4-worker runtime

Tier 4 - Error coverage:
- Display tests for all 8 error variants
- From conversion tests for top-level Error enum

Supersedes the fix in PR #411 with the same bug fix plus comprehensive test coverage.

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

* fix: remove trailing whitespace in registry.rs

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

---------

Co-authored-by: Jerome Revillard <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
2026-03-06 04:39:04 +00:00

167 lines
5.5 KiB
Rust

//! Validates that all built-in tool schemas conform to OpenAI strict-mode rules.
//!
//! This catches the class of bugs where `required` keys aren't in `properties`,
//! properties are missing `type` (intentional freeform is allowed), or nested
//! objects/arrays are malformed.
//!
//! See: <https://github.com/nearai/ironclaw/issues/352> (QA plan, item 1.1)
use ironclaw::tools::validate_tool_schema;
use ironclaw::tools::{Tool, ToolRegistry};
/// Validate schemas of all tools registered via `register_builtin_tools()` and
/// `register_dev_tools()` (echo, time, json, http, shell, file tools).
///
/// These tools can be constructed without external dependencies (no DB, no
/// workspace, no extension manager). Tools requiring dependencies (memory, job,
/// skill, extension, routine) are validated individually below where test
/// construction helpers exist.
#[tokio::test]
async fn all_core_builtin_tool_schemas_are_valid() {
let registry = ToolRegistry::new();
registry.register_builtin_tools();
registry.register_dev_tools();
let tools = registry.all().await;
assert!(
!tools.is_empty(),
"registry should have tools after registration"
);
let mut all_errors = Vec::new();
for tool in &tools {
let schema = tool.parameters_schema();
let errors = validate_tool_schema(&schema, tool.name());
if !errors.is_empty() {
all_errors.push(format!(
"Tool '{}' has schema errors:\n {}",
tool.name(),
errors.join("\n ")
));
}
}
assert!(
all_errors.is_empty(),
"Tool schema validation failures:\n{}",
all_errors.join("\n\n")
);
}
/// Verify the exact set of tools registered by the core registration methods.
/// This guards against a new tool being added without schema validation coverage.
#[tokio::test]
async fn core_registration_covers_expected_tools() {
let registry = ToolRegistry::new();
registry.register_builtin_tools();
registry.register_dev_tools();
let mut names = registry.list().await;
names.sort();
let expected = &[
"apply_patch",
"echo",
"http",
"json",
"list_dir",
"read_file",
"shell",
"time",
"write_file",
];
assert_eq!(
names, expected,
"Core tool set changed. Update this test and ensure new tools have valid schemas."
);
}
/// Validate individual tool schemas that are known to use non-trivial patterns.
/// These are regression tests for specific bugs.
#[test]
fn json_tool_freeform_data_field_is_valid() {
// Regression: json tool's "data" field intentionally has no "type" for
// OpenAI compatibility (union types with arrays require "items").
let tool = ironclaw::tools::builtin::JsonTool;
let schema = tool.parameters_schema();
let errors = validate_tool_schema(&schema, "json");
assert!(errors.is_empty(), "json tool schema errors: {errors:?}");
// Verify the freeform pattern is still in place
let data = schema
.get("properties")
.and_then(|p| p.get("data"))
.expect("json tool should have 'data' property");
assert!(
data.get("type").is_none(),
"json.data should be freeform (no type) for OpenAI compatibility"
);
}
#[test]
fn http_tool_headers_array_is_valid() {
// Regression: http tool's "headers" is an array of {name, value} objects.
let tool = ironclaw::tools::builtin::HttpTool::new();
let schema = tool.parameters_schema();
let errors = validate_tool_schema(&schema, "http");
assert!(errors.is_empty(), "http tool schema errors: {errors:?}");
// Verify array structure
let headers = schema
.get("properties")
.and_then(|p| p.get("headers"))
.expect("http tool should have 'headers' property");
assert_eq!(
headers.get("type").and_then(|t| t.as_str()),
Some("array"),
"headers should be an array"
);
assert!(
headers.get("items").is_some(),
"headers array should have items defined"
);
}
#[test]
fn time_tool_schema_is_valid() {
let tool = ironclaw::tools::builtin::TimeTool;
let schema = tool.parameters_schema();
let errors = validate_tool_schema(&schema, "time");
assert!(errors.is_empty(), "time tool schema errors: {errors:?}");
}
#[test]
fn shell_tool_schema_is_valid() {
let tool = ironclaw::tools::builtin::ShellTool::new();
let schema = tool.parameters_schema();
let errors = validate_tool_schema(&schema, "shell");
assert!(errors.is_empty(), "shell tool schema errors: {errors:?}");
}
/// Validates that all core tools work correctly under a multi-threaded tokio runtime.
/// This catches sync-async boundary bugs like tokio::sync::RwLock::blocking_read()
/// panicking when called from within a multi-threaded runtime context.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn all_core_tools_work_in_multi_thread_runtime() {
let registry = ToolRegistry::new();
registry.register_builtin_tools();
registry.register_dev_tools();
let tools = registry.all().await;
assert!(
!tools.is_empty(),
"registry should have tools after registration"
);
for tool in &tools {
// These sync trait methods must not panic in multi-thread runtime
let _ = tool.name();
let _ = tool.description();
let _ = tool.parameters_schema();
let _ = tool.requires_approval(&serde_json::json!({}));
let _ = tool.requires_sanitization();
let _ = tool.domain();
}
}