Skip to content

Fix Windows auto-upgrade for local CLI installs#7553

Merged
2 commits merged into
mainfrom
fonso/fix-windows-autoupgrade-local-dep
May 15, 2026
Merged

Fix Windows auto-upgrade for local CLI installs#7553
2 commits merged into
mainfrom
fonso/fix-windows-autoupgrade-local-dep

Conversation

@alfonso-noriega
Copy link
Copy Markdown
Contributor

@alfonso-noriega alfonso-noriega commented May 15, 2026

WHY are these changes introduced?

Reported by Nick Wesselman in #shopify-cli: on Windows, running shopify app dev (or any command) in a project where @shopify/cli is a project-local dependency still triggers the global auto-upgrade flow — it runs npm install -g @shopify/cli@latest after every command. Confirmed working correctly on macOS + Linux.

Root-cause analysis by River:

currentProcessIsGlobal() in packages/cli-kit/src/public/node/is-global.ts decides between global and local installs by comparing the project root against process.argv[1]:

const isLocal = binDir.startsWith(projectDir.trim())
  • projectDir flows through getProjectDirfindPathUpSyncnormalizePath (pathe). Pathe always normalizes to forward slashes on every platformC:/Users/me/proj.
  • process.argv[1] arrives in OS-native form. On Windows that's backslash-separated → C:\Users\me\proj\node_modules\@shopify\cli\bin\run.js.
  • binDir.startsWith(projectDir)false.
  • The local install is classified as global → runCLIUpgrade() takes the global branch → npm install -g @shopify/cli@latest.

macOS + Linux are unaffected because their native argv[1] already uses /.

WHAT is this pull request doing?

Two-line behavioural fix + a regression test:

  • packages/cli-kit/src/public/node/is-global.ts — replace the raw startsWith with isSubpath(projectDir, binDir), which goes through pathe.relative and tolerates either separator. Also bail early if argv[1] is empty so we don't accidentally classify "no script path" as "inside any directory".

  • packages/cli-kit/src/public/node/is-global.test.ts — new test that constructs the exact Windows shape (projectDir = 'C:/Users/me/project', argv[1] = 'C:\\Users\\me\\project\\…'). The existing POSIX-shaped tests happened to agree on slashes when CI runs on Linux, which is why this slipped through.

  • .changeset/fix-windows-autoupgrade-local-dep.md — patch on @shopify/cli-kit.

Scenario OLD (startsWith) NEW (isSubpath) Expected
Windows local dep ❌ false ✅ true local
macOS local dep ✅ true ✅ true local
POSIX global ✅ false ✅ false global
Windows global ✅ false ✅ false global

How to test your changes?

cd packages/cli-kit
pnpm vitest run src/public/node/is-global.test.ts   # 18/18 ✓
pnpm type-check
pnpm lint

End-to-end, on Windows, in a project that has @shopify/cli declared in package.json:

$env:SHOPIFY_CLI_FORCE_AUTO_UPGRADE=1; npm run shopify -- version

Before: a global npm install -g @shopify/cli@latest runs after the command.
After: no global install runs — the project-local code path is taken (which, for now, also short-circuits per the auto-upgrade-skip-local change).

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows) — fix is specifically cross-platform; new test pins Windows shape
  • I've considered possible documentation changes — N/A
  • I've considered analytics changes to measure impact — none required
  • The change is user-facing — patch-level changeset added on @shopify/cli-kit

Credit: bug reported by @nickwesselman; root-cause + suggested fix by River.

`currentProcessIsGlobal()` decides between global and local installs by
comparing the project root against `process.argv[1]`. The project root
flows through `getProjectDir` -> `findPathUpSync` -> `normalizePath` (pathe),
which always returns forward slashes on every platform. `process.argv[1]`
is OS-native, so on Windows it arrives backslash-separated and the raw
`startsWith` comparison ends up as:

  binDir:     C:\\Users\\me\\proj\\node_modules\\@Shopify\\cli\\bin\\run.js
  projectDir: C:/Users/me/proj
  binDir.startsWith(projectDir) => false

The local install was misclassified as global, so the postrun hook ran
`npm install -g @shopify/cli@latest` after every command on Windows even
when @shopify/cli was declared as a project dependency.

Switches the comparison to `isSubpath` (pathe.relative under the hood)
which tolerates either separator, and bails early if argv[1] is empty so
we don't accidentally classify it as 'in any directory'.

Adds a regression test that constructs the exact Windows shape (forward-
slash projectDir vs. backslash argv[1]) — the existing POSIX-shaped tests
happened to agree on slashes when CI runs on Linux, so they didn't catch
this.

Reported by Nick Wesselman; root cause analysis by River.
@alfonso-noriega alfonso-noriega requested review from a team as code owners May 15, 2026 08:40
@github-actions github-actions Bot added the Area: @shopify/cli @shopify/cli package issues label May 15, 2026
@github-actions github-actions Bot added no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. and removed Area: @shopify/cli @shopify/cli package issues labels May 15, 2026
@alfonso-noriega
Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions
Copy link
Copy Markdown
Contributor

🫰✨ Thanks @alfonso-noriega! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20260515092709

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@github-merge-queue github-merge-queue Bot closed this pull request by merging all changes into main in dd4e5c1 May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant