Fix disconnection of subgraphInput links (#6258)

`LLink.disconnect` is intended to cleanup only the link itself. #4800
mistakenly assumed that it would perform all required steps for
disconnection. Later, #5015 would partially resolve this by adding some
of the missing functionality into `LLink.disconnect`, but this still
left output cleanup unhandled and failed to call
`node.onConnectionsChanged`.

This PR instead moves the disconnection code to call the function that
already has robust handling for these items and removes the
no-longer-needed and potentially misleading workaround.

Resolves #6247

Also un-skipped several SubgraphIO tests. They appear to function fine.
I'm assuming the reasons for them being skipped have been resolved.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6258-Fix-disconnection-of-subgraphInput-links-2966d73d36508112ad1fe602cdcf461b)
by [Unito](https://www.unito.io)

Author: AustinMroz

src/lib/litegraph/src/LLink.ts

@@ -418,18 +418,6 @@ export class LLink implements LinkSegment, Serialisable<SerialisableLLink> {
    * If `input` or `output`, reroutes will not be automatically removed, and retain a connection to the input or output, respectively.
    */
   disconnect(network: LinkNetwork, keepReroutes?: 'input' | 'output'): void {
-    // Clean up the target node's input slot
-    if (this.target_id !== -1) {
-      const targetNode = network.getNodeById(this.target_id)
-      if (targetNode) {
-        const targetInput = targetNode.inputs?.[this.target_slot]
-        if (targetInput && targetInput.link === this.id) {
-          targetInput.link = null
-          targetNode.setDirtyCanvas?.(true, false)
-        }
-      }
-    }
-
     const reroutes = LLink.getReroutes(network, this)
 
     const lastReroute = reroutes.at(-1)

src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts

@@ -137,7 +137,9 @@ export class ToInputFromIoNodeLink implements RenderLink {
   }
   disconnect(): boolean {
     if (!this.existingLink) return false
-    this.existingLink.disconnect(this.network, 'input')
+    const { input, inputNode } = this.existingLink.resolve(this.network)
+    if (!inputNode || !input) return false
+    this.node._disconnectNodeInput(inputNode, input, this.existingLink)
     return true
   }
 }

tests-ui/tests/litegraph/core/LLink.test.ts

@@ -1,6 +1,6 @@
-import { describe, expect, it, vi } from 'vitest'
+import { describe, expect } from 'vitest'
 
-import { LGraph, LGraphNode, LLink } from '@/lib/litegraph/src/litegraph'
+import { LLink } from '@/lib/litegraph/src/litegraph'
 
 import { test } from './fixtures/testExtensions'
 
@@ -14,84 +14,4 @@ describe('LLink', () => {
     const link = new LLink(1, 'float', 4, 2, 5, 3)
     expect(link.serialize()).toMatchSnapshot('Basic')
   })
-
-  describe('disconnect', () => {
-    it('should clear the target input link reference when disconnecting', () => {
-      // Create a graph and nodes
-      const graph = new LGraph()
-      const sourceNode = new LGraphNode('Source')
-      const targetNode = new LGraphNode('Target')
-
-      // Add nodes to graph
-      graph.add(sourceNode)
-      graph.add(targetNode)
-
-      // Add slots
-      sourceNode.addOutput('out', 'number')
-      targetNode.addInput('in', 'number')
-
-      // Connect the nodes
-      const link = sourceNode.connect(0, targetNode, 0)
-      expect(link).toBeDefined()
-      expect(targetNode.inputs[0].link).toBe(link?.id)
-
-      // Mock setDirtyCanvas
-      const setDirtyCanvasSpy = vi.spyOn(targetNode, 'setDirtyCanvas')
-
-      // Disconnect the link
-      link?.disconnect(graph)
-
-      // Verify the target input's link reference is cleared
-      expect(targetNode.inputs[0].link).toBeNull()
-
-      // Verify setDirtyCanvas was called
-      expect(setDirtyCanvasSpy).toHaveBeenCalledWith(true, false)
-    })
-
-    it('should handle disconnecting when target node is not found', () => {
-      // Create a link with invalid target
-      const graph = new LGraph()
-      const link = new LLink(1, 'number', 1, 0, 999, 0) // Invalid target id
-
-      // Should not throw when disconnecting
-      expect(() => link.disconnect(graph)).not.toThrow()
-    })
-
-    it('should only clear link reference if it matches the current link id', () => {
-      // Create a graph and nodes
-      const graph = new LGraph()
-      const sourceNode1 = new LGraphNode('Source1')
-      const sourceNode2 = new LGraphNode('Source2')
-      const targetNode = new LGraphNode('Target')
-
-      // Add nodes to graph
-      graph.add(sourceNode1)
-      graph.add(sourceNode2)
-      graph.add(targetNode)
-
-      // Add slots
-      sourceNode1.addOutput('out', 'number')
-      sourceNode2.addOutput('out', 'number')
-      targetNode.addInput('in', 'number')
-
-      // Create first connection
-      const link1 = sourceNode1.connect(0, targetNode, 0)
-      expect(link1).toBeDefined()
-
-      // Disconnect first connection
-      targetNode.disconnectInput(0)
-
-      // Create second connection
-      const link2 = sourceNode2.connect(0, targetNode, 0)
-      expect(link2).toBeDefined()
-      expect(targetNode.inputs[0].link).toBe(link2?.id)
-
-      // Try to disconnect the first link (which is already disconnected)
-      // It should not affect the current connection
-      link1?.disconnect(graph)
-
-      // The input should still have the second link
-      expect(targetNode.inputs[0].link).toBe(link2?.id)
-    })
-  })
 })

tests-ui/tests/litegraph/subgraph/SubgraphIO.test.ts

@@ -2,14 +2,16 @@
 import { describe, expect, it } from 'vitest'
 
 import { LGraphNode } from '@/lib/litegraph/src/litegraph'
+import { ToInputFromIoNodeLink } from '@/lib/litegraph/src/canvas/ToInputFromIoNodeLink'
+import { LinkDirection } from '@/lib/litegraph/src//types/globalEnums'
 
 import { subgraphTest } from './fixtures/subgraphFixtures'
 import {
   createTestSubgraph,
   createTestSubgraphNode
 } from './fixtures/subgraphHelpers'
 
-describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
+describe('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
   subgraphTest(
     'input accepts external connections from parent graph',
     ({ subgraphWithNode }) => {
@@ -77,6 +79,31 @@ describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
     }
   )
 
+  subgraphTest('handles link disconnection', ({ subgraphWithNode }) => {
+    const { subgraph } = subgraphWithNode
+    const internalNode = new LGraphNode('External Source')
+    internalNode.addInput('in', '*')
+    internalNode.onConnectionsChange = vi.fn()
+    subgraph.add(internalNode)
+
+    const link = subgraph.inputNode.slots[0].connect(
+      internalNode.inputs[0],
+      internalNode
+    )
+    new ToInputFromIoNodeLink(
+      subgraph,
+      subgraph.inputNode,
+      subgraph.inputNode.slots[0],
+      undefined,
+      LinkDirection.CENTER,
+      link
+    ).disconnect()
+
+    expect(internalNode.inputs[0].link).toBeNull()
+    expect(subgraph.inputNode.slots[0].linkIds.length).toBe(0)
+    expect(internalNode.onConnectionsChange).toHaveBeenCalled()
+  })
+
   subgraphTest(
     'handles slot renaming with active connections',
     ({ subgraphWithNode }) => {
@@ -103,7 +130,7 @@ describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
   )
 })
 
-describe.skip('SubgraphIO - Output Slot Dual-Nature Behavior', () => {
+describe('SubgraphIO - Output Slot Dual-Nature Behavior', () => {
   subgraphTest(
     'output provides connections to parent graph',
     ({ subgraphWithNode }) => {
@@ -199,7 +226,7 @@ describe.skip('SubgraphIO - Output Slot Dual-Nature Behavior', () => {
   )
 })
 
-describe.skip('SubgraphIO - Boundary Connection Management', () => {
+describe('SubgraphIO - Boundary Connection Management', () => {
   subgraphTest(
     'verifies cross-boundary link resolution',
     ({ complexSubgraph }) => {
@@ -309,7 +336,7 @@ describe.skip('SubgraphIO - Boundary Connection Management', () => {
   )
 })
 
-describe.skip('SubgraphIO - Advanced Scenarios', () => {
+describe('SubgraphIO - Advanced Scenarios', () => {
   it('handles multiple inputs and outputs with complex connections', () => {
     const subgraph = createTestSubgraph({
       name: 'Complex IO Test',
@@ -399,7 +426,7 @@ describe.skip('SubgraphIO - Advanced Scenarios', () => {
   })
 })
 
-describe.skip('SubgraphIO - Empty Slot Connection', () => {
+describe('SubgraphIO - Empty Slot Connection', () => {
   subgraphTest(
     'creates new input and connects when dragging from empty slot inside subgraph',
     ({ subgraphWithNode }) => {