fix: retry pending claim-link withdrawals without reclaiming (TASK-22686) - #3216
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks 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 |
Code-analysis diffPainscore total: 8051.06 → 8047.15 (-3.91) 🆕 New findings (257)
…and 237 more. ✅ Resolved (257)
…and 237 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
/chip review |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings in the generated OpenAPI snapshot and type updates for pending withdrawal confirmation responses.
Checked clean
- Confirmed the detached worktree HEAD and merge base match the supplied head and base SHAs.
- Compared the complete two-file diff and verified the OpenAPI 409 response additions agree with the generated TypeScript response shapes.
- Checked the Rain request layer and callers: 409 preparation errors remain generic ApiErrors with their code preserved, while submit only special-cases STALE_CARD_APPROVAL and preserves WITHDRAWAL_PENDING_CONFIRMATION as a generic coded error.
- Checked exact-head CI: ci-success, unit, typecheck, format, eslint, native-export, screen-tests, and contract baseline checks passed; visual capture jobs were still running and the change has no rendered UI surface.
- Parsed api.openapi.json successfully. Local regeneration could not be repeated because the detached worktree has no project node_modules; exact-head CI passed the generated artifacts through typecheck and formatting.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
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: e7d6a63ca808 · Context: repo, sibling-repo · Took 7m
🖼 Visual diff — 6 screens moved11 of 96 shots changed · 85 identical · baseline
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. |
jjramirezn
changed the title
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable correctness, security, adversarial, or maintainability findings at the pinned head.
Checked clean
- Pinned HEAD and merge base matched the supplied SHAs; PR author, base, and head metadata matched.
- The same-link retry retains the claimed transaction hash before any association or withdrawal await and blocks same-tick duplicate claims.
- Pre-claim failures, pending/store/transport retries, and different-link rerenders have focused regression coverage; exact-head unit, typecheck, lint, format, build, and native-export checks passed.
- The companion API contract keeps uncertain confirmation non-2xx and preserves the funding attempt across same-hash retries; no frontend contract mismatch was found.
- A focused local Jest run could not start because the detached worktree has no node_modules; the exact-head CI unit check passed.
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: 5e884c667920 · Context: repo, api · Took 14m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable correctness, security, adversarial, or maintainability findings at the pinned head.
Findings
- MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:119 · [claude-opus] product/send-links.md still says direct link claim to Pix/Mercado Pago is unsupported
This PR hardens the claim-link → Manteca offramp path: MantecaReviewStep spends the one-shot claim link into the Manteca deposit address and then calls POST /manteca/withdraw with the user's Pix key or Mercado Pago alias as destinationAddress (the new test fixture is literally destinationAddress="somepixkey@bank.br", currency BRL). The flow is live — Initial.view.tsx:111 renders MantecaFlowManager whenever claimToMercadoPago && !!user, gated only on canDo('pay', { provider: 'manteca' }).
/home/chip/mono/product/send-links.md says the opposite, in both the front-matter and the prose: excluded_claim_methods: [pix, mercadopago] # must claim to Peanut account first, then move to Pix/MP (line 11), and the "Claim to Pix or Mercado Pago" section (lines 81-88): "Direct claim to Pix or Mercado Pago is not supported. Recipients who want funds in Pix or Mercado Pago should: 1. Claim to their Peanut account (or create one) 2. Use the funds from their Peanut balance...". Note this is a distinct claim from product/feedback/problems/pix-direct-payment-broken.md, which is about paying a third party by Pix key, not about claiming a link to your own key.
The doc is the side that is wrong: the code ships the feature, behind a Manteca/LATAM KYC gate and a logged-in user. Support answers derived from send-links.md will tell AR/BR recipients a supported flow does not exist. To be clear, this drift is pre-existing — this diff does not introduce it — and I am raising it only because this PR is actively investing in that exact flow. Fix: update send-links.md to describe claim-to-Pix/MP as supported for logged-in, Manteca-verified recipients in AR/BR, and drop pix/mercadopago from excluded_claim_methods (via the update-content path, not by hand-editing generated pages).
Checked clean
- Pinned HEAD and merge base matched the supplied SHAs; trusted author, base, and head metadata matched.
- The same-link retry retains the claimed transaction hash before association and withdrawal awaits, while the synchronous in-flight guard prevents duplicate claims.
- Pre-claim failures, pending/store/transport retries, different-link rerenders, and same-tick double clicks have focused regression coverage.
- Exact-head ci-success, unit, typecheck, lint, format, build, native-export, screen, and review checks passed.
- The failing screen-library publish check was caused by Cloudflare token verification in the base publication workflow; the PR changes no publication or storage code.
- The companion API contract keeps uncertain confirmation non-2xx and preserves funding ownership across same-hash retries; no frontend contract mismatch was 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: 1 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: 5e884c667920 · Context: repo, api · Took 8m
|
/chip review |
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The same-hash retry implementation is coherent and exact-head CI is green, but one previously raised minor product-documentation contradiction remains.
Findings
- MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:119 · Canonical Send-link docs still contradict direct Pix/Mercado Pago claims — also flagged by claude-opus
product/send-links.mdstill lists Pix and Mercado Pago as excluded claim methods and says recipients must claim to a Peanut account first, while this path directly claims the Send link into Manteca and withdraws to the selected Pix/Mercado Pago destination. That leaves product and support guidance contradicting the shipped flow. If this direct path is intended, update the canonical Send-link claim methods, flow, and verification/session requirements; otherwise gate the direct options to match product truth.
Checked clean
- Exact head/base, component diff, parent step lifecycle, and same-link, different-link, and concurrent-click retry paths.
- Backend legacy-withdraw funding reservation behavior for pending liquidity, claim-store failure, duplicate transactions, and same-hash retry.
- Generated OpenAPI and TypeScript deltas for pending-confirmation responses and the exact EVM transaction-hash shape.
- Live Lexicon terminology and canonical Send-link product behavior; prior P1 remains true.
- Exact-head unit, typecheck, ESLint, and format checks passed; ds-shots was still in progress at review time.
- Focused Jest could not run locally because the detached worktree has no installed dependencies; the checkout remained clean.
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: 1 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: 2aa90a2f91d1 · Context: repo, product, backend, ci · Took 10m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One active minor retry defect remains when back-navigation remounts the review step. The duplicate product-documentation mismatch was accepted for a separate follow-up and is deferred.
Findings
-
MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:47 · Preserve the claimed hash across review-step remounts
After a claim succeeds and/withdrawreturns a retryable error, the existing back control changes the parent toDETAILS, unmounting this component and destroyingclaimedRef. Returning toREVIEWcreates an empty ref, so the next Withdraw reaches the one-shot claim call again instead of reusing the accepted txHash; the tests only rerender the same mounted instance. Keep the hash keyed by Send link in state that survives step changes (or prevent leaving after the claim), and add a DETAILS→REVIEW remount regression test. -
MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:47 · [claude-opus] Recovered txHash is lost when the review step unmounts on back-navigation, untested
claimedRef(src/components/Claim/Link/views/MantecaReviewStep.tsx:47) is component-local. MantecaFlowManager.tsx:81-111 renders MantecaReviewStep only whilecurrentStep === REVIEW, andonPrev(MantecaFlowManager.tsx:119-122) sets the step back to DETAILS, which unmounts the component and destroys the ref. Failure scenario: user clicks Withdraw, the link is claimed (funds now sit at the Manteca deposit address), /withdraw answers FUNDING_PENDING_CONFIRMATION, the user taps back to re-check the Pix key, returns to REVIEW and retries — the remounted step has no stored hash, so it runs init +claimLinkSecureagain on an already-spent one-shot link, which fails, and /withdraw is never re-called with the hash that would release the held claim. The in-code comment only acknowledges the reload case; back-navigation is an ordinary in-session gesture on the same screen. The new test file covers rerender-with-a-different-link but never an unmount/remount, so this money-path case is untested (CONTRIBUTING.md requires a test for money-moving paths). Fix: hoist the{claimLink, txHash}ref into MantecaFlowManager (or a module-level map keyed by claimLink) and add a test that unmounts and remounts the step between the failed attempt and the retry, asserting oneclaimLinkSecurecall and an identical txHash on the secondwithdraw.
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:119 · Canonical Send-link docs still contradict direct Pix/Mercado Pago claims — no task filed, please file one
Checked clean
- Exact head, base, changed component, parent step lifecycle, and transaction-hash retry paths.
- Same-mounted-view pending, claim-store, transport, different-link, and concurrent-click behavior remains covered and coherent.
- Backend legacy-withdraw funding reservation behavior supports same-hash retry for the intended recovery paths.
- Canonical Send-link product behavior still conflicts with the direct Pix/Mercado Pago path; the supplied author reply explicitly moves that content fix to separate work.
- Required exact-head CI aggregate, unit, typecheck, ESLint, format, screen, and design-system checks passed. The separate screen-library publish job failed Cloudflare credential verification and is unrelated to this diff.
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: 2 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: 2aa90a2f91d1 · Context: repo, product, backend, ci · Took 9m
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The same-hash retry survives review-step remounts and is covered against duplicate in-flight work. The accepted canonical documentation mismatch is deferred to its named follow-up.
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:128 · Canonical Send-link docs still contradict direct Pix/Mercado Pago claims — no task filed, please file one
Checked clean
- Pinned head, trusted author, dev base SHA, and merge base match the supplied review boundary.
- Claimed hash and synchronous in-flight state are owned by MantecaFlowManager, survive DETAILS/REVIEW remounts, and stay keyed to the claim link.
- Flow-level regressions cover back-navigation after a pending withdrawal and during in-flight claim and withdrawal requests; exact-head unit, typecheck, lint, format, build aggregate, and native-export checks are green.
- The backend sibling keeps one-deposit-one-payout enforcement and releases a funding claim only after a proven no-order failure, so same-hash retry does not authorize a duplicate payout.
- Generated OpenAPI changes are limited to the withdrawal txHash format and pending-conflict response variants relevant to the paired backend contract.
- Live Lexicon terminology and the canonical Send-link product document were checked for the deferred documentation mismatch.
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: 1 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: 83a57f82b9ef · Context: repo, product, api-sibling, ci, lexicon · Took 11m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The same-hash retry survives review-step remounts and is covered against duplicate in-flight work. The accepted canonical documentation mismatch is deferred to its named follow-up.
Answered by a human, not raised again
These are still present as far as this review can tell. You resolved the thread or deferred the fix, so Chip files them instead of repeating them.
- MINOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:128 · Canonical Send-link docs still contradict direct Pix/Mercado Pago claims — no task filed, please file one
Checked clean
- Pinned head, trusted author, dev base SHA, and merge base match the supplied review boundary.
- Claimed hash and synchronous in-flight state are owned by MantecaFlowManager, survive DETAILS/REVIEW remounts, and stay keyed to the claim link.
- Flow-level regressions cover back-navigation after a pending withdrawal and during in-flight claim and withdrawal requests.
- Exact-head code gates and visual captures pass; the separate screen-library publish job fails at external credential verification, outside this diff.
- The backend sibling keeps one-deposit-one-payout enforcement and releases a funding claim only after a proven no-order failure, so same-hash retry does not authorize a duplicate payout.
- Generated OpenAPI changes are limited to the withdrawal txHash format and pending-conflict response variants relevant to the paired backend contract.
- Live Lexicon terminology and the canonical Send-link product document were checked for the deferred documentation mismatch.
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: 1 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: 83a57f82b9ef · Context: repo, product, api-sibling, ci, lexicon · Took 8m
1 participant
A claim-link withdrawal could submit its transfer and then lose the transaction hash when confirmation failed. Pressing Withdraw again tried to claim the one-shot link a second time. Preserve the claimed hash in the flow manager and retry the withdrawal with that same hash, including after Back to Details and return to Review. The shared in-flight guard prevents duplicate claims and withdrawals across step remounts.
The generated API contract also includes pending-confirmation errors for Rain withdrawal and recovery preparation. Pending confirmation continues to use a non-2xx response.
Task: TASK-22686. Companion backend: peanut-api-ts #1615.
Risk: the legacy claim-link payment flow changes. Keep the initial entity-aware destination check and one-shot claim unchanged; retries reuse the accepted transfer. Land this UI fix before or alongside the backend recovery change. Hash retention is limited to the mounted flow; reloading or leaving the flow can still require ops reconciliation.
Validation: full frontend suite: 629 suites and 7,777 tests passed (seven skipped); TypeScript, full Prettier, changed-source ESLint, and production build passed; focused component and flow suites: 16 tests passed. Regression coverage includes pending confirmation, claim-store failures, thrown transport failures, initial claim/init failures, a different claim link, Back/Review remounts, and remounts during both claim and withdrawal requests. Generated types preserve unrelated API snapshot differences.
Screenshots: N/A. No layout, controls, or copy changes; the change is which request an existing retry sends. No legal impact. Shared content follow-up is documented in the backend PR.
The API snapshot also carries the legacy withdrawal hash rule (
0xplus 64 hexadecimal digits). Valid SDK hashes and generated TypeScript types are unchanged. Synced with current dev; full suite, typecheck, formatting, build and API generation check pass.Separate content follow-up: reconcile
product/send-links.mdwith the existing direct Pix/Mercado Pago claim flow. This PR changes retries, not which claim destinations are offered. Keep content changes separate from this code PR.Documentation follow-up is tracked in TASK-22697, assigned to Jota in the current sprint. It covers the collateral hold, uncertain-payment wording and direct Pix/Mercado Pago claim documentation.