[Phase 1] Fix pnpm v8+ and Homebrew timeout in auto-upgrade

.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.

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()
 

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')

packages/cli-kit/src/public/node/system.ts

@@ -26,6 +26,8 @@ export interface ExecOptions {
   externalErrorHandler?: (error: unknown) => Promise<void>
   // 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' : ''}:

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')
+  })
 })

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<void> {
       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 {

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`

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": [

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)