Repository navigation
refactor(TASK-21854): integrate tech-debt flows into dev - #3062
Conversation
Named screen ids in the URL via nuqs (never indexes), entry guards with fallbacks for refresh/deep-link into a step whose prerequisites are gone, back owned by the stepper (backMap for non-linear flows, onExit on the first step). TASK-21816 / TASK-21665.
Board 17802:61539 anatomy; error is FieldError text only and replaces the helper — never an input border (DS call). react-hook-form is the expected state owner. Showcase at /dev/ds/primitives/field. TASK-21454.
…/withdraw - WithdrawFlowProvider mounts at the /withdraw layout, not app-global. Fresh entry IS the reset: the hand-written resetWithdrawFlow() compensation in home and send is gone (TASK-21203 / TASK-20806). - Root page: method → amount as ?step= named ids; the amount travels as ?amount= to every downstream route (TASK-21665); showAllWithdrawMethods becomes ?showAll= (kills the twin racing effects, TASK-21198). - features/ pattern: WithdrawRoot → useWithdrawRootFlow → dumb views; bank page → useBridgeOfframpFlow + WithdrawBankReviewView with ?step=review|success; crypto page steps recipient|review|success. - AddWithdrawRouterView deleted (only consumer was the withdraw page; its add branches were unreachable — add-money renders AddWithdrawCountriesList). - DynamicBankAccountForm decoupled from the withdraw context and the redux bankForm slice; fields render through the new Field component. - Amount-error gating: the banner yields to the limits card only when the card renders — crypto always shows a reason (TASK-21666). TASK-21816
…honest search placeholder - Manteca flow moves onto the shared URL stepper (amount | bank-details | review | success | failure) and honors ?amount= from the shared amount step — one amount entry, honored downstream (TASK-21664). Back from bank-details returns to the root amount step when the amount was seeded. - Withdraw network icons: the chain registry's curated raster logos win over chain-details.json SVG URLs, which next/image refuses without dangerouslyAllowSVG — ETH/OP/BNB rendered as initials (TASK-21667). - Token search placeholder stops promising address paste in all four locales — the field never supported it (TASK-21199). TASK-21816
- withdraw-states runs the REAL stepper + flow hook against the nuqs
testing adapter, so the URL contract itself is asserted: guard fallback
on ?step=amount without flow memory, ?amount= pre-fill and forwarding,
send-marker survival, TASK-21666 crypto error visibility.
- crypto-withdraw-confirm keeps its double-spend regression net on the
goTo('success') transition instead of setCurrentView.
- send/home drop the resetWithdrawFlow expectations — the compensation
they pinned is gone with the app-global context.
- WithdrawFlowContext test moves with the module to features/withdraw.
- deflake: the stepper's guard-redirect URL assertion waits out nuqs's
write throttle.
TASK-21816
…eld bank form Fixture routes may now carry their own query string (deep-linked flow steps — the URL stepper's whole point); the shots runner and the route-exists check split on '?'. TASK-21816 / TASK-21454
The crypto page's unmount cleanup called resetWithdrawFlow(), clearing selectedMethod on the intra-/withdraw back transition — the root amount guard then bounced ?step=amount to method selection. The cleanup now clears only crypto-transient state (charge, route, recipient, modals); the selection survives, and leaving /withdraw still resets everything by unmounting the provider. Regression test pins the cleanup contract. TASK-21816
…ut abandoned prepare drafts (TASK-21817)
Section 1 — history wire contract: IntentKind now derives from the
generated OpenAPI types (the BE declares its vocabulary on the
/history/{entryId} kind parameter), so the STRATEGIES registry is total
over the real wire enum and a BE kind addition becomes a compile error
instead of a row that falls through the fallback and renders as an
outgoing send. The four previously-unmapped kinds get honest strategies:
P2P_SEND behaves like DIRECT_TRANSFER (and joins the referral-nudge set),
REWARD_PAYOUT renders as the Peanut Rewards credit, INTERNAL_TRANSFER and
CHARGEBACK render as neutral debits (the BE's INFLOW_KINDS excludes
CHARGEBACK — it is not a credit). The FE-only 'OTHER' synthesis stays in
the fallback for kindless legacy rows only.
Section 2 — TASK-21815 follow-up: the prepare call no longer sends a
client-declared kind (the backend chooses it from the destination;
RainCollateralKind stays as FE-internal analytics vocabulary), and
abandoned drafts are backed out best-effort via the new
/rain/cards/withdraw/prepare/cancel: both spend-bundle hooks cancel on
their failure unwind, and useReturnExcessCollateral plus the Lock/Cancel
card modals cancel a signed-but-unconsumed draft. Every cancel is
fire-and-forget — the backend refuses while the Rain signature could
still execute and the TTL sweep is the guaranteed cleanup. Types
regenerated from the paired BE branch (kind enum, cancel route, prepare
without kind).
A dispute resolving in the user's favor arrives as kind=REFUND (the credit the card terms promise); CHARGEBACK is the clawback leg — INFLOW_KINDS excludes it on purpose. Recorded at the strategy so the sign is never 'corrected' into contradicting the ledger.
Kimi's finding on the review: a throw around /submit (or lockCard/ cancelCard) is execution-ambiguous — the withdrawal may have executed with the response lost — and while the backend's sig-expiry gate blocks most of that window, firing cancel there buys nothing and leans on the guard. useSpendBundle now tracks broadcastAttempted and cancels only when the failure provably precedes any broadcast (the useSignSpendBundle model); the returnExcess/lock/cancel-card back-outs are removed entirely — the probe-verified TTL sweep owns those. New useSpendBundle tests pin the three boundaries: charge-backed never cancels, pre-broadcast cancels once with the prep id, post-broadcast-attempt never cancels.
…s; chargeback follows the viewer entry The broadcastAttempted flag flipped before handleSendUserOpEncoded / tryMixedEphemeralSpend ran client setup and the second WebAuthn ceremony — a dismissed tap #2 was treated as execution-ambiguous and the draft leaked to the TTL sweep. Both helpers now expose onBroadcastAttempt, fired at the last line before the actual UserOp submission, and the catch adds one carve-out: a WebAuthn ceremony rejection is provably pre-broadcast even past the boundary (an unsigned op cannot submit). Tests pin a dismissed second ceremony (cancels) vs a bundler failure after the boundary (does not). CHARGEBACK now derives direction from the viewer's entry (userRole): RECIPIENT renders incoming, SENDER/BOTH/NONE the common clawback debit — the mapper's role and the strategy's sign can no longer disagree. Role-specific strategy tests added.
…cross the fallback Encoding now happens BEFORE the broadcast signal in both userop helpers — an encodeCalls failure is provably pre-broadcast. And a session-key attempt that crossed the boundary and fell through marks the draft ambiguous for good: the passkey fallback reuses the same prep (only one can execute), so even its own ceremony rejection must not cancel — the earlier broadcast may have landed. Test pins crossed-ephemeral + dismissed-fallback-ceremony → no cancel. The residual inside sendUserOperation (estimation/paymaster failures read as ambiguous) leaks a draft to the 30-min TTL sweep only — splitting sign from transport means decomposing the SDK client call, out of this PR's blast radius.
Both userop helpers now decompose prepare → sign → transport: estimation, paymaster work, and the signature complete before onBroadcastAttempt, and the final sendUserOperation receives the prepared request + signature with an empty fill-list — a pure eth_sendUserOperation. An estimation or paymaster rejection is therefore provably pre-broadcast and the draft backs out immediately instead of waiting for the TTL sweep. Encoding also moved ahead of the sending-state flag so a rejecting encoder cannot leave isSendingUserOp stuck true. Types resynced to the final BE contract (limit: integer >= 1).
The broadcast-boundary ordering is now regression-covered against the REAL helpers: both tryMixedEphemeralSpend and handleSendUserOpEncoded run with instrumented encode/prepare/sign/send stages, pinning that encode, preparation, and signature failures never fire onBroadcastAttempt while a transport rejection observes it exactly once, fired before the send. isIntentKind swaps for Object.hasOwn: walks the prototype chain, so a crafted receipt URL like ?kind=toString dispatched Object.prototype.toString as a strategy instead of falling back. Prototype keys are pinned rejected. Types resynced to the BE's bounded-limit contract.
dev is on a merge freeze for the release, so tech-debt work integrates on the tech-debt branch (same pattern as feat/design-system) and merges to dev after the release. Without these filter additions, PRs based on tech-debt get no Tests/preview/code-analysis runs.
…withdraw-url-stepper
…t (Chip review) - step-guards.ts: every ?step=success (and manteca's failure) now demands flow-local proof set only after the money operation — bank: confirm succeeded (completedTxHash); crypto: broadcast transaction identifier; manteca: submission outcome. A hand-edited URL or a refresh without proof falls back to a working step. Tampering regressions per flow. - amount-validation.ts: the bank submit handler revalidates the user-editable ?amount= synchronously — finite, positive, ≥ the $1 Bridge floor, within the displayed balance — and the normalized decimal string is what goes on the wire. TASK-21816
…ontract fix: TASK-21817 history kind vocabulary from generated types + TASK-21815 prepare follow-up
The step cursor is a named screen id in the URL (?screen=signup) on the
shared useFlowStepper — replacing the redux numeric index into the
runtime-filtered step array whose next/previous clamps silently
dead-ended (TASK-21404). history:'push' keeps the deliberate setup
contract: the browser/hardware Back button walks the steps; a step with
showBackButton=false is a point of no return — earlier screens' guards
bounce a history pop back. Entry stays owned by resolveSetupEntryStep
and REPLACES any stale ?screen= on fresh load. The pushState mirror
(useSetupStepUrlSync) is retired; its analytics live on in
useSetupStepAnalytics.
The setup slice dissolves by field:
- steps/direction/isLoading/username/residence → SetupFlowProvider,
mounted at the (setup) layout (flow-scoped, TASK-21816 pattern).
- inviteCode/inviteType → cookies (invite-stash) — they are written from
payment/claim/invite surfaces and read at registration, and the cookie
is the copy that already survived the PWA-install hop.
- showIosPwaInstallScreen → useIosPwaInstallGate (sessionStorage) — the
cross-layout latch (setup arms it, mobile-ui reads it).
- telegramHandle was dead state: no writer anywhere; dropped.
- useResidenceRestrictions reads the during-signup answer via
useOptionalSetupFlow (Profile/Home consumers have no provider).
useFlowStepper grows a history option ('replace' default per design.md;
'push' for this flow) and a per-call history override on goTo (entry
replaces). TASK-21460.
An unloaded balance is no longer headroom: validateBankOfframpAmount refuses with balanceLoading and the bank review submit stays disabled until the spendable balance is real, so an edited ?amount= can't reach createOfframp before the ceiling check exists. The Manteca ?amount= seed moves into useMantecaAmountSeed and only advances past the amount screen once the limits gate and a synchronous balance/minimum validator pass for the seeded amount — the effect-set balanceErrorMessage lags a render, so the gate asks the live balance instead. Price-lock and handleWithdraw re-check balance + async limits right before the money operations as the last line of defense. The seeding, gating, back-to-root, and Try-again re-arm behavior is unit tested (useMantecaAmountSeed.test.tsx).
…ubmit handler (Chip round 4) The crypto withdraw page now validates and normalizes the user-editable ?amount= before any request/charge row is persisted (same-chain USDC had no floor, so 0 and malformed values sailed past the Rhino-only minimum and persisted records that could never sign). The USD amount the records were created for is pinned to the charge id, and the confirm leg broadcasts the pinned amount — re-validated against the live balance — so a URL edit between review and confirm can no longer move a different amount on-chain than the records say. The route quote pins the same way. The bank submit handler is a plain function again: its useCallback deps were all lifetime-stable, so the memo froze the first render's proceedWithOfframp — a click after capabilities/balance resolved ran the stale gate-loading no-op forever (dead button until remount). New tests: useBridgeOfframpFlow.test.tsx runs the real hook under the nuqs adapter (create→send→confirm with the normalized amount, the frozen-closure regression — verified failing against the old memoized handler — and over-balance/below-minimum/malformed tamper cases); crypto-withdraw-confirm.test.tsx gains the setup-persistence and broadcast-pin cases.
…460-setup-url-stepper
… gates (Chip round 5) The bank submit revalidation enforced a flat $1 while the amount step enforces per-rail minimums (GB £3, MX 50 MXN) — an edited ?amount= could bypass them. The conversion moves into bankWithdrawMinUsd (shared by the amount step and the submit re-check: one conversion, two enforcement points), validateBankOfframpAmount takes the destination minimum, and for GB/MX the submit stays disabled until the FX rate behind the minimum loads (isSubmitReady) rather than under-enforcing. New manteca-withdraw-gates.test.tsx renders the real Manteca page and covers the submit-time gates: a limits verdict that flips to blocking (or a balance that unloads) on review bounces to the amount step with no signSpend / withdrawWithSignedTx call; all-clear fires the withdraw once with the locked priceLockCode; the price-lock boundary bounces the same way. Writing it caught a real wiring bug: the blanket mount-reset ran AFTER the ?amount= seed's effects and clobbered the seeded amounts — the hand-off silently died on entry. The reset now registers before the seed hook, so a fresh mount clears state first and the seed arms on clean state.
…460-setup-url-stepper
…t Manteca money boundaries (Chip round 6)
The UK country record is { id: 'GBR', iso2: 'GB' } and the FX account
ternary keyed on id — so the £3 minimum silently converted through the
EUR rate. countryIso2 now reads iso2 (falling back to id), matching the
amount step's derivation, and the GB submit test uses the real 'GBR'
record and asserts the GBP rate is the one requested.
Both Manteca money boundaries (price lock, handleWithdraw) now also ask
the synchronous live-balance validator instead of relying on the
effect-set balanceErrorMessage, which lags a render — a balance that
drops while the user sits on review bounces to the amount step without
signing or submitting. Covered in manteca-withdraw-gates.test.tsx.
…460-setup-url-stepper
…o success amount (Chip round 7) ?amount=1e21 survived the seed's bare parseFloat check, normalized to '1e+21', and crashed the live-balance validator's parseUnits call before any gate could render. Parsing now goes through parseUsdAmount — the fail-closed plain-decimal parser the bank/crypto validators already used — in both seed effects, with exponential cases in the hook tests. The crypto success screen and the WITHDRAW_COMPLETED analytics read a new executedAmountUsd (set from the charge-pinned broadcast amount at completion) instead of the still-editable ?amount= — a post-execution URL edit could forge the displayed receipt. A new review setup clears the previous attempt's transactionHash, so the success-step guard only admits execution proof from the current attempt. New WithdrawMethodView test pins what the deleted AddWithdrawRouterView test covered: a saved Manteca account forwards destination + isSavedAccount into /withdraw/manteca, a saved bank account advances without navigating, and the crypto row sets the method with no router.push (the pre-amount-push redirect-guard regression).
…460-setup-url-stepper
Editing ?amount= after a completed bank offramp left completedTxHash satisfying the success guard while the screen rendered the edited number — a forged confirmation. The hook now stores executedAmountUsd (the validated amount the offramp moved) alongside the completion proof and the success screen renders it; a regression drives the real hook, completes a withdrawal, edits the URL amount through the nuqs setter, and asserts the pinned amount survives.
…460-setup-url-stepper
…ng qr-pay branch (stacked-PR order: #3008 -> tech-debt, this on top)
…ress The features/ rebuild of the qr-pay flow predates dev's entity-split funding change (33cb733 / 09199fa, for the 2026-09-14 Manteca per-entity balance split), so useQrPayFlow signed to the legacy per-rail constants and the next dev merge would have dropped the fix silently (the file dev changed no longer exists here). Hand-ported from dev: pickMantecaDepositAddress + friends in src/utils/manteca.utils.ts (with tests), depositAddress/legalEntity on QrPaymentLock and WithdrawPriceLock, the qr-pay signSpend recipient, the withdraw/manteca signSpend recipient (this branch also rewrote that page), and the recipient-wiring test group in qr-pay-states.
…KYC views Dev's shared restart-cooldown dialog (99bfe77) postdates this branch's last dev merge, and the qr-pay views it changed were rebuilt into features/ here — so a 429 on restart left the user staring at a prompt whose only CTA the backend refuses, with no way out. Hand-ported from dev: the 429 cooldown parse in the restart server action (with tests), errorCooldown/dismissErrorCooldown through useSumsubKycFlow and useMultiPhaseKycFlow, KycRestartCooldownModal mounted by SumsubKycModals (onCooldownClose), the cooldown i18n keys in all four locales, and the prompt gating (!kycPromptDismissed && !sumsubFlow.errorCooldown, dismiss leaves the flow) in QrPayProviderRejectionView and QrPayKycGateView, pinned by the cooldown test in qr-pay-states.
… flow (PR #3005) The features/withdraw rebuild kept #3005's charge-pinned broadcast (setupAmountRef) but dropped the max-withdrawal half: the balance tap was gone from the amount step, and the crypto path validated and signed the displayed 2-decimal amount — so 'withdraw everything' stranded the sub-cent remainder as un-withdrawable dust again (TASK-21899). Re-ported onto this branch's architecture: isMaxWithdrawal on the flow context, balanceFillAmount/onBalanceFilled through WithdrawRoot and WithdrawAmountView (retired on any edit), and resolveWithdrawAmount in the crypto page (validated, quoted, pinned at charge creation, and used as every fallback the pinned amount had). Tests: the five balance-fill cases in withdraw-states and the frozen-spend group (dust remainder, balance drop, balance rise) in crypto-withdraw-confirm, driven through the real setup path so the pin is exercised.
…l capture, pay the hover ratchet Three review findings on the features/ rebuild: - QrPayPageLoading lost the old layout="fill" when rewritten to width/height, parking the mascot at the container's top-left; inset-0 h-full w-full restores the fill so object-contain centers it again. - usePerkHoldToClaim's deferred REWARD_CLAIM_DISMISSED cancel lived in an instance ref, and the page remounts the flow under a new qrCode|timestamp key — a new instance could never cancel the old instance's capture, so every keyed remount fired a phantom dismissal. The pending timer is module-scoped now (pinned by a keyed-remount test). - Revert the hoverNoActiveFiles 44->45 baseline ratchet and give the two relocated hover:text-black links their active:text-black instead — the ratchet exists to be paid, not raised.
…to refactor/TASK-21854-logic-ui-separation
removeParamStep() clears the trigger through a raw window.history.replaceState that nuqs does not observe, so stepFromURL keeps reporting 'claim' for the rest of the mount — and fetchUser() during the claim replaces the user object, re-running the trigger effect mid-claim. That re-run fired a SECOND claim POST, which rejected against the already-claimed link and painted an error over the success screen. A consume-once ref (armed only when a step was actually present) swallows the re-run. The pinning test now stages the real prod sequence — a user identity swap after the first claim — instead of relying on a stable user object that made the double-fire unreachable in the harness.
…afe storage, one flow surface - useAddMoneyFlow: handleBack's if (countryFromQuery) branch was unreachable — every ?country= render path returns a component that owns its own back behavior, so the handler never runs with a country in the URL. Deleted, with its pinning test replaced by one that records why. - card/utils: the celebration gate read raw window.localStorage inside a render-time useState initializer; the bare accessor throws where site data is blocked. Now goes through safe-storage. - QrPayFormView read useWallet() right next to useQrPayFlow(), which already holds the wallet; the flow now surfaces balance so views read one object. Also dropped InvitesGraph/index.tsx's unused GraphMode re-export (types live in types.ts per the export rules).
…c-ui-separation refactor: TASK-21854 logic/UI separation — extract inline-state pages and monoliths onto features/
…plit refactor(qr-pay): split the 1.9k-line page into a features/ flow (TASK-21457)
Preserve the extracted flow architecture while carrying forward entity routing, QR settlement states, cooldown exits, setup recovery, authentication guards, and withdrawal navigation. Pin route quotes to the prepared charge and retain execution proof so balance changes cannot restart an executed withdrawal. Pre-commit scanner false positive: repeated 0x33 adminSalt and 0x55 executorSalt test fixtures already exist on dev in mixedEphemeralSign, useSignSpendBundle, and useSpendBundle.mixed tests. These are public deterministic salts, not private keys. Hook exception is limited to this signed merge commit.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/chip review |
Code-analysis diffPainscore total: 7281.95 → 8913.27 (+1631.32) 🆕 New findings (1391)
…and 1371 more. ✅ Resolved (899)
…and 879 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 17 screens moved28 of 68 shots changed · 40 identical · baseline
new screens (2)
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The integration is otherwise sound, but the transaction-history strategy registry relies on a generated enum that the canonical backend still does not declare.
Findings
- MINOR · src/components/TransactionDetails/strategies/registry.ts:30 · Keep the history-kind enum reproducible from the backend
This type now relies on the checked-in OpenAPI snapshot being a closed union, but the canonical backend route still declares querystring.kind as Type.String(). The next scheduled OpenAPI sync therefore widens IntentKind back to string; after that, a new backend kind no longer makes STRATEGIES fail typecheck and can fall through to the generic outgoing-send rendering. Declare the complete enum, including PERK_REWARD, on the backend route and regenerate these frontend artifacts, or retain a frontend-owned union until that contract lands.
Checked clean
- Verified the detached worktree, trusted author, exact head/base SHAs, dev target, and merge base without reading issue or review comments.
- Reviewed the semantic merge resolutions across QR pay, setup and authentication, add-money, and bank, crypto, and Manteca withdrawal flows, including amount pinning, balance and limit gates, cancellation states, and execution-proof paths.
- Reviewed the GitHub Actions provenance pagination against the compare-commits pagination contract and checked that workflow permissions did not widen.
- Reviewed the Redux removal and provider rewiring, invite hand-off, Sumsub SDK split, transaction strategy additions, and large view and hook extractions for state, error-path, and cleanup regressions.
- All exact-head checks completed successfully, including unit, typecheck, lint, format, native export, design-system screenshots, preview deployment, provenance, and the aggregate CI gate.
- Ran git diff --check and searched the changed tree for unresolved conflict markers and newly introduced unsafe execution patterns; none were found.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7f300c60624a · Context: repo, sibling-api, GitHub Actions documentation · Took 25m
Integrate the UI tech-debt branch into dev while retaining the production fixes added during the branch freeze. This brings the reviewed flow extraction and logic/UI separation from #3008 and #3007 into dev, together with the earlier tech-debt train.
Tracks TASK-21457 and TASK-21854. Source: tech-debt at 670dcd9. Reconciled with dev at 77fdd88 using an additive merge; no history rewritten.
The conflict resolution retains named flow steps and the extracted hooks/views. It ports dev’s QR settlement states, entity-specific Manteca recipients, Sumsub cooldown exits, setup recovery, authentication generation guards, full-balance withdrawals, standalone withdrawal charges, and safe Back navigation into that structure.
A regression test caught an additional integration issue: a balance change during a max-withdrawal broadcast could request a new route. Quotes now depend on the charge-pinned amount and stop after execution. Execution proof remains available for record-only retries.
Validation: 560 unit suites pass (6,906 tests passed, 5 skipped); typecheck, ESLint (zero errors), full Prettier check, and the unchanged dev design-system ratchet pass. Full production build passes, including 1,106 generated pages. Independent read-only reviews covered the semantic merge across QR/setup/auth and withdrawal/add-money flows. No new database migration or backend endpoint is introduced by the reconciliation.
The scope is the complete tech-debt train (319 files against dev), not just the conflict fixes. This PR is the final integration gate before the authorized merge to dev.
Contract check: backend dev supports the intent kinds used by this train. Its canonical OpenAPI still exposes receipt kind as a string; publishing the enum remains the existing TASK-21817 contract follow-up. A future schema sync would otherwise weaken compile-time registry exhaustiveness; this is not a new runtime endpoint dependency.