Skip to content

ci(e2e-pay): drop universal-link (deeplink) support from wallet E2E#402

Merged
ignaciosantise merged 2 commits into
developfrom
ci-drop-universal-link-wallet-e2e
Jul 7, 2026
Merged

ci(e2e-pay): drop universal-link (deeplink) support from wallet E2E#402
ignaciosantise merged 2 commits into
developfrom
ci-drop-universal-link-wallet-e2e

Conversation

@ignaciosantise

@ignaciosantise ignaciosantise commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Flutter mirror of reown-com/react-native-examples#563.

Universal linking is no longer in scope for the wallet Maestro pay suite — WalletConnect/actions#109 removes the Universal Link pay test (pay_single_option_nokyc_deeplink.yaml) from the shared suite. This PR strips the infrastructure in this repo that existed only to make that test pass. Pay links still reach the wallet via NFC.

Changes

  • Stop registering pay.walletconnect universal links — remove the https App Links autoVerify intent-filter (AndroidManifest.xml) and the applinks:pay.walletconnect.* entries (ios/Runner/Runner.entitlements). Kept lab.reown.com / appkit-lab.reown.com (real dApp deep-link-back / link-mode) and the NFC NDEF_DISCOVERED intent-filter. The Dart pay-link handler (deep_link_handler.dart) is unchanged — pay links still route to the pay flow, they just arrive via NFC now.
  • Build the Maestro iOS simulator app unsigned (ci_e2e_pay_tests.yml): CODE_SIGNING_ALLOWED=NO, drop DEVELOPMENT_TEAM / CODE_SIGN_STYLE / PROVISIONING_PROFILE_SPECIFIER. Signing (dev, never prod) was only needed so the applinks: associated-domains entitlement survived for the UL test. ARCHS=x86_64 is kept — that's the mobile_scanner/MLKit simulator-slice workaround, unrelated to UL.
  • Remove the iOS UL plumbing: the associated-domains developer-mode entitlement patch, swcutil reset, the AASA-cache priming loop, and the swcutil show diagnostic dumps.
  • Bump the shared-flow pins: pay-tests, setup, permit2-reset (both Android + iOS legs) and the setup-maestro-pay-tests.sh default REF re-pointed to the 109 merge commit (4be7c01) so the deeplink flow is no longer downloaded.

No web leg / WEB_SKIP_FLOWS exists in this suite (unlike RN), so nothing there to change.

🤖 Generated with Claude Code

Mirrors reown-com/react-native-examples#563. WalletConnect/actions#109
removes the Universal Link pay test (pay_single_option_nokyc_deeplink) from
the shared suite, so the wallet-side infra that existed only to make it pass
is dead weight. Pay links still reach the wallet via NFC.

- app config: stop registering pay.walletconnect universal links — remove the
  https App Links autoVerify intent-filter (AndroidManifest.xml) and the
  applinks:pay.walletconnect.* entries (Runner.entitlements). Kept lab.reown.com
  / appkit-lab.reown.com (real dApp deep-link-back / link-mode) and the NFC
  NDEF_DISCOVERED intent-filter. Dart pay-link handler is unchanged.
- ci: build the Maestro iOS simulator app unsigned (CODE_SIGNING_ALLOWED=NO,
  drop DEVELOPMENT_TEAM / CODE_SIGN_STYLE) — signing existed only to keep the
  associated-domains entitlement for the UL test. ARCHS=x86_64 kept (MLKit).
- ci: remove the associated-domains developer-mode entitlement patch, swcutil
  reset, AASA cache priming, and the swcutil show dumps.
- Bump the three shared-flow pins (pay-tests, setup, permit2-reset) and the
  setup-script REF to the #109 head (28181b5) so the deeplink flow is no longer
  downloaded.

Follow-up: re-point all pins to the merge commit once WalletConnect/actions#109
lands on master.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 6, 2026 18:03
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown

Claude finished @ignaciosantise's task in 2m 33s —— View job


Review Complete

  • Gather context
  • Read changed files
  • Breaking changes subagent → no issues
  • Data classification subagent → no sensitive data concerns
  • Compile and post review

Clean removal of dead CI infrastructure. The changes are well-scoped, well-commented, and correctly symmetric across Android/iOS/workflow. No breaking changes, no sensitive data exposed.

🔒 External Domain URL (Non-blocking)
URL: https://polygon-bor-rpc.publicnode.com
File: .github/workflows/ci_e2e_pay_tests.yml:166,380
Pre-existing (not added in this PR) — verify the public RPC endpoint is an acceptable dependency for the permit2-reset step.

Found 1 issue(s)

Issue 1: Action pins point to an unmerged PR head SHA

ID: ci-e2e-pay-tests-unmerged-sha-pin-a3f1
File: .github/workflows/ci_e2e_pay_tests.yml:116,119,163,349,352,377
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: All six action refs (pay-tests, setup, permit2-reset on both legs) and the setup-maestro-pay-tests.sh default REF are pinned to 28181b5f133836380d2d9bb8ff06af5c3234fb9d, which is the HEAD of the still-open WalletConnect/actions#109.
  • Risk: If Select Chain Method is not working properly on Android #109 is force-pushed, rebased, or its branch is deleted before merging, the SHA becomes inaccessible and every CI run that hits these steps fails with a 404. GitHub caches action downloads, but a fresh runner or cache eviction exposes this.
  • Impact: CI becomes permanently broken until the pins are updated — scheduled nightly runs and PRs targeting wallet code will all fail.
  • Trigger: Any force-push or history rewrite to the WalletConnect/actions branch backing 28181b5 before the PR merges.

Recommendation: The PR description already calls this out as a known follow-up. Once WalletConnect/actions#109 merges, update all six pin sites and the script default to the merge commit SHA before this PR lands on develop, or at minimum immediately after.

# After #109 merges, replace all occurrences of:
uses: WalletConnect/actions/maestro/pay-tests@28181b5f133836380d2d9bb8ff06af5c3234fb9d
# with the stable post-merge SHA, e.g.:
uses: WalletConnect/actions/maestro/pay-tests@<merge-commit-sha>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes Universal Link/App Link infrastructure that existed solely for the WalletKit Flutter Maestro Pay E2E deep-link flow, aligning this repo with the updated shared pay test suite (where the UL flow has been removed). This keeps pay-link routing logic intact while ensuring links are expected to arrive via non-UL paths (e.g., NFC on Android), and simplifies iOS CI by removing signing requirements that were only needed for associated-domains entitlements.

Changes:

  • Remove pay.* associated-domains (iOS) and pay.* https App Links autoVerify intent-filter (Android), keeping NFC routing support.
  • Update iOS pay E2E CI build to be unsigned and delete UL-related simulator priming/plumbing steps.
  • Repin shared Maestro pay test flow/setup actions and the local setup script to the updated shared suite ref.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
scripts/setup-maestro-pay-tests.sh Updates the default pinned ref for downloading shared Maestro pay flows.
packages/reown_walletkit/example/lib/dependencies/deep_link_handler.dart Adds clarifying comment about pay-link handling after removing UL/App Link registration.
packages/reown_walletkit/example/ios/Runner/Runner.entitlements Removes pay.* applinks: entries from associated-domains entitlement.
packages/reown_walletkit/example/android/app/src/main/AndroidManifest.xml Removes pay.* https App Links autoVerify intent-filter; retains NFC NDEF intent-filter.
.github/workflows/ci_e2e_pay_tests.yml Repins shared Maestro actions; removes UL-related iOS simulator steps and builds unsigned.

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

Comment on lines +52 to +55
// Pay links reach the wallet via NFC (the NDEF_DISCOVERED intent-filter on
// Android / associated NFC delivery on iOS). Universal-link registration for
// these hosts was removed from AndroidManifest.xml / Runner.entitlements, so
// on native these URLs now arrive via NFC rather than a tapped App Link.
@ignaciosantise ignaciosantise marked this pull request as draft July 6, 2026 18:10
@ignaciosantise ignaciosantise requested a review from jakubuid July 6, 2026 18:11
WalletConnect/actions#109 merged, so replace the temporary #109-head pin
(28181b5) with the merge commit (4be7c01) across the workflow pins and the
setup-maestro-pay-tests.sh default REF.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ignaciosantise ignaciosantise marked this pull request as ready for review July 7, 2026 13:53
@claude

claude Bot commented Jul 7, 2026

Copy link
Copy Markdown

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


🔄 Incremental Review: (since first review on 2026-07-06)

Resolved from previous review:

🔒 External Domain URL (Non-blocking)
URL: https://polygon-bor-rpc.publicnode.com
File: .github/workflows/ci_e2e_pay_tests.yml:166,380
Pre-existing — verify the public RPC endpoint is an acceptable dependency for the permit2-reset step.

Found 2 issue(s)

Issue 1: iOS comment claims NFC delivery that doesn't exist on the platform

ID: deep-link-handler-ios-nfc-comment-inaccurate-c2f1
File: packages/reown_walletkit/example/lib/dependencies/deep_link_handler.dart:52
Severity: LOW
Category: code_quality

Context:

  • Pattern: The added comment (lines 52–55) says "associated NFC delivery on iOS" as the mechanism by which pay links now arrive. iOS has no NDEF_DISCOVERED intent equivalent — Core NFC on iOS is a foreground-session API that can't deliver URLs to an app the way Android's NDEF_DISCOVERED intent-filter does.
  • Risk: Future engineers debugging a pay-link routing failure on iOS will chase a non-existent NFC path.
  • Impact: Misleads debugging; the actual delivery mechanism on iOS post-UL-removal is unspecified.
  • Trigger: Anyone reading this code when iOS pay-link routing is broken.

Recommendation: Drop the iOS-specific NFC claim; describe only what is accurate:

// Pay links are routed through the payment flow regardless of how they arrive
// (NFC on Android via NDEF_DISCOVERED, or other delivery on iOS). Universal-link
// registration for pay.walletconnect.* was removed from AndroidManifest.xml /
// Runner.entitlements, so the handler here is the sole routing point.

Also noted by Copilot reviewer. Fix this →


Issue 2: iOS Maestro step inlines secrets as CLI args; Android uses safer env-block pattern

ID: dcl-ci-e2e-pay-tests-ios-secrets-inline-cmd-3f8a
File: .github/workflows/ci_e2e_pay_tests.yml:359
Severity: MEDIUM
Category: security

Context:

  • Pattern: The Run Maestro iOS tests step directly expands ${{ secrets.WPAY_CUSTOMER_KEY_* }} / ${{ secrets.WPAY_MERCHANT_ID_* }} inside the run: script body (6 secrets). The Android job (lines 127–135) correctly channels the same secrets through a step-level env: block and then references them as $VARNAME.
  • Risk: When ${{ secrets.X }} is expanded in a run: block, the literal value lands in the shell command string — visible in /proc/<pid>/cmdline and ps aux to other processes on the runner.
  • Impact: Customer API keys and merchant IDs could be harvested by a concurrent process on a shared runner.
  • Trigger: Any execution of the pay-tests-ios job.

Recommendation: Mirror the Android pattern — move secrets to a step-level env: block and reference via $VAR:

- name: Run Maestro iOS tests
  id: maestro_ios
  env:
    WPAY_CUSTOMER_KEY_SINGLE_NOKYC: ${{ secrets.WPAY_CUSTOMER_KEY_SINGLE_NOKYC }}
    WPAY_MERCHANT_ID_SINGLE_NOKYC: ${{ secrets.WPAY_MERCHANT_ID_SINGLE_NOKYC }}
    WPAY_CUSTOMER_KEY_MULTI_NOKYC: ${{ secrets.WPAY_CUSTOMER_KEY_MULTI_NOKYC }}
    WPAY_MERCHANT_ID_MULTI_NOKYC: ${{ secrets.WPAY_MERCHANT_ID_MULTI_NOKYC }}
    WPAY_CUSTOMER_KEY_MULTI_KYC: ${{ secrets.WPAY_CUSTOMER_KEY_MULTI_KYC }}
    WPAY_MERCHANT_ID_MULTI_KYC: ${{ secrets.WPAY_MERCHANT_ID_MULTI_KYC }}
  run: |
    maestro test \
      --env APP_ID=com.walletconnect.flutterwallet.internal \
      --env WPAY_CUSTOMER_KEY_SINGLE_NOKYC=$WPAY_CUSTOMER_KEY_SINGLE_NOKYC \
      ...

Note: this pattern was pre-existing before this PR but is worth fixing while touching this file.

@ignaciosantise ignaciosantise merged commit 26b50c7 into develop Jul 7, 2026
11 checks passed
@ignaciosantise ignaciosantise deleted the ci-drop-universal-link-wallet-e2e branch July 7, 2026 14:18
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants