Files
ironclaw/tests/tool_schema_validation.rs
Illia Polosukhin 2cc5546017 feat(tools): production-grade coding tools, file history, and skills (#2025)
* feat(tools): add production-grade coding tools, file history, and coding skills

Add dedicated coding tools inspired by Claude Code's architecture to make
IronClaw a more effective coding assistant:

New tools:
- GlobTool: fast file pattern matching via `glob` crate, sorted by mtime,
  with default exclusions (.git, node_modules, target, etc.)
- GrepTool: content search wrapping ripgrep with 3 output modes
  (content, files_with_matches, count), pagination, and context lines
- FileUndoTool: restore files to pre-modification state using in-memory
  file history snapshots

Enhanced tools:
- ReadFileTool: 10MB limit, 2000-line default, binary detection, device
  path blocking (/dev/zero, /proc/*/fd/*)
- ApplyPatchTool: uniqueness validation (error on ambiguous matches),
  workspace path rejection, 10MB size limit, file history integration
- WriteFileTool: file history integration for undo support

Updated tool descriptions to guide LLM behavior (prefer apply_patch over
write_file, always read before editing, use glob/grep instead of shell).

New skills:
- coding: best practices for code editing, search, and file operations
- commit: git commit message generation workflow
- review: code review workflow with structured checklist

Shared infrastructure:
- DEFAULT_EXCLUDED_DIRS constant in path_utils.rs
- FileHistory module with SharedFileHistory for cross-tool snapshots

66 new tests covering all tools, edge cases, and regression scenarios.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: apply cargo fmt formatting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address PR review — security, correctness, and robustness fixes

- Move device path blocking after validate_path() to prevent traversal bypass
- Add /proc/kcore, /proc/kmem to blocked paths
- Reject absolute patterns and '..' in glob tool, add strip_prefix defense
- Wrap glob sync I/O in spawn_blocking to avoid blocking tokio executor
- Sort files_with_matches globally before pagination in grep tool
- Add default exclusions for node_modules/target in grep tool
- Inject ctx.extra_env into rg environment matching ShellTool policy
- Use per-line strip_prefix for content mode path relativization
- Change FileSnapshot.content_before to Vec<u8> for binary file support
- Log snapshot errors with tracing::debug instead of silently discarding
- Fix skill name mismatch: code-review → review to match directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(skills): rename review skill directory to code-review

Aligns the directory name with the manifest name (code-review) to prevent
incorrect override/dedup behavior in the bundled-skill loader. The name
stays "code-review" since other domains may also need review-type skills.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat(tools): add file edit guards — staleness detection, fuzzy matching, encoding preservation

Add file_edit_guard module with production-grade safeguards for file editing:
- ReadFileState tracks file reads with mtime for staleness detection
- 4-level fuzzy matching fallback (exact → whitespace-normalized → quote-normalized → both)
- UTF-16LE BOM detection and line ending style preservation (LF/CRLF/CR)
- Read-before-edit enforcement for ApplyPatch and WriteFile tools
- No-op edit rejection (old_string == new_string)
- Shared state injection via Arc<RwLock<>> across ReadFile, WriteFile, ApplyPatch

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address all PR review comments — session scoping, parallelism, security

- Session-scoped state: ReadFileState and FileHistory now keyed by job_id
  so concurrent sessions sharing the same registry don't leak state (#2025)
- Parallel metadata: grep files_with_matches uses JoinSet (max 64 concurrency)
  instead of sequential await per file for mtime sorting
- Shared env allowlist: grep_tool imports SAFE_ENV_VARS from shell.rs
  (made pub(crate)) instead of maintaining a divergent copy
- Glob traversal: uses Component::ParentDir check instead of substring ".."
  match, so patterns like "foo..bar" are no longer falsely rejected
- UTF-16LE in read_file: binary detection skips null-byte check for files
  with UTF-16LE BOM; read_file uses encoding-aware read path
- Partial flag: default 2000-line truncation now marks read as partial,
  preventing edits against unseen content
- write_file guard softened: staleness check logs warning instead of
  hard error (full-file replacement has lower risk than apply_patch)
- Updated e2e trace to include read_file before apply_patch
- Updated expected tool list in schema validation tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): use async metadata instead of blocking path.exists() in write_file

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(ci): fix false-positive panic detection for lifetimes in char lexer

The check_no_panics.py lexer misinterpreted Rust lifetimes ('static) as
char literal starts, causing in_char state to persist across lines and
hide all subsequent brace-delimited blocks — including #[cfg(test)] mod
tests. Reset in_char at line boundaries since Rust char literals cannot
span lines.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* test: verify MCP push works

* test

* chore: remove test file

* style: apply cargo fmt to file.rs

Collapse multi-line method chain to single line per rustfmt.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* style: apply cargo fmt to file.rs

Collapse multi-line method chain to single line per rustfmt.

https://claude.ai/code/session_012bJjER6L5zSAFd9BqYMUmC

* fix(file-tools): harden fuzzy patch matching and undo

* fix(ci): formatting + wasmtime 43 cache config compatibility

After merging latest staging, cargo fmt had diffs in file tools and the
wasmtime cache TOML format changed (v43 dropped the `enabled` field
under `[cache]`). Also removes accidental .fmt-test artifact.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(file-tools): simplify strip_trailing_whitespace

Remove redundant double-pass through .lines() — the first
collect+join was a no-op since .lines() already handles line endings.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(tools): address PR review comments — security, correctness, tests

- Add is_sensitive_path checks to GlobTool and GrepTool, matching the
  defense-in-depth posture of ReadFileTool/WriteFileTool/ListDirTool
- Fix UTF-8 panicking byte-index slice in apply_patch error preview
  (old_string[..200] → chars().take(200))
- Add 10MB size guard on file_history snapshots to prevent memory
  exhaustion from snapshotting large files
- Replace dead turn_number field with auto-incrementing sequence_number
  in FileHistory — callers no longer pass a hardcoded 0
- Fix glob mtime test flakiness by increasing sleep to 1100ms (above
  1s filesystem granularity)
- Fix emoji test to actually include emoji/non-ASCII content

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Zaki Manian <zaki@iqlusion.io>
2026-04-11 01:37:17 +09:00

171 lines
5.6 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",
"file_undo",
"glob",
"grep",
"http",
"json",
"list_dir",
"plan_update",
"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();
}
}