diff --git a/src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.tsx b/src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.tsx index e19a0a2e2ec..faceaab0f0b 100644 --- a/src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.tsx +++ b/src/renderer/src/components/browser-pane/annotate/BrowserAnnotationSendMenuContent.tsx @@ -6,13 +6,15 @@ export type BrowserAnnotationSendMenuContentProps = { groupId: string prompt: string onPromptDelivered?: () => void + onPromptHandedOff?: (delivered: Promise) => void } export function BrowserAnnotationSendMenuContent({ worktreeId, groupId, prompt, - onPromptDelivered + onPromptDelivered, + onPromptHandedOff }: BrowserAnnotationSendMenuContentProps): React.JSX.Element { return ( ) } diff --git a/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx b/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx index 200670cd536..c8141bd0685 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx +++ b/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx @@ -75,6 +75,7 @@ export function BrowserGuestAnnotateOverlays({ activeGroupId, browserAnnotationsPrompt, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, handleCopyBrowserAnnotations, browserAnnotationsCopied, handleClearBrowserAnnotations, @@ -119,6 +120,7 @@ export function BrowserGuestAnnotateOverlays({ activeGroupId={activeGroupId} browserAnnotationsPrompt={browserAnnotationsPrompt} handleBrowserAnnotationsSentToAgent={handleBrowserAnnotationsSentToAgent} + handleBrowserAnnotationsHandedOff={handleBrowserAnnotationsHandedOff} handleCopyBrowserAnnotations={handleCopyBrowserAnnotations} browserAnnotationsCopied={browserAnnotationsCopied} handleClearBrowserAnnotations={handleClearBrowserAnnotations} diff --git a/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx index a22f18b06d5..06e35269429 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx +++ b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx @@ -29,6 +29,7 @@ export function BrowserPageAnnotationTray({ activeGroupId, browserAnnotationsPrompt, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, handleCopyBrowserAnnotations, browserAnnotationsCopied, handleClearBrowserAnnotations, @@ -43,6 +44,7 @@ export function BrowserPageAnnotationTray({ activeGroupId: string | undefined browserAnnotationsPrompt: string handleBrowserAnnotationsSentToAgent: () => void + handleBrowserAnnotationsHandedOff: (delivered: Promise) => void handleCopyBrowserAnnotations: () => void browserAnnotationsCopied: boolean handleClearBrowserAnnotations: () => void @@ -135,6 +137,7 @@ export function BrowserPageAnnotationTray({ groupId={activeGroupId ?? worktreeId} prompt={browserAnnotationsPrompt} onPromptDelivered={handleBrowserAnnotationsSentToAgent} + onPromptHandedOff={handleBrowserAnnotationsHandedOff} /> diff --git a/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.test.tsx b/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.test.tsx index 9c7ca72e954..52bd02ba05a 100644 --- a/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.test.tsx +++ b/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.test.tsx @@ -8,6 +8,7 @@ import { createTestStore } from '@/store/slices/browser-slice-test-harness' import type { OpenAgentSendPopoverTargetModeArgs } from '@/store/slices/ui' import { BrowserPageAnnotationTray } from './browser-page-annotation-tray' import { useBrowserPageAnnotationSend } from './use-browser-page-annotation-send' +import { resetNotesInFlightForTests } from '@/lib/notes-send-in-flight' const state = vi.hoisted((): { store?: ReturnType } => ({})) vi.mock('@/store', () => ({ @@ -93,6 +94,7 @@ beforeEach(() => { store = createTestStore() state.store = store mode = undefined + resetNotesInFlightForTests() store.setState({ activeGroupIdByWorktree: {}, openAgentSendPopoverTargetMode: (next) => { @@ -214,3 +216,54 @@ describe('website annotation delivery', () => { expect(notes()).toEqual([]) }) }) + +describe('website annotations handed to a send', () => { + const second = (): BrowserPageAnnotation => ({ + ...makeAnnotation('page-1', 'second'), + comment: 'Second note' + }) + + it('sends only an annotation added while an earlier send is still on its way', async () => { + const view = mount() + const firstDelivered = view.result.current.handleBrowserAnnotationsSentToAgent + let deliverFirst!: (result: { delivered: boolean }) => void + act(() => + view.result.current.handleBrowserAnnotationsHandedOff( + new Promise((resolve) => (deliverFirst = resolve)) + ) + ) + expect(view.result.current.browserAnnotationsPrompt).toBe('') + + act(() => store.getState().addBrowserPageAnnotation(second())) + expect(view.result.current.browserAnnotationsPrompt).toContain('Second note') + expect(view.result.current.browserAnnotationsPrompt).not.toContain('Fix this button') + const secondDelivered = view.result.current.handleBrowserAnnotationsSentToAgent + + act(firstDelivered) + await act(async () => deliverFirst({ delivered: true })) + expect(notes().map((note) => note.id)).toEqual(['second']) + act(secondDelivered) + expect(notes()).toEqual([]) + }) + + it('puts annotations back for the next send when their delivery fails', async () => { + const view = mount() + const delivered = Promise.resolve({ delivered: false, failureNotified: true }) + act(() => view.result.current.handleBrowserAnnotationsHandedOff(delivered)) + expect(view.result.current.browserAnnotationsPrompt).toBe('') + + await act(async () => { + await delivered + }) + + expect(view.result.current.browserAnnotationsPrompt).toContain('Fix this button') + expect(notes()).toHaveLength(1) + }) + + it('holds what a running-agent send carries too', () => { + const view = mount() + act(() => view.result.current.handleAnnotationTraySendOpenChange(true)) + act(() => mode?.onPromptHandedOff?.(new Promise(() => undefined))) + expect(view.result.current.browserAnnotationsPrompt).toBe('') + }) +}) diff --git a/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.ts b/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.ts index b770302715f..5480c030bfe 100644 --- a/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.ts +++ b/src/renderer/src/components/browser-pane/annotate/use-browser-page-annotation-send.ts @@ -16,6 +16,11 @@ import type { } from '../../../../../shared/browser-grab-types' import { formatBrowserAnnotationsAsMarkdown } from './browser-annotation-output' import { EMPTY_BROWSER_ANNOTATIONS } from '../describe-page/browser-annotation-geometry' +import { + holdNotesForSend, + isNoteInFlight, + useNotesInFlightVersion +} from '@/lib/notes-send-in-flight' export function useBrowserPageAnnotationSend({ browserTabId, @@ -42,6 +47,7 @@ export function useBrowserPageAnnotationSend({ intent: BrowserAnnotationIntent ) => void handleBrowserAnnotationsSentToAgent: () => void + handleBrowserAnnotationsHandedOff: (delivered: Promise) => void activeGroupId: string | undefined } { const browserAnnotations = useAppStore( @@ -52,10 +58,20 @@ export function useBrowserPageAnnotationSend({ const [browserAnnotationTrayOpen, setBrowserAnnotationTrayOpen] = useState(true) const [browserAnnotationsCopied, setBrowserAnnotationsCopied] = useState(false) const annotationCopyTimerRef = useRef>(undefined) - const browserAnnotationsPrompt = useMemo( + const copyPrompt = useMemo( () => formatBrowserAnnotationsAsMarkdown(browserAnnotations), [browserAnnotations] ) + // Annotations another send holds are left out of the next one. + const inFlightVersion = useNotesInFlightVersion() + const sendableAnnotations = useMemo(() => { + void inFlightVersion + return browserAnnotations.filter((annotation) => !isNoteInFlight(annotation)) + }, [browserAnnotations, inFlightVersion]) + const browserAnnotationsPrompt = useMemo( + () => formatBrowserAnnotationsAsMarkdown(sendableAnnotations), + [sendableAnnotations] + ) const openAgentSendPopoverTargetMode = useAppStore((s) => s.openAgentSendPopoverTargetMode) const closeAgentSendPopoverTargetMode = useAppStore((s) => s.closeAgentSendPopoverTargetMode) const activeAgentSendTargetModeId = useAppStore((s) => s.agentSendPopoverTargetMode?.id ?? null) @@ -82,26 +98,31 @@ export function useBrowserPageAnnotationSend({ }, []) const handleCopyBrowserAnnotations = useCallback((): void => { - if (!browserAnnotationsPrompt) { + if (!copyPrompt) { return } - void window.api.ui.writeClipboardText(browserAnnotationsPrompt) + void window.api.ui.writeClipboardText(copyPrompt) recordFeatureInteraction('browser-annotations') clearTimeout(annotationCopyTimerRef.current) setBrowserAnnotationsCopied(true) annotationCopyTimerRef.current = setTimeout(() => setBrowserAnnotationsCopied(false), 1400) - }, [browserAnnotationsPrompt, recordFeatureInteraction]) + }, [copyPrompt, recordFeatureInteraction]) const handleBrowserAnnotationsSentToAgent = useCallback((): void => { recordFeatureInteraction('browser-annotations-sent-to-agent') - removeDeliveredBrowserPageAnnotations(browserTabId, browserAnnotations) + removeDeliveredBrowserPageAnnotations(browserTabId, sendableAnnotations) }, [ - browserAnnotations, + sendableAnnotations, browserTabId, recordFeatureInteraction, removeDeliveredBrowserPageAnnotations ]) + const handleBrowserAnnotationsHandedOff = useCallback( + (delivered: Promise): void => holdNotesForSend(sendableAnnotations, delivered), + [sendableAnnotations] + ) + const handleClearBrowserAnnotations = useCallback((): void => { if (browserAnnotationsRef.current.length === 0) { return @@ -125,7 +146,8 @@ export function useBrowserPageAnnotationSend({ 'Browser annotations' ), launchSource: 'notes_send', - onPromptDelivered: handleBrowserAnnotationsSentToAgent + onPromptDelivered: handleBrowserAnnotationsSentToAgent, + onPromptHandedOff: handleBrowserAnnotationsHandedOff }) } else { closeAgentSendPopoverTargetMode(modeId) @@ -134,6 +156,7 @@ export function useBrowserPageAnnotationSend({ [ browserAnnotationsPrompt, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, closeAgentSendPopoverTargetMode, openAgentSendPopoverTargetMode, worktreeId @@ -199,6 +222,7 @@ export function useBrowserPageAnnotationSend({ handleDeleteBrowserAnnotation, handleUpdateBrowserAnnotation, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, activeGroupId } } diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx index 48dc0a6afe0..90fbc4464b4 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-banners.tsx @@ -28,6 +28,7 @@ export function BrowserPageChromeBanners({ activeGroupId, browserAnnotationsPrompt, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, handleCopyBrowserAnnotations, browserAnnotationsCopied, handleClearBrowserAnnotations, @@ -45,6 +46,7 @@ export function BrowserPageChromeBanners({ activeGroupId: string | undefined browserAnnotationsPrompt: string handleBrowserAnnotationsSentToAgent: () => void + handleBrowserAnnotationsHandedOff: (delivered: Promise) => void handleCopyBrowserAnnotations: () => void browserAnnotationsCopied: boolean handleClearBrowserAnnotations: () => void @@ -155,6 +157,7 @@ export function BrowserPageChromeBanners({ groupId={activeGroupId ?? worktreeId} prompt={browserAnnotationsPrompt} onPromptDelivered={handleBrowserAnnotationsSentToAgent} + onPromptHandedOff={handleBrowserAnnotationsHandedOff} /> diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-header.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-header.tsx index 8d5b5b68ade..b162a2ba34b 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-header.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/browser-page-chrome-header.tsx @@ -121,6 +121,7 @@ export function BrowserPageChromeHeader({ activeGroupId={annotationSend.activeGroupId} browserAnnotationsPrompt={annotationSend.browserAnnotationsPrompt} handleBrowserAnnotationsSentToAgent={annotationSend.handleBrowserAnnotationsSentToAgent} + handleBrowserAnnotationsHandedOff={annotationSend.handleBrowserAnnotationsHandedOff} handleCopyBrowserAnnotations={annotationSend.handleCopyBrowserAnnotations} browserAnnotationsCopied={annotationSend.browserAnnotationsCopied} handleClearBrowserAnnotations={annotationSend.handleClearBrowserAnnotations} diff --git a/src/renderer/src/components/diff-comments/diff-comment-zone-card.tsx b/src/renderer/src/components/diff-comments/diff-comment-zone-card.tsx index d34de51a2bc..5eb6d312971 100644 --- a/src/renderer/src/components/diff-comments/diff-comment-zone-card.tsx +++ b/src/renderer/src/components/diff-comments/diff-comment-zone-card.tsx @@ -39,7 +39,8 @@ function getSingleCommentSendScopes( 'This note' ), notes: comment.sentAt ? [] : [comment], - prompt: formatCommentPrompt ? formatCommentPrompt(comment) : formatDiffComments([comment]) + formatPrompt: () => + formatCommentPrompt ? formatCommentPrompt(comment) : formatDiffComments([comment]) } ] } diff --git a/src/renderer/src/components/editor/DiffNotesSendMenu.tsx b/src/renderer/src/components/editor/DiffNotesSendMenu.tsx index d5ce2434449..08a0e07440a 100644 --- a/src/renderer/src/components/editor/DiffNotesSendMenu.tsx +++ b/src/renderer/src/components/editor/DiffNotesSendMenu.tsx @@ -54,20 +54,18 @@ export function DiffNotesSendMenu({ [consumeOpenRequest, worktreeId] ) const unsentNotes = useMemo(() => comments.filter((comment) => !comment.sentAt), [comments]) - const unsentPrompt = useMemo(() => formatDiffComments(unsentNotes), [unsentNotes]) const fileNotes = useMemo( () => (filePath ? comments.filter((comment) => comment.filePath === filePath) : []), [comments, filePath] ) const unsentFileNotes = useMemo(() => fileNotes.filter((comment) => !comment.sentAt), [fileNotes]) - const unsentFilePrompt = useMemo(() => formatDiffComments(unsentFileNotes), [unsentFileNotes]) const canSendFileScope = showFileScope && Boolean(filePath) const scopes = useMemo[]>(() => { const allNotesScope = { id: 'all', label: translate('auto.components.editor.DiffNotesSendMenu.8b87612461', 'All unsent notes'), notes: unsentNotes, - prompt: unsentPrompt + formatPrompt: formatDiffComments } if (!canSendFileScope) { return [allNotesScope] @@ -77,11 +75,11 @@ export function DiffNotesSendMenu({ id: 'file', label: translate('auto.components.editor.DiffNotesSendMenu.f1aa04b5cf', 'This file'), notes: unsentFileNotes, - prompt: unsentFilePrompt + formatPrompt: formatDiffComments }, allNotesScope ] - }, [canSendFileScope, unsentFileNotes, unsentFilePrompt, unsentNotes, unsentPrompt]) + }, [canSendFileScope, unsentFileNotes, unsentNotes]) return ( formatMarkdownReviewNotes(notes, content) } ]} targetModeLabel="This note" diff --git a/src/renderer/src/components/editor/NotesSendMenu.test.tsx b/src/renderer/src/components/editor/NotesSendMenu.test.tsx index 64556e6b83d..7ce7fe77b85 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.test.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.test.tsx @@ -1,14 +1,18 @@ import React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { buildNotesSendTargetModeId, NotesSendMenu } from './NotesSendMenu' +import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' +import { resetNotesInFlightForTests } from '@/lib/notes-send-in-flight' type ReactElementLike = { type: unknown props: Record } -type TestNote = { - id: string +type TestNote = DiffCommentDeliverySnapshot + +function note(id: string): TestNote { + return { id, body: `body of ${id}`, filePath: 'README.md', lineNumber: 1 } } const hookRuntime = vi.hoisted(() => ({ @@ -41,6 +45,9 @@ vi.mock('react', async () => { useMemo(factory: () => T): T { return factory() }, + useSyncExternalStore(_subscribe: unknown, getSnapshot: () => T): T { + return getSnapshot() + }, useState(initial: T | (() => T)) { const stateIndex = hookRuntime.index++ if (!(stateIndex in hookRuntime.states)) { @@ -233,8 +240,8 @@ function renderMenu( { id: 'all', label: 'All unsent notes', - notes: [{ id: 'note-1' }], - prompt: 'prompt-all' + notes: [note('note-1')], + formatPrompt: () => 'prompt-all' } ]} onDelivered={vi.fn()} @@ -274,11 +281,12 @@ describe('NotesSendMenu', () => { storeMocks.openAgentSendPopoverTargetMode.mockReset() storeMocks.closeAgentSendPopoverTargetMode.mockReset() storeMocks.state.agentSendPopoverTargetMode = null + resetNotesInFlightForTests() }) it('disables the trigger when no scope has deliverable notes', () => { const tree = renderMenu({ - scopes: [{ id: 'all', label: 'All unsent notes', notes: [], prompt: '' }] + scopes: [{ id: 'all', label: 'All unsent notes', notes: [], formatPrompt: () => '' }] }) expect(findByType(tree, 'button').props.disabled).toBe(true) @@ -288,7 +296,7 @@ describe('NotesSendMenu', () => { it('uses caller-provided disabled tooltip copy for disabled note actions', () => { const tree = renderMenu({ - scopes: [{ id: 'note', label: 'This note', notes: [], prompt: '' }], + scopes: [{ id: 'note', label: 'This note', notes: [], formatPrompt: () => '' }], disabledTooltip: 'Note already sent' }) @@ -317,7 +325,7 @@ describe('NotesSendMenu', () => { const delivered = storeMocks.openAgentSendPopoverTargetMode.mock.calls[0][0] .onPromptDelivered as () => void delivered() - expect(onDelivered).toHaveBeenCalledWith([{ id: 'note-1' }]) + expect(onDelivered).toHaveBeenCalledWith([note('note-1')]) ;(dropdown.props.onOpenChange as (open: boolean) => void)(false) expect(storeMocks.closeAgentSendPopoverTargetMode).toHaveBeenCalledWith( @@ -341,8 +349,18 @@ describe('NotesSendMenu', () => { const tree = renderMenu({ defaultScopeId: 'file', scopes: [ - { id: 'file', label: 'This file', notes: [{ id: 'file-note' }], prompt: 'prompt-file' }, - { id: 'all', label: 'All unsent notes', notes: [{ id: 'all-note' }], prompt: 'prompt-all' } + { + id: 'file', + label: 'This file', + notes: [note('file-note')], + formatPrompt: () => 'prompt-file' + }, + { + id: 'all', + label: 'All unsent notes', + notes: [note('all-note')], + formatPrompt: () => 'prompt-all' + } ] }) const [fileTrigger, allTrigger] = findAllByType(tree, 'DropdownMenuSubTrigger') @@ -375,7 +393,7 @@ describe('NotesSendMenu', () => { renderMenu({ openRequestNonce: 1, onOpenRequestHandled, - scopes: [{ id: 'all', label: 'All unsent notes', notes: [], prompt: '' }] + scopes: [{ id: 'all', label: 'All unsent notes', notes: [], formatPrompt: () => '' }] }) expect(storeMocks.openAgentSendPopoverTargetMode).not.toHaveBeenCalled() @@ -437,3 +455,85 @@ describe('NotesSendMenu', () => { ) }) }) + +describe('NotesSendMenu notes in flight', () => { + const noteA = note('note-a') + const noteB = note('note-b') + const scopeOf = (notes: TestNote[]) => [ + { + id: 'all', + label: 'All unsent notes', + notes, + formatPrompt: (sent: readonly TestNote[]) => sent.map((entry) => entry.id).join('+') + } + ] + /** Calls a rendered callback prop, failing the test if it is missing. */ + const invoke = (props: Record, name: string, ...args: unknown[]): unknown => { + const callback = props[name] + if (typeof callback !== 'function') { + throw new Error(`${name} is not a function`) + } + return callback(...args) + } + const contentProps = (tree: unknown) => { + const props = findByType(tree, 'ReviewNotesSendMenuContent').props + return { + prompt: props.prompt, + onPromptDelivered: () => invoke(props, 'onPromptDelivered'), + onPromptHandedOff: (delivered: Promise) => + invoke(props, 'onPromptHandedOff', delivered) + } + } + + beforeEach(() => { + resetHookRuntime() + storeMocks.openAgentSendPopoverTargetMode.mockReset() + storeMocks.state.agentSendPopoverTargetMode = null + resetNotesInFlightForTests() + }) + + it('sends only a note added while an earlier send to a new agent is still on its way', async () => { + const onDelivered = vi.fn() + let deliverA!: (result: { delivered: boolean }) => void + const first = contentProps(renderMenu({ scopes: scopeOf([noteA]), onDelivered })) + first.onPromptHandedOff(new Promise((resolve) => (deliverA = resolve))) + + const second = contentProps(renderMenu({ scopes: scopeOf([noteA, noteB]), onDelivered })) + expect(second.prompt).toBe('note-b') + second.onPromptHandedOff(new Promise(() => undefined)) + + first.onPromptDelivered() + deliverA({ delivered: true }) + second.onPromptDelivered() + await Promise.resolve() + + expect(onDelivered.mock.calls).toEqual([[[noteA]], [[noteB]]]) + }) + + it('leaves the notes out of the running-agent target mode too', () => { + contentProps(renderMenu({ scopes: scopeOf([noteA]) })).onPromptHandedOff( + new Promise(() => undefined) + ) + + const tree = renderMenu({ scopes: scopeOf([noteA, noteB]) }) + invoke(findByType(tree, 'DropdownMenu').props, 'onOpenChange', true) + + expect(storeMocks.openAgentSendPopoverTargetMode).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'note-b', onPromptHandedOff: expect.any(Function) }) + ) + }) + + it.each([ + ['an undelivered result', () => Promise.resolve({ delivered: false, failureNotified: true })], + ['a failed start', () => Promise.reject(new Error('refused'))] + ])('puts the notes back for the next send after %s', async (_name, deliver) => { + const delivered = deliver() + contentProps(renderMenu({ scopes: scopeOf([noteA]) })).onPromptHandedOff(delivered) + expect(contentProps(renderMenu({ scopes: scopeOf([noteA]) })).prompt).toBe('') + + await delivered.catch(() => undefined) + await Promise.resolve() + + expect(contentProps(renderMenu({ scopes: scopeOf([noteA]) })).prompt).toBe('note-a') + }) +}) diff --git a/src/renderer/src/components/editor/NotesSendMenu.tsx b/src/renderer/src/components/editor/NotesSendMenu.tsx index 8315d4512df..fceab2d1960 100644 --- a/src/renderer/src/components/editor/NotesSendMenu.tsx +++ b/src/renderer/src/components/editor/NotesSendMenu.tsx @@ -14,6 +14,13 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' import { ReviewNotesSendMenuContent } from './ReviewNotesSendMenuContent' +import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' +import { + diffCommentSendKey, + holdNotesForSend, + isNoteInFlight, + useNotesInFlightVersion +} from '@/lib/notes-send-in-flight' import { translate } from '@/i18n/i18n' const ENABLED_SEND_TOOLTIP = 'Send notes to an agent' @@ -22,6 +29,11 @@ export type NotesSendMenuScope = { id: string label: string notes: readonly TNote[] + /** The prompt for the notes this send carries: notes another send holds are left out. */ + formatPrompt: (notes: readonly TNote[]) => string +} + +type SendableNotesScope = Omit, 'formatPrompt'> & { prompt: string } @@ -57,7 +69,7 @@ export function buildNotesSendTargetModeId(modeIdParts: readonly string[]): stri return `note-send:${modeIdParts.map((part) => `${part.length}:${part}`).join('|')}` } -export function NotesSendMenu({ +export function NotesSendMenu({ worktreeId, groupId, modeIdParts, @@ -82,7 +94,18 @@ export function NotesSendMenu({ const activeTargetModeId = useAppStore((s) => s.agentSendPopoverTargetMode?.id ?? null) const [sendMenuOpen, setSendMenuOpen] = useState(false) const targetModeId = useMemo(() => buildNotesSendTargetModeId(modeIdParts), [modeIdParts]) - const enabledScopes = useMemo(() => scopes.filter((scope) => scope.notes.length > 0), [scopes]) + const inFlightVersion = useNotesInFlightVersion() + const sendableScopes = useMemo[]>(() => { + void inFlightVersion + return scopes.map(({ formatPrompt, ...scope }) => { + const notes = scope.notes.filter((note) => !isNoteInFlight(diffCommentSendKey(note))) + return { ...scope, notes, prompt: notes.length > 0 ? formatPrompt(notes) : '' } + }) + }, [inFlightVersion, scopes]) + const enabledScopes = useMemo( + () => sendableScopes.filter((scope) => scope.notes.length > 0), + [sendableScopes] + ) const defaultScope = useMemo(() => { const requested = enabledScopes.find((scope) => scope.id === defaultScopeId) return requested ?? enabledScopes[0] ?? null @@ -95,9 +118,14 @@ export function NotesSendMenu({ }, [onDelivered] ) + const holdInFlight = useCallback( + (notes: readonly TNote[]) => (delivered: Promise) => + holdNotesForSend(notes.map(diffCommentSendKey), delivered), + [] + ) const openTargetMode = useCallback( - (scope: NotesSendMenuScope) => { + (scope: SendableNotesScope) => { if (scope.notes.length === 0) { return } @@ -108,10 +136,12 @@ export function NotesSendMenu({ prompt: scope.prompt, label: targetModeLabel ?? scope.label, launchSource: 'notes_send', - onPromptDelivered: () => markDelivered(scope.notes) + onPromptDelivered: () => markDelivered(scope.notes), + onPromptHandedOff: holdInFlight(scope.notes) }) }, [ + holdInFlight, markDelivered, openAgentSendPopoverTargetMode, source, @@ -226,7 +256,7 @@ export function NotesSendMenu({ {translate('auto.components.editor.NotesSendMenu.44dc5e60a6', 'Send notes')} - {scopes.map((scope) => ( + {sendableScopes.map((scope) => ( ({ promptDelivery="submit-after-ready" launchSource="notes_send" onPromptDelivered={() => markDelivered(scope.notes)} + onPromptHandedOff={holdInFlight(scope.notes)} /> @@ -261,6 +292,7 @@ export function NotesSendMenu({ markDelivered(defaultScope.notes) } }} + onPromptHandedOff={holdInFlight(defaultScope?.notes ?? [])} /> )} diff --git a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx index 7c1961f7d7c..980b7f3918f 100644 --- a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx +++ b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsx @@ -650,6 +650,7 @@ describe('ReviewNotesSendMenuContent', () => { it('sends notes to the chosen agent and tracks the send once it succeeds', async () => { const statusPaneKey = makePaneKey(TAB_A, LEAF_A) const onPromptDelivered = vi.fn() + const onPromptHandedOff = vi.fn() setStore({ tabsByWorktree: { 'wt-1': [tab(TAB_A, { title: 'Terminal 1' })] }, terminalLayoutsByTabId: { [TAB_A]: leafLayout(LEAF_A, 'pty-a') } @@ -665,7 +666,7 @@ describe('ReviewNotesSendMenuContent', () => { } ] - const tree = render({ onPromptDelivered }) + const tree = render({ onPromptDelivered, onPromptHandedOff }) ;(findByType(tree, 'DropdownMenuItem').props.onSelect as () => void)() await flushMicrotasks() @@ -680,6 +681,10 @@ describe('ReviewNotesSendMenuContent', () => { launch_source: 'notes_send', request_kind: 'followup' }) + // The notes are held from the hand-off until this send's own outcome, after its delivery. + expect(onPromptHandedOff).toHaveBeenCalledOnce() + await onPromptHandedOff.mock.calls[0][0] + expect(onPromptDelivered).toHaveBeenCalledTimes(1) }) it('keeps selected-target note failures undelivered and uses selected wording', async () => { @@ -854,13 +859,15 @@ describe('ReviewNotesSendMenuContent', () => { }) it('always offers the new-agent launcher', () => { - const tree = render() + const onPromptHandedOff = vi.fn() + const tree = render({ onPromptHandedOff }) expect(findByType(tree, 'QuickLaunchAgentMenuItems').props).toMatchObject({ worktreeId: 'wt-1', groupId: 'group-1', prompt: 'my notes', - launchSource: 'notes_send' + launchSource: 'notes_send', + onPromptHandedOff }) }) }) diff --git a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx index 688113e7a01..6c97b0f3ad6 100644 --- a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx +++ b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx @@ -46,7 +46,8 @@ export function ReviewNotesSendMenuContent({ prompt, promptDelivery = 'submit-after-ready', launchSource = 'notes_send', - onPromptDelivered + onPromptDelivered, + onPromptHandedOff }: { worktreeId: string groupId: string @@ -54,6 +55,8 @@ export function ReviewNotesSendMenuContent({ promptDelivery?: 'auto-submit' | 'draft' | 'submit-after-ready' launchSource?: LaunchSource onPromptDelivered?: () => void + /** Given each send's own result the moment its prompt is handed to an agent. */ + onPromptHandedOff?: (delivered: Promise) => void }): React.JSX.Element { const hasPrompt = prompt.trim().length > 0 @@ -114,7 +117,7 @@ export function ReviewNotesSendMenuContent({ ) ) - void send() + const sending = send() .then((result) => { if (result.status === 'sent') { onSent() @@ -147,8 +150,9 @@ export function ReviewNotesSendMenuContent({ { id: pending } ) }) + onPromptHandedOff?.(sending) }, - [] + [onPromptHandedOff] ) const sendToAgentTarget = useCallback( @@ -208,6 +212,7 @@ export function ReviewNotesSendMenuContent({ promptDelivery={promptDelivery} launchSource={launchSource} onPromptDelivered={onPromptDelivered} + onPromptHandedOff={onPromptHandedOff} /> ) diff --git a/src/renderer/src/components/editor/RichMarkdownReviewNoteLayer.tsx b/src/renderer/src/components/editor/RichMarkdownReviewNoteLayer.tsx index 0aa90569f98..2caa79b5a8d 100644 --- a/src/renderer/src/components/editor/RichMarkdownReviewNoteLayer.tsx +++ b/src/renderer/src/components/editor/RichMarkdownReviewNoteLayer.tsx @@ -134,10 +134,8 @@ export function RichMarkdownReviewNoteLayer({ 'This note' ), notes: comment.sentAt ? [] : [comment as MarkdownReviewNote], - prompt: formatMarkdownReviewNotes( - [comment as MarkdownReviewNote], - markdownReviewContent - ) + formatPrompt: (notes) => + formatMarkdownReviewNotes(notes, markdownReviewContent) } ]} targetModeLabel="This note" diff --git a/src/renderer/src/components/editor/use-markdown-preview-foundation.ts b/src/renderer/src/components/editor/use-markdown-preview-foundation.ts index 48631ce6897..accf06ca409 100644 --- a/src/renderer/src/components/editor/use-markdown-preview-foundation.ts +++ b/src/renderer/src/components/editor/use-markdown-preview-foundation.ts @@ -101,20 +101,16 @@ export function useMarkdownPreviewFoundation({ () => markdownReviewNotes.filter((note) => !note.sentAt), [markdownReviewNotes] ) - const unsentMarkdownReviewPrompt = useMemo( - () => formatMarkdownReviewNotes(unsentMarkdownReviewNotes, renderedContent), - [renderedContent, unsentMarkdownReviewNotes] - ) const unsentMarkdownReviewScope = useMemo[]>( () => [ { id: 'all', label: translate('auto.components.editor.MarkdownPreview.ddf087d12e', 'All unsent notes'), notes: unsentMarkdownReviewNotes, - prompt: unsentMarkdownReviewPrompt + formatPrompt: (notes) => formatMarkdownReviewNotes(notes, renderedContent) } ], - [unsentMarkdownReviewNotes, unsentMarkdownReviewPrompt] + [renderedContent, unsentMarkdownReviewNotes] ) const canShowReviewTools = Boolean( markdownAnnotationsEnabled && sourceWorktree && sourceRelativePath !== null diff --git a/src/renderer/src/components/editor/useRichMarkdownReviewData.ts b/src/renderer/src/components/editor/useRichMarkdownReviewData.ts index a447562f5d4..447bd938b56 100644 --- a/src/renderer/src/components/editor/useRichMarkdownReviewData.ts +++ b/src/renderer/src/components/editor/useRichMarkdownReviewData.ts @@ -65,7 +65,7 @@ export function useRichMarkdownReviewData({ 'All unsent notes' ), notes: unsentNotes, - prompt: formatMarkdownReviewNotes(unsentNotes, markdownReviewContent) + formatPrompt: (notes) => formatMarkdownReviewNotes(notes, markdownReviewContent) } ] }, [markdownReviewContent, markdownReviewNotes]) diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx index ebeed7630cd..3e43d66f52f 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import type { ReactNode } from 'react' -import { cleanup, render } from '@testing-library/react' +import { cleanup, fireEvent, render } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { StructuredLaunchState } from '@/lib/structured-agent-session-launch-registry' import { @@ -35,8 +35,13 @@ vi.mock('@/lib/agent-catalog', () => ({ AgentIcon: ({ agent }: { agent: string }) => {agent} })) vi.mock('@/components/ui/dropdown-menu', () => ({ - DropdownMenuItem: ({ children, disabled, title }: { children: ReactNode } & DivProps) => ( -
+ DropdownMenuItem: ({ + children, + disabled, + title, + onSelect + }: { children: ReactNode } & DivProps) => ( +
{children}
), @@ -46,9 +51,10 @@ vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string, values?: Record) => fallback.replace('{{value0}}', values?.value0 ?? '') })) -vi.mock('@/lib/launch-agent-in-new-tab', () => ({ launchAgentInNewTab: vi.fn() })) +const launchMock = vi.hoisted(() => vi.fn()) +vi.mock('@/lib/launch-agent-in-new-tab', () => ({ launchAgentInNewTab: launchMock })) -type DivProps = { disabled?: boolean; title?: string } +type DivProps = { disabled?: boolean; title?: string; onSelect?: () => void } import { QuickLaunchAgentMenuItems } from './QuickLaunchButton' import { @@ -173,4 +179,28 @@ describe('QuickLaunchAgentMenuItems launch status', () => { render(menu('review notes')) expect(agentRowDisabled('Codex')).toBe('true') }) + + // Why: the notes menu holds what it sent until this result, so a second send leaves them out. + it("hands the launch's own delivery result to the notes menu", () => { + const delivery = Promise.resolve({ delivered: true, failureNotified: false }) + launchMock.mockReturnValue({ + surface: { kind: 'local-agent-session', tabId: 'tab-1', sessionId: 'codex-session' }, + promptDeliveryResult: delivery + }) + const onPromptHandedOff = vi.fn() + + render( + + ) + fireEvent.click(document.querySelector('[title="Launch Codex in a new terminal"]')!) + + expect(onPromptHandedOff).toHaveBeenCalledWith(delivery) + }) }) diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx index b065595fb65..49a362e9826 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx @@ -38,6 +38,8 @@ export type QuickLaunchAgentMenuItemsProps = { launchSource?: LaunchSource /** Called after a prompt is queued into the agent, or immediately for argv prompt launches. */ onPromptDelivered?: () => void + /** Given the launch's own delivery result while the prompt is still on its way. */ + onPromptHandedOff?: (delivered: Promise) => void } function getCatalogEntry(agent: TuiAgent): { id: TuiAgent; label: string } | null { @@ -104,7 +106,8 @@ function QuickLaunchAgentMenuItemsInner({ prompt, promptDelivery, launchSource, - onPromptDelivered + onPromptDelivered, + onPromptHandedOff }: QuickLaunchAgentMenuItemsProps): React.JSX.Element | null { // Why: resolving only the SSH connectionId here made paired-runtime // worktrees fall back to LOCAL detection, listing the client's agents @@ -155,6 +158,9 @@ function QuickLaunchAgentMenuItemsInner({ ) return } + if (result.promptDeliveryResult) { + onPromptHandedOff?.(result.promptDeliveryResult) + } if (result.surface.kind !== 'local-terminal') { return } @@ -181,7 +187,16 @@ function QuickLaunchAgentMenuItemsInner({ toast.message(getLaunchWatchdogTimeoutMessage(label)) }) }, - [worktreeId, groupId, onFocusTerminal, prompt, promptDelivery, launchSource, onPromptDelivered] + [ + worktreeId, + groupId, + onFocusTerminal, + prompt, + promptDelivery, + launchSource, + onPromptDelivered, + onPromptHandedOff + ] ) const enabledDetectedIds = detectedIds ? filterEnabledTuiAgents(detectedIds, disabledAgents) : [] diff --git a/src/renderer/src/lib/notes-send-in-flight.ts b/src/renderer/src/lib/notes-send-in-flight.ts new file mode 100644 index 00000000000..b8f221a6e0e --- /dev/null +++ b/src/renderer/src/lib/notes-send-in-flight.ts @@ -0,0 +1,73 @@ +import { useSyncExternalStore } from 'react' +import type { DiffCommentDeliverySnapshot } from '@/store/slices/diffComments' + +// Why: notes handed to a send leave the next send at once and come back only if that delivery +// fails, as a submitted composer clears and restores on error. Delivered notes are still removed +// by their owner; a hold lives only until its delivery settles. +const holds = new Map() +const listeners = new Set<() => void>() +let version = 0 + +function changed(): void { + version += 1 + for (const listener of listeners) { + listener() + } +} + +/** Takes `keys` out of the next send until `delivered` settles, whatever its result. */ +export function holdNotesForSend(keys: readonly unknown[], delivered: Promise): void { + if (keys.length === 0) { + return + } + for (const key of keys) { + holds.set(key, (holds.get(key) ?? 0) + 1) + } + changed() + const release = (): void => { + for (const key of keys) { + const count = (holds.get(key) ?? 1) - 1 + if (count > 0) { + holds.set(key, count) + } else { + holds.delete(key) + } + } + changed() + } + void delivered.then(release, release) +} + +export function isNoteInFlight(key: unknown): boolean { + return holds.has(key) +} + +/** Changes whenever a hold starts or ends, for memos that filter by `isNoteInFlight`. */ +export function useNotesInFlightVersion(): number { + return useSyncExternalStore( + (listener) => { + listeners.add(listener) + return () => listeners.delete(listener) + }, + () => version, + () => version + ) +} + +/** A note's identity for delivery: an edit makes it a new pending note, as for its removal. */ +export function diffCommentSendKey(note: DiffCommentDeliverySnapshot): string { + return JSON.stringify([ + note.id, + note.body, + note.filePath, + note.lineNumber, + note.startLine ?? null, + note.selectedText ?? null, + note.source ?? null + ]) +} + +export function resetNotesInFlightForTests(): void { + holds.clear() + changed() +} diff --git a/src/renderer/src/store/slices/ui-agent-send-target.test.ts b/src/renderer/src/store/slices/ui-agent-send-target.test.ts index 61f98a2afba..c94e71edd26 100644 --- a/src/renderer/src/store/slices/ui-agent-send-target.test.ts +++ b/src/renderer/src/store/slices/ui-agent-send-target.test.ts @@ -282,6 +282,7 @@ describe('createUISlice agent send target mode', () => { it('sends to the live leaf PTY, runs delivery callback, tracks followup, and closes', async () => { const store = createAgentSendStore() const onPromptDelivered = vi.fn() + const onPromptHandedOff = vi.fn() seedAgentSendState(store) store.getState().openAgentSendPopoverTargetMode({ id: 'send-1', @@ -290,11 +291,16 @@ describe('createUISlice agent send target mode', () => { prompt: 'Review this', label: 'All unsent notes', launchSource: 'notes_send', - onPromptDelivered + onPromptDelivered, + onPromptHandedOff }) await expect(store.getState().sendPromptToSidebarAgentTarget(readyPaneKey)).resolves.toBe(true) + // The notes leave the next send for exactly this send's lifetime. + expect(onPromptHandedOff).toHaveBeenCalledOnce() + await expect(onPromptHandedOff.mock.calls[0][0]).resolves.toMatchObject({ status: 'sent' }) + expect(mocks.sendNotesToActiveAgentSession).toHaveBeenCalledWith({ worktreeId, prompt: 'Review this', diff --git a/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts b/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts index 8beecf0e12d..d267b00b7b8 100644 --- a/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts +++ b/src/renderer/src/store/slices/ui/ui-slice-agent-actions.ts @@ -154,7 +154,7 @@ export function createUiAgentActions( import('@/lib/active-agent-note-send'), import('@/lib/agent-message-send') ]) - const result = await sendMessageToAgent({ + const sending = sendMessageToAgent({ worktreeId: mode.worktreeId, prompt: mode.prompt, target: runningAgentMessageTarget(target) @@ -164,6 +164,8 @@ export function createUiAgentActions( }) return { status: 'status-unavailable' as const, code: 'runtime-unverifiable' as const } }) + mode.onPromptHandedOff?.(sending) + const result = await sending const stillCurrent = (): boolean => { const current = get().agentSendPopoverTargetMode diff --git a/src/renderer/src/store/slices/ui/ui-slice-contract-core.ts b/src/renderer/src/store/slices/ui/ui-slice-contract-core.ts index ffb37c40a01..5d187bcf788 100644 --- a/src/renderer/src/store/slices/ui/ui-slice-contract-core.ts +++ b/src/renderer/src/store/slices/ui/ui-slice-contract-core.ts @@ -39,6 +39,8 @@ export type AgentSendPopoverTargetMode = { sendingPaneKey?: string error?: string onPromptDelivered?: () => void + /** Told the send's own result the moment the prompt is handed to an agent. */ + onPromptHandedOff?: (delivered: Promise) => void } export type OpenAgentSendPopoverTargetModeArgs = { @@ -49,6 +51,7 @@ export type OpenAgentSendPopoverTargetModeArgs = { label: string launchSource: LaunchSource onPromptDelivered?: () => void + onPromptHandedOff?: (delivered: Promise) => void } export type TaskPageData = {