From ff119531d43506e014d1e2101899c983e2ff0ec8 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sun, 19 Apr 2026 17:34:01 +0900 Subject: [PATCH] test(replay): promote engine traces to insta-backed snapshot gate (#2621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(replay): promote engine replay traces to insta-backed snapshot gate Adds a ReplayOutcome snapshot type, a replay-gate CI workflow, and a developer script wrapper for cargo-insta. Replaces unreviewable 3,000-line JSON diffs on engine changes with a YAML snapshot of the observable run shape (tool sequence, final state, retrospective analyzer issues). Why: engine v2 live-fixture traces had grown past reviewability. A single prompt-wording change could move the whole fixture, and reviewers had no way to see which behaviour actually changed. Splitting the fixture into a "replay driver" (JSON stays in tests/fixtures/) and a "regression snapshot" (YAML in tests/snapshots/) gives reviewers a narrow, stable diff to approve, while keeping the full recorded context for deterministic replay. Changes: - `tests/support/replay_outcome.rs` — ReplayOutcome + assert_replay_snapshot! macro; snapshots include retrospective analyzer output (TraceIssue severity/category) via a new `ironclaw::bridge::engine_retrospectives_for_test()` helper that runs `build_trace()` over engine threads - `tests/e2e_engine_v2.rs` — three POC snapshot tests (single_tool_echo, tool_error_recovery, zizmor_scan_v2) - `tests/e2e_bug_bash_snapshots.rs` + `tests/fixtures/llm_traces/bug_bash/` — bug-regression fixture template, mapped to open issues in the README - `.github/workflows/replay-gate.yml` — cargo insta test --check on engine/agent/LLM/tools/bridge path changes; rejects committed .snap.new - `scripts/replay-snap.sh` — review/accept/test/record wrappers around cargo-insta and IRONCLAW_RECORD_TRACE - `scripts/trace-coverage.sh` — reports EventKind variants with snapshot coverage; `--strict` mode for future CI promotion - `tests/e2e_live.rs` — `#[ignore]` swapped for `cfg_attr(not(feature="replay"), ignore)` so the replay CI job can run the scenarios without `-- --ignored` - `Cargo.toml` — new `replay = ["libsql"]` feature; insta gains the `yaml` feature - `tests/fixtures/llm_traces/README.md` — documents the two-role driver/snapshot split Co-Authored-By: Claude Opus 4.7 (1M context) * test(replay): address PR #2621 review + swap cargo-insta installer Review fixes: - Replay gate was missing the bug-bash snapshot suite. Adds `tests/e2e_bug_bash_snapshots.rs` to the workflow paths trigger and the `cargo insta test --check` invocation so bug-regression snapshots are actually gated. (copilot-pull-request-reviewer) - `cargo install cargo-insta --locked` added ~40s of cold-cache compile to the gate. Swapped for `taiki-e/install-action@v2`, which downloads a precompiled binary in a few seconds. Also updated `scripts/replay-snap.sh` to *fail closed* when cargo-insta is missing instead of silently auto-installing it. (gemini-code-assist) - `engine_retrospectives_for_test` was `pub` and re-exported under the default-enabled `libsql` feature, contradicting its "not part of any public API" doc. Split the re-export, kept `reset_engine_state` as a plain `pub use`, and hid `engine_retrospectives_for_test` behind `#[doc(hidden)]` — it still needs to cross the crate boundary for integration tests (which live in a separate crate, so `#[cfg(test)]` doesn't reach them), but no longer appears in published docs. (copilot-pull-request-reviewer) - Added an explicit "caller must serialize" note on `engine_retrospectives_for_test` explaining the `ENGINE_STATE` singleton and pointing new callers at `engine_v2_test_lock()` / `reset_engine_state()`. Matches what the existing snapshot tests already do. (gemini-code-assist) Doc corrections: - `snapshot_zizmor_scan_v2` doc claimed the snapshot pinned `ApprovalNeeded` events and response wording — it doesn't. Rewrote to describe what the snapshot actually asserts (tool order, step count, retrospective issues, final state). (copilot-pull-request-reviewer) - `llm_call_count` was documented as "bucketed" but passed through verbatim. Updated the field doc to reflect the raw value. Bucketing wasn't needed because fixtures are deterministic. (copilot-pull-request-reviewer) - `src/bridge/router.rs` doc referenced a non-existent `ReplayOutcome.trace_issues` field — the struct uses `engine_threads`. Fixed the reference. (copilot-pull-request-reviewer) - `scripts/trace-coverage.sh` header claimed CI runs it with `--strict`; the workflow runs it in advisory mode. Rewrote the header to match, with a pointer for when to promote to strict. (copilot-pull-request-reviewer) No-change replies (rationale commented in the code): - `event_kind_name` uses an exhaustive `match` on `EventKind` rather than `Debug` or a `strum` derive. The compile-time exhaustiveness check is the point — adding a new engine event should force a conscious decision about how the snapshot represents it, not a silent fallthrough. Added a comment making that intent explicit. - `trace-coverage.sh` awk parser of `event.rs` is fragile — agreed, but the script is advisory and its failure mode is false negatives (uncovered variants simply aren't gated). Documented the tradeoff and the rewrite-in-Rust escape hatch in the script header. Co-Authored-By: Claude Opus 4.7 (1M context) * ci(replay-gate): prime cache on staging, restrict PR runs to read-only The second run on PR #2621 missed the cache ("No cache found" in the rust-cache restore step) even though the workflow is wired correctly. Root cause: the repo sits close to GitHub's 10 GB per-repo cache quota (~59 entries, many >500 MB), and the LRU policy evicts PR-scoped caches before they get reused. Fix: - Add `push: [staging, main]` so the gate runs (and saves a ~1.2 GB cache under the `replay-gate` key) on every merge to the branches PRs actually target. Subsequent PRs restore from that base-branch cache — GitHub Actions permits cross-ref restore when the restoring ref's base matches the saved ref. - Set `save-if: ${{ github.event_name == 'push' }}` so PR runs only *read* the cache. Without this gate, each PR push would save its own copy and crowd out the primed base-branch cache, putting us right back in the eviction loop. Expected effect: cold-cache 9m → warm ~2-3m once staging has a run with the new workflow. Base-branch prime run still pays 9m (no regression). Co-Authored-By: Claude Opus 4.7 (1M context) * test(replay): drop bug-bash fixture scaffolding Replay fixtures can't reproduce the Phase 3 target bugs because the fixture *is* the LLM's output — handwriting a trace where the LLM emits a tool call doesn't test whether the real LLM would have emitted that call, only that the harness dispatches a scripted one. What `summarization_uses_tools.json` actually pinned was the happy path, not the #2541 bug. Of the 7 open bug-bash issues, only #2544 ("plans and delegates but never executes") is catchable by replay, and only via a live-recorded fixture. The other six are LLM-behavior or infra-timing bugs outside replay's reach. Rather than ship regression theater, tear out the scaffolding. Removed: - tests/e2e_bug_bash_snapshots.rs - tests/fixtures/llm_traces/bug_bash/ - tests/snapshots/replay__bug_bash_summarization_uses_tools.snap Unwired: - Replay-gate workflow paths + test list no longer mention bug_bash - scripts/replay-snap.sh test command drops the extra --test flag Co-Authored-By: Claude Opus 4.7 (1M context) * ci: switch to cargo-nextest with per-test timeouts Nextest runs each integration test in its own process and runs test binaries in parallel, which is a big unlock for this repo: - Engine v2 tests share a process-global `ENGINE_STATE` singleton (OnceLock), which the current test lock serialises inside a single test binary. Nextest's process-per-test model gives each test a clean state automatically, so the 16 engine_v2 tests stop running one-by-one. - Cross-binary parallelism: `cargo test --test A --test B` runs binaries in sequence; nextest runs them concurrently. Measured locally: the replay-gate test set (3 binaries, 21 tests) went from ~30s sequential to **2.7s parallel**. Adds `.config/nextest.toml` with: - `slow-timeout = 60s / terminate-after 3` in the default profile so a hung test fails fast instead of blocking the workflow-level 25- minute cap. - A `ci` profile with `fail-fast = false` (one flake shouldn't mask other failures), `failure-output = immediate-final`, `success-output = never` for readable Actions logs. - Per-test 300s override for the handful of genuinely slow scenarios (zizmor scan, e2e_thread_scheduling). Workflows updated: - `replay-gate.yml`: installs cargo-nextest via taiki-e/install-action alongside cargo-insta (one step), runs `cargo insta test --test-runner nextest` with `NEXTEST_PROFILE=ci`. - `test.yml`: all five `cargo test` invocations swapped for `cargo nextest run --profile ci`. Nextest doesn't execute doctests, so every nextest step is paired with a `cargo test --doc` follow-up to preserve coverage. Local dev is unchanged — `cargo test` still works; nextest is only required in CI. Co-Authored-By: Claude Opus 4.7 (1M context) * ci: re-trigger replay-gate workflow after nextest migration Previous push only modified workflow files and `.config/nextest.toml`; GitHub skipped the `pull_request` workflow events for that sync, so the nextest migration didn't actually get exercised in CI. Empty commit forces re-evaluation. Co-Authored-By: Claude Opus 4.7 (1M context) * docs(replay): note nextest wiring in the fixtures README Also forces a CI re-run: the previous empty commit had no matching paths, so the `pull_request.paths` filters skipped every workflow including replay-gate. Touching a file under `tests/fixtures/llm_traces/**` re-matches the filter and runs the nextest-based gate. Co-Authored-By: Claude Opus 4.7 (1M context) * ci(test): defer test.yml nextest migration Staging restructured test.yml significantly while this PR was open (matrix-config dynamic matrix, `changes` code-detection job, composite install-cargo-component action, save-if restricted to base-branch pushes). The merge into staging had heavy conflicts for every nextest-swap hunk. Rather than force a re-layering of the new staging structure on top of the nextest migration in this PR, revert test.yml to staging's current version. This PR now scopes the nextest change to just the replay-gate workflow (where it cleanly demonstrates the value) plus the shared `.config/nextest.toml` profile. Migrating the rest of test.yml to nextest is a follow-up that can rebase on the new structure without the heavy conflict surface. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Henry Park Co-authored-by: Claude Opus 4.7 (1M context) --- .config/nextest.toml | 36 ++ .github/workflows/replay-gate.yml | 102 +++++ Cargo.lock | 1 + Cargo.toml | 6 +- scripts/replay-snap.sh | 77 ++++ scripts/trace-coverage.sh | 89 ++++ src/bridge/mod.rs | 8 + src/bridge/router.rs | 44 ++ tests/e2e_engine_v2.rs | 85 ++++ tests/e2e_live.rs | 10 +- tests/fixtures/llm_traces/README.md | 35 ++ .../replay__single_tool_echo_v2.snap | 59 +++ .../replay__tool_error_recovery_v2.snap | 61 +++ tests/snapshots/replay__zizmor_scan_v2.snap | 120 ++++++ tests/support/mod.rs | 1 + tests/support/replay_outcome.rs | 384 ++++++++++++++++++ 16 files changed, 1115 insertions(+), 3 deletions(-) create mode 100644 .config/nextest.toml create mode 100644 .github/workflows/replay-gate.yml create mode 100755 scripts/replay-snap.sh create mode 100755 scripts/trace-coverage.sh create mode 100644 tests/snapshots/replay__single_tool_echo_v2.snap create mode 100644 tests/snapshots/replay__tool_error_recovery_v2.snap create mode 100644 tests/snapshots/replay__zizmor_scan_v2.snap create mode 100644 tests/support/replay_outcome.rs 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); + }); + }}; +}