From 8f174e18e9e0d97938ceff7e3f1d7e4977f004c8 Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Wed, 1 Apr 2026 09:41:43 +0100 Subject: [PATCH] cli-module-build: pack sequentially when cycles are detected in dependencies Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: MT Lewis --- .../packager/computeTopologicalLayers.test.ts | 20 ++++++++++++------- .../lib/packager/computeTopologicalLayers.ts | 9 ++++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.test.ts b/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.test.ts index 055e481d79..a81ea0fc26 100644 --- a/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.test.ts +++ b/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.test.ts @@ -84,7 +84,7 @@ describe('computeTopologicalLayers', () => { expect(layers[2]).toEqual([d]); }); - it('falls back to a single layer on circular dependencies', () => { + it('falls back to sequential single-package layers on circular dependencies', () => { const a = makeNode('a'); const b = makeNode('b'); // Create a cycle: a -> b -> a @@ -92,9 +92,11 @@ describe('computeTopologicalLayers', () => { b.publishedLocalDependencies.set('a', a); const layers = computeTopologicalLayers([a, b]); - // Should still return all packages rather than hanging or throwing - expect(layers).toHaveLength(1); - expect(layers[0]).toEqual(expect.arrayContaining([a, b])); + // Should pack each remaining package sequentially rather than in parallel + expect(layers).toHaveLength(2); + expect(layers[0]).toHaveLength(1); + expect(layers[1]).toHaveLength(1); + expect(layers.flat()).toEqual(expect.arrayContaining([a, b])); }); it('handles a partial cycle with non-cyclic packages separated into earlier layers', () => { @@ -108,9 +110,13 @@ describe('computeTopologicalLayers', () => { const layers = computeTopologicalLayers([a, b, c]); // a has no deps, so it goes in layer 0 expect(layers[0]).toEqual([a]); - // b and c form a cycle, so they are dumped together - expect(layers[1]).toEqual(expect.arrayContaining([b, c])); - expect(layers).toHaveLength(2); + // b and c form a cycle, so they are packed sequentially + expect(layers[1]).toHaveLength(1); + expect(layers[2]).toHaveLength(1); + expect([layers[1][0], layers[2][0]]).toEqual( + expect.arrayContaining([b, c]), + ); + expect(layers).toHaveLength(3); }); it('produces correct layers regardless of input order', () => { diff --git a/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.ts b/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.ts index 0a0e78eb8a..e626996dbb 100644 --- a/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.ts +++ b/packages/cli-module-build/src/lib/packager/computeTopologicalLayers.ts @@ -47,9 +47,12 @@ export function computeTopologicalLayers( } if (layer.length === 0) { - // Circular dependency — fall back to packing everything remaining - // together, accepting the (pre-existing) race risk for this cycle. - layers.push(Array.from(remaining.values())); + // Circular dependency — fall back to packing the remaining packages + // sequentially as single-package layers to avoid parallel races. + for (const pkg of remaining.values()) { + layers.push([pkg]); + } + remaining.clear(); break; }