feat(review): add assignable "Send Review Notes to Agent" shortcut (#10027) (#10070)

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 <help@stably.ai>
This commit is contained in:
Neil 2026-07-22 20:38:10 -07:00 committed by GitHub
parent d152039e9d
commit 059a80b298
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 210 additions and 2 deletions

View File

@ -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')

View File

@ -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)}
/>
)

View File

@ -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' }

View File

@ -40,6 +40,10 @@ export type NotesSendMenuProps<TNote> = {
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<TNote>({
disabledTooltip = 'All notes sent',
iconClassName = 'size-3.5',
align = 'end',
openRequestNonce = null,
onOpenRequestHandled,
onDelivered
}: NotesSendMenuProps<TNote>): React.JSX.Element {
const openAgentSendPopoverTargetMode = useAppStore((s) => s.openAgentSendPopoverTargetMode)
@ -138,6 +144,18 @@ export function NotesSendMenu<TNote>({
[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 (
<DropdownMenu modal={false} open={effectiveSendMenuOpen} onOpenChange={handleOpenChange}>
<Tooltip>

View File

@ -5525,6 +5525,7 @@ function SourceControlInner(): React.JSX.Element {
groupId={activeGroupId ?? activeWorktreeId}
comments={diffCommentsForActive}
triggerClassName="size-6"
respondToOpenRequest
/>
{diffCommentCount > 0 && (
<TooltipProvider delayDuration={400}>

View File

@ -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<AppState>; setRightSidebarTab: ReturnType<typeof vi.fn> } {
const store = createUIStore()
const setRightSidebarTab = vi.fn()
store.setState({
activeWorktreeId,
getDiffComments: () => comments,
setRightSidebarTab,
setRightSidebarOpen: vi.fn()
} as unknown as Partial<AppState>)
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()
})
})

View File

@ -593,6 +593,11 @@ export type UISlice = {
openAgentSendPopoverTargetMode: (args: OpenAgentSendPopoverTargetModeArgs) => void
closeAgentSendPopoverTargetMode: (id?: string, instanceId?: string) => void
sendPromptToSidebarAgentTarget: (paneKey: string) => Promise<boolean>
/** 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<string, number>
acknowledgeAgents: (paneKeys: string[]) => void
@ -1008,6 +1013,32 @@ export const createUISlice: StateCreator<AppState, [], [], UISlice> = (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) {

View File

@ -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.

View File

@ -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',