Skip to content

[rush-daemon] Run rushx scripts without exclusive admission and complete on child exit - #6091

Merged
Sean Larkin (TheLarkInn) merged 4 commits into
mainfrom
thelarkinn-fix-rushx-exclusive-admission-and-hang
Sep 24, 2026
Merged

Sean Larkin (TheLarkInn) merged 4 commits into
mainfrom
thelarkinn-fix-rushx-exclusive-admission-and-hang

Conversation

@TheLarkInn

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

Copy link
Copy Markdown
Member

Summary

Two rushx-via-daemon problems from #6085:

  1. Every rushx script took exclusive workspace admission for its whole lifetime. Concurrent scripts ran one after another, and builds queued behind a long script failed with a wait timeout.
  2. rushx-client hung when a script left a background descendant holding stdout/stderr (for example (sleep 120 &); echo started). Also, cancelling after the direct child had exited did not kill the child's process group.

Root cause

  • DaemonRequestDispatcher routes rushx as commandOrigin: 'custom'. classifyRushCommand fails closed to Exclusive for non-built-in commands, and GlobalCommandRequestRouter.executeAsync held that lease until the script finished.
  • GlobalCommandExecutionContext.#trackChildAsync completed only on the child's 'close'. That event waits for every holder of the pipes, including background descendants. On abort it called SubprocessTerminator.killProcessTree, which returns early once exitCode is set, so the surviving group was never signalled.

Fix

  • Admission: IResolve(d)GlobalCommandRequest now carries invocationKind (the dispatcher sets it from the envelope). For rushx, the router skips RequestScheduler admission.
    • Reasoning: preparation (config check, environment, lifecycle resolution) already happens in RushXDaemonRequestResolver before the router runs, and the script only runs a package script. Native rushx takes no workspace lock either.
    • Switching to a shared class would not help, because RequestScheduler serializes different shared classes and would still block builds.
    • The workspace fences stay in place: assertWorkspaceRequestResourcesHealthy and assertActive still run before execution. Repository-defined rush custom commands stay Exclusive.
  • Completion (POSIX): a tracked child now completes on 'exit'. Its process group is then SIGKILLed right away (this is the existing "owned group" policy; the kill used to happen after 'close').
    • After that, forwarded output gets a bounded drain. Pipes are destroyed only after 250 ms with no forwarding progress and no pending terminal writes, so a slow client does not truncate output.
    • Output that the caller consumes itself (forwardOutput: false) is still awaited until 'close' or abort.
    • Windows keeps 'close'-based completion. Windows cannot recover a child's descendants after the child exits, so the pipes closing is the only signal that, for example, native install/update workers have finished. An earlier revision completed on 'exit' on Windows too, and SuccessfulNativeMutation failed with EBUSY in CI.
  • Cancel: the new #terminateChild signals the process group directly (process.kill(-pid, 'SIGKILL')) when the direct child has already exited, and uses killProcessTree otherwise.

Behaviour change: rushx queue flags

Because rushx scripts no longer take workspace admission, --no-wait and --wait-timeout have no effect on a rushx script. They are still forwarded in the request and still never reach the script's argv, but the script no longer queues or fails behind an exclusive Rush request.

Three rush-cli-client tests were written for the old queueing behaviour, and CI failed on them. I updated them to check the new behaviour:

  • pipedInput: queue flags.
  • RushXDaemonBoundaries: hooks changed while queued.
  • RushXDaemonAlias: alias retargeted while queued.

The executor still re-checks config and cwd, as before.

Tests

New tests in GlobalCommandRequestRouter.test.ts:

  • Two rushx requests run concurrently while an exclusive lease is held. In the same setup, a rush custom command with noWait still fails admission.
  • (POSIX) A request completes when a detached (setsid) grandchild holds stdout/stderr.
  • (POSIX) Cancelling after the direct child has exited signals -pid and does not call killProcessTree.

The three rush-cli-client tests above were updated. rush test for @rushstack/rush-daemon and @rushstack/rush-cli-client is green on Linux.

Linux validation (WSL Ubuntu-24.04)

lab 'fix-sync rushx-admission && cd $LAB/fixes/rushx-admission && rush build --to @rushstack/rush-cli-client && rush test --only @rushstack/rush-daemon --only @rushstack/rush-cli-client'   # green, incl. lint + API report
lab 'bash $LAB/fixes/rushx-admission-repro.sh'   # synthetic 12-project ws, RUSH_DAEMON=1, before=main toolchain, after=this branch
scenario before (main) after
2 concurrent rushx-client slow (sleep 10) 10.8 s / 20.8 s 10.7 s / 10.7 s
rushx-client bg ((sleep 60 &); echo started) 60.7 s 1.4 s (exit 0, output intact)

Follow-ups (not in this PR)

Fixes #6085

This came out of the automated rushd Linux analysis (Rushd Hive; board bugs #155, #156).

…ete on child exit

Fixes #6085

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Windows cannot recover a child's descendants after it exits, so closing pipes remain the only signal that native install/update workers have finished.

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

Context-wide output progress can prevent an exited child’s drain timeout when another child continues producing output.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Updates rushd so Rushx scripts bypass workspace admission and complete safely when their direct child exits.

Changes:

  • Bypasses admission for explicit Rushx invocations.
  • Adds POSIX child-exit completion, bounded output draining, and process-group termination.
  • Updates integration tests for non-queued Rushx behavior.
File Description
libraries/​rush-daemon/​src/​GlobalCommandExecutionContext.ts Implements child-exit cleanup and output draining.
libraries/​rush-daemon/​src/​GlobalCommandRequestRouter.ts Skips admission for Rushx requests.
libraries/​rush-daemon/​src/​GlobalCommandRequest.ts Carries resolved invocation kind.
libraries/​rush-daemon/​src/​DaemonRequestDispatcher.ts Propagates invocation kind.
libraries/​rush-daemon/​src/​test/​GlobalCommandRequestRouter.test.ts Tests admission, draining, and cancellation.
apps/​rush-cli-client/​src/​test/​RushXDaemonBoundaries.test.ts Verifies scripts bypass exclusive requests.
apps/​rush-cli-client/​src/​test/​RushXDaemonAlias.test.ts Updates alias behavior coverage.
apps/​rush-cli-client/​src/​test/​pipedInput.test.ts Updates queue-flag expectations.
common/​reviews/​api/​rush-daemon.api.md Records the API surface change.
common/​changes/​@rushstack/​rush-daemon/​rushx-admission-6085_2026-09-24.json Adds the change description.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libraries/rush-daemon/src/GlobalCommandExecutionContext.ts Outdated
Comment thread libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@TheLarkInn
Sean Larkin (TheLarkInn) merged commit 8c34d13 into main Sep 24, 2026
18 of 22 checks passed
@TheLarkInn
Sean Larkin (TheLarkInn) deleted the thelarkinn-fix-rushx-exclusive-admission-and-hang branch September 24, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Closed

3 participants