Uh oh!
There was an error while loading. Please reload this page.
fix(cli): await the next-steps outro so post-command output prints below it - #435
Conversation
…low it The next-steps outro animation moves the cursor back onto the header line for ~450ms, but several commands (switch-env, auth logout, unlink, users create, apps create, and others) did not await outro(). The command's promise resolved mid-animation, so the postAction update check printed the "Update available" notice at the parked cursor position — overwriting the step lines and leaving a stray duplicate "Next steps" header. Await every outro() call so output printed after a command lands below the finished block. Adds a regression test that runs the real spinner/gradient on a forced-interactive TTY and asserts the notice prints after the animation's cursor-restore.
🦋 Changeset detectedLatest commit: fa65fd7 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 Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 6 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 WalkthroughWalkthroughCLI command exit paths now await asynchronous Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk:⚪ Minimal · up to This change ensures post-command update notices print after the completed Next steps block. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
thiskevinwang
left a comment
There was a problem hiding this comment.
LGTM! Looks like repeated applying of missed await to outro()
Consider a eslint or oxlint rule to catch this at compiler level
… Bun 1.4 Bun 1.4 runs macros in a sealed transpiler context that --define globals no longer reach, so resolveVersionAtBuildTime() always took the checkout-derived fallback and the compiled-version tests failed on CI (which floats to the latest Bun satisfying engines). Move the injected-vs-fallback choice and dev classification into version.ts module scope, where define substitution still applies on every Bun version. The macro now only derives the Git checkout fallback, so the compiled CLI still never runs Git at runtime. Verified: version tests and the full suite pass under both Bun 1.3.14 and 1.4.0.
There was a problem hiding this comment.
Checked that the regression test genuinely goes red on the un-awaited code and green on the fix, and that the Bun 1.4 define change still resolves correctly on Bun 1.3.14 across all three build shapes (release define, no define, dev-flavored define). Two nits below; neither blocks.
Sent from Claude
- reviewed by Wyatt Johnson
Uh oh!
There was an error while loading. Please reload this page.
…e poll On fixed code the animation completes before switchEnv() resolves, so the 700ms unconditional sleep was pure dead time on every green run. Poll for the cursor-restore escape with a 2s bound instead: green runs return immediately, and a regression still drains the orphaned animation so the ordering assertion fails with its precise message.
Summary
Fixes the garbled "Update available" output reported in Slack: after
clerk switch-env, the update notice printed on top of the "Next steps" block (Update available: 3.0.1 → 3.1.0henticate for this environment), with missing spaces, doubled backticks, and a stray duplicate "Next steps" header.Root cause:
outro(steps)inlib/spinner.tsis async — it awaitsanimateHeader, which parks the cursor on the header line for ~450ms of shine animation before restoring it. Several commands calledoutro(...)withoutawait(a floating promise), so the command's promise resolved at the animation's first frame. Commander'spostActionhook then ranmaybeNotifyUpdate, whose lines printed at the parked cursor position — overwriting the step lines mid-block, while the animation's remaining frames repainted the header further down.Fix:
awaiteveryoutro(...)call site (47 sites across 18 command files;deployandwebhooks tokenalready awaited correctly). Output printed after a command now always lands below the finished block — the ordering.claude/rules/interrupts.mdalready documents as the post-command tail.Test plan
switch-env/update-notice-race.test.tsruns the real spinner/gradient on a forced-interactive TTY, prints the update notice the instantswitchEnv()resolves (exactly whatpostActiondoes), and asserts it lands after the animation's cursor-restore escape. Red before the fix (notice at stream index 606, cursor restored at 4377), green after. The assertion is enforced by promise sequencing, not wall-clock timing.bun run format:check/lint/typecheckclean; full unit suite 2654 pass / 0 fail.clerkpatch.