Skip to content

feat(rush-lib): early cycle detection for workspace packages in rush install/update#5904

Draft
dmichon-msft with Copilot wants to merge 3 commits into
mainfrom
copilot/add-early-validation-for-update
Draft

feat(rush-lib): early cycle detection for workspace packages in rush install/update#5904
dmichon-msft with Copilot wants to merge 3 commits into
mainfrom
copilot/add-early-validation-for-update

Conversation

Copilot AI commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Undeclared cycles between workspace packages cause pnpm to fail deep in the install process with a cryptic error. This adds an early validation that detects such cycles before any package-manager work begins and fails immediately with a clear, actionable message.

Changes

  • src/logic/WorkspaceCycleDetector.ts (new) — DFS-based O(V+E) cycle detection over RushConfigurationProject.dependencyProjects, which already excludes decoupledLocalDependencies. Exports _findWorkspaceCycle() (returns cycle path or undefined) and detectAndReportWorkspaceCycles() (logs + throws AlreadyReportedError).
  • BaseInstallManager.prepareAsync() — calls detectAndReportWorkspaceCycles() immediately after policy validation, before any shrinkwrap or package-manager work.
  • WorkspaceCycleDetector.test.ts + three minimal fixture repos — covers no-cycle, undeclared cycle, and decoupledLocalDependencies-broken cycle cases.

Example error

A cyclic dependency was detected among workspace packages:
  pkg-a -> pkg-b -> pkg-a

To fix this, add one of the packages in the cycle to the "decoupledLocalDependencies"
field for the dependent project in rush.json.

Cycles declared via decoupledLocalDependencies are not flagged. Only the first detected cycle is reported per run.

@github-project-automation github-project-automation Bot moved this to Needs triage in Bug Triage Jul 23, 2026
Copilot AI changed the title feat(rush-lib): add early workspace cycle detection for rush install/update feat(rush-lib): early cycle detection for workspace packages in rush install/update Jul 23, 2026
Copilot AI requested a review from dmichon-msft July 23, 2026 01:46
@dmichon-msft

Copy link
Copy Markdown
Contributor

@copilot Do NOT recommend decoupledLocalDependencies as the default way to break a cycle. Recommend first actually breaking the cycle, e.g. refactoring code out into a project that can be depended on by the projects involved in the cycle, or moving code from one project to another.

Copilot AI commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Do NOT recommend decoupledLocalDependencies as the default way to break a cycle. Recommend first actually breaking the cycle, e.g. refactoring code out into a project that can be depended...

Updated in the latest commit. The error message now leads with refactoring as the recommended fix (extracting shared code into a new package, or moving code so the dependency flows in one direction), and only mentions decoupledLocalDependencies at the end as a last resort — explicitly noting the cost of losing workspace linking for that edge. The JSDoc comment was updated to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

2 participants