From 93cc4d864cfe3d5fd44ec73b723c8aebeedf0619 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Mon, 7 Sep 2026 20:39:08 +0300 Subject: [PATCH 1/2] Recover gracefully when pnpm blocks build scripts during extension generation When generating an extension in a pnpm app, recent pnpm versions fail the non-interactive dependency install with ERR_PNPM_IGNORED_BUILDS because the build-script approval prompt can't be answered from within the generation tasks. The raw install error was surfaced as-is, and if the cleanup of the partially generated extension failed, the cleanup error masked it and left files behind. Now the failure is mapped to an actionable error (run pnpm approve-builds, then generate again), and the cleanup is best-effort: a cleanup failure warns about the leftover directory instead of replacing the original error. Co-Authored-By: Claude Fable 5 Assisted-By: devx/9ab0e8f6-64e1-4036-bcd3-470ba040dcdf --- ...raceful-pnpm-blocked-builds-on-generate.md | 5 ++ .../cli/services/generate/extension.test.ts | 71 +++++++++++++++++++ .../src/cli/services/generate/extension.ts | 44 +++++++++++- 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 .changeset/graceful-pnpm-blocked-builds-on-generate.md diff --git a/.changeset/graceful-pnpm-blocked-builds-on-generate.md b/.changeset/graceful-pnpm-blocked-builds-on-generate.md new file mode 100644 index 00000000000..1d3ffcc88dc --- /dev/null +++ b/.changeset/graceful-pnpm-blocked-builds-on-generate.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Recover gracefully when pnpm blocks dependency build scripts during `app generate extension`: clean up the partially generated extension and explain how to approve the builds diff --git a/packages/app/src/cli/services/generate/extension.test.ts b/packages/app/src/cli/services/generate/extension.test.ts index 8c97436e93d..38d25d3f73c 100644 --- a/packages/app/src/cli/services/generate/extension.test.ts +++ b/packages/app/src/cli/services/generate/extension.test.ts @@ -31,6 +31,7 @@ import * as file from '@shopify/cli-kit/node/fs' import * as git from '@shopify/cli-kit/node/git' import {joinPath, dirname} from '@shopify/cli-kit/node/path' import {slugify} from '@shopify/cli-kit/common/string' +import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' vi.mock('../../models/app/validation/multi-cli-warning.js') vi.mock('@shopify/cli-kit/node/node-package-manager', async () => { @@ -459,6 +460,76 @@ describe('initialize a extension', async () => { }) }) + test('cleans up and explains how to recover when pnpm blocks dependency build scripts', async () => { + await withTemporaryApp( + async (tmpDir) => { + // Given + const name = 'my-ext-1' + vi.mocked(installNodeModules).mockRejectedValueOnce( + new Error( + 'Command failed with exit code 1: pnpm install\nERR_PNPM_IGNORED_BUILDS Ignored build scripts: esbuild@0.24.2.', + ), + ) + + // When + const got = createFromTemplate({ + name, + extensionTemplate: checkoutUITemplate, + extensionFlavor: 'vanilla-js', + appDirectory: tmpDir, + specifications, + onGetTemplateRepository, + }) + + // Then + await expect(got).rejects.toThrow( + "Your extension couldn't be generated because pnpm blocked the build scripts of some of its dependencies.", + ) + expect(file.fileExistsSync(joinPath(tmpDir, 'extensions', name))).toBeFalsy() + }, + {useWorkspaces: true}, + ) + }) + + test.skipIf(process.platform === 'win32')( + 'warns and surfaces the original error when the extension directory cleanup fails', + async () => { + await withTemporaryApp( + async (tmpDir) => { + // Given + const name = 'my-ext-1' + const extensionsDirectory = joinPath(tmpDir, 'extensions') + const outputMock = mockAndCaptureOutput() + vi.mocked(installNodeModules).mockImplementationOnce(async () => { + // Make the parent directory read-only so the cleanup can't remove the extension directory. + await file.chmod(extensionsDirectory, 0o555) + throw new Error('pnpm install failed') + }) + + try { + // When + const got = createFromTemplate({ + name, + extensionTemplate: checkoutUITemplate, + extensionFlavor: 'vanilla-js', + appDirectory: tmpDir, + specifications, + onGetTemplateRepository, + }) + + // Then + await expect(got).rejects.toThrow('pnpm install failed') + expect(outputMock.warn()).toContain("Couldn't remove") + } finally { + await file.chmod(extensionsDirectory, 0o755) + outputMock.clear() + } + }, + {useWorkspaces: true}, + ) + }, + ) + test('reloads the app after generating the extension', async () => { await withTemporaryApp(async (tmpDir) => { const downloadGitRepositorySpy = vi.spyOn(git, 'downloadGitRepository').mockResolvedValue() diff --git a/packages/app/src/cli/services/generate/extension.ts b/packages/app/src/cli/services/generate/extension.ts index ab50ff65835..5ffd899ad0c 100644 --- a/packages/app/src/cli/services/generate/extension.ts +++ b/packages/app/src/cli/services/generate/extension.ts @@ -15,12 +15,14 @@ import { readAndParsePackageJson, } from '@shopify/cli-kit/node/node-package-manager' import {recursiveLiquidTemplateCopy} from '@shopify/cli-kit/node/liquid' -import {renderTasks} from '@shopify/cli-kit/node/ui' +import {renderTasks, renderWarning} from '@shopify/cli-kit/node/ui' import {downloadGitRepository} from '@shopify/cli-kit/node/git' import {fileExists, inTemporaryDirectory, mkdir, moveFile, removeFile, glob} from '@shopify/cli-kit/node/fs' import {joinPath, relativizePath} from '@shopify/cli-kit/node/path' import {slugify} from '@shopify/cli-kit/common/string' import {nonRandomUUID} from '@shopify/cli-kit/node/crypto' +import {AbortError} from '@shopify/cli-kit/node/error' +import {formatPackageManagerCommand} from '@shopify/cli-kit/node/output' export interface GenerateExtensionTemplateOptions { app: AppLinkedInterface @@ -125,11 +127,49 @@ async function extensionInit(options: ExtensionInitOptions) { const lockFilePath = joinPath(options.directory, configurationFileNames.lockFile) await removeFile(lockFilePath) } catch (error) { - await removeFile(options.directory) + await removePartiallyGeneratedExtension(options.directory) + if (isPnpmBlockedBuildsError(error)) { + throw new AbortError( + "Your extension couldn't be generated because pnpm blocked the build scripts of some of its dependencies.", + null, + [ + ['Run', {command: 'pnpm approve-builds'}, 'in your app directory to approve the build scripts.'], + [ + 'Run', + {command: formatPackageManagerCommand(options.project.packageManager, 'shopify app generate extension')}, + 'again.', + ], + ], + ) + } throw error } } +/** + * Removes the partially generated extension directory so a failed generation leaves no files + * behind. The removal is best-effort: if it fails we warn about the leftover directory instead of + * throwing, so the error that interrupted the generation is still surfaced. + */ +async function removePartiallyGeneratedExtension(directory: string): Promise { + try { + await removeFile(directory) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + renderWarning({ + headline: ["Couldn't remove", {filePath: directory}, {char: '.'}], + body: 'Delete this directory manually before generating an extension with the same name.', + }) + } +} + +// pnpm refuses to run the build scripts of newly installed dependencies until they are approved, +// and recent pnpm versions fail the install when they can't prompt for that approval, which is +// the case while dependencies are installed from within the generation tasks. +function isPnpmBlockedBuildsError(error: unknown): boolean { + return error instanceof Error && error.message.includes('ERR_PNPM_IGNORED_BUILDS') +} + async function themeExtensionInit({ directory, url, From 46dcc144bf20c56cfe6bc9168fa4dc7a3e3d9ecd Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Tue, 8 Sep 2026 00:07:16 +0300 Subject: [PATCH 2/2] Retry transient errors when removing a failed extension generation A failed install can leave transient locks on the extension's node_modules (for example, from an antivirus scanning the freshly written files). Let removeFile pass maxRetries/retryDelay through to Node's fs.rm, which retries EBUSY/ENOTEMPTY/EPERM with linear backoff, and use that (10 retries, 100ms) when removing the partially generated extension before falling back to the leftover-directory warning. Co-Authored-By: Claude Fable 5 Assisted-By: devx/9ab0e8f6-64e1-4036-bcd3-470ba040dcdf --- .changeset/remove-file-retry-options.md | 5 ++++ .../src/cli/services/generate/extension.ts | 17 ++++++------- packages/cli-kit/src/public/node/fs.ts | 24 +++++++++++++++---- 3 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 .changeset/remove-file-retry-options.md diff --git a/.changeset/remove-file-retry-options.md b/.changeset/remove-file-retry-options.md new file mode 100644 index 00000000000..013307a51d1 --- /dev/null +++ b/.changeset/remove-file-retry-options.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Add optional `maxRetries` and `retryDelay` options to `removeFile`, passed through to Node's `fs.rm` to retry transient removal errors diff --git a/packages/app/src/cli/services/generate/extension.ts b/packages/app/src/cli/services/generate/extension.ts index 5ffd899ad0c..79d3fbbe3cb 100644 --- a/packages/app/src/cli/services/generate/extension.ts +++ b/packages/app/src/cli/services/generate/extension.ts @@ -128,7 +128,7 @@ async function extensionInit(options: ExtensionInitOptions) { await removeFile(lockFilePath) } catch (error) { await removePartiallyGeneratedExtension(options.directory) - if (isPnpmBlockedBuildsError(error)) { + if (options.project.packageManager === 'pnpm' && isPnpmBlockedBuildsError(error)) { throw new AbortError( "Your extension couldn't be generated because pnpm blocked the build scripts of some of its dependencies.", null, @@ -146,14 +146,12 @@ async function extensionInit(options: ExtensionInitOptions) { } } -/** - * Removes the partially generated extension directory so a failed generation leaves no files - * behind. The removal is best-effort: if it fails we warn about the leftover directory instead of - * throwing, so the error that interrupted the generation is still surfaced. - */ +// Retries transient removal errors, such as an antivirus lock on the freshly written files. +// If removal still fails, we warn about the leftover directory rather than throwing, so the +// original error isn't lost. async function removePartiallyGeneratedExtension(directory: string): Promise { try { - await removeFile(directory) + await removeFile(directory, {maxRetries: 10, retryDelay: 100}) // eslint-disable-next-line no-catch-all/no-catch-all } catch { renderWarning({ @@ -163,9 +161,8 @@ async function removePartiallyGeneratedExtension(directory: string): Promise { +export async function removeFile(path: string, options: RemoveFileOptions = {}): Promise { outputDebug(outputContent`Removing file at ${outputToken.path(path)}...`) - await fsRemove(path) + await fsRm(path, {recursive: true, force: true, ...options}) } /**