Repository navigation
Fix swallowed deposit-status refusals and closing manual refunds in the B2B refund path - #1415
Merged
Merged
Conversation
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vortexfi canceled.
|
✅ Deploy Preview for vrtx-dashboard canceled.
|
The processed-order branch marked the recovery redeemed before setting the deposit refunded and ignored the refusal, so a refused or failed transition left the deposit recovering forever with no DEPOSIT_RETURNED. fail() ignored the refusal too, and the error it had already written turned a refused park into an operator retry that reset the five-attempt cap. Change the deposit first and only record the outcome once it succeeded.
A refund that stays manual (EUR 15,000 and above, or no payer) parks as recovery_failed, but the lattice only allowed recovery_failed -> recovering, so the documented PATCH refunded was refused. Moving it to recovering made the next cycle reset its attempts and park it again, so the deposit never reached refunded and the client's later refunds stayed blocked. Allow the edge; the orchestrator already closes a recovery whose deposit is refunded without placing an order. The runbook now has the operator place the order under the vortex-refund memo and close straight from recovery_failed.
The line points the operator at the manual procedure, but it fired before the park and again every cycle while the park was refused, when the keeper was still driving the refund. The refusal line now carries the reason, and both new error lines get a triage row.
A retry after a rejected redeem order re-reads the same order, or adopts it again by memo, and parks the deposit again, yet the runbook called it a fixable retry. Only five failed attempts are retried now; the rejection joins the parks the operator refunds by hand. A rejected order's id is dropped from the recovery so DEPOSIT_RETURNED for that hand close never names an order that paid nothing.
Invariant 7 still said only the keeper and the admin endpoints move a deposit along the refund branch, the checklist claimed the status PATCH refuses pending executions (only the recover POST does) and missed the hand close, and the sandbox row closed a no-payer park without saying the refund wallet must be empty first.
Another file leaves logger methods as bun mocks, so spyOn returns the existing mock with its earlier calls and the log assertions count lines from previous tests.
Staging's newer refund tests stub setDepositStatus without the refusal result this branch introduced, and one of this branch's tests checked the in-flight recover globally; activeRecoveryExists is per client since the per-client refunds landed.
ebma
force-pushed
the
fix/monerium-refund-status-closure
branch
from
October 8, 2026 16:49
70e575c to
4430df4
Compare
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
1 participant
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.
What this fixes
Two bugs in the Monerium B2B refund path that were already on staging. The vortex-review of #1411 found them.
1. Refused deposit-status changes were swallowed (
recovery.ts)redeemedbeforesetDepositStatus(refunded). The refusal string was ignored, and the "refunded" info log was written either way. If that call threw or was refused, the recovery dropped out of every open query. The deposit then stayedrecoveringforever andDEPOSIT_RETURNEDnever fired.fail(): it wroteerrorbefore parking the deposit and ignored the refusal. A refused park lefterrorset on a deposit that was stillrecovering. The orchestrator reads that as an operator retry, so it reset the attempts and the five-attempt cap no longer applied.Fix: change the deposit first. The recovery is marked
redeemed, or gets itserroron a park, only after the transition succeeded. On a refusal, an error is logged and the recovery stays open, so the next cycle retries it. If the transition throws, nothing is written and the orchestrator's catch logs it.RecoveryDeps.setDepositStatusis now typedPromise<string | null>, which is what the live function already returned.2. A manual refund could not be closed with the documented PATCH
Some refunds stay manual: EUR 15,000 and above, or a deposit with no payer. These park as
recovery_failedattopped_up. The lattice only allowedrecovery_failed -> recovering, so the runbook'sPATCH .../status {"status": "refunded"}got a 409. Moving the deposit torecoveringinstead made the next cycle treat it as an operator retry and park it again. The deposit never reachedrefunded, and that client's later refunds stayed blocked. If the order placed by hand did not carry thevortex-refund:memo, a retried redeem could also place a second order.Fix: add
recovery_failed -> refundedto the lattice. Only two callers ever requestrefunded: the admin PATCH and the processed-order branch, and both are correct fromrecovery_failed. The orchestrator already closes an open recovery whose deposit isrefundedwithout placing an order, so nothing else changes. The admin route keeps its status codes and messages for every existing transition. The one change is thatrecovery_failed -> refundednow returns 200 instead of 409.The runbook now gives the procedure that works. For a refund that stays manual: don't set
recovering. Place the order by hand under the memovortex-refund:<depositId>, then close the deposit directly fromrecovery_failedwithrefunded.Tests
Every new test fails on the old code and passes with the fix.
recovery.test.ts,driveRecovery: a processed order with asetDepositStatusthat returns a refusal, then one that throws. The recovery staysredeemingand no "refunded" log appears. A later successful transition closes it.recovery.test.ts, orchestrator on a real database: the fifth failed attempt hits a refused park, then a throwing one.errorstays empty and the attempt count goes 5 -> 6 -> 7, with no reset to a fresh run. The deposit staysrecovering.recovery.test.ts, orchestrator: an EUR 20,000 refund parks asrecovery_failed("supporting document"). The operator closes it with the realsetDepositStatus(refunded), and the next cycle marks the recoveryredeemed. The deposit endsrefunded, no redeem order is placed, andactiveRecoveryExists()is false.moneriumB2b.controller.test.ts:PATCH refundedon arecovery_faileddeposit returns 200 and the deposit becomesrefunded.deposit-processor.test.ts: lattice assertion for the new edge.Gates: the B2B, workers and controllers tests pass on an isolated test database (398 pass).
bun run typecheck,bun run verifyandbun run wire-contract:checkare clean, and the snapshot is unchanged.Docs
05-integrations/monerium-b2b.md): the lattice invariant, the refund invariant (status-first ordering, closing a manual refund) and the test checklist.recovery_failed --> refundededge.vortex-refund:<depositId>; step 6 says the close works fromrecoveringorrecovery_failed.Merge notes
recovery.test.ts(new tests inserted before "holds the queue on a failed refund…") and in the spec, runbook and architecture docs. Keep both sides.setDepositStatus: async (row, status) => { await row.update({ status }); }needs areturn null;to satisfy the newPromise<string | null>type.fail()writeserror. On staging without Fix the refund retry cap and a reverse swap skipped on a lagging read #1410,retryOrFailstill writeserrorbelow the cap. That is Fix the refund retry cap and a reverse swap skipped on a lagging read #1410's bug, and it is fixed there.Review follow-up (vortex-review)
The review found no P0 or P1. Fixed here: the one P2 and the cheap P3s.
recovering, but that retry re-reads the same order, or adopts it again by memo, and parks the deposit again. §2.7 and the §3REFUND FAILEDrow now retry only after five failed attempts. A rejected order joins the cases the operator refunds by hand and closes asrefunded.error, soDEPOSIT_RETURNEDafter a hand close never names an order that paid nothing.docs/api/pages/07-webhooks.mdnow saysredeemOrderIdis null for a refund completed by hand.REFUND FAILEDonly once parked. The line is logged only after the park succeeds. The refusal line carries the reason, and both new error lines have a §3 triage row.recovery.ts. The spec checklist covers the hand close and no longer claims the status PATCH refuses pending executions (only the recover POST does).redeemedwithredeemOrderIdnull, and only one redeem order is ever placed.REFUND FAILEDis not logged until the park succeeds.Not changed (P3, left for later):
refundedAtis derived fromrecovery.updatedAt, a narrow crash window infail(), no balance check on a hand close, an emptypayerIbanMaskedfor a no-payer hand close, and the memo as the payer-visible reference.Gates: the B2B, workers and controllers tests pass on an isolated database (399).
typecheck,verifyandwire-contract:checkare clean.