diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index deb0531489f..d6a6c2acc86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -779,6 +779,11 @@ jobs: continue-on-error: true run: pnpm test:gateway:watch-regression + - name: Run import cycle guard + id: import_cycles + continue-on-error: true + run: pnpm check:import-cycles + - name: Upload gateway watch regression artifacts if: always() uses: actions/upload-artifact@v7 @@ -811,6 +816,7 @@ jobs: NO_RAW_WINDOW_OPEN_OUTCOME: ${{ steps.no_raw_window_open.outcome }} CONTROL_UI_I18N_OUTCOME: ${{ steps.control_ui_i18n.outcome == 'skipped' && 'success' || steps.control_ui_i18n.outcome }} GATEWAY_WATCH_REGRESSION_OUTCOME: ${{ steps.gateway_watch_regression.outcome }} + IMPORT_CYCLES_OUTCOME: ${{ steps.import_cycles.outcome }} run: | failures=0 for result in \ @@ -834,7 +840,8 @@ jobs: "test:extensions:package-boundary|$EXTENSION_PACKAGE_BOUNDARY_TSC_OUTCOME" \ "lint:ui:no-raw-window-open|$NO_RAW_WINDOW_OPEN_OUTCOME" \ "ui:i18n:check|$CONTROL_UI_I18N_OUTCOME" \ - "gateway-watch-regression|$GATEWAY_WATCH_REGRESSION_OUTCOME"; do + "gateway-watch-regression|$GATEWAY_WATCH_REGRESSION_OUTCOME" \ + "check:import-cycles|$IMPORT_CYCLES_OUTCOME"; do name="${result%%|*}" outcome="${result#*|}" if [ "$outcome" != "success" ]; then diff --git a/docs/ci.md b/docs/ci.md index 7f992577095..f1c24fb32f9 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -21,7 +21,7 @@ The CI runs on every push to `main` and every pull request. It uses smart scopin | `checks-fast-extensions` | Aggregate the extension shard lanes after `checks-fast-extensions-shard` completes | Node-relevant changes | | `extension-fast` | Focused tests for only the changed bundled plugins | When extension changes are detected | | `check` | Main local gate in CI: `pnpm check` plus `pnpm build:strict-smoke` | Node-relevant changes | -| `check-additional` | Architecture and boundary guards plus the gateway watch regression harness | Node-relevant changes | +| `check-additional` | Architecture, boundary, import-cycle guards plus the gateway watch regression harness | Node-relevant changes | | `build-smoke` | Built-CLI smoke tests and startup-memory smoke | Node-relevant changes | | `checks` | Heavier Linux Node lanes: full tests, channel tests, and push-only Node 22 compatibility | Node-relevant changes | | `check-docs` | Docs formatting, lint, and broken-link checks | Docs changed | @@ -58,6 +58,7 @@ On pushes, the `checks` matrix adds the push-only `compat-node22` lane. On pull ```bash pnpm check # types + lint + format pnpm build:strict-smoke +pnpm check:import-cycles pnpm test:gateway:watch-regression pnpm test # vitest tests pnpm test:channels diff --git a/package.json b/package.json index 86c8958ed97..5ce954661b4 100644 --- a/package.json +++ b/package.json @@ -1092,6 +1092,7 @@ "check:bundled-channel-config-metadata": "node --import tsx scripts/generate-bundled-channel-config-metadata.ts --check", "check:docs": "pnpm format:docs:check && pnpm lint:docs && pnpm docs:check-i18n-glossary && pnpm docs:check-links", "check:host-env-policy:swift": "node scripts/generate-host-env-security-policy-swift.mjs --check", + "check:import-cycles": "node --import tsx scripts/check-import-cycles.ts", "check:loc": "node --import tsx scripts/check-ts-max-loc.ts --max 500", "check:no-conflict-markers": "node scripts/check-no-conflict-markers.mjs", "config:channels:check": "node --import tsx scripts/generate-bundled-channel-config-metadata.ts --check", diff --git a/scripts/check-import-cycles.ts b/scripts/check-import-cycles.ts new file mode 100644 index 00000000000..6578c55d7dd --- /dev/null +++ b/scripts/check-import-cycles.ts @@ -0,0 +1,278 @@ +#!/usr/bin/env node +import { readdirSync, readFileSync, statSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const scanRoots = ["src", "extensions", "scripts"] as const; +const sourceExtensions = [".ts", ".tsx", ".mts", ".cts", ".js", ".mjs", ".cjs"] as const; +const testSourcePattern = /(?:\.test|\.e2e\.test)\.[cm]?[tj]sx?$/; +const generatedSourcePattern = /\.(?:generated|bundle)\.[tj]s$/; +const declarationSourcePattern = /\.d\.[cm]?ts$/; +const ignoredPathPartPattern = + /(^|\/)(node_modules|dist|build|coverage|\.artifacts|\.git|assets)(\/|$)/; + +function normalizeRepoPath(filePath: string): string { + return path.relative(repoRoot, filePath).split(path.sep).join("/"); +} + +function cycleSignature(files: readonly string[]): string { + return files.toSorted((left, right) => left.localeCompare(right)).join("\n"); +} + +function shouldSkipRepoPath(repoPath: string): boolean { + return ( + ignoredPathPartPattern.test(repoPath) || + testSourcePattern.test(repoPath) || + generatedSourcePattern.test(repoPath) || + declarationSourcePattern.test(repoPath) + ); +} + +function collectSourceFiles(root: string): string[] { + const repoPath = normalizeRepoPath(root); + if (shouldSkipRepoPath(repoPath)) { + return []; + } + const stats = statSync(root); + if (stats.isFile()) { + return sourceExtensions.some((extension) => repoPath.endsWith(extension)) ? [repoPath] : []; + } + if (!stats.isDirectory()) { + return []; + } + return readdirSync(root, { withFileTypes: true }) + .flatMap((entry) => collectSourceFiles(path.join(root, entry.name))) + .toSorted((left, right) => left.localeCompare(right)); +} + +function createSourceResolver(files: readonly string[]) { + const fileSet = new Set(files); + const pathMap = new Map(); + for (const file of files) { + const parsed = path.posix.parse(file); + const extensionless = path.posix.join(parsed.dir, parsed.name); + pathMap.set(extensionless, file); + if (file.endsWith(".ts")) { + pathMap.set(`${extensionless}.js`, file); + } else if (file.endsWith(".tsx")) { + pathMap.set(`${extensionless}.jsx`, file); + } else if (file.endsWith(".mts")) { + pathMap.set(`${extensionless}.mjs`, file); + } else if (file.endsWith(".cts")) { + pathMap.set(`${extensionless}.cjs`, file); + } + } + return (importer: string, specifier: string): string | null => { + if (!specifier.startsWith(".")) { + return null; + } + const base = path.posix.normalize(path.posix.join(path.posix.dirname(importer), specifier)); + const candidates = [ + base, + ...sourceExtensions.map((extension) => `${base}${extension}`), + `${base}/index.ts`, + `${base}/index.tsx`, + `${base}/index.js`, + `${base}/index.mjs`, + ]; + for (const candidate of candidates) { + if (fileSet.has(candidate)) { + return candidate; + } + const mapped = pathMap.get(candidate); + if (mapped) { + return mapped; + } + } + return null; + }; +} + +function importDeclarationHasRuntimeEdge(node: ts.ImportDeclaration): boolean { + if (!node.importClause) { + return true; + } + if (node.importClause.isTypeOnly) { + return false; + } + const bindings = node.importClause.namedBindings; + if (node.importClause.name || !bindings || ts.isNamespaceImport(bindings)) { + return true; + } + return bindings.elements.some((element) => !element.isTypeOnly); +} + +function exportDeclarationHasRuntimeEdge(node: ts.ExportDeclaration): boolean { + if (!node.moduleSpecifier || node.isTypeOnly) { + return false; + } + const clause = node.exportClause; + if (!clause || ts.isNamespaceExport(clause)) { + return true; + } + return clause.elements.some((element) => !element.isTypeOnly); +} + +function collectRuntimeStaticImports( + file: string, + resolveSource: ReturnType, +) { + const sourceFile = ts.createSourceFile( + file, + readFileSync(path.join(repoRoot, file), "utf8"), + ts.ScriptTarget.Latest, + true, + ); + const imports: string[] = []; + const visit = (node: ts.Node) => { + let specifier: string | undefined; + let include = false; + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + specifier = node.moduleSpecifier.text; + include = importDeclarationHasRuntimeEdge(node); + } else if ( + ts.isExportDeclaration(node) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) { + specifier = node.moduleSpecifier.text; + include = exportDeclarationHasRuntimeEdge(node); + } + if (include && specifier) { + const resolved = resolveSource(file, specifier); + if (resolved) { + imports.push(resolved); + } + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + return imports.toSorted((left, right) => left.localeCompare(right)); +} + +function collectStronglyConnectedComponents( + graph: ReadonlyMap, +): string[][] { + let nextIndex = 0; + const stack: string[] = []; + const onStack = new Set(); + const indexByNode = new Map(); + const lowLinkByNode = new Map(); + const components: string[][] = []; + + const visit = (node: string) => { + indexByNode.set(node, nextIndex); + lowLinkByNode.set(node, nextIndex); + nextIndex += 1; + stack.push(node); + onStack.add(node); + + for (const next of graph.get(node) ?? []) { + if (!indexByNode.has(next)) { + visit(next); + lowLinkByNode.set(node, Math.min(lowLinkByNode.get(node)!, lowLinkByNode.get(next)!)); + } else if (onStack.has(next)) { + lowLinkByNode.set(node, Math.min(lowLinkByNode.get(node)!, indexByNode.get(next)!)); + } + } + + if (lowLinkByNode.get(node) !== indexByNode.get(node)) { + return; + } + const component: string[] = []; + let current: string | undefined; + do { + current = stack.pop(); + if (!current) { + throw new Error("Import cycle stack underflow"); + } + onStack.delete(current); + component.push(current); + } while (current !== node); + if (component.length > 1 || (graph.get(node) ?? []).includes(node)) { + components.push(component.toSorted((left, right) => left.localeCompare(right))); + } + }; + + for (const node of graph.keys()) { + if (!indexByNode.has(node)) { + visit(node); + } + } + return components.toSorted( + (left, right) => + right.length - left.length || cycleSignature(left).localeCompare(cycleSignature(right)), + ); +} + +function findCycleWitness( + component: readonly string[], + graph: ReadonlyMap, +): string[] { + const componentSet = new Set(component); + const start = component[0]; + if (!start) { + return []; + } + const activePath: string[] = []; + const visited = new Set(); + const visit = (node: string): string[] | null => { + activePath.push(node); + visited.add(node); + for (const next of graph.get(node) ?? []) { + if (!componentSet.has(next)) { + continue; + } + const existingIndex = activePath.indexOf(next); + if (existingIndex >= 0) { + return [...activePath.slice(existingIndex), next]; + } + if (!visited.has(next)) { + const result = visit(next); + if (result) { + return result; + } + } + } + activePath.pop(); + return null; + }; + return visit(start) ?? component; +} + +function formatCycle( + component: readonly string[], + graph: ReadonlyMap, +): string { + const witness = findCycleWitness(component, graph); + return witness.map((file, index) => `${index === 0 ? " " : " -> "}${file}`).join("\n"); +} + +function main(): number { + const files = scanRoots.flatMap((root) => collectSourceFiles(path.join(repoRoot, root))); + const resolveSource = createSourceResolver(files); + const graph = new Map( + files.map((file): [string, string[]] => [ + file, + collectRuntimeStaticImports(file, resolveSource), + ]), + ); + const components = collectStronglyConnectedComponents(graph); + + console.log(`Import cycle check: ${components.length} runtime value cycle(s).`); + if (components.length === 0) { + return 0; + } + + console.error("\nRuntime value import cycles:"); + for (const component of components) { + console.error(`\n# component size ${component.length}`); + console.error(formatCycle(component, graph)); + } + console.error("\nBreak the cycle or convert type-only edges to `import type`."); + return 1; +} + +process.exitCode = main();