diff --git a/common/changes/@rushstack/rush-daemon-protocol/mojazayeri-command-results_2026-08-21-19-36.json b/common/changes/@rushstack/rush-daemon-protocol/mojazayeri-command-results_2026-08-21-19-36.json new file mode 100644 index 0000000000..806de5d9dd --- /dev/null +++ b/common/changes/@rushstack/rush-daemon-protocol/mojazayeri-command-results_2026-08-21-19-36.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon-protocol", + "comment": "Add a typed final daemon command result with Rush-compatible outcome and exit-code semantics.", + "type": "minor" + } + ], + "packageName": "@rushstack/rush-daemon-protocol", + "email": "mojazayeri@users.noreply.github.com" +} diff --git a/common/changes/@rushstack/rush-daemon/mojazayeri-exit-semantics_2026-08-21-19-36.json b/common/changes/@rushstack/rush-daemon/mojazayeri-exit-semantics_2026-08-21-19-36.json new file mode 100644 index 0000000000..0753992bdb --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/mojazayeri-exit-semantics_2026-08-21-19-36.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "Add authoritative Rush-compatible command result policy and ordered exact-once final result delivery for phased and global requests.", + "type": "minor" + } + ], + "packageName": "@rushstack/rush-daemon", + "email": "mojazayeri@users.noreply.github.com" +} diff --git a/common/reviews/api/rush-daemon-protocol.api.md b/common/reviews/api/rush-daemon-protocol.api.md index 9f406ac255..e880b9645e 100644 --- a/common/reviews/api/rush-daemon-protocol.api.md +++ b/common/reviews/api/rush-daemon-protocol.api.md @@ -46,6 +46,9 @@ export const DAEMON_EVENT_TYPES: readonly [ // @beta export const DAEMON_PROTOCOL_VERSION: IDaemonProtocolVersion; +// @beta +export type DaemonCommandOutcome = 'success' | 'success-with-warning' | 'failure' | 'aborted'; + // @beta export type DaemonControlMessage = IDaemonHelloMessage | IDaemonHelloAckMessage | IDaemonSubscribeMessage | IDaemonUnsubscribeMessage | IDaemonPingMessage | IDaemonPongMessage | IDaemonErrorMessage; @@ -159,6 +162,15 @@ export interface IDaemonClientCaps { readonly verbosity?: DaemonVerbosity; } +// @beta +export interface IDaemonCommandResult { + readonly aborted: boolean; + readonly errorMessage?: string; + readonly exitCode: number; + readonly outcome: DaemonCommandOutcome; + readonly requestId: string; +} + // @beta export interface IDaemonDiagnosticPayload { // (undocumented) @@ -304,13 +316,13 @@ export interface IDaemonPhasedOperationSelection { export interface IDaemonPhasedRequest { readonly commandName: string; readonly engineShape: IDaemonPhasedEngineShape; + readonly environment: Readonly>; readonly operationSelection: ReadonlyArray; readonly requestId: string; } // @beta -export interface IDaemonPhasedRequestResult { - readonly aborted: boolean; +export interface IDaemonPhasedRequestResult extends IDaemonCommandResult { readonly operationResults: ReadonlyArray; readonly requestId: string; readonly scheduled: boolean; diff --git a/common/reviews/api/rush-daemon.api.md b/common/reviews/api/rush-daemon.api.md index f19633b3f1..17b4f625c1 100644 --- a/common/reviews/api/rush-daemon.api.md +++ b/common/reviews/api/rush-daemon.api.md @@ -8,6 +8,7 @@ import * as childProcess from 'node:child_process'; import type { GetInputsSnapshotAsyncFn } from '@microsoft/rush-lib'; +import type { IDaemonCommandResult } from '@rushstack/rush-daemon-protocol'; import type { IDaemonEventEnvelope } from '@rushstack/rush-daemon-protocol'; import type { IDaemonPaths } from '@rushstack/rush-daemon-transport'; import type { IDaemonPhasedRequest } from '@rushstack/rush-daemon-protocol'; @@ -27,7 +28,7 @@ export type CreateWorkspaceEngineComponentsAsync = (options: ICreateWorkspaceEng export type CreateWorkspaceSessionComponentsAsync = (options: ICreateWorkspaceSessionComponentsOptions) => Promise; // @beta -export type GlobalCommandExecutor = (context: IGlobalCommandExecutionContext) => Promise; +export type GlobalCommandExecutor = (context: IGlobalCommandExecutionContext) => Promise; // @beta export class GlobalCommandRequestRouter { @@ -90,19 +91,21 @@ export interface IGlobalCommandExecutionContext { readonly workspaceSession: IWorkspaceSession; } +// @beta +export interface IGlobalCommandExecutionResult { + // (undocumented) + readonly exitCode: number; +} + // @beta export interface IGlobalCommandRequestClient { readonly abortSignal: AbortSignal; + writeResultAsync(result: IDaemonCommandResult): Promise; writeTerminalChunkAsync(stream: 'stdout' | 'stderr', chunk: Uint8Array): Promise; } // @beta -export interface IGlobalCommandRequestResult { - // (undocumented) - readonly aborted: boolean; - // (undocumented) - readonly requestId: string; -} +export type IGlobalCommandRequestResult = IDaemonCommandResult; // @beta export interface IGlobalCommandSpawnOptions { @@ -145,6 +148,7 @@ export interface IPhasedRequestClient { readonly sessionId: string; writeEventAsync(event: IDaemonEventEnvelope): Promise; writeLogChunkAsync(operationId: string, stream: 'stdout' | 'stderr', chunk: Uint8Array): Promise; + writeResultAsync(result: IDaemonPhasedRequestResult): Promise; } // @public diff --git a/libraries/rush-daemon-protocol/README.md b/libraries/rush-daemon-protocol/README.md index 96a59894e1..05647aa49f 100644 --- a/libraries/rush-daemon-protocol/README.md +++ b/libraries/rush-daemon-protocol/README.md @@ -20,6 +20,8 @@ The engine-agnostic **wire layer** spoken by every client of the Rush daemon (`r - **Resolved phased-request contracts** — engine-agnostic request, enabled-state selection, and client-scoped result types for integrations that have already parsed a command and resolved it against a real warm operation graph. +- **Final command result contract** — one typed success, warning, failure, or abort outcome + with the authoritative Rush-compatible exit code, delivered after request output drains. Part of the Rush 6 / rushd re-architecture: [microsoft/rushstack#5894](https://github.com/microsoft/rushstack/issues/5894). diff --git a/libraries/rush-daemon-protocol/src/DaemonCommandResult.ts b/libraries/rush-daemon-protocol/src/DaemonCommandResult.ts new file mode 100644 index 0000000000..8f816b6cb4 --- /dev/null +++ b/libraries/rush-daemon-protocol/src/DaemonCommandResult.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +/** + * The semantic outcome of a daemon command. + * + * @beta + */ +export type DaemonCommandOutcome = 'success' | 'success-with-warning' | 'failure' | 'aborted'; + +/** + * The authoritative final result delivered after a daemon command's output has drained. + * + * @beta + */ +export interface IDaemonCommandResult { + /** Whether cancellation or disconnect was observed, even if a cleanup failure determines the outcome. */ + readonly aborted: boolean; + /** The process exit code a compatible in-process Rush invocation would return. */ + readonly exitCode: number; + /** A failure description for execution or cleanup failures that were not already operation-scoped. */ + readonly errorMessage?: string; + /** The semantic command outcome. */ + readonly outcome: DaemonCommandOutcome; + /** The identifier copied from the request. */ + readonly requestId: string; +} diff --git a/libraries/rush-daemon-protocol/src/DaemonPhasedRequest.ts b/libraries/rush-daemon-protocol/src/DaemonPhasedRequest.ts index dc0ee92028..ead42dcee1 100644 --- a/libraries/rush-daemon-protocol/src/DaemonPhasedRequest.ts +++ b/libraries/rush-daemon-protocol/src/DaemonPhasedRequest.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { IDaemonCommandResult } from './DaemonCommandResult'; + /** * The enabled state assigned to one selected operation by a phased request. * @@ -46,6 +48,8 @@ export interface IDaemonPhasedRequest { readonly commandName: string; /** The exact warm engine shape against which the selection was resolved. */ readonly engineShape: IDaemonPhasedEngineShape; + /** The request environment used for Rush command policy without mutating the daemon process environment. */ + readonly environment: Readonly>; /** The caller-resolved selected operations and their enabled states. */ readonly operationSelection: ReadonlyArray; /** A client-generated identifier unique within the connection. */ @@ -71,9 +75,7 @@ export interface IDaemonPhasedOperationResult { * * @beta */ -export interface IDaemonPhasedRequestResult { - /** Whether cancellation or disconnect aborted the iteration. */ - readonly aborted: boolean; +export interface IDaemonPhasedRequestResult extends IDaemonCommandResult { /** Results only for operations enabled for this client. */ readonly operationResults: ReadonlyArray; /** The identifier copied from the request. */ diff --git a/libraries/rush-daemon-protocol/src/index.ts b/libraries/rush-daemon-protocol/src/index.ts index 878386d550..b6b0841f12 100644 --- a/libraries/rush-daemon-protocol/src/index.ts +++ b/libraries/rush-daemon-protocol/src/index.ts @@ -38,6 +38,7 @@ export { decodeDaemonLogChunk, encodeDaemonLogChunk, type IDaemonLogChunk } from export { createDaemonHello, createDaemonHelloAck, negotiateDaemonHello } from './DaemonHandshake'; export type { DaemonHandshakeOutcome } from './DaemonHandshake'; export type { DaemonJsonNull, DaemonJsonValue } from './DaemonJsonValue'; +export type { DaemonCommandOutcome, IDaemonCommandResult } from './DaemonCommandResult'; export { DAEMON_EVENT_TYPES, isDaemonEventType, type DaemonEventType } from './DaemonEventType'; export type { DaemonEventPrivacy, IDaemonEventEnvelope, IDaemonEventScope, IDaemonEventSource } from './DaemonEventEnvelope'; export { isDaemonEventEnvelope, validateDaemonEventEnvelope } from './DaemonEventValidation'; diff --git a/libraries/rush-daemon/README.md b/libraries/rush-daemon/README.md index 2fec834737..e9390f22ba 100644 --- a/libraries/rush-daemon/README.md +++ b/libraries/rush-daemon/README.md @@ -31,10 +31,13 @@ lifetime tracked by [rushstack#5895](https://github.com/microsoft/rushstack/issu `PhasedRequestRouter` is the opt-in execution boundary once an integration has supplied that real warm graph. The integration parses the command and supplies an explicit phase/plugin shape plus operation enabled-state selection; the router validates both, reconciles retained invalidations, applies the selection with `IOperationGraph.setEnabledStates`, -and runs at most one scheduled iteration. Requests are serialized until shared-build merging is implemented. A -requesting client receives only its enabled dependency closure's WS1 raw chunks and structured events through -backpressured, ordered callbacks, followed by client-scoped operation results. Cancellation or disconnect aborts the -current iteration without closing daemon-owned runners or the graph. +and runs at most one scheduled iteration. Requests are serialized until shared-build merging is implemented. A requesting client receives only its enabled +dependency closure's WS1 raw chunks and structured events through backpressured, ordered callbacks, followed exactly +once by a typed final command result after all preceding output drains. The result translates only that client's +operation subset to Rush's success, warning, failure, or abort exit semantics. Warning-only builds honor the +operation's configured `allowWarningsInSuccessfulBuild` state plus the request's immutable +`RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD` environment override without mutating `process.env`. Cancellation or +disconnect aborts the current iteration without closing daemon-owned runners or the graph. This layer deliberately does not add control-frame admission or reconstruct `PhasedScriptAction` command/plugin initialization. The typed phased request contract begins after an integration has produced a validated selection for @@ -48,11 +51,13 @@ through cancellation or disconnect. Concurrent requests never change `process.cw stdin/stdout/stderr; child commands receive cwd, environment, cancellation, and output routing through the injected execution context. Executors must cooperatively observe the context abort signal and settle before cancellation completes, ensuring no -caller-owned logic can outlive its request resources. +caller-owned logic can outlive its request resources. Executors return their command exit code; the router preserves +that code, translates thrown or cleanup failures to Rush's failure exit code, drains terminal output, and delivers one +final result. The existing `RushCommandLineParser`, `BaseRushAction`, and some built-in/global action helpers still consult or mutate process-global state. This layer therefore does not pretend that arbitrary existing actions are daemon-safe: the integration must supply already resolved command logic that consumes `IGlobalCommandExecutionContext`, including `spawnChild()` for command-local subprocesses. Adapting the complete action surface remains bounded by the open -[rushstack#5895](https://github.com/microsoft/rushstack/issues/5895) engine/action prerequisite work. Exit-code policy, -interactive stdin/raw-mode/PTY support, scheduling classification, and shared-build merging belong to later layers. +[rushstack#5895](https://github.com/microsoft/rushstack/issues/5895) engine/action prerequisite work. Interactive +stdin/raw-mode/PTY support, scheduling classification, and shared-build merging belong to later layers. diff --git a/libraries/rush-daemon/src/CommandResultPolicy.ts b/libraries/rush-daemon/src/CommandResultPolicy.ts new file mode 100644 index 0000000000..126fc67107 --- /dev/null +++ b/libraries/rush-daemon/src/CommandResultPolicy.ts @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { OperationStatus } from '@microsoft/rush-lib'; +import { EnvironmentMap } from '@rushstack/node-core-library'; +import type { + DaemonCommandOutcome, + IDaemonCommandResult, + IDaemonPhasedOperationResult, + IDaemonPhasedRequestResult +} from '@rushstack/rush-daemon-protocol'; + +export const RUSH_SUCCESS_EXIT_CODE: number = 0; +export const RUSH_FAILURE_EXIT_CODE: number = 1; +export const RUSH_ALLOW_WARNINGS_ENVIRONMENT_VARIABLE: string = + 'RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD'; + +export interface IPhasedOperationOutcome { + readonly observedInCurrentIteration: boolean; + readonly result: IDaemonPhasedOperationResult; + readonly warningsAreAllowed: boolean; +} + +export interface IPhasedCommandResultOptions { + readonly aborted: boolean; + readonly error: unknown; + readonly graphStatus: OperationStatus; + readonly operationOutcomes: ReadonlyArray; + readonly requestId: string; + readonly scheduled: boolean; + readonly warningsAllowedByEnvironment: boolean; +} + +const SUCCESS_STATUSES: ReadonlySet = new Set([ + OperationStatus.Success, + OperationStatus.Skipped, + OperationStatus.FromCache, + OperationStatus.NoOp +]); + +export function parseWarningsAllowedByEnvironment( + environment: Readonly> +): boolean { + const value: string | undefined = new EnvironmentMap(environment).get( + RUSH_ALLOW_WARNINGS_ENVIRONMENT_VARIABLE + ); + if (value === undefined || value === '' || value === '0') { + return false; + } + if (value === '1') { + return true; + } + throw new Error( + `The ${RUSH_ALLOW_WARNINGS_ENVIRONMENT_VARIABLE} environment variable must be set to 1 or 0.` + ); +} + +export function createGlobalCommandResult(options: { + readonly aborted: boolean; + readonly error: unknown; + readonly exitCode: number | undefined; + readonly requestId: string; +}): IDaemonCommandResult { + if (options.error !== undefined) { + return createResult('failure', RUSH_FAILURE_EXIT_CODE, options.requestId, options.aborted, options.error); + } + if (options.aborted) { + return createResult('aborted', RUSH_FAILURE_EXIT_CODE, options.requestId, true); + } + const exitCode: number = validateExitCode(options.exitCode); + return createResult( + exitCode === RUSH_SUCCESS_EXIT_CODE ? 'success' : 'failure', + exitCode, + options.requestId, + false + ); +} + +export function createPhasedCommandResult( + options: IPhasedCommandResultOptions +): IDaemonPhasedRequestResult { + const operationResults: ReadonlyArray = options.operationOutcomes.map( + ({ result }) => result + ); + if (options.error !== undefined) { + return createPhasedResult('failure', RUSH_FAILURE_EXIT_CODE, options, operationResults); + } + const outcome: DaemonCommandOutcome = getPhasedOutcome(options); + const warningsAllowed: boolean = options.operationOutcomes.every( + ({ observedInCurrentIteration, result, warningsAreAllowed }) => + !observedInCurrentIteration || + result.status !== OperationStatus.SuccessWithWarning || + warningsAreAllowed || + options.warningsAllowedByEnvironment + ); + const exitCode: number = + outcome === 'success' || (outcome === 'success-with-warning' && warningsAllowed) + ? RUSH_SUCCESS_EXIT_CODE + : RUSH_FAILURE_EXIT_CODE; + return createPhasedResult(outcome, exitCode, options, operationResults); +} + +function getPhasedOutcome(options: IPhasedCommandResultOptions): DaemonCommandOutcome { + if (!options.scheduled) { + return options.aborted ? 'aborted' : 'success'; + } + if (options.graphStatus === OperationStatus.Failure || options.graphStatus === OperationStatus.Blocked) { + return 'failure'; + } + if (options.graphStatus === OperationStatus.Aborted) { + return 'aborted'; + } + if ( + options.graphStatus === OperationStatus.SuccessWithWarning || + options.operationOutcomes.some( + ({ observedInCurrentIteration, result }) => + observedInCurrentIteration && result.status === OperationStatus.SuccessWithWarning + ) + ) { + return 'success-with-warning'; + } + if (SUCCESS_STATUSES.has(options.graphStatus)) { + return 'success'; + } + return 'failure'; +} + +function createPhasedResult( + outcome: DaemonCommandOutcome, + exitCode: number, + options: IPhasedCommandResultOptions, + operationResults: ReadonlyArray +): IDaemonPhasedRequestResult { + return { + aborted: options.aborted, + errorMessage: normalizeErrorMessage(options.error), + exitCode, + operationResults, + outcome, + requestId: options.requestId, + scheduled: options.scheduled + }; +} + +function createResult( + outcome: DaemonCommandOutcome, + exitCode: number, + requestId: string, + aborted: boolean, + error?: unknown +): IDaemonCommandResult { + return { aborted, errorMessage: normalizeErrorMessage(error), exitCode, outcome, requestId }; +} + +function normalizeErrorMessage(error: unknown): string | undefined { + if (error === undefined) { + return undefined; + } + return error instanceof Error ? error.message : String(error); +} + +function validateExitCode(exitCode: number | undefined): number { + if (exitCode === undefined || !Number.isSafeInteger(exitCode) || exitCode < RUSH_SUCCESS_EXIT_CODE) { + throw new Error('A global command executor must return a nonnegative safe-integer exit code.'); + } + return exitCode; +} diff --git a/libraries/rush-daemon/src/GlobalCommandExecutionContext.ts b/libraries/rush-daemon/src/GlobalCommandExecutionContext.ts index a01c8817db..879f7527fd 100644 --- a/libraries/rush-daemon/src/GlobalCommandExecutionContext.ts +++ b/libraries/rush-daemon/src/GlobalCommandExecutionContext.ts @@ -156,6 +156,7 @@ export class GlobalCommandExecutionContext readonly #childTerminationErrors: unknown[] = []; readonly #writer: OrderedTerminalWriter; #closed: boolean = false; + #requestAborted: boolean = false; public readonly terminal: ITerminal; public readonly workspaceSession: IWorkspaceSession; @@ -168,10 +169,8 @@ export class GlobalCommandExecutionContext this.#request = request; this.#client = client; this.workspaceSession = workspaceSession; - this.#onClientAbort = () => this.#abortController.abort(client.abortSignal.reason); - this.#writer = new OrderedTerminalWriter(client, (error: Error) => - this.#abortController.abort(error) - ); + this.#onClientAbort = () => this.#abortRequest(client.abortSignal.reason); + this.#writer = new OrderedTerminalWriter(client, (error: Error) => this.#abortRequest(error)); this.terminal = new Terminal( new GlobalCommandTerminalProvider(this.#writer, request.terminal.supportsColor) ); @@ -194,6 +193,10 @@ export class GlobalCommandExecutionContext return this.#request.environment; } + public get requestAborted(): boolean { + return this.#requestAborted; + } + public get terminalProperties(): IGlobalCommandTerminalProperties { return this.#request.terminal; } @@ -240,24 +243,32 @@ export class GlobalCommandExecutionContext return; } this.#closed = true; - this.#client.abortSignal.removeEventListener('abort', this.#onClientAbort); this.#abortController.abort(new Error('The global command execution context was disposed.')); const cleanupErrors: unknown[] = []; - await Promise.all( - Array.from(this.#trackedChildren, ({ completion }) => - collectCleanupErrorAsync(completion, cleanupErrors) - ) - ); - cleanupErrors.push(...this.#childCompletionErrors); - cleanupErrors.push(...this.#childTerminationErrors); - for (const disposable of this.#disposables.reverse()) { - await collectCleanupErrorAsync( - Promise.resolve().then(() => disposable[Symbol.asyncDispose]()), - cleanupErrors + try { + await Promise.all( + Array.from(this.#trackedChildren, ({ completion }) => + collectCleanupErrorAsync(completion, cleanupErrors) + ) ); + cleanupErrors.push(...this.#childCompletionErrors); + cleanupErrors.push(...this.#childTerminationErrors); + for (const disposable of this.#disposables.reverse()) { + await collectCleanupErrorAsync( + Promise.resolve().then(() => disposable[Symbol.asyncDispose]()), + cleanupErrors + ); + } + await collectCleanupErrorAsync(this.#writer.closeAsync(), cleanupErrors); + throwCleanupErrors(cleanupErrors); + } finally { + this.#client.abortSignal.removeEventListener('abort', this.#onClientAbort); } - await collectCleanupErrorAsync(this.#writer.closeAsync(), cleanupErrors); - throwCleanupErrors(cleanupErrors); + } + + #abortRequest(reason: unknown): void { + this.#requestAborted = true; + this.#abortController.abort(reason); } async #trackChildAsync(child: childProcess.ChildProcessWithoutNullStreams): Promise { diff --git a/libraries/rush-daemon/src/GlobalCommandRequestClient.ts b/libraries/rush-daemon/src/GlobalCommandRequestClient.ts index a7346509d9..2f922e4a8c 100644 --- a/libraries/rush-daemon/src/GlobalCommandRequestClient.ts +++ b/libraries/rush-daemon/src/GlobalCommandRequestClient.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { IDaemonCommandResult } from '@rushstack/rush-daemon-protocol'; + /** * A client-scoped destination for one global command request. * @@ -16,4 +18,7 @@ export interface IGlobalCommandRequestClient { /** Writes one request-scoped terminal chunk through the client's backpressured destination. */ writeTerminalChunkAsync(stream: 'stdout' | 'stderr', chunk: Uint8Array): Promise; + + /** Writes the final command result after every preceding terminal chunk has drained. */ + writeResultAsync(result: IDaemonCommandResult): Promise; } diff --git a/libraries/rush-daemon/src/GlobalCommandRequestRouter.ts b/libraries/rush-daemon/src/GlobalCommandRequestRouter.ts index 264ba6e4a6..15377f2773 100644 --- a/libraries/rush-daemon/src/GlobalCommandRequestRouter.ts +++ b/libraries/rush-daemon/src/GlobalCommandRequestRouter.ts @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type { IDaemonCommandResult } from '@rushstack/rush-daemon-protocol'; + +import { createGlobalCommandResult } from './CommandResultPolicy'; import type { IGlobalCommandExecutionContext } from './GlobalCommandExecutionContext'; import { GlobalCommandExecutionContext } from './GlobalCommandExecutionContext'; import { @@ -21,18 +24,26 @@ import type { IWorkspaceSession } from './WorkspaceSession'; * * @beta */ -export type GlobalCommandExecutor = (context: IGlobalCommandExecutionContext) => Promise; +export type GlobalCommandExecutor = ( + context: IGlobalCommandExecutionContext +) => Promise; /** - * The completion state for one global command request. + * The process result returned by caller-owned global command logic. * * @beta */ -export interface IGlobalCommandRequestResult { - readonly aborted: boolean; - readonly requestId: string; +export interface IGlobalCommandExecutionResult { + readonly exitCode: number; } +/** + * The completion state for one global command request. + * + * @beta + */ +export type IGlobalCommandRequestResult = IDaemonCommandResult; + /** * Routes caller-owned global command logic through isolated per-request process and terminal context. * @@ -67,18 +78,23 @@ export class GlobalCommandRequestRouter { this.#workspaceSession ); let executionError: unknown; + let executionResult: IGlobalCommandExecutionResult | undefined; let aborted: boolean = context.abortSignal.aborted; try { if (!aborted) { - const executorPromise: Promise = Promise.resolve().then(() => executor(context)); + const executorPromise: Promise = Promise.resolve().then(() => + executor(context) + ); const outcome: 'aborted' | 'completed' = await waitForExecutionAsync( executorPromise, context.abortSignal ); aborted = outcome === 'aborted'; + executionResult = await executorPromise; } } catch (error) { executionError = error; + aborted = context.abortSignal.aborted; } let cleanupError: unknown; @@ -87,13 +103,31 @@ export class GlobalCommandRequestRouter { } catch (error) { cleanupError = error; } - throwExecutionAndCleanupErrors(executionError, cleanupError); - return { aborted: aborted || client.abortSignal.aborted, requestId: request.requestId }; + aborted ||= context.requestAborted; + const combinedError: unknown = combineExecutionAndCleanupErrors(executionError, cleanupError); + let result: IDaemonCommandResult; + try { + result = createGlobalCommandResult({ + aborted, + error: combinedError, + exitCode: executionResult?.exitCode, + requestId: request.requestId + }); + } catch (error) { + result = createGlobalCommandResult({ + aborted: false, + error, + exitCode: undefined, + requestId: request.requestId + }); + } + await client.writeResultAsync(result); + return result; } } async function waitForExecutionAsync( - executorPromise: Promise, + executorPromise: Promise, abortSignal: AbortSignal ): Promise<'aborted' | 'completed'> { let removeAbortListener: (() => void) | undefined; @@ -110,7 +144,7 @@ async function waitForExecutionAsync( try { const outcome: 'aborted' | 'completed' = await Promise.race([completedPromise, abortPromise]); if (outcome === 'aborted') { - await executorPromise.catch(() => undefined); + await executorPromise; } return outcome; } finally { @@ -119,17 +153,18 @@ async function waitForExecutionAsync( } } -function throwExecutionAndCleanupErrors(executionError: unknown, cleanupError: unknown): void { +function combineExecutionAndCleanupErrors(executionError: unknown, cleanupError: unknown): unknown { if (executionError !== undefined && cleanupError !== undefined) { - throw new AggregateError( + return new AggregateError( [executionError, cleanupError], 'The global command failed and could not clean up its request context.' ); } if (executionError !== undefined) { - throw executionError; + return executionError; } if (cleanupError !== undefined) { - throw cleanupError; + return cleanupError; } + return undefined; } diff --git a/libraries/rush-daemon/src/PhasedRequestClient.ts b/libraries/rush-daemon/src/PhasedRequestClient.ts index 68dfc72a6f..046a4163c2 100644 --- a/libraries/rush-daemon/src/PhasedRequestClient.ts +++ b/libraries/rush-daemon/src/PhasedRequestClient.ts @@ -1,7 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { IDaemonEventEnvelope } from '@rushstack/rush-daemon-protocol'; +import type { + IDaemonEventEnvelope, + IDaemonPhasedRequestResult +} from '@rushstack/rush-daemon-protocol'; /** * A client-scoped destination for one routed phased request. @@ -30,4 +33,7 @@ export interface IPhasedRequestClient { stream: 'stdout' | 'stderr', chunk: Uint8Array ): Promise; + + /** Writes the final command result after every preceding event and log chunk has drained. */ + writeResultAsync(result: IDaemonPhasedRequestResult): Promise; } diff --git a/libraries/rush-daemon/src/PhasedRequestRouter.ts b/libraries/rush-daemon/src/PhasedRequestRouter.ts index 1c4c1049b0..1f751c4a8a 100644 --- a/libraries/rush-daemon/src/PhasedRequestRouter.ts +++ b/libraries/rush-daemon/src/PhasedRequestRouter.ts @@ -10,7 +10,6 @@ import type { import { OperationStatus } from '@microsoft/rush-lib'; import type { IDaemonPhasedEngineShape, - IDaemonPhasedOperationResult, IDaemonPhasedOperationSelection, IDaemonPhasedRequest, IDaemonPhasedRequestResult @@ -28,6 +27,11 @@ import { import type { IRequestLease } from './RequestScheduler'; import type { IWorkspaceEngineShape } from './WorkspaceEngineComponentFactory'; import type { IWorkspaceSession } from './WorkspaceSession'; +import { + createPhasedCommandResult, + type IPhasedOperationOutcome, + parseWarningsAllowedByEnvironment +} from './CommandResultPolicy'; interface IDualEmitOperationGraph extends IOperationGraph { eventSink: _IOperationGraphEventSink | undefined; @@ -80,7 +84,7 @@ export class PhasedRequestRouter { error instanceof RequestSchedulerError && error.code === RequestSchedulerErrorCode.Aborted ) { - return createAbortedResult(request.requestId); + return await writeAbortedResultAsync(request.requestId, client); } throw error; } @@ -102,16 +106,32 @@ export class PhasedRequestRouter { validateEngineShape(request.engineShape, this.#workspaceSession.engineShape); const operationById: ReadonlyMap = indexOperations(graph.operations); const selection: IResolvedSelection = resolveSelection(request.operationSelection, operationById); + let warningsAllowedByEnvironment: boolean; + try { + warningsAllowedByEnvironment = parseWarningsAllowedByEnvironment(request.environment); + } catch (error) { + const result: IDaemonPhasedRequestResult = createPhasedCommandResult({ + aborted: client.abortSignal.aborted, + error, + graphStatus: graph.status, + operationOutcomes: [], + requestId: request.requestId, + scheduled: false, + warningsAllowedByEnvironment: false + }); + await client.writeResultAsync(result); + return result; + } if (client.abortSignal.aborted) { - return createAbortedResult(request.requestId); + return await writeAbortedResultAsync(request.requestId, client); } if (graph.hasScheduledIteration || graph.status === OperationStatus.Executing) { throw new Error('The warm workspace operation graph is not idle.'); } await this.#workspaceSession.reconcileInvalidationsAsync(); if (client.abortSignal.aborted) { - return createAbortedResult(request.requestId); + return await writeAbortedResultAsync(request.requestId, client); } applySelection(graph, selection); @@ -187,14 +207,17 @@ export class PhasedRequestRouter { } await abortTail; cleanupErrors.push(...abortErrors.slice(observedAbortErrorCount)); - throwCombinedErrors(executionError, cleanupErrors); - - return { + const result: IDaemonPhasedRequestResult = createPhasedCommandResult({ aborted: wasAborted || client.abortSignal.aborted, - operationResults: collectOperationResults(activeOperations, graph, requestSink), + error: combineErrors(executionError, cleanupErrors), + graphStatus: graph.status, + operationOutcomes: collectOperationOutcomes(activeOperations, graph, requestSink), requestId: request.requestId, - scheduled - }; + scheduled, + warningsAllowedByEnvironment + }); + await client.writeResultAsync(result); + return result; } } @@ -343,12 +366,12 @@ function applySelection(graph: IOperationGraph, selection: IResolvedSelection): ); } -function collectOperationResults( +function collectOperationOutcomes( activeOperations: ReadonlyArray, graph: IOperationGraph, requestSink: PhasedRequestEventSink -): ReadonlyArray { - const results: IDaemonPhasedOperationResult[] = []; +): ReadonlyArray { + const outcomes: IPhasedOperationOutcome[] = []; for (const operation of [...activeOperations].sort(compareOperations)) { const observed: ReturnType = requestSink.getObservedResult(operation); @@ -360,33 +383,51 @@ function collectOperationResults( const errorMessage: string | undefined = observed ? observed.executionResult.error?.message : retained?.error?.message; - results.push({ operationId: operation.name, status, errorMessage }); + outcomes.push({ + observedInCurrentIteration: observed !== undefined, + result: { operationId: operation.name, status, errorMessage }, + warningsAreAllowed: operation.runner?.warningsAreAllowed ?? false + }); } - return results; + return outcomes; } function compareOperations(left: Operation, right: Operation): number { return left.name.localeCompare(right.name); } -function createAbortedResult(requestId: string): IDaemonPhasedRequestResult { - return { aborted: true, operationResults: [], requestId, scheduled: false }; +async function writeAbortedResultAsync( + requestId: string, + client: IPhasedRequestClient +): Promise { + const result: IDaemonPhasedRequestResult = createPhasedCommandResult({ + aborted: true, + error: undefined, + graphStatus: OperationStatus.Aborted, + operationOutcomes: [], + requestId, + scheduled: false, + warningsAllowedByEnvironment: false + }); + await client.writeResultAsync(result); + return result; } -function throwCombinedErrors(executionError: unknown, cleanupErrors: unknown[]): void { +function combineErrors(executionError: unknown, cleanupErrors: unknown[]): unknown { if (executionError !== undefined && cleanupErrors.length > 0) { - throw new AggregateError( + return new AggregateError( [executionError, ...cleanupErrors], 'The phased request failed and could not clean up its client subscription.' ); } if (executionError !== undefined) { - throw executionError; + return executionError; } if (cleanupErrors.length === 1) { - throw cleanupErrors[0]; + return cleanupErrors[0]; } if (cleanupErrors.length > 1) { - throw new AggregateError(cleanupErrors, 'Failed to clean up the phased request client subscription.'); + return new AggregateError(cleanupErrors, 'Failed to clean up the phased request client subscription.'); } + return undefined; } diff --git a/libraries/rush-daemon/src/index.ts b/libraries/rush-daemon/src/index.ts index a95b8b766f..6d3282d61b 100644 --- a/libraries/rush-daemon/src/index.ts +++ b/libraries/rush-daemon/src/index.ts @@ -25,6 +25,7 @@ export { type IGlobalCommandRequestClient } from './GlobalCommandRequestClient'; export { type GlobalCommandExecutor, GlobalCommandRequestRouter, + type IGlobalCommandExecutionResult, type IGlobalCommandRequestResult } from './GlobalCommandRequestRouter'; export { RushDaemonHost, type IRushDaemonHostOptions } from './RushDaemonHost'; diff --git a/libraries/rush-daemon/src/test/CommandResultPolicy.test.ts b/libraries/rush-daemon/src/test/CommandResultPolicy.test.ts new file mode 100644 index 0000000000..0bb7841614 --- /dev/null +++ b/libraries/rush-daemon/src/test/CommandResultPolicy.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import process from 'node:process'; + +import { OperationStatus } from '@microsoft/rush-lib'; + +import { + createPhasedCommandResult, + type IPhasedOperationOutcome, + parseWarningsAllowedByEnvironment +} from '../CommandResultPolicy'; + +interface IParityCase { + readonly aborted?: boolean; + readonly expectedExitCode: number; + readonly expectedOutcome: 'success' | 'success-with-warning' | 'failure' | 'aborted'; + readonly graphStatus?: OperationStatus; + readonly scheduled?: boolean; + readonly statuses: ReadonlyArray<{ + readonly status: OperationStatus; + readonly warningsAreAllowed?: boolean; + }>; + readonly warningEnvironment?: string; +} + +const PARITY_CASES: ReadonlyArray = [ + { expectedExitCode: 0, expectedOutcome: 'success', statuses: [status(OperationStatus.Success)] }, + { expectedExitCode: 0, expectedOutcome: 'success', statuses: [status(OperationStatus.Skipped)] }, + { expectedExitCode: 0, expectedOutcome: 'success', statuses: [status(OperationStatus.FromCache)] }, + { expectedExitCode: 0, expectedOutcome: 'success', statuses: [status(OperationStatus.NoOp)] }, + { + expectedExitCode: 1, + expectedOutcome: 'success-with-warning', + statuses: [status(OperationStatus.SuccessWithWarning)] + }, + { + expectedExitCode: 0, + expectedOutcome: 'success-with-warning', + graphStatus: OperationStatus.Success, + statuses: [status(OperationStatus.SuccessWithWarning, true)] + }, + { + expectedExitCode: 0, + expectedOutcome: 'success-with-warning', + statuses: [status(OperationStatus.SuccessWithWarning)], + warningEnvironment: '1' + }, + { + expectedExitCode: 0, + expectedOutcome: 'success-with-warning', + graphStatus: OperationStatus.Success, + statuses: [status(OperationStatus.SuccessWithWarning, true)], + warningEnvironment: '0' + }, + { expectedExitCode: 1, expectedOutcome: 'failure', statuses: [status(OperationStatus.Failure)] }, + { expectedExitCode: 1, expectedOutcome: 'failure', statuses: [status(OperationStatus.Blocked)] }, + { + aborted: true, + expectedExitCode: 1, + expectedOutcome: 'aborted', + statuses: [status(OperationStatus.Aborted)] + }, + { + aborted: true, + expectedExitCode: 1, + expectedOutcome: 'failure', + graphStatus: OperationStatus.Failure, + statuses: [status(OperationStatus.Failure), status(OperationStatus.Aborted)] + }, + { + expectedExitCode: 0, + expectedOutcome: 'success', + scheduled: false, + statuses: [status(OperationStatus.Failure)] + } +]; + +describe('Rush command result parity', () => { + it.each(PARITY_CASES)('matches the in-process outcome for %#', (testCase: IParityCase) => { + const operationOutcomes: IPhasedOperationOutcome[] = testCase.statuses.map( + ({ status: operationStatus, warningsAreAllowed = false }, index: number) => ({ + observedInCurrentIteration: true, + result: { operationId: `operation-${index}`, status: operationStatus }, + warningsAreAllowed + }) + ); + + const result = createPhasedCommandResult({ + aborted: testCase.aborted ?? false, + error: undefined, + graphStatus: + testCase.graphStatus ?? + testCase.statuses[testCase.statuses.length - 1]?.status ?? + OperationStatus.NoOp, + operationOutcomes, + requestId: 'parity', + scheduled: testCase.scheduled ?? true, + warningsAllowedByEnvironment: parseWarningsAllowedByEnvironment({ + RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: testCase.warningEnvironment ?? '' + }) + }); + + expect(result).toMatchObject({ + exitCode: testCase.expectedExitCode, + outcome: testCase.expectedOutcome + }); + }); + + it('rejects an invalid warnings override like EnvironmentConfiguration', () => { + expect(() => + parseWarningsAllowedByEnvironment({ RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: 'true' }) + ).toThrow('must be set to 1 or 0'); + }); + + it('uses platform environment-name semantics for the warnings override', () => { + const lowercaseEnvironmentName: string = 'rush_allow_warnings_in_successful_build'; + expect( + parseWarningsAllowedByEnvironment({ [lowercaseEnvironmentName]: '1' }) + ).toBe(process.platform === 'win32'); + }); + + it('ignores a retained warning when the current incremental iteration succeeds', () => { + const result = createPhasedCommandResult({ + aborted: false, + error: undefined, + graphStatus: OperationStatus.Success, + operationOutcomes: [ + { + observedInCurrentIteration: false, + result: { operationId: 'retained-warning', status: OperationStatus.SuccessWithWarning }, + warningsAreAllowed: false + }, + { + observedInCurrentIteration: true, + result: { operationId: 'current-success', status: OperationStatus.Success }, + warningsAreAllowed: false + } + ], + requestId: 'incremental', + scheduled: true, + warningsAllowedByEnvironment: false + }); + + expect(result).toMatchObject({ exitCode: 0, outcome: 'success' }); + expect(result.operationResults).toEqual([ + { operationId: 'retained-warning', status: OperationStatus.SuccessWithWarning }, + { operationId: 'current-success', status: OperationStatus.Success } + ]); + }); +}); + +function status( + operationStatus: OperationStatus, + warningsAreAllowed: boolean = false +): { readonly status: OperationStatus; readonly warningsAreAllowed: boolean } { + return { status: operationStatus, warningsAreAllowed }; +} diff --git a/libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts b/libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts index 9540e1201c..91bb5a1153 100644 --- a/libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts +++ b/libraries/rush-daemon/src/test/GlobalCommandRequestRouter.test.ts @@ -5,6 +5,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { SubprocessTerminator } from '@rushstack/node-core-library'; +import type { IDaemonCommandResult } from '@rushstack/rush-daemon-protocol'; import type { IGlobalCommandExecutionContext } from '../GlobalCommandExecutionContext'; import type { @@ -14,6 +15,7 @@ import type { import type { IGlobalCommandRequestClient } from '../GlobalCommandRequestClient'; import { GlobalCommandRequestRouter, + type IGlobalCommandExecutionResult, type IGlobalCommandRequestResult } from '../GlobalCommandRequestRouter'; import { TestWorkspaceSession, TEST_REPO_ROOT } from './TestWorkspaceSession'; @@ -30,7 +32,10 @@ interface IClientChunk { class TestGlobalCommandClient implements IGlobalCommandRequestClient { public readonly abortController: AbortController = new AbortController(); public readonly chunks: IClientChunk[] = []; + public readonly results: IDaemonCommandResult[] = []; + public readonly writeOrder: Array<'chunk' | 'result'> = []; public onWriteAsync: ((chunk: IClientChunk) => Promise) | undefined; + public onResultAsync: ((result: IDaemonCommandResult) => Promise) | undefined; public get abortSignal(): AbortSignal { return this.abortController.signal; @@ -43,6 +48,13 @@ class TestGlobalCommandClient implements IGlobalCommandRequestClient { const clientChunk: IClientChunk = { stream, text: TEXT_DECODER.decode(chunk) }; this.chunks.push(clientChunk); await this.onWriteAsync?.(clientChunk); + this.writeOrder.push('chunk'); + } + + public async writeResultAsync(result: IDaemonCommandResult): Promise { + await this.onResultAsync?.(result); + this.results.push(result); + this.writeOrder.push('result'); } } @@ -102,7 +114,7 @@ describe(GlobalCommandRequestRouter.name, () => { ): Promise => router.executeAsync( request, - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { observations.push( [ context.cwd, @@ -118,6 +130,7 @@ describe(GlobalCommandRequestRouter.name, () => { await executorsStarted; expect(process.cwd()).toBe(processCwd); expect(process.env.RUSHD_CONTEXT_TEST).toBe(processEnvironmentValue); + return { exitCode: 0 }; }, client ); @@ -136,8 +149,20 @@ describe(GlobalCommandRequestRouter.name, () => { ]); expect(results).toEqual([ - { aborted: false, requestId: 'first' }, - { aborted: false, requestId: 'second' } + { + aborted: false, + errorMessage: undefined, + exitCode: 0, + outcome: 'success', + requestId: 'first' + }, + { + aborted: false, + errorMessage: undefined, + exitCode: 0, + outcome: 'success', + requestId: 'second' + } ]); expect(new Set(observations)).toEqual( new Set([ @@ -147,10 +172,62 @@ describe(GlobalCommandRequestRouter.name, () => { ); expect(firstClient.chunks.map(({ text }) => text).join('')).toContain('first'); expect(secondClient.chunks.map(({ text }) => text).join('')).toContain('second'); + expect(firstClient.results).toEqual([results[0]]); + expect(secondClient.results).toEqual([results[1]]); + expect(firstClient.writeOrder[firstClient.writeOrder.length - 1]).toBe('result'); + expect(secondClient.writeOrder[secondClient.writeOrder.length - 1]).toBe('result'); expect(process.cwd()).toBe(processCwd); expect(process.env.RUSHD_CONTEXT_TEST).toBe(processEnvironmentValue); }); + it('preserves a global command exit code and delivers it exactly once', async () => { + const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter( + new TestWorkspaceSession(TEST_REPO_ROOT) + ); + const client: TestGlobalCommandClient = new TestGlobalCommandClient(); + + const result: IGlobalCommandRequestResult = await router.executeAsync( + router.resolveRequest(createRequestOptions('exit-code', FIRST_CWD, {}, 80)), + async (): Promise => ({ exitCode: 7 }), + client + ); + + expect(result).toEqual({ + aborted: false, + errorMessage: undefined, + exitCode: 7, + outcome: 'failure', + requestId: 'exit-code' + }); + expect(client.results).toEqual([result]); + expect(client.writeOrder).toEqual(['result']); + }); + + it.each([undefined, {}])( + 'converts malformed global executor result %# to failure', + async (invalidResult) => { + const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter( + new TestWorkspaceSession(TEST_REPO_ROOT) + ); + const client: TestGlobalCommandClient = new TestGlobalCommandClient(); + + const result: IGlobalCommandRequestResult = await router.executeAsync( + router.resolveRequest(createRequestOptions('invalid-result', FIRST_CWD, {}, 80)), + async (): Promise => + invalidResult as IGlobalCommandExecutionResult, + client + ); + + expect(result).toMatchObject({ + aborted: false, + errorMessage: 'A global command executor must return a nonnegative safe-integer exit code.', + exitCode: 1, + outcome: 'failure' + }); + expect(client.results).toEqual([result]); + } + ); + it('snapshots request environment and propagates isolated context to child processes', async () => { const mutableEnvironment: NodeJS.ProcessEnv = { CHILD_CONTEXT: 'request', @@ -168,7 +245,7 @@ describe(GlobalCommandRequestRouter.name, () => { await router.executeAsync( request, - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { const child = context.spawnChild( process.execPath, [ @@ -187,6 +264,7 @@ describe(GlobalCommandRequestRouter.name, () => { child.once('error', reject); child.once('close', () => resolve()); }); + return { exitCode: 0 }; }, client ); @@ -219,10 +297,13 @@ describe(GlobalCommandRequestRouter.name, () => { try { await router.executeAsync( router.resolveRequest(createRequestOptions('completed-child', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async ( + context: IGlobalCommandExecutionContext + ): Promise => { const child = context.spawnChild(process.execPath, ['-e', '']); childPid = child.pid; await new Promise((resolve) => child.once('close', () => resolve())); + return { exitCode: 0 }; }, new TestGlobalCommandClient() ); @@ -244,15 +325,22 @@ describe(GlobalCommandRequestRouter.name, () => { await expect( router.executeAsync( router.resolveRequest(createRequestOptions('spawn-failure', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async ( + context: IGlobalCommandExecutionContext + ): Promise => { context.spawnChild(path.join(FIRST_CWD, 'missing-global-command'), [], { forwardOutput: false }); await new Promise((resolve) => setImmediate(resolve)); + return { exitCode: 0 }; }, new TestGlobalCommandClient() ) - ).rejects.toThrow(/ENOENT|spawn/); + ).resolves.toMatchObject({ + errorMessage: expect.stringMatching(/ENOENT|spawn/), + exitCode: 1, + outcome: 'failure' + }); }); it('rejects non-string values in untrusted environment snapshots and overlays', async () => { @@ -268,12 +356,19 @@ describe(GlobalCommandRequestRouter.name, () => { await expect( router.executeAsync( router.resolveRequest(createRequestOptions('invalid-overlay', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async ( + context: IGlobalCommandExecutionContext + ): Promise => { context.spawnChild(process.execPath, [], { environmentOverlay: invalidOverlay }); + return { exitCode: 0 }; }, new TestGlobalCommandClient() ) - ).rejects.toThrow('environment variable "INVALID_OVERLAY" must have a string value'); + ).resolves.toMatchObject({ + errorMessage: 'The global command environment variable "INVALID_OVERLAY" must have a string value.', + exitCode: 1, + outcome: 'failure' + }); }); it('cleans registered resources after success and failure without disposing the warm session', async () => { @@ -295,19 +390,26 @@ describe(GlobalCommandRequestRouter.name, () => { await router.executeAsync( router.resolveRequest(createRequestOptions('success', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => registerDisposable(context), + async (context: IGlobalCommandExecutionContext): Promise => { + registerDisposable(context); + return { exitCode: 0 }; + }, new TestGlobalCommandClient() ); await expect( router.executeAsync( router.resolveRequest(createRequestOptions('failure', SECOND_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { registerDisposable(context); throw new Error('global command failed'); }, new TestGlobalCommandClient() ) - ).rejects.toThrow('global command failed'); + ).resolves.toMatchObject({ + errorMessage: 'global command failed', + exitCode: 1, + outcome: 'failure' + }); expect(requestDisposeCount).toBe(2); expect(sessionDisposeCount).toBe(0); @@ -336,7 +438,7 @@ describe(GlobalCommandRequestRouter.name, () => { router.resolveRequest( createRequestOptions('cancelled', FIRST_CWD, { RUSHD_CONTEXT_TEST: 'child' }, 80) ), - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { context.registerDisposable({ [Symbol.asyncDispose]: (): Promise => { resourceDisposed = true; @@ -346,6 +448,7 @@ describe(GlobalCommandRequestRouter.name, () => { const child = context.spawnChild(process.execPath, ['-e', 'setInterval(() => {}, 1000)']); child.once('spawn', () => markChildStarted?.()); await new Promise((resolve) => child.once('close', () => resolve())); + return { exitCode: 0 }; }, client ); @@ -353,7 +456,13 @@ describe(GlobalCommandRequestRouter.name, () => { await childStarted; client.abortController.abort(new Error('client cancelled')); - await expect(resultPromise).resolves.toEqual({ aborted: true, requestId: 'cancelled' }); + await expect(resultPromise).resolves.toEqual({ + aborted: true, + errorMessage: undefined, + exitCode: 1, + outcome: 'aborted', + requestId: 'cancelled' + }); expect(resourceDisposed).toBe(true); expect(killProcessTreeOnExitSpy).toHaveBeenCalledTimes(1); expect(killProcessTreeSpy).toHaveBeenCalledTimes(1); @@ -376,10 +485,11 @@ describe(GlobalCommandRequestRouter.name, () => { }); const resultPromise: Promise = router.executeAsync( router.resolveRequest(createRequestOptions('cooperative-cancel', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { await waitForAbortAsync(context.abortSignal); await executorRelease; executorSettled = true; + return { exitCode: 0 }; }, client ); @@ -395,6 +505,9 @@ describe(GlobalCommandRequestRouter.name, () => { await expect(resultPromise).resolves.toEqual({ aborted: true, + errorMessage: undefined, + exitCode: 1, + outcome: 'aborted', requestId: 'cooperative-cancel' }); expect(executorSettled).toBe(true); @@ -414,13 +527,14 @@ describe(GlobalCommandRequestRouter.name, () => { }); const resultPromise: Promise = router.executeAsync( router.resolveRequest(createRequestOptions('cleanup-cancel', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async (context: IGlobalCommandExecutionContext): Promise => { context.registerDisposable({ [Symbol.asyncDispose]: async (): Promise => { markDisposalStarted?.(); await disposalRelease; } }); + return { exitCode: 0 }; }, client ); @@ -431,10 +545,125 @@ describe(GlobalCommandRequestRouter.name, () => { await expect(resultPromise).resolves.toEqual({ aborted: true, + errorMessage: undefined, + exitCode: 1, + outcome: 'aborted', + requestId: 'cleanup-cancel' + }); + }); + + it('preserves cancellation when the executor rejects while aborting', async () => { + const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter( + new TestWorkspaceSession(TEST_REPO_ROOT) + ); + const client: TestGlobalCommandClient = new TestGlobalCommandClient(); + const resultPromise: Promise = router.executeAsync( + router.resolveRequest(createRequestOptions('failed-cancel', FIRST_CWD, {}, 80)), + async (context: IGlobalCommandExecutionContext): Promise => { + await waitForAbortAsync(context.abortSignal); + throw new Error('executor failed while aborting'); + }, + client + ); + + client.abortController.abort(); + + await expect(resultPromise).resolves.toEqual({ + aborted: true, + errorMessage: 'executor failed while aborting', + exitCode: 1, + outcome: 'failure', + requestId: 'failed-cancel' + }); + }); + + it('preserves a client abort observed during cleanup', async () => { + const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter( + new TestWorkspaceSession(TEST_REPO_ROOT) + ); + const client: TestGlobalCommandClient = new TestGlobalCommandClient(); + let releaseCleanup: (() => void) | undefined; + let markCleanupStarted: (() => void) | undefined; + const cleanupStarted: Promise = new Promise((resolve) => { + markCleanupStarted = resolve; + }); + const cleanupRelease: Promise = new Promise((resolve) => { + releaseCleanup = resolve; + }); + const resultPromise: Promise = router.executeAsync( + router.resolveRequest(createRequestOptions('cleanup-cancel', FIRST_CWD, {}, 80)), + async (context: IGlobalCommandExecutionContext): Promise => { + context.registerDisposable({ + [Symbol.asyncDispose]: async (): Promise => { + markCleanupStarted?.(); + await cleanupRelease; + } + }); + return { exitCode: 0 }; + }, + client + ); + await cleanupStarted; + client.abortController.abort(new Error('client cancelled during cleanup')); + releaseCleanup?.(); + + await expect(resultPromise).resolves.toEqual({ + aborted: true, + errorMessage: undefined, + exitCode: 1, + outcome: 'aborted', requestId: 'cleanup-cancel' }); }); + it('preserves a delayed terminal disconnect observed during cleanup', async () => { + const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter( + new TestWorkspaceSession(TEST_REPO_ROOT) + ); + const client: TestGlobalCommandClient = new TestGlobalCommandClient(); + let releaseWrite: (() => void) | undefined; + const writeRelease: Promise = new Promise((resolve) => { + releaseWrite = resolve; + }); + client.onWriteAsync = async (): Promise => { + await writeRelease; + throw new Error('client disconnected during cleanup'); + }; + let releaseCleanup: (() => void) | undefined; + let markCleanupStarted: (() => void) | undefined; + const cleanupStarted: Promise = new Promise((resolve) => { + markCleanupStarted = resolve; + }); + const cleanupRelease: Promise = new Promise((resolve) => { + releaseCleanup = resolve; + }); + const resultPromise: Promise = router.executeAsync( + router.resolveRequest(createRequestOptions('cleanup-disconnect', FIRST_CWD, {}, 80)), + async (context: IGlobalCommandExecutionContext): Promise => { + context.registerDisposable({ + [Symbol.asyncDispose]: async (): Promise => { + markCleanupStarted?.(); + await cleanupRelease; + } + }); + context.terminal.writeLine('pending output'); + return { exitCode: 0 }; + }, + client + ); + await cleanupStarted; + releaseWrite?.(); + releaseCleanup?.(); + + await expect(resultPromise).resolves.toEqual({ + aborted: true, + errorMessage: 'client disconnected during cleanup', + exitCode: 1, + outcome: 'failure', + requestId: 'cleanup-disconnect' + }); + }); + it('continues request cleanup after a disposer throws synchronously', async () => { const session: TestWorkspaceSession = new TestWorkspaceSession(TEST_REPO_ROOT); const router: GlobalCommandRequestRouter = new GlobalCommandRequestRouter(session); @@ -443,7 +672,9 @@ describe(GlobalCommandRequestRouter.name, () => { await expect( router.executeAsync( router.resolveRequest(createRequestOptions('cleanup-errors', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async ( + context: IGlobalCommandExecutionContext + ): Promise => { context.registerDisposable(createRecordingDisposable('first', disposalOrder)); context.registerDisposable({ [Symbol.asyncDispose]: (): Promise => { @@ -452,10 +683,15 @@ describe(GlobalCommandRequestRouter.name, () => { } }); context.registerDisposable(createRecordingDisposable('last', disposalOrder)); + return { exitCode: 0 }; }, new TestGlobalCommandClient() ) - ).rejects.toThrow('synchronous cleanup failure'); + ).resolves.toMatchObject({ + errorMessage: 'synchronous cleanup failure', + exitCode: 1, + outcome: 'failure' + }); expect(disposalOrder).toEqual(['last', 'throwing', 'first']); }); @@ -465,11 +701,14 @@ describe(GlobalCommandRequestRouter.name, () => { const client: TestGlobalCommandClient = new TestGlobalCommandClient(); let resourceDisposed: boolean = false; client.onWriteAsync = (): Promise => Promise.reject(new Error('client disconnected')); + client.onResultAsync = (): Promise => Promise.reject(new Error('client disconnected')); await expect( router.executeAsync( router.resolveRequest(createRequestOptions('disconnect', FIRST_CWD, {}, 80)), - async (context: IGlobalCommandExecutionContext): Promise => { + async ( + context: IGlobalCommandExecutionContext + ): Promise => { context.registerDisposable({ [Symbol.asyncDispose]: (): Promise => { resourceDisposed = true; @@ -478,6 +717,7 @@ describe(GlobalCommandRequestRouter.name, () => { }); context.terminal.writeLine('disconnect'); await waitForAbortAsync(context.abortSignal); + return { exitCode: 0 }; }, client ) @@ -501,10 +741,13 @@ describe(GlobalCommandRequestRouter.name, () => { const request: IResolvedGlobalCommandRequest = firstRouter.resolveRequest( createRequestOptions('first-workspace', FIRST_CWD, {}, 80) ); - const executor: jest.Mock, [IGlobalCommandExecutionContext]> = jest.fn( + const executor: jest.Mock< + Promise, + [IGlobalCommandExecutionContext] + > = jest.fn( (context: IGlobalCommandExecutionContext) => { void context; - return Promise.resolve(); + return Promise.resolve({ exitCode: 0 }); } ); diff --git a/libraries/rush-daemon/src/test/PhasedRequestRouter.test.ts b/libraries/rush-daemon/src/test/PhasedRequestRouter.test.ts index e5f2bf9471..3462ccb9e3 100644 --- a/libraries/rush-daemon/src/test/PhasedRequestRouter.test.ts +++ b/libraries/rush-daemon/src/test/PhasedRequestRouter.test.ts @@ -35,6 +35,7 @@ function createRequest( return { commandName: 'build', engineShape: TEST_ENGINE_SHAPE, + environment: {}, operationSelection, requestId: 'request-1' }; @@ -184,10 +185,11 @@ describe(PhasedRequestRouter.name, () => { fixture.graph, 'executeScheduledIterationAsync' ); + const client: TestPhasedRequestClient = new TestPhasedRequestClient(); const result = await new PhasedRequestRouter(fixture.session).executeAsync( createRequest([select(OPERATION_B)]), - new TestPhasedRequestClient() + client ); expect(order).toEqual(['reconcile', 'schedule', 'run']); @@ -201,6 +203,8 @@ describe(PhasedRequestRouter.name, () => { OPERATION_B ]); expect(result.scheduled).toBe(true); + expect(result).toMatchObject({ exitCode: 0, outcome: 'success' }); + expect(clientResultWrites(client)).toEqual([{ result }]); }); it('forwards only enabled operations with ordered client backpressure', async () => { @@ -234,13 +238,19 @@ describe(PhasedRequestRouter.name, () => { OPERATION_A ]); expect(logWrites.map(({ stream }) => stream)).toEqual(['stdout', 'stderr']); - expect(logWrites.map(({ text }) => text)).toEqual(['stdout-a\n', 'stderr-a\n']); + expect(logWrites[0]?.text).toContain('stdout-a'); + expect(logWrites[1]?.text).toContain('stderr-a'); const eventOperationIds: string[] = client.writes .map((write: ITestClientWrite) => write.event) .filter((event: IDaemonEventEnvelope | undefined): event is IDaemonEventEnvelope => !!event) .map(getEventOperationId) .filter((operationId: string | undefined): operationId is string => !!operationId); expect(new Set(eventOperationIds)).toEqual(new Set([OPERATION_A])); + expect(clientResultWrites(client)).toHaveLength(1); + expect(client.writes[client.writes.length - 1]?.result).toMatchObject({ + exitCode: 0, + outcome: 'success' + }); const streamClosedEvent: IDaemonEventEnvelope | undefined = client.writes .map((write: ITestClientWrite) => write.event) .find( @@ -300,6 +310,36 @@ describe(PhasedRequestRouter.name, () => { expect(result.operationResults).toEqual([ { errorMessage: undefined, operationId: OPERATION_A, status: OperationStatus.Failure } ]); + expect(result).toMatchObject({ exitCode: 1, outcome: 'failure' }); + }); + + it('preserves cancellation when warning environment validation fails', async () => { + const fixture: ITestRoutingFixture = createThreeOperationFixture(); + const client: TestPhasedRequestClient = new TestPhasedRequestClient(); + const abortSignalReadForResult: number = 2; + let abortSignalReadCount: number = 0; + jest.spyOn(client, 'abortSignal', 'get').mockImplementation(() => { + abortSignalReadCount++; + if (abortSignalReadCount === abortSignalReadForResult) { + client.abortController.abort(new Error('client cancelled')); + } + return client.abortController.signal; + }); + + const result = await new PhasedRequestRouter(fixture.session).executeAsync( + { + ...createRequest([select(OPERATION_A)]), + environment: { RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: 'invalid' } + }, + client + ); + + expect(result).toMatchObject({ + aborted: true, + errorMessage: expect.stringContaining('must be set to 1 or 0'), + exitCode: 1, + outcome: 'failure' + }); }); it('aborts a cancelled iteration, restores subscriptions, and keeps runners reusable', async () => { @@ -336,6 +376,7 @@ describe(PhasedRequestRouter.name, () => { const result = await requestPromise; expect(result.aborted).toBe(true); + expect(result).toMatchObject({ exitCode: 1, outcome: 'aborted' }); expect( result.operationResults.find(({ operationId }) => operationId === OPERATION_B)?.status ).toBe(OperationStatus.Aborted); @@ -419,7 +460,12 @@ describe(PhasedRequestRouter.name, () => { createRequest([select(OPERATION_B)]), client ) - ).rejects.toThrow('client disconnected'); + ).resolves.toMatchObject({ + errorMessage: 'client disconnected', + exitCode: 1, + outcome: 'failure' + }); + expect(clientResultWrites(client)).toHaveLength(1); expect(fixture.graph.pauseNextIteration).toBe(false); expect(fixture.runners.get(OPERATION_A)?.closeCount).toBe(0); }); @@ -478,12 +524,15 @@ describe(PhasedRequestRouter.name, () => { throw new Error('scheduling hook failed'); }); - await expect( - new PhasedRequestRouter(fixture.session).executeAsync( - createRequest([select(OPERATION_A)]), - new TestPhasedRequestClient() - ) - ).rejects.toThrow('scheduling hook failed'); + const result = await new PhasedRequestRouter(fixture.session).executeAsync( + createRequest([select(OPERATION_A)]), + new TestPhasedRequestClient() + ); + expect(result).toMatchObject({ + errorMessage: 'scheduling hook failed', + exitCode: 1, + outcome: 'failure' + }); expect(fixture.graph.hasScheduledIteration).toBe(false); expect(fixture.graph.status).not.toBe(OperationStatus.Executing); const completedRunCount: number = fixture.runners.get(OPERATION_A)?.runCount ?? 0; @@ -491,6 +540,10 @@ describe(PhasedRequestRouter.name, () => { expect(fixture.runners.get(OPERATION_A)?.runCount).toBe(completedRunCount); }); + function clientResultWrites(client: TestPhasedRequestClient): ITestClientWrite[] { + return client.writes.filter(({ result }) => result !== undefined); + } + it('returns retained results when real graph hooks collapse a repeated warm request to no work', async () => { const fixture: ITestRoutingFixture = createThreeOperationFixture(); let iteration: number = 0; diff --git a/libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts b/libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts index bde8d5885c..251de684a5 100644 --- a/libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts +++ b/libraries/rush-daemon/src/test/PhasedRequestRouterTestUtilities.ts @@ -15,7 +15,10 @@ import type { import { Operation, OperationStatus } from '@microsoft/rush-lib'; import { OperationGraph } from '@microsoft/rush-lib/lib/logic/operations/OperationGraph'; import type { IOperationGraphOptions } from '@microsoft/rush-lib/lib/logic/operations/OperationGraph'; -import type { IDaemonEventEnvelope } from '@rushstack/rush-daemon-protocol'; +import type { + IDaemonEventEnvelope, + IDaemonPhasedRequestResult +} from '@rushstack/rush-daemon-protocol'; import type { IPhasedRequestClient } from '../PhasedRequestClient'; import type { @@ -47,6 +50,7 @@ const TEST_PHASE: IPhase = { export interface ITestClientWrite { readonly event?: IDaemonEventEnvelope; readonly operationId?: string; + readonly result?: IDaemonPhasedRequestResult; readonly stream?: 'stdout' | 'stderr'; readonly text?: string; } @@ -91,6 +95,12 @@ export class TestPhasedRequestClient implements IPhasedRequestClient { await this.onWriteAsync?.(write); this.writes.push(write); } + + public async writeResultAsync(result: IDaemonPhasedRequestResult): Promise { + const write: ITestClientWrite = { result }; + await this.onWriteAsync?.(write); + this.writes.push(write); + } } export class TestOperationRunner implements IOperationRunner {