Merge pull request #7422 from Shopify/add-autoupgrade-notification-switch

Add notification-driven kill switch for auto-upgrade

Author: alfonso-noriega

packages/cli-kit/src/public/node/hooks/postrun.test.ts

@@ -1,6 +1,11 @@
 import {autoUpgradeIfNeeded} from './postrun.js'
 import {mockAndCaptureOutput} from '../testing/output.js'
-import {getOutputUpdateCLIReminder, runCLIUpgrade, versionToAutoUpgrade} from '../upgrade.js'
+import {
+  getOutputUpdateCLIReminder,
+  hasBlockingAutoUpgradeNotification,
+  runCLIUpgrade,
+  versionToAutoUpgrade,
+} from '../upgrade.js'
 import {isMajorVersionChange} from '../version.js'
 import {inferPackageManagerForGlobalCLI} from '../is-global.js'
 import {addPublicMetadata} from '../metadata.js'
@@ -13,6 +18,7 @@ vi.mock('../upgrade.js', async (importOriginal) => {
     runCLIUpgrade: vi.fn(),
     getOutputUpdateCLIReminder: vi.fn(),
     versionToAutoUpgrade: vi.fn(),
+    hasBlockingAutoUpgradeNotification: vi.fn().mockResolvedValue(false),
   }
 })
 
@@ -55,6 +61,7 @@ vi.mock('../../../private/node/conf-store.js', async (importOriginal) => {
 afterEach(() => {
   mockAndCaptureOutput().clear()
   vi.mocked(addPublicMetadata).mockClear()
+  vi.mocked(hasBlockingAutoUpgradeNotification).mockResolvedValue(false)
 })
 
 describe('autoUpgradeIfNeeded', () => {
@@ -126,6 +133,8 @@ describe('autoUpgradeIfNeeded', () => {
         env_auto_upgrade_skipped_reason: 'major_version',
       }),
     )
+    // Notifications should not be queried on the major-version skip path.
+    expect(hasBlockingAutoUpgradeNotification).not.toHaveBeenCalled()
   })
 
   test('records success=true on successful upgrade', async () => {
@@ -140,6 +149,29 @@ describe('autoUpgradeIfNeeded', () => {
     expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_success: true}))
   })
 
+  test('silently skips the upgrade when a blocking autoupgrade notification is active', async () => {
+    const outputMock = mockAndCaptureOutput()
+    vi.mocked(versionToAutoUpgrade).mockReturnValue('3.100.0')
+    vi.mocked(isMajorVersionChange).mockReturnValue(false)
+    vi.mocked(hasBlockingAutoUpgradeNotification).mockResolvedValue(true)
+
+    await autoUpgradeIfNeeded()
+
+    expect(runCLIUpgrade).not.toHaveBeenCalled()
+    expect(outputMock.warn()).toBe('')
+    expect(outputMock.info()).toBe('')
+    const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
+    expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_skipped_reason: 'blocked_by_notification'}))
+  })
+
+  test('does not query notifications when there is no version to upgrade to', async () => {
+    vi.mocked(versionToAutoUpgrade).mockReturnValue(undefined)
+
+    await autoUpgradeIfNeeded()
+
+    expect(hasBlockingAutoUpgradeNotification).not.toHaveBeenCalled()
+  })
+
   test('records success=false on failed upgrade', async () => {
     vi.mocked(versionToAutoUpgrade).mockReturnValue('3.100.0')
     vi.mocked(isMajorVersionChange).mockReturnValue(false)

packages/cli-kit/src/public/node/hooks/postrun.ts

@@ -66,7 +66,7 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
     {CLI_KIT_VERSION},
     {isMajorVersionChange},
     {outputWarn, outputDebug},
-    {getOutputUpdateCLIReminder, runCLIUpgrade},
+    {getOutputUpdateCLIReminder, runCLIUpgrade, hasBlockingAutoUpgradeNotification},
     metadata,
   ] = await Promise.all([
     import('../../common/version.js'),
@@ -84,6 +84,17 @@ async function performAutoUpgrade(newerVersion: string): Promise<void> {
     return
   }
 
+  // Notification kill switch: an `error`-type notification on the `autoupgrade` surface
+  // silently disables auto-upgrade. Checked last — after every other gate, including the
+  // daily rate limit and the major-version check — so the network fetch only happens when
+  // we're about to actually run the upgrade.
+  if (await hasBlockingAutoUpgradeNotification()) {
+    await metadata.addPublicMetadata(() => ({
+      env_auto_upgrade_skipped_reason: 'blocked_by_notification',
+    }))
+    return
+  }
+
   try {
     await runCLIUpgrade()
     await metadata.addPublicMetadata(() => ({env_auto_upgrade_success: true}))

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

@@ -2,11 +2,25 @@ import {isDevelopment} 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'
-import {cliInstallCommand, getOutputUpdateCLIReminder, runCLIUpgrade, versionToAutoUpgrade} from './upgrade.js'
+import {
+  cliInstallCommand,
+  getOutputUpdateCLIReminder,
+  hasBlockingAutoUpgradeNotification,
+  runCLIUpgrade,
+  versionToAutoUpgrade,
+} from './upgrade.js'
+import {Notification, fetchNotifications} from './notifications-system.js'
 import {isPreReleaseVersion} from './version.js'
 import {getAutoUpgradeEnabled} from '../../private/node/conf-store.js'
 import {vi, describe, test, expect, beforeEach} from 'vitest'
 
+vi.mock('./notifications-system.js', async (importOriginal) => {
+  const actual: any = await importOriginal()
+  return {
+    ...actual,
+    fetchNotifications: vi.fn(),
+  }
+})
 vi.mock('./context/local.js')
 vi.mock('./is-global.js')
 vi.mock('./node-package-manager.js')
@@ -253,3 +267,56 @@ describe('versionToAutoUpgrade', () => {
     vi.mocked(isPreReleaseVersion).mockReturnValue(false)
   })
 })
+
+describe('hasBlockingAutoUpgradeNotification', () => {
+  function notification(overrides: Partial<Notification> = {}): Notification {
+    return {
+      id: 'block-autoupgrade',
+      message: 'Auto-upgrade temporarily disabled',
+      type: 'error',
+      frequency: 'always',
+      ownerChannel: '#cli',
+      surface: 'autoupgrade',
+      ...overrides,
+    }
+  }
+
+  test('returns true when an error notification on the autoupgrade surface is active', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({notifications: [notification()]})
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(true)
+  })
+
+  test('returns false for non-error notifications on the autoupgrade surface', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({
+      notifications: [notification({type: 'warning'}), notification({type: 'info'})],
+    })
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+
+  test('returns false for error notifications on other surfaces', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({notifications: [notification({surface: 'app'})]})
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+
+  test('returns false when the current CLI version is below minVersion', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({notifications: [notification({minVersion: '999.0.0'})]})
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+
+  test('returns false when the current CLI version is above maxVersion', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({notifications: [notification({maxVersion: '0.0.1'})]})
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+
+  test('returns false when the notification window is in the past', async () => {
+    vi.mocked(fetchNotifications).mockResolvedValue({
+      notifications: [notification({minDate: '2000-01-01', maxDate: '2000-01-02'})],
+    })
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+
+  test('fails open and returns false when fetching notifications throws', async () => {
+    vi.mocked(fetchNotifications).mockRejectedValue(new Error('network down'))
+    await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false)
+  })
+})

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

@@ -1,3 +1,4 @@
+import {fetchNotifications, filterNotifications} from './notifications-system.js'
 import {isDevelopment} from './context/local.js'
 import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, getProjectDir} from './is-global.js'
 import {
@@ -112,6 +113,32 @@ export function versionToAutoUpgrade(): string | undefined {
   return newerVersion
 }
 
+/**
+ * Checks the freshly fetched notifications feed for a kill-switch notification that
+ * disables auto-upgrade. A blocking notification is one with `surface: "autoupgrade"`,
+ * `type: "error"`, and matching version/date ranges for the current CLI.
+ *
+ * Fails open: any error fetching or parsing the feed results in `false`, so a broken
+ * notifications endpoint never prevents users from auto-upgrading. Intentionally silent
+ * (no logs) — this is invoked on the auto-upgrade hot path.
+ *
+ * @returns `true` when an active blocking notification is found, `false` otherwise.
+ */
+export async function hasBlockingAutoUpgradeNotification(): Promise<boolean> {
+  try {
+    const {notifications} = await fetchNotifications()
+    // Reuse the standard notifications filtering for version/date/surface. The empty
+    // commandId disables the command filter; ['autoupgrade'] scopes to our surface.
+    // The frequency filter is a no-op here because we never render, so nothing gets
+    // written to the lastShown cache for these notifications.
+    const matching = filterNotifications(notifications, '', ['autoupgrade'])
+    return matching.some((notification) => notification.type === 'error')
+    // eslint-disable-next-line no-catch-all/no-catch-all
+  } catch {
+    return false
+  }
+}
+
 /**
  * Shows a daily upgrade-available warning for users who have not enabled auto-upgrade.
  * Skipped in CI and for pre-release versions. When auto-upgrade is enabled this is a no-op