fix(widgets): tolerate un-keyable widget ids in widget value store

browser_tests/tests/vueNodes/widgets/emptyNameWidget.spec.ts

@@ -0,0 +1,61 @@
+import {
+  comfyExpect as expect,
+  comfyPageFixture as test
+} from '@e2e/fixtures/ComfyPage'
+import { TestIds } from '@e2e/fixtures/selectors'
+
+const HOST_NODE_TYPE = 'KSampler'
+const HOST_NODE_TITLE = 'KSampler'
+
+/**
+ * Regression for #13773: a custom node that registers a client-side widget with
+ * an empty/placeholder name produces an un-keyable widget id (empty name
+ * segment). Before the fix, `parseWidgetId` threw on that id, which tripped the
+ * Vue `NodeWidgets` error boundary and blanked the node's ENTIRE widget grid
+ * (e.g. rgthree Power Lora Loader rendered no widgets). The un-storable widget
+ * must not take down its schema-declared siblings.
+ */
+test.describe(
+  'Empty-name custom widget',
+  { tag: ['@vue-nodes', '@widget'] },
+  () => {
+    test.beforeEach(async ({ comfyPage }) => {
+      // Emulate a custom node registering an extra, un-keyable widget
+      await comfyPage.nodeOps.clearGraph()
+      await comfyPage.nodeOps.addNode(HOST_NODE_TITLE, undefined, {
+        x: 400,
+        y: 200
+      })
+      await comfyPage.page.evaluate(() =>
+        graph!.nodes[0].addWidget('text', '', '', () => {})
+      )
+    })
+
+    test('renders schema widgets despite an un-keyable sibling', async ({
+      comfyPage
+    }) => {
+      const node = comfyPage.vueNodes.getNodeByTitle(HOST_NODE_TITLE)
+      await expect(node).toBeVisible()
+
+      // Precondition: the un-keyable empty-name widget really is on the node, so
+      // this test exercises the regression rather than passing vacuously.
+      await expect
+        .poll(() =>
+          comfyPage.page.evaluate((type) => {
+            const host = window.app!.graph.nodes.find((n) => n.type === type)
+            return host?.widgets?.some((w) => w.name === '') ?? false
+          }, HOST_NODE_TYPE)
+        )
+        .toBe(true)
+
+      // The widget grid must render (v-else branch), not the error boundary.
+      await expect(node.getByTestId(TestIds.widgets.container)).toBeVisible()
+      await expect(node.locator('.node-error')).toHaveCount(0)
+
+      // Schema-declared siblings must still render and be interactable.
+      await expect(node.getByLabel('seed', { exact: true })).toBeVisible()
+      await expect(node.getByLabel('steps', { exact: true })).toBeVisible()
+      await expect(node.getByLabel('cfg', { exact: true })).toBeVisible()
+    })
+  }
+)

src/lib/litegraph/src/widgets/BaseWidget.test.ts

@@ -237,4 +237,19 @@ describe('BaseWidget store integration', () => {
       expect(state?.disabled).toBe(false)
     })
   })
+
+  describe('un-keyable widget id (empty name)', () => {
+    it('keeps local state usable when registration is declined', () => {
+      const widget = createTestWidget(node, { name: '', value: 55 })
+
+      expect(() => widget.setNodeId(toNodeId(1))).not.toThrow()
+      expect(
+        store.getWidget(widgetId(graph.id, toNodeId(1), ''))
+      ).toBeUndefined()
+
+      expect(widget.value).toBe(55)
+      widget.value = 88
+      expect(widget.value).toBe(88)
+    })
+  })
 })

src/lib/litegraph/src/widgets/BaseWidget.ts

@@ -147,13 +147,14 @@ export abstract class BaseWidget<TWidget extends IBaseWidget = IBaseWidget>
     const graphId = this.node.graph?.rootGraph.id
     if (!graphId) return
 
-    this._state = useWidgetValueStore().registerWidget(
+    const registered = useWidgetValueStore().registerWidget(
       widgetId(graphId, nodeId, this.name),
       {
         ...this._state,
         value: this.value
       }
     )
+    if (registered) this._state = registered
   }
 
   constructor(widget: TWidget & { node: LGraphNode })

src/stores/widgetValueStore.test.ts

@@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it } from 'vitest'
 import type { UUID } from '@/utils/uuid'
 import { toNodeId } from '@/types/nodeId'
 import { widgetId } from '@/types/widgetId'
+import type { WidgetId } from '@/types/widgetId'
 import type { WidgetState } from '@/types/widgetState'
 
 import { useWidgetValueStore } from './widgetValueStore'
@@ -35,7 +36,7 @@ describe('useWidgetValueStore', () => {
 
     it('widgetState.value can be read and written directly', () => {
       const store = useWidgetValueStore()
-      const registered = store.registerWidget(seedA, state('number', 100))
+      const registered = store.registerWidget(seedA, state('number', 100))!
       expect(registered.value).toBe(100)
 
       registered.value = 200
@@ -79,7 +80,7 @@ describe('useWidgetValueStore', () => {
   describe('widget registration', () => {
     it('registers a widget with minimal properties', () => {
       const store = useWidgetValueStore()
-      const registered = store.registerWidget(seedA, state('number', 12345))
+      const registered = store.registerWidget(seedA, state('number', 12345))!
 
       expect(registered.nodeId).toBe('node-1')
       expect(registered.name).toBe('seed')
@@ -96,21 +97,36 @@ describe('useWidgetValueStore', () => {
       const registered = store.registerWidget(
         seedA,
         state('number', 12345, { y: 42 })
-      )
+      )!
 
       expect(registered.y).toBe(42)
     })
 
     it('registerWidget is idempotent and does not overwrite existing state', () => {
       const store = useWidgetValueStore()
-      const first = store.registerWidget(seedA, state('number', 11))
+      const first = store.registerWidget(seedA, state('number', 11))!
       first.value = 99
 
-      const second = store.registerWidget(seedA, state('number', 11))
+      const second = store.registerWidget(seedA, state('number', 11))!
       expect(second).toBe(first)
       expect(second.value).toBe(99)
     })
 
+    it('replaces a stale entry when the widget type changes', () => {
+      const store = useWidgetValueStore()
+      const first = store.registerWidget(seedA, state('number', 5))!
+      first.value = 42
+
+      // After a subgraph convert the graphId:nodeId:name key can be reused by a
+      // different widget. A type mismatch means it is not the same widget, so
+      // the live type/value must win rather than the stale entry (BUG: a text
+      // widget rendered as int until reload).
+      const reconciled = store.registerWidget(seedA, state('string', 'hello'))!
+      expect(reconciled.type).toBe('string')
+      expect(reconciled.value).toBe('hello')
+      expect(store.getWidget(seedA)?.type).toBe('string')
+    })
+
     it('registers a widget with all properties', () => {
       const store = useWidgetValueStore()
       const registered = store.registerWidget(
@@ -121,7 +137,7 @@ describe('useWidgetValueStore', () => {
           serialize: false,
           options: { multiline: true }
         })
-      )
+      )!
 
       expect(registered.label).toBe('Prompt Text')
       expect(registered.disabled).toBe(true)
@@ -187,15 +203,15 @@ describe('useWidgetValueStore', () => {
   describe('direct property mutation', () => {
     it('disabled can be set directly via getWidget', () => {
       const store = useWidgetValueStore()
-      const registered = store.registerWidget(seedA, state('number', 100))
+      const registered = store.registerWidget(seedA, state('number', 100))!
 
       registered.disabled = true
       expect(store.getWidget(seedA)?.disabled).toBe(true)
     })
 
     it('label can be set directly via getWidget', () => {
       const store = useWidgetValueStore()
-      const registered = store.registerWidget(seedA, state('number', 100))
+      const registered = store.registerWidget(seedA, state('number', 100))!
 
       registered.label = 'Random Seed'
       expect(store.getWidget(seedA)?.label).toBe('Random Seed')
@@ -226,4 +242,52 @@ describe('useWidgetValueStore', () => {
       expect(store.getWidget(seedB)?.value).toBe(2)
     })
   })
+
+  describe('un-keyable widget ids', () => {
+    // A custom node can register a widget with an empty/malformed name (spacer,
+    // header, preview, button). Such an id cannot be keyed; the store must
+    // decline it rather than throw and blank every widget on the node.
+    const malformedIds = [
+      widgetId(graphA, toNodeId('node-1'), '') as WidgetId, // empty name
+      'no-colons' as WidgetId,
+      `${graphA}:node-1` as WidgetId, // missing name segment
+      `${graphA}:node-1:seed:extra` as WidgetId, // extra segment
+      `:node-1:seed` as WidgetId, // empty graphId
+      `${graphA}::seed` as WidgetId // empty nodeId
+    ]
+
+    it('registerWidget declines every un-keyable id instead of throwing', () => {
+      const store = useWidgetValueStore()
+      for (const id of malformedIds) {
+        expect(() =>
+          store.registerWidget(id, state('button', null))
+        ).not.toThrow()
+        expect(store.registerWidget(id, state('button', null))).toBeUndefined()
+      }
+    })
+
+    it('getWidget / setValue / deleteWidget tolerate un-keyable ids', () => {
+      const store = useWidgetValueStore()
+      for (const id of malformedIds) {
+        expect(store.getWidget(id)).toBeUndefined()
+        expect(store.setValue(id, 1)).toBe(false)
+        expect(store.deleteWidget(id)).toBe(false)
+      }
+    })
+
+    it('declined operations never disturb a valid sibling on the same node', () => {
+      const store = useWidgetValueStore()
+      store.registerWidget(seedA, state('number', 7))
+
+      for (const id of malformedIds) {
+        store.registerWidget(id, state('button', null))
+        store.setValue(id, 999)
+        store.deleteWidget(id)
+      }
+
+      expect(store.getWidget(seedA)?.value).toBe(7)
+      expect(store.setValue(seedA, 8)).toBe(true)
+      expect(store.getWidget(seedA)?.value).toBe(8)
+    })
+  })
 })

src/stores/widgetValueStore.ts

@@ -4,7 +4,7 @@ import { reactive, ref } from 'vue'
 import type { UUID } from '@/utils/uuid'
 import { parseNodeId } from '@/types/nodeId'
 import type { NodeId, SerializedNodeId } from '@/types/nodeId'
-import { parseWidgetId } from '@/types/widgetId'
+import { isWidgetId, parseWidgetId } from '@/types/widgetId'
 import type { WidgetId } from '@/types/widgetId'
 import type { WidgetState, WidgetStateInit } from '@/types/widgetState'
 
@@ -27,9 +27,19 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
   function registerWidget<TValue = unknown>(
     widgetId: WidgetId,
     init: WidgetStateInit<TValue>
-  ): WidgetState<TValue> {
+  ): WidgetState<TValue> | undefined {
+    if (!isWidgetId(widgetId)) {
+      console.warn(
+        'widgetValueStore.registerWidget: ignoring un-keyable widget id',
+        widgetId
+      )
+      return undefined
+    }
+
     const existing = getWidget(widgetId)
-    if (existing) return existing as WidgetState<TValue>
+    if (existing && existing.type === init.type) {
+      return existing as WidgetState<TValue>
+    }
 
     const { graphId, nodeId, name } = parseWidgetId(widgetId)
     const state: WidgetState<TValue> = {
@@ -44,6 +54,8 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
   }
 
   function getWidget(widgetId: WidgetId): WidgetState | undefined {
+    if (!isWidgetId(widgetId)) return undefined
+
     const { graphId } = parseWidgetId(widgetId)
     return getGraphWidgetStates(graphId).get(widgetId)
   }
@@ -56,6 +68,8 @@ export const useWidgetValueStore = defineStore('widgetValue', () => {
   }
 
   function deleteWidget(widgetId: WidgetId): boolean {
+    if (!isWidgetId(widgetId)) return false
+
     const { graphId } = parseWidgetId(widgetId)
     return getGraphWidgetStates(graphId).delete(widgetId)
   }