feat(init): Allow agent init without implicit app selection#244
Conversation
🦋 Changeset detectedLatest commit: a6c44d6 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates the Clerk CLI's Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
|
||
| // Auto-keyless is scoped to bootstrap (new-project) flows only in human | ||
| // mode. Existing projects keep the authenticated flow so real keys can be | ||
| // pulled unless an agent explicitly chooses keyless by omitting an app. |
There was a problem hiding this comment.
to be honest i'm not sure this is what we want, but its what we were doing before. i find the fact that clerk init doesn't require auth for bootstrapping and does require auth in existing projects a bit odd
| // Auto-keyless is scoped to bootstrap (new-project) flows only in human | ||
| // mode. Existing projects keep the authenticated flow so real keys can be | ||
| // pulled unless an agent explicitly chooses keyless by omitting an app. | ||
| if (!bootstrap) return false; |
There was a problem hiding this comment.
Refactoring resolveKeylessMode moved the if (!bootstrap) return false; early-return inside the if (ctx.framework.supportsKeyless) block. The non-keyless branch (which still ends in log.info("X requires API keys — keyless mode is not yet supported for this framework.")) is no longer guarded by bootstrap, so the log now fires on paths where keyless was never being considered:
- Human mode re-running
clerk initin an existing Vue / Express / Fastify project (previously silent). - Agent mode with
--appon a non-keyless framework (the real-app flow is what's running; the user gets a misleading 'keyless not supported' note). - The new agent manual-setup path, where
printBootstrapManualSetupInfoalready prints a clearer 'requires API keys — set them up manually' block. Users see two near-duplicate messages back-to-back.
The new integration test init prints manual setup for non-keyless framework without an app target in agent mode only asserts expect(stderr).toContain("clerk init --app <app_id>"), which passes despite the duplicate output, so the regression isn't caught.
Suggested fix: short-circuit on the non-keyless framework before the function does anything else, and let the caller own user-facing copy:
function resolveKeylessMode({...}): boolean {
if (!ctx.framework.supportsKeyless) return false;
if (agent) return !hasRealAppTarget;
if (!bootstrap) return false;
return !authed;
}If the bootstrap-of-non-keyless framework hint is still wanted, emit it at the bootstrap call site (e.g., next to printBootstrapManualSetupInfo) rather than from inside the resolver. Either way, please add a regression test that asserts stderr does not contain keyless mode is not yet supported for an existing-project non-keyless human-mode run.
Summary
Validation