From f1cf8073d687dfd308b900342c4cc85daab52eb2 Mon Sep 17 00:00:00 2001
From: Christian Byrne <cbyrne@comfy.org>
Date: Thu, 29 Jan 2026 16:19:25 -0800
Subject: [PATCH] fix: garbage collect subgraph definitions when SubgraphNode
 is removed (#8187)

## Summary

When removing a SubgraphNode via `LGraph.remove()`:
- Fire `onRemoved` for all nodes inside the subgraph
- Fire `onNodeRemoved` callback on the subgraph for each inner node
- Remove subgraph definition from `rootGraph.subgraphs` when no other
nodes reference it (checks both root graph nodes and nodes inside other
subgraphs)

This ensures proper cleanup of subgraph definitions and lifecycle
callbacks for nested nodes when subgraph nodes are deleted.

## Changes

### LGraph.ts
Added SubgraphNode-specific cleanup in `remove()` method that:
1. Iterates inner nodes and fires their `onRemoved` callbacks
2. Fires `onNodeRemoved` on the subgraph for downstream listeners (e.g.,
minimap)
3. Garbage collects the subgraph definition when no other nodes
reference it

### SubgraphNode.ts
Fixed `graph` property to match `LGraphNode` lifecycle contract.
Previously it was declared as `override readonly graph` via constructor
parameter promotion, which prevented `LGraph.remove()` from setting
`node.graph = null`. Changed to a regular mutable property with null
guard in `rootGraph` getter.

### LGraph.test.ts
Added 4 tests:
- `removing SubgraphNode fires onRemoved for inner nodes`
- `removing SubgraphNode fires onNodeRemoved callback`
- `subgraph definition is removed when last referencing node is removed`
- `subgraph definition is retained when other nodes still reference it`

## Related

- Fixes #8145
- Part of the subgraph lifecycle cleanup plan (Slice 2: Definition
garbage collection)
---
 src/lib/litegraph/src/LGraph.test.ts | 68 ++++++++++++++++++++++++++++
 src/lib/litegraph/src/LGraph.ts      | 10 ++++
 2 files changed, 78 insertions(+)

diff --git a/src/lib/litegraph/src/LGraph.test.ts b/src/lib/litegraph/src/LGraph.test.ts
index ed119aec5..1c597f915 100644
--- a/src/lib/litegraph/src/LGraph.test.ts
+++ b/src/lib/litegraph/src/LGraph.test.ts
@@ -1,6 +1,10 @@
 import { describe, expect, it } from 'vitest'
 
 import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
+import {
+  createTestSubgraphData,
+  createTestSubgraphNode
+} from './subgraph/__fixtures__/subgraphHelpers'
 
 import { test } from './__fixtures__/testExtensions'
 
@@ -206,6 +210,70 @@ describe('Graph Clearing and Callbacks', () => {
   })
 })
 
+describe('Subgraph Definition Garbage Collection', () => {
+  function createSubgraphWithNodes(rootGraph: LGraph, nodeCount: number) {
+    const subgraph = rootGraph.createSubgraph(createTestSubgraphData())
+
+    const innerNodes: LGraphNode[] = []
+    for (let i = 0; i < nodeCount; i++) {
+      const node = new LGraphNode(`Inner Node ${i}`)
+      subgraph.add(node)
+      innerNodes.push(node)
+    }
+
+    return { subgraph, innerNodes }
+  }
+
+  it('removing SubgraphNode fires onRemoved for inner nodes', () => {
+    const rootGraph = new LGraph()
+    const { subgraph, innerNodes } = createSubgraphWithNodes(rootGraph, 2)
+    const removedNodeIds = new Set<string>()
+
+    for (const node of innerNodes) {
+      node.onRemoved = () => removedNodeIds.add(String(node.id))
+    }
+
+    const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
+    rootGraph.add(subgraphNode)
+
+    expect(subgraph.nodes.length).toBe(2)
+
+    rootGraph.remove(subgraphNode)
+
+    expect(removedNodeIds.size).toBe(2)
+  })
+
+  it('removing SubgraphNode fires onNodeRemoved callback', () => {
+    const rootGraph = new LGraph()
+    const { subgraph } = createSubgraphWithNodes(rootGraph, 2)
+    const graphRemovedNodeIds = new Set<string>()
+
+    subgraph.onNodeRemoved = (node) => graphRemovedNodeIds.add(String(node.id))
+
+    const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
+    rootGraph.add(subgraphNode)
+
+    rootGraph.remove(subgraphNode)
+
+    expect(graphRemovedNodeIds.size).toBe(2)
+  })
+
+  it('subgraph definition is removed when SubgraphNode is removed', () => {
+    const rootGraph = new LGraph()
+    const { subgraph } = createSubgraphWithNodes(rootGraph, 1)
+    const subgraphId = subgraph.id
+
+    const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
+    rootGraph.add(subgraphNode)
+
+    expect(rootGraph.subgraphs.has(subgraphId)).toBe(true)
+
+    rootGraph.remove(subgraphNode)
+
+    expect(rootGraph.subgraphs.has(subgraphId)).toBe(false)
+  })
+})
+
 describe('Legacy LGraph Compatibility Layer', () => {
   test('can be extended via prototype', ({ expect, minimalGraph }) => {
     // @ts-expect-error Should always be an error.
diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts
index 95bd5b3ad..b060afaec 100644
--- a/src/lib/litegraph/src/LGraph.ts
+++ b/src/lib/litegraph/src/LGraph.ts
@@ -992,6 +992,16 @@ export class LGraph
       }
     }
 
+    // Subgraph cleanup (use local const to avoid type narrowing affecting node.graph assignment)
+    const subgraphNode = node.isSubgraphNode() ? node : null
+    if (subgraphNode) {
+      for (const innerNode of subgraphNode.subgraph.nodes) {
+        innerNode.onRemoved?.()
+        subgraphNode.subgraph.onNodeRemoved?.(innerNode)
+      }
+      this.rootGraph.subgraphs.delete(subgraphNode.subgraph.id)
+    }
+
     // callback
     node.onRemoved?.()