diff --git a/.config/nextest.toml b/.config/nextest.toml new file mode 100644 index 0000000000..f5c47173eb --- /dev/null +++ b/.config/nextest.toml @@ -0,0 +1,36 @@ +# Cross-binary parallel test runner config for cargo-nextest. +# See https://nexte.st/docs/configuration/ for the full schema. +# +# Profiles: +# - default — `cargo nextest run` (local dev) +# - ci — `cargo nextest run --profile ci` (GitHub Actions) + +[profile.default] +# Per-test slow warning: nextest prints "SLOW" for every `period`; after +# `terminate-after` warnings it kills the process. 60s/3 catches obvious +# hangs without surprising tests that happen to run ~45-55s on a loaded +# laptop. Raise per-test via the `[[overrides]]` block below. +slow-timeout = { period = "60s", terminate-after = 3 } +# Leak detection fires if a test spawns threads/processes that outlive it; +# the default 100ms grace is fine. +leak-timeout = "100ms" + +[profile.ci] +# One flake shouldn't mask other real failures. +fail-fast = false +# Inline failures, suppress passing noise, keep Actions logs scannable. +failure-output = "immediate-final" +success-output = "never" +# Stricter CI timeout: a hung test should crash the job fast, not sit +# against the workflow-level 25-minute cap. +slow-timeout = { period = "60s", terminate-after = 3 } + +# Per-test windows for the handful of scenarios that genuinely need more +# than 60s (live-recorded fixtures + heavy integration). +[[profile.default.overrides]] +filter = "test(=live_tests::zizmor_scan) | test(=live_tests::zizmor_scan_v2) | test(=engine_v2_tests::snapshot_zizmor_scan_v2) | test(~e2e_thread_scheduling) | test(~heavy_integration)" +slow-timeout = { period = "300s", terminate-after = 2 } + +[[profile.ci.overrides]] +filter = "test(=live_tests::zizmor_scan) | test(=live_tests::zizmor_scan_v2) | test(=engine_v2_tests::snapshot_zizmor_scan_v2) | test(~e2e_thread_scheduling) | test(~heavy_integration)" +slow-timeout = { period = "300s", terminate-after = 2 } diff --git a/.github/workflows/replay-gate.yml b/.github/workflows/replay-gate.yml new file mode 100644 index 0000000000..2ca7606db6 --- /dev/null +++ b/.github/workflows/replay-gate.yml @@ -0,0 +1,102 @@ +name: Replay Snapshot Gate + +# Runs `cargo insta test --check` over the committed replay fixtures so any +# change to engine dispatch, agent loop, or tool execution has to come with +# an accepted snapshot diff. Scoped to paths that can actually move the +# replay output, so unrelated PRs don't pay the build cost. +# +# Cache strategy: GitHub Actions enforces a 10 GB per-repo cache quota, and +# the repo routinely sits close to that ceiling, so PR-scoped caches get +# LRU-evicted between runs. The `push: [staging, main]` trigger primes a +# warm cache on the base branches every time work lands there — PRs +# targeting those branches then restore from that cache (GitHub allows +# cross-ref restore from the PR's base) instead of paying the full +# cold-cache compile on every push. + +on: + pull_request: + paths: + - 'crates/ironclaw_engine/**' + - 'src/agent/**' + - 'src/llm/**' + - 'src/tools/builtin/**' + - 'src/bridge/**' + - 'tests/fixtures/llm_traces/**' + - 'tests/snapshots/**' + - 'tests/support/replay_outcome.rs' + - 'tests/e2e_engine_v2.rs' + - 'tests/e2e_live.rs' + - 'tests/e2e_recorded_trace.rs' + - 'Cargo.toml' + - '.github/workflows/replay-gate.yml' + push: + branches: + # PRs default to staging in this repo, so priming staging is what + # actually benefits most PRs. main is kept so the cache also warms + # whenever staging is promoted forward. + - staging + - main + +jobs: + replay-snapshots: + name: Replay snapshot gate + runs-on: ubuntu-latest + timeout-minutes: 25 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + # Keep `save-if` restricted to base-branch pushes so PR runs only read + # the cache and never write — this prevents a single big PR cache + # from filling the 10 GB quota and evicting the primed base-branch + # cache that every other PR depends on. + - uses: Swatinem/rust-cache@v2 + with: + key: replay-gate + save-if: ${{ github.event_name == 'push' }} + + # Precompiled binaries — avoids the ~40s cargo-insta source build and + # wires nextest in the same step since insta's `--test-runner nextest` + # needs it on the PATH. + - name: Install cargo-insta and cargo-nextest + uses: taiki-e/install-action@v2 + with: + tool: cargo-insta,cargo-nextest + + # `cargo insta test` runs the tests and, in `--check` mode, fails if any + # new or pending `.snap.new` would be produced. Combined with the + # `replay` feature flag, this promotes the `#[ignore]`d live tests into + # a required status check while keeping them off the default `cargo test`. + # + # `--test-runner nextest` runs each test binary in parallel processes + # and enforces the per-test timeouts defined in `.config/nextest.toml`. + # `NEXTEST_PROFILE=ci` selects the stricter profile there (no + # fail-fast, inline failure output, suppressed success noise). + - name: Run replay snapshot tests + env: + NEXTEST_PROFILE: ci + run: | + cargo insta test \ + --check \ + --test-runner nextest \ + --no-default-features \ + --features "libsql,replay" \ + --test e2e_engine_v2 \ + --test e2e_recorded_trace \ + --test e2e_live + + # Reports EventKind variants that have no snapshot exercising them. + # Runs in non-strict mode so it stays advisory while coverage ramps up — + # flip to `--strict` once every variant is covered at least once. + - name: Report trace coverage + run: ./scripts/trace-coverage.sh + + - name: Reject committed .snap.new files + run: | + if git ls-files 'tests/snapshots/*.snap.new' | grep .; then + echo "Committed .snap.new files found — run 'cargo insta review' and commit the accepted .snap." + exit 1 + fi diff --git a/Cargo.lock b/Cargo.lock index 5845c2fdb3..aac3921eee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3809,6 +3809,7 @@ checksum = "99322078b2c076829a1db959d49da554fabc4342257fc0ba5a070a1eb3a01cd8" dependencies = [ "console", "once_cell", + "serde", "similar", "tempfile", ] diff --git a/Cargo.toml b/Cargo.toml index 2bb3b4e2ca..3003d785db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -226,7 +226,7 @@ tracing-test = "0.2" testcontainers-modules = { version = "0.11", features = ["postgres"] } pretty_assertions = "1" tempfile = "3" -insta = "1.46.3" +insta = { version = "1.46.3", features = ["yaml"] } [features] default = ["postgres", "libsql", "html-to-markdown", "tui"] @@ -246,6 +246,10 @@ libsql = ["dep:libsql"] # Opt-in feature for especially heavy integration-test targets that run in a # dedicated CI job instead of the default Rust test matrix. integration = [] +# Replay-gate snapshot tests promoted out of `--ignored` into a dedicated CI +# job. Enabled by `.github/workflows/replay-gate.yml`; implies `libsql` so the +# test rig can spin up its embedded database. +replay = ["libsql"] html-to-markdown = ["dep:html-to-markdown-rs", "dep:readabilityrs"] bedrock = ["dep:aws-config", "dep:aws-sdk-bedrockruntime", "dep:aws-smithy-types"] tui = ["dep:ironclaw_tui"] diff --git a/scripts/replay-snap.sh b/scripts/replay-snap.sh new file mode 100755 index 0000000000..8512b9f50f --- /dev/null +++ b/scripts/replay-snap.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# Thin wrapper around cargo-insta for the replay snapshot gate. +# +# Subcommands: +# review — interactive review + accept/reject of pending snapshots +# accept — accept all pending snapshots without prompting +# test — run the replay test set and fail on pending snapshots +# record — record a fresh fixture from a live agent session +# +# Usage: +# scripts/replay-snap.sh review +# scripts/replay-snap.sh accept +# scripts/replay-snap.sh test +# scripts/replay-snap.sh record [] + +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$ROOT" + +FIXTURE_DIR="tests/fixtures/llm_traces" + +ensure_insta() { + if ! command -v cargo-insta >/dev/null 2>&1; then + cat >&2 <<'EOF' +error: cargo-insta is required but not installed. + +Install with one of: + cargo binstall cargo-insta # precompiled binary, fast + cargo install cargo-insta --locked # source build, slow + +CI uses taiki-e/install-action@v2 for a precompiled binary. +EOF + exit 1 + fi +} + +case "${1:-}" in + review) + ensure_insta + cargo insta review + ;; + accept) + ensure_insta + cargo insta accept --all + ;; + test) + ensure_insta + cargo insta test \ + --check \ + --no-default-features \ + --features "libsql,replay" \ + --test e2e_engine_v2 \ + --test e2e_recorded_trace \ + --test e2e_live + ;; + record) + name="${2:-}" + if [ -z "$name" ]; then + echo "usage: $0 record []" >&2 + exit 2 + fi + model="${3:-recording-$name}" + out="$FIXTURE_DIR/$name.json" + mkdir -p "$(dirname "$out")" + echo "Recording $out (model_name: $model)" + echo "Interact with the agent, then quit to flush the trace." + IRONCLAW_RECORD_TRACE=1 \ + IRONCLAW_TRACE_OUTPUT="$out" \ + IRONCLAW_TRACE_MODEL_NAME="$model" \ + cargo run + ;; + *) + echo "usage: $0 {review|accept|test|record}" + exit 2 + ;; +esac diff --git a/scripts/trace-coverage.sh b/scripts/trace-coverage.sh new file mode 100755 index 0000000000..a6ddcd33e2 --- /dev/null +++ b/scripts/trace-coverage.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Report which EventKind variants have snapshot coverage. +# +# A replay snapshot proves the engine reached a given EventKind variant in at +# least one scenario. This script is a diagnostic: it enumerates EventKind +# variants and checks whether each appears in any `tests/snapshots/*.snap`. +# +# Runs as a **soft gate** by default (always exit 0); the replay-gate +# workflow runs it this way so uncovered variants don't block the merge +# while coverage ramps up. Pass `--strict` to promote uncovered variants +# to a hard failure — flip the workflow to `--strict` once every variant +# has at least one snapshot exercising it. +# +# Parsing note: variant names are extracted from `src/types/event.rs` with +# awk, which is deliberately loose and biased toward false negatives. A +# missed variant simply isn't coverage-gated (benign); a false positive on +# an attribute-decorated or cfg-gated line is not — if the parser ever +# misidentifies a real variant, rewrite this in Rust with `syn`. +# +# Intentionally skipped: `ThreadState` and `EffectType` are also engine +# enums, but their variants flow through EventKind (`StateChanged`, +# `LeaseGranted`) rather than appearing as top-level keys, so snapshotting +# them independently would just duplicate coverage. + +set -euo pipefail + +STRICT=false +for arg in "$@"; do + case "$arg" in + --strict) STRICT=true ;; + *) echo "unknown flag: $arg" >&2; exit 2 ;; + esac +done + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +SNAP_DIR="$ROOT/tests/snapshots" +EVENT_RS="$ROOT/crates/ironclaw_engine/src/types/event.rs" + +if [ ! -d "$SNAP_DIR" ]; then + echo "No tests/snapshots directory yet — skipping coverage gate." + exit 0 +fi + +# Extract the EventKind enum body and pull top-level identifiers. +# Matches `EventKind {` opening and stops at the matching closing brace. Only +# picks up variant names — lines starting with a capital letter followed by +# `{`, `,`, or a newline. +variants=$(awk ' + /^pub enum EventKind/ { start=1; depth=0; next } + start { + depth += gsub(/\{/, "{") + depth -= gsub(/\}/, "}") + if (depth<0) { exit } + # Variant lines: ` VariantName {` or ` VariantName,` + if (match($0, /^[[:space:]]+([A-Z][A-Za-z0-9_]+)[[:space:]]*(\{|,|$)/, m)) { + print m[1] + } + } +' "$EVENT_RS" | sort -u) + +covered=() +missing=() +while IFS= read -r v; do + [ -z "$v" ] && continue + # Match the YAML-serialized form in our ThreadSummary output, e.g. + # `- StateChanged` inside `event_kinds:`. + if grep -rqF -- "- $v" "$SNAP_DIR"; then + covered+=("$v") + else + missing+=("$v") + fi +done <<< "$variants" + +total=$(( ${#covered[@]} + ${#missing[@]} )) +echo "EventKind coverage (${#covered[@]}/${total} — see snapshot event_kinds lists):" +for v in ${covered[@]+"${covered[@]}"}; do + echo " [x] $v" +done +for v in ${missing[@]+"${missing[@]}"}; do + echo " [ ] $v" +done + +if [ ${#missing[@]} -gt 0 ] && [ "$STRICT" = true ]; then + echo + echo "Uncovered EventKind variants fail --strict mode. Add a replay fixture" + echo "that exercises each, or mark the variant #[allow(dead_code)] if it is" + echo "genuinely unreachable." + exit 1 +fi diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index f7c57f9611..bfa980fd56 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -70,6 +70,14 @@ pub use router::{ #[cfg(feature = "libsql")] pub use router::reset_engine_state; +// `engine_retrospectives_for_test` is a test-only reachability surface — +// integration tests live in a separate crate, so `#[cfg(test)]` wouldn't +// expose it. `#[doc(hidden)]` keeps it out of public docs and signals +// that it is not a supported API. +#[cfg(feature = "libsql")] +#[doc(hidden)] +pub use router::engine_retrospectives_for_test; + // Exposed for caller-level testing of the cross-user thread_id guard #[cfg(test)] pub(crate) use router::handle_mission_notification; diff --git a/src/bridge/router.rs b/src/bridge/router.rs index 0775348b60..06969e011e 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -4631,6 +4631,50 @@ pub async fn reset_engine_state() { } } +/// Build retrospective `ExecutionTrace`s for every currently-known engine +/// thread. Returns an empty vector when engine v2 is not initialized. +/// +/// Test-only helper: snapshot-based replay tests fold each trace into +/// per-thread entries under `ReplayOutcome.engine_threads`. Not part of any +/// public API; exposed under `#[doc(hidden)]` because integration tests live +/// in a separate crate and cannot see `#[cfg(test)]`-only items. +/// +/// **Caller must serialize access** when more than one engine v2 replay can +/// run concurrently — `ENGINE_STATE` is a process-global singleton and this +/// function iterates every thread across every project. Snapshot tests in +/// `tests/e2e_engine_v2.rs` take `engine_v2_test_lock()` for this reason; +/// new test suites that spawn engine threads must do the same or clear state +/// via `reset_engine_state()` before calling. +#[cfg(feature = "libsql")] +pub async fn engine_retrospectives_for_test() +-> Vec { + let Some(lock) = ENGINE_STATE.get() else { + return Vec::new(); + }; + let guard = lock.read().await; + let Some(state) = guard.as_ref() else { + return Vec::new(); + }; + let projects = match state.store.list_all_projects().await { + Ok(projects) => projects, + Err(_) => return Vec::new(), + }; + let mut out = Vec::new(); + for project in projects { + let threads = match state.store.list_all_threads(project.id).await { + Ok(threads) => threads, + Err(_) => continue, + }; + for mut thread in threads { + if let Ok(events) = state.store.load_events(thread.id).await { + thread.events = events; + } + out.push(ironclaw_engine::executor::trace::build_trace(&thread)); + } + } + out +} + /// Resolve the effective user_id for mission management operations. /// /// If the mission is shared-owned, requires admin role and returns the shared owner id diff --git a/tests/e2e_engine_v2.rs b/tests/e2e_engine_v2.rs index 0c06d1b6c8..0fad076370 100644 --- a/tests/e2e_engine_v2.rs +++ b/tests/e2e_engine_v2.rs @@ -495,4 +495,89 @@ mod engine_v2_tests { rig.shutdown(); } + + // ----------------------------------------------------------------------- + // Phase 3: Replay regression snapshots (insta-based) + // + // Each test replays a committed trace fixture and asserts a YAML snapshot + // of `ReplayOutcome` — the observable shape of the run (tool order, final + // state, engine issues). Review drift with `cargo insta review`. + // ----------------------------------------------------------------------- + + use crate::assert_replay_snapshot; + use crate::support::replay_outcome::ReplayOutcome; + + /// Snapshot: single_tool_echo through engine v2. + /// Guards the minimum tool-call contract: one `echo` invocation and a + /// text response, with no retrospective issues. + #[tokio::test] + async fn snapshot_single_tool_echo() { + let _guard = engine_v2_test_lock().lock().await; + let trace = LlmTrace::from_file(format!("{FIXTURES}/single_tool_echo.json")).unwrap(); + let rig = TestRigBuilder::new() + .with_engine_v2() + .with_trace(trace) + .build() + .await; + + rig.send_message("Use the echo tool to repeat: 'V2 echo test'") + .await; + let responses = rig.wait_for_responses(1, TIMEOUT).await; + + let outcome = ReplayOutcome::capture(&rig, &responses).await; + assert_replay_snapshot!("single_tool_echo_v2", outcome); + rig.shutdown(); + } + + /// Snapshot: tool_error_recovery through engine v2. + /// Guards that a tool error surfaces through the status stream and the + /// agent still produces a final text response (recovery path). + #[tokio::test] + async fn snapshot_tool_error_recovery() { + let _guard = engine_v2_test_lock().lock().await; + let trace = LlmTrace::from_file(format!("{FIXTURES}/tool_error_recovery.json")).unwrap(); + let rig = TestRigBuilder::new() + .with_engine_v2() + .with_trace(trace) + .build() + .await; + + rig.send_message("Parse this json for me: not valid json {") + .await; + let responses = rig.wait_for_responses(1, TIMEOUT).await; + + let outcome = ReplayOutcome::capture(&rig, &responses).await; + assert_replay_snapshot!("tool_error_recovery_v2", outcome); + rig.shutdown(); + } + + /// Snapshot: zizmor_scan_v2 recorded live fixture. + /// Replays the largest live engine v2 trace and pins the tool-call order, + /// step count, retrospective-analyzer issue set, and final thread state + /// captured in `ReplayOutcome`. The source fixture is 3,000 lines of + /// recorded JSON; the snapshot distills it to the shape reviewers can + /// diff without context-switching into the raw driver. + #[tokio::test] + async fn snapshot_zizmor_scan_v2() { + let _guard = engine_v2_test_lock().lock().await; + let path = format!( + "{}/tests/fixtures/llm_traces/live/zizmor_scan_v2.json", + env!("CARGO_MANIFEST_DIR") + ); + let trace = LlmTrace::from_file(&path).unwrap(); + let rig = TestRigBuilder::new() + .with_engine_v2() + .with_max_tool_iterations(40) + .with_trace(trace) + .build() + .await; + + rig.send_message("can we run https://github.com/zizmorcore/zizmor") + .await; + let responses = rig.wait_for_responses(1, Duration::from_secs(300)).await; + + let outcome = ReplayOutcome::capture(&rig, &responses).await; + assert_replay_snapshot!("zizmor_scan_v2", outcome); + rig.shutdown(); + } } diff --git a/tests/e2e_live.rs b/tests/e2e_live.rs index db172600dd..0c93c2c160 100644 --- a/tests/e2e_live.rs +++ b/tests/e2e_live.rs @@ -75,8 +75,14 @@ mod live_tests { } /// Zizmor scan via engine v1 (default agentic loop). + /// + /// Marked `#[ignore]` by default because the `IRONCLAW_LIVE_TEST=1` + /// recording path needs real LLM credentials. The `replay` feature flag + /// promotes this test out of `--ignored` so CI can run the committed + /// fixture without extra `-- --ignored` gymnastics; see + /// `.github/workflows/replay-gate.yml`. #[tokio::test] - #[ignore] // Live tier: requires LLM API keys or a recorded trace fixture + #[cfg_attr(not(feature = "replay"), ignore)] async fn zizmor_scan() { let harness = LiveTestHarnessBuilder::new("zizmor_scan") .with_max_tool_iterations(40) @@ -96,7 +102,7 @@ mod live_tests { /// mentions zizmor in its response (even if it can't execute shell). /// When v2 gains auto-approve support, update this to use `run_zizmor_scan`. #[tokio::test] - #[ignore] // Live tier: requires LLM API keys or a recorded trace fixture + #[cfg_attr(not(feature = "replay"), ignore)] async fn zizmor_scan_v2() { let harness = LiveTestHarnessBuilder::new("zizmor_scan_v2") .with_engine_v2(true) diff --git a/tests/fixtures/llm_traces/README.md b/tests/fixtures/llm_traces/README.md index 03f3262cb9..1fd22e82e8 100644 --- a/tests/fixtures/llm_traces/README.md +++ b/tests/fixtures/llm_traces/README.md @@ -4,6 +4,41 @@ Trace fixtures are JSON files that script LLM behavior for deterministic E2E tes Traces can be **hand-written** or **recorded** from a live session using the `RecordingLlm` wrapper (`src/llm/recording.rs`). Recorded traces include additional fields (memory snapshots, HTTP exchanges, expected tool results) that enable fully deterministic replay. +## Two files per scenario: driver + regression snapshot + +A trace plays two distinct roles, owned by two different files: + +| Role | File | Owner | When it changes | +|------|------|-------|-----------------| +| **Replay driver** | `.json` under this directory | `RecordingLlm` / hand-written | Only when you re-record from a new live session | +| **Regression snapshot** | `tests/snapshots/replay__.snap` | `insta` via `ReplayOutcome` | Every time engine dispatch, tools, or safety changes observable output | + +The JSON encodes *what the LLM would have said*. It's a stub, not a contract, and it lives in this directory. The `.snap` encodes *what the agent actually did with that response* (tool order, final state, retrospective issues) — it's the reviewable contract, lives in `tests/snapshots/`, and is what reviewers diff on every PR touching engine code. + +Editing the JSON without re-generating the snapshot is a code smell: it means the regression the snapshot was pinning moved. Run `cargo insta review` to inspect and accept the drift. + +### When to touch which + +- **Prompt-wording change** in the engine: usually no JSON change, often a snapshot drift — normal, review and accept. +- **Tool sequencing change**: JSON stays the same; snapshot drifts to reflect the new order. +- **LLM provider switch** (new model, new backend): re-record fixtures; snapshots may or may not drift depending on how the new model routes. +- **Recording a fresh fixture**: `scripts/replay-snap.sh record ` drives both. + +## Developer ergonomics + +```bash +scripts/replay-snap.sh review # interactive diff review (cargo insta review) +scripts/replay-snap.sh accept # accept all pending snapshots +scripts/replay-snap.sh test # run the replay gate locally (cargo insta test --check) +scripts/replay-snap.sh record # record a fresh fixture against a real LLM +scripts/trace-coverage.sh # report which EventKind variants have snapshot coverage +``` + +CI runs the gate via `cargo insta test --test-runner nextest` with +`NEXTEST_PROFILE=ci`, so test binaries execute in parallel processes +and the per-test timeouts defined in `.config/nextest.toml` apply. +Local dev does not require nextest — `cargo test` still works. + ## Trace Format A trace is a model name and a list of **turns**. Each turn pairs a user message with the LLM response steps that follow it. diff --git a/tests/snapshots/replay__single_tool_echo_v2.snap b/tests/snapshots/replay__single_tool_echo_v2.snap new file mode 100644 index 0000000000..541ff28ef3 --- /dev/null +++ b/tests/snapshots/replay__single_tool_echo_v2.snap @@ -0,0 +1,59 @@ +--- +source: tests/e2e_engine_v2.rs +assertion_line: 528 +--- +response_count: 1 +has_final_response: true +tool_calls: + - name: echo + success: true +events: + - kind: thinking + message: processing... + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 100 in / 30 out tokens + - kind: tool_started + name: echo + - kind: tool_completed + name: echo + success: true + error: ~ + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 150 in / 15 out tokens + - kind: status + message: done + - kind: other + variant: Other +event_kind_counts: + Other: 1 + Status: 1 + Thinking: 5 + ToolCompleted: 1 + ToolStarted: 1 +llm_call_count: 2 +safety_warning_count: 0 +engine_threads: + - final_state: Done + step_count: 2 + message_roles: + - System + - User + - Assistant + event_kinds: + - MessageAdded + - StateChanged + - StepStarted + - StepCompleted + - ActionExecuted + - StepStarted + - StepCompleted + - StateChanged + - MessageAdded + - StateChanged + issues: + - severity: info + category: mixed_mode diff --git a/tests/snapshots/replay__tool_error_recovery_v2.snap b/tests/snapshots/replay__tool_error_recovery_v2.snap new file mode 100644 index 0000000000..7602b643ff --- /dev/null +++ b/tests/snapshots/replay__tool_error_recovery_v2.snap @@ -0,0 +1,61 @@ +--- +source: tests/e2e_engine_v2.rs +assertion_line: 550 +--- +response_count: 1 +has_final_response: true +tool_calls: + - name: json + success: false +events: + - kind: thinking + message: processing... + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 100 in / 25 out tokens + - kind: tool_started + name: json + - kind: tool_completed + name: json + success: false + error: "tool 'json' failed: tool error: tool jso" + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 180 in / 35 out tokens + - kind: status + message: done + - kind: other + variant: Other +event_kind_counts: + Other: 1 + Status: 1 + Thinking: 5 + ToolCompleted: 1 + ToolStarted: 1 +llm_call_count: 2 +safety_warning_count: 0 +engine_threads: + - final_state: Done + step_count: 2 + message_roles: + - System + - User + - Assistant + event_kinds: + - MessageAdded + - StateChanged + - StepStarted + - StepCompleted + - ActionFailed + - StepStarted + - StepCompleted + - StateChanged + - MessageAdded + - StateChanged + issues: + - severity: warning + category: tool_error + - severity: info + category: mixed_mode diff --git a/tests/snapshots/replay__zizmor_scan_v2.snap b/tests/snapshots/replay__zizmor_scan_v2.snap new file mode 100644 index 0000000000..66fe3bbaa9 --- /dev/null +++ b/tests/snapshots/replay__zizmor_scan_v2.snap @@ -0,0 +1,120 @@ +--- +source: tests/e2e_engine_v2.rs +assertion_line: 580 +--- +response_count: 1 +has_final_response: true +tool_calls: + - name: tool_search + success: true + - name: http + success: true + - name: tool_search + success: true + - name: tool_list + success: true + - name: tool_search + success: true + - name: shell + success: true +events: + - kind: thinking + message: processing... + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 9624 in / 118 out tokens + - kind: tool_started + name: tool_search + - kind: tool_completed + name: tool_search + success: true + error: ~ + - kind: tool_started + name: http + - kind: tool_completed + name: http + success: true + error: ~ + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 10895 in / 125 out token + - kind: tool_started + name: tool_search + - kind: tool_completed + name: tool_search + success: true + error: ~ + - kind: tool_started + name: tool_list + - kind: tool_completed + name: tool_list + success: true + error: ~ + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 15823 in / 65 out tokens + - kind: tool_started + name: tool_search + - kind: tool_completed + name: tool_search + success: true + error: ~ + - kind: thinking + message: calling llm... + - kind: thinking + message: step complete — 15858 in / 169 out token + - kind: tool_started + name: shell + - kind: tool_completed + name: shell + success: true + error: ~ + - kind: thinking + message: calling llm... + - kind: status + message: done + - kind: other + variant: Other +event_kind_counts: + Other: 1 + Status: 1 + Thinking: 10 + ToolCompleted: 6 + ToolStarted: 6 +llm_call_count: 4 +safety_warning_count: 0 +engine_threads: + - final_state: Failed + step_count: 4 + message_roles: + - System + - User + event_kinds: + - MessageAdded + - StateChanged + - StepStarted + - StepCompleted + - ActionExecuted + - ActionExecuted + - StepStarted + - StepCompleted + - ActionExecuted + - ActionExecuted + - StepStarted + - StepCompleted + - ActionExecuted + - StepStarted + - StepCompleted + - ActionExecuted + - StepStarted + - StateChanged + issues: + - severity: error + category: thread_failure + - severity: warning + category: no_response + - severity: error + category: llm_error diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 21817ede01..d799b07740 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -8,6 +8,7 @@ pub mod live_harness; pub mod metrics; pub mod mock_mcp_server; pub mod mock_openai_server; +pub mod replay_outcome; pub mod test_channel; pub mod test_rig; pub mod trace_llm; diff --git a/tests/support/replay_outcome.rs b/tests/support/replay_outcome.rs new file mode 100644 index 0000000000..8f28a2ce8f --- /dev/null +++ b/tests/support/replay_outcome.rs @@ -0,0 +1,384 @@ +//! Snapshot-oriented view of what a replay produced. +//! +//! A trace fixture plays two roles. The `.json` file is the **replay driver**: +//! recorded LLM responses and HTTP exchanges the harness uses to stub real +//! calls deterministically. The `.snap` file generated from this struct is the +//! **regression snapshot**: the observable output of replaying that fixture — +//! what tools fired, in what order, the final state, and any issues the +//! retrospective analyzer flagged. +//! +//! Reviewers diff the snapshot, not the raw JSON. Keep the struct narrow so +//! small prompt-wording changes don't force snapshot churn. + +#![allow(dead_code)] // Consumed by snapshot tests gated by features. + +use std::collections::BTreeMap; + +use serde::Serialize; + +use ironclaw::channels::{OutgoingResponse, StatusUpdate}; + +use crate::support::test_rig::TestRig; + +/// Short summary of a single status event, ordered and typed for snapshot +/// review. Excludes wall-clock fields, request IDs, and full tool output — +/// those live in the replay driver JSON, not here. +#[derive(Debug, Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum EventSummary { + Thinking { + message: String, + }, + ToolStarted { + name: String, + }, + ToolCompleted { + name: String, + success: bool, + error: Option, + }, + ToolResultPreview { + name: String, + /// Character length of the preview the UI received. The full preview + /// text lives in the replay driver and is sensitive to prompt drift — + /// using a bucketed length keeps snapshots stable. + preview_len_bucket: usize, + }, + Status { + message: String, + }, + ApprovalNeeded { + tool_name: String, + }, + AuthRequired { + extension_name: String, + }, + AuthCompleted { + extension_name: String, + success: bool, + }, + Suggestions { + count: usize, + }, + /// Anything else we haven't explicitly modelled. Kept as a bucketed + /// variant so unrelated new status kinds don't spam snapshot diffs. + Other { + variant: &'static str, + }, +} + +/// Summary of a single tool invocation that the rig observed. +#[derive(Debug, Serialize)] +pub struct ToolCallSummary { + pub name: String, + pub success: bool, +} + +/// Summary of a retrospective issue the engine flagged. +#[derive(Debug, Serialize)] +pub struct TraceIssueSummary { + pub severity: String, + pub category: String, +} + +/// Summary of one engine thread's post-run state. +#[derive(Debug, Serialize)] +pub struct ThreadSummary { + pub final_state: String, + pub step_count: usize, + pub message_roles: Vec, + pub event_kinds: Vec, + pub issues: Vec, +} + +/// Regression snapshot of a replay run. +/// +/// Serialized as YAML by [`assert_replay_snapshot`]. Stable under LLM prompt +/// drift: it captures shape (tool sequence, final state, issue categories) +/// rather than model text. +#[derive(Debug, Serialize)] +pub struct ReplayOutcome { + /// Number of outbound text responses the channel received. + pub response_count: usize, + /// Whether any final response was produced. Cheap for reviewers to + /// interpret — `false` means the scenario hit a dead-end. + pub has_final_response: bool, + /// Tool invocations the channel observed, in order. + pub tool_calls: Vec, + /// Ordered status events, bucketed and trimmed for stability. + pub events: Vec, + /// Histogram of status event kinds — makes coverage assertions cheap. + pub event_kind_counts: BTreeMap, + /// Raw number of LLM calls observed during the replay. Not bucketed — + /// the fixture pins each step's response, so drift here reflects a real + /// change in how many times the engine called the provider. + pub llm_call_count: u32, + /// Number of safety-warning status events observed. + pub safety_warning_count: usize, + /// Per-thread retrospective analyzer output. Empty for engine v1 replays. + pub engine_threads: Vec, +} + +impl ReplayOutcome { + /// Capture the outcome of a just-completed replay from the rig. + /// + /// Call this after `wait_for_responses` / `run_trace`, before `shutdown`. + pub async fn capture(rig: &TestRig, responses: &[OutgoingResponse]) -> Self { + let status_events = rig.captured_status_events(); + let mut events: Vec = Vec::with_capacity(status_events.len()); + let mut kind_counts: BTreeMap = BTreeMap::new(); + let mut safety_warning_count = 0usize; + + for event in status_events { + let summary = match event { + StatusUpdate::Thinking(msg) => { + *kind_counts.entry("Thinking".into()).or_default() += 1; + EventSummary::Thinking { + message: bucket_text(&msg), + } + } + StatusUpdate::ToolStarted { name, .. } => { + *kind_counts.entry("ToolStarted".into()).or_default() += 1; + EventSummary::ToolStarted { + name: strip_tool_params(&name), + } + } + StatusUpdate::ToolCompleted { + name, + success, + error, + .. + } => { + *kind_counts.entry("ToolCompleted".into()).or_default() += 1; + EventSummary::ToolCompleted { + name: strip_tool_params(&name), + success, + error: error.map(|e| bucket_text(&e)), + } + } + StatusUpdate::ToolResult { name, preview, .. } => { + *kind_counts.entry("ToolResult".into()).or_default() += 1; + EventSummary::ToolResultPreview { + name: strip_tool_params(&name), + preview_len_bucket: bucket_usize(preview.chars().count(), 100), + } + } + StatusUpdate::StreamChunk(_) => { + *kind_counts.entry("StreamChunk".into()).or_default() += 1; + continue; + } + StatusUpdate::Status(msg) => { + *kind_counts.entry("Status".into()).or_default() += 1; + if is_safety_warning(&msg) { + safety_warning_count += 1; + } + EventSummary::Status { + message: bucket_text(&msg), + } + } + StatusUpdate::JobStarted { .. } => { + *kind_counts.entry("JobStarted".into()).or_default() += 1; + EventSummary::Other { + variant: "JobStarted", + } + } + StatusUpdate::ApprovalNeeded { tool_name, .. } => { + *kind_counts.entry("ApprovalNeeded".into()).or_default() += 1; + EventSummary::ApprovalNeeded { tool_name } + } + StatusUpdate::AuthRequired { extension_name, .. } => { + *kind_counts.entry("AuthRequired".into()).or_default() += 1; + EventSummary::AuthRequired { extension_name } + } + StatusUpdate::AuthCompleted { + extension_name, + success, + .. + } => { + *kind_counts.entry("AuthCompleted".into()).or_default() += 1; + EventSummary::AuthCompleted { + extension_name, + success, + } + } + StatusUpdate::ImageGenerated { .. } => { + *kind_counts.entry("ImageGenerated".into()).or_default() += 1; + EventSummary::Other { + variant: "ImageGenerated", + } + } + StatusUpdate::Suggestions { suggestions } => { + *kind_counts.entry("Suggestions".into()).or_default() += 1; + EventSummary::Suggestions { + count: suggestions.len(), + } + } + StatusUpdate::ReasoningUpdate { .. } => { + *kind_counts.entry("ReasoningUpdate".into()).or_default() += 1; + EventSummary::Other { + variant: "ReasoningUpdate", + } + } + _ => { + *kind_counts.entry("Other".into()).or_default() += 1; + EventSummary::Other { variant: "Other" } + } + }; + events.push(summary); + } + + let tool_calls: Vec = rig + .tool_calls_completed() + .into_iter() + .map(|(name, success)| ToolCallSummary { + name: strip_tool_params(&name), + success, + }) + .collect(); + + let has_final_response = !responses.is_empty(); + + let engine_threads = capture_engine_threads().await; + + Self { + response_count: responses.len(), + has_final_response, + tool_calls, + events, + event_kind_counts: kind_counts, + llm_call_count: rig.llm_call_count(), + safety_warning_count, + engine_threads, + } + } +} + +/// Engine v2 formats tool names as `"echo(hello...)"`. The parameter summary +/// is useful for logs but depends on model wording and is a churn source — +/// strip it before snapshotting. +fn strip_tool_params(name: &str) -> String { + match name.find('(') { + Some(i) => name[..i].to_string(), + None => name.to_string(), + } +} + +/// Bucket free-form message text to something stable under small rewording. +/// Keeps leading ~40 chars, lowercased, to help reviewers recognize which +/// event fired without comparing full model output. +fn bucket_text(s: &str) -> String { + let trimmed: String = s.chars().take(40).collect(); + trimmed.to_lowercase() +} + +fn bucket_usize(value: usize, bucket: usize) -> usize { + if bucket == 0 { + return value; + } + (value / bucket) * bucket +} + +fn is_safety_warning(msg: &str) -> bool { + let lower = msg.to_lowercase(); + lower.contains("sanitiz") || lower.contains("inject") || lower.contains("warning") +} + +#[cfg(feature = "libsql")] +async fn capture_engine_threads() -> Vec { + let traces = ironclaw::bridge::engine_retrospectives_for_test().await; + traces.into_iter().map(thread_summary_from).collect() +} + +#[cfg(not(feature = "libsql"))] +async fn capture_engine_threads() -> Vec { + Vec::new() +} + +#[cfg(feature = "libsql")] +fn thread_summary_from(trace: ironclaw_engine::executor::trace::ExecutionTrace) -> ThreadSummary { + use ironclaw_engine::executor::trace::IssueSeverity; + + let issues = trace + .issues + .into_iter() + .map(|issue| { + let severity = match issue.severity { + IssueSeverity::Error => "error", + IssueSeverity::Warning => "warning", + IssueSeverity::Info => "info", + } + .to_string(); + TraceIssueSummary { + severity, + category: issue.category, + } + }) + .collect(); + + let message_roles = trace.messages.iter().map(|m| m.role.clone()).collect(); + + let event_kinds = trace + .events + .iter() + .map(|e| event_kind_name(&e.kind).to_string()) + .collect(); + + ThreadSummary { + final_state: format!("{:?}", trace.final_state), + step_count: trace.step_count, + message_roles, + event_kinds, + issues, + } +} + +/// Exhaustive `match` on `EventKind` — not pulled from `Debug` or a `strum` +/// derive on purpose. Adding a variant upstream breaks this match, which is +/// the signal we want: a new engine event must be consciously classified as +/// either worth snapshotting or explicitly ignored, not silently swallowed +/// under the default `Debug` string. The duplication is the enforcement. +#[cfg(feature = "libsql")] +fn event_kind_name(kind: &ironclaw_engine::EventKind) -> &'static str { + use ironclaw_engine::EventKind; + match kind { + EventKind::StateChanged { .. } => "StateChanged", + EventKind::StepStarted { .. } => "StepStarted", + EventKind::StepCompleted { .. } => "StepCompleted", + EventKind::StepFailed { .. } => "StepFailed", + EventKind::ActionExecuted { .. } => "ActionExecuted", + EventKind::ActionFailed { .. } => "ActionFailed", + EventKind::LeaseGranted { .. } => "LeaseGranted", + EventKind::LeaseRevoked { .. } => "LeaseRevoked", + EventKind::LeaseExpired { .. } => "LeaseExpired", + EventKind::MessageAdded { .. } => "MessageAdded", + EventKind::ChildSpawned { .. } => "ChildSpawned", + EventKind::ChildCompleted { .. } => "ChildCompleted", + EventKind::ApprovalRequested { .. } => "ApprovalRequested", + EventKind::ApprovalReceived { .. } => "ApprovalReceived", + EventKind::SelfImprovementStarted => "SelfImprovementStarted", + EventKind::SelfImprovementComplete { .. } => "SelfImprovementComplete", + EventKind::SelfImprovementFailed { .. } => "SelfImprovementFailed", + EventKind::SkillActivated { .. } => "SkillActivated", + EventKind::CodeExecutionFailed { .. } => "CodeExecutionFailed", + EventKind::OrchestratorRollback { .. } => "OrchestratorRollback", + EventKind::Unknown => "Unknown", + } +} + +/// Assert that `outcome` matches the saved YAML snapshot for `name`. +/// +/// Snapshots live at `tests/snapshots/replay__{name}.snap`. Use +/// `cargo insta review` to accept snapshot diffs interactively. +#[macro_export] +macro_rules! assert_replay_snapshot { + ($name:expr, $outcome:expr) => {{ + let mut settings = ::insta::Settings::clone_current(); + settings.set_snapshot_path(concat!(env!("CARGO_MANIFEST_DIR"), "/tests/snapshots")); + settings.set_prepend_module_to_snapshot(false); + settings.set_sort_maps(true); + settings.set_omit_expression(true); + settings.bind(|| { + ::insta::assert_yaml_snapshot!(format!("replay__{}", $name), $outcome); + }); + }}; +}