feat(sdk): configurable top-up bonus multiplier#2894
Conversation
Add an optional per-project top-up bonus for the embeddable Payments SDK: end-users are credited more spend power than they pay (e.g. a 50% bonus turns a $10 top-up into $15). The extra credits are funded by debiting the developer org's credit balance at fulfillment time, capped at the org's available credits so it can never go negative, and can be changed back to 0 (disabled) anytime. - schema: project.endUserTopUpBonusPercent + wallet.bonusPercentOverride; new walletLedger "bonus" and transaction "end_user_bonus" types - session auth resolves bonusPercent (wallet override → project) - top-up route quotes bonusCredited; webhook applies it atomically (live wallets only) alongside the wallet credit + ledger + org debit - projects API validation/serialization + SDK settings UI input + docs Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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:
WalkthroughThis PR adds a configurable end-user top-up bonus, carries it through session and top-up flows, records bonus credits and reversals in Stripe webhook handling, and updates admin metrics and dashboard reporting to track bonus credits separately. ChangesEnd-user top-up bonus feature
Admin bonus metrics reporting
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds a configurable, developer-funded “top-up bonus” for end-user wallet top-ups in the embeddable Payments SDK, extending the existing markup mechanics so projects (and optionally wallets) can credit users more than they paid while debiting the developer org’s credit balance.
Changes:
- Extends DB schema/types to support project-level
endUserTopUpBonusPercent, per-wallet overrides, and new ledger/transaction types (bonus,end_user_bonus). - Updates API to resolve a
bonusPercentfor end-user sessions, include bonus quotes in/wallet/top-up, and apply the bonus onpayment_intent.succeededwith org-credit capping and ledger/transaction writes. - Updates the dashboard SDK settings UI and Payments SDK docs to surface/configure/describe the feature.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/types.ts | Adds "bonus" to the serialized WalletLedger type union. |
| packages/db/src/schema.ts | Adds project/wallet bonus percent fields; adds bonus ledger type and end_user_bonus transaction type. |
| packages/db/migrations/meta/_journal.json | Registers the new DB migration in the migration journal. |
| packages/db/migrations/1783033216_gray_komodo.sql | Adds the new end_user_top_up_bonus_percent and bonus_percent_override columns. |
| apps/ui/src/components/settings/sdk-settings.tsx | Adds “Top-up bonus percent” input and sends it in project settings updates. |
| apps/gateway/src/lib/end-user-session.spec.ts | Updates wallet fixture to include bonusPercentOverride. |
| apps/docs/content/features/embeddable-payments.mdx | Documents the new optional top-up bonus and dashboard setup guidance. |
| apps/api/src/stripe.ts | Applies developer-funded bonus during end-user top-up fulfillment; records ledger + org transaction + org credits debit. |
| apps/api/src/routes/projects.ts | Adds validation/serialization for endUserTopUpBonusPercent in project routes. |
| apps/api/src/routes/platform-wallet.ts | Computes/returns bonusCredited quote and stamps bonus fields into PaymentIntent metadata. |
| apps/api/src/routes/organization.ts | Extends org route schemas for the new project and transaction fields/types. |
| apps/api/src/lib/end-user-session-auth.ts | Resolves bonusPercent from wallet override or project default into the authenticated end-user session context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| step={0.01} | ||
| value={bonusPercent} | ||
| disabled={isPreview} | ||
| onChange={(event) => setBonusPercent(Number(event.target.value))} |
| logger.info( | ||
| `Credited ${netCredited} to end-user wallet ${walletId} (margin ${developerMargin}, platform fee ${platformFee})`, | ||
| `Credited ${netCredited} to end-user wallet ${walletId} (margin ${developerMargin}, platform fee ${platformFee}, bonus quote ${bonusCredited}, balance now ${newBalance})`, | ||
| ); |
| // Developer-funded bonus: credit the wallet extra spend power on top of | ||
| // the paid amount, debited from the developer org's credit balance. Only | ||
| // for live wallets, and capped at the org's available credits so the | ||
| // balance can never go negative. The org row is locked (SELECT … FOR | ||
| // UPDATE) so a concurrent debit can't oversell the balance. |
| - Your **secret key** (`sk_…`) never leaves your backend. It mints short-lived **ephemeral session tokens** (`es_…`) scoped to one end-user wallet. | ||
| - The **browser** only ever holds the `es_…` token (and a publishable Stripe key). It calls the gateway directly; usage is billed to that user's wallet. | ||
| - **Markup is applied at top-up time**: if you set a 20% markup and a user buys $10, their wallet is credited the net spend power and your **margin accrues to your organization** for later payout. | ||
| - **Top-up bonus (optional)**: set a bonus percent to credit end-users _more_ than they pay — e.g. a 50% bonus turns a $10 top-up into $15 of spend power. The extra credits are funded from **your organization's credit balance** at top-up time (capped at your available credits), so it's a promotional lever you can switch on or off anytime. Markup and bonus are independent: the bonus is applied on top of the net credited amount. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa9542ffb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [org] = await tx | ||
| .select({ credits: tables.organization.credits }) | ||
| .from(tables.organization) | ||
| .where(eq(tables.organization.id, wallet.organizationId)) | ||
| .for("update") |
There was a problem hiding this comment.
When bonusCredited > 0, this takes the organization FOR UPDATE lock after the transaction has already updated the wallet row above. The worker's credit-deduction transaction can do the inverse for batches that include both this org's regular credit usage and this end-user wallet (apps/worker/src/worker.ts:1229 updates the organization, then apps/worker/src/worker.ts:1276 updates the wallet), so a concurrent bonus top-up can deadlock and abort one side. Keep the lock order consistent, e.g. lock/debit the org before touching the wallet.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/api/src/routes/projects.ts (1)
19-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared
projectSchemato avoid drift.This
projectSchemais duplicated almost verbatim inapps/api/src/routes/organization.ts(lines 97-114). This PR had to update both copies in lockstep for the newendUserTopUpBonusPercentfield; a shared schema module would remove that duplication and the risk of the two drifting apart in the future.🤖 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 `@apps/api/src/routes/projects.ts` around lines 19 - 36, The `projectSchema` definition is duplicated in `projects.ts` and `organization.ts`, which creates drift risk whenever fields like `endUserTopUpBonusPercent` change. Extract the shared `projectSchema` into a common module and import it from both routes, keeping the schema shape in one place while preserving the existing `projectSchema` symbol usage in the route handlers.
🤖 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 `@apps/api/src/stripe.ts`:
- Around line 2044-2046: The bonus application in the Stripe credits flow can
round up past the available balance and make org credits negative; update the
`bonusToApply` calculation in `stripe.ts` so it never exceeds
`availableCredits`. In the logic around the `availableCredits` and
`bonusToApply` variables, replace the rounding approach with a floor-based clamp
so the computed bonus is always less than or equal to the current balance before
`credits - bonusToApply` is applied. Keep the invariant consistent in the same
bonus-crediting path by ensuring the final subtraction cannot underflow.
- Around line 2048-2082: The refund path in handleEndUserTopUpRefunded only
reverses the paid top-up amount, leaving the developer-funded bonus created in
the bonus application block unchanged. Update the refund logic to also claw back
the bonus recorded in tables.walletLedger / tables.transaction for the
end_user_bonus entry, or otherwise ensure that bonus credit becomes
non-spendable once the refunded payment intent is processed. Use the existing
wallet, organization, and paymentIntent handling in apps/api/src/stripe.ts to
locate the bonus application and mirror it in the refund flow.
---
Nitpick comments:
In `@apps/api/src/routes/projects.ts`:
- Around line 19-36: The `projectSchema` definition is duplicated in
`projects.ts` and `organization.ts`, which creates drift risk whenever fields
like `endUserTopUpBonusPercent` change. Extract the shared `projectSchema` into
a common module and import it from both routes, keeping the schema shape in one
place while preserving the existing `projectSchema` symbol usage in the route
handlers.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50afb24d-83ff-4021-a065-f4c0c4cd9080
📒 Files selected for processing (13)
apps/api/src/lib/end-user-session-auth.tsapps/api/src/routes/organization.tsapps/api/src/routes/platform-wallet.tsapps/api/src/routes/projects.tsapps/api/src/stripe.tsapps/docs/content/features/embeddable-payments.mdxapps/gateway/src/lib/end-user-session.spec.tsapps/ui/src/components/settings/sdk-settings.tsxpackages/db/migrations/1783033216_gray_komodo.sqlpackages/db/migrations/meta/1783033216_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.tspackages/db/src/types.ts
- refund now claws back the developer-funded bonus and restores the org's credits, so a refunded top-up can't leave gifted spend power in the wallet - lock/debit the org before the wallet on bonus apply + claw-back, matching the worker's org→wallet order to avoid a deadlock with usage debits - clamp the bonus with Math.floor so org credits can never round negative - include actual bonusCredited + totalCredited in the wallet.credited webhook - add DB-backed tests: bonus applied, capped by credits, test-mode skipped, refund claw-back - docs: note the bonus in the Buying credits section Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review feedback in Fixed (correctness / money movement)
Fixed (accuracy / docs)
Tests (Copilot): added Intentionally skipped
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad0e6dac82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const topupReversed = Math.min(credited, currentBalance); | ||
| const bonusReversed = | ||
| Math.floor( | ||
| Math.min(bonusOriginal, currentBalance - topupReversed) * 1e6, | ||
| ) / 1e6; |
There was a problem hiding this comment.
When a bonus top-up is partially spent before the Stripe refund, this allocation under-restores the developer’s org credits. For example, a $10 top-up with a $5 bonus that is spent down to a $5 wallet balance will set topupReversed to 5 and bonusReversed to 0, then debit the wallet to zero without returning any of the clawed-back bonus to the org. Since the user’s paid amount is already being refunded by Stripe, the remaining wallet balance should restore the org-funded bonus before consuming the paid top-up portion; otherwise refunded top-ups can permanently burn developer credits even when the bonus balance was removed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: def9ea1c44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const bonusReversed = | ||
| Math.floor( | ||
| Math.min(bonusOriginal, currentBalance - topupReversed) * 1e6, | ||
| ) / 1e6; |
There was a problem hiding this comment.
When only part of the Stripe charge is refunded, handleChargeRefunded finds the original top-up row and returns before loading the individual refund amount, so this function cannot distinguish a partial refund from a full one. With an unspent $10 top-up plus a $5 bonus, a $1 refund still makes bonusReversed equal 5 and restores the entire promotional bonus to the org, over-debiting the wallet for a partial refund. Pass the refund amount into the end-user path and cap/prorate the bonus reversal to that amount.
Useful? React with 👍 / 👎.
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 (1)
apps/api/src/stripe.ts (1)
2166-2170: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftHandle partial Stripe refunds before reversing the wallet balance.
charge.refundedcan represent a partial refund, but this path dedupes bystripePaymentIntentIdand Line 2228 reverses against the full originalnetCredited. A $1 partial refund on a $10 top-up with a $5 bonus can therefore reverse up to $15 and mark the whole PaymentIntent reversed, preventing later partial refunds from being reconciled. Pass the latest Stripe refund amount/id into this handler and make the top-up/bonus reversal proportional/idempotent per refund, similar to the generic refund branch below that fetcheslatestRefund.Also applies to: 2225-2233
🤖 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 `@apps/api/src/stripe.ts` around lines 2166 - 2170, The `charge.refunded` path in `apps/api/src/stripe.ts` is treating all refunds for a `stripePaymentIntentId` as one full reversal, which breaks partial refunds. Update the handler around the `alreadyReversed` check and the `netCredited` reversal logic to accept the latest Stripe refund id/amount, then make the wallet and bonus reversal proportional and idempotent per refund instead of reversing the full top-up once. Mirror the `latestRefund` handling used in the generic refund branch so each refund can be reconciled independently.
🧹 Nitpick comments (1)
apps/api/src/stripe-end-user-bonus.spec.ts (1)
189-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a partial-refund regression case.
This only exercises a full refund with the full wallet balance still present. Add a case for a partial Stripe refund once the handler accepts refund amount/id, so the suite catches over-reversal and duplicate suppression across multiple partial refunds.
🤖 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 `@apps/api/src/stripe-end-user-bonus.spec.ts` around lines 189 - 221, Add a regression test in stripe-end-user-bonus.spec.ts that covers partial Stripe refunds once handleEndUserTopUpRefunded accepts refund amount/id metadata, not just the full-refund path. Use the existing helpers seedWallet, handleEndUserTopUpSucceeded, getWalletBalance, and getOrgCredits to verify a partial refund only reverses the proportional top-up/bonus amounts and does not over-credit the org. Also add a follow-up duplicate refund call for the same refund id to confirm the handler suppresses double-processing across multiple partial refunds.
🤖 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 `@apps/api/src/stripe.ts`:
- Around line 2166-2170: The `charge.refunded` path in `apps/api/src/stripe.ts`
is treating all refunds for a `stripePaymentIntentId` as one full reversal,
which breaks partial refunds. Update the handler around the `alreadyReversed`
check and the `netCredited` reversal logic to accept the latest Stripe refund
id/amount, then make the wallet and bonus reversal proportional and idempotent
per refund instead of reversing the full top-up once. Mirror the `latestRefund`
handling used in the generic refund branch so each refund can be reconciled
independently.
---
Nitpick comments:
In `@apps/api/src/stripe-end-user-bonus.spec.ts`:
- Around line 189-221: Add a regression test in stripe-end-user-bonus.spec.ts
that covers partial Stripe refunds once handleEndUserTopUpRefunded accepts
refund amount/id metadata, not just the full-refund path. Use the existing
helpers seedWallet, handleEndUserTopUpSucceeded, getWalletBalance, and
getOrgCredits to verify a partial refund only reverses the proportional
top-up/bonus amounts and does not over-credit the org. Also add a follow-up
duplicate refund call for the same refund id to confirm the handler suppresses
double-processing across multiple partial refunds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eb8dd2be-f502-4553-ad01-b1b97ee5e1c4
📒 Files selected for processing (9)
apps/api/src/routes/organization.tsapps/api/src/stripe-end-user-bonus.spec.tsapps/api/src/stripe.tsapps/docs/content/features/embeddable-payments.mdxpackages/db/migrations/1783098602_regular_mad_thinker.sqlpackages/db/migrations/meta/1783098602_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/schema.tspackages/db/src/types.ts
💤 Files with no reviewable changes (1)
- packages/db/migrations/1783098602_regular_mad_thinker.sql
✅ Files skipped from review due to trivial changes (2)
- packages/db/src/types.ts
- apps/docs/content/features/embeddable-payments.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/db/migrations/meta/_journal.json
- apps/api/src/routes/organization.ts
- packages/db/src/schema.ts
The admin /metrics + /metrics/timeseries revenue/processed/topped-up sums used a denylist (!= credit_gift, not-devpass), which swept in every SDK end-user wallet transaction. The new end_user_bonus rows carry a negative creditAmount, so a developer-funded bonus would have *reduced* reported revenue; the end_user_margin_* rows (developer liability, not org credit purchases) were also inflating it. - add notEndUserWalletFilter (end_user_topup/margin_accrual/refund/ margin_payout/bonus) and apply it to all 7 revenue/processed/topped-up aggregations in /metrics and /metrics/timeseries - add totalBonusCredits metric (net bonus credits granted to end-user wallets, refunds netted out) so it's recorded and subtractable in stats, mirroring totalGiftedCredits; surface it in the admin Credit Flow card - test: admin /metrics excludes bonus/margin/gift from revenue and reports bonus separately Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abff00605c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const bonusFraction = | ||
| session.mode === "test" ? 0 : session.bonusPercent / 100; | ||
| const bonusCredited = Math.round(netCredited * bonusFraction * 1e6) / 1e6; |
There was a problem hiding this comment.
When the developer org already has fewer credits than the configured bonus, this returns and stores the full bonusCredited amount even though the success handler later caps the actual bonus to the org's available balance. In that scenario the payment UI can show (for example) a $15 credit total for a $10 top-up with a 50% bonus, but after the user pays the wallet may receive only $10 because the org had no credits left. Cap the quoted bonus against the current org balance, or make the response explicitly an estimate so the checkout does not promise spend power that fulfillment will not apply.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/src/routes/admin.ts (1)
737-760: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNet bonus figure can go negative for narrow date windows.
totalBonusCreditsnets grants (negativecreditAmount) against clawbacks (positivecreditAmount) withintransactionDateFilter. If a selected range only captures a refund clawback whose original grant falls outside the window, this yields a negative value, which the UI would render as e.g. "-$5.00" under "SDK bonus" — technically accurate as a net-change figure but potentially confusing without a supporting explanation in the UI/tooltip.💡 Optional: clarify via comment or floor for all-time-only display
// Total developer-funded end-user top-up bonus credits granted (net of // refund claw-backs). end_user_bonus rows store creditAmount as the change to // the developer org's credit balance — negative when a bonus is granted, // positive when clawed back on refund — so negate the sum to report the net // credits actually gifted into end-user wallets. Excluded from revenue above // and surfaced here so it can be subtracted/considered in stats separately. + // Note: for narrow date ranges this can be negative if a clawback's window + // excludes the original grant date. const [bonusRow] = await db🤖 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 `@apps/api/src/routes/admin.ts` around lines 737 - 760, The `totalBonusCredits` calculation in `admin.ts` can become negative for narrow `transactionDateFilter` ranges because it inverts the summed `end_user_bonus` `creditAmount` values, including clawbacks without the original grant. Update the logic around the `bonusRow` query / `totalBonusCredits` assignment to either clamp the displayed metric to zero when it is meant to represent only gifted credits, or explicitly document in the UI-facing stats path that this is a net-change figure that may go negative. Use the existing `transactionDateFilter`, `tables.transaction.type === "end_user_bonus"`, and `totalBonusCredits` symbols to locate the computation.
🤖 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.
Nitpick comments:
In `@apps/api/src/routes/admin.ts`:
- Around line 737-760: The `totalBonusCredits` calculation in `admin.ts` can
become negative for narrow `transactionDateFilter` ranges because it inverts the
summed `end_user_bonus` `creditAmount` values, including clawbacks without the
original grant. Update the logic around the `bonusRow` query /
`totalBonusCredits` assignment to either clamp the displayed metric to zero when
it is meant to represent only gifted credits, or explicitly document in the
UI-facing stats path that this is a net-change figure that may go negative. Use
the existing `transactionDateFilter`, `tables.transaction.type ===
"end_user_bonus"`, and `totalBonusCredits` symbols to locate the computation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2d9c2734-8e0d-4f75-96eb-ae88591a32c3
📒 Files selected for processing (4)
apps/api/src/routes/admin-metrics-bonus.spec.tsapps/api/src/routes/admin.tsapps/api/src/utils/devpass-filter.tsee/admin/src/app/page.tsx
Verify the end-user top-up bonus end to end and give a local way to try it. - seed a "Payments SDK POC" org/project (50% bonus, live platform secret, $100 org credits) so the flow can be exercised locally / in the UI - packages/scripts/poc-end-user-bonus: mint session → top-up → confirm the Stripe PaymentIntent → poll + print wallet ledger, org credits, and transactions, then refund and re-print (run against a dev stack built from this branch with `stripe listen` forwarding webhooks) - apps/api end-user-bonus-flow.spec: real Stripe test-mode round trip (create+confirm PaymentIntent, refund) driving the actual webhook handlers and admin /metrics. Asserts wallet 10+5 bonus, org credits 100→95→100, reversal of the full 15, bonus grant/claw-back netting to 0, and admin revenue unmoved by the SDK flow. Gated on a Stripe test key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/routes/end-user-bonus-flow.spec.ts (1)
115-228: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider an explicit timeout for this real-Stripe round-trip test.
The test chains several real network calls (local API + Stripe confirm/retrieve/refund/retrieve); the default 5s vitest timeout may be tight and cause intermittent flakiness.
⏱️ Proposed fix
- test("purchase gets +50% bonus; refund reverses it; revenue stays put", async () => { + test("purchase gets +50% bonus; refund reverses it; revenue stays put", async () => { ... - }); + }, 30_000);🤖 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 `@apps/api/src/routes/end-user-bonus-flow.spec.ts` around lines 115 - 228, The end-user bonus flow test is doing multiple real API and Stripe calls and may exceed Vitest’s default timeout. Update the specific test in end-user-bonus-flow.spec.ts (the “purchase gets +50% bonus; refund reverses it; revenue stays put” case) to use an explicit longer timeout, so the real Stripe round-trip has enough time to complete without flaking.
🤖 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 `@apps/api/src/routes/end-user-bonus-flow.spec.ts`:
- Around line 47-51: The test setup in end-user-bonus-flow.spec.ts mutates
process.env.ADMIN_EMAILS in beforeEach without restoring it, so the value leaks
into later tests in the same worker. Update the relevant beforeEach/afterEach
pair around createTestUser and deleteAll() to save the previous ADMIN_EMAILS
value, restore it after each test, and handle the case where it was originally
unset.
---
Nitpick comments:
In `@apps/api/src/routes/end-user-bonus-flow.spec.ts`:
- Around line 115-228: The end-user bonus flow test is doing multiple real API
and Stripe calls and may exceed Vitest’s default timeout. Update the specific
test in end-user-bonus-flow.spec.ts (the “purchase gets +50% bonus; refund
reverses it; revenue stays put” case) to use an explicit longer timeout, so the
real Stripe round-trip has enough time to complete without flaking.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 01188343-bf8c-45c8-98a8-e55c4cb56607
📒 Files selected for processing (4)
apps/api/src/routes/end-user-bonus-flow.spec.tspackages/db/src/seed.tspackages/scripts/package.jsonpackages/scripts/src/poc-end-user-bonus.ts
| beforeEach(async () => { | ||
| process.env.ADMIN_EMAILS = "admin@example.com"; | ||
| // Seeds admin@example.com + returns an admin session cookie (also wipes DB). | ||
| adminCookie = await createTestUser(); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
ADMIN_EMAILS is set but never restored.
process.env.ADMIN_EMAILS is mutated in beforeEach but afterEach only calls deleteAll(); the env var leaks into subsequent tests running in the same worker process.
🧹 Proposed fix
afterEach(async () => {
+ delete process.env.ADMIN_EMAILS;
await deleteAll();
});Also applies to: 89-91
🤖 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 `@apps/api/src/routes/end-user-bonus-flow.spec.ts` around lines 47 - 51, The
test setup in end-user-bonus-flow.spec.ts mutates process.env.ADMIN_EMAILS in
beforeEach without restoring it, so the value leaks into later tests in the same
worker. Update the relevant beforeEach/afterEach pair around createTestUser and
deleteAll() to save the previous ADMIN_EMAILS value, restore it after each test,
and handle the case where it was originally unset.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d001e3a01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await tx.insert(tables.transaction).values({ | ||
| organizationId: wallet.organizationId, | ||
| type: "end_user_bonus", | ||
| amount: String(bonusApplied), |
There was a problem hiding this comment.
When a live top-up has a bonus, this stores the internal developer-funded grant with a positive amount. GET /orgs/{id}/transactions returns these rows, and downloadTransactionInvoice gates invoices through isInvoiceableTransaction, which only checks for completed plus a positive amount; as a result a bonus grant, and the claw-back row below with the same pattern, can be downloaded/displayed as a paid invoice even though no customer paid LLM Gateway for it. Store these internal transfers with no payable amount, or make invoiceability type-aware.
Useful? React with 👍 / 👎.
The end-user's actual payment is real revenue and should be reported; only the developer-funded bonus (a cost) and the pass-through developer margin (a liability) should be excluded. The previous change wrongly excluded the whole end-user family. - record an `end_user_topup` transaction on top-up success (amount = gross Stripe, creditAmount = net credit value), live wallets only, and a negative `end_user_topup` reversal on refund (full payment reversed) - split the admin filters: revenue/processed queries exclude only the non-revenue end-user rows (margin accrual/payout/refund claw-back + bonus) via notEndUserNonRevenueFilter, so end_user_topup counts; topped-up keeps the full end-user exclusion (separate wallet economy) - so a $10 top-up with a $5 bonus reads as +$10 revenue (not +$30 or +$15), and nets back to baseline after a refund; bonus stays a separate metric - update the real-Stripe flow spec (revenue $20 -> $30 -> $20) and the admin metrics spec (top-up counted, bonus/margin excluded) Co-Authored-By: Claude Opus 4.8 (1M context) <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 (1)
apps/api/src/stripe.ts (1)
1937-1937: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNormalize bonus metadata before debiting credits.
bonusCreditedis untrusted metadata; values likeInfinityor1e309pass> 0and can make the clamp spend the full org balance. Coerce non-finite values to0before entering the transaction.🛡️ Proposed fix
- const bonusCredited = Number(md.bonusCredited ?? "0"); + const requestedBonusCredited = Number(md.bonusCredited ?? "0"); + const bonusCredited = + Number.isFinite(requestedBonusCredited) && requestedBonusCredited > 0 + ? requestedBonusCredited + : 0;🤖 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 `@apps/api/src/stripe.ts` at line 1937, The bonus metadata handling is trusting unvalidated input in the credit-debit flow. In the logic that reads bonusCredited from md, normalize it before the transaction by treating any non-finite value as 0, then use only the sanitized value in the downstream clamp/spend calculation so values like Infinity or 1e309 cannot affect the org balance.
🤖 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 `@apps/api/src/stripe.ts`:
- Line 1937: The bonus metadata handling is trusting unvalidated input in the
credit-debit flow. In the logic that reads bonusCredited from md, normalize it
before the transaction by treating any non-finite value as 0, then use only the
sanitized value in the downstream clamp/spend calculation so values like
Infinity or 1e309 cannot affect the org balance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9cc67795-7ff7-4275-aff1-47caf6a6f192
📒 Files selected for processing (6)
apps/api/src/routes/admin-metrics-bonus.spec.tsapps/api/src/routes/admin.tsapps/api/src/routes/end-user-bonus-flow.spec.tsapps/api/src/stripe.tsapps/api/src/utils/devpass-filter.tspackages/scripts/src/poc-end-user-bonus.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/routes/admin-metrics-bonus.spec.ts
- packages/scripts/src/poc-end-user-bonus.ts
- apps/api/src/routes/end-user-bonus-flow.spec.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c8580c07f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type: "end_user_topup", | ||
| amount: String(-grossReversed), | ||
| creditAmount: String(-revenueReversed), |
There was a problem hiding this comment.
When a live end-user top-up is refunded, this writes the reversal as a negative end_user_topup instead of a refund row/type. I checked the admin metrics paths in apps/api/src/routes/admin.ts: both totalRefunds and refundsPerDay only sum credit_refund, so a refunded SDK top-up reduces revenue/processed but leaves the dashboard's Refunds metric and refund series at $0, unlike ordinary credit top-up refunds. Use a refund transaction type or include these negative reversal rows in the refund aggregations.
Useful? React with 👍 / 👎.
Add a "Credits Given" dashboard card that sums gifted credits and end-user top-up bonus credits into one net figure, with the per-source breakdown below it. Previously the two were only shown as separate stats with no combined total. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00c6001833
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type: "bonus", | ||
| amount: String(bonusApplied), | ||
| balanceAfter: bonusUpdated.balance, |
There was a problem hiding this comment.
When a top-up applies a bonus, this new bonus ledger row increases the wallet balance, but the customer analytics aggregate in apps/api/src/routes/platform-customers.ts only sums type = 'topup'/netCredited for lifetimeToppedUp. In the enabled-bonus scenario, a $10 top-up with a $5 bonus leaves the wallet at $15 while both customer list/detail report only $10 topped up, so developer-facing wallet analytics understate credited spend power unless bonus rows are included or reported separately.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
What
Adds an optional, per-project top-up bonus to the embeddable Payments SDK. The developer who sets up the SDK can configure a bonus multiplier so their end-users receive more spend power than they pay — e.g. a 50% bonus turns a $10 top-up into $15 of wallet credit. The extra credit is funded from the developer org's own credit balance, and the bonus can be set back to
0(disabled) at any time.This is the inverse of the existing markup (which credits less than paid and accrues developer margin); the two are independent and can be combined.
How it works
project.endUserTopUpBonusPercent(default"0", 0–1000%) plus an optional per-walletwallet.bonusPercentOverride, mirroring the markup pattern. Editable from Settings → Payments SDK in the dashboard./wallet/top-uproute computesbonusCredited = netCredited × bonus%and returns it (so the widget can show the boosted total) and stamps it into the PaymentIntent metadata. Test-mode (sandbox) wallets never earn a bonus.handleEndUserTopUpSucceeded): onpayment_intent.succeeded, inside the same atomic transaction that credits the wallet, the bonus is applied for live wallets only:SELECT … FOR UPDATE) and the bonus is capped at the org's available credits, so the balance can never go negative (a shortfall logs a warning and skips/reduces the bonus);bonusledger row;creditsare debited and anend_user_bonustransaction is recorded.Changes
packages/db: schema (endUserTopUpBonusPercent,bonusPercentOverride,walletLedgerbonustype,transactionend_user_bonustype) + migration; serializedWalletLedgerunion.apps/api: session-auth resolvesbonusPercent; top-up route quote + metadata; webhook fulfillment;projectsvalidation/serialization +organizationtransaction/project schemas.apps/ui: "Top-up bonus percent" input in SDK settings.apps/docs: Payments SDK feature docs.Testing
pnpm build— full monorepo build passes (17/17).pnpm format/ lint-staged clean.apps/gateway/src/lib/end-user-session.spec.tspasses.🤖 Generated with Claude Code
Summary by CodeRabbit