feat(payments): GlobePay365 deposit + withdrawal gateway - #252
Open
elstonyth wants to merge 39 commits into
Open
feat(payments): GlobePay365 deposit + withdrawal gateway#252elstonyth wants to merge 39 commits into
elstonyth wants to merge 39 commits into
Conversation
Adds the GlobePay365 integration layer (no route wiring yet — the deposit
callback route and pending-top-up state land once a public NotifyUrl exists).
globepay.ts — the wire format, pure and container-free:
- AES-256-CBC with PBKDF2-HMAC-SHA1(pw=key, salt=key, 1000, 32) and the IV
prepended to the ciphertext (§1.11).
- RSA-SHA1 PKCS#1 v1.5 sign/verify, accepting the unarmored base64 keys both
sides actually exchange (§1.14).
- buildEnvelope serializes the payload ONCE so the encrypted and signed bytes
are identical — they verify against the JSON they decrypt (§1.16).
- openCallback verifies before JSON.parse, so an unverified callback can never
reach a caller.
- Deposit status 4 ("Verify Fail") maps to pending, not failed: the doc marks
it explicitly non-final, and treating it as failure would strand real money.
globepay-client.ts — SubmitDeposit, GetDepositDetail, CheckBalance:
- Amount is sent as a 2dp string ("25.00"); a JS number drops their format.
- GlobePayError carries their PMT* codes so PMT10000 (duplicate reference) can
be handled as a replay rather than a failure.
- isSuccess:true with a null data is treated as failure.
- Config throws on a missing env var instead of signing with an empty key.
- Mandatory request timeout: a hung gateway call would pin a worker mid-deposit.
Verified live against staging (MerchantCode Testpolycard): CheckBalance returns
isSuccess:true, which exercises the whole chain — API IP whitelist, AES payload
they could decrypt, and a signature verified against our uploaded public key.
Envelope key casing tested both ways; their binding is case-insensitive.
Specs use throwaway keys — the provider's real AES key is env-only.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First real staging deposit (D2026072112415767, BQR, MYR 50). Records what the live calls proved that the doc did not: - Only BQR has a provisioned channel. FPX returns PMT10018, OB returns PMT10005, and DN returns isSuccess:false with an EMPTY errorList. "Active in the back office" does not mean a channel exists. - The blank Merchant Cashier URL does NOT trip PMT10007 — that open question is closed, no config needed for this flow. - An unpaid deposit requeries as statusId 4 "VerifyFail". Live confirmation that mapping 4 to failed would strand money that can still settle. - Amount as the 2dp string "50.00" is accepted. - Unknown-id requery returns HTTP 400 plain-text "Not found", not JSON and not the documented PMT10016. - qrCode is a ~130KB base64 PNG data URI: never log or persist it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…table Credit now happens asynchronously, on a verified callback, instead of inside the request. Adds the state that makes that possible and the route that does it. globepay_deposit (model + additive migration): the deposit callback echoes MerchantTransactionId but NOT MerchantClientId (§1.2.3), so without a row written before SubmitDeposit there is no customer to credit. The alternative — encoding customer_id into MerchantTransactionId — would publish an internal id into their back office and still leave outstanding deposits unrequeryable. This table is NOT the ledger; credit_transaction stays the money record. POST /hooks/globepay/deposit — deliberately outside /store/*, since a webhook carries neither a customer session nor a publishable key. Authentication is the RSA-SHA1 signature over the AES payload, not a header and not the source IP. Ack contract (§1.2.1), the part that is easy to get wrong: - verified AND durably handled -> "success", INCLUDING status 7. A failed deposit is handled, not an error; 400-ing it makes them retry a dead deposit forever. - signature/decrypt failure or a transient error -> non-2xx, so a genuine callback we failed to process is retried and the money still lands. Other money rules: - Credits the amount THEY confirmed, not the amount we requested — a customer can pay a different sum. - Idempotency anchored on their TransactionId (globally unique, repeated on every retry) through the existing source_transaction_id dedupe. - Status 4 is a no-op, never marked failed: the doc calls it non-final. - Refuses to credit a non-positive amount. Tests: 12 route specs covering forged signature, tampered amount, each status branch, unknown reference, credit failure, and idempotency anchoring. Full unit suite green (875 passed). Open question recorded in docs: Amount vs NetAmount. The route credits Amount; confirm against the first genuinely settled callback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sits A late status-7 arriving after a deposit settled would flip the row to failed while the credit stands, leaving the record contradicting the ledger. Guard on the row's own status before acting. Safe against legitimate retries: the transient-error path returns non-2xx WITHOUT touching the row, so a callback we genuinely failed to process is still 'pending' when they retry it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion Completes the deposit loop: POST /store/credits/deposit records intent, asks the gateway for a cashier page, and returns the URL. No credit is issued here — that stays with the verified callback. globepay-deposit.ts: - The pending row is written BEFORE the gateway call, so a callback can never arrive for a reference we have no record of (their callback echoes MerchantTransactionId but not MerchantClientId). - MerchantTransactionId is an opaque PC-<uuid>. It is displayed in THEIR back office, so it must not carry our customer id; the row is the mapping. - A refused submit closes the row out instead of leaving it pending forever and polluting the reconciliation sweep. - globepayEnabled() fails closed: needs GLOBEPAY_ENABLED=true AND a merchant code, mirroring mockTopupAllowed. The mock top-up route is untouched and stays the local/dev path. The route lives alongside /store/credits/topup rather than replacing it, and callback/return URLs are explicit env vars — deriving them from a STOREFRONT_URL-style var would silently fall back to a localhost default in production, where the var is named MERCUR_STOREFRONT_URL. MIGRATION FIX, caught only by running it: model.bigNumber() is TWO columns — the numeric plus a raw_* jsonb. The original migration omitted raw_amount_* and passed every mocked test, then failed on the first real insert with 'column "raw_amount_requested" does not exist'. Now matches credit_transaction. Verified against a real database (local medusa): migration applies, and create/list/update/delete GlobePayDeposits all work with the generated method names. The unique index rejects a duplicate merchant_transaction_id before the gateway's PMT10000 would. Tests: 47 globepay specs; full unit suite 885 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… server Proves what the mocked unit specs cannot: - /hooks/globepay/deposit is reachable with NO customer token and NO publishable key. If it had landed behind the store middleware, every real callback would have been rejected and every paid deposit lost. - A verified status 6 writes a real credit_transaction row (reason topup, pull_id null, reference = their TransactionId) and flips the deposit row to settled with amount_settled and settled_at. - A retried callback credits exactly once — the TransactionId idempotency anchor works through the real per-customer locked mutation, not a mock. - The credited amount comes from the callback, not the requested row. - A forged signature writes nothing and leaves the row pending, so the genuine callback can still settle it. - Status 7 marks failed without crediting; status 4 stays pending and a later status 6 still credits. The test signs as the gateway with a throwaway keypair, so their actual signature bytes remain the only unverified link — that needs Bryan to enable staging settlement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security review finding. Only the `Data` field of a GlobePay365 callback is covered by the RSA signature. Every other top-level field — TransactionId, MerchantTransactionId, Version — can be altered on an otherwise-genuine body without invalidating anything. The credit's idempotency anchor was derived from the UNSIGNED TransactionId, with precedence over the signed reference. Since mutateCreditAtomic dedupes purely by that anchor, one captured callback body replayed with N different TransactionId values produced N different anchors, missed the dedupe every time, and appended N credits. One payment could mint arbitrary credit. The attacker path needs a captured body, which the signature makes hard. The reason this is worth fixing regardless is that it double-credits with NO attacker at all: the anchor rested on an unverified assumption that the gateway repeats TransactionId verbatim across its own retries. We have never seen a genuine callback, so that assumption was never tested. Fixes: - Anchor on the signed MerchantTransactionId. Every replay of a deposit now collapses to one anchor and the existing per-customer locked dedupe holds, whatever the unsigned field says. TransactionId stays as the display-only reconciliation handle. - Reject a callback whose SIGNED payload carries no MerchantTransactionId instead of falling back to the unsigned envelope copy — that fallback let an unsigned field select which deposit row, and therefore which customer, got credited. - Key the feed receipt on the signed reference too, so a varied TransactionId cannot produce a second receipt for one payment. - Flip the row with a conditional selector (id + status pending) so a losing concurrent callback no-ops rather than overwriting settled_at/amount_settled. Tests: 5 new unit specs (anchors collapse across varied TransactionId, distinct anchors for distinct deposits, missing signed reference rejected, conditional claim) and 2 new integration specs proving against the REAL ledger that a varied-TransactionId replay leaves exactly one credit row. Unit suite 888 passed; globepay integration 10/10. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ards Reconciliation (src/jobs/globepay-reconcile.ts, every 10 minutes) Their callback is fire-and-forget over the public internet. One dropped POST — our deploy, their retry budget, a DNS blip — means a customer paid and never got credit, permanently, with nothing in the system that would notice. GetDepositDetail is the authoritative read and the provider's own guidance is to requery rather than trust a callback. - Crediting uses the SAME idempotency anchor as the callback route (the signed MerchantTransactionId), so a callback and a sweep racing on one deposit produce exactly one credit, whichever wins. - A success settles at ANY age: the stale window must never write off money that actually landed. Only non-final deposits older than an hour expire, and expiry just stops us chasing them — it never contradicts the gateway. - A requery 400 "Not found" means SubmitDeposit never took, so nobody can ever pay it; those expire once old enough that an in-flight submit is impossible. - Oldest first, 50 per sweep, and one failing deposit never aborts the run — the next row may be a customer waiting on credit. Hardening from the security review (both below the reporting bar): - The callback refuses to credit unless CurrencyCode matches the configured currency. The ledger is Ringgit and credits 1:1, so a settled callback in another currency would credit 500 VND as RM 500. CurrencyCode is signed, so this guards against reconfiguration, not an attacker. - payment_method_code from POST /store/credits/deposit is allow-listed to the documented MYR set (FPX/DN/BQR/OB) instead of being passed through, so a caller cannot request another currency's method and depend on gateway-side behaviour we cannot see. Tests: 8 reconciliation unit specs (policy) and 8 integration specs against a real ledger — dropped callback credited, late callback does not double-credit, double sweep credits once, failed closed, young left pending, stale expired, sweep survives a mid-run error. Unit suite 899 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wires the top-up sheet to the real gateway behind NEXT_PUBLIC_PAYMENTS_PROVIDER=globepay. Anything else keeps the mock gateway, which stays the local/dev path — this is a switch, not a replacement. Redirect flow rather than an in-page QR: the provider's cashier page already owns method selection, the QR, expiry and error states, so rendering our own would duplicate all of it for no gain today. The response still carries qrCode/bankCode/accountNumber if we ever want that. The sheet's contract changes on this path, and the copy has to match: nothing is credited when the button is pressed. startDeposit returns a cashier URL and the balance only moves later, when the provider's signed callback settles the deposit. So there is no Idempotency-Key here — the backend mints a fresh reference per attempt, and a re-clicked button costs an abandoned deposit row, not a double charge. Two live-tested details from staging: - The gateway's floor is RM 30 (25 is rejected). The mock's 10/25 presets would have guaranteed a rejection, so presets become 30/50/100/200 and the default 50 on this path. The sheet checks the floor itself, because the gateway's own refusal is a generic "Invalid Transaction Amount" that tells the customer nothing. - Full navigation, not window.open: a popup that follows an await gets eaten by blockers, and the customer has to land back on our return URL anyway. tsc clean, vitest 285 passed, lint 0 errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ging BQR was recorded as working because SubmitDeposit returns a cashier URL. It does not work. Opening that URL shows a caution modal and then, after agreeing, "Sorry, we're experiencing technical issues" — on a deposit about a minute old, so this is not the QR timing out. Four MYR methods fail at the API and the fifth fails at the cashier: nothing is payable on staging. A returned cashier URL is not evidence a channel works; only a rendered QR is. Also records that two deposits created minutes earlier requeried as HTTP 400 "Not found" while an older one resolved normally — either their requery lags submit, or those were never persisted. Unresolved, and the reason the sweep waits an hour before expiring an unknown deposit rather than acting on the first Not found. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iver Found by driving the sheet in a real browser. On the gateway path the button read "Proceed — add RM 50.00", the summary said "New balance", and the footnote advertised the mock gateway's fake .13 decline. All three are false once the button leaves the site: nothing is added, the balance does not move, and the .13 rule belongs to the mock. - Button: "Pay RM 50.00" / "Taking you to payment…" on the gateway path. - Summary row: "Balance once paid" rather than "New balance". - Footnote: explains the redirect and that credits appear after the payment is confirmed. The mock path keeps its original copy verbatim. Verified in Chromium against the built storefront on :4200 and the backend on :9200: the sheet opens with the gateway rungs (RM 30/50/100/200), and pressing the button leaves our origin for cashier.globepay365stg.com/Deposit/DuitNow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Provider answered three of our open questions (2026-07-22).
Amount vs NetAmount — ANSWERED, and our code was already right. Credit
`Amount`; `NetAmount` is that figure minus their fees ("net amount 是减了费用").
Crediting NetAmount would have silently shorted every customer by the fee. Only
the comment and docs changed.
Amount limits — RM 30 min, RM 1000 max, confirmed and verified live (1000
accepted, 1001 returns PMT10005). Enforced in both the sheet and
startGlobePayDeposit: the gateway's own refusal is a bare "Invalid Transaction
Amount" that names no numbers, and a doomed request would still leave a failed
row behind. The sheet previously checked only the floor.
Also records what their reply did NOT resolve:
- They recommended OB, but OB fails at EVERY amount in the band they gave:
PMT10005 up to 200, then PMT10024 "Invalid Payment Method Routing Amount"
from 300 up. PMT10024 is a routing gap on their side, not an amount problem.
BQR is still the only method returning a cashier page, and that page still
fails at QR generation — so nothing is payable yet.
- How to settle a staging deposit is still unanswered (asked twice), so no
genuine callback can reach us and their signature bytes stay unverified.
Tests: 5 new band specs (29/1001/5000 rejected before any network call, 30 and
1000 accepted at the exact edges). Backend 904 passed, storefront 285, both
typechecks clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gateway path (presets, band guard, cashier redirect, no-credit copy) shipped with zero tests; a misleading-copy bug there was only caught by manual browser driving. Six jsdom tests now pin the behaviors that matter. window.location.assign moves behind a leaveFor() seam because jsdom marks window.location unforgeable and navigation is otherwise unobservable in a test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @elstonyth, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds GlobePay365 deposit and withdrawal integration with encrypted gateway communication, persistent transaction records, authenticated callbacks, idempotent ledger settlement/refunds, scheduled reconciliation, protected APIs, admin deposit visibility, and gateway-aware customer interfaces. ChangesGlobePay365 payment integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Customer
participant TopUpSheet
participant DepositAPI
participant GlobePay
participant DepositCallback
participant Ledger
Customer->>TopUpSheet: Submit gateway deposit
TopUpSheet->>DepositAPI: POST /store/credits/deposit
DepositAPI->>GlobePay: SubmitDeposit
GlobePay-->>DepositAPI: Cashier URL
DepositAPI-->>TopUpSheet: Deposit start result
TopUpSheet->>GlobePay: Open cashier
GlobePay->>DepositCallback: Send signed callback
DepositCallback->>Ledger: Apply confirmed amount idempotently
DepositCallback-->>GlobePay: Return success
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
backend/packages/api/src/modules/packs/globepay.ts (1)
1-1: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSHA-1/RSA-1024 SAST flags across three files are protocol-mandated, not fixable — suppress rather than re-flag.
CodeQL and ast-grep repeatedly flag
RSA-SHA1signing/verification and 1024-bit RSA key generation as weak crypto (CWE-327). All three sites exist because GlobePay365's fixed wire protocol requires exactly this scheme — the code comments anddocs/payments/globepay365-setup.mdare explicit that upgrading to 2048-bit/SHA-256 would break interop with the gateway. Since this can't be changed, the actionable fix is to silence the recurring finding (inline suppression comment or a SAST allowlist entry) so it stops generating noise on every scan and doesn't drown out genuinely new findings.
backend/packages/api/src/modules/packs/globepay.ts#L82-108: add a suppression annotation (or SAST config allowlist entry) for theRSA-SHA1sign/verify calls, referencing the protocol constraint.backend/packages/api/src/modules/packs/__tests__/globepay.unit.spec.ts#L16-20: suppress the 1024-bitgenerateKeyPairSyncflag on this test fixture the same way.backend/packages/api/src/modules/packs/__tests__/globepay-client.unit.spec.ts#L12-16: same suppression for this fixture's 1024-bit keypair.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/modules/packs/globepay.ts` at line 1, Suppress the mandated weak-crypto findings for the GlobePay365 protocol sites: annotate the RSA-SHA1 sign/verify calls in the globepay implementation and both 1024-bit generateKeyPairSync test fixtures, or add equivalent SAST allowlist entries. Reference the protocol interoperability constraint in each suppression and limit changes to these three identified locations.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.ts`:
- Around line 11-15: All five generateKeyPairSync fixtures use a weak 1024-bit
RSA modulus; update modulusLength to 2048 in
backend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.ts
lines 11-15 and 105-109,
backend/packages/api/integration-tests/http/globepay-callback.spec.ts lines
19-23 and 189-193, and
backend/packages/api/integration-tests/http/globepay-reconcile.spec.ts lines
27-31.
In `@backend/packages/api/src/api/store/credits/deposit/route.ts`:
- Around line 45-53: Update the IP selection in the deposit route so req.ip is
preferred before any raw X-Forwarded-For or socket fallback, preventing clients
from overriding the value sent to GlobePay365. Preserve the existing final
default and verify Express trust-proxy configuration is enabled; add the
appropriate configuration if it is not already set.
In `@backend/packages/api/src/modules/packs/globepay-client.ts`:
- Around line 22-43: Update globepayConfigFromEnv so baseUrl uses
required('GLOBEPAY_API_BASE') instead of defaulting to the staging host, while
preserving trailing-slash removal. Keep the existing defaults for optional
currencyCode unchanged.
In `@backend/packages/api/src/modules/packs/globepay-deposit.ts`:
- Around line 166-180: Move the failed-status update in the catch block of the
GlobePay deposit flow so it executes only when error is an instance of
GlobePayError. Preserve the existing invalid-data MedusaError conversion for
definitive gateway rejections, while rethrowing transient errors without
changing the row from pending so globepay-reconcile.ts can requery it.
In `@src/components/app-shell/TopUpSheet.tsx`:
- Around line 249-254: Update TopUpSheet’s static Demo badge rendering so it is
hidden or replaced when USE_GATEWAY is enabled, while preserving the badge for
the non-gateway flow. Extend the gateway-specific test to assert that the Demo
badge is not rendered.
---
Nitpick comments:
In `@backend/packages/api/src/modules/packs/globepay.ts`:
- Line 1: Suppress the mandated weak-crypto findings for the GlobePay365
protocol sites: annotate the RSA-SHA1 sign/verify calls in the globepay
implementation and both 1024-bit generateKeyPairSync test fixtures, or add
equivalent SAST allowlist entries. Reference the protocol interoperability
constraint in each suppression and limit changes to these three identified
locations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 636f3ee5-52db-447e-93d0-277869286517
📒 Files selected for processing (24)
backend/packages/api/integration-tests/http/globepay-callback.spec.tsbackend/packages/api/integration-tests/http/globepay-reconcile.spec.tsbackend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/deposit/route.tsbackend/packages/api/src/api/middlewares.tsbackend/packages/api/src/api/store/credits/deposit/route.tsbackend/packages/api/src/jobs/globepay-reconcile.tsbackend/packages/api/src/modules/packs/__tests__/globepay-client.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-deposit.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-reconcile.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay.unit.spec.tsbackend/packages/api/src/modules/packs/globepay-client.tsbackend/packages/api/src/modules/packs/globepay-deposit.tsbackend/packages/api/src/modules/packs/globepay-reconcile.tsbackend/packages/api/src/modules/packs/globepay.tsbackend/packages/api/src/modules/packs/migrations/Migration20260721140000.tsbackend/packages/api/src/modules/packs/models/globepay-deposit.tsbackend/packages/api/src/modules/packs/service.tsdocs/payments/globepay365-setup.mdsrc/components/app-shell/TopUpSheet.tsxsrc/components/app-shell/__tests__/topup-sheet-gateway.test.tssrc/lib/actions/vault.tssrc/lib/data/schemas.tssrc/lib/navigation.ts
CodeQL flagged the 1024-bit fixture keys on PR #252 (js/insufficient-key-size, 8 alerts). They are self-signed test keys, not the provider's, but 2048 costs nothing and keeps the scanner quiet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deposit loop's mirror with the money ordering inverted: row -> atomic
ledger DEBIT (floor 0) -> SubmitWithdrawal, refund on refusal. A failed
payout (callback status 5, requery, or a stale gateway-unknown row) refunds
through one idempotency anchor, so any mix of retries produces exactly one
refund. Payout Verification (§1.7) is implemented as a second factor: only a
pending payout we recorded, at the exact debited amount, gets "success".
- globepay_withdrawal table (raw_ columns included) + 10-min reconcile sweep
that never expires a debit — settle, refund, or chase loudly after 24h
- POST /store/credits/withdraw + GET /store/credits/withdraw/banks (proxied
GetSupportedBanks, cached), both authenticated + rate-limited
- hooks: /hooks/globepay/withdrawal, /hooks/globepay/payout-verify — RSA
signature is the auth; only signed fields decide anything
- storefront: real form on /bank-withdrawal behind
NEXT_PUBLIC_WITHDRAWALS_ENABLED, async-honest copy ("on its way"), feed
templates withdrawal_paid / withdrawal_refunded (toast: no owning tab)
- fail-closed switches: GLOBEPAY_WITHDRAWALS_ENABLED on top of
GLOBEPAY_ENABLED, so deposits can stay live while payouts wait
Tested: 37 new unit + 7 integration (booted server + real Postgres: refund
idempotency under varied unsigned TransactionId, late-fail-after-settle,
payout-verify tamper). Verified LIVE against staging: GetSupportedBanks (31
banks), SubmitWithdrawal accepted (W2026072202370961, RM 30 debited), requery
returns processing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
elstonyth
changed the title
The backend's withdrawal messages are already customer-facing, but none of them matched a VAULT_RULES pattern, so a live gateway refusal rendered as the generic fallback. Pass them through (band, refusal, not-open, insufficient) — observed live when the staging merchant balance could not cover a second RM 30 payout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code review (pre-merge) found three critical gaps; all fixed with the
machinery already in place:
1. Cashout now routes through walletSummary's withdrawable gate, honoring
the withdrawable.ts invariant ("the cashout writer MUST route through
this"): frozen accounts, locked unmatured commissions, and unplayed
deposits (playthrough / anti-laundering) can no longer leave to a bank.
The form shows the withdrawable figure, not raw balance, with distinct
messages for each closed gate.
2. Ambiguous submit errors (timeout, reset, WAF page — the payout may have
been accepted with the response lost) no longer refund immediately, which
could double-pay when the payout later executed. GlobePayError now
carries a `definite` flag set only on a parsed isSuccess:false refusal;
ambiguity leaves the row pending for the sweep to resolve.
3. The sweep's unknown-refund path refuses any row that has a gateway
transaction id: the payout provably exists there, so a requery 400 is our
own credentials being broken — refunding would systematically double-pay
every in-flight payout on one bad config deploy.
Also: the sweep refunds only if the wd: debit row actually exists (a crash
before the debit must not mint money), contradictory final callbacks on
resolved rows are logged loudly as double-payment tripwires, and
payout-verify now rejects currency mismatches.
11 new unit tests pin all of it; 110 globepay unit + 7 integration green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/app/bank-withdrawal/WithdrawForm.tsx (2)
15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
WD_MIN_RM/WD_MAX_RMduplicate the backend limits and the friendly-error text invault.ts.See consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/bank-withdrawal/WithdrawForm.tsx` around lines 15 - 16, Remove the duplicated WD_MIN_RM and WD_MAX_RM constants from WithdrawForm and reuse the centralized withdrawal-limit definitions from vault.ts, updating validation and user-facing error handling to reference those shared symbols consistently.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWithdrawal RM 30/RM 1,000 band is hardcoded in two frontend places (plus the backend). Both sites independently encode the same limits with no shared constant, so a backend limit change requires manually chasing down and updating both copies or the client-side gate/error text silently drifts from the real limits.
src/app/bank-withdrawal/WithdrawForm.tsx#L15-16: extractWD_MIN_RM/WD_MAX_RMinto a single shared module (e.g.src/lib/constants/withdrawal.ts) that both this file andvault.tsimport.src/lib/actions/vault.ts#L65-68: build the friendly-error message from the same sharedWD_MIN_RM/WD_MAX_RMconstants instead of a separately hardcoded string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/bank-withdrawal/WithdrawForm.tsx` at line 1, Extract the withdrawal minimum and maximum limits currently defined in WithdrawForm into shared WD_MIN_RM and WD_MAX_RM constants in a reusable module, then import and use them in both WithdrawForm and vault.ts. Update vault.ts’s friendly withdrawal-limit error message to interpolate those shared constants, removing the duplicated hardcoded values while preserving the existing validation behavior.src/app/bank-withdrawal/page.tsx (2)
26-28: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip
getWallet()when withdrawals are closed.
walletResult/withdrawableare unused in the "not open yet" branch, so this fetches the wallet on every signed-in page load for no benefit whileWITHDRAWALS_OPENis false.♻️ Proposed fix
- const walletResult = customer ? await getWallet() : null; + const walletResult = + customer && WITHDRAWALS_OPEN ? await getWallet() : null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/bank-withdrawal/page.tsx` around lines 26 - 28, Update the wallet-loading logic around getWallet so it only runs when withdrawals are open, in addition to requiring a customer; otherwise set walletResult and withdrawable to their existing null values. Preserve the current successful wallet extraction behavior when withdrawals are enabled.
17-19: 🩺 Stability & Availability | 🔵 TrivialKeep the withdrawal gate runtime-safe
NEXT_PUBLIC_WITHDRAWALS_ENABLEDis still baked in at build time, so a backend-only flip ofGLOBEPAY_WITHDRAWALS_ENABLEDcan leave the page and API out of sync until the frontend is rebuilt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/bank-withdrawal/page.tsx` around lines 17 - 19, Update the withdrawal gate represented by WITHDRAWALS_OPEN so it reads the runtime-controlled backend setting GLOBEPAY_WITHDRAWALS_ENABLED rather than the build-time NEXT_PUBLIC_WITHDRAWALS_ENABLED value. Preserve the existing enabled-when-true behavior and keep the page gate aligned with the API without requiring a frontend rebuild.src/lib/actions/vault.ts (1)
65-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded RM 30/RM 1,000 band duplicates the client-side gate in
WithdrawForm.tsx.See consolidated comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/vault.ts` around lines 65 - 68, The withdrawal validation in the vault action hardcodes the RM 30–RM 1,000 range already enforced by WithdrawForm.tsx. Replace the literal bounds in the `/withdrawals must be between/i` rule and its message with the shared withdrawal-limit constants or configuration used by the client-side gate, keeping server validation and displayed limits synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/notifications/copy.ts`:
- Around line 214-219: Update the withdrawal_refunded notification body in the
withdrawal copy definition to use cause-agnostic wording instead of stating that
the bank rejected the transfer. Preserve the existing amount formatting and null
handling while ensuring the message remains accurate for gateway failures,
timeouts, and reconciliation-triggered refunds.
---
Nitpick comments:
In `@src/app/bank-withdrawal/page.tsx`:
- Around line 26-28: Update the wallet-loading logic around getWallet so it only
runs when withdrawals are open, in addition to requiring a customer; otherwise
set walletResult and withdrawable to their existing null values. Preserve the
current successful wallet extraction behavior when withdrawals are enabled.
- Around line 17-19: Update the withdrawal gate represented by WITHDRAWALS_OPEN
so it reads the runtime-controlled backend setting GLOBEPAY_WITHDRAWALS_ENABLED
rather than the build-time NEXT_PUBLIC_WITHDRAWALS_ENABLED value. Preserve the
existing enabled-when-true behavior and keep the page gate aligned with the API
without requiring a frontend rebuild.
In `@src/app/bank-withdrawal/WithdrawForm.tsx`:
- Around line 15-16: Remove the duplicated WD_MIN_RM and WD_MAX_RM constants
from WithdrawForm and reuse the centralized withdrawal-limit definitions from
vault.ts, updating validation and user-facing error handling to reference those
shared symbols consistently.
- Line 1: Extract the withdrawal minimum and maximum limits currently defined in
WithdrawForm into shared WD_MIN_RM and WD_MAX_RM constants in a reusable module,
then import and use them in both WithdrawForm and vault.ts. Update vault.ts’s
friendly withdrawal-limit error message to interpolate those shared constants,
removing the duplicated hardcoded values while preserving the existing
validation behavior.
In `@src/lib/actions/vault.ts`:
- Around line 65-68: The withdrawal validation in the vault action hardcodes the
RM 30–RM 1,000 range already enforced by WithdrawForm.tsx. Replace the literal
bounds in the `/withdrawals must be between/i` rule and its message with the
shared withdrawal-limit constants or configuration used by the client-side gate,
keeping server validation and displayed limits synchronized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5701771e-0bd4-4ae1-a65a-c9ef5e6d3006
📒 Files selected for processing (31)
backend/packages/api/.mercur/index.d.tsbackend/packages/api/integration-tests/http/globepay-callback.spec.tsbackend/packages/api/integration-tests/http/globepay-reconcile.spec.tsbackend/packages/api/integration-tests/http/globepay-withdrawal.spec.tsbackend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/payout-verify/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/payout-verify/route.tsbackend/packages/api/src/api/hooks/globepay/withdrawal/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/withdrawal/route.tsbackend/packages/api/src/api/middlewares.tsbackend/packages/api/src/api/store/credits/withdraw/banks/route.tsbackend/packages/api/src/api/store/credits/withdraw/route.tsbackend/packages/api/src/jobs/globepay-withdrawal-reconcile.tsbackend/packages/api/src/modules/packs/__tests__/globepay-client.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-withdrawal.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay.unit.spec.tsbackend/packages/api/src/modules/packs/feed-events.tsbackend/packages/api/src/modules/packs/globepay-client.tsbackend/packages/api/src/modules/packs/globepay-reconcile.tsbackend/packages/api/src/modules/packs/globepay-withdrawal.tsbackend/packages/api/src/modules/packs/migrations/Migration20260722170000.tsbackend/packages/api/src/modules/packs/models/globepay-withdrawal.tsbackend/packages/api/src/modules/packs/notify-feed.tsbackend/packages/api/src/modules/packs/service.tssrc/app/bank-withdrawal/WithdrawForm.tsxsrc/app/bank-withdrawal/__tests__/withdraw-form.test.tssrc/app/bank-withdrawal/page.tsxsrc/lib/actions/vault.tssrc/lib/data/schemas.tssrc/lib/notifications/__tests__/copy.test.tssrc/lib/notifications/copy.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/packages/api/src/modules/packs/service.ts
- backend/packages/api/integration-tests/http/globepay-callback.spec.ts
- backend/packages/api/src/modules/packs/tests/globepay-client.unit.spec.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/packages/api/src/modules/packs/service.ts (2)
3501-3515: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThread
sharedContextintocreditSummary.
loadVipStateInputspasses the context to the lifetime read immediately above, butcreditSummary(customerId)creates a context-less read. When called inside a transaction, the lifetime and net-turnover values can come from different snapshots, producing an inconsistent VIP projection or reward decision.Proposed fix
- const netBasisMyr = (await this.creditSummary(customerId)).vipSpendTotal; + const netBasisMyr = ( + await this.creditSummary(customerId, sharedContext) + ).vipSpendTotal;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/modules/packs/service.ts` around lines 3501 - 3515, Update loadVipStateInputs to pass sharedContext into creditSummary alongside customerId, preserving the existing sequential reads and ensuring both VIP inputs use the same transaction context.
3422-3442: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the external-funded contract for commission tiers.
This changes
lifetimeExternalSenForto count every originalpack_open, butsettleOpenstill uses it at Lines 2037-2050 as the sponsor’s lifetime external-funded spend when selecting commission rates. Winnings-funded opens can therefore promote sponsors and overpay future commissions. Keep this helper external-funded and add a separate turnover helper, or update all consumers and projection naming together.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/modules/packs/service.ts` around lines 3422 - 3442, Keep lifetimeExternalSenFor restricted to external-funded pack_open debits so settleOpen continues selecting commission tiers from the sponsor’s external-funded spend. Move the all-funding-source turnover calculation into a separate helper with turnover-specific naming, and update only turnover consumers to use it; preserve the existing reversal and deleted-record exclusions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/packages/api/src/modules/packs/service.ts`:
- Around line 3501-3515: Update loadVipStateInputs to pass sharedContext into
creditSummary alongside customerId, preserving the existing sequential reads and
ensuring both VIP inputs use the same transaction context.
- Around line 3422-3442: Keep lifetimeExternalSenFor restricted to
external-funded pack_open debits so settleOpen continues selecting commission
tiers from the sponsor’s external-funded spend. Move the all-funding-source
turnover calculation into a separate helper with turnover-specific naming, and
update only turnover consumers to use it; preserve the existing reversal and
deleted-record exclusions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b0b0239-1440-4af0-b017-ae57d2d89138
📒 Files selected for processing (3)
backend/packages/api/src/api/middlewares.tsbackend/packages/api/src/modules/packs/service.tssrc/lib/data/schemas.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/packages/api/src/api/middlewares.ts
- src/lib/data/schemas.ts
…probe Two gaps that would make an unattended launch unsurvivable, neither of which replaces a human scan-and-pay test. Admin "Deposits" page (Orders -> Deposits): GET /admin/globepay/deposits plus the dashboard route. Read-only by design -- the 10-minute reconcile sweep is the authoritative repair path, and a manual credit button would be a second, unaudited way to mint credit. Pending sorts oldest first because the row most likely to be a stranded payment is the one that has waited longest. `stale` is computed from the SAME GLOBEPAY_STALE_AFTER_MS the sweep uses, so the page and the job can never disagree about what counts as overdue. Before this, finding a stranded payment meant hand-written SQL against production. scripts/probe-globepay-callback.mjs: POSTs two unverifiable bodies at /hooks/globepay/deposit and demands a 400. Safe against production -- an unsigned body can never pass openCallback's RSA check. It distinguishes UNREACHABLE / NOT_DEPLOYED / ACCEPTS_UNSIGNED / SERVER_ERROR, because the route acks with the literal body "success" to stop gateway retries, so a broken deploy that acked everything would look healthy to a naive check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ateway # Conflicts: # backend/apps/admin/src/lib/admin-rest.ts # backend/apps/admin/src/lib/queries.ts # backend/packages/api/src/modules/packs/service.ts
Two review findings on the deposit money path. 1. startGlobePayDeposit marked the row 'failed' before checking the error type, so a timeout or socket reset closed out a deposit that may well have landed at the gateway. The reconciliation sweep only scans status='pending', so that row left requery permanently and the payment was stranded -- the exact failure the sweep exists to prevent. Only a GlobePayError (their API answered and refused) is definitive enough to mark failed; everything else stays pending for the sweep, which is the authoritative read. 2. GLOBEPAY_API_BASE defaulted to the STAGING host while every other credential used required(). A production deploy that forgot one var would have pointed production credentials at staging and started fine. It now fails loudly like the rest. Regressions for both, plus the doc's env table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two review findings on customer-facing copy in the real-money path. The TopUpSheet kept its static "Demo" badge in gateway mode, on the one screen that redirects to a live cashier and takes real money. Hidden when NEXT_PUBLIC_PAYMENTS_PROVIDER=globepay, and the gateway spec now asserts it. withdrawal_refunded said "Your bank rejected the transfer". That notification also fires on gateway-side status-5 failures and on the reconcile sweep's ambiguous/stale payouts, so it blamed the bank for failures the bank never saw. Now cause-agnostic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
admin.polycards.gg is the PRIMARY domain on polycards-backend; api.polycards.gg does not resolve. The wrong host in the pre-launch instruction would have made the callback probe report UNREACHABLE against a URL nobody serves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…API host
CI caught two mistakes in the previous two commits.
1. Resolving the admin-rest.ts merge conflict as a union dropped the closing
brace of getGlobePayDeposits: both conflict sides ended before the same
shared '}', so the next function swallowed it. Lint caught it as a parse
error at the end of the file. Local `tsc --noEmit` did NOT: the admin
tsconfig is solution-style ("files": [], project references), so that
invocation type-checks zero files and exits 0. `tsc -b` is the real check.
2. Making GLOBEPAY_API_BASE required broke every spec that leaned on an ambient
value: three hook route specs and the three GlobePay integration specs boot
against process.env, and CI has no local dotenv file to fall back on. They
now set the host explicitly, like the other GlobePay credentials.
Verified with the host var blanked so the CI condition is reproduced locally:
1126 backend unit tests pass, and the 3 GlobePay HTTP integration suites (25
tests) pass against a booted server. Storefront format check restored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ateway # Conflicts: # backend/packages/api/src/modules/packs/notify-feed.ts # backend/packages/api/src/modules/packs/service.ts # src/lib/notifications/__tests__/copy.test.ts # src/lib/notifications/copy.ts
…header The deposit route read the raw X-Forwarded-For first hop before req.ip, so a caller could choose the IPAddress we report to GlobePay365 — the comment above it already claimed the opposite. Medusa's express-loader sets `trust proxy` 1 unconditionally (node_modules/@medusajs/framework/dist/http/express-loader.js), so req.ip is proxy-derived and not client-settable; the header stays as a fallback for a deployment where req.ip comes back empty. Spec covers the whole precedence chain plus the fail-closed branch. Docs, from the wiring audit: - The env table listed 8 vars; the code reads 12. Added the withdrawal three (GLOBEPAY_WITHDRAWALS_ENABLED, GLOBEPAY_WITHDRAW_NOTIFY_URL, GLOBEPAY_PAYOUT_VERIFY_URL) and GLOBEPAY_CURRENCY. An operator following the table as a deploy checklist would have shipped with payouts silently off. - "Rules to bank now" claimed we allowlist the callback source IP. We do not, and should not: behind DO's load balancer the observed address is not theirs, so an IP gate would reject genuine settlement callbacks and lose money to defend what the RSA signature already covers. GLOBEPAY_CALLBACK_IPS stays as documentation of their egress addresses, deliberately unwired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wiring audit findings that have no code fix in this PR. The GlobePay money paths never write a ledger row. ledger-conservation.spec asserts Sum(ledger rows) == creditSummary().balance, and four sites mutate the balance through raw mutateCreditAtomic: the deposit callback, the deposit sweep, the withdrawal debit and both withdrawal refunds. Only the mock top-up path calls topUpCreditsWithLedger. The invariant holds today purely because the gateway is off; it breaks the moment GLOBEPAY_ENABLED=true. Deposits fit the existing TP type, but withdrawals have no LedgerType at all, which is an Epic-4 spec decision rather than a wiring fix. Written down where the deploy checklist will see it. Also documented NEXT_PUBLIC_WITHDRAWALS_ENABLED and which app each storefront switch belongs on — they are set on polycards-storefront, and each mirrors a backend switch that must be set with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…annot mock CONSERVATION. integration-tests/http/ledger-conservation.spec.ts asserts Sum(ledger rows) == creditSummary().balance. Five GlobePay sites moved the balance through raw mutateCreditAtomic and wrote nothing: the deposit callback, the deposit sweep, the withdrawal debit, and both withdrawal refunds. Only the mock top-up path called topUpCreditsWithLedger, so the invariant held purely because the gateway was off and would have broken on the first real deposit. CI could not catch it: the conservation spec drives the mock gateway. - topUpCreditsWithLedger takes an optional ledgerPaymentMethod/ledgerGatewayRef (defaulting to the mock, so its original caller is untouched). The deposit callback and the sweep now credit through it and label the TP row with the real method (BQR/OB) and their transaction id. Both share one idempotency anchor and refId is the credit's own id, so a callback racing the sweep produces one credit AND one ledger row. - New WD LedgerType for money leaving by payout: nothing represented it before (WP is the weekly challenge payout, RF is period rakeback, AD is an admin adjustment). withdrawCreditsWithLedger pairs the debit (negative) and each refund (positive) with a WD row carrying outcome, bank code, account last-4 (last-4 only: the ledger is a customer-visible surface) and the gateway ref. Migration20260729180000 widens the CHECK constraint, the model enum and the ORM snapshot move with it, and the admin ledger filter/labels gained the tab. - Proven end-to-end, not just at the unit seam: a settled deposit callback now asserts its TP row (real method + gateway ref) and that a retried callback writes exactly one; a status-5 payout failure asserts the WD refund row. NO MOCK IN PRODUCTION. The 'unsafe-demo' escape hatch is gone from mockTopupAllowed, and assertMockTopupSafe now refuses to boot a production server with ALLOW_MOCK_TOPUP set to ANY value, not just 'true'. It existed to run the always-approving mock in production while there was no real gateway; GlobePay365 is that gateway. A value the guard does not recognise must fail loudly rather than boot with a silently disabled top-up path. DEPLOY ORDER (also at the top of the cutover doc): production still carries ALLOW_MOCK_TOPUP=unsafe-demo. It must come off in the SAME spec update that adds the GLOBEPAY_* vars — earlier is a customer-facing outage, later is a failed boot. Docs: a production cutover checklist for the live merchant account — the blockers in dependency order (rotate the leaked password, generate the keypair locally outside the repo, get a static egress IP for their active whitelist), the exact per-app variable list with secrets left to a human, and the verification order ending in one human scan-and-pay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/packages/api/src/api/admin/ledger/route.ts (1)
15-29: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve nullable ledger payloads in the API contract.
ledger_entry.payloadis defined as nullable, butAdminLedgerRow.payloadis typed non-null andasPayload(e.payload)blindly casts database values. Returnnullwhene.payloadis null and type the field aspayload: LedgerPayload | null; add a null-payload case in the admin ledger renderer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/api/admin/ledger/route.ts` around lines 15 - 29, Update AdminLedgerRow.payload and the asPayload read boundary to preserve null database payloads: type the field as LedgerPayload | null and return null when the input is null, otherwise narrow it to LedgerPayload. Add the corresponding null-payload handling in the admin ledger renderer while preserving the existing rendering for non-null payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/packages/api/src/api/store/credits/deposit/__tests__/route.unit.spec.ts`:
- Around line 29-33: Restore the original GLOBEPAY_NOTIFY_URL and
GLOBEPAY_RETURN_URL values after this test suite instead of leaving the
beforeEach assignments in process.env. Add suite-level cleanup around the
existing beforeEach setup, preserving whether each variable was originally unset
or restoring its prior value.
- Around line 64-70: Extend the missing-callback-URL test around POST to also
remove GLOBEPAY_RETURN_URL and assert the same temporary-unavailability
rejection with startMock not called. Ensure the test independently verifies the
route fails closed when either GLOBEPAY_NOTIFY_URL or GLOBEPAY_RETURN_URL is
absent.
In `@docs/payments/globepay365-setup.md`:
- Around line 312-313: Update the credit idempotency guidance to use the signed
MerchantTransactionId rather than TransactionId, and remove the conflicting
recommendation to key credits on TransactionId. Retain the existing
mutateCreditAtomic and topupIdempotencyReference guidance.
- Around line 3-4: Update the status banner in globepay365-setup.md to reflect
the implemented Phase 2 integration and documented staging merchant, removing
the outdated claims that no code exists and no MerchantCode has been issued.
---
Outside diff comments:
In `@backend/packages/api/src/api/admin/ledger/route.ts`:
- Around line 15-29: Update AdminLedgerRow.payload and the asPayload read
boundary to preserve null database payloads: type the field as LedgerPayload |
null and return null when the input is null, otherwise narrow it to
LedgerPayload. Add the corresponding null-payload handling in the admin ledger
renderer while preserving the existing rendering for non-null payloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27df2f7f-d0fb-461d-9652-99277b2079db
📒 Files selected for processing (40)
backend/apps/admin/src/i18n/en.jsonbackend/apps/admin/src/lib/admin-rest.tsbackend/apps/admin/src/lib/queries.tsbackend/apps/admin/src/lib/query-keys.tsbackend/apps/admin/src/routes/deposits/page.tsxbackend/apps/admin/src/routes/ledger/page.tsxbackend/packages/api/.mercur/index.d.tsbackend/packages/api/integration-tests/http/globepay-callback.spec.tsbackend/packages/api/integration-tests/http/globepay-reconcile.spec.tsbackend/packages/api/integration-tests/http/globepay-withdrawal.spec.tsbackend/packages/api/src/api/admin/globepay/deposits/__tests__/list.unit.spec.tsbackend/packages/api/src/api/admin/globepay/deposits/route.tsbackend/packages/api/src/api/admin/ledger/route.tsbackend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/deposit/route.tsbackend/packages/api/src/api/hooks/globepay/payout-verify/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/withdrawal/__tests__/route.unit.spec.tsbackend/packages/api/src/api/hooks/globepay/withdrawal/route.tsbackend/packages/api/src/api/middlewares.tsbackend/packages/api/src/api/store/credits/deposit/__tests__/route.unit.spec.tsbackend/packages/api/src/api/store/credits/deposit/route.tsbackend/packages/api/src/jobs/globepay-reconcile.tsbackend/packages/api/src/jobs/globepay-withdrawal-reconcile.tsbackend/packages/api/src/modules/packs/__tests__/globepay-client.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-deposit.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-withdrawal.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/topup.unit.spec.tsbackend/packages/api/src/modules/packs/feed-events.tsbackend/packages/api/src/modules/packs/globepay-client.tsbackend/packages/api/src/modules/packs/globepay-deposit.tsbackend/packages/api/src/modules/packs/globepay-withdrawal.tsbackend/packages/api/src/modules/packs/ledger.tsbackend/packages/api/src/modules/packs/migrations/.snapshot-packs.jsonbackend/packages/api/src/modules/packs/migrations/Migration20260729180000.tsbackend/packages/api/src/modules/packs/models/ledger-entry.tsbackend/packages/api/src/modules/packs/notify-feed.tsbackend/packages/api/src/modules/packs/service.tsbackend/packages/api/src/modules/packs/topup.tsdocs/payments/globepay365-setup.mdscripts/probe-globepay-callback.mjs
🚧 Files skipped from review as they are similar to previous changes (18)
- backend/packages/api/src/api/hooks/globepay/payout-verify/tests/route.unit.spec.ts
- backend/packages/api/src/api/store/credits/deposit/route.ts
- backend/packages/api/src/jobs/globepay-reconcile.ts
- backend/packages/api/integration-tests/http/globepay-reconcile.spec.ts
- backend/packages/api/src/jobs/globepay-withdrawal-reconcile.ts
- backend/packages/api/src/modules/packs/globepay-client.ts
- backend/packages/api/integration-tests/http/globepay-callback.spec.ts
- backend/packages/api/src/modules/packs/globepay-deposit.ts
- backend/packages/api/src/api/middlewares.ts
- backend/packages/api/src/modules/packs/tests/globepay-client.unit.spec.ts
- backend/packages/api/src/api/hooks/globepay/deposit/route.ts
- backend/packages/api/src/modules/packs/tests/globepay-deposit.unit.spec.ts
- backend/packages/api/src/modules/packs/tests/globepay-withdrawal.unit.spec.ts
- backend/packages/api/integration-tests/http/globepay-withdrawal.spec.ts
- backend/packages/api/src/modules/packs/globepay-withdrawal.ts
- backend/packages/api/src/api/hooks/globepay/withdrawal/tests/route.unit.spec.ts
- backend/packages/api/src/api/hooks/globepay/deposit/tests/route.unit.spec.ts
- backend/packages/api/src/api/hooks/globepay/withdrawal/route.ts
…d with CI check-types caught what my local run missed: I added the ledger assertion after the last tsc pass and only re-ran jest, which does not type-check the fixture object. pendingRow had no payment_method_code, so the new assertion read a property that does not exist on its inferred type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sean confirmed the LIVE account's limits on 2026-07-29, and they are not the test account's: Online Banking (OB) bank-to-bank RM 30 – RM 10,000 QR / e-wallet (BQR) RM 30 – RM 10,000 Payout (WD) RM 50 – RM 50,000 Deposits go RM 30–1,000 -> RM 30–10,000, payouts RM 30–1,000 -> RM 50–50,000. The payout FLOOR is higher than the deposit floor, so GLOBEPAY_WD_MIN_RM no longer mirrors GLOBEPAY_MIN_RM — they are separate bands and the code says so. Their note that amounts over RM 10,000 settle as several bank slips is their internal batching: invisible to us, and it changes neither what we submit nor what a callback reports. The deposit ceiling now coincides exactly with the site-wide TOPUP_MAX_RM, which is checked first, so an over-ceiling top-up is refused with the site's wording and the gateway band effectively guards only the floor. Both checks stay — TOPUP_MAX_RM is our anti-typo guard, the band is theirs, and they are free to move apart again. Specs pin both messages so a change to either number cannot silently let one through. Storefront presets: RM 50 / 250 / 500 / 5,000. The old 30/50/100/200 rungs hugged a RM 1,000 ceiling that no longer exists, and RM 5,000 was impossible under the test account. The sheet's own RM 10,000 validity cap means there is no Pay button at all above the ceiling, which the gateway spec now pins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The RM 50 - RM 50,000 change updated WithdrawForm's constants but not its spec, which still probed 20/1001 against the old RM 30 - RM 1,000 wording. Local 'npm run check' does not run vitest, so CI's quality job caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dedicated egress IPs were applied straight to the live app this afternoon, which is drift: .do/backend.app.yaml is the source of truth and do-apply.ps1 pushes it verbatim, so the next apply would have silently removed the two addresses GlobePay whitelists. The egress block now lives in the committed spec with the addresses and the reason written next to it. While in there, the cutover itself is now committed rather than a list of values to retype: - ALLOW_MOCK_TOPUP is gone from the backend spec (the go-live blocker banner it carried is cleared) and replaced by the GlobePay vars: ENABLED, MERCHANT_CODE=Polycard, the production API host, both callback URLs, and the three secrets as __SECRET__ placeholders. do-apply.ps1 now demands those three like every other secret and still aborts on any unresolved placeholder. - Both key values are documented as the BARE base64 body on one line. toPem() re-wraps them; a multi-line value would break the YAML and the literal substitution. Verified against the real keypair: the bare body signs and verifies, 128-byte signature. - A banner says not to apply the spec until PR #252 merges. It is deliberately wrong for the current code and exactly right after: the boot guard refuses to start production with ALLOW_MOCK_TOPUP set to anything. The storefront switch turned out not to be a variable at all. Next inlines NEXT_PUBLIC_* at build time and App Platform does not reliably pass build-time env, so the root Dockerfile's ARG default is what reaches the bundle - the same note already on NEXT_PUBLIC_MEDUSA_BACKEND_URL. Setting the spec value alone would have left every customer on the mock sheet with the gateway armed behind it. Added ARG/ENV NEXT_PUBLIC_PAYMENTS_PROVIDER defaulting to 'mock', so merging changes nothing, and the cutover is a one-line default change. Docs: the cutover section is now the do-apply flow, and step 4 records that GlobePay's production account enforces its BackOffice whitelist and our address is not on it - the portal answers "Access Denied: Your IP address is not authorized", so they must add both lists themselves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
'doctl apps spec validate' echoes the fully RESOLVED spec, and do-apply piped it straight to the console. -Validate therefore printed the database and Redis connection strings, JWT_SECRET, COOKIE_SECRET, the admin password, both S3 keys, the Google client secret, the Resend key and the PriceCharting token in plaintext. That happened for real on 2026-07-29 and the output reached a chat log, so those values need rotating. Validation output is now captured, not printed. On failure only lines matching error/invalid/failed are surfaced, so a broken spec still tells you what is wrong without dumping the body that holds the secrets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/packages/api/src/modules/packs/globepay-deposit.ts (2)
77-92: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReturn GlobePay’s submitted deposit amount.
StartDepositResult.amountis the value the cashier displays and the reconciler uses, but the current mapping ignoresSubmitDepositResult.depositActualAmountand returns the requestedamount. Returnresult.depositActualAmountinstead so the storefront and reconciliation always use the amount GlobePay accepted.Proposed mapping
return { url: result.url, transactionId: result.transactionId, merchantTransactionId, - amount, + amount: result.depositActualAmount,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/modules/packs/globepay-deposit.ts` around lines 77 - 92, The StartDepositResult mapping must use GlobePay’s accepted deposit amount rather than the requested amount. In the deposit submission flow, update the amount field to return SubmitDepositResult.depositActualAmount from the existing result object, while leaving the other StartDepositResult fields unchanged.
63-66: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftEnsure ambiguous submit retries reuse the same deposit.
newMerchantTransactionId()yields a new reference on every entry intostartGlobePayDeposit. IfsubmitDepositaccepts the gateway request but throws ambiguously, the row stayspending; a retried deposit request can currently create another row with a differentMerchantTransactionIdand gateway transaction, leading to duplicate credits. Require the route to carry a stable submitted deposit idempotency key and have ambiguous retries lookup that row before creating a new deposit or sending anotherMerchantTransactionId.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/packages/api/src/modules/packs/globepay-deposit.ts` around lines 63 - 66, Update startGlobePayDeposit to require and propagate a stable submitted deposit idempotency key from the route. Before creating a new row or calling submitDeposit, look up the existing deposit by that key and reuse its MerchantTransactionId for ambiguous retries. Ensure retries return or continue the existing deposit rather than generating a new value through newMerchantTransactionId or submitting another gateway transaction.
🧹 Nitpick comments (1)
docs/payments/globepay365-setup.md (1)
446-462: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse fenced code blocks for shell commands.
The indented snippets at Lines 448-450, 457, and 461-462 trigger Markdownlint
MD046. Convert them to fenced blocks with appropriate language labels so documentation lint and rendering remain consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/payments/globepay365-setup.md` around lines 446 - 462, Convert each indented shell or PowerShell command snippet in the deployment instructions to fenced code blocks with an appropriate language label, including the three secret assignments, the Get-Content command, and both do-apply commands. Preserve all command text and surrounding explanatory prose.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/packages/api/src/modules/packs/globepay-deposit.ts`:
- Around line 77-92: The StartDepositResult mapping must use GlobePay’s accepted
deposit amount rather than the requested amount. In the deposit submission flow,
update the amount field to return SubmitDepositResult.depositActualAmount from
the existing result object, while leaving the other StartDepositResult fields
unchanged.
- Around line 63-66: Update startGlobePayDeposit to require and propagate a
stable submitted deposit idempotency key from the route. Before creating a new
row or calling submitDeposit, look up the existing deposit by that key and reuse
its MerchantTransactionId for ambiguous retries. Ensure retries return or
continue the existing deposit rather than generating a new value through
newMerchantTransactionId or submitting another gateway transaction.
---
Nitpick comments:
In `@docs/payments/globepay365-setup.md`:
- Around line 446-462: Convert each indented shell or PowerShell command snippet
in the deployment instructions to fenced code blocks with an appropriate
language label, including the three secret assignments, the Get-Content command,
and both do-apply commands. Preserve all command text and surrounding
explanatory prose.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: af40f8d6-3c6b-482f-9425-00f45d7dd213
📒 Files selected for processing (14)
.do/backend.app.yaml.do/storefront.app.yamlDockerfilebackend/packages/api/src/api/hooks/globepay/deposit/__tests__/route.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-deposit.unit.spec.tsbackend/packages/api/src/modules/packs/__tests__/globepay-withdrawal.unit.spec.tsbackend/packages/api/src/modules/packs/globepay-deposit.tsbackend/packages/api/src/modules/packs/globepay-withdrawal.tsdocs/payments/globepay365-setup.mdscripts/do-apply.ps1src/app/bank-withdrawal/WithdrawForm.tsxsrc/app/bank-withdrawal/__tests__/withdraw-form.test.tssrc/components/app-shell/TopUpSheet.tsxsrc/components/app-shell/__tests__/topup-sheet-gateway.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/app/bank-withdrawal/WithdrawForm.tsx
- src/app/bank-withdrawal/tests/withdraw-form.test.ts
- src/components/app-shell/TopUpSheet.tsx
- backend/packages/api/src/modules/packs/tests/globepay-withdrawal.unit.spec.ts
- backend/packages/api/src/api/hooks/globepay/deposit/tests/route.unit.spec.ts
- backend/packages/api/src/modules/packs/tests/globepay-deposit.unit.spec.ts
- backend/packages/api/src/modules/packs/globepay-withdrawal.ts
Customers got an in-app feed row and nothing they could keep. This sends a real receipt to their email at the moment the money lands. Sent from BOTH settlement paths — the signed callback and the reconciliation sweep — inside the same `!replayed` guard the feed row uses, and anchored on the same SIGNED merchant reference. A retried callback, or a callback racing the sweep, therefore produces one credit, one feed row and one receipt. sendTopupReceipt NEVER throws. The credit is already committed when it runs, so no email problem may undo money that landed: a failure is logged and dropped, and the customer still has the balance, the feed row and the admin Deposits row. An account with no email is a skip, not an error. The template states the amount at 2dp (a receipt reading "RM 50" for RM 50.00 looks rounded), the customer-facing method name rather than the gateway's code, the reference support will be asked to quote, and the settlement time in MYT — every customer and every amount here is Malaysian, so UTC would only invite "why does it say yesterday". It fails closed on a missing amount or reference: a receipt that says money moved without saying how much is worse than none. Colours and structure mirror the existing password-reset email (dark, inline styles, table layout for Outlook). The logo is loaded from polycards.gg/branding/polycards-logo.png — white artwork on transparent, verified 255,255,255 over its opaque pixels, so it reads on the dark ground. A URL rather than an inlined data URI because Gmail strips those. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GlobePay confirmed all three setup items on 2026-07-30 (back-office whitelist, API whitelist, merchant public key). The back-office half is independently verified; the public key matches merchant_public_prod.pem byte for byte. But the API whitelist they set is already stale: a do-apply from master (which lacked the egress block) released the dedicated IPs the same morning, and the restore allocated a fresh pair — 188.166.181.61 / 188.166.181.204 — instead of returning the whitelisted ones. Document the wipe, the standing read-back rule, and the corrected cutover order: send the NEW pair before arming deposits. Also mark Phase 1 done and note which deploy secrets still have to come from GlobePay (AES key reissued, their public key). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cy key Review feedback on #252. **The doc contradicted the code, in the dangerous direction.** "Rules to bank now" told the implementer to key credits on the gateway's `TransactionId`, eight lines after the same list explains that only `Data` is covered by the RSA signature. `TransactionId` is top-level and therefore attacker-mutable on a captured callback: one genuine settlement replayed with a fresh value each time mints a fresh credit, because each replay resolves to a different idempotency anchor. The deposit hook already gets this right — it anchors on the signed `Data.MerchantTransactionId` and says why in a comment. Only the doc was wrong, so this corrects the doc and points at the implementation, rather than changing any money path. Also two test gaps in the deposit route spec: - `GLOBEPAY_NOTIFY_URL` / `GLOBEPAY_RETURN_URL` were assigned in `beforeEach` and never restored, so they leaked process-wide and made later suites order-dependent. Captured and restored in `afterEach`. - The fail-closed test only deleted `GLOBEPAY_NOTIFY_URL`, so dropping the `GLOBEPAY_RETURN_URL` guard would not have been caught. Parameterized over both; the new case passes, which confirms the route really does check each. Backend: tsc clean, 1198 unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Review responsesPicked this up to land it — it was 2 commits behind master after #303/#304, now updated. All three open threads addressed in aa37f3d. 🟠 Major — "Use signed 🟡 Minor — env leakage in the deposit spec. Valid. 🟡 Minor — untested return-URL branch. Valid. Parameterized the fail-closed test over both variables. Worth noting the new case passes, which is the actual evidence that the route guards each one independently — a vacuous pass here would have meant the guard was missing. Verification on this branch: backend Remaining before this can ship, unchanged from the PR description: human scan-and-pay, payout-verify, and the prod env vars. 🤖 Generated with Claude Code |
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
2 participants
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.
Full deposit path for the GlobePay365 payment gateway — backend, storefront, and reconciliation. The mock gateway is untouched and stays the local/dev path; this switches on per environment.
Why this shape
Credit becomes asynchronous.
SubmitDepositreturns a cashier URL and no money has moved; credit is issued only when a verified callback reports success. That is not a drop-in swap formockCharge, so it needs state, a webhook route, and a safety net for callbacks that never arrive.The pending table is load-bearing, not bookkeeping: their callback echoes
MerchantTransactionIdbut notMerchantClientId, so without a row written before the gateway call there is no customer to credit. The alternative — encoding our customer id intoMerchantTransactionId— would publish an internal id into their back office and still leave outstanding deposits unrequeryable.What's here
globepay.tsglobepay-client.tsPMT*errorsglobepay-deposit.tsPOST /store/credits/depositPOST /hooks/globepay/depositglobepay-reconcile.ts+ jobTopUpSheetNEXT_PUBLIC_PAYMENTS_PROVIDER=globepayglobepay-withdrawal.tsPOST /store/credits/withdraw(+/banks)POST /hooks/globepay/withdrawalPOST /hooks/globepay/payout-verifyglobepay-withdrawal-reconcilejobWithdrawFormon /bank-withdrawalNEXT_PUBLIC_WITHDRAWALS_ENABLEDMoney rules worth reviewing closely
Datais covered by the signature.TransactionIdis not, so nothing security-relevant derives from it — the idempotency anchor uses the signedMerchantTransactionId. A security review caught the original code anchoring on the unsigned field, which let one captured callback be replayed with varied ids to mint unlimited credit (99fc439f). Regression tests cover it at both unit and integration level."success", including status 7 — a failed deposit is handled, and 400-ing it makes them retry a dead deposit forever. Non-2xx only on bad signature or a transient error, so a genuine credit still lands on retry.Amount— confirmed by the provider 2026-07-22 (NetAmountis amount minus their fees and is not what the customer paid us).Verification
D2026072112415803, and their real signed callback landed on our receiver — signature verified against our uploaded public key (their bytes, not a self-signed test key), AES payload decrypted clean,Status:6→ success, source IP matched their documented staging outbound.globepay.ts, written from their doc, handled their production-format bytes with zero change.tscclean, vitest 291 passed (including 6 new tests pinningTopUpSheet's gateway branch: presets, the RM 30–1,000 band guard, cashier redirect, no-credit copy), lint 0 errors.bigNumberbug was caught:model.bigNumber()is two columns (numeric +raw_*jsonb), and the omission passed every mocked test before failing on the first real insert.CheckBalanceverified live against staging (Testpolycard), exercising IP whitelist, AES, and a signature they verified against our uploaded key. Real deposits were created live.Withdrawals (added 22 July, same day)
The deposit loop's mirror with the money ordering inverted — the debit happens BEFORE SubmitWithdrawal, so money can never be queued to leave the merchant balance while the customer's balance still shows it. Every failure route (gateway refusal, callback status 5, requery says failed, gateway never heard of a stale row) refunds through ONE idempotency anchor: any mix of retries produces exactly one refund. Withdrawals have their own fail-closed switch (
GLOBEPAY_WITHDRAWALS_ENABLED) so deposits can ship live while payouts wait.Verified live against staging on 22 July: GetSupportedBanks returned 31 MYR banks, SubmitWithdrawal was ACCEPTED (
W2026072202370961, RM 30 debited from a real ledger, requery reports processing) — the payout channel turns out to be active. 37 new unit tests + 7 integration tests against a booted server and real Postgres, including refund idempotency under a varied unsigned TransactionId and a late status-5 after settlement (must not pay twice).A pre-merge adversarial review then closed three money-critical gaps (
580f98f6): cashout now routes through walletSummary's withdrawable gate (freeze / locked commissions / playthrough — the withdrawable.ts invariant), ambiguous submit errors no longer refund (a timeout-after-accept would have double-paid; the sweep resolves them via a newGlobePayError.definiteflag), and the sweep never unknown-refunds a row that has a gateway id (a requery 400 there is broken credentials, not non-existence — one bad config deploy would have mass-refunded live payouts). Plus: sweep refunds only when the wd: debit row exists, contradictory final callbacks log as double-payment tripwires, payout-verify rejects currency mismatches. 11 more unit tests pin these.Known gaps — please read before approving
The full loop through a booted Medusa route with a genuine payloadClosed 22 July: the captured genuine callback was replayed against the booted route + real Postgres — signature verified, RM 50 credited to a real ledger row, deposit flipped to settled, and a second replay of the same bytes acked without double-crediting.GLOBEPAY_MYR_METHODSallow-list.The live payout still processingClosed 22 July evening: the provider rejectedW2026072202370961and their REAL withdrawal callback (status 5) hit the booted hook — signature verified, row failed, RM 30 auto-refunded on thewd-refund:anchor, refund notification sent. The full money-out loop is now observed live end-to-end.🤖 Generated with Claude Code
Pre-merge update (23 July)
Merged
origin/masterinto the branch (13 commits: turnover-VIP #254, per-rank challenge rewards #250/#253, QA-gate fixes #244/#251, and others). Overlap files (middlewares.ts,packs/service.ts,schemas.ts) merged clean in disjoint regions; GlobePay wiring untouched. Full local re-verify after the merge: backendtscclean, backend unit 957 passed, storefront vitest + lint + typecheck + build all green.Production rollout checklist (post-merge, in order)
GLOBEPAY_MERCHANT_CODE,GLOBEPAY_MERCHANT_PRIVATE_KEY,GLOBEPAY_PUBLIC_KEY,GLOBEPAY_AES_KEY(production credentials — not the stagingTestpolycardset),GLOBEPAY_API_BASE=https://mapi.GlobePay365.com,GLOBEPAY_NOTIFY_URL,GLOBEPAY_RETURN_URL,GLOBEPAY_WITHDRAW_NOTIFY_URL,GLOBEPAY_PAYOUT_VERIFY_URL, andGLOBEPAY_ENABLED=false(armed later).ALLOW_MOCK_TOPUP=unsafe-demofrom the backend spec (same deploy).NEXT_PUBLIC_PAYMENTS_PROVIDER=globepay(leaveNEXT_PUBLIC_WITHDRAWALS_ENABLEDoff).GLOBEPAY_ENABLED=true, run the monitored RM 30 first-payment smoke test (deposits only, team card, logs watched).GLOBEPAY_WITHDRAWALS_ENABLED+NEXT_PUBLIC_WITHDRAWALS_ENABLED.Summary by CodeRabbit