feat(stripe): stamp metadata.accountId on sub Customers + one-time backfill#555
Open
sweetmantech wants to merge 5 commits into
Open
feat(stripe): stamp metadata.accountId on sub Customers + one-time backfill#555sweetmantech wants to merge 5 commits into
sweetmantech wants to merge 5 commits into
Conversation
backfill script for existing subs
Prepares for credit auto-charge (PR 2b) by ensuring every Stripe
Customer linked to a Recoup account has discoverable
`metadata.accountId`. The upcoming auto-charge flow looks up Customers
via `stripe.customers.search`, which only finds Customers with the
metadata stamped — without this PR, every legacy Pro subscriber's
first top-up would create a duplicate Customer instead of reusing
their subscription Customer.
Three things in this PR (zero behavior change to public APIs):
1. New `lib/stripe/resolveStripeCustomerForAccount.ts` — looks up a
Customer via `customers.search` by `metadata['accountId']`; if no
match, creates one stamped with `metadata.accountId`. Pure 2-call
Stripe interaction, no DB.
2. Subscription Checkout flow (`createStripeSession.ts`) now resolves
the Customer BEFORE creating the session and passes `customer:` to
Stripe. New subscribers from this PR onward will have findable
Customers automatically.
3. One-time backfill script
`scripts/backfill-subscriber-customer-metadata.ts` — iterates
`stripe.subscriptions.list` paginated, retrieves each subscription's
Customer, and stamps `metadata.accountId` if missing. Idempotent
(skips already-stamped Customers, logs conflicts without
overwriting). Supports `--dry-run`. Run once after this deploys:
STRIPE_SK=sk_live_... npx tsx scripts/backfill-subscriber-customer-metadata.ts --dry-run
STRIPE_SK=sk_live_... npx tsx scripts/backfill-subscriber-customer-metadata.ts
The decision function `shouldStampCustomerMetadata` is extracted as
a pure unit with its own test file.
TDD: new tests for resolveStripeCustomerForAccount (3), updated
createStripeSession tests (3), new shouldStampCustomerMetadata tests
(3). Full suite: 471 files / 2763 tests green; lint:check clean;
typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a helper that resolves (or creates) a Stripe Customer by ChangesStripe Customer Resolution
Sequence DiagramsequenceDiagram
participant createStripeSession
participant resolveStripeCustomerForAccount
participant StripeAPI as Stripe API
createStripeSession->>resolveStripeCustomerForAccount: accountId
resolveStripeCustomerForAccount->>StripeAPI: search customers by metadata.accountId (limit=1)
alt Customer found
StripeAPI-->>resolveStripeCustomerForAccount: existing customerId
else Customer not found
resolveStripeCustomerForAccount->>StripeAPI: create customer with metadata.accountId (idempotencyKey)
StripeAPI-->>resolveStripeCustomerForAccount: new customerId
end
resolveStripeCustomerForAccount-->>createStripeSession: customerId
createStripeSession->>StripeAPI: create checkout session with customerId
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Confidence score: 3/5
- There is some merge risk because
lib/stripe/resolveStripeCustomerForAccount.tshas a medium-severity issue (6/10, confidence 7/10): interpolating unescapedaccountIdinto a Stripe search query can produce malformed or potentially injectable queries, which could affect customer lookup behavior. - The issue in
scripts/backfill-subscriber-customer-metadata.tsis lower impact (4/10) and primarily maintainability-focused (file length over the 100-line guideline), so it is unlikely to block functionality by itself. - Pay close attention to
lib/stripe/resolveStripeCustomerForAccount.ts,scripts/backfill-subscriber-customer-metadata.ts- first for query-safety in Stripe search construction, second for maintainability/style-rule compliance.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/stripe/resolveStripeCustomerForAccount.ts">
<violation number="1" location="lib/stripe/resolveStripeCustomerForAccount.ts:18">
P2: Escape `accountId` before interpolating it into the Stripe search query string to avoid malformed/injectable queries.</violation>
</file>
<file name="scripts/backfill-subscriber-customer-metadata.ts">
<violation number="1" location="scripts/backfill-subscriber-customer-metadata.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
New file exceeds the rule-mandated 100-line maximum, violating the maintainability guideline.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client Frontend
participant Route as API Route
participant Session as createStripeSession
participant Resolver as resolveStripeCustomerForAccount
participant Stripe as Stripe API
participant DB as Internal Database
Note over Client,DB: Subscription Checkout Flow (PR 2a)
Client->>Route: POST /api/subscriptions/sessions
Route->>Session: createStripeSession(accountId, successUrl)
Session->>Resolver: resolveStripeCustomerForAccount(accountId)
Resolver->>Stripe: customers.search(query: metadata['accountId']:'{accountId}', limit: 1)
alt Customer found via search
Stripe-->>Resolver: return existing Customer ID
else No Customer found
Resolver->>Stripe: customers.create(metadata: { accountId })
Stripe-->>Resolver: return new Customer ID
end
Resolver-->>Session: customer ID (cus_xxx)
Session->>Stripe: checkout.sessions.create(customer: cus_xxx, line_items, mode: subscription, client_reference_id: accountId)
Stripe-->>Session: session with URL
Session-->>Route: checkout session
Route-->>Client: redirect to Stripe Checkout
Note over Client,Stripe: Credits Checkout Flow (unchanged)
Client->>Route: POST /api/credits/sessions
Route->>Stripe: checkout.sessions.create(line_items, mode: payment)
Stripe-->>Route: session with URL
Route-->>Client: redirect to Stripe Checkout
Note over Resolver,Stripe: Backfill Script (one-time, for legacy Customers)
participant Script as backfill-subscriber-customer-metadata
participant Decision as shouldStampCustomerMetadata
Script->>Stripe: subscriptions.list(limit: 100)
loop For each subscription
Stripe-->>Script: subscription with metadata.accountId
alt Subscription has no accountId
Script->>Script: skip
else Subscription has accountId
Script->>Stripe: customers.retrieve(sub.customer)
alt Customer is deleted
Script->>Script: skip
else Customer exists
Script->>Decision: shouldStampCustomerMetadata(customer.metadata, accountId)
alt Decision: skip (already-stamped)
Decision-->>Script: skip
else Decision: skip (conflict)
Decision-->>Script: skip + log warning
else Decision: stamp (missing)
opt Not dry-run
Script->>Stripe: customers.update(customerId, metadata: { ...existing, accountId })
end
end
end
end
end
Script->>Script: log stats summary
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| */ | ||
| export async function resolveStripeCustomerForAccount(accountId: string): Promise<string> { | ||
| const search = await stripeClient.customers.search({ | ||
| query: `metadata['accountId']:'${accountId}'`, |
There was a problem hiding this comment.
P2: Escape accountId before interpolating it into the Stripe search query string to avoid malformed/injectable queries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/stripe/resolveStripeCustomerForAccount.ts, line 18:
<comment>Escape `accountId` before interpolating it into the Stripe search query string to avoid malformed/injectable queries.</comment>
<file context>
@@ -0,0 +1,31 @@
+ */
+export async function resolveStripeCustomerForAccount(accountId: string): Promise<string> {
+ const search = await stripeClient.customers.search({
+ query: `metadata['accountId']:'${accountId}'`,
+ limit: 1,
+ });
</file context>
Per review: the prior backfill only stamped Customers attached to a
subscription. Ex-subscribers, trial users, manual/admin creations, and
the one legacy sub with missing metadata.accountId would never be
backfilled — they'd end up with duplicate Customers after PR 2b ships
(their existing un-stamped Customer plus a new precreated one).
Restructured to iterate ALL Stripe Customers with a two-stage
resolution:
1. **Subscription map (authoritative)** — Phase 1 prefetches every
subscription's `metadata.accountId` and indexes by customer.id.
Same source as before, just collected up-front.
2. **Email lookup (heuristic)** — Phase 2 iterates `customers.list`
and falls back to `selectAccountByEmail(customer.email)` for any
Customer not in the subscription map. Email is trimmed +
lowercased before lookup since Stripe email casing isn't normalized.
The pure resolver (`resolveAccountIdForCustomer`) is extracted with
its own test file — 5 unit tests cover sub-priority, email fallback,
email normalization, no-email, and no-match cases.
Renamed: `backfill-subscriber-customer-metadata.ts` →
`backfill-customer-metadata.ts` to reflect the broader scope.
New env requirement at runtime: `SUPABASE_URL` and `SUPABASE_KEY`
alongside `STRIPE_SK`, used by the email lookup.
New stats:
{
customersScanned, customersAlreadyStamped,
stampedViaSub, stampedViaEmail,
conflicts, unresolved, errors
}
Idempotency preserved: re-runs skip already-stamped Customers
(`customersAlreadyStamped`); conflicts (existing different accountId)
are logged without overwriting.
Tests: 5 new for `resolveAccountIdForCustomer`. Full suite 472 files
/ 2768 tests green; lint clean; typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Update — backfill now covers all Customers, not just active subscribersPushed What changedThe script (renamed
Pure resolver Runtime envSUPABASE_URL=... SUPABASE_KEY=... STRIPE_SK=sk_live_... \
npx tsx scripts/backfill-customer-metadata.ts --dry-runNew stats outputExpected vs prior dry-runYour prior dry-run showed 12 sub-based stamps + 1 missingAccountIdOnSub. With the new logic, you'll likely see:
Tests
Full suite: 472 / 2768 ✅, lint clean, typecheck clean. Suggested re-runSUPABASE_URL=... SUPABASE_KEY=... STRIPE_SK=sk_live_... \
npx tsx scripts/backfill-customer-metadata.ts --dry-runInspect the output, then drop |
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/backfill-customer-metadata.ts">
<violation number="1" location="scripts/backfill-customer-metadata.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
File exceeds the 100-line maximum and combines multiple responsibilities in one script module.</violation>
<violation number="2" location="scripts/backfill-customer-metadata.ts:55">
P2: `subscriptions.list()` without `status: 'all'` excludes canceled subscriptions, so the authoritative subscription map will miss customers whose subscriptions ended. Add `status: 'all'` to match the documented intent of scanning all subscriptions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The customer-metadata backfill has been run live against production
Stripe (42 customers scanned, 40 stamped, 0 unresolved, 0 errors).
The script and its pure-helper companions have served their purpose
and don't need to live in the repo permanently.
Removed:
scripts/backfill-customer-metadata.ts
scripts/resolveAccountIdForCustomer.ts
scripts/shouldStampCustomerMetadata.ts
scripts/__tests__/resolveAccountIdForCustomer.test.ts
scripts/__tests__/shouldStampCustomerMetadata.test.ts
Retained from this PR:
lib/stripe/resolveStripeCustomerForAccount.ts (used by sub
Checkout +
PR 2b)
lib/stripe/__tests__/resolveStripeCustomerForAccount.test.ts
lib/stripe/createStripeSession.ts (sub Checkout
precreates
Customer)
lib/stripe/__tests__/createStripeSession.test.ts
Future stamping happens automatically — every new subscription
Checkout calls `resolveStripeCustomerForAccount` which stamps
`metadata.accountId` on the Customer at creation time. The one-time
backfill closed the gap for legacy Customers; no need to keep the
script around.
Tests: 470 files / 2760 ✅ ; lint clean ; typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@lib/stripe/resolveStripeCustomerForAccount.ts`:
- Around line 17-20: The Stripe search query is vulnerable to syntax breakage
because accountId is interpolated directly into the query passed to
stripeClient.customers.search; sanitize/validate accountId before building the
query used in resolveStripeCustomerForAccount.ts. Fix by normalizing accountId
(prefer a whitelist: allow only expected chars like [A-Za-z0-9-_] or, if broader
values required, escape special characters) and/or escaping backslashes and
single quotes (replace "\" with "\\\\" and "'" with "\\'") before constructing
the query string for stripeClient.customers.search so queries like `query:
\`metadata['accountId']:'${sanitizedAccountId}'\`` cannot be broken by
malicious/odd input.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 64a37b7c-4c4d-4b30-9a6f-57bde5436abf
⛔ Files ignored due to path filters (2)
lib/stripe/__tests__/createStripeSession.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/stripe/__tests__/resolveStripeCustomerForAccount.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/stripe/createStripeSession.tslib/stripe/resolveStripeCustomerForAccount.ts
Comment on lines
+17
to
+20
| const search = await stripeClient.customers.search({ | ||
| query: `metadata['accountId']:'${accountId}'`, | ||
| limit: 1, | ||
| }); |
There was a problem hiding this comment.
Escape or validate accountId to prevent Stripe query injection.
The accountId is directly interpolated into the Stripe search query without escaping. If accountId contains special characters like single quotes ('), colons (:), or backslashes (\), the query will break or behave unexpectedly.
For example, if accountId = "test'foo", the resulting query becomes:
metadata['accountId']:'test'foo'
This breaks Stripe's query syntax and could lead to failed lookups, causing duplicate customer creation even when a customer exists.
🛡️ Proposed fix: sanitize accountId before query construction
export async function resolveStripeCustomerForAccount(accountId: string): Promise<string> {
+ // Escape single quotes and backslashes for Stripe query syntax
+ const escapedAccountId = accountId.replace(/\\/g, '\\\\').replace(/'/g, "\\'");
+
const search = await stripeClient.customers.search({
- query: `metadata['accountId']:'${accountId}'`,
+ query: `metadata['accountId']:'${escapedAccountId}'`,
limit: 1,
});🤖 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 `@lib/stripe/resolveStripeCustomerForAccount.ts` around lines 17 - 20, The
Stripe search query is vulnerable to syntax breakage because accountId is
interpolated directly into the query passed to stripeClient.customers.search;
sanitize/validate accountId before building the query used in
resolveStripeCustomerForAccount.ts. Fix by normalizing accountId (prefer a
whitelist: allow only expected chars like [A-Za-z0-9-_] or, if broader values
required, escape special characters) and/or escaping backslashes and single
quotes (replace "\" with "\\\\" and "'" with "\\'") before constructing the
query string for stripeClient.customers.search so queries like `query:
\`metadata['accountId']:'${sanitizedAccountId}'\`` cannot be broken by
malicious/odd input.
Surfaced during preview testing of PR 2a: two back-to-back sub Checkout calls for a brand-new accountId created TWO Customers instead of reusing the first one. The race is exactly the one the helper's JSDoc warned about — Stripe's customer search index is eventually consistent (~60s lag), so call #2's search misses the Customer that call #1 just created. Fix: pass a deterministic Stripe idempotency key derived from accountId to `customers.create`. Within Stripe's 24-hour idempotency window, any two creates with the same key resolve to the SAME Customer — Stripe dedupes server-side. The search-lag race is now harmless: even when both calls hit the empty-search branch, the underlying create call collapses to one Customer. Behavior change is invisible outside the race scenario. For normal user behavior (sign up, return days later) the 24-hour window has long expired and everything works as before. Tests: existing test now asserts the idempotency key is passed; new test asserts two back-to-back calls for the same accountId send the same idempotency key. Full suite: 470 files / 2761 tests green; lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sweetmantech
commented
May 12, 2026
|
|
||
| const created = await stripeClient.customers.create( | ||
| { metadata: { accountId } }, | ||
| { idempotencyKey: `customer-create-account-${accountId}` }, |
Contributor
Author
There was a problem hiding this comment.
KISS - why not simplify to just
Suggested change
| { idempotencyKey: `customer-create-account-${accountId}` }, | |
| { idempotencyKey: accountId }, |
Per sweetman review on PR #555 (KISS): the prefixed `customer-create-account-${accountId}` was ceremony — the accountId itself is already globally unique within our system, and Stripe's idempotency keys are scoped per-account-per-resource. Dropping the prefix has identical dedupe semantics with less noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 2a of 4 in the credits-auto-charge rollout (docs ✅ → api 2a (this PR) → run backfill → open-agents tolerant UI → api 2b auto-charge handler).
Prepares for the auto-charge path by ensuring every Stripe Customer linked to a Recoup account has a discoverable
metadata.accountId. The upcoming auto-charge flow (PR 2b) usesstripe.customers.searchto find a card on file — which only works for Customers stamped with the metadata.Zero behavior change to public APIs. The credits flow still always returns Checkout URLs; the subscription flow still produces working Checkout sessions. Internally:
lib/stripe/resolveStripeCustomerForAccount.ts(new) — looks up a Customer viacustomers.searchbymetadata['accountId']; if no match, creates one stamped withmetadata.accountId. Used by the subscription Checkout flow today and the credits auto-charge flow in PR 2b.createStripeSession.ts— subscription Checkout now resolves the Customer first and passescustomer:to Stripe. New subscribers from this PR onward get findable Customers automatically. (Previously Stripe auto-created a guest-style Customer with no metadata.)scripts/backfill-subscriber-customer-metadata.ts(new, one-time) — iteratesstripe.subscriptions.listand stampsmetadata.accountIdon each subscription's Customer if missing. Idempotent, supports--dry-run, skips deleted Customers, logs conflicts without overwriting. Pure decision functionshouldStampCustomerMetadatais extracted with its own test file.TDD trail
lib/stripe/__tests__/resolveStripeCustomerForAccount.test.tslib/stripe/__tests__/createStripeSession.test.ts(updated)scripts/__tests__/shouldStampCustomerMetadata.test.tsLocal:
pnpm test→ 2763 / 2763 ✅,pnpm lint:checkclean, typecheck clean.Required action before PR 2b can ship safely
Run the backfill against prod Stripe once this deploys:
The script logs a stats summary at the end:
A non-zero
customersWithConflictis logged per-record and doesn't fail the run — it indicates a Customer whosemetadata.accountIdalready exists but disagrees with the subscription's. Worth manual review if any appear.Test plan (preview)
POST /api/subscriptions/sessionsproduces a session that passescustomer: cus_…to Stripe (verify in Stripe Dashboard → Sessions → that session has a Customer attached, and that the Customer hasmetadata.accountIdset)POST /api/credits/sessionsstill works exactly as before (no behavior change)checkout.session.completedFollow-ups
payment_intent.succeededwebhook🤖 Generated with Claude Code
Summary by cubic
Stamp
metadata.accountIdon Stripe Customers and reuse the resolved Customer in subscription Checkout. Prevent duplicate Customers by usingaccountIdas the idempotency key; one-time backfill script removed after running in prod; no public API changes.New Features
resolveStripeCustomerForAccountto find-or-create Customers bymetadata.accountIdviacustomers.search.createStripeSessionnow resolves the Customer and passescustomerto Checkout.Bug Fixes
accountIdas the deterministic idempotency key oncustomers.createto prevent duplicates when the search index lags.Written for commit 1234882. Summary will update on new commits.
Summary by CodeRabbit