(MOT-4107) refactor(integration): simplify boot readiness and add the observe driver#556
Merged
ytallo merged 5 commits intoJul 22, 2026
Conversation
… 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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. |
ytallo
added a commit
that referenced
this pull request
Jul 22, 2026
* feat(harness): add conformance E2E runner (first slice) 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). * test(conformance): quarantined crash-recovery repro for #507 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 * test(conformance): console recording profile + quarantined repros for #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 * docs(conformance): runner README with implementation notes 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. * ci(conformance): run the suite on every PR and test the runner crate 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. * fix(conformance): make CLI paths absolute before children chdir 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. * fix(harness): keep cargo Make targets off the Python scrapling worker 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. * fix(conformance): truncate mismatch previews on char boundaries 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. * fix(conformance): accept slim delta frames per the current router contract 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. * fix(conformance): error when --scenario all selects nothing 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. * fix(conformance): classify RPC errors after a child death as process_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). * fix(conformance): scope the cassette token denylist to credential keys 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. * test(conformance): pin the recovered turn's shape in C-E2E-507 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. * refactor(harness): rename the conformance E2E suite to integration-e2e Per review follow-up: the suite reads better as the harness's integration test track. Renames the Make target (conformance-e2e → integration-e2e), the crate/dir (harness/evals/conformance → harness/evals/integration, harness-conformance → harness-integration), the CI job/artifacts/cache keys, the recorder/router worker and function ids, error prefixes, schema names and golden files, and the default artifacts dir (target/conformance → target/integration). Deliberately unchanged: the C-E2E-xxx scenario ids (the harness- evaluation tech spec in the iii repo references them as its catalog) and engine.lock. * fix(integration): refresh expected prompts after merging main main's MOT-3948/MOT-4100 work rewrote the harness built-in prompt, so the checked-in expected/system-prompt.txt templates (old prompt + aid tail) no longer matched what the merged harness sends — CI runs the PR merge commit and failed both scenarios on the system_prompt sha256. Regenerated every template as new default.txt + unchanged aid tail. * (MOT-4107) refactor(harness): simplify integration E2E scenarios and runner * (MOT-4107) refactor(harness): split integration scenario compiler modules * (MOT-4107) refactor(harness): centralize integration compiler contracts * (MOT-4107) refactor(harness): centralize integration runtime ownership * (MOT-4107) refactor(harness): decompose integration scenario runner (#534) * (MOT-4107) refactor(harness): simplify integration E2E scenarios and runner (#531) * (MOT-4107) refactor(harness): simplify integration E2E scenarios and runner * (MOT-4107) refactor(harness): split integration scenario compiler modules * (MOT-4107) refactor(harness): centralize integration compiler contracts (#532) * (MOT-4107) refactor(harness): centralize integration compiler contracts * (MOT-4107) refactor(harness): centralize integration runtime ownership (#533) * (MOT-4107) refactor(harness): centralize integration runtime ownership * (MOT-4107) refactor(harness): decompose integration scenario runner (#534) * fix(harness): resolve(execute) resumes the pre-trigger chain with mutated arguments (#520) * fix(harness): release held calls with hook-mutated arguments A call held by a pre_trigger hook and released with harness::function::resolve {action: "execute"} ran the target with the model's ORIGINAL arguments recovered from the transcript — mutations from hooks that ran before the holder were dropped, and the remaining chain never resumed, breaking the harness.md § function::resolve promise that the pre_trigger chain resumes after the holding hook. - PreTriggerOutcome::Hold now carries the arguments as mutated up to the hold; the loop persists them on the call's checkpoint (held_arguments), so they survive the park durably. Post-trigger holds checkpoint the fully pre-mutated args for the same reason. - The execute release path prefers the checkpointed arguments (transcript recovery remains the fallback for records written before the field existed), resumes the pre_trigger chain AFTER the holder — hooks up to and including it already ran, so re-running would double-apply mutations and re-hold — and handles a resumed Deny/Hold like the loop. The filesystem scope stamp is still re-applied after the resumed chain. - A holder no longer bound at release runs no further hooks: the chain shape changed under the hold and the safe resume point is unknowable. Fixes #506 * test(conformance): unquarantine C-E2E-506 — regression gate for #506 The fix on this branch makes the scenario pass; --scenario all (and the per-PR CI job) now executes it. Verified locally: C-E2E-001, C-E2E-002, C-E2E-506 all pass. * fix(harness): pre-trigger hold keeps its own argument mutations (#519) A pre-trigger hook returning decision:"hold" WITH mutations had them discarded at parse (parse_output mapped "hold" to a data-less outcome), so the approval-gate pattern (hold AND stamp validated context) was impossible. HookOutcome::Hold now carries HookMutations via a shared parse_mutations helper, and run_pre_trigger folds the holding hook's argument rewrite into the effective arguments before parking. Those args ride the call checkpoint added for #506 (fix/506-release-runs-original-args, which this is based on), so the resolve(execute) release runs with the gate's mutation. Unquarantines C-E2E-505, now a regression gate. Fixes #505 * test(integration): retry empty descendant PID reads * (MOT-4107) fix(harness): harden integration runner contracts (#545) * (MOT-4107) refactor(harness): author integration scenarios as Rust builder modules (#546) * (MOT-4107) refactor(harness): author integration scenarios as Rust builder modules Spec revision 2026-07-20 (harness-evaluation): the authored layer is code, not YAML. One builder module per scenario under src/scenarios/ registers in a code registry; the authored structs are shared with the compiler and never serialized, so authored-scenario.v1.json, its golden test, the authored round trip, and the init template CLI are gone. Selection (--scenario id|slug|all), quarantine semantics, validate/render, and all five committed compiled snapshots are unchanged — the snapshots passing unmodified proves the ported modules compile to byte-identical fixtures. - src/scenarios/{builder,mod}.rs: data-only typed builders + registry with compile-and-register-exactly-once tests - five scenario modules ported from scenarios/*/scenario.yaml (306 lines) - fixtures: discovery selects from the registry; loading compiles a registered entry against scenarios/system-prompt.txt - authored-only types drop Serialize/Deserialize/JsonSchema; compiled-shared types (DeadlinesV1, ReleaseV1, FaultKind, invariant parameter structs) keep wire derives - CLI: init removed; validate/render unchanged in behavior - README: builder-module authoring workflow * (MOT-4107) refactor(harness): add recorder, recovery, and release scenario builders Scenario modules repeated the recorder fixture, the recovery-boundary match-override block, and reached into raw ReleaseActionV1. Lift all three into the builder vocabulary: - Function::recorder() for the canonical string-in/recorded-out fixture - Reply::...().recovery_boundary() for durable-outcome-only grading at fault restarts and hook releases - Release::execute() / Release::deliver() for the held-call action Compiled snapshots are byte-identical; no behavior change. * (MOT-4107) test(console): add deterministic Playwright E2E * (MOT-4107) docs(integration): document Console scenario driver * (MOT-4107) refactor(harness): remove integration runner dead code and boilerplate - Fold the DecodedInvariantV1 shadow enum into InvariantKind: the grader dispatches on the kind directly and every variant carried the same untyped parameter map. - Share one teardown spine between run and serve: StackTeardownReport, fail_before_services, and finalize now encode the load-bearing order (service shutdown before process teardown, double early-exit check) in a single place. - Add RunError::setup/runner constructors and a readiness_failed helper, collapsing the repeated map_err and readiness-failure.json ceremony across phases. - Delete unused surface: Deadline::run and timeout_within, five is_default methods, GenerationMatchOverridesV1::is_empty, the write_run_artifact/write_artifact twin bodies, and main.rs's duplicated artifacts-dir preparation. No behavior change; snapshots and schema goldens are untouched. * (MOT-4107) refactor(integration): remove duplicate scenario goldens * (MOT-4107) refactor(integration): simplify scenario contracts * (MOT-4107) chore(integration): checkpoint scenario authoring WIP * (MOT-4107) chore: keep local tooling files untracked * (MOT-4107) refactor(integration): streamline scenario evidence flow * (MOT-4107) refactor(integration): replace declarative expectations with verify over run evidence - Scenarios are now stimulus plus checks: `.trigger(...).model((...))` closes with `.verify(|run| ...)`, where the author writes plain Rust (ensure!/assert!) over the returned RunEvidence dataset — accessors cover assistant_texts, message_counts, calls(alias), all_calls_closed, and subset matching. A scenario without checks does not typecheck. - The floor moved from declared assertions to runner post-conditions: completed turn, exactly-once lifecycle, fully consumed script, and a clean send are enforced before verify runs and fail as "floor: ..." contract failures; the send-flags check disarms itself under serve. - Deleted the declarative layer end to end: grader/, the invariant and expectation types, and the expectation compiler. Ported floor and detection logic lives in scenario/floor.rs and evidence_data.rs with its unit tests. - Result contracts: result.json carries the classification plus the first scrubbed failure message (byte-stable under --repeat); serve-result.json carries the failure plus the raw evidence dataset. The Playwright fixture and the exactly-once spec now check evidence instead of invariant ids. * (MOT-4107) refactor(integration): drop unused per-scenario timeout setters readiness_timeout_ms and teardown_timeout_ms had no scenario callers; only the scenario budget is ever raised (crash-recovery). The fields and their runner defaults are unchanged. * (MOT-4107) fix(integration): accept the turn-completed terminal flag 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. * (MOT-4107) refactor(integration): simplify boot readiness and add the 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 * (MOT-4107) feat(harness): keep multi-turn traces distinct and observable 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. * (MOT-4107) refactor(harness): use traces as integration evidence * (MOT-4107) refactor(harness): replace integration polling with events * (MOT-4107) fix(harness): stabilize integration E2E validation * (MOT-4107) test(console): stabilize integration E2E interactions
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
1 participant
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.
Summary
readiness/, ~850 lines of golden schemas/config-entry diffing/queue-topic checks) with presence-onlydiscovery.rs(~90 lines): wait for the harness function surface plus the workers it calls mid-turn, nothing more. Completion is event-driven — Arm bindsharness::turn-completedonce, Await blocks on delivery instead of pollingharness::statusevery 250ms.harness::sendafter astart.jsonsignal); Playwright now owns the Console process directly (spawnsCONSOLE_BINitself) instead of asking the integration binary to.--console-bin/spawn_console/HTTP-port plumbing are gone from the crate;ObserveReadyV1/ObserveResultV1replace the old serve types. Console e2e specs callstack.start()instead ofstack.trigger('harness::send', ...).llm-router/tests/golden/schemas/*.jsonviainclude_str!instead of a local embedded copy.--repeat/INTEGRATION_REPEAT(CLI, Makefile, CI) — byte-stable scrub stays covered bytests/determinism.rs; booting the full stack twice per scenario was a meta-test, not a contract check.ModelFixtureV1dropspricing,reasoning_efforts,thinking_budgets,input_limit,display_name— fields the compiler always leftNoneand nothing downstream (scripted router or the harness's ownModelparse) ever reads.Recorder::snapshot()drops its unused sequence filter;Client::call_with_timeout,Deadline::at/cap, and the stack config re-exports move topub(crate)/private;readiness_deadlinerenamedsetup_deadline.No scenario behavior change.
Test plan
cargo fmt --check,cargo clippy --all-targets -D warnings— cleancargo test— 96 passing (84 lib + 12 integration)npm run typecheck:e2e(console/web) — cleaniii-sdkreconnect race re-registering a trigger before the harness re-announcesharness::turn-completed, not to the discovery gating in this PR. No pre-change baseline was run in this environment to confirm it isn't a regression — worth an isolated look before de-quarantining 505/506/507.observeround (npm run test:e2e) — not run; would need a Console build + browser install