feat: Encore System, Tab Description, Unified Inbox, Fuzzy Search#476
Open
felipeggv wants to merge 47 commits intoRunMaestro:mainfrom
Open
feat: Encore System, Tab Description, Unified Inbox, Fuzzy Search#476felipeggv wants to merge 47 commits intoRunMaestro:mainfrom
felipeggv wants to merge 47 commits intoRunMaestro:mainfrom
Conversation
Added optional `description?: string` to AITab for user-defined tab context annotations. This is the data model foundation for the tab description feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extends the Encore Feature system with two new feature flags for upcoming Unified Inbox and Tab Description features. tabDescription defaults to true (enabled by default), unifiedInbox defaults to false. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add missing tests for scoring hierarchy (exact > substring > scattered), case-insensitive scored matching, special characters in query (regex metacharacters, unicode, emoji), and deterministic scoring verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Handlers Adds a new handler for updating AI tab descriptions with trim and empty-string-to-undefined normalization. Follows existing immutable state update pattern via useSessionStore. Includes 4 tests covering set, trim, empty, and whitespace-only cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add early returns in fuzzyMatch and fuzzyMatchWithScore for: - Empty text (can't match anything) - Query longer than text (impossible to match all chars) Empty query already had early return. All 5 consumers already skip fuzzy matching on empty input. No consumer changes needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gs Encore tab - Import Inbox and FileText icons from lucide-react - Change Encore tab icon from FlaskConical to Sparkles per spec - Add Unified Inbox feature card with toggle, Beta badge, left-border accent - Add Tab Descriptions feature card with toggle, left-border accent - Both cards follow existing toggle pattern (spread-copy + flip flag) - All 113 SettingsModal tests pass, lint clean Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… TabBar Thread the tab description handler from useTabHandlers through useMainPanelProps → App.tsx → MainPanel → TabBar. Gate with encoreFeatures.tabDescription flag (default: false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…es encore features Add agentInbox shortcut (Alt+I) with encoreFeatures.unifiedInbox guard. Director's Notes shortcut already had its guard. Register agentInbox modal in the modal store and wire through App.tsx keyboard handler context. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a description section to the AI tab hover overlay menu with two modes: - Display mode: FileText icon + description text (2-line clamp) or italic placeholder - Edit mode: auto-focus textarea with Enter to save, Shift+Enter for newline, Escape to cancel Feature-gated behind onUpdateTabDescription prop. Only calls handler when value actually changed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…edit Add cleanup logic so that if the overlay closes while the user is editing a tab description, the current draft is saved rather than discarded. Uses a useRef to track the latest draft value, avoiding stale closure issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n tab overlay Adds requestAnimationFrame-based focus management so that when the user saves or cancels description editing, focus returns to the description display button for keyboard accessibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
…nd unifiedInbox encore feature - Add onOpenAgentInbox gated prop (App.tsx → AppModals → QuickActionsModal) - Add AgentInbox rendering placeholder in App.tsx (commented, component TBD) - Add Unified Inbox entry to QuickActionsModal command palette - DirectorNotesModal gating was already in place from prior work Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds 2 missing useTabHandlers tests (non-active session isolation, other tabs isolation) and 7 TabBar description UI tests covering rendering, edit mode, Enter/Escape behavior, and feature gating. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…on consistency The tabDescription encore feature was defaulting to true, inconsistent with the encore pattern (all features disabled by default). No handler or UI exists yet for tab descriptions, so this is purely a default fix. 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>
…egration # Conflicts: # src/renderer/stores/settingsStore.ts # src/renderer/types/index.ts
…ration # Conflicts: # src/renderer/App.tsx # src/renderer/components/AppModals.tsx # src/renderer/components/QuickActionsModal.tsx # src/renderer/components/SettingsModal.tsx # src/renderer/hooks/keyboard/useMainKeyboardHandler.ts # src/renderer/stores/modalStore.ts # src/renderer/stores/settingsStore.ts # src/renderer/types/index.ts
Added Alt+I (Unified Inbox) and Cmd+Shift+O (Director's Notes) to the Global Shortcuts table in keyboard-shortcuts.md with footnote noting Encore Feature requirement. Also added Unified Inbox to the Available Features table in encore-features.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…zzy Search Merges 4 feature branches rebuilt from clean main baseline: - feat/rebuild-encore-system: Encore feature flags (unifiedInbox, tabDescription) - feat/rebuild-tab-description: Inline tab description editing in hover overlay - feat/rebuild-unified-inbox: Agent Inbox modal with Focus Mode, filters, keyboard nav - Fuzzy search engine with ranked scoring and fast-paths 25 commits across 4 features, all lint-clean and tested. 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 Unified Inbox (Agent Inbox) feature with list and focus UIs, a new useAgentInbox hook and types, modal/shortcut/store wiring, tab description editing (handler + UI), settings feature flags, fuzzy command/search updates, and many new tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App
participant ModalStore as ModalStore
participant AgentInbox as AgentInbox
participant InboxListView as InboxListView
participant FocusModeView as FocusModeView
participant Session as Session
User->>App: Press Unified Inbox shortcut (Alt/Opt+I)
App->>ModalStore: setAgentInboxOpen(true)
App->>AgentInbox: Mount (lazy)
AgentInbox->>AgentInbox: useAgentInbox(sessions, groups) -> items
AgentInbox->>InboxListView: Render list (filters/sort)
User->>InboxListView: Select item / Enter
InboxListView->>AgentInbox: onEnterFocus(index)
AgentInbox->>FocusModeView: Render focused item
FocusModeView->>Session: Read session.aiLogs
User->>FocusModeView: Type reply + Enter
FocusModeView->>AgentInbox: onQuickReply(sessionId, tabId, text)
AgentInbox->>App: call processInput / focus session
sequenceDiagram
participant User as User
participant TabBar as TabBar
participant Tab as Tab
participant useTabHandlers as useTabHandlers
participant Session as Session
User->>TabBar: Open tab overlay
TabBar->>Tab: Show description UI (if prop provided)
User->>Tab: Edit description and press Enter
Tab->>useTabHandlers: handleUpdateTabDescription(tabId, trimmedText)
useTabHandlers->>Session: Update active session.aiTabs.description
Session->>Tab: Re-render with updated description
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 |
Greptile SummaryIntroduces three Encore features (Unified Inbox, Tab Descriptions, Fuzzy Search) with comprehensive testing and feature flag gating. Key changes:
Architecture:
Testing: Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User presses Option+I] --> B[useMainKeyboardHandler]
B --> C[setAgentInboxOpen true]
C --> D[AgentInbox Modal Opens]
D --> E[useAgentInbox Hook]
E --> F[Filter sessions by mode]
F --> G[Sort items by mode]
G --> H[Generate smart summaries]
D --> I[InboxListView]
I --> J[Virtual scrolling]
J --> K[j/k navigation]
K --> L[Enter to open or F for Focus]
D --> M{User Action?}
M -->|Press F| N[FocusModeView]
M -->|Double-click| O[Navigate to session]
M -->|Enter| O
N --> P[Show full conversation]
P --> Q[Inline reply textarea]
Q --> R{Reply Type?}
R -->|Quick Reply| S[Send & return to previous]
R -->|Open & Reply| T[Navigate to session]
S --> U[handleAgentInboxQuickReply]
U --> V[Set pending state]
V --> W[Switch active session]
W --> X[Process input via ref]
X --> Y[Restore previous session]
D --> Z[TabBar with descriptions]
Z --> AA[Hover shows overlay]
AA --> AB[Edit description inline]
AB --> AC[Auto-save on blur]
style D fill:#e1f5ff
style N fill:#fff4e6
style Z fill:#f3e5f5
Last reviewed commit: e50f079 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (3)
src/__tests__/renderer/components/TabBar.test.tsx (1)
5841-5872: Add a regression test for empty-description normalization.This block validates trimming on save, but it doesn’t explicitly assert the empty-after-trim path (
' '->undefined), which is core behavior for this feature.🧪 Suggested test addition
+ it('pressing Enter with whitespace-only description saves undefined', () => { + const tabs = [ + createTab({ + id: 'tab-1', + name: 'Tab 1', + agentSessionId: 'session-1', + }), + ]; + + render( + <TabBar + tabs={tabs} + activeTabId="tab-1" + theme={mockThemeDesc} + onTabSelect={mockOnTabSelect} + onTabClose={mockOnTabClose} + onNewTab={mockOnNewTab} + onUpdateTabDescription={mockOnUpdateTabDescription} + /> + ); + + openOverlay('Tab 1'); + fireEvent.click(screen.getByLabelText('Add tab description')); + const textarea = screen.getByLabelText('Tab description'); + fireEvent.change(textarea, { target: { value: ' ' } }); + fireEvent.keyDown(textarea, { key: 'Enter' }); + + expect(mockOnUpdateTabDescription).toHaveBeenCalledWith('tab-1', undefined); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 5841 - 5872, The test covers trimming non-empty descriptions but misses the empty-after-trim case; add a new assertion in the same test (or an adjacent test) that opens the overlay for "Tab 1", clicks the description button, sets the textarea value to a whitespace-only string (e.g. ' '), simulates pressing Enter (same sequence using getByLabelText('Tab description') and fireEvent.keyDown), and then asserts mockOnUpdateTabDescription was called with the tab id ('tab-1') and undefined to verify empty descriptions are normalized to undefined; keep using the existing TabBar render and mockOnUpdateTabDescription setup.src/renderer/types/agent-inbox.ts (1)
1-1: NarrowtoolTypeto the existing union type.Line 10 uses
string, which allows invalid tool identifiers to flow through inbox items. PreferToolTypefor stronger type safety.Suggested fix
-import type { SessionState } from './index'; +import type { SessionState, ToolType } from './index'; @@ - toolType: string; + toolType: ToolType;Also applies to: 10-10
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/types/agent-inbox.ts` at line 1, Change the toolType property from a plain string to the existing union type ToolType to prevent invalid identifiers from being used; update the import statement to include ToolType (e.g., import type { SessionState, ToolType } from './index') and replace any occurrences/annotations of toolType: string with toolType: ToolType in the inbox item types/interfaces so the compiler enforces only valid tool identifiers.src/renderer/stores/modalStore.ts (1)
145-147: Tighten Agent Inbox modal typing end-to-end.Line 145-147 and Line 777 currently accept broad
string/Record<string, unknown>, which allows invalid filter/sort payloads to be persisted.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: 777-777
🤖 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 145 - 147, Tighten the modal typing by replacing the broad string/Record types with explicit types/enums and a typed payload interface: change filterMode?: string and sortMode?: string to concrete union types (e.g., type AgentInboxFilter = 'all' | 'unread' | 'assigned'; type AgentInboxSort = 'newest' | 'oldest' | 'priority') and replace any Record<string, unknown> modal payload with a dedicated interface (e.g., AgentInboxModalState { filterMode?: AgentInboxFilter; sortMode?: AgentInboxSort; isExpanded?: boolean; ... }). Update modalStore's modal state type, and update any setters/accessors (e.g., functions that read/write modal state such as setModal, updateModal, persistModalState or similar usages around the previous line ~777) to use the new types so only valid filter/sort values are accepted and persisted.
🤖 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/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5784-5791: The test fixtures use unnecessary "as any" casts when
calling createTab (e.g., createTab({ id: 'tab-1', name: 'Tab 1', agentSessionId:
'session-1', description: 'My existing description' } as any)); remove the "as
any" casts and pass the object directly since description is already an optional
string on the AITab interface; update any other similar occurrences in
TabBar.test.tsx (e.g., around lines where createTab is invoked with description)
to restore proper type checking without changing the fixture shape.
In `@src/renderer/App.tsx`:
- Around line 4265-4275: The AgentInbox invocation is missing the enterToSendAI
prop so quick-reply behavior won't respect user settings; update the AgentInbox
JSX (the component instance named AgentInbox) to include
enterToSendAI={enterToSendAI} (or the appropriate local variable) so the prop is
forwarded into AgentInbox (which in turn forwards it to Focus Mode) and keeps
quick-reply/Enter-to-send behavior consistent.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 751-753: The effect that clears reply text (useEffect that calls
setReplyText('') tied to [item.sessionId, item.tabId]) only empties content but
doesn't reset the textarea height; update that effect to also reset the textarea
DOM height (e.g., replyTextareaRef.current.style.height = 'auto' or '' if a ref
is used) so the input collapses when switching items. Do the same reset in the
send handler (the function around lines 1099-1104, e.g., handleSend/sendReply)
after clearing text so the textarea height is restored when a message is sent;
reference the textarea ref (replyTextareaRef or replyInputRef) and ensure you
guard for its existence before manipulating style.
- Line 595: The onMarkAsRead handler passed as onMarkAsRead/_onMarkAsRead is
never invoked by the reply flow, so replied items remain unread; update the
explicit reply handlers (the functions that send replies—e.g., the local reply
submit/handler methods used around lines ~755-770 and the handler referenced
near 786-787) to call _onMarkAsRead (or props.onMarkAsRead) after a successful
reply send: locate the reply submit functions (e.g., onReplySubmit,
handleReply/submitReply or similar) and, after awaiting the send/submit result
and confirming success, invoke _onMarkAsRead() (with a null-check or optional
chaining) so the unread state is cleared, and ensure this call is executed only
on success and does not break existing error paths.
- Around line 427-438: The sidebar renders option-like children without a
listbox container and marks collapsible headers as role="option"; update
FocusModeView so the scrollable container (the element with className "flex-1
overflow-y-auto py-1") has role="listbox" and manage focus/aria-activedescendant
for options, and change the collapsible header elements (currently using
role="option") to an appropriate semantic role such as role="group" or
role="heading" with a nested toggle as role="button" plus
aria-expanded/aria-controls; apply the same change to the other header block
around the 491-497 area so headers are not role="option" while list items remain
role="option" inside the listbox.
- Around line 387-388: The grouping currently uses groupName which can merge
distinct groups that share a display name; change the grouping key logic in
FocusModeView so groupKey prefers itm.groupId when present (falling back to
itm.groupName and then 'Ungrouped') while leaving groupName as the
human-readable label (itm.groupName ?? 'Ungrouped' or itm.sessionName). Update
the expression that defines groupKey (and any consumers that track collapse
state) to use itm.groupId ?? itm.groupName ?? 'Ungrouped' when useGroupName is
true, keeping sessionId as the fallback when useGroupName is false, so collapse
buckets are stable and unique per actual group identity.
- Around line 228-233: The toggle button in FocusModeView (the element with
onToggleRaw and using showRawMarkdown for state) is visually hidden via opacity
until hover and lacks an accessible name; update the button so it’s keyboard and
screen-reader discoverable by adding an explicit accessible label (aria-label or
visually hidden text that reflects showRawMarkdown state), ensure it remains
visible/focusable when keyboard-focused (remove or override the opacity:0 rule
for :focus or add a focus-visible style), and keep the existing title for mouse
users; this preserves onToggleRaw behavior while making the control accessible.
- Around line 988-1005: The resize handle is mouse-only; make it
keyboard-accessible by adding tabIndex={0}, appropriate ARIA (e.g.,
role="separator" and aria-orientation="vertical") and a onKeyDown handler that
implements keyboard resize (handle ArrowLeft/ArrowRight, Home/End) and reuses or
delegates to the same resize logic used by handleResizeStart; also add onFocus
and onBlur to mirror the current onMouseEnter/onMouseLeave visual feedback (use
isResizingRef and theme.colors to set backgroundColor) so keyboard focus shows
the same highlight, and ensure the element keeps cursor styling and flex
behavior.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 1269-1271: The current click handlers on the row call
handleNavigate on single-click and onEnterFocus on double-click, but
handleNavigate closes the modal so onDoubleClick never fires; choose one
interaction model and update the handlers accordingly: either make onClick a
non-navigating selection action (e.g., setSelected / highlight only) and keep
onDoubleClick calling onEnterFocus for entry (ensure handleNavigate is not
invoked on single click), or remove onDoubleClick and move the onEnterFocus
behavior into handleNavigate/onClick so a single click performs navigation and
closes the modal; update the handlers referenced (handleNavigate, onEnterFocus)
in InboxListView to match the chosen model and adjust any modal-closing logic so
double-click remains reachable if you keep it.
- Around line 1059-1074: The focusable list container (the div with
role="listbox" and handlers like onKeyDown={handleKeyDown}) is not wired to the
existing containerRef, so calls to containerRef.current?.focus() do nothing;
attach the ref by adding ref={containerRef} to that div (the element using
aria-activedescendant and onFocus/onBlur) so containerRef points to the listbox
and keyboard focus returns work as intended.
- Around line 808-811: getHeaderFocusables currently includes disabled controls,
which can cause focus to land on non-focusable elements; update the
getHeaderFocusables function to exclude disabled/aria-disabled and non-focusable
elements by changing the query and/or filtering the result (for example use
headerRef.current.querySelectorAll<HTMLElement>('button:not([disabled]):not([aria-disabled="true"]),
[tabindex="0"]:not([aria-disabled="true"])') and then further filter the
NodeList to only include elements that are not hidden and have a non-negative
tabIndex (e.g. filter by !el.hasAttribute('disabled') &&
el.getAttribute('aria-disabled') !== 'true' && el.tabIndex >= 0 && !el.hidden)
so that only truly focusable header controls are returned; apply the same
filtering logic where getHeaderFocusables is used to prevent trapping focus.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3687-3709: The icon-only switch buttons in SettingsModal (the
button controlling encoreFeatures.unifiedInbox using setEncoreFeatures and the
similar switch around the other block) have role="switch" but no accessible
name; add an accessible name by supplying either aria-label or aria-labelledby
on each button (for example aria-label="Enable unified inbox" for the
unifiedInbox switch) and ensure aria-checked remains a boolean, and mark the
decorative inner <span> as aria-hidden="true" so screen readers only announce
the labeled switch state.
In `@src/renderer/components/TabBar.tsx`:
- Around line 482-505: The blur handler handleDescriptionBlur is saving the
draft even when handleDescriptionCancel was invoked; change the flow to set a
transient "cancelled" flag (e.g., cancelledRef.current = true) inside
handleDescriptionCancel before moving focus, then in handleDescriptionBlur check
that flag and skip calling handleDescriptionSave if cancelled, and finally clear
the flag (via requestAnimationFrame or setTimeout 0) so subsequent blurs behave
normally; update references in handleDescriptionKeyDown (Escape path) to use
handleDescriptionCancel so the flag prevents the blur-save from persisting
cancelled edits.
---
Nitpick comments:
In `@src/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5841-5872: The test covers trimming non-empty descriptions but
misses the empty-after-trim case; add a new assertion in the same test (or an
adjacent test) that opens the overlay for "Tab 1", clicks the description
button, sets the textarea value to a whitespace-only string (e.g. ' '),
simulates pressing Enter (same sequence using getByLabelText('Tab description')
and fireEvent.keyDown), and then asserts mockOnUpdateTabDescription was called
with the tab id ('tab-1') and undefined to verify empty descriptions are
normalized to undefined; keep using the existing TabBar render and
mockOnUpdateTabDescription setup.
In `@src/renderer/stores/modalStore.ts`:
- Around line 145-147: Tighten the modal typing by replacing the broad
string/Record types with explicit types/enums and a typed payload interface:
change filterMode?: string and sortMode?: string to concrete union types (e.g.,
type AgentInboxFilter = 'all' | 'unread' | 'assigned'; type AgentInboxSort =
'newest' | 'oldest' | 'priority') and replace any Record<string, unknown> modal
payload with a dedicated interface (e.g., AgentInboxModalState { filterMode?:
AgentInboxFilter; sortMode?: AgentInboxSort; isExpanded?: boolean; ... }).
Update modalStore's modal state type, and update any setters/accessors (e.g.,
functions that read/write modal state such as setModal, updateModal,
persistModalState or similar usages around the previous line ~777) to use the
new types so only valid filter/sort values are accepted and persisted.
In `@src/renderer/types/agent-inbox.ts`:
- Line 1: Change the toolType property from a plain string to the existing union
type ToolType to prevent invalid identifiers from being used; update the import
statement to include ToolType (e.g., import type { SessionState, ToolType } from
'./index') and replace any occurrences/annotations of toolType: string with
toolType: ToolType in the inbox item types/interfaces so the compiler enforces
only valid tool identifiers.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
docs/encore-features.mddocs/keyboard-shortcuts.mdsrc/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/__tests__/renderer/utils/search.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/MainPanel.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/search.tssrc/renderer/utils/tabDisplayName.ts
- Fix resize handler stale closure via ref instead of sidebarWidth dep - Simplify double rAF focus to single rAF - Add runtime validation for filterMode/sortMode instead of unsafe cast - Add aria-labels to nav buttons, close button, toggle controls, switches - Call onMarkAsRead after quick-reply and open-and-reply actions - Use groupId for sidebar grouping to prevent name-collision merges - Fix sidebar ARIA: role=listbox container, role=separator for headers - Make raw/markdown toggle keyboard-focusable with focus:opacity - Exclude disabled buttons from Tab focus cycling in InboxListView - Fix TabBar description Escape cancel being overridden by blur-save - Reset textarea height on item switch in FocusModeView - Guard against concurrent quick-reply race condition in App.tsx - Pass enterToSendAI prop to AgentInbox for consistent reply behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tainerRef, line clamp)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/components/AgentInbox/InboxListView.tsx (1)
809-814: Consider filtering[tabindex="0"]elements that are disabled.The current selector
'button:not(:disabled), [tabindex="0"]'excludes disabled buttons but doesn't filter[tabindex="0"]elements that might be disabled viaaria-disabled. While this may not be an issue currently, it could lead to focus landing on non-interactable elements if such patterns are introduced later.♻️ Suggested defensive improvement
const getHeaderFocusables = useCallback((): HTMLElement[] => { if (!headerRef.current) return []; return Array.from( headerRef.current.querySelectorAll<HTMLElement>('button:not(:disabled), [tabindex="0"]') - ); + ).filter((el) => el.getAttribute('aria-disabled') !== 'true'); }, []);🤖 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 809 - 814, The getHeaderFocusables function's selector includes [tabindex="0"] but doesn't exclude elements that are aria-disabled, so focus could land on non-interactable elements; update the query in getHeaderFocusables (using headerRef) to exclude aria-disabled elements (e.g. add :not([aria-disabled="true"]) for the [tabindex="0"] part) or post-filter the NodeList to remove elements where element.getAttribute('aria-disabled') === 'true', ensuring returned HTMLElements are actionable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 809-814: The getHeaderFocusables function's selector includes
[tabindex="0"] but doesn't exclude elements that are aria-disabled, so focus
could land on non-interactable elements; update the query in getHeaderFocusables
(using headerRef) to exclude aria-disabled elements (e.g. add
:not([aria-disabled="true"]) for the [tabindex="0"] part) or post-filter the
NodeList to remove elements where element.getAttribute('aria-disabled') ===
'true', ensuring returned HTMLElements are actionable.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/InboxListView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/TabBar.tsx
…nsion Long descriptions no longer push the overlay wider. The overlay is now fixed at 220px width with word-break forcing text to wrap within bounds. The existing 3-line clamp with ellipsis handles vertical truncation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y width Reverted overlay width back to minWidth: 220px (production default). Added overflow: hidden to description container so long descriptions wrap within the overlay's natural width instead of expanding it. The 3-line clamp with word-break handles vertical truncation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g overlay width" This reverts commit 45ae1d2.
Locked overlay to exactly 220px (minWidth + maxWidth) so long descriptions cannot expand it horizontally. Added overflow-hidden to description container and word-break for proper text wrapping. The existing 3-line clamp with ellipsis handles vertical truncation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…normalization tests Addresses remaining CodeRabbit findings from PR RunMaestro#476: - agent-inbox.ts: toolType changed from `string` to `ToolType` for stronger type safety - TabBar.test.tsx: added regression tests for whitespace-only description normalization Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Collapse two-bar header (header + subheader) into single h-16 px-6 bar - Adopt app title bar pipe-separated pattern (Group | Name | Tab) - Move metadata badges (git branch, context %, status, thinking) to right side - Widen list-mode dialog from 780px to 900px - Add backdrop-filter blur(6px) to modal overlay for focus effect Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…o triage Replace numeric focusIndex with identity-based focusId tracking so that items re-entering the filtered list (e.g. agent responds → hasUnread flips back to true) no longer shift the focused item or steal textarea focus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…idebar - Embed ⌘[ and ⌘] kbd badges inside Prev/Next buttons - Simplify center hint to just "Esc Back" - Widen sidebar default from 220px to 300px, resize range 200-440px - Correct shortcut tooltips from ⌘←/→ to ⌘[/] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…400px cap Replaces the fixed expandedWidth calculation (Math.min(window.innerWidth * 0.92, 1400)) with a simple 75vw viewport-relative width. This scales naturally across all screen sizes (~1440px on 4K/1920px effective) while the existing maxWidth: 95vw CSS acts as safety net. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/renderer/types/agent-inbox.ts (1)
52-58: Simplify the type guards by removing broad string-array casts.Current casts work, but they widen intent unnecessarily. You can keep stricter typing with a narrower cast on
value.♻️ Proposed cleanup
export function isValidFilterMode(value: unknown): value is InboxFilterMode { - return typeof value === 'string' && (VALID_FILTER_MODES as readonly string[]).includes(value); + return typeof value === 'string' && VALID_FILTER_MODES.includes(value as InboxFilterMode); } export function isValidSortMode(value: unknown): value is InboxSortMode { - return typeof value === 'string' && (VALID_SORT_MODES as readonly string[]).includes(value); + return typeof value === 'string' && VALID_SORT_MODES.includes(value as InboxSortMode); }🤖 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 52 - 58, The type guards isValidFilterMode and isValidSortMode should avoid broad array casts; instead use a narrow cast on the checked value. Replace the checks that use (VALID_FILTER_MODES as readonly string[]).includes(value) and (VALID_SORT_MODES as readonly string[]).includes(value) with (VALID_FILTER_MODES as readonly InboxFilterMode[]).includes(value as InboxFilterMode) and (VALID_SORT_MODES as readonly InboxSortMode[]).includes(value as InboxSortMode) respectively (or cast only value: includes(value as InboxFilterMode / value as InboxSortMode)), so the intent is preserved and TypeScript understands the specific union types without widening the arrays to string[].src/renderer/components/AgentInbox/index.tsx (2)
113-119: Simplify the fallback index calculation.Line 118's
Math.min(items.length - 1, 0)always returns0(or-1whenitemsis empty, but that case is guarded by line 114). The expression is correct but unnecessarily complex.♻️ Simplify fallback to index 0
const safeFocusIndex = useMemo(() => { if (!focusId || items.length === 0) return 0; const idx = items.findIndex( (i) => i.sessionId === focusId.sessionId && i.tabId === focusId.tabId ); - return idx >= 0 ? idx : Math.min(items.length - 1, 0); + return idx >= 0 ? idx : 0; }, [focusId, items]);🤖 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 113 - 119, The fallback calculation in the safeFocusIndex useMemo is overly complex: when items is non-empty the expression Math.min(items.length - 1, 0) always yields 0; update the return so that after computing idx you return idx >= 0 ? idx : 0 instead (modify the safeFocusIndex useMemo that references focusId and items).
314-315: Remove empty event handlers.The
onFocusandonBlurhandlers are empty no-ops. If they're not needed, removing them improves readability.♻️ Remove empty handlers
onKeyDownCapture={handleCaptureKeyDown} onKeyDown={handleShellKeyDown} - onFocus={() => {}} - onBlur={() => {}} >🤖 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 314 - 315, Remove the no-op onFocus and onBlur props from the JSX in the AgentInbox component—delete the empty arrow handlers (onFocus={() => {}} and onBlur={() => {}}) in src/renderer/components/AgentInbox/index.tsx so the markup is cleaner; before removing, quickly verify there are no external expectations for those handlers (e.g., passed-in props or tests referencing AgentInbox.onFocus/onBlur) and, if any logic is required, replace the empty handlers with the intended function references.src/renderer/components/AgentInbox/FocusModeView.tsx (3)
869-905: Consider addingaria-pressedfor the thinking toggle.The thinking toggle button cycles through three states but doesn't announce the current state to screen readers beyond the title tooltip.
♻️ Optional: Add aria-pressed for better screen reader support
<button onClick={cycleThinking} + aria-pressed={showThinking !== 'off'} + aria-label={`Thinking: ${showThinking}`} className={`flex items-center gap-1 text-[10px] px-2 py-1 rounded-full cursor-pointer transition-all ${🤖 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 869 - 905, The thinking toggle button (in FocusModeView, using showThinking and cycleThinking) should expose its current state to assistive tech by adding aria-pressed; set aria-pressed to true when showThinking is 'on' or 'sticky' and false when 'off' (e.g., aria-pressed={showThinking !== 'off'}) so screen readers know whether the toggle is active, keeping the existing title and onClick behavior unchanged.
204-206: Consider logging clipboard failures for debugging.The empty catch block silently swallows clipboard errors. While this prevents UI disruption, it may complicate debugging clipboard issues in certain environments.
♻️ Optional: Add minimal error logging
const handleCopy = (text: string) => { - navigator.clipboard.writeText(text).catch(() => {}); + navigator.clipboard.writeText(text).catch((err) => { + console.debug('Clipboard write failed:', err); + }); };🤖 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 204 - 206, The empty catch in handleCopy swallowing clipboard errors makes debugging hard; update the handleCopy function (the navigator.clipboard.writeText(...) call) to log errors in the catch handler—e.g., call console.error (or the app logger if available) with a descriptive message, the text attempted to copy, and the caught error so failures are visible for troubleshooting.
1133-1166: Consider adding focus styling for nav buttons.The Prev/Next buttons have hover effects via
onMouseEnter/onMouseLeavebut lack corresponding focus styling. Keyboard users won't see the visual feedback that mouse users get.♻️ Optional: Add focus styling matching hover
onMouseLeave={(e) => { e.currentTarget.style.backgroundColor = 'transparent'; }} + onFocus={(e) => { + if (items.length > 1) + e.currentTarget.style.backgroundColor = `${theme.colors.accent}10`; + }} + onBlur={(e) => { + e.currentTarget.style.backgroundColor = 'transparent'; + }} title="Previous item (⌘[)"🤖 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 1133 - 1166, The Prev/Next navigation buttons in FocusModeView lack keyboard focus styling, so add focus handling that mirrors the hover effect: for the button elements (the ones calling onNavigateItem and rendering ChevronLeft/ChevronRight), apply the same visual change used onMouseEnter (backgroundColor = `${theme.colors.accent}10`) when the element receives keyboard focus and revert it on blur; ensure this only happens when items.length > 1 (respect disabled/aria-disabled), and also consider adding a visible outline or box-shadow on focus for stronger accessibility. Use onFocus/onBlur handlers (or a focus-visible CSS class) on the same buttons to implement this consistently for both Prev and Next.
🤖 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/TabBar.tsx`:
- Line 775: The inline callback ref ref={(el) => el?.focus()} on the textarea in
TabBar causes focus to be called on every render/keystroke; replace it with a
persistent ref created via useRef<HTMLTextAreaElement | null>() (e.g., const
editTextRef = useRef(null)) and attach it to the textarea (ref={editTextRef}),
then use useEffect that watches the edit mode flag (e.g., isEditing or editMode)
to call editTextRef.current?.focus() only when entering edit mode; remove the
inline callback ref to avoid repeatedly resetting focus/caret during onChange
updates.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 869-905: The thinking toggle button (in FocusModeView, using
showThinking and cycleThinking) should expose its current state to assistive
tech by adding aria-pressed; set aria-pressed to true when showThinking is 'on'
or 'sticky' and false when 'off' (e.g., aria-pressed={showThinking !== 'off'})
so screen readers know whether the toggle is active, keeping the existing title
and onClick behavior unchanged.
- Around line 204-206: The empty catch in handleCopy swallowing clipboard errors
makes debugging hard; update the handleCopy function (the
navigator.clipboard.writeText(...) call) to log errors in the catch
handler—e.g., call console.error (or the app logger if available) with a
descriptive message, the text attempted to copy, and the caught error so
failures are visible for troubleshooting.
- Around line 1133-1166: The Prev/Next navigation buttons in FocusModeView lack
keyboard focus styling, so add focus handling that mirrors the hover effect: for
the button elements (the ones calling onNavigateItem and rendering
ChevronLeft/ChevronRight), apply the same visual change used onMouseEnter
(backgroundColor = `${theme.colors.accent}10`) when the element receives
keyboard focus and revert it on blur; ensure this only happens when items.length
> 1 (respect disabled/aria-disabled), and also consider adding a visible outline
or box-shadow on focus for stronger accessibility. Use onFocus/onBlur handlers
(or a focus-visible CSS class) on the same buttons to implement this
consistently for both Prev and Next.
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 113-119: The fallback calculation in the safeFocusIndex useMemo is
overly complex: when items is non-empty the expression Math.min(items.length -
1, 0) always yields 0; update the return so that after computing idx you return
idx >= 0 ? idx : 0 instead (modify the safeFocusIndex useMemo that references
focusId and items).
- Around line 314-315: Remove the no-op onFocus and onBlur props from the JSX in
the AgentInbox component—delete the empty arrow handlers (onFocus={() => {}} and
onBlur={() => {}}) in src/renderer/components/AgentInbox/index.tsx so the markup
is cleaner; before removing, quickly verify there are no external expectations
for those handlers (e.g., passed-in props or tests referencing
AgentInbox.onFocus/onBlur) and, if any logic is required, replace the empty
handlers with the intended function references.
In `@src/renderer/types/agent-inbox.ts`:
- Around line 52-58: The type guards isValidFilterMode and isValidSortMode
should avoid broad array casts; instead use a narrow cast on the checked value.
Replace the checks that use (VALID_FILTER_MODES as readonly
string[]).includes(value) and (VALID_SORT_MODES as readonly
string[]).includes(value) with (VALID_FILTER_MODES as readonly
InboxFilterMode[]).includes(value as InboxFilterMode) and (VALID_SORT_MODES as
readonly InboxSortMode[]).includes(value as InboxSortMode) respectively (or cast
only value: includes(value as InboxFilterMode / value as InboxSortMode)), so the
intent is preserved and TypeScript understands the specific union types without
widening the arrays to string[].
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/__tests__/renderer/components/TabBar.test.tsxsrc/renderer/components/AgentInbox/FocusModeView.tsxsrc/renderer/components/AgentInbox/index.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/types/agent-inbox.ts
src/renderer/components/TabBar.tsx
Outdated
| > | ||
| {isEditingDescription ? ( | ||
| <textarea | ||
| ref={(el) => el?.focus()} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's view the file around line 775 to understand the context
sed -n '700,830p' src/renderer/components/TabBar.tsx | cat -nRepository: RunMaestro/Maestro
Length of output: 5101
🏁 Script executed:
# Also check the state variables and hooks being used
sed -n '230,250p' src/renderer/components/TabBar.tsx | cat -nRepository: RunMaestro/Maestro
Length of output: 991
🏁 Script executed:
# Check if there are any other ref-related or focus-related code patterns
rg -n "ref.*focus|useRef|focus\(\)" src/renderer/components/TabBar.tsx -A 2 -B 2Repository: RunMaestro/Maestro
Length of output: 2365
🏁 Script executed:
# Search for useEffect blocks and see which ones involve isEditingDescription
rg -n "useEffect\|isEditingDescription" src/renderer/components/TabBar.tsx | head -40Repository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
# Look at the keydown and blur handlers for description
sed -n '500,560p' src/renderer/components/TabBar.tsx | cat -nRepository: RunMaestro/Maestro
Length of output: 2156
Use useRef and useEffect to focus the textarea only when entering edit mode, not on every keystroke.
The inline callback ref at line 775 (ref={(el) => el?.focus()}) is recreated on each render. Since the onChange handler updates state on every keystroke, this causes focus to be called repeatedly during typing, potentially interrupting user input and resetting the caret position.
♻️ Proposed fix
const descriptionButtonRef = useRef<HTMLButtonElement>(null);
+ const descriptionTextareaRef = useRef<HTMLTextAreaElement>(null);
const skipBlurSaveRef = useRef(false);
const handleDescriptionSave = useCallback( // Sync draft with tab.description when it changes externally
useEffect(() => {
if (!isEditingDescription) {
setDescriptionDraft(tab.description ?? '');
}
}, [tab.description, isEditingDescription]);
+ // Focus textarea once when entering edit mode
+ useEffect(() => {
+ if (isEditingDescription) {
+ descriptionTextareaRef.current?.focus();
+ }
+ }, [isEditingDescription]);
// Keep ref in sync with latest draft value to avoid stale closures in cleanup
useEffect(() => {
descriptionDraftRef.current = descriptionDraft;
}, [descriptionDraft]); <textarea
- ref={(el) => el?.focus()}
+ ref={descriptionTextareaRef}
value={descriptionDraft}
onChange={(e) => setDescriptionDraft(e.target.value)}
onKeyDown={handleDescriptionKeyDown}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/TabBar.tsx` at line 775, The inline callback ref
ref={(el) => el?.focus()} on the textarea in TabBar causes focus to be called on
every render/keystroke; replace it with a persistent ref created via
useRef<HTMLTextAreaElement | null>() (e.g., const editTextRef = useRef(null))
and attach it to the textarea (ref={editTextRef}), then use useEffect that
watches the edit mode flag (e.g., isEditing or editMode) to call
editTextRef.current?.focus() only when entering edit mode; remove the inline
callback ref to avoid repeatedly resetting focus/caret during onChange updates.
Slash command filtering used startsWith() while the rest of the app (File Search, Tab Switcher, @mentions) used fuzzyMatchWithScore. Now typing partial/out-of-order characters like /cmpct finds /compact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Gated by encoreFeatures.unifiedInbox flag, with Inbox icon, shortcut badge, and setAgentInboxOpen handler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/renderer/components/TabBar.tsx (1)
775-775:⚠️ Potential issue | 🟠 MajorAvoid focusing the textarea via an inline callback ref.
ref={(el) => el?.focus()}is recreated every render; while typing, this can repeatedly refocus and disturb caret behavior.Run this to verify the pattern is still present:
#!/bin/bash rg -n "ref=\\{\\(el\\) => el\\?\\.focus\\(\\)\\}" src/renderer/components/TabBar.tsx -C 2♻️ Proposed fix
const descriptionButtonRef = useRef<HTMLButtonElement>(null); + const descriptionTextareaRef = useRef<HTMLTextAreaElement>(null); const skipBlurSaveRef = useRef(false); + useEffect(() => { + if (isEditingDescription) { + descriptionTextareaRef.current?.focus(); + } + }, [isEditingDescription]); + // Sync draft with tab.description when it changes externally useEffect(() => { if (!isEditingDescription) { setDescriptionDraft(tab.description ?? ''); @@ {isEditingDescription ? ( <textarea - ref={(el) => el?.focus()} + ref={descriptionTextareaRef} value={descriptionDraft} onChange={(e) => setDescriptionDraft(e.target.value)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` at line 775, The inline callback ref ref={(el) => el?.focus()} in the TabBar component causes a new function each render and repeatedly refocuses the textarea; replace it with a stable ref and effect or a memoized callback: create a textareaRef via useRef<HTMLTextAreaElement | null>(null) (or a useCallback setTextareaRef) and assign that to the textarea's ref, then call textareaRef.current.focus() inside a useEffect that runs only when the desired focus condition changes (or on mount), so caret/focus isn't disturbed during typing. Ensure you update the textarea ref usage in TabBar.tsx (the textarea element) and remove the inline arrow ref.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
752-760:⚠️ Potential issue | 🟡 MinorReset textarea height after quick-reply send.
After clearing
replyText, the textarea can remain visually expanded from prior auto-resize.🔧 Proposed fix
const handleQuickReply = useCallback(() => { const text = replyText.trim(); if (!text || !onQuickReply) return; onQuickReply(item.sessionId, item.tabId, text); onMarkAsRead?.(item.sessionId, item.tabId); setReplyText(''); + if (replyInputRef.current) { + replyInputRef.current.style.height = 'auto'; + } }, [replyText, item, onQuickReply, onMarkAsRead]);Also applies to: 1081-1086
🤖 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 752 - 760, handleQuickReply clears replyText but doesn't reset the textarea's auto-resized height, leaving it visually expanded; update handleQuickReply (and the analogous handler at 1081-1086) to reset the textarea height after setReplyText('') by invoking the existing auto-resize helper (e.g., adjustTextareaHeight/autoResizeTextarea) or, if no helper exists, set the textarea ref's style height back to its default (via textareaRef.current.style.height = 'auto' or '') to collapse the field; reference the handleQuickReply function, replyText state and setReplyText call when making this change.
🧹 Nitpick comments (3)
src/renderer/hooks/tabs/useTabHandlers.ts (1)
970-984: Avoid no-op session writes in description updates.Line 973 currently rebuilds active-session state even when nothing changes. Add a guard for “tab not found” or unchanged normalized value to reduce unnecessary re-renders.
♻️ Proposed refactor
const handleUpdateTabDescription = useCallback((tabId: string, description: string) => { - const trimmed = description.trim(); + const normalized = description.trim() || undefined; const { setSessions, activeSessionId } = useSessionStore.getState(); setSessions((prev: Session[]) => prev.map((s) => { if (s.id !== activeSessionId) return s; + const tabIndex = s.aiTabs.findIndex((tab) => tab.id === tabId); + if (tabIndex === -1) return s; + if (s.aiTabs[tabIndex].description === normalized) return s; return { ...s, aiTabs: s.aiTabs.map((tab) => - tab.id === tabId ? { ...tab, description: trimmed || undefined } : tab + tab.id === tabId ? { ...tab, description: normalized } : tab ), }; }) ); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/tabs/useTabHandlers.ts` around lines 970 - 984, handleUpdateTabDescription currently always calls setSessions which rebuilds the active session even if the description is unchanged or the tab isn't found; modify the function to early-return instead of calling setSessions when no change is needed. Specifically, inside useTabHandlers.handleUpdateTabDescription (using useSessionStore.getState(), setSessions and activeSessionId), read the active session and locate the aiTabs entry with matching tabId, compute the normalized value (trimmed or undefined), and if the tab is missing or the normalized value equals the existing tab.description, do nothing; only call setSessions to map/update sessions when the description actually changes.src/__tests__/renderer/components/TabBar.test.tsx (1)
5657-5691: Reduce duplicated local test fixture/setup in this suite.
mockThemeDescand the timer/clipboard setup are redefined here despite existing equivalents in the file. Reusing shared fixtures/helpers will reduce maintenance drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 5657 - 5691, Replace the duplicated local test fixture and setup by reusing the existing shared fixtures/helpers: remove the local mockThemeDesc and the beforeEach/afterEach setup in this test and reference the shared theme fixture (mockThemeDesc or sharedTheme) and common setup helper (e.g., setupTestEnvironment or global beforeEach in the file) instead; ensure the tests still call Element.prototype.scrollTo and Element.prototype.scrollIntoView mocks and navigator.clipboard.writeText mock from the shared setup so TabBar.test.tsx no longer redeclares mockThemeDesc, the vi.useFakeTimers()/vi.useRealTimers() blocks, or the clipboard/element prototype mocks but uses the central definitions and teardown.src/renderer/utils/tabDisplayName.ts (1)
13-15: Handle whitespace-only names to avoid blank tab labels.If
tab.nameis' ', this currently returns an empty-looking title. Trim before the name check so fallback naming still works.♻️ Proposed tweak
export function getTabDisplayName(tab: Pick<AITab, 'name' | 'agentSessionId'>): string { - if (tab.name) { - return tab.name; + const normalizedName = tab.name?.trim(); + if (normalizedName) { + return normalizedName; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/tabDisplayName.ts` around lines 13 - 15, The check that returns tab.name should ignore whitespace-only names — trim tab.name before testing so a string like " " falls back to the default; in the tabDisplayName utility (where you reference tab.name) call .trim() (or store trimmedName) and use that trimmed value for the truthy check and return so fallback naming still applies for whitespace-only titles.
🤖 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 992-999: The reply composer remains interactive even when
sessionExists is false; update FocusModeView to fully disable the composer and
actions whenever sessionExists is false by (1) rendering the non-interactive
placeholder instead of the interactive ReplyComposer/textarea and action
buttons, or by passing disabled props to the ReplyComposer, textarea and
buttons, (2) short-circuiting submit/handleSubmit, onChange and send handlers to
no-op when sessionExists is false, and (3) ensuring any keyboard shortcuts or
form submit handlers in FocusModeView are ignored when sessionExists is false so
the UI shows "Agent no longer available" and is not actionable.
- Around line 752-769: The handlers handleQuickReply and handleOpenAndReply
currently call onMarkAsRead regardless of whether the reply action actually ran;
change them so onMarkAsRead(item.sessionId, item.tabId) is only invoked after
the corresponding action has executed (i.e., inside the conditional that checks
onQuickReply or onOpenAndReply and after calling it) and keep the existing
reply-text clearing (setReplyText('')) only for the quick-reply path that
succeeded; update handleQuickReply, handleOpenAndReply to return early when no
handler exists so unread state isn't cleared erroneously.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 63-64: The grouping key derivation is inconsistent: buildRows uses
item.groupId ?? item.groupName ?? 'Ungrouped' but other places use only
groupName, causing header toggle state to drift from item visibility. Replace
the disparate uses with a single shared derived key (e.g., compute const
groupKey = sortMode === 'byAgent' ? item.sessionId : (item.groupId ??
item.groupName ?? 'Ungrouped')) and use that groupKey everywhere in this
component — in buildRows, in the collapse/filtering logic, and in the item
click/toggle handler that toggles group visibility — so header collapse state
and item visibility always reference the exact same identifier.
In `@src/renderer/hooks/input/useInputKeyDown.ts`:
- Around line 209-213: The key handler in useInputKeyDown.ts currently builds
filteredCommands from allSlashCommands using fuzzyMatch and mode checks, but the
UI highlights a separately computed score-ranked list, causing mismatches;
change the handler to compute the same score-ranked list the renderer uses
(apply the same scoring/fuzzy function and mode filters to allSlashCommands, map
each cmd to {cmd, score}, filter out non-matches, sort by score descending) and
then use that ranked list for Enter/Tab resolution (and for any index-based
selection), referencing symbols: allSlashCommands, inputValue, isTerminalMode,
fuzzyMatch/score function, and filteredCommands so both input handler and UI use
identical ranked output.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 752-760: handleQuickReply clears replyText but doesn't reset the
textarea's auto-resized height, leaving it visually expanded; update
handleQuickReply (and the analogous handler at 1081-1086) to reset the textarea
height after setReplyText('') by invoking the existing auto-resize helper (e.g.,
adjustTextareaHeight/autoResizeTextarea) or, if no helper exists, set the
textarea ref's style height back to its default (via
textareaRef.current.style.height = 'auto' or '') to collapse the field;
reference the handleQuickReply function, replyText state and setReplyText call
when making this change.
In `@src/renderer/components/TabBar.tsx`:
- Line 775: The inline callback ref ref={(el) => el?.focus()} in the TabBar
component causes a new function each render and repeatedly refocuses the
textarea; replace it with a stable ref and effect or a memoized callback: create
a textareaRef via useRef<HTMLTextAreaElement | null>(null) (or a useCallback
setTextareaRef) and assign that to the textarea's ref, then call
textareaRef.current.focus() inside a useEffect that runs only when the desired
focus condition changes (or on mount), so caret/focus isn't disturbed during
typing. Ensure you update the textarea ref usage in TabBar.tsx (the textarea
element) and remove the inline arrow ref.
---
Nitpick comments:
In `@src/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5657-5691: Replace the duplicated local test fixture and setup by
reusing the existing shared fixtures/helpers: remove the local mockThemeDesc and
the beforeEach/afterEach setup in this test and reference the shared theme
fixture (mockThemeDesc or sharedTheme) and common setup helper (e.g.,
setupTestEnvironment or global beforeEach in the file) instead; ensure the tests
still call Element.prototype.scrollTo and Element.prototype.scrollIntoView mocks
and navigator.clipboard.writeText mock from the shared setup so TabBar.test.tsx
no longer redeclares mockThemeDesc, the vi.useFakeTimers()/vi.useRealTimers()
blocks, or the clipboard/element prototype mocks but uses the central
definitions and teardown.
In `@src/renderer/hooks/tabs/useTabHandlers.ts`:
- Around line 970-984: handleUpdateTabDescription currently always calls
setSessions which rebuilds the active session even if the description is
unchanged or the tab isn't found; modify the function to early-return instead of
calling setSessions when no change is needed. Specifically, inside
useTabHandlers.handleUpdateTabDescription (using useSessionStore.getState(),
setSessions and activeSessionId), read the active session and locate the aiTabs
entry with matching tabId, compute the normalized value (trimmed or undefined),
and if the tab is missing or the normalized value equals the existing
tab.description, do nothing; only call setSessions to map/update sessions when
the description actually changes.
In `@src/renderer/utils/tabDisplayName.ts`:
- Around line 13-15: The check that returns tab.name should ignore
whitespace-only names — trim tab.name before testing so a string like " "
falls back to the default; in the tabDisplayName utility (where you reference
tab.name) call .trim() (or store trimmedName) and use that trimmed value for the
truthy check and return so fallback naming still applies for whitespace-only
titles.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
docs/encore-features.mddocs/keyboard-shortcuts.mdsrc/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/components/InputArea.test.tsxsrc/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/hooks/useInputKeyDown.test.tssrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/__tests__/renderer/utils/search.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/InputArea.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/input/useInputKeyDown.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/search.tssrc/renderer/utils/tabDisplayName.ts
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
1 similar comment
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/renderer/components/TabBar.tsx (1)
774-776:⚠️ Potential issue | 🟠 MajorReplace inline focus callback ref to avoid repeated refocus on every rerender.
ref={(el) => el?.focus()}re-runs during typing rerenders, which can reset caret position and interrupt edits.♻️ Proposed fix
- const descriptionButtonRef = useRef<HTMLButtonElement>(null); + const descriptionButtonRef = useRef<HTMLButtonElement>(null); + const descriptionTextareaRef = useRef<HTMLTextAreaElement>(null); const skipBlurSaveRef = useRef(false);+ useEffect(() => { + if (isEditingDescription) { + descriptionTextareaRef.current?.focus(); + } + }, [isEditingDescription]);- <textarea - ref={(el) => el?.focus()} + <textarea + ref={descriptionTextareaRef}#!/bin/bash # Verify the inline callback ref focus pattern still exists in TabBar rg -n "ref=\{\(el\)\s*=>\s*el\?\.focus\(\)\}" src/renderer/components/TabBar.tsx -C 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` around lines 774 - 776, The inline ref on the textarea (ref={(el) => el?.focus()}) in TabBar.tsx causes repeated refocusing during rerenders and resets the caret; replace this pattern by creating a stable ref (e.g., descriptionRef via useRef) and use an effect or a specific lifecycle/event handler to call descriptionRef.current.focus() only when you need to set initial focus (such as on mount or when entering edit mode), and then assign ref={descriptionRef} to the textarea to prevent caret jumps while typing.src/renderer/components/AgentInbox/FocusModeView.tsx (1)
752-758:⚠️ Potential issue | 🟡 MinorReset composer height after successful quick reply.
Line 757 clears text, but the auto-resized textarea height can stay expanded after send.
♻️ Suggested fix
const handleQuickReply = useCallback(() => { const text = replyText.trim(); if (!text || !onQuickReply) return; onQuickReply(item.sessionId, item.tabId, text); onMarkAsRead?.(item.sessionId, item.tabId); setReplyText(''); + if (replyInputRef.current) { + replyInputRef.current.style.height = 'auto'; + } }, [replyText, item, onQuickReply, onMarkAsRead]);🤖 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 752 - 758, When clearing the composer in handleQuickReply, also reset the auto-resized textarea height so the composer collapses back to its default size: after setReplyText('') call the composer textarea ref (e.g. replyTextareaRef or composerRef) and set its style.height to '' or 'auto' (or call the existing adjust/reset function if one exists), and add that ref/function to the handleQuickReply dependency array so the linter doesn't complain.
🧹 Nitpick comments (3)
src/__tests__/renderer/components/TabBar.test.tsx (1)
5649-5978: LGTM — comprehensive test coverage for the new tab description feature.The test suite provides solid coverage for:
- Conditional rendering based on
onUpdateTabDescriptionprop- Placeholder vs. existing description display
- Edit mode entry/exit (click, Enter, Escape)
- Input trimming and whitespace handling
One optional refinement:
mockThemeDesc(lines 5657-5675) is identical tomockTheme(lines 117-135). Consider reusing the existing mock to reduce duplication.,
♻️ Optional: Reuse existing mockTheme
describe('TabBar description section', () => { const mockOnTabSelect = vi.fn(); const mockOnTabClose = vi.fn(); const mockOnNewTab = vi.fn(); const mockOnUpdateTabDescription = vi.fn(); const mockOnTabStar = vi.fn(); const mockOnRequestRename = vi.fn(); - - const mockThemeDesc: Theme = { - id: 'test-theme', - name: 'Test Theme', - mode: 'dark', - colors: { - bgMain: '#1a1a1a', - bgSidebar: '#2a2a2a', - bgActivity: '#3a3a3a', - textMain: '#ffffff', - textDim: '#888888', - accent: '#007acc', - border: '#444444', - error: '#ff4444', - success: '#44ff44', - warning: '#ffaa00', - vibe: '#ff00ff', - agentStatus: '#00ff00', - }, - }; + // Reuse mockTheme from outer scopeThen replace all
mockThemeDescreferences withmockThemein the tests below.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 5649 - 5978, Duplicate theme mock: mockThemeDesc duplicates mockTheme; replace usages of mockThemeDesc with the existing mockTheme to remove duplication. Locate the mock definition named mockThemeDesc in the TabBar.test.tsx tests and change the tests to pass mockTheme (the already defined Theme mock) to TabBar in the describe('TabBar description section') cases; remove or delete the mockThemeDesc declaration if no longer referenced. Ensure calls that reference mockThemeDesc (in render(...) prop theme={mockThemeDesc}) are updated to theme={mockTheme} and run tests to confirm no other references remain.src/__tests__/renderer/hooks/useAgentInbox.test.ts (1)
16-67: Test factories duplicate those in AgentInbox.test.tsx.The
makeTab,makeSession, andmakeGroupfactories are nearly identical to those inAgentInbox.test.tsx. Consider extracting these to a shared test utilities file (e.g.,src/__tests__/renderer/factories/) to maintain consistency and reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/hooks/useAgentInbox.test.ts` around lines 16 - 67, The test file duplicates factory functions makeTab, makeSession, and makeGroup; extract these factories into a shared test utilities module (e.g., a new src/__tests__/renderer/factories/ file) and import them into both useAgentInbox.test.ts and AgentInbox.test.tsx, updating references to use the shared exports (makeTab, makeSession, makeGroup) so both tests reuse the single canonical implementations and remove the duplicated definitions from the test files.src/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsx (1)
111-139: Consider extracting session factory to a shared test utility.The
makeSessionfactory usesas unknown as Sessiontype assertion. While acceptable for testing, this pattern is duplicated inuseAgentInbox.test.ts(lines 31-59). Consider extracting shared test factories to a common test utility file to reduce duplication and ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsx` around lines 111 - 139, The makeSession test factory is duplicated (see makeSession in AgentInbox.test.tsx and similar factory in useAgentInbox.test.ts) and uses the ad-hoc "as unknown as Session" assertion; extract this factory into a shared test utility (e.g., tests/utils/testFactories.ts) and export makeSession so both AgentInbox.test.tsx and useAgentInbox.test.ts can import it; update the tests to import makeSession from the new module and remove the local duplicate factories, keeping the same shape and keeping the "as unknown as Session" cast only in the shared factory.
🤖 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 1086-1111: The Quick Reply and Open & Reply buttons currently only
check sessionExists and replyText but not whether their handlers exist, so they
can appear enabled but do nothing; update the disabled checks and UI state for
the button using the handler presence (handleQuickReply relies on onQuickReply,
handleOpenAndReply relies on onOpenAndReply) by including a guard like
(!onQuickReply || !sessionExists || !replyText.trim()) and (!onOpenAndReply ||
!sessionExists || !replyText.trim()) respectively, and propagate that same
condition to cursor, opacity, title, and any other interactive styling so the
buttons are truly disabled when the callbacks are not provided.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 830-845: The Shift+Tab handling traps when focusIdx === -1 because
the shift branch always focuses the container; update the keyboard handler in
InboxListView (the block using focusIdx, focusables, containerRef) so that when
e.shiftKey and focusIdx === -1 you move focus to the last focusable item
(focusables[focusables.length - 1].focus()) instead of re-focusing
containerRef.current; ensure you still call e.preventDefault() and preserve
existing behaviors for other focusIdx cases.
In `@src/renderer/stores/modalStore.ts`:
- Around line 774-777: The Agent Inbox's persisted view state is being cleared
because setAgentInboxOpen(false) calls closeModal('agentInbox'), and closeModal
clears modal data; to fix, avoid clearing data for the 'agentInbox' modal by
either (A) adding an optional flag to closeModal (e.g., closeModal(id, {
preserveData: true })) and calling closeModal('agentInbox', { preserveData: true
}) from setAgentInboxOpen, or (B) add a new helper like
closeModalPreserveData('agentInbox') that performs the same close flow but does
not clear data; update setAgentInboxOpen to use the new behavior and ensure
updateModalData('agentInbox', ...) remains unchanged so filters/sort/expanded
state persist across closes/opens.
In `@src/renderer/utils/tabDisplayName.ts`:
- Around line 13-15: Normalize tab.name by trimming whitespace before returning:
instead of returning tab.name blindly, compute a trimmedName = tab.name?.trim()
and if trimmedName is non-empty return it, otherwise fall back to the existing
ID/default logic (use tab.id or whatever default the current function uses).
Update the code in the function that reads tab.name (the tab.name check in
tabDisplayName.ts) to use trimmedName so whitespace-only names like " " do not
produce blank-looking titles.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 752-758: When clearing the composer in handleQuickReply, also
reset the auto-resized textarea height so the composer collapses back to its
default size: after setReplyText('') call the composer textarea ref (e.g.
replyTextareaRef or composerRef) and set its style.height to '' or 'auto' (or
call the existing adjust/reset function if one exists), and add that
ref/function to the handleQuickReply dependency array so the linter doesn't
complain.
In `@src/renderer/components/TabBar.tsx`:
- Around line 774-776: The inline ref on the textarea (ref={(el) =>
el?.focus()}) in TabBar.tsx causes repeated refocusing during rerenders and
resets the caret; replace this pattern by creating a stable ref (e.g.,
descriptionRef via useRef) and use an effect or a specific lifecycle/event
handler to call descriptionRef.current.focus() only when you need to set initial
focus (such as on mount or when entering edit mode), and then assign
ref={descriptionRef} to the textarea to prevent caret jumps while typing.
---
Nitpick comments:
In `@src/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsx`:
- Around line 111-139: The makeSession test factory is duplicated (see
makeSession in AgentInbox.test.tsx and similar factory in useAgentInbox.test.ts)
and uses the ad-hoc "as unknown as Session" assertion; extract this factory into
a shared test utility (e.g., tests/utils/testFactories.ts) and export
makeSession so both AgentInbox.test.tsx and useAgentInbox.test.ts can import it;
update the tests to import makeSession from the new module and remove the local
duplicate factories, keeping the same shape and keeping the "as unknown as
Session" cast only in the shared factory.
In `@src/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5649-5978: Duplicate theme mock: mockThemeDesc duplicates
mockTheme; replace usages of mockThemeDesc with the existing mockTheme to remove
duplication. Locate the mock definition named mockThemeDesc in the
TabBar.test.tsx tests and change the tests to pass mockTheme (the already
defined Theme mock) to TabBar in the describe('TabBar description section')
cases; remove or delete the mockThemeDesc declaration if no longer referenced.
Ensure calls that reference mockThemeDesc (in render(...) prop
theme={mockThemeDesc}) are updated to theme={mockTheme} and run tests to confirm
no other references remain.
In `@src/__tests__/renderer/hooks/useAgentInbox.test.ts`:
- Around line 16-67: The test file duplicates factory functions makeTab,
makeSession, and makeGroup; extract these factories into a shared test utilities
module (e.g., a new src/__tests__/renderer/factories/ file) and import them into
both useAgentInbox.test.ts and AgentInbox.test.tsx, updating references to use
the shared exports (makeTab, makeSession, makeGroup) so both tests reuse the
single canonical implementations and remove the duplicated definitions from the
test files.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
docs/encore-features.mddocs/keyboard-shortcuts.mdsrc/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/components/InputArea.test.tsxsrc/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/hooks/useInputKeyDown.test.tssrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/__tests__/renderer/utils/search.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/InputArea.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/input/useInputKeyDown.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/search.tssrc/renderer/utils/tabDisplayName.ts
Comment on lines
+1086
to
+1111
| <button | ||
| onClick={handleQuickReply} | ||
| disabled={!sessionExists || !replyText.trim()} | ||
| className="p-2 rounded-lg transition-colors flex-shrink-0" | ||
| style={{ | ||
| backgroundColor: replyText.trim() | ||
| ? theme.colors.accent | ||
| : `${theme.colors.accent}30`, | ||
| color: replyText.trim() ? theme.colors.accentForeground : theme.colors.textDim, | ||
| cursor: replyText.trim() ? 'pointer' : 'default', | ||
| }} | ||
| title={enterToSendAI ? 'Quick reply (Enter)' : 'Quick reply (⌘Enter)'} | ||
| > | ||
| <ArrowUp className="w-4 h-4" /> | ||
| </button> | ||
| {/* Open & Reply button (secondary) */} | ||
| <button | ||
| onClick={handleOpenAndReply} | ||
| disabled={!sessionExists || !replyText.trim()} | ||
| className="p-1.5 rounded-lg transition-colors flex-shrink-0 text-xs" | ||
| style={{ | ||
| border: `1px solid ${theme.colors.border}`, | ||
| color: replyText.trim() ? theme.colors.textMain : theme.colors.textDim, | ||
| backgroundColor: 'transparent', | ||
| cursor: replyText.trim() ? 'pointer' : 'default', | ||
| opacity: replyText.trim() ? 1 : 0.5, |
There was a problem hiding this comment.
Disable no-op reply actions when handlers are unavailable.
onQuickReply and onOpenAndReply are optional, but Line 1088 and Line 1104 only gate on session/text state. This can render enabled actions that do nothing.
♻️ Suggested fix
+ const canQuickReply = sessionExists && !!replyText.trim() && !!onQuickReply;
+ const canOpenAndReply = sessionExists && !!replyText.trim() && !!onOpenAndReply;
...
<button
onClick={handleQuickReply}
- disabled={!sessionExists || !replyText.trim()}
+ disabled={!canQuickReply}
className="p-2 rounded-lg transition-colors flex-shrink-0"
style={{
- backgroundColor: replyText.trim()
+ backgroundColor: canQuickReply
? theme.colors.accent
: `${theme.colors.accent}30`,
- color: replyText.trim() ? theme.colors.accentForeground : theme.colors.textDim,
- cursor: replyText.trim() ? 'pointer' : 'default',
+ color: canQuickReply ? theme.colors.accentForeground : theme.colors.textDim,
+ cursor: canQuickReply ? 'pointer' : 'default',
}}
...
<button
onClick={handleOpenAndReply}
- disabled={!sessionExists || !replyText.trim()}
+ disabled={!canOpenAndReply}
className="p-1.5 rounded-lg transition-colors flex-shrink-0 text-xs"
style={{
border: `1px solid ${theme.colors.border}`,
- color: replyText.trim() ? theme.colors.textMain : theme.colors.textDim,
+ color: canOpenAndReply ? theme.colors.textMain : theme.colors.textDim,
backgroundColor: 'transparent',
- cursor: replyText.trim() ? 'pointer' : 'default',
- opacity: replyText.trim() ? 1 : 0.5,
+ cursor: canOpenAndReply ? 'pointer' : 'default',
+ opacity: canOpenAndReply ? 1 : 0.5,
}}🤖 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 1086 -
1111, The Quick Reply and Open & Reply buttons currently only check
sessionExists and replyText but not whether their handlers exist, so they can
appear enabled but do nothing; update the disabled checks and UI state for the
button using the handler presence (handleQuickReply relies on onQuickReply,
handleOpenAndReply relies on onOpenAndReply) by including a guard like
(!onQuickReply || !sessionExists || !replyText.trim()) and (!onOpenAndReply ||
!sessionExists || !replyText.trim()) respectively, and propagate that same
condition to cursor, opacity, title, and any other interactive styling so the
buttons are truly disabled when the callbacks are not provided.
Comment on lines
+830
to
+845
| if (e.shiftKey) { | ||
| if (focusIdx <= 0) { | ||
| e.preventDefault(); | ||
| containerRef.current?.focus(); | ||
| } else { | ||
| e.preventDefault(); | ||
| focusables[focusIdx - 1].focus(); | ||
| } | ||
| } else { | ||
| if (focusIdx === -1) { | ||
| e.preventDefault(); | ||
| focusables[0].focus(); | ||
| } else if (focusIdx >= focusables.length - 1) { | ||
| e.preventDefault(); | ||
| containerRef.current?.focus(); | ||
| } else { |
There was a problem hiding this comment.
Fix Shift+Tab trapping on the list container.
When focus is on the listbox (focusIdx === -1), Shift+Tab currently re-focuses the same element (Line 833), so backward keyboard navigation stalls.
♻️ Suggested fix
if (e.shiftKey) {
- if (focusIdx <= 0) {
+ if (focusIdx === -1) {
+ e.preventDefault();
+ focusables[focusables.length - 1].focus();
+ } else if (focusIdx === 0) {
e.preventDefault();
containerRef.current?.focus();
} else {
e.preventDefault();
focusables[focusIdx - 1].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 830 - 845,
The Shift+Tab handling traps when focusIdx === -1 because the shift branch
always focuses the container; update the keyboard handler in InboxListView (the
block using focusIdx, focusables, containerRef) so that when e.shiftKey and
focusIdx === -1 you move focus to the last focusable item
(focusables[focusables.length - 1].focus()) instead of re-focusing
containerRef.current; ensure you still call e.preventDefault() and preserve
existing behaviors for other focusIdx cases.
Comment on lines
774
to
777
| // Agent Inbox Modal (Unified Inbox) | ||
| setAgentInboxOpen: (open: boolean) => | ||
| open ? openModal('agentInbox') : closeModal('agentInbox'), | ||
| updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), |
There was a problem hiding this comment.
Closing Agent Inbox currently drops its persisted view state.
setAgentInboxOpen(false) calls closeModal('agentInbox'), and closeModal clears data (Line 348). That conflicts with the persisted-state contract on Line 143 and resets filter/sort/expanded preferences on reopen.
💡 Proposed fix (preserve data when closing Agent Inbox)
// Agent Inbox Modal (Unified Inbox)
- setAgentInboxOpen: (open: boolean) =>
- open ? openModal('agentInbox') : closeModal('agentInbox'),
+ setAgentInboxOpen: (open: boolean) => {
+ if (open) {
+ openModal('agentInbox');
+ return;
+ }
+
+ useModalStore.setState((state) => {
+ const current = state.modals.get('agentInbox');
+ if (!current?.open) return state;
+ const newModals = new Map(state.modals);
+ newModals.set('agentInbox', { open: false, data: current.data });
+ return { modals: newModals };
+ });
+ },📝 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
| // Agent Inbox Modal (Unified Inbox) | |
| setAgentInboxOpen: (open: boolean) => | |
| open ? openModal('agentInbox') : closeModal('agentInbox'), | |
| updateAgentInboxData: (data: Record<string, unknown>) => updateModalData('agentInbox', data), | |
| // Agent Inbox Modal (Unified Inbox) | |
| setAgentInboxOpen: (open: boolean) => { | |
| if (open) { | |
| openModal('agentInbox'); | |
| return; | |
| } | |
| useModalStore.setState((state) => { | |
| const current = state.modals.get('agentInbox'); | |
| if (!current?.open) return state; | |
| const newModals = new Map(state.modals); | |
| newModals.set('agentInbox', { open: false, data: current.data }); | |
| return { modals: newModals }; | |
| }); | |
| }, | |
| updateAgentInboxData: (data: Record<string, unknown>) => 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 Agent Inbox's
persisted view state is being cleared because setAgentInboxOpen(false) calls
closeModal('agentInbox'), and closeModal clears modal data; to fix, avoid
clearing data for the 'agentInbox' modal by either (A) adding an optional flag
to closeModal (e.g., closeModal(id, { preserveData: true })) and calling
closeModal('agentInbox', { preserveData: true }) from setAgentInboxOpen, or (B)
add a new helper like closeModalPreserveData('agentInbox') that performs the
same close flow but does not clear data; update setAgentInboxOpen to use the new
behavior and ensure updateModalData('agentInbox', ...) remains unchanged so
filters/sort/expanded state persist across closes/opens.
src/renderer/utils/tabDisplayName.ts
Outdated
Comment on lines
13
to
15
| if (tab.name) { | ||
| return tab.name; | ||
| } |
There was a problem hiding this comment.
Normalize whitespace-only tab names before returning.
Line 13 currently treats " " as a valid name, which can produce blank-looking tab titles instead of falling back to ID/default naming.
Proposed fix
export function getTabDisplayName(tab: Pick<AITab, 'name' | 'agentSessionId'>): string {
- if (tab.name) {
- return tab.name;
+ const normalizedName = tab.name?.trim();
+ if (normalizedName) {
+ return normalizedName;
}
if (tab.agentSessionId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/utils/tabDisplayName.ts` around lines 13 - 15, Normalize
tab.name by trimming whitespace before returning: instead of returning tab.name
blindly, compute a trimmedName = tab.name?.trim() and if trimmedName is
non-empty return it, otherwise fall back to the existing ID/default logic (use
tab.id or whatever default the current function uses). Update the code in the
function that reads tab.name (the tab.name check in tabDisplayName.ts) to use
trimmedName so whitespace-only names like " " do not produce blank-looking
titles.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/renderer/utils/tabDisplayName.ts (1)
13-15:⚠️ Potential issue | 🟡 MinorNormalize whitespace-only tab names before returning.
Line 13 returns
tab.namedirectly if truthy, but" "(whitespace-only string) is truthy and would produce a blank-looking tab title instead of falling back to ID-based naming.Proposed fix
export function getTabDisplayName(tab: Pick<AITab, 'name' | 'agentSessionId'>): string { - if (tab.name) { - return tab.name; + const trimmedName = tab.name?.trim(); + if (trimmedName) { + return trimmedName; } if (tab.agentSessionId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/tabDisplayName.ts` around lines 13 - 15, The current early return in tabDisplayName returns tab.name when truthy, but fails to normalize whitespace-only names; update the logic in tabDisplayName to trim tab.name (e.g., const name = tab.name?.trim()) and only return it if the trimmed value is non-empty, otherwise fall back to the existing ID-based naming path (use tab.id or the existing fallback logic) so whitespace-only names do not produce blank titles.src/renderer/components/AgentInbox/InboxListView.tsx (1)
830-850:⚠️ Potential issue | 🟠 MajorShift+Tab still traps when focus is on the list container.
When the list container itself has focus (
focusIdx === -1) and Shift+Tab is pressed, the code focuses the container again (Line 833), creating a keyboard trap. The condition should distinguish between "focus is on container" vs "focus is on first header element".🔧 Proposed fix
if (e.shiftKey) { - if (focusIdx <= 0) { + if (focusIdx === -1) { + // Focus is on container itself, cycle to last header focusable + e.preventDefault(); + if (focusables.length > 0) { + focusables[focusables.length - 1].focus(); + } + } else if (focusIdx === 0) { + // Focus is on first header element, return to container e.preventDefault(); containerRef.current?.focus(); } else {🤖 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 830 - 850, The Shift+Tab branch is treating container focus (focusIdx === -1) the same as the first header and re-focusing the container, causing a keyboard trap; update the key handling in InboxListView.tsx so that when e.shiftKey is true you only call e.preventDefault() and move focus to containerRef.current when focusIdx === 0 (the first header), but if focusIdx === -1 simply return without preventDefault (allow the browser to move focus out); reference the focusIdx, focusables and containerRef symbols when making this change.src/renderer/components/AgentInbox/FocusModeView.tsx (2)
752-758:⚠️ Potential issue | 🟡 MinorReset composer height after successful quick reply.
setReplyText('')clears content, but the textarea can remain expanded after send.🔧 Suggested patch
const handleQuickReply = useCallback(() => { const text = replyText.trim(); if (!text || !onQuickReply) return; onQuickReply(item.sessionId, item.tabId, text); onMarkAsRead?.(item.sessionId, item.tabId); setReplyText(''); + if (replyInputRef.current) { + replyInputRef.current.style.height = 'auto'; + } }, [replyText, item, onQuickReply, onMarkAsRead]);🤖 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 752 - 758, In handleQuickReply, after calling setReplyText(''), also reset the composer textarea height so it doesn't remain expanded; locate the handleQuickReply function and either call an existing reset function (e.g., resetComposerHeight or setComposerHeight(DEFAULT_HEIGHT)) or, if no helper exists, access the textarea ref (e.g., composerRef or textareaRef) and set its style.height = '' (or call the existing adjustTextareaHeight/compressTextarea helper) guarded by a null check so the composer returns to its initial height after a successful quick reply.
1086-1114:⚠️ Potential issue | 🟡 MinorDisable reply actions when callbacks are unavailable.
Buttons can appear enabled even when
onQuickReply/onOpenAndReplyare undefined, resulting in no-op interactions.🔧 Suggested patch
+ const canQuickReply = sessionExists && !!replyText.trim() && !!onQuickReply; + const canOpenAndReply = sessionExists && !!replyText.trim() && !!onOpenAndReply; + <button onClick={handleQuickReply} - disabled={!sessionExists || !replyText.trim()} + disabled={!canQuickReply} @@ - backgroundColor: replyText.trim() + backgroundColor: canQuickReply ? theme.colors.accent : `${theme.colors.accent}30`, - color: replyText.trim() ? theme.colors.accentForeground : theme.colors.textDim, - cursor: replyText.trim() ? 'pointer' : 'default', + color: canQuickReply ? theme.colors.accentForeground : theme.colors.textDim, + cursor: canQuickReply ? 'pointer' : 'default', }} @@ <button onClick={handleOpenAndReply} - disabled={!sessionExists || !replyText.trim()} + disabled={!canOpenAndReply} @@ - color: replyText.trim() ? theme.colors.textMain : theme.colors.textDim, + color: canOpenAndReply ? theme.colors.textMain : theme.colors.textDim, backgroundColor: 'transparent', - cursor: replyText.trim() ? 'pointer' : 'default', - opacity: replyText.trim() ? 1 : 0.5, + cursor: canOpenAndReply ? 'pointer' : 'default', + opacity: canOpenAndReply ? 1 : 0.5, }}🤖 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 1086 - 1114, The reply buttons can be clickable even if the callbacks are not provided; update the two buttons and their handlers so they are truly disabled when callbacks are missing: in the button JSX for the ArrowUp action and the "Open & Reply" secondary action include the callback existence in the disabled condition (e.g. disabled={!sessionExists || !replyText.trim() || !onQuickReply} and disabled={!sessionExists || !replyText.trim() || !onOpenAndReply}) and adjust styles/titles accordingly, and also add early-return guards inside handleQuickReply and handleOpenAndReply to no-op if their respective onQuickReply/onOpenAndReply props are undefined to prevent no-op interactions.
🧹 Nitpick comments (6)
src/renderer/utils/tabDisplayName.ts (1)
39-39: Consider using "New Agent" instead of "New Session" for user-facing text.Per coding guidelines, user-facing language should use "agent" rather than "session" (which is reserved for provider-level conversation contexts).
Proposed fix
- return 'New Session'; + return 'New Agent';As per coding guidelines: "Use the Session interface in code to represent agents (due to historical naming), but use 'agent' in user-facing language."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/tabDisplayName.ts` at line 39, Update the user-facing label returned from the function in src/renderer/utils/tabDisplayName.ts so it uses "New Agent" instead of "New Session"; locate the return statement (the one returning 'New Session') inside the tab display name helper (e.g., getTabDisplayName or default export in tabDisplayName.ts) and change the string to "New Agent" to follow the guideline that UI text should use "agent" while keeping the existing Session interface names in code.src/renderer/components/SessionList.tsx (1)
737-739: Prefer agent-centric copy in the subtitle.Consider changing
Unified tabs inboxtoUnified agent inboxfor consistent user-facing terminology.As per coding guidelines, "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/components/SessionList.tsx` around lines 737 - 739, In the SessionList component update the user-facing subtitle string from "Unified tabs inbox" to "Unified agent inbox" to follow agent-centric wording; locate the div with className "text-xs" (inside SessionList) that currently renders "Unified tabs inbox" and replace its text content with "Unified agent inbox" so UI copy aligns with the guideline that uses "agent" in user-facing language while keeping the Session interface name in code.src/renderer/components/SettingsModal.tsx (1)
3687-3710: AddtabIndex={0}to the switch buttons for keyboard focus consistency.Other switch buttons in this file (e.g., lines 2221, 2627, 2660) include
tabIndex={0}andoutline-nonefor keyboard navigation. The new Unified Inbox and Tab Descriptions switches are missingtabIndex, which may cause inconsistent keyboard navigation.♻️ Proposed fix
<button role="switch" aria-label="Toggle Unified Inbox" aria-checked={encoreFeatures.unifiedInbox} onClick={(e) => { e.stopPropagation(); setEncoreFeatures({ ...encoreFeatures, unifiedInbox: !encoreFeatures.unifiedInbox, }); }} - className="relative w-10 h-5 rounded-full transition-colors flex-shrink-0" + className="relative w-10 h-5 rounded-full transition-colors flex-shrink-0 outline-none" + tabIndex={0} style={{Apply the same change to the Tab Descriptions switch button.
Also applies to: 3745-3768
🤖 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 3687 - 3710, The switch button for the Unified Inbox (the element with role="switch" and aria-checked tied to encoreFeatures.unifiedInbox, using setEncoreFeatures to toggle) is missing tabIndex={0} and the keyboard outline class; update that button (and the analogous Tab Descriptions switch around the same area) to include tabIndex={0} and the same focus/outline styling (e.g., include outline-none or the project's focus class) so it matches other switches and supports keyboard navigation consistently.src/renderer/hooks/tabs/useTabHandlers.ts (1)
970-984: Skip no-op state writes in description updates.Line 973 currently remaps
sessions/aiTabseven when the target tab is missing or the normalized description is unchanged, causing avoidable store updates.♻️ Proposed refactor
const handleUpdateTabDescription = useCallback((tabId: string, description: string) => { - const trimmed = description.trim(); + const trimmed = description.trim(); + const nextDescription = trimmed || undefined; const { setSessions, activeSessionId } = useSessionStore.getState(); setSessions((prev: Session[]) => prev.map((s) => { if (s.id !== activeSessionId) return s; + let changed = false; + const aiTabs = s.aiTabs.map((tab) => { + if (tab.id !== tabId) return tab; + if (tab.description === nextDescription) return tab; + changed = true; + return { ...tab, description: nextDescription }; + }); return { - ...s, - aiTabs: s.aiTabs.map((tab) => - tab.id === tabId ? { ...tab, description: trimmed || undefined } : tab - ), + ...(changed ? { ...s, aiTabs } : s), }; }) ); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/tabs/useTabHandlers.ts` around lines 970 - 984, The handler handleUpdateTabDescription currently remaps sessions/aiTabs even when the target tab doesn't exist or the normalized description hasn't changed; update it to first locate the active session via useSessionStore.getState().activeSessionId and the specific tab by tabId, compute the newDesc = trimmed || undefined, and if the tab is missing or its existing description === newDesc return prev unchanged to avoid a no-op set; otherwise call setSessions with a new sessions array where only that session's aiTabs replace the specific tab's description. Ensure you reference setSessions, activeSessionId, aiTabs, tabId and description when implementing the conditional early return.src/renderer/components/AgentInbox/index.tsx (1)
314-315: Consider removing empty event handlers.The
onFocusandonBlurhandlers are no-ops. If they're placeholders for future functionality, consider adding a brief comment. Otherwise, removing them would clean up the code.♻️ Optional cleanup
onKeyDownCapture={handleCaptureKeyDown} onKeyDown={handleShellKeyDown} - onFocus={() => {}} - onBlur={() => {}} >🤖 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 314 - 315, Remove the no-op handlers on the input/component in AgentInbox (the onFocus and onBlur props) or replace them with a short comment if they’re intentional placeholders; locate the JSX in AgentInbox/index.tsx where onFocus={() => {}} and onBlur={() => {}} are attached and either delete those props or add a comment like // placeholder for future focus/blur handling to make intent explicit.src/renderer/hooks/useAgentInbox.ts (1)
14-32: UnusedsessionStateparameter.The
sessionStateparameter is passed tomatchesFilterbut never used in the function body. If this is reserved for a future filter mode (e.g., "needs-input"), consider adding a comment. Otherwise, it can be removed to reduce confusion.♻️ Option A: Remove if not needed
function matchesFilter( - sessionState: Session['state'], hasUnread: boolean, filterMode: InboxFilterMode, isStarred: boolean ): boolean {And update the call site at line 209:
- if (!matchesFilter(session.state, hasUnread, filterMode, tab.starred === true)) continue; + if (!matchesFilter(hasUnread, filterMode, tab.starred === true)) continue;♻️ Option B: Document intent for future use
function matchesFilter( + // Reserved for future 'needs-input' filter mode sessionState: Session['state'], hasUnread: boolean,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/useAgentInbox.ts` around lines 14 - 32, The parameter sessionState on matchesFilter is unused; remove it from the function signature in useAgentInbox.ts (function matchesFilter) and update all call sites to stop passing a Session['state'] argument, adjusting any type annotations accordingly; if the parameter is intentionally reserved for a future "needs-input" filter, instead keep the parameter but add a clear comment above matchesFilter explaining it's reserved for future use and why so it doesn't appear unused.
🤖 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-1425: The effect that flushes pendingInboxQuickReply should
also clear it when the target session no longer exists; update the useEffect
handling pendingInboxQuickReply (which currently checks activeSession?.id !==
pendingInboxQuickReply.targetSessionId) to detect when the targetSessionId
cannot be found (or the session list no longer contains it) and call
setPendingInboxQuickReply(null) in that branch so stale pendingInboxQuickReply
is cleared and doesn't block future quick replies handled by
handleAgentInboxQuickReply; keep the existing logic that processes the reply
when the activeSession matches and the microtask that restores
previousActiveSessionId.
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 433-509: The list container currently uses role="listbox" while
rendering interactive header divs with role="button" inside rows.map in
FocusModeView.tsx, creating an invalid ARIA hierarchy; change the container role
from "listbox" to "list" and mark each rendered row (both header and item) as
role="listitem" (or wrap header contents in a real <button> while keeping its
listitem wrapper) so headers remain interactive but the ARIA tree is valid;
update the div that currently has role="button" (the header rendering block) to
be a listitem wrapper with the interactive element inside (or keep tabIndex and
onKeyDown on a child button) and ensure aria-expanded is moved to the
interactive control while the outer element stays role="listitem".
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 984-1024: The Focus button (uses items, selectedIndex,
onEnterFocus) and the Expand button (uses onToggleExpanded, isExpanded) need
keyboard-focus styling like the Close button: add tabIndex={0} and
onFocus/onBlur handlers to both buttons that apply and remove the same visible
styles used for the Close button (e.g., set backgroundColor and
outline/boxShadow consistent with theme.colors.accent variants), ensuring the
handlers reference the same theme values and element via e.currentTarget so
keyboard users see the focus indicator; keep existing onMouseEnter/onMouseLeave
logic intact and implement focus handlers analogously for both elements.
---
Duplicate comments:
In `@src/renderer/components/AgentInbox/FocusModeView.tsx`:
- Around line 752-758: In handleQuickReply, after calling setReplyText(''), also
reset the composer textarea height so it doesn't remain expanded; locate the
handleQuickReply function and either call an existing reset function (e.g.,
resetComposerHeight or setComposerHeight(DEFAULT_HEIGHT)) or, if no helper
exists, access the textarea ref (e.g., composerRef or textareaRef) and set its
style.height = '' (or call the existing adjustTextareaHeight/compressTextarea
helper) guarded by a null check so the composer returns to its initial height
after a successful quick reply.
- Around line 1086-1114: The reply buttons can be clickable even if the
callbacks are not provided; update the two buttons and their handlers so they
are truly disabled when callbacks are missing: in the button JSX for the ArrowUp
action and the "Open & Reply" secondary action include the callback existence in
the disabled condition (e.g. disabled={!sessionExists || !replyText.trim() ||
!onQuickReply} and disabled={!sessionExists || !replyText.trim() ||
!onOpenAndReply}) and adjust styles/titles accordingly, and also add
early-return guards inside handleQuickReply and handleOpenAndReply to no-op if
their respective onQuickReply/onOpenAndReply props are undefined to prevent
no-op interactions.
In `@src/renderer/components/AgentInbox/InboxListView.tsx`:
- Around line 830-850: The Shift+Tab branch is treating container focus
(focusIdx === -1) the same as the first header and re-focusing the container,
causing a keyboard trap; update the key handling in InboxListView.tsx so that
when e.shiftKey is true you only call e.preventDefault() and move focus to
containerRef.current when focusIdx === 0 (the first header), but if focusIdx ===
-1 simply return without preventDefault (allow the browser to move focus out);
reference the focusIdx, focusables and containerRef symbols when making this
change.
In `@src/renderer/utils/tabDisplayName.ts`:
- Around line 13-15: The current early return in tabDisplayName returns tab.name
when truthy, but fails to normalize whitespace-only names; update the logic in
tabDisplayName to trim tab.name (e.g., const name = tab.name?.trim()) and only
return it if the trimmed value is non-empty, otherwise fall back to the existing
ID-based naming path (use tab.id or the existing fallback logic) so
whitespace-only names do not produce blank titles.
---
Nitpick comments:
In `@src/renderer/components/AgentInbox/index.tsx`:
- Around line 314-315: Remove the no-op handlers on the input/component in
AgentInbox (the onFocus and onBlur props) or replace them with a short comment
if they’re intentional placeholders; locate the JSX in AgentInbox/index.tsx
where onFocus={() => {}} and onBlur={() => {}} are attached and either delete
those props or add a comment like // placeholder for future focus/blur handling
to make intent explicit.
In `@src/renderer/components/SessionList.tsx`:
- Around line 737-739: In the SessionList component update the user-facing
subtitle string from "Unified tabs inbox" to "Unified agent inbox" to follow
agent-centric wording; locate the div with className "text-xs" (inside
SessionList) that currently renders "Unified tabs inbox" and replace its text
content with "Unified agent inbox" so UI copy aligns with the guideline that
uses "agent" in user-facing language while keeping the Session interface name in
code.
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 3687-3710: The switch button for the Unified Inbox (the element
with role="switch" and aria-checked tied to encoreFeatures.unifiedInbox, using
setEncoreFeatures to toggle) is missing tabIndex={0} and the keyboard outline
class; update that button (and the analogous Tab Descriptions switch around the
same area) to include tabIndex={0} and the same focus/outline styling (e.g.,
include outline-none or the project's focus class) so it matches other switches
and supports keyboard navigation consistently.
In `@src/renderer/hooks/tabs/useTabHandlers.ts`:
- Around line 970-984: The handler handleUpdateTabDescription currently remaps
sessions/aiTabs even when the target tab doesn't exist or the normalized
description hasn't changed; update it to first locate the active session via
useSessionStore.getState().activeSessionId and the specific tab by tabId,
compute the newDesc = trimmed || undefined, and if the tab is missing or its
existing description === newDesc return prev unchanged to avoid a no-op set;
otherwise call setSessions with a new sessions array where only that session's
aiTabs replace the specific tab's description. Ensure you reference setSessions,
activeSessionId, aiTabs, tabId and description when implementing the conditional
early return.
In `@src/renderer/hooks/useAgentInbox.ts`:
- Around line 14-32: The parameter sessionState on matchesFilter is unused;
remove it from the function signature in useAgentInbox.ts (function
matchesFilter) and update all call sites to stop passing a Session['state']
argument, adjusting any type annotations accordingly; if the parameter is
intentionally reserved for a future "needs-input" filter, instead keep the
parameter but add a clear comment above matchesFilter explaining it's reserved
for future use and why so it doesn't appear unused.
In `@src/renderer/utils/tabDisplayName.ts`:
- Line 39: Update the user-facing label returned from the function in
src/renderer/utils/tabDisplayName.ts so it uses "New Agent" instead of "New
Session"; locate the return statement (the one returning 'New Session') inside
the tab display name helper (e.g., getTabDisplayName or default export in
tabDisplayName.ts) and change the string to "New Agent" to follow the guideline
that UI text should use "agent" while keeping the existing Session interface
names in code.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
docs/encore-features.mddocs/keyboard-shortcuts.mdsrc/__tests__/renderer/components/AgentInbox/AgentInbox.test.tsxsrc/__tests__/renderer/components/InputArea.test.tsxsrc/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useAgentInbox.test.tssrc/__tests__/renderer/hooks/useInputKeyDown.test.tssrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/__tests__/renderer/stores/modalStore.test.tssrc/__tests__/renderer/utils/search.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/InputArea.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList.tsxsrc/renderer/components/SettingsModal.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/input/useInputKeyDown.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/hooks/useAgentInbox.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/agent-inbox.tssrc/renderer/types/index.tssrc/renderer/utils/search.tssrc/renderer/utils/tabDisplayName.ts
src/renderer/App.tsx
Outdated
Comment on lines
1408
to
1425
| // 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.
Clear stale pendingInboxQuickReply when the target session no longer exists.
If the target session is removed before activation, this effect never clears pendingInboxQuickReply, and later quick replies get blocked by the guard in handleAgentInboxQuickReply.
🔧 Suggested patch
useEffect(() => {
if (!pendingInboxQuickReply) return;
+ const targetExists = sessions.some((s) => s.id === pendingInboxQuickReply.targetSessionId);
+ if (!targetExists) {
+ setPendingInboxQuickReply(null);
+ return;
+ }
if (activeSession?.id !== pendingInboxQuickReply.targetSessionId) return;
inboxProcessInputRef.current(pendingInboxQuickReply.text);
const previousActiveSessionId = pendingInboxQuickReply.previousActiveSessionId;
setPendingInboxQuickReply(null);
@@
- }, [pendingInboxQuickReply, activeSession?.id, setActiveSessionId]);
+ }, [pendingInboxQuickReply, activeSession?.id, sessions, setActiveSessionId]);🤖 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 - 1425, The effect that flushes
pendingInboxQuickReply should also clear it when the target session no longer
exists; update the useEffect handling pendingInboxQuickReply (which currently
checks activeSession?.id !== pendingInboxQuickReply.targetSessionId) to detect
when the targetSessionId cannot be found (or the session list no longer contains
it) and call setPendingInboxQuickReply(null) in that branch so stale
pendingInboxQuickReply is cleared and doesn't block future quick replies handled
by handleAgentInboxQuickReply; keep the existing logic that processes the reply
when the activeSession matches and the microtask that restores
previousActiveSessionId.
Comment on lines
433
to
509
| <div role="listbox" aria-label="Inbox items" 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="button" | ||
| aria-expanded={!collapsedGroups.has(row.groupKey)} | ||
| onClick={() => { | ||
| setCollapsedGroups((prev) => { | ||
| const next = new Set(prev); | ||
| if (next.has(row.groupKey)) next.delete(row.groupKey); | ||
| else next.add(row.groupKey); | ||
| return next; | ||
| }); | ||
| }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| setCollapsedGroups((prev) => { | ||
| const next = new Set(prev); | ||
| if (next.has(row.groupKey)) next.delete(row.groupKey); | ||
| else next.add(row.groupKey); | ||
| return next; | ||
| }); | ||
| } | ||
| }} | ||
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | ||
| onFocus={(e) => { | ||
| e.currentTarget.style.boxShadow = `inset 0 0 0 2px ${theme.colors.accent}`; | ||
| }} | ||
| onBlur={(e) => { | ||
| e.currentTarget.style.boxShadow = 'none'; | ||
| }} | ||
| style={{ | ||
| color: theme.colors.textDim, | ||
| fontWeight: 600, | ||
| backgroundColor: theme.colors.bgSidebar, | ||
| outline: 'none', | ||
| }} | ||
| > | ||
| {collapsedGroups.has(row.groupKey) ? ( | ||
| <ChevronRight className="w-3 h-3" /> | ||
| ) : ( | ||
| <ChevronDown className="w-3 h-3" /> | ||
| )} | ||
| {row.groupName} | ||
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | ||
| {row.type === 'header' ? row.count : 0} | ||
| </span> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Skip items in collapsed groups | ||
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | ||
|
|
||
| const itm = row.item; | ||
| const idx = row.index; | ||
| const isCurrent = idx === currentIndex; | ||
| const statusColor = resolveStatusColor(itm.state, theme); | ||
|
|
||
| const previewText = itm.lastMessage | ||
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | ||
| : ''; | ||
|
|
||
| return ( | ||
| <div | ||
| key={`${itm.sessionId}-${itm.tabId}`} | ||
| ref={isCurrent ? currentRowRef : undefined} | ||
| tabIndex={0} | ||
| role="option" | ||
| aria-selected={isCurrent} |
There was a problem hiding this comment.
Fix ARIA role hierarchy in the sidebar list.
role="listbox" expects option/group semantics, but headers are interactive role="button" siblings. This creates inconsistent screen-reader navigation.
♿ Suggested patch
- <div role="listbox" aria-label="Inbox items" className="flex-1 overflow-y-auto py-1">
+ <div role="list" aria-label="Inbox items" className="flex-1 overflow-y-auto py-1">
@@
- role="option"
- aria-selected={isCurrent}
+ role="listitem"
+ aria-current={isCurrent ? 'true' : undefined}📝 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
| <div role="listbox" aria-label="Inbox items" 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="button" | |
| aria-expanded={!collapsedGroups.has(row.groupKey)} | |
| onClick={() => { | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupKey)) next.delete(row.groupKey); | |
| else next.add(row.groupKey); | |
| return next; | |
| }); | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupKey)) next.delete(row.groupKey); | |
| else next.add(row.groupKey); | |
| return next; | |
| }); | |
| } | |
| }} | |
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | |
| onFocus={(e) => { | |
| e.currentTarget.style.boxShadow = `inset 0 0 0 2px ${theme.colors.accent}`; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.boxShadow = 'none'; | |
| }} | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontWeight: 600, | |
| backgroundColor: theme.colors.bgSidebar, | |
| outline: 'none', | |
| }} | |
| > | |
| {collapsedGroups.has(row.groupKey) ? ( | |
| <ChevronRight className="w-3 h-3" /> | |
| ) : ( | |
| <ChevronDown className="w-3 h-3" /> | |
| )} | |
| {row.groupName} | |
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | |
| {row.type === 'header' ? row.count : 0} | |
| </span> | |
| </div> | |
| ); | |
| } | |
| // Skip items in collapsed groups | |
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | |
| const itm = row.item; | |
| const idx = row.index; | |
| const isCurrent = idx === currentIndex; | |
| const statusColor = resolveStatusColor(itm.state, theme); | |
| const previewText = itm.lastMessage | |
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | |
| : ''; | |
| return ( | |
| <div | |
| key={`${itm.sessionId}-${itm.tabId}`} | |
| ref={isCurrent ? currentRowRef : undefined} | |
| tabIndex={0} | |
| role="option" | |
| aria-selected={isCurrent} | |
| <div role="list" aria-label="Inbox items" 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="button" | |
| aria-expanded={!collapsedGroups.has(row.groupKey)} | |
| onClick={() => { | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupKey)) next.delete(row.groupKey); | |
| else next.add(row.groupKey); | |
| return next; | |
| }); | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setCollapsedGroups((prev) => { | |
| const next = new Set(prev); | |
| if (next.has(row.groupKey)) next.delete(row.groupKey); | |
| else next.add(row.groupKey); | |
| return next; | |
| }); | |
| } | |
| }} | |
| className="flex items-center gap-1.5 px-3 py-1.5 text-[10px] uppercase tracking-wider cursor-pointer" | |
| onFocus={(e) => { | |
| e.currentTarget.style.boxShadow = `inset 0 0 0 2px ${theme.colors.accent}`; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.boxShadow = 'none'; | |
| }} | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontWeight: 600, | |
| backgroundColor: theme.colors.bgSidebar, | |
| outline: 'none', | |
| }} | |
| > | |
| {collapsedGroups.has(row.groupKey) ? ( | |
| <ChevronRight className="w-3 h-3" /> | |
| ) : ( | |
| <ChevronDown className="w-3 h-3" /> | |
| )} | |
| {row.groupName} | |
| <span style={{ color: theme.colors.textDim, opacity: 0.5, marginLeft: 'auto' }}> | |
| {row.type === 'header' ? row.count : 0} | |
| </span> | |
| </div> | |
| ); | |
| } | |
| // Skip items in collapsed groups | |
| if (activeGroup && collapsedGroups.has(activeGroup)) return null; | |
| const itm = row.item; | |
| const idx = row.index; | |
| const isCurrent = idx === currentIndex; | |
| const statusColor = resolveStatusColor(itm.state, theme); | |
| const previewText = itm.lastMessage | |
| ? itm.lastMessage.replace(/[#*`>]/g, '').slice(0, 60) | |
| : ''; | |
| return ( | |
| <div | |
| key={`${itm.sessionId}-${itm.tabId}`} | |
| ref={isCurrent ? currentRowRef : undefined} | |
| tabIndex={0} | |
| role="listitem" | |
| aria-current={isCurrent ? 'true' : undefined} |
🤖 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 433 - 509,
The list container currently uses role="listbox" while rendering interactive
header divs with role="button" inside rows.map in FocusModeView.tsx, creating an
invalid ARIA hierarchy; change the container role from "listbox" to "list" and
mark each rendered row (both header and item) as role="listitem" (or wrap header
contents in a real <button> while keeping its listitem wrapper) so headers
remain interactive but the ARIA tree is valid; update the div that currently has
role="button" (the header rendering block) to be a listitem wrapper with the
interactive element inside (or keep tabIndex and onKeyDown on a child button)
and ensure aria-expanded is moved to the interactive control while the outer
element stays role="listitem".
Comment on lines
984
to
1024
| <button | ||
| onClick={() => { | ||
| if (items.length > 0 && items[selectedIndex]) { | ||
| onEnterFocus(items[selectedIndex]); | ||
| } | ||
| }} | ||
| disabled={items.length === 0} | ||
| className="text-xs px-2.5 py-1 rounded transition-colors" | ||
| style={{ | ||
| backgroundColor: items.length > 0 ? `${theme.colors.accent}15` : 'transparent', | ||
| color: items.length > 0 ? theme.colors.accent : theme.colors.textDim, | ||
| cursor: items.length > 0 ? 'pointer' : 'default', | ||
| opacity: items.length === 0 ? 0.5 : 1, | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| if (items.length > 0) { | ||
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}25`; | ||
| } | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| if (items.length > 0) { | ||
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}15`; | ||
| } | ||
| }} | ||
| title="Enter Focus Mode (F)" | ||
| > | ||
| Focus ▶ | ||
| </button> | ||
| <button | ||
| onClick={() => onToggleExpanded((prev) => !prev)} | ||
| 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')} | ||
| title={isExpanded ? 'Collapse' : 'Expand'} | ||
| aria-label={isExpanded ? 'Collapse modal' : 'Expand modal'} | ||
| > | ||
| {isExpanded ? <Minimize2 className="w-4 h-4" /> : <Maximize2 className="w-4 h-4" />} | ||
| </button> |
There was a problem hiding this comment.
Add focus styling to Focus and Expand buttons for keyboard accessibility.
The close button (Lines 1033-1038) has onFocus/onBlur handlers for visible focus, but the Focus button (Lines 984-1011) and Expand button (Lines 1012-1024) lack these handlers. Keyboard users navigating through header controls won't see focus indicators on these buttons.
♿ Proposed fix for Focus button
<button
onClick={() => {
if (items.length > 0 && items[selectedIndex]) {
onEnterFocus(items[selectedIndex]);
}
}}
disabled={items.length === 0}
className="text-xs px-2.5 py-1 rounded transition-colors"
style={{
backgroundColor: items.length > 0 ? `${theme.colors.accent}15` : 'transparent',
color: items.length > 0 ? theme.colors.accent : theme.colors.textDim,
cursor: items.length > 0 ? 'pointer' : 'default',
opacity: items.length === 0 ? 0.5 : 1,
}}
onMouseEnter={(e) => {
if (items.length > 0) {
e.currentTarget.style.backgroundColor = `${theme.colors.accent}25`;
}
}}
onMouseLeave={(e) => {
if (items.length > 0) {
e.currentTarget.style.backgroundColor = `${theme.colors.accent}15`;
}
}}
+ onFocus={(e) => {
+ e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`;
+ e.currentTarget.style.outlineOffset = '-2px';
+ }}
+ onBlur={(e) => {
+ e.currentTarget.style.outline = 'none';
+ }}
title="Enter Focus Mode (F)"
>As per coding guidelines, "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 | |
| onClick={() => { | |
| if (items.length > 0 && items[selectedIndex]) { | |
| onEnterFocus(items[selectedIndex]); | |
| } | |
| }} | |
| disabled={items.length === 0} | |
| className="text-xs px-2.5 py-1 rounded transition-colors" | |
| style={{ | |
| backgroundColor: items.length > 0 ? `${theme.colors.accent}15` : 'transparent', | |
| color: items.length > 0 ? theme.colors.accent : theme.colors.textDim, | |
| cursor: items.length > 0 ? 'pointer' : 'default', | |
| opacity: items.length === 0 ? 0.5 : 1, | |
| }} | |
| onMouseEnter={(e) => { | |
| if (items.length > 0) { | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}25`; | |
| } | |
| }} | |
| onMouseLeave={(e) => { | |
| if (items.length > 0) { | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}15`; | |
| } | |
| }} | |
| title="Enter Focus Mode (F)" | |
| > | |
| Focus ▶ | |
| </button> | |
| <button | |
| onClick={() => onToggleExpanded((prev) => !prev)} | |
| 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')} | |
| title={isExpanded ? 'Collapse' : 'Expand'} | |
| aria-label={isExpanded ? 'Collapse modal' : 'Expand modal'} | |
| > | |
| {isExpanded ? <Minimize2 className="w-4 h-4" /> : <Maximize2 className="w-4 h-4" />} | |
| </button> | |
| <button | |
| onClick={() => { | |
| if (items.length > 0 && items[selectedIndex]) { | |
| onEnterFocus(items[selectedIndex]); | |
| } | |
| }} | |
| disabled={items.length === 0} | |
| className="text-xs px-2.5 py-1 rounded transition-colors" | |
| style={{ | |
| backgroundColor: items.length > 0 ? `${theme.colors.accent}15` : 'transparent', | |
| color: items.length > 0 ? theme.colors.accent : theme.colors.textDim, | |
| cursor: items.length > 0 ? 'pointer' : 'default', | |
| opacity: items.length === 0 ? 0.5 : 1, | |
| }} | |
| onMouseEnter={(e) => { | |
| if (items.length > 0) { | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}25`; | |
| } | |
| }} | |
| onMouseLeave={(e) => { | |
| if (items.length > 0) { | |
| e.currentTarget.style.backgroundColor = `${theme.colors.accent}15`; | |
| } | |
| }} | |
| onFocus={(e) => { | |
| e.currentTarget.style.outline = `2px solid ${theme.colors.accent}`; | |
| e.currentTarget.style.outlineOffset = '-2px'; | |
| }} | |
| onBlur={(e) => { | |
| e.currentTarget.style.outline = 'none'; | |
| }} | |
| title="Enter Focus Mode (F)" | |
| > | |
| Focus ▶ | |
| </button> | |
| <button | |
| onClick={() => onToggleExpanded((prev) => !prev)} | |
| 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')} | |
| title={isExpanded ? 'Collapse' : 'Expand'} | |
| aria-label={isExpanded ? 'Collapse modal' : 'Expand modal'} | |
| > | |
| {isExpanded ? <Minimize2 className="w-4 h-4" /> : <Maximize2 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 984 -
1024, The Focus button (uses items, selectedIndex, onEnterFocus) and the Expand
button (uses onToggleExpanded, isExpanded) need keyboard-focus styling like the
Close button: add tabIndex={0} and onFocus/onBlur handlers to both buttons that
apply and remove the same visible styles used for the Close button (e.g., set
backgroundColor and outline/boxShadow consistent with theme.colors.accent
variants), ensuring the handlers reference the same theme values and element via
e.currentTarget so keyboard users see the focus indicator; keep existing
onMouseEnter/onMouseLeave logic intact and implement focus handlers analogously
for both elements.
- Fix 1: Reply guards — only call onMarkAsRead if handler exists and executes - Fix 2: Textarea disabled state when session unavailable - Fix 3: Align groupId collapse key across buildRows, filter, and T-key toggle - Fix 4: ARIA roles — role="button" on interactive sidebar headers (was "presentation") - Fix 5: Focus ring styling on Focus/Expand buttons (consistent with Close) - Fix 6: Guard stale pendingInboxQuickReply when target session is deleted - Nice-to-have: sessionExists guard on textarea onKeyDown - Nice-to-have: previousActiveSessionId guard in microtask - Nice-to-have: Extract getGroupCollapseKey() helper for DRY Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
felipeggv
force-pushed
the
feat/encore-inbox-tabdesc-fuzzy
branch
from
826ca20 to
a28ac12
Compare
Thread allSlashCommands (built-in + custom + speckit + openspec + agent) from App.tsx through AgentInbox to FocusModeView. Includes fuzzy matching, keyboard navigation (arrows/Enter/Escape), and dropdown rendering. Perf-gated: filtering only runs when dropdown is open, ref-based onChange skips no-op state updates to avoid keystroke lag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix ARIA role hierarchy in FocusModeView sidebar: change group headers from role="button" to role="presentation" (children of role="list" must be listitem or presentation, not button) - Add fuzzy score-based sorting to slash command autocomplete in useInputKeyDown (was using boolean fuzzyMatch without scoring, now matches InputArea/FocusModeView pattern with fuzzyMatchWithScore) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…te preservation, tab name trimming and textarea ref stability
… column names The inbox quick reply effect (`App.tsx`) entered an infinite loop because processInput() calls Zustand setSessions() (synchronous), which re-triggers the effect before React commits the batched setPendingInboxQuickReply(null). Added inboxQuickReplyFiredRef as a synchronous guard to prevent re-entry. Also fixed stats-db column names from camelCase (startTime, createdAt) to snake_case (start_time, created_at) to match the actual SQLite schema. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
unifiedInboxandtabDescriptionflags to EncoreFeatureFlags, gated keyboard shortcuts and modal rendering behind feature togglesDetails
Rebuilt from clean
mainbaseline using 4 parallel worktrees, each developed independently then merged via integration branch. 29 files changed, +5,090 lines.New Files
src/renderer/components/AgentInbox/(index, InboxListView, FocusModeView)src/renderer/hooks/useAgentInbox.tssrc/renderer/types/agent-inbox.tssrc/renderer/utils/tabDisplayName.tsModified Files
App.tsx— Inbox modal integration + encore gatingSettingsModal.tsx— Encore toggle cards for new featuresTabBar.tsx— Description display/edit UImodalStore.ts— updateModalData behaviorsettingsStore.ts— New encore flag defaultsshortcuts.ts+useMainKeyboardHandler.ts— Option+I shortcutTest plan
npm run lint— cleannpm run test— 21,081 passing (3 pre-existing CodexSessionStorage timeouts)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Keyboard Shortcuts
Settings / Documentation
Improvements
Tests