From 173842a1a7bb5947b1620f0e8d1c1636566c2047 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:22 -0400 Subject: [PATCH 01/27] feat(migrate): port throughput, the target, the run store and the importer 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/import-users.test.ts | 610 ++++++++++++++++++ .../src/commands/migrate/import-users.ts | 473 ++++++++++++++ .../src/commands/migrate/lib/instance.test.ts | 97 +++ .../src/commands/migrate/lib/instance.ts | 111 ++++ .../src/commands/migrate/lib/progress.test.ts | 108 ++++ .../src/commands/migrate/lib/progress.ts | 131 ++++ .../src/commands/migrate/lib/retry.test.ts | 156 +++++ .../src/commands/migrate/lib/retry.ts | 66 ++ .../commands/migrate/lib/run-store.test.ts | 249 +++++++ .../src/commands/migrate/lib/run-store.ts | 454 +++++++++++++ .../commands/migrate/lib/scheduler.test.ts | 69 ++ .../src/commands/migrate/lib/scheduler.ts | 62 ++ .../src/commands/migrate/lib/target.test.ts | 155 +++++ .../src/commands/migrate/lib/target.ts | 177 +++++ packages/cli-core/src/lib/config.ts | 2 +- 15 files changed, 2919 insertions(+), 1 deletion(-) create mode 100644 packages/cli-core/src/commands/migrate/import-users.test.ts create mode 100644 packages/cli-core/src/commands/migrate/import-users.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/instance.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/instance.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/progress.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/progress.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/retry.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/retry.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/run-store.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/run-store.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/scheduler.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/scheduler.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/target.test.ts create mode 100644 packages/cli-core/src/commands/migrate/lib/target.ts diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts new file mode 100644 index 000000000..47751cc73 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -0,0 +1,610 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { BapiError } from "../../lib/errors.ts"; +import { + buildCreateUserBody, + importUsers, + normalizeErrorMessage, + readRetryAfter, + splitIdentifiers, +} from "./import-users.ts"; +import type { ResolvedLimits } from "./lib/instance.ts"; +import type { UserLine } from "./lib/run-store.ts"; +import type { User } from "./types.ts"; + +const LIMITS: ResolvedLimits = { instanceType: "dev", rateLimit: 10_000, concurrencyLimit: 8 }; + +const user = (overrides: Partial = {}): User => + ({ userId: "u1", email: "a@x.dev", ...overrides }) as User; + +describe("splitIdentifiers", () => { + test("promotes the first verified email and phone to primary", () => { + const result = splitIdentifiers( + user({ email: ["a@x.dev", "b@x.dev"], phone: ["+15555550100", "+15555550101"] }), + ); + expect(result.primaryEmail).toBe("a@x.dev"); + expect(result.additionalEmails).toEqual(["b@x.dev"]); + expect(result.primaryPhone).toBe("+15555550100"); + expect(result.additionalPhones).toEqual(["+15555550101"]); + }); + + test("merges the email and emailAddresses fields, deduping", () => { + const result = splitIdentifiers( + user({ email: "a@x.dev", emailAddresses: ["a@x.dev", "b@x.dev"] }), + ); + expect(result.primaryEmail).toBe("a@x.dev"); + expect(result.additionalEmails).toEqual(["b@x.dev"]); + }); + + test("drops an unverified identifier that is already verified", () => { + const result = splitIdentifiers( + user({ email: ["a@x.dev"], unverifiedEmailAddresses: ["a@x.dev", "c@x.dev"] }), + ); + expect(result.unverifiedEmails).toEqual(["c@x.dev"]); + }); + + test("copes with a user identified only by username", () => { + const result = splitIdentifiers({ userId: "u1", username: "alice" } as User); + expect(result.primaryEmail).toBeUndefined(); + expect(result.additionalEmails).toEqual([]); + }); +}); + +describe("buildCreateUserBody", () => { + test("maps the schema onto BAPI's snake_case body", () => { + const target = user({ + firstName: "Alice", + lastName: "Smith", + username: "alice", + createdAt: "2024-01-01T00:00:00.000Z", + publicMetadata: { plan: "pro" }, + createOrganizationsLimit: 3, + banned: true, + }); + const body = buildCreateUserBody(target, splitIdentifiers(target), true); + + expect(body).toMatchObject({ + external_id: "u1", + email_address: ["a@x.dev"], + first_name: "Alice", + last_name: "Smith", + username: "alice", + created_at: "2024-01-01T00:00:00.000Z", + public_metadata: { plan: "pro" }, + create_organizations_limit: 3, + banned: true, + }); + }); + + test("sends only the primary identifier; the rest are attached separately", () => { + const target = user({ email: ["a@x.dev", "b@x.dev"] }); + expect(buildCreateUserBody(target, splitIdentifiers(target), true).email_address).toEqual([ + "a@x.dev", + ]); + }); + + // Allowlists and blocklists police sign-ups; these users already signed up. + test("skips the instance's sign-up restrictions", () => { + expect(buildCreateUserBody(user(), splitIdentifiers(user()), true)).toMatchObject({ + skip_restriction_checks: true, + }); + }); + + test("omits fields the source platform never recorded", () => { + const body = buildCreateUserBody(user(), splitIdentifiers(user()), true); + expect("first_name" in body).toBe(false); + expect("banned" in body).toBe(false); + expect("created_at" in body).toBe(false); + }); + + test("sends the password digest and hasher together", () => { + const target = user({ password: "digest", passwordHasher: "bcrypt" }); + const body = buildCreateUserBody(target, splitIdentifiers(target), true); + expect(body).toMatchObject({ password_digest: "digest", password_hasher: "bcrypt" }); + expect("skip_password_requirement" in body).toBe(false); + }); + + test.each([ + [true, true], + [false, false], + ])("skipPasswordRequirement=%p on a passwordless user -> flag present: %p", (skip, present) => { + const body = buildCreateUserBody(user(), splitIdentifiers(user()), skip); + expect("skip_password_requirement" in body).toBe(present); + }); +}); + +describe("readRetryAfter", () => { + const withHeader = (value: string) => + new BapiError(429, "{}", new Headers({ "retry-after": value })); + + test.each([ + ["12", 12], + ["0", undefined], + ["soon", undefined], + ])("Retry-After: %s -> %p", (header, expected) => { + expect(readRetryAfter(withHeader(header))).toBe(expected as number | undefined); + }); + + test("falls back to the error body's retryAfter meta", () => { + const error = new BapiError( + 429, + JSON.stringify({ + errors: [{ code: "rate_limit", message: "slow down", meta: { retryAfter: 7 } }], + }), + new Headers(), + ); + expect(readRetryAfter(error)).toBe(7); + }); + + test("returns undefined when neither source carries a value", () => { + expect(readRetryAfter(new BapiError(429, "{}", new Headers()))).toBeUndefined(); + }); +}); + +describe("normalizeErrorMessage", () => { + test("sorts field arrays so equivalent errors group together", () => { + const a = normalizeErrorMessage('["last_name" "first_name"] data does not match'); + const b = normalizeErrorMessage('["first_name" "last_name"] data does not match'); + expect(a).toBe(b); + expect(a).toBe('["first_name" "last_name"] data does not match'); + }); + + test("leaves messages without field arrays untouched", () => { + expect(normalizeErrorMessage("that email is taken")).toBe("that email is taken"); + }); +}); + +describe("importUsers", () => { + let originalFetch: typeof globalThis.fetch; + let requests: { method: string; url: string; body: unknown }[]; + let lines: UserLine[]; + let allLines: UserLine[]; + // `lines` leaves out the `creating` marker every user gets first; one test + // below covers it through `allLines`. + const record = (line: UserLine) => { + allLines.push(line); + if (line.status !== "creating") lines.push(line); + }; + + beforeAll(() => { + originalFetch = globalThis.fetch; + }); + + afterAll(() => { + globalThis.fetch = originalFetch; + }); + + beforeEach(() => { + requests = []; + lines = []; + allLines = []; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + /** Installs a fetch that records every request and replies per `respond`. */ + function stub(respond: (url: string, attempt: number) => Response): void { + const attempts = new Map(); + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = input.toString(); + requests.push({ + method: init?.method ?? "GET", + url, + body: init?.body ? JSON.parse(init.body as string) : null, + }); + const attempt = (attempts.get(url) ?? 0) + 1; + attempts.set(url, attempt); + return respond(url, attempt); + }) as typeof fetch; + } + + const ok = (id: string) => new Response(JSON.stringify({ id }), { status: 200 }); + + const clerkError = (status: number, message: string, headers?: Record) => + new Response(JSON.stringify({ errors: [{ code: "err", message, long_message: message }] }), { + status, + headers, + }); + + test("creates each user and reports them as successful", async () => { + stub(() => ok("user_created")); + + const summary = await importUsers({ + users: [user({ userId: "u1" }), user({ userId: "u2", email: "b@x.dev" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ totalProcessed: 2, successful: 2, failed: 0 }); + expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(2); + expect(lines.filter((line) => line.status === "created")).toHaveLength(2); + }); + + test("attaches additional and unverified identifiers after the user exists", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [ + user({ + email: ["a@x.dev", "b@x.dev"], + unverifiedEmailAddresses: ["c@x.dev"], + phone: ["+15555550100", "+15555550101"], + }), + ], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + const emails = requests.filter((r) => r.url.endsWith("/v1/email_addresses")); + expect(emails.map((r) => r.body)).toEqual([ + { user_id: "user_created", email_address: "b@x.dev", primary: false, verified: true }, + { user_id: "user_created", email_address: "c@x.dev", primary: false, verified: false }, + ]); + expect(requests.filter((r) => r.url.endsWith("/v1/phone_numbers"))).toHaveLength(1); + }); + + test("marks a user whose password the source dropped", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [user({ passwordDropped: true })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(lines[0]).toMatchObject({ status: "created", passwordDropped: true }); + // Never sent: it is the CLI's own bookkeeping, not a Clerk field. + expect(JSON.stringify(requests[0]?.body)).not.toContain("passwordDropped"); + }); + + test("notes a failed additional identifier without failing the user", async () => { + stub((url) => + url.endsWith("/v1/email_addresses") + ? clerkError(422, "that email is taken") + : ok("user_created"), + ); + + const summary = await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 1, failed: 0 }); + // On record once created, then with its attach pending, then with what + // the attach added. A refused attach is not retried, so nothing is pending. + expect(lines).toHaveLength(3); + expect(lines[0]).toEqual({ sourceId: "u1", clerkId: "user_created", status: "created" }); + expect(lines[1]?.pending).toEqual([{ kind: "email", value: "b@x.dev", verified: true }]); + expect(lines[2]).toMatchObject({ status: "created", clerkId: "user_created" }); + expect(lines[2]?.error).toContain("Failed to add additional email b@x.dev"); + expect(lines[2]).not.toHaveProperty("pending"); + }); + + test("retries an attach that hits a 429", async () => { + stub((url, attempt) => + url.endsWith("/v1/email_addresses") && attempt === 1 + ? clerkError(429, "slow down", { "retry-after": "1" }) + : ok("user_created"), + ); + + await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(requests.filter((r) => r.url.endsWith("/v1/email_addresses"))).toHaveLength(2); + expect(lines.at(-1)).not.toHaveProperty("error"); + expect(lines.at(-1)).not.toHaveProperty("pending"); + }); + + test("keeps an attach with no answer pending, for a continued run", async () => { + stub((url) => + url.endsWith("/v1/email_addresses") ? clerkError(503, "unavailable") : ok("user_created"), + ); + + await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(lines.at(-1)?.pending).toEqual([{ kind: "email", value: "b@x.dev", verified: true }]); + }); + + // One slot: a user's attaches go ahead of the next queued create, so a run + // stopped midway leaves few users without their extra identifiers. + test("attaches a user's identifiers before the next queued create", async () => { + stub((url) => ok(url.endsWith("/v1/users") ? "user_created" : "idn_1")); + + await importUsers({ + users: [ + user({ userId: "u1", email: ["a@x.dev", "b@x.dev"] }), + user({ userId: "u2", email: ["c@x.dev", "d@x.dev"] }), + user({ userId: "u3", email: ["e@x.dev", "f@x.dev"] }), + ], + secretKey: "sk_test_x", + limits: { ...LIMITS, concurrencyLimit: 1 }, + record, + }); + + expect(requests.map((r) => new URL(r.url).pathname)).toEqual([ + "/v1/users", + "/v1/email_addresses", + "/v1/users", + "/v1/email_addresses", + "/v1/users", + "/v1/email_addresses", + ]); + }); + + test("attachOnly sends just the pending attaches, and clears them", async () => { + stub(() => ok("idn_1")); + + await importUsers({ + users: [], + attachOnly: [ + { + sourceId: "u1", + clerkId: "user_1", + status: "created", + pending: [{ kind: "phone", value: "+15555550100", verified: false }], + }, + ], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(requests.map((r) => [new URL(r.url).pathname, r.body])).toEqual([ + [ + "/v1/phone_numbers", + { user_id: "user_1", phone_number: "+15555550100", primary: false, verified: false }, + ], + ]); + expect(lines.at(-1)).toEqual({ sourceId: "u1", clerkId: "user_1", status: "created" }); + }); + + test("an adopted user is not created again; only its extras attach", async () => { + stub(() => ok("idn_1")); + + const summary = await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + adopted: new Map([["u1", "user_found"]]), + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(requests.map((r) => new URL(r.url).pathname)).toEqual(["/v1/email_addresses"]); + expect(summary.successful).toBe(1); + expect(lines.at(-1)).toMatchObject({ clerkId: "user_found", status: "created" }); + }); + + // Shapes from clerk_go's apierror: the country error carries its own code + // and no param_name; the E.164 error is a form error on phone_number. + test.each([ + [ + 403, + { + code: "unsupported_country_code", + message: "Unsupported country code", + long_message: "Phone numbers from this country (Netherlands) are currently not supported.", + meta: { alpha2: "NL", country_code: "31" }, + }, + ], + [ + 422, + { + code: "form_param_format_invalid", + message: "is invalid", + long_message: + "Phone number must be a valid phone number according to E.164 international standard.", + meta: { param_name: "phone_number" }, + }, + ], + ])("retries without a phone Clerk refuses (%i), and notes it", async (status, clerkErr) => { + stub((url, attempt) => + url.endsWith("/v1/users") && attempt === 1 + ? new Response(JSON.stringify({ errors: [clerkErr] }), { status }) + : ok("user_created"), + ); + + const summary = await importUsers({ + users: [user({ phone: "+31612345678" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 1, failed: 0 }); + expect(requests).toHaveLength(2); + expect(requests[1]?.body).not.toHaveProperty("phone_number"); + expect(lines.at(-1)?.error).toContain( + `Failed to add phone +31612345678: ${clerkErr.long_message}`, + ); + }); + + test("does not retry without the phone when it is the only identifier", async () => { + stub( + () => + new Response( + JSON.stringify({ + errors: [{ code: "x", message: "bad phone", meta: { param_name: "phone_number" } }], + }), + { status: 422 }, + ), + ); + + const summary = await importUsers({ + users: [user({ email: undefined, phone: "+31612345678" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary.failed).toBe(1); + expect(requests).toHaveLength(1); + }); + + // The marker `undo` uses to find a user whose create was in flight when the + // run stopped. + test("marks each user as creating before its POST /v1/users", async () => { + let recordedBeforePost = false; + stub((url) => { + if (url.endsWith("/v1/users")) { + recordedBeforePost = allLines.some((line) => line.status === "creating"); + } + return ok("user_created"); + }); + + await importUsers({ users: [user()], secretKey: "sk_test_x", limits: LIMITS, record }); + + expect(recordedBeforePost).toBe(true); + expect(allLines.map((line) => line.status)).toEqual(["creating", "created"]); + }); + + // A run stopped while attaches wait on the scheduler must still have the + // user on record, or `undo` leaves it behind. + test("records the user before its additional identifiers attach", async () => { + let recordedBeforeAttach = false; + stub((url) => { + if (url.endsWith("/v1/email_addresses")) { + recordedBeforeAttach = lines.some( + (line) => line.status === "created" && line.clerkId === "user_created", + ); + } + return ok("user_created"); + }); + + await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(recordedBeforeAttach).toBe(true); + }); + + // A user is on record only once its create may land, so `undo` never + // looks up users that were still queued when the run stopped. + test("writes creating only as each POST /v1/users goes out", async () => { + const creatingAtFirstPost: number[] = []; + stub((url) => { + if (url.endsWith("/v1/users") && creatingAtFirstPost.length === 0) { + creatingAtFirstPost.push(allLines.filter((line) => line.status === "creating").length); + } + return ok("user_created"); + }); + + await importUsers({ + users: [user({ userId: "u1" }), user({ userId: "u2" }), user({ userId: "u3" })], + secretKey: "sk_test_x", + limits: { ...LIMITS, concurrencyLimit: 1 }, + record, + }); + + expect(creatingAtFirstPost).toEqual([1]); + }); + + test.each([ + ["a 5xx", () => clerkError(502, "bad gateway")], + [ + "a network error", + () => { + throw new TypeError("fetch failed"); + }, + ], + ])("a create that gets %s keeps creating, not failed", async (_label, respond) => { + stub(respond); + + const summary = await importUsers({ + users: [user()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary.failed).toBe(1); + expect(allLines.map((line) => line.status)).toEqual(["creating"]); + expect([...summary.errorBreakdown.keys()][0]).toContain("a re-run checks"); + }); + + test("records a failed user and keeps going", async () => { + stub((_url, attempt) => + attempt === 1 ? clerkError(422, "that email is taken") : ok("user_ok"), + ); + + const summary = await importUsers({ + users: [user({ userId: "u1" }), user({ userId: "u2", email: "b@x.dev" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary.successful + summary.failed).toBe(2); + expect(summary.failed).toBe(1); + expect([...summary.errorBreakdown.values()]).toEqual([1]); + expect(lines.some((line) => line.status === "failed" && line.code === "422")).toBe(true); + }); + + test("retries a 429 after the interval the server asked for", async () => { + stub((_url, attempt) => + attempt === 1 ? clerkError(429, "slow down", { "retry-after": "1" }) : ok("user_ok"), + ); + + const started = performance.now(); + const summary = await importUsers({ + users: [user()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 1, failed: 0 }); + expect(performance.now() - started).toBeGreaterThanOrEqual(900); + expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(2); + expect(lines).toHaveLength(2); + expect(lines[1]).toMatchObject({ status: "created", clerkId: "user_ok" }); + expect(lines[1]?.error).toContain("Rate limit hit (429)"); + }); + + test("gives up after the retry ceiling and records the user as failed", async () => { + stub(() => clerkError(429, "slow down", { "retry-after": "1" })); + + const summary = await importUsers({ + users: [user()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 0, failed: 1 }); + // One initial attempt plus MAX_RETRIES retries. + expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(6); + expect(lines).toMatchObject([{ status: "failed", code: "429" }]); + }, 20_000); + + test("carries the validation failure count into the summary", async () => { + stub(() => ok("user_ok")); + + const summary = await importUsers({ + users: [user()], + secretKey: "sk_test_x", + limits: LIMITS, + record, + validationFailed: 4, + }); + + expect(summary.validationFailed).toBe(4); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts new file mode 100644 index 000000000..533dd7938 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -0,0 +1,473 @@ +/** + * Creates users in Clerk from a validated batch. + * + * Ported from the standalone migration-tool's `src/migrate/import-users.ts`, + * rewritten onto `bapiRequest` instead of `@clerk/backend`. Two things fall out + * of that move: + * + * - The request body is BAPI's snake_case shape directly, so `created_at` and + * `legal_accepted_at` stay RFC3339 strings rather than round-tripping + * through `Date`. + * - `banned`, `delete_self_enabled` and the organization limits are all + * accepted by `POST /v1/users`, so the follow-up `updateUser`/`banUser` + * calls the SDK version needed are gone. + * + * Run state is local to {@link importUsers} rather than module-level, so two + * runs in one process (or one test file) cannot see each other's counters. + */ + +import { bapiRequest } from "../../lib/bapi.ts"; +import { BapiError } from "../../lib/errors.ts"; +import { interruptSignal } from "../../lib/signals.ts"; +import type { ResolvedLimits } from "./lib/instance.ts"; +import type { ProgressUpdate } from "./lib/progress.ts"; +import { RateLimitExceededError, retryOn429 } from "./lib/retry.ts"; +import type { PendingIdentifier, UserLine } from "./lib/run-store.ts"; +import { createApiScheduler, type ApiScheduler } from "./lib/scheduler.ts"; +import type { ImportSummary, User } from "./types.ts"; + +// Re-exported for the tests and callers that grew up against this module. +export { readRetryAfter } from "./lib/retry.ts"; + +/** + * Groups error messages that differ only in field ordering, so the summary + * reports "12 users: [\"first_name\" \"last_name\"] ..." once instead of twice. + */ +export function normalizeErrorMessage(errorMessage: string): string { + let normalized = ""; + let lastCopiedIndex = 0; + let arrayStartIndex = -1; + + for (let i = 0; i < errorMessage.length; i++) { + const char = errorMessage[i]; + + if (arrayStartIndex === -1) { + if (char === "[") arrayStartIndex = i; + continue; + } + if (char !== "]") continue; + + normalized += errorMessage.slice(lastCopiedIndex, arrayStartIndex); + normalized += normalizeFieldArray(errorMessage.slice(arrayStartIndex + 1, i)); + lastCopiedIndex = i + 1; + arrayStartIndex = -1; + } + + return normalized + errorMessage.slice(lastCopiedIndex); +} + +function normalizeFieldArray(fields: string): string { + const fieldNames: string[] = []; + let current = ""; + + for (const char of fields) { + if (char === '"' || char === "'" || char.trim() === "") { + if (current.length > 0) { + fieldNames.push(current); + current = ""; + } + continue; + } + current += char; + } + if (current.length > 0) fieldNames.push(current); + + fieldNames.sort(); + return `[${fieldNames.map((name) => `"${name}"`).join(" ")}]`; +} + +function toArray(value: string | string[] | undefined): string[] { + if (!value) return []; + return Array.isArray(value) ? value : [value]; +} + +function dedupe(values: string[]): string[] { + const seen: string[] = []; + for (const value of values) { + if (value && !seen.includes(value)) seen.push(value); + } + return seen; +} + +type Identifiers = { + primaryEmail: string | undefined; + additionalEmails: string[]; + unverifiedEmails: string[]; + primaryPhone: string | undefined; + additionalPhones: string[]; + unverifiedPhones: string[]; +}; + +/** + * Splits a user's identifiers into the one that goes on `POST /v1/users` and + * the rest, which are attached afterwards. + */ +export function splitIdentifiers(user: User): Identifiers { + const verifiedEmails = dedupe([...toArray(user.email), ...toArray(user.emailAddresses)]); + const verifiedPhones = dedupe([...toArray(user.phone), ...toArray(user.phoneNumbers)]); + + return { + primaryEmail: verifiedEmails[0], + additionalEmails: verifiedEmails.slice(1), + unverifiedEmails: dedupe( + toArray(user.unverifiedEmailAddresses).filter((email) => !verifiedEmails.includes(email)), + ), + primaryPhone: verifiedPhones[0], + additionalPhones: verifiedPhones.slice(1), + unverifiedPhones: dedupe( + toArray(user.unverifiedPhoneNumbers).filter((phone) => !verifiedPhones.includes(phone)), + ), + }; +} + +/** + * Builds the `POST /v1/users` request body. + * + * Optional fields are omitted rather than sent as null so Clerk applies its own + * defaults for anything the source platform did not record. + */ +export function buildCreateUserBody( + user: User, + identifiers: Identifiers, + skipPasswordRequirement: boolean, +): Record { + // The instance's allowlist, blocklist, disposable-email and subaddress rules + // police sign-ups. These users already signed up, on the source platform. + const body: Record = { external_id: user.userId, skip_restriction_checks: true }; + + if (identifiers.primaryEmail) body.email_address = [identifiers.primaryEmail]; + if (identifiers.primaryPhone) body.phone_number = [identifiers.primaryPhone]; + if (user.firstName) body.first_name = user.firstName; + if (user.lastName) body.last_name = user.lastName; + if (user.username) body.username = user.username; + if (user.totpSecret) body.totp_secret = user.totpSecret; + if (user.backupCodes) body.backup_codes = user.backupCodes; + if (user.unsafeMetadata) body.unsafe_metadata = user.unsafeMetadata; + if (user.privateMetadata) body.private_metadata = user.privateMetadata; + if (user.publicMetadata) body.public_metadata = user.publicMetadata; + if (user.createdAt) body.created_at = user.createdAt; + if (user.legalAcceptedAt) body.legal_accepted_at = user.legalAcceptedAt; + if (user.skipLegalChecks !== undefined) body.skip_legal_checks = user.skipLegalChecks; + if (user.skipPasswordChecks !== undefined) body.skip_password_checks = user.skipPasswordChecks; + if (user.banned !== undefined) body.banned = user.banned; + if (user.bypassClientTrust !== undefined) body.bypass_client_trust = user.bypassClientTrust; + if (user.deleteSelfEnabled !== undefined) body.delete_self_enabled = user.deleteSelfEnabled; + if (user.createOrganizationEnabled !== undefined) { + body.create_organization_enabled = user.createOrganizationEnabled; + } + if (user.createOrganizationsLimit !== undefined) { + body.create_organizations_limit = user.createOrganizationsLimit; + } + + if (user.password && user.passwordHasher) { + body.password_digest = user.password; + body.password_hasher = user.passwordHasher; + } else if (skipPasswordRequirement) { + body.skip_password_requirement = true; + } + // Without a password and without skipPasswordRequirement, Clerk rejects the + // user — which is exactly what --require-password is asking for. + + return body; +} + +type CreateContext = { + secretKey: string; + schedule: ApiScheduler; +}; + +/** + * True when no answer says whether a request landed: an abort, a network + * error, or a 5xx after which Clerk may still have committed it. A 4xx, and a + * 429 that ran out of retries, are definite refusals. + */ +export function outcomeUnknown(error: unknown): boolean { + if (error instanceof RateLimitExceededError) return false; + if (error instanceof BapiError) return error.status >= 500; + return true; +} + +/** The extra identifiers a user carries, in the order they are attached. */ +export function pendingIdentifiers(identifiers: Identifiers): PendingIdentifier[] { + return [ + ...identifiers.additionalEmails.map((value) => ({ + kind: "email" as const, + value, + verified: true, + })), + ...identifiers.unverifiedEmails.map((value) => ({ + kind: "email" as const, + value, + verified: false, + })), + ...identifiers.additionalPhones.map((value) => ({ + kind: "phone" as const, + value, + verified: true, + })), + ...identifiers.unverifiedPhones.map((value) => ({ + kind: "phone" as const, + value, + verified: false, + })), + ]; +} + +/** + * Attaches one extra identifier, backing off on a 429. + * + * @returns A note when Clerk refused it, `pending` when nothing says whether + * it attached. Never throws: the user itself was already created. + */ +async function attachIdentifier( + ctx: CreateContext, + clerkUserId: string, + { kind, value, verified }: PendingIdentifier, +): Promise<{ note?: string; pending?: boolean }> { + const path = kind === "email" ? "/v1/email_addresses" : "/v1/phone_numbers"; + const body = + kind === "email" + ? { user_id: clerkUserId, email_address: value, primary: false, verified } + : { user_id: clerkUserId, phone_number: value, primary: false, verified }; + + try { + await retryOn429(async () => + ctx.schedule( + async () => + bapiRequest({ + method: "POST", + path, + secretKey: ctx.secretKey, + body: JSON.stringify(body), + }), + { first: true }, + ), + ); + return {}; + } catch (error) { + if (outcomeUnknown(error)) return { pending: true }; + const label = `${verified ? "additional" : "unverified"} ${kind} ${value}`; + return { note: `Failed to add ${label}: ${(error as Error).message}` }; + } +} + +/** + * Attaches each identifier. Extra identifiers are best-effort: a duplicate + * secondary email should not undo a user who was otherwise imported. + * + * @returns A note per identifier Clerk refused, and those still pending. + */ +async function attachAll( + ctx: CreateContext, + clerkUserId: string, + identifiers: PendingIdentifier[], +): Promise<{ notes: string[]; pending: PendingIdentifier[] }> { + const results = await Promise.all( + identifiers.map(async (identifier) => attachIdentifier(ctx, clerkUserId, identifier)), + ); + return { + notes: results.flatMap((result) => (result.note ? [result.note] : [])), + pending: identifiers.filter((_, index) => results[index]?.pending), + }; +} + +/** + * Creates one user, retrying without a phone Clerk refuses. + * + * @param sending - Called as each `POST /v1/users` goes out, so the run + * records the user only once a create may actually land. + * @returns The Clerk ID, and a note when the phone was dropped. + */ +async function createUser( + ctx: CreateContext, + user: User, + identifiers: Identifiers, + skipPasswordRequirement: boolean, + sending: () => void, +): Promise<{ clerkUserId: string; notes: string[] }> { + const create = async (body: Record) => + ctx.schedule(async () => { + // A Ctrl-C hands the slot on to queued creates; none of them was sent. + interruptSignal().throwIfAborted(); + sending(); + return bapiRequest({ + method: "POST", + path: "/v1/users", + secretKey: ctx.secretKey, + body: JSON.stringify(body), + }); + }); + + const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); + const notes: string[] = []; + let response; + try { + response = await create(body); + } catch (error) { + // A phone Clerk refuses (a country it does not support, a number that is + // not E.164) should not cost a user who has an email to be created under. + // The country error names no parameter, only its own code. + const phoneRefused = + error instanceof BapiError && + (error.code === "unsupported_country_code" || error.meta?.param_name === "phone_number"); + if (!phoneRefused || !identifiers.primaryEmail) throw error; + const { phone_number: _dropped, ...withoutPhone } = body; + response = await create(withoutPhone); + notes.push( + `Failed to add phone ${identifiers.primaryPhone}: ${(error as BapiError).longMessage ?? (error as BapiError).message}`, + ); + } + + return { clerkUserId: (response.body as { id?: string })?.id ?? "", notes }; +} + +export type ImportUsersOptions = { + users: User[]; + secretKey: string; + limits: ResolvedLimits; + /** Receives each user's lines as they happen. */ + record: (line: UserLine) => void; + /** + * Users a continued run created whose extra identifiers never attached: their + * latest `created` line, with `pending`. Only the attaches are sent. + */ + attachOnly?: UserLine[]; + /** + * Source ID → Clerk ID for users whose create a stopped run sent with no + * answer, and which a continued run then found in the instance. They are + * not created again; only their extra identifiers are sent. + */ + adopted?: Map; + /** Allow users that carry no password. */ + skipPasswordRequirement?: boolean; + /** Carried into the summary so the report covers the whole file. */ + validationFailed?: number; + /** Receives the counts as each user finishes. */ + progress?: ProgressUpdate; +}; + +/** + * Imports every user, concurrently and within the instance's rate limit. + * + * A failed user is recorded and the run continues; a 429 backs off (honouring + * `Retry-After`) and retries up to {@link MAX_RETRIES} times. A create with no + * answer keeps its `creating` line, for a continued run or `undo` to resolve. + */ +export async function importUsers(options: ImportUsersOptions): Promise { + const { + users, + secretKey, + limits, + record, + attachOnly = [], + adopted = new Map(), + skipPasswordRequirement = true, + validationFailed = 0, + progress: report, + } = options; + + const total = users.length; + const errorBreakdown = new Map(); + let processed = 0; + let successful = 0; + let failed = 0; + + const ctx: CreateContext = { + secretKey, + schedule: createApiScheduler(limits.concurrencyLimit, limits.rateLimit), + }; + + const progress = () => report?.({ done: processed, ok: successful, failed }); + + const recordFailure = ( + userId: string, + message: string, + code: string, + notes: string[], + unknown: boolean, + ) => { + failed++; + processed++; + const normalized = normalizeErrorMessage(message); + errorBreakdown.set(normalized, (errorBreakdown.get(normalized) ?? 0) + 1); + // With no answer, the `creating` line stays the latest: Clerk may hold + // the user, and a re-run looks it up before creating it again. + if (!unknown) { + record({ sourceId: userId, status: "failed", error: [message, ...notes].join("; "), code }); + } + progress(); + }; + + /** + * Attaches a created user's extra identifiers. The user goes on record with + * them `pending` first, so a run stopped before they attach can finish them. + */ + const finishUser = async (line: UserLine, toAttach: PendingIdentifier[], notes: string[]) => { + const { error: _error, pending: _pending, ...base } = line; + if (toAttach.length > 0) record({ ...base, pending: toAttach }); + const attached = await attachAll(ctx, base.clerkId ?? "", toAttach); + const error = [...notes, ...attached.notes].join("; "); + // A second line, which wins as the latest, adds what happened on the way. + if (toAttach.length > 0 || error) { + record({ + ...base, + ...(error ? { error } : {}), + ...(attached.pending.length > 0 ? { pending: attached.pending } : {}), + }); + } + }; + + const processUser = async (user: User): Promise => { + const retries: string[] = []; + const identifiers = splitIdentifiers(user); + let created: { clerkUserId: string; notes: string[] }; + const adoptedId = adopted.get(user.userId); + try { + created = adoptedId + ? { clerkUserId: adoptedId, notes: [] } + : await retryOn429( + async () => + createUser(ctx, user, identifiers, skipPasswordRequirement, () => + record({ sourceId: user.userId, status: "creating" }), + ), + { onRetry: ({ message }) => retries.push(message) }, + ); + } catch (error) { + if (error instanceof RateLimitExceededError) { + recordFailure(user.userId, error.message, "429", retries, false); + return; + } + const apiError = error as BapiError; + const unknown = outcomeUnknown(error); + const message = apiError.longMessage ?? apiError.message ?? "Unknown error"; + recordFailure( + user.userId, + unknown ? `${message} (Clerk may have created the user; a re-run checks)` : message, + String(apiError.status ?? "unknown"), + retries, + unknown, + ); + return; + } + + const line: UserLine = { + sourceId: user.userId, + clerkId: created.clerkUserId, + status: "created", + ...(user.passwordDropped ? { passwordDropped: true } : {}), + }; + record(line); + await finishUser(line, pendingIdentifiers(identifiers), [...created.notes, ...retries]); + successful++; + processed++; + progress(); + }; + + progress(); + await Promise.all([ + ...users.map(async (user) => processUser(user)), + ...attachOnly.map(async (line) => finishUser(line, line.pending ?? [], [])), + ]); + + return { totalProcessed: total, successful, failed, validationFailed, errorBreakdown }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/instance.test.ts b/packages/cli-core/src/commands/migrate/lib/instance.test.ts new file mode 100644 index 000000000..5eac24b94 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/instance.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, test } from "bun:test"; +import { + DEV_USER_LIMIT, + resolveDevUserLimit, + detectInstanceType, + getDefaultConcurrencyLimit, + getDefaultRateLimit, + getRetryDelay, + resolveLimits, +} from "./instance.ts"; + +describe("detectInstanceType", () => { + test.each([ + ["sk_live_abc123", "prod"], + ["sk_test_abc123", "dev"], + ["sk_something_else", "dev"], + ["nonsense", "dev"], + ])("%s -> %s", (key, expected) => { + expect(detectInstanceType(key)).toBe(expected as "dev" | "prod"); + }); +}); + +describe("default limits", () => { + test.each([ + ["prod", 100], + ["dev", 10], + ])("%s instances get %i req/s", (instanceType, expected) => { + expect(getDefaultRateLimit(instanceType as "dev" | "prod")).toBe(expected); + }); + + test.each([ + [100, 9], + [10, 1], + [1, 1], + ])("a %i req/s limit yields %i concurrent calls", (rateLimit, expected) => { + expect(getDefaultConcurrencyLimit(rateLimit)).toBe(expected); + }); + + test("development instances default to 100 users", () => { + expect(DEV_USER_LIMIT).toBe(100); + }); +}); + +describe("resolveDevUserLimit", () => { + test.each([ + [undefined, 100], + ["500", 500], + ["250.7", 250], + ["0", 100], + ["-5", 100], + ["lots", 100], + ])("CLERK_MIGRATE_DEV_USER_LIMIT=%p gives %i", (value, expected) => { + expect(resolveDevUserLimit({ CLERK_MIGRATE_DEV_USER_LIMIT: value })).toBe(expected); + }); +}); + +describe("resolveLimits", () => { + test("derives both limits from the key when nothing is overridden", () => { + expect(resolveLimits("sk_live_x", {})).toEqual({ + instanceType: "prod", + rateLimit: 100, + concurrencyLimit: 9, + }); + }); + + test("honours environment overrides", () => { + expect( + resolveLimits("sk_test_x", { + CLERK_MIGRATE_RATE_LIMIT: "50", + CLERK_MIGRATE_CONCURRENCY_LIMIT: "4", + }), + ).toEqual({ instanceType: "dev", rateLimit: 50, concurrencyLimit: 4 }); + }); + + test("derives concurrency from an overridden rate limit", () => { + expect(resolveLimits("sk_test_x", { CLERK_MIGRATE_RATE_LIMIT: "200" }).concurrencyLimit).toBe( + 19, + ); + }); + + test.each([["0"], ["-5"], ["fast"], [""]])( + "ignores the unusable override %p in favour of the default", + (value) => { + expect(resolveLimits("sk_test_x", { CLERK_MIGRATE_RATE_LIMIT: value }).rateLimit).toBe(10); + }, + ); +}); + +describe("getRetryDelay", () => { + test.each([ + [undefined, 10_000, 10_000, 10], + [15, 10_000, 15_000, 15], + [1, 10_000, 1000, 1], + ])("Retry-After %p -> %i ms", (retryAfter, fallback, delayMs, delaySeconds) => { + expect(getRetryDelay(retryAfter, fallback)).toEqual({ delayMs, delaySeconds }); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/instance.ts b/packages/cli-core/src/commands/migrate/lib/instance.ts new file mode 100644 index 000000000..afd09c46e --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/instance.ts @@ -0,0 +1,111 @@ +/** + * Instance-type detection and the throughput limits that follow from it. + * + * Ported from the standalone migration-tool's `src/envs-constants.ts`, minus + * its dotenv/Zod env bootstrap: the secret key arrives from + * `resolveBapiSecretKey`, and only the two override knobs read the environment. + */ + +/** + * The user limit a development instance is created with. + * + * Only a default: Clerk raises it per instance on request, and the real value + * (`max_allowed_users`) is not served by BAPI, DAPI or FAPI — only by Clerk's + * internal staff API. The checks reject users past it, so an instance Clerk has + * raised overrides it with `CLERK_MIGRATE_DEV_USER_LIMIT` (see + * {@link resolveDevUserLimit}). Production instances have no limit at all. + */ +export const DEV_USER_LIMIT = 100; + +/** A positive number from the environment, or undefined. */ +function positive(value: string | undefined): number | undefined { + if (!value) return undefined; + const parsed = Number(value); + return Number.isFinite(parsed) && parsed > 0 ? parsed : undefined; +} + +/** + * The development-instance user limit for this run: `CLERK_MIGRATE_DEV_USER_LIMIT` + * when it is a positive number, otherwise {@link DEV_USER_LIMIT}. + */ +export function resolveDevUserLimit(env: Record = process.env): number { + const override = positive(env.CLERK_MIGRATE_DEV_USER_LIMIT); + return override ? Math.floor(override) : DEV_USER_LIMIT; +} + +/** How many times a 429 is retried before the user is recorded as failed. */ +export const MAX_RETRIES = 5; + +/** Fallback backoff when a 429 response carries no `Retry-After`. */ +export const RETRY_DELAY_MS = 10_000; + +export type InstanceType = "dev" | "prod"; + +/** + * Derives the instance type from the secret key's prefix. + * + * @example detectInstanceType("sk_live_xxx") // "prod" + * @example detectInstanceType("sk_test_xxx") // "dev" + */ +export function detectInstanceType(secretKey: string): InstanceType { + return secretKey.split("_")[1] === "live" ? "prod" : "dev"; +} + +/** + * Clerk's documented `POST /v1/users` rate limits, as requests per second: + * 1000 per 10s for production, 100 per 10s for development. + */ +export function getDefaultRateLimit(instanceType: InstanceType): number { + return instanceType === "prod" ? 100 : 10; +} + +/** + * Concurrency that saturates ~95% of the rate limit, assuming ~100ms of API + * latency per call: N concurrent requests at 100ms each yield N * 10 req/s. + * + * Override with `CLERK_MIGRATE_CONCURRENCY_LIMIT` when actual latency differs. + */ +export function getDefaultConcurrencyLimit(rateLimit: number): number { + return Math.max(1, Math.floor(rateLimit * 0.095)); +} + +export type ResolvedLimits = { + instanceType: InstanceType; + rateLimit: number; + concurrencyLimit: number; +}; + +/** + * Resolves throughput limits for a run: defaults from the detected instance + * type, each overridable by an environment variable. + * + * Non-numeric or non-positive overrides are ignored in favour of the default + * rather than failing the run — an unusable limit would stall the import. + */ +export function resolveLimits( + secretKey: string, + env: Record = process.env, +): ResolvedLimits { + const instanceType = detectInstanceType(secretKey); + + const rateLimit = positive(env.CLERK_MIGRATE_RATE_LIMIT) ?? getDefaultRateLimit(instanceType); + const concurrencyLimit = + positive(env.CLERK_MIGRATE_CONCURRENCY_LIMIT) ?? getDefaultConcurrencyLimit(rateLimit); + + return { instanceType, rateLimit, concurrencyLimit }; +} + +/** + * Backoff for a 429, preferring the server's `Retry-After` over the default. + * + * @param retryAfterSeconds - `Retry-After` value from the response, if present. + * @param defaultDelayMs - Fallback delay in milliseconds. + */ +export function getRetryDelay( + retryAfterSeconds: number | undefined, + defaultDelayMs: number, +): { delayMs: number; delaySeconds: number } { + const delayMs = retryAfterSeconds ? retryAfterSeconds * 1000 : defaultDelayMs; + const delaySeconds = retryAfterSeconds || defaultDelayMs / 1000; + return { delayMs, delaySeconds }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/progress.test.ts b/packages/cli-core/src/commands/migrate/lib/progress.test.ts new file mode 100644 index 000000000..182fb2899 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/progress.test.ts @@ -0,0 +1,108 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { getMode, setMode, type Mode } from "../../../mode.ts"; +import { useCaptureLog } from "../../../test/lib/stubs.ts"; +import { formatProgress, formatRemaining, withProgress } from "./progress.ts"; + +const captured = useCaptureLog(); + +describe("formatRemaining", () => { + test.each([ + [9_000, "~9s"], + [90_000, "~1m 30s"], + [900_000, "~15m"], + [500_000, "~8m 20s"], + [7_500_000, "~2h 5m"], + ])("%p ms -> %p", (ms, expected) => { + expect(formatRemaining(ms)).toBe(expected); + }); +}); + +describe("formatProgress", () => { + // 10,000 users at ~100 a second, 1% failing: the agreed mockup. + test("draws an 80-column bar with the report under it", () => { + const [bar, report] = formatProgress({ + total: 10_000, + verb: "created", + counts: { done: 7_500, ok: 7_425, failed: 75 }, + elapsedMs: 75_000, + }); + + expect(bar).toBe(`│ ${"█".repeat(54)}${"░".repeat(18)} 75%`); + expect([...bar]).toHaveLength(80); + expect(report).toBe("│ 7,500/10,000 users · ✓ 7,425 created · ✗ 75 failed · ~25s left"); + }); + + test("shrinks the bar to a narrower terminal", () => { + const [bar] = formatProgress({ + total: 100, + verb: "created", + counts: { done: 50, ok: 50, failed: 0 }, + elapsedMs: 0, + columns: 60, + }); + expect([...bar]).toHaveLength(60); + }); + + test.each([ + ["before there is a rate to go on", 1_000, 500], + ["once every user is done", 10_000, 1_000], + ])("gives no estimate %s", (_label, elapsedMs, done) => { + const [, report] = formatProgress({ + total: 1_000, + verb: "deleted", + counts: { done, ok: done, failed: 0 }, + elapsedMs, + }); + expect(report).not.toContain("left"); + }); + + // Floored, so 999 of 1,000 never reads as finished. + test("reads 100% only when every user is done", () => { + const [bar] = formatProgress({ + total: 1_000, + verb: "created", + counts: { done: 999, ok: 999, failed: 0 }, + elapsedMs: 0, + }); + expect(bar).toEndWith(" 99%"); + expect(bar).toContain("░"); + }); +}); + +describe("withProgress", () => { + let mode: Mode; + let isTTY: boolean | undefined; + + beforeEach(() => { + mode = getMode(); + isTTY = process.stderr.isTTY; + }); + + afterEach(() => { + setMode(mode); + process.stderr.isTTY = isTTY as boolean; + }); + + test("prints nothing for an agent", async () => { + setMode("agent"); + await withProgress({ total: 10, verb: "created" }, async (update) => { + update({ done: 10, ok: 10, failed: 0 }); + }); + expect(captured.err).toBe(""); + }); + + // A log file gets a line per 10%, not a redraw per user. + test("without a terminal, prints the report at each tenth", async () => { + setMode("human"); + process.stderr.isTTY = false; + + await withProgress({ total: 100, verb: "created" }, async (update) => { + for (let done = 1; done <= 100; done++) update({ done, ok: done, failed: 0 }); + }); + + const lines = captured.err.split("\n").filter((line) => line.includes("users")); + // 0% at the start, each tenth from 10% to 100%, and the final state. + expect(lines).toHaveLength(12); + expect(lines.at(-1)).toContain("100/100 users"); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/progress.ts b/packages/cli-core/src/commands/migrate/lib/progress.ts new file mode 100644 index 000000000..19ed123f0 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/progress.ts @@ -0,0 +1,131 @@ +/** + * The progress display for the import and undo loops: a bar on one line, the + * counts on the next. + * + * Used instead of a spinner on purpose. A clack spinner puts stdin in raw mode + * and calls `process.exit(0)` on Ctrl-C, so an interrupted import looked like + * a finished one and `clerk migrate import … && rm -rf ` carried on. + * This leaves stdin alone: Ctrl-C reaches the CLI's handler, in-flight + * requests abort, and the process exits 130 (`.claude/rules/interrupts.md`). + */ + +import { dim } from "../../../lib/color.ts"; +import { log } from "../../../lib/log.ts"; +import { isHuman } from "../../../mode.ts"; + +export type ProgressCounts = { done: number; ok: number; failed: number }; + +/** Receives the counts as each user finishes. */ +export type ProgressUpdate = (counts: ProgressCounts) => void; + +type ProgressLine = { + total: number; + /** What happened to the users counted in `ok`: "created", "deleted". */ + verb: string; + counts: ProgressCounts; + elapsedMs: number; + /** Terminal columns; the bar fills the line up to 80. */ + columns?: number; +}; + +const MAX_WIDTH = 80; +/** No estimate until there is this much of a rate to go on. */ +const ESTIMATE_AFTER_MS = 2000; +/** At most one redraw per this many ms: an import can report 100 users a second. */ +const REDRAW_MS = 100; + +const GUTTER = "│ "; +const number = (value: number) => value.toLocaleString("en-US"); + +/** `~9s`, `~1m 30s`, `~15m`, `~2h 5m`. */ +export function formatRemaining(ms: number): string { + const seconds = Math.max(1, Math.round(ms / 1000)); + if (seconds < 60) return `~${seconds}s`; + const minutes = Math.floor(seconds / 60); + if (minutes < 60) { + const rest = seconds % 60; + return rest ? `~${minutes}m ${rest}s` : `~${minutes}m`; + } + const hours = Math.floor(minutes / 60); + const rest = minutes % 60; + return rest ? `~${hours}h ${rest}m` : `~${hours}h`; +} + +/** The bar line and the report line, without color. */ +export function formatProgress(line: ProgressLine): [string, string] { + const { total, verb, counts, elapsedMs } = line; + const width = Math.min(MAX_WIDTH, line.columns ?? MAX_WIDTH); + const fraction = total > 0 ? Math.min(1, counts.done / total) : 1; + + // Floored, so the bar reads full only when every user is done. + const percent = `${Math.floor(fraction * 100)}%`.padStart(4); + const cells = Math.max(10, width - GUTTER.length - 1 - percent.length); + const filled = Math.floor(cells * fraction); + const bar = `${GUTTER}${"█".repeat(filled)}${"░".repeat(cells - filled)} ${percent}`; + + const parts = [ + `${number(counts.done)}/${number(total)} users`, + `✓ ${number(counts.ok)} ${verb}`, + `✗ ${number(counts.failed)} failed`, + ]; + if (elapsedMs >= ESTIMATE_AFTER_MS && counts.done > 0 && counts.done < total) { + parts.push(`${formatRemaining(((total - counts.done) * elapsedMs) / counts.done)} left`); + } + return [bar, `${GUTTER}${parts.join(" · ")}`]; +} + +/** + * Runs `fn` with a progress display for `total` users. + * + * At a terminal, the two lines redraw in place. When stderr is not a terminal + * (a log file, CI), the report line is printed at each 10% instead. An agent + * gets nothing, as it got nothing from the spinner. + */ +export async function withProgress( + options: { total: number; verb: string }, + fn: (update: ProgressUpdate) => Promise, +): Promise { + if (!isHuman()) return fn(() => {}); + + const tty = Boolean(process.stderr.isTTY); + const started = Date.now(); + let counts: ProgressCounts = { done: 0, ok: 0, failed: 0 }; + let drawn = false; + let lastDraw = 0; + let lastTenth = -1; + + const draw = (force: boolean) => { + const now = Date.now(); + const [bar, report] = formatProgress({ + ...options, + counts, + elapsedMs: now - started, + ...(process.stderr.columns ? { columns: process.stderr.columns } : {}), + }); + + if (!tty) { + const tenth = Math.floor((counts.done / Math.max(1, options.total)) * 10); + if (!force && tenth === lastTenth) return; + lastTenth = tenth; + log.ui(`${report}\n`); + return; + } + + if (!force && now - lastDraw < REDRAW_MS) return; + lastDraw = now; + // Up over the two lines drawn last time, then clear and rewrite each. + const up = drawn ? "\x1b[2A" : ""; + // The gutter in the gray clack draws it in. + const gutter = (line: string) => `${dim("│")}${line.slice(1)}`; + log.ui(`${up}\r\x1b[2K${gutter(bar)}\n\x1b[2K${gutter(report)}\n`); + drawn = true; + }; + + draw(true); + const result = await fn((next) => { + counts = next; + draw(false); + }); + draw(true); + return result; +} diff --git a/packages/cli-core/src/commands/migrate/lib/retry.test.ts b/packages/cli-core/src/commands/migrate/lib/retry.test.ts new file mode 100644 index 000000000..ed7f10091 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/retry.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, test } from "bun:test"; +import { BapiError, CliError } from "../../../lib/errors.ts"; +import { RateLimitExceededError, readRetryAfter, retryOn429 } from "./retry.ts"; + +const rateLimited = (headers: Record = {}) => + new BapiError( + 429, + JSON.stringify({ errors: [{ code: "e", message: "slow" }] }), + new Headers(headers), + ); + +const failed = (status: number) => + new BapiError( + status, + JSON.stringify({ errors: [{ code: "e", message: "nope" }] }), + new Headers(), + ); + +describe("readRetryAfter", () => { + test.each([ + ["12", 12], + ["0", undefined], + ["-1", undefined], + ["soon", undefined], + ])("Retry-After: %s -> %p", (header, expected) => { + expect(readRetryAfter(rateLimited({ "retry-after": header }))).toBe( + expected as number | undefined, + ); + }); + + test("falls back to the error body's retryAfter meta", () => { + const error = new BapiError( + 429, + JSON.stringify({ errors: [{ code: "e", message: "slow", meta: { retryAfter: 7 } }] }), + new Headers(), + ); + expect(readRetryAfter(error)).toBe(7); + }); + + test("prefers the header over the body", () => { + const error = new BapiError( + 429, + JSON.stringify({ errors: [{ code: "e", message: "slow", meta: { retryAfter: 7 } }] }), + new Headers({ "retry-after": "3" }), + ); + expect(readRetryAfter(error)).toBe(3); + }); + + test("returns undefined when neither carries a value", () => { + expect(readRetryAfter(rateLimited())).toBeUndefined(); + }); +}); + +describe("retryOn429", () => { + test("returns the value when the call succeeds first time", async () => { + expect(await retryOn429(async () => "ok")).toBe("ok"); + }); + + test("retries after a 429 and returns the eventual value", async () => { + let attempts = 0; + const result = await retryOn429( + async () => { + attempts++; + if (attempts === 1) throw rateLimited({ "retry-after": "1" }); + 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(); + + await retryOn429(async () => { + attempts++; + if (attempts === 1) throw rateLimited({ "retry-after": "1" }); + return "ok"; + }); + + expect(performance.now() - started).toBeGreaterThanOrEqual(900); + }); + + test("falls back to the default delay when no Retry-After is given", async () => { + let attempts = 0; + await retryOn429( + async () => { + attempts++; + if (attempts === 1) throw rateLimited(); + return "ok"; + }, + { defaultDelayMs: 5 }, + ); + expect(attempts).toBe(2); + }); + + test("reports each backoff to the caller so it can log against its own run", async () => { + const seen: { attempt: number; delaySeconds: number }[] = []; + let attempts = 0; + + await retryOn429( + async () => { + attempts++; + if (attempts <= 2) throw rateLimited(); + return "ok"; + }, + { + defaultDelayMs: 5, + onRetry: ({ attempt, delaySeconds }) => seen.push({ attempt, delaySeconds }), + }, + ); + + expect(seen.map((entry) => entry.attempt)).toEqual([1, 2]); + expect(seen[0]?.delaySeconds).toBe(0.005); + }); + + test("gives up after the ceiling, distinctly from an ordinary failure", async () => { + let attempts = 0; + + await expect( + retryOn429( + async () => { + attempts++; + throw rateLimited(); + }, + { maxRetries: 2, defaultDelayMs: 5 }, + ), + ).rejects.toThrow(RateLimitExceededError); + + // One initial attempt plus maxRetries retries. + expect(attempts).toBe(3); + }); + + // Only rate limiting is transient; retrying a 422 would just repeat it. + test.each([[400], [401], [404], [422], [500]])("lets a %i through untouched", async (status) => { + let attempts = 0; + + await expect( + retryOn429(async () => { + attempts++; + throw failed(status); + }), + ).rejects.toThrow(BapiError); + + expect(attempts).toBe(1); + }); + + test("lets a non-API error through untouched", async () => { + await expect(retryOn429(async () => Promise.reject(new CliError("boom")))).rejects.toThrow( + CliError, + ); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/retry.ts b/packages/cli-core/src/commands/migrate/lib/retry.ts new file mode 100644 index 000000000..add3ae203 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/retry.ts @@ -0,0 +1,66 @@ +/** + * Rate-limit backoff, shared by `migrate import` and `migrate undo`. + * + * Both walk the whole user set through BAPI and hit the same limits, so they + * back off identically rather than approximately: extracting this is what + * makes "deletion retries the same as import" true by construction. + */ + +import { BapiError } from "../../../lib/errors.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 { + const header = error.headers?.get("retry-after"); + if (header) { + const parsed = Number(header); + if (Number.isFinite(parsed) && parsed > 0) return parsed; + } + const meta = error.meta?.retryAfter; + return typeof meta === "number" && meta > 0 ? meta : undefined; +} + +/** Raised once a 429 has been retried {@link MAX_RETRIES} times. */ +export class RateLimitExceededError extends Error { + constructor(public readonly attempts: number) { + super(`Rate limit exceeded after ${attempts} retries`); + this.name = "RateLimitExceededError"; + } +} + +export type RetryOptions = { + /** Called before each backoff, so the caller can log it against its own run. */ + onRetry?: (info: { attempt: number; delaySeconds: number; message: string }) => void; + maxRetries?: number; + /** Backoff when the response carries no `Retry-After`. */ + defaultDelayMs?: number; +}; + +/** + * Runs `fn`, backing off and retrying whenever BAPI answers 429. + * + * Anything other than a 429 propagates untouched — only rate limiting is + * transient. Exhausting the retries raises {@link RateLimitExceededError} so + * the caller can record it distinctly from an ordinary API failure. + */ +export async function retryOn429(fn: () => Promise, options: RetryOptions = {}): Promise { + const maxRetries = options.maxRetries ?? MAX_RETRIES; + const defaultDelayMs = options.defaultDelayMs ?? RETRY_DELAY_MS; + + for (let attempt = 0; ; attempt++) { + try { + return await fn(); + } catch (error) { + if (!(error instanceof BapiError) || error.status !== 429) throw error; + if (attempt >= maxRetries) throw new RateLimitExceededError(maxRetries); + + const { delayMs, delaySeconds } = getRetryDelay(readRetryAfter(error), defaultDelayMs); + options.onRetry?.({ + attempt: attempt + 1, + delaySeconds, + message: `Rate limit hit (429), retrying in ${delaySeconds}s (attempt ${attempt + 1}/${maxRetries})`, + }); + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } +} diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts new file mode 100644 index 000000000..0616b6343 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts @@ -0,0 +1,249 @@ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + setSystemTime, + spyOn, + test, +} from "bun:test"; +import * as crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { _setConfigDir } from "../../../lib/config.ts"; +import { + continueRun, + latestUserLines, + listRuns, + newRunId, + readRun, + resolveRunsDir, + runState, + RUNS_DIR_ENV, + startRun, +} from "./run-store.ts"; + +let workDir: string; +let configDir: string; +let runsDir: string; +let originalEnv: string | undefined; + +beforeAll(() => { + originalEnv = process.env[RUNS_DIR_ENV]; + configDir = fs.mkdtempSync(path.join(os.tmpdir(), "clerk-run-store-config-")); + _setConfigDir(configDir); +}); + +afterAll(() => { + if (originalEnv === undefined) delete process.env[RUNS_DIR_ENV]; + else process.env[RUNS_DIR_ENV] = originalEnv; + _setConfigDir(undefined); + fs.rmSync(configDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + delete process.env[RUNS_DIR_ENV]; + workDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-run-store-"))); + runsDir = path.join(workDir, "runs"); +}); + +afterEach(() => { + fs.rmSync(workDir, { recursive: true, force: true }); +}); + +const init = { kind: "import" as const, target: { instanceId: "ins_1" }, source: "clerk" }; + +describe("resolveRunsDir", () => { + test("prefers --runs-dir, resolved against the cwd", async () => { + process.env[RUNS_DIR_ENV] = "/from/env"; + expect(await resolveRunsDir("flag", { cwd: workDir })).toBe(path.join(workDir, "flag")); + }); + + test("falls back to CLERK_MIGRATE_DIR", async () => { + process.env[RUNS_DIR_ENV] = "from-env"; + expect(await resolveRunsDir(undefined, { cwd: workDir })).toBe(path.join(workDir, "from-env")); + }); + + test("defaults to .clerk/migrate in the project, gitignored only when writing", async () => { + expect(await resolveRunsDir(undefined, { cwd: workDir })).toBe( + path.join(workDir, ".clerk", "migrate"), + ); + expect(fs.existsSync(path.join(workDir, ".gitignore"))).toBe(false); + + await resolveRunsDir(undefined, { cwd: workDir, write: true }); + expect(fs.readFileSync(path.join(workDir, ".gitignore"), "utf-8")).toBe(".clerk/\n"); + }); +}); + +describe("a run's life", () => { + test("IDs sort by start time and read as a date", () => { + expect(newRunId(new Date(2026, 8, 29, 14, 5, 2))).toMatch(/^20260929-140502-[0-9a-f]{4}$/); + }); + + test("starts running, holding a lock", () => { + const run = startRun(runsDir, init); + expect(readRun(runsDir, run.record.id)).toMatchObject({ status: "running", kind: "import" }); + expect(fs.readFileSync(path.join(run.dir, "lock"), "utf-8")).toBe(String(process.pid)); + }); + + test("finishes complete when every user made it", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + + const record = run.finish(); + + expect(record).toMatchObject({ status: "complete", counts: { total: 1, created: 1 } }); + expect(record.finishedAt).toBeDefined(); + expect(fs.existsSync(path.join(run.dir, "lock"))).toBe(false); + }); + + test.each([["failed"], ["skipped"], ["creating"]] as const)( + "finishes partial when a user was %s", + (status) => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created" }); + run.append({ sourceId: "b", status }); + expect(run.finish().status).toBe("partial"); + }, + ); + + test("counts each source ID by its last line", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "failed", error: "boom" }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + + expect(run.finish()).toMatchObject({ status: "complete", counts: { total: 1, created: 1 } }); + expect(latestUserLines(runsDir, run.record.id).get("a")?.clerkId).toBe("user_a"); + }); + + test("reads past a line a crash cut short", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created" }); + fs.appendFileSync(path.join(run.dir, "users.ndjson"), '{"sourceId":"b","sta'); + + expect([...latestUserLines(runsDir, run.record.id).keys()]).toEqual(["a"]); + }); + + test("a continued run starts a fresh line after one a crash cut short", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created" }); + fs.appendFileSync(path.join(run.dir, "users.ndjson"), '{"sourceId":"b","sta'); + fs.rmSync(path.join(run.dir, "lock")); + + continueRun(runsDir, run.record).append({ sourceId: "c", status: "created" }); + + expect([...latestUserLines(runsDir, run.record.id).keys()]).toEqual(["a", "c"]); + }); + + // A user created with no line is beyond both undo and a re-run, so the + // create that would follow must not go out. + test("append throws when the line cannot be written", () => { + const run = startRun(runsDir, init); + fs.mkdirSync(path.join(run.dir, "users.ndjson")); + + expect(() => run.append({ sourceId: "a", status: "creating" })).toThrow(); + }); +}); + +describe("locks and interruptions", () => { + // A PID no process can have. + const DEAD_PID = "2147483646"; + + test("a run whose process died is interrupted", () => { + const run = startRun(runsDir, init); + fs.writeFileSync(path.join(run.dir, "lock"), DEAD_PID); + expect(runState(runsDir, run.record)).toBe("interrupted"); + }); + + test("a run with no lock and no finish time is interrupted", () => { + const run = startRun(runsDir, init); + fs.rmSync(path.join(run.dir, "lock")); + expect(runState(runsDir, run.record)).toBe("interrupted"); + }); + + // Undo marks an interrupted import undone, but leaves it with no finish time. + test("an interrupted run that was undone reads as undone", () => { + const run = startRun(runsDir, init); + fs.rmSync(path.join(run.dir, "lock")); + expect(runState(runsDir, { ...run.record, status: "undone" })).toBe("undone"); + }); + + test("continuing takes the lock from a dead process and reopens the run", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created" }); + run.finish(); + + const again = continueRun(runsDir, readRun(runsDir, run.record.id)!); + expect(again.record).toMatchObject({ id: run.record.id, status: "running" }); + expect(again.record.finishedAt).toBeUndefined(); + again.append({ sourceId: "b", status: "created" }); + expect(again.finish().counts.total).toBe(2); + }); + + // In a container the CLI often gets the same PID every run, so a killed + // run's lock can hold this process's own PID. + test("a lock holding this process's own PID is stale", () => { + const run = startRun(runsDir, init); + expect(runState(runsDir, run.record)).toBe("interrupted"); + expect(() => continueRun(runsDir, run.record)).not.toThrow(); + }); + + test("reads as running while another live process holds the lock", () => { + const run = startRun(runsDir, init); + fs.writeFileSync(path.join(run.dir, "lock"), "1"); + expect(runState(runsDir, run.record)).toBe("running"); + }); + + // Two runs in the same second share an ID one time in 65,536. + test("a run ID that is already taken gets a new one, not a shared folder", () => { + setSystemTime(new Date("2026-10-02T12:00:00")); + const bytes = spyOn(crypto, "randomBytes") + .mockReturnValueOnce(Buffer.from([0xab, 0xcd]) as never) + .mockReturnValueOnce(Buffer.from([0xab, 0xcd]) as never) + .mockReturnValueOnce(Buffer.from([0x12, 0x34]) as never); + try { + const first = startRun(runsDir, init); + const second = startRun(runsDir, init); + expect(first.record.id).toEndWith("-abcd"); + expect(second.record.id).toEndWith("-1234"); + } finally { + bytes.mockRestore(); + setSystemTime(); + } + }); + + test("refuses a run another live process holds, with exit 2", () => { + const run = startRun(runsDir, init); + // PID 1 is always alive, and never this test. + fs.writeFileSync(path.join(run.dir, "lock"), "1"); + + expect(() => continueRun(runsDir, run.record)).toThrow( + new RegExp( + `in use by another process \\(PID 1\\).*delete ${path.join(run.dir, "lock")}`, + "s", + ), + ); + }); +}); + +describe("listRuns", () => { + test("lists newest first and ignores folders that are not runs", () => { + const first = startRun(runsDir, init); + first.update({ startedAt: "2026-01-01T00:00:00.000Z" }); + const second = startRun(runsDir, { ...init, kind: "export" }); + second.update({ startedAt: "2026-02-01T00:00:00.000Z" }); + fs.mkdirSync(path.join(runsDir, "not-a-run")); + + expect(listRuns(runsDir).map((record) => record.id)).toEqual([ + second.record.id, + first.record.id, + ]); + }); + + test("is empty when the folder does not exist", () => { + expect(listRuns(path.join(workDir, "nowhere"))).toEqual([]); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.ts b/packages/cli-core/src/commands/migrate/lib/run-store.ts new file mode 100644 index 000000000..6ac2f2bf3 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/run-store.ts @@ -0,0 +1,454 @@ +/** + * The run store: the one place `clerk migrate` keeps state. + * + * Every import, export and undo is a run. A run is a folder under the runs + * directory holding: + * + * - `run.json` — what ran, against what, from which file, and how it ended. + * - `users.ndjson` — one line per user outcome. The last line for a source ID + * wins, so a continued run appends rather than rewrites. + * - `lock` — the PID of the process working on the run. A live PID refuses a + * second writer; a dead one, or a run with no `finishedAt`, means the run + * was interrupted. + * + * `runs`, `undo`, re-runs and exports all read or write it, so there is no + * second record to drift out of step. + * + * Appends are synchronous so a run interrupted with Ctrl-C still leaves a + * complete record of everything already processed. + */ + +import { createHash, randomBytes } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; +import { resolveProfile } from "../../../lib/config.ts"; +import { throwUsageError } from "../../../lib/errors.ts"; +import { ensureGitignoreEntry, getGitRepoRoot } from "../../../lib/git.ts"; +import { log } from "../../../lib/log.ts"; + +/** Overrides the runs directory, below `--runs-dir`. */ +export const RUNS_DIR_ENV = "CLERK_MIGRATE_DIR"; + +/** The `--runs-dir` option every migrate subcommand takes. */ +export const RUNS_DIR_FLAG = "--runs-dir "; +export const RUNS_DIR_DESCRIPTION = `Where migration runs are kept (default: .clerk/migrate in the project, or ${RUNS_DIR_ENV})`; + +export type RunKind = "import" | "undo" | "export"; +export type RunStatus = "running" | "complete" | "partial" | "undone"; +/** + * `creating` is written as `POST /v1/users` goes out, and stays the latest line + * when no answer says whether the create landed (an abort, a network error, a + * 5xx). The user may then exist in Clerk without its ID on record, so `undo` + * and a continued run look it up by `external_id`. + */ +export type UserStatus = "creating" | "created" | "failed" | "skipped" | "deleted" | "exported"; + +/** + * What a run acted on. For an import or undo, the Clerk instance. For an + * export, the source platform, plus the Clerk instance for `export clerk`. + */ +export type RunTarget = { + env?: string; + appId?: string; + appLabel?: string; + instanceId?: string; + instanceType?: string; + keySource?: string; + /** The platform an export read from. */ + platform?: string; +}; + +export type RunFile = { path: string; sha256: string }; + +export type RunCounts = Partial> & { total: number }; + +export type RunRecord = { + id: string; + kind: RunKind; + status: RunStatus; + startedAt: string; + finishedAt?: string; + target: RunTarget; + source?: string; + /** Content hash of a custom source, so an edited one is a different source. */ + sourceHash?: string; + file?: RunFile; + /** The export run an import read its file from. */ + fromExport?: string; + /** The import run an undo reverses. */ + undoes?: string; + /** The undo run that reversed this one. */ + undoneBy?: string; + counts: RunCounts; +}; + +/** An extra email or phone still to attach to a created user. */ +export type PendingIdentifier = { kind: "email" | "phone"; value: string; verified: boolean }; + +export type UserLine = { + sourceId: string; + clerkId?: string; + status: UserStatus; + /** + * On a `created` line: the extra identifiers not yet attached. A continued + * run attaches them; a later `created` line without it means they are done. + */ + pending?: PendingIdentifier[]; + /** Why a user was skipped. */ + reason?: string; + error?: string; + code?: string; + passwordDropped?: boolean; +}; + +/** What `runs` shows for a run: its stored status, or how it stopped. */ +export type RunState = RunStatus | "interrupted"; + +const RUN_FILE = "run.json"; +const USERS_FILE = "users.ndjson"; +const LOCK_FILE = "lock"; + +// --- Location -------------------------------------------------------------- + +/** + * Where the project lives, for the default runs directory. + * + * A profile linked by directory names it outright. One linked by git remote + * or repository names the repository, whose root is the git toplevel. An + * unlinked directory outside git is its own project. + */ +async function projectRoot(cwd: string): Promise { + const profile = await resolveProfile(cwd); + if (profile?.resolvedVia === "directory") return profile.path; + return (await getGitRepoRoot(cwd)) ?? cwd; +} + +/** + * The runs directory: `--runs-dir`, then `CLERK_MIGRATE_DIR`, then + * `/.clerk/migrate`. + * + * @param options.write - The caller is about to write a run. The default + * location is gitignored first, because run files carry user data. + */ +export async function resolveRunsDir( + runsDir: string | undefined, + options: { write?: boolean; cwd?: string } = {}, +): Promise { + const cwd = options.cwd ?? process.cwd(); + if (runsDir) return path.resolve(cwd, runsDir); + + const fromEnv = process.env[RUNS_DIR_ENV]; + if (fromEnv) return path.resolve(cwd, fromEnv); + + const root = await projectRoot(cwd); + if (options.write) await ensureGitignoreEntry(root, ".clerk/"); + return path.join(root, ".clerk", "migrate"); +} + +/** The shape of a run ID, so one can be told apart from a file path. */ +export const RUN_ID_PATTERN = /^\d{8}-\d{6}-[0-9a-f]{4}$/; + +/** The folder one run lives in. */ +export function runDir(runsDir: string, id: string): string { + return path.join(runsDir, id); +} + +// --- IDs and hashes -------------------------------------------------------- + +/** + * `YYYYMMDD-HHmmss-xxxx`, local time. + * + * Sorts by start time, reads as a date, and the random suffix keeps two runs + * started in the same second apart. + */ +export function newRunId(now: Date = new Date()): string { + const pad = (value: number) => String(value).padStart(2, "0"); + const date = `${now.getFullYear()}${pad(now.getMonth() + 1)}${pad(now.getDate())}`; + const time = `${pad(now.getHours())}${pad(now.getMinutes())}${pad(now.getSeconds())}`; + return `${date}-${time}-${randomBytes(2).toString("hex")}`; +} + +/** Hex sha256 of a file's bytes. */ +export function sha256File(file: string): string { + return createHash("sha256").update(fs.readFileSync(file)).digest("hex"); +} + +// --- Locking --------------------------------------------------------------- + +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + // EPERM: the process exists but belongs to someone else. + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +/** + * The PID holding the run's lock, when that process is still alive. + * + * This process's own PID counts as stale: in a container the CLI often gets + * the same PID every run, so a killed run's lock would otherwise read as live. + */ +export function liveLockPid(runsDir: string, id: string): number | undefined { + let raw: string; + try { + raw = fs.readFileSync(lockFile(runsDir, id), "utf-8"); + } catch { + return undefined; + } + const pid = Number(raw.trim()); + return Number.isInteger(pid) && pid > 0 && pid !== process.pid && isPidAlive(pid) + ? pid + : undefined; +} + +/** The run's lock file. */ +export function lockFile(runsDir: string, id: string): string { + return path.join(runDir(runsDir, id), LOCK_FILE); +} + +const isExists = (error: unknown) => (error as NodeJS.ErrnoException).code === "EEXIST"; + +/** + * Takes the run's lock. Created exclusively (`wx`), so two processes can't + * both read "free" and both write; a stale lock is removed and taken once. + */ +function acquireLock(runsDir: string, id: string): void { + const file = lockFile(runsDir, id); + const refuse = (holder: number | undefined): never => + throwUsageError( + `Run ${id} is in use by another process${holder ? ` (PID ${holder})` : ""}. Wait for it to finish, then try again. ` + + `If that process is not a migrate run, delete ${file}.`, + ); + const take = () => fs.writeFileSync(file, String(process.pid), { flag: "wx" }); + + try { + take(); + return; + } catch (error) { + if (!isExists(error)) throw error; + } + const holder = liveLockPid(runsDir, id); + if (holder !== undefined) refuse(holder); + fs.rmSync(file, { force: true }); + try { + take(); + } catch (error) { + // Another process took the stale lock between the remove and the write. + if (isExists(error)) refuse(liveLockPid(runsDir, id)); + throw error; + } +} + +// --- Reading --------------------------------------------------------------- + +/** One run's record, or `undefined` when the folder holds no readable run. */ +export function readRun(runsDir: string, id: string): RunRecord | undefined { + try { + const parsed = JSON.parse( + fs.readFileSync(path.join(runDir(runsDir, id), RUN_FILE), "utf-8"), + ) as RunRecord; + return parsed && typeof parsed === "object" && parsed.id === id ? parsed : undefined; + } catch { + return undefined; + } +} + +/** Every line of a run's `users.ndjson`, in the order written. */ +export function readUserLines(runsDir: string, id: string): UserLine[] { + let raw: string; + try { + raw = fs.readFileSync(path.join(runDir(runsDir, id), USERS_FILE), "utf-8"); + } catch { + return []; + } + const lines: UserLine[] = []; + for (const line of raw.split("\n")) { + if (!line.trim()) continue; + try { + lines.push(JSON.parse(line) as UserLine); + } catch { + // A line cut short by a crash mid-write. Every earlier line still counts. + } + } + return lines; +} + +/** Each source ID's latest outcome, in first-seen order. */ +export function latestUserLines(runsDir: string, id: string): Map { + const latest = new Map(); + for (const line of readUserLines(runsDir, id)) latest.set(line.sourceId, line); + return latest; +} + +/** + * How a run stands now. A run that never finished and whose process is gone + * was interrupted, whatever its stored status says. + */ +export function runState(runsDir: string, record: RunRecord): RunState { + // An interrupted run that was then undone has no finish time of its own. + if (record.finishedAt || record.status === "undone") return record.status; + return liveLockPid(runsDir, record.id) === undefined ? "interrupted" : "running"; +} + +/** + * Clerk IDs that import runs other than `exceptId` record as created. + * + * A user found by `external_id` may belong to another run of the same source + * IDs, so lookups for in-flight creates leave these out. + */ +export function clerkIdsCreatedByOtherRuns(runsDir: string, exceptId: string): Set { + const ids = new Set(); + for (const record of listRuns(runsDir)) { + if (record.kind !== "import" || record.id === exceptId) continue; + for (const line of latestUserLines(runsDir, record.id).values()) { + if (line.status === "created" && line.clerkId) ids.add(line.clerkId); + } + } + return ids; +} + +/** Every readable run, newest first. */ +export function listRuns(runsDir: string): RunRecord[] { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(runsDir, { withFileTypes: true }); + } catch { + return []; + } + return entries + .filter((entry) => entry.isDirectory()) + .map((entry) => readRun(runsDir, entry.name)) + .filter((record): record is RunRecord => record !== undefined) + .sort((a, b) => b.startedAt.localeCompare(a.startedAt) || b.id.localeCompare(a.id)); +} + +// --- Writing --------------------------------------------------------------- + +function writeRecord(runsDir: string, record: RunRecord): void { + const file = path.join(runDir(runsDir, record.id), RUN_FILE); + // Written whole and renamed into place, so a reader never sees half a file. + const temp = `${file}.tmp`; + fs.writeFileSync(temp, `${JSON.stringify(record, null, 2)}\n`); + fs.renameSync(temp, file); +} + +/** Totals by each source ID's latest status. */ +export function countLines(latest: Iterable): RunCounts { + const counts: RunCounts = { total: 0 }; + for (const line of latest) { + counts.total++; + counts[line.status] = (counts[line.status] ?? 0) + 1; + } + return counts; +} + +/** A run being written by this process. */ +export type Run = { + readonly runsDir: string; + readonly dir: string; + record: RunRecord; + /** Appends one user outcome. */ + append(line: UserLine): void; + /** Merges fields into `run.json`. */ + update(patch: Partial): void; + /** + * Counts the outcomes, settles the status and releases the lock. + * + * `partial` when any user failed, was skipped or may not have been created, + * `complete` otherwise. + */ + finish(): RunRecord; +}; + +function openRun(runsDir: string, record: RunRecord): Run { + const dir = runDir(runsDir, record.id); + const usersFile = path.join(dir, USERS_FILE); + + const run: Run = { + runsDir, + dir, + record, + // Throws when the line cannot be written: a user created with no record is + // beyond both `undo` and a re-run, so the create it precedes must not go out. + append(line) { + fs.appendFileSync(usersFile, `${JSON.stringify(line)}\n`); + }, + update(patch) { + run.record = { ...run.record, ...patch }; + writeRecord(runsDir, run.record); + }, + finish() { + const counts = countLines(latestUserLines(runsDir, run.record.id).values()); + const unfinished = (counts.failed ?? 0) + (counts.skipped ?? 0) + (counts.creating ?? 0); + run.update({ + counts, + status: unfinished > 0 ? "partial" : "complete", + finishedAt: new Date().toISOString(), + }); + fs.rmSync(path.join(dir, LOCK_FILE), { force: true }); + return run.record; + }, + }; + return run; +} + +export type StartRunInit = Omit; + +/** Creates a run folder, takes its lock and writes the first `run.json`. */ +export function startRun(runsDir: string, init: StartRunInit): Run { + // Owner-only: a run's files hold user data. + fs.mkdirSync(runsDir, { recursive: true, mode: 0o700 }); + // Created exclusively: two runs started in the same second share an ID one + // time in 65,536, and must not share a folder. + let id = newRunId(); + for (;;) { + try { + fs.mkdirSync(runDir(runsDir, id), { mode: 0o700 }); + break; + } catch (error) { + if (!isExists(error)) throw error; + id = newRunId(); + } + } + acquireLock(runsDir, id); + + const record: RunRecord = { + id, + status: "running", + startedAt: new Date().toISOString(), + counts: { total: 0 }, + ...init, + }; + writeRecord(runsDir, record); + log.debug(`migrate: started ${init.kind} run ${id} in ${runsDir}`); + return openRun(runsDir, record); +} + +/** + * Reopens a finished or interrupted run to keep working on it. + * + * @throws UsageError when another live process holds its lock. + */ +export function continueRun(runsDir: string, record: RunRecord): Run { + acquireLock(runsDir, record.id); + // A crash mid-write leaves a torn last line; end it so the next append + // starts a line of its own instead of fusing with it. + const usersFile = path.join(runDir(runsDir, record.id), USERS_FILE); + const written = fs.existsSync(usersFile) ? fs.readFileSync(usersFile, "utf-8") : ""; + if (written && !written.endsWith("\n")) fs.appendFileSync(usersFile, "\n"); + const run = openRun(runsDir, record); + run.record = { ...record, status: "running" }; + delete run.record.finishedAt; + writeRecord(runsDir, run.record); + log.debug(`migrate: continuing ${record.kind} run ${record.id} in ${runsDir}`); + return run; +} + +/** Merges fields into a run this process is not writing, such as `undoneBy`. */ +export function patchRun(runsDir: string, id: string, patch: Partial): void { + const record = readRun(runsDir, id); + if (record) writeRecord(runsDir, { ...record, ...patch }); +} diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts new file mode 100644 index 000000000..68c44e50b --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -0,0 +1,69 @@ +import { expect, test } from "bun:test"; +import { createApiScheduler } from "./scheduler.ts"; + +/** Resolves after `ms`, so a task can be held open while others queue behind it. */ +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +test("never runs more tasks at once than the concurrency limit", async () => { + const schedule = createApiScheduler(3, 10_000); + let active = 0; + let peak = 0; + + await Promise.all( + Array.from({ length: 20 }, () => + schedule(async () => { + active++; + peak = Math.max(peak, active); + await wait(5); + active--; + }), + ), + ); + + expect(peak).toBe(3); + expect(active).toBe(0); +}); + +test("frees a slot when a task throws, instead of deadlocking the queue", async () => { + const schedule = createApiScheduler(1, 10_000); + + await expect(schedule(() => Promise.reject(new Error("boom")))).rejects.toThrow("boom"); + + // If release() had been skipped on the failure path, this would hang. + expect(await schedule(async () => "ok")).toBe("ok"); +}); + +test("paces calls to the rate limit", async () => { + // 100 req/s -> 10ms between starts; 5 calls span at least 4 intervals. + const schedule = createApiScheduler(5, 100); + const started = performance.now(); + + await Promise.all(Array.from({ length: 5 }, () => schedule(async () => {}))); + + expect(performance.now() - started).toBeGreaterThanOrEqual(35); +}); + +test("returns each task's own resolved value", async () => { + const schedule = createApiScheduler(2, 10_000); + const results = await Promise.all([1, 2, 3].map((n) => schedule(async () => n * 2))); + expect(results).toEqual([2, 4, 6]); +}); + +test("treats zero or negative limits as one", async () => { + const schedule = createApiScheduler(0, 10_000); + let active = 0; + let peak = 0; + + await Promise.all( + Array.from({ length: 4 }, () => + schedule(async () => { + active++; + peak = Math.max(peak, active); + await wait(2); + active--; + }), + ), + ); + + expect(peak).toBe(1); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.ts new file mode 100644 index 000000000..0971bf98e --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.ts @@ -0,0 +1,62 @@ +/** + * Concurrency gate plus rate pacing for BAPI calls. + * + * Replaces the standalone migration-tool's `p-limit` dependency: a bounded + * queue is a few lines, and the compiled binary carries one fewer package. + * + * Both limits apply to individual API calls rather than whole users, so a user + * with ten extra email addresses cannot burst past the instance's rate limit. + */ + +/** + * Runs `fn` once a slot is free and the pacing interval has elapsed. + * + * `first` puts `fn` ahead of every queued call without it: a user's extra + * identifiers attach before the next user is created, so a run stopped midway + * leaves few users waiting on attaches. + */ +export type ApiScheduler = (fn: () => Promise, options?: { first?: boolean }) => Promise; + +export function createApiScheduler(concurrencyLimit: number, rateLimit: number): ApiScheduler { + const maxConcurrent = Math.max(1, Math.floor(concurrencyLimit)); + const intervalMs = Math.ceil(1000 / Math.max(1, rateLimit)); + const waiting: (() => void)[] = []; + const waitingFirst: (() => void)[] = []; + let active = 0; + let nextRequestAt = 0; + + async function acquire(first: boolean): Promise { + if (active < maxConcurrent) { + active++; + return Promise.resolve(); + } + return new Promise((resolve) => (first ? waitingFirst : waiting).push(resolve)); + } + + function release(): void { + // One macrotask later, so the finished call's follow-up (a user's attaches) + // is queued before the slot is handed on. + setImmediate(() => { + const next = waitingFirst.shift() ?? waiting.shift(); + // Hand the slot straight to the next waiter; `active` is unchanged + // because the slot never actually frees up. + if (next) next(); + else active--; + }); + } + + return async (fn, options) => { + await acquire(options?.first ?? false); + try { + const now = Date.now(); + const waitMs = Math.max(0, nextRequestAt - now); + nextRequestAt = Math.max(now, nextRequestAt) + intervalMs; + if (waitMs > 0) { + await new Promise((resolve) => setTimeout(resolve, waitMs)); + } + return await fn(); + } finally { + release(); + } + }; +} diff --git a/packages/cli-core/src/commands/migrate/lib/target.test.ts b/packages/cli-core/src/commands/migrate/lib/target.test.ts new file mode 100644 index 000000000..ac192ca7e --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/target.test.ts @@ -0,0 +1,155 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { EXIT_CODE, type CliError } from "../../../lib/errors.ts"; +import { useCaptureLog } from "../../../test/lib/stubs.ts"; +import { + describeTarget, + fetchInstanceIdentity, + printTarget, + resolveClerkTarget, +} from "./target.ts"; + +const captured = useCaptureLog(); + +describe("printTarget", () => { + test.each([ + [ + "a linked app", + { + env: "production", + appId: "app_1", + appLabel: "My App", + instanceId: "ins_1", + keySource: "linked profile", + }, + "Target: My App (app_1), production instance ins_1", + "Key from: linked profile", + ], + [ + "a bare --secret-key", + { env: "development", instanceId: "ins_2", keySource: "--secret-key" }, + "Target: development instance ins_2", + "Key from: --secret-key", + ], + [ + "an exported CLERK_SECRET_KEY", + { env: "development", instanceId: "ins_3", keySource: "CLERK_SECRET_KEY env var" }, + "Target: development instance ins_3", + "Key from: CLERK_SECRET_KEY env var", + ], + ])("names %s", (_label, target, first, second) => { + printTarget(target); + const lines = Bun.stripANSI(captured.err).split("\n"); + expect(lines).toEqual([first, second]); + }); + + test("names an export's source platform", () => { + printTarget({ platform: "auth0" }); + expect(captured.err).toBe("Source: auth0"); + }); + + test("names the Clerk instance a Clerk export reads", () => { + printTarget({ platform: "clerk", env: "production", instanceId: "ins_9" }); + expect(Bun.stripANSI(captured.err)).toBe("Source: Clerk, production instance ins_9"); + }); +}); + +describe("describeTarget", () => { + test("is one line for a list", () => { + expect(describeTarget({ appLabel: "My App", env: "development", instanceId: "ins_1" })).toBe( + "My App (development, ins_1)", + ); + expect(describeTarget({ platform: "auth0" })).toBe("auth0"); + }); +}); + +describe("fetchInstanceIdentity", () => { + let originalFetch: typeof globalThis.fetch; + + beforeAll(() => { + originalFetch = globalThis.fetch; + }); + + afterAll(() => { + globalThis.fetch = originalFetch; + }); + + test("reads the instance behind the key", async () => { + globalThis.fetch = (async () => + Response.json({ + object: "instance", + id: "ins_7", + environment_type: "production", + })) as unknown as typeof fetch; + + expect(await fetchInstanceIdentity("sk_live_x")).toEqual({ + instanceId: "ins_7", + env: "production", + }); + }); + + test("retries a 429 rather than falling back", async () => { + let calls = 0; + globalThis.fetch = (async () => + ++calls === 1 + ? new Response("{}", { status: 429, headers: { "retry-after": "1" } }) + : Response.json({ + id: "ins_7", + environment_type: "development", + })) as unknown as typeof fetch; + + expect((await fetchInstanceIdentity("sk_test_x")).instanceId).toBe("ins_7"); + expect(calls).toBe(2); + }); + + // The same key always addresses the same instance, so a hash still tells two + // instances apart when the API cannot say. + test("falls back to a stable hash of the key when the instance cannot be read", async () => { + globalThis.fetch = (async () => + new Response("nope", { status: 500 })) as unknown as typeof fetch; + + const first = await fetchInstanceIdentity("sk_test_a"); + expect(first.instanceId).toMatch(/^key_[0-9a-f]{16}$/); + expect(first.env).toBe("development"); + expect(await fetchInstanceIdentity("sk_test_a")).toEqual(first); + expect((await fetchInstanceIdentity("sk_test_b")).instanceId).not.toBe(first.instanceId); + }); +}); + +describe("resolveClerkTarget --instance", () => { + let originalFetch: typeof globalThis.fetch; + let originalKey: string | undefined; + + beforeAll(() => { + originalFetch = globalThis.fetch; + originalKey = process.env.CLERK_SECRET_KEY; + globalThis.fetch = (async () => + Response.json({ id: "ins_prod", environment_type: "production" })) as unknown as typeof fetch; + }); + + afterAll(() => { + globalThis.fetch = originalFetch; + if (originalKey === undefined) delete process.env.CLERK_SECRET_KEY; + else process.env.CLERK_SECRET_KEY = originalKey; + }); + + // An exported production key would otherwise win over `--instance dev` + // without a word, and the import would write to production. + test.each([ + ["an exported key", { instance: "dev" }, "sk_live_x"], + ["--secret-key", { instance: "dev", secretKey: "sk_live_x" }, undefined], + ["--secret-key, with a literal ID", { instance: "ins_dev", secretKey: "sk_live_x" }, undefined], + ])("refuses %s that addresses another instance", async (_label, options, envKey) => { + if (envKey) process.env.CLERK_SECRET_KEY = envKey; + else delete process.env.CLERK_SECRET_KEY; + + const error = (await resolveClerkTarget(options).catch((e: unknown) => e)) as CliError; + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain("does not match the key"); + }); + + test.each([["prod"], ["production"], ["ins_prod"]])("accepts --instance %s", async (instance) => { + delete process.env.CLERK_SECRET_KEY; + const { target } = await resolveClerkTarget({ instance, secretKey: "sk_live_x" }); + expect(target.instanceId).toBe("ins_prod"); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts new file mode 100644 index 000000000..998a60276 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -0,0 +1,177 @@ +/** + * The Clerk instance a migrate command acts on, and where its key came from. + * + * `resolveBapiSecretKey` picks the key. This names what that key points at: + * the instance ID and environment come from `GET /v1/instance`, so every key + * source yields the same identity. That identity is what the run store keys + * resumes and undos on, so an app label or an `--instance` alias is never + * enough on its own. + */ + +import { createHash } from "node:crypto"; +import { bapiRequest } from "../../../lib/bapi.ts"; +import { dim } from "../../../lib/color.ts"; +import { resolveBapiSecretKey } from "../../../lib/bapi-command.ts"; +import { INSTANCE_ALIASES, resolveAppContext } from "../../../lib/config.ts"; +import { throwUsageError } from "../../../lib/errors.ts"; +import { resolveKeylessTarget } from "../../../lib/keyless-target.ts"; +import { log } from "../../../lib/log.ts"; +import { detectInstanceType } from "./instance.ts"; +import { retryOn429 } from "./retry.ts"; +import type { RunTarget } from "./run-store.ts"; + +export type TargetOptions = { + secretKey?: string; + app?: string; + instance?: string; +}; + +/** A resolved Clerk instance. Every field a run needs is present. */ +export type ClerkTarget = RunTarget & { + env: string; + instanceId: string; + instanceType: "dev" | "prod"; + keySource: string; +}; + +/** + * Which rung of `resolveBapiSecretKey`'s chain supplies the key, and the app + * it belongs to when that rung knows one. + * + * Mirrors the chain's order exactly. An exported `CLERK_SECRET_KEY` wins over + * a linked profile, so the profile's app is not named for it: that key may + * belong to any app at all. + */ +async function describeKeySource( + options: TargetOptions, +): Promise<{ keySource: string; appId?: string; appLabel?: string }> { + if (options.secretKey) return { keySource: "--secret-key" }; + + if (options.app) { + const ctx = await resolveAppContext({ app: options.app, instance: options.instance }); + return { keySource: "--app", appId: ctx.appId, appLabel: ctx.appLabel }; + } + + if (process.env.CLERK_SECRET_KEY) return { keySource: "CLERK_SECRET_KEY env var" }; + + const keyless = await resolveKeylessTarget({ instance: options.instance }); + if (keyless) { + return { keySource: `accountless app (${keyless.source})`, appLabel: "accountless app" }; + } + + const ctx = await resolveAppContext({ instance: options.instance }); + return { keySource: "linked profile", appId: ctx.appId, appLabel: ctx.appLabel }; +} + +/** + * The instance behind a key. + * + * Falls back to a hash of the key when `GET /v1/instance` cannot be read: the + * same key always addresses the same instance, so it still tells two + * instances apart, and nothing is sent anywhere. A 429 is retried first: it is + * most likely straight after a large import, which is when `undo` runs. + */ +export async function fetchInstanceIdentity( + secretKey: string, +): Promise<{ instanceId: string; env: string }> { + const fallbackEnv = detectInstanceType(secretKey) === "prod" ? "production" : "development"; + try { + const { body } = await retryOn429(async () => + bapiRequest({ method: "GET", path: "/v1/instance", secretKey }), + ); + const instance = body as { id?: unknown; environment_type?: unknown }; + if (typeof instance.id === "string" && instance.id.startsWith("ins_")) { + return { + instanceId: instance.id, + env: + typeof instance.environment_type === "string" ? instance.environment_type : fallbackEnv, + }; + } + } catch (error) { + log.debug(`migrate: could not read the instance behind the key: ${String(error)}`); + } + return { instanceId: keyInstanceId(secretKey), env: fallbackEnv }; +} + +/** + * The stand-in ID for a key whose instance Clerk did not name: a hash of the + * key. A run recorded under it is matched by the same key later, once + * `GET /v1/instance` answers again. + */ +export function keyInstanceId(secretKey: string): string { + return `key_${createHash("sha256").update(secretKey).digest("hex").slice(0, 16)}`; +} + +/** + * Refuses an `--instance` the key does not address. + * + * With a key from `--secret-key` or `CLERK_SECRET_KEY`, the key alone picks + * the instance and `--instance` would be ignored without a word. Migrate + * writes and deletes in bulk, so `--instance dev` next to an exported + * `sk_live_` key must not reach production. + */ +function assertInstanceFlagMatches( + options: TargetOptions, + keySource: string, + identity: { instanceId: string; env: string }, +): void { + const flag = options.instance; + if (!flag || (keySource !== "--secret-key" && !keySource.startsWith("CLERK_SECRET_KEY"))) return; + const wanted = INSTANCE_ALIASES[flag]; + const matches = wanted ? identity.env === wanted : identity.instanceId === flag; + if (matches) return; + throwUsageError( + `--instance ${flag} does not match the key from ${keySource}, which addresses the ` + + `${identity.env} instance ${identity.instanceId}. Nothing was changed.\n` + + "Pass the key for that instance with --secret-key, or drop --instance.", + ); +} + +/** Resolves the key, then names the instance it addresses. */ +export async function resolveClerkTarget( + options: TargetOptions, +): Promise<{ secretKey: string; target: ClerkTarget }> { + const secretKey = await resolveBapiSecretKey(options); + const source = await describeKeySource(options); + const { instanceId, env } = await fetchInstanceIdentity(secretKey); + assertInstanceFlagMatches(options, source.keySource, { instanceId, env }); + + return { + secretKey, + target: { + env, + instanceId, + instanceType: detectInstanceType(secretKey), + ...source, + }, + }; +} + +/** One line naming a target, for prose: `My App (development, ins_123)`. */ +export function describeTarget(target: RunTarget): string { + if (target.platform && !target.instanceId) return target.platform; + const where = [target.env, target.instanceId].filter(Boolean).join(", "); + const name = target.appLabel ?? (target.platform ? `${target.platform}` : "instance"); + return where ? `${name} (${where})` : name; +} + +/** + * The header every command prints first: which instance it acts on, and where + * the key came from. An export names its source platform instead, plus the + * Clerk instance when that is what it reads. + * + * `--json` carries the same facts as `target`, so this is for humans only. + */ +export function printTarget(target: RunTarget): void { + const heading = target.platform ? "Source" : "Target"; + const instance = target.instanceId + ? `${target.env ?? "unknown"} instance ${target.instanceId}` + : undefined; + const app = target.appLabel + ? `${target.appLabel}${target.appId ? ` (${target.appId})` : ""}` + : undefined; + const platform = target.platform && target.platform !== "clerk" ? target.platform : undefined; + const parts = [platform ?? (target.platform === "clerk" ? "Clerk" : undefined), app, instance]; + log.info(`${heading}: ${parts.filter(Boolean).join(", ")}`); + if (target.keySource) log.info(dim(`Key from: ${target.keySource}`)); +} diff --git a/packages/cli-core/src/lib/config.ts b/packages/cli-core/src/lib/config.ts index 41943b990..a7e1f085b 100644 --- a/packages/cli-core/src/lib/config.ts +++ b/packages/cli-core/src/lib/config.ts @@ -308,7 +308,7 @@ export async function resolveProfile(cwd: string): Promise< return undefined; } -const INSTANCE_ALIASES: Record = { +export const INSTANCE_ALIASES: Record = { dev: "development", development: "development", prod: "production", From 4190aeaa0ca4ed9c744f0a8413b5df4a6c4c496e Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:22 -0400 Subject: [PATCH 02/27] feat(migrate): trim throughput, the target, the run store and the importer 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/import-users.test.ts | 45 +-------- .../src/commands/migrate/import-users.ts | 41 +++------ .../commands/migrate/lib/run-store.test.ts | 91 +++---------------- .../src/commands/migrate/lib/run-store.ts | 75 +-------------- 4 files changed, 27 insertions(+), 225 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 47751cc73..e783004e7 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -305,7 +305,7 @@ describe("importUsers", () => { expect(lines.at(-1)).not.toHaveProperty("pending"); }); - test("keeps an attach with no answer pending, for a continued run", async () => { + test("keeps an attach with no answer pending", async () => { stub((url) => url.endsWith("/v1/email_addresses") ? clerkError(503, "unavailable") : ok("user_created"), ); @@ -346,49 +346,6 @@ describe("importUsers", () => { ]); }); - test("attachOnly sends just the pending attaches, and clears them", async () => { - stub(() => ok("idn_1")); - - await importUsers({ - users: [], - attachOnly: [ - { - sourceId: "u1", - clerkId: "user_1", - status: "created", - pending: [{ kind: "phone", value: "+15555550100", verified: false }], - }, - ], - secretKey: "sk_test_x", - limits: LIMITS, - record, - }); - - expect(requests.map((r) => [new URL(r.url).pathname, r.body])).toEqual([ - [ - "/v1/phone_numbers", - { user_id: "user_1", phone_number: "+15555550100", primary: false, verified: false }, - ], - ]); - expect(lines.at(-1)).toEqual({ sourceId: "u1", clerkId: "user_1", status: "created" }); - }); - - test("an adopted user is not created again; only its extras attach", async () => { - stub(() => ok("idn_1")); - - const summary = await importUsers({ - users: [user({ email: ["a@x.dev", "b@x.dev"] })], - adopted: new Map([["u1", "user_found"]]), - secretKey: "sk_test_x", - limits: LIMITS, - record, - }); - - expect(requests.map((r) => new URL(r.url).pathname)).toEqual(["/v1/email_addresses"]); - expect(summary.successful).toBe(1); - expect(lines.at(-1)).toMatchObject({ clerkId: "user_found", status: "created" }); - }); - // Shapes from clerk_go's apierror: the country error carries its own code // and no param_name; the E.164 error is a form error on phone_number. test.each([ diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 533dd7938..f6eb7a2d7 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -327,17 +327,6 @@ export type ImportUsersOptions = { limits: ResolvedLimits; /** Receives each user's lines as they happen. */ record: (line: UserLine) => void; - /** - * Users a continued run created whose extra identifiers never attached: their - * latest `created` line, with `pending`. Only the attaches are sent. - */ - attachOnly?: UserLine[]; - /** - * Source ID → Clerk ID for users whose create a stopped run sent with no - * answer, and which a continued run then found in the instance. They are - * not created again; only their extra identifiers are sent. - */ - adopted?: Map; /** Allow users that carry no password. */ skipPasswordRequirement?: boolean; /** Carried into the summary so the report covers the whole file. */ @@ -351,7 +340,8 @@ export type ImportUsersOptions = { * * A failed user is recorded and the run continues; a 429 backs off (honouring * `Retry-After`) and retries up to {@link MAX_RETRIES} times. A create with no - * answer keeps its `creating` line, for a continued run or `undo` to resolve. + * answer keeps its `creating` line: Clerk may hold the user, and a re-run's + * checks look it up before creating it again. */ export async function importUsers(options: ImportUsersOptions): Promise { const { @@ -359,8 +349,6 @@ export async function importUsers(options: ImportUsersOptions): Promise(), skipPasswordRequirement = true, validationFailed = 0, progress: report, @@ -400,7 +388,8 @@ export async function importUsers(options: ImportUsersOptions): Promise { const { error: _error, pending: _pending, ...base } = line; @@ -421,17 +410,14 @@ export async function importUsers(options: ImportUsersOptions): Promise - createUser(ctx, user, identifiers, skipPasswordRequirement, () => - record({ sourceId: user.userId, status: "creating" }), - ), - { onRetry: ({ message }) => retries.push(message) }, - ); + created = await retryOn429( + async () => + createUser(ctx, user, identifiers, skipPasswordRequirement, () => + record({ sourceId: user.userId, status: "creating" }), + ), + { onRetry: ({ message }) => retries.push(message) }, + ); } catch (error) { if (error instanceof RateLimitExceededError) { recordFailure(user.userId, error.message, "429", retries, false); @@ -464,10 +450,7 @@ export async function importUsers(options: ImportUsersOptions): Promise processUser(user)), - ...attachOnly.map(async (line) => finishUser(line, line.pending ?? [], [])), - ]); + await Promise.all(users.map(async (user) => processUser(user))); return { totalProcessed: total, successful, failed, validationFailed, errorBreakdown }; } diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts index 0616b6343..8c9ef687d 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts @@ -15,13 +15,11 @@ import os from "node:os"; import path from "node:path"; import { _setConfigDir } from "../../../lib/config.ts"; import { - continueRun, latestUserLines, - listRuns, + liveLockPid, newRunId, readRun, resolveRunsDir, - runState, RUNS_DIR_ENV, startRun, } from "./run-store.ts"; @@ -127,17 +125,6 @@ describe("a run's life", () => { expect([...latestUserLines(runsDir, run.record.id).keys()]).toEqual(["a"]); }); - test("a continued run starts a fresh line after one a crash cut short", () => { - const run = startRun(runsDir, init); - run.append({ sourceId: "a", status: "created" }); - fs.appendFileSync(path.join(run.dir, "users.ndjson"), '{"sourceId":"b","sta'); - fs.rmSync(path.join(run.dir, "lock")); - - continueRun(runsDir, run.record).append({ sourceId: "c", status: "created" }); - - expect([...latestUserLines(runsDir, run.record.id).keys()]).toEqual(["a", "c"]); - }); - // A user created with no line is beyond both undo and a re-run, so the // create that would follow must not go out. test("append throws when the line cannot be written", () => { @@ -148,53 +135,26 @@ describe("a run's life", () => { }); }); -describe("locks and interruptions", () => { - // A PID no process can have. - const DEAD_PID = "2147483646"; - - test("a run whose process died is interrupted", () => { +describe("locks", () => { + test("a lock whose process died is stale", () => { const run = startRun(runsDir, init); - fs.writeFileSync(path.join(run.dir, "lock"), DEAD_PID); - expect(runState(runsDir, run.record)).toBe("interrupted"); - }); - - test("a run with no lock and no finish time is interrupted", () => { - const run = startRun(runsDir, init); - fs.rmSync(path.join(run.dir, "lock")); - expect(runState(runsDir, run.record)).toBe("interrupted"); - }); - - // Undo marks an interrupted import undone, but leaves it with no finish time. - test("an interrupted run that was undone reads as undone", () => { - const run = startRun(runsDir, init); - fs.rmSync(path.join(run.dir, "lock")); - expect(runState(runsDir, { ...run.record, status: "undone" })).toBe("undone"); - }); - - test("continuing takes the lock from a dead process and reopens the run", () => { - const run = startRun(runsDir, init); - run.append({ sourceId: "a", status: "created" }); - run.finish(); - - const again = continueRun(runsDir, readRun(runsDir, run.record.id)!); - expect(again.record).toMatchObject({ id: run.record.id, status: "running" }); - expect(again.record.finishedAt).toBeUndefined(); - again.append({ sourceId: "b", status: "created" }); - expect(again.finish().counts.total).toBe(2); + // A PID no process can have. + fs.writeFileSync(path.join(run.dir, "lock"), "2147483646"); + expect(liveLockPid(runsDir, run.record.id)).toBeUndefined(); }); // In a container the CLI often gets the same PID every run, so a killed // run's lock can hold this process's own PID. test("a lock holding this process's own PID is stale", () => { const run = startRun(runsDir, init); - expect(runState(runsDir, run.record)).toBe("interrupted"); - expect(() => continueRun(runsDir, run.record)).not.toThrow(); + expect(liveLockPid(runsDir, run.record.id)).toBeUndefined(); }); - test("reads as running while another live process holds the lock", () => { + test("a lock another live process holds is live", () => { const run = startRun(runsDir, init); + // PID 1 is always alive, and never this test. fs.writeFileSync(path.join(run.dir, "lock"), "1"); - expect(runState(runsDir, run.record)).toBe("running"); + expect(liveLockPid(runsDir, run.record.id)).toBe(1); }); // Two runs in the same second share an ID one time in 65,536. @@ -215,35 +175,8 @@ describe("locks and interruptions", () => { } }); - test("refuses a run another live process holds, with exit 2", () => { + test("run folders are owner-only", () => { const run = startRun(runsDir, init); - // PID 1 is always alive, and never this test. - fs.writeFileSync(path.join(run.dir, "lock"), "1"); - - expect(() => continueRun(runsDir, run.record)).toThrow( - new RegExp( - `in use by another process \\(PID 1\\).*delete ${path.join(run.dir, "lock")}`, - "s", - ), - ); - }); -}); - -describe("listRuns", () => { - test("lists newest first and ignores folders that are not runs", () => { - const first = startRun(runsDir, init); - first.update({ startedAt: "2026-01-01T00:00:00.000Z" }); - const second = startRun(runsDir, { ...init, kind: "export" }); - second.update({ startedAt: "2026-02-01T00:00:00.000Z" }); - fs.mkdirSync(path.join(runsDir, "not-a-run")); - - expect(listRuns(runsDir).map((record) => record.id)).toEqual([ - second.record.id, - first.record.id, - ]); - }); - - test("is empty when the folder does not exist", () => { - expect(listRuns(path.join(workDir, "nowhere"))).toEqual([]); + expect(fs.statSync(run.dir).mode & 0o777).toBe(0o700); }); }); diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.ts b/packages/cli-core/src/commands/migrate/lib/run-store.ts index 6ac2f2bf3..a181da91e 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.ts @@ -11,8 +11,8 @@ * second writer; a dead one, or a run with no `finishedAt`, means the run * was interrupted. * - * `runs`, `undo`, re-runs and exports all read or write it, so there is no - * second record to drift out of step. + * Everything `clerk migrate` records lives here, so there is no second record + * to drift out of step. * * Appends are synchronous so a run interrupted with Ctrl-C still leaves a * complete record of everything already processed. @@ -101,9 +101,6 @@ export type UserLine = { passwordDropped?: boolean; }; -/** What `runs` shows for a run: its stored status, or how it stopped. */ -export type RunState = RunStatus | "interrupted"; - const RUN_FILE = "run.json"; const USERS_FILE = "users.ndjson"; const LOCK_FILE = "lock"; @@ -283,48 +280,6 @@ export function latestUserLines(runsDir: string, id: string): Map { - const ids = new Set(); - for (const record of listRuns(runsDir)) { - if (record.kind !== "import" || record.id === exceptId) continue; - for (const line of latestUserLines(runsDir, record.id).values()) { - if (line.status === "created" && line.clerkId) ids.add(line.clerkId); - } - } - return ids; -} - -/** Every readable run, newest first. */ -export function listRuns(runsDir: string): RunRecord[] { - let entries: fs.Dirent[]; - try { - entries = fs.readdirSync(runsDir, { withFileTypes: true }); - } catch { - return []; - } - return entries - .filter((entry) => entry.isDirectory()) - .map((entry) => readRun(runsDir, entry.name)) - .filter((record): record is RunRecord => record !== undefined) - .sort((a, b) => b.startedAt.localeCompare(a.startedAt) || b.id.localeCompare(a.id)); -} - // --- Writing --------------------------------------------------------------- function writeRecord(runsDir: string, record: RunRecord): void { @@ -426,29 +381,3 @@ export function startRun(runsDir: string, init: StartRunInit): Run { log.debug(`migrate: started ${init.kind} run ${id} in ${runsDir}`); return openRun(runsDir, record); } - -/** - * Reopens a finished or interrupted run to keep working on it. - * - * @throws UsageError when another live process holds its lock. - */ -export function continueRun(runsDir: string, record: RunRecord): Run { - acquireLock(runsDir, record.id); - // A crash mid-write leaves a torn last line; end it so the next append - // starts a line of its own instead of fusing with it. - const usersFile = path.join(runDir(runsDir, record.id), USERS_FILE); - const written = fs.existsSync(usersFile) ? fs.readFileSync(usersFile, "utf-8") : ""; - if (written && !written.endsWith("\n")) fs.appendFileSync(usersFile, "\n"); - const run = openRun(runsDir, record); - run.record = { ...record, status: "running" }; - delete run.record.finishedAt; - writeRecord(runsDir, run.record); - log.debug(`migrate: continuing ${record.kind} run ${record.id} in ${runsDir}`); - return run; -} - -/** Merges fields into a run this process is not writing, such as `undoneBy`. */ -export function patchRun(runsDir: string, id: string, patch: Partial): void { - const record = readRun(runsDir, id); - if (record) writeRecord(runsDir, { ...record, ...patch }); -} From 03a460d607e6cd173b13aff535825f2fe1929b59 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:22 -0400 Subject: [PATCH 03/27] fix(migrate): skip creates a Ctrl-C stopped before they were sent A queued create aborted before its POST /v1/users counted as a failure whose outcome was unknown, inflating the failed count and the error summary. It is now left unrecorded, so a re-run imports it. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 21 +++++++++++++++++++ .../src/commands/migrate/import-users.ts | 7 ++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index e783004e7..5a4345b47 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -1,5 +1,6 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; import { BapiError } from "../../lib/errors.ts"; +import { _resetInterruptState, abortInFlight } from "../../lib/signals.ts"; import { buildCreateUserBody, importUsers, @@ -222,6 +223,26 @@ describe("importUsers", () => { expect(lines.filter((line) => line.status === "created")).toHaveLength(2); }); + test("neither fails nor records a create a Ctrl-C stopped before it was sent", async () => { + stub(() => ok("user_created")); + abortInFlight(); + try { + const summary = await importUsers({ + users: [user({ userId: "u1" }), user({ userId: "u2", email: "b@x.dev" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 0, failed: 0 }); + expect(summary.errorBreakdown.size).toBe(0); + expect(requests).toHaveLength(0); + expect(allLines).toHaveLength(0); + } finally { + _resetInterruptState(); + } + }); + test("attaches additional and unverified identifiers after the user exists", async () => { stub(() => ok("user_created")); diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index f6eb7a2d7..4f0ddba51 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -176,6 +176,9 @@ type CreateContext = { schedule: ApiScheduler; }; +/** A create a Ctrl-C stopped before it went out: neither a failure nor unknown. */ +class NotSentError extends Error {} + /** * True when no answer says whether a request landed: an abort, a network * error, or a 5xx after which Clerk may still have committed it. A 4xx, and a @@ -288,7 +291,7 @@ async function createUser( const create = async (body: Record) => ctx.schedule(async () => { // A Ctrl-C hands the slot on to queued creates; none of them was sent. - interruptSignal().throwIfAborted(); + if (interruptSignal().aborted) throw new NotSentError(); sending(); return bapiRequest({ method: "POST", @@ -419,6 +422,8 @@ export async function importUsers(options: ImportUsersOptions): Promise retries.push(message) }, ); } catch (error) { + // Unrecorded, so a re-run picks the user up like any other. + if (error instanceof NotSentError) return; if (error instanceof RateLimitExceededError) { recordFailure(user.userId, error.message, "429", retries, false); return; From 789bc42d1951cd3acb68e043f3381adf279b10f0 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 12:59:23 -0400 Subject: [PATCH 04/27] docs(changeset): add an empty changeset; migrate stays hidden and refuses every call Co-Authored-By: Claude Opus 5.5 --- .changeset/migrate-slice-1b.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/migrate-slice-1b.md diff --git a/.changeset/migrate-slice-1b.md b/.changeset/migrate-slice-1b.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/migrate-slice-1b.md @@ -0,0 +1,2 @@ +--- +--- From 73e7d439bafef73adc6e6261477cb750ed334795 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 17:16:19 -0400 Subject: [PATCH 05/27] fix(migrate): leave a create answered without a user ID as unknown A 2xx from POST /v1/users with no `id` was recorded as created with an empty clerkId, so `undo` could never find that user. It now throws, so the outcome is unknown: `creating` stays the latest line and a re-run looks the user up. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 14 ++++++++++++++ .../cli-core/src/commands/migrate/import-users.ts | 8 +++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 5a4345b47..fa669a55f 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -517,6 +517,20 @@ describe("importUsers", () => { expect([...summary.errorBreakdown.keys()][0]).toContain("a re-run checks"); }); + test("leaves a create answered without a user ID as unknown, never created", async () => { + stub(() => new Response(JSON.stringify({}), { status: 200 })); + + const summary = await importUsers({ + users: [user({ userId: "u1" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 0, failed: 1 }); + expect(allLines).toEqual([{ sourceId: "u1", status: "creating" }]); + }); + test("records a failed user and keeps going", async () => { stub((_url, attempt) => attempt === 1 ? clerkError(422, "that email is taken") : ok("user_ok"), diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 4f0ddba51..ad842e055 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -321,7 +321,13 @@ async function createUser( ); } - return { clerkUserId: (response.body as { id?: string })?.id ?? "", notes }; + // Untracked, the user could never be undone. Thrown, the outcome is unknown, + // so `creating` stays the latest line and a re-run looks the user up. + const clerkUserId = (response.body as { id?: unknown })?.id; + if (typeof clerkUserId !== "string" || !clerkUserId) { + throw new Error("Clerk answered POST /v1/users without a user ID"); + } + return { clerkUserId, notes }; } export type ImportUsersOptions = { From f8072a1346f7f2af4356b67e1a7ab2318a6fbf1c Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 6 Oct 2026 17:16:53 -0400 Subject: [PATCH 06/27] fix(migrate): say a literal --instance could not be verified when the lookup fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When GET /v1/instance fails, the instance ID falls back to a `key_` hash that a literal `--instance ins_…` can never match, so the error blamed the flag. It now says the instance could not be verified. The run still refuses. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/target.test.ts | 18 ++++++++++++++++++ .../src/commands/migrate/lib/target.ts | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/packages/cli-core/src/commands/migrate/lib/target.test.ts b/packages/cli-core/src/commands/migrate/lib/target.test.ts index ac192ca7e..be12b5381 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.test.ts @@ -147,6 +147,24 @@ describe("resolveClerkTarget --instance", () => { expect(error.message).toContain("does not match the key"); }); + test("says a literal --instance could not be verified when the lookup fails", async () => { + delete process.env.CLERK_SECRET_KEY; + const working = globalThis.fetch; + globalThis.fetch = (async () => + new Response("nope", { status: 500 })) as unknown as typeof fetch; + try { + const error = (await resolveClerkTarget({ + instance: "ins_prod", + secretKey: "sk_live_x", + }).catch((e: unknown) => e)) as CliError; + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain("Could not verify"); + expect(error.message).not.toContain("does not match"); + } finally { + globalThis.fetch = working; + } + }); + test.each([["prod"], ["production"], ["ins_prod"]])("accepts --instance %s", async (instance) => { delete process.env.CLERK_SECRET_KEY; const { target } = await resolveClerkTarget({ instance, secretKey: "sk_live_x" }); diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts index 998a60276..71bd96334 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -120,6 +120,15 @@ function assertInstanceFlagMatches( const wanted = INSTANCE_ALIASES[flag]; const matches = wanted ? identity.env === wanted : identity.instanceId === flag; if (matches) return; + // A `key_` ID means Clerk did not name the instance, so a literal ID cannot + // be checked; saying it "does not match" would blame the flag. + if (!wanted && identity.instanceId.startsWith("key_")) { + throwUsageError( + `Could not verify that the key from ${keySource} addresses --instance ${flag}: ` + + "Clerk did not answer GET /v1/instance. Nothing was changed.\n" + + "Try again, or pass --instance dev or --instance prod.", + ); + } throwUsageError( `--instance ${flag} does not match the key from ${keySource}, which addresses the ` + `${identity.env} instance ${identity.instanceId}. Nothing was changed.\n` + From 393e0da72a58348e12d1d5393baea61818901a52 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:00:17 -0400 Subject: [PATCH 07/27] fix(migrate): stop sending creates once the user quota is reached A `403 user_quota_exceeded` was recorded for that user, but every queued user still went out, each refused the same way. The first one now stops the run: later creates are not sent and stay unrecorded, so a re-run picks them up once the limit is raised. A warning says so. The instance.ts comment said production has no user limit; a plan sets one. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 40 +++++++++++++++++++ .../src/commands/migrate/import-users.ts | 16 +++++++- .../src/commands/migrate/lib/instance.ts | 3 +- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index fa669a55f..baaf1b9f4 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -1,6 +1,7 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; import { BapiError } from "../../lib/errors.ts"; import { _resetInterruptState, abortInFlight } from "../../lib/signals.ts"; +import { useCaptureLog } from "../../test/lib/stubs.ts"; import { buildCreateUserBody, importUsers, @@ -155,6 +156,7 @@ describe("normalizeErrorMessage", () => { }); describe("importUsers", () => { + const captured = useCaptureLog(); let originalFetch: typeof globalThis.fetch; let requests: { method: string; url: string; body: unknown }[]; let lines: UserLine[]; @@ -531,6 +533,44 @@ describe("importUsers", () => { expect(allLines).toEqual([{ sourceId: "u1", status: "creating" }]); }); + // Every create after the first refusal would be refused too, so none is + // sent: they stay unrecorded, and a re-run picks them up. + test("stops sending creates once the instance's user quota is reached", async () => { + stub((url) => + url.endsWith("/v1/users") + ? new Response( + JSON.stringify({ + errors: [ + { + code: "user_quota_exceeded", + message: "user quota exceeded", + long_message: "You have reached your limit of 100 users.", + }, + ], + }), + { status: 403 }, + ) + : ok("unused"), + ); + + const summary = await importUsers({ + users: ["u1", "u2", "u3", "u4"].map((userId) => user({ userId, email: `${userId}@x.dev` })), + secretKey: "sk_test_x", + limits: { ...LIMITS, concurrencyLimit: 1 }, + record, + }); + + expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(1); + expect(summary).toMatchObject({ successful: 0, failed: 1 }); + expect(allLines.filter((line) => line.status !== "creating")).toMatchObject([ + { sourceId: "u1", status: "failed", code: "403" }, + ]); + expect(allLines.map((line) => line.sourceId)).not.toContain("u2"); + expect(captured.err).toContain( + "You have reached your limit of 100 users. No more users are sent", + ); + }); + test("records a failed user and keeps going", async () => { stub((_url, attempt) => attempt === 1 ? clerkError(422, "that email is taken") : ok("user_ok"), diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index ad842e055..aaf490f41 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -18,6 +18,7 @@ import { bapiRequest } from "../../lib/bapi.ts"; import { BapiError } from "../../lib/errors.ts"; +import { log } from "../../lib/log.ts"; import { interruptSignal } from "../../lib/signals.ts"; import type { ResolvedLimits } from "./lib/instance.ts"; import type { ProgressUpdate } from "./lib/progress.ts"; @@ -174,6 +175,8 @@ export function buildCreateUserBody( type CreateContext = { secretKey: string; schedule: ApiScheduler; + /** Set by the first `user_quota_exceeded`: every later create would be refused too. */ + quotaReached: boolean; }; /** A create a Ctrl-C stopped before it went out: neither a failure nor unknown. */ @@ -290,8 +293,9 @@ async function createUser( ): Promise<{ clerkUserId: string; notes: string[] }> { const create = async (body: Record) => ctx.schedule(async () => { - // A Ctrl-C hands the slot on to queued creates; none of them was sent. - if (interruptSignal().aborted) throw new NotSentError(); + // A Ctrl-C or a full instance hands the slot on to queued creates; none + // of them was sent. + if (interruptSignal().aborted || ctx.quotaReached) throw new NotSentError(); sending(); return bapiRequest({ method: "POST", @@ -372,6 +376,7 @@ export async function importUsers(options: ImportUsersOptions): Promise report?.({ done: processed, ok: successful, failed }); @@ -435,6 +440,13 @@ export async function importUsers(options: ImportUsersOptions): Promise Date: Thu, 8 Oct 2026 18:01:54 -0400 Subject: [PATCH 08/27] fix(migrate): pause the whole import on a 429, not just one user A 429's backoff held back only the user that hit it; every other user kept going through the scheduler and hit the limit in turn. The scheduler can now be paused, and each backoff in the import (creates and attaches) pauses it for the Retry-After wait, so the run waits together. A call already waiting on the pacing interval is held too. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 21 +++++++++++++ .../src/commands/migrate/import-users.ts | 31 ++++++++++++------- .../commands/migrate/lib/scheduler.test.ts | 26 ++++++++++++++++ .../src/commands/migrate/lib/scheduler.ts | 24 ++++++++++++-- 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index baaf1b9f4..0d0b2c25d 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -610,6 +610,27 @@ describe("importUsers", () => { expect(lines[1]?.error).toContain("Rate limit hit (429)"); }); + // The instance is over its limit for every user, so the others wait too. + test("holds every other user's create through a 429's wait", async () => { + const started = performance.now(); + const sentAt: number[] = []; + stub((_url, attempt) => { + sentAt.push(performance.now() - started); + return attempt === 1 ? clerkError(429, "slow down", { "retry-after": "1" }) : ok("user_ok"); + }); + + const summary = await importUsers({ + users: [user({ userId: "u1" }), user({ userId: "u2", email: "b@x.dev" })], + secretKey: "sk_test_x", + limits: { ...LIMITS, concurrencyLimit: 1 }, + record, + }); + + expect(summary).toMatchObject({ successful: 2, failed: 0 }); + expect(sentAt).toHaveLength(3); + for (const at of sentAt.slice(1)) expect(at).toBeGreaterThanOrEqual(900); + }); + test("gives up after the retry ceiling and records the user as failed", async () => { stub(() => clerkError(429, "slow down", { "retry-after": "1" })); diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index aaf490f41..1e27a9c73 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -237,17 +237,19 @@ async function attachIdentifier( : { user_id: clerkUserId, phone_number: value, primary: false, verified }; try { - await retryOn429(async () => - ctx.schedule( - async () => - bapiRequest({ - method: "POST", - path, - secretKey: ctx.secretKey, - body: JSON.stringify(body), - }), - { first: true }, - ), + await retryOn429( + async () => + ctx.schedule( + async () => + bapiRequest({ + method: "POST", + path, + secretKey: ctx.secretKey, + body: JSON.stringify(body), + }), + { first: true }, + ), + { onRetry: ({ delaySeconds }) => ctx.schedule.pause(delaySeconds * 1000) }, ); return {}; } catch (error) { @@ -430,7 +432,12 @@ export async function importUsers(options: ImportUsersOptions): Promise record({ sourceId: user.userId, status: "creating" }), ), - { onRetry: ({ message }) => retries.push(message) }, + { + onRetry: ({ message, delaySeconds }) => { + retries.push(message); + ctx.schedule.pause(delaySeconds * 1000); + }, + }, ); } catch (error) { // Unrecorded, so a re-run picks the user up like any other. diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index 68c44e50b..2bba051fb 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -67,3 +67,29 @@ test("treats zero or negative limits as one", async () => { expect(peak).toBe(1); }); + +// A 429 means the instance is over its limit for everyone, not just the call +// that hit it. +test("holds every unsent call through a pause", async () => { + const schedule = createApiScheduler(4, 10_000); + const started = performance.now(); + schedule.pause(300); + + const startedAt = await Promise.all( + Array.from({ length: 3 }, async () => schedule(async () => performance.now() - started)), + ); + + for (const at of startedAt) expect(at).toBeGreaterThanOrEqual(290); +}); + +test("holds a call already waiting on the pacing interval", async () => { + // 2 req/s: the second call waits ~500ms. The first, like a 429, pauses the + // run while the second is in that wait. + const schedule = createApiScheduler(2, 2); + const started = performance.now(); + const first = schedule(async () => schedule.pause(800)); + const second = schedule(async () => performance.now() - started); + + await first; + expect(await second).toBeGreaterThanOrEqual(790); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.ts index 0971bf98e..4d018ee68 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.ts @@ -15,7 +15,16 @@ * identifiers attach before the next user is created, so a run stopped midway * leaves few users waiting on attaches. */ -export type ApiScheduler = (fn: () => Promise, options?: { first?: boolean }) => Promise; +export type ApiScheduler = (( + fn: () => Promise, + options?: { first?: boolean }, +) => Promise) & { + /** + * Holds every call not yet sent until `ms` from now, so a 429 pauses the + * whole run rather than only the call that hit it. + */ + pause(ms: number): void; +}; export function createApiScheduler(concurrencyLimit: number, rateLimit: number): ApiScheduler { const maxConcurrent = Math.max(1, Math.floor(concurrencyLimit)); @@ -24,6 +33,7 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): const waitingFirst: (() => void)[] = []; let active = 0; let nextRequestAt = 0; + let pausedUntil = 0; async function acquire(first: boolean): Promise { if (active < maxConcurrent) { @@ -45,7 +55,7 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): }); } - return async (fn, options) => { + const schedule = async (fn: () => Promise, options?: { first?: boolean }) => { await acquire(options?.first ?? false); try { const now = Date.now(); @@ -54,9 +64,19 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): if (waitMs > 0) { await new Promise((resolve) => setTimeout(resolve, waitMs)); } + // Checked after the pacing wait too: a pause can start during it. + while (Date.now() < pausedUntil) { + await new Promise((resolve) => setTimeout(resolve, pausedUntil - Date.now())); + } return await fn(); } finally { release(); } }; + + return Object.assign(schedule, { + pause(ms: number) { + pausedUntil = Math.max(pausedUntil, Date.now() + ms); + }, + }); } From e7068669e1360a7a18c4a28182c7fa8e150e365f Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:02:55 -0400 Subject: [PATCH 09/27] fix(migrate): call a key accountless only when it came from the SDK's file A secret key found in `.env` or `.env.local` was labelled an "accountless app", including an account-owned production app's key. Only `.clerk/.tmp/keyless.json` says the app is accountless; any other file is named as the key's source and nothing more. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/target.test.ts | 58 ++++++++++++++++++- .../src/commands/migrate/lib/target.ts | 6 +- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/target.test.ts b/packages/cli-core/src/commands/migrate/lib/target.test.ts index be12b5381..1a8d95236 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.test.ts @@ -1,4 +1,8 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { _setConfigDir } from "../../../lib/config.ts"; import { EXIT_CODE, type CliError } from "../../../lib/errors.ts"; import { useCaptureLog } from "../../../test/lib/stubs.ts"; import { @@ -171,3 +175,55 @@ describe("resolveClerkTarget --instance", () => { expect(target.instanceId).toBe("ins_prod"); }); }); + +describe("resolveClerkTarget key source", () => { + let originalFetch: typeof globalThis.fetch; + let originalKey: string | undefined; + let originalCwd: string; + let workDir: string; + let configDir: string; + + beforeEach(() => { + originalFetch = globalThis.fetch; + originalKey = process.env.CLERK_SECRET_KEY; + originalCwd = process.cwd(); + delete process.env.CLERK_SECRET_KEY; + workDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-migrate-target-"))); + configDir = fs.mkdtempSync(path.join(os.tmpdir(), "clerk-migrate-target-config-")); + _setConfigDir(configDir); + process.chdir(workDir); + globalThis.fetch = (async () => + Response.json({ id: "ins_1", environment_type: "development" })) as unknown as typeof fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + if (originalKey === undefined) delete process.env.CLERK_SECRET_KEY; + else process.env.CLERK_SECRET_KEY = originalKey; + _setConfigDir(undefined); + process.chdir(originalCwd); + fs.rmSync(workDir, { recursive: true, force: true }); + fs.rmSync(configDir, { recursive: true, force: true }); + }); + + // A key in `.env` may be any app's, an account's included. + test("names a key from .env by its file, not as an accountless app", async () => { + fs.writeFileSync(path.join(workDir, ".env"), "CLERK_SECRET_KEY=sk_test_x\n"); + const { target } = await resolveClerkTarget({}); + expect(target.keySource).toBe(".env"); + expect(target.appLabel).toBeUndefined(); + }); + + test("names the SDK's own keyless file as an accountless app", async () => { + fs.mkdirSync(path.join(workDir, ".clerk", ".tmp"), { recursive: true }); + fs.writeFileSync( + path.join(workDir, ".clerk", ".tmp", "keyless.json"), + JSON.stringify({ secretKey: "sk_test_x", publishableKey: "pk_test_x" }), + ); + const { target } = await resolveClerkTarget({}); + expect(target).toMatchObject({ + keySource: "accountless app (.clerk/.tmp/keyless.json)", + appLabel: "accountless app", + }); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts index 71bd96334..7796a0247 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -54,9 +54,13 @@ async function describeKeySource( if (process.env.CLERK_SECRET_KEY) return { keySource: "CLERK_SECRET_KEY env var" }; + // Only the SDK's own file says the app is accountless: a key in `.env` + // may belong to any app, an account's included. const keyless = await resolveKeylessTarget({ instance: options.instance }); if (keyless) { - return { keySource: `accountless app (${keyless.source})`, appLabel: "accountless app" }; + return keyless.source === ".clerk/.tmp/keyless.json" + ? { keySource: `accountless app (${keyless.source})`, appLabel: "accountless app" } + : { keySource: keyless.source }; } const ctx = await resolveAppContext({ instance: options.instance }); From 9f2fc5591427a927b8be34178844c9c602c4fd24 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:03:30 -0400 Subject: [PATCH 10/27] fix(migrate): print the key source without escape codes for agents `printTarget` dimmed its "Key from" line in every mode, so an agent running without `--json` got ANSI codes in its output. The line is plain text outside human mode. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/target.test.ts | 11 +++++++++++ packages/cli-core/src/commands/migrate/lib/target.ts | 7 ++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/lib/target.test.ts b/packages/cli-core/src/commands/migrate/lib/target.test.ts index 1a8d95236..78c98c89c 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { _setConfigDir } from "../../../lib/config.ts"; +import { _resetMode, setMode } from "../../../mode.ts"; import { EXIT_CODE, type CliError } from "../../../lib/errors.ts"; import { useCaptureLog } from "../../../test/lib/stubs.ts"; import { @@ -46,6 +47,16 @@ describe("printTarget", () => { expect(lines).toEqual([first, second]); }); + test("prints no escape codes for an agent", () => { + setMode("agent"); + try { + printTarget({ env: "development", instanceId: "ins_2", keySource: "--secret-key" }); + } finally { + _resetMode(); + } + expect(captured.err).toBe("Target: development instance ins_2\nKey from: --secret-key"); + }); + test("names an export's source platform", () => { printTarget({ platform: "auth0" }); expect(captured.err).toBe("Source: auth0"); diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts index 7796a0247..7180dc3f0 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -16,6 +16,7 @@ import { INSTANCE_ALIASES, resolveAppContext } from "../../../lib/config.ts"; import { throwUsageError } from "../../../lib/errors.ts"; import { resolveKeylessTarget } from "../../../lib/keyless-target.ts"; import { log } from "../../../lib/log.ts"; +import { isHuman } from "../../../mode.ts"; import { detectInstanceType } from "./instance.ts"; import { retryOn429 } from "./retry.ts"; import type { RunTarget } from "./run-store.ts"; @@ -186,5 +187,9 @@ export function printTarget(target: RunTarget): void { const platform = target.platform && target.platform !== "clerk" ? target.platform : undefined; const parts = [platform ?? (target.platform === "clerk" ? "Clerk" : undefined), app, instance]; log.info(`${heading}: ${parts.filter(Boolean).join(", ")}`); - if (target.keySource) log.info(dim(`Key from: ${target.keySource}`)); + if (target.keySource) { + // Agents read this without `--json` too; an escape code is noise to them. + const line = `Key from: ${target.keySource}`; + log.info(isHuman() ? dim(line) : line); + } } From c519c8a5bec03275f99afb15d8c5a2d85c13a249 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:04:09 -0400 Subject: [PATCH 11/27] fix(migrate): create a run's files owner-only The run folders were 0700 but `users.ndjson` (emails and phones), `run.json` and the lock got the default file mode. They are created 0600 now, in case a folder's mode is ever looser than expected. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/run-store.test.ts | 9 +++++++++ packages/cli-core/src/commands/migrate/lib/run-store.ts | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts index 8c9ef687d..7aaef1803 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts @@ -179,4 +179,13 @@ describe("locks", () => { const run = startRun(runsDir, init); expect(fs.statSync(run.dir).mode & 0o777).toBe(0o700); }); + + // In case a folder's mode is ever looser than the run store sets it. + test("run files are owner-only", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "u1", status: "creating" }); + for (const file of ["run.json", "users.ndjson", "lock"]) { + expect(fs.statSync(path.join(run.dir, file)).mode & 0o777).toBe(0o600); + } + }); }); diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.ts b/packages/cli-core/src/commands/migrate/lib/run-store.ts index a181da91e..a1ab12f12 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.ts @@ -219,7 +219,7 @@ function acquireLock(runsDir: string, id: string): void { `Run ${id} is in use by another process${holder ? ` (PID ${holder})` : ""}. Wait for it to finish, then try again. ` + `If that process is not a migrate run, delete ${file}.`, ); - const take = () => fs.writeFileSync(file, String(process.pid), { flag: "wx" }); + const take = () => fs.writeFileSync(file, String(process.pid), { flag: "wx", mode: 0o600 }); try { take(); @@ -286,7 +286,7 @@ function writeRecord(runsDir: string, record: RunRecord): void { const file = path.join(runDir(runsDir, record.id), RUN_FILE); // Written whole and renamed into place, so a reader never sees half a file. const temp = `${file}.tmp`; - fs.writeFileSync(temp, `${JSON.stringify(record, null, 2)}\n`); + fs.writeFileSync(temp, `${JSON.stringify(record, null, 2)}\n`, { mode: 0o600 }); fs.renameSync(temp, file); } @@ -329,7 +329,8 @@ function openRun(runsDir: string, record: RunRecord): Run { // Throws when the line cannot be written: a user created with no record is // beyond both `undo` and a re-run, so the create it precedes must not go out. append(line) { - fs.appendFileSync(usersFile, `${JSON.stringify(line)}\n`); + // Owner-only like the folder: the lines carry emails and phones. + fs.appendFileSync(usersFile, `${JSON.stringify(line)}\n`, { mode: 0o600 }); }, update(patch) { run.record = { ...run.record, ...patch }; From 33972ae815c1d089d8b64fe9dafb39a578e45ef4 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:16:57 -0400 Subject: [PATCH 12/27] fix(migrate): keep calls paced when a 429 pause ends Every call held through a pause resumed at the same moment, so after a 429 up to the concurrency limit went out together. A call a pause held now takes a fresh paced slot after it, so they resume an interval apart. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/scheduler.test.ts | 14 ++++++++++++++ .../src/commands/migrate/lib/scheduler.ts | 18 +++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index 2bba051fb..5707f82d7 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -82,6 +82,20 @@ test("holds every unsent call through a pause", async () => { for (const at of startedAt) expect(at).toBeGreaterThanOrEqual(290); }); +// Released together, the held calls would hit the limit the pause waited out. +test("keeps calls paced when a pause ends", async () => { + const schedule = createApiScheduler(4, 10); + const started = performance.now(); + const first = schedule(async () => schedule.pause(300)); + const held = [1, 2, 3].map(async () => schedule(async () => performance.now() - started)); + + await first; + const startedAt = (await Promise.all(held)).sort((a, b) => a - b); + for (let i = 1; i < startedAt.length; i++) { + expect(startedAt[i]! - startedAt[i - 1]!).toBeGreaterThanOrEqual(90); + } +}); + test("holds a call already waiting on the pacing interval", async () => { // 2 req/s: the second call waits ~500ms. The first, like a 429, pauses the // run while the second is in that wait. diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.ts index 4d018ee68..f7d174f19 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.ts @@ -58,15 +58,15 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): const schedule = async (fn: () => Promise, options?: { first?: boolean }) => { await acquire(options?.first ?? false); try { - const now = Date.now(); - const waitMs = Math.max(0, nextRequestAt - now); - nextRequestAt = Math.max(now, nextRequestAt) + intervalMs; - if (waitMs > 0) { - await new Promise((resolve) => setTimeout(resolve, waitMs)); - } - // Checked after the pacing wait too: a pause can start during it. - while (Date.now() < pausedUntil) { - await new Promise((resolve) => setTimeout(resolve, pausedUntil - Date.now())); + // A pause that starts during the wait takes a fresh paced slot after it, + // so calls held through a pause resume an interval apart, not together. + for (;;) { + const pauseSeen = pausedUntil; + const now = Date.now(); + const startAt = Math.max(now, nextRequestAt, pausedUntil); + nextRequestAt = startAt + intervalMs; + if (startAt > now) await new Promise((resolve) => setTimeout(resolve, startAt - now)); + if (pausedUntil === pauseSeen) break; } return await fn(); } finally { From cf6eccdd3db0f6e76f79ed48f7832d0bfb4591b0 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:16:58 -0400 Subject: [PATCH 13/27] test(migrate): skip the run store's mode checks on Windows Windows ignores POSIX file modes, so the owner-only checks would fail there. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/run-store.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts index 7aaef1803..8b2a0d442 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts @@ -175,13 +175,14 @@ describe("locks", () => { } }); - test("run folders are owner-only", () => { + // Windows ignores POSIX modes. + test.skipIf(process.platform === "win32")("run folders are owner-only", () => { const run = startRun(runsDir, init); expect(fs.statSync(run.dir).mode & 0o777).toBe(0o700); }); // In case a folder's mode is ever looser than the run store sets it. - test("run files are owner-only", () => { + test.skipIf(process.platform === "win32")("run files are owner-only", () => { const run = startRun(runsDir, init); run.append({ sourceId: "u1", status: "creating" }); for (const file of ["run.json", "users.ndjson", "lock"]) { From 7bd0f9a7453e5c611da28d29310f46604ef77b8e Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:46:03 -0400 Subject: [PATCH 14/27] fix(migrate): count the users an import never sent A Ctrl-C or the user quota stops the run before every user is sent. The unsent users have no line in the run, so nothing counted them, and the run could read as complete. The summary now carries `notSent`. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/import-users.test.ts | 4 ++-- packages/cli-core/src/commands/migrate/import-users.ts | 8 ++++++-- packages/cli-core/src/commands/migrate/types.ts | 5 +++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 0d0b2c25d..e79075554 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -236,7 +236,7 @@ describe("importUsers", () => { record, }); - expect(summary).toMatchObject({ successful: 0, failed: 0 }); + expect(summary).toMatchObject({ successful: 0, failed: 0, notSent: 2 }); expect(summary.errorBreakdown.size).toBe(0); expect(requests).toHaveLength(0); expect(allLines).toHaveLength(0); @@ -561,7 +561,7 @@ describe("importUsers", () => { }); expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(1); - expect(summary).toMatchObject({ successful: 0, failed: 1 }); + expect(summary).toMatchObject({ successful: 0, failed: 1, notSent: 3 }); expect(allLines.filter((line) => line.status !== "creating")).toMatchObject([ { sourceId: "u1", status: "failed", code: "403" }, ]); diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 1e27a9c73..daadb3b80 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -374,6 +374,7 @@ export async function importUsers(options: ImportUsersOptions): Promise processUser(user))); - return { totalProcessed: total, successful, failed, validationFailed, errorBreakdown }; + return { totalProcessed: total, successful, failed, notSent, validationFailed, errorBreakdown }; } diff --git a/packages/cli-core/src/commands/migrate/types.ts b/packages/cli-core/src/commands/migrate/types.ts index 44e7d5c62..756ac3fac 100644 --- a/packages/cli-core/src/commands/migrate/types.ts +++ b/packages/cli-core/src/commands/migrate/types.ts @@ -46,6 +46,11 @@ export type ImportSummary = { totalProcessed: number; successful: number; failed: number; + /** + * Users never sent: a Ctrl-C or the instance's user quota stopped the run + * first. They have no line in the run, so a re-run picks them up. + */ + notSent: number; validationFailed: number; errorBreakdown: Map; }; From 15b36ff4f78221ce1b6ddd00e7a0aa4221b3f2ac Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Thu, 8 Oct 2026 18:47:02 -0400 Subject: [PATCH 15/27] fix(migrate): count the phones an import dropped on a retry When Clerk refused a user's phone and the create was retried without it, the user counted as imported and the refusal went only into that user's line. The summary now carries `droppedPhones`, by Clerk's reason, so the import can report them. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 2 ++ .../src/commands/migrate/import-users.ts | 30 ++++++++++++++----- .../cli-core/src/commands/migrate/types.ts | 5 ++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index e79075554..36945af76 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -406,6 +406,8 @@ describe("importUsers", () => { }); expect(summary).toMatchObject({ successful: 1, failed: 0 }); + // Imported, but the summary has to say the phone was lost. + expect([...summary.droppedPhones]).toEqual([[clerkErr.long_message, 1]]); expect(requests).toHaveLength(2); expect(requests[1]?.body).not.toHaveProperty("phone_number"); expect(lines.at(-1)?.error).toContain( diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index daadb3b80..b1f6b0e6a 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -284,7 +284,8 @@ async function attachAll( * * @param sending - Called as each `POST /v1/users` goes out, so the run * records the user only once a create may actually land. - * @returns The Clerk ID, and a note when the phone was dropped. + * @returns The Clerk ID, a note when the phone was dropped, and Clerk's + * reason for refusing it. */ async function createUser( ctx: CreateContext, @@ -292,7 +293,7 @@ async function createUser( identifiers: Identifiers, skipPasswordRequirement: boolean, sending: () => void, -): Promise<{ clerkUserId: string; notes: string[] }> { +): Promise<{ clerkUserId: string; notes: string[]; phoneRefusal?: string }> { const create = async (body: Record) => ctx.schedule(async () => { // A Ctrl-C or a full instance hands the slot on to queued creates; none @@ -309,6 +310,7 @@ async function createUser( const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); const notes: string[] = []; + let phoneRefusal: string | undefined; let response; try { response = await create(body); @@ -322,9 +324,8 @@ async function createUser( if (!phoneRefused || !identifiers.primaryEmail) throw error; const { phone_number: _dropped, ...withoutPhone } = body; response = await create(withoutPhone); - notes.push( - `Failed to add phone ${identifiers.primaryPhone}: ${(error as BapiError).longMessage ?? (error as BapiError).message}`, - ); + phoneRefusal = error.longMessage ?? error.message; + notes.push(`Failed to add phone ${identifiers.primaryPhone}: ${phoneRefusal}`); } // Untracked, the user could never be undone. Thrown, the outcome is unknown, @@ -333,7 +334,7 @@ async function createUser( if (typeof clerkUserId !== "string" || !clerkUserId) { throw new Error("Clerk answered POST /v1/users without a user ID"); } - return { clerkUserId, notes }; + return { clerkUserId, notes, ...(phoneRefusal ? { phoneRefusal } : {}) }; } export type ImportUsersOptions = { @@ -375,6 +376,7 @@ export async function importUsers(options: ImportUsersOptions): Promise(); const ctx: CreateContext = { secretKey, @@ -426,7 +428,7 @@ export async function importUsers(options: ImportUsersOptions): Promise => { const retries: string[] = []; const identifiers = splitIdentifiers(user); - let created: { clerkUserId: string; notes: string[] }; + let created: { clerkUserId: string; notes: string[]; phoneRefusal?: string }; try { created = await retryOn429( async () => @@ -477,6 +479,10 @@ export async function importUsers(options: ImportUsersOptions): Promise processUser(user))); - return { totalProcessed: total, successful, failed, notSent, validationFailed, errorBreakdown }; + return { + totalProcessed: total, + successful, + failed, + notSent, + droppedPhones, + validationFailed, + errorBreakdown, + }; } diff --git a/packages/cli-core/src/commands/migrate/types.ts b/packages/cli-core/src/commands/migrate/types.ts index 756ac3fac..389ca6c1d 100644 --- a/packages/cli-core/src/commands/migrate/types.ts +++ b/packages/cli-core/src/commands/migrate/types.ts @@ -51,6 +51,11 @@ export type ImportSummary = { * first. They have no line in the run, so a re-run picks them up. */ notSent: number; + /** + * Users created without the phone Clerk refused, by Clerk's reason. They + * count as imported, so these are warnings, not failures. + */ + droppedPhones: Map; validationFailed: number; errorBreakdown: Map; }; From 9984b3257a51265735216e457c1f88a875d99f8f Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 14:15:21 -0400 Subject: [PATCH 16/27] test(migrate): allow timer drift in the pause pacing check The check that calls held through a pause resume an interval apart required 90 of the 100ms, and a loaded CI runner measured 89.97. Half the interval still fails if they resume together, about 0ms apart. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/scheduler.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index 5707f82d7..b443058cf 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -91,8 +91,10 @@ test("keeps calls paced when a pause ends", async () => { await first; const startedAt = (await Promise.all(held)).sort((a, b) => a - b); + // Half the 100ms interval: a loaded runner's timers drift, and released + // together the gaps would be about 0. for (let i = 1; i < startedAt.length; i++) { - expect(startedAt[i]! - startedAt[i - 1]!).toBeGreaterThanOrEqual(90); + expect(startedAt[i]! - startedAt[i - 1]!).toBeGreaterThanOrEqual(50); } }); From e8b4d5bcb8aae72cab7393878e5afe31f429c7b5 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 14:57:46 -0400 Subject: [PATCH 17/27] test(migrate): hold the pause pacing check to 80% of the interval Half the interval would let a doubled request rate through. 80ms of the 100ms interval still allows the drift a loaded CI runner shows (89.97ms) and fails on a doubled rate (50ms) or a burst. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/lib/scheduler.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index b443058cf..221ba19b2 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -91,10 +91,10 @@ test("keeps calls paced when a pause ends", async () => { await first; const startedAt = (await Promise.all(held)).sort((a, b) => a - b); - // Half the 100ms interval: a loaded runner's timers drift, and released - // together the gaps would be about 0. + // 80 of the 100ms: a loaded runner's timers drift (89.97ms seen in CI), + // while a doubled rate would space them 50ms apart and a burst about 0. for (let i = 1; i < startedAt.length; i++) { - expect(startedAt[i]! - startedAt[i - 1]!).toBeGreaterThanOrEqual(50); + expect(startedAt[i]! - startedAt[i - 1]!).toBeGreaterThanOrEqual(80); } }); From b94350d99daeb94657a6cb4348e18f04818c7f51 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 15:51:43 -0400 Subject: [PATCH 18/27] fix(migrate): drain the queue at once after a quota stop or a Ctrl-C After the quota stop or a Ctrl-C, each queued create still waited for its paced turn before finding the run stopped. On a development instance at 10 requests a second, a 5,500-user file spent about nine minutes sending nothing. The scheduler now lets a call that will only find the run stopped skip its wait and take no paced turn. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 27 ++++++++++++++++++ .../src/commands/migrate/import-users.ts | 28 +++++++++++-------- .../commands/migrate/lib/scheduler.test.ts | 17 +++++++++++ .../src/commands/migrate/lib/scheduler.ts | 18 ++++++++---- 4 files changed, 73 insertions(+), 17 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 36945af76..346ad8e8a 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -573,6 +573,33 @@ describe("importUsers", () => { ); }); + // On dev, at 10 a second, thousands of queued users would otherwise spend + // minutes taking paced turns only to be skipped. + test("after the quota stop, queued creates are skipped without waiting their turn", async () => { + stub((url) => + url.endsWith("/v1/users") + ? new Response( + JSON.stringify({ + errors: [{ code: "user_quota_exceeded", message: "quota", long_message: "Full." }], + }), + { status: 403 }, + ) + : ok("unused"), + ); + + const started = performance.now(); + const summary = await importUsers({ + users: Array.from({ length: 6 }, (_, i) => user({ userId: `u${i}`, email: `u${i}@x.dev` })), + secretKey: "sk_test_x", + limits: { ...LIMITS, concurrencyLimit: 1, rateLimit: 2 }, + record, + }); + + expect(summary).toMatchObject({ failed: 1, notSent: 5 }); + // Paced, the five would take about 2.5s; skipped, they take none. + expect(performance.now() - started).toBeLessThan(1000); + }); + test("records a failed user and keeps going", async () => { stub((_url, attempt) => attempt === 1 ? clerkError(422, "that email is taken") : ok("user_ok"), diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index b1f6b0e6a..2e40087b4 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -294,19 +294,23 @@ async function createUser( skipPasswordRequirement: boolean, sending: () => void, ): Promise<{ clerkUserId: string; notes: string[]; phoneRefusal?: string }> { + // A Ctrl-C or a full instance hands the slot on to queued creates; none + // of them is sent, and none waits for a paced turn to find that out. + const stopped = () => interruptSignal().aborted || ctx.quotaReached; const create = async (body: Record) => - ctx.schedule(async () => { - // A Ctrl-C or a full instance hands the slot on to queued creates; none - // of them was sent. - if (interruptSignal().aborted || ctx.quotaReached) throw new NotSentError(); - sending(); - return bapiRequest({ - method: "POST", - path: "/v1/users", - secretKey: ctx.secretKey, - body: JSON.stringify(body), - }); - }); + ctx.schedule( + async () => { + if (stopped()) throw new NotSentError(); + sending(); + return bapiRequest({ + method: "POST", + path: "/v1/users", + secretKey: ctx.secretKey, + body: JSON.stringify(body), + }); + }, + { skipWait: stopped }, + ); const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); const notes: string[] = []; diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index 221ba19b2..15bb6309a 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -109,3 +109,20 @@ test("holds a call already waiting on the pacing interval", async () => { await first; expect(await second).toBeGreaterThanOrEqual(790); }); + +// A queue that will only find the run stopped should drain, not wait out +// one paced turn per call. +test("a call that skips its wait runs at once and takes no paced turn", async () => { + const schedule = createApiScheduler(1, 2); // 500ms apart + const started = performance.now(); + await schedule(async () => undefined); + const skipped = await Promise.all( + [1, 2, 3].map(async () => + schedule(async () => performance.now() - started, { skipWait: () => true }), + ), + ); + for (const at of skipped) expect(at).toBeLessThan(250); + // Skipped calls took no turn, so the next paced call is the second one. + const next = await schedule(async () => performance.now() - started); + expect(next).toBeLessThan(750); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.ts index f7d174f19..1d5db6ff4 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.ts @@ -15,10 +15,17 @@ * identifiers attach before the next user is created, so a run stopped midway * leaves few users waiting on attaches. */ -export type ApiScheduler = (( - fn: () => Promise, - options?: { first?: boolean }, -) => Promise) & { +export type ScheduleOptions = { + first?: boolean; + /** + * Checked once the call has a slot: when it returns true, `fn` runs at once + * and takes no paced turn. For a call that will only find the run stopped, + * so a Ctrl-C or a full instance drains the queue instead of pacing it. + */ + skipWait?: () => boolean; +}; + +export type ApiScheduler = ((fn: () => Promise, options?: ScheduleOptions) => Promise) & { /** * Holds every call not yet sent until `ms` from now, so a 429 pauses the * whole run rather than only the call that hit it. @@ -55,9 +62,10 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): }); } - const schedule = async (fn: () => Promise, options?: { first?: boolean }) => { + const schedule = async (fn: () => Promise, options?: ScheduleOptions) => { await acquire(options?.first ?? false); try { + if (options?.skipWait?.()) return await fn(); // A pause that starts during the wait takes a fresh paced slot after it, // so calls held through a pause resume an interval apart, not together. for (;;) { From f6b3c04860506818b45821242dc51e2a746c9afe Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 15:52:44 -0400 Subject: [PATCH 19/27] fix(migrate): return the quota stop for the caller to print after the progress bar The quota stop was logged mid-import, where a terminal's progress bar redrew over it. `importUsers` now returns Clerk's message as the summary's `stopReason`, for the caller to print once the bar is done. `notSent` also now says it includes a user whose first create got a 429 and so has a `creating` line: none of them is lost, as a re-run picks them all up. Co-Authored-By: Claude Opus 5.5 --- .../cli-core/src/commands/migrate/import-users.test.ts | 7 ++++--- packages/cli-core/src/commands/migrate/import-users.ts | 8 ++++---- packages/cli-core/src/commands/migrate/types.ts | 10 ++++++++-- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 346ad8e8a..d757290ce 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -568,9 +568,10 @@ describe("importUsers", () => { { sourceId: "u1", status: "failed", code: "403" }, ]); expect(allLines.map((line) => line.sourceId)).not.toContain("u2"); - expect(captured.err).toContain( - "You have reached your limit of 100 users. No more users are sent", - ); + // Returned for the caller to print after the progress bar, not logged + // under it, where the bar would redraw over it. + expect(summary.stopReason).toBe("You have reached your limit of 100 users."); + expect(captured.err).not.toContain("limit of 100 users"); }); // On dev, at 10 a second, thousands of queued users would otherwise spend diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 2e40087b4..895e6bfc3 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -380,6 +380,7 @@ export async function importUsers(options: ImportUsersOptions): Promise(); const ctx: CreateContext = { @@ -459,10 +460,8 @@ export async function importUsers(options: ImportUsersOptions): Promise Date: Fri, 9 Oct 2026 15:53:06 -0400 Subject: [PATCH 20/27] refactor(migrate): read the SDK keyless file's source from keyless-target target.ts restated `.clerk/.tmp/keyless.json` to tell an SDK keyless key apart, so a change to that path would quietly drop the accountless label. keyless-target.ts now exports the source string it gives such a key, and target.ts compares against that. Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/lib/target.ts | 4 ++-- packages/cli-core/src/lib/keyless-target.ts | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts index 7180dc3f0..0735e528b 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -14,7 +14,7 @@ import { dim } from "../../../lib/color.ts"; import { resolveBapiSecretKey } from "../../../lib/bapi-command.ts"; import { INSTANCE_ALIASES, resolveAppContext } from "../../../lib/config.ts"; import { throwUsageError } from "../../../lib/errors.ts"; -import { resolveKeylessTarget } from "../../../lib/keyless-target.ts"; +import { resolveKeylessTarget, SDK_KEYLESS_SOURCE } from "../../../lib/keyless-target.ts"; import { log } from "../../../lib/log.ts"; import { isHuman } from "../../../mode.ts"; import { detectInstanceType } from "./instance.ts"; @@ -59,7 +59,7 @@ async function describeKeySource( // may belong to any app, an account's included. const keyless = await resolveKeylessTarget({ instance: options.instance }); if (keyless) { - return keyless.source === ".clerk/.tmp/keyless.json" + return keyless.source === SDK_KEYLESS_SOURCE ? { keySource: `accountless app (${keyless.source})`, appLabel: "accountless app" } : { keySource: keyless.source }; } diff --git a/packages/cli-core/src/lib/keyless-target.ts b/packages/cli-core/src/lib/keyless-target.ts index 99a63edbb..0ae9492b9 100644 --- a/packages/cli-core/src/lib/keyless-target.ts +++ b/packages/cli-core/src/lib/keyless-target.ts @@ -47,6 +47,12 @@ const ENV_FILES = [".env", ".env.local"]; */ const SDK_KEYLESS_FILE = [".clerk", ".tmp", "keyless.json"]; +/** + * The `source` a key found in the SDK's own keyless file carries. Exported so + * a caller can tell that source apart without restating the path. + */ +export const SDK_KEYLESS_SOURCE = SDK_KEYLESS_FILE.join("/"); + /** * Reads the SDK's own keyless file, ignoring a partially-written one. * Exported for `init`, whose keep-the-existing-app guard must recognise an @@ -130,7 +136,7 @@ export async function findLocalSecretKey(cwd: string): Promise { const sdkApp = await readSdkKeylessApp(cwd); if (!sdkApp?.secretKey) return undefined; - return { secretKey: sdkApp.secretKey, source: SDK_KEYLESS_FILE.join("/") }; + return { secretKey: sdkApp.secretKey, source: SDK_KEYLESS_SOURCE }; } /** The publishable key a keyless project holds locally, when one can be found. */ From 547170f17e674ea536346409b87279421d373582 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 16:22:02 -0400 Subject: [PATCH 21/27] chore(migrate): drop the log import the import's stop reason left unused Co-Authored-By: Claude Opus 5.5 --- packages/cli-core/src/commands/migrate/import-users.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 895e6bfc3..e9dab60f2 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -18,7 +18,6 @@ import { bapiRequest } from "../../lib/bapi.ts"; import { BapiError } from "../../lib/errors.ts"; -import { log } from "../../lib/log.ts"; import { interruptSignal } from "../../lib/signals.ts"; import type { ResolvedLimits } from "./lib/instance.ts"; import type { ProgressUpdate } from "./lib/progress.ts"; From 59f1c36fad8a31ec4b6ec7d0d39bf596f88d7f76 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 17:55:10 -0400 Subject: [PATCH 22/27] fix(migrate): cut a backoff or paced wait short on a stop, and count only unsent users as not sent Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 24 +++++++++++++ .../src/commands/migrate/import-users.ts | 34 ++++++++++++------- .../src/commands/migrate/lib/retry.test.ts | 20 +++++++++++ .../src/commands/migrate/lib/retry.ts | 12 ++++++- .../commands/migrate/lib/scheduler.test.ts | 16 +++++++-- .../src/commands/migrate/lib/scheduler.ts | 18 ++++++---- .../cli-core/src/commands/migrate/types.ts | 7 ++-- 7 files changed, 104 insertions(+), 27 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index d757290ce..28627d1de 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -245,6 +245,30 @@ describe("importUsers", () => { } }); + // Its `creating` line already marks it unfinished; `notSent` is for users + // with no line at all. + test("does not count a user whose first create went out as not sent", async () => { + stub(() => { + abortInFlight(); + return clerkError(429, "slow down", { "retry-after": "5" }); + }); + try { + const summary = await importUsers({ + users: [user({ userId: "u1" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 0, failed: 0, notSent: 0 }); + expect(requests).toHaveLength(1); + expect(lines).toHaveLength(0); + expect(allLines.at(-1)).toMatchObject({ sourceId: "u1", status: "creating" }); + } finally { + _resetInterruptState(); + } + }); + test("attaches additional and unverified identifiers after the user exists", async () => { stub(() => ok("user_created")); diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index e9dab60f2..942bf08a7 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -174,8 +174,10 @@ export function buildCreateUserBody( type CreateContext = { secretKey: string; schedule: ApiScheduler; - /** Set by the first `user_quota_exceeded`: every later create would be refused too. */ - quotaReached: boolean; + /** Aborted by the first `user_quota_exceeded`: every later create would be refused too. */ + quota: AbortController; + /** Aborted by a Ctrl-C or the quota. No create goes out after it. */ + stop: AbortSignal; }; /** A create a Ctrl-C stopped before it went out: neither a failure nor unknown. */ @@ -295,11 +297,10 @@ async function createUser( ): Promise<{ clerkUserId: string; notes: string[]; phoneRefusal?: string }> { // A Ctrl-C or a full instance hands the slot on to queued creates; none // of them is sent, and none waits for a paced turn to find that out. - const stopped = () => interruptSignal().aborted || ctx.quotaReached; const create = async (body: Record) => ctx.schedule( async () => { - if (stopped()) throw new NotSentError(); + if (ctx.stop.aborted) throw new NotSentError(); sending(); return bapiRequest({ method: "POST", @@ -308,7 +309,7 @@ async function createUser( body: JSON.stringify(body), }); }, - { skipWait: stopped }, + { stop: ctx.stop }, ); const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); @@ -382,10 +383,12 @@ export async function importUsers(options: ImportUsersOptions): Promise(); + const quota = new AbortController(); const ctx: CreateContext = { secretKey, schedule: createApiScheduler(limits.concurrencyLimit, limits.rateLimit), - quotaReached: false, + quota, + stop: AbortSignal.any([interruptSignal(), quota.signal]), }; const progress = () => report?.({ done: processed, ok: successful, failed }); @@ -433,13 +436,16 @@ export async function importUsers(options: ImportUsersOptions): Promise - createUser(ctx, user, identifiers, skipPasswordRequirement, () => - record({ sourceId: user.userId, status: "creating" }), - ), + createUser(ctx, user, identifiers, skipPasswordRequirement, () => { + sent = true; + record({ sourceId: user.userId, status: "creating" }); + }), { + signal: ctx.stop, onRetry: ({ message, delaySeconds }) => { retries.push(message); ctx.schedule.pause(delaySeconds * 1000); @@ -447,9 +453,11 @@ export async function importUsers(options: ImportUsersOptions): Promise { expect(performance.now() - started).toBeGreaterThanOrEqual(900); }); + // A Ctrl-C or the quota landing mid-backoff should not wait out the server's wait. + test("an abort cuts a backoff short, and the retry goes at once", async () => { + let attempts = 0; + const stop = new AbortController(); + const started = performance.now(); + setTimeout(() => stop.abort(), 50); + + await retryOn429( + async () => { + attempts++; + if (attempts === 1) throw rateLimited({ "retry-after": "5" }); + return "ok"; + }, + { signal: stop.signal }, + ); + + expect(attempts).toBe(2); + expect(performance.now() - started).toBeLessThan(1000); + }); + test("falls back to the default delay when no Retry-After is given", async () => { let attempts = 0; await retryOn429( diff --git a/packages/cli-core/src/commands/migrate/lib/retry.ts b/packages/cli-core/src/commands/migrate/lib/retry.ts index add3ae203..5f0f99324 100644 --- a/packages/cli-core/src/commands/migrate/lib/retry.ts +++ b/packages/cli-core/src/commands/migrate/lib/retry.ts @@ -6,7 +6,9 @@ * makes "deletion retries the same as import" true by construction. */ +import { setTimeout as delay } from "node:timers/promises"; import { BapiError } 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. */ @@ -34,6 +36,11 @@ export type RetryOptions = { maxRetries?: number; /** Backoff when the response carries no `Retry-After`. */ defaultDelayMs?: number; + /** + * Cuts a backoff short: the retry then goes at once, for `fn` to find the + * stop. A Ctrl-C by default. + */ + signal?: AbortSignal; }; /** @@ -60,7 +67,10 @@ export async function retryOn429(fn: () => Promise, options: RetryOptions delaySeconds, message: `Rate limit hit (429), retrying in ${delaySeconds}s (attempt ${attempt + 1}/${maxRetries})`, }); - await new Promise((resolve) => setTimeout(resolve, delayMs)); + // Rejects only when the signal aborts, which ends the wait. + await delay(delayMs, undefined, { signal: options.signal ?? interruptSignal() }).catch( + () => {}, + ); } } } diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts index 15bb6309a..3541e3c43 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.test.ts @@ -116,13 +116,23 @@ test("a call that skips its wait runs at once and takes no paced turn", async () const schedule = createApiScheduler(1, 2); // 500ms apart const started = performance.now(); await schedule(async () => undefined); + const stop = AbortSignal.abort(); const skipped = await Promise.all( - [1, 2, 3].map(async () => - schedule(async () => performance.now() - started, { skipWait: () => true }), - ), + [1, 2, 3].map(async () => schedule(async () => performance.now() - started, { stop })), ); for (const at of skipped) expect(at).toBeLessThan(250); // Skipped calls took no turn, so the next paced call is the second one. const next = await schedule(async () => performance.now() - started); expect(next).toBeLessThan(750); }); + +// A Ctrl-C or the quota landing mid-wait should not wait out the turn. +test("a stop cuts short a wait already under way", async () => { + const schedule = createApiScheduler(1, 1); // 1000ms apart + const started = performance.now(); + await schedule(async () => undefined); + const stop = new AbortController(); + const waiting = schedule(async () => performance.now() - started, { stop: stop.signal }); + setTimeout(() => stop.abort(), 50); + expect(await waiting).toBeLessThan(500); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/scheduler.ts b/packages/cli-core/src/commands/migrate/lib/scheduler.ts index 1d5db6ff4..ceaff566b 100644 --- a/packages/cli-core/src/commands/migrate/lib/scheduler.ts +++ b/packages/cli-core/src/commands/migrate/lib/scheduler.ts @@ -8,6 +8,8 @@ * with ten extra email addresses cannot burst past the instance's rate limit. */ +import { setTimeout as delay } from "node:timers/promises"; + /** * Runs `fn` once a slot is free and the pacing interval has elapsed. * @@ -18,11 +20,12 @@ export type ScheduleOptions = { first?: boolean; /** - * Checked once the call has a slot: when it returns true, `fn` runs at once - * and takes no paced turn. For a call that will only find the run stopped, - * so a Ctrl-C or a full instance drains the queue instead of pacing it. + * Once aborted, `fn` runs at once and takes no paced turn, and a wait + * already under way is cut short. For a call that will only find the run + * stopped, so a Ctrl-C or a full instance drains the queue instead of + * pacing it. */ - skipWait?: () => boolean; + stop?: AbortSignal; }; export type ApiScheduler = ((fn: () => Promise, options?: ScheduleOptions) => Promise) & { @@ -65,15 +68,16 @@ export function createApiScheduler(concurrencyLimit: number, rateLimit: number): const schedule = async (fn: () => Promise, options?: ScheduleOptions) => { await acquire(options?.first ?? false); try { - if (options?.skipWait?.()) return await fn(); + const stop = options?.stop; // A pause that starts during the wait takes a fresh paced slot after it, // so calls held through a pause resume an interval apart, not together. - for (;;) { + while (!stop?.aborted) { const pauseSeen = pausedUntil; const now = Date.now(); const startAt = Math.max(now, nextRequestAt, pausedUntil); nextRequestAt = startAt + intervalMs; - if (startAt > now) await new Promise((resolve) => setTimeout(resolve, startAt - now)); + // Rejects only when `stop` aborts, which ends the wait. + if (startAt > now) await delay(startAt - now, undefined, { signal: stop }).catch(() => {}); if (pausedUntil === pauseSeen) break; } return await fn(); diff --git a/packages/cli-core/src/commands/migrate/types.ts b/packages/cli-core/src/commands/migrate/types.ts index 7c8e2d9f2..74527c367 100644 --- a/packages/cli-core/src/commands/migrate/types.ts +++ b/packages/cli-core/src/commands/migrate/types.ts @@ -47,9 +47,10 @@ export type ImportSummary = { successful: number; failed: number; /** - * Users not created because a Ctrl-C or the instance's user quota stopped - * the run first. Most have no line in the run; one whose first create got a - * 429 keeps its `creating` line. A re-run picks up both. + * Users never sent because a Ctrl-C or the instance's user quota stopped + * the run first, so they have no line in the run. One whose first create + * went out (a 429, a refused phone) is not among them: its `creating` line + * marks it unfinished. A re-run picks up both. */ notSent: number; /** From f75c7bb975b8cd07864355b8965c4267192bcbb7 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Fri, 9 Oct 2026 18:11:07 -0400 Subject: [PATCH 23/27] fix(migrate): leave an attach pending when a Ctrl-C lands while it waits Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 24 +++++++++++++++++++ .../src/commands/migrate/import-users.ts | 19 +++++++++------ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 28627d1de..ffaf8aa22 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -333,6 +333,30 @@ describe("importUsers", () => { expect(lines[2]).not.toHaveProperty("pending"); }); + // A Ctrl-C mid-backoff should not wait out the pause, nor send the attach. + test("leaves an attach pending when a Ctrl-C lands during its backoff", async () => { + stub((url) => { + if (!url.endsWith("/v1/email_addresses")) return ok("user_created"); + abortInFlight(); + return clerkError(429, "slow down", { "retry-after": "5" }); + }); + const started = performance.now(); + try { + await importUsers({ + users: [user({ email: ["a@x.dev", "b@x.dev"] })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + } finally { + _resetInterruptState(); + } + + expect(performance.now() - started).toBeLessThan(2000); + expect(requests.filter((r) => r.url.endsWith("/v1/email_addresses"))).toHaveLength(1); + expect(lines.at(-1)?.pending).toEqual([{ kind: "email", value: "b@x.dev", verified: true }]); + }); + test("retries an attach that hits a 429", async () => { stub((url, attempt) => url.endsWith("/v1/email_addresses") && attempt === 1 diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 942bf08a7..e571dae99 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -180,7 +180,7 @@ type CreateContext = { stop: AbortSignal; }; -/** A create a Ctrl-C stopped before it went out: neither a failure nor unknown. */ +/** A request a stop kept from going out: neither a failure nor unknown. */ class NotSentError extends Error {} /** @@ -237,24 +237,29 @@ async function attachIdentifier( ? { user_id: clerkUserId, email_address: value, primary: false, verified } : { user_id: clerkUserId, phone_number: value, primary: false, verified }; + // A Ctrl-C ends the wait for a slot or a backoff, and the attach is left + // pending for a re-run. The quota does not: the user exists already. + const stop = interruptSignal(); try { await retryOn429( async () => ctx.schedule( - async () => - bapiRequest({ + async () => { + if (stop.aborted) throw new NotSentError(); + return bapiRequest({ method: "POST", path, secretKey: ctx.secretKey, body: JSON.stringify(body), - }), - { first: true }, + }); + }, + { first: true, stop }, ), - { onRetry: ({ delaySeconds }) => ctx.schedule.pause(delaySeconds * 1000) }, + { signal: stop, onRetry: ({ delaySeconds }) => ctx.schedule.pause(delaySeconds * 1000) }, ); return {}; } catch (error) { - if (outcomeUnknown(error)) return { pending: true }; + if (error instanceof NotSentError || outcomeUnknown(error)) return { pending: true }; const label = `${verified ? "additional" : "unverified"} ${kind} ${value}`; return { note: `Failed to add ${label}: ${(error as Error).message}` }; } From c3acf76d51f4c17ca5b61aaf429510fee3163a73 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 16:36:00 -0400 Subject: [PATCH 24/27] fix(migrate): finish a run partial while a created user has identifiers pending A complete run is never continued, so its pending emails and phones would never attach. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/lib/run-store.test.ts | 11 +++++++++++ .../cli-core/src/commands/migrate/lib/run-store.ts | 14 ++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts index 8b2a0d442..48fab60e2 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.test.ts @@ -108,6 +108,17 @@ describe("a run's life", () => { }, ); + test("finishes partial when a created user has identifiers still pending", () => { + const run = startRun(runsDir, init); + run.append({ + sourceId: "a", + status: "created", + clerkId: "user_a", + pending: [{ kind: "email", value: "b@x.dev", verified: true }], + }); + expect(run.finish().status).toBe("partial"); + }); + test("counts each source ID by its last line", () => { const run = startRun(runsDir, init); run.append({ sourceId: "a", status: "failed", error: "boom" }); diff --git a/packages/cli-core/src/commands/migrate/lib/run-store.ts b/packages/cli-core/src/commands/migrate/lib/run-store.ts index a1ab12f12..acc0017ef 100644 --- a/packages/cli-core/src/commands/migrate/lib/run-store.ts +++ b/packages/cli-core/src/commands/migrate/lib/run-store.ts @@ -312,8 +312,8 @@ export type Run = { /** * Counts the outcomes, settles the status and releases the lock. * - * `partial` when any user failed, was skipped or may not have been created, - * `complete` otherwise. + * `partial` when any user failed, was skipped, may not have been created or + * has identifiers still pending, `complete` otherwise. */ finish(): RunRecord; }; @@ -337,8 +337,14 @@ function openRun(runsDir: string, record: RunRecord): Run { writeRecord(runsDir, run.record); }, finish() { - const counts = countLines(latestUserLines(runsDir, run.record.id).values()); - const unfinished = (counts.failed ?? 0) + (counts.skipped ?? 0) + (counts.creating ?? 0); + const lines = [...latestUserLines(runsDir, run.record.id).values()]; + const counts = countLines(lines); + // A created user whose extra identifiers never attached is left for a continue. + const pendingAttaches = lines.filter( + (line) => line.status === "created" && (line.pending?.length ?? 0) > 0, + ).length; + const unfinished = + (counts.failed ?? 0) + (counts.skipped ?? 0) + (counts.creating ?? 0) + pendingAttaches; run.update({ counts, status: unfinished > 0 ? "partial" : "complete", From 86ff6b81c2cfdc6d3ad332811f369369b82c129d Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 16:36:45 -0400 Subject: [PATCH 25/27] fix(migrate): retry without the phone only when Clerk refuses the phone itself A phone another user holds also names `phone_number`, as `form_identifier_exists`. That is an identifier conflict and fails the user, rather than importing them without it. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 37 ++++++++++++++++++- .../src/commands/migrate/import-users.ts | 6 ++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index ffaf8aa22..e2e321ff1 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -468,7 +468,13 @@ describe("importUsers", () => { () => new Response( JSON.stringify({ - errors: [{ code: "x", message: "bad phone", meta: { param_name: "phone_number" } }], + errors: [ + { + code: "form_param_format_invalid", + message: "bad phone", + meta: { param_name: "phone_number" }, + }, + ], }), { status: 422 }, ), @@ -485,6 +491,35 @@ describe("importUsers", () => { expect(requests).toHaveLength(1); }); + test("fails a user whose phone another user holds, rather than drop the phone", async () => { + stub( + () => + new Response( + JSON.stringify({ + errors: [ + { + code: "form_identifier_exists", + message: "That phone number is taken. Please try another.", + meta: { param_name: "phone_number" }, + }, + ], + }), + { status: 422 }, + ), + ); + + const summary = await importUsers({ + users: [user({ phone: "+15555550100" })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(summary).toMatchObject({ successful: 0, failed: 1 }); + expect(summary.droppedPhones.size).toBe(0); + expect(requests).toHaveLength(1); + }); + // The marker `undo` uses to find a user whose create was in flight when the // run stopped. test("marks each user as creating before its POST /v1/users", async () => { diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index e571dae99..045fa898c 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -326,10 +326,12 @@ async function createUser( } catch (error) { // A phone Clerk refuses (a country it does not support, a number that is // not E.164) should not cost a user who has an email to be created under. - // The country error names no parameter, only its own code. + // The country error names no parameter, only its own code. A phone another + // user holds also names `phone_number`, and fails the user. const phoneRefused = error instanceof BapiError && - (error.code === "unsupported_country_code" || error.meta?.param_name === "phone_number"); + (error.code === "unsupported_country_code" || + (error.code === "form_param_format_invalid" && error.meta?.param_name === "phone_number")); if (!phoneRefused || !identifiers.primaryEmail) throw error; const { phone_number: _dropped, ...withoutPhone } = body; response = await create(withoutPhone); From 45f39efcbe8ecbe9187045b69c255646894e7fcb Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 16:38:29 -0400 Subject: [PATCH 26/27] fix(migrate): halt the import when a run line cannot be written A failed write was taken as a create with an unknown outcome, though the request never went out, and a throw after a create rejected the import while the other users kept sending. Now the first failed write stops every create and attach still to go out, and the import rejects with it once every user has settled. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 40 +++++++++++++++++ .../src/commands/migrate/import-users.ts | 43 +++++++++++++++---- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index e2e321ff1..7925df6ad 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -684,6 +684,46 @@ describe("importUsers", () => { expect(performance.now() - started).toBeLessThan(1000); }); + test("rejects with a line it could not write, never sending that create", async () => { + stub(() => ok("user_created")); + const full = new Error("ENOSPC: no space left on device"); + + const run = importUsers({ + users: [user()], + secretKey: "sk_test_x", + limits: LIMITS, + record: (line) => { + if (line.status === "creating") throw full; + record(line); + }, + }); + + // Not a failure with an unknown outcome: the request never went out. + await expect(run).rejects.toBe(full); + expect(requests).toHaveLength(0); + expect(allLines).toHaveLength(0); + }); + + test("after a line it could not write, sends nothing more before rejecting", async () => { + stub(() => ok("user_created")); + const full = new Error("ENOSPC: no space left on device"); + + const run = importUsers({ + users: ["u1", "u2", "u3"].map((userId) => user({ userId, email: `${userId}@x.dev` })), + secretKey: "sk_test_x", + // u2 waits a paced turn, which the halt ends. + limits: { ...LIMITS, concurrencyLimit: 1, rateLimit: 2 }, + record: (line) => { + if (line.status === "created") throw full; + record(line); + }, + }); + + await expect(run).rejects.toBe(full); + await Bun.sleep(700); + expect(requests.filter((r) => r.url.endsWith("/v1/users"))).toHaveLength(1); + }); + test("records a failed user and keeps going", async () => { stub((_url, attempt) => attempt === 1 ? clerkError(422, "that email is taken") : ok("user_ok"), diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index 045fa898c..f2e9395d7 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -176,7 +176,9 @@ type CreateContext = { schedule: ApiScheduler; /** Aborted by the first `user_quota_exceeded`: every later create would be refused too. */ quota: AbortController; - /** Aborted by a Ctrl-C or the quota. No create goes out after it. */ + /** Aborted by a run-store write that failed. Nothing goes out after it. */ + halt: AbortSignal; + /** Aborted by a Ctrl-C, the quota or a halt. No create goes out after it. */ stop: AbortSignal; }; @@ -237,9 +239,9 @@ async function attachIdentifier( ? { user_id: clerkUserId, email_address: value, primary: false, verified } : { user_id: clerkUserId, phone_number: value, primary: false, verified }; - // A Ctrl-C ends the wait for a slot or a backoff, and the attach is left - // pending for a re-run. The quota does not: the user exists already. - const stop = interruptSignal(); + // A Ctrl-C or a halt ends the wait for a slot or a backoff, and the attach is + // left pending for a re-run. The quota does not: the user exists already. + const stop = AbortSignal.any([interruptSignal(), ctx.halt]); try { await retryOn429( async () => @@ -375,7 +377,7 @@ export async function importUsers(options: ImportUsersOptions): Promise(); + // A line that cannot be written halts the import: a user created with no + // record is beyond both `undo` and a re-run. The error is rethrown once + // every user has settled, so no request goes out after the import rejects. + const halt = new AbortController(); + let storeFailure: { error: unknown } | undefined; + const record = (line: UserLine) => { + try { + recordLine(line); + } catch (error) { + storeFailure ??= { error }; + halt.abort(); + throw error; + } + }; + const quota = new AbortController(); const ctx: CreateContext = { secretKey, schedule: createApiScheduler(limits.concurrencyLimit, limits.rateLimit), quota, - stop: AbortSignal.any([interruptSignal(), quota.signal]), + halt: halt.signal, + stop: AbortSignal.any([interruptSignal(), quota.signal, halt.signal]), }; const progress = () => report?.({ done: processed, ok: successful, failed }); @@ -448,8 +466,8 @@ export async function importUsers(options: ImportUsersOptions): Promise createUser(ctx, user, identifiers, skipPasswordRequirement, () => { - sent = true; record({ sourceId: user.userId, status: "creating" }); + sent = true; }), { signal: ctx.stop, @@ -460,6 +478,8 @@ export async function importUsers(options: ImportUsersOptions): Promise processUser(user))); + await Promise.all( + users.map(async (user) => + processUser(user).catch((error: unknown) => { + if (!storeFailure) throw error; + }), + ), + ); + if (storeFailure) throw storeFailure.error; return { totalProcessed: total, From 27762df10cfaa13c77e9a3da06859aea6297b668 Mon Sep 17 00:00:00 2001 From: Roy Anger Date: Tue, 13 Oct 2026 16:39:06 -0400 Subject: [PATCH 27/27] fix(migrate): fail a user a stop catches after Clerk refused its create Only a 429 or a refused phone is retried, so a stop between attempts leaves a user Clerk does not hold. Recorded as failed, a re-run creates it without looking it up first. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/migrate/import-users.test.ts | 16 ++++++++++------ .../src/commands/migrate/import-users.ts | 18 ++++++++++++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/cli-core/src/commands/migrate/import-users.test.ts b/packages/cli-core/src/commands/migrate/import-users.test.ts index 7925df6ad..baae3b284 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -245,9 +245,9 @@ describe("importUsers", () => { } }); - // Its `creating` line already marks it unfinished; `notSent` is for users - // with no line at all. - test("does not count a user whose first create went out as not sent", async () => { + // `notSent` is for users with no line at all. A create Clerk refused leaves + // no user, so a re-run need not look it up. + test("fails a user stopped after its first create was refused, not as not sent", async () => { stub(() => { abortInFlight(); return clerkError(429, "slow down", { "retry-after": "5" }); @@ -260,10 +260,14 @@ describe("importUsers", () => { record, }); - expect(summary).toMatchObject({ successful: 0, failed: 0, notSent: 0 }); + expect(summary).toMatchObject({ successful: 0, failed: 1, notSent: 0 }); expect(requests).toHaveLength(1); - expect(lines).toHaveLength(0); - expect(allLines.at(-1)).toMatchObject({ sourceId: "u1", status: "creating" }); + expect(allLines.at(-1)).toMatchObject({ + sourceId: "u1", + status: "failed", + code: "stopped", + error: expect.stringContaining("Stopped before the user was created"), + }); } finally { _resetInterruptState(); } diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index f2e9395d7..f09b838c4 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -480,11 +480,21 @@ export async function importUsers(options: ImportUsersOptions): Promise