diff --git a/README.md b/README.md index a619562..87f9935 100644 --- a/README.md +++ b/README.md @@ -114,11 +114,12 @@ registerUpdateCommand(program, { currentVersion: packageJson.version, configPath: getConfigPath('todoist-cli'), changelogCommandName: 'td changelog', + brewFormula: 'doist/tap/todoist-cli', withSpinner, }) ``` -`update` checks the configured channel's npm dist-tag (`stable` → `latest`, `pre-release` → `next`), compares against `currentVersion`, and shells out to `npm i -g` (or `pnpm add -g` if `npm_execpath` indicates pnpm). `update switch --stable | --pre-release` flips the persisted `update_channel` field via `updateConfig`, preserving any sibling keys. Both subcommands accept `--json` / `--ndjson`. Errors are `CliError` (`INVALID_FLAGS`, `UPDATE_CHECK_FAILED`, `UPDATE_INSTALL_FAILED`, or the canonical `CONFIG_*` codes if the config file is broken). +`update` checks the configured channel's npm dist-tag (`stable` → `latest`, `pre-release` → `next`), compares against `currentVersion`, and shells out to `npm i -g` (or `pnpm add -g` if `npm_execpath` indicates pnpm). When the CLI was installed via Homebrew (its binary resolves into a brew `Cellar`), it runs `brew upgrade ` instead — set `brewFormula` on brew-distributed CLIs (the brew formula may lag the npm publish, so an upgrade can be a no-op until the formula is bumped). `update switch --stable | --pre-release` flips the persisted `update_channel` field via `updateConfig`, preserving any sibling keys. Both subcommands accept `--json` / `--ndjson`. Errors are `CliError` (`INVALID_FLAGS`, `UPDATE_CHECK_FAILED`, `UPDATE_INSTALL_FAILED`, or the canonical `CONFIG_*` codes if the config file is broken). The semver helpers (`parseVersion`, `compareVersions`, `isNewer`, `getInstallTag`, `fetchLatestVersion`, `getConfiguredUpdateChannel`) are also exported for ad-hoc use outside the registered command. diff --git a/src/commands/update.test.ts b/src/commands/update.test.ts index 9efc250..19d3d71 100644 --- a/src/commands/update.test.ts +++ b/src/commands/update.test.ts @@ -13,6 +13,11 @@ import { vi.mock('node:child_process', () => ({ spawn: vi.fn() })) +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, realpathSync: vi.fn(actual.realpathSync) } +}) + vi.mock('../config.js', async (importOriginal) => { const actual = await importOriginal() return { @@ -23,8 +28,10 @@ vi.mock('../config.js', async (importOriginal) => { }) const { spawn } = await import('node:child_process') +const { realpathSync } = await import('node:fs') const config = await import('../config.js') const mockSpawn = vi.mocked(spawn) +const mockRealpathSync = vi.mocked(realpathSync) const mockReadConfigOrThrow = vi.mocked(config.readConfigOrThrow) const mockUpdateConfigOrThrow = vi.mocked(config.updateConfigOrThrow) @@ -35,10 +42,10 @@ const BASE_OPTIONS: UpdateCommandOptions = { changelogCommandName: 'td changelog', } -function createProgram(): Command { +function createProgram(overrides?: Partial): Command { const program = new Command() program.name('td').exitOverride() - registerUpdateCommand(program, BASE_OPTIONS) + registerUpdateCommand(program, { ...BASE_OPTIONS, ...overrides }) return program } @@ -79,6 +86,9 @@ beforeEach(() => { mockReadConfigOrThrow.mockReset().mockResolvedValue({}) mockUpdateConfigOrThrow.mockReset().mockResolvedValue(undefined) mockSpawn.mockClear() + // Identity by default → resolved path has no `/Cellar/`, so the npm/pnpm + // cases stay brew-negative. Brew tests override this per-test. + mockRealpathSync.mockReset().mockImplementation((p) => String(p)) }) afterEach(() => { @@ -257,8 +267,160 @@ describe('update install flow', () => { latestVersion: '99.99.99', channel: 'stable', installed: true, + via: 'npm', + }) + }) +}) + +describe('update brew install flow', () => { + const CELLAR_PATH = '/opt/homebrew/Cellar/todoist-cli/1.1.0/bin/td' + const FORMULA = 'doist/tap/todoist-cli' + const realPlatform = process.platform + + // `brew upgrade` exits `upgradeExit`; the follow-up `brew list --versions` + // reports `listedVersion` on stdout (used to derive the installed result). + function mockBrew({ + upgradeExit = 0, + listedVersion, + }: { upgradeExit?: number; listedVersion?: string } = {}) { + mockSpawn.mockImplementation(((_cmd: string, args: readonly string[]) => ({ + stdout: { + on: vi.fn((event: string, cb: (chunk: Buffer) => void) => { + if (event === 'data' && args[0] === 'list' && listedVersion) { + cb(Buffer.from(`todoist-cli ${listedVersion}\n`)) + } + }), + }, + stderr: { on: vi.fn() }, + on: vi.fn((event: string, cb: (arg?: unknown) => void) => { + if (event === 'close') cb(args[0] === 'list' ? 0 : upgradeExit) + }), + })) as never) + } + + beforeEach(() => { + // isBrewInstall() short-circuits on win32, bypassing the realpathSync + // mock; pin a posix platform so these tests are deterministic on any host. + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: realPlatform, configurable: true }) + }) + + it('runs `brew upgrade ` with inherited stdio and a neutral success line', async () => { + mockRealpathSync.mockReturnValue(CELLAR_PATH) + mockFetchOk('99.99.99') + mockBrew({ listedVersion: '1.1.0' }) + await createProgram({ brewFormula: FORMULA }).parseAsync(['node', 'td', 'update']) + expect(mockSpawn).toHaveBeenCalledWith('brew', ['upgrade', FORMULA], { stdio: 'inherit' }) + // The success line must not claim the npm dist-tag version was installed + // (the pre-install headline may still mention it as the available target). + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('brew upgrade complete')) + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Updated to v')) + }) + + it.each([ + ['version changed → installed', '1.1.0', true], + ['no-op (formula lags npm) → not installed', '1.0.0', false], + ])( + 'derives installed from the on-disk brew version, keeping a stable --json schema: %s', + async (_, listedVersion, installed) => { + mockRealpathSync.mockReturnValue(CELLAR_PATH) + mockFetchOk('99.99.99') + mockBrew({ listedVersion }) + await createProgram({ brewFormula: FORMULA }).parseAsync([ + 'node', + 'td', + 'update', + '--json', + ]) + // stdout piped (not inherited) under --json so brew can't corrupt the stream. + expect(mockSpawn).toHaveBeenCalledWith('brew', ['upgrade', FORMULA], { + stdio: ['ignore', 'ignore', 'pipe'], + }) + const payloads = consoleSpy.mock.calls.map((call: unknown[]) => + JSON.parse(call[0] as string), + ) + expect(payloads).toContainEqual({ + currentVersion: '1.0.0', + latestVersion: '99.99.99', + channel: 'stable', + installed, + via: 'brew', + installedVersion: listedVersion, + }) + }, + ) + + it('throws UPDATE_INSTALL_FAILED on a non-zero brew exit', async () => { + mockRealpathSync.mockReturnValue(CELLAR_PATH) + mockFetchOk('99.99.99') + mockBrew({ upgradeExit: 1 }) + await expect( + createProgram({ brewFormula: FORMULA }).parseAsync(['node', 'td', 'update']), + ).rejects.toMatchObject({ + code: 'UPDATE_INSTALL_FAILED', + message: expect.stringContaining('brew exited with code 1'), }) }) + + it('fails fast (before any registry call) when brew-installed without a formula', async () => { + mockRealpathSync.mockReturnValue(CELLAR_PATH) + mockFetchOk('99.99.99') + await expect(createProgram().parseAsync(['node', 'td', 'update'])).rejects.toMatchObject({ + code: 'UPDATE_INSTALL_FAILED', + hints: [expect.stringContaining('brew upgrade')], + }) + expect(fetch).not.toHaveBeenCalled() + expect(mockSpawn).not.toHaveBeenCalled() + }) + + it('uses npm/pnpm (not brew) when a formula is set but the install is not brew-managed', async () => { + // realpathSync defaults to identity (no `/Cellar/`), so this is a plain + // npm global install even though brewFormula is configured. + mockFetchOk('99.99.99') + mockSpawnExit() + await createProgram({ brewFormula: FORMULA }).parseAsync(['node', 'td', 'update']) + expect(mockSpawn).toHaveBeenCalledWith( + 'npm', + ['install', '-g', '@doist/todoist-cli@latest'], + { + stdio: ['ignore', 'ignore', 'pipe'], + shell: false, + }, + ) + expect(mockSpawn).not.toHaveBeenCalledWith('brew', expect.anything(), expect.anything()) + }) + + it.each([ + ['interactive: no install spinner', [] as string[], false], + ['--json: install spinner (stdout silenced)', ['--json'], true], + ])( + 'threads the install spinner for brew only when quiet (%s)', + async (_, flags, expectSpinner) => { + mockRealpathSync.mockReturnValue(CELLAR_PATH) + mockFetchOk('99.99.99') + mockBrew({ listedVersion: '1.1.0' }) + const withSpinner = vi.fn((_opts: SpinnerOptions, op: () => Promise) => op()) + const program = new Command() + program.name('td').exitOverride() + registerUpdateCommand(program, { + ...BASE_OPTIONS, + brewFormula: FORMULA, + withSpinner: withSpinner as unknown as UpdateCommandOptions['withSpinner'], + }) + await program.parseAsync(['node', 'td', 'update', ...flags]) + const installSpinner = expect.objectContaining({ + text: expect.stringContaining('Updating to v'), + }) + if (expectSpinner) { + expect(withSpinner).toHaveBeenCalledWith(installSpinner, expect.any(Function)) + } else { + expect(withSpinner).not.toHaveBeenCalledWith(installSpinner, expect.any(Function)) + } + }, + ) }) describe('update error paths', () => { diff --git a/src/commands/update.ts b/src/commands/update.ts index acb66f9..d128fb6 100644 --- a/src/commands/update.ts +++ b/src/commands/update.ts @@ -1,4 +1,5 @@ -import { spawn } from 'node:child_process' +import { spawn, type SpawnOptions } from 'node:child_process' +import { realpathSync } from 'node:fs' import chalk from 'chalk' import type { Command } from 'commander' import { @@ -22,6 +23,12 @@ export type UpdateCommandOptions = { currentVersion: string /** Absolute path to the CLI's config file (use `getConfigPath(appName)`). */ configPath: string + /** + * Homebrew formula to `brew upgrade` when the CLI was installed via + * Homebrew, e.g. `'todoist-cli'` or a tapped `'doist/tap/todoist-cli'`. Set + * this on CLIs distributed through brew; omit for npm-only CLIs. + */ + brewFormula?: string /** Override the npm registry base URL. Default `'https://registry.npmjs.org'`. */ registryUrl?: string /** @@ -126,6 +133,24 @@ export async function getConfiguredUpdateChannel(configPath: string): Promise { - const command = pm === 'pnpm' ? 'add' : 'install' return new Promise((resolve, reject) => { - const child = spawn(pm, [command, '-g', `${packageName}@${tag}`], { - // Ignore stdout so a chatty install can't deadlock by filling an - // unread pipe buffer; keep stderr piped so we can surface the tail - // in the CliError hint on a non-zero exit. - stdio: ['ignore', 'ignore', 'pipe'], - // npm/pnpm on Windows are `.cmd` shims that spawn() can't resolve - // without the shell. Safe to enable here because every argv element - // is library-controlled (literal command, fixed flags, validated - // package name + tag). - shell: process.platform === 'win32', - }) + const child = spawn(command, args, options) let stderr = '' child.stderr?.on('data', (data: Buffer) => { stderr += data.toString() @@ -164,6 +178,56 @@ function runInstall( }) } +function runInstall( + pm: string, + packageName: string, + tag: string, +): Promise<{ exitCode: number; stderr: string }> { + const command = pm === 'pnpm' ? 'add' : 'install' + return spawnCapture(pm, [command, '-g', `${packageName}@${tag}`], { + // Ignore stdout so a chatty install can't deadlock by filling an unread + // pipe buffer; keep stderr piped so we can surface the tail in the + // CliError hint on a non-zero exit. + stdio: ['ignore', 'ignore', 'pipe'], + // npm/pnpm on Windows are `.cmd` shims that spawn() can't resolve + // without the shell. Safe to enable here because every argv element is + // library-controlled (literal command, fixed flags, validated package + // name + tag). + shell: process.platform === 'win32', + }) +} + +function runBrewUpgrade( + formula: string, + quiet: boolean, +): Promise<{ exitCode: number; stderr: string }> { + return spawnCapture('brew', ['upgrade', formula], { + // Inherit so brew's own download/upgrade progress reaches the user + // (these can run for a while). Under machine-readable output, fall back + // to the npm-style pipe so brew's chatter can't corrupt the JSON/NDJSON + // stream on stdout. + stdio: quiet ? ['ignore', 'ignore', 'pipe'] : 'inherit', + }) +} + +function brewInstalledVersion(formula: string): Promise { + // `brew list --versions ` prints e.g. `todoist-cli 1.1.0`; the version + // is the last whitespace token. Tap-qualified formulae list under their leaf + // name, so strip the tap prefix. Best-effort — any failure yields undefined. + const leaf = formula.split('/').pop() ?? formula + return new Promise((resolve) => { + let stdout = '' + const child = spawn('brew', ['list', '--versions', leaf], { + stdio: ['ignore', 'pipe', 'ignore'], + }) + child.stdout?.on('data', (data: Buffer) => { + stdout += data.toString() + }) + child.on('error', () => resolve(undefined)) + child.on('close', () => resolve(stdout.trim().split(/\s+/).pop() || undefined)) + }) +} + function formatChannel(channel: UpdateChannel): string { return channel === 'pre-release' ? chalk.magenta('pre-release') : chalk.green('stable') } @@ -203,6 +267,17 @@ async function runUpdate(options: UpdateCommandOptions, cmd: UpdateCmdOptions): const tag = getInstallTag(channel) const label = channelLabel(channel) + // Fail fast on a guaranteed-broken install path before spending a registry + // round-trip. `--check` never installs, so a missing formula is fine there. + const brew = isBrewInstall() + if (!cmd.check && brew && !options.brewFormula) { + throw new CliError( + 'UPDATE_INSTALL_FAILED', + 'Installed via Homebrew but no brew formula is configured.', + { hints: ['Update manually: brew upgrade '] }, + ) + } + let latestVersion: string try { latestVersion = await runWithSpinner( @@ -252,16 +327,27 @@ async function runUpdate(options: UpdateCommandOptions, cmd: UpdateCmdOptions): } const pm = detectPackageManager() + const quiet = Boolean(view.json || view.ndjson) + const task = () => + brew + ? runBrewUpgrade(options.brewFormula as string, quiet) + : runInstall(pm, options.packageName, tag) + // brew prints its own progress, so skip the spinner for it unless we've + // silenced its output for machine-readable mode. + const useSpinner = !brew || quiet let result: { exitCode: number; stderr: string } try { - result = await runWithSpinner( - options.withSpinner, - `Updating to v${latestVersion}${label}...`, - () => runInstall(pm, options.packageName, tag), - ) + result = useSpinner + ? await runWithSpinner( + options.withSpinner, + `Updating to v${latestVersion}${label}...`, + task, + ) + : await task() } catch (error) { if ( + !brew && error instanceof Error && 'code' in error && (error as { code?: unknown }).code === 'EACCES' @@ -279,14 +365,37 @@ async function runUpdate(options: UpdateCommandOptions, cmd: UpdateCmdOptions): if (result.exitCode !== 0) { throw new CliError( 'UPDATE_INSTALL_FAILED', - `${pm} exited with code ${result.exitCode}`, + `${brew ? 'brew' : pm} exited with code ${result.exitCode}`, result.stderr ? { hints: [result.stderr.trim()] } : {}, ) } - emitView(view, { currentVersion, latestVersion, channel, installed: true }, () => { - const lines = [`${chalk.green('✓')} Updated to v${latestVersion}${label}`] - if (channel === 'stable' && options.changelogCommandName) { + // For npm/pnpm we install the exact dist-tag, so the applied version is + // `latestVersion`. brew owns its own versioning and may be a no-op when the + // formula lags npm, so read the version brew actually has on disk and derive + // `installed` from it; if it can't be read, assume the upgrade applied. + const installedVersion = brew + ? await brewInstalledVersion(options.brewFormula as string) + : latestVersion + const installed = brew + ? installedVersion === undefined || installedVersion !== currentVersion + : true + // Stable schema across installers: always carry the registry target + // (`latestVersion`) plus a `via` discriminator; brew adds the applied + // `installedVersion` when known. + const summary = { + currentVersion, + latestVersion, + channel, + installed, + via: brew ? ('brew' as const) : pm, + ...(brew && installedVersion ? { installedVersion } : {}), + } + emitView(view, summary, () => { + const lines = [ + `${chalk.green('✓')} ${brew ? 'brew upgrade complete' : `Updated to v${latestVersion}`}${label}`, + ] + if (installed && channel === 'stable' && options.changelogCommandName) { lines.push( `${chalk.dim(' Run')} ${chalk.cyan(options.changelogCommandName)} ${chalk.dim('to see what changed')}`, ) @@ -340,8 +449,12 @@ async function runSwitch( * Commander program. The `update` action checks the npm registry for the * configured channel's dist-tag, compares against `currentVersion`, and shells * out to `npm i -g` (or `pnpm add -g` if pnpm is detected on the running - * script's path). `update switch` flips the persisted `update_channel` field - * between `'stable'` and `'pre-release'`. + * script's path). When the CLI was installed via Homebrew (the running script + * resolves into a brew Cellar) it instead runs `brew upgrade ` — + * note the brew formula can lag the npm publish, so the registry may report an + * update while `brew upgrade` is a no-op until the formula is bumped. + * `update switch` flips the persisted `update_channel` field between `'stable'` + * and `'pre-release'`. * * Errors as `CliError` (`INVALID_FLAGS`, `INVALID_UPDATE_CHANNEL`, * `UPDATE_CHECK_FAILED`, `UPDATE_INSTALL_FAILED`, or the canonical `CONFIG_*` @@ -363,6 +476,7 @@ async function runSwitch( * currentVersion: packageJson.version, * configPath: getConfigPath('todoist-cli'), * changelogCommandName: 'td changelog', + * brewFormula: 'doist/tap/todoist-cli', * withSpinner, * }) * ```