diff --git a/.changeset/migrate-runs-undo.md b/.changeset/migrate-runs-undo.md new file mode 100644 index 0000000000..2c498fb32e --- /dev/null +++ b/.changeset/migrate-runs-undo.md @@ -0,0 +1,7 @@ +--- +"clerk": minor +--- + +Add `clerk migrate runs` and `clerk migrate undo`, and let a re-run of `clerk migrate import` continue where a stopped one left off. `clerk migrate` is still experimental and needs `CLERK_EXPERIMENTAL=migrate`. + +Every user an import creates now carries `clerkMigrateRun` (the run's ID) in its `private_metadata`, replacing any value the source had under that key. A re-run reads it to adopt a user whose create was cut off. Nothing removes it afterwards, and your backend can read it. diff --git a/packages/cli-core/src/commands/migrate/README.md b/packages/cli-core/src/commands/migrate/README.md index f285935815..bf28cdaf35 100644 --- a/packages/cli-core/src/commands/migrate/README.md +++ b/packages/cli-core/src/commands/migrate/README.md @@ -15,7 +15,9 @@ Migrate users into a Clerk instance from another auth provider, or from another Clerk instance. ``` -clerk migrate import [--source ] [--dry-run] [--allow-partial] [--yes] [--json] +clerk migrate import [--source ] [--dry-run] [--allow-partial] [--new-run] [--yes] [--json] +clerk migrate runs [run-id] [--json] +clerk migrate undo [--dry-run] [--yes] [--include-signed-in] [--json] clerk migrate help ``` @@ -26,30 +28,36 @@ clerk migrate import users.json --source supabase --dry-run # 1. check it agai clerk migrate import users.json --source supabase --yes # 2. import it ``` -`--runs-dir ` (or `CLERK_MIGRATE_DIR`) keeps runs somewhere else. +`clerk migrate undo ` takes an import back out, and `clerk migrate runs` +shows what every run did. Every subcommand takes `--runs-dir ` (or +`CLERK_MIGRATE_DIR`) to keep its runs somewhere else. `clerk migrate` on its own is a group name, not a command: it prints its help. ## The rules Every command follows these: -1. **Nothing writes without consent.** Consent is a yes at a terminal prompt, or - `--yes`. Without either, `import` prints what it would do and exits 2 with - the command to run. `--json` means non-interactive: it never prompts. +1. **Nothing writes to Clerk without consent.** Consent is a yes at a terminal + prompt, or `--yes`. Without either, `import` and `undo` print what they would + do and exit 2 with the command to run. `--json` means non-interactive: it + never prompts. A run with nothing left to send is still marked finished, which + touches only its own record. 2. **`--dry-run` checks against the real instance, and writes nothing.** An import's [checks](#checks) run before anything is written. Predicted rejects stop the import unless `--allow-partial` is passed; fields that would be dropped are warnings. 3. **State lives in one place: the [run store](#the-run-store).** Each run records its target, its file, and every source ID → Clerk ID outcome, - including the error for each user who failed. + including the error for each user who failed. `runs`, `undo` and re-runs all + read or write it. 4. **Every command prints its target first:** the environment, app and instance, and where the key came from. 5. **Every subcommand takes `--json`.** Exit codes: `0` all good, `1` some users failed, `2` a usage error or a refusal, and `130` (death by SIGINT) when - Ctrl-C stops an import partway. The UI goes to stderr and data to stdout. + Ctrl-C stops an import or an undo partway. The UI goes to stderr and data to + stdout. - While users are created, a terminal shows a bar and the counts under it, not + While users are created or deleted, a terminal shows a bar and the counts under it, not a spinner: ``` @@ -77,30 +85,33 @@ Resolution order: `--secret-key` → `--app` + Platform API lookup → from `clerk link`. With a key from `--secret-key` or `CLERK_SECRET_KEY`, the key alone picks the -instance, so `import` refuses (exit 2) an `--instance` that names a different -one. `--instance dev` next to an exported `sk_live_…` key would otherwise write +instance, so `import` and `undo` refuse (exit 2) an `--instance` that names a +different one. `--instance dev` next to an exported `sk_live_…` key would otherwise write to production. The **instance type is read from the key**: `sk_live_…` is treated as production, anything else as development. That choice drives the throughput defaults and the development-instance user limit below. -**Every command prints its target first.** `import` names the instance — its +**Every command prints its target first.** `import` and `undo` name the +instance — its environment, its app when the key came from one, and its ID from `GET /v1/instance` — and where the key came from: `--secret-key`, `--app`, the `CLERK_SECRET_KEY` env var, an accountless app's `.env.local`, or the linked -profile. `--json` carries the same facts as `target`. +profile. `runs` names the runs folder. `--json` carries the same facts as +`target`. ``` Target: My App (app_2x9k…), production instance ins_2x9k… Key from: linked profile ``` -The instance ID is what a run records. +The instance ID is what a run records, so `undo` and re-runs can tell whether +the key now in use still addresses the same instance. ## The run store -Every import that gets as far as writing is a **run**, and the run store is +Every import and undo that gets as far as writing is a **run**, and the run store is the one place `clerk migrate` keeps state. A dry run, a refusal, a run that needs consent and an empty file write none (`run: null`). An import whose users `--require-password` all leaves out still writes one, recording them @@ -126,26 +137,37 @@ Each run is a folder named for its ID, `YYYYMMDD-HHmmss-xxxx`: | File | Contents | | -------------- | -------------------------------------------------------------------------------------------------------------------------------------- | -| `run.json` | Kind, status, start and finish times, the target, the source, the file and its sha256, and the counts | +| `run.json` | Kind, status, start and finish times, the target, the source, the file and its sha256, an import's flags, and the counts | | `users.ndjson` | One line per user outcome: `sourceId`, `clerkId`, `status`, and `reason`, `error`, `code`, `pending` or `passwordDropped` when present | | `lock` | The PID of the process writing the run, while it runs | -A user's status is `creating`, `created`, `failed` or `skipped`. The last line +On a `failed` line, `code` is the HTTP status, such as `422` or `429`, or +`unknown`. On a `skipped` line it gives the reason in machine form: +`no_password` for a user `--require-password` left out, or `in_file` for one +the checks skipped for the file's own data (invalid, a duplicate within it, an +email Clerk refuses, a malformed hash). + +A user's status is `creating`, `created`, `failed`, `skipped` or `deleted`. The last line for each `sourceId` wins. A `429` retry, an extra email or phone that did not attach, a first phone Clerk refused (which the summary also counts), and a validation failure all land in the line's `error` field. -`creating` is written as a user's `POST /v1/users` goes out. It stays the -latest line when no answer says whether the create landed: an abort, a -network error, or a 5xx. A `created` line with `pending` lists the extra emails -and phones not yet attached. - -A run is `partial` when any user failed, was skipped, is still `creating` or -was never sent (`counts.notSent`), and `complete` otherwise. A run interrupted -with Ctrl-C stays `running`, with no `finishedAt`. Its run ID and folder are -printed as the run starts, so they are on screen however it ends. A lock -holding this process's own PID is stale: in a container the CLI often gets the -same PID every run. +`creating` is written as a user's `POST /v1/users` goes out. It stays the latest +status when no answer says whether the create landed: an abort, a network error, +or a 5xx. The import counts a network error or a 5xx as failed, and a second +`creating` line records its `error`. A `created` line with `pending` lists the +extra emails and phones not yet attached, and `counts.pending` (`result.pending` +in the import's `--json`) counts those users. + +A run is `partial` when any user failed, was skipped, is still `creating`, has +identifiers still `pending`, or was never sent (`counts.notSent`), and +`complete` otherwise. A run whose process died, or that never recorded a finish +time, lists as `interrupted`. A Ctrl-C leaves a run that way. Its run ID and +folder are printed as the run starts, so they are on screen however it ends. A +lock held by another live process refuses a second writer with exit 2, and names +the lock file to delete if that process is not a migrate run. A lock holding +this process's own PID is stale: in a container the CLI often gets the same PID +every run. Run folders are created owner-only (`0700`), because they hold user data. @@ -184,6 +206,7 @@ API. clerk migrate import users.json --source supabase --dry-run # check, write nothing clerk migrate import users.json --source supabase --yes # import clerk migrate import users.json --source clerk --allow-partial --yes +clerk migrate import users.json --source clerk --new-run --yes clerk migrate import users.json --source clerk --json --yes clerk migrate import users.json --source clerk --require-password --yes clerk migrate import users.json --source clerk --skip-legal-checks --yes @@ -199,6 +222,7 @@ clerk migrate import # a human is asked | `--source ` | Where the file came from: one of the [sources](#sources) | | `--dry-run` | Run the [checks](#checks) against the instance, and write nothing | | `--allow-partial` | Import the users that pass, and record the rest as skipped | +| `--new-run` | Start a new run instead of [continuing](#re-running) an earlier one | | `--require-password` | Import only users that carry a password digest | | `--skip-legal-checks` | Import users with no legal acceptance into an instance requiring it | | `-y, --yes` | Import without prompting | @@ -218,13 +242,14 @@ leaves out the file is asked for its path, and then for its source. An agent, a non-TTY run, or `--json` without the file or `--source` exits 2 naming what to pass. -**Nothing is written without consent.** After the checks, a human is asked +**Nothing is written to Clerk without consent.** After the checks, a human is asked `Import N users?`, and declining writes nothing. `--yes` skips the question. Without either — an agent, a non-TTY run, `--json` — the run prints the checks and exits 2 with the exact command to run. Printed commands shell-quote their paths, keep `--json`, and put `` in place of a secret key. -**Every run prints its target first**, then the checks. +**Every run prints its target first**, then which [case](#re-running) applies, +then the checks. Failures do not stop the run: each user's outcome is written to the [run](#the-run-store) and the import continues. A `429` backs off — @@ -232,22 +257,66 @@ honouring `Retry-After` when the response carries it — and retries up to 5 times before the user is recorded as failed. The command exits 1 if any user failed. -`--require-password` records each user it leaves out as `skipped`, so the run -ends `partial`. - -`--json` returns `{ target, run, checks, result }`. When a run stops before -importing, it carries one of `dryRun: true`, `refused: true`, -`consent: "required"` or `nothingToImport: true` in place of `result`, with -`run: null`. With `--require-password`, `withoutPassword` counts the users it -left out before the checks, so `checks.total` plus it is the file's size. +`--require-password` records each user it leaves out as `skipped`, with +`code: "no_password"`, so the run ends `partial`. A continue carries the flag +and leaves them out again, so when they are all that's left, neither the import +nor `runs ` suggests one. To import them, run the same import again without +the flag: it continues this run and creates them. A skip for the file's own data +(`code: "in_file"`) comes back on every continue too, so it isn't counted as +something to retry: fix the file, which makes a new run. + +`--json` returns `{ target, run, resume, checks, result }`. When a run stops +before importing, it carries one of `dryRun: true`, `refused: true`, +`consent: "required"` or `nothingToImport: true` in place of `result`, and a +file already imported in full returns `alreadyImported: true`. With +`--require-password`, `withoutPassword` counts the users it left out before the +checks, so on a first import `checks.total` plus it is the file's size. A +continue leaves out the users the run already created. A continue with +identifiers left to attach carries `attachOnly`: the users it attaches them to, +which skip the checks too. #### Re-running -Running the same import again starts a new run. The users the first run -created are rejected as [already in the instance](#checks), so -`--allow-partial --yes` imports the rest. A user an interrupted run left at -`creating` is rejected the same way when Clerk holds it, and imported when it -doesn't. +Running the same import again continues where it left off. The match is the +file's sha256, the source, and the instance ID; the latest matching import run +decides what happens: + +| Latest match | Re-running does | +| ----------------------------------------- | ---------------------------------------------------------------------- | +| none | a new run | +| interrupted (dead lock or no finish time) | continues the same run, skipping the users it created | +| `partial` | continues the same run, retrying the users that did not finish earlier | +| `complete` | nothing: prints "Already imported in run …" and exits 0 | +| `undone` | a new run | +| has an undo that did not finish | exits 2, naming the `clerk migrate undo` that finishes it | + +`--new-run` skips the lookup. A run another live process holds exits 2. When +Clerk cannot name the instance (`GET /v1/instance` failed, often from rate +limiting right after a large import) and a run of this file exists under a real +instance ID, it exits 2 rather than starting over: try again, or pass +`--new-run`. A continue whose run changed while it waited at the prompt, from +an undo or another continue, exits 2 too, with nothing written. + +A continued run also finishes what the last one left open: + +- A user still `creating` is looked up by `external_id`. A match that + carries this run's marker is adopted as `created`, and not created again. + A match without the marker is someone else's user: the checks reject that + record as already in the instance. Only when the lookup finds no user at + all is it created. + +- A user whose `created` line has `pending` identifiers gets just those + attaches. + +Every create sends the run's ID in the user's private metadata, as +`clerkMigrateRun`, merged with any private metadata the source carries. It +replaces a `clerkMigrateRun` the source already has, from a run that imported +the user into the source instance. It is how a cut-off create is told apart +from a user an app or another tool made with the same `external_id`, and it +stays on the user. A run recorded before the marker existed adopts nothing. + +When an import completes, it names its run folder, which only `undo` needs and +which holds user data, with the `rm -rf` to remove it. #### Checks @@ -306,7 +375,8 @@ after them. They sort the users three ways: either holds, and the reject names it (`kept: …`) - the instance already has a user with its source ID, email, phone or username (a batched `GET /v1/users` lookup, 100 values a request, through - the scheduler) + the scheduler). A user a continued run found behind its own interrupted + create does not count - a development instance: it is past the 100-user headroom (`CLERK_MIGRATE_DEV_USER_LIMIT` when Clerk raised it), counted in file order @@ -368,7 +438,7 @@ additional verified identifier, and every unverified one, is attached afterwards with its own request, ahead of any create still queued, and backs off on a `429` like the create. A refusal there is logged and the user still counts as imported — a duplicate secondary email should not undo an otherwise -successful user. An attach with no answer stays `pending` on the user's line. +successful user. An attach with no answer stays `pending` for a re-run. The first phone gets the same treatment when Clerk refuses it — a country the instance does not support, or a number that is not E.164 — and the user has an @@ -402,13 +472,156 @@ Users that do exceed the limit come back in the error breakdown as `You have reached your limit of N users`, annotated with what a development instance can do about it. The first refusal stops the import: the users it never sent are counted under "Not sent" (`result.notSent` in `--json`, and -`counts.notSent` in `run.json`). Once the limit is raised, -[run the import again](#re-running) with `--allow-partial --yes` to send them. +`counts.notSent` in `run.json`), written when the run finishes. A run stopped +with Ctrl-C never finishes, so it can't count them, and `runs ` offers its +continue instead. Once the limit is raised, [run the import again](#re-running) +to send them. + +### `clerk migrate runs` + +`runs` reads the [run store](#the-run-store). + +```sh +clerk migrate runs # every run, newest first +clerk migrate runs 20260929-141502-a1b2 # one run in full +clerk migrate runs --json +``` + +| Flag | Description | +| ------------------- | ----------------------------- | +| `[run-id]` | Show one run instead of all | +| `--json` | The same data, on stdout | +| `--runs-dir ` | Read runs from somewhere else | + +It prints the runs folder first. The listing shows each run's ID, date, kind, +status, target, file and counts. `runs ` adds the error breakdown and the +users that failed, were skipped, or are `creating` with no answer to confirm the +create (`unconfirmed` in `--json`), with the path to the full record, then what +to run next: for an interrupted or partial import, the import that continues it, +with the flags it ran with, then the undo. The continue is left out when +importing the file again might not reach this run: a newer run of the same file +and source that may be for the same instance (a run under a key's stand-in ID +could be for any instance), a file changed since, or any undo of it. It is also +left out when only `--require-password` skips, or skips for the file's own +data, are left, or for a run from an earlier build, whose flags can't be read. +Both carry what reaches +the run's instance again, from the key its latest attempt used: `--secret-key +` for a key passed with `--secret-key` or exported (as `CLERK_SECRET_KEY`, +or a framework's own variable such as `NUXT_CLERK_SECRET_KEY`), and `--app` and +`--instance` for `--app` or a linked profile (`--secret-key ` when the run +is under a stand-in ID: no `--instance` resolves one, and a bare `--app` +resolves development). A key file (`.env`, or a keyless app's) gets no target +flags: it refuses `--instance`, and run from the same project it names the key +itself. The import's own next steps, and a partial undo's, carry the same. An +unknown ID exits 2. + +### `clerk migrate undo` + +Deletes the users an import run created. The import run is the whole record of +what to delete: every source ID whose latest line is `created`, by the Clerk ID +recorded beside it. + +The one search is for a source ID whose latest line is `creating`: the run +stopped with that user's `POST /v1/users` sent and unanswered, so Clerk may +hold the user without its ID on record. Those are looked up by `external_id`, +and only a user carrying the import run's `clerkMigrateRun` marker is deleted: +an app or another tool can make a user with the same source ID, and one +without the marker is left alone. + +```sh +clerk migrate undo 20260929-141502-a1b2 --dry-run # preview, delete nothing +clerk migrate undo 20260929-141502-a1b2 # confirms first +clerk migrate undo 20260929-141502-a1b2 --yes # no prompt +clerk migrate undo 20260929-141502-a1b2 --json --yes +``` + +| Flag | Description | +| --------------------- | -------------------------------------------------------------- | +| `` | The import run to undo | +| `--dry-run` | Show the preview and delete nothing | +| `-y, --yes` | Delete without prompting | +| `--include-signed-in` | With `--yes`, also delete users who signed in since the import | +| `--json` | Output as JSON. Never prompts, so deleting needs `--yes` | +| `--runs-dir ` | Read runs from somewhere else | + +Plus the targeting flags: `--secret-key`, `--app` and `--instance`. + +It prints the target first, then a preview: how many users will be deleted, and +how many of them have signed in since the import (from each user's +`last_sign_in_at`). When users remain to delete or record as gone, nothing is +deleted without consent: a yes at the prompt, or `--yes`. Without either — an +agent, a non-TTY run, or `--json` — it prints the preview and exits 2 with the +command to run. That command keeps `--runs-dir`, `--app`, `--instance` and +`--secret-key` (as ``) when they were passed, or it would look in another +runs folder or reach another instance. When none remain, undo completes +without prompting. + +`--yes` is consent to undo the import, not to delete accounts people have used +since: when any user has signed in after the import, `--yes` alone exits 2 and +deletes nothing, naming the command with `--include-signed-in` that deletes them +too. A yes at the prompt follows the preview's warning, so it needs no flag. +Right before the deletes, undo looks again, by user rather than by count: a user +who signed in after the preview was in no one's consent, so without +`--include-signed-in` it exits 2 and deletes nothing. + +It refuses with exit 2, and deletes nothing, when: + +- the resolved key addresses a different instance than the run imported into + (the error names both) +- the run is not an import run +- the run has already been undone +- the run, or an undo of it, changed while the preview waited for consent (a + re-import, or another undo): the preview is out of date, so run it again + +Deletes go through the same scheduler and `429` backoff as the import. A user +already gone from the instance counts as deleted. The undo is a run of its own, +`kind: "undo"` with `undoes: `. The import is marked `undone` only when +every user is deleted. A partial undo exits 1, and running `undo` again retries +the users that failed, in the same undo run. A Ctrl-C leaves the undo run +`interrupted`, as it does an import, and running `undo` again carries on. + +Once undo reaches its preview, `--json` prints one object on stdout, whatever +happens next, unless a Ctrl-C exits first (see `stopped` below). A refusal +before that — no such run, a run that isn't an import, +or a key for another instance — is an error, as it is for every command. The +object: + +```json +{ + "target": { "instanceId": "ins_…", "env": "development", "keySource": "--secret-key" }, + "import": { "id": "20260929-141502-a1b2", "kind": "import", … }, + "run": null, + "preview": { "toDelete": 2, "signedInSince": 1, "alreadyGone": 0, "alreadyDeleted": 0 } +} +``` + +`import` is the run being undone. `run` is the undo's own run, `null` until one +is written, as an import's is. Once the deletes run, `run` is the undo run, and +`result` holds `deleted`, `failed` and an `errors` breakdown. When undo stops +before that, the object says why: + +- `"dryRun": true`: a dry run. +- `"consent": "required"`: refused for consent. When + `"includeSignedIn": "required"` is beside it, `--yes` was given, but users + who signed in since the import also need `--include-signed-in`. +- `"changed": true`: refused because the run changed while the preview waited, + or another process holds it. +- `"recheckFailed": true`: the second look at sign-ins, right before the + deletes, failed. +- `"writeFailed": true`: the file system refused a write to the runs folder. +- `"stopped": true`: anything else stopped it. A Ctrl-C may exit before the + object is printed: `clerk migrate runs` then lists the undo run, if one was + written, and `runs ` shows how far it got. + +With `writeFailed` or `stopped`, `run` is the undo run as it stands on disk, +once one was written: its counts come from its lines, and `state` says how it +stopped, as in `clerk migrate runs`. ### `clerk migrate help` -`clerk migrate help` and `clerk migrate import --help` print the help for the -group or the command, with examples. +`clerk migrate help` and `clerk migrate --help` print the help for the +group or one command, with examples. `clerk migrate help ` does the +same. ## Sources @@ -461,7 +674,8 @@ warn about each one (`Clerk won't store: …`). The schema lives in `validator.ts`; adding a platform means adding a source, not editing it. -**Required:** `userId` (`string`). It becomes the Clerk user's `external_id`. +**Required:** `userId` (`string`). It becomes the Clerk user's `external_id`, +which is what makes a migration re-runnable. **Identifiers.** At least one of these must be present, or the import's checks reject the user as invalid. Each accepts a single value or an array. @@ -526,15 +740,19 @@ stamping every user with today's. ## API endpoints -| Method | Path | Used by | -| ------ | ------------------------- | ------------------------------------------------------------------------------ | -| `POST` | `/v1/users` | `migrate import` — creates each user | -| `POST` | `/v1/email_addresses` | `migrate import` — attaches additional emails | -| `POST` | `/v1/phone_numbers` | `migrate import` — attaches additional phones | -| `GET` | `/v1/users/count` | `migrate import` — headroom against a development instance's user limit | -| `GET` | `/v1/users?external_id=…` | `migrate import` — checks for users already in the instance, 100 values a call | -| `GET` | `/v1/instance` | `migrate import` — names the instance behind the key | -| `GET` | `/v1/domains` | `migrate import` checks — resolves the Frontend API host | +| Method | Path | Used by | +| -------- | ------------------------- | ------------------------------------------------------------------------------- | +| `POST` | `/v1/users` | `migrate import` — creates each user | +| `POST` | `/v1/email_addresses` | `migrate import` — attaches additional emails | +| `POST` | `/v1/phone_numbers` | `migrate import` — attaches additional phones | +| `GET` | `/v1/users/count` | `migrate import` — headroom against a development instance's user limit | +| `GET` | `/v1/users?external_id=…` | `migrate import` — checks for users already in the instance, 100 values a call | +| `GET` | `/v1/users?external_id=…` | `migrate import` — a continue looks up users whose create was in flight | +| `GET` | `/v1/users?external_id=…` | `migrate undo` — finds users whose create was in flight when the import stopped | +| `GET` | `/v1/users?user_id=…` | `migrate undo` — reads the imported users back, 100 a call | +| `DELETE` | `/v1/users/{user_id}` | `migrate undo` — deletes one user | +| `GET` | `/v1/instance` | `migrate import`, `undo` — names the instance behind the key | +| `GET` | `/v1/domains` | `migrate import` checks — resolves the Frontend API host | The checks also read the instance's Frontend API `GET /v1/environment` (bootstrapping a dev browser first on development instances) for its @@ -543,7 +761,8 @@ instance settings: the checks print the `clerk config patch` to run instead. ## Notes -- `userId` in the source file becomes the Clerk user's `external_id`. +- `userId` in the source file becomes the Clerk user's `external_id`. That is + what makes a migration re-runnable and reversible. - CSV input is coerced before validation: `a@x.dev,b@x.dev` and `["a@x.dev"]` both become arrays, `"true"`/`1` become booleans, and JSON metadata columns are parsed. An empty column is dropped rather than sent as null. 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 baae3b2842..9a0af5d242 100644 --- a/packages/cli-core/src/commands/migrate/import-users.test.ts +++ b/packages/cli-core/src/commands/migrate/import-users.test.ts @@ -5,10 +5,10 @@ import { useCaptureLog } from "../../test/lib/stubs.ts"; import { buildCreateUserBody, importUsers, - normalizeErrorMessage, readRetryAfter, splitIdentifiers, } from "./import-users.ts"; +import { normalizeErrorMessage } from "./lib/failures.ts"; import type { ResolvedLimits } from "./lib/instance.ts"; import type { UserLine } from "./lib/run-store.ts"; import type { User } from "./types.ts"; @@ -210,6 +210,42 @@ describe("importUsers", () => { headers, }); + // How a continue or an undo tells its own cut-off create from a user + // someone else made with the same external_id. + test("marks each create with its run, keeping the source's private metadata", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [user({ userId: "u1", privateMetadata: { plan: "pro" } })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + runId: "20260101-000000-abcd", + }); + + expect(requests.find((r) => r.url.endsWith("/v1/users"))?.body).toMatchObject({ + private_metadata: { plan: "pro", clerkMigrateRun: "20260101-000000-abcd" }, + }); + }); + + // A Clerk export of users an earlier migration imported carries that run's + // marker; this run's must win, or a cut-off create could never be adopted. + test("replaces a marker the source's private metadata already carries", async () => { + stub(() => ok("user_created")); + + await importUsers({ + users: [user({ userId: "u1", privateMetadata: { clerkMigrateRun: "20250101-000000-0000" } })], + secretKey: "sk_test_x", + limits: LIMITS, + record, + runId: "20260101-000000-abcd", + }); + + expect(requests.find((r) => r.url.endsWith("/v1/users"))?.body).toMatchObject({ + private_metadata: { clerkMigrateRun: "20260101-000000-abcd" }, + }); + }); + test("creates each user and reports them as successful", async () => { stub(() => ok("user_created")); @@ -327,14 +363,18 @@ describe("importUsers", () => { }); 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"); + // On record once created with its attach pending, in one line, then with + // what the attach added. A refused attach is not retried, so nothing is pending. + expect(lines).toHaveLength(2); + expect(lines[0]).toEqual({ + sourceId: "u1", + clerkId: "user_created", + status: "created", + pending: [{ kind: "email", value: "b@x.dev", verified: true }], + }); + expect(lines[1]).toMatchObject({ status: "created", clerkId: "user_created" }); + expect(lines[1]?.error).toContain("Failed to add additional email b@x.dev"); + expect(lines[1]).not.toHaveProperty("pending"); }); // A Ctrl-C mid-backoff should not wait out the pause, nor send the attach. @@ -380,7 +420,7 @@ describe("importUsers", () => { expect(lines.at(-1)).not.toHaveProperty("pending"); }); - test("keeps an attach with no answer pending", async () => { + 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"), ); @@ -421,6 +461,103 @@ 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" }); + }); + + // A continue with only attaches left would otherwise read 0/0 throughout. + test("counts an attach-only user towards progress once its attaches are sent", async () => { + stub(() => ok("idn_1")); + const reported: { done: number; ok: number }[] = []; + + const summary = 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, + progress: ({ done, ok }) => reported.push({ done, ok }), + }); + + // Created by the run's earlier attempt: the bar and the summary agree. + expect(reported.at(-1)).toEqual({ done: 1, ok: 1 }); + expect(summary.successful).toBe(1); + }); + + // 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({ + users: [], + attachOnly: [ + { + sourceId: "u1", + clerkId: "user_1", + status: "created", + error: "b@x.dev: That email address is taken.", + pending: [{ kind: "phone", value: "+15555550100", verified: false }], + }, + ], + secretKey: "sk_test_x", + limits: LIMITS, + record, + }); + + expect(lines.at(-1)).toMatchObject({ + sourceId: "u1", + status: "created", + error: "b@x.dev: That email address is taken.", + }); + expect(lines.at(-1)?.pending).toBeUndefined(); + }); + + 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([ @@ -604,7 +741,9 @@ describe("importUsers", () => { }); expect(summary.failed).toBe(1); - expect(allLines.map((line) => line.status)).toEqual(["creating"]); + // A second `creating` line, so `runs ` can say why. + expect(allLines.map((line) => line.status)).toEqual(["creating", "creating"]); + expect(allLines[1]?.error).toContain("a re-run checks"); expect([...summary.errorBreakdown.keys()][0]).toContain("a re-run checks"); }); @@ -619,7 +758,15 @@ describe("importUsers", () => { }); expect(summary).toMatchObject({ successful: 0, failed: 1 }); - expect(allLines).toEqual([{ sourceId: "u1", status: "creating" }]); + expect(allLines).toEqual([ + { sourceId: "u1", status: "creating" }, + { + sourceId: "u1", + status: "creating", + error: expect.stringContaining("without a user ID"), + code: "unknown", + }, + ]); }); // Every create after the first refusal would be refused too, so none is diff --git a/packages/cli-core/src/commands/migrate/import-users.ts b/packages/cli-core/src/commands/migrate/import-users.ts index f09b838c46..67c37867b8 100644 --- a/packages/cli-core/src/commands/migrate/import-users.ts +++ b/packages/cli-core/src/commands/migrate/import-users.ts @@ -21,61 +21,16 @@ 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 { failureOf, normalizeErrorMessage, tallyError } from "./lib/failures.ts"; import { RateLimitExceededError, retryOn429 } from "./lib/retry.ts"; import type { PendingIdentifier, UserLine } from "./lib/run-store.ts"; +import { RUN_MARKER_KEY } from "./lib/user-lookup.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]; @@ -174,6 +129,8 @@ export function buildCreateUserBody( type CreateContext = { secretKey: string; schedule: ApiScheduler; + /** The run sending these creates; each carries it as its marker. */ + runId?: string; /** Aborted by the first `user_quota_exceeded`: every later create would be refused too. */ quota: AbortController; /** Aborted by a run-store write that failed. Nothing goes out after it. */ @@ -320,6 +277,16 @@ async function createUser( ); const body = buildCreateUserBody(user, identifiers, skipPasswordRequirement); + // The run's marker, with whatever private metadata the source brought: + // without it a create cut off mid-flight could never be told from a user + // someone else made with the same external_id. It replaces a marker the + // source carries, from a run that imported the user there. + if (ctx.runId) { + body.private_metadata = { + ...(body.private_metadata as Record | undefined), + [RUN_MARKER_KEY]: ctx.runId, + }; + } const notes: string[] = []; let phoneRefusal: string | undefined; let response; @@ -356,6 +323,19 @@ 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[]; + /** The run these creates belong to, sent on each as its marker. */ + runId?: string; + /** + * 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. */ @@ -369,8 +349,7 @@ 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: Clerk may hold the user, and a re-run's - * checks look it up before creating it again. + * answer keeps its `creating` line, for a continued run or `undo` to resolve. */ export async function importUsers(options: ImportUsersOptions): Promise { const { @@ -378,6 +357,9 @@ export async function importUsers(options: ImportUsersOptions): Promise(), skipPasswordRequirement = true, validationFailed = 0, progress: report, @@ -414,9 +396,13 @@ export async function importUsers(options: ImportUsersOptions): Promise report?.({ done: processed, ok: successful, failed }); + // 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 }); const recordFailure = ( userId: string, @@ -427,24 +413,24 @@ export async function importUsers(options: ImportUsersOptions): Promise { 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 }); - } + tallyError(errorBreakdown, message); + // With no answer, the user stays `creating`: Clerk may hold it, and a + // re-run looks it up before creating it again. The line still says why. + record({ + sourceId: userId, + status: unknown ? "creating" : "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 records which - * never did. + * 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. */ 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. @@ -461,22 +447,25 @@ export async function importUsers(options: ImportUsersOptions): Promise - createUser(ctx, user, identifiers, skipPasswordRequirement, () => { - record({ sourceId: user.userId, status: "creating" }); - sent = true; - }), - { - signal: ctx.stop, - onRetry: ({ message, delaySeconds }) => { - retries.push(message); - ctx.schedule.pause(delaySeconds * 1000); - }, - }, - ); + created = adoptedId + ? { clerkUserId: adoptedId, notes: [] } + : await retryOn429( + async () => + createUser(ctx, user, identifiers, skipPasswordRequirement, () => { + record({ sourceId: user.userId, status: "creating" }); + sent = true; + }), + { + signal: ctx.stop, + onRetry: ({ message, delaySeconds }) => { + retries.push(message); + ctx.schedule.pause(delaySeconds * 1000); + }, + }, + ); } catch (error) { // Not an API outcome: the request never went out. if (storeFailure && error === storeFailure.error) throw error; @@ -497,10 +486,6 @@ export async function importUsers(options: ImportUsersOptions): Promise 0 ? { ...line, pending: toAttach } : line); if (created.phoneRefusal) { const reason = normalizeErrorMessage(created.phoneRefusal); droppedPhones.set(reason, (droppedPhones.get(reason) ?? 0) + 1); } - await finishUser(line, pendingIdentifiers(identifiers), [...created.notes, ...retries]); + await finishUser(line, toAttach, [...created.notes, ...retries]); successful++; processed++; progress(); }; progress(); - await Promise.all( - users.map(async (user) => - processUser(user).catch((error: unknown) => { - if (!storeFailure) throw error; - }), - ), - ); + const attachOnlyUser = async (line: UserLine) => { + // Its earlier error names identifiers Clerk refused; the new line keeps it. + await finishUser(line, line.pending ?? [], line.error ? [line.error] : []); + // Created by this run's earlier attempt, as an adopted user was: the bar, + // the summary and `result.created` all count it. + successful++; + attachedUsers++; + progress(); + }; + // A halt leaves its error to be rethrown below, once every user has settled. + const settle = async (work: Promise) => + work.catch((error: unknown) => { + if (!storeFailure) throw error; + }); + await Promise.all([ + ...users.map(async (user) => settle(processUser(user))), + ...attachOnly.map(async (line) => settle(attachOnlyUser(line))), + ]); if (storeFailure) throw storeFailure.error; return { diff --git a/packages/cli-core/src/commands/migrate/index.test.ts b/packages/cli-core/src/commands/migrate/index.test.ts index d26c5556f2..7d742c553d 100644 --- a/packages/cli-core/src/commands/migrate/index.test.ts +++ b/packages/cli-core/src/commands/migrate/index.test.ts @@ -31,8 +31,12 @@ describe("registerMigrate", () => { expect(migrate?.description()).toContain("Migrate users"); }); - test("registers import as its only subcommand", () => { - expect(findCommand(["migrate"])?.commands.map((cmd) => cmd.name())).toEqual(["import"]); + test("registers import, undo and runs", () => { + expect(findCommand(["migrate"])?.commands.map((cmd) => cmd.name())).toEqual([ + "import", + "undo", + "runs", + ]); }); test("leaves migrate with no default subcommand", () => { @@ -45,6 +49,7 @@ describe("registerMigrate", () => { "--source", "--dry-run", "--allow-partial", + "--new-run", "--require-password", "--skip-legal-checks", "--yes", @@ -56,6 +61,27 @@ describe("registerMigrate", () => { ]); }); + test("registers runs with an optional run ID", () => { + const runsCommand = findCommand(["migrate", "runs"]); + expect(runsCommand?.registeredArguments[0]?.required).toBe(false); + expect(runsCommand?.options.map((option) => option.long)).toEqual(["--json", "--runs-dir"]); + }); + + test("registers undo with a required run ID and its flags", () => { + const undoCommand = findCommand(["migrate", "undo"]); + expect(undoCommand?.registeredArguments[0]?.required).toBe(true); + expect(undoCommand?.options.map((option) => option.long)).toEqual([ + "--dry-run", + "--yes", + "--include-signed-in", + "--json", + "--secret-key", + "--app", + "--instance", + "--runs-dir", + ]); + }); + // No `.choices()`: an unknown key reaches resolveSource, whose error lists // the valid ones. Completion offers them through `KNOWN_OPTION_VALUES`. test("--source accepts any value", () => { diff --git a/packages/cli-core/src/commands/migrate/index.ts b/packages/cli-core/src/commands/migrate/index.ts index 4323a68930..cbf062084a 100644 --- a/packages/cli-core/src/commands/migrate/index.ts +++ b/packages/cli-core/src/commands/migrate/index.ts @@ -4,6 +4,8 @@ import { setMode } from "../../mode.ts"; import { setAssumeYes } from "./lib/assume-yes.ts"; import { RUNS_DIR_DESCRIPTION, RUNS_DIR_FLAG } from "./lib/run-store.ts"; import { run } from "./run.ts"; +import { runs } from "./runs.ts"; +import { undo } from "./undo.ts"; export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = process.env): void { if (!isExperimentEnabled("migrate", env)) { @@ -36,7 +38,12 @@ export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = proce }, { command: "clerk migrate import users.json --source supabase --yes", - description: "Import it", + description: "Import it. Run it again to continue after a failure", + }, + { command: "clerk migrate runs", description: "List every migration run" }, + { + command: "clerk migrate undo 20260929-141502-a1b2", + description: "Delete the users an import created", }, ]); @@ -65,6 +72,7 @@ export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = proce .option("--source ", "Where the file came from") .option("--dry-run", "Check the file against the instance, report, and write nothing") .option("--allow-partial", "Import the users that pass the checks, and skip the rest") + .option("--new-run", "Start a new run instead of continuing an earlier one of this file") .option("--require-password", "Import only users that have a password") .option( "--skip-legal-checks", @@ -92,4 +100,50 @@ export function registerMigrate(program: Program, env: NodeJS.ProcessEnv = proce ...(input ? { input } : {}), }), ); + + // Flat, not under a noun group: this is the one command in the tree that + // destroys data in Clerk, and it is worth keeping short and prominent. + migrateCommand + .command("undo") + .description("Delete the users an import run created") + .argument("", "The import run to undo (see `clerk migrate runs`)") + .option("--dry-run", "Show what would be deleted, and delete nothing") + .option("-y, --yes", "Delete without prompting") + .option("--include-signed-in", "With --yes, also delete users who signed in since the import") + .option("--json", "Output as JSON; never prompts, so pair it with --yes to delete") + .option("--secret-key ", "Backend API secret key to use") + .option("--app ", "Application ID to target (works from any directory)") + .option("--instance ", "Instance to target (dev, prod, or a full instance ID)") + .option(RUNS_DIR_FLAG, RUNS_DIR_DESCRIPTION) + .setExamples([ + { + command: "clerk migrate undo 20260929-141502-a1b2 --dry-run", + description: "Preview what would be deleted", + }, + { + command: "clerk migrate undo 20260929-141502-a1b2 --yes", + description: "Delete without prompting", + }, + ]) + .action(async (runId, _opts, cmd) => + undo(runId, cmd.optsWithGlobals() as Parameters[1]), + ); + + migrateCommand + .command("runs") + .description("List migration runs, or show one") + .argument("[run-id]", "A run to show in full") + .option("--json", "Output as JSON") + .option(RUNS_DIR_FLAG, RUNS_DIR_DESCRIPTION) + .setExamples([ + { command: "clerk migrate runs", description: "List every run, newest first" }, + { + command: "clerk migrate runs 20260929-141502-a1b2", + description: "Show one run: counts, errors and the users that did not make it", + }, + { command: "clerk migrate runs --json", description: "Machine-readable listing" }, + ]) + .action(async (runId, _opts, cmd) => + runs(runId, cmd.optsWithGlobals() as Parameters[1]), + ); } diff --git a/packages/cli-core/src/commands/migrate/lib/carry-flags.test.ts b/packages/cli-core/src/commands/migrate/lib/carry-flags.test.ts new file mode 100644 index 0000000000..ec2173abfa --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/carry-flags.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from "bun:test"; +import { carryFlags, undoCommand } from "./carry-flags.ts"; + +describe("carryFlags", () => { + test("keeps the runs folder and target, with the key as a placeholder", () => { + expect( + carryFlags({ secretKey: "sk_test_x", app: "app_1", instance: "dev", runsDir: "/a b" }), + ).toEqual([ + "--secret-key", + "", + "--app", + "app_1", + "--instance", + "dev", + "--runs-dir", + "'/a b'", + ]); + }); + + test("an undo command keeps them, after the run ID", () => { + expect(undoCommand("r1", carryFlags({ secretKey: "sk_test_x", runsDir: "/a b" }))).toBe( + "clerk migrate undo r1 --secret-key --runs-dir '/a b'", + ); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/lib/carry-flags.ts b/packages/cli-core/src/commands/migrate/lib/carry-flags.ts new file mode 100644 index 0000000000..c076702919 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/carry-flags.ts @@ -0,0 +1,72 @@ +import { quoteArg } from "../../../lib/json-body.ts"; + +/** + * The flags that chose a migrate command's runs folder and instance, for a + * command it suggests. Left out, the suggested command looks in the default + * runs folder or reaches another instance, and refuses: an agent that follows + * it loops on exit 2. Values are shell-quoted, and the secret key is a + * placeholder. + */ +export type CarryOptions = { + secretKey?: string; + app?: string; + instance?: string; + runsDir?: string; +}; + +export function carryFlags(options: CarryOptions): string[] { + return [ + ...(options.secretKey ? ["--secret-key", ""] : []), + ...(options.app ? ["--app", quoteArg(options.app)] : []), + ...(options.instance ? ["--instance", quoteArg(options.instance)] : []), + ...(options.runsDir ? ["--runs-dir", quoteArg(options.runsDir)] : []), + ]; +} + +/** + * The import flags that choose which users go in. The run records them, so + * the command that continues it, wherever it is suggested, repeats them: + * left out, `--require-password` no longer holds back the users it skipped. + */ +export type ImportFlags = { + allowPartial?: boolean; + requirePassword?: boolean; + skipLegalChecks?: boolean; +}; + +export function importFlags(options: ImportFlags): string[] { + return [ + ...(options.allowPartial ? ["--allow-partial"] : []), + ...(options.requirePassword ? ["--require-password"] : []), + ...(options.skipLegalChecks ? ["--skip-legal-checks"] : []), + ]; +} + +/** + * The `ImportFlags` fields of wider options, for the run to record. The return + * type names every key, so a new flag fails to compile until it is picked here. + */ +export function pickImportFlags( + options: ImportFlags, +): Record { + return { + allowPartial: options.allowPartial, + requirePassword: options.requirePassword, + skipLegalChecks: options.skipLegalChecks, + }; +} + +/** `clerk migrate runs [id]`, reading the runs folder these flags name. */ +export function runsCommand(carry: string[], id?: string): string { + return withFlags(id ? `clerk migrate runs ${id}` : "clerk migrate runs", carry); +} + +/** `clerk migrate undo ` with the flags that reach its runs folder and instance. */ +export function undoCommand(id: string, carry: string[]): string { + return withFlags(`clerk migrate undo ${id}`, carry); +} + +/** `--` flags appended to a command, or nothing. */ +export function withFlags(command: string, flags: string[]): string { + return flags.length > 0 ? `${command} ${flags.join(" ")}` : command; +} diff --git a/packages/cli-core/src/commands/migrate/lib/checks.test.ts b/packages/cli-core/src/commands/migrate/lib/checks.test.ts index 8ffc06774c..70539dc6e4 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.test.ts @@ -164,7 +164,9 @@ describe("rejects", () => { const checks = await checkImport( input({ failures: [{ userId: "bad", row: 0, error: "Invalid email", path: ["email"] }] }), ); - expect(checks.rejects).toEqual([{ sourceId: "bad", reason: "invalid: Invalid email" }]); + expect(checks.rejects).toEqual([ + { sourceId: "bad", reason: "invalid: Invalid email", inFile: true }, + ]); expect(checks.total).toBe(1); }); @@ -247,6 +249,7 @@ describe("rejects", () => { sourceId: "auth0|1", reason: "email is also used by an earlier user in the file, which is kept", keptSourceId: "email|1", + inFile: true, }, ]); }); @@ -307,11 +310,32 @@ describe("rejects", () => { // A repeated record (an export that paged past a sign-up) must not take the // kept copy down with it. + // Adopted means created by this run, but only for its own record: another + // user that shares its email would still clash at create. + test("an adopted user's identifiers still clash with other users", async () => { + existing = [ + { + id: "user_mine", + external_id: "mine", + email_addresses: [{ email_address: "mine@x.dev" }, { email_address: "shared@x.dev" }], + }, + ]; + expect( + await reasonsOf({ + users: [ + user("mine", { emailAddresses: ["shared@x.dev"] }), + user("other", { email: "shared@x.dev" }), + ], + adoptedClerkIds: new Set(["user_mine"]), + }), + ).toEqual({ other: "email is already used by a user in the instance" }); + }); + test("a repeated source ID rejects only the later copy", async () => { const checks = await checkImport(input({ users: [user("a"), user("a")] })); expect(checks.importable.map((u) => u.userId)).toEqual(["a"]); expect(checks.rejects).toEqual([ - { sourceId: "a", reason: "duplicate source ID in the file", keptSourceId: "a" }, + { sourceId: "a", reason: "duplicate source ID in the file", keptSourceId: "a", inFile: true }, ]); }); @@ -327,6 +351,15 @@ describe("rejects", () => { expect(checks.importable.map((u) => u.userId)).toEqual(["kept"]); }); + // A continued run found this user behind its own in-flight create. + test("not a user the continued run adopted", async () => { + existing = [{ id: "user_1", external_id: "mine" }]; + + expect( + await reasonsOf({ users: [user("mine")], adoptedClerkIds: new Set(["user_1"]) }), + ).toEqual({}); + }); + test("a supabase user whose only provider is disabled", async () => { const rows = [ { id: "only-discord", raw_app_meta_data: { providers: ["discord"] } }, @@ -449,6 +482,97 @@ describe("rejects", () => { expect(checks.quota).toEqual({ existing: 98, limit: 100, headroom: 2, over: 1 }); }); + // A continued run's adopted users exist already and are never created + // again: the live count has them, and only new users take headroom. + test("an adopted user takes no headroom, wherever it sits in the file", async () => { + const checks = await checkImport( + input({ + instanceType: "dev", + existingUsers: 99, + adoptedClerkIds: new Set(["user_c"]), + adoptedSourceIds: new Set(["c"]), + users: [user("a"), user("b"), user("c")], + }), + ); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["a", "c"]); + expect(checks.rejects.map((reject) => reject.sourceId)).toEqual(["b"]); + expect(checks.quota).toEqual({ existing: 99, limit: 100, headroom: 1, over: 1 }); + }); + + // The checks stop creates; an adopted user needs none, so a reject would + // only leave it `creating` for good. + test("an adopted user is never rejected", async () => { + const checks = await checkImport( + input({ + settings: { + ...settings({ email_address: { enabled: true } }), + sign_up: { legal_consent_enabled: true }, + } as never, + adoptedSourceIds: new Set(["a"]), + users: [user("a"), user("b")], + }), + ); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["a"]); + expect(checks.rejects.map((reject) => reject.sourceId)).toEqual(["b"]); + }); + + // Both would go to importUsers as the one adopted user. + // The adopted user is in Clerk with that email, so the earlier record's + // create would be the one Clerk refuses. + test("rejects an earlier record that shares an adopted user's email", async () => { + const checks = await checkImport( + input({ + adoptedSourceIds: new Set(["late"]), + users: [user("early", { email: "same@x.dev" }), user("late", { email: "same@x.dev" })], + }), + ); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["late"]); + expect(checks.rejects).toEqual([ + { + sourceId: "early", + reason: "shares an email, phone or username with a user an earlier attempt already created", + keptSourceId: "late", + inFile: true, + }, + ]); + }); + + // A, rejected for B's email, no longer holds its phone against C. + test("a record rejected for an adopted user's email leaves its phone to a later one", async () => { + const checks = await checkImport( + input({ + adoptedSourceIds: new Set(["b"]), + users: [ + user("a", { email: "x@x.dev", phone: "+15555550100" }), + user("b", { email: "x@x.dev" }), + user("c", { email: "c@x.dev", phone: "+15555550100" }), + ], + }), + ); + expect(checks.importable.map((entry) => entry.userId)).toEqual(["b", "c"]); + expect(checks.rejects).toEqual([ + { + sourceId: "a", + reason: "shares an email, phone or username with a user an earlier attempt already created", + keptSourceId: "b", + inFile: true, + }, + ]); + }); + + test("a second record with an adopted user's source ID is still rejected", async () => { + const checks = await checkImport( + input({ + adoptedSourceIds: new Set(["a"]), + users: [user("a"), user("a", { email: "z@x.dev" })], + }), + ); + expect(checks.importable).toHaveLength(1); + expect(checks.rejects).toEqual([ + { sourceId: "a", reason: "duplicate source ID in the file", keptSourceId: "a", inFile: true }, + ]); + }); + test("warns that an unreadable user count was checked as empty", async () => { const checks = await checkImport( input({ instanceType: "dev", existingUsers: null, users: [user("a")] }), @@ -473,7 +597,9 @@ describe("rejects", () => { const checks = await checkImport( input({ users: [user("a", { skipReason: "anonymous Better Auth user" }), user("b")] }), ); - expect(checks.rejects).toEqual([{ sourceId: "a", reason: "anonymous Better Auth user" }]); + expect(checks.rejects).toEqual([ + { sourceId: "a", reason: "anonymous Better Auth user", inFile: true }, + ]); expect(checks.importable.map((entry) => entry.userId)).toEqual(["b"]); }); @@ -488,6 +614,7 @@ describe("rejects", () => { { sourceId: "a", reason: "only has emails Clerk refuses (malformed, or a domain that can't receive mail)", + inFile: true, }, ]); }); @@ -677,7 +804,7 @@ describe("rejects", () => { expect( await reasonsOf({ settings: required, users: [user("a", { phone: "415 555 0100" })] }), ).toEqual({ - a: "only has phones not in E.164 form, and this instance requires a phone number (numeric usernames are on)", + a: "its phones aren't in E.164 form, and this instance requires a phone number (numeric usernames are on)", }); }); diff --git a/packages/cli-core/src/commands/migrate/lib/checks.ts b/packages/cli-core/src/commands/migrate/lib/checks.ts index 263e02a6f3..e80ad51a43 100644 --- a/packages/cli-core/src/commands/migrate/lib/checks.ts +++ b/packages/cli-core/src/commands/migrate/lib/checks.ts @@ -16,6 +16,8 @@ * Each rejected user gets the first reason that applies to them, so the counts * add up to the file. */ +import { isClerkInstanceId } from "./target.ts"; +import { plural } from "./plural.ts"; import type { UserSettingsJSON } from "../../../lib/fapi.ts"; import type { SpinnerControls } from "../../../lib/spinner.ts"; @@ -48,6 +50,11 @@ export type Reject = { reason: string; /** For a duplicate: the earlier user in the file that is kept instead. */ keptSourceId?: string; + /** + * Caused by the file's own data, so the same file rejects it again: no + * setting or instance change lets it in on a continue. + */ + inFile?: true; }; export type ReasonCount = { reason: string; count: number }; @@ -103,11 +110,20 @@ export type CheckInput = { * sending `skip_legal_checks`. Without it they are rejected. */ skipLegalChecks?: boolean; + /** + * Clerk IDs a continued run found behind its own in-flight creates: finding + * them in the instance is expected. + */ + adoptedClerkIds?: Set; + /** + * Source IDs of those adopted users. Each already exists in Clerk, so none + * is rejected: the checks stop creates, and these need none. They count + * toward no quota either; the instance's live count has them already. + */ + adoptedSourceIds?: Set; spinner?: SpinnerControls; }; -const plural = (count: number, word: string) => `${count} ${word}${count === 1 ? "" : "s"}`; - // --- Hash shapes ----------------------------------------------------------- /** @@ -514,6 +530,8 @@ const hasAnyIdentifier = (user: User) => hasValue(user[field as keyof User]), ); +const DUPLICATE_SOURCE_ID = "duplicate source ID in the file"; + /** * First user in the file to claim each email, phone and source ID. * @@ -539,7 +557,7 @@ function findFileDuplicates(users: User[]): { for (const user of users) { if (seenIds.has(user.userId)) { - reasons.set(user, "duplicate source ID in the file"); + reasons.set(user, DUPLICATE_SOURCE_ID); // The earlier record under the same ID is the one kept. keptBy.set(user, user.userId); continue; @@ -632,13 +650,16 @@ async function findInstanceDuplicates( ).flat(); const reasons = new Map(); - const claim = (sourceId: string | undefined, reason: string) => { - if (sourceId && !reasons.has(sourceId)) reasons.set(sourceId, reason); - }; for (const existing of found) { + // An adopted user is its own record's create, so it clashes with nothing + // in that record; its identifiers still clash with every other user's. + const own = input.adoptedClerkIds?.has(existing.id) ? existing.external_id : undefined; + const claim = (sourceId: string | undefined, reason: string) => { + if (sourceId && sourceId !== own && !reasons.has(sourceId)) reasons.set(sourceId, reason); + }; if (existing.external_id) { - claim(byExternalId.get(existing.external_id), "already in the instance, with this source ID"); + claim(byExternalId.get(existing.external_id), ALREADY_IMPORTED); } for (const email of existing.email_addresses ?? []) { claim( @@ -838,7 +859,7 @@ function buildFixes(input: CheckInput, users: User[]): Fix[] { // passes as is and Clerk answers "app not found", where `` would // be read as a redirect. const { appId, instanceId } = input.target; - const named = instanceId.startsWith("ins_"); + const named = isClerkInstanceId(instanceId); const flags = ` --app ${appId ?? "APP_ID"} --instance ${instanceId}`; const settings = input.settings; const mfa = MFA_SETTINGS.filter( @@ -957,13 +978,22 @@ function placeholderWarning(count: number): string[] { ]; } +/** + * The reason for a user an earlier import already created here. Exported so + * the import can name the run that created them. + */ +export const ALREADY_IMPORTED = "already in the instance, with this source ID"; + export async function checkImport(input: CheckInput): Promise { const rejects: Reject[] = input.failures.map((failure) => ({ sourceId: failure.userId, reason: `invalid: ${failure.error}`, + inFile: true, })); const disabledProviders = findDisabledProviderRejects(input); + // Users an earlier attempt already created, which this one only attaches. + const adopted = (user: User) => input.adoptedSourceIds?.has(user.userId) ?? false; // Users that pass every per-user check. Only these claim identifiers in // the file: a rejected record must not cost a later one its email. Each is @@ -988,44 +1018,52 @@ export async function checkImport(input: CheckInput): Promise { if (phoned.dropped) loosePhoneUsers.add(original.userId); const user = phoned.user; const sent = dropDisabledIdentifiers(user, input.settings); - const reason = - original.skipReason ?? - (refused.length > 0 && phoned.dropped && !hasAnyIdentifier(user) - ? "only has emails Clerk refuses and phones not in E.164 form (numeric usernames are on)" - : undefined) ?? - (refused.length > 0 && !hasAnyIdentifier(user) + // The file's own data, which the same file brings back on a continue. + const refusedOnly = + refused.length > 0 && !phoned.dropped && !hasAnyIdentifier(user) ? "only has emails Clerk refuses (malformed, or a domain that can't receive mail)" - : undefined) ?? - (phoned.dropped && !hasAnyIdentifier(user) - ? "only has phones not in E.164 form, which this instance refuses (numeric usernames are on)" - : undefined) ?? - // A required phone the file has, but only in a form Clerk refuses here. - (phoned.dropped && - missingRequiredIdentifier(user, input.settings) && - !missingRequiredIdentifier(emailed, input.settings) - ? "only has phones not in E.164 form, and this instance requires a phone number (numeric usernames are on)" - : undefined) ?? - missingRequiredIdentifier(user, input.settings) ?? - // Stripping the identifiers the instance has off can leave nothing to - // sign in with; Clerk would still create the user. - (!hasAnyIdentifier(sent) - ? "has no identifier this instance accepts (its email or phone is turned off, or its username is one Clerk refuses)" - : undefined) ?? - missingRequiredName(user, input.settings) ?? - mfaProblem(user, input.settings) ?? - (!user.password && input.settings && passwordIsOnlySignIn(input.settings) - ? "no password, and password is this instance's only way to sign in" - : undefined) ?? - (!input.skipLegalChecks && lacksLegalAcceptance(user, input.settings) - ? "no legal acceptance on record, which this instance requires (--skip-legal-checks imports them without it)" - : undefined) ?? - usernameProblem(user, input.settings, loosePhones) ?? - (user.password && user.passwordHasher + : undefined; + const hashProblem = + user.password && user.passwordHasher ? hashShapeProblem(user.password, user.passwordHasher) - : undefined) ?? - disabledProviders.get(user.userId); + : undefined; + const reason = adopted(original) + ? undefined + : (original.skipReason ?? + (refused.length > 0 && phoned.dropped && !hasAnyIdentifier(user) + ? "only has emails Clerk refuses and phones not in E.164 form (numeric usernames are on)" + : undefined) ?? + refusedOnly ?? + (phoned.dropped && !hasAnyIdentifier(user) + ? "only has phones not in E.164 form, which this instance refuses (numeric usernames are on)" + : undefined) ?? + // A required phone the file has, but only in a form Clerk refuses here. + (phoned.dropped && + missingRequiredIdentifier(user, input.settings) && + !missingRequiredIdentifier(emailed, input.settings) + ? "its phones aren't in E.164 form, and this instance requires a phone number (numeric usernames are on)" + : undefined) ?? + missingRequiredIdentifier(user, input.settings) ?? + // Stripping the identifiers the instance has off can leave nothing to + // sign in with; Clerk would still create the user. + (!hasAnyIdentifier(sent) + ? "has no identifier this instance accepts (its email or phone is turned off, or its username is one Clerk refuses)" + : undefined) ?? + missingRequiredName(user, input.settings) ?? + mfaProblem(user, input.settings) ?? + (!user.password && input.settings && passwordIsOnlySignIn(input.settings) + ? "no password, and password is this instance's only way to sign in" + : undefined) ?? + (!input.skipLegalChecks && lacksLegalAcceptance(user, input.settings) + ? "no legal acceptance on record, which this instance requires (--skip-legal-checks imports them without it)" + : undefined) ?? + usernameProblem(user, input.settings, loosePhones) ?? + hashProblem ?? + disabledProviders.get(user.userId)); if (reason) { - rejects.push({ sourceId: user.userId, reason }); + const inFile = + reason === original.skipReason || reason === refusedOnly || reason === hashProblem; + rejects.push({ sourceId: user.userId, reason, ...(inFile ? { inFile: true as const } : {}) }); } else { passed.push(sent); unstripped.set(sent, user); @@ -1036,13 +1074,39 @@ export async function checkImport(input: CheckInput): Promise { // No setting would let a duplicate in, so none earns a fix. const duplicates = new Set(); - const { reasons: fileDuplicates, keptBy } = findFileDuplicates(passed); + // An adopted user already holds its email, phone and username in Clerk, so + // it claims them first, wherever it sits in the file: a fresh record sharing + // one is the create Clerk would refuse, and is not kept in its place. + const { reasons: fileDuplicates, keptBy } = findFileDuplicates([ + ...passed.filter(adopted), + ...passed.filter((user) => !adopted(user)), + ]); let candidates: User[] = []; for (const user of passed) { - const reason = fileDuplicates.get(user); const kept = keptBy.get(user); + const holder = kept && !adopted(user) && input.adoptedSourceIds?.has(kept) ? kept : undefined; + if (holder) { + rejects.push({ + sourceId: user.userId, + reason: "shares an email, phone or username with a user an earlier attempt already created", + keptSourceId: holder, + inFile: true, + }); + duplicates.add(originals.get(user) ?? user); + continue; + } + // An adopted user is in Clerk already, so an earlier record sharing its + // email costs it nothing. A second record with its source ID would be + // created as the same user, so that one is still rejected. + const duplicate = fileDuplicates.get(user); + const reason = adopted(user) && duplicate !== DUPLICATE_SOURCE_ID ? undefined : duplicate; if (reason) { - rejects.push({ sourceId: user.userId, reason, ...(kept ? { keptSourceId: kept } : {}) }); + rejects.push({ + sourceId: user.userId, + reason, + ...(kept ? { keptSourceId: kept } : {}), + inFile: true, + }); duplicates.add(originals.get(user) ?? user); } else candidates.push(user); } @@ -1053,7 +1117,7 @@ export async function checkImport(input: CheckInput): Promise { : new Map(); const unique: User[] = []; for (const user of candidates) { - const reason = instanceDuplicates.get(user.userId); + const reason = adopted(user) ? undefined : instanceDuplicates.get(user.userId); if (reason) { rejects.push({ sourceId: user.userId, reason }); duplicates.add(originals.get(user) ?? user); @@ -1067,17 +1131,21 @@ export async function checkImport(input: CheckInput): Promise { let quota: Quota | undefined; if (input.instanceType === "dev") { const limit = resolveDevUserLimit(); + // An adopted user is in the instance's count already, and is never + // created: only the new users take headroom, in file order. const headroom = Math.max(0, limit - (input.existingUsers ?? 0)); - const over = Math.max(0, candidates.length - headroom); + const fresh = candidates.filter((user) => !adopted(user)); + const over = Math.max(0, fresh.length - headroom); quota = { existing: input.existingUsers ?? null, limit, headroom, over }; if (over > 0) { - for (const user of candidates.slice(headroom)) { + const past = new Set(fresh.slice(headroom)); + for (const user of past) { rejects.push({ sourceId: user.userId, reason: `over the development instance's ${limit}-user limit (raised by Clerk? set CLERK_MIGRATE_DEV_USER_LIMIT)`, }); } - candidates = candidates.slice(0, headroom); + candidates = candidates.filter((user) => !past.has(user)); } } diff --git a/packages/cli-core/src/commands/migrate/lib/failures.ts b/packages/cli-core/src/commands/migrate/lib/failures.ts new file mode 100644 index 0000000000..c6dbbf4ff5 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/failures.ts @@ -0,0 +1,37 @@ +/** + * How a failed request reads in a run's lines and summaries, shared by import, + * undo and `runs`. + */ + +import type { BapiError } from "../../../lib/errors.ts"; +import { RateLimitExceededError } from "./retry.ts"; + +/** A failed request's message and code, as a run line records them. */ +export function failureOf(error: unknown): { message: string; code: string } { + if (error instanceof RateLimitExceededError) return { message: error.message, code: "429" }; + const apiError = error as BapiError; + return { + message: apiError.longMessage ?? apiError.message ?? "Unknown error", + code: String(apiError.status ?? "unknown"), + }; +} + +/** Counts a failure under its normalized message, the way every summary groups them. */ +export function tallyError(breakdown: Map, message: string): void { + const key = normalizeErrorMessage(message); + breakdown.set(key, (breakdown.get(key) ?? 0) + 1); +} + +/** + * 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(message: string): string { + return message.replace(/\[([^\]]*)\]/g, (_, fields: string) => { + const names = fields + .split(/["'\s]+/) + .filter(Boolean) + .sort(); + return `[${names.map((name) => `"${name}"`).join(" ")}]`; + }); +} diff --git a/packages/cli-core/src/commands/migrate/lib/plural.ts b/packages/cli-core/src/commands/migrate/lib/plural.ts new file mode 100644 index 0000000000..2f3ae26cd1 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/plural.ts @@ -0,0 +1,3 @@ +/** `1 user`, `2 users`: the count and the word, in English. */ +export const plural = (count: number, word: string): string => + `${count} ${word}${count === 1 ? "" : "s"}`; 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 049cffb8f3..974b287400 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,11 +15,15 @@ import os from "node:os"; import path from "node:path"; import { _setConfigDir } from "../../../lib/config.ts"; import { + continueRun, latestUserLines, - liveLockPid, + listRuns, + lockFile, newRunId, + patchRun, readRun, resolveRunsDir, + runState, RUNS_DIR_ENV, startRun, } from "./run-store.ts"; @@ -136,6 +140,17 @@ 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", () => { @@ -146,26 +161,53 @@ describe("a run's life", () => { }); }); -describe("locks", () => { - test("a lock whose process died is stale", () => { +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); - // A PID no process can have. - fs.writeFileSync(path.join(run.dir, "lock"), "2147483646"); - expect(liveLockPid(runsDir, run.record.id)).toBeUndefined(); + 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(liveLockPid(runsDir, run.record.id)).toBeUndefined(); + expect(runState(runsDir, run.record)).toBe("interrupted"); + expect(() => continueRun(runsDir, run.record)).not.toThrow(); }); - test("a lock another live process holds is live", () => { + test("reads as running while another live process holds the lock", () => { const run = startRun(runsDir, init); - // PID 1 is always alive, and never this test. fs.writeFileSync(path.join(run.dir, "lock"), "1"); - expect(liveLockPid(runsDir, run.record.id)).toBe(1); + expect(runState(runsDir, run.record)).toBe("running"); }); // Two runs in the same second share an ID one time in 65,536. @@ -221,4 +263,89 @@ describe("locks", () => { expect(fs.statSync(path.join(run.dir, file)).mode & 0o777).toBe(0o600); } }); + + 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", + ), + ); + }); +}); + +// A continue plans from what it read before its consent prompt; an undo or +// another continue can write to the run while that prompt waits. +describe("continueRun against a plan", () => { + const stopped = () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created" }); + return run.finish(); + }; + + test("continues a run nothing touched", () => { + const record = stopped(); + expect(() => continueRun(runsDir, record, 1).finish()).not.toThrow(); + }); + + test("refuses a run an undo marked while it waited, and lets go of the lock", () => { + const record = stopped(); + patchRun(runsDir, record.id, { status: "undone", undoneBy: "20260101-000000-abcd" }); + + expect(() => continueRun(runsDir, record, 1)).toThrow(/changed while this import was waiting/); + expect(fs.existsSync(lockFile(runsDir, record.id))).toBe(false); + expect(readRun(runsDir, record.id)?.status).toBe("undone"); + }); + + test("refuses a run another continue wrote to while it waited", () => { + const record = stopped(); + fs.appendFileSync( + path.join(runsDir, record.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "b", status: "created", clerkId: "user_b" })}\n`, + ); + + expect(() => continueRun(runsDir, record, 1)).toThrow(/changed while this import was waiting/); + }); +}); + +describe("continueRun and an unfinished undo", () => { + // The caller checked before taking the lock; an undo can start, and stop + // part-way, after that. + test("refuses an import with an undo that did not finish, and leaves its record", () => { + const run = startRun(runsDir, init); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + const record = run.finish(); + const undo = startRun(runsDir, { ...init, kind: "undo", undoes: record.id }); + undo.release(); + + expect(() => continueRun(runsDir, record)).toThrow(/has an undo that did not finish/); + expect(readRun(runsDir, record.id)).toMatchObject({ + status: record.status, + finishedAt: record.finishedAt, + }); + expect(fs.existsSync(lockFile(runsDir, record.id))).toBe(false); + }); +}); + +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 index f39b5150a6..69780dfd1b 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. * - * Everything `clerk migrate` records lives here, so there is no second record - * to drift out of step. + * `runs`, `undo` and re-runs 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. @@ -25,6 +25,7 @@ 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"; +import { type ImportFlags, runsCommand, undoCommand } from "./carry-flags.ts"; /** Overrides the runs directory, below `--runs-dir`. */ export const RUNS_DIR_ENV = "CLERK_MIGRATE_DIR"; @@ -61,7 +62,12 @@ export type RunTarget = { export type RunFile = { path: string; sha256: string }; /** `notSent`: users a stopped import never sent, so they have no line. */ -export type RunCounts = Partial> & { total: number; notSent?: number }; +export type RunCounts = Partial> & { + total: number; + notSent?: number; + /** Created users with extra identifiers left to attach. */ + pending?: number; +}; export type RunRecord = { id: string; @@ -76,6 +82,13 @@ export type RunRecord = { file?: RunFile; /** The export run an import read its file from. */ fromExport?: string; + /** + * The flags an import's latest attempt ran with, as values: a record + * outlives the CLI that wrote it, and `importFlags` spells them for this one. + * An earlier build wrote a `string[]`, or none; `runs ` offers no + * continue for those. + */ + flags?: ImportFlags; /** The import run an undo reverses. */ undoes?: string; /** The undo run that reversed this one. */ @@ -98,12 +111,16 @@ export type UserLine = { /** Why a user was skipped. */ reason?: string; error?: string; + /** Failed: the HTTP status, or "unknown". Skipped: a machine reason, NO_PASSWORD_CODE or IN_FILE_CODE. */ 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"; +export const USERS_FILE = "users.ndjson"; const LOCK_FILE = "lock"; // --- Location -------------------------------------------------------------- @@ -208,6 +225,7 @@ export function lockFile(runsDir: string, id: string): string { } const isExists = (error: unknown) => (error as NodeJS.ErrnoException).code === "EEXIST"; +const isMissing = (error: unknown) => (error as NodeJS.ErrnoException).code === "ENOENT"; /** * Takes the run's lock. Created exclusively (`wx`), so two processes can't @@ -224,15 +242,18 @@ function acquireLock(runsDir: string, id: string): void { try { take(); + log.debug(`migrate: locked run ${id} (${file})`); return; } catch (error) { if (!isExists(error)) throw error; } const holder = liveLockPid(runsDir, id); if (holder !== undefined) refuse(holder); + log.debug(`migrate: removing a stale lock on run ${id} (${file})`); fs.rmSync(file, { force: true }); try { take(); + log.debug(`migrate: locked run ${id} (${file})`); } catch (error) { // Another process took the stale lock between the remove and the write. if (isExists(error)) refuse(liveLockPid(runsDir, id)); @@ -240,18 +261,24 @@ function acquireLock(runsDir: string, id: string): void { } } +function releaseLock(runsDir: string, id: string): void { + log.debug(`migrate: released run ${id}`); + fs.rmSync(lockFile(runsDir, id), { force: true }); +} + // --- Reading --------------------------------------------------------------- /** One run's record, or `undefined` when the folder holds no readable run. */ export function readRun(runsDir: string, id: string): RunRecord | undefined { + const file = path.join(runDir(runsDir, id), RUN_FILE); 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; + const parsed = JSON.parse(fs.readFileSync(file, "utf-8")) as RunRecord; + if (parsed && typeof parsed === "object" && parsed.id === id) return parsed; + log.debug(`migrate: ${file} is not run ${id}'s record`); + } catch (error) { + if (!isMissing(error)) log.debug(`migrate: could not read ${file}: ${String(error)}`); } + return undefined; } /** Every line of a run's `users.ndjson`, in the order written. */ @@ -281,6 +308,128 @@ export function latestUserLines(runsDir: string, id: string): Map): boolean { + if (!record.finishedAt || (record.counts.notSent ?? 0) > 0) return true; + return [...latest].some((line) => + line.status === "skipped" + ? line.code !== NO_PASSWORD_CODE && line.code !== IN_FILE_CODE + : line.status !== "created" || (line.pending?.length ?? 0) > 0, + ); +} + +/** + * The imports of this file with this source, newest first: the runs a + * re-import of it chooses among, before it looks at the instance. + */ +export function importsOfFile( + runs: RunRecord[], + file: { sha256: string; source?: string }, +): RunRecord[] { + return runs.filter( + (record) => + record.kind === "import" && + record.file?.sha256 === file.sha256 && + record.source === file.source, + ); +} + +/** Every undo of this import, finished or not, newest first. */ +export function undosOf(runs: RunRecord[], importId: string): RunRecord[] { + return runs.filter((record) => record.kind === "undo" && record.undoes === importId); +} + +/** + * The undo of this import that has not finished, if any. `runs` is the + * caller's `listRuns`, when it has one already. + */ +export function openUndoOf( + runsDir: string, + importId: string, + runs = listRuns(runsDir), +): RunRecord | undefined { + return undosOf(runs, importId).find((undo) => runState(runsDir, undo) !== "complete"); +} + +/** + * Why an import with an unfinished undo can't continue: some of its users are + * gone and some are not. `carry` reaches the same runs folder and instance. + */ +export function unfinishedUndoMessage(importId: string, undoId: string, carry: string[]): string { + return ( + `Run ${importId} has an undo that did not finish (run ${undoId}). ` + + `Finish it with \`${undoCommand(importId, carry)}\`, or pass --new-run to import into a new run.` + ); +} + +/** Why a run ID matched nothing, and the listing of that same runs folder. */ +export function noRunMessage(runsDir: string, id: string, carry: string[]): string { + return `No run \`${id}\` in ${runsDir}. Run \`${runsCommand(carry)}\` to list them.`; +} + +/** Why a run another live process holds is refused, and how to clear a stale lock. */ +export function inUseMessage(runsDir: string, id: string, doing: string, then: string): string { + return ( + `Run ${id} is ${doing} right now in another process (PID ${liveLockPid(runsDir, id)}). ${then} ` + + `If that process is not a migrate run, delete ${lockFile(runsDir, id)}.` + ); +} + +/** Every readable run, newest first. */ +export function listRuns(runsDir: string): RunRecord[] { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(runsDir, { withFileTypes: true }); + } catch (error) { + if (!isMissing(error)) log.debug(`migrate: could not read ${runsDir}: ${String(error)}`); + 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 { @@ -291,12 +440,18 @@ function writeRecord(runsDir: string, record: RunRecord): void { fs.renameSync(temp, file); } -/** Totals by each source ID's latest status. */ +/** + * Totals by each source ID's latest status, and `pending`: the created users + * whose extra identifiers never attached, left for a continue. + */ 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; + if (line.status === "created" && (line.pending?.length ?? 0) > 0) { + counts.pending = (counts.pending ?? 0) + 1; + } } return counts; } @@ -344,19 +499,14 @@ function openRun(runsDir: string, record: RunRecord): Run { writeRecord(runsDir, run.record); }, finish(options) { - const lines = [...latestUserLines(runsDir, run.record.id).values()]; - const counts = countLines(lines); + const counts = countLines(latestUserLines(runsDir, run.record.id).values()); if (options?.notSent) counts.notSent = options.notSent; - // 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) + (counts.notSent ?? 0) + - pendingAttaches; + (counts.pending ?? 0); run.update({ counts, status: unfinished > 0 ? "partial" : "complete", @@ -366,7 +516,7 @@ function openRun(runsDir: string, record: RunRecord): Run { return run.record; }, release() { - fs.rmSync(path.join(dir, LOCK_FILE), { force: true }); + releaseLock(runsDir, record.id); }, }; return run; @@ -403,3 +553,101 @@ export function startRun(runsDir: string, init: StartRunInit): Run { log.debug(`migrate: started ${init.kind} run ${id} in ${runsDir}`); return openRun(runsDir, record); } + +/** + * A run's record with what this attempt ran with, which its later commands + * repeat: where its key came from (never the key), and an import's flags. The + * instance ID stays: `findResume` and `undo` key on it. + */ +export function withAttempt(record: RunRecord, live: RunTarget, flags?: ImportFlags): RunRecord { + const { keySource, appId, appLabel } = live; + return { + ...record, + target: { ...record.target, keySource, appId, appLabel }, + ...(flags ? { flags } : {}), + }; +} + +/** + * 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, + /** + * How many user lines the caller planned against. The record and the lines + * were read before the lock was taken, while a prompt may have waited: an + * undo, or another continue, can change either in the meantime. + */ + plannedLines?: number, + /** The caller's runs-folder and target flags, for the command a refusal suggests. */ + carry: string[] = [], +): Run { + acquireLock(runsDir, record.id); + if (plannedLines !== undefined) { + const fresh = readRun(runsDir, record.id); + const changed = + !fresh || + fresh.status !== record.status || + fresh.undoneBy !== record.undoneBy || + readUserLines(runsDir, record.id).length !== plannedLines; + if (changed) { + releaseLock(runsDir, record.id); + throwUsageError( + `Run ${record.id} changed while this import was waiting, so its plan is out of date. ` + + "Nothing was imported. Run the command again.", + ); + } + } + // Checked again under the lock, before the record says running: an undo + // can start, and stop part-way, after the caller's own check, leaving the + // import's status and lines as they were. + const undoRun = record.kind === "import" ? openUndoOf(runsDir, record.id) : undefined; + if (undoRun) { + releaseLock(runsDir, record.id); + throwUsageError(unfinishedUndoMessage(record.id, undoRun.id, carry)); + } + // 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; +} + +/** + * Takes a run's lock without opening it, so no other process can continue it. + * + * @returns Releases the lock. + * @throws UsageError when another live process holds it. + */ +export function lockRun(runsDir: string, id: string): () => void { + acquireLock(runsDir, id); + return () => releaseLock(runsDir, id); +} + +/** + * Merges fields into a run this process is not writing, such as `undoneBy`, + * and returns what was written. + */ +export function patchRun( + runsDir: string, + id: string, + patch: Partial, +): RunRecord | undefined { + const record = readRun(runsDir, id); + if (!record) return undefined; + const patched = { ...record, ...patch }; + log.debug( + `migrate: patching run ${id} (${path.join(runDir(runsDir, id), RUN_FILE)}): ${Object.keys(patch).join(", ")}`, + ); + writeRecord(runsDir, patched); + return patched; +} 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 78c98c89c3..a13398caf7 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.test.ts @@ -1,8 +1,17 @@ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + spyOn, + 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 { _setConfigDir, setProfile } 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"; @@ -14,6 +23,7 @@ import { } from "./target.ts"; const captured = useCaptureLog(); +const plapiModule = await import("../../../lib/plapi.ts"); describe("printTarget", () => { test.each([ @@ -237,4 +247,88 @@ describe("resolveClerkTarget key source", () => { appLabel: "accountless app", }); }); + + // The profile names the instance; a stand-in would leave `runs` nothing it + // can pass back with `--instance`. + describe("a linked profile while GET /v1/instance fails", () => { + let fetchApplication: ReturnType; + + beforeEach(async () => { + await setProfile(workDir, { + workspaceId: "org_1", + appId: "app_1", + appName: "My App", + instances: { development: "ins_dev", production: "ins_prod" }, + }); + fetchApplication = spyOn(plapiModule, "fetchApplication").mockResolvedValue({ + application_id: "app_1", + name: "My App", + instances: [ + { + instance_id: "ins_prod", + environment_type: "production", + publishable_key: "pk_live_x", + secret_key: "sk_live_x", + }, + ], + } as never); + globalThis.fetch = (async () => + new Response("nope", { status: 500 })) as unknown as typeof fetch; + }); + + afterEach(() => { + fetchApplication.mockRestore(); + }); + + test("records the profile's instance", async () => { + const { target } = await resolveClerkTarget({ instance: "prod" }); + expect(target).toMatchObject({ + instanceId: "ins_prod", + env: "production", + keySource: "linked profile", + }); + }); + + // The profile labels a literal ID with itself, which is no environment, + // and the key prefix would only guess one: a staging key is `sk_live_`. + test("records the environment as unknown for a literal --instance", async () => { + const { target } = await resolveClerkTarget({ instance: "ins_prod" }); + expect(target).toMatchObject({ instanceId: "ins_prod", env: "unknown" }); + }); + }); +}); + +// A stand-in is no ID `--app --instance` can resolve again, so a run recorded +// under one would suggest commands that fail, or reach another instance. +describe("resolveClerkTarget --app", () => { + let originalFetch: typeof globalThis.fetch; + let fetchApplication: ReturnType; + + beforeEach(() => { + originalFetch = globalThis.fetch; + globalThis.fetch = (async () => + new Response("nope", { status: 500 })) as unknown as typeof fetch; + fetchApplication = spyOn(plapiModule, "fetchApplication").mockResolvedValue({ + application_id: "app_123", + name: "My App", + instances: [ + { + instance_id: "ins_staging", + environment_type: "staging", + publishable_key: "pk_test_x", + secret_key: "sk_test_x", + }, + ], + } as never); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + fetchApplication.mockRestore(); + }); + + test("records the instance the Platform API named when GET /v1/instance fails", async () => { + const { target } = await resolveClerkTarget({ app: "app_123", instance: "ins_staging" }); + expect(target).toMatchObject({ instanceId: "ins_staging", env: "staging", keySource: "--app" }); + }); }); diff --git a/packages/cli-core/src/commands/migrate/lib/target.ts b/packages/cli-core/src/commands/migrate/lib/target.ts index 0735e528b4..01a8798e44 100644 --- a/packages/cli-core/src/commands/migrate/lib/target.ts +++ b/packages/cli-core/src/commands/migrate/lib/target.ts @@ -3,9 +3,10 @@ * * `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. + * source yields the same identity. When that call fails, `--app` and a linked + * profile use the instance the Platform API or the profile names. 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"; @@ -14,11 +15,16 @@ 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, SDK_KEYLESS_SOURCE } from "../../../lib/keyless-target.ts"; +import { + ENV_VAR_SOURCE_SUFFIX, + 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"; import { retryOn429 } from "./retry.ts"; +import { type CarryOptions, carryFlags } from "./carry-flags.ts"; import type { RunTarget } from "./run-store.ts"; export type TargetOptions = { @@ -35,25 +41,62 @@ export type ClerkTarget = RunTarget & { keySource: string; }; +/** + * The key sources a run records by name. A `.env` file or a keyless app is + * recorded by its file, so those stay plain strings. + */ +export const KEY_SOURCE = { + secretKeyFlag: "--secret-key", + app: "--app", + envVar: `CLERK_SECRET_KEY${ENV_VAR_SOURCE_SUFFIX}`, + linkedProfile: "linked profile", +} as const; + +type NamedKeySource = (typeof KEY_SOURCE)[keyof typeof KEY_SOURCE]; + +type InstanceIdentity = { instanceId: string; env: string }; + +type KeySource = { + keySource: string; + appId?: string; + appLabel?: string; + /** No `env` when the instance was named by ID, which is no environment. */ + instance?: { instanceId: string; env?: string }; +}; + +/** The app and instance `--app` or a linked profile names. */ +async function fromAppContext( + keySource: NamedKeySource, + options: TargetOptions, +): Promise { + const ctx = await resolveAppContext(options); + return { + keySource, + appId: ctx.appId, + appLabel: ctx.appLabel, + instance: { + instanceId: ctx.instanceId, + // A profile labels a literal `--instance ins_…` with the ID itself. + ...(isClerkInstanceId(ctx.instanceLabel) ? {} : { env: ctx.instanceLabel }), + }, + }; +} + /** * Which rung of `resolveBapiSecretKey`'s chain supplies the key, and the app - * it belongs to when that rung knows one. + * it belongs to when that rung knows one. `--app` and a linked profile also + * know the instance, from the Platform API or the profile. * * 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" }; +async function describeKeySource(options: TargetOptions): Promise { + if (options.secretKey) return { keySource: KEY_SOURCE.secretKeyFlag }; - if (options.app) { - const ctx = await resolveAppContext({ app: options.app, instance: options.instance }); - return { keySource: "--app", appId: ctx.appId, appLabel: ctx.appLabel }; - } + if (options.app) return fromAppContext(KEY_SOURCE.app, options); - if (process.env.CLERK_SECRET_KEY) return { keySource: "CLERK_SECRET_KEY env var" }; + if (process.env.CLERK_SECRET_KEY) return { keySource: KEY_SOURCE.envVar }; // Only the SDK's own file says the app is accountless: a key in `.env` // may belong to any app, an account's included. @@ -64,8 +107,54 @@ async function describeKeySource( : { keySource: keyless.source }; } - const ctx = await resolveAppContext({ instance: options.instance }); - return { keySource: "linked profile", appId: ctx.appId, appLabel: ctx.appLabel }; + return fromAppContext(KEY_SOURCE.linkedProfile, options); +} + +type TargetCarry = Omit; +type CarryFor = (target: RunTarget, instance?: string) => TargetCarry; + +// The key again, as a placeholder: the key alone picks the instance, from +// any directory. Not `--instance`: in an unlinked directory with a `.env`, +// the keyless rung refuses it before a profile could resolve it. +const keyAgain: CarryFor = () => ({ secretKey: "" }); +// The app and the instance. A profile's own link resolves development, and +// only from a directory linked to that app. Without a real instance, the key: +// a bare `--app` resolves development too. +const appAndInstance: CarryFor = (target, instance) => + instance ? { app: target.appId, instance } : keyAgain(target); + +/** + * For each named key source, what reaches a recorded target again. Every + * name needs an entry, so a new one fails to compile until it says how. + */ +const TARGET_CARRY: Record = { + [KEY_SOURCE.secretKeyFlag]: keyAgain, + [KEY_SOURCE.envVar]: keyAgain, + [KEY_SOURCE.app]: appAndInstance, + [KEY_SOURCE.linkedProfile]: appAndInstance, +}; + +function isNamedKeySource(source: string | undefined): source is NamedKeySource { + return source !== undefined && Object.hasOwn(TARGET_CARRY, source); +} + +/** + * The flags a command meant for later needs to reach a run's target and runs + * folder again: `runs `'s, and an import's or undo's own next steps. The + * instance goes by ID, not alias (a staging one has none), and only a real + * one: no `--instance` resolves a stand-in. A framework's own key variable, + * exported like `CLERK_SECRET_KEY`, gets the key again. A key file (`.env`, a + * keyless app's) gets nothing: it refuses `--instance`, and run from the same + * project it names the key itself. + */ +export function laterCarry(runsDir: string | undefined, target: RunTarget): string[] { + const instance = isClerkInstanceId(target.instanceId) ? target.instanceId : undefined; + const carry = isNamedKeySource(target.keySource) + ? TARGET_CARRY[target.keySource](target, instance) + : target.keySource?.endsWith(ENV_VAR_SOURCE_SUFFIX) + ? keyAgain(target, instance) + : {}; + return carryFlags({ runsDir, ...carry }); } /** @@ -76,16 +165,14 @@ async function describeKeySource( * 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 }> { +export async function fetchInstanceIdentity(secretKey: string): Promise { 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_")) { + if (typeof instance.id === "string" && isClerkInstanceId(instance.id)) { return { instanceId: instance.id, env: @@ -98,6 +185,16 @@ export async function fetchInstanceIdentity( return { instanceId: keyInstanceId(secretKey), env: fallbackEnv }; } +/** A stand-in for an instance Clerk did not name: {@link keyInstanceId}'s `key_` hash. */ +export function isStandInInstanceId(id: string | undefined): boolean { + return id?.startsWith("key_") === true; +} + +/** An instance ID Clerk itself returned. */ +export function isClerkInstanceId(id: string | undefined): boolean { + return id?.startsWith("ins_") === true; +} + /** * 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 @@ -118,16 +215,16 @@ export function keyInstanceId(secretKey: string): string { function assertInstanceFlagMatches( options: TargetOptions, keySource: string, - identity: { instanceId: string; env: string }, + identity: InstanceIdentity, ): void { const flag = options.instance; - if (!flag || (keySource !== "--secret-key" && !keySource.startsWith("CLERK_SECRET_KEY"))) return; + if (!flag || (keySource !== KEY_SOURCE.secretKeyFlag && keySource !== KEY_SOURCE.envVar)) return; 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_")) { + if (!wanted && isStandInInstanceId(identity.instanceId)) { throwUsageError( `Could not verify that the key from ${keySource} addresses --instance ${flag}: ` + "Clerk did not answer GET /v1/instance. Nothing was changed.\n" + @@ -146,8 +243,15 @@ 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); + const { instance: named, ...source } = await describeKeySource(options); + const fetched = await fetchInstanceIdentity(secretKey); + // The Platform API or the profile already named the instance: recorded + // instead of a stand-in, a run's commands can reach it with `--instance`. + // Named by ID, its environment is unknown: the key prefix only guesses it. + const { instanceId, env } = + named && isStandInInstanceId(fetched.instanceId) && isClerkInstanceId(named.instanceId) + ? { instanceId: named.instanceId, env: named.env ?? "unknown" } + : fetched; assertInstanceFlagMatches(options, source.keySource, { instanceId, env }); return { diff --git a/packages/cli-core/src/commands/migrate/lib/terminal.ts b/packages/cli-core/src/commands/migrate/lib/terminal.ts new file mode 100644 index 0000000000..12f361448c --- /dev/null +++ b/packages/cli-core/src/commands/migrate/lib/terminal.ts @@ -0,0 +1,25 @@ +import fs from "node:fs"; +import { isHuman } from "../../../mode.ts"; + +/** + * Whether a prompt has a terminal to read: stdin, or `/dev/tty` behind a piped + * stdin. `CLERK_MODE=human` forces human mode without one, and a prompt with + * no terminal fails to open it and spins. An object so tests can swap it. + */ +export const terminal = { + available(): boolean { + if (process.stdin.isTTY) return true; + try { + fs.closeSync(fs.openSync(process.platform === "win32" ? "CONIN$" : "/dev/tty", "r")); + return true; + } catch { + return false; + } + }, +}; + +/** Whether someone can answer a prompt: not an agent, a non-TTY run, or `--json`. */ +export function canPrompt(options: { json?: boolean }): boolean { + // A mode is human or agent, so isHuman() alone rules an agent out. + return !options.json && isHuman() && terminal.available(); +} diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts index 8f74286c4f..9fa040931f 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.test.ts @@ -87,7 +87,8 @@ test("reads every page of a batch that matches more than one page", async () => expect(urls[0]?.searchParams.getAll("external_id")).toEqual(["+-dup"]); }); -test("stops paging when the cursor does not move", async () => { +// Stopping quietly would pass a short lookup off as complete. +test("fails, rather than looping or stopping short, when the cursor does not move", async () => { let requests = 0; // A server that ignores `starting_after` serves the same full page again. globalThis.fetch = (async () => { @@ -97,12 +98,13 @@ test("stops paging when the cursor does not move", async () => { ); }) as unknown as typeof fetch; - await lookupUsers({ - filter: "external_id", - values: ["a"], - secretKey: "sk_test_x", - schedule: createApiScheduler(1, 10_000), - }); - + await expect( + lookupUsers({ + filter: "external_id", + values: ["a"], + secretKey: "sk_test_x", + schedule: createApiScheduler(1, 10_000), + }), + ).rejects.toThrow(/same page of users twice/); expect(requests).toBe(2); }); diff --git a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts index efca044d9d..0e44207c85 100644 --- a/packages/cli-core/src/commands/migrate/lib/user-lookup.ts +++ b/packages/cli-core/src/commands/migrate/lib/user-lookup.ts @@ -10,6 +10,7 @@ */ import { bapiRequest } from "../../../lib/bapi.ts"; +import { CliError } from "../../../lib/errors.ts"; import type { SpinnerControls } from "../../../lib/spinner.ts"; import { retryOn429 } from "./retry.ts"; import type { ApiScheduler } from "./scheduler.ts"; @@ -22,6 +23,7 @@ export type LookedUpUser = { id: string; external_id?: string | null; username?: string | null; + private_metadata?: Record | null; last_sign_in_at?: number | null; email_addresses?: { email_address?: string }[]; phone_numbers?: { phone_number?: string }[]; @@ -87,8 +89,13 @@ export async function lookupUsers(options: { found.push(...page); const last = page.at(-1)?.id; if (page.length < LOOKUP_BATCH || typeof last !== "string") break; - // A cursor that does not move would page forever. - if (params.get("starting_after") === last) break; + // A cursor that does not move would page forever, and stopping there + // would pass a short lookup off as complete. + if (params.get("starting_after") === last) { + throw new CliError( + "Clerk returned the same page of users twice, so the lookup can't finish", + ); + } params.set("starting_after", last); } progress.done++; @@ -101,3 +108,45 @@ export async function lookupUsers(options: { return pages.flat().filter((user) => typeof user.id === "string"); } + +/** + * The `private_metadata` key every import create carries: the ID of the run + * that sent it. It is how a continue or an undo knows a user found by + * `external_id` is that run's own create, and not one an app or another tool + * made with the same source ID. + */ +export const RUN_MARKER_KEY = "clerkMigrateRun"; + +/** A user a run recorded: its source ID, and the Clerk user it became. */ +export type SourceUser = { sourceId: string; clerkId: string }; + +/** + * The users behind creates that were in flight when run `runId` stopped: + * found by `external_id`, and counted only when they carry the run's marker. + * One without it stays unresolved: a continue's checks find it in the + * instance, and `undo` leaves it alone. That includes a create from a run + * recorded before the marker existed. + */ +export async function findInFlight(options: { + runId: string; + sourceIds: string[]; + secretKey: string; + schedule: ApiScheduler; +}): Promise { + if (options.sourceIds.length === 0) return []; + const found = await lookupUsers({ + filter: "external_id", + values: options.sourceIds, + secretKey: options.secretKey, + schedule: options.schedule, + }); + const wanted = new Set(options.sourceIds); + return found + .filter( + (user) => + user.external_id && + wanted.has(user.external_id) && + user.private_metadata?.[RUN_MARKER_KEY] === options.runId, + ) + .map((user) => ({ sourceId: user.external_id as string, clerkId: user.id })); +} diff --git a/packages/cli-core/src/commands/migrate/readme.test.ts b/packages/cli-core/src/commands/migrate/readme.test.ts index 05faefc31f..88e5f6af44 100644 --- a/packages/cli-core/src/commands/migrate/readme.test.ts +++ b/packages/cli-core/src/commands/migrate/readme.test.ts @@ -167,7 +167,7 @@ describe("migrate README", () => { test("states the rules every command follows", () => { expect(README).toContain("## The rules"); for (const rule of [ - "Nothing writes without consent", + "Nothing writes to Clerk without consent", "`--dry-run` checks against the real instance", "State lives in one place", "Every command prints its target first", diff --git a/packages/cli-core/src/commands/migrate/run-interactive.test.ts b/packages/cli-core/src/commands/migrate/run-interactive.test.ts index c4c201526d..473b56ce09 100644 --- a/packages/cli-core/src/commands/migrate/run-interactive.test.ts +++ b/packages/cli-core/src/commands/migrate/run-interactive.test.ts @@ -8,7 +8,17 @@ * `mode.ts` has no way to clear a forced mode once the file is done. */ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + mock, + test, + spyOn, +} from "bun:test"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -42,6 +52,9 @@ mock.module("../../lib/prompts.ts", () => ({ })); const { run } = await import("./run.ts"); +const { terminal } = await import("./lib/terminal.ts"); +// The prompts are stubs; whether this process has a terminal is a test of its own. +const terminalAvailable = spyOn(terminal, "available"); const { UserAbortError } = await import("../../lib/errors.ts"); const { _setConfigDir } = await import("../../lib/config.ts"); @@ -72,6 +85,7 @@ beforeAll(() => { }); afterAll(() => { + terminalAvailable.mockRestore(); if (originalMode === undefined) delete process.env.CLERK_MODE; else process.env.CLERK_MODE = originalMode; globalThis.fetch = originalFetch; @@ -82,6 +96,7 @@ afterAll(() => { }); beforeEach(() => { + terminalAvailable.mockReturnValue(true); requests = []; confirmAnswer = true; instanceSettings = undefined; @@ -125,6 +140,25 @@ const created = () => const importOptions = { input: "export.json", source: "clerk", secretKey: "sk_test_x" }; +// A yes at the prompt is consent to this run, so the retry it suggests runs +// as printed, like `runs `'s continue. +test("after a yes at the prompt, the suggested retry carries --yes", async () => { + const clerk = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const body = typeof init?.body === "string" ? init.body : ""; + return init?.method === "POST" && body.includes('"u2"') + ? Response.json( + { errors: [{ message: "taken", code: "form_identifier_exists" }] }, + { status: 422 }, + ) + : clerk(input, init); + }) as typeof fetch; + + await run(importOptions); + + expect(Bun.stripANSI(captured.err)).toMatch(/--yes` to retry them/); +}); + describe("prompts for what was not passed", () => { test("bare `clerk migrate import` asks for the file, then the source, then imports", async () => { await run({ secretKey: "sk_test_x" }); @@ -142,6 +176,16 @@ describe("prompts for what was not passed", () => { }); }); +// `CLERK_MODE=human` with no terminal: the prompt would fail to open one and spin. +test("with no terminal, refuses for consent rather than prompting", async () => { + terminalAvailable.mockReturnValue(false); + + await expect(run(importOptions)).rejects.toThrow(/--yes/); + + expect(confirmMessages).toEqual([]); + expect(requests.some((r) => r.url.endsWith("/v1/users") && r.method === "POST")).toBe(false); +}); + describe("consent", () => { test("asks before writing, naming how many users", async () => { await run(importOptions); diff --git a/packages/cli-core/src/commands/migrate/run.test.ts b/packages/cli-core/src/commands/migrate/run.test.ts index 54ad5a9a87..85b4f0f38e 100644 --- a/packages/cli-core/src/commands/migrate/run.test.ts +++ b/packages/cli-core/src/commands/migrate/run.test.ts @@ -1,17 +1,31 @@ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + mock, + spyOn, + 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 { type CliError, EXIT_CODE } from "../../lib/errors.ts"; +import * as plapi from "../../lib/plapi.ts"; import { credentialStoreStubs, useCaptureLog } from "../../test/lib/stubs.ts"; // Every test below names its own `--secret-key`, which short-circuits the // signed-in check — except the one that asserts what happens without it. mock.module("../../lib/credential-store.ts", () => credentialStoreStubs); -import { _resetInterruptState, abortInFlight, beginInterrupt } from "../../lib/signals.ts"; -import { latestUserLines, readRun, type RunRecord } from "./lib/run-store.ts"; +import { importFlags } from "./lib/carry-flags.ts"; +import { latestUserLines, listRuns, patchRun, readRun, startRun } from "./lib/run-store.ts"; import { explainErrors, run, validateRunOptions } from "./run.ts"; +// After run.ts: imported first, signals.ts loads version.ts ahead of its Bun +// macro here, and the file fails to load. +import { _resetInterruptState, abortInFlight, beginInterrupt } from "../../lib/signals.ts"; /** A real-shaped bcrypt digest: the checks reject anything that is not. */ const BCRYPT = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy"; @@ -23,13 +37,6 @@ let originalCwd: string; /** Where runs land for a project rooted at `workDir`. */ const runsDir = () => path.join(workDir, ".clerk", "migrate"); -/** The runs in `dir`, newest first. */ -const runsIn = (dir: string): RunRecord[] => - (fs.existsSync(dir) ? fs.readdirSync(dir) : []) - .map((id) => readRun(dir, id)) - .filter((record): record is RunRecord => record !== undefined) - .sort((a, b) => b.id.localeCompare(a.id)); - beforeAll(() => { originalCwd = process.cwd(); workDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-migrate-run-"))); @@ -82,12 +89,15 @@ type Stub = { id: string; external_id?: string; username?: string; + private_metadata?: Record; email_addresses?: { email_address: string }[]; }[]; /** `GET /v1/users/count`. */ count?: number; /** Source IDs whose `POST /v1/users` fails with a 422. */ failing?: Set; + /** `GET /v1/instance` fails, so the target falls back to the key's stand-in ID. */ + instanceDown?: boolean; }; describe("run", () => { @@ -113,6 +123,9 @@ describe("run", () => { const body = init?.body ? (JSON.parse(init.body as string) as Record) : null; requests.push({ method, url: url.toString(), body }); + if (url.pathname === "/v1/instance" && stub.instanceDown) { + return new Response("unavailable", { status: 503 }); + } if (url.pathname === "/v1/instance") { return Response.json({ object: "instance", id: "ins_1", environment_type: "development" }); } @@ -251,7 +264,7 @@ describe("run", () => { test("records the run in the project's run store", async () => { await run(baseOptions); - const [record, ...rest] = runsIn(runsDir()); + const [record, ...rest] = listRuns(runsDir()); expect(rest).toHaveLength(0); expect(record).toMatchObject({ kind: "import", @@ -270,6 +283,22 @@ describe("run", () => { ]); }); + // A complete import leaves the export's user data behind; say how to remove it. + test("names the folders a complete import no longer needs", async () => { + await run(baseOptions); + const [record] = listRuns(runsDir()); + expect(captured.err).toContain(`rm -rf ${path.join(runsDir(), record!.id)}`); + }); + + // Pasted unquoted, `rm -rf …/app copy/…` deletes `…/app`. + test("quotes the cleanup paths", async () => { + const spaced = path.join(workDir, "app copy"); + await run({ ...baseOptions, runsDir: spaced }); + + const [record] = listRuns(spaced); + expect(captured.err).toContain(`rm -rf '${path.join(spaced, record!.id)}'`); + }); + test("gitignores the project's .clerk folder before writing a run", async () => { await run(baseOptions); expect(fs.readFileSync(path.join(workDir, ".gitignore"), "utf-8")).toContain(".clerk/"); @@ -285,8 +314,8 @@ describe("run", () => { test("--runs-dir puts the run somewhere else", async () => { await run({ ...baseOptions, runsDir: "elsewhere" }); - expect(runsIn(path.join(workDir, "elsewhere"))).toHaveLength(1); - expect(runsIn(runsDir())).toHaveLength(0); + expect(listRuns(path.join(workDir, "elsewhere"))).toHaveLength(1); + expect(listRuns(runsDir())).toHaveLength(0); }); test("--require-password leaves out the users without one", async () => { @@ -294,12 +323,244 @@ describe("run", () => { expect(created()).toEqual(["u1"]); expect(captured.err).toContain("leaving out 1 user without a password"); - const [record] = runsIn(runsDir()); + const [record] = listRuns(runsDir()); expect(latestUserLines(runsDir(), record!.id).get("u2")).toMatchObject({ status: "skipped", reason: "no password (--require-password)", + code: "no_password", }); expect(record?.status).toBe("partial"); + // So a continue would still leave them out. + expect(importFlags(record?.flags ?? {})).toEqual(["--require-password"]); + // ...which is why none is offered: following it would print it again. + const err = Bun.stripANSI(captured.err); + expect(err).not.toContain("to retry them"); + expect(err).toContain("to see what happened to each user"); + }); + + // The README's way to import them: the same import without the flag. + test("the same import without --require-password continues the run and creates them", async () => { + await run({ ...baseOptions, requirePassword: true }); + const [first] = listRuns(runsDir()); + requests = []; + + await run(baseOptions); + + expect(created()).toEqual(["u2"]); + expect(listRuns(runsDir())).toHaveLength(1); + expect(readRun(runsDir(), first!.id)?.status).toBe("complete"); + }); + + // --new-run refuses the users the run created; it names the same file as such. + test("--new-run on the same file names it as this file, not an earlier version", async () => { + await run({ ...baseOptions, requirePassword: true }); + const [first] = listRuns(runsDir()); + captured.clear(); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain(`created by run ${first!.id}, from this file.`); + // Dropping --new-run continues that run, so the refusal offers it first. + expect(error.message).toContain(`Drop --new-run to continue run ${first!.id}.`); + expect(error.examples?.[0]?.command).not.toContain("--new-run"); + expect(error.examples?.[0]?.description).toBe(`Continue run ${first!.id}`); + }); + + // A complete run has nothing to continue: without --new-run it is already imported. + test("--new-run on the same file doesn't offer to continue a complete run", async () => { + await run(baseOptions); + captured.clear(); + stubClerk({ + existing: [ + { id: "user_u1", external_id: "u1" }, + { id: "user_u2", external_id: "u2" }, + ], + }); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain("from this file."); + expect(error.message).not.toContain("Drop --new-run"); + }); + + // findResume continues the newest run of the file, so the refusal offers the + // continue only when that is the run that created the users. + test("--new-run doesn't offer to continue a run a newer one would take over", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + stubClerk({ failing: new Set(["u2"]), existing: [{ id: "user_u1", external_id: "u1" }] }); + await run({ ...baseOptions, newRun: true, allowPartial: true }); + expect(listRuns(runsDir())).toHaveLength(2); + captured.clear(); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain("from this file."); + expect(error.message).not.toContain("Drop --new-run"); + }); + + // Without --new-run the import refuses over the unfinished undo. + test("--new-run doesn't offer to continue a run with an unfinished undo", async () => { + await run({ ...baseOptions, requirePassword: true }); + const [first] = listRuns(runsDir()); + const undoRun = startRun(runsDir(), { + kind: "undo", + target: { instanceId: "ins_1" }, + undoes: first!.id, + }); + undoRun.append({ sourceId: "u1", status: "failed", clerkId: "user_u1" }); + undoRun.finish(); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain(`created by run ${first!.id}, from this file.`); + expect(error.message).not.toContain("Drop --new-run"); + }); + + // Stopped before finish(), a run has no lines for the users it never sent. + test("--new-run --require-password offers to continue an interrupted run", async () => { + const both = export2.map((user) => ({ + ...user, + password_digest: BCRYPT, + password_hasher: "bcrypt", + })); + fs.writeFileSync(path.join(workDir, "export.json"), JSON.stringify(both)); + await run(baseOptions); + const [first] = listRuns(runsDir()); + // As a Ctrl-C after u1 leaves it: no finish time, the opening counts, one line. + const record = readRun(runsDir(), first!.id)!; + delete record.finishedAt; + fs.writeFileSync( + path.join(runsDir(), first!.id, "run.json"), + JSON.stringify({ ...record, status: "running", counts: { total: 0 } }), + ); + fs.writeFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u1", clerkId: "user_u1", status: "created" })}\n`, + ); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true, requirePassword: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain(`Drop --new-run to continue run ${first!.id}.`); + }); + + // The same file rejects it again, so a continue would only loop. + test("a run left partial only by the file's own bad data offers no retry, nor a continue", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([...export2, { id: "u3", primary_email_address: "not-an-email" }]), + ); + await run({ ...baseOptions, allowPartial: true }); + const [first] = listRuns(runsDir()); + expect(first).toMatchObject({ status: "partial" }); + expect(latestUserLines(runsDir(), first!.id).get("u3")).toMatchObject({ + status: "skipped", + code: "in_file", + }); + expect(Bun.stripANSI(captured.err)).not.toContain("to retry them"); + + captured.clear(); + stubClerk({ + existing: [ + { id: "user_u1", external_id: "u1" }, + { id: "user_u2", external_id: "u2" }, + ], + }); + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + expect(error.message).not.toContain("Drop --new-run"); + }); + + // Without the flag that got the earlier run past its checks, its continue + // would refuse the same users again. + test("--new-run's continue carries the --allow-partial the earlier run needed", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([...export2, { id: "u3", primary_email_address: "not-an-email" }]), + ); + stubClerk({ failing: new Set(["u2"]) }); + await run({ ...baseOptions, allowPartial: true }); + const [first] = listRuns(runsDir()); + captured.clear(); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain(`Drop --new-run to continue run ${first!.id}.`); + expect(error.examples?.[0]?.command).toContain("--allow-partial"); + expect(error.examples?.[0]?.command).not.toContain("--new-run"); + }); + + // --allow-partial would start a run that imports nobody. + test("--new-run with every user rejected offers the undo, not --allow-partial", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([...export2, { id: "u3", primary_email_address: "not-an-email" }]), + ); + await run({ ...baseOptions, allowPartial: true }); + const [first] = listRuns(runsDir()); + captured.clear(); + stubClerk({ + existing: [ + { id: "user_u1", external_id: "u1" }, + { id: "user_u2", external_id: "u2" }, + ], + }); + + const error = (await run({ ...baseOptions, newRun: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain(`Undo run ${first!.id} first (`); + expect(error.message).not.toContain("import the rest into a new run"); + expect(error.examples?.map((example) => example.description)).toEqual([ + `Undo run ${first!.id} first`, + ]); + }); + + test("a refusal of a file where no user passes doesn't offer --allow-partial", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([{ id: "u3", primary_email_address: "not-an-email" }]), + ); + + const error = (await run(baseOptions).catch((caught: unknown) => caught)) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain( + "with every user rejected, --allow-partial would import nobody", + ); + expect(error.examples ?? []).toEqual([]); + }); + + // The continue would carry --require-password and leave the same users out. + test("--new-run --require-password doesn't offer a continue that imports nobody", async () => { + await run({ ...baseOptions, requirePassword: true }); + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run({ ...baseOptions, newRun: true, requirePassword: true }).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain("from this file."); + expect(error.message).not.toContain("Drop --new-run"); }); // They never reach the checks, so the preview has to count them itself. @@ -339,7 +600,43 @@ describe("run", () => { await run(baseOptions); expect(process.exitCode).toBe(1); - expect(runsIn(runsDir())[0]).toMatchObject({ status: "partial", counts: { failed: 1 } }); + expect(listRuns(runsDir())[0]).toMatchObject({ status: "partial", counts: { failed: 1 } }); + }); + + // Run later, the key may no longer be exported: the steps name it as a + // placeholder, as `runs ` does, rather than nothing. + test("after failures with an exported key, the retry and undo name the key", async () => { + stubClerk({ failing: new Set(["u2"]) }); + const previous = process.env.CLERK_SECRET_KEY; + process.env.CLERK_SECRET_KEY = "sk_test_x"; + try { + await run({ source: "clerk", input: "export.json", yes: true }); + } finally { + if (previous === undefined) delete process.env.CLERK_SECRET_KEY; + else process.env.CLERK_SECRET_KEY = previous; + } + + const [record] = listRuns(runsDir()); + const err = Bun.stripANSI(captured.err); + expect(err).toContain("--source clerk --secret-key --yes` to retry them"); + expect(err).toContain(`\`clerk migrate undo ${record!.id} --secret-key \``); + }); + + // `runs` reads only a runs folder; the retry is the same import, which + // continues this run, so it drops --new-run. + test("after failures, names the runs listing and the import that retries them", async () => { + stubClerk({ failing: new Set(["u2"]) }); + const dir = path.join(workDir, "runs"); + + await run({ ...baseOptions, runsDir: dir, newRun: true }); + + const [record] = listRuns(dir); + const err = Bun.stripANSI(captured.err); + expect(err).toContain(`\`clerk migrate runs ${record!.id} --runs-dir ${dir}\``); + expect(err).toContain( + `\`clerk migrate import export.json --source clerk --secret-key --runs-dir ${dir} --yes\` to retry them`, + ); + expect(err).not.toContain("--new-run"); }); // Tests run non-TTY, the same signal an agent gives. @@ -373,7 +670,7 @@ describe("run", () => { expect(error.message).toMatch(/nothing was imported\. Reason: .+\. Fix them/); expect(error.examples?.[0]?.command).toContain("--allow-partial --yes"); expect(created()).toHaveLength(0); - expect(runsIn(runsDir())).toHaveLength(0); + expect(listRuns(runsDir())).toHaveLength(0); }); test("with several reasons, names the most common and points --json at the rest", async () => { @@ -419,7 +716,7 @@ describe("run", () => { await run({ ...baseOptions, allowPartial: true }); expect(created()).toEqual(["u1", "u2"]); - const [record] = runsIn(runsDir()); + const [record] = listRuns(runsDir()); expect(record).toMatchObject({ status: "partial", counts: { created: 2, skipped: 1 } }); expect(latestUserLines(runsDir(), record!.id).get("u3")).toMatchObject({ status: "skipped", @@ -438,7 +735,7 @@ describe("run", () => { await run({ ...baseOptions, dryRun: true }); expect(created()).toHaveLength(0); - expect(runsIn(runsDir())).toHaveLength(0); + expect(listRuns(runsDir())).toHaveLength(0); expect(captured.err).toContain("Dry run: nothing was written."); expect(process.exitCode).toBe(2); }); @@ -520,7 +817,7 @@ describe("run", () => { await run({ ...baseOptions, allowPartial: true }); expect(created()).toEqual(["u1"]); - const [record] = runsIn(runsDir()); + const [record] = listRuns(runsDir()); expect(latestUserLines(runsDir(), record!.id).get("u2")?.reason).toContain("100-user limit"); }); @@ -657,7 +954,7 @@ describe("run", () => { delete process.env.CLERK_MIGRATE_CONCURRENCY_LIMIT; } - const [record] = runsIn(runsDir()); + const [record] = listRuns(runsDir()); // The run folder is the only record of who was created. expect(captured.err).toContain(`Run ${record?.id}: `); expect(record?.finishedAt).toBeUndefined(); @@ -726,7 +1023,7 @@ describe("run", () => { const err = Bun.stripANSI(captured.err); expect(err).toContain("Not sent: 1"); expect(err).toContain( - "No more users were sent. Once the limit is raised, run the import again with `--allow-partial --yes` to send them.", + "No more users were sent. Once the limit is raised, run the import again to send them.", ); }); }); @@ -784,22 +1081,475 @@ describe("run", () => { }); }); - describe("re-running", () => { - // Slice 1 has no continuing: the second run sees the first run's users in - // the instance, and refuses until --allow-partial. - test("a second import of the same file rejects the users the first created", async () => { + describe("continuing an earlier run", () => { + test("a complete run is not imported again", async () => { + await run(baseOptions); + requests = []; + + await run(baseOptions); + + expect(created()).toHaveLength(0); + expect(captured.err).toContain("Already imported in run"); + expect(listRuns(runsDir())).toHaveLength(1); + }); + + test("--new-run imports it again as a new run", async () => { + await run(baseOptions); + requests = []; + + await run({ ...baseOptions, newRun: true }); + + expect(created()).toEqual(["u1", "u2"]); + expect(listRuns(runsDir())).toHaveLength(2); + }); + + test("a dry run says what it would start or continue", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run({ ...baseOptions, dryRun: true }); + expect(captured.err).toContain("Would start a new run."); + expect(captured.err).not.toContain("Starting a new run."); + + await run(baseOptions); + const [first] = listRuns(runsDir()); + process.exitCode = 0; + captured.clear(); + await run({ ...baseOptions, dryRun: true }); + expect(captured.err).toContain(`Would continue run ${first!.id}, which finished partial`); + expect(captured.err).not.toContain("Continuing run"); + }); + + // The steps are run later: they need the key this attempt used, not the + // first one's `.env`, which may not be there. + test("a continue records its own key source, which its next steps carry", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + patchRun(runsDir(), first!.id, { target: { ...first!.target, keySource: ".env" } }); + + process.exitCode = 0; + captured.clear(); await run(baseOptions); + + expect(readRun(runsDir(), first!.id)?.target.keySource).toBe("--secret-key"); + expect(Bun.stripANSI(captured.err)).toContain("--secret-key --yes` to retry them"); + }); + + // The run stays under the first attempt's stand-in, which no `--instance` + // resolves, and a bare `--app` resolves development: the key reaches it. + test("a stand-in run continued with --app carries the key, not a bare --app", async () => { + const application = spyOn(plapi, "fetchApplication").mockResolvedValue({ + application_id: "app_1", + instances: [ + { instance_id: "ins_dev", environment_type: "development", secret_key: "sk_test_dev" }, + { instance_id: "ins_1", environment_type: "production", secret_key: "sk_live_x" }, + ], + } as never); + process.env.CLERK_PLATFORM_API_KEY = "ak_x"; + try { + stubClerk({ failing: new Set(["u2"]), instanceDown: true }); + await run({ ...baseOptions, secretKey: "sk_live_x" }); + const [first] = listRuns(runsDir()); + + process.exitCode = 0; + captured.clear(); + stubClerk({ failing: new Set(["u2"]) }); + await run({ + source: "clerk", + input: "export.json", + yes: true, + app: "app_1", + instance: "prod", + }); + + expect(listRuns(runsDir())).toHaveLength(1); + expect(readRun(runsDir(), first!.id)?.target.keySource).toBe("--app"); + const err = Bun.stripANSI(captured.err); + expect(err).toContain("--secret-key --yes` to retry them"); + expect(err).not.toContain("--app app_1"); + } finally { + application.mockRestore(); + delete process.env.CLERK_PLATFORM_API_KEY; + } + }); + + // Created, but its second email never attached: the run is partial, and + // the import says so and offers the continue that attaches it. + test("an attach with no answer is reported, with the continue", async () => { + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev", unverified_email_addresses: "b@x.dev" }, + ]), + ); + 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); + + expect(listRuns(runsDir())[0]).toMatchObject({ status: "partial", counts: { pending: 1 } }); + const err = Bun.stripANSI(captured.err); + expect(err).toContain("Identifiers left to attach: 1 user"); + expect(err).toContain("--secret-key --yes` to retry them"); + + // Without --yes, the consent names the attach, not just "0 users". + globalThis.fetch = clerk; + const error = (await run({ ...baseOptions, yes: false }).catch( + (caught: unknown) => caught, + )) as CliError; + expect(error.message).toContain("will create 0 users, attach identifiers to 1 user."); + + // The --json refusal counts them too, as it counts --require-password's. + captured.clear(); + await run({ ...baseOptions, yes: false, json: true }).catch(() => undefined); + expect(JSON.parse(captured.out)).toMatchObject({ consent: "required", attachOnly: 1 }); + }); + + // A skip the checks named a fix for imports once the setting changes: the + // import offers the same continue `runs ` does, and it finishes the run. + test("a run left partial only by skips offers the continue, which finishes it", async () => { + const settings = (required: boolean) => ({ + attributes: { email_address: { enabled: true, required } }, + enterprise_sso: { enabled: true }, + }); + stubClerk({ settings: settings(true) }); + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([ + { id: "u1", primary_email_address: "a@x.dev" }, + { id: "u2", unverified_email_addresses: "b@x.dev" }, + ]), + ); + await run({ ...baseOptions, allowPartial: true }); + const [first] = listRuns(runsDir()); + expect(first).toMatchObject({ status: "partial", counts: { created: 1, skipped: 1 } }); + expect(Bun.stripANSI(captured.err)).toContain("--secret-key --yes` to retry them"); + + requests = []; + stubClerk({ settings: settings(false) }); + await run({ ...baseOptions, allowPartial: true }); + + expect(created()).toEqual(["u2"]); + expect(readRun(runsDir(), first!.id)?.status).toBe("complete"); + }); + + test("a partial run retries only the users that did not make it, in the same run", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + + requests = []; + process.exitCode = 0; + stubClerk(); + await run(baseOptions); + + expect(created()).toEqual(["u2"]); + expect(captured.err).toContain(`Continuing run ${first!.id}, which finished partial`); + const runs = listRuns(runsDir()); + expect(runs).toHaveLength(1); + expect(runs[0]).toMatchObject({ id: first!.id, status: "complete", counts: { created: 2 } }); + }); + + test("an interrupted run skips the users it already created", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + // A crash never writes a finish time. + const record = readRun(runsDir(), first!.id)!; + delete record.finishedAt; + fs.writeFileSync(path.join(runsDir(), first!.id, "run.json"), JSON.stringify(record)); + + requests = []; + process.exitCode = 0; + stubClerk(); + await run(baseOptions); + + expect(created()).toEqual(["u2"]); + expect(captured.err).toContain("which was interrupted"); + }); + + /** Rewrites a finished run as one a crash stopped: no finish time. */ + const interrupt = (id: string, patch: Record = {}) => { + const record = readRun(runsDir(), id)!; + delete record.finishedAt; + fs.writeFileSync( + path.join(runsDir(), id, "run.json"), + JSON.stringify({ ...record, ...patch }), + ); + }; + + // The create went out and the run stopped before the answer came back. + test("an interrupted run adopts a user Clerk created with no ID on record", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating" })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; stubClerk({ existing: [ - { id: "user_u1", external_id: "u1" }, - { id: "user_u2", external_id: "u2" }, + { id: "user_found", external_id: "u2", private_metadata: { clerkMigrateRun: first!.id } }, ], }); + await run(baseOptions); + + expect(created()).toEqual([]); + expect(captured.err).toContain("1 user whose create was cut off is already in the instance"); + expect(latestUserLines(runsDir(), first!.id).get("u2")).toMatchObject({ + status: "created", + clerkId: "user_found", + }); + expect(readRun(runsDir(), first!.id)?.status).toBe("complete"); + }); + + // A reject would stop a create; an adopted user needs none. Rejected, it + // stayed `creating` and the run could never finish. + test("an adopted user a check would reject still finishes the run", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating" })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk({ + // Legal consent turned on since: u2 has no acceptance on record. + settings: { + attributes: { email_address: { enabled: true } }, + sign_up: { legal_consent_enabled: true }, + enterprise_sso: { enabled: true }, + }, + existing: [ + { id: "user_found", external_id: "u2", private_metadata: { clerkMigrateRun: first!.id } }, + ], + }); + await run(baseOptions); + + expect(latestUserLines(runsDir(), first!.id).get("u2")).toMatchObject({ + status: "created", + clerkId: "user_found", + }); + expect(readRun(runsDir(), first!.id)?.status).toBe("complete"); + }); + + // Same source ID, but no marker from this run: an app or another tool's + // user, so it is never adopted. + test("an interrupted run does not adopt a match without its marker", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating" })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk({ + existing: [{ id: "user_theirs", external_id: "u2", private_metadata: {} }], + }); + await run({ ...baseOptions, allowPartial: true }); + + expect(created()).toEqual([]); + expect(captured.err).not.toContain("whose create was cut off"); + expect(latestUserLines(runsDir(), first!.id).get("u2")).toMatchObject({ + status: "skipped", + reason: "already in the instance, with this source ID", + }); + }); + + test("an interrupted run creates a user whose in-flight create never landed", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.appendFileSync( + path.join(runsDir(), first!.id, "users.ndjson"), + `${JSON.stringify({ sourceId: "u2", status: "creating" })}\n`, + ); + interrupt(first!.id); + + requests = []; + process.exitCode = 0; + stubClerk(); + await run(baseOptions); + + expect(created()).toEqual(["u2"]); + }); + + test("a continued run with nothing left to do is finished", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + interrupt(first!.id); + requests = []; + + await run(baseOptions); + + expect(created()).toEqual([]); + expect(captured.err).toContain("No users left to import"); + expect(readRun(runsDir(), first!.id)).toMatchObject({ status: "complete" }); + expect(readRun(runsDir(), first!.id)?.finishedAt).toBeDefined(); + }); + + // `runs `'s undo is built from the key the settling attempt used. + test("a continued run with nothing left to do records its own key source", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + interrupt(first!.id); + patchRun(runsDir(), first!.id, { target: { ...first!.target, keySource: ".env" } }); + + await run(baseOptions); + + expect(readRun(runsDir(), first!.id)?.target.keySource).toBe("--secret-key"); + }); + + // Automation reads the status from the JSON, not run.json. + test("--json reports a continued run with nothing left as finished", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + // A crash leaves the record as it was while running. + interrupt(first!.id, { status: "running" }); + requests = []; + captured.clear(); + + await run({ ...baseOptions, json: true }); + + expect(JSON.parse(captured.out)).toMatchObject({ + nothingToImport: true, + run: { id: first!.id, status: "complete", finishedAt: expect.any(String) }, + }); + }); + + // "Interrupted, so undo it and start over": the undo marks the run undone + // but leaves it with no finish time. + test("an interrupted run that was then undone is imported again as a new run", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + interrupt(first!.id, { status: "undone" }); + requests = []; + + await run(baseOptions); - expect(await exitCodeOf(run(baseOptions))).toBe(EXIT_CODE.USAGE); - expect(captured.err).toContain("already in the instance, with this source ID"); expect(created()).toEqual(["u1", "u2"]); - expect(runsIn(runsDir())).toHaveLength(1); + expect(listRuns(runsDir()).filter((record) => record.kind === "import")).toHaveLength(2); + }); + + test("a run with an undo that did not finish refuses with exit 2", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + const undoRun = startRun(runsDir(), { + kind: "undo", + target: { instanceId: "ins_1" }, + undoes: first!.id, + }); + undoRun.append({ sourceId: "u1", status: "deleted", clerkId: "user_u1" }); + undoRun.append({ sourceId: "u2", status: "failed", clerkId: "user_u2" }); + undoRun.finish(); + requests = []; + + const error = (await run(baseOptions).catch((caught: unknown) => caught)) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + // With the key it was given, or it reaches whatever key it finds next. + expect(error.message).toContain(`clerk migrate undo ${first!.id} --secret-key `); + expect(created()).toEqual([]); + }); + + test("an undone run is imported again as a new run", async () => { + await run(baseOptions); + const [first] = listRuns(runsDir()); + const record = readRun(runsDir(), first!.id)!; + fs.writeFileSync( + path.join(runsDir(), first!.id, "run.json"), + JSON.stringify({ ...record, status: "undone" }), + ); + requests = []; + + await run(baseOptions); + + expect(created()).toEqual(["u1", "u2"]); + expect(listRuns(runsDir())).toHaveLength(2); + }); + + test("a run another live process holds refuses with exit 2", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + const record = readRun(runsDir(), first!.id)!; + delete record.finishedAt; + fs.writeFileSync(path.join(runsDir(), first!.id, "run.json"), JSON.stringify(record)); + // PID 1 is always alive, and never this test. + fs.writeFileSync(path.join(runsDir(), first!.id, "lock"), "1"); + + expect(await exitCodeOf(run(baseOptions))).toBe(EXIT_CODE.USAGE); + }); + + // An edited file is a different job. + // Rate limiting right after a large import is when this lookup fails, and + // that is when someone re-runs after an interruption. + test("refuses to start over when Clerk can't name the instance a run of this file was in", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + + requests = []; + process.exitCode = 0; + stubClerk({ instanceDown: true }); + await expect(run(baseOptions)).rejects.toThrow(/did not confirm which instance/); + expect(created()).toEqual([]); + expect(listRuns(runsDir())).toHaveLength(1); + + await run({ ...baseOptions, newRun: true, allowPartial: true }); + expect(listRuns(runsDir())).toHaveLength(2); + }); + + // Fixing the row a partial run skipped changes the file, so its run meets + // the users the first one created: the refusal says which run that was. + test("a changed file's refusal names the earlier run that created the users it rejects", async () => { + stubClerk({ failing: new Set(["u2"]) }); + await run(baseOptions); + const [first] = listRuns(runsDir()); + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([export2[0], { ...export2[1], primary_email_address: "fixed@x.dev" }]), + ); + requests = []; + process.exitCode = 0; + stubClerk({ existing: [{ id: "user_u1", external_id: "u1" }] }); + + const error = (await run(baseOptions).catch((caught: unknown) => caught)) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain( + `1 of them was created by run ${first!.id}, from an earlier version of this file.`, + ); + expect(error.examples?.map((example) => example.command)).toContain( + `clerk migrate undo ${first!.id} --secret-key `, + ); + expect(created()).toEqual([]); + }); + + test("a changed file is a new run", async () => { + await run(baseOptions); + fs.writeFileSync( + path.join(workDir, "export.json"), + JSON.stringify([...export2, { id: "u3", primary_email_address: "c@x.dev" }]), + ); + requests = []; + + await run(baseOptions); + + expect(listRuns(runsDir())).toHaveLength(2); }); }); diff --git a/packages/cli-core/src/commands/migrate/run.ts b/packages/cli-core/src/commands/migrate/run.ts index e089cd8ebb..08270ea5fe 100644 --- a/packages/cli-core/src/commands/migrate/run.ts +++ b/packages/cli-core/src/commands/migrate/run.ts @@ -6,9 +6,10 @@ * * An import goes through the same steps every time: * - * 1. Settle the file, the source and the target instance, and print the target. - * 2. Load the users: read the file, map it through the source, then normalize - * and validate it. + * 1. Settle the file, the source and the target instance, and print the + * target. + * 2. Decide whether this continues an earlier run of the same file, source and + * instance, from the run store. * 3. Run `checkImport()` against the real instance. `--dry-run` stops here. * 4. Refuse on any reject unless `--allow-partial`, then ask for consent. * Nothing is written without `--yes` or a yes at the prompt. @@ -28,21 +29,59 @@ import { } from "../../lib/errors.ts"; import { resolveKeylessTarget } from "../../lib/keyless-target.ts"; import { quoteArg } from "../../lib/json-body.ts"; +import { + carryFlags, + importFlags, + pickImportFlags, + runsCommand, + undoCommand, +} from "./lib/carry-flags.ts"; +import { plural } from "./lib/plural.ts"; +import { canPrompt } from "./lib/terminal.ts"; import { log } from "../../lib/log.ts"; import { NEXT_STEPS, printAgentNextSteps } from "../../lib/next-steps.ts"; import { confirm } from "../../lib/prompts.ts"; import { interruptedExitCode } from "../../lib/signals.ts"; import { withGutter, withSpinner } from "../../lib/spinner.ts"; -import { isAgent, isHuman } from "../../mode.ts"; import { importUsers } from "./import-users.ts"; -import { checkImport, type ImportChecks } from "./lib/checks.ts"; +import { ALREADY_IMPORTED, checkImport, type ImportChecks } from "./lib/checks.ts"; import { fetchInstanceSettings, fetchUserCount } from "./lib/clerk-config.ts"; import { DEV_USER_LIMIT, resolveLimits, type InstanceType } from "./lib/instance.ts"; -import { resolveRunsDir, sha256File, startRun, type Run, type RunRecord } from "./lib/run-store.ts"; +import { + continueRun, + latestUserLines, + listRuns, + readUserLines, + resolveRunsDir, + runDir, + runState, + sha256File, + startRun, + type Run, + type RunRecord, + type UserLine, + hasRetryableUsers, + NO_PASSWORD_CODE, + IN_FILE_CODE, + unfinishedUndoMessage, + inUseMessage, + openUndoOf, + importsOfFile, + undosOf, + withAttempt, +} from "./lib/run-store.ts"; import { withProgress } from "./lib/progress.ts"; import { createApiScheduler } from "./lib/scheduler.ts"; import { readSupabaseRows } from "./lib/supabase-providers.ts"; -import { printTarget, resolveClerkTarget } from "./lib/target.ts"; +import { + isClerkInstanceId, + isStandInInstanceId, + keyInstanceId, + laterCarry, + printTarget, + resolveClerkTarget, +} from "./lib/target.ts"; +import { findInFlight } from "./lib/user-lookup.ts"; import { fileExists, getFileType, @@ -72,6 +111,8 @@ export type MigrateRunOptions = { dryRun?: boolean; /** Import the users that pass, and record the rest as skipped. */ allowPartial?: boolean; + /** Start a fresh run even when an earlier one matches. */ + newRun?: boolean; yes?: boolean; json?: boolean; secretKey?: string; @@ -81,13 +122,6 @@ export type MigrateRunOptions = { runsDir?: string; }; -const plural = (count: number, word: string) => `${count} ${word}${count === 1 ? "" : "s"}`; - -/** Nobody can answer a prompt: an agent, a non-TTY run, or `--json`. */ -function canPrompt(options: MigrateRunOptions): boolean { - return !options.json && isHuman() && !isAgent(); -} - /** * Validates what a run needs before anything is read or sent. * @@ -270,6 +304,84 @@ async function applySource(options: MigrateRunOptions): Promise + record.target.instanceId === match.instanceId || + record.target.instanceId === match.keyInstanceId, + ); + if (!latest) { + // Clerk did not name the instance (GET /v1/instance failed), so a run of + // this file recorded under a real ID can be neither matched nor ruled out. + // Starting over would reject every user it created and leave it unfinished. + if (isStandInInstanceId(match.instanceId)) { + const unconfirmed = ofFile.find((record) => isClerkInstanceId(record.target.instanceId)); + if (unconfirmed) { + throwUsageError( + `Clerk did not confirm which instance this key addresses, so this import cannot tell whether run ${unconfirmed.id} is for it. ` + + "Nothing was imported. Try again, or pass --new-run to start a new run.", + ); + } + } + return { kind: "new" }; + } + + const state = runState(runsDir, latest); + if (state === "running") { + throwUsageError( + inUseMessage(runsDir, latest.id, "importing this file", "Wait for it to finish."), + ); + } + if (state === "undone") return { kind: "new" }; + + const openUndo = openUndoOf(runsDir, latest.id, runs); + if (openUndo) throwUsageError(unfinishedUndoMessage(latest.id, openUndo.id, carry)); + // Undone in full, though the import run was never marked so. + if (undosOf(runs, latest.id).length > 0) return { kind: "new" }; + + if (state === "complete") return { kind: "complete", record: latest }; + return { + kind: "continue", + record: latest, + because: state === "interrupted" ? "interrupted" : "partial", + }; +} + // --- Reporting ------------------------------------------------------------- /** How many rejected source IDs to name per reason before summing the rest. */ @@ -347,12 +459,16 @@ function formatSummary( if (skipped > 0) lines.push(`${yellow("Skipped:")} ${skipped}`); // The user quota stopped the run: these go out on a re-run. if (summary.notSent > 0) lines.push(`${yellow("Not sent:")} ${summary.notSent}`); + // Created, but an email or phone did not attach: a re-run attaches it. + const pending = record.counts.pending ?? 0; + if (pending > 0) + lines.push(`${yellow("Identifiers left to attach:")} ${plural(pending, "user")}`); // Printed here, after the progress bar, which would redraw over it mid-run. if (summary.stopReason) { lines.push( "", yellow( - "No more users were sent. Once the limit is raised, run the import again with `--allow-partial --yes` to send them.", + "No more users were sent. Once the limit is raised, run the import again to send them.", ), ); } @@ -376,23 +492,37 @@ function formatSummary( return lines; } +/** + * Once every user is in, the run is only needed for `undo`, and it holds user + * data. + */ +function cleanupLines(runsDir: string, record: RunRecord): string[] { + return [ + `Keep run ${record.id} while you might still undo it. After that:`, + dim(` rm -rf ${quoteArg(runDir(runsDir, record.id))}`), + ]; +} + // --- The import ------------------------------------------------------------ /** - * The exact command that would carry on from here, for the consent and - * refusal messages. Every value is shell-quoted; secrets are placeholders. + * The exact command that would carry on from here. The consent and refusal + * messages, run straight away, use the live flags; the retry step after a run, + * run later, passes `carry` built from the run's target. Every value is + * shell-quoted; secrets are placeholders. */ -function commandFor(options: MigrateRunOptions, extra: string[]) { +function commandFor( + options: MigrateRunOptions, + extra: string[], + /** The target and runs-folder flags; a command for later passes the run's. */ + carry = carryFlags(options), +) { const input = options.input ?? options.file ?? ""; const parts = ["clerk migrate import", quoteArg(input)]; if (options.source) parts.push("--source", quoteArg(options.source)); - if (options.allowPartial) parts.push("--allow-partial"); - if (options.requirePassword) parts.push("--require-password"); - if (options.skipLegalChecks) parts.push("--skip-legal-checks"); - if (options.secretKey) parts.push("--secret-key", ""); - if (options.app) parts.push("--app", quoteArg(options.app)); - if (options.instance) parts.push("--instance", quoteArg(options.instance)); - if (options.runsDir) parts.push("--runs-dir", quoteArg(options.runsDir)); + if (options.newRun) parts.push("--new-run"); + parts.push(...importFlags(options)); + parts.push(...carry); if (options.json) parts.push("--json"); return [...parts, ...extra].join(" "); } @@ -411,13 +541,75 @@ function rejectSummary(checks: ImportChecks, json = false): string { return ` ${why}${json ? " See `checks.rejects` for each user." : ""}`; } -/** Records the checks' rejects as skipped users, so the run says who they were. */ +/** + * The earlier import run, of this source into this instance, that created the + * users the checks reject as already there: how many of them, and whether it + * ran this same file. Fixing a skipped row changes the file, and the changed + * file's run meets them; so does the same file run with `--new-run`. + */ +function earlierRunHolding( + runsDir: string, + source: string, + instanceId: string, + checks: ImportChecks, + sha256: string, +): { id: string; count: number; sameFile: boolean } | undefined { + const already = new Set( + checks.rejects.filter((reject) => reject.reason === ALREADY_IMPORTED).map((r) => r.sourceId), + ); + if (already.size === 0) return undefined; + for (const record of listRuns(runsDir)) { + if (record.kind !== "import" || record.source !== source) continue; + if (record.target.instanceId !== instanceId || runState(runsDir, record) === "undone") continue; + let count = 0; + for (const line of latestUserLines(runsDir, record.id).values()) { + if (line.status === "created" && already.has(line.sourceId)) count++; + } + if (count > 0) return { id: record.id, count, sameFile: record.file?.sha256 === sha256 }; + } + return undefined; +} + +/** + * The run the same import without `--new-run` would continue, when that is + * run `id` and it would import someone: `findResume` has to pick it, and it + * has to hold a user the continue retries. A `--require-password` skip is one + * when this import drops the flag. + */ +function continuesWithoutNewRun( + runsDir: string, + match: Parameters[1], + carry: string[], + id: string, + requirePassword: boolean | undefined, +): RunRecord | undefined { + let resume: ResumeCase; + try { + resume = findResume(runsDir, match, carry); + } catch { + // It would refuse: see findResume's @throws. + return undefined; + } + if (resume.kind !== "continue" || resume.record.id !== id) return undefined; + const lines = [...latestUserLines(runsDir, id).values()]; + const retries = + hasRetryableUsers(resume.record, lines) || + (!requirePassword && + lines.some((line) => line.status === "skipped" && line.code === NO_PASSWORD_CODE)); + return retries ? resume.record : undefined; +} + +/** + * Records the checks' rejects as skipped users, so the run says who they were. + * The checks never reject an adopted user: it is created already. + */ function recordRejects(run: Run, checks: ImportChecks): void { - for (const { sourceId, reason, keptSourceId } of checks.rejects) { + for (const { sourceId, reason, keptSourceId, inFile } of checks.rejects) { run.append({ sourceId, status: "skipped", reason: keptSourceId ? `${reason} (kept: ${keptSourceId})` : reason, + ...(inFile ? { code: IN_FILE_CODE } : {}), }); } } @@ -435,6 +627,8 @@ export async function run(rawOptions: MigrateRunOptions): Promise { } const { source, file } = validateRunOptions(options); + // The flags every suggested command needs to reach this runs folder and instance. + const carry = carryFlags(options); await withGutter( "Migrating users to Clerk", @@ -446,18 +640,89 @@ export async function run(rawOptions: MigrateRunOptions): Promise { const filePath = resolveImportFilePath(file); const sha256 = sha256File(filePath); const runsDir = await resolveRunsDir(options.runsDir); + + const match = { + sha256, + source, + instanceId: target.instanceId, + keyInstanceId: keyInstanceId(secretKey), + }; + const resume: ResumeCase = options.newRun + ? { kind: "new" } + : findResume(runsDir, match, carry); + + if (resume.kind === "complete") { + if (options.json) { + log.data(JSON.stringify({ target, run: resume.record, alreadyImported: true }, null, 2)); + } else { + log.success( + `Already imported in run ${resume.record.id}. Pass --new-run to import it again.`, + ); + } + return; + } + + // Users the run being continued already created are done: they are not + // checked or sent again. Those whose extra identifiers never attached + // get just the attaches. + const continued = resume.kind === "continue" ? resume.record : undefined; + // What the plan below is built from; checked again once the run's lock + // is held, since a prompt can wait while an undo or a continue writes. + const plannedLines = continued ? readUserLines(runsDir, continued.id).length : undefined; + const done = new Map(); + const attachOnly: UserLine[] = []; + const inFlight: string[] = []; + if (continued) { + for (const line of latestUserLines(runsDir, continued.id).values()) { + if (line.status === "creating") inFlight.push(line.sourceId); + if (line.status !== "created" || !line.clerkId) continue; + done.set(line.sourceId, line.clerkId); + if (line.pending?.length) attachOnly.push(line); + } + } + + // Creates the run stopped with no answer to. Those Clerk holds are + // adopted: checked as usual, but never created again. const schedule = createApiScheduler(limits.concurrencyLimit, limits.rateLimit); + const adopted = new Map(); + if (continued) { + const found = await findInFlight({ + runId: continued.id, + sourceIds: inFlight, + secretKey, + schedule, + }); + for (const user of found) adopted.set(user.sourceId, user.clerkId); + } + + if (!options.json) { + // A dry run writes nothing, so it only says what the import would do. + 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.` + : options.dryRun + ? "Would start a new run." + : "Starting a new run.", + ); + if (adopted.size > 0) { + log.info( + `${plural(adopted.size, "user")} whose create was cut off ${adopted.size === 1 ? "is" : "are"} already in the instance, and won't be created again.`, + ); + } + } const loaded = await withSpinner(`Loading users from ${file}...`, async () => loadUsersFromFile(file, source), ); - let users = loaded.users; + let users = loaded.users.filter((user) => !done.has(user.userId)); + const failures = loaded.failures.filter((failure) => !done.has(failure.userId)); // An instruction about this import, not a prediction: users without a - // password are left out of the job rather than recorded as rejects. + // password are left out of the checks and recorded as skipped. let withoutPassword: User[] = []; if (options.requirePassword) { - withoutPassword = users.filter((user) => !user.password); + withoutPassword = users.filter((user) => !user.password && !adopted.has(user.userId)); if (withoutPassword.length > 0 && !options.json) { log.info( `--require-password: leaving out ${plural(withoutPassword.length, "user")} without a password.`, @@ -512,7 +777,7 @@ export async function run(rawOptions: MigrateRunOptions): Promise { checkImport({ users, skipLegalChecks, - failures: loaded.failures, + failures, unknownFields: loaded.unknownFields, ...(supabaseRows ? { supabaseRows } : {}), settings, @@ -521,6 +786,8 @@ export async function run(rawOptions: MigrateRunOptions): Promise { target, secretKey, schedule, + adoptedClerkIds: new Set(adopted.values()), + adoptedSourceIds: new Set(adopted.keys()), spinner, }), ); @@ -528,11 +795,22 @@ export async function run(rawOptions: MigrateRunOptions): Promise { const refused = checks.rejects.length > 0 && !options.allowPartial; // The users --require-password left out never reach the checks, so the // JSON says how many, or `checks.total` would not add up to the file. - const leftOut = withoutPassword.length > 0 ? { withoutPassword: withoutPassword.length } : {}; + // Attach-only users skip them too, and a consent refusal names them. + const leftOut = { + ...(withoutPassword.length > 0 ? { withoutPassword: withoutPassword.length } : {}), + ...(attachOnly.length > 0 ? { attachOnly: attachOnly.length } : {}), + }; const preview = (extra: Record) => log.data( JSON.stringify( - { target, run: null, checks: checksJson(checks), ...leftOut, ...extra }, + { + target, + run: continued ?? null, + resume: resume.kind, + checks: checksJson(checks), + ...leftOut, + ...extra, + }, null, 2, ), @@ -553,17 +831,64 @@ export async function run(rawOptions: MigrateRunOptions): Promise { if (refused) { if (options.json) preview({ refused: true }); + const earlier = earlierRunHolding(runsDir, source, target.instanceId, checks, sha256); + const undoEarlier = earlier ? undoCommand(earlier.id, carry) : undefined; + const earlierRun = + options.newRun && earlier + ? continuesWithoutNewRun(runsDir, match, carry, earlier.id, options.requirePassword) + : undefined; + // The flags that got that run past its checks: without them its + // continue refuses the same users again. + const recorded = + earlierRun?.flags && !Array.isArray(earlierRun.flags) ? earlierRun.flags : {}; + const continueEarlier = earlierRun + ? commandFor( + { + ...options, + newRun: false, + allowPartial: options.allowPartial || recorded.allowPartial, + skipLegalChecks: options.skipLegalChecks || recorded.skipLegalChecks, + }, + ["--yes"], + ) + : undefined; + // With every user rejected, --allow-partial would start a run that + // imports nobody, so it isn't offered. + const nonePass = checks.importable.length === 0 && attachOnly.length === 0; + const ways = earlier + ? [ + ...(nonePass ? [] : ["pass --allow-partial to import the rest into a new run"]), + `undo run ${earlier.id} first (\`${undoEarlier}\`) to import the whole file again`, + ].join(", or ") + : ""; throwUsageError( `${plural(checks.rejects.length, "user")} would be rejected, so nothing was imported.` + `${rejectSummary(checks, options.json)} ` + - "Fix them, or pass --allow-partial to import the rest and record them as skipped.", + (earlier + ? `${earlier.count} of them ${earlier.count === 1 ? "was" : "were"} created by run ${earlier.id}, from ${earlier.sameFile ? "this file" : "an earlier version of this file"}. ` + + (continueEarlier + ? `Drop --new-run to continue run ${earlier.id}. Or ${ways}.` + : `${ways[0]!.toUpperCase()}${ways.slice(1)}.`) + : nonePass + ? "Fix them: with every user rejected, --allow-partial would import nobody." + : "Fix them, or pass --allow-partial to import the rest and record them as skipped."), undefined, undefined, [ - { - command: commandFor(options, ["--allow-partial", "--yes"]), - description: "Import the users that pass", - }, + ...(continueEarlier + ? [{ command: continueEarlier, description: `Continue run ${earlier!.id}` }] + : []), + ...(nonePass + ? [] + : [ + { + command: commandFor(options, ["--allow-partial", "--yes"]), + description: "Import the users that pass", + }, + ]), + ...(undoEarlier + ? [{ command: undoEarlier, description: `Undo run ${earlier!.id} first` }] + : []), ], ); } @@ -571,24 +896,39 @@ export async function run(rawOptions: MigrateRunOptions): Promise { if ( checks.importable.length === 0 && checks.rejects.length === 0 && - withoutPassword.length === 0 + withoutPassword.length === 0 && + attachOnly.length === 0 ) { - if (options.json) preview({ nothingToImport: true }); + // Settled, so a continued run is finished rather than left interrupted, + // and the JSON reports it as finished, as run.json now does. + const finished = continued + ? continueRun( + runsDir, + withAttempt(continued, target, pickImportFlags(options)), + plannedLines, + carry, + ).finish() + : undefined; + if (options.json) + preview({ nothingToImport: true, ...(finished ? { run: finished } : {}) }); else log.warn("No users left to import."); return; } // Rule 1: nothing is written without consent — `--yes`, or a yes at a // prompt. An agent, a non-TTY run and `--json` never prompt. With users - // skipped, the question names them: "Import 0 users?" hides the skips. + // skipped or attaches left, the question names them: "Import 0 users?" + // hides both. const toCreate = plural(checks.importable.length, "user"); + const attaches = + attachOnly.length > 0 ? `, attach identifiers to ${plural(attachOnly.length, "user")}` : ""; const skipping = checks.rejects.length + withoutPassword.length; const skips = skipping > 0 ? ` and skip ${plural(skipping, "user")}` : ""; if (!options.yes) { if (!canPrompt(options)) { if (options.json) preview({ consent: "required" }); throwUsageError( - `\`clerk migrate import\` will create ${toCreate}${skips}. Pass --yes to confirm.`, + `\`clerk migrate import\` will create ${toCreate}${attaches}${skips}. Pass --yes to confirm.`, undefined, undefined, [ @@ -601,7 +941,8 @@ export async function run(rawOptions: MigrateRunOptions): Promise { } log.blank(); const proceed = await confirm({ - message: skips ? `Create ${toCreate}${skips}?` : `Import ${toCreate}?`, + message: + skips || attaches ? `Create ${toCreate}${attaches}${skips}?` : `Import ${toCreate}?`, default: false, }); if (!proceed) throwUserAbort(); @@ -609,12 +950,16 @@ export async function run(rawOptions: MigrateRunOptions): Promise { // Gitignored only now, once there is consent to write a run. await resolveRunsDir(options.runsDir, { write: true }); - const run = startRun(runsDir, { - kind: "import", - target, - source, - file: { path: filePath, sha256 }, - }); + const flags = pickImportFlags(options); + const run = continued + ? continueRun(runsDir, withAttempt(continued, target, flags), plannedLines, carry) + : startRun(runsDir, { + kind: "import", + target, + source, + file: { path: filePath, sha256 }, + flags, + }); // Printed now, not on the way out: on a Ctrl-C the signal handler exits // before the import returns, and the folder is the only record of who // was created. @@ -625,19 +970,23 @@ export async function run(rawOptions: MigrateRunOptions): Promise { sourceId: user.userId, status: "skipped", reason: "no password (--require-password)", + code: NO_PASSWORD_CODE, }); } const summary: ImportSummary = - checks.importable.length > 0 + checks.importable.length > 0 || attachOnly.length > 0 ? await withProgress( - { total: checks.importable.length, verb: "created" }, + { total: checks.importable.length + attachOnly.length, verb: "created" }, async (progress) => importUsers({ users: checks.importable, secretKey, limits, record: run.append, + runId: run.record.id, + attachOnly, + adopted, skipPasswordRequirement: !options.requirePassword, progress, }), @@ -668,12 +1017,14 @@ export async function run(rawOptions: MigrateRunOptions): Promise { { target, run: record, + resume: resume.kind, checks: checksJson(checks), ...leftOut, result: { created: summary.successful, failed: summary.failed, notSent: summary.notSent, + pending: record.counts.pending ?? 0, ...(summary.stopReason ? { stopReason: summary.stopReason } : {}), skipped: checks.rejects.length + withoutPassword.length, errors: [...summary.errorBreakdown].map(([error, count]) => ({ error, count })), @@ -700,11 +1051,30 @@ export async function run(rawOptions: MigrateRunOptions): Promise { )) { log.info(line); } + if (record.status === "complete") { + log.blank(); + for (const line of cleanupLines(runsDir, record)) log.info(line); + } + // `runs` takes only --runs-dir. + const runsStep = runsCommand(carryFlags({ runsDir: options.runsDir }), record.id); + // Run later, maybe without the key exported: from the run's target. + const later = laterCarry(options.runsDir, record.target); + const undo = undoCommand(record.id, later); + // A partial run continues, as `runs ` says: a skip fixed since (a + // setting the checks named) imports then. Not when only `--require-password` + // skips are left: the continue would leave them out again. const steps = - summary.failed > 0 - ? NEXT_STEPS.MIGRATE_DONE_WITH_ERRORS(run.dir) - : NEXT_STEPS.MIGRATE_DONE(run.dir); + record.status === "partial" && + hasRetryableUsers(record, latestUserLines(runsDir, record.id).values()) + ? NEXT_STEPS.MIGRATE_PARTIAL( + runsStep, + // The same import again continues this run; --new-run would not. + // Run as printed: consent was given to this run, at the prompt or by -y. + commandFor({ ...options, newRun: false }, ["--yes"], later), + undo, + ) + : NEXT_STEPS.MIGRATE_DONE(runsStep, undo); setNextSteps(steps); printAgentNextSteps(steps); }, diff --git a/packages/cli-core/src/commands/migrate/runs.test.ts b/packages/cli-core/src/commands/migrate/runs.test.ts new file mode 100644 index 0000000000..eb2c4b6cb4 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/runs.test.ts @@ -0,0 +1,418 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { quoteArg } from "../../lib/json-body.ts"; +import { useCaptureLog } from "../../test/lib/stubs.ts"; +import { + continueRun, + type RunRecord, + sha256File, + startRun, + type StartRunInit, +} from "./lib/run-store.ts"; +import { runs } from "./runs.ts"; + +const captured = useCaptureLog(); + +let runsDir: string; + +beforeEach(() => { + runsDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-runs-"))); +}); + +afterEach(() => { + fs.rmSync(runsDir, { recursive: true, force: true }); +}); + +const TARGET = { appLabel: "My App", env: "development", instanceId: "ins_1" }; + +/** The file the fixture's import read, so a continue can find it unchanged. */ +const usersFile = () => path.join(runsDir, "users.json"); + +function partialImport(init: Partial = {}) { + fs.writeFileSync(usersFile(), "[]"); + const run = startRun(runsDir, { + kind: "import", + target: TARGET, + source: "clerk", + file: { path: usersFile(), sha256: sha256File(usersFile()) }, + // What an import with none of the behaviour flags records. + flags: {}, + ...init, + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "b", status: "failed", error: "That email address is taken." }); + run.append({ sourceId: "c", status: "failed", error: "That email address is taken." }); + run.append({ + sourceId: "d", + status: "skipped", + reason: "no password (--require-password)", + code: "no_password", + }); + return run.finish(); +} + +/** A finished import of the same file and source, a second after `record`. */ +function newerImport(record: RunRecord, instanceId: string) { + const run = startRun(runsDir, { + kind: "import", + target: { ...record.target, instanceId }, + source: "clerk", + file: record.file, + }); + run.update({ startedAt: new Date(Date.parse(record.startedAt) + 1000).toISOString() }); + return run.finish(); +} + +describe("runs", () => { + test("names the runs folder, then lists each run", async () => { + const record = partialImport(); + + await runs(undefined, { runsDir }); + + expect(captured.err).toContain(`Runs folder: ${runsDir}`); + expect(captured.err).toContain(record.id); + expect(captured.err).toContain("My App (development, ins_1)"); + expect(captured.err).toContain("1 created, 2 failed, 1 skipped"); + }); + + // Counts are written at the finish, which an interrupted run never reaches. + test("counts an interrupted run from its user lines", async () => { + const run = startRun(runsDir, { + kind: "import", + target: { env: "development", instanceId: "ins_1" }, + source: "clerk", + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "b", status: "created", clerkId: "user_b" }); + fs.rmSync(path.join(run.dir, "lock")); + + await runs(undefined, { runsDir }); + + expect(captured.err).toContain("interrupted"); + expect(captured.err).toContain("2 created"); + }); + + test("says so when there are no runs", async () => { + await runs(undefined, { runsDir }); + expect(captured.err).toContain("No migration runs yet."); + }); + + test("--json lists the runs on stdout, with each one's state", async () => { + const record = partialImport(); + + await runs(undefined, { runsDir, json: true }); + + const parsed = JSON.parse(captured.out) as { runsDir: string; runs: { id: string }[] }; + expect(parsed).toMatchObject({ runsDir, runs: [{ id: record.id, state: "partial" }] }); + }); +}); + +describe("runs ", () => { + test("shows the counts, the error breakdown and who did not make it", async () => { + const record = partialImport(); + + await runs(record.id, { runsDir }); + + expect(captured.err).toContain("partial"); + expect(Bun.stripANSI(captured.err)).toContain("2 users: That email address is taken."); + expect(captured.err).toContain("Failed (2)"); + expect(captured.err).toContain("Skipped (1)"); + expect(captured.err).toContain("no password (--require-password)"); + expect(captured.err).toContain(path.join(runsDir, record.id, "users.ndjson")); + }); + + test("--json returns the run, its errors, and the failed and skipped users", async () => { + const record = partialImport(); + + await runs(record.id, { runsDir, json: true }); + + const parsed = JSON.parse(captured.out) as Record; + expect(parsed).toMatchObject({ + run: { id: record.id, state: "partial", counts: { created: 1, failed: 2, skipped: 1 } }, + errors: [{ error: "That email address is taken.", count: 2 }], + failed: [{ sourceId: "b" }, { sourceId: "c" }], + skipped: [{ sourceId: "d" }], + }); + }); + + // The import counted these as failed, so `runs` lists them and why: a + // create no answer confirmed, and a user with an email left to attach. + test("shows creates no answer confirmed, and identifiers left to attach", async () => { + fs.writeFileSync(usersFile(), "[]"); + const run = startRun(runsDir, { + kind: "import", + target: TARGET, + source: "clerk", + file: { path: usersFile(), sha256: sha256File(usersFile()) }, + }); + run.append({ sourceId: "a", status: "creating" }); + run.append({ sourceId: "a", status: "creating", error: "bad gateway (a re-run checks)" }); + run.append({ + sourceId: "b", + status: "created", + clerkId: "user_b", + pending: [{ kind: "email", value: "b2@x.dev", verified: true }], + }); + const record = run.finish(); + + await runs(record.id, { runsDir, json: true }); + expect(JSON.parse(captured.out)).toMatchObject({ + run: { state: "partial", counts: { creating: 1, pending: 1 } }, + errors: [{ error: "bad gateway (a re-run checks)", count: 1 }], + unconfirmed: [{ sourceId: "a" }], + }); + + captured.clear(); + await runs(record.id, { runsDir }); + const err = Bun.stripANSI(captured.err); + expect(err).toContain("1 created, 1 left to attach, 1 not confirmed"); + expect(err).toContain("Not confirmed (Clerk may have created them; a continue checks) (1)"); + expect(err).toContain("1 user: bad gateway (a re-run checks)"); + }); + + // A stopped import's unsent users have no lines: only the count says so. + test("shows the users a stopped import never sent, in the list and the run", async () => { + fs.writeFileSync(usersFile(), "[]"); + const run = startRun(runsDir, { + kind: "import", + target: TARGET, + source: "clerk", + file: { path: usersFile(), sha256: sha256File(usersFile()) }, + flags: {}, + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + const record = run.finish({ notSent: 9000 }); + + await runs(undefined, { runsDir }); + expect(Bun.stripANSI(captured.err)).toContain("1 created, 9000 not sent"); + + captured.clear(); + await runs(record.id, { runsDir }); + expect(Bun.stripANSI(captured.err)).toContain("1 created, 9000 not sent"); + }); + + // An unfinished run is counted from its lines, `pending` included. + test("counts identifiers left to attach on a run that never finished", async () => { + const record = partialImport(); + const run = continueRun(runsDir, record); + run.append({ + sourceId: "e", + status: "created", + clerkId: "user_e", + pending: [{ kind: "email", value: "e2@x.dev", verified: true }], + }); + run.release(); + + await runs(record.id, { runsDir }); + + expect(Bun.stripANSI(captured.err)).toContain("interrupted"); + expect(Bun.stripANSI(captured.err)).toContain("2 created, 1 left to attach"); + }); + + // The record says where the key came from, and which instance to reach + // again. One row per key source, so none is left to drop its target. + test.each([ + [{ keySource: "--secret-key" }, "--secret-key "], + [{ keySource: "--app", appId: "app_1" }, "--app app_1 --instance ins_1"], + // Left out, a profile resolves development, and only from its own directory. + [{ keySource: "linked profile", appId: "app_1" }, "--app app_1 --instance ins_1"], + // Maybe no longer exported, and `--instance` is refused next to a `.env`. + [{ keySource: "CLERK_SECRET_KEY env var" }, "--secret-key "], + // A framework's own variable, exported the same way: without it, `.env`'s + // key would be read instead. + [{ keySource: "NUXT_CLERK_SECRET_KEY env var" }, "--secret-key "], + // Both refuse `--instance`, and the file names the key. + [{ keySource: ".env" }, "--runs-dir"], + [{ keySource: "accountless app (.clerk/.tmp/keyless.json)" }, "--runs-dir"], + // No `--instance` resolves a stand-in, and a bare `--app` resolves development: the key. + [ + { keySource: "--app", appId: "app_1", instanceId: "key_abc" }, + "--secret-key --runs-dir", + ], + [ + { keySource: "linked profile", appId: "app_1", instanceId: "key_abc" }, + "--secret-key --runs-dir", + ], + ])("the next command keeps the import's target from %o", async (target, expected) => { + const record = partialImport(); + const file = path.join(runsDir, record.id, "run.json"); + fs.writeFileSync(file, JSON.stringify({ ...record, target: { ...record.target, ...target } })); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next.at(-1)).toContain(`clerk migrate undo ${record.id} ${expected}`); + }); + + // The suggested undo has to look in the same runs folder. + test("the next command keeps the runs folder", async () => { + const record = partialImport(); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next.at(-1)).toContain( + `clerk migrate undo ${record.id} --runs-dir ${quoteArg(runsDir)}`, + ); + }); + + // A partial run is continued by importing the same file again. + test("a partial import suggests re-running it, then undoing it", async () => { + const record = partialImport(); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next).toHaveLength(2); + expect(next[0]).toContain( + `clerk migrate import ${quoteArg(usersFile())} --source clerk --runs-dir ${quoteArg(runsDir)}`, + ); + expect(next[0]).toContain("to retry the users it didn't finish"); + // Run as printed, it goes ahead rather than asking for consent. + expect(next[0]).toContain(`--runs-dir ${quoteArg(runsDir)} --yes\``); + }); + + // An earlier build of this branch wrote none, or a string[]: a continue + // without them would create the users --require-password held back. + test.each([ + ["no flags", undefined], + ["flags as a string[]", ["--require-password"]], + ])("offers no continue for a run with %s", async (_, flags) => { + const record = partialImport({ flags: flags as StartRunInit["flags"] }); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next).toEqual([expect.stringContaining(`clerk migrate undo ${record.id}`)]); + }); + + // Left out, the continue creates the users --require-password held back. + test("the continue repeats the flags the import ran with", async () => { + const record = partialImport({ flags: { requirePassword: true } }); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next[0]).toContain("--source clerk --require-password --runs-dir"); + }); + + // The continue carries --require-password, so it would leave them out again. + test("offers no continue when only --require-password skips are left", async () => { + fs.writeFileSync(usersFile(), "[]"); + const run = startRun(runsDir, { + kind: "import", + target: TARGET, + source: "clerk", + file: { path: usersFile(), sha256: sha256File(usersFile()) }, + flags: { requirePassword: true }, + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ + sourceId: "d", + status: "skipped", + reason: "no password (--require-password)", + code: "no_password", + }); + const record = run.finish(); + + await runs(record.id, { runsDir, json: true }); + + const { run: shown, next } = JSON.parse(captured.out) as { run: RunRecord; next: string[] }; + expect(shown.status).toBe("partial"); + expect(next).toEqual([expect.stringContaining(`clerk migrate undo ${record.id}`)]); + }); + + // The continue reads the same file, so the checks skip them again. + test("offers no continue when only skips for the file's own data are left", async () => { + fs.writeFileSync(usersFile(), "[]"); + const run = startRun(runsDir, { + kind: "import", + target: TARGET, + source: "clerk", + file: { path: usersFile(), sha256: sha256File(usersFile()) }, + flags: { allowPartial: true }, + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ + sourceId: "b", + status: "skipped", + reason: "invalid: Invalid email", + code: "in_file", + }); + const record = run.finish(); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next).toEqual([expect.stringContaining(`clerk migrate undo ${record.id}`)]); + }); + + // Each of these would start another run, or refuse, rather than continue it. + describe("offers only the undo when importing the file again would not continue the run", () => { + const onlyUndo = async (id: string) => { + await runs(id, { runsDir, json: true }); + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next).toEqual([expect.stringContaining(`clerk migrate undo ${id}`)]); + }; + + // In the stand-in rows, `runs` has no key, so it can't tell whether a + // stand-in is `ins_1`: an import with that key could take the newer run. + test.each([ + ["a newer run of the same file", "ins_1", "ins_1"], + ["a newer run under a key's stand-in, when this one has a real ID", "ins_1", "key_abc"], + ["a newer run under a real ID, when this one has a key's stand-in", "key_abc", "ins_1"], + ])("%s", async (_name, instanceId, newerInstanceId) => { + const record = partialImport({ target: { ...TARGET, instanceId } }); + newerImport(record, newerInstanceId); + + await onlyUndo(record.id); + }); + + test("a file changed since", async () => { + const record = partialImport(); + fs.writeFileSync(usersFile(), "[{}]"); + + await onlyUndo(record.id); + }); + + test("an undo of it left unfinished", async () => { + const record = partialImport(); + startRun(runsDir, { kind: "undo", target: record.target, undoes: record.id }).release(); + + await onlyUndo(record.id); + }); + + // Cut off before it marked the import undone: an import starts a new run. + test("an undo of it that finished", async () => { + const record = partialImport(); + startRun(runsDir, { kind: "undo", target: record.target, undoes: record.id }).finish(); + + await onlyUndo(record.id); + }); + }); + + // Dev first, then prod, from the same file: the dev run is still the one + // an import into dev continues. + test("a newer run on another instance keeps the continue", async () => { + const record = partialImport(); + newerImport(record, "ins_2"); + + await runs(record.id, { runsDir, json: true }); + + const { next } = JSON.parse(captured.out) as { next: string[] }; + expect(next).toHaveLength(2); + expect(next[0]).toContain("to retry the users it didn't finish"); + }); + + test("an unknown ID is a usage error", async () => { + await expect(runs("nope", { runsDir })).rejects.toThrow(/No run `nope`/); + }); + + test("an unknown ID lists the same runs folder", async () => { + await expect(runs("nope", { runsDir })).rejects.toThrow( + `clerk migrate runs --runs-dir ${quoteArg(runsDir)}`, + ); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/runs.ts b/packages/cli-core/src/commands/migrate/runs.ts new file mode 100644 index 0000000000..70660eaa14 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/runs.ts @@ -0,0 +1,316 @@ +/** + * `clerk migrate runs [run-id]` — what every import, export and undo did. + * + * Reads the run store and nothing else. `runs` alone lists every run, newest + * first. `runs ` shows one: its header, counts, the error breakdown, the + * users that did not make it, and the command to run next. + */ + +import path from "node:path"; +import { bold, dim, green, red, yellow } from "../../lib/color.ts"; +import { throwUsageError } from "../../lib/errors.ts"; +import { quoteArg } from "../../lib/json-body.ts"; +import { log } from "../../lib/log.ts"; +import { tallyError } from "./lib/failures.ts"; +import { + hasRetryableUsers, + latestUserLines, + listRuns, + importsOfFile, + undosOf, + readRun, + resolveRunsDir, + runDir, + runState, + sha256File, + type RunCounts, + type RunRecord, + type RunState, + type UserLine, + USERS_FILE, + noRunMessage, + withLiveCounts, +} from "./lib/run-store.ts"; +import { carryFlags, importFlags, undoCommand, withFlags } from "./lib/carry-flags.ts"; +import { plural } from "./lib/plural.ts"; +import { describeTarget, isStandInInstanceId, laterCarry } from "./lib/target.ts"; + +export type RunsOptions = { + json?: boolean; + runsDir?: string; +}; + +/** How many users of each list `runs ` prints before pointing at the file. */ +const USER_LIST_LIMIT = 20; + +function formatDate(iso: string): string { + const date = new Date(iso); + if (Number.isNaN(date.getTime())) return iso; + return date.toLocaleString(undefined, { dateStyle: "medium", timeStyle: "short" }); +} + +function colorState(state: RunState): string { + if (state === "complete") return green(state); + if (state === "partial" || state === "interrupted") return yellow(state); + if (state === "undone") return dim(state); + return state; +} + +// `creating`: a create no answer confirmed. Clerk may hold the user. +const COUNT_LABELS: Partial> = { + creating: "not confirmed", + pending: "left to attach", + notSent: "not sent", +}; + +/** The counts that matter for a run's kind, in the order they are read. */ +function formatCounts(record: RunRecord): string { + const counts = record.counts; + const order = + record.kind === "export" + ? (["exported", "skipped"] as const) + : record.kind === "undo" + ? (["deleted", "failed"] as const) + : (["created", "pending", "creating", "failed", "notSent", "skipped"] as const); + const parts = order + .filter((status) => (counts[status] ?? 0) > 0 || status === order[0]) + .map((status) => `${counts[status] ?? 0} ${COUNT_LABELS[status] ?? status}`); + return parts.join(", "); +} + +function fileLabel(record: RunRecord): string { + if (!record.file) return ""; + const relative = path.relative(process.cwd(), record.file.path); + return relative.startsWith("..") ? record.file.path : relative; +} + +function pad(rows: string[][]): string[] { + const widths = rows[0]!.map((_, column) => + Math.max(...rows.map((row) => Bun.stringWidth(row[column] ?? ""))), + ); + return rows.map((row) => + row + .map((cell, column) => + column === row.length - 1 + ? cell + : cell + " ".repeat(widths[column]! - Bun.stringWidth(cell)), + ) + .join(" ") + .trimEnd(), + ); +} + +/** Error messages grouped the way the import summary groups them. */ +function errorBreakdown(lines: UserLine[]): { error: string; count: number }[] { + const counts = new Map(); + for (const line of lines) { + if ((line.status === "failed" || line.status === "creating") && line.error) { + tallyError(counts, line.error); + } + } + return [...counts].map(([error, count]) => ({ error, count })).sort((a, b) => b.count - a.count); +} + +/** + * The import that continues this run: its file (or export run) and source, + * with the flags that reach the same runs folder and instance, ending in + * `--yes`. None when the run has no file, or flags this build can't read. + */ +function continueCommand(record: RunRecord, carry: string[]): string | undefined { + // Every import records its flags. None, or the string[] an earlier build + // wrote, can't be read back, and a continue without them would create the + // users --require-password held back. + if (!record.flags || Array.isArray(record.flags)) return undefined; + const input = record.fromExport ?? record.file?.path; + if (!input) return undefined; + const parts = ["clerk migrate import", quoteArg(input)]; + if (!record.fromExport && record.source) parts.push("--source", quoteArg(record.source)); + return withFlags(parts.join(" "), [...importFlags(record.flags), ...carry, "--yes"]); +} + +/** + * Whether importing the record's file again continues this run, rather than + * starting another or refusing: no newer import of that file and source may + * be for the same instance, the file is unchanged, and no undo of it exists. + * An unfinished undo makes an import refuse; a finished one that never marked + * the run makes it start a new one. + */ +function continuesThisRun(runsDir: string, record: RunRecord): boolean { + const file = record.file; + if (!file) return false; + const runs = listRuns(runsDir); + // An import also takes a run recorded under its key's stand-in ID. Without + // the key, a stand-in on either side may be the same instance: offering the + // continue then could send the user to a newer run instead. + const mayBeSameInstance = (id: string | undefined) => + id === record.target.instanceId || + isStandInInstanceId(id) || + isStandInInstanceId(record.target.instanceId); + const newest = importsOfFile(runs, { sha256: file.sha256, source: record.source }).find((other) => + mayBeSameInstance(other.target.instanceId), + ); + if (newest?.id !== record.id) return false; + if (undosOf(runs, record.id).length > 0) return false; + try { + return sha256File(file.path) === file.sha256; + } catch (error) { + log.debug(`migrate: could not read ${file.path} to check it is unchanged: ${String(error)}`); + return false; + } +} + +/** + * What to run after reading about this run. `carry` is the runs folder this + * listing read, and the target flags the record says the import used. + */ +function nextCommands( + runsDir: string, + record: RunRecord, + state: RunState, + carry: string[], + latest: UserLine[], +): string[] { + if (state === "running") return []; + if (record.kind === "import" && state !== "undone") { + const undo = `Run \`${undoCommand(record.id, carry)}\` to delete the users it created`; + const again = continueCommand(record, carry); + if (!again || state === "complete" || !continuesThisRun(runsDir, record)) return [undo]; + if (state === "partial" && !hasRetryableUsers(record, latest)) return [undo]; + // Continuing is what a stopped run is for: the same file, source and + // target pick it up where it left off. + const why = state === "interrupted" ? "continue it" : "retry the users it didn't finish"; + return [`Run \`${again}\` to ${why}`, undo]; + } + if (record.kind === "undo" && state !== "complete" && record.undoes) { + return [`Run \`${undoCommand(record.undoes, carry)}\` to retry the users that failed`]; + } + return []; +} + +function listJson(runsDir: string, records: RunRecord[]) { + return { + runsDir, + runs: records.map((record) => ({ ...record, state: runState(runsDir, record) })), + }; +} + +function printList(runsDir: string, records: RunRecord[]): void { + log.info(`Runs folder: ${runsDir}`); + log.blank(); + + if (records.length === 0) { + log.info("No migration runs yet."); + return; + } + + const rows = [ + ["RUN", "DATE", "KIND", "STATUS", "TARGET", "FILE", "RESULT"].map((header) => bold(header)), + ...records.map((record) => [ + record.id, + formatDate(record.startedAt), + record.kind, + colorState(runState(runsDir, record)), + describeTarget(record.target), + fileLabel(record), + formatCounts(record), + ]), + ]; + for (const line of pad(rows)) log.info(line); + log.blank(); + log.info(dim(plural(records.length, "run"))); +} + +/** One run in full: `--json` prints it, and `printRun` renders it. */ +function showJson(runsDir: string, record: RunRecord, carry: string[]) { + const lines = [...latestUserLines(runsDir, record.id).values()]; + const state = runState(runsDir, record); + return { + runsDir, + run: { ...record, state }, + next: nextCommands(runsDir, record, state, carry, lines), + errors: errorBreakdown(lines), + failed: lines.filter((line) => line.status === "failed"), + skipped: lines.filter((line) => line.status === "skipped"), + unconfirmed: lines.filter((line) => line.status === "creating"), + }; +} + +function printUsers(title: string, lines: UserLine[], usersFile: string): void { + if (lines.length === 0) return; + log.blank(); + log.info(bold(`${title} (${lines.length})`)); + for (const line of lines.slice(0, USER_LIST_LIMIT)) { + const why = line.reason ?? line.error ?? ""; + log.info(` ${line.sourceId}${why ? dim(` ${why}`) : ""}`); + } + if (lines.length > USER_LIST_LIMIT) { + log.info(dim(` …and ${lines.length - USER_LIST_LIMIT} more in ${usersFile}`)); + } +} + +function printRun(runsDir: string, record: RunRecord, carry: string[]): void { + const view = showJson(runsDir, record, carry); + const usersFile = path.join(runDir(runsDir, record.id), USERS_FILE); + + log.info(`Runs folder: ${runsDir}`); + log.blank(); + const header: [string, string | undefined][] = [ + ["Run", record.id], + ["Kind", record.kind], + ["Status", colorState(view.run.state)], + ["Started", formatDate(record.startedAt)], + ["Finished", record.finishedAt ? formatDate(record.finishedAt) : undefined], + ["Target", describeTarget(record.target)], + ["Key from", record.target.keySource], + ["Source", record.source], + ["File", record.file?.path], + ["From export", record.fromExport], + ["Undoes", record.undoes], + ["Undone by", record.undoneBy], + ["Result", formatCounts(record)], + ]; + const shown = header.filter((entry): entry is [string, string] => Boolean(entry[1])); + const width = Math.max(...shown.map(([label]) => label.length)) + 2; + for (const [label, value] of shown) log.info(`${bold(label.padEnd(width))}${value}`); + + if (view.errors.length > 0) { + log.blank(); + log.info(bold("Error breakdown")); + for (const { error, count } of view.errors) { + log.info(` ${red(String(count))} ${count === 1 ? "user" : "users"}: ${error}`); + } + } + + printUsers("Failed", view.failed, usersFile); + printUsers( + "Not confirmed (Clerk may have created them; a continue checks)", + view.unconfirmed, + usersFile, + ); + printUsers("Skipped", view.skipped, usersFile); + + log.blank(); + log.info(dim(`Every outcome: ${usersFile}`)); + for (const step of view.next) log.info(` → ${step}`); +} + +export async function runs(id: string | undefined, options: RunsOptions = {}): Promise { + const runsDir = await resolveRunsDir(options.runsDir); + + if (id === undefined) { + const records = listRuns(runsDir).map((record) => withLiveCounts(runsDir, record)); + if (options.json) log.data(JSON.stringify(listJson(runsDir, records), null, 2)); + else printList(runsDir, records); + return; + } + + const stored = readRun(runsDir, id); + if (!stored) throwUsageError(noRunMessage(runsDir, id, carryFlags({ runsDir: options.runsDir }))); + const record = withLiveCounts(runsDir, stored); + + // The record says where the import's key came from, and so which flags + // reach that instance again. + const carry = laterCarry(options.runsDir, record.target); + if (options.json) log.data(JSON.stringify(showJson(runsDir, record, carry), null, 2)); + else printRun(runsDir, record, carry); +} diff --git a/packages/cli-core/src/commands/migrate/undo-interactive.test.ts b/packages/cli-core/src/commands/migrate/undo-interactive.test.ts new file mode 100644 index 0000000000..d2eafce392 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/undo-interactive.test.ts @@ -0,0 +1,111 @@ +/** + * `clerk migrate undo` with a human at the prompt. + * + * Its own file because `mock.module` replaces `prompts.ts` for the whole + * process, and `undo.test.ts` runs non-interactively. + */ +import { afterAll, afterEach, beforeAll, beforeEach, expect, mock, spyOn, test } from "bun:test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { libPromptsStubs } from "../../test/lib/stubs.ts"; + +// The whole module, so a file sharing this process still finds `text` and the rest. +let confirmed: () => void = () => {}; +mock.module("../../lib/prompts.ts", () => ({ + ...libPromptsStubs, + confirm: async () => { + confirmed(); + return true; + }, +})); + +const { startRun } = await import("./lib/run-store.ts"); +const { terminal } = await import("./lib/terminal.ts"); +const { undo } = await import("./undo.ts"); + +const IMPORT_STARTED = "2026-09-01T00:00:00.000Z"; +const AFTER_IMPORT = Date.parse("2026-09-02T00:00:00.000Z"); + +let runsDir: string; +let originalFetch: typeof globalThis.fetch; +let originalMode: string | undefined; +const terminalAvailable = spyOn(terminal, "available"); +/** Clerk IDs the stubbed instance holds, with each one's last sign-in. */ +let instanceUsers: Map; +let deletes: string[]; + +beforeAll(() => { + originalFetch = globalThis.fetch; + originalMode = process.env.CLERK_MODE; +}); + +afterAll(() => { + terminalAvailable.mockRestore(); + if (originalMode === undefined) delete process.env.CLERK_MODE; + else process.env.CLERK_MODE = originalMode; +}); + +beforeEach(() => { + process.env.CLERK_MODE = "human"; + terminalAvailable.mockReturnValue(true); + runsDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-undo-human-"))); + deletes = []; + confirmed = () => {}; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(input.toString()); + const method = init?.method ?? "GET"; + if (url.pathname === "/v1/instance") { + return Response.json({ object: "instance", id: "ins_1", environment_type: "development" }); + } + if (method === "GET" && url.pathname === "/v1/users") { + return Response.json( + url.searchParams + .getAll("user_id") + .filter((id) => instanceUsers.has(id)) + .map((id) => ({ id, last_sign_in_at: instanceUsers.get(id) })), + ); + } + if (method === "DELETE") { + const id = url.pathname.split("/").pop() as string; + deletes.push(id); + instanceUsers.delete(id); + return Response.json({ id, deleted: true }); + } + return new Response("unexpected", { status: 500 }); + }) as typeof fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + process.exitCode = 0; + fs.rmSync(runsDir, { recursive: true, force: true }); +}); + +// The prompt warned about user_a. While it waited, user_a was deleted +// elsewhere and user_b signed in: one signed in either way, so a count would +// match, and user_b would be deleted with nobody agreeing to it. +test("refuses a user who signed in while the prompt waited, though the count is the same", async () => { + instanceUsers = new Map([ + ["user_a", AFTER_IMPORT], + ["user_b", null], + ]); + const run = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + run.update({ startedAt: IMPORT_STARTED }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "b", status: "created", clerkId: "user_b" }); + const record = run.finish(); + confirmed = () => { + instanceUsers.delete("user_a"); + instanceUsers.set("user_b", AFTER_IMPORT); + }; + + await expect(undo(record.id, { secretKey: "sk_test_x", runsDir })).rejects.toThrow( + /1 more user the import created signed in after the preview/, + ); + expect(deletes).toEqual([]); +}); diff --git a/packages/cli-core/src/commands/migrate/undo.test.ts b/packages/cli-core/src/commands/migrate/undo.test.ts new file mode 100644 index 0000000000..8a40ac8581 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/undo.test.ts @@ -0,0 +1,797 @@ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + spyOn, + test, +} from "bun:test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { EXIT_CODE, type CliError } from "../../lib/errors.ts"; +import { quoteArg } from "../../lib/json-body.ts"; +import { _resetInterruptState, abortInFlight, beginInterrupt } from "../../lib/signals.ts"; +import { useCaptureLog } from "../../test/lib/stubs.ts"; +import { + continueRun, + latestUserLines, + listRuns, + lockFile, + patchRun, + readRun, + startRun, + type RunRecord, +} from "./lib/run-store.ts"; +import { keyInstanceId } from "./lib/target.ts"; +import { terminal } from "./lib/terminal.ts"; +import { undo } from "./undo.ts"; + +const captured = useCaptureLog(); + +let runsDir: string; +let originalFetch: typeof globalThis.fetch; +let requests: { method: string; url: string }[]; +/** Clerk IDs the stubbed instance still holds, with each one's last sign-in. */ +let instanceUsers: Map; +/** Clerk IDs whose DELETE fails with a 500. */ +let failing: Set; +/** external_id → Clerk ID, for users whose create was in flight. */ +let inFlight: Map; +/** external_id → the run whose marker Clerk holds on that in-flight user. */ +let inFlightMarker: Map; + +const IMPORT_STARTED = "2026-09-01T00:00:00.000Z"; +const AFTER_IMPORT = Date.parse("2026-09-02T00:00:00.000Z"); + +beforeAll(() => { + originalFetch = globalThis.fetch; +}); + +afterAll(() => { + globalThis.fetch = originalFetch; +}); + +beforeEach(() => { + runsDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "clerk-undo-"))); + requests = []; + failing = new Set(); + inFlight = new Map(); + inFlightMarker = new Map(); + instanceUsers = new Map([ + ["user_a", null], + ["user_b", AFTER_IMPORT], + ]); + + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(input.toString()); + const method = init?.method ?? "GET"; + requests.push({ method, url: url.toString() }); + + if (url.pathname === "/v1/instance") { + return Response.json({ object: "instance", id: "ins_1", environment_type: "development" }); + } + if (method === "GET" && url.pathname === "/v1/users" && url.searchParams.has("external_id")) { + return Response.json( + url.searchParams + .getAll("external_id") + .map((externalId) => externalId.replace(/^\+/, "")) + .filter((externalId) => inFlight.has(externalId)) + .map((externalId) => ({ + id: inFlight.get(externalId), + external_id: externalId, + private_metadata: { clerkMigrateRun: inFlightMarker.get(externalId) }, + })), + ); + } + if (method === "GET" && url.pathname === "/v1/users") { + const ids = url.searchParams.getAll("user_id"); + return Response.json( + ids + .filter((id) => instanceUsers.has(id)) + .map((id) => ({ id, last_sign_in_at: instanceUsers.get(id) })), + ); + } + if (method === "DELETE") { + const id = url.pathname.split("/").pop() as string; + if (failing.has(id)) { + return Response.json({ errors: [{ code: "boom", message: "boom" }] }, { status: 500 }); + } + instanceUsers.delete(id); + return Response.json({ id, deleted: true }); + } + return new Response("unexpected", { status: 500 }); + }) as typeof fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + process.exitCode = 0; + fs.rmSync(runsDir, { recursive: true, force: true }); +}); + +/** An import into `ins_1` that created two users and failed one. */ +function importRun(overrides: Partial = {}): RunRecord { + const run = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + run.update({ startedAt: IMPORT_STARTED }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "b", status: "created", clerkId: "user_b" }); + run.append({ sourceId: "c", status: "failed", error: "taken" }); + const record = run.finish(); + if (Object.keys(overrides).length > 0) run.update(overrides); + return { ...record, ...overrides }; +} + +// `user_b` has signed in since the import; the guard on that has its own tests. +const withDir = (extra: Record = {}) => ({ + secretKey: "sk_test_x", + includeSignedIn: true, + runsDir, + ...extra, +}); +const deletes = () => requests.filter((request) => request.method === "DELETE"); + +describe("refusals, all exit 2 and delete nothing", () => { + const exitCodeOf = async (promise: Promise) => { + const error = (await promise.catch((caught: unknown) => caught)) as CliError; + return error.exitCode; + }; + + test("an unknown run", async () => { + expect(await exitCodeOf(undo("nope", withDir({ yes: true })))).toBe(EXIT_CODE.USAGE); + }); + + test("an unknown run lists the same runs folder", async () => { + await expect(undo("nope", withDir({ yes: true }))).rejects.toThrow( + `clerk migrate runs --runs-dir ${quoteArg(runsDir)}`, + ); + }); + + test("a run that is not an import", async () => { + const run = startRun(runsDir, { kind: "export", target: { platform: "auth0" } }); + run.finish(); + await expect(undo(run.record.id, withDir({ yes: true }))).rejects.toThrow( + /is an export run\. Only an import run can be undone/, + ); + expect(deletes()).toHaveLength(0); + }); + + test("a run already undone", async () => { + const record = importRun({ status: "undone", undoneBy: "20260901-000000-beef" }); + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow( + /already undone by run 20260901-000000-beef/, + ); + }); + + test("a key that addresses a different instance, naming both", async () => { + const record = importRun({ target: { instanceId: "ins_other", env: "production" } }); + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow( + /imported into instance \(production, ins_other\).*addresses instance \(development, ins_1\)/s, + ); + expect(deletes()).toHaveLength(0); + }); + + // `key_` is the identity fallback when GET /v1/instance failed, e.g. rate + // limited straight after a large import. That is unknown, not different. + test("an unconfirmed instance is not reported as a different one", async () => { + const record = importRun({ + target: { instanceId: "key_0123456789abcdef", env: "development" }, + }); + const error = (await undo(record.id, withDir({ yes: true })).catch((e: unknown) => e)) as Error; + expect(error.message).toContain("Could not confirm"); + expect(error.message).not.toContain("addresses instance"); + expect(deletes()).toHaveLength(0); + }); + + // The import ran while Clerk could not name the instance; the same key is + // the same instance once it can. + test("accepts a run recorded under this key's stand-in ID", async () => { + const record = importRun({ + target: { instanceId: keyInstanceId("sk_test_x"), env: "development" }, + }); + + await undo(record.id, withDir({ yes: true })); + + expect(deletes()).toHaveLength(2); + }); + + test("no consent where nobody can be asked: the preview, then the command", async () => { + const record = importRun(); + + const error = (await undo(record.id, withDir()).catch((caught: unknown) => caught)) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain("Pass --yes to confirm"); + expect(captured.err).toContain("Will delete 2 users"); + expect(deletes()).toHaveLength(0); + }); + + // Nothing is left to delete, but the import is still marked undone. + test("no consent with every user already gone names the mark, not 0 deletes", async () => { + instanceUsers.clear(); + const record = importRun(); + + const error = (await undo(record.id, withDir()).catch((caught: unknown) => caught)) as CliError; + + expect(error.message).toContain(`marks run ${record.id} undone and needs consent`); + expect(error.message).not.toContain("0 users"); + expect(error.examples?.[0]?.description).toBe(`Mark run ${record.id} undone`); + }); + + // The preview and the dry run say the same, rather than "Will delete 0 users". + test("a dry run with every user already gone names the mark, not 0 deletes", async () => { + instanceUsers.clear(); + const record = importRun(); + + await undo(record.id, withDir({ dryRun: true })); + + const err = Bun.stripANSI(captured.err); + expect(err).toContain(`Will mark import run ${record.id} undone: nothing is left to delete.`); + expect(err).toContain("to mark it undone."); + expect(err).not.toContain("Will delete 0"); + }); + + // Another process continues the import while the undo previews it. + describe("the run changing during the preview", () => { + const duringPreview = (act: () => void) => { + const stubbed = globalThis.fetch; + let acted = false; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + if (!acted && new URL(input.toString()).searchParams.has("user_id")) { + acted = true; + act(); + } + return stubbed(input, init); + }) as typeof fetch; + }; + + // The run's own folder takes the lock, but the undo run can't be created + // beside it. `--runs-dir` is never written to by resolveRunsDir. + // Root writes through 0o555, so the undo run would be created. + test.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "--json prints the preview, marked writeFailed, when the runs folder can't be written", + async () => { + const record = importRun(); + fs.chmodSync(runsDir, 0o555); + try { + await expect(undo(record.id, withDir({ yes: true, json: true }))).rejects.toThrow(); + } finally { + fs.chmodSync(runsDir, 0o755); + } + expect(JSON.parse(captured.out)).toMatchObject({ + import: { id: record.id }, + run: null, + writeFailed: true, + }); + expect(deletes()).toHaveLength(0); + }, + ); + + // After the preview, --json always prints its object. + test("--json prints the preview, marked changed, when another process holds the run", async () => { + const record = importRun(); + duringPreview(() => fs.writeFileSync(lockFile(runsDir, record.id), String(process.ppid))); + + await expect(undo(record.id, withDir({ yes: true, json: true }))).rejects.toThrow( + /in use by another process/, + ); + expect(JSON.parse(captured.out)).toMatchObject({ import: { id: record.id }, changed: true }); + expect(deletes()).toHaveLength(0); + }); + + test("still running: refused by the import run's lock", async () => { + const record = importRun(); + duringPreview(() => fs.writeFileSync(lockFile(runsDir, record.id), String(process.ppid))); + + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow( + /in use by another process/, + ); + expect(deletes()).toHaveLength(0); + expect(readRun(runsDir, record.id)?.status).not.toBe("undone"); + }); + + test("finished: refused because the preview is out of date", async () => { + const record = importRun(); + duringPreview(() => { + const run = continueRun(runsDir, record); + run.append({ sourceId: "c", status: "created", clerkId: "user_c" }); + run.finish(); + }); + + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow( + /preview is out of date/, + ); + expect(deletes()).toHaveLength(0); + expect(fs.existsSync(lockFile(runsDir, record.id))).toBe(false); + }); + + // After the preview, --json always prints its object. + test("--json prints the preview, marked changed, when it is out of date", async () => { + const record = importRun(); + duringPreview(() => { + const run = continueRun(runsDir, record); + run.append({ sourceId: "c", status: "created", clerkId: "user_c" }); + run.finish(); + }); + + await expect(undo(record.id, withDir({ yes: true, json: true }))).rejects.toThrow( + /preview is out of date/, + ); + expect(JSON.parse(captured.out)).toMatchObject({ import: { id: record.id }, changed: true }); + }); + + // --yes agreed to the preview's count, not to someone who signed in since. + test("refuses when a user signs in after the preview, without --include-signed-in", async () => { + instanceUsers.set("user_b", null); + const record = importRun(); + let lookups = 0; + const stubbed = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(input.toString()); + if (url.searchParams.has("user_id") && lookups++ === 1) + instanceUsers.set("user_a", AFTER_IMPORT); + return stubbed(input, init); + }) as typeof fetch; + + await expect(undo(record.id, withDir({ yes: true, includeSignedIn: false }))).rejects.toThrow( + /1 more user the import created signed in after the preview/, + ); + expect(deletes()).toHaveLength(0); + expect(readRun(runsDir, record.id)?.status).not.toBe("undone"); + }); + + // Past the preview, --json still prints its object, whatever happens next. + test("--json prints the preview, marked, when the recheck itself fails", async () => { + instanceUsers.set("user_b", null); + const record = importRun(); + let lookups = 0; + const stubbed = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(input.toString()); + if (url.searchParams.has("user_id") && lookups++ === 1) { + return Response.json({ errors: [{ code: "boom", message: "boom" }] }, { status: 500 }); + } + return stubbed(input, init); + }) as typeof fetch; + + await expect( + undo(record.id, withDir({ yes: true, includeSignedIn: false, json: true })), + ).rejects.toThrow(); + expect(JSON.parse(captured.out)).toMatchObject({ + import: { id: record.id }, + recheckFailed: true, + }); + expect(deletes()).toHaveLength(0); + }); + + test("--include-signed-in deletes a user who signed in after the preview", async () => { + instanceUsers.set("user_b", null); + const record = importRun(); + let lookups = 0; + const stubbed = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const url = new URL(input.toString()); + if (url.searchParams.has("user_id") && lookups++ === 0) + instanceUsers.set("user_a", AFTER_IMPORT); + return stubbed(input, init); + }) as typeof fetch; + + await undo(record.id, withDir({ yes: true, includeSignedIn: true })); + + expect(deletes()).toHaveLength(2); + }); + + test("undone by another undo: refused, and its undo is kept", async () => { + const record = importRun(); + duringPreview(() => + patchRun(runsDir, record.id, { status: "undone", undoneBy: "20260901-000000-beef" }), + ); + + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow( + /already undone by run 20260901-000000-beef/, + ); + expect(deletes()).toHaveLength(0); + expect(readRun(runsDir, record.id)?.undoneBy).toBe("20260901-000000-beef"); + expect(listRuns(runsDir).filter((run) => run.kind === "undo")).toHaveLength(0); + }); + }); +}); + +describe("--dry-run", () => { + test("previews the deletes and how many users signed in since, and deletes nothing", async () => { + const record = importRun(); + + await undo(record.id, withDir({ dryRun: true })); + + expect(captured.err).toContain("Target: development instance ins_1"); + expect(captured.err).toContain("Will delete 2 users"); + expect(captured.err).toContain("1 of them has signed in since the import"); + expect(deletes()).toHaveLength(0); + expect(listRuns(runsDir)).toHaveLength(1); + }); + + // `run` is the undo's own run, as an import's is: null until one is written. + test("--json returns the target, the import, no run yet, and the preview", async () => { + const record = importRun(); + + await undo(record.id, withDir({ dryRun: true, json: true })); + + expect(JSON.parse(captured.out)).toMatchObject({ + target: { instanceId: "ins_1", keySource: "--secret-key" }, + import: { id: record.id }, + run: null, + preview: { toDelete: 2, signedInSince: 1, alreadyGone: 0, alreadyDeleted: 0 }, + dryRun: true, + }); + }); +}); + +describe("another undo during the preview", () => { + test("refuses when another undo started an undo run on the import", async () => { + const record = importRun(); + const stubbed = globalThis.fetch; + let raced = false; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + // The other undo gets its lock between this one's preview and its own. + if (!raced && (init?.method ?? "GET") === "GET" && input.toString().includes("/v1/users")) { + raced = true; + const other = startRun(runsDir, { + kind: "undo", + target: { instanceId: "ins_1", env: "development" }, + undoes: record.id, + source: "clerk", + }); + other.append({ sourceId: "a", clerkId: "user_a", status: "deleted" }); + other.append({ sourceId: "b", clerkId: "user_b", status: "failed", error: "boom" }); + other.finish(); + } + return stubbed(input, init); + }) as typeof fetch; + + const error = (await undo(record.id, withDir({ yes: true })).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.message).toContain("preview is out of date"); + expect(deletes()).toHaveLength(0); + expect(listRuns(runsDir).filter((run) => run.kind === "undo")).toHaveLength(1); + }); +}); + +describe("stopped part-way", () => { + // The deletes still queued are rejected; the undo must not read as done. + test("a Ctrl-C during the deletes leaves the undo unfinished, as interrupted", async () => { + const record = importRun(); + const clerk = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const response = await clerk(input, init); + if (init?.method === "DELETE") { + beginInterrupt(); + abortInFlight(); + } + return response; + }) as typeof fetch; + try { + await expect(undo(record.id, withDir({ yes: true }))).rejects.toThrow(); + } finally { + _resetInterruptState(); + globalThis.fetch = clerk; + } + + const undoRun = listRuns(runsDir).find((r) => r.kind === "undo"); + expect(undoRun?.finishedAt).toBeUndefined(); + expect(fs.existsSync(path.join(runsDir, undoRun?.id ?? "", "lock"))).toBe(false); + expect(readRun(runsDir, record.id)?.status).not.toBe("undone"); + }); + + // A script reads `run` to find the undo's folder. + test("--json names the undo run a Ctrl-C left on disk", async () => { + const record = importRun(); + const clerk = globalThis.fetch; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const response = await clerk(input, init); + if (init?.method === "DELETE") { + beginInterrupt(); + abortInFlight(); + } + return response; + }) as typeof fetch; + try { + await expect(undo(record.id, withDir({ yes: true, json: true }))).rejects.toThrow(); + } finally { + _resetInterruptState(); + globalThis.fetch = clerk; + } + + const undoRun = listRuns(runsDir).find((r) => r.kind === "undo"); + // As it stands on disk, not as it opened: counted from its lines. + const printed = JSON.parse(captured.out) as { run: RunRecord }; + expect(printed).toMatchObject({ + stopped: true, + run: { id: undoRun?.id, state: "interrupted" }, + }); + expect(printed.run.counts.deleted).toBe(latestUserLines(runsDir, undoRun!.id).size); + expect(printed.run.counts.deleted).toBeGreaterThan(0); + }); +}); + +// `CLERK_MODE=human` with no terminal: the prompt would fail to open one and spin. +describe("forced human mode with no terminal", () => { + test("refuses for consent rather than prompting", async () => { + const record = importRun(); + const mode = process.env.CLERK_MODE; + const available = spyOn(terminal, "available").mockReturnValue(false); + process.env.CLERK_MODE = "human"; + try { + await expect(undo(record.id, withDir())).rejects.toThrow(/needs consent/); + } finally { + available.mockRestore(); + if (mode === undefined) delete process.env.CLERK_MODE; + else process.env.CLERK_MODE = mode; + } + expect(deletes()).toHaveLength(0); + }); +}); + +describe("users who signed in since the import", () => { + test("--yes alone refuses to delete them, and deletes nothing", async () => { + const record = importRun(); + + const error = (await undo(record.id, withDir({ yes: true, includeSignedIn: false })).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.exitCode).toBe(EXIT_CODE.USAGE); + expect(error.message).toContain("1 user the import created has signed in since"); + expect(error.examples?.[0]?.command).toContain("--yes --include-signed-in"); + expect(deletes()).toHaveLength(0); + }); + + // `consent` alone would tell a script to retry with --yes, which it gave. + test("--yes --json marks the refusal as needing --include-signed-in", async () => { + const record = importRun(); + + await expect( + undo(record.id, withDir({ yes: true, json: true, includeSignedIn: false })), + ).rejects.toThrow("--include-signed-in"); + + expect(JSON.parse(captured.out)).toMatchObject({ + consent: "required", + includeSignedIn: "required", + }); + }); + + // Without them the suggested command looks in the default runs folder, or + // reaches another instance, and refuses. + test("the suggested command keeps the runs folder and the key", async () => { + const record = importRun(); + + const error = (await undo(record.id, withDir({ yes: true, includeSignedIn: false })).catch( + (caught: unknown) => caught, + )) as CliError; + + expect(error.examples?.[0]?.command).toBe( + `clerk migrate undo ${record.id} --yes --include-signed-in --secret-key --runs-dir ${quoteArg(runsDir)}`, + ); + }); + + test("--include-signed-in deletes them too", async () => { + const record = importRun(); + + await undo(record.id, withDir({ yes: true, includeSignedIn: true })); + + expect(deletes()).toHaveLength(2); + }); +}); + +describe("deleting", () => { + // The run stopped with this user's POST /v1/users in flight: Clerk created + // it, but its ID never reached the run record. + test("finds a user whose create was in flight by external_id, and deletes it", async () => { + const run = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + run.update({ startedAt: IMPORT_STARTED }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "d", status: "creating" }); + run.append({ sourceId: "e", status: "creating" }); + const record = run.finish(); + inFlight.set("d", "user_d"); + inFlightMarker.set("d", record.id); + instanceUsers.set("user_d", null); + + await undo(record.id, withDir({ yes: true })); + + expect(deletes().map((request) => request.url.split("/").pop())).toEqual( + expect.arrayContaining(["user_a", "user_d"]), + ); + expect(deletes()).toHaveLength(2); + }); + + // Same source ID, but no marker from this run: an app or another tool's user. + test("leaves an in-flight match that does not carry the run's marker", async () => { + const run = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + run.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + run.append({ sourceId: "d", status: "creating" }); + const record = run.finish(); + inFlight.set("d", "user_d"); + instanceUsers.set("user_d", null); + + await undo(record.id, withDir({ yes: true })); + + expect(deletes().map((request) => request.url.split("/").pop())).toEqual(["user_a"]); + }); + + // Run A stopped with d's create in flight; a later run B then created d. + // The user Clerk holds is B's, so undoing A leaves it alone. + test("leaves an in-flight user that another import run records as created", async () => { + const runA = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + runA.append({ sourceId: "a", status: "created", clerkId: "user_a" }); + runA.append({ sourceId: "d", status: "creating" }); + const recordA = runA.finish(); + const runB = startRun(runsDir, { + kind: "import", + target: { instanceId: "ins_1", env: "development" }, + source: "clerk", + }); + runB.append({ sourceId: "d", status: "created", clerkId: "user_d" }); + runB.finish(); + inFlight.set("d", "user_d"); + inFlightMarker.set("d", runB.record.id); + instanceUsers.set("user_d", null); + + await undo(recordA.id, withDir({ yes: true })); + + expect(deletes().map((request) => request.url.split("/").pop())).toEqual(["user_a"]); + }); + + // The instance is over its limit for every delete, not just the one that hit it. + test("holds every other delete through a 429's wait", async () => { + const record = importRun(); + const stub = globalThis.fetch; + const started = performance.now(); + const deletedAt: number[] = []; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + if (init?.method !== "DELETE") return stub(input, init); + deletedAt.push(performance.now() - started); + if (deletedAt.length === 1) { + requests.push({ method: "DELETE", url: input.toString() }); + return new Response(JSON.stringify({ errors: [{ code: "x", message: "slow down" }] }), { + status: 429, + headers: { "retry-after": "1" }, + }); + } + return stub(input, init); + }) as typeof fetch; + process.env.CLERK_MIGRATE_CONCURRENCY_LIMIT = "1"; + try { + await undo(record.id, withDir({ yes: true })); + } finally { + delete process.env.CLERK_MIGRATE_CONCURRENCY_LIMIT; + } + + expect(deletedAt).toHaveLength(3); + for (const at of deletedAt.slice(1)) expect(at - deletedAt[0]!).toBeGreaterThanOrEqual(900); + }); + + test("deletes what the import created, records an undo run, and marks the import undone", async () => { + const record = importRun(); + + await undo(record.id, withDir({ yes: true })); + + expect( + deletes() + .map((request) => new URL(request.url).pathname) + .sort(), + ).toEqual(["/v1/users/user_a", "/v1/users/user_b"]); + const undoRun = listRuns(runsDir).find((candidate) => candidate.kind === "undo"); + expect(undoRun).toMatchObject({ + status: "complete", + undoes: record.id, + counts: { deleted: 2 }, + }); + expect(readRun(runsDir, record.id)).toMatchObject({ + status: "undone", + undoneBy: undoRun?.id, + }); + expect(process.exitCode).toBe(0); + }); + + test("counts a user already gone from the instance as deleted, without asking Clerk", async () => { + instanceUsers.delete("user_a"); + const record = importRun(); + + await undo(record.id, withDir({ yes: true })); + + expect(deletes()).toHaveLength(1); + expect(readRun(runsDir, record.id)?.status).toBe("undone"); + }); + + // Run later, the exported key may be gone: the retry names it as a placeholder. + test("a failed delete with an exported key suggests the retry with the key", async () => { + failing.add("user_b"); + const record = importRun(); + const previous = process.env.CLERK_SECRET_KEY; + process.env.CLERK_SECRET_KEY = "sk_test_x"; + try { + await undo(record.id, withDir({ yes: true, secretKey: undefined })); + } finally { + if (previous === undefined) delete process.env.CLERK_SECRET_KEY; + else process.env.CLERK_SECRET_KEY = previous; + } + + expect(Bun.stripANSI(captured.err)).toContain( + `\`clerk migrate undo ${record.id} --secret-key --runs-dir ${quoteArg(runsDir)}\` again to retry the users that failed`, + ); + }); + + test("a failed delete leaves the undo partial, exits 1, and a re-run continues it", async () => { + failing.add("user_b"); + const record = importRun(); + + await undo(record.id, withDir({ yes: true })); + + expect(process.exitCode).toBe(1); + const [partial] = listRuns(runsDir).filter((candidate) => candidate.kind === "undo"); + expect(partial).toMatchObject({ status: "partial", counts: { deleted: 1, failed: 1 } }); + expect(readRun(runsDir, record.id)?.status).toBe("partial"); + + failing.clear(); + requests = []; + process.exitCode = 0; + await undo(record.id, withDir({ yes: true })); + + // Only the user that failed is retried, in the same undo run. + expect(deletes().map((request) => new URL(request.url).pathname)).toEqual(["/v1/users/user_b"]); + const undoRuns = listRuns(runsDir).filter((candidate) => candidate.kind === "undo"); + expect(undoRuns).toHaveLength(1); + expect(undoRuns[0]).toMatchObject({ id: partial?.id, status: "complete" }); + expect(latestUserLines(runsDir, partial!.id).get("b")?.status).toBe("deleted"); + expect(readRun(runsDir, record.id)?.status).toBe("undone"); + }); + + // `runs ` builds the undo's retry from its record: the key this + // attempt used, not the first one's `.env`, which may not be there. + test("a continued undo records its own key source", async () => { + failing.add("user_b"); + const record = importRun(); + await undo(record.id, withDir({ yes: true })); + const [partial] = listRuns(runsDir).filter((candidate) => candidate.kind === "undo"); + patchRun(runsDir, partial!.id, { target: { ...partial!.target, keySource: ".env" } }); + + process.exitCode = 0; + await undo(record.id, withDir({ yes: true })); + + expect(readRun(runsDir, partial!.id)?.target.keySource).toBe("--secret-key"); + }); + + test("--json --yes returns the undo run and the result", async () => { + const record = importRun(); + + await undo(record.id, withDir({ yes: true, json: true })); + + const out = JSON.parse(captured.out); + // The import as it is now: undone, by this undo. + expect(out.import).toMatchObject({ id: record.id, status: "undone", undoneBy: out.run.id }); + expect(out).toMatchObject({ + run: { kind: "undo", undoes: record.id, status: "complete" }, + result: { deleted: 2, failed: 0, errors: [] }, + }); + }); +}); diff --git a/packages/cli-core/src/commands/migrate/undo.ts b/packages/cli-core/src/commands/migrate/undo.ts new file mode 100644 index 0000000000..45f1721116 --- /dev/null +++ b/packages/cli-core/src/commands/migrate/undo.ts @@ -0,0 +1,622 @@ +/** + * `clerk migrate undo ` — delete the users an import created. + * + * The import run is the whole record of what to delete: every source ID whose + * latest line is `created`, by the Clerk ID recorded next to it. Nothing is + * matched by searching the instance, so nothing the import did not create is + * ever in scope. + * + * Refuses (exit 2) rather than guessing when: + * - the resolved key addresses a different instance than the run imported into + * - the run is not an import + * - the run has already been undone + * + * The undo is a run of its own (`kind: "undo"`, `undoes: `). The import is + * marked `undone` only once every user is gone. A partial undo exits 1, and + * running `undo` again continues the same undo run. + */ + +import { bapiRequest } from "../../lib/bapi.ts"; +import { bold, dim, green, red } from "../../lib/color.ts"; +import { BapiError, CliError, throwUsageError, throwUserAbort } from "../../lib/errors.ts"; +import { log } from "../../lib/log.ts"; +import { interruptedExitCode } from "../../lib/signals.ts"; +import { confirm } from "../../lib/prompts.ts"; +import { withSpinner, type SpinnerControls } from "../../lib/spinner.ts"; +import { failureOf, tallyError } from "./lib/failures.ts"; +import { carryFlags, runsCommand, undoCommand } from "./lib/carry-flags.ts"; +import { canPrompt } from "./lib/terminal.ts"; +import { resolveLimits } from "./lib/instance.ts"; +import { retryOn429 } from "./lib/retry.ts"; +import { + continueRun, + latestUserLines, + lockRun, + patchRun, + readRun, + readUserLines, + resolveRunsDir, + runState, + startRun, + type Run, + type RunRecord, + type RunState, + openUndoOf, + inUseMessage, + noRunMessage, + withAttempt, + withLiveCounts, +} from "./lib/run-store.ts"; +import { withProgress, type ProgressUpdate } from "./lib/progress.ts"; +import { createApiScheduler, type ApiScheduler } from "./lib/scheduler.ts"; +import { + type ClerkTarget, + describeTarget, + isStandInInstanceId, + keyInstanceId, + laterCarry, + printTarget, + resolveClerkTarget, +} from "./lib/target.ts"; +import { plural } from "./lib/plural.ts"; +import { findInFlight, lookupUsers, type SourceUser } from "./lib/user-lookup.ts"; + +export type UndoOptions = { + dryRun?: boolean; + yes?: boolean; + /** With `--yes`, also delete users who have signed in since the import. */ + includeSignedIn?: boolean; + json?: boolean; + secretKey?: string; + app?: string; + instance?: string; + runsDir?: string; +}; + +type UndoPreview = { + /** Users that will be deleted. */ + toDelete: number; + /** Of those, how many have signed in since the import started. */ + signedInSince: number; + /** Users the import created that are already gone from the instance. */ + alreadyGone: number; + /** Users an earlier attempt at this undo already deleted. */ + alreadyDeleted: number; +}; + +/** + * Loads the import run, refusing anything that cannot be undone. `runsCarry` + * is `--runs-dir`, so a suggested listing reads the same runs folder. + */ +function readImportRun(runsDir: string, runId: string, runsCarry: string[]): RunRecord { + const record = readRun(runsDir, runId); + if (!record) throwUsageError(noRunMessage(runsDir, runId, runsCarry)); + if (record.kind !== "import") { + throwUsageError( + `Run ${runId} is an ${record.kind} run. Only an import run can be undone.`, + undefined, + undefined, + [{ command: runsCommand(runsCarry), description: "Find the import run to undo" }], + ); + } + if (record.status === "undone") { + throwUsageError(`Run ${runId} was already undone by run ${record.undoneBy ?? "(unknown)"}.`); + } + if (runState(runsDir, record) === "running") { + throwUsageError( + inUseMessage(runsDir, runId, "running", "Wait for it to finish, then undo it."), + ); + } + return record; +} + +/** Refuses to delete from an instance the import did not write to. */ +function assertSameInstance(record: RunRecord, target: ClerkTarget, secretKey: string): void { + if (record.target.instanceId === target.instanceId) return; + // Recorded under the key's stand-in ID while Clerk could not name the + // instance: the same key is the same instance. + if (record.target.instanceId === keyInstanceId(secretKey)) return; + // A `key_` ID is the fallback for an instance lookup that failed. It cannot + // be compared with an `ins_` ID, so this is "unknown", not "different". + const unconfirmed = [record.target.instanceId, target.instanceId].some(isStandInInstanceId); + if (unconfirmed) { + throwUsageError( + `Could not confirm that the resolved key addresses the instance run ${record.id} imported into: ` + + "Clerk did not answer GET /v1/instance (often rate limiting straight after a large import). " + + "Nothing was deleted. Try again in a minute; --verbose shows the response.", + ); + } + throwUsageError( + `Run ${record.id} imported into ${describeTarget(record.target)}, but the resolved key ` + + `addresses ${describeTarget(target)}. Nothing was deleted.\n` + + "Target the instance the import used with --secret-key, --app or --instance.", + ); +} + +/** + * Every user the import created, minus those an earlier undo already deleted. + * + * `unconfirmed` holds the source IDs whose create was in flight when the run + * stopped: they may exist in Clerk with no ID on record. + */ +function usersToDelete( + runsDir: string, + record: RunRecord, + openUndo: RunRecord | undefined, +): { users: SourceUser[]; unconfirmed: string[]; alreadyDeleted: number } { + const deleted = new Set(); + if (openUndo) { + for (const line of latestUserLines(runsDir, openUndo.id).values()) { + if (line.status === "deleted") deleted.add(line.sourceId); + } + } + + const users: SourceUser[] = []; + const unconfirmed: string[] = []; + for (const line of latestUserLines(runsDir, record.id).values()) { + if (deleted.has(line.sourceId)) continue; + if (line.status === "creating") unconfirmed.push(line.sourceId); + if (line.status !== "created" || !line.clerkId) continue; + users.push({ sourceId: line.sourceId, clerkId: line.clerkId }); + } + return { users, unconfirmed, alreadyDeleted: deleted.size }; +} + +/** + * Reads each user back from the instance, for the preview. + * + * @returns The users still there, and how many of them have signed in since + * the import started — the ones whose deletion someone will notice. + */ +async function inspectUsers(options: { + users: SourceUser[]; + since: string; + secretKey: string; + schedule: ApiScheduler; + spinner?: SpinnerControls; +}): Promise<{ + present: SourceUser[]; + gone: SourceUser[]; + /** The Clerk IDs of the users who signed in since the import. */ + signedIn: string[]; +}> { + const found = await lookupUsers({ + filter: "user_id", + values: options.users.map((user) => user.clerkId), + secretKey: options.secretKey, + schedule: options.schedule, + spinner: options.spinner, + label: "Checking the imported users", + }); + + const byId = new Map(found.map((user) => [user.id, user])); + const since = Date.parse(options.since); + const present = options.users.filter((user) => byId.has(user.clerkId)); + const gone = options.users.filter((user) => !byId.has(user.clerkId)); + const signedIn = present + .filter((user) => { + const lastSignIn = byId.get(user.clerkId)?.last_sign_in_at; + return typeof lastSignIn === "number" && lastSignIn > since; + }) + .map((user) => user.clerkId); + + return { present, gone, signedIn }; +} + +type UndoSummary = { + deleted: number; + failed: number; + errorBreakdown: Map; +}; + +/** Deletes each user, rate-limited and 429-retried exactly as the import is. */ +async function deleteUsers(options: { + users: SourceUser[]; + secretKey: string; + /** Undo's own scheduler, so its lookups and deletes share one rate budget. */ + schedule: ApiScheduler; + run: Run; + progress?: ProgressUpdate; +}): Promise { + const { users, secretKey, schedule, run, progress: report } = options; + const errorBreakdown = new Map(); + let deleted = 0; + let failed = 0; + + const progress = () => report?.({ done: deleted + failed, ok: deleted, failed }); + + // A failure on one user must not stop the rest: a half-undone import with + // no record of which half is far worse than a reported failure. + const deleteOne = async (user: SourceUser): Promise => { + const retries: string[] = []; + try { + await retryOn429( + async () => + schedule(async () => + bapiRequest({ method: "DELETE", path: `/v1/users/${user.clerkId}`, secretKey }), + ), + { + // A 429 pauses every delete still queued, as on import. + onRetry: ({ message, delaySeconds }) => { + retries.push(message); + schedule.pause(delaySeconds * 1000); + }, + }, + ); + deleted++; + run.append({ + sourceId: user.sourceId, + clerkId: user.clerkId, + status: "deleted", + ...(retries.length > 0 ? { error: retries.join("; ") } : {}), + }); + } catch (error) { + // Gone already, by the dashboard or another undo: the goal is met. + if (error instanceof BapiError && error.status === 404) { + deleted++; + run.append({ + sourceId: user.sourceId, + clerkId: user.clerkId, + status: "deleted", + reason: "already deleted", + }); + } else { + failed++; + const { message, code } = failureOf(error); + tallyError(errorBreakdown, message); + run.append({ + sourceId: user.sourceId, + clerkId: user.clerkId, + status: "failed", + error: [message, ...retries].join("; "), + code, + }); + } + } + progress(); + }; + + progress(); + await Promise.all(users.map(deleteOne)); + return { deleted, failed, errorBreakdown }; +} + +function printPreview(record: RunRecord, preview: UndoPreview): void { + log.blank(); + // Every user already gone: the undo only records the import as undone. + log.info( + preview.toDelete > 0 + ? `${bold(`Will delete ${plural(preview.toDelete, "user")}`)} created by import run ${record.id}.` + : `${bold(`Will mark import run ${record.id} undone`)}: nothing is left to delete.`, + ); + if (preview.signedInSince > 0) { + const who = + preview.signedInSince === 1 ? "1 of them has" : `${preview.signedInSince} of them have`; + log.warn(`${who} signed in since the import. Deleting removes their accounts and sessions.`); + } + if (preview.alreadyGone > 0) { + log.info( + dim( + `${plural(preview.alreadyGone, "user")} the import created ${preview.alreadyGone === 1 ? "is" : "are"} already gone.`, + ), + ); + } + if (preview.alreadyDeleted > 0) { + log.info( + dim( + `${plural(preview.alreadyDeleted, "user")} ${preview.alreadyDeleted === 1 ? "was" : "were"} deleted by an earlier attempt.`, + ), + ); + } +} + +/** The undo run a stop left on disk, as `runs` shows an unfinished run. */ +type StoppedRun = RunRecord & { state: RunState }; + +/** + * What the JSON adds to the preview: why undo stopped before deleting, or, + * once it did, its run and result. One of these, never two. + */ +type UndoOutcome = + | { dryRun: true } + | { consent: "required"; includeSignedIn?: "required" } + | { changed: true; signedInSincePreview?: number } + | { recheckFailed: true } + | { writeFailed: true; run: StoppedRun | null } + | { stopped: true; run: StoppedRun | null } + | { + run: RunRecord; + result: { deleted: number; failed: number; errors: { error: string; count: number }[] }; + }; + +export async function undo(runId: string, options: UndoOptions = {}): Promise { + const runsDir = await resolveRunsDir(options.runsDir); + const runsCarry = carryFlags({ runsDir: options.runsDir }); + const record = readImportRun(runsDir, runId, runsCarry); + const importLines = readUserLines(runsDir, record.id).length; + // The flags that found this run and instance, for a refusal's command, run + // again straight away in the same shell. + const carry = carryFlags(options); + const undoAgain = undoCommand(record.id, carry); + + const { secretKey, target } = await resolveClerkTarget(options); + // After a partial undo, run later, maybe without the key exported: from the target. + const undoLater = undoCommand(record.id, laterCarry(options.runsDir, target)); + if (!options.json) printTarget(target); + assertSameInstance(record, target, secretKey); + + const { concurrencyLimit, rateLimit } = resolveLimits(secretKey); + const openUndo = openUndoOf(runsDir, record.id); + const undoLines = openUndo ? readUserLines(runsDir, openUndo.id).length : 0; + const recorded = usersToDelete(runsDir, record, openUndo); + + const schedule = createApiScheduler(concurrencyLimit, rateLimit); + const users = [ + ...recorded.users, + ...(await findInFlight({ + runId: record.id, + sourceIds: recorded.unconfirmed, + secretKey, + schedule, + })), + ]; + const { present, gone, signedIn } = + users.length > 0 + ? await withSpinner("Checking the imported users...", async (spinner) => + inspectUsers({ users, since: record.startedAt, secretKey, schedule, spinner }), + ) + : { present: [], gone: [], signedIn: [] }; + + const preview: UndoPreview = { + toDelete: present.length, + signedInSince: signedIn.length, + alreadyGone: gone.length, + alreadyDeleted: recorded.alreadyDeleted, + }; + + if (!options.json) printPreview(record, preview); + + // Consent to go ahead, for the same run and instance. + const command = undoCommand(record.id, [ + "--yes", + ...(preview.signedInSince > 0 ? ["--include-signed-in"] : []), + ...carry, + ...(options.json ? ["--json"] : []), + ]); + const nothingLeft = present.length === 0 && gone.length === 0; + + // After the preview, --json prints its object exactly once, whatever + // happens: the dry run, the result, a refusal with its marker, or, for + // anything else that stops the undo, the preview marked `writeFailed` (the + // file system refused) or `stopped`. The reason goes to stderr as for any + // error. `import` is the run being undone; `run` is the undo's own run, null + // until one is written, as an import's is. + let printed = false; + const printJson = (extra: UndoOutcome, imported = record) => { + if (!options.json || printed) return; + printed = true; + log.data(JSON.stringify({ target, import: imported, run: null, preview, ...extra }, null, 2)); + }; + + if (options.dryRun) { + printJson({ dryRun: true }); + if (!options.json) { + log.blank(); + const then = present.length > 0 ? "to delete them" : "to mark it undone"; + log.info(dim(`Dry run: nothing was deleted. Run \`${command}\` ${then}.`)); + } + return; + } + + // Rule 1: nothing is deleted without consent — `--yes`, or a yes at a + // prompt. `--json` never prompts. + if (!nothingLeft && !options.yes) { + // Every user already gone: the undo only records the import as undone. + const what = + present.length > 0 + ? `permanently deletes ${plural(present.length, "user")}` + : `marks run ${record.id} undone`; + if (!canPrompt(options)) { + printJson({ consent: "required" }); + throwUsageError( + `\`clerk migrate undo\` ${what} and needs consent. Pass --yes to confirm.`, + undefined, + undefined, + [ + { + command, + description: + present.length > 0 + ? "Delete the users this import created" + : `Mark run ${record.id} undone`, + }, + ], + ); + } + const proceed = await confirm({ + message: + present.length > 0 + ? `Permanently delete ${plural(present.length, "user")}?` + : `Mark run ${record.id} undone? Its users are already gone.`, + default: false, + }); + if (!proceed) throwUserAbort(); + } + + // Rule 2: `--yes` consents to undoing the import, not to deleting accounts + // people have used since. That takes its own flag; a human at the prompt has + // the warning above. + if (options.yes && preview.signedInSince > 0 && !options.includeSignedIn) { + // Its own marker: a script that retries with --yes would loop on exit 2. + printJson({ consent: "required", includeSignedIn: "required" }); + throwUsageError( + `${plural(preview.signedInSince, "user")} the import created ${preview.signedInSince === 1 ? "has" : "have"} signed in since, and --yes alone does not delete them. ` + + "Pass --include-signed-in to delete them too. Nothing was deleted.", + undefined, + undefined, + [{ command, description: "Delete every user this import created" }], + ); + } + + const refuseChanged = (message: string, extra: { signedInSincePreview?: number } = {}): never => { + printJson({ changed: true, ...extra }); + throwUsageError(message); + }; + + // The undo's own run, once it is on disk: a stop still names it. + let run: Run | undefined; + try { + await deleteAfterPreview(); + } catch (error) { + // As `runs` shows it: the record still holds the counts it opened with. + const written: StoppedRun | null = run + ? { + ...withLiveCounts(runsDir, run.record), + state: runState(runsDir, run.record), + } + : null; + printJson( + isFileSystemError(error) + ? { writeFailed: true, run: written } + : { stopped: true, run: written }, + ); + throw error; + } + + async function deleteAfterPreview(): Promise { + // Gitignored only now, once there is consent to write a run. + await resolveRunsDir(options.runsDir, { write: true }); + + // Held until the import is marked undone, so a re-import cannot continue + // the run while its users are deleted and overwrite the `undone` mark. + let releaseImport: () => void; + try { + releaseImport = lockRun(runsDir, record.id); + } catch (error) { + if (error instanceof CliError) refuseChanged(error.message); + throw error; + } + let summary: UndoSummary; + let undoRecord: RunRecord; + let importRecord = record; + try { + // Another undo that finished during the preview has already marked it. + try { + readImportRun(runsDir, record.id, runsCarry); + } catch (error) { + if (error instanceof CliError) refuseChanged(error.message); + throw error; + } + // A re-import that continued the run during the preview created users + // the preview never listed. Another undo that started or carried on an + // undo run in the meantime changed which users are left to delete, and a + // second undo run would split the import's deletes between the two. + const openNow = openUndoOf(runsDir, record.id); + if ( + readUserLines(runsDir, record.id).length !== importLines || + openNow?.id !== openUndo?.id || + (openUndo !== undefined && readUserLines(runsDir, openUndo.id).length !== undoLines) + ) { + refuseChanged( + `Run ${record.id} changed while this undo was waiting, so the preview is out of date. ` + + `Nothing was deleted. Run \`${undoAgain}\` again.`, + ); + } + + // A user who signed in after the preview is one nobody agreed to delete: + // --yes covered the preview's users, and so did a yes at the prompt. By + // ID, not count: one the preview showed can leave as another signs in. + if (present.length > 0 && !options.includeSignedIn) { + let now: Awaited>; + try { + now = await inspectUsers({ + users: present, + since: record.startedAt, + secretKey, + schedule, + }); + } catch (error) { + printJson({ recheckFailed: true }); + throw error; + } + const seen = new Set(signedIn); + const more = now.signedIn.filter((clerkId) => !seen.has(clerkId)).length; + if (more > 0) { + refuseChanged( + `${plural(more, "more user")} the import created signed in after the preview, and nothing agreed to delete them. Nothing was deleted.\n` + + `Run \`${undoAgain}\` again to see them, or pass --include-signed-in to delete them too.`, + { signedInSincePreview: more }, + ); + } + } + + run = openUndo + ? continueRun(runsDir, withAttempt(openUndo, target)) + : startRun(runsDir, { kind: "undo", target, undoes: record.id, source: record.source }); + + // Users the import created that are no longer there: the undo's goal for + // them is already met, so they count as deleted without another request. + for (const user of gone) { + run.append({ ...user, status: "deleted", reason: "not found in the instance" }); + } + + summary = + present.length > 0 + ? await withProgress({ total: present.length, verb: "deleted" }, async (progress) => + // Set above; a callback doesn't keep the narrowing of an outer `let`. + deleteUsers({ users: present, secretKey, schedule, run: run!, progress }), + ) + : { deleted: 0, failed: 0, errorBreakdown: new Map() }; + + // A Ctrl-C rejects every delete still queued. Left unfinished, the undo + // reads as interrupted, as an import does. + if (interruptedExitCode() !== null) { + run.release(); + throwUserAbort(); + } + undoRecord = run.finish(); + if (undoRecord.status === "complete") { + importRecord = + patchRun(runsDir, record.id, { status: "undone", undoneBy: undoRecord.id }) ?? record; + } + } finally { + releaseImport(); + } + + if (options.json) { + printJson( + { + run: undoRecord, + result: { + deleted: summary.deleted, + failed: summary.failed, + errors: [...summary.errorBreakdown].map(([error, count]) => ({ error, count })), + }, + }, + importRecord, + ); + } else { + log.blank(); + log.info(`${bold("Deleted:")} ${green(String(summary.deleted))}`); + log.info(`${bold("Failed:")} ${red(String(summary.failed))}`); + for (const [error, count] of summary.errorBreakdown) { + log.info(` ${plural(count, "user")}: ${error}`); + } + log.blank(); + log.info(dim(`Undo run ${undoRecord.id}: ${run.dir}`)); + if (undoRecord.status === "complete") { + log.success(`Import run ${record.id} is undone.`); + } else { + log.info(`Run \`${undoLater}\` again to retry the users that failed.`); + } + } + + if (undoRecord.status !== "complete") process.exitCode = 1; + } +} + +/** An error the file system raised (EACCES, ENOSPC, …), rather than the API or the user. */ +function isFileSystemError(error: unknown): boolean { + const { code, errno } = (error ?? {}) as { code?: unknown; errno?: unknown }; + return typeof code === "string" && typeof errno === "number"; +} diff --git a/packages/cli-core/src/lib/keyless-target.ts b/packages/cli-core/src/lib/keyless-target.ts index 0ae9492b99..7accee935b 100644 --- a/packages/cli-core/src/lib/keyless-target.ts +++ b/packages/cli-core/src/lib/keyless-target.ts @@ -82,6 +82,9 @@ interface LocatedKey { source: string; } +/** How an exported variable is named as a key's source: `NUXT_CLERK_SECRET_KEY env var`. */ +export const ENV_VAR_SOURCE_SUFFIX = " env var"; + /** * Looks for a key under any of `names`, in the order the app itself would * resolve one: the environment first, then env files with a later file @@ -90,7 +93,7 @@ interface LocatedKey { async function findKeyInProject(cwd: string, names: string[]): Promise { for (const name of new Set(names)) { const value = process.env[name]; - if (value) return { value, source: `${name} env var` }; + if (value) return { value, source: `${name}${ENV_VAR_SOURCE_SUFFIX}` }; } // Priority is by name, not by position: the framework-specific name beats diff --git a/packages/cli-core/src/lib/next-steps.ts b/packages/cli-core/src/lib/next-steps.ts index e11e94a509..331f8e2cfb 100644 --- a/packages/cli-core/src/lib/next-steps.ts +++ b/packages/cli-core/src/lib/next-steps.ts @@ -75,9 +75,15 @@ export const NEXT_STEPS = { "Run `clerk apps list` to see your other applications", "Run `clerk config pull` to inspect the live configuration of this instance", ], - MIGRATE_DONE: (runFolder: string) => [`See each user's outcome in ${runFolder}/users.ndjson`], - MIGRATE_DONE_WITH_ERRORS: (runFolder: string) => [ - `See every user that failed, and why, in ${runFolder}/users.ndjson`, + // Ready-made commands, with the flags that reach the same run and instance. + MIGRATE_DONE: (runsCommand: string, undoCommand: string) => [ + `Run \`${runsCommand}\` to see what happened to each user`, + `Run \`${undoCommand}\` to delete the users it created`, + ], + MIGRATE_PARTIAL: (runsCommand: string, retryCommand: string, undoCommand: string) => [ + `Run \`${runsCommand}\` to see every user that didn't finish, and why`, + `Run \`${retryCommand}\` to retry them, in the same run`, + `Run \`${undoCommand}\` to delete the users it created`, ], } as const; @@ -99,9 +105,9 @@ export function printNextSteps(steps: readonly string[]): void { * * Opt-in rather than folded into `printNextSteps`, because most steps are a * nudge towards a command someone might like to run next. The migrate ones are - * not: they name the file holding the per-user record of what landed. An agent - * that imported 10,000 users and lost 300 of them needs it, and has no gutter - * to read it from. + * not: they name the command that shows each user's outcome, and the one that + * undoes the run. An agent that imported 10,000 users and lost + * 300 of them needs both, and has no gutter to read them from. */ export function printAgentNextSteps(steps: readonly string[]): void { if (isHuman()) return; diff --git a/test/e2e/migrate.test.ts b/test/e2e/migrate.test.ts index df94ca644f..9b7c15ddbc 100644 --- a/test/e2e/migrate.test.ts +++ b/test/e2e/migrate.test.ts @@ -1,11 +1,11 @@ /** - * Live-BAPI tests for `clerk migrate import`, covering what only a real + * Live-BAPI tests for `clerk migrate import` and `undo`, covering what only a real * instance can answer: * * - A Supabase export round-trips: the dry run counts it without writing, an * import without `--yes` writes nothing, the import creates every user and - * records the run, and each bcrypt hash verifies against the password it - * was made from. + * records the run, each bcrypt hash verifies against the password it was + * made from, and `undo` deletes them again. * - A user whose only email is unverified, imported into an instance that * requires an email, is refused by Clerk. The import's checks reject that * user up front on the strength of this test, so it checks both halves. @@ -93,7 +93,7 @@ function writeExport(users: Record[]): string { return file; } -test("a Supabase export dry-runs, imports, and its passwords verify", async () => { +test("a Supabase export dry-runs, imports, its passwords verify, and undo deletes it", async () => { const users = await Promise.all( [0, 1].map(async () => { const hex = randomBytes(6).toString("hex"); @@ -175,6 +175,20 @@ test("a Supabase export dry-runs, imports, and its passwords verify", async () = ]); expect(JSON.parse(verify.stdout.toString())).toMatchObject({ verified: true }); } + + const undo = await cli(["migrate", "undo", result.run.id, "--yes", "--json"]); + expect(undo.exitCode).toBe(0); + for (const { record } of users) { + const line = lines.find((candidate) => candidate.sourceId === record.id); + expect(typeof line?.clerkId).toBe("string"); + // Clerk's own answer, not just any failure: a wrong path or an auth error + // would exit non-zero too. `clerk api` prints the error body to stdout. + const gone = await cli(["api", `/users/${line?.clerkId as string}`]); + expect(gone.exitCode).not.toBe(0); + expect(JSON.parse(gone.stdout.toString())).toMatchObject({ + errors: [{ code: "resource_not_found" }], + }); + } }, 60_000); test("a user whose only email is unverified is refused where email is required", async () => {