[Feat] new scripts on migrate/seed/init run for internal#1421
[Feat] new scripts on migrate/seed/init run for internal#1421
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a deploy-time backfill script that automatically grants the internal free plan to all qualifying billing teams in Stack Auth's internal tenancy. It changes ChangesInternal Free Plan Backfill
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 |
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.
Greptile SummaryThis PR adds two deploy-time idempotent scripts —
Confidence Score: 5/5Safe to merge; both scripts are idempotent, failures are isolated per-team/sub, and all migration entry points get the correct ordering. The dual-write ordering (Stripe first, then Prisma+Bulldozer) is correct and self-healing. The retry path after a partial Bulldozer failure works because getSubscriptionMapForCustomer reads from Bulldozer, so a stale Bulldozer row causes the DB branch to re-enter on the next run. Fail-fast misconfiguration checks and comprehensive integration tests covering all code paths give high confidence. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant M as db-migrations.ts
participant B as runBulldozerPaymentsInit
participant FP as runBackfillInternalFreePlans
participant RG as runRegenInternalSubscriptionsToLatest
participant DB as Prisma/Postgres
participant BUL as Bulldozer
participant STR as Stripe API
M->>B: await runBulldozerPaymentsInit
B-->>M: done
M->>FP: await runBackfillInternalFreePlans
loop per team
FP->>BUL: getSubscriptionMapForCustomer
alt no occupying sub
FP->>DB: retryTransaction(SERIALIZABLE)
FP->>BUL: bulldozerWriteSubscription
end
end
FP-->>M: granted/failed/total
M->>RG: await runRegenInternalSubscriptionsToLatest
loop per team per sub
RG->>BUL: getSubscriptionMapForCustomer
alt stale Stripe pointer
RG->>STR: subscriptions.update
end
alt stale DB snapshot
RG->>DB: retryTransaction
RG->>BUL: bulldozerWriteSubscription
end
end
RG-->>M: Counters
Reviews (2): Last reviewed commit: "feat: new scripts on migrate/seed/init r..." | Re-trigger Greptile |
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
bc1883f to
28ad379
Compare
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
|
@greptileai please rereview, and read the updated PR description |
28ad379 to
f5389ae
Compare
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.
f5389ae to
ca2d729
Compare
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
| "wait-until-postgres-is-ready": "command -v pg_isready >/dev/null 2>&1 && pnpm run wait-until-postgres-is-ready:pg_isready || sleep 10 # not everyone has pg_isready installed, so we fallback to sleeping", | ||
| "wait-until-clickhouse-is-ready": "pnpm exec wait-on http://localhost:${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}36/ping", | ||
| "start-deps:no-delay": "pnpm pre && pnpm run deps-compose up --detach --build && pnpm run wait-until-postgres-is-ready && pnpm run wait-until-clickhouse-is-ready && pnpm run db:init && echo \"\\nDependencies started in the background as Docker containers. 'pnpm run stop-deps' to stop them\"n", | ||
| "start-deps:no-delay": "pnpm pre && pnpm run deps-compose up --detach --build && pnpm run wait-until-postgres-is-ready && pnpm run wait-until-clickhouse-is-ready && pnpm run db:init && pnpm run db:backfill-internal-free-plans && echo \"\\nDependencies started in the background as Docker containers. 'pnpm run stop-deps' to stop them\"n", |
There was a problem hiding this comment.
Stray n outside the closing quote in the echo command.
After JSON-unescaping, the shell sees:
echo "\nDependencies started in the background as Docker containers. 'pnpm run stop-deps' to stop them"n
Bash performs adjacent-token concatenation, so echo receives a single argument ending in ...stop themn, which prints as ...stop themn instead of the intended message. The n is a leftover fragment of a \n escape.
🐛 Proposed fix
- "start-deps:no-delay": "pnpm pre && pnpm run deps-compose up --detach --build && pnpm run wait-until-postgres-is-ready && pnpm run wait-until-clickhouse-is-ready && pnpm run db:init && pnpm run db:backfill-internal-free-plans && echo \"\\nDependencies started in the background as Docker containers. 'pnpm run stop-deps' to stop them\"n",
+ "start-deps:no-delay": "pnpm pre && pnpm run deps-compose up --detach --build && pnpm run wait-until-postgres-is-ready && pnpm run wait-until-clickhouse-is-ready && pnpm run db:init && pnpm run db:backfill-internal-free-plans && echo \"\\nDependencies started in the background as Docker containers. 'pnpm run stop-deps' to stop them\"",🤖 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 `@package.json` at 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).
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 allow for users to be able to migrate teams to latest version of their product.
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
Summary by CodeRabbit
New Features
Chores