feat: little things - shortcuts, worktrees, unread filter, CLI read-only#515
feat: little things - shortcuts, worktrees, unread filter, CLI read-only#515pedramamini wants to merge 22 commits intomainfrom
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds font-size keyboard shortcuts (Cmd+=/+, Cmd+-, Cmd+0) to the main keyboard handler using the settings store with bounds (10–24, default 14); threads a new onQuickCreateWorktree prop through App -> AppModals -> QuickActionsModal and auto-focuses newly created worktree sessions. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (keyboard)
participant KH as KeyboardHandler
participant MS as ModalService
participant SS as SettingsStore
participant TR as Tracker
participant TM as TabManager
User->>KH: Press Cmd+= / Cmd+- / Cmd+0
KH->>MS: Is modal/overlay open?
alt modal/overlay open
KH->>SS: read/update fontSize
SS-->>KH: new fontSize
KH->>TR: track(font-size shortcut)
KH-->>User: consume shortcut (no tab navigation)
else no modal/overlay
KH->>SS: read/update fontSize
SS-->>KH: new fontSize
KH->>TR: track(font-size shortcut)
KH-->>TM: prevent/conflict with tab shortcuts (early return)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/hooks/keyboard/useMainKeyboardHandler.ts (1)
494-520: Add font size shortcuts to the FIXED_SHORTCUTS registry for consistency.The tracking implementation works correctly—
recordShortcutUsage()accepts any string ID without pre-registration. However, the font size shortcuts (fontSizeIncrease,fontSizeDecrease,fontSizeReset) are missing fromsrc/renderer/constants/shortcuts.ts, unlike all other hard-coded shortcuts. These should be added to theFIXED_SHORTCUTSregistry to maintain consistency and ensure they appear in the help dialog.🤖 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 494 - 520, Add the three missing shortcut entries for fontSizeIncrease, fontSizeDecrease, and fontSizeReset to the FIXED_SHORTCUTS registry in shortcuts.ts so they are treated like other hard-coded shortcuts and appear in the help dialog; create entries keyed by "fontSizeIncrease", "fontSizeDecrease", and "fontSizeReset" with appropriate human-readable names (e.g., "Increase font size", "Decrease font size", "Reset font size") and default keybindings matching the handler (Cmd/Ctrl + =/+, Cmd/Ctrl + -, Cmd/Ctrl + 0). Ensure the IDs exactly match the strings used in trackShortcut/recordShortcutUsage in useMainKeyboardHandler (fontSizeIncrease, fontSizeDecrease, fontSizeReset).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts`:
- Around line 494-520: Add the three missing shortcut entries for
fontSizeIncrease, fontSizeDecrease, and fontSizeReset to the FIXED_SHORTCUTS
registry in shortcuts.ts so they are treated like other hard-coded shortcuts and
appear in the help dialog; create entries keyed by "fontSizeIncrease",
"fontSizeDecrease", and "fontSizeReset" with appropriate human-readable names
(e.g., "Increase font size", "Decrease font size", "Reset font size") and
default keybindings matching the handler (Cmd/Ctrl + =/+, Cmd/Ctrl + -, Cmd/Ctrl
+ 0). Ensure the IDs exactly match the strings used in
trackShortcut/recordShortcutUsage in useMainKeyboardHandler (fontSizeIncrease,
fontSizeDecrease, fontSizeReset).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2c141e87-2a54-409b-a2a7-ad624341b342
📒 Files selected for processing (2)
src/__tests__/renderer/hooks/useMainKeyboardHandler.test.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.ts
The custom Electron menu (added to fix Cmd+Shift+{/} tab cycling) removed
the View menu role, which provided native Cmd+=/- zoom and Cmd+0 reset.
This adds font size shortcuts directly in the renderer keyboard handler
using the existing fontSize setting (2px steps, range 10-24). Cmd+0 now
resets font size instead of goToLastTab since zoom reset is standard behavior.
- Add "Create Worktree" action to QuickActionsModal (Cmd+K) for git repo sessions - Resolve to parent session when invoked from a worktree child - Auto-focus newly created worktree sessions after creation
Format symphony-registry.json to fix CI prettier check failure. Add fontSizeIncrease/Decrease/Reset to FIXED_SHORTCUTS registry per CodeRabbit review feedback.
b25709b to
f6e9ebb
Compare
…data RightPanel was consuming flatFileList (FlatTreeNode[]) where it needed tree-structured FileNode[], causing double-flattening and duplicate entries. Added filteredFileTree to the store as the proper bridge between useFileTreeManagement and RightPanel.
Applies provider-specific read-only args from centralized agent definitions (e.g. --permission-mode plan for Claude Code, --sandbox read-only for Codex) without duplicating logic.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/__tests__/cli/services/agent-spawner.test.ts (1)
1078-1112: Consider adding test coverage for CodexreadOnlyMode.The tests verify
readOnlyModebehavior for Claude Code (--permission-mode plan), but there's no corresponding test for Codex which uses different read-only args (--sandbox read-onlyper the context snippets). Adding a similar test for Codex would ensure both agent types handle read-only mode correctly.💡 Example test for Codex readOnlyMode
it('should include read-only args for Codex when readOnlyMode is true', async () => { const resultPromise = spawnAgent('codex', '/project', 'prompt', undefined, { readOnlyMode: true, }); await new Promise((resolve) => setTimeout(resolve, 0)); const [, args] = mockSpawn.mock.calls[0]; // Should include Codex's read-only args from centralized definitions expect(args).toContain('--sandbox'); expect(args).toContain('read-only'); // Should still have base args expect(args).toContain('exec'); expect(args).toContain('--json'); mockStdout.emit('data', Buffer.from('{"type":"init","session_id":"test"}\n')); mockStdout.emit('data', Buffer.from('{"type":"message","content":"Done"}\n')); mockChild.emit('close', 0); await resultPromise; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/cli/services/agent-spawner.test.ts` around lines 1078 - 1112, Add a test to cover Codex readOnlyMode: create a new it block calling spawnAgent('codex', '/project', 'prompt', undefined, { readOnlyMode: true }) then wait a tick and inspect mockSpawn.mock.calls[0][1] to assert it contains Codex read-only flags '--sandbox' and 'read-only' as well as base args like 'exec' and '--json'; finally simulate agent output using mockStdout.emit for init and message events and close the process via mockChild.emit('close', 0) before awaiting the resultPromise. Reference spawnAgent, mockSpawn, mockStdout, and mockChild when adding the test.src/cli/services/agent-spawner.ts (1)
513-527: Consider consolidatingagentSessionIdintoSpawnAgentOptionsto avoid redundancy.The
spawnAgentfunction has both a standaloneagentSessionIdparameter (line 517) and anagentSessionIdfield inSpawnAgentOptions(line 505). Currently only the standalone parameter is used (line 523), whileoptions.agentSessionIdis ignored. This creates confusion about which to use.Consider either:
- Removing
agentSessionIdfromSpawnAgentOptionssince it's not used, or- Deprecating the standalone parameter and using
options.agentSessionIdinsteadFor now, this works correctly since callers pass the session ID via the standalone parameter, but this inconsistency could lead to bugs if callers start using
options.agentSessionIdexpecting it to work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/services/agent-spawner.ts` around lines 513 - 527, The spawnAgent function currently ignores options.agentSessionId; change the function to derive a single sessionId variable like `const sessionId = options?.agentSessionId ?? agentSessionId` and use `sessionId` in the calls to spawnCodexAgent and spawnClaudeAgent (and any other downstream uses) so both callers using the old standalone parameter and callers passing it via SpawnAgentOptions work; after that, update type docs and call sites as desired and optionally remove the standalone parameter from spawnAgent (and adjust callers) once callers have migrated.src/renderer/hooks/git/useFileExplorerEffects.ts (1)
223-224: Align stored tree with the same hidden-files projection used for flattening.
newFlatListis computed fromdisplayTree, butfilteredFileTreeis stored from the pre-filtered input. Consider storingdisplayTreeto keep both representations in sync for downstream consumers.Suggested change
- setFilteredFileTree(filteredFileTree); + setFilteredFileTree(displayTree); setFlatFileList(newFlatList);Also applies to: 238-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/git/useFileExplorerEffects.ts` around lines 223 - 224, The stored tree is kept as filteredFileTree while newFlatList is built from displayTree (filterHiddenFiles(filteredFileTree)), causing mismatch; change the code to store displayTree (the result of filterHiddenFiles) so both the persisted tree and flattened list use the same hidden-files projection. Update occurrences where filteredFileTree is assigned/stored to use displayTree instead (referencing filteredFileTree, displayTree, newFlatList, flattenTree, filterHiddenFiles, expandedSet); apply the same change for the later similar block around the code that mirrors lines 238-239 to keep both representations consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/cli/services/agent-spawner.test.ts`:
- Around line 1078-1112: Add a test to cover Codex readOnlyMode: create a new it
block calling spawnAgent('codex', '/project', 'prompt', undefined, {
readOnlyMode: true }) then wait a tick and inspect mockSpawn.mock.calls[0][1] to
assert it contains Codex read-only flags '--sandbox' and 'read-only' as well as
base args like 'exec' and '--json'; finally simulate agent output using
mockStdout.emit for init and message events and close the process via
mockChild.emit('close', 0) before awaiting the resultPromise. Reference
spawnAgent, mockSpawn, mockStdout, and mockChild when adding the test.
In `@src/cli/services/agent-spawner.ts`:
- Around line 513-527: The spawnAgent function currently ignores
options.agentSessionId; change the function to derive a single sessionId
variable like `const sessionId = options?.agentSessionId ?? agentSessionId` and
use `sessionId` in the calls to spawnCodexAgent and spawnClaudeAgent (and any
other downstream uses) so both callers using the old standalone parameter and
callers passing it via SpawnAgentOptions work; after that, update type docs and
call sites as desired and optionally remove the standalone parameter from
spawnAgent (and adjust callers) once callers have migrated.
In `@src/renderer/hooks/git/useFileExplorerEffects.ts`:
- Around line 223-224: The stored tree is kept as filteredFileTree while
newFlatList is built from displayTree (filterHiddenFiles(filteredFileTree)),
causing mismatch; change the code to store displayTree (the result of
filterHiddenFiles) so both the persisted tree and flattened list use the same
hidden-files projection. Update occurrences where filteredFileTree is
assigned/stored to use displayTree instead (referencing filteredFileTree,
displayTree, newFlatList, flattenTree, filterHiddenFiles, expandedSet); apply
the same change for the later similar block around the code that mirrors lines
238-239 to keep both representations consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 725b190e-0b82-463c-b362-e0cc5cbd1578
📒 Files selected for processing (9)
src/__tests__/cli/commands/send.test.tssrc/__tests__/cli/services/agent-spawner.test.tssrc/__tests__/renderer/hooks/useFileExplorerEffects.test.tssrc/cli/commands/send.tssrc/cli/index.tssrc/cli/services/agent-spawner.tssrc/renderer/components/RightPanel.tsxsrc/renderer/hooks/git/useFileExplorerEffects.tssrc/renderer/stores/fileExplorerStore.ts
…ype pill The session name pill had flex-shrink-0 which prevented it from shrinking when the right panel was narrow, causing the date to overlap the USER/AUTO pill. Now uses flex-shrink so it truncates gracefully and expands when space is available.
Adds a robot face icon button to the right side of the left sidebar bottom panel that toggles "Unread Agent Only" mode, hiding all agents without unread indicators. Reassigns toggleTabUnread to Alt+Shift+U.
Session names used textDim for inactive sessions, which caused agents with expanded worktree drawers to appear visually dimmer due to contrast with the tinted worktree background. Active state is already indicated by accent border and background highlight.
… Notes stats agentCount was using sessionIds.length which counts all history files on disk, including deleted agents and those with no entries in the selected time range. Now tracks which agents actually contribute entries using a Set, matching how the synopsis handler already does it.
handleReplayMessage now captures the current draft text and staged images before calling processInput, then restores them after the send clears the input box.
Use -top-0.5 -right-0.5 positioning to match the Mail icon's notification dot in TabBar, placing it slightly outside button bounds.
Previously the "View history" link in the auto-run status bar was only visible on the autorun tab. Now it appears on all tabs except history.
The session name had max-w-[150px] causing premature truncation even when ample header space was available. The parent flex containers already handle natural truncation via min-w-0 and overflow-hidden.
Directory overlap detection now compares SSH remote IDs so that a local agent and an SSH agent (or two agents on different SSH remotes) sharing the same working directory path are not flagged as conflicting.
The previous fix only checked sshRemoteId and sshRemote.id on sessions, but the canonical source for per-session SSH config is sessionSshRemoteConfig.remoteId. Added getSessionSshRemoteId() helper that checks all three locations in priority order.
The warning banner used light text colors (yellow-300, red-300) hardcoded for dark backgrounds, making text invisible in light themes. Now uses theme.mode to select appropriate contrast colors (yellow-800, red-800) for light mode.
…ote Access to Remote Control Add z-10 to LIVE and hamburger button containers to create proper stacking contexts above the sidebar scroll area. Adjust maxHeight from calc(100vh-90px) to calc(100vh-120px) and add overflow-y-auto to the live overlay panel. Rename Remote Access to Remote Control across docs, UI labels, tour steps, and tests.
Syntax highlighting used vscDarkPlus (dark theme) unconditionally, making code illegible on light themes. Now uses a shared getSyntaxStyle() helper that selects vs (light) or vscDarkPlus (dark/vibe) based on theme mode, matching the pattern mobile code already used. Also adjusted all 6 light theme color palettes to meet WCAG AA contrast (4.5:1) for text on backgrounds. Worst offender was Solarized Light where even textMain failed. Darkened textDim, accent, accentText, success, warning, and error values while preserving original hues.
Working agents should remain visible when filtering by unread, so users can see both active work and unread output at a glance.
When filtering by unread agents, hide groups with no matching sessions and remove New Group buttons to reduce clutter and focus on relevant agents.
Shows a vertically centered Bot icon and "No unread or working agents" message when the filter is active but no agents match.
Summary
A collection of small features, fixes, and improvements across the app.
New Features
Bug Fixes
Files Changed (by feature)
Unread Agents Filter:
Worktree Creation:
CLI Read-Only:
Font Size Shortcuts:
Session Name Pill Fix:
File Tree Dedup:
Test plan