From 162a38304cd1d622e331bc303fdf0a997d38531c Mon Sep 17 00:00:00 2001 From: Kevin Yang <61671317+kevin-y-ang@users.noreply.github.com> Date: Tue, 7 Apr 2026 21:41:00 -0700 Subject: [PATCH 1/4] Use AsyncRecycler for autoinstaller cleanup --- ...-modules-to-recycler_2026-04-07-18-05.json | 10 ++ .../src/cli/test/Autoinstaller.test.ts | 101 ++++++++++++++++++ libraries/rush-lib/src/logic/Autoinstaller.ts | 7 +- 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json create mode 100644 libraries/rush-lib/src/cli/test/Autoinstaller.test.ts diff --git a/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json b/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json new file mode 100644 index 00000000000..6b3ae6a1d21 --- /dev/null +++ b/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Move stale autoinstaller node_modules folders into Rush's recycler before deleting them, instead of clearing them in place.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts new file mode 100644 index 00000000000..5f804406677 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'node:path'; + +import './mockRushCommandLineParser'; + +import { FileSystem } from '@rushstack/node-core-library'; + +import { Autoinstaller } from '../../logic/Autoinstaller'; +import { InstallHelpers } from '../../logic/installManager/InstallHelpers'; +import { RushConstants } from '../../logic/RushConstants'; +import { Utilities } from '../../utilities/Utilities'; +import { + getCommandLineParserInstanceAsync, + isolateEnvironmentConfigurationForTests, + type IEnvironmentConfigIsolation +} from './TestUtils'; + +describe('Autoinstaller', () => { + let _envIsolation: IEnvironmentConfigIsolation; + + beforeEach(() => { + _envIsolation = isolateEnvironmentConfigurationForTests(); + }); + + afterEach(() => { + _envIsolation.restore(); + jest.restoreAllMocks(); + }); + + it('moves an existing node_modules folder into the Rush recycler before reinstalling', async () => { + const { parser, repoPath, spawnMock } = await getCommandLineParserInstanceAsync( + 'pluginWithBuildCommandRepo', + 'update' + ); + const autoinstallerPath: string = path.join(repoPath, 'common/autoinstallers/plugins'); + const nodeModulesFolder: string = path.join(autoinstallerPath, RushConstants.nodeModulesFolderName); + const staleFilePath: string = path.join(nodeModulesFolder, 'stale-package/index.js'); + const recyclerFolder: string = path.join( + parser.rushConfiguration.commonTempFolder, + RushConstants.rushRecyclerFolderName + ); + + FileSystem.writeFile(staleFilePath, 'stale', { + ensureFolderExists: true + }); + + const recyclerEntriesBefore: Set = FileSystem.exists(recyclerFolder) + ? new Set(FileSystem.readFolderItemNames(recyclerFolder)) + : new Set(); + + jest.spyOn(InstallHelpers, 'ensureLocalPackageManagerAsync').mockResolvedValue(undefined); + jest.spyOn(Utilities, 'syncNpmrc').mockImplementation(() => undefined); + jest + .spyOn(Utilities, 'executeCommandAsync') + .mockImplementation(async (options: Parameters[0]) => { + FileSystem.ensureFolder(path.join(options.workingDirectory, RushConstants.nodeModulesFolderName)); + }); + + const autoinstaller: Autoinstaller = new Autoinstaller({ + autoinstallerName: 'plugins', + rushConfiguration: parser.rushConfiguration, + rushGlobalFolder: parser.rushGlobalFolder + }); + + await autoinstaller.prepareAsync(); + + const recyclerEntriesAfter: string[] = FileSystem.readFolderItemNames(recyclerFolder).filter( + (entry: string) => !recyclerEntriesBefore.has(entry) + ); + + expect(recyclerEntriesAfter).toHaveLength(1); + expect( + FileSystem.exists(path.join(recyclerFolder, recyclerEntriesAfter[0], 'stale-package/index.js')) + ).toBe(true); + expect(FileSystem.exists(staleFilePath)).toBe(false); + expect(FileSystem.exists(path.join(nodeModulesFolder, 'rush-autoinstaller.flag'))).toBe(true); + + if (process.platform === 'win32') { + expect(spawnMock).toHaveBeenCalledWith( + 'cmd.exe', + expect.arrayContaining(['/c']), + expect.objectContaining({ + detached: true, + stdio: 'ignore', + windowsVerbatimArguments: true + }) + ); + } else { + expect(spawnMock).toHaveBeenCalledWith( + 'rm', + expect.arrayContaining(['-rf']), + expect.objectContaining({ + detached: true, + stdio: 'ignore' + }) + ); + } + }); +}); diff --git a/libraries/rush-lib/src/logic/Autoinstaller.ts b/libraries/rush-lib/src/logic/Autoinstaller.ts index ae55a8c361b..0a1772aef79 100644 --- a/libraries/rush-lib/src/logic/Autoinstaller.ts +++ b/libraries/rush-lib/src/logic/Autoinstaller.ts @@ -14,6 +14,7 @@ import { } from '@rushstack/node-core-library'; import { Colorize } from '@rushstack/terminal'; +import { AsyncRecycler } from '../utilities/AsyncRecycler'; import { Utilities } from '../utilities/Utilities'; import type { RushConfiguration } from '../api/RushConfiguration'; import { PackageJsonEditor } from '../api/PackageJsonEditor'; @@ -131,7 +132,11 @@ export class Autoinstaller { if (isLastInstallFlagDirty || lock.dirtyWhenAcquired) { if (FileSystem.exists(nodeModulesFolder)) { this._logIfConsoleOutputIsNotRestricted('Deleting old files from ' + nodeModulesFolder); - FileSystem.ensureEmptyFolder(nodeModulesFolder); + const recycler = new AsyncRecycler( + path.join(this._rushConfiguration.commonTempFolder, RushConstants.rushRecyclerFolderName) + ); + recycler.moveFolder(nodeModulesFolder); + await recycler.startDeleteAllAsync(); } // Copy: .../common/autoinstallers/my-task/.npmrc From db36fa164888cd921d15b073c823cf04597711e5 Mon Sep 17 00:00:00 2001 From: Kevin Yang <61671317+kevin-y-ang@users.noreply.github.com> Date: Wed, 8 Apr 2026 14:51:39 -0700 Subject: [PATCH 2/4] Restore AsyncRecycler type annotation --- libraries/rush-lib/src/logic/Autoinstaller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/logic/Autoinstaller.ts b/libraries/rush-lib/src/logic/Autoinstaller.ts index 0a1772aef79..3145615a34d 100644 --- a/libraries/rush-lib/src/logic/Autoinstaller.ts +++ b/libraries/rush-lib/src/logic/Autoinstaller.ts @@ -132,7 +132,7 @@ export class Autoinstaller { if (isLastInstallFlagDirty || lock.dirtyWhenAcquired) { if (FileSystem.exists(nodeModulesFolder)) { this._logIfConsoleOutputIsNotRestricted('Deleting old files from ' + nodeModulesFolder); - const recycler = new AsyncRecycler( + const recycler: AsyncRecycler = new AsyncRecycler( path.join(this._rushConfiguration.commonTempFolder, RushConstants.rushRecyclerFolderName) ); recycler.moveFolder(nodeModulesFolder); From ff0f09401919342a0ba3ebba8c52070e98329bcf Mon Sep 17 00:00:00 2001 From: Kevin Yang <61671317+kevin-y-ang@users.noreply.github.com> Date: Thu, 9 Apr 2026 15:07:16 -0700 Subject: [PATCH 3/4] Apply suggestions from code review - Use string interpolation instead of path.join - Avoid race condition from checking if file exists before operating on it Co-authored-by: Ian Clanton-Thuon --- ...-modules-to-recycler_2026-04-07-18-05.json | 2 +- .../src/cli/test/Autoinstaller.test.ts | 40 +++++++++++-------- libraries/rush-lib/src/logic/Autoinstaller.ts | 2 +- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json b/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json index 6b3ae6a1d21..74e96cdb0f0 100644 --- a/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json +++ b/common/changes/@microsoft/rush/autoinstaller-move-node-modules-to-recycler_2026-04-07-18-05.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Move stale autoinstaller node_modules folders into Rush's recycler before deleting them, instead of clearing them in place.", + "comment": "Move stale autoinstaller `node_modules` folders into Rush's recycler before asynchronously deleting them, instead of synchronously deleting them in place.", "type": "none" } ], diff --git a/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts index 5f804406677..50724ae4409 100644 --- a/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts +++ b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts @@ -17,7 +17,7 @@ import { type IEnvironmentConfigIsolation } from './TestUtils'; -describe('Autoinstaller', () => { +describe(Autoinstaller.name, () => { let _envIsolation: IEnvironmentConfigIsolation; beforeEach(() => { @@ -34,28 +34,34 @@ describe('Autoinstaller', () => { 'pluginWithBuildCommandRepo', 'update' ); - const autoinstallerPath: string = path.join(repoPath, 'common/autoinstallers/plugins'); - const nodeModulesFolder: string = path.join(autoinstallerPath, RushConstants.nodeModulesFolderName); - const staleFilePath: string = path.join(nodeModulesFolder, 'stale-package/index.js'); - const recyclerFolder: string = path.join( - parser.rushConfiguration.commonTempFolder, - RushConstants.rushRecyclerFolderName - ); + const autoinstallerPath: string = `${repoPath}/common/autoinstallers/plugins`; + const nodeModulesFolder: string = `${autoinstallerPath}/${RushConstants.nodeModulesFolderName}`; + const staleFilePath: string = `${nodeModulesFolder}/stale-package/index.js`; + const recyclerFolder: string = `${parser.rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}`; - FileSystem.writeFile(staleFilePath, 'stale', { + await FileSystem.writeFileAsync(staleFilePath, 'stale', { ensureFolderExists: true }); const recyclerEntriesBefore: Set = FileSystem.exists(recyclerFolder) ? new Set(FileSystem.readFolderItemNames(recyclerFolder)) - : new Set(); + let recyclerEntriesBefore: Set; + try { + recyclerEntriesBefore =new Set(await FileSystem.readFolderItemNamesAsync(recyclerFolder)); + } catch (error) { + if (FileSystem.isNotExistError(error)) { + recyclerEntriesBefore= new Set(); + } else { + throw error; + } + } jest.spyOn(InstallHelpers, 'ensureLocalPackageManagerAsync').mockResolvedValue(undefined); jest.spyOn(Utilities, 'syncNpmrc').mockImplementation(() => undefined); jest .spyOn(Utilities, 'executeCommandAsync') .mockImplementation(async (options: Parameters[0]) => { - FileSystem.ensureFolder(path.join(options.workingDirectory, RushConstants.nodeModulesFolderName)); + await FileSystem.ensureFolderAsync(`${options.workingDirectory}/${RushConstants.nodeModulesFolderName}`); }); const autoinstaller: Autoinstaller = new Autoinstaller({ @@ -66,16 +72,16 @@ describe('Autoinstaller', () => { await autoinstaller.prepareAsync(); - const recyclerEntriesAfter: string[] = FileSystem.readFolderItemNames(recyclerFolder).filter( + const recyclerEntriesAfter: string[] = (await FileSystem.readFolderItemNamesAsync(recyclerFolder)).filter( (entry: string) => !recyclerEntriesBefore.has(entry) ); expect(recyclerEntriesAfter).toHaveLength(1); - expect( - FileSystem.exists(path.join(recyclerFolder, recyclerEntriesAfter[0], 'stale-package/index.js')) - ).toBe(true); - expect(FileSystem.exists(staleFilePath)).toBe(false); - expect(FileSystem.exists(path.join(nodeModulesFolder, 'rush-autoinstaller.flag'))).toBe(true); + await expect( + FileSystem.existsAsync(`${recyclerFolder}/${recyclerEntriesAfter[0]}/stale-package/index.js`) + ).resolves.toBe(true); + await expect(FileSystem.existsAsync(staleFilePath)).resolves.toBe(false); + await expect(FileSystem.existsAsync(`${nodeModulesFolder}/rush-autoinstaller.flag`)).resolves.toBe(true); if (process.platform === 'win32') { expect(spawnMock).toHaveBeenCalledWith( diff --git a/libraries/rush-lib/src/logic/Autoinstaller.ts b/libraries/rush-lib/src/logic/Autoinstaller.ts index 3145615a34d..a47dd0d89be 100644 --- a/libraries/rush-lib/src/logic/Autoinstaller.ts +++ b/libraries/rush-lib/src/logic/Autoinstaller.ts @@ -133,7 +133,7 @@ export class Autoinstaller { if (FileSystem.exists(nodeModulesFolder)) { this._logIfConsoleOutputIsNotRestricted('Deleting old files from ' + nodeModulesFolder); const recycler: AsyncRecycler = new AsyncRecycler( - path.join(this._rushConfiguration.commonTempFolder, RushConstants.rushRecyclerFolderName) + `${this._rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}` ); recycler.moveFolder(nodeModulesFolder); await recycler.startDeleteAllAsync(); From 828a44d4600f63a8cea17555ced16608746032c0 Mon Sep 17 00:00:00 2001 From: Kevin Yang <61671317+kevin-y-ang@users.noreply.github.com> Date: Thu, 9 Apr 2026 15:21:41 -0700 Subject: [PATCH 4/4] fix(Autoinstaller): fix conflict --- .../src/cli/test/Autoinstaller.test.ts | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts index 50724ae4409..f975427f50a 100644 --- a/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts +++ b/libraries/rush-lib/src/cli/test/Autoinstaller.test.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import * as path from 'node:path'; - import './mockRushCommandLineParser'; import { FileSystem } from '@rushstack/node-core-library'; @@ -34,23 +32,21 @@ describe(Autoinstaller.name, () => { 'pluginWithBuildCommandRepo', 'update' ); - const autoinstallerPath: string = `${repoPath}/common/autoinstallers/plugins`; - const nodeModulesFolder: string = `${autoinstallerPath}/${RushConstants.nodeModulesFolderName}`; - const staleFilePath: string = `${nodeModulesFolder}/stale-package/index.js`; - const recyclerFolder: string = `${parser.rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}`; + const autoinstallerPath: string = `${repoPath}/common/autoinstallers/plugins`; + const nodeModulesFolder: string = `${autoinstallerPath}/${RushConstants.nodeModulesFolderName}`; + const staleFilePath: string = `${nodeModulesFolder}/stale-package/index.js`; + const recyclerFolder: string = `${parser.rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}`; await FileSystem.writeFileAsync(staleFilePath, 'stale', { ensureFolderExists: true }); - const recyclerEntriesBefore: Set = FileSystem.exists(recyclerFolder) - ? new Set(FileSystem.readFolderItemNames(recyclerFolder)) let recyclerEntriesBefore: Set; try { - recyclerEntriesBefore =new Set(await FileSystem.readFolderItemNamesAsync(recyclerFolder)); + recyclerEntriesBefore = new Set(await FileSystem.readFolderItemNamesAsync(recyclerFolder)); } catch (error) { if (FileSystem.isNotExistError(error)) { - recyclerEntriesBefore= new Set(); + recyclerEntriesBefore = new Set(); } else { throw error; } @@ -61,7 +57,9 @@ describe(Autoinstaller.name, () => { jest .spyOn(Utilities, 'executeCommandAsync') .mockImplementation(async (options: Parameters[0]) => { - await FileSystem.ensureFolderAsync(`${options.workingDirectory}/${RushConstants.nodeModulesFolderName}`); + await FileSystem.ensureFolderAsync( + `${options.workingDirectory}/${RushConstants.nodeModulesFolderName}` + ); }); const autoinstaller: Autoinstaller = new Autoinstaller({