From fdae48f7d0d68d7ef73d60316485f95cfc196737 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 22 Sep 2026 13:18:43 +0200 Subject: [PATCH 1/4] Add typed JSON output to upgrade --- .changeset/quiet-upgrades-report.md | 5 + .../generated/generated_docs_data_v2.json | 29 ++- .../cli-kit/src/public/node/upgrade.test.ts | 118 +++++++++- packages/cli-kit/src/public/node/upgrade.ts | 58 +++-- .../cli-kit/src/public/node/upgrade/output.ts | 32 +++ .../src/public/node/upgrade/result.test.ts | 62 ++++++ .../cli-kit/src/public/node/upgrade/result.ts | 20 ++ .../cli-kit/src/public/node/upgrade/types.ts | 30 +++ packages/cli/README.md | 123 ++++++++++- packages/cli/oclif.manifest.json | 27 ++- .../src/cli/commands/upgrade-output.test.ts | 209 ++++++++++++++++++ packages/cli/src/cli/commands/upgrade.test.ts | 35 ++- packages/cli/src/cli/commands/upgrade.ts | 15 +- .../rules/json-output-command-exceptions.js | 1 - 14 files changed, 733 insertions(+), 31 deletions(-) create mode 100644 .changeset/quiet-upgrades-report.md create mode 100644 packages/cli-kit/src/public/node/upgrade/output.ts create mode 100644 packages/cli-kit/src/public/node/upgrade/result.test.ts create mode 100644 packages/cli-kit/src/public/node/upgrade/result.ts create mode 100644 packages/cli-kit/src/public/node/upgrade/types.ts create mode 100644 packages/cli/src/cli/commands/upgrade-output.test.ts diff --git a/.changeset/quiet-upgrades-report.md b/.changeset/quiet-upgrades-report.md new file mode 100644 index 00000000000..28912c7d24a --- /dev/null +++ b/.changeset/quiet-upgrades-report.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli': minor +'@shopify/cli-kit': minor +--- +Add typed JSON output to `shopify upgrade`. diff --git a/docs-shopify.dev/generated/generated_docs_data_v2.json b/docs-shopify.dev/generated/generated_docs_data_v2.json index 989e46cebb3..7ace58daab6 100644 --- a/docs-shopify.dev/generated/generated_docs_data_v2.json +++ b/docs-shopify.dev/generated/generated_docs_data_v2.json @@ -10559,9 +10559,36 @@ "description": "Print the command's JSON schemas.", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_JSON_SCHEMA" + }, + { + "filePath": "docs-shopify.dev/commands/interfaces/upgrade.interface.ts", + "syntaxKind": "PropertySignature", + "name": "--no-color", + "value": "''", + "description": "Disable color output.", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_NO_COLOR" + }, + { + "filePath": "docs-shopify.dev/commands/interfaces/upgrade.interface.ts", + "syntaxKind": "PropertySignature", + "name": "--verbose", + "value": "''", + "description": "Increase the verbosity of the output. May include sensitive data.", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_VERBOSE" + }, + { + "filePath": "docs-shopify.dev/commands/interfaces/upgrade.interface.ts", + "syntaxKind": "PropertySignature", + "name": "-j, --json", + "value": "''", + "description": "Output the result as JSON. Automatically disables color output.", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_JSON" } ], - "value": "export interface upgrade {\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n}" + "value": "export interface upgrade {\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } }, "version": { diff --git a/packages/cli-kit/src/public/node/upgrade.test.ts b/packages/cli-kit/src/public/node/upgrade.test.ts index 29c3534374d..5909f769790 100644 --- a/packages/cli-kit/src/public/node/upgrade.test.ts +++ b/packages/cli-kit/src/public/node/upgrade.test.ts @@ -1,17 +1,28 @@ import {isDevelopment, isUnitTest} from './context/local.js' -import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI} from './is-global.js' -import {checkForCachedNewVersion, packageManagerFromUserAgent, PackageManager} from './node-package-manager.js' +import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, getProjectDir} from './is-global.js' +import { + checkForCachedNewVersion, + checkForNewVersion, + addNPMDependencies, + getPackageManager, + usesWorkspaces, + packageManagerFromUserAgent, + PackageManager, +} from './node-package-manager.js' import {exec, isCI} from './system.js' import { cliInstallCommand, getOutputUpdateCLIReminder, hasBlockingAutoUpgradeNotification, runCLIUpgrade, + upgradeCLI, versionToAutoUpgrade, } from './upgrade.js' import {Notification, fetchNotifications} from './notifications-system.js' import {globalCLIVersion, isPreReleaseVersion} from './version.js' import {mockAndCaptureOutput} from './testing/output.js' +import {inTemporaryDirectory, writeFile} from './fs.js' +import {joinPath} from './path.js' import {getAutoUpgradeEnabled} from '../../private/node/conf-store.js' import {CLI_KIT_VERSION} from '../common/version.js' import {SemVer} from 'semver' @@ -26,7 +37,15 @@ vi.mock('./notifications-system.js', async (importOriginal) => { }) vi.mock('./context/local.js') vi.mock('./is-global.js') -vi.mock('./node-package-manager.js') +vi.mock('./node-package-manager.js', async (importOriginal) => ({ + ...(await importOriginal()), + checkForCachedNewVersion: vi.fn(), + checkForNewVersion: vi.fn(), + addNPMDependencies: vi.fn(), + getPackageManager: vi.fn(), + usesWorkspaces: vi.fn(), + packageManagerFromUserAgent: vi.fn(), +})) vi.mock('./system.js') vi.mock('../../private/node/conf-store.js') vi.mock('./version.js', async (importOriginal) => { @@ -403,3 +422,96 @@ describe('hasBlockingAutoUpgradeNotification', () => { await expect(hasBlockingAutoUpgradeNotification()).resolves.toBe(false) }) }) + +describe('upgradeCLI result', () => { + test('returns the verified global version without presenting success', async () => { + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('pnpm') + vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) + mockAndCaptureOutput().clear() + + await expect(upgradeCLI()).resolves.toEqual({ + status: 'upgraded', + scope: 'global', + previousVersion: CLI_KIT_VERSION, + version: CLI_KIT_VERSION, + packageManager: 'pnpm', + }) + expect(mockAndCaptureOutput().info()).not.toContain('Shopify CLI upgraded.') + }) + + test('returns a development skip without installing', async () => { + vi.mocked(isDevelopment).mockReturnValue(true) + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + + await expect(upgradeCLI()).resolves.toEqual({status: 'skipped', reason: 'development', scope: 'global'}) + expect(exec).not.toHaveBeenCalled() + }) + + test('returns an automatic local upgrade skip', async () => { + vi.mocked(currentProcessIsGlobal).mockReturnValue(false) + + await expect(upgradeCLI({autoupgrade: true})).resolves.toEqual({ + status: 'skipped', + reason: 'local_autoupgrade', + scope: 'local', + }) + expect(addNPMDependencies).not.toHaveBeenCalled() + }) + + test.each([undefined, '999.0.0'])( + 'returns local dependency updates with available version %s', + async (availableVersion) => { + await inTemporaryDirectory(async (directory) => { + // In the unbundled source, the upgrade service belongs to cli-kit. + await writeFile( + joinPath(directory, 'package.json'), + JSON.stringify({ + dependencies: {'@shopify/cli-kit': '^4.0.0', unrelated: '1.0.0'}, + }), + ) + vi.mocked(currentProcessIsGlobal).mockReturnValue(false) + vi.mocked(getProjectDir).mockReturnValue(directory) + vi.mocked(checkForNewVersion).mockResolvedValue(availableVersion) + vi.mocked(getPackageManager).mockResolvedValue('npm') + vi.mocked(usesWorkspaces).mockResolvedValue(false) + + await expect(upgradeCLI()).resolves.toEqual({ + status: 'dependencies_updated', + scope: 'local', + directory, + previousVersion: CLI_KIT_VERSION, + availableVersion, + packages: ['@shopify/cli-kit'], + }) + expect(addNPMDependencies).toHaveBeenCalledExactlyOnceWith([{name: '@shopify/cli-kit', version: 'latest'}], { + directory, + type: 'prod', + packageManager: 'npm', + addToRootDirectory: false, + stdout: process.stdout, + stderr: process.stderr, + }) + expect(checkForNewVersion).toHaveBeenCalledWith('@shopify/cli-kit', CLI_KIT_VERSION) + }) + }, + ) + + test('skips a local project without a CLI dependency', async () => { + await inTemporaryDirectory(async (directory) => { + await writeFile(joinPath(directory, 'package.json'), '{}') + vi.mocked(currentProcessIsGlobal).mockReturnValue(false) + vi.mocked(getProjectDir).mockReturnValue(directory) + + await expect(upgradeCLI()).resolves.toEqual({status: 'skipped', reason: 'dependency_not_found', scope: 'local'}) + expect(addNPMDependencies).not.toHaveBeenCalled() + }) + }) + + test('preserves the failure for a missing local project', async () => { + vi.mocked(currentProcessIsGlobal).mockReturnValue(false) + vi.mocked(getProjectDir).mockReturnValue(undefined) + + await expect(upgradeCLI()).rejects.toThrow('Could not determine the local project directory') + }) +}) diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index b1fcf2055c0..0e1497710f0 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -12,14 +12,16 @@ import { getPackageManager, } from './node-package-manager.js' import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from './output.js' -import {renderSuccess} from './ui.js' +import {presentUpgradeResult} from './upgrade/result.js' +import {execUpgradeCommand, upgradeOutputStreams} from './upgrade/output.js' import {cwd, moduleDirectory, sniffForPath} from './path.js' -import {exec, isCI} from './system.js' +import {isCI} from './system.js' import {globalCLIVersion, isPreReleaseVersion} from './version.js' import {AbortError} from './error.js' import {getAutoUpgradeEnabled, setAutoUpgradeEnabled, runAtMinimumInterval} from '../../private/node/conf-store.js' import {CLI_KIT_VERSION} from '../common/version.js' import {lt as semverLt} from 'semver' +import type {UpgradeResult} from './upgrade/types.js' export {getAutoUpgradeEnabled, setAutoUpgradeEnabled} @@ -64,6 +66,16 @@ export interface RunCLIUpgradeOptions { * @throws AbortError if the package manager or command cannot be determined. */ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise { + presentUpgradeResult(await upgradeCLI(options), 'text') +} + +/** + * Upgrades the CLI and returns the outcome independently of final presentation. + * + * @param options - Whether the upgrade was triggered automatically. + * @returns The verified global version, local dependency update, or skip reason. + */ +export async function upgradeCLI(options: RunCLIUpgradeOptions = {}): Promise { // Path where the current project is (app/hydrogen) const path = sniffForPath() ?? cwd() const projectDir = getProjectDir(path) @@ -74,7 +86,7 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise // Don't auto-upgrade for development mode if (isDevelopment()) { outputInfo('Skipping upgrade in development mode.') - return + return {status: 'skipped', reason: 'development', scope: isGlobal ? 'global' : 'local'} } // When triggered by the automatic postrun hook, skip project-local upgrades. @@ -82,7 +94,7 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise // and produce noisy diffs; explicit `shopify upgrade` invocations still upgrade the // local project. if (options.autoupgrade && !isGlobal) { - return + return {status: 'skipped', reason: 'local_autoupgrade', scope: 'local'} } // Generate the install command for the global CLI and execute it @@ -105,7 +117,7 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise outputContent`${headline} Now upgrading by running: ${outputToken.genericShellCommand(installCommand)}...`, ) - await exec(command, args, {stdio: 'inherit'}) + await execUpgradeCommand(command, args) // A zero exit code doesn't guarantee the right version landed: the version check above // queries the public npm registry, while the install goes through whatever registry the @@ -126,12 +138,15 @@ export async function runCLIUpgrade(options: RunCLIUpgradeOptions = {}): Promise 'Your package manager may be resolving @shopify/cli from a registry with outdated versions. Check your npm registry configuration and try again.', ) } - renderSuccess({ - headline: 'Shopify CLI upgraded.', - body: `You're now on version ${installedVersion}.`, - }) + return { + status: 'upgraded', + scope: 'global', + previousVersion: CLI_KIT_VERSION, + version: installedVersion, + packageManager: command === 'brew' ? 'homebrew' : command, + } } else if (projectDir) { - await upgradeLocalShopify(projectDir, CLI_KIT_VERSION) + return upgradeLocalShopify(projectDir, CLI_KIT_VERSION) } else { throw new Error('Could not determine the local project directory') } @@ -238,7 +253,7 @@ export function getOutputUpdateCLIReminder(version: string, isMajor = false): st return base } -async function upgradeLocalShopify(projectDir: string, currentVersion: string) { +async function upgradeLocalShopify(projectDir: string, currentVersion: string): Promise { const packageJson = (await findUpAndReadPackageJson(projectDir)).content const packageJsonDependencies = packageJson.dependencies ?? {} const packageJsonDevDependencies = packageJson.devDependencies ?? {} @@ -247,7 +262,7 @@ async function upgradeLocalShopify(projectDir: string, currentVersion: string) { let resolvedCLIVersion = allDependencies[await cliDependency()] if (!resolvedCLIVersion) { outputDebug('Auto-upgrade: CLI dependency not found in project dependencies, skipping local upgrade.') - return + return {status: 'skipped', reason: 'dependency_not_found', scope: 'local'} } if (resolvedCLIVersion.slice(0, 1).match(/[\^~]/)) resolvedCLIVersion = currentVersion @@ -259,15 +274,24 @@ async function upgradeLocalShopify(projectDir: string, currentVersion: string) { outputWontInstallMessage(resolvedCLIVersion) } - await installJsonDependencies('prod', packageJsonDependencies, projectDir) - await installJsonDependencies('dev', packageJsonDevDependencies, projectDir) + const dependencies = await installJsonDependencies('prod', packageJsonDependencies, projectDir) + const devDependencies = await installJsonDependencies('dev', packageJsonDevDependencies, projectDir) + // Local installs are not verified, so the registry version is not an installed-version claim. + return { + status: 'dependencies_updated', + scope: 'local', + directory: projectDir, + previousVersion: resolvedCLIVersion, + availableVersion: newestCLIVersion, + packages: [...new Set([...dependencies, ...devDependencies])], + } } async function installJsonDependencies( depsEnv: DependencyType, deps: {[key: string]: string}, directory: string, -): Promise { +): Promise { const packagesToUpdate = [await cliDependency(), ...(await oclifPlugins())] .filter((pkg: string): boolean => { const pkgRequirement: string | undefined = deps[pkg] @@ -284,11 +308,11 @@ async function installJsonDependencies( packageManager: await getPackageManager(directory), type: depsEnv, directory, - stdout: process.stdout, - stderr: process.stderr, + ...upgradeOutputStreams(), addToRootDirectory: appUsesWorkspaces, }) } + return packagesToUpdate.map(({name}) => name) } async function cliDependency(): Promise { diff --git a/packages/cli-kit/src/public/node/upgrade/output.ts b/packages/cli-kit/src/public/node/upgrade/output.ts new file mode 100644 index 00000000000..e54dd2d67d2 --- /dev/null +++ b/packages/cli-kit/src/public/node/upgrade/output.ts @@ -0,0 +1,32 @@ +import {commandEventOutputMode} from '../command-events.js' +import {outputInfo} from '../output.js' +import {exec} from '../system.js' +import {Writable} from 'node:stream' + +/** + * Keeps package-manager output off the result channel in JSON mode. + * + * @returns Streams for package-manager diagnostics, or the original terminal streams. + */ +export function upgradeOutputStreams(): {stdout: Writable; stderr: Writable} { + if (commandEventOutputMode() !== 'json') return {stdout: process.stdout, stderr: process.stderr} + + const diagnostics = new Writable({ + write(chunk, _encoding, callback) { + outputInfo(Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk)) + callback() + }, + }) + return {stdout: diagnostics, stderr: diagnostics} +} + +/** + * Runs a global upgrade with terminal input and format-appropriate diagnostics. + * + * @param command - The package-manager executable. + * @param args - The install arguments. + */ +export async function execUpgradeCommand(command: string, args: string[]): Promise { + const streams = upgradeOutputStreams() + await exec(command, args, streams.stdout === process.stdout ? {stdio: 'inherit'} : {stdin: 'inherit', ...streams}) +} diff --git a/packages/cli-kit/src/public/node/upgrade/result.test.ts b/packages/cli-kit/src/public/node/upgrade/result.test.ts new file mode 100644 index 00000000000..a03a412811f --- /dev/null +++ b/packages/cli-kit/src/public/node/upgrade/result.test.ts @@ -0,0 +1,62 @@ +import {presentUpgradeResult} from './result.js' +import {upgradeJsonOutputSchema, type UpgradeResult} from './types.js' +import {mockAndCaptureOutput} from '../testing/output.js' +import {afterEach, describe, expect, test} from 'vitest' + +afterEach(() => mockAndCaptureOutput().clear()) + +const globalResult: UpgradeResult = { + status: 'upgraded', + scope: 'global', + previousVersion: '4.8.0', + version: '4.9.0', + packageManager: 'npm', +} + +const localResult: UpgradeResult = { + status: 'dependencies_updated', + scope: 'local', + directory: '/project', + previousVersion: '4.8.0', + packages: ['@shopify/cli'], +} + +describe('upgrade result contract', () => { + test.each([ + globalResult, + localResult, + {...localResult, availableVersion: '4.9.0'}, + {status: 'skipped', scope: 'global', reason: 'development'}, + {status: 'skipped', scope: 'local', reason: 'local_autoupgrade'}, + {status: 'skipped', scope: 'local', reason: 'dependency_not_found'}, + ])('encodes $status', (result) => { + presentUpgradeResult(result, 'json') + expect(JSON.parse(mockAndCaptureOutput().output())).toEqual(result) + }) + + test.each([ + {...globalResult, version: undefined}, + {...globalResult, version: 42}, + {...globalResult, scope: 'local'}, + {...localResult, packages: [42]}, + {...localResult, availableVersion: false}, + {status: 'skipped', scope: 'local', reason: 'unknown'}, + ])('rejects invalid result %j', (result) => { + expect(() => upgradeJsonOutputSchema.validate(result)).toThrow() + }) + + test('preserves the global success banner', () => { + presentUpgradeResult(globalResult, 'text') + expect(mockAndCaptureOutput().info()).toContain('Shopify CLI upgraded.') + expect(mockAndCaptureOutput().info()).toContain("You're now on version 4.9.0.") + }) + + test.each([localResult, {status: 'skipped', scope: 'local', reason: 'development'}])( + 'does not add terminal output for $status', + (result) => { + presentUpgradeResult(result, 'text') + expect(mockAndCaptureOutput().output()).toBe('') + expect(mockAndCaptureOutput().info()).toBe('') + }, + ) +}) diff --git a/packages/cli-kit/src/public/node/upgrade/result.ts b/packages/cli-kit/src/public/node/upgrade/result.ts new file mode 100644 index 00000000000..60696917975 --- /dev/null +++ b/packages/cli-kit/src/public/node/upgrade/result.ts @@ -0,0 +1,20 @@ +import {upgradeJsonOutputSchema, type UpgradeResult} from './types.js' +import {outputResult} from '../output.js' +import {renderSuccess} from '../ui.js' + +/** + * Presents the upgrade result without changing progress or package-manager output. + * + * @param result - The completed upgrade outcome. + * @param format - The output format selected by the caller. + */ +export function presentUpgradeResult(result: UpgradeResult, format: 'json' | 'text'): void { + if (format === 'json') { + outputResult(upgradeJsonOutputSchema.encode(result)) + } else if (result.status === 'upgraded') { + renderSuccess({ + headline: 'Shopify CLI upgraded.', + body: `You're now on version ${result.version}.`, + }) + } +} diff --git a/packages/cli-kit/src/public/node/upgrade/types.ts b/packages/cli-kit/src/public/node/upgrade/types.ts new file mode 100644 index 00000000000..2518634c8c1 --- /dev/null +++ b/packages/cli-kit/src/public/node/upgrade/types.ts @@ -0,0 +1,30 @@ +import {defineJsonOutputSchema, type InferJsonOutputSchema} from '../json-output-schema.js' +import {zod} from '../schema.js' + +export const upgradeJsonOutputSchema = defineJsonOutputSchema({ + name: 'UpgradeResult', + schema: zod.discriminatedUnion('status', [ + zod.object({ + status: zod.literal('upgraded'), + scope: zod.literal('global'), + previousVersion: zod.string(), + version: zod.string(), + packageManager: zod.string(), + }), + zod.object({ + status: zod.literal('dependencies_updated'), + scope: zod.literal('local'), + directory: zod.string(), + previousVersion: zod.string(), + availableVersion: zod.string().optional(), + packages: zod.array(zod.string()), + }), + zod.object({ + status: zod.literal('skipped'), + reason: zod.enum(['development', 'local_autoupgrade', 'dependency_not_found']), + scope: zod.enum(['global', 'local']), + }), + ]), +}) + +export type UpgradeResult = InferJsonOutputSchema diff --git a/packages/cli/README.md b/packages/cli/README.md index f9de295f888..a27a0084dce 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -10545,17 +10545,138 @@ Upgrades Shopify CLI. ``` USAGE - $ shopify upgrade [--json-schema] + $ shopify upgrade [-j] [--json-schema] [--no-color] [--verbose] FLAGS + -j, --json + Output the result as JSON. Automatically disables color output. + [env: SHOPIFY_FLAG_JSON] + --json-schema Print the command's JSON schemas. [env: SHOPIFY_FLAG_JSON_SCHEMA] + --no-color + Disable color output. + [env: SHOPIFY_FLAG_NO_COLOR] + + --verbose + Increase the verbosity of the output. May include sensitive data. + [env: SHOPIFY_FLAG_VERBOSE] + DESCRIPTION Upgrades Shopify CLI. Upgrades Shopify CLI using your package manager. + + Output from `--json` conforms to the `UpgradeResult` schema. + + Use `--json-schema` to print the result, error, and event schemas. + + ```json + { + "anyOf": [ + { + "type": "object", + "properties": { + "status": { + "type": "string", + "const": "upgraded" + }, + "scope": { + "type": "string", + "const": "global" + }, + "previousVersion": { + "type": "string" + }, + "version": { + "type": "string" + }, + "packageManager": { + "type": "string" + } + }, + "required": [ + "status", + "scope", + "previousVersion", + "version", + "packageManager" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "status": { + "type": "string", + "const": "dependencies_updated" + }, + "scope": { + "type": "string", + "const": "local" + }, + "directory": { + "type": "string" + }, + "previousVersion": { + "type": "string" + }, + "availableVersion": { + "type": "string" + }, + "packages": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "status", + "scope", + "directory", + "previousVersion", + "packages" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "status": { + "type": "string", + "const": "skipped" + }, + "reason": { + "type": "string", + "enum": [ + "development", + "local_autoupgrade", + "dependency_not_found" + ] + }, + "scope": { + "type": "string", + "enum": [ + "global", + "local" + ] + } + }, + "required": [ + "status", + "reason", + "scope" + ], + "additionalProperties": false + } + ], + "title": "UpgradeResult", + "$schema": "http://json-schema.org/draft-07/schema#" + } + ``` ``` ## `shopify version` diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 9575a999b11..4a5c4270cb5 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -12896,16 +12896,41 @@ ], "args": { }, - "description": "Upgrades Shopify CLI using your package manager.", + "description": "Upgrades Shopify CLI using your package manager.\n\nOutput from `--json` conforms to the `UpgradeResult` schema.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\n```json\n{\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"upgraded\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"global\"\n },\n \"previousVersion\": {\n \"type\": \"string\"\n },\n \"version\": {\n \"type\": \"string\"\n },\n \"packageManager\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"status\",\n \"scope\",\n \"previousVersion\",\n \"version\",\n \"packageManager\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"dependencies_updated\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"local\"\n },\n \"directory\": {\n \"type\": \"string\"\n },\n \"previousVersion\": {\n \"type\": \"string\"\n },\n \"availableVersion\": {\n \"type\": \"string\"\n },\n \"packages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"status\",\n \"scope\",\n \"directory\",\n \"previousVersion\",\n \"packages\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"skipped\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"development\",\n \"local_autoupgrade\",\n \"dependency_not_found\"\n ]\n },\n \"scope\": {\n \"type\": \"string\",\n \"enum\": [\n \"global\",\n \"local\"\n ]\n }\n },\n \"required\": [\n \"status\",\n \"reason\",\n \"scope\"\n ],\n \"additionalProperties\": false\n }\n ],\n \"title\": \"UpgradeResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", "descriptionWithMarkdown": "Upgrades Shopify CLI using your package manager.", "enableJsonFlag": false, "flags": { + "json": { + "allowNo": false, + "char": "j", + "description": "Output the result as JSON. Automatically disables color output.", + "env": "SHOPIFY_FLAG_JSON", + "hidden": false, + "name": "json", + "type": "boolean" + }, "json-schema": { "allowNo": false, "description": "Print the command's JSON schemas.", "env": "SHOPIFY_FLAG_JSON_SCHEMA", "name": "json-schema", "type": "boolean" + }, + "no-color": { + "allowNo": false, + "description": "Disable color output.", + "env": "SHOPIFY_FLAG_NO_COLOR", + "hidden": false, + "name": "no-color", + "type": "boolean" + }, + "verbose": { + "allowNo": false, + "description": "Increase the verbosity of the output. May include sensitive data.", + "env": "SHOPIFY_FLAG_VERBOSE", + "hidden": false, + "name": "verbose", + "type": "boolean" } }, "hasDynamicHelp": false, diff --git a/packages/cli/src/cli/commands/upgrade-output.test.ts b/packages/cli/src/cli/commands/upgrade-output.test.ts new file mode 100644 index 00000000000..98080191f13 --- /dev/null +++ b/packages/cli/src/cli/commands/upgrade-output.test.ts @@ -0,0 +1,209 @@ +import Upgrade from './upgrade.js' +import {isDevelopment} from '@shopify/cli-kit/node/context/local' +import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, getProjectDir} from '@shopify/cli-kit/node/is-global' +import {exec} from '@shopify/cli-kit/node/system' +import {globalCLIVersion} from '@shopify/cli-kit/node/version' +import { + checkForCachedNewVersion, + checkForNewVersion, + addNPMDependencies, + getPackageManager, +} from '@shopify/cli-kit/node/node-package-manager' +import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {ExternalError} from '@shopify/cli-kit/node/error' +import {upgradeJsonOutputSchema} from '@shopify/cli-kit/node/upgrade/types' +import {commandEventOutputSchema} from '@shopify/cli-kit/node/command-events' +import {afterEach, expect, onTestFinished, test, vi} from 'vitest' +// Vitest intercepts console.warn; exercise the real stderr writer. +// eslint-disable-next-line n/prefer-global/console +import {Console} from 'node:console' +import type {Writable} from 'node:stream' + +vi.mock('@shopify/cli-kit/node/context/local', async (importOriginal) => ({ + ...(await importOriginal()), + isDevelopment: vi.fn(() => false), +})) +vi.mock('@shopify/cli-kit/node/is-global') +vi.mock('@shopify/cli-kit/node/system', async (importOriginal) => ({ + ...(await importOriginal()), + exec: vi.fn(), +})) +vi.mock('@shopify/cli-kit/node/version', async (importOriginal) => ({ + ...(await importOriginal()), + globalCLIVersion: vi.fn(), +})) +vi.mock('@shopify/cli-kit/node/node-package-manager', async (importOriginal) => ({ + ...(await importOriginal()), + checkForCachedNewVersion: vi.fn(), + checkForNewVersion: vi.fn(), + addNPMDependencies: vi.fn(), + getPackageManager: vi.fn(), +})) + +function captureStreams() { + const stdout: string[] = [] + const stderr: string[] = [] + vi.stubEnv('SHOPIFY_UNIT_TEST', 'false') + const stdoutSpy = vi + .spyOn(process.stdout, 'write') + .mockImplementation( + ( + chunk, + encoding?: BufferEncoding | ((error?: Error | null) => void), + callback?: (error?: Error | null) => void, + ) => { + stdout.push(String(chunk)) + if (typeof encoding === 'function') encoding() + else callback?.() + return true + }, + ) + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation( + ( + chunk, + encoding?: BufferEncoding | ((error?: Error | null) => void), + callback?: (error?: Error | null) => void, + ) => { + stderr.push(String(chunk)) + if (typeof encoding === 'function') encoding() + else callback?.() + return true + }, + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(new Console(process.stdout, process.stderr).warn) + onTestFinished(() => { + stdoutSpy.mockRestore() + stderrSpy.mockRestore() + warnSpy.mockRestore() + }) + return {stdout: () => stdout.join(''), stderr: () => stderr.join('')} +} + +afterEach(() => { + vi.unstubAllEnvs() + process.exitCode = 0 +}) + +test('writes one global JSON result and package-manager diagnostics as stderr events', async () => { + const streams = captureStreams() + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') + vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) + vi.mocked(exec).mockImplementation(async (_command, _args, options) => { + expect(options?.stdin).toBe('inherit') + ;(options?.stdout as Writable).write('installed packages\n') + ;(options?.stderr as Writable).write('package manager warning\n') + }) + + await Upgrade.run(['--json'], import.meta.url) + + const expected = { + status: 'upgraded', + scope: 'global', + previousVersion: CLI_KIT_VERSION, + version: CLI_KIT_VERSION, + packageManager: 'npm', + } + expect(streams.stdout()).toBe(`${JSON.stringify(expected, null, 2)}\n`) + expect(upgradeJsonOutputSchema.validate(JSON.parse(streams.stdout()))).toEqual(expected) + const events = streams + .stderr() + .trim() + .split('\n') + .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) + expect(events).toEqual( + expect.arrayContaining([ + expect.objectContaining({type: 'diagnostic', message: 'installed packages\n'}), + expect.objectContaining({type: 'diagnostic', message: 'package manager warning\n'}), + ]), + ) + expect(streams.stderr()).not.toContain('Shopify CLI upgraded.') +}) + +test('writes one local JSON result and routes dependency installation output through events', async () => { + await inTemporaryDirectory(async (directory) => { + await writeFile( + joinPath(directory, 'package.json'), + JSON.stringify({devDependencies: {'@shopify/cli-kit': '4.0.0'}}), + ) + const streams = captureStreams() + vi.mocked(currentProcessIsGlobal).mockReturnValue(false) + vi.mocked(getProjectDir).mockReturnValue(directory) + vi.mocked(checkForNewVersion).mockResolvedValue('4.9.0') + vi.mocked(getPackageManager).mockResolvedValue('npm') + vi.mocked(addNPMDependencies).mockImplementation(async (_dependencies, options) => { + options.stdout?.write('updated dependency\n') + options.stderr?.write('dependency warning\n') + }) + + await Upgrade.run(['--json'], import.meta.url) + + expect(JSON.parse(streams.stdout())).toEqual({ + status: 'dependencies_updated', + scope: 'local', + directory, + previousVersion: '4.0.0', + availableVersion: '4.9.0', + packages: ['@shopify/cli-kit'], + }) + expect(addNPMDependencies).toHaveBeenCalledWith( + [{name: '@shopify/cli-kit', version: 'latest'}], + expect.objectContaining({type: 'dev', directory}), + ) + const events = streams + .stderr() + .trim() + .split('\n') + .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) + expect(events).toEqual( + expect.arrayContaining([ + expect.objectContaining({type: 'diagnostic', message: 'updated dependency\n'}), + expect.objectContaining({type: 'diagnostic', message: 'dependency warning\n'}), + ]), + ) + }) +}) + +test('keeps development skip guidance on stderr and returns its reason on stdout', async () => { + const streams = captureStreams() + vi.mocked(isDevelopment).mockReturnValue(true) + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + + await Upgrade.run(['--json'], import.meta.url) + + expect(JSON.parse(streams.stdout())).toEqual({status: 'skipped', reason: 'development', scope: 'global'}) + expect(JSON.parse(streams.stderr())).toMatchObject({ + type: 'diagnostic', + message: 'Skipping upgrade in development mode.', + }) + expect(exec).not.toHaveBeenCalled() +}) + +test.each(['install', 'verification'] as const)('preserves fatal JSON errors for %s failures', async (failure) => { + const streams = captureStreams() + // The shared fatal-error path inspects the process environment/argv, as it does in a real CLI run. + vi.stubEnv('SHOPIFY_FLAG_JSON', '1') + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('exit') + }) + onTestFinished(() => exitSpy.mockRestore()) + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') + vi.mocked(checkForCachedNewVersion).mockReturnValue(CLI_KIT_VERSION) + vi.mocked(globalCLIVersion).mockResolvedValue(undefined) + if (failure === 'install') { + vi.mocked(exec).mockRejectedValue(new ExternalError('install failed', 'npm', ['install'])) + } + + await expect(Upgrade.run(['--json'], import.meta.url)).rejects.toThrow() + + const result = JSON.parse(streams.stdout()) + expect(exitSpy).toHaveBeenCalledWith(1) + expect(result.error.type).toBe(failure === 'install' ? 'external' : 'abort') + expect(result).not.toHaveProperty('status') + expect(streams.stdout()).toContain(failure === 'install' ? 'install failed' : "Couldn't verify") +}) diff --git a/packages/cli/src/cli/commands/upgrade.test.ts b/packages/cli/src/cli/commands/upgrade.test.ts index 231d94df2c7..d84d8047c8b 100644 --- a/packages/cli/src/cli/commands/upgrade.test.ts +++ b/packages/cli/src/cli/commands/upgrade.test.ts @@ -1,15 +1,40 @@ import Upgrade from './upgrade.js' -import {runCLIUpgrade} from '@shopify/cli-kit/node/upgrade' -import {describe, test, vi, expect} from 'vitest' +import {upgradeCLI} from '@shopify/cli-kit/node/upgrade' +import {upgradeJsonOutputSchema} from '@shopify/cli-kit/node/upgrade/types' +import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' +import {afterEach, describe, test, vi, expect} from 'vitest' vi.mock('@shopify/cli-kit/node/upgrade') +afterEach(() => mockAndCaptureOutput().clear()) + describe('upgrade command', () => { - test('calls runCLIUpgrade directly without prompting', async () => { - vi.mocked(runCLIUpgrade).mockResolvedValue(undefined) + test('exposes the result schema and JSON flags in help', () => { + expect(Upgrade.jsonOutputSchema).toBe(upgradeJsonOutputSchema) + expect(Upgrade.flags.json).toBeDefined() + expect(Upgrade.description).toContain('Output from `--json` conforms to the `UpgradeResult` schema.') + }) + + test('encodes the result when JSON is requested', async () => { + const result = { + status: 'upgraded', + scope: 'global', + previousVersion: '4.8.0', + version: '4.9.0', + packageManager: 'npm', + } as const + vi.mocked(upgradeCLI).mockResolvedValue(result) + + await Upgrade.run(['--json'], import.meta.url) + + expect(mockAndCaptureOutput().output()).toBe(upgradeJsonOutputSchema.encode(result)) + }) + + test('calls upgradeCLI directly without prompting', async () => { + vi.mocked(upgradeCLI).mockResolvedValue({status: 'skipped', reason: 'development', scope: 'global'}) await Upgrade.run([], import.meta.url) - expect(runCLIUpgrade).toHaveBeenCalledOnce() + expect(upgradeCLI).toHaveBeenCalledOnce() }) }) diff --git a/packages/cli/src/cli/commands/upgrade.ts b/packages/cli/src/cli/commands/upgrade.ts index 3107f4730e1..02d8edbd3e9 100644 --- a/packages/cli/src/cli/commands/upgrade.ts +++ b/packages/cli/src/cli/commands/upgrade.ts @@ -1,5 +1,8 @@ import Command from '@shopify/cli-kit/node/base-command' -import {runCLIUpgrade} from '@shopify/cli-kit/node/upgrade' +import {upgradeCLI} from '@shopify/cli-kit/node/upgrade' +import {presentUpgradeResult} from '@shopify/cli-kit/node/upgrade/result' +import {upgradeJsonOutputSchema} from '@shopify/cli-kit/node/upgrade/types' +import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' export default class Upgrade extends Command { static summary = 'Upgrades Shopify CLI.' @@ -8,7 +11,15 @@ export default class Upgrade extends Command { static description = this.descriptionForHelp() + static flags = {...globalFlags, ...jsonFlag} + + static get jsonOutputSchema() { + return upgradeJsonOutputSchema + } + async run(): Promise { - await runCLIUpgrade() + const {flags} = await this.parse(Upgrade) + const result = await upgradeCLI() + presentUpgradeResult(result, flags.json ? 'json' : 'text') } } diff --git a/packages/eslint-plugin-cli/rules/json-output-command-exceptions.js b/packages/eslint-plugin-cli/rules/json-output-command-exceptions.js index 3ab142dc901..b6e49b7527e 100644 --- a/packages/eslint-plugin-cli/rules/json-output-command-exceptions.js +++ b/packages/eslint-plugin-cli/rules/json-output-command-exceptions.js @@ -28,7 +28,6 @@ const commandExceptions = [ 'packages/app/src/cli/commands/app/subscription-migrations/status.ts', 'packages/app/src/cli/commands/app/subscription-migrations/unschedule.ts', 'packages/app/src/cli/commands/app/webhook/trigger.ts', - 'packages/cli/src/cli/commands/upgrade.ts', 'packages/plugin-did-you-mean/src/commands/config/autocorrect/off.ts', 'packages/plugin-did-you-mean/src/commands/config/autocorrect/on.ts', 'packages/plugin-did-you-mean/src/commands/config/autocorrect/status.ts', From a6a8495663e3e61c967b095881876f9d4c07d8a6 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 6 Oct 2026 14:12:22 +0200 Subject: [PATCH 2/4] Align upgrade JSON with the shared conventions --- .../generated/generated_docs_data_v2.json | 11 ++- .../cli-kit/src/public/node/upgrade.test.ts | 12 +-- packages/cli-kit/src/public/node/upgrade.ts | 17 +++-- .../src/public/node/upgrade/result.test.ts | 18 ++++- .../cli-kit/src/public/node/upgrade/result.ts | 2 +- .../cli-kit/src/public/node/upgrade/types.ts | 55 ++++++++------ packages/cli/README.md | 61 ++++++++++++---- packages/cli/oclif.manifest.json | 9 ++- .../src/cli/commands/upgrade-output.test.ts | 73 ++++++++++--------- packages/cli/src/cli/commands/upgrade.test.ts | 3 +- 10 files changed, 173 insertions(+), 88 deletions(-) diff --git a/docs-shopify.dev/generated/generated_docs_data_v2.json b/docs-shopify.dev/generated/generated_docs_data_v2.json index 7ace58daab6..720acfe8b25 100644 --- a/docs-shopify.dev/generated/generated_docs_data_v2.json +++ b/docs-shopify.dev/generated/generated_docs_data_v2.json @@ -10569,6 +10569,15 @@ "isOptional": true, "environmentValue": "SHOPIFY_FLAG_NO_COLOR" }, + { + "filePath": "docs-shopify.dev/commands/interfaces/upgrade.interface.ts", + "syntaxKind": "PropertySignature", + "name": "--no-input", + "value": "''", + "description": "Disable interactive prompts and browser authentication.", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_NO_INPUT" + }, { "filePath": "docs-shopify.dev/commands/interfaces/upgrade.interface.ts", "syntaxKind": "PropertySignature", @@ -10588,7 +10597,7 @@ "environmentValue": "SHOPIFY_FLAG_JSON" } ], - "value": "export interface upgrade {\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface upgrade {\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } }, "version": { diff --git a/packages/cli-kit/src/public/node/upgrade.test.ts b/packages/cli-kit/src/public/node/upgrade.test.ts index 5909f769790..edd7cad4ddf 100644 --- a/packages/cli-kit/src/public/node/upgrade.test.ts +++ b/packages/cli-kit/src/public/node/upgrade.test.ts @@ -431,7 +431,8 @@ describe('upgradeCLI result', () => { mockAndCaptureOutput().clear() await expect(upgradeCLI()).resolves.toEqual({ - status: 'upgraded', + status: 'success', + changed: false, scope: 'global', previousVersion: CLI_KIT_VERSION, version: CLI_KIT_VERSION, @@ -453,7 +454,7 @@ describe('upgradeCLI result', () => { await expect(upgradeCLI({autoupgrade: true})).resolves.toEqual({ status: 'skipped', - reason: 'local_autoupgrade', + reason: 'local-autoupgrade', scope: 'local', }) expect(addNPMDependencies).not.toHaveBeenCalled() @@ -477,11 +478,12 @@ describe('upgradeCLI result', () => { vi.mocked(usesWorkspaces).mockResolvedValue(false) await expect(upgradeCLI()).resolves.toEqual({ - status: 'dependencies_updated', + status: 'success', + changed: null, scope: 'local', directory, previousVersion: CLI_KIT_VERSION, - availableVersion, + availableVersion: availableVersion ?? null, packages: ['@shopify/cli-kit'], }) expect(addNPMDependencies).toHaveBeenCalledExactlyOnceWith([{name: '@shopify/cli-kit', version: 'latest'}], { @@ -503,7 +505,7 @@ describe('upgradeCLI result', () => { vi.mocked(currentProcessIsGlobal).mockReturnValue(false) vi.mocked(getProjectDir).mockReturnValue(directory) - await expect(upgradeCLI()).resolves.toEqual({status: 'skipped', reason: 'dependency_not_found', scope: 'local'}) + await expect(upgradeCLI()).resolves.toEqual({status: 'skipped', reason: 'dependency-not-found', scope: 'local'}) expect(addNPMDependencies).not.toHaveBeenCalled() }) }) diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index 0e1497710f0..d89d1530637 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -94,13 +94,14 @@ export async function upgradeCLI(options: RunCLIUpgradeOptions = {}): Promise mockAndCaptureOutput().clear()) const globalResult: UpgradeResult = { - status: 'upgraded', + status: 'success', + changed: true, scope: 'global', previousVersion: '4.8.0', version: '4.9.0', @@ -14,10 +15,12 @@ const globalResult: UpgradeResult = { } const localResult: UpgradeResult = { - status: 'dependencies_updated', + status: 'success', + changed: null, scope: 'local', directory: '/project', previousVersion: '4.8.0', + availableVersion: null, packages: ['@shopify/cli'], } @@ -27,8 +30,8 @@ describe('upgrade result contract', () => { localResult, {...localResult, availableVersion: '4.9.0'}, {status: 'skipped', scope: 'global', reason: 'development'}, - {status: 'skipped', scope: 'local', reason: 'local_autoupgrade'}, - {status: 'skipped', scope: 'local', reason: 'dependency_not_found'}, + {status: 'skipped', scope: 'local', reason: 'local-autoupgrade'}, + {status: 'skipped', scope: 'local', reason: 'dependency-not-found'}, ])('encodes $status', (result) => { presentUpgradeResult(result, 'json') expect(JSON.parse(mockAndCaptureOutput().output())).toEqual(result) @@ -40,6 +43,13 @@ describe('upgrade result contract', () => { {...globalResult, scope: 'local'}, {...localResult, packages: [42]}, {...localResult, availableVersion: false}, + {...localResult, availableVersion: undefined}, + {...localResult, directory: 'relative/project'}, + {...localResult, changed: false}, + {...globalResult, internalValue: true}, + {...globalResult, packageManager: 'unknown'}, + {...globalResult, version: ''}, + {...globalResult, changed: undefined}, {status: 'skipped', scope: 'local', reason: 'unknown'}, ])('rejects invalid result %j', (result) => { expect(() => upgradeJsonOutputSchema.validate(result)).toThrow() diff --git a/packages/cli-kit/src/public/node/upgrade/result.ts b/packages/cli-kit/src/public/node/upgrade/result.ts index 60696917975..d45b819bfd4 100644 --- a/packages/cli-kit/src/public/node/upgrade/result.ts +++ b/packages/cli-kit/src/public/node/upgrade/result.ts @@ -11,7 +11,7 @@ import {renderSuccess} from '../ui.js' export function presentUpgradeResult(result: UpgradeResult, format: 'json' | 'text'): void { if (format === 'json') { outputResult(upgradeJsonOutputSchema.encode(result)) - } else if (result.status === 'upgraded') { + } else if (result.status === 'success' && result.scope === 'global') { renderSuccess({ headline: 'Shopify CLI upgraded.', body: `You're now on version ${result.version}.`, diff --git a/packages/cli-kit/src/public/node/upgrade/types.ts b/packages/cli-kit/src/public/node/upgrade/types.ts index 2518634c8c1..a78801caefc 100644 --- a/packages/cli-kit/src/public/node/upgrade/types.ts +++ b/packages/cli-kit/src/public/node/upgrade/types.ts @@ -1,29 +1,42 @@ import {defineJsonOutputSchema, type InferJsonOutputSchema} from '../json-output-schema.js' +import {isAbsolutePath} from '../path.js' import {zod} from '../schema.js' export const upgradeJsonOutputSchema = defineJsonOutputSchema({ name: 'UpgradeResult', - schema: zod.discriminatedUnion('status', [ - zod.object({ - status: zod.literal('upgraded'), - scope: zod.literal('global'), - previousVersion: zod.string(), - version: zod.string(), - packageManager: zod.string(), - }), - zod.object({ - status: zod.literal('dependencies_updated'), - scope: zod.literal('local'), - directory: zod.string(), - previousVersion: zod.string(), - availableVersion: zod.string().optional(), - packages: zod.array(zod.string()), - }), - zod.object({ - status: zod.literal('skipped'), - reason: zod.enum(['development', 'local_autoupgrade', 'dependency_not_found']), - scope: zod.enum(['global', 'local']), - }), + schema: zod.union([ + zod + .object({ + status: zod.literal('success'), + changed: zod.boolean().describe('Whether the verified installed version differs from previousVersion.'), + scope: zod.literal('global'), + previousVersion: zod.string().min(1), + version: zod.string().min(1), + packageManager: zod.enum(['npm', 'pnpm', 'yarn', 'bun', 'homebrew']), + }) + .strict(), + zod + .object({ + status: zod.literal('success'), + changed: zod.null().describe('Null because local dependency changes and installed versions are not verified.'), + scope: zod.literal('local'), + directory: zod.string().refine(isAbsolutePath, 'Must be an absolute filesystem path.'), + previousVersion: zod.string().min(1), + availableVersion: zod + .string() + .min(1) + .nullable() + .describe('The available registry version, or null when unknown.'), + packages: zod.array(zod.string().min(1)), + }) + .strict(), + zod + .object({ + status: zod.literal('skipped'), + reason: zod.enum(['development', 'local-autoupgrade', 'dependency-not-found']), + scope: zod.enum(['global', 'local']), + }) + .strict(), ]), }) diff --git a/packages/cli/README.md b/packages/cli/README.md index a27a0084dce..4a49c024dd0 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -10545,7 +10545,7 @@ Upgrades Shopify CLI. ``` USAGE - $ shopify upgrade [-j] [--json-schema] [--no-color] [--verbose] + $ shopify upgrade [-j] [--json-schema] [--no-color] [--no-input] [--verbose] FLAGS -j, --json @@ -10560,6 +10560,10 @@ FLAGS Disable color output. [env: SHOPIFY_FLAG_NO_COLOR] + --no-input + Disable interactive prompts and browser authentication. + [env: SHOPIFY_FLAG_NO_INPUT] + --verbose Increase the verbosity of the output. May include sensitive data. [env: SHOPIFY_FLAG_VERBOSE] @@ -10569,10 +10573,10 @@ DESCRIPTION Upgrades Shopify CLI using your package manager. - Output from `--json` conforms to the `UpgradeResult` schema. - Use `--json-schema` to print the result, error, and event schemas. + Output from `--json` conforms to the `UpgradeResult` schema. + ```json { "anyOf": [ @@ -10581,24 +10585,38 @@ DESCRIPTION "properties": { "status": { "type": "string", - "const": "upgraded" + "const": "success" + }, + "changed": { + "type": "boolean", + "description": "Whether the verified installed version differs from previousVersion." }, "scope": { "type": "string", "const": "global" }, "previousVersion": { - "type": "string" + "type": "string", + "minLength": 1 }, "version": { - "type": "string" + "type": "string", + "minLength": 1 }, "packageManager": { - "type": "string" + "type": "string", + "enum": [ + "npm", + "pnpm", + "yarn", + "bun", + "homebrew" + ] } }, "required": [ "status", + "changed", "scope", "previousVersion", "version", @@ -10611,7 +10629,11 @@ DESCRIPTION "properties": { "status": { "type": "string", - "const": "dependencies_updated" + "const": "success" + }, + "changed": { + "type": "null", + "description": "Null because local dependency changes and installed versions are not verified." }, "scope": { "type": "string", @@ -10621,23 +10643,36 @@ DESCRIPTION "type": "string" }, "previousVersion": { - "type": "string" + "type": "string", + "minLength": 1 }, "availableVersion": { - "type": "string" + "anyOf": [ + { + "type": "string", + "minLength": 1 + }, + { + "type": "null" + } + ], + "description": "The available registry version, or null when unknown." }, "packages": { "type": "array", "items": { - "type": "string" + "type": "string", + "minLength": 1 } } }, "required": [ "status", + "changed", "scope", "directory", "previousVersion", + "availableVersion", "packages" ], "additionalProperties": false @@ -10653,8 +10688,8 @@ DESCRIPTION "type": "string", "enum": [ "development", - "local_autoupgrade", - "dependency_not_found" + "local-autoupgrade", + "dependency-not-found" ] }, "scope": { diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 4a5c4270cb5..b320fc349b1 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -12896,7 +12896,7 @@ ], "args": { }, - "description": "Upgrades Shopify CLI using your package manager.\n\nOutput from `--json` conforms to the `UpgradeResult` schema.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\n```json\n{\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"upgraded\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"global\"\n },\n \"previousVersion\": {\n \"type\": \"string\"\n },\n \"version\": {\n \"type\": \"string\"\n },\n \"packageManager\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"status\",\n \"scope\",\n \"previousVersion\",\n \"version\",\n \"packageManager\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"dependencies_updated\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"local\"\n },\n \"directory\": {\n \"type\": \"string\"\n },\n \"previousVersion\": {\n \"type\": \"string\"\n },\n \"availableVersion\": {\n \"type\": \"string\"\n },\n \"packages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"status\",\n \"scope\",\n \"directory\",\n \"previousVersion\",\n \"packages\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"skipped\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"development\",\n \"local_autoupgrade\",\n \"dependency_not_found\"\n ]\n },\n \"scope\": {\n \"type\": \"string\",\n \"enum\": [\n \"global\",\n \"local\"\n ]\n }\n },\n \"required\": [\n \"status\",\n \"reason\",\n \"scope\"\n ],\n \"additionalProperties\": false\n }\n ],\n \"title\": \"UpgradeResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", + "description": "Upgrades Shopify CLI using your package manager.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `UpgradeResult` schema.\n\n```json\n{\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"success\"\n },\n \"changed\": {\n \"type\": \"boolean\",\n \"description\": \"Whether the verified installed version differs from previousVersion.\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"global\"\n },\n \"previousVersion\": {\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"version\": {\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"packageManager\": {\n \"type\": \"string\",\n \"enum\": [\n \"npm\",\n \"pnpm\",\n \"yarn\",\n \"bun\",\n \"homebrew\"\n ]\n }\n },\n \"required\": [\n \"status\",\n \"changed\",\n \"scope\",\n \"previousVersion\",\n \"version\",\n \"packageManager\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"success\"\n },\n \"changed\": {\n \"type\": \"null\",\n \"description\": \"Null because local dependency changes and installed versions are not verified.\"\n },\n \"scope\": {\n \"type\": \"string\",\n \"const\": \"local\"\n },\n \"directory\": {\n \"type\": \"string\"\n },\n \"previousVersion\": {\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"availableVersion\": {\n \"anyOf\": [\n {\n \"type\": \"string\",\n \"minLength\": 1\n },\n {\n \"type\": \"null\"\n }\n ],\n \"description\": \"The available registry version, or null when unknown.\"\n },\n \"packages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\",\n \"minLength\": 1\n }\n }\n },\n \"required\": [\n \"status\",\n \"changed\",\n \"scope\",\n \"directory\",\n \"previousVersion\",\n \"availableVersion\",\n \"packages\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"skipped\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"development\",\n \"local-autoupgrade\",\n \"dependency-not-found\"\n ]\n },\n \"scope\": {\n \"type\": \"string\",\n \"enum\": [\n \"global\",\n \"local\"\n ]\n }\n },\n \"required\": [\n \"status\",\n \"reason\",\n \"scope\"\n ],\n \"additionalProperties\": false\n }\n ],\n \"title\": \"UpgradeResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", "descriptionWithMarkdown": "Upgrades Shopify CLI using your package manager.", "enableJsonFlag": false, "flags": { @@ -12924,6 +12924,13 @@ "name": "no-color", "type": "boolean" }, + "no-input": { + "allowNo": false, + "description": "Disable interactive prompts and browser authentication.", + "env": "SHOPIFY_FLAG_NO_INPUT", + "name": "no-input", + "type": "boolean" + }, "verbose": { "allowNo": false, "description": "Increase the verbosity of the output. May include sensitive data.", diff --git a/packages/cli/src/cli/commands/upgrade-output.test.ts b/packages/cli/src/cli/commands/upgrade-output.test.ts index 98080191f13..182c45fb88f 100644 --- a/packages/cli/src/cli/commands/upgrade-output.test.ts +++ b/packages/cli/src/cli/commands/upgrade-output.test.ts @@ -88,41 +88,45 @@ afterEach(() => { process.exitCode = 0 }) -test('writes one global JSON result and package-manager diagnostics as stderr events', async () => { - const streams = captureStreams() - vi.mocked(currentProcessIsGlobal).mockReturnValue(true) - vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') - vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) - vi.mocked(exec).mockImplementation(async (_command, _args, options) => { - expect(options?.stdin).toBe('inherit') - ;(options?.stdout as Writable).write('installed packages\n') - ;(options?.stderr as Writable).write('package manager warning\n') - }) +test.each([{argv: ['--json']}, {argv: ['--json', '--no-input']}])( + 'writes one global JSON result and package-manager diagnostics as stderr events for %j', + async ({argv}) => { + const streams = captureStreams() + vi.mocked(currentProcessIsGlobal).mockReturnValue(true) + vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') + vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) + vi.mocked(exec).mockImplementation(async (_command, _args, options) => { + expect(options?.stdin).toBe('inherit') + ;(options?.stdout as Writable).write('installed packages\n') + ;(options?.stderr as Writable).write('package manager warning\n') + }) - await Upgrade.run(['--json'], import.meta.url) + await Upgrade.run(argv, import.meta.url) - const expected = { - status: 'upgraded', - scope: 'global', - previousVersion: CLI_KIT_VERSION, - version: CLI_KIT_VERSION, - packageManager: 'npm', - } - expect(streams.stdout()).toBe(`${JSON.stringify(expected, null, 2)}\n`) - expect(upgradeJsonOutputSchema.validate(JSON.parse(streams.stdout()))).toEqual(expected) - const events = streams - .stderr() - .trim() - .split('\n') - .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) - expect(events).toEqual( - expect.arrayContaining([ - expect.objectContaining({type: 'diagnostic', message: 'installed packages\n'}), - expect.objectContaining({type: 'diagnostic', message: 'package manager warning\n'}), - ]), - ) - expect(streams.stderr()).not.toContain('Shopify CLI upgraded.') -}) + const expected = { + status: 'success', + changed: false, + scope: 'global', + previousVersion: CLI_KIT_VERSION, + version: CLI_KIT_VERSION, + packageManager: 'npm', + } + expect(streams.stdout()).toBe(`${JSON.stringify(expected, null, 2)}\n`) + expect(upgradeJsonOutputSchema.validate(JSON.parse(streams.stdout()))).toEqual(expected) + const events = streams + .stderr() + .trim() + .split('\n') + .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) + expect(events).toEqual( + expect.arrayContaining([ + expect.objectContaining({type: 'diagnostic', message: 'installed packages\n'}), + expect.objectContaining({type: 'diagnostic', message: 'package manager warning\n'}), + ]), + ) + expect(streams.stderr()).not.toContain('Shopify CLI upgraded.') + }, +) test('writes one local JSON result and routes dependency installation output through events', async () => { await inTemporaryDirectory(async (directory) => { @@ -143,7 +147,8 @@ test('writes one local JSON result and routes dependency installation output thr await Upgrade.run(['--json'], import.meta.url) expect(JSON.parse(streams.stdout())).toEqual({ - status: 'dependencies_updated', + status: 'success', + changed: null, scope: 'local', directory, previousVersion: '4.0.0', diff --git a/packages/cli/src/cli/commands/upgrade.test.ts b/packages/cli/src/cli/commands/upgrade.test.ts index d84d8047c8b..b7a79f1f950 100644 --- a/packages/cli/src/cli/commands/upgrade.test.ts +++ b/packages/cli/src/cli/commands/upgrade.test.ts @@ -17,7 +17,8 @@ describe('upgrade command', () => { test('encodes the result when JSON is requested', async () => { const result = { - status: 'upgraded', + status: 'success', + changed: true, scope: 'global', previousVersion: '4.8.0', version: '4.9.0', From 35da153023aa933eccf016fee066b2b7593e11c9 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 6 Oct 2026 15:53:29 +0200 Subject: [PATCH 3/4] Encode upgrade directories as native filesystem paths --- packages/cli-kit/src/public/node/upgrade/result.test.ts | 9 ++++++++- packages/cli-kit/src/public/node/upgrade/types.ts | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/upgrade/result.test.ts b/packages/cli-kit/src/public/node/upgrade/result.test.ts index 9ed80efb5b0..f8350b297eb 100644 --- a/packages/cli-kit/src/public/node/upgrade/result.test.ts +++ b/packages/cli-kit/src/public/node/upgrade/result.test.ts @@ -2,6 +2,8 @@ import {presentUpgradeResult} from './result.js' import {upgradeJsonOutputSchema, type UpgradeResult} from './types.js' import {mockAndCaptureOutput} from '../testing/output.js' import {afterEach, describe, expect, test} from 'vitest' +// eslint-disable-next-line no-restricted-imports -- Verify native filesystem paths in JSON output. +import {resolve} from 'node:path' afterEach(() => mockAndCaptureOutput().clear()) @@ -18,13 +20,18 @@ const localResult: UpgradeResult = { status: 'success', changed: null, scope: 'local', - directory: '/project', + directory: resolve('/project'), previousVersion: '4.8.0', availableVersion: null, packages: ['@shopify/cli'], } describe('upgrade result contract', () => { + test('encodes local project directories with native filesystem separators', () => { + presentUpgradeResult({...localResult, directory: '/project/nested/..'}, 'json') + expect(JSON.parse(mockAndCaptureOutput().output()).directory).toBe(resolve('/project')) + }) + test.each([ globalResult, localResult, diff --git a/packages/cli-kit/src/public/node/upgrade/types.ts b/packages/cli-kit/src/public/node/upgrade/types.ts index a78801caefc..42faf6be02d 100644 --- a/packages/cli-kit/src/public/node/upgrade/types.ts +++ b/packages/cli-kit/src/public/node/upgrade/types.ts @@ -1,6 +1,8 @@ import {defineJsonOutputSchema, type InferJsonOutputSchema} from '../json-output-schema.js' import {isAbsolutePath} from '../path.js' import {zod} from '../schema.js' +// eslint-disable-next-line no-restricted-imports -- JSON filesystem paths use native platform separators. +import {resolve} from 'node:path' export const upgradeJsonOutputSchema = defineJsonOutputSchema({ name: 'UpgradeResult', @@ -20,7 +22,10 @@ export const upgradeJsonOutputSchema = defineJsonOutputSchema({ status: zod.literal('success'), changed: zod.null().describe('Null because local dependency changes and installed versions are not verified.'), scope: zod.literal('local'), - directory: zod.string().refine(isAbsolutePath, 'Must be an absolute filesystem path.'), + directory: zod + .string() + .refine(isAbsolutePath, 'Must be an absolute filesystem path.') + .transform((directory) => resolve(directory)), previousVersion: zod.string().min(1), availableVersion: zod .string() From 99214b716d7f08b97af116a4d312288ab0651d08 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 6 Oct 2026 16:36:59 +0200 Subject: [PATCH 4/4] Fix upgrade JSON path assertion on Windows --- packages/cli-kit/src/public/node/upgrade.ts | 21 +++-- .../src/public/node/upgrade/output.test.ts | 80 +++++++++++++++++++ .../cli-kit/src/public/node/upgrade/output.ts | 24 +++++- .../src/cli/commands/upgrade-output.test.ts | 73 ++++++++--------- packages/cli/src/cli/commands/upgrade.ts | 8 +- 5 files changed, 150 insertions(+), 56 deletions(-) create mode 100644 packages/cli-kit/src/public/node/upgrade/output.test.ts diff --git a/packages/cli-kit/src/public/node/upgrade.ts b/packages/cli-kit/src/public/node/upgrade.ts index d89d1530637..56b00d23fe7 100644 --- a/packages/cli-kit/src/public/node/upgrade.ts +++ b/packages/cli-kit/src/public/node/upgrade.ts @@ -307,13 +307,20 @@ async function installJsonDependencies( const appUsesWorkspaces = await usesWorkspaces(directory) if (packagesToUpdate.length > 0) { - await addNPMDependencies(packagesToUpdate, { - packageManager: await getPackageManager(directory), - type: depsEnv, - directory, - ...upgradeOutputStreams(), - addToRootDirectory: appUsesWorkspaces, - }) + const packageManager = await getPackageManager(directory) + const streams = upgradeOutputStreams() + try { + await addNPMDependencies(packagesToUpdate, { + packageManager, + type: depsEnv, + directory, + ...streams, + addToRootDirectory: appUsesWorkspaces, + }) + } finally { + if (streams.stdout !== process.stdout) streams.stdout.end() + if (streams.stderr !== process.stderr) streams.stderr.end() + } } return packagesToUpdate.map(({name}) => name) } diff --git a/packages/cli-kit/src/public/node/upgrade/output.test.ts b/packages/cli-kit/src/public/node/upgrade/output.test.ts new file mode 100644 index 00000000000..a46a6e63898 --- /dev/null +++ b/packages/cli-kit/src/public/node/upgrade/output.test.ts @@ -0,0 +1,80 @@ +import {execUpgradeCommand, upgradeOutputStreams} from './output.js' +import {commandEventOutputSchema, renderCommandEventAsJson, runWithCommandEvents} from '../command-events.js' +import {withCapturedStandardStreams} from '../testing/output.js' +import * as system from '../system.js' +import {expect, test, vi} from 'vitest' + +test('uses the original streams for terminal output', () => { + expect(upgradeOutputStreams()).toEqual({stdout: process.stdout, stderr: process.stderr}) +}) + +test('decodes interleaved UTF-8 chunks independently for stdout and stderr', async () => { + await withCapturedStandardStreams(({stdout, stderr}) => { + runWithCommandEvents({outputMode: 'json', sink: renderCommandEventAsJson}, () => { + const streams = upgradeOutputStreams() + const stdoutBytes = Buffer.from('café') + const stderrBytes = Buffer.from('🛍') + + streams.stdout.write(stdoutBytes.subarray(0, 4)) + streams.stderr.write(stderrBytes.subarray(0, 2)) + streams.stdout.write(stdoutBytes.subarray(4)) + streams.stderr.write(stderrBytes.subarray(2)) + streams.stdout.end() + streams.stderr.end() + }) + + const events = stderr() + .trim() + .split('\n') + .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) + expect(events).toEqual([ + expect.objectContaining({type: 'diagnostic', message: 'caf'}), + expect.objectContaining({type: 'diagnostic', message: 'é'}), + expect.objectContaining({type: 'diagnostic', message: '🛍'}), + ]) + expect(stdout()).toBe('') + }) +}) + +test.each(['success', 'failure'])('flushes both decoders after a global upgrade %s', async (outcome) => { + const error = new Error('Installation failed') + const exec = vi.spyOn(system, 'exec').mockImplementation(async (_command, _args, options) => { + if (!options?.stdout || options.stdout === 'inherit' || !options.stderr || options.stderr === 'inherit') { + throw new Error('Expected diagnostic streams') + } + options.stdout.write(Buffer.from([0xc3])) + options.stderr.write(Buffer.from([0xf0, 0x9f])) + if (outcome === 'failure') throw error + }) + + await withCapturedStandardStreams(async ({stdout, stderr}) => { + await runWithCommandEvents({outputMode: 'json', sink: renderCommandEventAsJson}, async () => { + const upgrade = execUpgradeCommand('npm', ['install']) + if (outcome === 'failure') await expect(upgrade).rejects.toBe(error) + else await upgrade + }) + + const events = stderr() + .trim() + .split('\n') + .map((line) => commandEventOutputSchema.validate(JSON.parse(line))) + expect(events).toEqual([ + expect.objectContaining({type: 'diagnostic', message: '�'}), + expect.objectContaining({type: 'diagnostic', message: '�'}), + ]) + expect(stdout()).toBe('') + expect(exec).toHaveBeenCalledExactlyOnceWith('npm', ['install'], expect.objectContaining({stdin: 'inherit'})) + }) +}) + +test('preserves inherited terminal streams after a global upgrade', async () => { + const exec = vi.spyOn(system, 'exec').mockResolvedValue(undefined) + const stdoutEnd = vi.spyOn(process.stdout, 'end') + const stderrEnd = vi.spyOn(process.stderr, 'end') + + await execUpgradeCommand('npm', ['install']) + + expect(exec).toHaveBeenCalledExactlyOnceWith('npm', ['install'], {stdio: 'inherit'}) + expect(stdoutEnd).not.toHaveBeenCalled() + expect(stderrEnd).not.toHaveBeenCalled() +}) diff --git a/packages/cli-kit/src/public/node/upgrade/output.ts b/packages/cli-kit/src/public/node/upgrade/output.ts index e54dd2d67d2..a9223880590 100644 --- a/packages/cli-kit/src/public/node/upgrade/output.ts +++ b/packages/cli-kit/src/public/node/upgrade/output.ts @@ -2,6 +2,7 @@ import {commandEventOutputMode} from '../command-events.js' import {outputInfo} from '../output.js' import {exec} from '../system.js' import {Writable} from 'node:stream' +import {StringDecoder} from 'node:string_decoder' /** * Keeps package-manager output off the result channel in JSON mode. @@ -11,13 +12,23 @@ import {Writable} from 'node:stream' export function upgradeOutputStreams(): {stdout: Writable; stderr: Writable} { if (commandEventOutputMode() !== 'json') return {stdout: process.stdout, stderr: process.stderr} - const diagnostics = new Writable({ + return {stdout: diagnosticStream(), stderr: diagnosticStream()} +} + +function diagnosticStream(): Writable { + const decoder = new StringDecoder('utf8') + return new Writable({ write(chunk, _encoding, callback) { - outputInfo(Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk)) + const message = decoder.write(chunk) + if (message) outputInfo(message) + callback() + }, + final(callback) { + const message = decoder.end() + if (message) outputInfo(message) callback() }, }) - return {stdout: diagnostics, stderr: diagnostics} } /** @@ -28,5 +39,10 @@ export function upgradeOutputStreams(): {stdout: Writable; stderr: Writable} { */ export async function execUpgradeCommand(command: string, args: string[]): Promise { const streams = upgradeOutputStreams() - await exec(command, args, streams.stdout === process.stdout ? {stdio: 'inherit'} : {stdin: 'inherit', ...streams}) + try { + await exec(command, args, streams.stdout === process.stdout ? {stdio: 'inherit'} : {stdin: 'inherit', ...streams}) + } finally { + if (streams.stdout !== process.stdout) streams.stdout.end() + if (streams.stderr !== process.stderr) streams.stderr.end() + } } diff --git a/packages/cli/src/cli/commands/upgrade-output.test.ts b/packages/cli/src/cli/commands/upgrade-output.test.ts index 182c45fb88f..4d4c8ae47a7 100644 --- a/packages/cli/src/cli/commands/upgrade-output.test.ts +++ b/packages/cli/src/cli/commands/upgrade-output.test.ts @@ -1,46 +1,34 @@ import Upgrade from './upgrade.js' -import {isDevelopment} from '@shopify/cli-kit/node/context/local' +import * as localContext from '@shopify/cli-kit/node/context/local' import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, getProjectDir} from '@shopify/cli-kit/node/is-global' -import {exec} from '@shopify/cli-kit/node/system' -import {globalCLIVersion} from '@shopify/cli-kit/node/version' -import { - checkForCachedNewVersion, - checkForNewVersion, - addNPMDependencies, - getPackageManager, -} from '@shopify/cli-kit/node/node-package-manager' +import * as system from '@shopify/cli-kit/node/system' +import * as version from '@shopify/cli-kit/node/version' +import * as nodePackageManager from '@shopify/cli-kit/node/node-package-manager' import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {ExternalError} from '@shopify/cli-kit/node/error' import {upgradeJsonOutputSchema} from '@shopify/cli-kit/node/upgrade/types' import {commandEventOutputSchema} from '@shopify/cli-kit/node/command-events' -import {afterEach, expect, onTestFinished, test, vi} from 'vitest' +import {beforeEach, afterEach, expect, onTestFinished, test, vi} from 'vitest' // Vitest intercepts console.warn; exercise the real stderr writer. // eslint-disable-next-line n/prefer-global/console import {Console} from 'node:console' +// eslint-disable-next-line no-restricted-imports -- Verify native filesystem paths in JSON output. +import {resolve} from 'node:path' import type {Writable} from 'node:stream' -vi.mock('@shopify/cli-kit/node/context/local', async (importOriginal) => ({ - ...(await importOriginal()), - isDevelopment: vi.fn(() => false), -})) vi.mock('@shopify/cli-kit/node/is-global') -vi.mock('@shopify/cli-kit/node/system', async (importOriginal) => ({ - ...(await importOriginal()), - exec: vi.fn(), -})) -vi.mock('@shopify/cli-kit/node/version', async (importOriginal) => ({ - ...(await importOriginal()), - globalCLIVersion: vi.fn(), -})) -vi.mock('@shopify/cli-kit/node/node-package-manager', async (importOriginal) => ({ - ...(await importOriginal()), - checkForCachedNewVersion: vi.fn(), - checkForNewVersion: vi.fn(), - addNPMDependencies: vi.fn(), - getPackageManager: vi.fn(), -})) + +beforeEach(() => { + vi.spyOn(localContext, 'isDevelopment').mockReturnValue(false) + vi.spyOn(system, 'exec').mockResolvedValue(undefined) + vi.spyOn(version, 'globalCLIVersion').mockResolvedValue(undefined) + vi.spyOn(nodePackageManager, 'checkForCachedNewVersion').mockReturnValue(undefined) + vi.spyOn(nodePackageManager, 'checkForNewVersion').mockResolvedValue(undefined) + vi.spyOn(nodePackageManager, 'addNPMDependencies').mockResolvedValue(undefined) + vi.spyOn(nodePackageManager, 'getPackageManager').mockResolvedValue('npm') +}) function captureStreams() { const stdout: string[] = [] @@ -94,8 +82,8 @@ test.each([{argv: ['--json']}, {argv: ['--json', '--no-input']}])( const streams = captureStreams() vi.mocked(currentProcessIsGlobal).mockReturnValue(true) vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') - vi.mocked(globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) - vi.mocked(exec).mockImplementation(async (_command, _args, options) => { + vi.mocked(version.globalCLIVersion).mockResolvedValue(CLI_KIT_VERSION) + vi.mocked(system.exec).mockImplementation(async (_command, _args, options) => { expect(options?.stdin).toBe('inherit') ;(options?.stdout as Writable).write('installed packages\n') ;(options?.stderr as Writable).write('package manager warning\n') @@ -137,11 +125,13 @@ test('writes one local JSON result and routes dependency installation output thr const streams = captureStreams() vi.mocked(currentProcessIsGlobal).mockReturnValue(false) vi.mocked(getProjectDir).mockReturnValue(directory) - vi.mocked(checkForNewVersion).mockResolvedValue('4.9.0') - vi.mocked(getPackageManager).mockResolvedValue('npm') - vi.mocked(addNPMDependencies).mockImplementation(async (_dependencies, options) => { + vi.mocked(nodePackageManager.checkForNewVersion).mockResolvedValue('4.9.0') + vi.mocked(nodePackageManager.getPackageManager).mockResolvedValue('npm') + vi.mocked(nodePackageManager.addNPMDependencies).mockImplementation(async (_dependencies, options) => { options.stdout?.write('updated dependency\n') options.stderr?.write('dependency warning\n') + options.stdout?.write(Buffer.from([0xc3])) + options.stderr?.write(Buffer.from([0xf0, 0x9f])) }) await Upgrade.run(['--json'], import.meta.url) @@ -150,12 +140,12 @@ test('writes one local JSON result and routes dependency installation output thr status: 'success', changed: null, scope: 'local', - directory, + directory: resolve(directory), previousVersion: '4.0.0', availableVersion: '4.9.0', packages: ['@shopify/cli-kit'], }) - expect(addNPMDependencies).toHaveBeenCalledWith( + expect(nodePackageManager.addNPMDependencies).toHaveBeenCalledWith( [{name: '@shopify/cli-kit', version: 'latest'}], expect.objectContaining({type: 'dev', directory}), ) @@ -170,12 +160,13 @@ test('writes one local JSON result and routes dependency installation output thr expect.objectContaining({type: 'diagnostic', message: 'dependency warning\n'}), ]), ) + expect(events.filter((event) => event.type === 'diagnostic' && event.message === '�')).toHaveLength(2) }) }) test('keeps development skip guidance on stderr and returns its reason on stdout', async () => { const streams = captureStreams() - vi.mocked(isDevelopment).mockReturnValue(true) + vi.mocked(localContext.isDevelopment).mockReturnValue(true) vi.mocked(currentProcessIsGlobal).mockReturnValue(true) await Upgrade.run(['--json'], import.meta.url) @@ -185,7 +176,7 @@ test('keeps development skip guidance on stderr and returns its reason on stdout type: 'diagnostic', message: 'Skipping upgrade in development mode.', }) - expect(exec).not.toHaveBeenCalled() + expect(system.exec).not.toHaveBeenCalled() }) test.each(['install', 'verification'] as const)('preserves fatal JSON errors for %s failures', async (failure) => { @@ -198,10 +189,10 @@ test.each(['install', 'verification'] as const)('preserves fatal JSON errors for onTestFinished(() => exitSpy.mockRestore()) vi.mocked(currentProcessIsGlobal).mockReturnValue(true) vi.mocked(inferPackageManagerForGlobalCLI).mockReturnValue('npm') - vi.mocked(checkForCachedNewVersion).mockReturnValue(CLI_KIT_VERSION) - vi.mocked(globalCLIVersion).mockResolvedValue(undefined) + vi.mocked(nodePackageManager.checkForCachedNewVersion).mockReturnValue(CLI_KIT_VERSION) + vi.mocked(version.globalCLIVersion).mockResolvedValue(undefined) if (failure === 'install') { - vi.mocked(exec).mockRejectedValue(new ExternalError('install failed', 'npm', ['install'])) + vi.mocked(system.exec).mockRejectedValue(new ExternalError('install failed', 'npm', ['install'])) } await expect(Upgrade.run(['--json'], import.meta.url)).rejects.toThrow() diff --git a/packages/cli/src/cli/commands/upgrade.ts b/packages/cli/src/cli/commands/upgrade.ts index 02d8edbd3e9..c21a78d1ca6 100644 --- a/packages/cli/src/cli/commands/upgrade.ts +++ b/packages/cli/src/cli/commands/upgrade.ts @@ -9,14 +9,14 @@ export default class Upgrade extends Command { static descriptionWithMarkdown = 'Upgrades Shopify CLI using your package manager.' - static description = this.descriptionForHelp() - - static flags = {...globalFlags, ...jsonFlag} - static get jsonOutputSchema() { return upgradeJsonOutputSchema } + static description = this.descriptionForHelp() + + static flags = {...globalFlags, ...jsonFlag} + async run(): Promise { const {flags} = await this.parse(Upgrade) const result = await upgradeCLI()