[rush-daemon] Terminate running operations when a daemon client cancels - #6065
Open
Sean Larkin (TheLarkInn) wants to merge 5 commits into
Open
Sean Larkin (TheLarkInn) wants to merge 5 commits into
Sean Larkin (TheLarkInn) wants to merge 5 commits into
Conversation
Fixes #6060 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sean Larkin (TheLarkInn)
force-pushed
the
thelarkinn-fix-rushd-ctrl-c-cancellation
branch
from
September 24, 2026 02:34
2b2d6c4 to
af2f5c5
Compare
Sean Larkin (TheLarkInn)
marked this pull request as ready for review
September 24, 2026 02:39
Sean Larkin (TheLarkInn)
enabled auto-merge (squash)
September 24, 2026 17:12
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Cancellation still has process-tree, result-classification, stale-status, and batch-joining race defects.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds prompt hard cancellation for daemon-routed Rush operations and signal-correct client exits.
Changes:
- Terminates running shell process trees when the final client cancels.
- Immediately detaches cancelled clients from shared batches.
- Maps SIGINT, SIGTERM, and SIGHUP to conventional exit codes.
| File | Description |
|---|---|
libraries/rush-lib/src/logic/operations/test/ShellOperationRunnerAbort.test.ts |
Tests hard and soft abort behavior. |
libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts |
Terminates shell processes on abort. |
libraries/rush-lib/src/logic/operations/OperationGraph.ts |
Adds opt-in hard-abort signaling. |
libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts |
Exposes the termination signal. |
libraries/rush-lib/src/logic/operations/IOperationRunner.ts |
Documents runner cancellation support. |
libraries/rush-lib/src/logic/operations/IOperationGraph.ts |
Extends the abort API. |
libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts |
Enables hard abort for engine graphs. |
libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts |
Extends cancellation test fixtures. |
libraries/rush-daemon/src/test/PhasedRequestCancellation.test.ts |
Tests single and shared-client cancellation. |
libraries/rush-daemon/src/test/PhasedRequestBatching.test.ts |
Updates immediate-detachment expectations. |
libraries/rush-daemon/src/PhasedRequestRouter.ts |
Implements hard abort and client detachment. |
common/reviews/api/rush-lib.api.md |
Updates the API report. |
common/changes/@rushstack/rush-daemon/rushd-cancel-hard-abort_2026-09-23.json |
Records the daemon patch. |
common/changes/@rushstack/rush-cli-client/rushd-cancel-hard-abort_2026-09-23.json |
Records the client patch. |
common/changes/@microsoft/rush/rushd-cancel-hard-abort_2026-09-23.json |
Records the Rush API change. |
apps/rush-cli-client/src/test/persistentIpcCancellation.test.ts |
Updates integration expectations. |
apps/rush-cli-client/src/test/launchClient.test.ts |
Tests cancellation classification and codes. |
apps/rush-cli-client/src/launchClient.ts |
Handles signals and cancellation outcomes. |
apps/rush-cli-client/src/clientCancellation.ts |
Adds cancellation utilities. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… aborted flag, detach pre-execution cancellations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ctrl-c-cancellation # Conflicts: # libraries/rush-daemon/src/PhasedRequestRouter.ts
Mo Jazayeri (mojaza)
approved these changes
Sep 24, 2026
…ctrl-c-cancellation # Conflicts: # apps/rush-cli-client/src/launchClient.ts
… in native engine tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Ctrl+C / SIGTERM on a daemon-routed
rush-client builddidn't stop the running operations. The client waited 5 s ("Daemon did not finish cancellation") and exited1. The operation kept running to completion (~26 s in the repro), and other clients stayed blocked on the execution lease. This PR adds a hard-abort path for client cancellation and fixes the client's exit code and message.This came out of the automated rushd Linux (WSL Ubuntu-24.04) analysis (board bugs #79 and #82).
Root cause
PhasedRequestRouterhandledrequestCancelwithOperationGraph.abortCurrentIterationAsync(). That is a soft abort: the signal is only checked before starting the next operation, so running processes were never killed.{ outcome: 'aborted', exitCode: 1 }(CommandResultPolicy), andlaunchClientcopied that exit code. The130branch only covered non-result outcomes.Fix
rush-lib
IOperationGraphOptions.supportsTerminateRunning, enabled only for daemon engine graphs, so nativerushis unchanged.abortCurrentIterationAsync({ terminateRunning: true }), which also aborts a per-iteration terminate signal exposed asIOperationRunnerContext.abortSignal.ShellOperationRunner: whenabortSignalis present, it spawns withconnectSubprocessTerminator. On abort it callsSubprocessTerminator.killProcessTree(the same mechanismGlobalCommandExecutionContextuses) and reportsOperationStatus.Aborted. Aborted operations write no cache entry and keep no retained result.rush-daemon
terminateRunning: true).abortedresult right away (operations still in flight are reported as Aborted), and the work keeps running for everyone else. The batch release barrier ignores detached entries.rush-cli-client
128 + signo(SIGINT → 130, SIGTERM → 143, SIGHUP → 129) and printsrush-client: build cancelled.to stderr.Tests
rush-libShellOperationRunnerAbort.test.ts:supportsTerminateRunning, the soft behaviour is kept.rush-daemonPhasedRequestCancellation.test.ts:PhasedRequestBatching.test.ts: two tests were updated for the immediate-detach behaviour.rush-cli-clientlaunchClient.test.ts: new exit-code and cancellation-outcome tests.persistentIpcCancellation.test.tsnow expects 130 and the cancellation message.Linux validation (WSL Ubuntu-24.04)
Repro: the A08
signals6.shflow on a synthetic 12-project workspace.p02sleeps 30 s, the client gets SIGINT at t=8 s, and a second client runsbuild --only p05at t=9 s.Daemon did not finish cancellation; disconnected…rush-client: build cancelled.p02process treeKnown unrelated CI failures
After merging current main (7a0348d), three rush-daemon tests fail on main itself, and they fail the same way on pristine main without this PR:
PhasedRequestBatching› "publishes an early failure result while the batch continues for other clients"PhasedRequestBatching› "derives shared and disjoint failure results from each client subset"DaemonRequestWirePhased› "merges two connections into one shared iteration with subset-specific results"They throw
StdioSummarizer: The summary cannot be prepared until after close() is called. #6068 writes the operation summary when a client's result is produced. #6092 now produces a coalesced client's result early, from insideonOperationCompleted, before that operation's output summary is closed. The two changes conflict at runtime even though they didn't conflict in the diff. A separate fix PR is in progress; I'll merge main into this branch again once it lands.Follow-ups
IPCOperationRunner(persistent IPC runners) does not observeabortSignalyet, so hard abort doesn't apply to them.Fixes #6060