feat: Unified Inbox with Focus Mode (Encore Feature)#474
Open
felipeggv wants to merge 7 commits intoRunMaestro:mainfrom
Open
feat: Unified Inbox with Focus Mode (Encore Feature)#474felipeggv wants to merge 7 commits intoRunMaestro:mainfrom
felipeggv wants to merge 7 commits intoRunMaestro:mainfrom
Conversation
Port 6 inbox files from feat/unified-inbox-encore (which already incorporates all 8 rounds of CodeRabbit/Greptile review fixes) and integrate into the existing codebase with full App.tsx handler wiring. CRITICAL fixes included: - Race condition: effect-driven pendingInboxQuickReply replaces setTimeout - Memory leak: resizeCleanupRef + useEffect unmount guard for drag listeners - Group collision: sessionId as group key, sessionName as display-only label - Ref binding: inboxProcessInputRef.current = processInput after useInputHandlers - Duplicate log: quick reply delegates to processInput (no manual log push) - AI mode: force inputMode='ai' before sending inbox replies - Modal data: openModal preserves data, updateModalData handles undefined base Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reply input auto-focus in FocusModeView used a setTimeout(60ms) fallback alongside requestAnimationFrame. Replaced with double requestAnimationFrame which reliably fires after browser paint without arbitrary timing delays. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ection Adds role="switch" and aria-checked for accessibility compliance. Follows existing Director's Notes toggle pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gating Add onOpenAgentInbox prop through App.tsx → AppModals → QuickActionsModal chain, gated behind encoreFeatures.unifiedInbox. The action calls the same setAgentInboxOpen(true) used by the Alt+I keyboard shortcut, ensuring all entry points share the same open/close state from modalStore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stamp) 26 tests covering: - Filter logic: all/unread/read/starred modes - Sort logic: newest/oldest/grouped/byAgent modes - Summary truncation at MAX_MESSAGE_LENGTH (90 chars) - Timestamp fallback chain: log → createdAt → Date.now() - Exported utilities: truncate() and generateSmartSummary() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ocus mode) 18 tests across 6 describe blocks covering: - List mode rendering with ARIA attributes - Filter segmented control (All/Unread/Read/Starred) - Keyboard navigation (ArrowUp/Down, Enter to navigate) - Escape/close behavior (overlay click, close button) - Focus mode entry (F key, double-click) and exit (Escape, Backspace) - Reply textarea presence in focus mode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
updateModalData now stores data even on unopened modals (needed for AgentInbox pattern). Updated test expectation accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
@coderabbitai review |
📝 WalkthroughWalkthroughAdds a new "Unified Inbox" feature providing a two-view modal interface (list and focus modes) for browsing and interacting with agent sessions, including filtering, sorting, keyboard navigation, and persistent state management integration. Changes
Sequence DiagramsequenceDiagram
actor User
participant KeyboardHandler as Keyboard Handler
participant App as App Component
participant ModalStore as Modal Store
participant AgentInbox as AgentInbox Modal
participant ListView as InboxListView
participant FocusView as FocusModeView
participant SessionMgr as Session Manager
User->>KeyboardHandler: Alt+I (agentInbox shortcut)
activate KeyboardHandler
KeyboardHandler->>ModalStore: setAgentInboxOpen(true)
deactivate KeyboardHandler
activate ModalStore
ModalStore->>ModalStore: openModal('agentInbox')
ModalStore-->>App: agentInboxOpen=true
deactivate ModalStore
App->>AgentInbox: render with sessions/groups
activate AgentInbox
AgentInbox->>ListView: render list mode (default)
activate ListView
ListView->>ListView: useAgentInbox hook
ListView->>ListView: filter/sort items
ListView-->>User: display filtered inbox items
User->>ListView: ArrowDown/Up navigation
ListView->>ListView: update selected row
ListView-->>User: highlight next/prev item
User->>ListView: Enter key
ListView->>AgentInbox: onNavigateToSession(sessionId, tabId)
deactivate ListView
AgentInbox->>SessionMgr: navigate to session/tab
SessionMgr-->>User: activate target session
User->>ListView: F key or double-click
activate ListView
ListView->>AgentInbox: switch to focus mode
deactivate ListView
AgentInbox->>FocusView: render focus mode
activate FocusView
FocusView->>FocusView: render sidebar + conversation
FocusView-->>User: show logs and reply composer
User->>FocusView: type reply + Enter
FocusView->>AgentInbox: onQuickReply(sessionId, message)
AgentInbox->>SessionMgr: process reply in target session
SessionMgr-->>User: update session with reply
User->>FocusView: Escape key
FocusView->>AgentInbox: switch back to list mode
deactivate FocusView
AgentInbox->>ListView: render list mode
activate ListView
ListView-->>User: return to inbox list
deactivate ListView
User->>AgentInbox: click close / Escape
deactivate AgentInbox
AgentInbox->>ModalStore: setAgentInboxOpen(false)
activate ModalStore
ModalStore-->>App: agentInboxOpen=false
deactivate ModalStore
App->>User: close modal, restore focus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/renderer/types/agent-inbox.ts (1)
1-11: NarrowtoolTypeto the session model instead ofstring.
toolTypeis sourced fromSession, so typing it asstringweakens safety and can hide invalid values.♻️ Proposed type-safety refactor
-import type { SessionState } from './index'; +import type { SessionState, Session } from './index'; @@ - toolType: string; + toolType: Session['toolType'];As per coding guidelines:
src/**/*.{ts,tsx}: Use the Session interface in code to represent agents (due to historical naming), but use 'agent' in user-facing language and 'session' for provider-level conversation contexts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/types/agent-inbox.ts` around lines 1 - 11, The InboxItem.toolType is currently typed as string; narrow it to the session model type by importing the Session (or Session interface exported from ./index) and changing toolType's type to Session['toolType'] (or the exact property name on Session that represents the agent/model), so InboxItem uses the Session-derived union instead of a plain string; update the import to pull the Session type if needed and adjust the InboxItem interface accordingly.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
434-457: Add focus handlers to keyboard-focusable group headers.These headers are intentionally keyboard-focusable (
tabIndex={0}) but don’t implement focus/blur handlers, unlike item rows.As per coding guidelines:
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 434 - 457, The keyboard-focusable header div (the element using tabIndex={0} and toggling setCollapsedGroups with row.groupKey) needs onFocus and onBlur handlers like item rows have; add onFocus and onBlur to this header element that update the same focus state used by item rows (reuse the existing item-row focus handlers if present, e.g., onItemFocus/onItemBlur or the state setter that tracks focused rows/groups, otherwise create matching handlers such as setFocusedGroup(row.groupKey) and clear it on blur) so headers get the same focus/blur behavior as items.src/renderer/App.tsx (1)
4261-4271: Consider passingenterToSendAIprop to AgentInbox for consistency with quick reply keyboard behavior.The
enterToSendAIprop is optional inAgentInboxProps, but it's available in App.tsx scope (line 401) and should be passed to maintain consistency with the user's Enter-to-send setting. The AgentInbox component passes this to FocusModeView, which uses it to determine keyboard behavior for quick replies. Omitting it means the inbox quick reply won't respect the global Enter-to-send preference.Suggested enhancement
<AgentInbox sessions={sessions} groups={groups} theme={theme} + enterToSendAI={enterToSendAI} onClose={() => setAgentInboxOpen(false)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/App.tsx` around lines 4261 - 4271, App.tsx is rendering <AgentInbox> but not passing the enterToSendAI prop; update the <AgentInbox> props to include enterToSendAI={enterToSendAI} so AgentInbox receives the user's Enter-to-send preference (enterToSendAI is defined in App.tsx scope) and will forward it through AgentInboxProps to FocusModeView for correct quick-reply keyboard behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/App.tsx`:
- Around line 1408-1424: The quick-reply flow currently uses queueMicrotask to
flip back to previous session which can cause a visible flicker; change the
restore step to use React.startTransition to defer the UI update and avoid the
snap. In the useEffect handling pendingInboxQuickReply (references:
pendingInboxQuickReply, inboxProcessInputRef.current, setPendingInboxQuickReply,
setActiveSessionId, activeSession?.id), import startTransition from React and
replace queueMicrotask(() => setActiveSessionId(previousActiveSessionId)) with
startTransition(() => setActiveSessionId(previousActiveSessionId))). This
ensures the session restore is rendered non-urgently and reduces flicker.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 228-233: The icon-only button in FocusModeView (the button using
onToggleRaw and showRawMarkdown) currently relies on title for screen readers;
add an explicit aria-label that matches the title logic (e.g.,
aria-label={showRawMarkdown ? 'Show formatted' : 'Show plain text'}) so
assistive tech announces it reliably. Apply the same fix to the other icon-only
buttons referenced (around lines 874-883 and 1107-1137) — locate buttons using
only title/tooltip and add corresponding aria-label attributes that reflect the
same descriptive text as their title.
- Around line 427-438: The header elements in FocusModeView are using
role="option" incorrectly (no listbox and headers aren’t selectable); update the
ARIA semantics by replacing role="option" on the header divs with an appropriate
interactive/structural role such as role="button" (or role="heading" with a
nested button) and expose their collapsed state via aria-expanded and
aria-controls on the clickable element; ensure only actual selectable rows
retain role="option" inside a proper listbox container (or remove option roles
entirely if not using a listbox). Apply the same change to the other header
instance referenced around the rows mapping (the second header block at the
similar code region noted in the comment).
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1141-1178: The group header button currently removes the native
focus outline (className "outline-none") which leaves keyboard users without a
visible focus state; update the button (the element that calls
toggleGroup(row.groupKey) and uses isRowSelected) to include tabIndex={0} and
add onFocus/onBlur handlers that set a focused state (or apply a CSS class) to
render a visible focus ring (e.g., using theme.colors.accent or a focus-visible
style) when focused, and restore the original look on blur; ensure the onKeyDown
behavior remains and consider replacing unconditional "outline-none" with a
conditional class that hides the outline only when not focused so keyboard users
see the focus indicator.
- Around line 808-811: getHeaderFocusables currently includes disabled controls
which can trap Tab; update it to only return genuinely focusable, enabled
elements by filtering the querySelectorAll results for elements that are not
disabled and not aria-disabled and have a non-negative tabIndex. Specifically,
in getHeaderFocusables (and apply the same change to the other
focusable-collector functions in this component), use
Array.from(headerRef.current.querySelectorAll<HTMLElement>('button,
[tabindex="0"]')).filter(el => !el.hasAttribute('disabled') &&
el.getAttribute('aria-disabled') !== 'true' && el.tabIndex >= 0) so
disabled/aria-disabled elements are excluded from the custom Tab loop.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3666-3671: The switch control in SettingsModal (the span element
rendered alongside the "Unified Inbox" label that uses role="switch" and
aria-checked) is missing an accessible name; update the span(s) to include an
accessible name by either adding aria-label="Unified Inbox" (or the appropriate
localized label) or by adding aria-labelledby that references the id of the
visible label element (e.g., give the label element an id and set
aria-labelledby on the span). Apply the same change to the other switch instance
mentioned (the span with role="switch" around the other Unified Inbox toggle) so
both controls are properly labeled for assistive tech.
In `@src/renderer/stores/modalStore.ts`:
- Around line 775-778: The payload type for updateAgentInboxData is too broad
(Record<string, unknown>) and bypasses updateModalData's contract; change the
parameter type to Partial<AgentInboxModalData> so updateAgentInboxData(open:
boolean) remains same for setAgentInboxOpen and updateAgentInboxData accepts
Partial<AgentInboxModalData> to enforce fields filterMode, sortMode, isExpanded
when calling updateModalData('agentInbox', ...); update the signature in
modalStore.ts and any callers to pass the narrowed type.
---
Nitpick comments:
In `@src/renderer/App.tsx`:
- Around line 4261-4271: App.tsx is rendering <AgentInbox> but not passing the
enterToSendAI prop; update the <AgentInbox> props to include
enterToSendAI={enterToSendAI} so AgentInbox receives the user's Enter-to-send
preference (enterToSendAI is defined in App.tsx scope) and will forward it
through AgentInboxProps to FocusModeView for correct quick-reply keyboard
behavior.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 434-457: The keyboard-focusable header div (the element using
tabIndex={0} and toggling setCollapsedGroups with row.groupKey) needs onFocus
and onBlur handlers like item rows have; add onFocus and onBlur to this header
element that update the same focus state used by item rows (reuse the existing
item-row focus handlers if present, e.g., onItemFocus/onItemBlur or the state
setter that tracks focused rows/groups, otherwise create matching handlers such
as setFocusedGroup(row.groupKey) and clear it on blur) so headers get the same
focus/blur behavior as items.
In `@src/renderer/types/agent-inbox.ts`:
- Around line 1-11: The InboxItem.toolType is currently typed as string; narrow
it to the session model type by importing the Session (or Session interface
exported from ./index) and changing toolType's type to Session['toolType'] (or
the exact property name on Session that represents the agent/model), so
InboxItem uses the Session-derived union instead of a plain string; update the
import to pull the Session type if needed and adjust the InboxItem interface
accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/tabDisplayName.ts
Comment on lines
+1408
to
+1424
| useEffect(() => { | ||
| if (!pendingInboxQuickReply) return; | ||
| if (activeSession?.id !== pendingInboxQuickReply.targetSessionId) return; | ||
|
|
||
| inboxProcessInputRef.current(pendingInboxQuickReply.text); | ||
| const previousActiveSessionId = pendingInboxQuickReply.previousActiveSessionId; | ||
| setPendingInboxQuickReply(null); | ||
|
|
||
| if ( | ||
| previousActiveSessionId && | ||
| previousActiveSessionId !== pendingInboxQuickReply.targetSessionId | ||
| ) { | ||
| queueMicrotask(() => { | ||
| setActiveSessionId(previousActiveSessionId); | ||
| }); | ||
| } | ||
| }, [pendingInboxQuickReply, activeSession?.id, setActiveSessionId]); |
There was a problem hiding this comment.
Minor: queueMicrotask may cause brief UI flicker when restoring previous session.
The quick-reply flow switches to the target session, sends the message, then uses queueMicrotask to switch back to the previous session. This creates a brief window where the UI displays the target session before snapping back. Users may perceive a flicker, especially on slower machines.
Consider using a flag or batching mechanism to suppress UI updates during this transient state, or accept the flicker as acceptable UX for this feature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/App.tsx` around lines 1408 - 1424, The quick-reply flow
currently uses queueMicrotask to flip back to previous session which can cause a
visible flicker; change the restore step to use React.startTransition to defer
the UI update and avoid the snap. In the useEffect handling
pendingInboxQuickReply (references: pendingInboxQuickReply,
inboxProcessInputRef.current, setPendingInboxQuickReply, setActiveSessionId,
activeSession?.id), import startTransition from React and replace
queueMicrotask(() => setActiveSessionId(previousActiveSessionId)) with
startTransition(() => setActiveSessionId(previousActiveSessionId))). This
ensures the session restore is rendered non-urgently and reduces flicker.
Comment on lines
+228
to
+233
| <button | ||
| onClick={onToggleRaw} | ||
| className="p-1 rounded opacity-0 group-hover:opacity-50 hover:!opacity-100 transition-opacity" | ||
| style={{ color: showRawMarkdown ? theme.colors.accent : theme.colors.textDim }} | ||
| title={showRawMarkdown ? 'Show formatted' : 'Show plain text'} | ||
| > |
There was a problem hiding this comment.
Add explicit accessible names to icon-only buttons.
Several icon-only controls rely on title only; they should have aria-label for reliable screen-reader announcements.
♿ Suggested accessibility labels
<button
onClick={onToggleRaw}
+ aria-label={showRawMarkdown ? 'Show formatted text' : 'Show plain text'}
className="p-1 rounded opacity-0 group-hover:opacity-50 hover:!opacity-100 transition-opacity"
@@
<button
onClick={onClose}
+ aria-label="Close focus mode"
className="p-1.5 rounded"
@@
<button
onClick={handleQuickReply}
+ aria-label="Send quick reply"
disabled={!replyText.trim()}
@@
<button
onClick={handleOpenAndReply}
+ aria-label="Open session and reply"
disabled={!replyText.trim()}Also applies to: 874-883, 1107-1137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 228 - 233,
The icon-only button in FocusModeView (the button using onToggleRaw and
showRawMarkdown) currently relies on title for screen readers; add an explicit
aria-label that matches the title logic (e.g., aria-label={showRawMarkdown ?
'Show formatted' : 'Show plain text'}) so assistive tech announces it reliably.
Apply the same fix to the other icon-only buttons referenced (around lines
874-883 and 1107-1137) — locate buttons using only title/tooltip and add
corresponding aria-label attributes that reflect the same descriptive text as
their title.
Comment on lines
+427
to
+438
| <div className="flex-1 overflow-y-auto py-1"> | ||
| {(() => { | ||
| let activeGroup: string | null = null; | ||
| return rows.map((row, rowIdx) => { | ||
| if (row.type === 'header') { | ||
| activeGroup = row.groupKey; | ||
| return ( | ||
| <div | ||
| key={`header-${row.groupKey}-${rowIdx}`} | ||
| tabIndex={0} | ||
| role="option" | ||
| aria-selected={false} |
There was a problem hiding this comment.
Fix sidebar ARIA semantics (option roles are currently invalid).
role="option" is applied without a listbox container, and collapsible headers are exposed as options even though they’re not selectable items. This creates invalid SR semantics.
♿ Suggested ARIA structure fix
- <div className="flex-1 overflow-y-auto py-1">
+ <div className="flex-1 overflow-y-auto py-1" role="listbox" aria-label="Inbox items">
@@
- <div
+ <div
key={`header-${row.groupKey}-${rowIdx}`}
tabIndex={0}
- role="option"
- aria-selected={false}
+ role="button"
+ aria-expanded={!collapsedGroups.has(row.groupKey)}
@@
- <div
+ <div
key={`${itm.sessionId}-${itm.tabId}`}
ref={isCurrent ? currentRowRef : undefined}
tabIndex={0}
role="option"
aria-selected={isCurrent}Also applies to: 495-497
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 427 - 438,
The header elements in FocusModeView are using role="option" incorrectly (no
listbox and headers aren’t selectable); update the ARIA semantics by replacing
role="option" on the header divs with an appropriate interactive/structural role
such as role="button" (or role="heading" with a nested button) and expose their
collapsed state via aria-expanded and aria-controls on the clickable element;
ensure only actual selectable rows retain role="option" inside a proper listbox
container (or remove option roles entirely if not using a listbox). Apply the
same change to the other header instance referenced around the rows mapping (the
second header block at the similar code region noted in the comment).
Comment on lines
+808
to
+811
| const getHeaderFocusables = useCallback((): HTMLElement[] => { | ||
| if (!headerRef.current) return []; | ||
| return Array.from(headerRef.current.querySelectorAll<HTMLElement>('button, [tabindex="0"]')); | ||
| }, []); |
There was a problem hiding this comment.
Filter disabled controls out of the custom Tab loop.
The current focus list can include disabled buttons; when that happens, Tab is prevented but focus may not move, causing keyboard trap behavior.
Suggested fix
const getHeaderFocusables = useCallback((): HTMLElement[] => {
if (!headerRef.current) return [];
- return Array.from(headerRef.current.querySelectorAll<HTMLElement>('button, [tabindex="0"]'));
+ return Array.from(
+ headerRef.current.querySelectorAll<HTMLElement>('button, [tabindex="0"]')
+ ).filter(
+ (el) =>
+ !el.hasAttribute('disabled') &&
+ el.getAttribute('aria-disabled') !== 'true' &&
+ el.tabIndex >= 0
+ );
}, []);Also applies to: 817-841
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 808 - 811,
getHeaderFocusables currently includes disabled controls which can trap Tab;
update it to only return genuinely focusable, enabled elements by filtering the
querySelectorAll results for elements that are not disabled and not
aria-disabled and have a non-negative tabIndex. Specifically, in
getHeaderFocusables (and apply the same change to the other focusable-collector
functions in this component), use
Array.from(headerRef.current.querySelectorAll<HTMLElement>('button,
[tabindex="0"]')).filter(el => !el.hasAttribute('disabled') &&
el.getAttribute('aria-disabled') !== 'true' && el.tabIndex >= 0) so
disabled/aria-disabled elements are excluded from the custom Tab loop.
Comment on lines
+1141
to
+1178
| <button | ||
| type="button" | ||
| key={`header-${row.groupKey}`} | ||
| className="outline-none" | ||
| style={{ | ||
| position: 'absolute', | ||
| top: 0, | ||
| left: 0, | ||
| width: '100%', | ||
| height: `${virtualRow.size}px`, | ||
| transform: `translateY(${virtualRow.start}px)`, | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| paddingLeft: 16, | ||
| paddingRight: 16, | ||
| fontSize: 13, | ||
| fontWeight: 600, | ||
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | ||
| letterSpacing: '0.5px', | ||
| textTransform: 'uppercase', | ||
| borderBottom: `2px solid ${theme.colors.border}40`, | ||
| borderLeft: isRowSelected | ||
| ? `3px solid ${theme.colors.accent}` | ||
| : '3px solid transparent', | ||
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | ||
| cursor: 'pointer', | ||
| borderTop: 'none', | ||
| borderRight: 'none', | ||
| textAlign: 'left', | ||
| }} | ||
| onClick={() => toggleGroup(row.groupKey)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| toggleGroup(row.groupKey); | ||
| } | ||
| }} | ||
| > |
There was a problem hiding this comment.
Group header buttons remove native focus outline without replacement.
These buttons are keyboard-focusable but outline-none suppresses visible focus state, so keyboard users can lose position.
Suggested fix
<button
type="button"
key={`header-${row.groupKey}`}
className="outline-none"
+ onFocus={(e) => {
+ e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`;
+ e.currentTarget.style.outlineOffset = '-2px';
+ }}
+ onBlur={(e) => {
+ e.currentTarget.style.outline = 'none';
+ }}
style={{
position: 'absolute',As per coding guidelines, src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| type="button" | |
| key={`header-${row.groupKey}`} | |
| className="outline-none" | |
| style={{ | |
| position: 'absolute', | |
| top: 0, | |
| left: 0, | |
| width: '100%', | |
| height: `${virtualRow.size}px`, | |
| transform: `translateY(${virtualRow.start}px)`, | |
| display: 'flex', | |
| alignItems: 'center', | |
| paddingLeft: 16, | |
| paddingRight: 16, | |
| fontSize: 13, | |
| fontWeight: 600, | |
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | |
| letterSpacing: '0.5px', | |
| textTransform: 'uppercase', | |
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| borderTop: 'none', | |
| borderRight: 'none', | |
| textAlign: 'left', | |
| }} | |
| onClick={() => toggleGroup(row.groupKey)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| toggleGroup(row.groupKey); | |
| } | |
| }} | |
| > | |
| <button | |
| type="button" | |
| key={`header-${row.groupKey}`} | |
| className="outline-none" | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | |
| e.currentTarget.style.outlineOffset = '-2px'; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| style={{ | |
| position: 'absolute', | |
| top: 0, | |
| left: 0, | |
| width: '100%', | |
| height: `${virtualRow.size}px`, | |
| transform: `translateY(${virtualRow.start}px)`, | |
| display: 'flex', | |
| alignItems: 'center', | |
| paddingLeft: 16, | |
| paddingRight: 16, | |
| fontSize: 13, | |
| fontWeight: 600, | |
| color: isRowSelected ? theme.colors.accent : theme.colors.textDim, | |
| letterSpacing: '0.5px', | |
| textTransform: 'uppercase', | |
| borderBottom: `2px solid ${theme.colors.border}40`, | |
| borderLeft: isRowSelected | |
| ? `3px solid ${theme.colors.accent}` | |
| : '3px solid transparent', | |
| backgroundColor: isRowSelected ? `${theme.colors.accent}10` : 'transparent', | |
| cursor: 'pointer', | |
| borderTop: 'none', | |
| borderRight: 'none', | |
| textAlign: 'left', | |
| }} | |
| onClick={() => toggleGroup(row.groupKey)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| toggleGroup(row.groupKey); | |
| } | |
| }} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1141 -
1178, The group header button currently removes the native focus outline
(className "outline-none") which leaves keyboard users without a visible focus
state; update the button (the element that calls toggleGroup(row.groupKey) and
uses isRowSelected) to include tabIndex={0} and add onFocus/onBlur handlers that
set a focused state (or apply a CSS class) to render a visible focus ring (e.g.,
using theme.colors.accent or a focus-visible style) when focused, and restore
the original look on blur; ensure the onKeyDown behavior remains and consider
replacing unconditional "outline-none" with a conditional class that hides the
outline only when not focused so keyboard users see the focus indicator.
Comment on lines
+3666
to
+3671
| <div | ||
| className="text-sm font-bold flex items-center gap-2" | ||
| style={{ color: theme.colors.textMain }} | ||
| > | ||
| Unified Inbox | ||
| <span |
There was a problem hiding this comment.
The Unified Inbox switch is missing an accessible name.
role="switch" + aria-checked is present, but there is no aria-label/aria-labelledby, so assistive tech gets an unlabeled control.
Suggested fix
- <div
+ <div
+ id="unified-inbox-feature-label"
className="text-sm font-bold flex items-center gap-2"
style={{ color: theme.colors.textMain }}
>
Unified Inbox
@@
<button
role="switch"
+ aria-labelledby="unified-inbox-feature-label"
aria-checked={encoreFeatures.unifiedInbox}
onClick={(e) => {Also applies to: 3686-3708
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SettingsModal.tsx` around lines 3666 - 3671, The
switch control in SettingsModal (the span element rendered alongside the
"Unified Inbox" label that uses role="switch" and aria-checked) is missing an
accessible name; update the span(s) to include an accessible name by either
adding aria-label="Unified Inbox" (or the appropriate localized label) or by
adding aria-labelledby that references the id of the visible label element
(e.g., give the label element an id and set aria-labelledby on the span). Apply
the same change to the other switch instance mentioned (the span with
role="switch" around the other Unified Inbox toggle) so both controls are
properly labeled for assistive tech.
Comment on lines
+775
to
+778
| setAgentInboxOpen: (open: boolean) => | ||
| open ? openModal('agentInbox') : closeModal('agentInbox'), | ||
| updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/renderer/stores/modalStore.ts | sed -n '770,785p'Repository: RunMaestro/Maestro
Length of output: 803
🏁 Script executed:
rg -nP "AgentInboxModalData" src/renderer/stores/modalStore.tsRepository: RunMaestro/Maestro
Length of output: 142
🏁 Script executed:
rg -nP "type AgentInboxModalData|interface AgentInboxModalData" src/renderer/stores/Repository: RunMaestro/Maestro
Length of output: 138
🏁 Script executed:
rg -nP "function updateModalData|const updateModalData" src/renderer/stores/modalStore.ts -A 3Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
rg -nP "updateAgentInboxData\s*\(" src/ --type ts --type tsx -C 2Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
cat -n src/renderer/stores/modalStore.ts | sed -n '144,160p'Repository: RunMaestro/Maestro
Length of output: 641
🏁 Script executed:
rg -nP "updateModalData" src/renderer/stores/modalStore.ts -B 2 -A 5 | head -50Repository: RunMaestro/Maestro
Length of output: 1976
🏁 Script executed:
rg -nP "updateAgentInboxData" src/ -C 2Repository: RunMaestro/Maestro
Length of output: 1565
Tighten updateAgentInboxData payload type to match modal contract.
Line 777 uses Record<string, unknown>, which bypasses the type safety of updateModalData. The function should accept Partial<AgentInboxModalData> to enforce the correct shape for agentInbox modal data (filterMode, sortMode, isExpanded).
Proposed fix
// Agent Inbox Modal
setAgentInboxOpen: (open: boolean) =>
open ? openModal('agentInbox') : closeModal('agentInbox'),
- updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data),
+ updateAgentInboxData: (data: Partial<AgentInboxModalData>) =>
+ updateModalData('agentInbox', data),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setAgentInboxOpen: (open: boolean) => | |
| open ? openModal('agentInbox') : closeModal('agentInbox'), | |
| updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), | |
| setAgentInboxOpen: (open: boolean) => | |
| open ? openModal('agentInbox') : closeModal('agentInbox'), | |
| updateAgentInboxData: (data: Partial<AgentInboxModalData>) => | |
| updateModalData('agentInbox', data), | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/stores/modalStore.ts` around lines 775 - 778, The payload type
for updateAgentInboxData is too broad (Record<string, unknown>) and bypasses
updateModalData's contract; change the parameter type to
Partial<AgentInboxModalData> so updateAgentInboxData(open: boolean) remains same
for setAgentInboxOpen and updateAgentInboxData accepts
Partial<AgentInboxModalData> to enforce fields filterMode, sortMode, isExpanded
when calling updateModalData('agentInbox', ...); update the signature in
modalStore.ts and any callers to pass the narrowed type.
Greptile SummaryAdds a fully-gated Unified Inbox feature (disabled by default) that aggregates latest messages from all active agents into a keyboard-navigable overlay with focus mode for deep reading and inline replies. Key architectural decisions:
Integration points:
Test coverage:
Bug fixes applied from review branches:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([User presses Alt+I]) --> CheckGate{Feature enabled?}
CheckGate -->|No| End1([Nothing happens])
CheckGate -->|Yes| OpenModal[Open AgentInbox Modal]
OpenModal --> LoadData[useAgentInbox hook aggregates data]
LoadData --> FilterSort[Filter + Sort items]
FilterSort --> ListView[InboxListView renders]
ListView --> UserAction{User Action}
UserAction -->|Arrow Keys| Navigate[Navigate items]
Navigate --> ListView
UserAction -->|F or Enter| EnterFocus[Enter Focus Mode]
EnterFocus --> FreezeOrder[Freeze item order in ref]
FreezeOrder --> FocusView[FocusModeView renders]
FocusView --> FocusAction{User Action in Focus}
FocusAction -->|Cmd+Bracket Keys| PrevNext[Navigate prev/next]
PrevNext --> FocusView
FocusAction -->|Type reply| QuickReply[Quick Reply handler]
QuickReply --> SwitchSession[Switch to target session]
SwitchSession --> ProcessInput[processInput sends text]
ProcessInput --> RestoreSession[Restore previous session]
FocusAction -->|Escape or B| ExitFocus[Exit to List View]
ExitFocus --> ClearFreeze[Clear frozen order]
ClearFreeze --> ListView
UserAction -->|Escape| CloseModal[Close Modal]
FocusAction -->|Click Navigate| CloseAndNav[Navigate to session]
CloseModal --> RestoreFocus[Restore focus to trigger element]
CloseAndNav --> RestoreFocus
RestoreFocus --> End2([Complete])
Last reviewed commit: ee926c2 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
src/renderer/components/SettingsModal.tsx (1)
3686-3689:⚠️ Potential issue | 🟠 MajorUnified Inbox switch still has no accessible name.
Line 3686 defines a switch control with
role="switch"andaria-checked, but it’s still unlabeled for assistive tech.Proposed fix
- <div + <div + id="unified-inbox-feature-label" className="text-sm font-bold flex items-center gap-2" style={{ color: theme.colors.textMain }} > Unified Inbox @@ <button role="switch" + aria-labelledby="unified-inbox-feature-label" aria-checked={encoreFeatures.unifiedInbox} onClick={(e) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SettingsModal.tsx` around lines 3686 - 3689, The switch button in SettingsModal.tsx (the <button role="switch"> that reads aria-checked={encoreFeatures.unifiedInbox} and has the onClick handler) lacks an accessible name; update that element to provide one (either via aria-label or aria-labelledby referencing a visible label) so screen readers can announce it (e.g., aria-label="Toggle unified inbox" or aria-labelledby pointing to the corresponding label element). Ensure the chosen label accurately reflects the control state/context and keep the existing role and aria-checked attributes intact.src/renderer/stores/modalStore.ts (1)
774-777:⚠️ Potential issue | 🟠 MajorNarrow
updateAgentInboxDatapayload to modal-specific shape.Line 777 still accepts
Record<string, unknown>, which bypasses theagentInboxmodal contract and permits invalid state payloads.🔧 Proposed fix
// Agent Inbox Modal setAgentInboxOpen: (open: boolean) => open ? openModal('agentInbox') : closeModal('agentInbox'), - updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), + updateAgentInboxData: (data: Partial<AgentInboxModalData>) => + updateModalData('agentInbox', data),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/stores/modalStore.ts` around lines 774 - 777, The updateAgentInboxData currently accepts a wide Record<string, unknown> which breaks the agentInbox modal contract; tighten the payload by defining and using a concrete modal-specific type/interface (e.g., AgentInboxData) and change the signature of updateAgentInboxData to (data: AgentInboxData) and pass that into updateModalData('agentInbox', data); update any callers to construct/validate the AgentInboxData shape and import the type where needed so invalid payloads are rejected at compile time.src/renderer/components/AgentInbox/InboxListView.tsx (2)
808-811:⚠️ Potential issue | 🟠 MajorExclude disabled elements from the custom Tab cycle.
Line 810 collects disabled/aria-disabled elements, which can break your custom Tab loop when focus does not move.
⌨️ Suggested fix
const getHeaderFocusables = useCallback((): HTMLElement[] => { if (!headerRef.current) return []; - return Array.from(headerRef.current.querySelectorAll<HTMLElement>('button, [tabindex="0"]')); + return Array.from( + headerRef.current.querySelectorAll<HTMLElement>('button, [tabindex="0"]') + ).filter( + (el) => + !el.hasAttribute('disabled') && + el.getAttribute('aria-disabled') !== 'true' && + el.tabIndex >= 0 + ); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 808 - 811, The custom focusable collector getHeaderFocusables currently includes disabled or aria-disabled elements which can stall the tab loop; update getHeaderFocusables (used with headerRef) to only return truly focusable elements by excluding elements with the disabled attribute and those with aria-disabled="true" (and optionally filter out tabindex="-1"), e.g. adjust the querySelectorAll to omit disabled/aria-disabled and/or post-filter the NodeList so the returned array only contains elements without element.hasAttribute('disabled') and element.getAttribute('aria-disabled') !== 'true'.
1141-1178:⚠️ Potential issue | 🟠 MajorRestore visible focus styles for group header buttons.
Line 1144 removes native outline, but there is no replacement focus style. Keyboard users can lose track of focus on headers.
♿ Suggested fix
<button type="button" key={`header-${row.groupKey}`} className="outline-none" + tabIndex={0} @@ onClick={() => toggleGroup(row.groupKey)} + onFocus={(e) => { + e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; + e.currentTarget.style.outlineOffset = '-2px'; + }} + onBlur={(e) => { + e.currentTarget.style.outline = 'none'; + }} onKeyDown={(e) => {As per coding guidelines,
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1141 - 1178, The header button removed the native outline but never provided an accessible focus replacement; restore keyboard focusability by adding tabIndex={0} to the group header button and provide visible focus styling via focus handlers or a focused state: add onFocus={() => setFocusedGroup(row.groupKey)} and onBlur={() => setFocusedGroup(undefined)} (or use a focus-visible CSS class) and when focused apply a clear visual indicator (e.g., a 2px accent outline/boxShadow or change background/border similar to isRowSelected). Keep existing onKeyDown/toggleGroup behavior and reference the button rendering in InboxListView (the button using row.groupKey, virtualRow, isRowSelected and toggleGroup) when making the change.src/renderer/components/AgentInbox/FocusModeView.tsx (2)
228-233:⚠️ Potential issue | 🟠 MajorAdd explicit accessible names to icon-only buttons.
Line 228, Line 874, Line 1107, and Line 1123 use icon-only controls with
titleonly. Addaria-labelso screen readers announce these actions reliably.♿ Suggested fix
<button onClick={onToggleRaw} + aria-label={showRawMarkdown ? 'Show formatted text' : 'Show plain text'} className="p-1 rounded opacity-0 group-hover:opacity-50 hover:!opacity-100 transition-opacity" style={{ color: showRawMarkdown ? theme.colors.accent : theme.colors.textDim }} title={showRawMarkdown ? 'Show formatted' : 'Show plain text'} > @@ <button onClick={onClose} + aria-label="Close focus mode" className="p-1.5 rounded" style={{ color: theme.colors.textDim }} @@ <button onClick={handleQuickReply} + aria-label={enterToSendAI ? 'Quick reply (Enter)' : 'Quick reply (⌘Enter)'} disabled={!replyText.trim()} className="p-2 rounded-lg transition-colors flex-shrink-0" @@ <button onClick={handleOpenAndReply} + aria-label="Open session and reply (Shift+Enter)" disabled={!replyText.trim()} className="p-1.5 rounded-lg transition-colors flex-shrink-0 text-xs"Also applies to: 874-883, 1107-1137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 228 - 233, Add explicit aria-label attributes to all icon-only buttons in the FocusModeView component that currently only use title for accessibility; for example, the button using onToggleRaw (which reads showRawMarkdown and theme.colors) should get an aria-label (e.g., aria-label={showRawMarkdown ? 'Show formatted' : 'Show plain text'}). Do the same for the other icon-only controls in this component (the ones with only title attributes) so screen readers can reliably announce their actions. Ensure the aria-label matches the existing title text and toggles where appropriate.
427-438:⚠️ Potential issue | 🟠 MajorFix sidebar ARIA semantics and header keyboard-focus feedback.
Line 437 exposes collapsible headers as
role="option"without a listbox context. Headers are controls, not selectable options. Also, header focus targets need explicit focus styling for keyboard visibility.♿ Suggested fix
- <div className="flex-1 overflow-y-auto py-1"> + <div className="flex-1 overflow-y-auto py-1" role="listbox" aria-label="Inbox items"> @@ <div key={`header-${row.groupKey}-${rowIdx}`} tabIndex={0} - role="option" - aria-selected={false} + role="button" + aria-expanded={!collapsedGroups.has(row.groupKey)} onClick={() => { @@ className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" style={{ color: theme.colors.textDim, fontWeight: 600, backgroundColor: theme.colors.bgSidebar, }} + onFocus={(e) => { + e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; + e.currentTarget.style.outlineOffset = '-2px'; + }} + onBlur={(e) => { + e.currentTarget.style.outline = 'none'; + }} >As per coding guidelines,
src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus.Also applies to: 434-474, 495-497
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 427 - 438, The header div inside the rows.map (where activeGroup and row.groupKey are used) should be treated as a control, not an option: replace role="option" and aria-selected with role="button", add aria-expanded={activeGroup === row.groupKey} and aria-controls={`group-${row.groupKey}`}, keep tabIndex={0}, and wire a keyboard handler (onKeyDown handling Enter/Space) to call the same toggle/expand function used for clicks (or create a small toggleHeader / onHeaderToggle handler if none exists). Also add explicit focus-visible handling for keyboard users by applying a focus class (or onFocus/onBlur to set a focused state) so the header displays visible focus styling when focused. Ensure these changes are applied to the header rendering in FocusModeView (the header div created in rows.map).src/renderer/App.tsx (1)
1408-1424:⚠️ Potential issue | 🟡 MinorQuick-reply restore still has a transient session-switch flicker risk.
Line 1420 restores the previous session in a microtask right after sending, so the UI can briefly jump to the target session and snap back.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/App.tsx` around lines 1408 - 1424, The restore logic for quick-reply causes a UI flicker because setActiveSessionId(previousActiveSessionId) is deferred via queueMicrotask; change the effect in useEffect so that after capturing previousActiveSessionId and clearing pendingInboxQuickReply you restore synchronously (remove queueMicrotask) or use React startTransition to update the active session without a frame jump; specifically update the block that reads previousActiveSessionId, setPendingInboxQuickReply(null), and the conditional that calls setActiveSessionId(previousActiveSessionId) (all in the useEffect handling pendingInboxQuickReply, activeSession?.id, setActiveSessionId) so the session restore happens immediately (or inside startTransition) to avoid the transient switch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 676-677: The current single boolean showRawMarkdown in
FocusModeView causes raw-markdown to bleed across sessions; replace it with a
keyed state (e.g. a map keyed by `${sessionId}:${tabId}`) so raw mode is stored
per-session/tab. Update the state declaration (replace
showRawMarkdown/setShowRawMarkdown with something like rawMarkdownByKey and
setRawMarkdownByKey), compute a key from the existing sessionId and tabId
values, and change all reads/writes (the toggle handler and any consumers
referenced around the showRawMarkdown usage) to read/write the boolean at that
key; apply the same keyed-state change to the other instance of this logic
referenced (around the 1039–1045 usage) so both places use the per-session/tab
map.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1016-1033: The close button in the InboxListView component (the
button with onClick={onClose} and the <X /> icon) is icon-only and needs an
explicit accessible name; add an aria-label (e.g., aria-label="Close" or
aria-label="Close conversation") to the button element so screen readers
announce it consistently while keeping the existing title and event handlers
intact.
---
Duplicate comments:
In `@src/renderer/App.tsx`:
- Around line 1408-1424: The restore logic for quick-reply causes a UI flicker
because setActiveSessionId(previousActiveSessionId) is deferred via
queueMicrotask; change the effect in useEffect so that after capturing
previousActiveSessionId and clearing pendingInboxQuickReply you restore
synchronously (remove queueMicrotask) or use React startTransition to update the
active session without a frame jump; specifically update the block that reads
previousActiveSessionId, setPendingInboxQuickReply(null), and the conditional
that calls setActiveSessionId(previousActiveSessionId) (all in the useEffect
handling pendingInboxQuickReply, activeSession?.id, setActiveSessionId) so the
session restore happens immediately (or inside startTransition) to avoid the
transient switch.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 228-233: Add explicit aria-label attributes to all icon-only
buttons in the FocusModeView component that currently only use title for
accessibility; for example, the button using onToggleRaw (which reads
showRawMarkdown and theme.colors) should get an aria-label (e.g.,
aria-label={showRawMarkdown ? 'Show formatted' : 'Show plain text'}). Do the
same for the other icon-only controls in this component (the ones with only
title attributes) so screen readers can reliably announce their actions. Ensure
the aria-label matches the existing title text and toggles where appropriate.
- Around line 427-438: The header div inside the rows.map (where activeGroup and
row.groupKey are used) should be treated as a control, not an option: replace
role="option" and aria-selected with role="button", add
aria-expanded={activeGroup === row.groupKey} and
aria-controls={`group-${row.groupKey}`}, keep tabIndex={0}, and wire a keyboard
handler (onKeyDown handling Enter/Space) to call the same toggle/expand function
used for clicks (or create a small toggleHeader / onHeaderToggle handler if none
exists). Also add explicit focus-visible handling for keyboard users by applying
a focus class (or onFocus/onBlur to set a focused state) so the header displays
visible focus styling when focused. Ensure these changes are applied to the
header rendering in FocusModeView (the header div created in rows.map).
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 808-811: The custom focusable collector getHeaderFocusables
currently includes disabled or aria-disabled elements which can stall the tab
loop; update getHeaderFocusables (used with headerRef) to only return truly
focusable elements by excluding elements with the disabled attribute and those
with aria-disabled="true" (and optionally filter out tabindex="-1"), e.g. adjust
the querySelectorAll to omit disabled/aria-disabled and/or post-filter the
NodeList so the returned array only contains elements without
element.hasAttribute('disabled') and element.getAttribute('aria-disabled') !==
'true'.
- Around line 1141-1178: The header button removed the native outline but never
provided an accessible focus replacement; restore keyboard focusability by
adding tabIndex={0} to the group header button and provide visible focus styling
via focus handlers or a focused state: add onFocus={() =>
setFocusedGroup(row.groupKey)} and onBlur={() => setFocusedGroup(undefined)} (or
use a focus-visible CSS class) and when focused apply a clear visual indicator
(e.g., a 2px accent outline/boxShadow or change background/border similar to
isRowSelected). Keep existing onKeyDown/toggleGroup behavior and reference the
button rendering in InboxListView (the button using row.groupKey, virtualRow,
isRowSelected and toggleGroup) when making the change.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3686-3689: The switch button in SettingsModal.tsx (the <button
role="switch"> that reads aria-checked={encoreFeatures.unifiedInbox} and has the
onClick handler) lacks an accessible name; update that element to provide one
(either via aria-label or aria-labelledby referencing a visible label) so screen
readers can announce it (e.g., aria-label="Toggle unified inbox" or
aria-labelledby pointing to the corresponding label element). Ensure the chosen
label accurately reflects the control state/context and keep the existing role
and aria-checked attributes intact.
In `@src/renderer/stores/modalStore.ts`:
- Around line 774-777: The updateAgentInboxData currently accepts a wide
Record<string, unknown> which breaks the agentInbox modal contract; tighten the
payload by defining and using a concrete modal-specific type/interface (e.g.,
AgentInboxData) and change the signature of updateAgentInboxData to (data:
AgentInboxData) and pass that into updateModalData('agentInbox', data); update
any callers to construct/validate the AgentInboxData shape and import the type
where needed so invalid payloads are rejected at compile time.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/tabDisplayName.ts
Comment on lines
+676
to
+677
| // ---- Raw markdown toggle (per-session, not per-log) ---- | ||
| const [showRawMarkdown, setShowRawMarkdown] = useState(false); |
There was a problem hiding this comment.
Scope raw-markdown state by session/tab key.
Line 677 keeps a single boolean, so toggling raw mode in one thread carries into other threads when navigating. This should be keyed per sessionId:tabId.
🛠️ Suggested fix
- const [showRawMarkdown, setShowRawMarkdown] = useState(false);
+ const [showRawByItem, setShowRawByItem] = useState<Record<string, boolean>>({});
+ const currentItemKey = `${item.sessionId}:${item.tabId}`;
+ const showRawMarkdown = showRawByItem[currentItemKey] ?? false;
@@
{visibleLogs.map((log) => (
<FocusLogEntry
key={log.id}
log={log}
theme={theme}
showRawMarkdown={showRawMarkdown}
- onToggleRaw={() => setShowRawMarkdown((v) => !v)}
+ onToggleRaw={() =>
+ setShowRawByItem((prev) => ({
+ ...prev,
+ [currentItemKey]: !prev[currentItemKey],
+ }))
+ }
/>
))}Also applies to: 1039-1045
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx` around lines 676 - 677,
The current single boolean showRawMarkdown in FocusModeView causes raw-markdown
to bleed across sessions; replace it with a keyed state (e.g. a map keyed by
`${sessionId}:${tabId}`) so raw mode is stored per-session/tab. Update the state
declaration (replace showRawMarkdown/setShowRawMarkdown with something like
rawMarkdownByKey and setRawMarkdownByKey), compute a key from the existing
sessionId and tabId values, and change all reads/writes (the toggle handler and
any consumers referenced around the showRawMarkdown usage) to read/write the
boolean at that key; apply the same keyed-state change to the other instance of
this logic referenced (around the 1039–1045 usage) so both places use the
per-session/tab map.
Comment on lines
+1016
to
+1033
| <button | ||
| onClick={onClose} | ||
| className="p-1.5 rounded" | ||
| style={{ color: theme.colors.textDim }} | ||
| onMouseEnter={(e) => | ||
| (e.currentTarget.style.backgroundColor = `${theme.colors.accent}20`) | ||
| } | ||
| onMouseLeave={(e) => (e.currentTarget.style.backgroundColor = 'transparent')} | ||
| onFocus={(e) => { | ||
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | ||
| }} | ||
| onBlur={(e) => { | ||
| e.currentTarget.style.outline = 'none'; | ||
| }} | ||
| title="Close (Esc)" | ||
| > | ||
| <X className="w-4 h-4" /> | ||
| </button> |
There was a problem hiding this comment.
Add an explicit accessible name to the close icon button.
Line 1016 is icon-only and currently relies on title. Add aria-label for consistent screen-reader announcement.
♿ Suggested fix
<button
onClick={onClose}
+ aria-label="Close inbox"
className="p-1.5 rounded"
style={{ color: theme.colors.textDim }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| onClick={onClose} | |
| className="p-1.5 rounded" | |
| style={{ color: theme.colors.textDim }} | |
| onMouseEnter={(e) => | |
| (e.currentTarget.style.backgroundColor = `${theme.colors.accent}20`) | |
| } | |
| onMouseLeave={(e) => (e.currentTarget.style.backgroundColor = 'transparent')} | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| title="Close (Esc)" | |
| > | |
| <X className="w-4 h-4" /> | |
| </button> | |
| <button | |
| onClick={onClose} | |
| aria-label="Close inbox" | |
| className="p-1.5 rounded" | |
| style={{ color: theme.colors.textDim }} | |
| onMouseEnter={(e) => | |
| (e.currentTarget.style.backgroundColor = `${theme.colors.accent}20`) | |
| } | |
| onMouseLeave={(e) => (e.currentTarget.style.backgroundColor = 'transparent')} | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| title="Close (Esc)" | |
| > | |
| <X className="w-4 h-4" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/AgentInbox/InboxListView.tsx` around lines 1016 -
1033, The close button in the InboxListView component (the button with
onClick={onClose} and the <X /> icon) is icon-only and needs an explicit
accessible name; add an aria-label (e.g., aria-label="Close" or
aria-label="Close conversation") to the button element so screen readers
announce it consistently while keeping the existing title and event handlers
intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a fully gated Unified Inbox feature behind the
encoreFeatures.unifiedInboxflag (disabled by default). The inbox aggregates the latest messages from all active agents into a single, keyboard-navigable overlay with focus mode for deep-dive reading and inline replies.Key changes:
AgentInbox/index.tsx,InboxListView.tsx,FocusModeView.tsx,useAgentInbox.ts,agent-inbox.tstypes,tabDisplayName.tsutilityAlt+Iopens inbox (triple-gated: shortcut match + feature flag + setter exists)Cmd+Kquick actions with encore gatingagentInboxregistered in modalStore withAgentInboxModalDatatypeFixes applied from review branches:
sessionId(stable) notsessionName(mutable)useMemo/useCallbackon filters, sorts, and derived data; double-rAF focus instead of setTimeout hackTest coverage:
useAgentInbox.test.ts: 26 tests (filters, sorts, truncation, timestamp fallback)AgentInbox.test.tsx: 18 tests (render, filter pills, keyboard nav, escape, focus mode, reply)modalStore.test.ts: Updated 1 test to reflect newupdateModalDatabehavior (stores data on unopened modals)Test plan
unifiedInboxin Settings > Encore and verifyAlt+Iopens the inboxEscapecloses inbox and returns focus to previous elementCmd+Kshows "Open Unified Inbox" when feature is enablednpm run test— all non-baseline failures should pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests