fix(actions): keyboard-occlusion guard Phase 1 — inject hideKeyboard in Maestro generator (#356)

.changeset/keyboard-occlusion-guard-phase1.md

@@ -0,0 +1,6 @@
+---
+"rn-dev-agent-cdp": patch
+"rn-dev-agent-plugin": patch
+---
+
+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/specs/2026-06-20-356-keyboard-occlusion-guard-phase1-design.md

@@ -0,0 +1,170 @@
+# #356 — Keyboard-occlusion guard, Phase 1: Maestro `hideKeyboard` injection
+
+Date: 2026-06-20
+Issue: [#356](https://github.com/Lykhoyda/rn-dev-agent/issues/356) (extracted from #300, item 2)
+Status: Design approved — ready for implementation plan
+
+## Problem
+
+When a tap targets a **bottom-pinned control** (a `submit`/`continue` button) while
+the soft keyboard is up, the tap lands on the keyboard instead of the button, so the
+next screen is never reached. Per the #300 session this was **the single biggest
+source of flaky replays**, on **both iOS and Android**. The reporter's proven manual
+workaround was to add `- hideKeyboard` before each bottom-pinned tap in the flow.
+
+## Why two phases (and what this phase is NOT)
+
+The bug spans two independent device-control engines that do **not** share a tap path:
+
+| Surface | Execution path | Phase |
+|---|---|---|
+| Learned-action **replay** (`cdp_run_action` → `maestro_run`) | external **maestro-runner** Go binary (WDA / UIAutomator) — **never** calls `runNative()` | **Phase 1 (this spec)** |
+| Live `device_press` / `device_batch` / `cdp_interact` native fallback | `runNative()` → in-tree `rn-fast-runner` / `rn-android-runner` | Phase 2 (separate spec/PR) |
+
+Evidence: `cdp_run_action` delegates to `maestroRun()` (`run-action.ts:278`, retry at
+`:551`); `maestro_run` shells out via `execFile(dispatch.binPath, …)`
+(`maestro-run.ts:235`). A guard placed in `runNative()` is therefore a **no-op for
+action replays**. Because the issue's headline is flaky *replays*, Phase 1 fixes the
+**Maestro/L3** path; the in-runner frame-precise guard for live taps is **Phase 2**
+(decisions already taken for it: in-runner atomic, frame-precise on both platforms via
+iOS `app.keyboards.frame` and Android `UiAutomation.getWindows()` → `TYPE_INPUT_METHOD`
+bounds — recorded here so Phase 2 starts from them).
+
+**This phase does NOT** touch `runNative`, the native runners, repair-time injection,
+or existing on-disk action files. See "Out of scope".
+
+## Approach
+
+A **pure event-stream transform inside `generateMaestro()`**
+(`scripts/cdp-bridge/src/tools/test-recorder-generators.ts`) — the single chokepoint
+every generated/saved Maestro flow passes through (`cdp_record_test_generate` and
+`save-as-action` both call it). When the generator is about to emit a button tap that
+follows text entry, it first emits `- hideKeyboard`.
+
+Why this layer is correct and safe:
+
+1. `hideKeyboard` is already in the validator allowlist (`maestro-validator.ts:105`)
+   and is a **no-op when no keyboard is showing** — so over-injection costs only a few
+   hundred ms, never correctness.
+2. Maestro taps are **selector-based** (`id:` / `text:`), so after the dismiss Maestro
+   **re-resolves** the element's (possibly relayouted) position. There is **zero
+   stale-coordinate risk** — the precise concern Phase 2's coordinate guard must handle,
+   this phase avoids structurally.
+3. It is a pure function of `RecordedEvent[]` → `string`, so it is fully unit-testable
+   with no device.
+
+## Detection rule
+
+While walking the event stream, maintain one boolean `keyboardLikelyUp`:
+
+- `type` event → keyboard comes up → set `true`.
+  (The existing `type` branch emits `- tapOn` to focus + `- inputText: …`.)
+- Before emitting a `tap` or `long_press` step: if `keyboardLikelyUp`, emit
+  `- hideKeyboard` first, then set `keyboardLikelyUp = false`.
+- `navigate` event → new screen → set `keyboardLikelyUp = false`.
+- The focusing `tapOn` *inside* a `type` step is **not** guarded (the keyboard is wanted
+  there).
+- `submit` (`- pressKey: Enter`) leaves the flag unchanged — Enter's dismiss behavior is
+  field-type-dependent (single-line submits/dismisses; multiline inserts a newline), so
+  bias toward guarding any subsequent tap.
+
+Properties: conservative (may over-inject — harmless) but never misses a
+typed-then-tapped sequence, which is the exact reported failure. Deterministic and
+order-only — no geometry needed (`RecordedEvent` carries no rects; geometry-gating would
+require recorder changes and is an explicit non-goal here).
+
+### Emitted shape
+
+Before:
+```yaml
+- tapOn:
+    id: emailInput
+- inputText: "user@example.com"
+- tapOn:
+    id: submitButton
+```
+After:
+```yaml
+- tapOn:
+    id: emailInput
+- inputText: "user@example.com"
+# rn-dev-agent: keyboard-occlusion guard (#356)
+- hideKeyboard
+- tapOn:
+    id: submitButton
+```
+
+The marker is emitted as a **preceding full comment line**, matching the generator's
+existing comment style (`# navigated:`, `# NOTE:`) — not a trailing inline comment,
+which a line-oriented validator could reject. It makes the auto-injection auditable in
+the YAML and in PR diffs. The comment text is a static literal (no user-controlled
+interpolation), so it needs no `stripNewlines`/`maestroScalar` handling.
+
+## Components & data flow
+
+- **`generateMaestro(events, opts)`** — only changed function. Add the `keyboardLikelyUp`
+  state and the pre-tap emission. No new exported symbols required; the rule is internal
+  to the walk. (Optionally factor the rule into a small pure helper if it aids testing.)
+- No changes to `RecordedEvent`, the recorder, `save-as-action`, `maestro-validator`,
+  `repair-engine`, `runNative`, or either native runner.
+- `generateDetox` is intentionally untouched (Detox is secondary and its `.typeText`
+  path is not the reported failure); noted as optional future parity.
+
+## Error handling
+
+There is no new failure mode: the transform only inserts an allowlisted, idempotent
+step. Generated flows continue to pass `parseAndValidateFlow()` unchanged
+(`hideKeyboard` is already allowed). If `events` is empty or contains no typed-then-tap
+sequence, output is byte-identical to today.
+
+## Testing
+
+**Unit (primary, table-driven over event sequences):**
+- typed → tap ⇒ `- hideKeyboard` injected before the tap.
+- tap with no preceding type ⇒ no injection.
+- type → tap → tap ⇒ injected before the first tap only (flag cleared).
+- type → navigate → tap ⇒ no injection (navigate reset).
+- type → long_press ⇒ injected before long_press.
+- consecutive types (type → type → tap) ⇒ keyboard stays up; injected before the final tap; the focusing `tapOn` of the second `type` is not guarded.
+- type → submit → tap ⇒ injected before the tap (submit does not reset).
+- existing generator behaviors (navigation lookahead comments, selectors, metadata header, YAML-injection sanitization) remain unchanged — guard against regressions.
+
+**Device verification (both platforms — required by the issue):** record/replay a flow
+that fills a text field then taps a bottom-pinned submit and asserts the next screen.
+Confirm it was flaky before and replays reliably after, on **iOS and Android**.
+
+## Out of scope (follow-ups)
+
+- **Phase 2** — in-runner frame-precise occlusion guard for live `device_*` taps
+  (in-runner atomic; iOS `app.keyboards.frame`; Android `UiAutomation.getWindows()` →
+  `TYPE_INPUT_METHOD` bounds). Separate spec/PR, stacked on this one.
+- **Phase 1.5** — repair-time injection in `repair-engine.ts` (insert `- hideKeyboard`
+  before a patched `tapOn` via body string-surgery) and a one-time **backfill** command
+  for existing on-disk action YAMLs (rewrites committed files → opt-in).
+- Detox parity; `swipe` / `fill` occlusion handling.
+
+## Post-verification addition — Android replay-runner routing (B223)
+
+On-device verification found that the plugin's default L3 replay engine,
+maestro-runner v1.0.9 (DeviceLab.dev), **silently no-ops `hideKeyboard` on
+Android** (reports pass in ~5ms; `dumpsys input_method` shows `mInputShown=true`
+after), while official Maestro v2.3.0 dismisses it. So the generation-time
+injection alone, replayed via the default runner, did not actually fix Android.
+
+Added in this phase: `chooseMaestroDispatch` (`maestro-dispatch.ts`) gains a
+`flowHasHideKeyboard` input; on **Android** it prefers the official Maestro CLI
+when a flow contains `hideKeyboard` (verified to dismiss the keyboard on-device),
+and falls through to maestro-runner with a `degradedReason` warning when the CLI
+is absent. `maestro_run` detects `hideKeyboard` in the parsed commands
+(`flowContainsHideKeyboard`) and passes the flag. iOS is unaffected
+(maestro-runner honors `hideKeyboard` there). The maestro-runner no-op itself is
+filed as **B223** for an upstream/runtime follow-up.
+
+## Success criteria
+
+- Newly generated/saved Maestro action flows emit `- hideKeyboard` before any
+  button tap that follows text entry.
+- The transform is covered by deterministic unit tests.
+- A fill→bottom-pinned-submit flow replays reliably on iOS and Android where it was
+  previously flaky.
+- No change to generated output for flows without a typed-then-tapped sequence.

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

@@ -66,10 +66,41 @@ export function shouldWarnFallback(reason) {
     warnedFallbackReasons.add(reason);
     return true;
 }
+/**
+ * GH #356 / B223: detect whether a parsed Maestro flow contains a
+ * `hideKeyboard` step. Maestro represents it as the bare string command
+ * `'hideKeyboard'`; we also accept the object form `{ hideKeyboard: ... }`
+ * defensively. Used to route Android hideKeyboard flows to the official
+ * Maestro CLI (maestro-runner no-ops hideKeyboard on Android).
+ */
+export function flowContainsHideKeyboard(commands) {
+    return commands.some((c) => c === 'hideKeyboard' ||
+        (typeof c === 'object' && c !== null && 'hideKeyboard' in c));
+}
 export function chooseMaestroDispatch(inputs) {
     const whichAdb = inputs.whichAdb ?? defaultWhichAdb;
     const whichMaestro = inputs.whichMaestro ?? defaultWhichMaestro;
     const runnerPath = (inputs.maestroRunnerPath ?? defaultMaestroRunnerPath)();
+    // GH #356 / B223: maestro-runner v1.0.9 silently no-ops `hideKeyboard` on
+    // Android (reports pass in ~5ms, `mInputShown` stays true), which defeats the
+    // keyboard-occlusion guard's whole purpose. When an Android flow contains a
+    // hideKeyboard step, prefer the official Maestro CLI — verified to honor it on
+    // Android (`mInputShown=false` after). iOS maestro-runner honors hideKeyboard,
+    // so this only applies to Android.
+    const needsOfficialForKeyboard = inputs.platform === 'android' && inputs.flowHasHideKeyboard === true;
+    if (needsOfficialForKeyboard) {
+        const maestroPath = whichMaestro();
+        if (maestroPath) {
+            return {
+                runner: 'maestro',
+                binPath: maestroPath,
+                buildArgs: (platform, flowFile, _appFile) => ['test', '--platform', platform, flowFile],
+                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',
+            };
+        }
+        // CLI unavailable: fall through to maestro-runner (Tier 1) but mark the
+        // result degraded so the caller warns the keyboard will not be dismissed.
+    }
     // Tier 1: maestro-runner. Viable when (a) the binary is installed and
     // (b) we're on android OR adb is reachable (so the upstream bug doesn't bite).
     const runnerViable = runnerPath !== null && (inputs.platform === 'android' || whichAdb() !== null);
@@ -80,6 +111,11 @@ export function chooseMaestroDispatch(inputs) {
             buildArgs: (platform, flowFile, appFile) => appFile
                 ? ['--app-file', appFile, '--platform', platform, 'test', flowFile]
                 : ['--platform', platform, 'test', flowFile],
+            ...(needsOfficialForKeyboard
+                ? {
+                    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.',
+                }
+                : {}),
         };
     }
     // Tier 2: Maestro CLI fallback. Slower JVM cold start (~2s) but works on

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

@@ -6,7 +6,7 @@ import { join, dirname } from 'node:path';
 import { okResult, failResult, warnResult } from '../utils.js';
 import { getActiveSession } from '../agent-device-wrapper.js';
 import { resolveBundleId, readExpoSlug } from '../project-config.js';
-import { chooseMaestroDispatch, shouldWarnFallback } from './maestro-dispatch.js';
+import { chooseMaestroDispatch, shouldWarnFallback, flowContainsHideKeyboard, } from './maestro-dispatch.js';
 import { resolveAppFileForClearState } from './resolve-ios-app-file.js';
 import { buildMaestroFlow, parseAndValidateFlow, isValidBundleId, MaestroValidationError, } from '../domain/maestro-validator.js';
 import { outputIndicatesFlowFailure } from '../domain/maestro-error-parser.js';
@@ -89,12 +89,9 @@ export function createMaestroRunHandler() {
         if (!platform) {
             return failResult('Cannot determine platform. Pass platform or open a device session first.');
         }
-        // B59: tiered dispatch — maestro-runner when viable, Maestro CLI fallback
-        // when iOS-only and adb is missing, fail-fast with install hints when neither.
-        const dispatch = chooseMaestroDispatch({ platform });
-        if ('error' in dispatch) {
-            return failResult(dispatch.error);
-        }
+        // GH #356/B223: the dispatch tier depends on whether the validated flow
+        // uses hideKeyboard on Android, so the runner is chosen AFTER parsing below.
+        let flowHasHideKeyboard = false;
         // Phase 134.1 (deepsec CRITICAL #4): both inlineYaml and flowPath
         // are caller-controlled. Parse, validate against the command allowlist
         // (rejecting runScript and other host-executing directives by default),
@@ -132,6 +129,7 @@ export function createMaestroRunHandler() {
                 ? { flowDir: dirname(args.flowPath), flowRoot: dirname(args.flowPath) }
                 : {};
             const parsed = parseAndValidateFlow(rawYaml, runFlowOpts);
+            flowHasHideKeyboard = flowContainsHideKeyboard(parsed.commands);
             const rawAppId = resolveAppId(args.appId, platform);
             headerAppId = parsed.appId ?? (rawAppId && isValidBundleId(rawAppId) ? rawAppId : undefined);
             if (rawAppId && !parsed.appId && !isValidBundleId(rawAppId)) {
@@ -151,6 +149,13 @@ export function createMaestroRunHandler() {
             }
             throw err;
         }
+        // B59 + GH #356/B223: tiered dispatch — maestro-runner when viable, Maestro
+        // CLI fallback when iOS-only and adb is missing, and (B223) the Maestro CLI
+        // for Android flows that use hideKeyboard (maestro-runner no-ops it there).
+        const dispatch = chooseMaestroDispatch({ platform, flowHasHideKeyboard });
+        if ('error' in dispatch) {
+            return failResult(dispatch.error);
+        }
         const timeout = args.timeoutMs ?? 120_000;
         // GH #116: build the final argv. Start with the dispatch tier's
         // base args, then append `-e KEY=VALUE` pairs for any supplied
@@ -196,19 +201,23 @@ export function createMaestroRunHandler() {
                 timedOut: false,
                 outputTruncated: false,
                 ...(dispatch.fallbackReason ? { fallbackReason: dispatch.fallbackReason } : {}),
+                ...(dispatch.degradedReason ? { degradedReason: dispatch.degradedReason } : {}),
             };
+            // GH #356/B223: a degradedReason (Android hideKeyboard with no Maestro CLI)
+            // is a caveat surfaced the same way as a fallbackReason.
+            const caveat = dispatch.fallbackReason ?? dispatch.degradedReason;
             if (passed) {
                 // B59 (Gemini review, conf 82): on success-with-fallback, only emit
                 // a loud warning the FIRST time per process so a 100-flow loop
                 // doesn't generate 100 identical warnings. Subsequent successes
-                // carry the reason silently in meta.fallbackReason.
-                if (dispatch.fallbackReason && shouldWarnFallback(dispatch.fallbackReason)) {
-                    return warnResult(meta, dispatch.fallbackReason);
+                // carry the reason silently in meta.
+                if (caveat && shouldWarnFallback(caveat)) {
+                    return warnResult(meta, caveat);
                 }
                 return okResult(meta);
             }
-            const baseWarnMsg = dispatch.fallbackReason
-                ? `${dispatch.fallbackReason}; flow completed with warnings or failures`
+            const baseWarnMsg = caveat
+                ? `${caveat}; flow completed with warnings or failures`
                 : 'Flow completed with warnings or failures';
             // GH #263: classify on the FULL output (not the sliced meta.output).
             const warnAug = augmentFailureWithDegradation(output, resolveFloorMs(process.env.RN_RUNTIME_DEGRADED_FLOOR_MS), baseWarnMsg, meta);