feat: project-centric navigation with inbox system#549
Open
wistfulvariable wants to merge 20 commits intoRunMaestro:mainfrom
Open
feat: project-centric navigation with inbox system#549wistfulvariable wants to merge 20 commits intoRunMaestro:mainfrom
wistfulvariable wants to merge 20 commits intoRunMaestro:mainfrom
Conversation
Introduce tiered documentation optimized for AI coding agents: - Tier 1: Expand CLAUDE.md with Documentation Hierarchy section - Tier 2: Add 11 topic-specific memory files (.claude/memory/) covering testing, data model, agents, IPC, patterns, performance, platform/SSH, wizard, features, pitfalls, and build/deploy - Tier 1: MEMORY.md index with critical cross-session patterns This system minimizes token waste by loading only relevant docs per task while keeping critical rules always available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Redesigns Maestro from agent-centric to project/repo-centric navigation: projects in left sidebar, session tabs per project, global inbox for tabs needing attention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
18-task plan covering: types, stores, IPC persistence, migration, sidebar UI, inbox watcher, App.tsx integration, keyboard shortcuts, and dead code cleanup. TDD throughout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Loads projects from disk on startup, runs one-time migration to convert existing groups into projects, and provides debounced persistence for the project store. Migration flag is only set after data is safely on disk. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Item, ProjectSidebar) Implements Tasks 8-11 of the project-centric navigation redesign: - InboxItem: attention item with reason dot, tab name, relative time - InboxSection: collapsible inbox with count badge and clear button - ProjectItem: project row with color accent, active highlight, session count - ProjectSidebar: main sidebar composing inbox + project list with navigation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- InboxItem reason dots now use theme.colors.success/error/warning instead of hardcoded hex values for theme consistency - selectInboxItems returns stable reference (items stored pre-sorted) - addItem prepends to maintain newest-first order Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… into App Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove group CRUD actions (addGroup, removeGroup, updateGroup, toggleGroupCollapsed), toggleBookmark action, and group/bookmark selectors (selectBookmarkedSessions, selectSessionsByGroup, selectUngroupedSessions, selectGroupById) from the session store. Retained: groups state array, Group import, and setGroups action because they are still consumed by production code (SessionList, useSessionFilterMode, and ~12 other renderer files) for legacy migration purposes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added mockProjectsStore to test setup and projects:getAll/projects:setAll to expected channels list. Added 5 new test cases for project persistence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added Navigation Model section, ProjectSidebar references, and key files for project sidebar, inbox triggers, and project persistence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces project-centric navigation, replacing a session-list sidebar with a Projects hierarchy containing per-project sessions, plus a global Inbox for attention items. It adds Project and InboxItem types, new Zustand stores (projectStore, inboxStore), IPC persistence for projects, UI components (ProjectSidebar with InboxSection), restoration and migration logic from groups to projects, and comprehensive test coverage, while removing group/bookmark store APIs. Changes
Sequence DiagramssequenceDiagram
participant App
participant useProjectRestoration
participant projectStore
participant sessionStore
participant IPC as maestro.projects
participant Disk
App->>useProjectRestoration: mount
useProjectRestoration->>sessionStore: check initialLoadComplete
alt initialLoadComplete
useProjectRestoration->>IPC: getAll() projects
IPC->>Disk: read projects
Disk-->>IPC: projects or null
IPC-->>useProjectRestoration: projects[]
alt projects exist
useProjectRestoration->>projectStore: setProjects(loaded)
useProjectRestoration->>projectStore: setActiveProjectId(fromSession)
else no projects
useProjectRestoration->>useProjectRestoration: migrateGroupsToProjects()
useProjectRestoration->>sessionStore: setSessions(migratedSessions)
useProjectRestoration->>projectStore: setProjects(migrated)
useProjectRestoration->>IPC: setAll(migrated)
IPC->>Disk: persist projects
end
useProjectRestoration->>useProjectRestoration: debounce persistence
useProjectRestoration->>IPC: setAll(projects)
IPC->>Disk: write projects
end
sequenceDiagram
participant sessionStore
participant useInboxWatcher
participant shouldCreateInboxItem
participant inboxStore
participant projectStore
participant Inbox
sessionStore->>useInboxWatcher: state change detected
useInboxWatcher->>shouldCreateInboxItem: (prevState, newState, sessionId, activeSessionId)
alt session is active
shouldCreateInboxItem-->>useInboxWatcher: null (suppress)
else non-active session
alt busy → idle/error or any → waiting_input
shouldCreateInboxItem-->>useInboxWatcher: reason (finished/error/waiting_input)
useInboxWatcher->>projectStore: getProject(session.projectId)
useInboxWatcher->>inboxStore: addItem(sessionId, reason, projectName, ...)
inboxStore->>Inbox: dedup by sessionId+reason
Inbox->>Inbox: render InboxItem with badge
else no transition
shouldCreateInboxItem-->>useInboxWatcher: null
end
end
sequenceDiagram
participant User
participant KeyboardHandler
participant projectStore
participant sessionStore
participant ProjectSidebar
participant UI
User->>KeyboardHandler: Ctrl+Shift+]
KeyboardHandler->>projectStore: getState() → activeProjectId, projects
KeyboardHandler->>projectStore: compute nextProjectId
KeyboardHandler->>projectStore: setActiveProjectId(nextProjectId)
KeyboardHandler->>sessionStore: selectSessionsByProject(nextProjectId)
KeyboardHandler->>sessionStore: setActiveSessionId(firstSessionId)
projectStore->>ProjectSidebar: notify activeProjectId change
sessionStore->>ProjectSidebar: notify activeSessionId change
ProjectSidebar->>UI: re-render with new active project highlight
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The registerAllHandlers function was missing projectsStore in its deps, causing a TypeScript build error after adding projects persistence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR replaces the agent-centric left sidebar with a project-centric navigation model. It introduces Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant S as SessionStore
participant IW as useInboxWatcher
participant IS as inboxStore
participant PS as ProjectSidebar
participant App as App.tsx
Note over App: App startup
App->>App: useProjectRestoration()
App->>App: useInboxWatcher()
Note over App: Load projects from disk
App->>App: window.maestro.projects.getAll()
App->>S: setProjects(savedProjects)
Note over S,IW: Session state transition (background session)
S-->>IW: subscribe callback fires (prevState, newState)
IW->>IW: shouldCreateInboxItem(prevState, newState, sessionId, activeId)
IW->>IS: addItem({ id, sessionId, reason, projectName, ... })
IS->>IS: deduplicate by (sessionId + reason)
Note over PS,App: User clicks inbox item
PS->>S: setActiveSessionId(item.sessionId) [raw — no group chat dismissal]
PS->>S: setActiveProjectId(item.projectId)
PS->>IS: dismissItem(item.id)
PS->>IS: dismissAllForSession(item.sessionId)
Note over App: User navigates via setActiveSessionId wrapper
App->>App: setActiveGroupChatId(null)
App->>S: setActiveSessionIdFromContext(id)
App->>IS: dismissAllForSession(id)
Note over App: Project store change → debounced persist
S-->>App: useProjectRestoration debounce (2000ms)
App->>App: window.maestro.projects.setAll(projects)
Last reviewed commit: 8873b3e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+47
to
+48
| const handleClick = useCallback(() => { | ||
| onNavigate(item); |
There was a problem hiding this comment.
Stale relative time — never refreshes
timeAgo is computed once via useMemo(() => formatRelativeTime(item.timestamp), [item.timestamp]). Because item.timestamp never changes after the item is created, this value is frozen at the moment the component first mounts. An item that arrived an hour ago will continue to display "just now" until the component is unmounted and remounted for an unrelated reason.
A minimal fix is to drive the display with a periodic timer:
Suggested change
| const handleClick = useCallback(() => { | |
| onNavigate(item); | |
| const [timeAgo, setTimeAgo] = useState(() => formatRelativeTime(item.timestamp)); | |
| useEffect(() => { | |
| const update = () => setTimeAgo(formatRelativeTime(item.timestamp)); | |
| const id = setInterval(update, 60_000); // refresh every minute | |
| return () => clearInterval(id); | |
| }, [item.timestamp]); |
Don't forget to add the useState/useEffect imports at the top of the file.
Comment on lines
+38
to
+44
| // Deduplicate: don't add if same session+reason already exists | ||
| const exists = s.items.some( | ||
| (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason | ||
| ); | ||
| if (exists) return s; | ||
| // Insert at beginning (newest first) so items are pre-sorted | ||
| return { items: [item, ...s.items] }; |
There was a problem hiding this comment.
Deduplication silently drops subsequent completions for the same session
The deduplication key is (sessionId, reason). Consider this sequence:
- Session A finishes (
busy → idle). AnInboxItem{reason:'finished'}is added. - User does not click the item — it stays in the inbox.
- Session A starts work again and finishes a second time (
busy → idle). addItemis called again for the same(sessionId, 'finished')pair — the early-return kicks in and the second completion is silently dropped.
The user is left with the stale first-completion item in the inbox and never learns that a second run completed. A more correct approach would be to replace the existing item (updating its id, tabId, and timestamp) rather than ignoring the new one:
Suggested change
| // Deduplicate: don't add if same session+reason already exists | |
| const exists = s.items.some( | |
| (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason | |
| ); | |
| if (exists) return s; | |
| // Insert at beginning (newest first) so items are pre-sorted | |
| return { items: [item, ...s.items] }; | |
| addItem: (item) => | |
| set((s) => { | |
| const existingIndex = s.items.findIndex( | |
| (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason | |
| ); | |
| if (existingIndex !== -1) { | |
| // Replace stale entry with the fresh one (keeps list position) | |
| const updated = [...s.items]; | |
| updated[existingIndex] = item; | |
| return { items: updated }; | |
| } | |
| return { items: [item, ...s.items] }; | |
| }), |
Comment on lines
+43
to
+56
| }, [sessions]); | ||
|
|
||
| const handleSelectProject = useCallback( | ||
| (projectId: string) => { | ||
| setActiveProjectId(projectId); | ||
| // When switching projects, select the first session in the new project | ||
| const projectSessions = sessions.filter((s) => s.projectId === projectId); | ||
| if (projectSessions.length > 0) { | ||
| setActiveSessionId(projectSessions[0].id); | ||
| } | ||
| }, | ||
| [setActiveProjectId, setActiveSessionId, sessions] | ||
| ); | ||
|
|
There was a problem hiding this comment.
Navigation bypasses the group-chat-dismissal wrapper in App.tsx
Both handleSelectProject and handleNavigateToInboxItem call the raw setActiveSessionId pulled straight from useSessionStore. In App.tsx, however, setActiveSessionId is wrapped in a useCallback that also calls setActiveGroupChatId(null) to dismiss any open group chat on navigation:
// App.tsx
const setActiveSessionId = useCallback(
(id: string) => {
setActiveGroupChatId(null); // 👈 this never runs when called from ProjectSidebar
setActiveSessionIdFromContext(id);
useInboxStore.getState().dismissAllForSession(id);
},
[...]
);If a user has an active group chat open and then clicks a project or an inbox item in the sidebar, the group chat panel stays visible instead of switching to the selected session. The wrapped version should be threaded down via a prop (e.g. onSelectSession) rather than the store's raw setter being used directly.
Comment on lines
2279
to
2281
| // Helper functions | ||
| getActiveTab, | ||
| }); |
There was a problem hiding this comment.
Dead-code import and @ts-expect-error signal unfinished migration
SessionList is imported (renamed to _SessionList) but never rendered. useSessionListProps is still called but its result is silently discarded under @ts-expect-error. This means:
- Any side-effects inside
useSessionListPropsstill run on every render. - The suppressed error hides whatever real type mismatch exists, making future refactors harder.
If SessionList is fully replaced by ProjectSidebar, the import, the _sessionListProps variable, and the useSessionListProps call should be removed entirely before merging.
Comment on lines
+64
to
+71
| dismissItem(item.id); | ||
| // Also dismiss any other items for this session | ||
| dismissAllForSession(item.sessionId); | ||
| }, | ||
| [setActiveProjectId, setActiveSessionId, dismissItem, dismissAllForSession] | ||
| ); | ||
|
|
||
| const handleProjectContextMenu = useCallback( |
There was a problem hiding this comment.
dismissItem is redundant before dismissAllForSession
dismissAllForSession(item.sessionId) already removes every item whose sessionId matches — including the specific item being passed to dismissItem(item.id). Calling both is harmless but adds a superfluous store update. Remove the dismissItem call (and the dismissItem from the store subscription) to keep the logic clear.
Suggested change
| dismissItem(item.id); | |
| // Also dismiss any other items for this session | |
| dismissAllForSession(item.sessionId); | |
| }, | |
| [setActiveProjectId, setActiveSessionId, dismissItem, dismissAllForSession] | |
| ); | |
| const handleProjectContextMenu = useCallback( | |
| // Switch to the project | |
| setActiveProjectId(item.projectId); | |
| // Switch to the session | |
| setActiveSessionId(item.sessionId); | |
| // Dismiss all items for this session at once | |
| dismissAllForSession(item.sessionId); |
Comment on lines
+53
to
+59
| * Creates the projects persistence API object for preload exposure | ||
| */ | ||
| export function createProjectsApi() { | ||
| return { | ||
| getAll: () => ipcRenderer.invoke('projects:getAll'), | ||
| setAll: (projects: any[]) => ipcRenderer.invoke('projects:setAll', projects), | ||
| }; |
There was a problem hiding this comment.
any[] type loses type safety on the IPC boundary
createProjectsApi uses any[] for the projects parameter:
setAll: (projects: any[]) => ipcRenderer.invoke('projects:setAll', projects),The groups equivalent uses the same pattern, but since Project is already imported in the broader codebase, this can and should be typed properly:
Suggested change
| * Creates the projects persistence API object for preload exposure | |
| */ | |
| export function createProjectsApi() { | |
| return { | |
| getAll: () => ipcRenderer.invoke('projects:getAll'), | |
| setAll: (projects: any[]) => ipcRenderer.invoke('projects:setAll', projects), | |
| }; | |
| import type { Project } from '../../../shared/types'; | |
| // ... | |
| export function createProjectsApi() { | |
| return { | |
| getAll: (): Promise<Project[]> => ipcRenderer.invoke('projects:getAll'), | |
| setAll: (projects: Project[]) => ipcRenderer.invoke('projects:setAll', projects), | |
| }; | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (10)
.claude/memory/agents.md (1)
19-26: Add the SSH-remote integration step to this checklist.Right now this “add a new agent” flow stops at parser/storage wiring, so someone following it can still miss the required remote-execution path. Please call out the
sshRemoteConfig.enabledcheck pluswrapSpawnWithSsh()/binaryName/custom-config handling here.Based on learnings, "When spawning agents with SSH support, ensure the correct agent type is used (don't hardcode 'claude-code'), custom agent configuration is passed through, and the agent's binaryName is used for remote execution rather than local paths."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/memory/agents.md around lines 19 - 26, Update the "Adding a New Agent" checklist to include the SSH-remote integration step: when supporting remote execution check sshRemoteConfig.enabled and call wrapSpawnWithSsh(), ensure you pass the agent's binaryName (not a local path) to the SSH wrapper, forward any custom agent configuration through (don’t hardcode the agent type like 'claude-code'), and ensure the spawn path/arguments use the agent's binaryName and custom config handling so remote execution mirrors local behavior..claude/memory/features.md (1)
30-30: Capitalize "Markdown" as a proper noun.When referring to the formatting language, "Markdown" should be capitalized.
📝 Proposed fix
-- File-based document runner: markdown docs with checkbox tasks +- File-based document runner: Markdown docs with checkbox tasks🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/memory/features.md at line 30, The phrase "File-based document runner: markdown docs with checkbox tasks" uses "markdown" lowercase; update that occurrence in .claude/memory/features.md (the string "File-based document runner: markdown docs with checkbox tasks") to capitalize "Markdown" so it reads "File-based document runner: Markdown docs with checkbox tasks"..claude/memory/testing.md (1)
13-24: Add language specifier to fenced code block.The directory structure code block should have a language specifier for consistent rendering. Use
textorplaintextfor non-code content.📝 Proposed fix
-``` +```text src/__tests__/ ├── cli/ # CLI command + service tests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/memory/testing.md around lines 13 - 24, Update the fenced code block that shows the test directory tree (the block containing "src/__tests__/ ├── cli/ ...") to include a language specifier for consistent rendering; change the opening fence from ``` to ```text (or ```plaintext) so the directory tree renders as plain text in .claude/memory/testing.md.src/__tests__/renderer/stores/projectStore.test.ts (1)
10-18: Consider usingPartial<Project>instead ofPartial<any>for type safety.The mock factory uses
Partial<any>, which defeats TypeScript's type checking. Using the actualProjecttype would catch mismatches between test data and the real type.💡 Proposed type improvement
+import type { Project } from '../../../shared/types'; + -function createMockProject(overrides: Partial<any> = {}) { +function createMockProject(overrides: Partial<Project> = {}): Project { return { id: overrides.id ?? `project-${Math.random().toString(36).slice(2, 8)}`, name: overrides.name ?? 'Test Project', repoPath: overrides.repoPath ?? '/test/repo', createdAt: overrides.createdAt ?? Date.now(), ...overrides, - }; + } as Project; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/stores/projectStore.test.ts` around lines 10 - 18, The createMockProject test factory currently types its parameter as Partial<any>; change that to Partial<Project> so TypeScript enforces the real Project shape—update the function signature to createMockProject(overrides: Partial<Project> = {}) and add the appropriate import or type reference for Project, leaving the implementation (id/name/repoPath/createdAt defaults and the spread of overrides) unchanged.src/main/preload/settings.ts (2)
52-60: Use a typed parameter instead ofany[]for type safety.The
createGroupsApiusesGroup[]for its parameter, butcreateProjectsApiusesany[]. This inconsistency reduces type safety at the IPC boundary.💡 Proposed fix
+import type { Project } from '../../shared/types'; + /** * Creates the projects persistence API object for preload exposure */ export function createProjectsApi() { return { getAll: () => ipcRenderer.invoke('projects:getAll'), - setAll: (projects: any[]) => ipcRenderer.invoke('projects:setAll', projects), + setAll: (projects: Project[]) => ipcRenderer.invoke('projects:setAll', projects), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/preload/settings.ts` around lines 52 - 60, The setAll parameter in createProjectsApi is using any[]; change it to a typed Project[] for consistency and type safety: update createProjectsApi's setAll signature from (projects: any[]) => ... to (projects: Project[]) => ipcRenderer.invoke('projects:setAll', projects), and import or reference the Project type used elsewhere (or the same module that provides Group) at the top of the file so the symbol Project is available to the preload API.
1-8: Update the file header comment to include projects.The module doc comment lists settings, sessions, and groups but doesn't mention the newly added projects namespace.
📝 Proposed doc update
/** * Preload API for settings and persistence * - * Provides the window.maestro.settings, sessions, and groups namespaces for: + * Provides the window.maestro.settings, sessions, groups, and projects namespaces for: * - Application settings persistence * - Session list persistence * - Group list persistence + * - Project list persistence */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/preload/settings.ts` around lines 1 - 8, Update the file header comment to mention the new projects namespace: modify the top module doc that currently lists "window.maestro.settings, sessions, and groups" to include "projects" (e.g., "window.maestro.settings, sessions, groups, and projects") and add a short description line under the bullet list for "Project list persistence" so the header accurately documents the projects namespace..claude/memory/ipc-api.md (1)
32-55: Add the newprojectsnamespace to the documentation.The documentation lists key IPC namespaces but omits the
projectsnamespace added in this PR. This could cause confusion for future developers.📝 Proposed addition
| Namespace | Purpose | |-----------|---------| | `settings` | Get/set/getAll app settings | -| `sessions` / `groups` | Agent and group persistence | +| `sessions` / `groups` / `projects` | Agent, group, and project persistence | | `process` | spawn, write, interrupt, kill, resize |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/memory/ipc-api.md around lines 32 - 55, Add the new `projects` IPC namespace to the "Key Namespaces" table by inserting a table row for `projects` under the same format as the other entries; for example: | `projects` | Project CRUD and workspace/project management | so future devs know this namespace exists and its purpose (update the Key Namespaces table where the other namespaces are listed and keep formatting consistent).src/renderer/components/ProjectSidebar/InboxSection.tsx (2)
93-106: Addtype="button"to prevent unintended form submissions.While unlikely in this context, buttons without an explicit
typedefault totype="submit", which can cause issues if the component is ever placed inside a form.🛡️ Proposed fix
<button onClick={handleClear} + type="button" style={{🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/ProjectSidebar/InboxSection.tsx` around lines 93 - 106, The Clear button in InboxSection.tsx lacks an explicit type, which defaults to "submit" and can trigger unintended form submissions; update the <button> element that uses the onClick handler handleClear to include type="button" so it behaves as a non-submitting control (locate the button within the InboxSection component where handleClear is passed and add the type attribute).
44-54: Clickable header lacks keyboard accessibility.The header
divresponds to clicks but cannot be operated via keyboard. Users navigating with Tab cannot toggle the collapse state.♿ Proposed fix to add keyboard support
<div onClick={toggleCollapsed} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleCollapsed(); + } + }} + tabIndex={0} + role="button" + aria-expanded={!collapsed} style={{ display: 'flex', alignItems: 'center', justifyContent: 'space-between', padding: '8px 12px', cursor: 'pointer', userSelect: 'none', }} + className="outline-none" >Based on learnings: "For focus-related UI bugs, add tabIndex={0} or tabIndex={-1}, add outline-none class".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/ProjectSidebar/InboxSection.tsx` around lines 44 - 54, The header div with onClick={toggleCollapsed} in InboxSection lacks keyboard accessibility; make it operable via keyboard by adding tabIndex={0}, role="button", aria-expanded (based on the collapsed state), and an onKeyDown handler that invokes toggleCollapsed when Enter or Space is pressed, and keep focus styling/outline control (e.g., outline-none class or equivalent) so keyboard users can focus and toggle the header; update the JSX in InboxSection where the clickable div is defined to include these attributes and handler.src/renderer/hooks/project/useProjectRestoration.ts (1)
180-183: Persistence errors are logged but not reported.Same issue as above - errors during persistence are caught and logged but not re-thrown. Consider using Sentry's
captureExceptionfor visibility into persistence failures without crashing the app.Proposed fix
const timer = setTimeout(() => { - window.maestro.projects.setAll(projects).catch((err: unknown) => { - console.error('[useProjectRestoration] Failed to persist projects:', err); - }); + window.maestro.projects.setAll(projects).catch((err: unknown) => { + console.error('[useProjectRestoration] Failed to persist projects:', err); + // Report to Sentry for observability without blocking the user + if (typeof Sentry !== 'undefined') { + Sentry.captureException(err, { tags: { context: 'project-persistence' } }); + } + }); }, PERSISTENCE_DEBOUNCE_MS);As per coding guidelines: "Use Sentry utilities like captureException() and captureMessage() for explicit error reporting with context."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/project/useProjectRestoration.ts` around lines 180 - 183, The catch block inside the setTimeout in useProjectRestoration.ts (where window.maestro.projects.setAll(projects) is called) only logs persistence errors to console; update the catch to also report the error to Sentry by calling captureException(err) (and optionally captureMessage with context) so failures are recorded for monitoring—locate the anonymous catch in the timer callback and add Sentry.captureException(err) with a concise contextual message referencing the projects persistence operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/memory/data-model.md:
- Around line 3-18: The memory doc describes the old 11-store layout; update it
to reflect the new project/inbox navigation by replacing the store list with the
current stores (add projectStore and inboxStore, remove or rename any deprecated
stores), update `sessionStore` to include `projectId` on Session objects, and
add the new projects persistence file and its purpose; also revise the sections
referenced (lines ~19-36 and ~47-55) so descriptions, key state (e.g.,
`projects[]`, `activeProjectId`, `inboxItems[]`), and purposes match the new
architecture and navigation model, ensuring identifiers like sessionStore,
projectStore, inboxStore, Session.projectId, and the projects persistence file
are mentioned.
In `@src/__tests__/renderer/hooks/useProjectRestoration.test.ts`:
- Around line 434-456: Update the test for useProjectRestoration to stop
asserting only on console.error and instead verify that unexpected
projects.getAll() failures are propagated/reported: replace the console.error
spy with a spy/assertion against the error-reporting mechanism (e.g.,
Sentry.captureException) or assert that rendering/initialization of
useProjectRestoration throws/rejects when
mockProjectsApi.getAll.mockRejectedValue is set; keep the renderHook call and
the vi.advanceTimersByTimeAsync usage inside act, but wrap the
renderHook/initialization in an expect(...).rejects/toThrow or check the Sentry
spy was invoked with the error so the test enforces propagation/reporting rather
than silent swallowing.
In `@src/renderer/components/ProjectSidebar/InboxItem.tsx`:
- Around line 54-98: Replace the outer clickable div with a semantic,
keyboard-focusable <button> (or make it a div with role="button" and tabindex=0)
so handleClick works for keyboard users; keep the same inline styles but add
className="outline-none focus:ring-1 focus:ring-offset-1" (or equivalent focus
styles) so tab focus is visible. Also expose the visual-only status
(reasonColor) as readable text for assistive tech by including the reason label
in the rendered content or an aria-label (e.g., include item.reason or map
finished/error/waiting_input to text) near the colored dot (the span using
reasonColor) so screen readers can announce why the item is in the inbox; ensure
onKeyDown handles Enter/Space to call handleClick if you use role/button. Keep
existing content (item.tabName, item.projectName, timeAgo) and styles otherwise.
In `@src/renderer/components/ProjectSidebar/ProjectItem.tsx`:
- Around line 38-91: ProjectItem currently only handles mouse events so keyboard
users can't select projects; update the root div (the element with
onClick={handleClick} and onContextMenu={handleContextMenu}) to be
keyboard-focusable by adding tabIndex={0} and add an onKeyDown handler that
triggers the same selection logic: call handleClick when Enter or Space is
pressed (prevent default for Space), and keep existing focus/hover styling
behavior; ensure you reference the ProjectItem root div and reuse the existing
handleClick handler to avoid duplicating selection logic.
In `@src/renderer/components/ProjectSidebar/ProjectSidebar.tsx`:
- Around line 45-53: The handleSelectProject callback updates activeProjectId
but leaves activeSessionId unchanged when the chosen project has no sessions;
modify handleSelectProject so after computing projectSessions (from
sessions.filter(...)) you explicitly clear the session selection when
projectSessions.length === 0 by calling setActiveSessionId with a neutral value
(null/undefined/empty id consistent with the app's session state type) and
otherwise set the first session id as it already does; update any related types
or callers if necessary to accept the cleared value.
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts`:
- Around line 270-285: The shortcut handler in useMainKeyboardHandler (the
ctx.isShortcut branch for 'cycleProjectNext') does a modulo using
projects.length which can be zero; add a guard at the start of that branch to
return early if projects.length === 0 (or !projects || projects.length === 0) so
you never compute (idx + 1) % projects.length; locate the block that calls
useProjectStore.getState(), check projects length and bail out before computing
idx/next or calling setActiveProjectId / ctx.setActiveSessionId, then leave the
rest of the logic (finding projectSessions and calling trackShortcut) unchanged.
- Around line 286-301: The cycleProjectPrev branch lacks a guard for an empty
projects array; update the handler in useMainKeyboardHandler (the ctx.isShortcut
check for 'cycleProjectPrev') to early-return if
useProjectStore.getState().projects.length === 0 (same pattern as
cycleProjectNext), so you don't compute idx/prev or index into projects when
projects is empty; keep using projects, activeProjectId, setActiveProjectId and
then proceed to setActiveProjectId and ctx.setActiveSessionId as before.
In `@src/renderer/hooks/project/useProjectRestoration.ts`:
- Around line 163-165: The catch on loadProjects in useProjectRestoration is
swallowing unexpected errors; change the handler so it still logs but re-throws
(or remove the catch so the promise rejection bubbles) to allow Sentry to
capture failures. Locate the loadProjects().catch(...) call inside
useProjectRestoration and either re-throw the caught error after calling
console.error (e.g., throw err) or remove the .catch entirely and let the
rejection propagate, keeping logging for known recoverable errors only.
In `@src/renderer/stores/inboxStore.ts`:
- Around line 36-45: The addItem handler currently drops a new incoming item if
a duplicate (same sessionId+reason) exists; instead, when a duplicate is found
in addItem (in inboxStore.ts) remove the existing entry and insert the incoming
item at the front so the alert is refreshed and moves to top. Concretely: in
addItem's set callback detect the existing index via existing.sessionId ===
item.sessionId && existing.reason === item.reason, if found create a new items
array that excludes that existing entry and returns { items: [item, ...rest] };
otherwise insert the item normally. This ensures timestamp/projectName/tabName
from the new event replace the stale row and keeps items deduplicated and
pre-sorted.
In `@src/renderer/types/index.ts`:
- Around line 505-509: The Session interface currently allows projectId to be
optional; make projectId a required string on the Session interface (change
projectId?: string to projectId: string in Session) and then update all code
paths that construct or restore sessions (e.g., functions like createSession,
restoreSession, migrateSession or any deserializers) to always supply a valid
projectId (generate or map to a default project where necessary) and update any
related derived types or validators to reflect the non-optional projectId
invariant.
---
Nitpick comments:
In @.claude/memory/agents.md:
- Around line 19-26: Update the "Adding a New Agent" checklist to include the
SSH-remote integration step: when supporting remote execution check
sshRemoteConfig.enabled and call wrapSpawnWithSsh(), ensure you pass the agent's
binaryName (not a local path) to the SSH wrapper, forward any custom agent
configuration through (don’t hardcode the agent type like 'claude-code'), and
ensure the spawn path/arguments use the agent's binaryName and custom config
handling so remote execution mirrors local behavior.
In @.claude/memory/features.md:
- Line 30: The phrase "File-based document runner: markdown docs with checkbox
tasks" uses "markdown" lowercase; update that occurrence in
.claude/memory/features.md (the string "File-based document runner: markdown
docs with checkbox tasks") to capitalize "Markdown" so it reads "File-based
document runner: Markdown docs with checkbox tasks".
In @.claude/memory/ipc-api.md:
- Around line 32-55: Add the new `projects` IPC namespace to the "Key
Namespaces" table by inserting a table row for `projects` under the same format
as the other entries; for example: | `projects` | Project CRUD and
workspace/project management | so future devs know this namespace exists and its
purpose (update the Key Namespaces table where the other namespaces are listed
and keep formatting consistent).
In @.claude/memory/testing.md:
- Around line 13-24: Update the fenced code block that shows the test directory
tree (the block containing "src/__tests__/ ├── cli/ ...") to include a language
specifier for consistent rendering; change the opening fence from ``` to ```text
(or ```plaintext) so the directory tree renders as plain text in
.claude/memory/testing.md.
In `@src/__tests__/renderer/stores/projectStore.test.ts`:
- Around line 10-18: The createMockProject test factory currently types its
parameter as Partial<any>; change that to Partial<Project> so TypeScript
enforces the real Project shape—update the function signature to
createMockProject(overrides: Partial<Project> = {}) and add the appropriate
import or type reference for Project, leaving the implementation
(id/name/repoPath/createdAt defaults and the spread of overrides) unchanged.
In `@src/main/preload/settings.ts`:
- Around line 52-60: The setAll parameter in createProjectsApi is using any[];
change it to a typed Project[] for consistency and type safety: update
createProjectsApi's setAll signature from (projects: any[]) => ... to (projects:
Project[]) => ipcRenderer.invoke('projects:setAll', projects), and import or
reference the Project type used elsewhere (or the same module that provides
Group) at the top of the file so the symbol Project is available to the preload
API.
- Around line 1-8: Update the file header comment to mention the new projects
namespace: modify the top module doc that currently lists
"window.maestro.settings, sessions, and groups" to include "projects" (e.g.,
"window.maestro.settings, sessions, groups, and projects") and add a short
description line under the bullet list for "Project list persistence" so the
header accurately documents the projects namespace.
In `@src/renderer/components/ProjectSidebar/InboxSection.tsx`:
- Around line 93-106: The Clear button in InboxSection.tsx lacks an explicit
type, which defaults to "submit" and can trigger unintended form submissions;
update the <button> element that uses the onClick handler handleClear to include
type="button" so it behaves as a non-submitting control (locate the button
within the InboxSection component where handleClear is passed and add the type
attribute).
- Around line 44-54: The header div with onClick={toggleCollapsed} in
InboxSection lacks keyboard accessibility; make it operable via keyboard by
adding tabIndex={0}, role="button", aria-expanded (based on the collapsed
state), and an onKeyDown handler that invokes toggleCollapsed when Enter or
Space is pressed, and keep focus styling/outline control (e.g., outline-none
class or equivalent) so keyboard users can focus and toggle the header; update
the JSX in InboxSection where the clickable div is defined to include these
attributes and handler.
In `@src/renderer/hooks/project/useProjectRestoration.ts`:
- Around line 180-183: The catch block inside the setTimeout in
useProjectRestoration.ts (where window.maestro.projects.setAll(projects) is
called) only logs persistence errors to console; update the catch to also report
the error to Sentry by calling captureException(err) (and optionally
captureMessage with context) so failures are recorded for monitoring—locate the
anonymous catch in the timer callback and add Sentry.captureException(err) with
a concise contextual message referencing the projects persistence operation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67b6179f-7670-4a62-b0b0-2d90e5ab9a8f
📒 Files selected for processing (45)
.claude/memory/agents.md.claude/memory/build-deploy.md.claude/memory/data-model.md.claude/memory/features.md.claude/memory/ipc-api.md.claude/memory/patterns.md.claude/memory/performance.md.claude/memory/pitfalls.md.claude/memory/platform.md.claude/memory/testing.md.claude/memory/wizard.mdCLAUDE.mddocs/plans/2026-03-10-project-centric-navigation-design.mddocs/plans/2026-03-10-project-centric-navigation-plan.mdsrc/__tests__/main/ipc/handlers/persistence.test.tssrc/__tests__/renderer/hooks/useInboxWatcher.test.tssrc/__tests__/renderer/hooks/useProjectRestoration.test.tssrc/__tests__/renderer/stores/inboxStore.test.tssrc/__tests__/renderer/stores/projectStore.test.tssrc/__tests__/renderer/stores/sessionStore.test.tssrc/main/index.tssrc/main/ipc/handlers/index.tssrc/main/ipc/handlers/persistence.tssrc/main/preload/index.tssrc/main/preload/settings.tssrc/main/stores/defaults.tssrc/main/stores/getters.tssrc/main/stores/index.tssrc/main/stores/instances.tssrc/main/stores/types.tssrc/renderer/App.tsxsrc/renderer/components/ProjectSidebar/InboxItem.tsxsrc/renderer/components/ProjectSidebar/InboxSection.tsxsrc/renderer/components/ProjectSidebar/ProjectItem.tsxsrc/renderer/components/ProjectSidebar/ProjectSidebar.tsxsrc/renderer/components/ProjectSidebar/index.tssrc/renderer/constants/shortcuts.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/project/useProjectRestoration.tssrc/renderer/hooks/useInboxWatcher.tssrc/renderer/stores/inboxStore.tssrc/renderer/stores/projectStore.tssrc/renderer/stores/sessionStore.tssrc/renderer/types/index.tssrc/shared/types.ts
Comment on lines
+3
to
+18
| ## Zustand Stores (11 total, `src/renderer/stores/`) | ||
|
|
||
| | Store | Purpose | Key state | | ||
| |-------|---------|-----------| | ||
| | `sessionStore` | Sessions, groups, active session | `sessions[]`, `groups[]`, `activeSessionId` | | ||
| | `settingsStore` | User preferences | themes, fonts, agent config, SSH remotes | | ||
| | `modalStore` | 50+ modal visibility + data | Registry pattern, not 50 fields | | ||
| | `tabStore` | Tab management | unified tab order, closed tab history | | ||
| | `agentStore` | Agent detection/capabilities | detected agents, capabilities map | | ||
| | `uiStore` | UI toggles | right panel tab, focus area | | ||
| | `batchStore` | Batch runner state | progress, docs, tasks | | ||
| | `groupChatStore` | Group chat | messages, participants | | ||
| | `notificationStore` | Toast queue | notifications[] | | ||
| | `operationStore` | Long-running ops | merge/transfer progress | | ||
| | `fileExplorerStore` | File tree | expansion state, scroll | | ||
|
|
There was a problem hiding this comment.
Update this data-model doc to match the new project/inbox architecture.
It still documents the pre-PR shape: “11” stores, no projectStore/inboxStore, no Session.projectId, and no projects persistence file. That makes this memory doc misleading for anyone implementing follow-up changes on top of the new navigation model.
Also applies to: 19-36, 47-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/memory/data-model.md around lines 3 - 18, The memory doc describes
the old 11-store layout; update it to reflect the new project/inbox navigation
by replacing the store list with the current stores (add projectStore and
inboxStore, remove or rename any deprecated stores), update `sessionStore` to
include `projectId` on Session objects, and add the new projects persistence
file and its purpose; also revise the sections referenced (lines ~19-36 and
~47-55) so descriptions, key state (e.g., `projects[]`, `activeProjectId`,
`inboxItems[]`), and purposes match the new architecture and navigation model,
ensuring identifiers like sessionStore, projectStore, inboxStore,
Session.projectId, and the projects persistence file are mentioned.
Comment on lines
+434
to
+456
| it('handles errors gracefully without crashing', async () => { | ||
| const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); | ||
| mockProjectsApi.getAll.mockRejectedValue(new Error('Disk read failed')); | ||
|
|
||
| useSessionStore.setState({ | ||
| initialLoadComplete: true, | ||
| sessions: [], | ||
| activeSessionId: '', | ||
| }); | ||
|
|
||
| // Should not throw | ||
| renderHook(() => useProjectRestoration()); | ||
|
|
||
| await act(async () => { | ||
| await vi.advanceTimersByTimeAsync(0); | ||
| }); | ||
|
|
||
| expect(consoleSpy).toHaveBeenCalledWith( | ||
| '[useProjectRestoration] Failed to load/migrate projects:', | ||
| expect.any(Error) | ||
| ); | ||
|
|
||
| consoleSpy.mockRestore(); |
There was a problem hiding this comment.
Don't lock in console.error-only handling for restore failures.
A rejected projects.getAll() is an unexpected startup failure. This test currently codifies “log and continue”, which means the left bar can come up empty with no Sentry signal. Please assert reporting/propagation instead of treating silent recovery as the desired behavior.
As per coding guidelines, "Let exceptions bubble up to Sentry rather than silently swallowing errors in try-catch blocks. Only explicitly handle known, recoverable error modes and re-throw unexpected exceptions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/renderer/hooks/useProjectRestoration.test.ts` around lines 434
- 456, Update the test for useProjectRestoration to stop asserting only on
console.error and instead verify that unexpected projects.getAll() failures are
propagated/reported: replace the console.error spy with a spy/assertion against
the error-reporting mechanism (e.g., Sentry.captureException) or assert that
rendering/initialization of useProjectRestoration throws/rejects when
mockProjectsApi.getAll.mockRejectedValue is set; keep the renderHook call and
the vi.advanceTimersByTimeAsync usage inside act, but wrap the
renderHook/initialization in an expect(...).rejects/toThrow or check the Sentry
spy was invoked with the error so the test enforces propagation/reporting rather
than silent swallowing.
Comment on lines
+54
to
+98
| <div | ||
| onClick={handleClick} | ||
| style={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| padding: '6px 12px', | ||
| cursor: 'pointer', | ||
| borderRadius: 4, | ||
| gap: 8, | ||
| minHeight: 36, | ||
| backgroundColor: 'transparent', | ||
| transition: 'background-color 0.1s', | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.currentTarget.style.backgroundColor = theme.colors.bgActivity; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.currentTarget.style.backgroundColor = 'transparent'; | ||
| }} | ||
| > | ||
| <span style={{ color: reasonColor, fontSize: 10, flexShrink: 0 }}>{'\u25CF'}</span> | ||
| <div style={{ flex: 1, minWidth: 0 }}> | ||
| <div | ||
| style={{ | ||
| color: theme.colors.textMain, | ||
| fontSize: 12, | ||
| fontWeight: 500, | ||
| whiteSpace: 'nowrap', | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| }} | ||
| > | ||
| {item.tabName} | ||
| </div> | ||
| <div | ||
| style={{ | ||
| color: theme.colors.textDim, | ||
| fontSize: 10, | ||
| whiteSpace: 'nowrap', | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| }} | ||
| > | ||
| {item.projectName} · {timeAgo} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the relevant section
head -n 100 src/renderer/components/ProjectSidebar/InboxItem.tsx | tail -n +50Repository: RunMaestro/Maestro
Length of output: 1242
🏁 Script executed:
# Check for REASON_LABELS definition in the file
rg "REASON_LABELS" src/renderer/components/ProjectSidebar/InboxItem.tsx -B 2 -A 2Repository: RunMaestro/Maestro
Length of output: 128
🏁 Script executed:
# Search for any other usages of REASON_LABELS in the codebase to understand the structure
rg "REASON_LABELS" src/ --type tsx --type ts -B 1 -A 1Repository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Find REASON_LABELS definition without type restriction
rg "REASON_LABELS\s*=" src/ -A 5Repository: RunMaestro/Maestro
Length of output: 483
🏁 Script executed:
# Check usage of tabIndex in renderer components to understand the pattern
rg "tabIndex" src/renderer/components/ -B 2 -A 2 --max-count 10Repository: RunMaestro/Maestro
Length of output: 50374
🏁 Script executed:
# Check if button elements are used in similar sidebar/list contexts
rg "<button" src/renderer/components/ProjectSidebar/ -B 1 -A 3Repository: RunMaestro/Maestro
Length of output: 816
Use a keyboard-focusable button and include the reason in text.
This row is mouse-only, and finished/error/waiting_input are differentiated only by color. Keyboard users can't activate it, and assistive tech users can't tell why the item is in the inbox.
💡 Suggested fix
- <div
+ <button
+ type="button"
onClick={handleClick}
+ aria-label={`${REASON_LABELS[item.reason]}: ${item.tabName} in ${item.projectName}, ${timeAgo}`}
style={{
display: 'flex',
alignItems: 'center',
+ width: '100%',
padding: '6px 12px',
cursor: 'pointer',
borderRadius: 4,
gap: 8,
minHeight: 36,
- backgroundColor: 'transparent',
+ background: 'none',
+ backgroundColor: 'transparent',
+ border: 'none',
+ textAlign: 'left',
transition: 'background-color 0.1s',
}}
onMouseEnter={(e) => {
e.currentTarget.style.backgroundColor = theme.colors.bgActivity;
}}
onMouseLeave={(e) => {
e.currentTarget.style.backgroundColor = 'transparent';
}}
+ className="outline-none focus:ring-1 focus:ring-offset-1"
>
- <span style={{ color: reasonColor, fontSize: 10, flexShrink: 0 }}>{'\u25CF'}</span>
+ <span aria-hidden="true" style={{ color: reasonColor, fontSize: 10, flexShrink: 0 }}>
+ {'\u25CF'}
+ </span>
<div style={{ flex: 1, minWidth: 0 }}>
<div
style={{
color: theme.colors.textMain,
@@
}}
>
- {item.projectName} · {timeAgo}
+ {REASON_LABELS[item.reason]} · {item.projectName} · {timeAgo}
</div>
</div>
- </div>
+ </button>Per coding guidelines, add outline-none class and focus styling for keyboard users (the diff above adds className="outline-none focus:ring-1 focus:ring-offset-1" to provide visible focus indication on tab).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/ProjectSidebar/InboxItem.tsx` around lines 54 - 98,
Replace the outer clickable div with a semantic, keyboard-focusable <button> (or
make it a div with role="button" and tabindex=0) so handleClick works for
keyboard users; keep the same inline styles but add className="outline-none
focus:ring-1 focus:ring-offset-1" (or equivalent focus styles) so tab focus is
visible. Also expose the visual-only status (reasonColor) as readable text for
assistive tech by including the reason label in the rendered content or an
aria-label (e.g., include item.reason or map finished/error/waiting_input to
text) near the colored dot (the span using reasonColor) so screen readers can
announce why the item is in the inbox; ensure onKeyDown handles Enter/Space to
call handleClick if you use role/button. Keep existing content (item.tabName,
item.projectName, timeAgo) and styles otherwise.
Comment on lines
+38
to
+91
| return ( | ||
| <div | ||
| onClick={handleClick} | ||
| onContextMenu={handleContextMenu} | ||
| style={{ | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| padding: '8px 12px', | ||
| cursor: 'pointer', | ||
| borderRadius: 4, | ||
| borderLeft: project.color ? `3px solid ${project.color}` : '3px solid transparent', | ||
| backgroundColor: isActive ? `${theme.colors.accent}20` : 'transparent', | ||
| boxShadow: isActive ? `inset 2px 0 0 ${theme.colors.accent}` : 'none', | ||
| transition: 'background-color 0.1s', | ||
| gap: 8, | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| if (!isActive) { | ||
| e.currentTarget.style.backgroundColor = theme.colors.bgActivity; | ||
| } | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| if (!isActive) { | ||
| e.currentTarget.style.backgroundColor = 'transparent'; | ||
| } | ||
| }} | ||
| > | ||
| <div style={{ flex: 1, minWidth: 0 }}> | ||
| <div | ||
| style={{ | ||
| color: isActive ? theme.colors.textMain : theme.colors.textDim, | ||
| fontSize: 13, | ||
| fontWeight: isActive ? 600 : 400, | ||
| whiteSpace: 'nowrap', | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| }} | ||
| > | ||
| {project.name} | ||
| </div> | ||
| </div> | ||
| {sessionCount > 0 && ( | ||
| <span | ||
| style={{ | ||
| color: theme.colors.textDim, | ||
| fontSize: 10, | ||
| flexShrink: 0, | ||
| }} | ||
| > | ||
| {sessionCount} | ||
| </span> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Add keyboard accessibility for project selection.
The component handles mouse clicks but lacks keyboard support. Users navigating with keyboard cannot select projects. As per coding guidelines for focus-related UI, add tabIndex and keyboard handlers.
♿ Proposed accessibility fix
+ const handleKeyDown = useCallback(
+ (e: React.KeyboardEvent) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onSelect(project.id);
+ }
+ },
+ [project.id, onSelect]
+ );
+
return (
<div
+ role="button"
+ tabIndex={0}
onClick={handleClick}
onContextMenu={handleContextMenu}
+ onKeyDown={handleKeyDown}
style={{📝 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
| return ( | |
| <div | |
| onClick={handleClick} | |
| onContextMenu={handleContextMenu} | |
| style={{ | |
| display: 'flex', | |
| alignItems: 'center', | |
| padding: '8px 12px', | |
| cursor: 'pointer', | |
| borderRadius: 4, | |
| borderLeft: project.color ? `3px solid ${project.color}` : '3px solid transparent', | |
| backgroundColor: isActive ? `${theme.colors.accent}20` : 'transparent', | |
| boxShadow: isActive ? `inset 2px 0 0 ${theme.colors.accent}` : 'none', | |
| transition: 'background-color 0.1s', | |
| gap: 8, | |
| }} | |
| onMouseEnter={(e) => { | |
| if (!isActive) { | |
| e.currentTarget.style.backgroundColor = theme.colors.bgActivity; | |
| } | |
| }} | |
| onMouseLeave={(e) => { | |
| if (!isActive) { | |
| e.currentTarget.style.backgroundColor = 'transparent'; | |
| } | |
| }} | |
| > | |
| <div style={{ flex: 1, minWidth: 0 }}> | |
| <div | |
| style={{ | |
| color: isActive ? theme.colors.textMain : theme.colors.textDim, | |
| fontSize: 13, | |
| fontWeight: isActive ? 600 : 400, | |
| whiteSpace: 'nowrap', | |
| overflow: 'hidden', | |
| textOverflow: 'ellipsis', | |
| }} | |
| > | |
| {project.name} | |
| </div> | |
| </div> | |
| {sessionCount > 0 && ( | |
| <span | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontSize: 10, | |
| flexShrink: 0, | |
| }} | |
| > | |
| {sessionCount} | |
| </span> | |
| )} | |
| </div> | |
| ); | |
| const handleKeyDown = useCallback( | |
| (e: React.KeyboardEvent) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| onSelect(project.id); | |
| } | |
| }, | |
| [project.id, onSelect] | |
| ); | |
| return ( | |
| <div | |
| role="button" | |
| tabIndex={0} | |
| onClick={handleClick} | |
| onContextMenu={handleContextMenu} | |
| onKeyDown={handleKeyDown} | |
| style={{ | |
| display: 'flex', | |
| alignItems: 'center', | |
| padding: '8px 12px', | |
| cursor: 'pointer', | |
| borderRadius: 4, | |
| borderLeft: project.color ? `3px solid ${project.color}` : '3px solid transparent', | |
| backgroundColor: isActive ? `${theme.colors.accent}20` : 'transparent', | |
| boxShadow: isActive ? `inset 2px 0 0 ${theme.colors.accent}` : 'none', | |
| transition: 'background-color 0.1s', | |
| gap: 8, | |
| }} | |
| onMouseEnter={(e) => { | |
| if (!isActive) { | |
| e.currentTarget.style.backgroundColor = theme.colors.bgActivity; | |
| } | |
| }} | |
| onMouseLeave={(e) => { | |
| if (!isActive) { | |
| e.currentTarget.style.backgroundColor = 'transparent'; | |
| } | |
| }} | |
| > | |
| <div style={{ flex: 1, minWidth: 0 }}> | |
| <div | |
| style={{ | |
| color: isActive ? theme.colors.textMain : theme.colors.textDim, | |
| fontSize: 13, | |
| fontWeight: isActive ? 600 : 400, | |
| whiteSpace: 'nowrap', | |
| overflow: 'hidden', | |
| textOverflow: 'ellipsis', | |
| }} | |
| > | |
| {project.name} | |
| </div> | |
| </div> | |
| {sessionCount > 0 && ( | |
| <span | |
| style={{ | |
| color: theme.colors.textDim, | |
| fontSize: 10, | |
| flexShrink: 0, | |
| }} | |
| > | |
| {sessionCount} | |
| </span> | |
| )} | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/ProjectSidebar/ProjectItem.tsx` around lines 38 - 91,
ProjectItem currently only handles mouse events so keyboard users can't select
projects; update the root div (the element with onClick={handleClick} and
onContextMenu={handleContextMenu}) to be keyboard-focusable by adding
tabIndex={0} and add an onKeyDown handler that triggers the same selection
logic: call handleClick when Enter or Space is pressed (prevent default for
Space), and keep existing focus/hover styling behavior; ensure you reference the
ProjectItem root div and reuse the existing handleClick handler to avoid
duplicating selection logic.
Comment on lines
+45
to
+53
| const handleSelectProject = useCallback( | ||
| (projectId: string) => { | ||
| setActiveProjectId(projectId); | ||
| // When switching projects, select the first session in the new project | ||
| const projectSessions = sessions.filter((s) => s.projectId === projectId); | ||
| if (projectSessions.length > 0) { | ||
| setActiveSessionId(projectSessions[0].id); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Selecting an empty project keeps the previous session active.
New projects can exist before any session is attached. In that case this only updates activeProjectId; activeSessionId stays pointed at the last project, so the sidebar and main content can disagree about what's selected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/ProjectSidebar/ProjectSidebar.tsx` around lines 45 -
53, The handleSelectProject callback updates activeProjectId but leaves
activeSessionId unchanged when the chosen project has no sessions; modify
handleSelectProject so after computing projectSessions (from
sessions.filter(...)) you explicitly clear the session selection when
projectSessions.length === 0 by calling setActiveSessionId with a neutral value
(null/undefined/empty id consistent with the app's session state type) and
otherwise set the first session id as it already does; update any related types
or callers if necessary to accept the cleared value.
Comment on lines
+270
to
+285
| } else if (ctx.isShortcut(e, 'cycleProjectNext')) { | ||
| // Cycle to next project | ||
| e.preventDefault(); | ||
| const { projects, activeProjectId, setActiveProjectId } = useProjectStore.getState(); | ||
| const idx = projects.findIndex((p) => p.id === activeProjectId); | ||
| const next = (idx + 1) % projects.length; | ||
| if (projects[next]) { | ||
| setActiveProjectId(projects[next].id); | ||
| const projectSessions = ctx.sessions.filter( | ||
| (s: Session) => s.projectId === projects[next].id | ||
| ); | ||
| if (projectSessions.length > 0) { | ||
| ctx.setActiveSessionId(projectSessions[0].id); | ||
| } | ||
| } | ||
| trackShortcut('cycleProjectNext'); |
There was a problem hiding this comment.
Guard against empty projects array before cycling.
When projects.length === 0, the modulo operation produces NaN, and while projects[NaN] is undefined (so the if fails safely), this is confusing and relies on implicit behavior.
🛡️ Proposed guard
} else if (ctx.isShortcut(e, 'cycleProjectNext')) {
// Cycle to next project
e.preventDefault();
const { projects, activeProjectId, setActiveProjectId } = useProjectStore.getState();
+ if (projects.length === 0) {
+ trackShortcut('cycleProjectNext');
+ return;
+ }
const idx = projects.findIndex((p) => p.id === activeProjectId);
const next = (idx + 1) % projects.length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts` around lines 270 -
285, The shortcut handler in useMainKeyboardHandler (the ctx.isShortcut branch
for 'cycleProjectNext') does a modulo using projects.length which can be zero;
add a guard at the start of that branch to return early if projects.length === 0
(or !projects || projects.length === 0) so you never compute (idx + 1) %
projects.length; locate the block that calls useProjectStore.getState(), check
projects length and bail out before computing idx/next or calling
setActiveProjectId / ctx.setActiveSessionId, then leave the rest of the logic
(finding projectSessions and calling trackShortcut) unchanged.
Comment on lines
+286
to
+301
| } else if (ctx.isShortcut(e, 'cycleProjectPrev')) { | ||
| // Cycle to previous project | ||
| e.preventDefault(); | ||
| const { projects, activeProjectId, setActiveProjectId } = useProjectStore.getState(); | ||
| const idx = projects.findIndex((p) => p.id === activeProjectId); | ||
| const prev = (idx - 1 + projects.length) % projects.length; | ||
| if (projects[prev]) { | ||
| setActiveProjectId(projects[prev].id); | ||
| const projectSessions = ctx.sessions.filter( | ||
| (s: Session) => s.projectId === projects[prev].id | ||
| ); | ||
| if (projectSessions.length > 0) { | ||
| ctx.setActiveSessionId(projectSessions[0].id); | ||
| } | ||
| } | ||
| trackShortcut('cycleProjectPrev'); |
There was a problem hiding this comment.
Same guard needed for cycleProjectPrev.
Apply the same empty-array guard for consistency and clarity.
🛡️ Proposed guard
} else if (ctx.isShortcut(e, 'cycleProjectPrev')) {
// Cycle to previous project
e.preventDefault();
const { projects, activeProjectId, setActiveProjectId } = useProjectStore.getState();
+ if (projects.length === 0) {
+ trackShortcut('cycleProjectPrev');
+ return;
+ }
const idx = projects.findIndex((p) => p.id === activeProjectId);
const prev = (idx - 1 + projects.length) % projects.length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts` around lines 286 -
301, The cycleProjectPrev branch lacks a guard for an empty projects array;
update the handler in useMainKeyboardHandler (the ctx.isShortcut check for
'cycleProjectPrev') to early-return if
useProjectStore.getState().projects.length === 0 (same pattern as
cycleProjectNext), so you don't compute idx/prev or index into projects when
projects is empty; keep using projects, activeProjectId, setActiveProjectId and
then proceed to setActiveProjectId and ctx.setActiveSessionId as before.
Comment on lines
+163
to
+165
| loadProjects().catch((err) => { | ||
| console.error('[useProjectRestoration] Failed to load/migrate projects:', err); | ||
| }); |
There was a problem hiding this comment.
Errors are swallowed instead of bubbling to Sentry.
The catch block logs errors but doesn't re-throw, preventing Sentry from capturing unexpected failures during project loading/migration. Per coding guidelines, only explicitly handle known, recoverable error modes and re-throw unexpected exceptions.
Proposed fix
- loadProjects().catch((err) => {
- console.error('[useProjectRestoration] Failed to load/migrate projects:', err);
- });
+ loadProjects().catch((err) => {
+ console.error('[useProjectRestoration] Failed to load/migrate projects:', err);
+ throw err; // Let Sentry capture unexpected failures
+ });As per coding guidelines: "Let exceptions bubble up to Sentry rather than silently swallowing errors in try-catch blocks."
📝 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
| loadProjects().catch((err) => { | |
| console.error('[useProjectRestoration] Failed to load/migrate projects:', err); | |
| }); | |
| loadProjects().catch((err) => { | |
| console.error('[useProjectRestoration] Failed to load/migrate projects:', err); | |
| throw err; // Let Sentry capture unexpected failures | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/hooks/project/useProjectRestoration.ts` around lines 163 - 165,
The catch on loadProjects in useProjectRestoration is swallowing unexpected
errors; change the handler so it still logs but re-throws (or remove the catch
so the promise rejection bubbles) to allow Sentry to capture failures. Locate
the loadProjects().catch(...) call inside useProjectRestoration and either
re-throw the caught error after calling console.error (e.g., throw err) or
remove the .catch entirely and let the rejection propagate, keeping logging for
known recoverable errors only.
Comment on lines
+36
to
+45
| addItem: (item) => | ||
| set((s) => { | ||
| // Deduplicate: don't add if same session+reason already exists | ||
| const exists = s.items.some( | ||
| (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason | ||
| ); | ||
| if (exists) return s; | ||
| // Insert at beginning (newest first) so items are pre-sorted | ||
| return { items: [item, ...s.items] }; | ||
| }), |
There was a problem hiding this comment.
Refresh duplicate inbox items instead of dropping the new event.
If the same session hits the same reason again before its old inbox row is dismissed, this keeps the stale row unchanged. The fresh alert never moves back to the top, and the old timestamp/projectName/tabName keep showing.
💡 Suggested fix
addItem: (item) =>
set((s) => {
- // Deduplicate: don't add if same session+reason already exists
- const exists = s.items.some(
- (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason
- );
- if (exists) return s;
- // Insert at beginning (newest first) so items are pre-sorted
- return { items: [item, ...s.items] };
+ const remaining = s.items.filter(
+ (existing) => !(existing.sessionId === item.sessionId && existing.reason === item.reason)
+ );
+ return { items: [item, ...remaining] };
}),📝 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
| addItem: (item) => | |
| set((s) => { | |
| // Deduplicate: don't add if same session+reason already exists | |
| const exists = s.items.some( | |
| (existing) => existing.sessionId === item.sessionId && existing.reason === item.reason | |
| ); | |
| if (exists) return s; | |
| // Insert at beginning (newest first) so items are pre-sorted | |
| return { items: [item, ...s.items] }; | |
| }), | |
| addItem: (item) => | |
| set((s) => { | |
| const remaining = s.items.filter( | |
| (existing) => !(existing.sessionId === item.sessionId && existing.reason === item.reason) | |
| ); | |
| return { items: [item, ...remaining] }; | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/stores/inboxStore.ts` around lines 36 - 45, The addItem handler
currently drops a new incoming item if a duplicate (same sessionId+reason)
exists; instead, when a duplicate is found in addItem (in inboxStore.ts) remove
the existing entry and insert the incoming item at the front so the alert is
refreshed and moves to top. Concretely: in addItem's set callback detect the
existing index via existing.sessionId === item.sessionId && existing.reason ===
item.reason, if found create a new items array that excludes that existing entry
and returns { items: [item, ...rest] }; otherwise insert the item normally. This
ensures timestamp/projectName/tabName from the new event replace the stale row
and keeps items deduplicated and pre-sorted.
Comment on lines
505
to
509
| export interface Session { | ||
| id: string; | ||
| groupId?: string; | ||
| projectId?: string; // Links to Project. Optional during migration period. | ||
| name: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make project membership a steady-state session invariant.
Now that the left bar is project-centric, leaving Session.projectId optional means new session creation paths can still compile without assigning a project. That keeps the old legacy shape alive in the main renderer type and makes orphaned sessions much easier to introduce. I’d strongly consider narrowing during migration/restoration and making projectId required in the steady-state Session type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/types/index.ts` around lines 505 - 509, The Session interface
currently allows projectId to be optional; make projectId a required string on
the Session interface (change projectId?: string to projectId: string in
Session) and then update all code paths that construct or restore sessions
(e.g., functions like createSession, restoreSession, migrateSession or any
deserializers) to always supply a valid projectId (generate or map to a default
project where necessary) and update any related derived types or validators to
reflect the non-optional projectId invariant.
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
Key Changes
src/renderer/components/ProjectSidebar/— InboxItem, InboxSection, ProjectItem, ProjectSidebarsrc/renderer/stores/projectStore.ts+inboxStore.ts— new Zustand storessrc/renderer/hooks/project/useProjectRestoration.ts— startup + migrationsrc/renderer/hooks/useInboxWatcher.ts— state transition → inbox triggerssrc/main/ipc/handlers/persistence.ts—projects:getAll,projects:setAllhandlerssrc/renderer/App.tsx— wired in ProjectSidebar, hooks, keyboard shortcutsCLAUDE.md— updated documentation for navigation modelTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation