From d3dad95ce70e4e01bb74448670e38ab2bfdce1d5 Mon Sep 17 00:00:00 2001
From: Glary-Bot <glary-bot@users.noreply.github.com>
Date: Wed, 6 May 2026 20:02:22 +0000
Subject: [PATCH] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20try/final?=
 =?UTF-8?q?ly=20restoration=20and=20skip=20RAF=20on=20no-op=20processSelec?=
 =?UTF-8?q?t?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeRabbit raised two valid concerns:

1. If processSelect throws (e.g. through a node/extension callback), the
   plain assign-and-restore pattern leaves onSelectionChange unset and
   silently drops every later selection change on this canvas.

2. The unconditional RAF schedules a pointless store sync on the
   already-selected sticky-resselect path where processSelect early-returns
   without notifying.

Both fixes are the same shape: replace the temporary undefined with a
sentinel that records whether processSelect actually attempted a
notification, wrap processSelect in try/finally so the original listener
is always restored, then schedule the RAF only if the sentinel fired.

Adds two tests covering each guarantee.
---
 .../LGraphCanvas.dragStartDeferral.test.ts    | 27 +++++++++++++++++++
 src/lib/litegraph/src/LGraphCanvas.ts         | 17 +++++++++---
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/lib/litegraph/src/LGraphCanvas.dragStartDeferral.test.ts b/src/lib/litegraph/src/LGraphCanvas.dragStartDeferral.test.ts
index b4732d3886..281a5b748f 100644
--- a/src/lib/litegraph/src/LGraphCanvas.dragStartDeferral.test.ts
+++ b/src/lib/litegraph/src/LGraphCanvas.dragStartDeferral.test.ts
@@ -120,4 +120,31 @@ describe('_startDraggingItems defers onSelectionChange', () => {
 
     expect(canvas.onSelectionChange).toBe(onSelectionChange)
   })
+
+  it('does not schedule a deferred notification when starting a drag on an already-selected sticky item', () => {
+    canvas.select(node)
+    const onSelectionChange = vi.fn()
+    canvas.onSelectionChange = onSelectionChange
+
+    canvas['_startDraggingItems'](node, pointer, true)
+
+    vi.advanceTimersByTime(16)
+    expect(onSelectionChange).not.toHaveBeenCalled()
+  })
+
+  it('restores onSelectionChange even when processSelect throws', () => {
+    const onSelectionChange = vi.fn()
+    canvas.onSelectionChange = onSelectionChange
+    const original = canvas.processSelect
+    canvas.processSelect = () => {
+      throw new Error('boom')
+    }
+
+    expect(() => canvas['_startDraggingItems'](node, pointer, true)).toThrow(
+      'boom'
+    )
+
+    expect(canvas.onSelectionChange).toBe(onSelectionChange)
+    canvas.processSelect = original
+  })
 })
diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts
index a7d2641508..5c320b9466 100644
--- a/src/lib/litegraph/src/LGraphCanvas.ts
+++ b/src/lib/litegraph/src/LGraphCanvas.ts
@@ -3615,11 +3615,20 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
     // Selection-update side effects (onSelectionChange callback) are deferred
     // to the next frame so the node visibly starts following the pointer
     // before downstream reactivity (e.g. Vue store updates) runs.
+    // The sentinel records whether processSelect actually notified, so we
+    // skip the RAF on the no-op sticky-resselect path and avoid swallowing
+    // the listener if processSelect throws.
     const onSelectionChange = this.onSelectionChange
-    this.onSelectionChange = undefined
-    this.processSelect(item, pointer.eDown, sticky)
-    this.onSelectionChange = onSelectionChange
-    if (onSelectionChange) {
+    let selectionNotified = false
+    this.onSelectionChange = () => {
+      selectionNotified = true
+    }
+    try {
+      this.processSelect(item, pointer.eDown, sticky)
+    } finally {
+      this.onSelectionChange = onSelectionChange
+    }
+    if (onSelectionChange && selectionNotified) {
       requestAnimationFrame(() => onSelectionChange(this.selected_nodes))
     }