mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
81aec813e1
* test(e2e): add v2 engine tool execution lifecycle tests The v2 engine had zero e2e coverage for the tool call -> result -> response path. This gap was flagged in the #2193 audit and is the same code path that breaks in QA bug #2402 (infinite loop after tool operations). New test file: test_v2_engine_tool_lifecycle.py - Single tool call (echo, time) completes through v2 - Text-only message completes through v2 - Parallel tool calls (2 tools in one response) - Multi-step chain (echo -> result -> time -> result -> completion) - Multi-turn tool usage across conversation turns Mock LLM additions: - "parallel echo and time" trigger for multi-call responses - "multi step echo then time" trigger for sequential chains Also documents that v2 engine does not populate the tool_calls array in chat history (tool names show as "unknown"). This is a separate gap from execution correctness. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gateway): persist v2 engine tool_calls to chat history The v2 engine executed tools correctly but never wrote a `role="tool_calls"` message to the v1 conversation DB. This meant the chat history API returned `tool_calls: []` for all v2 threads, breaking the web UI's tool call display. Fix: after thread completion, extract ActionExecuted/ActionFailed events from the v2 event log and write them as a tool_calls DB row before the assistant response. The v1 history API now shows tool names, results, and errors for v2 engine threads. Steps are evicted from the in-memory store after join_thread, so this reads from the append-only event log instead. E2E test updated to assert tool_calls are populated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use thread internal_messages for tool_calls persistence The events approach used params_summary (input parameters) where result_preview (output) was expected. Thread internal_messages carry the actual tool output in ActionResult messages. Also fixes stale test file docstring that said tool_calls were not populated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: log conversation ID resolution failures instead of swallowing The v1 write_v1_response silently drops errors via .ok(). Don't replicate that -- log a warning so failed tool_calls persistence is diagnosable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback on v2 tool_calls persistence - Drop redundant .chain(thread.messages.iter()) — ActionResult messages only exist in internal_messages - Change tracing::warn! to debug! for fire-and-forget persistence failures (warn corrupts TUI per CLAUDE.md) - Add tool_calls assertions to parallel, multi-step, and multi-turn tests — all 6 tests now verify the core persistence feature - Add result_preview content assertion to echo test for tighter coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: cargo fmt + add V24 migration checksum Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: move persist_v2_tool_calls to Completed arm + add unit tests Move persist_v2_tool_calls into the ThreadOutcome::Completed match arm so it only fires for final outcomes. Previously it ran for all outcomes including GatePaused, which caused duplicate/orphaned tool_calls rows when a gate resumed. Also fixes the Completed { response: None } gap where tool_calls were never persisted for threads that completed with tool output but no final text. Add two libsql-backed unit tests for persist_v2_tool_calls verifying correct extraction from internal_messages and skip behavior for text-only threads. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: cargo fmt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): address PR #2452 review follow-ups Three polish items from the PR #2452 review (https://github.com/nearai/ironclaw/pull/2452#pullrequestreview-4135957005), flagged under the Engine v2 review-follow-up tracker issue #2669. 1. **Restore `warn!` for `persist_v2_tool_calls` failures** — commit `ff372e11` changed them to `debug!` citing CLAUDE.md's "background tasks must not use info/warn" rule. That rule is about REPL/TUI corruption; `router.rs` is an HTTP handler path, not a background task. Silent `debug!` hid a user-visible bug (chat history missing `tool_calls` array) unless someone set `RUST_LOG=debug`. All four failure sites (load thread, serialize, resolve conv id, DB write) now emit at `warn!` and include the `thread_id` field for correlation. 2. **Regression test: `persist_v2_tool_calls` must only be called from the `Completed` arm** — commit `652315e8` fixed the original bug where the call was shared across all `ThreadOutcome` variants, causing partial tool executions on `GatePaused` to orphan DB rows that duplicated on resume. The existing unit tests call the function directly, so they cover the write path but not the gating. A future refactor could silently move the call back out of the `Completed` arm and nothing would fail. The new `persist_v2_tool_calls_only_called_from_completed_arm` test parses the source of `router.rs`, asserts exactly one call site, and asserts that site sits between the `Completed` and `GatePaused` match arms. 3. **Multi-byte UTF-8 truncation test** — the 500-byte preview truncation uses `char_indices()` + `len_utf8()` to avoid slicing mid-char. Behavior was correct but unexercised. New test constructs an ActionResult with 400 × 3-byte CJK chars (1200 bytes) and pins (a) no panic, (b) valid UTF-8 (via JSON round-trip), (c) body length < 500+max_char_width, (d) body contains only complete 3-byte chars. Verified: `cargo fmt`, `cargo clippy --no-default-features --features libsql --tests -- -D warnings` (0 warnings), `cargo test -p ironclaw --lib --features libsql` (5125 passed, +3 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>