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.
This commit is contained in:
Brennan Benson
2026-10-04 01:30:19 -07:00
parent 4b72d035de
commit 0fd16b7d93
23 changed files with 411 additions and 56 deletions
@@ -6,13 +6,15 @@ export type BrowserAnnotationSendMenuContentProps = {
groupId: string
prompt: string
onPromptDelivered?: () => void
onPromptHandedOff?: (delivered: Promise<unknown>) => void
}
export function BrowserAnnotationSendMenuContent({
worktreeId,
groupId,
prompt,
onPromptDelivered
onPromptDelivered,
onPromptHandedOff
}: BrowserAnnotationSendMenuContentProps): React.JSX.Element {
return (
<ReviewNotesSendMenuContent
@@ -24,6 +26,7 @@ export function BrowserAnnotationSendMenuContent({
promptDelivery="submit-after-ready"
launchSource="notes_send"
onPromptDelivered={onPromptDelivered}
onPromptHandedOff={onPromptHandedOff}
/>
)
}
@@ -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}
@@ -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<unknown>) => void
handleCopyBrowserAnnotations: () => void
browserAnnotationsCopied: boolean
handleClearBrowserAnnotations: () => void
@@ -135,6 +137,7 @@ export function BrowserPageAnnotationTray({
groupId={activeGroupId ?? worktreeId}
prompt={browserAnnotationsPrompt}
onPromptDelivered={handleBrowserAnnotationsSentToAgent}
onPromptHandedOff={handleBrowserAnnotationsHandedOff}
/>
</DropdownMenuContent>
</DropdownMenu>
@@ -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<typeof createTestStore> } => ({}))
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('')
})
})
@@ -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<unknown>) => 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<ReturnType<typeof setTimeout>>(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<unknown>): 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
}
}
@@ -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<unknown>) => void
handleCopyBrowserAnnotations: () => void
browserAnnotationsCopied: boolean
handleClearBrowserAnnotations: () => void
@@ -155,6 +157,7 @@ export function BrowserPageChromeBanners({
groupId={activeGroupId ?? worktreeId}
prompt={browserAnnotationsPrompt}
onPromptDelivered={handleBrowserAnnotationsSentToAgent}
onPromptHandedOff={handleBrowserAnnotationsHandedOff}
/>
</DropdownMenuContent>
</DropdownMenu>
@@ -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}
@@ -39,7 +39,8 @@ function getSingleCommentSendScopes(
'This note'
),
notes: comment.sentAt ? [] : [comment],
prompt: formatCommentPrompt ? formatCommentPrompt(comment) : formatDiffComments([comment])
formatPrompt: () =>
formatCommentPrompt ? formatCommentPrompt(comment) : formatDiffComments([comment])
}
]
}
@@ -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<NotesSendMenuScope<DiffComment>[]>(() => {
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 (
<NotesSendMenu
@@ -32,7 +32,7 @@ export function MarkdownPreviewSingleNoteSendMenu({
id: 'note',
label: translate('auto.components.editor.MarkdownPreview.f37b98999e', 'This note'),
notes: note.sentAt ? [] : [note],
prompt: formatMarkdownReviewNotes([note], content)
formatPrompt: (notes) => formatMarkdownReviewNotes(notes, content)
}
]}
targetModeLabel="This note"
@@ -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<string, unknown>
}
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<T>(factory: () => T): T {
return factory()
},
useSyncExternalStore<T>(_subscribe: unknown, getSnapshot: () => T): T {
return getSnapshot()
},
useState<T>(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<string, unknown>, 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<unknown>) =>
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')
})
})
@@ -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<TNote> = {
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<TNote> = Omit<NotesSendMenuScope<TNote>, '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<TNote>({
export function NotesSendMenu<TNote extends DiffCommentDeliverySnapshot>({
worktreeId,
groupId,
modeIdParts,
@@ -82,7 +94,18 @@ export function NotesSendMenu<TNote>({
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<SendableNotesScope<TNote>[]>(() => {
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<TNote>({
},
[onDelivered]
)
const holdInFlight = useCallback(
(notes: readonly TNote[]) => (delivered: Promise<unknown>) =>
holdNotesForSend(notes.map(diffCommentSendKey), delivered),
[]
)
const openTargetMode = useCallback(
(scope: NotesSendMenuScope<TNote>) => {
(scope: SendableNotesScope<TNote>) => {
if (scope.notes.length === 0) {
return
}
@@ -108,10 +136,12 @@ export function NotesSendMenu<TNote>({
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<TNote>({
<DropdownMenuLabel>
{translate('auto.components.editor.NotesSendMenu.44dc5e60a6', 'Send notes')}
</DropdownMenuLabel>
{scopes.map((scope) => (
{sendableScopes.map((scope) => (
<DropdownMenuSub key={scope.id}>
<DropdownMenuSubTrigger
disabled={scope.notes.length === 0}
@@ -244,6 +274,7 @@ export function NotesSendMenu<TNote>({
promptDelivery="submit-after-ready"
launchSource="notes_send"
onPromptDelivered={() => markDelivered(scope.notes)}
onPromptHandedOff={holdInFlight(scope.notes)}
/>
</DropdownMenuSubContent>
</DropdownMenuSub>
@@ -261,6 +292,7 @@ export function NotesSendMenu<TNote>({
markDelivered(defaultScope.notes)
}
}}
onPromptHandedOff={holdInFlight(defaultScope?.notes ?? [])}
/>
)}
</DropdownMenuContent>
@@ -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
})
})
})
@@ -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<unknown>) => 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}
/>
</>
)
@@ -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"
@@ -101,20 +101,16 @@ export function useMarkdownPreviewFoundation({
() => markdownReviewNotes.filter((note) => !note.sentAt),
[markdownReviewNotes]
)
const unsentMarkdownReviewPrompt = useMemo(
() => formatMarkdownReviewNotes(unsentMarkdownReviewNotes, renderedContent),
[renderedContent, unsentMarkdownReviewNotes]
)
const unsentMarkdownReviewScope = useMemo<NotesSendMenuScope<MarkdownReviewNote>[]>(
() => [
{
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
@@ -65,7 +65,7 @@ export function useRichMarkdownReviewData({
'All unsent notes'
),
notes: unsentNotes,
prompt: formatMarkdownReviewNotes(unsentNotes, markdownReviewContent)
formatPrompt: (notes) => formatMarkdownReviewNotes(notes, markdownReviewContent)
}
]
}, [markdownReviewContent, markdownReviewNotes])
@@ -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 }) => <span>{agent}</span>
}))
vi.mock('@/components/ui/dropdown-menu', () => ({
DropdownMenuItem: ({ children, disabled, title }: { children: ReactNode } & DivProps) => (
<div aria-disabled={disabled ? 'true' : 'false'} title={title}>
DropdownMenuItem: ({
children,
disabled,
title,
onSelect
}: { children: ReactNode } & DivProps) => (
<div aria-disabled={disabled ? 'true' : 'false'} title={title} onClick={onSelect}>
{children}
</div>
),
@@ -46,9 +51,10 @@ vi.mock('@/i18n/i18n', () => ({
translate: (_key: string, fallback: string, values?: Record<string, string>) =>
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(
<QuickLaunchAgentMenuItems
worktreeId={WORKTREE_ID}
groupId="group-1"
onFocusTerminal={vi.fn()}
prompt="review notes"
promptDelivery="submit-after-ready"
onPromptHandedOff={onPromptHandedOff}
/>
)
fireEvent.click(document.querySelector('[title="Launch Codex in a new terminal"]')!)
expect(onPromptHandedOff).toHaveBeenCalledWith(delivery)
})
})
@@ -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<unknown>) => 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) : []
@@ -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<unknown, number>()
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<unknown>): 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()
}
@@ -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',
@@ -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
@@ -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<unknown>) => void
}
export type OpenAgentSendPopoverTargetModeArgs = {
@@ -49,6 +51,7 @@ export type OpenAgentSendPopoverTargetModeArgs = {
label: string
launchSource: LaunchSource
onPromptDelivered?: () => void
onPromptHandedOff?: (delivered: Promise<unknown>) => void
}
export type TaskPageData = {