From 4f4b2a60692c779628b924c4167acb886886c9c3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:17:23 +0000 Subject: [PATCH 01/16] Initial plan From 32366654179990a3fa5f72042fa9e484ee28ea68 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:25:08 +0000 Subject: [PATCH 02/16] Add excludeVersionOnlyChanges option to ProjectChangeAnalyzer Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- common/reviews/api/rush-lib.api.md | 1 + .../rush-lib/src/cli/actions/ChangeAction.ts | 4 +- .../src/logic/ProjectChangeAnalyzer.ts | 92 ++++++++++++++++++- 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index caa4928eba5..471a2e19f6f 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -497,6 +497,7 @@ export interface IGenerateCacheEntryIdOptions { // @beta (undocumented) export interface IGetChangedProjectsOptions { enableFiltering: boolean; + excludeVersionOnlyChanges?: boolean; includeExternalDependencies: boolean; // (undocumented) shouldFetch?: boolean; diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index ed4ab9486f4..9167d5bca15 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -366,7 +366,9 @@ export class ChangeAction extends BaseRushAction { // Not enabling, since this would be a breaking change includeExternalDependencies: false, // Since install may not have happened, cannot read rush-project.json - enableFiltering: false + enableFiltering: false, + // Exclude version-only changes to prevent 'rush version --bump' from triggering 'rush change --verify' + excludeVersionOnlyChanges: true }); const projectHostMap: Map = this._generateHostMap(); diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index fdb021acb2a..cc3bd45fc5e 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -50,6 +50,13 @@ export interface IGetChangedProjectsOptions { * and exclude matched files from change detection. */ enableFiltering: boolean; + + /** + * If set to `true`, for each project, if the only change to the project is to its `package.json`, + * diffs the file against the base version to see if the only change is to the value of the "version" field, + * and if it is, ignores the change to the project. + */ + excludeVersionOnlyChanges?: boolean; } /** @@ -83,8 +90,15 @@ export class ProjectChangeAnalyzer { ): Promise> { const { _rushConfiguration: rushConfiguration } = this; - const { targetBranchName, terminal, includeExternalDependencies, enableFiltering, shouldFetch, variant } = - options; + const { + targetBranchName, + terminal, + includeExternalDependencies, + enableFiltering, + shouldFetch, + variant, + excludeVersionOnlyChanges + } = options; const gitPath: string = this._git.getGitPathOrThrow(); const repoRoot: string = getRepoRoot(rushConfiguration.rushJsonFolder); @@ -130,6 +144,36 @@ export class ProjectChangeAnalyzer { } } + // Check for version-only changes if excludeVersionOnlyChanges is enabled + if (excludeVersionOnlyChanges) { + const projectsToCheck: RushConfigurationProject[] = Array.from(changedProjects); + await Async.forEachAsync( + projectsToCheck, + async (project) => { + const projectChanges: Map | undefined = changesByProject.get(project); + if (projectChanges && projectChanges.size === 1) { + // Check if the only change is to package.json + const packageJsonPath: string = Path.convertToSlashes( + path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) + ); + const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); + if (diffStatus) { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( + project, + packageJsonPath, + diffStatus, + repoRoot + ); + if (isVersionOnlyChange) { + changedProjects.delete(project); + } + } + } + }, + { concurrency: 10 } + ); + } + // External dependency changes are not allowed to be filtered, so add these after filtering if (includeExternalDependencies) { // Even though changing the installed version of a nested dependency merits a change file, @@ -395,6 +439,50 @@ export class ProjectChangeAnalyzer { } } + /** + * Checks if the only change to a package.json file is to the "version" field. + * @internal + */ + private async _isVersionOnlyChangeAsync( + project: RushConfigurationProject, + packageJsonPath: string, + diffStatus: IFileDiffStatus, + repoRoot: string + ): Promise { + try { + // Get the old version of package.json from Git using the blob id from IFileDiffStatus + const oldPackageJsonContent: string = await this._git.getBlobContentAsync({ + blobSpec: diffStatus.oldhash, + repositoryRoot: repoRoot + }); + + // Get the current version of package.json + const currentPackageJsonPath: string = path.join(repoRoot, packageJsonPath); + const currentPackageJsonContent: string = await FileSystem.readFileAsync(currentPackageJsonPath); + + // Parse both versions + const oldPackageJson: { version?: string } = JSON.parse(oldPackageJsonContent); + const currentPackageJson: { version?: string } = JSON.parse(currentPackageJsonContent); + + // Create copies without the version field for comparison + const oldPackageJsonWithoutVersion: Record = { ...oldPackageJson }; + const currentPackageJsonWithoutVersion: Record = { ...currentPackageJson }; + delete oldPackageJsonWithoutVersion.version; + delete currentPackageJsonWithoutVersion.version; + + // Check if the only difference is the version field + const oldVersionlessJson: string = JSON.stringify(oldPackageJsonWithoutVersion, null, 2); + const currentVersionlessJson: string = JSON.stringify(currentPackageJsonWithoutVersion, null, 2); + + return ( + oldVersionlessJson === currentVersionlessJson && oldPackageJson.version !== currentPackageJson.version + ); + } catch (error) { + // If we can't read the file or parse it, assume it's not a version-only change + return false; + } + } + /** * @internal */ From 1d0d94eb1dbb03ecc7774f1df69600339484ea07 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:28:30 +0000 Subject: [PATCH 03/16] Add tests for excludeVersionOnlyChanges functionality Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../logic/test/ProjectChangeAnalyzer.test.ts | 224 ++++++++++++++++-- 1 file changed, 201 insertions(+), 23 deletions(-) diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index b90254d5652..da3eabab9cb 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -21,6 +21,10 @@ const mockHashes: Map = new Map([ ['j/package.json', 'hash17'], ['rush.json', 'hash18'] ]); + +// Allow customizing repo changes for different test scenarios +let mockRepoChanges: Map = new Map(); + jest.mock(`@rushstack/package-deps-hash`, () => { return { getRepoRoot(dir: string): string { @@ -44,33 +48,16 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath])); }, getRepoChanges(): Map { - return new Map([ - [ - // Test subspace lockfile change detection - 'common/config/subspaces/project-change-analyzer-test-subspace/pnpm-lock.yaml', - { - mode: 'modified', - newhash: 'newhash', - oldhash: 'oldhash', - status: 'M' - } - ], - [ - // Test lockfile deletion detection - 'common/config/subspaces/default/pnpm-lock.yaml', - { - mode: 'deleted', - newhash: '', - oldhash: 'oldhash', - status: 'D' - } - ] - ]); + return mockRepoChanges; } }; }); const { Git: OriginalGit } = jest.requireActual('../Git'); + +// Store mock package.json contents for testing excludeVersionOnlyChanges +const mockPackageJsonContents: Map = new Map(); + /** Mock Git to test `getChangedProjectsAsync` */ jest.mock('../Git', () => { return { @@ -82,7 +69,9 @@ jest.mock('../Git', () => { return 'merge-base-sha'; } public async getBlobContentAsync(opts: { blobSpec: string; repositoryRoot: string }): Promise { - return ''; + // Check if we have a mock for this blob spec + const content = mockPackageJsonContents.get(opts.blobSpec); + return content || ''; } } }; @@ -139,6 +128,8 @@ import type { describe(ProjectChangeAnalyzer.name, () => { beforeEach(() => { mockSnapshot.mockClear(); + mockPackageJsonContents.clear(); + mockRepoChanges = new Map(); }); describe(ProjectChangeAnalyzer.prototype._tryGetSnapshotProviderAsync.name, () => { @@ -172,6 +163,30 @@ describe(ProjectChangeAnalyzer.name, () => { describe(ProjectChangeAnalyzer.prototype.getChangedProjectsAsync.name, () => { it('Subspaces detects external changes', async () => { + // Set up mock repo changes for this test + mockRepoChanges = new Map([ + [ + // Test subspace lockfile change detection + 'common/config/subspaces/project-change-analyzer-test-subspace/pnpm-lock.yaml', + { + mode: 'modified', + newhash: 'newhash', + oldhash: 'oldhash', + status: 'M' + } + ], + [ + // Test lockfile deletion detection + 'common/config/subspaces/default/pnpm-lock.yaml', + { + mode: 'deleted', + newhash: '', + oldhash: 'oldhash', + status: 'D' + } + ] + ]); + const rootDir: string = resolve(__dirname, 'repoWithSubspaces'); const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( resolve(rootDir, 'rush.json') @@ -200,6 +215,169 @@ describe(ProjectChangeAnalyzer.name, () => { expect(changedProjects.has(rushConfiguration.getProjectByName(projectName)!)).toBe(false); }); }); + + it('excludeVersionOnlyChanges excludes projects with only version field changes', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + // Mock package.json with only version change + const oldPackageJsonContent = JSON.stringify( + { + name: 'a', + version: '1.0.0', + description: 'Test package', + dependencies: { + b: '1.0.0' + } + }, + null, + 2 + ); + + const newPackageJsonContent = JSON.stringify( + { + name: 'a', + version: '1.0.1', + description: 'Test package', + dependencies: { + b: '1.0.0' + } + }, + null, + 2 + ); + + // Set up mock repo changes - only package.json changed for project 'a' + mockRepoChanges = new Map([ + [ + 'a/package.json', + { + mode: 'modified', + newhash: 'newhash1', + oldhash: 'oldhash1', + status: 'M' + } + ] + ]); + + // Mock the blob content to return old package.json + mockPackageJsonContents.set('oldhash1', oldPackageJsonContent); + + // Mock FileSystem.readFileAsync to return new package.json + const FileSystem = require('@rushstack/node-core-library').FileSystem; + const originalReadFileAsync = FileSystem.readFileAsync; + FileSystem.readFileAsync = jest.fn().mockImplementation((filePath: string) => { + if (filePath.endsWith('a/package.json')) { + return Promise.resolve(newPackageJsonContent); + } + return originalReadFileAsync(filePath); + }); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true); + const terminal: Terminal = new Terminal(terminalProvider); + + // Test without excludeVersionOnlyChanges - project should be detected as changed + const changedProjectsWithoutExclude = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal + }); + expect(changedProjectsWithoutExclude.has(rushConfiguration.getProjectByName('a')!)).toBe(true); + + // Test with excludeVersionOnlyChanges - project should NOT be detected as changed + const changedProjectsWithExclude = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + expect(changedProjectsWithExclude.has(rushConfiguration.getProjectByName('a')!)).toBe(false); + + // Restore original function + FileSystem.readFileAsync = originalReadFileAsync; + }); + + it('excludeVersionOnlyChanges does not exclude projects with non-version changes', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + // Mock package.json with version AND dependency change + const oldPackageJsonContent = JSON.stringify( + { + name: 'b', + version: '1.0.0', + description: 'Test package', + dependencies: { + a: '1.0.0' + } + }, + null, + 2 + ); + + const newPackageJsonContent = JSON.stringify( + { + name: 'b', + version: '1.0.1', + description: 'Test package', + dependencies: { + a: '1.0.1' // Dependency version also changed + } + }, + null, + 2 + ); + + // Set up mock repo changes - only package.json changed for project 'b' + mockRepoChanges = new Map([ + [ + 'b/package.json', + { + mode: 'modified', + newhash: 'newhash2', + oldhash: 'oldhash2', + status: 'M' + } + ] + ]); + + // Mock the blob content to return old package.json + mockPackageJsonContents.set('oldhash2', oldPackageJsonContent); + + // Mock FileSystem.readFileAsync to return new package.json + const FileSystem = require('@rushstack/node-core-library').FileSystem; + const originalReadFileAsync = FileSystem.readFileAsync; + FileSystem.readFileAsync = jest.fn().mockImplementation((filePath: string) => { + if (filePath.endsWith('b/package.json')) { + return Promise.resolve(newPackageJsonContent); + } + return originalReadFileAsync(filePath); + }); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true); + const terminal: Terminal = new Terminal(terminalProvider); + + // Test with excludeVersionOnlyChanges - project should still be detected as changed + const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true); + + // Restore original function + FileSystem.readFileAsync = originalReadFileAsync; + }); }); }); From 585ee428f95a04ddf2d127b63421c706fe340808 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:31:03 +0000 Subject: [PATCH 04/16] Address code review feedback Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts | 9 +++++++-- .../src/logic/test/ProjectChangeAnalyzer.test.ts | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index cc3bd45fc5e..53ead883b0b 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -461,8 +461,13 @@ export class ProjectChangeAnalyzer { const currentPackageJsonContent: string = await FileSystem.readFileAsync(currentPackageJsonPath); // Parse both versions - const oldPackageJson: { version?: string } = JSON.parse(oldPackageJsonContent); - const currentPackageJson: { version?: string } = JSON.parse(currentPackageJsonContent); + const oldPackageJson: Record = JSON.parse(oldPackageJsonContent); + const currentPackageJson: Record = JSON.parse(currentPackageJsonContent); + + // Ensure both have a version field + if (!oldPackageJson.version || !currentPackageJson.version) { + return false; + } // Create copies without the version field for comparison const oldPackageJsonWithoutVersion: Record = { ...oldPackageJson }; diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index da3eabab9cb..419118182aa 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -23,6 +23,8 @@ const mockHashes: Map = new Map([ ]); // Allow customizing repo changes for different test scenarios +// Using 'let' here because we need to reassign this in beforeEach and individual tests +// to set up different mock scenarios for each test case let mockRepoChanges: Map = new Map(); jest.mock(`@rushstack/package-deps-hash`, () => { From db48dd88371a7812559ab6e8bbfc96ef1128cc39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:44:24 +0000 Subject: [PATCH 05/16] Refactor test mocks to use jest.fn() for cleaner test setup Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../logic/test/ProjectChangeAnalyzer.test.ts | 118 +++++++++--------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index 419118182aa..675b84cc4a8 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -22,10 +22,8 @@ const mockHashes: Map = new Map([ ['rush.json', 'hash18'] ]); -// Allow customizing repo changes for different test scenarios -// Using 'let' here because we need to reassign this in beforeEach and individual tests -// to set up different mock scenarios for each test case -let mockRepoChanges: Map = new Map(); +// Mock function for customizing repo changes in each test +const mockGetRepoChanges = jest.fn, []>(); jest.mock(`@rushstack/package-deps-hash`, () => { return { @@ -50,15 +48,15 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath])); }, getRepoChanges(): Map { - return mockRepoChanges; + return mockGetRepoChanges(); } }; }); const { Git: OriginalGit } = jest.requireActual('../Git'); -// Store mock package.json contents for testing excludeVersionOnlyChanges -const mockPackageJsonContents: Map = new Map(); +// Mock function for getBlobContentAsync to be customized in each test +const mockGetBlobContentAsync = jest.fn, [{ blobSpec: string; repositoryRoot: string }]>(); /** Mock Git to test `getChangedProjectsAsync` */ jest.mock('../Git', () => { @@ -71,9 +69,7 @@ jest.mock('../Git', () => { return 'merge-base-sha'; } public async getBlobContentAsync(opts: { blobSpec: string; repositoryRoot: string }): Promise { - // Check if we have a mock for this blob spec - const content = mockPackageJsonContents.get(opts.blobSpec); - return content || ''; + return mockGetBlobContentAsync(opts); } } }; @@ -130,8 +126,8 @@ import type { describe(ProjectChangeAnalyzer.name, () => { beforeEach(() => { mockSnapshot.mockClear(); - mockPackageJsonContents.clear(); - mockRepoChanges = new Map(); + mockGetBlobContentAsync.mockClear(); + mockGetRepoChanges.mockClear(); }); describe(ProjectChangeAnalyzer.prototype._tryGetSnapshotProviderAsync.name, () => { @@ -166,28 +162,30 @@ describe(ProjectChangeAnalyzer.name, () => { describe(ProjectChangeAnalyzer.prototype.getChangedProjectsAsync.name, () => { it('Subspaces detects external changes', async () => { // Set up mock repo changes for this test - mockRepoChanges = new Map([ - [ - // Test subspace lockfile change detection - 'common/config/subspaces/project-change-analyzer-test-subspace/pnpm-lock.yaml', - { - mode: 'modified', - newhash: 'newhash', - oldhash: 'oldhash', - status: 'M' - } - ], - [ - // Test lockfile deletion detection - 'common/config/subspaces/default/pnpm-lock.yaml', - { - mode: 'deleted', - newhash: '', - oldhash: 'oldhash', - status: 'D' - } - ] - ]); + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + // Test subspace lockfile change detection + 'common/config/subspaces/project-change-analyzer-test-subspace/pnpm-lock.yaml', + { + mode: 'modified', + newhash: 'newhash', + oldhash: 'oldhash', + status: 'M' + } + ], + [ + // Test lockfile deletion detection + 'common/config/subspaces/default/pnpm-lock.yaml', + { + mode: 'deleted', + newhash: '', + oldhash: 'oldhash', + status: 'D' + } + ] + ]) + ); const rootDir: string = resolve(__dirname, 'repoWithSubspaces'); const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( @@ -252,20 +250,22 @@ describe(ProjectChangeAnalyzer.name, () => { ); // Set up mock repo changes - only package.json changed for project 'a' - mockRepoChanges = new Map([ - [ - 'a/package.json', - { - mode: 'modified', - newhash: 'newhash1', - oldhash: 'oldhash1', - status: 'M' - } - ] - ]); + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'a/package.json', + { + mode: 'modified', + newhash: 'newhash1', + oldhash: 'oldhash1', + status: 'M' + } + ] + ]) + ); // Mock the blob content to return old package.json - mockPackageJsonContents.set('oldhash1', oldPackageJsonContent); + mockGetBlobContentAsync.mockResolvedValue(oldPackageJsonContent); // Mock FileSystem.readFileAsync to return new package.json const FileSystem = require('@rushstack/node-core-library').FileSystem; @@ -338,20 +338,22 @@ describe(ProjectChangeAnalyzer.name, () => { ); // Set up mock repo changes - only package.json changed for project 'b' - mockRepoChanges = new Map([ - [ - 'b/package.json', - { - mode: 'modified', - newhash: 'newhash2', - oldhash: 'oldhash2', - status: 'M' - } - ] - ]); + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'b/package.json', + { + mode: 'modified', + newhash: 'newhash2', + oldhash: 'oldhash2', + status: 'M' + } + ] + ]) + ); // Mock the blob content to return old package.json - mockPackageJsonContents.set('oldhash2', oldPackageJsonContent); + mockGetBlobContentAsync.mockResolvedValue(oldPackageJsonContent); // Mock FileSystem.readFileAsync to return new package.json const FileSystem = require('@rushstack/node-core-library').FileSystem; From ab2c18767ab5d978e6c377255cbc317718287165 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 01:54:46 +0000 Subject: [PATCH 06/16] Optimize version-only change detection per review feedback - Move check into main loop to avoid race conditions - Use getBlobContentAsync for both old and new versions - Simplify JSON comparison without pretty printing - Check for 'M' status to exclude additions/deletions - Sort results by projectRelativeFolder for consistency - Update tests to mock getBlobContentAsync correctly Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 104 +++++++++--------- .../logic/test/ProjectChangeAnalyzer.test.ts | 36 ++---- 2 files changed, 67 insertions(+), 73 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 53ead883b0b..b7be0264298 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -131,43 +131,49 @@ export class ProjectChangeAnalyzer { ); if (filteredChanges.size > 0) { + // Check for version-only changes if excludeVersionOnlyChanges is enabled + if (excludeVersionOnlyChanges && filteredChanges.size === 1) { + const packageJsonPath: string = Path.convertToSlashes( + path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) + ); + const diffStatus: IFileDiffStatus | undefined = filteredChanges.get(packageJsonPath); + if (diffStatus) { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( + diffStatus, + repoRoot + ); + if (isVersionOnlyChange) { + return; // Skip adding this project + } + } + } changedProjects.add(project); } }, { concurrency: 10 } ); } else { - for (const [project, projectChanges] of changesByProject) { - if (projectChanges.size > 0) { - changedProjects.add(project); - } - } - } - - // Check for version-only changes if excludeVersionOnlyChanges is enabled - if (excludeVersionOnlyChanges) { - const projectsToCheck: RushConfigurationProject[] = Array.from(changedProjects); await Async.forEachAsync( - projectsToCheck, - async (project) => { - const projectChanges: Map | undefined = changesByProject.get(project); - if (projectChanges && projectChanges.size === 1) { - // Check if the only change is to package.json - const packageJsonPath: string = Path.convertToSlashes( - path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) - ); - const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); - if (diffStatus) { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( - project, - packageJsonPath, - diffStatus, - repoRoot + changesByProject, + async ([project, projectChanges]) => { + if (projectChanges.size > 0) { + // Check for version-only changes if excludeVersionOnlyChanges is enabled + if (excludeVersionOnlyChanges && projectChanges.size === 1) { + const packageJsonPath: string = Path.convertToSlashes( + path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) ); - if (isVersionOnlyChange) { - changedProjects.delete(project); + const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); + if (diffStatus) { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( + diffStatus, + repoRoot + ); + if (isVersionOnlyChange) { + return; // Skip adding this project + } } } + changedProjects.add(project); } }, { concurrency: 10 } @@ -256,7 +262,12 @@ export class ProjectChangeAnalyzer { }); } - return changedProjects; + // Sort the set by projectRelativeFolder to avoid race conditions in the results + const sortedChangedProjects: RushConfigurationProject[] = Array.from(changedProjects).sort((a, b) => + a.projectRelativeFolder.localeCompare(b.projectRelativeFolder) + ); + + return new Set(sortedChangedProjects); } protected getChangesByProject( @@ -443,22 +454,24 @@ export class ProjectChangeAnalyzer { * Checks if the only change to a package.json file is to the "version" field. * @internal */ - private async _isVersionOnlyChangeAsync( - project: RushConfigurationProject, - packageJsonPath: string, - diffStatus: IFileDiffStatus, - repoRoot: string - ): Promise { + private async _isVersionOnlyChangeAsync(diffStatus: IFileDiffStatus, repoRoot: string): Promise { try { + // Only check modified files, not additions or deletions + if (diffStatus.status !== 'M') { + return false; + } + // Get the old version of package.json from Git using the blob id from IFileDiffStatus const oldPackageJsonContent: string = await this._git.getBlobContentAsync({ blobSpec: diffStatus.oldhash, repositoryRoot: repoRoot }); - // Get the current version of package.json - const currentPackageJsonPath: string = path.join(repoRoot, packageJsonPath); - const currentPackageJsonContent: string = await FileSystem.readFileAsync(currentPackageJsonPath); + // Get the current version of package.json from Git (staged/committed version, not working tree) + const currentPackageJsonContent: string = await this._git.getBlobContentAsync({ + blobSpec: diffStatus.newhash, + repositoryRoot: repoRoot + }); // Parse both versions const oldPackageJson: Record = JSON.parse(oldPackageJsonContent); @@ -469,19 +482,12 @@ export class ProjectChangeAnalyzer { return false; } - // Create copies without the version field for comparison - const oldPackageJsonWithoutVersion: Record = { ...oldPackageJson }; - const currentPackageJsonWithoutVersion: Record = { ...currentPackageJson }; - delete oldPackageJsonWithoutVersion.version; - delete currentPackageJsonWithoutVersion.version; + // Remove the version field from both (no need to clone, these are fresh objects from JSON.parse) + oldPackageJson.version = undefined; + currentPackageJson.version = undefined; - // Check if the only difference is the version field - const oldVersionlessJson: string = JSON.stringify(oldPackageJsonWithoutVersion, null, 2); - const currentVersionlessJson: string = JSON.stringify(currentPackageJsonWithoutVersion, null, 2); - - return ( - oldVersionlessJson === currentVersionlessJson && oldPackageJson.version !== currentPackageJson.version - ); + // Compare the objects without the version field + return JSON.stringify(oldPackageJson) === JSON.stringify(currentPackageJson); } catch (error) { // If we can't read the file or parse it, assume it's not a version-only change return false; diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index 675b84cc4a8..7be90febfad 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -264,17 +264,14 @@ describe(ProjectChangeAnalyzer.name, () => { ]) ); - // Mock the blob content to return old package.json - mockGetBlobContentAsync.mockResolvedValue(oldPackageJsonContent); - - // Mock FileSystem.readFileAsync to return new package.json - const FileSystem = require('@rushstack/node-core-library').FileSystem; - const originalReadFileAsync = FileSystem.readFileAsync; - FileSystem.readFileAsync = jest.fn().mockImplementation((filePath: string) => { - if (filePath.endsWith('a/package.json')) { + // Mock the blob content to return different versions based on the hash + mockGetBlobContentAsync.mockImplementation((opts: { blobSpec: string; repositoryRoot: string }) => { + if (opts.blobSpec === 'oldhash1') { + return Promise.resolve(oldPackageJsonContent); + } else if (opts.blobSpec === 'newhash1') { return Promise.resolve(newPackageJsonContent); } - return originalReadFileAsync(filePath); + return Promise.resolve(''); }); const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); @@ -299,9 +296,6 @@ describe(ProjectChangeAnalyzer.name, () => { excludeVersionOnlyChanges: true }); expect(changedProjectsWithExclude.has(rushConfiguration.getProjectByName('a')!)).toBe(false); - - // Restore original function - FileSystem.readFileAsync = originalReadFileAsync; }); it('excludeVersionOnlyChanges does not exclude projects with non-version changes', async () => { @@ -352,17 +346,14 @@ describe(ProjectChangeAnalyzer.name, () => { ]) ); - // Mock the blob content to return old package.json - mockGetBlobContentAsync.mockResolvedValue(oldPackageJsonContent); - - // Mock FileSystem.readFileAsync to return new package.json - const FileSystem = require('@rushstack/node-core-library').FileSystem; - const originalReadFileAsync = FileSystem.readFileAsync; - FileSystem.readFileAsync = jest.fn().mockImplementation((filePath: string) => { - if (filePath.endsWith('b/package.json')) { + // Mock the blob content to return different versions based on the hash + mockGetBlobContentAsync.mockImplementation((opts: { blobSpec: string; repositoryRoot: string }) => { + if (opts.blobSpec === 'oldhash2') { + return Promise.resolve(oldPackageJsonContent); + } else if (opts.blobSpec === 'newhash2') { return Promise.resolve(newPackageJsonContent); } - return originalReadFileAsync(filePath); + return Promise.resolve(''); }); const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); @@ -378,9 +369,6 @@ describe(ProjectChangeAnalyzer.name, () => { excludeVersionOnlyChanges: true }); expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true); - - // Restore original function - FileSystem.readFileAsync = originalReadFileAsync; }); }); }); From 57fbb6bb35f07611f94cdf0e7eff43134938ce40 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 19:44:00 +0000 Subject: [PATCH 07/16] Address review feedback - improve type safety, deduplicate code, add tests - Add proper jest.MockedFunction types for mock functions - Add test for package.json + other file changes scenario - Deduplicate version-only check logic into helper function - Use Sort.sortBy for stable ordinal string comparison - Extract isPackageJsonVersionOnlyChange as exported helper - Add comprehensive unit tests for the helper function Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 120 +++++------ .../logic/test/ProjectChangeAnalyzer.test.ts | 189 +++++++++++++++++- 2 files changed, 247 insertions(+), 62 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index b7be0264298..f562424bfa5 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -6,7 +6,7 @@ import * as path from 'node:path'; import ignore, { type Ignore } from 'ignore'; import type { IReadonlyLookupByPath, LookupByPath } from '@rushstack/lookup-by-path'; -import { Path, FileSystem, Async, AlreadyReportedError } from '@rushstack/node-core-library'; +import { Path, FileSystem, Async, AlreadyReportedError, Sort } from '@rushstack/node-core-library'; import { getRepoChanges, getRepoRoot, @@ -118,6 +118,30 @@ export class ProjectChangeAnalyzer { > = this.getChangesByProject(lookup, changedFiles); const changedProjects: Set = new Set(); + + // Helper function to check and add project to changedProjects + const checkAndAddProject = async ( + project: RushConfigurationProject, + projectChanges: Map + ): Promise => { + if (projectChanges.size > 0) { + // Check for version-only changes if excludeVersionOnlyChanges is enabled + if (excludeVersionOnlyChanges && projectChanges.size === 1) { + const packageJsonPath: string = Path.convertToSlashes( + path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) + ); + const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); + if (diffStatus) { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); + if (isVersionOnlyChange) { + return; // Skip adding this project + } + } + } + changedProjects.add(project); + } + }; + if (enableFiltering) { // Reading rush-project.json may be problematic if, e.g. rush install has not yet occurred and rigs are in use await Async.forEachAsync( @@ -130,25 +154,7 @@ export class ProjectChangeAnalyzer { terminal ); - if (filteredChanges.size > 0) { - // Check for version-only changes if excludeVersionOnlyChanges is enabled - if (excludeVersionOnlyChanges && filteredChanges.size === 1) { - const packageJsonPath: string = Path.convertToSlashes( - path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) - ); - const diffStatus: IFileDiffStatus | undefined = filteredChanges.get(packageJsonPath); - if (diffStatus) { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( - diffStatus, - repoRoot - ); - if (isVersionOnlyChange) { - return; // Skip adding this project - } - } - } - changedProjects.add(project); - } + await checkAndAddProject(project, filteredChanges); }, { concurrency: 10 } ); @@ -156,25 +162,7 @@ export class ProjectChangeAnalyzer { await Async.forEachAsync( changesByProject, async ([project, projectChanges]) => { - if (projectChanges.size > 0) { - // Check for version-only changes if excludeVersionOnlyChanges is enabled - if (excludeVersionOnlyChanges && projectChanges.size === 1) { - const packageJsonPath: string = Path.convertToSlashes( - path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) - ); - const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); - if (diffStatus) { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync( - diffStatus, - repoRoot - ); - if (isVersionOnlyChange) { - return; // Skip adding this project - } - } - } - changedProjects.add(project); - } + await checkAndAddProject(project, projectChanges); }, { concurrency: 10 } ); @@ -263,9 +251,8 @@ export class ProjectChangeAnalyzer { } // Sort the set by projectRelativeFolder to avoid race conditions in the results - const sortedChangedProjects: RushConfigurationProject[] = Array.from(changedProjects).sort((a, b) => - a.projectRelativeFolder.localeCompare(b.projectRelativeFolder) - ); + const sortedChangedProjects: RushConfigurationProject[] = Array.from(changedProjects); + Sort.sortBy(sortedChangedProjects, (project) => project.projectRelativeFolder); return new Set(sortedChangedProjects); } @@ -473,21 +460,7 @@ export class ProjectChangeAnalyzer { repositoryRoot: repoRoot }); - // Parse both versions - const oldPackageJson: Record = JSON.parse(oldPackageJsonContent); - const currentPackageJson: Record = JSON.parse(currentPackageJsonContent); - - // Ensure both have a version field - if (!oldPackageJson.version || !currentPackageJson.version) { - return false; - } - - // Remove the version field from both (no need to clone, these are fresh objects from JSON.parse) - oldPackageJson.version = undefined; - currentPackageJson.version = undefined; - - // Compare the objects without the version field - return JSON.stringify(oldPackageJson) === JSON.stringify(currentPackageJson); + return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent); } catch (error) { // If we can't read the file or parse it, assume it's not a version-only change return false; @@ -612,3 +585,36 @@ async function getAdditionalFilesFromRushProjectConfigurationAsync( return additionalFilesFromRushProjectConfiguration; } + +/** + * Compares two package.json file contents and determines if the only difference is the "version" field. + * @param oldPackageJsonContent - The old package.json content as a string + * @param newPackageJsonContent - The new package.json content as a string + * @returns true if the only difference is the version field, false otherwise + * @public + */ +export function isPackageJsonVersionOnlyChange( + oldPackageJsonContent: string, + newPackageJsonContent: string +): boolean { + try { + // Parse both versions + const oldPackageJson: Record = JSON.parse(oldPackageJsonContent); + const newPackageJson: Record = JSON.parse(newPackageJsonContent); + + // Ensure both have a version field + if (!oldPackageJson.version || !newPackageJson.version) { + return false; + } + + // Remove the version field from both (no need to clone, these are fresh objects from JSON.parse) + oldPackageJson.version = undefined; + newPackageJson.version = undefined; + + // Compare the objects without the version field + return JSON.stringify(oldPackageJson) === JSON.stringify(newPackageJson); + } catch (error) { + // If we can't parse the JSON, assume it's not a version-only change + return false; + } +} diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index 7be90febfad..2fb8bdda8fe 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -23,7 +23,8 @@ const mockHashes: Map = new Map([ ]); // Mock function for customizing repo changes in each test -const mockGetRepoChanges = jest.fn, []>(); +const mockGetRepoChanges: jest.MockedFunction = + jest.fn(); jest.mock(`@rushstack/package-deps-hash`, () => { return { @@ -47,8 +48,12 @@ jest.mock(`@rushstack/package-deps-hash`, () => { hashFilesAsync(rootDirectory: string, filePaths: Iterable): ReadonlyMap { return new Map(Array.from(filePaths, (filePath: string) => [filePath, filePath])); }, - getRepoChanges(): Map { - return mockGetRepoChanges(); + getRepoChanges( + currentWorkingDirectory: string, + revision?: string, + gitPath?: string + ): Map { + return mockGetRepoChanges(currentWorkingDirectory, revision, gitPath); } }; }); @@ -56,7 +61,9 @@ jest.mock(`@rushstack/package-deps-hash`, () => { const { Git: OriginalGit } = jest.requireActual('../Git'); // Mock function for getBlobContentAsync to be customized in each test -const mockGetBlobContentAsync = jest.fn, [{ blobSpec: string; repositoryRoot: string }]>(); +const mockGetBlobContentAsync: jest.MockedFunction< + typeof import('../Git').Git.prototype.getBlobContentAsync +> = jest.fn(); /** Mock Git to test `getChangedProjectsAsync` */ jest.mock('../Git', () => { @@ -110,7 +117,7 @@ import { resolve } from 'node:path'; import type { IDetailedRepoState, IFileDiffStatus } from '@rushstack/package-deps-hash'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; -import { ProjectChangeAnalyzer } from '../ProjectChangeAnalyzer'; +import { ProjectChangeAnalyzer, isPackageJsonVersionOnlyChange } from '../ProjectChangeAnalyzer'; import { RushConfiguration } from '../../api/RushConfiguration'; import type { IInputsSnapshot, @@ -370,6 +377,178 @@ describe(ProjectChangeAnalyzer.name, () => { }); expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true); }); + + it('excludeVersionOnlyChanges does not exclude projects when package.json and other files changed', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + // Mock package.json with only version change + const oldPackageJsonContent = JSON.stringify( + { + name: 'c', + version: '1.0.0', + description: 'Test package', + dependencies: { + a: '1.0.0' + } + }, + null, + 2 + ); + + const newPackageJsonContent = JSON.stringify( + { + name: 'c', + version: '1.0.1', + description: 'Test package', + dependencies: { + a: '1.0.0' + } + }, + null, + 2 + ); + + // Set up mock repo changes - package.json AND another file changed for project 'c' + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'c/package.json', + { + mode: 'modified', + newhash: 'newhash3', + oldhash: 'oldhash3', + status: 'M' + } + ], + [ + 'c/src/index.ts', + { + mode: 'modified', + newhash: 'newhash4', + oldhash: 'oldhash4', + status: 'M' + } + ] + ]) + ); + + // Mock the blob content to return different versions based on the hash + mockGetBlobContentAsync.mockImplementation((opts: { blobSpec: string; repositoryRoot: string }) => { + if (opts.blobSpec === 'oldhash3') { + return Promise.resolve(oldPackageJsonContent); + } else if (opts.blobSpec === 'newhash3') { + return Promise.resolve(newPackageJsonContent); + } + return Promise.resolve(''); + }); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true); + const terminal: Terminal = new Terminal(terminalProvider); + + // Test with excludeVersionOnlyChanges - project should still be detected as changed because multiple files changed + const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + expect(changedProjects.has(rushConfiguration.getProjectByName('c')!)).toBe(true); + }); + }); + + describe('isPackageJsonVersionOnlyChange', () => { + it('returns true when only version field changed', () => { + const oldContent = JSON.stringify({ + name: 'test-package', + version: '1.0.0', + description: 'Test package', + dependencies: { foo: '1.0.0' } + }); + const newContent = JSON.stringify({ + name: 'test-package', + version: '1.0.1', + description: 'Test package', + dependencies: { foo: '1.0.0' } + }); + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(true); + }); + + it('returns false when other fields changed', () => { + const oldContent = JSON.stringify({ + name: 'test-package', + version: '1.0.0', + description: 'Test package', + dependencies: { foo: '1.0.0' } + }); + const newContent = JSON.stringify({ + name: 'test-package', + version: '1.0.1', + description: 'Test package', + dependencies: { foo: '1.0.1' } + }); + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(false); + }); + + it('returns false when version field is missing in old content', () => { + const oldContent = JSON.stringify({ + name: 'test-package', + description: 'Test package' + }); + const newContent = JSON.stringify({ + name: 'test-package', + version: '1.0.1', + description: 'Test package' + }); + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(false); + }); + + it('returns false when version field is missing in new content', () => { + const oldContent = JSON.stringify({ + name: 'test-package', + version: '1.0.0', + description: 'Test package' + }); + const newContent = JSON.stringify({ + name: 'test-package', + description: 'Test package' + }); + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(false); + }); + + it('returns false when JSON is invalid', () => { + const oldContent = 'invalid json'; + const newContent = '{ "name": "test" }'; + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(false); + }); + + it('returns true even with whitespace differences', () => { + const oldContent = JSON.stringify( + { + name: 'test-package', + version: '1.0.0', + description: 'Test package' + }, + null, + 2 + ); + const newContent = JSON.stringify({ + name: 'test-package', + version: '1.0.1', + description: 'Test package' + }); + + expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(true); + }); }); }); From 0138de39860ab68dede6ed7852c532c80ef2df90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:12:12 +0000 Subject: [PATCH 08/16] Add Rush change file for excludeVersionOnlyChanges behavior Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- ...exclude-version-only-changes_2026-02-04-23-11.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json diff --git a/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json b/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json new file mode 100644 index 00000000000..519d62ca270 --- /dev/null +++ b/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix `rush change --verify` to ignore version-only changes in package.json files, preventing false positives after `rush version --bump` updates package versions.", + "type": "none" + } + ], + "packageName": "@microsoft/rush", + "email": "copilot@github.com" +} From 3691d49fd169e955d98b4fda6569441fe0d067c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:41:25 +0000 Subject: [PATCH 09/16] Extend excludeVersionOnlyChanges to ignore CHANGELOG files - Filter out CHANGELOG.md and CHANGELOG.json changes - Update logic to check all changed files, not just single file - Add tests for CHANGELOG file exclusion scenarios - Update documentation to reflect expanded behavior Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- ...version-only-changes_2026-02-04-23-11.json | 2 +- .../src/logic/ProjectChangeAnalyzer.ts | 47 +++++-- .../logic/test/ProjectChangeAnalyzer.test.ts | 124 ++++++++++++++++++ 3 files changed, 159 insertions(+), 14 deletions(-) diff --git a/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json b/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json index 519d62ca270..07495336612 100644 --- a/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json +++ b/common/changes/@microsoft/rush/exclude-version-only-changes_2026-02-04-23-11.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Fix `rush change --verify` to ignore version-only changes in package.json files, preventing false positives after `rush version --bump` updates package versions.", + "comment": "Fix `rush change --verify` to ignore version-only changes in package.json files and changes to CHANGELOG.md and CHANGELOG.json files, preventing false positives after `rush version --bump` updates package versions and changelogs.", "type": "none" } ], diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index f562424bfa5..808fb73bbb2 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -52,9 +52,12 @@ export interface IGetChangedProjectsOptions { enableFiltering: boolean; /** - * If set to `true`, for each project, if the only change to the project is to its `package.json`, - * diffs the file against the base version to see if the only change is to the value of the "version" field, - * and if it is, ignores the change to the project. + * If set to `true`, excludes projects where the only changes are: + * - A version-only change to `package.json` (only the "version" field differs) + * - Changes to `CHANGELOG.md` and/or `CHANGELOG.json` files + * + * This prevents `rush version --bump` from triggering `rush change --verify` to request change files + * for the version bumps and changelog updates it creates. */ excludeVersionOnlyChanges?: boolean; } @@ -126,19 +129,37 @@ export class ProjectChangeAnalyzer { ): Promise => { if (projectChanges.size > 0) { // Check for version-only changes if excludeVersionOnlyChanges is enabled - if (excludeVersionOnlyChanges && projectChanges.size === 1) { - const packageJsonPath: string = Path.convertToSlashes( - path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) - ); - const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); - if (diffStatus) { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); - if (isVersionOnlyChange) { - return; // Skip adding this project + if (excludeVersionOnlyChanges) { + // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json + const filteredProjectChanges: Map = new Map(); + + for (const [filePath, diffStatus] of projectChanges) { + const fileName: string = path.basename(filePath); + + // Skip CHANGELOG.md and CHANGELOG.json files + if (fileName === 'CHANGELOG.md' || fileName === 'CHANGELOG.json') { + continue; } + + // Check if this is package.json with version-only changes + if (fileName === 'package.json') { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); + if (isVersionOnlyChange) { + continue; // Skip version-only package.json changes + } + } + + // Add all other changes to filtered list + filteredProjectChanges.set(filePath, diffStatus); + } + + // Only add project if there are non-excluded changes + if (filteredProjectChanges.size > 0) { + changedProjects.add(project); } + } else { + changedProjects.add(project); } - changedProjects.add(project); } }; diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index 2fb8bdda8fe..7be396977c9 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -459,6 +459,130 @@ describe(ProjectChangeAnalyzer.name, () => { }); expect(changedProjects.has(rushConfiguration.getProjectByName('c')!)).toBe(true); }); + + it('excludeVersionOnlyChanges ignores CHANGELOG.md and CHANGELOG.json files', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + // Mock package.json with only version change + const oldPackageJsonContent = JSON.stringify({ + name: 'd', + version: '1.0.0', + description: 'Test package' + }); + + const newPackageJsonContent = JSON.stringify({ + name: 'd', + version: '1.0.1', + description: 'Test package' + }); + + // Set up mock repo changes - package.json (version only), CHANGELOG.md, and CHANGELOG.json changed + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'd/package.json', + { + mode: 'modified', + newhash: 'newhash4', + oldhash: 'oldhash4', + status: 'M' + } + ], + [ + 'd/CHANGELOG.md', + { + mode: 'modified', + newhash: 'newhash5', + oldhash: 'oldhash5', + status: 'M' + } + ], + [ + 'd/CHANGELOG.json', + { + mode: 'modified', + newhash: 'newhash6', + oldhash: 'oldhash6', + status: 'M' + } + ] + ]) + ); + + // Mock the blob content to return different versions based on the hash + mockGetBlobContentAsync.mockImplementation((opts: { blobSpec: string; repositoryRoot: string }) => { + if (opts.blobSpec === 'oldhash4') { + return Promise.resolve(oldPackageJsonContent); + } else if (opts.blobSpec === 'newhash4') { + return Promise.resolve(newPackageJsonContent); + } + return Promise.resolve(''); + }); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true); + const terminal: Terminal = new Terminal(terminalProvider); + + // Test with excludeVersionOnlyChanges - project should NOT be detected as changed + // because only version-only package.json and CHANGELOG files changed + const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + expect(changedProjects.has(rushConfiguration.getProjectByName('d')!)).toBe(false); + }); + + it('excludeVersionOnlyChanges does not ignore projects with CHANGELOG and other substantive changes', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + // Set up mock repo changes - CHANGELOG.md and src file changed + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'e/CHANGELOG.md', + { + mode: 'modified', + newhash: 'newhash7', + oldhash: 'oldhash7', + status: 'M' + } + ], + [ + 'e/src/index.ts', + { + mode: 'modified', + newhash: 'newhash8', + oldhash: 'oldhash8', + status: 'M' + } + ] + ]) + ); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminalProvider: StringBufferTerminalProvider = new StringBufferTerminalProvider(true); + const terminal: Terminal = new Terminal(terminalProvider); + + // Test with excludeVersionOnlyChanges - project should be detected as changed + // because there's a substantive change in addition to CHANGELOG + const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + expect(changedProjects.has(rushConfiguration.getProjectByName('e')!)).toBe(true); + }); }); describe('isPackageJsonVersionOnlyChange', () => { From b51485634b6b38dc345d55db814cab757bebbd35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:58:12 +0000 Subject: [PATCH 10/16] Simplify and improve excludeVersionOnlyChanges logic - Add early returns to reduce nesting depth - Use lookup.findLongestPrefixMatch for accurate path matching - Only exclude /CHANGELOG.md, /CHANGELOG.json, /package.json at project root - Add type annotation for IPrefixMatch - Simplify control flow with immediate returns Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 808fb73bbb2..f069c012ac4 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -5,7 +5,7 @@ import * as path from 'node:path'; import ignore, { type Ignore } from 'ignore'; -import type { IReadonlyLookupByPath, LookupByPath } from '@rushstack/lookup-by-path'; +import type { IReadonlyLookupByPath, LookupByPath, IPrefixMatch } from '@rushstack/lookup-by-path'; import { Path, FileSystem, Async, AlreadyReportedError, Sort } from '@rushstack/node-core-library'; import { getRepoChanges, @@ -127,39 +127,46 @@ export class ProjectChangeAnalyzer { project: RushConfigurationProject, projectChanges: Map ): Promise => { - if (projectChanges.size > 0) { - // Check for version-only changes if excludeVersionOnlyChanges is enabled - if (excludeVersionOnlyChanges) { - // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json - const filteredProjectChanges: Map = new Map(); - - for (const [filePath, diffStatus] of projectChanges) { - const fileName: string = path.basename(filePath); - - // Skip CHANGELOG.md and CHANGELOG.json files - if (fileName === 'CHANGELOG.md' || fileName === 'CHANGELOG.json') { - continue; - } + // Early return if no changes + if (projectChanges.size === 0) { + return; + } - // Check if this is package.json with version-only changes - if (fileName === 'package.json') { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); - if (isVersionOnlyChange) { - continue; // Skip version-only package.json changes - } - } + // If excludeVersionOnlyChanges is not enabled, add the project + if (!excludeVersionOnlyChanges) { + changedProjects.add(project); + return; + } - // Add all other changes to filtered list - filteredProjectChanges.set(filePath, diffStatus); - } + // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json + for (const [filePath, diffStatus] of projectChanges) { + // Use lookup to find the project-relative path + const match: IPrefixMatch | undefined = + lookup.findLongestPrefixMatch(filePath); + if (!match) { + // File is outside any project, consider it a change + changedProjects.add(project); + return; + } + + const projectRelativePath: string = filePath.slice(match.index); + + // Skip CHANGELOG.md and CHANGELOG.json files at project root + if (projectRelativePath === '/CHANGELOG.md' || projectRelativePath === '/CHANGELOG.json') { + continue; + } - // Only add project if there are non-excluded changes - if (filteredProjectChanges.size > 0) { - changedProjects.add(project); + // Check if this is package.json at project root with version-only changes + if (projectRelativePath === '/package.json') { + const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); + if (isVersionOnlyChange) { + continue; // Skip version-only package.json changes } - } else { - changedProjects.add(project); } + + // Found a non-excluded change, add the project + changedProjects.add(project); + return; } }; From 5c3169d2d888ea4163dcd6b27f1afa596c94f471 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 00:57:00 +0000 Subject: [PATCH 11/16] Address review feedback: update comment and type annotation - Update comment to clarify unreachable code condition - Use simpler type annotation for package.json parsing Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index f069c012ac4..bec7b6b8fb1 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -144,7 +144,7 @@ export class ProjectChangeAnalyzer { const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); if (!match) { - // File is outside any project, consider it a change + // This should be unreachable as projectChanges contains files where match.value === project changedProjects.add(project); return; } @@ -626,9 +626,9 @@ export function isPackageJsonVersionOnlyChange( newPackageJsonContent: string ): boolean { try { - // Parse both versions - const oldPackageJson: Record = JSON.parse(oldPackageJsonContent); - const newPackageJson: Record = JSON.parse(newPackageJsonContent); + // Parse both versions - use specific type since we only care about version field + const oldPackageJson: { version?: string } = JSON.parse(oldPackageJsonContent); + const newPackageJson: { version?: string } = JSON.parse(newPackageJsonContent); // Ensure both have a version field if (!oldPackageJson.version || !newPackageJson.version) { From 4ac21b94c73d5a185fc3da9e6f05790766e888df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 00:07:42 +0000 Subject: [PATCH 12/16] Hoist checkAndAddProject to module scope per review feedback - Convert arrow function to module-level checkAndAddProjectAsync - Make all closure variables explicit parameters - Extract isVersionOnlyChangeHelperAsync to module scope - Remove old _isVersionOnlyChangeAsync instance method Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 186 ++++++++++-------- 1 file changed, 106 insertions(+), 80 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index bec7b6b8fb1..30494b3af9d 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -122,54 +122,6 @@ export class ProjectChangeAnalyzer { const changedProjects: Set = new Set(); - // Helper function to check and add project to changedProjects - const checkAndAddProject = async ( - project: RushConfigurationProject, - projectChanges: Map - ): Promise => { - // Early return if no changes - if (projectChanges.size === 0) { - return; - } - - // If excludeVersionOnlyChanges is not enabled, add the project - if (!excludeVersionOnlyChanges) { - changedProjects.add(project); - return; - } - - // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json - for (const [filePath, diffStatus] of projectChanges) { - // Use lookup to find the project-relative path - const match: IPrefixMatch | undefined = - lookup.findLongestPrefixMatch(filePath); - if (!match) { - // This should be unreachable as projectChanges contains files where match.value === project - changedProjects.add(project); - return; - } - - const projectRelativePath: string = filePath.slice(match.index); - - // Skip CHANGELOG.md and CHANGELOG.json files at project root - if (projectRelativePath === '/CHANGELOG.md' || projectRelativePath === '/CHANGELOG.json') { - continue; - } - - // Check if this is package.json at project root with version-only changes - if (projectRelativePath === '/package.json') { - const isVersionOnlyChange: boolean = await this._isVersionOnlyChangeAsync(diffStatus, repoRoot); - if (isVersionOnlyChange) { - continue; // Skip version-only package.json changes - } - } - - // Found a non-excluded change, add the project - changedProjects.add(project); - return; - } - }; - if (enableFiltering) { // Reading rush-project.json may be problematic if, e.g. rush install has not yet occurred and rigs are in use await Async.forEachAsync( @@ -182,7 +134,15 @@ export class ProjectChangeAnalyzer { terminal ); - await checkAndAddProject(project, filteredChanges); + await checkAndAddProjectAsync( + project, + filteredChanges, + excludeVersionOnlyChanges, + lookup, + repoRoot, + this._git, + changedProjects + ); }, { concurrency: 10 } ); @@ -190,7 +150,15 @@ export class ProjectChangeAnalyzer { await Async.forEachAsync( changesByProject, async ([project, projectChanges]) => { - await checkAndAddProject(project, projectChanges); + await checkAndAddProjectAsync( + project, + projectChanges, + excludeVersionOnlyChanges, + lookup, + repoRoot, + this._git, + changedProjects + ); }, { concurrency: 10 } ); @@ -465,36 +433,6 @@ export class ProjectChangeAnalyzer { } } - /** - * Checks if the only change to a package.json file is to the "version" field. - * @internal - */ - private async _isVersionOnlyChangeAsync(diffStatus: IFileDiffStatus, repoRoot: string): Promise { - try { - // Only check modified files, not additions or deletions - if (diffStatus.status !== 'M') { - return false; - } - - // Get the old version of package.json from Git using the blob id from IFileDiffStatus - const oldPackageJsonContent: string = await this._git.getBlobContentAsync({ - blobSpec: diffStatus.oldhash, - repositoryRoot: repoRoot - }); - - // Get the current version of package.json from Git (staged/committed version, not working tree) - const currentPackageJsonContent: string = await this._git.getBlobContentAsync({ - blobSpec: diffStatus.newhash, - repositoryRoot: repoRoot - }); - - return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent); - } catch (error) { - // If we can't read the file or parse it, assume it's not a version-only change - return false; - } - } - /** * @internal */ @@ -541,6 +479,94 @@ export class ProjectChangeAnalyzer { } } +/** + * Helper function to check if a project should be added to the changed projects set, + * optionally filtering out version-only and changelog changes. + */ +async function checkAndAddProjectAsync( + project: RushConfigurationProject, + projectChanges: Map, + excludeVersionOnlyChanges: boolean | undefined, + lookup: LookupByPath, + repoRoot: string, + git: Git, + changedProjects: Set +): Promise { + // Early return if no changes + if (projectChanges.size === 0) { + return; + } + + // If excludeVersionOnlyChanges is not enabled, add the project + if (!excludeVersionOnlyChanges) { + changedProjects.add(project); + return; + } + + // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json + for (const [filePath, diffStatus] of projectChanges) { + // Use lookup to find the project-relative path + const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); + if (!match) { + // This should be unreachable as projectChanges contains files where match.value === project + changedProjects.add(project); + return; + } + + const projectRelativePath: string = filePath.slice(match.index); + + // Skip CHANGELOG.md and CHANGELOG.json files at project root + if (projectRelativePath === '/CHANGELOG.md' || projectRelativePath === '/CHANGELOG.json') { + continue; + } + + // Check if this is package.json at project root with version-only changes + if (projectRelativePath === '/package.json') { + const isVersionOnlyChange: boolean = await isVersionOnlyChangeHelperAsync(diffStatus, repoRoot, git); + if (isVersionOnlyChange) { + continue; // Skip version-only package.json changes + } + } + + // Found a non-excluded change, add the project + changedProjects.add(project); + return; + } +} + +/** + * Helper function to check if a diff represents a version-only change to package.json. + */ +async function isVersionOnlyChangeHelperAsync( + diffStatus: IFileDiffStatus, + repoRoot: string, + git: Git +): Promise { + try { + // Only check modified files, not additions or deletions + if (diffStatus.status !== 'M') { + return false; + } + + // Get the old version of package.json from Git using the blob id from IFileDiffStatus + const oldPackageJsonContent: string = await git.getBlobContentAsync({ + blobSpec: diffStatus.oldhash, + repositoryRoot: repoRoot + }); + + // Get the current version of package.json from Git (staged/committed version, not working tree) + const currentPackageJsonContent: string = await git.getBlobContentAsync({ + blobSpec: diffStatus.newhash, + repositoryRoot: repoRoot + }); + + return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent); + } catch (error) { + // If we can't read the file or parse it, assume it's not a version-only change + return false; + } +} + interface IAdditionalGlob { project: RushConfigurationProject; operationName: string; From a1acc508cabdea6c216b7edce9992d0cac320089 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 00:25:18 +0000 Subject: [PATCH 13/16] Refactor for better readability per review feedback - Rename shouldIncludeProjectAsync (return bool vs void) - Pass isDiffVersionOnlyChange function as parameter - Remove "Helper" from isVersionOnlyChangeAsync name - Caller adds to changedProjects based on return value Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 30494b3af9d..bec95f68e78 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -122,6 +122,11 @@ export class ProjectChangeAnalyzer { const changedProjects: Set = new Set(); + // Arrow function that references this._git for version-only change checking + const isDiffVersionOnlyChange = async (diffStatus: IFileDiffStatus): Promise => { + return await isVersionOnlyChangeAsync(diffStatus, repoRoot, this._git); + }; + if (enableFiltering) { // Reading rush-project.json may be problematic if, e.g. rush install has not yet occurred and rigs are in use await Async.forEachAsync( @@ -134,15 +139,16 @@ export class ProjectChangeAnalyzer { terminal ); - await checkAndAddProjectAsync( + const shouldInclude: boolean = await shouldIncludeProjectAsync( project, filteredChanges, excludeVersionOnlyChanges, lookup, - repoRoot, - this._git, - changedProjects + isDiffVersionOnlyChange ); + if (shouldInclude) { + changedProjects.add(project); + } }, { concurrency: 10 } ); @@ -150,15 +156,16 @@ export class ProjectChangeAnalyzer { await Async.forEachAsync( changesByProject, async ([project, projectChanges]) => { - await checkAndAddProjectAsync( + const shouldInclude: boolean = await shouldIncludeProjectAsync( project, projectChanges, excludeVersionOnlyChanges, lookup, - repoRoot, - this._git, - changedProjects + isDiffVersionOnlyChange ); + if (shouldInclude) { + changedProjects.add(project); + } }, { concurrency: 10 } ); @@ -480,27 +487,24 @@ export class ProjectChangeAnalyzer { } /** - * Helper function to check if a project should be added to the changed projects set, - * optionally filtering out version-only and changelog changes. + * Checks if a project should be considered changed, optionally filtering out version-only and changelog changes. + * @returns true if the project has substantive changes, false if changes should be excluded */ -async function checkAndAddProjectAsync( +async function shouldIncludeProjectAsync( project: RushConfigurationProject, projectChanges: Map, excludeVersionOnlyChanges: boolean | undefined, lookup: LookupByPath, - repoRoot: string, - git: Git, - changedProjects: Set -): Promise { - // Early return if no changes + isDiffVersionOnlyChange: (diffStatus: IFileDiffStatus) => Promise +): Promise { + // No changes means don't include if (projectChanges.size === 0) { - return; + return false; } - // If excludeVersionOnlyChanges is not enabled, add the project + // If excludeVersionOnlyChanges is not enabled, include the project if (!excludeVersionOnlyChanges) { - changedProjects.add(project); - return; + return true; } // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json @@ -509,8 +513,7 @@ async function checkAndAddProjectAsync( const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); if (!match) { // This should be unreachable as projectChanges contains files where match.value === project - changedProjects.add(project); - return; + return true; } const projectRelativePath: string = filePath.slice(match.index); @@ -522,22 +525,24 @@ async function checkAndAddProjectAsync( // Check if this is package.json at project root with version-only changes if (projectRelativePath === '/package.json') { - const isVersionOnlyChange: boolean = await isVersionOnlyChangeHelperAsync(diffStatus, repoRoot, git); + const isVersionOnlyChange: boolean = await isDiffVersionOnlyChange(diffStatus); if (isVersionOnlyChange) { continue; // Skip version-only package.json changes } } - // Found a non-excluded change, add the project - changedProjects.add(project); - return; + // Found a non-excluded change + return true; } + + // All changes were excluded + return false; } /** - * Helper function to check if a diff represents a version-only change to package.json. + * Checks if a diff represents a version-only change to package.json. */ -async function isVersionOnlyChangeHelperAsync( +async function isVersionOnlyChangeAsync( diffStatus: IFileDiffStatus, repoRoot: string, git: Git From 28975344ade56caeac1c5f6b7e34b9c5eb88544d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 02:00:32 +0000 Subject: [PATCH 14/16] Deduplicate and inline logic per review feedback - Deduplicate filtering branches using conditional expression - Inline shouldIncludeProjectAsync logic into main loop - Remove outer if statement for cleaner code flow Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 139 ++++++------------ 1 file changed, 47 insertions(+), 92 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index bec95f68e78..1966a55cd3b 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -127,49 +127,57 @@ export class ProjectChangeAnalyzer { return await isVersionOnlyChangeAsync(diffStatus, repoRoot, this._git); }; - if (enableFiltering) { - // Reading rush-project.json may be problematic if, e.g. rush install has not yet occurred and rigs are in use - await Async.forEachAsync( - changesByProject, - async ([project, projectChanges]) => { - const filteredChanges: Map = await this._filterProjectDataAsync( - project, - projectChanges, - repoRoot, - terminal - ); + await Async.forEachAsync( + changesByProject, + async ([project, projectChanges]) => { + const filteredChanges: Map = enableFiltering + ? await this._filterProjectDataAsync(project, projectChanges, repoRoot, terminal) + : projectChanges; + + // Skip if no changes + if (filteredChanges.size === 0) { + return; + } - const shouldInclude: boolean = await shouldIncludeProjectAsync( - project, - filteredChanges, - excludeVersionOnlyChanges, - lookup, - isDiffVersionOnlyChange - ); - if (shouldInclude) { + // If excludeVersionOnlyChanges is not enabled, include the project + if (!excludeVersionOnlyChanges) { + changedProjects.add(project); + return; + } + + // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json + for (const [filePath, diffStatus] of filteredChanges) { + // Use lookup to find the project-relative path + const match: IPrefixMatch | undefined = + lookup.findLongestPrefixMatch(filePath); + if (!match) { + // This should be unreachable as projectChanges contains files where match.value === project changedProjects.add(project); + return; } - }, - { concurrency: 10 } - ); - } else { - await Async.forEachAsync( - changesByProject, - async ([project, projectChanges]) => { - const shouldInclude: boolean = await shouldIncludeProjectAsync( - project, - projectChanges, - excludeVersionOnlyChanges, - lookup, - isDiffVersionOnlyChange - ); - if (shouldInclude) { - changedProjects.add(project); + + const projectRelativePath: string = filePath.slice(match.index); + + // Skip CHANGELOG.md and CHANGELOG.json files at project root + if (projectRelativePath === '/CHANGELOG.md' || projectRelativePath === '/CHANGELOG.json') { + continue; } - }, - { concurrency: 10 } - ); - } + + // Check if this is package.json at project root with version-only changes + if (projectRelativePath === '/package.json') { + const isVersionOnlyChange: boolean = await isDiffVersionOnlyChange(diffStatus); + if (isVersionOnlyChange) { + continue; // Skip version-only package.json changes + } + } + + // Found a non-excluded change + changedProjects.add(project); + return; + } + }, + { concurrency: 10 } + ); // External dependency changes are not allowed to be filtered, so add these after filtering if (includeExternalDependencies) { @@ -486,59 +494,6 @@ export class ProjectChangeAnalyzer { } } -/** - * Checks if a project should be considered changed, optionally filtering out version-only and changelog changes. - * @returns true if the project has substantive changes, false if changes should be excluded - */ -async function shouldIncludeProjectAsync( - project: RushConfigurationProject, - projectChanges: Map, - excludeVersionOnlyChanges: boolean | undefined, - lookup: LookupByPath, - isDiffVersionOnlyChange: (diffStatus: IFileDiffStatus) => Promise -): Promise { - // No changes means don't include - if (projectChanges.size === 0) { - return false; - } - - // If excludeVersionOnlyChanges is not enabled, include the project - if (!excludeVersionOnlyChanges) { - return true; - } - - // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json - for (const [filePath, diffStatus] of projectChanges) { - // Use lookup to find the project-relative path - const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); - if (!match) { - // This should be unreachable as projectChanges contains files where match.value === project - return true; - } - - const projectRelativePath: string = filePath.slice(match.index); - - // Skip CHANGELOG.md and CHANGELOG.json files at project root - if (projectRelativePath === '/CHANGELOG.md' || projectRelativePath === '/CHANGELOG.json') { - continue; - } - - // Check if this is package.json at project root with version-only changes - if (projectRelativePath === '/package.json') { - const isVersionOnlyChange: boolean = await isDiffVersionOnlyChange(diffStatus); - if (isVersionOnlyChange) { - continue; // Skip version-only package.json changes - } - } - - // Found a non-excluded change - return true; - } - - // All changes were excluded - return false; -} - /** * Checks if a diff represents a version-only change to package.json. */ From f1a8058becdbb4bad76d78f5c0d82ac47d36687f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 02:14:39 +0000 Subject: [PATCH 15/16] Inline isDiffVersionOnlyChange function at call site - Remove isDiffVersionOnlyChange arrow function - Call isVersionOnlyChangeAsync directly with this._git Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 1966a55cd3b..3f7fd7a3da1 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -122,11 +122,6 @@ export class ProjectChangeAnalyzer { const changedProjects: Set = new Set(); - // Arrow function that references this._git for version-only change checking - const isDiffVersionOnlyChange = async (diffStatus: IFileDiffStatus): Promise => { - return await isVersionOnlyChangeAsync(diffStatus, repoRoot, this._git); - }; - await Async.forEachAsync( changesByProject, async ([project, projectChanges]) => { @@ -165,7 +160,11 @@ export class ProjectChangeAnalyzer { // Check if this is package.json at project root with version-only changes if (projectRelativePath === '/package.json') { - const isVersionOnlyChange: boolean = await isDiffVersionOnlyChange(diffStatus); + const isVersionOnlyChange: boolean = await isVersionOnlyChangeAsync( + diffStatus, + repoRoot, + this._git + ); if (isVersionOnlyChange) { continue; // Skip version-only package.json changes } From 8cf6d87e639c300054e37d3ac19002360230e385 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 19:33:48 +0000 Subject: [PATCH 16/16] Address final review feedback - Replace return with break in loop for better control flow - Remove @public annotation (not exported from package index) - Use Promise.all for parallel Git blob content retrieval Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --- .../src/logic/ProjectChangeAnalyzer.ts | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 3f7fd7a3da1..77ded049ca3 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -172,7 +172,7 @@ export class ProjectChangeAnalyzer { // Found a non-excluded change changedProjects.add(project); - return; + break; } }, { concurrency: 10 } @@ -507,17 +507,17 @@ async function isVersionOnlyChangeAsync( return false; } - // Get the old version of package.json from Git using the blob id from IFileDiffStatus - const oldPackageJsonContent: string = await git.getBlobContentAsync({ - blobSpec: diffStatus.oldhash, - repositoryRoot: repoRoot - }); - - // Get the current version of package.json from Git (staged/committed version, not working tree) - const currentPackageJsonContent: string = await git.getBlobContentAsync({ - blobSpec: diffStatus.newhash, - repositoryRoot: repoRoot - }); + // Get both versions of package.json from Git in parallel + const [oldPackageJsonContent, currentPackageJsonContent] = await Promise.all([ + git.getBlobContentAsync({ + blobSpec: diffStatus.oldhash, + repositoryRoot: repoRoot + }), + git.getBlobContentAsync({ + blobSpec: diffStatus.newhash, + repositoryRoot: repoRoot + }) + ]); return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent); } catch (error) { @@ -604,7 +604,6 @@ async function getAdditionalFilesFromRushProjectConfigurationAsync( * @param oldPackageJsonContent - The old package.json content as a string * @param newPackageJsonContent - The new package.json content as a string * @returns true if the only difference is the version field, false otherwise - * @public */ export function isPackageJsonVersionOnlyChange( oldPackageJsonContent: string,