feat: add connection not found error toast for MetaMask Connect #23700
Conversation
|
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. |
jiexi
changed the title
jiexi
changed the title
jiexi
added
the
no-changelog
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| handler, | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: Event subscription leaks when wallet stays locked
The handleSimpleDeeplink method subscribes to 'KeyringController:unlock' when the wallet is locked, but if the wallet is never unlocked (user kills app, navigates away, etc.), the subscription and associated Promise callback remain in memory indefinitely. Each call to this method while the wallet is locked creates a new subscription that may never be cleaned up, causing accumulated memory retention over time.
| }); | ||
|
|
||
| describe('isConnectDeeplink', () => { | ||
| describe('isMwpDeeplink', () => { |
There was a problem hiding this comment.
Bug: Test names use prohibited "should" prefix (Bugbot Rules)
All new test cases in this file use "should" in their names (e.g., 'should return true for valid MWP connect deeplinks', 'should handle a valid simple deeplink', 'should not show error if connection is found in the store'). Per unit testing guidelines, test names must never use "should" - this is a hard rule with zero exceptions. Test names need to be action-oriented and describe what the code does directly.
Additional Locations (2)
| }); | ||
|
|
||
| describe('when the connection is not found in the store', () => { | ||
| it('should show error if connection when the keyring is unlocked', async () => { |
There was a problem hiding this comment.
Bug: Test names use "should" violating naming rules (Bugbot Rules)
The new test names at lines 292 and 307 use "should" which violates the unit testing guidelines that state "NEVER use 'should' in test names - this is a hard rule with zero exceptions." Additionally, both test names have grammatical issues making them confusing: line 292 says "if connection when" (missing "not found") and line 307 says "connection is keyring is" (extra "is").
Additional Locations (1)
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThis PR modifies critical core functionality in the SDKConnectV2 and DeeplinkManager modules:
Since no E2E tests directly cover SDKConnectV2 deeplinks (confirmed by grep), I'm selecting:
The changes are well-covered by unit tests (connection-registry.test.ts, QRScanner tests updated), but the E2E risk is around potential regressions in deeplink handling and QR scanning flows that could affect user connectivity with dapps. |
|
Description
When deeplinking the user into the wallet for non-initial connection cases, the wallet has no idea which connection/session the user is opening the wallet from. In the case that the connection the dapp is using no longer exists on the wallet side, the user will sit in the wallet indefinitely waiting for a request that will never arrive.
This PR fixes that by checking the connection id from the new simple deeplink of format `metamask://connect/mwp?id=CONN_ID_HERE. This will allow the wallet to prompt the user with an error toast if that connection no longer exists on the wallet side.
Changelog
CHANGELOG entry: null
MetaMask Connect has not been released yet
Related issues
See: MetaMask/connect-monorepo#63
Manual testing steps
Writing these testing steps as if you have context already. If not, please reach out to the Wallet Integrations team.
NOTE: if you are using expo, I highly recommend swiping away the metamask app between each deeplink
Screenshots/Recordings
Before
After
Screen.Recording.2025-12-04.at.3.00.22.PM.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Implements simple MWP deeplink handling that shows a "connection not found" toast when a session is missing, renames SDKConnectV2 deeplink APIs, and updates QR scanner, adapter, i18n, and tests accordingly.
isMwpDeeplink,handleMwpDeeplink, andhandleSimpleDeeplinkincore/SDKConnectV2/services/connection-registry.tsto support MWP deeplinks and a simpleidflow.core/DeeplinkManager/handleDeeplink.ts) to use new MWP handlers.HostApplicationAdapter.showNotFoundError()and wires notification display.sdk_connect_v2.show_not_foundstrings inlocales/languages/en.json.SDKConnectV2.isMwpDeeplink/handleMwpDeeplinkinViews/QRScannerlogic and utils.IHostApplicationAdapterwithshowNotFoundError.Written by Cursor Bugbot for commit 5d66e68. This will update automatically on new commits. Configure here.