Repository navigation
fix(billing): converge Stripe subscription syncs and stop webhook echoes overwriting unsynced changes - #8764
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
We detected this is a high-risk PR and are running a free ultrareview. An ultrareview is a deeper, multi-pass review that catches hard-to-find bugs a standard review can miss. We'll post the findings when it completes. This PR appears to change concurrency-sensitive code such as locks, queues, or retries, where a missed race may only surface under production load, so a deeper multi-pass review is worth running. Want an ultrareview on every high-risk PR? Set up automated ultrareviews. |
|
There was a problem hiding this comment.
We detected this is a high-risk PR and ran a free ultrareview. An ultrareview is a deeper, multi-pass review that catches hard-to-find bugs a standard review can miss.
This PR appears to change concurrency-sensitive code such as locks, queues, or retries, where a missed race may only surface under production load, so a deeper multi-pass review is worth running.
Want an ultrareview on every high-risk PR? Set up automated ultrareviews.
2 issues found across 22 files
Confidence score: 3/5
- In
apps/sim/lib/auth/auth.ts, if the DB/outbox transaction fails after Stripe restores a subscription, the endpoint still succeeds and an older cancellation sync can re-cancel it. Keep the restore bookkeeping durable or make the failure retryable. - In
packages/testing/src/mocks/stripe.mock.ts,failNextRequestis documented as failing after the update, but it fails before processing. Move that description tofailNextUpdateAfterApplying.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/testing/src/mocks/stripe.mock.ts">
<violation number="1" location="packages/testing/src/mocks/stripe.mock.ts:343">
P3: `failNextRequest` is documented as applying the update before failing, but it fails before processing; move that description above `failNextUpdateAfterApplying`.</violation>
</file>
<file name="apps/sim/lib/auth/auth.ts">
<violation number="1" location="apps/sim/lib/auth/auth.ts:1110">
P1: This catch makes restore bookkeeping best-effort: if the DB/outbox transaction fails after Stripe has restored the subscription, the endpoint still succeeds and an older cancellation sync can later re-cancel it. Rethrow the error so the idempotent restore can be retried, or enqueue a durable recovery job.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
5e44efe to
26f7bc6
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Requires human review: Auto-approval blocked because this review re-detected 2 unresolved issues already reported by Cubic.
Turn on auto-fix | Re-trigger cubic
26f7bc6 to
2bd650a
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
2bd650a to
39f069b
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
…echoes over unsynced DB changes
…nt so no stale intent can resurface
…egacy echoes, customer restore)
… row lock; drop the settled-event scan
… older than a newer Sim commit
…convergence, bound the reconcile's outbox reads
…alue is applied, not replayed
…st the committed value, not a possibly-stale row
…e committed value already match
39f069b to
4f1c786
Compare
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 22 files
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
…nistic latest-sync test helper
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 22 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 22 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Turn on auto-fix | Re-trigger cubic
Summary
Sim syncs
cancel_at_period_end, Team seats and the customer contact from the DB to Stripe through outbox events, while the Better Auth Stripe plugin copiescancel_at_period_end/seatsfrom everycustomer.subscription.updatedback into the DB unconditionally. Five real convergence bugs followed — each reproduced against unmodified staging code first (real Postgres, the real plugin driven over HTTP, a fake Stripe that controls landing order):customer.subscription.updatedoverwrote a DB change Sim had committed but not yet pushed, and the pending sync then pushed the overwritten value (no outbox concurrency needed); a lost race was echoed back into the DB, so both systems agreed on the wrong valueoutbox:<eventId>with a different value and Stripe rejected it until dead-letterFix
cancelAtPeriodEnd/seatsrecords the value under the subscription row lock with a DB-clock timestamp onto every pending/processing/dead-lettered sync for that subscription (lib/billing/webhooks/subscription-sync.ts), so no event can carry a stale value — including requeued dead letters, Team activation, and Better Auth's/subscription/restore(via the existinghooks.after)onEventafter the plugin's write: while a Sim sync is in flight its latest committed value is restored; a change made in Stripe itself (portal, dashboard, Better Auth endpoints — detected by Sim's idempotency-key prefix) wins and supersedes in-flight values; with nothing in flight the live Stripe value is persisted (never the event payload), so out-of-order delivery can't regress it. Events from an older deploy leave the field as todayType of Change
Testing
stripe-sync-convergence.integration.ts(22 tests, real Postgres + the real Better Auth Stripe plugin over HTTP): each bug, every revival path (portal change then unrelated webhook, requeued dead letter, slow in-flight sync, enterprise follow-up retry, legacy deploy events, customer restore), Team-activation race, operator-retry deadlocks (deterministic lock interleaving), and a plan check that no reconcile query scans settled rows. Every test failed before its fix; every guard shown red when revertedbun run test:integrationbilling + outbox + auth adapter suites; rootbun run test;bun run lint,bun run type-check,bun run check:audits,bun run docs-manifest:checkKnown, intentional: a Stripe-dashboard seat edit made while a seat sync is in flight loses to Sim (Team seats follow the member count);
cancelAt/canceledAtstay as the plugin writes them.Checklist
test-auditauthoring gate)