Better Auth - Backfill scripts - #4285
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds three Better Auth migration cron routes and a script that triggers them through QStash. Each route processes records in batches, schedules follow-up work, and supports completion verification through Prisma counts. ChangesBetter Auth migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Script as migrate.ts
participant QStash
participant Route as Better Auth cron route
participant Prisma
Script->>QStash: Publish migration request
QStash->>Route: Invoke POST endpoint
Route->>Prisma: Process one migration batch
Prisma-->>Route: Return batch result
Route->>QStash: Schedule next batch if needed
Script->>Prisma: Verify remaining records
Prisma-->>Script: Return migration counts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/scripts/better-auth/migrate.ts (1)
8-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a CLI argument instead of commented-out calls.
Lines 9-11 comment out the three migration triggers. In the committed state,
migrateUsers,migrateAccounts, andmigrateCredentialsare unreachable. The three functions also differ only by route path and flow control key. A single helper plus an argument switch removes both problems.♻️ Proposed refactor
-async function main() { - // await migrateUsers(); - // await migrateAccounts(); - // await migrateCredentials(); - await verifyMigration(); -} - -async function migrateUsers() { - const qstashResponse = await qstash.publishJSON({ - method: "POST", - url: `${APP_DOMAIN_WITH_NGROK}/api/cron/better-auth/migrate-users`, - retries: 0, - flowControl: { - key: "better-auth-migrate-users", - parallelism: 1, - }, - }); - - console.log(`migrateUsers executed: ${qstashResponse.messageId}`); -} - -async function migrateAccounts() { - const qstashResponse = await qstash.publishJSON({ - method: "POST", - url: `${APP_DOMAIN_WITH_NGROK}/api/cron/better-auth/migrate-accounts`, - retries: 0, - flowControl: { - key: "better-auth-migrate-accounts", - parallelism: 1, - }, - }); - - console.log(`migrateAccounts executed: ${qstashResponse.messageId}`); -} - -async function migrateCredentials() { - const qstashResponse = await qstash.publishJSON({ - method: "POST", - url: `${APP_DOMAIN_WITH_NGROK}/api/cron/better-auth/migrate-credentials`, - retries: 0, - flowControl: { - key: "better-auth-migrate-credentials", - parallelism: 1, - }, - }); - - console.log(`migrateCredentials executed: ${qstashResponse.messageId}`); -} +const STEPS = ["users", "accounts", "credentials"] as const; +type Step = (typeof STEPS)[number]; + +async function main() { + const step = process.argv[2]; + + if (STEPS.includes(step as Step)) { + await trigger(step as Step); + return; + } + + await verifyMigration(); +} + +async function trigger(step: Step) { + const { messageId } = await qstash.publishJSON({ + method: "POST", + url: `${APP_DOMAIN_WITH_NGROK}/api/cron/better-auth/migrate-${step}`, + retries: 0, + flowControl: { + key: `better-auth-migrate-${step}`, + parallelism: 1, + }, + }); + + console.log(`migrate-${step} triggered: ${messageId}`); +}🤖 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/web/scripts/better-auth/migrate.ts` around lines 8 - 55, Replace the commented-out calls in main with a CLI argument switch that selects users, accounts, or credentials migration, while preserving verifyMigration as the default or explicit option as appropriate. Consolidate migrateUsers, migrateAccounts, and migrateCredentials into one parameterized migration helper that derives the endpoint route and flow-control key from the selected migration type, and invoke it from main.
🤖 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/web/app/`(ee)/api/cron/better-auth/migrate-credentials/route.ts:
- Around line 33-48: In the migration loop after prisma.account.createMany, add
the same count === 0 progress guard used by migrate-users and migrate-accounts:
call logAndRespond with an error response before continuing or scheduling
another batch. Confirm and use the existing second-argument convention for
marking errors, while preserving normal logging and completion behavior when
count is positive.
In `@apps/web/app/`(ee)/api/cron/better-auth/migrate-users/route.ts:
- Around line 7-13: Add the deployment plan's 300-second maxDuration export
beside the existing dynamic export in
apps/web/app/(ee)/api/cron/better-auth/migrate-users/route.ts (lines 7-13),
migrate-accounts/route.ts (lines 7-13), and migrate-credentials/route.ts (lines
7-13); no other changes are required.
---
Nitpick comments:
In `@apps/web/scripts/better-auth/migrate.ts`:
- Around line 8-55: Replace the commented-out calls in main with a CLI argument
switch that selects users, accounts, or credentials migration, while preserving
verifyMigration as the default or explicit option as appropriate. Consolidate
migrateUsers, migrateAccounts, and migrateCredentials into one parameterized
migration helper that derives the endpoint route and flow-control key from the
selected migration type, and invoke it from main.
🪄 Autofix
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 Plus
Run ID: 3c7e6b39-bd29-438c-80a1-f37f7501800f
📒 Files selected for processing (4)
apps/web/app/(ee)/api/cron/better-auth/migrate-accounts/route.tsapps/web/app/(ee)/api/cron/better-auth/migrate-credentials/route.tsapps/web/app/(ee)/api/cron/better-auth/migrate-users/route.tsapps/web/scripts/better-auth/migrate.ts
| if (users.length === 0) { | ||
| return logAndRespond("Finished migrating credentials."); | ||
| } | ||
|
|
||
| const { count } = await prisma.account.createMany({ | ||
| skipDuplicates: true, | ||
| data: users.map((user) => ({ | ||
| userId: user.id, | ||
| accountId: user.id, | ||
| providerId: "credential", | ||
| password: user.passwordHash, | ||
| })), | ||
| }); | ||
|
|
||
| console.log(`Migrated ${count} credentials.`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Add a progress guard. The batch can repeat forever.
createMany uses skipDuplicates: true. The returned count is logged but never checked. If count is 0 while users.length is greater than 0, the findMany filter still matches the same users on the next iteration. The loop then processes the same 1000 rows 10 times and schedules another QStash message. retries: 0 does not stop this, because each run publishes a new message. The chain repeats every 10 seconds without end.
migrate-users and migrate-accounts both exit on count === 0. This route needs the same guard.
🐛 Proposed fix
const { count } = await prisma.account.createMany({
skipDuplicates: true,
data: users.map((user) => ({
userId: user.id,
accountId: user.id,
providerId: "credential",
password: user.passwordHash,
})),
});
console.log(`Migrated ${count} credentials.`);
+
+ // No rows inserted while candidates remain means the batch cannot progress.
+ if (count === 0) {
+ return logAndRespond(
+ `Stopped migrating credentials: ${users.length} users matched but no accounts were created.`,
+ true,
+ );
+ }
}Confirm the second argument of logAndRespond marks an error response before you apply it.
🤖 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/web/app/`(ee)/api/cron/better-auth/migrate-credentials/route.ts around
lines 33 - 48, In the migration loop after prisma.account.createMany, add the
same count === 0 progress guard used by migrate-users and migrate-accounts: call
logAndRespond with an error response before continuing or scheduling another
batch. Confirm and use the existing second-argument convention for marking
errors, while preserving normal logging and completion behavior when count is
positive.
| export const dynamic = "force-dynamic"; | ||
|
|
||
| const BATCH_SIZE = 1000; | ||
| const ITERATIONS = 10; | ||
|
|
||
| // POST /api/cron/better-auth/migrate-users | ||
| export const POST = withCron(async () => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No migration route declares maxDuration. All three routes run up to 10 sequential batches of 1000 records in a single request but export only dynamic = "force-dynamic". If the platform default timeout ends a request, retries: 0 prevents a QStash retry, and the migration chain stops without publishing the follow-up message.
apps/web/app/(ee)/api/cron/better-auth/migrate-users/route.ts#L7-L13: addexport const maxDuration = 300;next to thedynamicexport.apps/web/app/(ee)/api/cron/better-auth/migrate-accounts/route.ts#L7-L13: add the samemaxDurationexport; the rawUPDATE ... LIMIT 1000loop carries the same timeout risk.apps/web/app/(ee)/api/cron/better-auth/migrate-credentials/route.ts#L7-L13: add the samemaxDurationexport; this route runs two queries per iteration, so it is the slowest of the three.
Match the value to the plan limit for this deployment.
📍 Affects 3 files
apps/web/app/(ee)/api/cron/better-auth/migrate-users/route.ts#L7-L13(this comment)apps/web/app/(ee)/api/cron/better-auth/migrate-accounts/route.ts#L7-L13apps/web/app/(ee)/api/cron/better-auth/migrate-credentials/route.ts#L7-L13
🤖 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/web/app/`(ee)/api/cron/better-auth/migrate-users/route.ts around lines 7
- 13, Add the deployment plan's 300-second maxDuration export beside the
existing dynamic export in
apps/web/app/(ee)/api/cron/better-auth/migrate-users/route.ts (lines 7-13),
migrate-accounts/route.ts (lines 7-13), and migrate-credentials/route.ts (lines
7-13); no other changes are required.
Inherit Account.id_token restore from the schema PR.
Summary by CodeRabbit