fix: keep the link the input references when deduplicating duplicate connections

Author: christian-byrne

src/lib/litegraph/src/LGraph.inputSlotRealign.test.ts

@@ -109,8 +109,8 @@ function shiftedNodesAndLinks(sourceId: number, targetId: number) {
 /**
  * As {@link shiftedNodesAndLinks}, but `in_a` carries a duplicate link: the
  * link registered first (id 3) is not the id the serialized input references
- * (id 4). Registration rejects 4, so realignment must follow
- * the rejected alias through to the registered link to correct its slot.
+ * (id 4). The input reference is authoritative, so registration keeps 4 and
+ * rejects 3; realignment must still correct the survivor's slot.
  */
 function duplicateDriftedNodesAndLinks(sourceId: number, targetId: number) {
   const base = shiftedNodesAndLinks(sourceId, targetId)
@@ -288,12 +288,18 @@ const LINK_BY_INPUT_NAME: Record<string, number> = {
   in_c: 2
 }
 
-function assertLinksRealigned(graph: LGraph, targetNodeId: NodeId) {
+function assertLinksRealigned(
+  graph: LGraph,
+  targetNodeId: NodeId,
+  overrides: Record<string, number> = {}
+) {
   const target = graph.getNodeById(targetNodeId)!
   const linkStore = useLinkStore()
 
   for (const [slot, input] of target.inputs.entries()) {
-    const expectedLinkId = toLinkId(LINK_BY_INPUT_NAME[input.name])
+    const expectedLinkId = toLinkId(
+      overrides[input.name] ?? LINK_BY_INPUT_NAME[input.name]
+    )
     const link = graph.links.get(expectedLinkId)!
 
     expect(link.target_slot, `link.target_slot for input ${input.name}`).toBe(
@@ -339,8 +345,8 @@ describe('LGraph.configure input slot realignment (#3348)', () => {
     const graph = new LGraph()
     graph.configure(savedWorkflow({ duplicate: true }))
 
-    expect(graph.links.has(toLinkId(4))).toBe(false)
-    assertLinksRealigned(graph, toNodeId(2))
+    expect(graph.links.has(toLinkId(3))).toBe(false)
+    assertLinksRealigned(graph, toNodeId(2), { in_a: 4 })
   })
 
   it('maps a rejected subgraph input fanout branch to its exact survivor', () => {
@@ -355,7 +361,7 @@ describe('LGraph.configure input slot realignment (#3348)', () => {
     graph.configure(workflow)
 
     const subgraph = graph.subgraphs.get(SUBGRAPH_ID)!
-    expect(subgraph.inputs[0].linkIds).toEqual([toLinkId(2)])
+    expect(subgraph.inputs[0].linkIds).toEqual([toLinkId(3)])
   })
 
   it('uses the first slot when one link is referenced by multiple inputs', () => {

src/lib/litegraph/src/__fixtures__/duplicateLinks.ts

@@ -141,6 +141,107 @@ export const conflictingOriginLinksRoot: SerialisableGraph = {
   ]
 }
 
+/**
+ * Root graph containing a SubgraphNode whose subgraph definition has two
+ * links from different origins into the same subgraph output boundary slot.
+ * The boundary slot's `linkIds` array lists both candidates, so the target
+ * side cannot name a single survivor and document order must win, with the
+ * dropped id pruned from `linkIds` rather than left dangling.
+ */
+export const conflictingOriginLinksSubgraphBoundary: SerialisableGraph = {
+  id: 'dd000000-0000-4000-8000-000000000005',
+  version: 1,
+  revision: 0,
+  state: {
+    lastNodeId: 1,
+    lastLinkId: 0,
+    lastGroupId: 0,
+    lastRerouteId: 0
+  },
+  nodes: [
+    {
+      id: 1,
+      type: 'dd222222-2222-4222-8222-222222222222',
+      pos: [0, 0],
+      size: [200, 100],
+      flags: {},
+      order: 0,
+      mode: 0,
+      properties: {}
+    }
+  ],
+  definitions: {
+    subgraphs: [
+      {
+        id: 'dd222222-2222-4222-8222-222222222222',
+        version: 1,
+        revision: 0,
+        state: {
+          lastNodeId: 2,
+          lastLinkId: 2,
+          lastGroupId: 0,
+          lastRerouteId: 0
+        },
+        name: 'Subgraph With Conflicting Boundary Links',
+        config: {},
+        inputNode: { id: SUBGRAPH_INPUT_ID, bounding: [0, 100, 120, 60] },
+        outputNode: { id: SUBGRAPH_OUTPUT_ID, bounding: [500, 100, 120, 60] },
+        inputs: [],
+        outputs: [
+          { id: 'output', name: 'output', type: 'number', linkIds: [1, 2] }
+        ],
+        widgets: [],
+        nodes: [
+          {
+            id: 1,
+            type: 'test/DupTestNode',
+            pos: [100, 100],
+            size: [200, 100],
+            flags: {},
+            order: 0,
+            mode: 0,
+            inputs: [{ name: 'input_0', type: 'number', link: null }],
+            outputs: [{ name: 'output_0', type: 'number', links: [1] }],
+            properties: {}
+          },
+          {
+            id: 2,
+            type: 'test/DupTestNode',
+            pos: [100, 300],
+            size: [200, 100],
+            flags: {},
+            order: 1,
+            mode: 0,
+            inputs: [{ name: 'input_0', type: 'number', link: null }],
+            outputs: [{ name: 'output_0', type: 'number', links: [2] }],
+            properties: {}
+          }
+        ],
+        groups: [],
+        links: [
+          {
+            id: 1,
+            origin_id: 1,
+            origin_slot: 0,
+            target_id: SUBGRAPH_OUTPUT_ID,
+            target_slot: 0,
+            type: 'number'
+          },
+          {
+            id: 2,
+            origin_id: 2,
+            origin_slot: 0,
+            target_id: SUBGRAPH_OUTPUT_ID,
+            target_slot: 0,
+            type: 'number'
+          }
+        ],
+        extra: {}
+      }
+    ]
+  }
+}
+
 /**
  * Root graph with slot-shifted duplicates. Target node has an extra input
  * (simulating widget-to-input conversion) that shifts the connected input

src/lib/litegraph/src/linkDeduplication.conflictingOrigins.test.ts

@@ -2,6 +2,7 @@ import { createTestingPinia } from '@pinia/testing'
 import { setActivePinia } from 'pinia'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
+import { SUBGRAPH_OUTPUT_ID } from '@/lib/litegraph/src/constants'
 import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
 import type { SerialisedLLinkArray } from '@/lib/litegraph/src/LLink'
 import type { SerialisableLLink } from '@/lib/litegraph/src/types/serialisation'
@@ -11,7 +12,10 @@ import { toLinkId } from '@/types/linkId'
 import { toNodeId } from '@/types/nodeId'
 import type { NodeId } from '@/types/nodeId'
 
-import { conflictingOriginLinksRoot } from './__fixtures__/duplicateLinks'
+import {
+  conflictingOriginLinksRoot,
+  conflictingOriginLinksSubgraphBoundary
+} from './__fixtures__/duplicateLinks'
 
 class DupTestNode extends LGraphNode {
   constructor(title?: string) {
@@ -71,15 +75,15 @@ describe('normalizeConfiguredTopology with conflicting origins (#15577)', () =>
     LiteGraph.registerNodeType('test/DupTestNode', DupTestNode)
   })
 
-  it.fails('keeps the link that input.link references', () => {
+  it('keeps the link that input.link references', () => {
     const graph = configureConflictingOrigins()
 
     expect(graph.getNodeById(toNodeId(3))?.getInputLink(0)?.origin_id).toBe(
       toNodeId(2)
     )
   })
 
-  it.fails('warns when a link is dropped in favour of a different origin', () => {
+  it('warns when a link is dropped in favour of a different origin', () => {
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
 
     configureConflictingOrigins()
@@ -97,7 +101,7 @@ describe('normalizeConfiguredTopology with conflicting origins (#15577)', () =>
     expect(graph.getNodeById(toNodeId(3))?.getInputLink(0)).toBeDefined()
   })
 
-  it.fails('re-saves the workflow without changing the upstream node', () => {
+  it('re-saves the workflow without changing the upstream node', () => {
     const graph = configureConflictingOrigins()
 
     const [survivor] = linksIntoTargetSlot(
@@ -110,6 +114,29 @@ describe('normalizeConfiguredTopology with conflicting origins (#15577)', () =>
   })
 })
 
+describe('conflicting origins at a subgraph output boundary (#15577)', () => {
+  beforeEach(() => {
+    setActivePinia(createTestingPinia({ stubActions: false }))
+    LiteGraph.registerNodeType('test/DupTestNode', DupTestNode)
+  })
+
+  it('keeps exactly one link and prunes the dropped id from linkIds', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+    const graph = new LGraph()
+    graph.configure(structuredClone(conflictingOriginLinksSubgraphBoundary))
+    const subgraph = graph.subgraphs.get(
+      'dd222222-2222-4222-8222-222222222222'
+    )!
+
+    expect(subgraph.links.size).toBe(1)
+    expect(subgraph.outputs[0].linkIds).toEqual([toLinkId(1)])
+    expect(warn.mock.calls.flat().join(' ')).toContain(
+      `${SUBGRAPH_OUTPUT_ID}:0`
+    )
+  })
+})
+
 describe('legacy mirror link creation (#15577 reachability)', () => {
   beforeEach(() => {
     setActivePinia(createTestingPinia({ stubActions: false }))

src/lib/litegraph/src/linkDeduplication.ts

@@ -54,36 +54,123 @@ export function remapLinkReferences(
   }
 }
 
+/**
+ * Removes serialized link ids from every list and scalar that names them.
+ * Used for links dropped because a *different* connection already owns the
+ * target slot: remapping would hand the loser's origin a link it does not own,
+ * so the reference must be deleted rather than repointed.
+ */
+export function pruneLinkReferences(
+  data: ConfiguredGraph,
+  dropped: ReadonlySet<number>
+): void {
+  if (!dropped.size) return
+  const nodes = data.nodes ?? []
+
+  for (const input of nodes.flatMap((node) => node.inputs ?? [])) {
+    if (input.link != null && dropped.has(input.link)) input.link = null
+  }
+
+  const linkIdLists = [
+    ...nodes.flatMap((node) =>
+      (node.outputs ?? []).map((output) => output.links)
+    ),
+    ...(data.inputs ?? []).map((slot) => slot.linkIds),
+    ...(data.outputs ?? []).map((slot) => slot.linkIds),
+    ...(data.reroutes ?? []).map((reroute) => reroute.linkIds)
+  ]
+  for (const ids of linkIdLists) {
+    if (!ids) continue
+    const kept = ids.filter((id) => !dropped.has(id))
+    if (kept.length !== ids.length) ids.splice(0, ids.length, ...kept)
+  }
+
+  if (data.extra?.linkExtensions) {
+    data.extra.linkExtensions = data.extra.linkExtensions.filter(
+      (extension) => !dropped.has(extension.id)
+    )
+  }
+}
+
+/**
+ * Maps each `target_id:target_slot` to the link id the *target side* of the
+ * serialized data names, when that side can name exactly one.
+ *
+ * Positional, not membership. A node input holds a single scalar at a known
+ * index, so it is authoritative. A subgraph boundary slot holds an *array*
+ * (`SubgraphIO.linkIds`), and a file with two links into one boundary slot
+ * lists both ids, so containment cannot discriminate between them - such
+ * slots deliberately produce no entry here and fall back to document order.
+ */
+export function authoritativeSurvivorByTarget(
+  data: ConfiguredGraph
+): Map<string, number> {
+  const authoritative = new Map<string, number>()
+  for (const node of data.nodes ?? []) {
+    const inputs = node.inputs ?? []
+    for (const [slot, input] of inputs.entries()) {
+      if (input?.link == null) continue
+      authoritative.set(`${toNodeId(node.id)}:${slot}`, input.link)
+    }
+  }
+  return authoritative
+}
+
 export function normalizeConfiguredTopology<T extends ConfiguredGraph>(
   data: T
 ): T {
   if (!data.links?.length) return data
 
-  const survivorByTarget = new Map<string, ReturnType<typeof linkFields>>()
-  const survivorByDuplicateId = new Map<number, number>()
-  const links = data.links.filter((link) => {
+  const authoritative = authoritativeSurvivorByTarget(data)
+
+  // Pass 1: group every link by the slot it targets, in document order.
+  const byTarget = new Map<string, ReturnType<typeof linkFields>[]>()
+  for (const link of data.links) {
     const fields = linkFields(link)
     const key = `${toNodeId(fields.target_id)}:${fields.target_slot}`
-    const survivor = survivorByTarget.get(key)
-    if (!survivor) {
-      survivorByTarget.set(key, fields)
-      return true
-    }
-    if (
-      toNodeId(survivor.origin_id) === toNodeId(fields.origin_id) &&
-      survivor.origin_slot === fields.origin_slot
-    ) {
-      survivorByDuplicateId.set(fields.id, survivor.id)
+    const group = byTarget.get(key)
+    if (group) group.push(fields)
+    else byTarget.set(key, [fields])
+  }
+
+  // Pass 2: pick the survivor per slot. The target side wins when it names a
+  // link that is actually in the group; otherwise keep document order.
+  const remapped = new Map<number, number>()
+  const dropped = new Set<number>()
+  for (const [key, group] of byTarget) {
+    if (group.length === 1) continue
+    const named = authoritative.get(key)
+    const survivor =
+      (named == null
+        ? undefined
+        : group.find((fields) => fields.id === named)) ?? group[0]
+    for (const fields of group) {
+      if (fields.id === survivor.id) continue
+      if (
+        toNodeId(survivor.origin_id) === toNodeId(fields.origin_id) &&
+        survivor.origin_slot === fields.origin_slot
+      ) {
+        remapped.set(fields.id, survivor.id)
+      } else {
+        dropped.add(fields.id)
+        console.warn(
+          `LiteGraph: link ${fields.id} (origin ${String(fields.origin_id)}:${fields.origin_slot}) dropped; ` +
+            `${key} is already connected by link ${survivor.id} (origin ${String(survivor.origin_id)}:${survivor.origin_slot})`
+        )
+      }
     }
-    return false
-  })
-  if (links.length === data.links.length) return data
+  }
+
+  if (!remapped.size && !dropped.size) return data
 
-  const normalized = Object.assign({}, data, { links })
-  if (!survivorByDuplicateId.size) return normalized
+  const links = data.links.filter((link) => {
+    const { id } = linkFields(link)
+    return !remapped.has(id) && !dropped.has(id)
+  })
 
-  const cloned = cloneDeep(normalized)
-  remapLinkReferences(cloned, survivorByDuplicateId)
+  const cloned = cloneDeep(Object.assign({}, data, { links }))
+  if (remapped.size) remapLinkReferences(cloned, remapped)
+  pruneLinkReferences(cloned, dropped)
   return cloned
 }