From f456318073d852df57b3595d741ec2928123d0cb Mon Sep 17 00:00:00 2001 From: Alfonso Noriega Date: Wed, 15 Apr 2026 18:34:02 +0200 Subject: [PATCH] Fix autoupgrade incorrectly treating pnpm shopify (from CLI repo) as global When running `pnpm shopify` from the CLI repo, getProjectDir returns undefined because there is no shopify.app.toml or hydrogen.config.js in the repo. The previous code immediately returned true (global) in that case, causing autoupgrade to fire as if the CLI were globally installed. Fix: when no app/hydrogen project is found, fall back to checking whether the binary (argv[1]) is inside a pnpm workspace root (detected via pnpm-workspace.yaml). If it is, the process is local and autoupgrade is skipped. --- .../cli-kit/src/public/node/is-global.test.ts | 60 ++++++++++++++++++- packages/cli-kit/src/public/node/is-global.ts | 37 ++++++++++-- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/packages/cli-kit/src/public/node/is-global.test.ts b/packages/cli-kit/src/public/node/is-global.test.ts index 4e2e0b91e9e..0d89fba5c26 100644 --- a/packages/cli-kit/src/public/node/is-global.test.ts +++ b/packages/cli-kit/src/public/node/is-global.test.ts @@ -1,4 +1,9 @@ -import {currentProcessIsGlobal, inferPackageManagerForGlobalCLI, installGlobalCLIPrompt} from './is-global.js' +import { + currentProcessIsGlobal, + inferPackageManagerForGlobalCLI, + installGlobalCLIPrompt, + getWorkspaceRoot, +} from './is-global.js' import {findPathUpSync} from './fs.js' import {cwd} from './path.js' import {terminalSupportsPrompting} from './system.js' @@ -12,14 +17,16 @@ vi.mock('./ui.js') vi.mock('which') vi.mock('./version.js') -// Mock fs.js to make findPathUpSync controllable for getProjectDir. +// Mock fs.js to make findPathUpSync and fileExistsSync controllable. // find-up v6 runs returned paths through locatePathSync which checks file existence, // so we need to mock findPathUpSync directly rather than globSync. +// fileExistsSync is used by getWorkspaceRoot when searching for pnpm-workspace.yaml. vi.mock('./fs.js', async (importOriginal) => { const actual = await importOriginal() return { ...actual, findPathUpSync: vi.fn((...args: Parameters) => actual.findPathUpSync(...args)), + fileExistsSync: vi.fn((...args: Parameters) => actual.fileExistsSync(...args)), } }) @@ -81,6 +88,55 @@ describe('currentProcessIsGlobal', () => { // Then expect(got).toBeFalsy() }) + + test('returns false when no app project found but binary is within a workspace (pnpm shopify from CLI repo)', () => { + // Simulate running `pnpm shopify` from the CLI repo itself: + // - No shopify.app.toml anywhere up the tree (getProjectDir returns undefined) + // - A pnpm-workspace.yaml exists at the repo root (getWorkspaceRoot finds it) + // - The binary lives within the repo (it's a local workspace package) + vi.mocked(findPathUpSync) + // getProjectDir: no shopify.app.toml found + .mockReturnValueOnce(undefined) + // getWorkspaceRoot: found workspace root + .mockReturnValueOnce(`${cwd()}/pnpm-workspace.yaml`) + const argv = ['node', `${cwd()}/packages/cli/bin/shopify.js`, 'shopify'] + + const got = currentProcessIsGlobal(argv) + + expect(got).toBe(false) + }) + + test('returns true when no app project and no workspace found', () => { + // Simulate a global install in a plain directory with no project context + vi.mocked(findPathUpSync) + // getProjectDir: no shopify.app.toml + .mockReturnValueOnce(undefined) + // getWorkspaceRoot: no pnpm-workspace.yaml + .mockReturnValueOnce(undefined) + const argv = ['node', globalNPMPath, 'shopify'] + + const got = currentProcessIsGlobal(argv) + + expect(got).toBe(true) + }) +}) + +describe('getWorkspaceRoot', () => { + test('returns workspace root when pnpm-workspace.yaml is found', () => { + vi.mocked(findPathUpSync).mockReturnValueOnce(`${cwd()}/pnpm-workspace.yaml`) + + const got = getWorkspaceRoot(cwd()) + + expect(got).toBe(cwd()) + }) + + test('returns undefined when no pnpm-workspace.yaml is found', () => { + vi.mocked(findPathUpSync).mockReturnValueOnce(undefined) + + const got = getWorkspaceRoot('/some/random/directory') + + expect(got).toBeUndefined() + }) }) describe('inferPackageManagerForGlobalCLI', () => { diff --git a/packages/cli-kit/src/public/node/is-global.ts b/packages/cli-kit/src/public/node/is-global.ts index 825d9e5952b..8365b58193e 100644 --- a/packages/cli-kit/src/public/node/is-global.ts +++ b/packages/cli-kit/src/public/node/is-global.ts @@ -5,7 +5,7 @@ import {exec, terminalSupportsPrompting} from './system.js' import {renderSelectPrompt} from './ui.js' import {globalCLIVersion} from './version.js' import {isUnitTest} from './context/local.js' -import {findPathUpSync, globSync} from './fs.js' +import {fileExistsSync, findPathUpSync, globSync} from './fs.js' import {realpathSync} from 'fs' let _isGlobal: boolean | undefined @@ -25,13 +25,19 @@ export function currentProcessIsGlobal(argv = process.argv): boolean { const path = sniffForPath() ?? cwd() const projectDir = getProjectDir(path) - if (!projectDir) { - return true - } // From node docs: "The second element [of the array] will be the path to the JavaScript file being executed" const binDir = argv[1] ?? '' + if (!projectDir) { + // No app/hydrogen project found. Fall back to checking if the binary is + // within a workspace (monorepo) root — e.g. running `pnpm shopify` from + // the CLI repo itself, which has no shopify.app.toml. + const workspaceRoot = getWorkspaceRoot(cwd()) + _isGlobal = !(workspaceRoot && binDir.startsWith(workspaceRoot)) + return _isGlobal + } + // If binDir starts with projectDir, then we are running a local CLI const isLocal = binDir.startsWith(projectDir.trim()) @@ -116,6 +122,29 @@ export function inferPackageManagerForGlobalCLI(argv = process.argv, env = proce return 'npm' } +/** + * Returns the workspace (monorepo) root for the given path by searching upward + * for a pnpm-workspace.yaml file. + * + * @param directory - The path to search upward from. + * @returns The workspace root directory, or undefined if not in a workspace. + */ +export function getWorkspaceRoot(directory: string): string | undefined { + try { + const found = findPathUpSync( + (dir) => { + const yamlPath = joinPath(dir, 'pnpm-workspace.yaml') + return fileExistsSync(yamlPath) ? yamlPath : undefined + }, + {cwd: directory, type: 'file'}, + ) + return found ? dirname(found) : undefined + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + return undefined + } +} + /** * Returns the project directory for the given path. *