[subgraph] Extract cycle detection to shared utility

- Create src/subgraph/utils/cycleDetection.ts with validation functions
- Add validateNoSubgraphCycles and validateSubgraphAddition utilities
- Integrate cycle detection into LGraph.convertToSubgraph()
- Add comprehensive tests for cycle detection functionality
- Prevent creating subgraphs that would cause infinite recursion

Author: christian-byrne

src/LGraph.ts

@@ -37,6 +37,7 @@ import { type GraphOrSubgraph, Subgraph } from "./subgraph/Subgraph"
 import { SubgraphInput } from "./subgraph/SubgraphInput"
 import { SubgraphOutput } from "./subgraph/SubgraphOutput"
 import { getBoundaryLinks, groupResolvedByOutput, mapSubgraphInputsAndLinks, mapSubgraphOutputsAndLinks, multiClone, splitPositionables } from "./subgraph/subgraphUtils"
+import { validateNoSubgraphCycles } from "./subgraph/utils/cycleDetection"
 import { Alignment, LGraphEventMode } from "./types/globalEnums"
 import { getAllNestedItems } from "./utils/collections"
 
@@ -1412,6 +1413,9 @@ export class LGraph implements LinkNetwork, BaseLGraph, Serialisable<Serialisabl
 
     const clonedNodes = multiClone(nodes)
 
+    // Validate that we won't create circular references
+    validateNoSubgraphCycles(Array.from(nodes), this)
+
     // Inputs, outputs, and links
     const links = internalLinks.map(x => x.asSerialisable())
     const inputs = mapSubgraphInputsAndLinks(resolvedInputLinks, links)

src/subgraph/utils/cycleDetection.ts

@@ -0,0 +1,64 @@
+import type { LGraphNode } from "@/LGraphNode"
+import type { SubgraphNode } from "@/subgraph/SubgraphNode"
+
+import { RecursionError } from "@/infrastructure/RecursionError"
+
+/**
+ * Validates that a set of nodes doesn't contain circular references during subgraph creation.
+ * This prevents creating subgraphs that would cause infinite recursion during execution.
+ * @param nodes Nodes to check for cycles
+ * @param targetSubgraph Optional: the subgraph these nodes will be added to
+ * @throws RecursionError if cycle detected
+ */
+export function validateNoSubgraphCycles(
+  nodes: LGraphNode[],
+  targetSubgraph?: any, // Using any to avoid circular dependency
+): void {
+  // Check if any of the nodes being added is a subgraph that contains the target
+  for (const node of nodes) {
+    if (node.isSubgraphNode?.()) {
+      const subgraphNode = node as SubgraphNode
+      if (targetSubgraph && containsSubgraph(subgraphNode.subgraph, targetSubgraph)) {
+        throw new RecursionError(
+          `Circular reference detected: Cannot add node ${node.id} (${node.title || "untitled"}) ` +
+          `because its subgraph would contain its parent subgraph`,
+        )
+      }
+    }
+  }
+}
+
+/**
+ * Checks if a subgraph contains another subgraph (directly or indirectly).
+ * @param container The potential container subgraph
+ * @param target The target subgraph to search for
+ * @returns true if container contains target
+ */
+function containsSubgraph(container: any, target: any): boolean {
+  if (container === target) return true
+
+  // Check all nodes in the container
+  for (const node of container.nodes.values()) {
+    if (node.isSubgraphNode?.()) {
+      const subgraphNode = node as SubgraphNode
+      if (subgraphNode.subgraph === target || containsSubgraph(subgraphNode.subgraph, target)) {
+        return true
+      }
+    }
+  }
+
+  return false
+}
+
+/**
+ * Validates that adding nodes to a subgraph won't create cycles.
+ * @param nodes Nodes being added
+ * @param targetSubgraph The subgraph they're being added to
+ * @throws RecursionError if adding would create a cycle
+ */
+export function validateSubgraphAddition(
+  nodes: LGraphNode[],
+  targetSubgraph: any,
+): void {
+  validateNoSubgraphCycles(nodes, targetSubgraph)
+}

test/LGraph.convertToSubgraph.test.ts

@@ -0,0 +1,75 @@
+/**
+ * LGraph.convertToSubgraph Tests
+ *
+ * Tests for converting nodes to subgraphs with cycle detection.
+ */
+
+import { describe, expect, it } from "vitest"
+
+import { RecursionError } from "@/infrastructure/RecursionError"
+import { LGraph } from "@/litegraph"
+
+import { createTestSubgraph, createTestSubgraphNode } from "./subgraph/fixtures/subgraphHelpers"
+
+describe("LGraph.convertToSubgraph", () => {
+  describe("cycle detection", () => {
+    it("should prevent creating subgraphs that would contain themselves", () => {
+      const parentGraph = new LGraph()
+
+      // Create a subgraph
+      const childSubgraph = createTestSubgraph({ name: "Child" })
+      const subgraphNode = createTestSubgraphNode(childSubgraph)
+      parentGraph.add(subgraphNode)
+
+      // Try to convert the subgraph node back into the child subgraph
+      // This would create childSubgraph -> subgraphNode -> childSubgraph
+      const nodesToConvert = new Set([subgraphNode])
+
+      expect(() => {
+        // This should be caught by our cycle detection
+        childSubgraph.convertToSubgraph(nodesToConvert)
+      }).toThrow(RecursionError)
+
+      expect(() => {
+        childSubgraph.convertToSubgraph(nodesToConvert)
+      }).toThrow(/Circular reference detected.*because its subgraph would contain its parent subgraph/)
+    })
+
+    it("should prevent complex nested cycles", () => {
+      const rootGraph = new LGraph()
+
+      // Create subgraph A
+      const subgraphA = createTestSubgraph({ name: "Subgraph A" })
+
+      // Create subgraph B that contains A
+      const subgraphB = createTestSubgraph({ name: "Subgraph B" })
+      const nodeAinB = createTestSubgraphNode(subgraphA)
+      subgraphB.add(nodeAinB)
+
+      // Add B to root
+      const nodeBinRoot = createTestSubgraphNode(subgraphB)
+      rootGraph.add(nodeBinRoot)
+
+      // Now try to convert nodeBinRoot into subgraphA
+      // This would create A -> B -> A cycle
+      const nodesToConvert = new Set([nodeBinRoot])
+
+      expect(() => {
+        subgraphA.convertToSubgraph(nodesToConvert)
+      }).toThrow(RecursionError)
+    })
+
+    it("should allow valid nodes when no cycles exist", () => {
+      const parentGraph = new LGraph()
+
+      // Create a subgraph that has no relationship to parentGraph
+      const independentSubgraph = createTestSubgraph({ name: "Independent" })
+      const subgraphNode = createTestSubgraphNode(independentSubgraph)
+
+      // This should not throw because there's no cycle
+      expect(() => {
+        parentGraph.convertToSubgraph(new Set([subgraphNode]))
+      }).not.toThrow(RecursionError)
+    })
+  })
+})

test/subgraph/SubgraphMemory.test.ts

@@ -1,11 +1,9 @@
 import { describe, expect, it, vi } from "vitest"
 
 import { LGraph } from "@/litegraph"
-import { SubgraphNode } from "@/subgraph/SubgraphNode"
 
 import { subgraphTest } from "./fixtures/subgraphFixtures"
 import {
-  createEventCapture,
   createTestSubgraph,
   createTestSubgraphNode,
 } from "./fixtures/subgraphHelpers"
@@ -427,4 +425,4 @@ describe("SubgraphMemory - Performance and Scale", () => {
       expect(rootGraph.nodes.length).toBe(0)
     }
   })
-})
+})

test/subgraph/utils/cycleDetection.test.ts

@@ -0,0 +1,111 @@
+/**
+ * Cycle Detection Utility Tests
+ *
+ * Tests for cycle detection during subgraph creation to prevent
+ * infinite recursion issues.
+ */
+
+import { describe, expect, it } from "vitest"
+
+import { RecursionError } from "@/infrastructure/RecursionError"
+import { LGraph, LGraphNode } from "@/litegraph"
+import { validateNoSubgraphCycles, validateSubgraphAddition } from "@/subgraph/utils/cycleDetection"
+
+import { createTestSubgraph, createTestSubgraphNode } from "../fixtures/subgraphHelpers"
+
+describe("Cycle Detection Utilities", () => {
+  describe("validateNoSubgraphCycles", () => {
+    it("should allow adding regular nodes without issues", () => {
+      const node1 = new LGraphNode("Test Node 1")
+      const node2 = new LGraphNode("Test Node 2")
+
+      expect(() => {
+        validateNoSubgraphCycles([node1, node2])
+      }).not.toThrow()
+    })
+
+    it("should allow adding subgraph nodes that don't create cycles", () => {
+      const parentGraph = new LGraph()
+      const childSubgraph = createTestSubgraph({ name: "Child Subgraph" })
+      const subgraphNode = createTestSubgraphNode(childSubgraph)
+
+      expect(() => {
+        validateNoSubgraphCycles([subgraphNode], parentGraph)
+      }).not.toThrow()
+    })
+
+    it("should detect direct self-reference cycles", () => {
+      const subgraph = createTestSubgraph({ name: "Self Reference" })
+      const subgraphNode = createTestSubgraphNode(subgraph)
+
+      expect(() => {
+        validateNoSubgraphCycles([subgraphNode], subgraph)
+      }).toThrow(RecursionError)
+
+      expect(() => {
+        validateNoSubgraphCycles([subgraphNode], subgraph)
+      }).toThrow(/Circular reference detected.*because its subgraph would contain its parent subgraph/)
+    })
+
+    it("should detect indirect cycles through nested subgraphs", () => {
+      // Create subgraph A
+      const subgraphA = createTestSubgraph({ name: "Subgraph A" })
+
+      // Create subgraph B that contains A
+      const subgraphB = createTestSubgraph({ name: "Subgraph B" })
+      const nodeAinB = createTestSubgraphNode(subgraphA)
+      subgraphB.add(nodeAinB)
+
+      // Now try to add B to A (would create A -> B -> A cycle)
+      const nodeBinA = createTestSubgraphNode(subgraphB)
+
+      expect(() => {
+        validateNoSubgraphCycles([nodeBinA], subgraphA)
+      }).toThrow(RecursionError)
+    })
+
+    it("should handle mixed node types correctly", () => {
+      const parentGraph = new LGraph()
+      const subgraph = createTestSubgraph({ name: "Mixed Test" })
+
+      const regularNode = new LGraphNode("Regular Node")
+      const subgraphNode = createTestSubgraphNode(subgraph)
+
+      // Should allow mixed nodes when no cycle exists
+      expect(() => {
+        validateNoSubgraphCycles([regularNode, subgraphNode], parentGraph)
+      }).not.toThrow()
+    })
+
+    it("should include node information in error messages", () => {
+      const subgraph = createTestSubgraph({ name: "Error Test" })
+      const subgraphNode = createTestSubgraphNode(subgraph)
+      subgraphNode.id = 123
+      subgraphNode.title = "My Subgraph Instance"
+
+      expect(() => {
+        validateNoSubgraphCycles([subgraphNode], subgraph)
+      }).toThrow(/Cannot add node 123 \(My Subgraph Instance\)/)
+    })
+  })
+
+  describe("validateSubgraphAddition", () => {
+    it("should validate nodes being added to a subgraph", () => {
+      const targetSubgraph = createTestSubgraph({ name: "Target" })
+      const node = new LGraphNode()
+
+      expect(() => {
+        validateSubgraphAddition([node], targetSubgraph)
+      }).not.toThrow()
+    })
+
+    it("should detect cycles when adding to subgraph", () => {
+      const subgraph = createTestSubgraph({ name: "Cyclic" })
+      const subgraphNode = createTestSubgraphNode(subgraph)
+
+      expect(() => {
+        validateSubgraphAddition([subgraphNode], subgraph)
+      }).toThrow(RecursionError)
+    })
+  })
+})