From 4847818eb4b0b8be039152abde19efb24737f8aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:45:16 +0000 Subject: [PATCH 1/3] feat: add workspace cycle detection to rush install/update --- ...pace-cycle-detection_2026-07-23-01-32.json | 9 ++ .../src/logic/WorkspaceCycleDetector.ts | 98 +++++++++++++++++++ .../src/logic/base/BaseInstallManager.ts | 6 ++ .../logic/test/WorkspaceCycleDetector.test.ts | 54 ++++++++++ .../decoupled-cycle/pkg-a/package.json | 7 ++ .../decoupled-cycle/pkg-b/package.json | 7 ++ .../decoupled-cycle/rush.json | 15 +++ .../no-cycle/pkg-a/package.json | 4 + .../no-cycle/pkg-b/package.json | 7 ++ .../workspaceCycleDetector/no-cycle/rush.json | 14 +++ .../with-cycle/pkg-a/package.json | 7 ++ .../with-cycle/pkg-b/package.json | 7 ++ .../with-cycle/rush.json | 14 +++ 13 files changed, 249 insertions(+) create mode 100644 common/changes/@microsoft/rush/add-workspace-cycle-detection_2026-07-23-01-32.json create mode 100644 libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts create mode 100644 libraries/rush-lib/src/logic/test/WorkspaceCycleDetector.test.ts create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-a/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/pkg-b/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/decoupled-cycle/rush.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-a/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/pkg-b/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/no-cycle/rush.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-a/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/pkg-b/package.json create mode 100644 libraries/rush-lib/src/logic/test/workspaceCycleDetector/with-cycle/rush.json 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 00000000000..970da9764da --- /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 00000000000..d95602d4ca4 --- /dev/null +++ b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts @@ -0,0 +1,98 @@ +// 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. + * + * If a cycle is intentional, add one of the involved packages to the + * `decoupledLocalDependencies` field in rush.json. + */ +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, add one of the packages in the cycle to the "decoupledLocalDependencies" ` + + `field for the dependent project in ${RushConstants.rushJsonFilename}. ` + + `This will cause Rush to treat that dependency as an external package ` + + `rather than a local workspace package.` + ) + ); + 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 a7016129ae6..0642d77af0c 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 00000000000..66f1c422d36 --- /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 00000000000..7189d052e2b --- /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 00000000000..fbfb2c70709 --- /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 00000000000..4dc29674fd1 --- /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 00000000000..ee09b194db8 --- /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 00000000000..fbfb2c70709 --- /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 00000000000..ddd8b558437 --- /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 00000000000..7189d052e2b --- /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 00000000000..fbfb2c70709 --- /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 00000000000..ddd8b558437 --- /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" + } + ] +} From 0c07c91945c3a0453d5fc2dcf8f58c33f1222f86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:53:36 +0000 Subject: [PATCH 2/3] fix(rush-lib): update cycle error message to recommend refactoring over decoupledLocalDependencies --- .../src/logic/WorkspaceCycleDetector.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts index d95602d4ca4..b40efedf426 100644 --- a/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts +++ b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts @@ -16,8 +16,10 @@ import { RushConstants } from './RushConstants'; * 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. * - * If a cycle is intentional, add one of the involved packages to the - * `decoupledLocalDependencies` field in rush.json. + * The recommended 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. If the cycle truly cannot be broken, + * `decoupledLocalDependencies` can be used as a last resort. */ export function detectAndReportWorkspaceCycles( rushConfiguration: RushConfiguration, @@ -31,10 +33,13 @@ export function detectAndReportWorkspaceCycles( Colorize.red( 'A cyclic dependency was detected among workspace packages:\n' + ` ${cycle.join(' -> ')}\n\n` + - `To fix this, add one of the packages in the cycle to the "decoupledLocalDependencies" ` + - `field for the dependent project in ${RushConstants.rushJsonFilename}. ` + - `This will cause Rush to treat that dependency as an external package ` + - `rather than a local workspace package.` + `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` + + `If the cycle truly cannot be broken by refactoring, you can use the ` + + `"decoupledLocalDependencies" field in ${RushConstants.rushJsonFilename} as a last resort. ` + + `This causes Rush to treat that dependency as an external package rather than a local ` + + `workspace package, at the cost of losing workspace linking for that edge.` ) ); throw new AlreadyReportedError(); From df8ba76772ea5978d0d85508bec168d4841add76 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:55:36 +0000 Subject: [PATCH 3/3] fix(rush-lib): narrow decoupledLocalDependencies guidance to bootstrapping problem only --- .../rush-lib/src/logic/WorkspaceCycleDetector.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts index b40efedf426..d204135d6eb 100644 --- a/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts +++ b/libraries/rush-lib/src/logic/WorkspaceCycleDetector.ts @@ -16,10 +16,11 @@ import { RushConstants } from './RushConstants'; * 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 recommended 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. If the cycle truly cannot be broken, - * `decoupledLocalDependencies` can be used as a last resort. + * 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, @@ -36,10 +37,9 @@ export function detectAndReportWorkspaceCycles( `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` + - `If the cycle truly cannot be broken by refactoring, you can use the ` + - `"decoupledLocalDependencies" field in ${RushConstants.rushJsonFilename} as a last resort. ` + - `This causes Rush to treat that dependency as an external package rather than a local ` + - `workspace package, at the cost of losing workspace linking for that edge.` + `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();