Fix false success message when CLI self-upgrade installs the wrong version

.changeset/verify-installed-version-after-upgrade.md

@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': patch
+'@shopify/cli': patch
+---
+
+Verify the installed version after `shopify upgrade` and fail instead of reporting a false success when the upgrade didn't complete

packages/cli-kit/src/public/node/upgrade.test.ts

@@ -1,4 +1,4 @@
-import {isDevelopment} from './context/local.js'
+import {isDevelopment, isUnitTest} from './context/local.js'
 import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI} from './is-global.js'
 import {checkForCachedNewVersion, packageManagerFromUserAgent, PackageManager} from './node-package-manager.js'
 import {exec, isCI} from './system.js'
@@ -10,8 +10,10 @@ import {
   versionToAutoUpgrade,
 } from './upgrade.js'
 import {Notification, fetchNotifications} from './notifications-system.js'
-import {isPreReleaseVersion} from './version.js'
+import {globalCLIVersion, isPreReleaseVersion} from './version.js'
+import {mockAndCaptureOutput} from './testing/output.js'
 import {getAutoUpgradeEnabled} from '../../private/node/conf-store.js'
+import {CLI_KIT_VERSION} from '../common/version.js'
 import {vi, describe, test, expect, beforeEach} from 'vitest'
 
 vi.mock('./notifications-system.js', async (importOriginal) => {
@@ -31,6 +33,7 @@ vi.mock('./version.js', async (importOriginal) => {
   return {
     ...actual,
     isPreReleaseVersion: vi.fn(() => false),
+    globalCLIVersion: vi.fn(),
   }
 })
 
@@ -146,6 +149,12 @@ describe('runCLIUpgrade', () => {
   beforeEach(() => {
     // Mock isDevelopment to return false by default (not in CLI development mode)
     vi.mocked(isDevelopment).mockReturnValue(false)
+    // context/local.js is auto-mocked; restore isUnitTest so rendered banners are collected
+    // by mockAndCaptureOutput instead of printed to the real console.
+    vi.mocked(isUnitTest).mockReturnValue(true)
+    // By default the post-install verification finds the current version installed,
+    // which counts as success when no newer version was cached.
+    vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION)
   })
 
   test('runs the install command via exec for a global npm install', async () => {
@@ -219,6 +228,63 @@ describe('runCLIUpgrade', () => {
     // Then
     expect(exec).not.toHaveBeenCalled()
   })
+
+  test('reports the verified installed version on success', async () => {
+    // Given
+    vi.mocked(currentProcessIsGlobal).mockReturnValue(true)
+    vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
+    vi.mocked(exec).mockResolvedValue()
+    vi.mocked(checkForCachedNewVersion).mockReturnValue('4.7.1')
+    vi.mocked(globalCLIVersion).mockResolvedValue('4.7.1')
+    const outputMock = mockAndCaptureOutput()
+    outputMock.clear()
+
+    // When
+    await runCLIUpgrade()
+
+    // Then
+    expect(outputMock.info()).toContain("You're now on version 4.7.1")
+  })
+
+  test('throws when the install lands an older version than expected (e.g. a stale private registry)', async () => {
+    // Given
+    vi.mocked(currentProcessIsGlobal).mockReturnValue(true)
+    vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
+    vi.mocked(exec).mockResolvedValue()
+    vi.mocked(checkForCachedNewVersion).mockReturnValue('4.7.1')
+    vi.mocked(globalCLIVersion).mockResolvedValue('3.94.3')
+    const outputMock = mockAndCaptureOutput()
+    outputMock.clear()
+
+    // When/Then
+    await expect(runCLIUpgrade()).rejects.toThrow(
+      'Expected to be on version 4.7.1, but version 3.94.3 is now installed',
+    )
+    expect(outputMock.info()).not.toContain('Shopify CLI upgraded')
+  })
+
+  test('throws when the install leaves the CLI on the current version instead of the expected one', async () => {
+    // Given
+    vi.mocked(currentProcessIsGlobal).mockReturnValue(true)
+    vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
+    vi.mocked(exec).mockResolvedValue()
+    vi.mocked(checkForCachedNewVersion).mockReturnValue('4.7.1')
+    vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION)
+
+    // When/Then
+    await expect(runCLIUpgrade()).rejects.toThrow(`Expected to be on version 4.7.1, but version ${CLI_KIT_VERSION}`)
+  })
+
+  test('throws when the installed version cannot be verified', async () => {
+    // Given
+    vi.mocked(currentProcessIsGlobal).mockReturnValue(true)
+    vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
+    vi.mocked(exec).mockResolvedValue()
+    vi.mocked(globalCLIVersion).mockResolvedValue(undefined)
+
+    // When/Then
+    await expect(runCLIUpgrade()).rejects.toThrow("Couldn't verify the Shopify CLI version after upgrading")
+  })
 })
 
 describe('versionToAutoUpgrade', () => {

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

@@ -15,9 +15,11 @@ import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from '.
 import {renderSuccess} from './ui.js'
 import {cwd, moduleDirectory, sniffForPath} from './path.js'
 import {exec, isCI} from './system.js'
-import {isPreReleaseVersion} from './version.js'
+import {globalCLIVersion, isPreReleaseVersion} from './version.js'
+import {AbortError} from './error.js'
 import {getAutoUpgradeEnabled, setAutoUpgradeEnabled, runAtMinimumInterval} from '../../private/node/conf-store.js'
 import {CLI_KIT_VERSION} from '../common/version.js'
+import {lt as semverLt} from 'semver'
 
 export {getAutoUpgradeEnabled, setAutoUpgradeEnabled}
 
@@ -104,9 +106,29 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise
    Now upgrading by running: ${outputToken.genericShellCommand(installCommand)}...`,
     )
     await exec(command, args, {stdio: 'inherit'})
+
+    // A zero exit code doesn't guarantee the right version landed: the version check above
+    // queries the public npm registry, while the install goes through whatever registry the
+    // user has configured. A private registry with a stale `latest` tag (or one that serves
+    // stale metadata on auth errors) can "successfully" install an outdated version. Verify
+    // what's actually installed before claiming success.
+    const installedVersion = await globalCLIVersion()
+    if (!installedVersion) {
+      throw new AbortError(
+        "Couldn't verify the Shopify CLI version after upgrading.",
+        outputContent`Check the installed version by running ${outputToken.genericShellCommand('shopify version')}.`,
+      )
+    }
+    const expectedVersion = newerVersion ?? CLI_KIT_VERSION
+    if (semverLt(installedVersion, expectedVersion)) {
+      throw new AbortError(
+        `Failed to upgrade Shopify CLI. Expected to be on version ${expectedVersion}, but version ${installedVersion} is now installed.`,
+        'Your package manager may be resolving @shopify/cli from a registry with outdated versions. Check your npm registry configuration and try again.',
+      )
+    }
     renderSuccess({
       headline: 'Shopify CLI upgraded.',
-      body: newerVersion ? `You're now on version ${newerVersion}.` : "You're now on the latest version.",
+      body: `You're now on version ${installedVersion}.`,
     })
   } else if (projectDir) {
     await upgradeLocalShopify(projectDir, CLI_KIT_VERSION)