[rush-daemon][WS2.7][6/9] Forward interactive I/O - #6
Closed
mojaza wants to merge 2 commits into
Closed
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved issues remain with request-local sink failures, concurrent raw-mode ownership, and consistent asynchronous error handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds opt-in interactive I/O support for rushd, including request-scoped stdin, raw-mode controls, terminal policies, and serialized transport handling.
Changes:
- Adds interactive protocol contracts and codecs.
- Integrates input and terminal lifecycle handling.
- Adds transport, router, protocol, and documentation updates.
File summaries
| File | Description |
|---|---|
libraries/rush-daemon/src/test/RushDaemonHost.test.ts |
Host interactive I/O tests |
libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts |
Interactive test utilities |
libraries/rush-daemon/src/test/PhasedRequestInteractive.test.ts |
Phased interactive tests |
libraries/rush-daemon/src/test/InteractiveRequestInputRouter.test.ts |
Input routing tests |
libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts |
Global interactive tests |
libraries/rush-daemon/src/test/DaemonInteractiveConnection.test.ts |
Connection lifecycle tests |
libraries/rush-daemon/src/RushDaemonHost.ts |
Interactive connection hosting |
libraries/rush-daemon/src/PhasedRequestRouter.ts |
Phased interactive lifecycle |
libraries/rush-daemon/src/PhasedRequestClient.ts |
Phased client contract |
libraries/rush-daemon/src/InteractiveRequestInputRouter.ts |
Request-scoped input routing |
libraries/rush-daemon/src/index.ts |
Daemon API exports |
libraries/rush-daemon/src/GlobalCommandRequestRouter.ts |
Global interactive lifecycle |
libraries/rush-daemon/src/GlobalCommandRequestClient.ts |
Global client contract |
libraries/rush-daemon/src/GlobalCommandRequest.ts |
Terminal request properties |
libraries/rush-daemon/src/GlobalCommandExecutionContext.ts |
Child stdin forwarding |
libraries/rush-daemon/src/DaemonTerminalPolicy.ts |
Terminal policy implementation |
libraries/rush-daemon/src/DaemonInteractiveConnection.ts |
Connection interactive services |
libraries/rush-daemon/src/DaemonControlSession.ts |
Interactive frame routing |
libraries/rush-daemon/README.md |
Interactive behavior documentation |
libraries/rush-daemon-transport/src/test/AsyncFrameHandler.test.ts |
Async transport tests |
libraries/rush-daemon-transport/src/DaemonFrameConnection.ts |
Serialized frame handling |
libraries/rush-daemon-transport/README.md |
Inbound flow-control documentation |
libraries/rush-daemon-protocol/src/test/StdinFrameCodec.test.ts |
Stdin codec tests |
libraries/rush-daemon-protocol/src/test/InteractiveControl.test.ts |
Interactive control tests |
libraries/rush-daemon-protocol/src/test/ControlFrame.test.ts |
Control frame tests |
libraries/rush-daemon-protocol/src/StdinFrameCodec.ts |
Request-tagged stdin encoding |
libraries/rush-daemon-protocol/src/InteractiveControlValidation.ts |
Interactive payload validation |
libraries/rush-daemon-protocol/src/index.ts |
Protocol exports |
libraries/rush-daemon-protocol/src/FrameConstants.ts |
Stdin framing constants |
libraries/rush-daemon-protocol/src/DaemonTerminalPolicy.ts |
Terminal policy types |
libraries/rush-daemon-protocol/src/DaemonProtocolVersion.ts |
Protocol version update |
libraries/rush-daemon-protocol/src/DaemonPhasedRequest.ts |
Phased terminal requirements |
libraries/rush-daemon-protocol/src/DaemonInteractiveControl.ts |
Interactive wire contracts |
libraries/rush-daemon-protocol/src/DaemonControlMessage.ts |
Control message union |
libraries/rush-daemon-protocol/src/DaemonControlKinds.ts |
Control kind registry |
libraries/rush-daemon-protocol/src/DaemonClientCaps.ts |
Client capability declaration |
libraries/rush-daemon-protocol/src/ControlMessageValidation.ts |
Control message validation |
libraries/rush-daemon-protocol/README.md |
Protocol documentation |
common/reviews/api/rush-daemon.api.md |
Daemon API report |
common/reviews/api/rush-daemon-transport.api.md |
Transport API report |
common/reviews/api/rush-daemon-protocol.api.md |
Protocol API report |
common/changes/@rushstack/rush-daemon/mojazayeri-interactive-io_2026-08-21-21-00.json |
Daemon change entry |
common/changes/@rushstack/rush-daemon-transport/mojazayeri-interactive-io_2026-08-21-21-00.json |
Transport change entry |
common/changes/@rushstack/rush-daemon-protocol/mojazayeri-interactive-io_2026-08-21-21-00.json |
Protocol change entry |
Review details
Suppressed comments (5)
libraries/rush-daemon/src/DaemonInteractiveConnection.ts:144
- If a request is cancelled after the
setRawMode(true)frame has been sent but before its acknowledgement arrives, the waiter is removed and cleanup queuessetRawMode(false)with the same request ID. A client that received the first frame can still acknowledgetrue; this code then treats that valid late acknowledgement as fatal (the pending entry is either absent or forfalse) and closes the connection before restoration completes. Keep ordered/tombstoned acknowledgements or otherwise ignore stale acknowledgements from this cancellation race.
#acknowledgeRawMode(message: IDaemonRawModeChangedMessage): void {
const acknowledgement: IRawModeAcknowledgement | undefined =
this.#pendingRawModeByRequestId.get(message.payload.requestId);
if (!acknowledgement || acknowledgement.enabled !== message.payload.enabled) {
throw new Error(`Unexpected raw-mode acknowledgement for request "${message.payload.requestId}".`);
libraries/rush-daemon/src/GlobalCommandRequestRouter.ts:94
- This branch runs before the normal global execution cancellation handling. If the client is already aborted but the request declares
controllingTerminal, the router still sendsrequiresInProcessand throws, allowing a thin client to launch fallback for a canceled request rather than publishing an aborted result. Check cancellation before announcing terminal fallback and preserve that precedence across the cleanup await.
libraries/rush-daemon/src/GlobalCommandRequestRouter.ts:87 - Interactive-session validation runs before terminal policy evaluation. A request that accepts stdin and requires a controlling terminal may legitimately have no daemon input session because it must fall back to in-process execution, but this call throws before the
requiresInProcesspolicy can be emitted. Evaluate the policy first or skip the session requirement for requests rejected by that policy.
libraries/rush-daemon/src/PhasedRequestRouter.ts:95 - This branch runs before the existing cancellation/admission path. If a phased request is already aborted but declares
controllingTerminal, it still publishesrequiresInProcessand throws, so the client may start an in-process fallback for a canceled request instead of receiving the normal aborted result. Check cancellation before announcing terminal fallback (and preserve that precedence across the cleanup await).
libraries/rush-daemon/src/PhasedRequestRouter.ts:87 - Interactive-session validation runs before terminal policy evaluation. A request that accepts stdin and requires a controlling terminal may legitimately have no daemon input session because it must fall back to in-process execution, but this call throws before the
requiresInProcesspolicy can be emitted. Evaluate the policy first or skip the session requirement for requests rejected by that policy.
- Files reviewed: 44/44 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.
| 'The first frame on a connection must be a hello control message.' | ||
| ); | ||
| } | ||
| await this._interactiveConnection.routeStdinFrameAsync(frame.payload); |
| export class DaemonInteractiveConnection implements IDaemonInteractiveConnection { | ||
| readonly #abortController: AbortController = new AbortController(); | ||
| readonly #inputRouter: InteractiveRequestInputRouter = new InteractiveRequestInputRouter(); | ||
| readonly #pendingRawModeByRequestId: Map<string, IRawModeAcknowledgement> = new Map(); |
Comment on lines
+151
to
+152
| public routeStdinFrameAsync(payload: Uint8Array): Promise<void> { | ||
| const { chunk, requestId } = decodeDaemonStdinChunk(payload); |
mojaza
force-pushed
the
mojazayeri-microsoft-rushd-ws2-interactive-io
branch
from
August 24, 2026 01:01
f42efe9 to
b6db096
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
mojaza
force-pushed
the
mojazayeri-microsoft-rushd-ws2-interactive-io
branch
from
August 24, 2026 20:47
b6db096 to
7cfc9f0
Compare
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 request-scoped interactive input and terminal-control contracts for rushd, advancing microsoft#5897 after WS2.6.
Depends on #5. This stack follows merged microsoft#5949. PBI: https://onedrive.visualstudio.com/EFun/_workitems/edit/3216023
Details
requiresInProcessfor commands needing a real controlling terminal.Limitations
SIGWINCH.How it was tested
node common/scripts/install-run-rush.js test --only @rushstack/rush-daemon-protocol --only @rushstack/rush-daemon-transport --only @rushstack/rush-daemonnode common/scripts/install-run-rush.js change --verify