fix(migrate): count users a continue only attached to apart from imports

.changeset/migrate-attached-not-imported.md

@@ -0,0 +1,5 @@
+---
+"clerk": patch
+---
+
+A continue of `clerk migrate import` no longer counts a user it only attached identifiers to as imported. Once every pending identifier has attached, its summary lists those users under "Finished attaching" and `--json` under `result.attached`, the line that opens the continue no longer counts them among the users left alone, and the progress bar counts the continue's users as finished rather than created. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`.

packages/cli-core/src/commands/migrate/README.md

@@ -70,7 +70,8 @@ Every command follows these:
 
    A spinner takes the keyboard and exits 0 on Ctrl-C, so a script that deletes
    the export once the import succeeds would delete it after an interrupted
-   one. Without a terminal, the counts are printed at each 10%.
+   one. Without a terminal, the counts are printed at each 10%. A continue with
+   identifiers left to attach counts its users as finished rather than created.
 
 ## Targeting and auth
 
@@ -825,7 +826,9 @@ A continued run also finishes what the last one left open:
   all is it created.
 
 - A user whose `created` line has `pending` identifiers gets just those
-  attaches.
+  attaches. The run imported it before, so once every identifier left to
+  attach has attached, the summary counts it under "Finished attaching"
+  (`result.attached` in `--json`), not "Imported" (`result.created`).
 
 Every create sends the run's ID in the user's private metadata, as
 `clerkMigrateRun`, merged with any private metadata the source carries. It

packages/cli-core/src/commands/migrate/import-users.test.ts

@@ -918,16 +918,51 @@ describe("importUsers", () => {
       progress: ({ done, ok }) => reported.push({ done, ok }),
     });
 
-    // Created by the run's earlier attempt: the bar and the summary agree.
+    // Done on the bar, but created by the run's earlier attempt: the summary
+    // counts it as attached, not as imported again.
     expect(reported.at(-1)).toEqual({ done: 1, ok: 1 });
-    expect(summary.successful).toBe(1);
+    expect(summary).toMatchObject({ successful: 0, attached: 1 });
+  });
+
+  // One identifier attaches and the other stays pending: the user isn't finished.
+  test("an attach-only user with an identifier still pending is not counted as attached", async () => {
+    // No answer: left pending, with no note to say why.
+    stub((url) =>
+      url.endsWith("/v1/phone_numbers") ? clerkError(503, "unavailable") : ok("idn_1"),
+    );
+    const reported: { done: number; ok: number }[] = [];
+
+    const summary = await importUsers({
+      users: [],
+      attachOnly: [
+        {
+          sourceId: "u1",
+          clerkId: "user_1",
+          status: "created",
+          pending: [
+            { kind: "email", value: "b@x.dev", verified: true },
+            { kind: "phone", value: "+15555550100", verified: false },
+          ],
+        },
+      ],
+      secretKey: "sk_test_x",
+      limits: LIMITS,
+      record,
+      progress: ({ done, ok }) => reported.push({ done, ok }),
+    });
+
+    expect(summary).toMatchObject({ successful: 0, attached: 0 });
+    expect(reported.at(-1)).toEqual({ done: 1, ok: 0 });
+    expect(lines.at(-1)?.pending).toEqual([
+      { kind: "phone", value: "+15555550100", verified: false },
+    ]);
   });
 
   // The pending phone attaches now, but the email Clerk refused stays refused.
   test("an attach-only user keeps the error its earlier line recorded", async () => {
     stub(() => ok("idn_1"));
 
-    await importUsers({
+    const summary = await importUsers({
       users: [],
       attachOnly: [
         {
@@ -949,6 +984,8 @@ describe("importUsers", () => {
       error: "b@x.dev: That email address is taken.",
     });
     expect(lines.at(-1)?.pending).toBeUndefined();
+    // The earlier refusal was reported then. What was left to attach has now attached.
+    expect(summary.attached).toBe(1);
   });
 
   test("an adopted user is not created again; only its extras attach", async () => {

packages/cli-core/src/commands/migrate/import-users.ts

@@ -615,7 +615,10 @@ export async function importUsers(options: ImportUsersOptions): Promise<ImportSu
   // Attach-only users count towards the bar too: a continue with only those
   // left would otherwise sit at 0/0.
   let attachedUsers = 0;
-  const progress = () => report?.({ done: processed + attachedUsers, ok: successful, failed });
+  // Of those, the ones whose identifiers all attached.
+  let finishedAttaching = 0;
+  const progress = () =>
+    report?.({ done: processed + attachedUsers, ok: successful + finishedAttaching, failed });
 
   const recordFailure = (
     userId: string,
@@ -647,6 +650,9 @@ export async function importUsers(options: ImportUsersOptions): Promise<ImportSu
   /**
    * Attaches a created user's extra identifiers. The user is already on record
    * with them `pending`, so a run stopped before they attach can finish them.
+   *
+   * @returns Whether every identifier this attempt tried attached: none
+   *   refused, none pending. `notes` from an earlier attempt don't count.
    */
   const finishUser = async (line: UserLine, toAttach: PendingIdentifier[], notes: string[]) => {
     const { error: _error, pending: _pending, refused: earlier = [], ...base } = line;
@@ -668,6 +674,7 @@ export async function importUsers(options: ImportUsersOptions): Promise<ImportSu
         ...(attached.pending.length > 0 ? { pending: attached.pending } : {}),
       });
     }
+    return attached.pending.length === 0 && attached.notes.length === 0;
   };
 
   /**
@@ -820,10 +827,11 @@ export async function importUsers(options: ImportUsersOptions): Promise<ImportSu
     const earlier = (line.error ?? "")
       .split("; ")
       .filter((note) => note && !note.startsWith(NOT_ADDED_YET));
-    await finishUser(line, line.pending ?? [], earlier);
-    // Created by this run's earlier attempt, as an adopted user was: the bar,
-    // the summary and `result.created` all count it.
-    successful++;
+    if (await finishUser(line, line.pending ?? [], earlier)) {
+      finishedAttaching++;
+    }
+    // Created by this run's earlier attempt: done on the bar, but counted apart
+    // from the users this attempt creates.
     attachedUsers++;
     progress();
   };
@@ -841,6 +849,7 @@ export async function importUsers(options: ImportUsersOptions): Promise<ImportSu
   return {
     totalProcessed: total,
     successful,
+    attached: finishedAttaching,
     failed,
     notSent,
     ...(stopReason ? { stopReason } : {}),

packages/cli-core/src/commands/migrate/lib/progress.ts

@@ -20,7 +20,7 @@ export type ProgressUpdate = (counts: ProgressCounts) => void;
 
 type ProgressLine = {
   total: number;
-  /** What happened to the users counted in `ok`: "created", "deleted". */
+  /** What happened to the users counted in `ok`: "created", "finished", "deleted". */
   verb: string;
   counts: ProgressCounts;
   elapsedMs: number;

packages/cli-core/src/commands/migrate/run.test.ts

@@ -2189,6 +2189,83 @@ describe("run", () => {
       expect(JSON.parse(captured.out)).toMatchObject({ consent: "required", attachOnly: 1 });
     });
 
+    // u1 created with its second email left to attach, u2 refused: the
+    // continue creates one user and only attaches to the other.
+    async function partialRun(): Promise<void> {
+      fs.writeFileSync(
+        path.join(workDir, "export.json"),
+        JSON.stringify([
+          { id: "u1", primary_email_address: "a@x.dev", unverified_email_addresses: "b@x.dev" },
+          { id: "u2", primary_email_address: "c@x.dev" },
+        ]),
+      );
+      stubClerk({ failing: new Set(["u2"]) });
+      const clerk = globalThis.fetch;
+      globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) =>
+        new URL(input.toString()).pathname === "/v1/email_addresses"
+          ? new Response("boom", { status: 503 })
+          : clerk(input, init)) as typeof fetch;
+      await run(baseOptions);
+      process.exitCode = 0;
+      captured.clear();
+      stubClerk();
+    }
+
+    test("a continue counts a user it only attaches to apart from the ones it creates", async () => {
+      await partialRun();
+
+      await run(baseOptions);
+
+      const err = Bun.stripANSI(captured.err);
+      expect(err).toContain("1 user already imported gets the identifiers left to attach.");
+      expect(err).not.toContain("left alone");
+      expect(err).toContain("Imported: 1");
+      expect(err).toContain("Finished attaching: 1");
+      expect(err).not.toContain("Identifiers left to attach");
+    });
+
+    // Still pending after the continue: nothing finished, and the summary
+    // says what's left instead.
+    test("a continue doesn't count an attach that fails again as finished", async () => {
+      await partialRun();
+      const clerk = globalThis.fetch;
+      globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) =>
+        new URL(input.toString()).pathname === "/v1/email_addresses"
+          ? new Response("boom", { status: 503 })
+          : clerk(input, init)) as typeof fetch;
+
+      await run(baseOptions);
+
+      const err = Bun.stripANSI(captured.err);
+      expect(err).not.toContain("Finished attaching");
+      expect(err).toContain("Identifiers left to attach: 1 user");
+    });
+
+    // Refused, not attached: nothing is pending, but nothing finished either.
+    test("a continue doesn't count an attach Clerk refuses as finished", async () => {
+      await partialRun();
+      const clerk = globalThis.fetch;
+      globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) =>
+        new URL(input.toString()).pathname === "/v1/email_addresses"
+          ? Response.json(
+              { errors: [{ code: "form_identifier_exists", message: "That email is taken." }] },
+              { status: 422 },
+            )
+          : clerk(input, init)) as typeof fetch;
+
+      await run(baseOptions);
+
+      expect(Bun.stripANSI(captured.err)).not.toContain("Finished attaching");
+    });
+
+    test("a continue puts a user it only attaches to under result.attached in --json", async () => {
+      await partialRun();
+
+      await run({ ...baseOptions, json: true });
+
+      expect(JSON.parse(captured.out).result).toMatchObject({ created: 1, attached: 1 });
+    });
+
     // A skip the checks named a fix for imports once the setting changes: the
     // import offers the same continue `runs <id>` does, and it finishes the run.
     test("a run left partial only by skips offers the continue, which finishes it", async () => {

packages/cli-core/src/commands/migrate/run.ts

@@ -697,6 +697,27 @@ function checksJson(checks: ImportChecks) {
   };
 }
 
+/**
+ * What a continue does with the users the run already imported: most are left
+ * alone, and those with identifiers still pending get them attached.
+ */
+function continuedUsers(imported: number, attaching: number): string {
+  const leftAlone = imported - attaching;
+  const parts = [
+    ...(leftAlone > 0 || attaching === 0
+      ? [
+          `${plural(leftAlone, "user")} already imported ${leftAlone === 1 ? "is" : "are"} left alone`,
+        ]
+      : []),
+    ...(attaching > 0
+      ? [
+          `${plural(attaching, "user")} already imported ${attaching === 1 ? "gets" : "get"} the identifiers left to attach`,
+        ]
+      : []),
+  ];
+  return parts.join(", and ");
+}
+
 function formatSummary(
   summary: ImportSummary,
   skipped: number,
@@ -706,6 +727,8 @@ function formatSummary(
 ): string[] {
   const lines = [
     `${green("Imported:")} ${summary.successful}`,
+    // Imported by the run's earlier attempt, so not counted again above.
+    ...(summary.attached > 0 ? [`${green("Finished attaching:")} ${summary.attached}`] : []),
     `${red("Failed:")} ${summary.failed}`,
   ];
   if (skipped > 0) lines.push(`${yellow("Skipped:")} ${skipped}`);
@@ -1171,7 +1194,7 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi
         log.info(
           resume.kind === "continue"
             ? `${options.dryRun ? "Would continue" : "Continuing"} run ${resume.record.id}, which ${resume.because === "interrupted" ? "was interrupted" : "finished partial"}: ` +
-                `${plural(done.size, "user")} already imported ${done.size === 1 ? "is" : "are"} left alone.`
+                `${continuedUsers(done.size, attachOnly.length)}.`
             : options.dryRun
               ? "Would start a new run."
               : "Starting a new run.",
@@ -1561,7 +1584,12 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi
       const summary: ImportSummary =
         checks.importable.length > 0 || attachOnly.length > 0
           ? await withProgress(
-              { total: checks.importable.length + attachOnly.length, verb: "created" },
+              {
+                total: checks.importable.length + attachOnly.length,
+                // An attach-only user isn't created, so "created" would
+                // contradict "Imported" in the summary.
+                verb: attachOnly.length > 0 ? "finished" : "created",
+              },
               async (progress) =>
                 importUsers({
                   users: checks.importable,
@@ -1580,6 +1608,7 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi
           : {
               totalProcessed: 0,
               successful: 0,
+              attached: 0,
               failed: 0,
               notSent: 0,
               droppedPhones: new Map(),
@@ -1613,6 +1642,7 @@ async function runImport(rawOptions: MigrateRunOptions, lock: ImportLock): Promi
               ...leftOut,
               result: {
                 created: summary.successful,
+                attached: summary.attached,
                 failed: summary.failed,
                 notSent: summary.notSent,
                 pending: record.counts.pending ?? 0,

packages/cli-core/src/commands/migrate/types.ts

@@ -45,6 +45,12 @@ export type User = z.infer<typeof userSchema>;
 export type ImportSummary = {
   totalProcessed: number;
   successful: number;
+  /**
+   * Users the run's earlier attempt created whose remaining identifiers this
+   * one attached, every one of them: none it tried was refused, and none is
+   * left pending. Not in `successful`: they were imported before.
+   */
+  attached: number;
   failed: number;
   /**
    * Users never sent because a Ctrl-C or the instance's user quota stopped