fix(combobox): report every open, not just the dismissals Radix initiates (#6931)

The Combobox owns its `open` state but renders a controlled Radix `Popover`
(`PopoverAnchor` + `open={open}`, no `PopoverTrigger`), and the consumer's
`onOpenChange` hung off that Popover alone. A controlled popover reports only
transitions it initiates itself, so outside-click and Escape arrived and nothing
else did: the trigger, the chevron, focus, Enter/Space/ArrowDown, and
select-to-close were all the component's own `setOpen`, invisible from outside.

Every consumer refreshed on open or reset on close, so the damage stayed quiet —
the credential selectors, MCP tool selector, workspace-file picker, connector
modal, and the sub-block dropdown's remote option list simply never refreshed
when opened. Then #6881 gated the agent block's `toolGroups` on the same signal
to keep the group build off the canvas's hot path, and a picker that could not
learn it was open built nothing: the dropdown rendered "No tools found" over the
full block registry.

Every transition now goes through one `changeOpen`, which Radix's own
`onOpenChange` also feeds, so `setOpen` has exactly one caller and the callback
cannot be missed. It dedupes through a ref, because several paths both close and
let the popover dismiss — a redundancy the raw setState absorbed silently but a
consumer callback would not — and reading that ref lets the toggles resolve
their next value without re-creating their handlers on every open.

Tests cover the transitions Radix never reported (trigger click both ways,
keyboard open, Escape) and the consumer shape that made this visible: options
supplied only once the dropdown says it opened must render, not the empty state.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

packages/emcn/src/components/combobox/combobox.dom.test.tsx

@@ -0,0 +1,114 @@
+/**
+ * @vitest-environment jsdom
+ *
+ * `onOpenChange` is the only signal a consumer has for whether the dropdown is
+ * on screen, and some build their option list from it — the agent block's tool
+ * picker skips building its groups while closed. The popover is controlled, so
+ * Radix reports only the dismissals it initiates itself; every other
+ * transition (trigger click, chevron, focus, keyboard, selecting a row) is the
+ * component's own state write and has to notify on its own. These tests pin
+ * that it does, in both directions.
+ */
+import { act, type ReactNode, useState } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { Combobox } from './combobox'
+
+let root: Root | null = null
+let container: HTMLDivElement | null = null
+
+const OPTIONS = [
+  { label: 'Alpha', value: 'alpha' },
+  { label: 'Beta', value: 'beta' },
+]
+
+function render(node: ReactNode) {
+  ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
+  container = document.createElement('div')
+  document.body.appendChild(container)
+  root = createRoot(container)
+  act(() => root?.render(node))
+}
+
+function trigger(selector = '[role="combobox"]'): HTMLElement {
+  const node = document.querySelector(selector)
+  if (!node) throw new Error(`No ${selector} rendered`)
+  return node as HTMLElement
+}
+
+function click(node: HTMLElement) {
+  act(() => {
+    node.dispatchEvent(new MouseEvent('click', { bubbles: true }))
+  })
+}
+
+function press(node: HTMLElement, key: string) {
+  act(() => {
+    node.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true }))
+  })
+}
+
+afterEach(() => {
+  if (root) act(() => root?.unmount())
+  container?.remove()
+  root = null
+  container = null
+  vi.restoreAllMocks()
+})
+
+describe('Combobox onOpenChange', () => {
+  it('reports the open a trigger click causes', () => {
+    const onOpenChange = vi.fn()
+    render(<Combobox options={OPTIONS} onOpenChange={onOpenChange} />)
+
+    click(trigger())
+
+    expect(onOpenChange).toHaveBeenCalledWith(true)
+  })
+
+  it('reports the close a second trigger click causes', () => {
+    const onOpenChange = vi.fn()
+    render(<Combobox options={OPTIONS} onOpenChange={onOpenChange} />)
+
+    click(trigger())
+    click(trigger())
+
+    expect(onOpenChange).toHaveBeenNthCalledWith(1, true)
+    expect(onOpenChange).toHaveBeenNthCalledWith(2, false)
+  })
+
+  it('reports the open a keyboard press causes', () => {
+    const onOpenChange = vi.fn()
+    render(<Combobox options={OPTIONS} onOpenChange={onOpenChange} />)
+
+    press(trigger(), 'ArrowDown')
+
+    expect(onOpenChange).toHaveBeenCalledWith(true)
+  })
+
+  it('reports the close Escape causes', () => {
+    const onOpenChange = vi.fn()
+    render(<Combobox options={OPTIONS} onOpenChange={onOpenChange} />)
+
+    click(trigger())
+    onOpenChange.mockClear()
+    press(trigger(), 'Escape')
+
+    expect(onOpenChange).toHaveBeenCalledWith(false)
+  })
+
+  it('renders options a consumer supplies only once it is told the dropdown opened', () => {
+    function Picker() {
+      const [open, setOpen] = useState(false)
+      return (
+        <Combobox options={open ? OPTIONS : []} onOpenChange={setOpen} emptyMessage='No tools' />
+      )
+    }
+    render(<Picker />)
+
+    click(trigger())
+
+    expect(document.body.textContent).toContain('Alpha')
+    expect(document.body.textContent).not.toContain('No tools')
+  })
+})

packages/emcn/src/components/combobox/combobox.tsx

@@ -221,6 +221,37 @@ const Combobox = memo(
         setSearchQueryState(next)
         onSearchChangeRef.current?.(next)
       }, [])
+      /**
+       * Read through a ref so `changeOpen` keeps a stable identity — every path
+       * that opens or closes the dropdown captures it without listing it as a
+       * dependency.
+       */
+      const onOpenChangeRef = useRef(onOpenChange)
+      useEffect(() => {
+        onOpenChangeRef.current = onOpenChange
+      }, [onOpenChange])
+      /**
+       * Single write path for the open state so `onOpenChange` cannot be missed.
+       * The popover is controlled, so Radix reports only the dismissals it initiates
+       * itself; the trigger, chevron, focus, keyboard, and selection paths are all
+       * state writes here, and a consumer that refreshes its options on open — or,
+       * like the agent block's tool picker, builds them only while open — hears about
+       * none of them unless each one reports. Deduped, because several paths both
+       * close and let the popover dismiss, which the raw setState absorbed silently
+       * but a consumer callback would not. The ref also lets the toggles read the
+       * current value without re-creating their handlers on every open.
+       */
+      const openRef = useRef(false)
+      const changeOpen = useCallback(
+        (next: boolean) => {
+          if (openRef.current === next) return
+          openRef.current = next
+          setOpen(next)
+          if (!next) updateSearchQuery('')
+          onOpenChangeRef.current?.(next)
+        },
+        [updateSearchQuery]
+      )
       const searchInputRef = useRef<HTMLInputElement>(null)
       const containerRef = useRef<HTMLDivElement>(null)
       const dropdownRef = useRef<HTMLDivElement>(null)
@@ -375,7 +406,7 @@ const Combobox = memo(
             updateSearchQuery('')
             setHighlightedIndex(-1)
             if (!keepOpen) {
-              setOpen(false)
+              changeOpen(false)
             }
             return
           }
@@ -389,7 +420,7 @@ const Combobox = memo(
           } else {
             onChange?.(selectedValue)
             if (!keepOpen) {
-              setOpen(false)
+              changeOpen(false)
               setHighlightedIndex(-1)
               updateSearchQuery('')
               if (editable && inputRef.current) {
@@ -399,7 +430,16 @@ const Combobox = memo(
             }
           }
         },
-        [onChange, multiSelect, onMultiSelectChange, multiSelectValues, editable, inputRef]
+        [
+          onChange,
+          multiSelect,
+          onMultiSelectChange,
+          multiSelectValues,
+          editable,
+          inputRef,
+          changeOpen,
+          updateSearchQuery,
+        ]
       )
 
       /**
@@ -418,10 +458,10 @@ const Combobox = memo(
        */
       const handleFocus = useCallback(() => {
         if (!disabled) {
-          setOpen(true)
+          changeOpen(true)
           setHighlightedIndex(-1)
         }
-      }, [disabled])
+      }, [disabled, changeOpen])
 
       /**
        * Handles blur for editable mode
@@ -438,12 +478,12 @@ const Combobox = memo(
           const isInDropdown = dropdownRef.current?.contains(activeElement)
           const isSearchInput = activeElement === searchInputRef.current
           if (!activeElement || (!isInContainer && !isInDropdown && !isSearchInput)) {
-            setOpen(false)
+            changeOpen(false)
             setHighlightedIndex(-1)
             updateSearchQuery('')
           }
         }, 150)
-      }, [])
+      }, [changeOpen, updateSearchQuery])
 
       /**
        * Handles keyboard navigation
@@ -453,7 +493,7 @@ const Combobox = memo(
           if (disabled) return
 
           if (e.key === 'Escape') {
-            setOpen(false)
+            changeOpen(false)
             setHighlightedIndex(-1)
             updateSearchQuery('')
             if (editable && inputRef.current) {
@@ -471,7 +511,7 @@ const Combobox = memo(
               }
             } else if (!editable) {
               e.preventDefault()
-              setOpen(true)
+              changeOpen(true)
               setHighlightedIndex(0)
             }
             return
@@ -480,7 +520,7 @@ const Combobox = memo(
           if (e.key === ' ' && !editable) {
             e.preventDefault()
             if (!open) {
-              setOpen(true)
+              changeOpen(true)
               setHighlightedIndex(0)
             }
             return
@@ -489,7 +529,7 @@ const Combobox = memo(
           if (e.key === 'ArrowDown') {
             e.preventDefault()
             if (!open) {
-              setOpen(true)
+              changeOpen(true)
               setHighlightedIndex(0)
             } else {
               setHighlightedIndex((prev) => (prev < filteredOptions.length - 1 ? prev + 1 : 0))
@@ -531,6 +571,8 @@ const Combobox = memo(
           editable,
           inputRef,
           onArrowLeft,
+          changeOpen,
+          updateSearchQuery,
         ]
       )
 
@@ -539,10 +581,10 @@ const Combobox = memo(
        */
       const handleToggle = useCallback(() => {
         if (!disabled && !editable) {
-          setOpen((prev) => !prev)
+          changeOpen(!openRef.current)
           setHighlightedIndex(-1)
         }
-      }, [disabled, editable])
+      }, [disabled, editable, changeOpen])
 
       /**
        * Handles chevron click for editable mode
@@ -552,16 +594,14 @@ const Combobox = memo(
           e.preventDefault()
           e.stopPropagation()
           if (!disabled) {
-            setOpen((prev) => {
-              const newOpen = !prev
-              if (newOpen && editable && inputRef.current) {
-                inputRef.current.focus()
-              }
-              return newOpen
-            })
+            const nextOpen = !openRef.current
+            changeOpen(nextOpen)
+            if (nextOpen && editable && inputRef.current) {
+              inputRef.current.focus()
+            }
           }
         },
-        [disabled, editable, inputRef]
+        [disabled, editable, inputRef, changeOpen]
       )
 
       const effectiveHighlightedIndex =
@@ -596,14 +636,7 @@ const Combobox = memo(
       const SelectedIcon = selectedOption?.icon
 
       return (
-        <Popover
-          open={open}
-          onOpenChange={(next) => {
-            setOpen(next)
-            if (!next) updateSearchQuery('')
-            onOpenChange?.(next)
-          }}
-        >
+        <Popover open={open} onOpenChange={changeOpen}>
           <div ref={containerRef} className='relative w-full' {...props}>
             <PopoverAnchor asChild>
               <div className='w-full'>