[rush-daemon][WS2.5][4/9] Isolate global command context - #3
Open
mojaza wants to merge 1 commit into
Open
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete correctness/test-isolation issues (child tracking promise handling/leak, untrusted env validation robustness, and a lingering Jest spy) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the WS2.5 “global command” execution seam to @rushstack/rush-daemon, enabling integrations to run caller-resolved global command logic through a per-request execution context that avoids mutating daemon-wide process/terminal globals.
Changes:
- Introduces
GlobalCommandRequestRouterplus request validation/snapshotting (cwd, environment snapshot, terminal properties) and request result shape. - Adds
GlobalCommandExecutionContextwith request-scoped terminal output routing/backpressure, cancellation, child-process spawning with environment overlays, and async resource cleanup. - Exposes the new public surface via the package barrel/API report and adds focused tests + README documentation + a change file.
File summaries
| File | Description |
|---|---|
| libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts | Adds coverage for concurrent isolation, env snapshotting, cancellation/cleanup, disconnect behavior, and request validation. |
| libraries/rush-daemon/src/index.ts | Re-exports the new WS2.5 global-command router/context request types from the package entrypoint. |
| libraries/rush-daemon/src/GlobalCommandRequestRouter.ts | Implements the router that validates requests, constructs the execution context, and ensures cleanup/error aggregation. |
| libraries/rush-daemon/src/GlobalCommandRequestClient.ts | Defines the per-request client contract for abort + terminal chunk backpressure. |
| libraries/rush-daemon/src/GlobalCommandRequest.ts | Validates/canonicalizes cwd within workspace and snapshots environment + terminal properties for safe execution. |
| libraries/rush-daemon/src/GlobalCommandExecutionContext.ts | Provides the isolated execution context: terminal routing, child spawning, cancellation, and LIFO async disposal. |
| libraries/rush-daemon/README.md | Documents the new opt-in global-command seam and its limitations/expectations. |
| common/reviews/api/rush-daemon.api.md | Updates the public API report to include the new global-command types. |
| common/changes/@rushstack/rush-daemon/mojazayeri-global-command-context_2026-08-21-18-44.json | Declares a minor change entry for the new opt-in API surface. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+215
to
+218
| const trackedChild: ITrackedChild = { | ||
| completion: this.#trackChildAsync(child) | ||
| }; | ||
| this.#trackedChildren.add(trackedChild); |
Comment on lines
+152
to
+156
| function validateEnvironmentValue(name: string, value: string): void { | ||
| if (value.includes('\0')) { | ||
| throw new Error(`The global command environment variable "${name}" contains a null character.`); | ||
| } | ||
| } |
Comment on lines
+278
to
+282
| await expect(resultPromise).resolves.toEqual({ aborted: true, requestId: 'cancelled' }); | ||
| expect(resourceDisposed).toBe(true); | ||
| expect(killProcessTreeSpy).toHaveBeenCalledTimes(1); | ||
| expect(process.cwd()).toBe(processCwd); | ||
| expect(process.env.RUSHD_CONTEXT_TEST).toBe(processEnvironmentValue); |
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
Adds the opt-in WS2.5 global-command execution seam for rushd, with validated per-request cwd, immutable environment snapshots/overlays, terminal capabilities, cancellation, and isolated child-process propagation.
Depends on #2. This stack follows merged microsoft#5949 and advances microsoft#5897. Azure PBI: https://onedrive.visualstudio.com/EFun/_workitems/edit/3216021
Details
WorkspaceSessionthroughIGlobalCommandExecutionContext, without changing daemon-wide cwd, environment, stdin/stdout/stderr, or terminal globals.Limitations
This layer intentionally does not parse commands or directly run arbitrary existing
RushCommandLineParser/BaseRushActioninstances, because those APIs still consult or mutate process-global state. Integrations must supply already resolved command logic that consumes the injected context. The remaining action/engine adaptation boundary is tracked by open microsoft#5895.Exit-code and warnings-as-errors parity (WS2.6), stdin/raw-mode/PTY fallback (WS2.7), scheduler classification/admission (WS2.8), shared-build merging (WS2.9), lifecycle/restart, and CLI cutover are out of scope.
How it was tested
rush test --only @rushstack/rush-daemonrushx buildinlibraries/rush-daemon