Uh oh!
There was an error while loading. Please reload this page.
refactor(init): short-circuit on fully-clean re-run - #117
Conversation
7cddd7a to
7d8855dCompare6cf5261 to
dd71548Compare7d8855d to
b050c71Compareb050c71 to
a1d00b6Comparewyattjoh
commented
Apr 7, 2026
Stack: init-already-set-up Part of a stacked PR chain. Do not merge manually. |
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR adds detection and handling for projects where Clerk is already set up. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. Comment |
a1d00b6 to
7d31b09CompareWhen `clerk init` runs against a project where Clerk is already set up
(supported framework, SDK installed, every scaffold action would be a
skip, no postInstructions), the init flow used to print "Clerk is already
set up" inside scaffoldAndWrite but kept running env pull, the skills
prompt, and the outro. The message lied about init completing.
This refactors detectAndInstall and scaffoldAndWrite to return an
{ alreadySetUp: boolean } flag. init() owns the green message and
short-circuits the entire post-scaffold flow when alreadySetUp is true.
The "framework detected but unsupported" path (Express/Fastify/Expo)
still falls through because postInstructions are useful guidance the
user should see every time.
Adds a test that overrides the default null gatherContext mock with a
stub Next.js context and asserts pull, printKeylessInfo, and
installSkills are NOT called when scaffold returns an empty plan.7d31b09 to
baabc3eCompare
Railly
left a comment
There was a problem hiding this comment.
One broader thought, not for this PR: setup() is getting heavy (19 spies) and the tests are drifting toward asserting on implementation rather than behavior. Might be worth a follow-up to split the helper by concern, or lean on a small integration test against a tmpdir fixture.
LGTM
Uh oh!
There was an error while loading. Please reload this page.
Addresses review nit on #117: the previous spies.find(...) lookup never matched (spy !== module export), making the mockRestore a no-op. Return gatherContextSpy from setup() so tests can call mockResolvedValueOnce directly.
Uh oh!
There was an error while loading. Please reload this page.
Summary
clerk initruns against a project where Clerk is already fully set up (supported framework, SDK installed, every scaffold action is a skip, no postInstructions), init used to print "Clerk is already set up" insidescaffoldAndWritebut then kept running env pull, the skills prompt, and the outro. The message lied about init completing.detectAndInstallandscaffoldAndWriteto return{ alreadySetUp: boolean }.init()owns the green message and short-circuits the entire post-scaffold flow whenalreadySetUpis true.pull,printKeylessInfo, andinstallSkillsare NOT called.Stacked on #116.
Test plan
bun run testpasses (57 passed)short-circuits env pull and skills install when already set uppassesclerk inittwice in a scaffolded Next.js project and confirm the second run exits after the "already set up" message without prompting for skills