refactor: decompose useTabHandlers by domain and AgentSessionsBrowser into coordinator pattern - #1057
Conversation
1,512-line god-component split into shell + barrel + types + 9 components + 8 hooks + 3 utils. Shell reduced to ~433 lines (-71%). 170 new unit tests added across 17 test files; existing 85 integration tests unchanged. Key invariants preserved: dual-mode aggregate stats (claude-code IPC keyed on projectRoot NOT projectPathForSessions - SSH regression guard), tool-call filtering in messagesToLogEntries, token zeroing in buildUsageStats, modal Escape priority, and Cmd+F gating migrated to useEventListener.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Agent Sessions Browser UI (components, hooks, utils, types), comprehensive unit tests, and a refactor of tab-handling into internal helpers and modular hooks plus a thin public composition layer. ChangesAgent Sessions Browser UI and Hooks
Tab Handlers Internal Refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as AgentSessionsBrowser
participant Tabs as useTabHandlers
participant Maestro as window.maestro
User->>Browser: search/select/star/rename/resume
Browser->>Tabs: open/select/close tabs
Browser->>Maestro: agentSessions/claude IPC
Tabs->>Maestro: fs/process/session ops
Maestro-->>Browser: data/events
Maestro-->>Tabs: results
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR decomposes two large god-components -
Confidence Score: 3/5Safe to merge after fixing the star-toggle rollback; the style violations are low-risk but leave behind banned patterns the team explicitly tracks. The star-toggle optimism in
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["AgentSessionsBrowser.tsx\n(shell / coordinator, 433 lines)"] --> B["hooks/"]
A --> C["components/"]
A --> D["utils/"]
B --> B1["useAgentSessionsAggregateStats\n(SSH key invariant)"]
B --> B2["useAgentSessionsSearch\n(debounced IPC)"]
B --> B3["useAgentSessionsRename"]
B --> B4["useAgentSessionsStar\n(no error rollback)"]
B --> B5["useAgentSessionsResume"]
B --> B6["useAgentSessionsAutoView"]
B --> B7["useAgentSessionsFocusRestore\n(useEffect+setTimeout)"]
B --> B8["useAgentSessionsActivityEntries"]
C --> C1["SessionListView"]
C --> C2["SessionDetailHeader"]
C --> C3["SessionDetailStatsPanel"]
C --> C4["SessionMessagesView"]
C --> C5["SessionSearchBar"]
C --> C6["SessionListHeader"]
C --> C7["SessionListStatsBar"]
C --> C8["SearchModeDropdown"]
C --> C9["SessionMessageBubble"]
D --> D1["buildUsageStats\n(token-zeroing on resume)"]
D --> D2["messagesToLogEntries\n(tool-call filter)"]
D --> D3["sessionProjectPath\n(SSH path resolution)"]
|
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/renderer/components/AgentSessionsBrowser/components/SearchModeDropdown.tsx (1)
15-43: 💤 Low valueDerive
getModeIconfromMODESto avoid duplicate mode→icon mapping.The icon mapping lives in two places (
MODESand thegetModeIconswitch). They agree today, but they can drift independently if a mode/icon is added or changed. A single lookup keeps them in sync.♻️ Proposed consolidation
-function getModeIcon(mode: SearchMode) { - switch (mode) { - case 'title': - return Search; - case 'user': - return User; - case 'assistant': - return Bot; - default: - return MessageSquare; - } -} +function getModeIcon(mode: SearchMode) { + return MODES.find((m) => m.mode === mode)?.icon ?? MessageSquare; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/AgentSessionsBrowser/components/SearchModeDropdown.tsx` around lines 15 - 43, The getModeIcon switch duplicates the icon mapping already defined in MODES; replace the switch in getModeIcon with a lookup into MODES (e.g., find the entry where entry.mode === mode) and return its icon, falling back to MessageSquare if not found. Update getModeIcon to reference the const MODES array and preserve the same return type and fallback behavior so adding/changing modes only needs to be done in MODES.src/renderer/hooks/tabs/internal/useAITabHandlers.ts (1)
189-203: ⚡ Quick winRemove leftover
[DEBUG renameTab]logging.These
logger.infocalls are debug artifacts that will add noise in production. Drop them (or gate behind a debug flag) before merge.♻️ Proposed cleanup
const handleRequestTabRename = useCallback((tabId: string) => { - logger.info('[DEBUG renameTab] handleRequestTabRename called', undefined, { tabId }); const { setSessions } = useSessionStore.getState(); const session = selectActiveSession(useSessionStore.getState()); if (!session) { - logger.info('[DEBUG renameTab] no session found'); return; } const tab = session.aiTabs?.find((t) => t.id === tabId); - logger.info('[DEBUG renameTab] tab found:', undefined, [ - !!tab, - { - aiTabCount: session.aiTabs?.length, - tabId, - }, - ]); if (tab) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/tabs/internal/useAITabHandlers.ts` around lines 189 - 203, Remove the leftover debug logger.info calls in the handleRequestTabRename flow: delete the three '[DEBUG renameTab]' logger.info lines (the initial call, the "no session found" log, and the "tab found" log) in useAITabHandlers.ts, or wrap them behind an existing debug flag if you prefer conditional logging; keep the surrounding logic that uses selectActiveSession and setSessions intact (references: handleRequestTabRename, selectActiveSession, useSessionStore.getState(), setSessions).src/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.test.ts (1)
1-1: ⚡ Quick win
afterEachis used without an import, but it won’t break this test run
src/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.test.tscallsafterEach(line 24) without importing it. The default Vitest config setstest.globals: trueinvitest.config.mts, soafterEachis available globally. For consistency with the explicit Vitest imports, consider importingafterEach(or removing the named imports and relying entirely on globals).Optional consistency fix
-import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.test.ts` at line 1, The test file mixes explicit Vitest imports and global usage by calling afterEach without importing it; update the import statement (the one that currently imports describe, it, expect, vi, beforeEach from 'vitest') to also import afterEach so the test consistently uses explicit Vitest imports (or alternatively remove all named imports and rely on globals—preferred: add afterEach to the existing import list).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsFocusRestore.test.ts`:
- Around line 1-29: The test file's vitest import is missing afterEach; update
the import statement that currently reads "import { describe, it, expect, vi,
beforeEach } from 'vitest';" to include afterEach so it becomes "import {
describe, it, expect, vi, beforeEach, afterEach } from 'vitest';" and ensure the
existing afterEach() call in the file uses that imported symbol.
In `@src/renderer/components/AgentSessionsBrowser/AgentSessionsBrowser.tsx`:
- Around line 252-265: The local Escape handling in handleKeyDown (used by
SessionMessagesView and the search input) duplicates the modal-layer Escape
handling provided by useModalLayer and can cause double-close; remove Escape
branches from handleKeyDown so Escape is handled only by the modal layer,
leaving Enter behavior (handleResume) intact for when viewingSession is true,
and ensure clearViewingSession/onClose are only invoked by the modal-layer
listener; update usages of handleKeyDown in SessionMessagesView and the search
input so they no longer rely on it to handle Escape.
In
`@src/renderer/components/AgentSessionsBrowser/components/SessionDetailHeader.tsx`:
- Around line 127-145: The tooltip and hover absolute time are using
viewingSession.timestamp while the visible relative label uses
formatRelativeTime(viewingSession.modifiedAt); update the title and the hover
span to use viewingSession.modifiedAt (e.g., new
Date(viewingSession.modifiedAt).toLocaleString()) so the absolute time matches
the displayed relative time from formatRelativeTime; ensure you reference
viewingSession.modifiedAt consistently in the SessionDetailHeader component.
In
`@src/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.ts`:
- Around line 84-86: In useAgentSessionsRename's catch block (the one that
currently calls logger.error('Failed to rename session:', undefined, error)),
ensure the error is reported to Sentry by calling captureException(error, {
extra: { agentId, sessionId, projectPath: activeSession.projectRoot } }) (or if
you must keep UI state cleanup there, call captureException and then rethrow the
error); update the catch inside the useAgentSessionsRename hook so it both logs
via logger.error and calls captureException with the specified extra context
(agentId, sessionId, projectPath) to ensure failures are captured.
In
`@src/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsSearch.ts`:
- Around line 56-59: In useAgentSessionsSearch.ts update the catch block to
report the caught error to Sentry by importing and calling captureException from
src/utils/sentry.ts (in addition to the existing logger.error and
setSearchResults([])); pass the caught error and a short context object (e.g., {
fn: 'useAgentSessionsSearch', action: 'search' } or relevant search params) to
captureException so failures are tracked in production.
In `@src/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsStar.ts`:
- Around line 37-52: The IPC calls in useAgentSessionsStar (the block that calls
window.maestro.claude.updateSessionStarred and
window.maestro.agentSessions.setSessionStarred using activeSession.projectRoot,
sessionId and isNowStarred) can throw and currently leave local state
inconsistent; wrap those await calls in a try/catch, and on exception revert the
optimistic local state change (e.g., reset whatever state updater you used to
flip the star, such as setIsStarred or the sessions list mutation) so the UI
matches backend, then report the exception to Sentry (e.g.,
Sentry.captureException(err) or your app’s error reporting facade) and rethrow
or return appropriately so the error is tracked.
- Around line 28-35: The current toggle logic reads starredSessions from the
closure causing a race condition; update the handler to use the functional state
updater of setStarredSessions(prev => { ... }) so you operate on the latest
state (use prev to create a new Set, add/delete sessionId, and return it), and
remove starredSessions from the hook's dependency array since it will no longer
be referenced directly in the closure; keep references to setStarredSessions and
sessionId to locate the code.
In `@src/renderer/hooks/tabs/internal/useFilePreviewTabHandlers.ts`:
- Line 417: The handlers in useFilePreviewTabHandlers.ts currently abort
navigation on empty-file content by using the truthy check "if (!content)
return"; update these guards to specifically check for null (use "if (content
=== null) return") so empty strings are allowed and only a true read failure
stops navigation—make this change for the three occurrences flagged (the two
guards near lines 416-417, 464-465 and 510-511) to match the existing behavior
in handleReloadFileTab which already uses content === null.
In `@src/renderer/hooks/tabs/internal/useTerminalTabHandlers.ts`:
- Around line 27-41: The promise catch for
window.maestro.process.isTerminalBusy(ptySessionId) currently swallows errors;
modify the catch to report the error (e.g., call captureException or
captureMessage with the caught error) and then fallback to
closeTerminalTab(tabId). Locate the isTerminalBusy call in
useTerminalTabHandlers (the block that uses useModalStore.getState().openModal
and closeTerminalTab) and add error reporting inside the .catch handler before
invoking closeTerminalTab.
---
Nitpick comments:
In
`@src/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.test.ts`:
- Line 1: The test file mixes explicit Vitest imports and global usage by
calling afterEach without importing it; update the import statement (the one
that currently imports describe, it, expect, vi, beforeEach from 'vitest') to
also import afterEach so the test consistently uses explicit Vitest imports (or
alternatively remove all named imports and rely on globals—preferred: add
afterEach to the existing import list).
In
`@src/renderer/components/AgentSessionsBrowser/components/SearchModeDropdown.tsx`:
- Around line 15-43: The getModeIcon switch duplicates the icon mapping already
defined in MODES; replace the switch in getModeIcon with a lookup into MODES
(e.g., find the entry where entry.mode === mode) and return its icon, falling
back to MessageSquare if not found. Update getModeIcon to reference the const
MODES array and preserve the same return type and fallback behavior so
adding/changing modes only needs to be done in MODES.
In `@src/renderer/hooks/tabs/internal/useAITabHandlers.ts`:
- Around line 189-203: Remove the leftover debug logger.info calls in the
handleRequestTabRename flow: delete the three '[DEBUG renameTab]' logger.info
lines (the initial call, the "no session found" log, and the "tab found" log) in
useAITabHandlers.ts, or wrap them behind an existing debug flag if you prefer
conditional logging; keep the surrounding logic that uses selectActiveSession
and setSessions intact (references: handleRequestTabRename, selectActiveSession,
useSessionStore.getState(), setSessions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e054f7e3-55e5-4310-b2a5-68a5f39c43ea
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (64)
src/__tests__/renderer/components/AgentSessionsBrowser/components/SearchModeDropdown.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/components/SessionDetailStatsPanel.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/components/SessionListHeader.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/components/SessionListStatsBar.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/components/SessionMessageBubble.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/components/SessionMessagesView.test.tsxsrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsActivityEntries.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsAggregateStats.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsAutoView.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsFocusRestore.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsResume.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsSearch.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsStar.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/utils/buildUsageStats.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/utils/messagesToLogEntries.test.tssrc/__tests__/renderer/components/AgentSessionsBrowser/utils/sessionProjectPath.test.tssrc/__tests__/renderer/hooks/tabs/internal/browserTabHelpers.test.tssrc/__tests__/renderer/hooks/tabs/internal/filePreviewTabHelpers.test.tssrc/__tests__/renderer/hooks/tabs/internal/testUtils.tssrc/__tests__/renderer/hooks/tabs/internal/unifiedCloseHelpers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useAITabHandlers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useBrowserTabHandlers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useFilePreviewTabHandlers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useScrollLogHandlers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useTabDerivedState.test.tssrc/__tests__/renderer/hooks/tabs/internal/useTerminalTabHandlers.test.tssrc/__tests__/renderer/hooks/tabs/internal/useUnifiedTabHandlers.test.tssrc/renderer/components/AgentSessionsBrowser.tsxsrc/renderer/components/AgentSessionsBrowser/AgentSessionsBrowser.tsxsrc/renderer/components/AgentSessionsBrowser/components/SearchModeDropdown.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionDetailHeader.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionDetailStatsPanel.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionListHeader.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionListStatsBar.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionListView.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionMessageBubble.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionMessagesView.tsxsrc/renderer/components/AgentSessionsBrowser/components/SessionSearchBar.tsxsrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsActivityEntries.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsAggregateStats.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsAutoView.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsFocusRestore.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsRename.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsResume.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsSearch.tssrc/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsStar.tssrc/renderer/components/AgentSessionsBrowser/index.tssrc/renderer/components/AgentSessionsBrowser/types.tssrc/renderer/components/AgentSessionsBrowser/utils/buildUsageStats.tssrc/renderer/components/AgentSessionsBrowser/utils/messagesToLogEntries.tssrc/renderer/components/AgentSessionsBrowser/utils/sessionProjectPath.tssrc/renderer/hooks/tabs/internal/browserTabHelpers.tssrc/renderer/hooks/tabs/internal/filePreviewTabHelpers.tssrc/renderer/hooks/tabs/internal/types.tssrc/renderer/hooks/tabs/internal/unifiedCloseHelpers.tssrc/renderer/hooks/tabs/internal/useAITabHandlers.tssrc/renderer/hooks/tabs/internal/useBrowserTabHandlers.tssrc/renderer/hooks/tabs/internal/useFilePreviewTabHandlers.tssrc/renderer/hooks/tabs/internal/useScrollLogHandlers.tssrc/renderer/hooks/tabs/internal/useTabDerivedState.tssrc/renderer/hooks/tabs/internal/useTerminalTabHandlers.tssrc/renderer/hooks/tabs/internal/useUnifiedTabHandlers.tssrc/renderer/hooks/tabs/useTabHandlers.ts
💤 Files with no reviewable changes (1)
- src/renderer/components/AgentSessionsBrowser.tsx
| const newStarred = new Set(starredSessions); | ||
| const isNowStarred = !newStarred.has(sessionId); | ||
| if (isNowStarred) { | ||
| newStarred.add(sessionId); | ||
| } else { | ||
| newStarred.delete(sessionId); | ||
| } | ||
| setStarredSessions(newStarred); |
There was a problem hiding this comment.
Fix race condition in state update.
Line 28 reads starredSessions from the closure. If the user rapidly clicks star on multiple sessions before React batches state updates, both callbacks will see the same stale starredSessions value, causing lost updates. Use the functional form of setState to read the latest state.
🔒 Proposed fix
- const newStarred = new Set(starredSessions);
- const isNowStarred = !newStarred.has(sessionId);
- if (isNowStarred) {
- newStarred.add(sessionId);
- } else {
- newStarred.delete(sessionId);
- }
- setStarredSessions(newStarred);
+ let isNowStarred = false;
+ setStarredSessions((prev) => {
+ const newStarred = new Set(prev);
+ isNowStarred = !newStarred.has(sessionId);
+ if (isNowStarred) {
+ newStarred.add(sessionId);
+ } else {
+ newStarred.delete(sessionId);
+ }
+ return newStarred;
+ });Also update the dependency array to remove starredSessions since it's no longer read directly in the closure:
- [starredSessions, activeSession?.projectRoot, agentId, onUpdateTab]
+ [activeSession?.projectRoot, agentId, onUpdateTab]📝 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.
| const newStarred = new Set(starredSessions); | |
| const isNowStarred = !newStarred.has(sessionId); | |
| if (isNowStarred) { | |
| newStarred.add(sessionId); | |
| } else { | |
| newStarred.delete(sessionId); | |
| } | |
| setStarredSessions(newStarred); | |
| let isNowStarred = false; | |
| setStarredSessions((prev) => { | |
| const newStarred = new Set(prev); | |
| isNowStarred = !newStarred.has(sessionId); | |
| if (isNowStarred) { | |
| newStarred.add(sessionId); | |
| } else { | |
| newStarred.delete(sessionId); | |
| } | |
| return newStarred; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/AgentSessionsBrowser/hooks/useAgentSessionsStar.ts`
around lines 28 - 35, The current toggle logic reads starredSessions from the
closure causing a race condition; update the handler to use the functional state
updater of setStarredSessions(prev => { ... }) so you operate on the latest
state (use prev to create a new Set, add/delete sessionId, and return it), and
remove starredSessions from the hook's dependency array since it will no longer
be referenced directly in the closure; keep references to setStarredSessions and
sessionId to locate the code.
|
Hi @reachrazamair — thank you for this contribution! 🙏 This is a genuinely high-quality decomposition: collapsing the 2,044-line I reviewed the PR alongside the Greptile and CodeRabbit passes and verified the findings against the code. There's one item I'd like fixed before approval, plus a few smaller things worth a look. No merge conflicts — the branch is clean against 🔴 Please fix before merge
Both bots flagged this and I confirmed it by reading the code. Two small changes resolve it:
🟡 Worth addressing (your call)
⚪ Minor nits
Note on one bot suggestionGreptile recommends replacing the Thanks again — the star rollback is the only real blocker; everything else is polish. Ping me once that's in and I'll take another pass. 🎬 |
…lers - Fix missing afterEach imports in focusRestore and rename test files - Remove duplicate Escape handling from handleKeyDown; modal layer is sole handler - Fix SessionDetailHeader tooltip to use modifiedAt consistently with the visible label - Add captureException to useAgentSessionsRename and useAgentSessionsSearch catch blocks - Fix useAgentSessionsStar: functional state updater prevents stale-closure race, try/catch reverts optimistic star on IPC failure - Fix useFilePreviewTabHandlers nav guards: !content -> content === null so empty files navigate correctly - Remove leftover [DEBUG renameTab] logger.info calls from useAITabHandlers - Add captureMessage(warning) to useTerminalTabHandlers isTerminalBusy catch - Replace getModeIcon switch with MODES.find lookup in SearchModeDropdown
React schedules state updater callbacks asynchronously, so deriving isNowStarred as a side-effect inside setStarredSessions caused the IPC call to always receive false. Read starredSessions synchronously from the closure to compute isNowStarred, then use the functional updater only for the state mutation to keep rapid multi-session toggles correct.
# Conflicts: # src/renderer/hooks/tabs/useTabHandlers.ts
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
2 participants
Summary
Two consecutive renderer decompositions that continue the §4.x god-component
reduction campaign. Both follow the established coordinator pattern
(shell + barrel + domain hooks + leaf components), pass the full test suite
with zero regressions, and make no behavior changes.
Commit 1 -
useTabHandlersdecomposition by domainsrc/renderer/hooks/tabs/useTabHandlers.tswas a 2,044-line monolith handlingevery tab type (AI, file-preview, browser, terminal, scroll-log) in a single
flat file. It is now a 35-line re-exporting shell backed by 11 focused modules
under
src/renderer/hooks/tabs/internal/:useAITabHandlers.tsuseFilePreviewTabHandlers.tsuseUnifiedTabHandlers.tsuseScrollLogHandlers.tsuseBrowserTabHandlers.tsunifiedCloseHelpers.tstypes.tsuseTabDerivedState.tsuseTerminalTabHandlers.tsfilePreviewTabHelpers.tsbrowserTabHelpers.tsShell reduction: 2,044 → 35 lines (-98%)
11 new test files added covering all extracted units. All existing
useTabHandlerscallers (App.tsx, MainPanel) import through the unchangedpublic surface - no call site edits required.
Commit 2 -
AgentSessionsBrowserdecompositionsrc/renderer/components/AgentSessionsBrowser.tsxwas a 1,512-linegod-component owning four orthogonal concerns: list view (search, filter,
paginated list, activity graph), detail view (session header, stats grid,
messages), aggregate-stats lifecycle (dual-mode: claude-code progressive IPC
vs other-agent locally computed), and coordination (rename, star, auto-jump,
focus restore, keyboard nav, Cmd+F, modal-layer Escape).
Now a 23-file directory:
AgentSessionsBrowser/
Shell reduction: 1,512 → 433 lines (-71%)
Key invariants preserved and guarded by dedicated regression tests:
onProjectStatsUpdatesubscription is keyed onactiveSession.projectRoot, NOTprojectPathForSessions. Using the remotepath would silently drop all SSH session stats. Two unit tests in
useAgentSessionsAggregateStats.test.tsguard this explicitly.buildUsageStats- resumed tabs receive cost only(all 4 token fields zeroed) so they cannot display a misleading 100% context
bar from stale cumulative totals.
messagesToLogEntries- tool-use messages areexcluded to match live-session behavior (thinking off by default on restore).
document.addEventListenerpair touseEventListener('keydown', handler, { target: document })per CLAUDE.md.85 existing integration tests pass unchanged through the barrel. 170 new unit
tests added across 17 files.
Test plan
npm run test- full suite green (29,964 tests, 0 failures)npx tsc --noEmit- 0 type errorsnpm run lint+npm run lint:eslint- cleannpm run format:check- cleanSummary by CodeRabbit
New Features
Tests
Refactor