Skip to content

chore(ci): inline iOS maestro run and gate tests on balance check#477

Open
ignaciosantise wants to merge 4 commits intomainfrom
chore/ios-maestro-script
Open

chore(ci): inline iOS maestro run and gate tests on balance check#477
ignaciosantise wants to merge 4 commits intomainfrom
chore/ios-maestro-script

Conversation

@ignaciosantise
Copy link
Copy Markdown
Collaborator

Summary

  • Replace WalletConnect/actions/maestro/run on the iOS job of ci_e2e_walletkit.yaml with an inline maestro test script that mirrors the Android job (same env vars, --include-tags, --test-output-dir, output log, exit-code capture), plus the matching Upload Maestro artifacts step the action used to handle.
  • Add a check-balance job that calls the reusable e2e-balance-check.yml workflow, and make both e2e-ios and e2e-android depend on it with if: always() && inputs.platform != '<other>' so a balance alert won't block tests.
  • Add workflow_call: {} to e2e-balance-check.yml so it's invokable as a reusable workflow.

Test plan

  • Trigger CI E2E WalletKit via workflow_dispatch with platform=ios and confirm the check-balance job runs first, iOS tests run after regardless of alert, and maestro-ios-artifacts are uploaded.
  • Trigger with platform=android and confirm the check-balancee2e-android chain still works.
  • Trigger with platform=both and confirm both jobs run in parallel after check-balance.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 22, 2026 18:47
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pos-demo Ready Ready Preview, Comment Apr 24, 2026 5:39pm

Request Review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 22, 2026

Claude finished @ignaciosantise's task in 3m 27s —— View job


Review

  • Gather context and read changed files
  • Run breaking changes subagent — no issues
  • Run data classification subagent — no issues
  • Analyze GitHub Actions workflow security
  • Post final review

GitHub Actions Security: No pull_request_target usage — no elevated security risk from the trigger model.

Breaking changes: None. workflow_call: {} is purely additive; existing schedule and workflow_dispatch triggers are unaffected. The check-balance caller preserves the calling workflow's semantics.

Logic correctness: always() && inputs.platform != 'android' is correct for the stated intent. On non-workflow_dispatch triggers (pull_request, push), inputs.platform is empty/undefined, so '' != 'android' and '' != 'ios' both evaluate to true, which matches the prior behavior of running both jobs.

Found 1 issue(s)

Issue 1: Secrets passed as CLI arguments expose values in process listings

ID: ci-e2e-walletkit-secrets-cli-args-b2f1
File: .github/workflows/ci_e2e_walletkit.yaml:155
Severity: LOW
Category: security

Context:

  • Pattern: ${{ secrets.* }} values are interpolated directly into --env KEY="VALUE" CLI arguments for the maestro test command
  • Risk: GitHub Actions substitutes secrets before spawning the shell process, so raw secret values appear in the process's argument list — visible via /proc/[pid]/cmdline or ps aux to co-located processes. GitHub masks them in UI logs but not in process introspection.
  • Impact: On a shared runner this would be a significant exposure; on an isolated macOS runner (as used here) the blast radius is limited to the job itself
  • Trigger: Applies on every iOS test run; mirrors the pre-existing Android job pattern at line 326

Recommendation: Pass secrets via the step's env: block so they are set as OS-level environment variables (not CLI args), then reference them without --env:

- name: Run Maestro tests
  id: maestro
  env:
    MAESTRO_APP_ID: ${{ env.MAESTRO_APP_ID }}
    WPAY_CUSTOMER_KEY_MULTI_KYC: ${{ secrets.WPAY_CUSTOMER_KEY_MULTI_KYC }}
    WPAY_MERCHANT_ID_MULTI_KYC: ${{ secrets.WPAY_MERCHANT_ID_MULTI_KYC }}
    # ... repeat for remaining secrets
  run: |
    xcrun simctl launch "$DEVICE_ID" "$MAESTRO_APP_ID" || true
    $HOME/.maestro/bin/maestro test \
      --include-tags "${{ env.MAESTRO_TAGS }}" \
      --test-output-dir maestro-artifacts \
      .maestro/ >maestro-output.log 2>&1; echo $? > maestro-exit-code
    cat maestro-output.log
    exit $(cat maestro-exit-code)

Note: this also applies to the pre-existing Android job at line 326. Fix this →

Comment thread .github/workflows/ci_e2e_walletkit.yaml Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the WalletKit E2E GitHub Actions workflows to (1) run iOS Maestro tests inline (matching the Android job behavior) and (2) introduce a reusable “balance check” job that runs before E2E jobs without blocking them on alerts.

Changes:

  • Added workflow_call support to e2e-balance-check.yml so it can be invoked as a reusable workflow.
  • Added a check-balance reusable-workflow job in ci_e2e_walletkit.yaml, and made e2e-ios / e2e-android depend on it while still running even if it fails (always()).
  • Replaced the iOS WalletConnect/actions/maestro/run action with an inline maestro test script and an explicit artifact upload step.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
.github/workflows/e2e-balance-check.yml Adds workflow_call trigger to allow reuse from other workflows.
.github/workflows/ci_e2e_walletkit.yaml Adds a balance-check prerequisite job, updates job gating logic, and inlines the iOS Maestro run + artifact upload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/e2e-balance-check.yml Outdated
Comment thread .github/workflows/ci_e2e_walletkit.yaml Outdated
Comment thread .github/workflows/ci_e2e_walletkit.yaml
Comment thread .github/workflows/ci_e2e_walletkit.yaml Outdated
- Move WPAY secrets and APP_ID into step env so they aren't interpolated
  into the maestro --env CLI args (keeps them out of YAML source and
  avoids duplicating the values into argv).
- Capture maestro exit code with set +e / $? so non-zero runs still
  print the log and propagate the failure correctly under bash -e.
- Declare SLACK_FAUCETBOT_WEBHOOK_URL under on.workflow_call.secrets
  in e2e-balance-check.yml and pass it explicitly from the caller
  instead of using secrets: inherit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…script

The WalletConnect Blockchain API gates eth_getBlockByNumber and
eth_blockNumber on some chains (e.g. Arbitrum), which breaks
ethers.sendTransaction and the tx receipt poll. Add a --rpcUrl override
so users can point at a chain-native RPC, and make the baseFee read
fail-soft so fee estimation works even when the gateway blocks it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
reactivecircus/android-emulator-runner executes each line of the
script: block as a separate sh -c, so backslash continuations get
passed literally (maestro receives \ as the flow path) and
variables/set options don't cross lines. Collapse the maestro test
invocation onto one line with ; separators so set +e, the test run,
the exit code capture, and the final exit all share a shell.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants