Add Maestro E2E suite for Android Chrome (physical device)#657
Add Maestro E2E suite for Android Chrome (physical device)#657theDavidCoen wants to merge 7 commits into
Conversation
Coordinate-based flows for onboarding, smoke, and settings scenarios; config via gitignored config.yaml from config.yaml.example. Recorded on Xiaomi 13T Pro (1220×2712, Android 16, 3-button nav). Co-authored-by: Cursor <cursoragent@cursor.com>
…iles. Viewport and chrome-zone mapping from device-profiles.yaml; detect and init profiles via adb. Document workflow in maestro/README.md. Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThis PR establishes a comprehensive Maestro Android E2E test infrastructure for Arkade Wallet. It introduces local configuration, tap recording utilities, automated report generation, device profile adaptation, foundational synchronization primitives, reusable interaction subflows, and a complete catalog of 15 E2E test flows with documented secret management and scenario orchestration. ChangesMaestro Android E2E Suite
🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — #657
Scope: Purely additive — 88 new files under maestro/, zero modifications to existing code. No CI/workflow changes. No protocol-critical code (no VTXO, signing, forfeit, round, or exit path changes). Low integration risk.
Shell scripts and Python code are well-written overall — set -euo pipefail everywhere, proper variable quoting, no shell=True in subprocess calls, yaml.safe_load used correctly. Good work there.
Requesting changes for two issues:
🔴 CRITICAL — Hardcoded nsec private key committed to source
maestro/complete-tests/flows/02_startup_restore.yaml and maestro/flows/01_startup_restore.yaml:
- inputText: "nsec197q5zzwnj5569azegmgwjnkzn5za3y0nm4y3khkyf2u84wgud28q562vpm"This is a bech32-encoded Nostr private key in plain text, committed to a public repo. Even if it's a throwaway test key with no funds today, this is a hard no:
- Anyone who forks the repo gets the key
- Automated secret scanners will flag it
- If someone accidentally loads this key into a funded wallet, funds are exposed
Fix: Move to config.yaml (already gitignored) as WALLET_NSEC env var. Reference as ${WALLET_NSEC} in the flow. Document the placeholder in config.yaml.example. The infrastructure for this already exists — WALLET_URL and WALLET_PASSWORD are already handled this way.
🟡 MEDIUM — Hardcoded password 123qwerty. in complete-tests
maestro/complete-tests/flows/13_unlock_wallet.yaml (3 occurrences):
- inputText: "123qwerty."Inconsistent with the existing pattern: maestro/subflows/ correctly uses ${WALLET_PASSWORD} from config. The complete-tests flows should do the same.
Fix: Replace all "123qwerty." literals with ${WALLET_PASSWORD}.
ℹ️ Minor notes (non-blocking)
-
maestro/scripts/build-report.py—ET.parse(path)uses stdlib ElementTree which is safe against XXE in modern CPython, butdefusedxmlwould be more defensive if the project ever consumes XML from untrusted sources. Low priority. -
maestro/scripts/adapt-device.py—--writeoverwrites flow YAML files in place. The explicit flag requirement is good design, but consider adding a confirmation prompt or always creating.bakfiles by default (currently opt-in via--backup). -
88 files is a large PR. Consider whether future changes to this suite could be split (e.g., scripts + infra in one PR, flows in another) to make review more tractable.
Verdict: Fix the hardcoded nsec (critical) and the hardcoded password (medium), then this is good to merge. The test infrastructure itself is solid — well-documented, clean shell scripting, sensible gitignore, and a thoughtful device-adaptation pipeline.
🤖 Reviewed by Arkana
Use WALLET_NSEC and WALLET_PASSWORD env vars in flows; validate in run-scenario for restore/unlock scenarios. Remove hardcoded secrets from YAML. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Arkana Follow-up Review — 5ff4c067
Previous findings addressed ✅
🔴 Hardcoded nsec — Fixed. Both maestro/flows/01_startup_restore.yaml and maestro/complete-tests/flows/02_startup_restore.yaml now use ${WALLET_NSEC}.
🟡 Hardcoded password — Fixed. All three "123qwerty." literals in maestro/complete-tests/flows/13_unlock_wallet.yaml replaced with ${WALLET_PASSWORD}.
Implementation review of new code:
The refactoring in maestro/scripts/maestro-config.sh is clean — read_config_env, export_maestro_env, maestro_env_args, and per-scenario validation in run-scenario.sh (require_wallet_nsec for restore scenarios, require_wallet_password for unlock scenarios). Good separation.
One minor nit:
maestro/studio.sh:22 — -e flags are passed before the studio subcommand:
exec maestro -p android --device "$DEVICE" --config "$CONFIG" "${MAESTRO_ENV[@]}" studiomaestro studio may not accept -e env flags (they're a maestro test feature). This could error or silently drop the vars. Since export_maestro_env already exports them as shell env vars (which Maestro should pick up from the environment), you might want to skip MAESTRO_ENV in studio.sh only — or verify maestro studio -e works on your setup. Non-blocking.
No protocol-critical code. LGTM.
🤖 Reviewed by Arkana
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
maestro/complete-tests/subflows/keyboard_enter_sats.yaml (1)
1-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete implementation: no keyboard taps present.
The file is named
keyboard_enter_sats.yamland the comment mentions "coordinate taps," but the file only contains a wait for the "Save" button to appear—no tap actions are defined. Either the keyboard tap sequence is missing or this file should be renamed to reflect its actual behavior (e.g.,wait_for_save_button.yaml).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/keyboard_enter_sats.yaml` around lines 1 - 8, The YAML currently only waits for the "Save" button via the extendedWaitUntil step but is missing the keyboard coordinate tap sequence implied by the filename keyboard_enter_sats.yaml; either implement the missing taps or rename the file to match behavior. To fix, add the keyboard tap steps (e.g., a sequence of tap/tapCoordinate actions that press the numeric keys and then confirm) after the extendedWaitUntil step and before any final action, referencing the same element coordinates or input field used in other keyboard subflows; alternatively, if no keyboard interaction is needed, rename the file and update any references so the name reflects the wait-for-save behavior and remove expectations of keyboard taps. Ensure the changes touch the step sequence around extendedWaitUntil and any calling flows that expect keyboard_enter_sats.yaml.maestro/complete-tests/subflows/scroll_quick_actions_into_view.yaml (1)
1-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFilename does not match behavior.
The file is named
scroll_quick_actions_into_view.yamlbut it only executes a 1-second wait with no scroll action. Either the scroll action is missing, or the file should be renamed to reflect its actual behavior (e.g.,wait_quick_actions_settle.yaml).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/scroll_quick_actions_into_view.yaml` around lines 1 - 4, The file scroll_quick_actions_into_view.yaml currently only sets appId: com.android.chrome and runs ../../subflows/wait_1s.yaml, so its name is misleading; either add the missing scroll behavior or rename the file. Fix by either (A) implementing the intended scroll action(s) into this flow (e.g., replace or augment runFlow with a subflow that performs the quick-actions scroll or add explicit scroll steps) referencing this file's existing runFlow symbol, or (B) rename the file to something accurate like wait_quick_actions_settle.yaml and update any callers to reference the new name. Ensure the change keeps appId: com.android.chrome and that any references to this file elsewhere are updated accordingly.maestro/complete-tests/flows/01_fresh_wallet.yaml (1)
9-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlow can false-pass because it never verifies wallet creation succeeded.
Lines 9–35 only execute taps/waits, so this scenario may pass even when onboarding fails. Add an explicit post-condition check.
Suggested patch
- tapOn: point: "53%, 91%" waitToSettleTimeoutMs: 800 + - runFlow: ../subflows/wait_wallet_ready_after_create.yaml + - runFlow: ../subflows/assert_on_home.yaml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/flows/01_fresh_wallet.yaml` around lines 9 - 35, The flow currently only performs taps/waits (the tapOn steps and runFlow calls) and can false-pass; add a post-condition verification after the final tap to ensure wallet creation succeeded by invoking a check (for example a new runFlow like runFlow: ../../subflows/check_wallet_created.yaml or an explicit assertion step such as waitForText/assertVisible) that looks for a definitive success indicator (success message, account ID, wallet home element, or onboarding-complete screen). Ensure the new check runs after the last tapOn step and fails the flow if the expected element/text is not present.maestro/complete-tests/flows/04_home_open_receive.yaml (1)
10-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScenario intent is unverified (
opens QR screen) and can false-pass.After opening Receive/QR, add explicit assertions for receive state and return-to-home state.
Suggested patch
- runFlow: ../subflows/tap_receive_qr.yaml - runFlow: ../../subflows/wait_2s.yaml + - runFlow: ../subflows/assert_receive_screen.yaml - runFlow: ../subflows/tap_receive_back.yaml - runFlow: ../../subflows/wait_2s.yaml + - runFlow: ../subflows/assert_on_home.yaml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/flows/04_home_open_receive.yaml` around lines 10 - 16, The "opens QR screen" step is unverified and may false-pass; after running the tap_home_receive flow (runFlow: ../subflows/tap_home_receive.yaml) insert an explicit assertion subflow that verifies the app is in the receive state (e.g., check for QR screen element/state), and after running tap_receive_back.yaml insert an assertion subflow that verifies the app returned to the home state (e.g., home screen element/state); use or create assertion subflows (e.g., assert_receive_state.yaml and assert_home_state.yaml) and place them immediately after the respective runFlow calls (keeping wait_2s where needed).
🧹 Nitpick comments (10)
maestro/README.md (2)
72-72: ⚡ Quick winDocument the security model for the hardcoded test password.
Flow 13 uses a "fixed test password" visible in the flow file. While acceptable for ephemeral test wallets, this pattern should be explicitly documented as test-only to prevent accidental reuse with real credentials.
Consider adding a sentence clarifying that test flows must never contain real passwords or seed phrases, and that the unlock flow uses a known-safe test value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/README.md` at line 72, Add a security note to the README documenting that the "fixed test password" used by the 13_unlock_wallet.yaml flow is strictly a test-only value: explicitly state that flows containing hardcoded passwords or seed phrases (e.g., 13_unlock_wallet.yaml) are for ephemeral test wallets only, must never contain real credentials, and that the unlock flow relies on a known-safe test value; include a short sentence on recommended practice (remove/replace any hardcoded secrets before using with real wallets).
182-182: 💤 Low valueConsider redacting or generalizing the device serial.
The ADB serial
INBIJVCASW5HS4YXuniquely identifies the reference device. While low-risk for a test device, consider using a placeholder like<DEVICE_SERIAL>in documentation to avoid setting a precedent of exposing device identifiers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/README.md` at line 182, The README currently exposes a concrete ADB serial value in the table cell under the "ADB serial" heading; replace the literal string "INBIJVCASW5HS4YX" with a generalized placeholder (e.g. "<DEVICE_SERIAL>" or "REDACTED") so the table entry under "ADB serial" no longer contains a unique device identifier; update the README table row where the "ADB serial" cell appears accordingly.maestro/scripts/build-report.py (1)
202-202: ⚖️ Poor tradeoffConsider documenting the 4000-character truncation limit.
Test failure details are truncated to 4000 characters to prevent excessively large Markdown reports. For long stack traces or verbose Maestro output, this may hide relevant diagnostic information. Consider either documenting this limit in a comment or making it configurable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/scripts/build-report.py` at line 202, The truncation of test failure details is hardcoded via slicing c.details[:4000]; make this limit explicit and configurable by introducing a named constant (e.g., MAX_DETAIL_LENGTH) or a CLI/env option (e.g., --max-detail-length) and use it in the build-report logic where lines.append(c.details[:4000]) occurs, and add a brief inline comment explaining why truncation is necessary and what the limit means; update any relevant help text or docs to mention the default 4000-character limit.maestro/scripts/run-scenario.sh (1)
33-83: ⚡ Quick winValidate that FLOWS array is non-empty before running maestro test.
If a scenario case returns no flows (e.g., due to a logic error or missing files),
maestro testwill be invoked with an empty flow list. While Maestro will likely fail gracefully, an explicit check would provide a clearer error message.🛡️ Proposed validation
esac ) +if [[ ${`#FLOWS`[@]} -eq 0 ]]; then + echo "error: no flows resolved for scenario: $SCENARIO" >&2 + exit 1 +fi + # shellcheck source=maestro-device.sh source "$SCRIPTS/maestro-device.sh"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/scripts/run-scenario.sh` around lines 33 - 83, The case block can produce an empty FLOWS array; after the mapfile command that populates FLOWS, add a validation step that checks the length of the FLOWS array (e.g., test ${`#FLOWS`[@]}), and if zero print a clear error including SCENARIO and COMPLETE_FLOWS context and exit non‑zero so maestro test is never invoked with an empty flow list; locate the check immediately after the mapfile -t FLOWS < <(...) block and reference the FLOWS variable for the condition.maestro/scripts/maestro-config.sh (1)
17-20: ⚖️ Poor tradeoffConsider using a YAML parser for more robust config extraction.
The
sedpattern assumes a specific YAML format (two-space indent, double quotes, exact key name). While this works for the controlledconfig.yamlfile, it's fragile to formatting changes. For a simple single-value extraction, this is acceptable, but if more config values are needed in the future, consider usingyqor a Python YAML parser.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/scripts/maestro-config.sh` around lines 17 - 20, The read_wallet_url function currently uses a fragile sed regex to extract WALLET_URL; replace that logic with a proper YAML parser call (e.g., yq or a small python -c snippet using PyYAML) to robustly read the WALLET_URL value from the provided config file; update read_wallet_url to invoke the parser and return the first/only WALLET_URL, ensuring it handles different quoting, indentation and key orders and falls back gracefully if the key is missing.maestro/complete-tests/subflows/ensure_on_wallet_home.yaml (1)
7-42: ⚖️ Poor tradeoffConsider adding a catch-all for unexpected wallet states.
The flow conditionally handles "Continue anyway", "Unlock wallet", "Welcome to Arkade", and "Receive" visibility checks. If the wallet lands in an unexpected state (e.g., error screen, network failure, update prompt), the flow continues silently without validation. Consider adding a final assertion to confirm the wallet is on the home screen, or a catch-all condition to fail explicitly.
Optional: Add final home screen assertion
- runFlow: scroll_home_to_top.yaml - runFlow: ../../subflows/wait_2s.yaml + +# Final validation: ensure we're on wallet home. +- assertVisible: "^Receive$"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/ensure_on_wallet_home.yaml` around lines 7 - 42, The flow lacks a catch-all for unexpected wallet states; after the existing runFlow blocks (including scroll_home_to_top.yaml and before/after reset_to_wallet_home.yaml) add a final validation step that asserts the wallet home is visible (e.g., check visible "Receive" or another home-unique selector) and fail explicitly if not present, or add a fallback runFlow that captures/logs unexpected screens and calls reset_to_wallet_home.yaml before proceeding; update the runFlow sequence to include this final assertion/fallback so ensure_on_wallet_home.yaml always verifies home state.maestro/complete-tests/subflows/keyboard_tap_1000.yaml (1)
1-11: ⚡ Quick winConsider adding delays between keyboard taps.
Similar to
keyboard_tap_100.yaml, these rapid taps might miss if the keyboard doesn't register them quickly enough. Adding short delays (e.g., 100-200ms) between taps would improve reliability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/keyboard_tap_1000.yaml` around lines 1 - 11, The sequence of consecutive tapOn steps in keyboard_tap_1000.yaml (the repeated tapOn entries at points "16%, 55%" and "50%, 84%") can be missed due to too-rapid input; insert short delay/wait steps (100–200ms) between each tapOn step to give the keyboard time to register taps. Locate the repeated tapOn steps and add a wait/sleep action (using your project's existing wait step name) between each tapOn call so the flow becomes tapOn → wait(100-200ms) → tapOn → wait → tapOn etc., ensuring timing is consistent with keyboard_tap_100.yaml’s approach.maestro/complete-tests/subflows/scroll_home_to_top.yaml (1)
3-7: ⚡ Quick winComment wording may be misleading.
The comment states "Pull content down" but the swipe gesture moves from 32% to 72% vertically (downward finger motion), which typically scrolls content upward. This appears to be scrolling to the top of the page (bringing top content into view). Consider clarifying the comment to say "Scroll to top" or "Bring quick actions into view from above" to match the actual gesture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/scroll_home_to_top.yaml` around lines 3 - 7, The inline comment is misleading: the swipe block (swipe: start: "50%, 32%" end: "50%, 72%" duration: 450) performs a downward finger motion that scrolls the page content upward, so update the comment to accurately describe the effect (for example "Scroll to top to bring quick actions into view" or "Scroll content up to reveal quick actions") to match the actual gesture.maestro/complete-tests/subflows/keyboard_tap_100.yaml (1)
1-9: ⚡ Quick winConsider adding delays between keyboard taps.
Similar to the backspace subflow, these rapid taps might miss if the keyboard doesn't register them quickly enough. Adding short delays (e.g., 100-200ms) between taps would improve reliability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/keyboard_tap_100.yaml` around lines 1 - 9, The three rapid tapOn steps (points "16%, 55%" and two at "50%, 84%") should have short delays inserted between them to improve reliability; update the YAML to add a delay/wait action (e.g., 100–200ms) after the first tapOn and again after the second tapOn so the keyboard has time to register each tap, keeping the existing tapOn entries and points intact.maestro/complete-tests/subflows/keyboard_backspace_x4.yaml (1)
3-7: ⚡ Quick winAdd delays between rapid keyboard taps across all keyboard subflows.
Three keyboard subflows (
keyboard_backspace_x4.yaml,keyboard_tap_100.yaml, andkeyboard_tap_1000.yaml) perform rapid successive taps without delays. If the on-screen keyboard has animation or doesn't register taps immediately, some inputs may be missed. Adding a short delay (e.g., 100-200ms) between each tap would improve reliability across all keyboard interaction subflows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/keyboard_backspace_x4.yaml` around lines 3 - 7, The keyboard subflows (keyboard_backspace_x4.yaml, keyboard_tap_100.yaml, keyboard_tap_1000.yaml) perform consecutive tapOn actions with no pause; update each subflow's repeat block (the repeat/times and its commands containing tapOn) to insert a short delay (about 100–200ms) between each tap — e.g., add a delay/wait command immediately after the tapOn entry inside the repeat so each iteration waits before the next tap, ensuring the keyboard has time to animate/register input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@maestro/complete-tests/flows/02_startup_restore.yaml`:
- Line 51: The flow currently contains a hardcoded private key in the inputText
field of 02_startup_restore.yaml; remove the literal and replace inputText with
a runtime-injected secret reference (e.g., read from an environment variable or
secret manager) so the key is not stored in source. Update the flow to reference
a placeholder like inputText: "${PRIVATE_KEY}" (or the project's secret
injection syntax), ensure the runtime/test harness (where the flow is executed)
sets PRIVATE_KEY from environment/config or vault, and update any test fixtures
or CI to supply the secret via encrypted vars rather than committing the key.
In `@maestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yaml`:
- Around line 8-13: The flow currently opens the Home Swap sheet (runFlow:
../subflows/tap_home_swap.yaml) and immediately dismisses it (tapOn: point "50%,
88%") without verifying content; insert an assertion step between opening the
sheet and tapping to dismiss that checks the sheet text/element (e.g.,
assertText/expectElement with the "Coming Soon" string or the sheet's unique
selector) and keep the existing wait runs (runFlow: ../../subflows/wait_2s.yaml)
as needed to allow the sheet to render; target the sequence after runFlow:
../subflows/tap_home_swap.yaml and before tapOn to ensure the "Coming Soon"
content is present.
In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml`:
- Line 7: The visible guard ".*wants to see.*" is English-specific and causes
branches to be skipped on non-English UIs; replace this `visible` check in the
dismiss clipboard permission subflow (the property currently set to ".*wants to
see.*") with a locale-agnostic condition — e.g., match a stable element
id/attribute or dialog role/class (resource-id, accessibility-id, or a regex
matching punctuation/structure like ".*clipboard.*" or matching the dialog
container via selector) or add localized alternatives for each supported locale
so the modal-handling branch triggers regardless of UI language.
In `@maestro/complete-tests/subflows/ensure_on_wallet_home.yaml`:
- Line 23: The unlock flow uses inputText: ${WALLET_PASSWORD} in the "Unlock
wallet" branch but doesn't guard empty envs; add a fail-fast check so the runner
or the ensure_on_wallet_home.yaml unlock step explicitly errors when
WALLET_PASSWORD is empty (or alternately skip the input and assert wallet is
unlocked). Locate the "Unlock wallet" branch and the inputText:
${WALLET_PASSWORD} occurrence and either (a) add a pre-step that verifies
WALLET_PASSWORD is non-empty and fails with a clear message, or (b) wrap the
existing unlock step with a condition that only runs if WALLET_PASSWORD is set
and otherwise raises a clear error/abort so tests don’t silently proceed.
In `@maestro/complete-tests/subflows/scroll_settings_down_2.yaml`:
- Around line 5-8: The swipe in the subflow scroll_settings_down_2 is inverted:
the current start "12%, 46%" to end "7%, 77%" moves downward (y increases) but
the other scroll_settings_down_* flows use an upward swipe; fix by making the
end y smaller than the start y so the gesture scrolls up (either swap the
start/end coordinates or adjust the end y to a value less than 46%), updating
the swipe block in scroll_settings_down_2 accordingly.
In `@maestro/config.yaml.example`:
- Line 19: Update the default test configuration to disable device animations by
changing the disableAnimations setting to true in maestro/config.yaml.example;
modify the disableAnimations: false entry to disableAnimations: true (or make it
configurable via an env var) and add a brief comment noting to set it to false
only when testing transition/animation-specific behavior so tests remain
reliable.
In `@maestro/flows/01_startup_restore.yaml`:
- Line 8: Replace the incorrect cased runner path string "./Maestro/run.sh
flows/01_startup_restore.yaml" with the correct "./maestro/run.sh
flows/01_startup_restore.yaml" in the inline hint; search for the exact token
"./Maestro/run.sh" in 01_startup_restore.yaml and update it to use lowercase
"maestro" so the command works on case-sensitive filesystems.
- Line 52: The flow file contains a hardcoded secret in the inputText field;
remove the literal and replace it with a runtime-injected secret (e.g., a
placeholder variable) that your loader resolves from a gitignored config or
environment variable (e.g., RESTORE_PRIVATE_KEY) at runtime; update any code
that parses 01_startup_restore.yaml to read that env/config key instead of the
literal and ensure the real private key is provided via CI secret store, local
gitignored .env/config, or a secrets manager so no credentials are committed.
In `@maestro/record-taps.sh`:
- Around line 19-21: The WM/W/H extraction can yield empty or non-numeric
values; validate WM, W, and H after extracting them (symbols: WM, W, H) and
before invoking the Python parser: check WM is non-empty and that W and H match
numeric-only regex (e.g. digits), and if validation fails log a clear error
(include WM contents and DEVICE) and exit non-zero or set safe defaults; update
the script to perform this numeric validation and early exit so the Python
consumer always receives valid integers.
In `@maestro/scripts/adapt-device.py`:
- Around line 128-136: The parse_wm_size function currently returns the first
"size:" line it finds which may be the Physical size; update parse_wm_size to
prefer an "Override size" line when present by scanning all lines and selecting
the token from a line that contains "override" (case-insensitive) first,
otherwise fall back to the first physical "size" line; keep the existing parsing
logic (split on ":" then "x" and int conversion) and retain the sys.exit error
behavior if no valid size is found.
- Around line 196-204: The chrome-zone remap currently scales x/y by full screen
sizes (src_w/src_h), ignoring differing chrome cutoffs; update the branch in
adapt-device.py that handles in_chrome_zone to map coordinates relative to the
source chrome bounds and then project into the destination chrome bounds using
src_chrome and dst_chrome values rather than raw screen dims. Concretely,
compute the source chrome region min/max (e.g., nav/status/header bounds from
src_chrome), convert x/y to a percentage inside that source chrome region, then
interpolate that percentage into the destination chrome region using dst_chrome
min/max and clamp_pct; keep using clamp_pct and existing variables
(in_chrome_zone, src_chrome, dst_chrome, clamp_pct, src/dst screen sizes) to
locate and replace the current simple src_w/src_h scaling.
- Around line 96-114: adb_shell and adb_devices call subprocess.run without
timeouts and can block indefinitely; add a timeout argument (e.g., timeout=30 or
configurable constant) to both subprocess.run calls in the functions adb_shell
and adb_devices, and catch subprocess.TimeoutExpired to call sys.exit with a
clear error message including the command and that it timed out; keep existing
capture_output, text and check flags and ensure the error handling in adb_shell
still exits with stderr when non-zero returncode.
In `@maestro/scripts/parse-taps.py`:
- Line 73: The out.append call uses an unnecessary f-string: change the call
that appends "- tapOn:" (the out.append(f"- tapOn:") expression referencing the
out list and its append method) to use a regular string literal without the
f-prefix so the code becomes out.append("- tapOn:") to remove the redundant
formatting marker.
- Around line 87-89: The script currently calls int() directly on sys.argv
entries when building touch_x, touch_y and calling parse(Path(sys.argv[1]),
int(sys.argv[2]), int(sys.argv[3]), ...) which will raise ValueError on bad
input; wrap these conversions in a try/except (or extract a small helper like
validate_int(name, value, default=None)) to validate numeric input, catch
ValueError, print a clear user-facing error message indicating which argument is
invalid (e.g., width, height, touch_x, touch_y), and exit with a non-zero status
so parse() only receives valid integers; update the code paths that set touch_x,
touch_y and the int() calls for parse to use this validated conversion.
---
Outside diff comments:
In `@maestro/complete-tests/flows/01_fresh_wallet.yaml`:
- Around line 9-35: The flow currently only performs taps/waits (the tapOn steps
and runFlow calls) and can false-pass; add a post-condition verification after
the final tap to ensure wallet creation succeeded by invoking a check (for
example a new runFlow like runFlow: ../../subflows/check_wallet_created.yaml or
an explicit assertion step such as waitForText/assertVisible) that looks for a
definitive success indicator (success message, account ID, wallet home element,
or onboarding-complete screen). Ensure the new check runs after the last tapOn
step and fails the flow if the expected element/text is not present.
In `@maestro/complete-tests/flows/04_home_open_receive.yaml`:
- Around line 10-16: The "opens QR screen" step is unverified and may
false-pass; after running the tap_home_receive flow (runFlow:
../subflows/tap_home_receive.yaml) insert an explicit assertion subflow that
verifies the app is in the receive state (e.g., check for QR screen
element/state), and after running tap_receive_back.yaml insert an assertion
subflow that verifies the app returned to the home state (e.g., home screen
element/state); use or create assertion subflows (e.g.,
assert_receive_state.yaml and assert_home_state.yaml) and place them immediately
after the respective runFlow calls (keeping wait_2s where needed).
In `@maestro/complete-tests/subflows/keyboard_enter_sats.yaml`:
- Around line 1-8: The YAML currently only waits for the "Save" button via the
extendedWaitUntil step but is missing the keyboard coordinate tap sequence
implied by the filename keyboard_enter_sats.yaml; either implement the missing
taps or rename the file to match behavior. To fix, add the keyboard tap steps
(e.g., a sequence of tap/tapCoordinate actions that press the numeric keys and
then confirm) after the extendedWaitUntil step and before any final action,
referencing the same element coordinates or input field used in other keyboard
subflows; alternatively, if no keyboard interaction is needed, rename the file
and update any references so the name reflects the wait-for-save behavior and
remove expectations of keyboard taps. Ensure the changes touch the step sequence
around extendedWaitUntil and any calling flows that expect
keyboard_enter_sats.yaml.
In `@maestro/complete-tests/subflows/scroll_quick_actions_into_view.yaml`:
- Around line 1-4: The file scroll_quick_actions_into_view.yaml currently only
sets appId: com.android.chrome and runs ../../subflows/wait_1s.yaml, so its name
is misleading; either add the missing scroll behavior or rename the file. Fix by
either (A) implementing the intended scroll action(s) into this flow (e.g.,
replace or augment runFlow with a subflow that performs the quick-actions scroll
or add explicit scroll steps) referencing this file's existing runFlow symbol,
or (B) rename the file to something accurate like wait_quick_actions_settle.yaml
and update any callers to reference the new name. Ensure the change keeps appId:
com.android.chrome and that any references to this file elsewhere are updated
accordingly.
---
Nitpick comments:
In `@maestro/complete-tests/subflows/ensure_on_wallet_home.yaml`:
- Around line 7-42: The flow lacks a catch-all for unexpected wallet states;
after the existing runFlow blocks (including scroll_home_to_top.yaml and
before/after reset_to_wallet_home.yaml) add a final validation step that asserts
the wallet home is visible (e.g., check visible "Receive" or another home-unique
selector) and fail explicitly if not present, or add a fallback runFlow that
captures/logs unexpected screens and calls reset_to_wallet_home.yaml before
proceeding; update the runFlow sequence to include this final assertion/fallback
so ensure_on_wallet_home.yaml always verifies home state.
In `@maestro/complete-tests/subflows/keyboard_backspace_x4.yaml`:
- Around line 3-7: The keyboard subflows (keyboard_backspace_x4.yaml,
keyboard_tap_100.yaml, keyboard_tap_1000.yaml) perform consecutive tapOn actions
with no pause; update each subflow's repeat block (the repeat/times and its
commands containing tapOn) to insert a short delay (about 100–200ms) between
each tap — e.g., add a delay/wait command immediately after the tapOn entry
inside the repeat so each iteration waits before the next tap, ensuring the
keyboard has time to animate/register input.
In `@maestro/complete-tests/subflows/keyboard_tap_100.yaml`:
- Around line 1-9: The three rapid tapOn steps (points "16%, 55%" and two at
"50%, 84%") should have short delays inserted between them to improve
reliability; update the YAML to add a delay/wait action (e.g., 100–200ms) after
the first tapOn and again after the second tapOn so the keyboard has time to
register each tap, keeping the existing tapOn entries and points intact.
In `@maestro/complete-tests/subflows/keyboard_tap_1000.yaml`:
- Around line 1-11: The sequence of consecutive tapOn steps in
keyboard_tap_1000.yaml (the repeated tapOn entries at points "16%, 55%" and
"50%, 84%") can be missed due to too-rapid input; insert short delay/wait steps
(100–200ms) between each tapOn step to give the keyboard time to register taps.
Locate the repeated tapOn steps and add a wait/sleep action (using your
project's existing wait step name) between each tapOn call so the flow becomes
tapOn → wait(100-200ms) → tapOn → wait → tapOn etc., ensuring timing is
consistent with keyboard_tap_100.yaml’s approach.
In `@maestro/complete-tests/subflows/scroll_home_to_top.yaml`:
- Around line 3-7: The inline comment is misleading: the swipe block (swipe:
start: "50%, 32%" end: "50%, 72%" duration: 450) performs a downward finger
motion that scrolls the page content upward, so update the comment to accurately
describe the effect (for example "Scroll to top to bring quick actions into
view" or "Scroll content up to reveal quick actions") to match the actual
gesture.
In `@maestro/README.md`:
- Line 72: Add a security note to the README documenting that the "fixed test
password" used by the 13_unlock_wallet.yaml flow is strictly a test-only value:
explicitly state that flows containing hardcoded passwords or seed phrases
(e.g., 13_unlock_wallet.yaml) are for ephemeral test wallets only, must never
contain real credentials, and that the unlock flow relies on a known-safe test
value; include a short sentence on recommended practice (remove/replace any
hardcoded secrets before using with real wallets).
- Line 182: The README currently exposes a concrete ADB serial value in the
table cell under the "ADB serial" heading; replace the literal string
"INBIJVCASW5HS4YX" with a generalized placeholder (e.g. "<DEVICE_SERIAL>" or
"REDACTED") so the table entry under "ADB serial" no longer contains a unique
device identifier; update the README table row where the "ADB serial" cell
appears accordingly.
In `@maestro/scripts/build-report.py`:
- Line 202: The truncation of test failure details is hardcoded via slicing
c.details[:4000]; make this limit explicit and configurable by introducing a
named constant (e.g., MAX_DETAIL_LENGTH) or a CLI/env option (e.g.,
--max-detail-length) and use it in the build-report logic where
lines.append(c.details[:4000]) occurs, and add a brief inline comment explaining
why truncation is necessary and what the limit means; update any relevant help
text or docs to mention the default 4000-character limit.
In `@maestro/scripts/maestro-config.sh`:
- Around line 17-20: The read_wallet_url function currently uses a fragile sed
regex to extract WALLET_URL; replace that logic with a proper YAML parser call
(e.g., yq or a small python -c snippet using PyYAML) to robustly read the
WALLET_URL value from the provided config file; update read_wallet_url to invoke
the parser and return the first/only WALLET_URL, ensuring it handles different
quoting, indentation and key orders and falls back gracefully if the key is
missing.
In `@maestro/scripts/run-scenario.sh`:
- Around line 33-83: The case block can produce an empty FLOWS array; after the
mapfile command that populates FLOWS, add a validation step that checks the
length of the FLOWS array (e.g., test ${`#FLOWS`[@]}), and if zero print a clear
error including SCENARIO and COMPLETE_FLOWS context and exit non‑zero so maestro
test is never invoked with an empty flow list; locate the check immediately
after the mapfile -t FLOWS < <(...) block and reference the FLOWS variable for
the condition.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de616852-55f7-4e0c-8805-2c9720df6a56
📒 Files selected for processing (100)
maestro/.gitignoremaestro/README.mdmaestro/REPORTING.mdmaestro/complete-tests/README.mdmaestro/complete-tests/flows/01_fresh_wallet.yamlmaestro/complete-tests/flows/02_startup_restore.yamlmaestro/complete-tests/flows/03_home_quick_actions_visible.yamlmaestro/complete-tests/flows/04_home_open_receive.yamlmaestro/complete-tests/flows/05_home_open_send.yamlmaestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yamlmaestro/complete-tests/flows/07_home_open_activity.yamlmaestro/complete-tests/flows/08_home_open_settings.yamlmaestro/complete-tests/flows/09_home_tabs_and_asset_filters.yamlmaestro/complete-tests/flows/10_receive_copy_sheet.yamlmaestro/complete-tests/flows/11_send_form_address_field.yamlmaestro/complete-tests/flows/12_home_open_scan.yamlmaestro/complete-tests/flows/13_unlock_wallet.yamlmaestro/complete-tests/flows/14_home_scroll_balance_sticky.yamlmaestro/complete-tests/flows/15_settings_deep_reset_wallet.yamlmaestro/complete-tests/flows/_run_all_smoke.yamlmaestro/complete-tests/flows/_run_reset_then_create.yamlmaestro/complete-tests/subflows/assert_home_quick_actions.yamlmaestro/complete-tests/subflows/assert_on_home.yamlmaestro/complete-tests/subflows/assert_receive_screen.yamlmaestro/complete-tests/subflows/assert_send_form.yamlmaestro/complete-tests/subflows/close_copy_sheet.yamlmaestro/complete-tests/subflows/dismiss_clipboard_permission.yamlmaestro/complete-tests/subflows/ensure_on_wallet_home.yamlmaestro/complete-tests/subflows/exercise_send_form.yamlmaestro/complete-tests/subflows/go_back_to_home.yamlmaestro/complete-tests/subflows/keyboard_backspace_x4.yamlmaestro/complete-tests/subflows/keyboard_enter_sats.yamlmaestro/complete-tests/subflows/keyboard_tap_100.yamlmaestro/complete-tests/subflows/keyboard_tap_1000.yamlmaestro/complete-tests/subflows/keyboard_tap_500.yamlmaestro/complete-tests/subflows/navigate_home.yamlmaestro/complete-tests/subflows/open_receive_from_home.yamlmaestro/complete-tests/subflows/open_send_from_home.yamlmaestro/complete-tests/subflows/open_settings_from_home.yamlmaestro/complete-tests/subflows/receive_add_amount_keyboard.yamlmaestro/complete-tests/subflows/receive_copy_share_sheet.yamlmaestro/complete-tests/subflows/reset_to_wallet_home.yamlmaestro/complete-tests/subflows/scroll_home_to_top.yamlmaestro/complete-tests/subflows/scroll_quick_actions_into_view.yamlmaestro/complete-tests/subflows/scroll_settings_down_1.yamlmaestro/complete-tests/subflows/scroll_settings_down_2.yamlmaestro/complete-tests/subflows/scroll_settings_down_3.yamlmaestro/complete-tests/subflows/tap_header_activity.yamlmaestro/complete-tests/subflows/tap_header_back.yamlmaestro/complete-tests/subflows/tap_header_settings.yamlmaestro/complete-tests/subflows/tap_home_receive.yamlmaestro/complete-tests/subflows/tap_home_scan.yamlmaestro/complete-tests/subflows/tap_home_send.yamlmaestro/complete-tests/subflows/tap_home_swap.yamlmaestro/complete-tests/subflows/tap_receive_add_amount.yamlmaestro/complete-tests/subflows/tap_receive_back.yamlmaestro/complete-tests/subflows/tap_receive_copy.yamlmaestro/complete-tests/subflows/tap_receive_copy_sheet_row.yamlmaestro/complete-tests/subflows/tap_receive_qr.yamlmaestro/complete-tests/subflows/tap_send_amount.yamlmaestro/complete-tests/subflows/tap_send_back.yamlmaestro/complete-tests/subflows/tap_send_paste.yamlmaestro/complete-tests/subflows/tap_send_recipient.yamlmaestro/complete-tests/subflows/tap_send_scan.yamlmaestro/complete-tests/subflows/tap_settings_back.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_28.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_36.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_43.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_51.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_58.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_66.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_69.yamlmaestro/complete-tests/subflows/tap_settings_menu_row_83.yamlmaestro/complete-tests/subflows/tap_settings_sub_21.yamlmaestro/complete-tests/subflows/tap_settings_sub_63.yamlmaestro/complete-tests/subflows/tap_welcome_create_wallet.yamlmaestro/complete-tests/subflows/wait_receive_ready.yamlmaestro/complete-tests/subflows/wait_wallet_ready_after_create.yamlmaestro/config.yaml.examplemaestro/device-profiles.yamlmaestro/flows/01_startup_restore.yamlmaestro/record-taps.shmaestro/recordings/.gitkeepmaestro/run.shmaestro/scripts/adapt-device.pymaestro/scripts/adapt-device.shmaestro/scripts/build-report.pymaestro/scripts/maestro-config.shmaestro/scripts/maestro-device.shmaestro/scripts/open-report.shmaestro/scripts/parse-taps.pymaestro/scripts/run-scenario.shmaestro/studio.shmaestro/subflows/open_wallet_url.yamlmaestro/subflows/tap_header_back.yamlmaestro/subflows/wait_1s.yamlmaestro/subflows/wait_2s.yamlmaestro/subflows/wait_3s.yamlmaestro/subflows/wait_4s.yamlmaestro/subflows/wait_5s.yaml
| - tapOn: | ||
| point: "50%, 50%" | ||
| waitToSettleTimeoutMs: 800 | ||
| - inputText: ${WALLET_PASSWORD} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if WALLET_PASSWORD is validated or documented.
# Test: Search for WALLET_PASSWORD in config and runner scripts.
rg -n 'WALLET_PASSWORD' maestro/config.yaml.example maestro/scripts/ maestro/README.md --context 3Repository: arkade-os/wallet
Length of output: 849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the exact lines around the password input in ensure_on_wallet_home.yaml
sed -n '1,120p' maestro/complete-tests/subflows/ensure_on_wallet_home.yaml | nl -ba | sed -n '1,120p'
# 2) Find where WALLET_PASSWORD / env.WALLET_PASSWORD is referenced across maestro
rg -n 'WALLET_PASSWORD|env\.WALLET_PASSWORD' maestro -S --hidden --context 3Repository: arkade-os/wallet
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show ensure_on_wallet_home.yaml with line numbers around the unlock/password area
awk 'NR>=1 && NR<=140 {print NR ":" $0}' maestro/complete-tests/subflows/ensure_on_wallet_home.yaml | sed -n '1,140p'
# Locate WALLET_PASSWORD usage and the specific README/config documentation
rg -n 'WALLET_PASSWORD|env\.WALLET_PASSWORD' maestro -S --hidden --context 3Repository: arkade-os/wallet
Length of output: 2708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find runner scripts that mention WALLET_PASSWORD
rg -n 'WALLET_PASSWORD|env\.WALLET_PASSWORD|\$\{WALLET_PASSWORD\}' maestro -S --hidden
# If run.sh/run-scenario.sh exist, print them (or relevant parts) to see how env vars are passed to maestro
ls -la maestro | sed -n '1,200p'
find maestro -maxdepth 2 -type f -name 'run*.sh' -o -name '*scenario*' | sort
# Print key runner scripts if found
for f in $(find maestro -maxdepth 3 -type f \( -name 'run.sh' -o -name 'run-scenario.sh' -o -name '*scenario*.sh' \) | sort); do
echo "---- $f ----"
awk 'NR<=200{print NR ":" $0}' "$f"
doneRepository: arkade-os/wallet
Length of output: 6611
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show config loader to see how env vars get passed to Maestro
ls -ლა maestro/scripts
sed -n '1,200p' maestro/scripts/maestro-config.sh
# Inspect config.yaml.example fully (it's small)
sed -n '1,200p' maestro/config.yaml.example
# Inspect README section around ensure_on_wallet_home / WALLET_PASSWORD
rg -n "ensure_on_wallet_home|WALLET_PASSWORD" maestro/README.md
sed -n '140,220p' maestro/README.mdRepository: arkade-os/wallet
Length of output: 124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect config loader
ls -la maestro/scripts
sed -n '1,200p' maestro/scripts/maestro-config.sh
# Inspect config example (small)
sed -n '1,120p' maestro/config.yaml.example
# Inspect README around WALLET_PASSWORD docs
rg -n "ensure_on_wallet_home|WALLET_PASSWORD" maestro/README.md
sed -n '150,210p' maestro/README.mdRepository: arkade-os/wallet
Length of output: 4407
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for how other env vars are referenced in flows (with/without env.)
rg -n '\$\{(WALLET_URL|WALLET_PASSWORD|env\.)' maestro -S
# See how WALLET_PASSWORD is referenced (in case other flows guard it)
rg -n 'WALLET_PASSWORD' maestro/complete-tests -S
# Inspect ensure_on_wallet_home again with a bit more surrounding context
sed -n '1,120p' maestro/complete-tests/subflows/ensure_on_wallet_home.yamlRepository: arkade-os/wallet
Length of output: 1794
🌐 Web query:
Maestro inputText empty string behavior skip unlock input
💡 Result:
In Maestro, the inputText command is designed to enter a specified text string into a UI element, whether or not a field is currently focused [1][2]. There is no native "skip unlock" parameter or behavior specific to providing an empty string to inputText. If an empty string is provided to inputText (e.g., inputText: ""), the command generally does not trigger an input action or may be treated as a no-op, depending on the underlying driver implementation and target platform. Maestro's documentation does not explicitly define a "skip" behavior for this command; it is primarily used for deterministic text entry [1][3]. Regarding your interest in "skipping" or "unlocking" input, users experiencing issues with inputText often report the following behaviors rather than intentional "skip" functionality: 1. Automatic Focus: Maestro may automatically focus on an available text field (like an email or username field) if the command is executed when no field is explicitly selected [4][3]. This can sometimes lead to unexpected input if multiple fields are present [4]. 2. Flakiness and Character Skipping: On some platforms, particularly when an application experiences jank or stutter, inputText has been known to flakily skip or mangle characters [5][6][7]. This is not a "skip" feature but a synchronization issue between the test runner and the application state [5][7]. 3. Workarounds: If you are looking to bypass or conditionally control input, users often implement custom logic using JavaScript expressions within Maestro (e.g., using evalScript or repeat commands) to control when or how text is entered [6][8][9]. If you are attempting to handle scenarios where you do not want to trigger an input, the standard practice is to use conditional logic within your Maestro Flow, such as assertVisible or checking the state of the UI before calling the inputText command [8][9].
Citations:
- 1: https://docs.maestro.dev/reference/commands-available/inputtext.md
- 2: https://docs.maestro.dev/reference/commands-available/inputtext
- 3: https://deepwiki.com/mobile-dev-inc/maestro-docs/4.2-text-input-commands
- 4: inputText command automatically focuses on any initial field and begins typing mobile-dev-inc/Maestro#2386
- 5: inputText is typing too fast and skipping characters when app stutters/jank mobile-dev-inc/Maestro#395
- 6: inputText flakily skips/mangles numbers in number strings on iOS (on Expo?) mobile-dev-inc/Maestro#2382
- 7: inputText is typing too fast and skipping characters when app stutters/jank mobile-dev-inc/Maestro#395
- 8: https://docs.maestro.dev/maestro-flows/javascript/manage-data-and-states
- 9: https://docs.maestro.dev/maestro-flows/javascript/manage-data-and-states.md
Guard WALLET_PASSWORD for the “Unlock wallet” branch
maestro/complete-tests/subflows/ensure_on_wallet_home.yaml uses inputText: ${WALLET_PASSWORD} when the “Unlock wallet” screen is visible. The docs state env.WALLET_PASSWORD empty = skip unlock input (and maestro/config.yaml.example defaults to WALLET_PASSWORD: ""), so password-protected wallets will likely fail to unlock without a clear error. Add a fail-fast/explicit check in the runner or around the unlock/input step when the unlock screen appears but WALLET_PASSWORD is empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/complete-tests/subflows/ensure_on_wallet_home.yaml` at line 23, The
unlock flow uses inputText: ${WALLET_PASSWORD} in the "Unlock wallet" branch but
doesn't guard empty envs; add a fail-fast check so the runner or the
ensure_on_wallet_home.yaml unlock step explicitly errors when WALLET_PASSWORD is
empty (or alternately skip the input and assert wallet is unlocked). Locate the
"Unlock wallet" branch and the inputText: ${WALLET_PASSWORD} occurrence and
either (a) add a pre-step that verifies WALLET_PASSWORD is non-empty and fails
with a clear message, or (b) wrap the existing unlock step with a condition that
only runs if WALLET_PASSWORD is set and otherwise raises a clear error/abort so
tests don’t silently proceed.
| - swipe: | ||
| start: "12%, 46%" | ||
| end: "7%, 77%" | ||
| duration: 300 |
There was a problem hiding this comment.
Swipe direction appears inverted for a “scroll down” subflow.
On Line 7, end: "7%, 77%" moves the finger downward from Line 6 (46% -> 77%), which is opposite of the upward swipe used by scroll_settings_down_1 and scroll_settings_down_3. This can make this step scroll the wrong way and destabilize downstream settings-row taps.
Suggested fix
- swipe:
start: "12%, 46%"
- end: "7%, 77%"
+ end: "7%, 27%"
duration: 300🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/complete-tests/subflows/scroll_settings_down_2.yaml` around lines 5 -
8, The swipe in the subflow scroll_settings_down_2 is inverted: the current
start "12%, 46%" to end "7%, 77%" moves downward (y increases) but the other
scroll_settings_down_* flows use an upward swipe; fix by making the end y
smaller than the start y so the gesture scrolls up (either swap the start/end
coordinates or adjust the end y to a value less than 46%), updating the swipe
block in scroll_settings_down_2 accordingly.
| def adb_shell(device: str | None, *args: str) -> str: | ||
| cmd = ["adb"] | ||
| if device: | ||
| cmd.extend(["-s", device]) | ||
| cmd.extend(["shell", *args]) | ||
| result = subprocess.run(cmd, capture_output=True, text=True, check=False) | ||
| if result.returncode != 0: | ||
| sys.exit(f"error: adb failed: {' '.join(cmd)}\n{result.stderr}") | ||
| return result.stdout.strip().replace("\r", "") | ||
|
|
||
|
|
||
| def adb_devices() -> list[str]: | ||
| out = subprocess.run(["adb", "devices"], capture_output=True, text=True, check=True) | ||
| serials = [] | ||
| for line in out.stdout.splitlines()[1:]: | ||
| parts = line.split() | ||
| if len(parts) >= 2 and parts[1] == "device": | ||
| serials.append(parts[0]) | ||
| return serials |
There was a problem hiding this comment.
ADB subprocesses should have timeouts to avoid indefinite hangs.
Lines 101 and 108 execute external commands without a timeout. If adb stalls, the CLI can block forever and stall scenario automation.
Proposed fix
def adb_shell(device: str | None, *args: str) -> str:
@@
- result = subprocess.run(cmd, capture_output=True, text=True, check=False)
+ try:
+ result = subprocess.run(cmd, capture_output=True, text=True, check=False, timeout=15)
+ except subprocess.TimeoutExpired:
+ sys.exit(f"error: adb timed out: {' '.join(cmd)}")
+ except FileNotFoundError:
+ sys.exit("error: adb not found in PATH")
@@
def adb_devices() -> list[str]:
- out = subprocess.run(["adb", "devices"], capture_output=True, text=True, check=True)
+ try:
+ out = subprocess.run(["adb", "devices"], capture_output=True, text=True, check=False, timeout=15)
+ except subprocess.TimeoutExpired:
+ sys.exit("error: adb devices timed out")
+ except FileNotFoundError:
+ sys.exit("error: adb not found in PATH")
+ if out.returncode != 0:
+ sys.exit(f"error: adb devices failed:\n{out.stderr}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def adb_shell(device: str | None, *args: str) -> str: | |
| cmd = ["adb"] | |
| if device: | |
| cmd.extend(["-s", device]) | |
| cmd.extend(["shell", *args]) | |
| result = subprocess.run(cmd, capture_output=True, text=True, check=False) | |
| if result.returncode != 0: | |
| sys.exit(f"error: adb failed: {' '.join(cmd)}\n{result.stderr}") | |
| return result.stdout.strip().replace("\r", "") | |
| def adb_devices() -> list[str]: | |
| out = subprocess.run(["adb", "devices"], capture_output=True, text=True, check=True) | |
| serials = [] | |
| for line in out.stdout.splitlines()[1:]: | |
| parts = line.split() | |
| if len(parts) >= 2 and parts[1] == "device": | |
| serials.append(parts[0]) | |
| return serials | |
| def adb_shell(device: str | None, *args: str) -> str: | |
| cmd = ["adb"] | |
| if device: | |
| cmd.extend(["-s", device]) | |
| cmd.extend(["shell", *args]) | |
| try: | |
| result = subprocess.run(cmd, capture_output=True, text=True, check=False, timeout=15) | |
| except subprocess.TimeoutExpired: | |
| sys.exit(f"error: adb timed out: {' '.join(cmd)}") | |
| except FileNotFoundError: | |
| sys.exit("error: adb not found in PATH") | |
| if result.returncode != 0: | |
| sys.exit(f"error: adb failed: {' '.join(cmd)}\n{result.stderr}") | |
| return result.stdout.strip().replace("\r", "") | |
| def adb_devices() -> list[str]: | |
| try: | |
| out = subprocess.run(["adb", "devices"], capture_output=True, text=True, check=False, timeout=15) | |
| except subprocess.TimeoutExpired: | |
| sys.exit("error: adb devices timed out") | |
| except FileNotFoundError: | |
| sys.exit("error: adb not found in PATH") | |
| if out.returncode != 0: | |
| sys.exit(f"error: adb devices failed:\n{out.stderr}") | |
| serials = [] | |
| for line in out.stdout.splitlines()[1:]: | |
| parts = line.split() | |
| if len(parts) >= 2 and parts[1] == "device": | |
| serials.append(parts[0]) | |
| return serials |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 101-101: subprocess call: check for execution of untrusted input
(S603)
[error] 108-108: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/scripts/adapt-device.py` around lines 96 - 114, adb_shell and
adb_devices call subprocess.run without timeouts and can block indefinitely; add
a timeout argument (e.g., timeout=30 or configurable constant) to both
subprocess.run calls in the functions adb_shell and adb_devices, and catch
subprocess.TimeoutExpired to call sys.exit with a clear error message including
the command and that it timed out; keep existing capture_output, text and check
flags and ensure the error handling in adb_shell still exits with stderr when
non-zero returncode.
| def parse_wm_size(raw: str) -> tuple[int, int]: | ||
| # Physical size: 1220x2712 or Override size: ... | ||
| for line in raw.splitlines(): | ||
| if "size:" in line.lower(): | ||
| token = line.split(":")[-1].strip() | ||
| if "x" in token: | ||
| w, h = token.split("x", 1) | ||
| return int(w), int(h) | ||
| sys.exit(f"error: could not parse wm size:\n{raw}") |
There was a problem hiding this comment.
Prefer Override size over Physical size when both are present.
Line 131 returns the first size: entry. adb shell wm size can report both values; using physical size when override is active produces wrong detected dimensions and therefore wrong remapping.
Proposed fix
def parse_wm_size(raw: str) -> tuple[int, int]:
- # Physical size: 1220x2712 or Override size: ...
- for line in raw.splitlines():
- if "size:" in line.lower():
- token = line.split(":")[-1].strip()
- if "x" in token:
- w, h = token.split("x", 1)
- return int(w), int(h)
+ # Prefer override size when present; otherwise fall back to physical size.
+ override_token = None
+ physical_token = None
+ for line in raw.splitlines():
+ lower = line.lower()
+ if "override size:" in lower:
+ override_token = line.split(":", 1)[-1].strip()
+ elif "physical size:" in lower or "size:" in lower:
+ physical_token = line.split(":", 1)[-1].strip()
+ token = override_token or physical_token
+ if token and "x" in token:
+ w, h = token.split("x", 1)
+ return int(w), int(h)
sys.exit(f"error: could not parse wm size:\n{raw}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/scripts/adapt-device.py` around lines 128 - 136, The parse_wm_size
function currently returns the first "size:" line it finds which may be the
Physical size; update parse_wm_size to prefer an "Override size" line when
present by scanning all lines and selecting the token from a line that contains
"override" (case-insensitive) first, otherwise fall back to the first physical
"size" line; keep the existing parsing logic (split on ":" then "x" and int
conversion) and retain the sys.exit error behavior if no valid size is found.
| if in_chrome_zone(x, y, src_chrome) or in_chrome_zone(x, y, dst_chrome): | ||
| # Status bar, nav bar, header back — scale across full screen percentages | ||
| src_w = src["screen_width"] | ||
| src_h = src["screen_height"] | ||
| dst_w = dst["screen_width"] | ||
| dst_h = dst["screen_height"] | ||
| new_x = clamp_pct(round(x * dst_w / src_w)) if src_w else x | ||
| new_y = clamp_pct(round(y * dst_h / src_h)) if src_h else y | ||
| return new_x, new_y |
There was a problem hiding this comment.
Chrome-zone mapping ignores chrome boundary deltas between profiles.
On Line 196, chrome points are remapped only by screen_width/screen_height. For profiles with equal resolution but different chrome cutoffs (e.g., nav_bar_y_min_pct 92 → 97), a source nav-bar tap like y=94 stays 94, so it no longer lands in the target nav-bar zone.
Proposed fix
- if in_chrome_zone(x, y, src_chrome) or in_chrome_zone(x, y, dst_chrome):
- # Status bar, nav bar, header back — scale across full screen percentages
- src_w = src["screen_width"]
- src_h = src["screen_height"]
- dst_w = dst["screen_width"]
- dst_h = dst["screen_height"]
- new_x = clamp_pct(round(x * dst_w / src_w)) if src_w else x
- new_y = clamp_pct(round(y * dst_h / src_h)) if src_h else y
- return new_x, new_y
+ # Map chrome regions against their corresponding chrome ranges.
+ if y <= src_chrome["header_y_max_pct"]:
+ return (
+ map_axis(x, 0, 100, 0, 100),
+ map_axis(y, 0, src_chrome["header_y_max_pct"], 0, dst_chrome["header_y_max_pct"]),
+ )
+ if y >= src_chrome["nav_bar_y_min_pct"]:
+ return (
+ map_axis(x, 0, 100, 0, 100),
+ map_axis(y, src_chrome["nav_bar_y_min_pct"], 100, dst_chrome["nav_bar_y_min_pct"], 100),
+ )
+ if x <= src_chrome["side_x_max_pct"]:
+ return (
+ map_axis(x, 0, src_chrome["side_x_max_pct"], 0, dst_chrome["side_x_max_pct"]),
+ map_axis(y, 0, 100, 0, 100),
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/scripts/adapt-device.py` around lines 196 - 204, The chrome-zone
remap currently scales x/y by full screen sizes (src_w/src_h), ignoring
differing chrome cutoffs; update the branch in adapt-device.py that handles
in_chrome_zone to map coordinates relative to the source chrome bounds and then
project into the destination chrome bounds using src_chrome and dst_chrome
values rather than raw screen dims. Concretely, compute the source chrome region
min/max (e.g., nav/status/header bounds from src_chrome), convert x/y to a
percentage inside that source chrome region, then interpolate that percentage
into the destination chrome region using dst_chrome min/max and clamp_pct; keep
using clamp_pct and existing variables (in_chrome_zone, src_chrome, dst_chrome,
clamp_pct, src/dst screen sizes) to locate and replace the current simple
src_w/src_h scaling.
Move all flows and subflows into complete-tests/, remove duplicate top-level flows/ and subflows/, and generate gitignored secret inputs via sync-secret-subflows.py. Update paths, config, adapt-device, and README with setup instructions for WALLET_NSEC and WALLET_PASSWORD. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Arkana Incremental Review — 62bf5522
Reviewed the new commit (consolidate under complete-tests/, add sync-secret-subflows.py). Not protocol-critical — test infra only.
Previous findings addressed ✅ — secrets moved to gitignored generated subflows, inputText: ${WALLET_NSEC} replaced with runFlow: _input_wallet_nsec.yaml.
New findings
1. Bug — Wrong casing in error messages (Maestro/ vs maestro/)
maestro/scripts/maestro-config.sh lines 11-12, 57, 63, 69:
echo "error: missing Maestro/config.yaml" >&2
echo " cp Maestro/config.yaml.example Maestro/config.yaml" >&2
The directory is maestro/ (lowercase). These error messages will mislead users on case-sensitive filesystems (Linux). Same issue in:
maestro/scripts/sync-secret-subflows.py:63—"set WALLET_NSEC in Maestro/config.yaml"maestro/REPORTING.md:4-5—./Maestro/studio.sh,./Maestro/run.shmaestro/complete-tests/subflows/_input_wallet_nsec.yaml.example:3—Maestro/scripts/sync-secret-subflows.py
2. Low — config.yaml.example includes subflows in flows: glob
flows:
- complete-tests/flows/**
- complete-tests/subflows/**Subflows aren't standalone tests. If someone runs maestro test (discovery mode), Maestro will try to execute every subflow as a top-level flow — most will fail or produce nonsense results. Should be complete-tests/flows/** only.
Everything else looks clean: path consolidation is correct, .gitignore covers the generated secret files, sync-secret-subflows.py is called by all three entrypoints, YAML escaping handles \ and " properly. 👍
🤖 Reviewed by Arkana
Use lowercase maestro/ in CLI messages and docs so Linux users see correct paths. Drop complete-tests/subflows/** from config flows glob so discovery mode does not run subflows as top-level tests. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Arkana Incremental Review — 07516b5a
Reviewed the new commit since 62bf5522. Changes are strictly cosmetic:
- Path casing fix —
Maestro/→maestro/in error messages, docs, comments, and usage strings across 5 files. Matches the actual directory name. ✅ - Flows glob fix — Removed
complete-tests/subflows/**fromconfig.yaml.exampleflows:list. Correct — subflows are helper fragments, not standalone tests; including them would cause spurious failures. ✅
No protocol code touched. No logic changes. No security implications. LGTM.
🤖 Reviewed by Arkana
There was a problem hiding this comment.
Arkana Incremental Review — 07516b5a
Reviewed the new commit since 62bf5522. Changes are strictly cosmetic:
- Path casing fix —
Maestro/→maestro/in error messages, docs, comments, and usage strings across 5 files. Matches the actual directory name. ✅ - Flows glob fix — Removed
complete-tests/subflows/**fromconfig.yaml.exampleflows:list. Correct — subflows are helper fragments, not standalone tests; including them would cause spurious failures. ✅
No protocol code touched. No logic changes. No security implications. LGTM.
🤖 Reviewed by Arkana
Remove keyboard period taps left from manual recording; inputText already types the trailing dot in WALLET_PASSWORD. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@maestro/scripts/sync-secret-subflows.py`:
- Line 51: Replace the unnecessary f-string literal f"# AUTO-GENERATED by
sync-secret-subflows.py — do not edit" with a plain string "# AUTO-GENERATED by
sync-secret-subflows.py — do not edit" (remove the leading "f") wherever it
appears in the script to avoid the redundant f-string prefix and clarify intent.
- Line 37: The env value unescape only handles escaped quotes (env[key] =
value.replace('\\"', '"')) causing asymmetric handling with the writer which
escapes backslashes and quotes; fix by first unescaping backslashes then
unescaping quotes (i.e., apply the inverse of the writer's escaping in reverse
order) where env values are parsed so that literal "\" in secrets become single
backslashes before later re-escaping in the write routine.
- Around line 22-38: The read_config_env function currently regex-parses YAML;
replace that with PyYAML safe_load to robustly parse the file: import yaml, open
and safe_load the file contents, verify existence of the top-level "env"
mapping, return it as a dict[str,str] (coerce/str-cast values if needed), and
keep the same sys.exit behavior when the path is missing; update references to
ENV_RE (remove if unused) and ensure any unescaping logic for quoted values is
preserved when converting YAML values to strings.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f480c8a-fb55-4507-9615-79beb26889e6
📒 Files selected for processing (51)
maestro/.gitignoremaestro/README.mdmaestro/REPORTING.mdmaestro/complete-tests/README.mdmaestro/complete-tests/flows/01_fresh_wallet.yamlmaestro/complete-tests/flows/02_startup_restore.yamlmaestro/complete-tests/flows/04_home_open_receive.yamlmaestro/complete-tests/flows/05_home_open_send.yamlmaestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yamlmaestro/complete-tests/flows/07_home_open_activity.yamlmaestro/complete-tests/flows/08_home_open_settings.yamlmaestro/complete-tests/flows/09_home_tabs_and_asset_filters.yamlmaestro/complete-tests/flows/10_receive_copy_sheet.yamlmaestro/complete-tests/flows/11_send_form_address_field.yamlmaestro/complete-tests/flows/13_unlock_wallet.yamlmaestro/complete-tests/flows/14_home_scroll_balance_sticky.yamlmaestro/complete-tests/flows/15_settings_deep_reset_wallet.yamlmaestro/complete-tests/flows/_run_all_smoke.yamlmaestro/complete-tests/flows/_run_reset_then_create.yamlmaestro/complete-tests/subflows/_input_wallet_nsec.yaml.examplemaestro/complete-tests/subflows/assert_home_quick_actions.yamlmaestro/complete-tests/subflows/assert_on_home.yamlmaestro/complete-tests/subflows/assert_receive_screen.yamlmaestro/complete-tests/subflows/assert_send_form.yamlmaestro/complete-tests/subflows/close_copy_sheet.yamlmaestro/complete-tests/subflows/dismiss_clipboard_permission.yamlmaestro/complete-tests/subflows/ensure_on_wallet_home.yamlmaestro/complete-tests/subflows/exercise_send_form.yamlmaestro/complete-tests/subflows/open_receive_from_home.yamlmaestro/complete-tests/subflows/open_send_from_home.yamlmaestro/complete-tests/subflows/open_settings_from_home.yamlmaestro/complete-tests/subflows/open_wallet_url.yamlmaestro/complete-tests/subflows/receive_add_amount_keyboard.yamlmaestro/complete-tests/subflows/receive_copy_share_sheet.yamlmaestro/complete-tests/subflows/reset_to_wallet_home.yamlmaestro/complete-tests/subflows/scroll_home_to_top.yamlmaestro/complete-tests/subflows/scroll_quick_actions_into_view.yamlmaestro/complete-tests/subflows/wait_1s.yamlmaestro/complete-tests/subflows/wait_2s.yamlmaestro/complete-tests/subflows/wait_3s.yamlmaestro/complete-tests/subflows/wait_4s.yamlmaestro/complete-tests/subflows/wait_5s.yamlmaestro/complete-tests/subflows/wait_wallet_ready_after_create.yamlmaestro/config.yaml.examplemaestro/run.shmaestro/scripts/adapt-device.pymaestro/scripts/maestro-config.shmaestro/scripts/parse-taps.pymaestro/scripts/run-scenario.shmaestro/scripts/sync-secret-subflows.pymaestro/studio.sh
💤 Files with no reviewable changes (8)
- maestro/complete-tests/subflows/wait_2s.yaml
- maestro/complete-tests/subflows/open_wallet_url.yaml
- maestro/complete-tests/subflows/wait_5s.yaml
- maestro/complete-tests/subflows/wait_4s.yaml
- maestro/complete-tests/subflows/wait_3s.yaml
- maestro/complete-tests/subflows/wait_1s.yaml
- maestro/config.yaml.example
- maestro/scripts/adapt-device.py
✅ Files skipped from review due to trivial changes (7)
- maestro/complete-tests/subflows/_input_wallet_nsec.yaml.example
- maestro/complete-tests/subflows/assert_send_form.yaml
- maestro/complete-tests/subflows/open_send_from_home.yaml
- maestro/complete-tests/subflows/scroll_home_to_top.yaml
- maestro/README.md
- maestro/complete-tests/subflows/assert_receive_screen.yaml
- maestro/complete-tests/flows/10_receive_copy_sheet.yaml
🚧 Files skipped from review as they are similar to previous changes (29)
- maestro/REPORTING.md
- maestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yaml
- maestro/complete-tests/subflows/assert_on_home.yaml
- maestro/complete-tests/flows/11_send_form_address_field.yaml
- maestro/complete-tests/flows/07_home_open_activity.yaml
- maestro/complete-tests/subflows/assert_home_quick_actions.yaml
- maestro/complete-tests/subflows/scroll_quick_actions_into_view.yaml
- maestro/complete-tests/subflows/receive_copy_share_sheet.yaml
- maestro/complete-tests/subflows/receive_add_amount_keyboard.yaml
- maestro/.gitignore
- maestro/complete-tests/subflows/close_copy_sheet.yaml
- maestro/complete-tests/flows/_run_reset_then_create.yaml
- maestro/complete-tests/flows/09_home_tabs_and_asset_filters.yaml
- maestro/complete-tests/flows/05_home_open_send.yaml
- maestro/complete-tests/flows/_run_all_smoke.yaml
- maestro/scripts/maestro-config.sh
- maestro/complete-tests/subflows/open_receive_from_home.yaml
- maestro/complete-tests/subflows/ensure_on_wallet_home.yaml
- maestro/complete-tests/subflows/open_settings_from_home.yaml
- maestro/complete-tests/subflows/exercise_send_form.yaml
- maestro/complete-tests/flows/08_home_open_settings.yaml
- maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml
- maestro/complete-tests/subflows/reset_to_wallet_home.yaml
- maestro/complete-tests/subflows/wait_wallet_ready_after_create.yaml
- maestro/complete-tests/flows/13_unlock_wallet.yaml
- maestro/scripts/run-scenario.sh
- maestro/complete-tests/flows/14_home_scroll_balance_sticky.yaml
- maestro/complete-tests/flows/15_settings_deep_reset_wallet.yaml
- maestro/scripts/parse-taps.py
| def read_config_env(path: Path) -> dict[str, str]: | ||
| if not path.is_file(): | ||
| sys.exit(f"error: missing {path} — cp config.yaml.example config.yaml") | ||
| env: dict[str, str] = {} | ||
| in_env = False | ||
| for line in path.read_text().splitlines(): | ||
| if line.strip() == "env:": | ||
| in_env = True | ||
| continue | ||
| if in_env: | ||
| if line and not line.startswith(" "): | ||
| break | ||
| m = ENV_RE.match(line) | ||
| if m: | ||
| key, value = m.groups() | ||
| env[key] = value.replace('\\"', '"') | ||
| return env |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Prefer PyYAML library over manual regex parsing.
The script manually parses YAML using regex instead of using a proper YAML library. Based on the PR objectives, PyYAML is already a project dependency (mentioned for adapt-device.py). Manual parsing is fragile and won't handle YAML edge cases (unquoted values, comments, multiline strings, indentation variations).
📚 Recommended refactor to use PyYAML
+import yaml
+
from pathlib import Path
MAESTRO_DIR = Path(__file__).resolve().parents[1]
CONFIG = MAESTRO_DIR / "config.yaml"
SUBFLOWS = MAESTRO_DIR / "complete-tests" / "subflows"
-ENV_RE = re.compile(r'^ ([A-Z_]+): *"(.*)"\s*$')
-
def read_config_env(path: Path) -> dict[str, str]:
if not path.is_file():
sys.exit(f"error: missing {path} — cp config.yaml.example config.yaml")
- env: dict[str, str] = {}
- in_env = False
- for line in path.read_text().splitlines():
- if line.strip() == "env:":
- in_env = True
- continue
- if in_env:
- if line and not line.startswith(" "):
- break
- m = ENV_RE.match(line)
- if m:
- key, value = m.groups()
- env[key] = value.replace('\\"', '"')
- return env
+ config = yaml.safe_load(path.read_text())
+ return config.get("env", {})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/scripts/sync-secret-subflows.py` around lines 22 - 38, The
read_config_env function currently regex-parses YAML; replace that with PyYAML
safe_load to robustly parse the file: import yaml, open and safe_load the file
contents, verify existence of the top-level "env" mapping, return it as a
dict[str,str] (coerce/str-cast values if needed), and keep the same sys.exit
behavior when the path is missing; update references to ENV_RE (remove if
unused) and ensure any unescaping logic for quoted values is preserved when
converting YAML values to strings.
There was a problem hiding this comment.
Arkana Incremental Review — a9b2df05
Single file changed: maestro/complete-tests/flows/13_unlock_wallet.yaml (+4 −10)
Removes three now-redundant tapOn gestures that were manually tapping the period key on the soft keyboard. Since inputText in the _input_wallet_password subflow already includes the trailing ., these taps were producing a double-period.
Verdict: Clean fix. No protocol code touched. No concerns.
🤖 Reviewed by Arkana
Harden sync-secret-subflows and parse-taps parsing, validate screen dimensions in record-taps, assert Coming Soon on swap smoke, broaden clipboard permission guards for more locales, and document locked-wallet password requirements for smoke scenarios. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Arkana Incremental Review — 16f4115e
Reviewed the delta from a9b2df05. This commit addresses prior CodeRabbit feedback — all changes are test tooling, no protocol-level code.
Changes reviewed
| File | Verdict |
|---|---|
maestro/scripts/sync-secret-subflows.py |
✅ unescape_yaml_double_quoted correctly inverts yaml_double_quoted (backslashes first, then quotes). F-string lint fix is correct. |
maestro/scripts/parse-taps.py |
✅ try/except ValueError around int() parsing is a proper fix. F-string lint fix correct. |
maestro/record-taps.sh:23-26 |
✅ Screen dimension validation prevents garbage values propagating to parse-taps.py. |
maestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yaml |
✅ Adding assertVisible: "Coming Soon" before dismiss tap gives the test an actual assertion instead of blindly tapping. |
maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml |
✅ Locale guards for IT/ES/DE clipboard dialogs. See nit below. |
maestro/README.md, complete-tests/README.md |
✅ Good docs on locked-wallet/empty-password edge case. |
maestro/config.yaml.example |
✅ Comment clarification. |
Nit (non-blocking)
dismiss_clipboard_permission.yaml — The five locale blocks (lines 5-128) are ~25 lines each with identical tap targets, differing only in the when.visible regex. Consider extracting the tap sequence into a shared subflow and calling it from each guard, or collapsing into a single when.visible: ".*([Cc]lipboard|[Aa]ppunti|portapapeles|Zwischenablage|wants to see).*" block. Not blocking — it's test tooling and correctness is fine as-is.
No protocol-critical changes. LGTM.
🤖 Reviewed by Arkana
There was a problem hiding this comment.
Arkana Incremental Review — 16f4115e
Reviewed the delta from a9b2df05. This commit addresses prior CodeRabbit feedback — all changes are test tooling, no protocol-level code.
Changes reviewed
| File | Verdict |
|---|---|
maestro/scripts/sync-secret-subflows.py |
✅ unescape_yaml_double_quoted correctly inverts yaml_double_quoted (backslashes first, then quotes). F-string lint fix is correct. |
maestro/scripts/parse-taps.py |
✅ try/except ValueError around int() parsing is a proper fix. F-string lint fix correct. |
maestro/record-taps.sh:23-26 |
✅ Screen dimension validation prevents garbage values propagating to parse-taps.py. |
maestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yaml |
✅ Adding assertVisible: "Coming Soon" before dismiss tap gives the test an actual assertion instead of blindly tapping. |
maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml |
✅ Locale guards for IT/ES/DE clipboard dialogs. See nit below. |
maestro/README.md, complete-tests/README.md |
✅ Good docs on locked-wallet/empty-password edge case. |
maestro/config.yaml.example |
✅ Comment clarification. |
Nit (non-blocking)
dismiss_clipboard_permission.yaml — The five locale blocks (lines 5-128) are ~25 lines each with identical tap targets, differing only in the when.visible regex. Consider extracting the tap sequence into a shared subflow and calling it from each guard, or collapsing into a single when.visible: ".*([Cc]lipboard|[Aa]ppunti|portapapeles|Zwischenablage|wants to see).*" block. Not blocking — it's test tooling and correctness is fine as-is.
No protocol-critical changes. LGTM.
🤖 Reviewed by Arkana
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml (1)
5-128:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Multiple blocks execute for a single permission dialog.
When a Chrome clipboard permission dialog contains text matching multiple regex patterns (e.g., "Chrome wants to see clipboard" matches both
".*wants to see.*"at line 7 and".*[Cc]lipboard.*"at line 32), Maestro executes all matchingrunFlowblocks in sequence. This causes:
- Redundant dismissal attempts — each matched block repeats the full tap sequence on an already-dismissed dialog
- Unnecessary delays — each matched block adds 1+ seconds via
wait_1s.yaml- Fragility — tapping on a dismissed dialog's remnants may interact with the underlying screen
Consolidate into a single
runFlowblock with a combined regex guard that matches any locale variant:- runFlow: when: visible: ".*(?:wants to see|[Cc]lipboard|[Aa]ppunti|portapapeles|Zwischenablage).*" commands: # (single unified tap sequence here)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml` around lines 5 - 128, Multiple overlapping runFlow blocks cause repeated dismissal attempts and delays; consolidate them into one runFlow block by replacing the separate entries (the runFlow blocks with when.visible patterns ".*wants to see.*", ".*[Cc]lipboard.*", ".*[Aa]ppunti.*", ".*portapapeles.*", ".*Zwischenablage.*") with a single runFlow that uses a combined regex in when.visible (e.g., ".*(?:wants to see|[Cc]lipboard|[Aa]ppunti|portapapeles|Zwischenablage).*") and keep one unified commands sequence (the tapOn steps and the runFlow: wait_1s.yaml) so the dialog is dismissed only once and no duplicate waits occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml`:
- Around line 109-127: The tap sequence for dismissing the clipboard permission
is missing German localized button text: add tapOn entries for the German labels
(e.g., "Zulassen" and/or "Erlauben") alongside the existing "Allow" and
"Consenti" entries so the flow can match German devices before falling back to
the coordinate tap (the existing tapOn entries with text "Allow", "Consenti" and
the coordinate point "75%, 62%"); make the same additions to the other four
matching tap blocks to keep all locales consistent.
- Around line 84-102: The Spanish "Allow" label is missing from the
clipboard-permission tap sequence; add a tapOn entry with text "Permitir" and
optional: true alongside the existing tapOn entries (the same place where tapOn
text: "Allow", "Consenti", "While using the app", "Only this time", "Solo esta
vez"/coordinate tap are defined) so Spanish devices can be handled; apply the
identical addition to the other four similar permission blocks to keep behavior
consistent across all copies of the tap sequence.
---
Outside diff comments:
In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml`:
- Around line 5-128: Multiple overlapping runFlow blocks cause repeated
dismissal attempts and delays; consolidate them into one runFlow block by
replacing the separate entries (the runFlow blocks with when.visible patterns
".*wants to see.*", ".*[Cc]lipboard.*", ".*[Aa]ppunti.*", ".*portapapeles.*",
".*Zwischenablage.*") with a single runFlow that uses a combined regex in
when.visible (e.g., ".*(?:wants to
see|[Cc]lipboard|[Aa]ppunti|portapapeles|Zwischenablage).*") and keep one
unified commands sequence (the tapOn steps and the runFlow: wait_1s.yaml) so the
dialog is dismissed only once and no duplicate waits occur.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 417705e2-bdc8-4de0-b414-1ac9e55b593f
📒 Files selected for processing (9)
maestro/README.mdmaestro/complete-tests/README.mdmaestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yamlmaestro/complete-tests/flows/13_unlock_wallet.yamlmaestro/complete-tests/subflows/dismiss_clipboard_permission.yamlmaestro/config.yaml.examplemaestro/record-taps.shmaestro/scripts/parse-taps.pymaestro/scripts/sync-secret-subflows.py
✅ Files skipped from review due to trivial changes (2)
- maestro/config.yaml.example
- maestro/complete-tests/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
- maestro/complete-tests/flows/06_home_swap_coming_soon_sheet.yaml
- maestro/record-taps.sh
- maestro/README.md
- maestro/complete-tests/flows/13_unlock_wallet.yaml
- maestro/scripts/sync-secret-subflows.py
- maestro/scripts/parse-taps.py
| - tapOn: | ||
| text: "Allow" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Consenti" | ||
| optional: true | ||
| - tapOn: | ||
| text: "While using the app" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Only this time" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Solo questa volta" | ||
| optional: true | ||
| - tapOn: | ||
| point: "75%, 62%" | ||
| optional: true | ||
| waitToSettleTimeoutMs: 800 |
There was a problem hiding this comment.
Major: Missing Spanish "Allow" button text.
The Spanish clipboard guard (line 82: ".*portapapeles.*") lacks the localized "Allow" equivalent. The tap sequence attempts only "Allow" (English, line 85) and "Consenti" (Italian, line 88), then falls back to a device-specific coordinate tap at line 100. On a Spanish-localized Android device, the button text is typically "Permitir" — add it to ensure robust dismissal:
- tapOn:
text: "Consenti"
optional: true
+ - tapOn:
+ text: "Permitir"
+ optional: trueApply the same fix to the other four blocks for consistency (even though Spanish dialogs should only match this block once consolidated).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - tapOn: | |
| text: "Allow" | |
| optional: true | |
| - tapOn: | |
| text: "Consenti" | |
| optional: true | |
| - tapOn: | |
| text: "While using the app" | |
| optional: true | |
| - tapOn: | |
| text: "Only this time" | |
| optional: true | |
| - tapOn: | |
| text: "Solo questa volta" | |
| optional: true | |
| - tapOn: | |
| point: "75%, 62%" | |
| optional: true | |
| waitToSettleTimeoutMs: 800 | |
| - tapOn: | |
| text: "Allow" | |
| optional: true | |
| - tapOn: | |
| text: "Consenti" | |
| optional: true | |
| - tapOn: | |
| text: "Permitir" | |
| optional: true | |
| - tapOn: | |
| text: "While using the app" | |
| optional: true | |
| - tapOn: | |
| text: "Only this time" | |
| optional: true | |
| - tapOn: | |
| text: "Solo questa volta" | |
| optional: true | |
| - tapOn: | |
| point: "75%, 62%" | |
| optional: true | |
| waitToSettleTimeoutMs: 800 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml` around
lines 84 - 102, The Spanish "Allow" label is missing from the
clipboard-permission tap sequence; add a tapOn entry with text "Permitir" and
optional: true alongside the existing tapOn entries (the same place where tapOn
text: "Allow", "Consenti", "While using the app", "Only this time", "Solo esta
vez"/coordinate tap are defined) so Spanish devices can be handled; apply the
identical addition to the other four similar permission blocks to keep behavior
consistent across all copies of the tap sequence.
| - tapOn: | ||
| text: "Allow" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Consenti" | ||
| optional: true | ||
| - tapOn: | ||
| text: "While using the app" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Only this time" | ||
| optional: true | ||
| - tapOn: | ||
| text: "Solo questa volta" | ||
| optional: true | ||
| - tapOn: | ||
| point: "75%, 62%" | ||
| optional: true | ||
| waitToSettleTimeoutMs: 800 |
There was a problem hiding this comment.
Major: Missing German "Allow" button text.
The German clipboard guard (line 107: ".*Zwischenablage.*") lacks the localized "Allow" equivalent. The tap sequence attempts only "Allow" (English, line 110) and "Consenti" (Italian, line 113), then falls back to the coordinate tap at line 125. On a German-localized Android device, the button text is typically "Zulassen" or "Erlauben" — add it:
- tapOn:
text: "Consenti"
optional: true
+ - tapOn:
+ text: "Zulassen"
+ optional: true
+ - tapOn:
+ text: "Erlauben"
+ optional: trueApply the same fix to the other four blocks for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - tapOn: | |
| text: "Allow" | |
| optional: true | |
| - tapOn: | |
| text: "Consenti" | |
| optional: true | |
| - tapOn: | |
| text: "While using the app" | |
| optional: true | |
| - tapOn: | |
| text: "Only this time" | |
| optional: true | |
| - tapOn: | |
| text: "Solo questa volta" | |
| optional: true | |
| - tapOn: | |
| point: "75%, 62%" | |
| optional: true | |
| waitToSettleTimeoutMs: 800 | |
| - tapOn: | |
| text: "Allow" | |
| optional: true | |
| - tapOn: | |
| text: "Consenti" | |
| optional: true | |
| - tapOn: | |
| text: "Zulassen" | |
| optional: true | |
| - tapOn: | |
| text: "Erlauben" | |
| optional: true | |
| - tapOn: | |
| text: "While using the app" | |
| optional: true | |
| - tapOn: | |
| text: "Only this time" | |
| optional: true | |
| - tapOn: | |
| text: "Solo questa volta" | |
| optional: true | |
| - tapOn: | |
| point: "75%, 62%" | |
| optional: true | |
| waitToSettleTimeoutMs: 800 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maestro/complete-tests/subflows/dismiss_clipboard_permission.yaml` around
lines 109 - 127, The tap sequence for dismissing the clipboard permission is
missing German localized button text: add tapOn entries for the German labels
(e.g., "Zulassen" and/or "Erlauben") alongside the existing "Allow" and
"Consenti" entries so the flow can match German devices before falling back to
the coordinate tap (the existing tapOn entries with text "Allow", "Consenti" and
the coordinate point "75%, 62%"); make the same additions to the other four
matching tap blocks to keep all locales consistent.
1 participant
Summary
Adds a Maestro-based E2E test suite under
maestro/for running Arkade Wallet on a physical Android device via Google Chrome.This complements existing Playwright tests (
src/test/e2e/) with real mobile Chrome interaction on hardware — useful for PWA layout, touch targets, Chrome permissions, and device-specific viewport behavior.Includes:
maestro/complete-tests/) — flows 01–15 covering onboarding, smoke, unlock, settings, and wallet resetmaestro/flows/01_startup_restore.yaml) — baseline recording patternrun.sh,studio.sh,run-scenario.sh(smoke, onboarding, prerelease, etc.)record-taps.sh+parse-taps.pyadapt-device.py+device-profiles.yamlto remap coordinates across resolutions/navigation modesmaestro/README.mdwith setup, reference device specs, and troubleshootingDesign choices:
%) recorded from real device input — noevalScriptappId: com.android.chrome+-p android— taps on phone, not desktop ChromiumopenLinksubflow opensWALLET_URLbefore each runconfig.yamlgitignored;config.yaml.examplecommittedReference device (coordinates calibrated for):
Test plan
cp maestro/config.yaml.example maestro/config.yamland setWALLET_URLadb devices)./maestro/scripts/run-scenario.sh onboarding— flows 01 + 02 pass./maestro/scripts/run-scenario.sh smoke— flows 03–12 pass (wallet on home, unlocked)./maestro/studio.sh— Studio targets physical device, not desktop Chromium./maestro/scripts/adapt-device.sh --detecton another device./maestro/scripts/adapt-device.sh --to-profile <profile> --dry-runNotes for reviewers
adb— not wired into CI yetPyYAMLneeded only foradapt-device.py(pip install pyyaml)settingsandprereleasescenarios are destructiveMade with Cursor
Summary by CodeRabbit
New Features
Documentation
Chores