diff --git a/common/changes/@microsoft/rush/external-execution-contract_2026-11-12.json b/common/changes/@microsoft/rush/external-execution-contract_2026-11-12.json new file mode 100644 index 00000000000..68bf0555822 --- /dev/null +++ b/common/changes/@microsoft/rush/external-execution-contract_2026-11-12.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add pure execution planning and export configured commands, explicit environment overrides and per-operation metadata through rush-buildxl-graph-plugin.", + "type": "minor" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/common/changes/@rushstack/rush-daemon/external-execution-contract_2026-11-12.json b/common/changes/@rushstack/rush-daemon/external-execution-contract_2026-11-12.json new file mode 100644 index 00000000000..f522da65f32 --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/external-execution-contract_2026-11-12.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "Update the operation graph test double for external execution planning.", + "type": "none" + } + ], + "packageName": "@rushstack/rush-daemon" +} diff --git a/common/reviews/api/rush-buildxl-graph-plugin.api.md b/common/reviews/api/rush-buildxl-graph-plugin.api.md index dbec0c82af6..7c7fbe2d7c7 100644 --- a/common/reviews/api/rush-buildxl-graph-plugin.api.md +++ b/common/reviews/api/rush-buildxl-graph-plugin.api.md @@ -25,6 +25,7 @@ export interface IBuildXLRushGraph { // (undocumented) repoSettings: { commonTempFolder: string; + executionContractVersion?: 1; }; } @@ -38,9 +39,12 @@ export interface IGraphNode { cacheable?: false; command: string; dependencies: string[]; + environmentVariables?: Record; id: string; + outputFolderNames?: string[]; package: string; task: string; + warningsAreAllowed?: true; workingDirectory: string; } diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 914ae2604f6..8e5452418c4 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -404,6 +404,7 @@ export function getWorkspaceRequestOperationEnvironment(hostEnvironment: Readonl // @alpha (undocumented) export interface IBaseOperationExecutionResult { + readonly environmentOverrides?: Readonly>; getStateHash(): string; getStateHashComponents(): IOperationStateHashComponents; readonly metadataFolderPath: string; @@ -475,6 +476,7 @@ export interface ICobuildLockProvider { // @alpha export interface IConfigurableOperation extends IBaseOperationExecutionResult { enabled: boolean; + environmentOverrides?: Record; shouldRunnerPersist: boolean; } @@ -771,6 +773,7 @@ export interface IOperationGraph { addTerminalDestination(destination: TerminalWritable): void; allowOversubscription: boolean; closeRunnersAsync(operations?: Iterable): Promise; + createExecutionPlanAsync(options: IOperationGraphIterationOptions): Promise>; debugMode: boolean; deleteResults?(operations: Iterable): void; discardScheduledIteration(): boolean; @@ -891,6 +894,7 @@ export interface IOperationRunner { cacheable: boolean; closeAsync?(): Promise; executeAsync(context: IOperationRunnerContext, lastState?: IOperationLastState): Promise; + getCommandForExecution?(): string; getConfigHash(): string; readonly isActive?: boolean; readonly isNoOp?: boolean; @@ -1679,6 +1683,10 @@ export class PhasedCommandEngineUsageError extends Error { // @alpha export class PhasedCommandHooks { + readonly beforeExecuteGraphAsync: AsyncSeriesBailHook<[ + IOperationGraph, + IOperationGraphContext + ], true | undefined | void>; readonly createOperationsAsync: AsyncSeriesWaterfallHook<[ Set, ICreateOperationsContext diff --git a/libraries/rush-daemon/src/test/WorkspaceEngineComponentFactory.test.ts b/libraries/rush-daemon/src/test/WorkspaceEngineComponentFactory.test.ts index c8355957538..785ea3d1f4e 100644 --- a/libraries/rush-daemon/src/test/WorkspaceEngineComponentFactory.test.ts +++ b/libraries/rush-daemon/src/test/WorkspaceEngineComponentFactory.test.ts @@ -86,6 +86,10 @@ class TestOperationGraph implements IOperationGraph { return Promise.resolve(); } + public async createExecutionPlanAsync(): Promise> { + throw new Error('External execution is not used by this test graph.'); + } + public executeScheduledIterationAsync(): Promise { return Promise.resolve(false); } diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index 69b030f04f6..515bfb1e405 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -712,6 +712,9 @@ export class PhasedScriptAction extends BaseScriptAction i public async createEngineAsync(abortSignal?: AbortSignal): Promise { this.validateEngineCommand(); await this.initializePluginsAsync(); + if (this.hooks.beforeExecuteGraphAsync.isUsed()) { + throw new Error('External execution hooks require --no-daemon.'); + } let engine: IPhasedCommandEngine | undefined; await this.#runAsync((result) => { engine = result; @@ -1175,6 +1178,9 @@ export class PhasedScriptAction extends BaseScriptAction i await hooks.onGraphCreatedAsync.promise(graph, graphContext); }); if (onEngine) { + if (hooks.beforeExecuteGraphAsync.isUsed()) { + throw new Error('External execution hooks require --no-daemon.'); + } // Before the graph is transferred, so that it is disposed below. abortSignal?.throwIfAborted(); if (!getGraphInputsSnapshotAsync || !initialSnapshot) { @@ -1202,6 +1208,23 @@ export class PhasedScriptAction extends BaseScriptAction i return; } + let executeLocally: boolean = false; + try { + executeLocally = !(await hooks.beforeExecuteGraphAsync.promise(graph, graphContext)); + } finally { + if (!executeLocally) { + await graph.closeRunnersAsync(); + } + } + if (!executeLocally) { + this.sessionAbortController.signal.throwIfAborted(); + stopwatch.stop(); + if (!this.#ignoreHooksParameter.value) { + this.#doAfterTask(); + } + return; + } + const abortPromise: Promise = once(this.sessionAbortController.signal, 'abort').then(async () => { terminal.writeLine(`Shutting down Rush...`); return await graph.abortCurrentIterationAsync(); diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts index db15266114b..ea895f4cbb9 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -2,17 +2,24 @@ // See LICENSE in the project root for license information. jest.mock(`@rushstack/package-deps-hash`, () => { + function getDetailedRepoStateAsync(): IDetailedRepoState { + return { + hasSubmodules: false, + hasUncommittedChanges: false, + files: new Map([['common/config/rush/npm-shrinkwrap.json', 'hash']]), + symlinks: new Map() + }; + } + return { getRepoRoot(dir: string): string { return dir; }, - getDetailedRepoStateAsync(): IDetailedRepoState { - return { - hasSubmodules: false, - hasUncommittedChanges: false, - files: new Map([['common/config/rush/npm-shrinkwrap.json', 'hash']]), - symlinks: new Map() - }; + getDetailedRepoStateAsync, + RepoStateCache: class MockRepoStateCache { + public async getDetailedRepoStateAsync(): Promise { + return getDetailedRepoStateAsync(); + } }, getRepoChangesAsync(): ReadonlyMap { return new Map(); @@ -40,6 +47,7 @@ import { OperationStatus } from '../../logic/operations/OperationStatus'; import type { IRepositoryLockWait } from '../../logic/RepositoryLockWait'; import type { ITelemetryData } from '../../logic/Telemetry'; import type { RushCommandLineParser as RushCommandLineParserType } from '../RushCommandLineParser'; +import type { PhasedScriptAction } from '../scriptActions/PhasedScriptAction'; import { getCommandLineParserInstanceAsync, setSpawnMock, @@ -50,7 +58,7 @@ import { type IEnvironmentConfigIsolation } from './TestUtils'; import { IS_WINDOWS } from '../../utilities/executionUtilities'; -import { AnsiEscape, StringBufferTerminalProvider } from '@rushstack/terminal'; +import { AnsiEscape, StringBufferTerminalProvider, TerminalProviderSeverity } from '@rushstack/terminal'; // Ordinals into the `mock.calls` array referencing each of the arguments to `spawn`. Note that // the exact structure of these arguments differs between Windows and non-Windows platforms, so @@ -844,6 +852,80 @@ describe('RushCommandLineParser', () => { }); }); + describe('external graph execution', () => { + it.each([true, undefined])( + 'runs after graph-created taps and handles external execution: %s', + async (handled) => { + const { parser, spawnMock } = await getCommandLineParserInstanceAsync( + 'basicAndRunBuildActionRepo', + 'build' + ); + const calls: string[] = []; + const close = jest.fn(async () => {}); + parser.rushSession.hooks.runAnyPhasedCommand.tap('ExternalExecutionTest', (command) => { + command.hooks.beforeExecuteGraphAsync.tapPromise( + 'export', + async (graph, context): Promise => { + calls.push('export'); + if (handled) { + const plan = await graph.createExecutionPlanAsync({ + inputsSnapshot: context.initialSnapshot + }); + expect(plan.size).toBe(2); + expect( + [...plan.values()].every((record) => record.environmentOverrides?.PUBLIC_ID === 'id') + ).toBe(true); + } + return handled ? true : undefined; + } + ); + command.hooks.onGraphCreatedAsync.tap('configure', (graph) => { + calls.push('graph-created'); + graph.hooks.configureIteration.tap('identity', (records) => { + calls.push('configure'); + for (const record of records.values()) record.environmentOverrides = { PUBLIC_ID: 'id' }; + }); + for (const operation of graph.operations) operation.runner!.closeAsync = close; + }); + }); + await expect(parser.executeAsync()).resolves.toBe(true); + expect(calls.slice(0, 3)).toEqual(['graph-created', 'export', 'configure']); + expect(spawnMock).toHaveBeenCalledTimes(handled ? 0 : 2); + if (handled) expect(close).toHaveBeenCalledTimes(2); + } + ); + + it('refuses engine transfer rather than ignoring external execution hooks', async () => { + const { parser, spawnMock } = await getCommandLineParserInstanceAsync( + 'basicAndRunBuildActionRepo', + 'build' + ); + const exportGraph = jest.fn(); + parser.rushSession.hooks.runAnyPhasedCommand.tap('ExternalExecutionTest', (command) => { + command.hooks.beforeExecuteGraphAsync.tap('export', exportGraph); + }); + const action: PhasedScriptAction = parser.getAction('build') as PhasedScriptAction; + const terminalWriteSpy = jest.spyOn(parser.rushSession.terminalProvider, 'write'); + try { + await expect(action.createEngineAsync()).rejects.toThrow( + 'External execution hooks require --no-daemon.' + ); + expect(exportGraph).not.toHaveBeenCalled(); + expect(spawnMock).not.toHaveBeenCalled(); + expect(terminalWriteSpy).not.toHaveBeenCalledWith( + expect.any(String), + TerminalProviderSeverity.warning + ); + expect(terminalWriteSpy).not.toHaveBeenCalledWith( + expect.any(String), + TerminalProviderSeverity.error + ); + } finally { + terminalWriteSpy.mockRestore(); + } + }); + }); + describe('in repo plugin that produces no operations', () => { it('succeeds when a plugin returns an empty operation set', async () => { // Regression test: `@rushstack/rush-buildxl-graph-plugin` writes the build graph to disk in diff --git a/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts b/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts index 8400f371d59..2ffebbd4f98 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationExecutionResult.ts @@ -33,6 +33,13 @@ export interface IOperationStateHashComponents { * @alpha */ export interface IBaseOperationExecutionResult { + /** + * Explicit, non-secret environment overrides for local and external execution. + * These values may be serialized in graph exports; never include credentials. + * Plugins must account for these values in the operation's input/configuration identity. + */ + readonly environmentOverrides?: Readonly>; + /** * The operation itself */ @@ -63,6 +70,11 @@ export interface IBaseOperationExecutionResult { * @alpha */ export interface IConfigurableOperation extends IBaseOperationExecutionResult { + /** + * Explicit environment overrides, applied after local environment hooks. + */ + environmentOverrides?: Record; + /** * True if the operation should execute in this iteration, false otherwise. */ diff --git a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts index 85d73919c4a..61b1544e2e7 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationGraph.ts @@ -130,7 +130,7 @@ export interface IOperationGraphExtensionResult { readonly reason?: string; /** - * The operations whose `enabled` state, runner policy or state hash the extension changed. Empty if the iteration + * The operations whose `enabled` state, runner policy, environment overrides or state hash the extension changed. Empty if the iteration * was not extended. */ readonly changedOperations: ReadonlySet; @@ -286,6 +286,16 @@ export interface IOperationGraph { */ closeRunnersAsync(operations?: Iterable): Promise; + /** + * Creates configured records without scheduling operations or running execution, cache or environment hooks. + * Runs only `configureIteration`, using the supplied inputs (or the graph's snapshot provider). + * Intended for external executors and graph export. The graph must not have an active or scheduled iteration. + * Planning does not replace execution results or consume an iteration ID. + */ + createExecutionPlanAsync( + options: IOperationGraphIterationOptions + ): Promise>; + /** * Drops retained results after the host has awaited runner and watcher cleanup. * Rejects executing/prepared iterations and runners that still report active resources. diff --git a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts index b880671778f..2875c369608 100644 --- a/libraries/rush-lib/src/logic/operations/IOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/IOperationRunner.ts @@ -215,6 +215,13 @@ export interface IOperationRunner { */ getConfigHash(): string; + /** + * Returns the initial shell command for execution by an external executor. + * Unlike {@link IOperationRunner.getConfigHash}, this includes parameters excluded from cache identity. + * Runners that cannot be represented as a shell command should omit this method. + */ + getCommandForExecution?(): string; + /** * If this runner performs any background work to optimize future runs, this method will clean it up. * Must be safe to call again after the resources have been released. diff --git a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts index 3cf1622541d..ff7abb3508d 100644 --- a/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts +++ b/libraries/rush-lib/src/logic/operations/OperationExecutionRecord.ts @@ -15,7 +15,7 @@ import { Terminal, type TerminalWritable } from '@rushstack/terminal'; -import { InternalError, NewlineKind, FileError } from '@rushstack/node-core-library'; +import { EnvironmentMap, InternalError, NewlineKind, FileError } from '@rushstack/node-core-library'; import { CollatedTerminal, type CollatedWriter, type StreamCollator } from '@rushstack/stream-collator'; import { coerceParallelism } from './ParseParallelism'; @@ -127,6 +127,8 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera */ public enabled: boolean; + public environmentOverrides?: Record = {}; + /** * If true, this operation's runner should remain active after this iteration. */ @@ -274,7 +276,13 @@ export class OperationExecutionRecord implements IOperationRunnerContext, IOpera } public get environment(): IEnvironment | undefined { - return this.#context.createEnvironment?.(this); + const environment: IEnvironment | undefined = this.#context.createEnvironment?.(this); + if (this.environmentOverrides && Object.keys(this.environmentOverrides).length > 0) { + const merged: EnvironmentMap = new EnvironmentMap(environment ?? process.env); + merged.mergeFromObject(this.environmentOverrides); + return merged.toObject(); + } + return environment; } public get abortSignal(): AbortSignal | undefined { diff --git a/libraries/rush-lib/src/logic/operations/OperationGraph.ts b/libraries/rush-lib/src/logic/operations/OperationGraph.ts index 4d624a5aa31..5818c2979d3 100644 --- a/libraries/rush-lib/src/logic/operations/OperationGraph.ts +++ b/libraries/rush-lib/src/logic/operations/OperationGraph.ts @@ -217,6 +217,7 @@ class ConfigurableOperation implements IConfigurableOperation { public readonly operation: Operation; public readonly metadataFolderPath: string; public enabled: boolean; + public environmentOverrides?: Record = {}; public shouldRunnerPersist: boolean = true; readonly #stateHashEntry: IOperationStateHashCacheEntry; @@ -738,6 +739,19 @@ export class OperationGraph implements IOperationGraph { record.shouldRunnerPersist = configuration.shouldRunnerPersist; changedRecords.set(record.operation, record); } + for (const [operation, record] of records) { + if (isUndispatched(record)) { + const overrides: Record = plan.get(operation)!.environmentOverrides ?? {}; + const previous: Record = record.environmentOverrides ?? {}; + if ( + Object.keys(overrides).length !== Object.keys(previous).length || + Object.keys(overrides).some((key: string) => overrides[key] !== previous[key]) + ) { + record.environmentOverrides = { ...overrides }; + changedRecords.set(operation, record); + } + } + } const releasedRecords: OperationExecutionRecord[] = []; for (const record of collectNeededRecords(records, [...joiningRecords, ...enabledRecords])) { @@ -768,6 +782,46 @@ export class OperationGraph implements IOperationGraph { return { extended: true, changedOperations }; } + public async createExecutionPlanAsync( + iterationOptions: IOperationGraphIterationOptions + ): Promise> { + const checkIdle = (): void => { + this.abortController.signal.throwIfAborted(); + if (this.#currentIteration || this.hasScheduledIteration) { + throw new Error( + 'Cannot create an external execution plan while an iteration is active or scheduled.' + ); + } + }; + checkIdle(); + const { inputsSnapshot = await this.#getInputsSnapshotAsync?.(), startTime = performance.now() } = + iterationOptions; + checkIdle(); + const records: Map = new Map(); + try { + this.#configureRecords( + records, + { + iterationId: this.#nextIterationId, + streamCollator: undefined, + inputsSnapshot, + getOperationEnvironment: iterationOptions.getOperationEnvironment, + maxParallelism: this.#maxParallelism, + debugMode: this.debugMode, + quietMode: this.quietMode + }, + { ...iterationOptions, inputsSnapshot, startTime } + ); + this.abortController.signal.throwIfAborted(); + return records; + } finally { + for (const record of records.values()) { + record.stdioSummarizer.close(); + record.problemCollector.close(); + } + } + } + public deleteResults(operations: Iterable): void { if (this.#currentIteration || this.#scheduledIteration) { throw new Error('Cannot delete results of an executing or prepared graph.'); @@ -994,8 +1048,6 @@ export class OperationGraph implements IOperationGraph { onWriterActive }); - const sortedOperations: readonly Operation[] = this.#sortedOperations; - const graph: OperationGraph = this; function createEnvironmentForOperation(record: OperationExecutionRecord): IEnvironment { @@ -1042,38 +1094,7 @@ export class OperationGraph implements IOperationGraph { const executionRecords: Map = iterationContext.records; const { eventSink } = this; - for (const operation of sortedOperations) { - const executionRecord: OperationExecutionRecord = new OperationExecutionRecord( - operation, - iterationContext - ); - executionRecords.set(operation, executionRecord); - } - - for (const [operation, record] of executionRecords) { - for (const dependency of operation.dependencies) { - const dependencyRecord: OperationExecutionRecord | undefined = executionRecords.get(dependency); - if (!dependencyRecord) { - throw new Error( - `Operation "${record.name}" declares a dependency on operation "${dependency.name}" that is not in the set of operations to execute.` - ); - } - record.dependencies.add(dependencyRecord); - dependencyRecord.consumers.add(record); - } - } - - // Configure operations to execute. - // Ensure we compute the compute the state hashes for all operations before the runtime graph potentially mutates. - if (inputsSnapshot) { - for (const record of executionRecords.values()) { - record.getStateHash(); - } - } - - measureFn(`${PERF_PREFIX}:configureIteration`, () => { - hooks.configureIteration.call(executionRecords, this.resultByOperation, iterationOptionsForCallbacks); - }); + this.#configureRecords(executionRecords, iterationContext, iterationOptionsForCallbacks); for (const executionRecord of executionRecords.values()) { if (!executionRecord.silent) { @@ -1155,6 +1176,37 @@ export class OperationGraph implements IOperationGraph { } } + #configureRecords( + records: Map, + context: IOperationExecutionRecordContext, + iterationOptions: IOperationGraphIterationOptions + ): void { + for (const operation of this.#sortedOperations) { + records.set(operation, new OperationExecutionRecord(operation, context)); + } + for (const [operation, record] of records) { + for (const dependency of operation.dependencies) { + const dependencyRecord: OperationExecutionRecord | undefined = records.get(dependency); + if (!dependencyRecord) { + throw new Error( + `Operation "${record.name}" declares a dependency on operation "${dependency.name}" that is not in the set of operations to execute.` + ); + } + record.dependencies.add(dependencyRecord); + dependencyRecord.consumers.add(record); + } + } + // Hash inputs before configureIteration can change the runtime graph. + if (context.inputsSnapshot) { + for (const record of records.values()) { + record.getStateHash(); + } + } + measureFn(`${PERF_PREFIX}:configureIteration`, () => { + this.hooks.configureIteration.call(records, this.resultByOperation, iterationOptions); + }); + } + /** * Debounce configuration change notifications so that multiple property setters invoked within the same tick * only trigger the hook once. This avoids redundant re-computation in listeners (e.g. UI refresh) while preserving diff --git a/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts b/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts index 887a0e7c1c9..b8e11840225 100644 --- a/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts +++ b/libraries/rush-lib/src/logic/operations/ShellOperationRunner.ts @@ -167,6 +167,10 @@ export class ShellOperationRunner implements IOperationRunner { return this.#commandForHash; } + public getCommandForExecution(): string { + return this.#initialCommand; + } + async #invokeCommandAsync( context: IOperationRunnerContext, terminals: ICommandTerminals, diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraphExtension.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraphExtension.test.ts index 1af100454b2..696fda57d71 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraphExtension.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraphExtension.test.ts @@ -193,6 +193,48 @@ async function getStateHashesAsync( describe('OperationGraph iteration extension', () => { const snapshotB: IInputsSnapshot = createSnapshot({ a: 'a1', b: 'b1', c: 'c1', d: 'd1', e: 'e1', x: 'x1' }); + it('commits environment overrides only to undispatched records of an accepted extension', async () => { + const testGraph: TestGraph = new TestGraph(['a', 'b', 'c'], { + edges: [['b', 'a']], + enabled: ['a', 'b'] + }); + const { graph } = testGraph; + let identity: string = 'initial'; + graph.hooks.configureIteration.tap('identity', (records) => { + for (const record of records.values()) { + record.environmentOverrides = { PUBLIC_IDENTITY: identity }; + } + }); + const resultPromise: Promise = graph.executeAsync({ + inputsSnapshot: snapshotB, + holdUnneededOperations: true + }); + await testGraph.startedAsync('a'); + identity = 'extension'; + testGraph.operation('c').enabled = true; + expect(() => + graph.tryExtendCurrentIteration({ + inputsSnapshot: snapshotB, + neededOperations: [testGraph.operation('c')], + beforeCommit: () => { + throw new Error('Refused'); + } + }) + ).toThrow('Refused'); + expect(testGraph.record('b').environmentOverrides).toEqual({ PUBLIC_IDENTITY: 'initial' }); + const result: IOperationGraphExtensionResult = graph.tryExtendCurrentIteration({ + inputsSnapshot: snapshotB, + neededOperations: [testGraph.operation('c')] + }); + expect(result.extended).toBe(true); + expect(result.changedOperations).toEqual(new Set([testGraph.operation('b'), testGraph.operation('c')])); + expect(testGraph.record('a').environmentOverrides).toEqual({ PUBLIC_IDENTITY: 'initial' }); + expect(testGraph.record('b').environmentOverrides).toEqual({ PUBLIC_IDENTITY: 'extension' }); + expect(testGraph.record('c').environmentOverrides).toEqual({ PUBLIC_IDENTITY: 'extension' }); + testGraph.finishAll(); + expect((await resultPromise).status).toBe(OperationStatus.Success); + }); + it('holds the operations that no enabled operation needs until the others complete', async () => { const testGraph: TestGraph = new TestGraph(['a', 'b', 'c'], { edges: [['b', 'a']], enabled: ['a'] }); const resultPromise: Promise = testGraph.graph.executeAsync({ diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraphOperationEnvironment.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraphOperationEnvironment.test.ts index 1cd94685df4..8594ff8fe5b 100644 --- a/libraries/rush-lib/src/logic/operations/test/OperationGraphOperationEnvironment.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraphOperationEnvironment.test.ts @@ -151,6 +151,7 @@ describe('OperationGraph operation environment', () => { inputsSnapshot, getOperationEnvironment: (operation: Operation) => (operation.runner === first ? sessionA : {}) }); + expect(hashedEnvironments).toEqual( new Map([ ['first', sessionA], @@ -168,6 +169,45 @@ describe('OperationGraph operation environment', () => { ); }); + it('applies explicit overrides after request environments and local taps, and resets them each iteration', async () => { + const runner: EnvironmentRecordingRunner = new EnvironmentRecordingRunner('operation'); + const graph: OperationGraph = createGraph([runner]); + const requestEnvironment: Record = { [SESSION_VARIABLE]: 'request' }; + let override: string | undefined = 'explicit'; + graph.hooks.configureIteration.tap('identity', (records) => { + for (const record of records.values()) { + if (override) record.environmentOverrides = { [SESSION_VARIABLE]: override }; + } + }); + graph.hooks.createEnvironmentForOperation.tap('local', (environment) => ({ + ...environment, + [SESSION_VARIABLE]: `${environment[SESSION_VARIABLE]}+local` + })); + await graph.executeAsync({ getOperationEnvironment: () => requestEnvironment }); + override = undefined; + await graph.executeAsync({ getOperationEnvironment: () => requestEnvironment }); + expect(runner.sessions).toEqual(['explicit', 'request+local']); + expect(requestEnvironment).toEqual({ [SESSION_VARIABLE]: 'request' }); + }); + + it('uses platform environment name casing when applying overrides', async () => { + const runner: EnvironmentRecordingRunner = new EnvironmentRecordingRunner('operation'); + const execute = jest.spyOn(runner, 'executeAsync'); + const graph: OperationGraph = createGraph([runner]); + const lowerCaseName: string = SESSION_VARIABLE.toLowerCase(); + graph.hooks.configureIteration.tap('identity', (records) => { + for (const record of records.values()) { + record.environmentOverrides = { [SESSION_VARIABLE]: 'explicit' }; + } + }); + await graph.executeAsync({ getOperationEnvironment: () => ({ [lowerCaseName]: 'request' }) }); + expect(execute.mock.calls[0][0].environment).toEqual( + process.platform === 'win32' + ? { [SESSION_VARIABLE]: 'explicit' } + : { [lowerCaseName]: 'request', [SESSION_VARIABLE]: 'explicit' } + ); + }); + it('gives every iteration hook the environment lookup that the iteration was scheduled with', async () => { const graph: OperationGraph = createGraph([new EnvironmentRecordingRunner('operation')]); type GetOperationEnvironment = IOperationGraphIterationOptions['getOperationEnvironment']; diff --git a/libraries/rush-lib/src/logic/operations/test/OperationGraphPlanning.test.ts b/libraries/rush-lib/src/logic/operations/test/OperationGraphPlanning.test.ts new file mode 100644 index 00000000000..16f661d5244 --- /dev/null +++ b/libraries/rush-lib/src/logic/operations/test/OperationGraphPlanning.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +jest.mock('../OperationStateFile'); + +import { MockWritable } from '@rushstack/terminal'; + +import type { IPhase } from '../../../api/CommandLineConfiguration'; +import type { RushConfigurationProject } from '../../../api/RushConfigurationProject'; +import { Operation } from '../Operation'; +import { OperationGraph, type IOperationGraphOptions } from '../OperationGraph'; +import { OperationStatus } from '../OperationStatus'; +import { MockOperationRunner } from './MockOperationRunner'; + +const phase: IPhase = { + name: 'phase', + allowWarningsOnSuccess: false, + associatedParameters: new Set(), + dependencies: { self: new Set(), upstream: new Set() }, + isSynthetic: false, + logFilenameIdentifier: 'phase', + missingScriptBehavior: 'silent' +}; + +function createGraph(options: IOperationGraphOptions, runner: MockOperationRunner): OperationGraph { + const operation: Operation = new Operation({ + runner, + logFilenameIdentifier: 'operation', + phase, + project: { packageName: 'project' } as RushConfigurationProject + }); + return new OperationGraph(new Set([operation]), options); +} + +describe('OperationGraph external execution planning', () => { + let graphOptions: IOperationGraphOptions; + let writable: MockWritable; + + beforeEach(() => { + writable = new MockWritable(); + graphOptions = { + quietMode: true, + debugMode: false, + parallelism: 1, + allowOversubscription: false, + destinations: [writable], + abortController: new AbortController() + }; + }); + + it('plans disabled operations without scheduling, executing hooks or exporting the ambient environment', async () => { + const run = jest.fn(async () => OperationStatus.Success); + const graph: OperationGraph = createGraph(graphOptions, new MockOperationRunner('planned', run)); + const environmentHook = jest.fn((environment) => environment); + const beforeExecution = jest.fn(); + const beforeOperation = jest.fn(); + const afterExecution = jest.fn((status) => status); + const scheduled = jest.fn(); + const event = jest.fn(); + graph.eventSink = { onOperationRegistered: event, onOperationStatusChanged: event }; + graph.hooks.createEnvironmentForOperation.tap('local-only', environmentHook); + graph.hooks.beforeExecuteIterationAsync.tap('must-not-run', beforeExecution); + graph.hooks.beforeExecuteOperationAsync.tap('must-not-run', beforeOperation); + graph.hooks.afterExecuteIterationAsync.tap('must-not-run', afterExecution); + graph.hooks.onIterationScheduled.tap('must-not-schedule', scheduled); + graph.hooks.configureIteration.tap('configure-export', (records) => { + for (const record of records.values()) { + record.enabled = false; + record.environmentOverrides = { EXPORTED_IDENTITY: 'generation' }; + } + }); + + const plan = await graph.createExecutionPlanAsync({}); + expect(plan.size).toBe(1); + expect([...plan.values()][0]).toMatchObject({ + enabled: false, + environmentOverrides: { EXPORTED_IDENTITY: 'generation' } + }); + expect(graph.hasScheduledIteration).toBe(false); + expect(graph.resultByOperation.size).toBe(0); + expect(run).not.toHaveBeenCalled(); + expect(environmentHook).not.toHaveBeenCalled(); + expect(beforeExecution).not.toHaveBeenCalled(); + expect(beforeOperation).not.toHaveBeenCalled(); + expect(afterExecution).not.toHaveBeenCalled(); + expect(event).not.toHaveBeenCalled(); + expect(scheduled).not.toHaveBeenCalled(); + expect(writable.getAllOutput()).toBe(''); + }); + + it('applies explicit overrides locally without reusing planned records or consuming iteration IDs', async () => { + const runner: MockOperationRunner = new MockOperationRunner('planned'); + const execute = jest.spyOn(runner, 'executeAsync'); + const graph: OperationGraph = createGraph(graphOptions, runner); + let generation: string = 'first'; + graph.hooks.configureIteration.tap('identity', (records) => { + for (const record of records.values()) record.environmentOverrides = { EXPORTED_IDENTITY: generation }; + }); + graph.hooks.createEnvironmentForOperation.tap('local-only', (environment) => ({ + ...environment, + EXPORTED_IDENTITY: 'must-not-override', + LOCAL_ONLY: 'not-exported' + })); + + const firstPlan = await graph.createExecutionPlanAsync({}); + const secondPlan = await graph.createExecutionPlanAsync({}); + expect([...secondPlan.values()][0].iterationId).toBe([...firstPlan.values()][0].iterationId); + generation = 'second'; + await graph.executeAsync({}); + expect(execute).toHaveBeenCalledTimes(1); + expect(execute.mock.calls[0][0].environment).toMatchObject({ + EXPORTED_IDENTITY: 'second', + LOCAL_ONLY: 'not-exported' + }); + expect([...graph.resultByOperation.values()][0].iterationId).toBe([...firstPlan.values()][0].iterationId); + expect([...firstPlan.values()][0].environmentOverrides).toEqual({ EXPORTED_IDENTITY: 'first' }); + }); + + it('returns an empty plan without scheduling an empty iteration', async () => { + const graph: OperationGraph = new OperationGraph(new Set(), graphOptions); + await expect(graph.createExecutionPlanAsync({})).resolves.toHaveProperty('size', 0); + expect(graph.hasScheduledIteration).toBe(false); + }); + + it('rejects planning while an iteration is scheduled or active, or after abort', async () => { + const runner: MockOperationRunner = new MockOperationRunner('planned'); + const graph: OperationGraph = createGraph({ ...graphOptions, pauseNextIteration: true }, runner); + await graph.scheduleIterationAsync({}); + await expect(graph.createExecutionPlanAsync({})).rejects.toThrow(/active or scheduled/); + let activeCheck: Promise | undefined; + graph.hooks.beforeExecuteIterationAsync.tap('check-active', () => { + activeCheck = expect(graph.createExecutionPlanAsync({})).rejects.toThrow(/active or scheduled/); + }); + await graph.executeScheduledIterationAsync(); + await activeCheck; + graph.abortController.abort(); + await expect(graph.createExecutionPlanAsync({})).rejects.toThrow(); + }); + + it('propagates configuration failure without scheduling or replacing previous results', async () => { + const graph: OperationGraph = createGraph(graphOptions, new MockOperationRunner('planned')); + await graph.executeAsync({}); + const previousResults = new Map(graph.resultByOperation); + graph.hooks.configureIteration.tap('fail', () => { + throw new Error('Invalid external configuration'); + }); + await expect(graph.createExecutionPlanAsync({})).rejects.toThrow('Invalid external configuration'); + expect(graph.resultByOperation).toEqual(previousResults); + expect(graph.hasScheduledIteration).toBe(false); + }); +}); diff --git a/libraries/rush-lib/src/logic/operations/test/ShellOperationRunner.test.ts b/libraries/rush-lib/src/logic/operations/test/ShellOperationRunner.test.ts index 837877c8899..6ba76f9bf9d 100644 --- a/libraries/rush-lib/src/logic/operations/test/ShellOperationRunner.test.ts +++ b/libraries/rush-lib/src/logic/operations/test/ShellOperationRunner.test.ts @@ -22,6 +22,20 @@ import { OperationStatus } from '../OperationStatus'; import { ShellOperationRunner, convertSlashesForWindows, isHeftCommand } from '../ShellOperationRunner'; describe(convertSlashesForWindows.name, () => { + it('exports the executable command rather than a cache configuration hash', () => { + const runner: ShellOperationRunner = new ShellOperationRunner({ + phase: { allowWarningsOnSuccess: false } as IPhase, + rushProject: { projectFolder: process.cwd() } as RushConfigurationProject, + displayName: 'exported command', + initialCommand: 'node build.js --verbose', + incrementalCommand: 'node build.js --incremental', + commandForHash: 'opaque-config-hash', + ignoredParameterValues: [] + }); + expect(runner.getCommandForExecution()).toBe('node build.js --verbose'); + expect(runner.getConfigHash()).toBe('opaque-config-hash'); + }); + it('converted inputs', () => { expect(convertSlashesForWindows('./node_modules/.bin/tslint -c config/tslint.json')).toEqual( '.\\node_modules\\.bin\\tslint -c config/tslint.json' diff --git a/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts b/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts index e282b924195..90802bd8cbd 100644 --- a/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts +++ b/libraries/rush-lib/src/pluginFramework/OperationGraphHooks.ts @@ -50,8 +50,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 `enabled` and `shouldRunnerPersist` on the values in `initialRecords` to control which operations - * execute and which runners remain active after their operation completes. + * Set `enabled`, `shouldRunnerPersist` and `environmentOverrides` on the values in `initialRecords` to control + * execution. Explicit non-secret environment overrides are also available to external execution plans. * * @remarks * This hook is synchronous to guarantee that the `lastExecutedRecords` map remains stable for the @@ -65,6 +65,7 @@ export class OperationGraphHooks { * work of a request that joins it (see {@link IOperationGraph.tryExtendCurrentIteration}). The graph can still * refuse that plan after this hook returns, and then discards it. A plugin that keeps state across iterations * applies the effects of such a plan in `extendIteration`, which is called only for an accepted plan. + * External execution planning also calls this hook without scheduling or executing an iteration. */ public readonly configureIteration: SyncHook< [ @@ -77,7 +78,7 @@ export class OperationGraphHooks { /** * Hook invoked when the work of another request is added to an iteration that is already executing (see * {@link IOperationGraph.tryExtendCurrentIteration}), before any of the given records is dispatched. It receives the - * records whose `enabled` state, runner policy or state hash changed, and the options of the extension, whose + * records whose `enabled` state, runner policy, environment overrides or state hash changed, and the options of the extension, whose * `inputsSnapshot` is the newer snapshot that the state hashes of these records were calculated from. A plugin that * prepares state for each operation in `beforeExecuteIterationAsync` prepares it again here for these records. * diff --git a/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts b/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts index d3a7fc15074..c434bdc9daf 100644 --- a/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts +++ b/libraries/rush-lib/src/pluginFramework/PhasedCommandHooks.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import { AsyncSeriesHook, AsyncSeriesWaterfallHook } from 'tapable'; +import { AsyncSeriesBailHook, AsyncSeriesHook, AsyncSeriesWaterfallHook } from 'tapable'; import type { CommandLineParameter } from '@rushstack/ts-command-line'; @@ -111,6 +111,7 @@ export interface IOperationGraphContext extends ICreateOperationsContext { * 2. `onGraphCreatedAsync` - Invoked after the operation graph is created, allowing plugins to * tap into graph-level hooks (e.g. `configureIteration`, `onIdle`). * See {@link OperationGraphHooks} for the per-iteration lifecycle. + * 3. `beforeExecuteGraphAsync` - Allows an external executor to handle the fully configured graph. * * @alpha */ @@ -127,4 +128,16 @@ export class PhasedCommandHooks { */ public readonly onGraphCreatedAsync: AsyncSeriesHook<[IOperationGraph, IOperationGraphContext]> = new AsyncSeriesHook(['operationGraph', 'context'], 'onGraphCreatedAsync'); + + /** + * Invoked after all onGraphCreatedAsync taps, before any iteration is scheduled. + * Return true after handling the graph externally to suppress local execution. + * Return undefined to allow the normal execution path. + * Commands that tap this hook require native execution (`--no-daemon`); an engine cannot transfer its graph + * to an external executor. Watch support is the responsibility of the tap. + */ + public readonly beforeExecuteGraphAsync: AsyncSeriesBailHook< + [IOperationGraph, IOperationGraphContext], + true | undefined | void + > = new AsyncSeriesBailHook(['operationGraph', 'context'], 'beforeExecuteGraphAsync'); } diff --git a/rush-plugins/rush-buildxl-graph-plugin/README.md b/rush-plugins/rush-buildxl-graph-plugin/README.md index da426481736..8a04be7e64f 100644 --- a/rush-plugins/rush-buildxl-graph-plugin/README.md +++ b/rush-plugins/rush-buildxl-graph-plugin/README.md @@ -2,6 +2,25 @@ Rush plugin for generating a BuildXL graph. +Operation IDs are opaque: sharded phases have multiple operation IDs. Consumers must preserve the exported +commands and dependency IDs rather than reconstructing them from package and phase names. + +Execution contract version `1` (`repoSettings.executionContractVersion`) exports actual initial commands, +explicit non-secret `environmentVariables`, declared `outputFolderNames`, runner cache exclusions and +`warningsAreAllowed`. Apply the environment overrides, honor `cacheable: false` for reads and writes, and +allow diagnostic stderr only on warning-tolerant nodes. Never reconstruct commands from configuration hashes. + +Export runs after all graph-created hooks and configures an execution plan without running operations, +cache hooks or local environment hooks. Disabled/no-op nodes are pruned while preserving dependency edges. +Only explicitly declared environment overrides are serialized, never the ambient environment. Custom runners +must expose `getCommandForExecution()`; watch and daemon execution are unsupported (use `--no-daemon`). +External consumers must adopt this +contract; exporting a graph alone does not upgrade an existing BuildXL/QTest integration. +Associate `--drop-graph` with the phased command, not its phases, so the export-only flag is not forwarded +to child commands. Planning and execution must use the same declared runtime/toolchain and semantic inputs. +Plugins must include semantic overrides in input/configuration identity; exporting an override does not hash it. +This generic contract does not implement Heft shard identity, manifests, collation, coverage or JUnit reporting. + ## Links - [CHANGELOG.md]( diff --git a/rush-plugins/rush-buildxl-graph-plugin/src/DropBuildGraphPlugin.ts b/rush-plugins/rush-buildxl-graph-plugin/src/DropBuildGraphPlugin.ts index 8266b17f43f..faf875e256b 100644 --- a/rush-plugins/rush-buildxl-graph-plugin/src/DropBuildGraphPlugin.ts +++ b/rush-plugins/rush-buildxl-graph-plugin/src/DropBuildGraphPlugin.ts @@ -3,10 +3,12 @@ import { RushConstants, - type ICreateOperationsContext, + type IOperationGraph, + type IOperationGraphContext, + type IOperationExecutionResult, + type Operation, type IPhasedCommand, type IRushPlugin, - type Operation, type RushConfiguration, type RushSession } from '@rushstack/rush-sdk'; @@ -24,6 +26,11 @@ export interface IBuildXLRushGraph { nodes: IGraphNode[]; repoSettings: { commonTempFolder: string; + /** + * Version of the command, environment, output and cache metadata contract. + * Consumers must honor these fields when executing exported operations. + */ + executionContractVersion?: 1; }; } @@ -52,11 +59,11 @@ export class DropBuildGraphPlugin implements IRushPlugin { } public apply(session: RushSession, rushConfiguration: RushConfiguration): void { - async function handleCreateOperationsForCommandAsync( + async function handleExternalGraphAsync( commandName: string, - operations: Set, - context: ICreateOperationsContext - ): Promise> { + graph: IOperationGraph, + context: IOperationGraphContext + ): Promise { // TODO: Introduce an API to allow plugins to register command line options for arbitrary, existing commands // in a repo const dropGraphParameter: CommandLineStringParameter | undefined = context.customParameters.get( @@ -75,9 +82,17 @@ export class DropBuildGraphPlugin implements IRushPlugin { const dropGraphPath: string | undefined = dropGraphParameter?.value; if (dropGraphPath) { + if (context.isWatch) { + throw new Error('Graph export does not support watch mode.'); + } + const executionRecords: ReadonlyMap = + await graph.createExecutionPlanAsync({ + inputsSnapshot: context.initialSnapshot + }); const { dropGraphAsync } = await import('./dropGraph'); const isValid: boolean = await dropGraphAsync({ - operations, + operations: new Set(graph.operations), + executionRecords, context, dropGraphPath, rushConfiguration, @@ -87,23 +102,25 @@ export class DropBuildGraphPlugin implements IRushPlugin { if (!isValid) { throw new Error('Failed to validate the graph'); } else { - // If the --drop-graph flag is present, we want to exit the process after dropping the graph - return new Set(); + return true; } } else { - return operations; + return undefined; } } for (const buildXLCommandName of this.#buildXLCommandNames) { session.hooks.runPhasedCommand.for(buildXLCommandName).tap(PLUGIN_NAME, (command: IPhasedCommand) => { - command.hooks.createOperationsAsync.tapPromise( + if (!command.hooks.beforeExecuteGraphAsync) { + throw new Error(`${PLUGIN_NAME} requires Rush with external execution planning support.`); + } + command.hooks.beforeExecuteGraphAsync.tapPromise( { name: PLUGIN_NAME, - stage: Number.MAX_SAFE_INTEGER // Run this after other plugins have created all operations + stage: Number.MAX_SAFE_INTEGER }, - async (operations: Set, context: ICreateOperationsContext) => - await handleCreateOperationsForCommandAsync(command.actionName, operations, context) + async (graph: IOperationGraph, context: IOperationGraphContext) => + await handleExternalGraphAsync(command.actionName, graph, context) ); }); } diff --git a/rush-plugins/rush-buildxl-graph-plugin/src/GraphProcessor.ts b/rush-plugins/rush-buildxl-graph-plugin/src/GraphProcessor.ts index ff780f552f8..57f347559e8 100644 --- a/rush-plugins/rush-buildxl-graph-plugin/src/GraphProcessor.ts +++ b/rush-plugins/rush-buildxl-graph-plugin/src/GraphProcessor.ts @@ -70,6 +70,22 @@ export interface IGraphNode { * If false, the Pip is uncacheable */ cacheable?: false; + + /** + * Explicit non-secret environment values to apply to this operation. + * Ambient environment variables and credentials are not exported. + */ + environmentVariables?: Record; + + /** + * Declared output folders, relative to workingDirectory. Cache entries must not capture other operations' outputs. + */ + outputFolderNames?: string[]; + + /** + * If true, stderr alone must not fail the operation. Nonzero exit codes still fail. + */ + warningsAreAllowed?: true; } interface IGraphNodeInternal extends Omit { @@ -79,6 +95,25 @@ interface IGraphNodeInternal extends Omit; +interface IGraphOperation { + associatedPhase: Pick; + associatedProject: Pick; + logFilenameIdentifier: string; + dependencies: Iterable; + runner: + | Pick< + NonNullable, + 'isNoOp' | 'getCommandForExecution' | 'cacheable' | 'warningsAreAllowed' + > + | undefined; + settings?: Pick, 'disableBuildCacheForOperation' | 'outputFolderNames'>; +} + +interface IGraphExecutionRecord { + readonly enabled: boolean; + readonly environmentOverrides?: Readonly>; +} + const REQUIRED_FIELDS: Array = [ // command is absent because it is not required 'id', @@ -91,12 +126,18 @@ const REQUIRED_FIELDS: Array = [ /* * Get the operation id */ -export function getOperationId(operation: Operation): string { +export function getOperationId(operation: IGraphOperation): string { const { - associatedPhase: { name: task }, - associatedProject: { packageName } + associatedPhase: { name: task, logFilenameIdentifier: phaseLogFilenameIdentifier }, + associatedProject: { packageName }, + logFilenameIdentifier } = operation; - return `${packageName}#${task}`; + // Native shard operations share a phase but have distinct filesystem-safe operation identifiers. + const suffix: string = + logFilenameIdentifier && logFilenameIdentifier !== phaseLogFilenameIdentifier + ? `#${logFilenameIdentifier}` + : ''; + return `${packageName}#${task}${suffix}`; } export class GraphProcessor { @@ -109,13 +150,33 @@ export class GraphProcessor { /* * Convert the operationMap into an array of graph nodes with empty commands pruned */ - public processOperations(operations: Set): IGraphNode[] { + public processOperations( + operations: ReadonlySet, + executionRecords?: ReadonlyMap + ): IGraphNode[] { const nodeMap: Map = new Map(); for (const operation of operations) { - const entry: IGraphNodeInternal = this.#operationAsHashedEntry(operation); + const record: IGraphExecutionRecord | undefined = executionRecords?.get(operation); + if (executionRecords && !record) { + throw new Error(`Missing execution plan for ${getOperationId(operation)}`); + } + const entry: IGraphNodeInternal = this.#operationAsHashedEntry(operation, record); + if (nodeMap.has(entry.id)) { + throw new Error(`Duplicate operation ID: ${entry.id}`); + } nodeMap.set(entry.id, entry); } + for (const node of nodeMap.values()) { + for (const dependency of node.dependencies) { + if (!nodeMap.has(dependency)) { + throw new Error(`${node.id} has a dependency on ${dependency} which does not exist`); + } + } + } + if (this.#hasDependencyCycle(nodeMap)) { + throw new Error('Graph contains a dependency cycle'); + } return this.#pruneNoOps(nodeMap); } @@ -126,8 +187,14 @@ export class GraphProcessor { */ public validateGraph(entries: readonly Readonly[]): boolean { const entryIDs: Set = new Set(entries.map((entry) => entry.id)); + const nodeMap: Map> = new Map(); let isValid: boolean = true; for (const entry of entries) { + if (nodeMap.has(entry.id)) { + this.#logger.emitError(new Error(`Duplicate operation ID: ${entry.id}`)); + isValid = false; + } + nodeMap.set(entry.id, entry); for (const depId of entry.dependencies) { if (!entryIDs.has(depId)) { this.#logger.emitError(new Error(`${entry.id} has a dependency on ${depId} which does not exist`)); @@ -141,6 +208,11 @@ export class GraphProcessor { } } + if (this.#hasDependencyCycle(nodeMap)) { + this.#logger.emitError(new Error('Graph contains a dependency cycle')); + isValid = false; + } + if (isValid) { this.#logger.terminal.writeLine( Colorize.green('All nodes have non-empty commands and dependencies which exist') @@ -152,6 +224,45 @@ export class GraphProcessor { return isValid; } + #hasDependencyCycle(nodes: ReadonlyMap }>): boolean { + const dependencyCounts: Map = new Map(); + const consumers: Map = new Map(); + for (const [id, node] of nodes) { + let count: number = 0; + for (const dependency of node.dependencies) { + if (nodes.has(dependency)) { + count++; + let dependents: string[] | undefined = consumers.get(dependency); + if (!dependents) { + dependents = []; + consumers.set(dependency, dependents); + } + dependents.push(id); + } + } + dependencyCounts.set(id, count); + } + const ready: string[] = []; + for (const [id, count] of dependencyCounts) { + if (count === 0) { + ready.push(id); + } + } + let visited: number = 0; + while (ready.length > 0) { + const id: string = ready.pop()!; + visited++; + for (const consumer of consumers.get(id) ?? []) { + const remaining: number = dependencyCounts.get(consumer)! - 1; + dependencyCounts.set(consumer, remaining); + if (remaining === 0) { + ready.push(consumer); + } + } + } + return visited !== nodes.size; + } + /* * remove all entries with empty commands * if an entry has a dependency with an empty command, it should inherit the dependencies of the empty command @@ -199,7 +310,7 @@ export class GraphProcessor { /* * Convert an operation into a graph node */ - #operationAsHashedEntry(operation: Operation): IGraphNodeInternal { + #operationAsHashedEntry(operation: IGraphOperation, record?: IGraphExecutionRecord): IGraphNodeInternal { const { runner, associatedPhase: { name: task }, @@ -213,21 +324,35 @@ export class GraphProcessor { } = operation; const dependencies: Set = new Set(); - for (const dependency of operationDependencies.values()) { + for (const dependency of operationDependencies) { const id: string = getOperationId(dependency); dependencies.add(id); } - const node: Partial = { + const enabled: boolean = record?.enabled !== false; + if (enabled && (!runner || (!runner.isNoOp && !runner.getCommandForExecution))) { + throw new Error(`${getOperationId(operation)}: Operation runner does not support command export`); + } + const command: string | undefined = enabled ? runner?.getCommandForExecution?.() : undefined; + const node: IGraphNodeInternal = { id: getOperationId(operation), task, package: packageName, dependencies, workingDirectory, - command: runner?.getConfigHash() + command }; - if (settings?.disableBuildCacheForOperation) { + if (record?.environmentOverrides && Object.keys(record.environmentOverrides).length > 0) { + node.environmentVariables = { ...record.environmentOverrides }; + } + if (settings?.outputFolderNames) { + node.outputFolderNames = [...settings.outputFolderNames]; + } + if (runner?.warningsAreAllowed) { + node.warningsAreAllowed = true; + } + if (runner?.cacheable === false || settings?.disableBuildCacheForOperation) { node.cacheable = false; } @@ -239,18 +364,16 @@ export class GraphProcessor { } if (missingFields.length > 0) { - this.#logger.emitError( - new Error(`Operation is missing required fields ${missingFields.join(', ')}: ${JSON.stringify(node)}`) + throw new Error( + `Operation is missing required fields ${missingFields.join(', ')}: ${JSON.stringify(node)}` ); } // the runner is a no-op if and only if the command is empty - if (!!runner?.isNoOp !== !node.command) { - this.#logger.emitError( - new Error(`${node.id}: Operation runner isNoOp does not match commandToRun existence`) - ); + if (enabled && !!runner?.isNoOp !== !command) { + throw new Error(`${node.id}: Operation runner isNoOp does not match commandToRun existence`); } - return node as IGraphNodeInternal; + return node; } } diff --git a/rush-plugins/rush-buildxl-graph-plugin/src/dropGraph.ts b/rush-plugins/rush-buildxl-graph-plugin/src/dropGraph.ts index c60eb558092..c33ee6f88c7 100644 --- a/rush-plugins/rush-buildxl-graph-plugin/src/dropGraph.ts +++ b/rush-plugins/rush-buildxl-graph-plugin/src/dropGraph.ts @@ -3,7 +3,13 @@ import path from 'node:path'; -import type { ICreateOperationsContext, ILogger, Operation, RushConfiguration } from '@rushstack/rush-sdk'; +import type { + ICreateOperationsContext, + ILogger, + IOperationExecutionResult, + Operation, + RushConfiguration +} from '@rushstack/rush-sdk'; import { JsonFile } from '@rushstack/node-core-library'; import type { IBuildXLRushGraph } from './DropBuildGraphPlugin'; @@ -12,6 +18,7 @@ import { filterObjectForDebug, filterObjectForTesting } from './debugGraphFilter export interface IDropGraphOptions { operations: Set; + executionRecords?: ReadonlyMap; context: ICreateOperationsContext; dropGraphPath: string; rushConfiguration: RushConfiguration; @@ -23,6 +30,7 @@ const DEBUG_RUSH_BUILD_GRAPH_ENV_VAR_NAME: 'DEBUG_RUSH_BUILD_GRAPH' = 'DEBUG_RUS export async function dropGraphAsync(options: IDropGraphOptions): Promise { const { operations, + executionRecords, context, dropGraphPath, rushConfiguration: { commonTempFolder }, @@ -63,14 +71,18 @@ export async function dropGraphAsync(options: IDropGraphOptions): Promise jest.requireActual('@microsoft/rush-lib')); +jest.mock('../dropGraph'); + +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +import { JsonFile } from '@rushstack/node-core-library'; +import { + RushConfiguration, + RushSession, + PhasedCommandHooks, + OperationGraphHooks, + OperationStatus, + type IOperationGraph, + type IOperationGraphContext +} from '@rushstack/rush-sdk'; +import { NoOpTerminalProvider } from '@rushstack/terminal'; +import { CommandLineParser } from '@rushstack/ts-command-line'; + +import { DropBuildGraphPlugin } from '../DropBuildGraphPlugin'; +import { dropGraphAsync } from '../dropGraph'; + +describe(DropBuildGraphPlugin.name, () => { + let folder: string; + let rushConfiguration: RushConfiguration; + + beforeAll(async () => { + folder = await fs.mkdtemp(path.join(os.tmpdir(), 'rush-export-plugin-')); + const configPath: string = path.join(folder, 'rush.json'); + await JsonFile.saveAsync({ rushVersion: '5.182.0', pnpmVersion: '10.27.0', projects: [] }, configPath); + rushConfiguration = RushConfiguration.loadFromConfigurationFile(configPath); + }); + + afterAll(async () => { + await fs.rm(folder, { recursive: true, force: true }); + }); + + it.each(['export', 'watch', 'local', 'invalid'])('handles graph export (%s)', async (mode) => { + const dropGraphMock = jest.mocked(dropGraphAsync); + dropGraphMock.mockReset().mockResolvedValue(mode !== 'invalid'); + if (mode === 'export') { + dropGraphMock.mockImplementation( + jest.requireActual('../dropGraph').dropGraphAsync + ); + } + const hooks: PhasedCommandHooks = new PhasedCommandHooks(); + const session: RushSession = new RushSession({ + terminalProvider: new NoOpTerminalProvider(), + getIsDebugMode: () => false + }); + new DropBuildGraphPlugin({ buildXLCommandNames: ['build'] }).apply(session, rushConfiguration); + const graph: IOperationGraph = { + abortController: new AbortController(), + hooks: new OperationGraphHooks(), + operations: new Set(), + resultByOperation: new Map(), + status: OperationStatus.Ready, + parallelism: 1, + debugMode: false, + quietMode: true, + allowOversubscription: false, + pauseNextIteration: false, + terminalDestinations: new Set(), + hasScheduledIteration: false, + createExecutionPlanAsync: jest.fn(async () => new Map()), + abortCurrentIterationAsync: jest.fn(async () => {}), + closeRunnersAsync: jest.fn(async () => {}), + addTerminalDestination: jest.fn(), + removeTerminalDestination: jest.fn(() => false), + scheduleIterationAsync: jest.fn(async () => false), + executeScheduledIterationAsync: jest.fn(async () => false), + discardScheduledIteration: jest.fn(() => false), + setEnabledStates: jest.fn(() => false), + invalidateOperations: jest.fn() + }; + await session.hooks.runPhasedCommand.for('build').promise({ + actionName: 'build', + hooks, + sessionAbortController: graph.abortController + }); + const parser: CommandLineParser = new CommandLineParser({ + toolFilename: 'test', + toolDescription: 'Graph export test' + }); + const parameter = parser.defineStringParameter({ + parameterLongName: '--drop-graph', + argumentName: 'FILE', + description: 'Graph output' + }); + const dropGraphPath: string = path.join(folder, 'graph.json'); + await parser.executeAsync(mode === 'local' ? [] : ['--drop-graph', dropGraphPath]); + const context: IOperationGraphContext = { + buildCacheConfiguration: undefined, + changedProjectsOnly: false, + cobuildConfiguration: undefined, + customParameters: new Map([['--drop-graph', parameter]]), + includePhaseDeps: true, + projectConfigurations: new Map(), + isIncrementalBuildAllowed: false, + isWatch: mode === 'watch', + parallelism: 1, + phaseSelection: new Set(), + projectSelection: new Set(), + rushConfiguration + }; + expect(hooks.createOperationsAsync.isUsed()).toBe(false); + if (mode === 'watch') { + await expect(hooks.beforeExecuteGraphAsync.promise(graph, context)).rejects.toThrow(/watch mode/); + } else if (mode === 'invalid') { + await expect(hooks.beforeExecuteGraphAsync.promise(graph, context)).rejects.toThrow( + /Failed to validate/ + ); + } else { + await expect(hooks.beforeExecuteGraphAsync.promise(graph, context)).resolves.toBe( + mode === 'export' ? true : undefined + ); + } + if (mode === 'local' || mode === 'watch') { + expect(graph.createExecutionPlanAsync).not.toHaveBeenCalled(); + expect(dropGraphMock).not.toHaveBeenCalled(); + } else { + expect(graph.createExecutionPlanAsync).toHaveBeenCalledWith({ inputsSnapshot: undefined }); + expect(dropGraphMock).toHaveBeenCalledWith({ + operations: graph.operations, + executionRecords: new Map(), + context, + dropGraphPath, + rushConfiguration, + logger: expect.anything() + }); + } + expect(graph.scheduleIterationAsync).not.toHaveBeenCalled(); + expect(graph.executeScheduledIterationAsync).not.toHaveBeenCalled(); + if (mode === 'export') { + expect(await JsonFile.loadAsync(dropGraphPath)).toEqual({ + nodes: [], + repoSettings: { + commonTempFolder: rushConfiguration.commonTempFolder, + executionContractVersion: 1 + } + }); + } + }); +}); diff --git a/rush-plugins/rush-buildxl-graph-plugin/src/test/GraphProcessor.test.ts b/rush-plugins/rush-buildxl-graph-plugin/src/test/GraphProcessor.test.ts index caa1f4a0491..2044f759495 100644 --- a/rush-plugins/rush-buildxl-graph-plugin/src/test/GraphProcessor.test.ts +++ b/rush-plugins/rush-buildxl-graph-plugin/src/test/GraphProcessor.test.ts @@ -1,8 +1,23 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { IOperationRunner, Operation, OperationStatus } from '@rushstack/rush-sdk'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + import { Terminal, NoOpTerminalProvider } from '@rushstack/terminal'; +import { JsonFile } from '@rushstack/node-core-library'; +import type { IPhase } from '@microsoft/rush-lib/lib/api/CommandLineConfiguration'; +import { RushConfiguration } from '@microsoft/rush-lib/lib/api/RushConfiguration'; +import type { IOperationRunner } from '@microsoft/rush-lib/lib/logic/operations/IOperationRunner'; +import type { OperationStatus } from '@microsoft/rush-lib/lib/logic/operations/OperationStatus'; +import { + PhasedCommandHooks, + type ICreateOperationsContext +} from '@microsoft/rush-lib/lib/pluginFramework/PhasedCommandHooks'; +import { Operation } from '@microsoft/rush-lib/lib/logic/operations/Operation'; +import { ShardedPhasedOperationPlugin } from '@microsoft/rush-lib/lib/logic/operations/ShardedPhaseOperationPlugin'; +import { ShellOperationRunnerPlugin } from '@microsoft/rush-lib/lib/logic/operations/ShellOperationRunnerPlugin'; import { GraphProcessor, type IGraphNode } from '../GraphProcessor'; @@ -31,6 +46,10 @@ class MockRunner implements IOperationRunner { public getConfigHash(): string { return this._configHash; } + + public getCommandForExecution(): string { + return this._configHash; + } } function loadDebugGraph(): Operation[] { @@ -50,14 +69,87 @@ function loadDebugGraph(): Operation[] { return operations; } +async function createShardedOperationsAsync(rushConfiguration: RushConfiguration): Promise> { + const phase: IPhase = { + name: '_phase:test', + logFilenameIdentifier: '_phase_test', + isSynthetic: false, + associatedParameters: new Set(), + dependencies: { self: new Set(), upstream: new Set() }, + allowWarningsOnSuccess: false, + missingScriptBehavior: 'error' + }; + const project = rushConfiguration.projects[0]; + const preparation: Operation = new Operation({ + phase: { ...phase, name: '_phase:build', logFilenameIdentifier: '_phase_build' }, + project, + logFilenameIdentifier: '_phase_build' + }); + const collator: Operation = new Operation({ + phase, + project, + logFilenameIdentifier: phase.logFilenameIdentifier, + settings: { operationName: phase.name, sharding: { count: 2, shardOperationSettings: {} } } + }); + collator.addDependency(preparation); + const hooks: PhasedCommandHooks = new PhasedCommandHooks(); + new ShardedPhasedOperationPlugin().apply(hooks); + new ShellOperationRunnerPlugin().apply(hooks); + const context: ICreateOperationsContext = { + buildCacheConfiguration: undefined, + changedProjectsOnly: false, + cobuildConfiguration: undefined, + customParameters: new Map(), + includePhaseDeps: true, + projectConfigurations: new Map(), + isIncrementalBuildAllowed: false, + isWatch: false, + parallelism: 1, + phaseSelection: new Set([phase]), + projectSelection: new Set([project]), + rushConfiguration + }; + return await hooks.createOperationsAsync.promise(new Set([preparation, collator]), context); +} + describe(GraphProcessor.name, () => { let exampleGraph: readonly IGraphNode[]; let graphParser: GraphProcessor; let emittedErrors: Error[]; let emittedWarnings: Error[]; + let fixtureFolder: string; + let rushConfiguration: RushConfiguration; - beforeAll(() => { + beforeAll(async () => { exampleGraph = sortGraphNodes(Array.from(graph.nodes)); + fixtureFolder = await fs.mkdtemp(path.join(os.tmpdir(), 'rush-shard-graph-')); + const configPath: string = path.join(fixtureFolder, 'rush.json'); + await JsonFile.saveAsync( + { + rushVersion: '5.179.0', + pnpmVersion: '10.27.0', + projects: [{ packageName: 'sharded-project', projectFolder: 'projects/sharded-project' }] + }, + configPath + ); + await JsonFile.saveAsync( + { + name: 'sharded-project', + version: '1.0.0', + scripts: { + '_phase:build': 'node prepare.js', + '_phase:test': 'node collate.js', + '_phase:test:shard': 'node test.js' + } + }, + path.join(fixtureFolder, 'projects', 'sharded-project', 'package.json'), + { ensureFolderExists: true } + ); + rushConfiguration = RushConfiguration.loadFromConfigurationFile(configPath); + }); + + afterAll(async () => { + if (fixtureFolder) await fs.rm(fixtureFolder, { recursive: true, force: true }); }); beforeEach(() => { @@ -96,13 +188,169 @@ describe(GraphProcessor.name, () => { it('should fail if isNoOp mismatches a command', () => { const clonedOperationMap: Operation[] = loadDebugGraph(); (clonedOperationMap[0].runner as IOperationRunner & { isNoOp: boolean }).isNoOp = true; - (clonedOperationMap[0].runner as IOperationRunner).getConfigHash = () => 'echo "hello world"'; + (clonedOperationMap[0].runner as IOperationRunner).getCommandForExecution = () => 'echo "hello world"'; const operations: Set = new Set(clonedOperationMap); - graphParser.processOperations(operations); - expect(emittedErrors).not.toEqual([]); + expect(() => graphParser.processOperations(operations)).toThrow(/isNoOp does not match/); + expect(emittedErrors).toEqual([]); expect(emittedWarnings).toEqual([]); }); + it('exports every native shard and its collator with preparation dependencies', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const nodes: IGraphNode[] = graphParser.processOperations(operations); + expect(nodes).toHaveLength(4); + expect(new Set(nodes.map((node) => node.id)).size).toBe(4); + const preparation: IGraphNode = nodes.find((node) => node.command.startsWith('node prepare.js'))!; + const collator: IGraphNode = nodes.find((node) => node.command.startsWith('node collate.js'))!; + const shards: IGraphNode[] = nodes.filter((node) => node.command.startsWith('node test.js')); + expect(preparation.id).toBe('sharded-project#_phase:build'); + expect(shards).toHaveLength(2); + expect(shards.map((node) => node.dependencies)).toEqual([[preparation.id], [preparation.id]]); + expect(new Set(collator.dependencies)).toEqual(new Set(shards.map((node) => node.id))); + expect(shards.map((node) => node.command)).toEqual([ + expect.stringContaining('--shard=1/2'), + expect.stringContaining('--shard=2/2') + ]); + expect(collator.command).toContain('--shard-count="2"'); + expect(sortGraphNodes(graphParser.processOperations(new Set([...operations].reverse())))).toEqual( + sortGraphNodes(nodes) + ); + expect(graphParser.validateGraph(nodes)).toBe(true); + expect(emittedErrors).toEqual([]); + }); + + it('preserves runner-level cache exclusions without disabling sibling shards', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const collator: Operation = [...operations].find( + (operation) => operation.settings?.sharding && !operation.isNoOp + )!; + collator.runner!.cacheable = false; + const nodes: IGraphNode[] = graphParser.processOperations(operations); + expect(nodes.find((node) => node.command.startsWith('node collate.js'))!.cacheable).toBe(false); + expect( + nodes.filter((node) => node.command.startsWith('node test.js')).map((node) => node.cacheable) + ).toEqual([undefined, undefined]); + }); + + it('exports explicit execution metadata without treating opaque no-op hashes as commands', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + for (const operation of operations) { + operation.runner!.getConfigHash = () => 'opaque-hash-not-a-command'; + } + const plan = new Map( + [...operations].map( + (operation) => + [ + operation, + { enabled: true, environmentOverrides: { PUBLIC_GENERATION: 'current-generation' } } + ] as const + ) + ); + const nodes: IGraphNode[] = graphParser.processOperations(operations, plan); + expect(nodes).toHaveLength(4); + expect(nodes.every((node) => node.command.startsWith('node '))).toBe(true); + expect(nodes.every((node) => node.environmentVariables?.PUBLIC_GENERATION === 'current-generation')).toBe( + true + ); + expect(emittedErrors).toEqual([]); + }); + + it('refuses to export a runner without an explicit shell command', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + [...operations].find((operation) => !operation.isNoOp)!.runner!.getCommandForExecution = undefined; + expect(() => graphParser.processOperations(operations)).toThrow(/does not support command export/); + }); + + it('exports only declared metadata and copies it without reading the full environment', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const plan: Map }> = new Map( + [...operations].map( + (operation) => + [ + operation, + { + enabled: true, + environmentOverrides: { PUBLIC_IDENTITY: 'current' }, + get environment(): never { + throw new Error('The computed environment must not be read'); + } + } + ] as const + ) + ); + const shard: Operation = [...operations].find((operation) => operation.settings?.outputFolderNames)!; + Object.defineProperty(shard.runner!, 'warningsAreAllowed', { value: true }); + const nodes: IGraphNode[] = graphParser.processOperations(operations, plan); + const exported: IGraphNode = nodes.find((node) => node.id.endsWith(shard.logFilenameIdentifier))!; + expect(exported.task).toBe('_phase:test'); + expect(exported).not.toHaveProperty('phase'); + expect(exported.environmentVariables).toEqual({ PUBLIC_IDENTITY: 'current' }); + expect(exported.outputFolderNames).toEqual(shard.settings!.outputFolderNames); + expect(exported.outputFolderNames).not.toBe(shard.settings!.outputFolderNames); + expect(exported.warningsAreAllowed).toBe(true); + plan.get(shard)!.environmentOverrides.PUBLIC_IDENTITY = 'later'; + expect(exported.environmentVariables).toEqual({ PUBLIC_IDENTITY: 'current' }); + }); + + it('prunes disabled nodes while preserving their transitive dependencies', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const shard: Operation = [...operations].find((operation) => operation.settings?.outputFolderNames)!; + shard.runner!.getCommandForExecution = () => { + throw new Error('Disabled commands must not be read'); + }; + const plan = new Map([...operations].map((operation) => [operation, { enabled: operation !== shard }])); + const nodes: IGraphNode[] = graphParser.processOperations(operations, plan); + expect(nodes).toHaveLength(3); + const preparation: IGraphNode = nodes.find((node) => node.command.startsWith('node prepare.js'))!; + const collator: IGraphNode = nodes.find((node) => node.command.startsWith('node collate.js'))!; + const remainingShard: IGraphNode = nodes.find((node) => node.command.startsWith('node test.js'))!; + expect(new Set(collator.dependencies)).toEqual(new Set([preparation.id, remainingShard.id])); + expect(graphParser.validateGraph(nodes)).toBe(true); + }); + + it('rejects an incomplete execution plan', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + expect(() => graphParser.processOperations(operations, new Map())).toThrow(/Missing execution plan/); + }); + + it('rejects duplicate operation identities rather than overwriting a command', () => { + const operations: Operation[] = loadDebugGraph(); + const duplicate: Operation = new Operation({ + phase: operations[0].associatedPhase, + project: operations[0].associatedProject, + runner: operations[0].runner, + logFilenameIdentifier: operations[0].logFilenameIdentifier + }); + expect(() => graphParser.processOperations(new Set([...operations, duplicate]))).toThrow( + /Duplicate operation ID/ + ); + }); + + it('rejects cycles before pruning no-op operations', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const preShard: Operation = [...operations].find((operation) => operation.isNoOp)!; + preShard.addDependency(preShard); + expect(() => graphParser.processOperations(operations)).toThrow(/dependency cycle/); + }); + + it('rejects missing operation dependencies before pruning', async () => { + const operations: Set = await createShardedOperationsAsync(rushConfiguration); + const operation: Operation = [...operations][0]; + operation.addDependency( + new Operation({ + phase: { + ...operation.associatedPhase, + name: '_phase:missing', + logFilenameIdentifier: '_phase_missing' + }, + project: operation.associatedProject, + runner: operation.runner, + logFilenameIdentifier: '_phase_missing' + }) + ); + expect(() => graphParser.processOperations(operations)).toThrow(/dependency .* which does not exist/); + }); + describe(GraphProcessor.prototype.validateGraph.name, () => { it('should validate graph.json', () => { const isValid: boolean = graphParser.validateGraph(exampleGraph); @@ -155,5 +403,33 @@ describe(GraphProcessor.name, () => { expect(emittedErrors).toMatchSnapshot(); expect(emittedWarnings).toEqual([]); }); + + it('rejects duplicate node IDs', () => { + expect(graphParser.validateGraph([...exampleGraph, exampleGraph[0]])).toBe(false); + expect(emittedErrors.some((error) => error.message.includes('Duplicate operation ID'))).toBe(true); + }); + + it.each([false, true])('rejects dependency cycles (self-dependency=%s)', (selfDependency) => { + const entries: IGraphNode[] = [ + { + id: 'a', + task: 'test', + package: 'a', + workingDirectory: '/repo', + command: 'test', + dependencies: [selfDependency ? 'a' : 'b'] + }, + { + id: 'b', + task: 'test', + package: 'b', + workingDirectory: '/repo', + command: 'test', + dependencies: ['a'] + } + ]; + expect(graphParser.validateGraph(entries)).toBe(false); + expect(emittedErrors.map((error) => error.message)).toEqual(['Graph contains a dependency cycle']); + }); }); });