Files
optimclaw/tests/e2e_wasm_github_coercion.rs
T
8ad7d78a70 fix: parameter coercion and validation for oneOf/anyOf/allOf schemas (#1397)
* fix: parameter coercion and validation for oneOf/anyOf/allOf schemas

WASM extension tools with multi-action schemas (e.g. github extension)
fail when the LLM passes numeric parameters as strings because the
coercion layer skips JSON Schema combinators. This causes serde
deserialization errors like `invalid type: string "100", expected u32`.

Add discriminated-union resolution to the coercion layer: for oneOf/anyOf,
match the active variant by const or single-element enum discriminators;
for allOf, merge all variants' properties. Also propagate combinator
awareness to schema validators, WASM wrapper helpers, and tool discovery
so they no longer reject or ignore valid combinator-based schemas.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* test: add e2e tests for oneOf discriminated union parameter coercion

Add three end-to-end tests using a fixture tool that mirrors the github
WASM tool's oneOf schema with #[serde(tag = "action")] deserialization.
Each test sends string-typed numeric/boolean params through the full
agent loop, verifying that coercion resolves them before serde runs:

- list_issues: limit "100" → 100 (integer in oneOf variant)
- get_issue: issue_number "42" → 42 (integer in different variant)
- create_pull_request: draft "true" → true (boolean in variant)

Without the coercion fix these fail with:
  invalid type: string "100", expected u32

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* test: add real WASM github tool e2e tests with HTTP interception

Load the actual compiled github WASM binary, send params with string-typed
numbers through the coercion layer, and verify the WASM tool constructs
correct HTTP API calls via a new HTTP interceptor in the WASM wrapper.

Changes:
- Add `http_interceptor` field to `StoreData` and `WasmToolWrapper` so
  WASM tool HTTP requests can be captured/mocked in tests
- Make `prepare_tool_params` and `coercion` module public for integration tests
- Add 3 e2e tests loading the real github WASM binary:
  - list_issues: `limit: "50"` → URL contains `per_page=50`
  - get_issue: `issue_number: "42"` → URL contains `/issues/42`
  - list_pull_requests: `limit: "25"` → URL contains `per_page=25`

Tests gracefully skip if the WASM binary isn't compiled.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* refactor: simplify WASM e2e tests to use TestRig with with_wasm_tool()

Replace the manual WasmToolWrapper construction with TestRig integration:

- Add `with_wasm_tool(name, wasm_path, capabilities_path)` to TestRigBuilder
  that loads real WASM binaries and wires the shared HTTP interceptor
- Build the HTTP interceptor before tool registration so it can be shared
  between AgentDeps and WASM tool wrappers
- Rewrite github WASM e2e tests to use the standard trace pattern:
  TraceLlm sends tool calls with string params, http_exchanges specify
  expected outgoing requests and canned responses

The test code is now identical to other trace-based e2e tests — no custom
interceptors or manual WASM construction needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: address review comments on combinator schema support

- Validate `has_combinators` checks array type (`.as_array().is_some()`)
  instead of bare `.is_some()` to reject malformed `{ "oneOf": {} }`
- Validate top-level `required` keys against merged combinator variant
  properties when no top-level `properties` exists (both validators)
- Deduplicate oneOf/anyOf handling into single loop in coercion.rs
- Revert `pub mod coercion` to private; only re-export `prepare_tool_params`
- Call `after_response` on interceptor after real HTTP when `before_request`
  returns None (recording mode correctness)
- Fix formatting (CI failure)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: address second round of review comments

- Fix headers deserialization bug: deserialize resp.headers_json as
  HashMap<String, String> then convert to Vec, not directly as Vec
- Sort interceptor headers for deterministic trace fixtures
- Update after_response comment: RecordingHttpInterceptor does exercise
  this path (returns None from before_request)
- Mark WASM tests #[ignore] instead of silent skip — avoids false-green
  CI while keeping them runnable with --ignored
- Fix with_wasm_tool signature: Option<PathBuf> instead of
  Option<impl Into<PathBuf>> which doesn't compile in nested position
- Fix with_wasm_tool doc comment to match actual behavior
- Revert prepare_tool_params to pub(crate) — no longer needed publicly

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: coerce empty strings to null for optional tool parameters

LLMs often send "" instead of null/omitting optional parameters, causing
parse errors in tools that expect typed values (e.g., timezone, schedule).

PR #1127 fixed this per-field in the time tool. This commit adds
dispatcher-level coercion so all tools benefit:

- Non-required properties with value "" are coerced to null at the
  object level (based on the schema's `required` array)
- Explicitly nullable schemas (`type: ["string", "null"]`) coerce ""
  to null in the per-value coercion path
- Required string-only fields keep "" unchanged

Closes #755

Co-Authored-By: spiritj <[email protected]>
Co-Authored-By: Xing Ji <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* feat: complete coercion coverage for $ref, nested combinators, and additionalProperties

Close remaining coercion gaps so 3rd-party tools (MCP servers, complex
WASM tools) work correctly:

- $ref resolution: inline all #/definitions/<name> and #/$defs/<name>
  references in a pre-pass before coercion, with depth limit (16) for
  circular ref safety
- Nested combinators: resolve_effective_properties now recurses into
  variants that themselves contain allOf/oneOf/anyOf (depth limit 4)
- additionalProperties inheritance: check allOf variants and matched
  oneOf/anyOf variant for additionalProperties schemas

New tests:
- resolves_ref_and_coerces_referenced_properties
- resolves_nested_refs_in_oneof_variants
- coerces_nested_combinators_allof_containing_oneof
- coerces_array_items_with_oneof_discriminator
- circular_ref_does_not_infinite_loop

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: address third round of review comments

- Validators: tighten has_combinators to require at least one object-typed
  variant (has type:"object" or properties), rejecting non-object combinator
  schemas like { "oneOf": [{"type":"integer"}] }
- Empty-string coercion: only coerce "" → null when schema allows null or
  doesn't allow string; pure type:"string" fields keep "" as meaningful
- Fix comment: "coerce to null" → "return unchanged" for empty strings
  with no type match (code returns None, not null)
- Redact credentials before passing to after_response interceptor to
  prevent secret leakage into recorded trace files
- Switch to tokio::fs::read for async WASM binary loading in test rig
- Add doc comment explaining soft URL check in WASM e2e tests

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* ci: retrigger after staging merge [skip-regression-check]

* fix: merge staging, report non-array combinator values as errors

Merge latest staging to fix CI (missing fallback_deliverable field).
Add explicit error reporting when oneOf/anyOf/allOf values are not
arrays in both strict and lenient validators.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

* fix: recurse into combinator variants that have properties but no explicit type

Both validators only recursed into variants with `type: "object"`,
missing variants that define `properties` without an explicit type
(common in allOf patterns). Now recurse when variant has either.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Co-authored-by: spiritj <[email protected]>
Co-authored-by: Xing Ji <[email protected]>
2026-03-21 12:41:46 -07:00

278 lines
11 KiB
Rust

//! E2E test: real github WASM tool with parameter coercion via TestRig.
//!
//! Loads the compiled github WASM binary into the test rig, replays an LLM
//! trace that sends string-typed numeric params, and verifies the WASM tool
//! constructs the correct HTTP API call via `http_exchanges` in the trace.
//!
//! These tests are `#[ignore]` by default because they require a pre-compiled
//! WASM binary. Build it with:
//! cargo build -p github-tool --target wasm32-wasip2 --release
//! Then run with:
//! cargo test --features libsql --test e2e_wasm_github_coercion -- --ignored
#[cfg(feature = "libsql")]
mod support;
/// Note on URL verification: the `ReplayingHttpInterceptor` logs warnings on
/// URL mismatch but still returns the canned response. The real verification is
/// that the tool succeeds end-to-end: coercion produced the correct typed
/// parameters, serde deserialization succeeded, and the WASM tool constructed a
/// valid HTTP request. A URL mismatch warning in logs does not indicate test
/// failure — it is a soft check only.
#[cfg(feature = "libsql")]
mod tests {
use std::time::Duration;
use serde_json::json;
use ironclaw::llm::recording::{HttpExchange, HttpExchangeRequest, HttpExchangeResponse};
use crate::support::test_rig::TestRigBuilder;
use crate::support::trace_llm::{
LlmTrace, TraceExpects, TraceResponse, TraceStep, TraceToolCall,
};
const GITHUB_WASM: &str = "tools-src/github/target/wasm32-wasip2/release/github_tool.wasm";
const GITHUB_CAPS: &str = "tools-src/github/github-tool.capabilities.json";
fn github_ok(body: &str) -> HttpExchangeResponse {
HttpExchangeResponse {
status: 200,
headers: vec![
("content-type".to_string(), "application/json".to_string()),
("x-ratelimit-remaining".to_string(), "100".to_string()),
],
body: body.to_string(),
}
}
/// LLM sends `limit: "50"` (string) to `list_issues`. Coercion converts it
/// to integer, and the WASM tool must call `GET /repos/.../issues?...&per_page=50`.
#[tokio::test]
#[ignore] // requires pre-compiled WASM binary
async fn wasm_github_list_issues_coerces_string_limit() {
let expected_url =
"https://api.github.com/repos/nearai/ironclaw/issues?state=open&per_page=50";
let trace = LlmTrace {
model_name: "test-wasm-coercion-list-issues".to_string(),
turns: vec![crate::support::trace_llm::TraceTurn {
user_input: "List issues in nearai/ironclaw with limit 50".to_string(),
steps: vec![
TraceStep {
request_hint: None,
response: TraceResponse::ToolCalls {
tool_calls: vec![TraceToolCall {
id: "call_gh_1".to_string(),
name: "github".to_string(),
arguments: json!({
"action": "list_issues",
"owner": "nearai",
"repo": "ironclaw",
"state": "open",
"limit": "50"
}),
}],
input_tokens: 100,
output_tokens: 30,
},
expected_tool_results: Vec::new(),
},
TraceStep {
request_hint: None,
response: TraceResponse::Text {
content: "Found 1 issue.".to_string(),
input_tokens: 150,
output_tokens: 10,
},
expected_tool_results: Vec::new(),
},
],
expects: TraceExpects::default(),
}],
memory_snapshot: Vec::new(),
http_exchanges: vec![HttpExchange {
request: HttpExchangeRequest {
method: "GET".to_string(),
url: expected_url.to_string(),
headers: vec![],
body: None,
},
response: github_ok(r#"[{"number":1,"title":"Test issue","state":"open"}]"#),
}],
expects: TraceExpects {
tools_used: vec!["github".to_string()],
all_tools_succeeded: Some(true),
max_tool_calls: Some(1),
min_responses: Some(1),
..Default::default()
},
steps: Vec::new(),
};
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_wasm_tool("github", GITHUB_WASM, Some(GITHUB_CAPS.into()))
.build()
.await;
rig.send_message("List issues in nearai/ironclaw with limit 50")
.await;
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
rig.verify_trace_expects(&trace, &responses);
rig.shutdown();
}
/// LLM sends `issue_number: "42"` (string) to `get_issue`. Coercion converts
/// it to integer, and the URL must contain `/issues/42`.
#[tokio::test]
#[ignore] // requires pre-compiled WASM binary
async fn wasm_github_get_issue_coerces_string_issue_number() {
let expected_url = "https://api.github.com/repos/nearai/ironclaw/issues/42";
let trace = LlmTrace {
model_name: "test-wasm-coercion-get-issue".to_string(),
turns: vec![crate::support::trace_llm::TraceTurn {
user_input: "Get issue 42 from nearai/ironclaw".to_string(),
steps: vec![
TraceStep {
request_hint: None,
response: TraceResponse::ToolCalls {
tool_calls: vec![TraceToolCall {
id: "call_gh_2".to_string(),
name: "github".to_string(),
arguments: json!({
"action": "get_issue",
"owner": "nearai",
"repo": "ironclaw",
"issue_number": "42"
}),
}],
input_tokens: 80,
output_tokens: 20,
},
expected_tool_results: Vec::new(),
},
TraceStep {
request_hint: None,
response: TraceResponse::Text {
content: "Issue 42 retrieved.".to_string(),
input_tokens: 100,
output_tokens: 10,
},
expected_tool_results: Vec::new(),
},
],
expects: TraceExpects::default(),
}],
memory_snapshot: Vec::new(),
http_exchanges: vec![HttpExchange {
request: HttpExchangeRequest {
method: "GET".to_string(),
url: expected_url.to_string(),
headers: vec![],
body: None,
},
response: github_ok(r#"{"number":42,"title":"Test","state":"open","body":"desc"}"#),
}],
expects: TraceExpects {
tools_used: vec!["github".to_string()],
all_tools_succeeded: Some(true),
max_tool_calls: Some(1),
min_responses: Some(1),
..Default::default()
},
steps: Vec::new(),
};
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_wasm_tool("github", GITHUB_WASM, Some(GITHUB_CAPS.into()))
.build()
.await;
rig.send_message("Get issue 42 from nearai/ironclaw").await;
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
rig.verify_trace_expects(&trace, &responses);
rig.shutdown();
}
/// LLM sends `limit: "25"` (string) to `list_pull_requests`. URL must
/// contain `per_page=25`.
#[tokio::test]
#[ignore] // requires pre-compiled WASM binary
async fn wasm_github_list_prs_coerces_string_limit() {
let expected_url =
"https://api.github.com/repos/nearai/ironclaw/pulls?state=open&per_page=25";
let trace = LlmTrace {
model_name: "test-wasm-coercion-list-prs".to_string(),
turns: vec![crate::support::trace_llm::TraceTurn {
user_input: "List PRs in nearai/ironclaw".to_string(),
steps: vec![
TraceStep {
request_hint: None,
response: TraceResponse::ToolCalls {
tool_calls: vec![TraceToolCall {
id: "call_gh_3".to_string(),
name: "github".to_string(),
arguments: json!({
"action": "list_pull_requests",
"owner": "nearai",
"repo": "ironclaw",
"limit": "25"
}),
}],
input_tokens: 80,
output_tokens: 20,
},
expected_tool_results: Vec::new(),
},
TraceStep {
request_hint: None,
response: TraceResponse::Text {
content: "Found PRs.".to_string(),
input_tokens: 100,
output_tokens: 10,
},
expected_tool_results: Vec::new(),
},
],
expects: TraceExpects::default(),
}],
memory_snapshot: Vec::new(),
http_exchanges: vec![HttpExchange {
request: HttpExchangeRequest {
method: "GET".to_string(),
url: expected_url.to_string(),
headers: vec![],
body: None,
},
response: github_ok(r#"[{"number":1,"title":"Test PR","state":"open"}]"#),
}],
expects: TraceExpects {
tools_used: vec!["github".to_string()],
all_tools_succeeded: Some(true),
max_tool_calls: Some(1),
min_responses: Some(1),
..Default::default()
},
steps: Vec::new(),
};
let rig = TestRigBuilder::new()
.with_trace(trace.clone())
.with_wasm_tool("github", GITHUB_WASM, Some(GITHUB_CAPS.into()))
.build()
.await;
rig.send_message("List PRs in nearai/ironclaw").await;
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
rig.verify_trace_expects(&trace, &responses);
rig.shutdown();
}
}