Fix autoupgrade incorrectly skipping in development mode#7313
Fix autoupgrade incorrectly skipping in development mode#7313alfonso-noriega merged 1 commit intomainfrom
Conversation
6c59415 to
f456318
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/is-global.d.ts@@ -30,6 +30,14 @@ export declare function installGlobalCLIPrompt(): Promise<InstallGlobalCLIPrompt
* @returns The package manager used by the global CLI.
*/
export declare function inferPackageManagerForGlobalCLI(argv?: string[], env?: NodeJS.ProcessEnv): PackageManager;
+/**
+ * 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 declare function getWorkspaceRoot(directory: string): string | undefined;
/**
* Returns the project directory for the given path.
*
|
gonzaloriestra
left a comment
There was a problem hiding this comment.
What about just adding another guard clause before to check if isDevelopment()? In that case, just skip the auto-upgrade before checking if it's global or not.
This code seems a bit complex and our dev environment is a bit of a special case...
we are currently checking This fix is actually addressing a problem with But yes, we could actually just check |
|
|
Use isDevelopment() as the sole guard instead of the previous workspace-root detection. isDevelopment() checks SHOPIFY_CLI_ENV=development, which is only set by the pnpm dev scripts, so global CLI runs from the CLI folder are unaffected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
f456318 to
a92af4d
Compare
|
|
||
| // Don't auto-upgrade for development mode | ||
| if (!isGlobal && isDevelopment()) { | ||
| if (isDevelopment()) { |
There was a problem hiding this comment.
Is there no test for this which needs updated?

WHY are these changes introduced?
When running `pnpm shopify` from the CLI repo itself, autoupgrade incorrectly fires as if the CLI were globally installed. This causes contributors to see unexpected upgrade attempts during local development.
Root cause: The previous guard was `!isGlobal && isDevelopment()`, which only skipped the upgrade when both conditions were true. But `currentProcessIsGlobal()` misidentified a pnpm run from the CLI repo as global (since there's no `shopify.app.toml` to detect a local project), so the guard never triggered.
WHAT is this pull request doing?
Simplifies the guard to just `isDevelopment()`. `isDevelopment()` checks `SHOPIFY_CLI_ENV === development`, which is only set by the CLI repo's pnpm dev scripts — not by a globally-installed binary, even if run from the CLI folder. So contributors are protected and regular users are unaffected.
How to test your changes?
You can also force the autoupgrade check with:
```
SHOPIFY_CLI_FORCE_AUTO_UPGRADE=1 pnpm shopify version
```
Before this fix: upgrade runs (SHOPIFY_CLI_ENV=development is set but `!isGlobal` was false, so the guard was skipped).
After this fix: upgrade is skipped (isDevelopment() returns true).
Checklist