feat(lint): detect import cycles with import-x/no-cycle

.gitignore

@@ -46,3 +46,16 @@ __pycache__/
 
 # Agent and manual git worktrees (full second checkouts; see worktrees-pnpm skill)
 worktrees/
+
+# Scratch fixture written by `packages/cli/test/import-cycle-lint.test.ts`. That
+# test writes a real a.ts <-> b.ts cycle INSIDE `packages/cli/src` and asserts
+# `import-x/no-cycle` reports it. The location is forced, not a convenience: the
+# rule only sees files the flat config matches, and the type-aware block needs
+# the file inside a tsconfig (`packages/cli/tsconfig.json` includes `src`) — a
+# fixture in the OS temp dir is refused as outside the config base path, and one
+# elsewhere in the repo fails to parse and reports zero cycles, which is the
+# vacuous green the test exists to rule out. Cleanup runs in `afterAll`, so a
+# killed run (Ctrl+C, OOM, CI cancellation) can strand a live cycle in the
+# source tree. This keeps that debris out of commits; delete the directory, not
+# this line.
+/packages/cli/src/__cycle-guard-*/

eslint.config.js

@@ -1,6 +1,7 @@
 import eslint from "@eslint/js";
 import tseslint from "typescript-eslint";
 import unicorn from "eslint-plugin-unicorn";
+import importX, { createNodeResolver } from "eslint-plugin-import-x";
 import prettierConfig from "eslint-config-prettier";
 
 export default tseslint.config(
@@ -100,6 +101,104 @@ export default tseslint.config(
       ],
     },
   },
+  // Import cycle detection.
+  //
+  // A circular import leaves one of the modules in the cycle holding
+  // `undefined` for whatever it imported, and which module loses depends on
+  // which one the graph is entered at first. That makes it a bug that the
+  // built CLI and most tests never see: it only fires when something enters
+  // the graph at the unlucky module. We hit exactly that — a migration
+  // registry that read as `undefined` and failed with
+  // `TypeError: migrate is not a function`, visible in two tests by luck.
+  // This rule is the check that would have caught it at author time.
+  {
+    files: ["**/*.ts", "**/*.tsx"],
+    plugins: { "import-x": importX },
+    settings: {
+      // Which extensions the plugin will parse when it follows an edge out of
+      // the file being linted. This is NOT cosmetic and it is not the same
+      // knob as the resolver below: the default is `['.js', '.mjs', '.cjs']`,
+      // and a file whose extension is not on this list is dropped by
+      // `ExportMap.get` before its imports are ever read. Without `.ts` here
+      // the rule resolves our files correctly, walks into them, finds nothing,
+      // and reports no cycles — on a tree that provably contains one. A lint
+      // run that is green because the rule is inert looks exactly like a lint
+      // run that is green because the code is clean, which is why the
+      // reintroduced-cycle check in this PR's description exists.
+      "import-x/extensions": [
+        ".ts",
+        ".tsx",
+        ".mts",
+        ".cts",
+        ".js",
+        ".jsx",
+        ".mjs",
+        ".cjs",
+      ],
+      // The plugin's own resolver, configured for TypeScript. It is backed by
+      // `unrs-resolver`, a direct dependency of eslint-plugin-import-x, so
+      // this needs no separate resolver package. It does need the extension
+      // list spelled out: the built-in default is
+      // `['.mjs', '.cjs', '.js', '.json', '.node']`, which resolves no `.ts`
+      // at all, and our sources import extensionlessly under
+      // `moduleResolution: "bundler"`. `.js` stays in the list for the handful
+      // of specifiers that carry an explicit extension.
+      "import-x/resolver-next": [
+        createNodeResolver({
+          extensions: [
+            ".ts",
+            ".tsx",
+            ".mts",
+            ".cts",
+            ".js",
+            ".mjs",
+            ".cjs",
+            ".json",
+          ],
+        }),
+      ],
+    },
+    rules: {
+      // On `import type` edges: the rule ignores them, in both directions —
+      // it returns early on an `ImportDeclaration` whose `importKind` is
+      // `type` (or whose every specifier is), and it filters
+      // `isOnlyImportingTypes` edges out of the graph walk. That is the
+      // behavior we want and it is not configurable, so there is no option
+      // below for it. It is also correct for us: a type-only edge is erased
+      // before the module ever runs, so it cannot produce the `undefined`
+      // binding this rule exists to catch, and flagging it would push people
+      // toward restructuring real code to satisfy an import that has no
+      // runtime existence. `verbatimModuleSyntax: true` in `tsconfig.base.json`
+      // is what makes this safe to lean on: it forces a type-only import to be
+      // written as `import type`, so the erasure is explicit in the syntax the
+      // rule reads rather than something the compiler infers later.
+      "import-x/no-cycle": [
+        "error",
+        {
+          // `maxDepth` is deliberately not set. Omitting it means unlimited
+          // (the rule reads it as `Number.POSITIVE_INFINITY` unless a number
+          // is given), and unlimited is what we want: the cycle we shipped was
+          // not a two-module A->B->A, and capping the depth would trade away
+          // exactly the cycles that are hard to spot by reading the code,
+          // which are the only ones worth spending a lint rule on. It is left
+          // out rather than passed as `Infinity` because the rule's schema
+          // accepts only an integer or the string "∞" there.
+          // Do not traverse into node_modules. A cycle that runs through a
+          // published dependency is not ours to break — we cannot edit it, so
+          // a report on it is noise we would have to suppress — and walking
+          // the dependency graph is where this rule's cost actually goes.
+          ignoreExternal: true,
+          // Keep the default (false). This would suppress a cycle whenever any
+          // edge in it is a dynamic `import()`, on the theory that the deferred
+          // evaluation breaks the loop. It does not reliably: a dynamic import
+          // awaited during module init is as circular as a static one, and the
+          // failure mode is the same `undefined`. We have no cycle that needs
+          // the escape hatch, so we do not open it.
+          allowUnsafeDynamicCyclicDependency: false,
+        },
+      ],
+    },
+  },
   // File naming conventions - enforce kebab-case for all TS/TSX files
   {
     files: ["**/*.ts", "**/*.tsx"],

package.json

@@ -48,6 +48,7 @@
     "@types/node": "^25.3.0",
     "eslint": "^9.39.2",
     "eslint-config-prettier": "^10.1.8",
+    "eslint-plugin-import-x": "^4.17.1",
     "eslint-plugin-unicorn": "^62.0.0",
     "husky": "^9.1.7",
     "lint-staged": "^15.4.3",

packages/cli/test/import-cycle-lint.test.ts

@@ -0,0 +1,84 @@
+import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { join, resolve } from "node:path";
+
+import { ESLint } from "eslint";
+import { afterAll, describe, expect, it } from "vitest";
+
+/**
+ * A guard that `import-x/no-cycle` is actually ON and actually reaching
+ * `packages/cli/src`.
+ *
+ * This does NOT re-implement cycle detection — that would be exactly the
+ * "re-derive what the tool already knows" mistake the style guide forbids.
+ * Every assertion below asks ESLint, running the repository's real
+ * `eslint.config.js`, and checks what it answers.
+ *
+ * It exists because the rule's failure mode is silence. While this rule was
+ * being added, the config resolved correctly, matched the right files, and
+ * reported `import-x/no-cycle` as an enabled error — and still found nothing on
+ * a tree that provably contained a cycle, because `import-x/extensions`
+ * defaults to `['.js', '.mjs', '.cjs']` and so every `.ts` file was dropped
+ * before its imports were read. A green `pnpm lint` looked identical whether
+ * the rule was working or inert. Nothing but an actual cycle distinguishes
+ * those two states, which is why the second test below writes one.
+ */
+
+const REPO_ROOT = resolve(import.meta.dirname, "..", "..", "..");
+const CLI_SOURCE = resolve(REPO_ROOT, "packages/cli/src");
+
+const temporaryDirectories: string[] = [];
+
+afterAll(async () => {
+  await Promise.all(
+    temporaryDirectories.map(async (directory) =>
+      rm(directory, { recursive: true, force: true })
+    )
+  );
+});
+
+function createESLint(): ESLint {
+  return new ESLint({ cwd: REPO_ROOT });
+}
+
+describe("import-x/no-cycle", () => {
+  it("is enabled as an error for files in packages/cli/src", async () => {
+    const config = (await createESLint().calculateConfigForFile(
+      join(CLI_SOURCE, "index.ts")
+    )) as { rules?: Record<string, unknown> };
+
+    // "error" is 2 once ESLint normalizes it. A config block that stopped
+    // matching `packages/cli/src` would leave this undefined.
+    expect(config.rules?.["import-x/no-cycle"]).toBeDefined();
+    expect((config.rules?.["import-x/no-cycle"] as unknown[])[0]).toBe(2);
+  });
+
+  it("reports a value cycle written into packages/cli/src", async () => {
+    // Written inside `packages/cli/src` on purpose: the point of the check is
+    // that the rule reaches THIS tree, so linting a fixture parked somewhere
+    // the config does not match would prove nothing. The directory name is
+    // prefixed so it is obviously not product code if cleanup is ever missed.
+    const directory = await mkdtemp(join(CLI_SOURCE, "__cycle-guard-"));
+    temporaryDirectories.push(directory);
+
+    // A -> B -> A over VALUE imports. Kept to real value edges because
+    // type-only edges are erased before the module runs and the rule ignores
+    // them by design; see the note in eslint.config.js.
+    await writeFile(
+      join(directory, "a.ts"),
+      'import { b } from "./b";\n\nexport const a = (): string => b();\n'
+    );
+    await writeFile(
+      join(directory, "b.ts"),
+      'import { a } from "./a";\n\nexport const b = (): string => a();\n'
+    );
+
+    const results = await createESLint().lintFiles([join(directory, "*.ts")]);
+    const cycleMessages = results.flatMap((result) =>
+      result.messages.filter(
+        (message) => message.ruleId === "import-x/no-cycle"
+      )
+    );
+
+    expect(cycleMessages.length).toBeGreaterThan(0);
+  });
+});