fix(cli): correct the update-notifier privacy claim and prerelease parsing

Review round 1: five findings, all valid.

The privacy statement was too absolute. The request carries no Sim API key, but
`npm_config_registry` can point at a private mirror, and a token embedded in
that URL is sent with the request - it has to be, or the mirror rejects it. Both
docs now say which credentials are involved and where they go: your registry's,
to the host you configured, never Sim's.

`parseVersion` accepted zero-padded prerelease identifiers. Semver forbids them,
and accepting `2.1.3-preview.09` was worse than cosmetic: `09` failed the
numeric test and fell through to being an alphanumeric identifier, and
alphanumerics outrank every number, so `preview.010` sorted ABOVE `preview.2`.
The file's own doc comment already claimed leading zeroes were rejected "the way
the specification rejects them" - true of the release triple, not of the
prerelease. Now true of both.

The `--version`/`--help` test did not hold the guarantee it advertised. It
watched for a request and a cache file, but neither ever appears from inside a
checkout no matter what runs, because the check suppresses itself there - so it
would have passed even if the hook fired, which is the exact regression it
claims to prevent. It now swaps a sentinel into commander's registered
preAction hooks and asserts the sentinel does not fire while parsing those two,
then asserts it DOES fire for a real action command, so the negative assertion
means something. No module mocking, which this package bans.

The troubleshooting page hardcoded `npm install -g`, which installs a second
copy under a different package manager rather than replacing the executable on
PATH. It now shows all three, and says the notice already prints the one
matching your install - which the notifier has always done.

Tests: 861 -> 863. Both new guards mutation-checked: dropping the leading-zero
rejection and deleting the hook each fail the suite.

Author: mzxchandra

apps/docs/content/docs/cli/configuration.mdx

@@ -132,8 +132,16 @@ both versions and the command that upgrades:
 Update available: sim 2.1.2 → 2.1.5. Run: npm install -g sim@latest
 ```
 
-The request carries the CLI version and nothing else — no API key, no workspace,
-no command — and it never follows a redirect away from the registry you asked.
+The request carries the CLI version and nothing else — no Sim API key, no
+workspace, no command — and it never follows a redirect away from the registry
+it asked.
+
+One caveat worth stating plainly: if you point `npm_config_registry` at a
+private mirror, the check goes to that mirror instead of npm, and any
+credentials embedded in that URL (an Artifactory or Nexus `?token=…`) are sent
+with it — they have to be, or the mirror would reject the request. Those are
+your registry's credentials, not Sim's, and they go only to the host you
+configured.
 
 The notice is skipped entirely when:
 
@@ -149,6 +157,11 @@ The notice is skipped entirely when:
 Set `npm_config_registry` to ask a mirror instead; its path and query are
 preserved, so a token-authenticated Artifactory or Nexus base works.
 
+The command the notice prints matches how Sim was installed — `npm install -g`,
+`pnpm add -g`, `bun add -g`, or `yarn global add` — so running it updates the
+executable already on your `PATH` rather than installing a second copy under a
+different package manager.
+
 Node ignores `HTTPS_PROXY` unless you also set `NODE_USE_ENV_PROXY=1`, and only
 from Node 22.21 and 24.5. The CLI warns when a proxy is configured but will not
 be used.

apps/docs/content/docs/cli/troubleshooting.mdx

@@ -3,6 +3,8 @@ title: Troubleshooting
 description: The failures whose cause is not obvious from the error message
 ---
 
+import { Tab, Tabs } from 'fumadocs-ui/components/tabs'
+
 Errors print one line to stderr, prefixed `Error:`, and exit `1` — except
 `sim whoami`, which exits `2` when it could not reach the API to check at all.
 Most say what to do next; the cases below are the ones that do not.
@@ -93,11 +95,32 @@ The docs track the current release, so a command that exists here and not in
 
 ```bash
 sim --version
-npm install -g sim@latest
 ```
 
-The CLI normally tells you this itself, once a day, on stderr. It stays quiet
-when stderr is redirected, in CI, and under `npx`.
+Then upgrade with the package manager you installed it with — using a different
+one installs a second copy instead of replacing the executable on your `PATH`:
+
+<Tabs items={['npm', 'pnpm', 'bun']}>
+  <Tab value="npm">
+    ```bash
+    npm install -g sim@latest
+    ```
+  </Tab>
+  <Tab value="pnpm">
+    ```bash
+    pnpm add -g sim@latest
+    ```
+  </Tab>
+  <Tab value="bun">
+    ```bash
+    bun add -g sim@latest
+    ```
+  </Tab>
+</Tabs>
+
+The CLI normally tells you this itself, once a day, on stderr, and the command
+it prints already matches your installation. It stays quiet when stderr is
+redirected, in CI, and under `npx`.
 
 ## An update notice appears in output I am parsing
 

packages/sim-cli/README.md

@@ -263,9 +263,12 @@ The main environment variables are:
 
 Once a day, at an interactive terminal, `sim` asks `registry.npmjs.org` which
 version is published under the tag it was installed from, and prints one line on
-stderr when a newer one exists. It sends nothing but its own version, never a
-key. Set `SIM_NO_UPDATE_CHECK=1` to turn it off; the full list of cases where it
-stays quiet is in the [configuration guide](https://docs.sim.ai/cli/configuration).
+stderr when a newer one exists. It sends nothing but its own version and never
+your Sim API key. If `npm_config_registry` points at a private mirror, the check
+goes there instead and carries whatever credentials that URL embeds, since the
+mirror would otherwise refuse it. Set `SIM_NO_UPDATE_CHECK=1` to turn it off;
+the full list of cases where it stays quiet is in the
+[configuration guide](https://docs.sim.ai/cli/configuration).
 
 ## Documentation
 

packages/sim-cli/src/program.test.ts

@@ -1,17 +1,19 @@
 /**
  * @vitest-environment node
  */
-import { mkdtempSync, readdirSync, rmSync } from 'node:fs'
+import { mkdtempSync, rmSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import type { Command } from 'commander'
-import { describe, expect, it, vi } from 'vitest'
+import { describe, expect, it } from 'vitest'
 import { buildProgram } from './program'
 import { CLI_VERSION } from './version'
 
 /** Parses argv against a program whose output and exits are captured, not taken. */
-async function parse(argv: string[]): Promise<{ out: string; code: string | null }> {
-  const program = buildProgram()
+async function parse(
+  argv: string[],
+  program: Command = buildProgram()
+): Promise<{ out: string; code: string | null }> {
   let out = ''
   const capture = (command: Command) => {
     command.exitOverride()
@@ -163,33 +165,48 @@ describe('help typed after a command that does not exist', () => {
   })
 })
 
+/** Commander keeps lifecycle hooks on a private field and offers no getter. */
+function preActionHooks(program: Command): Array<(a: Command, b: Command) => unknown> {
+  const { _lifeCycleHooks: hooks } = program as Command & {
+    _lifeCycleHooks?: Record<string, Array<(a: Command, b: Command) => unknown>>
+  }
+  return hooks?.preAction ?? []
+}
+
 describe('the update check', () => {
   /**
    * The notice must cost `--version` and `--help` nothing. Commander answers
    * both during parsing, before any action hook runs, so the guarantee is
    * structural — this holds it in place if the check is ever moved.
+   *
+   * It swaps in a sentinel hook rather than watching for a request or a cache
+   * file. Those side effects never appear from inside a checkout no matter
+   * what runs, because the check suppresses itself there — so asserting on
+   * them would pass even if the hook fired, which is precisely the regression
+   * this is meant to catch.
    */
-  it('never runs for the two commands commander answers during parsing', async () => {
-    const stderr = process.stderr
-    const wasTty = stderr.isTTY
-    const dir = mkdtempSync(join(tmpdir(), 'sim-cli-program-'))
-    const requests: string[] = []
-    Object.defineProperty(stderr, 'isTTY', { configurable: true, value: true })
-    process.env.SIM_CONFIG_DIR = dir
-    vi.stubGlobal('fetch', (input: URL) => {
-      requests.push(String(input))
-      return Promise.resolve(Response.json({ latest: '99.0.0' }))
+  it('fires no preAction hook for the two commands commander answers while parsing', async () => {
+    let fired = 0
+    const program = buildProgram()
+    const hooks = preActionHooks(program)
+    expect(hooks).toHaveLength(1)
+    hooks.splice(0, hooks.length, () => {
+      fired += 1
     })
 
+    await parse(['--version'], program)
+    await parse(['--help'], program)
+    expect(fired).toBe(0)
+
+    // And the sentinel is not inert: the same hook does fire for a real action,
+    // which is what makes the assertion above mean something.
+    const dir = mkdtempSync(join(tmpdir(), 'sim-cli-program-'))
+    process.env.SIM_CONFIG_DIR = dir
     try {
-      await parse(['--version'])
-      await parse(['--help'])
-      expect(requests).toEqual([])
-      expect(readdirSync(dir)).toEqual([])
+      await parse(['configure', '--set-output', 'json'], program)
+      expect(fired).toBe(1)
     } finally {
-      vi.unstubAllGlobals()
       process.env.SIM_CONFIG_DIR = undefined
-      Object.defineProperty(stderr, 'isTTY', { configurable: true, value: wasTty })
       rmSync(dir, { recursive: true, force: true })
     }
   })
@@ -207,12 +224,7 @@ describe('the update check', () => {
    */
   it('registers the update check as a root preAction hook', async () => {
     const program = buildProgram()
-    // Commander keeps lifecycle hooks on a private field and offers no getter,
-    // the same way `rawArgs` is read elsewhere in this file.
-    const { _lifeCycleHooks: hooks } = program as Command & {
-      _lifeCycleHooks?: Record<string, Array<(a: Command, b: Command) => unknown>>
-    }
-    const preAction = hooks?.preAction ?? []
+    const preAction = preActionHooks(program)
 
     expect(preAction).toHaveLength(1)
     // Invoking it must resolve, never throw: it runs in front of the user's

packages/sim-cli/src/update/semver.test.ts

@@ -34,6 +34,8 @@ describe('parsing a published version', () => {
     ['01.2.3', 'a leading zero'],
     ['2.1.2-', 'an empty prerelease'],
     ['2.1.2-preview..1', 'an empty identifier'],
+    ['2.1.3-preview.09', 'a zero-padded numeric identifier'],
+    ['2.1.3-01', 'a zero-padded identifier on its own'],
     ['', 'nothing at all'],
     ['latest', 'a dist-tag mistaken for a version'],
   ])('rejects %s (%s)', (version) => {
@@ -63,6 +65,13 @@ describe('precedence', () => {
     expect(order('2.1.3', '2.1.3-preview.44.1')).toBe(1)
   })
 
+  it('does not let a malformed identifier outrank every number', () => {
+    // `09` is not a valid numeric identifier. Accepting it would reclassify it
+    // as alphanumeric, and alphanumerics outrank numbers — so `preview.010`
+    // would sort above `preview.2`.
+    expect(parseVersion('2.1.3-preview.010')).toBeNull()
+  })
+
   it('orders two alphanumeric identifiers by ASCII', () => {
     expect(order('2.1.3-alpha', '2.1.3-beta')).toBe(-1)
     expect(order('2.1.3-beta', '2.1.3-alpha')).toBe(1)

packages/sim-cli/src/update/semver.ts

@@ -19,6 +19,15 @@ const VERSION_PATTERN =
 /** A prerelease identifier that is all digits compares as a number. */
 const NUMERIC_IDENTIFIER = /^(0|[1-9]\d*)$/
 
+/**
+ * A numeric prerelease identifier carrying a leading zero, which the
+ * specification forbids. It has to be spotted rather than simply failing
+ * `NUMERIC_IDENTIFIER`: falling through would silently reclassify `09` as an
+ * alphanumeric identifier, and alphanumerics outrank every number — so
+ * `preview.010` would sort above `preview.2`.
+ */
+const LEADING_ZERO_IDENTIFIER = /^0\d+$/
+
 export interface ParsedVersion {
   major: number
   minor: number
@@ -58,13 +67,12 @@ export function parseVersion(version: string): ParsedVersion | null {
     return null
   }
 
-  const prerelease = match[4]
-    ? match[4]
-        .split('.')
-        .map((identifier) =>
-          NUMERIC_IDENTIFIER.test(identifier) ? Number(identifier) : identifier
-        )
-    : []
+  const identifiers = match[4] ? match[4].split('.') : []
+  if (identifiers.some((identifier) => LEADING_ZERO_IDENTIFIER.test(identifier))) return null
+
+  const prerelease = identifiers.map((identifier) =>
+    NUMERIC_IDENTIFIER.test(identifier) ? Number(identifier) : identifier
+  )
   if (
     prerelease.some(
       (identifier) => typeof identifier === 'number' && !Number.isSafeInteger(identifier)