From 06b1fefd35f74b45e93f2eddedff51c085e0214f Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Mon, 18 May 2026 00:42:09 +0200 Subject: [PATCH] fix(circular): break coding-agent core mega-cycle + skip function-body imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 2 (the 13-node coding-agent mega) closed via two changes: 1. scripts/check-circular-deps.mjs — track function-body depth and skip require()/import() calls inside function bodies. They run on call, not at module evaluation, and therefore cannot cause module-graph cycles — same reasoning as the existing dynamic `await import()` skip. Generic improvement; benefits any pattern that uses lazy CommonJS require() to break a static cycle. 2. packages/coding-agent/src/core/extensions/loader.ts — removed the static `import * as _bundledCodingAgent from "../../index.js"` self-reference, which was the cycle-closer. It only populated STATIC_BUNDLED_MODULES for the Bun virtualModules path (`isBunBinary` branch in getJitiOptions), and SF is Node-26-only per operator policy (no Bun) — so the Bun branch is dead at runtime and dropping the static self-reference is safe. The two map entries that referenced it (@singularity-forge/coding-agent and the @mariozechner alias) are commented out at the same site with a pointer to the top-of-file note. Net effect across the full session: start of session: 9 cycles walker false-positive cleanups landed: dropped 6 type-only + dynamic-import false positives tui ↔ overlay-layout: CURSOR_MARKER moved to overlay-types.ts SF autonomous-rollback chain (3 targeted cuts): experimental → preferences-serializer, classifier → lazy rollback import, preferences-models → runaway-defaults.js this commit: coding-agent loader self-reference dropped Final state: ✅ zero circular dependencies in 1193 scanned files. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/core/extensions/loader.ts | 14 +++++++-- scripts/check-circular-deps.mjs | 30 ++++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/core/extensions/loader.ts b/packages/coding-agent/src/core/extensions/loader.ts index 419cd6370..8bb457e2b 100644 --- a/packages/coding-agent/src/core/extensions/loader.ts +++ b/packages/coding-agent/src/core/extensions/loader.ts @@ -32,7 +32,14 @@ import * as _bundledYaml from "yaml"; import { getAgentDir, isBunBinary } from "../../config.js"; // NOTE: This import works because loader.ts exports are NOT re-exported from index.ts, // avoiding a circular dependency. Extensions can import from "@singularity-forge/coding-agent. -import * as _bundledCodingAgent from "../../index.js"; +// `@singularity-forge/coding-agent` self-reference removed. +// It was the cycle-closer for the coding-agent core mega-cycle +// (loader → index → main → modes → ... → loader). The static +// `import * as _bundledCodingAgent from "../../index.js"` only +// populated STATIC_BUNDLED_MODULES below for the Bun virtualModules +// path. SF is Node-26-only per operator policy (memory: +// feedback_node_only) — no bun, no better-sqlite3 — so the Bun +// branch is dead at runtime and dropping the static import is safe. import { createEventBus, type EventBus } from "../event-bus.js"; import type { ExecOptions } from "../exec.js"; import { execCommand } from "../exec.js"; @@ -68,7 +75,7 @@ const STATIC_BUNDLED_MODULES: Record = { "@singularity-forge/tui": _bundledTui, "@singularity-forge/ai": _bundledAi, "@singularity-forge/ai/oauth": _bundledAiOauth, - "@singularity-forge/coding-agent": _bundledCodingAgent, + // Self-reference handled lazily — see buildVirtualModules() below. yaml: _bundledYaml, "@modelcontextprotocol/sdk/client": _bundledMcpClient, "@modelcontextprotocol/sdk/client/stdio": _bundledMcpStdio, @@ -94,7 +101,8 @@ const STATIC_BUNDLED_MODULES: Record = { "@mariozechner/tui": _bundledTui, "@mariozechner/pi-ai": _bundledAi, "@mariozechner/pi-ai/oauth": _bundledAiOauth, - "@mariozechner/coding-agent": _bundledCodingAgent, + // "@mariozechner/coding-agent" / "@singularity-forge/coding-agent" + // self-references intentionally omitted — see top-of-file note. }; /** Modules available to extensions via virtualModules (for compiled Bun binary) */ diff --git a/scripts/check-circular-deps.mjs b/scripts/check-circular-deps.mjs index 3503f0b46..2e04a3529 100644 --- a/scripts/check-circular-deps.mjs +++ b/scripts/check-circular-deps.mjs @@ -159,7 +159,32 @@ function extractImports(filePath) { ); const specs = []; + // Track function-body depth so we can skip imports/requires that run only + // when their enclosing function is called (i.e. not at module evaluation + // time). These cannot cause module-graph cycles for the same reason + // dynamic `await import()` cannot. Counts cover FunctionDeclaration, + // FunctionExpression, ArrowFunction, MethodDeclaration, accessors, and + // class member functions. + let functionDepth = 0; + const FN_KINDS = new Set([ + ts.SyntaxKind.FunctionDeclaration, + ts.SyntaxKind.FunctionExpression, + ts.SyntaxKind.ArrowFunction, + ts.SyntaxKind.MethodDeclaration, + ts.SyntaxKind.GetAccessor, + ts.SyntaxKind.SetAccessor, + ts.SyntaxKind.Constructor, + ]); const visit = (node) => { + const isFn = FN_KINDS.has(node.kind); + if (isFn) functionDepth += 1; + try { + visitNode(node); + } finally { + if (isFn) functionDepth -= 1; + } + }; + const visitNode = (node) => { // import X from "..." | import "..." | import { X } from "..." // // Skip top-level type-only imports (`import type { X } from "..."`) — @@ -199,8 +224,11 @@ function extractImports(filePath) { // cause initialization-order cycles. Madge's prior config used // `skipAsyncImports: true` for the same reason — matching that // here so we don't false-positive on intentional lazy seams. - // CommonJS require("...") + // CommonJS require("...") — only count top-level requires; require() + // inside a function body runs lazily on call, same semantics as + // dynamic `await import()` and cannot cause module-graph cycles. if ( + functionDepth === 0 && ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === "require" &&