fix(network): harden custom address picker

src/renderer/src/components/network/AddressPicker.test.tsx

@@ -0,0 +1,187 @@
+// @vitest-environment happy-dom
+
+import '@testing-library/jest-dom/vitest'
+
+import type { ComponentProps } from 'react'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { cleanup, render, screen, waitFor, within } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
+import { AddressPicker } from './AddressPicker'
+
+afterEach(() => {
+  cleanup()
+  vi.restoreAllMocks()
+})
+
+const LAN_OPTION = { value: '192.168.1.24', label: '192.168.1.24 (en0)' }
+const CUSTOM_VALUE = 'orca.example.com'
+
+function pickerProps(
+  overrides: Partial<ComponentProps<typeof AddressPicker>> = {}
+): ComponentProps<typeof AddressPicker> {
+  return {
+    options: [LAN_OPTION],
+    value: LAN_OPTION.value,
+    onValueChange: vi.fn(),
+    formatCustomLabel: (value) => `${value} (custom)`,
+    addCustomLabel: 'Add custom address…',
+    customDialogCopy: {
+      title: 'Custom network address',
+      description: 'Use an address another device can reach.',
+      inputLabel: 'Address',
+      placeholder: 'orca.example.com',
+      hint: 'Enter a hostname or IP address.',
+      cancel: 'Cancel',
+      confirm: 'Use address'
+    },
+    validateCustom: (input) =>
+      input.trim() === '' ? { ok: false } : { ok: true, value: input.trim() },
+    customInputId: 'custom-address',
+    placeholder: 'No addresses found',
+    triggerAriaLabel: 'Network address',
+    ...overrides
+  }
+}
+
+describe('AddressPicker', () => {
+  it('preserves the active item when a custom address becomes discovered', async () => {
+    const user = userEvent.setup()
+    const onValueChange = vi.fn()
+    const { rerender } = render(
+      <AddressPicker
+        {...pickerProps({ value: CUSTOM_VALUE, onValueChange, options: [LAN_OPTION] })}
+      />
+    )
+
+    await user.click(screen.getByRole('combobox'))
+    const customItem = screen.getByRole('option', { name: `${CUSTOM_VALUE} (custom)` })
+
+    rerender(
+      <AddressPicker
+        {...pickerProps({
+          value: CUSTOM_VALUE,
+          onValueChange,
+          options: [LAN_OPTION, { value: CUSTOM_VALUE, label: `${CUSTOM_VALUE} (tailscale0)` }]
+        })}
+      />
+    )
+
+    expect(screen.getByRole('option', { name: `${CUSTOM_VALUE} (tailscale0)` })).toBe(customItem)
+  })
+
+  it('renders the first label for duplicate address values', async () => {
+    const user = userEvent.setup()
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined)
+    render(
+      <AddressPicker
+        {...pickerProps({
+          options: [
+            LAN_OPTION,
+            { value: LAN_OPTION.value, label: `${LAN_OPTION.value} (bridge0)` },
+            { value: '100.64.1.20', label: '100.64.1.20 (tailscale0)' }
+          ]
+        })}
+      />
+    )
+
+    await user.click(screen.getByRole('combobox'))
+    const listbox = screen.getByRole('listbox')
+    const renderedOptions = within(listbox).getAllByRole('option')
+
+    expect(renderedOptions.map((option) => option.textContent)).toEqual([
+      LAN_OPTION.label,
+      '100.64.1.20 (tailscale0)'
+    ])
+    expect(within(listbox).getByRole('option', { name: LAN_OPTION.label })).toBeVisible()
+    expect(within(listbox).queryByRole('option', { name: /bridge0/ })).toBeNull()
+    expect(consoleError.mock.calls.flat().join(' ')).not.toContain('same key')
+  })
+
+  it('uses stable address values for typeahead', async () => {
+    const user = userEvent.setup()
+    render(
+      <AddressPicker
+        {...pickerProps({
+          options: [LAN_OPTION, { value: CUSTOM_VALUE, label: `Tailnet host (${CUSTOM_VALUE})` }]
+        })}
+      />
+    )
+
+    await user.click(screen.getByRole('combobox'))
+    await user.keyboard('o')
+
+    expect(screen.getByRole('option', { name: `Tailnet host (${CUSTOM_VALUE})` })).toHaveFocus()
+  })
+
+  it('keeps the custom action outside the address listbox', async () => {
+    const user = userEvent.setup()
+    render(<AddressPicker {...pickerProps()} />)
+
+    const addButton = screen.getByRole('button', { name: 'Add custom address…' })
+    await user.click(screen.getByRole('combobox'))
+
+    const listbox = screen.getByRole('listbox')
+    expect(within(listbox).queryByRole('button', { name: 'Add custom address…' })).toBeNull()
+    expect(within(listbox).getAllByRole('option')).toHaveLength(1)
+    expect(addButton).toHaveAttribute('aria-haspopup', 'dialog')
+  })
+
+  it('restores focus to the custom action after every dialog close path', async () => {
+    const user = userEvent.setup()
+    const onValueChange = vi.fn()
+    render(<AddressPicker {...pickerProps({ onValueChange })} />)
+
+    const addButton = screen.getByRole('button', { name: 'Add custom address…' })
+    await user.click(addButton)
+    expect(screen.getByLabelText('Address')).toHaveFocus()
+    expect(onValueChange).not.toHaveBeenCalled()
+
+    await user.keyboard('{Escape}')
+    await waitFor(() => expect(addButton).toHaveFocus())
+
+    await user.keyboard('{Enter}')
+    await user.click(screen.getByRole('button', { name: 'Cancel' }))
+    await waitFor(() => expect(addButton).toHaveFocus())
+
+    await user.keyboard('{Enter}')
+    await user.type(screen.getByLabelText('Address'), CUSTOM_VALUE)
+    await user.click(screen.getByRole('button', { name: 'Use address' }))
+
+    expect(onValueChange).toHaveBeenCalledWith(CUSTOM_VALUE)
+    await waitFor(() => expect(addButton).toHaveFocus())
+  })
+
+  it('does not reseed an open custom dialog when options refresh', async () => {
+    const user = userEvent.setup()
+    const onValueChange = vi.fn()
+    const { rerender } = render(
+      <AddressPicker
+        {...pickerProps({ value: CUSTOM_VALUE, onValueChange, options: [LAN_OPTION] })}
+      />
+    )
+
+    await user.click(screen.getByRole('button', { name: 'Add custom address…' }))
+    const input = screen.getByLabelText('Address')
+    await user.clear(input)
+    await user.type(input, 'edited.example.com')
+
+    rerender(
+      <AddressPicker
+        {...pickerProps({
+          value: CUSTOM_VALUE,
+          onValueChange,
+          options: [LAN_OPTION, { value: CUSTOM_VALUE, label: `${CUSTOM_VALUE} (tailscale0)` }]
+        })}
+      />
+    )
+
+    expect(input).toHaveValue('edited.example.com')
+  })
+
+  it('disables both address controls', () => {
+    render(<AddressPicker {...pickerProps({ disabled: true })} />)
+
+    expect(screen.getByRole('combobox')).toBeDisabled()
+    expect(screen.getByRole('button', { name: 'Add custom address…' })).toBeDisabled()
+  })
+})

src/renderer/src/components/network/AddressPicker.tsx

@@ -1,13 +1,7 @@
-import React, { useState } from 'react'
+import React, { useRef, useState } from 'react'
 import { Plus } from 'lucide-react'
-import {
-  Select,
-  SelectContent,
-  SelectItem,
-  SelectSeparator,
-  SelectTrigger,
-  SelectValue
-} from '../ui/select'
+import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select'
+import { Button } from '../ui/button'
 import {
   CustomAddressDialog,
   type CustomAddressDialogCopy,
@@ -19,9 +13,16 @@ export type AddressOption = {
   label: string
 }
 
-// Why: a sentinel Select value for the footer action. It is never committed as
-// a real address; selecting it opens the custom-address dialog instead.
-const ADD_CUSTOM_VALUE = '__add_custom_address__'
+function getUniqueAddressOptions(options: readonly AddressOption[]): AddressOption[] {
+  const seenValues = new Set<string>()
+  return options.filter((option) => {
+    if (seenValues.has(option.value)) {
+      return false
+    }
+    seenValues.add(option.value)
+    return true
+  })
+}
 
 export type AddressPickerProps = {
   options: readonly AddressOption[]
@@ -42,10 +43,6 @@ export type AddressPickerProps = {
   id?: string
 }
 
-// Note (crash cluster C6, React #185 in settings): the throwing dispatch is in
-// @radix-ui/react-select's SelectItem unmount cleanup, not in our code — this
-// file holds no unmount-phase setState, so there is nothing here to guard.
-// Item churn here is derived from props only; re-audit if that stops holding.
 export function AddressPicker({
   options,
   value,
@@ -62,47 +59,51 @@ export function AddressPicker({
   id
 }: AddressPickerProps): React.JSX.Element {
   const [dialogOpen, setDialogOpen] = useState(false)
+  const dialogInitialValueRef = useRef<string | undefined>(undefined)
 
-  const isCustomSelection =
-    value !== undefined && value !== '' && !options.some((option) => option.value === value)
-
-  const handleValueChange = (next: string): void => {
-    if (next === ADD_CUSTOM_VALUE) {
-      setDialogOpen(true)
-      return
+  const uniqueOptions = getUniqueAddressOptions(options)
+  const customOption =
+    value !== undefined && value !== '' && !uniqueOptions.some((option) => option.value === value)
+      ? { value, label: formatCustomLabel(value) }
+      : undefined
+  const selectOptions = customOption ? [...uniqueOptions, customOption] : uniqueOptions
+  const handleDialogOpenChange = (open: boolean): void => {
+    if (open) {
+      dialogInitialValueRef.current = customOption?.value
     }
-    onValueChange(next)
+    setDialogOpen(open)
   }
 
   return (
     <>
-      <Select value={value ?? ''} onValueChange={handleValueChange} disabled={disabled}>
+      <Select value={value ?? ''} onValueChange={onValueChange} disabled={disabled}>
         <SelectTrigger id={id} size="sm" className={className} aria-label={triggerAriaLabel}>
           <SelectValue placeholder={placeholder} />
         </SelectTrigger>
         <SelectContent>
-          {options.map((option) => (
-            <SelectItem key={option.value} value={option.value}>
+          {selectOptions.map((option) => (
+            <SelectItem key={option.value} value={option.value} textValue={option.value}>
               {option.label}
             </SelectItem>
           ))}
-          {isCustomSelection ? (
-            <SelectItem value={value}>{formatCustomLabel(value)}</SelectItem>
-          ) : null}
-          {options.length > 0 || isCustomSelection ? <SelectSeparator /> : null}
-          <SelectItem
-            value={ADD_CUSTOM_VALUE}
-            className="text-muted-foreground focus:text-foreground"
-          >
-            <Plus className="size-3.5" />
-            {addCustomLabel}
-          </SelectItem>
         </SelectContent>
       </Select>
       <CustomAddressDialog
         open={dialogOpen}
-        onOpenChange={setDialogOpen}
-        initialValue={isCustomSelection ? value : undefined}
+        onOpenChange={handleDialogOpenChange}
+        trigger={
+          <Button
+            type="button"
+            variant="ghost"
+            size="sm"
+            disabled={disabled}
+            className="max-w-full text-muted-foreground"
+          >
+            <Plus className="size-3.5" />
+            <span className="min-w-0 truncate">{addCustomLabel}</span>
+          </Button>
+        }
+        initialValue={dialogInitialValueRef.current}
         validate={validateCustom}
         copy={customDialogCopy}
         inputId={customInputId}

src/renderer/src/components/network/CustomAddressDialog.tsx

@@ -5,7 +5,8 @@ import {
   DialogDescription,
   DialogFooter,
   DialogHeader,
-  DialogTitle
+  DialogTitle,
+  DialogTrigger
 } from '@/components/ui/dialog'
 import { Button } from '@/components/ui/button'
 import { Input } from '@/components/ui/input'
@@ -26,6 +27,7 @@ export type CustomAddressDialogCopy = {
 type CustomAddressDialogProps = {
   open: boolean
   onOpenChange: (open: boolean) => void
+  trigger?: React.ReactElement
   // Why: prefill from the current selection when it is already a custom value
   // so reopening to edit shows what is in use rather than a blank field.
   initialValue?: string
@@ -42,6 +44,7 @@ type CustomAddressDialogProps = {
 export function CustomAddressDialog({
   open,
   onOpenChange,
+  trigger,
   initialValue,
   validate,
   copy,
@@ -73,6 +76,7 @@ export function CustomAddressDialog({
 
   return (
     <Dialog open={open} onOpenChange={onOpenChange}>
+      {trigger ? <DialogTrigger asChild>{trigger}</DialogTrigger> : null}
       <DialogContent className="sm:max-w-md">
         <DialogHeader>
           <DialogTitle>{copy.title}</DialogTitle>