From 0fd16b7d938bbfe68e14ae3b1307b3cd9adabe7e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:30:19 -0700 Subject: [PATCH] fix(notes): notes handed to a send leave the next send until it settles Notes sent to an agent stayed in the notes shelf until their chat delivered them, so a second "Send notes" made while the first new chat was still starting collected the first notes again. With every different request now opening its own chat, those notes reached two chats. When notes or browser annotations are handed to a send (a new agent, a running agent from the menu, or a sidebar agent row), they are held in memory against that send's own delivery result and left out of the next send. Delivered notes are removed as before; a failed, refused or undelivered send releases the hold, so they come back for the next send. The notes menu now builds each scope's prompt from the notes it will send. --- .../BrowserAnnotationSendMenuContent.tsx | 5 +- .../browser-guest-annotate-overlays.tsx | 2 + .../annotate/browser-page-annotation-tray.tsx | 3 + .../use-browser-page-annotation-send.test.tsx | 53 ++++++++ .../use-browser-page-annotation-send.ts | 38 +++++- .../browser-page-chrome-banners.tsx | 3 + .../browser-page-chrome-header.tsx | 1 + .../diff-comments/diff-comment-zone-card.tsx | 3 +- .../components/editor/DiffNotesSendMenu.tsx | 8 +- .../MarkdownPreviewAnnotationComposer.tsx | 2 +- .../components/editor/NotesSendMenu.test.tsx | 120 ++++++++++++++++-- .../src/components/editor/NotesSendMenu.tsx | 42 +++++- .../ReviewNotesSendMenuContent.test.tsx | 13 +- .../editor/ReviewNotesSendMenuContent.tsx | 11 +- .../editor/RichMarkdownReviewNoteLayer.tsx | 6 +- .../editor/use-markdown-preview-foundation.ts | 8 +- .../editor/useRichMarkdownReviewData.ts | 2 +- .../QuickLaunchButton.launch-status.test.tsx | 40 +++++- .../components/tab-bar/QuickLaunchButton.tsx | 19 ++- src/renderer/src/lib/notes-send-in-flight.ts | 73 +++++++++++ .../store/slices/ui-agent-send-target.test.ts | 8 +- .../store/slices/ui/ui-slice-agent-actions.ts | 4 +- .../store/slices/ui/ui-slice-contract-core.ts | 3 + 23 files changed, 411 insertions(+), 56 deletions(-) create mode 100644 src/renderer/src/lib/notes-send-in-flight.ts 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 = {