feat: auto-claim keyless applications on login#157
Conversation
rafa-thayto
force-pushed
the
rafa-thayto/auto-claim
branch
from
a56b83d to
d338623
Compare
|
📝 WalkthroughWalkthroughThis pull request introduces automatic claiming of keyless applications during the login flow. After a keyless app is created during initialization (with temporary keys persisted via a breadcrumb file), the new Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rafa-thayto
force-pushed
the
rafa-thayto/auto-claim
branch
from
d338623 to
76b9642
Compare
Add machine-readable error code for autoclaim failures, available for agent mode and CI error classification.
Add PLAPI client function for POST /v1/platform/accountless_applications/claim. Sends claim token and app name, returns the claimed Application.
Extract profile-building logic from autolink() into a reusable linkApp() function. This allows both autolink (key detection) and autoclaim (keyless claim) to share the same profile persistence code.
Add keyless.ts for managing accountless Clerk applications: - createAccountlessApp(): calls BAPI to create app with no auth - writeKeysToEnvFile(): writes framework-specific keys to .env.local - parseClaimToken(): extracts token from claim URL - Breadcrumb I/O: read/write/clear .clerk/keyless.json Includes 15 unit tests covering all functions and edge cases.
Add autoclaim.ts that detects keyless projects and claims them automatically after clerk auth login. Never throws — returns a discriminated union (claimed|not_found|already_claimed|failed|not_keyless) so the login flow is never interrupted. Key behaviors: - Truncates app name to 50 chars (backend limit) - Transient errors (5xx) preserve breadcrumb for retry - Terminal errors (404/403) clear breadcrumb Includes 8 unit tests covering all status paths.
When bootstrap mode skips authentication, create an accountless app via BAPI, write keys to .env.local, and store the claim token in .clerk/keyless.json for autoclaim on next login. Also simplifies the keyless info message to promote the autoclaim flow instead of requiring manual clerk link + env pull.
Wire autoclaim into the login flow with improved UX: - Specific warning messages per failure cause (expired token, missing org, transient error) instead of generic fallback - Contextual next-steps based on claim result: manual link for terminal failures, retry guidance for transient errors - Mock autoclaim in login tests to isolate from transitive deps
After a successful autoclaim, automatically run `env pull` to refresh .env.local with the claimed app's keys — no manual step needed.
rafa-thayto
force-pushed
the
rafa-thayto/auto-claim
branch
from
76b9642 to
55ea73e
Compare
- Extract shared plapiRequest<T> helper; collapse 6 PLAPI functions to 2-3 lines each (6 near-duplicate fetch blocks → one) - Promote errorMessage() from doctor/checks.ts to lib/errors.ts so it can be shared with autoclaim.ts and init/index.ts - Replace two TOCTOU file.exists()+read patterns in keyless.ts with atomic .text()/.json() catch-ENOENT variants - Parallelize detectPublishableKeyName + detectSecretKeyName in writeKeysToEnvFile (independent I/O) - Table-drive classifyClaimError (404/403 status → result mapping) - Drop file-level docstrings, section-header banners, and WHAT/ narration comments across new files per house style
wyattjoh
left a comment
wyattjoh
left a comment
There was a problem hiding this comment.
Code Review — PR #157
Reviewed with Opus + Codex second-opinion validation.
Blocker
B1. .clerk/keyless.json stores a live claim token with no .gitignore protection (codex confirmed)
packages/cli-core/src/lib/keyless.ts:88-106 writes claimToken under .clerk/ in the user's project. Nothing in this PR (or clerk init today) appends .clerk/ to the project's .gitignore. A developer can git add . and commit the token to a public repo, at which point any third party can claim the app before the original developer logs in, hijacking an app that will later hold production workloads.
Suggestion: in setupKeylessApp or writeKeylessBreadcrumb, append .clerk/ to .gitignore if not already present (mirroring what create-next-app and similar tools do). Add a test that verifies the .gitignore update.
B2. Keyless init writes .env.local but post-claim pull writes the framework default, producing two env files with drifting keys
This is a cross-file interaction with existing code rather than a change in this PR alone, but the PR is where the regression becomes user-visible:
packages/cli-core/src/lib/keyless.ts:57-76hardcodesconst targetFile = join(cwd, ".env.local").packages/cli-core/src/lib/framework.ts(unchanged) declaresenvFile: ".env"for Next.js, Astro, and Nuxt.packages/cli-core/src/commands/env/pull.ts:30-46(resolveTargetFile) prefers the framework-detected file.
Net effect on a Next.js project: .env.local written at init, .env written by pull after claim, same keys, different values.
Suggestion: use detectEnvFile(cwd) in writeKeysToEnvFile so there is one canonical target. Also update the JSDoc that references .env.local.
B3. Missing changeset
git diff main..HEAD -- .changeset/ is empty. This is a feat: PR touching @clerk/cli-core runtime. The Enforce Changeset workflow will block merge.
Major
M1. App name sent to backend uses basename(cwd).slice(0, 50) with no sanitization (codex confirmed)
packages/cli-core/src/lib/autoclaim.ts:32:
- Leaks potentially-sensitive directory names to the Clerk platform API.
.slice(50)truncates on UTF-16 code units; an emoji or non-BMP char at the boundary produces an orphaned surrogate (400 on strict UTF-8 validators).- Backend-invalid chars are not sanitized.
Suggestion: derive the name from package.json#name or the git repo name with basename(cwd) as a fallback, and sanitize + byte-length-truncate before sending.
M2. attemptAutoclaim(cwd) threads cwd but tryPullEnv ignores it
autoclaim.ts:37-50 accepts a cwd argument for the breadcrumb but calls pull({}), which uses process.cwd() internally (pull.ts:53). Today's single call site passes process.cwd() so it is benign, but this is fragile. Either thread cwd through pull options or assert at entry.
M3. createAccountlessApp has no timeout or abort; clerk init hangs on captive portal / slow BAPI (codex confirmed)
packages/cli-core/src/lib/keyless.ts:35-52 and packages/cli-core/src/commands/init/index.ts:272-287: fetch(...) has no AbortController. On slow/broken networks, withSpinner hangs indefinitely. log.debug is only visible with --verbose, so users see nothing.
M4. setupKeylessApp swallows all errors while success copy still renders (codex confirmed)
commands/init/index.ts:270-287 wraps the body in try { ... } catch { log.debug(...) }. Even on filesystem errors, parseClaimToken throws, or the fetch fails, printKeylessInfo still tells the user "Your app is ready with development keys in .env.local". Distinguish expected network failures from unexpected bugs and suppress the success message on failure.
M5. classifyClaimError semantic mismatch and aggressive breadcrumb-clear
autoclaim.ts:62-74 maps 403 to already_claimed, but the message at login.ts:128-132 says "your account does not have an active organization". Name and message disagree; rename the status (e.g. no_organization). Also, 404 unconditionally clears the breadcrumb; a transient DNS blip that returns 404 would destroy the retry signal.
Minor
readKeylessBreadcrumb(keyless.ts:108-119) has no schema validation; a malformed-but-JSON file yields{claimToken: undefined}andautoclaim.ts:33will POST{token: undefined}.parseClaimTokenuseshttps://placeholder.comas the synthetic URL base (keyless.ts:81-87); preferhttps://example.invalid(RFC 6761).- Em-dashes added in
login.ts:130-131,163andautoclaim.ts:7(global style rule). handleAutoclaimprintslog.successthenlog.warnthenoutro(login.ts:117-124,135-148); noisy ordering.- No retry for 5xx / 429 despite the design claim that "transient failures preserve breadcrumb for retry on next login" (
autoclaim.ts:28-36); a short in-process retry with backoff would save users a manual re-login.
Nits
CLAIM_WARNINGScould be an explicit object-literal map of the three warning statuses.log.info("Environment variables written to .env.local")will be wrong once B2 is fixed.- Test
expect(parsed.name).toBeTruthy()(autoclaim.test.ts:164) is too loose; assert the exact truncation. - CI refactor (composite action,
workflow_call) is bundled into afeat:PR; splitting would simplify bisect.
Positives
AutoclaimResult discriminated union is cleanly typed and enables exhaustive handling in loginNextSteps. attemptAutoclaim never throws (the type system enforces it). linkApp extraction in autolink.ts is a good refactor. Test coverage for the orchestrator is thorough: all status paths, breadcrumb clear/preserve semantics, env-pull success/failure, linkApp invocation, and request body shape.
2 participants
Summary
keyless.ts): create accountless apps via BAPI, write framework-specific keys to.env.local, manage.clerk/keyless.jsonclaim token breadcrumbautoclaim.ts): afterclerk auth login, detect keyless breadcrumb and automatically claim the application via PLAPI — never throws, returns discriminated union so login is never interruptedclerk env pullstep neededlinkApp()fromautolink.tsfor reuse by both autolink (key detection) and autoclaim (keyless claim)claimApplication()PLAPI endpoint (POST /v1/platform/accountless_applications/claim)clerk initbootstrap when user skips authclerk auth loginwith contextual warnings and next-steps per failure modeHow it works
clerk init(skip auth) → creates accountless app, writes keys to.env.local, stores claim token in.clerk/keyless.jsonclerk auth login→ after OAuth, reads breadcrumb, calls claim endpoint, links app to account, auto-pulls env vars, clears breadcrumbCleanup pass (commit 6cda4f5)
Applied KISS/DRY simplification across the new surface and touched neighboring code where it shared the same patterns. Net: -121 lines (107 insertions, 228 deletions).
plapi.ts: extracted sharedplapiRequest<T>helper; 6 near-identical fetch-with-auth blocks (20 lines each) collapsed to 2–3 line call sites. Helper normalizes URL construction, query-param serialization, auth headers,PlapiErrorthrow, and JSON parsing.errors.ts: promotederrorMessage(unknown): stringout ofcommands/doctor/checks.tssoautoclaim.tsandinit/index.tscan reuse it instead of re-implementingerror instanceof Error ? error.message : String(error)inline.keyless.ts: replaced two TOCTOUBun.file(...).exists()+ read patterns with atomic.text().catch(() => "")/.json().catch(...); parallelized independentdetectPublishableKeyName+detectSecretKeyNamewithPromise.all; factored.env.local/.clerk/keyless.jsoninto named constants.autoclaim.ts: table-drivenclassifyClaimError— replaced back-to-backerror instanceof PlapiError && error.status === Nbranches with aTERMINAL_BY_STATUS: Record<number, Terminal["status"]>lookup.as never→as Profile, dropped thespies[]array in favor of explicit per-spymockRestore(), deleted redundant setup commentary.Test plan
clerk init -y→ skip auth → verify.env.localand.clerk/keyless.jsoncreated →clerk auth login→ verify app claimed, linked, and env vars refreshed