Skip to content

Commit d61985f

Browse files
Lykhoydaclaude
andauthored
fix(actions): keyboard-occlusion guard Phase 1 — inject hideKeyboard in Maestro generator (#356) (#367)
* docs(356): design spec — keyboard-occlusion guard Phase 1 (Maestro hideKeyboard injection) Generation-time hideKeyboard injection in generateMaestro() to fix flaky action replays where bottom-pinned taps land on the soft keyboard. Phase 1 covers the L3/Maestro replay path (cdp_run_action -> maestro-runner); the L2/in-runner live-tap guard is deferred to Phase 2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(356): TDD implementation plan — keyboard-occlusion guard Phase 1 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(356): inject hideKeyboard before taps following text entry (Maestro generator) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(356): reset keyboard guard on navigate; guard long_press too Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): keyboard-occlusion guard Phase 1 (cdp + plugin patch) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build(356): stage rebuilt dist for hideKeyboard injection dist/ is tracked and loaded by the runtime MCP bridge; the src change in prior commits was inert in installs until this rebuilt artifact ships. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(356): route Android hideKeyboard flows to official Maestro CLI (B223) maestro-runner v1.0.9 silently no-ops hideKeyboard on Android (reports pass in ~5ms, mInputShown stays true), defeating the keyboard-occlusion guard. chooseMaestroDispatch now prefers the official Maestro CLI for Android flows that contain hideKeyboard (verified to honor it on-device), and surfaces a degradedReason warning when the CLI is unavailable. iOS is unaffected (maestro-runner honors hideKeyboard there). Device-verified: official Maestro dismisses the Android keyboard and the bottom-pinned tap lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(356): record Android hideKeyboard runner-routing (B223) in spec + changeset Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(356): route Android hideKeyboard flows in maestro_test_all too (B223) The batch replay surface (maestro_test_all) chose dispatch once before its per-flow loop, so Android replays of saved actions containing hideKeyboard still hit maestro-runner's no-op. Now re-routes per flow to the official Maestro CLI when a flow uses hideKeyboard on Android, surfacing a degraded warning if the CLI is unavailable. Mirrors the maestro_run routing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 57c2ef9 commit d61985f

13 files changed

Lines changed: 958 additions & 51 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"rn-dev-agent-cdp": patch
3+
"rn-dev-agent-plugin": patch
4+
---
5+
6+
fix(actions): inject `- hideKeyboard` before button taps that follow text entry when generating/saving Maestro action flows, and route Android hideKeyboard replays to the official Maestro CLI (#356, Phase 1). Bottom-pinned taps (submit/continue) previously landed on the soft keyboard during replays — the single biggest source of flaky replays. `generateMaestro` now tracks soft-keyboard state and emits a `hideKeyboard` step before a `tap`/`long_press` that follows an `inputText`, reset on navigation. `hideKeyboard` is a no-op when no keyboard is showing and Maestro re-resolves the selector after dismiss, so the injection is safe. Device verification surfaced that maestro-runner v1.0.9 silently no-ops `hideKeyboard` on Android (B223), so `maestro_run` now prefers the official Maestro CLI for Android flows containing `hideKeyboard` (verified to dismiss the keyboard on-device), warning when the CLI is unavailable; iOS is unaffected (maestro-runner honors hideKeyboard there). Live `device_*` taps (the in-runner guard) and existing-corpus backfill are deferred to later phases.

docs/superpowers/plans/2026-06-20-356-keyboard-occlusion-guard-phase1.md

Lines changed: 387 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
# #356 — Keyboard-occlusion guard, Phase 1: Maestro `hideKeyboard` injection
2+
3+
Date: 2026-06-20
4+
Issue: [#356](https://github.com/Lykhoyda/rn-dev-agent/issues/356) (extracted from #300, item 2)
5+
Status: Design approved — ready for implementation plan
6+
7+
## Problem
8+
9+
When a tap targets a **bottom-pinned control** (a `submit`/`continue` button) while
10+
the soft keyboard is up, the tap lands on the keyboard instead of the button, so the
11+
next screen is never reached. Per the #300 session this was **the single biggest
12+
source of flaky replays**, on **both iOS and Android**. The reporter's proven manual
13+
workaround was to add `- hideKeyboard` before each bottom-pinned tap in the flow.
14+
15+
## Why two phases (and what this phase is NOT)
16+
17+
The bug spans two independent device-control engines that do **not** share a tap path:
18+
19+
| Surface | Execution path | Phase |
20+
|---|---|---|
21+
| Learned-action **replay** (`cdp_run_action``maestro_run`) | external **maestro-runner** Go binary (WDA / UIAutomator) — **never** calls `runNative()` | **Phase 1 (this spec)** |
22+
| Live `device_press` / `device_batch` / `cdp_interact` native fallback | `runNative()` → in-tree `rn-fast-runner` / `rn-android-runner` | Phase 2 (separate spec/PR) |
23+
24+
Evidence: `cdp_run_action` delegates to `maestroRun()` (`run-action.ts:278`, retry at
25+
`:551`); `maestro_run` shells out via `execFile(dispatch.binPath, …)`
26+
(`maestro-run.ts:235`). A guard placed in `runNative()` is therefore a **no-op for
27+
action replays**. Because the issue's headline is flaky *replays*, Phase 1 fixes the
28+
**Maestro/L3** path; the in-runner frame-precise guard for live taps is **Phase 2**
29+
(decisions already taken for it: in-runner atomic, frame-precise on both platforms via
30+
iOS `app.keyboards.frame` and Android `UiAutomation.getWindows()``TYPE_INPUT_METHOD`
31+
bounds — recorded here so Phase 2 starts from them).
32+
33+
**This phase does NOT** touch `runNative`, the native runners, repair-time injection,
34+
or existing on-disk action files. See "Out of scope".
35+
36+
## Approach
37+
38+
A **pure event-stream transform inside `generateMaestro()`**
39+
(`scripts/cdp-bridge/src/tools/test-recorder-generators.ts`) — the single chokepoint
40+
every generated/saved Maestro flow passes through (`cdp_record_test_generate` and
41+
`save-as-action` both call it). When the generator is about to emit a button tap that
42+
follows text entry, it first emits `- hideKeyboard`.
43+
44+
Why this layer is correct and safe:
45+
46+
1. `hideKeyboard` is already in the validator allowlist (`maestro-validator.ts:105`)
47+
and is a **no-op when no keyboard is showing** — so over-injection costs only a few
48+
hundred ms, never correctness.
49+
2. Maestro taps are **selector-based** (`id:` / `text:`), so after the dismiss Maestro
50+
**re-resolves** the element's (possibly relayouted) position. There is **zero
51+
stale-coordinate risk** — the precise concern Phase 2's coordinate guard must handle,
52+
this phase avoids structurally.
53+
3. It is a pure function of `RecordedEvent[]``string`, so it is fully unit-testable
54+
with no device.
55+
56+
## Detection rule
57+
58+
While walking the event stream, maintain one boolean `keyboardLikelyUp`:
59+
60+
- `type` event → keyboard comes up → set `true`.
61+
(The existing `type` branch emits `- tapOn` to focus + `- inputText: …`.)
62+
- Before emitting a `tap` or `long_press` step: if `keyboardLikelyUp`, emit
63+
`- hideKeyboard` first, then set `keyboardLikelyUp = false`.
64+
- `navigate` event → new screen → set `keyboardLikelyUp = false`.
65+
- The focusing `tapOn` *inside* a `type` step is **not** guarded (the keyboard is wanted
66+
there).
67+
- `submit` (`- pressKey: Enter`) leaves the flag unchanged — Enter's dismiss behavior is
68+
field-type-dependent (single-line submits/dismisses; multiline inserts a newline), so
69+
bias toward guarding any subsequent tap.
70+
71+
Properties: conservative (may over-inject — harmless) but never misses a
72+
typed-then-tapped sequence, which is the exact reported failure. Deterministic and
73+
order-only — no geometry needed (`RecordedEvent` carries no rects; geometry-gating would
74+
require recorder changes and is an explicit non-goal here).
75+
76+
### Emitted shape
77+
78+
Before:
79+
```yaml
80+
- tapOn:
81+
id: emailInput
82+
- inputText: "user@example.com"
83+
- tapOn:
84+
id: submitButton
85+
```
86+
After:
87+
```yaml
88+
- tapOn:
89+
id: emailInput
90+
- inputText: "user@example.com"
91+
# rn-dev-agent: keyboard-occlusion guard (#356)
92+
- hideKeyboard
93+
- tapOn:
94+
id: submitButton
95+
```
96+
97+
The marker is emitted as a **preceding full comment line**, matching the generator's
98+
existing comment style (`# navigated:`, `# NOTE:`) — not a trailing inline comment,
99+
which a line-oriented validator could reject. It makes the auto-injection auditable in
100+
the YAML and in PR diffs. The comment text is a static literal (no user-controlled
101+
interpolation), so it needs no `stripNewlines`/`maestroScalar` handling.
102+
103+
## Components & data flow
104+
105+
- **`generateMaestro(events, opts)`** — only changed function. Add the `keyboardLikelyUp`
106+
state and the pre-tap emission. No new exported symbols required; the rule is internal
107+
to the walk. (Optionally factor the rule into a small pure helper if it aids testing.)
108+
- No changes to `RecordedEvent`, the recorder, `save-as-action`, `maestro-validator`,
109+
`repair-engine`, `runNative`, or either native runner.
110+
- `generateDetox` is intentionally untouched (Detox is secondary and its `.typeText`
111+
path is not the reported failure); noted as optional future parity.
112+
113+
## Error handling
114+
115+
There is no new failure mode: the transform only inserts an allowlisted, idempotent
116+
step. Generated flows continue to pass `parseAndValidateFlow()` unchanged
117+
(`hideKeyboard` is already allowed). If `events` is empty or contains no typed-then-tap
118+
sequence, output is byte-identical to today.
119+
120+
## Testing
121+
122+
**Unit (primary, table-driven over event sequences):**
123+
- typed → tap ⇒ `- hideKeyboard` injected before the tap.
124+
- tap with no preceding type ⇒ no injection.
125+
- type → tap → tap ⇒ injected before the first tap only (flag cleared).
126+
- type → navigate → tap ⇒ no injection (navigate reset).
127+
- type → long_press ⇒ injected before long_press.
128+
- consecutive types (type → type → tap) ⇒ keyboard stays up; injected before the final tap; the focusing `tapOn` of the second `type` is not guarded.
129+
- type → submit → tap ⇒ injected before the tap (submit does not reset).
130+
- existing generator behaviors (navigation lookahead comments, selectors, metadata header, YAML-injection sanitization) remain unchanged — guard against regressions.
131+
132+
**Device verification (both platforms — required by the issue):** record/replay a flow
133+
that fills a text field then taps a bottom-pinned submit and asserts the next screen.
134+
Confirm it was flaky before and replays reliably after, on **iOS and Android**.
135+
136+
## Out of scope (follow-ups)
137+
138+
- **Phase 2** — in-runner frame-precise occlusion guard for live `device_*` taps
139+
(in-runner atomic; iOS `app.keyboards.frame`; Android `UiAutomation.getWindows()` →
140+
`TYPE_INPUT_METHOD` bounds). Separate spec/PR, stacked on this one.
141+
- **Phase 1.5** — repair-time injection in `repair-engine.ts` (insert `- hideKeyboard`
142+
before a patched `tapOn` via body string-surgery) and a one-time **backfill** command
143+
for existing on-disk action YAMLs (rewrites committed files → opt-in).
144+
- Detox parity; `swipe` / `fill` occlusion handling.
145+
146+
## Post-verification addition — Android replay-runner routing (B223)
147+
148+
On-device verification found that the plugin's default L3 replay engine,
149+
maestro-runner v1.0.9 (DeviceLab.dev), **silently no-ops `hideKeyboard` on
150+
Android** (reports pass in ~5ms; `dumpsys input_method` shows `mInputShown=true`
151+
after), while official Maestro v2.3.0 dismisses it. So the generation-time
152+
injection alone, replayed via the default runner, did not actually fix Android.
153+
154+
Added in this phase: `chooseMaestroDispatch` (`maestro-dispatch.ts`) gains a
155+
`flowHasHideKeyboard` input; on **Android** it prefers the official Maestro CLI
156+
when a flow contains `hideKeyboard` (verified to dismiss the keyboard on-device),
157+
and falls through to maestro-runner with a `degradedReason` warning when the CLI
158+
is absent. `maestro_run` detects `hideKeyboard` in the parsed commands
159+
(`flowContainsHideKeyboard`) and passes the flag. iOS is unaffected
160+
(maestro-runner honors `hideKeyboard` there). The maestro-runner no-op itself is
161+
filed as **B223** for an upstream/runtime follow-up.
162+
163+
## Success criteria
164+
165+
- Newly generated/saved Maestro action flows emit `- hideKeyboard` before any
166+
button tap that follows text entry.
167+
- The transform is covered by deterministic unit tests.
168+
- A fill→bottom-pinned-submit flow replays reliably on iOS and Android where it was
169+
previously flaky.
170+
- No change to generated output for flows without a typed-then-tapped sequence.

scripts/cdp-bridge/dist/tools/maestro-dispatch.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,41 @@ export function shouldWarnFallback(reason) {
6666
warnedFallbackReasons.add(reason);
6767
return true;
6868
}
69+
/**
70+
* GH #356 / B223: detect whether a parsed Maestro flow contains a
71+
* `hideKeyboard` step. Maestro represents it as the bare string command
72+
* `'hideKeyboard'`; we also accept the object form `{ hideKeyboard: ... }`
73+
* defensively. Used to route Android hideKeyboard flows to the official
74+
* Maestro CLI (maestro-runner no-ops hideKeyboard on Android).
75+
*/
76+
export function flowContainsHideKeyboard(commands) {
77+
return commands.some((c) => c === 'hideKeyboard' ||
78+
(typeof c === 'object' && c !== null && 'hideKeyboard' in c));
79+
}
6980
export function chooseMaestroDispatch(inputs) {
7081
const whichAdb = inputs.whichAdb ?? defaultWhichAdb;
7182
const whichMaestro = inputs.whichMaestro ?? defaultWhichMaestro;
7283
const runnerPath = (inputs.maestroRunnerPath ?? defaultMaestroRunnerPath)();
84+
// GH #356 / B223: maestro-runner v1.0.9 silently no-ops `hideKeyboard` on
85+
// Android (reports pass in ~5ms, `mInputShown` stays true), which defeats the
86+
// keyboard-occlusion guard's whole purpose. When an Android flow contains a
87+
// hideKeyboard step, prefer the official Maestro CLI — verified to honor it on
88+
// Android (`mInputShown=false` after). iOS maestro-runner honors hideKeyboard,
89+
// so this only applies to Android.
90+
const needsOfficialForKeyboard = inputs.platform === 'android' && inputs.flowHasHideKeyboard === true;
91+
if (needsOfficialForKeyboard) {
92+
const maestroPath = whichMaestro();
93+
if (maestroPath) {
94+
return {
95+
runner: 'maestro',
96+
binPath: maestroPath,
97+
buildArgs: (platform, flowFile, _appFile) => ['test', '--platform', platform, flowFile],
98+
fallbackReason: 'Android flow uses hideKeyboard; maestro-runner v1.0.9 no-ops it on Android (B223) — using the Maestro CLI so the keyboard is actually dismissed',
99+
};
100+
}
101+
// CLI unavailable: fall through to maestro-runner (Tier 1) but mark the
102+
// result degraded so the caller warns the keyboard will not be dismissed.
103+
}
73104
// Tier 1: maestro-runner. Viable when (a) the binary is installed and
74105
// (b) we're on android OR adb is reachable (so the upstream bug doesn't bite).
75106
const runnerViable = runnerPath !== null && (inputs.platform === 'android' || whichAdb() !== null);
@@ -80,6 +111,11 @@ export function chooseMaestroDispatch(inputs) {
80111
buildArgs: (platform, flowFile, appFile) => appFile
81112
? ['--app-file', appFile, '--platform', platform, 'test', flowFile]
82113
: ['--platform', platform, 'test', flowFile],
114+
...(needsOfficialForKeyboard
115+
? {
116+
degradedReason: 'Android flow uses hideKeyboard but the Maestro CLI is not installed; maestro-runner v1.0.9 no-ops hideKeyboard on Android (B223), so the keyboard will NOT be dismissed. Install the Maestro CLI (`brew install maestro`) for the keyboard-occlusion fix to work on Android.',
117+
}
118+
: {}),
83119
};
84120
}
85121
// Tier 2: Maestro CLI fallback. Slower JVM cold start (~2s) but works on

scripts/cdp-bridge/dist/tools/maestro-run.js

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { join, dirname } from 'node:path';
66
import { okResult, failResult, warnResult } from '../utils.js';
77
import { getActiveSession } from '../agent-device-wrapper.js';
88
import { resolveBundleId, readExpoSlug } from '../project-config.js';
9-
import { chooseMaestroDispatch, shouldWarnFallback } from './maestro-dispatch.js';
9+
import { chooseMaestroDispatch, shouldWarnFallback, flowContainsHideKeyboard, } from './maestro-dispatch.js';
1010
import { resolveAppFileForClearState } from './resolve-ios-app-file.js';
1111
import { buildMaestroFlow, parseAndValidateFlow, isValidBundleId, MaestroValidationError, } from '../domain/maestro-validator.js';
1212
import { outputIndicatesFlowFailure } from '../domain/maestro-error-parser.js';
@@ -89,12 +89,9 @@ export function createMaestroRunHandler() {
8989
if (!platform) {
9090
return failResult('Cannot determine platform. Pass platform or open a device session first.');
9191
}
92-
// B59: tiered dispatch — maestro-runner when viable, Maestro CLI fallback
93-
// when iOS-only and adb is missing, fail-fast with install hints when neither.
94-
const dispatch = chooseMaestroDispatch({ platform });
95-
if ('error' in dispatch) {
96-
return failResult(dispatch.error);
97-
}
92+
// GH #356/B223: the dispatch tier depends on whether the validated flow
93+
// uses hideKeyboard on Android, so the runner is chosen AFTER parsing below.
94+
let flowHasHideKeyboard = false;
9895
// Phase 134.1 (deepsec CRITICAL #4): both inlineYaml and flowPath
9996
// are caller-controlled. Parse, validate against the command allowlist
10097
// (rejecting runScript and other host-executing directives by default),
@@ -132,6 +129,7 @@ export function createMaestroRunHandler() {
132129
? { flowDir: dirname(args.flowPath), flowRoot: dirname(args.flowPath) }
133130
: {};
134131
const parsed = parseAndValidateFlow(rawYaml, runFlowOpts);
132+
flowHasHideKeyboard = flowContainsHideKeyboard(parsed.commands);
135133
const rawAppId = resolveAppId(args.appId, platform);
136134
headerAppId = parsed.appId ?? (rawAppId && isValidBundleId(rawAppId) ? rawAppId : undefined);
137135
if (rawAppId && !parsed.appId && !isValidBundleId(rawAppId)) {
@@ -151,6 +149,13 @@ export function createMaestroRunHandler() {
151149
}
152150
throw err;
153151
}
152+
// B59 + GH #356/B223: tiered dispatch — maestro-runner when viable, Maestro
153+
// CLI fallback when iOS-only and adb is missing, and (B223) the Maestro CLI
154+
// for Android flows that use hideKeyboard (maestro-runner no-ops it there).
155+
const dispatch = chooseMaestroDispatch({ platform, flowHasHideKeyboard });
156+
if ('error' in dispatch) {
157+
return failResult(dispatch.error);
158+
}
154159
const timeout = args.timeoutMs ?? 120_000;
155160
// GH #116: build the final argv. Start with the dispatch tier's
156161
// base args, then append `-e KEY=VALUE` pairs for any supplied
@@ -196,19 +201,23 @@ export function createMaestroRunHandler() {
196201
timedOut: false,
197202
outputTruncated: false,
198203
...(dispatch.fallbackReason ? { fallbackReason: dispatch.fallbackReason } : {}),
204+
...(dispatch.degradedReason ? { degradedReason: dispatch.degradedReason } : {}),
199205
};
206+
// GH #356/B223: a degradedReason (Android hideKeyboard with no Maestro CLI)
207+
// is a caveat surfaced the same way as a fallbackReason.
208+
const caveat = dispatch.fallbackReason ?? dispatch.degradedReason;
200209
if (passed) {
201210
// B59 (Gemini review, conf 82): on success-with-fallback, only emit
202211
// a loud warning the FIRST time per process so a 100-flow loop
203212
// doesn't generate 100 identical warnings. Subsequent successes
204-
// carry the reason silently in meta.fallbackReason.
205-
if (dispatch.fallbackReason && shouldWarnFallback(dispatch.fallbackReason)) {
206-
return warnResult(meta, dispatch.fallbackReason);
213+
// carry the reason silently in meta.
214+
if (caveat && shouldWarnFallback(caveat)) {
215+
return warnResult(meta, caveat);
207216
}
208217
return okResult(meta);
209218
}
210-
const baseWarnMsg = dispatch.fallbackReason
211-
? `${dispatch.fallbackReason}; flow completed with warnings or failures`
219+
const baseWarnMsg = caveat
220+
? `${caveat}; flow completed with warnings or failures`
212221
: 'Flow completed with warnings or failures';
213222
// GH #263: classify on the FULL output (not the sliced meta.output).
214223
const warnAug = augmentFailureWithDegradation(output, resolveFloorMs(process.env.RN_RUNTIME_DEGRADED_FLOOR_MS), baseWarnMsg, meta);

0 commit comments

Comments
 (0)