[feat] carve out path to call asset browser in combo widget (#5464)

* [ci] ignore local browser tests files

this is where i have claude put its one off playwright scripts

* [feat] carve out path to call asset browser in combo widget

* [feat] use buttons on Model Loaders when Asset API setting is on

tests-ui/tests/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts

@@ -1,39 +1,211 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
+import { LGraphNode } from '@/lib/litegraph/src/litegraph'
+import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
 import { useComboWidget } from '@/renderer/extensions/vueNodes/widgets/composables/useComboWidget'
 import type { InputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
+import { assetService } from '@/services/assetService'
 
 vi.mock('@/scripts/widgets', () => ({
   addValueControlWidgets: vi.fn()
 }))
 
+const mockSettingStoreGet = vi.fn(() => false)
+vi.mock('@/stores/settingStore', () => ({
+  useSettingStore: vi.fn(() => ({
+    get: mockSettingStoreGet
+  }))
+}))
+
+vi.mock('@/i18n', () => ({
+  t: vi.fn((key: string) =>
+    key === 'widgets.selectModel' ? 'Select model' : key
+  )
+}))
+
+vi.mock('@/services/assetService', () => ({
+  assetService: {
+    isAssetBrowserEligible: vi.fn(() => false)
+  }
+}))
+
+// Test factory functions
+function createMockWidget(overrides: Partial<IBaseWidget> = {}): IBaseWidget {
+  return {
+    type: 'combo',
+    options: {},
+    name: 'testWidget',
+    value: undefined,
+    ...overrides
+  } as IBaseWidget
+}
+
+function createMockNode(comfyClass = 'TestNode'): LGraphNode {
+  const node = new LGraphNode('TestNode')
+  node.comfyClass = comfyClass
+
+  // Spy on the addWidget method
+  vi.spyOn(node, 'addWidget').mockReturnValue(createMockWidget())
+
+  return node
+}
+
+function createMockInputSpec(overrides: Partial<InputSpec> = {}): InputSpec {
+  return {
+    type: 'COMBO',
+    name: 'testInput',
+    ...overrides
+  } as InputSpec
+}
+
 describe('useComboWidget', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    // Reset to defaults
+    mockSettingStoreGet.mockReturnValue(false)
+    vi.mocked(assetService.isAssetBrowserEligible).mockReturnValue(false)
   })
 
   it('should handle undefined spec', () => {
     const constructor = useComboWidget()
-    const mockNode = {
-      addWidget: vi.fn().mockReturnValue({ options: {} } as any)
-    }
+    const mockWidget = createMockWidget()
+    const mockNode = createMockNode()
+    vi.mocked(mockNode.addWidget).mockReturnValue(mockWidget)
+    const inputSpec = createMockInputSpec({ name: 'inputName' })
 
-    const inputSpec: InputSpec = {
-      type: 'COMBO',
-      name: 'inputName'
-    }
-
-    const widget = constructor(mockNode as any, inputSpec)
+    const widget = constructor(mockNode, inputSpec)
 
     expect(mockNode.addWidget).toHaveBeenCalledWith(
       'combo',
       'inputName',
-      undefined, // default value
-      expect.any(Function), // callback
+      undefined,
+      expect.any(Function),
       expect.objectContaining({
         values: []
       })
     )
-    expect(widget).toEqual({ options: {} })
+    expect(widget).toBe(mockWidget)
+  })
+
+  it('should create normal combo widget when asset API is disabled', () => {
+    mockSettingStoreGet.mockReturnValue(false)
+    vi.mocked(assetService.isAssetBrowserEligible).mockReturnValue(true)
+
+    const constructor = useComboWidget()
+    const mockWidget = createMockWidget()
+    const mockNode = createMockNode('CheckpointLoaderSimple')
+    vi.mocked(mockNode.addWidget).mockReturnValue(mockWidget)
+    const inputSpec = createMockInputSpec({
+      name: 'ckpt_name',
+      options: ['model1.safetensors', 'model2.safetensors']
+    })
+
+    const widget = constructor(mockNode, inputSpec)
+
+    expect(mockNode.addWidget).toHaveBeenCalledWith(
+      'combo',
+      'ckpt_name',
+      'model1.safetensors',
+      expect.any(Function),
+      { values: ['model1.safetensors', 'model2.safetensors'] }
+    )
+    expect(mockSettingStoreGet).toHaveBeenCalledWith('Comfy.Assets.UseAssetAPI')
+    expect(widget).toBe(mockWidget)
+  })
+
+  it('should create normal combo widget when widget is not eligible for asset browser', () => {
+    mockSettingStoreGet.mockReturnValue(true)
+    vi.mocked(assetService.isAssetBrowserEligible).mockReturnValue(false)
+
+    const constructor = useComboWidget()
+    const mockWidget = createMockWidget()
+    const mockNode = createMockNode()
+    vi.mocked(mockNode.addWidget).mockReturnValue(mockWidget)
+    const inputSpec = createMockInputSpec({
+      name: 'not_eligible_widget',
+      options: ['option1', 'option2']
+    })
+
+    const widget = constructor(mockNode, inputSpec)
+
+    expect(mockNode.addWidget).toHaveBeenCalledWith(
+      'combo',
+      'not_eligible_widget',
+      'option1',
+      expect.any(Function),
+      { values: ['option1', 'option2'] }
+    )
+    expect(vi.mocked(assetService.isAssetBrowserEligible)).toHaveBeenCalledWith(
+      'not_eligible_widget',
+      'TestNode'
+    )
+    expect(widget).toBe(mockWidget)
+  })
+
+  it('should create asset browser button widget when API enabled and widget eligible', () => {
+    mockSettingStoreGet.mockReturnValue(true)
+    vi.mocked(assetService.isAssetBrowserEligible).mockReturnValue(true)
+
+    const constructor = useComboWidget()
+    const mockWidget = createMockWidget({
+      type: 'button',
+      name: 'ckpt_name',
+      value: 'Select model'
+    })
+    const mockNode = createMockNode('CheckpointLoaderSimple')
+    vi.mocked(mockNode.addWidget).mockReturnValue(mockWidget)
+    const inputSpec = createMockInputSpec({
+      name: 'ckpt_name',
+      options: ['model1.safetensors', 'model2.safetensors']
+    })
+
+    const widget = constructor(mockNode, inputSpec)
+
+    expect(mockNode.addWidget).toHaveBeenCalledWith(
+      'button',
+      'ckpt_name',
+      'Select model',
+      expect.any(Function)
+    )
+    expect(mockSettingStoreGet).toHaveBeenCalledWith('Comfy.Assets.UseAssetAPI')
+    expect(vi.mocked(assetService.isAssetBrowserEligible)).toHaveBeenCalledWith(
+      'ckpt_name',
+      'CheckpointLoaderSimple'
+    )
+    expect(widget).toBe(mockWidget)
+  })
+
+  it('should use asset browser button even when inputSpec has a default value but no options', () => {
+    mockSettingStoreGet.mockReturnValue(true)
+    vi.mocked(assetService.isAssetBrowserEligible).mockReturnValue(true)
+
+    const constructor = useComboWidget()
+    const mockWidget = createMockWidget({
+      type: 'button',
+      name: 'ckpt_name',
+      value: 'Select model'
+    })
+    const mockNode = createMockNode('CheckpointLoaderSimple')
+    vi.mocked(mockNode.addWidget).mockReturnValue(mockWidget)
+    const inputSpec = createMockInputSpec({
+      name: 'ckpt_name',
+      default: 'fallback.safetensors'
+      // Note: no options array provided
+    })
+
+    const widget = constructor(mockNode, inputSpec)
+
+    expect(mockNode.addWidget).toHaveBeenCalledWith(
+      'button',
+      'ckpt_name',
+      'Select model',
+      expect.any(Function)
+    )
+    expect(mockSettingStoreGet).toHaveBeenCalledWith('Comfy.Assets.UseAssetAPI')
+    expect(vi.mocked(assetService.isAssetBrowserEligible)).toHaveBeenCalledWith(
+      'ckpt_name',
+      'CheckpointLoaderSimple'
+    )
+    expect(widget).toBe(mockWidget)
   })
 })

tests-ui/tests/services/assetService.test.ts

@@ -3,6 +3,20 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { api } from '@/scripts/api'
 import { assetService } from '@/services/assetService'
 
+vi.mock('@/stores/modelToNodeStore', () => ({
+  useModelToNodeStore: vi.fn(() => ({
+    getRegisteredNodeTypes: vi.fn(
+      () =>
+        new Set([
+          'CheckpointLoaderSimple',
+          'LoraLoader',
+          'VAELoader',
+          'TestNode'
+        ])
+    )
+  }))
+}))
+
 // Test data constants
 const MOCK_ASSETS = {
   checkpoints: {
@@ -147,4 +161,43 @@ describe('assetService', () => {
       )
     })
   })
+
+  describe('isAssetBrowserEligible', () => {
+    it('should return true for eligible widget names with registered node types', () => {
+      expect(
+        assetService.isAssetBrowserEligible(
+          'ckpt_name',
+          'CheckpointLoaderSimple'
+        )
+      ).toBe(true)
+      expect(
+        assetService.isAssetBrowserEligible('lora_name', 'LoraLoader')
+      ).toBe(true)
+      expect(assetService.isAssetBrowserEligible('vae_name', 'VAELoader')).toBe(
+        true
+      )
+    })
+
+    it('should return false for non-eligible widget names', () => {
+      expect(assetService.isAssetBrowserEligible('seed', 'TestNode')).toBe(
+        false
+      )
+      expect(assetService.isAssetBrowserEligible('steps', 'TestNode')).toBe(
+        false
+      )
+      expect(
+        assetService.isAssetBrowserEligible('sampler_name', 'TestNode')
+      ).toBe(false)
+      expect(assetService.isAssetBrowserEligible('', 'TestNode')).toBe(false)
+    })
+
+    it('should return false for eligible widget names with unregistered node types', () => {
+      expect(
+        assetService.isAssetBrowserEligible('ckpt_name', 'UnknownNode')
+      ).toBe(false)
+      expect(
+        assetService.isAssetBrowserEligible('lora_name', 'UnknownNode')
+      ).toBe(false)
+    })
+  })
 })

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

@@ -21,45 +21,44 @@ const EXPECTED_DEFAULT_TYPES = [
 
 type NodeDefStoreType = typeof import('@/stores/nodeDefStore')
 
+// Create minimal but valid ComfyNodeDefImpl for testing
+function createMockNodeDef(name: string): ComfyNodeDefImpl {
+  const def: ComfyNodeDefV1 = {
+    name,
+    display_name: name,
+    category: 'test',
+    python_module: 'nodes',
+    description: '',
+    input: { required: {}, optional: {} },
+    output: [],
+    output_name: [],
+    output_is_list: [],
+    output_node: false
+  }
+  return new ComfyNodeDefImpl(def)
+}
+
+const MOCK_NODE_NAMES = [
+  'CheckpointLoaderSimple',
+  'ImageOnlyCheckpointLoader',
+  'LoraLoader',
+  'LoraLoaderModelOnly',
+  'VAELoader',
+  'ControlNetLoader',
+  'UNETLoader',
+  'UpscaleModelLoader',
+  'StyleModelLoader',
+  'GLIGENLoader'
+] as const
+
+const mockNodeDefsByName = Object.fromEntries(
+  MOCK_NODE_NAMES.map((name) => [name, createMockNodeDef(name)])
+)
+
 // Mock nodeDefStore dependency - modelToNodeStore relies on this for registration
 // Most tests expect this to be populated; tests that need empty state can override
 vi.mock('@/stores/nodeDefStore', async (importOriginal) => {
   const original = await importOriginal<NodeDefStoreType>()
-  const { ComfyNodeDefImpl } = original
-
-  // Create minimal but valid ComfyNodeDefImpl for testing
-  function createMockNodeDef(name: string): ComfyNodeDefImpl {
-    const def: ComfyNodeDefV1 = {
-      name,
-      display_name: name,
-      category: 'test',
-      python_module: 'nodes',
-      description: '',
-      input: { required: {}, optional: {} },
-      output: [],
-      output_name: [],
-      output_is_list: [],
-      output_node: false
-    }
-    return new ComfyNodeDefImpl(def)
-  }
-
-  const MOCK_NODE_NAMES = [
-    'CheckpointLoaderSimple',
-    'ImageOnlyCheckpointLoader',
-    'LoraLoader',
-    'LoraLoaderModelOnly',
-    'VAELoader',
-    'ControlNetLoader',
-    'UNETLoader',
-    'UpscaleModelLoader',
-    'StyleModelLoader',
-    'GLIGENLoader'
-  ] as const
-
-  const mockNodeDefsByName = Object.fromEntries(
-    MOCK_NODE_NAMES.map((name) => [name, createMockNodeDef(name)])
-  )
 
   return {
     ...original,
@@ -72,6 +71,7 @@ vi.mock('@/stores/nodeDefStore', async (importOriginal) => {
 describe('useModelToNodeStore', () => {
   beforeEach(() => {
     setActivePinia(createPinia())
+    vi.clearAllMocks()
   })
 
   describe('modelToNodeMap', () => {
@@ -288,12 +288,58 @@ describe('useModelToNodeStore', () => {
     })
 
     it('should not register when nodeDefStore is empty', () => {
+      // Create fresh Pinia for this test to avoid state persistence
+      setActivePinia(createPinia())
+
       vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({
         nodeDefsByName: {}
       })
       const modelToNodeStore = useModelToNodeStore()
       modelToNodeStore.registerDefaults()
       expect(modelToNodeStore.getNodeProvider('checkpoints')).toBeUndefined()
+
+      // Restore original mock for subsequent tests
+      vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({
+        nodeDefsByName: mockNodeDefsByName
+      })
+    })
+  })
+
+  describe('getRegisteredNodeTypes', () => {
+    it('should return a Set instance', () => {
+      const modelToNodeStore = useModelToNodeStore()
+      const result = modelToNodeStore.getRegisteredNodeTypes()
+      expect(result).toBeInstanceOf(Set)
+    })
+
+    it('should return empty set when nodeDefStore is empty', () => {
+      // Create fresh Pinia for this test to avoid state persistence
+      setActivePinia(createPinia())
+
+      vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({
+        nodeDefsByName: {}
+      })
+      const modelToNodeStore = useModelToNodeStore()
+
+      const result = modelToNodeStore.getRegisteredNodeTypes()
+      expect(result.size).toBe(0)
+
+      // Restore original mock for subsequent tests
+      vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({
+        nodeDefsByName: mockNodeDefsByName
+      })
+    })
+
+    it('should contain node types for efficient Set.has() lookups', () => {
+      const modelToNodeStore = useModelToNodeStore()
+      modelToNodeStore.registerDefaults()
+
+      const result = modelToNodeStore.getRegisteredNodeTypes()
+
+      // Test Set.has() functionality which assetService depends on
+      expect(result.has('CheckpointLoaderSimple')).toBe(true)
+      expect(result.has('LoraLoader')).toBe(true)
+      expect(result.has('NonExistentNode')).toBe(false)
     })
   })