From fad094f19afcd797b2f4280497833efbfbfc3014 Mon Sep 17 00:00:00 2001 From: Yashwanth Reddy Verupaka <59129034+yashwanth195@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:35:30 -0800 Subject: [PATCH] Fix environment snapshot hashing --- .../rush/environment-snapshot_2026-09-16.json | 10 + .../src/logic/incremental/InputsSnapshot.ts | 57 +++--- .../incremental/test/InputsSnapshot.test.ts | 186 ++++++++++++++++++ .../__snapshots__/InputsSnapshot.test.ts.snap | 4 +- 4 files changed, 229 insertions(+), 28 deletions(-) create mode 100644 common/changes/@microsoft/rush/environment-snapshot_2026-09-16.json diff --git a/common/changes/@microsoft/rush/environment-snapshot_2026-09-16.json b/common/changes/@microsoft/rush/environment-snapshot_2026-09-16.json new file mode 100644 index 00000000000..670090e0c5c --- /dev/null +++ b/common/changes/@microsoft/rush/environment-snapshot_2026-09-16.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix declared environment input hashing to preserve Windows case-insensitive lookup, distinguish absent and empty values, and snapshot supplied environments before querying operation hashes. Operations with declared environment inputs will receive new cache keys.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts b/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts index ad96a3e9007..c46b597db55 100644 --- a/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts +++ b/libraries/rush-lib/src/logic/incremental/InputsSnapshot.ts @@ -7,7 +7,7 @@ import { createHash, type Hash } from 'node:crypto'; import ignore, { type Ignore } from 'ignore'; import { type IReadonlyLookupByPath, LookupByPath } from '@rushstack/lookup-by-path'; -import { InternalError, Path, Sort } from '@rushstack/node-core-library'; +import { EnvironmentMap, InternalError, Path, Sort } from '@rushstack/node-core-library'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; import type { @@ -90,7 +90,7 @@ export interface IInputsSnapshotParameters { */ additionalHashes?: ReadonlyMap; /** - * The environment to use for `dependsOnEnvVars`. By default performs a snapshot of process.env upon construction. + * The environment to snapshot for `dependsOnEnvVars`. Defaults to process.env upon construction. * @defaultValue \{ ...process.env \} */ environment?: Record; @@ -251,7 +251,7 @@ export class InputsSnapshot implements IInputsSnapshot { /** * The environment to use for `dependsOnEnvVars`. */ - readonly #environment: Record; + readonly #environment: EnvironmentMap; /** * Pre-computed Node.js version strings at each granularity level for `dependsOnNodeVersion`. */ @@ -272,7 +272,7 @@ export class InputsSnapshot implements IInputsSnapshot { public constructor(params: IInputsSnapshotParameters) { const { additionalHashes, - environment = { ...process.env }, + environment = process.env, globalAdditionalFiles, hashes, hasUncommittedChanges, @@ -306,11 +306,13 @@ export class InputsSnapshot implements IInputsSnapshot { const operationAdditionalFileCount: number = countOperationAdditionalFiles(projectMap); + // Snapshot the environment before comparing or querying it. + this.#environment = new EnvironmentMap(environment); this.#projectMetadataMap = (previousSnapshot && previousSnapshot.#tryDeriveProjectMetadataMap( params, - environment, + this.#environment, nodeVersion, globalAdditionalHashes, operationAdditionalFileCount @@ -318,8 +320,6 @@ export class InputsSnapshot implements IInputsSnapshot { createProjectMetadataMap(hashes, lookupByPath, projectMap, rootDir); this.#additionalHashes = additionalHashes; this.#globalAdditionalHashes = globalAdditionalHashes; - // Snapshot the environment so that queries are not impacted by when they happen - this.#environment = environment; // Parse Node.js version once so it doesn't need to be re-parsed per operation this.#nodeVersionByGranularity = _parseNodeVersion(nodeVersion); this.#lookupByPath = lookupByPath; @@ -427,15 +427,22 @@ export class InputsSnapshot implements IInputsSnapshot { const operationSettings: Readonly | undefined = operationName ? record.projectConfig?.operationSettingsByOperationName.get(operationName) : undefined; - const snapshotEnvironment: Readonly> = this.#environment; - if ( - environment && - operationSettings?.dependsOnEnvVars?.some( - (envVar: string) => (environment[envVar] || '') !== (snapshotEnvironment[envVar] || '') - ) - ) { - // The operation's environment differs from the snapshot's in a variable that it hashes, so don't memoize. - return this.#computeOperationOwnStateHash(project, operationName, operationSettings, environment); + const snapshotEnvironment: EnvironmentMap = this.#environment; + if (environment && operationSettings?.dependsOnEnvVars?.length) { + const operationEnvironment: EnvironmentMap = new EnvironmentMap(environment); + if ( + operationSettings.dependsOnEnvVars.some( + (envVar: string) => operationEnvironment.get(envVar) !== snapshotEnvironment.get(envVar) + ) + ) { + // The operation's environment differs from the snapshot's in a variable that it hashes, so don't memoize. + return this.#computeOperationOwnStateHash( + project, + operationName, + operationSettings, + operationEnvironment + ); + } } const { hashByOperationName } = record; @@ -457,7 +464,7 @@ export class InputsSnapshot implements IInputsSnapshot { project: IRushConfigurationProjectForSnapshot, operationName: string | undefined, operationSettings: Readonly | undefined, - environment: Readonly> + environment: EnvironmentMap ): string { const hashes: ReadonlyMap = this.getTrackedFileHashesForOperation(project, operationName); @@ -469,7 +476,8 @@ export class InputsSnapshot implements IInputsSnapshot { // As long as we enumerate environment variables in a consistent order, we will get a stable hash. // Changing the order in rush-project.json will change the hash anyway since the file contents are part of the hash. for (const envVar of dependsOnEnvVars) { - hasher.update(`${hashDelimiter}$${envVar}=${environment[envVar] || ''}`); + // Encode absence separately from an empty value, and escape delimiters within values. + hasher.update(`${hashDelimiter}$${JSON.stringify([envVar, environment.get(envVar)])}`); } } @@ -512,7 +520,7 @@ export class InputsSnapshot implements IInputsSnapshot { */ #tryDeriveProjectMetadataMap( params: IInputsSnapshotParameters, - environment: Readonly>, + environment: EnvironmentMap, nodeVersion: string, globalAdditionalHashes: ReadonlyMap | undefined, operationAdditionalFileCount: number @@ -748,17 +756,14 @@ function areMapsEqual(a: ReadonlyMap, b: ReadonlyMap>, - b: Readonly> -): boolean { - const keys: string[] = Object.keys(a); - if (keys.length !== Object.keys(b).length) { +function areEnvironmentsEqual(a: EnvironmentMap, b: EnvironmentMap): boolean { + const keys: string[] = Array.from(a.names()); + if (keys.length !== Array.from(b.names()).length) { return false; } for (const key of keys) { - if (a[key] !== b[key] || !Object.prototype.hasOwnProperty.call(b, key)) { + if (a.get(key) !== b.get(key)) { return false; } } diff --git a/libraries/rush-lib/src/logic/incremental/test/InputsSnapshot.test.ts b/libraries/rush-lib/src/logic/incremental/test/InputsSnapshot.test.ts index fc29aaa8487..4ed187c5b05 100644 --- a/libraries/rush-lib/src/logic/incremental/test/InputsSnapshot.test.ts +++ b/libraries/rush-lib/src/logic/incremental/test/InputsSnapshot.test.ts @@ -4,6 +4,7 @@ import { LookupByPath } from '@rushstack/lookup-by-path'; import type { RushProjectConfiguration } from '../../../api/RushProjectConfiguration'; +import { RushConstants } from '../../RushConstants'; import { InputsSnapshot, type IInputsSnapshotParameters, @@ -795,6 +796,122 @@ describe(InputsSnapshot.name, () => { }); }); + describe('environment identity', () => { + function createSnapshot( + environment: Record, + names: string[] = ['ENV_VAR'] + ): ReturnType { + const { project, options } = getTestConfig(); + const projectConfig: Pick = { + operationSettingsByOperationName: new Map([ + ['_phase:build', { operationName: '_phase:build', dependsOnEnvVars: names }] + ]) + }; + return { + project, + input: new InputsSnapshot({ + ...options, + environment, + projectMap: new Map([[project, { projectConfig: projectConfig as RushProjectConfiguration }]]) + }) + }; + } + + describe.each([false, true])('with an explicit operation environment: %s', (useOperationEnvironment) => { + function getHash(environment: Record, names?: string[]): string { + const { project, input } = createSnapshot( + useOperationEnvironment ? { ENV_VAR: 'snapshot' } : environment, + names + ); + return input.getOperationOwnStateHash( + project, + '_phase:build', + useOperationEnvironment ? environment : undefined + ); + } + + it('distinguishes absence, empty values and literal sentinel strings', () => { + const values: Array = [undefined, '', 'undefined', 'null', 'value']; + expect(new Set(values.map((value) => getHash({ ENV_VAR: value }))).size).toBe(values.length); + expect(getHash({})).toBe(getHash({ ENV_VAR: undefined })); + }); + + it.each(['win32', 'linux'])('uses %s environment-name semantics', (platform) => { + const descriptor: PropertyDescriptor = Object.getOwnPropertyDescriptor(process, 'platform')!; + Object.defineProperty(process, 'platform', { ...descriptor, value: platform }); + try { + const upper: string = getHash({ ENV_VAR: 'first' }); + const lower: string = getHash({ env_var: 'first' }); + const changed: string = getHash({ env_var: 'second' }); + if (platform === 'win32') { + expect(lower).toBe(upper); + expect(changed).not.toBe(lower); + } else { + expect(lower).toBe(getHash({})); + expect(changed).toBe(lower); + expect(lower).not.toBe(upper); + } + } finally { + Object.defineProperty(process, 'platform', descriptor); + } + }); + + it('does not include undeclared environment inputs', () => { + expect(getHash({ ENV_VAR: 'same', OTHER: 'first' })).toBe( + getHash({ ENV_VAR: 'same', OTHER: 'second' }) + ); + }); + + it('does not confuse embedded separators with another declared variable', () => { + const delimiter: string = RushConstants.hashDelimiter; + expect(getHash({ A: `a${delimiter}$B=b`, B: 'c' }, ['A', 'B'])).not.toBe( + getHash({ A: 'a', B: `b${delimiter}$B=c` }, ['A', 'B']) + ); + }); + }); + + it('captures supplied values before the first hash query', () => { + const environment: Record = { ENV_VAR: 'captured' }; + const { project, input } = createSnapshot(environment); + const { project: baselineProject, input: baseline } = createSnapshot({ ...environment }); + environment.ENV_VAR = 'changed'; + expect(input.getOperationOwnStateHash(project, '_phase:build')).toBe( + baseline.getOperationOwnStateHash(baselineProject, '_phase:build') + ); + expect(input.getOperationOwnStateHash(project, '_phase:build', environment)).not.toBe( + input.getOperationOwnStateHash(project, '_phase:build') + ); + }); + + it.each([undefined, ''])( + 'does not reuse or overwrite the snapshot hash for a different operation value (snapshot: %s)', + (snapshotValue) => { + const { project, input } = createSnapshot({ ENV_VAR: snapshotValue }); + const operationEnvironment: Record = { + ENV_VAR: snapshotValue === undefined ? '' : undefined + }; + const { project: operationProject, input: operationSnapshot } = createSnapshot(operationEnvironment); + const operationHash: string = operationSnapshot.getOperationOwnStateHash( + operationProject, + '_phase:build' + ); + expect(input.getOperationOwnStateHash(project, '_phase:build', operationEnvironment)).toBe( + operationHash + ); + const snapshotHash: string = input.getOperationOwnStateHash(project, '_phase:build'); + expect(snapshotHash).not.toBe(operationHash); + expect(input.getOperationOwnStateHash(project, '_phase:build', operationEnvironment)).toBe( + operationHash + ); + operationEnvironment.ENV_VAR = snapshotValue; + expect(input.getOperationOwnStateHash(project, '_phase:build', operationEnvironment)).toBe( + snapshotHash + ); + expect(input.getOperationOwnStateHash(project, '_phase:build')).toBe(snapshotHash); + } + ); + }); + describe('previousSnapshot', () => { const operationNames: (string | undefined)[] = [undefined, '_phase:build', '_phase:test']; @@ -1064,6 +1181,75 @@ describe(InputsSnapshot.name, () => { } }); + it.each([false, true])( + 'does not reuse stale environment state after caller mutation (previous hash computed: %s)', + (computePreviousHash) => { + const { projects, a, options } = getDerivationTestConfig(); + const environment: Record = { FOO: 'captured' }; + options.environment = environment; + const previousSnapshot: InputsSnapshot = new InputsSnapshot(options); + const baselineHash: string = new InputsSnapshot(options).getOperationOwnStateHash(a, '_phase:build'); + if (computePreviousHash) { + expect(previousSnapshot.getOperationOwnStateHash(a, '_phase:build')).toBe(baselineHash); + } + + environment.FOO = 'changed'; + const derivedOptions: IInputsSnapshotParameters = { ...options, previousSnapshot }; + const derived: InputsSnapshot = new InputsSnapshot(derivedOptions); + expect(derived.getOperationOwnStateHash(a, '_phase:build')).not.toBe(baselineHash); + expect(previousSnapshot.getOperationOwnStateHash(a, '_phase:build')).toBe(baselineHash); + expectSameAsNewSnapshot(derived, derivedOptions, projects); + } + ); + + it.each([undefined, ''])('distinguishes absent and empty environments during reuse: %s', (value) => { + const { projects, a, options } = getDerivationTestConfig(); + options.environment = { FOO: value }; + const previousSnapshot: InputsSnapshot = new InputsSnapshot(options); + const baselineHash: string = previousSnapshot.getOperationOwnStateHash(a, '_phase:build'); + const derivedOptions: IInputsSnapshotParameters = { + ...options, + environment: { FOO: value === undefined ? '' : undefined }, + previousSnapshot + }; + const derived: InputsSnapshot = new InputsSnapshot(derivedOptions); + expect(derived.getOperationOwnStateHash(a, '_phase:build')).not.toBe(baselineHash); + expectSameAsNewSnapshot(derived, derivedOptions, projects); + }); + + it.each(['win32', 'linux'])('uses %s environment-name semantics during reuse', (platform) => { + const descriptor: PropertyDescriptor = Object.getOwnPropertyDescriptor(process, 'platform')!; + Object.defineProperty(process, 'platform', { ...descriptor, value: platform }); + try { + const { projects, a, options } = getDerivationTestConfig(); + const previousSnapshot: InputsSnapshot = new InputsSnapshot(options); + const previousFiles: ReadonlyMap = previousSnapshot.getTrackedFileHashesForOperation( + a, + '_phase:build' + ); + queryAll(previousSnapshot, projects); + + const derivedOptions: IInputsSnapshotParameters = { + ...options, + environment: { foo: '1' }, + previousSnapshot + }; + const derived: InputsSnapshot = new InputsSnapshot(derivedOptions); + expect(derived.getTrackedFileHashesForOperation(a, '_phase:build') === previousFiles).toBe( + platform === 'win32' + ); + expectSameAsNewSnapshot(derived, derivedOptions, projects); + + const changedOptions: IInputsSnapshotParameters = { + ...derivedOptions, + environment: { foo: '2' } + }; + expectSameAsNewSnapshot(new InputsSnapshot(changedOptions), changedOptions, projects); + } finally { + Object.defineProperty(process, 'platform', descriptor); + } + }); + it('Computes the same state as a new snapshot after any sequence of changes', () => { const { projects, bAdditionalFiles, options } = getDerivationTestConfig(); const files: string[] = [ diff --git a/libraries/rush-lib/src/logic/incremental/test/__snapshots__/InputsSnapshot.test.ts.snap b/libraries/rush-lib/src/logic/incremental/test/__snapshots__/InputsSnapshot.test.ts.snap index f8dcd553d3f..fb766150fdb 100644 --- a/libraries/rush-lib/src/logic/incremental/test/__snapshots__/InputsSnapshot.test.ts.snap +++ b/libraries/rush-lib/src/logic/incremental/test/__snapshots__/InputsSnapshot.test.ts.snap @@ -6,9 +6,9 @@ exports[`InputsSnapshot getOperationOwnStateHash Handles trivial input 1`] = `"a exports[`InputsSnapshot getOperationOwnStateHash Respects additionalOutputFilesByOperationName 1`] = `"07f4147294d21a07865de84875d60bfbcc690652"`; -exports[`InputsSnapshot getOperationOwnStateHash Respects dependsOnEnvVars 1`] = `"ad1f915d0ac6331c09febbbe1496c970a5401b73"`; +exports[`InputsSnapshot getOperationOwnStateHash Respects dependsOnEnvVars 1`] = `"47a31b2ea9deae3582aa2b3e87d0313a240220fc"`; -exports[`InputsSnapshot getOperationOwnStateHash Respects dependsOnEnvVars 2`] = `"2c68d56fc9278b6495496070a6a992b929c37a83"`; +exports[`InputsSnapshot getOperationOwnStateHash Respects dependsOnEnvVars 2`] = `"3ded0c253e5e9d12f32fd502285cc4707e88af76"`; exports[`InputsSnapshot getOperationOwnStateHash Respects dependsOnNodeVersion 1`] = `"cc182bde6aa81c0410ede7db22f3af2f0a24d3e3"`;