feat(billing): packs de crédits Stripe (Checkout, ledger, gate génération)#23
Conversation
Le webhook Stripe crédite de façon asynchrone (1–3 s après le retour de Checkout). L'ancien unique refresh à 1,5 s manquait le solde si le webhook arrivait plus tard. On mémorise le solde avant la redirection (localStorage, survit au rechargement de page) et on relit le solde avec backoff jusqu'à ce qu'il dépasse la référence, avec toast de confirmation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion) Introduit le billing par packs de crédits one-shot (Stripe Checkout hébergé), sans abonnement. Le gating de génération passe du quota mensuel aux crédits. - Ledger append-only (credits.ts) : solde = somme des deltas, invariant « jamais déficitaire », idempotence par idempotencyKey (FREE_GRANT unique, PURCHASE = session id, GENERATION = -1 transactionnel). - API billing : checkout (session hébergée, prix résolu par lookup_key), webhook signé (crédite sur checkout.session.completed payé), summary (solde + octroi gratuit idempotent). - Client Stripe serveur (stripe.ts) + seed idempotent (scripts/stripe-seed.ts) créant produits/prix depuis config/pricing.ts. - Schéma Prisma + migration : table credit_ledger, stripeCustomerId. - Génération : gate solde >= 1 (consumeOneCredit), remplace le quota mensuel generation ; l'UI candidature et la nav basculent sur le solde de crédits. - Types partagés (@cvo/shared/billing) + tests (pricing, usage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for monumental-speculoos-a69398 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (21)
✨ Finishing Touches📝 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
apps/app/prisma/schema.prisma (1)
377-395: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider a foreign key from
CreditLedger.userIdtoUser.id.
userIdis a plain scalar with no@relation/FK toUser, so the DB won't enforce that ledger rows reference a real user. Given this is a financial audit trail, a non-cascading FK (e.g.onDelete: Restrict) would catch bugs/typos at write time while still preserving the append-only ledger (no cascade needed since users are soft-deleted viadeletedAt).♻️ Proposed addition
model CreditLedger { id String `@id` `@default`(uuid()) userId String + user User `@relation`(fields: [userId], references: [id], onDelete: Restrict) delta IntAdd the reciprocal side on
User:profile Profile? sessions Session[] accounts Account[] + creditLedger CreditLedger[]🤖 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/app/prisma/schema.prisma` around lines 377 - 395, Add a real Prisma relation for CreditLedger.userId so ledger rows must reference an existing User.id; update the CreditLedger model to declare the foreign key relation and add the reciprocal relation field on User. Use a non-cascading delete behavior such as Restrict to preserve the append-only audit trail, and keep the existing uniqueness/indexing intact.apps/app/pages/candidature.vue (1)
37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
refreshUsageto reflect billing, not usage.The fetch now targets
BILLING_SUMMARY_PATH/BillingSummary, but the destructured refresh function is still namedrefreshUsage(also called at Lines 160 and 169). Purely cosmetic but confusing given the PR's usage→credits migration.🤖 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/app/pages/candidature.vue` around lines 37 - 40, The destructured refresh function in useFetch for BillingSummary still uses the old usage-oriented name, which is misleading. Rename refreshUsage in the candidature.vue setup to a billing-specific name and update the corresponding calls in the same component so the identifier matches BILLING_SUMMARY_PATH/BillingSummary and the credits-oriented migration.apps/app/server/api/billing/checkout.post.ts (2)
40-50: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRace condition can create duplicate Stripe customers.
Two concurrent checkout requests from the same (new) user will both read
stripeCustomerId: null, each callstripe.customers.create(), and each overwrite the DB field — leaving an orphaned Stripe customer and non-deterministic finalcustomerId. Consider a conditional update (updateMany({ where: { id: userId, stripeCustomerId: null }, ... })) and re-reading the DB if the update affected 0 rows, or passing an idempotency key derived fromuserIdtocustomers.create.🤖 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/app/server/api/billing/checkout.post.ts` around lines 40 - 50, The checkout flow in checkout.post.ts can create duplicate Stripe customers when two requests race through the customer creation path in the checkout handler. Update the logic around the existing customerId / stripe.customers.create block to make it idempotent, either by using a userId-derived idempotency key on customers.create or by doing a conditional prisma.user.update/updateMany on stripeCustomerId being null and re-reading the stored value if another request already set it. Ensure the checkout path always reuses the persisted Stripe customer for the same user.
43-48: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd idempotency keys to both Stripe writes. A retry can create duplicate customers or checkout sessions. Pass a stable
idempotencyKeyin the request options forstripe.customers.create(...)andstripe.checkout.sessions.create(...)so retries reuse the same Stripe object.🤖 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/app/server/api/billing/checkout.post.ts` around lines 43 - 48, The Stripe writes in the checkout flow are not idempotent, so retries can create duplicate customer or session objects. Update the `checkout.post.ts` flow in the `stripe.customers.create(...)` and `stripe.checkout.sessions.create(...)` calls to pass a stable `idempotencyKey` in their request options, using the same retry-safe key derived from the current user/checkout request so repeated attempts reuse the same Stripe object.apps/app/server/api/billing/webhook.post.ts (1)
15-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMissing test coverage for a financial-correctness-critical endpoint.
This endpoint credits real purchases and has no accompanying spec in the diff. Given signature verification and idempotent crediting are both security/financial-sensitive, unit/integration tests (mocking
stripe.webhooks.constructEvent) would materially reduce regression risk.🤖 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/app/server/api/billing/webhook.post.ts` around lines 15 - 51, The webhook handler in defineEventHandler for billing/webhook.post.ts needs test coverage for its security- and money-sensitive behavior. Add unit/integration tests around the Stripe webhook flow by mocking getStripe().webhooks.constructEvent and covering valid checkout.session.completed processing, invalid signature handling, missing secret/configured-secret cases, and the idempotent grantPurchasedCredits path keyed by session.id. Use the existing handler logic and symbols like StripeNotConfiguredError and grantPurchasedCredits to locate and exercise the critical branches.apps/app/pages/credits.vue (1)
27-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFallback
beforevalue can mask an already-credited balance.At Line 42, if
CREDITS_BEFORE_KEYis missing fromlocalStorage(cleared, different device, direct navigation to?status=success),beforefalls back to the currentcreditBalance.value. If the webhook already credited the account by the time the page mounts,beforewill equal the already-updated balance, sowaitForCredit'snow > previouscheck never triggers, and the user incorrectly sees "Crédits en cours" (Line 49) despite credits already being applied. Low impact since the balance is correct on next load/refresh, but the toast is misleading.♻️ Possible mitigation
- const before = Number(localStorage.getItem(CREDITS_BEFORE_KEY) ?? creditBalance.value ?? 0) + const storedBefore = localStorage.getItem(CREDITS_BEFORE_KEY) + const before = storedBefore !== null ? Number(storedBefore) : -Infinity🤖 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/app/pages/credits.vue` around lines 27 - 50, The fallback used to compute the `before` value in `credits.vue` can hide an already-updated balance when `CREDITS_BEFORE_KEY` is missing. Update the `onMounted` success flow so `before` is derived from a stable pre-checkout baseline only, or explicitly detect the missing localStorage case and avoid comparing against `creditBalance.value` from the same render. Keep the `waitForCredit` helper and its `now > previous` check, but ensure the `toast.info('Crédits en cours', ...)` path is only used when the balance truly has not changed.apps/app/server/api/candidature/generate.post.ts (1)
65-66: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
ensureFreeGrantre-attempts a failing INSERT on every single generation request, forever.Per
credits.ts,getOrInitBalanceunconditionally callsensureFreeGrant, which Octroie les crédits gratuits une seule fois par utilisateur (idempotent via la contrainte unique suridempotencyKey) by attempting anINSERTand catching theP2002unique-violation once it's already been granted. Since this gate runs on every generation call (andconsumeOneCreditcallsensureFreeGranta second time on the same request), every generation after a user's first ever request issues at least one (and typically two) doomed-to-fail INSERT attempts against a unique constraint. This adds unnecessary round-trips/locking and constraint-violation error noise on a hot path indefinitely.Consider having
ensureFreeGrantcheck existence with a cheapfindUnique/countonidempotencyKeybefore attempting the insert, only falling back to insert+catch for the actual race window.🤖 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/app/server/api/candidature/generate.post.ts` around lines 65 - 66, The hot path around getOrInitBalance and ensureFreeGrant is repeatedly attempting an INSERT that is guaranteed to fail after the free grant already exists, causing avoidable unique-violation noise on every generation. Update ensureFreeGrant in credits.ts to first do a cheap existence check on the idempotencyKey (using findUnique or count) before inserting, and only use the insert-and-catch P2002 path for the race window; then keep getOrInitBalance and consumeOneCredit behavior the same so they no longer trigger doomed INSERTs on every request.
🤖 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/app/package.json`:
- Line 10: The stripe:seed script now relies on Node flags that only work on
20.6+, so update the Node version requirements used by the repo to match. Bump
the engines declaration in package.json and the version pin in .nvmrc/CI to at
least 20.6.0, and make sure any related version checks or toolchain configs stay
consistent with the new minimum. Refer to the stripe:seed script when verifying
the change.
In `@apps/app/server/api/billing/checkout.post.ts`:
- Line 63: The checkout redirect origin is currently derived from
getRequestURL(event).origin when APP_URL is unset, which trusts
client-controlled Host/X-Forwarded-Host input. Update checkout.post.ts so the
checkout flow in the origin/success_url/cancel_url path uses only a trusted
server-side base URL (prefer APP_URL) and fails closed if it is missing or
invalid. Locate the logic around the origin constant and the Stripe checkout
session creation, and remove any fallback that reads the request origin from
event headers.
In `@apps/app/server/api/billing/webhook.post.ts`:
- Around line 36-47: The Checkout webhook only credits on
checkout.session.completed when payment_status is paid, which misses delayed
payment methods; update the billing webhook handler in webhook.post.ts around
the stripeEvent.type / grantPurchasedCredits flow to either restrict Checkout to
synchronous methods or add handling for checkout.session.async_payment_succeeded
so async payments also trigger the same credit-granting path.
In `@apps/app/server/api/candidature/generate.post.ts`:
- Around line 163-166: The post-generation debit path in generate.post.ts does
not handle InsufficientCreditsError from consumeOneCredit, so a failed debit
after successful generation can bubble up as an uncaught 500. Wrap the
consumeOneCredit(userId) call in the handler logic and map
InsufficientCreditsError to the same structured QUOTA_EXCEEDED_CODE 403 response
used by the pre-check, preserving the generated result flow and keeping the
error shape consistent.
- Around line 61-73: The credit flow in generate.post.ts only performs a balance
pre-check, so concurrent requests can still overlap and both debit in
credits.ts. Update the actual debit path in the credits utility to be serialized
by user, either by wrapping the balance update in a SERIALIZABLE transaction
with retry handling or by taking a per-user advisory lock before the decrement.
Make sure the relevant balance-check/debit logic in getOrInitBalance and the
debit function in credits.ts are protected so only one request can consume the
last credit.
In `@apps/app/server/utils/credits.ts`:
- Around line 67-85: `consumeOneCredit` can still double-spend under concurrent
requests because `getCreditBalance` is read before the debit insert, so two
transactions may both pass the check and write `delta: -1`. Fix this by
serializing the per-user debit inside the `db.$transaction` in
`consumeOneCredit` using a per-user transaction lock (for example an advisory
lock keyed by `userId`) or by moving this path to serializable isolation with
retry logic, while keeping the existing `ensureFreeGrant`, `getCreditBalance`,
and `creditLedger.create` flow intact.
---
Nitpick comments:
In `@apps/app/pages/candidature.vue`:
- Around line 37-40: The destructured refresh function in useFetch for
BillingSummary still uses the old usage-oriented name, which is misleading.
Rename refreshUsage in the candidature.vue setup to a billing-specific name and
update the corresponding calls in the same component so the identifier matches
BILLING_SUMMARY_PATH/BillingSummary and the credits-oriented migration.
In `@apps/app/pages/credits.vue`:
- Around line 27-50: The fallback used to compute the `before` value in
`credits.vue` can hide an already-updated balance when `CREDITS_BEFORE_KEY` is
missing. Update the `onMounted` success flow so `before` is derived from a
stable pre-checkout baseline only, or explicitly detect the missing localStorage
case and avoid comparing against `creditBalance.value` from the same render.
Keep the `waitForCredit` helper and its `now > previous` check, but ensure the
`toast.info('Crédits en cours', ...)` path is only used when the balance truly
has not changed.
In `@apps/app/prisma/schema.prisma`:
- Around line 377-395: Add a real Prisma relation for CreditLedger.userId so
ledger rows must reference an existing User.id; update the CreditLedger model to
declare the foreign key relation and add the reciprocal relation field on User.
Use a non-cascading delete behavior such as Restrict to preserve the append-only
audit trail, and keep the existing uniqueness/indexing intact.
In `@apps/app/server/api/billing/checkout.post.ts`:
- Around line 40-50: The checkout flow in checkout.post.ts can create duplicate
Stripe customers when two requests race through the customer creation path in
the checkout handler. Update the logic around the existing customerId /
stripe.customers.create block to make it idempotent, either by using a
userId-derived idempotency key on customers.create or by doing a conditional
prisma.user.update/updateMany on stripeCustomerId being null and re-reading the
stored value if another request already set it. Ensure the checkout path always
reuses the persisted Stripe customer for the same user.
- Around line 43-48: The Stripe writes in the checkout flow are not idempotent,
so retries can create duplicate customer or session objects. Update the
`checkout.post.ts` flow in the `stripe.customers.create(...)` and
`stripe.checkout.sessions.create(...)` calls to pass a stable `idempotencyKey`
in their request options, using the same retry-safe key derived from the current
user/checkout request so repeated attempts reuse the same Stripe object.
In `@apps/app/server/api/billing/webhook.post.ts`:
- Around line 15-51: The webhook handler in defineEventHandler for
billing/webhook.post.ts needs test coverage for its security- and
money-sensitive behavior. Add unit/integration tests around the Stripe webhook
flow by mocking getStripe().webhooks.constructEvent and covering valid
checkout.session.completed processing, invalid signature handling, missing
secret/configured-secret cases, and the idempotent grantPurchasedCredits path
keyed by session.id. Use the existing handler logic and symbols like
StripeNotConfiguredError and grantPurchasedCredits to locate and exercise the
critical branches.
In `@apps/app/server/api/candidature/generate.post.ts`:
- Around line 65-66: The hot path around getOrInitBalance and ensureFreeGrant is
repeatedly attempting an INSERT that is guaranteed to fail after the free grant
already exists, causing avoidable unique-violation noise on every generation.
Update ensureFreeGrant in credits.ts to first do a cheap existence check on the
idempotencyKey (using findUnique or count) before inserting, and only use the
insert-and-catch P2002 path for the race window; then keep getOrInitBalance and
consumeOneCredit behavior the same so they no longer trigger doomed INSERTs on
every request.
🪄 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
Run ID: 149f2d13-90c6-43b1-a0b7-2640664e4a5f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/app/.env.exampleapps/app/config/pricing.tsapps/app/layouts/default.vueapps/app/package.jsonapps/app/pages/candidature.vueapps/app/pages/credits.vueapps/app/prisma/migrations/20260622084853_billing_credits/migration.sqlapps/app/prisma/schema.prismaapps/app/scripts/stripe-seed.tsapps/app/server/api/billing/checkout.post.tsapps/app/server/api/billing/summary.get.tsapps/app/server/api/billing/webhook.post.tsapps/app/server/api/candidature/generate.post.tsapps/app/server/api/usage/current.get.tsapps/app/server/utils/credits.tsapps/app/server/utils/stripe.tsapps/app/test/pricing.spec.tsapps/app/test/usage.spec.tspackages/shared/src/billing.tspackages/shared/src/index.tspackages/shared/src/usage.ts
| "prisma:generate": "prisma generate", | ||
| "prisma:migrate:dev": "prisma migrate dev", | ||
| "prisma:migrate:deploy": "prisma migrate deploy", | ||
| "stripe:seed": "node --env-file=.env --import tsx scripts/stripe-seed.ts", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check declared Node engine/toolchain versions
rg -n '"engines"' -A5 apps/app/package.json package.json 2>/dev/null
fd -a '.nvmrc' -o -a '.tool-versions'Repository: Tbeaumont79/cv-optimizer
Length of output: 552
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## apps/app/package.json\n'
cat -n apps/app/package.json
printf '\n## root package.json\n'
cat -n package.json
printf '\n## version/toolchain files\n'
fd -a -H '(.nvmrc|.tool-versions|volta.json|package.json|pnpm-workspace.yaml|dockerfile|Dockerfile)' . .
printf '\n## CI references to Node\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'node\s*[:=]|setup-node|actions/setup-node|20\.[0-9]|18\.[0-9]|22\.[0-9]' .github . apps package.json .nvmrc .tool-versions volta.json 2>/dev/null || trueRepository: Tbeaumont79/cv-optimizer
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## .nvmrc\n'
cat -n .nvmrc
printf '\n## .github/workflows/ci.yml (Node setup lines)\n'
rg -n -A4 -B2 'setup-node|node-version|node-version-file' .github/workflows/ci.yml
printf '\n## exact package-manager/engine lines in workspace packages\n'
rg -n '"engines"|"packageManager"|node: ' package.json apps/app/package.json packages/shared/package.jsonRepository: Tbeaumont79/cv-optimizer
Length of output: 878
🌐 Web query:
Node.js --env-file --import introduced version 20.6.0 release notes
💡 Result:
The Node.js version 20.6.0, released on September 4, 2023, officially introduced the --env-file and --import command-line flags [1][2]. --env-file This flag provides built-in support for loading environment variables from a file [2][3]. It parses a specified file (often named.env) and makes the defined variables available on process.env [3]. It also supports defining NODE_OPTIONS within these files [2]. If a variable is defined in both the environment and the file, the existing environment value takes precedence [3]. --import This flag is used to preload specified ECMAScript modules [3]. A primary use case highlighted in the 20.6.0 release is to facilitate the registration of module customization hooks (via the register function from node:module) before any application code, including the entry point, executes [2][4]. It serves as a modern alternative to using --require for loading modules, as --require is limited to CommonJS modules [3]. Modules preloaded with --require will run before those preloaded with --import [3].
Citations:
- 1: https://nodejs.org/en/blog/release/v20.6.0
- 2: https://github.com/nodejs/node/releases/tag/v20.6.0
- 3: https://nodejs.org/download/release/v20.6.0/docs/api/cli.html
- 4: v20.6.0 proposal nodejs/node#49185
Bump Node to 20.6+ for stripe:seed node --env-file and --import require Node 20.6.0+. The repo still declares >=20.0.0 in package.json and 20 in .nvmrc/CI, so older 20.0–20.5 installs can fail here.
🤖 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/app/package.json` at line 10, The stripe:seed script now relies on Node
flags that only work on 20.6+, so update the Node version requirements used by
the repo to match. Bump the engines declaration in package.json and the version
pin in .nvmrc/CI to at least 20.6.0, and make sure any related version checks or
toolchain configs stay consistent with the new minimum. Refer to the stripe:seed
script when verifying the change.
| throw createError({ statusCode: 503, message: 'Catalogue Stripe non initialisé (lance le seed).' }) | ||
| } | ||
|
|
||
| const origin = process.env.APP_URL || getRequestURL(event).origin |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Client-controlled Host header can hijack Checkout redirect URLs.
When APP_URL isn't set, getRequestURL(event).origin falls back to the request's Host/X-Forwarded-Host header, which is attacker-controllable unless the proxy strictly overwrites it. This origin is used verbatim for success_url/cancel_url (Line 68-69), letting an attacker craft a request that redirects the paying customer to an arbitrary domain post-payment (phishing/open-redirect risk).
🔒 Proposed fix: fail closed instead of trusting request origin
- const origin = process.env.APP_URL || getRequestURL(event).origin
+ const origin = process.env.APP_URL
+ if (!origin) {
+ throw createError({ statusCode: 500, message: 'APP_URL manquante en configuration.' })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const origin = process.env.APP_URL || getRequestURL(event).origin | |
| const origin = process.env.APP_URL | |
| if (!origin) { | |
| throw createError({ statusCode: 500, message: 'APP_URL manquante en configuration.' }) | |
| } |
🤖 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/app/server/api/billing/checkout.post.ts` at line 63, The checkout
redirect origin is currently derived from getRequestURL(event).origin when
APP_URL is unset, which trusts client-controlled Host/X-Forwarded-Host input.
Update checkout.post.ts so the checkout flow in the
origin/success_url/cancel_url path uses only a trusted server-side base URL
(prefer APP_URL) and fails closed if it is missing or invalid. Locate the logic
around the origin constant and the Stripe checkout session creation, and remove
any fallback that reads the request origin from event headers.
| if (stripeEvent.type === 'checkout.session.completed') { | ||
| const session = stripeEvent.data.object as Stripe.Checkout.Session | ||
| if (session.payment_status === 'paid') { | ||
| const userId = session.metadata?.userId | ||
| const packKey = session.metadata?.packKey ?? '' | ||
| const credits = Number(session.metadata?.credits) | ||
| if (userId && Number.isFinite(credits) && credits > 0) { | ||
| // Idempotent : un rejeu du même paiement ne crédite pas deux fois. | ||
| await grantPurchasedCredits({ userId, sessionId: session.id, packKey, credits }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Handle async Checkout payments too
checkout.session.completed can arrive before payment succeeds for delayed methods, but this webhook only credits payment_status === 'paid'. Since Checkout here isn’t limited to synchronous methods, those payments can be dropped. Either restrict Checkout to card/synchronous methods or also handle checkout.session.async_payment_succeeded.
🤖 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/app/server/api/billing/webhook.post.ts` around lines 36 - 47, The
Checkout webhook only credits on checkout.session.completed when payment_status
is paid, which misses delayed payment methods; update the billing webhook
handler in webhook.post.ts around the stripeEvent.type / grantPurchasedCredits
flow to either restrict Checkout to synchronous methods or add handling for
checkout.session.async_payment_succeeded so async payments also trigger the same
credit-granting path.
| // Gate CRÉDITS avant tout appel LLM (aucun token consommé si solde nul). Le solde | ||
| // inclut les générations offertes (octroi gratuit idempotent au 1er accès). 1 crédit | ||
| // = 1 génération, débité APRÈS succès. Bypass dev via DISABLE_GENERATION_LIMIT=true. | ||
| const limitDisabled = process.env.DISABLE_GENERATION_LIMIT === 'true' | ||
| const allowed = limitDisabled || (await isUsageAllowed(prisma, userId, 'generation', FREE_TIER_QUOTAS, now)) | ||
| if (!allowed) { | ||
| throw createError({ | ||
| statusCode: 403, | ||
| message: 'Tes générations offertes sont épuisées', | ||
| data: { code: QUOTA_EXCEEDED_CODE }, | ||
| }) | ||
| if (!limitDisabled) { | ||
| const balance = await getOrInitBalance(userId) | ||
| if (balance < 1) { | ||
| throw createError({ | ||
| statusCode: 403, | ||
| message: 'Tu n’as plus de crédits — achète un pack pour continuer', | ||
| data: { code: QUOTA_EXCEEDED_CODE }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does Prisma's default $transaction isolation level on PostgreSQL (READ COMMITTED) allow concurrent transactions to both read the same pre-write aggregate value before either commits?
💡 Result:
Yes, Prisma's default transaction isolation level on PostgreSQL—Read Committed—allows concurrent transactions to read the same pre-write aggregate value [1][2]. Under the Read Committed isolation level, each statement within a transaction sees only data that was committed before that specific statement began [3][4]. It does not provide a single, consistent snapshot for the entire duration of a transaction [3][4]. If two concurrent transactions both perform an aggregate read (such as SUM or COUNT) on the same data, they will each read whatever state has been committed at the moment their respective queries execute [3]. If a third transaction modifies and commits data between the execution of the first transaction's query and the second transaction's query, the two transactions will see different results [3][4]. Furthermore, Read Committed does not prevent non-repeatable reads [3][4]. If a single transaction performs the same aggregate query twice, it may receive different results if another transaction modifies and commits data in between those two queries [3]. To ensure consistent reads across a transaction or to prevent other transactions from modifying data that you have read, you must use a higher isolation level, such as Repeatable Read or Serializable, or employ locking reads (e.g., SELECT... FOR UPDATE) [2][3]. Prisma allows you to set these higher isolation levels when starting a transaction [2].
Citations:
- 1: https://stackoverflow.com/questions/69539942/prisma-postgres-transaction-problem-read-incremental-number-in-concurrent-tr
- 2: https://www.prisma.io/docs/v6/orm/prisma-client/queries/transactions
- 3: https://www.postgresql.org/docs/18/transaction-iso.html
- 4: https://www.postgresql.org/docs/13/transaction-iso.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
git ls-files 'apps/app/server/api/candidature/generate.post.ts' 'apps/app/server/utils/credits.ts'
echo
echo '--- generate.post.ts outline ---'
ast-grep outline apps/app/server/api/candidature/generate.post.ts --view expanded || true
echo
echo '--- credits.ts outline ---'
ast-grep outline apps/app/server/utils/credits.ts --view expanded || true
echo
echo '--- key locations in generate.post.ts ---'
nl -ba apps/app/server/api/candidature/generate.post.ts | sed -n '1,240p'
echo
echo '--- key locations in credits.ts ---'
nl -ba apps/app/server/utils/credits.ts | sed -n '1,260p'Repository: Tbeaumont79/cv-optimizer
Length of output: 1266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any transaction isolation configuration or locking in the credit path.
rg -n "Serializable|isolationLevel|advisory|for update|FOR UPDATE|consumeOneCredit|getOrInitBalance|balance" apps/app/server -SRepository: Tbeaumont79/cv-optimizer
Length of output: 1296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- generate.post.ts (around the gate and debit) ---'
sed -n '55,175p' apps/app/server/api/candidature/generate.post.ts | cat -n
echo
echo '--- credits.ts ---'
sed -n '1,180p' apps/app/server/utils/credits.ts | cat -nRepository: Tbeaumont79/cv-optimizer
Length of output: 10729
Serialize the credit debit path apps/app/server/api/candidature/generate.post.ts:66-73 only pre-checks the balance; it doesn’t protect the real debit in apps/app/server/utils/credits.ts:77-83. With the default Prisma/Postgres isolation, two overlapping requests can both pass, both run the LLM, and both commit -1, which can drive the ledger negative. Move the debit to SERIALIZABLE with retry, or take a per-user advisory lock in credits.ts.
🤖 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/app/server/api/candidature/generate.post.ts` around lines 61 - 73, The
credit flow in generate.post.ts only performs a balance pre-check, so concurrent
requests can still overlap and both debit in credits.ts. Update the actual debit
path in the credits utility to be serialized by user, either by wrapping the
balance update in a SERIALIZABLE transaction with retry handling or by taking a
per-user advisory lock before the decrement. Make sure the relevant
balance-check/debit logic in getOrInitBalance and the debit function in
credits.ts are protected so only one request can consume the last credit.
| // Débite 1 crédit pour cette génération réussie (transactionnel, jamais | ||
| // déficitaire). Bypass dev : on ne débite pas quand le gate est désactivé. | ||
| if (!limitDisabled) await consumeOneCredit(userId) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unhandled InsufficientCreditsError after a successful generation.
consumeOneCredit throws a plain Error subclass (InsufficientCreditsError) when the balance check fails inside its own transaction. If this happens (e.g. via the race described above, or simply a second in-flight request), it's not caught here — it will propagate as an uncaught exception (likely surfacing as a generic 500) after the LLM generation and recordUsageEvent have already succeeded and consumed resources. The user loses their already-generated result and gets an opaque error instead of the structured QUOTA_EXCEEDED_CODE 403 used by the pre-check at Line 68-72.
🛡️ Proposed fix: map the debit failure to the same structured error
- if (!limitDisabled) await consumeOneCredit(userId)
+ if (!limitDisabled) {
+ try {
+ await consumeOneCredit(userId)
+ } catch (err) {
+ if (err instanceof InsufficientCreditsError) {
+ throw createError({
+ statusCode: 403,
+ message: 'Tu n’as plus de crédits — achète un pack pour continuer',
+ data: { code: QUOTA_EXCEEDED_CODE },
+ })
+ }
+ throw err
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Débite 1 crédit pour cette génération réussie (transactionnel, jamais | |
| // déficitaire). Bypass dev : on ne débite pas quand le gate est désactivé. | |
| if (!limitDisabled) await consumeOneCredit(userId) | |
| // Débite 1 crédit pour cette génération réussie (transactionnel, jamais | |
| // déficitaire). Bypass dev : on ne débite pas quand le gate est désactivé. | |
| if (!limitDisabled) { | |
| try { | |
| await consumeOneCredit(userId) | |
| } catch (err) { | |
| if (err instanceof InsufficientCreditsError) { | |
| throw createError({ | |
| statusCode: 403, | |
| message: 'Tu n’as plus de crédits — achète un pack pour continuer', | |
| data: { code: QUOTA_EXCEEDED_CODE }, | |
| }) | |
| } | |
| throw err | |
| } | |
| } |
🤖 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/app/server/api/candidature/generate.post.ts` around lines 163 - 166, The
post-generation debit path in generate.post.ts does not handle
InsufficientCreditsError from consumeOneCredit, so a failed debit after
successful generation can bubble up as an uncaught 500. Wrap the
consumeOneCredit(userId) call in the handler logic and map
InsufficientCreditsError to the same structured QUOTA_EXCEEDED_CODE 403 response
used by the pre-check, preserving the generated result flow and keeping the
error shape consistent.
| /** | ||
| * Consomme 1 crédit pour une génération RÉUSSIE, de façon atomique et sûre : | ||
| * dans une transaction, on relit le solde puis on insère la ligne -1 seulement | ||
| * s'il reste ≥ 1 crédit. Lève `InsufficientCreditsError` sinon (jamais déficitaire). | ||
| */ | ||
| export async function consumeOneCredit(userId: string, db: PrismaClient = defaultPrisma): Promise<number> { | ||
| // Octroi gratuit AVANT la transaction : il s'appuie sur un catch de violation | ||
| // d'unicité (P2002), or une erreur SQL dans une transaction Postgres l'avorte | ||
| // entièrement (25P02). On le fait donc hors transaction (idempotent, sans risque). | ||
| await ensureFreeGrant(db, userId) | ||
| return db.$transaction(async (tx) => { | ||
| const balance = await getCreditBalance(tx, userId) | ||
| if (balance < 1) throw new InsufficientCreditsError() | ||
| await tx.creditLedger.create({ | ||
| data: { userId, delta: -1, reason: 'GENERATION' }, | ||
| }) | ||
| return balance - 1 | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no isolation-level override or locking exists elsewhere for this transaction.
rg -n -A5 -B5 'consumeOneCredit' apps/app/server/utils/credits.tsRepository: Tbeaumont79/cv-optimizer
Length of output: 880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== credits.ts outline =="
ast-grep outline apps/app/server/utils/credits.ts --view expanded || true
echo
echo "== targeted search for locking/isolation in credits.ts =="
rg -n -C 3 'pg_advisory_xact_lock|FOR UPDATE|Serializable|isolation|$transaction|creditLedger|getCreditBalance|ensureFreeGrant|grantPurchasedCredits' apps/app/server/utils/credits.ts
echo
echo "== nearby lines in credits.ts =="
sed -n '1,220p' apps/app/server/utils/credits.tsRepository: Tbeaumont79/cv-optimizer
Length of output: 7490
Serialize per-user debit before checking the balance. consumeOneCredit still has a double-spend race under concurrent calls: two transactions can read the same balance, both pass balance < 1, and both insert delta: -1, which can push the ledger negative. Add a per-user transaction lock here (for example pg_advisory_xact_lock(hashtext(userId))) or move this path to SERIALIZABLE with retry.
🤖 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/app/server/utils/credits.ts` around lines 67 - 85, `consumeOneCredit`
can still double-spend under concurrent requests because `getCreditBalance` is
read before the debit insert, so two transactions may both pass the check and
write `delta: -1`. Fix this by serializing the per-user debit inside the
`db.$transaction` in `consumeOneCredit` using a per-user transaction lock (for
example an advisory lock keyed by `userId`) or by moving this path to
serializable isolation with retry logic, while keeping the existing
`ensureFreeGrant`, `getCreditBalance`, and `creditLedger.create` flow intact.
1 participant
Contexte
Introduit le billing par packs de crédits one-shot (Stripe Checkout hébergé, mode
payment) — pas d'abonnement, pas d'illimité. Le gating de génération passe du quota mensuel aux crédits (1 crédit = 1 CV optimisé).Ce que fait la PR
server/utils/credits.ts) : solde = somme des deltas, invariant « jamais déficitaire », idempotence paridempotencyKey(FREE_GRANTunique /PURCHASE= session id /GENERATION= −1 transactionnel). 5 générations offertes à vie.checkout— session hébergée, prix résolu parlookup_key(aucun ID Stripe en base), montant/crédits déterminés server-side.webhook— signature vérifiée sur le corps brut, crédite uniquement surcheckout.session.completedpayé, idempotent.summary— solde + octroi gratuit idempotent au premier accès.server/utils/stripe.ts) + seed idempotent (scripts/stripe-seed.ts) créant produits/prix depuisconfig/pricing.ts(source de vérité).credit_ledger,stripeCustomerIdsurusers.solde >= 1viaconsumeOneCredit(remplace le quota mensuelgeneration; l'export PDF reste borné par période). L'UI candidature et la nav basculent sur le solde de crédits./credits: achat de packs + polling du solde au retour de Checkout (le webhook crédite de façon asynchrone).@cvo/shared/billing) + tests (pricing,usage).Config requise
.envserveur :STRIPE_SECRET_KEY,STRIPE_WEBHOOK_SECRET. Lancer le seed une fois (pnpm --filter @cvo/app stripe:seed).stripe listendoit cibler le même compte Stripe queSTRIPE_SECRET_KEY(sinon les events ne sont jamais forwardés → crédits non accordés) :stripe listen --api-key <clé sk_test .env> --forward-to localhost:3000/api/billing/webhookValidation
pnpm typecheck✅pricing+usage: 15/15 ✅[200], ledgerPURCHASE, solde à jour).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes