Skip to content

[rush-client-core] Reclaim provably stale daemon artifacts; make daemon stop/restart idempotent; add daemon stop --force - #6080

Merged
Sean Larkin (TheLarkInn) merged 4 commits into
mainfrom
thelarkinn-fix-rushd-recovery-and-stop-restart
Sep 24, 2026
Merged

Sean Larkin (TheLarkInn) merged 4 commits into
mainfrom
thelarkinn-fix-rushd-recovery-and-stop-restart

Conversation

@TheLarkInn

Copy link
Copy Markdown
Member

Summary

Stale or foreign daemon artifacts no longer disable the opt-in Rush daemon for a workspace permanently, and the rush-client daemon management commands now handle "no daemon running" cleanly.

  • Self-healing startup (@rushstack/rush-client-core): startup now reclaims leftover artifacts when it can prove they are safe to remove:
    • a socket without pid.json;
    • a corrupt or unparseable pid.json;
    • on Linux, a pid.json whose PID has been reused by an unrelated process.
  • rush-client daemon stop: exits 0 with {"state":"notRunning"} when no daemon is listening. Before this change it exited 1 with "Could not connect".
  • rush-client daemon restart: starts a daemon when none is running, the same as daemon start.
  • New rush-client daemon stop --force: backed by the new resetDaemonArtifactsAsync() API. It removes this workspace's pid.json, .sock and .starting files after checking that no listener is bound and no live owner holds the record. Every fail-closed startup message now points to it.

Root cause

  • assertNoLiveOwner / readDaemonOwnership failed closed on all three artifact states. waitForHandoffAsync also polled a live but unrelated PID until the 15 s deadline, so each command paid about 16.5 s before falling back to in-process Rush.
  • daemonCommands.ts always used DaemonClient.connectAsync for stop and restart and let the connection error through.

Fix

  • New ProcessStartTime.ts (Linux only): computes a process's wall-clock start time from /proc/<pid>/stat field 22 and /proc/uptime.
    • Before trusting the result, it checks the calculation against this process's own performance.timeOrigin. If that check fails (unexpected USER_HZ, a clock jump, or a non-standard /proc), the start time is treated as unknown.
    • A PID counts as reused only if its process started more than 2 s after the record's startedAt. When the start time is unknown, behaviour stays fail-closed as before.
  • New DaemonOwnership.ts, with reclaimAbandonedOwnershipAsync. It runs only while holding the start mutex with no .starting reservation, so no legitimate daemon can be between binding its socket and writing its record.
    • A socket-only leftover, a corrupt record, or a reused-PID record is reclaimed only after a raw connect attempt fails with ECONNREFUSED/ENOENT, which proves nothing is listening.
    • The record is removed only if its contents have not changed since they were read. The existing transport reclaim then removes the socket.
    • A live owner whose PID has not been shown to be reused still fails closed; no process is ever killed.
  • waitForHandoffAsync and waitForPreviousDaemonAsync stop waiting on an owner whose PID is shown to be reused.
  • Coordination with FIX-startup-wedge ([rush] rush-client-core: a failed or slow daemon startup leaves a durable .starting reservation that wedges the workspace (every command waits ~16 s, daemon start refuses); an invalid RUSH_* env value triggers it and hides the real error #6050): automatic startup does not change how .starting reservations are handled. The only code that removes a .starting file is the explicit, user-invoked stop --force.
  • READMEs updated: the rush-cli-client Management section and the rush-client-core startup section.

Tests

  • connectOrStartDaemon.test.ts now covers each artifact state:
    • sock-only → recovers;
    • corrupt record → recovers;
    • corrupt record with something still listening → fails closed, record kept;
    • live PID whose process started after startedAt → recovers without waiting out the deadline, and the unrelated process is untouched;
    • live PID that may still own the record → fails closed with the --force hint, and resetDaemonArtifactsAsync refuses too;
    • force reset removes the record, the reservation and the stale socket.
  • ProcessStartTime.test.ts: start-time estimate, reuse detection, and unknown or invalid inputs.
  • launchClient.test.ts: stop and stop --force with no daemon (notRunning, exit 0); restart with no daemon starts one; stop --force on the leftovers of a killed daemon (state: "reset", removedPaths).
  • Linux (WSL Ubuntu 24.04), on a heavily loaded host (load average around 70):
    • rush-client-core: 83/83 tests pass;
    • rush-cli-client launchClient: 16/16 pass;
    • rush build --to @rushstack/rush-cli-client (including lint) passes.

Linux validation

The script combines A05's s68.sh S8a–c and s1.sh. It ran as one lab invocation on a 12-project synthetic workspace: first with the unfixed toolchain rush-client, then with the fixed apps/rush-cli-client/bin/rush-client. Timings are noisy because the host was heavily loaded.

Scenario Before After
S8a: .sock with no pid.json falls back every time, 5.1 s / 5.5 s first build reclaims and auto-starts (8.0 s, cold); next build 2.0 s on the warm daemon
S8b: corrupt pid.json falls back every time, 4.4 s / 4.6 s first build reclaims (10.0 s, cold); next build 1.8 s warm
S8c: pid.json names a live unrelated PID 19.4 s / 19.0 s per command, then falls back first build reclaims (8.7 s, cold); next build 1.7 s warm; the unrelated PID is untouched
stop --force on kill -9 leftovers usage error, exit 1 {"state":"reset","removedPaths":[pid.json, sock]}, exit 0
stop with no daemon "Could not connect", exit 1 {"state":"notRunning"}, exit 0
restart with no daemon "Could not connect", exit 1, nothing started {"state":"ready",...}, exit 0; status then reports ready

Optional follow-ups

  • Detect PID reuse on macOS as well (for example with ps -o etime=). Today macOS and Windows still fail closed and point to stop --force.
  • Have daemon status suggest rush-client daemon start when nothing is listening.

Fixes #6061

This change came out of the automated rushd Linux behaviour/performance analysis (the "Rushd Hive", board bugs #77 and #78).

…on stop/restart idempotent

Fixes #6061

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

Copy link
Copy Markdown
Member Author

CI note: the Node.js v24 (windows-latest) failure in run 35947334840 looks like runner flakiness, not this change, so I re-ran the failed job once.

  • \WindowsOwnershipRead.test: both cases hit Jest's default 5 s timeout. The same test passed with this change on Node.js v26 (windows-latest) in the same run, and on green main it takes 3.8–5.2 s for both cases, which is already close to the limit.
  • I also reproduced both cases locally on Windows against this build: 1.0 s (release) and 0.14 s (persistent denial).
  • The
    ush-reporter\ failures (ReporterHost, AiReporterQualification, FileReporter) are in a package this PR doesn't touch, and are consistent with the existing Windows flakiness on main.

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

Malformed live-PID records can still wedge startup, and one force-stop invocation may leave the startup reservation behind.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Adds self-healing daemon artifact recovery and more resilient lifecycle commands.

Changes:

  • Detects stale ownership, including Linux PID reuse.
  • Makes stop/restart idempotent and adds stop --force.
  • Adds tests, documentation, API reports, and change records.
File Description
libraries/​rush-client-core/​src/​ProcessStartTime.ts Implements Linux process start-time detection.
libraries/​rush-client-core/​src/​test/​ProcessStartTime.test.ts Tests start-time and PID-reuse detection.
libraries/​rush-client-core/​src/​DaemonOwnership.ts Adds artifact inspection, reclaim, and reset APIs.
libraries/​rush-client-core/​src/​connectOrStartDaemon.ts Integrates stale-ownership recovery into startup.
libraries/​rush-client-core/​src/​test/​connectOrStartDaemon.test.ts Covers stale and conflicting artifact states.
libraries/​rush-client-core/​src/​index.ts Exports the reset API.
libraries/​rush-client-core/​README.md Documents recovery behavior.
common/​reviews/​api/​rush-client-core.api.md Records the new public API.
apps/​rush-cli-client/​src/​daemonCommands.ts Implements idempotent lifecycle commands and force reset.
apps/​rush-cli-client/​src/​test/​launchClient.test.ts Tests stop, restart, and force-reset behavior.
apps/​rush-cli-client/​README.md Documents lifecycle command semantics.
common/​changes/​@rushstack/​rush-client-core/​daemon-recovery_2026-09-23.json Adds the core package change record.
common/​changes/​@rushstack/​rush-cli-client/​daemon-recovery_2026-09-23.json Adds the CLI package change record.

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

Comment thread apps/rush-cli-client/src/daemonCommands.ts
Comment thread libraries/rush-client-core/src/connectOrStartDaemon.ts Outdated
Comment thread libraries/rush-client-core/src/DaemonOwnership.ts
…shutdown, validate records before waiting, reset hint on unresolved handoff

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@TheLarkInn
Sean Larkin (TheLarkInn) merged commit 46441da into main Sep 24, 2026
10 checks passed
@TheLarkInn
Sean Larkin (TheLarkInn) deleted the thelarkinn-fix-rushd-recovery-and-stop-restart branch September 24, 2026 19:08
@github-project-automation github-project-automation Bot moved this from Needs triage to Closed in Bug Triage Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

[rush] rush-cli-client: daemon recovery and management gaps (stale artifacts permanently disable the daemon; daemon stop/restart are not idempotent)

3 participants