From 7ba3bb3399d1d468cd1c2ad64f9097bca85df7fe Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 12 Apr 2026 08:25:29 +0100 Subject: [PATCH] fix(ci): guard static import SCCs --- .github/workflows/ci.yml | 9 +- AGENTS.md | 1 + package.json | 1 + scripts/check-static-import-sccs.ts | 249 ++++++++++++++++++++++++++++ 4 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 scripts/check-static-import-sccs.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f129a71533c..1bb8fe68083 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -894,6 +894,11 @@ jobs: continue-on-error: true run: pnpm check:import-cycles + - name: Run static import SCC guard + id: static_import_sccs + continue-on-error: true + run: pnpm check:static-import-sccs + - name: Upload gateway watch regression artifacts if: always() uses: actions/upload-artifact@v7 @@ -927,6 +932,7 @@ jobs: 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 }} + STATIC_IMPORT_SCCS_OUTCOME: ${{ steps.static_import_sccs.outcome }} run: | failures=0 for result in \ @@ -951,7 +957,8 @@ jobs: "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" \ - "check:import-cycles|$IMPORT_CYCLES_OUTCOME"; do + "check:import-cycles|$IMPORT_CYCLES_OUTCOME" \ + "check:static-import-sccs|$STATIC_IMPORT_SCCS_OUTCOME"; do name="${result%%|*}" outcome="${result#*|}" if [ "$outcome" != "success" ]; then diff --git a/AGENTS.md b/AGENTS.md index b2ca2e6528d..e974968b564 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,6 +186,7 @@ - New runtime control-flow code should not branch on `error: string` or `reason: string` when a closed code union would be reasonable. - Dynamic import guardrail: do not mix `await import("x")` and static `import ... from "x"` for the same module in production code paths. If you need lazy loading, create a dedicated `*.runtime.ts` boundary (that re-exports from `x`) and dynamically import that boundary from lazy callers only. - Dynamic import verification: after refactors that touch lazy-loading/module boundaries, run `pnpm build` and check for `[INEFFECTIVE_DYNAMIC_IMPORT]` warnings before submitting. +- Circular dependencies: keep both `pnpm check:import-cycles` and `pnpm check:static-import-sccs` green; do not reintroduce runtime import cycles or static SCCs. - Extension SDK self-import guardrail: inside an extension package, do not import that same extension via `openclaw/plugin-sdk/` from production files. Route internal imports through a local barrel such as `./api.ts` or `./runtime-api.ts`, and keep the `plugin-sdk/` path as the external contract only. - Extension package boundary guardrail: inside a bundled plugin package, do not use relative imports/exports that resolve outside that same package root. If shared code belongs in the plugin SDK, import `openclaw/plugin-sdk/` instead of reaching into `src/plugin-sdk/**` or other repo paths via `../`. - Extension API surface rule: `openclaw/plugin-sdk/` is the only public cross-package contract for extension-facing SDK code. If an extension needs a new seam, add a public subpath first; do not reach into `src/plugin-sdk/**` by relative path. diff --git a/package.json b/package.json index 92aed4ebdc8..3dcdbe0f2ef 100644 --- a/package.json +++ b/package.json @@ -1116,6 +1116,7 @@ "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", + "check:static-import-sccs": "node --import tsx scripts/check-static-import-sccs.ts", "codex-app-server:protocol:check": "node --import tsx scripts/check-codex-app-server-protocol.ts", "config:channels:check": "node --import tsx scripts/generate-bundled-channel-config-metadata.ts --check", "config:channels:gen": "node --import tsx scripts/generate-bundled-channel-config-metadata.ts --write", diff --git a/scripts/check-static-import-sccs.ts b/scripts/check-static-import-sccs.ts new file mode 100644 index 00000000000..f581549d0db --- /dev/null +++ b/scripts/check-static-import-sccs.ts @@ -0,0 +1,249 @@ +#!/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", "ui"] 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 collectStaticImports( + file: string, + resolveSource: ReturnType, +): string[] { + const sourceFile = ts.createSourceFile( + file, + readFileSync(path.join(repoRoot, file), "utf8"), + ts.ScriptTarget.Latest, + true, + ); + const imports = new Set(); + const visit = (node: ts.Node) => { + let specifier: string | undefined; + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + specifier = node.moduleSpecifier.text; + } else if ( + ts.isExportDeclaration(node) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) { + specifier = node.moduleSpecifier.text; + } + if (specifier) { + const resolved = resolveSource(file, specifier); + if (resolved) { + imports.add(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, collectStaticImports(file, resolveSource)]), + ); + const components = collectStronglyConnectedComponents(graph); + + console.log(`Static import SCC check: ${components.length} component(s).`); + if (components.length === 0) { + return 0; + } + + console.error("\nStatic import SCCs:"); + for (const component of components) { + console.error(`\n# component size ${component.length}`); + console.error(formatCycle(component, graph)); + } + console.error( + "\nBreak the static cycle or extract a leaf contract instead of routing through a barrel.", + ); + return 1; +} + +process.exitCode = main();