From 3818b7ce571623a917f3acb6742a62e95c17db00 Mon Sep 17 00:00:00 2001 From: Comfy Org PR Bot Date: Sat, 16 Aug 2025 04:36:48 +0800 Subject: [PATCH] Fix widget disconnection issue in subgraphs #4922 (#5015) (#5019) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [bugfix] Fix widget disconnection issue in subgraphs When disconnecting a node from a SubgraphInput, the target input's link reference was not being cleared in LLink.disconnect(). This caused widgets to remain greyed out because they still thought they were connected (slot.link was not null). The fix ensures that when a link is disconnected, the target node's input slot is properly cleaned up by setting input.link = null. Fixes #4922 🤖 Generated with [Claude Code](https://claude.ai/code) * [test] Add tests for LLink disconnect fix for widget issue Add comprehensive tests for the LLink.disconnect() method to verify that target input link references are properly cleared when disconnecting. This prevents widgets from remaining greyed out after disconnection. Tests cover: - Basic disconnect functionality with link reference cleanup - Edge cases with invalid target nodes - Preventing interference between different connections Related to #4922 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Christian Byrne Co-authored-by: Claude --- src/lib/litegraph/src/LLink.ts | 12 ++++ src/lib/litegraph/test/LLink.test.ts | 84 +++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/lib/litegraph/src/LLink.ts b/src/lib/litegraph/src/LLink.ts index b8de0f9304..bd36719edf 100644 --- a/src/lib/litegraph/src/LLink.ts +++ b/src/lib/litegraph/src/LLink.ts @@ -414,6 +414,18 @@ export class LLink implements LinkSegment, Serialisable { * 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) diff --git a/src/lib/litegraph/test/LLink.test.ts b/src/lib/litegraph/test/LLink.test.ts index 58f2501fbf..3f0e38f557 100644 --- a/src/lib/litegraph/test/LLink.test.ts +++ b/src/lib/litegraph/test/LLink.test.ts @@ -1,6 +1,6 @@ -import { describe, expect } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { LLink } from '@/lib/litegraph/src/litegraph' +import { LGraph, LGraphNode, LLink } from '@/lib/litegraph/src/litegraph' import { test } from './testExtensions' @@ -14,4 +14,84 @@ 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) + }) + }) })