Skip to content

feat(topup): ask the backend what happened instead of claiming a deposit - #700

Open
islandbitcoin wants to merge 14 commits into
mainfrom
feat/fygaro-signed-checkout-app
Open

feat(topup): ask the backend what happened instead of claiming a deposit#700
islandbitcoin wants to merge 14 commits into
mainfrom
feat/fygaro-signed-checkout-app

Conversation

@islandbitcoin

@islandbitcoin islandbitcoin commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

App half of the signed-checkout work. Pairs with flash#486, flash#487 and flash#488 — all merged and deployed (chart 3.2.68).

Why

On any Fygaro success redirect, this app navigated to "Payment Successful / Your payment has been processed successfully / Deposited to <wallet>" with a fabricated txn_${Date.now()}, having asked the backend nothing.

On 2026-08-16 one customer saw that screen three times. Twice, nothing had been deposited. Fygaro capturing the card and Flash crediting the wallet are different events, and only the second is the one they came for.

The same incident had a second cause: the app knew only the flat per-level cap. $100, $80 and $60 each pass a $125 limit individually, and nothing on the device knew $180 was already spent.

What changes

The checkout is requested from the server, which authorises the amount before the card is charged and signs it into the link. A refusal there costs the customer nothing; the same refusal at the webhook means they have already paid.

The success screen reports rather than asserts. It polls fygaroTopupStatus and shows what it is told: credited (with the net that actually landed), held for review (with the server's own wording), "we've received your payment and are crediting it", or — distinctly — "we haven't seen this payment yet", because the payment page closes on a decline exactly as it does on a success.

The amount gate checks what is REMAINING, not the flat cap, and the screen shows spent and held next to it so the gap is explicable. Below the minimum it says the limit is used up rather than quoting a figure no amount can satisfy.

Failing safe, deliberately

Refusing is the residual. Degrading to the legacy editable link has to be earned by positive identification — the server rejected our document, or the request never reached a server. Every unrecognised error shape refuses, because a fall-through that fails open into capture-without-credit is the wrong fall-through however many doors are individually closed.

Never a spinner past 10s. Poll 1s for 10s, then resolve to a definite state; a quieter 5s poll continues ~60s so a late credit still upgrades the screen. The deadline is its own timer — a deadline that waits on the request it is a deadline for is not a deadline.

A failed poll changes nothing on screen. It tells us nothing about the payment; showing "failed" because we could not ask would be inventing an outcome.

fygaroCheckoutCreate is on noRetryOperations — it mints an allowance reservation, so a RetryLink replay would silently eat the customer's own allowance.

Every new operation is isolated in its own document, matching the cardTopupLimits precedent: an older backend rejects a whole document over one unknown field, which once hid the Transfer button for every user on test.

Deploy ordering

flash#487 is deployed (verified: fygaroTopupAllowance and fygaroTopupStatus are live on the prod schema), so this is unblocked. With fygaro.checkout.enabled still off, the app degrades to exactly current behaviour; the signed path activates when the flag flips.

Tests

77 suites / 655 tests green, tsc clean, changed-lines lint clean, translation drift clean.

New coverage includes: a request that never settles still resolves; a terminal answer stops the poll; UNCONFIRMED never renders as receipt; a 429/401/403 refuses rather than degrading; an unrecognised error shape refuses; a tampered/expired case; the incident replay ($60 against a $125 cap with $100 spent is blocked and names the $25 left); Continue held while the allowance is in flight; and the below-minimum copy rule.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV

Dread and others added 14 commits August 18, 2026 13:33
On any Fygaro success redirect this app navigated to "Payment Successful
/ Your payment has been processed successfully / Deposited to <wallet>"
with a fabricated `txn_${Date.now()}`, having asked no one. On
2026-08-16 one customer saw that screen three times; twice nothing had
been deposited. The card charge succeeding and Flash crediting the
wallet are different events, and only the second is what they came for.

Three changes, all of them about not asserting what we cannot know:

The checkout is now requested from the server, which authorises the
amount BEFORE the card is charged and signs it into the link. A refusal
at that point is free; the same refusal at the webhook means they have
already paid. When the server will not or cannot answer — feature off,
older backend, network — this falls back to the device-built URL that
has always worked, because a dead Top Up button is worse than an
editable one.

The success screen polls `fygaroTopupStatus` and reports what it is
told: credited (with the net that actually landed), held for review
(with the server's own wording, which is the only side that knows which
threshold was tripped and by how much), or "we've received your payment
and are crediting it". Never a spinner past 10s — at that point the
customer is owed a definite answer, and the quiet poll continues for a
further 60s so a late credit still upgrades the screen.

The amount gate now checks what is REMAINING, not the flat per-level
cap. Checking each amount against the cap is precisely why $100, $80 and
$60 all passed the client against a $125 limit: each is individually
under it, and the app had no idea $180 was already spent. The screen
also shows spent and held alongside remaining, because "you've spent $0
and have $65 of $125" is otherwise unexplainable.

Every new operation is isolated in its own document, matching the
`cardTopupLimits` precedent: an older backend rejects a whole document
over one unknown field, so these must fail alone rather than take an
unrelated screen down with them.

REQUIRES flash#487 deployed before this ships — fygaroTopupAllowance and
fygaroTopupStatus do not exist on an older backend, and a query naming
them is rejected outright rather than returning null.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…held payments

Review fixes for the signed-checkout PR.

Blocking:

- `requestCheckout` was recreated every render, and Apollo re-renders the
  screen synchronously when a mutation is invoked. The checkout effect
  depended on it, so firing the request tore the effect down, cancelled the
  in-flight run and discarded the answer: `paymentUrl` stayed null forever
  and card top-ups were dead on arrival. Wrapped in `useCallback`, and the
  effect now depends only on what identifies the request (`LL`/`navigation`
  moved to a ref).
- The error screen was derived from "no URL", which is also the normal state
  while the server is answering, so "Something went wrong / Retry" was the
  first paint of every card top-up. Checkout is now an explicit
  requesting/ready/failed status; the spinner covers the round trip, and
  Retry re-requests instead of only refetching the account query.
- `exceedsDailyLimit` applied the Fygaro CARD allowance to bank transfers and
  Bridge deposits, blocking a $500 wire against whatever was left of a $125
  card allowance. Guarded on `isCard`, and the allowance query is skipped
  off the card flow.
- Held and failed fell back to `pendingMessage` when the backend sent no
  reason, telling customers whose payment was frozen for manual review that
  we were "crediting your wallet". Both now have their own headline, message
  and icon.
- No test file existed for payment-success-screen, the screen the PR is named
  after. Added one covering every phase.

Should-fix:

- Polling continued for the full 70s after a terminal answer — ~21 wasted
  round trips and re-renders per credit, and the same for HELD_FOR_REVIEW,
  which retrying cannot change. Terminal states now clear the timers.
- Removed the dead `refused ? receivedTitle : receivedTitle` branch.
- `spent`/`resetsAt` were queried and converted but rendered nowhere; dropped
  them and put the unused `allowanceResets` string to work showing when a
  hold lapses.
- Gave the CardPayment mutation mock real `loading` state that flips on
  invocation, so a mock that cannot re-render stops hiding this class of bug;
  added coverage for the signed URL and the server's checkoutId.
- `payload.errors?.[0]` only inspected the first error, so a non-customer
  error sorted ahead of FYGARO_DAILY_ALLOWANCE_EXCEEDED would hand the
  customer the legacy editable link and let them be charged for a top-up the
  webhook then refuses. Now searches the whole array.
- Deleted the dead `secondaryButton` style and the unreferenced
  `PaymentSuccessScreen.transactionId` / `viewTransaction` strings.

Every behavioural fix is pinned by a test that fails when the fix is reverted.
…e refusal on screen

Review fixes for PR #700.

- use-fygaro-topup-status: a failed poll no longer returns before the
  fast-window check. It returned, so `resolvedRef` was never set and the
  phase never left `checking` — on a flaky connection, any 5xx, or a
  backend older than this query (which rejects it outright), the customer
  sat on "Confirming your top-up" until the last timer was cleared at
  t=70s and then forever after. A permanent spinner on the screen they
  land on immediately after being charged, breaking the PR's own "never
  a spinner past 10s" contract. `status` now stays undefined so the
  terminal branch is skipped and the window still resolves to `pending`.

- payment-success-screen: "Crediting to" was rendered for every
  uncredited phase, so "Payment on hold" was followed one row later by
  "Crediting to: USD Wallet", and "Payment not credited" sat directly
  above it. The stalled phases get a neutral `wallet` label; the row
  still names the wallet, because an account with two of them needs to
  know which one this was.

- CardPayment: a customer refusal is its own state carrying the server's
  sentence, rendered as the screen body with a "Change amount" button.
  It used to set `failed` and put the reason only in an Alert — which is
  cancelable by default on Android, so the back button dismissed it
  without firing goBack and left the customer on a generic "Something
  went wrong" next to a Retry that re-requested the same amount and was
  refused identically.

- CardPayment: the signed link is retired when it expires. `expiresAt`
  was selected and ignored, so someone who stepped away came back to a
  form the provider would reject, with no explanation. Guarded against
  the 32-bit setTimeout ceiling, which fires an over-range delay
  immediately and would kill a good link on arrival.

- Dead payload dropped from the documents, types and mapping:
  `authorizedAmount` on the status query, `amount` and
  `remainingAllowance` on the checkout mutation, and
  `remainingAllowanceCents` off the public refusal type.

- i18n regenerated. raw-i18n/source/en.json had none of the new keys and
  still carried the two deleted ones, so the new copy never entered the
  Crowdin pipeline and check:translation-drift passed vacuously against
  a stale source. Regenerating it exposed a second hole: every locale
  dictionary is merge({}, en, rawTranslated), and all 23 translation
  files carried the OLD English "Payment Successful / Your payment has
  been processed successfully" verbatim — which overrode the corrected
  en, so this PR's entire headline fix applied to English only. Both
  strings refreshed (only where the stale English was still in place),
  and the 19 new keys backfilled, per the precedent in 170e15a.

Tests: 76 suites / 606 green (was 552). New coverage pins each fix:
every poll failing still resolves to pending and is not `checking` once
the timers are dead; held and failed never render "Crediting to"; the
refusal text survives a dismissed alert and offers no Retry; the link
expires, does not expire early on an absurd deadline, and never starts a
clock for a legacy link; and no locale may claim success in the old
wording or miss a payment-outcome key. Each was verified to fail against
the unfixed code. No existing test weakened.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Three blockers left after the review rounds, two of which the rounds
themselves introduced.

The permanent spinner was only half fixed. Every path to resolving the
screen still sat inside `tick`, behind `await fetchStatus(...)`, so a
request that never SETTLES never reached it — and React Native sets no
default network timeout on Android while this app's HttpLink passes no
AbortController, so a stalled connection yields a promise that hangs
rather than one that rejects. The deadline is now its own timer, which
is the only shape that can survive the thing it is a deadline for. Test
drives a request that never settles.

The signed-link expiry screen is REMOVED rather than repaired. It
asserted "Nothing has been charged" — a fact only the backend knows —
and it fired on wall-clock time, so it could kill a payment mid-3DS
while the customer waited on an OTP, and a device fifteen minutes fast
killed every top-up on arrival with no way out but a loop. It was added
to solve a customer returning to a stale form; the provider already
rejects an expired token and says so, and the status poll reports the
truth afterwards. Removing it drops two blockers and a whole class of
clock-skew failure, and takes an unbacked claim off the screen — which
is the entire premise of this PR.

Continue is now held while the allowance is in flight. That query is
network-only (always a round trip) while the level resolves instantly
from cache, so the gap between them was a window where Continue was
enabled and the screen quoted the FLAT cap — inviting $125 from someone
with $25 left, then refusing it. Same posture the level already takes:
hold the flow rather than read "not known yet" as "no limit".

Also dropped `requesting` and `refetch` from the hooks' public surfaces;
both were consumed nowhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
flash#487 split UNCONFIRMED out of PROCESSING, and the distinction is
load-bearing here: the Fygaro payment page closes on a DECLINE exactly
as it does on a success. Mapping everything non-terminal to "pending"
told a customer whose card bounced that we had received their payment
and were crediting their wallet — the same false claim this PR exists to
remove, one state later.

`unconfirmed` is now its own phase, and the deadline resolves to it
unless the server has actually answered PROCESSING at least once. That
also covers the case where we could not ask at all: "we don't know" and
"we have your money" are different things, and only one of them is safe
to assert. A later poll can upgrade unconfirmed → pending, never the
reverse.

Copy says what is true in both directions: "If your card was charged,
we'll credit your wallet and let you know. If it wasn't, nothing has
left your account and you can try again."

A legacy device-built link stays `pending` deliberately. It has no id to
ask about, but the app only reaches this screen on a Fygaro SUCCESS
redirect, which IS evidence the card was charged — just not evidence
that Flash credited it, which is the whole distinction. `unconfirmed` is
for the signed path, where we can ask and are told nothing was seen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
Review fixes for the signed-checkout work.

An allowlist of "refusals we recognise" cannot be maintained from the app
side of the wire: the server owns the enum and grows it without this file.
It was already stale — it had no FYGARO_ALLOWANCE_UNAVAILABLE, which is the
backend deliberately failing CLOSED when it cannot measure the allowance at
all (ERPNext or Redis down). That unknown code fell through to "degrade",
loaded the legacy editable ?amount= link, and let the customer be charged
during exactly the outage in which the server had just refused to authorise
— while the webhook, reading the same unavailable data, 500s without
crediting. Card captured, wallet not credited: the 2026-08-16 incident,
reproduced by the change meant to end it. Inverted: only
FYGARO_CHECKOUT_DISABLED degrades, everything else is a refusal rendered
with the server's own wording.

The `unconfirmed` phase — "no payment observed; the card may have been
declined" — fell through the wallet-label and icon ternaries to "Crediting
to: USD Wallet" under the ⏱ "on its way" icon, beneath the headline "We
haven't seen this payment yet". The claim this PR removes, reintroduced for
the decline path. Claiming a credit is now an allowlist (credited, pending)
rather than "not held or failed", so a phase added later has to opt in.

The allowance gate held Continue on a network-only query with no deadline
anywhere in the stack — no RN timeout on Android, no AbortController in
HttpLink — so a stalled connection left a permanent unlabelled spinner and
an early-returning handleContinue: the card flow unstartable, no error, no
escape. It now falls back to the flat per-level cap after 5s.

Also: delete the wreckage of the expiry screen removed in f4827d4
(expiresAtSeconds, the mutation field, and the expiredTitle/expiredMessage/
startAgain strings still sitting in 24 locale files), drop the unreachable
checkout-retry branch and the checkoutAttempt state that drove it, and
revert the drive-by reformatting of five unrelated hooks that turned the
changed-lines Lint gate red.

Tests: each new case verified to fail against the unfixed code first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…ance on return

Two review findings on the card top-up flow, both of them the same shape as
bugs this branch already fixed elsewhere.

The checkout mutation was the last of the three network gates here with no
deadline. Nothing under it can produce one: Apollo's HttpLink is constructed
bare (no fetchOptions, no AbortController), RetryLink only retries on an error,
and a hang never produces one — so on a stalled Android connection the promise
neither resolved nor rejected, `checkout` stayed "requesting" forever, and the
screen showed "Loading…" with no WebView, no error screen and no Retry. That is
worse than before the signed link existed, when the URL was built synchronously
and a dead network at least reached the WebView's own onError and its working
Retry. The request now races a separate timer that resolves to `unavailable`,
which loads the legacy link — the same degrade a network failure already gets.

The allowance was fetched once on mount and never again. TopupDetails is not
unmounted when it pushes CardPayment, and asking for a checkout MINTS a
reservation, so a customer with $65 left who enters $60 and is then refused
(or simply backs out) came back — via goBack() from either the refusal screen
or the refusal alert — to a screen still reading "$65.00 of $125.00 left today"
and still gating Continue against $65. It invited the very top-up it was about
to refuse. useCardTopupAllowance now returns a refetch (a no-op while the query
is skipped, and rejection-safe) and TopupDetails re-asks on focus, following the
pattern its sibling TopupCashout already uses.

Tests: a CardPayment case whose mutation never settles at all, asserting the
legacy URL mounts rather than the spinner persisting; a companion case pinning
that a prompt answer does not wait out the deadline; and a TopupDetails case
that returns to the screen and asserts both the rendered line and the Continue
gate move from $65 to $5, plus one that the card allowance is not re-asked on a
bank transfer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
The checkout deadline resolved to `unavailable`, which loads the legacy
editable `?amount=` link — and that reopens the 2026-08-16 incident
through a different door.

`unavailable` is the right answer for the three cases the hook actually
mints it for: a network throw means the request never reached the
server, an older backend has no mutation, and CHECKOUT_DISABLED means
the feature is off. In all three the server genuinely has no opinion, so
the legacy link is exactly the status quo.

A TIMEOUT is different in the way that matters. The server may well have
decided — and refused — and we simply did not wait to hear it. Handing
over an editable link there charges a customer the backend was in the
middle of protecting.

So it is now its own result kind, and it refuses. Nothing has been taken
at that point, so "check your connection and try again" costs the
customer nothing, while an unauthorised capture costs them a payment we
then cannot credit. Free to say no now, expensive later — the asymmetry
this whole screen exists to exploit.

Also fixes the two prettier errors the changed-lines CI gate caught;
`node scripts/ci/lint-changed.mjs` is now clean, which is the check that
actually answers the question rather than a whole-repo lint drowning in
pre-existing debt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
… credit

Review fixes across the card top-up flow. Each one is a place the screen
could still charge a customer we then cannot credit, ask a question twice,
or claim something nobody was asked.

- graphql: add `fygaroCheckoutCreate` to `noRetryOperations`. The mutation
  MINTS an allowance reservation, so the global RetryLink (max 5, ~300ms
  backoff, all inside CardPayment's 10s deadline) could hold $60 five times
  over on a lost response — customer charged nothing, locked out of card
  top-ups until the holds lapse. The list moves to its own module so the
  invariant is asserted against the generated documents in a test.

- use-fygaro-checkout: stop blanket-degrading in the catch. `errorPolicy`
  defaults to "none", so client.mutate rejects on top-level GraphQL errors,
  not just on a dead network — a resolver throwing (ERPNext/Redis down) used
  to load the legacy editable link and reproduce the 2026-08-16 incident
  through the one path with no test. Only a transport failure or a schema
  rejection degrades now; anything else is a new `serverError` refusal.

- CardPayment: the outer `run().catch` failed OPEN downstream of the branch
  that must fail closed — a throw after the refusal state write replaced it
  with the link the server had just refused. It now refuses.

- use-fygaro-topup-status: guard the poll with an in-flight mutex. A round
  trip longer than the 1s cadence stacked ~22 concurrent network-only
  queries at the backend the customer is waiting on.

- use-fygaro-topup-status / payment-success-screen: give the no-checkout-id
  case its own `unaskable` phase. It resolved to `pending`, so every card
  top-up in production (signed checkout off) claimed "we are crediting your
  wallet", with a clock and "Crediting to", from a Fygaro redirect — the same
  claim this flow exists to stop making.

- use-card-topup-allowance / TopupDetails: `notifyOnNetworkStatusChange` plus
  an exposed `refreshing`, folded into `cardAllowancePending`. The on-focus
  refetch closed only half its loop: Apollo keeps serving the stale figure,
  so Continue was waved through against an allowance already spent. The
  deadline is now armed per hold rather than once per mount.

- i18n: localise the payment-failure alert and the TopupDetails alert titles,
  and regenerate raw-i18n (which also restores `checkoutTimedOut`, added to
  `en` earlier in this branch but never exported, so translation drift was
  failing).

Every fix has a test that fails without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…d the wrong thing

Review fixes on the signed-checkout PR.

use-fygaro-checkout: refuse on a 5xx. The catch split on `graphQLErrors`
alone, but Apollo turns EVERY response with `status >= 300` into a
ServerError on `networkError` and leaves `graphQLErrors` empty
(link/http/parseAndCheckHttpResponse.js → link/utils/throwServerError.js).
So a 502/503/504 from the ingress, or a 500 out of a failed
apollo-server context function — exactly what an ERPNext/Redis failure
upstream of the resolver produces — degraded to the editable `?amount=`
link with no pre-charge allowance check. The customer pays and the
webhook, reading the same 5xx backend, cannot credit: the 2026-08-16
incident, through the last door left open. Putting fygaroCheckoutCreate
on noRetryOperations made it MORE reachable, not less, since the
RetryLink no longer papers over the first transient 502. 4xx still
degrades — that is the status an older backend rejects an unknown field
with, and the legacy link exists for exactly that.

payment-success-screen: `unaskable` gets its own glyph. It fell through
to ⚠, and it is the phase EVERY card top-up lands in while the signed
checkout is off — so a successful payer saw an 80px warning triangle
over "Payment received". The colour split already had this right
(warning, not error); the glyph did not.

CardPayment: stop blaming the amount for failures that never judged it.
`serverError`, `timedOut` and the fail-closed catch all rendered "We
couldn't set up your payment / please try again" under the headline
"Can't top up this amount", above a button whose only label was "Change
amount" — so a customer whose backend is down retries $50, $40, $30 into
the identical error and concludes their account is limited. Those three
now carry `retryable`, which picks a headline and a button that agree
with the sentence between them. An amount refusal is unchanged.

Tests: 5xx → serverError and the apollo-server 400 rollback shape →
unavailable in the hook spec (the 400 case pins the threshold at 500);
a 5xx reaching CardPayment mounts no WebView; the retryable refusals
assert their headline and button; `unaskable` asserts no ⚠; and
TopupDetails now pins `notifyOnNetworkStatusChange` + `network-only` on
the allowance query, which its module-level mock could not otherwise
observe — deleting either line left all 632 tests green while the
on-focus refetch silently stopped gating.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
… absence

Review fixes. Five doors that all fail the same way — the app deciding
something on the customer's behalf that it was never told.

use-fygaro-checkout: the fail-closed rule ("an error the server DID return
is a refusal") only held for 5xx. Apollo turns every `status >= 300` into a
ServerError on `networkError` with an EMPTY `graphQLErrors`, so a 401 from a
revoked token, a 403, or the 429 the gateway rate-limits with all fell
through to `unavailable` — and CardPayment then loaded the editable
`?amount=` link with no pre-charge allowance check. `fygaroCheckoutCreate`
is on `noRetryOperations`, so the 429 lands on the first attempt, while
`useHomeAuthedQuery` still serves a username cache-first: the screen looks
signed in, degrades, and the customer is charged for an over-limit top-up
the webhook parks in HELD_FOR_REVIEW. That is 2026-08-16 again.

The HTTP branch is now inverted the same way the payload allowlist is:
degrade only on a 400 whose BODY (`networkError.result.errors`) proves it
was our own document being rejected — the old-backend/rollback case the
legacy link exists for — and refuse on every other answer. The existing 400
test claimed to pin that and did not: it passed only because every non-5xx
degraded, so it would have passed with 429 in place of 400. It now passes
for the stated reason, alongside its negative twins (400 "Rate limit
exceeded", 400 with no body, 401, 403, 429).

CardPayment: the explicit outcome lost to the keyword heuristic.
`successParam === "1" || hostAndPath.includes("success")` ran first, so
Fygaro's own decline return — `/checkout/payment_success?success=0` — took
the success branch and the `success=0` arm was unreachable for that shape.
A declined card landed on "Payment received". `success=0` is now read
first; the keyword match stays as the fallback for redirects carrying no
param.

TopupDetails: the 5s deadline released the hold for both reasons the
allowance can be untrustworthy, but only the first-load one has a safe
fallback. On a refresh the figure is the PRE-reservation one, so the screen
quoted "$65.00 of $125.00 left today" after the server had taken $60, waved
the same $60 through, minted a second hold and refused again. Past the
deadline that figure is now discarded and the flat cap applies, rather than
a number we know the server has superseded.

payment-success-screen: `resolution.reason ?? heldMessage()` only falls back
on null, so an empty reason rendered a blank body under "Payment on hold" —
on the screen a customer lands on straight after being charged. `||`.

TopupSuccess: `PaymentSuccessScreen.title` now means "the backend confirmed
the credit", and this screen rendered it under a green success animation
without asking anything. It was unreachable — registered in root-navigator,
navigated to from nowhere — so it is deleted rather than left for the next
person to route to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
…s still an answer

Two fail-closed controls that were only closed against the failure they
were written for.

The stale-allowance guard shut the stall door and left the failure door
open. `allowanceSuperseded` was gated on `allowanceRefreshing`, which is
`networkStatus === NetworkStatus.refetch` — and a refetch that REJECTS
never passes through that status at all. Apollo routes it via
`ObservableQuery.reportError` into useQuery's error observer, which
re-serves `previousResult.data` with `loading: false` and
`NetworkStatus.error`. So the PRE-reservation figure came back looking
fresh, no deadline was ever armed, and the screen gated money on it.

That is the loop the on-focus refetch was added to close, reached through
that refetch's most likely failure: L1, $125 cap, nothing held. Enter $60,
Continue mints a $60 server hold, back out of the Fygaro page. The focus
refetch fires and the connection drops for that one round trip. The screen
re-renders "$125.00 of $125.00 left today", shows no held line, gates
against $125, waves through another $65 — a second hold, $125 held, $0
available — and card top-ups are locked out until both lapse.

`useCardTopupAllowance` now derives `stale` (a retained figure plus a
failed attempt can only mean a refresh did not land; a failed FIRST load
has no previous data and is already on the flat cap), and TopupDetails
discards the figure on it. No deadline applies — there is no round trip
left to wait out — so the flat cap takes over immediately and Continue
stays usable.

The checkout hook's whole fail-closed design rested on `createCheckout`
REJECTING, and Apollo stops rejecting the moment anyone adds an `onError`:
`useMutation`'s catch returns `{ data: undefined, errors: error }` instead
of rethrowing whenever one exists on the hook or execute options. One line
added for Sentry, in another file, would have sent every 5xx, 429, 403 and
thrown resolver past the catch to `!payload` → `unavailable` → the
editable `?amount=` legacy link with no pre-charge allowance check. All 21
existing tests would still have passed, because every one asserts through
`mockRejectedValue`. A control that inverts silently on a benign edit is
not a control, so the resolved envelope is read too — non-empty, because
`[]` is truthy and refusing on it would fail closed on every top-up.

Tests, each verified to fail without its fix:
- allowance mock can now emit `NetworkStatus.error`; three cases pin that a
  failed refresh drops to the flat cap for the quote, the held line and the
  gate, and that it does NOT hold Continue.
- checkout: resolves (not rejects) with an ApolloError envelope, and with a
  populated `errors` array, both → serverError; an EMPTY array still signs.
Every individual door was bolted and the fall-through still failed OPEN.
Three positive tests returned `serverError`, and anything else — a throw
carrying neither a statusCode nor graphQLErrors — landed on
`unavailable`, which hands the customer the editable legacy `?amount=`
link with no pre-charge allowance check. An Apollo invariant, a cache
error, a link bug, or an @apollo/client upgrade moving the fields this
hook reads off `networkError` all take that path. A residual that fails
open into the capture-without-credit class this change exists to end is
the wrong residual however many doors are individually closed.

Inverted: `serverError` is the fall-through, and a degrade has to be
earned by positive identification — the server rejected our DOCUMENT
(schema validation, or the HTTP 400 whose body says so), or the request
never reached a server at all (a networkError with no status). Anything
unrecognised refuses.

The tests could not have caught this and still cannot in general: they
hand the catch block hand-built error objects, so they assert what we
BELIEVE Apollo throws rather than what it throws, and a shape change
keeps them green while the residual widens. That is the argument for
making the unknown shape safe by construction rather than by enumeration.

Two tests were themselves built on a shape Apollo never produces — a
bare `Error("Cannot query field")` standing in for an old backend. Both
now use the real one (a top-level GRAPHQL_VALIDATION_FAILED), and the
bare-Error case is repurposed to pin the new residual: an unrecognised
rejection refuses.

Also renamed a test whose name asserted the opposite of its body — it
said the code falls back to the legacy link on a hang, and it verifies
that the code refuses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
"$5.00 of $125.00 left today" against a $10 minimum is true and useless:
every amount it invites is below the minimum and refused. That is
offer-then-refuse — the failure this whole flow exists to stop —
relocated from the gate into the copy.

Below the minimum, both render sites now say "You've used today's
$125.00 top-up limit" instead. One helper feeds the limit note and the
Continue alert, so the screen cannot say one thing in the banner and
another in the alert.

Two existing refresh tests happened to use $5 as their post-refetch
figure, which collided with the new rule; raised to $25 so each keeps
asserting the refresh behaviour it was written for rather than the copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
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.

1 participant