fix(native): route push notification taps inside the app - #2470
Merged
Conversation
Tapping a push left the app: OneSignal fires its own ACTION_VIEW/openURL for the notification's launch URL, so Android only re-entered the app for paths in the App Links filter, and iOS — which won't re-enter an app for its own universal link — always bounced to Safari. Suppress launch URLs on both platforms and route the tap from the click listener in useNativePlugins, reusing the App Links deep-link path. That covers destinations outside the intent filter and doesn't depend on link verification. deepLinkToNativePath now accepts a bare path (push payloads carry one), rejects off-domain links instead of rewriting them, and maps two destinations that had no native route: /receipt/<id> and /<username>?chargeId=. /receipt/[entryId] is a server component stripped from the static export, so add a client twin at (mobile-ui)/receipt reading ?id=&kind= — receipts are the most common push destination and previously 404'd in-app. Also widen App Links to /history, /rewards, /badges and /profile (email CTA targets that still open the browser today), and map absolute peanut.me hrefs in the in-app inbox back to in-app paths.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughDeep-link handling now validates peanut.me URLs, maps native receipt and payment routes, routes notification clicks, buffers cold-start clicks, adds a polling receipt page, and expands route-mapping and OneSignal tests. ChangesNative deep-linking
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant OneSignal
participant nativeOneSignalAdapter
participant useNativePlugins
participant deepLinkToNativePath
participant NativeReceiptPage
OneSignal->>nativeOneSignalAdapter: notification click
nativeOneSignalAdapter-->>useNativePlugins: buffered or live click
useNativePlugins->>deepLinkToNativePath: resolve deep-link target
deepLinkToNativePath-->>useNativePlugins: native route or null
useNativePlugins->>NativeReceiptPage: navigate to receipt route
NativeReceiptPage->>NativeReceiptPage: fetch and poll history entry
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code-analysis diffPainscore total: 6215.51 → 6224.69 (+9.18) 🆕 New findings (24)
…and 4 more. ✅ Resolved (19)
📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(mobile-ui)/receipt/page.tsx:
- Around line 42-46: Update the receipt page’s redirect effect and render guard
to redirect only when required identifiers are missing, or when the query is
errored without usable existing entry data. Preserve the already-loaded receipt
view during transient polling failures by allowing rendering whenever entry
contains the last successful data, while retaining the existing behavior for
initial/unusable errors; anchor the change to the useEffect and adjacent
early-return guard.
- Around line 20-24: Wrap the useSearchParams-dependent logic in
NativeReceiptPage with a visible React Suspense boundary, either by moving the
query-param reads into a child component or by adding a route-level boundary.
Ensure the fallback renders while search parameters are being resolved, while
preserving the existing entryId and kind resolution behavior.
In `@src/utils/native-routes.ts`:
- Around line 85-95: Widen the try/catch in deepLinkToNativePath to encompass
URL parsing, hostname validation, and all subsequent decodeURIComponent-based
path mapping, including the receipt branch. Return null for any malformed URL or
URI encoding while preserving the existing valid-link mapping behavior.
- Around line 120-125: Restrict the Capacitor catch-all handling in the native
route resolver around isCapacitor() to exclude reserved single-segment routes
such as rewards, badges, profile, and history before converting chargeId links
via chargePayUrl. Preserve username-style profile routing for non-reserved
segments, and add a native-routes.test.ts case confirming /rewards?chargeId=x
remains /rewards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c34aa9dd-fa6e-4e09-a7da-4fb1bbb669e7
⛔ Files ignored due to path filters (2)
android/app/src/main/AndroidManifest.xmlis excluded by!android/**ios/App/App/Info.plistis excluded by!ios/**
📒 Files selected for processing (5)
src/app/(mobile-ui)/notifications/page.tsxsrc/app/(mobile-ui)/receipt/page.tsxsrc/hooks/useNativePlugins.tssrc/utils/__tests__/native-routes.test.tssrc/utils/native-routes.ts
…paths Three fixes from review: deepLinkToNativePath only guarded the URL parse, but decodeURIComponent throws URIError on a stray '%'. This PR made the function reachable from a render path (the notifications list calls it inside .map), so one malformed ctaDeeplink would have thrown during render and blanked the page for everyone. Guard the whole parse-and-map body so bad input degrades to null. The single-segment chargeId branch treated any one-segment path as a username, so /rewards?chargeId=x would have been rewritten to /pay-request — and this PR is what added /rewards, /history, /badges and /profile to App Links. Gate it on the same isReservedRoute + couldBeRecipient rules the web catch-all already uses. On the receipt screen, isError also trips on a failed background poll, so a flaky refetch bounced the user to /home mid-settlement. Bail out only when there's no data at all.
kushagrasarathe
previously approved these changes
Jul 22, 2026
Contributor
There was a problem hiding this comment.
Reviewed. Tap routing is guarded at two more fail-closed layers: mapDeepLink host-pins to an anchored ^(.+\.)?peanut\.me$ regex (tested against \\evil.com, peanut.me.evil.com, javascript:, encoded // — all rejected/normalized) and sanitizeRedirectURL enforces same-origin before router.push. Malformed/reserved routes fail closed (decode error → null, not a render-throw). SAFE — only open item is on-device QA (author flags it), no code condition.
…owser Two gaps in the push tap routing: - Capacitor retains a cold-start click only until the first JS listener attaches, which adapter.init() (useNotifications) does on its own async path. If it wins the race against useNativePlugins registering the routing callback, the retained tap was consumed by an empty listener set. The adapter now holds the last unconsumed click and replays it to the next listener. - With launch URLs suppressed, an off-domain https deep link (operator sends) opened the app and went nowhere. Hand those to the system browser instead.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useNativePlugins.ts (1)
54-79: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPrevent registering the click listener after effect cleanup.
If this effect unmounts while
getOneSignalAdapter()is pending, Line 114 runs before the unsubscribe is pushed. The continuation then registers a leaked listener; subsequent mounts can route a single notification multiple times.Proposed fix
const cleanups: Array<() => void> = [] + let disposed = false ... const adapter = await getOneSignalAdapter() + if (disposed) return cleanups.push( adapter.onNotificationClick(({ deepLink, additionalData }) => { ... return () => { + disposed = true cleanups.forEach((fn) => fn()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useNativePlugins.ts` around lines 54 - 79, Guard the asynchronous initialization in the useNativePlugins effect with its cleanup state so the notification click listener is not registered after unmount. Check the active/mounted flag immediately after getOneSignalAdapter resolves and before calling adapter.onNotificationClick or pushing its cleanup; preserve normal registration and cleanup behavior while the effect remains mounted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/hooks/useNativePlugins.ts`:
- Around line 54-79: Guard the asynchronous initialization in the
useNativePlugins effect with its cleanup state so the notification click
listener is not registered after unmount. Check the active/mounted flag
immediately after getOneSignalAdapter resolves and before calling
adapter.onNotificationClick or pushing its cleanup; preserve normal registration
and cleanup behavior while the effect remains mounted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a7ba5899-6757-4360-985b-15879cb9982e
📒 Files selected for processing (6)
src/app/(mobile-ui)/receipt/page.tsxsrc/hooks/useNativePlugins.tssrc/services/onesignal/native.adapter.test.tssrc/services/onesignal/native.adapter.tssrc/utils/__tests__/native-routes.test.tssrc/utils/native-routes.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/(mobile-ui)/receipt/page.tsx
- src/utils/tests/native-routes.test.ts
- src/utils/native-routes.ts
Collaborator
Author
|
Follow-up in 86e0766, closing two gaps found in review:
|
kushagrasarathe
approved these changes
Jul 22, 2026
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Re-approving after the new commit — safe: cold-start click buffering (replays a tap that arrives before the JS listener attaches, correctness) and off-domain links open in the system browser ONLY when ^https:// (regex rejects javascript:/data:), else in-app routing. Operator/admin-sourced links only. SAFE. (Device QA still the one open item, author-flagged.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Companion to peanutprotocol/peanut-api-ts#1220, which fixes the malformed launch URL (
http:///receipt/<id>→ Chrome on nothing). That alone still leaves the tap outside the app:ACTION_VIEW/openURLfor the launch URL, so Android only re-enters the app for paths in the App Links filter —/history,/rewards,/badges,/profile/*all bounce to the browseropenURLhands the tap to Safari/receipt/<id>404s inside the app.scripts/native-build.jsstrips thereceiptdirectory from the static export (it's a server component), yet/receiptis in the intent filter and is the most common push destination/<username>?chargeId=…(charge-paid pushes) targets the catch-all route, which is also disabled in native buildsChange
Suppress launch URLs on both platforms —
com.onesignal.suppressLaunchURLs(AndroidManifest) andOneSignal_suppress_launch_urls(Info.plist) — and route the tap ourselves from the click listener inuseNativePlugins, reusing the existing App Links path (deepLinkToNativePath→sanitizeRedirectURL→router.push). That covers destinations outside the intent filter and drops the dependency on link verification. It prefersadditionalData.deepLink(the canonical relative path the API now sends) and falls back to the launch URL for notifications sent before that field existed.deepLinkToNativePathnow accepts a bare path (push payloads carry one), rejects off-domain links instead of rewriting them into a bogus in-app path, and maps the two missing destinations:/receipt/<id>?kind=Xand/<username>?chargeId=→/pay-request?chargeId=(which(mobile-ui)/pay-request/page.tsxalready describes as the native stand-in for the disabled catch-all). The host allowlist is a hardcodedpeanut.mematcher, deliberately notNEXT_PUBLIC_BASE_URL— a build pointed at a preview origin must still route a real notification link.New client receipt screen at
(mobile-ui)/receiptreading?id=&kind=, fetchingGET /history/:id?kind=throughapiFetch+completeHistoryEntry— the same endpoint the web server component uses — and rendering the existingTransactionDetailsReceipt. Pending transactions poll until they reach a final state.Also: widen App Links to
/history,/rewards,/badges,/profile(email CTA targets that open the browser today —/helpdeliberately left out as a marketing page), and map absolutepeanut.mehrefs in the in-app inbox back to in-app paths, since dashboard-webhook rows store the operator'surlverbatim.Test
pnpm jest src/utils/__tests__/native-routes.test.ts— 58 pass, incl. 9 newdeepLinkToNativePathcases across capacitor/webpnpm build(web) passes, with/receipt(static) and/receipt/[entryId](dynamic) coexisting — no route collisionnode scripts/native-build.jsproducesout/receipt/index.html, so receipt deep links now render natively/history//rewards/ charge-paid, app cold + backgrounded + foregrounded, both platforms). Cold start specifically depends on the OneSignal SDK holding the click event until a JS listener attaches. Reinstall is required for App Links to re-verify; check withadb shell pm get-app-links me.peanut.wallet.OneSignal_suppress_launch_urlscomes from OneSignal's deep-linking docs; the Android counterpart is confirmed present innotifications-5.9.3.aar(OSNotificationOpenAppSettings.getSuppressLaunchURL). Worth confirming on-device during the iOS pass.Note:
pnpm native:buildcurrently fails onmainbefore it reaches any of this — the anti-rot guard only consultsITEMS_TO_DISABLEand so rejects(mobile-ui)/claim/page.tsx, whichP0_TRANSFORMSalready fixes up. Pre-existing and tracked separately; patched locally to run the export check above.Summary by CodeRabbit