Skip to content

[rush-daemon] Terminate running operations when a daemon client cancels - #6065

Open
Sean Larkin (TheLarkInn) wants to merge 5 commits into
mainfrom
thelarkinn-fix-rushd-ctrl-c-cancellation
Open

Sean Larkin (TheLarkInn) wants to merge 5 commits into
mainfrom
thelarkinn-fix-rushd-ctrl-c-cancellation

Conversation

@TheLarkInn

@TheLarkInn Sean Larkin (TheLarkInn) commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

Ctrl+C / SIGTERM on a daemon-routed rush-client build didn't stop the running operations. The client waited 5 s ("Daemon did not finish cancellation") and exited 1. 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

  • PhasedRequestRouter handled requestCancel with OperationGraph.abortCurrentIterationAsync(). That is a soft abort: the signal is only checked before starting the next operation, so running processes were never killed.
  • The daemon returns { outcome: 'aborted', exitCode: 1 } (CommandResultPolicy), and launchClient copied that exit code. The 130 branch only covered non-result outcomes.
  • In a coalesced batch, a cancelling client waited for the whole batch to finish before getting its result.

Fix

rush-lib

  • New IOperationGraphOptions.supportsTerminateRunning, enabled only for daemon engine graphs, so native rush is unchanged.
  • New abortCurrentIterationAsync({ terminateRunning: true }), which also aborts a per-iteration terminate signal exposed as IOperationRunnerContext.abortSignal.
  • ShellOperationRunner: when abortSignal is present, it spawns with connectSubprocessTerminator. On abort it calls SubprocessTerminator.killProcessTree (the same mechanism GlobalCommandExecutionContext uses) and reports OperationStatus.Aborted. Aborted operations write no cache entry and keep no retained result.
  • Soft abort (no options) is unchanged and is still used for watch-mode invalidation.

rush-daemon

  • When the last live participant cancels, the router hard-aborts the iteration (terminateRunning: true).
  • When other live clients still need the shared work, only the cancelling client is detached. It gets its aborted result 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

  • Exits with 128 + signo (SIGINT → 130, SIGTERM → 143, SIGHUP → 129) and prints rush-client: build cancelled. to stderr.
  • SIGHUP is handled like SIGTERM.
  • A cancellation-timeout error that arrives after a signal is also treated as cancelled.

Tests

  • rush-lib ShellOperationRunnerAbort.test.ts:
    • A hard abort kills a running shell operation's process tree, including a grandchild, and reports Aborted with no retained result.
    • Without supportsTerminateRunning, the soft behaviour is kept.
  • rush-daemon PhasedRequestCancellation.test.ts:
    • When the last client cancels, the operation is terminated, the aborted result arrives in under 1 s, and the lease is released for the next request.
    • When one client in a coalesced batch cancels, it is answered immediately and the shared operation is not aborted for the other client.
  • PhasedRequestBatching.test.ts: two tests were updated for the immediate-detach behaviour.
  • rush-cli-client launchClient.test.ts: new exit-code and cancellation-outcome tests. persistentIpcCancellation.test.ts now expects 130 and the cancellation message.

Linux validation (WSL Ubuntu-24.04)

rush build --to @rushstack/rush-daemon --to @rushstack/rush-cli-client        # incl. lint: passes
rush test --only @rushstack/rush-cli-client                                   # passes
libraries/rush-daemon: heft test                                              # 365 passed, 0 failed
libraries/rush-lib: heft test --clean --test-path-pattern "ShellOperationRunner|OperationGraph"   # all pass

Repro: the A08 signals6.sh flow on a synthetic 12-project workspace. p02 sleeps 30 s, the client gets SIGINT at t=8 s, and a second client runs build --only p05 at t=9 s.

before (current toolchain) after (this PR)
client A exits t=13.07 s (5.06 s after SIGINT) t=8.15 s (0.14 s after SIGINT)
client A exit code 1 130
client A message Daemon did not finish cancellation; disconnected… rush-client: build cancelled.
p02 process tree kept running until ~t=32 s gone by the next 1 s sample (t=10.08 s)
second client done t=33.7 s (blocked behind the orphaned op) t=10.8 s

Known 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 inside onOperationCompleted, 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 observe abortSignal yet, so hard abort doesn't apply to them.
  • Add a rushd-wire-e2e signal/cancellation scenario.

Fixes #6060

Fixes #6060

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 High severity · 2 Medium severity

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.

Comment thread libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts Outdated
Comment thread apps/rush-cli-client/src/clientCancellation.ts Outdated
Comment thread libraries/rush-daemon/src/PhasedRequestRouter.ts
… 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
…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Needs triage

3 participants