feat: Unified Inbox as Encore Feature with Focus Mode#468
Open
felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
Open
feat: Unified Inbox as Encore Feature with Focus Mode#468felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
felipeggv wants to merge 24 commits intoRunMaestro:mainfrom
Conversation
Adds src/renderer/types/agent-inbox.ts with InboxItem, InboxFilterMode, InboxSortMode, InboxViewMode, STATUS_LABELS, and STATUS_COLORS. All imports resolve against current codebase (SessionState at types/index.ts:53). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Register Unified Inbox as an Encore Feature with type flag (boolean) and default value (false) in both EncoreFeatureFlags interface and DEFAULT_ENCORE_FEATURES constant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…re branch Port 4 files from feature/inbox-focus-polish into current architecture: - useAgentInbox.ts: data aggregation hook (filter, sort, smart summaries) - AgentInbox/index.tsx: modal shell with list/focus view switching - AgentInbox/InboxListView.tsx: virtualized list (react-window → @tanstack/react-virtual) - AgentInbox/FocusModeView.tsx: focus mode detail view with reply input Also adds: - AGENT_INBOX priority (555) in modalPriorities.ts - agentInbox ModalId, AgentInboxModalData, and modal actions in modalStore.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…re gating - Add agentInboxOpen reactive state to useModalActions() in modalStore.ts - Add lazy import for AgentInbox component in App.tsx - Destructure agentInboxOpen/setAgentInboxOpen from useModalActions() - Create handleAgentInboxNavigateToSession callback (close modal + switch session) - Add gated modal render: encoreFeatures.unifiedInbox && agentInboxOpen - Wire conditional setters for SessionList and QuickActions/AppModals - Add setAgentInboxOpen to keyboard handler ref for future keybinding support - 2 expected TS errors for props not yet updated (phases 06-08) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ncore gate Added agentInbox shortcut definition (Alt+I) to shortcuts.ts and wired the keyboard handler in useMainKeyboardHandler.ts, gated behind encoreFeatures.unifiedInbox. App.tsx context already had setAgentInboxOpen wired from Phase 05. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire setAgentInboxOpen through all prop interfaces (HamburgerMenuContentProps, SessionListProps, UseSessionListPropsDeps) and add conditionally-rendered Inbox button after Director's Notes in the hamburger menu. Button only appears when the Encore feature gate provides the setter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added onOpenUnifiedInbox prop to QuickActionsModal and threaded it through AppModals (both AppUtilityModalsProps and AppModalsProps interfaces). The action entry appears conditionally after Director's Notes, matching the existing Encore gate pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prefix onMarkAsRead with _ since it's destructured but not yet used in the FocusModeView component body. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace unconditional scroll-to-bottom on every log update with smart proximity detection: only auto-scroll when user is within 150px of bottom or when navigating to a new item. Prevents scroll hijacking while reading earlier messages during active agent output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lection sync - Bug 1: Changed group header <div> to <button type="button"> with onKeyDown handler for Enter/Space keyboard accessibility - Bug 2: Scoped collapse filtering to only apply when sortMode is 'grouped' or 'byAgent', preventing hidden items in other sort modes - Bug 3: Added guard useEffect to reset parent selectedIndex when it points to a collapsed-away item Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Overlay animation: duration-150 → duration-100 - InboxListView header: px-6 → px-4 py-3, text-base → text-lg, added Bot icon (w-5 h-5, accent) - FocusModeView header: added py-3, removed fixed height, text-sm font-bold → text-lg font-semibold - Footer padding aligned to px-4 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…'s Notes tokens Consistent px-4 horizontal padding across header, info bar, content scroll area, and reply input. Info bar uses py-2 padding instead of fixed height. Reply input uses py-3 matching the header. Scroll area uses uniform p-4 padding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Star indicators were oversized and redundant — the "Starred" filter already communicates starred state. Removed star icon from inbox list rows, focus sidebar items, focus info bar badge, and unicode star from filter pill label. Cleaned up unused Star imports from lucide-react. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…SessionStorage timeouts documented Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…10 integration checks green Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
processInput already adds the user log entry, so the optimistic push in handleAgentInboxQuickReply was creating duplicates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a gated "Unified Inbox": feature flag and default, new AgentInbox UI (list + focus), a useAgentInbox hook, modal state/actions and priority, App & UI wiring (SessionList, QuickActions, Settings), keyboard shortcut, type defs, and playbook docs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Session UI
participant App as App.tsx
participant ModalStore as Modal Store
participant AgentInbox as AgentInbox
participant InboxHook as useAgentInbox
participant Sessions as Session Data
User->>UI: Click "Unified Inbox" / press Alt+I
UI->>App: call setAgentInboxOpen(true)
App->>ModalStore: openModal('agentInbox')
ModalStore->>App: agentInboxOpen = true
App->>AgentInbox: lazy render modal (props)
AgentInbox->>InboxHook: useAgentInbox(sessions, groups, filter, sort)
InboxHook->>Sessions: read sessions & tabs
InboxHook-->>AgentInbox: return InboxItem[]
AgentInbox->>User: render list / focus UI
User->>AgentInbox: navigate/reply/toggle
AgentInbox->>App: onNavigateToSession/onQuickReply/onToggleThinking
App->>Sessions: update session/tab state
App-->>AgentInbox: updated state reflected via hook
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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 |
This was referenced Feb 26, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
src/renderer/stores/modalStore.ts (1)
142-146: Tighten Agent Inbox modal data types.
filterMode/sortModeasstringandupdateAgentInboxData(data: Record<string, unknown>)weaken type safety and allow invalid values into persisted modal state. UseInboxFilterMode,InboxSortMode, andPartial<AgentInboxModalData>.Also applies to: 667-667
🤖 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 142 - 146, The AgentInbox modal types are too loose: change AgentInboxModalData to use the specific enums/types InboxFilterMode and InboxSortMode for filterMode and sortMode (instead of string) and tighten updating to accept Partial<AgentInboxModalData> rather than Record<string, unknown>; update the signature and usages of updateAgentInboxData to accept Partial<AgentInboxModalData> so only valid keys/values can be persisted and adjust any call sites that pass generic records to construct a proper partial AgentInboxModalData object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.md`:
- Around line 18-27: The code comment and implementation disagree about whether
the collapsed-group filter should also run for sortMode === 'byAgent'; update
the useMemo guard to match the noted intent by changing the condition in the
rows computation (function using useMemo with symbols sortMode, allRows,
collapsedGroups) so it returns allRows when neither 'grouped' nor 'byAgent' is
active—i.e., replace the current if (sortMode !== 'grouped' ||
collapsedGroups.size === 0) return allRows; with a condition that returns early
when (sortMode !== 'grouped' && sortMode !== 'byAgent') || collapsedGroups.size
=== 0, or alternatively adjust the surrounding comment/text to explicitly state
the implemented behavior if you prefer the current logic.
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md`:
- Line 55: Update the contradictory test gate wording by replacing the phrase
"`npm run test` passes (all tests green)" with a statement that reflects the
reported failing tests (e.g., "`npm run test` completed with no new failures; 3
known pre-existing test failures remain") so the gate matches the note about 3
failing tests on Lines 11–12; edit the markdown line containing the exact string
"`npm run test` passes (all tests green)" to the new wording.
In `@src/renderer/App.tsx`:
- Around line 1529-1531: The ref inboxProcessInputRef is never assigned so calls
to inboxProcessInputRef.current are no-ops; after the component defines
processInput (inside or returned by useInputHandlers) assign
inboxProcessInputRef.current = processInput (or update it in a useEffect that
depends on processInput) so the ref always points to the latest implementation;
ensure the same assignment is added where similar refs are used (the other
occurrences noted) so Quick Reply calls invoke the real processInput handler.
- Around line 6158-6165: AgentInbox action handlers are not passed in; wire up
the implemented callbacks by adding props to the <AgentInbox /> instance: pass
handleAgentInboxQuickReply, handleAgentInboxOpenAndReply,
handleAgentInboxMarkAsRead, and handleAgentInboxToggleThinking (e.g.,
onQuickReply={handleAgentInboxQuickReply},
onOpenAndReply={handleAgentInboxOpenAndReply},
onMarkAsRead={handleAgentInboxMarkAsRead},
onToggleThinking={handleAgentInboxToggleThinking}), and update AgentInbox's prop
types/usage if necessary to accept these handlers.
- Around line 1521-1527: handleAgentInboxNavigateToSession currently only
accepts sessionId and thus drops any tabId target; update the function signature
to accept an optional tabId (e.g. (sessionId: string, tabId?: string)), call
setActiveSessionId(sessionId) and if tabId is provided call the tab setter (e.g.
setActiveTabId(tabId)), and add setActiveTabId to the useCallback dependency
array; also update any callers of handleAgentInboxNavigateToSession to pass the
tabId where available.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 425-452: The clickable header element that toggles collapsedGroups
(the div with onClick that references setCollapsedGroups, collapsedGroups and
row.groupName) is not keyboard-accessible—change it to a semantic interactive
control (preferably a <button>) or add proper ARIA/keyboard handling: give it
tabIndex=0, an accessible role if not using button, onKeyDown to handle
Enter/Space by invoking the same toggle logic, and visible focus styles; apply
the same changes to the other interactive row block that renders group rows (the
one using row.type / row.count) so both keyboard and screen-reader users can
toggle groups the same way as mouse users. Ensure existing onClick logic with
setCollapsedGroups is reused exactly and that focus and contrast styles use
theme colors for accessibility.
- Around line 566-591: The current handleResizeStart installs document listeners
(onMouseMove/onMouseUp) and mutates document.body styles but only removes them
in onMouseUp, causing leaks if the component unmounts mid-drag; update the
implementation so the listeners and body style changes are always cleaned up on
unmount: attach onMouseMove/onMouseUp as named functions (as you already do),
store them (or a flag) in a ref (isResizingRef) and add a useEffect with a
cleanup function that removes those listeners and resets
document.body.style.cursor and userSelect, and ensure handleResizeStart still
removes listeners on mouseup—target the handleResizeStart, isResizingRef,
onMouseMove, onMouseUp and setSidebarWidth symbols when making the change.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 39-42: Update user-facing copy in the InboxListView component to
use "agent" instead of "session": in the object literal that defines the
messages for keys all, unread, read, and starred (the entries with text 'No
active sessions to show.', 'No unread sessions.', 'No read sessions with
activity.', 'No starred sessions.') replace "session(s)" with "agent(s)"; also
update any ARIA labels or visible strings around lines 1042-1050 in the same
InboxListView component that mention "sessions" to use "agent" terminology.
Ensure you keep the same keys (all, unread, read, starred) and only change the
user-facing text/ARIA attributes.
- Around line 1058-1063: The listbox container in InboxListView is not
focusable; make the element with role="listbox" focusable by adding tabIndex={0}
and implement onFocus and onBlur handlers (e.g., handleListboxFocus,
handleListboxBlur) to set/clear a local focus state used for keyboard
interaction and styling, ensuring aria-activedescendant (selectedItemId) updates
while focused so keyboard users can reliably enter/exit and navigate the list.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 269-287: The dialog container element needs focus event handlers
in addition to tabIndex to meet keyboard-focus rules: add onFocus and onBlur
props to the same element that uses containerRef (the top-level dialog in
AgentInbox) and wire them to focus handler functions (e.g., implement or reuse
handleFocus and handleBlur alongside handleCaptureKeyDown and
handleShellKeyDown). Update the AgentInbox component to define these handlers
(or pass existing focus-management functions) so the container calls
onFocus={handleFocus} and onBlur={handleBlur} to manage component-level keyboard
focus state.
In `@src/renderer/components/SessionList.tsx`:
- Around line 750-752: In SessionList.tsx update the user-facing string
"Cross-session notifications" to use agent wording (e.g. "Cross-agent
notifications") so the Unified Inbox menu uses 'agent' for user-facing copy;
locate the text node inside the <div className="text-xs" ...> in the SessionList
component and replace the string while leaving the surrounding JSX and styling
unchanged, and ensure any related occurrences in this component follow the same
'agent' terminology.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3656-3712: The toggle button for unifiedInbox currently conveys
state only visually; update the interactive element (the button wrapping the
unifiedInbox control in SettingsModal.tsx) to expose switch semantics by adding
role="switch" and aria-checked={encoreFeatures.unifiedInbox}, and provide an
accessible name (either aria-label="Unified Inbox" or aria-labelledby pointing
to the existing "Unified Inbox" label) so screen readers announce the state;
keep the existing onClick handler (setEncoreFeatures) intact and ensure the
attributes reflect the current encoreFeatures.unifiedInbox value.
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts`:
- Around line 423-426: The shortcut handler currently prevents default and calls
trackShortcut('agentInbox') even when the callback ctx.setAgentInboxOpen is
undefined, causing false positives; update the branch in useMainKeyboardHandler
so it first checks that ctx.setAgentInboxOpen is present (and
ctx.encoreFeatures?.unifiedInbox) before calling e.preventDefault(),
ctx.setAgentInboxOpen?.(true) and trackShortcut('agentInbox'), otherwise do
nothing; reference the ctx.isShortcut check for 'agentInbox', the
ctx.encoreFeatures?.unifiedInbox guard, the ctx.setAgentInboxOpen callback, and
the trackShortcut call when making this change.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 5-7: The constant MAX_MESSAGE_LENGTH in useAgentInbox.ts currently
uses 250 which conflicts with the InboxItem.lastMessage contract that specifies
truncation to 90 chars; update MAX_MESSAGE_LENGTH to 90 so it matches the
documented contract for InboxItem.lastMessage and ensure any truncation logic
that references MAX_MESSAGE_LENGTH (e.g., in the useAgentInbox hook) uses the
updated value to keep UI and tests consistent.
- Around line 148-172: In the 'byAgent' branch the grouping uses
item.sessionName which can collide for different sessions; change the grouping
key in agentGroups (and subsequent groupMeta logic) to use a unique identifier
(e.g., item.sessionId or a composite `${item.sessionId}:${item.sessionName}`) so
groups are per-session not per-display-name, and ensure you still surface the
human-readable sessionName when building groupMeta and sorting (use sessionName
for localeCompare/display but sessionId for identity).
In `@src/renderer/stores/modalStore.ts`:
- Around line 665-668: The inbox persistence bug happens because
setAgentInboxOpen(true) calls openModal('agentInbox') which creates a modal
without data, then updateAgentInboxData calls updateModalData('agentInbox', ...)
but updateModalData bails when current.data is undefined (see current.data check
in updateModalData); to fix, ensure the agentInbox modal has an initialized data
object before updates by either (A) changing openModal('agentInbox')/openModal
implementation to initialize current.data = {} for the 'agentInbox' modal, or
(B) updating updateModalData (and the function that merges data) to
create/assign current.data = {} when missing and then merge the incoming data so
filter/sort/expanded persist on first open; reference functions:
setAgentInboxOpen, openModal, updateAgentInboxData, updateModalData, and the
current.data undefined guard.
---
Nitpick comments:
In `@src/renderer/stores/modalStore.ts`:
- Around line 142-146: The AgentInbox modal types are too loose: change
AgentInboxModalData to use the specific enums/types InboxFilterMode and
InboxSortMode for filterMode and sortMode (instead of string) and tighten
updating to accept Partial<AgentInboxModalData> rather than Record<string,
unknown>; update the signature and usages of updateAgentInboxData to accept
Partial<AgentInboxModalData> so only valid keys/values can be persisted and
adjust any call sites that pass generic records to construct a proper partial
AgentInboxModalData object.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-02.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-04.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-05.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-08.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-10.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-12.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.mdsrc/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/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useSessionListProps.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.ts
Comment on lines
18
to
27
| > Done — added `sortMode !== 'grouped' && sortMode !== 'byAgent'` guard (also covers byAgent which uses groups). | ||
| ```typescript | ||
| const rows = useMemo(() => { | ||
| if (sortMode !== 'grouped' || collapsedGroups.size === 0) return allRows; | ||
| return allRows.filter(row => { | ||
| if (row.type === 'header') return true; | ||
| const itemGroup = row.item.groupName ?? 'Ungrouped'; | ||
| return !collapsedGroups.has(itemGroup); | ||
| }); | ||
| }, [allRows, collapsedGroups, sortMode]); |
There was a problem hiding this comment.
Align the Bug 2 completion note with the shown condition.
Line 18 says collapse logic includes byAgent, but Lines 21-27 show grouped-only filtering. Keep one consistent condition to avoid follow-up implementation drift.
📝 Suggested doc-only fix
- > Done — added `sortMode !== 'grouped' && sortMode !== 'byAgent'` guard (also covers byAgent which uses groups).
+ > Done — collapse filtering is scoped to grouped mode as shown in the snippet below.📝 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
| > Done — added `sortMode !== 'grouped' && sortMode !== 'byAgent'` guard (also covers byAgent which uses groups). | |
| ```typescript | |
| const rows = useMemo(() => { | |
| if (sortMode !== 'grouped' || collapsedGroups.size === 0) return allRows; | |
| return allRows.filter(row => { | |
| if (row.type === 'header') return true; | |
| const itemGroup = row.item.groupName ?? 'Ungrouped'; | |
| return !collapsedGroups.has(itemGroup); | |
| }); | |
| }, [allRows, collapsedGroups, sortMode]); | |
| > Done — collapse filtering is scoped to grouped mode as shown in the snippet below. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.md`
around lines 18 - 27, The code comment and implementation disagree about whether
the collapsed-group filter should also run for sortMode === 'byAgent'; update
the useMemo guard to match the noted intent by changing the condition in the
rows computation (function using useMemo with symbols sortMode, allRows,
collapsedGroups) so it returns allRows when neither 'grouped' nor 'byAgent' is
active—i.e., replace the current if (sortMode !== 'grouped' ||
collapsedGroups.size === 0) return allRows; with a condition that returns early
when (sortMode !== 'grouped' && sortMode !== 'byAgent') || collapsedGroups.size
=== 0, or alternatively adjust the surrounding comment/text to explicitly state
the implemented behavior if you prefer the current logic.
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md
Outdated
Show resolved
Hide resolved
Greptile SummaryImplements Unified Inbox as an Encore Feature following the established pattern from Director's Notes. The feature aggregates unread notifications and agent status across all active sessions into a single modal with keyboard-first navigation and inline reply capabilities. Key changes:
Code quality:
TODO remaining: Cost badge placeholder at Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User triggers inbox<br/>Alt+I / Menu / Cmd+K] --> B{encoreFeatures.unifiedInbox?}
B -->|false| C[Feature disabled]
B -->|true| D[modalStore opens agentInbox]
D --> E[AgentInbox modal renders]
E --> F[useAgentInbox hook aggregates data]
F --> G[Filter sessions/tabs by mode]
G --> H[Sort by mode: newest/oldest/grouped/byAgent]
H --> I{View Mode?}
I -->|list| J[InboxListView<br/>Virtual scroll + keyboard nav]
I -->|focus| K[FocusModeView<br/>Conversation + reply composer]
J -->|F key / Enter| K
K -->|Esc / Back| J
K --> L{User action?}
L -->|Quick Reply| M[handleAgentInboxQuickReply<br/>Temp switch session + processInput]
L -->|Open & Reply| N[handleAgentInboxOpenAndReply<br/>Navigate + prefill input]
L -->|Mark Read| O[handleAgentInboxMarkAsRead<br/>Clear hasUnread flag]
L -->|Toggle Thinking| P[handleAgentInboxToggleThinking<br/>Cycle off→on→sticky]
M --> Q[Close inbox, restore focus]
N --> Q
O --> Q
P --> K
Last reviewed commit: d5ec10f |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
6 tasks
…gation for Unified Inbox Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1543-1573: The current handleAgentInboxQuickReply uses fixed
setTimeout delays which can restore the wrong session; replace the timing-based
logic with a guarded restore using a unique token or an async acknowledgement:
create a local ref/state like pendingQuickReplyRestoreId, set it to a new UUID
before calling setActiveSessionId(sessionId) and invoking
inboxProcessInputRef.current(text), then have inboxProcessInputRef.current
return a Promise or accept a callback/ack that you await; once the send
completes, check that pendingQuickReplyRestoreId still equals the local token
and only then restore previousActiveSessionId via
setActiveSessionId(previousActiveSessionId) and clear the token; remove the two
setTimeout calls and ensure you update setSessions as before (symbols:
handleAgentInboxQuickReply, inboxProcessInputRef, setActiveSessionId,
setSessions, previousActiveSessionId, pendingQuickReplyRestoreId).
- Around line 1578-1593: handleAgentInboxOpenAndReply currently sets
activeSessionId and activeTabId and pre-fills the AI tab input but doesn't clear
activeFileTabId or switch inputMode to 'ai', which can leave the UI in
terminal/file preview; update the handler (handleAgentInboxOpenAndReply) to also
call setActiveFileTabId(null) (or the equivalent clearing action) and
setInputMode('ai') when mapping the session state (alongside setActiveSessionId
and setSessions) so the session always opens in the AI input surface with the
prefilled text; ensure these updates occur before calling
setAgentInboxOpen(false).
…11y in Unified Inbox
- FocusModeView: Add resizeCleanupRef + useEffect unmount guard so
mousemove/mouseup listeners are removed if component unmounts mid-drag
- useAgentInbox: Group byAgent items by sessionId (unique) instead of
sessionName (display-only) to prevent merging different agents with
the same name
- InboxListView: Add tabIndex={0} and onKeyDown to the listbox div so
it is keyboard-focusable and navigable
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/renderer/hooks/useAgentInbox.ts (1)
5-5:⚠️ Potential issue | 🟡 Minor
MAX_MESSAGE_LENGTHstill violates theInboxItem.lastMessagecontract.Line 5 uses
250, butsrc/renderer/types/agent-inbox.tsdocumentslastMessageas truncated to 90 chars. This creates contract/UI/test drift.Suggested fix
-const MAX_MESSAGE_LENGTH = 250; +const MAX_MESSAGE_LENGTH = 90;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useAgentInbox.ts` at line 5, MAX_MESSAGE_LENGTH is set to 250 but the InboxItem.lastMessage contract in agent-inbox.ts specifies messages must be truncated to 90 chars; change the constant MAX_MESSAGE_LENGTH in useAgentInbox.ts from 250 to 90 so the hook's truncation logic matches the documented contract (ensure any references to MAX_MESSAGE_LENGTH in functions within useAgentInbox.ts reflect the new value).src/renderer/components/AgentInbox/InboxListView.tsx (2)
1058-1066:⚠️ Potential issue | 🟡 MinorListbox focus handling is incomplete for keyboard UX.
The listbox is focusable, but it still lacks explicit focus/blur handlers for focus-visible behavior and reliable keyboard interaction state.
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 1058 - 1066, The listbox in InboxListView.tsx (the div with role="listbox", tabIndex={0}, onKeyDown={handleKeyDown}, aria-activedescendant={selectedItemId}) needs explicit focus/blur handling: add onFocus and onBlur handlers that set a local focus state (e.g., isListboxFocused or isFocusVisible) so keyboard navigation and visual focus styles can rely on that state, and ensure handleKeyDown uses that state when handling arrow/enter keys; update the element's className to conditionally include a focus-visible class based on that state and clear/reset any selected/active item on blur to keep interaction state consistent.
39-42:⚠️ Potential issue | 🟡 MinorUse “agent” in user-facing copy and ARIA labels.
Strings like “No unread sessions.” and labels like “Sort sessions” should use “agent” terminology for UI consistency.
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.Also applies to: 1042-1050
🤖 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 39 - 42, Update user-facing copy and ARIA labels in InboxListView to use "agent" instead of "session" while leaving the underlying Session interface and variable names intact; specifically change the text values in the no-results map (the object with keys all, unread, read, starred) from "No ... sessions" to "No ... agents" and update any visible labels/ARIA text such as the "Sort sessions" label to "Sort agents" (also apply the same replacements referenced around the other block at lines 1042-1050). Ensure only displayed strings and aria-label props are modified — do not rename the Session type, variables, or provider-level identifiers.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
425-452:⚠️ Potential issue | 🟠 MajorSidebar interactive rows are still mouse-only.
These rows are clickable
divs without keyboard semantics/focus handling, so keyboard users can’t reliably navigate or activate them.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: 468-533
🤖 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 425 - 452, The header row div used to toggle groups (keyed by `header-${row.groupName}-${rowIdx}`) is only clickable via mouse; make it keyboard accessible by adding tabIndex={0}, role="button", and an onKeyDown handler that calls the same toggle logic used in the onClick (the setter `setCollapsedGroups`) when Enter or Space is pressed, and optionally add onFocus/onBlur handlers to manage focus styles; apply the same changes to the other similar interactive row block around the `row` rendering (the block referenced at lines ~468-533) so keyboard users can navigate and activate group toggles.
🤖 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 384-390: The current grouping logic builds groupKey from display
names (useGroupName ? (itm.groupName ?? 'Ungrouped') : itm.sessionName) which
collapses distinct sessions that share the same sessionName; change the grouping
key to use a stable unique identifier when available (e.g., use itm.sessionId
when grouping by session) while still using sessionName/groupName only for the
visible label. Specifically, in the block that computes groupKey and updates
groupMap/groupOrder, construct groupKey as something like useGroupName ?
`group:${itm.groupId ?? itm.groupName ?? 'Ungrouped'}` :
`session:${itm.sessionId ?? itm.sessionName}` and keep
itm.sessionName/itm.groupName for display; update references to groupMap,
groupOrder and the push into groupMap.get(groupKey) to use that new key so
navigation routes remain unambiguous.
- Line 969: Change the user-facing copy in the FocusModeView component so it
uses "agent" instead of "session": replace the literal "Session no longer
available" (the span in FocusModeView.tsx) with phrasing that uses agent
terminology (e.g., "Agent no longer available" or "Agent is no longer
available") to follow the UI language guideline while leaving any internal
Session interface and provider-level names unchanged.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 62-63: The grouping currently uses display name (sessionName)
which causes collisions for distinct sessions with the same name; change
grouping/collapse/stat keys to use a stable identifier (sessionId) while still
rendering sessionName as the visible label. Update the groupKey computation in
InboxListView (and any collapse/stat logic that references groupKey/lastGroup)
to use sessionId when sortMode === 'byAgent' (e.g., use item.sessionId for
internal keys and item.sessionName for the header text), and propagate this
pattern to the same affected areas (lines around the other occurrences
referenced) so headers, collapse state and counts are keyed by sessionId but
display the sessionName.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 425-452: The header row div used to toggle groups (keyed by
`header-${row.groupName}-${rowIdx}`) is only clickable via mouse; make it
keyboard accessible by adding tabIndex={0}, role="button", and an onKeyDown
handler that calls the same toggle logic used in the onClick (the setter
`setCollapsedGroups`) when Enter or Space is pressed, and optionally add
onFocus/onBlur handlers to manage focus styles; apply the same changes to the
other similar interactive row block around the `row` rendering (the block
referenced at lines ~468-533) so keyboard users can navigate and activate group
toggles.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1058-1066: The listbox in InboxListView.tsx (the div with
role="listbox", tabIndex={0}, onKeyDown={handleKeyDown},
aria-activedescendant={selectedItemId}) needs explicit focus/blur handling: add
onFocus and onBlur handlers that set a local focus state (e.g., isListboxFocused
or isFocusVisible) so keyboard navigation and visual focus styles can rely on
that state, and ensure handleKeyDown uses that state when handling arrow/enter
keys; update the element's className to conditionally include a focus-visible
class based on that state and clear/reset any selected/active item on blur to
keep interaction state consistent.
- Around line 39-42: Update user-facing copy and ARIA labels in InboxListView to
use "agent" instead of "session" while leaving the underlying Session interface
and variable names intact; specifically change the text values in the no-results
map (the object with keys all, unread, read, starred) from "No ... sessions" to
"No ... agents" and update any visible labels/ARIA text such as the "Sort
sessions" label to "Sort agents" (also apply the same replacements referenced
around the other block at lines 1042-1050). Ensure only displayed strings and
aria-label props are modified — do not rename the Session type, variables, or
provider-level identifiers.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Line 5: MAX_MESSAGE_LENGTH is set to 250 but the InboxItem.lastMessage
contract in agent-inbox.ts specifies messages must be truncated to 90 chars;
change the constant MAX_MESSAGE_LENGTH in useAgentInbox.ts from 250 to 90 so the
hook's truncation logic matches the documented contract (ensure any references
to MAX_MESSAGE_LENGTH in functions within useAgentInbox.ts reflect the new
value).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/hooks/useAgentInbox.ts
…d Inbox - Replace "session" with "agent" in all user-facing labels (empty states, ARIA labels, menu text) - Align MAX_MESSAGE_LENGTH (250→90) with documented InboxItem.lastMessage truncation - Fix read filter to show all non-unread items regardless of session state - Add role="switch" and aria-checked to Encore feature toggle buttons - Guard agentInbox keyboard shortcut on setAgentInboxOpen callback availability - Add keyboard a11y (tabIndex, role="option", aria-selected, onKeyDown) to FocusModeView sidebar - Add onFocus/onBlur handlers to dialog container per focus guidelines - Fix context threshold label (>80% → ≥80%) to match >= 80 condition - Remove background/border shorthand overrides from group header styles - Fix playbook doc contradictions (ENCORE-INBOX-07, -11, -14) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/renderer/components/AgentInbox/FocusModeView.tsx (3)
992-992:⚠️ Potential issue | 🟡 MinorUse “agent” wording in this user-facing empty-state message.
The message should follow product terminology used in UI copy.
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`.📝 Suggested copy fix
-<span className="text-sm">Session no longer available</span> +<span className="text-sm">Agent no longer available</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/FocusModeView.tsx` at line 992, Update the user-facing empty-state copy in the FocusModeView component: replace the JSX string "Session no longer available" (the <span className="text-sm"> in FocusModeView) with wording that uses "agent" per product terminology (e.g., "Agent no longer available" or "The agent is no longer available") so UI text uses 'agent' while keeping internal code using the Session interface.
384-390:⚠️ Potential issue | 🟠 MajorUse stable identity keys for by-agent grouping.
Grouping by
sessionNamecan merge distinct sessions that share a label, which corrupts header collapse and navigation behavior in by-agent mode.🔧 Proposed fix
-const groupKey = useGroupName ? (itm.groupName ?? 'Ungrouped') : itm.sessionName; +const groupKey = useGroupName + ? `group:${itm.groupId ?? itm.groupName ?? 'Ungrouped'}` + : `session:${itm.sessionId}`; +const groupLabel = useGroupName ? (itm.groupName ?? 'Ungrouped') : itm.sessionName; ... -if (!groupMap.has(groupKey)) { - groupMap.set(groupKey, []); - groupOrder.push(groupKey); -} +if (!groupMap.has(groupKey)) { + groupMap.set(groupKey, []); + groupOrder.push(groupKey); +} ... -result.push({ type: 'header', groupName: groupKey, count: groupItems.length }); +result.push({ type: 'header', groupName: groupLabel, count: groupItems.length });🤖 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 384 - 390, The grouping currently uses sessionName when useGroupName is false which can merge distinct sessions with the same label; change the group key to a stable unique identifier (e.g., use itm.sessionId or a composite key like `${itm.sessionId}::${itm.sessionName}`) instead of itm.sessionName, keeping the existing branch that uses itm.groupName when useGroupName is true; update the code that builds groupKey (the variables groupKey, useGroupName, itm.groupName, itm.sessionName and the groupMap/groupOrder logic) so headers/collapse/navigation rely on the stable id rather than the human-readable sessionName.
425-429:⚠️ Potential issue | 🟡 MinorKeyboard-focusable sidebar rows still need focus handlers.
These rows are focusable (
tabIndex={0}) but do not defineonFocus/onBlur, so they miss the keyboard-focus handling contract and visible focus management.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: 485-488
🤖 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 425 - 429, The header row divs in FocusModeView.tsx (the element with key={`header-${row.groupName}-${rowIdx}`} and the similar row at the other location) are keyboard-focusable via tabIndex={0} but lack focus handlers; add onFocus and onBlur props that call new or existing handlers (e.g., handleRowFocus and handleRowBlur) to manage focus state and visible focus styles, update any local state (like focusedRow or focusedGroup) used by rendering, and ensure the handlers receive identifying info (row.groupName and rowIdx) so focus can be tracked per-row; apply the same change to the other focusable div referenced in the comment.src/renderer/components/AgentInbox/InboxListView.tsx (2)
60-63:⚠️ Potential issue | 🟠 Major
sessionNameis still used as identity in by-agent flows.This can merge different sessions sharing the same display name, which affects headers, collapse state, quick toggles, and “Agents” stats.
🔧 Suggested direction
-const groupKey = sortMode === 'byAgent' ? item.sessionName : (item.groupName ?? 'Ungrouped'); +const groupKey = + sortMode === 'byAgent' + ? `session:${item.sessionId}` + : `group:${item.groupId ?? item.groupName ?? 'Ungrouped'}`;Also update:
- collapse maps (
agentUnreads,collapseKey,toggleGroupinputs) to use stable keys.- header display text to continue showing
sessionName.InboxStatsStripunique count to usesessionIdinstead ofsessionName.Also applies to: 409-409, 541-543, 560-562, 883-883
🤖 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 60 - 63, The code uses sessionName as the identity key (e.g., in groupKey calculation inside InboxListView) which collapses different sessions that share the same display name; change grouping and all collapse/track maps to use the stable sessionId for identity while continuing to display sessionName in headers. Specifically, replace uses of sessionName as the unique key in groupKey (when sortMode === 'byAgent'), agentUnreads map keys, collapseKey/toggleGroup inputs, and any unique-count logic in InboxStatsStrip to use sessionId, but keep sessionName only for visible header text; update any helper functions that build or compare keys to accept sessionId + optional groupName (or fallbacks) so collapse state and quick toggles remain stable across sessions with identical display names.
1058-1065:⚠️ Potential issue | 🟡 MinorFocusable listbox should include focus event handlers.
The listbox is keyboard-focusable but does not define
onFocus/onBlur, which misses the component keyboard-focus rule.As per coding guidelines `src/renderer/components/**/*.{ts,tsx}: Add tabIndex attribute and focus event handlers when implementing components that need keyboard focus`.🔧 Suggested patch
<div role="listbox" tabIndex={0} onKeyDown={handleKeyDown} aria-activedescendant={selectedItemId} aria-label="Inbox items" 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={{ flex: 1, overflow: 'hidden' }} >🤖 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 1058 - 1065, The listbox div in InboxListView is keyboard-focusable but missing focus event handlers; add onFocus and onBlur handlers (e.g., handleListFocus and handleListBlur or reuse existing focus management functions) to the element with role="listbox" so the component satisfies the keyboard-focus rule; update the component (InboxListView) to implement these handlers to set/clear any focus state (used alongside handleKeyDown and aria-activedescendant/selectedItemId) and ensure they are bound to the div (onFocus={...}, onBlur={...}) to keep focus behavior and accessibility consistent.
🤖 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/index.tsx`:
- Around line 255-258: When delegating keyboard events to InboxListView you must
avoid double-handling by first checking that the event hasn't already been
handled and that the event target is not already inside the InboxListView;
change the delegate in the block that references viewMode and listKeyDownRef so
it only calls listKeyDownRef.current(e) when e.defaultPrevented is false (or
e.isTrusted as appropriate) AND the event target is not contained in the
InboxListView root (use an existing list root ref or querySelector for the
InboxListView container to implement an isEventInsideInboxList check). This
ensures the InboxListView handles keys when focus is inside it and prevents the
parent dialog from re-processing the same keystroke.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 47-50: The truncate function currently returns text.slice(0,
MAX_MESSAGE_LENGTH) + '...' which can exceed the intended MAX_MESSAGE_LENGTH
(the lastMessage 90-char contract); update truncate (referencing function name
truncate and constant MAX_MESSAGE_LENGTH) to ensure the returned string length
never exceeds MAX_MESSAGE_LENGTH by slicing to MAX_MESSAGE_LENGTH - 3 and then
appending '...' only when text.length > MAX_MESSAGE_LENGTH, and handle the edge
case where MAX_MESSAGE_LENGTH <= 3 by just slicing to MAX_MESSAGE_LENGTH without
adding ellipsis.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Line 992: Update the user-facing empty-state copy in the FocusModeView
component: replace the JSX string "Session no longer available" (the <span
className="text-sm"> in FocusModeView) with wording that uses "agent" per
product terminology (e.g., "Agent no longer available" or "The agent is no
longer available") so UI text uses 'agent' while keeping internal code using the
Session interface.
- Around line 384-390: The grouping currently uses sessionName when useGroupName
is false which can merge distinct sessions with the same label; change the group
key to a stable unique identifier (e.g., use itm.sessionId or a composite key
like `${itm.sessionId}::${itm.sessionName}`) instead of itm.sessionName, keeping
the existing branch that uses itm.groupName when useGroupName is true; update
the code that builds groupKey (the variables groupKey, useGroupName,
itm.groupName, itm.sessionName and the groupMap/groupOrder logic) so
headers/collapse/navigation rely on the stable id rather than the human-readable
sessionName.
- Around line 425-429: The header row divs in FocusModeView.tsx (the element
with key={`header-${row.groupName}-${rowIdx}`} and the similar row at the other
location) are keyboard-focusable via tabIndex={0} but lack focus handlers; add
onFocus and onBlur props that call new or existing handlers (e.g.,
handleRowFocus and handleRowBlur) to manage focus state and visible focus
styles, update any local state (like focusedRow or focusedGroup) used by
rendering, and ensure the handlers receive identifying info (row.groupName and
rowIdx) so focus can be tracked per-row; apply the same change to the other
focusable div referenced in the comment.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 60-63: The code uses sessionName as the identity key (e.g., in
groupKey calculation inside InboxListView) which collapses different sessions
that share the same display name; change grouping and all collapse/track maps to
use the stable sessionId for identity while continuing to display sessionName in
headers. Specifically, replace uses of sessionName as the unique key in groupKey
(when sortMode === 'byAgent'), agentUnreads map keys, collapseKey/toggleGroup
inputs, and any unique-count logic in InboxStatsStrip to use sessionId, but keep
sessionName only for visible header text; update any helper functions that build
or compare keys to accept sessionId + optional groupName (or fallbacks) so
collapse state and quick toggles remain stable across sessions with identical
display names.
- Around line 1058-1065: The listbox div in InboxListView is keyboard-focusable
but missing focus event handlers; add onFocus and onBlur handlers (e.g.,
handleListFocus and handleListBlur or reuse existing focus management functions)
to the element with role="listbox" so the component satisfies the keyboard-focus
rule; update the component (InboxListView) to implement these handlers to
set/clear any focus state (used alongside handleKeyDown and
aria-activedescendant/selectedItemId) and ensure they are bound to the div
(onFocus={...}, onBlur={...}) to keep focus behavior and accessibility
consistent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-11.mdplaybooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.mdsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/useAgentInbox.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/renderer/hooks/keyboard/useMainKeyboardHandler.ts
- playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-07.md
- src/renderer/components/SettingsModal.tsx
- playbooks/agent-inbox/2026-02-26-Encore-Inbox-Refactor/ENCORE-INBOX-14.md
Author
|
All review feedback addressed: inboxProcessInputRef binding, handler prop wiring, tabId navigation, resize listener cleanup, byAgent group key collision (sessionId), listbox a11y, terminology (session->agent), MAX_MESSAGE_LENGTH alignment, read filter fix, Encore toggle a11y (role=switch), shortcut guard, sidebar keyboard a11y, context threshold label, header style override, playbook doc corrections. Requesting re-review. @coderabbitai review @greptile review |
|
Ready to review! ✅ Actions performedReview triggered.
|
…indicators - Replace non-deterministic setTimeout with requestAnimationFrame in handleAgentInboxQuickReply; use activeSessionIdRef to avoid stale closures - Add role="switch" + aria-checked to tabDescription Encore toggle for consistency with unifiedInbox toggle - Add onFocus/onBlur handlers to FocusModeView sidebar items mirroring onMouseEnter/onMouseLeave hover background - Add visible focus outline to InboxListView listbox via onFocus/onBlur inline handlers (keeps outline-none class to prevent double-outline) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace O(n*m) .find() loop in resolveFrozenItems with O(n+m) Map lookup - Replace setTimeout(200) with double requestAnimationFrame for reply focus - Add circular dependency comment to AgentInboxModalData loose string types - Add comment explaining STATUS_COLORS cannot use keyof ThemeColors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- handleAgentInboxQuickReply and handleAgentInboxOpenAndReply now set inputMode: 'ai' and activeFileTabId: null before sending, preventing misrouted text when target session is in terminal mode - Simplify AgentInbox focus restoration: remove useEffect cleanup that was cancelling the close-time RAF, capture trigger as HTMLElement directly, and schedule focus after onClose without ref tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Addressed remaining review feedback on Unified Inbox: byAgent key collision fixed with stable sessionId grouping, quick-reply timing made deterministic (no nested rAF), truncate now preserves max length contract, list keyboard delegation guarded against double handling, modal store first-open data merge fixed, focus/listbox a11y and agent-facing copy aligned. Requesting re-review. @coderabbitai review @greptile review |
|
✅ Actions performedReview triggered.
|
Author
|
Follow-up push applied on top of the re-review request: keyboard delegate guard in AgentInbox list mode () and user-facing copy update (). Please re-run review on latest commit d5ec10f. @coderabbitai review @greptile review |
Author
|
Follow-up push applied on top of the re-review request: added keyboard delegate guard in AgentInbox list mode (check defaultPrevented and target===currentTarget) and updated user-facing copy to "Agent no longer available". Please review latest commit d5ec10f. @coderabbitai review @greptile review |
|
✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/renderer/components/AgentInbox/FocusModeView.tsx (2)
434-457:⚠️ Potential issue | 🟡 MinorHeader toggle is focusable but missing focus event handling.
Line 436 adds keyboard focus (
tabIndex={0}), but this interactive header block does not attach focus/blur handlers like the item rows do, so keyboard focus affordance is inconsistent.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 header div that toggles groups (the element with key `header-${row.groupKey}-${rowIdx}`) is focusable via tabIndex={0} but lacks focus/blur handlers; update this element to add onFocus and onBlur handlers (mirroring the item row handlers) that call the same focus state updater used elsewhere so keyboard focus affordance is consistent, e.g. invoke the component's focus handler(s) used for item rows and ensure these handlers accept the `row.groupKey`/`rowIdx` context; keep the existing onClick/onKeyDown toggle logic and only add symmetric focus management to integrate with setCollapsedGroups and the component's focus state.
387-391:⚠️ Potential issue | 🟠 MajorUse stable IDs for grouped keys to avoid collisions.
Line 387 keys grouped rows by
groupName. Two different groups with the same label will merge into one bucket and break collapse/navigation behavior.🔧 Proposed fix
- const groupKey = useGroupName ? (itm.groupName ?? 'Ungrouped') : itm.sessionId; + const groupKey = useGroupName + ? `group:${itm.groupId ?? itm.groupName ?? 'Ungrouped'}` + : `session:${itm.sessionId}`; const groupName = useGroupName ? (itm.groupName ?? 'Ungrouped') : itm.sessionName;🤖 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 387 - 391, The grouping currently uses groupName as the groupKey which causes distinct groups with identical labels to collide; change the groupKey calculation in FocusModeView to use a stable unique identifier (e.g., itm.groupId or a deterministic composite key) instead of the human-visible label: useGroupName ? (itm.groupId ?? `name:${itm.groupName}`) : itm.sessionId (or fall back to a composite like `name:${itm.groupName}|session:${itm.sessionId}`) so groupMap, groupOrder, and collapse/navigation logic reference a collision-free key while still storing the readable groupName for display.src/renderer/stores/modalStore.ts (1)
668-671:⚠️ Potential issue | 🟠 MajorPreserve Agent Inbox state on repeated open calls.
Line 670 opens with
undefineddata. If the modal is already open, this can wipe existingfilterMode/sortMode/isExpandedstate when the open action is triggered again (e.g., shortcut/menu re-entry).🔧 Proposed fix
// Agent Inbox Modal - setAgentInboxOpen: (open: boolean) => - open ? openModal('agentInbox') : closeModal('agentInbox'), - updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), + setAgentInboxOpen: (open: boolean) => { + if (!open) { + closeModal('agentInbox'); + return; + } + const current = useModalStore.getState().getData('agentInbox'); + openModal('agentInbox', current ?? { filterMode: 'all', sortMode: 'newest', isExpanded: false }); + }, + 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 668 - 671, The setAgentInboxOpen handler currently triggers an open that ends up wiping modal data (clearing filterMode/sortMode/isExpanded) on repeated opens; change it so opening the 'agentInbox' modal does not overwrite existing modal data—either call openModal('agentInbox') in a way that does not pass/overwrite data or modify openModal to merge incoming data with existing modal state instead of replacing it; ensure the change references setAgentInboxOpen, openModal, closeModal and preserves filterMode/sortMode/isExpanded for the 'agentInbox' modal.
🧹 Nitpick comments (2)
src/renderer/components/AgentInbox/index.tsx (1)
292-293: Empty focus handlers provide no visible feedback.Lines 292-293 have placeholder focus/blur handlers that do nothing:
onFocus={() => {}} onBlur={() => {}}Since the container uses
tabIndex={-1}(programmatically focused, not tab-navigable), this is acceptable for the current design. However, if you want visible focus indication when the dialog receives focus, consider adding outline styling similar to other components in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AgentInbox/index.tsx` around lines 292 - 293, The empty onFocus and onBlur handlers on the AgentInbox container (the JSX attributes onFocus={() => {}} and onBlur={() => {}}) are no-ops; either remove these placeholder handlers or implement focus styling similar to other components so the dialog shows visible focus when programmatically focused (tabIndex={-1}); to fix, update the AgentInbox component by deleting the empty handlers or replacing them with handlers that toggle a focused CSS class (e.g., set state in the component like isFocused via handleFocus/handleBlur) and apply an outline/focus style in the component's CSS so the dialog displays a visible focus ring when focused.src/renderer/components/AgentInbox/InboxListView.tsx (1)
1126-1138: Consider pre-computing header metadata forbyAgentmode.The loop at Lines 1130-1137 iterates subsequent rows on every render to derive
agentToolTypeandunreadCount. While groups are typically small, this could be pre-computed in thebuildRowsfunction for better performance.🤖 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 1126 - 1138, Pre-compute the header metadata instead of scanning subsequent rows during render: update buildRows to, when creating a header row for sortMode 'byAgent', walk its group once and attach metadata fields (e.g., header.agentToolType and header.unreadCount) to the header row object; then change the render logic in InboxListView (where virtualRow, rows, agentToolType and unreadCount are referenced) to read those new header properties directly when sortMode === 'byAgent' rather than iterating subsequent rows at render time.
🤖 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 1541-1564: The pendingInboxQuickReply state currently holds a
single object and can be overwritten, causing dropped quick-replies; change it
to a queue (array) type, e.g., useState<Array<{targetSessionId:string;
previousActiveSessionId:string|null; text:string}>> and update enqueue calls to
push to the array rather than replace; update the effect that flushes
pendingInboxQuickReply (and the analogous block at lines 1588-1594) to process
items FIFO: while the first item matches activeSession?.id, call
inboxProcessInputRef.current(item.text), capture previousActiveSessionId from
that item, remove the processed item from the queue (set state to the remaining
array), and if previousActiveSessionId requires restoring, queueMicrotask to
setActiveSessionId(previousActiveSessionId); ensure all state updates use
functional setState to avoid races.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 228-239: The icon-only toggle button for raw/formatted view
(onToggleRaw, showRawMarkdown, Eye, FileText) lacks an accessible name; add an
explicit accessible attribute (e.g., aria-label or aria-pressed) to the <button>
so screen readers announce its purpose and state—for example
aria-label={showRawMarkdown ? "Show formatted view" : "Show plain text view"} or
aria-pressed={showRawMarkdown}; apply the same fix to the other icon-only
buttons in this component that use only title attributes.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 434-457: The header div that toggles groups (the element with key
`header-${row.groupKey}-${rowIdx}`) is focusable via tabIndex={0} but lacks
focus/blur handlers; update this element to add onFocus and onBlur handlers
(mirroring the item row handlers) that call the same focus state updater used
elsewhere so keyboard focus affordance is consistent, e.g. invoke the
component's focus handler(s) used for item rows and ensure these handlers accept
the `row.groupKey`/`rowIdx` context; keep the existing onClick/onKeyDown toggle
logic and only add symmetric focus management to integrate with
setCollapsedGroups and the component's focus state.
- Around line 387-391: The grouping currently uses groupName as the groupKey
which causes distinct groups with identical labels to collide; change the
groupKey calculation in FocusModeView to use a stable unique identifier (e.g.,
itm.groupId or a deterministic composite key) instead of the human-visible
label: useGroupName ? (itm.groupId ?? `name:${itm.groupName}`) : itm.sessionId
(or fall back to a composite like
`name:${itm.groupName}|session:${itm.sessionId}`) so groupMap, groupOrder, and
collapse/navigation logic reference a collision-free key while still storing the
readable groupName for display.
In `@src/renderer/stores/modalStore.ts`:
- Around line 668-671: The setAgentInboxOpen handler currently triggers an open
that ends up wiping modal data (clearing filterMode/sortMode/isExpanded) on
repeated opens; change it so opening the 'agentInbox' modal does not overwrite
existing modal data—either call openModal('agentInbox') in a way that does not
pass/overwrite data or modify openModal to merge incoming data with existing
modal state instead of replacing it; ensure the change references
setAgentInboxOpen, openModal, closeModal and preserves
filterMode/sortMode/isExpanded for the 'agentInbox' modal.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1126-1138: Pre-compute the header metadata instead of scanning
subsequent rows during render: update buildRows to, when creating a header row
for sortMode 'byAgent', walk its group once and attach metadata fields (e.g.,
header.agentToolType and header.unreadCount) to the header row object; then
change the render logic in InboxListView (where virtualRow, rows, agentToolType
and unreadCount are referenced) to read those new header properties directly
when sortMode === 'byAgent' rather than iterating subsequent rows at render
time.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 292-293: The empty onFocus and onBlur handlers on the AgentInbox
container (the JSX attributes onFocus={() => {}} and onBlur={() => {}}) are
no-ops; either remove these placeholder handlers or implement focus styling
similar to other components so the dialog shows visible focus when
programmatically focused (tabIndex={-1}); to fix, update the AgentInbox
component by deleting the empty handlers or replacing them with handlers that
toggle a focused CSS class (e.g., set state in the component like isFocused via
handleFocus/handleBlur) and apply an outline/focus style in the component's CSS
so the dialog displays a visible focus ring when focused.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/types/agent-inbox.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/types/agent-inbox.ts
Comment on lines
+1541
to
+1564
| const [pendingInboxQuickReply, setPendingInboxQuickReply] = useState<{ | ||
| targetSessionId: string; | ||
| previousActiveSessionId: string | null; | ||
| text: string; | ||
| } | null>(null); | ||
|
|
||
| // Flush pending quick-reply once target session is active (deterministic, no RAF timing chain). | ||
| 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.
Back-to-back quick replies can be dropped due to single-slot pending state.
pendingInboxQuickReply only stores one item. A second quick reply can overwrite the first before the effect flushes it, causing silent message loss.
💡 Proposed fix (queue pending quick replies instead of overwrite)
- const [pendingInboxQuickReply, setPendingInboxQuickReply] = useState<{
+ type PendingInboxQuickReply = {
targetSessionId: string;
previousActiveSessionId: string | null;
text: string;
- } | null>(null);
+ };
+ const [pendingInboxQuickReplies, setPendingInboxQuickReplies] = useState<
+ PendingInboxQuickReply[]
+ >([]);
useEffect(() => {
- if (!pendingInboxQuickReply) return;
- if (activeSession?.id !== pendingInboxQuickReply.targetSessionId) return;
+ const pending = pendingInboxQuickReplies[0];
+ if (!pending) return;
+ if (activeSession?.id !== pending.targetSessionId) {
+ setActiveSessionId(pending.targetSessionId);
+ return;
+ }
- inboxProcessInputRef.current(pendingInboxQuickReply.text);
- const previousActiveSessionId = pendingInboxQuickReply.previousActiveSessionId;
- setPendingInboxQuickReply(null);
+ inboxProcessInputRef.current(pending.text);
+ const previousActiveSessionId = pending.previousActiveSessionId;
+ setPendingInboxQuickReplies((prev) => prev.slice(1));
- if (previousActiveSessionId && previousActiveSessionId !== pendingInboxQuickReply.targetSessionId) {
+ if (previousActiveSessionId && previousActiveSessionId !== pending.targetSessionId) {
queueMicrotask(() => {
setActiveSessionId(previousActiveSessionId);
});
}
- }, [pendingInboxQuickReply, activeSession?.id, setActiveSessionId]);
+ }, [pendingInboxQuickReplies, activeSession?.id, setActiveSessionId]);
const handleAgentInboxQuickReply = useCallback(
(sessionId: string, tabId: string, text: string) => {
const previousActiveSessionId = activeSessionIdRef.current;
setSessions((prev) =>
prev.map((s) => {
if (s.id !== sessionId) return s;
return {
...s,
activeTabId: tabId,
activeFileTabId: null,
inputMode: 'ai' as const,
aiTabs: s.aiTabs.map((t) => (t.id === tabId ? { ...t, hasUnread: false } : t)),
};
})
);
- setPendingInboxQuickReply({
- targetSessionId: sessionId,
- previousActiveSessionId,
- text,
- });
- setActiveSessionId(sessionId);
+ setPendingInboxQuickReplies((prev) => [
+ ...prev,
+ { targetSessionId: sessionId, previousActiveSessionId, text },
+ ]);
},
[setSessions, setActiveSessionId, activeSessionIdRef]
);Also applies to: 1588-1594
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/App.tsx` around lines 1541 - 1564, The pendingInboxQuickReply
state currently holds a single object and can be overwritten, causing dropped
quick-replies; change it to a queue (array) type, e.g.,
useState<Array<{targetSessionId:string; previousActiveSessionId:string|null;
text:string}>> and update enqueue calls to push to the array rather than
replace; update the effect that flushes pendingInboxQuickReply (and the
analogous block at lines 1588-1594) to process items FIFO: while the first item
matches activeSession?.id, call inboxProcessInputRef.current(item.text), capture
previousActiveSessionId from that item, remove the processed item from the queue
(set state to the remaining array), and if previousActiveSessionId requires
restoring, queueMicrotask to setActiveSessionId(previousActiveSessionId); ensure
all state updates use functional setState to avoid races.
Comment on lines
+228
to
+239
| <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'} | ||
| > | ||
| {showRawMarkdown ? ( | ||
| <Eye className="w-3.5 h-3.5" /> | ||
| ) : ( | ||
| <FileText className="w-3.5 h-3.5" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Add accessible names to icon-only buttons.
These controls are icon-only and currently rely on title. Screen readers may announce them as unlabeled buttons.
🔧 Proposed fix
<button
onClick={onToggleRaw}
+ aria-label={showRawMarkdown ? 'Show formatted' : '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 inbox"
className="p-1.5 rounded"
style={{ color: theme.colors.textDim }}
@@
<button
onClick={handleQuickReply}
+ aria-label="Send quick reply"
disabled={!replyText.trim()}
@@
<button
onClick={handleOpenAndReply}
+ aria-label="Open agent 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 - 239,
The icon-only toggle button for raw/formatted view (onToggleRaw,
showRawMarkdown, Eye, FileText) lacks an accessible name; add an explicit
accessible attribute (e.g., aria-label or aria-pressed) to the <button> so
screen readers announce its purpose and state—for example
aria-label={showRawMarkdown ? "Show formatted view" : "Show plain text view"} or
aria-pressed={showRawMarkdown}; apply the same fix to the other icon-only
buttons in this component that use only title attributes.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/renderer/components/AgentInbox/FocusModeView.tsx (1)
228-239:⚠️ Potential issue | 🟠 MajorAdd explicit accessible names to icon-only buttons.
These controls still depend on
title, which is not a reliable accessible name for screen readers.♿ Proposed fix
<button onClick={onToggleRaw} + aria-label={showRawMarkdown ? 'Show formatted view' : 'Show plain text view'} className="p-1 rounded opacity-0 group-hover:opacity-50 hover:!opacity-100 transition-opacity" ... > <button onClick={onClose} + aria-label="Close inbox" className="p-1.5 rounded" ... > <button onClick={handleQuickReply} + aria-label="Send quick reply" disabled={!replyText.trim()} ... > <button onClick={handleOpenAndReply} + aria-label="Open agent 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 - 239, The icon-only toggle button using onToggleRaw and state showRawMarkdown relies on title for accessibility; add an explicit accessible name by adding an aria-label (e.g., aria-label={showRawMarkdown ? 'Show formatted' : 'Show plain text'}) to the button (keep title if desired) so screen readers receive a proper name; apply the same fix to the other icon-only buttons in this file (the ones rendering Eye/FileText-like icons) so each button uses a clear aria-label tied to its state or action.src/renderer/App.tsx (1)
1541-1564:⚠️ Potential issue | 🟠 MajorQueue pending quick replies instead of storing only one.
pendingInboxQuickReplyis single-slot state, so a second quick reply can overwrite the first before the effect flushes, causing silent message loss.💡 Proposed fix
-const [pendingInboxQuickReply, setPendingInboxQuickReply] = useState<{ +type PendingInboxQuickReply = { targetSessionId: string; previousActiveSessionId: string | null; text: string; -} | null>(null); +}; +const [pendingInboxQuickReplies, setPendingInboxQuickReplies] = useState< + PendingInboxQuickReply[] +>([]); useEffect(() => { - if (!pendingInboxQuickReply) return; - if (activeSession?.id !== pendingInboxQuickReply.targetSessionId) return; + const pending = pendingInboxQuickReplies[0]; + if (!pending) return; + if (activeSession?.id !== pending.targetSessionId) return; - inboxProcessInputRef.current(pendingInboxQuickReply.text); - const previousActiveSessionId = pendingInboxQuickReply.previousActiveSessionId; - setPendingInboxQuickReply(null); + inboxProcessInputRef.current(pending.text); + const previousActiveSessionId = pending.previousActiveSessionId; + setPendingInboxQuickReplies((prev) => prev.slice(1)); - if (previousActiveSessionId && previousActiveSessionId !== pendingInboxQuickReply.targetSessionId) { + if (previousActiveSessionId && previousActiveSessionId !== pending.targetSessionId) { queueMicrotask(() => setActiveSessionId(previousActiveSessionId)); } -}, [pendingInboxQuickReply, activeSession?.id, setActiveSessionId]); +}, [pendingInboxQuickReplies, activeSession?.id, setActiveSessionId]); - setPendingInboxQuickReply({ + setPendingInboxQuickReplies((prev) => [ + ...prev, + { targetSessionId: sessionId, previousActiveSessionId, text, - }); + }, + ]);Also applies to: 1588-1594
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/App.tsx` around lines 1541 - 1564, The current single-slot pendingInboxQuickReply state can be overwritten and drop a second quick reply; change pendingInboxQuickReply to a FIFO queue (array) and update its setter (setPendingInboxQuickReply) usage to push new entries instead of replacing, then update the effect that flushes pendingInboxQuickReply to dequeue and process entries one-by-one (calling inboxProcessInputRef.current for each entry and restoring previousActiveSessionId via setActiveSessionId as the original logic dictates, using queueMicrotask when needed); apply the same queue change to the other occurrence around the code that mirrors this logic (the block referenced at 1588-1594) so both places handle multiple pending quick replies safely.
🧹 Nitpick comments (1)
src/renderer/stores/modalStore.ts (1)
141-147: Type the Agent Inbox data path end-to-end.Line [671] accepts
Record<string, unknown>, which bypasses theAgentInboxModalDatacontract and allows invalid payloads into persisted inbox state.💡 Suggested fix
export interface AgentInboxModalData { - filterMode?: string; - sortMode?: string; + filterMode?: 'all' | 'unread' | 'read' | 'starred'; + sortMode?: 'newest' | 'oldest' | 'grouped' | 'byAgent'; isExpanded?: boolean; } @@ - updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), + updateAgentInboxData: (data: Partial<AgentInboxModalData>) => + updateModalData('agentInbox', data),Also applies to: 668-672
🤖 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 141 - 147, The persisted inbox updater currently accepts Record<string, unknown> which circumvents the AgentInboxModalData contract and lets invalid payloads through; change the function signatures and any call sites that take or store Record<string, unknown> (the usages around the block that includes the Record parameter near lines 668–672) to accept AgentInboxModalData instead, propagate that type through any helpers that read/write the modal data, and adjust filterMode/sortMode handling to validate or narrow to the correct enum/union if needed so the AgentInboxModalData shape is enforced end-to-end.
🤖 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 327-337: The code removes native focus outlines on the filter chip
button (the inline style with outline: 'none'), which breaks keyboard
accessibility; restore visible focus by removing outline: 'none', add
tabIndex={0} to the button rendering (the button keyed by opt.value that calls
onChange(opt.value)), and implement a visible focus treatment either via a CSS
class (e.g., add a focus-visible/focus class to the button) or by adding
onFocus/onBlur handlers to toggle a focused state and apply a distinct style
(e.g., border or box-shadow using theme.colors.accent) so keyboard users can see
which chip is focused.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1269-1271: The single-click handler handleNavigate is being
invoked immediately so a double-click always fires the click first; change the
click/double-click wiring in InboxListView so single-click navigation is delayed
and cancelled when a double-click occurs: replace the inline onClick that calls
handleNavigate(row.item) with a click handler that starts a short setTimeout to
call handleNavigate(row.item) and store its id (e.g., clickTimeoutRef), and
modify the onDoubleClick handler to clear that timeout and then call
onEnterFocus(row.item); ensure the timeout is cleared on unmount to avoid leaks.
- Around line 808-811: getHeaderFocusables currently collects header controls
via headerRef.current.querySelectorAll('button, [tabindex="0"]') and can include
non-focusable or disabled elements (causing keyboard dead-ends); update
getHeaderFocusables to only return truly focusable elements by querying for
relevant selectors and then filtering out elements that are disabled, have
tabIndex < 0 (e.g., tabindex="-1"), or are inert/aria-hidden; reference the
getHeaderFocusables function and headerRef.current.querySelectorAll call and
ensure the same filtering approach is applied to the other similar collectors
(the blocks around the other header focus logic at 817-841 and the controls
referenced at ~981-982).
---
Duplicate comments:
In `@src/renderer/App.tsx`:
- Around line 1541-1564: The current single-slot pendingInboxQuickReply state
can be overwritten and drop a second quick reply; change pendingInboxQuickReply
to a FIFO queue (array) and update its setter (setPendingInboxQuickReply) usage
to push new entries instead of replacing, then update the effect that flushes
pendingInboxQuickReply to dequeue and process entries one-by-one (calling
inboxProcessInputRef.current for each entry and restoring
previousActiveSessionId via setActiveSessionId as the original logic dictates,
using queueMicrotask when needed); apply the same queue change to the other
occurrence around the code that mirrors this logic (the block referenced at
1588-1594) so both places handle multiple pending quick replies safely.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 228-239: The icon-only toggle button using onToggleRaw and state
showRawMarkdown relies on title for accessibility; add an explicit accessible
name by adding an aria-label (e.g., aria-label={showRawMarkdown ? 'Show
formatted' : 'Show plain text'}) to the button (keep title if desired) so screen
readers receive a proper name; apply the same fix to the other icon-only buttons
in this file (the ones rendering Eye/FileText-like icons) so each button uses a
clear aria-label tied to its state or action.
---
Nitpick comments:
In `@src/renderer/stores/modalStore.ts`:
- Around line 141-147: The persisted inbox updater currently accepts
Record<string, unknown> which circumvents the AgentInboxModalData contract and
lets invalid payloads through; change the function signatures and any call sites
that take or store Record<string, unknown> (the usages around the block that
includes the Record parameter near lines 668–672) to accept AgentInboxModalData
instead, propagate that type through any helpers that read/write the modal data,
and adjust filterMode/sortMode handling to validate or narrow to the correct
enum/union if needed so the AgentInboxModalData shape is enforced end-to-end.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/renderer/App.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/types/agent-inbox.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/hooks/useAgentInbox.ts
- src/renderer/types/agent-inbox.ts
Comment on lines
+327
to
+337
| <button | ||
| key={opt.value} | ||
| onClick={() => onChange(opt.value)} | ||
| className="text-[11px] px-2.5 py-1 rounded-full cursor-pointer transition-all" | ||
| style={{ | ||
| backgroundColor: isActive ? `${theme.colors.accent}25` : 'transparent', | ||
| color: isActive ? theme.colors.accentText : theme.colors.textDim, | ||
| border: isActive ? `1px solid ${theme.colors.accent}50` : '1px solid transparent', | ||
| outline: 'none', | ||
| opacity: isActive ? 1 : 0.6, | ||
| }} |
There was a problem hiding this comment.
Don’t remove focus visibility from filter chips.
Line 335 removes outlines, but there’s no replacement focus treatment. Keyboard users can lose track of focus.
🎯 Proposed fix
<button
key={opt.value}
onClick={() => onChange(opt.value)}
+ onFocus={(e) => {
+ e.currentTarget.style.boxShadow = `0 0 0 2px ${theme.colors.accent}66`;
+ }}
+ onBlur={(e) => {
+ e.currentTarget.style.boxShadow = 'none';
+ }}
className="text-[11px] px-2.5 py-1 rounded-full cursor-pointer transition-all"
style={{
backgroundColor: isActive ? `${theme.colors.accent}25` : 'transparent',
color: isActive ? theme.colors.accentText : theme.colors.textDim,
border: isActive ? `1px solid ${theme.colors.accent}50` : '1px solid transparent',
- outline: 'none',
opacity: isActive ? 1 : 0.6,
}}
>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 327 - 337,
The code removes native focus outlines on the filter chip button (the inline
style with outline: 'none'), which breaks keyboard accessibility; restore
visible focus by removing outline: 'none', add tabIndex={0} to the button
rendering (the button keyed by opt.value that calls onChange(opt.value)), and
implement a visible focus treatment either via a CSS class (e.g., add a
focus-visible/focus class to the button) or by adding onFocus/onBlur handlers to
toggle a focused state and apply a distinct style (e.g., border or box-shadow
using theme.colors.accent) so keyboard users can see which chip is focused.
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 header tab-cycle targets to actually focusable controls.
At Line [810], the query includes disabled buttons (e.g., Line [981]), so Tab handling can prevent default without moving focus, creating a keyboard dead-end.
💡 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.matches(':disabled') && el.tabIndex >= 0
+ );
}, []);Also applies to: 817-841, 981-982
🤖 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 collects header controls via
headerRef.current.querySelectorAll('button, [tabindex="0"]') and can include
non-focusable or disabled elements (causing keyboard dead-ends); update
getHeaderFocusables to only return truly focusable elements by querying for
relevant selectors and then filtering out elements that are disabled, have
tabIndex < 0 (e.g., tabindex="-1"), or are inert/aria-hidden; reference the
getHeaderFocusables function and headerRef.current.querySelectorAll call and
ensure the same filtering approach is applied to the other similar collectors
(the blocks around the other header focus logic at 817-841 and the controls
referenced at ~981-982).
Comment on lines
+1269
to
+1271
| onClick={() => handleNavigate(row.item)} | ||
| onDoubleClick={() => onEnterFocus(row.item)} | ||
| /> |
There was a problem hiding this comment.
Double-click currently triggers navigation before Focus Mode.
At Line [1269], click always navigates immediately. A double-click fires click first, so Line [1270] usually won’t get a chance to switch to Focus Mode.
🤖 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 1269 -
1271, The single-click handler handleNavigate is being invoked immediately so a
double-click always fires the click first; change the click/double-click wiring
in InboxListView so single-click navigation is delayed and cancelled when a
double-click occurs: replace the inline onClick that calls
handleNavigate(row.item) with a click handler that starts a short setTimeout to
call handleNavigate(row.item) and store its id (e.g., clickTimeoutRef), and
modify the onDoubleClick handler to clear that timeout and then call
onEnterFocus(row.item); ensure the timeout is cleared on unmount to avoid leaks.
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
2 participants
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
Refactors the Unified Inbox to follow the Encore Features pattern, as requested in PR #379 review.
unifiedInboxflag inEncoreFeatureFlags— disabled by default, toggle in Settings > Encore tabChanges
17 commits, touching ~15 files with ~3500 lines added. All inbox functionality gated behind the
unifiedInboxEncore feature flag.Addresses feedback from @pedramamini in #379:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes
Settings