feat(web): port ConversationActionsMenu from platform (LUM-1661)

[ashleeradka]

Prompt / plan

Port ConversationActionsMenu from vellum-assistant-platform/web/src/components/app/assistant/ConversationActionsMenu/ to the OSS repo. This is the highest-value unblocked migration ticket — it unblocks LUM-1662 (CollapsedConversationsButton) and LUM-1663 (AssistantSideMenu).

What this component does: Dual-surface dropdown/bottom-sheet menu for conversation rows. On desktop, renders a Radix Menu.Root dropdown triggered by a hover-revealed ellipsis button. On mobile (useIsMobile()), renders a BottomSheet with PanelItem rows. Both surfaces render the same item set: Pin/Unpin, Rename, Archive/Unarchive, Mark as read/unread, Move to Group (submenu), Analyze, Open in New Window, Share Feedback, Inspect, Copy/Fork/Refresh.

Key exports:

• ConversationActionsMenu — the main component (dropdown trigger + menu content)
• renderConversationMenuItems() — shared item builder accepting a ConversationMenuPrimitive type param so both the dropdown and the right-click context menu (future LUM-1663) render identical items
• ConversationMenuPrimitive type — union of Menu and ContextMenu compound APIs
• ConversationMenuItemsProps interface — all action handlers and state flags

Convention compliance:

• kebab-case filename (conversation-actions-menu.tsx)
• .js extensions on all imports
• @/ aliases for app-layer imports
• Design library barrel imports (@vellum/design-library)
• All Capacitor/mobile code paths preserved (BottomSheet branch, useIsMobile)
• No "use client" directive

Improvements over platform source:

• Default trigger uses native <button type="button"> instead of <span role="button"> with manual keyboard handling — Radix handles Enter/Space and aria attributes natively (MDN: button element, Radix Menu.Trigger)
• MobileMenuDivider uses Tailwind className instead of inline style for consistency with the repo's no-inline-style convention

Dependencies (all pre-existing in OSS repo):

• Design library: Menu, ContextMenu, BottomSheet, PanelItem
• App: useIsMobile hook, MoveToGroupTarget type from groupConversations
• Icons: 17 lucide-react icons

Test plan

17 tests covering:

• renderConversationMenuItems pure helper: Pin/Unpin, Archive/Unarchive, read-only gating, Move to Group submenu, header variant ordering, mark read/unread mutual exclusivity
• Desktop branch: default trigger with native button + aria attributes, menu item rendering
• Mobile branch: BottomSheet surface, archive, disabled mark-unread panel item, Move-to submenu with remove-from-group, header variant ordering
• Read-only conversations: Archive visible, Mark-as-unread/Analyze hidden

Verification:

• bun test src/domains/chat/components/conversation-actions-menu.test.tsx — 17/17 pass
• bun run lint — clean
• bun run build — succeeds
• CI typecheck and build verify full compilation

Closes LUM-1661

Link to Devin session: https://app.devin.ai/sessions/565d827296144ac9bf12bd108169e5ef
Requested by: @ashleeradka

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[linear]

LUM-1661

[vex-assistant-bot]

✦ APPROVE

Value: Gets the conversation row action menu into the OSS repo — the last piece blocking the sidebar assembly (LUM-1662 + LUM-1663 can't land until this is in.

What this does: Ports ConversationActionsMenu as two new files (~730 + ~282 lines). The component splits on useIsMobile(): desktop renders a Radix Menu.Root dropdown; mobile renders a BottomSheet with PanelItem rows. Both surfaces consume a shared renderConversationMenuItems pure helper, so item definitions live exactly once and context-menu re-use (LUM-1663) gets them for free via the ConversationMenuPrimitive type param.

Architecture — clean:

• renderConversationMenuItems is a pure function (no hooks, no side effects) — straightforward to test with renderToStaticMarkup and easy to reuse from the right-click context menu. The ConversationMenuPrimitive abstraction is the right move; Menu and ContextMenu share the same compound-component shape, and this makes that structural fact explicit.
• buildPanelItem helper in the mobile path keeps the bottom-sheet items DRY and the run → onClose() ordering correct (action fires before sheet dismisses, preventing UI from appearing under a still-open sheet).
• variant prop cleanly handles header vs. sidebar ordering differences without branching on call sites. The header variant's item order (Copy → Fork → Analyze → Open → Refresh → Pin → Rename → Archive → Inspect) matches the expected macOS-parity sequence.
• Read-only gating (hiding Analyze / Mark as unread, keeping Archive) is well-commented and consistently applied across both dropdown and panel renderers.

Tests — adequate:
13 tests cover the critical paths: Pin/Unpin toggle, Archive/Unarchive flip, read-only gating, Move to Group submenu, header variant ordering, desktop + mobile branch rendering, and the read-only Archive-stays-visible case. The renderToStaticMarkup approach is the right call given the workspace's lack of jsdom.

Non-blocking notes:

1. MobileMenuDivider uses inline style={{background: "var(--border-overlay)"}} — inconsistent with the rest of the file which uses bg-[var(--surface-hover)] Tailwind arbitrary notation. Should be className="bg-[var(--border-overlay)]". Minor.

2. Default trigger is <span role="button"> not <button> — keyboard handling (Enter/Space) is manually wired, and Radix asChild handles expanded state, so this is functionally fine. A native <button type="button"> would be semantically cleaner and avoid the (event.currentTarget as HTMLElement).click() cast in the onKeyDown handler. Non-blocking since the a11y coverage is complete.

3. Missing test coverage (not regressions, just gaps for future):

   • onMarkRead vs onMarkUnread mutual exclusivity (both provided simultaneously)
   • isMarkUnreadDisabled in the mobile panel path
   • onRemoveFromGroup item appearing in the mobile "Move to" block
   • variant="header" in the mobile branch

CI: 3/3 green (Socket × 2, Lint/Type Check & Build). Codex is still reviewing (👀).

Merge gate: ✅ Vex approved · ⏳ awaiting second approval (Codex 👍 or Devin APPROVE) · ✅ CI green.

Vellum Constitution — Yours: a single source of truth for menu items means the assistant's actions stay consistent and predictable no matter how they're invoked.

[devin-ai-integration]

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

[devin-ai-integration]

🚩 No callers of the new exports yet — component is orphaned

Searched for imports of conversation-actions-menu across apps/web/src/ and found no callers. The component (ConversationActionsMenu), helper (renderConversationMenuItems), and type (ConversationMenuPrimitive) are all exported but unused. This is expected during an incremental migration (per CONTRIBUTING.md's active migration status for apps/web/), but worth confirming this is part of a planned integration rather than dead code.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Correct — this is an incremental migration port. The component will be consumed by LUM-1662 (CollapsedConversationsButton) and LUM-1663 (AssistantSideMenu) in follow-up PRs. No dead code.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7580b7762b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Let Radix handle Enter/Space on the default trigger

The custom onKeyDown prevents default on Enter/Space and then calls click() manually, which bypasses Radix DropdownMenu’s keyboard-open path (it listens on key/pointer events, not synthetic click from keyboard). In practice this can leave the default ellipsis trigger non-operable for keyboard users on desktop (Tab focus + Enter/Space does not reliably open the menu). Removing the manual key handling (or not preventing default) lets Radix preserve its built-in accessible trigger behavior.

Useful? React with 👍 / 👎.

[devin-ai-integration]

Fixed in fcf1389 — replaced <span role="button"> with a native <button type="button">. This eliminates the manual onKeyDown handler entirely (native buttons handle Enter/Space natively) and removes the HTMLElement cast. Radix Menu.Trigger manages aria-haspopup/aria-expanded automatically on the child element.

Also fixed the MobileMenuDivider inline style → Tailwind className in the same commit per Vex's note.

[devin-ai-integration]

All 3 Vex review notes addressed:

1. MobileMenuDivider inline style → Fixed in fcf1389 — uses className="bg-[var(--border-overlay)]" now.
2. <span role="button"> → Fixed in fcf1389 — native <button type="button">, manual keyboard handling removed, Radix manages aria attributes.
3. Missing test coverage → Added in 285bfe6 — 4 new tests covering:
   • onMarkRead vs onMarkUnread mutual exclusivity (Mark as read takes precedence)
   • isMarkUnreadDisabled in mobile panel path (pointer-events-none + opacity-50)
   • onRemoveFromGroup in mobile "Move to" block
   • variant="header" in mobile branch (header-order items as panel items)

17/17 tests pass.

[vex-assistant-bot]

✦ REQUEST_CHANGES (correction supersedes my earlier APPROVE at 7580b776)

Reviewed at 285bfe63. New commit since my approval adds 4 Devin-authored tests (mark read/unread exclusivity, mobile disabled state, mobile Move-to submenu, mobile header variant) — pure additive coverage, +62 lines in the test file. Does not address the keyboard-accessibility issue I'm flagging below, so the gate stands.

---

Correction: Codex's P1 at line 687 is real, and my prior "non-blocking" note #2 underestimated it. The default trigger is not operable via keyboard.

Why — defaultTrigger onKeyDown:

onKeyDown={(event) => {
  if (event.key === "Enter" || event.key === " ") {
    event.preventDefault();
    event.stopPropagation();
    (event.currentTarget as HTMLElement).click();
  }
}}

Two things conspire:

1. event.preventDefault() short-circuits Radix's own handler. DropdownMenu.Trigger (via asChild) composes handlers using composeEventHandlers, which skips its own logic when event.defaultPrevented === true. Radix's keyboard-open path never runs.
2. Synthetic .click() from a keyboard handler has event.detail === 0. Radix DropdownMenu.Trigger's click handler intentionally filters keyboard-initiated clicks (to avoid double-handling alongside the keydown path). The manual .click() is silently dropped.

Net: focus the ellipsis with Tab, hit Enter or Space → menu does not open. WCAG 2.1.1 (Keyboard) regression. Ships the moment LUM-1662 / LUM-1663 wire this up.

Minimal fix — drop the manual handler entirely:

const defaultTrigger = (
  <span
    role="button"
    tabIndex={0}
    aria-label="Conversation actions"
    aria-haspopup="menu"
    onClick={(event) => event.stopPropagation()}
    onContextMenu={(event) => {
      event.stopPropagation();
      event.preventDefault();
    }}
    className="..."  /* unchanged */
  >
    <MoreHorizontal size={14} aria-hidden />
  </span>
);

Radix's composed handler will then own Enter/Space/ArrowDown with proper first-item focus.

Better fix: switch the span to <button type="button">. Gets you disabled semantics + native keyboard handling for free, drops the role="button" / tabIndex={0} boilerplate.

---

What still stands from my prior approval:

• ✅ Architecture (shared renderConversationMenuItems, ConversationMenuPrimitive type param, mobile/desktop split) — clean, reusable for LUM-1663 context menu.
• ✅ Tests now 17 total (13 original + 4 new) — strong coverage across surfaces, read-only gating, submenu, header variant ordering on both desktop and mobile.
• ✅ Convention compliance (kebab-case, .js extensions, @/ aliases, design-library barrel imports).
• ✅ MobileMenuDivider inline style → Tailwind arbitrary bg-[var(--border-overlay)] (minor, non-blocking).
• ✅ Devin's "orphaned exports" finding is expected during incremental migration (Devin self-confirmed). Not a concern.

Merge gate:

• 🔴 Fix defaultTrigger keyboard handling (drop manual onKeyDown or convert to <button>)
• ⏳ CI: Socket green, Lint/Type/Build still running on new commit
• ⏳ Re-trigger Codex + Devin after the fix
• ⏳ Second approval

Vellum Constitution — Trust-seeking: keyboard navigation is a load-bearing accessibility commitment for the macOS-parity sidebar. The bug is invisible today because nothing consumes this component yet, but it ships silently the moment LUM-1662 lands.

[devin-ai-integration]

This was already fixed in commit fcf1389 (the commit before the test additions). The <span role="button"> with manual onKeyDown was replaced with a native <button type="button"> — exactly the "better fix" you recommend here. No onKeyDown, no role="button", no tabIndex={0}, no (event.currentTarget as HTMLElement).click() cast. Radix owns Enter/Space/ArrowDown handling via Menu.Trigger asChild.

Current code at line 672:

const defaultTrigger = (
  <button
    type="button"
    aria-label="Conversation actions"
    onClick={(event) => event.stopPropagation()}
    onContextMenu={(event) => {
      event.stopPropagation();
      event.preventDefault();
    }}
    className="..."
  >
    <MoreHorizontal size={14} aria-hidden />
  </button>
);

The REQUEST_CHANGES appears to be reviewing the original commit (7580b77), not the current HEAD (285bfe6 which includes fcf1389).

[vex-assistant-bot]

✦ APPROVE — correcting my prior REQUEST_CHANGES, which I posted in error

Reviewed at 285bfe63.

I apologize for the noise. My earlier REQUEST_CHANGES at this SHA was wrong: commit fcf13896 (between my original approval and current HEAD) already replaced the <span role="button"> with a native <button type="button"> and removed the manual onKeyDown entirely — exactly the fix I was asking for. When I re-read after the HEAD moved I looked at the patch of the latest commit only (285bfe63, the +62 lines of tests) instead of diffing the full file against my original approval SHA, so I missed the middle commit. Devin called this out correctly.

Verified at current HEAD (line 671-683):

const defaultTrigger = (
  <button
    type="button"
    aria-label="Conversation actions"
    onClick={(event) => event.stopPropagation()}
    onContextMenu={(event) => {
      event.stopPropagation();
      event.preventDefault();
    }}
    className="flex h-6 w-6 items-center justify-center rounded-[4px] outline-none focus-visible:ring-2 focus-visible:ring-[var(--ring)] text-[var(--content-tertiary)] transition-colors hover:bg-[var(--surface-hover)] hover:text-[var(--content-secondary)] aria-[expanded=true]:bg-[var(--surface-active)] aria-[expanded=true]:text-[var(--content-emphasised)]"
  >
    <MoreHorizontal size={14} aria-hidden />
  </button>
);

Radix Menu.Trigger / BottomSheet.Trigger via asChild now owns Enter/Space/ArrowDown with proper first-item focus. WCAG 2.1.1 concern resolved.

Also verified resolved:

• ✅ MobileMenuDivider now uses className="my-1 h-px bg-[var(--border-overlay)]" (Tailwind arbitrary, matches the rest of the file).
• ✅ +4 Devin tests covering mark read/unread exclusivity, mobile disabled state, mobile Move-to submenu, mobile header variant ordering. Total now 17 tests.
• ✅ CI 3/3 green (Socket PR Alerts, Socket Project Report, Lint/Type/Build).

Architecture, conventions, and prior approval criteria all stand. Ready for merge once second human approval lands.

Procedural lesson for me: when re-reviewing after a HEAD move, diff the file against the original approval SHA, not just the patch of the latest commit. Adding to my review skill.