Skip to content

[rush] Treat a phased command with zero operations as success - #5915

Merged
Sean Larkin (TheLarkInn) merged 2 commits into
mainfrom
fix/phased-command-noop-exit-code
Aug 5, 2026
Merged

[rush] Treat a phased command with zero operations as success#5915
Sean Larkin (TheLarkInn) merged 2 commits into
mainfrom
fix/phased-command-noop-exit-code

Conversation

@TheLarkInn

Copy link
Copy Markdown
Member

Summary

Fixes #5914.

A phased command whose plugins legitimately produce no operations exits with code 1, even
though nothing failed. This breaks @rushstack/rush-buildxl-graph-plugin, so every BuildXL build
using the Rush resolver fails.

Details

OperationGraph._scheduleIterationAsync does not schedule an iteration when there are no non-silent
operations:

if (iterationContext.totalOperations === 0) {
  return;
}

so executeAsync early-returns OperationStatus.NoOp without firing any graph hooks:

const scheduled = await this._scheduleIterationAsync(iterationOptions);
if (!scheduled) {
  return { operationResults: this.resultByOperation, status: OperationStatus.NoOp };
}

and PhasedScriptAction computed:

success = definiteResult.status === OperationStatus.Success;

NoOp !== Successsuccess = falseAlreadyReportedError → exit 1.

DropBuildGraphPlugin writes the build graph to disk in response to --drop-graph and then returns
new Set() because there is nothing left for Rush to execute. The result is that
rush <command> --drop-graph exits 1 with no error message and a correctly-written graph file,
which BuildXL reports as DX11901 / DX11230.

Because the early return skips all graph hooks, this cannot be worked around by a plugin — there
is no hook available to observe or correct the status.

Why this is the right fix

PhasedScriptAction already treats an empty project selection as success:

if (!generateFullGraph && !projectSelection.size) {
  terminal.writeLine(Colorize.yellow(`The command line selection parameters did not match any projects.`));
  return;   // exits 0
}

So "nothing to do because no projects matched" was a success while "nothing to do because no
operations were produced" was a failure. This change makes them agree.

The completion message now keys off success rather than re-deriving the status, so a no-op
invocation is also reported in green rather than looking like a silent failure.

Changes

  • PhasedScriptAction.ts — treat OperationStatus.NoOp as success; use success for the
    completion message. Removes the now-unused result local.
  • New regression test in RushCommandLineParser.test.ts plus fixtures:
    • rush-mock-clear-operations-plugin/ — an in-repo plugin that taps createOperationsAsync at
      stage: Number.MAX_SAFE_INTEGER and returns an empty set, mirroring DropBuildGraphPlugin.
    • clearOperationsAndRunBuildActionRepo/ — mock repo wiring that plugin.
    • config/heft.json — copy the mock plugin into the fixture's autoinstaller, matching the
      existing rush-mock-flush-telemetry-plugin pattern.
  • Change file for @microsoft/rush (patch).

Verification

Check Result
New test without the fix FAILSexpect(parser.executeAsync()).resolves.toEqual(true)Received: false
New test with the fix passes
Full rush-lib suite 721 passed, 0 failed
rush build --to @microsoft/rush-lib clean, no lint warnings

Notes for reviewers

An alternative would be to give phased commands a supported "handled — stop here successfully"
mechanism, analogous to IGlobalCommand.setHandled(), so plugins like DropBuildGraphPlugin do not
have to signal completion by returning an empty operation set. That seemed like a larger API change
than warranted for a regression fix, but I'm happy to go that route instead.

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression where phased commands that legitimately produce zero operations (e.g., a plugin consumes the work and returns an empty operation set) would exit with a nonzero code due to OperationStatus.NoOp being treated as failure in PhasedScriptAction. It aligns “no operations to execute” with existing behavior where “no projects matched” is considered a successful no-op.

Changes:

  • Treat OperationStatus.NoOp as a success outcome for phased commands and key the completion message off the computed success flag.
  • Add a regression test that wires an in-repo plugin which clears the operation set and verifies the command succeeds without spawning any tasks.
  • Add test fixtures + Heft copy wiring to stage the mock plugin into the fixture autoinstaller (mirroring the existing flush-telemetry mock plugin pattern), plus a patch change file for @microsoft/rush.
Show a summary per file
File Description
libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts Treats NoOp execution status as success and uses the computed success for the final terminal status line.
libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts Adds regression test ensuring a phased command succeeds when a plugin returns an empty operation set, and asserts no tasks were spawned.
libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/rush-plugin-manifest.json Adds manifest for a mock plugin that clears operations.
libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/package.json Adds package metadata for the mock plugin.
libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts Implements mock plugin that taps createOperationsAsync at a very late stage and returns an empty Set<Operation>().
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/rush.json Adds a new unit-test repo fixture for the regression scenario.
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/config/rush/rush-plugins.json Configures the fixture to load the mock plugin via an autoinstaller.
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/package.json Defines the fixture autoinstaller dependency on the mock plugin (file: reference).
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/common/autoinstallers/plugins/rush-plugins/rush-mock-clear-operations-plugin/rush-plugin-manifest.json Stages the plugin manifest in the fixture’s autoinstaller plugin manifest folder.
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/a/package.json Adds fixture project “a” with build/rebuild scripts for baseline behavior.
libraries/rush-lib/src/cli/test/clearOperationsAndRunBuildActionRepo/b/package.json Adds fixture project “b” with build/rebuild scripts for baseline behavior.
libraries/rush-lib/config/heft.json Copies the built mock plugin output into the fixture autoinstaller node_modules location (matching the existing mock plugin pattern).
common/changes/@microsoft/rush/fix-noop-exit-code_2026-07-28-18-56-00.json Records the patch change for @microsoft/rush.

Review details

  • Files reviewed: 13/13 changed files
  • Comments generated: 0
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

  • Files reviewed: 13/13 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread libraries/rush-lib/src/cli/test/rush-mock-clear-operations-plugin/index.ts Outdated
Comment thread libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts Outdated
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>
@TheLarkInn
Sean Larkin (TheLarkInn) merged commit 67e113d into main Aug 5, 2026
9 checks passed
@TheLarkInn
Sean Larkin (TheLarkInn) deleted the fix/phased-command-noop-exit-code branch August 5, 2026 14:23
@github-project-automation github-project-automation Bot moved this from Needs triage to Closed in Bug Triage Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

rush <phased-command> --drop-graph exits 1 even though the graph is fine

4 participants