test: checkNoNewCycles reports the same cycle with non-deterministic node order across calls

[carlos-alm]

Summary

tests/integration/check.test.ts's checkNoNewCycles > excludeSpeculative has no effect on cycles formed entirely of static import edges fails intermittently/consistently (reproduced 3/3 runs locally) with a node-order mismatch, not a content mismatch:

- Expected
+ Received

  [
    {
      "nodes": [
-       "src/utils.js",
        "src/math.js",
+       "src/utils.js",
      ],
      "speculative": false,
    },
  ]

The test:

test('excludeSpeculative has no effect on cycles formed entirely of static import edges', () => {
  const withSpeculative = checkNoNewCycles(db, new Set(['src/math.js']), false, false);
  const withoutSpeculative = checkNoNewCycles(db, new Set(['src/math.js']), false, true);
  expect(withoutSpeculative.cycles).toEqual(withSpeculative.cycles);
});

checkNoNewCycles calls findCycles() (src/domain/graph/cycles.ts) internally and excludeSpeculative only applies a .filter() afterward (return opts.excludeSpeculative ? cycles.filter((c) => !c.speculative) : cycles;) — it never reorders anything. Since both calls query the exact same DB fixture with no writes in between, the two calls should be producing byte-for-byte identical cycles arrays (same node order within each cycle), but the node order inside the single reported cycle differs between the two invocations.

Suspected root cause

Likely a missing ORDER BY somewhere in the query that feeds node/edge data into the cycle-detection (Tarjan) algorithm, allowing SQLite to return rows in a different order across two separate query executions of the same underlying SQL — which then changes the traversal order in Tarjan's algorithm and thus the order nodes are pushed into a discovered cycle. Should be investigated in src/domain/graph/cycles.ts and src/graph/algorithms/tarjan.ts.

Impact

Cosmetic for callers that only care about which files are in a cycle (most callers), but:

• Makes the affected test flaky/consistently-red depending on execution timing.
• Any caller that compares cycle results for equality (as this test does) or displays them to users could see non-deterministic, confusing output across runs on unchanged code.

Repro

npx vitest run tests/integration/check.test.ts -t "excludeSpeculative has no effect on cycles formed entirely of static import edges"

Failed consistently in 3/3 local runs, unrelated to any specific engine (checkNoNewCycles/findCycles here operate purely against a hand-inserted better-sqlite3 fixture, not through buildGraph).

Found while validating an unrelated fix (#1938); not fixed there to keep that PR scoped to a single concern.