fix(init): simplify auth/keyless flow and fix --starter#175
fix(init): simplify auth/keyless flow and fix --starter#175rafa-thayto wants to merge 5 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 1682ce6 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 |
📝 WalkthroughWalkthroughThe init command flow was changed to check authentication explicitly before skipping prompts. resolveKeylessMode now returns false when already authenticated and only enables keyless for unauthenticated, keyless-capable frameworks; it avoids logging “keyless not yet supported” unless bootstrap is requested. handleStarter and bootstrap calls now pass an implicitBootstrap flag instead of skipConfirm to bypass only the initial “create a new one?” confirmation. The exported askSkipAuth helper was removed and promptAndBootstrap gained an implicitBootstrap option. A new heuristics.isAuthenticated() helper was added (checks CLERK_PLATFORM_API_KEY or stored auth). Tests were updated to use isAuthenticated and to expand keyless/auth coverage. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
|
!snapshot |
Snapshot publishednpm install -g clerk@0.0.3-snapshot.v20260417120320
|
5cf3c19 to
7d45a72
Compare
|
!snapshot |
Snapshot publishednpm install -g clerk@0.0.3-snapshot.v20260417132423
|
|
!snapshot |
Snapshot publishednpm install -g clerk@0.0.3-snapshot.v20260417160442
|
Skip the askSkipAuth prompt and keyless flow when the user is already authenticated, and suppress the "keyless not supported" message outside of the bootstrap flow.
`handleStarter` forced `skipConfirm: true` when calling the bootstrap flow, which conflated "user already opted into bootstrapping" with "non-interactive mode" inside `promptAndBootstrap` and caused `clerk init --starter` (without `-y`) to fail the framework guard with: "Non-interactive mode requires --framework for new projects". Add a separate `implicitBootstrap` override that only skips the initial "create a new one?" confirm, leaving `skipConfirm` to mean non-interactive.
Previously, resolveKeylessMode and the skipAuth check only consulted getAuthenticatedEmail (OAuth). When CLERK_PLATFORM_API_KEY was set (as in CI E2E), the user was treated as unauthenticated, so `clerk init --yes` on a keyless-supporting framework fell through to keyless mode instead of pulling the real keys — breaking every E2E fixture. Introduce an isAuthenticated helper that returns true for either auth mechanism, and use it in both branches.
Previously, an unauthenticated user on a keyless-capable framework was asked "Skip authentication for now?" with a default of yes. The prompt added a step without meaningful choice in practice — the unauthenticated path can't pull keys regardless, and the keyless info message already points users to `clerk auth login` for later. Now the flow just follows auth state: signed in → authenticated flow + env pull; not signed in → keyless. Removes `askSkipAuth` entirely.
9a70430 to
1682ce6
Compare
wyattjoh
left a comment
There was a problem hiding this comment.
Code Review — PR #175
Reviewed with Opus + Codex second-opinion validation.
Major
M1. Removing the bootstrap != null guard broadens keyless auto-select to existing projects (codex partial)
packages/cli-core/src/commands/init/index.ts:215-225 now returns keyless = true for any unauthenticated user on a keyless-capable framework, not only during bootstrap. Downstream effects:
authenticateAndLinkis skipped when users re-runclerk initsigned out on an existing Clerk project.ctx.keyless = trueatframeworks/helpers.ts:204-221,461-467toggles permissiveclerkMiddleware()(no route protection). Idempotency viahasClerkImportshould prevent overwrites, but a scaffold-idempotency test forexistingClerk: true + keyless: truewould make the invariant load-bearing rather than incidental.env pullis skipped for the keyless branch.
The changeset does note keyless auto-select behavior, but the existing-project qualifier is not spelled out. Consider either restoring the if (!bootstrap) return false guard, or explicitly documenting the existing-project case in README and changeset.
M2. isAuthenticated() conflates "no credentials" with "network/API failure" (codex partial)
packages/cli-core/src/commands/init/heuristics.ts:139-147 delegates to getAuthenticatedEmail() which catches all errors and returns null. Expired tokens, Clerk 5xx, or offline laptops all silently demote the user into keyless (for keyless-capable frameworks) or skip-auth. Previously askSkipAuth gave the user a decision point. Consider a lighter credential-presence check (getToken() + CLERK_PLATFORM_API_KEY) for flow selection, reserving getAuthenticatedEmail() for the display label.
Minor
- M3.
isAuthenticated()is called twice on the authenticated + keyless-capable path (index.ts:90,94); resolve once and pass the boolean. - M4.
packages/cli-core/src/commands/init/README.md:48-52still documents the removed "Continue with temporary keys" prompt. Per.claude/rules/commands.md, the README must match behavior. - M5. New test
"--starter without -y runs bootstrap interactively..."(index.test.ts:424-436) fully mockspromptAndBootstrapand never exercises the real guard atbootstrap.ts:173-177that was the actual bug. Add a direct test for the guard.
Missed on first pass (codex caught)
- Second README inconsistency.
init/README.md:86says non-keyless frameworks "always force authentication even with--yes", butindex.ts:93-99skips auth for unauthenticated users when-yis set.
Nits
isAuthenticateddocstring (heuristics.ts:139-143) still references prompts that no longer exist.BootstrapOverrides.implicitBootstrapdoc (bootstrap.ts:135-140) doesn't note thatskipConfirmsupersedes it.handleStarter(index.ts:153-163) droppingskipConfirm: truechanges interactive--starterfrompreviewPlan()topreviewAndConfirm(). The changeset mentions--starteris "fully interactive", so this looks intentional; worth a bullet for reviewer transparency.
Positives
implicitBootstrap cleanly expresses "user opted into bootstrapping but did not opt out of prompts", better than overloading skipConfirm. Test additions cover the authentication x keyless x bootstrap/existing matrix well. isAuthenticated correctly short-circuits on CLERK_PLATFORM_API_KEY before network calls.
Summary
Four
clerk initfixes on this branch:askSkipAuthand keyless flow when signed in. If the user has an OAuth token,clerk initgoes straight to the authenticated flow (auth + link + env pull).CLERK_PLATFORM_API_KEYas authenticated. Non-interactive / CI runs with an API key no longer fall through to keyless mode. A newisAuthenticated()helper covers both OAuth tokens and the env var.clerk auth loginfor later).clerk init --starterinteractive without--framework. Running--starterwithout-yno longer errors with "Non-interactive mode requires --framework" — it only implies "yes, bootstrap," not "skip every prompt."Also suppresses the "keyless not supported" framework message outside the bootstrap flow.
Test plan
bun run test— init suite covers the permutations (auth vs. not, API key vs. OAuth, bootstrap vs. existing repo,--starterwith and without-y)bun run lint/bun run typecheckclerk initon a keyless framework while signed in → authenticated flow, no keyless promptclerk initon a keyless framework while signed out → keyless flow, no promptclerk initin an existing repo with keyless framework while signed in → no keyless prompt, env pulledclerk init --starter(no-y) → interactive framework picker instead of an error