Skip to content

fix(agentic-cli): open settings on any Android deny after Turn on - #33744

Merged
tylerc-consensys merged 9 commits into
mainfrom
fix/agentic-cli-android-push-nudge-permanent-deny
Jul 28, 2026
Merged

fix(agentic-cli): open settings on any Android deny after Turn on#33744
tylerc-consensys merged 9 commits into
mainfrom
fix/agentic-cli-android-push-nudge-permanent-deny

Conversation

@tylerc-consensys

@tylerc-consensys tylerc-consensys commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #33688 (MMAI-925: Agentic CLI login push-permission nudge, toast variant).

On Android 13+, tapping Turn on on the nudge calls PermissionsAndroid.request(POST_NOTIFICATIONS) instead of relying on Notifee's isPushPermissionPromptable(), which reports DENIED for both first-time and permanently blocked states and can leave the nudge in a no-op path.

  • GrantedenableNotifications() and close the toast
  • Denied or never ask again → open device notification settings (the user already signaled intent by tapping Turn on), then retry enabling on foreground return if OS permission is granted

Android < 13 has no runtime permission dialog; tapping Turn on opens notification settings directly.

iOS is unchanged: when the OS can still show its dialog, enableNotifications() runs and a denial closes the toast without opening Settings; when the OS can no longer prompt, MetaMask deep-links to notification settings.

This simplifies earlier iterations by removing denial counters, elapsed-time heuristics, and shouldShowRequestPermissionRationale branching.

Video

Screen.Recording.2026-07-24.at.4.51.14.PM.mov

Changelog

CHANGELOG entry: null

Related issues

Refs: MMAI-925

Manual testing steps

Feature: CLI login push nudge (toast variant, MMAI-925)

  Scenario: Android 13+ grants notification permission
    Given MetaMask is installed on Android 13+ with push permission not granted
    And the CLI login push nudge toast is visible after Agentic CLI QR login

    When the user taps Turn on and grants the OS notification prompt
    Then in-app notifications are enabled
    And the nudge toast closes

  Scenario: Android 13+ denies notification permission
    Given MetaMask is installed on Android 13+ with push permission not granted
    And the CLI login push nudge toast is visible

    When the user taps Turn on and denies the OS notification prompt
    Then MetaMask opens device notification settings
    And in-app notifications remain disabled until OS permission is granted

  Scenario: Android 13+ permanently denies notification permission
    Given MetaMask is installed on Android 13+ with push permission permanently denied
    And the CLI login push nudge toast is visible

    When the user taps Turn on
    Then MetaMask opens device notification settings

  Scenario: iOS still prompts when promptable
    Given MetaMask is installed on iOS with push permission not yet granted
    And the CLI login push nudge toast is visible

    When the user taps Turn on and denies the OS prompt
    Then the nudge toast closes
    And MetaMask does not open notification settings

  Scenario: iOS opens settings when no longer promptable
    Given MetaMask is installed on iOS with push permission permanently denied
    And the CLI login push nudge toast is visible

    When the user taps Turn on
    Then MetaMask opens device notification settings

Screenshots/Recordings

Before

N/A

After

N/A — verified manually on physical Android device (SM_A336E).

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches post-login notification permission UX on Android (runtime permission + settings deep link) with moderate refactor of async flow guards; iOS path is mostly extracted, not redesigned.

Overview
Android “Turn on” on the CLI login push nudge no longer relies on Notifee promptability (which couldn’t distinguish first ask vs blocked). The hook now branches by platform after checking whether push is already granted.

On Android 13+, it calls PermissionsAndroid.request(POST_NOTIFICATIONS): granted runs the existing enable flow and closes the toast; denied or never ask again closes the toast, opens notification settings, and retries enable when the app returns to foreground if OS permission was granted. On Android < 13, Turn on skips a runtime dialog and goes straight to settings with the same foreground retry.

iOS behavior is unchanged but shares a new openSettingsAndScheduleRetry helper with Android. The Turn on handler is refactored into smaller flows, the in-flight guard is released before opening settings, and foreground retry handlers swallow promise rejections.

Tests add Android 13+ and pre-13 cases with PermissionsAndroid / Platform mocks.

Reviewed by Cursor Bugbot for commit 4a35570. Bugbot is set up for automated code reviews on this repo. Configure here.

…anent deny

Android Notifee reports DENIED for both first-time and permanently blocked
push permission, so request OS permission first and deep-link to notification
settings when the request returns without showing a dialog.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tylerc-consensys tylerc-consensys self-assigned this Jul 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamask-ci metamask-ci Bot added the team-onboarding Onboarding team label Jul 23, 2026
@metamask-ci

metamask-ci Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Pre-merge author checklist has unchecked items (e.g. "I've tested with a power user scenario"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@tylerc-consensys
tylerc-consensys marked this pull request as ready for review July 23, 2026 23:25

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ff87ff2. Configure here.

Comment thread app/components/UI/CliLoginPushNudge/useCliLoginPushNudge.ts Outdated
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 23, 2026
tylerc-consensys and others added 2 commits July 24, 2026 14:45
Replace elapsed-time heuristic for detecting permanent notification
denial with PermissionsAndroid.request() and NEVER_ASK_AGAIN result,
which is the authoritative signal that the OS dialog can no longer
be shown. Also handle Android < 13 by opening settings directly.

Fixes Bugbot finding: Android elapsed time misclassifies deny.

Co-authored-by: Cursor <cursoragent@cursor.com>
Android 13+ may return DENIED (not NEVER_ASK_AGAIN) when the OS dialog
is skipped after permanent denial. Treat fast PermissionsAndroid.request
completion as a no-dialog denial and deep-link to notification settings.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.76%. Comparing base (1192934) to head (f147200).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...nents/UI/CliLoginPushNudge/useCliLoginPushNudge.ts 83.78% 2 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #33744    +/-   ##
========================================
  Coverage   84.75%   84.76%            
========================================
  Files        6223     6226     +3     
  Lines      167061   167338   +277     
  Branches    40853    40935    +82     
========================================
+ Hits       141596   141845   +249     
- Misses      15797    15801     +4     
- Partials     9668     9692    +24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tylerc-consensys and others added 2 commits July 24, 2026 15:29
Replace elapsed-time heuristic with shouldShowRequestPermissionRationale
after DENIED to distinguish dialog dismissal from silent permanent deny
without misclassifying a quick in-dialog deny.

Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap shouldShowRequestPermissionRationale in a typed helper because RN
PermissionsAndroidStatic typings omit the runtime API. Also fix no-void lint.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added risk:low AI analysis: low risk and removed risk:medium AI analysis: medium risk labels Jul 24, 2026
Simplify Android push nudge logic: once the user taps Turn on, any OS
permission denial opens notification settings instead of dismissing the
toast. Removes shouldShowRequestPermissionRationale branching.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tylerc-consensys tylerc-consensys changed the title fix(agentic-cli): open settings when Android CLI push nudge hits permanent deny fix(agentic-cli): open settings on any Android deny after Turn on Jul 24, 2026
Use jest.SpyInstance for PermissionsAndroid.request spy to satisfy lint:tsc.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 24, 2026
Extract granted, Android, and iOS permission flows into dedicated
callbacks so the tap handler is a thin orchestrator, resolving the
SonarCloud cognitive-complexity warning. Behavior is unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread app/components/UI/CliLoginPushNudge/useCliLoginPushNudge.ts
Comment thread app/components/UI/CliLoginPushNudge/useCliLoginPushNudge.ts
Comment thread app/components/UI/CliLoginPushNudge/useCliLoginPushNudge.ts
… defer Android loading toast

- Move inFlightRef release before NotificationService.openSystemSettings()
  so a throw can't leave "Turn on" permanently locked (iOS + Android
  consistent).
- Defer the Android loading toast to after RESULTS.GRANTED so it no longer
  sits behind the OS permission dialog.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 95%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR modifies only two files:

  1. useCliLoginPushNudge.ts - Adds Android-specific push notification permission handling (PermissionsAndroid.request for Android 13+, settings redirect for Android < 13). This is a narrow, self-contained hook used only for the CLI QR login push nudge toast. It does not touch any core wallet flows, navigation, Engine, controllers, or shared components.

  2. useCliLoginPushNudge.test.tsx - Adds unit tests for the new Android permission flows. These are Jest unit tests, not E2E tests.

No E2E smoke tests are relevant because:

  • The CLI Login Push Nudge is a very specific feature (post-QR-login notification nudge) with no existing E2E smoke coverage
  • No shared components (TabBar, BrowserTab, modals, confirmations) are affected
  • No test infrastructure (page objects, selectors, flows, smoke specs) is changed
  • No controller or Engine code is modified
  • The change is purely about Android OS permission dialog handling within a single hook

Performance tests are also not warranted as this is a UI hook for a notification nudge with no impact on app launch, login, asset loading, or any other performance-sensitive flow.

Performance Test Selection:
The changes are limited to Android push notification permission handling in a CLI login nudge hook. There is no impact on app launch, login performance, asset loading, swap flows, or any other performance-sensitive scenario.

View GitHub Actions results

@github-actions github-actions Bot added risk:low AI analysis: low risk and removed risk:medium AI analysis: medium risk labels Jul 28, 2026
@sonarqubecloud

Copy link
Copy Markdown

@tylerc-consensys
tylerc-consensys added this pull request to the merge queue Jul 28, 2026
Merged via the queue into main with commit 2e7dbfb Jul 28, 2026
104 checks passed
@tylerc-consensys
tylerc-consensys deleted the fix/agentic-cli-android-push-nudge-permanent-deny branch July 28, 2026 07:41
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 28, 2026
@metamask-ci metamask-ci Bot added the release-8.6.0 Issue or pull request that will be included in release 8.6.0 label Jul 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-8.6.0 Issue or pull request that will be included in release 8.6.0 risk:low AI analysis: low risk size-M team-onboarding Onboarding team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants