From 7f19f4eeed982e938980d8fdb15c6e9df0493ef5 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:23 -0400 Subject: [PATCH 01/34] feat(migrate): port instance reads, readiness and checks from #479 at 7a820406 Copies these files from #479 (ra/integrate-migration-tool-into-cli) at 7a820406 unchanged, so the next commit shows exactly what slice 1 cuts. This commit does not build on its own. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/analysis.test.ts | 97 ++ .../src/commands/migrate/lib/analysis.ts | 88 ++ .../src/commands/migrate/lib/checks.test.ts | 712 ++++++++++++++ .../src/commands/migrate/lib/checks.ts | 894 ++++++++++++++++++ .../commands/migrate/lib/clerk-config.test.ts | 89 ++ .../src/commands/migrate/lib/clerk-config.ts | 193 ++++ .../migrate/lib/modify-settings.test.ts | 231 +++++ .../commands/migrate/lib/modify-settings.ts | 146 +++ .../commands/migrate/lib/readiness.test.ts | 281 ++++++ .../src/commands/migrate/lib/readiness.ts | 215 +++++ .../migrate/lib/supabase-providers.test.ts | 143 +++ .../migrate/lib/supabase-providers.ts | 145 +++ .../src/commands/migrate/lib/user-lookup.ts | 110 +++ 13 files changed, 3344 insertions(+) create mode 100644 packages/cli-core/src/commands/migrate/lib/analysis.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/analysis.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/checks.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/checks.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/clerk-config.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/modify-settings.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/modify-settings.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/readiness.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/readiness.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/supabase-providers.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/supabase-providers.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/user-lookup.ts diff --git a/packages/cli-core/src/commands/migrate/lib/analysis.test.ts b/packages/cli-core/src/commands/migrate/lib/analysis.test.ts new file mode 100644 index 000000000..322c2d753 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/analysis.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, test } from "bun:test"; +import { analyzeFields, hasValue } from "./analysis.ts"; + +describe("hasValue", () => { + test.each([ + ["a string", "x", true], + ["zero", 0, true], + ["false", false, true], + ["a populated array", ["a"], true], + ["an object", {}, true], + ["an empty string", "", false], + ["an empty array", [], false], + ["null", null, false], + ["undefined", undefined, false], + ])("treats %s as present: %p", (_label, value, expected) => { + expect(hasValue(value)).toBe(expected); + }); +}); + +describe("analyzeFields", () => { + test("returns zeroed counts for an empty file", () => { + const result = analyzeFields([]); + expect(result.totalUsers).toBe(0); + expect(result.identifiers.hasAnyIdentifier).toBe(0); + expect(result.fieldCounts).toEqual({}); + }); + + test("counts each identifier kind separately", () => { + const result = analyzeFields([ + { userId: "1", email: "a@x.dev" }, + { userId: "2", unverifiedEmailAddresses: ["b@x.dev"] }, + { userId: "3", phone: "+15555550100" }, + { userId: "4", unverifiedPhoneNumbers: ["+15555550101"] }, + { userId: "5", username: "carol" }, + ]); + + expect(result.identifiers).toMatchObject({ + verifiedEmails: 1, + unverifiedEmails: 1, + verifiedPhones: 1, + unverifiedPhones: 1, + username: 1, + hasAnyIdentifier: 5, + }); + }); + + test("counts emailAddresses towards verified emails", () => { + const result = analyzeFields([{ userId: "1", emailAddresses: ["a@x.dev"] }]); + expect(result.identifiers.verifiedEmails).toBe(1); + }); + + test("counts a user with several identifiers once", () => { + const result = analyzeFields([ + { userId: "1", email: "a@x.dev", phone: "+15555550100", username: "ada" }, + ]); + expect(result.identifiers.hasAnyIdentifier).toBe(1); + expect(result.identifiers.verifiedEmails).toBe(1); + expect(result.identifiers.verifiedPhones).toBe(1); + }); + + // These users cannot be imported under any instance configuration, which is + // what makes the count worth surfacing separately. + test("counts users carrying no identifier at all", () => { + const result = analyzeFields([ + { userId: "1", email: "a@x.dev" }, + { userId: "2", firstName: "Nobody" }, + { userId: "3" }, + ]); + expect(result.totalUsers).toBe(3); + expect(result.identifiers.hasAnyIdentifier).toBe(1); + }); + + test("counts the analyzed non-identifier fields", () => { + const result = analyzeFields([ + { userId: "1", email: "a@x.dev", firstName: "Ada", password: "d", totpSecret: "s" }, + { userId: "2", email: "b@x.dev", firstName: "Grace" }, + { userId: "3", email: "c@x.dev", lastName: "Hopper" }, + ]); + expect(result.fieldCounts).toEqual({ + firstName: 2, + lastName: 1, + password: 1, + totpSecret: 1, + }); + }); + + test("omits fields no user carries, rather than reporting them as zero", () => { + const result = analyzeFields([{ userId: "1", email: "a@x.dev" }]); + expect("password" in result.fieldCounts).toBe(false); + }); + + test("does not count an empty value as present", () => { + const result = analyzeFields([{ userId: "1", email: "a@x.dev", firstName: "", lastName: [] }]); + expect(result.fieldCounts.firstName).toBeUndefined(); + expect(result.fieldCounts.lastName).toBeUndefined(); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/analysis.ts b/packages/cli-core/src/commands/migrate/lib/analysis.ts new file mode 100644 index 000000000..be371127e --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/analysis.ts @@ -0,0 +1,88 @@ +/** + * Counts what an import file actually contains, per field. + * + * Ported from the standalone migration-tool's `src/lib/analysis.ts`. Runs on + * transformed-but-unvalidated users so the report describes the whole file, + * including the rows that will be skipped. + */ + +import type { User } from "../types.ts"; + +/** Non-identifier fields the readiness rows report coverage for. */ +export const ANALYZED_FIELDS = [ + { key: "firstName", label: "First name" }, + { key: "lastName", label: "Last name" }, + { key: "password", label: "Password" }, + { key: "totpSecret", label: "TOTP secret" }, +] as const; + +export type IdentifierCounts = { + verifiedEmails: number; + unverifiedEmails: number; + verifiedPhones: number; + unverifiedPhones: number; + username: number; + /** Users with any email, verified or not: each user counted once. */ + anyEmail: number; + /** Users with any phone, verified or not: each user counted once. */ + anyPhone: number; + /** Users with at least one identifier — the rest cannot be imported at all. */ + hasAnyIdentifier: number; +}; + +export type FieldAnalysis = { + identifiers: IdentifierCounts; + totalUsers: number; + fieldCounts: Record; +}; + +/** True for anything with real content — `0` and `false` count, `""` and `[]` do not. */ +export function hasValue(value: unknown): boolean { + if (value === undefined || value === null || value === "") return false; + if (Array.isArray(value)) return value.length > 0; + return true; +} + +export function analyzeFields(users: (User | Record)[]): FieldAnalysis { + const identifiers: IdentifierCounts = { + verifiedEmails: 0, + unverifiedEmails: 0, + verifiedPhones: 0, + unverifiedPhones: 0, + username: 0, + anyEmail: 0, + anyPhone: 0, + hasAnyIdentifier: 0, + }; + const fieldCounts: Record = {}; + + for (const entry of users) { + const user = entry as Record; + + for (const field of ANALYZED_FIELDS) { + if (hasValue(user[field.key])) { + fieldCounts[field.key] = (fieldCounts[field.key] ?? 0) + 1; + } + } + + const verifiedEmail = hasValue(user.email) || hasValue(user.emailAddresses); + const unverifiedEmail = hasValue(user.unverifiedEmailAddresses); + const verifiedPhone = hasValue(user.phone) || hasValue(user.phoneNumbers); + const unverifiedPhone = hasValue(user.unverifiedPhoneNumbers); + const username = hasValue(user.username); + + if (verifiedEmail) identifiers.verifiedEmails++; + if (unverifiedEmail) identifiers.unverifiedEmails++; + if (verifiedPhone) identifiers.verifiedPhones++; + if (unverifiedPhone) identifiers.unverifiedPhones++; + if (username) identifiers.username++; + if (verifiedEmail || unverifiedEmail) identifiers.anyEmail++; + if (verifiedPhone || unverifiedPhone) identifiers.anyPhone++; + + if (verifiedEmail || unverifiedEmail || verifiedPhone || unverifiedPhone || username) { + identifiers.hasAnyIdentifier++; + } + } + + return { identifiers, totalUsers: users.length, fieldCounts }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts new file mode 100644 index 000000000..858a609a1 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -0,0 +1,712 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +import type { User } from "../types.ts"; +import { checkImport, hashShapeProblem, passwordIsOnlySignIn, type CheckInput } from "./checks.ts"; + +const BCRYPT = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy"; + +const settings = (attributes: object, social: object = {}) => + ({ attributes, social }) as unknown as UserSettingsJSON; + +const EMAIL_REQUIRED = settings({ email_address: { enabled: true, required: true } }); + +const user = (userId: string, fields: Partial = {}): User => + ({ userId, email: `${userId}@x.dev`, ...fields }) as User; + +let originalFetch: typeof globalThis.fetch; +/** Users `GET /v1/users` finds, whatever the filter. */ +let existing: Record[]; + +beforeAll(() => { + originalFetch = globalThis.fetch; +}); + +afterAll(() => { + globalThis.fetch = originalFetch; +}); + +beforeEach(() => { + existing = []; + globalThis.fetch = (async (input: string | URL | Request) => { + const url = new URL(input.toString()); + // BAPI strips the `+` the lookup puts on each external_id. + const wanted = new Set([...url.searchParams.values()].map((value) => value.replace(/^\+/, ""))); + return Response.json( + existing.filter((candidate) => + [ + candidate.external_id, + candidate.username, + ...((candidate.email_addresses as { email_address: string }[] | undefined) ?? []).map( + (email) => email.email_address, + ), + ].some((value) => wanted.has(value as string)), + ), + ); + }) as typeof fetch; +}); + +function input(overrides: Partial = {}): CheckInput { + return { + users: [], + failures: [], + settings: null, + instanceType: "prod", + target: { + env: "production", + instanceId: "ins_1", + instanceType: "prod", + keySource: "--secret-key", + }, + secretKey: "sk_live_x", + schedule: async (fn) => fn(), + ...overrides, + }; +} + +const reasonsOf = async (overrides: Partial) => + Object.fromEntries( + (await checkImport(input(overrides))).rejects.map((reject) => [reject.sourceId, reject.reason]), + ); + +describe("rejects", () => { + // Each one is a user POST /v1/users refuses (clerk_go create_service.go). + describe("what the create refuses", () => { + const EMAIL_ON = { email_address: { enabled: true } }; + + test.each([ + ["first_name", { lastName: "L" }, "no first name, which this instance requires"], + ["last_name", { firstName: "F" }, "no last name, which this instance requires"], + ] as const)("a missing required %s", async (attribute, fields, reason) => { + const reasons = await reasonsOf({ + users: [user("a", fields), user("b", { firstName: "F", lastName: "L" })], + settings: settings({ ...EMAIL_ON, [attribute]: { enabled: true, required: true } }), + }); + expect(reasons).toEqual({ a: reason }); + }); + + test.each([ + [ + "totpSecret", + "authenticator_app", + "has an authenticator app (TOTP) secret, and this instance has authenticator apps off", + ], + ["backupCodes", "backup_code", "has backup codes, and this instance has backup codes off"], + ] as const)("%s with %s off, offering to turn it on", async (field, attribute, reason) => { + const value = field === "backupCodes" ? ["code1"] : "SECRET"; + const checks = await checkImport( + input({ + users: [user("a", { [field]: value })], + settings: settings({ ...EMAIL_ON, [attribute]: { enabled: false } }), + }), + ); + expect(checks.rejects).toEqual([{ sourceId: "a", reason }]); + expect(checks.fixes.map((fix) => fix.command).join("\n")).toContain( + `"auth_multi_factor":{"${attribute}":{"enabled":true}}`, + ); + }); + + test("no password where password is the only way to sign in", async () => { + const passwordOnly = settings({ + email_address: { enabled: true, used_for_first_factor: false, first_factors: [] }, + password: { enabled: true, used_for_first_factor: true, first_factors: ["password"] }, + }); + const reasons = await reasonsOf({ + users: [user("a"), user("b", { password: BCRYPT, passwordHasher: "bcrypt" })], + settings: passwordOnly, + }); + expect(reasons).toEqual({ + a: "no password, and password is this instance's only way to sign in", + }); + }); + + // Firebase or Supabase phone-auth users, into an instance with phone off. + test("a user left with no identifier once disabled ones are stripped", async () => { + const reasons = await reasonsOf({ + users: [user("phone-only", { email: undefined, phone: "+15555550100" }), user("b")], + settings: settings({ email_address: { enabled: true }, phone_number: { enabled: false } }), + }); + expect(reasons).toEqual({ + "phone-only": + "has no identifier this instance accepts (its email, phone or username is turned off)", + }); + }); + + describe("legal consent", () => { + const LEGAL = { + ...settings(EMAIL_ON), + sign_up: { legal_consent_enabled: true }, + } as unknown as UserSettingsJSON; + const users = [user("a"), user("b", { legalAcceptedAt: "2024-01-01T00:00:00.000Z" })]; + + test("rejects a user with no acceptance on record, naming the flag", async () => { + expect((await reasonsOf({ users, settings: LEGAL })).a).toContain("--skip-legal-checks"); + }); + + test("with skipLegalChecks, imports them with skip_legal_checks and a warning", async () => { + const checks = await checkImport(input({ users, settings: LEGAL, skipLegalChecks: true })); + expect(checks.rejects).toEqual([]); + expect(checks.importable.map((u) => u.skipLegalChecks)).toEqual([true, undefined]); + expect(checks.warnings.join("\n")).toContain("1 user has no legal acceptance on record"); + }); + }); + }); + + test("a user that failed validation", async () => { + const checks = await checkImport( + input({ failures: [{ userId: "bad", row: 0, error: "Invalid email", path: ["email"] }] }), + ); + expect(checks.rejects).toEqual([{ sourceId: "bad", reason: "invalid: Invalid email" }]); + expect(checks.total).toBe(1); + }); + + test("a duplicate source ID, email, phone or username within the file", async () => { + expect( + await reasonsOf({ + users: [ + user("a"), + user("a", { email: "other@x.dev" }), + user("b", { email: "A@x.dev" }), + user("c", { phone: "+15555550100" }), + user("d", { phone: "+15555550100" }), + // The same number, punctuated. + user("f", { phone: "+1 555-555-0100" }), + user("g", { username: "Ada" }), + // Clerk lowercases usernames. + user("h", { username: "ada" }), + ], + }), + ).toEqual({ + a: "duplicate source ID in the file", + b: "email is also used by an earlier user in the file, which is kept", + d: "phone number is also used by an earlier user in the file, which is kept", + f: "phone number is also used by an earlier user in the file, which is kept", + h: "username is also used by an earlier user in the file, which is kept", + }); + }); + + // The source's order decides which duplicate survives, so the dry run says. + test("a duplicate names the earlier user kept in its place", async () => { + const checks = await checkImport( + input({ + users: [user("email|1", { email: "a@x.dev" }), user("auth0|1", { email: "a@x.dev" })], + }), + ); + expect(checks.rejects).toEqual([ + { + sourceId: "auth0|1", + reason: "email is also used by an earlier user in the file, which is kept", + keptSourceId: "email|1", + }, + ]); + }); + + // G20: an unverified email is attached after the user exists, so it cannot + // satisfy a sign-up requirement. + test("a user with only an unverified email, where email is required", async () => { + expect( + await reasonsOf({ + settings: EMAIL_REQUIRED, + users: [ + user("ok"), + user("unverified", { email: undefined, unverifiedEmailAddresses: ["u@x.dev"] }), + user("none", { email: undefined, username: "none" }), + ], + }), + ).toEqual({ + unverified: "only has an unverified email, and this instance requires an email", + none: "no email, which this instance requires", + }); + }); + + test("a password that is not the shape its hasher says", async () => { + expect( + await reasonsOf({ + users: [ + user("good", { password: BCRYPT, passwordHasher: "bcrypt" }), + user("bad", { password: "not-a-hash", passwordHasher: "bcrypt" }), + ], + }), + ).toEqual({ + bad: "password is not a bcrypt hash Clerk accepts ($2a$/$2b$/$2y$, cost up to 15, 60 characters)", + }); + }); + + test("a user already in the instance, by source ID, email or username", async () => { + existing = [ + { id: "user_1", external_id: "by-id" }, + { id: "user_2", email_addresses: [{ email_address: "by-email@x.dev" }] }, + { id: "user_3", username: "taken" }, + ]; + + expect( + await reasonsOf({ + users: [ + user("by-id"), + user("by-email"), + user("by-username", { username: "Taken" }), + user("new"), + ], + }), + ).toEqual({ + "by-id": "already in the instance, with this source ID", + "by-email": "email is already used by a user in the instance", + "by-username": "username is already taken in the instance", + }); + }); + + // A repeated record (an export that paged past a sign-up) must not take the + // kept copy down with it. + test("a repeated source ID rejects only the later copy", async () => { + const checks = await checkImport(input({ users: [user("a"), user("a")] })); + expect(checks.importable.map((u) => u.userId)).toEqual(["a"]); + expect(checks.rejects).toEqual([{ sourceId: "a", reason: "duplicate source ID in the file" }]); + }); + + test("a rejected user does not claim its email from a later one", async () => { + const checks = await checkImport( + input({ + users: [ + user("skipped", { email: "same@x.dev", skipReason: "anonymous Better Auth user" }), + user("kept", { email: "same@x.dev" }), + ], + }), + ); + expect(checks.importable.map((u) => u.userId)).toEqual(["kept"]); + }); + + // A continued run found this user behind its own in-flight create. + test("not a user the continued run adopted", async () => { + existing = [{ id: "user_1", external_id: "mine" }]; + + expect( + await reasonsOf({ users: [user("mine")], adoptedClerkIds: new Set(["user_1"]) }), + ).toEqual({}); + }); + + test("a supabase user whose only provider is disabled", async () => { + const rows = [ + { id: "only-discord", raw_app_meta_data: { providers: ["discord"] } }, + { id: "has-email", raw_app_meta_data: { providers: ["email", "discord"] } }, + ]; + expect( + await reasonsOf({ + settings: settings( + { email_address: { enabled: true } }, + { oauth_google: { enabled: true } }, + ), + supabaseRows: rows, + users: [user("only-discord"), user("has-email")], + }), + ).toEqual({ "only-discord": "only signs in with Discord, which is not enabled in Clerk" }); + }); + + // Clerk can't turn on a provider it doesn't offer, so none is suggested. + test("a provider Clerk doesn't offer is named as such, with no fix", async () => { + const rows = [ + { id: "figma", raw_app_meta_data: { providers: ["figma"] } }, + { id: "both", raw_app_meta_data: { providers: ["discord", "figma"] } }, + ]; + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true } }), + supabaseRows: rows, + users: [user("figma"), user("both")], + }), + ); + expect(Object.fromEntries(checks.rejects.map((r) => [r.sourceId, r.reason]))).toEqual({ + figma: "only signs in with Figma, which is not offered by Clerk", + both: "only signs in with Discord (not enabled in Clerk), Figma (not offered by Clerk)", + }); + expect(checks.fixes.map((fix) => fix.label)).not.toContain("Enable Figma sign-in"); + }); + + // Each reject names that user's providers, not every disabled one in the file. + test("a supabase reject names only that user's own providers", async () => { + const rows = [ + { id: "discord", raw_app_meta_data: { providers: ["discord"] } }, + { id: "twitch", raw_app_meta_data: { providers: ["twitch"] } }, + ]; + expect( + await reasonsOf({ + settings: settings({ email_address: { enabled: true } }), + supabaseRows: rows, + users: [user("discord"), user("twitch")], + }), + ).toEqual({ + discord: "only signs in with Discord, which is not enabled in Clerk", + twitch: "only signs in with Twitch, which is not enabled in Clerk", + }); + }); + + test("the users past a development instance's headroom, in file order", async () => { + const checks = await checkImport( + input({ + instanceType: "dev", + existingUsers: 98, + users: [user("a"), user("b"), user("c")], + }), + ); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["a", "b"]); + expect(checks.rejects).toEqual([ + { + sourceId: "c", + reason: + "over the development instance's 100-user limit (raised by Clerk? set CLERK_MIGRATE_DEV_USER_LIMIT)", + }, + ]); + expect(checks.quota).toEqual({ existing: 98, limit: 100, headroom: 2, over: 1 }); + }); + + test("warns that an unreadable user count was checked as empty", async () => { + const checks = await checkImport( + input({ instanceType: "dev", existingUsers: null, users: [user("a")] }), + ); + expect(checks.warnings.join("\n")).toContain("Could not read how many users"); + }); + + test("CLERK_MIGRATE_DEV_USER_LIMIT raises the headroom", async () => { + process.env.CLERK_MIGRATE_DEV_USER_LIMIT = "500"; + try { + const checks = await checkImport( + input({ instanceType: "dev", existingUsers: 98, users: [user("a"), user("b"), user("c")] }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.quota).toEqual({ existing: 98, limit: 500, headroom: 402, over: 0 }); + } finally { + delete process.env.CLERK_MIGRATE_DEV_USER_LIMIT; + } + }); + + test("a user its source asked to skip, with the source's reason", async () => { + const checks = await checkImport( + input({ users: [user("a", { skipReason: "anonymous Better Auth user" }), user("b")] }), + ); + expect(checks.rejects).toEqual([{ sourceId: "a", reason: "anonymous Better Auth user" }]); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["b"]); + }); + + describe("emails Clerk refuses", () => { + test("a user whose only identifier is a placeholder email is rejected", async () => { + const checks = await checkImport( + input({ users: [user("a", { email: "15551234@phone.local" })] }), + ); + // One fixed reason, so these group in the report and the address + // itself stays out of users.ndjson. + expect(checks.rejects).toEqual([ + { + sourceId: "a", + reason: "only has emails Clerk refuses (malformed, or a domain that can't receive mail)", + }, + ]); + }); + + test.each([ + ["a@localhost", "no dotted domain"], + ["not-an-email", "no @"], + ["a b@x.dev", "a space"], + ["ada@corp.internal", "a private TLD"], + ])("drops %p (%s) and keeps the user on its other email", async (bad) => { + const checks = await checkImport( + input({ users: [user("a", { email: "a@x.dev", emailAddresses: [bad] })] }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable[0]?.emailAddresses).toBeUndefined(); + }); + + // clerk_go accepts a non-ASCII local part; Zod's email check did not. + test("keeps a non-ASCII address", async () => { + const checks = await checkImport(input({ users: [user("a", { email: "josé@x.dev" })] })); + expect(checks.importable).toEqual([user("a", { email: "josé@x.dev" })]); + }); + + test("a placeholder email is dropped, and the user imports on what is left", async () => { + const checks = await checkImport( + input({ + users: [ + user("a", { + email: "a@x.dev", + unverifiedEmailAddresses: ["anon-1@anonymous.invalid"], + }), + ], + }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a", { email: "a@x.dev" })]); + expect(checks.warnings).toContain( + "1 user has an email Clerk refuses (malformed, or a domain such as .local or .invalid), which is dropped", + ); + }); + + test("Clerk's own .clerk.test addresses are kept", async () => { + const checks = await checkImport( + input({ users: [user("a", { email: "a@dev.clerk.test" })] }), + ); + expect(checks.importable).toEqual([user("a", { email: "a@dev.clerk.test" })]); + }); + }); + + describe("names Clerk refuses", () => { + test.each([ + ["+447836887904"], + ["4165550123"], + ["https://spam.example"], + ["see x.com/win"], + ["Ada"], + ])("%p is dropped, and the user still imports", async (firstName) => { + const checks = await checkImport(input({ users: [user("a", { firstName, lastName: "L" })] })); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a", { lastName: "L" })]); + expect(checks.warnings).toContain( + "1 user has a name Clerk refuses (a phone number, URL or HTML), which is dropped", + ); + }); + + // Clerk accepts an email as a name, and a URL that is part of one. + test.each([ + ["Ada"], + ["Mary-Jane O'Neil"], + ["Louis XIV"], + ["Agent 007"], + ["redacted.io"], + ["ada@x.dev"], + ["ada@x.dev/x"], + ])("%p is kept", async (firstName) => { + const checks = await checkImport(input({ users: [user("a", { firstName })] })); + expect(checks.importable).toEqual([user("a", { firstName })]); + }); + }); + + describe("usernames", () => { + const withUsernames = (rules: object) => + ({ + attributes: { email_address: { enabled: true }, username: { enabled: true } }, + social: {}, + username_settings: { min_length: 4, max_length: 64, ...rules }, + }) as unknown as UserSettingsJSON; + const reasonFor = async (username: string, rules: object = {}) => + ( + await checkImport( + input({ settings: withUsernames(rules), users: [user("a", { username })] }), + ) + ).rejects[0]?.reason; + + test.each([ + ["ada_l-1", {}], + ["ada.l", { allow_extended_special_characters: true }], + ])("%p is accepted", async (username, rules) => { + expect(await reasonFor(username, rules)).toBeUndefined(); + }); + + test("a . needs extended special characters", async () => { + expect(await reasonFor("ada.lovelace")).toContain("turn on extended special characters"); + }); + + test.each([ + ["ada", "4–64 characters"], + ["12345", "no letters"], + ["ada@x", "characters Clerk does not allow"], + ])("%p is rejected: %s", async (username, reason) => { + expect(await reasonFor(username)).toContain(reason); + }); + + test("a username is not checked when usernames are off", async () => { + const checks = await checkImport( + input({ settings: EMAIL_REQUIRED, users: [user("a", { username: "a.b" })] }), + ); + expect(checks.rejects).toEqual([]); + }); + }); + + test("groups the rejects by reason", async () => { + const checks = await checkImport( + input({ + settings: EMAIL_REQUIRED, + users: [ + user("a", { email: undefined, username: "a" }), + user("b", { email: undefined, username: "b" }), + ], + }), + ); + expect(checks.rejectReasons).toEqual([ + { reason: "no email, which this instance requires", count: 2 }, + ]); + }); +}); + +describe("warnings", () => { + test("a field the instance is not set up to store", async () => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, phone_number: { enabled: false } }), + users: [user("a", { phone: "+15555550100" })], + }), + ); + expect(checks.warnings).toContain( + "1 user has a phone, which this instance is not set up to store", + ); + expect(checks.rejects).toEqual([]); + }); + + // Clerk refuses the whole create when it is sent an identifier the instance + // has off, so the dropped phone has to actually be dropped. + test("strips identifiers the instance has off from the importable users", async () => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, phone_number: { enabled: false } }), + users: [ + user("a", { + phone: "+15555550100", + unverifiedPhoneNumbers: ["+15555550101"], + username: "ada.l", + }), + ], + }), + ); + expect(checks.importable).toEqual([user("a")]); + }); + + test("says a password is kept, not dropped, when passwords are off", async () => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, password: { enabled: false } }), + users: [user("a", { password: BCRYPT, passwordHasher: "bcrypt" })], + }), + ); + expect(checks.warnings).toContain( + "1 user has a password, which this instance does not use: it is stored, and works only once passwords are turned on", + ); + }); + + test("fields Clerk has no place for", async () => { + const checks = await checkImport( + input({ users: [user("a")], unknownFields: { department: 3, role: 1 } }), + ); + expect(checks.warnings).toContain("Clerk won't store: department (3 users), role (1 user)"); + }); + + test("passwords a source had to drop", async () => { + const checks = await checkImport(input({ users: [user("a", { passwordDropped: true })] })); + expect(checks.warnings[0]).toContain("1 password Clerk cannot verify will be dropped"); + }); +}); + +describe("fixes", () => { + test("print a `clerk config patch` per flagged setting, targeting the instance", async () => { + const checks = await checkImport( + input({ + settings: settings({ + email_address: { enabled: true, required: true }, + username: { enabled: true }, + }), + target: { + env: "production", + appId: "app_1", + instanceId: "ins_1", + instanceType: "prod", + keySource: "linked profile", + }, + users: [user("a"), user("b", { email: undefined, username: "b" })], + }), + ); + expect(checks.fixes).toEqual([ + { + label: "Make Email optional at sign-up", + command: `clerk config patch --app app_1 --instance ins_1 --json '{"auth_email":{"required_for_sign_up":false}}'`, + }, + ]); + }); + + // Without --instance, `clerk config patch` changes the linked profile's + // development instance, not the one a --secret-key import targets. + test("name the instance even when the key came from --secret-key", async () => { + const checks = await checkImport( + input({ + settings: EMAIL_REQUIRED, + users: [user("b", { email: undefined, username: "b" })], + }), + ); + expect(checks.fixes[0]?.command).toStartWith("clerk config patch --instance ins_1 --json"); + }); + + test("point at the Dashboard when the instance could not be named", async () => { + const checks = await checkImport( + input({ + settings: EMAIL_REQUIRED, + target: { + env: "production", + instanceId: "key_0123", + instanceType: "prod", + keySource: "--secret-key", + }, + users: [user("b", { email: undefined, username: "b" })], + }), + ); + expect(checks.fixes[0]).toEqual({ + label: "Make Email optional at sign-up", + url: "https://dashboard.clerk.com", + }); + }); + + test("offer nothing when the settings could not be read", async () => { + const checks = await checkImport(input({ users: [user("a")] })); + expect(checks.fixes).toEqual([]); + expect(checks.settingsUnavailable).toBe(true); + }); +}); + +describe("passwordIsOnlySignIn", () => { + const withFactors = (factors: Record, social: object = {}) => + settings( + Object.fromEntries( + Object.entries(factors).map(([name, first_factors]) => [ + name, + { enabled: true, used_for_first_factor: first_factors.length > 0, first_factors }, + ]), + ), + social, + ); + + test.each([ + ["password alone", { password: ["password"] }, {}, true], + // Mirrors clerk_go: a passkey is not counted as a way in without a password. + ["password and passkey", { password: ["password"], passkey: ["passkey"] }, {}, true], + [ + "password and email codes", + { password: ["password"], email_address: ["email_code"] }, + {}, + false, + ], + [ + "password and Google", + { password: ["password"] }, + { oauth_google: { enabled: true, authenticatable: true } }, + false, + ], + ["no password factor", { email_address: ["email_code"] }, {}, false], + ])("%s -> %p", (_label, factors, social, expected) => { + expect(passwordIsOnlySignIn(withFactors(factors, social))).toBe(expected); + }); +}); + +describe("hashShapeProblem", () => { + test.each([ + [BCRYPT, "bcrypt"], + ["hash$salt$signer$sep$8$14", "scrypt_firebase"], + ["$argon2id$v=19$m=65536,t=3,p=4$c2FsdA$aGFzaA", "argon2id"], + [`scrypt:16384:16:1$${"a".repeat(32)}$${"b".repeat(128)}`, "scrypt_werkzeug"], + // Not checked: a bad digest there fails only at sign-in. + ["anything", "pbkdf2_sha256"], + ])("accepts %s as %s", (password, hasher) => { + expect(hashShapeProblem(password, hasher)).toBeUndefined(); + }); + + test.each([ + ["$2a$10$short", "bcrypt"], + ["hash$salt$signer$sep$eight$14", "scrypt_firebase"], + ["hash$salt", "scrypt_firebase"], + ["argon2id$...", "argon2id"], + ["scrypt:16384:16:1$salt$not-hex!", "scrypt_werkzeug"], + ])("rejects %s as %s", (password, hasher) => { + expect(hashShapeProblem(password, hasher)).toBeDefined(); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts new file mode 100644 index 000000000..a384588ac --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -0,0 +1,894 @@ +/** + * `checkImport()`: what the instance will do with each user, worked out before + * anything is written. + * + * Every import runs these checks, and `--dry-run` stops after them. They sort + * the file into three piles: + * + * - **Rejects** — users Clerk would refuse, or that the run would refuse on + * Clerk's behalf. Any reject stops the import unless `--allow-partial` is + * passed, and then each one is recorded as `skipped` with its reason. + * - **Warnings** — users that import, but lose something on the way: a field + * the instance is not set up to store, a field Clerk has no place for, a + * password it cannot verify. + * - **Importable** — everyone else, in file order. + * + * Each rejected user gets the first reason that applies to them, so the counts + * add up to the file. + */ + +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +import type { SpinnerControls } from "../../../lib/spinner.ts"; +import { isEnabled, isRequired, type AttributeName } from "../../users/interactive/attributes.ts"; +import { splitIdentifiers } from "../import-users.ts"; +import type { User } from "../types.ts"; +import { analyzeFields, hasValue } from "./analysis.ts"; +import { + clerkOffersProvider, + enabledSocialProviders, + providerLabel, + toClerkStrategy, +} from "./clerk-config.ts"; +import { resolveDevUserLimit } from "./instance.ts"; +import { buildChangePayload, buildSettingChanges } from "./modify-settings.ts"; +import { buildReadinessReport } from "./readiness.ts"; +import type { ApiScheduler } from "./scheduler.ts"; +import { + countSocialProviders, + findDisabledProviders, + findUsersWithOnlyDisabledProviders, + getUserProviders, +} from "./supabase-providers.ts"; +import type { ClerkTarget } from "./target.ts"; +import type { ValidationFailure } from "./transform.ts"; +import { lookupUsers, type LookedUpUser } from "./user-lookup.ts"; + +export type Reject = { + sourceId: string; + reason: string; + /** For a duplicate: the earlier user in the file that is kept instead. */ + keptSourceId?: string; +}; + +export type ReasonCount = { reason: string; count: number }; + +/** A `clerk config patch` that would stop a setting costing users. */ +/** + * A setting change that would stop users being flagged: a `clerk config patch` + * command, or a Dashboard link when the instance can't be named for one. + */ +export type Fix = { label: string; command?: string; url?: string }; + +const DASHBOARD_URL = "https://dashboard.clerk.com"; + +export type Quota = { + /** Users already in the instance, or `null` when the count could not be read. */ + existing: number | null; + limit: number; + headroom: number; + /** Importable users over the headroom. */ + over: number; +}; + +export type ImportChecks = { + /** Users in the file being considered, valid or not. */ + total: number; + /** Users that pass every check, in file order. */ + importable: User[]; + /** Users that do not, each with the first reason that applies to them. */ + rejects: Reject[]; + rejectReasons: ReasonCount[]; + warnings: string[]; + fixes: Fix[]; + quota?: Quota; + /** True when the instance's settings could not be read. */ + settingsUnavailable: boolean; +}; + +export type CheckInput = { + users: User[]; + failures: ValidationFailure[]; + /** Fields the source produced that Clerk has no place for → how many users carry each. */ + unknownFields?: Record; + /** The raw Supabase rows, for the provider checks. Absent for any other source. */ + supabaseRows?: Record[]; + settings: UserSettingsJSON | null; + /** Users already in the instance, for the quota. Development instances only. */ + existingUsers?: number | null; + instanceType: "dev" | "prod"; + target: ClerkTarget; + secretKey: string; + schedule: ApiScheduler; + /** + * Import users with no legal acceptance into an instance that requires it, + * sending `skip_legal_checks`. Without it they are rejected. + */ + skipLegalChecks?: boolean; + /** + * Clerk IDs a continued run found behind its own in-flight creates: finding + * them in the instance is expected. + */ + adoptedClerkIds?: Set; + spinner?: SpinnerControls; +}; + +const plural = (count: number, word: string) => `${count} ${word}${count === 1 ? "" : "s"}`; + +// --- Hash shapes ----------------------------------------------------------- + +/** + * Why a password digest cannot be what its hasher says it is, for the hashers + * whose shape is cheap and certain to check. + * + * Every other hasher's shape is not checked here: BAPI validates it at + * create, so a bad digest there fails that user mid-import. + */ +export function hashShapeProblem(password: string, hasher: string): string | undefined { + switch (hasher) { + case "bcrypt": + // clerk_go caps the cost at 15 (pkg/hash/bcrypt.go). + return /^\$2[aby]\$(0\d|1[0-5])\$[./A-Za-z0-9]{53}$/.test(password) + ? undefined + : "password is not a bcrypt hash Clerk accepts ($2a$/$2b$/$2y$, cost up to 15, 60 characters)"; + case "scrypt_firebase": { + const parts = password.split("$"); + const numeric = (value: string | undefined) => /^\d+$/.test(value ?? ""); + return parts.length === 6 && + parts.slice(0, 4).every(Boolean) && + numeric(parts[4]) && + numeric(parts[5]) + ? undefined + : "password is not a Firebase scrypt digest (hash$salt$signerKey$saltSeparator$rounds$memCost)"; + } + case "argon2i": + case "argon2id": + return password.startsWith("$argon2") ? undefined : "password is not an argon2 hash"; + case "scrypt_werkzeug": + return /^scrypt:\d+:\d+:\d+\$[^$]+\$[0-9a-f]+$/i.test(password) + ? undefined + : "password is not a Werkzeug scrypt hash (scrypt:N:r:p$salt$hex)"; + default: + return undefined; + } +} + +// --- Per-user checks ------------------------------------------------------- + +/** + * Identifiers Clerk creates a user under. A required one the user does not + * carry leaves nothing to create them with. + * + * An email or phone counts only when it is verified: an unverified one is + * attached after the user exists, so it cannot satisfy a sign-up requirement. + */ +function missingRequiredIdentifier( + user: User, + settings: UserSettingsJSON | null, +): string | undefined { + if (!settings) return undefined; + const required = (attribute: AttributeName) => isRequired(settings, attribute); + const identifiers = splitIdentifiers(user); + + if (required("email_address") && !identifiers.primaryEmail) { + return identifiers.unverifiedEmails.length > 0 + ? "only has an unverified email, and this instance requires an email" + : "no email, which this instance requires"; + } + if (required("phone_number") && !identifiers.primaryPhone) { + return identifiers.unverifiedPhones.length > 0 + ? "only has an unverified phone number, and this instance requires one" + : "no phone number, which this instance requires"; + } + if (required("username") && !hasValue(user.username)) { + return "no username, which this instance requires"; + } + return undefined; +} + +/** A required first or last name the user lacks: `POST /v1/users` refuses it. */ +function missingRequiredName(user: User, settings: UserSettingsJSON | null): string | undefined { + if (!settings) return undefined; + if (isRequired(settings, "first_name") && !hasValue(user.firstName)) { + return "no first name, which this instance requires"; + } + if (isRequired(settings, "last_name") && !hasValue(user.lastName)) { + return "no last name, which this instance requires"; + } + return undefined; +} + +/** MFA a user can carry, and the setting `POST /v1/users` refuses it without. */ +const MFA_SETTINGS = [ + { + field: "totpSecret", + attribute: "authenticator_app", + reason: "has an authenticator app (TOTP) secret, and this instance has authenticator apps off", + label: "Enable authenticator apps", + path: ["auth_multi_factor", "authenticator_app", "enabled"], + }, + { + field: "backupCodes", + attribute: "backup_code", + reason: "has backup codes, and this instance has backup codes off", + label: "Enable backup codes", + path: ["auth_multi_factor", "backup_code", "enabled"], + }, +] as const; + +/** + * MFA the instance has off. Rejected rather than dropped: importing the user + * without it would quietly take away their second factor. + */ +function mfaProblem(user: User, settings: UserSettingsJSON | null): string | undefined { + if (!settings) return undefined; + return MFA_SETTINGS.find( + ({ field, attribute }) => hasValue(user[field]) && !isEnabled(settings, attribute), + )?.reason; +} + +/** Sign-in strategies that don't count as a way in without a password, per clerk_go. */ +const NOT_ALTERNATIVE_SIGN_IN = new Set([ + "password", + "passkey", + "ticket", + "reset_password_email_code", + "reset_password_phone_code", +]); + +/** + * True when the instance has no sign-in strategy but a password. Clerk then + * refuses `skip_password_requirement`, so a user without a digest can't be + * created. Mirrors `create_service.go` and `UserSettings.FirstFactors()`. + */ +export function passwordIsOnlySignIn(settings: UserSettingsJSON): boolean { + const strategies = new Set(); + for (const attribute of Object.values(settings.attributes ?? {})) { + if (attribute?.used_for_first_factor) { + for (const strategy of attribute.first_factors ?? []) strategies.add(strategy); + } + } + for (const [strategy, social] of Object.entries(settings.social ?? {})) { + if (social?.enabled && social.authenticatable) strategies.add(strategy); + } + if (settings.enterprise_sso?.enabled) strategies.add("enterprise_sso"); + return ( + strategies.has("password") && + [...strategies].every((strategy) => NOT_ALTERNATIVE_SIGN_IN.has(strategy)) + ); +} + +/** True when the instance needs legal acceptance this user has no record of. */ +function lacksLegalAcceptance(user: User, settings: UserSettingsJSON | null): boolean { + return ( + Boolean(settings?.sign_up?.legal_consent_enabled) && + !user.legalAcceptedAt && + !user.skipLegalChecks + ); +} + +/** What FAPI serves under `username_settings`; `@clerk/shared` types only the lengths. */ +type UsernameSettings = { + min_length?: number; + max_length?: number; + allow_extended_special_characters?: boolean; + allow_numeric_usernames?: boolean; +}; + +const USERNAME_DEFAULT = /^[a-zA-Z0-9_-]+$/; +const USERNAME_EXTENDED = /^[a-zA-Z0-9!#$'+.^_`~-]+$/; + +/** + * Clerk's username rules, mirrored from `validate.Username` in clerk_go, so a + * username the instance would refuse is a reject here rather than a failed + * create. Skipped when usernames are off: the readiness warnings cover that. + */ +function usernameProblem(user: User, settings: UserSettingsJSON | null): string | undefined { + const username = user.username; + if (!settings || typeof username !== "string" || !username) return undefined; + if (!isEnabled(settings, "username")) return undefined; + + const rules = (settings as { username_settings?: UsernameSettings }).username_settings ?? {}; + const length = [...username].length; + if (rules.min_length !== undefined && rules.max_length !== undefined) { + if (length < rules.min_length || length > rules.max_length) { + return `username is not ${rules.min_length}–${rules.max_length} characters, which this instance requires`; + } + } + if (!rules.allow_numeric_usernames && !/[a-zA-Z]/.test(username)) { + return "username has no letters; turn on numeric usernames to allow it"; + } + if (rules.allow_extended_special_characters) { + if (!USERNAME_EXTENDED.test(username)) return "username has characters Clerk does not allow"; + if (/^\+[1-9]\d{1,14}$/.test(username)) + return "username is a phone number, which Clerk does not allow"; + return undefined; + } + if (!USERNAME_DEFAULT.test(username)) { + return USERNAME_EXTENDED.test(username) + ? "username has special characters this instance does not allow; turn on extended special characters" + : "username has characters Clerk does not allow"; + } + return undefined; +} + +/** + * TLDs Clerk refuses for any email: clerk_go's `emailaddress.nonRoutableTLDs`, + * where placeholder addresses live (`…@phone.local`, `anon-…@anonymous.invalid`), + * plus the common private ones the public suffix list leaves out. + * + * ponytail: Clerk refuses any TLD not on the public suffix list; this names + * the usual ones instead of taking the list as a dependency. + */ +const NON_ROUTABLE_TLDS = new Set([ + "arpa", + "local", + "invalid", + "example", + "test", + "internal", + "lan", + "corp", + "home", + "localdomain", + "intranet", + "private", +]); + +/** + * An address shape Clerk accepts, loosely: one `@`, no spaces, a dotted host, + * a local part of at most 64 bytes and 254 in all. Non-ASCII is fine, as it + * is in clerk_go (`josé@x.dev`). + */ +function isEmailShaped(email: string): boolean { + const at = email.lastIndexOf("@"); + return ( + /^[^\s@]+@[^\s@]+\.[^\s@.]+$/.test(email) && + Buffer.byteLength(email.slice(0, at)) <= 64 && + email.length <= 254 + ); +} +const EMAIL_FIELDS = ["email", "emailAddresses", "unverifiedEmailAddresses"] as const; + +function isRefusedEmail(email: string): boolean { + if (!isEmailShaped(email)) return true; + const host = email.slice(email.lastIndexOf("@") + 1).toLowerCase(); + // Clerk's own dev domains sit under `.test` and are accepted. + if (host.endsWith(".clerk.test")) return false; + return NON_ROUTABLE_TLDS.has(host.slice(host.lastIndexOf(".") + 1)); +} + +/** The user without the emails Clerk would refuse, and those emails. */ +function dropRefusedEmails(user: User): { user: User; refused: string[] } { + const refused: string[] = []; + let kept: User | undefined; + for (const field of EMAIL_FIELDS) { + const value = user[field]; + if (value === undefined) continue; + const list = Array.isArray(value) ? value : [value]; + const bad = list.filter(isRefusedEmail); + if (bad.length === 0) continue; + refused.push(...bad); + kept ??= { ...user }; + const good = list.filter((email) => !isRefusedEmail(email)); + if (good.length > 0) kept[field] = good; + else delete kept[field]; + } + return { user: kept ?? user, refused }; +} + +/** + * A name Clerk refuses, approximating clerk_go's `NameForAbusePreventionLoose`: + * a phone number (10–15 digits, or fewer behind a `+`/`00`), a URL with a + * scheme or path that isn't part of an email, or an HTML tag. Better Auth's phone sign-up stores the + * number as the name, so this is common, not exotic. + */ +function nameProblem(name: string): string | undefined { + for (const candidate of name.match(/(?:\+|00)?\d[\d\s().-]{5,}\d/g) ?? []) { + const digits = candidate.replace(/\D/g, "").length; + const international = /^(\+|00)/.test(candidate.trim()); + if (digits <= 15 && (digits >= 10 || (international && digits >= 7))) return "a phone number"; + } + // Clerk accepts an email as a name (NameForAbusePreventionLoose), and a URL + // that is part of one. + const hasEmail = /\S+@\S+\.\S+/.test(name); + if (!hasEmail && /:\/\/|\b[\w-]+(\.[\w-]+)+[/?#]/.test(name)) return "a URL"; + if (/<\/?[a-z!][^>]*>/i.test(name)) return "HTML"; + return undefined; +} + +/** The user without a first or last name Clerk would refuse. */ +function dropRefusedNames(user: User): { user: User; dropped: boolean } { + let kept: User | undefined; + for (const field of ["firstName", "lastName"] as const) { + const value = user[field]; + if (typeof value === "string" && nameProblem(value)) { + kept ??= { ...user }; + delete kept[field]; + } + } + return { user: kept ?? user, dropped: kept !== undefined }; +} + +/** + * A phone number with its punctuation stripped, so `+1 555-555-0100` and + * `+15555550100` compare equal. + * + * ponytail: punctuation only; a national number without its country code + * still differs from its E.164 form. Full parsing would need a dependency. + */ +const phoneKey = (phone: string) => phone.replace(/[^\d+]/g, ""); + +const hasAnyIdentifier = (user: User) => + [...EMAIL_FIELDS, "phone", "phoneNumbers", "unverifiedPhoneNumbers", "username"].some((field) => + hasValue(user[field as keyof User]), + ); + +/** + * First user in the file to claim each email, phone and source ID. + * + * Keyed by record, not source ID: two records with one source ID must not + * share a verdict, or the one kept would be rejected along with its copy. + * + * @returns Each duplicate's reason, and the earlier user kept in its place. + */ +function findFileDuplicates(users: User[]): { + reasons: Map; + keptBy: Map; +} { + const reasons = new Map(); + const keptBy = new Map(); + const seenIds = new Set(); + const emails = new Map(); + const phones = new Map(); + const usernames = new Map(); + + for (const user of users) { + if (seenIds.has(user.userId)) { + reasons.set(user, "duplicate source ID in the file"); + continue; + } + seenIds.add(user.userId); + + const identifiers = splitIdentifiers(user); + const ownEmails = [identifiers.primaryEmail, ...identifiers.additionalEmails].filter( + (value): value is string => Boolean(value), + ); + const ownPhones = [identifiers.primaryPhone, ...identifiers.additionalPhones].filter( + (value): value is string => Boolean(value), + ); + + const emailOwner = ownEmails.map((email) => emails.get(email.toLowerCase())).find(Boolean); + const phoneOwner = ownPhones.map((phone) => phones.get(phoneKey(phone))).find(Boolean); + // Clerk lowercases usernames, so the second create would fail. + const username = typeof user.username === "string" ? user.username.toLowerCase() : ""; + const usernameOwner = username ? usernames.get(username) : undefined; + // The first record in the file wins, whatever either holds: the source's + // order decides, so the kept ID is named alongside the reject. + if (emailOwner) { + reasons.set(user, "email is also used by an earlier user in the file, which is kept"); + keptBy.set(user, emailOwner); + continue; + } + if (phoneOwner) { + reasons.set(user, "phone number is also used by an earlier user in the file, which is kept"); + keptBy.set(user, phoneOwner); + continue; + } + if (usernameOwner) { + reasons.set(user, "username is also used by an earlier user in the file, which is kept"); + keptBy.set(user, usernameOwner); + continue; + } + for (const email of ownEmails) emails.set(email.toLowerCase(), user.userId); + for (const phone of ownPhones) phones.set(phoneKey(phone), user.userId); + if (username) usernames.set(username, user.userId); + } + return { reasons, keptBy }; +} + +/** + * Users the instance already holds, found by source ID, primary email, + * primary phone or username. + * + * Only what `POST /v1/users` itself carries is looked up: an extra email that + * collides is attached after the user exists, fails on its own, and is noted + * on the user's line rather than failing them. + */ +async function findInstanceDuplicates( + users: User[], + input: CheckInput, +): Promise> { + const byExternalId = new Map(); + const byEmail = new Map(); + const byPhone = new Map(); + const byUsername = new Map(); + + for (const user of users) { + const identifiers = splitIdentifiers(user); + byExternalId.set(user.userId, user.userId); + if (identifiers.primaryEmail) byEmail.set(identifiers.primaryEmail.toLowerCase(), user.userId); + if (identifiers.primaryPhone) byPhone.set(phoneKey(identifiers.primaryPhone), user.userId); + if (typeof user.username === "string" && user.username) { + byUsername.set(user.username.toLowerCase(), user.userId); + } + } + + const lookup = async ( + filter: "external_id" | "email_address" | "phone_number" | "username", + values: Iterable, + ) => + lookupUsers({ + filter, + values: [...values], + secretKey: input.secretKey, + schedule: input.schedule, + spinner: input.spinner, + label: "Checking for users already in the instance", + }); + + const found: LookedUpUser[] = ( + await Promise.all([ + lookup("external_id", byExternalId.keys()), + lookup("email_address", byEmail.keys()), + lookup("phone_number", byPhone.keys()), + lookup("username", byUsername.keys()), + ]) + ).flat(); + + const reasons = new Map(); + const claim = (sourceId: string | undefined, reason: string) => { + if (sourceId && !reasons.has(sourceId)) reasons.set(sourceId, reason); + }; + + for (const existing of found) { + if (input.adoptedClerkIds?.has(existing.id)) continue; + if (existing.external_id) { + claim(byExternalId.get(existing.external_id), "already in the instance, with this source ID"); + } + for (const email of existing.email_addresses ?? []) { + claim( + byEmail.get((email.email_address ?? "").toLowerCase()), + "email is already used by a user in the instance", + ); + } + for (const phone of existing.phone_numbers ?? []) { + claim( + byPhone.get(phoneKey(phone.phone_number ?? "")), + "phone number is already used by a user in the instance", + ); + } + if (existing.username) { + claim( + byUsername.get(existing.username.toLowerCase()), + "username is already taken in the instance", + ); + } + } + return reasons; +} + +/** Supabase users whose every provider is off in Clerk: they could never sign in. */ +function findDisabledProviderRejects(input: CheckInput): Map { + const reasons = new Map(); + if (!input.supabaseRows || !input.settings) return reasons; + + const enabled = enabledSocialProviders(input.settings); + const disabled = findDisabledProviders(input.supabaseRows, enabled, toClerkStrategy); + if (disabled.length === 0) return reasons; + + const { excludedIds } = findUsersWithOnlyDisabledProviders(input.supabaseRows, disabled); + // Each reject names only that user's own providers. + const rowsById = new Map(input.supabaseRows.map((row) => [String(row.id), row])); + for (const id of excludedIds) { + const own = getUserProviders(rowsById.get(id) ?? {}).filter((p) => disabled.includes(p)); + // Clerk can't turn on a provider it doesn't offer, so say which is which. + const why = (provider: string) => + clerkOffersProvider(provider) ? "not enabled in Clerk" : "not offered by Clerk"; + const kinds = new Set(own.map(why)); + const names = + kinds.size === 1 + ? `${own.map(providerLabel).join(", ")}, which ${own.length === 1 ? "is" : "are"} ${[...kinds][0]}` + : own.map((provider) => `${providerLabel(provider)} (${why(provider)})`).join(", "); + reasons.set(id, `only signs in with ${names}`); + } + return reasons; +} + +// --- Warnings and fixes ---------------------------------------------------- + +/** The Supabase rows for just these users, so provider counts describe them alone. */ +function rowsFor(input: CheckInput, users: User[]): Record[] { + if (!input.supabaseRows) return []; + const ids = new Set(users.map((user) => user.userId)); + return input.supabaseRows.filter((row) => ids.has(String(row.id))); +} + +function buildWarnings(input: CheckInput, importable: User[]): string[] { + const warnings: string[] = []; + + if (input.settings && importable.length > 0) { + const report = buildReadinessReport({ + analysis: analyzeFields(importable), + settings: input.settings, + providerCounts: countSocialProviders(rowsFor(input, importable)), + }); + for (const item of report.blocking) { + if (item.consequence !== "drops") continue; + if (item.clerkRequired === true) { + const missing = importable.length - item.userCount; + warnings.push( + item.key === "password" + ? `${plural(missing, "user")} without a password, which this instance requires: they reset it to sign in` + : `${plural(missing, "user")} without a ${item.label.toLowerCase()}, which this instance requires`, + ); + } else if (item.key === "password") { + // Clerk stores a digest even with passwords off, so nothing is lost: + // it starts working if passwords are turned on. + warnings.push( + `${plural(item.userCount, "user")} ${item.userCount === 1 ? "has" : "have"} a password, which this instance does not use: it is stored, and works only once passwords are turned on`, + ); + } else { + warnings.push( + item.section === "social" + ? `${plural(item.userCount, "user")} signed in with ${item.label}, which ${clerkOffersProvider(item.key) ? "is not enabled in Clerk" : "Clerk doesn't offer"}` + : `${plural(item.userCount, "user")} ${item.userCount === 1 ? "has" : "have"} a ${item.label.toLowerCase()}, which this instance is not set up to store`, + ); + } + } + } + + const dropped = importable.filter((user) => user.passwordDropped).length; + if (dropped > 0) { + warnings.push( + `${plural(dropped, "password")} Clerk cannot verify will be dropped: those users reset it to sign in`, + ); + } + + const unknown = Object.entries(input.unknownFields ?? {}); + if (unknown.length > 0) { + warnings.push( + `Clerk won't store: ${unknown + .sort((a, b) => b[1] - a[1]) + .map(([field, count]) => `${field} (${plural(count, "user")})`) + .join(", ")}`, + ); + } + + return warnings; +} + +/** Shell-quotes a JSON payload for a single-quoted argument. */ +function quoteJson(payload: unknown): string { + return `'${JSON.stringify(payload).replace(/'/g, "'\\''")}'`; +} + +/** + * One `clerk config patch` per flagged setting, built from the same rows the + * warnings and rejects come from. + * + * These are offers, not corrections: an instance that requires an email is + * configured as its owner intended, and fixing the export may be the answer. + */ +function buildFixes(input: CheckInput, users: User[]): Fix[] { + if (!input.settings || users.length === 0) return []; + + const report = buildReadinessReport({ + analysis: analyzeFields(users), + settings: input.settings, + providerCounts: countSocialProviders(rowsFor(input, users)), + }); + + // An unverified email does not satisfy a required one, so a file of only + // unverified addresses flags the requirement even when every user has one. + const unverifiedOnly = users.some((user) => { + const identifiers = splitIdentifiers(user); + return !identifiers.primaryEmail && identifiers.unverifiedEmails.length > 0; + }); + const flagged = report.blocking.slice(); + if ( + unverifiedOnly && + isRequired(input.settings, "email_address") && + isEnabled(input.settings, "email_address") && + !flagged.some((item) => item.key === "email_address") + ) { + const email = report.items.find((item) => item.key === "email_address"); + if (email) + flagged.unshift({ ...email, clerkRequired: true, blocking: true, consequence: "rejects" }); + } + + // Always name the instance: without it, `clerk config patch` acts on the + // linked profile's development instance, whatever key this import used. A + // `key_` ID is a stand-in for an instance Clerk didn't name, so there is + // nothing to pass; point at the Dashboard instead. + const { appId, instanceId } = input.target; + const named = instanceId.startsWith("ins_"); + const flags = `${appId ? ` --app ${appId}` : ""} --instance ${instanceId}`; + const settings = input.settings; + const mfa = MFA_SETTINGS.filter( + ({ field, attribute }) => + !isEnabled(settings, attribute) && users.some((user) => hasValue(user[field])), + ).map(({ label, path }) => ({ label, writes: [{ path: [...path], value: true }] })); + + return [...buildSettingChanges(flagged), ...mfa].map((change) => + named + ? { + label: change.label, + command: `clerk config patch${flags} --json ${quoteJson(buildChangePayload([change]))}`, + } + : { label: change.label, url: DASHBOARD_URL }, + ); +} + +// --- The whole check ------------------------------------------------------- + +function countReasons(rejects: Reject[]): ReasonCount[] { + const counts = new Map(); + for (const { reason } of rejects) counts.set(reason, (counts.get(reason) ?? 0) + 1); + return [...counts].map(([reason, count]) => ({ reason, count })); +} + +/** + * Removes the emails, phones or usernames of an instance that has that + * identifier off. + * + * The warnings already say they are dropped, but Clerk does not drop them. It + * refuses the whole create for a phone (`phone_number is not a valid + * parameter`), and for a username it stores it anyway, or refuses the create + * when the username breaks the default rules. + */ +function dropDisabledIdentifiers(user: User, settings: UserSettingsJSON | null): User { + if (!settings) return user; + const fields = [ + ...(isEnabled(settings, "email_address") + ? [] + : (["email", "emailAddresses", "unverifiedEmailAddresses"] as const)), + ...(isEnabled(settings, "phone_number") + ? [] + : (["phone", "phoneNumbers", "unverifiedPhoneNumbers"] as const)), + ...(isEnabled(settings, "username") ? [] : (["username"] as const)), + ]; + if (!fields.some((field) => field in user)) return user; + const kept = { ...user }; + for (const field of fields) delete kept[field]; + return kept; +} + +function refusedNameWarning(count: number): string[] { + if (count === 0) return []; + return [ + `${plural(count, "user")} ${count === 1 ? "has" : "have"} a name Clerk refuses (a phone number, URL or HTML), which is dropped`, + ]; +} + +function legalWarning(count: number): string[] { + if (count === 0) return []; + return [ + `${plural(count, "user")} ${count === 1 ? "has" : "have"} no legal acceptance on record, and ${count === 1 ? "is" : "are"} created without it (--skip-legal-checks)`, + ]; +} + +function placeholderWarning(count: number): string[] { + if (count === 0) return []; + return [ + `${plural(count, "user")} ${count === 1 ? "has" : "have"} an email Clerk refuses (malformed, or a domain such as .local or .invalid), which is dropped`, + ]; +} + +export async function checkImport(input: CheckInput): Promise { + const rejects: Reject[] = input.failures.map((failure) => ({ + sourceId: failure.userId, + reason: `invalid: ${failure.error}`, + })); + + const disabledProviders = findDisabledProviderRejects(input); + + // Users that pass every per-user check. Only these claim identifiers in + // the file: a rejected record must not cost a later one its email. + const passed: User[] = []; + const placeholderEmails = new Set(); + const refusedNames = new Set(); + for (const original of input.users) { + const named = dropRefusedNames(original); + if (named.dropped) refusedNames.add(original.userId); + const { user, refused } = dropRefusedEmails(named.user); + const reason = + original.skipReason ?? + (refused.length > 0 && !hasAnyIdentifier(user) + ? "only has emails Clerk refuses (malformed, or a domain that can't receive mail)" + : undefined) ?? + missingRequiredIdentifier(user, input.settings) ?? + // Stripping the identifiers the instance has off can leave nothing to + // sign in with; Clerk would still create the user. + (!hasAnyIdentifier(dropDisabledIdentifiers(user, input.settings)) + ? "has no identifier this instance accepts (its email, phone or username is turned off)" + : undefined) ?? + missingRequiredName(user, input.settings) ?? + mfaProblem(user, input.settings) ?? + (!user.password && input.settings && passwordIsOnlySignIn(input.settings) + ? "no password, and password is this instance's only way to sign in" + : undefined) ?? + (!input.skipLegalChecks && lacksLegalAcceptance(user, input.settings) + ? "no legal acceptance on record, which this instance requires (--skip-legal-checks imports them without it)" + : undefined) ?? + usernameProblem(user, input.settings) ?? + (user.password && user.passwordHasher + ? hashShapeProblem(user.password, user.passwordHasher) + : undefined) ?? + disabledProviders.get(user.userId); + if (reason) { + rejects.push({ sourceId: user.userId, reason }); + } else { + passed.push(user); + if (refused.length > 0) placeholderEmails.add(user.userId); + } + } + + const { reasons: fileDuplicates, keptBy } = findFileDuplicates(passed); + let candidates: User[] = []; + for (const user of passed) { + const reason = fileDuplicates.get(user); + const kept = keptBy.get(user); + if (reason) + rejects.push({ sourceId: user.userId, reason, ...(kept ? { keptSourceId: kept } : {}) }); + else candidates.push(user); + } + + const instanceDuplicates = + candidates.length > 0 + ? await findInstanceDuplicates(candidates, input) + : new Map(); + const unique: User[] = []; + for (const user of candidates) { + const reason = instanceDuplicates.get(user.userId); + if (reason) rejects.push({ sourceId: user.userId, reason }); + else unique.push(user); + } + candidates = unique; + + // The limit is a development-instance default, not a number the API serves, + // so it is checked against the live count and the importable users alone. + // An instance Clerk has raised sets CLERK_MIGRATE_DEV_USER_LIMIT. + let quota: Quota | undefined; + if (input.instanceType === "dev") { + const limit = resolveDevUserLimit(); + const headroom = Math.max(0, limit - (input.existingUsers ?? 0)); + const over = Math.max(0, candidates.length - headroom); + quota = { existing: input.existingUsers ?? null, limit, headroom, over }; + if (over > 0) { + for (const user of candidates.slice(headroom)) { + rejects.push({ + sourceId: user.userId, + reason: `over the development instance's ${limit}-user limit (raised by Clerk? set CLERK_MIGRATE_DEV_USER_LIMIT)`, + }); + } + candidates = candidates.slice(0, headroom); + } + } + + return { + total: input.users.length + input.failures.length, + importable: candidates.map((user) => { + const kept = dropDisabledIdentifiers(user, input.settings); + return lacksLegalAcceptance(kept, input.settings) ? { ...kept, skipLegalChecks: true } : kept; + }), + rejects, + rejectReasons: countReasons(rejects), + warnings: [ + ...buildWarnings(input, candidates), + ...placeholderWarning(candidates.filter((user) => placeholderEmails.has(user.userId)).length), + ...refusedNameWarning(candidates.filter((user) => refusedNames.has(user.userId)).length), + ...legalWarning( + candidates.filter((user) => lacksLegalAcceptance(user, input.settings)).length, + ), + // An unknown count is checked as zero; say so rather than imply it fit. + ...(quota && quota.existing === null + ? [ + `Could not read how many users this development instance holds, so the ${quota.limit}-user limit was checked as if it were empty`, + ] + : []), + ], + fixes: buildFixes(input, input.users), + ...(quota ? { quota } : {}), + settingsUnavailable: input.settings === null, + }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts new file mode 100644 index 000000000..efceabe1d --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts @@ -0,0 +1,89 @@ +import { test, expect, describe, mock, beforeEach, afterAll } from "bun:test"; +import { stubFetch, useCaptureLog } from "../../../test/lib/stubs.ts"; +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +import { fetchInstanceSettings } from "./clerk-config.ts"; + +const USER_SETTINGS = { + attributes: { email_address: { enabled: true, required: true } }, +} as unknown as UserSettingsJSON; + +function json(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("fetchInstanceSettings", () => { + const originalFetch = globalThis.fetch; + useCaptureLog(); + const mockFetch = mock(); + + beforeEach(() => { + mockFetch.mockReset(); + stubFetch(mockFetch); + }); + afterAll(() => { + globalThis.fetch = originalFetch; + }); + + /** Routes the three hops: BAPI domains → FAPI dev browser → FAPI environment. */ + function route(domains: unknown): void { + mockFetch.mockImplementation((input: string | URL) => { + const url = String(input); + if (url.includes("/v1/domains")) return Promise.resolve(json({ data: domains })); + if (url.includes("/v1/dev_browser")) return Promise.resolve(json({ token: "jwt" })); + if (url.includes("/v1/environment")) { + return Promise.resolve(json({ user_settings: USER_SETTINGS })); + } + throw new Error(`unexpected request: ${url}`); + }); + } + + test("reads settings off the primary domain's Frontend API", async () => { + route([{ is_satellite: false, frontend_api_url: "https://clerk.example.com" }]); + + expect(await fetchInstanceSettings("sk_test_abc")).toEqual(USER_SETTINGS); + + const urls = mockFetch.mock.calls.map(([input]) => String(input)); + expect(urls.some((url) => url.includes("clerk.example.com/v1/dev_browser"))).toBe(true); + expect(urls.some((url) => url.includes("clerk.example.com/v1/environment"))).toBe(true); + }); + + test("prefers the primary domain over a satellite", async () => { + route([ + { is_satellite: true, frontend_api_url: "https://satellite.example.com" }, + { is_satellite: false, frontend_api_url: "https://clerk.example.com" }, + ]); + + await fetchInstanceSettings("sk_test_abc"); + + // Compared as a parsed hostname rather than a substring: the satellite's + // domain can appear anywhere in a URL — in a path or a query parameter — + // so `includes` would pass a request that never went near that host, and + // fail one that did. + const urls = mockFetch.mock.calls.map(([input]) => String(input)); + expect(urls.every((url) => new URL(url).hostname !== "satellite.example.com")).toBe(true); + }); + + test("skips the dev browser bootstrap for a production key", async () => { + route([{ is_satellite: false, frontend_api_url: "https://clerk.example.com" }]); + + expect(await fetchInstanceSettings("sk_live_abc")).toEqual(USER_SETTINGS); + + const urls = mockFetch.mock.calls.map(([input]) => String(input)); + expect(urls.some((url) => url.includes("/v1/dev_browser"))).toBe(false); + }); + + // `null` means "unknown", so callers degrade rather than treating a failed + // lookup as "nothing is enabled". + test("returns null when no domain names a Frontend API URL", async () => { + route([{ is_satellite: false }]); + expect(await fetchInstanceSettings("sk_test_abc")).toBeNull(); + }); + + test("returns null when the domains lookup fails", async () => { + mockFetch.mockResolvedValue(new Response("nope", { status: 401 })); + expect(await fetchInstanceSettings("sk_test_abc")).toBeNull(); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts new file mode 100644 index 000000000..6e05eaf10 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts @@ -0,0 +1,193 @@ +/** + * The destination instance's live user settings: which identifiers it accepts + * and requires, and which social providers it has enabled. + * + * Ported from the standalone migration-tool's `src/lib/clerk.ts`, rewritten + * onto the CLI's own primitives: the FAPI host comes from BAPI `/v1/domains` + * — a secret key is all `migrate import` is given — and the settings come from + * `lib/fapi.ts` rather than a bespoke fetch. + */ + +import { bapiRequest } from "../../../lib/bapi.ts"; +import { + bootstrapDevBrowser, + fetchUserSettings, + type UserSettingsJSON, +} from "../../../lib/fapi.ts"; +import { log } from "../../../lib/log.ts"; +import { detectInstanceType } from "./instance.ts"; + +/** + * Supabase provider keys whose Clerk strategy is not simply `oauth_`. + * + * Everything not listed here maps by prefix, which covers google, github, + * discord, spotify, twitch, notion, figma, gitlab, bitbucket and the rest. + */ +const CLERK_STRATEGY_ALIASES: Record = { + azure: "oauth_microsoft", + twitter: "oauth_x", + slack_oidc: "oauth_slack", + fly: "oauth_fly", +}; + +/** Supabase's provider key as Clerk's OAuth strategy name. */ +export function toClerkStrategy(provider: string): string { + return CLERK_STRATEGY_ALIASES[provider] ?? `oauth_${provider}`; +} + +/** + * The OAuth strategies Clerk has built in: clerk_go's registration in + * `api/shared/sso/oauth.go`, less the customer-specific ones. Nothing the CLI + * can call lists them, and an instance's settings name only the providers it + * has configured. + * + * ponytail: a copy of that list; check it there when Clerk adds a provider. + */ +const CLERK_OAUTH_STRATEGIES = new Set([ + "oauth_agentid", + "oauth_apple", + "oauth_atlassian", + "oauth_bitbucket", + "oauth_box", + "oauth_coinbase", + "oauth_discord", + "oauth_dropbox", + "oauth_facebook", + "oauth_github", + "oauth_gitlab", + "oauth_google", + "oauth_hubspot", + "oauth_huggingface", + "oauth_instagram", + "oauth_line", + "oauth_linear", + "oauth_linkedin", + "oauth_linkedin_oidc", + "oauth_microsoft", + "oauth_notion", + "oauth_slack", + "oauth_spotify", + "oauth_tiktok", + "oauth_twitch", + "oauth_twitter", + "oauth_vercel", + "oauth_x", + "oauth_xero", +]); + +/** + * True when Clerk offers a Supabase provider at all. One it doesn't (Figma, + * Kakao, Keycloak, WorkOS, Zoom, Fly) can't be turned on, so the checks offer + * no fix for it. + */ +export function clerkOffersProvider(provider: string): boolean { + return CLERK_OAUTH_STRATEGIES.has(toClerkStrategy(provider)); +} + +/** Human label for a provider key, for report output. */ +export function providerLabel(provider: string): string { + const special: Record = { + github: "GitHub", + gitlab: "GitLab", + linkedin_oidc: "LinkedIn (OIDC)", + slack_oidc: "Slack (OIDC)", + twitter: "Twitter (X)", + azure: "Microsoft (Azure)", + workos: "WorkOS", + fly: "Fly.io", + }; + return special[provider] ?? provider.charAt(0).toUpperCase() + provider.slice(1); +} + +/** + * The Frontend API host of the instance a secret key addresses. + * + * `/v1/instance` carries no publishable key — for any instance, linked or not + * — so the primary domain's `frontend_api_url` is the only route from a secret + * key to the host its settings live behind. Every instance has at least one + * domain; satellites share the primary's Frontend API, so ordering only + * matters for tidiness. + */ +async function fetchFapiHost(secretKey: string): Promise { + const response = await bapiRequest({ method: "GET", path: "/v1/domains", secretKey }); + const domains = (response.body as { data?: unknown })?.data; + if (!Array.isArray(domains)) return null; + + const primary = + domains.find((domain) => !(domain as { is_satellite?: boolean }).is_satellite) ?? domains[0]; + const frontendApiUrl = (primary as { frontend_api_url?: unknown })?.frontend_api_url; + if (typeof frontendApiUrl !== "string" || !frontendApiUrl) return null; + + return new URL(frontendApiUrl).host; +} + +/** + * Fetches the user settings for the instance a secret key addresses. + * + * @returns The settings, or `null` when they could not be read. Callers must + * treat `null` as "unknown" rather than as "nothing is enabled" — the + * import checks say so and skip the checks that need them. + */ +export async function fetchInstanceSettings(secretKey: string): Promise { + try { + const fapiHost = await fetchFapiHost(secretKey); + if (!fapiHost) { + log.debug("migrate: no domain on this instance named a Frontend API URL"); + return null; + } + + // Development FAPI rejects an environment request without a dev browser JWT. + const jwt = + detectInstanceType(secretKey) === "dev" ? await bootstrapDevBrowser(fapiHost) : undefined; + return await fetchUserSettings(fapiHost, jwt ? { jwt } : {}); + } catch (error) { + log.debug( + `migrate: could not read instance settings: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return null; + } +} + +/** The enabled social strategies (`oauth_google`, …) in a settings payload. */ +export function enabledSocialProviders(settings: UserSettingsJSON): string[] { + return Object.entries(settings.social ?? {}) + .filter(([, value]) => value?.enabled) + .map(([strategy]) => strategy); +} + +/** + * Convenience wrapper for callers that only need the enabled strategies. + * + * @returns `null` when the instance settings could not be read. + */ +export async function fetchEnabledSocialProviders(secretKey: string): Promise { + const settings = await fetchInstanceSettings(secretKey); + return settings ? enabledSocialProviders(settings) : null; +} + +/** + * How many users the destination instance already holds. + * + * The closest thing to a live quota check the CLI has: `max_allowed_users` is + * not exposed by any public API, so headroom on a development instance can + * only be estimated from the count and {@link DEV_USER_LIMIT}. + * + * @returns `null` when the count could not be read — an unknown count must not + * be reported as zero. + */ +export async function fetchUserCount(secretKey: string): Promise { + try { + const response = await bapiRequest({ method: "GET", path: "/v1/users/count", secretKey }); + const total = (response.body as { total_count?: unknown })?.total_count; + return typeof total === "number" ? total : null; + } catch (error) { + log.debug( + `migrate: could not read the instance's user count: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return null; + } +} diff --git a/packages/cli-core/src/commands/migrate/lib/modify-settings.test.ts b/packages/cli-core/src/commands/migrate/lib/modify-settings.test.ts new file mode 100644 index 000000000..f48871bf8 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/modify-settings.test.ts @@ -0,0 +1,231 @@ +import { describe, expect, test } from "bun:test"; +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +import type { FieldAnalysis } from "./analysis.ts"; +import { buildReadinessReport } from "./readiness.ts"; +import { buildChangePayload, buildSettingChanges } from "./modify-settings.ts"; + +/** Instance settings carrying only the attributes and providers a test names. */ +function settings(config: { + attributes?: Record; + social?: Record; +}): UserSettingsJSON { + return { + attributes: Object.fromEntries( + Object.entries(config.attributes ?? {}).map(([name, value]) => [ + name, + { enabled: value.enabled, required: value.required ?? false }, + ]), + ), + social: config.social ?? {}, + } as unknown as UserSettingsJSON; +} + +function analysis(overrides: Partial & { totalUsers: number }): FieldAnalysis { + const identifiers = { + verifiedEmails: 0, + unverifiedEmails: 0, + verifiedPhones: 0, + unverifiedPhones: 0, + username: 0, + hasAnyIdentifier: overrides.totalUsers, + ...overrides.identifiers, + }; + return { + identifiers: { + // Hand-built rows have no user overlap, so "any" is the sum. + anyEmail: identifiers.verifiedEmails + identifiers.unverifiedEmails, + anyPhone: identifiers.verifiedPhones + identifiers.unverifiedPhones, + ...identifiers, + }, + fieldCounts: overrides.fieldCounts ?? {}, + totalUsers: overrides.totalUsers, + }; +} + +/** + * Changes are built from a real report rather than hand-written rows, so a row + * whose `key` stops matching the path table fails here instead of silently + * dropping out of the offer. + */ +function changesFor(input: Parameters[0]) { + return buildSettingChanges(buildReadinessReport(input).blocking); +} + +describe("what gets offered", () => { + test("a required field not every user has is offered as a relaxation", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 5, + identifiers: { verifiedEmails: 3, hasAnyIdentifier: 5 } as never, + }), + settings: settings({ attributes: { email_address: { enabled: true, required: true } } }), + }); + + expect(changes).toEqual([ + { + id: "email_address", + label: "Make Email optional at sign-up", + section: "identifiers", + kind: "relax", + writes: [{ path: ["auth_email", "required_for_sign_up"], value: false }], + }, + ]); + }); + + test("a field the file carries but Clerk has switched off is offered as an enable", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 2, + identifiers: { username: 2, hasAnyIdentifier: 2 } as never, + }), + settings: settings({ attributes: { username: { enabled: false } } }), + }); + + expect(changes).toEqual([ + { + id: "username", + label: "Enable Username", + section: "identifiers", + kind: "enable", + writes: [{ path: ["auth_username", "used_for_sign_up"], value: true }], + }, + ]); + }); + + /** + * Clerk refuses a verifiable attribute that is on with no way to verify it — + * `422 phone_number: verifiable attributes need to have at least one + * verification` — and switching the attribute off empties the strategies, so + * every enable that turned one off has to put one back. + */ + test.each([ + ["phone_number", "auth_phone", "phone_code"], + ["email_address", "auth_email", "email_code"], + ])("enabling %s also restores its verification strategy", (attribute, group, strategy) => { + const carriesIt = attribute === "phone_number" ? { verifiedPhones: 2 } : { verifiedEmails: 2 }; + + const changes = changesFor({ + analysis: analysis({ + totalUsers: 2, + identifiers: { ...carriesIt, hasAnyIdentifier: 2 } as never, + }), + settings: settings({ attributes: { [attribute]: { enabled: false } } }), + }); + + expect(changes[0]?.writes).toEqual([ + { path: [group, "used_for_sign_up"], value: true }, + { path: [group, "verification_strategies"], value: [strategy] }, + ]); + expect(buildChangePayload(changes)).toEqual({ + [group]: { used_for_sign_up: true, verification_strategies: [strategy] }, + }); + }); + + // Not verifiable, so no strategy to restore — one write is the whole change. + test("enabling username takes a single write", () => { + const changes = changesFor({ + analysis: analysis({ totalUsers: 2, identifiers: { username: 2 } as never }), + settings: settings({ attributes: { username: { enabled: false } } }), + }); + expect(changes[0]?.writes).toHaveLength(1); + }); + + test("a disabled social provider is offered under Clerk's own strategy name", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 2, + identifiers: { verifiedEmails: 2, hasAnyIdentifier: 2 } as never, + }), + settings: settings({ + attributes: { email_address: { enabled: true } }, + social: { oauth_x: { enabled: false } }, + }), + // Supabase calls it `twitter`; the config document calls it `oauth_x`. + providerCounts: { twitter: 2 }, + }); + + expect(changes).toEqual([ + { + id: "twitter", + label: "Enable Twitter (X) sign-in", + section: "social", + kind: "enable", + writes: [{ path: ["connection_oauth_x", "enabled"], value: true }], + }, + ]); + }); + + // "Could not read" is not "switched off", so nothing is flagged and nothing + // is offered — the report already degrades to a coverage-only listing. + test("nothing is offered when the instance settings could not be read", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 2, + identifiers: { verifiedEmails: 1, hasAnyIdentifier: 2 } as never, + }), + settings: null, + }); + + expect(changes).toEqual([]); + }); + + test("nothing is offered when every field is already configured", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 2, + identifiers: { verifiedEmails: 2, hasAnyIdentifier: 2 } as never, + }), + settings: settings({ attributes: { email_address: { enabled: true, required: true } } }), + }); + + expect(changes).toEqual([]); + }); +}); + +describe("the payload", () => { + test("collapses changes that share a parent into one branch", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 3, + identifiers: { verifiedEmails: 3, hasAnyIdentifier: 3 } as never, + fieldCounts: { firstName: 2, lastName: 1 }, + }), + settings: settings({ + attributes: { + email_address: { enabled: true }, + first_name: { enabled: true, required: true }, + last_name: { enabled: true, required: true }, + }, + }), + }); + + expect(buildChangePayload(changes)).toEqual({ + user_model: { first_name: { required: false }, last_name: { required: false } }, + }); + }); + + test("carries only the changes it is given", () => { + const changes = changesFor({ + analysis: analysis({ + totalUsers: 3, + identifiers: { verifiedEmails: 2, hasAnyIdentifier: 3 } as never, + fieldCounts: { password: 1 }, + }), + settings: settings({ + attributes: { + email_address: { enabled: true, required: true }, + password: { enabled: true, required: true }, + }, + }), + }); + expect(changes.map((change) => change.id)).toEqual(["email_address", "password"]); + + expect(buildChangePayload(changes.filter((change) => change.id === "password"))).toEqual({ + auth_password: { required: false }, + }); + }); + + test("is empty when nothing was selected", () => { + expect(buildChangePayload([])).toEqual({}); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/modify-settings.ts b/packages/cli-core/src/commands/migrate/lib/modify-settings.ts new file mode 100644 index 000000000..6ed1cc506 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/modify-settings.ts @@ -0,0 +1,146 @@ +/** + * Turning a flagged readiness row into the instance-config change that would + * stop it being flagged, printed as a `clerk config patch` command. + * + * The checks already know which settings will cost users; without this the + * only way to act on them is to leave the CLI, find the setting in the + * dashboard, and come back. Each change is a single leaf in the Platform API's + * config document. + * + * These are offers, not corrections. A flagged setting is not a wrong setting — + * an instance that genuinely requires an email address is configured exactly as + * its owner intended, and the right answer may well be to fix the export + * instead. Nothing here is preselected and nothing is applied unasked. + * + * Only the two verdicts `buildReadinessReport` produces are mapped: "required + * in Clerk" (relax the requirement) and "not enabled in Clerk" (turn it on). A + * row this file has no path for is simply not offered — the report still names + * it and still points at the dashboard. + */ + +import { clerkOffersProvider, toClerkStrategy } from "./clerk-config.ts"; +import type { ReadinessItem, ReadinessSection } from "./readiness.ts"; + +/** One leaf of the config document, and what to set it to. */ +export type SettingWrite = { path: string[]; value: boolean | string[] }; + +/** One offered change: what it says, and the config leaves it writes. */ +export type SettingChange = { + /** Stable identity for the multiselect, and for tests. */ + id: string; + label: string; + section: ReadinessSection; + /** `enable` turns something on; `relax` drops a requirement. */ + kind: "enable" | "relax"; + writes: SettingWrite[]; +}; + +type ChangeWrites = { enable: SettingWrite[]; relax: SettingWrite[] }; + +/** + * Where each attribute lives in the config document. `used_for_sign_up` is the + * enable field that matters here: `POST /v1/users` validates an import against + * the instance's sign-up requirements, not its sign-in strategies. + * + * Email and phone are **verifiable** attributes, so enabling one takes two + * writes rather than one. Clerk rejects a verifiable attribute that is on with + * no way to verify it — `422 phone_number: verifiable attributes need to have + * at least one verification` — and switching the attribute off empties + * `verification_strategies`, so whatever turns it back on has to put a strategy + * back. Username, password and the name fields are not verifiable and take one + * write each. + */ +const ATTRIBUTE_WRITES: Record = { + email_address: { + enable: [ + { path: ["auth_email", "used_for_sign_up"], value: true }, + { path: ["auth_email", "verification_strategies"], value: ["email_code"] }, + ], + relax: [{ path: ["auth_email", "required_for_sign_up"], value: false }], + }, + phone_number: { + enable: [ + { path: ["auth_phone", "used_for_sign_up"], value: true }, + { path: ["auth_phone", "verification_strategies"], value: ["phone_code"] }, + ], + relax: [{ path: ["auth_phone", "required_for_sign_up"], value: false }], + }, + username: { + enable: [{ path: ["auth_username", "used_for_sign_up"], value: true }], + relax: [{ path: ["auth_username", "required_for_sign_up"], value: false }], + }, + password: { + enable: [{ path: ["auth_password", "enabled"], value: true }], + relax: [{ path: ["auth_password", "required"], value: false }], + }, + first_name: { + enable: [{ path: ["user_model", "first_name", "enabled"], value: true }], + relax: [{ path: ["user_model", "first_name", "required"], value: false }], + }, + last_name: { + enable: [{ path: ["user_model", "last_name", "enabled"], value: true }], + relax: [{ path: ["user_model", "last_name", "required"], value: false }], + }, +}; + +/** `github` → `connection_oauth_github`, via Clerk's own strategy name. */ +function socialPath(provider: string): string[] { + return [`connection_oauth_${toClerkStrategy(provider).replace(/^oauth_/, "")}`, "enabled"]; +} + +function changeFor(item: ReadinessItem): SettingChange | undefined { + // Required-but-not-universal is the only verdict that relaxes rather than + // enables; every other flagged row is something switched off in Clerk. + const relax = item.clerkRequired === true; + + if (item.section === "social") { + // A provider has no "required" in Clerk, so there is nothing to relax, and + // one Clerk doesn't offer has nothing to turn on. + if (relax || !clerkOffersProvider(item.key)) return undefined; + return { + id: item.key, + label: `Enable ${item.label} sign-in`, + section: item.section, + kind: "enable", + writes: [{ path: socialPath(item.key), value: true }], + }; + } + + const writes = ATTRIBUTE_WRITES[item.key]; + if (!writes) return undefined; + + return { + id: item.key, + label: relax ? `Make ${item.label} optional at sign-up` : `Enable ${item.label}`, + section: item.section, + kind: relax ? "relax" : "enable", + writes: relax ? writes.relax : writes.enable, + }; +} + +/** The changes offerable for a report's flagged rows, in report order. */ +export function buildSettingChanges(flagged: ReadinessItem[]): SettingChange[] { + return flagged.map(changeFor).filter((change): change is SettingChange => change !== undefined); +} + +/** + * Collapses the chosen changes into one config payload. + * + * Changes share parents — `first_name` and `last_name` both write `user_model` + * — so leaves are written into a shared tree rather than merged after the fact. + */ +export function buildChangePayload( + changes: Pick[], +): Record { + const payload: Record = {}; + + for (const write of changes.flatMap((change) => change.writes)) { + let node = payload; + for (const key of write.path.slice(0, -1)) { + node = (node[key] ??= {}) as Record; + } + node[write.path[write.path.length - 1] as string] = write.value; + } + + return payload; +} diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.test.ts b/packages/cli-core/src/commands/migrate/lib/readiness.test.ts new file mode 100644 index 000000000..76d9a4eb4 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/readiness.test.ts @@ -0,0 +1,281 @@ +import { describe, expect, test } from "bun:test"; +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +import { analyzeFields, type FieldAnalysis } from "./analysis.ts"; +import { buildReadinessReport, type ReadinessItem } from "./readiness.ts"; + +/** Instance settings carrying only the attributes and providers a test names. */ +function settings(config: { + attributes?: Record; + social?: Record; +}): UserSettingsJSON { + return { + attributes: Object.fromEntries( + Object.entries(config.attributes ?? {}).map(([name, value]) => [ + name, + { enabled: value.enabled, required: value.required ?? false }, + ]), + ), + social: config.social ?? {}, + } as unknown as UserSettingsJSON; +} + +/** Field analysis with everything absent unless the test says otherwise. */ +function analysis(overrides: Partial & { totalUsers: number }): FieldAnalysis { + const identifiers = { + verifiedEmails: 0, + unverifiedEmails: 0, + verifiedPhones: 0, + unverifiedPhones: 0, + username: 0, + hasAnyIdentifier: overrides.totalUsers, + ...overrides.identifiers, + }; + return { + identifiers: { + // Hand-built rows have no user overlap, so "any" is the sum. + anyEmail: identifiers.verifiedEmails + identifiers.unverifiedEmails, + anyPhone: identifiers.verifiedPhones + identifiers.unverifiedPhones, + ...identifiers, + }, + fieldCounts: overrides.fieldCounts ?? {}, + totalUsers: overrides.totalUsers, + }; +} + +const item = (report: { items: ReadinessItem[] }, label: string) => + report.items.find((entry) => entry.label === label); + +describe("which rows appear", () => { + test("reports only the fields the file actually carries", () => { + const report = buildReadinessReport({ + analysis: analysis({ totalUsers: 3, identifiers: { verifiedEmails: 3 } as never }), + settings: settings({ attributes: { email_address: { enabled: true } } }), + }); + expect(report.items.map((entry) => entry.label)).toEqual(["Email"]); + }); + + test("counts verified and unverified identifiers together", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 5, + identifiers: { verifiedEmails: 3, unverifiedEmails: 2, hasAnyIdentifier: 5 } as never, + }), + settings: settings({ attributes: { email_address: { enabled: true } } }), + }); + expect(item(report, "Email")?.userCount).toBe(5); + }); + + // A user with both a verified and an unverified phone is one user, not two. + test("counts a user with both kinds of phone once", () => { + const users = [ + { userId: "a", phone: "+15555550100", unverifiedPhoneNumbers: ["+15555550101"] }, + { userId: "b", phone: "+15555550102", unverifiedPhoneNumbers: ["+15555550103"] }, + { userId: "c", email: "c@x.dev" }, + ]; + const report = buildReadinessReport({ + analysis: analyzeFields(users), + settings: settings({ attributes: { phone_number: { enabled: true } } }), + }); + expect(item(report, "Phone")?.userCount).toBe(2); + }); + + test("groups rows into identifiers, auth and user model", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 2, + identifiers: { verifiedEmails: 2, username: 2, hasAnyIdentifier: 2 } as never, + fieldCounts: { password: 2, firstName: 2, lastName: 1 }, + }), + settings: settings({}), + }); + expect(report.items.map((entry) => [entry.label, entry.section])).toEqual([ + ["Email", "identifiers"], + ["Username", "identifiers"], + ["Password", "auth"], + ["First name", "model"], + ["Last name", "model"], + ]); + }); +}); + +describe("required in Clerk but missing from the file", () => { + // The expensive case: those users fail one at a time, mid-import, after + // earlier users have already been created. + test("flags an attribute Clerk requires that not every user has", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 10, + identifiers: { verifiedEmails: 7, hasAnyIdentifier: 10, username: 10 } as never, + }), + settings: settings({ + attributes: { + email_address: { enabled: true, required: true }, + username: { enabled: true }, + }, + }), + }); + + const email = item(report, "Email"); + expect(email?.blocking).toBe(true); + expect(email?.detail).toContain("required in Clerk"); + // A required identifier is the one verdict Clerk refuses the user over. + expect(email?.consequence).toBe("rejects"); + expect(report.blocking).toHaveLength(1); + }); + + test("does not flag a required attribute every user has", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 4, + identifiers: { verifiedEmails: 4, hasAnyIdentifier: 4 } as never, + }), + settings: settings({ attributes: { email_address: { enabled: true, required: true } } }), + }); + expect(report.blocking).toHaveLength(0); + }); + + test("does not flag an enabled-but-optional attribute that some users lack", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 10, + identifiers: { verifiedEmails: 10, hasAnyIdentifier: 10 } as never, + fieldCounts: { firstName: 2 }, + }), + settings: settings({ + attributes: { email_address: { enabled: true }, first_name: { enabled: true } }, + }), + }); + expect(report.blocking).toHaveLength(0); + }); + + // The import sends `skip_password_requirement`, so a required password costs + // the user their password rather than their whole account. + test("a required password drops rather than rejects", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 4, + identifiers: { verifiedEmails: 4, hasAnyIdentifier: 4 } as never, + fieldCounts: { password: 1 }, + }), + settings: settings({ + attributes: { + email_address: { enabled: true }, + password: { enabled: true, required: true }, + }, + }), + }); + expect(item(report, "Password")?.consequence).toBe("drops"); + }); +}); + +describe("present in the file but disabled in Clerk", () => { + test("flags an attribute the instance has switched off", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 3, + identifiers: { verifiedEmails: 3, username: 3, hasAnyIdentifier: 3 } as never, + }), + settings: settings({ + attributes: { email_address: { enabled: true }, username: { enabled: false } }, + }), + }); + + const username = item(report, "Username"); + expect(username?.blocking).toBe(true); + expect(username?.detail).toBe("not enabled in Clerk"); + }); + + test("flags a social provider users signed up with that Clerk lacks", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 4, + identifiers: { verifiedEmails: 4, hasAnyIdentifier: 4 } as never, + }), + settings: settings({ + attributes: { email_address: { enabled: true } }, + social: { oauth_google: { enabled: true } }, + }), + providerCounts: { google: 3, discord: 1 }, + }); + + expect(item(report, "Google")?.blocking).toBe(false); + expect(item(report, "Discord")?.blocking).toBe(true); + expect(report.blocking.map((entry) => entry.label)).toEqual(["Discord"]); + }); + + test("maps a provider whose Clerk strategy name differs", () => { + const report = buildReadinessReport({ + analysis: analysis({ totalUsers: 1, identifiers: { verifiedEmails: 1 } as never }), + settings: settings({ + attributes: { email_address: { enabled: true } }, + social: { oauth_microsoft: { enabled: true } }, + }), + providerCounts: { azure: 1 }, + }); + expect(item(report, "Microsoft (Azure)")?.blocking).toBe(false); + }); + + test("ignores a provider no user actually signed up with", () => { + const report = buildReadinessReport({ + analysis: analysis({ totalUsers: 1, identifiers: { verifiedEmails: 1 } as never }), + settings: settings({ attributes: { email_address: { enabled: true } } }), + providerCounts: { discord: 0 }, + }); + expect(item(report, "Discord")).toBeUndefined(); + }); +}); + +describe("when the instance settings cannot be read", () => { + const unreadable = () => + buildReadinessReport({ + analysis: analysis({ + totalUsers: 3, + identifiers: { verifiedEmails: 3, hasAnyIdentifier: 3 } as never, + }), + settings: null, + }); + + test("marks the report as degraded rather than failing", () => { + expect(unreadable().settingsUnavailable).toBe(true); + }); + + // `null` means "not read", which must not be confused with `false` + // ("read, and it is off") — the latter blocks, the former cannot. + test("claims nothing about Clerk, so nothing blocks", () => { + const report = unreadable(); + expect(item(report, "Email")?.clerkEnabled).toBeNull(); + expect(report.blocking).toHaveLength(0); + }); + + test("still reports what the file contains", () => { + expect(unreadable().items.map((entry) => entry.label)).toEqual(["Email"]); + }); +}); + +describe("file-level totals", () => { + test("counts users with no identifier at all", () => { + const report = buildReadinessReport({ + analysis: analysis({ + totalUsers: 10, + identifiers: { verifiedEmails: 7, hasAnyIdentifier: 7 } as never, + }), + settings: settings({}), + }); + expect(report.withoutIdentifier).toBe(3); + }); + + test("carries the validation failure count through", () => { + const report = buildReadinessReport({ + analysis: analysis({ totalUsers: 2 }), + settings: settings({}), + validationFailed: 5, + }); + expect(report.validationFailed).toBe(5); + }); +}); + +/** + * The counts an operator actually decides on. Built from the users themselves, + * because per-field coverage cannot answer them: the users missing an email and + * the users missing a password overlap by an amount only a per-user pass knows. + */ diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts new file mode 100644 index 000000000..bd445405d --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -0,0 +1,215 @@ +/** + * What the import file contains, cross-referenced against what the + * destination instance accepts, one row per field. + * + * Ported from the standalone migration-tool's `displayCrossReference`. The + * import's checks read the flagged rows for their drop warnings and their + * `clerk config patch` fixes; which users are rejected is decided per user in + * `checks.ts`. + */ + +import type { UserSettingsJSON } from "../../../lib/fapi.ts"; +// Pure attribute lookups, shared with the `users` create wizard. +import { isEnabled, isRequired, type AttributeName } from "../../users/interactive/attributes.ts"; +import type { FieldAnalysis } from "./analysis.ts"; +import { clerkOffersProvider, providerLabel, toClerkStrategy } from "./clerk-config.ts"; + +export type ReadinessSection = "identifiers" | "auth" | "social" | "model"; + +/** + * One row of the report. + * + * @property clerkEnabled - `null` when the instance settings could not be read, + * which is different from `false` ("read, and it is off"). + * @property blocking - This row will cost users unless the operator acts. + */ +export type ReadinessItem = { + label: string; + /** + * What the row is about, machine-side: an {@link AttributeName} for every + * section but `social`, and the source platform's provider key for that one. + * `label` is for humans; this is what `modify-settings.ts` looks up. + */ + key: string; + section: ReadinessSection; + /** Users in the file that carry this field or provider. */ + userCount: number; + clerkEnabled: boolean | null; + clerkRequired: boolean | null; + blocking: boolean; + /** + * What this row costs the users it affects. + * + * - `rejects` — Clerk refuses the user outright. Only a required identifier + * does this: `POST /v1/users` enforces the instance's sign-up identifier + * requirements, and a user carrying none of them has nothing to be created + * under. + * - `drops` — the user is created, but this piece of them is not. A required + * password is in this group rather than `rejects` because the import sends + * `skip_password_requirement` (see `import-users.ts`), so the user lands + * without one and has to reset it before they can sign in that way. + */ + consequence?: "rejects" | "drops"; + /** Why it blocks — omitted when it does not. */ + detail?: string; +}; + +export type ReadinessReport = { + totalUsers: number; + /** Users with no identifier at all; they cannot be imported under any settings. */ + withoutIdentifier: number; + validationFailed: number; + items: ReadinessItem[]; + /** Every item flagged `blocking`, in report order. */ + blocking: ReadinessItem[]; + /** True when the instance settings could not be read. */ + settingsUnavailable: boolean; +}; + +type BuildInput = { + analysis: FieldAnalysis; + /** `null` when no publishable key was available, or FAPI could not be read. */ + settings: UserSettingsJSON | null; + validationFailed?: number; + /** Source-platform provider key → user count. Supabase exports only. */ + providerCounts?: Record; +}; + +/** + * Identifiers Clerk creates a user *under*. A required one that a user does not + * carry leaves nothing to create them with, so the API refuses them — which is + * why these are the only attributes whose consequence is `rejects`. + */ +/** Attributes `POST /v1/users` refuses a user without, when they are required. */ +const REJECTING_ATTRIBUTES = new Set([ + "email_address", + "phone_number", + "username", + "first_name", + "last_name", +]); + +/** An identifier or user-model row, with its blocking verdict. */ +function buildAttributeItem( + label: string, + section: ReadinessSection, + attribute: AttributeName, + userCount: number, + settings: UserSettingsJSON | null, + totalUsers: number, +): ReadinessItem { + const enabled = settings ? isEnabled(settings, attribute) : null; + const required = settings ? isRequired(settings, attribute) : null; + const missing = totalUsers - userCount; + + // Required but not universal is the expensive case: those users fail one by + // one, mid-import, after earlier users have already been created. + if (required === true && missing > 0) { + return { + label, + key: attribute, + section, + userCount, + clerkEnabled: enabled, + clerkRequired: required, + blocking: true, + consequence: REJECTING_ATTRIBUTES.has(attribute) ? "rejects" : "drops", + // How many users this costs is the outcome block's job. Restating it here + // reads as a contradiction, because that block counts each user once and + // this row counts the field — a user missing both an email and a password + // appears in both rows but only in the first outcome. + detail: "required in Clerk, and not every user has one", + }; + } + + // Present in the file but switched off in Clerk: the data is silently dropped. + if (enabled === false && userCount > 0) { + return { + label, + key: attribute, + section, + userCount, + clerkEnabled: enabled, + clerkRequired: required, + blocking: true, + consequence: "drops", + detail: "not enabled in Clerk", + }; + } + + return { + label, + key: attribute, + section, + userCount, + clerkEnabled: enabled, + clerkRequired: required, + blocking: false, + }; +} + +/** + * Cross-references the file against the instance. + * + * A field absent from the file contributes no row — the report describes what + * is actually being imported, not every setting Clerk supports. + */ +export function buildReadinessReport(input: BuildInput): ReadinessReport { + const { analysis, settings, validationFailed = 0, providerCounts = {} } = input; + const total = analysis.totalUsers; + const items: ReadinessItem[] = []; + + // Users, not fields: one with both a verified and an unverified phone is one. + const emailCount = analysis.identifiers.anyEmail; + const phoneCount = analysis.identifiers.anyPhone; + + const attributeRows: [string, ReadinessSection, AttributeName, number][] = [ + ["Email", "identifiers", "email_address", emailCount], + ["Phone", "identifiers", "phone_number", phoneCount], + ["Username", "identifiers", "username", analysis.identifiers.username], + ["Password", "auth", "password", analysis.fieldCounts.password ?? 0], + ["First name", "model", "first_name", analysis.fieldCounts.firstName ?? 0], + ["Last name", "model", "last_name", analysis.fieldCounts.lastName ?? 0], + ]; + + for (const [label, section, attribute, count] of attributeRows) { + // A required field nobody has still costs every user, so it gets a row. + if (count > 0 || (settings && isRequired(settings, attribute))) { + items.push(buildAttributeItem(label, section, attribute, count, settings, total)); + } + } + + for (const [provider, count] of Object.entries(providerCounts)) { + if (count === 0) continue; + const enabled = settings + ? (settings.social?.[toClerkStrategy(provider) as keyof typeof settings.social]?.enabled ?? + false) + : null; + items.push({ + label: providerLabel(provider), + key: provider, + section: "social", + userCount: count, + clerkEnabled: enabled, + clerkRequired: null, + blocking: enabled === false, + ...(enabled === false + ? { + consequence: "drops" as const, + detail: clerkOffersProvider(provider) ? "not enabled in Clerk" : "not offered by Clerk", + } + : {}), + }); + } + + const blocking = items.filter((item) => item.blocking); + + return { + totalUsers: total, + withoutIdentifier: total - analysis.identifiers.hasAnyIdentifier, + validationFailed, + items, + blocking, + settingsUnavailable: settings === null, + }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/supabase-providers.test.ts b/packages/cli-core/src/commands/migrate/lib/supabase-providers.test.ts new file mode 100644 index 000000000..00cde92c5 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/supabase-providers.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, test } from "bun:test"; +import { toClerkStrategy } from "./clerk-config.ts"; +import { + countProviders, + findDisabledProviders, + findUsersWithOnlyDisabledProviders, + getUserProviders, +} from "./supabase-providers.ts"; + +/** A Supabase row carrying the given providers, in the JSON export's shape. */ +const user = (id: string, providers: string[] | string | undefined) => ({ + id, + raw_app_meta_data: + providers === undefined ? undefined : JSON.stringify({ provider: "email", providers }), +}); + +describe("getUserProviders", () => { + test("reads providers from a JSON-string column, as a CSV export writes it", () => { + expect(getUserProviders(user("u1", ["email", "discord"]))).toEqual(["email", "discord"]); + }); + + test("reads providers from an object column, as a JSON export writes it", () => { + expect(getUserProviders({ id: "u1", raw_app_meta_data: { providers: ["google"] } })).toEqual([ + "google", + ]); + }); + + test("splits a delimited providers string", () => { + expect( + getUserProviders({ id: "u1", raw_app_meta_data: { providers: "email, discord" } }), + ).toEqual(["email", "discord"]); + }); + + test.each([ + ["missing column", { id: "u1" }], + ["unparseable column", { id: "u1", raw_app_meta_data: "{not json" }], + ["array column", { id: "u1", raw_app_meta_data: "[]" }], + ["no providers key", { id: "u1", raw_app_meta_data: '{"provider":"email"}' }], + ])("returns nothing for a %s", (_label, row) => { + expect(getUserProviders(row)).toEqual([]); + }); +}); + +describe("toClerkStrategy", () => { + test.each([ + ["google", "oauth_google"], + ["discord", "oauth_discord"], + ["github", "oauth_github"], + ["azure", "oauth_microsoft"], + ["twitter", "oauth_x"], + ["slack_oidc", "oauth_slack"], + ])("%s -> %s", (provider, strategy) => { + expect(toClerkStrategy(provider)).toBe(strategy); + }); +}); + +describe("countProviders", () => { + test("counts each provider across the export", () => { + expect( + countProviders([ + user("u1", ["email"]), + user("u2", ["email", "discord"]), + user("u3", ["discord"]), + ]), + ).toEqual({ email: 2, discord: 2 }); + }); +}); + +describe("findDisabledProviders", () => { + test("names the social providers Clerk does not have enabled", () => { + const rows = [user("u1", ["email", "google"]), user("u2", ["discord"])]; + expect(findDisabledProviders(rows, ["oauth_google"], toClerkStrategy)).toEqual(["discord"]); + }); + + test("never treats email or phone as disabled", () => { + const rows = [user("u1", ["email"]), user("u2", ["phone"]), user("u3", ["anonymous_users"])]; + expect(findDisabledProviders(rows, [], toClerkStrategy)).toEqual([]); + }); + + test("returns nothing when every provider is enabled", () => { + const rows = [user("u1", ["google"]), user("u2", ["github"])]; + expect(findDisabledProviders(rows, ["oauth_google", "oauth_github"], toClerkStrategy)).toEqual( + [], + ); + }); +}); + +describe("findUsersWithOnlyDisabledProviders", () => { + test("excludes a user whose sole provider is disabled", () => { + const result = findUsersWithOnlyDisabledProviders([user("u1", ["discord"])], ["discord"]); + expect([...result.excludedIds]).toEqual(["u1"]); + expect(result.byProvider).toEqual({ discord: 1 }); + }); + + test("keeps a user who can still sign in with email", () => { + const result = findUsersWithOnlyDisabledProviders( + [user("u1", ["email", "discord"])], + ["discord"], + ); + expect(result.excludedIds.size).toBe(0); + }); + + test("keeps a user who has another enabled social provider", () => { + const result = findUsersWithOnlyDisabledProviders( + [user("u1", ["google", "discord"])], + ["discord"], + ); + expect(result.excludedIds.size).toBe(0); + }); + + test("excludes a user whose every provider is disabled", () => { + const result = findUsersWithOnlyDisabledProviders( + [user("u1", ["discord", "twitch"])], + ["discord", "twitch"], + ); + expect([...result.excludedIds]).toEqual(["u1"]); + expect(result.byProvider).toEqual({ discord: 1, twitch: 1 }); + }); + + test("keeps a user with no provider data at all", () => { + const result = findUsersWithOnlyDisabledProviders([user("u1", undefined)], ["discord"]); + expect(result.excludedIds.size).toBe(0); + }); + + test("excludes nobody when no provider is disabled", () => { + const result = findUsersWithOnlyDisabledProviders([user("u1", ["discord"])], []); + expect(result.excludedIds.size).toBe(0); + }); + + test("reports a per-provider breakdown across many users", () => { + const result = findUsersWithOnlyDisabledProviders( + [ + user("u1", ["discord"]), + user("u2", ["discord"]), + user("u3", ["twitch"]), + user("u4", ["email", "discord"]), + ], + ["discord", "twitch"], + ); + expect([...result.excludedIds]).toEqual(["u1", "u2", "u3"]); + expect(result.byProvider).toEqual({ discord: 2, twitch: 1 }); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/supabase-providers.ts b/packages/cli-core/src/commands/migrate/lib/supabase-providers.ts new file mode 100644 index 000000000..d123dd5e0 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/supabase-providers.ts @@ -0,0 +1,145 @@ +/** + * Cross-references the social providers in a Supabase export against what the + * destination Clerk instance has enabled. + * + * Ported from the standalone migration-tool's `src/lib/supabase.ts`, minus its + * hand-rolled CSV parser — the export is read through the same + * `readRawUsers` path every other Supabase read uses. + */ + +import { readRawUsers } from "./transform.ts"; + +/** + * Supabase lists these alongside social providers in `providers`, but they are + * built into Clerk and can never be "not enabled". + */ +export const NON_SOCIAL_PROVIDERS = new Set(["email", "phone", "anonymous_users"]); + +function parseMaybeJson(value: unknown): unknown { + if (typeof value !== "string") return value; + try { + return JSON.parse(value); + } catch { + return value; + } +} + +/** + * Reads a user's auth providers from `raw_app_meta_data`. + * + * The column arrives as a JSON string from a CSV export and as an object from + * a JSON one, and its `providers` value is itself sometimes a string. + */ +export function getUserProviders(user: Record): string[] { + const appMeta = parseMaybeJson(user.raw_app_meta_data); + if (!appMeta || typeof appMeta !== "object" || Array.isArray(appMeta)) return []; + + const providers = parseMaybeJson((appMeta as Record).providers); + if (Array.isArray(providers)) { + return providers.map((provider) => String(provider).trim()).filter(Boolean); + } + if (typeof providers === "string") { + return providers + .split(/[,|]/) + .map((provider) => provider.trim()) + .filter(Boolean); + } + return []; +} + +export type ProviderExclusions = { + /** Source IDs of users to skip. */ + excludedIds: Set; + /** How many excluded users each disabled provider accounts for. */ + byProvider: Record; +}; + +/** + * Finds the users whose *only* way in is a provider Clerk does not have + * enabled. + * + * A user keeps their place if any one of their providers still works — + * including email and phone. Excluding on "has at least one disabled provider" + * instead would drop users who could sign in perfectly well another way. + * + * @param disabled - Supabase provider keys not enabled in Clerk. + */ +export function findUsersWithOnlyDisabledProviders( + users: Record[], + disabled: string[], +): ProviderExclusions { + const empty: ProviderExclusions = { excludedIds: new Set(), byProvider: {} }; + if (disabled.length === 0) return empty; + + const disabledSet = new Set(disabled); + const excludedIds = new Set(); + const byProvider: Record = {}; + + for (const user of users) { + const providers = getUserProviders(user); + // No provider data means no basis to exclude — err towards importing. + if (providers.length === 0) continue; + + const hasUsableProvider = providers.some( + (provider) => NON_SOCIAL_PROVIDERS.has(provider) || !disabledSet.has(provider), + ); + if (hasUsableProvider) continue; + + excludedIds.add(String(user.id)); + for (const provider of providers.filter((p) => disabledSet.has(p))) { + byProvider[provider] = (byProvider[provider] ?? 0) + 1; + } + } + + return { excludedIds, byProvider }; +} + +/** Counts users per provider across the export, for reporting. */ +export function countProviders(users: Record[]): Record { + const counts: Record = {}; + for (const user of users) { + for (const provider of getUserProviders(user)) { + counts[provider] = (counts[provider] ?? 0) + 1; + } + } + return counts; +} + +/** + * The same counts, minus Supabase's pseudo-providers. + * + * Supabase lists `email` and `phone` in `providers` next to real connections, + * but Clerk has no `oauth_email` to enable — so anything cross-referencing + * against the instance's social settings must drop them, or every + * password-based user reads as "not enabled in Clerk". + */ +export function countSocialProviders(users: Record[]): Record { + return Object.fromEntries( + Object.entries(countProviders(users)).filter( + ([provider]) => !NON_SOCIAL_PROVIDERS.has(provider), + ), + ); +} + +/** + * Every social provider present in the export that Clerk does not have + * enabled. + * + * @param enabledStrategies - Clerk strategy names (`oauth_google`, …). + * @param toStrategy - Maps a Supabase provider key to its Clerk strategy. + */ +export function findDisabledProviders( + users: Record[], + enabledStrategies: string[], + toStrategy: (provider: string) => string, +): string[] { + const enabled = new Set(enabledStrategies); + return Object.keys(countSocialProviders(users)).filter( + (provider) => !enabled.has(toStrategy(provider)), + ); +} + +/** Reads a Supabase export and returns its raw rows for provider analysis. */ +export async function readSupabaseRows(file: string): Promise[]> { + return readRawUsers(file, "supabase"); +} diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts new file mode 100644 index 000000000..dca4a8673 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -0,0 +1,110 @@ +/** + * Batched `GET /v1/users` lookups by a list of values. + * + * BAPI filters on up to 100 values per call and leaves out any it does not + * find, so the work is proportional to the list rather than to the instance. + * Every call goes through the run's scheduler and backs off on a 429, exactly + * as the writes do. + */ + +import { bapiRequest } from "../../../lib/bapi.ts"; +import type { SpinnerControls } from "../../../lib/spinner.ts"; +import { retryOn429 } from "./retry.ts"; +import { clerkIdsCreatedByOtherRuns } from "./run-store.ts"; +import type { ApiScheduler } from "./scheduler.ts"; + +/** BAPI accepts at most 100 values per filter on `GET /v1/users`. */ +export const LOOKUP_BATCH = 100; + +/** The fields of a BAPI user these lookups read. */ +export type LookedUpUser = { + id: string; + external_id?: string | null; + username?: string | null; + last_sign_in_at?: number | null; + email_addresses?: { email_address?: string }[]; + phone_numbers?: { phone_number?: string }[]; +}; + +export type LookupFilter = + | "user_id" + | "external_id" + | "email_address" + | "phone_number" + | "username"; + +/** Splits `items` into chunks of at most `size`. */ +export function batch(items: T[], size: number): T[][] { + const batches: T[][] = []; + for (let i = 0; i < items.length; i += size) batches.push(items.slice(i, i + size)); + return batches; +} + +/** Every user matching any of `values` on `filter`. */ +export async function lookupUsers(options: { + filter: LookupFilter; + values: string[]; + secretKey: string; + schedule: ApiScheduler; + spinner?: SpinnerControls; + label?: string; +}): Promise { + const batches = batch([...new Set(options.values)], LOOKUP_BATCH); + let done = 0; + + const pages = await Promise.all( + batches.map(async (values) => { + const params = new URLSearchParams(); + params.set("limit", String(LOOKUP_BATCH)); + // BAPI reads a leading `-` on an `external_id` as "exclude" and strips a + // leading `+`, so an explicit `+` keeps a source ID like `-abc` literal. + for (const value of values) { + params.append(options.filter, options.filter === "external_id" ? `+${value}` : value); + } + + const response = await retryOn429(async () => + options.schedule(async () => + bapiRequest({ + method: "GET", + path: `/v1/users?${params.toString()}`, + secretKey: options.secretKey, + }), + ), + ); + done++; + options.spinner?.update(`${options.label ?? "Looking up users"}: ${done}/${batches.length}`); + const users = response.body; + return Array.isArray(users) ? (users as LookedUpUser[]) : []; + }), + ); + + return pages.flat().filter((user) => typeof user.id === "string"); +} + +/** + * The users behind creates that were in flight when run `runId` stopped. + * + * Found by `external_id`, which the import's checks refused to reuse, but a + * later run of the same source IDs can still have created one after this run + * stopped. So any Clerk ID another import run records as created is left out. + */ +export async function findInFlight(options: { + runsDir: string; + runId: string; + sourceIds: string[]; + secretKey: string; + schedule: ApiScheduler; +}): Promise<{ sourceId: string; clerkId: string }[]> { + if (options.sourceIds.length === 0) return []; + const found = await lookupUsers({ + filter: "external_id", + values: options.sourceIds, + secretKey: options.secretKey, + schedule: options.schedule, + }); + const otherRuns = clerkIdsCreatedByOtherRuns(options.runsDir, options.runId); + const wanted = new Set(options.sourceIds); + return found + .filter((user) => user.external_id && wanted.has(user.external_id) && !otherRuns.has(user.id)) + .map((user) => ({ sourceId: user.external_id as string, clerkId: user.id })); +} From 70e24dbb29b585e816670eb877154cec008fc637 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:23 -0400 Subject: [PATCH 02/34] feat(migrate): trim instance reads, readiness and checks to slice 1 Cuts the files ported from #479 down to slice 1's scope, as slice-prs.md sets it, and adds the files slice 1 needs that #479 does not have. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 9 ------ .../src/commands/migrate/lib/checks.ts | 6 ---- .../src/commands/migrate/lib/user-lookup.ts | 29 ------------------- 3 files changed, 44 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 858a609a1..8649a8799 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -274,15 +274,6 @@ describe("rejects", () => { expect(checks.importable.map((u) => u.userId)).toEqual(["kept"]); }); - // A continued run found this user behind its own in-flight create. - test("not a user the continued run adopted", async () => { - existing = [{ id: "user_1", external_id: "mine" }]; - - expect( - await reasonsOf({ users: [user("mine")], adoptedClerkIds: new Set(["user_1"]) }), - ).toEqual({}); - }); - test("a supabase user whose only provider is disabled", async () => { const rows = [ { id: "only-discord", raw_app_meta_data: { providers: ["discord"] } }, diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index a384588ac..1c2df0bbd 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -104,11 +104,6 @@ export type CheckInput = { * sending `skip_legal_checks`. Without it they are rejected. */ skipLegalChecks?: boolean; - /** - * Clerk IDs a continued run found behind its own in-flight creates: finding - * them in the instance is expected. - */ - adoptedClerkIds?: Set; spinner?: SpinnerControls; }; @@ -541,7 +536,6 @@ async function findInstanceDuplicates( }; for (const existing of found) { - if (input.adoptedClerkIds?.has(existing.id)) continue; if (existing.external_id) { claim(byExternalId.get(existing.external_id), "already in the instance, with this source ID"); } diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index dca4a8673..1c8990d1c 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -10,7 +10,6 @@ import { bapiRequest } from "../../../lib/bapi.ts"; import type { SpinnerControls } from "../../../lib/spinner.ts"; import { retryOn429 } from "./retry.ts"; -import { clerkIdsCreatedByOtherRuns } from "./run-store.ts"; import type { ApiScheduler } from "./scheduler.ts"; /** BAPI accepts at most 100 values per filter on `GET /v1/users`. */ @@ -80,31 +79,3 @@ export async function lookupUsers(options: { return pages.flat().filter((user) => typeof user.id === "string"); } - -/** - * The users behind creates that were in flight when run `runId` stopped. - * - * Found by `external_id`, which the import's checks refused to reuse, but a - * later run of the same source IDs can still have created one after this run - * stopped. So any Clerk ID another import run records as created is left out. - */ -export async function findInFlight(options: { - runsDir: string; - runId: string; - sourceIds: string[]; - secretKey: string; - schedule: ApiScheduler; -}): Promise<{ sourceId: string; clerkId: string }[]> { - if (options.sourceIds.length === 0) return []; - const found = await lookupUsers({ - filter: "external_id", - values: options.sourceIds, - secretKey: options.secretKey, - schedule: options.schedule, - }); - const otherRuns = clerkIdsCreatedByOtherRuns(options.runsDir, options.runId); - const wanted = new Set(options.sourceIds); - return found - .filter((user) => user.external_id && wanted.has(user.external_id) && !otherRuns.has(user.id)) - .map((user) => ({ sourceId: user.external_id as string, clerkId: user.id })); -} From 796d7f6a8d9b1bb547b92d7367d299f7724da29c Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:23 -0400 Subject: [PATCH 03/34] docs(changeset): add an empty changeset; migrate stays hidden and refuses every call Co-Authored-By: Claude Opus 5.5 --- .changeset/migrate-slice-1c.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/migrate-slice-1c.md diff --git a/.changeset/migrate-slice-1c.md b/.changeset/migrate-slice-1c.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/migrate-slice-1c.md @@ -0,0 +1,2 @@ +--- +--- From b5d0db6e466ab92b3a967fab300fe748f81b1fe1 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 21:11:40 -0400 Subject: [PATCH 04/34] fix(migrate): retry a rate-limited read of the instance's settings Under load, a 429 on /v1/domains or the Frontend API made the settings read return null, so the checks ran without the instance's rules and passed users the instance then refused one create at a time. Each of the three requests now backs off like every other call, and retryOn429 retries a FAPI 429 as well as a BAPI one. Co-Authored-By: Claude Opus 5.5 --- .../commands/migrate/lib/clerk-config.test.ts | 24 +++++++++++++++++++ .../src/commands/migrate/lib/clerk-config.ts | 11 ++++++--- .../src/commands/migrate/lib/retry.test.ts | 18 +++++++++++++- .../src/commands/migrate/lib/retry.ts | 8 +++---- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts index efceabe1d..dd85884ac 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts @@ -75,6 +75,30 @@ describe("fetchInstanceSettings", () => { expect(urls.some((url) => url.includes("/v1/dev_browser"))).toBe(false); }); + // A missing settings read lets the checks pass users the instance refuses. + test("retries a rate-limited domains lookup", async () => { + route([{ is_satellite: false, frontend_api_url: "https://clerk.example.com" }]); + const routed = mockFetch.getMockImplementation()!; + let domainCalls = 0; + mockFetch.mockImplementation((input: string | URL) => { + if (String(input).includes("/v1/domains") && ++domainCalls === 1) { + return Promise.resolve( + new Response( + JSON.stringify({ errors: [{ code: "too_many_requests", message: "slow" }] }), + { + status: 429, + headers: { "Content-Type": "application/json", "Retry-After": "0.01" }, + }, + ), + ); + } + return routed(input); + }); + + expect(await fetchInstanceSettings("sk_test_abc")).toEqual(USER_SETTINGS); + expect(domainCalls).toBe(2); + }); + // `null` means "unknown", so callers degrade rather than treating a failed // lookup as "nothing is enabled". test("returns null when no domain names a Frontend API URL", async () => { diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts index 6e05eaf10..9ecec6b28 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts @@ -16,6 +16,7 @@ import { } from "../../../lib/fapi.ts"; import { log } from "../../../lib/log.ts"; import { detectInstanceType } from "./instance.ts"; +import { retryOn429 } from "./retry.ts"; /** * Supabase provider keys whose Clerk strategy is not simply `oauth_`. @@ -130,7 +131,9 @@ async function fetchFapiHost(secretKey: string): Promise { */ export async function fetchInstanceSettings(secretKey: string): Promise { try { - const fapiHost = await fetchFapiHost(secretKey); + // A 429 is retried: under load, a missing settings read would let the + // checks pass users the instance then refuses one create at a time. + const fapiHost = await retryOn429(async () => fetchFapiHost(secretKey)); if (!fapiHost) { log.debug("migrate: no domain on this instance named a Frontend API URL"); return null; @@ -138,8 +141,10 @@ export async function fetchInstanceSettings(secretKey: string): Promise bootstrapDevBrowser(fapiHost)) + : undefined; + return await retryOn429(async () => fetchUserSettings(fapiHost, jwt ? { jwt } : {})); } catch (error) { log.debug( `migrate: could not read instance settings: ${ diff --git a/packages/cli-core/src/commands/migrate/lib/retry.test.ts b/packages/cli-core/src/commands/migrate/lib/retry.test.ts index 47c3041d8..4b5def750 100644 --- a/packages/cli-core/src/commands/migrate/lib/retry.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/retry.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { BapiError, CliError } from "../../../lib/errors.ts"; +import { BapiError, CliError, FapiError } from "../../../lib/errors.ts"; import { RateLimitExceededError, readRetryAfter, retryOn429 } from "./retry.ts"; const rateLimited = (headers: Record = {}) => @@ -71,6 +71,22 @@ describe("retryOn429", () => { expect(attempts).toBe(2); }); + // The settings read goes through FAPI, which is rate limited the same way. + test("retries a FAPI 429 as well", async () => { + let attempts = 0; + const result = await retryOn429( + async () => { + attempts++; + if (attempts === 1) throw new FapiError(429, JSON.stringify({ errors: [] })); + return "ok"; + }, + { defaultDelayMs: 5 }, + ); + + expect(result).toBe("ok"); + expect(attempts).toBe(2); + }); + test("waits the interval the server asked for", async () => { let attempts = 0; const started = performance.now(); diff --git a/packages/cli-core/src/commands/migrate/lib/retry.ts b/packages/cli-core/src/commands/migrate/lib/retry.ts index 5f0f99324..8d18c78f6 100644 --- a/packages/cli-core/src/commands/migrate/lib/retry.ts +++ b/packages/cli-core/src/commands/migrate/lib/retry.ts @@ -7,12 +7,12 @@ */ import { setTimeout as delay } from "node:timers/promises"; -import { BapiError } from "../../../lib/errors.ts"; +import { ApiError } from "../../../lib/errors.ts"; import { interruptSignal } from "../../../lib/signals.ts"; import { MAX_RETRIES, RETRY_DELAY_MS, getRetryDelay } from "./instance.ts"; /** Seconds to wait per a 429's `Retry-After` header or error meta, if given. */ -export function readRetryAfter(error: BapiError): number | undefined { +export function readRetryAfter(error: ApiError): number | undefined { const header = error.headers?.get("retry-after"); if (header) { const parsed = Number(header); @@ -44,7 +44,7 @@ export type RetryOptions = { }; /** - * Runs `fn`, backing off and retrying whenever BAPI answers 429. + * Runs `fn`, backing off and retrying whenever BAPI or FAPI answers 429. * * Anything other than a 429 propagates untouched — only rate limiting is * transient. Exhausting the retries raises {@link RateLimitExceededError} so @@ -58,7 +58,7 @@ export async function retryOn429(fn: () => Promise, options: RetryOptions try { return await fn(); } catch (error) { - if (!(error instanceof BapiError) || error.status !== 429) throw error; + if (!(error instanceof ApiError) || error.status !== 429) throw error; if (attempt >= maxRetries) throw new RateLimitExceededError(maxRetries); const { delayMs, delaySeconds } = getRetryDelay(readRetryAfter(error), defaultDelayMs); From 32c31aad01ca42477e85a95ca8f2eae87875d9b0 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 14:54:26 -0400 Subject: [PATCH 05/34] fix(migrate): keep a Supabase user on a disabled provider who can sign in by code A user whose only social providers are off in Clerk was rejected as unable to sign in, even with a verified email or phone the instance signs in with by code or link. One such user stopped the import. The reject now applies only when there is no other way in, and both checks read the instance's first factors through one helper. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 48 +++++++++++++++++++ .../src/commands/migrate/lib/checks.ts | 32 ++++++++++--- 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 8649a8799..f8091271c 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -291,6 +291,54 @@ describe("rejects", () => { ).toEqual({ "only-discord": "only signs in with Discord, which is not enabled in Clerk" }); }); + // A disabled provider strands only a user with no other way in. + test("a supabase user on a disabled provider who can sign in by email or phone code", async () => { + const rows = [ + { id: "email", raw_app_meta_data: { providers: ["discord"] } }, + { id: "phone", raw_app_meta_data: { providers: ["discord"] } }, + { id: "neither", raw_app_meta_data: { providers: ["discord"] } }, + ]; + expect( + await reasonsOf({ + settings: settings({ + email_address: { + enabled: true, + used_for_first_factor: true, + first_factors: ["email_code"], + }, + phone_number: { + enabled: true, + used_for_first_factor: true, + first_factors: ["phone_code"], + }, + username: { enabled: true }, + }), + supabaseRows: rows, + users: [ + user("email"), + user("phone", { email: undefined, phone: "+15555550100" }), + user("neither", { email: undefined, username: "neither" }), + ], + }), + ).toEqual({ neither: "only signs in with Discord, which is not enabled in Clerk" }); + }); + + test("an unverified email is no way in for a supabase user on a disabled provider", async () => { + expect( + await reasonsOf({ + settings: settings({ + email_address: { + enabled: true, + used_for_first_factor: true, + first_factors: ["email_link"], + }, + }), + supabaseRows: [{ id: "u", raw_app_meta_data: { providers: ["discord"] } }], + users: [user("u", { email: undefined, unverifiedEmailAddresses: ["u@x.dev"] })], + }), + ).toEqual({ u: "only signs in with Discord, which is not enabled in Clerk" }); + }); + // Clerk can't turn on a provider it doesn't offer, so none is suggested. test("a provider Clerk doesn't offer is named as such, with no fix", async () => { const rows = [ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 1c2df0bbd..947b7211a 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -236,12 +236,7 @@ const NOT_ALTERNATIVE_SIGN_IN = new Set([ * created. Mirrors `create_service.go` and `UserSettings.FirstFactors()`. */ export function passwordIsOnlySignIn(settings: UserSettingsJSON): boolean { - const strategies = new Set(); - for (const attribute of Object.values(settings.attributes ?? {})) { - if (attribute?.used_for_first_factor) { - for (const strategy of attribute.first_factors ?? []) strategies.add(strategy); - } - } + const strategies = firstFactorStrategies(settings); for (const [strategy, social] of Object.entries(settings.social ?? {})) { if (social?.enabled && social.authenticatable) strategies.add(strategy); } @@ -252,6 +247,17 @@ export function passwordIsOnlySignIn(settings: UserSettingsJSON): boolean { ); } +/** The first-factor strategies the instance's identifiers offer (`email_code`, `password`, …). */ +function firstFactorStrategies(settings: UserSettingsJSON): Set { + const strategies = new Set(); + for (const attribute of Object.values(settings.attributes ?? {})) { + if (attribute?.used_for_first_factor) { + for (const strategy of attribute.first_factors ?? []) strategies.add(strategy); + } + } + return strategies; +} + /** True when the instance needs legal acceptance this user has no record of. */ function lacksLegalAcceptance(user: User, settings: UserSettingsJSON | null): boolean { return ( @@ -571,9 +577,23 @@ function findDisabledProviderRejects(input: CheckInput): Map { if (disabled.length === 0) return reasons; const { excludedIds } = findUsersWithOnlyDisabledProviders(input.supabaseRows, disabled); + // A disabled provider only strands a user with no other way in: a verified + // email or phone the instance signs in with by code or link still works. + const strategies = firstFactorStrategies(input.settings); + const usersById = new Map(input.users.map((user) => [user.userId, user])); + const canSignInOtherwise = (id: string) => { + const user = usersById.get(id); + if (!user) return false; + const { primaryEmail, primaryPhone } = splitIdentifiers(user); + return ( + (Boolean(primaryEmail) && (strategies.has("email_code") || strategies.has("email_link"))) || + (Boolean(primaryPhone) && strategies.has("phone_code")) + ); + }; // Each reject names only that user's own providers. const rowsById = new Map(input.supabaseRows.map((row) => [String(row.id), row])); for (const id of excludedIds) { + if (canSignInOtherwise(id)) continue; const own = getUserProviders(rowsById.get(id) ?? {}).filter((p) => disabled.includes(p)); // Clerk can't turn on a provider it doesn't offer, so say which is which. const why = (provider: string) => From 4e84266d258059ab232daa0c9130150ac0c7dcfc Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 14:54:49 -0400 Subject: [PATCH 06/34] fix(migrate): retry a rate-limited read of the instance's user count The settings reads were retried on 429 but the count was not, so a 429 checked a dev instance's quota as if it were empty. Co-Authored-By: Claude Opus 5.5 --- .../commands/migrate/lib/clerk-config.test.ts | 37 ++++++++++++++++++- .../src/commands/migrate/lib/clerk-config.ts | 6 ++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts index dd85884ac..5fe3842bc 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts @@ -1,7 +1,7 @@ import { test, expect, describe, mock, beforeEach, afterAll } from "bun:test"; import { stubFetch, useCaptureLog } from "../../../test/lib/stubs.ts"; import type { UserSettingsJSON } from "../../../lib/fapi.ts"; -import { fetchInstanceSettings } from "./clerk-config.ts"; +import { fetchInstanceSettings, fetchUserCount } from "./clerk-config.ts"; const USER_SETTINGS = { attributes: { email_address: { enabled: true, required: true } }, @@ -111,3 +111,38 @@ describe("fetchInstanceSettings", () => { expect(await fetchInstanceSettings("sk_test_abc")).toBeNull(); }); }); + +describe("fetchUserCount", () => { + const originalFetch = globalThis.fetch; + useCaptureLog(); + const mockFetch = mock(); + + beforeEach(() => { + mockFetch.mockReset(); + stubFetch(mockFetch); + }); + afterAll(() => { + globalThis.fetch = originalFetch; + }); + + // A missing count checks a dev instance's quota as if it were empty. + test("retries a rate-limited count", async () => { + let calls = 0; + mockFetch.mockImplementation(() => + Promise.resolve( + ++calls === 1 + ? new Response( + JSON.stringify({ errors: [{ code: "too_many_requests", message: "slow" }] }), + { + status: 429, + headers: { "Content-Type": "application/json", "Retry-After": "0.01" }, + }, + ) + : json({ object: "total_count", total_count: 42 }), + ), + ); + + expect(await fetchUserCount("sk_test_abc")).toBe(42); + expect(calls).toBe(2); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts index 9ecec6b28..4897ae548 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts @@ -184,7 +184,11 @@ export async function fetchEnabledSocialProviders(secretKey: string): Promise { try { - const response = await bapiRequest({ method: "GET", path: "/v1/users/count", secretKey }); + // A 429 is retried, as the settings read is: a missing count checks a dev + // instance's quota as if it were empty. + const response = await retryOn429(async () => + bapiRequest({ method: "GET", path: "/v1/users/count", secretKey }), + ); const total = (response.body as { total_count?: unknown })?.total_count; return typeof total === "number" ? total : null; } catch (error) { From 6c67c6f496ad7fd08420659a3e625885b2666ca0 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 14:55:19 -0400 Subject: [PATCH 07/34] fix(migrate): name the app in a suggested config patch, or leave it to fill in With a key from --secret-key the target has no app ID, so the printed `clerk config patch` had no --app and went to the linked profile's app: it failed outside a linked directory, and patched the wrong app in one linked elsewhere. It now carries a placeholder --app . Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/checks.test.ts | 9 ++++++--- packages/cli-core/src/commands/migrate/lib/checks.ts | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index f8091271c..3715dbbe6 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -656,15 +656,18 @@ describe("fixes", () => { }); // Without --instance, `clerk config patch` changes the linked profile's - // development instance, not the one a --secret-key import targets. - test("name the instance even when the key came from --secret-key", async () => { + // development instance, not the one a --secret-key import targets; without + // --app, the linked profile's app. A key names neither app nor profile. + test("name the instance, and leave the app to fill in, when the key came from --secret-key", async () => { const checks = await checkImport( input({ settings: EMAIL_REQUIRED, users: [user("b", { email: undefined, username: "b" })], }), ); - expect(checks.fixes[0]?.command).toStartWith("clerk config patch --instance ins_1 --json"); + expect(checks.fixes[0]?.command).toStartWith( + "clerk config patch --app --instance ins_1 --json", + ); }); test("point at the Dashboard when the instance could not be named", async () => { diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 947b7211a..d4b43037a 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -713,10 +713,12 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { // Always name the instance: without it, `clerk config patch` acts on the // linked profile's development instance, whatever key this import used. A // `key_` ID is a stand-in for an instance Clerk didn't name, so there is - // nothing to pass; point at the Dashboard instead. + // nothing to pass; point at the Dashboard instead. A key from --secret-key + // names no app, and without --app the patch goes to the linked one, so the + // command carries a placeholder the user fills in. const { appId, instanceId } = input.target; const named = instanceId.startsWith("ins_"); - const flags = `${appId ? ` --app ${appId}` : ""} --instance ${instanceId}`; + const flags = ` --app ${appId ?? ""} --instance ${instanceId}`; const settings = input.settings; const mfa = MFA_SETTINGS.filter( ({ field, attribute }) => From 49c98bdcdbe0b26a58d7037298302a018d5232d2 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 14:55:30 -0400 Subject: [PATCH 08/34] chore(migrate): drop two leftover doc comments Each sat directly above the block that documents the same declaration. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.ts | 1 - packages/cli-core/src/commands/migrate/lib/readiness.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index d4b43037a..64c1475a3 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -52,7 +52,6 @@ export type Reject = { export type ReasonCount = { reason: string; count: number }; -/** A `clerk config patch` that would stop a setting costing users. */ /** * A setting change that would stop users being flagged: a `clerk config patch` * command, or a Dashboard link when the instance can't be named for one. diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts index bd445405d..f5cdf56d5 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -80,7 +80,6 @@ type BuildInput = { * carry leaves nothing to create them with, so the API refuses them — which is * why these are the only attributes whose consequence is `rejects`. */ -/** Attributes `POST /v1/users` refuses a user without, when they are required. */ const REJECTING_ATTRIBUTES = new Set([ "email_address", "phone_number", From f9dfd0287a9800da86f475cabadcc4ae1a92c86a Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:05:22 -0400 Subject: [PATCH 09/34] test(migrate): give the checks' scheduler stub a pause The scheduler gained `pause` for a run-wide 429 wait, so the pass-through stub in the checks tests needs one to type-check. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 3715dbbe6..6d411ae13 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -58,7 +58,7 @@ function input(overrides: Partial = {}): CheckInput { keySource: "--secret-key", }, secretKey: "sk_live_x", - schedule: async (fn) => fn(), + schedule: Object.assign(async (fn: () => Promise) => fn(), { pause: () => {} }), ...overrides, }; } From 1fa8532a22913d11b53ed01d3a77c997d01e6ad6 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:26:23 -0400 Subject: [PATCH 10/34] fix(migrate): reject a user without a password when nothing else signs in The check required `password` among the instance's first factors, which Clerk never lists, so it never fired: on an email + password instance with no code or social sign-in, a user without a password passed the checks and the create then failed on `skip_password_requirement`. It now mirrors `create_service.go`: `FirstFactors()` minus the strategies that are no way in on their own (passkey, ticket, the reset codes). The fixtures use Clerk's real shape, and the warnings no longer say users reset their password: Clerk doesn't count a reset as a way in. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 37 +++++++++++++------ .../src/commands/migrate/lib/checks.ts | 20 +++++----- .../src/commands/migrate/lib/readiness.ts | 3 +- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 6d411ae13..76b17c3f9 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -5,9 +5,17 @@ import { checkImport, hashShapeProblem, passwordIsOnlySignIn, type CheckInput } const BCRYPT = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy"; -const settings = (attributes: object, social: object = {}) => +/** Settings as given, with no way to sign in but what they list. */ +const bare = (attributes: object, social: object = {}) => ({ attributes, social }) as unknown as UserSettingsJSON; +/** + * Settings with a way to sign in besides a password (SSO), so users without + * one import: Clerk refuses them on an instance with no other way in. + */ +const settings = (attributes: object, social: object = {}) => + ({ ...bare(attributes, social), enterprise_sso: { enabled: true } }) as UserSettingsJSON; + const EMAIL_REQUIRED = settings({ email_address: { enabled: true, required: true } }); const user = (userId: string, fields: Partial = {}): User => @@ -106,9 +114,10 @@ describe("rejects", () => { }); test("no password where password is the only way to sign in", async () => { - const passwordOnly = settings({ + const passwordOnly = bare({ email_address: { enabled: true, used_for_first_factor: false, first_factors: [] }, - password: { enabled: true, used_for_first_factor: true, first_factors: ["password"] }, + // Clerk's real shape: a password is never itself a listed first factor. + password: { enabled: true, used_for_first_factor: false, first_factors: [] }, }); const reasons = await reasonsOf({ users: [user("a"), user("b", { password: BCRYPT, passwordHasher: "bcrypt" })], @@ -520,6 +529,7 @@ describe("rejects", () => { ({ attributes: { email_address: { enabled: true }, username: { enabled: true } }, social: {}, + enterprise_sso: { enabled: true }, username_settings: { min_length: 4, max_length: 64, ...rules }, }) as unknown as UserSettingsJSON; const reasonFor = async (username: string, rules: object = {}) => @@ -698,7 +708,7 @@ describe("fixes", () => { describe("passwordIsOnlySignIn", () => { const withFactors = (factors: Record, social: object = {}) => - settings( + bare( Object.fromEntries( Object.entries(factors).map(([name, first_factors]) => [ name, @@ -708,23 +718,26 @@ describe("passwordIsOnlySignIn", () => { social, ); + // Clerk's shape: `password` lists no first factors of its own. test.each([ - ["password alone", { password: ["password"] }, {}, true], - // Mirrors clerk_go: a passkey is not counted as a way in without a password. - ["password and passkey", { password: ["password"], passkey: ["passkey"] }, {}, true], + ["password alone", { password: [] }, {}, true], + ["password, and an email used only for sign-up", { password: [], email_address: [] }, {}, true], + // Mirrors clerk_go: neither a passkey nor a reset is a way in on its own. + ["password and passkey", { password: [], passkey: ["passkey"] }, {}, true], [ - "password and email codes", - { password: ["password"], email_address: ["email_code"] }, + "password and a reset code", + { password: [], email_address: ["reset_password_email_code"] }, {}, - false, + true, ], + ["password and email codes", { password: [], email_address: ["email_code"] }, {}, false], [ "password and Google", - { password: ["password"] }, + { password: [] }, { oauth_google: { enabled: true, authenticatable: true } }, false, ], - ["no password factor", { email_address: ["email_code"] }, {}, false], + ["email links without passwords", { email_address: ["email_link"] }, {}, false], ])("%s -> %p", (_label, factors, social, expected) => { expect(passwordIsOnlySignIn(withFactors(factors, social))).toBe(expected); }); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 64c1475a3..f043c6b88 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -222,7 +222,6 @@ function mfaProblem(user: User, settings: UserSettingsJSON | null): string | und /** Sign-in strategies that don't count as a way in without a password, per clerk_go. */ const NOT_ALTERNATIVE_SIGN_IN = new Set([ - "password", "passkey", "ticket", "reset_password_email_code", @@ -230,9 +229,15 @@ const NOT_ALTERNATIVE_SIGN_IN = new Set([ ]); /** - * True when the instance has no sign-in strategy but a password. Clerk then + * True when the instance offers no way to sign in but a password. Clerk then * refuses `skip_password_requirement`, so a user without a digest can't be - * created. Mirrors `create_service.go` and `UserSettings.FirstFactors()`. + * created. Mirrors `create_service.go`: `FirstFactors()` minus the strategies + * that are no way in on their own. Clerk never lists `password` itself as a + * first factor, and a password reset doesn't count. + * + * ponytail: Google One Tap counts for Clerk when Google uses custom + * credentials, which FAPI doesn't serve; a Google set up for One Tap alone + * reads as no way in, so those users are rejected rather than created. */ export function passwordIsOnlySignIn(settings: UserSettingsJSON): boolean { const strategies = firstFactorStrategies(settings); @@ -240,10 +245,7 @@ export function passwordIsOnlySignIn(settings: UserSettingsJSON): boolean { if (social?.enabled && social.authenticatable) strategies.add(strategy); } if (settings.enterprise_sso?.enabled) strategies.add("enterprise_sso"); - return ( - strategies.has("password") && - [...strategies].every((strategy) => NOT_ALTERNATIVE_SIGN_IN.has(strategy)) - ); + return [...strategies].every((strategy) => NOT_ALTERNATIVE_SIGN_IN.has(strategy)); } /** The first-factor strategies the instance's identifiers offer (`email_code`, `password`, …). */ @@ -631,7 +633,7 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { const missing = importable.length - item.userCount; warnings.push( item.key === "password" - ? `${plural(missing, "user")} without a password, which this instance requires: they reset it to sign in` + ? `${plural(missing, "user")} without a password, which this instance requires: they sign in another way, such as a code or a social account` : `${plural(missing, "user")} without a ${item.label.toLowerCase()}, which this instance requires`, ); } else if (item.key === "password") { @@ -653,7 +655,7 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { const dropped = importable.filter((user) => user.passwordDropped).length; if (dropped > 0) { warnings.push( - `${plural(dropped, "password")} Clerk cannot verify will be dropped: those users reset it to sign in`, + `${plural(dropped, "password")} Clerk cannot verify will be dropped: those users sign in another way, such as a code or a social account`, ); } diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts index f5cdf56d5..4618fa535 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -47,7 +47,8 @@ export type ReadinessItem = { * - `drops` — the user is created, but this piece of them is not. A required * password is in this group rather than `rejects` because the import sends * `skip_password_requirement` (see `import-users.ts`), so the user lands - * without one and has to reset it before they can sign in that way. + * without one and signs in another way. With no other way, the checks + * reject the user (`passwordIsOnlySignIn`). */ consequence?: "rejects" | "drops"; /** Why it blocks — omitted when it does not. */ From 0e10defca44934bc935ca357217da6e3bab34373 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:27:53 -0400 Subject: [PATCH 11/34] fix(migrate): keep the identifiers Clerk keeps, by Clerk's rule The checks read "off" as the sign-up toggle. Clerk accepts an email or phone that is enabled, used for sign-in, or used for MFA (`IsEnabledOrFactor` in `create_service.go`), and never checks whether usernames are on. So an instance with SMS MFA and phone sign-up off had its phones stripped, phone-only users rejected, and a suggestion to turn phone sign-up on. The strip, the no-identifier reject and the readiness rows now use Clerk's rule. A username is kept with usernames off and noted as stored, like a password; one Clerk would refuse is dropped there with a warning, since nothing signs in with it. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 38 +++++++++- .../src/commands/migrate/lib/checks.ts | 71 ++++++++++++++----- .../src/commands/migrate/lib/readiness.ts | 31 ++++++-- 3 files changed, 116 insertions(+), 24 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 76b17c3f9..f8982e2cf 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -558,11 +558,28 @@ describe("rejects", () => { expect(await reasonFor(username)).toContain(reason); }); - test("a username is not checked when usernames are off", async () => { + // Clerk checks a username with usernames off too, but nothing signs in + // with it there, so it costs the user nothing to lose it. + test("with usernames off, one Clerk would refuse is dropped, not rejected", async () => { const checks = await checkImport( input({ settings: EMAIL_REQUIRED, users: [user("a", { username: "a.b" })] }), ); expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a")]); + expect(checks.warnings).toContain( + "1 user has a username Clerk refuses, which is dropped: this instance has usernames off", + ); + }); + + // Clerk never checks whether usernames are on (create_service.go). + test("with usernames off, a valid one is kept and said to be stored", async () => { + const checks = await checkImport( + input({ settings: EMAIL_REQUIRED, users: [user("a", { username: "ada" })] }), + ); + expect(checks.importable).toEqual([user("a", { username: "ada" })]); + expect(checks.warnings).toContain( + "1 user has a username, which this instance does not use: it is stored, and works only once usernames are turned on", + ); }); }); @@ -614,6 +631,25 @@ describe("warnings", () => { expect(checks.importable).toEqual([user("a")]); }); + // Clerk keeps an email or phone the instance signs in or does MFA with, + // whatever the sign-up setting (`IsEnabledOrFactor` in clerk_go). + test.each([ + ["sign-in", { enabled: false, used_for_first_factor: true, first_factors: ["phone_code"] }], + ["MFA", { enabled: false, used_for_second_factor: true, second_factors: ["phone_code"] }], + ])("keeps a phone used only for %s, with no warning or fix", async (_label, phone_number) => { + const phoneOnly = user("p", { email: undefined, phone: "+15555550100" }); + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, phone_number }), + users: [user("a", { phone: "+15555550101" }), phoneOnly], + }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a", { phone: "+15555550101" }), phoneOnly]); + expect(checks.warnings).toEqual([]); + expect(checks.fixes).toEqual([]); + }); + test("says a password is kept, not dropped, when passwords are off", async () => { const checks = await checkImport( input({ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index f043c6b88..18a21ea48 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -31,7 +31,7 @@ import { } from "./clerk-config.ts"; import { resolveDevUserLimit } from "./instance.ts"; import { buildChangePayload, buildSettingChanges } from "./modify-settings.ts"; -import { buildReadinessReport } from "./readiness.ts"; +import { acceptsIdentifier, buildReadinessReport } from "./readiness.ts"; import type { ApiScheduler } from "./scheduler.ts"; import { countSocialProviders, @@ -282,12 +282,11 @@ const USERNAME_EXTENDED = /^[a-zA-Z0-9!#$'+.^_`~-]+$/; /** * Clerk's username rules, mirrored from `validate.Username` in clerk_go, so a * username the instance would refuse is a reject here rather than a failed - * create. Skipped when usernames are off: the readiness warnings cover that. + * create. Clerk checks them with usernames off too. */ function usernameProblem(user: User, settings: UserSettingsJSON | null): string | undefined { const username = user.username; if (!settings || typeof username !== "string" || !username) return undefined; - if (!isEnabled(settings, "username")) return undefined; const rules = (settings as { username_settings?: UsernameSettings }).username_settings ?? {}; const length = [...username].length; @@ -628,6 +627,10 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { providerCounts: countSocialProviders(rowsFor(input, importable)), }); for (const item of report.blocking) { + if (item.consequence === "stored") { + warnings.push(storedWarning(item.key, item.userCount)); + continue; + } if (item.consequence !== "drops") continue; if (item.clerkRequired === true) { const missing = importable.length - item.userCount; @@ -636,12 +639,6 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { ? `${plural(missing, "user")} without a password, which this instance requires: they sign in another way, such as a code or a social account` : `${plural(missing, "user")} without a ${item.label.toLowerCase()}, which this instance requires`, ); - } else if (item.key === "password") { - // Clerk stores a digest even with passwords off, so nothing is lost: - // it starts working if passwords are turned on. - warnings.push( - `${plural(item.userCount, "user")} ${item.userCount === 1 ? "has" : "have"} a password, which this instance does not use: it is stored, and works only once passwords are turned on`, - ); } else { warnings.push( item.section === "social" @@ -672,6 +669,15 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { return warnings; } +/** + * A field Clerk stores with its setting off, so nothing is lost: it starts + * working once the setting is turned on. + */ +function storedWarning(key: string, count: number): string { + const noun = key === "password" ? "password" : "username"; + return `${plural(count, "user")} ${count === 1 ? "has" : "have"} a ${noun}, which this instance does not use: it is stored, and works only once ${noun}s are turned on`; +} + /** Shell-quotes a JSON payload for a single-quoted argument. */ function quoteJson(payload: unknown): string { return `'${JSON.stringify(payload).replace(/'/g, "'\\''")}'`; @@ -745,24 +751,22 @@ function countReasons(rejects: Reject[]): ReasonCount[] { } /** - * Removes the emails, phones or usernames of an instance that has that - * identifier off. + * Removes the emails or phones Clerk would refuse: those of an instance that + * neither has the identifier on nor signs in or does MFA with it. * - * The warnings already say they are dropped, but Clerk does not drop them. It - * refuses the whole create for a phone (`phone_number is not a valid - * parameter`), and for a username it stores it anyway, or refuses the create - * when the username breaks the default rules. + * The warnings already say they are dropped, but Clerk does not drop them: it + * refuses the whole create (`phone_number is not a valid parameter`). A + * username is not among them: Clerk stores it with usernames off. */ function dropDisabledIdentifiers(user: User, settings: UserSettingsJSON | null): User { if (!settings) return user; const fields = [ - ...(isEnabled(settings, "email_address") + ...(acceptsIdentifier(settings, "email_address") ? [] : (["email", "emailAddresses", "unverifiedEmailAddresses"] as const)), - ...(isEnabled(settings, "phone_number") + ...(acceptsIdentifier(settings, "phone_number") ? [] : (["phone", "phoneNumbers", "unverifiedPhoneNumbers"] as const)), - ...(isEnabled(settings, "username") ? [] : (["username"] as const)), ]; if (!fields.some((field) => field in user)) return user; const kept = { ...user }; @@ -770,6 +774,29 @@ function dropDisabledIdentifiers(user: User, settings: UserSettingsJSON | null): return kept; } +/** + * The user without a username Clerk would refuse, where usernames are off: + * Clerk still checks it there, and nothing signs in with it, so it is dropped + * rather than costing the user. + */ +function dropRefusedOffUsername( + user: User, + settings: UserSettingsJSON | null, +): { user: User; dropped: boolean } { + if (!settings || isEnabled(settings, "username") || !usernameProblem(user, settings)) { + return { user, dropped: false }; + } + const { username: _dropped, ...kept } = user; + return { user: kept as User, dropped: true }; +} + +function refusedUsernameWarning(count: number): string[] { + if (count === 0) return []; + return [ + `${plural(count, "user")} ${count === 1 ? "has" : "have"} a username Clerk refuses, which is dropped: this instance has usernames off`, + ]; +} + function refusedNameWarning(count: number): string[] { if (count === 0) return []; return [ @@ -804,10 +831,13 @@ export async function checkImport(input: CheckInput): Promise { const passed: User[] = []; const placeholderEmails = new Set(); const refusedNames = new Set(); + const refusedUsernames = new Set(); for (const original of input.users) { const named = dropRefusedNames(original); if (named.dropped) refusedNames.add(original.userId); - const { user, refused } = dropRefusedEmails(named.user); + const usernamed = dropRefusedOffUsername(named.user, input.settings); + if (usernamed.dropped) refusedUsernames.add(original.userId); + const { user, refused } = dropRefusedEmails(usernamed.user); const reason = original.skipReason ?? (refused.length > 0 && !hasAnyIdentifier(user) @@ -894,6 +924,9 @@ export async function checkImport(input: CheckInput): Promise { ...buildWarnings(input, candidates), ...placeholderWarning(candidates.filter((user) => placeholderEmails.has(user.userId)).length), ...refusedNameWarning(candidates.filter((user) => refusedNames.has(user.userId)).length), + ...refusedUsernameWarning( + candidates.filter((user) => refusedUsernames.has(user.userId)).length, + ), ...legalWarning( candidates.filter((user) => lacksLegalAcceptance(user, input.settings)).length, ), diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts index 4618fa535..486c399eb 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -49,8 +49,11 @@ export type ReadinessItem = { * `skip_password_requirement` (see `import-users.ts`), so the user lands * without one and signs in another way. With no other way, the checks * reject the user (`passwordIsOnlySignIn`). + * - `stored` — the user is created with it, but the instance does not use + * it until the setting is turned on. Clerk stores these whatever the + * setting (`create_service.go` validates them but never checks it). */ - consequence?: "rejects" | "drops"; + consequence?: "rejects" | "drops" | "stored"; /** Why it blocks — omitted when it does not. */ detail?: string; }; @@ -89,6 +92,22 @@ const REJECTING_ATTRIBUTES = new Set([ "last_name", ]); +/** Fields Clerk stores with their setting off, where it works once turned on. */ +const STORED_WHEN_OFF = new Set(["password", "username"]); + +/** + * True when Clerk keeps an email or phone on create: its setting is on, or + * the instance signs in or does MFA with it (`IsEnabledOrFactor` in clerk_go). + * Sign-up off alone does not refuse it. + */ +export function acceptsIdentifier( + settings: UserSettingsJSON, + attribute: "email_address" | "phone_number", +): boolean { + const data = settings.attributes?.[attribute]; + return Boolean(data?.enabled || data?.used_for_first_factor || data?.used_for_second_factor); +} + /** An identifier or user-model row, with its blocking verdict. */ function buildAttributeItem( label: string, @@ -100,6 +119,10 @@ function buildAttributeItem( ): ReadinessItem { const enabled = settings ? isEnabled(settings, attribute) : null; const required = settings ? isRequired(settings, attribute) : null; + const accepted = + settings && (attribute === "email_address" || attribute === "phone_number") + ? acceptsIdentifier(settings, attribute) + : enabled; const missing = totalUsers - userCount; // Required but not universal is the expensive case: those users fail one by @@ -122,8 +145,8 @@ function buildAttributeItem( }; } - // Present in the file but switched off in Clerk: the data is silently dropped. - if (enabled === false && userCount > 0) { + // Present in the file but switched off in Clerk: dropped, or stored unused. + if (accepted === false && userCount > 0) { return { label, key: attribute, @@ -132,7 +155,7 @@ function buildAttributeItem( clerkEnabled: enabled, clerkRequired: required, blocking: true, - consequence: "drops", + consequence: STORED_WHEN_OFF.has(attribute) ? "stored" : "drops", detail: "not enabled in Clerk", }; } From 1796eb604f78bf2b5658aa0631c0513a6c071641 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:28:53 -0400 Subject: [PATCH 12/34] fix(migrate): check duplicates only on the identifiers the create sends The duplicate checks ran before the strip, so with email off two users sharing an email got one rejected though the email never went out. The file check also matched extra emails and phones, which are attached after the create and fail only as a note, as the instance check already allows. The strip now runs per user, before the duplicate checks, and the file check matches only the primary email and phone. The warnings still describe the users as the file has them. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 40 +++++++++++++++++++ .../src/commands/migrate/lib/checks.ts | 37 ++++++++++------- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index f8982e2cf..601141cb6 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -193,6 +193,46 @@ describe("rejects", () => { }); }); + // Only what the create carries can clash: an extra email is attached after + // it, and a clash there is a note on the user, not a failure. + test("an extra email or phone shared with an earlier user is no duplicate", async () => { + expect( + await reasonsOf({ + users: [ + user("a", { + emailAddresses: ["shared@x.dev"], + phone: ["+15555550101", "+15555550100"], + }), + user("b", { + emailAddresses: ["shared@x.dev"], + phone: ["+15555550102", "+15555550100"], + }), + ], + }), + ).toEqual({}); + }); + + // A stripped email never goes out, so it cannot clash with anything. + test("an email the instance would strip is no duplicate", async () => { + existing = [{ id: "user_1", email_addresses: [{ email_address: "taken@x.dev" }] }]; + const users = [ + user("a", { email: "same@x.dev", username: "a" }), + user("b", { email: "same@x.dev", username: "b" }), + user("c", { email: "taken@x.dev", username: "c" }), + ]; + const checks = await checkImport( + input({ users, settings: settings({ username: { enabled: true } }) }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable.map((u) => [u.userId, u.email])).toEqual([ + ["a", undefined], + ["b", undefined], + ["c", undefined], + ]); + // The warning still describes the file: the emails are dropped. + expect(checks.warnings.join("\n")).toContain("3 users have a email"); + }); + // The source's order decides which duplicate survives, so the dry run says. test("a duplicate names the earlier user kept in its place", async () => { const checks = await checkImport( diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 18a21ea48..ab00979db 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -430,6 +430,10 @@ const hasAnyIdentifier = (user: User) => * Keyed by record, not source ID: two records with one source ID must not * share a verdict, or the one kept would be rejected along with its copy. * + * Only what `POST /v1/users` carries is matched, as in + * {@link findInstanceDuplicates}: an extra email is attached after the + * create, and a clash there fails only as a note on the user. + * * @returns Each duplicate's reason, and the earlier user kept in its place. */ function findFileDuplicates(users: User[]): { @@ -450,13 +454,9 @@ function findFileDuplicates(users: User[]): { } seenIds.add(user.userId); - const identifiers = splitIdentifiers(user); - const ownEmails = [identifiers.primaryEmail, ...identifiers.additionalEmails].filter( - (value): value is string => Boolean(value), - ); - const ownPhones = [identifiers.primaryPhone, ...identifiers.additionalPhones].filter( - (value): value is string => Boolean(value), - ); + const { primaryEmail, primaryPhone } = splitIdentifiers(user); + const ownEmails = primaryEmail ? [primaryEmail] : []; + const ownPhones = primaryPhone ? [primaryPhone] : []; const emailOwner = ownEmails.map((email) => emails.get(email.toLowerCase())).find(Boolean); const phoneOwner = ownPhones.map((phone) => phones.get(phoneKey(phone))).find(Boolean); @@ -827,8 +827,11 @@ export async function checkImport(input: CheckInput): Promise { const disabledProviders = findDisabledProviderRejects(input); // Users that pass every per-user check. Only these claim identifiers in - // the file: a rejected record must not cost a later one its email. + // the file: a rejected record must not cost a later one its email. Each is + // stripped of what Clerk would refuse, so the duplicate checks see only + // what is sent; `unstripped` keeps what the warnings describe. const passed: User[] = []; + const unstripped = new Map(); const placeholderEmails = new Set(); const refusedNames = new Set(); const refusedUsernames = new Set(); @@ -838,6 +841,7 @@ export async function checkImport(input: CheckInput): Promise { const usernamed = dropRefusedOffUsername(named.user, input.settings); if (usernamed.dropped) refusedUsernames.add(original.userId); const { user, refused } = dropRefusedEmails(usernamed.user); + const sent = dropDisabledIdentifiers(user, input.settings); const reason = original.skipReason ?? (refused.length > 0 && !hasAnyIdentifier(user) @@ -846,7 +850,7 @@ export async function checkImport(input: CheckInput): Promise { missingRequiredIdentifier(user, input.settings) ?? // Stripping the identifiers the instance has off can leave nothing to // sign in with; Clerk would still create the user. - (!hasAnyIdentifier(dropDisabledIdentifiers(user, input.settings)) + (!hasAnyIdentifier(sent) ? "has no identifier this instance accepts (its email, phone or username is turned off)" : undefined) ?? missingRequiredName(user, input.settings) ?? @@ -865,7 +869,8 @@ export async function checkImport(input: CheckInput): Promise { if (reason) { rejects.push({ sourceId: user.userId, reason }); } else { - passed.push(user); + passed.push(sent); + unstripped.set(sent, user); if (refused.length > 0) placeholderEmails.add(user.userId); } } @@ -914,14 +919,16 @@ export async function checkImport(input: CheckInput): Promise { return { total: input.users.length + input.failures.length, - importable: candidates.map((user) => { - const kept = dropDisabledIdentifiers(user, input.settings); - return lacksLegalAcceptance(kept, input.settings) ? { ...kept, skipLegalChecks: true } : kept; - }), + importable: candidates.map((user) => + lacksLegalAcceptance(user, input.settings) ? { ...user, skipLegalChecks: true } : user, + ), rejects, rejectReasons: countReasons(rejects), warnings: [ - ...buildWarnings(input, candidates), + ...buildWarnings( + input, + candidates.map((user) => unstripped.get(user) ?? user), + ), ...placeholderWarning(candidates.filter((user) => placeholderEmails.has(user.userId)).length), ...refusedNameWarning(candidates.filter((user) => refusedNames.has(user.userId)).length), ...refusedUsernameWarning( From 0a0e661fcaae9fbce105aec685a76204ac565bff Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:29:24 -0400 Subject: [PATCH 13/34] fix(migrate): say a name is stored, not dropped, when names are off With first or last name turned off, the checks warned the name was dropped. Clerk stores it anyway: `create_service.go` validates it but never checks the setting. Names now get the note passwords and usernames get: stored, and shown once the setting is on. The fix to turn the setting on is still offered. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 15 +++++++++++++++ .../cli-core/src/commands/migrate/lib/checks.ts | 14 +++++++++++--- .../src/commands/migrate/lib/readiness.ts | 2 +- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 601141cb6..262b94508 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -690,6 +690,21 @@ describe("warnings", () => { expect(checks.fixes).toEqual([]); }); + // Clerk validates a name but never checks its setting (create_service.go). + test("says a name is kept, not dropped, when names are off", async () => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, first_name: { enabled: false } }), + users: [user("a", { firstName: "Ada" })], + }), + ); + expect(checks.importable).toEqual([user("a", { firstName: "Ada" })]); + expect(checks.warnings).toEqual([ + "1 user has a first name, which this instance does not use: it is stored, and shows only once first names are turned on", + ]); + expect(checks.fixes.map((fix) => fix.label)).toEqual(["Enable First name"]); + }); + test("says a password is kept, not dropped, when passwords are off", async () => { const checks = await checkImport( input({ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index ab00979db..277934fda 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -669,13 +669,21 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { return warnings; } +/** Each field Clerk stores with its setting off, and what it does once turned on. */ +const STORED_FIELDS: Record = { + password: { noun: "password", then: "works" }, + username: { noun: "username", then: "works" }, + first_name: { noun: "first name", then: "shows" }, + last_name: { noun: "last name", then: "shows" }, +}; + /** * A field Clerk stores with its setting off, so nothing is lost: it starts - * working once the setting is turned on. + * working, or showing, once the setting is turned on. */ function storedWarning(key: string, count: number): string { - const noun = key === "password" ? "password" : "username"; - return `${plural(count, "user")} ${count === 1 ? "has" : "have"} a ${noun}, which this instance does not use: it is stored, and works only once ${noun}s are turned on`; + const { noun, then } = STORED_FIELDS[key] ?? { noun: key, then: "works" }; + return `${plural(count, "user")} ${count === 1 ? "has" : "have"} a ${noun}, which this instance does not use: it is stored, and ${then} only once ${noun}s are turned on`; } /** Shell-quotes a JSON payload for a single-quoted argument. */ diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts index 486c399eb..22b54aff7 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -93,7 +93,7 @@ const REJECTING_ATTRIBUTES = new Set([ ]); /** Fields Clerk stores with their setting off, where it works once turned on. */ -const STORED_WHEN_OFF = new Set(["password", "username"]); +const STORED_WHEN_OFF = new Set(["password", "username", "first_name", "last_name"]); /** * True when Clerk keeps an email or phone on create: its setting is on, or From 1ed0909b7b7233e33f346010ceb971d8c9ca3deb Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:30:06 -0400 Subject: [PATCH 14/34] fix(migrate): drop a blank or over-long name, as Clerk refuses both Clerk refuses a first or last name that is only whitespace or unprintable characters, or longer than 256 bytes (`first_name.go`). Those passed the checks and failed one at a time mid-import. They are now dropped with the existing warning, which names both. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 7 ++++++- .../src/commands/migrate/lib/checks.ts | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 262b94508..59a842545 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -540,12 +540,15 @@ describe("rejects", () => { ["https://spam.example"], ["see x.com/win"], ["Ada"], + [" "], + ["\u200b"], + ["é".repeat(129)], ])("%p is dropped, and the user still imports", async (firstName) => { const checks = await checkImport(input({ users: [user("a", { firstName, lastName: "L" })] })); expect(checks.rejects).toEqual([]); expect(checks.importable).toEqual([user("a", { lastName: "L" })]); expect(checks.warnings).toContain( - "1 user has a name Clerk refuses (a phone number, URL or HTML), which is dropped", + "1 user has a name Clerk refuses (a phone number, a URL, HTML, blank, or over 256 bytes), which is dropped", ); }); @@ -558,6 +561,8 @@ describe("rejects", () => { ["redacted.io"], ["ada@x.dev"], ["ada@x.dev/x"], + // 256 bytes exactly. + ["é".repeat(128)], ])("%p is kept", async (firstName) => { const checks = await checkImport(input({ users: [user("a", { firstName })] })); expect(checks.importable).toEqual([user("a", { firstName })]); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 277934fda..ac5e865b4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -377,13 +377,21 @@ function dropRefusedEmails(user: User): { user: User; refused: string[] } { return { user: kept ?? user, refused }; } +/** Clerk's cap on a first or last name, in bytes (`firstNameMaxLen` in clerk_go). */ +const NAME_MAX_BYTES = 256; + /** - * A name Clerk refuses, approximating clerk_go's `NameForAbusePreventionLoose`: - * a phone number (10–15 digits, or fewer behind a `+`/`00`), a URL with a - * scheme or path that isn't part of an email, or an HTML tag. Better Auth's phone sign-up stores the - * number as the name, so this is common, not exotic. + * A name Clerk refuses: blank (only whitespace or unprintable characters), + * over {@link NAME_MAX_BYTES}, or what clerk_go's `NameForAbusePreventionLoose` + * refuses, approximated: a phone number (10–15 digits, or fewer behind a + * `+`/`00`), a URL with a scheme or path that isn't part of an email, or an + * HTML tag. Better Auth's phone sign-up stores the number as the name, so this + * is common, not exotic. */ function nameProblem(name: string): string | undefined { + // Go's `unicode.IsPrint` refuses control and format characters (\p{C}). + if (/^[\s\p{C}]+$/u.test(name)) return "blank"; + if (Buffer.byteLength(name) > NAME_MAX_BYTES) return "too long"; for (const candidate of name.match(/(?:\+|00)?\d[\d\s().-]{5,}\d/g) ?? []) { const digits = candidate.replace(/\D/g, "").length; const international = /^(\+|00)/.test(candidate.trim()); @@ -808,7 +816,7 @@ function refusedUsernameWarning(count: number): string[] { function refusedNameWarning(count: number): string[] { if (count === 0) return []; return [ - `${plural(count, "user")} ${count === 1 ? "has" : "have"} a name Clerk refuses (a phone number, URL or HTML), which is dropped`, + `${plural(count, "user")} ${count === 1 ? "has" : "have"} a name Clerk refuses (a phone number, a URL, HTML, blank, or over ${NAME_MAX_BYTES} bytes), which is dropped`, ]; } From d2b85a30a8113f6407c234f47a9a3c58622e746d Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:30:37 -0400 Subject: [PATCH 15/34] fix(migrate): put the right article on each field in the warnings The warnings built "a" plus the field's label, so they read "1 user has a email" and "has a phone". Each field now carries its own noun: "an email", "a phone number". Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 6 ++++-- .../cli-core/src/commands/migrate/lib/checks.ts | 13 +++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 59a842545..ecdf079a7 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -230,7 +230,9 @@ describe("rejects", () => { ["c", undefined], ]); // The warning still describes the file: the emails are dropped. - expect(checks.warnings.join("\n")).toContain("3 users have a email"); + expect(checks.warnings.join("\n")).toContain( + "3 users have an email, which this instance is not set up to store", + ); }); // The source's order decides which duplicate survives, so the dry run says. @@ -653,7 +655,7 @@ describe("warnings", () => { }), ); expect(checks.warnings).toContain( - "1 user has a phone, which this instance is not set up to store", + "1 user has a phone number, which this instance is not set up to store", ); expect(checks.rejects).toEqual([]); }); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index ac5e865b4..4b5247a06 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -625,6 +625,15 @@ function rowsFor(input: CheckInput, users: User[]): Record[] { return input.supabaseRows.filter((row) => ids.has(String(row.id))); } +/** Each readiness row's field, with its article: "an email", not "a email". */ +const FIELD_NOUNS: Record = { + email_address: "an email", + phone_number: "a phone number", + username: "a username", + first_name: "a first name", + last_name: "a last name", +}; + function buildWarnings(input: CheckInput, importable: User[]): string[] { const warnings: string[] = []; @@ -645,13 +654,13 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { warnings.push( item.key === "password" ? `${plural(missing, "user")} without a password, which this instance requires: they sign in another way, such as a code or a social account` - : `${plural(missing, "user")} without a ${item.label.toLowerCase()}, which this instance requires`, + : `${plural(missing, "user")} without ${FIELD_NOUNS[item.key] ?? item.label.toLowerCase()}, which this instance requires`, ); } else { warnings.push( item.section === "social" ? `${plural(item.userCount, "user")} signed in with ${item.label}, which ${clerkOffersProvider(item.key) ? "is not enabled in Clerk" : "Clerk doesn't offer"}` - : `${plural(item.userCount, "user")} ${item.userCount === 1 ? "has" : "have"} a ${item.label.toLowerCase()}, which this instance is not set up to store`, + : `${plural(item.userCount, "user")} ${item.userCount === 1 ? "has" : "have"} ${FIELD_NOUNS[item.key] ?? item.label.toLowerCase()}, which this instance is not set up to store`, ); } } From b3bf4d41f0de01cc87a318efbf24596c444bd173 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:31:09 -0400 Subject: [PATCH 16/34] fix(migrate): use APP_ID as the fix command's app placeholder Pasted as is, `--app ` makes the shell read a redirect and fail with "no such file or directory: app_id". `APP_ID` goes through, and Clerk answers "app not found", which says what to fill in. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.test.ts | 2 +- packages/cli-core/src/commands/migrate/lib/checks.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index ecdf079a7..24351679c 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -774,7 +774,7 @@ describe("fixes", () => { }), ); expect(checks.fixes[0]?.command).toStartWith( - "clerk config patch --app --instance ins_1 --json", + "clerk config patch --app APP_ID --instance ins_1 --json", ); }); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 4b5247a06..2cdd52292 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -747,10 +747,12 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { // `key_` ID is a stand-in for an instance Clerk didn't name, so there is // nothing to pass; point at the Dashboard instead. A key from --secret-key // names no app, and without --app the patch goes to the linked one, so the - // command carries a placeholder the user fills in. + // command carries a placeholder the user fills in: `APP_ID`, which a shell + // passes as is and Clerk answers "app not found", where `` would + // be read as a redirect. const { appId, instanceId } = input.target; const named = instanceId.startsWith("ins_"); - const flags = ` --app ${appId ?? ""} --instance ${instanceId}`; + const flags = ` --app ${appId ?? "APP_ID"} --instance ${instanceId}`; const settings = input.settings; const mfa = MFA_SETTINGS.filter( ({ field, attribute }) => From 27fcc3a94572187818b4a676b0e6db950a7d2800 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:31:48 -0400 Subject: [PATCH 17/34] fix(migrate): offer the config patch a username reject asks for A username reject said to turn on numeric usernames or extended special characters but gave no way to. The fixes now include a `clerk config patch` for `auth_username.allow_numeric_usernames` or `allow_extended_special_characters`, as the MFA rejects already do. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 23 ++++++++++++++++ .../src/commands/migrate/lib/checks.ts | 27 +++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 24351679c..1ebf7b6c8 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -797,6 +797,29 @@ describe("fixes", () => { }); }); + // The rejects name the option; the fix gives the command for it. + test.each([ + ["12345", "allow_numeric_usernames", "Allow numeric usernames"], + [ + "ada.l", + "allow_extended_special_characters", + "Allow extended special characters in usernames", + ], + ])("offer the username option %p needs", async (username, rule, label) => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, username: { enabled: true } }), + users: [user("a", { username }), user("b", { username: "ada" })], + }), + ); + expect(checks.fixes).toEqual([ + { + label, + command: `clerk config patch --app APP_ID --instance ins_1 --json '{"auth_username":{"${rule}":true}}'`, + }, + ]); + }); + test("offer nothing when the settings could not be read", async () => { const checks = await checkImport(input({ users: [user("a")] })); expect(checks.fixes).toEqual([]); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 2cdd52292..618703ea2 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -279,6 +279,21 @@ type UsernameSettings = { const USERNAME_DEFAULT = /^[a-zA-Z0-9_-]+$/; const USERNAME_EXTENDED = /^[a-zA-Z0-9!#$'+.^_`~-]+$/; +/** The username options a reject asks for, and the config leaf that turns each on. */ +const USERNAME_OPTIONS = [ + { + rule: "allow_numeric_usernames", + label: "Allow numeric usernames", + needs: (username: string) => !/[a-zA-Z]/.test(username), + }, + { + rule: "allow_extended_special_characters", + label: "Allow extended special characters in usernames", + needs: (username: string) => + !USERNAME_DEFAULT.test(username) && USERNAME_EXTENDED.test(username), + }, +] as const; + /** * Clerk's username rules, mirrored from `validate.Username` in clerk_go, so a * username the instance would refuse is a reject here rather than a failed @@ -758,8 +773,16 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { ({ field, attribute }) => !isEnabled(settings, attribute) && users.some((user) => hasValue(user[field])), ).map(({ label, path }) => ({ label, writes: [{ path: [...path], value: true }] })); - - return [...buildSettingChanges(flagged), ...mfa].map((change) => + // Usernames off are not checked by these rejects; see `dropRefusedOffUsername`. + const rules = (settings as { username_settings?: UsernameSettings }).username_settings ?? {}; + const usernames = isEnabled(settings, "username") + ? users.flatMap((user) => (typeof user.username === "string" ? [user.username] : [])) + : []; + const username = USERNAME_OPTIONS.filter( + ({ rule, needs }) => !rules[rule] && usernames.some(needs), + ).map(({ rule, label }) => ({ label, writes: [{ path: ["auth_username", rule], value: true }] })); + + return [...buildSettingChanges(flagged), ...mfa, ...username].map((change) => named ? { label: change.label, From afc1b0c79ba511bfe99406f6a8c926e80c8acc1f Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 00:54:13 -0400 Subject: [PATCH 18/34] fix(migrate): pause every user lookup on a 429, not just the one that hit it The import's creates and attaches pause the whole scheduler on a 429, but the batched `GET /v1/users` lookups backed off one at a time, so under a sustained limit each queued lookup hit it in turn. A lookup's backoff now pauses the scheduler for the Retry-After wait too. Co-Authored-By: Claude Opus 5.5 --- .../commands/migrate/lib/user-lookup.test.ts | 39 +++++++++++++++++++ .../src/commands/migrate/lib/user-lookup.ts | 19 +++++---- 2 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts new file mode 100644 index 000000000..d7c4aed0a --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts @@ -0,0 +1,39 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { createApiScheduler } from "./scheduler.ts"; +import { LOOKUP_BATCH, lookupUsers } from "./user-lookup.ts"; + +let originalFetch: typeof globalThis.fetch; + +beforeEach(() => { + originalFetch = globalThis.fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +// The instance is over its limit for every lookup, not just the one that hit it. +test("holds every other lookup through a 429's wait", async () => { + const started = performance.now(); + const sentAt: number[] = []; + globalThis.fetch = (async () => { + sentAt.push(performance.now() - started); + return sentAt.length === 1 + ? new Response(JSON.stringify({ errors: [{ code: "x", message: "slow down" }] }), { + status: 429, + headers: { "retry-after": "1" }, + }) + : Response.json([]); + }) as unknown as typeof fetch; + + await lookupUsers({ + filter: "external_id", + // Two batches, so a second lookup is waiting when the first hits the limit. + values: Array.from({ length: LOOKUP_BATCH + 1 }, (_, index) => `u${index}`), + secretKey: "sk_test_x", + schedule: createApiScheduler(1, 10_000), + }); + + expect(sentAt).toHaveLength(3); + for (const at of sentAt.slice(1)) expect(at).toBeGreaterThanOrEqual(900); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index 1c8990d1c..cb87b163e 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -61,14 +61,17 @@ export async function lookupUsers(options: { params.append(options.filter, options.filter === "external_id" ? `+${value}` : value); } - const response = await retryOn429(async () => - options.schedule(async () => - bapiRequest({ - method: "GET", - path: `/v1/users?${params.toString()}`, - secretKey: options.secretKey, - }), - ), + // A 429 pauses every lookup still queued, as on import. + const response = await retryOn429( + async () => + options.schedule(async () => + bapiRequest({ + method: "GET", + path: `/v1/users?${params.toString()}`, + secretKey: options.secretKey, + }), + ), + { onRetry: ({ delaySeconds }) => options.schedule.pause(delaySeconds * 1000) }, ); done++; options.spinner?.update(`${options.label ?? "Looking up users"}: ${done}/${batches.length}`); From f9f6ddbc570b7584e2ba6cf733080a9102ef4fc1 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 13:26:38 -0400 Subject: [PATCH 19/34] fix(migrate): warn that a ban due to end stays until it is lifted in Clerk A source's temporary ban carries to Clerk as `banned`, and Clerk's ban has no end, so the user stays banned after the source's ban would have ended, with nothing saying so. The checks now warn for users whose source recorded an end date (`banEndsAt`), naming the latest one. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 16 ++++++++++++++++ .../cli-core/src/commands/migrate/lib/checks.ts | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 1ebf7b6c8..35cf9580f 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -724,6 +724,22 @@ describe("warnings", () => { ); }); + // Clerk's ban has no end, so a source's temporary ban would outlast itself. + test("a ban that was due to end", async () => { + const checks = await checkImport( + input({ + users: [ + user("a", { banned: true, banEndsAt: "2026-11-01T00:00:00.000Z" }), + user("b", { banned: true, banEndsAt: "2026-12-01T00:00:00.000Z" }), + user("c", { banned: true }), + ], + }), + ); + expect(checks.warnings).toContain( + "2 users have bans that end by 2026-12-01; Clerk's ban has no end, so they stay banned until unbanned in Clerk", + ); + }); + test("fields Clerk has no place for", async () => { const checks = await checkImport( input({ users: [user("a")], unknownFields: { department: 3, role: 1 } }), diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 618703ea2..692210723 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -854,6 +854,19 @@ function refusedNameWarning(count: number): string[] { ]; } +/** + * Users whose ban was due to end. Clerk's ban has no end, so they stay banned + * until someone lifts it in Clerk; the latest end date says how long to watch. + */ +function temporaryBanWarning(users: User[]): string[] { + const ends = users.flatMap((user) => (user.banned && user.banEndsAt ? [user.banEndsAt] : [])); + if (ends.length === 0) return []; + const latest = ends.sort().at(-1)?.slice(0, 10); + return [ + `${plural(ends.length, "user")} ${ends.length === 1 ? "has a ban that ends" : "have bans that end"} by ${latest}; Clerk's ban has no end, so ${ends.length === 1 ? "it stays" : "they stay"} banned until unbanned in Clerk`, + ]; +} + function legalWarning(count: number): string[] { if (count === 0) return []; return [ @@ -984,6 +997,7 @@ export async function checkImport(input: CheckInput): Promise { ...refusedUsernameWarning( candidates.filter((user) => refusedUsernames.has(user.userId)).length, ), + ...temporaryBanWarning(candidates), ...legalWarning( candidates.filter((user) => lacksLegalAcceptance(user, input.settings)).length, ), From 58ea9c809d13c7be5e88b1d0b66594c1e33a00a5 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 15:38:12 -0400 Subject: [PATCH 20/34] fix(migrate): reject a bcrypt digest under cost 4 in the checks The checks took any bcrypt cost up to 15. clerk_go stores a lower one, but Go's bcrypt refuses a cost under 4 when it compares, so that user's password could never sign in. The checks now require 4 to 15, as the Supabase and Better Auth hasher detection does. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.test.ts | 4 +++- packages/cli-core/src/commands/migrate/lib/checks.ts | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 35cf9580f..42e84f065 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -278,7 +278,7 @@ describe("rejects", () => { ], }), ).toEqual({ - bad: "password is not a bcrypt hash Clerk accepts ($2a$/$2b$/$2y$, cost up to 15, 60 characters)", + bad: "password is not a bcrypt hash Clerk can verify ($2a$/$2b$/$2y$, cost 4 to 15, 60 characters)", }); }); @@ -894,6 +894,8 @@ describe("hashShapeProblem", () => { test.each([ ["$2a$10$short", "bcrypt"], + // Stored by Clerk, but Go's bcrypt refuses it at sign-in. + ["$2a$03$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy", "bcrypt"], ["hash$salt$signer$sep$eight$14", "scrypt_firebase"], ["hash$salt", "scrypt_firebase"], ["argon2id$...", "argon2id"], diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 692210723..2f01012a4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -120,10 +120,11 @@ const plural = (count: number, word: string) => `${count} ${word}${count === 1 ? export function hashShapeProblem(password: string, hasher: string): string | undefined { switch (hasher) { case "bcrypt": - // clerk_go caps the cost at 15 (pkg/hash/bcrypt.go). - return /^\$2[aby]\$(0\d|1[0-5])\$[./A-Za-z0-9]{53}$/.test(password) + // clerk_go caps the cost at 15 (pkg/hash/bcrypt.go), and Go's bcrypt + // refuses any cost under 4 at sign-in, so that password could never work. + return /^\$2[aby]\$(0[4-9]|1[0-5])\$[./A-Za-z0-9]{53}$/.test(password) ? undefined - : "password is not a bcrypt hash Clerk accepts ($2a$/$2b$/$2y$, cost up to 15, 60 characters)"; + : "password is not a bcrypt hash Clerk can verify ($2a$/$2b$/$2y$, cost 4 to 15, 60 characters)"; case "scrypt_firebase": { const parts = password.split("$"); const numeric = (value: string | undefined) => /^\d+$/.test(value ?? ""); From d894514226e53082f72ed3aa82fbf91fd16e11f2 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 16:05:13 -0400 Subject: [PATCH 21/34] refactor(migrate): name the stored-field fix rows' verb so oxlint's no-thenable stops firing Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/checks.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 2f01012a4..771c967c0 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -703,11 +703,11 @@ function buildWarnings(input: CheckInput, importable: User[]): string[] { } /** Each field Clerk stores with its setting off, and what it does once turned on. */ -const STORED_FIELDS: Record = { - password: { noun: "password", then: "works" }, - username: { noun: "username", then: "works" }, - first_name: { noun: "first name", then: "shows" }, - last_name: { noun: "last name", then: "shows" }, +const STORED_FIELDS: Record = { + password: { noun: "password", verb: "works" }, + username: { noun: "username", verb: "works" }, + first_name: { noun: "first name", verb: "shows" }, + last_name: { noun: "last name", verb: "shows" }, }; /** @@ -715,8 +715,8 @@ const STORED_FIELDS: Record = { * working, or showing, once the setting is turned on. */ function storedWarning(key: string, count: number): string { - const { noun, then } = STORED_FIELDS[key] ?? { noun: key, then: "works" }; - return `${plural(count, "user")} ${count === 1 ? "has" : "have"} a ${noun}, which this instance does not use: it is stored, and ${then} only once ${noun}s are turned on`; + const { noun, verb } = STORED_FIELDS[key] ?? { noun: key, verb: "works" }; + return `${plural(count, "user")} ${count === 1 ? "has" : "have"} a ${noun}, which this instance does not use: it is stored, and ${verb} only once ${noun}s are turned on`; } /** Shell-quotes a JSON payload for a single-quoted argument. */ From 63fa798b77aaddc7b388f22ecf562c581e7cd05d Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 16:05:14 -0400 Subject: [PATCH 22/34] fix(migrate): say a user with no identifier may have a username Clerk refuses Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.test.ts | 2 +- packages/cli-core/src/commands/migrate/lib/checks.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 42e84f065..124d1e4df 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -136,7 +136,7 @@ describe("rejects", () => { }); expect(reasons).toEqual({ "phone-only": - "has no identifier this instance accepts (its email, phone or username is turned off)", + "has no identifier this instance accepts (its email or phone is turned off, or its username is one Clerk refuses)", }); }); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 771c967c0..4a3765e72 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -915,7 +915,7 @@ export async function checkImport(input: CheckInput): Promise { // Stripping the identifiers the instance has off can leave nothing to // sign in with; Clerk would still create the user. (!hasAnyIdentifier(sent) - ? "has no identifier this instance accepts (its email, phone or username is turned off)" + ? "has no identifier this instance accepts (its email or phone is turned off, or its username is one Clerk refuses)" : undefined) ?? missingRequiredName(user, input.settings) ?? mfaProblem(user, input.settings) ?? From cf197b6f59f5cba60f9b3532c1bf34801d81ce81 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 16:05:14 -0400 Subject: [PATCH 23/34] fix(migrate): say numeric usernames make Clerk require E.164 phones, and skip that fix for a file without them Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 18 +++++++++++++++++- .../src/commands/migrate/lib/checks.ts | 15 +++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 124d1e4df..e4bcfdc4c 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -815,7 +815,11 @@ describe("fixes", () => { // The rejects name the option; the fix gives the command for it. test.each([ - ["12345", "allow_numeric_usernames", "Allow numeric usernames"], + [ + "12345", + "allow_numeric_usernames", + "Allow numeric usernames (Clerk then requires phone numbers in E.164 form, on import and at sign-up)", + ], [ "ada.l", "allow_extended_special_characters", @@ -836,6 +840,18 @@ describe("fixes", () => { ]); }); + // With numeric usernames on, Clerk refuses a phone not in E.164 form, so + // the fix would cost those users their phone. + test("offer no numeric-username fix to a file with a phone not in E.164 form", async () => { + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, username: { enabled: true } }), + users: [user("a", { username: "12345" }), user("b", { phone: "(415) 555-0100" })], + }), + ); + expect(checks.fixes.map((fix) => fix.label).join("\n")).not.toContain("numeric usernames"); + }); + test("offer nothing when the settings could not be read", async () => { const checks = await checkImport(input({ users: [user("a")] })); expect(checks.fixes).toEqual([]); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 4a3765e72..33ceb7359 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -284,7 +284,9 @@ const USERNAME_EXTENDED = /^[a-zA-Z0-9!#$'+.^_`~-]+$/; const USERNAME_OPTIONS = [ { rule: "allow_numeric_usernames", - label: "Allow numeric usernames", + // Clerk ties strict E.164 phones to this setting (create_service.go). + label: + "Allow numeric usernames (Clerk then requires phone numbers in E.164 form, on import and at sign-up)", needs: (username: string) => !/[a-zA-Z]/.test(username), }, { @@ -779,8 +781,17 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { const usernames = isEnabled(settings, "username") ? users.flatMap((user) => (typeof user.username === "string" ? [user.username] : [])) : []; + // Numeric usernames make Clerk refuse any phone not in E.164 form, so the + // fix is not offered to a file whose phones would then be refused. + const looseFormat = users.some((user) => { + const { primaryPhone, additionalPhones, unverifiedPhones } = splitIdentifiers(user); + return [primaryPhone, ...additionalPhones, ...unverifiedPhones].some( + (phone) => phone !== undefined && !/^\+[1-9]\d{1,14}$/.test(phone), + ); + }); const username = USERNAME_OPTIONS.filter( - ({ rule, needs }) => !rules[rule] && usernames.some(needs), + ({ rule, needs }) => + !rules[rule] && usernames.some(needs) && !(rule === "allow_numeric_usernames" && looseFormat), ).map(({ rule, label }) => ({ label, writes: [{ path: ["auth_username", rule], value: true }] })); return [...buildSettingChanges(flagged), ...mfa, ...username].map((change) => From 3c5e92d1e919524417cac9ade79c92d56cc5a5b2 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 18:12:28 -0400 Subject: [PATCH 24/34] fix(migrate): drop phones not in E.164 form where numeric usernames are on, and say what turning it on would cost Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 49 +++++++++++ .../src/commands/migrate/lib/checks.ts | 81 ++++++++++++++++--- 2 files changed, 120 insertions(+), 10 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index e4bcfdc4c..ecf5d9cd4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -605,6 +605,55 @@ describe("rejects", () => { expect(await reasonFor(username)).toContain(reason); }); + // With numeric usernames on, Clerk refuses any phone not in E.164 form. + describe("phones, with numeric usernames on", () => { + const numeric = (on: boolean) => + ({ + attributes: { + email_address: { enabled: true }, + phone_number: { enabled: true }, + username: { enabled: true }, + }, + social: {}, + enterprise_sso: { enabled: true }, + username_settings: { min_length: 4, max_length: 64, allow_numeric_usernames: on }, + }) as unknown as UserSettingsJSON; + + test("one not in E.164 form is dropped, and the user kept on their email", async () => { + const checks = await checkImport( + input({ settings: numeric(true), users: [user("a", { phone: "(415) 555-0100" })] }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a")]); + expect(checks.warnings).toContain( + "1 user has a phone not in E.164 form, which is dropped: with numeric usernames on, Clerk requires E.164", + ); + }); + + test("a user with only such phones is rejected", async () => { + expect( + await reasonsOf({ + settings: numeric(true), + users: [user("a", { email: undefined, phone: "415 555 0100" })], + }), + ).toEqual({ + a: "only has phones not in E.164 form, which this instance refuses (numeric usernames are on)", + }); + }); + + // The fix is withheld for these phones, so the reject says why. + test("a numeric username's reject names the phones turning it on would cost", async () => { + expect( + await reasonsOf({ + settings: numeric(false), + users: [user("a", { username: "12345" }), user("b", { phone: "(415) 555-0100" })], + }), + ).toEqual({ + a: "username has no letters; turn on numeric usernames to allow it, though Clerk would then refuse the file's 1 phone not in E.164 form", + }); + }); + }); + // Clerk checks a username with usernames off too, but nothing signs in // with it there, so it costs the user nothing to lose it. test("with usernames off, one Clerk would refuse is dropped, not rejected", async () => { diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 33ceb7359..7896342b4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -280,6 +280,23 @@ type UsernameSettings = { const USERNAME_DEFAULT = /^[a-zA-Z0-9_-]+$/; const USERNAME_EXTENDED = /^[a-zA-Z0-9!#$'+.^_`~-]+$/; +const PHONE_FIELDS = ["phone", "phoneNumbers", "unverifiedPhoneNumbers"] as const; + +/** + * Numeric usernames make Clerk require phones in E.164 form, on import and at + * sign-up (`requireE164` in clerk_go's create_service.go). + */ +const isE164 = (phone: string) => /^\+[1-9]\d{1,14}$/.test(phone); + +/** The file's phones Clerk would refuse once numeric usernames are on. */ +function loosePhoneCount(users: User[]): number { + return users.reduce((count, user) => { + const { primaryPhone, additionalPhones, unverifiedPhones } = splitIdentifiers(user); + const phones = [primaryPhone, ...additionalPhones, ...unverifiedPhones]; + return count + phones.filter((phone) => phone !== undefined && !isE164(phone)).length; + }, 0); +} + /** The username options a reject asks for, and the config leaf that turns each on. */ const USERNAME_OPTIONS = [ { @@ -302,7 +319,11 @@ const USERNAME_OPTIONS = [ * username the instance would refuse is a reject here rather than a failed * create. Clerk checks them with usernames off too. */ -function usernameProblem(user: User, settings: UserSettingsJSON | null): string | undefined { +function usernameProblem( + user: User, + settings: UserSettingsJSON | null, + loosePhones = 0, +): string | undefined { const username = user.username; if (!settings || typeof username !== "string" || !username) return undefined; @@ -314,7 +335,11 @@ function usernameProblem(user: User, settings: UserSettingsJSON | null): string } } if (!rules.allow_numeric_usernames && !/[a-zA-Z]/.test(username)) { - return "username has no letters; turn on numeric usernames to allow it"; + const phones = + loosePhones > 0 + ? `, though Clerk would then refuse the file's ${plural(loosePhones, "phone")} not in E.164 form` + : ""; + return `username has no letters; turn on numeric usernames to allow it${phones}`; } if (rules.allow_extended_special_characters) { if (!USERNAME_EXTENDED.test(username)) return "username has characters Clerk does not allow"; @@ -395,6 +420,38 @@ function dropRefusedEmails(user: User): { user: User; refused: string[] } { return { user: kept ?? user, refused }; } +/** + * The user without the phones Clerk would refuse where numeric usernames are + * on: not in E.164 form. Dropped, as the create's retry would drop them, so + * the checks see what is sent. + */ +function dropLoosePhones( + user: User, + settings: UserSettingsJSON | null, +): { user: User; dropped: boolean } { + const rules = (settings as { username_settings?: UsernameSettings } | null)?.username_settings; + if (!rules?.allow_numeric_usernames) return { user, dropped: false }; + let kept: User | undefined; + for (const field of PHONE_FIELDS) { + const value = user[field]; + if (value === undefined) continue; + const list = Array.isArray(value) ? value : [value]; + if (list.every(isE164)) continue; + kept ??= { ...user }; + const good = list.filter(isE164); + if (good.length > 0) kept[field] = good; + else delete kept[field]; + } + return { user: kept ?? user, dropped: kept !== undefined }; +} + +function loosePhoneWarning(count: number): string[] { + if (count === 0) return []; + return [ + `${plural(count, "user")} ${count === 1 ? "has" : "have"} a phone not in E.164 form, which is dropped: with numeric usernames on, Clerk requires E.164`, + ]; +} + /** Clerk's cap on a first or last name, in bytes (`firstNameMaxLen` in clerk_go). */ const NAME_MAX_BYTES = 256; @@ -783,12 +840,7 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { : []; // Numeric usernames make Clerk refuse any phone not in E.164 form, so the // fix is not offered to a file whose phones would then be refused. - const looseFormat = users.some((user) => { - const { primaryPhone, additionalPhones, unverifiedPhones } = splitIdentifiers(user); - return [primaryPhone, ...additionalPhones, ...unverifiedPhones].some( - (phone) => phone !== undefined && !/^\+[1-9]\d{1,14}$/.test(phone), - ); - }); + const looseFormat = loosePhoneCount(users) > 0; const username = USERNAME_OPTIONS.filter( ({ rule, needs }) => !rules[rule] && usernames.some(needs) && !(rule === "allow_numeric_usernames" && looseFormat), @@ -910,18 +962,26 @@ export async function checkImport(input: CheckInput): Promise { const placeholderEmails = new Set(); const refusedNames = new Set(); const refusedUsernames = new Set(); + const loosePhoneUsers = new Set(); + const loosePhones = loosePhoneCount(input.users); for (const original of input.users) { const named = dropRefusedNames(original); if (named.dropped) refusedNames.add(original.userId); const usernamed = dropRefusedOffUsername(named.user, input.settings); if (usernamed.dropped) refusedUsernames.add(original.userId); - const { user, refused } = dropRefusedEmails(usernamed.user); + const { user: emailed, refused } = dropRefusedEmails(usernamed.user); + const phoned = dropLoosePhones(emailed, input.settings); + if (phoned.dropped) loosePhoneUsers.add(original.userId); + const user = phoned.user; const sent = dropDisabledIdentifiers(user, input.settings); const reason = original.skipReason ?? (refused.length > 0 && !hasAnyIdentifier(user) ? "only has emails Clerk refuses (malformed, or a domain that can't receive mail)" : undefined) ?? + (phoned.dropped && !hasAnyIdentifier(user) + ? "only has phones not in E.164 form, which this instance refuses (numeric usernames are on)" + : undefined) ?? missingRequiredIdentifier(user, input.settings) ?? // Stripping the identifiers the instance has off can leave nothing to // sign in with; Clerk would still create the user. @@ -936,7 +996,7 @@ export async function checkImport(input: CheckInput): Promise { (!input.skipLegalChecks && lacksLegalAcceptance(user, input.settings) ? "no legal acceptance on record, which this instance requires (--skip-legal-checks imports them without it)" : undefined) ?? - usernameProblem(user, input.settings) ?? + usernameProblem(user, input.settings, loosePhones) ?? (user.password && user.passwordHasher ? hashShapeProblem(user.password, user.passwordHasher) : undefined) ?? @@ -1009,6 +1069,7 @@ export async function checkImport(input: CheckInput): Promise { ...refusedUsernameWarning( candidates.filter((user) => refusedUsernames.has(user.userId)).length, ), + ...loosePhoneWarning(candidates.filter((user) => loosePhoneUsers.has(user.userId)).length), ...temporaryBanWarning(candidates), ...legalWarning( candidates.filter((user) => lacksLegalAcceptance(user, input.settings)).length, From 211354c70e5fe4475802c6f6bf8a3a0b29344cc4 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 12:49:16 -0400 Subject: [PATCH 25/34] fix(migrate): trim a phone before its E.164 check, as Clerk does Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/checks.test.ts | 10 ++++++++++ packages/cli-core/src/commands/migrate/lib/checks.ts | 5 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index ecf5d9cd4..075c2151a 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -630,6 +630,16 @@ describe("rejects", () => { ); }); + // Clerk trims a phone before its E.164 check (create_service.go). + test("one with spaces around it is kept", async () => { + const checks = await checkImport( + input({ settings: numeric(true), users: [user("a", { phone: " +14155550100 " })] }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a", { phone: " +14155550100 " })]); + expect(checks.warnings).toEqual([]); + }); + test("a user with only such phones is rejected", async () => { expect( await reasonsOf({ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 7896342b4..e6ebfea35 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -284,9 +284,10 @@ const PHONE_FIELDS = ["phone", "phoneNumbers", "unverifiedPhoneNumbers"] as cons /** * Numeric usernames make Clerk require phones in E.164 form, on import and at - * sign-up (`requireE164` in clerk_go's create_service.go). + * sign-up (`requireE164` in clerk_go's create_service.go). Clerk trims the + * phone before it checks. */ -const isE164 = (phone: string) => /^\+[1-9]\d{1,14}$/.test(phone); +const isE164 = (phone: string) => /^\+[1-9]\d{1,14}$/.test(phone.trim()); /** The file's phones Clerk would refuse once numeric usernames are on. */ function loosePhoneCount(users: User[]): number { From 65a2a3e91c4b4675b19b4262d67fea8855e12dff Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 12:54:00 -0400 Subject: [PATCH 26/34] fix(migrate): leave phones the instance has off out of the E.164 checks Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 43 +++++++++++++++++-- .../src/commands/migrate/lib/checks.ts | 16 ++++--- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 075c2151a..2a5919b5a 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -607,11 +607,11 @@ describe("rejects", () => { // With numeric usernames on, Clerk refuses any phone not in E.164 form. describe("phones, with numeric usernames on", () => { - const numeric = (on: boolean) => + const numeric = (on: boolean, phones = true) => ({ attributes: { email_address: { enabled: true }, - phone_number: { enabled: true }, + phone_number: { enabled: phones }, username: { enabled: true }, }, social: {}, @@ -662,6 +662,39 @@ describe("rejects", () => { a: "username has no letters; turn on numeric usernames to allow it, though Clerk would then refuse the file's 1 phone not in E.164 form", }); }); + + // Phones off are stripped before the create, so Clerk never sees them. + describe("with phones off", () => { + test("a numeric username's reject offers the fix, with no word on phones", async () => { + const checks = await checkImport( + input({ + settings: numeric(false, false), + users: [user("a", { username: "12345" }), user("b", { phone: "(415) 555-0100" })], + }), + ); + expect(checks.rejects).toEqual([ + { + sourceId: "a", + reason: "username has no letters; turn on numeric usernames to allow it", + }, + ]); + expect(checks.fixes.map((fix) => fix.label)).toContain( + "Allow numeric usernames (Clerk then requires phone numbers in E.164 form, on import and at sign-up)", + ); + }); + + test("a phone not in E.164 form is stripped, not blamed on E.164", async () => { + const checks = await checkImport( + input({ + settings: numeric(true, false), + users: [user("a", { username: "12345", phone: "(415) 555-0100" })], + }), + ); + expect(checks.rejects).toEqual([]); + expect(checks.importable).toEqual([user("a", { username: "12345" })]); + expect(checks.warnings.join("\n")).not.toContain("E.164"); + }); + }); }); // Clerk checks a username with usernames off too, but nothing signs in @@ -904,7 +937,11 @@ describe("fixes", () => { test("offer no numeric-username fix to a file with a phone not in E.164 form", async () => { const checks = await checkImport( input({ - settings: settings({ email_address: { enabled: true }, username: { enabled: true } }), + settings: settings({ + email_address: { enabled: true }, + phone_number: { enabled: true }, + username: { enabled: true }, + }), users: [user("a", { username: "12345" }), user("b", { phone: "(415) 555-0100" })], }), ); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index e6ebfea35..e39dfcaf1 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -289,8 +289,12 @@ const PHONE_FIELDS = ["phone", "phoneNumbers", "unverifiedPhoneNumbers"] as cons */ const isE164 = (phone: string) => /^\+[1-9]\d{1,14}$/.test(phone.trim()); -/** The file's phones Clerk would refuse once numeric usernames are on. */ -function loosePhoneCount(users: User[]): number { +/** + * The file's phones Clerk would refuse once numeric usernames are on. None + * where the instance has phones off: they are stripped before the create. + */ +function loosePhoneCount(users: User[], settings: UserSettingsJSON | null): number { + if (settings && !acceptsIdentifier(settings, "phone_number")) return 0; return users.reduce((count, user) => { const { primaryPhone, additionalPhones, unverifiedPhones } = splitIdentifiers(user); const phones = [primaryPhone, ...additionalPhones, ...unverifiedPhones]; @@ -431,7 +435,9 @@ function dropLoosePhones( settings: UserSettingsJSON | null, ): { user: User; dropped: boolean } { const rules = (settings as { username_settings?: UsernameSettings } | null)?.username_settings; - if (!rules?.allow_numeric_usernames) return { user, dropped: false }; + // Phones the instance has off are stripped anyway, whatever their form. + if (!rules?.allow_numeric_usernames || (settings && !acceptsIdentifier(settings, "phone_number"))) + return { user, dropped: false }; let kept: User | undefined; for (const field of PHONE_FIELDS) { const value = user[field]; @@ -841,7 +847,7 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { : []; // Numeric usernames make Clerk refuse any phone not in E.164 form, so the // fix is not offered to a file whose phones would then be refused. - const looseFormat = loosePhoneCount(users) > 0; + const looseFormat = loosePhoneCount(users, settings) > 0; const username = USERNAME_OPTIONS.filter( ({ rule, needs }) => !rules[rule] && usernames.some(needs) && !(rule === "allow_numeric_usernames" && looseFormat), @@ -964,7 +970,7 @@ export async function checkImport(input: CheckInput): Promise { const refusedNames = new Set(); const refusedUsernames = new Set(); const loosePhoneUsers = new Set(); - const loosePhones = loosePhoneCount(input.users); + const loosePhones = loosePhoneCount(input.users, input.settings); for (const original of input.users) { const named = dropRefusedNames(original); if (named.dropped) refusedNames.add(original.userId); From 9fb8a97fcaee1373e7aeef79845afd6d8fd00b0b Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 12:54:39 -0400 Subject: [PATCH 27/34] fix(migrate): name the E.164 drop when it leaves a user without a required phone or any identifier Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 28 +++++++++++++++++++ .../src/commands/migrate/lib/checks.ts | 9 ++++++ 2 files changed, 37 insertions(+) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 2a5919b5a..12685bbf1 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -651,6 +651,34 @@ describe("rejects", () => { }); }); + test("a user with only a refused email and such phones is rejected for both", async () => { + expect( + await reasonsOf({ + settings: numeric(true), + users: [user("a", { email: "15551234@phone.local", phone: "415 555 0100" })], + }), + ).toEqual({ + a: "only has emails Clerk refuses and phones not in E.164 form (numeric usernames are on)", + }); + }); + + // The file has a phone; dropping it is what leaves the requirement unmet. + test("a required phone that is dropped is blamed on E.164, not a missing phone", async () => { + const required = { + ...numeric(true), + attributes: { + email_address: { enabled: true }, + phone_number: { enabled: true, required: true }, + username: { enabled: true }, + }, + } as unknown as UserSettingsJSON; + expect( + await reasonsOf({ settings: required, users: [user("a", { phone: "415 555 0100" })] }), + ).toEqual({ + a: "only has phones not in E.164 form, and this instance requires a phone number (numeric usernames are on)", + }); + }); + // The fix is withheld for these phones, so the reject says why. test("a numeric username's reject names the phones turning it on would cost", async () => { expect( diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index e39dfcaf1..7d5a32724 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -983,12 +983,21 @@ export async function checkImport(input: CheckInput): Promise { const sent = dropDisabledIdentifiers(user, input.settings); const reason = original.skipReason ?? + (refused.length > 0 && phoned.dropped && !hasAnyIdentifier(user) + ? "only has emails Clerk refuses and phones not in E.164 form (numeric usernames are on)" + : undefined) ?? (refused.length > 0 && !hasAnyIdentifier(user) ? "only has emails Clerk refuses (malformed, or a domain that can't receive mail)" : undefined) ?? (phoned.dropped && !hasAnyIdentifier(user) ? "only has phones not in E.164 form, which this instance refuses (numeric usernames are on)" : undefined) ?? + // A required phone the file has, but only in a form Clerk refuses here. + (phoned.dropped && + missingRequiredIdentifier(user, input.settings) && + !missingRequiredIdentifier(emailed, input.settings) + ? "only has phones not in E.164 form, and this instance requires a phone number (numeric usernames are on)" + : undefined) ?? missingRequiredIdentifier(user, input.settings) ?? // Stripping the identifiers the instance has off can leave nothing to // sign in with; Clerk would still create the user. From 930305435ea4e24e3347130778645c9f5b53d70e Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 12:55:05 -0400 Subject: [PATCH 28/34] fix(migrate): count the instance lookups on one spinner total, so it only climbs Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.ts | 3 +++ .../commands/migrate/lib/user-lookup.test.ts | 21 +++++++++++++++++++ .../src/commands/migrate/lib/user-lookup.ts | 14 ++++++++++--- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 7d5a32724..c3267f40f 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -604,6 +604,8 @@ async function findInstanceDuplicates( } } + // The four lookups run at once, so they share one count. + const progress = { done: 0, total: 0 }; const lookup = async ( filter: "external_id" | "email_address" | "phone_number" | "username", values: Iterable, @@ -615,6 +617,7 @@ async function findInstanceDuplicates( schedule: input.schedule, spinner: input.spinner, label: "Checking for users already in the instance", + progress, }); const found: LookedUpUser[] = ( diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts index d7c4aed0a..6b0e12f23 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts @@ -37,3 +37,24 @@ test("holds every other lookup through a 429's wait", async () => { expect(sentAt).toHaveLength(3); for (const at of sentAt.slice(1)) expect(at).toBeGreaterThanOrEqual(900); }); + +// Lookups run at once share one count, so the spinner only climbs. +test("lookups sharing a count report one total that only climbs", async () => { + globalThis.fetch = (async () => Response.json([])) as unknown as typeof fetch; + const updates: string[] = []; + const progress = { done: 0, total: 0 }; + const lookup = async (filter: "external_id" | "email_address", count: number) => + lookupUsers({ + filter, + values: Array.from({ length: count }, (_, index) => `u${index}`), + secretKey: "sk_test_x", + schedule: createApiScheduler(4, 10_000), + spinner: { update: (message) => updates.push(message) }, + label: "Checking", + progress, + }); + + await Promise.all([lookup("external_id", LOOKUP_BATCH + 1), lookup("email_address", 1)]); + + expect(updates).toEqual(["Checking: 1/3", "Checking: 2/3", "Checking: 3/3"]); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index cb87b163e..8759d1c1f 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -47,9 +47,15 @@ export async function lookupUsers(options: { schedule: ApiScheduler; spinner?: SpinnerControls; label?: string; + /** + * A count shared by lookups run at once, so the spinner only climbs. Each + * adds its batches to `total` before its first request goes out. + */ + progress?: { done: number; total: number }; }): Promise { const batches = batch([...new Set(options.values)], LOOKUP_BATCH); - let done = 0; + const progress = options.progress ?? { done: 0, total: 0 }; + progress.total += batches.length; const pages = await Promise.all( batches.map(async (values) => { @@ -73,8 +79,10 @@ export async function lookupUsers(options: { ), { onRetry: ({ delaySeconds }) => options.schedule.pause(delaySeconds * 1000) }, ); - done++; - options.spinner?.update(`${options.label ?? "Looking up users"}: ${done}/${batches.length}`); + progress.done++; + options.spinner?.update( + `${options.label ?? "Looking up users"}: ${progress.done}/${progress.total}`, + ); const users = response.body; return Array.isArray(users) ? (users as LookedUpUser[]) : []; }), From a698253e0b6f7c84e5cab4fd58bd517e700a8a4f Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:10:48 -0400 Subject: [PATCH 29/34] fix(migrate): read every page of a lookup batch, from starting_after External IDs are not unique, so 100 of them can match more than one page of users, and a user missed that way would be created again. Co-Authored-By: Claude Opus 5.5 --- .../commands/migrate/lib/user-lookup.test.ts | 28 ++++++++++++++ .../src/commands/migrate/lib/user-lookup.ts | 37 ++++++++++++------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts index 6b0e12f23..a5ad65e4f 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts @@ -58,3 +58,31 @@ test("lookups sharing a count report one total that only climbs", async () => { expect(updates).toEqual(["Checking: 1/3", "Checking: 2/3", "Checking: 3/3"]); }); + +// External IDs are not unique: a batch can match more users than one page holds. +test("reads every page of a batch that matches more than one page", async () => { + const urls: URL[] = []; + const page = (from: number, count: number) => + Array.from({ length: count }, (_, index) => ({ + id: `user_${from + index}`, + external_id: "-dup", + })); + globalThis.fetch = (async (input: string | URL | Request) => { + urls.push(new URL(input.toString())); + return Response.json(urls.length === 1 ? page(0, LOOKUP_BATCH) : page(LOOKUP_BATCH, 5)); + }) as unknown as typeof fetch; + + const users = await lookupUsers({ + filter: "external_id", + values: ["-dup"], + secretKey: "sk_test_x", + schedule: createApiScheduler(1, 10_000), + }); + + expect(users).toHaveLength(LOOKUP_BATCH + 5); + expect(urls).toHaveLength(2); + expect(urls[0]?.searchParams.has("starting_after")).toBe(false); + expect(urls[1]?.searchParams.get("starting_after")).toBe(`user_${LOOKUP_BATCH - 1}`); + // The `+` keeps a leading `-` from reading as "exclude". + expect(urls[0]?.searchParams.getAll("external_id")).toEqual(["+-dup"]); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index 8759d1c1f..85fabd07e 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -3,6 +3,8 @@ * * BAPI filters on up to 100 values per call and leaves out any it does not * find, so the work is proportional to the list rather than to the instance. + * External IDs are not unique, so a batch can match more than a page: a full + * page is followed by the next, from `starting_after`. * Every call goes through the run's scheduler and backs off on a 429, exactly * as the writes do. */ @@ -67,24 +69,31 @@ export async function lookupUsers(options: { params.append(options.filter, options.filter === "external_id" ? `+${value}` : value); } - // A 429 pauses every lookup still queued, as on import. - const response = await retryOn429( - async () => - options.schedule(async () => - bapiRequest({ - method: "GET", - path: `/v1/users?${params.toString()}`, - secretKey: options.secretKey, - }), - ), - { onRetry: ({ delaySeconds }) => options.schedule.pause(delaySeconds * 1000) }, - ); + const found: LookedUpUser[] = []; + for (;;) { + // A 429 pauses every lookup still queued, as on import. + const response = await retryOn429( + async () => + options.schedule(async () => + bapiRequest({ + method: "GET", + path: `/v1/users?${params.toString()}`, + secretKey: options.secretKey, + }), + ), + { onRetry: ({ delaySeconds }) => options.schedule.pause(delaySeconds * 1000) }, + ); + const page = Array.isArray(response.body) ? (response.body as LookedUpUser[]) : []; + found.push(...page); + const last = page.at(-1)?.id; + if (page.length < LOOKUP_BATCH || typeof last !== "string") break; + params.set("starting_after", last); + } progress.done++; options.spinner?.update( `${options.label ?? "Looking up users"}: ${progress.done}/${progress.total}`, ); - const users = response.body; - return Array.isArray(users) ? (users as LookedUpUser[]) : []; + return found; }), ); From 36f728e0ec763c07b1f74227daa7fd3687319489 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:10:59 -0400 Subject: [PATCH 30/34] fix(migrate): name the kept record on a duplicate source ID too Every other in-file duplicate carries keptSourceId. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/checks.test.ts | 4 +++- packages/cli-core/src/commands/migrate/lib/checks.ts | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 12685bbf1..d6f0c718f 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -310,7 +310,9 @@ describe("rejects", () => { test("a repeated source ID rejects only the later copy", async () => { const checks = await checkImport(input({ users: [user("a"), user("a")] })); expect(checks.importable.map((u) => u.userId)).toEqual(["a"]); - expect(checks.rejects).toEqual([{ sourceId: "a", reason: "duplicate source ID in the file" }]); + expect(checks.rejects).toEqual([ + { sourceId: "a", reason: "duplicate source ID in the file", keptSourceId: "a" }, + ]); }); test("a rejected user does not claim its email from a later one", async () => { diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index c3267f40f..26cdc9fb7 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -540,6 +540,8 @@ function findFileDuplicates(users: User[]): { for (const user of users) { if (seenIds.has(user.userId)) { reasons.set(user, "duplicate source ID in the file"); + // The earlier record under the same ID is the one kept. + keptBy.set(user, user.userId); continue; } seenIds.add(user.userId); From fe59c2adf31b6ce38ccdcfcb8083319fc93d470f Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:11:29 -0400 Subject: [PATCH 31/34] fix(migrate): offer no setting fix for a user that is a duplicate On a re-run, the users already in the instance would suggest changes that affect no user who will be imported. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/checks.test.ts | 12 ++++++++++++ .../src/commands/migrate/lib/checks.ts | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index d6f0c718f..1923f2aa6 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -876,6 +876,18 @@ describe("warnings", () => { }); describe("fixes", () => { + // A re-run: the users already imported would gain nothing from the change. + test("leave out a user already in the instance", async () => { + existing = [{ id: "user_1", external_id: "a" }]; + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, first_name: { enabled: false } }), + users: [user("a", { firstName: "Ada" }), user("b")], + }), + ); + expect(checks.fixes).toEqual([]); + }); + test("print a `clerk config patch` per flagged setting, targeting the instance", async () => { const checks = await checkImport( input({ diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 26cdc9fb7..365ba9ced 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -1031,14 +1031,17 @@ export async function checkImport(input: CheckInput): Promise { } } + // No setting would let a duplicate in, so none earns a fix. + const duplicates = new Set(); const { reasons: fileDuplicates, keptBy } = findFileDuplicates(passed); let candidates: User[] = []; for (const user of passed) { const reason = fileDuplicates.get(user); const kept = keptBy.get(user); - if (reason) + if (reason) { rejects.push({ sourceId: user.userId, reason, ...(kept ? { keptSourceId: kept } : {}) }); - else candidates.push(user); + duplicates.add(unstripped.get(user) ?? user); + } else candidates.push(user); } const instanceDuplicates = @@ -1048,8 +1051,10 @@ export async function checkImport(input: CheckInput): Promise { const unique: User[] = []; for (const user of candidates) { const reason = instanceDuplicates.get(user.userId); - if (reason) rejects.push({ sourceId: user.userId, reason }); - else unique.push(user); + if (reason) { + rejects.push({ sourceId: user.userId, reason }); + duplicates.add(unstripped.get(user) ?? user); + } else unique.push(user); } candidates = unique; @@ -1102,7 +1107,10 @@ export async function checkImport(input: CheckInput): Promise { ] : []), ], - fixes: buildFixes(input, input.users), + fixes: buildFixes( + input, + input.users.filter((user) => !duplicates.has(user)), + ), ...(quota ? { quota } : {}), settingsUnavailable: input.settings === null, }; From f191d4598689fd954140df138424be5f905b5281 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:11:59 -0400 Subject: [PATCH 32/34] fix(migrate): count the legacy Twitter strategy as enabling X An instance with `oauth_twitter` on flagged Supabase's Twitter users as having a provider that is not enabled, and could reject them. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/clerk-config.test.ts | 9 ++++++++- .../src/commands/migrate/lib/clerk-config.ts | 4 +++- .../src/commands/migrate/lib/readiness.test.ts | 13 +++++++++++++ .../cli-core/src/commands/migrate/lib/readiness.ts | 10 +++++++--- 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts index 5fe3842bc..b7af5ac64 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.test.ts @@ -1,7 +1,7 @@ import { test, expect, describe, mock, beforeEach, afterAll } from "bun:test"; import { stubFetch, useCaptureLog } from "../../../test/lib/stubs.ts"; import type { UserSettingsJSON } from "../../../lib/fapi.ts"; -import { fetchInstanceSettings, fetchUserCount } from "./clerk-config.ts"; +import { enabledSocialProviders, fetchInstanceSettings, fetchUserCount } from "./clerk-config.ts"; const USER_SETTINGS = { attributes: { email_address: { enabled: true, required: true } }, @@ -146,3 +146,10 @@ describe("fetchUserCount", () => { expect(calls).toBe(2); }); }); + +test("enabledSocialProviders counts the legacy Twitter strategy as X", () => { + const enabled = enabledSocialProviders({ + social: { oauth_twitter: { enabled: true }, oauth_github: { enabled: false } }, + } as unknown as UserSettingsJSON); + expect(enabled).toEqual(["oauth_twitter", "oauth_x"]); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts index 4897ae548..7474ed077 100644 --- a/packages/cli-core/src/commands/migrate/lib/clerk-config.ts +++ b/packages/cli-core/src/commands/migrate/lib/clerk-config.ts @@ -157,9 +157,11 @@ export async function fetchInstanceSettings(secretKey: string): Promise value?.enabled) .map(([strategy]) => strategy); + // The legacy Twitter strategy signs a Twitter user in as well as X does. + return enabled.includes("oauth_twitter") ? [...enabled, "oauth_x"] : enabled; } /** diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.test.ts b/packages/cli-core/src/commands/migrate/lib/readiness.test.ts index 76d9a4eb4..2f685bd1e 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.test.ts @@ -215,6 +215,19 @@ describe("present in the file but disabled in Clerk", () => { expect(item(report, "Microsoft (Azure)")?.blocking).toBe(false); }); + // Supabase's Twitter users sign in to either of Clerk's strategies for X. + test("counts the legacy Twitter strategy as enabling X", () => { + const report = buildReadinessReport({ + analysis: analysis({ totalUsers: 1, identifiers: { verifiedEmails: 1 } as never }), + settings: settings({ + attributes: { email_address: { enabled: true } }, + social: { oauth_twitter: { enabled: true } }, + }), + providerCounts: { twitter: 1 }, + }); + expect(item(report, "Twitter (X)")?.blocking).toBe(false); + }); + test("ignores a provider no user actually signed up with", () => { const report = buildReadinessReport({ analysis: analysis({ totalUsers: 1, identifiers: { verifiedEmails: 1 } as never }), diff --git a/packages/cli-core/src/commands/migrate/lib/readiness.ts b/packages/cli-core/src/commands/migrate/lib/readiness.ts index 22b54aff7..f2686c29c 100644 --- a/packages/cli-core/src/commands/migrate/lib/readiness.ts +++ b/packages/cli-core/src/commands/migrate/lib/readiness.ts @@ -12,7 +12,12 @@ import type { UserSettingsJSON } from "../../../lib/fapi.ts"; // Pure attribute lookups, shared with the `users` create wizard. import { isEnabled, isRequired, type AttributeName } from "../../users/interactive/attributes.ts"; import type { FieldAnalysis } from "./analysis.ts"; -import { clerkOffersProvider, providerLabel, toClerkStrategy } from "./clerk-config.ts"; +import { + clerkOffersProvider, + enabledSocialProviders, + providerLabel, + toClerkStrategy, +} from "./clerk-config.ts"; export type ReadinessSection = "identifiers" | "auth" | "social" | "model"; @@ -205,8 +210,7 @@ export function buildReadinessReport(input: BuildInput): ReadinessReport { for (const [provider, count] of Object.entries(providerCounts)) { if (count === 0) continue; const enabled = settings - ? (settings.social?.[toClerkStrategy(provider) as keyof typeof settings.social]?.enabled ?? - false) + ? enabledSocialProviders(settings).includes(toClerkStrategy(provider)) : null; items.push({ label: providerLabel(provider), From dea6f90f1631b50fb40e01b9a02f1789d446a638 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:32:11 -0400 Subject: [PATCH 33/34] fix(migrate): leave a duplicate out of the fixes when a check changed its record The duplicate set held the stripped copy, so filtering the file's own records by it kept a duplicate whose name, username, email or phone a check dropped. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/checks.test.ts | 12 ++++++++++++ packages/cli-core/src/commands/migrate/lib/checks.ts | 7 +++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 1923f2aa6..8ffc06774 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -876,6 +876,18 @@ describe("warnings", () => { }); describe("fixes", () => { + // A check that strips a value hands the duplicate checks a copy of the record. + test("leave out a user already in the instance whose record a check changed", async () => { + existing = [{ id: "user_1", external_id: "a" }]; + const checks = await checkImport( + input({ + settings: settings({ email_address: { enabled: true }, first_name: { enabled: false } }), + users: [user("a", { firstName: "Ada", lastName: "Lovelace" }), user("b")], + }), + ); + expect(checks.fixes).toEqual([]); + }); + // A re-run: the users already imported would gain nothing from the change. test("leave out a user already in the instance", async () => { existing = [{ id: "user_1", external_id: "a" }]; diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 365ba9ced..263e02a6f 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -971,6 +971,8 @@ export async function checkImport(input: CheckInput): Promise { // what is sent; `unstripped` keeps what the warnings describe. const passed: User[] = []; const unstripped = new Map(); + // The file's own record for each, which the fixes are built from. + const originals = new Map(); const placeholderEmails = new Set(); const refusedNames = new Set(); const refusedUsernames = new Set(); @@ -1027,6 +1029,7 @@ export async function checkImport(input: CheckInput): Promise { } else { passed.push(sent); unstripped.set(sent, user); + originals.set(sent, original); if (refused.length > 0) placeholderEmails.add(user.userId); } } @@ -1040,7 +1043,7 @@ export async function checkImport(input: CheckInput): Promise { const kept = keptBy.get(user); if (reason) { rejects.push({ sourceId: user.userId, reason, ...(kept ? { keptSourceId: kept } : {}) }); - duplicates.add(unstripped.get(user) ?? user); + duplicates.add(originals.get(user) ?? user); } else candidates.push(user); } @@ -1053,7 +1056,7 @@ export async function checkImport(input: CheckInput): Promise { const reason = instanceDuplicates.get(user.userId); if (reason) { rejects.push({ sourceId: user.userId, reason }); - duplicates.add(unstripped.get(user) ?? user); + duplicates.add(originals.get(user) ?? user); } else unique.push(user); } candidates = unique; From d96fc3e81d0178226b96fb9ad1a8585360e9e285 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 17:32:22 -0400 Subject: [PATCH 34/34] fix(migrate): stop paging a lookup when its cursor does not move A server that ignored `starting_after` would serve the same full page forever. Co-Authored-By: Claude Opus 5.5 --- .../commands/migrate/lib/user-lookup.test.ts | 20 +++++++++++++++++++ .../src/commands/migrate/lib/user-lookup.ts | 2 ++ 2 files changed, 22 insertions(+) diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts index a5ad65e4f..8f74286c4 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts @@ -86,3 +86,23 @@ test("reads every page of a batch that matches more than one page", async () => // The `+` keeps a leading `-` from reading as "exclude". expect(urls[0]?.searchParams.getAll("external_id")).toEqual(["+-dup"]); }); + +test("stops paging when the cursor does not move", async () => { + let requests = 0; + // A server that ignores `starting_after` serves the same full page again. + globalThis.fetch = (async () => { + requests++; + return Response.json( + Array.from({ length: LOOKUP_BATCH }, (_, index) => ({ id: `user_${index}` })), + ); + }) as unknown as typeof fetch; + + await lookupUsers({ + filter: "external_id", + values: ["a"], + secretKey: "sk_test_x", + schedule: createApiScheduler(1, 10_000), + }); + + expect(requests).toBe(2); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index 85fabd07e..efca044d9 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -87,6 +87,8 @@ export async function lookupUsers(options: { found.push(...page); const last = page.at(-1)?.id; if (page.length < LOOKUP_BATCH || typeof last !== "string") break; + // A cursor that does not move would page forever. + if (params.get("starting_after") === last) break; params.set("starting_after", last); } progress.done++;