Skip to content

CP-14651: subscribe a new recurring swap for push notifications without needing a screen visit - #4015

Open
B0Y3R-AVA wants to merge 2 commits into
mainfrom
boyer/cp-14651
Open

CP-14651: subscribe a new recurring swap for push notifications without needing a screen visit#4015
B0Y3R-AVA wants to merge 2 commits into
mainfrom
boyer/cp-14651

Conversation

@B0Y3R-AVA

@B0Y3R-AVA B0Y3R-AVA commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Ticket: CP-14651

Reported as "recurring swap notifications don't come through on Android". It is not an Android bug. The deciding factor is whether a RECURRING_SCHEDULES React Query observer is mounted when a listOrders refetch could land.

The client has no orderId at broadcast time (executeFirstFill returns only { txHash }), so the only route to a push subscription is ensureOrderSubscriptions seeing the order in a landed listOrders snapshot. submitRecurringSwap invalidates at broadcast, but Markr's indexer lags by a few seconds, so that snapshot comes back empty. scheduleStaggeredInvalidate queues catch-up invalidates at t=5/15/30s to cover the lag — and then SwapScreen calls dismissAll() the moment submitRecurringSwap resolves.

invalidateQueries defaults to refetchType: 'active'. With the modal gone there is no observer, so all three catch-up timers marked the query stale and refetched nothing. The order never landed in the cache, the schedules cache subscriber never fired, and the device was never subscribed. Because the notification-history row is written per subscribed device (webhook/recurring-swapscreateNotificationHistoryAfterSend(subscription.clientId, …)), every leg executing before the user's next visit to Activity / Swap / the schedules screen was silently dropped: no push and no notification-center row.

Fix is one line — the catch-up batch now uses refetchType: 'all' so it refetches the cached-but-inactive query. Also updates a now-stale comment in _makeOrderActionHook.ts that described the scoped invalidate as "a harmless stale-mark", which is no longer accurate.

Measured on a Pixel 8 Pro (mainnet C-Chain, user stayed on Portfolio throughout):

Before After
Subscribe latency 4m22s, and only after navigating to Activity 5.3s, no navigation
Legs lost every leg until the user opens a schedules screen leg 1 only (see follow-up)

No dependencies introduced. Not breaking.

Screenshots/Videos

Device logs are the meaningful artifact here rather than UI. Before the fix, order 0x579c0ad8…:

14:45:15   order created → cache event count=0      (Markr not yet indexed)
14:45:20   catch-up  5s → observers=0, no refetch
14:45:30   catch-up 15s → observers=0, no refetch
14:45:45   catch-up 30s → observers=0, no refetch
           ── 3m51s, device NOT subscribed ──
14:49:36   user opens a schedules-observing screen
14:49:37   count=1 statuses=["active"] → POST /v1/push/recurring-swaps/subscribe
14:49:38   status=200                                (4m22s after creation)

After the fix, order 0x7a7045de…:

15:05:29   count=1                                  (new order not yet indexed)
15:05:33   catch-up 5s → observers=0 → refetch HAPPENS
15:05:33   count=2 statuses=["active","active"] → POST subscribe
15:05:49   status=200                                (5.3s after creation)
15:11      "Recurring swap executed — Swap 2 of USDC to AVAX complete" row arrives

Notification center before the fix showed no row for the leg that executed at creation, and rows for every leg after the subscribe timestamp (Swap 2 at 2:50, Swap 3 at 2:56, Swap 4 at 3:02, Swap 5 at 3:08 Completed).

Testing

Dev Testing
iOS: 9421
Android: 9422

Happy path (this is the reported repro):

  1. Create a recurring swap, then stay on Portfolio. Do not open Activity, Swap, or the schedules screen.
  2. Within ~30s the device should POST /v1/push/recurring-swaps/subscribe and get a 200.
  3. The next leg should produce a push and a notification-center row without any navigation.

Before this change, step 2 never happened and step 3 produced nothing until you visited a schedules-observing screen.

Edge cases covered by unit tests in subscribeAfterCreate.test.ts, which drives a real QueryClient:

  • observer torn down before Markr indexes the order (the bug) — now subscribes
  • observer left mounted (e.g. Swap opened from the Activity tab) — still subscribes, guards against a fix that only moves the problem

Also exercised: cancel / pause / resume still settle correctly via _makeOrderActionHook (scheduleStaggeredInvalidate is shared).

yarn test for recurringSwap, notifications, services/notifications: 275 passing / 26 suites. tsc and eslint clean for the changed files.

QA Testing

Acceptance criteria: after creating a recurring swap, you receive notifications for subsequent legs without opening the recurring schedules screen or the Activity tab.

Note for QA: the very first swap executes at order-creation time, before any subscription can exist, so its notification-center row is still expected to be missing. That is a known backend follow-up, not a regression in this PR.

Checklist

Please check all that apply (if applicable)

  • I have performed a self-review of my code
  • I have verified the code works
  • I have included screenshots / videos of android and ios
  • I have added testing steps
  • I have added/updated necessary unit tests
  • I have updated the documentation

Follow-up (needs backend)

Leg 1 executes inside executeFirstFill at order creation, before the subscription exists, so getActiveSubscriptionsByOrderId returns empty and no history row is written. Confirmed on device. A client cannot subscribe to an order before that order exists, so this race is structural to per-(orderId, deviceArn) subscriptions. Two options for the notification-sender service:

  1. Backfill notification history for already-executed legs when a subscribe arrives late (push suppressed, matching the existing first-leg behaviour).
  2. Subscribe recurring-swap notifications by owner address, like balance changes, and fan out to every registered device. Removes this race and also fixes the separate case where a second device on the same wallet never subscribes.

Option 2 preferred.

Copilot AI review requested due to automatic review settings July 29, 2026 19:29
@B0Y3R-AVA
B0Y3R-AVA requested a review from a team as a code owner July 29, 2026 19:29

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

This PR fixes recurring swap push-notification subscription timing by ensuring the staggered post-broadcast React Query invalidations actually refetch the cached RECURRING_SCHEDULES query even when no observers are mounted (e.g., after the swap modal dismisses), so ensureOrderSubscriptions can see the newly indexed order and subscribe the device.

Changes:

  • Update scheduleStaggeredInvalidate to invalidate with refetchType: 'all' so inactive-but-cached queries refetch during the catch-up window.
  • Add a regression unit test that drives a real QueryClient to cover the “observer unmounts before indexer catches up” scenario.
  • Refresh an explanatory comment in _makeOrderActionHook.ts to reflect the new refetch behavior.

Reviewed changes

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

File Description
packages/core-mobile/app/new/features/recurringSwap/utils/staggeredInvalidate.ts Forces staggered invalidations to refetch inactive cached queries (refetchType: 'all') to catch indexer lag after modal dismissal.
packages/core-mobile/app/new/features/recurringSwap/store/subscribeAfterCreate.test.ts Adds regression tests verifying subscription occurs even when the schedules observer is torn down before the order appears.
packages/core-mobile/app/new/features/recurringSwap/hooks/_makeOrderActionHook.ts Updates commentary around staggered invalidation semantics and account-scoped query keys.

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

Comment on lines +321 to +323
// marking it stale — which is the intent here: the catch-up must settle
// the status of the order the user acted on, whichever account is active
// by the time it fires. The builder lowercases the owner so this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair catch, the sentence was ambiguous. "whichever account is active by the time it fires" was meant as a concessive (i.e. regardless of what is active), but it reads just as easily as describing the refetch target, which is the opposite of what happens.

Reworded in 97aae14 to state the behaviour positively instead: the call always targets the acting account (the owner of the order that was cancelled/paused/resumed) and never the account that happens to be active when the timer fires. Since the whole point of this comment is to stop a future reader from misreading the cross-account behaviour, leaving it ambiguous defeated the purpose.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Coverage report ✅

2/2 packages passed thresholds
Thresholds are shown inline against each package baseline.

🟢 @avalabs/core-mobile

St. Category Percentage Covered / Total
🟢 Statements 27.89% (+9.89% vs 18% ▲) 12357/44297
🟢 Branches 23.56% (+11.56% vs 12% ▲) 6071/25767
🟢 Functions 22.19% (+9.19% vs 13% ▲) 2271/10234
🟢 Lines 27.97% (+9.97% vs 18% ▲) 11833/42300

🟢 @avalabs/k2-alpine

St. Category Percentage Covered / Total
🟢 Statements 8.49% (+6.49% vs 2% ▲) 304/3578
🟢 Branches 7.90% (+6.90% vs 1% ▲) 172/2177
🟢 Functions 4.99% (+3.99% vs 1% ▲) 41/821
🟢 Lines 7.82% (+5.82% vs 2% ▲) 257/3283
Artifacts and threshold sources
  • @avalabs/core-mobile: summary core-mobile/coverage/coverage-summary.json, thresholds core-mobile/coverage-thresholds.json
  • @avalabs/k2-alpine: summary k2-alpine/coverage/coverage-summary.json, thresholds k2-alpine/coverage-thresholds.json

Source run: Mobile PR

Copilot AI review requested due to automatic review settings July 29, 2026 20:46

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

… new recurring order subscribes for push without needing a screen visit
Copilot AI review requested due to automatic review settings July 29, 2026 20:54

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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.

4 participants