fix: dont remove unowned callbacks when cleaning hooks on unmount

browser_tests/tests/minimap.spec.ts

@@ -476,6 +476,42 @@ test.describe('Minimap', { tag: '@canvas' }, () => {
       })
       .toBe(true)
   })
+
+  test('Closing minimap after subgraph navigation keeps Vue render in sync', async ({
+    comfyPage
+  }) => {
+    await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
+    await comfyPage.workflow.loadWorkflow('subgraphs/basic-subgraph')
+    await comfyPage.vueNodes.waitForNodes()
+
+    const subgraphNodeId = await comfyPage.subgraph.findSubgraphNodeId()
+
+    // Round-trip layers Vue's onNodeAdded wrapper on top of the minimap's.
+    await comfyPage.vueNodes.enterSubgraph(subgraphNodeId)
+    await comfyPage.subgraph.exitViaBreadcrumb()
+
+    // Minimap unmount must not clobber the Vue wrapper layered above it.
+    await comfyPage.page.getByTestId(TestIds.canvas.closeMinimapButton).click()
+
+    await comfyPage.page.evaluate((id) => {
+      const graph = window.app!.graph!
+      const subgraphNode = graph.getNodeById(id)
+      if (!subgraphNode?.isSubgraphNode()) {
+        throw new Error('expected subgraph node at root')
+      }
+      graph.unpackSubgraph(subgraphNode)
+    }, subgraphNodeId)
+
+    await expect
+      .poll(() =>
+        comfyPage.page.evaluate(() => window.app!.graph!.nodes.length)
+      )
+      .toBe(2)
+    await expect.poll(() => comfyPage.vueNodes.getNodeCount()).toBe(2)
+    await expect(comfyPage.vueNodes.getNodeLocator(subgraphNodeId)).toHaveCount(
+      0
+    )
+  })
 })
 
 test.describe('Minimap mobile', { tag: ['@mobile', '@canvas'] }, () => {

src/renderer/extensions/minimap/composables/useMinimapGraph.test.ts

@@ -159,6 +159,50 @@ describe('useMinimapGraph', () => {
     expect(() => graphManager.cleanupEventListeners()).not.toThrow()
   })
 
+  it('cleanup leaves a later wrapper alone when one is layered on top', () => {
+    const graphRef = ref(mockGraph) as Ref<LGraph | null>
+    const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)
+
+    graphManager.setupEventListeners()
+    const minimapWrapper = mockGraph.onNodeAdded
+
+    // Simulate another system ddding its own wrapper on top
+    const downstream = vi.fn()
+    const layeredWrapper = vi.fn(function (this: unknown, node: LGraphNode) {
+      minimapWrapper?.call(this, node)
+      downstream(node)
+    })
+    mockGraph.onNodeAdded = layeredWrapper
+
+    graphManager.cleanupEventListeners()
+
+    // The newer wrapper must survive cleanup
+    expect(mockGraph.onNodeAdded).toBe(layeredWrapper)
+  })
+
+  it('a buried wrapper becomes inert after cleanup', () => {
+    const originalOnNodeAdded = vi.fn()
+    mockGraph.onNodeAdded = originalOnNodeAdded
+
+    const graphRef = ref(mockGraph) as Ref<LGraph | null>
+    const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)
+
+    graphManager.setupEventListeners()
+    const buriedWrapper = mockGraph.onNodeAdded
+
+    // Layer something on top so cleanup can't restore.
+    mockGraph.onNodeAdded = vi.fn()
+    graphManager.cleanupEventListeners()
+    vi.mocked(onGraphChangedMock).mockClear()
+
+    // Call the method directly and ensure it is a no-op
+    const testNode = { id: '9' } as LGraphNode
+    buriedWrapper!(testNode)
+
+    expect(originalOnNodeAdded).toHaveBeenCalledWith(testNode)
+    expect(onGraphChangedMock).not.toHaveBeenCalled()
+  })
+
   it('should detect node position changes', () => {
     const graphRef = ref(mockGraph) as Ref<LGraph | null>
     const graphManager = useMinimapGraph(graphRef, onGraphChangedMock)

src/renderer/extensions/minimap/composables/useMinimapGraph.ts

@@ -38,8 +38,13 @@ export function useMinimapGraph(
   // Track LayoutStore version for change detection
   const layoutStoreVersion = layoutStore.getVersion()
 
-  // Map to store original callbacks per graph ID
-  const originalCallbacksMap = new Map<string, GraphCallbacks>()
+  // Cleanup restores originals only when our wrapper is still on top, and
+  // marks any buried wrapper inert via `isLive()` so it can't fire dead work.
+  interface InstalledHooks {
+    originals: GraphCallbacks
+    wrappers: GraphCallbacks
+  }
+  const hooksMap = new Map<string, InstalledHooks>()
 
   const handleGraphChangedThrottled = useThrottleFn(() => {
     onGraphChanged()
@@ -47,40 +52,44 @@ export function useMinimapGraph(
 
   const setupEventListeners = () => {
     const g = graph.value
-    if (!g) return
+    if (!g || hooksMap.has(g.id)) return
 
-    // Check if we've already wrapped this graph's callbacks
-    if (originalCallbacksMap.has(g.id)) {
-      return
-    }
-
-    // Store the original callbacks for this graph
-    const originalCallbacks: GraphCallbacks = {
+    const originals: GraphCallbacks = {
       onNodeAdded: g.onNodeAdded,
       onNodeRemoved: g.onNodeRemoved,
       onConnectionChange: g.onConnectionChange,
       onTrigger: g.onTrigger
     }
-    originalCallbacksMap.set(g.id, originalCallbacks)
+    const wrappers: GraphCallbacks = {}
+    const entry: InstalledHooks = { originals, wrappers }
+    hooksMap.set(g.id, entry)
+    const isLive = () => hooksMap.get(g.id) === entry
 
-    g.onNodeAdded = function (node: LGraphNode) {
-      originalCallbacks.onNodeAdded?.call(this, node)
+    wrappers.onNodeAdded = function (node: LGraphNode) {
+      originals.onNodeAdded?.call(this, node)
+      if (!isLive()) return
       void handleGraphChangedThrottled()
     }
+    g.onNodeAdded = wrappers.onNodeAdded
 
-    g.onNodeRemoved = function (node: LGraphNode) {
-      originalCallbacks.onNodeRemoved?.call(this, node)
+    wrappers.onNodeRemoved = function (node: LGraphNode) {
+      originals.onNodeRemoved?.call(this, node)
+      if (!isLive()) return
       nodeStatesCache.delete(node.id)
       void handleGraphChangedThrottled()
     }
+    g.onNodeRemoved = wrappers.onNodeRemoved
 
-    g.onConnectionChange = function (node: LGraphNode) {
-      originalCallbacks.onConnectionChange?.call(this, node)
+    wrappers.onConnectionChange = function (node: LGraphNode) {
+      originals.onConnectionChange?.call(this, node)
+      if (!isLive()) return
       void handleGraphChangedThrottled()
     }
+    g.onConnectionChange = wrappers.onConnectionChange
 
-    g.onTrigger = function (event: LGraphTriggerEvent) {
-      originalCallbacks.onTrigger?.call(this, event)
+    wrappers.onTrigger = function (event: LGraphTriggerEvent) {
+      originals.onTrigger?.call(this, event)
+      if (!isLive()) return
 
       // Listen for visual property changes that affect minimap rendering
       if (
@@ -94,24 +103,25 @@ export function useMinimapGraph(
         void handleGraphChangedThrottled()
       }
     }
+    g.onTrigger = wrappers.onTrigger
   }
 
   const cleanupEventListeners = (oldGraph?: LGraph) => {
     const g = oldGraph || graph.value
     if (!g) return
+    const entry = hooksMap.get(g.id)
+    if (!entry) return
+    const { originals, wrappers } = entry
 
-    const originalCallbacks = originalCallbacksMap.get(g.id)
-    if (!originalCallbacks) {
-      // Graph was never set up (e.g., minimap destroyed before init) - nothing to clean up
-      return
-    }
+    if (g.onNodeAdded === wrappers.onNodeAdded)
+      g.onNodeAdded = originals.onNodeAdded
+    if (g.onNodeRemoved === wrappers.onNodeRemoved)
+      g.onNodeRemoved = originals.onNodeRemoved
+    if (g.onConnectionChange === wrappers.onConnectionChange)
+      g.onConnectionChange = originals.onConnectionChange
+    if (g.onTrigger === wrappers.onTrigger) g.onTrigger = originals.onTrigger
 
-    g.onNodeAdded = originalCallbacks.onNodeAdded
-    g.onNodeRemoved = originalCallbacks.onNodeRemoved
-    g.onConnectionChange = originalCallbacks.onConnectionChange
-    g.onTrigger = originalCallbacks.onTrigger
-
-    originalCallbacksMap.delete(g.id)
+    hooksMap.delete(g.id)
   }
 
   const checkForChangesInternal = () => {