feat(keyless): make keyless the default and let agents configure a keyless instance - #395
Open
rafa-thayto wants to merge 8 commits into
Open
feat(keyless): make keyless the default and let agents configure a keyless instance#395rafa-thayto wants to merge 8 commits into
rafa-thayto wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: 21f9572 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds unified account/keyless target resolution and Backend API configuration support for unclaimed applications. Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
rafa-thayto
force-pushed
the
rafa-thayto/keyless-mode-default
branch
from
July 24, 2026 21:20
ee5482e to
92f9de8
Compare
rafa-thayto
marked this pull request as draft
July 24, 2026 21:26
![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)
rafa-thayto
changed the title
rafa-thayto
marked this pull request as ready for review
July 29, 2026 14:14
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/cli-core/src/commands/users/open.test.ts (1)
87-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFixture error shape doesn't match production.
resolveAppContextrejecting with a plainErrorshort-circuitsresolveKnownUserDashboardTargetimmediately. In production it throwsCliErrorwithERROR_CODE.NOT_LINKED, which that function swallows and then continues intoresolveUsersInstanceContext— so the failure that actually reachesopen()'s catch (and thusassertNotUnclaimedKeyless) comes from a different call. Using the real error shape keeps the test honest if that fall-through ever changes.♻️ Suggested fixture
- mockResolveAppContext.mockRejectedValue( - new Error("No Clerk project linked to this directory."), - ); + mockResolveAppContext.mockRejectedValue( + new CliError("No Clerk project linked to this directory.", { + code: ERROR_CODE.NOT_LINKED, + }), + );Also confirm
resolveUsersInstanceContextis stubbed to fail in this suite so the keyless branch is reached the same way it is at runtime.🤖 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/users/open.test.ts` around lines 87 - 98, Update the unclaimed keyless application fixture in the relevant open test to reject mockResolveAppContext with the production CliError using ERROR_CODE.NOT_LINKED, so resolveKnownUserDashboardTarget falls through correctly. Verify that mockResolveUsersInstanceContext is also stubbed to fail, ensuring open() reaches the keyless branch and assertNotUnclaimedKeyless through the same runtime path.packages/cli-core/src/commands/open/keyless-claim.ts (1)
90-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for keyless-instance BAPI lookup. Both sites independently fetch
/v1/instancewith the keyless secret key and map the response into aKeylessInstanceInfo-shaped object, but with inconsistent field naming (instanceIdvsid) and slightly different error handling (raw error casting vs the sharederrorMessage()helper).
packages/cli-core/src/commands/open/keyless-claim.ts#L90-L99: replacedescribeKeylessInstance()'s body with a call to a shared helper (e.g. moved tolib/keyless-target.tsor a newlib/keyless-instance.ts) so both call sites share one implementation and one field-naming convention.packages/cli-core/src/commands/doctor/context.ts#L62-L85: havegetKeylessInstance()call the same shared helper instead of re-implementing thebapiRequest/mapping/try-catch logic inline.🤖 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/open/keyless-claim.ts` around lines 90 - 99, Create a shared keyless-instance lookup helper that performs the /v1/instance request, maps fields consistently, and uses errorMessage() for failures. Update describeKeylessInstance() in packages/cli-core/src/commands/open/keyless-claim.ts:90-99 and getKeylessInstance() in packages/cli-core/src/commands/doctor/context.ts:62-85 to delegate to that helper, removing their duplicated request, mapping, and try-catch logic.packages/cli-core/src/commands/config/keyless.ts (1)
259-281: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLeaf verification is key-order sensitive for object-valued array elements.
collectVerifiedLeavescompares non-object leaves withJSON.stringify(value) === JSON.stringify(observedValue). This works fine for primitives and arrays of primitives, but if a sent array ever contains objects (e.g. a future or other group's field shaped like[{a: 1, b: 2}]), a semantically-identical value returned with keys in a different order would serialize differently and get misreported as adroppedField— a false "this wasn't applied" warning for a write that actually succeeded. None of today'sINSTANCE_FIELDShit this (arrays here are string arrays), but this function runs across every payload group, not justinstance.♻️ Suggested fix
Use a deep-equality comparison instead of relying on JSON.stringify's key-order sensitivity, e.g. via a small recursive
deepEqualhelper (or a well-known utility) for the leaf-array case.🤖 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/keyless.ts` around lines 259 - 281, Update collectVerifiedLeaves to compare leaf values with deep structural equality rather than JSON.stringify, including arrays containing objects whose key order differs. Add or reuse a recursive deepEqual-style helper and use it for the matched calculation, while preserving the existing handling of primitives, objects, and array leaves.
🤖 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 `@docs/keyless-demos/lib/demo-env.sh`:
- Line 101: Update the retry failure logging around the keyless key-minting flow
to remove the raw $response from the stderr message. Emit only a sanitized
failure message, optionally including a non-sensitive HTTP status, so secret_key
or other credential material cannot appear in recordings or CI logs.
- Around line 86-88: Bound the curl request in the accountless application flow
by adding both a connection timeout and an overall request timeout to the curl
invocation inside the retry loop. Keep the existing response capture and
retry/fail-loudly behavior unchanged, ensuring stalled DNS, TLS, or HTTP
connections return control to the loop.
In `@docs/keyless-demos/tapes/09-health-and-users.tape`:
- Around line 31-34: Update the `clerk open --print` step in the
health-and-users tape to avoid displaying the claim token. Record only a
redacted claim URL or its origin/path while preserving the command’s intended
demonstration flow.
In `@packages/cli-core/src/commands/open/README.md`:
- Around line 41-45: Add the text language specifier to the fenced code block
containing the unclaimed-application message in the README, preserving the
message content and formatting.
In `@packages/cli-core/src/lib/bapi-command.ts`:
- Around line 21-33: Update describeBapiTarget to handle options.secretKey
before calling resolveKeylessTarget, matching resolveBapiSecretKey’s
highest-precedence branch. Return a description for the explicitly supplied
secret key first, and only perform the keyless lookup when no secretKey option
is provided.
In `@packages/cli-core/src/lib/credential-store.ts`:
- Around line 305-319: Update isUnrecoverableRefreshFailure to exclude HTTP 429
from the terminal 4xx range, preserving it as a transient refresh failure while
continuing to classify other unrecoverable 4xx responses as expired sessions.
---
Nitpick comments:
In `@packages/cli-core/src/commands/config/keyless.ts`:
- Around line 259-281: Update collectVerifiedLeaves to compare leaf values with
deep structural equality rather than JSON.stringify, including arrays containing
objects whose key order differs. Add or reuse a recursive deepEqual-style helper
and use it for the matched calculation, while preserving the existing handling
of primitives, objects, and array leaves.
In `@packages/cli-core/src/commands/open/keyless-claim.ts`:
- Around line 90-99: Create a shared keyless-instance lookup helper that
performs the /v1/instance request, maps fields consistently, and uses
errorMessage() for failures. Update describeKeylessInstance() in
packages/cli-core/src/commands/open/keyless-claim.ts:90-99 and
getKeylessInstance() in packages/cli-core/src/commands/doctor/context.ts:62-85
to delegate to that helper, removing their duplicated request, mapping, and
try-catch logic.
In `@packages/cli-core/src/commands/users/open.test.ts`:
- Around line 87-98: Update the unclaimed keyless application fixture in the
relevant open test to reject mockResolveAppContext with the production CliError
using ERROR_CODE.NOT_LINKED, so resolveKnownUserDashboardTarget falls through
correctly. Verify that mockResolveUsersInstanceContext is also stubbed to fail,
ensuring open() reaches the keyless branch and assertNotUnclaimedKeyless through
the same runtime path.
🪄 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: 2f290259-1834-46e7-a903-01fee15ea2b8
⛔ Files ignored due to path filters (6)
docs/keyless-demos/screenshots/01-signin-before.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/02-signup-before.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/03-orgs-enabled-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/04-orgs-disabled-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/05-signup-restricted-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/06-signup-allowed-after.pngis excluded by!**/*.png
📒 Files selected for processing (52)
.changeset/keyless-mode-default.mddocs/keyless-demos/.gitignoredocs/keyless-demos/README.mddocs/keyless-demos/lib/demo-env.shdocs/keyless-demos/record.shdocs/keyless-demos/screenshots/README.mddocs/keyless-demos/tapes/01-bootstrap.tapedocs/keyless-demos/tapes/02-read-config.tapedocs/keyless-demos/tapes/03-change-config.tapedocs/keyless-demos/tapes/04-toggles.tapedocs/keyless-demos/tapes/05-templates.tapedocs/keyless-demos/tapes/06-guardrails.tapedocs/keyless-demos/tapes/07-agent-mode.tapedocs/keyless-demos/tapes/08-sso-connections.tapedocs/keyless-demos/tapes/09-health-and-users.tapedocs/keyless-demos/tapes/_common.tapepackages/cli-core/src/commands/api/README.mdpackages/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/push.tspackages/cli-core/src/commands/doctor/README.mdpackages/cli-core/src/commands/doctor/checks.tspackages/cli-core/src/commands/doctor/context.test.tspackages/cli-core/src/commands/doctor/context.tspackages/cli-core/src/commands/doctor/doctor.test.tspackages/cli-core/src/commands/doctor/types.tspackages/cli-core/src/commands/env/README.mdpackages/cli-core/src/commands/env/pull.test.tspackages/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.tspackages/cli-core/src/commands/init/strategy.test.tspackages/cli-core/src/commands/open/README.mdpackages/cli-core/src/commands/open/index.test.tspackages/cli-core/src/commands/open/index.tspackages/cli-core/src/commands/open/keyless-claim.test.tspackages/cli-core/src/commands/open/keyless-claim.tspackages/cli-core/src/commands/users/README.mdpackages/cli-core/src/commands/users/open.test.tspackages/cli-core/src/commands/users/open.tspackages/cli-core/src/commands/whoami/README.mdpackages/cli-core/src/commands/whoami/index.test.tspackages/cli-core/src/commands/whoami/index.tspackages/cli-core/src/lib/bapi-command.test.tspackages/cli-core/src/lib/bapi-command.tspackages/cli-core/src/lib/credential-store.tspackages/cli-core/src/lib/keyless-target.test.tspackages/cli-core/src/lib/keyless-target.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)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/cli-core/src/commands/whoami/README.md
- .changeset/keyless-mode-default.md
- packages/cli-core/src/commands/config/push.ts
- packages/cli-core/src/commands/whoami/index.ts
- packages/cli-core/src/commands/init/index.ts
- packages/cli-core/src/commands/env/pull.ts
- packages/cli-core/src/commands/config/README.md
…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.
…an account Close the gaps between "keyless is the default" and "keyless actually works with no human intervention": - config patch validates instance fields against the Backend API's own schema before sending — PATCH /v1/instance answers 204 and silently discards unknown fields, so a typo or an account-only setting (password, social, second_factors, ...) used to print "Config pushed successfully" and change nothing. The refusal names the accepted fields and, for settings BAPI has no route for at all, says claiming is the only path. - config patch no longer prints a stale eventually-consistent re-read as the result of a write; bodyless groups are reported as unconfirmed. - allowlist_identifiers/blocklist_identifiers point at `clerk api` (which reaches them keyless) instead of demanding a login. - users open explains that an unclaimed app has no dashboard page and offers `clerk api /users/<id>`, instead of blaming "not linked" and suggesting remedies that need an application ID that doesn't exist yet. - whoami/env pull detect publishable/secret keys that belong to different applications; env pull refuses to write a mismatched pair. - doctor, open, users, api, env pull, whoami, enable/disable orgs and templates verified end to end on a live unclaimed instance, all under --mode agent with non-TTY stdin: no hangs, no prompts, no browser. docs/keyless-demos: before/after screenshots proving CLI writes reach the live hosted UI pre-claim (orgs step inserted into the sign-in flow, allowlist enforced then lifted), with a README on the FAPI /v1/environment oracle and its limits.
…r classification - demo-env.sh: bound the accountless-application curl request with connect/max timeouts, and stop echoing the raw minting response (it can contain a live secret_key when only one of the two fields fails to parse). - 09-health-and-users.tape: redact the claim token before it's recorded by piping `clerk open --print` through sed. - open/README.md: add a language to the fenced usage-error example (MD040). - bapi-command.ts: describeBapiTarget now checks options.secretKey first, mirroring resolveBapiSecretKey's actual precedence. - credential-store.ts: exclude 429 from isUnrecoverableRefreshFailure so a rate-limited token refresh isn't reported as an expired session.
rafa-thayto
force-pushed
the
rafa-thayto/keyless-mode-default
branch
from
July 30, 2026 12:17
d5df0c0 to
3d52adf
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
packages/cli-core/src/commands/config/io.ts (1)
74-104: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
writeInstanceConfigsilently ignoresoptions.methodfor keyless targets.The keyless branch always executes a PATCH via
patchKeylessConfig, regardless of whatoptions.methodis. The "PUT is rejected before reaching here" invariant is only enforced by callers today (configPushchecks it explicitly;applyConfigPatchhardcodes"PATCH"). If a future caller passesmethod: "PUT"with a keyless target directly to this function, it would silently perform a PATCH instead of failing loudly.♻️ Proposed defensive check
if (target.kind === "keyless") { + if (options.method === "PUT") { + throw new Error("writeInstanceConfig: PUT is not supported for a keyless target"); + } // PUT is rejected before reaching here: there is no full document to // replace when the instance is addressed by its own key. The payload was // validated by `assertPayloadWritable` before the diff was shown. const { applied, verification } = await patchKeylessConfig(🤖 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 74 - 104, Update writeInstanceConfig so the keyless branch validates options.method before calling patchKeylessConfig: reject method "PUT" explicitly with an error, while preserving PATCH behavior and the existing non-keyless flow.packages/cli-core/src/commands/config/keyless.test.ts (1)
650-662: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
requests.every(...)passes vacuously if nothing was requested.The dry-run assertion holds even when
requestsis empty, so it can't distinguish "only GETs" from "no calls at all". Addingexpect(requests.length).toBeGreaterThan(0)(or asserting the exact expected GET set) makes the intent explicit.🤖 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/keyless.test.ts` around lines 650 - 662, Strengthen the “patch --dry-run sends nothing” test by asserting that requests is non-empty before checking requests.every(...). Update the test around configPatch so it verifies at least one GET request occurred while preserving the existing dry-run log assertion.packages/cli-core/src/commands/init/index.ts (1)
204-227: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAgent
--loginruns validation twice.
assertUsableFlagscallsisAuthenticatedForAgent()(Line 207) andinitcalls it again at Line 118 for the same run, so an agent--logininvocation performs twogetValidToken()+fetchUserInforound-trips. Memoizing the result (or passing the already-computed value intopickStrategy) removes the redundant network call.🤖 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 204 - 227, Avoid duplicate authentication validation for agent-mode --login by reusing the result from assertUsableFlags instead of calling isAuthenticatedForAgent() again during init/pickStrategy. Memoize the validation or pass its computed boolean through the existing flow, preserving the unauthenticated guard behavior while ensuring getAuthenticatedEmail() runs only once per invocation.packages/cli-core/src/test/lib/init-harness.ts (1)
118-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrack the tracked spies and hold
gatherContextSpyexplicitly.
track()pushes the spy onto the original array beforesetup()reassignsspies, so those tracked spies are not restored for the current describe block. Keep existing tracked spies when building the fresh list instead. Also, BungetMockName()is set viamockName()for thespyOn()spied property name, so the name lookup is safe — but storing thegatherContextSpyin setup state avoids an extra lookup/re-creation entirely.🤖 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/test/lib/init-harness.ts` around lines 118 - 183, Update useInitHarness so setup preserves the existing spies array entries when constructing the new spy list, ensuring spies added through track are restored by afterEach. Hold gatherContextSpy in setup state rather than locating it through getMockName in setupBootstrapSuccess, and have that helper reuse the current setup spy without creating a fallback spy.packages/cli-core/src/commands/open/keyless-claim.ts (1)
95-98: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse the shared
errorMessage()helper instead of(error as Error).message.A non-
Errorrejection (string, undefined) would throw inside the catch block.errorMessagefrom../../lib/errors.tsalready handles this and is used elsewhere in this PR.🤖 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/open/keyless-claim.ts` around lines 95 - 98, Update the catch block in the keyless claim flow to use the shared errorMessage() helper from ../../lib/errors.ts when constructing the log message, replacing the unsafe (error as Error).message access while preserving the existing fallback return.packages/cli-core/src/commands/users/open.ts (1)
85-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
ERROR_CODE.NOT_LINKEDfor consistency with the keyless path inopen/index.ts.
openKeylessDashboard(packages/cli-core/src/commands/open/index.ts, lines 130-142) reports the same "unclaimed keyless, no dashboard page" state asNOT_LINKED, while this throwsINSTANCE_NOT_FOUND. Agents/scripts branching oncodewould see two different codes for one condition.🤖 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/users/open.ts` around lines 85 - 90, Update the error code in the unclaimed keyless application branch of open.ts to use ERROR_CODE.NOT_LINKED instead of ERROR_CODE.INSTANCE_NOT_FOUND, matching openKeylessDashboard in open/index.ts while preserving the existing message and error behavior.packages/cli-core/src/commands/open/index.ts (1)
153-169: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
instanceis only consumed by the agent and human branches, but it's resolved before theoptions.printearly return — adding a/v1/instancecall (plus secret-key discovery) to the scriptable path. Move the lookup below the print branch.♻️ Proposed reordering
const { url, source } = destination; + if (options.print) { + log.data(url); + return; + } + // Best-effort only: the claim link is already fully formed, so a bad or // missing secret key shouldn't block opening it — it just means the // output won't include instance details. const instance = await resolveKeylessTarget({ cwd }) .then((keyless) => (keyless ? describeKeylessInstance(keyless.secretKey) : null)) .catch(() => null); - // Output strategy mirrors the linked-app flow above: - // --print → plain URL on stdout (scriptable) - // agent mode → JSON object with full context (parseable) - // human mode → intro/outro logging flow with browser open - if (options.print) { - log.data(url); - return; - } + // Output strategy mirrors the linked-app flow above: + // agent mode → JSON object with full context (parseable) + // human mode → intro/outro logging flow with browser open🤖 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/open/index.ts` around lines 153 - 169, Move the resolveKeylessTarget/describeKeylessInstance lookup into the agent/human output path after the options.print early return, while preserving its existing best-effort error handling and leaving the plain URL print behavior unchanged.
🤖 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 `@docs/keyless-demos/record.sh`:
- Line 21: Require Bun before recording starts by adding a command -v bun
preflight alongside the existing VHS check in
docs/keyless-demos/record.sh:21-21. Document Bun as a required dependency in
docs/keyless-demos/README.md:14-14.
In `@docs/keyless-demos/tapes/03-change-config.tape`:
- Around line 26-30: Update the read-back command in the tape after the
configuration patch to pull the readable organization_settings group instead of
the write-only instance group, preserving the existing patch/read-back flow.
In `@packages/cli-core/src/commands/doctor/checks.ts`:
- Around line 90-94: Make claimHint() use a non-destructive keyless breadcrumb
read so doctor never clears .clerk/keyless.json or logs mutation warnings.
Ensure the breadcrumb is resolved once and reused by checkLoggedIn and
checkProjectLinked, preventing contradictory hints when both checks run in the
same doctor invocation.
In `@packages/cli-core/src/commands/env/README.md`:
- Around line 74-78: Update the sequence diagram in the environment command
README to distinguish the local keyless path from the standard flow: show local
publishable-key discovery, the optional authenticated GET /v1/domains
verification, and that a pairing mismatch performs no write. Ensure the diagram
no longer implies every run fetches an application from the Platform API, while
preserving the documented standard application-fetch flow.
In `@packages/cli-core/src/commands/init/README.md`:
- Line 80: Update the “Unauthenticated mode” step to apply only when the
resolved strategy is keyless, including both minting a keyless application and
preserving/reporting an existing unclaimed keyless app. Align the wording with
the authenticated-flow behavior described for reruns in the surrounding
initialization steps.
- Line 269: Revise the keyless breadcrumb rationale in the README to distinguish
loss of .clerk/keyless.json and environment keys from deletion of server-side
application state. State that these files preserve local recovery and claim
references, while configurations and resources already stored in the unclaimed
application persist even if the references are lost.
In `@packages/cli-core/src/commands/open/keyless-claim.ts`:
- Around line 37-48: Update readSdkClaimUrl to parse claimUrl with URL and
return it only when valid and using the https: protocol; otherwise return
undefined without allowing malformed or unsafe values to reach openBrowser. In
openKeylessDashboard, encode breadcrumb.claimToken with encodeURIComponent
before interpolating it into the query string.
In `@packages/cli-core/src/lib/keyless-target.ts`:
- Around line 152-182: Update hasKeyPairMismatch to catch BAPI and network
failures from the bapiRequest call, returning false when the domains request
fails or returns a non-2xx response. Preserve the existing domain matching logic
and ensure env pull remains nonblocking while reporting no mismatch on
unavailable BAPI data.
- Around line 79-97: Update findKeyInProject so file-based lookup honors names
priority rather than the last matching line: for each env file, collect matching
entries and select the value whose key appears earliest in names, while
preserving ENV_FILES precedence and the existing env-var lookup behavior. Return
the selected entry for the highest-priority name in the first applicable file.
---
Nitpick comments:
In `@packages/cli-core/src/commands/config/io.ts`:
- Around line 74-104: Update writeInstanceConfig so the keyless branch validates
options.method before calling patchKeylessConfig: reject method "PUT" explicitly
with an error, while preserving PATCH behavior and the existing non-keyless
flow.
In `@packages/cli-core/src/commands/config/keyless.test.ts`:
- Around line 650-662: Strengthen the “patch --dry-run sends nothing” test by
asserting that requests is non-empty before checking requests.every(...). Update
the test around configPatch so it verifies at least one GET request occurred
while preserving the existing dry-run log assertion.
In `@packages/cli-core/src/commands/init/index.ts`:
- Around line 204-227: Avoid duplicate authentication validation for agent-mode
--login by reusing the result from assertUsableFlags instead of calling
isAuthenticatedForAgent() again during init/pickStrategy. Memoize the validation
or pass its computed boolean through the existing flow, preserving the
unauthenticated guard behavior while ensuring getAuthenticatedEmail() runs only
once per invocation.
In `@packages/cli-core/src/commands/open/index.ts`:
- Around line 153-169: Move the resolveKeylessTarget/describeKeylessInstance
lookup into the agent/human output path after the options.print early return,
while preserving its existing best-effort error handling and leaving the plain
URL print behavior unchanged.
In `@packages/cli-core/src/commands/open/keyless-claim.ts`:
- Around line 95-98: Update the catch block in the keyless claim flow to use the
shared errorMessage() helper from ../../lib/errors.ts when constructing the log
message, replacing the unsafe (error as Error).message access while preserving
the existing fallback return.
In `@packages/cli-core/src/commands/users/open.ts`:
- Around line 85-90: Update the error code in the unclaimed keyless application
branch of open.ts to use ERROR_CODE.NOT_LINKED instead of
ERROR_CODE.INSTANCE_NOT_FOUND, matching openKeylessDashboard in open/index.ts
while preserving the existing message and error behavior.
In `@packages/cli-core/src/test/lib/init-harness.ts`:
- Around line 118-183: Update useInitHarness so setup preserves the existing
spies array entries when constructing the new spy list, ensuring spies added
through track are restored by afterEach. Hold gatherContextSpy in setup state
rather than locating it through getMockName in setupBootstrapSuccess, and have
that helper reuse the current setup spy without creating a fallback spy.
🪄 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: eea038a1-5a00-49fc-8b42-a68e29b37968
⛔ Files ignored due to path filters (6)
docs/keyless-demos/screenshots/01-signin-before.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/02-signup-before.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/03-orgs-enabled-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/04-orgs-disabled-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/05-signup-restricted-after.pngis excluded by!**/*.pngdocs/keyless-demos/screenshots/06-signup-allowed-after.pngis excluded by!**/*.png
📒 Files selected for processing (64)
.changeset/keyless-mode-default.mddocs/keyless-demos/.gitignoredocs/keyless-demos/README.mddocs/keyless-demos/lib/demo-env.shdocs/keyless-demos/record.shdocs/keyless-demos/screenshots/README.mddocs/keyless-demos/tapes/01-bootstrap.tapedocs/keyless-demos/tapes/02-read-config.tapedocs/keyless-demos/tapes/03-change-config.tapedocs/keyless-demos/tapes/04-toggles.tapedocs/keyless-demos/tapes/05-templates.tapedocs/keyless-demos/tapes/06-guardrails.tapedocs/keyless-demos/tapes/07-agent-mode.tapedocs/keyless-demos/tapes/08-sso-connections.tapedocs/keyless-demos/tapes/09-health-and-users.tapedocs/keyless-demos/tapes/_common.tapepackages/cli-core/src/commands/api/README.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/doctor/README.mdpackages/cli-core/src/commands/doctor/checks.tspackages/cli-core/src/commands/doctor/context.test.tspackages/cli-core/src/commands/doctor/context.tspackages/cli-core/src/commands/doctor/doctor.test.tspackages/cli-core/src/commands/doctor/types.tspackages/cli-core/src/commands/env/README.mdpackages/cli-core/src/commands/env/pull.test.tspackages/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/open/README.mdpackages/cli-core/src/commands/open/index.test.tspackages/cli-core/src/commands/open/index.tspackages/cli-core/src/commands/open/keyless-claim.test.tspackages/cli-core/src/commands/open/keyless-claim.tspackages/cli-core/src/commands/orgs/README.mdpackages/cli-core/src/commands/orgs/index.tspackages/cli-core/src/commands/users/README.mdpackages/cli-core/src/commands/users/open.test.tspackages/cli-core/src/commands/users/open.tspackages/cli-core/src/commands/whoami/README.mdpackages/cli-core/src/commands/whoami/index.test.tspackages/cli-core/src/commands/whoami/index.tspackages/cli-core/src/lib/bapi-command.test.tspackages/cli-core/src/lib/bapi-command.tspackages/cli-core/src/lib/credential-store.test.tspackages/cli-core/src/lib/credential-store.tspackages/cli-core/src/lib/keyless-target.test.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)
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/keyless-demos/.gitignore
- packages/cli-core/src/commands/orgs/README.md
- packages/cli-core/src/commands/doctor/README.md
- docs/keyless-demos/screenshots/README.md
- packages/cli-core/src/commands/billing/README.md
- packages/cli-core/src/commands/whoami/README.md
- packages/cli-core/src/commands/open/README.md
- packages/cli-core/src/commands/api/README.md
- packages/cli-core/src/commands/doctor/types.ts
- packages/cli-core/src/commands/users/README.md
- .changeset/keyless-mode-default.md
- packages/cli-core/src/commands/config/README.md
…im-url trust, and env-file key priority
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/cli-core/src/commands/open/keyless-claim.ts (1)
43-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReturn the normalized URL, not the raw SDK value.
Line 48 returns the untrusted original string after validation. A parseable HTTPS URL can contain a literal
", whichnew URL()normalizes but the Windows launcher still receives raw; that can break out of its quotedcmd /c startargument. Returnurl.toString()and add a quote-containing HTTPS regression case.Proposed fix
- const claimUrl = typeof parsed.claimUrl === "string" ? parsed.claimUrl : undefined; - if (claimUrl && !isHttpsUrl(claimUrl)) { + const rawClaimUrl = typeof parsed.claimUrl === "string" ? parsed.claimUrl : undefined; + const claimUrl = rawClaimUrl === undefined ? undefined : normalizeHttpsUrl(rawClaimUrl); + if (rawClaimUrl !== undefined && !claimUrl) { log.debug(`open: ignoring non-https claimUrl in ${SDK_KEYLESS_FILE.join("/")}`); return undefined; } return claimUrl;-function isHttpsUrl(value: string): boolean { +function normalizeHttpsUrl(value: string): string | undefined { try { - return new URL(value).protocol === "https:"; + const url = new URL(value); + return url.protocol === "https:" ? url.toString() : undefined; } catch { - return false; + return undefined; } }#!/bin/bash set -euo pipefail bun -e ' const raw = `https://dashboard.clerk.com/apps/claim?token=x" & echo injected`; const parsed = new URL(raw); console.log({ raw, normalized: parsed.toString(), protocol: parsed.protocol }); if (parsed.protocol !== "https:" || raw === parsed.toString()) process.exit(1); '🤖 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/open/keyless-claim.ts` around lines 43 - 48, Update the claim URL parsing flow around parsed.claimUrl and isHttpsUrl to retain the successfully parsed URL object and return its normalized toString() value instead of the raw string. Preserve rejection of non-HTTPS URLs, and add a regression case covering an HTTPS claim URL containing a quote to verify the returned value is normalized before launcher use.
🧹 Nitpick comments (1)
packages/cli-core/src/lib/keyless-target.test.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Bun’s file-writing API for fixtures.
Replace the new
node:fs/promiseswriteFilecalls withBun.write; the test already runs on Bun.Proposed fix
-import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm } from "node:fs/promises"; -await writeFile(join(tempDir, ".env.local"), content); +await Bun.write(join(tempDir, ".env.local"), content);As per coding guidelines, “Prefer
Bun.fileovernode:fs's readFile/writeFile for file operations.”Also applies to: 108-145
🤖 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/lib/keyless-target.test.ts` at line 2, Replace the imported node:fs/promises writeFile usage in the keyless-target test fixtures with Bun.write, updating each affected fixture setup in the test cases around the referenced range. Remove writeFile from the import while preserving the existing fixture paths and contents; leave mkdtemp and rm unchanged.Source: Coding guidelines
🤖 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/lib/keyless-target.test.ts`:
- Around line 105-112: Update the keyless-target test setup around beforeEach to
capture the original NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY and CLERK_PUBLISHABLE_KEY
values, then restore those values after each test rather than unconditionally
deleting them. Preserve the current per-test environment setup while ensuring
inherited or CI-injected keys are returned unchanged.
---
Duplicate comments:
In `@packages/cli-core/src/commands/open/keyless-claim.ts`:
- Around line 43-48: Update the claim URL parsing flow around parsed.claimUrl
and isHttpsUrl to retain the successfully parsed URL object and return its
normalized toString() value instead of the raw string. Preserve rejection of
non-HTTPS URLs, and add a regression case covering an HTTPS claim URL containing
a quote to verify the returned value is normalized before launcher use.
---
Nitpick comments:
In `@packages/cli-core/src/lib/keyless-target.test.ts`:
- Line 2: Replace the imported node:fs/promises writeFile usage in the
keyless-target test fixtures with Bun.write, updating each affected fixture
setup in the test cases around the referenced range. Remove writeFile from the
import while preserving the existing fixture paths and contents; leave mkdtemp
and rm unchanged.
🪄 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: bba1ab17-ae35-4cad-8552-b0dcbdafa7f9
📒 Files selected for processing (12)
packages/cli-core/src/commands/doctor/checks.tspackages/cli-core/src/commands/doctor/context.tspackages/cli-core/src/commands/doctor/doctor.test.tspackages/cli-core/src/commands/doctor/types.tspackages/cli-core/src/commands/env/README.mdpackages/cli-core/src/commands/init/README.mdpackages/cli-core/src/commands/open/keyless-claim.test.tspackages/cli-core/src/commands/open/keyless-claim.tspackages/cli-core/src/lib/keyless-target.test.tspackages/cli-core/src/lib/keyless-target.tspackages/cli-core/src/lib/keyless.test.tspackages/cli-core/src/lib/keyless.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)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli-core/src/commands/env/README.md
- packages/cli-core/src/commands/doctor/doctor.test.ts
- packages/cli-core/src/commands/init/README.md
wyattjoh
reviewed
Jul 30, 2026
…and doctor diagnostics - Restore CLERK_SECRET_KEY's precedence: the env var wins over a linked profile again, and --instance next to it stays a no-op instead of a usage error. Routing it through resolveKeylessTarget had silently changed both, breaking invocations that work on main; real env-var-path tests pin it now. - init's keep-the-existing-app guard also recognizes an application the SDK minted for itself (.clerk/.tmp/keyless.json), instead of silently orphaning it with a replacement. - doctor reports a malformed local secret key as one named failing check with a remedy, instead of five anonymous 'Check crashed' lines. - whoami: keyless next-steps no longer point at clerk link (which cannot work pre-claim), and an unlinked directory holding a local secret key is called out, since clerk api/users will use that key's instance, not the account. - Smaller review items: billing asserts on an already-resolved target, config schema resolves its target once, apply-patch dry-run messages reuse the verb, claim-url validation inlined, keylessTargetStubs covers every export for module mocks, positive test for the non-429 refresh refusal, keyless disable-orgs test, README corrections, changeset trimmed.
One function per sentence with typed parameters, so a future i18n layer can swap the module's bodies without touching call sites; pluralisation and agreement rules move with the prose. No visible string changes.
Signed-out `clerk init` changes behaviour — it now mints a keyless application instead of launching the browser login — so the release is not purely additive even with CLERK_SECRET_KEY precedence restored.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/doctor/checks.ts`:
- Around line 110-119: Update checkLoggedIn so it resolves and checks
ctx.getKeylessKeyError() before returning success for a stored account token.
Preserve the existing malformed-key failure diagnosis and token-success behavior
when no key error exists.
In `@packages/cli-core/src/commands/whoami/index.ts`:
- Around line 103-112: Update the whoami flow around findLocalSecretKey and the
localSecretKeySource field so exported CLERK_SECRET_KEY overrides are detected
and reported even when a profile resolves. Match the precedence used by
resolveBapiSecretKey, while preserving the existing unlinked-directory behavior,
and add coverage for a linked profile with an exported secret key.
In `@packages/cli-core/src/lib/copy.ts`:
- Around line 29-30: Update apiReachableKeysLine so the hint includes a direct
clerk api example for every key in keys, rather than only keys[0], while
preserving the existing singular/plural wording and unclaimed-application
context.
🪄 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: 0bc7a305-60c3-4040-a7c2-fbdb4b48ebbb
📒 Files selected for processing (30)
.changeset/keyless-mode-default.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/keyless.test.tspackages/cli-core/src/commands/config/keyless.tspackages/cli-core/src/commands/config/push.tspackages/cli-core/src/commands/config/schema.tspackages/cli-core/src/commands/doctor/README.mdpackages/cli-core/src/commands/doctor/checks.tspackages/cli-core/src/commands/doctor/context.test.tspackages/cli-core/src/commands/doctor/context.tspackages/cli-core/src/commands/doctor/doctor.test.tspackages/cli-core/src/commands/doctor/types.tspackages/cli-core/src/commands/init/index.tspackages/cli-core/src/commands/init/strategy.test.tspackages/cli-core/src/commands/open/index.test.tspackages/cli-core/src/commands/open/keyless-claim.tspackages/cli-core/src/commands/users/open.test.tspackages/cli-core/src/commands/users/open.tspackages/cli-core/src/commands/whoami/index.test.tspackages/cli-core/src/commands/whoami/index.tspackages/cli-core/src/lib/bapi-command.test.tspackages/cli-core/src/lib/bapi-command.tspackages/cli-core/src/lib/copy.tspackages/cli-core/src/lib/credential-store.test.tspackages/cli-core/src/lib/keyless-target.tspackages/cli-core/src/lib/next-steps.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)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/keyless-mode-default.md
- packages/cli-core/src/commands/doctor/README.md
- packages/cli-core/src/commands/config/README.md
Comment on lines
+110
to
119
| // A local key that isn't a secret key at all is the one keyless state that | ||
| // is genuinely broken — report it here as the named diagnosis, once, rather | ||
| // than letting it crash every keyless-aware check (see getKeylessKeyError). | ||
| const keyError = await ctx.getKeylessKeyError(); | ||
| if (keyError) { | ||
| return check.fail(`Not logged in, and the local secret key is unusable: ${keyError.message}`, { | ||
| remedy: "Fix or remove the malformed key, or run `clerk auth login` to authenticate.", | ||
| fixable: false, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Surface malformed local keys before accepting a stored token.
checkLoggedIn returns at Line 98 before resolving the keyless target. In an unlinked project with both a stored account token and an invalid local CLERK_SECRET_KEY, doctor reports success instead of the malformed-key diagnosis—even though keyless resolution still rejects that local key. Resolve/check getKeylessKeyError() before the token-success return.
🤖 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/doctor/checks.ts` around lines 110 - 119,
Update checkLoggedIn so it resolves and checks ctx.getKeylessKeyError() before
returning success for a stored account token. Preserve the existing
malformed-key failure diagnosis and token-success behavior when no key error
exists.
Comment on lines
+103
to
+112
| // Unlinked but holding a local secret key: the Backend API commands will use | ||
| // that key, not this account, so whoami must say so or its answer is wrong | ||
| // for half the command surface. | ||
| const localKey = profile ? undefined : await findLocalSecretKey(process.cwd()); | ||
|
|
||
| return { | ||
| kind: "account", | ||
| email: userInfo.email, | ||
| profile, | ||
| localSecretKeySource: localKey?.source ?? null, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Report exported secret-key overrides for linked directories.
When a profile resolves, Line 106 skips discovery entirely. But resolveBapiSecretKey still returns exported CLERK_SECRET_KEY before profile resolution, so whoami can claim the linked application while clerk api and clerk users target a different instance. Detect and surface the exported-key override even when profile exists, and add linked-profile coverage.
Proposed direction
- const localKey = profile ? undefined : await findLocalSecretKey(process.cwd());
+ const localKey = profile
+ ? process.env.CLERK_SECRET_KEY
+ ? { source: "CLERK_SECRET_KEY environment variable" }
+ : undefined
+ : await findLocalSecretKey(process.cwd());🤖 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 103 - 112,
Update the whoami flow around findLocalSecretKey and the localSecretKeySource
field so exported CLERK_SECRET_KEY overrides are detected and reported even when
a profile resolves. Match the precedence used by resolveBapiSecretKey, while
preserving the existing unlinked-directory behavior, and add coverage for a
linked profile with an exported secret key.
Comment on lines
+29
to
+30
| apiReachableKeysLine: (keys: string[]): string => | ||
| `${keys.join(", ")} ${keys.length === 1 ? "is" : "are"} already reachable on an unclaimed application — use \`clerk api /${keys[0]}\` directly instead of this config document.`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Incomplete clerk api hint when multiple keys are reachable.
When unknown keys are all API-reachable and there's more than one, the sentence lists every key but only shows the clerk api /<path> example for keys[0], leaving guidance for the other key(s) unstated.
✏️ Proposed fix
- apiReachableKeysLine: (keys: string[]): string =>
- `${keys.join(", ")} ${keys.length === 1 ? "is" : "are"} already reachable on an unclaimed application — use \`clerk api /${keys[0]}\` directly instead of this config document.`,
+ apiReachableKeysLine: (keys: string[]): string =>
+ `${keys.join(", ")} ${keys.length === 1 ? "is" : "are"} already reachable on an unclaimed application — use ${keys.map((key) => `\`clerk api /${key}\``).join(" or ")} directly instead of this config document.`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| apiReachableKeysLine: (keys: string[]): string => | |
| `${keys.join(", ")} ${keys.length === 1 ? "is" : "are"} already reachable on an unclaimed application — use \`clerk api /${keys[0]}\` directly instead of this config document.`, | |
| apiReachableKeysLine: (keys: string[]): string => | |
| `${keys.join(", ")} ${keys.length === 1 ? "is" : "are"} already reachable on an unclaimed application — use ${keys.map((key) => `\`clerk api /${key}\``).join(" or ")} directly instead of this config document.`, |
🤖 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/lib/copy.ts` around lines 29 - 30, Update
apiReachableKeysLine so the hint includes a direct clerk api example for every
key in keys, rather than only keys[0], while preserving the existing
singular/plural wording and unclaimed-application context.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.