fix(chat): use the menu keyboard contract for busy send

[leonlaiyc]

Problem / Motivation

The busy-send picker declares role="menu", but it is wired through the
listbox keyboard hook. That gives it listbox behavior: ArrowUp clamps at the
first row and Tab closes the picker. Other menu surfaces use the shared menu
contract, where arrows wrap and Tab stays within the open menu.

Why it matters

Keyboard and assistive-technology users are told this is a menu, then receive a
different navigation model. At the first row ArrowUp appears broken, and Tab
dismisses the picker instead of cycling through its two choices.

What changed (motivation → approach → change)

The picker now uses the existing useMenuKeyboard hook. That supplies the
repository's one role="menu" contract: focus enters on open, arrows wrap,
Home/End select the boundaries, Tab and Shift-Tab remain contained, and the
document-level IME latch protects navigation keys.

Escape remains owned by BusySendButton, because this host must close the
portal and restore focus to its caret trigger. Selection, portal positioning,
outside-click dismissal, and the persisted busy-send mode are unchanged.

Tests

• Added BusySendButton.menuKeyboard.test.tsx for ArrowUp wrap, bidirectional
  Tab containment while the menu remains open, and Escape focus restoration.
• Red on the old implementation: 2 failed, 1 passed (ArrowUp clamped; Tab
  closed the menu).
• Green after the fix: 3 passed.
• Relevant focused set: 24 passed across the new test, the busy-send slot-scope
  tests, and the shared menu-hook tests.
• Exact-file ESLint: clean.
• tsc -b was attempted with the reused local dependency tree; that tree lacks
  the yaml package and reports only untouched SkillForm.tsx diagnostics.
  Authoritative CI will run against the lockfile-complete install.
• Full repository suites run: 0.

Manual verification

N/A — real-DOM focus assertions exercise every changed keyboard transition,
including the portaled menu and trigger focus restoration.

Screenshots / video

No pixel or layout changes. The delta is keyboard focus behavior inside the
existing two-row picker, which a still image cannot demonstrate; the focused
DOM tests record the transitions directly.

Related Issues

Follow-up to the accepted-and-deferred BusySendButton menu adoption noted by
the First Principles review on merged PR #6547.

Pattern harvest

Rule candidate: review-prompt
Pattern: a role="menu" surface must use useMenuKeyboard or document why its
host-specific keyboard contract differs.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (N/A — the shared menu hook already owns this contract)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 38f02cd67184e06ad509cd1df7d9325bc742b66f via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The patch is small and I've read both hooks and the host component. The picker declares role="menu" but was wired through the listbox hook (clamping arrows, Tab-closes); the change adopts the repo's shared useMenuKeyboard contract. I verified the hook's documented precondition (single open hook-driven menu, outside-mousedown dismissal) holds for this host — BusySendButton already dismisses on outside mousedown — and that host-owned Escape is the hook's sanctioned posture, with focus restoration to the caret trigger preserved. Tests exercise the three changed transitions and were red on the old wiring. No contract, data, or reversibility exposure: the delta is keyboard focus behavior in one two-row picker.

Design-Verdict: PASS

Real role/behavior mismatch fixed by adopting the repo's one existing role="menu" contract; host precondition (single-menu, outside-dismissal) verified to hold.

[DESIGN-REVIEWED] 38f02cd

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 38f02cd67184e06ad509cd1df7d9325bc742b66f via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff is a keyboard-contract swap for the busy-send mode picker (listbox hook → shared menu hook). I've compared the old and new behavior against the repo's shared useMenuKeyboard contract and its sibling role="menu" surfaces: focus still enters the first row on open, Escape still closes and restores the caret trigger (now host-owned, matching the documented contract), arrows now wrap and Tab is contained instead of closing — both aligning this menu with every other menu in the product (MenuBtn, SlotPopover). Focus-restore posture matches the sanctioned outside-click pattern, no strings or visuals change, and the pre-existing unmount edge case is unchanged by this diff. No user-facing regression survives the consequence-chain filter.

UX-Verdict: PASS

Keyboard behavior now matches the product's shared menu contract — wrapping arrows, contained Tab, Escape-to-trigger — with no visible or copy changes.

[UX-REVIEWED] 38f02cd

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 38f02cd67184e06ad509cd1df7d9325bc742b66f via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The fix adopts the repo's established shared menu contract (7 existing useMenuKeyboard hosts); MarkdownPanel is the one remaining role="menu" surface still wired through useListboxKeyboard with the identical defect. Producing the review now.

First-Principles-Verdict: CONCERNS

Right fix at cause level, but MarkdownPanel's role="menu" has the identical listbox wiring and stays broken — a counted, one-hook-swap sibling.

What this change ships

Intent: make the busy-send picker's keyboard behavior match the menu it announces to assistive technology. This is a FIX.

1. ArrowUp/ArrowDown now wrap at the picker's ends instead of stopping — justified (the reported defect)
2. Tab/Shift-Tab now cycle inside the open picker instead of dismissing it — justified (the reported defect)
3. Arrows now work the instant the menu opens, via a document-level listener — rides along with the shared hook, harmless
4. IME composition keys no longer move menu focus — declared, carried by the shared hook
5. Escape still closes and refocuses the caret, now host-owned — justified; the hook documents Escape as host-owned by design
6. Menu container becomes programmatically focusable (tabIndex={-1}) — undeclared
7. New keyboard test pinning wrap, Tab containment, Escape restore — justified

Every item routes through the existing useMenuKeyboard contract rather than adding one; net surface is negative (deletes noInputRef and a 7-argument hook call for a 2-argument one).

Watch

• Point patch with 1 counted unfixed sibling: grepping role="menu" against useListboxKeyboard consumers, MarkdownPanel.tsx:489 declares role="menu" while wiring onListKeyDown from useListboxKeyboard (MarkdownPanel.tsx:438, hasFilterInput: false) — the exact clamp-and-Tab-closes defect this PR fixes, and the same one-hook swap fixes it. The description's own harvested rule ("a role="menu" surface must use useMenuKeyboard or document why") indicts it.
• tabIndex={-1} on the menu container is undeclared and unexplained; the sibling hosts that omit it (MicSourceMenu, AgentPillBar) work without it. Drop it or it becomes cargo the next surface copies.

[FIRST-PRINCIPLES-REVIEWED] 38f02cd

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 38f02cd67184e06ad509cd1df7d9325bc742b66f via the fork AI-review pipeline; updated in place on each push.

Review details

I've examined the diff, both hooks, the full component, and the applicable frontend rules. Let me verify one interaction point — whether the removed useListboxKeyboard had a focus-entry/restore behavior the new hook drops, and confirm the Escape path works through the portal.

The analysis holds:

• Escape: still a React onKeyDown on the same portaled menu <div>; focus lands on a real <button> inside it, so the synthetic event reaches the handler → closeToTrigger(). Unchanged mechanism.
• Arrows/Tab/Home/End: useMenuKeyboard drives a document-level listener with an outside-editable guard; focus is inside the menu container during navigation (containerRef.current.contains(active)), so the guard doesn't suppress it, and Tab is contained.
• Focus entry: focusFirstOnOpen defaults true; the effect runs after the portal commits (menuRef attached), so menuItemsOf finds the rows.
• Focus restore: select and Escape both route through closeToTrigger (restore-on-explicit-dismissal posture); outside-mousedown leaves it to the browser per the hook's documented contract. No stranding path.
• Rows are native <button>s with onClick, so Enter/Space activate natively — the removed onEnterSingleMatch/filteredCount only served a filter input this menu never had.
• No rule triggers: no emoji, no hand-rolled SVG, no dangerouslySetInnerHTML, no hardcoded user-facing string, no unlocalized date/number, icon-only caret keeps its aria-label.

No defect groundable in the changed lines. The candidate list itself found no candidates, and nothing survives Step 2.

No findings.

[OPUS-REVIEWED] 38f02cd

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 38f02cd67184e06ad509cd1df7d9325bc742b66f via the fork AI-review pipeline; updated in place on each push.

Review details

FINDING -- website/src/components/BusySendButton.tsx:136 -- "useMenuKeyboard" lets forward Tab from the first tabIndex={-1} row escape while the menu stays open -> Fix: cycle every Tab/Shift-Tab in the changed onKeyDown.
[GPT-REVIEWED] 38f02cd

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: the portaled busy-send picker advertises role="menu" but was wired to useListboxKeyboard; swapped to the shared useMenuKeyboard contract (arrow wrap, Home/End, Tab containment) with Escape kept host-owned so closing restores the caret trigger. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.