From 871715113cfb0cefa23ac8df70ab056309a5fbfa Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Fri, 13 Mar 2026 08:35:27 -0700 Subject: [PATCH] fix: detect and remove duplicate links in subgraph unpacking (#9120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix duplicate LLink objects created during subgraph unpacking, where output.links contains multiple link IDs for the same connection but input.link only references one, leaving orphaned links. ## Changes - **What**: Three layers of defense against duplicate links: 1. **Serialization fix** (`slotUtils.ts`): Clone `output.links` array in `outputAsSerialisable` to prevent shared-reference mutation during serialization round-trips 2. **Self-healing** (`LGraph.ts`): `_removeDuplicateLinks()` sanitizes corrupted data during `configure()`, keeping the link referenced by `input.link` and removing orphaned duplicates from `output.links` and `_links` 3. **Unpack dedup** (`LGraph.ts`): Subgraph unpacking filters `newLinks` via a `seenLinks` Set before creating connections Runtime diagnostic logging via `graph.events` (no Sentry import in litegraph): - `_dupLinkIndex` Map for O(1) duplicate detection, only allocated when enabled - `_checkDuplicateLink()` called at the 3 link-creation sites (`connectSlots`, `SubgraphInput.connect`, `SubgraphOutput.connect`) - App layer listens for `diagnostic:duplicate-link` events and forwards to Sentry with rate-limiting (1 per key per 60s) ## Review Focus - The `_removeDuplicateLinks` strategy of keeping the link referenced by `input.link` and removing others from `output.links` + `_links` - The diagnostic index lifecycle: built on enable, updated on link create/remove, cleared on disable - Sentry integration in `app.ts` using the existing `graph.events` system to avoid coupling litegraph to Sentry ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9120-fix-detect-and-remove-duplicate-links-in-subgraph-unpacking-3106d73d3650815b995ddf8f41da67ae) by [Unito](https://www.unito.io) --- src/lib/litegraph/src/LGraph.test.ts | 135 +++++++++++++++++++ src/lib/litegraph/src/LGraph.ts | 59 +++++++- src/lib/litegraph/src/node/slotUtils.test.ts | 35 +++++ src/lib/litegraph/src/node/slotUtils.ts | 2 +- 4 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 src/lib/litegraph/src/node/slotUtils.test.ts diff --git a/src/lib/litegraph/src/LGraph.test.ts b/src/lib/litegraph/src/LGraph.test.ts index ee37d72ee8..0764959189 100644 --- a/src/lib/litegraph/src/LGraph.test.ts +++ b/src/lib/litegraph/src/LGraph.test.ts @@ -534,6 +534,141 @@ describe('ensureGlobalIdUniqueness', () => { }) }) +describe('_removeDuplicateLinks', () => { + class TestNode extends LGraphNode { + constructor(title?: string) { + super(title ?? 'TestNode') + this.addInput('input_0', 'number') + this.addOutput('output_0', 'number') + } + } + + function registerTestNodes() { + LiteGraph.registerNodeType('test/DupTestNode', TestNode) + } + + it('removes orphaned duplicate links from _links and output.links', () => { + registerTestNodes() + const graph = new LGraph() + + const source = LiteGraph.createNode('test/DupTestNode', 'Source')! + const target = LiteGraph.createNode('test/DupTestNode', 'Target')! + graph.add(source) + graph.add(target) + + source.connect(0, target, 0) + expect(graph._links.size).toBe(1) + + const existingLink = graph._links.values().next().value! + for (let i = 0; i < 3; i++) { + const dupLink = new LLink( + ++graph.state.lastLinkId, + existingLink.type, + existingLink.origin_id, + existingLink.origin_slot, + existingLink.target_id, + existingLink.target_slot + ) + graph._links.set(dupLink.id, dupLink) + source.outputs[0].links!.push(dupLink.id) + } + + expect(graph._links.size).toBe(4) + expect(source.outputs[0].links).toHaveLength(4) + + graph._removeDuplicateLinks() + + expect(graph._links.size).toBe(1) + expect(source.outputs[0].links).toHaveLength(1) + expect(target.inputs[0].link).toBe(source.outputs[0].links![0]) + }) + + it('keeps the link referenced by input.link', () => { + registerTestNodes() + const graph = new LGraph() + + const source = LiteGraph.createNode('test/DupTestNode', 'Source')! + const target = LiteGraph.createNode('test/DupTestNode', 'Target')! + graph.add(source) + graph.add(target) + + source.connect(0, target, 0) + const keptLinkId = target.inputs[0].link! + + const dupLink = new LLink( + ++graph.state.lastLinkId, + 'number', + source.id, + 0, + target.id, + 0 + ) + graph._links.set(dupLink.id, dupLink) + source.outputs[0].links!.push(dupLink.id) + + graph._removeDuplicateLinks() + + expect(graph._links.size).toBe(1) + expect(target.inputs[0].link).toBe(keptLinkId) + expect(graph._links.has(keptLinkId)).toBe(true) + expect(graph._links.has(dupLink.id)).toBe(false) + }) + + it('is a no-op when no duplicates exist', () => { + registerTestNodes() + const graph = new LGraph() + + const source = LiteGraph.createNode('test/DupTestNode', 'Source')! + const target = LiteGraph.createNode('test/DupTestNode', 'Target')! + graph.add(source) + graph.add(target) + + source.connect(0, target, 0) + const linksBefore = graph._links.size + + graph._removeDuplicateLinks() + + expect(graph._links.size).toBe(linksBefore) + }) + + it('cleans up duplicate links in subgraph during configure', () => { + const subgraphData = createTestSubgraphData() + const rootGraph = new LGraph() + const subgraph = rootGraph.createSubgraph(subgraphData) + + const source = new LGraphNode('Source') + source.addOutput('out', 'number') + const target = new LGraphNode('Target') + target.addInput('in', 'number') + subgraph.add(source) + subgraph.add(target) + + source.connect(0, target, 0) + expect(subgraph._links.size).toBe(1) + + const existingLink = subgraph._links.values().next().value! + for (let i = 0; i < 3; i++) { + const dup = new LLink( + ++subgraph.state.lastLinkId, + existingLink.type, + existingLink.origin_id, + existingLink.origin_slot, + existingLink.target_id, + existingLink.target_slot + ) + subgraph._links.set(dup.id, dup) + source.outputs[0].links!.push(dup.id) + } + expect(subgraph._links.size).toBe(4) + + // Serialize and reconfigure - should clean up during configure + const serialized = subgraph.asSerialisable() + subgraph.configure(serialized as never) + + expect(subgraph._links.size).toBe(1) + }) +}) + describe('Subgraph Unpacking', () => { class TestNode extends LGraphNode { constructor(title?: string) { diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index dcf5e0c776..7565276e8c 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -164,6 +164,11 @@ export class LGraph static STATUS_STOPPED = 1 static STATUS_RUNNING = 2 + /** Generates a unique string key for a link's connection tuple. */ + static _linkTupleKey(link: LLink): string { + return `${link.origin_id}\0${link.origin_slot}\0${link.target_id}\0${link.target_slot}` + } + /** List of LGraph properties that are manually handled by {@link LGraph.configure}. */ static readonly ConfigureProperties = new Set([ 'nodes', @@ -1611,6 +1616,52 @@ export class LGraph link.disconnect(this) } + /** + * Removes duplicate links that share the same connection tuple + * (origin_id, origin_slot, target_id, target_slot). Keeps the link + * referenced by input.link and removes orphaned duplicates from + * output.links and the graph's _links map. + */ + _removeDuplicateLinks(): void { + const seen = new Map() + const toRemove: LinkId[] = [] + + for (const [id, link] of this._links) { + const key = LGraph._linkTupleKey(link) + if (seen.has(key)) { + const existingId = seen.get(key)! + // Keep the link that the input side references + const node = this.getNodeById(link.target_id) + const input = node?.inputs?.[link.target_slot] + if (input?.link === id) { + toRemove.push(existingId) + seen.set(key, id) + } else { + toRemove.push(id) + } + } else { + seen.set(key, id) + } + } + + for (const id of toRemove) { + const link = this._links.get(id) + if (!link) continue + + // Remove from origin node's output.links array + const originNode = this.getNodeById(link.origin_id) + if (originNode) { + const output = originNode.outputs?.[link.origin_slot] + if (output?.links) { + const idx = output.links.indexOf(id) + if (idx !== -1) output.links.splice(idx, 1) + } + } + + this._links.delete(id) + } + } + /** * Creates a new subgraph definition, and adds it to the graph. * @param data Exported data (typically serialised) to configure the new subgraph with @@ -2072,7 +2123,7 @@ export class LGraph // disconnect/reconnect cycles on widget inputs that can shift slot indices. const seenLinks = new Set() const dedupedNewLinks = newLinks.filter((link) => { - const key = `${link.oid}:${link.oslot}:${link.tid}:${link.tslot}` + const key = `${link.oid}\0${link.oslot}\0${link.tid}\0${link.tslot}` if (seenLinks.has(key)) return false seenLinks.add(key) return true @@ -2568,6 +2619,12 @@ export class LGraph } } + // Remove duplicate links: links in output.links that share the same + // (origin_id, origin_slot, target_id, target_slot) tuple. + // This repairs corrupted data where extra link objects were created + // without proper cleanup of the previous connection. + this._removeDuplicateLinks() + // groups this._groups.length = 0 const groupData = data.groups diff --git a/src/lib/litegraph/src/node/slotUtils.test.ts b/src/lib/litegraph/src/node/slotUtils.test.ts new file mode 100644 index 0000000000..38da4f1f59 --- /dev/null +++ b/src/lib/litegraph/src/node/slotUtils.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' + +import type { INodeOutputSlot } from '@/lib/litegraph/src/interfaces' +import type { IWidget } from '@/lib/litegraph/src/litegraph' +import { LGraphNode } from '@/lib/litegraph/src/litegraph' + +import { outputAsSerialisable } from './slotUtils' + +type OutputSlotParam = INodeOutputSlot & { widget?: IWidget } + +describe('outputAsSerialisable', () => { + it('clones the links array to prevent shared reference mutation', () => { + const node = new LGraphNode('test') + const output = node.addOutput('out', 'number') + output.links = [1, 2, 3] + + const serialised = outputAsSerialisable(output as OutputSlotParam) + + expect(serialised.links).toEqual([1, 2, 3]) + expect(serialised.links).not.toBe(output.links) + + // Mutating the live array should NOT affect the serialised copy + output.links.push(4) + expect(serialised.links).toHaveLength(3) + }) + + it('preserves null links', () => { + const node = new LGraphNode('test') + const output = node.addOutput('out', 'number') + output.links = null + + const serialised = outputAsSerialisable(output as OutputSlotParam) + expect(serialised.links).toBeNull() + }) +}) diff --git a/src/lib/litegraph/src/node/slotUtils.ts b/src/lib/litegraph/src/node/slotUtils.ts index 712637040a..fc0ea02094 100644 --- a/src/lib/litegraph/src/node/slotUtils.ts +++ b/src/lib/litegraph/src/node/slotUtils.ts @@ -74,7 +74,7 @@ export function outputAsSerialisable( ...outputWidget, pos, slot_index, - links + links: links ? [...links] : links } }