(MOT-4107) feat(harness): event-driven integration E2E runner#518
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a deterministic harness-integration Rust crate: typed scenario/recorder/router contracts, process and stack supervision, recorder/readiness/scripted-router services, scenario expansion/compilation, phased execution and reporting, a scenario catalog with CLI. It also persists hook-mutated arguments across held/released calls in harness core, adds Playwright console E2E infrastructure and UI test hooks, and wires CI/Makefile/README for the new suite. ChangesHarness integration test framework
Estimated code review effort: 5 (Critical) | ~120 minutes Hook mutation persistence in harness core
Console E2E testing infrastructure
CI and build tooling for integration tests
Sequence Diagram(s)sequenceDiagram
participant Runner as ScenarioRunner
participant Stack as Stack
participant Recorder as Recorder
participant Router as ScriptedRouter
participant Engine as EngineClient
Runner->>Stack: boot isolated engine+workers
Runner->>Recorder: configure + reset
Runner->>Router: start scripted generations
Runner->>Engine: probe / arm readiness
Runner->>Engine: harness::send (scenario trigger)
Engine->>Router: router::chat
Router-->>Engine: streamed frames + terminal response
Engine->>Recorder: TargetCall event
Runner->>Engine: await harness::status (terminal)
Runner->>Runner: collect transcript/evidence
Runner->>Runner: floor::floor_failure / verify
Runner->>Stack: teardown
Runner->>Runner: write result.json / execution.json
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 47 skipped (no docs/).
Four for four. Nicely done. |
This was referenced Jul 16, 2026
ytallo
force-pushed
the
feat/harness-conformance-e2e
branch
from
July 16, 2026 17:48
d96fe06 to
5aa6b80
Compare
Implements the first slice of the harness conformance spec (tech-specs/2026-07-15-harness-evaluation/conformance-e2e.md): a standalone runner at harness/evals/conformance that boots a fresh isolated stack per scenario (pinned engine + real queue, session-manager, context-manager, iii-directory, harness), replaces only the router::* boundary with a strict scripted worker, and grades public evidence with pure-code invariants. - Scenarios C-E2E-001 (streamed text) and C-E2E-002 (allowed function executes exactly once) pass end-to-end in ~6s each. - make -C harness conformance-e2e III_BIN=<engine> entry point; non-required harness-conformance CI job builds the engine pinned in engine.lock (never downloads) and uploads results. - Fault-injection plumbing (engine SIGKILL + respawn), quarantine flag, and the calls_closed invariant land with the runner for crash-recovery scenarios. - Spec updated in place: status header plus implementation-verified corrections (system-prompt template hashing, boot order, resolved config subset checks, 0-based request_id steps, durable-usage semantics, invariant registry).
Adds scenario C-E2E-507 (scenarios/crash-recovery-507): the model dispatches one allow-listed call whose execution takes 8s; the engine is SIGKILLed mid-call and respawned from the same config and data directories. Invariants assert the issue's EXPECTED recovery behavior: the side effect runs at most once (holds today), the interrupted function_call is closed by a durable function_result, and the turn reaches a terminal usable state (both fail today). Marked quarantine: true — excluded from --scenario all so CI stays green; run explicitly with --scenario C-E2E-507. Unquarantine when the fix lands to make it the permanent crash-recovery regression gate. Observed on engine 0.21.8-next.1 while bringing this up (recorded in spec note 13): the in-flight harness-turn job stays stranded in the surviving queue worker's inflight set (attempts 0, 30-minute visibility timeout), the turn record dies with the engine's in-memory iii-state, harness::status returns null, and the transcript keeps the dangling function_call — the session poisoning reported in the issue, plus a wedge the issue's setup did not hit. Refs: #507
…505/#506 Console recording (diagnostics only, never an oracle): --console-bin spawns the console worker per-run; --record-console captures the chat view via headless system Chrome (playwright-core + tools/console-recorder, deps preinstalled — the runner never downloads during a test). Send holds until the recorder page loads so even seconds-long turns are fully on video; recordings land at scenarios/<id>/console-recording.webm. Hook-chain scenario support: recorder.extra_functions (run-scoped controlled functions with declared decisions), scenario bindings (trigger bindings created after harness boot, held until visible in engine::registered-triggers::list), scenario release (resolve a held call via harness::function::resolve once it parks), and payload_subset on the target.calls invariant. Quarantined repros (assert EXPECTED behavior; fail until fixed): - C-E2E-505: a pre-trigger hook returning hold + mutations has the mutations discarded at parse (hooks/runner.rs parse_output). - C-E2E-506: releasing a held call with resolve{action:execute} runs the ORIGINAL transcript arguments, skipping hook mutations (deferred.rs find_call_arguments). Both verified live: hook-envelope invariants pass (chain order proven), the target-payload invariant fails with the exact reported values. Refs: #505, #506
The harness-evaluation tech spec moved to the iii repo; this branch now carries only the test implementation. The runner's operational docs and the implementation-verified spec corrections (system-prompt templating, boot order vs the registry snapshot, resolved-config subset checks, 0-based request_id steps, durable-usage semantics, invariant registry, fault seeds, hook-chain scenario support, quarantine policy) live in harness/evals/conformance/README.md; code/CI references updated.
Drop the changed-paths gate (with engine and worker builds cached the job is cheap enough to gate every PR on the public harness contracts) and run the conformance crate's unit tests in the job — the crate has no iii.worker.yaml, so the discover-driven rust matrix never covers it. Document the quarantine lifecycle in the README: repro scenarios are un-quarantined in the same PR as their fix.
ytallo
force-pushed
the
feat/harness-conformance-e2e
branch
from
July 17, 2026 13:29
97ab899 to
a6ebf0d
Compare
The default --artifacts-dir is relative and children are spawned with per-run working directories, so the engine resolved its generated config path against the wrong cwd and the documented direct CLI could not boot (CI masked it by passing an absolute path). Canonicalize the artifacts dir and every CLI-supplied binary before anything spawns.
Adding scrapling to STACK broke build, install-local and cargo-clean, which assume every worker has a Cargo.toml and a Rust binary. Split STACK into RUST_WORKERS/PYTHON_WORKERS: cargo targets iterate the Rust subset, build routes scrapling to prepare-scrapling, install-local and cargo-clean skip it with an explicit message.
head() sliced the canonical JSON at byte 200, which panics when the cut lands inside a multibyte character — exactly while reporting a mismatch, with the router mutex held (poisoning follow-up evidence collection). Back the cut off to a char boundary and pin it with a regression test.
…tract The frame mirror required partial on every delta variant, but llm-router defines it as Option and emits slim deltas today — a valid slim fixture was rejected and the streaming scenario only ever exercised the legacy fat form. Mirror the Option, regenerate the golden schemas, and switch C-E2E-001 to slim deltas so the E2E covers the format providers emit.
An empty scenarios dir (or one with every scenario quarantined) ran zero scenarios and exited 0, letting the CI gate silently become a no-op. Empty selection is now a runner_error (exit 3) in both run and --validate-only modes. Also cover both delta wire forms (slim and legacy fat) in the fixture validation tests.
…crash harness::send and function::resolve failures returned contract_failure without consulting stack.early_exit(), so an engine or harness crash mid-RPC produced exit 2 instead of the documented process_crash/exit 3. Check for a dead child on both RPC error paths and once more before the final classification — a stack that did not outlive Collect cannot be trusted for grading, and an early exit outranks an ordinary timeout (spec step 7).
The generic "token" key fragment rejected the mandatory max_output_tokens model field — i.e. every structurally valid cassette; the clean-path test only passed because it scanned a reduced object. Match credential token keys by exact name (folded -/_) instead, keep the other fragments, and make the clean-path test scan a complete RouterCassetteV1.
transcript.calls_closed is vacuously true when the crash eats the function_call message entirely, and nothing required the second recovery generation to be consumed. Require the full recovered turn: message counts, a durable result closing call-1, the "recovered" assistant text, and both scripted generations consumed.
main's harness now stamps `terminal: bool` on turn-completed payloads (false = non-final completion while a wake is armed). The recorder's strict lifecycle parse rejected the unknown field, so no lifecycle delivery was ever recorded and every scenario failed the floor in CI — the suite catching a real wire-contract change, as designed. LifecycleEventV1 gains the required `terminal` field, and the floor now also requires `terminal: true`: these scenarios run single, final turns.
… observe driver (#556) * (MOT-4107) refactor(integration): simplify boot readiness and add the observe driver Boot readiness moves from structural contract probing (readiness/, ~850 lines: golden schemas, config-entry diffing, queue-topic checks) to presence-only discovery (discovery.rs, ~90 lines): wait for the harness function surface plus the workers it calls mid-turn to be registered, nothing more. Completion is now event-driven — Arm binds harness::turn-completed once and Await blocks on delivery instead of polling harness::status every 250ms, with one status call afterward as the durable-state confirmation the floor checks. Replaces the Console/serve driver with Observe: the integration keeps owning stimulus (harness::send after a start.json signal) and Playwright owns the Console process and DOM assertions directly, spawning CONSOLE_BIN itself instead of asking the integration binary to do it. --console-bin, spawn_console, and the HTTP-port plumbing are gone from the crate; ObserveReadyV1/ObserveResultV1 replace the old serve types. Console e2e specs call stack.start() instead of stack.trigger('harness::send', ...). Also: - Router function goldens are read directly from llm-router/tests/golden/schemas/*.json via include_str! instead of a local copy embedded in readiness contracts. - --repeat/INTEGRATION_REPEAT is gone (CLI, Makefile, CI): the byte-stable scrub is covered by tests/determinism.rs, so booting the full stack twice per scenario was a meta-test, not a contract check. - ModelFixtureV1 drops pricing, reasoning_efforts, thinking_budgets, input_limit, and display_name — fields the compiler always left None and nothing downstream (the scripted router or the harness's own Model parse) ever reads. - Recorder::snapshot() drops its unused after_sequence filter; a few internal-only helpers (Client::call_with_timeout, Deadline::at/cap, the stack config re-exports) move to pub(crate) or private; readiness_deadline is renamed setup_deadline now that the module it named is gone. No scenario behavior change. Validated with cargo fmt/clippy (clean), cargo test (96 passing), and live Direct-driver runs (E2E-001, E2E-002) against a local stack. * (MOT-4107) refactor(integration): focus conformance suite on core flows * (MOT-4107) refactor(integration): add typed scenario DSL * (MOT-4107) refactor(harness): move integration runner into test workspace * (MOT-4107) test: stabilize shell and integration process tests
Adds UI-002 (multi-turn-traces): a function-call turn followed by a
second Console-triggered turn on the same session must stay two
distinct, durable traces rather than collapsing into one.
- DSL: Scenario::terminal_turns(count) declares how many discrete
terminal turns a fixture expects; Request::turn_request_step(n) pins
a generation's request id to a specific turn instead of the
continuous single-turn ordinal.
- floor::lifecycle_failure takes the expected turn count: at-least-once
retries for the same turn_id are accepted if their normalized
payloads are identical, but distinct turns must each appear, stay
ordered, and the latest must bind to run.turn_id.
- Recorder::wait_for_lifecycle_turns(expected, deadline) waits for N
distinct turn ids instead of the first lifecycle event;
wait_for_lifecycle is now its single-turn special case.
- Playground driver: refresh_external_completion waits for every
declared terminal turn on shutdown (Playground can keep running
after its first completed turn) before confirming durable status.
- Console: GroupedTraceList exposes data-trace-group-{value,count,
errors} for the new Playwright assertions; ChatDock's default width
is now half the viewport instead of a fixed 440px, giving the wider
grouped trace view room to breathe.
Validated with cargo fmt/clippy (clean) and cargo test (77 passing) in
harness/tests/e2e; tsc typecheck (app + e2e), vitest (1052 passing),
and biome (clean) in console/web.
ytallo
changed the title
andersonleal
approved these changes
Jul 22, 2026
andersonleal
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the first slice of the harness conformance E2E runner described in the harness evaluation spec. The runner boots an isolated iii stack, drives scenarios through the public harness surface, and grades persisted evidence instead of relying on internal test hooks.
Scope
router::*boundary with a strict scripted worker.Event-driven coordination
harness::readytrigger so readiness is explicit and supports late subscribers.SIGCHLDon Unix and keeps bounded cleanup checks for descendant process groups.Validation
-D warnings, and all-feature tests pass.Fixes MOT-4107