diff --git a/.changeset/auto-upgrade-pnpm-homebrew.md b/.changeset/auto-upgrade-pnpm-homebrew.md new file mode 100644 index 0000000000..40f39a233d --- /dev/null +++ b/.changeset/auto-upgrade-pnpm-homebrew.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Fix pnpm v8+ auto-upgrade command (use `--global` instead of `-g`) and add a 120-second timeout guard for Homebrew upgrades. diff --git a/packages/cli-kit/src/public/node/hooks/postrun.test.ts b/packages/cli-kit/src/public/node/hooks/postrun.test.ts index 4d2bc6ff6d..a47e5a688e 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.test.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.test.ts @@ -47,7 +47,9 @@ describe('autoUpgradeIfNeeded', () => { vi.mocked(versionToAutoUpgrade).mockReturnValue('3.100.0') vi.mocked(isMajorVersionChange).mockReturnValue(false) vi.mocked(runCLIUpgrade).mockRejectedValue(new Error('upgrade failed')) - vi.mocked(getOutputUpdateCLIReminder).mockReturnValue('💡 Version 3.100.0 available! Run `npm install -g @shopify/cli@latest`') + vi.mocked(getOutputUpdateCLIReminder).mockReturnValue( + '💡 Version 3.100.0 available! Run `npm install -g @shopify/cli@latest`', + ) await autoUpgradeIfNeeded() diff --git a/packages/cli-kit/src/public/node/is-global.test.ts b/packages/cli-kit/src/public/node/is-global.test.ts index 5142ba9d77..5ed18d5186 100644 --- a/packages/cli-kit/src/public/node/is-global.test.ts +++ b/packages/cli-kit/src/public/node/is-global.test.ts @@ -2,7 +2,7 @@ import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, installGlobalCL import {terminalSupportsPrompting} from './system.js' import {renderSelectPrompt} from './ui.js' import {globalCLIVersion} from './version.js' -import {beforeEach, describe, expect, test, vi, afterEach} from 'vitest' +import {describe, expect, test, vi, afterEach} from 'vitest' vi.mock('./system.js') vi.mock('./ui.js') diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index 537275458e..f0ecc6ad83 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -26,6 +26,8 @@ export interface ExecOptions { externalErrorHandler?: (error: unknown) => Promise // Ignored on Windows background?: boolean + // Maximum time in milliseconds to wait for the process to exit + timeout?: number } /** @@ -311,6 +313,7 @@ function buildExec( windowsHide: false, detached: options?.background, cleanup: !options?.background, + timeout: options?.timeout, ...execaOptions, }) outputDebug(`Running system process${options?.background ? ' in background' : ''}: diff --git a/packages/cli-kit/src/public/node/upgrade.test.ts b/packages/cli-kit/src/public/node/upgrade.test.ts index 83a5d3d8b9..7c81f6e981 100644 --- a/packages/cli-kit/src/public/node/upgrade.test.ts +++ b/packages/cli-kit/src/public/node/upgrade.test.ts @@ -2,7 +2,8 @@ import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI} from './is-glob import {checkForCachedNewVersion} from './node-package-manager.js' import {cliInstallCommand, versionToAutoUpgrade, runCLIUpgrade} from './upgrade.js' import {exec} from './system.js' -import {vi, describe, test, expect, beforeEach} from 'vitest' +import {mockAndCaptureOutput} from './testing/output.js' +import {vi, describe, test, expect, beforeEach, afterEach} from 'vitest' vi.mock('./is-global.js') vi.mock('./node-package-manager.js') @@ -45,16 +46,21 @@ describe('cliInstallCommand', () => { expect(got).toMatchInlineSnapshot(`"npm install -g @shopify/cli@latest"`) }) - test('returns pnpm add -g for pnpm', () => { + test('returns pnpm add --global for pnpm (v8+ compatible)', () => { vi.mocked(currentProcessIsGlobal).mockReturnValue(true) vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('pnpm') const got = cliInstallCommand() - expect(got).toMatchInlineSnapshot(`"pnpm add -g @shopify/cli@latest"`) + expect(got).toMatchInlineSnapshot(`"pnpm add --global @shopify/cli@latest"`) }) }) +afterEach(() => { + mockAndCaptureOutput().clear() + vi.unstubAllEnvs() +}) + describe('versionToAutoUpgrade', () => { beforeEach(() => { vi.unstubAllEnvs() @@ -98,7 +104,7 @@ describe('versionToAutoUpgrade', () => { }) describe('runCLIUpgrade', () => { - test('calls exec with the install command for global installs', async () => { + test('calls exec with the install command for global npm installs', async () => { vi.mocked(currentProcessIsGlobal).mockReturnValue(true) vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') vi.mocked(exec).mockResolvedValue(undefined) @@ -107,4 +113,32 @@ describe('runCLIUpgrade', () => { expect(exec).toHaveBeenCalledWith('npm', ['install', '-g', '@shopify/cli@latest'], {stdio: 'inherit'}) }) + + test('calls exec with 120s timeout for homebrew installs', async () => { + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('homebrew') + vi.mocked(exec).mockResolvedValue(undefined) + + await runCLIUpgrade() + + expect(exec).toHaveBeenCalledWith( + 'brew', + ['upgrade', 'shopify-cli'], + expect.objectContaining({stdio: 'inherit', timeout: 120_000}), + ) + }) + + test('warns and re-throws on homebrew timeout', async () => { + const outputMock = mockAndCaptureOutput() + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('homebrew') + + const timeoutError = Object.assign(new Error('timed out'), {timedOut: true}) + vi.mocked(exec).mockImplementation(async (_cmd, _args, opts) => { + if (opts?.externalErrorHandler) await opts.externalErrorHandler(timeoutError) + }) + + await expect(runCLIUpgrade()).rejects.toThrow() + expect(outputMock.warn()).toContain('timed out') + }) }) diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index b7dd881016..142b8aee07 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -10,12 +10,15 @@ import { addNPMDependencies, getPackageManager, } from './node-package-manager.js' -import {outputContent, outputDebug, outputInfo, outputToken} from './output.js' +import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from './output.js' import {cwd, moduleDirectory, sniffForPath} from './path.js' import {exec, isCI} from './system.js' import {isPreReleaseVersion} from './version.js' import {CLI_KIT_VERSION} from '../common/version.js' +// Homebrew triggers `brew update` as a side effect, which can take 30–90 s on slow networks. +const HOMEBREW_TIMEOUT_MS = 120_000 + /** * Utility function for generating an install command for the user to run * to install an updated version of Shopify CLI. @@ -29,10 +32,12 @@ export function cliInstallCommand(): string | undefined { if (packageManager === 'homebrew') { return 'brew upgrade shopify-cli' } else if (packageManager === 'yarn') { - return `${packageManager} global add @shopify/cli@latest` + return 'yarn global add @shopify/cli@latest' + } else if (packageManager === 'pnpm') { + // pnpm v8+ requires --global instead of -g + return 'pnpm add --global @shopify/cli@latest' } else { - const verb = packageManager === 'pnpm' ? 'add' : 'install' - return `${packageManager} ${verb} -g @shopify/cli@latest` + return `${packageManager} install -g @shopify/cli@latest` } } @@ -67,7 +72,21 @@ export async function runCLIUpgrade(): Promise { throw new Error('Could not determine the command to run') } outputInfo(outputContent`Upgrading Shopify CLI by running: ${outputToken.genericShellCommand(installCommand)}...`) - await exec(command, args, {stdio: 'inherit'}) + const packageManager = inferPackageManagerForGlobalCLI() + if (packageManager === 'homebrew') { + await exec(command, args, { + stdio: 'inherit', + timeout: HOMEBREW_TIMEOUT_MS, + externalErrorHandler: async (error: unknown) => { + if ((error as {timedOut?: boolean}).timedOut) { + outputWarn('Homebrew upgrade timed out. Run `brew upgrade shopify-cli` manually.') + } + throw error + }, + }) + } else { + await exec(command, args, {stdio: 'inherit'}) + } } else if (projectDir) { await upgradeLocalShopify(projectDir, CLI_KIT_VERSION) } else { diff --git a/packages/cli/README.md b/packages/cli/README.md index 0ef528b2e4..07fb8dee74 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -2765,16 +2765,16 @@ DESCRIPTION ## `shopify upgrade` -Shows details on how to upgrade Shopify CLI. +Upgrades Shopify CLI. ``` USAGE $ shopify upgrade DESCRIPTION - Shows details on how to upgrade Shopify CLI. + Upgrades Shopify CLI. - Shows details on how to upgrade Shopify CLI. + Upgrades Shopify CLI using your package manager. ``` ## `shopify version` diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 8a053bd1ea..78675c99fc 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -7822,8 +7822,8 @@ ], "args": { }, - "description": "Shows details on how to upgrade Shopify CLI.", - "descriptionWithMarkdown": "Shows details on how to upgrade Shopify CLI.", + "description": "Upgrades Shopify CLI using your package manager.", + "descriptionWithMarkdown": "Upgrades Shopify CLI using your package manager.", "enableJsonFlag": false, "flags": { }, @@ -7835,7 +7835,7 @@ "pluginName": "@shopify/cli", "pluginType": "core", "strict": true, - "summary": "Shows details on how to upgrade Shopify CLI." + "summary": "Upgrades Shopify CLI." }, "version": { "aliases": [ diff --git a/packages/cli/src/cli/commands/upgrade.test.ts b/packages/cli/src/cli/commands/upgrade.test.ts index e0e72458a1..890b87ae6d 100644 --- a/packages/cli/src/cli/commands/upgrade.test.ts +++ b/packages/cli/src/cli/commands/upgrade.test.ts @@ -1,13 +1,12 @@ import Upgrade from './upgrade.js' +import {runCLIUpgrade} from '@shopify/cli-kit/node/upgrade' import {describe, test, vi, expect} from 'vitest' -vi.mock('@shopify/cli-kit/node/upgrade', () => ({ - runCLIUpgrade: vi.fn().mockResolvedValue(undefined), -})) +vi.mock('@shopify/cli-kit/node/upgrade') describe('upgrade command', () => { test('calls runCLIUpgrade', async () => { - const {runCLIUpgrade} = await import('@shopify/cli-kit/node/upgrade') + vi.mocked(runCLIUpgrade).mockResolvedValue(undefined) await Upgrade.run([], import.meta.url)