chore(ci): make the cycle check runnable on Windows and report drift honestly (#10548)

Two independent papercuts in the circular reference check, both hit while
investigating a failure on another PR.

`npm run check-cycle-references` could not run on Windows at all. It spawned
the extensionless `node_modules/.bin/depcruise` shim through `execFileSync`,
which Windows cannot launch; the `.cmd` sibling does not help either, since
Node >=18 refuses to spawn `.cmd` without a shell (CVE-2024-27980). Resolve
dependency-cruiser's own entry point and run it with `process.execPath`
instead — no shell, works everywhere.

The check also fails on baseline drift (cycles recorded in the baseline that
no longer exist), which is correct: the baseline is a ratchet, and leaving a
fixed cycle in it would silently keep permitting its reintroduction. But CI
labelled every non-success outcome "New circular references detected", so a PR
that *removed* two cycles was reported as having added some. That cost real
debugging time.

Give the script distinct exit codes — 1 for a new cycle, 2 for drift alone —
and have the workflow capture and map them to separate messages. The failure
behaviour is unchanged; only the wording is now accurate. Renamed the final
step to match what it actually gates on.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

.github/workflows/test.yml

@@ -107,7 +107,15 @@ jobs:
         id: check-cycles
         continue-on-error: true
         shell: bash
-        run: npm run check-cycle-references | tee cycle-report.txt
+        # Exit code 1 (a new cycle) and 2 (a stale baseline) are reported differently below, so
+        # capture it rather than letting `-e` collapse both into a generic step failure.
+        run: |
+          set +e
+          npm run check-cycle-references | tee cycle-report.txt
+          status=${PIPESTATUS[0]}
+          set -e
+          echo "status=$status" >> "$GITHUB_OUTPUT"
+          exit "$status"
 
       - name: Post circular references PR comment
         if: github.event_name == 'pull_request' && always() && steps.check-cycles.outcome != 'skipped'
@@ -117,12 +125,21 @@ jobs:
           script: |
             const fs = require('fs');
 
-            const passed = '${{ steps.check-cycles.outcome }}' === 'success';
+            // 0 = matches baseline, 1 = a new cycle appeared, 2 = baseline is stale. Anything
+            // else means the check itself blew up, which is not a claim about cycles either way.
+            const status = '${{ steps.check-cycles.outputs.status }}';
+            const passed = status === '0';
+            const summaryByStatus = {
+              '0': { icon: '✅', text: 'No new circular references' },
+              '1': { icon: '⚠️', text: 'New circular references detected' },
+              '2': { icon: '♻️', text: 'No new circular references, but the baseline is stale — regenerate and commit it' },
+            };
+            const { icon, text } = summaryByStatus[status] ?? { icon: '💥', text: 'The circular reference check failed to run' };
             const report = fs.readFileSync('cycle-report.txt', 'utf8').trim();
             const markdownContent = `<details>
-            <summary>${passed ? '✅' : '⚠️'} Circular References Report</summary>
+            <summary>${icon} Circular References Report</summary>
 
-            **Status:** ${passed ? 'No new circular references' : 'New circular references detected'}
+            **Status:** ${text}
 
             \`\`\`
             ${report}
@@ -170,6 +187,8 @@ jobs:
               console.log('Created new PR comment');
             }
 
-      - name: Fail if new circular references found
+      # Fails on a stale baseline too, not just on new cycles: the baseline is a ratchet, so a
+      # cycle left in it after being fixed would silently keep permitting its reintroduction.
+      - name: Fail if the circular reference check did not pass
         if: steps.check-cycles.outcome == 'failure'
         run: exit 1

scripts/circular-references/check-cycle-references.mjs

@@ -3,16 +3,28 @@
 // Runs dependency-cruiser per workspace package to find circular imports, and fails only on
 // cycles that are not already recorded in the baseline file. Run with --update-baseline to
 // regenerate the baseline from the current state (e.g. after fixing a cycle).
+//
+// Exits 0 when the tree matches the baseline, 1 when a new cycle appeared, and 2 when the
+// baseline is merely stale. CI distinguishes the last two when reporting.
 import { execFileSync } from 'node:child_process';
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 
 const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
-const depcruiseBin = path.join(repoRoot, 'node_modules', '.bin', 'depcruise');
+// Resolve dependency-cruiser's own entry point rather than the `.bin` shim, and run it with the
+// current Node binary. On Windows the shim is split into an extensionless shell script (which
+// `execFileSync` cannot launch) and a `.cmd` (which Node >=18 refuses to spawn without a shell,
+// see CVE-2024-27980), so going through `.bin` makes this script POSIX-only for no benefit.
+const depcruiseBin = path.join(repoRoot, 'node_modules', 'dependency-cruiser', 'bin', 'dependency-cruise.mjs');
 const configPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'dependency-cruiser.json');
 const baselinePath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'known-violations.json');
 
+/** A cycle exists that the baseline does not record — a regression. */
+const EXIT_NEW_CYCLES = 1;
+/** The baseline records cycles that no longer exist — it just needs regenerating. */
+const EXIT_BASELINE_DRIFT = 2;
+
 // Same scope as `npm run lint`/`type-check`/`test` (--workspaces --if-present): the packages
 // actually declared as npm workspaces, not every directory under packages/ (e.g.
 // insomnia-component-docs is intentionally excluded and its deps aren't installed at the root).
@@ -48,7 +60,11 @@ function cruisePackage({ name, dir }) {
 
   let stdout;
   try {
-    stdout = execFileSync(depcruiseBin, args, { cwd: dir, encoding: 'utf8', maxBuffer: 1024 * 1024 * 100 });
+    stdout = execFileSync(process.execPath, [depcruiseBin, ...args], {
+      cwd: dir,
+      encoding: 'utf8',
+      maxBuffer: 1024 * 1024 * 100,
+    });
   } catch (error) {
     if (error?.status !== 1 || !error.stdout) throw error;
     // dependency-cruiser exits with status 1 when forbidden violations are found. Parse its JSON
@@ -143,7 +159,10 @@ function main() {
     if (hasNew) console.log('\nNew circular dependencies detected that are not in the baseline.');
     if (hasDrift) console.log('\nBaseline contains circular dependencies no longer present in the current tree.');
     console.log(`Run "npm run check-cycle-references:baseline" and commit the updated ${path.basename(baselinePath)}.`);
-    process.exit(1);
+    // Both cases need the baseline regenerated, but they mean opposite things — a new cycle is a
+    // regression, while drift alone means cycles were fixed and the baseline was left behind.
+    // Exit 1 vs 2 so CI can say which happened instead of reporting every failure as "new".
+    process.exit(hasNew ? EXIT_NEW_CYCLES : EXIT_BASELINE_DRIFT);
   }
   console.log('\nNo new circular dependencies.');
 }