From 059a80b2989f4d072681a86d16da32b94d3cf4bc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:38:10 -0700 Subject: [PATCH] feat(review): add assignable "Send Review Notes to Agent" shortcut (#10027) (#10070) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a keyboard command that opens the "Send notes to an agent" picker for the active worktree's AI diff-review notes, enabling a fully keyboard-driven review flow. Unbound by default; users assign it in Settings → Keyboard Shortcuts. - New `sourceControl.sendReviewNotes` command (scope global, unbound). Set `conflictGroup: 'editor'` so Settings warns on collisions with editor chords (e.g. Add Review Note), not just global ones. - Dispatched from App.tsx's existing global capture handler so it respects the terminal-shortcut policy, the shortcut-recorder guard, and defaultPrevented. - Store thunk `openDiffNotesSendMenuForActiveWorktree` reveals Source Control and requests the notes send menu open; no-op when there are no unsent notes. - Menu opens via a nonce-based store request consumed on mount, TTL-bounded so a request the menu never consumed can't reopen it on a later remount. Co-authored-by: Orca --- src/renderer/src/App.tsx | 10 +++ .../components/editor/DiffNotesSendMenu.tsx | 26 +++++++- .../components/editor/NotesSendMenu.test.tsx | 30 +++++++++ .../src/components/editor/NotesSendMenu.tsx | 18 ++++++ .../right-sidebar/SourceControl.tsx | 1 + src/renderer/src/store/slices/ui.test.ts | 62 +++++++++++++++++++ src/renderer/src/store/slices/ui.ts | 31 ++++++++++ src/shared/keybindings.test.ts | 13 ++++ src/shared/keybindings.ts | 21 +++++++ 9 files changed, 210 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 5f9c5c22d..68a7f8ea3 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -459,6 +459,7 @@ function App(): React.JSX.Element { setRightSidebarTab: s.setRightSidebarTab, showRightSidebarFiles: s.showRightSidebarFiles, showRightSidebarSearch: s.showRightSidebarSearch, + openDiffNotesSendMenuForActiveWorktree: s.openDiffNotesSendMenuForActiveWorktree, setActiveView: s.setActiveView, updateSettings: s.updateSettings, pruneLastVisitedTimestamps: s.pruneLastVisitedTimestamps, @@ -1678,6 +1679,15 @@ function App(): React.JSX.Element { return } + // Unbound by default; opens the active worktree's Source Control notes send picker. Only consumes the chord when there are unsent notes. + if (matchShortcut('sourceControl.sendReviewNotes')) { + if (actions.openDiffNotesSendMenuForActiveWorktree()) { + input.preventDefault() + notifyTerminalCapture('sourceControl.sendReviewNotes') + return + } + } + if (matchShortcut('sidebar.checks.toggle')) { input.preventDefault() notifyTerminalCapture('sidebar.checks.toggle') diff --git a/src/renderer/src/components/editor/DiffNotesSendMenu.tsx b/src/renderer/src/components/editor/DiffNotesSendMenu.tsx index 32638d4fa..8fc9daefb 100644 --- a/src/renderer/src/components/editor/DiffNotesSendMenu.tsx +++ b/src/renderer/src/components/editor/DiffNotesSendMenu.tsx @@ -1,10 +1,14 @@ -import React, { useMemo } from 'react' +import React, { useCallback, useMemo } from 'react' import type { DiffComment } from '../../../../shared/types' import { useAppStore } from '@/store' import { formatDiffComments } from '@/lib/diff-comments-format' import { NotesSendMenu, type NotesSendMenuScope } from './NotesSendMenu' import { translate } from '@/i18n/i18n' +// Why: a keyboard open request the menu never got to consume (e.g. the user +// navigated away before it mounted) must not reopen the menu on a later remount. +const OPEN_REQUEST_TTL_MS = 5000 + export function DiffNotesSendMenu({ worktreeId, groupId, @@ -16,7 +20,8 @@ export function DiffNotesSendMenu({ triggerCount, actionLabel, iconClassName = 'size-3.5', - align = 'end' + align = 'end', + respondToOpenRequest = false }: { worktreeId: string groupId: string @@ -29,8 +34,23 @@ export function DiffNotesSendMenu({ actionLabel?: string iconClassName?: string align?: 'start' | 'center' | 'end' + // When set, this menu opens in response to the store's keyboard-shortcut open + // request. Enable on exactly one instance per worktree to avoid double-open. + respondToOpenRequest?: boolean }): React.JSX.Element { const clearDeliveredDiffComments = useAppStore((s) => s.clearDeliveredDiffComments) + const openRequest = useAppStore((s) => s.diffNotesSendMenuOpenRequest) + const consumeOpenRequest = useAppStore((s) => s.consumeDiffNotesSendMenuOpenRequest) + const openRequestNonce = + respondToOpenRequest && + openRequest?.worktreeId === worktreeId && + Date.now() - openRequest.issuedAt < OPEN_REQUEST_TTL_MS + ? openRequest.nonce + : null + const handleOpenRequestHandled = useCallback( + () => consumeOpenRequest(worktreeId), + [consumeOpenRequest, worktreeId] + ) const unsentNotes = useMemo(() => comments.filter((comment) => !comment.sentAt), [comments]) const unsentPrompt = useMemo(() => formatDiffComments(unsentNotes), [unsentNotes]) const fileNotes = useMemo( @@ -76,6 +96,8 @@ export function DiffNotesSendMenu({ actionLabel={actionLabel} iconClassName={iconClassName} align={align} + openRequestNonce={openRequestNonce} + onOpenRequestHandled={handleOpenRequestHandled} onDelivered={(notes) => void clearDeliveredDiffComments(worktreeId, notes)} /> ) diff --git a/src/renderer/src/components/editor/NotesSendMenu.test.tsx b/src/renderer/src/components/editor/NotesSendMenu.test.tsx index bb3f13e69..d977ec5ae 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.test.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.test.tsx @@ -361,6 +361,36 @@ describe('NotesSendMenu', () => { ) }) + it('opens and reports handled when an open request arrives with deliverable notes', () => { + const onOpenRequestHandled = vi.fn() + renderMenu({ openRequestNonce: 1, onOpenRequestHandled }) + + expect(storeMocks.openAgentSendPopoverTargetMode).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'prompt-all', label: 'All unsent notes' }) + ) + expect(onOpenRequestHandled).toHaveBeenCalledTimes(1) + }) + + it('reports the open request handled without opening when nothing is deliverable', () => { + const onOpenRequestHandled = vi.fn() + renderMenu({ + openRequestNonce: 1, + onOpenRequestHandled, + scopes: [{ id: 'all', label: 'All unsent notes', notes: [], prompt: '' }] + }) + + expect(storeMocks.openAgentSendPopoverTargetMode).not.toHaveBeenCalled() + expect(onOpenRequestHandled).toHaveBeenCalledTimes(1) + }) + + it('ignores a null open request', () => { + const onOpenRequestHandled = vi.fn() + renderMenu({ openRequestNonce: null, onOpenRequestHandled }) + + expect(storeMocks.openAgentSendPopoverTargetMode).not.toHaveBeenCalled() + expect(onOpenRequestHandled).not.toHaveBeenCalled() + }) + it('closes when another target mode becomes active and cleans up on unmount', () => { hookRuntime.states[0] = true storeMocks.state.agentSendPopoverTargetMode = { id: 'some-other-menu' } diff --git a/src/renderer/src/components/editor/NotesSendMenu.tsx b/src/renderer/src/components/editor/NotesSendMenu.tsx index 823a001dd..247a01c4a 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.tsx @@ -40,6 +40,10 @@ export type NotesSendMenuProps = { disabledTooltip?: string iconClassName?: string align?: 'start' | 'center' | 'end' + // A new nonce value asks this menu to open (e.g. from a keyboard shortcut). + // Only a single mounted instance should be driven this way. + openRequestNonce?: number | null + onOpenRequestHandled?: () => void onDelivered: (notes: readonly TNote[]) => void } @@ -64,6 +68,8 @@ export function NotesSendMenu({ disabledTooltip = 'All notes sent', iconClassName = 'size-3.5', align = 'end', + openRequestNonce = null, + onOpenRequestHandled, onDelivered }: NotesSendMenuProps): React.JSX.Element { const openAgentSendPopoverTargetMode = useAppStore((s) => s.openAgentSendPopoverTargetMode) @@ -138,6 +144,18 @@ export function NotesSendMenu({ [closeAgentSendPopoverTargetMode, targetModeId] ) + useEffect(() => { + if (openRequestNonce == null) { + return + } + // Why: only open when notes remain; either way clear the request so a stale + // nonce cannot reopen the menu on a later remount. + if (hasDeliverableNotes && defaultScope) { + handleOpenChange(true) + } + onOpenRequestHandled?.() + }, [openRequestNonce, hasDeliverableNotes, defaultScope, handleOpenChange, onOpenRequestHandled]) + return ( diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 47aeccc03..1f798aa81 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -5525,6 +5525,7 @@ function SourceControlInner(): React.JSX.Element { groupId={activeGroupId ?? activeWorktreeId} comments={diffCommentsForActive} triggerClassName="size-6" + respondToOpenRequest /> {diffCommentCount > 0 && ( diff --git a/src/renderer/src/store/slices/ui.test.ts b/src/renderer/src/store/slices/ui.test.ts index d2a02795a..66c9a1dbb 100644 --- a/src/renderer/src/store/slices/ui.test.ts +++ b/src/renderer/src/store/slices/ui.test.ts @@ -3333,3 +3333,65 @@ describe('createUISlice space navigation', () => { expect(store.getState().activeView).toBe('tasks') }) }) + +describe('openDiffNotesSendMenuForActiveWorktree', () => { + function stubDiffNotesStore( + comments: { sentAt?: number }[], + activeWorktreeId: string | null = 'wt-1' + ): { store: StoreApi; setRightSidebarTab: ReturnType } { + const store = createUIStore() + const setRightSidebarTab = vi.fn() + store.setState({ + activeWorktreeId, + getDiffComments: () => comments, + setRightSidebarTab, + setRightSidebarOpen: vi.fn() + } as unknown as Partial) + return { store, setRightSidebarTab } + } + + it('reveals Source Control and bumps the open request when unsent notes exist', () => { + const { store, setRightSidebarTab } = stubDiffNotesStore([{ sentAt: 10 }, {}]) + + expect(store.getState().openDiffNotesSendMenuForActiveWorktree()).toBe(true) + expect(setRightSidebarTab).toHaveBeenCalledWith('source-control') + expect(store.getState().diffNotesSendMenuOpenRequest).toMatchObject({ + worktreeId: 'wt-1', + nonce: 1 + }) + expect(store.getState().diffNotesSendMenuOpenRequest?.issuedAt).toBeTypeOf('number') + + // A second request increments the nonce so the menu reopens. + expect(store.getState().openDiffNotesSendMenuForActiveWorktree()).toBe(true) + expect(store.getState().diffNotesSendMenuOpenRequest).toMatchObject({ + worktreeId: 'wt-1', + nonce: 2 + }) + }) + + it('is a no-op when every note is already sent', () => { + const { store, setRightSidebarTab } = stubDiffNotesStore([{ sentAt: 10 }]) + + expect(store.getState().openDiffNotesSendMenuForActiveWorktree()).toBe(false) + expect(setRightSidebarTab).not.toHaveBeenCalled() + expect(store.getState().diffNotesSendMenuOpenRequest).toBeNull() + }) + + it('is a no-op when there is no active worktree', () => { + const { store } = stubDiffNotesStore([{}], null) + + expect(store.getState().openDiffNotesSendMenuForActiveWorktree()).toBe(false) + expect(store.getState().diffNotesSendMenuOpenRequest).toBeNull() + }) + + it('clears the request only for the matching worktree', () => { + const { store } = stubDiffNotesStore([{}]) + store.getState().openDiffNotesSendMenuForActiveWorktree() + + store.getState().consumeDiffNotesSendMenuOpenRequest('other-wt') + expect(store.getState().diffNotesSendMenuOpenRequest).not.toBeNull() + + store.getState().consumeDiffNotesSendMenuOpenRequest('wt-1') + expect(store.getState().diffNotesSendMenuOpenRequest).toBeNull() + }) +}) diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 5c371b711..110f51b3c 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -593,6 +593,11 @@ export type UISlice = { openAgentSendPopoverTargetMode: (args: OpenAgentSendPopoverTargetModeArgs) => void closeAgentSendPopoverTargetMode: (id?: string, instanceId?: string) => void sendPromptToSidebarAgentTarget: (paneKey: string) => Promise + /** Bumped to ask the active worktree's Source Control notes send menu to open (keyboard shortcut). `issuedAt` bounds staleness so a request the menu never consumed can't reopen it much later. */ + diffNotesSendMenuOpenRequest: { worktreeId: string; nonce: number; issuedAt: number } | null + /** Reveal Source Control and request its notes send menu open; returns false (no-op) when the active worktree has no unsent notes. */ + openDiffNotesSendMenuForActiveWorktree: () => boolean + consumeDiffNotesSendMenuOpenRequest: (worktreeId: string) => void /** Per-agent "I've looked at this" timestamps (paneKey → ts). A row is unvisited when no ack exists or stateStartedAt is newer than the last ack. Persisted so visited rows don't return bold on relaunch. */ acknowledgedAgentsByPaneKey: Record acknowledgeAgents: (paneKeys: string[]) => void @@ -1008,6 +1013,32 @@ export const createUISlice: StateCreator = (set, get) get().revealWorktreeInSidebar(args.worktreeId, { behavior: 'auto', highlight: true }) } }, + diffNotesSendMenuOpenRequest: null, + openDiffNotesSendMenuForActiveWorktree: () => { + const worktreeId = get().activeWorktreeId + if (!worktreeId) { + return false + } + // Why: no unsent notes means nothing to send, so don't hijack focus or reveal the panel. + if ( + !get() + .getDiffComments(worktreeId) + .some((comment) => !comment.sentAt) + ) { + return false + } + get().setRightSidebarTab('source-control') + get().setRightSidebarOpen(true) + const nonce = (get().diffNotesSendMenuOpenRequest?.nonce ?? 0) + 1 + set({ diffNotesSendMenuOpenRequest: { worktreeId, nonce, issuedAt: Date.now() } }) + return true + }, + consumeDiffNotesSendMenuOpenRequest: (worktreeId) => + set((s) => + s.diffNotesSendMenuOpenRequest?.worktreeId === worktreeId + ? { diffNotesSendMenuOpenRequest: null } + : s + ), closeAgentSendPopoverTargetMode: (id, instanceId) => set((s) => { if (!s.agentSendPopoverTargetMode) { diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index ea69dee11..61df504cd 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -472,6 +472,19 @@ describe('keybindings', () => { ]) }) + it('flags the global send-review-notes command against editor chords it can shadow', () => { + // Why: it fires from the global capture handler even while the editor is + // focused, so Settings must warn when a user binds it over Add Review Note. + expect( + findKeybindingConflicts('darwin', { 'sourceControl.sendReviewNotes': ['Mod+Shift+A'] }) + ).toContainEqual( + expect.objectContaining({ + binding: 'Mod+Shift+A', + actionIds: expect.arrayContaining(['editor.addReviewNote', 'sourceControl.sendReviewNotes']) + }) + ) + }) + it('defaults tab-switch chords to the swapped convention for fresh installs', () => { // New users get the widespread mapping: Shift+bracket cycles all tabs, // Alt+bracket cycles within the active type. diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 6b2860ced..6fa3318af 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -91,6 +91,7 @@ export type KeybindingActionId = | 'editor.previousChange' | 'editor.nextChange' | 'editor.addReviewNote' + | 'sourceControl.sendReviewNotes' | 'fileExplorer.undo' | 'fileExplorer.redo' | 'fileExplorer.copyPath' @@ -838,6 +839,26 @@ export const KEYBINDING_DEFINITIONS: readonly KeybindingDefinition[] = [ // Why: Ctrl+Alt+letter is AltGr text input on Windows/Linux, so an editor default must not reserve chars like Polish `ń`. defaultBindings: platformBindings(['Mod+Shift+A']) }, + { + id: 'sourceControl.sendReviewNotes', + title: 'Send Review Notes to Agent', + group: 'Global', + scope: 'global', + // Why: fires from the global capture handler even while the editor is focused, so Settings must warn on collisions with editor chords (e.g. Add Review Note) too, not just global ones. + conflictGroup: 'editor', + searchKeywords: [ + 'shortcut', + 'source control', + 'diff', + 'notes', + 'send', + 'agent', + 'review', + 'annotate' + ], + // Why: unbound by default so it never collides with existing chords; users opt in via Settings. + defaultBindings: platformBindings([]) + }, { id: 'fileExplorer.undo', title: 'Undo file operation',