Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a CI-focused Android launcher: new npm script and CI shell script to select/create AVDs, prepare/build/install the example app (env-configurable), then hand off to the existing Maestro Android runner; updates README and GitHub Actions to document and gate the workflow. (36 words) Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Environment
participant CI_Script as run-maestro-android-ci.sh
participant SDK as Android SDK / avdmanager
participant Emulator as Android Emulator
participant Builder as bun / Gradle
participant Launcher as run-maestro-android.sh
participant Maestro as Maestro CLI
CI->>CI_Script: start `maestro:android:ci` (env vars)
CI_Script->>SDK: ensure system-image, select/create AVD
SDK-->>CI_Script: AVD ready
CI_Script->>Emulator: start emulator
Emulator->>Emulator: boot, wait for `sys.boot_completed`
Emulator-->>CI_Script: device appears (discover ANDROID_SERIAL)
alt MAESTRO_ANDROID_SKIP_PREBUILD != 1
CI_Script->>Builder: bun install/build, sync, gradle assembleDebug
Builder-->>CI_Script: APK built
CI_Script->>Emulator: install APK on discovered device
Emulator-->>CI_Script: APK installed
end
CI_Script->>Launcher: exec launcher script (passes env)
alt MAESTRO_ANDROID_MANUAL_DRIVER != 1
Launcher->>Maestro: run `maestro test` (default driver bootstrap)
else
Launcher->>Launcher: start/install/manual driver, port-forwarding, instrumentation
Launcher->>Maestro: run `maestro test`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2ea7d5741
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6366412c6d
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example-app/scripts/run-maestro-android-ci.sh`:
- Around line 117-154: The script currently picks any device in "device" state
into BOOTED_DEVICE_ID which can select a physical phone; change the logic that
sets BOOTED_DEVICE_ID (both the initial assignment and the loop after launching
an AVD) to only accept emulator serials (match emulator-*) using ADB_BIN so the
emulator reuse path won't be short-circuited by a physical device, and ensure
wait_for_boot_completion is invoked for the reused emulator as well as after
launching one (so call wait_for_boot_completion "$BOOTED_DEVICE_ID" in both code
paths); keep STARTED_EMULATOR/EMULATOR_PID/cleanup behavior unchanged.
- Line 131: The script currently uses exec to start run-maestro-android.sh which
replaces the shell and prevents the trap cleanup EXIT handler (the cleanup
function that kills the background emulator) from running; remove the exec so
the shell remains and the trap cleanup EXIT executes when the script exits,
ensuring the cleanup function that kills the emulator process runs and prevents
emulator leaks (look for the trap cleanup EXIT registration and the call to
run-maestro-android.sh).
- Around line 8-11: Update the default locations and PATH derivation: change
SDKMANAGER_BIN and AVDMANAGER_BIN defaults to the cmdline-tools path
(cmdline-tools/latest/bin/sdkmanager and .../avdmanager) instead of tools/bin,
and modify the PATH export to build from the resolved bin variables (ADB_BIN,
EMULATOR_BIN, SDKMANAGER_BIN, AVDMANAGER_BIN) so any environment overrides are
honored; locate and update the variable definitions SDKMANAGER_BIN and
AVDMANAGER_BIN and the PATH assembly that currently hardcodes SDK_ROOT subdirs
to instead derive entries from the resolved *_BIN variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d44ebc4-75c2-4ae5-91bb-e32f1b467538
📒 Files selected for processing (3)
example-app/README.mdexample-app/package.jsonexample-app/scripts/run-maestro-android-ci.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1ebb4e8ff
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
example-app/scripts/run-maestro-android-ci.sh (3)
142-179:⚠️ Potential issue | 🟠 MajorOnly reuse emulator serials, and wait after reuse.
Line 142 and Line 166 accept any
device, so a connected phone can short-circuit the emulator path. The reuse path also skipswait_for_boot_completion, which makes the install/Maestro handoff racey when the emulator is still finishing boot.Suggested fix
+first_booted_emulator() { + "$ADB_BIN" devices | awk 'NR > 1 && $1 ~ /^emulator-[0-9]+$/ && $2 == "device" { print $1; exit }' +} + -BOOTED_DEVICE_ID="$("$ADB_BIN" devices | awk 'NR > 1 && $2 == "device" { print $1; exit }')" +BOOTED_DEVICE_ID="$(first_booted_emulator)" @@ - BOOTED_DEVICE_ID="$("$ADB_BIN" devices | awk 'NR > 1 && $2 == "device" { print $1; exit }')" + BOOTED_DEVICE_ID="$(first_booted_emulator)" @@ - wait_for_boot_completion "$BOOTED_DEVICE_ID" fi + +wait_for_boot_completion "$BOOTED_DEVICE_ID"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/scripts/run-maestro-android-ci.sh` around lines 142 - 179, The script currently treats any "device" as an emulator and may reuse a physical device, plus it skips waiting when reusing an emulator; update the ADB device selection and flow so only emulator serials (e.g., those matching emulator-* ) are considered and ensure wait_for_boot_completion is called for reused emulators. Concretely, change the awk/grep that sets BOOTED_DEVICE_ID (both the initial assignment and the loop that polls after launching) to filter for emulator serials only (look for the emulator- prefix), and if a BOOTED_DEVICE_ID is found from an existing emulator (reuse path), call wait_for_boot_completion "$BOOTED_DEVICE_ID" before proceeding; keep STARTED_EMULATOR/EMULATOR_PID behavior unchanged for the newly launched path.
210-212:⚠️ Potential issue | 🟠 MajorDrop
execso the EXIT trap can clean up the launched emulator.The shell installs
trap cleanup EXITon Line 156, butexecon Line 212 replaces that shell withrun-maestro-android.sh. The cleanup handler that callsemu kill/kill "$EMULATOR_PID"therefore never runs for a launcher-started emulator. (gnu.org)Suggested fix
export ANDROID_SERIAL="$BOOTED_DEVICE_ID" cd "$ROOT_DIR" -exec ./scripts/run-maestro-android.sh +./scripts/run-maestro-android.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/scripts/run-maestro-android-ci.sh` around lines 210 - 212, Remove the final exec so the EXIT trap (cleanup) in this script can run; instead of replacing the shell with ./scripts/run-maestro-android.sh, run it as a child process from example-app/scripts/run-maestro-android-ci.sh (preserving ANDROID_SERIAL/BOOTED_DEVICE_ID and cd "$ROOT_DIR"), capture its exit status and exit with that status so the cleanup function (installed via trap cleanup EXIT) can run and still propagate the child script's exit code.
13-55:⚠️ Potential issue | 🟠 MajorResolve tools from one SDK root and export that same toolchain.
find_sdk_toolletscommand -voutrankSDK_ROOT, and Line 55 then exports hard-codedSDK_ROOTsubdirs anyway. On a machine with multiple SDK installs, prep can use one toolchain whileexample-app/scripts/run-maestro-android.sh:9,34-79later uses a differentadbfromPATH, which reintroduces the non-determinism this launcher is supposed to remove.Suggested fix
find_sdk_tool() { local tool="$1" local candidate - if candidate="$(command -v "$tool" 2>/dev/null)"; then - printf '%s\n' "$candidate" - return 0 - fi - for candidate in \ "$SDK_ROOT/cmdline-tools/latest/bin/$tool" \ "$SDK_ROOT/cmdline-tools"/*/bin/"$tool" \ "$SDK_ROOT/tools/bin/$tool" \ "$SDK_ROOT/emulator/$tool" \ "$SDK_ROOT/platform-tools/$tool"; do if [[ -x "$candidate" ]]; then printf '%s\n' "$candidate" return 0 fi done + + if candidate="$(command -v "$tool" 2>/dev/null)"; then + printf '%s\n' "$candidate" + return 0 + fi return 1 } @@ -export PATH="$SDK_ROOT/platform-tools:$SDK_ROOT/emulator:$SDK_ROOT/tools/bin:$PATH" +export PATH="$(dirname "$ADB_BIN"):$(dirname "$EMULATOR_BIN"):$(dirname "$SDKMANAGER_BIN"):$(dirname "$AVDMANAGER_BIN"):$PATH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/scripts/run-maestro-android-ci.sh` around lines 13 - 55, find_sdk_tool currently allows command -v to return a tool outside SDK_ROOT, but the script later exports hard-coded SDK_ROOT subdirs; fix by making tool resolution and PATH export consistent: ensure find_sdk_tool prefers SDK_ROOT by either prepending "$SDK_ROOT/platform-tools:$SDK_ROOT/emulator:$SDK_ROOT/tools/bin" to PATH before calling find_sdk_tool/command -v, or after resolving ADB_BIN/EMULATOR_BIN/SDKMANAGER_BIN/AVDMANAGER_BIN capture their parent SDK root and export PATH to point to that same SDK root's platform-tools, emulator and tools/bin directories (update the PATH export line accordingly) so the resolved binaries and exported toolchain come from the same SDK_ROOT.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 201-202: The maestro_ios job currently lists guard_swiftpm_version
in its needs, causing maestro_ios to be skipped whenever guard_swiftpm_version
is skipped; update the maestro_ios job definition so its needs still include
guard_swiftpm_version but add always() around that dependency and augment the
maestro_ios if condition to explicitly allow continuation when
guard_swiftpm_version was skipped or succeeded (e.g., use
needs.guard_swiftpm_version.result == 'skipped' ||
needs.guard_swiftpm_version.result == 'success' combined with the existing
needs.changes.outputs.maestro_ios == 'true' check) so iOS-only PRs that don't
touch Package.swift will run Maestro tests.
---
Duplicate comments:
In `@example-app/scripts/run-maestro-android-ci.sh`:
- Around line 142-179: The script currently treats any "device" as an emulator
and may reuse a physical device, plus it skips waiting when reusing an emulator;
update the ADB device selection and flow so only emulator serials (e.g., those
matching emulator-* ) are considered and ensure wait_for_boot_completion is
called for reused emulators. Concretely, change the awk/grep that sets
BOOTED_DEVICE_ID (both the initial assignment and the loop that polls after
launching) to filter for emulator serials only (look for the emulator- prefix),
and if a BOOTED_DEVICE_ID is found from an existing emulator (reuse path), call
wait_for_boot_completion "$BOOTED_DEVICE_ID" before proceeding; keep
STARTED_EMULATOR/EMULATOR_PID behavior unchanged for the newly launched path.
- Around line 210-212: Remove the final exec so the EXIT trap (cleanup) in this
script can run; instead of replacing the shell with
./scripts/run-maestro-android.sh, run it as a child process from
example-app/scripts/run-maestro-android-ci.sh (preserving
ANDROID_SERIAL/BOOTED_DEVICE_ID and cd "$ROOT_DIR"), capture its exit status and
exit with that status so the cleanup function (installed via trap cleanup EXIT)
can run and still propagate the child script's exit code.
- Around line 13-55: find_sdk_tool currently allows command -v to return a tool
outside SDK_ROOT, but the script later exports hard-coded SDK_ROOT subdirs; fix
by making tool resolution and PATH export consistent: ensure find_sdk_tool
prefers SDK_ROOT by either prepending
"$SDK_ROOT/platform-tools:$SDK_ROOT/emulator:$SDK_ROOT/tools/bin" to PATH before
calling find_sdk_tool/command -v, or after resolving
ADB_BIN/EMULATOR_BIN/SDKMANAGER_BIN/AVDMANAGER_BIN capture their parent SDK root
and export PATH to point to that same SDK root's platform-tools, emulator and
tools/bin directories (update the PATH export line accordingly) so the resolved
binaries and exported toolchain come from the same SDK_ROOT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f09c5f5-7e0f-44cb-88a4-2592105ab9ea
📒 Files selected for processing (2)
.github/workflows/test.ymlexample-app/scripts/run-maestro-android-ci.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d63d11b4d
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
example-app/scripts/run-maestro-android-ci.sh (1)
165-176: Fail fast if the background emulator dies during startup.This loop only watches
adb devices, so a bad AVD name or early emulator crash still burns the full 120-second timeout before returning a generic error. Checking the PID here and pointing callers to"$EMULATOR_LOG"would make startup failures much easier to diagnose.Suggested change
for _ in $(seq 1 60); do + if ! kill -0 "$EMULATOR_PID" >/dev/null 2>&1; then + echo "Android emulator exited before it became available. See $EMULATOR_LOG." >&2 + exit 1 + fi + BOOTED_DEVICE_ID="$("$ADB_BIN" devices | awk 'NR > 1 && $2 == "device" { print $1; exit }')" if [[ -n "$BOOTED_DEVICE_ID" ]]; then break fi sleep 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/scripts/run-maestro-android-ci.sh` around lines 165 - 176, The loop waiting for BOOTED_DEVICE_ID only polls adb and can hang if the emulator process dies; update the startup loop (the block using BOOTED_DEVICE_ID, ADB_BIN, AVD_NAME) to also verify the emulator process is still running (check the emulator PID you captured when launching the AVD) on each iteration, and if the PID no longer exists, immediately print a clear error that includes the path in "$EMULATOR_LOG" and exit non‑zero; ensure the message references AVD_NAME and include the emulator log tail to aid debugging so callers don't wait the full timeout for a failed emulator start.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example-app/scripts/run-maestro-android-ci.sh`:
- Around line 101-119: select_avd_name currently picks any existing AVD (or
Pixel_9a) without verifying it matches the requested system image; update
select_avd_name to (1) if MAESTRO_ANDROID_AVD is set, validate that AVD exists
and matches MAESTRO_ANDROID_API/MAESTRO_ANDROID_TAG/MAESTRO_ANDROID_ABI and
return it only if it matches, otherwise error/fallback; (2) when auto-selecting
from the emulator -list-avds output, iterate candidates and for each candidate
read its AVD metadata (e.g. ~/.android/avd/<name>.avd/config.ini or avdmanager
output) and only accept candidates whose api/tag/abi match
MAESTRO_ANDROID_API/MAESTRO_ANDROID_TAG/MAESTRO_ANDROID_ABI; (3) if no matching
AVD is found, call create_default_avd; keep references to select_avd_name,
create_default_avd, EMULATOR_BIN, MAESTRO_ANDROID_AVD, MAESTRO_ANDROID_API,
MAESTRO_ANDROID_TAG and MAESTRO_ANDROID_ABI so reviewers can locate the changes.
- Around line 33-36: The script eagerly resolves EMULATOR_BIN, SDKMANAGER_BIN,
and AVDMANAGER_BIN causing failures under set -e even when only adb is needed;
change resolution/validation so ADB_BIN remains resolved and validated early but
move the find_sdk_tool substitutions and require_executable checks for
EMULATOR_BIN, SDKMANAGER_BIN, and AVDMANAGER_BIN into the branch that
launches/creates an emulator (the code path that currently begins around where
the script decides to launch/create an AVD), leaving the early
reuse/already-booted device path to only rely on ADB_BIN and its
require_executable call; update references to EMULATOR_BIN, SDKMANAGER_BIN, and
AVDMANAGER_BIN so they are defined before use in the launch/create branch (and
remove their unconditional resolution at top).
---
Nitpick comments:
In `@example-app/scripts/run-maestro-android-ci.sh`:
- Around line 165-176: The loop waiting for BOOTED_DEVICE_ID only polls adb and
can hang if the emulator process dies; update the startup loop (the block using
BOOTED_DEVICE_ID, ADB_BIN, AVD_NAME) to also verify the emulator process is
still running (check the emulator PID you captured when launching the AVD) on
each iteration, and if the PID no longer exists, immediately print a clear error
that includes the path in "$EMULATOR_LOG" and exit non‑zero; ensure the message
references AVD_NAME and include the emulator log tail to aid debugging so
callers don't wait the full timeout for a failed emulator start.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 192007c4-3791-4b1a-8041-1b036bdb2dd7
📒 Files selected for processing (1)
example-app/scripts/run-maestro-android-ci.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf43da4477
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb909b5ed4
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
211-212:⚠️ Potential issue | 🟠 Major
maestro_iosis still blocked by an intentionally skipped dependency.
guard_swiftpm_versiononly runs whenPackage.swiftchanges, butmaestro_iosstill lists it inneedswithout analways()-guarded job condition. GitHub skips downstream jobs when a required dependency is skipped unless the dependent job keeps evaluating itsif, so normal iOS PRs can still lose Maestro coverage here. (docs.github.com)Suggested fix
maestro_ios: timeout-minutes: 60 runs-on: macOS-latest needs: [changes, build_ios, guard_swiftpm_version] - if: needs.changes.outputs.maestro_ios == 'true' + if: >- + ${{ + always() && + needs.changes.outputs.maestro_ios == 'true' && + needs.build_ios.result == 'success' && + (needs.guard_swiftpm_version.result == 'success' || needs.guard_swiftpm_version.result == 'skipped') + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 211 - 212, The maestro iOS job is blocked because it lists guard_swiftpm_version in its needs array even though that guard job can be skipped; remove guard_swiftpm_version from the needs list (the array that currently reads needs: [changes, build_ios, guard_swiftpm_version]) so the downstream job no longer treats the guard as a required dependency, or alternatively ensure the guard_swiftpm_version job uses an if that includes always() (e.g., if: always() || <existing condition>) so it never gets skipped and won't block evaluation of the maestro job that checks needs.changes.outputs.maestro_ios.
🧹 Nitpick comments (1)
example-app/scripts/run-maestro-android.sh (1)
28-39: Keep the Maestro CLI arguments in one place.The helper already centralizes the common flags, but the manual-driver path repeats the same flow and report arguments below. Let
run_maestro_testsaccept passthrough flags so the two paths cannot drift.Suggested refactor
run_maestro_tests() { MAESTRO_CLI_NO_ANALYTICS=1 \ maestro test \ + "$@" \ -p android \ --device "$DEVICE_ID" \ --format junit \ --output "$REPORT_PATH" \ --test-output-dir "$ARTIFACT_DIR" \ "$ROOT_DIR/.maestro/runtime-checks.yaml" \ "$ROOT_DIR/.maestro/image-conversion.yaml" \ "$ROOT_DIR/.maestro/unsupported-reencode-android.yaml" } @@ -MAESTRO_CLI_NO_ANALYTICS=1 \ - maestro test \ - --no-reinstall-driver \ - -p android \ - --device "$DEVICE_ID" \ - --format junit \ - --output "$REPORT_PATH" \ - --test-output-dir "$ARTIFACT_DIR" \ - "$ROOT_DIR/.maestro/runtime-checks.yaml" \ - "$ROOT_DIR/.maestro/image-conversion.yaml" \ - "$ROOT_DIR/.maestro/unsupported-reencode-android.yaml" +run_maestro_tests --no-reinstall-driverAlso applies to: 91-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example-app/scripts/run-maestro-android.sh` around lines 28 - 39, The run_maestro_tests function duplicates common Maestro CLI flags causing drift; change run_maestro_tests to accept passthrough flags (e.g., add "$@" to the maestro invocation in run_maestro_tests) and remove the duplicated flags from the manual-driver invocation so both paths call run_maestro_tests with the necessary extra args; update any callers (the manual-driver path that currently repeats the flow/report/test-output args) to pass those flags through to run_maestro_tests instead of repeating them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 44-55: The ios and maestro_ios path filters currently omit files
that can change the iOS pipeline (specifically .github/workflows/test.yml) and
the example app's package deps (example-app/package.json); update the filters
named ios and maestro_ios to include '.github/workflows/test.yml' and
'example-app/package.json' so changes to workflow logic or example-app
dependencies will set the filter output to true and trigger the iOS jobs.
---
Duplicate comments:
In @.github/workflows/test.yml:
- Around line 211-212: The maestro iOS job is blocked because it lists
guard_swiftpm_version in its needs array even though that guard job can be
skipped; remove guard_swiftpm_version from the needs list (the array that
currently reads needs: [changes, build_ios, guard_swiftpm_version]) so the
downstream job no longer treats the guard as a required dependency, or
alternatively ensure the guard_swiftpm_version job uses an if that includes
always() (e.g., if: always() || <existing condition>) so it never gets skipped
and won't block evaluation of the maestro job that checks
needs.changes.outputs.maestro_ios.
---
Nitpick comments:
In `@example-app/scripts/run-maestro-android.sh`:
- Around line 28-39: The run_maestro_tests function duplicates common Maestro
CLI flags causing drift; change run_maestro_tests to accept passthrough flags
(e.g., add "$@" to the maestro invocation in run_maestro_tests) and remove the
duplicated flags from the manual-driver invocation so both paths call
run_maestro_tests with the necessary extra args; update any callers (the
manual-driver path that currently repeats the flow/report/test-output args) to
pass those flags through to run_maestro_tests instead of repeating them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c59549e3-3d39-4210-99ff-44deae99e684
📒 Files selected for processing (3)
.github/workflows/test.ymlexample-app/README.mdexample-app/scripts/run-maestro-android.sh
✅ Files skipped from review due to trivial changes (1)
- example-app/README.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/test.yml (2)
44-55:⚠️ Potential issue | 🟠 MajoriOS path filters missing workflow and example-app package files.
The
iosandmaestro_iosfilters omit.github/workflows/test.ymlandexample-app/package.json, which are included in theandroidandmaestro_androidfilters. A PR that exclusively changes workflow logic or example app dependencies will skip all iOS jobs even if the changes affect iOS behavior.Suggested fix
ios: + - '.github/workflows/test.yml' - 'ios/**' - 'Package.swift' - 'CapgoCapacitorFfmpeg.podspec' - 'src/**' - 'scripts/**' - 'package.json' - 'bun.lock' - 'example-app/ios/**' - 'example-app/src/**' + - 'example-app/package.json' - 'example-app/scripts/run-maestro-ios.sh' - 'example-app/.maestro/**' maestro_ios: + - '.github/workflows/test.yml' - 'ios/**' - 'Package.swift' - 'CapgoCapacitorFfmpeg.podspec' - 'src/**' - 'scripts/**' - 'package.json' - 'bun.lock' - 'example-app/ios/**' - 'example-app/src/**' + - 'example-app/package.json' - 'example-app/scripts/run-maestro-ios.sh' - 'example-app/.maestro/**'Also applies to: 68-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 44 - 55, The iOS path filter blocks (the `ios` and `maestro_ios` filter lists) are missing `.github/workflows/test.yml` and `example-app/package.json`; update those two filter arrays to include both `.github/workflows/test.yml` and `example-app/package.json` so they mirror the `android`/`maestro_android` filters and ensure iOS jobs run when workflow or example-app package changes.
211-212:⚠️ Potential issue | 🟠 Major
maestro_ioswill be skipped whenguard_swiftpm_versionis skipped.When
Package.swiftisn't modified,guard_swiftpm_versionis skipped, which causes GitHub Actions to skipmaestro_iosby default—even if themaestro_iosfilter matches. This prevents Maestro tests from running on iOS-only PRs that don't touchPackage.swift.Suggested fix using `always()` with explicit result checks
maestro_ios: timeout-minutes: 60 runs-on: macOS-latest needs: [changes, build_ios, guard_swiftpm_version] - if: needs.changes.outputs.maestro_ios == 'true' + if: >- + always() && + needs.changes.outputs.maestro_ios == 'true' && + needs.build_ios.result == 'success' && + (needs.guard_swiftpm_version.result == 'success' || needs.guard_swiftpm_version.result == 'skipped')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 211 - 212, The maestro_ios job is being skipped when guard_swiftpm_version is skipped; make guard_swiftpm_version run with always() so it always produces a result, and change the maestro_ios if-check to explicitly allow runner when changes.maestro_ios == 'true' irrespective of guard being skipped by checking needs.guard_swiftpm_version.result; specifically, set the guard_swiftpm_version job's if to always() (so it runs/exits deterministically) and update the maestro_ios if expression to something like: evaluate needs.changes.outputs.maestro_ios == 'true' && (needs.guard_swiftpm_version.result == 'success' || needs.guard_swiftpm_version.result == 'skipped') so maestro_ios only depends on the changes output while tolerating guard being skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/test.yml:
- Around line 44-55: The iOS path filter blocks (the `ios` and `maestro_ios`
filter lists) are missing `.github/workflows/test.yml` and
`example-app/package.json`; update those two filter arrays to include both
`.github/workflows/test.yml` and `example-app/package.json` so they mirror the
`android`/`maestro_android` filters and ensure iOS jobs run when workflow or
example-app package changes.
- Around line 211-212: The maestro_ios job is being skipped when
guard_swiftpm_version is skipped; make guard_swiftpm_version run with always()
so it always produces a result, and change the maestro_ios if-check to
explicitly allow runner when changes.maestro_ios == 'true' irrespective of guard
being skipped by checking needs.guard_swiftpm_version.result; specifically, set
the guard_swiftpm_version job's if to always() (so it runs/exits
deterministically) and update the maestro_ios if expression to something like:
evaluate needs.changes.outputs.maestro_ios == 'true' &&
(needs.guard_swiftpm_version.result == 'success' ||
needs.guard_swiftpm_version.result == 'skipped') so maestro_ios only depends on
the changes output while tolerating guard being skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 645d4a41-e29b-4904-94cb-43b123613595
📒 Files selected for processing (1)
.github/workflows/test.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b22e283a8d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4557c645c1
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 147009bd41
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66e82a3818
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0f44c1d12
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59399e23a6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a456aaec5f
ℹ️ 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".
1 participant
What
Why
cd example-app && ./scripts/run-maestro-android-ci.shwas failing withyes: standard output: Broken pipeFailed to install a compatible Android system imageHow
example-app/scripts/run-maestro-android-ci.shto use explicit Android SDK tool pathsgoogle_apisAVD automaticallyyes | sdkmanagerstyle license piping with a boundedprintfflowTesting
cd example-app && MAESTRO_ANDROID_PREPARE_ONLY=1 ./scripts/run-maestro-android-ci.shNot Tested
Summary by CodeRabbit
New Features
Documentation
Chores