test(android): add comprehensive emulator E2E coverage

[thymikee]

Summary

Adds catalog-complete Android emulator coverage: every public command is assigned to either a live release-APK scenario, an executable repository contract, or an explicit capability denial.

The live suite reuses the cached test-app APK and runs focused inventory/install, automation/system, form input, real-keyboard dismissal, and capture/close journeys. Runtime evidence is declared structurally and checked per scenario; timing remains diagnostic evidence rather than a pass/fail threshold.

The iOS and Android suites now share one live-device runtime, assertion layer, and coverage reporter. CI builds the CLI before reserving the emulator, emits the exact report path and timings from the runner, and removes the redundant throwaway native APK build.

The generated report includes the manifest-derived classification rollup: 28 live, 22 command-contract, 3 capability-denial, and 0 gap owners across 53 public commands.

This comprehensive live scope is the project-coordinator amendment now recorded on #1426. It supersedes that issue's original request to classify only existing Android evidence after the cached release test-app APK made focused live journeys cheap to reuse.

Also fixes three Android defects exposed by the suite:

• shell-quotes deep links so query separators reach the app intact
• lets native-alert snapshots use the normal bounded helper settle policy
• lets an acknowledged snapshot-helper shutdown release UiAutomation before force termination and force-stops the device-side instrumentation runtime before reuse, preventing the next accessibility capture from wedging

Validation

• pnpm check
• Android snapshot, persistent-helper lifecycle, touch-helper, and deep-link lifecycle unit suites
• Android/iOS coverage contracts and trusted fixture-artifact tests
• adversarial maintainability review with no remaining changed-file findings

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.88 MB	1.88 MB	+524 B
JS gzip	604.0 kB	604.1 kB	+183 B
npm tarball	722.5 kB	722.7 kB	+190 B
npm unpacked	2.53 MB	2.53 MB	+590 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.3 ms	27.4 ms	+0.1 ms
CLI --help	56.9 ms	57.0 ms	+0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/device.js	+538 B	+196 B

[thymikee]

Thermo-nuclear code quality review

The coverage idea here is good — catalog-complete ownership with runtime evidence is the right model. The implementation is a second copy of a model we already have, plus a static checker that greps source text, plus a wall-clock assertion in CI. I'd hold this until 1–3 are resolved.

Nothing in this diff crosses the 1k-line boundary. src/platforms/android/app-lifecycle.ts sits at 909 and this PR adds ~2 lines — fine for now, but the next Android lifecycle feature should split it rather than push it over.

---

1. ~450 lines of harness are a verbatim fork of ios-simulator-e2e/ — blocker

live-harness.ts, live-coverage-report.ts, scenarios.ts, and live-assertions.ts are the iOS files with the names swapped. runScenario, runStep, verifyCommand, verifyBehavior, cleanupSession, writeStepHistory, withCommonFlags, withJson, requiredEnv, stringValue, assertCoverageComplete, writeCoverageReport, liveBehaviorsForScenario, bind*Scenarios — all duplicated.

Worse, the copy has already drifted on day one: runStep silently dropped allowFailure/expectFailure, sessionOpen now flips before the command instead of after a successful exit, verifyCommand's failure message was reworded, assertScenarioCommandsVerified became assertNewEvidence. Two copies that diverged before the PR even merged will keep diverging, and every future harness fix will land in one of them.

The genuinely Android-specific surface is tiny: --serial vs --udid, the artifact root, and the two manifests. Everything else is identical.

Judo: extract test/integration/live-e2e/ and parameterize by a platform descriptor —

type LivePlatform<BehaviorId extends string> = {
  artifactRoot: string;                    // 'test/artifacts/android-emulator'
  deviceFlags: () => string[];             // ['--platform','android','--serial', serial]
  sessionPrefix: string;                   // 'android-e2e'
  behaviors: Record<BehaviorId, BehaviorEntry>;
  coverage: Record<PublicCommand, CoverageEntry>;
  scenarios: readonly Scenario[];
};

smoke-ios-simulator-coverage.test.ts and smoke-android-emulator-coverage.test.ts are ~80% the same too and should become one suite run over both descriptors. Done this way, this PR deletes net lines instead of adding 1299. If the extraction is too much to bundle, land it as a prep PR — but please don't merge a second copy.

2. The static coverage checker greps source code — blocker

new RegExp(`verifyCommand\\(\\s*context,\\s*(?:C\\.${commandKey}|['"]${command}['"])`)

This reads scenario files as strings and regexes for a call expression. It breaks on reformatting, on renaming the C alias, on extracting a helper, on calling it in a loop. Note the alternation (?:C.wait|'wait') exists only because live-assertions.ts writes verifyCommand(context, 'wait', …) while every other site writes C.wait — the test was widened to accommodate inconsistent code rather than the code being fixed. It also forces the hardcoded source: path in scenarios.ts, which is already off: smoke:capture-close points at live-runner.ts, the runner, not a scenario module.

And it's redundant — runScenario → assertNewEvidence already enforces exactly this at runtime, precisely, with no regexes.

Judo: delete source, delete the two regex tests, and make the static guarantee structural by declaring owned commands as data:

export const inventoryInstall = {
  id: 'smoke:inventory-install',
  commands: [C.devices, C.capabilities, C.install, C.apps, C.doctor],
  run: assertInventoryAndInstall,
};

The static test then becomes one assert.deepEqual against the manifest — no filesystem reads, no regexes, no path strings to drift.

3. Wall-clock assertion buried in the coverage assertion — blocker

MAX_ANDROID_EMULATOR_SCENARIO_BODY_MS = 4 * 60 * 1000 is asserted inside assertCoverageComplete. Two unrelated concerns in one function, and a hard wall-clock assert on shared CI emulator runners is flaky by construction: a slow runner turns a functionally-green run red with zero diagnostic value. The report already records per-scenario timings — report the number, don't assert it. If you want regression protection, compare against recorded timings out-of-band.

4. Two src behavior changes ride along in a test(android): PR, and one leaves dead code

• alert.ts dropping helperWaitForIdleTimeoutMs: 0 removed the only production caller of that option. After this PR, AndroidSnapshotOptions.helperWaitForIdleTimeoutMs is a knob nothing sets outside snapshot.test.ts. Delete the field and the ?? fallback at snapshot.ts:295 — that's a real deletion, and it makes the new unit test moot.
• alert.test.ts is a change-detector: it deep-equals the arguments handed to a mocked snapshotAndroid and calls that "retains the normal bounded helper settle policy". It asserts an argument shape, not a settle policy. (Separately, vi.restoreAllMocks() in afterEach does not clear a vi.mock factory's call log — mock.calls[0] will read stale calls the moment a second test is added; you'd want clearAllMocks.)
• The deep-link quoting fix is a genuine bug fix — & in a deep link was being re-tokenised by the device shell — and the implementation reusing quoteAndroidShellArg is exactly right. But it's invisible under a test(android) title. Please split it out so it's reviewable and greppable on its own.

5. android-fixture-cache-smoke.sh is now dead, but still asserted against

The workflow no longer invokes it and nothing else references it, yet trusted-fixture-artifact.test.mjs still does readFileSync('test/scripts/android-fixture-cache-smoke.sh') and asserts /snapshot -i .*--json > "$SNAPSHOT_PATH"/ on it — under a test you renamed to "Android smoke consumes the restored APK through catalog fixture E2E". The test now guards a fossil. Either delete the script along with those assertions, or keep running it.

6. The workflow embeds a JS program inside a YAML string inside a shell script

COVERAGE_REPORT="$(find test/artifacts/android-emulator -name coverage-report.json -print | tail -n 1)"
node --input-type=module -e 'import fs from "node:fs"; …'

find | tail -n 1 picks by traversal order, not by newest — with more than one run directory it silently reports the wrong run's timings. And the process that wrote the report already knows artifactDir.

Judo: delete both lines and have writeCoverageReport (or the tail of runAndroidEmulatorE2E) console.log the timing summary. The data is already in hand, CI already captures stdout, and the YAML collapses back to a single node --test line. Same argument for the inline pnpm build / pnpm clean:daemon — those belong in a checked-in script or a prior step, not in emulator-runner's script:.

---

Smaller, still structural

• bindAndroidEmulatorScenarios<Context> earns nothing. The generic is only ever instantiated with LiveContext; the ScenarioRunnerKey union plus Record<key, fn> lookup exists solely to keep two lists in sync, which a plain {id, run} array does for free. live-runner.ts has no import side effects, so the coverage test can import the array directly and scenarios.ts disappears entirely.
• Vestigial fields from the copy. BehaviorCoverageEntry.level has exactly one possible value — it drives two .filter(e => e.level === 'live') calls and one assert.equal(entry.level, 'live'). Delete the field, three checks vanish. Likewise known-gap + trackingIssue in the coverage manifest has zero Android entries but forces branches in liveCommandsForScenario, assertCoverageComplete, and two tests — drop it until Android has a real gap.
• owner: string | RepositoryEvidence is the reason the coverage test needs typeof entry.owner === 'string'. Name the fields for what they are (scenario: string on live entries, evidence: {path, test} on the rest) and the narrowing disappears.
• requireAndroidResourceId casts three times against any. CliJsonResult.json is any, so (node as { rect?: unknown }).rect, rect as {…}, and (node as { identifier: string }).identifier buy zero safety while reading as though they do. Every scenario reads json.data.* blind. Parse once against the real snapshot node type from src/ and let scenarios consume typed data.
• Redundant artifacts. step-history.json is rewritten on every step, and coverage-report.json now embeds the same stepHistory array and is rewritten after every scenario plus twice more at the end. Pick one file.
• assertJsonContains substring-matches serialized JSON. assertJsonContains(capabilities, 'click') passes on any response containing those five characters anywhere. Inherited from iOS, so not this PR's sin — but the manifest leans on it for C.capabilities and C.apps ownership. Tighten where a real field exists.
• examples/test-app: reordering the Window StateRow to the top of the Runtime state card is a product-fixture change made to suit a test. Probably fine, but call it out in the PR body — right now it reads as an unexplained drive-by.

---

Not approving. 1–3 are the ones I'd treat as blocking: merging a forked harness locks in the duplication, the regex-over-source checker is brittle magic standing in for a runtime check that already exists, and the wall-clock assert will produce red builds that mean nothing. 4–6 are cheap deletions that make the diff smaller rather than bigger.

---

Generated by Claude Code

[thymikee]

Re-review at 50f07fd is not ready.

The Form→Home follow-up plausibly corrects the prior Android failure by asserting the actual Agent Device Tester landmark, but the exact-head Android run is still in progress, so the full journey plus the later replay/snapshot-helper steps are not yet proven.

Three structural blockers remain unchanged:

• P2 — duplicated live harness. Android still carries a near-copy of the iOS harness/report/scenario machinery; the latest commits only touch the form scenario and manifest. Extract the shared live-E2E runtime and keep platform differences declarative.
• P2 — source-regex “coverage.” smoke-android-emulator-coverage.test.ts still reads implementation files and regex-matches verifyCommand/verifyBehavior. Make scenario ownership declarative and verify runtime evidence structurally rather than grepping source text.
• P2 — wall-clock as functional correctness. assertCoverageComplete still fails the device journey at four minutes. Keep per-scenario timing as reportable evidence, but do not turn shared-runner slowness into a product failure.

No ready-for-human label until these are tightened and the complete Android lane passes on this exact head.

[thymikee]

Re-review @ 7a8d56c

Three new commits since d8de04d. They're all live-scenario robustness fixes, and they're good ones on the merits:

• 25a9562 — the diff assertion now proves an actual added/removed line instead of additions + removals > 0. That's a real upgrade from a tautology to evidence.
• 50f07fd — restoration waits on id="automation-open-sheet" instead of heading text. More specific, less racy.
• 7a8d56c — separating the test IME from the emulator IME is the right call; asserting keyboard status before the dismissal screenshots closes a genuine hole.

I checked the new flags against src/ — --no-test-ime (flag-definitions-target.ts:255), --settle on click, and keyboard status (commands/system/index.ts:49) all exist. No typos waiting to fire on the emulator.

But none of the six structural findings were touched. I re-verified each against 7a8d56c:

#	Finding	Status
1	live-harness.ts fork of ios-simulator-e2e/	byte-identical to d8de04d
2	new RegExp(\verifyCommand\(...`)` source-grep	present, smoke-android-emulator-coverage.test.ts:62,103
3	4-minute wall-clock assert	present, live-coverage-report.ts:13,35
4	helperWaitForIdleTimeoutMs with zero prod callers	present, snapshot.ts:61,295
5	dead android-fixture-cache-smoke.sh still asserted	present, trusted-fixture-artifact.test.mjs:79
6	find | tail -n 1 + inline node -e in YAML	present, android.yml:72-73

The diff went 1299 → 1357 lines. Direction of travel is more lines, not fewer.

---

New in these commits

A. live-form-scenario.ts went 73 → 127 lines and now repeats a five-step block three times.

Lines 18-19, 54-62, and 100-108 are all variations of:

runStep(context, '…', ['close']);
runStep(context, '…', ['open', context.appId, /* --no-test-ime | --relaunch */]);
assertWaitText(context, 'Agent Device Tester');
runStep(context, 'open form tab …', ['click', 'label="Form"']);
assertWaitText(context, 'Checkout form');

Two full session teardown/reopen/re-navigate cycles pasted inline. This is the direct cost of putting three IME configurations into one linear function — exactly the "narrow edge-case handling in the middle of an already busy function" pattern.

IME mode is a scenario dimension, not an inline step. The manifest already models per-scenario ownership, so use it: split into smoke:form-input (test IME — owns fill/type/focus) and smoke:keyboard-ime (emulator IME — owns keyboard and the safe-keyboard-dismissal behavior). Each scenario opens its own session once, runScenario already handles per-scenario evidence, and the two mid-scenario close/open pairs disappear entirely. If you'd rather not split, at minimum extract openFormTab(context, { ime }) so three copies become three one-line calls.

B. The back-then-re-enter dance exists only to feed diff a transition.

C.diff is owned by smoke:form-input, but its evidence is now a navigation transition (Form → Home) — so the scenario navigates away from the form and then has to navigate back in. That's what makes the first restart block necessary.

Judo: move the diff evidence to smoke:automation-system, which already walks Catalog → Settings → Automation lab and has a transition sitting right there for free. smoke:form-input then becomes a clean linear form journey: no back, no re-entry, one fewer restart block. The manifest row is a one-line owner change.

C. The new assertion re-declares a type that already exists in src/.

diff.json.data.lines as { kind?: unknown; text?: unknown }[]

src/snapshot/snapshot-diff.ts:9 exports SnapshotDiffLine = { kind: 'added' | 'removed' | 'unchanged'; text: string; ref?: string }, and src/contracts/diff.ts:13 already types the wire payload as lines: SnapshotDiffLine[]. Import the contract rather than re-describing it with unknown fields — this is the same untyped-payload problem as requireAndroidResourceId, now spreading to a second call site.

D. Two nine-line blocks that differ in two string literals.

lines.some((line) => line.kind === 'removed' && typeof line.text === 'string' && line.text.includes('Checkout form'))
lines.some((line) => line.kind === 'added'   && typeof line.text === 'string' && line.text.includes('Agent Device Tester'))

That's a missing helper, and it belongs in live-assertions.ts beside the other shared assertions:

assertDiffLine(diff, 'removed', 'Checkout form');
assertDiffLine(diff, 'added', 'Agent Device Tester');

18 lines → 2, and with SnapshotDiffLine imported per (C) the typeof line.text === 'string' guard goes away too.

E. Minor. 50f07fd swapped assertWaitText for a raw runStep(…, ['wait', …]), which no longer credits C.wait. Harmless — wait is credited elsewhere in the same scenario — but it's one more instance of the raw-runStep-vs-helper inconsistency that forced the §2 regex to carry a (?:C.wait|'wait') alternation in the first place.

---

Still not approving. The assertion quality genuinely improved in these three commits — I'd keep all of that behavior. What hasn't moved is the structure, and (A) added a fresh copy-paste triple on top of it. My ask is unchanged and unblocked by any of this: extract the shared harness (§1), delete the source-grep checker (§2), and drop the wall-clock assert (§3). §§4-6 are pure deletions that shrink the diff.

---

Generated by Claude Code

[thymikee]

Re-review at 19a1f552: not ready.

The good news: exact-head CI is green and the Android live fixture journey now completed in 200.9s with all four scenarios, declared command/behavior evidence, and the expected Home/Recents and keyboard state artifacts. The prior missing-smoke-evidence concern is resolved.

Three P2 structural blockers remain unchanged:

• test/integration/android-emulator-e2e/ is still a platform-renamed fork of the iOS live harness. Extract the shared live-E2E runtime and keep platform/scenario differences declarative.
• smoke-android-emulator-coverage.test.ts still reads source files and regex-matches verifyCommand/behavior calls. Declare ownership as data and validate runtime evidence structurally; source spelling is not coverage.
• live-coverage-report.ts still turns a four-minute wall clock into functional correctness. This green run used 200,878ms, leaving ~39s of host/emulator headroom. Keep timing as evidence, not admission.

The only exact-head delta, find ... -print -quit in android.yml, still selects an arbitrary report if multiple run directories exist; have the runner emit its own report path.

No ready-for-human until the three structural blockers are fixed.

[thymikee]

Re-review at d835e27: the four prior architecture blockers are fixed, but the PR is still not ready because exact-head Android Smoke exposes a deterministic test defect.

Resolved:

• Android and iOS now share test/integration/live-device-e2e/ runtime/evidence/reporting
• coverage ownership is declarative; source regex checks are gone
• wall-clock timing is diagnostic only
• the runner emits the exact report path; CI no longer searches arbitrarily

Remaining failure: job 30457383552/90594354940 booted the emulator and ran 147.1s, then live-automation-scenario.ts:169 required a removed diff line containing Automation lab. The real diff snapshot -i output removed the Automation controls but contained no such line, so the assertion is false for the actual Android accessibility tree. This is not an infrastructure flake, and it stops the form/keyboard/capture scenarios plus replay from running.

Use stable transition evidence that is actually present in the baseline/result, then rerun the exact Android lane. No ready-for-human while that required job is red.

[thymikee]

Exact-head review found three blockers:

1. Required Android smoke is red and needs owner action. The emulator recovered and the new suite completed inventory, automation, form, and most keyboard work before smoke:keyboard-ime timed out waiting for Agent Device Tester after the close/reopen transition; cleanup also failed. This is not explained by the initial adb offline boot noise. Reproduce/rerun with the session diagnostics artifact and fix or establish the precise infrastructure cause.
2. The implementation exceeds test(android): command-coverage manifest parity with iOS #1426’s explicit scope guardrail. That issue asks to classify and credit existing Android smoke/replay evidence first and says not to add new live scenarios. This PR adds five fixture journeys and materially expands the live lane. Split/downscope to the registry-derived manifest and existing evidence, or obtain and document an explicit issue-level scope amendment before merge.
3. The required classification summary is missing from the artifact. test(android): command-coverage manifest parity with iOS #1426 requires live vs contract vs gap counts. coverage-report.json currently contains runtime command/behavior evidence, completed scenarios, and timings only; contract and capability-denial rows never appear. Add a manifest-derived classification summary and a focused assertion.

The registry-derived manifest, command-credit checks, URL quoting fix, and bounded helper-shutdown fallback otherwise look sound. No readiness label while the required smoke is failing.

[thymikee]

Exact head 4551525f is not ready: the required Android Smoke lane failed in the live smoke:form-input scenario. After navigation and fill, public get text id="field-name" returned COMMAND_FAILED because the Android snapshot helper produced zero accessibility/window nodes while the fixture app was foregrounded. The exact-run report therefore completed only inventory/install and automation/system; form, keyboard dismissal, capture/close, and the subsequent replay did not complete, so the claimed full live coverage is not yet proven. The prior scope and architecture blockers are resolved, including #1426’s Android amendment and the manifest-derived 28 live / 22 contract / 3 capability-denial / 0 gap summary. Please obtain a green exact-head Android live lane, or fix the empty-tree failure if it reproduces, before readiness.

[thymikee]

Clean re-review at 30b8aba. The prior architecture/guardrail blockers are resolved: the shared live runtime replaces duplicate harnesses, manifest ownership is structurally tied to runtime evidence, timing is diagnostic-only, and the #1426 report summary is manifest-derived. The root-capture stabilization is bounded to incomplete captures (500 ms), retries full captures, preserves atomic fallback behavior, and is wired into helper tests. Static/type/integration/coverage/Fallow gates are green; exact-head Android and iOS smoke are still running, so device evidence remains pending.