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 9de987c649f..e3ccd84585c 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.test.ts +++ b/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) diff --git a/packages/cli-kit/src/public/node/hooks/postrun.ts b/packages/cli-kit/src/public/node/hooks/postrun.ts index 3a3906a2efa..a21bf49f89b 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.ts @@ -66,7 +66,7 @@ async function performAutoUpgrade(newerVersion: string): Promise { {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 { 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})) diff --git a/packages/cli-kit/src/public/node/upgrade.test.ts b/packages/cli-kit/src/public/node/upgrade.test.ts index af82d4853de..eaa672632ce 100644 --- a/packages/cli-kit/src/public/node/upgrade.test.ts +++ b/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 { + 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) + }) +}) diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index d690324ab3b..00916066bc5 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/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 { + 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