Skip to content

refactor(version): simplify version handling - #414

Merged
wyattjoh merged 1 commit into
mainfrom
wyattjoh/version-tweaks
Aug 10, 2026
Merged

refactor(version): simplify version handling#414
wyattjoh merged 1 commit into
mainfrom
wyattjoh/version-tweaks

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

The version module exposed three functions (getCurrentVersion(), resolveCliVersion(), isDevVersion()) for what are really two compile-time facts: the version string this binary was built with, and whether that version is a development build. Callers had to remember which accessor answered which question, and resolveCliVersion() returning undefined for dev builds encoded "unversioned" as an absent string rather than a boolean.

This replaces the accessors with two constants, CURRENT_VERSION and IS_DEV_BUILD. The dev classification moves into the Bun macro so it happens once during transpilation alongside version derivation, and the macro now resolves CLI_VERSION itself rather than having the consuming module branch on it. That keeps the entire question of what version this is, and whether it is a dev build, in one place. The only signature change is shouldCheckForUpdates, which now takes the boolean directly instead of a version string.

Nothing user-visible changes. clerk --version, the outbound user agent, MCP client info, clerk doctor's CLI version check, clerk update, and the macOS keychain namespacing all produce the same output as before. A new .claude/rules/versioning.md records the constant-over-accessor convention, and the changeset is empty since the change is internal.

Test plan

  • Full suite: 2469 pass / 0 fail; format:check, lint, typecheck clean
  • version.test.ts compiles real binaries with and without --define CLI_VERSION, confirming an injected release version still wins over checkout metadata, an injected -dev.* version still classifies as dev, and canary versions stay non-dev
  • bun changeset status --since=origin/main exits clean

- Export compile-time version and dev-build constants
- Update version consumers to use the constants
- Classify development builds during macro evaluation
@changeset-bot

changeset-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: af0edc8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@wyattjoh
wyattjoh marked this pull request as ready for review August 10, 2026 20:33
@wyattjoh
wyattjoh requested a review from rafa-thayto August 10, 2026 20:33
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI now resolves its version and development-build status at build time. CURRENT_VERSION and IS_DEV_BUILD replace runtime version helpers across CLI commands, update checks, credential storage, MCP probing, and user-agent generation. Version and update-check tests now validate the constant-based API. Versioning guidance was updated, and an empty changeset was added.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • clerk/cli#406: Provides the preceding versioning implementation that this change refines.
  • clerk/cli#410: Shares changes to CLI version and user-agent behavior.

Suggested reviewers: dmoerner

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: simplifying version handling through a refactor.
Description check ✅ Passed The description directly explains the version-handling refactor, affected consumers, tests, and unchanged user-visible behavior.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli-core/src/lib/version.macro.ts`:
- Around line 15-20: Update isDevVersion to isolate the SemVer prerelease
portion before the + build-metadata delimiter, so dashes in build metadata
cannot identify a development version. Add a compiled-fixture case for
3.1.0+release-dev asserting isDev: false.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ebbbf613-0b5d-41e7-9d99-773e54804119

📥 Commits

Reviewing files that changed from the base of the PR and between 457146f and af0edc8.

📒 Files selected for processing (15)
  • .changeset/dry-spoons-bake.md
  • .claude/rules/versioning.md
  • CLAUDE.md
  • packages/cli-core/src/cli-program.ts
  • packages/cli-core/src/commands/doctor/checks.ts
  • packages/cli-core/src/commands/mcp/probe.ts
  • packages/cli-core/src/commands/update/index.ts
  • packages/cli-core/src/lib/credential-store.test.ts
  • packages/cli-core/src/lib/credential-store.ts
  • packages/cli-core/src/lib/update-check.test.ts
  • packages/cli-core/src/lib/update-check.ts
  • packages/cli-core/src/lib/user-agent.ts
  • packages/cli-core/src/lib/version.macro.ts
  • packages/cli-core/src/lib/version.test.ts
  • packages/cli-core/src/lib/version.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go (manual)
  • clerk/dashboard (manual)
  • clerk/accounts (manual)
  • clerk/backoffice (manual)
  • clerk/clerk (manual)
  • clerk/clerk-docs (manual)
  • clerk/cloudflare-workers (manual)
  • clerk/javascript (auto-detected)

Comment on lines +15 to +20
function isDevVersion(version: string): boolean {
const dash = version.indexOf("-");
if (dash === -1) return false;
const prerelease = version.slice(dash + 1);
return prerelease === DEV_TAG || prerelease.startsWith(`${DEV_TAG}.`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not classify build metadata as a prerelease.

isDevVersion("3.1.0+release-dev") returns true because Line 16 finds the dash in build metadata. This is a release SemVer value. IS_DEV_BUILD then disables update checks and changes the macOS keychain namespace.

Stop parsing at + before checking the prerelease identifier. Add a compiled-fixture test for 3.1.0+release-dev with isDev: false.

Proposed fix
 function isDevVersion(version: string): boolean {
   const dash = version.indexOf("-");
-  if (dash === -1) return false;
-  const prerelease = version.slice(dash + 1);
+  const buildMetadata = version.indexOf("+");
+  if (dash === -1 || (buildMetadata !== -1 && dash > buildMetadata)) return false;
+  const prerelease = version.slice(dash + 1, buildMetadata === -1 ? undefined : buildMetadata);
   return prerelease === DEV_TAG || prerelease.startsWith(`${DEV_TAG}.`);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function isDevVersion(version: string): boolean {
const dash = version.indexOf("-");
if (dash === -1) return false;
const prerelease = version.slice(dash + 1);
return prerelease === DEV_TAG || prerelease.startsWith(`${DEV_TAG}.`);
}
function isDevVersion(version: string): boolean {
const dash = version.indexOf("-");
const buildMetadata = version.indexOf("+");
if (dash === -1 || (buildMetadata !== -1 && dash > buildMetadata)) return false;
const prerelease = version.slice(dash + 1, buildMetadata === -1 ? undefined : buildMetadata);
return prerelease === DEV_TAG || prerelease.startsWith(`${DEV_TAG}.`);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli-core/src/lib/version.macro.ts` around lines 15 - 20, Update
isDevVersion to isolate the SemVer prerelease portion before the +
build-metadata delimiter, so dashes in build metadata cannot identify a
development version. Add a compiled-fixture case for 3.1.0+release-dev asserting
isDev: false.

@wyattjoh
wyattjoh merged commit a881cf0 into main Aug 10, 2026
11 checks passed
@wyattjoh
wyattjoh deleted the wyattjoh/version-tweaks branch August 10, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants