Skip to content

🐛 Use startsWith for the wc:// check in the UriService#330

Open
AlexV525 wants to merge 1 commit intoreown-com:developfrom
AlexV525:fix/uri-service-wc
Open

🐛 Use startsWith for the wc:// check in the UriService#330
AlexV525 wants to merge 1 commit intoreown-com:developfrom
AlexV525:fix/uri-service-wc

Conversation

@AlexV525
Copy link
Copy Markdown
Contributor

Description

Use startsWith instead of contains to only escape wc:// deeplink from the service.

Resolves #329

Copilot AI review requested due to automatic review settings January 27, 2026 13:52
Copy link
Copy Markdown
Contributor

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

Fixes overly broad WalletConnect deeplink detection in UriService by ensuring only URIs that start with wc:// are treated as the generic WalletConnect scheme (and therefore “not installed”), preventing false negatives for custom schemes like demowc://.

Changes:

  • Replace contains('wc://') with startsWith('wc://') in UriService.isInstalled to avoid rejecting custom wallet schemes that include wc:// later in the string.

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

@@ -17,7 +17,7 @@ class UriService extends IUriService {
}

// If the wallet is just a generic wc:// then it is not installed
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The comment says this only filters out a "generic wc://", but the new condition returns false for any URI that starts with wc:// (including wc://<something>). Update the comment to match the actual behavior (e.g., clarify that any WalletConnect wc:// scheme is treated as not-installed).

Suggested change
// If the wallet is just a generic wc:// then it is not installed
// Treat any WalletConnect wc:// scheme URI as not installed

Copilot uses AI. Check for mistakes.
Comment on lines +20 to 22
if (uri.startsWith('wc://')) {
return false;
}
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

This changes installation-detection behavior; please add a unit test covering both cases: wc://... returns false, while custom schemes that merely contain wc:// later in the string (e.g. demowc://) are not incorrectly rejected. This will prevent regressions of issue #329.

Copilot uses AI. Check for mistakes.
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.

[appkit] The check with wc:// of UriService exceeded expectations

2 participants