diff --git a/common/changes/@microsoft/rush/add-workspace-cycle-detection_2026-07-23-01-32.json b/common/changes/@microsoft/rush/add-workspace-cycle-detection_2026-07-23-01-32.json new file mode 100644 index 0000000000..970da9764d --- /dev/null +++ b/common/changes/@microsoft/rush/add-workspace-cycle-detection_2026-07-23-01-32.json @@ -0,0 +1,9 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Add early validation to `rush install`/`rush update` that immediately fails with a meaningful error message if an undeclared cycle is detected among workspace packages, specifying the cycle path.", + "type": "minor" + } + ] +} diff --git a/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts new file mode 100644 index 0000000000..d204135d6e --- /dev/null +++ b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { AlreadyReportedError } from '@rushstack/node-core-library'; +import { Colorize, type ITerminal } from '@rushstack/terminal'; + +import type { RushConfiguration } from '../api/RushConfiguration'; +import type { RushConfigurationProject } from '../api/RushConfigurationProject'; +import { RushConstants } from './RushConstants'; + +/** + * Detects cycles in the workspace package dependency graph (i.e., cycles that are not + * broken by `decoupledLocalDependencies`) and reports them as errors. + * + * @remarks + * A cycle means that pnpm would be unable to install the workspace, so it is better to + * fail fast with a clear message rather than let pnpm produce a cryptic error. + * + * The fix is to refactor the code to eliminate the cycle, for example by extracting shared + * code into a new package that both projects can depend on, or by moving code from one project + * to another. `decoupledLocalDependencies` is intended only for the bootstrapping problem + * (e.g. the version of a compiler used to compile itself) and should not be used as a + * general escape hatch for cycles. + */ +export function detectAndReportWorkspaceCycles( + rushConfiguration: RushConfiguration, + terminal: ITerminal +): void { + const cycle: ReadonlyArray | undefined = _findWorkspaceCycle(rushConfiguration.projects); + + if (cycle !== undefined) { + terminal.writeLine(); + terminal.writeLine( + Colorize.red( + 'A cyclic dependency was detected among workspace packages:\n' + + ` ${cycle.join(' -> ')}\n\n` + + `To fix this, refactor the code to eliminate the cycle. For example, extract the shared ` + + `code into a new package that both projects can depend on, or move code from one project ` + + `to another so the dependency only goes in one direction.\n\n` + + `NOTE: The "decoupledLocalDependencies" setting in ${RushConstants.rushJsonFilename} is ` + + `intended only for the bootstrapping problem (for example, the version of a compiler used ` + + `to compile itself). It is not a general solution for cyclic dependencies.` + ) + ); + throw new AlreadyReportedError(); + } +} + +/** + * Finds one cycle in the workspace dependency graph, or returns `undefined` if there are none. + * + * Uses depth-first search with a "currently visiting" set for O(V + E) detection. + */ +export function _findWorkspaceCycle( + projects: ReadonlyArray +): ReadonlyArray | undefined { + // Nodes that have been fully explored (no cycles reachable from them) + const visited: Set = new Set(); + // Nodes currently on the DFS recursion stack + const visiting: Set = new Set(); + // The current DFS path (used to extract the cycle path when one is found) + const path: RushConfigurationProject[] = []; + + function dfs(node: RushConfigurationProject): ReadonlyArray | undefined { + if (visited.has(node)) { + return undefined; + } + if (visiting.has(node)) { + // We've found a back-edge — extract the cycle from the current path + const cycleStartIndex: number = path.indexOf(node); + return [ + ...path.slice(cycleStartIndex).map((p) => p.packageName), + node.packageName // append the closing node to make the cycle explicit + ]; + } + + visiting.add(node); + path.push(node); + + for (const dep of node.dependencyProjects) { + const cycle: ReadonlyArray | undefined = dfs(dep); + if (cycle !== undefined) { + return cycle; + } + } + + path.pop(); + visiting.delete(node); + visited.add(node); + return undefined; + } + + for (const project of projects) { + if (!visited.has(project)) { + const cycle: ReadonlyArray | undefined = dfs(project); + if (cycle !== undefined) { + return cycle; + } + } + } + + return undefined; +} diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts index a7016129ae..0642d77af0 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts @@ -60,6 +60,7 @@ import { ProjectImpactGraphGenerator } from '../ProjectImpactGraphGenerator'; import { FlagFile } from '../../api/FlagFile'; import { PnpmSyncUtilities } from '../../utilities/PnpmSyncUtilities'; import { HotlinkManager } from '../../utilities/HotlinkManager'; +import { detectAndReportWorkspaceCycles } from '../WorkspaceCycleDetector'; /** * Pnpm don't support --ignore-compatibility-db, so use --config.ignoreCompatibilityDb for now. @@ -442,6 +443,11 @@ export abstract class BaseInstallManager { // Check the policies await PolicyValidator.validatePolicyAsync(this.rushConfiguration, subspace, variant, this.options); + // Fail fast if there are undeclared cycles in the workspace package dependency graph. + // Pnpm cannot install a workspace with cycles, so this gives a clearer error message + // than whatever pnpm would emit. + detectAndReportWorkspaceCycles(this.rushConfiguration, terminal); + await this._installGitHooksAsync(); const approvedPackagesChecker: ApprovedPackagesChecker = new ApprovedPackagesChecker( diff --git a/libraries/rush-lib/src/logic/test/WorkspaceCycleDetector.test.ts b/libraries/rush-lib/src/logic/test/WorkspaceCycleDetector.test.ts new file mode 100644 index 0000000000..66f1c422d3 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/WorkspaceCycleDetector.test.ts @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'node:path'; + +import { RushConfiguration } from '../../api/RushConfiguration'; +import { _findWorkspaceCycle } from '../WorkspaceCycleDetector'; + +describe(_findWorkspaceCycle.name, () => { + function loadProjectsFromRepo(repoName: string): RushConfiguration['projects'] { + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + path.resolve(__dirname, `workspaceCycleDetector/${repoName}/rush.json`) + ); + return rushConfiguration.projects; + } + + it('returns undefined when there are no cycles', () => { + const projects: RushConfiguration['projects'] = loadProjectsFromRepo('no-cycle'); + const result: ReadonlyArray | undefined = _findWorkspaceCycle(projects); + expect(result).toBeUndefined(); + }); + + it('returns the cycle path when an undeclared cycle is present', () => { + const projects: RushConfiguration['projects'] = loadProjectsFromRepo('with-cycle'); + const result: ReadonlyArray | undefined = _findWorkspaceCycle(projects); + expect(result).toBeDefined(); + // The cycle should form a closed loop: [..., pkg-a] or [..., pkg-b] depending on + // iteration order. Either way the first and last element must be the same package, + // and both pkg-a and pkg-b must appear in the cycle. + expect(result!.length).toBeGreaterThanOrEqual(2); + expect(result![0]).toBe(result![result!.length - 1]); + const uniqueNames: Set = new Set(result); + expect(uniqueNames.has('pkg-a')).toBe(true); + expect(uniqueNames.has('pkg-b')).toBe(true); + }); + + it('returns undefined when the cycle is intentionally broken with decoupledLocalDependencies', () => { + const projects: RushConfiguration['projects'] = loadProjectsFromRepo('decoupled-cycle'); + const result: ReadonlyArray | undefined = _findWorkspaceCycle(projects); + expect(result).toBeUndefined(); + }); + + it('returns the cycle path when using the workspacePackages test repo (cyclic-dep-1 <-> cyclic-dep-2)', () => { + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + path.resolve(__dirname, 'workspacePackages/rush.json') + ); + const result: ReadonlyArray | undefined = _findWorkspaceCycle(rushConfiguration.projects); + expect(result).toBeDefined(); + expect(result![0]).toBe(result![result!.length - 1]); + const uniqueNames: Set = new Set(result); + expect(uniqueNames.has('cyclic-dep-1')).toBe(true); + expect(uniqueNames.has('cyclic-dep-2')).toBe(true); + }); +}); diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-a/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-a/package.json new file mode 100644 index 0000000000..7189d052e2 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-a/package.json @@ -0,0 +1,7 @@ +{ + "name": "pkg-a", + "version": "1.0.0", + "dependencies": { + "pkg-b": "workspace:*" + } +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-b/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-b/package.json new file mode 100644 index 0000000000..fbfb2c7070 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-b/package.json @@ -0,0 +1,7 @@ +{ + "name": "pkg-b", + "version": "1.0.0", + "dependencies": { + "pkg-a": "workspace:*" + } +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/rush.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/rush.json new file mode 100644 index 0000000000..4dc29674fd --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/rush.json @@ -0,0 +1,15 @@ +{ + "rushVersion": "0.0.0", + "pnpmVersion": "8.0.0", + "projects": [ + { + "packageName": "pkg-a", + "projectFolder": "pkg-a" + }, + { + "packageName": "pkg-b", + "projectFolder": "pkg-b", + "decoupledLocalDependencies": ["pkg-a"] + } + ] +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-a/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-a/package.json new file mode 100644 index 0000000000..ee09b194db --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-a/package.json @@ -0,0 +1,4 @@ +{ + "name": "pkg-a", + "version": "1.0.0" +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-b/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-b/package.json new file mode 100644 index 0000000000..fbfb2c7070 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-b/package.json @@ -0,0 +1,7 @@ +{ + "name": "pkg-b", + "version": "1.0.0", + "dependencies": { + "pkg-a": "workspace:*" + } +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/rush.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/rush.json new file mode 100644 index 0000000000..ddd8b55843 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/rush.json @@ -0,0 +1,14 @@ +{ + "rushVersion": "0.0.0", + "pnpmVersion": "8.0.0", + "projects": [ + { + "packageName": "pkg-a", + "projectFolder": "pkg-a" + }, + { + "packageName": "pkg-b", + "projectFolder": "pkg-b" + } + ] +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-a/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-a/package.json new file mode 100644 index 0000000000..7189d052e2 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-a/package.json @@ -0,0 +1,7 @@ +{ + "name": "pkg-a", + "version": "1.0.0", + "dependencies": { + "pkg-b": "workspace:*" + } +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-b/package.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-b/package.json new file mode 100644 index 0000000000..fbfb2c7070 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-b/package.json @@ -0,0 +1,7 @@ +{ + "name": "pkg-b", + "version": "1.0.0", + "dependencies": { + "pkg-a": "workspace:*" + } +} diff --git a/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/rush.json b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/rush.json new file mode 100644 index 0000000000..ddd8b55843 --- /dev/null +++ b/libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/rush.json @@ -0,0 +1,14 @@ +{ + "rushVersion": "0.0.0", + "pnpmVersion": "8.0.0", + "projects": [ + { + "packageName": "pkg-a", + "projectFolder": "pkg-a" + }, + { + "packageName": "pkg-b", + "projectFolder": "pkg-b" + } + ] +}