Recover gracefully when pnpm blocks build scripts during extension generation

.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

.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

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()

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,46 @@ 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 (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,
+        [
+          ['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
   }
 }
 
+// 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<void> {
+  try {
+    await removeFile(directory, {maxRetries: 10, retryDelay: 100})
+    // 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 can't prompt to approve build scripts during a non-interactive install like this one,
+// so recent versions fail the install outright instead.
+function isPnpmBlockedBuildsError(error: unknown): boolean {
+  return error instanceof Error && error.message.includes('ERR_PNPM_IGNORED_BUILDS')
+}
+
 async function themeExtensionInit({
   directory,
   url,

packages/cli-kit/src/public/node/fs.ts

@@ -7,7 +7,6 @@ import {
   copy as fsCopy,
   ensureFile as fsEnsureFile,
   ensureFileSync as fsEnsureFileSync,
-  remove as fsRemove,
   removeSync as fsRemoveSync,
   move as fsMove,
   // eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -260,14 +259,29 @@ export function mkdirSync(path: string): void {
   fsMkdirSync(path, {recursive: true})
 }
 
+interface RemoveFileOptions {
+  /**
+   * Number of times Node retries the removal when it hits a transient error
+   * (EBUSY, EMFILE, ENFILE, ENOTEMPTY or EPERM), waiting `retryDelay` milliseconds
+   * longer on each try. Defaults to 0 (no retries).
+   */
+  maxRetries?: number
+  /**
+   * Milliseconds to wait between retries. Defaults to 100.
+   */
+  retryDelay?: number
+}
+
 /**
- * Removes a file at the given path.
+ * Removes a file or directory (recursively) at the given path.
  *
- * @param path - Path to the file to be removed.
+ * @param path - Path to the file or directory to be removed.
+ * @param options - Retry behavior, passed through to Node's `fs.rm`. Useful when the removal can
+ * race with transient locks, such as an antivirus scanning freshly written files.
  */
-export async function removeFile(path: string): Promise<void> {
+export async function removeFile(path: string, options: RemoveFileOptions = {}): Promise<void> {
   outputDebug(outputContent`Removing file at ${outputToken.path(path)}...`)
-  await fsRemove(path)
+  await fsRm(path, {recursive: true, force: true, ...options})
 }
 
 /**