From 38972c7b4804de6a3ded4996420a6dfaa8475000 Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Fri, 17 Apr 2026 12:10:53 -0300 Subject: [PATCH 1/6] fix(update): resolve installer by PATH priority, add --all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, `clerk update` called `detectInstaller()` on the running binary's `process.execPath`. Two bugs compounded: 1. Bun detection compared against `bun pm bin -g` (the shim dir `~/.bun/bin`), but the Bun-compiled platform binary lives in `~/.bun/install/global/node_modules/@clerk/cli-`. The match never succeeded, so detection fell through to npm. 2. The fallback `npm install -g clerk@` returned exit 0 and the spinner reported success — but on machines with asdf-managed node, the update landed in `~/.asdf/shims/` while the user's shell still resolved `clerk` to the (unchanged) `~/.bun/bin/clerk`. The fix splits resolution from execution: - New `findClerkOnPath()` walks `process.env.PATH` (shell-agnostic) and returns realpath'd, deduped, executable `clerk` binaries in PATH order. - New `getInstallerPackageDirs()` returns the dir where each PM stores *packages* (not shims); the bun entry uses `$BUN_INSTALL/install/ global/node_modules` — fixing bug 1. - New `ownerOfBinary()` maps a binary path to its owning installer, returning `null` on no match. The update command treats `null` as refuse-rather-than-guess, preventing bug 2. - The update command targets the *first* `clerk` on PATH (what the user's shell will actually execute), not whatever `process.execPath` reports. Also: - `--all` flag updates every `clerk` install on PATH in one run, skipping Homebrew on non-stable channels and `null`-owned binaries with a per-install warning and summary. - Other installs are reported after every run so shadowing is visible. - Post-update `hash -r` / `rehash` hint based on `$SHELL` (skipped for fish and PowerShell, which don't cache). --- .changeset/clerk-update-path-aware.md | 5 + README.md | 2 + packages/cli-core/src/cli-program.ts | 2 + .../cli-core/src/commands/update/README.md | 43 ++-- .../cli-core/src/commands/update/index.ts | 200 ++++++++++++++++-- packages/cli-core/src/lib/installer.test.ts | 176 +++++++++++++++ packages/cli-core/src/lib/installer.ts | 167 ++++++++++++++- 7 files changed, 555 insertions(+), 40 deletions(-) create mode 100644 .changeset/clerk-update-path-aware.md diff --git a/.changeset/clerk-update-path-aware.md b/.changeset/clerk-update-path-aware.md new file mode 100644 index 00000000..af3660bc --- /dev/null +++ b/.changeset/clerk-update-path-aware.md @@ -0,0 +1,5 @@ +--- +"clerk": patch +--- + +Fix `clerk update` silently writing to the wrong installer when multiple `clerk` binaries exist on PATH. The command now walks PATH to identify the binary the user's shell will actually execute, determines which installer owns that specific path (via a new `ownerOfBinary()` check), and runs the corresponding installer. Binaries installed outside any known package manager (e.g. via `install.sh`) are refused with reinstall guidance rather than silently updated via npm. Also fixes bun detection, which previously matched the shim dir (`~/.bun/bin`) instead of the install dir (`~/.bun/install/global/node_modules`) and fell through to the npm fallback. Adds a `--all` flag to update every `clerk` install on PATH in one run, skipping Homebrew on non-stable channels and unknown-owner binaries with a warning. Prints a `hash -r` / `rehash` hint based on `$SHELL` after a successful update. diff --git a/README.md b/README.md index 3442bf6a..7fcf4a70 100644 --- a/README.md +++ b/README.md @@ -214,10 +214,12 @@ clerk completion clerk update --channel Release channel to update to (e.g. latest, canary) -y, --yes Skip confirmation prompt + --all Update every clerk install found on PATH, not just the first Examples: $ clerk update Update to the latest stable release $ clerk update --channel canary Update to the latest canary release $ clerk update --yes Update without confirmation prompt + $ clerk update --all Update every clerk install on PATH ``` ## Open Questions diff --git a/packages/cli-core/src/cli-program.ts b/packages/cli-core/src/cli-program.ts index 1a636b0b..27d893c0 100644 --- a/packages/cli-core/src/cli-program.ts +++ b/packages/cli-core/src/cli-program.ts @@ -532,6 +532,7 @@ Tutorial — enable completions for your shell: .description("Update the Clerk CLI to the latest version") .option("--channel ", "Release channel to update to (e.g. latest, canary)") .option("-y, --yes", "Skip confirmation prompt") + .option("--all", "Update every clerk install found on PATH, not just the first") .setExamples([ { command: "clerk update", description: "Update to the latest stable release" }, { @@ -539,6 +540,7 @@ Tutorial — enable completions for your shell: description: "Update to the latest canary release", }, { command: "clerk update --yes", description: "Update without confirmation prompt" }, + { command: "clerk update --all", description: "Update every clerk install on PATH" }, ]) .action(update); diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index e3712d43..5f5547c1 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -14,28 +14,39 @@ clerk update [options] | ----------------- | --------------------------------------------------------------------------------- | | `--channel ` | Release channel to update from (default: `latest`; use `canary` for pre-releases) | | `-y, --yes` | Skip confirmation prompt | +| `--all` | Update every clerk install found on PATH, not just the first one | ## Behavior -1. Detects the installer (npm, bun, pnpm, yarn, or Homebrew) in parallel with the version check -2. Fetches the latest version for the given channel from the npm registry -3. If already up to date, exits cleanly -4. For Homebrew installations, prints `brew upgrade clerk` and exits (no auto-install) -5. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) -6. Runs the detected installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) +1. Fetches the latest version for the given channel from the npm registry +2. Walks `PATH` to find every `clerk` binary and picks the first one (the target the user's shell will actually execute) as the **primary target** +3. Determines which installer owns the primary target via `ownerOfBinary()`: + - Known installer (npm/bun/pnpm/yarn) → installs via that PM + - Homebrew → prints `brew upgrade clerk` and exits (stable channel only) + - `null` (binary not owned by any recognized installer, e.g. `install.sh`) → refuses and lists reinstall options +4. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) +5. Runs the installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) +6. With `--all`, repeats for every other `clerk` install on PATH, skipping Homebrew on non-stable channels and `null`-owned binaries +7. After a successful install, prints a shell-specific `hash -r` / `rehash` hint when applicable + +## Why PATH-walking matters + +A machine can host multiple `clerk` installs (bun + asdf-npm + Homebrew is common). `process.execPath` tells you what is running right now, but the binary the user's shell will resolve next may be a different one — e.g. `~/.bun/bin/clerk` shadowing `~/.asdf/shims/clerk`. To ensure the update actually affects the user's next `clerk` invocation, the command resolves the target from `PATH` order, not from `process.execPath`. ## Installer detection -Detection uses a multi-stage algorithm (see `lib/installer.ts`): +Detection uses path-based ownership (see `lib/installer.ts`). For a given binary path: -| Priority | Signal | What it detects | -| -------- | ----------------------------------------------- | ----------------------------------------------------------- | -| 1 | `npm_config_user_agent` env var | PM actively running the CLI (npx, bunx, pnpm dlx, yarn dlx) | -| 2 | `process.execPath` contains `/Cellar/clerk/` | Homebrew (macOS, Linuxbrew) | -| 3 | `process.execPath` matches a PM's global prefix | npm, bun, pnpm, or yarn global install | -| 4 | Fallback | npm | +| Check | Result | +| ----------------------------------------------------- | ------------------------- | +| Contains `/Cellar/clerk/` | `homebrew` | +| Under `/lib/node_modules` | `npm` | +| Under `/install/global/node_modules` | `bun` | +| Under `pnpm root -g` | `pnpm` | +| Under `/node_modules` | `yarn` | +| Nothing matches | `null` (refuse to update) | -`process.execPath` is the real, symlink-resolved path to the compiled binary. For Homebrew, this resolves through the symlink into the Cellar. For npm, this is the platform binary inside `node_modules/`. Symlinked binaries (e.g. `~/.local/bin/clerk` → Cellar) are correctly attributed to their installer. +When multiple PMs' dirs nest, the longest prefix wins. `null` is the signal to refuse rather than silently install via the wrong installer. ## Channels @@ -44,7 +55,7 @@ Detection uses a multi-stage algorithm (see `lib/installer.ts`): | Stable | `latest` | Production-ready releases (default) | | Canary | `canary` | Pre-release builds for early adopters | -Set `CLERK_UPDATE_CHANNEL=canary` to make canary the default for all update checks. +Set `CLERK_UPDATE_CHANNEL=canary` to make canary the default for all update checks. Homebrew is updatable only on `latest` (no canary tap). ## npm registry endpoints @@ -55,6 +66,6 @@ Set `CLERK_UPDATE_CHANNEL=canary` to make canary the default for all update chec ## Notes - Supports 5 installers: npm, bun, pnpm, yarn, and Homebrew. -- Homebrew installations are not auto-updated. The command prints `brew upgrade clerk` and exits. +- Binaries installed via `install.sh` (direct GitHub Release download) are owned by no PM — the update command refuses and lists reinstall options instead of silently writing to a different prefix. - Permission errors (EACCES) suggest retrying with `sudo` using the detected installer's command. - This command does not perform the update itself in agent/non-interactive mode unless `--yes` is passed. diff --git a/packages/cli-core/src/commands/update/index.ts b/packages/cli-core/src/commands/update/index.ts index 770f7d4c..93c1bdd5 100644 --- a/packages/cli-core/src/commands/update/index.ts +++ b/packages/cli-core/src/commands/update/index.ts @@ -1,7 +1,13 @@ import { isHuman } from "../../mode.ts"; -import { green, cyan } from "../../lib/color.ts"; +import { green, cyan, yellow, dim } from "../../lib/color.ts"; import { CliError } from "../../lib/errors.ts"; -import { detectInstaller, globalInstallCommand, type Installer } from "../../lib/installer.ts"; +import { + findClerkOnPath, + getInstallerPackageDirs, + globalInstallCommand, + ownerOfBinary, + type Installer, +} from "../../lib/installer.ts"; import { log } from "../../lib/log.ts"; import { intro, outro, withSpinner } from "../../lib/spinner.ts"; import { UPDATE_PACKAGE_NAME } from "../../lib/constants.ts"; @@ -18,16 +24,43 @@ import { export type UpdateOptions = { channel?: string; yes?: boolean; + all?: boolean; }; -async function confirmUpdate(currentVersion: string, latestVersion: string): Promise { - const { confirm } = await import("@inquirer/prompts"); - return confirm({ - message: `Update clerk ${currentVersion} → ${latestVersion}?`, - default: true, - }); +// ── Target resolution ──────────────────────────────────────────────────────── + +type Target = { + /** Symlink-resolved absolute path to the clerk binary on disk. */ + path: string; + /** Installer that owns this binary, or `null` if none recognized. */ + owner: Installer | null; +}; + +async function resolveTargets( + runningPath: string, + installDirs: Awaited>, +): Promise<{ primary: Target; others: Target[] }> { + const onPath = await findClerkOnPath(); + + // The primary is the first on PATH (what the user's shell will resolve to). + // If PATH discovery came up empty (corporate locked-down env, weird setup), + // fall back to the running binary so we still have a target. + const primaryPath = onPath[0] ?? runningPath; + const others = onPath + .filter((p) => p !== primaryPath) + .map((path) => ({ + path, + owner: ownerOfBinary(path, installDirs), + })); + + return { + primary: { path: primaryPath, owner: ownerOfBinary(primaryPath, installDirs) }, + others, + }; } +// ── Install execution ──────────────────────────────────────────────────────── + async function runGlobalInstall(installer: Installer, packageSpec: string): Promise { let result; switch (installer) { @@ -40,6 +73,9 @@ async function runGlobalInstall(installer: Installer, packageSpec: string): Prom case "yarn": result = await Bun.$`yarn global add ${packageSpec}`.quiet().nothrow(); break; + case "homebrew": + result = await Bun.$`brew upgrade ${UPDATE_PACKAGE_NAME}`.quiet().nothrow(); + break; default: result = await Bun.$`npm install -g ${packageSpec}`.quiet().nothrow(); break; @@ -57,6 +93,62 @@ async function runGlobalInstall(installer: Installer, packageSpec: string): Prom throw new CliError(`Update failed: ${stderr.trim() || "unknown error"}`); } +// ── Skip predicates ────────────────────────────────────────────────────────── + +/** Reason a target cannot be auto-updated (returns null if it can). */ +function whyCantUpdate(target: Target, channel: string): string | null { + if (target.owner === null) { + return "unknown installer — not a package-manager-owned binary"; + } + if (target.owner === "homebrew" && channel !== "latest") { + return `Homebrew has no ${channel} tap — update only works on the stable channel`; + } + return null; +} + +// ── User-facing reporting ──────────────────────────────────────────────────── + +function formatTarget(target: Target): string { + const owner = target.owner ?? dim("unknown"); + return `${target.path} ${dim(`(${owner})`)}`; +} + +function reportOtherInstalls(others: Target[], channel: string): void { + if (others.length === 0) return; + log.blank(); + log.info(`Also found ${others.length} other clerk install${others.length === 1 ? "" : "s"}:`); + for (const t of others) { + const skip = whyCantUpdate(t, channel); + const suffix = skip ? ` ${yellow(`— ${skip}`)}` : ""; + log.info(` ${formatTarget(t)}${suffix}`); + } + log.info(`Run ${cyan("clerk update --all")} to update them too.`); +} + +/** Hint for invalidating the current shell's command-hash cache after update. */ +function hashHint(): string | null { + const shell = (process.env.SHELL ?? "").toLowerCase(); + if (shell.endsWith("/fish") || shell.endsWith("fish.exe")) return null; // auto-rehashes + if (shell.endsWith("/pwsh") || shell.endsWith("powershell.exe")) return null; // no cache + if (shell.endsWith("/tcsh") || shell.endsWith("/csh")) { + return "If `clerk` still points to the old binary, run `rehash` or open a new shell."; + } + // bash, zsh, sh, dash, ksh — all support `hash -r`. + return "If `clerk` still points to the old binary, run `hash -r` or open a new shell."; +} + +// ── Confirmation ───────────────────────────────────────────────────────────── + +async function confirmUpdate(currentVersion: string, latestVersion: string): Promise { + const { confirm } = await import("@inquirer/prompts"); + return confirm({ + message: `Update clerk ${currentVersion} → ${latestVersion}?`, + default: true, + }); +} + +// ── Main ───────────────────────────────────────────────────────────────────── + export async function update(options: UpdateOptions): Promise { const currentVersion = getCurrentVersion(); @@ -69,30 +161,44 @@ export async function update(options: UpdateOptions): Promise { if (isHuman()) intro("clerk update"); - // Detect installer in parallel with the version check - const [latest, installer] = await Promise.all([ + const [latest, installDirs] = await Promise.all([ withSpinner("Checking for updates...", () => fetchLatestVersion(channel)).catch(() => { throw new CliError("Could not reach npm registry. Check your network connection."); }), - detectInstaller(), + getInstallerPackageDirs(), ]); + const { primary, others } = await resolveTargets(process.execPath, installDirs); + if (compareSemver(latest, currentVersion) <= 0) { log.info(`${green("✓")} Already on latest (${currentVersion})`); + reportOtherInstalls(others, channel); if (isHuman()) outro("Up to date"); return; } log.info(` Current: ${currentVersion}`); log.info(` Latest: ${cyan(latest)}${formatChannelLabel(channel)}`); + log.info(` Target: ${formatTarget(primary)}`); log.blank(); - // Homebrew: cannot auto-update — instruct the user instead - if (installer === "homebrew") { - log.info(` Installed via Homebrew. To update, run:`); - log.info(` ${cyan("brew upgrade clerk")}`); - log.blank(); - if (isHuman()) outro("Run `brew upgrade clerk` to update"); + // Primary target can't be updated by us: refuse (don't guess a different installer). + const primarySkip = whyCantUpdate(primary, channel); + if (primarySkip) { + log.warn(`Cannot auto-update: ${primarySkip}`); + if (primary.owner === "homebrew") { + log.info(` Run: ${cyan("brew upgrade clerk")}`); + } else if (primary.owner === null) { + log.info(` This binary appears to be installed outside any known package manager.`); + log.info(` Reinstall via your preferred method, e.g.:`); + log.info(` ${cyan(`bun add -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); + log.info(` ${cyan(`npm install -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); + log.info( + ` ${cyan(`curl -fsSL https://raw.githubusercontent.com/clerk/cli/main/install.sh | bash`)}`, + ); + } + reportOtherInstalls(others, channel); + if (isHuman()) outro("Update required manual action"); return; } @@ -106,13 +212,61 @@ export async function update(options: UpdateOptions): Promise { const packageSpec = `${UPDATE_PACKAGE_NAME}@${latest}`; - await withSpinner( - `Installing ${packageSpec}...`, - () => runGlobalInstall(installer, packageSpec), - `Updated to ${latest}`, - ); + // Build the target list: always primary first, optionally every other updatable install. + const toUpdate: Target[] = [primary]; + if (options.all) { + for (const t of others) { + if (whyCantUpdate(t, channel) === null) toUpdate.push(t); + } + } + + const results: Array<{ target: Target; ok: boolean; error?: string }> = []; + for (const t of toUpdate) { + // `owner` is non-null here because whyCantUpdate returned null for it. + const owner = t.owner as Installer; + try { + await withSpinner( + `Installing ${packageSpec} via ${owner} (${t.path})...`, + () => runGlobalInstall(owner, packageSpec), + `Updated ${owner}: ${t.path}`, + ); + results.push({ target: t, ok: true }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + results.push({ target: t, ok: false, error: message }); + // Keep going for --all; a single failure shouldn't block other installs. + if (!options.all) throw error; + } + } await writeUpdateCache({ checkedAt: Date.now(), latest, distTag: channel }); - if (isHuman()) outro(`Successfully updated to ${latest}`); + // Summary + skipped installs when --all. + if (options.all) { + log.blank(); + log.info("Summary:"); + for (const r of results) { + const icon = r.ok ? green("✓") : yellow("✗"); + const suffix = r.ok ? "" : ` ${yellow(`— ${r.error}`)}`; + log.info(` ${icon} ${formatTarget(r.target)}${suffix}`); + } + for (const t of others) { + const skip = whyCantUpdate(t, channel); + if (!skip) continue; + log.info(` ${yellow("⚠")} ${formatTarget(t)} ${yellow(`— skipped: ${skip}`)}`); + } + } else { + reportOtherInstalls(others, channel); + } + + const hint = hashHint(); + if (hint) { + log.blank(); + log.info(hint); + } + + if (isHuman()) { + const anyFailed = results.some((r) => !r.ok); + outro(anyFailed ? "Update completed with errors" : `Successfully updated to ${latest}`); + } } diff --git a/packages/cli-core/src/lib/installer.test.ts b/packages/cli-core/src/lib/installer.test.ts index 1d387635..6acf1ba4 100644 --- a/packages/cli-core/src/lib/installer.test.ts +++ b/packages/cli-core/src/lib/installer.test.ts @@ -1,9 +1,14 @@ import { test, expect, describe, beforeEach, afterEach } from "bun:test"; +import { chmod, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { delimiter, join } from "node:path"; import { detectFromUserAgent, isHomebrewPath, globalInstallCommand, detectInstaller, + findClerkOnPath, + ownerOfBinary, } from "./installer.ts"; // ── detectFromUserAgent ────────────────────────────────────────────────────── @@ -205,3 +210,174 @@ describe("detectInstaller", () => { expect(await detectInstaller()).toBe("npm"); }); }); + +// ── ownerOfBinary ──────────────────────────────────────────────────────────── + +describe("ownerOfBinary", () => { + const dirs = { + npm: "/opt/homebrew/lib/node_modules", + pnpm: "/Users/x/Library/pnpm/global/5/node_modules", + yarn: "/Users/x/.config/yarn/global/node_modules", + bun: "/Users/x/.bun/install/global/node_modules", + } as const; + + test("returns homebrew for Cellar paths regardless of installDirs", () => { + expect(ownerOfBinary("/opt/homebrew/Cellar/clerk/1.0.0/bin/clerk", dirs)).toBe("homebrew"); + }); + + test("returns bun for paths under bun's install dir", () => { + expect( + ownerOfBinary( + "/Users/x/.bun/install/global/node_modules/@clerk/cli-darwin-arm64/bin/clerk", + dirs, + ), + ).toBe("bun"); + }); + + test("returns npm for paths under npm prefix", () => { + expect( + ownerOfBinary("/opt/homebrew/lib/node_modules/@clerk/cli-darwin-arm64/bin/clerk", dirs), + ).toBe("npm"); + }); + + test("returns pnpm for paths under pnpm's global dir", () => { + expect( + ownerOfBinary( + "/Users/x/Library/pnpm/global/5/node_modules/@clerk/cli-darwin-arm64/bin/clerk", + dirs, + ), + ).toBe("pnpm"); + }); + + test("returns yarn for paths under yarn's global dir", () => { + expect( + ownerOfBinary( + "/Users/x/.config/yarn/global/node_modules/@clerk/cli-darwin-arm64/bin/clerk", + dirs, + ), + ).toBe("yarn"); + }); + + test("returns null for install.sh standalone binaries", () => { + expect(ownerOfBinary("/usr/local/bin/clerk", dirs)).toBe(null); + }); + + test("returns null when no installers are present on the system", () => { + expect( + ownerOfBinary( + "/Users/x/.bun/install/global/node_modules/@clerk/cli-darwin-arm64/bin/clerk", + {}, + ), + ).toBe(null); + }); + + test("trailing separator prevents /a/b matching /a/bother", () => { + // "/a/b" must not match a binary at "/a/bother/clerk". + const nested = { npm: "/a/b" } as const; + expect(ownerOfBinary("/a/bother/clerk", nested)).toBe(null); + expect(ownerOfBinary("/a/b/clerk", nested)).toBe("npm"); + }); + + test("longest match wins when dirs nest", () => { + const nested = { + npm: "/home/x/.asdf/installs/nodejs/22/lib/node_modules", + bun: "/home/x/.asdf/installs/nodejs/22/lib/node_modules/.bun-shim", + } as const; + expect( + ownerOfBinary( + "/home/x/.asdf/installs/nodejs/22/lib/node_modules/.bun-shim/@clerk/cli-linux-x64/bin/clerk", + nested, + ), + ).toBe("bun"); + }); +}); + +// ── findClerkOnPath ────────────────────────────────────────────────────────── + +describe("findClerkOnPath", () => { + let sandbox: string; + let savedPath: string | undefined; + + beforeEach(async () => { + // realpath so symlinks like macOS's /var -> /private/var are resolved + // upfront; findClerkOnPath also realpaths, so the comparison matches. + sandbox = await realpath(await mkdtemp(join(tmpdir(), "clerk-path-test-"))); + savedPath = process.env.PATH; + }); + + afterEach(async () => { + if (savedPath === undefined) delete process.env.PATH; + else process.env.PATH = savedPath; + await rm(sandbox, { recursive: true, force: true }); + }); + + test("returns empty array when PATH has no clerk", async () => { + process.env.PATH = sandbox; + expect(await findClerkOnPath()).toEqual([]); + }); + + test("finds a single executable clerk on PATH", async () => { + const bin = join(sandbox, "clerk"); + await writeFile(bin, "#!/bin/sh\necho fake"); + await chmod(bin, 0o755); + process.env.PATH = sandbox; + const found = await findClerkOnPath(); + expect(found).toEqual([bin]); + }); + + test("skips non-executable files on POSIX", async () => { + if (process.platform === "win32") return; + const bin = join(sandbox, "clerk"); + await writeFile(bin, "#!/bin/sh\necho fake"); + await chmod(bin, 0o644); // no execute bit + process.env.PATH = sandbox; + expect(await findClerkOnPath()).toEqual([]); + }); + + test("skips directories named clerk", async () => { + await mkdir(join(sandbox, "clerk")); + process.env.PATH = sandbox; + expect(await findClerkOnPath()).toEqual([]); + }); + + test("preserves PATH order across multiple hits", async () => { + const dirA = join(sandbox, "a"); + const dirB = join(sandbox, "b"); + await mkdir(dirA); + await mkdir(dirB); + const aBin = join(dirA, "clerk"); + const bBin = join(dirB, "clerk"); + await writeFile(aBin, "#!/bin/sh\necho a"); + await writeFile(bBin, "#!/bin/sh\necho b"); + await chmod(aBin, 0o755); + await chmod(bBin, 0o755); + process.env.PATH = [dirA, dirB].join(delimiter); + expect(await findClerkOnPath()).toEqual([aBin, bBin]); + // Reversed PATH should reverse the order too. + process.env.PATH = [dirB, dirA].join(delimiter); + expect(await findClerkOnPath()).toEqual([bBin, aBin]); + }); + + test("dedupes by realpath when two PATH entries resolve to the same file", async () => { + if (process.platform === "win32") return; // skip symlink test on win32 + const real = join(sandbox, "real"); + const link = join(sandbox, "link"); + await mkdir(real); + await symlink(real, link); + const bin = join(real, "clerk"); + await writeFile(bin, "#!/bin/sh\necho fake"); + await chmod(bin, 0o755); + process.env.PATH = [real, link].join(delimiter); + const found = await findClerkOnPath(); + expect(found.length).toBe(1); + expect(found[0]).toBe(bin); + }); + + test("ignores empty PATH entries (:: as CWD)", async () => { + const bin = join(sandbox, "clerk"); + await writeFile(bin, "#!/bin/sh\necho fake"); + await chmod(bin, 0o755); + process.env.PATH = `${sandbox}${delimiter}${delimiter}`; + expect(await findClerkOnPath()).toEqual([bin]); + }); +}); diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index 1133736e..a52b3a75 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -15,7 +15,10 @@ * user-facing install/update hints. */ -import { realpath } from "node:fs/promises"; +import { constants as fsConstants } from "node:fs"; +import { access, realpath, stat } from "node:fs/promises"; +import { homedir } from "node:os"; +import { delimiter, join, sep } from "node:path"; import { log } from "./log.ts"; import { UPDATE_PACKAGE_NAME } from "./constants.ts"; @@ -131,6 +134,168 @@ export async function detectInstaller(): Promise { return "npm"; } +// ── Strategy B: PATH-priority-aware update detection ──────────────────────── +// +// The single-`detectInstaller()` call is not enough: on a machine with more +// than one global install (e.g. bun + asdf-managed npm), `npm install -g` can +// land in the wrong prefix while the shell still resolves `clerk` to the +// bun-installed binary. To fix this the update command needs to (1) discover +// every `clerk` on PATH in PATH order, (2) ask which installer owns the FIRST +// one, and (3) run that installer — not whatever `process.execPath` suggests. +// +// The helpers below implement steps (1) and (2a). Step (2b) — the actual +// path→installer decision — is intentionally left as a TODO for a contributor +// to fill in; see `ownerOfBinary` at the bottom. + +/** + * Walk the current process PATH and return symlink-resolved absolute paths to + * every `clerk` binary found, in PATH order. Duplicates (same realpath reached + * via two PATH entries) are collapsed; the first occurrence wins so PATH order + * is preserved. + * + * Shell-agnostic by design — reads `process.env.PATH` directly rather than + * shelling out to `which`/`where`, so it behaves identically under bash, zsh, + * fish, Nushell, PowerShell, cmd, and any other shell that exports a standard + * PATH to child processes. The sole shell-specific concern post-update is the + * command hash table (bash/zsh `hash`, zsh/tcsh `rehash`, fish auto-rehashes) + * — handled separately in the success message, not here. + * + * Platform handling: + * - POSIX: iterates PATH as-is, filters to regular files with the X bit set. + * Empty PATH entries (common via `::`) are ignored rather than being + * treated as CWD — safer default, matches most modern shells. + * - Windows: iterates PATHEXT extensions in declared order (first match per + * dir wins, matching Windows resolution), accepts any regular file (the + * X bit is meaningless on NTFS; PATHEXT is the gate). + */ +export async function findClerkOnPath(binaryName = UPDATE_PACKAGE_NAME): Promise { + const dirs = (process.env.PATH ?? "").split(delimiter).filter(Boolean); + const exts = + process.platform === "win32" + ? (process.env.PATHEXT ?? ".EXE;.CMD;.BAT").split(";").filter(Boolean) + : [""]; + + const resolved: string[] = []; + const seen = new Set(); + for (const dir of dirs) { + for (const ext of exts) { + const candidate = join(dir, `${binaryName}${ext}`); + if (!(await isExecutableFile(candidate))) continue; + let real: string; + try { + real = await realpath(candidate); + } catch { + continue; + } + if (seen.has(real)) continue; + seen.add(real); + resolved.push(real); + break; // first matching extension in this dir wins (Windows resolution order) + } + } + return resolved; +} + +/** + * Is this path a regular file that the current process can actually execute? + * On POSIX, "executable" means the X bit is set for the effective user. + * On Windows, NTFS has no concept of executability — extension filtering in + * findClerkOnPath() is what gates executability there, so we only confirm + * the path is a regular file. + */ +async function isExecutableFile(path: string): Promise { + try { + const s = await stat(path); + if (!s.isFile()) return false; + if (process.platform === "win32") return true; + await access(path, fsConstants.X_OK); + return true; + } catch { + return false; + } +} + +/** + * Directories where each PM stores its globally-installed packages — i.e. the + * parent of the `@clerk/cli-/` package folder, NOT where PMs put shim + * symlinks. This is the bun-detection bug fix: `bun pm bin -g` returns the + * symlink dir (~/.bun/bin), but the compiled platform binary actually lives + * in ~/.bun/install/global/node_modules — so matching against the symlink dir + * never succeeds. All four PMs are queried in parallel; a PM that isn't on + * the system (nonzero exit, no output) is omitted from the result. + */ +export async function getInstallerPackageDirs(): Promise>> { + const queries: Array<[Installer, Promise]> = [ + ["npm", queryNpmPackageDir()], + ["pnpm", queryPnpmPackageDir()], + ["yarn", queryYarnPackageDir()], + ["bun", queryBunPackageDir()], + ]; + const out: Partial> = {}; + for (const [pm, p] of queries) { + const dir = await p; + if (dir) out[pm] = dir; + } + return out; +} + +async function queryNpmPackageDir(): Promise { + const result = await Bun.$`npm prefix -g`.quiet().nothrow(); + if (result.exitCode !== 0) return null; + const prefix = result.stdout.toString().trim(); + return prefix ? await safeRealpath(join(prefix, "lib", "node_modules")) : null; +} + +async function queryPnpmPackageDir(): Promise { + const result = await Bun.$`pnpm root -g`.quiet().nothrow(); + if (result.exitCode !== 0) return null; + const dir = result.stdout.toString().trim(); + return dir ? await safeRealpath(dir) : null; +} + +async function queryYarnPackageDir(): Promise { + const result = await Bun.$`yarn global dir`.quiet().nothrow(); + if (result.exitCode !== 0) return null; + const dir = result.stdout.toString().trim(); + return dir ? await safeRealpath(join(dir, "node_modules")) : null; +} + +async function queryBunPackageDir(): Promise { + // $BUN_INSTALL defaults to ~/.bun; packages live at $BUN_INSTALL/install/global/node_modules. + // Falling back to the conventional path is safe because `clerk update` only runs from an + // installed clerk, which means Bun's layout (if used) is already in place. + const root = process.env.BUN_INSTALL ?? join(homedir(), ".bun"); + return await safeRealpath(join(root, "install", "global", "node_modules")); +} + +/** + * Given the symlink-resolved absolute path to a clerk binary and the install + * dirs returned by `getInstallerPackageDirs()`, return which installer owns + * that specific binary — or `null` if no known installer does. + * + * Homebrew is matched first via its distinctive `/Cellar/clerk/` pattern. + * Each PM dir is matched with a trailing path separator to avoid the + * `/a/b` vs `/a/bother` false-positive. When multiple PMs match (possible + * under unusual nested-prefix configurations), the longest match wins — + * a more specific prefix is a better answer than a shorter one. + * + * Returns `null` (NOT `"npm"`) when nothing matches. Callers use `null` as + * the signal to refuse-rather-than-guess. + */ +export function ownerOfBinary( + binaryPath: string, + installDirs: Partial>, +): Installer | null { + if (isHomebrewPath(binaryPath)) return "homebrew"; + + let best: { installer: Installer; len: number } | null = null; + for (const [pm, dir] of Object.entries(installDirs) as Array<[Installer, string]>) { + if (!dir || !binaryPath.startsWith(dir + sep)) continue; + if (!best || dir.length > best.len) best = { installer: pm, len: dir.length }; + } + return best?.installer ?? null; +} + /** * Returns a human-readable install/update command for the given installer. */ From 581559acd1d144fc761dae4e9688da7c07492333 Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Fri, 17 Apr 2026 12:46:32 -0300 Subject: [PATCH 2/6] fix(update): handle asdf shims via `asdf which` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit asdf shims are bash scripts (not symlinks), so realpath returns the shim path itself and ownerOfBinary can't match it against any PM prefix. The PATH survey marked asdf-installed clerks as "unknown installer" and --all would skip them with a warning. resolveAsdfShim() calls `asdf which ` to find the underlying binary, realpaths the result, and feeds it into ownerOfBinary. Non- shim paths pass through unchanged. When asdf is absent or can't resolve, the shim path is returned and ownerOfBinary correctly reports null — preserving the refuse-on-unknown behavior. After a successful install, runs `asdf reshim ` for every asdf plugin whose binary was updated. Safety net for asdf versions that don't auto-reshim on npm install. Target splits into displayPath (what's on PATH — the shim) and resolvedPath (what ownerOfBinary consults) so users see the familiar shim path in output while install logic operates on the resolved binary. --- .../cli-core/src/commands/update/README.md | 12 ++- .../cli-core/src/commands/update/index.ts | 65 ++++++++++---- packages/cli-core/src/lib/installer.test.ts | 88 +++++++++++++++++++ packages/cli-core/src/lib/installer.ts | 45 ++++++++++ 4 files changed, 189 insertions(+), 21 deletions(-) diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index 5f5547c1..5dcf57bd 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -19,20 +19,26 @@ clerk update [options] ## Behavior 1. Fetches the latest version for the given channel from the npm registry -2. Walks `PATH` to find every `clerk` binary and picks the first one (the target the user's shell will actually execute) as the **primary target** +2. Walks `PATH` to find every `clerk` binary. For asdf shims (bash scripts, not symlinks), resolves through `asdf which ` so the underlying installer is visible. Picks the first one as the **primary target** 3. Determines which installer owns the primary target via `ownerOfBinary()`: - Known installer (npm/bun/pnpm/yarn) → installs via that PM - Homebrew → prints `brew upgrade clerk` and exits (stable channel only) - `null` (binary not owned by any recognized installer, e.g. `install.sh`) → refuses and lists reinstall options 4. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) 5. Runs the installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) -6. With `--all`, repeats for every other `clerk` install on PATH, skipping Homebrew on non-stable channels and `null`-owned binaries -7. After a successful install, prints a shell-specific `hash -r` / `rehash` hint when applicable +6. If any updated target was inside an asdf-managed tool, runs `asdf reshim ` so the shim picks up the new binary (safety net — modern asdf-nodejs auto-reshims) +7. With `--all`, repeats for every other `clerk` install on PATH, skipping Homebrew on non-stable channels and `null`-owned binaries +8. After a successful install, prints a shell-specific `hash -r` / `rehash` hint when applicable ## Why PATH-walking matters A machine can host multiple `clerk` installs (bun + asdf-npm + Homebrew is common). `process.execPath` tells you what is running right now, but the binary the user's shell will resolve next may be a different one — e.g. `~/.bun/bin/clerk` shadowing `~/.asdf/shims/clerk`. To ensure the update actually affects the user's next `clerk` invocation, the command resolves the target from `PATH` order, not from `process.execPath`. +## Version managers (asdf, nvm) + +- **nvm**: fully supported without special handling. nvm uses real symlinks, so `realpath` chases them into `/lib/node_modules/clerk/bin/clerk`, which matches the active `npm prefix -g` — `ownerOfBinary` returns `"npm"`. +- **asdf**: handled via `resolveAsdfShim()`. asdf shims (`~/.asdf/shims/`) are bash scripts, not symlinks, so `realpath` returns the shim itself. The update command calls `asdf which ` to find the underlying binary (e.g. `~/.asdf/installs/nodejs/22.16.0/bin/clerk`), realpaths it into the asdf-managed node's `lib/node_modules`, and treats it as `"npm"` with a post-install `asdf reshim `. Honors `$ASDF_DATA_DIR` when set. If `asdf` isn't on PATH the shim path is returned unchanged and ownership falls back to `null`. + ## Installer detection Detection uses path-based ownership (see `lib/installer.ts`). For a given binary path: diff --git a/packages/cli-core/src/commands/update/index.ts b/packages/cli-core/src/commands/update/index.ts index 93c1bdd5..27eef035 100644 --- a/packages/cli-core/src/commands/update/index.ts +++ b/packages/cli-core/src/commands/update/index.ts @@ -2,10 +2,13 @@ import { isHuman } from "../../mode.ts"; import { green, cyan, yellow, dim } from "../../lib/color.ts"; import { CliError } from "../../lib/errors.ts"; import { + asdfPluginFromPath, + asdfReshim, findClerkOnPath, getInstallerPackageDirs, globalInstallCommand, ownerOfBinary, + resolveAsdfShim, type Installer, } from "../../lib/installer.ts"; import { log } from "../../lib/log.ts"; @@ -30,9 +33,10 @@ export type UpdateOptions = { // ── Target resolution ──────────────────────────────────────────────────────── type Target = { - /** Symlink-resolved absolute path to the clerk binary on disk. */ - path: string; - /** Installer that owns this binary, or `null` if none recognized. */ + /** Path as it appears on PATH (shown to the user). */ + displayPath: string; + /** Underlying binary after asdf-shim resolution; equal to displayPath for non-shim targets. */ + resolvedPath: string; owner: Installer | null; }; @@ -42,20 +46,34 @@ async function resolveTargets( ): Promise<{ primary: Target; others: Target[] }> { const onPath = await findClerkOnPath(); - // The primary is the first on PATH (what the user's shell will resolve to). - // If PATH discovery came up empty (corporate locked-down env, weird setup), - // fall back to the running binary so we still have a target. - const primaryPath = onPath[0] ?? runningPath; - const others = onPath - .filter((p) => p !== primaryPath) - .map((path) => ({ - path, - owner: ownerOfBinary(path, installDirs), - })); + const resolved = await Promise.all(onPath.map((p) => resolveAsdfShim(p))); + const candidates = onPath.map((displayPath, i) => ({ + displayPath, + resolvedPath: resolved[i]!, + })); + + // Dedupe by resolvedPath: a shim and its resolved target shouldn't both appear. + const seen = new Set(); + const unique: Array<{ displayPath: string; resolvedPath: string }> = []; + for (const c of candidates) { + if (seen.has(c.resolvedPath)) continue; + seen.add(c.resolvedPath); + unique.push(c); + } + + // Fallback when PATH discovery yields nothing: use the running binary. + const effective = + unique.length > 0 ? unique : [{ displayPath: runningPath, resolvedPath: runningPath }]; + + const toTarget = (c: { displayPath: string; resolvedPath: string }): Target => ({ + displayPath: c.displayPath, + resolvedPath: c.resolvedPath, + owner: ownerOfBinary(c.resolvedPath, installDirs), + }); return { - primary: { path: primaryPath, owner: ownerOfBinary(primaryPath, installDirs) }, - others, + primary: toTarget(effective[0]!), + others: effective.slice(1).map(toTarget), }; } @@ -110,7 +128,7 @@ function whyCantUpdate(target: Target, channel: string): string | null { function formatTarget(target: Target): string { const owner = target.owner ?? dim("unknown"); - return `${target.path} ${dim(`(${owner})`)}`; + return `${target.displayPath} ${dim(`(${owner})`)}`; } function reportOtherInstalls(others: Target[], channel: string): void { @@ -226,9 +244,9 @@ export async function update(options: UpdateOptions): Promise { const owner = t.owner as Installer; try { await withSpinner( - `Installing ${packageSpec} via ${owner} (${t.path})...`, + `Installing ${packageSpec} via ${owner} (${t.displayPath})...`, () => runGlobalInstall(owner, packageSpec), - `Updated ${owner}: ${t.path}`, + `Updated ${owner}: ${t.displayPath}`, ); results.push({ target: t, ok: true }); } catch (error) { @@ -239,6 +257,17 @@ export async function update(options: UpdateOptions): Promise { } } + // Safety net: modern asdf-nodejs auto-reshims on npm install, older ones don't. + const asdfPlugins = new Set(); + for (const r of results) { + if (!r.ok) continue; + const plugin = asdfPluginFromPath(r.target.resolvedPath); + if (plugin) asdfPlugins.add(plugin); + } + for (const plugin of asdfPlugins) { + await asdfReshim(plugin); + } + await writeUpdateCache({ checkedAt: Date.now(), latest, distTag: channel }); // Summary + skipped installs when --all. diff --git a/packages/cli-core/src/lib/installer.test.ts b/packages/cli-core/src/lib/installer.test.ts index 6acf1ba4..e67efe8c 100644 --- a/packages/cli-core/src/lib/installer.test.ts +++ b/packages/cli-core/src/lib/installer.test.ts @@ -9,6 +9,9 @@ import { detectInstaller, findClerkOnPath, ownerOfBinary, + isAsdfShimPath, + asdfPluginFromPath, + resolveAsdfShim, } from "./installer.ts"; // ── detectFromUserAgent ────────────────────────────────────────────────────── @@ -381,3 +384,88 @@ describe("findClerkOnPath", () => { expect(await findClerkOnPath()).toEqual([bin]); }); }); + +// ── asdf helpers ───────────────────────────────────────────────────────────── + +describe("isAsdfShimPath", () => { + let savedDataDir: string | undefined; + + beforeEach(() => { + savedDataDir = process.env.ASDF_DATA_DIR; + }); + afterEach(() => { + if (savedDataDir === undefined) delete process.env.ASDF_DATA_DIR; + else process.env.ASDF_DATA_DIR = savedDataDir; + }); + + test("matches paths under the default ~/.asdf/shims directory", () => { + delete process.env.ASDF_DATA_DIR; + const home = process.env.HOME ?? ""; + expect(isAsdfShimPath(`${home}/.asdf/shims/clerk`)).toBe(true); + }); + + test("matches paths under an ASDF_DATA_DIR override", () => { + process.env.ASDF_DATA_DIR = "/opt/asdf-data"; + expect(isAsdfShimPath("/opt/asdf-data/shims/clerk")).toBe(true); + }); + + test("rejects non-shim paths (including trailing-separator-adjacent names)", () => { + process.env.ASDF_DATA_DIR = "/opt/asdf-data"; + expect(isAsdfShimPath("/opt/asdf-data/installs/nodejs/22/bin/clerk")).toBe(false); + expect(isAsdfShimPath("/usr/local/bin/clerk")).toBe(false); + expect(isAsdfShimPath("/opt/asdf-data/shimsxyz/clerk")).toBe(false); + }); +}); + +describe("asdfPluginFromPath", () => { + let savedDataDir: string | undefined; + + beforeEach(() => { + savedDataDir = process.env.ASDF_DATA_DIR; + process.env.ASDF_DATA_DIR = "/opt/asdf-data"; + }); + afterEach(() => { + if (savedDataDir === undefined) delete process.env.ASDF_DATA_DIR; + else process.env.ASDF_DATA_DIR = savedDataDir; + }); + + test("extracts the plugin name from a nodejs installs path", () => { + expect( + asdfPluginFromPath("/opt/asdf-data/installs/nodejs/22.16.0/lib/node_modules/clerk/bin/clerk"), + ).toBe("nodejs"); + }); + + test("returns null for paths outside the installs tree", () => { + expect(asdfPluginFromPath("/opt/asdf-data/shims/clerk")).toBe(null); + expect(asdfPluginFromPath("/usr/local/bin/clerk")).toBe(null); + }); + + test("returns null when the installs path has no plugin segment", () => { + expect(asdfPluginFromPath("/opt/asdf-data/installs")).toBe(null); + expect(asdfPluginFromPath("/opt/asdf-data/installs/")).toBe(null); + }); +}); + +describe("resolveAsdfShim", () => { + let savedDataDir: string | undefined; + + beforeEach(() => { + savedDataDir = process.env.ASDF_DATA_DIR; + }); + afterEach(() => { + if (savedDataDir === undefined) delete process.env.ASDF_DATA_DIR; + else process.env.ASDF_DATA_DIR = savedDataDir; + }); + + test("returns non-shim paths unchanged", async () => { + process.env.ASDF_DATA_DIR = "/opt/asdf-data"; + expect(await resolveAsdfShim("/usr/local/bin/clerk")).toBe("/usr/local/bin/clerk"); + expect(await resolveAsdfShim("/opt/homebrew/bin/clerk")).toBe("/opt/homebrew/bin/clerk"); + }); + + test("returns shim path unchanged when `asdf which` fails", async () => { + process.env.ASDF_DATA_DIR = "/nonexistent/asdf-sandbox"; + const shim = "/nonexistent/asdf-sandbox/shims/definitely-not-a-real-binary-xyzzy"; + expect(await resolveAsdfShim(shim)).toBe(shim); + }); +}); diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index a52b3a75..f01fd261 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -268,6 +268,51 @@ async function queryBunPackageDir(): Promise { return await safeRealpath(join(root, "install", "global", "node_modules")); } +// ── asdf shim handling ─────────────────────────────────────────────────────── + +// asdf shims are bash scripts, not symlinks — `realpath` returns the shim +// itself, so ownerOfBinary can't match it against an installer's package +// dir. `asdf which` is the only way to chase the shim to the real binary. + +function asdfShimsDir(): string { + return join(process.env.ASDF_DATA_DIR ?? join(homedir(), ".asdf"), "shims"); +} + +export function isAsdfShimPath(path: string): boolean { + return path.startsWith(asdfShimsDir() + sep); +} + +/** Returns `path` unchanged when not a shim, asdf is missing, or resolution fails. */ +export async function resolveAsdfShim(path: string): Promise { + if (!isAsdfShimPath(path)) return path; + const name = path.slice((asdfShimsDir() + sep).length).split(sep)[0]; + if (!name) return path; + try { + const result = await Bun.$`asdf which ${name}`.quiet().nothrow(); + if (result.exitCode !== 0) return path; + const real = result.stdout.toString().trim(); + if (!real) return path; + return await safeRealpath(real); + } catch { + return path; + } +} + +export function asdfPluginFromPath(path: string): string | null { + const installsDir = join(process.env.ASDF_DATA_DIR ?? join(homedir(), ".asdf"), "installs"); + const prefix = installsDir + sep; + if (!path.startsWith(prefix)) return null; + const plugin = path.slice(prefix.length).split(sep)[0]; + return plugin || null; +} + +/** Best-effort; swallows errors. */ +export async function asdfReshim(plugin: string): Promise { + try { + await Bun.$`asdf reshim ${plugin}`.quiet().nothrow(); + } catch {} +} + /** * Given the symlink-resolved absolute path to a clerk binary and the install * dirs returned by `getInstallerPackageDirs()`, return which installer owns From 71b730b52e6662a7d38647d686aa9a99fe036e90 Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Fri, 17 Apr 2026 14:39:51 -0300 Subject: [PATCH 3/6] refactor(installer): address review feedback - Remove dead detectInstaller, detectFromUserAgent, matchPmFromExecPath, and queryPmPrefix. They were the legacy runtime-based detection path, superseded by ownerOfBinary + findClerkOnPath. Only installer.test.ts still imported them. - Drop the stale "TODO for a contributor" comment in the Strategy B section header; ownerOfBinary has been implemented. - Rewrite getInstallerPackageDirs with Promise.all so the parallelism is obvious at the call site. - Replace em-dashes with regular punctuation throughout installer.ts, update/index.ts, and update/README.md. - Fix formatTarget double-dim: dim("unknown") was being wrapped inside another dim(...) in the null-owner case; the inner ANSI reset broke the outer styling. Pass the raw string instead. --- .../cli-core/src/commands/update/README.md | 8 +- .../cli-core/src/commands/update/index.ts | 17 +- packages/cli-core/src/lib/installer.test.ts | 140 ---------- packages/cli-core/src/lib/installer.ts | 254 +++++------------- 4 files changed, 83 insertions(+), 336 deletions(-) diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index 5dcf57bd..c4981f8b 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -26,17 +26,17 @@ clerk update [options] - `null` (binary not owned by any recognized installer, e.g. `install.sh`) → refuses and lists reinstall options 4. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) 5. Runs the installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) -6. If any updated target was inside an asdf-managed tool, runs `asdf reshim ` so the shim picks up the new binary (safety net — modern asdf-nodejs auto-reshims) +6. If any updated target was inside an asdf-managed tool, runs `asdf reshim ` so the shim picks up the new binary (safety net; modern asdf-nodejs auto-reshims) 7. With `--all`, repeats for every other `clerk` install on PATH, skipping Homebrew on non-stable channels and `null`-owned binaries 8. After a successful install, prints a shell-specific `hash -r` / `rehash` hint when applicable ## Why PATH-walking matters -A machine can host multiple `clerk` installs (bun + asdf-npm + Homebrew is common). `process.execPath` tells you what is running right now, but the binary the user's shell will resolve next may be a different one — e.g. `~/.bun/bin/clerk` shadowing `~/.asdf/shims/clerk`. To ensure the update actually affects the user's next `clerk` invocation, the command resolves the target from `PATH` order, not from `process.execPath`. +A machine can host multiple `clerk` installs (bun + asdf-npm + Homebrew is common). `process.execPath` tells you what is running right now, but the binary the user's shell will resolve next may be a different one (e.g. `~/.bun/bin/clerk` shadowing `~/.asdf/shims/clerk`). To ensure the update actually affects the user's next `clerk` invocation, the command resolves the target from `PATH` order, not from `process.execPath`. ## Version managers (asdf, nvm) -- **nvm**: fully supported without special handling. nvm uses real symlinks, so `realpath` chases them into `/lib/node_modules/clerk/bin/clerk`, which matches the active `npm prefix -g` — `ownerOfBinary` returns `"npm"`. +- **nvm**: fully supported without special handling. nvm uses real symlinks, so `realpath` chases them into `/lib/node_modules/clerk/bin/clerk`, which matches the active `npm prefix -g`; `ownerOfBinary` returns `"npm"`. - **asdf**: handled via `resolveAsdfShim()`. asdf shims (`~/.asdf/shims/`) are bash scripts, not symlinks, so `realpath` returns the shim itself. The update command calls `asdf which ` to find the underlying binary (e.g. `~/.asdf/installs/nodejs/22.16.0/bin/clerk`), realpaths it into the asdf-managed node's `lib/node_modules`, and treats it as `"npm"` with a post-install `asdf reshim `. Honors `$ASDF_DATA_DIR` when set. If `asdf` isn't on PATH the shim path is returned unchanged and ownership falls back to `null`. ## Installer detection @@ -72,6 +72,6 @@ Set `CLERK_UPDATE_CHANNEL=canary` to make canary the default for all update chec ## Notes - Supports 5 installers: npm, bun, pnpm, yarn, and Homebrew. -- Binaries installed via `install.sh` (direct GitHub Release download) are owned by no PM — the update command refuses and lists reinstall options instead of silently writing to a different prefix. +- Binaries installed via `install.sh` (direct GitHub Release download) are owned by no PM; the update command refuses and lists reinstall options instead of silently writing to a different prefix. - Permission errors (EACCES) suggest retrying with `sudo` using the detected installer's command. - This command does not perform the update itself in agent/non-interactive mode unless `--yes` is passed. diff --git a/packages/cli-core/src/commands/update/index.ts b/packages/cli-core/src/commands/update/index.ts index 27eef035..a86de7a6 100644 --- a/packages/cli-core/src/commands/update/index.ts +++ b/packages/cli-core/src/commands/update/index.ts @@ -116,10 +116,10 @@ async function runGlobalInstall(installer: Installer, packageSpec: string): Prom /** Reason a target cannot be auto-updated (returns null if it can). */ function whyCantUpdate(target: Target, channel: string): string | null { if (target.owner === null) { - return "unknown installer — not a package-manager-owned binary"; + return "unknown installer (not a package-manager-owned binary)"; } if (target.owner === "homebrew" && channel !== "latest") { - return `Homebrew has no ${channel} tap — update only works on the stable channel`; + return `Homebrew has no ${channel} tap; update only works on the stable channel`; } return null; } @@ -127,8 +127,7 @@ function whyCantUpdate(target: Target, channel: string): string | null { // ── User-facing reporting ──────────────────────────────────────────────────── function formatTarget(target: Target): string { - const owner = target.owner ?? dim("unknown"); - return `${target.displayPath} ${dim(`(${owner})`)}`; + return `${target.displayPath} ${dim(`(${target.owner ?? "unknown"})`)}`; } function reportOtherInstalls(others: Target[], channel: string): void { @@ -137,7 +136,7 @@ function reportOtherInstalls(others: Target[], channel: string): void { log.info(`Also found ${others.length} other clerk install${others.length === 1 ? "" : "s"}:`); for (const t of others) { const skip = whyCantUpdate(t, channel); - const suffix = skip ? ` ${yellow(`— ${skip}`)}` : ""; + const suffix = skip ? ` ${yellow(`- ${skip}`)}` : ""; log.info(` ${formatTarget(t)}${suffix}`); } log.info(`Run ${cyan("clerk update --all")} to update them too.`); @@ -151,7 +150,7 @@ function hashHint(): string | null { if (shell.endsWith("/tcsh") || shell.endsWith("/csh")) { return "If `clerk` still points to the old binary, run `rehash` or open a new shell."; } - // bash, zsh, sh, dash, ksh — all support `hash -r`. + // bash, zsh, sh, dash, ksh all support `hash -r`. return "If `clerk` still points to the old binary, run `hash -r` or open a new shell."; } @@ -171,7 +170,7 @@ export async function update(options: UpdateOptions): Promise { const currentVersion = getCurrentVersion(); if (isDevVersion(currentVersion)) { - log.info("Running development build (0.0.0-dev) — update not applicable."); + log.info("Running development build (0.0.0-dev); update not applicable."); return; } @@ -276,13 +275,13 @@ export async function update(options: UpdateOptions): Promise { log.info("Summary:"); for (const r of results) { const icon = r.ok ? green("✓") : yellow("✗"); - const suffix = r.ok ? "" : ` ${yellow(`— ${r.error}`)}`; + const suffix = r.ok ? "" : ` ${yellow(`- ${r.error}`)}`; log.info(` ${icon} ${formatTarget(r.target)}${suffix}`); } for (const t of others) { const skip = whyCantUpdate(t, channel); if (!skip) continue; - log.info(` ${yellow("⚠")} ${formatTarget(t)} ${yellow(`— skipped: ${skip}`)}`); + log.info(` ${yellow("⚠")} ${formatTarget(t)} ${yellow(`- skipped: ${skip}`)}`); } } else { reportOtherInstalls(others, channel); diff --git a/packages/cli-core/src/lib/installer.test.ts b/packages/cli-core/src/lib/installer.test.ts index e67efe8c..b5af66a2 100644 --- a/packages/cli-core/src/lib/installer.test.ts +++ b/packages/cli-core/src/lib/installer.test.ts @@ -3,10 +3,8 @@ import { chmod, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from "node:fs import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { - detectFromUserAgent, isHomebrewPath, globalInstallCommand, - detectInstaller, findClerkOnPath, ownerOfBinary, isAsdfShimPath, @@ -14,54 +12,6 @@ import { resolveAsdfShim, } from "./installer.ts"; -// ── detectFromUserAgent ────────────────────────────────────────────────────── - -describe("detectFromUserAgent", () => { - let savedUA: string | undefined; - - beforeEach(() => { - savedUA = process.env.npm_config_user_agent; - }); - - afterEach(() => { - if (savedUA === undefined) { - delete process.env.npm_config_user_agent; - } else { - process.env.npm_config_user_agent = savedUA; - } - }); - - test("detects bun", () => { - process.env.npm_config_user_agent = "bun/1.3.9"; - expect(detectFromUserAgent()).toBe("bun"); - }); - - test("detects pnpm", () => { - process.env.npm_config_user_agent = "pnpm/8.15.0 npm/? node/v22.0.0"; - expect(detectFromUserAgent()).toBe("pnpm"); - }); - - test("detects yarn", () => { - process.env.npm_config_user_agent = "yarn/3.6.0 npm/? node/v22.0.0"; - expect(detectFromUserAgent()).toBe("yarn"); - }); - - test("detects npm", () => { - process.env.npm_config_user_agent = "npm/10.5.0 node/v22.0.0 darwin arm64"; - expect(detectFromUserAgent()).toBe("npm"); - }); - - test("returns null for empty string", () => { - process.env.npm_config_user_agent = ""; - expect(detectFromUserAgent()).toBeNull(); - }); - - test("returns null when unset", () => { - delete process.env.npm_config_user_agent; - expect(detectFromUserAgent()).toBeNull(); - }); -}); - // ── isHomebrewPath ─────────────────────────────────────────────────────────── describe("isHomebrewPath", () => { @@ -124,96 +74,6 @@ describe("globalInstallCommand", () => { }); }); -// ── detectInstaller ────────────────────────────────────────────────────────── - -describe("detectInstaller", () => { - let savedUA: string | undefined; - let savedExecPath: string; - - beforeEach(() => { - savedUA = process.env.npm_config_user_agent; - savedExecPath = process.execPath; - delete process.env.npm_config_user_agent; - }); - - afterEach(() => { - if (savedUA === undefined) { - delete process.env.npm_config_user_agent; - } else { - process.env.npm_config_user_agent = savedUA; - } - Object.defineProperty(process, "execPath", { value: savedExecPath, writable: true }); - }); - - function setExecPath(path: string) { - Object.defineProperty(process, "execPath", { value: path, writable: true }); - } - - // ── Stage 1: npm_config_user_agent ───────────────────────────────────── - - test("stage 1: returns bun when npm_config_user_agent starts with bun/", async () => { - process.env.npm_config_user_agent = "bun/1.3.9"; - expect(await detectInstaller()).toBe("bun"); - }); - - test("stage 1: returns pnpm when npm_config_user_agent starts with pnpm/", async () => { - process.env.npm_config_user_agent = "pnpm/8.15.0 npm/? node/v22.0.0"; - expect(await detectInstaller()).toBe("pnpm"); - }); - - test("stage 1: returns yarn when npm_config_user_agent starts with yarn/", async () => { - process.env.npm_config_user_agent = "yarn/3.6.0 npm/? node/v22.0.0"; - expect(await detectInstaller()).toBe("yarn"); - }); - - test("stage 1: returns npm when npm_config_user_agent starts with npm/", async () => { - process.env.npm_config_user_agent = "npm/10.5.0 node/v22.0.0 darwin arm64"; - expect(await detectInstaller()).toBe("npm"); - }); - - test("stage 1: takes priority over Homebrew execPath", async () => { - process.env.npm_config_user_agent = "npm/10.5.0"; - setExecPath("/opt/homebrew/Cellar/clerk/1.0.0/bin/clerk"); - expect(await detectInstaller()).toBe("npm"); - }); - - // ── Stage 2a: Homebrew ───────────────────────────────────────────────── - - test("stage 2a: detects Homebrew from Apple Silicon Cellar path", async () => { - setExecPath("/opt/homebrew/Cellar/clerk/1.0.0/bin/clerk"); - expect(await detectInstaller()).toBe("homebrew"); - }); - - test("stage 2a: detects Homebrew from Intel Cellar path", async () => { - setExecPath("/usr/local/Cellar/clerk/2.0.0/bin/clerk"); - expect(await detectInstaller()).toBe("homebrew"); - }); - - test("stage 2a: detects Linuxbrew from Cellar path", async () => { - setExecPath("/home/linuxbrew/.linuxbrew/Cellar/clerk/1.0.0/bin/clerk"); - expect(await detectInstaller()).toBe("homebrew"); - }); - - // ── Stage 2b: PM prefix matching ─────────────────────────────────────── - - test("stage 2b: detects npm when execPath is under npm global prefix", async () => { - const result = Bun.spawnSync(["npm", "prefix", "-g"], { stdout: "pipe", stderr: "pipe" }); - if (result.exitCode !== 0) return; - const prefix = new TextDecoder().decode(result.stdout).trim(); - if (!prefix) return; - - setExecPath(`${prefix}/lib/node_modules/@clerk/cli-darwin-arm64/bin/clerk`); - expect(await detectInstaller()).toBe("npm"); - }); - - // ── Stage 3: Fallback ───────────────────────────────────────────────── - - test("stage 3: falls back to npm for unrecognized execPath", async () => { - setExecPath("/some/totally/unknown/path/to/clerk"); - expect(await detectInstaller()).toBe("npm"); - }); -}); - // ── ownerOfBinary ──────────────────────────────────────────────────────────── describe("ownerOfBinary", () => { diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index f01fd261..02117f6b 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -1,25 +1,16 @@ /** - * Global installer detection. + * Path-based installer detection for `clerk update`. * - * Detects how the CLI was installed globally — npm, bun, pnpm, yarn, or - * Homebrew — so the update command can use the correct update mechanism. - * - * Detection priority: - * 1. `npm_config_user_agent` env var (set when invoked via a PM script runner) - * 2. `process.execPath` — the real, symlink-resolved binary path: - * a. Contains `/Cellar/clerk/` → Homebrew - * b. Matches a PM's global prefix → that PM - * 3. Falls back to npm - * - * This module also provides `globalInstallCommand()` for building - * user-facing install/update hints. + * Maps a resolved binary path to the package manager that owns it by comparing + * against each PM's install directory. Handles asdf shims via `asdf which` and + * exposes helpers for walking PATH and invoking per-installer global install + * commands. */ import { constants as fsConstants } from "node:fs"; import { access, realpath, stat } from "node:fs/promises"; import { homedir } from "node:os"; import { delimiter, join, sep } from "node:path"; -import { log } from "./log.ts"; import { UPDATE_PACKAGE_NAME } from "./constants.ts"; // ── Types ──────────────────────────────────────────────────────────────────── @@ -27,146 +18,38 @@ import { UPDATE_PACKAGE_NAME } from "./constants.ts"; /** How the CLI was installed globally. */ export type Installer = "npm" | "bun" | "pnpm" | "yarn" | "homebrew"; -// ── Stage 1: npm_config_user_agent ─────────────────────────────────────────── - -export function detectFromUserAgent(): Installer | null { - const ua = process.env.npm_config_user_agent ?? ""; - if (ua.startsWith("bun/")) return "bun"; - if (ua.startsWith("pnpm/")) return "pnpm"; - if (ua.startsWith("yarn/")) return "yarn"; - if (ua.startsWith("npm/")) return "npm"; - return null; -} - -// ── Stage 2a: Homebrew ─────────────────────────────────────────────────────── +// ── Homebrew path check ────────────────────────────────────────────────────── export function isHomebrewPath(execPath: string): boolean { // Matches: - // /opt/homebrew/Cellar/clerk/... (macOS Apple Silicon) - // /usr/local/Cellar/clerk/... (macOS Intel) + // /opt/homebrew/Cellar/clerk/... (macOS Apple Silicon) + // /usr/local/Cellar/clerk/... (macOS Intel) // /home/linuxbrew/.linuxbrew/Cellar/clerk/... (Linuxbrew) return /\/Cellar\/clerk\//.test(execPath); } -// ── Stage 2b: PM prefix matching ───────────────────────────────────────────── +// ── PATH discovery ─────────────────────────────────────────────────────────── -async function safeRealpath(p: string): Promise { - try { - return await realpath(p); - } catch { - return p; - } -} - -async function queryPmPrefix(pm: "npm" | "bun" | "pnpm" | "yarn"): Promise { - try { - let result; - switch (pm) { - case "bun": - result = await Bun.$`bun pm bin -g`.quiet().nothrow(); - break; - case "pnpm": - result = await Bun.$`pnpm root -g`.quiet().nothrow(); - break; - case "yarn": - result = await Bun.$`yarn global dir`.quiet().nothrow(); - break; - default: { - result = await Bun.$`npm prefix -g`.quiet().nothrow(); - if (result.exitCode !== 0) return null; - const prefix = result.stdout.toString().trim(); - if (!prefix) return null; - return await safeRealpath(`${prefix}/lib/node_modules`); - } - } - if (result.exitCode !== 0) return null; - const dir = result.stdout.toString().trim(); - if (!dir) return null; - return await safeRealpath(dir); - } catch { - return null; - } -} - -async function matchPmFromExecPath(execPath: string): Promise { - const pms = ["bun", "pnpm", "yarn", "npm"] as const; - const results = await Promise.allSettled(pms.map((pm) => queryPmPrefix(pm))); - - for (const [i, result] of results.entries()) { - if (result.status !== "fulfilled" || !result.value) continue; - const pm = pms[i]; - if (pm && execPath.startsWith(result.value + "/")) return pm; - } - return null; -} - -// ── Public API ─────────────────────────────────────────────────────────────── +// On a machine with more than one global install (bun + asdf-npm + Homebrew +// is a common combo), runtime-based detection isn't enough: `npm install -g` +// can land in the wrong prefix while the shell still resolves `clerk` to a +// different binary. findClerkOnPath walks PATH so the caller can target the +// first-on-PATH install, i.e. what the user's shell will actually execute. /** - * Detect the installer that installed the CLI globally. + * Returns symlink-resolved absolute paths to every `clerk` binary on PATH, in + * PATH order. Duplicates (same realpath reached via two PATH entries) are + * collapsed; first occurrence wins so PATH order is preserved. * - * Uses `npm_config_user_agent` when available (highest confidence), then - * inspects `process.execPath` to identify Homebrew or match against PM - * global directories. `process.execPath` resolves through symlinks - * automatically, so a symlink at `~/.local/bin/clerk` pointing to a - * Homebrew Cellar or PM global dir is correctly attributed. - */ -export async function detectInstaller(): Promise { - // Stage 1: npm_config_user_agent - const fromUA = detectFromUserAgent(); - if (fromUA) return fromUA; - - // Stage 2: process.execPath - const execPath = process.execPath; - - // 2a: Homebrew — Cellar path is distinctive and unambiguous - if (isHomebrewPath(execPath)) return "homebrew"; - - // 2b: Match against PM global directories (parallel queries) - try { - const pm = await matchPmFromExecPath(execPath); - if (pm) return pm; - } catch (error) { - log.debug(`PM prefix detection failed: ${error}`); - } - - // Stage 3: Fallback - return "npm"; -} - -// ── Strategy B: PATH-priority-aware update detection ──────────────────────── -// -// The single-`detectInstaller()` call is not enough: on a machine with more -// than one global install (e.g. bun + asdf-managed npm), `npm install -g` can -// land in the wrong prefix while the shell still resolves `clerk` to the -// bun-installed binary. To fix this the update command needs to (1) discover -// every `clerk` on PATH in PATH order, (2) ask which installer owns the FIRST -// one, and (3) run that installer — not whatever `process.execPath` suggests. -// -// The helpers below implement steps (1) and (2a). Step (2b) — the actual -// path→installer decision — is intentionally left as a TODO for a contributor -// to fill in; see `ownerOfBinary` at the bottom. - -/** - * Walk the current process PATH and return symlink-resolved absolute paths to - * every `clerk` binary found, in PATH order. Duplicates (same realpath reached - * via two PATH entries) are collapsed; the first occurrence wins so PATH order - * is preserved. - * - * Shell-agnostic by design — reads `process.env.PATH` directly rather than - * shelling out to `which`/`where`, so it behaves identically under bash, zsh, - * fish, Nushell, PowerShell, cmd, and any other shell that exports a standard - * PATH to child processes. The sole shell-specific concern post-update is the - * command hash table (bash/zsh `hash`, zsh/tcsh `rehash`, fish auto-rehashes) - * — handled separately in the success message, not here. + * Shell-agnostic: reads `process.env.PATH` directly, no `which`/`where` + * subshell. Post-update command-hash invalidation (`hash -r`, `rehash`) is + * the caller's responsibility. * * Platform handling: - * - POSIX: iterates PATH as-is, filters to regular files with the X bit set. - * Empty PATH entries (common via `::`) are ignored rather than being - * treated as CWD — safer default, matches most modern shells. - * - Windows: iterates PATHEXT extensions in declared order (first match per - * dir wins, matching Windows resolution), accepts any regular file (the - * X bit is meaningless on NTFS; PATHEXT is the gate). + * POSIX: iterates PATH as-is; filters to regular files with the X bit set. + * Empty PATH entries (`::`) are ignored rather than treated as CWD. + * Windows: iterates PATHEXT in declared order; accepts any regular file + * since NTFS has no X bit. */ export async function findClerkOnPath(binaryName = UPDATE_PACKAGE_NAME): Promise { const dirs = (process.env.PATH ?? "").split(delimiter).filter(Boolean); @@ -197,11 +80,9 @@ export async function findClerkOnPath(binaryName = UPDATE_PACKAGE_NAME): Promise } /** - * Is this path a regular file that the current process can actually execute? - * On POSIX, "executable" means the X bit is set for the effective user. - * On Windows, NTFS has no concept of executability — extension filtering in - * findClerkOnPath() is what gates executability there, so we only confirm - * the path is a regular file. + * Regular file that the current process can execute. On POSIX, checks the X + * bit. On Windows, PATHEXT filtering at the caller is the executability gate, + * so we only verify the path is a regular file. */ async function isExecutableFile(path: string): Promise { try { @@ -215,27 +96,39 @@ async function isExecutableFile(path: string): Promise { } } +async function safeRealpath(p: string): Promise { + try { + return await realpath(p); + } catch { + return p; + } +} + +// ── Installer install dirs ────────────────────────────────────────────────── + /** - * Directories where each PM stores its globally-installed packages — i.e. the - * parent of the `@clerk/cli-/` package folder, NOT where PMs put shim - * symlinks. This is the bun-detection bug fix: `bun pm bin -g` returns the - * symlink dir (~/.bun/bin), but the compiled platform binary actually lives - * in ~/.bun/install/global/node_modules — so matching against the symlink dir - * never succeeds. All four PMs are queried in parallel; a PM that isn't on - * the system (nonzero exit, no output) is omitted from the result. + * Packages directory for each PM present on the system: the parent of the + * `@clerk/cli-/` package folder, NOT the shim or bin dir. + * + * Bun stores packages at `$BUN_INSTALL/install/global/node_modules` while + * `bun pm bin -g` returns the shim dir (`~/.bun/bin`); matching against the + * shim dir never succeeds against a resolved platform binary, which is why + * this helper returns the install dir directly. + * + * PMs not present on the system (nonzero exit, no output) are omitted. */ export async function getInstallerPackageDirs(): Promise>> { - const queries: Array<[Installer, Promise]> = [ - ["npm", queryNpmPackageDir()], - ["pnpm", queryPnpmPackageDir()], - ["yarn", queryYarnPackageDir()], - ["bun", queryBunPackageDir()], - ]; + const [npm, pnpm, yarn, bun] = await Promise.all([ + queryNpmPackageDir(), + queryPnpmPackageDir(), + queryYarnPackageDir(), + queryBunPackageDir(), + ]); const out: Partial> = {}; - for (const [pm, p] of queries) { - const dir = await p; - if (dir) out[pm] = dir; - } + if (npm) out.npm = npm; + if (pnpm) out.pnpm = pnpm; + if (yarn) out.yarn = yarn; + if (bun) out.bun = bun; return out; } @@ -261,18 +154,15 @@ async function queryYarnPackageDir(): Promise { } async function queryBunPackageDir(): Promise { - // $BUN_INSTALL defaults to ~/.bun; packages live at $BUN_INSTALL/install/global/node_modules. - // Falling back to the conventional path is safe because `clerk update` only runs from an - // installed clerk, which means Bun's layout (if used) is already in place. const root = process.env.BUN_INSTALL ?? join(homedir(), ".bun"); return await safeRealpath(join(root, "install", "global", "node_modules")); } // ── asdf shim handling ─────────────────────────────────────────────────────── -// asdf shims are bash scripts, not symlinks — `realpath` returns the shim -// itself, so ownerOfBinary can't match it against an installer's package -// dir. `asdf which` is the only way to chase the shim to the real binary. +// asdf shims are bash scripts, not symlinks. `realpath` returns the shim +// itself, so ownerOfBinary can't match it against an installer's package dir. +// `asdf which` is the only way to chase the shim to the real binary. function asdfShimsDir(): string { return join(process.env.ASDF_DATA_DIR ?? join(homedir(), ".asdf"), "shims"); @@ -313,19 +203,17 @@ export async function asdfReshim(plugin: string): Promise { } catch {} } +// ── Ownership decision ────────────────────────────────────────────────────── + /** - * Given the symlink-resolved absolute path to a clerk binary and the install - * dirs returned by `getInstallerPackageDirs()`, return which installer owns - * that specific binary — or `null` if no known installer does. - * - * Homebrew is matched first via its distinctive `/Cellar/clerk/` pattern. - * Each PM dir is matched with a trailing path separator to avoid the - * `/a/b` vs `/a/bother` false-positive. When multiple PMs match (possible - * under unusual nested-prefix configurations), the longest match wins — - * a more specific prefix is a better answer than a shorter one. + * Maps a resolved binary path to its owning installer, or `null` if none + * matches. Homebrew is checked first via its distinctive Cellar pattern. PM + * dirs are matched with a trailing separator (so `/a/b` doesn't match + * `/a/bother`); when multiple match due to nested prefixes, the longest match + * wins. * - * Returns `null` (NOT `"npm"`) when nothing matches. Callers use `null` as - * the signal to refuse-rather-than-guess. + * `null` is the refuse-rather-than-guess signal; callers must NOT default to + * "npm" on no match. */ export function ownerOfBinary( binaryPath: string, @@ -341,9 +229,9 @@ export function ownerOfBinary( return best?.installer ?? null; } -/** - * Returns a human-readable install/update command for the given installer. - */ +// ── Install command strings ───────────────────────────────────────────────── + +/** Human-readable install/update command for the given installer. */ export function globalInstallCommand(installer: Installer, packageSpec: string): string { switch (installer) { case "bun": From 968f8704b4195ee3629083d4bf14dcf375fbbff1 Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Fri, 17 Apr 2026 23:31:38 -0300 Subject: [PATCH 4/6] fix(update): address PR #179 review - Homebrew auto-upgrades on stable channel, docs said otherwise. Update README to reflect the actual behavior. - `npx clerk update` / `bunx clerk update` used to fall back to npm; after PATH-aware detection they hit the "unknown installer" refuse branch. Detect the runner via env vars and print a global-install hint instead of the install.sh message. - Agent mode was auto-confirming whenever stdout was not a TTY, which contradicted the documented `--yes`-required behavior. Refuse in agent mode without `--yes` and print the exact command to run. - Partial `--all` failures still refreshed the update cache; only write the cache when every attempted install succeeded. - Bump changeset from patch to minor (adds `--all`, changes Homebrew behavior). - Minor: `??` -> `||` for `ASDF_DATA_DIR` (empty string now treated as unset), require the bun install dir to exist before claiming ownership of paths under it, normalize case on Windows in `ownerOfBinary`, skip the POSIX `hash -r` hint on Windows, mark the primary target in the `--all` summary. --- .changeset/clerk-update-path-aware.md | 2 +- .../cli-core/src/commands/update/README.md | 4 +- .../cli-core/src/commands/update/index.ts | 67 ++++++++++++++----- packages/cli-core/src/lib/installer.ts | 29 ++++++-- 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/.changeset/clerk-update-path-aware.md b/.changeset/clerk-update-path-aware.md index af3660bc..a359095c 100644 --- a/.changeset/clerk-update-path-aware.md +++ b/.changeset/clerk-update-path-aware.md @@ -1,5 +1,5 @@ --- -"clerk": patch +"clerk": minor --- Fix `clerk update` silently writing to the wrong installer when multiple `clerk` binaries exist on PATH. The command now walks PATH to identify the binary the user's shell will actually execute, determines which installer owns that specific path (via a new `ownerOfBinary()` check), and runs the corresponding installer. Binaries installed outside any known package manager (e.g. via `install.sh`) are refused with reinstall guidance rather than silently updated via npm. Also fixes bun detection, which previously matched the shim dir (`~/.bun/bin`) instead of the install dir (`~/.bun/install/global/node_modules`) and fell through to the npm fallback. Adds a `--all` flag to update every `clerk` install on PATH in one run, skipping Homebrew on non-stable channels and unknown-owner binaries with a warning. Prints a `hash -r` / `rehash` hint based on `$SHELL` after a successful update. diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index c4981f8b..7cd3580f 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -22,7 +22,7 @@ clerk update [options] 2. Walks `PATH` to find every `clerk` binary. For asdf shims (bash scripts, not symlinks), resolves through `asdf which ` so the underlying installer is visible. Picks the first one as the **primary target** 3. Determines which installer owns the primary target via `ownerOfBinary()`: - Known installer (npm/bun/pnpm/yarn) → installs via that PM - - Homebrew → prints `brew upgrade clerk` and exits (stable channel only) + - Homebrew → runs `brew upgrade clerk` after confirmation (stable channel only; refuses on `canary` since there is no canary tap) - `null` (binary not owned by any recognized installer, e.g. `install.sh`) → refuses and lists reinstall options 4. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) 5. Runs the installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) @@ -74,4 +74,4 @@ Set `CLERK_UPDATE_CHANNEL=canary` to make canary the default for all update chec - Supports 5 installers: npm, bun, pnpm, yarn, and Homebrew. - Binaries installed via `install.sh` (direct GitHub Release download) are owned by no PM; the update command refuses and lists reinstall options instead of silently writing to a different prefix. - Permission errors (EACCES) suggest retrying with `sudo` using the detected installer's command. -- This command does not perform the update itself in agent/non-interactive mode unless `--yes` is passed. +- This command does not perform the update itself in agent/non-interactive mode unless `--yes` is passed. In agent mode without `--yes`, it prints the command the caller needs to run and exits. diff --git a/packages/cli-core/src/commands/update/index.ts b/packages/cli-core/src/commands/update/index.ts index a86de7a6..0687d744 100644 --- a/packages/cli-core/src/commands/update/index.ts +++ b/packages/cli-core/src/commands/update/index.ts @@ -1,4 +1,4 @@ -import { isHuman } from "../../mode.ts"; +import { isAgent, isHuman } from "../../mode.ts"; import { green, cyan, yellow, dim } from "../../lib/color.ts"; import { CliError } from "../../lib/errors.ts"; import { @@ -144,6 +144,9 @@ function reportOtherInstalls(others: Target[], channel: string): void { /** Hint for invalidating the current shell's command-hash cache after update. */ function hashHint(): string | null { + // Windows shells (cmd.exe, PowerShell) don't cache command paths, and `$SHELL` + // is typically unset — don't emit a POSIX hint in that case. + if (process.platform === "win32") return null; const shell = (process.env.SHELL ?? "").toLowerCase(); if (shell.endsWith("/fish") || shell.endsWith("fish.exe")) return null; // auto-rehashes if (shell.endsWith("/pwsh") || shell.endsWith("powershell.exe")) return null; // no cache @@ -154,6 +157,19 @@ function hashHint(): string | null { return "If `clerk` still points to the old binary, run `hash -r` or open a new shell."; } +/** + * Detect invocation via a package runner (npx/bunx/pnpm dlx). The binary lives + * in a runner cache (e.g. `~/.npm/_npx//...`) not on PATH, so `ownerOfBinary` + * can't identify it — but we can recognize the runner from env vars it sets. + */ +function detectPackageRunner(): "npx" | "bunx" | null { + const ua = (process.env.npm_config_user_agent ?? "").toLowerCase(); + const execPath = (process.env.npm_execpath ?? "").toLowerCase(); + if (execPath.includes("npx") || ua.startsWith("npm/")) return "npx"; + if (ua.startsWith("bun/")) return "bunx"; + return null; +} + // ── Confirmation ───────────────────────────────────────────────────────────── async function confirmUpdate(currentVersion: string, latestVersion: string): Promise { @@ -202,25 +218,40 @@ export async function update(options: UpdateOptions): Promise { // Primary target can't be updated by us: refuse (don't guess a different installer). const primarySkip = whyCantUpdate(primary, channel); if (primarySkip) { - log.warn(`Cannot auto-update: ${primarySkip}`); - if (primary.owner === "homebrew") { - log.info(` Run: ${cyan("brew upgrade clerk")}`); - } else if (primary.owner === null) { - log.info(` This binary appears to be installed outside any known package manager.`); - log.info(` Reinstall via your preferred method, e.g.:`); + const runner = detectPackageRunner(); + if (primary.owner === null && runner) { + // npx/bunx runs land in a runner cache that isn't on PATH. There's + // nothing to "update" in place — the user needs a global install. + log.warn(`Running via ${runner}; no installed clerk to update. Install globally first:`); log.info(` ${cyan(`bun add -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); log.info(` ${cyan(`npm install -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); - log.info( - ` ${cyan(`curl -fsSL https://raw.githubusercontent.com/clerk/cli/main/install.sh | bash`)}`, - ); + } else { + log.warn(`Cannot auto-update: ${primarySkip}`); + if (primary.owner === "homebrew") { + log.info(` Run: ${cyan("brew upgrade clerk")}`); + } else if (primary.owner === null) { + log.info(` This binary appears to be installed outside any known package manager.`); + log.info(` Reinstall via your preferred method, e.g.:`); + log.info(` ${cyan(`bun add -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); + log.info(` ${cyan(`npm install -g ${UPDATE_PACKAGE_NAME}@${latest}`)}`); + log.info( + ` ${cyan(`curl -fsSL https://raw.githubusercontent.com/clerk/cli/main/install.sh | bash`)}`, + ); + } } reportOtherInstalls(others, channel); if (isHuman()) outro("Update required manual action"); return; } - const autoConfirm = options.yes || !isHuman(); - const shouldInstall = autoConfirm || (await confirmUpdate(currentVersion, latest)); + // In agent/non-interactive mode, require explicit `--yes` rather than silently + // running a global install the caller didn't confirm. + if (isAgent() && !options.yes) { + log.info(`Run \`clerk update --yes\` to proceed.`); + return; + } + + const shouldInstall = options.yes || (await confirmUpdate(currentVersion, latest)); if (!shouldInstall) { if (isHuman()) outro("Update cancelled"); @@ -267,7 +298,13 @@ export async function update(options: UpdateOptions): Promise { await asdfReshim(plugin); } - await writeUpdateCache({ checkedAt: Date.now(), latest, distTag: channel }); + // Only refresh the update-notification cache when every attempted install + // succeeded — otherwise a partial --all failure would silently mark the + // latest version as cached. + const anyFailed = results.some((r) => !r.ok); + if (!anyFailed) { + await writeUpdateCache({ checkedAt: Date.now(), latest, distTag: channel }); + } // Summary + skipped installs when --all. if (options.all) { @@ -275,8 +312,9 @@ export async function update(options: UpdateOptions): Promise { log.info("Summary:"); for (const r of results) { const icon = r.ok ? green("✓") : yellow("✗"); + const primaryTag = r.target === primary ? dim(" [primary]") : ""; const suffix = r.ok ? "" : ` ${yellow(`- ${r.error}`)}`; - log.info(` ${icon} ${formatTarget(r.target)}${suffix}`); + log.info(` ${icon} ${formatTarget(r.target)}${primaryTag}${suffix}`); } for (const t of others) { const skip = whyCantUpdate(t, channel); @@ -294,7 +332,6 @@ export async function update(options: UpdateOptions): Promise { } if (isHuman()) { - const anyFailed = results.some((r) => !r.ok); outro(anyFailed ? "Update completed with errors" : `Successfully updated to ${latest}`); } } diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index 02117f6b..56dd3380 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -154,8 +154,15 @@ async function queryYarnPackageDir(): Promise { } async function queryBunPackageDir(): Promise { - const root = process.env.BUN_INSTALL ?? join(homedir(), ".bun"); - return await safeRealpath(join(root, "install", "global", "node_modules")); + const root = process.env.BUN_INSTALL || join(homedir(), ".bun"); + const dir = join(root, "install", "global", "node_modules"); + try { + const s = await stat(dir); + if (!s.isDirectory()) return null; + } catch { + return null; + } + return await safeRealpath(dir); } // ── asdf shim handling ─────────────────────────────────────────────────────── @@ -164,8 +171,12 @@ async function queryBunPackageDir(): Promise { // itself, so ownerOfBinary can't match it against an installer's package dir. // `asdf which` is the only way to chase the shim to the real binary. +function asdfDataDir(): string { + return process.env.ASDF_DATA_DIR || join(homedir(), ".asdf"); +} + function asdfShimsDir(): string { - return join(process.env.ASDF_DATA_DIR ?? join(homedir(), ".asdf"), "shims"); + return join(asdfDataDir(), "shims"); } export function isAsdfShimPath(path: string): boolean { @@ -189,7 +200,7 @@ export async function resolveAsdfShim(path: string): Promise { } export function asdfPluginFromPath(path: string): string | null { - const installsDir = join(process.env.ASDF_DATA_DIR ?? join(homedir(), ".asdf"), "installs"); + const installsDir = join(asdfDataDir(), "installs"); const prefix = installsDir + sep; if (!path.startsWith(prefix)) return null; const plugin = path.slice(prefix.length).split(sep)[0]; @@ -221,9 +232,17 @@ export function ownerOfBinary( ): Installer | null { if (isHomebrewPath(binaryPath)) return "homebrew"; + // Windows paths are case-insensitive and can mix forward/back slashes after + // realpath (e.g. `C:\…` vs `c:\…`). Normalize both sides before comparison. + const normalize = (p: string): string => + process.platform === "win32" ? p.toLowerCase().replace(/\//g, sep) : p; + const target = normalize(binaryPath); + let best: { installer: Installer; len: number } | null = null; for (const [pm, dir] of Object.entries(installDirs) as Array<[Installer, string]>) { - if (!dir || !binaryPath.startsWith(dir + sep)) continue; + if (!dir) continue; + const prefix = normalize(dir) + sep; + if (!target.startsWith(prefix)) continue; if (!best || dir.length > best.len) best = { installer: pm, len: dir.length }; } return best?.installer ?? null; From 8cea3dfe224848f3e6a1e5fc03f8d88e32b46b8a Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Mon, 20 Apr 2026 11:07:26 -0300 Subject: [PATCH 5/6] fix(update): use `npm root -g` for Windows compat `npm prefix -g` + hardcoded `lib/node_modules` gives the wrong path on Windows, where npm stores global packages directly under the prefix (`%AppData%\npm\node_modules`, no `lib` segment). `npm root -g` reports the correct directory on both POSIX and Windows. --- .../cli-core/src/commands/update/README.md | 18 +++++++++--------- packages/cli-core/src/lib/installer.ts | 9 ++++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index 7cd3580f..00ebbccc 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -36,21 +36,21 @@ A machine can host multiple `clerk` installs (bun + asdf-npm + Homebrew is commo ## Version managers (asdf, nvm) -- **nvm**: fully supported without special handling. nvm uses real symlinks, so `realpath` chases them into `/lib/node_modules/clerk/bin/clerk`, which matches the active `npm prefix -g`; `ownerOfBinary` returns `"npm"`. +- **nvm**: fully supported without special handling. nvm uses real symlinks, so `realpath` chases them into `/lib/node_modules/clerk/bin/clerk`, which matches the active `npm root -g`; `ownerOfBinary` returns `"npm"`. - **asdf**: handled via `resolveAsdfShim()`. asdf shims (`~/.asdf/shims/`) are bash scripts, not symlinks, so `realpath` returns the shim itself. The update command calls `asdf which ` to find the underlying binary (e.g. `~/.asdf/installs/nodejs/22.16.0/bin/clerk`), realpaths it into the asdf-managed node's `lib/node_modules`, and treats it as `"npm"` with a post-install `asdf reshim `. Honors `$ASDF_DATA_DIR` when set. If `asdf` isn't on PATH the shim path is returned unchanged and ownership falls back to `null`. ## Installer detection Detection uses path-based ownership (see `lib/installer.ts`). For a given binary path: -| Check | Result | -| ----------------------------------------------------- | ------------------------- | -| Contains `/Cellar/clerk/` | `homebrew` | -| Under `/lib/node_modules` | `npm` | -| Under `/install/global/node_modules` | `bun` | -| Under `pnpm root -g` | `pnpm` | -| Under `/node_modules` | `yarn` | -| Nothing matches | `null` (refuse to update) | +| Check | Result | +| ---------------------------------------------------------------------------------------------- | ------------------------- | +| Contains `/Cellar/clerk/` | `homebrew` | +| Under `npm root -g` (`/lib/node_modules` on POSIX, `\node_modules` on Windows) | `npm` | +| Under `/install/global/node_modules` | `bun` | +| Under `pnpm root -g` | `pnpm` | +| Under `/node_modules` | `yarn` | +| Nothing matches | `null` (refuse to update) | When multiple PMs' dirs nest, the longest prefix wins. `null` is the signal to refuse rather than silently install via the wrong installer. diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index 56dd3380..2eef6beb 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -133,10 +133,13 @@ export async function getInstallerPackageDirs(): Promise { - const result = await Bun.$`npm prefix -g`.quiet().nothrow(); + // `npm root -g` reports the actual global node_modules dir on both platforms + // (POSIX: `/lib/node_modules`; Windows: `\node_modules`, no + // `lib` segment). Constructing the path manually breaks on Windows. + const result = await Bun.$`npm root -g`.quiet().nothrow(); if (result.exitCode !== 0) return null; - const prefix = result.stdout.toString().trim(); - return prefix ? await safeRealpath(join(prefix, "lib", "node_modules")) : null; + const dir = result.stdout.toString().trim(); + return dir ? await safeRealpath(dir) : null; } async function queryPnpmPackageDir(): Promise { From 78171e356da12f7398fbec2576d05af5a39aa241 Mon Sep 17 00:00:00 2001 From: Rafael Thayto Date: Mon, 20 Apr 2026 15:29:54 -0300 Subject: [PATCH 6/6] fix(update): address wyattjoh PR review - brew upgrade: verify installed version post-install and fail when the tap lags the target; `brew upgrade` ignores the packageSpec pin and exits 0 even when it's a no-op, so the caller was silently left on the old version. - --all: when the primary target is blocked (e.g. Homebrew on canary, unknown owner), promote updatable others to effective primaries instead of refusing the whole run. Only refuse when every target is blocked. Surface the skipped primary in the summary so users can see why `clerk -v` didn't change. - detectPackageRunner: drop the `npm_config_user_agent` UA disjunct and key off npm_execpath only. The UA is set for every npm/bun invocation (e.g. `npm run build` that shells out to clerk), so matching `npm/` misfired on ordinary script use. - ownerOfBinary: strip Win32 extended-length (`\\?\`) and UNC (`\\?\UNC\`) prefixes before comparison so realpath-returned extended paths match plain install-dir prefixes. - hashHint: collapse the dead `powershell.exe` branch that was shadowed by the `process.platform === "win32"` early-return, keeping `$SHELL=/usr/bin/pwsh` (PowerShell on Linux) correctly opted out. --- .../cli-core/src/commands/update/README.md | 4 +- .../cli-core/src/commands/update/index.ts | 130 +++++++++++++----- packages/cli-core/src/lib/installer.test.ts | 27 ++++ packages/cli-core/src/lib/installer.ts | 28 +++- 4 files changed, 149 insertions(+), 40 deletions(-) diff --git a/packages/cli-core/src/commands/update/README.md b/packages/cli-core/src/commands/update/README.md index 00ebbccc..dee869e3 100644 --- a/packages/cli-core/src/commands/update/README.md +++ b/packages/cli-core/src/commands/update/README.md @@ -22,12 +22,12 @@ clerk update [options] 2. Walks `PATH` to find every `clerk` binary. For asdf shims (bash scripts, not symlinks), resolves through `asdf which ` so the underlying installer is visible. Picks the first one as the **primary target** 3. Determines which installer owns the primary target via `ownerOfBinary()`: - Known installer (npm/bun/pnpm/yarn) → installs via that PM - - Homebrew → runs `brew upgrade clerk` after confirmation (stable channel only; refuses on `canary` since there is no canary tap) + - Homebrew → runs `brew upgrade clerk` after confirmation (stable channel only; refuses on `canary` since there is no canary tap). After the brew command succeeds, verifies the installed version matches the npm registry's `latest`; fails loudly if the tap is stale. - `null` (binary not owned by any recognized installer, e.g. `install.sh`) → refuses and lists reinstall options 4. Prompts for confirmation (skipped with `--yes` or in non-interactive mode) 5. Runs the installer's global install command (e.g. `npm install -g clerk@`, `bun add -g clerk@`) 6. If any updated target was inside an asdf-managed tool, runs `asdf reshim ` so the shim picks up the new binary (safety net; modern asdf-nodejs auto-reshims) -7. With `--all`, repeats for every other `clerk` install on PATH, skipping Homebrew on non-stable channels and `null`-owned binaries +7. With `--all`, updates every on-PATH `clerk` install whose owner the CLI can drive. If the primary itself is blocked (e.g. Homebrew on `canary`, or `null` owner), the primary is skipped with a warning and the remaining installs still run; the run only refuses when every target is blocked. Failures on individual installs are recorded in the summary but don't short-circuit the rest. 8. After a successful install, prints a shell-specific `hash -r` / `rehash` hint when applicable ## Why PATH-walking matters diff --git a/packages/cli-core/src/commands/update/index.ts b/packages/cli-core/src/commands/update/index.ts index 0687d744..bd4980b1 100644 --- a/packages/cli-core/src/commands/update/index.ts +++ b/packages/cli-core/src/commands/update/index.ts @@ -79,7 +79,11 @@ async function resolveTargets( // ── Install execution ──────────────────────────────────────────────────────── -async function runGlobalInstall(installer: Installer, packageSpec: string): Promise { +async function runGlobalInstall( + installer: Installer, + packageSpec: string, + targetVersion: string, +): Promise { let result; switch (installer) { case "bun": @@ -98,17 +102,43 @@ async function runGlobalInstall(installer: Installer, packageSpec: string): Prom result = await Bun.$`npm install -g ${packageSpec}`.quiet().nothrow(); break; } - if (result.exitCode === 0) return; - - const stderr = result.stderr.toString(); - const hint = globalInstallCommand(installer, packageSpec); - if (stderr.includes("EACCES") || stderr.includes("permission denied")) { - throw new CliError(`Permission denied. Try: sudo ${hint}`); + if (result.exitCode !== 0) { + const stderr = result.stderr.toString(); + const hint = globalInstallCommand(installer, packageSpec); + if (stderr.includes("EACCES") || stderr.includes("permission denied")) { + throw new CliError(`Permission denied. Try: sudo ${hint}`); + } + if (result.exitCode === 127 || stderr.includes("not found")) { + throw new CliError(`${installer} not found on PATH.`); + } + throw new CliError(`Update failed: ${stderr.trim() || "unknown error"}`); } - if (result.exitCode === 127 || stderr.includes("not found")) { - throw new CliError(`${installer} not found on PATH.`); + + // Homebrew installs whatever version its tap currently publishes, ignoring + // the packageSpec pin. When the tap lags the npm release, `brew upgrade` + // exits 0 but leaves the old version in place. Verify post-install and + // surface the mismatch so the user isn't left with a stale binary believing + // the update succeeded. + if (installer === "homebrew") { + const installed = await installedBrewVersion(); + if (installed && installed !== targetVersion) { + throw new CliError( + `Homebrew tap is stale: installed ${installed}, expected ${targetVersion}. ` + + `Update via another installer (e.g. \`npm install -g ${packageSpec}\`) ` + + `or wait for the tap to catch up.`, + ); + } } - throw new CliError(`Update failed: ${stderr.trim() || "unknown error"}`); +} + +/** Returns the currently installed Homebrew clerk version, or null on failure. */ +async function installedBrewVersion(): Promise { + const result = await Bun.$`brew list --versions ${UPDATE_PACKAGE_NAME}`.quiet().nothrow(); + if (result.exitCode !== 0) return null; + // Output shape: "clerk 0.8.4 0.8.5" (package name + one or more versions). + // The last token is the most recently installed version that brew will use. + const tokens = result.stdout.toString().trim().split(/\s+/); + return tokens.length >= 2 ? (tokens.at(-1) ?? null) : null; } // ── Skip predicates ────────────────────────────────────────────────────────── @@ -144,12 +174,14 @@ function reportOtherInstalls(others: Target[], channel: string): void { /** Hint for invalidating the current shell's command-hash cache after update. */ function hashHint(): string | null { - // Windows shells (cmd.exe, PowerShell) don't cache command paths, and `$SHELL` - // is typically unset — don't emit a POSIX hint in that case. + // Windows native shells (cmd.exe, PowerShell) don't cache command paths. + // On Windows, $SHELL is typically unset, so nothing below would match anyway — + // but return early to be explicit. if (process.platform === "win32") return null; const shell = (process.env.SHELL ?? "").toLowerCase(); - if (shell.endsWith("/fish") || shell.endsWith("fish.exe")) return null; // auto-rehashes - if (shell.endsWith("/pwsh") || shell.endsWith("powershell.exe")) return null; // no cache + if (shell.endsWith("/fish")) return null; // auto-rehashes + // pwsh can run on Linux/macOS via $SHELL=/usr/bin/pwsh; no command-hash cache. + if (shell.endsWith("/pwsh")) return null; if (shell.endsWith("/tcsh") || shell.endsWith("/csh")) { return "If `clerk` still points to the old binary, run `rehash` or open a new shell."; } @@ -158,15 +190,24 @@ function hashHint(): string | null { } /** - * Detect invocation via a package runner (npx/bunx/pnpm dlx). The binary lives - * in a runner cache (e.g. `~/.npm/_npx//...`) not on PATH, so `ownerOfBinary` - * can't identify it — but we can recognize the runner from env vars it sets. + * Detect invocation via a package runner (npx/bunx). The binary lives in a + * runner cache (e.g. `~/.npm/_npx//...`) not on PATH, so `ownerOfBinary` + * can't identify it. + * + * Detection is execpath-only: `npm_config_user_agent` alone is unreliable + * because it's set for *every* npm/bun invocation (e.g. `npm run build` that + * internally calls `clerk update` would look like npx). The execpath — the + * actual binary that launched the process — is the only signal that + * distinguishes a runner from a regular script invocation. */ function detectPackageRunner(): "npx" | "bunx" | null { - const ua = (process.env.npm_config_user_agent ?? "").toLowerCase(); - const execPath = (process.env.npm_execpath ?? "").toLowerCase(); - if (execPath.includes("npx") || ua.startsWith("npm/")) return "npx"; - if (ua.startsWith("bun/")) return "bunx"; + const execPath = (process.env.npm_execpath ?? process.argv0 ?? "").toLowerCase(); + // Match the runner basename, not any substring (so `/home/npxyz/node` doesn't + // misfire; `_npx//…/npx-cli.js` and `…/npx` both end with `npx` after + // stripping the optional `.js`). + const stripped = execPath.replace(/\.(js|cjs|mjs|exe|cmd)$/, ""); + if (/(^|[\\/])npx$/.test(stripped) || execPath.includes("/_npx/")) return "npx"; + if (/(^|[\\/])bunx$/.test(stripped)) return "bunx"; return null; } @@ -215,9 +256,22 @@ export async function update(options: UpdateOptions): Promise { log.info(` Target: ${formatTarget(primary)}`); log.blank(); - // Primary target can't be updated by us: refuse (don't guess a different installer). + // Build the ordered updatable list. Without --all, only the primary counts; + // with --all, include every on-PATH install whose owner we can update. const primarySkip = whyCantUpdate(primary, channel); - if (primarySkip) { + const toUpdate: Target[] = []; + if (!primarySkip) toUpdate.push(primary); + if (options.all) { + for (const t of others) { + if (whyCantUpdate(t, channel) === null) toUpdate.push(t); + } + } + + // Nothing we can update: emit the refuse-path guidance keyed off the primary + // (still the most useful target to talk about), then exit. The --all branch + // shares this exit only when *every* install is blocked, which is the + // intended refuse behavior. + if (toUpdate.length === 0) { const runner = detectPackageRunner(); if (primary.owner === null && runner) { // npx/bunx runs land in a runner cache that isn't on PATH. There's @@ -244,6 +298,19 @@ export async function update(options: UpdateOptions): Promise { return; } + // --all with a blocked primary: tell the user before we proceed that we're + // skipping their shell's effective `clerk` and updating other installs + // instead. Without this, a user who runs `--all --channel canary` on a + // Homebrew-primary machine would see no reason their `clerk -v` stays the + // same until they read the summary. + if (primarySkip && options.all) { + log.warn(`Skipping primary (${formatTarget(primary)}): ${primarySkip}`); + log.info( + `Proceeding with ${toUpdate.length} other install${toUpdate.length === 1 ? "" : "s"}.`, + ); + log.blank(); + } + // In agent/non-interactive mode, require explicit `--yes` rather than silently // running a global install the caller didn't confirm. if (isAgent() && !options.yes) { @@ -260,14 +327,6 @@ export async function update(options: UpdateOptions): Promise { const packageSpec = `${UPDATE_PACKAGE_NAME}@${latest}`; - // Build the target list: always primary first, optionally every other updatable install. - const toUpdate: Target[] = [primary]; - if (options.all) { - for (const t of others) { - if (whyCantUpdate(t, channel) === null) toUpdate.push(t); - } - } - const results: Array<{ target: Target; ok: boolean; error?: string }> = []; for (const t of toUpdate) { // `owner` is non-null here because whyCantUpdate returned null for it. @@ -275,7 +334,7 @@ export async function update(options: UpdateOptions): Promise { try { await withSpinner( `Installing ${packageSpec} via ${owner} (${t.displayPath})...`, - () => runGlobalInstall(owner, packageSpec), + () => runGlobalInstall(owner, packageSpec, latest), `Updated ${owner}: ${t.displayPath}`, ); results.push({ target: t, ok: true }); @@ -316,6 +375,13 @@ export async function update(options: UpdateOptions): Promise { const suffix = r.ok ? "" : ` ${yellow(`- ${r.error}`)}`; log.info(` ${icon} ${formatTarget(r.target)}${primaryTag}${suffix}`); } + // Skipped installs. Include the primary when it was blocked (so users + // aren't left wondering why `clerk -v` didn't change after --all). + if (primarySkip) { + log.info( + ` ${yellow("⚠")} ${formatTarget(primary)}${dim(" [primary]")} ${yellow(`- skipped: ${primarySkip}`)}`, + ); + } for (const t of others) { const skip = whyCantUpdate(t, channel); if (!skip) continue; diff --git a/packages/cli-core/src/lib/installer.test.ts b/packages/cli-core/src/lib/installer.test.ts index b5af66a2..dc5ea1ee 100644 --- a/packages/cli-core/src/lib/installer.test.ts +++ b/packages/cli-core/src/lib/installer.test.ts @@ -153,6 +153,33 @@ describe("ownerOfBinary", () => { ), ).toBe("bun"); }); + + // Windows path normalization is only exercised when process.platform is + // "win32" — the helper no-ops on POSIX. The CI matrix runs Windows + // separately, so these cases guard against regressions there. + test("matches win32 extended-length realpath against plain install dir (windows-only)", () => { + if (process.platform !== "win32") return; + const win = { npm: "C:\\Users\\x\\AppData\\Roaming\\npm\\node_modules" } as const; + // Bun's realpath can prepend \\?\ for deep trees; install dir may not have it. + expect( + ownerOfBinary( + "\\\\?\\C:\\Users\\x\\AppData\\Roaming\\npm\\node_modules\\@clerk\\cli-win32-x64\\bin\\clerk.exe", + win, + ), + ).toBe("npm"); + }); + + test("matches win32 UNC realpath against install dir (windows-only)", () => { + if (process.platform !== "win32") return; + const win = { npm: "\\\\server\\share\\npm\\node_modules" } as const; + // \\?\UNC\server\share\... is the extended-length form of \\server\share\... + expect( + ownerOfBinary( + "\\\\?\\UNC\\server\\share\\npm\\node_modules\\@clerk\\cli-win32-x64\\bin\\clerk.exe", + win, + ), + ).toBe("npm"); + }); }); // ── findClerkOnPath ────────────────────────────────────────────────────────── diff --git a/packages/cli-core/src/lib/installer.ts b/packages/cli-core/src/lib/installer.ts index 2eef6beb..56e9cde9 100644 --- a/packages/cli-core/src/lib/installer.ts +++ b/packages/cli-core/src/lib/installer.ts @@ -236,21 +236,37 @@ export function ownerOfBinary( if (isHomebrewPath(binaryPath)) return "homebrew"; // Windows paths are case-insensitive and can mix forward/back slashes after - // realpath (e.g. `C:\…` vs `c:\…`). Normalize both sides before comparison. - const normalize = (p: string): string => - process.platform === "win32" ? p.toLowerCase().replace(/\//g, sep) : p; - const target = normalize(binaryPath); + // realpath (e.g. `C:\…` vs `c:\…`). They can also come back with the Win32 + // extended-length prefix `\\?\` (realpath returns this for deep trees), or + // the UNC variant `\\?\UNC\server\share\…` which is the same as + // `\\server\share\…`. Normalize all of these before comparison. + const target = normalizeWindowsPath(binaryPath); let best: { installer: Installer; len: number } | null = null; for (const [pm, dir] of Object.entries(installDirs) as Array<[Installer, string]>) { if (!dir) continue; - const prefix = normalize(dir) + sep; + const normalizedDir = normalizeWindowsPath(dir); + const prefix = normalizedDir + sep; if (!target.startsWith(prefix)) continue; - if (!best || dir.length > best.len) best = { installer: pm, len: dir.length }; + if (!best || normalizedDir.length > best.len) { + best = { installer: pm, len: normalizedDir.length }; + } } return best?.installer ?? null; } +/** + * Strips Win32 namespace prefixes (`\\?\` and `\\?\UNC\`), unifies slashes to + * the platform separator, and lowercases on Windows. No-op on POSIX. + */ +function normalizeWindowsPath(p: string): string { + if (process.platform !== "win32") return p; + let out = p; + if (out.startsWith("\\\\?\\UNC\\")) out = "\\\\" + out.slice(8); + else if (out.startsWith("\\\\?\\")) out = out.slice(4); + return out.toLowerCase().replace(/\//g, sep); +} + // ── Install command strings ───────────────────────────────────────────────── /** Human-readable install/update command for the given installer. */