From b8e0bfea69675159fc8d6946a22dd16a3f02330f Mon Sep 17 00:00:00 2001 From: Sean Larkin <3408176+TheLarkInn@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:08:45 +0000 Subject: [PATCH 1/2] Treat a phased command with zero operations as success A phased command whose plugins legitimately produce no operations exited with code 1, even though nothing failed. OperationGraph._scheduleIterationAsync does not schedule an iteration when there are no non-silent operations, so executeAsync early-returns OperationStatus.NoOp without firing any graph hooks. PhasedScriptAction then computed `success = status === OperationStatus.Success`, so NoOp was reported as a failure. This broke @rushstack/rush-buildxl-graph-plugin, which writes the build graph to disk in response to --drop-graph and then returns an empty operation set because there is nothing left to execute. Every BuildXL build using the Rush resolver failed with DX11901/DX11230 even though the graph was produced correctly. Because the early return skips all graph hooks, this could not be worked around by a plugin. PhasedScriptAction already treats an empty project selection as success, so treating an empty operation set as a failure was inconsistent. Adds a regression test: a mock in-repo plugin clears all operations during createOperationsAsync, mirroring what rush-buildxl-graph-plugin does, and the command is expected to succeed without spawning anything. --- ...ix-noop-exit-code_2026-07-28-18-56-00.json | 10 ++++++ libraries/rush-lib/config/heft.json | 8 +++++ .../cli/scriptActions/PhasedScriptAction.ts | 13 +++++--- .../cli/test/RushCommandLineParser.test.ts | 21 +++++++++++++ .../a/package.json | 9 ++++++ .../b/package.json | 9 ++++++ .../autoinstallers/plugins/package.json | 8 +++++ .../rush-plugin-manifest.json | 9 ++++++ .../common/config/rush/rush-plugins.json | 9 ++++++ .../rush.json | 17 ++++++++++ .../index.ts | 31 +++++++++++++++++++ .../package.json | 6 ++++ .../rush-plugin-manifest.json | 9 ++++++ 13 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/a/package.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/b/package.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/package.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/rush-plugins/rush-mock-clear-operations-plugin/rush-plugin-manifest.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/config/rush/rush-plugins.json create mode 100644 libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/rush.json create mode 100644 libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts create mode 100644 libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/package.json create mode 100644 libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/rush-plugin-manifest.json diff --git a/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json b/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json new file mode 100644 index 00000000000..35bb3e16d40 --- /dev/null +++ b/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix an issue where a phased command exited with a nonzero exit code when a plugin legitimately produced no operations, which broke `rush --drop-graph` in `@rushstack/rush-buildxl-graph-plugin`.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/config/heft.json b/libraries/rush-lib/config/heft.json index 4f76fe45dcf..02a4934f2bf 100644 --- a/libraries/rush-lib/config/heft.json +++ b/libraries/rush-lib/config/heft.json @@ -27,6 +27,14 @@ "fileExtensions": [".json", ".js", ".map"], "hardlink": true }, + { + "sourcePath": "lib-intermediate-commonjs/cli/test/rush-mock-clear-operations-plugin", + "destinationFolders": [ + "lib-intermediate-commonjs/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/node_modules/rush-mock-clear-operations-plugin" + ], + "fileExtensions": [".json", ".js", ".map"], + "hardlink": true + }, { "sourcePath": "src/cli/test", "destinationFolders": ["lib-intermediate-commonjs/cli/test"], diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 849aec4410a..875a04accfb 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -713,7 +713,6 @@ export class PhasedScriptAction extends BaseScriptAction i const { graph, ignoreHooks, stopwatch, terminal } = options; let success: boolean = false; - let result: IExecutionResult | undefined; try { const definiteResult: IExecutionResult = await measureAsyncFn( @@ -722,13 +721,19 @@ export class PhasedScriptAction extends BaseScriptAction i return await graph.executeAsync(iterationOptions); } ); - success = definiteResult.status === OperationStatus.Success; - result = definiteResult; + // An iteration that produced no operations is not a failure. This happens when a plugin + // legitimately consumes the work itself and returns an empty operation set -- for example + // `@rushstack/rush-buildxl-graph-plugin`, which writes the graph to disk in response to + // `--drop-graph` and then returns `new Set()` because there is nothing left to execute. + // Note that `PhasedScriptAction` already treats an empty *project* selection as success, so + // treating an empty *operation* set as a failure would be inconsistent. + success = + definiteResult.status === OperationStatus.Success || definiteResult.status === OperationStatus.NoOp; stopwatch.stop(); const message: string = `rush ${this.actionName} (${stopwatch.toString()})`; - if (result.status === OperationStatus.Success) { + if (success) { terminal.writeLine(Colorize.green(message)); } else { terminal.writeLine(message); diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts index 015c9871873..dcdbca339ff 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -317,6 +317,27 @@ describe('RushCommandLineParser', () => { }); }); + describe('in repo plugin that produces no operations', () => { + it('succeeds when a plugin returns an empty operation set', async () => { + // Regression test: `@rushstack/rush-buildxl-graph-plugin` writes the build graph to disk in + // response to `--drop-graph` and then returns an empty operation set, because there is + // nothing left for Rush to execute. An iteration with zero operations resolves to + // `OperationStatus.NoOp`, which must not be reported as a failure. + const repoName: string = 'clearOperationsAndRunBuildActionRepo'; + const { parser, spawnMock } = await getCommandLineParserInstanceAsync(repoName, 'build'); + + /** + * The plugin is copied into the autoinstaller folder using an option in /config/heft.json + */ + jest.spyOn(Autoinstaller.prototype, 'prepareAsync').mockImplementation(async function () {}); + + await expect(parser.executeAsync()).resolves.toEqual(true); + + // Nothing should have been executed, since the plugin removed every operation. + expect(spawnMock.mock.calls.length).toEqual(0); + }); + }); + describe('in repo plugin with build command', () => { describe("'build' action", () => { it(`executes the package's 'build' script`, async () => { diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/a/package.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/a/package.json new file mode 100644 index 00000000000..f00575e3099 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/a/package.json @@ -0,0 +1,9 @@ +{ + "name": "a", + "version": "1.0.0", + "description": "Test package a", + "scripts": { + "build": "fake_build_task_but_works_with_mock", + "rebuild": "fake_REbuild_task_but_works_with_mock" + } +} diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/b/package.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/b/package.json new file mode 100644 index 00000000000..8f203bb691d --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/b/package.json @@ -0,0 +1,9 @@ +{ + "name": "b", + "version": "1.0.0", + "description": "Test package b", + "scripts": { + "build": "fake_build_task_but_works_with_mock", + "rebuild": "fake_REbuild_task_but_works_with_mock" + } +} diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/package.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/package.json new file mode 100644 index 00000000000..9d477cd6aad --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/package.json @@ -0,0 +1,8 @@ +{ + "name": "plugins", + "version": "1.0.0", + "private": true, + "dependencies": { + "rush-mock-clear-operations-plugin": "file:../../../../rush-mock-clear-operations-plugin" + } +} diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/rush-plugins/rush-mock-clear-operations-plugin/rush-plugin-manifest.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/rush-plugins/rush-mock-clear-operations-plugin/rush-plugin-manifest.json new file mode 100644 index 00000000000..94c8982167f --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/rush-plugins/rush-mock-clear-operations-plugin/rush-plugin-manifest.json @@ -0,0 +1,9 @@ +{ + "plugins": [ + { + "pluginName": "rush-mock-clear-operations-plugin", + "description": "Rush plugin for testing a phased command that produces no operations", + "entryPoint": "index.js" + } + ] +} diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/config/rush/rush-plugins.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/config/rush/rush-plugins.json new file mode 100644 index 00000000000..94f6be8e009 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/config/rush/rush-plugins.json @@ -0,0 +1,9 @@ +{ + "plugins": [ + { + "packageName": "rush-mock-clear-operations-plugin", + "pluginName": "rush-mock-clear-operations-plugin", + "autoinstallerName": "plugins" + } + ] +} diff --git a/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/rush.json b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/rush.json new file mode 100644 index 00000000000..8bdab648130 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/rush.json @@ -0,0 +1,17 @@ +{ + "npmVersion": "6.4.1", + "rushVersion": "5.62.2", + "projectFolderMinDepth": 1, + "projectFolderMaxDepth": 99, + + "projects": [ + { + "packageName": "a", + "projectFolder": "a" + }, + { + "packageName": "b", + "projectFolder": "b" + } + ] +} diff --git a/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts new file mode 100644 index 00000000000..b9af8c02194 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import type { RushSession, RushConfiguration, IPhasedCommand, Operation } from '../../../index'; + +/** + * Mimics the shape of `@rushstack/rush-buildxl-graph-plugin`, which performs its work during + * `createOperationsAsync` and then returns an empty operation set because there is nothing left + * for Rush to execute. + * + * Such an invocation must be reported as a success, not a failure. + */ +export default class RushMockClearOperationsPlugin { + public apply(rushSession: RushSession, rushConfiguration: RushConfiguration): void { + rushSession.hooks.runAnyPhasedCommand.tapPromise( + RushMockClearOperationsPlugin.name, + async (command: IPhasedCommand) => { + command.hooks.createOperationsAsync.tapPromise( + { + name: RushMockClearOperationsPlugin.name, + // Run after every other plugin has finished creating operations. + stage: Number.MAX_SAFE_INTEGER + }, + async () => { + return new Set(); + } + ); + } + ); + } +} diff --git a/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/package.json b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/package.json new file mode 100644 index 00000000000..a28fd24a685 --- /dev/null +++ b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/package.json @@ -0,0 +1,6 @@ +{ + "name": "rush-mock-clear-operations-plugin", + "version": "1.0.0", + "private": true, + "dependencies": {} +} diff --git a/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/rush-plugin-manifest.json b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/rush-plugin-manifest.json new file mode 100644 index 00000000000..94c8982167f --- /dev/null +++ b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/rush-plugin-manifest.json @@ -0,0 +1,9 @@ +{ + "plugins": [ + { + "pluginName": "rush-mock-clear-operations-plugin", + "description": "Rush plugin for testing a phased command that produces no operations", + "entryPoint": "index.js" + } + ] +} From 98cc0bba3d9c0d0740cef6f980bb6612d5d3c1f9 Mon Sep 17 00:00:00 2001 From: Sean Larkin Date: Wed, 5 Aug 2026 01:26:26 +0000 Subject: [PATCH 2/2] Broaden phased command success statuses to Skipped and FromCache Address review feedback: use a shared SUCCESSFUL_EXECUTION_STATUSES set so that an iteration short-circuited by a tap with a successful bail status is reported as success, alongside the NoOp case. Also drop the unused rushConfiguration parameter from the mock plugin. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...ix-noop-exit-code_2026-07-28-18-56-00.json | 2 +- .../cli/scriptActions/PhasedScriptAction.ts | 31 ++++++++++++++----- .../index.ts | 4 +-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json b/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json index 35bb3e16d40..a44aa1b9be0 100644 --- a/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json +++ b/common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Fix an issue where a phased command exited with a nonzero exit code when a plugin legitimately produced no operations, which broke `rush --drop-graph` in `@rushstack/rush-buildxl-graph-plugin`.", + "comment": "Fix an issue where a phased command exited with a nonzero exit code when its overall execution status was successful but not `SUCCESS`. This covers an iteration that scheduled no operations because a plugin consumed the work itself (which broke `rush --drop-graph` in `@rushstack/rush-buildxl-graph-plugin`), as well as an iteration that a plugin short-circuited with a `SKIPPED` or `FROM CACHE` status.", "type": "patch" } ], diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 875a04accfb..7b79e36e081 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -64,6 +64,28 @@ import { measureAsyncFn, measureFn } from '../../utilities/performance'; const PERF_PREFIX: 'rush:phasedScriptAction' = 'rush:phasedScriptAction'; +/** + * The set of overall execution statuses that mean the command did what was asked of it and should + * exit with code 0. + * + * - `NoOp` -- the iteration scheduled no non-silent operations. This happens when a plugin + * legitimately consumes the work itself, either by returning an empty operation set from + * `createOperationsAsync` or by disabling every operation during `configureIteration` (a disabled + * record is silent, so both routes converge on this status). + * - `Skipped` / `FromCache` -- a tap short-circuited the iteration with a successful bail status, + * for example the bridge-cache plugin performing a cache read/write out of band. + * + * `PhasedScriptAction` already treats an empty *project* selection as success, so treating an empty + * *operation* set as a failure would be inconsistent. `SuccessWithWarning` is deliberately excluded + * because non-allowed warnings are expected to fail the command. + */ +const SUCCESSFUL_EXECUTION_STATUSES: ReadonlySet = new Set([ + OperationStatus.Success, + OperationStatus.Skipped, + OperationStatus.FromCache, + OperationStatus.NoOp +]); + /** * Constructor parameters for PhasedScriptAction. */ @@ -721,14 +743,7 @@ export class PhasedScriptAction extends BaseScriptAction i return await graph.executeAsync(iterationOptions); } ); - // An iteration that produced no operations is not a failure. This happens when a plugin - // legitimately consumes the work itself and returns an empty operation set -- for example - // `@rushstack/rush-buildxl-graph-plugin`, which writes the graph to disk in response to - // `--drop-graph` and then returns `new Set()` because there is nothing left to execute. - // Note that `PhasedScriptAction` already treats an empty *project* selection as success, so - // treating an empty *operation* set as a failure would be inconsistent. - success = - definiteResult.status === OperationStatus.Success || definiteResult.status === OperationStatus.NoOp; + success = SUCCESSFUL_EXECUTION_STATUSES.has(definiteResult.status); stopwatch.stop(); diff --git a/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts index b9af8c02194..f4f7fd9d557 100644 --- a/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts +++ b/libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { RushSession, RushConfiguration, IPhasedCommand, Operation } from '../../../index'; +import type { RushSession, IPhasedCommand, Operation } from '../../../index'; /** * Mimics the shape of `@rushstack/rush-buildxl-graph-plugin`, which performs its work during @@ -11,7 +11,7 @@ import type { RushSession, RushConfiguration, IPhasedCommand, Operation } from ' * Such an invocation must be reported as a success, not a failure. */ export default class RushMockClearOperationsPlugin { - public apply(rushSession: RushSession, rushConfiguration: RushConfiguration): void { + public apply(rushSession: RushSession): void { rushSession.hooks.runAnyPhasedCommand.tapPromise( RushMockClearOperationsPlugin.name, async (command: IPhasedCommand) => {