feat(cli-core): add clerk enable and clerk disable commands for orgs and billing#219
feat(cli-core): add clerk enable and clerk disable commands for orgs and billing#219nicolas-angelo wants to merge 10 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: e4669c9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds CLI commands to enable/disable Organizations and Billing on a linked Clerk instance: Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 28 minutes and 33 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/billing/index.ts`:
- Around line 109-120: Validate numeric CLI flags before assigning to plan: for
options.amount, options.annualAmount, and options.trialDays parseInt results
must be checked for NaN and integer-ness (e.g., const amt =
parseInt(options.amount,10); if (Number.isNaN(amt) || !Number.isFinite(amt))
return error/exit), and only then set plan.amount, plan.annual_monthly_amount,
and plan.free_trial_days; for trialDays also ensure it's a non-negative integer
and set plan.free_trial_enabled true only when validation passes. Apply the same
checks for the other numeric flags referenced later (the block noted at lines
~201-210) so no invalid or partial numeric input (like "12abc" → 12 or NaN) is
sent to the API.
In `@packages/cli-core/src/commands/orgs/index.ts`:
- Around line 20-24: The patch payload construction uses
parseInt(options.maxMembers, 10) which allows invalid or non-positive values
(NaN or truncated numbers) to be sent to patchInstanceConfig; validate
options.maxMembers before adding to patch: ensure it's present, is an integer
string with no trailing chars, and is > 0 (e.g., via Number.isInteger(parsed)
and parsed > 0) and only then set patch.max_allowed_memberships to the parsed
integer; if validation fails, return/throw a clear user-facing error or CLI exit
indicating the --max-members value is invalid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b25373c-146e-4858-b48e-3f4ebe70879f
📒 Files selected for processing (9)
.changeset/billing-orgs-shortcut-commands.mdREADME.mdpackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/billing/README.mdpackages/cli-core/src/commands/billing/index.test.tspackages/cli-core/src/commands/billing/index.tspackages/cli-core/src/commands/orgs/README.mdpackages/cli-core/src/commands/orgs/index.test.tspackages/cli-core/src/commands/orgs/index.ts
| .option("--auto-create", "Auto-create an organization for new users") | ||
| .option("--max-members <n>", "Maximum members per organization") | ||
| .option("--domains", "Enable verified domains") | ||
| .option("--yes", "Skip confirmation prompts") |
There was a problem hiding this comment.
--yes is advertised on orgs enable/disable, billing enable/disable, and plans create/update/remove, but no implementation consults options.yes or calls confirm(). The commands always mutate silently. Users who see --yes will reasonably assume there is a prompt to skip.
Compare with config patch, which these shortcuts wrap: packages/cli-core/src/commands/config/push.ts:94-102 does if (!options.dryRun && isHuman() && !options.yes) { await confirm(...) }. Without an equivalent branch here, the shortcut is strictly more dangerous than the command it's supposed to replace.
Fix: either remove --yes and the yes?: boolean fields until a prompt exists, or add the real confirmation branch for every mutating command. Option two is the right call, these are production config edits.
| `Removing plan ${cyan(slug)} on ${ctx.appLabel} (${ctx.instanceLabel})...`, | ||
| () => | ||
| withApiContext( | ||
| patchInstanceConfig(ctx.appId, ctx.instanceId, config, { destructive: true }), |
There was a problem hiding this comment.
plansRemove fetches the current config, deletes the slug in memory, and PATCHes with { destructive: true }. No prompt, no diff, no dry-run. A typo in <slug> or the active instance silently wipes a live subscription plan.
The equivalent clerk config patch path would print a diff, warn, and ask "Proceed?". The shortcut regresses this safety.
Fix: gate the mutation behind isHuman() && !options.yes confirmation, support --dry-run, and show the plan (name, price, payer) being removed before confirming.
| .argument("<slug>", "Plan slug (display name auto-derived via title case)") | ||
| .option("--name <name>", "Override display name") | ||
| .requiredOption("--amount <cents>", "Monthly price in cents") | ||
| .addOption(createOption("--payer <type>", "Who pays").choices(["org", "user"])) |
There was a problem hiding this comment.
Line 527 uses .requiredOption("--amount <cents>", …). Line 528 uses .addOption(createOption("--payer <type>", "Who pays").choices(["org", "user"])) with no .makeOptionMandatory(). packages/cli-core/src/commands/billing/README.md:18 claims --payer is (required).
When omitted, options.payer === undefined, so billing/index.ts:110 writes payer_type: undefined. JSON.stringify drops the field, sending a malformed plan to the API.
Fix:
.addOption(
createOption("--payer <type>", "Who pays")
.choices(["org", "user"])
.makeOptionMandatory(),
)Add a test covering the missing---payer rejection.
| .description("Create a subscription plan") | ||
| .argument("<slug>", "Plan slug (display name auto-derived via title case)") | ||
| .option("--name <name>", "Override display name") | ||
| .requiredOption("--amount <cents>", "Monthly price in cents") |
There was a problem hiding this comment.
patchInstanceConfig() accepts dryRun (see lib/plapi.ts:156-184). clerk config patch exposes --dry-run at cli-program.ts:367. None of the new shortcuts plumb it through.
A maintainer who wants to preview billing enable --for org or plans remove pro on production has to fall back to the raw config patch command, defeating the point of the wrappers. These are the exact operations you most want to dry-run.
Fix: add --dry-run to every mutating subcommand and pass { dryRun: options.dryRun } into patchInstanceConfig, matching config/push.ts:107-115.
|
|
||
| const plan: Record<string, unknown> = { | ||
| name: options.name || titleCase(slug), | ||
| amount: parseInt(options.amount, 10), |
There was a problem hiding this comment.
Every numeric option (--amount, --max-members, --trial-days, --annual-amount) goes through raw parseInt(value, 10) with no validation, here and at billing/index.ts:117, 120, 201, 204, 209, plus orgs/index.ts:23.
parseInt("abc", 10) returns NaN, which JSON.stringify emits as null. So clerk billing plans create pro --amount abc --payer org sends amount: null to the API, and --max-members abc sends max_allowed_memberships: null. Neither failure mode is user-friendly.
Fix: use Commander's option argParser at the registration site:
.option("--amount <cents>", "Monthly price in cents", (v) => {
const n = Number.parseInt(v, 10);
if (!Number.isFinite(n) || n < 0) {
throwUsageError(`Invalid --amount: "${v}". Must be a non-negative integer.`);
}
return n;
})Or extract a shared parseNonNegativeInt helper.
|
|
||
| const billing = current.billing as Record<string, unknown> | undefined; | ||
| if (billing?.organization_enabled) { | ||
| log.warn( |
There was a problem hiding this comment.
orgsDisable fetches billing config and calls log.warn("…Disabling organizations will also disable org billing."), then unconditionally issues the PATCH.
Emitting a warning and then doing the thing the warning describes is worse than not warning at all, in a CI log it looks like the warning was heeded. If the dependency is worth detecting (it is), it's worth gating behind confirmation.
Fix: when billing.organization_enabled is true, require explicit confirmation in human mode or a --force / --yes flag to proceed. In non-human mode, consider failing with a usage error directing the user to pass --force.
|
|
||
| const config = { billing: patch }; | ||
|
|
||
| const result = await withSpinner( |
There was a problem hiding this comment.
config push.ts:75-92 fetches the current config, computes a diff with hasConfigChanges / printDiff, prints it, then prompts. The shortcut commands send blind.
If confirmation is added per the critical-priority findings, pair it with the diff, otherwise the prompt just says "Proceed?" with no context about what's about to change.
Fix: reuse hasConfigChanges / printDiff from config/push.ts. They already handle partial-payload patch mode, which is what these wrappers generate.
|
|
||
| log.data(JSON.stringify(result, null, 2)); | ||
| log.success(`Billing enabled for ${target === "org" ? "organizations" : "users"}`); | ||
| } |
There was a problem hiding this comment.
log.data(JSON.stringify(result, null, 2)) emits the full server response on stdout after every mutation, here and at billing/index.ts:80, 128, 229, 267, plus orgs/index.ts:41, 73.
Per .claude/rules/logging.md, log.data is stdout for pipeable output. config patch does this because the user opted into a low-level config operation. For clerk orgs enable the user asked a single-intent question, piping a ~200-line config blob out every time is noise and will trip up shell pipelines.
Fix: demote to log.debug(...) for routine success, or emit only when --json / --verbose is set. Keep the log.success(...) line as the human-facing signal.
| const patch: Record<string, unknown> = { enabled: true }; | ||
| if (options.forceSelection) patch.force_organization_selection = true; | ||
| if (options.domains) patch.domains_enabled = true; | ||
| if (options.maxMembers) patch.max_allowed_memberships = parseInt(options.maxMembers, 10); |
There was a problem hiding this comment.
if (options.forceSelection) patch.force_organization_selection = true;
if (options.domains) patch.domains_enabled = true;
if (options.autoCreate) { … enabled: true }There's no --no-force-selection or --force-selection=false. To turn a flag off while keeping orgs enabled, the user has to fall back to clerk config patch, partially defeating the wrapper.
Fix: either (a) document in the README that these flags are one-way and point users to config patch for the inverse, or (b) accept Commander's boolean negation (--no-force-selection) and write false when it's set.
| const currency = (plan.currency as string) || "usd"; | ||
| const price = amount === 0 ? "Free" : `${(amount / 100).toFixed(2)} ${currency.toUpperCase()}`; | ||
| const payer = plan.payer_type as string; | ||
| const visible = plan.publicly_visible !== false; |
There was a problem hiding this comment.
cyan(plan.name as string) casts without checking. If the config contains a plan with no name field (legacy or malformed), output reads undefined (slug) — $X.XX/mo ….
Fix: cyan((plan.name as string) ?? slug).
…/disable commands Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rtcut-commands # Conflicts: # README.md
nicolas-angelo
changed the title
…rtcut-commands # Conflicts: # README.md
| } | ||
|
|
||
| async function offerBillingSkillInstall(options: BillingOptions): Promise<void> { | ||
| const skipPrompt = options.yes === true || isAgent(); |
There was a problem hiding this comment.
offerBillingSkillInstall runs even when applyConfigPatch returns early ("No changes detected"). On an already-configured instance, every clerk enable billing invocation in agent mode will silently attempt to install clerk-billing again since isAgent() skips the confirmation prompt.
Consider having applyConfigPatch return a boolean so you can gate the skill offer:
| const skipPrompt = options.yes === true || isAgent(); | |
| const applied = await applyConfigPatch({ | |
| ctx, | |
| payload, | |
| verb: `Enabling billing for ${describeTargets(targets)}`, | |
| successMessage: `Billing enabled for ${describeTargets(targets)}`, | |
| failureContext: "Failed to enable billing", | |
| yes: options.yes, | |
| dryRun: options.dryRun, | |
| }); | |
| // `clerk init` doesn't bundle clerk-billing -- it's opt-in. Surface it here. | |
| if (applied && !options.dryRun && options.skills !== false) { | |
| await offerBillingSkillInstall(options); | |
| } | |
| if (applied && !options.dryRun) printNextSteps(NEXT_STEPS.ENABLE_BILLING); |
(with applyConfigPatch returning false on the early "No changes" exit)
| if (!dryRun && isHuman() && !yes) { | ||
| if (warning) log.warn(warning); | ||
| const ok = await confirm({ message: "Proceed?" }); | ||
| if (!ok) throwUserAbort(); |
There was a problem hiding this comment.
The warning is only printed inside the !dryRun && isHuman() && !yes block. When orgsDisable calls this with --yes (or in agent mode with --yes), the warning about stranded billing is silently dropped and the PATCH fires with no indication.
Consider moving the warning emission before the confirmation gate so it's always visible:
| if (!ok) throwUserAbort(); | |
| if (warning) log.warn(warning); | |
| if (!dryRun && isHuman() && !yes) { | |
| const ok = await confirm({ message: "Proceed?" }); | |
| if (!ok) throwUserAbort(); | |
| } |
3 participants
Summary
This PR adds two top-level commands —
clerk enableandclerk disable— for toggling Clerk features on the linked instance. The toggle layer is intentionally narrow: simple on/off plus a few discoverable convenience flags. Anything beyond that continues to flow throughclerk config patch.What changed
clerk enable orgs/clerk disable orgs: toggle organizations.enableaccepts--force-selection,--auto-create,--max-members <n>, and--domainsfor the most common settings.clerk enable billing/clerk disable billing: toggle billing for organizations and/or users.--fordefaults to both targets when omitted; enabling fororgcascades to enabling organizations.enable billing, offers to install theclerk-billingskill fromclerk/skills(suppress with--no-skills).clerk initdoesn't bundle this one as a default — billing is opt-in — soenable billingis the natural moment to surface it.clerk config schema --keys <section>(what's tunable) andclerk config pull --keys <section>(what's currently set) for deeper exploration. Skipped in agent mode.Usage
Organizations
Billing
Before / After
Before (manual JSON):
After (convenience toggles):