fix: keep subscription payment sheet open

[ben-kaufman]

Fixes #1467

This PR keeps the subscription sheet open when an accepted proposal hands off to its first payment, avoiding a brief return to Home or the subscriptions list.

Description

• Replaces Review & Subscribe with the Send flow in place only when the initial payment matches the reviewed subscription ID and counterparty, including initial-payment preparation errors.
• Preserves the existing transition-job sequencing, payment preparation cleanup, and normal dismissal for other sheet changes.

Out of Scope

• Single-swipe on-chain payment and retaining the review layout while paying are handled separately in fix: pay first onchain subscription period in one swipe #1422; this fix works on master without that PR.
• iOS already embeds its initial payment flow inside the subscription sheet and needs no change for this issue.

Design

Subscribe to Subscription.

Preview

The sheet-host regression passed on an isolated Android 16 / API 36 emulator: expansion state, target, and offset stay unchanged for 30 frames after replacing the review with Send, with no dismissal callback. A full two-wallet subscription recording was not captured.

QA Notes

Journeys

• J1 updated review-and-subscribe.xml — the first-payment handoff keeps the sheet expanded without exposing Home or the subscriptions list.

Manual Tests

N/A

Automated Checks

• added SheetHostTest.kt — checks expansion, offset, visibility callbacks, and absence of dismissal frame by frame during the content replacement.
• updated AppViewModelSendFlowTest.kt — checks uninterrupted confirmation and preparation-error presentation, and preserves dismissal for non-initial payments, other subscription IDs or counterparties, details screens, and non-Send destinations. The two handoff regressions failed against master before the fix.
• ran SheetHostTest and SendConfirmScreenTest on an isolated API 36 emulator — all 8 instrumentation tests passed.

Local compile and all 3,962 unit tests passed. After the final test-formatting fix, all 88 subscription-focused tests passed again. Detekt/format diagnostics were compared in full against an untouched master checkout: 1,980 findings on each, with zero introduced. The full two-wallet journey remains for device QA.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the in-place replacement is limited to the matching subscription’s first payment.

Summary

Keeps the subscription sheet open when its matching first payment starts.

• The first subscription payment opens inside the review sheet.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Show next sheet] --> B{Send for the matching first payment?}
  B -- Yes --> C[Replace Review with Send in place]
  B -- No --> D[Clear current sheet and wait]
  D --> E[Show next sheet]

Loading

_{Reviews (1) · Last reviewed commit: "fix: keep subscription payment sheet ope..." · Reviewed by Greptile}

[talosmachina]

No findings. Lets showSheet swap a Subscription Review for the Send sheet without the null-and-delay step, but only when the send state is the initial payment of that same subscription. Reviewed e1394dc, full tier. I read the code and CI and did not build this branch. Bitkit Android branches are not built on this reviewer's machine.

What I checked, and 6 candidates I ruled out

Read in full: the diff; showSheet, replacesSubscriptionReview, hideSheet, onSheetVisible, acceptSubscriptionAndStartPayment, showInitialSubscriptionPaymentFailure, handleScan's send-state reset (AppViewModel.kt); SheetHost (SheetHost.kt); the Review route in SubscriptionsScreen.kt; the PaykitSubscriptionId / PaykitPaymentRequestId types.
Call sites traced: every showSheet(Sheet.Send(...)) that can run while a Review is up, and the init-block currentSheet.collect.
CI: lint, detekt, build-local and Greptile green; build and the local e2e shards still running when this was posted.

Ruled out

• Matching on the wrong key: the predicate compares paymentRequestId and counterparty and ignores billingPeriodStartsAt, which is right, since a subscription id has no period. Both success and preparation-error paths reach it because handleScan and showInitialSubscriptionPaymentFailure each set isInitialSubscriptionPayment and incomingPaymentRequestId before showSheet.
• Stale send state hijacking an unrelated Send: the flag is only set by ContactPaymentContext.isInitialSubscriptionPayment, and every scan resets the send state from the active context before showing Send. Also, nothing else can open Send while the Review sheet covers Home.
• Side effects skipped by not passing through null: the old path only did _currentSheet.update { null } plus a delay, not hideSheet(), so no cleanup is lost. Skipping the null emission also means the init collector no longer calls presentNextIncomingPaykitPaymentRequest() in the gap, so nothing else can grab the empty slot mid-handoff.
• Dismiss callback firing on the swap: in SheetHost, isVisible stays true and the key changes, so it takes the onVisible branch, not the dismiss branch. onSheetVisible(Send) then completes the request presentation exactly as it did after the old re-expand. SheetHostTest.subscriptionPaymentReplacementKeepsSheetExpanded pins this frame by frame.
• Fallback error sheet in acceptSubscriptionAndStartPayment: when the Send never appeared, it either replaces in place (state still matches) or falls back to dismiss-then-show (state reset). Both outcomes are correct.
• Non-matching transitions: the new negative test covers a non-initial payment, another id, another counterparty, Details, and a non-Send target, and all still pass through null.

Merge confidence: 4/5, the change is covered by AppViewModelSendFlowTest.kt and SheetHostTest.kt, but the main build and e2e had not finished and I did not build the branch.

[github-actions]

Regtest APK

Built from e1394dc (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[jvsena42]

Code scan at e1394dc: no findings. Device result follows.

Checked:

• replacesSubscriptionReview (AppViewModel.kt:5849-5854) only decides whether the sheet is swapped in place or dismissed and re-shown; what the Send sheet renders is _sendUiState. The predicate compares _sendUiState.incomingPaymentRequestId with the id of the Review on screen, inside the transition job on main with no suspension before _currentSheet.update (:5839), so the check and the presentation cannot be split by another request.
• isInitialSubscriptionPayment is set only by resetSendState from the accept path (:6397), the retry in the current sheet (:6572) and the failure path. A scan or deep link during the handoff resets it, so its Send sheet takes the dismiss-then-show path as on master.
• The old path was _currentSheet.update { null } plus the transition delay, not hideSheet(), so no send-state cleanup is skipped. Fee, tags, coin selection and hardware id are rebuilt by resetSendState before the swap; the two sheets are different branches in ContentView.kt, so no Compose state carries over.
• Back, swipe and scrim are disabled while isAcceptingSubscription (ContentView.kt:518-521), which is held until the scan job and the transition job return. After the swap, dismissing runs the normal Send branch of hideSheet().
• onSheetVisible still fires for the Send sheet, so the request is marked as presented; the preparation-error path shows the error route in place and stays dismissable.
• A second accept is rejected by _isAcceptingSubscription.compareAndSet (:6362); QuickPay stays out with an active contact context.
• The unit tests pin the in-place swap, the preparation error, and dismissal for a non-initial payment, another subscription id, another counterparty, a details route and a non-Send target.

One note, no change requested: PR CI does not compile or run SheetHostTest.kt (ui-tests.yml runs on master pushes and manual dispatch only), and that test drives SheetHost directly, so it also passes without the AppViewModel change. The unit tests are what cover the fix.

iOS already keeps the first payment inside the subscription sheet (SubscriptionsView.swift:787), so the platforms match after this.

perf: not relevant (one transition delay removed, no repeated work added).

Device gate (partial): e1394dc — not run yet; pending review-and-subscribe.xml on two linked emulators, with a recording of the handoff against master.

[piotr-iohk]

QA review

Scope: Reviewed the full PR diff against its merge base, at e1394dc. No prior assessment baseline was available.

No new actionable code findings.

The handoff skips the clear-and-delay only while the current sheet is still that subscription's Review and send state is its initial payment. The swap still runs inside sheetTransitionJob, so acceptance waits for the new sheet the way 0d9dc5d required. Other sheet changes still dismiss first. On iOS at 3ba47a44, the same subscription sheet changes its route to payment instead of closing; keeping the review layout during payment stays in #1422.

Validation: CI build on this revision, including testDevDebugUnitTest, succeeded. The new SheetHostTest case is not run by pull-request CI (ui-tests.yml is master and manual only); the author reports that instrumentation run separately. This review did not execute tests.

Device testing: not performed in this review.

Ready for device testing.

Suggested additional test cases

• Android, Paykit UI on, payer with a linked creator and a proposal whose first period is due, with payment preparation unable to open the updated payment list. Swipe to subscribe. The sheet stays expanded on the send error, without closing or exposing Home or the subscriptions list, and dismissing that error does not reopen Review.

[ovi-reviewer]

Verdict: ✅ Approve

QA for e1394dc:
Tests for the review: journey J1 passes.

Tested on two Android 16 emulators (Pixel 10 Pro), regtest.
Request: @ben-kaufman requested full two-wallet device QA; this run completed J1 with an on-chain first payment, recorded the uninterrupted handoff, and checked active renewal, payment history and paid-request cleanup.
Request: @piotr-iohk suggested a preparation-error check; this run exercised J1's success path and did not inject that error.

🟢 Test J1

Test J1

The recorded first-payment handoff stays expanded and the 5,000-sat payment reaches Active with its period cleared; Subscribed and Close were checked on screen between recording segments.

J1.mp4

The J1 checkbox is checked.

Tip

Test J1 worth a journey

Test J1

• Open Subscriptions and dismiss the automatically presented review without accepting.
• Verify Journey Sub under Proposals and open its Review and Subscribe sheet.
• Check the 5,000-sat monthly proposal, creator card and payment due on acceptance.
• Swipe Subscribe and Pay and observe the loading control change to Send inside the expanded sheet.
• Complete the on-chain payment and verify Subscribed.
• Tap Close, verify Active, and open Journey Sub details.
• Verify the renewal date has no year and the payment history names Journey Sub with its date.
• Verify the paid period shows Proof submitted with no Pay or Dismiss, then verify no request bell or Payment Requests sheet appears on Home for30 seconds.

---

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

[jvsena42]

Approving at e1394dc: no findings. The code scan is in my earlier review today; this is the device result.

Device gate: e1394dc — journeys: 1 passed (review-and-subscribe.xml, the due-on-acceptance branch, 3 runs); perf: swipe to the pay step 59 s, 83 s and 84 s, swipe to Subscribed 113 s and 135 s (master: 115 s and 119 s to the pay step, 155 s to Subscribed), so no slower than master; Figma: not compared (no layout change); iOS: no twin, the first payment is already embedded there.

Pixel Tablet emulator as payer (Spending balance, so the first payment starts on its own), Pixel 9 as creator, regtest, 5,000-sat monthly proposals.

• The symptom reproduces on master: in a recording, at the handoff the sheet is gone, the Subscriptions list is fully exposed for about 0.4 s, and the pay sheet slides back up from the bottom.
• On this build the sheet stays expanded through the same moment: the Review content is replaced by the pay state in place, with no frame showing the list uncovered (15 fps recording, 178 s).
• The rest of the journey holds: Subscribed → Close returns to the list with the subscription under ACTIVE, the detail shows STATUS Active, RENEWS November 12 and the payment under PAYMENTS, and the period is not offered again.

⚠️ Not run: the same handoff with a Savings or hardware payer, where the Confirm screen shows instead of an automatic payment, and the preparation-error path.

Note, outside this PR: between the swipe and the pay step the sheet shows no control for one to two minutes on both builds while the first payment is prepared. The wait is the same on master; this PR only keeps the sheet up during it.

Skipped — covered by CI at e1394dc: unit tests, detekt, lint, e2e (all green).