From 55e1bf44945cb6d0815f97b67884511a24abe5db Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:19:50 +0000 Subject: [PATCH 1/6] [WS0-T4] Per-iteration host-driven runner persist policy Move IPC runner teardown into the operation graph's per-iteration options while preserving resident runners when no policy is supplied. Cover persistent, one-shot, changing, and default policies across successive iterations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- .../rush/bmiddha-runner-persist-policy.json | 9 +++ common/reviews/api/rush-lib.api.md | 1 + .../src/logic/operations/IOperationGraph.ts | 9 +++ .../logic/operations/IPCOperationRunner.ts | 10 --- .../operations/IPCOperationRunnerPlugin.ts | 1 - .../src/logic/operations/OperationGraph.ts | 29 ++++++- .../operations/test/OperationGraph.test.ts | 79 +++++++++++++++++-- 7 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json diff --git a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json new file mode 100644 index 00000000000..c084a6f1098 --- /dev/null +++ b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add a per-iteration, host-driven runner persist policy so IPC runners can be kept hot or torn down per operation per iteration, instead of fixing persistence at plugin construction.", + "type": "minor" + } + ] +} diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 6f21ecfaec3..4d1eebbb418 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -651,6 +651,7 @@ export interface IOperationGraphContext extends ICreateOperationsContext { // @alpha export interface IOperationGraphIterationOptions { + getRunnerPersistence?: (operation: Operation) => boolean; // (undocumented) inputsSnapshot?: IInputsSnapshot; startTime?: number; diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 294a6205675..69c3f43f15d 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -21,6 +21,15 @@ export interface IOperationGraphIterationOptions { * The time when the iteration was scheduled, if available, as returned by `performance.now()`. */ startTime?: number; + + /** + * Returns whether the operation's runner should remain active after this iteration. + * + * @remarks + * When omitted, all runners remain active. Returning `false` causes the operation's runner to be closed + * after the iteration, including when the operation was disabled for that iteration. + */ + getRunnerPersistence?: (operation: Operation) => boolean; } /** diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 6d8396591c5..2ad46169a42 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -28,7 +28,6 @@ export interface IIPCOperationRunnerOptions { initialCommand: string; incrementalCommand: string | undefined; commandForHash: string; - persist: boolean; ignoredParameterValues: ReadonlyArray; } @@ -58,7 +57,6 @@ export class IPCOperationRunner implements IOperationRunner { private readonly _initialCommand: string; private readonly _incrementalCommand: string | undefined; private readonly _commandForHash: string; - private readonly _persist: boolean; private readonly _ignoredParameterValues: ReadonlyArray; private _ipcProcess: ChildProcess | undefined; @@ -72,7 +70,6 @@ export class IPCOperationRunner implements IOperationRunner { initialCommand, incrementalCommand, commandForHash, - persist, ignoredParameterValues } = options; this.name = name; @@ -83,7 +80,6 @@ export class IPCOperationRunner implements IOperationRunner { this._incrementalCommand = incrementalCommand; this._commandForHash = commandForHash; - this._persist = persist; this._ignoredParameterValues = ignoredParameterValues; } @@ -100,7 +96,6 @@ export class IPCOperationRunner implements IOperationRunner { const invalidate: (reason: string) => void = context.getInvalidateCallback(); return await context.runWithTerminalAsync( async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise => { - let isConnected: boolean = false; if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') { // Log any ignored parameters if (this._ignoredParameterValues.length > 0) { @@ -204,7 +199,6 @@ export class IPCOperationRunner implements IOperationRunner { subProcess.on('exit', onExit); this._processReadyPromise!.then(() => { - isConnected = true; terminal.writeLine('Child supports IPC protocol. Sending "run" command...'); const runCommand: IRunCommandMessage = { command: 'run' @@ -213,10 +207,6 @@ export class IPCOperationRunner implements IOperationRunner { }, reject); }); - if (isConnected && !this._persist) { - await this.closeAsync(); - } - // @rushstack/operation-graph does not currently have a concept of "Success with Warning" // To match existing ShellOperationRunner behavior we treat any stderr as a warning. return status === OperationStatus.Success && hasWarningOrError diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts index 7d5cccd7851..1823c22b4f1 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunnerPlugin.ts @@ -81,7 +81,6 @@ export class IPCOperationRunnerPlugin implements IPhasedCommandPlugin { initialCommand, incrementalCommand, commandForHash, - persist: true, ignoredParameterValues }); diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 8d01d746186..9ab09fe56a6 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -85,6 +85,7 @@ interface IExecutionIterationContext extends IOperationExecutionRecordContext { promise: Promise | undefined; startTime?: number; + getRunnerPersistence: IOperationGraphIterationOptions['getRunnerPersistence']; completedOperations: number; totalOperations: number; @@ -572,9 +573,16 @@ export class OperationGraph implements IOperationGraph { ): Promise { const { _getInputsSnapshotAsync: getInputsSnapshotAsync } = this; - const { startTime = performance.now(), inputsSnapshot = await getInputsSnapshotAsync?.() } = - iterationOptions; - const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { startTime, inputsSnapshot }; + const { + startTime = performance.now(), + inputsSnapshot = await getInputsSnapshotAsync?.(), + getRunnerPersistence + } = iterationOptions; + const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { + startTime, + inputsSnapshot, + getRunnerPersistence + }; const { hooks } = this; @@ -607,6 +615,7 @@ export class OperationGraph implements IOperationGraph { const iterationContext: IExecutionIterationContext = { abortController, startTime, + getRunnerPersistence, streamCollator, terminal, inputsSnapshot, @@ -757,7 +766,8 @@ export class OperationGraph implements IOperationGraph { const iterationOptions: IOperationGraphIterationOptions = { inputsSnapshot: iterationContext.inputsSnapshot, - startTime: iterationContext.startTime + startTime: iterationContext.startTime, + getRunnerPersistence: iterationContext.getRunnerPersistence }; const executionQueue: AsyncOperationQueue = new AsyncOperationQueue( @@ -1011,6 +1021,17 @@ export class OperationGraph implements IOperationGraph { telemetry.log(logEntry); } + const { getRunnerPersistence } = iterationContext; + if (getRunnerPersistence) { + const operationsToClose: Operation[] = []; + for (const operation of this.operations) { + if (!getRunnerPersistence(operation)) { + operationsToClose.push(operation); + } + } + await this.closeRunnersAsync(operationsToClose); + } + return status; // This function is a callback because it may write to the collatedWriter before diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index 913f15f705a..a4381421efc 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -107,6 +107,12 @@ function createGraph( return new OperationGraph(new Set([operation]), graphOptions); } +class ClosableRunner extends MockOperationRunner { + public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { + /* no-op */ + }); +} + describe('OperationGraph', () => { let graphOptions: IOperationGraphOptions; let graphIterationOptions: IOperationGraphIterationOptions; @@ -1134,13 +1140,76 @@ describe('deferred invalidation during active iteration', () => { }); }); -describe('closeRunnersAsync', () => { - class ClosableRunner extends MockOperationRunner { - public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { - /* no-op */ +describe('runner persistence policy', () => { + const graphOptions: IOperationGraphOptions = { + quietMode: false, + debugMode: false, + parallelism: 3, + allowOversubscription: true, + destinations: [mockWritable], + abortController: new AbortController() + }; + + it('keeps runners active across successive iterations when no policy is provided', async () => { + const runAsync: jest.Mock, []> = jest.fn(async () => OperationStatus.Success); + const runner: ClosableRunner = new ClosableRunner('default-persistent', runAsync); + const graph: OperationGraph = createGraph(graphOptions, runner); + + await graph.executeAsync({}); + expect(runAsync).toHaveBeenCalledTimes(1); + expect(runner.closeAsync).not.toHaveBeenCalled(); + + await graph.executeAsync({}); + expect(runAsync).toHaveBeenCalledTimes(2); + expect(runner.closeAsync).not.toHaveBeenCalled(); + }); + + it('applies per-operation policies independently on each iteration', async () => { + const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); + const oneShotRunner: ClosableRunner = new ClosableRunner('one-shot'); + const changingRunner: ClosableRunner = new ClosableRunner('changing'); + + const persistentOperation: Operation = new Operation({ + runner: persistentRunner, + logFilenameIdentifier: 'persistent', + phase: mockPhase, + project: getOrCreateProject('persistent') + }); + const oneShotOperation: Operation = new Operation({ + runner: oneShotRunner, + logFilenameIdentifier: 'one-shot', + phase: mockPhase, + project: getOrCreateProject('one-shot') + }); + const changingOperation: Operation = new Operation({ + runner: changingRunner, + logFilenameIdentifier: 'changing', + phase: mockPhase, + project: getOrCreateProject('changing') }); - } + const graph: OperationGraph = new OperationGraph( + new Set([persistentOperation, oneShotOperation, changingOperation]), + graphOptions + ); + + await graph.executeAsync({ + getRunnerPersistence: (operation) => operation !== oneShotOperation + }); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(changingRunner.closeAsync).not.toHaveBeenCalled(); + + await graph.executeAsync({ + getRunnerPersistence: (operation) => operation === persistentOperation + }); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(2); + expect(changingRunner.closeAsync).toHaveBeenCalledTimes(1); + }); +}); + +describe('closeRunnersAsync', () => { it('invokes closeAsync on runners and triggers onExecutionStatesUpdated hook', async () => { const localOptions: IOperationGraphOptions = { quietMode: false, From 94af331f342c79316a925f295461bc759178ff8f Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:26:16 -0700 Subject: [PATCH 2/6] Apply suggestion from @bmiddha --- .../changes/@microsoft/rush/bmiddha-runner-persist-policy.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json index c084a6f1098..336d44ae1ae 100644 --- a/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json +++ b/common/changes/@microsoft/rush/bmiddha-runner-persist-policy.json @@ -3,7 +3,7 @@ { "packageName": "@microsoft/rush", "comment": "Add a per-iteration, host-driven runner persist policy so IPC runners can be kept hot or torn down per operation per iteration, instead of fixing persistence at plugin construction.", - "type": "minor" + "type": "patch" } ] } From 4854b5bde1d451e8fac9b428afe0c54cdaad6a0a Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:42:59 +0000 Subject: [PATCH 3/6] Address runner persistence review feedback Close nonpersistent IPC runners before downstream execution, retain fallback cleanup for bypassed active runners, and rename the policy to shouldRunnerPersist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- common/reviews/api/rush-lib.api.md | 3 +- .../src/logic/operations/IOperationGraph.ts | 7 +- .../src/logic/operations/IOperationRunner.ts | 11 ++ .../logic/operations/IPCOperationRunner.ts | 10 ++ .../operations/OperationExecutionRecord.ts | 9 ++ .../src/logic/operations/OperationGraph.ts | 19 +-- .../operations/test/OperationGraph.test.ts | 108 +++++++++++++++++- 7 files changed, 152 insertions(+), 15 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 4d1eebbb418..39f0fced3f8 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -651,9 +651,9 @@ export interface IOperationGraphContext extends ICreateOperationsContext { // @alpha export interface IOperationGraphIterationOptions { - getRunnerPersistence?: (operation: Operation) => boolean; // (undocumented) inputsSnapshot?: IInputsSnapshot; + shouldRunnerPersist?: (operation: Operation) => boolean; startTime?: number; } @@ -722,6 +722,7 @@ export interface IOperationRunnerContext { createLogFile: boolean; logFileSuffix?: string; }): Promise; + shouldRunnerPersist?: boolean; status: OperationStatus; stopwatch: IStopwatchResult; } diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 69c3f43f15d..768fb2c2bcf 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -26,10 +26,11 @@ export interface IOperationGraphIterationOptions { * Returns whether the operation's runner should remain active after this iteration. * * @remarks - * When omitted, all runners remain active. Returning `false` causes the operation's runner to be closed - * after the iteration, including when the operation was disabled for that iteration. + * When omitted, all runners remain active. Returning `false` causes a runner that supports immediate + * teardown to close when its operation completes. Any cold runner that remains active because its operation + * did not execute is closed after the iteration. */ - getRunnerPersistence?: (operation: Operation) => boolean; + shouldRunnerPersist?: (operation: Operation) => boolean; } /** diff --git a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts index e8dc3f2d10d..5d4b27822df 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts @@ -69,6 +69,15 @@ export interface IOperationRunnerContext { */ error?: Error; + /** + * Whether the host wants this operation's runner to remain resident (kept warm) after the + * operation completes for the current iteration. Defaults to `true` when the host provides no + * persistence policy. Runners that own a long-lived child process (such as `IPCOperationRunner`) + * must release that process as soon as the operation completes when this is `false`, so that the + * subprocess does not consume memory while downstream operations execute. + */ + shouldRunnerPersist?: boolean; + /** * Returns a callback that invalidates this operation so that it will be re-executed in the next iteration. * The returned callback captures only the minimal state needed, avoiding retention of the full context. @@ -134,6 +143,8 @@ export interface IOperationRunner { * If true, this runner currently owns some kind of active resource (e.g. a service or a watch process). * This can be used to determine if the operation is "in progress" even if it is not currently executing. * If the runner supports this property, it should update it as appropriate during execution. + * A runner that closes its resource during `executeAsync()` when `context.shouldRunnerPersist` is false + * must report `false` afterward so that the graph's fallback cleanup does not close it again. * The property is optional to avoid breaking existing implementations of IOperationRunner. */ readonly isActive?: boolean; diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 2ad46169a42..21c3acb9e83 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -96,6 +96,7 @@ export class IPCOperationRunner implements IOperationRunner { const invalidate: (reason: string) => void = context.getInvalidateCallback(); return await context.runWithTerminalAsync( async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise => { + let isConnected: boolean = false; if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') { // Log any ignored parameters if (this._ignoredParameterValues.length > 0) { @@ -199,6 +200,7 @@ export class IPCOperationRunner implements IOperationRunner { subProcess.on('exit', onExit); this._processReadyPromise!.then(() => { + isConnected = true; terminal.writeLine('Child supports IPC protocol. Sending "run" command...'); const runCommand: IRunCommandMessage = { command: 'run' @@ -207,6 +209,14 @@ export class IPCOperationRunner implements IOperationRunner { }, reject); }); + // If the host does not want this runner kept warm for the next iteration, release the + // long-lived subprocess immediately now that the operation has completed. Deferring this to + // the end of the iteration would keep the subprocess resident (consuming memory) while + // downstream operations execute, risking RAM exhaustion. + if (isConnected && context.shouldRunnerPersist === false) { + await this.closeAsync(); + } + // @rushstack/operation-graph does not currently have a concept of "Success with Warning" // To match existing ShellOperationRunner behavior we treat any stderr as a warning. return status === OperationStatus.Success && hasWarningOrError diff --git a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts index 2de7655d1d3..7252795b80d 100644 --- a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts +++ b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts @@ -46,6 +46,7 @@ export interface IOperationExecutionRecordContext { onOperationStateChanged?: (record: OperationExecutionRecord) => void; createEnvironment?: (record: OperationExecutionRecord) => IEnvironment; invalidate?: (operations: Iterable, reason: string) => void; + shouldRunnerPersist?: (operation: Operation) => boolean; inputsSnapshot: IInputsSnapshot | undefined; maxParallelism: number; @@ -222,6 +223,14 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera return this._context.createEnvironment?.(this); } + /** + * Whether this operation's runner should remain resident after the operation completes for the + * current iteration. Defaults to `true` (kept warm) when the host supplies no persistence policy. + */ + public get shouldRunnerPersist(): boolean { + return this._context.shouldRunnerPersist?.(this.operation) ?? true; + } + public getInvalidateCallback(): (reason: string) => void { const invalidateFn: ((operations: Iterable, reason: string) => void) | undefined = this._context.invalidate; diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 9ab09fe56a6..5cbfc254c43 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -85,7 +85,7 @@ interface IExecutionIterationContext extends IOperationExecutionRecordContext { promise: Promise | undefined; startTime?: number; - getRunnerPersistence: IOperationGraphIterationOptions['getRunnerPersistence']; + shouldRunnerPersist: IOperationGraphIterationOptions['shouldRunnerPersist']; completedOperations: number; totalOperations: number; @@ -576,12 +576,12 @@ export class OperationGraph implements IOperationGraph { const { startTime = performance.now(), inputsSnapshot = await getInputsSnapshotAsync?.(), - getRunnerPersistence + shouldRunnerPersist } = iterationOptions; const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { startTime, inputsSnapshot, - getRunnerPersistence + shouldRunnerPersist }; const { hooks } = this; @@ -615,7 +615,7 @@ export class OperationGraph implements IOperationGraph { const iterationContext: IExecutionIterationContext = { abortController, startTime, - getRunnerPersistence, + shouldRunnerPersist, streamCollator, terminal, inputsSnapshot, @@ -767,7 +767,7 @@ export class OperationGraph implements IOperationGraph { const iterationOptions: IOperationGraphIterationOptions = { inputsSnapshot: iterationContext.inputsSnapshot, startTime: iterationContext.startTime, - getRunnerPersistence: iterationContext.getRunnerPersistence + shouldRunnerPersist: iterationContext.shouldRunnerPersist }; const executionQueue: AsyncOperationQueue = new AsyncOperationQueue( @@ -1021,11 +1021,14 @@ export class OperationGraph implements IOperationGraph { telemetry.log(logEntry); } - const { getRunnerPersistence } = iterationContext; - if (getRunnerPersistence) { + // IPC operations that executed this iteration release their runner immediately upon completion. + // This fallback closes cold runners that remain active because their operation did not execute, + // for example due to a cache hit, abort, blocked dependency, or disabled operation. + const { shouldRunnerPersist } = iterationContext; + if (shouldRunnerPersist) { const operationsToClose: Operation[] = []; for (const operation of this.operations) { - if (!getRunnerPersistence(operation)) { + if (!shouldRunnerPersist(operation) && operation.runner?.isActive !== false) { operationsToClose.push(operation); } } diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index a4381421efc..50ee72ec07c 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -108,9 +108,23 @@ function createGraph( } class ClosableRunner extends MockOperationRunner { + public isActive: boolean = false; + public readonly closeAsync: jest.Mock, []> = jest.fn(async () => { - /* no-op */ + this.isActive = false; }); + + public override async executeAsync(context: IOperationRunnerContext): Promise { + this.isActive = true; + const status: OperationStatus = await super.executeAsync(context); + // Mirror IPCOperationRunner: a runner that owns a long-lived resource must release it + // immediately upon completion when the host has opted this operation out of persistence for + // the current iteration, rather than waiting until the end of the iteration. + if (context.shouldRunnerPersist === false) { + await this.closeAsync(); + } + return status; + } } describe('OperationGraph', () => { @@ -1194,19 +1208,107 @@ describe('runner persistence policy', () => { ); await graph.executeAsync({ - getRunnerPersistence: (operation) => operation !== oneShotOperation + shouldRunnerPersist: (operation) => operation !== oneShotOperation }); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(1); expect(changingRunner.closeAsync).not.toHaveBeenCalled(); await graph.executeAsync({ - getRunnerPersistence: (operation) => operation === persistentOperation + shouldRunnerPersist: (operation) => operation === persistentOperation }); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(2); expect(changingRunner.closeAsync).toHaveBeenCalledTimes(1); }); + + it('releases a non-persistent runner before its dependents execute', async () => { + const executionOrder: string[] = []; + + const upstreamRunner: ClosableRunner = new ClosableRunner('upstream', async () => { + executionOrder.push('upstream:run'); + return OperationStatus.Success; + }); + upstreamRunner.closeAsync.mockImplementation(async () => { + executionOrder.push('upstream:close'); + upstreamRunner.isActive = false; + }); + + const downstreamRunner: ClosableRunner = new ClosableRunner('downstream', async () => { + executionOrder.push('downstream:run'); + return OperationStatus.Success; + }); + + const upstreamOperation: Operation = new Operation({ + runner: upstreamRunner, + logFilenameIdentifier: 'upstream', + phase: mockPhase, + project: getOrCreateProject('upstream') + }); + const downstreamOperation: Operation = new Operation({ + runner: downstreamRunner, + logFilenameIdentifier: 'downstream', + phase: mockPhase, + project: getOrCreateProject('downstream') + }); + downstreamOperation.addDependency(upstreamOperation); + + const graph: OperationGraph = new OperationGraph( + new Set([upstreamOperation, downstreamOperation]), + graphOptions + ); + + await graph.executeAsync({ + shouldRunnerPersist: (operation) => operation !== upstreamOperation + }); + + expect(upstreamRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(downstreamRunner.closeAsync).not.toHaveBeenCalled(); + // The non-persistent upstream runner must be released immediately upon its own completion, + // before the downstream operation begins executing — not deferred to the end of the iteration. + expect(executionOrder).toEqual(['upstream:run', 'upstream:close', 'downstream:run']); + }); + + it('closes an active cold runner when an iteration bypasses runner execution', async () => { + const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); + const coldRunner: ClosableRunner = new ClosableRunner('cold'); + + const persistentOperation: Operation = new Operation({ + runner: persistentRunner, + logFilenameIdentifier: 'persistent', + phase: mockPhase, + project: getOrCreateProject('persistent') + }); + const coldOperation: Operation = new Operation({ + runner: coldRunner, + logFilenameIdentifier: 'cold', + phase: mockPhase, + project: getOrCreateProject('cold') + }); + + const graph: OperationGraph = new OperationGraph( + new Set([persistentOperation, coldOperation]), + graphOptions + ); + + await graph.executeAsync({}); + expect(persistentRunner.isActive).toBe(true); + expect(coldRunner.isActive).toBe(true); + + graph.hooks.beforeExecuteIterationAsync.tapPromise( + 'test', + async (): Promise => OperationStatus.FromCache + ); + + await graph.executeAsync({ + shouldRunnerPersist: (operation) => operation === persistentOperation + }); + + expect(coldRunner.closeAsync).toHaveBeenCalledTimes(1); + expect(coldRunner.isActive).toBe(false); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + expect(persistentRunner.isActive).toBe(true); + }); }); describe('closeRunnersAsync', () => { From 9eb5f0ca2e809e6fb2d5fea1e611f820ea488b08 Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:00:36 +0000 Subject: [PATCH 4/6] Configure runner persistence on execution records Replace the per-iteration predicate with a mutable record boolean configured alongside enabled. Keep runner teardown in OperationGraph so cold runners close before dependents execute, with a pre-after-iteration fallback for bypassed records. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- common/reviews/api/rush-lib.api.md | 4 +- .../operations/IOperationExecutionResult.ts | 10 ++++ .../src/logic/operations/IOperationGraph.ts | 10 ---- .../src/logic/operations/IOperationRunner.ts | 11 ---- .../logic/operations/IPCOperationRunner.ts | 11 ---- .../operations/OperationExecutionRecord.ts | 14 ++--- .../src/logic/operations/OperationGraph.ts | 54 +++++++++---------- .../operations/test/OperationGraph.test.ts | 52 ++++++++++-------- .../pluginFramework/OperationGraphHooks.ts | 3 +- 9 files changed, 75 insertions(+), 94 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 39f0fced3f8..d99a8071cd9 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -407,6 +407,7 @@ export interface ICobuildLockProvider { // @alpha export interface IConfigurableOperation extends IBaseOperationExecutionResult { enabled: boolean; + shouldRunnerPersist: boolean; } // @public @@ -613,6 +614,7 @@ export interface IOperationExecutionResult extends IBaseOperationExecutionResult readonly logFilePaths: ILogFilePaths | undefined; readonly nonCachedDurationMs: number | undefined; readonly problemCollector: IProblemCollector; + readonly shouldRunnerPersist: boolean; readonly silent: boolean; readonly status: OperationStatus; readonly stdioSummarizer: StdioSummarizer; @@ -653,7 +655,6 @@ export interface IOperationGraphContext extends ICreateOperationsContext { export interface IOperationGraphIterationOptions { // (undocumented) inputsSnapshot?: IInputsSnapshot; - shouldRunnerPersist?: (operation: Operation) => boolean; startTime?: number; } @@ -722,7 +723,6 @@ export interface IOperationRunnerContext { createLogFile: boolean; logFileSuffix?: string; }): Promise; - shouldRunnerPersist?: boolean; status: OperationStatus; stopwatch: IStopwatchResult; } diff --git a/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts b/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts index 8df76ec1b69..dfe8e307156 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts @@ -67,6 +67,12 @@ export interface IConfigurableOperation extends IBaseOperationExecutionResult { * True if the operation should execute in this iteration, false otherwise. */ enabled: boolean; + + /** + * True if the operation's runner should remain active after this iteration, false otherwise. + * Defaults to true. + */ + shouldRunnerPersist: boolean; } /** @@ -94,6 +100,10 @@ export interface IOperationExecutionResult extends IBaseOperationExecutionResult * True if the operation should execute in this iteration, false otherwise. */ readonly enabled: boolean; + /** + * True if the operation's runner should remain active after this iteration, false otherwise. + */ + readonly shouldRunnerPersist: boolean; /** * Object tracking execution timing. */ diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 768fb2c2bcf..294a6205675 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -21,16 +21,6 @@ export interface IOperationGraphIterationOptions { * The time when the iteration was scheduled, if available, as returned by `performance.now()`. */ startTime?: number; - - /** - * Returns whether the operation's runner should remain active after this iteration. - * - * @remarks - * When omitted, all runners remain active. Returning `false` causes a runner that supports immediate - * teardown to close when its operation completes. Any cold runner that remains active because its operation - * did not execute is closed after the iteration. - */ - shouldRunnerPersist?: (operation: Operation) => boolean; } /** diff --git a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts index 5d4b27822df..e8dc3f2d10d 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts @@ -69,15 +69,6 @@ export interface IOperationRunnerContext { */ error?: Error; - /** - * Whether the host wants this operation's runner to remain resident (kept warm) after the - * operation completes for the current iteration. Defaults to `true` when the host provides no - * persistence policy. Runners that own a long-lived child process (such as `IPCOperationRunner`) - * must release that process as soon as the operation completes when this is `false`, so that the - * subprocess does not consume memory while downstream operations execute. - */ - shouldRunnerPersist?: boolean; - /** * Returns a callback that invalidates this operation so that it will be re-executed in the next iteration. * The returned callback captures only the minimal state needed, avoiding retention of the full context. @@ -143,8 +134,6 @@ export interface IOperationRunner { * If true, this runner currently owns some kind of active resource (e.g. a service or a watch process). * This can be used to determine if the operation is "in progress" even if it is not currently executing. * If the runner supports this property, it should update it as appropriate during execution. - * A runner that closes its resource during `executeAsync()` when `context.shouldRunnerPersist` is false - * must report `false` afterward so that the graph's fallback cleanup does not close it again. * The property is optional to avoid breaking existing implementations of IOperationRunner. */ readonly isActive?: boolean; diff --git a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts index 21c3acb9e83..020e3732b0d 100644 --- a/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IPCOperationRunner.ts @@ -96,7 +96,6 @@ export class IPCOperationRunner implements IOperationRunner { const invalidate: (reason: string) => void = context.getInvalidateCallback(); return await context.runWithTerminalAsync( async (terminal: ITerminal, terminalProvider: ITerminalProvider): Promise => { - let isConnected: boolean = false; if (!this._ipcProcess || typeof this._ipcProcess.exitCode === 'number') { // Log any ignored parameters if (this._ignoredParameterValues.length > 0) { @@ -198,9 +197,7 @@ export class IPCOperationRunner implements IOperationRunner { subProcess.on('message', finishHandler); subProcess.on('error', reject); subProcess.on('exit', onExit); - this._processReadyPromise!.then(() => { - isConnected = true; terminal.writeLine('Child supports IPC protocol. Sending "run" command...'); const runCommand: IRunCommandMessage = { command: 'run' @@ -209,14 +206,6 @@ export class IPCOperationRunner implements IOperationRunner { }, reject); }); - // If the host does not want this runner kept warm for the next iteration, release the - // long-lived subprocess immediately now that the operation has completed. Deferring this to - // the end of the iteration would keep the subprocess resident (consuming memory) while - // downstream operations execute, risking RAM exhaustion. - if (isConnected && context.shouldRunnerPersist === false) { - await this.closeAsync(); - } - // @rushstack/operation-graph does not currently have a concept of "Success with Warning" // To match existing ShellOperationRunner behavior we treat any stderr as a warning. return status === OperationStatus.Success && hasWarningOrError diff --git a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts index 7252795b80d..424730af49e 100644 --- a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts +++ b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts @@ -46,7 +46,6 @@ export interface IOperationExecutionRecordContext { onOperationStateChanged?: (record: OperationExecutionRecord) => void; createEnvironment?: (record: OperationExecutionRecord) => IEnvironment; invalidate?: (operations: Iterable, reason: string) => void; - shouldRunnerPersist?: (operation: Operation) => boolean; inputsSnapshot: IInputsSnapshot | undefined; maxParallelism: number; @@ -85,6 +84,11 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera */ public enabled: boolean; + /** + * If true, this operation's runner should remain active after this iteration. + */ + public shouldRunnerPersist: boolean = true; + /** * This number represents how far away this Operation is from the furthest "root" operation (i.e. * an operation with no consumers). This helps us to calculate the critical path (i.e. the @@ -223,14 +227,6 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera return this._context.createEnvironment?.(this); } - /** - * Whether this operation's runner should remain resident after the operation completes for the - * current iteration. Defaults to `true` (kept warm) when the host supplies no persistence policy. - */ - public get shouldRunnerPersist(): boolean { - return this._context.shouldRunnerPersist?.(this.operation) ?? true; - } - public getInvalidateCallback(): (reason: string) => void { const invalidateFn: ((operations: Iterable, reason: string) => void) | undefined = this._context.invalidate; diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 5cbfc254c43..bca1e0e9f14 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -85,7 +85,6 @@ interface IExecutionIterationContext extends IOperationExecutionRecordContext { promise: Promise | undefined; startTime?: number; - shouldRunnerPersist: IOperationGraphIterationOptions['shouldRunnerPersist']; completedOperations: number; totalOperations: number; @@ -573,16 +572,9 @@ export class OperationGraph implements IOperationGraph { ): Promise { const { _getInputsSnapshotAsync: getInputsSnapshotAsync } = this; - const { - startTime = performance.now(), - inputsSnapshot = await getInputsSnapshotAsync?.(), - shouldRunnerPersist - } = iterationOptions; - const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { - startTime, - inputsSnapshot, - shouldRunnerPersist - }; + const { startTime = performance.now(), inputsSnapshot = await getInputsSnapshotAsync?.() } = + iterationOptions; + const iterationOptionsForCallbacks: IOperationGraphIterationOptions = { startTime, inputsSnapshot }; const { hooks } = this; @@ -615,7 +607,6 @@ export class OperationGraph implements IOperationGraph { const iterationContext: IExecutionIterationContext = { abortController, startTime, - shouldRunnerPersist, streamCollator, terminal, inputsSnapshot, @@ -766,8 +757,7 @@ export class OperationGraph implements IOperationGraph { const iterationOptions: IOperationGraphIterationOptions = { inputsSnapshot: iterationContext.inputsSnapshot, - startTime: iterationContext.startTime, - shouldRunnerPersist: iterationContext.shouldRunnerPersist + startTime: iterationContext.startTime }; const executionQueue: AsyncOperationQueue = new AsyncOperationQueue( @@ -809,6 +799,8 @@ export class OperationGraph implements IOperationGraph { onStartAsync: onOperationStartAsync, onResultAsync: onOperationCompleteAsync }; + const recordsWithRunnerCleanup: Set = new Set(); + const graph: OperationGraph = this; if (!this.quietMode) { const plural: string = totalOperations === 1 ? '' : 's'; @@ -883,6 +875,16 @@ export class OperationGraph implements IOperationGraph { }); } + const operationsToClose: Operation[] = []; + for (const [operation, record] of executionRecords) { + if (!recordsWithRunnerCleanup.has(record) && !record.shouldRunnerPersist) { + operationsToClose.push(operation); + } + } + if (operationsToClose.length > 0) { + await this.closeRunnersAsync(operationsToClose); + } + const status: OperationStatus = (() => { if (bailStatus) return bailStatus; if (state.hasAnyFailures) return OperationStatus.Failure; @@ -1021,20 +1023,6 @@ export class OperationGraph implements IOperationGraph { telemetry.log(logEntry); } - // IPC operations that executed this iteration release their runner immediately upon completion. - // This fallback closes cold runners that remain active because their operation did not execute, - // for example due to a cache hit, abort, blocked dependency, or disabled operation. - const { shouldRunnerPersist } = iterationContext; - if (shouldRunnerPersist) { - const operationsToClose: Operation[] = []; - for (const operation of this.operations) { - if (!shouldRunnerPersist(operation) && operation.runner?.isActive !== false) { - operationsToClose.push(operation); - } - } - await this.closeRunnersAsync(operationsToClose); - } - return status; // This function is a callback because it may write to the collatedWriter before @@ -1051,6 +1039,16 @@ export class OperationGraph implements IOperationGraph { record.error = e; record.status = OperationStatus.Failure; } + if (!record.shouldRunnerPersist) { + recordsWithRunnerCleanup.add(record); + try { + await graph.closeRunnersAsync([record.operation]); + } catch (e) { + _reportOperationErrorIfAny(record); + record.error = e; + record.status = OperationStatus.Failure; + } + } _onOperationComplete(record, state); } } diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index 50ee72ec07c..b9b195f53e6 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -116,17 +116,21 @@ class ClosableRunner extends MockOperationRunner { public override async executeAsync(context: IOperationRunnerContext): Promise { this.isActive = true; - const status: OperationStatus = await super.executeAsync(context); - // Mirror IPCOperationRunner: a runner that owns a long-lived resource must release it - // immediately upon completion when the host has opted this operation out of persistence for - // the current iteration, rather than waiting until the end of the iteration. - if (context.shouldRunnerPersist === false) { - await this.closeAsync(); - } - return status; + return await super.executeAsync(context); } } +function configureRunnerPersistence( + graph: OperationGraph, + shouldRunnerPersist: (operation: Operation) => boolean +): void { + graph.hooks.configureIteration.tap('test-runner-persistence', (records) => { + for (const [operation, record] of records) { + record.shouldRunnerPersist = shouldRunnerPersist(operation); + } + }); +} + describe('OperationGraph', () => { let graphOptions: IOperationGraphOptions; let graphIterationOptions: IOperationGraphIterationOptions; @@ -1164,7 +1168,7 @@ describe('runner persistence policy', () => { abortController: new AbortController() }; - it('keeps runners active across successive iterations when no policy is provided', async () => { + it('keeps runners active across successive iterations by default', async () => { const runAsync: jest.Mock, []> = jest.fn(async () => OperationStatus.Success); const runner: ClosableRunner = new ClosableRunner('default-persistent', runAsync); const graph: OperationGraph = createGraph(graphOptions, runner); @@ -1178,7 +1182,7 @@ describe('runner persistence policy', () => { expect(runner.closeAsync).not.toHaveBeenCalled(); }); - it('applies per-operation policies independently on each iteration', async () => { + it('applies configured record policies independently on each iteration', async () => { const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); const oneShotRunner: ClosableRunner = new ClosableRunner('one-shot'); const changingRunner: ClosableRunner = new ClosableRunner('changing'); @@ -1207,16 +1211,16 @@ describe('runner persistence policy', () => { graphOptions ); - await graph.executeAsync({ - shouldRunnerPersist: (operation) => operation !== oneShotOperation - }); + let persistentOperations: ReadonlySet = new Set([persistentOperation, changingOperation]); + configureRunnerPersistence(graph, (operation) => persistentOperations.has(operation)); + + await graph.executeAsync({}); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(1); expect(changingRunner.closeAsync).not.toHaveBeenCalled(); - await graph.executeAsync({ - shouldRunnerPersist: (operation) => operation === persistentOperation - }); + persistentOperations = new Set([persistentOperation]); + await graph.executeAsync({}); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(oneShotRunner.closeAsync).toHaveBeenCalledTimes(2); expect(changingRunner.closeAsync).toHaveBeenCalledTimes(1); @@ -1258,9 +1262,8 @@ describe('runner persistence policy', () => { graphOptions ); - await graph.executeAsync({ - shouldRunnerPersist: (operation) => operation !== upstreamOperation - }); + configureRunnerPersistence(graph, (operation) => operation !== upstreamOperation); + await graph.executeAsync({}); expect(upstreamRunner.closeAsync).toHaveBeenCalledTimes(1); expect(downstreamRunner.closeAsync).not.toHaveBeenCalled(); @@ -1272,6 +1275,7 @@ describe('runner persistence policy', () => { it('closes an active cold runner when an iteration bypasses runner execution', async () => { const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); const coldRunner: ClosableRunner = new ClosableRunner('cold'); + let coldRunnerWasClosedBeforeAfterIteration: boolean = false; const persistentOperation: Operation = new Operation({ runner: persistentRunner, @@ -1295,17 +1299,21 @@ describe('runner persistence policy', () => { expect(persistentRunner.isActive).toBe(true); expect(coldRunner.isActive).toBe(true); + configureRunnerPersistence(graph, (operation) => operation === persistentOperation); graph.hooks.beforeExecuteIterationAsync.tapPromise( 'test', async (): Promise => OperationStatus.FromCache ); - - await graph.executeAsync({ - shouldRunnerPersist: (operation) => operation === persistentOperation + graph.hooks.afterExecuteIterationAsync.tapPromise('test', async (status) => { + coldRunnerWasClosedBeforeAfterIteration = coldRunner.isActive === false; + return status; }); + await graph.executeAsync({}); + expect(coldRunner.closeAsync).toHaveBeenCalledTimes(1); expect(coldRunner.isActive).toBe(false); + expect(coldRunnerWasClosedBeforeAfterIteration).toBe(true); expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(persistentRunner.isActive).toBe(true); }); diff --git a/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts b/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts index f72bf682de4..e78552903d7 100644 --- a/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts +++ b/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts @@ -42,7 +42,8 @@ export class OperationGraphHooks { /** * Hook invoked to decide what work a potential new iteration contains. * Use the `lastExecutedRecords` to determine which operations are new or have had their inputs changed. - * Set the `enabled` states on the values in `initialRecords` to control which operations will be executed. + * Set `enabled` and `shouldRunnerPersist` on the values in `initialRecords` to control which operations + * execute and which runners remain active after the iteration. * * @remarks * This hook is synchronous to guarantee that the `lastExecutedRecords` map remains stable for the From e20c69f805b901474d0ee1b0b49a4d80f916b38e Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:55:17 +0000 Subject: [PATCH 5/6] Handle runner cleanup failures during iteration Convert fallback close failures into operation and iteration failures so final status and post-iteration hooks still run. Cover the bypassed-runner failure path with an ordering regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 --- .../src/logic/operations/OperationGraph.ts | 26 +++++++-- .../operations/test/OperationGraph.test.ts | 55 +++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index bca1e0e9f14..5d5201b315c 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -875,19 +875,33 @@ export class OperationGraph implements IOperationGraph { }); } - const operationsToClose: Operation[] = []; - for (const [operation, record] of executionRecords) { + const recordsToClose: OperationExecutionRecord[] = []; + for (const record of executionRecords.values()) { if (!recordsWithRunnerCleanup.has(record) && !record.shouldRunnerPersist) { - operationsToClose.push(operation); + recordsToClose.push(record); } } - if (operationsToClose.length > 0) { - await this.closeRunnersAsync(operationsToClose); + function reportRunnerCleanupFailure(record: OperationExecutionRecord, error: Error): void { + record.error = error; + record.status = OperationStatus.Failure; + _reportOperationErrorIfAny(record); + state.hasAnyFailures = true; } + await Async.forEachAsync( + recordsToClose, + async (record: OperationExecutionRecord) => { + try { + await this.closeRunnersAsync([record.operation]); + } catch (e) { + reportRunnerCleanupFailure(record, e); + } + }, + { concurrency: this.parallelism } + ); const status: OperationStatus = (() => { - if (bailStatus) return bailStatus; if (state.hasAnyFailures) return OperationStatus.Failure; + if (bailStatus) return bailStatus; if (state.hasAnyAborted) return OperationStatus.Aborted; if (state.hasAnyNonAllowedWarnings) return OperationStatus.SuccessWithWarning; if (iterationContext.totalOperations === 0) return OperationStatus.NoOp; diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index b9b195f53e6..98f6c5f2d81 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -1317,6 +1317,61 @@ describe('runner persistence policy', () => { expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); expect(persistentRunner.isActive).toBe(true); }); + + it('reports a bypassed runner cleanup failure before finalizing the iteration', async () => { + const persistentRunner: ClosableRunner = new ClosableRunner('persistent'); + const coldRunner: ClosableRunner = new ClosableRunner('cold'); + const closeError: Error = new Error('close failed'); + const lifecycleEvents: string[] = []; + + const persistentOperation: Operation = new Operation({ + runner: persistentRunner, + logFilenameIdentifier: 'persistent', + phase: mockPhase, + project: getOrCreateProject('persistent') + }); + const coldOperation: Operation = new Operation({ + runner: coldRunner, + logFilenameIdentifier: 'cold', + phase: mockPhase, + project: getOrCreateProject('cold') + }); + + const graph: OperationGraph = new OperationGraph( + new Set([persistentOperation, coldOperation]), + graphOptions + ); + + await graph.executeAsync({}); + + configureRunnerPersistence(graph, (operation) => operation === persistentOperation); + coldRunner.closeAsync.mockImplementationOnce(async () => { + lifecycleEvents.push('close'); + throw closeError; + }); + graph.hooks.beforeExecuteIterationAsync.tapPromise( + 'test', + async (): Promise => OperationStatus.FromCache + ); + let afterIterationStatus: OperationStatus | undefined; + graph.hooks.afterExecuteIterationAsync.tapPromise('test', async (status) => { + lifecycleEvents.push('after-iteration'); + afterIterationStatus = status; + return status; + }); + + const result: IExecutionResult = await graph.executeAsync({}); + const coldResult: IOperationExecutionResult | undefined = + result.operationResults.get(coldOperation); + + expect(lifecycleEvents).toEqual(['close', 'after-iteration']); + expect(afterIterationStatus).toBe(OperationStatus.Failure); + expect(result.status).toBe(OperationStatus.Failure); + expect(graph.status).toBe(OperationStatus.Failure); + expect(coldResult?.status).toBe(OperationStatus.Failure); + expect(coldResult?.error).toBe(closeError); + expect(persistentRunner.closeAsync).not.toHaveBeenCalled(); + }); }); describe('closeRunnersAsync', () => { From bd9cd75ad9ba653b351aebc5e3a204504bf56d58 Mon Sep 17 00:00:00 2001 From: Bharat Middha <5100938+bmiddha@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:45:50 +0000 Subject: [PATCH 6/6] Finalize unexecuted operation output Close terminal records' output resources before iteration summary hooks. This lets runner cleanup failures on bypassed records be summarized without an open StdioSummarizer exception. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f21c8c8-452a-4922-aca3-fb40b007f948 Assistant-model: GPT-5.6 Sol --- .../operations/OperationExecutionRecord.ts | 17 ++++++++++++----- .../src/logic/operations/OperationGraph.ts | 3 +++ .../operations/test/OperationGraph.test.ts | 4 ++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts index 424730af49e..ffaa73d8026 100644 --- a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts +++ b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts @@ -453,11 +453,18 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera // Delegate global state reporting await executeContext.onResultAsync(this); } finally { - if (this.isTerminal) { - this._collatedWriter?.close(); - this.stdioSummarizer.close(); - this.problemCollector.close(); - } + this.finalize(); + } + } + + /** + * Closes per-record output resources after the record reaches a terminal state. + */ + public finalize(): void { + if (this.isTerminal) { + this._collatedWriter?.close(); + this.stdioSummarizer.close(); + this.problemCollector.close(); } } } diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 5d5201b315c..e56c1133189 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -898,6 +898,9 @@ export class OperationGraph implements IOperationGraph { }, { concurrency: this.parallelism } ); + for (const record of executionRecords.values()) { + record.finalize(); + } const status: OperationStatus = (() => { if (state.hasAnyFailures) return OperationStatus.Failure; diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts index 98f6c5f2d81..7b21ab91286 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraph.test.ts @@ -1359,6 +1359,10 @@ describe('runner persistence policy', () => { afterIterationStatus = status; return status; }); + graph.hooks.afterExecuteIterationAsync.tap('test-summary', (status, records) => { + _printOperationStatus(mockTerminal, { status, operationResults: records }); + return status; + }); const result: IExecutionResult = await graph.executeAsync({}); const coldResult: IOperationExecutionResult | undefined =