fix(qr-pay): cashier-action copy when the merchant charge expires (TASK-21384) - #2906
Merged
Conversation
Manteca's PAYMENT_DESTINATION_EXPIRED means the charge the cashier entered on the till timed out, not that the QR expired. On a static Mercado Pago POS sticker the QR never expires, so the app was telling users the payment rail was down when the fix was one sentence to the cashier. Three parts: the copy names the cashier action in en, es-419 and pt-BR; the code joins the non-retryable list, because retrying the same dead charge only burns the attempt budget; and 409 joins 400 and 422 on the qr-payment/init Sentry ignore list, since all three are user-facing outcomes, not server bugs. Needs peanut-api-ts #1484 deployed first — that is what turns the error field from "Unexpected error" into PAYMENT_DESTINATION_EXPIRED. TASK-21384
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
Code-analysis diffPainscore total: 7152.21 → 7145.25 (-6.96) 🆕 New findings (30)
…and 10 more. ✅ Resolved (30)
…and 10 more. |
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean: the exact 409 contract reaches the cashier-action copy, the typed rejection does not retry, and telemetry keeps the frequency signal without sending QR payload data.
Findings
- MINOR · src/utils/sentry.utils.ts:39 · [moonshotai/kimi-k3] Sentry 409 ignore on qr-payment/init suppresses all conflicts, not just the expired-charge code
Failure scenario: the ignore entry matches only on URL pattern + HTTP status, so after this ships, any 409 from qr-payment/init is silently dropped from Sentry. 409 Conflict is the canonical status payment backends use for duplicate / already-in-flight initiation conflicts — and this screen has client-side retry logic that can produce concurrent in-flight initiations. If Manteca/peanut-api ever returns 409 for a double-submit or state conflict (a payments race worth investigating), there will be zero server-side signal: the client falls through to a generic error branch and Sentry discards the event. Evidence: src/utils/sentry.utils.ts line 39,{ pattern: /qr-payment\/init/, statuses: [400, 409, 422] }— the SKIP_REPORTING structure cannot distinguishPAYMENT_DESTINATION_EXPIREDfrom any other 409 body. Fix: extend the SKIP_REPORTING entries with an optional error-code/message matcher (e.g. only ignore when the response/message containsPAYMENT_DESTINATION_EXPIRED), or keep 409 reportable until the backend filter can be scoped to that code.
Checked clean
- Verified the detached worktree head, supplied base SHA, and merge base; the review covers only the seven-file 32-line change at the exact head.
- Traced the response contract through mantecaApi: response.error becomes Error.message, and the local backend branch maps PAYMENT_DESTINATION_EXPIRED to HTTP 409 with that exact error code.
- Checked retry and state behavior: PAYMENT_DESTINATION_EXPIRED is non-retryable, clears merchant-waiting state, and renders the dedicated error instead of the provider-outage fallback.
- Checked English, Latin American Spanish, and Brazilian Portuguese copy plus the QR-pay state test; the new test asserts the cashier action and rejects the outage message.
- Checked telemetry and Sentry boundaries: the PostHog event includes only qr_type, and the 409 suppression remains limited to qr-payment/init where the backend classifies this expected outcome.
- Checked current CI at the exact head: unit, typecheck, eslint, format, analyze, deploy preview, and aggregate ci-success completed successfully; ds-shots was still running and no design-system files changed.
- Ran correctness, security, adversarial, and slop passes; no reachable regression or actionable defect survived verification.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: e8a2f858b65d · Context: repo, engineering, ci
abalinda
marked this pull request as ready for review
September 1, 2026 13:50
abalinda
requested review from
0xkkonrad,
Hugo0,
jjramirezn and
kushagrasarathe
as code owners
September 1, 2026 13:50
abalinda
requested review from
kushagrasarathe
and
a lite review from Copilot
September 1, 2026 13:50
There was a problem hiding this comment.
Pull request overview
Updates the QR Pay UX and telemetry for Manteca’s PAYMENT_DESTINATION_EXPIRED outcome so users are instructed to take the correct cashier action (re-enter the amount) instead of seeing a misleading “provider outage” message. This also reduces unnecessary retries and noise in Sentry while preserving observability via PostHog.
Changes:
- Adds a new localized error copy key
qrPay.errors.merchantChargeExpired(en, es-419, pt-BR) and renders it whenPAYMENT_DESTINATION_EXPIREDis returned. - Treats
PAYMENT_DESTINATION_EXPIREDas non-retryable in the/qr-payinit query to avoid burning the retry budget on a deterministic outcome. - Suppresses Sentry reporting for
409onqr-payment/initand adds a new PostHog eventqr_merchant_charge_expired_shownto keep a frequency signal.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/sentry.utils.ts | Adds 409 to the Sentry skip-reporting rule for qr-payment/init. |
| src/i18n/app/messages/en.json | Adds English merchantChargeExpired copy under qrPay.errors. |
| src/i18n/app/messages/es-419.json | Adds es-419 merchantChargeExpired copy under qrPay.errors. |
| src/i18n/app/messages/pt-BR.json | Adds pt-BR merchantChargeExpired copy under qrPay.errors. |
| src/constants/analytics.consts.ts | Introduces QR_MERCHANT_CHARGE_EXPIRED_SHOWN analytics event constant. |
| src/app/(mobile-ui)/qr-pay/page.tsx | Adds non-retryable code + maps expired-charge error to the new copy and PostHog event. |
| src/app/(mobile-ui)/qr-pay/tests/qr-pay-states.test.tsx | Adds a regression test ensuring the cashier-action copy renders and outage copy does not. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… code Chip review on #2906 (second opinion, kimi-k3): suppressing every 409 on qr-payment/init by status alone would swallow any future conflict on that route — a double-submit or an in-flight state clash is exactly the kind of payments race worth a Sentry event. SKIP_REPORTING rules now take an optional errorCodes list. A rule without one still decides on URL and status alone, so nothing else changes; the 409 entry carries PAYMENT_DESTINATION_EXPIRED and matches on the error field of the body, which is already parsed a few lines down. Two tests pin both halves: the expired charge stays silent, a different 409 on the same route still reports.
Contributor
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean. The expired merchant-charge path now shows actionable cashier copy, avoids futile retries, preserves frequency analytics, and suppresses only the matching 409 error code from Sentry.
Checked clean
- Earlier reviews contained no findings to re-evaluate.
- Verified the detached worktree, exact head SHA, base ref, base SHA, and merge base.
- Traced PAYMENT_DESTINATION_EXPIRED from the API response through fetch reporting, service error propagation, React Query retry policy, UI copy, and PostHog capture.
- Verified the scoped Sentry rule suppresses PAYMENT_DESTINATION_EXPIRED while a different qr-payment/init 409 remains reportable; unit coverage pins both branches.
- Validated all changed translation JSON files and git diff whitespace; unit, typecheck, lint, format, analyze, and aggregate CI checks passed on the exact head.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: daa998a29b4c · Context: repo, ci, backend
jjramirezn
approved these changes
Sep 2, 2026
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
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.
Summary
When Manteca returns
PAYMENT_DESTINATION_EXPIRED, the app showed "We are currently experiencing issues with Mercado Pago payments" — so users reported Peanut as down. It is not an outage, and it is not the QR: on a static Mercado Pago POS sticker (mpago.la/pos/<id>, EMV tag 01 = 11, no tag 54) the QR never expires. What expired is the charge the cashier entered on the till. Rescanning alone does not fix it. The cashier must enter the amount again.Three parts:
qrPay.errors.merchantChargeExpiredinen,es-419andpt-BR, naming the cashier action.PAYMENT_DESTINATION_EXPIREDjoinsNON_RETRYABLE_QR_PAY_ERRORS. Retrying the same dead charge cannot change the outcome, so it only burns the 3-attempt budget.409onqr-payment/initis suppressed, but scoped toPAYMENT_DESTINATION_EXPIRED, not to the status. This is what closes PEANUT-UI-9FR.SKIP_REPORTINGrules now take an optionalerrorCodeslist; a rule without one still decides on URL + status alone, so the existing entries behave exactly as before. Added after review — see below.A
qr_merchant_charge_expired_shownPostHog event keeps the frequency signal that Sentry stops carrying once this is no longer an error.Copy shipped (review the wording here — see the screenshots note below):
Task
TASK-21384 — https://app.notion.com/p/3ba838117579811082e3ce25496e2495
Risks / breaking changes
new Error(errorData.error || …), and today that field isUnexpected error— the backend has no branch for this code, so it falls to a 500. Until #1484 is live these branches never fire and behaviour is unchanged. Merging this first is harmless, just inert.The Sentry
409entry is scoped to theqr-payment/initpattern, so no other route's 409s are hidden.Design notes / accepted trade-offs
Review response (Chip second opinion, kimi-k3): the first version of this PR suppressed every 409 on
qr-payment/initby status alone. That would have swallowed any future conflict on the route — a double-submit or an in-flight state clash is exactly the kind of payments race worth an event. TodayPAYMENT_DESTINATION_EXPIREDis the only 409 the route can return (the classifier is its sole source; the one other 409 in the Manteca routes,deposit.ts:178, is a path this pattern does not match), so the finding was about tomorrow, not today. It was still the right call: the body is already parsed a few lines below the skip check, so scoping cost ~15 lines and removed a trap nobody would have found later. Fixed indaa998a2, with tests on both halves.Fixtures do not cover qr-pay error states, and
Fixture.failscan only answer 500 with a fixed body — it cannot express "409 with this error code". Extending that contract is framework work that does not belong in a bugfix, so it is noted as a follow-up rather than done here. That is also why there are no screenshots.QA
npx jest qr-pay-states— new case asserts the cashier copy renders and that the "currently experiencing issues" toast does not.Screenshots:⚠️ NONE — the state needs Manteca to return
PAYMENT_DESTINATION_EXPIREDagainst a real till, which no local sandbox or fixture can produce, and which needs #1484 deployed first. The exact copy is quoted above so the wording is still reviewable.