feat(keyless): make keyless the default and let agents configure a keyless instance#395
feat(keyless): make keyless the default and let agents configure a keyless instance#395rafa-thayto wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 92f9de8 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 |
📝 WalkthroughWalkthroughThis change introduces unified keyless/account target resolution across configuration workflows and adds Backend API reads and writes for unclaimed applications. Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…e a keyless instance Restores keyless mode as the default for unauthenticated `clerk init` (reversing PR #268), and extends keyless so an agent can bootstrap and configure Clerk end to end without a Clerk account. Init: - Unauthenticated runs on keyless-capable frameworks use keyless again — human bootstrap and all agent runs. Existing-project human runs still log in. - `--login` forces the authenticated flow; `--keyless` now forces keyless even when signed in. `--keyless --login`, `--keyless --app`, and agent `--login` while signed out are usage errors. - `--template <b2b-saas|b2c-saas|native|waitlist>` pre-configures the keyless application at creation. Operating a keyless instance (Backend API, instance secret key only): - `clerk config pull` and `clerk config patch` cover seven resource groups. - `clerk enable/disable orgs`, `clerk whoami`, and `clerk env pull` work with no account. Billing stays account-only and says why. - Keys are also discovered from `.clerk/.tmp/keyless.json`, so an application a Clerk SDK minted for itself is reachable from the CLI. Account credentials are deliberately not part of the keyless decision: only `--app` or a linked profile selects the account path, so these commands work with or without a platform API key or login session.
ee5482e to
92f9de8
Compare
![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
🧹 Nitpick comments (4)
packages/cli-core/src/commands/orgs/README.md (1)
65-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the keyless BAPI endpoint to the reference table.
The endpoints table only lists the two account-mode (PLAPI) endpoints, omitting the
PATCH /v1/instance/organization_settingsBAPI endpoint that the keyless-mode section above (Line 9) already documents. As per path instructions, flagging because this table is a reference/audit surface where an incomplete endpoint list is a real, if small, correctness gap.📝 Proposed table addition
| Method | Endpoint | Description | | ------ | ------------------------------------------------------------------ | -------------------------------------------------------------------------- | | GET | `/v1/platform/applications/{appId}/instances/{instanceId}/config` | Fetch current config for diff and the org-billing dependency check | | PATCH | `/v1/platform/applications/{appId}/instances/{instanceId}/config` | Patch `organization_settings` (with `?dry_run=true` when `--dry-run` set) | +| PATCH | `/v1/instance/organization_settings` | Keyless mode: patch org settings directly on the unclaimed application |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/orgs/README.md` around lines 65 - 71, Update the “Clerk API endpoints” reference table in the README to include the keyless BAPI PATCH /v1/instance/organization_settings endpoint, using a description that reflects its organization_settings patch behavior. Keep the existing PLAPI endpoint rows unchanged.Source: Path instructions
packages/cli-core/src/commands/config/io.ts (1)
58-83: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
writeInstanceConfigsilently drops--destructivefor keyless targets; PUT/PATCH invariant relies on caller discipline.
writeInstanceConfig's keyless branch ignoresoptions.methodandoptions.destructive, trusting comments-only invariants ("PUT is rejected before reaching here", "payload was validated before the diff was shown") instead of enforcing them. This surfaces today in push.ts:clerk config patch --destructiveagainst a keyless target silently no-ops the flag with no warning, and any future caller that skipsassertPayloadWritableor passesmethod: "PUT"for a keyless target would get an unvalidated payload sent to BAPI or a silent PATCH instead of the expected error.
packages/cli-core/src/commands/config/io.ts#L58-L83: havewriteInstanceConfigwarn (or throw) whendestructiveis set for a keyless target, and/or assertoptions.method === "PATCH"before callingpatchKeylessConfig, rather than relying solely on caller-side comments.packages/cli-core/src/commands/config/push.ts#L119-L126: until the above is added, consider warning the user (or strippingdestructive) whentarget.kind === "keyless"before this call, since the flag currently has no effect but no feedback either.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/config/io.ts` around lines 58 - 83, Update writeInstanceConfig in packages/cli-core/src/commands/config/io.ts (lines 58-83) to enforce keyless-target invariants: reject non-PATCH methods and explicitly handle destructive instead of silently dropping it before calling patchKeylessConfig. Update packages/cli-core/src/commands/config/push.ts (lines 119-126) to provide user feedback or strip destructive for keyless targets; this site may be made redundant if the centralized validation fully handles the flag.packages/cli-core/src/commands/whoami/index.ts (1)
76-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap the BAPI call with
withApiContextfor consistent error messaging.
describeKeylesscallsbapiRequestdirectly, unlike the analogous keyless-aware calls added elsewhere in this PR (e.g.,schema.ts'sfetchInstanceConfigSchema,env/pull.ts'sfetchApplication), which wrap the network call inwithApiContextfor a friendly failure message. A raw network failure here (revoked key, connectivity issue) will surface without that context.♻️ Proposed fix
async function describeKeyless(keyless: KeylessTarget): Promise<Identity> { const instance = await withSpinner("Fetching instance info...", async () => { - const response = await bapiRequest({ - method: "GET", - path: "/v1/instance", - secretKey: keyless.secretKey, - }); + const response = await withApiContext( + bapiRequest({ method: "GET", path: "/v1/instance", secretKey: keyless.secretKey }), + "Failed to fetch instance info", + ); return response.body as { id?: string; environment_type?: string }; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/whoami/index.ts` around lines 76 - 93, Update describeKeyless to wrap its bapiRequest call for /v1/instance with withApiContext, matching the established keyless-aware request pattern and preserving the existing response handling and identity mapping.packages/cli-core/src/commands/init/strategy.test.ts (1)
248-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest isolation relies on a manual workaround instead of fixing the root cause.
Both tests re-mock
config.resolveProfilespecifically to guard against "leakage from earlier tests that spy on resolveProfile ... but don't track those spies for restoration" (per the inline comment). That's an order-dependent test suite — untracked ad hocspyOncalls elsewhere can bleed into unrelated tests, and the defensive re-mock only patches the symptom for these two cases.Consider having tests consistently pipe ad hoc
spyOncalls through the harness'strack()helper (or wrappingspyOnin the harness to auto-track), soafterEachrestores everything and defensive re-mocking becomes unnecessary.Also applies to: 274-292
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/init/strategy.test.ts` around lines 248 - 259, Fix test isolation in the harness by ensuring ad hoc spyOn calls, including config.resolveProfile spies used by neighboring tests, are registered through the harness track() helper or automatically tracked by its spy wrapper so afterEach restores them. Then remove the defensive config.resolveProfile re-mocks from the authenticated-flow tests at lines 248-259 and 274-292, preserving their intended setup and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/commands/config/README.md`:
- Around line 204-213: Update the “API Endpoints (keyless mode)” table to
include the documented routes for the four omitted groups: communication,
protect, oauth_application_settings, and instance_settings. Keep their endpoint
paths, readability values, and coverage descriptions consistent with the
corresponding Payload shape entries and keyless.test.ts behavior.
In `@packages/cli-core/src/commands/init/index.ts`:
- Around line 160-184: Update assertUsableFlags to reject options.template
combined with options.app, using the same usage-error pattern as the existing
--keyless/--app validation. Ensure template requests are not silently discarded
when runStrategy selects a non-keyless path; preserve the existing
--template/--login validation and, if strategy selection is adjusted, make
pickStrategy or runStrategy explicitly warn or force keyless when a template is
provided without --keyless.
---
Nitpick comments:
In `@packages/cli-core/src/commands/config/io.ts`:
- Around line 58-83: Update writeInstanceConfig in
packages/cli-core/src/commands/config/io.ts (lines 58-83) to enforce
keyless-target invariants: reject non-PATCH methods and explicitly handle
destructive instead of silently dropping it before calling patchKeylessConfig.
Update packages/cli-core/src/commands/config/push.ts (lines 119-126) to provide
user feedback or strip destructive for keyless targets; this site may be made
redundant if the centralized validation fully handles the flag.
In `@packages/cli-core/src/commands/init/strategy.test.ts`:
- Around line 248-259: Fix test isolation in the harness by ensuring ad hoc
spyOn calls, including config.resolveProfile spies used by neighboring tests,
are registered through the harness track() helper or automatically tracked by
its spy wrapper so afterEach restores them. Then remove the defensive
config.resolveProfile re-mocks from the authenticated-flow tests at lines
248-259 and 274-292, preserving their intended setup and assertions.
In `@packages/cli-core/src/commands/orgs/README.md`:
- Around line 65-71: Update the “Clerk API endpoints” reference table in the
README to include the keyless BAPI PATCH /v1/instance/organization_settings
endpoint, using a description that reflects its organization_settings patch
behavior. Keep the existing PLAPI endpoint rows unchanged.
In `@packages/cli-core/src/commands/whoami/index.ts`:
- Around line 76-93: Update describeKeyless to wrap its bapiRequest call for
/v1/instance with withApiContext, matching the established keyless-aware request
pattern and preserving the existing response handling and identity mapping.
🪄 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 Plus
Run ID: 45807ed9-c06a-4293-a40a-630c1b79b426
📒 Files selected for processing (28)
.changeset/keyless-mode-default.mdpackages/cli-core/src/commands/billing/README.mdpackages/cli-core/src/commands/billing/index.tspackages/cli-core/src/commands/config/README.mdpackages/cli-core/src/commands/config/apply-patch.tspackages/cli-core/src/commands/config/io.tspackages/cli-core/src/commands/config/keyless.test.tspackages/cli-core/src/commands/config/keyless.tspackages/cli-core/src/commands/config/pull.tspackages/cli-core/src/commands/config/push.tspackages/cli-core/src/commands/config/schema.tspackages/cli-core/src/commands/env/README.mdpackages/cli-core/src/commands/env/pull.tspackages/cli-core/src/commands/init/README.mdpackages/cli-core/src/commands/init/heuristics.tspackages/cli-core/src/commands/init/index.test.tspackages/cli-core/src/commands/init/index.tspackages/cli-core/src/commands/init/strategy.test.tspackages/cli-core/src/commands/orgs/README.mdpackages/cli-core/src/commands/orgs/index.tspackages/cli-core/src/commands/whoami/README.mdpackages/cli-core/src/commands/whoami/index.tspackages/cli-core/src/lib/credential-store.tspackages/cli-core/src/lib/keyless-target.tspackages/cli-core/src/lib/keyless.tspackages/cli-core/src/test/integration/lib/harness.tspackages/cli-core/src/test/lib/init-harness.tspackages/cli-core/src/test/lib/stubs.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
| | Top-level key | Endpoint | Readable | Covers | | ||
| | ---------------------------- | ----------------------------------------- | -------- | ----------------------------------------------------------------------------------- | | ||
| | `instance` | `/v1/instance` | Yes | Support email, home URL, allowed origins | | ||
| | `communication` | `/v1/instance/communication` | Yes | Blocked country codes and communication settings | | ||
| | `restrictions` | `/v1/instance/restrictions` | No | Allowlist / blocklist sign-up restrictions | | ||
| | `organization_settings` | `/v1/instance/organization_settings` | Yes | Organizations: enabled, membership limits, domains, creation defaults | | ||
| | `protect` | `/v1/instance/protect` | Yes | Bot protection | | ||
| | `oauth_application_settings` | `/v1/instance/oauth_application_settings` | Yes | Dynamic OAuth client registration | | ||
| | `instance_settings` | `/v1/beta_features/instance_settings` | No | `test_mode`, `progressive_sign_up`, `from_email_address`, `restricted_to_allowlist` | | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
API Endpoints table is missing rows for 4 of 7 documented groups.
The Payload shape table lists communication, protect, oauth_application_settings, and instance_settings as supported/readable groups, and keyless.test.ts exercises GET/PATCH routes for all of them, but the "API Endpoints (keyless mode)" table only documents /v1/instance, /v1/instance/restrictions, and /v1/instance/organization_settings. Readers relying on this table alone will miss the other supported routes.
📝 Proposed additional rows
| Method | Endpoint | Description |
| ------- | ------------------------------------ | ------------------------------------------------------------------------------------------ |
| `GET` | `/v1/instance` | Reads instance settings for `config pull` and for the pre-write diff. |
| `PATCH` | `/v1/instance` | Updates instance settings. Answers `204` with no body, so the group is re-read afterwards. |
| `PATCH` | `/v1/instance/restrictions` | Updates sign-up restrictions (allowlist, blocklist). Write-only — no read route exists. |
+| `GET` | `/v1/instance/communication` | Reads communication settings for `config pull` and for the pre-write diff. |
+| `PATCH` | `/v1/instance/communication` | Updates communication settings. |
| `GET` | `/v1/instance/organization_settings` | Reads organization settings for `config pull` and for the pre-write diff. |
| `PATCH` | `/v1/instance/organization_settings` | Updates organization settings. |
+| `GET` | `/v1/instance/protect` | Reads bot-protection settings for `config pull` and for the pre-write diff. |
+| `PATCH` | `/v1/instance/protect` | Updates bot-protection settings. |
+| `GET` | `/v1/instance/oauth_application_settings` | Reads dynamic OAuth client registration settings. |
+| `PATCH` | `/v1/instance/oauth_application_settings` | Updates dynamic OAuth client registration settings. |
+| `PATCH` | `/v1/beta_features/instance_settings` | Updates `test_mode`, `progressive_sign_up`, `from_email_address`, `restricted_to_allowlist`. Beta route; write-only. |Also applies to: 234-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli-core/src/commands/config/README.md` around lines 204 - 213,
Update the “API Endpoints (keyless mode)” table to include the documented routes
for the four omitted groups: communication, protect, oauth_application_settings,
and instance_settings. Keep their endpoint paths, readability values, and
coverage descriptions consistent with the corresponding Payload shape entries
and keyless.test.ts behavior.
| /** | ||
| * Rejects flag combinations that can't both be honoured, before anything is | ||
| * bootstrapped on disk. `--keyless` and `--template` describe an application the | ||
| * CLI creates; `--login` and `--app` describe one that already exists. | ||
| */ | ||
| async function assertUsableFlags(options: InitOptions, agent: boolean): Promise<void> { | ||
| if (options.keyless && options.login) { | ||
| throwUsageError("--keyless and --login cannot be combined."); | ||
| } | ||
| if (options.keyless && options.app) { | ||
| throwUsageError( | ||
| "--keyless cannot be combined with --app. Drop --keyless to link the app, or drop --app to use temporary development keys.", | ||
| ); | ||
| } | ||
| if (options.template && options.login) { | ||
| throwUsageError( | ||
| "--template applies to keyless applications and cannot be combined with --login.", | ||
| ); | ||
| } | ||
| if (options.login && agent && !(await isAuthenticated())) { | ||
| throwUsageError( | ||
| "--login requires an interactive terminal to complete the browser login. Ask the user to run `clerk auth login`, then re-run `clerk init`.", | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--template is silently ignored whenever the resolved strategy isn't "keyless".
assertUsableFlags only rejects --template combined with --login (Line 174-178), but options.template is dropped with no warning whenever pickStrategy resolves to "authenticate" — e.g. --template b2b-saas --app app_123, or plain --template b2b-saas while already authenticated (CLERK_PLATFORM_API_KEY set or stored session) with no --keyless. runStrategy (Line 323-338) only consumes template in the "keyless" case, so the user's explicit intent is silently discarded — risky for agent scripts that assume the template was applied (e.g., expecting orgs enabled per the docs) but get a default app instead.
At minimum, mirror the existing --keyless+--app guard for --template+--app.
🐛 Proposed fix to reject the --template + --app combination
if (options.template && options.login) {
throwUsageError(
"--template applies to keyless applications and cannot be combined with --login.",
);
}
+ if (options.template && options.app) {
+ throwUsageError(
+ "--template applies to keyless applications and cannot be combined with --app.",
+ );
+ }The implicit-authenticated-drop case (no --keyless, no --app, but already logged in) is harder to guard upfront without extra I/O, but consider having setupKeylessApp/runStrategy warn (or pickStrategy account for template when deciding whether to force keyless) rather than silently ignoring it.
Also applies to: 317-327
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli-core/src/commands/init/index.ts` around lines 160 - 184, Update
assertUsableFlags to reject options.template combined with options.app, using
the same usage-error pattern as the existing --keyless/--app validation. Ensure
template requests are not silently discarded when runStrategy selects a
non-keyless path; preserve the existing --template/--login validation and, if
strategy selection is adjusted, make pickStrategy or runStrategy explicitly warn
or force keyless when a template is provided without --keyless.
1 participant
What
Restores keyless mode as the default for unauthenticated
clerk init(reversing #268), then extends keyless so an agent can bootstrap and configure Clerk end to end without a Clerk account — no login, noCLERK_PLATFORM_API_KEY.No backend changes required. Every route used here was verified live against
api.clerk.devwith an unclaimed keyless application'ssk_test_key before being written against.Init
--keyless--login--template <b2b-saas|b2c-saas|native|waitlist>Usage errors:
--keyless --login,--keyless --app,--template --login, and agent-mode--loginwhile signed out (agents can't complete browser OAuth).-ymeans confirmations only — it neither forces nor bypasses keyless.Operating a keyless instance
clerk config pull/patchfall back to the Backend API using the instance secret key. The payload names BAPI resources 1:1 rather than translating the Platform API's document shape:clerk config patch --json '{"instance":{"support_email":"dev@acme.com"},"organization_settings":{"enabled":true}}'Groups:
instance,communication,restrictions,organization_settings,protect,oauth_application_settings,instance_settings. Any other key is a usage error naming the supported ones.Also now account-free:
clerk enable/disable orgs,clerk whoami(reports the instance identity),clerk env pull(writes locally-held keys). Keys are additionally discovered from.clerk/.tmp/keyless.json, the file Clerk SDKs write when they mint their own application — previously invisible to the CLI.Account credentials are deliberately not part of the keyless decision. Only
--appor a linked profile selects the account path, so these work with or without a platform key/login. When credentials do exist and the directory simply isn't linked, the command still runs and warns that linking gives the full configuration.Known limits (documented, not worked around)
auth_requiredexplanation. RFC drafted separately.restrictionsandinstance_settingsare write-only (no GET route).GET /v1/instancereturns fewer fields thanPATCHaccepts, so the pre-write diff is best-effort; the write is unaffected.--dry-runpreviews locally and sends nothing.Test plan
bun run format:check,bun run lint,bun run typecheck— cleanbun run test— 1947 pass, 0 failenable orgs(membership limit change confirmed via Frontend API),whoami,env pullon an SDK-created app with no.env,--template b2b-saas(orgs enabled on creation vs. off without it), and every guard rail<SignIn />widget rendered live from a keyless appNotes for reviewers
commands/config/io.tsover a typedInstanceTarget; no command branches on it directly.init/index.test.tscrossed 1000 lines, so it was split intoindex.test.ts(plumbing) andstrategy.test.ts(strategy selection), with the shared spy harness extracted tosrc/test/lib/init-harness.ts.main; the diff is clean against the merge base but a rebase before merge is reasonable.