[ovitrif]

Refs #1624

Follow-up to #1643, from @coreyphillips' review #1643 (review).

Description

• Adds a test that the request being paid gets no expired toast when it expires: it confirms a hardware payment, then reports that request as expired and checks no toast. It fails if the active payment request is dropped from the expiry toast's exclusions.
• Corrects a test comment about when the clock jumps in refresh crossing expiry leaves the report to the expiry timer.

Out of Scope

• None.

Design

N/A — no UI changes.

Preview

N/A — no user-visible changes.

QA Notes

Journeys

N/A

Manual Tests

N/A

Automated Checks

• updated AppViewModelSendFlowTest.kt
• updated PaykitPaymentRequestRepoTest.kt
• ran AppViewModelSendFlowTest, PaykitPaymentRequestRepoTest and detekt

[github-actions]

Regtest APK

Built from 76fe2ce (run).

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

[coreyphillips]

Two independent reviews, nothing blocking a merge.

nits

• Test emits an ACCEPTED request, which the repo never reports as expired (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:1884). The new test feeds expiredPaykitProposals a request that the real PaykitPaymentRequestRepo.expiredProposals can never emit. reopenRetainedHardwareRequest builds the request with lifecycleState = ACCEPTED (AppViewModelSendFlowTest.kt:14695). The repo only reports incoming PROPOSED requests that are not in flight (PaykitPaymentRequestRepo.kt:1755-1757). The #1643 review describes the case this exclusion is for as a request that expires before acceptance while the user is paying it. The test is still a valid guard, because the view model filter compares ids only. I traced it: onSheetVisible clears requestedPaymentRequestId, and onConfirmPay sets isSubmittingPaymentRequest, which keeps the active context through the pending update. That leaves activeIncomingPaymentRequest()?.id as the only check suppressing the toast. CI at 76fe2ce shows the test passing. Emitting a proposed copy would make the input one the repo can produce, without changing what the test proves: kotlin expiredPaykitProposals.emit(listOf(request.copy(lifecycleState = PaymentRequestLifecycleState.PROPOSED)))

[talosmachina]

No findings. Adds a test that the request being paid gets no expired toast, and fixes a test comment. Reviewed 76fe2ce, light tier, reasoned from the code and CI: the branch was not run here.

What I checked, and 2 candidates I ruled out

Read in full: the diff; AppViewModel.toastExpiredPaymentRequests, finishExpiredPaymentRequestPresentation, markIncomingPaymentRequestPresented, the reopenRetainedHardwareRequest fixture
CI: build, build-local, lint, detekt green; e2e jobs still pending at review time

Ruled out

• Test passing for the wrong reason: the only path that skips the toast for a non-dismissed request is the handledIds exclusion (requested id or active request id), and the test reaches it after a confirmed hardware payment, the state the exclusion exists for. I did not run the mutation the description cites (dropping the active id), so that part rests on the author's run.
• verify(atLeast(0)) with a captor proving nothing: it captures every enqueued toast, including none, and the assertion is on their tags, so a stray expiry toast would be caught.

Merge confidence: 5/5, test-only change, unit CI green.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the runner does not report it)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-21T16:04:58.611874Z	76fe2ce	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[ovitrif]

@coreyphillips Thanks. Keeping the accepted request as is, since, as you traced, the view model filter compares ids only, so the test guards the same path either way. Switching the input to a proposed copy would cost this approval on a test-only PR.

[coreyphillips]

Rechecked after replies, with no new commit since 76fe2ce. Nothing blocking remains.

still open

• Test emits an ACCEPTED request, which the repo never reports as expired (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:1884). The new test feeds expiredPaykitProposals a request that the real PaykitPaymentRequestRepo.expiredProposals can never emit. reopenRetainedHardwareRequest builds the request with lifecycleState = ACCEPTED (AppViewModelSendFlowTest.kt:14695). The repo only reports incoming PROPOSED requests that are not in flight (PaykitPaymentRequestRepo.kt:1755-1757). The #1643 review describes the case this exclusion is for as a request that expires before acceptance while the user is paying it. The test is still a valid guard, because the view model filter compares ids only. I traced it: onSheetVisible clears requestedPaymentRequestId, and onConfirmPay sets isSubmittingPaymentRequest, which keeps the active context through the pending update. That leaves activeIncomingPaymentRequest()?.id as the only check suppressing the toast. CI at 76fe2ce shows the test passing. Emitting a proposed copy would make the input one the repo can produce, without changing what the test proves: kotlin expiredPaykitProposals.emit(listOf(request.copy(lifecycleState = PaymentRequestLifecycleState.PROPOSED))) . The code is unchanged. reopenRetainedHardwareRequest still builds an ACCEPTED request (AppViewModelSendFlowTest.kt:14695), and the test still emits it as-is (line 1884), while expiredProposals in PaykitPaymentRequestRepo.kt only reports PROPOSED, non-in-flight requests. @ovitrif's reply restates that toastExpiredPaymentRequests (AppViewModel.kt:1803-1807) compares ids only, which the original point already granted; it declines the nit rather than showing it wrong. As a nit, this should not hold the merge.

[greptile-apps]

Confidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Adds a test that confirms a hardware payment, reports its request as expired, and checks that no expiry toast appears.

• An expiring request being paid gets no generic toast.
• The expiry test comment clarifies when refresh reads the clock.

No actionable issues found. Tests were not run during this review.

_{Reviews (1) · Last reviewed commit: "test: cover no expiry toast for the requ..." · Reviewed by Greptile}

[coreyphillips]

Rechecked after replies, with no new commit since 76fe2ce. Nothing blocking remains.

still open

• Test emits an ACCEPTED request, which the repo never reports as expired (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:1884). The new test feeds expiredPaykitProposals a request that the real PaykitPaymentRequestRepo.expiredProposals can never emit. reopenRetainedHardwareRequest builds the request with lifecycleState = ACCEPTED (AppViewModelSendFlowTest.kt:14695). The repo only reports incoming PROPOSED requests that are not in flight (PaykitPaymentRequestRepo.kt:1755-1757). The #1643 review describes the case this exclusion is for as a request that expires before acceptance while the user is paying it. The test is still a valid guard, because the view model filter compares ids only. I traced it: onSheetVisible clears requestedPaymentRequestId, and onConfirmPay sets isSubmittingPaymentRequest, which keeps the active context through the pending update. That leaves activeIncomingPaymentRequest()?.id as the only check suppressing the toast. CI at 76fe2ce shows the test passing. Emitting a proposed copy would make the input one the repo can produce, without changing what the test proves: kotlin expiredPaykitProposals.emit(listOf(request.copy(lifecycleState = PaymentRequestLifecycleState.PROPOSED))) . The test at AppViewModelSendFlowTest.kt:1884 still emits listOf(request) from reopenRetainedHardwareRequest, which still builds the request with lifecycleState = ACCEPTED (line 14695). The repo's expiredProposals filter still requires PROPOSED. The only reply is Greptile's general summary, which doesn't mention this point. Emitting request.copy(lifecycleState = PaymentRequestLifecycleState.PROPOSED) would settle it.

[ovitrif]

@coreyphillips thanks, merged.