feat(replay): accept --test-ime on test and replay so flow-owned Android opens opt into the test IME - #3235
Conversation
…oid opens opt into the test IME A flow that owns its session had no surface to request the bundled Android test IME: the flag was accepted only by `open`, so a physical-device Maestro eraseText (backspace is a control character) hit the ASCII-only adb-shell channel and told the caller to run a command it never calls. The opt-in rides the request flags envelope end to end: the replay/test CLI schemas and metadata input readers, the test-to-replay fan-out, the Maestro runtime device flags, and the parent-flag inheritance shared with batch, arriving at the unchanged session-open decision (emulator default-on and --no-test-ime forcing are untouched). The shell-text refusal now states the channel limit, carries the typed reason `android_shell_text_unsupported`, and keeps the `open` recovery in details; the replay failure boundary rewrites the hint to the flow-owned flag off that reason, never off message text.
…e layering snapshot
|
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 21 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Reviewed 22889e7. The change itself looks right, but two Coverage checks fail because of this diff and the Android test IME route has no live run yet. The Coverage failures come from this diff. The new Open the live run first: session-replay-maestro-runtime.ts#L353 changes which Android device Could the new contracts subpath The cubic-dev-ai P2 thread on test strength still applies: #3235 (comment) Not blocking: adding Before merge, fix the two Coverage checks, then attach the live Android run. |
…ixture The production cause arrives with the open-route hint already hoisted to the top level, so the fixture must too: a rewrite that only filled a missing hint would have passed the hint-free shape.
|
The code change looks sound at c453e0e, but no live Android run backs it yet, and Coverage fails because of this PR. No conflicts. The change alters which Android session opens switch the device IME during test and replay, and it changes the hint users see on REPLAY_DIVERGENCE. The only proof is unit and router tests that mock Coverage fails in eager-closure-budgets on six entries, and each one is +1 from Could the same plumbing work without the new contracts subpath? A shared symbol should live in a contracts module that every closure it reaches already evaluates. So On the review threads, the cubic-dev-ai P2 thread on the failure-response test is fixed at this head, so you can resolve it: #3235 (comment). I did not trace whether project config or AGENT_DEVICE_TEST_IME is filtered by each command's allowedFlags before it reaches test and replay, so I cannot confirm that config now works for flow runs. I also did not read the test-to-replay suite fan-out code beyond the PR's nested-flags test. Before merge, please attach the two live emulator runs, remove the contracts/android-text-input subpath, and split the |
…losures already load The new android-text-input subpath cost +1 module on six eager-closure entries (Coverage red on PR #3235). The reason constant is the only cross-package symbol; it moves to contracts/input-validation.ts, which every failing closure already evaluates, via the contracts/command facade re-export and a manifest subpath. The two hints move to their only consumers: the open-route hint beside the platform error that states it, the flow-route hint beside the replay failure boundary that swaps them. The --test-ime parser cases split into their own file; the shared session-parse table had grown past the 1,000-line tripwire.
…s/command facade text-input.ts already reaches the command facade; routing the reason through the same façade (with replay-port taking the module subpath) keeps every importer's eager closure at its merge-base size and gives the facade re-export its consumer.
|
All review items addressed at
|
|
The PR is ready at 3bcd186. The two Coverage failures from c453e0e (eager-closure-budgets and test-file-size-ratchet) are fixed. The delta removes the contracts subpath and moves the parser cases into a new test file. All 21 checks now pass. Not blocking: the fixture in session-replay-runtime-failure-response.test.ts (https://github.com/callstack/agent-device/blob/3bcd186/packages/replay-port/src/daemon-port/__tests__/session-replay-runtime-failure-response.test.ts#L85) now writes the open-route hint as a literal, because replay-port cannot import the Android package. If ANDROID_TEST_IME_OPEN_HINT changes, the literal can drift silently, but the test still passes since the rewrite keys on details.reason. You could shorten it to a stable stand-in, or leave it as is. The cubic-dev-ai P2 thread on the failure-response test fixture is fixed at this head, so you can resolve it: #3235 (comment) I did not reproduce the live emulator runs from the PR body. I did not rerun eager-closure-budgets or the full gate, and relied on the passing checks. I did not trace whether AGENT_DEVICE_TEST_IME or project config reaches test and replay through the allowedFlags filter. The PR body claims it, and the flag declares projectConfig true with both schemas listing it. Physical-device behavior is unmeasured. Nothing else stands in the way, so the merge is a maintainer decision. |
* origin/main: (77 commits) fix(apple-runner): fence prep spawns behind a start-owned admission (callstack#3239) 0.21.22 test(apple): own the simctl settings plan tests in simctl-settings.test.ts (callstack#3244) fix(limrun): report the session device id in iOS settings refusals (callstack#3243) 0.21.21 feat(remote): add a host-allocated macos-app lease backend (callstack#3236) test(android): bound the screenshot write wait by wall time, not event-loop turns (callstack#3250) feat(recording): cap the touch overlay frame rate at the caller's --fps (callstack#3241) fix(ad-script): let .ad scripts carry scroll --until and wait capture flags (callstack#3197) (callstack#3234) feat(provider-webdriver): keyboard enter, dismiss, and status over WebDriver (callstack#3233) feat(selectors): match role= against snapshot kind with a node-scoped alias window (callstack#3232) fix(provider-webdriver): read field values, placeholders, secure fields, and checked state from page source (callstack#3231) feat(replay): accept --test-ime on test and replay so flow-owned Android opens opt into the test IME (callstack#3235) refactor(daemon): route daemon-level diagnostics through one scope helper (callstack#3242) docs(adr): correct ADR 0031 pointer event delivery evidence (callstack#3245) fix(ios): stop a tap's post-gesture lookup from recording an XCTest failure (callstack#3060) (callstack#3237) fix(recording): render the touch overlay at most 30 fps and inside the record request (callstack#3219) fix(daemon): keep an idle daemon alive only for retained leases (callstack#3227) fix(provider-webdriver): send an empty JSON object on bodyless POSTs (callstack#3230) Feat/maestro repeat while (callstack#3214) ...
Summary
Closes #2997.
testandreplaynow accept--test-ime/--no-test-ime, so a flow that owns its session can opt a real Android device into the bundled test IME that MaestroeraseText(backspace = control char) and non-ASCIIfillneed. The real-device default stays off (#1198); emulator default-on andopen --no-test-imeforcing are untouched.The opt-in rides the request flags envelope end to end — CLI readers/schemas, the test→replay fan-out, the Maestro runtime device flags, and the parent-flag inheritance shared with batch — arriving at the unchanged
shouldActivateAndroidTestImedecision. Absent the flag, nothing is written anywhere, so the defaults still decide. The shell-text refusal now states only the channel limit, carries typed reasonandroid_shell_text_unsupported, and keeps theopenrecovery indetails; the replay failure boundary rewrites the hint to the flow-owned flag off that reason, never on message text.projectConfig: truewas already on the declaration (no credential exposure; the restore-on-close safety is local), sotestImeinagent-device.jsonnow works for flow runs too — the issue's alternative ask.Validation
pnpm check:affected --rungreen at head3bcd1860dd8380a5036651aced3208f4ebf7b9fd(unit suites, layering, eager-closure budgets, test-file-size ratchet, fallow, admission gates)..adinheritance test, and the batch inheritance test all fail without the fix.createRequestHandler→ replay → dispatched open → lifecycle hostactivateAndroidTestImeseam).Live emulator evidence (API 35 AVD
ReactNative_API_35,test --maestro/replay --maestroon a Settings flow withinputText+eraseText)test … --no-test-ime→ fails at step 5 withREPLAY_DIVERGENCE, and the printed hint is the flow-owned one: "pass--test-imeto this test/replay run … for the sessions the flow opens" — it never saysopen --test-ime. Same shape onreplay --no-test-ime(top-levelHint:line).test …(no flag) → passes: the emulator default-on activated the test IME anderaseText's backspaces went through.test … --test-ime→ passes, andadb shell settings get secure default_input_methodshowscom.google.android.inputmethod.latin/...LatinIME(the original IME) restored after the run; it also reads back Latin between runs, so the close-time restore holds.