[Juliusolsson05]

Fixes #867.

The bug

Agent View Mode, Reorder Tabs and Pin Agents each handled Enter on DialogContent, called preventDefault — which cancels the focused button's native click — and ran the list's action instead.

dialog	Tab to Cancel, Enter
Agent View Mode	the highlighted override is applied
Reorder Tabs	confirm() commits the reorder being abandoned
Pin Agents	saves the unpinning; with nothing selected, an empty list

That last one is what DialogActions' confirmDisabled exists to prevent.

dialog-actions.tsx already documents the rule and #860 exported focusedControlOwnsEnter for exactly this; #862 fixed the same bug in Switch Provider. These are the three the audit of all 44 DialogContent consumers found afterwards.

Reorder Tabs shows why the check is on the focused control rather than on the footer slot: its footer is a plain <div>, not a DialogFooter.

The other half

Each dialog's list rows now leave the tab order, because Enter handling cannot cover this: Space clicks the focused control, so a Tab-focused row acts on a row other than the arrow-driven highlight the dialog is showing, whatever the Enter branch does.

Reorder Tabs' per-row Move up / Move down buttons stay focusable on purpose — they name their target (Move B up), so acting on the focused one is unambiguous. The test excludes aria-labelled buttons and says why, rather than sweeping them in.

Tests

Six, plus three controls:

• one per dialog for the bug (fireEvent.keyDown(...) === true asserted beside "the handler did not act" — it means the default was not prevented, which is what lets the real browser deliver the button's click; happy-dom does not synthesize it, so neither assertion alone is the property);
• one per dialog for the tab order;
• three controls proving the list still owns Enter when the list has focus. Without them, "Enter never acts" would satisfy the first three and every one of these dialogs would be broken. Reorder Tabs' control runs the whole two-phase model — Enter picks, arrows move, Enter commits — and asserts the reordered ids, because "entered move mode" is invisible from outside.

Mutation results

mutation	result
guard removed in Agent View Mode	1 failed
guard removed in Reorder Tabs	1 failed
guard removed in Pin Agents	1 failed
row tabIndex removed in Agent View Mode	1 failed
row tabIndex removed in Reorder Tabs	1 failed
row tabIndex removed in Pin Agents	1 failed

No survivors.

npx tsc -b clean on Node 24. --project renderer --project unit src/renderer/src/features/ src/renderer/src/components/: 938 passed.

🤖 Generated with Claude Code

---

Review round (reviewer requested changes; fixed in 62304098)

Two HIGH findings, both mine: tabIndex={-1} is half of the roving-focus pattern and this PR shipped only that half.

• Pin Sessions had no onOpenAutoFocus, so with the rows out of the tab order Radix focused Cancel — the very first Enter cancelled and discarded the pins.
• tabIndex={-1} does not stop CLICK focus. A clicked row held focus and owned every following Enter, so the dialog's own Enter bowed out permanently: in Pin Sessions Enter re-clicked the row, in Reorder Tabs move mode could never be entered again. Rows now cancel mousedown.
• Space had the identical bug (Cancel focused → Cancel does nothing and a pin silently toggles), fixed with a separate focusedControlOwnsSpace, because Space activates a button but only scrolls a link.
• The audit missed two consumers: ConversationsPicker (Enter on a filter chip resumed the highlighted conversation) and AgentActivityModal (an opacity-0 per-row close button was a tab stop, so Delete closed the selected row while a different row's button had focus).
• ARIA: the rows left the tab order without aria-activedescendant; all three lists are now proper listboxes.
• Reorder ↑/↓ buttons: the move now carries the picked marker and restores focus, since a move can disable the button that performed it.
• Coverage: the guard is now exercised as a predicate (Enter from a non-control descendant), the Move-button carve-out is asserted rather than argued, and the row filter uses role="option" instead of subtracting footer labels.

Post-fix mutation battery (954 tests) is in the review reply; every previously surviving mutation now fails, with one equivalent mutant reported.

[Juliusolsson05]

Both HIGH findings are real and both are mine — tabIndex={-1} is half of the roving-focus pattern and I shipped only that half. Fixed in 62304098. Point by point.

1. [HIGH] Pin Sessions auto-focuses Cancel → Enter discards. Accepted exactly as diagnosed: the two siblings had onOpenAutoFocus, this one did not, and taking the rows out of the tab order promoted Cancel to first-tabbable. Fixed with the same prop. The test asserts the starting focus and that the advertised Enter commits from it with no rescue gesture in between — document.activeElement === dialog, then onConfirm called.

2. [HIGH] Click focus. Accepted. Rows in all three dialogs now onMouseDown={e => e.preventDefault()}, which is the companion you named. happy-dom does not focus on click, so asserting the symptom would pass vacuously; the tests assert the structural property (fireEvent.mouseDown(row) === false) and the comment says why that stands in for the browser behaviour. The predicate's docstring now carries the rule too — it was the thing reasoning only about Tab.

3. [MEDIUM-HIGH] Space. Accepted, including that it made the comment false. Guarded with a separate focusedControlOwnsSpace, not the Enter predicate: Space presses a button and types into a text field, but on a link it scrolls, so reusing the Enter list would have swallowed that scroll. Both predicates are now pinned directly against <a>, which is the only place the difference is observable.

4. [MEDIUM] Incomplete audit. Both fixed.

• ConversationsPicker — guarded, with the chip case asserted against a real render (Enter on "everywhere" leaves aria-pressed="false" and replaceSession uncalled). Your framing is the one I put in the comment: the rule is about the focused control, not the footer slot.
• AgentActivityModal — the close button takes tabIndex={-1} plus the mousedown guard rather than a key-handler change, because the defect is that an opacity-0 button was a tab stop at all. Keeping focus on the scroller is what keeps "the row the keys act on" and "the row the user sees" the same row. Test asserts it is untabbable, cancels mousedown, and still closes on a real click.

I did not re-audit all 44 DialogContent consumers; I fixed the two you found and corrected "36" in the body.

5. [MEDIUM] Three survivors.

M-b is the one that mattered and you are right about why: every existing case fires Enter at Cancel or at the dialog root, which the weaker predicate answers identically. There is now a case firing Enter at the listbox — a descendant that is not a control — so the predicate is exercised as a predicate. M-d is asserted (the four Move buttons must have no tabindex). M-f I left as you scoped it.

Post-fix, whole features+components suites (954 tests):

mutation	result
weaker predicate target !== currentTarget	1 failed
tabIndex={-1} on the Move buttons	1 failed
drop Pin Sessions' onOpenAutoFocus	1 failed
drop the Space guard	1 failed
drop the row onMouseDown (pin / reorder)	1 failed each
drop aria-activedescendant	1 failed
drop the ConversationsPicker guard	1 failed
drop the activity-row tabIndex	1 failed
drop the moving-marker carry	1 failed
drop the focus restore	1 failed
Space guard reuses the Enter predicate	equivalent — see below

The last one survives and I am reporting it rather than hiding it: inside these three dialogs the two predicates cannot differ, because none of them contains a link. The distinction is real and is pinned on the predicates themselves.

Worth recording: my first version of the focus-restore test (finding 7) passed on the broken code. fireEvent.click moves no focus in happy-dom, so document.activeElement was still the dialog from mount and the assertion proved nothing. It now calls up.focus() first and asserts that precondition — the mutation kills it.

6. [LOW-MEDIUM] ARIA. Accepted — this is a regression I caused, not a pre-existing gap: those rows were reachable and announced before. All three lists are now role="listbox" + role="option" + aria-activedescendant, asserted for each dialog (the active id must resolve to a row inside the listbox). Still not verified with a real screen reader, same caveat as yours.

7. [LOW] Move-button edges. Both fixed, and I agree these are this PR's to own since Enter only reaches those buttons because of the guard. moveTabById now carries the picked marker with the row it moved — a row click already did this and the buttons did not — and restores focus to the dialog, because a disabled control keeps focus while dropping out of the event path.

8, 9. Nits. Indentation fixed. The row filter is now role="option" instead of label subtraction, which removes both the dead labels and the startsWith oracle. Unused useAppStore import removed.

npx tsc -b clean; 343 files / 2519 tests pass across the renderer.