fix: dismiss RN overlays in Maestro compat#650
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a590d95fe8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return await resolveMaestroSnapshotTarget(params, selector, options, 'tapOn', { | ||
| promoteTapTarget: true, | ||
| }); | ||
| if (!target.ok) return { response: target.response, targetResolved: false }; |
There was a problem hiding this comment.
Fail fast when RN overlays block Maestro taps
When a Maestro tapOn targets app content while a React Native RedBox is detected, resolveMaestroSnapshotTarget returns the "React Native overlay is covering app content" failure, but this path now reports targetResolved: false, so attemptMaestroTapOn treats it as retryable and invokeMaestroTapOn keeps re-snapshotting until the full tap timeout (30s, or 3s for optional taps). Since this commit removed the auto-dismiss retry, these overlay-blocked failures are no longer recoverable by waiting; they should return the overlay error immediately like the assertion path does, otherwise authored flows hang for the entire timeout before surfacing the actionable overlay message.
Useful? React with 👍 / 👎.
Summary
Stop preferring React Native RedBox Minimize when dismissing overlays.
Make Maestro compat stop auto-running
react-native dismiss-overlayduring assertions and tap resolution. Explicit MaestrotapOn: Dismissnow clicks the resolved Dismiss control directly, so test failures remain visible and authored flows keep their intended behavior.Update overlay tests and SkillGym guidance to match the new policy.
Validation
pnpm vitest run src/daemon/handlers/__tests__/react-native.test.ts src/compat/maestro/__tests__/runtime-assertions.test.ts src/compat/maestro/__tests__/runtime-interactions.test.ts src/__tests__/runtime-snapshot.test.tspnpm check:quick