[Feat] new scripts on migrate/seed/init run for internal#1421
Open
nams1570 wants to merge 2 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a bounded, batched backfill that grants the internal free plan to qualifying internal teams; changes ChangesInternal Free Plan Backfill
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds deploy-time/internal maintenance scripts to ensure Internal tenancy billing teams always have a baseline free subscription and that existing internal subscriptions’ product snapshots (and Stripe metadata pointers for Stripe-backed subs) are refreshed to the latest product config. It wires these scripts into the backend DB lifecycle commands so dev/prod environments converge on consistent billing state.
Changes:
- Add
backfill-internal-free-plans.tsto grant thefreeplan to orphaned internal billing teams, and run it ondb-migrationsreset/seed/init/migrate. - Add
regen-internal-subscriptions-to-latest.tsto rebase internal subscriptions onto the latest product snapshot (and Stripe metadataproductVersionIdwhere applicable), and run it on the same DB lifecycle commands. - Update
ensureFreePlanForBillingTeamto return a boolean indicating whether it actually inserted a new free-plan subscription row, and expand real-DB test coverage for both scripts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts | Adds real-DB tests covering snapshot regeneration, Stripe-metadata rebasing behavior, failure isolation, and Bulldozer downstream effects. |
| apps/backend/src/lib/payments/ensure-free-plan.ts | Changes ensureFreePlanForBillingTeam to return boolean and documents return semantics for deploy-time backfills. |
| apps/backend/src/lib/payments/ensure-free-plan.test.ts | Updates tests to assert the new boolean return value and adds a regression test for ended-only subscriptions. |
| apps/backend/scripts/regen-internal-subscriptions-to-latest.ts | Introduces the internal subscription regeneration script (DB snapshot + Stripe metadata rebase + ProductVersion upsert). |
| apps/backend/scripts/db-migrations.ts | Runs the new internal-tenancy backfills during reset/seed/init/migrate flows (after Bulldozer init). |
| apps/backend/scripts/backfill-internal-free-plans.ts | Introduces the internal free-plan backfill script with keyset pagination and per-team failure isolation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Greptile SummaryThis PR adds two idempotent migration scripts —
Confidence Score: 5/5Safe to merge — both scripts are idempotent, isolated to the internal billing tenancy, and the write ordering (Stripe-first, then DB) correctly handles partial failures via self-healing on the next run. The core correctness concern from a previous review (whether a retry actually re-issues a stale Bulldozer write) was checked against the read path: getSubscriptionMapForCustomer reads from the Bulldozer LFold, not from Prisma, so a failed bulldozerWriteSubscription leaves the LFold with the old snapshot and the next run correctly re-enters the write branch. Both scripts have real-DB integration tests covering all branching paths. The only findings are observability gaps (counter state under partial failure, exit-code 0 on full failure) that do not affect migration correctness. No files require special attention beyond the observability notes left inline on regen-internal-subscriptions-to-latest.ts and db-migrations.ts. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as db-migrations CLI
participant BInit as runBulldozerPaymentsInit
participant BF as runBackfillInternalFreePlans
participant Regen as runRegenInternalSubscriptionsToLatest
participant EFP as ensureFreePlanForBillingTeam
participant Prisma as Prisma DB
participant Bulldozer as Bulldozer LFold
participant Stripe as Stripe API
CLI->>BInit: await (Bulldozer LFold populated)
BInit-->>CLI: done
CLI->>BF: await
BF->>Bulldozer: getSubscriptionMapForCustomer (per team)
Bulldozer-->>BF: subMap
BF->>EFP: ensureFreePlanForBillingTeam(teamId)
EFP->>Prisma: SERIALIZABLE tx — insert subscription
Prisma-->>EFP: createdSub
EFP->>Bulldozer: bulldozerWriteSubscription
Bulldozer-->>EFP: ok
EFP-->>BF: true (granted)
BF-->>CLI: "{ granted, failed, total }"
CLI->>Regen: await
Regen->>Bulldozer: getSubscriptionMapForCustomer (per team)
Bulldozer-->>Regen: sub (with stored product snapshot)
Note over Regen: compare sub.product vs latestProduct
Regen->>Stripe: subscriptions.retrieve (Stripe-backed only)
Stripe-->>Regen: existing metadata
Regen->>Prisma: upsertProductVersion (content-hash, idempotent)
Regen->>Stripe: subscriptions.update metadata (Stripe-first)
Stripe-->>Regen: ok
Regen->>Prisma: retryTransaction — subscription.update product snapshot
Prisma-->>Regen: updated
Regen->>Bulldozer: bulldozerWriteSubscription
Bulldozer-->>Regen: ok
Regen-->>CLI: Counters
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/backend/scripts/regen-internal-subscriptions-to-latest.ts:521-551
**Counter state diverges when Stripe succeeds but the DB write throws**
`counters.stripeMetadataWrites` is incremented at line 521 before the Prisma/Bulldozer block. If `bulldozerWriteSubscription` (or even `retryTransaction`) then throws, the outer catch fires and increments `counters.skippedFailures` — but `counters.dbWrites` and `counters.mutated` are never reached. The final summary log would show `stripeMetadataWrites=1, dbWrites=0, mutated=0, skippedFailures=1` for that sub, making it look like no mutation happened when one actually did. The inverse case also holds: if the DB path completes but only Bulldozer fails, `counters.dbWrites` hasn't been incremented yet either (it sits after `bulldozerWriteSubscription` at line 547). Consider incrementing `dbWrites` right after the Prisma commit and `mutated` at the top of the DB/Stripe write section rather than at the very end.
### Issue 2 of 2
apps/backend/scripts/db-migrations.ts:239-256
**Partial failures exit with code 0**
Both `runBackfillInternalFreePlans()` and `runRegenInternalSubscriptionsToLatest()` swallow per-item errors internally and return a counters object. The callers here discard the return value entirely, so even a run where every team fails (e.g. a transient DB outage during the deploy window) exits with code 0. Any CI/CD pipeline gating on exit code would consider the step a success. The PR design deliberately accepts partial failures and retries on the next deploy, so this is a known trade-off — but if the intent is to surface systemic failures, checking `result.failed > 0` and optionally throwing (or at least printing a prominent warning to stderr) would make the gap more visible.
Reviews (3): Last reviewed commit: "feat(migration): Migrate internal teams ..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/backend/scripts/db-migrations.ts (1)
207-245: 💤 Low valueSequencing and idempotency rationale are well-documented.
Order (Bulldozer init → free-plan backfill → regen) is correct given the stated dependencies (regen dual-writes into stored tables; free-plan backfill reads the Subscription LFold). Both helpers are documented as idempotent so running on every deploy and every dev DB lifecycle is safe.
Minor optional thought: the same trio
runBulldozerPaymentsInit / runBackfillInternalFreePlans / runRegenInternalSubscriptionsToLatestis repeated verbatim in 4 branches. A smallrunPostMigrationInternalSetup()helper would deduplicate and ensure future steps stay in lockstep across branches — but the explicitness has its own value, so deferable.🤖 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/backend/scripts/db-migrations.ts` around lines 207 - 245, The repeated trio of calls runBulldozerPaymentsInit(globalPrismaClient), runBackfillInternalFreePlans(), and runRegenInternalSubscriptionsToLatest() should be extracted into a single helper (e.g., runPostMigrationInternalSetup) to remove duplication and keep future changes in one place; create a function runPostMigrationInternalSetup that invokes those three helpers (accepting any needed args like globalPrismaClient) and replace the verbatim calls in the branches ('generate-migration-file', 'seed', 'init', 'migrate' and the earlier default/init branch) with a single await runPostMigrationInternalSetup(...) call.apps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts (1)
225-225: ⚡ Quick winReplace
Object.keys(...)[0]!with?? throwErr(...)per coding guidelines.Lines 225, 310, 412, and 462 all use the
Object.keys(growth.includedItems)[0]!pattern. Ifgrowthever ships withoutincludedItems(config drift, future seed change, etc.),withoutItem(growth, undefined)would no-op and the test would silently pass on a non-stale snapshot rather than flagging the violated precondition.♻️ Suggested refactor (illustrative)
- const stale = withoutItem(growth, Object.keys(growth.includedItems)[0]!); + const someItemId = Object.keys(growth.includedItems)[0] + ?? throwErr("growth has no includedItems to drop"); + const stale = withoutItem(growth, someItemId);Apply at lines 225, 310, 412, and 462. Line 154 already extracts
someItemIdvia an explicitlength === 0throw — that pattern is preferred here too.As per coding guidelines: "Code defensively and prefer
?? throwErr(...)over non-null assertions, with explicit error messages stating the violated assumption".🤖 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/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts` at line 225, Replace the non-null assertion usage Object.keys(growth.includedItems)[0]! with a defensive null-check that throws when the assumption is violated: obtain the first key from Object.keys(growth.includedItems) and if it is undefined call throwErr(...) with a clear message (e.g., "expected growth.includedItems to contain at least one item") before passing it to withoutItem; apply this change for each occurrence that uses Object.keys(growth.includedItems)[0]! (the call sites around withoutItem and any related test setup) so tests will fail fast on missing includedItems instead of silently no-oping.
🤖 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/backend/scripts/db-migrations.ts`:
- Around line 207-245: The repeated trio of calls
runBulldozerPaymentsInit(globalPrismaClient), runBackfillInternalFreePlans(),
and runRegenInternalSubscriptionsToLatest() should be extracted into a single
helper (e.g., runPostMigrationInternalSetup) to remove duplication and keep
future changes in one place; create a function runPostMigrationInternalSetup
that invokes those three helpers (accepting any needed args like
globalPrismaClient) and replace the verbatim calls in the branches
('generate-migration-file', 'seed', 'init', 'migrate' and the earlier
default/init branch) with a single await runPostMigrationInternalSetup(...)
call.
In `@apps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts`:
- Line 225: Replace the non-null assertion usage
Object.keys(growth.includedItems)[0]! with a defensive null-check that throws
when the assumption is violated: obtain the first key from
Object.keys(growth.includedItems) and if it is undefined call throwErr(...) with
a clear message (e.g., "expected growth.includedItems to contain at least one
item") before passing it to withoutItem; apply this change for each occurrence
that uses Object.keys(growth.includedItems)[0]! (the call sites around
withoutItem and any related test setup) so tests will fail fast on missing
includedItems instead of silently no-oping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 039860b7-acba-44d0-8a07-864abff8843e
📒 Files selected for processing (6)
apps/backend/scripts/backfill-internal-free-plans.tsapps/backend/scripts/db-migrations.tsapps/backend/scripts/regen-internal-subscriptions-to-latest.tsapps/backend/src/lib/payments/ensure-free-plan.test.tsapps/backend/src/lib/payments/ensure-free-plan.tsapps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts
nams1570
force-pushed
the
migrate-plans-and-products-ui
branch
from
bc1883f to
28ad379
Compare
Contributor
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 `@apps/backend/scripts/backfill-internal-free-plans.ts`:
- Around line 95-99: The loop currently awaits each
ensureFreePlanForBillingTeam(teamId) sequentially which slows large backfills;
change to a bounded-concurrency pattern (small worker pool) so multiple teamIds
are processed in parallel while limiting DB pressure: use iterateInternalTeamIds
to produce IDs and dispatch ensureFreePlanForBillingTeam(teamId) into a
concurrency limiter (e.g., p-limit or a simple semaphore) sized to a small
constant (e.g., 5–20), await all inflight promises before finishing each batch,
and preserve/error-handle incrementing total and granted counters inside each
task; keep TEAM_BATCH_SIZE for outer batching if desired.
🪄 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: 809a0dc1-fe89-4cae-b273-28fc16a747a2
📒 Files selected for processing (6)
apps/backend/scripts/backfill-internal-free-plans.tsapps/backend/scripts/db-migrations.tsapps/backend/scripts/regen-internal-subscriptions-to-latest.tsapps/backend/src/lib/payments/ensure-free-plan.test.tsapps/backend/src/lib/payments/ensure-free-plan.tsapps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/backend/src/lib/payments/ensure-free-plan.test.ts
- apps/backend/scripts/db-migrations.ts
- apps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.ts
- apps/backend/scripts/regen-internal-subscriptions-to-latest.ts
Collaborator
Author
|
@greptileai please rereview, and read the updated PR description |
nams1570
force-pushed
the
migrate-plans-and-products-ui
branch
from
28ad379 to
f5389ae
Compare
nams1570
force-pushed
the
migrate-plans-and-products-ui
branch
from
f5389ae to
ca2d729
Compare
Contributor
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 `@package.json`:
- Line 39: The package.json script "start-deps:no-delay" contains a stray
trailing "n" after the closing quote in the echo command, causing the printed
message to end with "n"; open the "start-deps:no-delay" script and remove the
extraneous trailing "n" so the echo argument ends with the closing quote only
(i.e., ensure the echo string is "\\nDependencies started... 'pnpm run
stop-deps' to stop them" with no characters after the final quote).
🪄 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: 378403af-9aba-4bd2-9924-c53d116b7a9b
📒 Files selected for processing (6)
apps/backend/package.jsonapps/backend/scripts/backfill-internal-free-plans.tsapps/backend/scripts/db-migrations.tsapps/backend/src/lib/payments/ensure-free-plan.test.tsapps/backend/src/lib/payments/ensure-free-plan.tspackage.json
✅ Files skipped from review due to trivial changes (2)
- apps/backend/package.json
- apps/backend/src/lib/payments/ensure-free-plan.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/backend/scripts/backfill-internal-free-plans.ts
- apps/backend/src/lib/payments/ensure-free-plan.test.ts
- apps/backend/scripts/db-migrations.ts
BilalG1
approved these changes
May 7, 2026
This gives every team on internal proj w/o a plan the free plan. It is idempotent - only gives a plan if team has no other plans in the pricing plans product line.
nams1570
force-pushed
the
migrate-plans-and-products-ui
branch
from
ca2d729 to
b6ea8a5
Compare
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Contributor
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 `@apps/backend/scripts/regen-internal-subscriptions-to-latest.ts`:
- Around line 163-168: The current code casts getStripeForAccount's result with
"as unknown as Promise<StripeClientForRegen>", which circumvents TypeScript
safety; replace the cast by adding a small async wrapper (e.g.,
getStripeClientForRegen) that calls getStripeForAccount({ tenancy:
internalTenancy }), extracts and returns only the subscriptions API as a
StripeClientForRegen shape, and then set getStripe to initialize stripePromise
via getStripeClientForRegen(internalTenancy) when options.stripeClient is null;
this removes the double type-cast and makes intent explicit while keeping usage
in getStripe unchanged.
🪄 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: f12c1538-5bba-483b-8e7f-4a727bd96a8b
📒 Files selected for processing (8)
apps/backend/package.jsonapps/backend/scripts/backfill-internal-free-plans.tsapps/backend/scripts/db-migrations.tsapps/backend/scripts/regen-internal-subscriptions-to-latest.tsapps/backend/src/lib/payments/ensure-free-plan.test.tsapps/backend/src/lib/payments/ensure-free-plan.tsapps/backend/src/scripts/regen-internal-subscriptions-to-latest.test.tspackage.json
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backend/package.json
- apps/backend/src/lib/payments/ensure-free-plan.test.ts
- apps/backend/src/lib/payments/ensure-free-plan.ts
- apps/backend/scripts/db-migrations.ts
- apps/backend/scripts/backfill-internal-free-plans.ts
Stripe subscriptions need to have their product version updated in the stripe metadata, else on stripe webhook event they will be reset back to old prod snapshot. These two scripts will need to be run explicitly in prod. They're not part of the db:migrate script.
nams1570
force-pushed
the
migrate-plans-and-products-ui
branch
from
b6ea8a5 to
f813275
Compare
Collaborator
Author
|
@greptileai please do a full rereview |
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
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.
Context
One script grants free plan to any team which is a customer of the internal project who doesnt have it already.
We also want to migrate our users (internal) to the latest version of their products.
Needed because some subs on dev right now dont have a plan. And internal isnt using latest version of its own growth plan.
Describing the Paths we want to Account for
Deployment strategy in prod
Run the backfill and the regen scripts once each after your migrations on the prod db.
pnpm db:backfill-internal-free-planswill make sure every team has a free plan at least if they dont have an existing plan (and it is idempotent).After that, run
pnpm db:regen-internal-subscriptions-to-latestwhich will migrate every user to the latest version of their plan (i.e latest snapshot). This should also be idempotent.Summary by CodeRabbit
New Features
Chores
Tests