Skip to content

Commit 2ef5ef9

Browse files
Lykhoydaclaude
andauthored
docs(stories): Maestro-adoption epic — 12 implementation stories (#381)
* docs(plan): #370 keyboard-occlusion guard Phase 2 plan (multi-LLM reviewed + amended) TDD plan for the in-runner live-tap keyboard-occlusion guard (iOS + Android), amended per the Claude+Codex plan review: pure predicate in a dependency-free file, guard at the command handler (not tapAt), Android FLAG_RETRIEVE_INTERACTIVE_WINDOWS, containment predicate, destructive-back safety, off-device tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(keyboard-guard): resolveKeyboardGuard(env) (#370) * feat(keyboard-guard): thread guardKeyboard into iOS+Android tap/longPress commands (#370) * feat(ios-runner): pure KeyboardGuard.shouldDismiss containment predicate (#370) * feat(ios-runner): dismiss occluding keyboard before .tap/.longPress; telemetry (#370) * fix(ios-runner): decode guardKeyboard (var, not let-with-default) so the TS opt-out reaches iOS (#370) * feat(android-runner): pure KeyboardGuard predicate + JVM test (#370) * test(android-runner): explicit Assert imports + exclusive-boundary pins for KeyboardGuard (#370) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(android-runner): FLAG_RETRIEVE_INTERACTIVE_WINDOWS + IME-occlusion guard for live taps (#370) * feat(keyboard-guard): surface meta.keyboardGuard from runner outcomes on press/longpress (#370) * docs(stories): Maestro-adoption epic — 12 implementation stories Comparative deep-dive of mobile-dev-inc/maestro (iOS XCTest runner, core/Android orchestration, MCP server design) vs this plugin, distilled into an epic README + 12 stories with problem/evidence, Maestro reference mechanics, design, implementation steps, acceptance criteria, test plans, and risks: 01 prebuilt runner artifacts 07 native-first action replay (#317) 02 runner protocol versioning 08 token-efficient outputs 03 quiescence bypass 09 Android parity + shared core 04 two-tier settle engine 10 text-input reliability 05 self-healing taps 11 failure evidence + debt 06 native-runner CI + evals 12 tool-surface consolidation Logged as D1289 in the workspace ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(stories): postpone Story 07 (native-first replay); add Story 13 — seamless maestro-runner (Go) integration (D1290) Maintainer call: replay stays on the Go maestro-runner engine. Story 07 keeps the recorded effort assessment and becomes the documented escalation path. Story 13 closes the integration seams instead: version pinning + compat gate, proactive blind-probe (skip the 40s doomed WDA attempt on iOS 26), wider CDP fallback grammar (absorbs #334, incl. optional x-rn hybrid asserts), host-side keyboard seam (B223), single-step iteration mode (absorbs #240). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(stories): renumber postponement ADR ref D1290 -> D1291 (duplicate-ID fix in workspace DECISIONS.md) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e8e8067 commit 2ef5ef9

14 files changed

Lines changed: 1039 additions & 0 deletions
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Story 01 — Prebuilt runner artifacts: kill the 6-minute cold build
2+
3+
**Status:** Proposed (2026-07-02)
4+
**Epic:** [Maestro adoption](README.md)
5+
**Impact:** First-run UX (6 min → <30 s), CI device-smoke enabler (Story 06 Phase B)
6+
**Effort:** M
7+
**Depends on:** — (Story 02's version stamping dovetails but is not a blocker)
8+
9+
## Problem
10+
11+
The first `device_snapshot action=open` on iOS triggers a cold `xcodebuild test` of rn-fast-runner that can take up to 6 minutes (`scripts/cdp-bridge/src/runners/rn-fast-runner-client.ts:164-234`; ready timeout 30 s warm / 360 s cold). Android builds both APKs via `gradlew assembleDebug assembleDebugAndroidTest` + `adb install -r` on first use (`rn-android-runner-client.ts:210-292`). Consequences:
12+
13+
- Worst possible first-session experience for a new user (the plugin looks hung).
14+
- Every environment without Xcode command-line throughput (CI, laptops on battery) pays it again.
15+
- CI cannot cheaply smoke-test the runners because it would have to build them first.
16+
- A plugin version bump that touches runner sources silently invalidates DerivedData and re-triggers the cold path mid-session.
17+
18+
## What Maestro does
19+
20+
Maestro **never runs xcodebuild on a user machine.** The prebuilt XCTest runner (`maestro-driver-iosUITests-Runner.app` zipped + a `.xctestrun` file) ships as JAR resources, is extracted at runtime (`maestro-ios-driver/.../IOSBuildProductsExtractor.kt`), then installed with `xcrun simctl install` and launched via `simctl launch --terminate-running-process` with the port passed as `SIMCTL_CHILD_PORT` (`LocalSimulatorUtils.kt:342-367`). Android ships `maestro-app.apk` + `maestro-server.apk` as resources and `adb install`s them (`AndroidDriver.kt:1165-1214`). The `.xctestrun` + products-dir layout survives zipping because paths inside the xctestrun are `__TESTROOT__`-relative.
21+
22+
Interesting detail we already half-use: `rn-fast-runner-client.ts` scans DerivedData for a prebuilt `.xctestrun` and uses `test-without-building` when one exists (observed live 2026-07-01). This story makes that the *only* hot path and supplies the artifact.
23+
24+
## Design
25+
26+
### Build side (CI, release workflow)
27+
28+
1. **iOS job (macOS runner):**
29+
```bash
30+
xcodebuild build-for-testing \
31+
-project scripts/rn-fast-runner/RnFastRunner/RnFastRunner.xcodeproj \
32+
-scheme RnFastRunner \
33+
-destination 'generic/platform=iOS Simulator' \
34+
-derivedDataPath build/dd \
35+
CODE_SIGNING_ALLOWED=NO ONLY_ACTIVE_ARCH=NO
36+
```
37+
Collect from `build/dd/Build/Products/`: the `Debug-iphonesimulator/` products dir (contains `*-Runner.app`) and the generated `*.xctestrun`. Zip as `rn-fast-runner-<pluginVersion>-sim.zip` preserving relative layout (Maestro proves the layout survives). `ONLY_ACTIVE_ARCH=NO` gives arm64 + x86_64 slices.
38+
2. **Android job (ubuntu runner):**
39+
```bash
40+
(cd scripts/rn-android-runner && ./gradlew assembleDebug assembleDebugAndroidTest)
41+
```
42+
Zip `app-debug.apk` + `app-debug-androidTest.apk` as `rn-android-runner-<pluginVersion>.zip`.
43+
3. **Publish:** attach both zips to the GitHub Release the Version Packages PR creates. Also write `runner-manifest.json` (committed at release time, or attached to the release) with `{version, files: [{name, sha256, bytes}]}`.
44+
45+
### Client side (resolution order)
46+
47+
New `scripts/cdp-bridge/src/runners/runner-artifacts.ts`, used by `ensureRunnerForCommand` (iOS) and `startAndroidRunner`:
48+
49+
1. **Cache hit:** `~/Library/Caches/rn-dev-agent/runners/<pluginVersion>/{ios,android}/` exists and every file matches the manifest SHA-256 → use directly (`test-without-building` with the cached xctestrun; `adb install -r` the cached APKs).
50+
2. **Download:** fetch the release asset for the *exact* plugin version (bounded: 60 s timeout, size cap from manifest, SHA-256 verified before unzip, unzip with path-traversal guard). Progress surfaced via a one-line `meta.note` on the first tool call ("downloading prebuilt runner, ~4 MB").
51+
3. **Fallback — local build:** the current xcodebuild/gradle path, kept intact for offline machines, forks, and `RN_RUNNER_BUILD=local` override. Doctor reports which path was used.
52+
53+
### Version stamping
54+
55+
Inject the plugin version at build time (iOS: `Info.plist` key `RnFastRunnerVersion` via `-xcconfig` or a build phase; Android: `BuildConfig.RUNNER_VERSION`). `/health` returns it (consumed by Story 02's compatibility gate). The runner state file records `{runnerVersion, provenance: 'prebuilt'|'local'}` so `shouldReuseRunner` can refuse a version-mismatched leftover — this closes the live-observed 0.57.1-vs-0.57.3 cache-mirror mismatch class (session 2026-07-01).
56+
57+
## Implementation steps
58+
59+
1. `runner-artifacts.ts` with `resolveRunnerArtifacts(platform, version): {kind: 'cache'|'downloaded'|'build-local', paths}` + unit tests (fake fs + fake fetch; corrupt-zip, bad-checksum, offline, traversal-attempt cases).
60+
2. Release workflow additions (`.github/workflows/`): two build jobs + asset upload; gate on the existing version-sync check so artifact version always equals package version.
61+
3. Wire into `rn-fast-runner-client.ts` (`ensureRunnerForCommand`) and `rn-android-runner-client.ts` (`startAndroidRunner`), before their build-on-demand branches.
62+
4. `/doctor` (`rn-dev-agent:doctor` skill + `cdp_status`): report runner provenance, version, cache path.
63+
5. Docs: README install section note; CHANGELOG.
64+
65+
## Acceptance criteria
66+
67+
- Fresh machine (empty caches), released version: first `device_snapshot action=open` reaches `RN_FAST_RUNNER_LISTENER_READY` in < 30 s + download time; no xcodebuild invoked (assert via absence of the cold-build log marker).
68+
- Checksum mismatch or download failure → automatic fallback to local build with a clear `meta.note`, never a hard failure.
69+
- `RN_RUNNER_BUILD=local` forces the old path.
70+
- Doctor shows `runner: prebuilt v<X> (cache)` vs `local-built`.
71+
72+
## Test plan
73+
74+
- Unit: resolution-order matrix (cache valid / cache corrupt / download ok / download 404 / offline / override env).
75+
- Integration (Story 06 Phase B): nightly job downloads its own artifact, installs on a booted simulator, runs the golden command set.
76+
- Manual: one cold-start timing before/after on a dev machine, recorded in the PR body.
77+
78+
## Risks & open questions
79+
80+
- **xctestrun/Xcode compatibility:** an artifact built with Xcode N must run under users' Xcode N-1/N+1 simulators. The xctestrun format is stable for simulator destinations and Maestro ships one artifact for all users; mitigation: build with the oldest supported Xcode in CI, keep the local-build fallback, and record `xcodeBuildVersion` in the manifest for doctor diagnostics.
81+
- **Release-asset availability for pre-release/dev builds:** dev worktrees fall through to local build by design (provenance makes this visible, not silent).
82+
- **Artifact size:** expected 2–6 MB per platform; manifest size cap prevents surprise growth.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Story 02 — Version the runner wire protocol + relocate `/tmp` state files
2+
3+
**Status:** Proposed (2026-07-02)
4+
**Epic:** [Maestro adoption](README.md)
5+
**Impact:** Prevents an already-observed bug class (bridge/runner skew); removes predictable shared `/tmp` paths
6+
**Effort:** S
7+
**Depends on:**
8+
9+
## Problem
10+
11+
Two related gaps, both confirmed by code inspection:
12+
13+
1. **The runner HTTP `/command` contract is unversioned.** The JS-injection contract *is* versioned (`__HELPERS_VERSION__` with an idempotent re-inject guard, `scripts/cdp-bridge/src/injected-helpers.ts:9-11`), but neither the Swift nor the Kotlin runner carries any `protocolVersion` in requests, responses, or `/health`. A bridge upgrade against a still-running older runner (or vice versa) has no compatibility guard. This exact class fired live on 2026-07-01: the supervisor ran plugin cache 0.57.3 while the mirrored dist was 0.57.1, and nothing detected the skew.
14+
2. **Runner state lives at fixed, project-global `/tmp` paths**`/tmp/rn-fast-runner-state.json` (`rn-fast-runner-client.ts:19`) and `/tmp/rn-android-runner-state.json` (`rn-android-runner-client.ts:20`). Correctness of cross-project reuse relies solely on `shouldReuseRunner` deviceId matching. The session file was already migrated off `/tmp` for a symlink-race CVE (`agent-device-wrapper.ts:56-73`); the runner state files were not.
15+
16+
## What Maestro does
17+
18+
Maestro sidesteps most of this by never reusing a runner across driver versions — the runner artifact is embedded in the CLI release, so client and runner are version-locked by construction, and `simctl launch --terminate-running-process` guarantees a clean single instance (`LocalSimulatorUtils.kt:342-367`). Since we deliberately *do* reuse warm runners (that's a perf feature Maestro lacks), we need the explicit handshake Maestro gets implicitly.
19+
20+
## Design
21+
22+
### Protocol version handshake
23+
24+
- New `scripts/cdp-bridge/src/runners/protocol.ts`:
25+
```ts
26+
export const RUNNER_PROTOCOL_VERSION = 1;
27+
export const MIN_SUPPORTED_RUNNER_PROTOCOL = 1;
28+
```
29+
- Swift (`scripts/rn-fast-runner/RnFastRunner/.../RunnerProtocol.swift`) and Kotlin (`scripts/rn-android-runner/app/.../RunnerProtocol.kt`) mirror the constant. A unit test on the TS side greps all three files and asserts the constants agree (same pattern as the existing version-sync CI guard).
30+
- `/health` response becomes:
31+
```json
32+
{"ok": true, "protocolVersion": 1, "runnerVersion": "0.58.0", "capabilities": ["SCREEN_STATIC"]}
33+
```
34+
(`capabilities` is the negotiation hook Stories 04/05 build on — Maestro's `Capability.FAST_HIERARCHY` pattern, `maestro-client/.../Capability.kt`.)
35+
- Bridge-side gate in the liveness classifiers (`classifyFastRunnerLiveness`, and the Android equivalent from Story 09): a reachable runner whose `protocolVersion` is missing (legacy) or `< MIN_SUPPORTED_RUNNER_PROTOCOL`, or whose `runnerVersion` mismatches the plugin version, is classified **stale** → existing reap-then-restart path handles it automatically. Only if reinstall then fails do we surface a new typed error `RUNNER_PROTOCOL_MISMATCH` (added to `ToolErrorCode` in `types.ts`).
36+
- Every `/command` response body gains a cheap `"v": 1` field as defense-in-depth (checked only when present, so rollout is order-independent).
37+
38+
### State-file relocation
39+
40+
- New location: `~/Library/Application Support/rn-dev-agent/runner-state/ios-<udid>.json` and `android-<serial>.json` (per-device keying replaces the single global file, eliminating cross-project contention on the file itself).
41+
- Reuse the hardened IO already written for the session file — mode 0600, symlink-refusing open (`agent-device-wrapper.ts:40-108`) — extracted into `util/secure-state-file.ts` so session file, runner state, and any future state share one implementation.
42+
- State schema gains `{schemaVersion: 1, protocolVersion, runnerVersion, provenance}` (provenance from Story 01).
43+
- Migration: on first run, if a legacy `/tmp/rn-*-runner-state.json` exists → ignore and best-effort delete. No attempt to migrate contents (a runner restart is cheap and correct).
44+
45+
## Implementation steps
46+
47+
1. `protocol.ts` + Swift/Kotlin mirror constants + tri-file sync test.
48+
2. `/health` enrichment in both runners (Swift handler; Kotlin NanoHTTPD handler) — additive, old bridges ignore unknown fields.
49+
3. `secure-state-file.ts` extraction + both clients switched to per-device paths; delete the `/tmp` constants.
50+
4. Liveness gates: extend `shouldReuseRunner` and the tri-state classifier (`rn-fast-runner-client.ts:379-533`) to require protocol + version match.
51+
5. `RUNNER_PROTOCOL_MISMATCH` in `types.ts` + doctor surfacing.
52+
53+
## Acceptance criteria
54+
55+
- Bridge vN+1 against a live runner vN: first device tool call transparently reaps and reinstalls the runner; `meta.note` records `runner upgraded (protocol/version mismatch)`; no user-visible failure.
56+
- No file under `/tmp` is read or written by either runner client (grep-enforced in a unit test, same style as the gh-374 static-invariant guard, per D1288).
57+
- Two projects driving two different devices concurrently never touch the same state file.
58+
- Legacy `/tmp` files present → ignored, deleted, logged at debug level.
59+
60+
## Test plan
61+
62+
- Unit: constant-sync test; classifier matrix (missing version / older / equal / newer protocol × health-ok / health-timeout); secure-state-file symlink refusal + 0600 assertions; migration branch.
63+
- Integration: extend `gh-264-supervisor-respawn`-style harness with a fake runner serving an old `/health` payload → assert stale classification + restart request.
64+
65+
## Risks & open questions
66+
67+
- **Rollout ordering:** an old runner has no `protocolVersion` → classified stale → restarted. That is the desired behavior, but the first tool call after upgrade pays one runner restart; document in CHANGELOG.
68+
- **Windows/Linux host paths for Android:** use `env-paths`-style resolution (the session file code already resolves the platform-appropriate app-support dir — reuse it).
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Story 03 — Quiescence bypass in rn-fast-runner (FBQuiescence-style swizzle)
2+
3+
**Status:** Proposed (2026-07-02)
4+
**Epic:** [Maestro adoption](README.md)
5+
**Impact:** Eliminates the XCTest idle-wait flake class (Reanimated deadlocks, animation hangs, slow snapshots on busy apps) at the root instead of per-symptom
6+
**Effort:** M (small code, large verification surface)
7+
**Depends on:**
8+
9+
## Problem
10+
11+
XCTest blocks queries, snapshots, and typing until the target app reports "quiesced" — which a React Native app with Reanimated worklets, looping animations, or a busy bridge may *never* report. We currently route around individual symptoms:
12+
13+
- `device_scroll` deadlock on Reanimated → bypassed via HID synthesis (`tools/device-interact.ts:1028-1180`, README troubleshooting).
14+
- "main thread execution timed out" / "Could not detect idle state" on `.type` → treated as success via the runner-timeout shim (`rn-fast-runner-client.ts:731-740`).
15+
- 35 s slow-command timeouts on `snapshot`/`type` as a blunt backstop.
16+
17+
Each is a patch over the same root cause: we let XCTest wait for an idle state RN apps don't reach.
18+
19+
## What Maestro does
20+
21+
Maestro (inheriting WebDriverAgent's approach, BSD-licensed Facebook lineage) method-swizzles `XCUIApplicationProcess -waitForQuiescenceIncludingAnimationsIdle:` at `+load` time into a no-op when quiescence is disabled (`maestro-ios-xctest-runner/MaestroDriverLib/.../XCUIApplicationProcess+FBQuiescence.m:11-72`):
22+
23+
```objc
24+
static void swizzledWaitForQuiescenceIncludingAnimationsIdle(id self, SEL _cmd, BOOL includingAnimations) {
25+
if (![[self fb_shouldWaitForQuiescence] boolValue] || FBConfiguration.waitForIdleTimeout < DBL_EPSILON) {
26+
return; // make XCTest believe the app is idling
27+
}
28+
// else bound the original wait with _XCTSetApplicationStateTimeout(...)
29+
}
30+
```
31+
32+
It probes for both the classic selector and the newer `waitForQuiescenceIncludingAnimationsIdle:isPreEvent:` variant, and throws a clear "driver build not compatible with your OS version" if neither exists (`:59-103`). Settle detection then happens entirely on the host side (Story 04). **This is the single biggest reason Maestro doesn't hang on RN apps.**
33+
34+
## Design
35+
36+
1. **Vendor the swizzle.** New `scripts/rn-fast-runner/RnFastRunner/ThirdParty/FBQuiescence/` containing an adapted `XCUIApplicationProcess+RNQuiescence.m` (+ a minimal config singleton replacing `FBConfiguration`), with upstream attribution in `third_party`-style headers and an entry in `scripts/rn-fast-runner/IMPORT_NOTES.md` (the file already documents imported code provenance). License: WebDriverAgent is BSD; Maestro's adaptation is Apache-2.0 — attribute both.
37+
2. **Selector probing:** exactly Maestro's pattern — try `waitForQuiescenceIncludingAnimationsIdle:`, then the `:isPreEvent:` variant; if neither resolves, log one loud line `RN_FAST_RUNNER_QUIESCENCE_UNAVAILABLE` and continue *without* the bypass (never crash the runner over an optional optimization).
38+
3. **Toggle:** env `SIMCTL_CHILD_RN_QUIESCENCE_BYPASS` read by the runner at startup; TS side threads `RN_QUIESCENCE_BYPASS` (default **on**, opt-out `=0`) — same rollout shape as the keyboard guard (`runners/keyboard-guard.ts`, `RN_KEYBOARD_GUARD=0`).
39+
4. **Capability + telemetry:** `/health.capabilities` gains `"QUIESCENCE_BYPASS"` (Story 02). Runner responses include `meta.quiescenceBypass: true` on the first command after boot so sessions are auditable. Keep the runner-timeout shim in place but count its firings (`meta.runnerTimeoutShim` already exists) — the acceptance signal is that count trending to zero.
40+
5. **Follow-on (do not couple):** once bake-in confirms stability, re-evaluate whether the HID-synthesis scroll special-case can be simplified. Do not remove it in this story — it works.
41+
42+
## Implementation steps
43+
44+
1. Port + adapt the two ObjC files; wire into the runner target; bridging header if needed.
45+
2. Startup log markers (`..._QUIESCENCE_BYPASS_ACTIVE` / `..._UNAVAILABLE`) parsed by the existing chunk parser (`rn-fast-runner-client.ts:22-93`) into runner state.
46+
3. TS env threading + capability surfacing + docs.
47+
4. Verification fixture: add a screen to the test app with an infinite Reanimated loop (`withRepeat(withTiming(...))`) and a visible counter.
48+
49+
## Acceptance criteria
50+
51+
- On the Reanimated fixture screen with bypass ON: `device_snapshot` completes < 2 s; `device_fill` types without the timeout shim firing; `device_scroll` (non-HID path, forced via test flag) does not deadlock.
52+
- With bypass OFF (`RN_QUIESCENCE_BYPASS=0`): current behavior unchanged (shim still fires) — proves the toggle isolates the change.
53+
- On an OS where the private selector is missing: runner boots, logs `..._UNAVAILABLE`, all commands still work (no crash, no bypass).
54+
- No regression across the existing golden flows (TaskWizard fill/press/longpress with keyboard guard).
55+
56+
## Test plan
57+
58+
- Swift unit tests for the selector-probe logic (probe result enum, not the swizzle side effect).
59+
- Live matrix (manual, recorded in PR): iOS 18 sim + iOS 26 sim × bypass on/off × {snapshot, type, scroll, longpress} on the Reanimated fixture + TaskWizard.
60+
- Telemetry check over one week of dogfooding: `runnerTimeoutShim` firing count before/after.
61+
62+
## Risks & open questions
63+
64+
- **Private API drift:** Apple renames the selector in a future Xcode/iOS — mitigated by the probe-both-then-degrade design (Maestro's exact posture) and the loud log marker; doctor can surface `QUIESCENCE_BYPASS` capability absence.
65+
- **Behavioral surprise:** with quiescence gone, XCTest may snapshot mid-animation. That is *by design* — Story 04's settle engine becomes the correctness layer; until Story 04 lands, existing read-back verification (`device_fill`) and settle-reads cover the mutating paths.
66+
- **App Store review concerns do not apply** (runner is a dev-time XCTest bundle, never shipped in the user's app).

0 commit comments

Comments
 (0)