From 07ea80b406617a76282929f636bd654cb3b0b5ec Mon Sep 17 00:00:00 2001 From: Michael Novotny Date: Wed, 29 Jul 2026 20:14:07 -0500 Subject: [PATCH 1/2] fix(users): confirm before creating a user `users create` registers `--yes` as "Skip confirmation prompt" and its `setExamples` recommend passing it, but no prompt existed. The flag was declared on `CreateUserOptions` and never read, so the user was created immediately in every mode. The surrounding code already assumed a prompt: the `catch` handles `UserAbortError` and `isPromptExitError`, neither of which anything inside the `try` could throw. Nearly every existing test threads `yes: true` through to the BAPI call. This wires up the gate those were written against. Human mode now prints the redacted request body and confirms before the POST, matching `clerk api`, `config push`, `unlink`, and `impersonate`. Agent mode is untouched: `isHuman()` is false there, so it never prompts and `--dry-run` stays the safety net. Chose this over dropping the flag because `--help` and the shipped examples have been telling people the prompt exists. Removing `--yes` would break the documented invocations; adding the prompt makes them correct. Co-Authored-By: Claude Opus 5 --- .../users-create-confirmation-prompt.md | 5 ++ .../src/commands/users/create.test.ts | 69 ++++++++++++++++++- .../cli-core/src/commands/users/create.ts | 18 ++++- 3 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 .changeset/users-create-confirmation-prompt.md diff --git a/.changeset/users-create-confirmation-prompt.md b/.changeset/users-create-confirmation-prompt.md new file mode 100644 index 000000000..13832f68b --- /dev/null +++ b/.changeset/users-create-confirmation-prompt.md @@ -0,0 +1,5 @@ +--- +"clerk": patch +--- + +Make `clerk users create` confirm before writing. The command registers `--yes` as "Skip confirmation prompt" and its examples recommend passing it, but no prompt existed, so the flag did nothing and the user was created immediately. Human mode now previews the redacted request body and asks before the POST. Agent mode is unchanged: it never prompts, so `--dry-run` remains the safety net there. diff --git a/packages/cli-core/src/commands/users/create.test.ts b/packages/cli-core/src/commands/users/create.test.ts index 7e6707068..67957b7b8 100644 --- a/packages/cli-core/src/commands/users/create.test.ts +++ b/packages/cli-core/src/commands/users/create.test.ts @@ -1,6 +1,6 @@ import { test, expect, describe, beforeEach, afterEach, mock, spyOn } from "bun:test"; import { useCaptureLog } from "../../test/lib/stubs.ts"; -import { BapiError, CliError, ERROR_CODE, EXIT_CODE } from "../../lib/errors.ts"; +import { BapiError, CliError, ERROR_CODE, EXIT_CODE, UserAbortError } from "../../lib/errors.ts"; const mockResolveBapiSecretKey = mock(); const mockHandleBapiError = mock((_error: unknown) => false); @@ -27,6 +27,11 @@ mock.module("./create-wizard.ts", () => ({ runCreateWizard: (...args: unknown[]) => mockRunCreateWizard(...args), })); +const mockConfirm = mock(); +mock.module("../../lib/prompts.ts", () => ({ + confirm: (...args: unknown[]) => mockConfirm(...args), +})); + mock.module("../../lib/spinner.ts", () => ({ intro: () => {}, outro: () => {}, @@ -46,6 +51,7 @@ describe("users create", () => { mockIsAgent.mockReturnValue(false); mockResolveBapiSecretKey.mockResolvedValue("sk_test_123"); mockRunCreateWizard.mockResolvedValue({ fields: {}, targeting: {} }); + mockConfirm.mockResolvedValue(true); mockBapiRequest.mockResolvedValue({ status: 200, headers: new Headers(), @@ -64,6 +70,7 @@ describe("users create", () => { mockBapiRequest.mockReset(); mockIsAgent.mockReset(); mockRunCreateWizard.mockReset(); + mockConfirm.mockReset(); logSpy.mockRestore(); errorSpy.mockRestore(); }); @@ -260,4 +267,64 @@ describe("users create", () => { expect(mockResolveBapiSecretKey).not.toHaveBeenCalled(); expect(mockBapiRequest).not.toHaveBeenCalled(); }); + + test("confirms before creating in human mode and redacts sensitive preview fields", async () => { + await runCreate({ + app: "app_123", + email: "alice@example.com", + password: "Password123!", + }); + + expect(mockConfirm).toHaveBeenCalledWith({ message: "Proceed?" }); + expect(captured.err).toContain("About to POST /v1/users"); + expect(captured.err).toContain("[REDACTED]"); + expect(captured.err).not.toContain("Password123!"); + expect(mockBapiRequest).toHaveBeenCalled(); + }); + + test("aborts without calling BAPI when the confirmation is declined", async () => { + mockConfirm.mockResolvedValue(false); + + const error = await runCreate({ + app: "app_123", + email: "alice@example.com", + }).catch((caught) => caught); + + expect(error).toBeInstanceOf(UserAbortError); + expect(mockBapiRequest).not.toHaveBeenCalled(); + }); + + test("skips the confirmation when --yes is passed", async () => { + await runCreate({ + app: "app_123", + email: "alice@example.com", + yes: true, + }); + + expect(mockConfirm).not.toHaveBeenCalled(); + expect(mockBapiRequest).toHaveBeenCalled(); + }); + + test("never confirms in agent mode, with or without --yes", async () => { + mockIsAgent.mockReturnValue(true); + + await runCreate({ + app: "app_123", + email: "alice@example.com", + }); + + expect(mockConfirm).not.toHaveBeenCalled(); + expect(mockBapiRequest).toHaveBeenCalled(); + }); + + test("does not confirm on --dry-run", async () => { + await runCreate({ + app: "app_123", + email: "alice@example.com", + dryRun: true, + }); + + expect(mockConfirm).not.toHaveBeenCalled(); + expect(mockBapiRequest).not.toHaveBeenCalled(); + }); }); diff --git a/packages/cli-core/src/commands/users/create.ts b/packages/cli-core/src/commands/users/create.ts index e8540c888..475f1e302 100644 --- a/packages/cli-core/src/commands/users/create.ts +++ b/packages/cli-core/src/commands/users/create.ts @@ -1,5 +1,10 @@ import { handleBapiError, resolveBapiSecretKey } from "../../lib/bapi-command.ts"; -import { UserAbortError, isPromptExitError, throwUsageError } from "../../lib/errors.ts"; +import { + UserAbortError, + isPromptExitError, + throwUsageError, + throwUserAbort, +} from "../../lib/errors.ts"; import { isInsideGutter, log } from "../../lib/log.ts"; import { buildCreateUserPayload, @@ -9,6 +14,7 @@ import { redactUsersDisplayPayload, } from "../../lib/users.ts"; import { isAgent, isHuman } from "../../mode.ts"; +import { confirm } from "../../lib/prompts.ts"; import { bapiRequest } from "../../lib/bapi.ts"; import { withSpinner, intro, outro, pausedOutro } from "../../lib/spinner.ts"; import { handleUsersBapiError, printUsersMutationResult } from "./output.ts"; @@ -62,6 +68,16 @@ export async function create(options: CreateUserOptions): Promise { if (shouldWrap) intro("Creating user"); try { + if (isHuman() && !resolved.yes) { + log.info("\nAbout to POST /v1/users"); + log.blank(); + log.info(JSON.stringify(redactUsersDisplayPayload(payload), null, 2)); + const ok = await confirm({ message: "Proceed?" }); + if (!ok) { + throwUserAbort(); + } + } + const response = await withSpinner("Creating user...", () => bapiRequest({ method: "POST", From 118643893cda36ae737f25369f86ed410b66d56b Mon Sep 17 00:00:00 2001 From: Michael Novotny Date: Wed, 29 Jul 2026 20:14:17 -0500 Subject: [PATCH 2/2] docs(skills): point audit-clerk-skill at clerk/skills The audit that keeps the `clerk-cli` skill in sync with this binary targets `skills/clerk-cli/SKILL.md`. #315 moved the skill out to clerk/skills, so that path has not existed here since. The audit has been checking nothing, which is how the skill drifted into promising confirmation prompts and a guidance-only agent login that neither exist. Retarget it at a `$SKILL_ROOT` clone of clerk/skills, and drop the `{{CLI_VERSION}}` and `clerk skill install` references that went with the bundled copy. Co-Authored-By: Claude Opus 5 --- .claude/skills/audit-clerk-skill/SKILL.md | 32 ++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/.claude/skills/audit-clerk-skill/SKILL.md b/.claude/skills/audit-clerk-skill/SKILL.md index 8c5781e35..116f43673 100644 --- a/.claude/skills/audit-clerk-skill/SKILL.md +++ b/.claude/skills/audit-clerk-skill/SKILL.md @@ -1,6 +1,6 @@ --- name: audit-clerk-skill -description: Audits the Clerk CLI source tree and proposes updates to the bundled `clerk-cli` skill so it stays in sync with the binary. Use when the user says "audit the clerk-cli skill", "update the clerk-cli skill", "check the skill against the code", "resync clerk-cli skill", or after adding/renaming/removing CLI commands, flags, or agent-mode behavior. +description: Audits the Clerk CLI source tree and proposes updates to the `clerk-cli` skill in clerk/skills so it stays in sync with the binary. Use when the user says "audit the clerk-cli skill", "update the clerk-cli skill", "check the skill against the code", "resync clerk-cli skill", or after adding/renaming/removing CLI commands, flags, or agent-mode behavior. effort: high user-invocable: true disable-model-invocation: true @@ -11,15 +11,23 @@ metadata: # Audit the clerk-cli Skill -Cross-check `skills/clerk-cli/` against the actual CLI source in `packages/cli-core/` and propose precise edits wherever they have drifted. The binary is the source of truth; the skill is documentation that must track it. +Cross-check the `clerk-cli` skill against the actual CLI source in `packages/cli-core/` and propose precise edits wherever they have drifted. The binary is the source of truth; the skill is documentation that must track it. + +**The skill lives in another repo.** #315 removed the bundled copy, so it is now `skills/core/clerk-cli/` in [clerk/skills](https://github.com/clerk/skills), versioned independently of the CLI. This audit reads the CLI here and proposes edits there; they land as a PR against clerk/skills. Set `$SKILL_ROOT` to a local clone before starting, and stop if there is not one: + +```sh +SKILL_ROOT=../skills/skills/core/clerk-cli # adjust to the checkout +ls "$SKILL_ROOT/SKILL.md" || echo "clone clerk/skills first" +``` + +Every skill path below is relative to `$SKILL_ROOT`. **ultrathink** on this task. It requires building a full command tree, comparing two representations of it, and making judgment calls about what belongs in the skill vs. in `references/*.md` vs. in `--help`. Shallow passes miss drift. ## Inputs - **Source of truth**: `packages/cli-core/src/commands/**` (one directory per top-level command), plus `packages/cli-core/src/cli.ts`, `cli-program.ts`, `mode.ts`, and anything in `packages/cli-core/src/lib/` referenced by commands (runner preference, agent mode, doctor checks, key resolution). -- **Target**: `skills/clerk-cli/SKILL.md` and `skills/clerk-cli/references/*.md`. -- **Template markers**: the skill uses `{{CLI_VERSION}}` placeholders substituted at install time by `clerk skill install`. Preserve them; do not expand. +- **Target**: `$SKILL_ROOT/SKILL.md` and `$SKILL_ROOT/references/*.md`. ## Workflow @@ -39,7 +47,7 @@ Read the per-command `README.md` (`packages/cli-core/src/commands//README. - Flag commands or sub-paths marked mocked/stubbed (blockquote at top of the README). The skill should not document these as production-ready. - Cross-check Clerk API endpoint claims in `references/recipes.md`. -Do **not** propose bundling the READMEs into `skills/clerk-cli/references/` (symlinks or text imports). They ship internal detail agents do not need, and inflate the compiled binary. They are a reference for the audit, not for the skill. +Do **not** propose bundling the READMEs into `$SKILL_ROOT/references/` (symlinks or text imports). They ship internal detail agents do not need, and inflate the compiled binary. They are a reference for the audit, not for the skill. Also capture cross-cutting behavior: @@ -52,7 +60,7 @@ Don't memorize output. Prefer reading the source directly over running the binar ### 2. Extract the skill's current claims -Read `skills/clerk-cli/SKILL.md` and each file under `skills/clerk-cli/references/`. Extract every concrete claim: +Read `$SKILL_ROOT/SKILL.md` and each file under `$SKILL_ROOT/references/`. Extract every concrete claim: - Every command mentioned in the "Core commands at a glance" table and the Invoking-the-CLI table. - Every flag called out by name. @@ -94,25 +102,25 @@ If a new reference file is warranted (e.g. a `references/commands.md` table), pr Emit a review-ready proposal. For each change: -- Path (`skills/clerk-cli/SKILL.md` or `skills/clerk-cli/references/.md`). +- Path (`$SKILL_ROOT/SKILL.md` or `$SKILL_ROOT/references/.md`). - Why (source citation: `packages/cli-core/src/commands//.ts:`). - Either a unified diff (preferred) or a before/after block for prose sections. - Severity: `drift` (factually wrong today), `gap` (missing coverage), `polish` (clearer wording, better placement, or cuts that route the agent to `--help` instead of duplicating it). -Group the proposal by file. Do **not** touch `{{CLI_VERSION}}` markers. Do **not** rewrite sections that are still accurate just because they are near a change. +Group the proposal by file. Do **not** rewrite sections that are still accurate just because they are near a change. ### 6. Apply or hand back Default: present the proposal and stop. The user reviews, then says apply. -If invoked as `/audit-clerk-skill --apply`, apply `drift` and `gap` edits directly but still list `polish` suggestions for review. Run `bun run format` after applying so markdown tables and frontmatter match repo style. +If invoked as `/audit-clerk-skill --apply`, apply `drift` and `gap` edits directly to `$SKILL_ROOT` but still list `polish` suggestions for review. Edits land in the clerk/skills clone, not this repo, so commit them on a branch there and open a PR against clerk/skills. Nothing in this repo changes. ## Guardrails - **Never invent flags.** If a flag appears in a test but not in the command's argument parser, treat it as test-only and flag it for human review. - **Preserve voice.** The existing skill is terse and third-person; match it. No first- or second-person drift. - **No em-dashes** anywhere in proposals (repo style rule). -- **Respect the template.** `{{CLI_VERSION}}` stays verbatim. The Invoking-the-CLI runner table is generated from `preferredRunner` logic; if that logic changes, update the table, otherwise leave it alone. +- **Respect the template.** The Invoking-the-CLI runner table is generated from `preferredRunner` logic; if that logic changes, update the table, otherwise leave it alone. - **Stay within the 500-line guidance** for `SKILL.md`. When the budget is tight, move content to `references/` rather than deleting it outright. ## Output shape @@ -125,13 +133,13 @@ Return the proposal as: ## Summary -## skills/clerk-cli/SKILL.md +## $SKILL_ROOT/SKILL.md ###
- [drift|gap|polish] - source: packages/cli-core/src/commands/<...>: - -## skills/clerk-cli/references/.md +## $SKILL_ROOT/references/.md ... ## New files (if any)