fix: warn and skip duplicate extension registration instead of throwing

src/services/extensionService.test.ts

@@ -1,6 +1,10 @@
-import { describe, expect, it } from 'vitest'
+import { describe, expect, it, vi } from 'vitest'
+import { defineComponent } from 'vue'
 
-import { shouldLoadExtension } from './extensionService'
+import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
+import type { ComfyExtension } from '@/types/comfy'
+
+import { shouldLoadExtension, useExtensionService } from './extensionService'
 
 describe('shouldLoadExtension', () => {
   it.for(['/extensions/cloud/rum.js', '/extensions/cloud/sentry.js'])(
@@ -27,3 +31,31 @@ describe('shouldLoadExtension', () => {
     )
   })
 })
+
+describe('registerExtension', () => {
+  it('does not re-run registration side effects for a duplicate name', () => {
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    try {
+      const extension: ComfyExtension = {
+        name: 'dup.ext',
+        bottomPanelTabs: [
+          {
+            id: 'dup.tab',
+            title: 'Dup',
+            type: 'vue',
+            component: defineComponent({ render: () => null })
+          }
+        ]
+      }
+
+      const extensionService = useExtensionService()
+      extensionService.registerExtension(extension)
+      extensionService.registerExtension(extension)
+
+      const bottomPanelStore = useBottomPanelStore()
+      expect(bottomPanelStore.panels.terminal.tabs).toHaveLength(1)
+    } finally {
+      warnSpy.mockRestore()
+    }
+  })
+})

src/services/extensionService.ts

@@ -72,7 +72,7 @@ export const useExtensionService = () => {
    * @param extension The extension to register
    */
   const registerExtension = (extension: ComfyExtension) => {
-    extensionStore.registerExtension(extension)
+    if (!extensionStore.registerExtension(extension)) return
 
     const addKeybinding = wrapWithErrorHandling(
       keybindingStore.addDefaultKeybinding

src/stores/extensionStore.test.ts

@@ -17,12 +17,42 @@ describe('extensionStore', () => {
       )
     })
 
-    it('throws for duplicate registration', () => {
+    it('warns and keeps the first registration for a duplicate name', () => {
       const store = useExtensionStore()
-      store.registerExtension({ name: 'dup' })
-      expect(() => store.registerExtension({ name: 'dup' })).toThrow(
-        "Extension named 'dup' already registered."
-      )
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+      try {
+        const first = { name: 'dup' }
+        store.registerExtension(first)
+        expect(store.registerExtension({ name: 'dup' })).toBe(false)
+        expect(warnSpy).toHaveBeenCalledWith(
+          "Extension named 'dup' already registered - skipping"
+        )
+        expect(store.extensions).toHaveLength(1)
+        expect(store.extensions[0]).toBe(first)
+      } finally {
+        warnSpy.mockRestore()
+      }
+    })
+
+    it('registers names that collide with Object.prototype keys', () => {
+      const store = useExtensionStore()
+      store.registerExtension({ name: 'constructor' })
+      store.registerExtension({ name: '__proto__' })
+      expect(store.isExtensionInstalled('constructor')).toBe(true)
+      expect(store.isExtensionInstalled('__proto__')).toBe(true)
+      expect(store.extensions).toHaveLength(2)
+    })
+
+    it('lists extensions in registration order', () => {
+      const store = useExtensionStore()
+      store.registerExtension({ name: 'z.ext' })
+      store.registerExtension({ name: '2' })
+      store.registerExtension({ name: 'a.ext' })
+      expect(store.extensions.map((ext) => ext.name)).toEqual([
+        'z.ext',
+        '2',
+        'a.ext'
+      ])
     })
 
     it('warns when registering a disabled extension but still installs it', () => {
@@ -46,6 +76,7 @@ describe('extensionStore', () => {
     it('returns false for uninstalled extension', () => {
       const store = useExtensionStore()
       expect(store.isExtensionInstalled('missing')).toBe(false)
+      expect(store.isExtensionInstalled('toString')).toBe(false)
     })
   })
 

src/stores/extensionStore.ts

@@ -24,24 +24,24 @@ const ALWAYS_DISABLED_EXTENSIONS: readonly string[] = [
 ]
 
 export const useExtensionStore = defineStore('extension', () => {
-  // For legacy reasons, the name uniquely identifies an extension
-  const extensionByName = ref<Record<string, ComfyExtension>>({})
-  const extensions = computed(() => Object.values(extensionByName.value))
+  // For legacy reasons, the name uniquely identifies an extension.
+  const extensionByName = ref<Map<string, ComfyExtension>>(new Map())
+  const extensions = computed(() => [...extensionByName.value.values()])
   // Not using computed because disable extension requires reloading of the page.
   // Dynamically update this list won't affect extensions that are already loaded.
   const disabledExtensionNames = ref<Set<string>>(new Set())
 
+  const isExtensionInstalled = (name: string) => extensionByName.value.has(name)
+
   // Disabled extension names that are currently not in the extension list.
   // If a node pack is disabled in the backend, we shouldn't remove the configuration
   // of the frontend extension disable list, in case the node pack is re-enabled.
   const inactiveDisabledExtensionNames = computed(() => {
     return Array.from(disabledExtensionNames.value).filter(
-      (name) => !(name in extensionByName.value)
+      (name) => !isExtensionInstalled(name)
     )
   })
 
-  const isExtensionInstalled = (name: string) => name in extensionByName.value
-
   const isExtensionEnabled = (name: string) =>
     !disabledExtensionNames.value.has(name)
   const enabledExtensions = computed(() => {
@@ -55,20 +55,29 @@ export const useExtensionStore = defineStore('extension', () => {
     )
   }
 
-  function registerExtension(extension: ComfyExtension) {
+  /**
+   * Registers an extension by name. A name that is already registered keeps its
+   * first registration.
+   * @returns whether the extension was registered
+   */
+  function registerExtension(extension: ComfyExtension): boolean {
     if (!extension.name) {
       throw new Error("Extensions must have a 'name' property.")
     }
 
-    if (extensionByName.value[extension.name]) {
-      throw new Error(`Extension named '${extension.name}' already registered.`)
+    if (isExtensionInstalled(extension.name)) {
+      console.warn(
+        `Extension named '${extension.name}' already registered - skipping`
+      )
+      return false
     }
 
     if (disabledExtensionNames.value.has(extension.name)) {
       console.warn(`Extension ${extension.name} is disabled.`)
     }
 
-    extensionByName.value[extension.name] = markRaw(extension)
+    extensionByName.value.set(extension.name, markRaw(extension))
+    return true
   }
 
   function loadDisabledExtensionNames(names: string[]) {