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
3 changes: 3 additions & 0 deletions packages/cli-kit/src/private/node/analytics.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {getLastSeenAuthMethod} from './session.js'
import {getAutoUpgradeEnabled} from './conf-store.js'
import {hashString} from '../../public/node/crypto.js'
import {getPackageManager, packageManagerFromUserAgent} from '../../public/node/node-package-manager.js'
import BaseCommand from '../../public/node/base-command.js'
Expand Down Expand Up @@ -67,6 +68,7 @@ interface EnvironmentData {
env_auth_method: string
env_is_wsl: boolean
env_build_repository: string
env_auto_upgrade_enabled: boolean | null
}

export async function getEnvironmentData(config: Interfaces.Config): Promise<EnvironmentData> {
Expand All @@ -92,6 +94,7 @@ export async function getEnvironmentData(config: Interfaces.Config): Promise<Env
env_auth_method: await getLastSeenAuthMethod(),
env_is_wsl: await isWsl(),
env_build_repository: process.env.SHOPIFY_CLI_BUILD_REPO ?? 'unknown',
env_auto_upgrade_enabled: getAutoUpgradeEnabled() ?? null,
}
}

Expand Down
59 changes: 59 additions & 0 deletions packages/cli-kit/src/public/node/hooks/postrun.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import {autoUpgradeIfNeeded} from './postrun.js'
import {mockAndCaptureOutput} from '../testing/output.js'
import {getOutputUpdateCLIReminder, runCLIUpgrade, versionToAutoUpgrade} from '../upgrade.js'
import {isMajorVersionChange} from '../version.js'
import {inferPackageManagerForGlobalCLI} from '../is-global.js'
import {addPublicMetadata} from '../metadata.js'
import {describe, expect, test, vi, afterEach} from 'vitest'

vi.mock('../upgrade.js', async (importOriginal) => {
Expand All @@ -22,6 +24,22 @@ vi.mock('../version.js', async (importOriginal) => {
}
})

vi.mock('../is-global.js', async (importOriginal) => {
const actual: any = await importOriginal()
return {
...actual,
inferPackageManagerForGlobalCLI: vi.fn(),
}
})

vi.mock('../metadata.js', async (importOriginal) => {
const actual: any = await importOriginal()
return {
...actual,
addPublicMetadata: vi.fn().mockResolvedValue(undefined),
}
})

// Always execute the task so the rate limit doesn't interfere with tests
vi.mock('../../../private/node/conf-store.js', async (importOriginal) => {
const actual: any = await importOriginal()
Expand All @@ -36,6 +54,7 @@ vi.mock('../../../private/node/conf-store.js', async (importOriginal) => {

afterEach(() => {
mockAndCaptureOutput().clear()
vi.mocked(addPublicMetadata).mockClear()
})

describe('autoUpgradeIfNeeded', () => {
Expand Down Expand Up @@ -93,4 +112,44 @@ describe('autoUpgradeIfNeeded', () => {
expect(getOutputUpdateCLIReminder).toHaveBeenCalledWith('4.0.0', true)
expect(outputMock.warn()).toMatch(installReminder)
})

test('records skipped_reason for major version bump', async () => {
vi.mocked(versionToAutoUpgrade).mockReturnValue('4.0.0')
vi.mocked(isMajorVersionChange).mockReturnValue(true)
vi.mocked(getOutputUpdateCLIReminder).mockReturnValue('upgrade reminder')

await autoUpgradeIfNeeded()

const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
expect(calls).toContainEqual(
expect.objectContaining({
env_auto_upgrade_skipped_reason: 'major_version',
}),
)
})

test('records success=true on successful upgrade', async () => {
vi.mocked(versionToAutoUpgrade).mockReturnValue('3.100.0')
vi.mocked(isMajorVersionChange).mockReturnValue(false)
vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)

await autoUpgradeIfNeeded()

const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_success: true}))
})

test('records success=false on failed upgrade', async () => {
vi.mocked(versionToAutoUpgrade).mockReturnValue('3.100.0')
vi.mocked(isMajorVersionChange).mockReturnValue(false)
vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm')
vi.mocked(runCLIUpgrade).mockRejectedValue(new Error('upgrade failed'))
vi.mocked(getOutputUpdateCLIReminder).mockReturnValue('upgrade reminder')

await autoUpgradeIfNeeded()

const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_success: false}))
})
})
17 changes: 8 additions & 9 deletions packages/cli-kit/src/public/node/hooks/postrun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,7 @@ export const hook: Hook.Postrun = async ({config, Command}) => {
outputDebug(`Completed command ${command}`)
postRunHookCompleted = true

if (!Command.id?.includes('upgrade') && !Command.id?.startsWith('notifications')) {
try {
await autoUpgradeIfNeeded()
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
outputDebug(`Auto-upgrade check failed: ${error}`)
}
}
if (!command.includes('notifications') && !command.includes('upgrade')) await autoUpgradeIfNeeded()
}

/**
Expand Down Expand Up @@ -69,16 +62,22 @@ export async function autoUpgradeIfNeeded(): Promise<void> {

async function performAutoUpgrade(newerVersion: string): Promise<void> {
if (isMajorVersionChange(CLI_KIT_VERSION, newerVersion)) {
return outputWarn(getOutputUpdateCLIReminder(newerVersion, true))
outputWarn(getOutputUpdateCLIReminder(newerVersion, true))
await metadata.addPublicMetadata(() => ({
env_auto_upgrade_skipped_reason: 'major_version',
}))
return
}

try {
await runCLIUpgrade()
await metadata.addPublicMetadata(() => ({env_auto_upgrade_success: true}))
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
const errorMessage = `Auto-upgrade failed: ${error}`
outputDebug(errorMessage)
outputWarn(getOutputUpdateCLIReminder(newerVersion))
await metadata.addPublicMetadata(() => ({env_auto_upgrade_success: false}))
// Report to Observe as a handled error without showing anything extra to the user
const {sendErrorToBugsnag} = await import('../error-handler.js')
const enrichedError = Object.assign(new Error(errorMessage), {
Expand Down
5 changes: 3 additions & 2 deletions packages/cli-kit/src/public/node/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,13 @@ export function createRuntimeMetadataContainer<
}

// We want to track anything that ends up getting sent to monorail as `cmd_all_*`,
// `cmd_app_*`, `cmd_theme_*`, and `store_*`
// `cmd_app_*`, `cmd_theme_*`, `store_*`, and `env_auto_upgrade_*`
type CmdFieldsFromMonorail = PickByPrefix<MonorailEventPublic, 'cmd_all_'> &
PickByPrefix<MonorailEventPublic, 'cmd_app_'> &
PickByPrefix<MonorailEventPublic, 'cmd_create_app_'> &
PickByPrefix<MonorailEventPublic, 'cmd_theme_'> &
PickByPrefix<MonorailEventPublic, 'store_'>
PickByPrefix<MonorailEventPublic, 'store_'> &
PickByPrefix<MonorailEventPublic, 'env_auto_upgrade_'>

const coreData = createRuntimeMetadataContainer<
CmdFieldsFromMonorail,
Expand Down
8 changes: 7 additions & 1 deletion packages/cli-kit/src/public/node/monorail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const url = 'https://monorail-edge.shopifysvc.com/v1/produce'
type Optional<T> = T | null

// This is the topic name of the main event we log to Monorail, the command tracker
export const MONORAIL_COMMAND_TOPIC = 'app_cli3_command/1.20'
export const MONORAIL_COMMAND_TOPIC = 'app_cli3_command/1.21'

export interface Schemas {
[MONORAIL_COMMAND_TOPIC]: {
Expand Down Expand Up @@ -63,6 +63,12 @@ export interface Schemas {
cmd_all_timing_prompts_ms?: Optional<number>
cmd_all_timing_active_ms?: Optional<number>

// Auto-upgrade
env_auto_upgrade_enabled?: Optional<boolean>
env_auto_upgrade_accepted?: Optional<boolean>
env_auto_upgrade_skipped_reason?: Optional<string>
env_auto_upgrade_success?: Optional<boolean>

// Any extension related command
cmd_extensions_binary_from_source?: Optional<boolean>

Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/src/public/node/upgrade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {CLI_KIT_VERSION} from '../common/version.js'
* Utility function for generating an install command for the user to run
* to install an updated version of Shopify CLI.
*
* @returns A string with the command to run.
* @returns A string with the command to run, or undefined if the package manager cannot be determined.
*/
export function cliInstallCommand(): string | undefined {
const packageManager = inferPackageManagerForGlobalCLI()
Expand Down
45 changes: 40 additions & 5 deletions packages/cli/src/cli/commands/upgrade.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,45 @@
import {describe, test, vi, expect} from 'vitest'
import Upgrade from './upgrade.js'
import {promptAutoUpgrade, runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
import {addPublicMetadata} from '@shopify/cli-kit/node/metadata'
import {describe, test, vi, expect, afterEach} from 'vitest'

vi.mock('../services/upgrade.js')
vi.mock('@shopify/cli-kit/node/upgrade')
vi.mock('@shopify/cli-kit/node/metadata', () => ({
addPublicMetadata: vi.fn().mockResolvedValue(undefined),
}))

afterEach(() => {
vi.mocked(addPublicMetadata).mockClear()
})

describe('upgrade command', () => {
test('launches service with path', async () => {
// PENDING
expect(true).toBeTruthy()
test('calls promptAutoUpgrade and runCLIUpgrade', async () => {
vi.mocked(promptAutoUpgrade).mockResolvedValue(true)
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)

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

expect(promptAutoUpgrade).toHaveBeenCalledOnce()
expect(runCLIUpgrade).toHaveBeenCalledOnce()
})

test('records env_auto_upgrade_accepted=true when user opts in', async () => {
vi.mocked(promptAutoUpgrade).mockResolvedValue(true)
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)

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

const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_accepted: true}))
})

test('records env_auto_upgrade_accepted=false when user opts out', async () => {
vi.mocked(promptAutoUpgrade).mockResolvedValue(false)
vi.mocked(runCLIUpgrade).mockResolvedValue(undefined)

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

const calls = vi.mocked(addPublicMetadata).mock.calls.map((call) => call[0]())
expect(calls).toContainEqual(expect.objectContaining({env_auto_upgrade_accepted: false}))
})
})
4 changes: 3 additions & 1 deletion packages/cli/src/cli/commands/upgrade.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Command from '@shopify/cli-kit/node/base-command'
import {promptAutoUpgrade, runCLIUpgrade} from '@shopify/cli-kit/node/upgrade'
import {addPublicMetadata} from '@shopify/cli-kit/node/metadata'

export default class Upgrade extends Command {
static summary = 'Upgrades Shopify CLI.'
Expand All @@ -9,7 +10,8 @@ export default class Upgrade extends Command {
static description = this.descriptionWithoutMarkdown()

async run(): Promise<void> {
await promptAutoUpgrade()
const accepted = await promptAutoUpgrade()
await addPublicMetadata(() => ({env_auto_upgrade_accepted: accepted}))
await runCLIUpgrade()
}
}
Loading