Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion packages/cli-kit/src/public/node/hooks/postrun.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -13,6 +18,7 @@ vi.mock('../upgrade.js', async (importOriginal) => {
runCLIUpgrade: vi.fn(),
getOutputUpdateCLIReminder: vi.fn(),
versionToAutoUpgrade: vi.fn(),
hasBlockingAutoUpgradeNotification: vi.fn().mockResolvedValue(false),
}
})

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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)
Expand Down
13 changes: 12 additions & 1 deletion packages/cli-kit/src/public/node/hooks/postrun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand All @@ -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}))
Expand Down
69 changes: 68 additions & 1 deletion packages/cli-kit/src/public/node/upgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
})
})
27 changes: 27 additions & 0 deletions packages/cli-kit/src/public/node/upgrade.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading