diff --git a/packages/cli-kit/src/private/node/analytics.ts b/packages/cli-kit/src/private/node/analytics.ts index 0306936343c..968e5a24fd7 100644 --- a/packages/cli-kit/src/private/node/analytics.ts +++ b/packages/cli-kit/src/private/node/analytics.ts @@ -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' @@ -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 { @@ -92,6 +94,7 @@ export async function getEnvironmentData(config: Interfaces.Config): Promise { @@ -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() @@ -36,6 +54,7 @@ vi.mock('../../../private/node/conf-store.js', async (importOriginal) => { afterEach(() => { mockAndCaptureOutput().clear() + vi.mocked(addPublicMetadata).mockClear() }) describe('autoUpgradeIfNeeded', () => { @@ -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})) + }) }) diff --git a/packages/cli-kit/src/public/node/hooks/postrun.ts b/packages/cli-kit/src/public/node/hooks/postrun.ts index 922848a9df5..564938e7da1 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.ts @@ -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() } /** @@ -69,16 +62,22 @@ export async function autoUpgradeIfNeeded(): Promise { async function performAutoUpgrade(newerVersion: string): Promise { 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), { diff --git a/packages/cli-kit/src/public/node/metadata.ts b/packages/cli-kit/src/public/node/metadata.ts index ae77344aa77..1b6223f7b4b 100644 --- a/packages/cli-kit/src/public/node/metadata.ts +++ b/packages/cli-kit/src/public/node/metadata.ts @@ -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 & PickByPrefix & PickByPrefix & PickByPrefix & - PickByPrefix + PickByPrefix & + PickByPrefix const coreData = createRuntimeMetadataContainer< CmdFieldsFromMonorail, diff --git a/packages/cli-kit/src/public/node/monorail.ts b/packages/cli-kit/src/public/node/monorail.ts index dc6130bc372..3cbabc52275 100644 --- a/packages/cli-kit/src/public/node/monorail.ts +++ b/packages/cli-kit/src/public/node/monorail.ts @@ -10,7 +10,7 @@ const url = 'https://monorail-edge.shopifysvc.com/v1/produce' type Optional = 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]: { @@ -63,6 +63,12 @@ export interface Schemas { cmd_all_timing_prompts_ms?: Optional cmd_all_timing_active_ms?: Optional + // Auto-upgrade + env_auto_upgrade_enabled?: Optional + env_auto_upgrade_accepted?: Optional + env_auto_upgrade_skipped_reason?: Optional + env_auto_upgrade_success?: Optional + // Any extension related command cmd_extensions_binary_from_source?: Optional diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index cd5031550f8..482d62ceff8 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -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() diff --git a/packages/cli/src/cli/commands/upgrade.test.ts b/packages/cli/src/cli/commands/upgrade.test.ts index e56a55ace44..f7a5a44d194 100644 --- a/packages/cli/src/cli/commands/upgrade.test.ts +++ b/packages/cli/src/cli/commands/upgrade.test.ts @@ -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})) }) }) diff --git a/packages/cli/src/cli/commands/upgrade.ts b/packages/cli/src/cli/commands/upgrade.ts index aeb5217ee2f..28af7f872fe 100644 --- a/packages/cli/src/cli/commands/upgrade.ts +++ b/packages/cli/src/cli/commands/upgrade.ts @@ -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.' @@ -9,7 +10,8 @@ export default class Upgrade extends Command { static description = this.descriptionWithoutMarkdown() async run(): Promise { - await promptAutoUpgrade() + const accepted = await promptAutoUpgrade() + await addPublicMetadata(() => ({env_auto_upgrade_accepted: accepted})) await runCLIUpgrade() } }