From 7e825f37b08d4e3130daf46a41011980aaa08790 Mon Sep 17 00:00:00 2001
From: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
Date: Tue, 4 Nov 2025 11:51:47 +0200
Subject: [PATCH] fix: handle ENOTEMPTY errors in moveFile

Like EEXIST, we handle ENOTEMPTY when we fail to moveFile.

A user reported they consistently could not update our CLI tool. [1] In
that report it always had the same file it tried to rename to. I believe
what is happening here is when NPM retires an old version it renames it
to a deterministic path. However, I suspect a previous update was
interrupted leaving a non-empty directory at the destination.

When googling ENOTEMPTY all the results seem to be about npm failures
with users suggesting clearing out node_modules. Maybe this will also
solve those issues.

[1]: https://gist.github.com/keegancsmith/5dac339583cc127031da6ed0e8512d47
---
 workspaces/arborist/lib/arborist/reify.js  |  2 +-
 workspaces/arborist/test/arborist/reify.js | 66 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js
index abc2c8a5dd0bd..cdedfcecc79f8 100644
--- a/workspaces/arborist/lib/arborist/reify.js
+++ b/workspaces/arborist/lib/arborist/reify.js
@@ -510,7 +510,7 @@ module.exports = cls => class Reifier extends cls {
         if (er.code === 'ENOENT') {
           return didMkdirp ? null : mkdir(dirname(to), { recursive: true }).then(() =>
             this[_renamePath](from, to, true))
-        } else if (er.code === 'EEXIST') {
+        } else if (er.code === 'EEXIST' || er.code === 'ENOTEMPTY') {
           return rm(to, { recursive: true, force: true }).then(() => moveFile(from, to))
         } else {
           throw er
diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js
index 4209c605a9791..77e5fdfb69a04 100644
--- a/workspaces/arborist/test/arborist/reify.js
+++ b/workspaces/arborist/test/arborist/reify.js
@@ -818,6 +818,72 @@ t.test('rollbacks', { buffered: false }, t => {
     return printTree(tree)
   })
 
+  t.test('fail retiring nodes because rm fails after enotempty', t => {
+    const path = fixture(t, 'testing-bundledeps-3')
+    createRegistry(t, true)
+    const a = newArb({ path, installStrategy: 'nested' })
+    const enotempty = new Error('rename fail')
+    enotempty.code = 'ENOTEMPTY'
+    const kRenamePath = Symbol.for('renamePath')
+    const renamePath = a[kRenamePath]
+    a[kRenamePath] = (from, to) => {
+      a[kRenamePath] = renamePath
+      failRename = enotempty
+      failRm = true
+      return a[kRenamePath](from, to)
+    }
+    const kRollback = Symbol.for('rollbackRetireShallowNodes')
+    const rollbackRetireShallowNodes = a[kRollback]
+    let rolledBack = false
+    a[kRollback] = er => {
+      rolledBack = true
+      failRename = new Error('some other error')
+      failRm = false
+      t.match(er, new Error('rm fail'))
+      a[kRollback] = rollbackRetireShallowNodes
+      return a[kRollback](er).then(er => {
+        failRename = null
+        return er
+      }, er => {
+        failRename = null
+        throw er
+      })
+    }
+
+    return t.rejects(a.reify({
+      update: ['@isaacs/testing-bundledeps-parent'],
+    }), new Error('rm fail'))
+      .then(() => t.equal(rolledBack, true, 'rolled back'))
+  })
+
+  t.test('fail retiring node with enotempty, but then rm fixes it', async t => {
+    const path = fixture(t, 'testing-bundledeps-3')
+    createRegistry(t, true)
+    const a = newArb({ path, installStrategy: 'nested' })
+    const enotempty = new Error('rename fail')
+    enotempty.code = 'ENOTEMPTY'
+    const kRenamePath = Symbol.for('renamePath')
+    const renamePath = a[kRenamePath]
+    a[kRenamePath] = (from, to) => {
+      a[kRenamePath] = renamePath
+      failRenameOnce = enotempty
+      return a[kRenamePath](from, to)
+    }
+    const kRollback = Symbol.for('rollbackRetireShallowNodes')
+    const rollbackRetireShallowNodes = a[kRollback]
+    a[kRollback] = er => {
+      t.fail('should not roll back!')
+      a[kRollback] = rollbackRetireShallowNodes
+      return a[kRollback](er)
+    }
+
+    const tree = await a.reify({
+      update: ['@isaacs/testing-bundledeps-parent'],
+      save: false,
+    })
+    return printTree(tree)
+  })
+
   t.test('fail creating sparse tree', t => {
     t.teardown(() => failMkdir = null)
     const path = fixture(t, 'testing-bundledeps-3')