Skip to content

Commit 52d4095

Browse files
Merge pull request #7365 from Shopify/revert-7341-autoupgrade-on-by-default
Revert "Make auto-upgrade enabled by default, remove activation prompt"
2 parents 55fade5 + 5c0a581 commit 52d4095

8 files changed

Lines changed: 71 additions & 16 deletions

File tree

packages/cli-kit/src/private/node/conf-store.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,12 +268,11 @@ export async function runWithRateLimit(options: RunWithRateLimitOptions, config
268268

269269
/**
270270
* Get auto-upgrade preference.
271-
* Defaults to true if the preference has never been explicitly set.
272271
*
273-
* @returns Whether auto-upgrade is enabled.
272+
* @returns Whether auto-upgrade is enabled, or undefined if never set.
274273
*/
275-
export function getAutoUpgradeEnabled(config: LocalStorage<ConfSchema> = cliKitStore()): boolean {
276-
return config.get('autoUpgradeEnabled') ?? true
274+
export function getAutoUpgradeEnabled(config: LocalStorage<ConfSchema> = cliKitStore()): boolean | undefined {
275+
return config.get('autoUpgradeEnabled')
277276
}
278277

279278
/**

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,11 @@ describe('versionToAutoUpgrade', () => {
230230
expect(versionToAutoUpgrade()).toBeUndefined()
231231
})
232232

233-
test('returns the newer version when auto-upgrade preference has never been set (default enabled)', () => {
233+
test('returns undefined when auto-upgrade preference has never been set', () => {
234234
vi.mocked(checkForCachedNewVersion).mockReturnValue('3.91.0')
235235
vi.mocked(isCI).mockReturnValue(false)
236-
vi.mocked(getAutoUpgradeEnabled).mockReturnValue(true)
237-
expect(versionToAutoUpgrade()).toBe('3.91.0')
236+
vi.mocked(getAutoUpgradeEnabled).mockReturnValue(undefined)
237+
expect(versionToAutoUpgrade()).toBeUndefined()
238238
})
239239

240240
test('returns the newer version for a major version change when auto-upgrade is enabled', () => {

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from './output.js'
1414
import {cwd, moduleDirectory, sniffForPath} from './path.js'
1515
import {exec, isCI} from './system.js'
16+
import {renderConfirmationPrompt} from './ui.js'
1617
import {isPreReleaseVersion} from './version.js'
1718
import {getAutoUpgradeEnabled, setAutoUpgradeEnabled, runAtMinimumInterval} from '../../private/node/conf-store.js'
1819
import {CLI_KIT_VERSION} from '../common/version.js'
@@ -80,7 +81,7 @@ export async function runCLIUpgrade(): Promise<void> {
8081

8182
/**
8283
* Returns the version to auto-upgrade to, or undefined if auto-upgrade should be skipped.
83-
* Auto-upgrade is enabled by default and can be disabled via `setAutoUpgradeEnabled(false)`.
84+
* Auto-upgrade is disabled by default and must be enabled via `shopify upgrade`.
8485
* Also skips for CI, pre-release versions, or when no newer version is available.
8586
*
8687
* @returns The version string to upgrade to, or undefined if no upgrade should happen.
@@ -153,6 +154,24 @@ export function getOutputUpdateCLIReminder(version: string, isMajor = false): st
153154
return base
154155
}
155156

157+
/**
158+
* Prompts the user to enable or disable automatic upgrades, then persists their choice.
159+
*
160+
* @returns Whether the user chose to enable auto-upgrade.
161+
*/
162+
export async function promptAutoUpgrade(): Promise<boolean> {
163+
const current = getAutoUpgradeEnabled()
164+
if (current !== undefined) return current
165+
166+
const enabled = await renderConfirmationPrompt({
167+
message: 'Enable automatic updates for Shopify CLI?',
168+
confirmationMessage: 'Yes, automatically update',
169+
cancellationMessage: "No, I'll update manually",
170+
})
171+
setAutoUpgradeEnabled(enabled)
172+
return enabled
173+
}
174+
156175
async function upgradeLocalShopify(projectDir: string, currentVersion: string) {
157176
const packageJson = (await findUpAndReadPackageJson(projectDir)).content
158177
const packageJsonDependencies = packageJson.dependencies ?? {}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export const autoUpgradeStatus = {
22
on: 'Auto-upgrade on. Shopify CLI will update automatically after each command.',
33
off: "Auto-upgrade off. You'll need to run `shopify upgrade` to update manually.",
4+
notConfigured: 'Auto-upgrade not configured. Run `shopify config autoupgrade on` to enable it.',
45
} as const

packages/cli/src/cli/commands/config/autoupgrade/status.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ describe('AutoupgradeStatus', () => {
4949
`)
5050
})
5151

52-
test('displays auto-upgrade on message when never explicitly set (default enabled)', async () => {
52+
test('displays not configured message when never set', async () => {
5353
// Given
54-
vi.mocked(getAutoUpgradeEnabled).mockReturnValue(true)
54+
vi.mocked(getAutoUpgradeEnabled).mockReturnValue(undefined)
5555
const config = new Config({root: __dirname})
5656
const outputMock = mockAndCaptureOutput()
5757
outputMock.clear()
@@ -63,7 +63,8 @@ describe('AutoupgradeStatus', () => {
6363
expect(outputMock.info()).toMatchInlineSnapshot(`
6464
"╭─ info ───────────────────────────────────────────────────────────────────────╮
6565
│ │
66-
│ Auto-upgrade on. Shopify CLI will update automatically after each command. │
66+
│ Auto-upgrade not configured. Run \`shopify config autoupgrade on\` to enable │
67+
│ it. │
6768
│ │
6869
╰──────────────────────────────────────────────────────────────────────────────╯
6970
"

packages/cli/src/cli/commands/config/autoupgrade/status.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ export default class AutoupgradeStatus extends Command {
1717

1818
async run(): Promise<void> {
1919
const enabled = getAutoUpgradeEnabled()
20-
if (enabled) {
20+
if (enabled === undefined) {
21+
renderInfo({body: autoUpgradeStatus.notConfigured})
22+
} else if (enabled) {
2123
renderInfo({body: autoUpgradeStatus.on})
2224
} else {
2325
renderInfo({body: autoUpgradeStatus.off})
Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,45 @@
11
import Upgrade from './upgrade.js'
2-
import {runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
3-
import {describe, test, vi, expect} from 'vitest'
2+
import {promptAutoUpgrade, runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
3+
import {addPublicMetadata} from '@shopify/cli-kit/node/metadata'
4+
import {describe, test, vi, expect, afterEach} from 'vitest'
45

56
vi.mock('@shopify/cli-kit/node/upgrade')
7+
vi.mock('@shopify/cli-kit/node/metadata', () => ({
8+
addPublicMetadata: vi.fn().mockResolvedValue(undefined),
9+
}))
10+
11+
afterEach(() => {
12+
vi.mocked(addPublicMetadata).mockClear()
13+
})
614

715
describe('upgrade command', () => {
8-
test('calls runCLIUpgrade directly without prompting', async () => {
16+
test('calls promptAutoUpgrade and runCLIUpgrade', async () => {
17+
vi.mocked(promptAutoUpgrade).mockResolvedValue(true)
918
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)
1019

1120
await Upgrade.run([], import.meta.url)
1221

22+
expect(promptAutoUpgrade).toHaveBeenCalledOnce()
1323
expect(runCLIUpgrade).toHaveBeenCalledOnce()
1424
})
25+
26+
test('records env_auto_upgrade_accepted=true when user opts in', async () => {
27+
vi.mocked(promptAutoUpgrade).mockResolvedValue(true)
28+
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)
29+
30+
await Upgrade.run([], import.meta.url)
31+
32+
const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
33+
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_accepted: true}))
34+
})
35+
36+
test('records env_auto_upgrade_accepted=false when user opts out', async () => {
37+
vi.mocked(promptAutoUpgrade).mockResolvedValue(false)
38+
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)
39+
40+
await Upgrade.run([], import.meta.url)
41+
42+
const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
43+
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_accepted: false}))
44+
})
1545
})

packages/cli/src/cli/commands/upgrade.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Command from '@shopify/cli-kit/node/base-command'
2-
import {runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
2+
import {promptAutoUpgrade, runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
3+
import {addPublicMetadata} from '@shopify/cli-kit/node/metadata'
34

45
export default class Upgrade extends Command {
56
static summary = 'Upgrades Shopify CLI.'
@@ -9,6 +10,8 @@ export default class Upgrade extends Command {
910
static description = this.descriptionWithoutMarkdown()
1011

1112
async run(): Promise<void> {
13+
const accepted = await promptAutoUpgrade()
14+
await addPublicMetadata(() => ({env_auto_upgrade_accepted: accepted}))
1215
await runCLIUpgrade()
1316
}
1417
}

0 commit comments

Comments
 (0)