Update the frontend to support async nodes. (#4382)

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>

tests-ui/tests/store/executionStore.test.ts

@@ -1,11 +1,22 @@
 import { createPinia, setActivePinia } from 'pinia'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
+import { app } from '@/scripts/app'
 import { useExecutionStore } from '@/stores/executionStore'
+import { useWorkflowStore } from '@/stores/workflowStore'
+
+// Mock the workflowStore
+vi.mock('@/stores/workflowStore', () => ({
+  useWorkflowStore: vi.fn(() => ({
+    nodeExecutionIdToNodeLocatorId: vi.fn(),
+    nodeIdToNodeLocatorId: vi.fn(),
+    nodeLocatorIdToNodeExecutionId: vi.fn()
+  }))
+}))
 
 // Remove any previous global types
 declare global {
-  // Empty interface to override any previous declarations
+  // eslint-disable-next-line @typescript-eslint/no-empty-object-type
   interface Window {}
 }
 
@@ -22,12 +33,16 @@ vi.mock('@/composables/node/useNodeProgressText', () => ({
   })
 }))
 
-// Create a local mock instead of using global to avoid conflicts
-const mockApp = {
-  graph: {
-    getNodeById: vi.fn()
+// Mock the app import with proper implementation
+vi.mock('@/scripts/app', () => ({
+  app: {
+    graph: {
+      getNodeById: vi.fn()
+    },
+    revokePreviews: vi.fn(),
+    nodePreviewImages: {}
   }
-}
+}))
 
 describe('executionStore - display_component handling', () => {
   function createDisplayComponentEvent(
@@ -47,7 +62,7 @@ describe('executionStore - display_component handling', () => {
 
   function handleDisplayComponentMessage(event: CustomEvent) {
     const { node_id, component } = event.detail
-    const node = mockApp.graph.getNodeById(node_id)
+    const node = vi.mocked(app.graph.getNodeById)(node_id)
     if (node && component === 'ChatHistoryWidget') {
       mockShowChatHistory(node)
     }
@@ -60,23 +75,121 @@ describe('executionStore - display_component handling', () => {
   })
 
   it('handles ChatHistoryWidget display_component messages', () => {
-    const mockNode = { id: '123' }
-    mockApp.graph.getNodeById.mockReturnValue(mockNode)
+    const mockNode = { id: '123' } as any
+    vi.mocked(app.graph.getNodeById).mockReturnValue(mockNode)
 
     const event = createDisplayComponentEvent('123')
     handleDisplayComponentMessage(event)
 
-    expect(mockApp.graph.getNodeById).toHaveBeenCalledWith('123')
+    expect(app.graph.getNodeById).toHaveBeenCalledWith('123')
     expect(mockShowChatHistory).toHaveBeenCalledWith(mockNode)
   })
 
   it('does nothing if node is not found', () => {
-    mockApp.graph.getNodeById.mockReturnValue(null)
+    vi.mocked(app.graph.getNodeById).mockReturnValue(null)
 
     const event = createDisplayComponentEvent('non-existent')
     handleDisplayComponentMessage(event)
 
-    expect(mockApp.graph.getNodeById).toHaveBeenCalledWith('non-existent')
+    expect(app.graph.getNodeById).toHaveBeenCalledWith('non-existent')
     expect(mockShowChatHistory).not.toHaveBeenCalled()
   })
 })
+
+describe('useExecutionStore - NodeLocatorId conversions', () => {
+  let store: ReturnType<typeof useExecutionStore>
+  let workflowStore: ReturnType<typeof useWorkflowStore>
+
+  beforeEach(() => {
+    setActivePinia(createPinia())
+
+    // Create the mock workflowStore instance
+    const mockWorkflowStore = {
+      nodeExecutionIdToNodeLocatorId: vi.fn(),
+      nodeIdToNodeLocatorId: vi.fn(),
+      nodeLocatorIdToNodeExecutionId: vi.fn()
+    }
+
+    // Mock the useWorkflowStore function to return our mock
+    vi.mocked(useWorkflowStore).mockReturnValue(mockWorkflowStore as any)
+
+    workflowStore = mockWorkflowStore as any
+    store = useExecutionStore()
+    vi.clearAllMocks()
+  })
+
+  describe('executionIdToNodeLocatorId', () => {
+    it('should convert execution ID to NodeLocatorId', () => {
+      // Mock subgraph structure
+      const mockSubgraph = {
+        id: 'a1b2c3d4-e5f6-7890-abcd-ef1234567890',
+        _nodes: []
+      }
+
+      const mockNode = {
+        id: 123,
+        isSubgraphNode: () => true,
+        subgraph: mockSubgraph
+      } as any
+
+      // Mock app.graph.getNodeById to return the mock node
+      vi.mocked(app.graph.getNodeById).mockReturnValue(mockNode)
+
+      const result = store.executionIdToNodeLocatorId('123:456')
+
+      expect(result).toBe('a1b2c3d4-e5f6-7890-abcd-ef1234567890:456')
+    })
+
+    it('should convert simple node ID to NodeLocatorId', () => {
+      const result = store.executionIdToNodeLocatorId('123')
+
+      // For simple node IDs, it should return the ID as-is
+      expect(result).toBe('123')
+    })
+
+    it('should handle numeric node IDs', () => {
+      const result = store.executionIdToNodeLocatorId(123)
+
+      // For numeric IDs, it should convert to string and return as-is
+      expect(result).toBe('123')
+    })
+
+    it('should return null when conversion fails', () => {
+      // Mock app.graph.getNodeById to return null (node not found)
+      vi.mocked(app.graph.getNodeById).mockReturnValue(null)
+
+      // This should throw an error as the node is not found
+      expect(() => store.executionIdToNodeLocatorId('999:456')).toThrow(
+        'Subgraph not found: 999'
+      )
+    })
+  })
+
+  describe('nodeLocatorIdToExecutionId', () => {
+    it('should convert NodeLocatorId to execution ID', () => {
+      const mockExecutionId = '123:456'
+      vi.spyOn(workflowStore, 'nodeLocatorIdToNodeExecutionId').mockReturnValue(
+        mockExecutionId as any
+      )
+
+      const result = store.nodeLocatorIdToExecutionId(
+        'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
+      )
+
+      expect(workflowStore.nodeLocatorIdToNodeExecutionId).toHaveBeenCalledWith(
+        'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
+      )
+      expect(result).toBe(mockExecutionId)
+    })
+
+    it('should return null when conversion fails', () => {
+      vi.spyOn(workflowStore, 'nodeLocatorIdToNodeExecutionId').mockReturnValue(
+        null
+      )
+
+      const result = store.nodeLocatorIdToExecutionId('invalid:format')
+
+      expect(result).toBeNull()
+    })
+  })
+})

tests-ui/tests/store/workflowStore.test.ts

@@ -1,3 +1,4 @@
+import type { Subgraph } from '@comfyorg/litegraph'
 import { createPinia, setActivePinia } from 'pinia'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { nextTick } from 'vue'
@@ -11,6 +12,7 @@ import {
   useWorkflowBookmarkStore,
   useWorkflowStore
 } from '@/stores/workflowStore'
+import { isSubgraph } from '@/utils/typeGuardUtil'
 
 // Add mock for api at the top of the file
 vi.mock('@/scripts/api', () => ({
@@ -26,10 +28,15 @@ vi.mock('@/scripts/api', () => ({
 // Mock comfyApp globally for the store setup
 vi.mock('@/scripts/app', () => ({
   app: {
-    canvas: null // Start with canvas potentially undefined or null
+    canvas: {} // Start with empty canvas object
   }
 }))
 
+// Mock isSubgraph
+vi.mock('@/utils/typeGuardUtil', () => ({
+  isSubgraph: vi.fn(() => false)
+}))
+
 describe('useWorkflowStore', () => {
   let store: ReturnType<typeof useWorkflowStore>
   let bookmarkStore: ReturnType<typeof useWorkflowBookmarkStore>
@@ -518,8 +525,13 @@ describe('useWorkflowStore', () => {
           { name: 'Level 1 Subgraph' },
           { name: 'Level 2 Subgraph' }
         ]
-      }
-      vi.mocked(comfyApp.canvas).subgraph = mockSubgraph as any
+      } as any
+      vi.mocked(comfyApp.canvas).subgraph = mockSubgraph
+
+      // Mock isSubgraph to return true for our mockSubgraph
+      vi.mocked(isSubgraph).mockImplementation(
+        (obj): obj is Subgraph => obj === mockSubgraph
+      )
 
       // Act: Trigger the update
       store.updateActiveGraph()
@@ -536,8 +548,13 @@ describe('useWorkflowStore', () => {
         name: 'Initial Subgraph',
         pathToRootGraph: [{ name: 'Root' }, { name: 'Initial Subgraph' }],
         isRootGraph: false
-      }
-      vi.mocked(comfyApp.canvas).subgraph = initialSubgraph as any
+      } as any
+      vi.mocked(comfyApp.canvas).subgraph = initialSubgraph
+
+      // Mock isSubgraph to return true for our initialSubgraph
+      vi.mocked(isSubgraph).mockImplementation(
+        (obj): obj is Subgraph => obj === initialSubgraph
+      )
 
       // Trigger initial update based on the *first* workflow opened in beforeEach
       store.updateActiveGraph()
@@ -561,6 +578,11 @@ describe('useWorkflowStore', () => {
       // This ensures the watcher *does* cause a state change we can assert
       vi.mocked(comfyApp.canvas).subgraph = undefined
 
+      // Mock isSubgraph to return false for undefined
+      vi.mocked(isSubgraph).mockImplementation(
+        (_obj): _obj is Subgraph => false
+      )
+
       await store.openWorkflow(workflow2) // This changes activeWorkflow and triggers the watch
       await nextTick() // Allow watcher and potential async operations in updateActiveGraph to complete
 
@@ -569,4 +591,131 @@ describe('useWorkflowStore', () => {
       expect(store.activeSubgraph).toBeUndefined()
     })
   })
+
+  describe('NodeLocatorId conversions', () => {
+    beforeEach(() => {
+      // Setup mock graph structure with subgraphs
+      const mockSubgraph = {
+        id: 'a1b2c3d4-e5f6-7890-abcd-ef1234567890',
+        _nodes: []
+      }
+
+      const mockNode = {
+        id: 123,
+        isSubgraphNode: () => true,
+        subgraph: mockSubgraph
+      }
+
+      const mockRootGraph = {
+        _nodes: [mockNode],
+        subgraphs: new Map([[mockSubgraph.id, mockSubgraph]]),
+        getNodeById: (id: string | number) => {
+          if (String(id) === '123') return mockNode
+          return null
+        }
+      }
+
+      vi.mocked(comfyApp).graph = mockRootGraph as any
+      vi.mocked(comfyApp.canvas).subgraph = mockSubgraph as any
+      store.activeSubgraph = mockSubgraph as any
+    })
+
+    describe('nodeIdToNodeLocatorId', () => {
+      it('should convert node ID to NodeLocatorId for subgraph nodes', () => {
+        const result = store.nodeIdToNodeLocatorId(456)
+        expect(result).toBe('a1b2c3d4-e5f6-7890-abcd-ef1234567890:456')
+      })
+
+      it('should return simple node ID for root graph nodes', () => {
+        store.activeSubgraph = undefined
+        const result = store.nodeIdToNodeLocatorId(123)
+        expect(result).toBe('123')
+      })
+
+      it('should use provided subgraph instead of active one', () => {
+        const customSubgraph = {
+          id: 'custom-uuid-1234-5678-90ab-cdef12345678'
+        } as any
+        const result = store.nodeIdToNodeLocatorId(789, customSubgraph)
+        expect(result).toBe('custom-uuid-1234-5678-90ab-cdef12345678:789')
+      })
+    })
+
+    describe('nodeExecutionIdToNodeLocatorId', () => {
+      it('should convert execution ID to NodeLocatorId', () => {
+        const result = store.nodeExecutionIdToNodeLocatorId('123:456')
+        expect(result).toBe('a1b2c3d4-e5f6-7890-abcd-ef1234567890:456')
+      })
+
+      it('should return simple node ID for root level nodes', () => {
+        const result = store.nodeExecutionIdToNodeLocatorId('123')
+        expect(result).toBe('123')
+      })
+
+      it('should return null for invalid execution IDs', () => {
+        const result = store.nodeExecutionIdToNodeLocatorId('999:456')
+        expect(result).toBeNull()
+      })
+    })
+
+    describe('nodeLocatorIdToNodeId', () => {
+      it('should extract node ID from NodeLocatorId', () => {
+        const result = store.nodeLocatorIdToNodeId(
+          'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
+        )
+        expect(result).toBe(456)
+      })
+
+      it('should handle string node IDs', () => {
+        const result = store.nodeLocatorIdToNodeId(
+          'a1b2c3d4-e5f6-7890-abcd-ef1234567890:node_1'
+        )
+        expect(result).toBe('node_1')
+      })
+
+      it('should handle simple node IDs (root graph)', () => {
+        const result = store.nodeLocatorIdToNodeId('123')
+        expect(result).toBe(123)
+
+        const stringResult = store.nodeLocatorIdToNodeId('node_1')
+        expect(stringResult).toBe('node_1')
+      })
+
+      it('should return null for invalid NodeLocatorId', () => {
+        const result = store.nodeLocatorIdToNodeId('invalid:format')
+        expect(result).toBeNull()
+      })
+    })
+
+    describe('nodeLocatorIdToNodeExecutionId', () => {
+      it('should convert NodeLocatorId to execution ID', () => {
+        // Need to mock isSubgraph to identify our mockSubgraph
+        vi.mocked(isSubgraph).mockImplementation((obj): obj is Subgraph => {
+          return obj === store.activeSubgraph
+        })
+
+        const result = store.nodeLocatorIdToNodeExecutionId(
+          'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
+        )
+        expect(result).toBe('123:456')
+      })
+
+      it('should handle simple node IDs (root graph)', () => {
+        const result = store.nodeLocatorIdToNodeExecutionId('123')
+        expect(result).toBe('123')
+      })
+
+      it('should return null for unknown subgraph UUID', () => {
+        const result = store.nodeLocatorIdToNodeExecutionId(
+          'unknown-uuid-1234-5678-90ab-cdef12345678:456'
+        )
+        expect(result).toBeNull()
+      })
+
+      it('should return null for invalid NodeLocatorId', () => {
+        const result = store.nodeLocatorIdToNodeExecutionId('invalid:format')
+        expect(result).toBeNull()
+      })
+    })
+  })
 })