test: replace the hand-typed Maestro fixture with a generated conformance oracle#1289
Conversation
…ance oracle Closes #1274. The old harness (scripts/maestro-conformance*) compared 5 hand-authored flows against a hand-typed transcription of Maestro 2.5.1's command model. It proved parser self-consistency, not conformance: all four bug classes that cost #1217 days of live debugging slipped past it by construction, and it verified no upstream SHAs despite parsing them. Every expected value here is generated from the pinned upstream artifacts. dev.mobile:maestro-orchestra:2.5.1 is published on Maven Central, so the harness runs the real parser and reads the real bytecode — no full Maestro source build. Layer 1 (parser): a Gradle/Kotlin harness drives the pinned YamlCommandReader over a corpus of 42 vendored maestro-test flows (sha256-recorded) plus authored bug-class, coverage, and invalid flows, capturing each parse. The verifier parses each flow with the live engine and classifies it identical / both-reject / we-reject / mismatch / we-are-lenient. Every non-identical outcome must be a declared divergence, so the 17 we-reject entries in expected-divergence.ts are the mechanical parity backlog (assertTrue, clipboard, travel, killApp, and option-level gaps) rather than silent drift. Layer 2 (semantics): ASM reads static-final constants straight from the pinned bytecode without initializing driver classes (MAX_RETRIES_ALLOWED=3, SCREENSHOT_DIFF_THRESHOLD=0.005, ANIMATION_TIMEOUT_MS=15000, erase cap, and the iOS pre-tap gate we intentionally omit), plus the parser-observed 400ms swipe default. Each is cross-checked against MAESTRO_COMPATIBILITY_PRESETS. Layer 3 (differential): scheduled device scenarios. Cross-engine comparison is outcome parity only and says so; finer behavior is asserted engine-side via invariants over replay-timing.ndjson. Bug class 4's detector — a tap must not consume the whole settle budget, since a full-budget tap means the stability loop never latched while the flow still passes — is pure and unit-tested against synthetic traces; only the device run is scheduled-only. regenerate.mjs verifies the pinned jar SHA-256s before trusting output and is byte-deterministic across runs. Layers 1-2 verify in normal CI via node --test with no Java (the job installs deps: unlike the layering guard it copies, the verifier parses with the live engine, which imports the `yaml` package). Acceptance: the four bug classes each have a fixture; every command in SUPPORTED_MAESTRO_COMMAND_NAMES (the parser's own dispatch table, now exported as the single source of truth) is corpus-covered or listed unverified; the five documented deviations are expected-divergence entries.
|
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Current head is not merge-ready because the scheduled layer-3 path cannot currently prove the acceptance claim:\n\n- P1: the differential workflow boots a clean simulator but never installs a target app; the scenarios target and elements such as . It will fail before exercising settle ordering, which is bug class 4's only non-vacuous detector. Install/configure a fixture app with the required surface.\n- P2: layer 3 installs whatever Maestro version the online installer serves, while layers 1-2 and the oracle claim pinned Maestro 2.5.1 behavior. Pin and verify the layer-3 Maestro CLI/artifact too.\n- P3: requires , but this workflow omits it despite declaring . Pass the input as existing workflows do.\n- P2: normal CI trusts the checked-in generated fixture JSON and only compares its embedded upstream metadata with ; it does not bind fixture content to regeneration output. A hand edit to fixture commands/constants can therefore still pass. Add a deterministic provenance/hash check (or equivalent regeneration verification) so "generated from upstream" is enforced, not just documented.\n\nThe per-PR checks are green, but they do not execute layer 3. |
|
Current head is not merge-ready because the scheduled layer-3 path cannot currently prove the acceptance claim:
The per-PR checks are green, but they do not execute layer 3. |
|
Review finding (blocking readiness): the new layer-3 scheduled workflow has not been live-run, and its current setup cannot execute the declared scenarios.
Please wire a deterministic fixture app/screen for every differential scenario, pass the simulator runtime explicitly, and run the workflow once via |
P1 — layer-3 scenarios could never run. They pointed at layer-1 corpus flows,
which exist only to be PARSED: they name a fictional com.example.app and elements
that exist on no device. A device run would have failed before exercising any
runtime behavior, making bug class 4's detector silently vacuous. Layer 3 now has
its own flows under differential/flows/ driving the real fixture app
(examples/test-app, com.callstack.agentdevicelab); the workflow builds and
installs it and hard-fails if it is missing. A test enforces the separation so a
scenario can never point back at the parse corpus.
Nothing else in this repo builds or installs the Expo fixture app, so those steps
are new and unproven. The workflow is therefore dispatch-only: the cron is removed
until a supervised first run proves the path. A nightly job that fails at 05:00
every day teaches nothing.
P2 — layer 3 installed whatever version the online installer served. It now pins
MAESTRO_VERSION from pinned-upstream.json, so layer 3 cannot drift from the
version layers 1-2 claim, and asserts `maestro --version` matches.
P2 — fixture content was not bound to regeneration. CI compared only the embedded
upstream metadata, so a hand edit to a captured command or constant passed: the
transcription failure mode this oracle exists to remove. Two-layer fix, because
per-PR CI must stay Java-free and cannot re-derive:
- Each fixture now carries a contentHash seal that the verifier recomputes, so
editing a capture breaks the build. Tamper-evident, and tested by actually
tampering rather than assuming a hash comparison works.
- New scheduled conformance-regenerate job re-runs the harness against the
pinned jars and fails on any byte difference. Forgery cannot survive a real
re-derivation. This is what makes "generated from upstream" enforced.
P3 — boot-ios-test-simulator requires runtime-version; now passed alongside
preferred-device-name, as the other iOS workflows do.
|
All four addressed in P1 — layer-3 scenarios could never run. You're right, and the root cause was worse than a missing install step: the scenarios pointed at layer-1 corpus flows, which exist only to be parsed. They name a fictional Fixed properly rather than by bolting an install onto fictional flows:
On the cron: I removed it. Nothing else in this repo builds or installs the Expo fixture app — P2 — Maestro CLI pin. Now reads P2 — fixture content not bound to regeneration. This was the sharpest one: "generated from upstream" was enforced by documentation only. Per-PR CI can't re-derive (Java-free is an ADR constraint), so it's two layers:
P3 — Verified: typecheck 0 errors, oxlint, oxfmt, fallow, layering, 161 maestro unit tests, 27 conformance tests (up from 23), fixtures byte-identical across regenerations. I also ran the workflows' two shell one-liners locally (app-id and Maestro-version reads) since those are easy to get wrong silently. Still unproven, stated plainly: the fixture-app build/install and the device path have never executed. That's exactly what the supervised |
|
Current-head follow-up (
Also, current-head iOS Smoke is still running, so this head is not yet CI-complete. |
…path workflow_dispatch cannot run pre-merge (it registers from the default branch), so this temporary push trigger exists only to execute the never-run device path on the PR head and capture evidence. Removed before merge.
|
Re: the layer-3 readiness finding — first two bullets were already fixed in Bullet 1 — Bullet 2 — no app for the corpus Bullet 3 — live run + evidence: in progress, with one wrinkle worth recording.
To honour the intent rather than the letter, I pushed a temporary, clearly-marked Run: https://github.com/callstack/agent-device/actions/runs/29495850346 (queued — macOS runners are backed up). I'll report the outcome and attach the report/trace either way. If the Expo build breaks, that's precisely the information this run exists to produce, and I'd rather find it here than at 05:00 on a cron. Your framing is right and I'm not going to argue it: until that run succeeds, layer 3 is a unit-tested runner shape, not an app-observable oracle. Layers 1-2 stand on their own evidence (35 identical / 7 both-reject / 17 declared divergences / 0 mismatch, fixtures byte-identical across regenerations, seal proven by actual tampering) — but bug class 4's detector is only real once this goes green. |
First live run of the device path failed at the very first step: ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. CI implies --frozen-lockfile and the fixture app's lockfile is out of sync with its package.json overrides. No CI job has ever built examples/test-app, so that drift was never surfaced.
…rity overrides) The first live run of the layer-3 device path failed at ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, and the cause is a real latent bug rather than a stale lockfile. #649 moved the fixture app's `overrides` into examples/test-app/pnpm-workspace.yaml precisely because pnpm only honors overrides from a workspace root — they pin transitive deps (ws, brace-expansion, xmldom, postcss, uuid, shell-quote) to versions that clear Dependabot alerts. But `test-app:install` passes --ignore-workspace, which ignores that very file, so the overrides are dropped and no longer match the lockfile that has them baked in. It goes unnoticed locally because interactive installs are not frozen, and no CI job has ever installed this app. Dropping --ignore-workspace makes examples/test-app resolve as its own workspace root (it has its own pnpm-workspace.yaml and is not a member of the repo-root workspace), so the overrides apply and a frozen install succeeds. Verified both directions locally: with the flag + --frozen-lockfile reproduces the CI failure; without it, a frozen install completes and the lockfile's overrides stay intact. Note the workaround this replaces would have been actively harmful: installing with --no-frozen-lockfile resolves the mismatch by regenerating the lockfile WITHOUT the overrides, silently reverting the app to the vulnerable transitive versions #649 pinned away.
|
Current-head re-review (
A green live run proves the workflow executes; it does not make these two detectors revert-sensitive. |
…tro version Run 3 (29497919702) got the whole device path working: Expo build (30m), app installed, simctl check, pinned Maestro CLI install. Only the version ASSERTION failed — `maestro --version` prints an analytics banner before the version, and `tr -d '[:space:]'` mashed banner+version into one string. The CLI was correctly 2.5.1. Match the semver line instead, and set MAESTRO_CLI_NO_ANALYTICS (CI should not phone home). Verified the parse against the exact CI output: banner and clean forms both yield 2.5.1, wrong/empty still fail. tap-retry-if-no-change was vacuous: it tapped a navigating control, so the first tap always succeeded and retryIfNoChange never ran — it passed while proving nothing. It now taps the app's non-interactive title so the screen cannot change and the retry path is forced, and asserts tapRetries >= 1 from the trace (MaestroRuntimeMetrics already records it per step). A new metricAtLeast invariant kind carries the assertion; a test reproduces the old vacuity. percent-swipe no longer claims bug class 1. Truncation vs rounding is a <=1px delta that no app-observable device outcome can distinguish, so pass/pass could never back that claim up. The runtime half is instead pinned exactly by a pure unit test of resolveMaestroCoordinate (it short-circuits on a known viewport, so no device is needed) — verified to catch the regression by flipping trunc->round, which turns 3 of 6 tests red. Truncation had no test coverage at all before this. A test now forbids any device scenario from re-claiming bug class 1.
|
Current-head re-review ( One readiness blocker remains: |
…3 flows Run 4 (29500262301) reached the differential itself — build, install, simctl check and the pinned Maestro 2.5.1 verification all passed — and surfaced two real bugs, both mine: 1. The runner invoked `agent-device test <flow>` without --maestro, so every scenario failed with "test does not support this file type". The repo's own scripts/run-test-app-maestro-suite.mjs passes it; the flag is what routes a .yaml through the Maestro compat engine. 2. settle-after-tap and percent-swipe assumed home-open-form is on screen at launch. It is not: real Maestro reported "Element not found: home-open-form", and the app's own helper flow scrolls it into view first. settle-after-tap now scrolls before tapping, mirroring that helper; percent-swipe no longer navigates at all and swipes the scrollable home screen, so it tests the conversion and nothing else. The remaining two flows already reported maestro=pass, so only the agent-device invocation was wrong for those. Note the settle invariant correctly reported "no-data: no completed tapOn steps" and FAILED rather than passing — a detector that cannot run is a failure, as intended.
|
Current-head re-review (35842ac): the latest fixes match the failures from run 29500262301. The differential runner now invokes agent-device with --maestro, settle-after-tap uses the fixture app’s established scrollUntilVisible path, and percent-swipe no longer depends on an off-screen navigation control. Independent Terra review found no additional code defect in the revised Layer 3 flows or the Layer 1/2 oracle. One P1 readiness blocker remains: .github/workflows/conformance-differential.yml is still dispatch-only, retains the temporary branch push trigger marked REMOVE BEFORE MERGE, and has no schedule, while #1274 requires scheduled Layer 3 execution. The fresh live differential for this head is still building the fixture app. Require a successful report/trace from that run, then remove the temporary trigger and add the schedule before merge. The transient provider-integration failure has already passed on rerun; current required CI is green except iOS Smoke and the live differential, which are still running. I am not applying ready-for-human on this head. |
|
Agreed on the verdict — not ready. Status on each item. Required CI red (
|
Layer 3 ran both engines for the first time (29504440599) and immediately found a
real engine bug. Blocking the measurement instrument on repairing what it just
measured inverts the dependency, so layer 3 now gets the contract layer 1 already
had: every divergence is a decision on the record.
Adds `knownDivergence: { reason, tracking }` to the scenario type — the layer-3
twin of FLOW_DIVERGENCES. A declared divergence keeps the run green; only
UNDECLARED ones fail. Two rules stop that from rotting, both enforced
mechanically rather than by prose discipline:
- `tracking` is required and must be a real issue URL (run.test.ts), because a
declaration with nothing behind it is how "temporarily expected" becomes
permanent without anyone deciding to.
- a stale declaration FAILS: if a declared-divergent scenario starts passing,
the run goes red until the declaration is removed. The fix PR must delete it,
and the differential then enforces the gap stays closed — the oracle is the
acceptance test for its own findings.
Declared:
- settle-after-tap -> #1299. Our scrollUntilVisible times out finding
home-open-form where Maestro 2.5.1 scrolls to it and passes. Real engine
correctness bug in an advertised command, found by this differential. Blocks
bug class 4's device detector until fixed.
- tap-retry-if-no-change -> #1300. The invariant caught the scenario being
vacuous: both engines pass but tapRetries was 0, so retryIfNoChange never
ran. Needs an inert fixture control; a scenario defect, not an engine one.
Proven green on both engines and enforced now: percent-swipe,
optional-warned-not-failed — the latter is real device-verified warned-vs-failed
parity.
With declarations in place the differential is green, so the schedule goes in
(cron 05:00) per #1274. A green run still prints what it is not proving.
Run 29510020718 fired the stale-declaration guard on its first outing and caught my own mistake. tap-retry-if-no-change measured tapRetries=0 in run 29504440599 and tapRetries=1 in 29510020718 — same flow, same commit. So it is not vacuous as #1300 originally claimed: it is NON-DETERMINISTIC. The tap sometimes holds the hierarchy signature still and sometimes does not, because the fixture home screen carries live content. That exposes a real limit of the mechanism added in the previous commit: knownDivergence assumes the divergence REPRODUCES. A declared-but-flaky scenario flips between known-divergence (green) and stale-declaration (red) at random — a coin-flip scheduled job, which is worse than no scenario because it teaches people to ignore the differential. So the scenario is parked, not declared. The flow and the tapRetries invariant stay implemented and unit-tested, so the fix PR only re-adds the scenario once the fixture has an inert control. retryIfNoChange therefore has NO device coverage right now — tracked in #1300 and stated plainly rather than disguised by a green run. A test keeps it out of the active set until then. #1300 updated with the corrected diagnosis and both runs' evidence. Active differential: settle-after-tap (declared divergence, #1299), percent-swipe and optional-warned-not-failed (both enforced, pass/pass on real devices).
|
Current-head re-review (
Evidence note: the workflow uploads the computed report but not the |
… failure P1 from re-review, and a real flaw: the code did not do what its own comment claimed. runScenario() collapsed every unexpected outcome and every invariant failure into `misbehaved`, then turned ANY of them green if the scenario carried a declaration. So while the #1299 scrollUntilVisible waiver is open, upstream Maestro could start failing too — or a different invariant could break — and the scheduled job would still report known-divergence and pass. A waiver for one bug was silently amnesty for the next. That is the exact failure this oracle exists to prevent, committed one commit after building the guard against it. knownDivergence now requires an `expected` signature: both engines' outcomes plus each declared invariant's status. The runner matches it exactly — - matches -> known-divergence (green, tracked) - misbehaves differently -> failed (red): not the failure the waiver covers - stops misbehaving -> stale-declaration (red): remove the declaration #1299's signature pins what runs 29504440599/29510020718 actually observed: maestro=pass, agent-device=fail, settle invariant no-data. Tests prove unrelated failures stay red under an open waiver: upstream also failing, our engine unexpectedly passing, a different invariant status, and a new invariant appearing are each NOT covered. A signature where both engines pass is rejected outright as describing no divergence. Also retains replay-timing.ndjson as a run artifact (review evidence note): the invariants are computed from that trace, so a report saying "tapRetries was 0" cannot be audited once the runner is gone without it.
The differential job took ~30 minutes, of which 1331s (22 min, 79%) was building the Expo fixture app and only 347s was the differential itself — rebuilt from scratch on every run for an app that changes almost never. Cache the built .app, keyed on everything that can change the binary: the app's sources, native config, dependency graph, the build step itself, the iOS runtime, and the Xcode version. Mirrors the existing setup-apple-replay prebuilt-runner cache (same action pin, same Xcode-key + source-hash shape). On a hit the build is skipped entirely and the bundle is installed straight onto the booted simulator (~seconds), taking the job to roughly 8 minutes. On a miss it falls back to exactly the previous behaviour and repopulates, so the worst case is unchanged. The existing simctl verification still gates both paths, so a bad cache cannot produce a vacuous green: if the app is not installed, the job fails loudly rather than running scenarios against nothing. Note the first run after this lands is necessarily a miss.
|
Exact-head re-review ( One merge blocker remains: the explicitly temporary branch-only |
… app The fixture-app build + cache was inline in the differential workflow, so nothing else could reach it. Extracted to a composite action mirroring setup-apple-replay, because the capability is what #320 has been missing: it wants replay coverage moved off Apple system apps onto a controlled fixture with stable ids, and that fixture (examples/test-app) already exists — CI just had no way to build and install it. The cache is genuinely shared. GitHub caches are per-repository and readable across workflows, and a run restores from its own branch or the default branch, so once a run on main populates it every workflow gets the hit and only the first one pays the ~22 minutes. The key is computed inside the action from a fixed input list and deliberately contains nothing caller-specific — folding a caller's workflow path into it would silently unshare the cache. Also removes a duplication risk: the action reads the bundle id from the built app's Info.plist rather than hardcoding it, so it cannot drift from what was actually built, and it fails loudly if the app is not installed. The conformance workflow keeps its own narrower assertion — that the installed id is the one its scenarios target — since that is its concern, not the action's. Usage: - uses: ./.github/actions/setup-fixture-app with: runtime-version: ${{ env.IOS_RUNTIME_VERSION }} # outputs: app-path, app-id, cache-hit
Run 29519848340 on this head executed both engines against the real fixture app and came back green, so the trigger that existed only to prove the never-run device path has done its job. Merged config is now cron (05:00) + workflow_dispatch, as required by #1274. known-divergence settle-after-tap maestro=pass agent-device=fail (#1299) ok percent-swipe maestro=pass agent-device=pass ok optional-warned-not-failed maestro=pass agent-device=pass This commit will not itself trigger a run: GitHub evaluates triggers at the pushed commit, and the push trigger is gone in it.
|
Both blockers closed. Evidence below. Layer 3 ran green on this head, against real devicesRun 29519848340 — both engines, real fixture app (
Temporary trigger removed (
|
1 participant
Closes #1274.
Why
The old harness (
scripts/maestro-conformance*) compared 5 hand-authored flows against a hand-typed transcription of Maestro 2.5.1's command model. It proved parser self-consistency, not conformance — all four bug classes that cost #1217 days of live debugging slipped past it by construction, and it parsed upstream SHAs without ever verifying them.Every expected value here is generated from the pinned upstream artifacts. The enabler:
dev.mobile:maestro-orchestra:2.5.1(and-models/-client) are published on Maven Central, so the harness runs the real parser and reads the real bytecode — no full Maestro source build.The three layers
YamlCommandReaderover the corpusnode --test, per-PRnode --test, per-PRagent-device testLayer 1 drives the pinned parser over 42 vendored
maestro-testflows (sha256-recorded provenance) plus authored bug-class, coverage, and invalid flows. The verifier parses each with the live engine and classifies itidentical / both-reject / we-reject / mismatch / we-are-lenient. Any non-identical outcome must be a declared divergence, soexpected-divergence.tsis the mechanical parity backlog (17 entries) instead of silent drift.Layer 2 reads
static finalconstants straight from the pinned bytecode without initializing driver classes (MAX_RETRIES_ALLOWED=3,SCREENSHOT_DIFF_THRESHOLD=0.005,ANIMATION_TIMEOUT_MS=15000, erase cap, and the iOS pre-tap gate we intentionally omit), plus the parser-observed 400ms swipe default — each cross-checked againstMAESTRO_COMPATIBILITY_PRESETS.Layer 3 is honest about what it proves: cross-engine comparison is outcome parity only, and the scenario fields say so. Finer behavior is asserted engine-side via invariants over
replay-timing.ndjson.Current state: 35 identical / 7 both-reject / 17 we-reject / 0 mismatch / 0 coverage gaps.
Acceptance (#1274)
both-rejectat parse. 3 (retry cap) is the layer-2MAX_RETRIES_ALLOWEDcross-check. 4 (settle ordering) has no reflectable constant, so layer 3 is its only home — its detector is "a tap must not consume the entire settle budget" (a ~2093ms tap against 2000ms means the loop never latched while the flow still passes, which outcome parity cannot see). The evaluator is pure and unit-tested against synthetic traces; only the device run is scheduled-only.SUPPORTED_MAESTRO_COMMAND_NAMES— now exported from the parser's own dispatch table as the single source of truth.optional, and the omitted iOS 3s pre-tap gate.Wiring
regenerate.mjsverifies the pinned jar SHA-256s before trusting output (the gate the old harness faked), and is byte-deterministic across runs. Manual, pin-bump only.maestro-conformanceCI job. Note it installs deps, unlike the layering guard it otherwise mirrors: the verifier parses with the live engine, which imports theyamlpackage.conformance-differentialworkflow.distributionSha256Sum— this tool refuses unverified upstream jars, so its own toolchain gets the same treatment.Things worth a reviewer's eye
we-rejectentries are a backlog, not a regression — each names the unsupported command/option.Verification
typecheck (0 errors) · oxlint · oxfmt · layering guard · both fallow checks · 161 maestro unit tests · 23 conformance tests · fixtures byte-identical across regenerations.