[rush-client-core] Bound the daemon startup reservation and surface startup errors - #6066
Open
Sean Larkin (TheLarkInn) wants to merge 8 commits into
Open
Sean Larkin (TheLarkInn) wants to merge 8 commits into
Sean Larkin (TheLarkInn) wants to merge 8 commits into
Conversation
…tartup errors - Record owner (client, then helper) PID and start time plus the launcher PID in <lockfile>.starting; release it when the launcher exits before readiness. - Accept a ready, attested lockfile owner despite a reservation, and clean up after a dead reservation owner. - Reclaim a stale reservation (owner and launcher dead, or older than its timeout plus a 60 s grace) under the start mutex instead of spinning to the deadline. - Include the launcher log's last error lines in startup failures. - rush-daemon: validate a request's Rush environment before planning a process restart so an invalid value fails fast with the native message. - rush-cli-client: print result.errorMessage for failed results and fall back in-process when a pre-execution restart cannot start. Fixes #6050 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…vations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…t; clarify signal exits Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Daemon-specific environment values remain unvalidated, and reservation updates have a check-then-replace race.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Bounds daemon startup reservations, improves startup diagnostics, and prevents invalid request environments from unnecessarily replacing healthy daemons.
Changes:
- Adds verifiable, expiring startup reservations with launcher log diagnostics.
- Pre-validates request-scoped Rush environments.
- Prints daemon errors and safely falls back to in-process Rush.
| File | Description |
|---|---|
libraries/rush-daemon/src/WorkspaceRequestLifecycle.ts |
Validates environments before restart. |
libraries/rush-daemon/src/RushEnvironmentValidation.ts |
Implements request environment validation. |
libraries/rush-daemon/src/test/RushEnvironmentValidation.test.ts |
Tests validation behavior. |
libraries/rush-daemon/src/test/WorkspaceReloadTierStatus.test.ts |
Tests preservation of the healthy daemon. |
libraries/rush-client-core/src/DaemonStartupReservation.ts |
Adds structured, bounded reservations. |
libraries/rush-client-core/src/DaemonStartup.ts |
Tracks launcher ownership and cleanup. |
libraries/rush-client-core/src/DaemonLogFile.ts |
Formats startup log diagnostics. |
libraries/rush-client-core/src/connectOrStartDaemon.ts |
Reclaims stale reservations and accepts ready daemons. |
libraries/rush-client-core/src/test/connectOrStartDaemon.test.ts |
Tests reservation recovery scenarios. |
libraries/rush-client-core/README.md |
Documents reservation lifecycle rules. |
apps/rush-cli-client/src/launchClient.ts |
Prints failures and adds safe fallback. |
common/changes/@rushstack/rush-daemon/startup-wedge_2026-09-24-01-30.json |
Records daemon patch. |
common/changes/@rushstack/rush-client-core/startup-wedge_2026-09-24-01-30.json |
Records core patch. |
common/changes/@rushstack/rush-cli-client/startup-wedge_2026-09-24-01-30.json |
Records CLI patch. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…daemon env settings before restart Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…startup-wedge # Conflicts: # libraries/rush-client-core/src/connectOrStartDaemon.ts
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…startup-wedge # Conflicts: # apps/rush-cli-client/src/launchClient.ts # libraries/rush-daemon/src/WorkspaceRequestLifecycle.ts
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
A
<lockfile>.startingreservation that was never released wedged a workspace permanently. After that, everyrush-clientcommand waited about 16 s and then fell back to in-process Rush,daemon startrefused to run, and the real startup error was never shown. This PR puts a bound on the reservation and makes its owner checkable. It keeps the existing safety goal: one workspace never gets two daemons. It also stops an invalid request-scopedRUSH_*value from replacing a healthy daemon.Root cause
runDaemonStartupAsynckept the reservation on purpose whenever the launcher missed the deadline or exited before readiness. Nothing ever reclaimed it: the record held only a token, with no owner PID and no age limit.tryConnectAsyncrejected any endpoint while a reservation existed, including a daemon that was healthy and ready. The starter loop then waited out the full 15 s deadline.RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD=yes) caused a planned env restart.RushDaemonHostclosed the healthy daemon, and the successor threw inEnvironmentConfiguration.validate, exited with code 1, and left the reservation behind.launchClientnever printedresult.errorMessage. A restart failure also bypassed the in-process fallback.Fix
rush-client-core (new
DaemonStartupReservation.ts)createdAt, timeout, the owner PID and start time, and the launcher PID once it has been spawned. The owner is the starting client, and becomes the helper once the helper is spawned; the helper is recorded before the IPC handoff.<lockfile>.logfrom the current attempt. A live reservation's error reports the owner/launcher state. A launcher or helper killed by a signal is reported aswas terminated (SIGKILL).rush-daemon
validateRequestRushEnvironment()applies theEnvironmentConfiguration.validaterules (booleans, unknownRUSH_*names, mutually exclusive cache overrides) to the request env without touching global state.WorkspaceRequestLifecycleruns it before planning a restart and returns a pre-execution failure result with the native message. The current daemon keeps running.rush-cli-client
ERROR: <errorMessage>.executeWithDaemonRestartAsyncfails withstartupFailed, the client falls back to in-process Rush. This only happens before execution, so no work is replayed.Tests
RushEnvironmentValidation.test.ts.WorkspaceReloadTierStatus.test.ts: an invalidRUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILDreturnsexitCode: 1with the native message and noretryAfterRestart. The successor launcher is never called, and the next build runs on the same daemon PID.rush testpasses for rush-client-core, rush-daemon and rush-cli-client, andrush buildfor them reports no lint warnings.Linux validation (WSL Ubuntu 24.04, synthetic 8-project workspace)
Before (unfixed
rush-client):After (fixed client, same workspace):
A separate analysis agent also checked all four wedge triggers on Linux, and every one recovered.
Known unrelated CI failures
Three rush-daemon phased-batching tests currently fail on
main(since #6081 / #6068) withThe summary cannot be prepared until after close() is called.. They fail the same way on this branch. #6095 fixes them, and I'll mergemainagain once it lands.Fixes #6050
This PR came out of the automated rushd Linux analysis (board bug reports #16, #23 and #33).