fix(cli): upgrade + update notice use the detected install method

packages/cli/src/commands/upgrade.ts

@@ -10,7 +10,13 @@ export const examples: Example[] = [
   ["Upgrade non-interactively", "hyperframes upgrade --yes"],
 ];
 import { VERSION } from "../version.js";
-import { checkForUpdate, withMeta } from "../utils/updateCheck.js";
+import {
+  checkForUpdate,
+  withMeta,
+  isSafeVersion,
+  type UpdateCheckResult,
+} from "../utils/updateCheck.js";
+import { detectInstaller, installInvocation } from "../utils/installerDetection.js";
 
 export default defineCommand({
   meta: { name: "upgrade", description: "Check for updates and show upgrade instructions" },
@@ -19,6 +25,7 @@ export default defineCommand({
     check: { type: "boolean", description: "Check for updates and exit (no prompt)" },
     json: { type: "boolean", description: "Output as JSON", default: false },
   },
+  // fallow-ignore-next-line complexity
   async run({ args }) {
     const useJson = args.json === true;
     const checkOnly = args.check === true;
@@ -56,53 +63,96 @@ export default defineCommand({
       return;
     }
 
-    if (!autoYes) {
-      const shouldUpgrade = await clack.confirm({
-        message: "Upgrade now?",
-      });
-
-      if (clack.isCancel(shouldUpgrade) || !shouldUpgrade) {
-        clack.outro(c.dim("Skipped."));
-        return;
-      }
-    }
-
-    // Reject anything that isn't a strict semver-shaped string before it reaches
-    // the install command. A poisoned npm registry response could otherwise put
-    // shell metacharacters into `result.latest`; rejecting up front means the
-    // version flows through execFile (and the displayed command) as an opaque
-    // token, not something the shell might re-parse.
-    const SAFE_VERSION = /^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/;
-    if (!SAFE_VERSION.test(result.latest)) {
-      clack.outro(c.dim("Refusing to install: unexpected version string from npm registry."));
-      process.exitCode = 1;
+    if (!autoYes && !(await confirmUpgrade())) {
+      clack.outro(c.dim("Skipped."));
       return;
     }
 
-    const installArgs = ["install", "-g", `hyperframes@${result.latest}`];
-    const installCmd = `npm ${installArgs.join(" ")}`;
-    if (autoYes) {
-      console.log();
-      console.log(`   ${c.dim("Running:")} ${c.accent(installCmd)}`);
-      console.log();
-      try {
-        // execFileSync with shell:false — the version is now provably safe per
-        // SAFE_VERSION above, but keep the no-shell call so future edits can't
-        // regress the shell-injection surface area.
-        execFileSync("npm", installArgs, { stdio: "inherit", shell: false });
-        clack.outro(c.success(`Upgraded to v${result.latest}`));
-      } catch {
-        clack.outro(c.dim("Install failed. Try running manually:"));
-        console.log(`   ${c.accent(installCmd)}`);
-        process.exitCode = 1;
-      }
-    } else {
-      console.log();
-      console.log(`   ${c.accent(installCmd)}`);
-      console.log(`   ${c.dim("or")}`);
-      console.log(`   ${c.accent("npx hyperframes@" + result.latest + " --version")}`);
-      console.log();
-      clack.outro(c.success("Run one of the commands above to upgrade."));
-    }
+    applyUpgrade(result, autoYes);
   },
 });
+
+/** Interactive "Upgrade now?" prompt; false on decline or cancel. */
+async function confirmUpgrade(): Promise<boolean> {
+  const shouldUpgrade = await clack.confirm({ message: "Upgrade now?" });
+  return !clack.isCancel(shouldUpgrade) && shouldUpgrade === true;
+}
+
+/**
+ * Show (or, with `autoYes`, run) the upgrade for the user's ACTUAL install
+ * method — not a hardcoded `npm install -g`, which fails or silently shadows a
+ * bun/pnpm/brew install. Extracted from `run` to keep that handler simple.
+ */
+// fallow-ignore-next-line complexity
+function applyUpgrade(result: UpdateCheckResult, autoYes: boolean): void {
+  // Reject anything that isn't a strict semver before it reaches a command. A
+  // poisoned npm registry response could otherwise put shell metacharacters
+  // into `result.latest`; the guard means the version flows through execFile
+  // (and the displayed command) as an opaque token. Shared with the update
+  // notice via isSafeVersion.
+  if (!isSafeVersion(result.latest)) {
+    clack.outro(c.dim("Refusing to install: unexpected version string from npm registry."));
+    process.exitCode = 1;
+    return;
+  }
+
+  const installer = detectInstaller();
+  const invocation = installInvocation(installer.kind, result.latest);
+  const displayCmd = installer.installCommand(result.latest);
+  const npxFallback = `npx hyperframes@${result.latest}`;
+
+  // Undetectable / ephemeral (npx, bunx) / project-local / workspace: don't
+  // guess a manager command; point at the universal npx fallback instead.
+  if (!invocation || !displayCmd) {
+    printNpxFallback(installer.reason, npxFallback, autoYes);
+    return;
+  }
+
+  if (!autoYes) {
+    printManualCommands(displayCmd, npxFallback);
+    return;
+  }
+
+  runDetectedInstall(invocation, displayCmd, result.latest);
+}
+
+function printNpxFallback(reason: string, npxFallback: string, autoYes: boolean): void {
+  console.log();
+  if (autoYes) {
+    console.log(
+      `   ${c.dim("Couldn't detect a global install to upgrade")} ${c.dim("(" + reason + ")")}`,
+    );
+  }
+  console.log(`   ${c.accent(npxFallback)}`);
+  console.log();
+  clack.outro(c.success("Run the command above to use the latest version."));
+}
+
+function printManualCommands(displayCmd: string, npxFallback: string): void {
+  console.log();
+  console.log(`   ${c.accent(displayCmd)}`);
+  console.log(`   ${c.dim("or")}`);
+  console.log(`   ${c.accent(npxFallback)}`);
+  console.log();
+  clack.outro(c.success("Run one of the commands above to upgrade."));
+}
+
+function runDetectedInstall(
+  invocation: { bin: string; args: string[] },
+  displayCmd: string,
+  version: string,
+): void {
+  console.log();
+  console.log(`   ${c.dim("Running:")} ${c.accent(displayCmd)}`);
+  console.log();
+  try {
+    // shell:false — version is provably safe per isSafeVersion above; keep the
+    // no-shell call so future edits can't regress the injection surface.
+    execFileSync(invocation.bin, invocation.args, { stdio: "inherit", shell: false });
+    clack.outro(c.success(`Upgraded to v${version}`));
+  } catch {
+    clack.outro(c.dim("Install failed. Try running manually:"));
+    console.log(`   ${c.accent(displayCmd)}`);
+    process.exitCode = 1;
+  }
+}

packages/cli/src/utils/installerDetection.test.ts

@@ -123,3 +123,36 @@ describe("detectInstaller", () => {
     expect(info.reason).toMatch(/Unknown install layout/);
   });
 });
+
+import { installInvocation } from "./installerDetection.js";
+
+describe("installInvocation", () => {
+  it("returns the npm global argv for kind npm", () => {
+    expect(installInvocation("npm", "1.2.3")).toEqual({
+      bin: "npm",
+      args: ["install", "-g", "hyperframes@1.2.3"],
+    });
+  });
+
+  it("returns bun/pnpm add -g argv for those managers", () => {
+    expect(installInvocation("bun", "1.2.3")).toEqual({
+      bin: "bun",
+      args: ["add", "-g", "hyperframes@1.2.3"],
+    });
+    expect(installInvocation("pnpm", "1.2.3")).toEqual({
+      bin: "pnpm",
+      args: ["add", "-g", "hyperframes@1.2.3"],
+    });
+  });
+
+  it("returns a version-less brew upgrade for kind brew", () => {
+    expect(installInvocation("brew", "1.2.3")).toEqual({
+      bin: "brew",
+      args: ["upgrade", "hyperframes"],
+    });
+  });
+
+  it("returns null for kind skip (ephemeral / project-local / unknown)", () => {
+    expect(installInvocation("skip", "1.2.3")).toBeNull();
+  });
+});

packages/cli/src/utils/installerDetection.ts

@@ -156,3 +156,34 @@ export function detectInstaller(): InstallerInfo {
     reason: `Unknown install layout at ${realEntry}`,
   };
 }
+
+/** Argv-shaped install command for a no-shell `execFile`. */
+export interface InstallInvocation {
+  bin: string;
+  args: string[];
+}
+
+/**
+ * The argv form of {@link InstallerInfo.installCommand}, kept next to the
+ * detector so the command we *run* (execFile, no shell) and the command we
+ * *display* (installCommand string) can never drift. Returns `null` for `skip`
+ * kinds (ephemeral npx/bunx, workspace links, project-local, unknown layouts):
+ * the caller must print a manual instruction rather than run a guessed command
+ * (running the wrong manager is worse than running nothing).
+ */
+export function installInvocation(kind: InstallerKind, version: string): InstallInvocation | null {
+  switch (kind) {
+    case "npm":
+      return { bin: "npm", args: ["install", "-g", `hyperframes@${version}`] };
+    case "bun":
+      return { bin: "bun", args: ["add", "-g", `hyperframes@${version}`] };
+    case "pnpm":
+      return { bin: "pnpm", args: ["add", "-g", `hyperframes@${version}`] };
+    case "brew":
+      // brew has no per-version install; `brew upgrade` moves to the tap's
+      // current formula (a no-op if the tap hasn't caught up).
+      return { bin: "brew", args: ["upgrade", "hyperframes"] };
+    case "skip":
+      return null;
+  }
+}

packages/cli/src/utils/updateCheck.test.ts

@@ -0,0 +1,118 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { isSafeVersion } from "./updateCheck.js";
+
+describe("isSafeVersion", () => {
+  it("accepts strict semver, incl. prerelease/build metadata", () => {
+    expect(isSafeVersion("1.2.3")).toBe(true);
+    expect(isSafeVersion("0.7.28")).toBe(true);
+    expect(isSafeVersion("1.2.3-beta.1")).toBe(true);
+    expect(isSafeVersion("1.2.3+build.5")).toBe(true);
+  });
+
+  it("rejects anything that could carry shell metacharacters or isn't semver", () => {
+    expect(isSafeVersion("")).toBe(false);
+    expect(isSafeVersion("latest")).toBe(false);
+    expect(isSafeVersion("1.2")).toBe(false);
+    expect(isSafeVersion("1.2.3; rm -rf /")).toBe(false);
+    expect(isSafeVersion("1.2.3 && curl evil")).toBe(false);
+    expect(isSafeVersion("$(whoami)")).toBe(false);
+  });
+});
+
+/**
+ * Drive printUpdateNotice under controlled mocks. isDevMode() is true under
+ * vitest (the module path ends in .ts), which would suppress the notice, so we
+ * mock ./env.js. detectInstaller and readConfig are mocked to pick the branch.
+ */
+async function noticeWith(opts: {
+  installerCommand: string | null;
+  latestVersion?: string;
+  isTTY?: boolean;
+  env?: Record<string, string | undefined>;
+}): Promise<string> {
+  vi.resetModules();
+  vi.doMock("./env.js", () => ({ isDevMode: () => false }));
+  vi.doMock("./installerDetection.js", () => ({
+    detectInstaller: () => ({
+      kind: opts.installerCommand ? "npm" : "skip",
+      installCommand: () => opts.installerCommand,
+      reason: "test",
+    }),
+  }));
+  vi.doMock("../telemetry/config.js", () => ({
+    readConfig: () => ({ latestVersion: opts.latestVersion ?? "9.9.9" }),
+    writeConfig: () => {},
+  }));
+
+  const origEnv = { ...process.env };
+  for (const [k, v] of Object.entries(opts.env ?? {})) {
+    if (v === undefined) delete process.env[k];
+    else process.env[k] = v;
+  }
+  // Default to a non-CI interactive terminal unless the test overrides env.
+  if (!("CI" in (opts.env ?? {}))) delete process.env["CI"];
+
+  const origTTY = process.stderr.isTTY;
+  Object.defineProperty(process.stderr, "isTTY", {
+    value: opts.isTTY ?? true,
+    configurable: true,
+  });
+  const writes: string[] = [];
+  const origWrite = process.stderr.write.bind(process.stderr);
+  process.stderr.write = ((chunk: unknown) => {
+    writes.push(String(chunk));
+    return true;
+  }) as typeof process.stderr.write;
+
+  try {
+    const mod = await import("./updateCheck.js");
+    mod.printUpdateNotice();
+  } finally {
+    process.stderr.write = origWrite;
+    Object.defineProperty(process.stderr, "isTTY", { value: origTTY, configurable: true });
+    process.env = origEnv;
+  }
+  return writes.join("");
+}
+
+describe("printUpdateNotice — install-method-aware command", () => {
+  afterEach(() => {
+    vi.doUnmock("./env.js");
+    vi.doUnmock("./installerDetection.js");
+    vi.doUnmock("../telemetry/config.js");
+    vi.resetModules();
+  });
+
+  it("shows the detected manager's command for an owned global install", async () => {
+    const out = await noticeWith({ installerCommand: "brew upgrade hyperframes" });
+    expect(out).toContain("Update available");
+    expect(out).toContain("brew upgrade hyperframes");
+    expect(out).not.toContain("npx hyperframes@latest");
+  });
+
+  it("falls back to npx hyperframes@latest when the install method is skip/unknown", async () => {
+    const out = await noticeWith({ installerCommand: null });
+    expect(out).toContain("npx hyperframes@latest");
+  });
+
+  it("is suppressed on a non-TTY stderr", async () => {
+    const out = await noticeWith({ installerCommand: "brew upgrade hyperframes", isTTY: false });
+    expect(out).toBe("");
+  });
+
+  it("is suppressed in CI", async () => {
+    const out = await noticeWith({
+      installerCommand: "brew upgrade hyperframes",
+      env: { CI: "true" },
+    });
+    expect(out).toBe("");
+  });
+
+  it("is suppressed by the HYPERFRAMES_NO_UPDATE_CHECK opt-out", async () => {
+    const out = await noticeWith({
+      installerCommand: "brew upgrade hyperframes",
+      env: { HYPERFRAMES_NO_UPDATE_CHECK: "1" },
+    });
+    expect(out).toBe("");
+  });
+});

packages/cli/src/utils/updateCheck.ts

@@ -2,6 +2,18 @@ import { compareVersions } from "compare-versions";
 import { readConfig, writeConfig } from "../telemetry/config.js";
 import { VERSION } from "../version.js";
 import { isDevMode } from "./env.js";
+import { detectInstaller } from "./installerDetection.js";
+
+/**
+ * True when `v` is a strict semver-shaped string. Registry-supplied versions
+ * flow into displayed (and, in `upgrade`, executed) commands; rejecting
+ * non-semver up front means a poisoned `latest` can't smuggle shell
+ * metacharacters into a command the user might copy-paste or we might run.
+ * Shared by the update notice and the `upgrade` command so both guard once.
+ */
+export function isSafeVersion(v: string): boolean {
+  return /^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(v);
+}
 
 const NPM_REGISTRY_URL = "https://registry.npmjs.org/hyperframes/latest";
 const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; // 24 hours
@@ -125,8 +137,17 @@ export function printUpdateNotice(): void {
   const meta = getUpdateMeta();
   if (!meta.updateAvailable || !meta.latestVersion) return;
 
+  // Show the command that updates *this* install: the detected package
+  // manager's upgrade for owned global installs (npm/bun/pnpm/brew), and the
+  // universal `npx hyperframes@latest` for ephemeral/unknown installs (where a
+  // manager command wouldn't apply). detectInstaller() only runs here, after
+  // the suppression + update-available gates, so it adds no cost to normal runs.
+  const safeLatest = isSafeVersion(meta.latestVersion);
+  const managerCommand = safeLatest ? detectInstaller().installCommand(meta.latestVersion) : null;
+  const command = managerCommand ?? "npx hyperframes@latest";
+
   process.stderr.write(
     `\n  Update available: ${meta.version} \u2192 ${meta.latestVersion}\n` +
-      `  Run: npx hyperframes@latest\n\n`,
+      `  Run: ${command}\n\n`,
   );
 }