diff --git a/.changeset/deploy-wizard-copy.md b/.changeset/deploy-wizard-copy.md new file mode 100644 index 000000000..d4b5d90b7 --- /dev/null +++ b/.changeset/deploy-wizard-copy.md @@ -0,0 +1,17 @@ +--- +"clerk": patch +--- + +`clerk deploy` copy fixes: + +- `clerk deploy --help` describes what the bare command does and the JSON report it prints under an agent. +- The preamble says a hosting provider's generated URL can't be the production domain. +- The confirmation screen lists all five DNS record hosts, including DKIM, and says DNS records will be needed for them once the instance exists. +- The DNS check reports records as "not found yet" with a minutes-not-days expectation, tells you what to do based on what's actually pending, and links the Dashboard Domains page for changing the domain. +- `clerk auth login` prints the claimed app's Dashboard URL; the wizard prints the new production instance's URL and its next steps say the pulled keys go on the host alongside the other Clerk variables. +- The Google walkthrough adds a tip explaining that the OAuth consent screen's app name is what users see when they sign in, and to choose the name they should see. +- The DNS check footer points at the "Check again" prompt that follows it instead of telling you to quit and re-run; the closing screen no longer says "Production ready", "sign up at your domain", or "Success" when DNS verification was skipped. +- Resuming the wizard shows only the DNS records still outstanding, not ones Clerk already verified. When none are outstanding, the DNS screen says what is (the SSL certificate, Clerk finalizing, or a record list Clerk didn't return) instead of a "Configure DNS" page with no records, and the closing screen says "Not yet verified" rather than "DNS pending" so it is right when DNS is done and something else is pending. +- Each DNS record host is named the same way on every screen, the note about what Clerk manages moved off the rows the user has to add, the Domains-page pointer carries its link, and the closing line says what happens next instead of implying that skipping the check finishes the deploy, and no longer promises an OAuth step on resume when OAuth has already run. +- Agent-mode `nextAction` tells the agent to add pending DNS records instead of polling, at `complete` says the production keys still have to reach the host, and names OAuth providers the CLI could not configure so an agent doesn't report OAuth as done. Human-mode `clerk deploy status` prints the pending records, never says "ask the user", and resumes with `clerk deploy` rather than a flag that only affects agents. +- The wizard's new sentences wrap at 76 columns so they stay inside the gutter on an 80-column terminal. The agent JSON report gains a `urls` field with the production instance's Dashboard page and its Domains page. Human-mode `deploy status` now renders its sentence from the same classification of the report as `nextAction` rather than by rewriting that sentence; both read the same as before this change. diff --git a/packages/cli-core/src/cli-program.test.ts b/packages/cli-core/src/cli-program.test.ts index 45cee7322..8654d18ee 100644 --- a/packages/cli-core/src/cli-program.test.ts +++ b/packages/cli-core/src/cli-program.test.ts @@ -64,6 +64,45 @@ test("deploy relies on global options", () => { expect(optionNames).toEqual([]); }); +test("deploy --help describes the bare command, not only the status subcommand", () => { + // The wizard is a hidden default subcommand, so without the long + // description the help lists only `status` and reads as if the CLI can only + // watch a deploy. Both headless test agents concluded exactly that. + const program = createProgram(); + program.configureOutput({ getOutHelpWidth: () => 80 }); + const deploy = program.commands.find((command) => command.name() === "deploy")!; + deploy.configureOutput({ getOutHelpWidth: () => 80 }); + const help = deploy.helpInformation().replace(new RegExp(String.raw`\x1b\[[0-9;]*m`, "g"), ""); + + expect(help).toBe( + [ + "Usage: clerk deploy [options] [command]", + "", + "Deploy a Clerk application to production.", + "", + "Running `clerk deploy` with no subcommand starts an interactive setup that", + "creates the production instance, prints the DNS records you must add, collects", + "production OAuth credentials, and verifies the domain. It needs a terminal;", + "re-run it at any time to resume where you left off.", + "", + "When run by an agent (or without a TTY), it is read-only: it prints a JSON", + "status report with the current state and a `nextAction` field saying what to", + "do next. `clerk deploy status` prints the same report; add `--wait` to keep", + "checking until DNS, SSL, and email DNS are verified.", + "", + "Options:", + " -h, --help Display help for command", + "", + "Commands:", + " help [command] Display help for command", + " status [options] Show production deploy status (read-only)", + "", + ].join("\n"), + ); + // The root `clerk --help` table keeps the one-line summary. + expect(deploy.summary()).toBe("Deploy a Clerk application to production"); +}); + test("deploy status exposes wait option", () => { const program = createProgram(); const deploy = program.commands.find((command) => command.name() === "deploy")!; diff --git a/packages/cli-core/src/commands/auth/README.md b/packages/cli-core/src/commands/auth/README.md index ced584a3d..4a7a7b918 100644 --- a/packages/cli-core/src/commands/auth/README.md +++ b/packages/cli-core/src/commands/auth/README.md @@ -25,7 +25,7 @@ When `clerk init` runs in accountless mode it writes `.clerk/keyless.json` conta - **404** — claim token expired or application already deleted; breadcrumb is cleared and a warning is shown. - **403** — authenticated account has no active organization; breadcrumb is cleared and a warning is shown. - **Any other error** — treated as transient; breadcrumb is preserved so the next login retries. -- **Success** — application is claimed and linked, `.env` is updated via `clerk env pull`, breadcrumb is deleted. +- **Success** — application is claimed and linked, `.env` is updated via `clerk env pull`, breadcrumb is deleted. The command prints the Dashboard URL of the claimed app's development instance, since this is the first time the app has a home in an account. #### API Endpoints diff --git a/packages/cli-core/src/commands/auth/login.test.ts b/packages/cli-core/src/commands/auth/login.test.ts index e04b01743..6cb844544 100644 --- a/packages/cli-core/src/commands/auth/login.test.ts +++ b/packages/cli-core/src/commands/auth/login.test.ts @@ -92,8 +92,12 @@ mock.module("../../lib/first-application.ts", () => ({ ensureFirstApplication: () => mockEnsureFirstApplication(), })); +const mockAttemptAutoclaim = mock(async (_cwd: string): Promise => ({ + status: "not_keyless", +})); + mock.module("../../lib/autoclaim.ts", () => ({ - attemptAutoclaim: async () => ({ status: "not_keyless" }), + attemptAutoclaim: (cwd: string) => mockAttemptAutoclaim(cwd), })); const { setLogLevel } = await import("../../lib/log.ts"); @@ -127,6 +131,8 @@ describe("login", () => { mockOpenBrowser.mockReset(); mockEnsureFirstApplication.mockReset(); mockEnsureFirstApplication.mockResolvedValue(undefined); + mockAttemptAutoclaim.mockReset(); + mockAttemptAutoclaim.mockResolvedValue({ status: "not_keyless" }); mockIsHuman.mockReturnValue(false); mockOpenBrowser.mockResolvedValue({ ok: true, launcher: "test" }); mockRevokeToken.mockResolvedValue("revoked"); @@ -521,6 +527,88 @@ describe("login", () => { expect(captured.err).toContain("Linked to `app_abc123`"); }); + test("prints where the claimed app now lives in the Dashboard", async () => { + // First time this app has a home in an account; nothing else in the flow + // says where it is, so the claim line has to. + mockGetValidToken.mockResolvedValue(null); + mockOAuthSuccess(); + mockResolveProfile.mockResolvedValue(undefined); + mockAttemptAutoclaim.mockResolvedValue({ + status: "claimed", + envPulled: true, + app: { + application_id: "app_claimed", + name: "bad-agent", + instances: [ + { + instance_id: "ins_dev_claimed", + environment_type: "development", + publishable_key: "pk_test_claimed", + }, + ], + }, + }); + + await runLogin(); + + expect(captured.err).toContain("Claimed and linked application: `bad-agent`"); + // The URL sits on its own indented line so the sentence fits the frame. + expect(captured.err).toMatch( + /Your app now lives in your Clerk account:\n[^\n]*? {2}https:\/\/dashboard\.clerk\.com\/apps\/app_claimed\/instances\/ins_dev_claimed/, + ); + }); + + test("links to the development instance even when production is listed first", async () => { + mockGetValidToken.mockResolvedValue(null); + mockOAuthSuccess(); + mockResolveProfile.mockResolvedValue(undefined); + mockAttemptAutoclaim.mockResolvedValue({ + status: "claimed", + envPulled: true, + app: { + application_id: "app_claimed", + name: "bad-agent", + instances: [ + { + instance_id: "ins_prod_claimed", + environment_type: "production", + publishable_key: "pk_live_claimed", + }, + { + instance_id: "ins_dev_claimed", + environment_type: "development", + publishable_key: "pk_test_claimed", + }, + ], + }, + }); + + await runLogin(); + + expect(captured.err).toContain( + "https://dashboard.clerk.com/apps/app_claimed/instances/ins_dev_claimed", + ); + expect(captured.err).not.toContain("ins_prod_claimed"); + }); + + test("a claim response without instances still reports the claim and does not throw", async () => { + // The Dashboard line is a nice-to-have; a missing array in API JSON must + // not turn a claim that already succeeded server-side into a failed login. + mockGetValidToken.mockResolvedValue(null); + mockOAuthSuccess(); + mockResolveProfile.mockResolvedValue(undefined); + mockAttemptAutoclaim.mockResolvedValue({ + status: "claimed", + envPulled: true, + app: { application_id: "app_claimed", name: "bad-agent" }, + }); + + await runLogin(); + + expect(captured.err).toContain("Claimed and linked application: `bad-agent`"); + expect(captured.err).not.toContain("Your app now lives"); + }); + test("shows default next steps when not linked", async () => { mockGetValidToken.mockResolvedValue(null); mockOAuthSuccess(); diff --git a/packages/cli-core/src/commands/auth/login.ts b/packages/cli-core/src/commands/auth/login.ts index 29a80d12c..611f545de 100644 --- a/packages/cli-core/src/commands/auth/login.ts +++ b/packages/cli-core/src/commands/auth/login.ts @@ -6,7 +6,7 @@ import { revokeToken, type UserInfo, } from "../../lib/token-exchange.ts"; -import { getOAuthConfig } from "../../lib/environment.ts"; +import { buildDashboardUrl, getOAuthConfig } from "../../lib/environment.ts"; import { createOAuthSession, getStoredSession, @@ -230,6 +230,17 @@ async function handleAutoclaim(cwd: string): Promise { if (result.status === "claimed") { const label = result.app.name || result.app.application_id; log.success(`Claimed and linked application: \`${label}\``); + // First time this app has a home in an account; say where it is. + // Deserialized API JSON; a missing array must not fail a claim that + // already succeeded server-side. + const development = result.app.instances?.find( + (instance) => instance.environment_type === "development", + ); + if (development) { + // URL on its own line: with it, the sentence is wider than the frame. + log.info("Your app now lives in your Clerk account:"); + log.info(` ${buildDashboardUrl(result.app.application_id, development.instance_id)}`); + } } const warning = CLAIM_WARNINGS[result.status]; diff --git a/packages/cli-core/src/commands/deploy/README.md b/packages/cli-core/src/commands/deploy/README.md index 211d36ea9..4c97feea8 100644 --- a/packages/cli-core/src/commands/deploy/README.md +++ b/packages/cli-core/src/commands/deploy/README.md @@ -49,9 +49,10 @@ In agent mode, `clerk deploy status` emits JSON on stdout with: - `complete`: `true` only when the domain is verified and all supported OAuth providers enabled in development have production credentials. - `state`: `complete`, `domain_pending`, `oauth_pending`, `domain_provisioning`, `not_started`, or `interrupted`. - `domainStatus`: per-component DNS, SSL, and email DNS status when a domain exists. -- `pendingDnsRecords`: CNAME records still tied to pending DNS-backed checks. +- `pendingDnsRecords`: CNAME records still tied to pending DNS-backed checks, each with `host`, `value`, and Clerk's `required` flag (some targets are optional). - `oauth`: configured, pending, and unsupported provider slugs. -- `nextAction`: the next step an agent should present to the user, including the Clerk Dashboard domains URL when a production instance exists. Agents should ask whether to open that URL for the user. +- `urls`: the production instance's Dashboard page (`instance`) and its Domains page (`domains`), or `null` before a production instance exists. The same URLs appear in `nextAction` prose; this field is the one to read programmatically. +- `nextAction`: the next step an agent should present to the user. While domain setup remains (`domain_provisioning` and `domain_pending`) it includes the Clerk Dashboard domains URL, and agents should ask whether to open that URL for the user; `not_started`, `interrupted`, and `oauth_pending` carry no URL, and `complete` carries the instance root instead. While DNS or email DNS records are unverified it says to add the records in `pendingDnsRecords` at the domain's DNS provider rather than to keep polling; if that list is empty (the API returned no CNAME targets) it says so and points at the Dashboard Domains page instead; when only SSL is pending it says to wait. At `complete` it says the production keys still have to reach the host (`clerk env pull --instance prod`, alongside the other Clerk variables in the env file) and to sign up on the domain to confirm — "complete" is Clerk's side only — and links the instance root (users, settings, billing) instead of the domains page, since nothing is left to monitor there. At `complete` and `oauth_pending` it also names any providers in `oauth.unsupported`, since `oauth.complete` covers only what the CLI manages and those providers' sign-in fails in production until configured in the Dashboard. `oauth_pending` carries no Domains URL (the domain is verified). Human mode prints its own sentence, rendered from the same classification of the report (`deployNextStep` in `status.ts`) rather than by rewriting the agent's: no unsupported-provider clause (the warning row above already says it), no "ask the user" (the reader is the user), no `--wait` (human mode always waits; the wizard is what resumes setup), and, when records are pending, the records themselves printed first so the sentence only says what happens once they are added. Before a production instance exists, human mode omits the OAuth row rather than printing "pending: none" for something that was never checked. Exit codes: @@ -176,10 +177,10 @@ Most providers ask for `client_id` and `client_secret`. Provider-specific schema The CLI keeps small local overrides for provider setup details that schema does not fully describe: -| Provider | Override | -| -------- | ------------------------------------------------------------------------------------------ | -| Google | Optional Google Cloud Console JSON import and OAuth consent screen warning | -| Apple | `.p8` file import, production-required `team_id` and `key_id`, native-only field omissions | +| Provider | Override | +| -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Google | Optional Google Cloud Console JSON import, OAuth consent screen warning, and a tip that the consent-screen name is what users see, so choose the name you want them to see | +| Apple | `.p8` file import, production-required `team_id` and `key_id`, native-only field omissions | For Google, the wizard can load `client_id` and `client_secret` from the top-level `web` object in a Google Cloud Console OAuth client JSON file, or from `installed` for desktop-style client downloads. The file contents are used in memory and are not written to CLI config. diff --git a/packages/cli-core/src/commands/deploy/copy.test.ts b/packages/cli-core/src/commands/deploy/copy.test.ts index 0d60a6c9a..8d903282d 100644 --- a/packages/cli-core/src/commands/deploy/copy.test.ts +++ b/packages/cli-core/src/commands/deploy/copy.test.ts @@ -1,14 +1,38 @@ import { test, expect, describe } from "bun:test"; import { + DEPLOY_COMMAND_DESCRIPTION, + DEPLOY_COMMAND_SUMMARY, + INTRO_PREAMBLE, + OAUTH_SECTION_INTRO, bindZoneFile, deployComponentLabels, + deployStatusPendingFooter, deployStatusRetryMessage, + dnsIntro, + dnsDashboardHandoff, + dnsHandoffNothingToAdd, dnsRecords, - nextStepsBlock, - pendingDnsRecords, + domainAssociationSummary, + domainsDashboardUrl, + instanceDashboardUrl, + nextStepsBody, + pendingCnameTargets, + productionDnsHosts, + productionSummary, } from "./copy.ts"; import type { CnameTarget } from "../../lib/plapi.ts"; +const stripAnsi = (value: string): string => + value.replace(new RegExp(String.raw`\x1b\[[0-9;]*m`, "g"), ""); + +/** + * Lines with wrap continuations folded back into their sentence, for asserting + * on what a sentence says. Assertions about layout (blank lines, a URL on its + * own line, bullets) use the raw join instead. + */ +const flat = (lines: readonly string[] | string): string => + stripAnsi(typeof lines === "string" ? lines : lines.join("\n")).replace(/\n[ \t]*/g, " "); + describe("bindZoneFile", () => { const fixedDate = new Date("2026-05-20T18:30:00.000Z"); @@ -72,13 +96,27 @@ describe("dnsRecords", () => { expect(output).toContain(" Host: clk2._domainkey.example.com"); // Both DKIM hosts must carry the email label, not fall through to the // generic "CNAME" default (the host's first label is "clk"/"clk2"). - expect( - output.filter((line) => line.includes("Email (Clerk handles SPF/DKIM automatically)")), - ).toHaveLength(2); + expect(output.filter((line) => line.includes("Email (DKIM)"))).toHaveLength(2); expect(output.some((line) => line.trimStart().startsWith("CNAME"))).toBe(false); }); }); +describe("dnsRecords heading", () => { + const targets: CnameTarget[] = [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + ]; + + test("first hand-over tells the user to add the records", () => { + expect(dnsRecords(targets)[0]).toBe("Add the following records at your DNS provider:"); + }); + + test("after a check that didn't find them, the heading allows that they may already be added", () => { + expect(dnsRecords(targets, { afterCheck: true })[0]).toBe( + "Add the following records at your DNS provider if you haven't already:", + ); + }); +}); + describe("deployComponentLabels", () => { test("returns email DNS in-progress and done labels", () => { expect(deployComponentLabels("mail", "example.com")).toEqual({ @@ -110,44 +148,320 @@ describe("deployStatusRetryMessage", () => { }); }); -describe("nextStepsBlock", () => { - test("links directly to the production instance domain settings", () => { - const output = nextStepsBlock("app_123", "ins_456"); +describe("nextStepsBody", () => { + test("links to the production instance home and its domain settings", () => { + const output = stripAnsi(nextStepsBody("app_123", "ins_456", "example.com", "verified")); - expect(output).toContain("View and manage domain configuration in the Clerk Dashboard"); + expect(output).toContain("Manage this instance in the Clerk Dashboard"); + expect(output).toContain("- Users, settings, and billing:"); + expect(output).toContain("- DNS and SSL status:"); + // The instance root on its own line, not only as a prefix of the domains URL. + expect(output).toContain("https://dashboard.clerk.com/apps/app_123/instances/ins_456\n"); expect(output).toContain("https://dashboard.clerk.com/apps/app_123/instances/ins_456/domains"); }); + + test("says the pulled keys go on the host alongside the other Clerk variables", () => { + // `env pull --instance prod` writes only the two keys. The routing + // variables `init` wrote have to be carried over by hand, or sign-in + // silently falls back to the hosted Account Portal. + const output = nextStepsBody("app_123", "ins_456", "example.com", "verified"); + + expect(output).toContain("- Add the same pk_live_/sk_live_ values there."); + expect(output).toContain("- Also copy the other Clerk variables from your env file"); + expect(output).toContain("NEXT_PUBLIC_CLERK_SIGN_IN_URL"); + expect(output).toContain("writes only the two keys"); + }); + + test("ends with a real sign-up on the production domain", () => { + const output = nextStepsBody("app_123", "ins_456", "example.com", "verified"); + + expect(flat(output)).toContain( + "3. Redeploy your app, then sign up at https://example.com to confirm it works", + ); + }); + + test("does not send the user to a domain that is not verified yet", () => { + // After skipping DNS verification the domain doesn't resolve, so step 3 + // has to route through the wizard before the sign-up check. + const output = nextStepsBody("app_123", "ins_456", "example.com", "pending"); + + // "once the domain is verified", not "once your DNS records are added": + // the DNS may already be done with only the certificate outstanding. + expect(output).toContain("3. Run `clerk deploy` again once the domain is verified"); + expect(output).toContain("sign up at https://example.com to confirm it works"); + expect(output).not.toContain("3. Redeploy your app"); + }); }); -describe("pendingDnsRecords", () => { - const targets: CnameTarget[] = [ - { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, - { host: "accounts.example.com", value: "accounts.clerk.services", required: true }, +describe("productionSummary", () => { + test("only calls production ready once the domain is verified", () => { + const verified = stripAnsi(productionSummary("example.com", ["Google"], "verified").join("\n")); + const pending = stripAnsi(productionSummary("example.com", [], "pending").join("\n")); + + expect(verified).toContain("Production ready at https://example.com"); + expect(pending).toContain("Production instance created for https://example.com"); + expect(pending).toContain("Domain Not yet verified"); + expect(pending).not.toContain("DNS pending"); + expect(pending).not.toContain("Production ready"); + }); +}); + +describe("domainAssociationSummary", () => { + test("lists every record host the domain will need, including both DKIM hosts", () => { + // The confirmation screen runs before the instance exists, so this is a + // prediction from the domain alone. It must match what the create call + // returns, or the user commits without seeing the full list. + const output = domainAssociationSummary("example.com").join("\n"); + + expect(output).toContain("clerk.example.com"); + expect(output).toContain("accounts.example.com"); + expect(output).toContain("clkmail.example.com"); + expect(output).toContain("clk._domainkey.example.com"); + expect(output).toContain("clk2._domainkey.example.com"); + expect(productionDnsHosts("example.com")).toHaveLength(5); + }); + + test("lead sentence says records are coming, with no record count in it", () => { + // The lead now wraps onto several lines; read up to the first blank line. + const lines = domainAssociationSummary("example.com"); + const lead = flat(lines.slice(0, lines.indexOf(""))); + + expect(lead).toContain("Clerk will use these subdomains for"); + // Disclose the obligation before the one-way step without demanding + // action the user can't take yet. + expect(lead).toContain( + "You'll add DNS records for them after the instance is created. The exact list is printed once the instance exists:", + ); + expect(lead).not.toMatch(/\b(three|five|3|5)\b/); + }); + + test("labels the mail hosts plainly, without the 'Clerk handles it' parenthetical", () => { + // The lead says the user will add a record for each row; a label saying + // Clerk handles SPF/DKIM automatically would contradict it on this screen. + const output = domainAssociationSummary("example.com").join("\n"); + + expect(output).toContain("Email clkmail.example.com"); + expect(output).toContain("Email (DKIM) clk._domainkey.example.com"); + expect(output).toContain("Email (DKIM) clk2._domainkey.example.com"); + // Labels pad to one column so the hosts line up. + expect(output).toContain("Frontend API clerk.example.com"); + expect(output).toContain("Account portal accounts.example.com"); + // The server omits the Account portal record when the portal is disabled + // on the cloned instance, so the lead can't promise one record per row. + expect(flat(output)).toContain( + "You'll add DNS records for them after the instance is created. The exact list is printed once the instance exists:", + ); + expect(output).not.toContain("a DNS record for each"); + expect(output).not.toContain("Clerk handles SPF/DKIM"); + expect(output).not.toContain("CNAME clk._domainkey"); + }); +}); + +describe("dnsIntro", () => { + test("sets the propagation expectation as minutes, with 48 hours as the outlier", () => { + const output = dnsIntro("example.com").join("\n"); + + expect(flat(output)).toContain( + "usually propagate within minutes, but can occasionally take up to 48 hours", + ); + expect(output).not.toContain("It can take up to 48 hours"); + }); +}); + +describe("deployStatusPendingFooter", () => { + const DOMAINS_URL = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains"; + + test("reports missing DNS records as not found yet, not as a failure", () => { + const output = deployStatusPendingFooter( + "example.com", + { + dns: false, + ssl: false, + mail: false, + }, + DOMAINS_URL, + true, + ).join("\n"); + + expect(output).toContain("DNS and email DNS records not found yet for example.com."); + // A "Check again" prompt follows this footer, so it points there first and + // gives the resume command as the fallback. + expect(flat(output)).toContain( + "Add them at your DNS provider if you haven't already, then choose Check again below.", + ); + expect(flat(output)).toContain("skip for now and run `clerk deploy` later to resume"); + expect(flat(output)).toContain( + "usually takes minutes, but can occasionally take up to 48 hours", + ); + expect(flat(output)).toContain( + "change the domain in the Clerk Dashboard: https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + ); + expect(output).not.toContain("still pending"); + expect(output).not.toContain("SSL"); + }); + + test("names only the email records when the Frontend API records are verified", () => { + const output = deployStatusPendingFooter( + "example.com", + { + dns: true, + ssl: false, + mail: false, + }, + DOMAINS_URL, + true, + ).join("\n"); + + // Capitalized at the sentence start; lowercase "email DNS" is only right + // mid-sentence ("DNS and email DNS"). + expect(output).toContain("Email DNS records not found yet for example.com."); + expect(output).not.toContain("email DNS records not found"); + expect(output).not.toContain("DNS and email DNS"); + }); + + test("says when the record list is missing instead of telling the user to add nothing", () => { + // DNS unverified but the API returned no targets: "add them" would point + // at an empty list. One follow-up line, so blank line + sentence. + const output = deployStatusPendingFooter( + "example.com", + { dns: false, ssl: false, mail: false }, + DOMAINS_URL, + false, + ).join("\n"); + + expect(output).toContain("DNS and email DNS records not found yet for example.com.\n\n"); + expect(output).toContain("Clerk didn't return the list of records to add."); + // URL on its own line so terminal autolinkers don't swallow punctuation. + expect(flat(output)).toContain( + "Find them on the Domains page in the Clerk Dashboard, add them, then choose Check again below.", + ); + expect(output).toContain(`already created.\n ${DOMAINS_URL}`); + expect(output).toContain("run `clerk deploy` later to resume"); + expect(output).not.toContain(" - "); + expect(output).not.toContain("Add them at your DNS provider"); + }); + + test("tells the user to wait, not act, when only SSL is pending", () => { + // SSL is Clerk's side; there are no records the user could add. + const output = deployStatusPendingFooter( + "example.com", + { + dns: true, + ssl: false, + mail: true, + }, + DOMAINS_URL, + true, + ).join("\n"); + + expect(output).toContain("SSL certificate still pending for example.com."); + expect(flat(output)).toContain("choose Check again below in a few minutes"); + expect(flat(output)).toContain("run `clerk deploy` later to resume"); + // One follow-up line: a blank line and a sentence, not a one-item list. + expect(output).toContain("example.com.\n\nClerk issues it"); + expect(output).not.toContain(" - "); + expect(output).not.toContain("not found yet"); + expect(output).not.toContain("change the domain in the Clerk Dashboard"); + }); + + test("says Clerk is still finalizing when every component is verified", () => { + // The fallthrough branch: all three verified, server hasn't flipped the + // domain to complete yet. One follow-up line, so blank line + sentence. + const output = deployStatusPendingFooter( + "example.com", + { dns: true, ssl: true, mail: true }, + DOMAINS_URL, + false, + ).join("\n"); + + expect(output).toContain( + "Production setup for example.com is still finalizing on Clerk's side.\n\nRun `clerk deploy` again in a few minutes to resume.", + ); + expect(output).not.toContain("not found yet"); + expect(output).not.toContain("SSL"); + }); + + test.each([ + { label: "records pending", status: { dns: false, ssl: false, mail: false }, records: true }, + { label: "records missing", status: { dns: false, ssl: false, mail: false }, records: false }, + { label: "SSL only pending", status: { dns: true, ssl: false, mail: true }, records: false }, { - host: "clkmail.example.com", - value: "mail.example.com.nam1.clerk.services", - required: true, + label: "all components verified", + status: { dns: true, ssl: true, mail: true }, + records: false, }, + ])("always says how to resume and that re-running is safe ($label)", ({ status, records }) => { + const output = deployStatusPendingFooter("example.com", status, DOMAINS_URL, records).join( + "\n", + ); + expect(flat(output)).toMatch(/run `clerk deploy` (again|later).*to resume/i); + expect(flat(output)).toMatch(/production instance is already created\./i); + }); + + test("omits the Dashboard URL cleanly when no production instance id is known", () => { + const output = deployStatusPendingFooter( + "example.com", + { dns: false, ssl: false, mail: false }, + undefined, + true, + ).join("\n"); + expect(flat(output)).toContain("change the domain in the Clerk Dashboard."); + expect(output).not.toContain("undefined"); + }); +}); + +describe("pendingCnameTargets", () => { + const targets: CnameTarget[] = [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "clkmail.example.com", value: "mail.clerk.services", required: true }, + { host: "clk._domainkey.example.com", value: "dkim1.clerk.services", required: true }, ]; - test("returns no records when only SSL remains pending", () => { - expect(pendingDnsRecords(targets, { dns: true, ssl: false, mail: true })).toEqual([]); + test("returns only the mail targets when only email DNS is unverified", () => { + const pending = pendingCnameTargets(targets, { dns: true, ssl: true, mail: false }); + expect(pending.map((t) => t.host)).toEqual([ + "clkmail.example.com", + "clk._domainkey.example.com", + ]); }); - test("returns only email records when email DNS remains pending", () => { - const output = pendingDnsRecords(targets, { dns: true, ssl: true, mail: false }).join("\n"); + test("returns nothing when only SSL is pending", () => { + expect(pendingCnameTargets(targets, { dns: true, ssl: false, mail: true })).toEqual([]); + }); +}); - expect(output).toContain("clkmail.example.com"); - expect(output).not.toContain("clerk.example.com"); - expect(output).not.toContain("accounts.example.com"); +describe("dashboard URLs", () => { + test("instance URL is the instance root and the domains URL is nested under it", () => { + expect(instanceDashboardUrl("app_1", "ins_prod")).toBe( + "https://dashboard.clerk.com/apps/app_1/instances/ins_prod", + ); + expect(domainsDashboardUrl("app_1", "ins_prod")).toBe( + "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + ); }); +}); - test("returns non-email records when DNS remains pending", () => { - const output = pendingDnsRecords(targets, { dns: false, ssl: true, mail: true }).join("\n"); +describe("INTRO_PREAMBLE", () => { + test("rules out host-generated URLs as a production domain and says why", () => { + // Described rather than named: the rule covers every host, and the reject + // list at the prompt is where specific hosts belong. + expect(INTRO_PREAMBLE).toContain("The URL a hosting provider generated"); + expect(INTRO_PREAMBLE).toContain("for your deployment won't work here."); + expect(INTRO_PREAMBLE).not.toMatch(/railway|vercel|netlify/i); + // Subdomains you control are fine; the old "development subdomain" line + // read as if they weren't. + expect(INTRO_PREAMBLE).toContain("app.example.com"); + expect(INTRO_PREAMBLE).not.toContain("development subdomain"); + }); +}); - expect(output).toContain("clerk.example.com"); - expect(output).toContain("accounts.example.com"); - expect(output).not.toContain("clkmail.example.com"); +describe("DEPLOY_COMMAND_DESCRIPTION", () => { + test("describes the bare command and the agent-mode report", () => { + expect(DEPLOY_COMMAND_DESCRIPTION.startsWith(`${DEPLOY_COMMAND_SUMMARY}.`)).toBe(true); + expect(DEPLOY_COMMAND_DESCRIPTION).toContain("with no subcommand starts an interactive setup"); + expect(DEPLOY_COMMAND_DESCRIPTION).toContain("creates the production instance"); + expect(DEPLOY_COMMAND_DESCRIPTION).toContain("When run by an agent"); + expect(DEPLOY_COMMAND_DESCRIPTION).toContain("`nextAction`"); }); }); @@ -158,7 +472,273 @@ describe("dnsRecords", () => { { host: "clk2._domainkey.example.com", value: "dkim2.clerk.services", required: true }, ]).join("\n"); - expect(output).toContain("Email (Clerk handles SPF/DKIM automatically)"); + expect(output).toContain("Email (DKIM)"); expect(output).not.toContain("\n CNAME\n Type:"); + // Said once under the block, never on a row the user must act on. + expect(flat(output)).toContain( + "The email records point at Clerk, so you don't need to create SPF or DKIM values yourself.", + ); + expect(output).not.toMatch(/Email \(DKIM\).*Clerk handles/); + }); + + test("omits the SPF/DKIM sentence when no email record is listed", () => { + // On resume with email DNS already verified, only the Frontend API + // record is outstanding; a sentence about "the email records" under a + // list with none reads as if rows are missing. + const output = dnsRecords( + [{ host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }], + { afterCheck: true }, + ).join("\n"); + + expect(output).not.toContain("SPF or DKIM"); + expect(output).toContain("Frontend API"); + expect(output).toContain('set them to "DNS only"'); + }); + + test("labels the mail host and both DKIM hosts the same way the confirmation screen does", () => { + // One label set across screens: a host named two ways reads as two records. + const records = dnsRecords([ + { host: "clkmail.example.com", value: "mail.clerk.services", required: true }, + { host: "clk._domainkey.example.com", value: "dkim1.clerk.services", required: true }, + ]).join("\n"); + const confirmation = stripAnsi(domainAssociationSummary("example.com").join("\n")); + + for (const label of ["Email", "Email (DKIM)"]) { + expect(records).toContain(label); + expect(confirmation).toContain(label); + } + expect(records).not.toContain("Clerk handles SPF/DKIM automatically"); + expect(confirmation).not.toContain("Clerk handles SPF/DKIM automatically"); + }); +}); + +describe("dnsHandoffNothingToAdd", () => { + // The DNS screen when the list of records to add is empty. It must say what + // is actually outstanding rather than framing a records task with no + // records, and each state has its own action. + const URL = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains"; + + test("SSL pending: records are done, the certificate is Clerk's side, check again is offered", () => { + const out = stripAnsi( + dnsHandoffNothingToAdd("example.com", { dns: true, ssl: false, mail: true }, URL, { + oauthNext: false, + }).join("\n"), + ); + + expect(out).toContain("Your DNS records for example.com are verified."); + expect(flat(out)).toContain( + "The SSL certificate is still pending; Clerk issues it automatically.", + ); + expect(out).toContain(`Clerk Dashboard:\n ${URL}`); + expect(out).toContain("checks whether the certificate has been issued"); + expect(flat(out)).toContain( + "If it hasn't yet, you can either wait a few minutes and check again", + ); + // No timing promise the status can't back up. + expect(out).not.toContain("usually takes"); + expect(out).not.toContain("Configure DNS"); + expect(out).not.toContain("these records"); + }); + + test("finalizing: nothing for the user to do, and no in-session retry is promised", () => { + const out = stripAnsi( + dnsHandoffNothingToAdd("example.com", { dns: true, ssl: true, mail: true }, URL, { + oauthNext: false, + }).join("\n"), + ); + + expect(out).toContain("Your DNS records and SSL certificate for example.com are verified."); + expect(flat(out)).toContain("Clerk is still finalizing production setup."); + // The check pauses the run once everything is verified, so "check again" + // would name an option the prompt never offers. + expect(flat(out)).toContain("run `clerk deploy` again in a few minutes"); + expect(out).not.toContain("check again"); + }); + + test.each([ + { + label: "both", + status: { dns: false, ssl: false, mail: false }, + records: "DNS and email DNS", + }, + { label: "email only", status: { dns: true, ssl: false, mail: false }, records: "Email DNS" }, + { label: "DNS only", status: { dns: false, ssl: false, mail: true }, records: "DNS" }, + ])( + "no record list ($label): tells the user to find and add the records", + ({ status, records }) => { + const out = stripAnsi( + dnsHandoffNothingToAdd("example.com", status, URL, { oauthNext: false }).join("\n"), + ); + + expect(flat(out)).toContain( + `${records} records for example.com are not verified yet, but Clerk didn't return the list to add.`, + ); + // An instruction, not a wait: the records still have to be added. + expect(flat(out)).toContain("add them at your DNS provider, then choose Check DNS now below"); + expect(out).toContain(`Check DNS now below:\n ${URL}`); + expect(out).toContain("checks that they have taken effect"); + // Plural subject: the sentence is about records, not a certificate. + expect(flat(out)).toContain("If they haven't yet, you can either wait a few minutes"); + expect(out).not.toContain("If it hasn't"); + expect(out).not.toContain("Configure DNS"); + }, + ); + + test("no record list on a first run with providers: OAuth comes before the check", () => { + // The prompt after this screen is OAuth setup, not the DNS check, so the + // screen must not point at a "Check DNS now" that isn't there. + const out = stripAnsi( + dnsHandoffNothingToAdd("example.com", { dns: false, ssl: false, mail: false }, URL, { + oauthNext: true, + }).join("\n"), + ); + + expect(flat(out)).toContain( + "Find them on the Domains page in the Clerk Dashboard and add them at your DNS provider:", + ); + expect(flat(out)).toContain( + "Next you'll set up OAuth, then this command checks that they have taken effect.", + ); + expect(out).not.toContain("Check DNS now"); + }); + + test("ends sentences cleanly with no Dashboard URL", () => { + const out = stripAnsi( + dnsHandoffNothingToAdd("example.com", { dns: true, ssl: false, mail: true }, undefined, { + oauthNext: false, + }).join("\n"), + ); + expect(out).toContain("on the Domains page in the Clerk Dashboard."); + expect(out).not.toContain("undefined"); + }); +}); + +describe("dnsDashboardHandoff", () => { + const DOMAINS_URL = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains"; + + test("links the Domains page and says how to resume if the check is skipped", () => { + const output = dnsDashboardHandoff("example.com", DOMAINS_URL, { oauthNext: true }).join("\n"); + + expect(output).toContain(`Clerk Dashboard:\n ${DOMAINS_URL}`); + // "wizard" appears nowhere else the user can see, so it isn't introduced here. + expect(output).not.toContain("wizard"); + expect(flat(output)).toContain( + "Next you'll set up OAuth, then this command checks that these records have taken effect at your DNS provider.", + ); + // A failed check is not a dead end: "Check again" is the other choice on + // the prompt that follows, and the sentence names it. + expect(flat(output)).toContain( + "you can either wait a few minutes and check again, or skip the check", + ); + expect(flat(output)).toContain("run `clerk deploy` again later to finish"); + // "skip and finish" read as though skipping completed the deploy. + expect(output).not.toContain("skip and finish"); + }); + + test("does not promise an OAuth step when none is coming", () => { + // On resume OAuth already ran, and a fresh run with no providers skips it. + // Under a checklist showing OAuth done, "you'll set up OAuth" was wrong. + const output = dnsDashboardHandoff("example.com", DOMAINS_URL, { oauthNext: false }).join("\n"); + + expect(flat(output)).toContain( + "Next, this command checks that these records have taken effect at your DNS provider.", + ); + expect(output).not.toContain("set up OAuth"); + expect(output).toContain("run `clerk deploy` again later to finish"); + }); + + test("ends the sentence cleanly when no Dashboard URL is known", () => { + const output = dnsDashboardHandoff("example.com", undefined, { oauthNext: true }).join("\n"); + + expect(flat(output)).toContain("on the Domains page in the Clerk Dashboard."); + expect(output).not.toContain("undefined"); + }); +}); + +describe("every wizard screen fits inside the frame", () => { + // The wizard prefixes each printed line with a 3-column gutter, added per + // line the code emits; the terminal's own soft wrap lands outside it. So + // every line a screen emits must fit 76 visible columns. The one allowed + // overflow is a lone unbreakable token, optionally after a "Label:" (a URL + // on its own line, or "Reference: "); a sentence that merely contains + // a URL still has to wrap. + const DOMAIN = "auth.my-long-company-name.co.uk"; + const URL = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains"; + const STATUSES = [ + { dns: false, ssl: false, mail: false }, + { dns: true, ssl: false, mail: false }, + { dns: false, ssl: false, mail: true }, + { dns: true, ssl: false, mail: true }, + { dns: true, ssl: true, mail: true }, + ]; + const targets: CnameTarget[] = [ + { host: `clerk.${DOMAIN}`, value: "frontend-api.clerk.services", required: true }, + { host: `accounts.${DOMAIN}`, value: "accounts.clerk.services", required: false }, + { host: `clk2._domainkey.${DOMAIN}`, value: "dkim2.clerk.services", required: true }, + ]; + const screens: Record = { + INTRO_PREAMBLE: [INTRO_PREAMBLE], + OAUTH_SECTION_INTRO: [OAUTH_SECTION_INTRO], + dnsIntro: dnsIntro(DOMAIN), + domainAssociationSummary: domainAssociationSummary(DOMAIN), + dnsRecords: dnsRecords(targets), + "dnsRecords afterCheck": dnsRecords(targets, { afterCheck: true }), + "productionSummary verified": productionSummary(DOMAIN, ["Google"], "verified"), + "productionSummary pending": productionSummary(DOMAIN, [], "pending"), + "nextStepsBody verified": [nextStepsBody("app_1", "ins_prod", DOMAIN, "verified")], + "nextStepsBody pending": [nextStepsBody("app_1", "ins_prod", DOMAIN, "pending")], + }; + for (const oauthNext of [true, false]) { + screens[`dnsDashboardHandoff oauthNext=${oauthNext}`] = dnsDashboardHandoff(DOMAIN, URL, { + oauthNext, + }); + for (const status of STATUSES) { + const key = JSON.stringify(status); + screens[`dnsHandoffNothingToAdd ${key} oauthNext=${oauthNext}`] = dnsHandoffNothingToAdd( + DOMAIN, + status, + URL, + { oauthNext }, + ); + } + } + for (const status of STATUSES) { + for (const hasRecords of [true, false]) { + screens[`deployStatusPendingFooter ${JSON.stringify(status)} records=${hasRecords}`] = + deployStatusPendingFooter(DOMAIN, status, URL, hasRecords); + } + } + + const loneToken = /^\s*(\S+:\s+)?\S+$/; + // Deliberately not imported from wrap.ts: this asserts "fits an 80-column + // terminal", and sharing the constant would let a wider default pass the + // test while breaking the frame. + const FRAME_WIDTH = 76; + + test.each(Object.entries(screens).map(([name, lines]) => ({ name, lines })))( + "$name", + ({ lines }) => { + const tooWide = lines + .join("\n") + .split("\n") + .map((line) => stripAnsi(line)) + .filter((line) => line.length > FRAME_WIDTH && !loneToken.test(line)); + expect(tooWide).toEqual([]); + }, + ); + + test("step 3 keeps its numbered indent across wrapped lines", () => { + for (const status of ["verified", "pending"] as const) { + const lines = nextStepsBody("app_1", "ins_prod", DOMAIN, status).split("\n"); + const start = lines.findIndex((line) => line.startsWith(" 3. ")); + expect(start).toBeGreaterThan(0); + // Continuation lines align under the text after "3. ", like steps 1 and 2. + let next = start + 1; + while (lines[next] !== "") { + expect(lines[next]).toMatch(/^ {5}\S/); + next++; + } + expect(next).toBeGreaterThan(start + 1); + } }); }); diff --git a/packages/cli-core/src/commands/deploy/copy.ts b/packages/cli-core/src/commands/deploy/copy.ts index 3b8306685..f1004a542 100644 --- a/packages/cli-core/src/commands/deploy/copy.ts +++ b/packages/cli-core/src/commands/deploy/copy.ts @@ -1,18 +1,40 @@ import { bold, cyan, dim, green, yellow } from "../../lib/color.ts"; import type { CnameTarget } from "../../lib/plapi.ts"; +import { buildDashboardUrl } from "../../lib/environment.ts"; +import { wrap } from "../../lib/wrap.ts"; export type DeployPlanStep = { label: string; status: "done" | "pending"; }; +export const DEPLOY_COMMAND_SUMMARY = "Deploy a Clerk application to production"; + +/** + * Long description for `clerk deploy --help`. The wizard is registered as a + * hidden default subcommand, so without this prose the help lists only + * `status` and reads as if the CLI can only watch a deploy, not perform one. + */ +export const DEPLOY_COMMAND_DESCRIPTION = `${DEPLOY_COMMAND_SUMMARY}. + +Running \`clerk deploy\` with no subcommand starts an interactive setup that +creates the production instance, prints the DNS records you must add, collects +production OAuth credentials, and verifies the domain. It needs a terminal; +re-run it at any time to resume where you left off. + +When run by an agent (or without a TTY), it is read-only: it prints a JSON +status report with the current state and a \`nextAction\` field saying what to +do next. \`clerk deploy status\` prints the same report; add \`--wait\` to keep +checking until DNS, SSL, and email DNS are verified.`; + export const INTRO_PREAMBLE = `This will prepare your linked Clerk app for production by cloning your development instance into a new production instance and walking you through the setup the dashboard would otherwise guide you through. Before you begin you will need: - - A domain you own (production cannot use a development subdomain). - - The ability to add DNS records on that domain. + - A domain you own where you can add DNS records (example.com, or a + subdomain like app.example.com). The URL a hosting provider generated + for your deployment won't work here. - OAuth credentials for any social providers you have enabled in dev. ${dim("Reference: https://clerk.com/docs/guides/development/deployment/production")}`; @@ -37,7 +59,10 @@ export function dnsIntro(domain: string): string[] { "Clerk uses DNS records to provide session management and emails", "verified from your domain.", "", - `${yellow("NOTE")} It can take up to 48 hours for DNS records to fully propagate.`, + ...wrap( + `${yellow("NOTE")} DNS records usually propagate within minutes, but can occasionally take up to 48 hours.`, + { hang: 6 }, + ), `${dim(cyan("TIP"))} If you can't add a CNAME for the Frontend API, you can use a proxy:`, dim(" https://clerk.com/docs/guides/dashboard/dns-domains/proxy-fapi"), dim("Reference: https://clerk.com/docs/guides/development/deployment/production#dns-records"), @@ -56,20 +81,58 @@ export function clerkSubdomains(domain: string): { }; } -export function domainAssociationSummary(domain: string): string[] { +/** + * Every record host a new production domain needs, derived from the domain + * alone so the confirmation screen can show the full list before the instance + * exists. The DKIM selector is fixed server-side for domains the CLI creates + * (`clk`, so `clk._domainkey` and `clk2._domainkey`); the real targets, with + * their per-instance values, come back from the create call afterwards. + */ +export function productionDnsHosts(domain: string): string[] { const { frontendApi, accountPortal, mail } = clerkSubdomains(domain); - const hosts = [frontendApi, accountPortal, mail]; return [ - `Clerk will associate these subdomains with ${cyan(domain)}:`, + frontendApi, + accountPortal, + mail, + `clk._domainkey.${domain}`, + `clk2._domainkey.${domain}`, + ]; +} + +export function domainAssociationSummary(domain: string): string[] { + const hosts = productionDnsHosts(domain); + const labels = hosts.map((host) => cnameTargetLabel(host)); + const width = Math.max(...labels.map((label) => label.length)); + return [ + // Disclose the obligation before the one-way create step, without asking + // for action the user can't take yet (record values arrive after creation). + // "The exact list": the server omits the Account portal record when the + // portal is disabled on the instance being cloned, and this screen runs + // before the CLI can know that. + ...wrap( + `Clerk will use these subdomains for ${cyan(domain)}. You'll add DNS records for them after the instance is created. The exact list is printed once the instance exists:`, + ), "", - ...hosts.map((host) => ` ${cnameTargetLabel(host)} ${host}`), + ...hosts.map((host, i) => ` ${labels[i]!.padEnd(width)} ${host}`), "", "This will create a Clerk production instance for your application.", ]; } -export function dnsRecords(targets: readonly CnameTarget[]): string[] { - const lines = ["Add the following records at your DNS provider:"]; +/** + * `afterCheck`: the records are being shown again because a DNS check didn't + * find them. The user may already have added them and be waiting on + * propagation, so the heading hedges. The first hand-over doesn't. + */ +export function dnsRecords( + targets: readonly CnameTarget[], + options: { afterCheck?: boolean } = {}, +): string[] { + const lines = [ + options.afterCheck + ? "Add the following records at your DNS provider if you haven't already:" + : "Add the following records at your DNS provider:", + ]; for (const target of targets) { const label = cnameTargetLabel(target.host); const optional = target.required ? "" : ` ${dim("(optional)")}`; @@ -81,20 +144,39 @@ export function dnsRecords(targets: readonly CnameTarget[]): string[] { ` Value: ${target.value}`, ); } + if (targets.some(isMailCnameTarget)) { + // These are CNAMEs pointing at Clerk, so the user never generates or + // rotates key material and never hand-writes an SPF record. Said once + // here rather than on each email row, and only when an email row is on + // screen: a resume where email DNS is already verified lists none. + lines.push( + "", + ...wrap( + "The email records point at Clerk, so you don't need to create SPF or DKIM values yourself.", + ), + ); + } lines.push( "", - `${yellow("NOTE")} If your DNS host proxies these records, set them to "DNS only" or verification will fail.`, + ...wrap( + `${yellow("NOTE")} If your DNS host proxies these records, set them to "DNS only" or verification will fail.`, + { hang: 6 }, + ), ); return lines; } -export function pendingDnsRecords( +/** + * The targets the user still has to add, given what the domain check found. + * The single source of "are there records to show" for both the wizard and + * the agent report; formatters and classifiers derive from this, never from + * each other. + */ +export function pendingCnameTargets( targets: readonly CnameTarget[], status: DeployComponentStatus, -): string[] { - const pendingTargets = targets.filter((target) => cnameTargetPending(target, status)); - if (pendingTargets.length === 0) return []; - return dnsRecords(pendingTargets); +): CnameTarget[] { + return targets.filter((target) => cnameTargetPending(target, status)); } export function cnameTargetPending(target: CnameTarget, status: DeployComponentStatus): boolean { @@ -102,33 +184,181 @@ export function cnameTargetPending(target: CnameTarget, status: DeployComponentS return !status.dns; } +/** + * What each record host is, keyed by its first label (`clk._domainkey` and + * `clk2._domainkey` arrive as "clk"/"clk2"). One table drives both how a + * record is classified (email DNS vs DNS, for filtering and for the SPF/DKIM + * sentence) and how it is labelled on screen, so a new host can't be + * classified as email and still print as an unlabelled "CNAME" row. + * + * "DKIM" stays in the label: it is the standard name for these records and + * already appears in the host. What Clerk manages on the user's behalf is + * said once under the block instead of on each row, where it read as + * "nothing to do here". + * + * `productionDnsHosts` keeps its own list of the DKIM hosts: it answers + * "which records will this domain need" before any exist, not "what is this + * record", so the selector assumption documented there still lives there. + */ +const CNAME_HOSTS = new Map([ + ["clerk", { label: "Frontend API", mail: false }], + ["accounts", { label: "Account portal", mail: false }], + ["clkmail", { label: "Email", mail: true }], + ["clk", { label: "Email (DKIM)", mail: true }], + ["clk2", { label: "Email (DKIM)", mail: true }], +]); + +function cnameHostInfo(host: string): { label: string; mail: boolean } | undefined { + return CNAME_HOSTS.get(host.split(".", 1)[0] ?? ""); +} + function isMailCnameTarget(target: CnameTarget): boolean { - const prefix = target.host.split(".", 1)[0]; - return prefix === "clkmail" || prefix === "clk" || prefix === "clk2"; + return cnameHostInfo(target.host)?.mail ?? false; } +/** + * Human label for a record host, used by every screen that lists records so + * the confirmation screen and the records block can't name the same host two + * ways. + */ function cnameTargetLabel(host: string): string { - const prefix = host.split(".", 1)[0]; - switch (prefix) { - case "clerk": - return "Frontend API"; - case "accounts": - return "Account portal"; - // `host.split(".", 1)[0]` yields only the first label, so DKIM records - // (clk._domainkey, clk2._domainkey) arrive here as "clk"/"clk2". - case "clkmail": - case "clk": - case "clk2": - return "Email (Clerk handles SPF/DKIM automatically)"; - default: - return "CNAME"; + return cnameHostInfo(host)?.label ?? "CNAME"; +} + +/** + * The "what happens next" sentence both DNS screens end with. One place for + * the OAuth-first phrasing so the records-present and no-records branches + * can't drift: the first time they were composed separately, the no-records + * branch told the user to "check now" on a run where OAuth setup came first. + * `resume` differs by state (finalizing pauses the run rather than offering + * a retry), so the caller supplies it. + */ +function nextStepSentence(options: { oauthNext: boolean; check: string; resume: string }): string { + const lead = options.oauthNext + ? "Next you'll set up OAuth, then this command " + : "Next, this command "; + return `${lead}${options.check}. ${options.resume}`; +} + +/** + * The DNS screen when there is nothing left to add. Printed instead of + * `dnsIntro` + `dnsDashboardHandoff`: a "Configure DNS" page with an empty + * record list told the user to do work they had already done. What is + * outstanding comes from the same classifier the post-check footer uses, so + * the screen before the check and the footer after it can't disagree. + * `hasPendingRecords` is false by construction here (the display list is + * empty), so `records_available` is unreachable. + */ +export function dnsHandoffNothingToAdd( + domain: string, + status: DeployComponentStatus, + domainsUrl: string | undefined, + options: { oauthNext: boolean }, +): string[] { + const state = classifyDomainPending(status, false); + const url = domainsUrl ? [` ${domainsUrl}`] : []; + // Written out per subject rather than patched by string replacement, so a + // reword of one can't leave the other reading "it" about records. + const resume = (subject: "it hasn't" | "they haven't") => + `If ${subject} yet, you can either wait a few minutes and check again, or skip the check and run \`clerk deploy\` again later to finish.`; + switch (state) { + case "ssl_pending": + return [ + ...wrap( + `Your DNS records for ${cyan(domain)} are verified. The SSL certificate is still pending; Clerk issues it automatically.`, + ), + "", + `Monitor SSL issuance on the Domains page in the Clerk Dashboard${domainsUrl ? ":" : "."}`, + ...url, + "", + ...wrap( + nextStepSentence({ + ...options, + check: "checks whether the certificate has been issued", + resume: resume("it hasn't"), + }), + ), + ]; + case "finalizing": + // No "check again": once every component is verified, the check pauses + // the run instead of prompting. + return [ + ...wrap( + `Your DNS records and SSL certificate for ${cyan(domain)} are verified. Clerk is still finalizing production setup.`, + ), + "", + `Monitor it on the Domains page in the Clerk Dashboard${domainsUrl ? ":" : "."}`, + ...url, + "", + ...wrap( + nextStepSentence({ + ...options, + check: "checks whether Clerk has finished", + resume: "If it hasn't, run `clerk deploy` again in a few minutes.", + }), + ), + ]; + case "records_available": + case "records_unavailable": { + // Records are needed but Clerk returned no list: the user has to find + // and add them, so this is an instruction, not a wait. "Then choose + // Check DNS now below" only when that prompt really is next; on a fresh + // run with providers, OAuth setup comes first. + const records = capitalizeFirst(pendingRecordComponents(status)); + const find = options.oauthNext + ? `Find them on the Domains page in the Clerk Dashboard and add them at your DNS provider${domainsUrl ? ":" : "."}` + : `Find them on the Domains page in the Clerk Dashboard, add them at your DNS provider, then choose Check DNS now below${domainsUrl ? ":" : "."}`; + return [ + ...wrap( + `${records} records for ${cyan(domain)} are not verified yet, but Clerk didn't return the list to add.`, + ), + "", + ...wrap(find), + ...url, + "", + ...wrap( + nextStepSentence({ + ...options, + check: "checks that they have taken effect", + resume: resume("they haven't"), + }), + ), + ]; + } } } -export function dnsDashboardHandoff(domain: string): string[] { +/** + * `oauthNext` is required: on a fresh run with providers, OAuth setup comes + * between this screen and the DNS check; on resume (OAuth already done) and + * on a fresh run with no providers, the check is next. Saying "you'll set up + * OAuth" under a checklist that shows OAuth done was wrong. + */ +export function dnsDashboardHandoff( + domain: string, + domainsUrl: string | undefined, + options: { oauthNext: boolean }, +): string[] { return [ - `Check the Domains section in the Clerk Dashboard for ${domain} to monitor DNS propagation and SSL issuance.`, - "After OAuth setup, you can verify DNS or skip and finish. DNS propagation can take time.", + // "this command", not "the wizard": nothing the user sees uses that word. + // Skipping the check leaves setup unfinished, so name what resumes it. + ...wrap( + `Monitor DNS propagation and SSL issuance for ${domain} on the Domains page in the Clerk Dashboard${domainsUrl ? ":" : "."}`, + ), + ...(domainsUrl ? [` ${domainsUrl}`] : []), + "", + // "at your DNS provider" matches the records block's own heading, and "at" + // rather than "with": the check looks the records up, it doesn't contact + // the provider. Naming both options matters because a failed check isn't a + // dead end — "Check again" is the other choice on the prompt that follows. + ...wrap( + nextStepSentence({ + oauthNext: options.oauthNext, + check: "checks that these records have taken effect at your DNS provider", + resume: + "If they haven't yet, you can either wait a few minutes and check again, or skip the check and run `clerk deploy` again later to finish.", + }), + ), ]; } @@ -191,21 +421,94 @@ export function deployStatusRetryMessage( * components are complete. The user keeps the deploy state; rerunning * `clerk deploy` resumes from whichever component is still pending. */ -export function deployStatusPendingFooter(domain: string, status: DeployComponentStatus): string[] { - const pending: string[] = []; - if (!status.dns) pending.push("DNS"); - if (!status.ssl) pending.push("SSL"); - if (!status.mail) pending.push("email DNS"); +/** + * What is actually outstanding after a domain check, from the user's point of + * view. Records are theirs to add; SSL and final readiness are Clerk's side. + * `records_unavailable` is the case where DNS is unverified but the API gave + * us no record list to show — telling the user to "add the records" then + * points at nothing. Shared by the wizard footer and the agent `nextAction` + * so the two surfaces can't disagree. + */ +export type DomainPendingState = + | "records_available" + | "records_unavailable" + | "ssl_pending" + | "finalizing"; - const lead = - pending.length === 0 - ? `Production setup for ${domain} is still finalizing.` - : `${pending.join(", ")} still pending for ${domain}.`; +export function classifyDomainPending( + status: DeployComponentStatus, + hasPendingRecords: boolean, +): DomainPendingState { + if (!status.dns || !status.mail) { + return hasPendingRecords ? "records_available" : "records_unavailable"; + } + if (!status.ssl) return "ssl_pending"; + return "finalizing"; +} + +/** "DNS", "email DNS", or "DNS and email DNS" — whichever records are unverified. */ +export function pendingRecordComponents(status: DeployComponentStatus): string { + const records: string[] = []; + if (!status.dns) records.push("DNS"); + if (!status.mail) records.push("email DNS"); + return records.join(" and "); +} +export function deployStatusPendingFooter( + domain: string, + status: DeployComponentStatus, + domainsUrl: string | undefined, + hasPendingRecords: boolean, +): string[] { + const state = classifyDomainPending(status, hasPendingRecords); + const records = capitalizeFirst(pendingRecordComponents(status)); + + // A lead line, then either a bulleted list (several follow-ups) or a blank + // line and one sentence (a single follow-up). An empty string is a blank + // line; the caller renders it with `log.blank()`. + // The wizard is still running when this prints: a "Check again" prompt + // follows, so the resume command is the fallback, not the instruction. + const resume = + "You can also skip for now and run `clerk deploy` later to resume; the production instance is already created."; + if (state === "records_available") { + return [ + ...wrap(`${records} records not found yet for ${domain}.`), + ...wrap( + ` - Add them at your DNS provider if you haven't already, then choose Check again below. ${resume}`, + ), + ...wrap(" - Propagation usually takes minutes, but can occasionally take up to 48 hours."), + ...wrap( + ` - If you can't add DNS records for this domain, change the domain in the Clerk Dashboard${domainsUrl ? `: ${domainsUrl}` : "."}`, + ), + ]; + } + if (state === "records_unavailable") { + // URL on its own line: mid-sentence, terminal autolinkers swallow the + // trailing punctuation. + return [ + ...wrap(`${records} records not found yet for ${domain}.`), + "", + ...wrap( + `Clerk didn't return the list of records to add. Find them on the Domains page in the Clerk Dashboard, add them, then choose Check again below. ${resume}`, + ), + ...(domainsUrl ? [` ${domainsUrl}`] : []), + ]; + } + if (state === "ssl_pending") { + return [ + ...wrap(`SSL certificate still pending for ${domain}.`), + "", + ...wrap( + `Clerk issues it automatically now that DNS is verified; choose Check again below in a few minutes. ${resume}`, + ), + ]; + } return [ - lead, - "DNS propagation can take several hours depending on your provider.", - "Run `clerk deploy` again to resume. The production instance is already created.", + ...wrap(`Production setup for ${domain} is still finalizing on Clerk's side.`), + "", + ...wrap( + "Run `clerk deploy` again in a few minutes to resume. The production instance is already created.", + ), ]; } @@ -217,36 +520,59 @@ each enabled provider. ${dim("Reference: https://clerk.com/docs/guides/configure/auth-strategies/social-connections/overview")}`; +// `domainStatus` is required on both closing-screen functions: it selects the +// headline and step 3, and a caller that forgot it would print "Production +// ready" over a domain that doesn't resolve yet. export function productionSummary( domain: string, completedOAuthProviderLabels: readonly string[], - domainStatus: "verified" | "pending" = "verified", + domainStatus: "verified" | "pending", ): string[] { return [ - `Production ready at ${cyan(`https://${domain}`)}`, + domainStatus === "verified" + ? `Production ready at ${cyan(`https://${domain}`)}` + : `Production instance created for ${cyan(`https://${domain}`)}`, "", - ` Domain ${domainStatus === "verified" ? "Verified" : "DNS pending"}`, + // "Not yet verified", not "DNS pending": the DNS may be done and only + // the certificate or Clerk's own setup outstanding; the screen above + // said which. + ` Domain ${domainStatus === "verified" ? "Verified" : "Not yet verified"}`, ` OAuth ${completedOAuthProviderLabels.length ? completedOAuthProviderLabels.join(", ") : "Not applicable"}`, ]; } -export function nextStepsBlock(appId: string, productionInstanceId: string): string { - return `${bold("Next steps")}\n${nextStepsBody(appId, productionInstanceId)}`; -} - -export function nextStepsBody(appId: string, productionInstanceId: string): string { +export function nextStepsBody( + appId: string, + productionInstanceId: string, + domain: string, + domainStatus: "verified" | "pending", +): string { + // Until DNS is verified the domain doesn't resolve, so "sign up there" + // would send the user to a page that doesn't exist yet. + // Wrapped here, not by hand: the domain's length moves the break. + const step3 = wrap( + domainStatus === "verified" + ? ` 3. Redeploy your app, then sign up at https://${domain} to confirm it works` + : ` 3. Run \`clerk deploy\` again once the domain is verified, then redeploy your app and sign up at https://${domain} to confirm it works`, + ).join("\n"); + const keysNote = wrap( + `${yellow("NOTE")} Production keys only work on your production domain. They will not work on localhost. To run your dev environment, keep using your dev keys.`, + { hang: 6 }, + ).join("\n"); return ` 1. Pull production keys into your environment clerk env pull --instance prod - This writes pk_live_... and sk_live_... to your .env. They replace your - pk_test_... and sk_test_... keys. + This writes pk_live_... and sk_live_... to your env file. They + replace your pk_test_... and sk_test_... keys. 2. Update env vars on your hosting provider Vercel, AWS, GCP, Heroku, Render, etc. all expose env vars in their UI. - Add the same pk_live_/sk_live_ values there. + - Add the same pk_live_/sk_live_ values there. + - Also copy the other Clerk variables from your env file, such as + NEXT_PUBLIC_CLERK_SIGN_IN_URL. \`env pull\` writes only the two keys. - 3. Redeploy your app +${step3} 4. (If applicable) Update webhook URLs and signing secrets ${dim("https://clerk.com/docs/guides/development/webhooks/syncing#configure-your-production-instance")} @@ -254,17 +580,32 @@ export function nextStepsBody(appId: string, productionInstanceId: string): stri 5. (If applicable) Update your Content Security Policy ${dim("https://clerk.com/docs/guides/secure/best-practices/csp-headers")} - 6. View and manage domain configuration in the Clerk Dashboard - ${dim(domainsDashboardUrl(appId, productionInstanceId))} + 6. Manage this instance in the Clerk Dashboard + - Users, settings, and billing: + ${dim(instanceDashboardUrl(appId, productionInstanceId))} + - DNS and SSL status: + ${dim(domainsDashboardUrl(appId, productionInstanceId))} -${yellow("NOTE")} Production keys only work on your production domain. They will not work on localhost. - To run your dev environment, keep using your dev keys. +${keysNote} ${dim("Reference: https://clerk.com/docs/guides/development/deployment/production#api-keys-and-environment-variables")}`; } +/** + * Component labels are lowercase so they read naturally mid-sentence ("DNS + * and email DNS"); when one of them opens a sentence it needs a capital. + */ +export function capitalizeFirst(value: string): string { + return value.charAt(0).toUpperCase() + value.slice(1); +} + +/** Dashboard home for one instance: users, settings, billing. */ +export function instanceDashboardUrl(appId: string, instanceId: string): string { + return buildDashboardUrl(appId, instanceId); +} + export function domainsDashboardUrl(appId: string, productionInstanceId: string): string { - return `https://dashboard.clerk.com/apps/${appId}/instances/${productionInstanceId}/domains`; + return buildDashboardUrl(appId, productionInstanceId, "domains"); } export function pausedMessage(stepDescription: string): string { diff --git a/packages/cli-core/src/commands/deploy/index.test.ts b/packages/cli-core/src/commands/deploy/index.test.ts index 4ccd8c9d6..297907195 100644 --- a/packages/cli-core/src/commands/deploy/index.test.ts +++ b/packages/cli-core/src/commands/deploy/index.test.ts @@ -70,13 +70,21 @@ mock.module("../../lib/open.ts", () => ({ const { _setConfigDir, readConfig, setProfile } = await import("../../lib/config.ts"); const { deploy } = await import("./index.ts"); -const { providerSetupIntro } = await import("./providers.ts"); +const { providerSetupIntro, showOAuthWalkthrough } = await import("./providers.ts"); const { collectCustomDomain } = await import("./prompts.ts"); function stripAnsi(value: string): string { return value.replace(new RegExp(`${String.fromCharCode(27)}\\[[0-9;]*m`, "g"), ""); } +/** + * Captured wizard output with the gutter and wrap continuations folded back + * into sentences, for asserting on what a line says rather than how it wraps. + */ +function flat(value: string): string { + return stripAnsi(value).replace(/\n│?[ \t]*/g, " "); +} + /** What the clack prompt wrappers throw when the user presses Ctrl-C. */ function promptExitError(): Error { return new UserAbortError(); @@ -697,6 +705,7 @@ describe("deploy", () => { expect(mockGetApplicationDomainStatus.mock.calls.length).toBeGreaterThanOrEqual(2); expect(err).toContain("DNS verified for example.com"); expect(err).toContain("Production ready at https://example.com"); + expect(err).toMatch(/└\s+Success/); }); test("DNS verification triggers a fresh DNS check before polling status", async () => { @@ -882,7 +891,9 @@ describe("deploy", () => { expect(error?.message).toContain("Deploy paused at: DNS verification"); expect(error?.exitCode).toBe(EXIT_CODE.GENERAL); - expect(err).toContain("Production setup for example.com is still finalizing."); + expect(err).toContain( + "Production setup for example.com is still finalizing on Clerk's side.", + ); expect(err).toContain("Paused"); expect(err).not.toContain("Production ready at"); }); @@ -899,7 +910,7 @@ describe("deploy", () => { expect(err).toContain("[ ] Create production instance"); expect(err).toContain("[ ] Verify DNS records"); expect(err).toContain("[ ] Configure Google OAuth credentials"); - expect(err).toContain("Check the Domains section in the Clerk Dashboard"); + expect(flat(err)).toContain("on the Domains page in the Clerk Dashboard"); }); test("asks directly for an owned production domain and accepts short domains", async () => { @@ -979,6 +990,9 @@ describe("deploy", () => { expect(err).toContain("Next steps"); expect(err).toContain("clerk env pull --instance prod"); expect(err).toContain("Update env vars on your hosting provider"); + expect(err).toContain("Also copy the other Clerk variables from your env file"); + expect(flat(err)).toContain("sign up at https://example.com to confirm it works"); + expect(err).toContain("Manage this instance in the Clerk Dashboard"); expect(err).toContain( "https://dashboard.clerk.com/apps/app_xyz789/instances/ins_prod_mock/domains", ); @@ -997,15 +1011,21 @@ describe("deploy", () => { await runDeployUntilPause(); const err = stripAnsi(captured.err); - expect(err).toContain("Clerk will associate these subdomains with example.com"); + expect(flat(err)).toContain( + "Clerk will use these subdomains for example.com. You'll add DNS records for them after the instance is created. The exact list is printed once the instance exists:", + ); + // A blank line separates the domain the user just typed from this screen. + expect(err).toMatch(/│[ \t]*\n│[ \t]+Clerk will use these subdomains for example\.com\./); expect(err).toContain("clerk.example.com"); expect(err).toContain("accounts.example.com"); expect(err).toContain("clkmail.example.com"); expect(err).toContain("This will create a Clerk production instance"); expect(err).toContain("Add the following records at your DNS provider"); - expect(err).toContain("Check the Domains section in the Clerk Dashboard"); + expect(flat(err)).toContain("on the Domains page in the Clerk Dashboard"); expect(err).toContain("propagation and SSL issuance"); - expect(err).toContain("DNS propagation can take time"); + expect(flat(err)).toContain( + "Next you'll set up OAuth, then this command checks that these records have taken effect at your DNS provider", + ); expect(mockConfirm).toHaveBeenCalledTimes(3); expect(mockConfirm).toHaveBeenCalledWith({ message: "Create production instance?", @@ -1037,7 +1057,9 @@ describe("deploy", () => { await runDeploy({}); const err = stripAnsi(captured.err); - expect(err).toContain("Clerk will associate these subdomains with example.com"); + expect(flat(err)).toContain( + "Clerk will use these subdomains for example.com. You'll add DNS records for them after the instance is created. The exact list is printed once the instance exists:", + ); expect(err).toContain("No production instance was created."); expect(mockCreateProductionInstance).not.toHaveBeenCalled(); expect(mockConfirm).toHaveBeenCalledWith({ @@ -1253,6 +1275,84 @@ describe("deploy", () => { expect(err).not.toContain("https://accounts.example.com/v1/oauth_callback"); }); + test("Google OAuth walkthrough prints the consent-screen app name users will see", async () => { + await linkedProject(); + mockIsAgent.mockReturnValue(false); + stubCreateProductionInstance({ + frontendApiUrl: "https://clerk-fapi.example.com", + cnameTargets: [], + }); + mockConfirm + .mockResolvedValueOnce(true) // Proceed? + .mockResolvedValueOnce(true); // Create production instance? + mockOpenBrowser.mockResolvedValueOnce({ ok: true, launcher: "test" }); + mockSelect + .mockResolvedValueOnce("walkthrough") // Google OAuth credentials + .mockResolvedValueOnce("have-credentials") + .mockResolvedValueOnce("skip"); // DNS verification + mockInput.mockResolvedValueOnce("example.com").mockResolvedValueOnce("fake-client-id-12345"); + mockPassword.mockResolvedValueOnce("fake-secret"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + // Google requires the consent screen before it will create a client, and + // the name entered there is what end users see. + expect(flat(err)).toContain( + "The consent screen's app name is what users see when they sign in with Google. Use the name you want them to see.", + ); + // Wrapped inside the frame, continuation aligned under the text after + // the TIP label (3 + 8 columns), like the NOTE lines elsewhere. + expect(err).toMatch( + /TIP {8}The consent screen's app name is what users see when they sign in\n│ {13}with Google\. Use the name you want them to see\./, + ); + // The Clerk app name is often a directory-derived slug, so showing it + // read as a recommendation to reuse it on a user-facing screen. + expect(err).not.toContain('my-saas-app".'); + expect(err).not.toContain("Your Clerk app is named"); + // Guidance follows the values to paste and the IMPORTANT note, not before. + const tipAt = err.indexOf("The consent screen's app name is what users see"); + expect(err.indexOf("Authorized Redirect URI")).toBeLessThan(tipAt); + expect(err.indexOf("IMPORTANT")).toBeLessThan(tipAt); + }); + + test("the consent-screen TIP is Google-only", async () => { + mockOpenBrowser.mockResolvedValue({ ok: true, launcher: "test" }); + + await showOAuthWalkthrough("github", "example.com", "https://clerk.example.com"); + const github = stripAnsi(captured.err); + captured.clear(); + await showOAuthWalkthrough("google", "example.com", "https://clerk.example.com"); + const google = stripAnsi(captured.err); + + expect(github).toContain("Configure your GitHub OAuth app"); + expect(github).not.toContain("consent screen's app name"); + expect(google).toContain("consent screen's app name"); + expect(google).not.toContain("undefined"); + }); + + test("names the Clerk production instance and where it lives once created", async () => { + await linkedProject(); + mockIsAgent.mockReturnValue(false); + stubCreateProductionInstance({ cnameTargets: [] }); + mockConfirm + .mockResolvedValueOnce(true) // Proceed? + .mockResolvedValueOnce(true); // Create production instance? + mockSelect.mockResolvedValueOnce("skip").mockResolvedValueOnce("skip"); + mockInput.mockResolvedValueOnce("example.com"); + + await runDeployUntilPause(); + const err = stripAnsi(captured.err); + + // "Clerk production instance", not just "production instance": the user + // also has a deployment on their host, and this is the one Clerk manages. + // The URL sits on its own indented line so the sentence fits the frame + // and terminal autolinkers see the whole URL. + expect(err).toMatch( + /Clerk production instance created\. Manage it in the Dashboard:\n│ {4}https:\/\/dashboard\.clerk\.com\/apps\/app_xyz789\/instances\/ins_\S+/, + ); + }); + test("Apple .p8 file prompt validates path and PEM framing before continuing", async () => { await linkedProject({ instances: { development: "ins_dev_123", production: "ins_prod_apple" }, @@ -1669,8 +1769,175 @@ describe("deploy", () => { await runDeploy({}); const err = stripAnsi(captured.err); - expect(err).toContain("SSL, email DNS still pending for example.com"); - expect(err).not.toContain("DNS, SSL, email DNS still pending"); + expect(err).toContain("Email DNS records not found yet for example.com"); + expect(err).not.toContain("DNS and email DNS records"); + }); + + test("resume prints only the records still outstanding, with the hedged heading", async () => { + // Frontend API DNS is already verified on resume; reprinting its record + // under "Add the following records" would send the user to add it again. + await linkedProject({ + instances: { development: "ins_dev_123", production: "ins_prod_123" }, + }); + mockIsAgent.mockReturnValue(false); + mockLiveProduction({ + instanceId: "ins_prod_123", + developmentConfig: {}, + productionConfig: {}, + cnameTargets: [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "clkmail.example.com", value: "mail.clerk.services", required: true }, + ], + }); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: true, ssl: false, mail: false }), + ); + mockConfirm.mockResolvedValueOnce(false); // BIND export + mockSelect.mockResolvedValueOnce("skip"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(err).toContain( + "Add the following records at your DNS provider if you haven't already:", + ); + expect(err).not.toMatch(/Add the following records at your DNS provider:/); + expect(err).toContain("Host: clkmail.example.com"); + expect(err).not.toContain("Host: clerk.example.com"); + // OAuth ran before this screen on resume, so it is not "next". + expect(flat(err)).toContain( + "Next, this command checks that these records have taken effect at your DNS provider.", + ); + expect(err).not.toContain("set up OAuth"); + }); + + test("resume exports every record to the zone file even when the screen lists only the outstanding ones", async () => { + // A zone file with some of the domain's records isn't one to import. + await linkedProject({ + instances: { development: "ins_dev_123", production: "ins_prod_123" }, + }); + mockIsAgent.mockReturnValue(false); + mockLiveProduction({ + instanceId: "ins_prod_123", + developmentConfig: {}, + productionConfig: {}, + cnameTargets: [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "clkmail.example.com", value: "mail.clerk.services", required: true }, + ], + }); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: true, ssl: false, mail: false }), + ); + mockConfirm.mockResolvedValueOnce(true); // BIND export: yes + mockSelect.mockResolvedValueOnce("skip"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(err).not.toContain("Host: clerk.example.com"); + const zoneCall = writeSpy.mock.calls.find((call: unknown[]) => + String(call[0]).endsWith(".zone"), + ); + expect(zoneCall).toBeDefined(); + const zone = String(zoneCall![1]); + expect(zone).toContain("clerk.example.com"); + expect(zone).toContain("clkmail.example.com"); + }); + + test("resume still offers the zone-file export when only SSL is pending", async () => { + await linkedProject({ + instances: { development: "ins_dev_123", production: "ins_prod_123" }, + }); + mockIsAgent.mockReturnValue(false); + mockLiveProduction({ + instanceId: "ins_prod_123", + developmentConfig: {}, + productionConfig: {}, + }); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: true, ssl: false, mail: true }), + ); + mockConfirm.mockResolvedValueOnce(true); // BIND export: yes + mockSelect.mockResolvedValueOnce("skip"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(mockConfirm).toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining("BIND") }), + ); + const zoneCall = writeSpy.mock.calls.find((call: unknown[]) => + String(call[0]).endsWith(".zone"), + ); + expect(zoneCall).toBeDefined(); + // Nothing left to add: the screen says the certificate is pending + // instead of framing a DNS task around an empty record list. + expect(err).toContain("Your DNS records for example.com are verified."); + expect(flat(err)).toContain( + "The SSL certificate is still pending; Clerk issues it automatically.", + ); + expect(err).not.toContain("Configure DNS for"); + expect(err).not.toContain("these records"); + expect(err).not.toContain("Add the following records"); + // Skipping closes on the domain, not on DNS, which is already done. + expect(err).toContain("Domain Not yet verified"); + expect(err).toMatch(/└\s+Not verified/); + }); + + test("resume while Clerk is finalizing says so and does not promise a retry", async () => { + await linkedProject({ + instances: { development: "ins_dev_123", production: "ins_prod_123" }, + }); + mockIsAgent.mockReturnValue(false); + mockLiveProduction({ + instanceId: "ins_prod_123", + developmentConfig: {}, + productionConfig: {}, + }); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: true, ssl: true, mail: true }), + ); + mockConfirm.mockResolvedValueOnce(false); // BIND export + mockSelect.mockResolvedValueOnce("skip"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(flat(err)).toContain("Clerk is still finalizing production setup."); + expect(flat(err)).toContain("run `clerk deploy` again in a few minutes"); + expect(err).not.toContain("check again"); + expect(err).not.toContain("Configure DNS for"); + }); + + test("resume with no record list from Clerk tells the user to find and add the records", async () => { + await linkedProject({ + instances: { development: "ins_dev_123", production: "ins_prod_123" }, + }); + mockIsAgent.mockReturnValue(false); + mockLiveProduction({ + instanceId: "ins_prod_123", + developmentConfig: {}, + productionConfig: {}, + cnameTargets: [], + }); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: true, ssl: false, mail: false }), + ); + mockSelect.mockResolvedValueOnce("skip"); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(flat(err)).toContain( + "Email DNS records for example.com are not verified yet, but Clerk didn't return the list to add.", + ); + expect(flat(err)).toContain("add them at your DNS provider, then choose Check DNS now below"); + expect(err).not.toContain("Add the following records"); + // No records, no export offer. + expect(mockConfirm).not.toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining("BIND") }), + ); }); test("DNS verification treats absent components as pending", async () => { @@ -1692,10 +1959,83 @@ describe("deploy", () => { const err = stripAnsi(captured.err); expect(err).toContain("DNS: pending SSL: ✓ Email DNS: ✓"); - expect(err).toContain("DNS still pending for example.com"); + expect(err).toContain("DNS records not found yet for example.com"); expect(err).not.toContain("Domain Verified"); }); + test("no record list on a first run without providers points straight at the DNS check", async () => { + await linkedProject(); + mockIsAgent.mockReturnValue(false); + // No enabled providers: OAuth setup is skipped, so the check really is next. + mockFetchInstanceConfig.mockResolvedValue({}); + stubCreateProductionInstance({ cnameTargets: [] }); + mockConfirm + .mockResolvedValueOnce(true) // Proceed? + .mockResolvedValueOnce(true); // Create production instance? + mockInput.mockResolvedValueOnce("example.com"); + mockSelect.mockResolvedValueOnce("skip"); // DNS verification + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: false, ssl: false, mail: false }), + ); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + expect(flat(err)).toContain("add them at your DNS provider, then choose Check DNS now below"); + expect(err).toContain("Next, this command checks that they have taken effect."); + expect(err).not.toContain("set up OAuth"); + }); + + test("DNS verification footer says the record list is missing when the API returned no targets", async () => { + await linkedProject(); + mockIsAgent.mockReturnValue(false); + stubCreateProductionInstance({ cnameTargets: [] }); + mockConfirm + .mockResolvedValueOnce(true) // Proceed? + .mockResolvedValueOnce(true); // Create production instance? (no zone-file prompt: no targets) + mockInput.mockResolvedValueOnce("example.com"); + mockSelect + .mockResolvedValueOnce("have-credentials") + .mockResolvedValueOnce("check") + .mockResolvedValueOnce("skip"); + mockInput.mockResolvedValueOnce("google-client-id.apps.googleusercontent.com"); + mockPassword.mockResolvedValueOnce("google-secret"); + mockPatchInstanceConfig.mockResolvedValueOnce({}); + mockGetApplicationDomainStatus.mockResolvedValue( + domainStatus({ status: "incomplete", dns: false, ssl: false, mail: false }), + ); + + await runDeploy({}); + const err = stripAnsi(captured.err); + + // Before the check, the first-run screen already says the list is + // missing and what to do, instead of a "Configure DNS" page with no records. + // This run has a Google provider, so OAuth setup is the next prompt and + // the screen must not point at a "Check DNS now" that isn't there yet. + expect(flat(err)).toContain( + "DNS and email DNS records for example.com are not verified yet, but Clerk didn't return the list to add.", + ); + expect(flat(err)).toContain("and add them at your DNS provider:"); + expect(flat(err)).toContain( + "Next you'll set up OAuth, then this command checks that they have taken effect.", + ); + expect(err).not.toContain("Check DNS now below"); + expect(err).not.toContain("Configure DNS for"); + // After the check, the footer says the same in its own words. + expect(err).toContain("DNS and email DNS records not found yet for example.com."); + expect(err).toContain("Clerk didn't return the list of records to add."); + expect(flat(err)).toContain( + "Find them on the Domains page in the Clerk Dashboard, add them, then choose Check again below.", + ); + // The URL line belongs to the footer, so it precedes the skip, not the + // Next steps block (which also prints this URL). + expect(err).toMatch( + /already created\.\n│ {4}https:\/\/dashboard\.clerk\.com\/apps\/app_xyz789\/instances\/ins_prod_mock\/domains\n[\s\S]*Skipping DNS verification/, + ); + expect(err).not.toContain("Add them at your DNS provider"); + expect(err).not.toContain("Add the following records at your DNS provider"); + }); + test("DNS verification timeout does not reprint DNS records when only SSL remains pending", async () => { await linkedProject(); mockIsAgent.mockReturnValue(false); @@ -1719,8 +2059,10 @@ describe("deploy", () => { const err = stripAnsi(captured.err); expect(err).toContain("DNS: ✓ SSL: pending Email DNS: ✓"); - expect(err).toContain("SSL still pending for example.com"); + expect(err).toContain("SSL certificate still pending for example.com"); + expect(err).not.toContain("not found yet"); expect(err.match(/Add the following records at your DNS provider:/g)).toHaveLength(1); + expect(err).not.toContain("if you haven't already:"); }); test("plain deploy can skip DNS verification and continue configuring production", async () => { @@ -1745,8 +2087,11 @@ describe("deploy", () => { const err = stripAnsi(captured.err); expect(err).toContain("Saved Google OAuth credentials"); - expect(err).toContain("Domain DNS pending"); + expect(err).toContain("Domain Not yet verified"); expect(err).not.toContain("Domain Verified"); + // The closing word agrees with the status row instead of contradicting it. + expect(err).toMatch(/└\s+Not verified/); + expect(err).not.toMatch(/└\s+Success/); expect(mockSelect).toHaveBeenCalledWith({ message: "DNS verification", choices: [ @@ -1773,8 +2118,10 @@ describe("deploy", () => { await runDeployUntilPause(); const err = stripAnsi(captured.err); - expect(err).toContain("Check the Domains section in the Clerk Dashboard"); - expect(err).toContain("DNS propagation can take time"); + expect(flat(err)).toContain("on the Domains page in the Clerk Dashboard"); + expect(flat(err)).toContain( + "Next you'll set up OAuth, then this command checks that these records have taken effect at your DNS provider", + ); expect(err).toContain("Configure Google OAuth for production"); }); @@ -1890,7 +2237,7 @@ describe("deploy", () => { await runDeployUntilPause(); mockLiveProduction(); - expect(stripAnsi(captured.err)).toContain("Check the Domains section in the Clerk Dashboard"); + expect(flat(captured.err)).toContain("on the Domains page in the Clerk Dashboard"); expect(stripAnsi(captured.err)).toContain("Configure Google OAuth for production"); captured.clear(); @@ -2103,10 +2450,19 @@ describe("deploy", () => { await runDeploy({}); const err = stripAnsi(captured.err); - expect(err).toContain("DNS propagation can take several hours"); - expect(err).toContain("DNS, SSL, email DNS still pending for example.com"); + expect(err).toContain("Propagation usually takes minutes"); + expect(err).toContain("DNS and email DNS records not found yet for example.com"); + // The footer, not the Next steps block, carries the change-domain URL. + expect(flat(err)).toContain( + "change the domain in the Clerk Dashboard: https://dashboard.clerk.com/apps/app_xyz789/instances/ins_prod_mock/domains", + ); + expect(err).not.toContain("still pending"); expect(err).toContain("DNS: pending"); - expect(err.match(/Add the following records at your DNS provider:/g)).toHaveLength(2); + // First hand-over uses the plain heading; the reprint after a failed check hedges. + expect(err.match(/Add the following records at your DNS provider:/g)).toHaveLength(1); + expect( + err.match(/Add the following records at your DNS provider if you haven't already:/g), + ).toHaveLength(1); expect(err).toContain("Host: clerk.example.com"); expect(err).toContain("Value: frontend-api.clerk.services"); expect(err).toContain("Skipping DNS verification for now."); diff --git a/packages/cli-core/src/commands/deploy/index.ts b/packages/cli-core/src/commands/deploy/index.ts index 6f0958e0d..34be656bf 100644 --- a/packages/cli-core/src/commands/deploy/index.ts +++ b/packages/cli-core/src/commands/deploy/index.ts @@ -15,8 +15,11 @@ import { type ProductionInstanceResponse, } from "../../lib/plapi.ts"; import { + DEPLOY_COMMAND_DESCRIPTION, + DEPLOY_COMMAND_SUMMARY, INTRO_PREAMBLE, OAUTH_SECTION_INTRO, + type DeployComponentStatus, type DeployPlanStep, deployComponentLabels, deployComponentStatus, @@ -24,10 +27,13 @@ import { domainAssociationSummary, bindZoneFile, dnsDashboardHandoff, + dnsHandoffNothingToAdd, dnsIntro, dnsRecords, + domainsDashboardUrl, + instanceDashboardUrl, nextStepsBody, - pendingDnsRecords, + pendingCnameTargets, pausedOperationNotice, printPlan, productionSummary, @@ -182,6 +188,11 @@ async function startNewDeploy(ctx: DeployContext): Promise { } const production = productionOrExists; await persistProductionInstance(ctx, production.id); + // "Clerk production instance", not just "production instance": the user + // also has a deployment on their host, and this is the one Clerk manages. + // URL on its own line: with it, the sentence is wider than the frame. + log.success("Clerk production instance created. Manage it in the Dashboard:"); + log.info(` ${instanceDashboardUrl(ctx.appId, production.id)}`); if (!production.active_domain) { throw new CliError( @@ -209,7 +220,18 @@ async function startNewDeploy(ctx: DeployContext): Promise { cnameTargets, }; - await runDnsRecordHandoff({ ...operationState, pending: { type: "dns" } }, cnameTargets); + // OAuth setup only runs when there are providers; with none, the DNS check + // is what comes next. + await runDnsRecordHandoff( + { ...operationState, pending: { type: "dns" } }, + { + display: cnameTargets, + exportTargets: cnameTargets, + // Nothing has been checked yet on a fresh run. + status: { dns: false, ssl: false, mail: false }, + oauthNext: oauthProviders.length > 0, + }, + ); bar(); completedOAuthProviders = await runOAuthSetup(ctx, operationState, oauthProviders); @@ -279,10 +301,11 @@ async function reconcileExistingDeploy(ctx: DeployContext): Promise { } if (!snapshot.domainComplete) { - dnsStatus = await runExistingDomainDnsVerification(ctx, { - ...snapshot, - pending: { type: "dns" }, - }); + dnsStatus = await runExistingDomainDnsVerification( + ctx, + { ...snapshot, pending: { type: "dns" } }, + snapshot.componentStatus, + ); } await finishDeploy(ctx, snapshot.domain, snapshot.completedOAuthProviders, dnsStatus); @@ -352,6 +375,9 @@ async function createProductionInstance( } async function confirmProductionInstanceCreation(domain: string): Promise { + // Separate this screen from the domain the user just typed; without it the + // echoed answer runs straight into the lead sentence. + log.blank(); for (const line of domainAssociationSummary(domain)) log.info(line); log.blank(); const confirmed = await confirmCreateProductionInstance(); @@ -366,21 +392,50 @@ async function confirmProductionInstanceCreation(domain: string): Promise { - for (const line of dnsIntro(state.domain)) log.info(line); - log.blank(); - if (cnameTargets.length > 0) { - for (const line of dnsRecords(cnameTargets)) log.info(line); - log.blank(); + const { display, exportTargets, status } = options; + const handoffInstanceId = state.productionInstanceId; + const domainsUrl = handoffInstanceId + ? domainsDashboardUrl(state.appId, handoffInstanceId) + : undefined; + + // With no records to add, a "Configure DNS" page is the wrong page: say + // what is actually outstanding instead (certificate, Clerk finalizing, or + // a record list Clerk didn't return). + const lines = + display.length > 0 + ? [ + ...dnsIntro(state.domain), + "", + ...dnsRecords(display, { afterCheck: options.afterCheck }), + "", + ...dnsDashboardHandoff(state.domain, domainsUrl, { oauthNext: options.oauthNext }), + ] + : dnsHandoffNothingToAdd(state.domain, status, domainsUrl, { oauthNext: options.oauthNext }); + for (const line of lines) { + if (line === "") log.blank(); + else log.info(line); } - - for (const line of dnsDashboardHandoff(state.domain)) log.info(line); log.blank(); try { - await offerBindZoneExport(state.domain, cnameTargets); + await offerBindZoneExport(state.domain, exportTargets); log.blank(); } catch (error) { if (error instanceof UserAbortError) { @@ -393,8 +448,19 @@ async function runDnsRecordHandoff( async function runExistingDomainDnsVerification( ctx: DeployContext, state: DeployOperationState, + componentStatus: DeployComponentStatus, ): Promise { - await runDnsRecordHandoff(state, state.cnameTargets ?? []); + // On resume some records may already be verified; only the outstanding ones + // are records to add, and the user may have added those already. + const allTargets = state.cnameTargets ?? []; + // OAuth ran before this on the resume path, so the check is next. + await runDnsRecordHandoff(state, { + display: pendingCnameTargets(allTargets, componentStatus), + exportTargets: allTargets, + status: componentStatus, + afterCheck: true, + oauthNext: false, + }); return runDnsVerificationPrompt(ctx, state); } @@ -436,8 +502,19 @@ async function runDnsVerification( log.blank(); log.info(deployComponentStatus(outcome.status)); log.blank(); - for (const line of deployStatusPendingFooter(state.domain, outcome.status)) { - log.warn(line); + const productionInstanceId = + state.productionInstanceId ?? ctx.productionInstanceId ?? ctx.profile.instances.production; + // Computed before the footer so the footer can tell the user when there is + // no record list to print, instead of saying "add them" over nothing. + const pendingTargets = pendingCnameTargets(state.cnameTargets ?? [], outcome.status); + for (const line of deployStatusPendingFooter( + state.domain, + outcome.status, + productionInstanceId ? domainsDashboardUrl(ctx.appId, productionInstanceId) : undefined, + pendingTargets.length > 0, + )) { + if (line === "") log.blank(); + else log.warn(line); } // When all DNS components are verified but the server has not yet marked the @@ -446,12 +523,9 @@ async function runDnsVerification( throw deployPausedError(state); } - const pendingRecords = state.cnameTargets - ? pendingDnsRecords(state.cnameTargets, outcome.status) - : []; - if (pendingRecords.length > 0) { + if (pendingTargets.length > 0) { log.blank(); - for (const line of pendingRecords) log.info(line); + for (const line of dnsRecords(pendingTargets, { afterCheck: true })) log.info(line); } log.blank(); let action: Awaited>; @@ -638,15 +712,22 @@ async function finishDeploy( prefix: isInsideGutter() ? `${dim("│")} ` : "", label: "Next steps", fallback: bold, - body: `${applyPrefix(nextStepsBody(ctx.appId, productionInstanceId))}\n`, + body: `${applyPrefix(nextStepsBody(ctx.appId, productionInstanceId, domain, dnsStatus))}\n`, }); - await outro("Success"); + // The closing word summarizes how the run ended. After a skipped DNS check + // the status row four lines up says "DNS pending", and "Success" beneath it + // contradicted that; the same value that drives the headline drives this. + await outro(dnsStatus === "verified" ? "Success" : "Not verified"); } export function registerDeploy(program: Program): void { + // `summary` is what the root `clerk --help` table shows; `description` is + // the prose on `clerk deploy --help`, where the hidden default subcommand + // would otherwise leave no trace of what the bare command does. const deployCmd = program .command("deploy") - .description("Deploy a Clerk application to production"); + .summary(DEPLOY_COMMAND_SUMMARY) + .description(DEPLOY_COMMAND_DESCRIPTION); deployCmd.command("run", { isDefault: true, hidden: true }).action(deploy); deployCmd .command("status") diff --git a/packages/cli-core/src/commands/deploy/providers.ts b/packages/cli-core/src/commands/deploy/providers.ts index 3a2aabc99..52624ef05 100644 --- a/packages/cli-core/src/commands/deploy/providers.ts +++ b/packages/cli-core/src/commands/deploy/providers.ts @@ -2,6 +2,7 @@ import { OAUTH_PROVIDERS } from "@clerk/shared/oauth"; import { bold, cyan, dim, yellow } from "../../lib/color.ts"; import { clerkSubdomains } from "./copy.ts"; import { log } from "../../lib/log.ts"; +import { wrap } from "../../lib/wrap.ts"; import { openBrowser } from "../../lib/open.ts"; import type { ConfigSchemaProperty, InstanceConfigSchema } from "../../lib/plapi.ts"; @@ -472,6 +473,21 @@ export async function showOAuthWalkthrough( log.blank(); log.info(gotcha); } + // Google makes you fill in the OAuth consent screen before it will create a + // client, and the name entered there is what end users see. It is free-form + // and unrelated to the Clerk app's name, so this says to choose rather than + // naming a value: the Clerk name is often a directory-derived slug, and + // showing it read as a recommendation to use it. Guidance, not a value to + // paste, so it follows the values. + if (slug === "google") { + log.blank(); + for (const line of wrap( + `${dim(cyan("TIP"))} The consent screen's app name is what users see when they sign in with Google. Use the name you want them to see.`, + { hang: 11 }, + )) { + log.info(line); + } + } log.blank(); log.info(dim(`Provider guide: ${docsUrl}`)); diff --git a/packages/cli-core/src/commands/deploy/status-command.test.ts b/packages/cli-core/src/commands/deploy/status-command.test.ts index f9f263989..72836d9c3 100644 --- a/packages/cli-core/src/commands/deploy/status-command.test.ts +++ b/packages/cli-core/src/commands/deploy/status-command.test.ts @@ -23,7 +23,7 @@ mock.module("../../lib/sleep.ts", () => ({ const { _setConfigDir, setProfile } = await import("../../lib/config.ts"); const { setMode } = await import("../../mode.ts"); const { beginInterrupt, _resetInterruptState } = await import("../../lib/signals.ts"); -const { deployStatus } = await import("./status-command.ts"); +const { deployStatus, humanNextAction } = await import("./status-command.ts"); /** What an in-flight request rejects with once Ctrl-C aborts the shared signal. */ function abortError(): Error { @@ -196,9 +196,149 @@ describe("deploy status", () => { await deployStatus(); expect(captured.out).toBe(""); - expect(stripAnsi(captured.err)).toContain("clerk deploy"); + const output = stripAnsi(captured.err); + // The person reading this is the user, so the agent's "ask the user" is + // reworded, and OAuth was never checked so its row is not printed. + expect(output).toContain("No production instance yet."); + expect(output).toContain("Run `clerk deploy` to set it up."); + expect(output).not.toContain("ask the user"); + expect(output).not.toContain("human terminal"); + expect(output).not.toContain("OAuth"); + }); + + test("human mode domain_provisioning does not claim OAuth was checked or address an agent", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockListApplicationDomains.mockResolvedValue({ data: [], total_count: 0 }); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain("its domain is still provisioning"); + expect(output).toContain("or run `clerk deploy` to finish setup."); + expect(output).not.toContain("ask the user"); + expect(output).not.toContain("OAuth"); + }); + + test("human mode oauth_pending tells the person to finish the wizard, not to ask themselves", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + // Production config has the provider enabled but no credentials. + mockFetchInstanceConfig.mockImplementation(() => ({ + connection_oauth_google: { enabled: true }, + })); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(completeDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(completeDomainStatus()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain("OAuth pending: google"); + expect(output).toContain( + "missing production credentials: google. Run `clerk deploy` to finish setup.", + ); + expect(output).not.toContain("Ask the user"); + // The domain is verified; nothing to monitor on the Domains page. + expect(output).not.toContain("domains page"); + }); + + test("human mode records-missing says Clerk returned no list and how to resume", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockListApplicationDomains.mockResolvedValue({ + data: [ + { + object: "domain", + id: "dmn_1", + name: "example.com", + is_satellite: false, + is_provider_domain: false, + frontend_api_url: "https://clerk.example.com", + accounts_portal_url: "https://accounts.example.com", + development_origin: "", + }, + ], + total_count: 1, + }); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingDnsDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(pendingDnsDomainStatus()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain( + "DNS records not found yet for example.com, but Clerk didn't return the list of records to add. Find them on the Domains page in the Clerk Dashboard, add them, then run `clerk deploy` again to resume.", + ); + expect(output).not.toContain("this report"); + expect(output).not.toContain("--wait"); + expect(output).not.toContain("Add the following records"); }); + test("human mode says the unsupported-provider warning once, in its own row", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + // discord is enabled in both configs but absent from the schema, so the + // CLI can't configure it: the "unsupported" case. + mockFetchInstanceConfig.mockImplementation((_appId: string, instanceId: string) => + instanceId === "ins_prod" || instanceId === "production" + ? { + connection_oauth_google: { enabled: true, client_id: "x", client_secret: "y" }, + connection_oauth_discord: { enabled: true }, + } + : { + connection_oauth_google: { enabled: true }, + connection_oauth_discord: { enabled: true }, + }, + ); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(completeDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(completeDomainStatus()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain("not supported by automated deploy: discord"); + // The agent sentence carries the same fact; a person already has the row. + expect(output).not.toContain("could not configure them for production"); + expect(JSON.stringify(captured.out)).not.toContain("could not configure"); + }); + + test.each([ + { + label: "finalizing", + domain: () => ({ ...completeDomainStatus(), status: "incomplete" }), + expected: "still finalizing on Clerk's side", + }, + { + label: "complete", + domain: completeDomainStatus, + expected: + "Manage users, settings, and billing for this instance: https://dashboard.clerk.com/apps/app_1/instances/ins_prod", + }, + ])( + "human mode $label prints no records block and no agent copy", + async ({ domain, expected }) => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(domain()); + mockGetApplicationDomainStatus.mockResolvedValue(domain()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain(expected); + expect(output).not.toContain("Add the following records"); + expect(output).not.toContain("Ask the user"); + expect(output).not.toContain("--wait"); + }, + ); + test("agent mode domain pending reports pending DNS records and exit 1", async () => { mockFetchApplication.mockResolvedValue(appWith(true)); mockDomain(); @@ -222,6 +362,7 @@ describe("deploy status", () => { type: "CNAME", host: "clerk.example.com", value: "frontend-api.clerk.services", + required: true, }); }); @@ -339,6 +480,203 @@ describe("deploy status", () => { expect(captured.out).toBe(""); }); + test("human mode prints the pending records and refers to them, not to the JSON field", async () => { + // The agent reads `pendingDnsRecords` from the JSON; a person has no JSON, + // so the records are printed and the sentence points at them. + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingDnsDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(pendingDnsDomainStatus()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain( + "Add the following records at your DNS provider if you haven't already:", + ); + expect(output).toContain("Host: clerk.example.com"); + expect(output).toContain("Value: frontend-api.clerk.services"); + // The records block already says "add these"; the sentence says what's + // next. Human mode already waits, so `--wait` is never suggested; the + // wizard is what resumes setup. + expect(output).toContain("Once they're added, run `clerk deploy` again to resume."); + expect(output).not.toContain("--wait"); + expect(output).not.toContain("Add the records above"); + expect(output).not.toContain("pendingDnsRecords"); + expect(output).not.toContain("Ask the user"); + }); + + test("human mode keeps Clerk's optional flag on a pending record instead of inventing one", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockListApplicationDomains.mockResolvedValue({ + data: [ + { + object: "domain", + id: "dmn_1", + name: "example.com", + is_satellite: false, + is_provider_domain: false, + frontend_api_url: "https://clerk.example.com", + accounts_portal_url: "https://accounts.example.com", + development_origin: "", + cname_targets: [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "clk2._domainkey.example.com", value: "dkim2.clerk.services", required: false }, + ], + }, + ], + total_count: 1, + }); + mockOAuthComplete(); + const pendingBoth = { + status: "incomplete", + dns: { status: "not_started" }, + ssl: { status: "complete", required: true }, + mail: { status: "not_started", required: true }, + }; + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingBoth); + mockGetApplicationDomainStatus.mockResolvedValue(pendingBoth); + + await deployStatus(); + + const output = stripAnsi(captured.err); + // The wizard prints the same record as optional; status must agree. + expect(output).toMatch( + /Email \(DKIM\) \(optional\)\n\s+Type: CNAME\n\s+Host: clk2\._domainkey\.example\.com/, + ); + expect(output).not.toMatch(/Frontend API \(optional\)/); + }); + + test("human mode omits the email note when only non-email records are pending", async () => { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockListApplicationDomains.mockResolvedValue({ + data: [ + { + object: "domain", + id: "dmn_1", + name: "example.com", + is_satellite: false, + is_provider_domain: false, + frontend_api_url: "https://clerk.example.com", + accounts_portal_url: "https://accounts.example.com", + development_origin: "", + cname_targets: [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "clkmail.example.com", value: "mail.clerk.services", required: true }, + ], + }, + ], + total_count: 1, + }); + mockOAuthComplete(); + // Email DNS verified, Frontend API not: the filtered list has no email + // row, so a sentence about "the email records" would point at nothing. + const dnsOnly = { + status: "incomplete", + dns: { status: "not_started" }, + ssl: { status: "complete", required: true }, + mail: { status: "complete", required: true }, + }; + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(dnsOnly); + mockGetApplicationDomainStatus.mockResolvedValue(dnsOnly); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain("Host: clerk.example.com"); + expect(output).not.toContain("Host: clkmail.example.com"); + expect(output).not.toContain("SPF or DKIM"); + }); + + test("agent report carries each pending record's required flag", async () => { + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingDnsDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(pendingDnsDomainStatus()); + + await deployStatus(); + + const payload = JSON.parse(captured.out); + expect(payload.pendingDnsRecords).toEqual([ + { + type: "CNAME", + host: "clerk.example.com", + value: "frontend-api.clerk.services", + required: true, + }, + ]); + }); + + test("agent report passes an optional record through as required: false", async () => { + mockFetchApplication.mockResolvedValue(appWith(true)); + mockListApplicationDomains.mockResolvedValue({ + data: [ + { + object: "domain", + id: "dmn_1", + name: "example.com", + is_satellite: false, + is_provider_domain: false, + frontend_api_url: "https://clerk.example.com", + accounts_portal_url: "https://accounts.example.com", + development_origin: "", + cname_targets: [ + { host: "clerk.example.com", value: "frontend-api.clerk.services", required: true }, + { host: "accounts.example.com", value: "accounts.clerk.services", required: false }, + ], + }, + ], + total_count: 1, + }); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingDnsDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(pendingDnsDomainStatus()); + + await deployStatus(); + + const payload = JSON.parse(captured.out); + expect( + payload.pendingDnsRecords.map((r: { host: string; required: boolean }) => [ + r.host, + r.required, + ]), + ).toEqual([ + ["clerk.example.com", true], + ["accounts.example.com", false], + ]); + }); + + test("human mode rewrites the agent clause for a plain-http Dashboard URL too", async () => { + // Dashboard links follow CLERK_DASHBOARD_URL, which is http:// for a local + // Dashboard; the human rewrite must not depend on https. + const previous = process.env.CLERK_DASHBOARD_URL; + process.env.CLERK_DASHBOARD_URL = "http://localhost:4000"; + try { + setMode("human"); + mockFetchApplication.mockResolvedValue(appWith(true)); + mockDomain(); + mockOAuthComplete(); + mockTriggerApplicationDomainDNSCheck.mockResolvedValue(pendingSslDomainStatus()); + mockGetApplicationDomainStatus.mockResolvedValue(pendingSslDomainStatus()); + + await deployStatus(); + + const output = stripAnsi(captured.err); + expect(output).toContain( + "Visit the Clerk Dashboard domains page to monitor its status there: http://localhost:4000/apps/app_1/instances/ins_prod/domains", + ); + expect(output).not.toContain("Ask the user to visit"); + } finally { + if (previous === undefined) delete process.env.CLERK_DASHBOARD_URL; + else process.env.CLERK_DASHBOARD_URL = previous; + } + }); + test("human mode shows dashboard monitoring guidance without agent handoff copy", async () => { setMode("human"); mockFetchApplication.mockResolvedValue(appWith(true)); @@ -351,10 +689,12 @@ describe("deploy status", () => { const output = stripAnsi(captured.err); expect(output).toContain( - "SSL still provisioning for example.com. Re-run `clerk deploy status` in a few minutes, DNS propagation can take time. Visit the Clerk Dashboard domains page to monitor its status there: https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + "SSL certificate still pending for example.com. Clerk issues it automatically now that DNS is verified; re-run `clerk deploy status` in a few minutes. Visit the Clerk Dashboard domains page to monitor its status there: https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", ); expect(output).not.toContain("Ask the user to visit"); expect(output).not.toContain("offer to open it"); + // No records are outstanding, so no records block. + expect(output).not.toContain("Add the following records"); }); }); @@ -400,3 +740,47 @@ async function routePlapiFetch( return new Response("Not Found", { status: 404 }); } + +describe("humanNextAction", () => { + // These are the three things that used to leak from the agent sentence into + // the human one. Now that each audience has its own renderer, this pins the + // human side for every state rather than trusting seven replace rules. + const URL = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod"; + const steps = [ + { kind: "not_started" as const }, + { kind: "domain_provisioning" as const, domainsUrl: `${URL}/domains` }, + { kind: "interrupted" as const }, + { kind: "complete" as const, domain: "example.com", oauthUnsupported: ["x"], instanceUrl: URL }, + { kind: "oauth_pending" as const, oauthPending: ["github"], oauthUnsupported: ["x"] }, + ...(["records_available", "records_unavailable", "ssl_pending", "finalizing"] as const).map( + (kind) => ({ kind, domain: "example.com", records: "DNS", domainsUrl: `${URL}/domains` }), + ), + ]; + + test.each(steps.map((step) => ({ kind: step.kind, step })))( + "never speaks to an agent: $kind", + ({ step }) => { + const line = humanNextAction(step); + expect(line).not.toContain("ask the user"); + expect(line).not.toContain("Ask the user"); + expect(line).not.toContain("pendingDnsRecords"); + expect(line).not.toContain("--wait"); + expect(line).not.toContain("human terminal"); + // The warning row above the sentence already names unsupported providers. + expect(line).not.toContain("could not configure them"); + }, + ); + + test("points a person at the wizard to resume, and at the Dashboard to watch", () => { + const line = humanNextAction({ + kind: "records_available", + domain: "example.com", + records: "DNS", + domainsUrl: `${URL}/domains`, + }); + expect(line).toBe( + "DNS records not found yet for example.com. Once they're added, run `clerk deploy` again to resume. " + + `Propagation usually takes minutes. Visit the Clerk Dashboard domains page to monitor its status there: ${URL}/domains`, + ); + }); +}); diff --git a/packages/cli-core/src/commands/deploy/status-command.ts b/packages/cli-core/src/commands/deploy/status-command.ts index 21e8c894a..39705c6bd 100644 --- a/packages/cli-core/src/commands/deploy/status-command.ts +++ b/packages/cli-core/src/commands/deploy/status-command.ts @@ -4,15 +4,17 @@ import { log } from "../../lib/log.ts"; import { interruptedExitCode } from "../../lib/signals.ts"; import { sleep } from "../../lib/sleep.ts"; import { withSpinner } from "../../lib/spinner.ts"; -import { deployComponentLabels, type DeployComponentStatus } from "./copy.ts"; +import { deployComponentLabels, dnsRecords, type DeployComponentStatus } from "./copy.ts"; import { buildDeployStatusReport, buildInterruptedDeployStatusReport, + deployNextStep, loadProductionDomain, resolveDeployContext, resolveDeployState, triggerDeployStatusCheck, waitForDeployStatus, + type DeployNextStep, type DeployState, type DeployStatusOutcome, type DeployStatusReport, @@ -133,7 +135,8 @@ function emitReport(report: DeployStatusReport): void { renderHuman(report); } -function renderHuman(report: DeployStatusReport): void { +/** Exported so the human rendering can be exercised directly. */ +export function renderHuman(report: DeployStatusReport): void { log.blank(); if (report.domain) { log.info(`Deploy status for \`${report.domain}\``); @@ -146,7 +149,7 @@ function renderHuman(report: DeployStatusReport): void { // is true here. if (report.state === "interrupted") { log.blank(); - log.info(report.nextAction); + log.info(humanNextAction(deployNextStep(report))); log.blank(); return; } @@ -157,10 +160,14 @@ function renderHuman(report: DeployStatusReport): void { ); } - const oauthStatus = report.oauth.complete - ? "complete" - : `pending: ${report.oauth.pending.join(", ") || "none"}`; - log.info(` OAuth ${oauthStatus}`); + // No domain status means no production instance was read, so OAuth was + // never checked either; "pending: none" would claim it was. + if (report.domainStatus) { + const oauthStatus = report.oauth.complete + ? "complete" + : `pending: ${report.oauth.pending.join(", ") || "none"}`; + log.info(` OAuth ${oauthStatus}`); + } if (report.oauth.unsupported.length > 0) { log.warn( @@ -168,14 +175,92 @@ function renderHuman(report: DeployStatusReport): void { ); } + // The agent gets these as `pendingDnsRecords` in the JSON; a person has no + // JSON, so print the records themselves before the sentence that refers to + // them. + if (report.pendingDnsRecords.length > 0) { + log.blank(); + const targets = report.pendingDnsRecords.map((record) => ({ + host: record.host, + value: record.value, + required: record.required, + })); + for (const line of dnsRecords(targets, { afterCheck: true })) log.info(line); + } + log.blank(); - log.info(formatHumanNextAction(report.nextAction)); + log.info(humanNextAction(deployNextStep(report))); log.blank(); } -function formatHumanNextAction(nextAction: string): string { - return nextAction.replace( - /Ask the user to visit the Clerk Dashboard domains page, or offer to open it: (https:\/\/\S+)/, - "Visit the Clerk Dashboard domains page to monitor its status there: $1", - ); +/** + * The line a person sees under `clerk deploy status`. Rendered from the same + * {@link DeployNextStep} as the agent's `nextAction`, not from that sentence: + * the reader is the user (so never "ask the user"), has no JSON (so never + * `pendingDnsRecords`), already waits (so never `--wait`), and resumes setup + * with the wizard. The unsupported-provider warning row above says its piece, + * so it isn't repeated here. Exported so the wording can be tested per state. + */ +export function humanNextAction(step: DeployNextStep): string { + const domains = (url: string | null): string => + url ? ` Visit the Clerk Dashboard domains page to monitor its status there: ${url}` : ""; + + switch (step.kind) { + case "not_started": + return ( + "No production instance yet. `clerk deploy` configures production interactively and " + + "needs a terminal. Run `clerk deploy` to set it up." + ); + case "domain_provisioning": + return ( + "A production instance exists but its domain is still provisioning. " + + "Run `clerk deploy status` again shortly, or run `clerk deploy` to finish setup." + + domains(step.domainsUrl) + ); + case "interrupted": + return ( + "Interrupted before the deploy status could be read, so nothing is known about this " + + "deploy. Run `clerk deploy status` again to check it." + ); + case "complete": + return ( + `Clerk's production setup for https://${step.domain} is verified. If you haven't already: ` + + `run \`clerk env pull --instance prod\`, set those keys on your host alongside the other ` + + `Clerk variables from your env file, redeploy, then sign up at https://${step.domain} to confirm.` + + (step.instanceUrl + ? ` Manage users, settings, and billing for this instance: ${step.instanceUrl}` + : "") + ); + case "oauth_pending": + return ( + `Domain verified, but these OAuth providers are missing production credentials: ` + + `${step.oauthPending.join(", ")}. Run \`clerk deploy\` to finish setup.` + ); + case "records_available": + // The records block printed above already says "add these"; the + // sentence only needs to say what happens next. + return ( + `${step.records} records not found yet for ${step.domain}. ` + + `Once they're added, run \`clerk deploy\` again to resume. Propagation usually takes minutes.` + + domains(step.domainsUrl) + ); + case "records_unavailable": + return ( + `${step.records} records not found yet for ${step.domain}, but Clerk didn't return the list of records to add. ` + + `Find them on the Domains page in the Clerk Dashboard, add them, then run \`clerk deploy\` again to resume.` + + domains(step.domainsUrl) + ); + case "ssl_pending": + return ( + `SSL certificate still pending for ${step.domain}. Clerk issues it automatically now that ` + + `DNS is verified; re-run \`clerk deploy status\` in a few minutes.` + + domains(step.domainsUrl) + ); + case "finalizing": + return ( + `Production setup for ${step.domain} is still finalizing on Clerk's side. ` + + `Re-run \`clerk deploy status\` in a few minutes.` + + domains(step.domainsUrl) + ); + } } diff --git a/packages/cli-core/src/commands/deploy/status.test.ts b/packages/cli-core/src/commands/deploy/status.test.ts index 62435c391..4325335cf 100644 --- a/packages/cli-core/src/commands/deploy/status.test.ts +++ b/packages/cli-core/src/commands/deploy/status.test.ts @@ -19,8 +19,14 @@ mock.module("../../lib/plapi.ts", () => ({ mockTriggerApplicationDomainDNSCheck(...args), })); -const { buildDeployStatusReport, resolveDeployState, waitForDeployStatus } = - await import("./status.ts"); +const { + agentNextAction, + buildDeployStatusReport, + buildInterruptedDeployStatusReport, + deployNextStep, + resolveDeployState, + waitForDeployStatus, +} = await import("./status.ts"); const ctx = { profileKey: "/tmp/x", @@ -251,6 +257,7 @@ describe("buildDeployStatusReport", () => { type: "CNAME", host: "clerk.example.com", value: "frontend-api.clerk.services", + required: true, }); expect(report.oauth.pending).toEqual(["github"]); }); @@ -266,6 +273,7 @@ describe("buildDeployStatusReport", () => { type: "CNAME", host: "clkmail.example.com", value: "mail.clerk.services", + required: true, }, ]); }); @@ -278,9 +286,9 @@ describe("buildDeployStatusReport", () => { expect(report.state).toBe("oauth_pending"); expect(report.complete).toBe(false); - expect(report.nextAction).toContain( - "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", - ); + expect(report.nextAction).toContain("missing production credentials: github"); + // The domain is verified, so there is nothing to monitor on the Domains page. + expect(report.nextAction).not.toContain("/domains"); expect(report.oauth).toMatchObject({ complete: false, configured: ["google"], @@ -302,9 +310,112 @@ describe("buildDeployStatusReport", () => { expect(report.complete).toBe(true); expect(report.domainStatus).toEqual({ dns: "complete", ssl: "complete", mail: "complete" }); expect(report.nextAction).toContain("https://example.com"); + // Nothing left to monitor on the Domains page once complete; the pointer + // is the instance root, where users, settings, and billing live. expect(report.nextAction).toContain( - "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + "Manage users, settings, and billing for this instance: https://dashboard.clerk.com/apps/app_1/instances/ins_prod", + ); + expect(report.nextAction).not.toContain("/domains"); + expect(report.nextAction).not.toContain("Ask the user to visit"); + }); + + test("complete next action says the keys still have to reach the host", () => { + // Complete on Clerk's side only: the app runs on development keys until + // the production keys are set on the host, and the report can't tell + // whether that happened — so "if you haven't already", never "no action". + const allDone = { + ...activeSnapshot, + completedOAuthProviders: ["google", "github"], + } satisfies LiveDeploySnapshot; + const report = buildDeployStatusReport( + { kind: "active", snapshot: allDone }, + { verified: true, status: { dns: true, ssl: true, mail: true } }, + ); + + expect(report.nextAction).toContain( + "Clerk's production setup for https://example.com is verified. If you haven't already:", + ); + expect(report.nextAction).toContain("clerk env pull --instance prod"); + expect(report.nextAction).toContain("alongside the other Clerk variables from your env file"); + expect(report.nextAction).toContain("sign up at https://example.com to confirm"); + expect(report.nextAction).not.toContain("No action needed"); + }); + + test("pending DNS records tell the agent to add them, not to keep polling", () => { + const report = buildDeployStatusReport( + { kind: "active", snapshot: activeSnapshot }, + { verified: false, status: { dns: false, ssl: false, mail: true } }, + ); + + expect(report.state).toBe("domain_pending"); + expect(report.nextAction).toContain("DNS records not found yet for example.com."); + expect(report.nextAction).toContain("Add the records in `pendingDnsRecords`"); + expect(report.nextAction).toContain("re-run `clerk deploy status --wait`"); + expect(report.nextAction).not.toContain("still provisioning"); + }); + + test("pending email DNS records are named on their own when the Frontend API is verified", () => { + const report = buildDeployStatusReport( + { kind: "active", snapshot: activeSnapshot }, + { verified: false, status: { dns: true, ssl: true, mail: false } }, + ); + + expect(report.nextAction).toContain("Email DNS records not found yet for example.com."); + expect(report.nextAction).not.toContain("email DNS records not found"); + expect(report.nextAction).not.toContain("DNS and email DNS"); + }); + + test("SSL-only pending keeps the wait instruction, since there is nothing to add", () => { + const report = buildDeployStatusReport( + { kind: "active", snapshot: activeSnapshot }, + { verified: false, status: { dns: true, ssl: false, mail: true } }, + ); + + expect(report.state).toBe("domain_pending"); + expect(report.nextAction).toContain( + "SSL certificate still pending for example.com. Clerk issues it automatically now that DNS is verified; re-run `clerk deploy status` in a few minutes.", + ); + expect(report.nextAction).not.toContain("not found yet"); + // DNS is verified in this state, so the old "DNS propagation can take + // time" clause would be wrong here. + expect(report.nextAction).not.toContain("DNS propagation"); + }); + + test("pending DNS with no record list says so instead of pointing at an empty array", () => { + // cname_targets is optional on the API's domain object. When it's absent, + // "add the records in pendingDnsRecords" would send the agent to []. + const noTargets = { ...activeSnapshot, cnameTargets: [] } satisfies LiveDeploySnapshot; + const report = buildDeployStatusReport( + { kind: "active", snapshot: noTargets }, + { verified: false, status: { dns: false, ssl: false, mail: true } }, + ); + + expect(report.state).toBe("domain_pending"); + expect(report.pendingDnsRecords).toEqual([]); + expect(report.nextAction).toContain( + "DNS records not found yet for example.com, but this report has no record list.", ); + expect(report.nextAction).toContain("Find the records to add on the Domains page"); + expect(report.nextAction).toContain("re-run `clerk deploy status --wait`"); + expect(report.nextAction).not.toContain("Add the records in `pendingDnsRecords`"); + expect(report.nextAction).not.toContain("still provisioning"); + // The Dashboard URL appears once, via the shared trailing clause. + expect(report.nextAction.match(/\/domains/g)).toHaveLength(1); + }); + + test("all components verified but not yet complete says Clerk is still finalizing", () => { + const report = buildDeployStatusReport( + { kind: "active", snapshot: activeSnapshot }, + { verified: false, status: { dns: true, ssl: true, mail: true } }, + ); + + expect(report.state).toBe("domain_pending"); + expect(report.pendingDnsRecords).toEqual([]); + expect(report.nextAction).toContain( + "Production setup for example.com is still finalizing on Clerk's side.", + ); + expect(report.nextAction).not.toContain("not found yet"); + expect(report.nextAction).not.toContain("SSL"); }); test("unsupported OAuth providers surface without blocking completion", () => { @@ -322,4 +433,179 @@ describe("buildDeployStatusReport", () => { expect(report.complete).toBe(true); expect(report.oauth.unsupported).toEqual(["discord"]); }); + + test.each<{ label: string; completed: string[] }>([ + { label: "complete", completed: ["google", "github"] }, + { label: "oauth_pending", completed: ["google"] }, + ])( + "names providers the CLI could not configure so the agent does not call OAuth done ($label)", + ({ completed }) => { + // In development Clerk supplies shared OAuth credentials; in production + // it doesn't. A provider the CLI skipped has a sign-in button that fails + // for real users, and `oauth.complete` only covers what the CLI manages. + const withUnsupported = { + ...activeSnapshot, + completedOAuthProviders: completed, + unsupportedOAuthProviders: ["discord"], + unsupportedOAuthProviderCount: 1, + } satisfies LiveDeploySnapshot; + const report = buildDeployStatusReport( + { kind: "active", snapshot: withUnsupported }, + { verified: true, status: { dns: true, ssl: true, mail: true } }, + ); + + expect(report.nextAction).toContain( + "These providers are enabled in development but the CLI could not configure them for production: discord.", + ); + expect(report.nextAction).toContain("users signing in with them will fail"); + }, + ); + + test("does not mention unsupported providers when there are none", () => { + const report = buildDeployStatusReport( + { + kind: "active", + snapshot: { ...activeSnapshot, completedOAuthProviders: ["google", "github"] }, + }, + { verified: true, status: { dns: true, ssl: true, mail: true } }, + ); + + expect(report.nextAction).not.toContain("could not configure"); + }); + + test.each([ + { label: "complete", verified: true, status: { dns: true, ssl: true, mail: true } }, + { label: "records pending", verified: false, status: { dns: false, ssl: false, mail: false } }, + { label: "SSL pending", verified: false, status: { dns: true, ssl: false, mail: true } }, + ])( + "omits Dashboard links cleanly when the production instance id is unknown ($label)", + ({ verified, status }) => { + const noInstance = { + ...activeSnapshot, + productionInstanceId: undefined, + completedOAuthProviders: ["google", "github"], + } satisfies LiveDeploySnapshot; + const report = buildDeployStatusReport( + { kind: "active", snapshot: noInstance }, + { verified, status }, + ); + + expect(report.productionInstanceId).toBeNull(); + expect(report.nextAction).not.toContain("dashboard.clerk.com"); + expect(report.nextAction).not.toContain("undefined"); + expect(report.nextAction).not.toContain("Clerk Dashboard domains page"); + }, + ); +}); + +describe("report urls", () => { + const snapshot = { + appId: "app_1", + developmentInstanceId: "ins_dev", + productionInstanceId: "ins_prod", + productionDomainId: "dmn_1", + domain: "example.com", + oauthProviders: [], + oauthProviderDescriptors: [], + completedOAuthProviders: [], + cnameTargets: [], + domainComplete: false, + componentStatus: { dns: false, ssl: false, mail: false }, + unsupportedOAuthProviderCount: 0, + unsupportedOAuthProviders: [], + pending: undefined, + } satisfies LiveDeploySnapshot; + + test("carries the instance and Domains page URLs once a production instance exists", () => { + // Agents used to have to pull the URL out of the `nextAction` prose. + const report = buildDeployStatusReport({ kind: "active", snapshot }, null); + expect(report.urls).toEqual({ + instance: "https://dashboard.clerk.com/apps/app_1/instances/ins_prod", + domains: "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + }); + expect( + buildDeployStatusReport( + { kind: "domain_provisioning", appId: "app_1", productionInstanceId: "ins_prod" }, + null, + ).urls, + ).toEqual(report.urls); + }); + + test("is null when there is no production instance to point at", () => { + expect(buildDeployStatusReport({ kind: "not_started" }, null).urls).toBeNull(); + expect(buildInterruptedDeployStatusReport().urls).toBeNull(); + expect( + buildDeployStatusReport( + { kind: "active", snapshot: { ...snapshot, productionInstanceId: undefined } }, + null, + ).urls, + ).toBeNull(); + }); +}); + +describe("deployNextStep", () => { + // The step is derived from the report's own fields, so a report and the + // sentence stored in it can't describe different situations. + const base = { + complete: false, + state: "domain_pending" as const, + domain: "example.com", + productionInstanceId: "ins_prod", + domainStatus: { dns: "pending", ssl: "pending", mail: "pending" } as const, + pendingDnsRecords: [ + { type: "CNAME" as const, host: "clerk.example.com", value: "v", required: true }, + ], + oauth: { complete: true, configured: [], pending: [], unsupported: [] }, + urls: { + domains: "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains", + instance: "https://dashboard.clerk.com/apps/app_1/instances/ins_prod", + }, + }; + + test.each([ + { + label: "records to add", + domainStatus: { dns: "pending", ssl: "pending", mail: "pending" } as const, + records: 1, + kind: "records_available", + phrase: "DNS and email DNS", + }, + { + label: "records missing from the report", + domainStatus: { dns: "pending", ssl: "pending", mail: "complete" } as const, + records: 0, + kind: "records_unavailable", + phrase: "DNS", + }, + { + label: "only SSL pending", + domainStatus: { dns: "complete", ssl: "pending", mail: "complete" } as const, + records: 0, + kind: "ssl_pending", + phrase: "", + }, + { + label: "everything verified, Clerk finalizing", + domainStatus: { dns: "complete", ssl: "complete", mail: "complete" } as const, + records: 0, + kind: "finalizing", + phrase: "", + }, + ])("classifies a pending domain: $label", ({ domainStatus, records, kind, phrase }) => { + const step = deployNextStep({ + ...base, + domainStatus, + pendingDnsRecords: base.pendingDnsRecords.slice(0, records), + }); + expect(step.kind).toBe(kind); + if (step.kind === "records_available" || step.kind === "records_unavailable") { + expect(step.records).toBe(phrase); + expect(step.domainsUrl).toBe(base.urls.domains); + } + }); + + test("the agent sentence is rendered from the step the report classifies", () => { + const report = buildDeployStatusReport({ kind: "not_started" }, null); + expect(report.nextAction).toBe(agentNextAction(deployNextStep(report))); + }); }); diff --git a/packages/cli-core/src/commands/deploy/status.ts b/packages/cli-core/src/commands/deploy/status.ts index 86e6d3a20..341a9f430 100644 --- a/packages/cli-core/src/commands/deploy/status.ts +++ b/packages/cli-core/src/commands/deploy/status.ts @@ -14,10 +14,14 @@ import { import { sleep } from "../../lib/sleep.ts"; import { withSpinner, type SpinnerControls } from "../../lib/spinner.ts"; import { - cnameTargetPending, + pendingCnameTargets, deployComponentLabels, deployStatusRetryMessage, + capitalizeFirst, + classifyDomainPending, domainsDashboardUrl, + instanceDashboardUrl, + pendingRecordComponents, type DeployComponentStatus, } from "./copy.ts"; import { mapDeployError } from "./errors.ts"; @@ -61,17 +65,64 @@ export type DeployStatusState = // buildInterruptedDeployStatusReport. | "interrupted"; +/** + * Two values only. `deployNextStep` reads this back as booleans, so a third + * value would be silently classified as pending; the type makes adding one a + * compile error at every reader instead. + */ +export type DomainComponentState = "complete" | "pending"; + export interface DeployStatusReport { complete: boolean; state: DeployStatusState; domain: string | null; productionInstanceId: string | null; - domainStatus: { dns: string; ssl: string; mail: string } | null; - pendingDnsRecords: { type: "CNAME"; host: string; value: string }[]; + domainStatus: { + dns: DomainComponentState; + ssl: DomainComponentState; + mail: DomainComponentState; + } | null; + pendingDnsRecords: { type: "CNAME"; host: string; value: string; required: boolean }[]; oauth: { complete: boolean; configured: string[]; pending: string[]; unsupported: string[] }; + /** + * Dashboard pages for this deploy: the production instance and its Domains + * page. Null before a production instance exists, and when the run was + * interrupted before the state could be read. + */ + urls: { domains: string; instance: string } | null; + /** + * Derived, never written: every constructor goes through `withNextAction`, + * which renders this from `deployNextStep` over the other fields. Assigning + * it directly would let it drift from the facts it describes. + */ nextAction: string; } +/** + * What the report tells its reader to do next, as data. The agent's + * `nextAction` sentence and the human-mode line are both rendered from this, + * so neither audience's wording is derived from the other's: a reword on one + * side can't leak the other side's phrasing. + */ +export type DeployNextStep = + | { kind: "not_started" } + | { kind: "domain_provisioning"; domainsUrl: string | null } + | { kind: "interrupted" } + | { + kind: "complete"; + domain: string; + oauthUnsupported: readonly string[]; + instanceUrl: string | null; + } + | { kind: "oauth_pending"; oauthPending: readonly string[]; oauthUnsupported: readonly string[] } + | { + kind: "records_available" | "records_unavailable" | "ssl_pending" | "finalizing"; + domain: string; + /** "DNS", "Email DNS", or "DNS and email DNS": the record kinds still unverified. */ + records: string; + domainsUrl: string | null; + }; + export type LiveDeploySnapshot = Omit< DeployOperationState, "pending" | "oauthProviders" | "completedOAuthProviders" @@ -303,7 +354,7 @@ export function pendingDomainStatus(): DomainStatusResponse { }; } -function domainComponentState(value: boolean): "complete" | "pending" { +function domainComponentState(value: boolean): DomainComponentState { return value ? "complete" : "pending"; } @@ -311,6 +362,19 @@ export function buildDeployStatusReport( state: DeployState, outcome: DeployStatusOutcome | null, ): DeployStatusReport { + return withNextAction(buildDeployStatusFacts(state, outcome)); +} + +type DeployStatusFacts = Omit; + +function withNextAction(facts: DeployStatusFacts): DeployStatusReport { + return { ...facts, nextAction: agentNextAction(deployNextStep(facts)) }; +} + +function buildDeployStatusFacts( + state: DeployState, + outcome: DeployStatusOutcome | null, +): DeployStatusFacts { if (state.kind === "not_started") { return { complete: false, @@ -320,16 +384,11 @@ export function buildDeployStatusReport( domainStatus: null, pendingDnsRecords: [], oauth: { complete: false, configured: [], pending: [], unsupported: [] }, - nextAction: - "No production instance yet. `clerk deploy` configures production interactively and " + - "needs a human terminal, ask the user to run `clerk deploy`, then run `clerk deploy status` to verify.", + urls: null, }; } if (state.kind === "domain_provisioning") { - const domainsAction = domainSettingsNextAction( - domainsDashboardUrl(state.appId, state.productionInstanceId), - ); return { complete: false, state: "domain_provisioning", @@ -338,10 +397,7 @@ export function buildDeployStatusReport( domainStatus: null, pendingDnsRecords: [], oauth: { complete: false, configured: [], pending: [], unsupported: [] }, - nextAction: - "A production instance exists but its domain is still provisioning. " + - "Run `clerk deploy status` again shortly, or ask the user to finish `clerk deploy`. " + - domainsAction, + urls: dashboardUrls(state.appId, state.productionInstanceId), }; } @@ -356,9 +412,12 @@ export function buildDeployStatusReport( const reportState = resolveActiveReportState(domainComplete, complete); const pendingDnsRecords: DeployStatusReport["pendingDnsRecords"] = !domainComplete - ? (snapshot.cnameTargets ?? []) - .filter((target) => cnameTargetPending(target, componentStatus)) - .map((target) => ({ type: "CNAME" as const, host: target.host, value: target.value })) + ? pendingCnameTargets(snapshot.cnameTargets ?? [], componentStatus).map((target) => ({ + type: "CNAME" as const, + host: target.host, + value: target.value, + required: target.required, + })) : []; return { @@ -378,15 +437,16 @@ export function buildDeployStatusReport( pending: oauthPending, unsupported: [...snapshot.unsupportedOAuthProviders], }, - nextAction: deployNextAction( - reportState, - snapshot.domain, - componentStatus, - oauthPending, - snapshot.productionInstanceId - ? domainsDashboardUrl(snapshot.appId, snapshot.productionInstanceId) - : null, - ), + urls: snapshot.productionInstanceId + ? dashboardUrls(snapshot.appId, snapshot.productionInstanceId) + : null, + }; +} + +function dashboardUrls(appId: string, productionInstanceId: string): DeployStatusReport["urls"] { + return { + domains: domainsDashboardUrl(appId, productionInstanceId), + instance: instanceDashboardUrl(appId, productionInstanceId), }; } @@ -400,7 +460,7 @@ export function buildDeployStatusReport( * rather than the empty output this path used to produce. */ export function buildInterruptedDeployStatusReport(): DeployStatusReport { - return { + return withNextAction({ complete: false, state: "interrupted", domain: null, @@ -408,10 +468,8 @@ export function buildInterruptedDeployStatusReport(): DeployStatusReport { domainStatus: null, pendingDnsRecords: [], oauth: { complete: false, configured: [], pending: [], unsupported: [] }, - nextAction: - "Interrupted before the deploy status could be read, so nothing is known about this " + - "deploy. Run `clerk deploy status` again to check it.", - }; + urls: null, + }); } function resolveActiveReportState(domainComplete: boolean, complete: boolean): DeployStatusState { @@ -420,48 +478,142 @@ function resolveActiveReportState(domainComplete: boolean, complete: boolean): D return "oauth_pending"; } -function deployNextAction( - state: DeployStatusState, - domain: string, - componentStatus: DeployComponentStatus, - oauthPending: string[], - domainsUrl: string | null, -): string { - const domainsAction = domainsUrl ? ` ${domainSettingsNextAction(domainsUrl)}` : ""; - - if (state === "complete") { - return `Production is deployed and verified at https://${domain}. No action needed.${domainsAction}`; - } - if (state === "oauth_pending") { - return ( - `Domain verified, but these OAuth providers are missing production credentials: ` + - `${oauthPending.join(", ")}. Ask the user to finish \`clerk deploy\`, then run \`clerk deploy status\`.` + - domainsAction - ); - } - - const pendingComponents = [ - !componentStatus.dns ? "DNS" : null, - !componentStatus.ssl ? "SSL" : null, - !componentStatus.mail ? "email DNS" : null, - ].filter((value): value is string => value !== null); - - if (pendingComponents.length === 0) { - return ( - `Production setup for ${domain} is still finalizing on Clerk's side. ` + - `Re-run \`clerk deploy status\` in a few minutes.${domainsAction}` - ); +/** + * Classify what the reader should do next from the report's own fields, so + * the human line rendered from a report and the agent sentence stored in it + * always describe the same situation. + */ +export function deployNextStep(report: DeployStatusFacts): DeployNextStep { + switch (report.state) { + case "not_started": + return { kind: "not_started" }; + case "interrupted": + return { kind: "interrupted" }; + case "domain_provisioning": + // Always has a production instance, so always has its URLs; nullable + // only because the report type can't say so. + return { kind: "domain_provisioning", domainsUrl: report.urls?.domains ?? null }; + case "complete": + return { + kind: "complete", + domain: report.domain ?? "", + oauthUnsupported: report.oauth.unsupported, + instanceUrl: report.urls?.instance ?? null, + }; + case "oauth_pending": + return { + kind: "oauth_pending", + oauthPending: report.oauth.pending, + oauthUnsupported: report.oauth.unsupported, + }; + case "domain_pending": { + // DNS and email DNS are records someone has to add at the registrar; + // SSL is Clerk's side and waits on them. Polling can't move the first + // kind along, so those get "add the records" and only SSL gets "wait". + const status: DeployComponentStatus = { + dns: report.domainStatus?.dns === "complete", + ssl: report.domainStatus?.ssl === "complete", + mail: report.domainStatus?.mail === "complete", + }; + return { + kind: classifyDomainPending(status, report.pendingDnsRecords.length > 0), + domain: report.domain ?? "", + records: capitalizeFirst(pendingRecordComponents(status)), + domainsUrl: report.urls?.domains ?? null, + }; + } } - - return ( - `${pendingComponents.join(", ")} still provisioning for ${domain}. ` + - `Re-run \`clerk deploy status\` in a few minutes, DNS propagation can take time.` + - domainsAction - ); } -function domainSettingsNextAction(domainsUrl: string): string { - return `Ask the user to visit the Clerk Dashboard domains page, or offer to open it: ${domainsUrl}`; +/** The `nextAction` sentence: written for an agent that will relay it to a person. */ +export function agentNextAction(step: DeployNextStep): string { + // In development Clerk supplies shared OAuth credentials; in production it + // doesn't, so a provider the CLI couldn't configure has a sign-in button + // that fails for real users. `oauth.complete` only covers what the CLI + // manages, so the report has to say this out loud. + const unsupported = (providers: readonly string[]): string => + providers.length > 0 + ? ` These providers are enabled in development but the CLI could not configure them for ` + + `production: ${providers.join(", ")}. Configure them in the Clerk Dashboard before ` + + `going live, or users signing in with them will fail.` + : ""; + const domains = (url: string | null): string => + url + ? ` Ask the user to visit the Clerk Dashboard domains page, or offer to open it: ${url}` + : ""; + + switch (step.kind) { + case "not_started": + return ( + "No production instance yet. `clerk deploy` configures production interactively and " + + "needs a human terminal, ask the user to run `clerk deploy`, then run `clerk deploy status` to verify." + ); + case "domain_provisioning": + return ( + "A production instance exists but its domain is still provisioning. " + + "Run `clerk deploy status` again shortly, or ask the user to finish `clerk deploy`." + + domains(step.domainsUrl) + ); + case "interrupted": + return ( + "Interrupted before the deploy status could be read, so nothing is known about this " + + "deploy. Run `clerk deploy status` again to check it." + ); + case "complete": + // Complete on Clerk's side only. The app keeps running on development + // keys until the production keys reach the host, and the report can't + // tell whether that already happened — hence "if you haven't already". + // Nothing is left to monitor on the Domains page here, so the pointer is + // the instance itself (users, settings, billing) rather than the shared + // "visit the domains page" clause every pending state carries. + return ( + `Clerk's production setup for https://${step.domain} is verified. If you haven't already: ` + + `run \`clerk env pull --instance prod\`, set those keys on your host alongside the other ` + + `Clerk variables from your env file, redeploy, then sign up at https://${step.domain} to confirm.` + + unsupported(step.oauthUnsupported) + + (step.instanceUrl + ? ` Manage users, settings, and billing for this instance: ${step.instanceUrl}` + : "") + ); + case "oauth_pending": + // The domain is verified, so there is nothing to monitor on the Domains + // page; the wizard is the only way to supply credentials. + return ( + `Domain verified, but these OAuth providers are missing production credentials: ` + + `${step.oauthPending.join(", ")}. Ask the user to finish \`clerk deploy\`, then run \`clerk deploy status\`.` + + unsupported(step.oauthUnsupported) + ); + case "records_available": + return ( + `${step.records} records not found yet for ${step.domain}. ` + + `Add the records in \`pendingDnsRecords\` at the domain's DNS provider if you haven't already, ` + + `then re-run \`clerk deploy status --wait\`. Propagation usually takes minutes.` + + domains(step.domainsUrl) + ); + case "records_unavailable": + // The report has nothing to hand over; the Dashboard clause carries the + // URL, so this sentence doesn't repeat it. + return ( + `${step.records} records not found yet for ${step.domain}, but this report has no record list. ` + + `Find the records to add on the Domains page in the Clerk Dashboard, then re-run ` + + `\`clerk deploy status --wait\`.` + + domains(step.domainsUrl) + ); + case "ssl_pending": + // Records are verified; the certificate is Clerk's side and nobody can + // speed it up. Same message the wizard's footer prints for this state. + return ( + `SSL certificate still pending for ${step.domain}. Clerk issues it automatically now that ` + + `DNS is verified; re-run \`clerk deploy status\` in a few minutes.` + + domains(step.domainsUrl) + ); + case "finalizing": + return ( + `Production setup for ${step.domain} is still finalizing on Clerk's side. ` + + `Re-run \`clerk deploy status\` in a few minutes.` + + domains(step.domainsUrl) + ); + } } export async function loadProductionDomain( diff --git a/packages/cli-core/src/commands/open/index.ts b/packages/cli-core/src/commands/open/index.ts index c1ae8a91b..8489d565b 100644 --- a/packages/cli-core/src/commands/open/index.ts +++ b/packages/cli-core/src/commands/open/index.ts @@ -2,7 +2,7 @@ import { createArgument } from "@commander-js/extra-typings"; import type { Program } from "../../cli-program.ts"; import { resolveProfile } from "../../lib/config.ts"; import { CliError, ERROR_CODE } from "../../lib/errors.ts"; -import { getDashboardUrl } from "../../lib/environment.ts"; +import { buildDashboardUrl } from "../../lib/environment.ts"; import { openBrowser } from "../../lib/open.ts"; import { log } from "../../lib/log.ts"; import { bold, cyan, dim } from "../../lib/color.ts"; @@ -16,17 +16,9 @@ interface OpenOptions { print?: boolean; } -/** - * Build the dashboard deep-link URL for the linked app's instance. - * Exported for tests and reuse. - */ -export function buildDashboardUrl(appId: string, instanceId: string, subpath?: string): string { - const host = getDashboardUrl().replace(/\/$/, ""); - const base = `${host}/apps/${appId}/instances/${instanceId}`; - if (!subpath) return base; - const cleaned = subpath.replace(/^\//, "").replace(/\/$/, ""); - return cleaned ? `${base}/${cleaned}` : base; -} +// Re-exported for existing importers; the implementation lives in +// lib/environment.ts next to the host resolution it depends on. +export { buildDashboardUrl }; export async function openDashboard( subpath: string | undefined, diff --git a/packages/cli-core/src/lib/environment.ts b/packages/cli-core/src/lib/environment.ts index 1031c9cad..e8bba3dd6 100644 --- a/packages/cli-core/src/lib/environment.ts +++ b/packages/cli-core/src/lib/environment.ts @@ -152,6 +152,19 @@ export function getDashboardUrl(): string { ); } +/** + * Dashboard deep link for one instance of an app, optionally to a page under + * it (`domains`, `users`, ...). Lives here rather than in the `open` command + * so copy modules can build links without importing a command's module graph. + */ +export function buildDashboardUrl(appId: string, instanceId: string, subpath?: string): string { + const host = getDashboardUrl().replace(/\/$/, ""); + const base = `${host}/apps/${appId}/instances/${instanceId}`; + if (!subpath) return base; + const cleaned = subpath.replace(/^\//, "").replace(/\/$/, ""); + return cleaned ? `${base}/${cleaned}` : base; +} + /** * Remote MCP server URL for the active environment. * diff --git a/packages/cli-core/src/lib/wrap.test.ts b/packages/cli-core/src/lib/wrap.test.ts new file mode 100644 index 000000000..e5fd2c679 --- /dev/null +++ b/packages/cli-core/src/lib/wrap.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; +import { cyan } from "./color.ts"; +import { visibleWidth, wrap } from "./wrap.ts"; + +describe("wrap", () => { + test("keeps a short line as one line", () => { + expect(wrap("Short enough.")).toEqual(["Short enough."]); + }); + + test("breaks on spaces so no line exceeds the width", () => { + const text = "one two three four five six seven eight nine ten eleven twelve"; + const lines = wrap(text, { width: 20 }); + expect(lines.every((line) => line.length <= 20)).toBe(true); + expect(lines.join(" ")).toBe(text); + }); + + test("gives a bullet a hanging indent on continuation lines", () => { + const lines = wrap(" - Add them at your DNS provider if you haven't already.", { width: 30 }); + expect(lines[0]).toBe(" - Add them at your DNS"); + expect( + lines.slice(1).every((line) => line.startsWith(" ") && !line.startsWith(" ")), + ).toBe(true); + }); + + test("uses an explicit hang for a label prefix", () => { + const lines = wrap("NOTE DNS records usually propagate within minutes.", { + width: 32, + hang: 6, + }); + expect(lines[0]).toBe("NOTE DNS records usually"); + expect(lines[1]).toBe(" propagate within minutes."); + }); + + test("measures width without color codes", () => { + expect(visibleWidth(cyan("example.com"))).toBe("example.com".length); + const lines = wrap( + `Clerk will use these subdomains for ${cyan("example.com")}. More words here.`, + { + width: 50, + }, + ); + expect(lines.every((line) => visibleWidth(line) <= 50)).toBe(true); + expect(lines[0]).toContain(cyan("example.com")); + }); + + test("never splits a token wider than the width", () => { + const url = "https://dashboard.clerk.com/apps/app_1/instances/ins_prod/domains"; + const lines = wrap(`See ${url} for details.`, { width: 20 }); + expect(lines).toContain(url); + }); +}); diff --git a/packages/cli-core/src/lib/wrap.ts b/packages/cli-core/src/lib/wrap.ts new file mode 100644 index 000000000..1dc3b9353 --- /dev/null +++ b/packages/cli-core/src/lib/wrap.ts @@ -0,0 +1,49 @@ +const ANSI_ESCAPE_PATTERN = new RegExp(String.raw`\x1b\[[0-9;]*m`, "g"); + +/** Columns a string occupies on screen; color escape codes don't count. */ +export function visibleWidth(text: string): number { + return text.replace(ANSI_ESCAPE_PATTERN, "").length; +} + +/** + * Word-wrap one paragraph to `width` visible columns. + * + * The wizard prefixes each line it prints with a gutter, and the terminal's + * own soft wrap doesn't know about it: the wrapped remainder lands at column + * zero and breaks the frame. So prose is wrapped here, before printing. 76 + * leaves room for the gutter on an 80-column terminal, which is what the + * hand-wrapped text in the deploy module already assumes. + * + * Leading indentation and a bullet or number marker (" - ", " 1. ") turn + * into a hanging indent on continuation lines; `hang` overrides that for + * lines led by a label such as "NOTE ". A token wider than the room left, + * such as a URL, is never split; it overflows on its own line instead. + */ +export function wrap(text: string, options: { width?: number; hang?: number } = {}): string[] { + const width = options.width ?? 76; + const lead = /^(\s*)((?:[-*]|\d+\.)\s+)?/.exec(text.replace(ANSI_ESCAPE_PATTERN, ""))?.[0] ?? ""; + const hang = " ".repeat(options.hang ?? lead.length); + + const lines: string[] = []; + let line = ""; + let lineWidth = 0; + let empty = true; + for (const word of text.split(" ")) { + const wordWidth = visibleWidth(word); + if (!empty && lineWidth + 1 + wordWidth > width) { + lines.push(line.trimEnd()); + line = hang; + lineWidth = hang.length; + empty = true; + } + if (!empty) { + line += " "; + lineWidth += 1; + } + line += word; + lineWidth += wordWidth; + empty = false; + } + lines.push(line.trimEnd()); + return lines; +}