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.test.tsx b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.test.tsx index 1fe0b6bfd46..162e1c64bf0 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.test.tsx +++ b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.test.tsx @@ -87,6 +87,7 @@ function renderTray(currentUrl?: string): { activeGroupId={undefined} browserAnnotationsPrompt="prompt" handleBrowserAnnotationsSentToAgent={vi.fn()} + handleBrowserAnnotationsHandedOff={vi.fn()} handleCopyBrowserAnnotations={vi.fn()} browserAnnotationsCopied={false} handleClearBrowserAnnotations={vi.fn()} 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..6211684fb54 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 @@ -111,7 +113,12 @@ export function BrowserPageAnnotationTray({ - @@ -135,6 +142,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..8eb9934c5a8 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,65 @@ 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('') + }) + + it('offers no send while every annotation is already on its way', () => { + const view = mount() + act(() => view.result.current.handleBrowserAnnotationsHandedOff(new Promise(() => undefined))) + + act(() => view.result.current.handleAnnotationTraySendOpenChange(true)) + expect(mode).toBeUndefined() + + render() + expect(screen.getByRole('button', { name: 'Send' })).toBeDisabled() + }) +}) 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..700094fe874 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,32 @@ 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, handleBrowserAnnotationsSentToAgent), + [handleBrowserAnnotationsSentToAgent, sendableAnnotations] + ) + const handleClearBrowserAnnotations = useCallback((): void => { if (browserAnnotationsRef.current.length === 0) { return @@ -115,6 +137,10 @@ export function useBrowserPageAnnotationSend({ const handleAnnotationSendOpenChange = useCallback( (modeId: string, open: boolean): void => { if (open) { + // Every annotation may already be on its way: there is nothing to send. + if (!browserAnnotationsPrompt) { + return + } openAgentSendPopoverTargetMode({ id: modeId, worktreeId, @@ -125,7 +151,8 @@ export function useBrowserPageAnnotationSend({ 'Browser annotations' ), launchSource: 'notes_send', - onPromptDelivered: handleBrowserAnnotationsSentToAgent + onPromptDelivered: handleBrowserAnnotationsSentToAgent, + onPromptHandedOff: handleBrowserAnnotationsHandedOff }) } else { closeAgentSendPopoverTargetMode(modeId) @@ -134,6 +161,7 @@ export function useBrowserPageAnnotationSend({ [ browserAnnotationsPrompt, handleBrowserAnnotationsSentToAgent, + handleBrowserAnnotationsHandedOff, closeAgentSendPopoverTargetMode, openAgentSendPopoverTargetMode, worktreeId @@ -199,6 +227,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..5bced58139a 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 @@ -131,7 +133,12 @@ export function BrowserPageChromeBanners({ - @@ -155,6 +162,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..b8758850895 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,121 @@ 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() + + // Each send clears only its own note; a repeated clear of A is a no-op for its owner. + expect(onDelivered).not.toHaveBeenCalledWith([noteA, noteB]) + expect(onDelivered).toHaveBeenCalledWith([noteA]) + expect(onDelivered).toHaveBeenCalledWith([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') + }) + + it('offers no send once every note is on its way', () => { + contentProps(renderMenu({ scopes: scopeOf([noteA]) })).onPromptHandedOff( + new Promise(() => undefined) + ) + + const tree = renderMenu({ scopes: scopeOf([noteA]) }) + expect(findByType(tree, 'button').props.disabled).toBe(true) + invoke(findByType(tree, 'DropdownMenu').props, 'onOpenChange', true) + expect(storeMocks.openAgentSendPopoverTargetMode).not.toHaveBeenCalled() + }) + + it('says the notes are on their way, not sent, while every note is held', () => { + contentProps( + renderMenu({ scopes: scopeOf([noteA]), disabledTooltip: 'Note already sent' }) + ).onPromptHandedOff(new Promise(() => undefined)) + + const tree = renderMenu({ scopes: scopeOf([noteA]), disabledTooltip: 'Note already sent' }) + + expect(findByType(tree, 'button').props.title).toBe('Sending…') + }) + + // A failed new chat's Retry delivers them after the send's own callback is gone. + it('clears notes whose send reports delivery later', async () => { + const onDelivered = vi.fn() + const delivered = Promise.resolve({ delivered: true }) + contentProps(renderMenu({ scopes: scopeOf([noteA]), onDelivered })).onPromptHandedOff(delivered) + + await delivered + await Promise.resolve() + + expect(onDelivered).toHaveBeenCalledWith([noteA]) + }) +}) diff --git a/src/renderer/src/components/editor/NotesSendMenu.tsx b/src/renderer/src/components/editor/NotesSendMenu.tsx index 8315d4512df..bb8cac0511a 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,12 +94,27 @@ 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 }, [defaultScopeId, enabledScopes]) const hasDeliverableNotes = enabledScopes.length > 0 + // Notes only on their way to an agent are not sent yet. + const disabledTitle = scopes.some((scope) => scope.notes.length > 0) + ? translate('components.native-chat.question.sending', 'Sending…') + : disabledTooltip const markDelivered = useCallback( (notes: readonly TNote[]) => { @@ -95,9 +122,14 @@ export function NotesSendMenu({ }, [onDelivered] ) + const holdInFlight = useCallback( + (notes: readonly TNote[]) => (delivered: Promise) => + holdNotesForSend(notes.map(diffCommentSendKey), delivered, () => markDelivered(notes)), + [markDelivered] + ) const openTargetMode = useCallback( - (scope: NotesSendMenuScope) => { + (scope: SendableNotesScope) => { if (scope.notes.length === 0) { return } @@ -108,10 +140,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, @@ -181,7 +215,7 @@ export function NotesSendMenu({ triggerClassName )} disabled={!hasDeliverableNotes} - title={hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTooltip} + title={hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTitle} aria-label={ triggerLabel ? translate( @@ -212,7 +246,7 @@ export function NotesSendMenu({ - {hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTooltip} + {hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTitle} ({ {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 +296,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..af4c1e98445 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,23 @@ 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 }) }) + + // Every note already on its way leaves an empty prompt: no agent may start with no text. + it('disables the new-agent launcher when there is nothing left to send', () => { + expect(findByType(render({ prompt: '' }), 'QuickLaunchAgentMenuItems').props.disabled).toBe( + true + ) + expect(findByType(render(), 'QuickLaunchAgentMenuItems').props.disabled).toBe(false) + }) }) diff --git a/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx b/src/renderer/src/components/editor/ReviewNotesSendMenuContent.tsx index 688113e7a01..935d98dd986 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,8 @@ export function ReviewNotesSendMenuContent({ promptDelivery={promptDelivery} launchSource={launchSource} onPromptDelivered={onPromptDelivered} + onPromptHandedOff={onPromptHandedOff} + disabled={!hasPrompt} /> ) 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/native-chat/structured-agent-session-outbox-entry-watch.ts b/src/renderer/src/components/native-chat/structured-agent-session-outbox-entry-watch.ts new file mode 100644 index 00000000000..71644f31340 --- /dev/null +++ b/src/renderer/src/components/native-chat/structured-agent-session-outbox-entry-watch.ts @@ -0,0 +1,63 @@ +import type { StructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' + +/** How a watched message left its chat's outbox: sent on (accepted, or handed to the host or the + * composer), or thrown away with the chat. */ +export type StructuredAgentSessionOutboxEntryRemoval = 'spent' | 'discarded' + +type EntryWatch = { + entry: StructuredAgentSessionOutboxEntry + onGone: (removal: StructuredAgentSessionOutboxEntryRemoval) => void +} + +const watchesBySession = new Map>() + +// A user's Retry of a refused message gives it a new id; its text and queue time stay. +function stillQueued( + watched: StructuredAgentSessionOutboxEntry, + entries: readonly StructuredAgentSessionOutboxEntry[] +): boolean { + const body = JSON.stringify(watched.body) + return entries.some( + (entry) => + entry.clientMessageId === watched.clientMessageId || + (entry.queuedAt === watched.queuedAt && JSON.stringify(entry.body) === body) + ) +} + +/** Calls `onGone` once, when `entry` leaves its session's outbox. */ +export function watchStructuredAgentSessionOutboxEntry( + entry: StructuredAgentSessionOutboxEntry, + onGone: (removal: StructuredAgentSessionOutboxEntryRemoval) => void +): () => void { + const watches = watchesBySession.get(entry.sessionId) ?? new Set() + watchesBySession.set(entry.sessionId, watches) + const watch: EntryWatch = { entry, onGone } + watches.add(watch) + return () => { + watches.delete(watch) + if (watches.size === 0 && watchesBySession.get(entry.sessionId) === watches) { + watchesBySession.delete(entry.sessionId) + } + } +} + +/** Run by the outbox's one write funnel after a change takes effect. */ +export function settleStructuredAgentSessionOutboxEntryWatches( + sessionId: string, + entries: readonly StructuredAgentSessionOutboxEntry[], + removal: StructuredAgentSessionOutboxEntryRemoval +): void { + const watches = watchesBySession.get(sessionId) + if (!watches) { + return + } + for (const watch of watches) { + if (!stillQueued(watch.entry, entries)) { + watches.delete(watch) + watch.onGone(removal) + } + } + if (watches.size === 0) { + watchesBySession.delete(sessionId) + } +} diff --git a/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts b/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts index e240e220a02..b385fa64196 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-outbox-storage.ts @@ -7,6 +7,10 @@ import { import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation' import { createBrowserUuid } from '@/lib/browser-uuid' import { noteStructuredAgentSessionOutboxCommitted } from './structured-agent-session-entry-endings' +import { + settleStructuredAgentSessionOutboxEntryWatches, + type StructuredAgentSessionOutboxEntryRemoval +} from './structured-agent-session-outbox-entry-watch' const OUTBOX_PREFIX = 'orca:desktopStructuredAgentSessionOutbox:v1:' @@ -160,6 +164,15 @@ export function commitStructuredAgentSessionOutbox( sessionId: string, entries: StructuredAgentSessionOutboxEntry[], options: { onlyIfSaved?: boolean } = {} +): boolean { + return commitOutbox(sessionId, entries, options, 'spent') +} + +function commitOutbox( + sessionId: string, + entries: StructuredAgentSessionOutboxEntry[], + options: { onlyIfSaved?: boolean }, + removal: StructuredAgentSessionOutboxEntryRemoval ): boolean { const saved = writeOutbox(sessionId, entries) if (!saved && options.onlyIfSaved) { @@ -173,6 +186,7 @@ export function commitStructuredAgentSessionOutbox( } } noteStructuredAgentSessionOutboxCommitted(sessionId, entries) + settleStructuredAgentSessionOutboxEntryWatches(sessionId, entries, removal) return saved } @@ -211,5 +225,5 @@ export function enqueueStructuredAgentSessionLaunchPrompt( } export function discardStructuredAgentSessionLaunchOutbox(sessionId: string): void { - commitStructuredAgentSessionOutbox(sessionId, []) + commitOutbox(sessionId, [], {}, 'discarded') } 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 new file mode 100644 index 00000000000..1f06c3307c3 --- /dev/null +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.launch-status.test.tsx @@ -0,0 +1,224 @@ +// @vitest-environment happy-dom + +import type { ReactNode } from '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 { + BLANK_STRUCTURED_LAUNCH_REQUEST, + structuredLaunchRequest, + type StructuredLaunchAttempt +} from '@/lib/structured-agent-session-launch-request' + +vi.mock('@/hooks/useDetectedAgents', () => ({ + useDetectedAgents: () => ({ detectedIds: ['claude', 'codex'] }) +})) +vi.mock('@/hooks/useShortcutLabel', () => ({ useOptionalShortcutLabel: () => null })) +vi.mock('@/store', () => { + const state = { + settings: { defaultTuiAgent: 'codex', disabledTuiAgents: [] }, + worktreesByRepo: {}, + repos: [], + openSettingsPage: vi.fn(), + openSettingsTarget: vi.fn() + } + const useAppStore = Object.assign((selector: (s: typeof state) => unknown) => selector(state), { + getState: () => state + }) + return { useAppStore } +}) +vi.mock('@/lib/agent-catalog', () => ({ + getAgentCatalog: () => [ + { id: 'claude', label: 'Claude' }, + { id: 'codex', label: 'Codex' } + ], + AgentIcon: ({ agent }: { agent: string }) => {agent} +})) +vi.mock('@/components/ui/dropdown-menu', () => ({ + DropdownMenuItem: ({ + children, + disabled, + title, + onSelect + }: { children: ReactNode } & DivProps) => ( +
+ {children} +
+ ), + DropdownMenuShortcut: ({ children }: { children: ReactNode }) => {children} +})) +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, values?: Record) => + fallback.replace('{{value0}}', values?.value0 ?? '') +})) +const launchMock = vi.hoisted(() => vi.fn()) +vi.mock('@/lib/launch-agent-in-new-tab', () => ({ launchAgentInNewTab: launchMock })) + +type DivProps = { disabled?: boolean; title?: string; onSelect?: () => void } + +import { QuickLaunchAgentMenuItems } from './QuickLaunchButton' +import { + resetStructuredAgentLaunchRegistryForTests, + setStructuredLaunchState +} from '@/lib/structured-agent-session-launch-registry' + +const WORKTREE_ID = 'worktree-1' + +function registerLaunch( + agent: 'claude' | 'codex', + outcome: 'pending' | 'failed', + attempt: StructuredLaunchAttempt = { + kind: 'first', + request: BLANK_STRUCTURED_LAUNCH_REQUEST, + stagedEntry: null + } +): void { + const sessionId = `${agent}-session` + setStructuredLaunchState({ + identity: `${agent}:${WORKTREE_ID}`, + intent: { + worktreeId: WORKTREE_ID, + sessionId, + executionHostId: 'local', + target: { kind: 'local' }, + agent, + params: { + envelope: { + sessionId, + clientOperationId: `operation-${sessionId}`, + expectedRuntimeFence: null, + payloadFingerprint: `fingerprint-${sessionId}` + }, + worktree: `id:${WORKTREE_ID}`, + agent + } + }, + promptDelivery: undefined, + callers: { + outcome, + attempt, + entries: new Set(), + promptDeliveryResults: new Set(), + onSettled: () => undefined + }, + promise: new Promise(() => undefined), + visibilityUnknown: false, + cancelled: false, + selection: { held: {} } + } satisfies StructuredLaunchState) +} + +function agentRowDisabled(label: string): string | null | undefined { + return document + .querySelector(`[title="Launch ${label} in a new terminal"]`) + ?.getAttribute('aria-disabled') +} + +describe('QuickLaunchAgentMenuItems launch status', () => { + beforeEach(() => { + localStorage.clear() + resetStructuredAgentLaunchRegistryForTests() + }) + afterEach(cleanup) + + it('keeps an agent whose chat failed to start launchable while a starting one waits', () => { + registerLaunch('claude', 'pending') + registerLaunch('codex', 'failed') + + render( + + ) + + expect(agentRowDisabled('Claude')).toBe('true') + expect(agentRowDisabled('Codex')).toBe('false') + }) + + // A pick then opens a new chat; only a new start's own create would be joined. + it("keeps an agent launchable while a failed chat's Retry is in flight", () => { + registerLaunch('claude', 'pending') + registerLaunch('codex', 'pending', { kind: 'retry' }) + + render( + + ) + + expect(agentRowDisabled('Claude')).toBe('true') + expect(agentRowDisabled('Codex')).toBe('false') + }) + + // A pick joins only a start of the same request; any other opens its own chat. + it('disables an agent only for the request its starting chat carries', () => { + registerLaunch('codex', 'pending', { + kind: 'first', + request: structuredLaunchRequest({ prompt: 'review notes' }), + stagedEntry: null + }) + const menu = (prompt?: string) => ( + + ) + + render(menu()) + expect(agentRowDisabled('Codex')).toBe('false') + cleanup() + render(menu('other notes')) + expect(agentRowDisabled('Codex')).toBe('false') + cleanup() + 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 outcome to the notes menu", async () => { + 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).toHaveBeenCalledOnce() + await expect(onPromptHandedOff.mock.calls[0][0]).resolves.toEqual({ delivered: true }) + }) + + it('starts no agent when the menu has nothing left to send', () => { + launchMock.mockClear() + render( + + ) + + expect(agentRowDisabled('Codex')).toBe('true') + fireEvent.click(document.querySelector('[title="Launch Codex in a new terminal"]')!) + expect(launchMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx index 026944575c8..a5eda32b0e6 100644 --- a/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx +++ b/src/renderer/src/components/tab-bar/QuickLaunchButton.tsx @@ -17,6 +17,8 @@ import { } from '../../../../shared/tui-agent-selection' import { translate } from '@/i18n/i18n' import { useStructuredAgentLaunchStatus } from '@/lib/structured-agent-session-launch' +import { structuredLaunchRequest } from '@/lib/structured-agent-session-launch-request' +import { newAgentPromptOutcome } from '@/lib/new-agent-prompt-outcome' export type QuickLaunchAgentMenuItemsProps = { worktreeId: string @@ -37,6 +39,10 @@ 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 + /** Nothing to send: e.g. every note is already on its way, so no agent is started. */ + disabled?: boolean } function getCatalogEntry(agent: TuiAgent): { id: TuiAgent; label: string } | null { @@ -103,7 +109,9 @@ function QuickLaunchAgentMenuItemsInner({ prompt, promptDelivery, launchSource, - onPromptDelivered + onPromptDelivered, + onPromptHandedOff, + disabled = false }: 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 @@ -119,10 +127,11 @@ function QuickLaunchAgentMenuItemsInner({ const openSettingsTarget = useAppStore((s) => s.openSettingsTarget) const newAgentShortcut = useOptionalShortcutLabel('tab.newAgent') // One hook per structured provider: the launch registry is keyed by agent, and hooks cannot run - // inside the agent list's render loop. + // inside the agent list's render loop. Only a start of this menu's own request is joined. + const launchRequest = structuredLaunchRequest({ prompt, promptDelivery }) const structuredLaunchStatusByAgent = { - claude: useStructuredAgentLaunchStatus(worktreeId, 'claude'), - codex: useStructuredAgentLaunchStatus(worktreeId, 'codex') + claude: useStructuredAgentLaunchStatus(worktreeId, 'claude', launchRequest), + codex: useStructuredAgentLaunchStatus(worktreeId, 'codex', launchRequest) } const openAgentSettings = useCallback(() => { @@ -132,6 +141,9 @@ function QuickLaunchAgentMenuItemsInner({ const runLaunch = useCallback( (agent: TuiAgent) => { + if (disabled) { + return + } const entry = getCatalogEntry(agent) const label = entry?.label ?? agent const result = launchAgentInNewTab({ @@ -153,6 +165,17 @@ function QuickLaunchAgentMenuItemsInner({ ) return } + if (onPromptHandedOff && result.promptDeliveryResult) { + onPromptHandedOff( + newAgentPromptOutcome({ + prompt: prompt ?? '', + ...(result.surface.kind === 'local-agent-session' + ? { sessionId: result.surface.sessionId } + : {}), + delivery: result.promptDeliveryResult + }) + ) + } if (result.surface.kind !== 'local-terminal') { return } @@ -179,7 +202,17 @@ function QuickLaunchAgentMenuItemsInner({ toast.message(getLaunchWatchdogTimeoutMessage(label)) }) }, - [worktreeId, groupId, onFocusTerminal, prompt, promptDelivery, launchSource, onPromptDelivered] + [ + worktreeId, + groupId, + onFocusTerminal, + prompt, + promptDelivery, + launchSource, + onPromptDelivered, + onPromptHandedOff, + disabled + ] ) const enabledDetectedIds = detectedIds ? filterEnabledTuiAgents(detectedIds, disabledAgents) : [] @@ -210,7 +243,7 @@ function QuickLaunchAgentMenuItemsInner({ return ( runLaunch(agent)} className="gap-2 rounded-[7px] px-2 py-1.5 text-[12px] leading-5 font-medium" title={translate( diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx index d0766a7b1cf..0e876e994da 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx @@ -34,6 +34,7 @@ import { } from './tab-create-entry-copy' import { EMPTY_AGENT_OPTIONS, EMPTY_MENU_OPTIONS } from './tab-create-entry-empty-options' import { useStructuredAgentLaunchStatus } from '@/lib/structured-agent-session-launch' +import { BLANK_STRUCTURED_LAUNCH_REQUEST } from '@/lib/structured-agent-session-launch-request' import { isAgentSessionHandleProvider } from '../../../../shared/agent-session-provider-handle' import type { TuiAgent } from '../../../../shared/tui-agent' import type { TabEntryActionClassification } from './tab-create-entry-classifier' @@ -65,8 +66,8 @@ function TabBarCreateEntrySession({ // One hook per structured provider: the launch registry is keyed by agent, and hooks cannot run // inside the option render loop. const structuredLaunchStatusByAgent = { - claude: useStructuredAgentLaunchStatus(worktreeId, 'claude'), - codex: useStructuredAgentLaunchStatus(worktreeId, 'codex') + claude: useStructuredAgentLaunchStatus(worktreeId, 'claude', BLANK_STRUCTURED_LAUNCH_REQUEST), + codex: useStructuredAgentLaunchStatus(worktreeId, 'codex', BLANK_STRUCTURED_LAUNCH_REQUEST) } const isStructuredLaunchPending = (agent: TuiAgent): boolean => isAgentSessionHandleProvider(agent) && structuredLaunchStatusByAgent[agent] === 'pending' diff --git a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts index c6d49331c2d..53578fdce21 100644 --- a/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts +++ b/src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts @@ -471,7 +471,7 @@ describe('structured chat adoption guard on the launch path', () => { await vi.waitFor(() => expect(mockToastError).not.toHaveBeenCalled()) }) - it('does not create a sibling when post-create visibility proof is unknown', async () => { + it('re-checks a chat whose visibility proof is unknown through its own Retry, with its own intent', async () => { store.unifiedTabsByWorktree = {} const firstIntent = structuredLaunchIntent('wt-1', 'codex-session-1') const secondIntent = structuredLaunchIntent('wt-1', 'codex-session-2') @@ -519,7 +519,9 @@ describe('structured chat adoption guard on the launch path', () => { ) expect(mockToastError).not.toHaveBeenCalled() - launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) + // A new launch would open a new chat; the unconfirmed one is re-checked by its own Retry. + const { retryStructuredAgentSessionLaunch } = await import('./structured-agent-session-launch') + expect(retryStructuredAgentSessionLaunch('wt-1', firstIntent.sessionId)).toBe(true) await vi.waitFor(() => expect(mockRefreshLocalStructuredSessionTabs).toHaveBeenCalledTimes(3)) expect(mockCreateStructuredCodexSessionLaunchIntent).toHaveBeenCalledTimes(1) diff --git a/src/renderer/src/lib/new-agent-prompt-outcome.test.ts b/src/renderer/src/lib/new-agent-prompt-outcome.test.ts new file mode 100644 index 00000000000..dd26ded38b2 --- /dev/null +++ b/src/renderer/src/lib/new-agent-prompt-outcome.test.ts @@ -0,0 +1,234 @@ +// @vitest-environment happy-dom + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-session-contracts' +import type { StructuredAgentSessionLaunchIntent } from '@/lib/launch-structured-agent-session' + +const mocks = vi.hoisted(() => ({ + callStructuredAgentSession: vi.fn(), + createIntent: vi.fn(), + launch: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), message: vi.fn() } })) + +vi.mock('@/lib/launch-structured-agent-session', () => { + class StructuredAgentSessionCreateRefusalError extends Error {} + return { + createStructuredAgentSessionLaunchIntent: mocks.createIntent, + retryStructuredAgentSessionLaunchIntent: (intent: unknown) => intent, + restoreStructuredAgentSessionLaunchIntent: vi.fn(), + abandonStructuredAgentSessionLaunchIntent: vi.fn(), + launchStructuredAgentSession: mocks.launch, + StructuredAgentSessionCreateRefusalError + } +}) + +vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({ + refreshLocalStructuredSessionTabs: vi.fn() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.callStructuredAgentSession +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ + unifiedTabsByWorktree: {}, + seedNativeChatLaunchDraft: vi.fn(), + clearNativeChatLaunchDraft: vi.fn() + }), + subscribe: () => () => undefined + } +})) + +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) + +vi.mock('@/lib/agent-catalog', () => ({ + getAgentLabel: () => 'Codex', + getAgentCatalog: () => [{ id: 'codex', label: 'Codex' }] +})) + +import { StructuredAgentSessionCreateRefusalError } from '@/lib/launch-structured-agent-session' +import { refreshLocalStructuredSessionTabs } from '@/runtime/local-structured-session-tabs-sync' +import { + mutateStructuredAgentSessionLaunchPrompt, + readOutbox +} from '@/components/native-chat/structured-agent-session-outbox-storage' +import { + cancelStructuredAgentLaunch, + getStructuredAgentSessionLaunchLifecycle, + retryStructuredAgentSessionLaunch, + startStructuredAgentLaunch +} from './structured-agent-session-launch' +import { resetStructuredAgentLaunchPersistenceForTests } from './structured-agent-session-launch-persistence' +import { resetStructuredAgentLaunchRegistryForTests } from './structured-agent-session-launch-registry' +import { + holdNotesForSend, + isNoteInFlight, + resetNotesInFlightForTests +} from './notes-send-in-flight' +import { newAgentPromptOutcome } from './new-agent-prompt-outcome' + +const WORKTREE_ID = 'wt-notes-new-agent' +const NOTES = 'review notes' + +function launchIntent(sessionId: string): StructuredAgentSessionLaunchIntent { + return { + worktreeId: WORKTREE_ID, + sessionId, + executionHostId: 'local', + target: { kind: 'local' }, + agent: 'codex', + params: { + envelope: { + sessionId, + clientOperationId: `operation-${sessionId}`, + expectedRuntimeFence: null, + payloadFingerprint: `fingerprint-${sessionId}` + }, + worktree: `id:${WORKTREE_ID}`, + agent: 'codex' + } + } +} + +function published(sessionId: string): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [ + { + type: 'agent-session', + id: 'tab-1', + title: 'Codex', + sessionId, + agent: 'codex', + isActive: false + } + ] + } +} + +async function settle(): Promise { + for (let i = 0; i < 20; i += 1) { + await Promise.resolve() + } +} + +const chat = launchIntent('session-notes') + +/** What the notes menu does with a "New agent" pick: the launch, then the hold on its outcome. */ +function sendNotesToNewAgent(options: { paired?: boolean } = {}) { + const onDelivered = vi.fn() + const launch = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: NOTES, + promptDelivery: 'submit-after-ready' + }) + holdNotesForSend( + ['note-a'], + newAgentPromptOutcome({ + prompt: NOTES, + ...(options.paired ? {} : { sessionId: launch.sessionId }), + delivery: launch.promptDeliveryResult! + }), + onDelivered + ) + return { launch, onDelivered } +} + +/** A start the host refused outright: the chat shows it failed, with Retry. */ +async function failTheStart(): Promise { + await settle() + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, chat.sessionId)).toBe('failed') +} + +describe('notes sent to a new agent', () => { + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + resetNotesInFlightForTests() + mocks.createIntent.mockReturnValue(chat) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([published(chat.sessionId)]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('leaves the shelf once the new chat delivers them', async () => { + mocks.launch.mockResolvedValue({ sessionId: chat.sessionId, fence: 1 }) + const { onDelivered } = sendNotesToNewAgent() + expect(isNoteInFlight('note-a')).toBe(true) + + await settle() + + expect(onDelivered).toHaveBeenCalledOnce() + expect(isNoteInFlight('note-a')).toBe(false) + }) + + it('stay held, not resendable, while a failed chat keeps them for its Retry', async () => { + mocks.launch.mockRejectedValue(new StructuredAgentSessionCreateRefusalError('unsupported')) + const { onDelivered } = sendNotesToNewAgent() + await failTheStart() + + expect(readOutbox(chat.sessionId)).toHaveLength(1) + expect(isNoteInFlight('note-a')).toBe(true) + expect(onDelivered).not.toHaveBeenCalled() + }) + + it("leave the shelf when that chat's Retry delivers them", async () => { + mocks.launch.mockRejectedValueOnce(new StructuredAgentSessionCreateRefusalError('unsupported')) + const { onDelivered } = sendNotesToNewAgent() + await failTheStart() + mocks.launch.mockResolvedValue({ sessionId: chat.sessionId, fence: 1 }) + + expect(retryStructuredAgentSessionLaunch(WORKTREE_ID, chat.sessionId)).toBe(true) + await settle() + // The open chat's own send accepts the staged prompt, as every dispatch does. + const [entry] = readOutbox(chat.sessionId) + mutateStructuredAgentSessionLaunchPrompt(chat.sessionId, entry.clientMessageId, () => null) + await settle() + + expect(onDelivered).toHaveBeenCalledOnce() + expect(isNoteInFlight('note-a')).toBe(false) + }) + + it('come back to the shelf when the failed chat is closed', async () => { + mocks.launch.mockRejectedValue(new StructuredAgentSessionCreateRefusalError('unsupported')) + const { onDelivered } = sendNotesToNewAgent() + await failTheStart() + + cancelStructuredAgentLaunch(WORKTREE_ID, chat.sessionId) + await settle() + + expect(isNoteInFlight('note-a')).toBe(false) + expect(onDelivered).not.toHaveBeenCalled() + }) + + it('come back when a chat still starting is closed, without waiting on its create', async () => { + mocks.launch.mockImplementation(() => new Promise(() => undefined)) + const { onDelivered } = sendNotesToNewAgent() + + cancelStructuredAgentLaunch(WORKTREE_ID, chat.sessionId) + await settle() + + expect(isNoteInFlight('note-a')).toBe(false) + expect(onDelivered).not.toHaveBeenCalled() + }) + + it("stay held by a paired server's failed chat, found once its start settles", async () => { + mocks.launch.mockRejectedValue(new StructuredAgentSessionCreateRefusalError('unsupported')) + sendNotesToNewAgent({ paired: true }) + await failTheStart() + + expect(isNoteInFlight('note-a')).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/new-agent-prompt-outcome.ts b/src/renderer/src/lib/new-agent-prompt-outcome.ts new file mode 100644 index 00000000000..8fff9022213 --- /dev/null +++ b/src/renderer/src/lib/new-agent-prompt-outcome.ts @@ -0,0 +1,62 @@ +import type { StructuredAgentSessionOutboxEntry } from '../../../shared/structured-agent-session-outbox' +import { getStructuredAgentSessionOutbox } from '@/components/native-chat/structured-agent-session-outbox-storage' +import { watchStructuredAgentSessionOutboxEntry } from '@/components/native-chat/structured-agent-session-outbox-entry-watch' +import { structuredLaunchStates } from './structured-agent-session-launch-registry' + +export type NewAgentPromptOutcome = { delivered: boolean } + +function stagedLaunchPrompt( + sessionId: string, + text: string +): StructuredAgentSessionOutboxEntry | undefined { + return getStructuredAgentSessionOutbox(sessionId).findLast( + (entry) => + entry.source === 'launch' && + entry.body.blocks.some((block) => block.type === 'text' && block.text === text) + ) +} + +/** A failed or unconfirmed chat keeps its prompt staged for its own Retry or re-check. */ +function stagedInAnyLaunch(text: string): StructuredAgentSessionOutboxEntry | undefined { + let found: StructuredAgentSessionOutboxEntry | undefined + for (const state of structuredLaunchStates()) { + found = stagedLaunchPrompt(state.intent.sessionId, text) ?? found + } + return found +} + +function outcomeOf(entry: StructuredAgentSessionOutboxEntry): Promise { + return new Promise((resolve) => { + watchStructuredAgentSessionOutboxEntry(entry, (removal) => + resolve({ delivered: removal === 'spent' }) + ) + }) +} + +/** + * Settles once a new agent's prompt is sent on (delivered) or thrown away with its chat. A chat's + * staged prompt is the authority, so a Retry or re-check that sends it later still counts, and a + * failed start does not hand the text back while that chat still holds it. + */ +export function newAgentPromptOutcome(args: { + prompt: string + sessionId?: string + delivery: Promise<{ delivered: boolean }> +}): Promise { + const text = args.prompt.trim() + const staged = args.sessionId ? stagedLaunchPrompt(args.sessionId, text) : undefined + if (staged) { + return outcomeOf(staged) + } + // A paired server's chat exists only once it is admitted, so look again after the start. + const afterStart = ( + delivered: boolean + ): Promise | NewAgentPromptOutcome => { + const kept = delivered ? undefined : stagedInAnyLaunch(text) + return kept ? outcomeOf(kept) : { delivered } + } + return args.delivery.then( + (result) => afterStart(result.delivered), + () => afterStart(false) + ) +} 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..cd50f23dc7d --- /dev/null +++ b/src/renderer/src/lib/notes-send-in-flight.ts @@ -0,0 +1,96 @@ +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() + } +} + +function reportsDelivered(result: unknown): boolean { + return typeof result === 'object' && result !== null && 'delivered' in result + ? result.delivered === true + : false +} + +/** Takes `keys` out of the next send until `delivered` settles, whatever its result. A result that + * reports delivery also runs `onDelivered`, for a send whose own callback can no longer fire + * (a Retry of a failed new chat). */ +export function holdNotesForSend( + keys: readonly unknown[], + delivered: Promise, + onDelivered?: () => void +): void { + if (keys.length === 0) { + return + } + // Registered before the release, so delivered notes are gone before they could show again. + void delivered.then( + (result) => { + if (reportsDelivered(result)) { + onDelivered?.() + } + }, + () => undefined + ) + 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) +} + +function subscribe(listener: () => void): () => void { + listeners.add(listener) + return () => listeners.delete(listener) +} + +function getVersion(): number { + return version +} + +/** Changes whenever a hold starts or ends, for memos that filter by `isNoteInFlight`. */ +export function useNotesInFlightVersion(): number { + return useSyncExternalStore(subscribe, getVersion, getVersion) +} + +/** 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/lib/structured-agent-session-launch-after-failure.test.ts b/src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts new file mode 100644 index 00000000000..d7d3686eb2d --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-after-failure.test.ts @@ -0,0 +1,257 @@ +// @vitest-environment happy-dom + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-session-contracts' +import type { StructuredAgentSessionLaunchIntent } from '@/lib/launch-structured-agent-session' + +const mocks = vi.hoisted(() => ({ + abandonIntent: vi.fn(), + callStructuredAgentSession: vi.fn(), + createIntent: vi.fn(), + retryIntent: vi.fn(), + restoreIntent: vi.fn(), + launch: vi.fn(), + seedDraft: vi.fn(), + clearDraft: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), message: vi.fn() } })) + +vi.mock('@/lib/launch-structured-agent-session', () => { + class StructuredAgentSessionCreateRefusalError extends Error {} + return { + createStructuredAgentSessionLaunchIntent: mocks.createIntent, + retryStructuredAgentSessionLaunchIntent: mocks.retryIntent, + restoreStructuredAgentSessionLaunchIntent: mocks.restoreIntent, + abandonStructuredAgentSessionLaunchIntent: mocks.abandonIntent, + launchStructuredAgentSession: mocks.launch, + StructuredAgentSessionCreateRefusalError + } +}) + +vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({ + refreshLocalStructuredSessionTabs: vi.fn() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.callStructuredAgentSession +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ + unifiedTabsByWorktree: {}, + seedNativeChatLaunchDraft: mocks.seedDraft, + clearNativeChatLaunchDraft: mocks.clearDraft + }), + subscribe: () => () => undefined + } +})) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/lib/agent-catalog', () => ({ + getAgentLabel: () => 'Codex', + getAgentCatalog: () => [{ id: 'codex', label: 'Codex' }] +})) + +import { StructuredAgentSessionCreateRefusalError } from '@/lib/launch-structured-agent-session' +import { refreshLocalStructuredSessionTabs } from '@/runtime/local-structured-session-tabs-sync' +import { + cancelStructuredAgentLaunch, + getStructuredAgentLaunchStatus, + getStructuredAgentSessionLaunchLifecycle, + retryStructuredAgentSessionLaunch, + startStructuredAgentLaunch +} from './structured-agent-session-launch' +import { readOutbox } from '@/components/native-chat/structured-agent-session-outbox-storage' +import { resetStructuredAgentLaunchPersistenceForTests } from './structured-agent-session-launch-persistence' +import { resetStructuredAgentLaunchRegistryForTests } from './structured-agent-session-launch-registry' + +type Receipt = { sessionId: string; fence: number } + +const WORKTREE_ID = 'wt-after-failure' + +function launchIntent(sessionId: string): StructuredAgentSessionLaunchIntent { + return { + worktreeId: WORKTREE_ID, + sessionId, + executionHostId: 'local', + target: { kind: 'local' }, + agent: 'codex', + params: { + envelope: { + sessionId, + clientOperationId: `operation-${sessionId}`, + expectedRuntimeFence: null, + payloadFingerprint: `fingerprint-${sessionId}` + }, + worktree: `id:${WORKTREE_ID}`, + agent: 'codex' + } + } +} + +function publishedSnapshot(...sessionIds: string[]): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: sessionIds.map((sessionId) => ({ + type: 'agent-session' as const, + id: `tab-${sessionId}`, + title: 'Codex', + sessionId, + agent: 'codex' as const, + isActive: false + })) + } +} + +async function flushLaunchSettlement(): Promise { + for (let i = 0; i < 20; i += 1) { + await Promise.resolve() + } +} + +const failed = launchIntent('session-failed') +const fresh = launchIntent('session-new') + +async function refuseFirstLaunch(): Promise { + mocks.createIntent.mockReturnValueOnce(failed).mockReturnValueOnce(fresh) + mocks.launch.mockRejectedValueOnce(new StructuredAgentSessionCreateRefusalError('unsupported')) + startStructuredAgentLaunch(WORKTREE_ID, 'codex', { prompt: 'first task' }) + await flushLaunchSettlement() + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, failed.sessionId)).toBe('failed') +} + +describe('a new launch after a failed one', () => { + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + mocks.retryIntent.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => ({ + ...intent, + params: { + ...intent.params, + envelope: { ...intent.params.envelope, clientOperationId: 'retried-operation' } + } + })) + mocks.restoreIntent.mockImplementation((args: { sessionId: string }) => + launchIntent(args.sessionId) + ) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ + publishedSnapshot(failed.sessionId, fresh.sessionId) + ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + page: { fence: 1 }, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('opens a new chat that sends its own prompt instead of restarting the failed one', async () => { + await refuseFirstLaunch() + // Why: the + menu and new-tab search disable an agent only while its launch reads pending. + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('idle') + + mocks.launch.mockResolvedValueOnce({ sessionId: fresh.sessionId, fence: 1 }) + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'review notes', + promptDelivery: 'submit-after-ready' + }) + + expect(notes.sessionId).toBe(fresh.sessionId) + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('pending') + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(mocks.retryIntent).not.toHaveBeenCalled() + expect(mocks.callStructuredAgentSession).toHaveBeenCalledWith( + { kind: 'local' }, + 'agentSession.send', + expect.objectContaining({ + envelope: expect.objectContaining({ sessionId: fresh.sessionId }), + body: expect.objectContaining({ blocks: [{ type: 'text', text: 'review notes' }] }) + }) + ) + // The failed chat keeps its own preserved prompt for its own Retry. + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, failed.sessionId)).toBe('failed') + expect(readOutbox(failed.sessionId)).toEqual([ + expect.objectContaining({ + body: expect.objectContaining({ blocks: [{ type: 'text', text: 'first task' }] }) + }) + ]) + }) + + it('retries the failed chat beside an in-flight new launch without taking it over', async () => { + await refuseFirstLaunch() + let resolveFresh!: (receipt: Receipt) => void + let rejectRetry!: (error: unknown) => void + mocks.launch + .mockImplementationOnce(() => new Promise((resolve) => (resolveFresh = resolve))) + .mockImplementationOnce( + () => new Promise((_resolve, reject) => (rejectRetry = reject)) + ) + const next = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + + expect(retryStructuredAgentSessionLaunch(WORKTREE_ID, failed.sessionId)).toBe(true) + expect(mocks.launch.mock.calls[2]?.[0]).toMatchObject({ sessionId: failed.sessionId }) + // A third start joins the new launch, not the retried chat. + expect(startStructuredAgentLaunch(WORKTREE_ID, 'codex').sessionId).toBe(fresh.sessionId) + expect(mocks.createIntent).toHaveBeenCalledTimes(2) + + // Closing the retried chat leaves the new launch registered and starting. + rejectRetry(new StructuredAgentSessionCreateRefusalError('unsupported')) + await flushLaunchSettlement() + expect(cancelStructuredAgentLaunch(WORKTREE_ID, failed.sessionId)).toBe(true) + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, fresh.sessionId)).toBe('pending') + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('pending') + + resolveFresh({ sessionId: fresh.sessionId, fence: 1 }) + await expect(next.launchResult).resolves.toEqual({ sessionId: fresh.sessionId, fence: 1 }) + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('idle') + }) + + it('restores a failed chat after reload without unregistering a newer launch', async () => { + await refuseFirstLaunch() + // Reload: the registry is memory; the failed record is what survives. + resetStructuredAgentLaunchRegistryForTests() + let resolveFresh!: (receipt: Receipt) => void + mocks.launch + .mockImplementationOnce(() => new Promise((resolve) => (resolveFresh = resolve))) + .mockResolvedValueOnce({ sessionId: failed.sessionId, fence: 1 }) + const next = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + + expect(retryStructuredAgentSessionLaunch(WORKTREE_ID, failed.sessionId)).toBe(true) + await flushLaunchSettlement() + + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, failed.sessionId)).toBeNull() + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, fresh.sessionId)).toBe('pending') + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('pending') + resolveFresh({ sessionId: fresh.sessionId, fence: 1 }) + await expect(next.launchResult).resolves.toEqual({ sessionId: fresh.sessionId, fence: 1 }) + }) + + it('reads a failed resume as not starting, so resuming again opens a new chat', async () => { + const resumeFrom = { providerSessionId: 'provider-1' } + mocks.createIntent.mockReturnValueOnce(failed).mockReturnValueOnce(fresh) + mocks.launch + .mockRejectedValueOnce(new StructuredAgentSessionCreateRefusalError('unsupported')) + .mockResolvedValueOnce({ sessionId: fresh.sessionId, fence: 1 }) + startStructuredAgentLaunch(WORKTREE_ID, 'codex', { resumeFrom }) + await flushLaunchSettlement() + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('idle') + + expect(startStructuredAgentLaunch(WORKTREE_ID, 'codex', { resumeFrom }).sessionId).toBe( + fresh.sessionId + ) + }) +}) diff --git a/src/renderer/src/lib/structured-agent-session-launch-callers.ts b/src/renderer/src/lib/structured-agent-session-launch-callers.ts index b3ce31f1a6b..0015fbd8f75 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-callers.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-callers.ts @@ -4,6 +4,7 @@ import type { StructuredAgentSessionOutboxEntry } from '../../../shared/structur import type { StructuredAgentSessionResumeSource } from '../../../shared/structured-agent-session-create' import type { RuntimeClientTarget } from '@/runtime/runtime-client-target' import type { ExecutionHostId } from '../../../shared/execution-host' +import type { StructuredLaunchAttempt } from './structured-agent-session-launch-request' export type StructuredAgentLaunchOptions = { prompt?: string @@ -24,14 +25,18 @@ export type StructuredLaunchCaller = { export type StructuredLaunchCallerGroup = { outcome: 'pending' | 'published' | 'failed' | 'unknown' | 'cancelled' + attempt: StructuredLaunchAttempt entries: Set promptDeliveryResults: Set> onSettled: () => void } -export function createStructuredLaunchCallerGroup(): StructuredLaunchCallerGroup { +export function createStructuredLaunchCallerGroup( + attempt: StructuredLaunchAttempt +): StructuredLaunchCallerGroup { return { outcome: 'pending', + attempt, entries: new Set(), promptDeliveryResults: new Set(), onSettled: () => {} diff --git a/src/renderer/src/lib/structured-agent-session-launch-cancellation.test.ts b/src/renderer/src/lib/structured-agent-session-launch-cancellation.test.ts index f63b985586c..4f55e062309 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-cancellation.test.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-cancellation.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' import { suppressCancelledStructuredSessionTabs } from '@/runtime/structured-agent-session-tab-retirement' import type { StructuredLaunchState } from './structured-agent-session-launch-registry' +import { BLANK_STRUCTURED_LAUNCH_REQUEST } from './structured-agent-session-launch-request' import { hasStructuredAgentSessionLaunchCancellationTombstone, markStructuredAgentSessionLaunchCancelled, @@ -82,6 +83,7 @@ describe('structured launch cancellation retirement', () => { promptDelivery: 'auto-submit', callers: { outcome: 'pending', + attempt: { kind: 'first', request: BLANK_STRUCTURED_LAUNCH_REQUEST, stagedEntry: null }, entries: new Set(), promptDeliveryResults: new Set(), onSettled: () => undefined @@ -161,6 +163,7 @@ describe('structured launch cancellation retirement', () => { promptDelivery: 'auto-submit', callers: { outcome: 'pending', + attempt: { kind: 'first', request: BLANK_STRUCTURED_LAUNCH_REQUEST, stagedEntry: null }, entries: new Set(), promptDeliveryResults: new Set(), onSettled: () => undefined diff --git a/src/renderer/src/lib/structured-agent-session-launch-close-race.test.ts b/src/renderer/src/lib/structured-agent-session-launch-close-race.test.ts index 9c27384d5a9..f63575080de 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-close-race.test.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-close-race.test.ts @@ -177,7 +177,7 @@ describe('a close that races a structured launch', () => { expect(toast.error).not.toHaveBeenCalled() }) - it('discards every coalesced prompt when a close cancels the launch', async () => { + it("discards a repeated request's one staged prompt when a close cancels the launch", async () => { const worktreeId = 'wt-close-coalesced-prompts' const intent = launchIntent(worktreeId) let resolveRefresh!: (snapshots: RuntimeMobileSessionTabsResult[]) => void @@ -188,9 +188,9 @@ describe('a close that races a structured launch', () => { ) startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'first prompt' }) - startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) + startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'first prompt' }) await vi.waitFor(() => expect(refreshLocalStructuredSessionTabs).toHaveBeenCalledOnce()) - expect(readOutbox(intent.sessionId)).toHaveLength(2) + expect(readOutbox(intent.sessionId)).toHaveLength(1) expect(cancelStructuredAgentLaunch(worktreeId, intent.sessionId)).toBe(true) expect(readOutbox(intent.sessionId)).toEqual([]) diff --git a/src/renderer/src/lib/structured-agent-session-launch-different-request.test.ts b/src/renderer/src/lib/structured-agent-session-launch-different-request.test.ts new file mode 100644 index 00000000000..7dee1159dd3 --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-different-request.test.ts @@ -0,0 +1,450 @@ +// @vitest-environment happy-dom + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-session-contracts' +import type { StructuredAgentSessionLaunchIntent } from '@/lib/launch-structured-agent-session' + +const mocks = vi.hoisted(() => ({ + abandonIntent: vi.fn(), + callStructuredAgentSession: vi.fn(), + createIntent: vi.fn(), + retryIntent: vi.fn(), + restoreIntent: vi.fn(), + launch: vi.fn(), + seedDraft: vi.fn(), + clearDraft: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), message: vi.fn() } })) + +vi.mock('@/lib/launch-structured-agent-session', () => { + class StructuredAgentSessionCreateRefusalError extends Error {} + return { + createStructuredAgentSessionLaunchIntent: mocks.createIntent, + retryStructuredAgentSessionLaunchIntent: mocks.retryIntent, + restoreStructuredAgentSessionLaunchIntent: mocks.restoreIntent, + abandonStructuredAgentSessionLaunchIntent: mocks.abandonIntent, + launchStructuredAgentSession: mocks.launch, + StructuredAgentSessionCreateRefusalError + } +}) + +vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({ + refreshLocalStructuredSessionTabs: vi.fn() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.callStructuredAgentSession +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ + unifiedTabsByWorktree: {}, + seedNativeChatLaunchDraft: mocks.seedDraft, + clearNativeChatLaunchDraft: mocks.clearDraft + }), + subscribe: () => () => undefined + } +})) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/lib/agent-catalog', () => ({ + getAgentLabel: () => 'Codex', + getAgentCatalog: () => [{ id: 'codex', label: 'Codex' }] +})) + +import { refreshLocalStructuredSessionTabs } from '@/runtime/local-structured-session-tabs-sync' +import { + appendStructuredAgentSessionOutboxMessage, + readOutbox +} from '@/components/native-chat/structured-agent-session-outbox-storage' +import { + clearNativeChatDraftCacheForTests, + readNativeChatDraftCache, + writeNativeChatDraftCache +} from '@/components/native-chat/native-chat-draft-cache' +import { + structuredAgentSessionPaneKey, + structuredAgentSessionTabId +} from '../../../shared/structured-agent-session-projection' +import { StructuredAgentSessionCreateRefusalError } from '@/lib/launch-structured-agent-session' +import { + getStructuredAgentLaunchStatus, + getStructuredAgentSessionLaunchLifecycle, + startStructuredAgentLaunch +} from './structured-agent-session-launch' +import { resetStructuredAgentLaunchPersistenceForTests } from './structured-agent-session-launch-persistence' +import { resetStructuredAgentLaunchRegistryForTests } from './structured-agent-session-launch-registry' +import { structuredLaunchRequest } from './structured-agent-session-launch-request' + +const WORKTREE_ID = 'wt-different-request' + +function launchIntent(sessionId: string): StructuredAgentSessionLaunchIntent { + return { + worktreeId: WORKTREE_ID, + sessionId, + executionHostId: 'local', + target: { kind: 'local' }, + agent: 'codex', + params: { + envelope: { + sessionId, + clientOperationId: `operation-${sessionId}`, + expectedRuntimeFence: null, + payloadFingerprint: `fingerprint-${sessionId}` + }, + worktree: `id:${WORKTREE_ID}`, + agent: 'codex' + } + } +} + +function publishedSnapshot(...sessionIds: string[]): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: sessionIds.map((sessionId) => ({ + type: 'agent-session', + id: `tab-${sessionId}`, + title: 'Codex', + sessionId, + agent: 'codex', + isActive: false + })) + } +} + +const first = launchIntent('session-first') +const second = launchIntent('session-second') + +/** Every `agentSession.send` as [session, text]. */ +function sends(): [string, string][] { + return mocks.callStructuredAgentSession.mock.calls + .filter((call) => call[1] === 'agentSession.send') + .map((call) => [call[2].envelope.sessionId, call[2].body.blocks[0].text]) +} + +describe('a different new request while the first chat is still starting', () => { + let resolveFirstLaunch!: (receipt: { sessionId: string; fence: number }) => void + + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + mocks.createIntent.mockReturnValueOnce(first).mockReturnValueOnce(second) + mocks.launch.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => + intent.sessionId === first.sessionId + ? new Promise((resolve) => (resolveFirstLaunch = resolve)) + : Promise.resolve({ sessionId: intent.sessionId, fence: 1 }) + ) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ + publishedSnapshot(first.sessionId, second.sessionId) + ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('opens a new chat with its own text while the first create is in flight', async () => { + const checkA = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check A', + promptDelivery: 'submit-after-ready' + }) + const checkB = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check B', + promptDelivery: 'submit-after-ready' + }) + + expect(checkB.sessionId).toBe(second.sessionId) + await expect(checkB.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + resolveFirstLaunch({ sessionId: first.sessionId, fence: 1 }) + await expect(checkA.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([ + [second.sessionId, 'Fix check B'], + [first.sessionId, 'Fix check A'] + ]) + }) + + it('opens a new chat with its own text while the first chat is still sending its text', async () => { + let resolveFirstSend!: (result: unknown) => void + mocks.callStructuredAgentSession.mockImplementationOnce( + () => new Promise((resolve) => (resolveFirstSend = resolve)) + ) + const checkA = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check A', + promptDelivery: 'submit-after-ready' + }) + resolveFirstLaunch({ sessionId: first.sessionId, fence: 1 }) + await vi.waitFor(() => expect(sends()).toEqual([[first.sessionId, 'Fix check A']])) + + const checkB = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check B', + promptDelivery: 'submit-after-ready' + }) + + expect(checkB.sessionId).toBe(second.sessionId) + await expect(checkB.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + resolveFirstSend({ ok: true, value: { submission: { dispatchState: 'accepted' } } }) + await expect(checkA.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([ + [first.sessionId, 'Fix check A'], + [second.sessionId, 'Fix check B'] + ]) + }) +}) + +describe('the same request repeated while the first chat is still starting', () => { + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + mocks.createIntent.mockReturnValueOnce(first).mockReturnValueOnce(second) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ + publishedSnapshot(first.sessionId) + ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('makes one chat from a double click with no text', () => { + mocks.launch.mockImplementation(() => new Promise(() => undefined)) + + const pick = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + const again = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + promptDelivery: 'submit-after-ready' + }) + + expect(again.sessionId).toBe(pick.sessionId) + expect(mocks.createIntent).toHaveBeenCalledOnce() + }) + + it('makes one chat and sends its text once while the create is in flight', async () => { + let resolveLaunch!: (receipt: { sessionId: string; fence: number }) => void + mocks.launch.mockImplementation(() => new Promise((resolve) => (resolveLaunch = resolve))) + const onFirstDelivered = vi.fn() + const onRepeatDelivered = vi.fn() + + const click = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check A', + promptDelivery: 'submit-after-ready', + onPromptDelivered: onFirstDelivered + }) + const repeat = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check A', + promptDelivery: 'submit-after-ready', + onPromptDelivered: onRepeatDelivered + }) + resolveLaunch({ sessionId: first.sessionId, fence: 1 }) + + expect(repeat.sessionId).toBe(click.sessionId) + for (const caller of [click, repeat]) { + await expect(caller.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + } + expect(sends()).toEqual([[first.sessionId, 'Fix check A']]) + expect(onFirstDelivered).toHaveBeenCalledOnce() + expect(onRepeatDelivered).toHaveBeenCalledOnce() + }) + + it('seeds a repeated draft once', () => { + mocks.launch.mockImplementation(() => new Promise(() => undefined)) + + const click = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'PR context', + promptDelivery: 'draft' + }) + const repeat = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'PR context', + promptDelivery: 'draft' + }) + + expect(repeat.sessionId).toBe(click.sessionId) + expect(mocks.seedDraft).toHaveBeenCalledOnce() + }) +}) + +describe('an empty chat still starting', () => { + let resolveFirstLaunch!: (receipt: { sessionId: string; fence: number }) => void + const third = launchIntent('session-third') + const notesRequest = { prompt: 'review notes', promptDelivery: 'submit-after-ready' } as const + + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + mocks.createIntent + .mockReturnValueOnce(first) + .mockReturnValueOnce(second) + .mockReturnValueOnce(third) + clearNativeChatDraftCacheForTests() + mocks.launch.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => + intent.sessionId === first.sessionId + ? new Promise((resolve) => (resolveFirstLaunch = resolve)) + : Promise.resolve({ sessionId: intent.sessionId, fence: 1 }) + ) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ + publishedSnapshot(first.sessionId, second.sessionId, third.sessionId) + ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('takes notes sent to a new agent instead of opening a second chat', async () => { + const blank = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + // The notes menu stays enabled: its pick fills the empty chat rather than repeating a start. + expect( + getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex', structuredLaunchRequest(notesRequest)) + ).toBe('idle') + + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + resolveFirstLaunch({ sessionId: first.sessionId, fence: 1 }) + + expect(notes.sessionId).toBe(blank.sessionId) + expect(mocks.createIntent).toHaveBeenCalledOnce() + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([[first.sessionId, 'review notes']]) + }) + + it('opens a new chat for any other request once its notes claimed it', async () => { + startStructuredAgentLaunch(WORKTREE_ID, 'codex') + startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + + const fix = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'Fix check B', + promptDelivery: 'submit-after-ready' + }) + const pick = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + + expect(fix.sessionId).toBe(second.sessionId) + expect(pick.sessionId).toBe(third.sessionId) + await expect(fix.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([[second.sessionId, 'Fix check B']]) + }) + + it('delivers the claiming text the way its own request asked', async () => { + startStructuredAgentLaunch(WORKTREE_ID, 'codex', { promptDelivery: 'draft' }) + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + resolveFirstLaunch({ sessionId: first.sessionId, fence: 1 }) + + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([[first.sessionId, 'review notes']]) + expect(mocks.seedDraft).not.toHaveBeenCalled() + }) + + it('sends the same notes once when they are sent again', async () => { + const blank = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + const again = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + resolveFirstLaunch({ sessionId: first.sessionId, fence: 1 }) + + expect(again.sessionId).toBe(blank.sessionId) + expect(mocks.createIntent).toHaveBeenCalledOnce() + for (const caller of [notes, again]) { + await expect(caller.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + } + expect(sends()).toEqual([[first.sessionId, 'review notes']]) + }) + + it('leaves a chat its user already sent into to them, and opens a new chat for the notes', async () => { + const blank = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + appendStructuredAgentSessionOutboxMessage(blank.sessionId, 'my own question') + + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + + expect(notes.sessionId).toBe(second.sessionId) + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(sends()).toEqual([[second.sessionId, 'review notes']]) + expect(readOutbox(blank.sessionId).map((entry) => entry.body.blocks)).toEqual([ + [{ type: 'text', text: 'my own question' }] + ]) + }) + + it('leaves a chat its user is typing into to them, and opens a new chat for the notes', async () => { + const blank = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + const paneKey = structuredAgentSessionPaneKey( + structuredAgentSessionTabId(blank.sessionId), + blank.sessionId + ) + writeNativeChatDraftCache(paneKey, 'half a question') + + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + + expect(notes.sessionId).toBe(second.sessionId) + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(readOutbox(blank.sessionId)).toEqual([]) + expect(readNativeChatDraftCache(paneKey)).toBe('half a question') + }) + + it('opens a new chat that shows the failure when the claiming text cannot be saved', async () => { + const blank = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + const storageFailure = vi.spyOn(localStorage, 'setItem').mockImplementation(() => { + throw new Error('storage unavailable') + }) + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest) + storageFailure.mockRestore() + + // The new chat fails with its Retry line, the failure the caller is told was shown. + expect(notes.sessionId).toBe(second.sessionId) + await expect(notes.launchResult).rejects.toBeInstanceOf( + StructuredAgentSessionCreateRefusalError + ) + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: false, + failureNotified: true + }) + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, second.sessionId)).toBe('failed') + // No claim was recorded: the blank chat is still blank and still claimable. + expect(startStructuredAgentLaunch(WORKTREE_ID, 'codex').sessionId).toBe(blank.sessionId) + expect(startStructuredAgentLaunch(WORKTREE_ID, 'codex', notesRequest).sessionId).toBe( + blank.sessionId + ) + }) +}) diff --git a/src/renderer/src/lib/structured-agent-session-launch-empty-chat.ts b/src/renderer/src/lib/structured-agent-session-launch-empty-chat.ts new file mode 100644 index 00000000000..e8c5becd685 --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-empty-chat.ts @@ -0,0 +1,16 @@ +import { + structuredAgentSessionPaneKey, + structuredAgentSessionTabId +} from '../../../shared/structured-agent-session-projection' +import { getStructuredAgentSessionOutbox } from '@/components/native-chat/structured-agent-session-outbox-storage' +import { readNativeChatDraftCache } from '@/components/native-chat/native-chat-draft-cache' + +/** A starting chat is empty until its user sends into it or types in its composer; after that it + * is theirs, and another request's text never goes into it. */ +export function isStructuredLaunchChatEmpty(sessionId: string): boolean { + const paneKey = structuredAgentSessionPaneKey(structuredAgentSessionTabId(sessionId), sessionId) + return ( + getStructuredAgentSessionOutbox(sessionId).length === 0 && + readNativeChatDraftCache(paneKey).trim() === '' + ) +} diff --git a/src/renderer/src/lib/structured-agent-session-launch-holders.ts b/src/renderer/src/lib/structured-agent-session-launch-holders.ts new file mode 100644 index 00000000000..68b0c1728f6 --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-holders.ts @@ -0,0 +1,72 @@ +import { + launchStateLifecycle, + structuredLaunchStates, + type StructuredLaunchState +} from './structured-agent-session-launch-registry' +import { + joinsFirstLaunchAttempt, + type StructuredLaunchAttempt, + type StructuredLaunchRequest +} from './structured-agent-session-launch-request' +import { isStructuredLaunchChatEmpty } from './structured-agent-session-launch-empty-chat' + +// Why: coalescing stops a repeat of one request (a double click) racing into two chats. A different +// request, a failed or unconfirmed launch, or a Retry/re-check of one is not that race: a new start +// opens a new chat carrying its own text. A resume keeps holding: the host refuses a second adoption. +function holdsLaunchIdentity( + state: StructuredLaunchState, + request?: StructuredLaunchRequest +): boolean { + const lifecycle = launchStateLifecycle(state) + if (lifecycle === 'failed' || lifecycle === 'cancelled') { + return false + } + if (state.intent.params.resumeFrom) { + return true + } + return ( + lifecycle !== 'visibility-unknown' && joinsFirstLaunchAttempt(state.callers.attempt, request) + ) +} + +/** Launches a start of `request` would repeat; without `request`, every new start's own create. */ +export function structuredLaunchesHoldingIdentity( + matches: (identity: string) => boolean, + request?: StructuredLaunchRequest +): StructuredLaunchState[] { + return [...structuredLaunchStates()].filter( + (state) => matches(state.identity) && holdsLaunchIdentity(state, request) + ) +} + +/** An empty chat (a + pick, the empty-workspace default) still starting: the first request with text + * claims it once, and from then on it is that request's chat. A resume is never empty, nor a chat + * its user has already sent or typed into. */ +export function claimableStructuredLaunchAttempt( + state: StructuredLaunchState, + request: StructuredLaunchRequest +): Extract | undefined { + const { attempt } = state.callers + return !state.intent.params.resumeFrom && + attempt.kind === 'first' && + attempt.request.text === '' && + request.text !== '' && + isStructuredLaunchChatEmpty(state.intent.sessionId) + ? attempt + : undefined +} + +/** The launch a new start of `request` joins: one it repeats, else an empty chat it claims. The + * newest wins if a retried resume holds the identity too. */ +export function getJoinableStructuredLaunchState( + identity: string, + request: StructuredLaunchRequest +): StructuredLaunchState | undefined { + const matches = (candidate: string): boolean => candidate === identity + return ( + structuredLaunchesHoldingIdentity(matches, request).at(-1) ?? + structuredLaunchesHoldingIdentity(matches).findLast((state) => + claimableStructuredLaunchAttempt(state, request) + ) + ) +} diff --git a/src/renderer/src/lib/structured-agent-session-launch-join-delivery.test.ts b/src/renderer/src/lib/structured-agent-session-launch-join-delivery.test.ts index c597c552262..b8efed9d8fa 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-join-delivery.test.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-join-delivery.test.ts @@ -137,7 +137,9 @@ describe('coalesced launch delivery mode', () => { established: StructuredAgentLaunchOptions joining: StructuredAgentLaunchOptions }) { - const intent = launchIntent(args.worktreeId, args.sessionId) + const base = launchIntent(args.worktreeId, args.sessionId) + const { resumeFrom } = args.established + const intent = resumeFrom ? { ...base, params: { ...base.params, resumeFrom } } : base let resolveLaunch!: (receipt: { sessionId: string; fence: number }) => void mocks.createIntent.mockReturnValueOnce(intent) mocks.launch.mockImplementation( @@ -155,13 +157,14 @@ describe('coalesced launch delivery mode', () => { return { intent, joiner } } - it('keeps a joiner as a draft when the launch it joined established no mode', async () => { - // The onboarding folder launch establishes an identity carrying neither prompt nor mode. + // Only a resume joins a launch started by a different request; a new start opens its own chat. + it('keeps a resume joiner as a draft when the launch it joined established no mode', async () => { + const resumeFrom = { providerSessionId: 'provider-unset-delivery' } const { intent, joiner } = await coalesce({ worktreeId: 'wt-unset-delivery-mode', sessionId: 'unset-delivery-session', - established: {}, - joining: { prompt: 'PR context', promptDelivery: 'draft' } + established: { resumeFrom }, + joining: { resumeFrom, prompt: 'PR context', promptDelivery: 'draft' } }) // Why: an unset established mode must not read as submit; the joiner never consented to send. diff --git a/src/renderer/src/lib/structured-agent-session-launch-registry.ts b/src/renderer/src/lib/structured-agent-session-launch-registry.ts index 622fc58b379..9d21852b2fb 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-registry.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-registry.ts @@ -43,19 +43,17 @@ export type StructuredAgentSessionLaunchLifecycle = | 'published' | 'cancelled' -const pendingStructuredLaunchesByIdentity = new Map() const structuredLaunchesBySessionId = new Map() const structuredLaunchListeners = new Set<() => void>() export function resetStructuredAgentLaunchRegistryForTests(): void { - pendingStructuredLaunchesByIdentity.clear() structuredLaunchesBySessionId.clear() structuredLaunchListeners.clear() resetStructuredAgentLaunchCancellationForTests() } export function notifyStructuredLaunchListeners(): void { - for (const state of pendingStructuredLaunchesByIdentity.values()) { + for (const state of structuredLaunchesBySessionId.values()) { persistStructuredLaunchState(state) } for (const listener of structuredLaunchListeners) { @@ -80,10 +78,6 @@ export function structuredLaunchIdentity( : `${agent}:${worktreeId}` } -export function getStructuredLaunchState(identity: string): StructuredLaunchState | undefined { - return pendingStructuredLaunchesByIdentity.get(identity) -} - export function getStructuredLaunchStateBySessionId( sessionId: string ): StructuredLaunchState | undefined { @@ -91,19 +85,15 @@ export function getStructuredLaunchStateBySessionId( } export function setStructuredLaunchState(state: StructuredLaunchState): void { - pendingStructuredLaunchesByIdentity.set(state.identity, state) structuredLaunchesBySessionId.set(state.intent.sessionId, state) persistStructuredLaunchState(state) } export function deleteStructuredLaunchStateIfCurrent(state: StructuredLaunchState): boolean { - if (pendingStructuredLaunchesByIdentity.get(state.identity) !== state) { + if (structuredLaunchesBySessionId.get(state.intent.sessionId) !== state) { return false } - pendingStructuredLaunchesByIdentity.delete(state.identity) - if (structuredLaunchesBySessionId.get(state.intent.sessionId) === state) { - structuredLaunchesBySessionId.delete(state.intent.sessionId) - } + structuredLaunchesBySessionId.delete(state.intent.sessionId) deleteStructuredAgentLaunchRecord(state.intent.sessionId) return true } @@ -124,10 +114,12 @@ export function getPersistedStructuredAgentLaunchRecord( } export function structuredLaunchStates(): IterableIterator { - return pendingStructuredLaunchesByIdentity.values() + return structuredLaunchesBySessionId.values() } -function launchStateLifecycle(state: StructuredLaunchState): StructuredAgentSessionLaunchLifecycle { +export function launchStateLifecycle( + state: StructuredLaunchState +): StructuredAgentSessionLaunchLifecycle { if (state.cancelled || state.callers.outcome === 'cancelled') { return 'cancelled' } @@ -314,20 +306,3 @@ export function retireAbsentStructuredAgentSessionLaunchCancellationTombstones( } return changed } - -export function getStructuredAgentLaunchStatus( - worktreeId: string, - agent: AgentSessionHandleProvider -): StructuredAgentLaunchStatus { - // Any launch for this pair, including adopted conversations, means a chat is starting here. - const states = [ - getStructuredLaunchState(structuredLaunchIdentity(worktreeId, agent)), - ...[...pendingStructuredLaunchesByIdentity.entries()] - .filter(([identity]) => identity.startsWith(`${agent}:${worktreeId}:resume:`)) - .map(([, state]) => state) - ].filter((state): state is StructuredLaunchState => Boolean(state)) - if (states.length === 0) { - return 'idle' - } - return states.some((state) => state.visibilityUnknown) ? 'unknown' : 'pending' -} diff --git a/src/renderer/src/lib/structured-agent-session-launch-reload.ts b/src/renderer/src/lib/structured-agent-session-launch-reload.ts index 3ac9394a89c..35379324faa 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-reload.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-reload.ts @@ -42,7 +42,8 @@ export function restorePersistedStructuredLaunchState( } throw error } - const callers: StructuredLaunchCallerGroup = createStructuredLaunchCallerGroup() + // Only a Retry or re-check restarts a restored launch. + const callers: StructuredLaunchCallerGroup = createStructuredLaunchCallerGroup({ kind: 'retry' }) const state: StructuredLaunchState = { identity: structuredLaunchIdentity(worktreeId, record.agent, record.resumeFrom), intent, diff --git a/src/renderer/src/lib/structured-agent-session-launch-request.ts b/src/renderer/src/lib/structured-agent-session-launch-request.ts new file mode 100644 index 00000000000..adefefd11cc --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-request.ts @@ -0,0 +1,51 @@ +import type { StructuredAgentSessionOutboxEntry } from '../../../shared/structured-agent-session-outbox' + +/** What a new start asks its chat to receive. Callers carry no request id, so this is what tells a + * repeat of one request (a double click, a retried call) from a different request. */ +export type StructuredLaunchRequest = { text: string; draft: boolean } + +/** A new start's own create keeps its request and the text it staged; a Retry or re-check of an + * existing chat is no request of its own. */ +export type StructuredLaunchAttempt = + | { + kind: 'first' + request: StructuredLaunchRequest + stagedEntry: StructuredAgentSessionOutboxEntry | null + } + | { kind: 'retry' } + +/** A pick that carries no text, as the + menu and new-tab search make. */ +export const BLANK_STRUCTURED_LAUNCH_REQUEST: StructuredLaunchRequest = { text: '', draft: false } + +export function structuredLaunchRequest(options: { + prompt?: string + promptDelivery?: string +}): StructuredLaunchRequest { + const text = options.prompt?.trim() ?? '' + // Without text the delivery mode carries nothing: two blank starts are one request. + return { text, draft: text !== '' && options.promptDelivery === 'draft' } +} + +/** The first attempt `request` repeats, whose text is already staged or seeded. */ +export function repeatedStructuredLaunchAttempt( + attempt: StructuredLaunchAttempt, + request: StructuredLaunchRequest +): Extract | undefined { + return attempt.kind === 'first' && + attempt.request.text === request.text && + attempt.request.draft === request.draft + ? attempt + : undefined +} + +/** Only a new start's own create is joined, and only by a repeat of its request. Without `request`, + * any new start's own create counts. */ +export function joinsFirstLaunchAttempt( + attempt: StructuredLaunchAttempt, + request?: StructuredLaunchRequest +): boolean { + return ( + attempt.kind === 'first' && + (!request || repeatedStructuredLaunchAttempt(attempt, request) !== undefined) + ) +} diff --git a/src/renderer/src/lib/structured-agent-session-launch-status.ts b/src/renderer/src/lib/structured-agent-session-launch-status.ts index 6ae37ff6553..d82676e780a 100644 --- a/src/renderer/src/lib/structured-agent-session-launch-status.ts +++ b/src/renderer/src/lib/structured-agent-session-launch-status.ts @@ -1,17 +1,40 @@ import { useSyncExternalStore } from 'react' import type { AgentSessionHandleProvider } from '../../../shared/agent-session-provider-handle' +import { structuredLaunchesHoldingIdentity } from './structured-agent-session-launch-holders' import { - getStructuredAgentLaunchStatus, - subscribeStructuredAgentLaunchStatus + structuredLaunchIdentity, + subscribeStructuredAgentLaunchStatus, + type StructuredAgentLaunchStatus } from './structured-agent-session-launch-registry' +import type { StructuredLaunchRequest } from './structured-agent-session-launch-request' + +/** With `request`, only launches a start of it would repeat: any other request is new work. */ +export function getStructuredAgentLaunchStatus( + worktreeId: string, + agent: AgentSessionHandleProvider, + request?: StructuredLaunchRequest +): StructuredAgentLaunchStatus { + // Any launch holding an identity for this pair, adopted conversations included, is starting here. + // A failed chat is not, nor an unconfirmed or retried blank one: a new launch opens its own chat. + const identity = structuredLaunchIdentity(worktreeId, agent) + const states = structuredLaunchesHoldingIdentity( + (candidate) => candidate === identity || candidate.startsWith(`${identity}:resume:`), + request + ) + if (states.length === 0) { + return 'idle' + } + return states.some((state) => state.visibilityUnknown) ? 'unknown' : 'pending' +} export function useStructuredAgentLaunchStatus( worktreeId: string, - agent: AgentSessionHandleProvider -): ReturnType { + agent: AgentSessionHandleProvider, + request?: StructuredLaunchRequest +): StructuredAgentLaunchStatus { return useSyncExternalStore( subscribeStructuredAgentLaunchStatus, - () => getStructuredAgentLaunchStatus(worktreeId, agent), + () => getStructuredAgentLaunchStatus(worktreeId, agent, request), () => 'idle' ) } diff --git a/src/renderer/src/lib/structured-agent-session-launch-unconfirmed-or-retried.test.ts b/src/renderer/src/lib/structured-agent-session-launch-unconfirmed-or-retried.test.ts new file mode 100644 index 00000000000..d1f1a6abcbd --- /dev/null +++ b/src/renderer/src/lib/structured-agent-session-launch-unconfirmed-or-retried.test.ts @@ -0,0 +1,314 @@ +// @vitest-environment happy-dom + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-session-contracts' +import type { StructuredAgentSessionLaunchIntent } from '@/lib/launch-structured-agent-session' + +const mocks = vi.hoisted(() => ({ + abandonIntent: vi.fn(), + callStructuredAgentSession: vi.fn(), + createIntent: vi.fn(), + retryIntent: vi.fn(), + restoreIntent: vi.fn(), + launch: vi.fn(), + seedDraft: vi.fn(), + clearDraft: vi.fn(), + toastError: vi.fn() +})) + +vi.mock('sonner', () => ({ toast: { error: mocks.toastError, message: vi.fn() } })) + +vi.mock('@/lib/launch-structured-agent-session', () => { + class StructuredAgentSessionCreateRefusalError extends Error {} + return { + createStructuredAgentSessionLaunchIntent: mocks.createIntent, + retryStructuredAgentSessionLaunchIntent: mocks.retryIntent, + restoreStructuredAgentSessionLaunchIntent: mocks.restoreIntent, + abandonStructuredAgentSessionLaunchIntent: mocks.abandonIntent, + launchStructuredAgentSession: mocks.launch, + StructuredAgentSessionCreateRefusalError + } +}) + +vi.mock('@/runtime/local-structured-session-tabs-sync', () => ({ + refreshLocalStructuredSessionTabs: vi.fn() +})) + +vi.mock('@/runtime/structured-agent-session-client', () => ({ + callStructuredAgentSession: mocks.callStructuredAgentSession +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => ({ + unifiedTabsByWorktree: {}, + seedNativeChatLaunchDraft: mocks.seedDraft, + clearNativeChatLaunchDraft: mocks.clearDraft + }), + subscribe: () => () => undefined + } +})) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/lib/agent-catalog', () => ({ + getAgentLabel: () => 'Codex', + getAgentCatalog: () => [{ id: 'codex', label: 'Codex' }] +})) + +vi.mock('@/lib/focus-terminal-tab-surface', () => ({ focusTerminalTabSurface: vi.fn() })) + +// The structured route hands the launch's own delivery result back unchanged. +vi.mock('@/lib/launch-agent-in-new-tab', async () => { + const launch = await import('./structured-agent-session-launch') + return { + launchAgentInNewTab: (args: { + worktreeId: string + prompt: string + promptDelivery: 'auto-submit' | 'draft' | 'submit-after-ready' + }) => { + const started = launch.startStructuredAgentLaunch(args.worktreeId, 'codex', { + prompt: args.prompt, + promptDelivery: args.promptDelivery + }) + return { + surface: { kind: 'agent-session', sessionId: started.sessionId }, + ...(started.promptDeliveryResult + ? { promptDeliveryResult: started.promptDeliveryResult } + : {}) + } + } + } +}) + +import { StructuredAgentSessionCreateRefusalError } from '@/lib/launch-structured-agent-session' +import { refreshLocalStructuredSessionTabs } from '@/runtime/local-structured-session-tabs-sync' +import { runSourceControlAgentActionStart } from '@/components/right-sidebar/runSourceControlAgentActionStart' +import { + getStructuredAgentLaunchStatus, + getStructuredAgentSessionLaunchLifecycle, + retryStructuredAgentSessionLaunch, + startStructuredAgentLaunch +} from './structured-agent-session-launch' +import { resetStructuredAgentLaunchPersistenceForTests } from './structured-agent-session-launch-persistence' +import { resetStructuredAgentLaunchRegistryForTests } from './structured-agent-session-launch-registry' + +const WORKTREE_ID = 'wt-unconfirmed-or-retried' + +function launchIntent(sessionId: string): StructuredAgentSessionLaunchIntent { + return { + worktreeId: WORKTREE_ID, + sessionId, + executionHostId: 'local', + target: { kind: 'local' }, + agent: 'codex', + params: { + envelope: { + sessionId, + clientOperationId: `operation-${sessionId}`, + expectedRuntimeFence: null, + payloadFingerprint: `fingerprint-${sessionId}` + }, + worktree: `id:${WORKTREE_ID}`, + agent: 'codex' + } + } +} + +function publishedSnapshot(sessionId: string): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [ + { + type: 'agent-session', + id: `tab-${sessionId}`, + title: 'Codex', + sessionId, + agent: 'codex', + isActive: false + } + ] + } +} + +async function flushLaunchSettlement(): Promise { + for (let i = 0; i < 20; i += 1) { + await Promise.resolve() + } +} + +const unconfirmed = launchIntent('session-unconfirmed') +const fresh = launchIntent('session-new') +const resumeFrom = { providerSessionId: 'provider-1' } + +/** The create's answer is lost and inventory never shows the chat: it stays unconfirmed. */ +async function leaveFirstLaunchUnconfirmed(options: { resume?: boolean } = {}): Promise { + mocks.createIntent + .mockReturnValueOnce( + options.resume + ? { ...unconfirmed, params: { ...unconfirmed.params, resumeFrom } } + : unconfirmed + ) + .mockReturnValueOnce(fresh) + startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'first task', + ...(options.resume ? { resumeFrom } : {}) + }) + await flushLaunchSettlement() + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, unconfirmed.sessionId)).toBe( + 'visibility-unknown' + ) +} + +function expectSentTo(sessionId: string, text: string): void { + expect(mocks.callStructuredAgentSession).toHaveBeenCalledWith( + { kind: 'local' }, + 'agentSession.send', + expect.objectContaining({ + envelope: expect.objectContaining({ sessionId }), + body: expect.objectContaining({ blocks: [{ type: 'text', text }] }) + }) + ) +} + +describe('a new start beside an unconfirmed or retried chat', () => { + beforeEach(() => { + vi.resetAllMocks() + localStorage.clear() + resetStructuredAgentLaunchPersistenceForTests() + resetStructuredAgentLaunchRegistryForTests() + mocks.retryIntent.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => intent) + mocks.restoreIntent.mockImplementation((args: { sessionId: string }) => + launchIntent(args.sessionId) + ) + mocks.launch.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => + intent.sessionId === fresh.sessionId + ? Promise.resolve({ sessionId: fresh.sessionId, fence: 1 }) + : Promise.reject(new Error('response lost')) + ) + vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ + publishedSnapshot(fresh.sessionId) + ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + page: { fence: 1 }, + value: { submission: { dispatchState: 'accepted' } } + }) + }) + + it('starts a source-control agent in a new chat and reports success only once its text is sent', async () => { + await leaveFirstLaunchUnconfirmed() + const onLaunched = vi.fn() + + const started = await runSourceControlAgentActionStart({ + selectedAgent: 'codex', + trimmedCommandInput: 'Fix the failing check', + agentArgs: '', + agentArgsApply: false, + commandTemplate: '{basePrompt}', + saveTargetValue: 'none', + actionId: 'resolveComments', + settings: null, + repo: null, + worktreeId: WORKTREE_ID, + promptDelivery: 'submit-after-ready', + launchSource: 'source_control_recovery', + onLaunched, + onClose: vi.fn() + }) + + expect(started).toBe(true) + expect(onLaunched).toHaveBeenCalledOnce() + expect(mocks.createIntent).toHaveBeenCalledTimes(2) + expectSentTo(fresh.sessionId, 'Fix the failing check') + expect(mocks.toastError).not.toHaveBeenCalled() + }) + + it('sends notes to a new chat while another chat is unconfirmed', async () => { + await leaveFirstLaunchUnconfirmed() + + const notes = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'review notes', + promptDelivery: 'submit-after-ready' + }) + + expect(notes.sessionId).toBe(fresh.sessionId) + await expect(notes.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expectSentTo(fresh.sessionId, 'review notes') + // The unconfirmed chat is left for its own re-check. + expect(getStructuredAgentSessionLaunchLifecycle(WORKTREE_ID, unconfirmed.sessionId)).toBe( + 'visibility-unknown' + ) + }) + + it("opens a new chat with its text, not a draft, while a restored chat's Retry is in flight", async () => { + const failed = launchIntent('session-failed') + mocks.createIntent.mockReturnValueOnce(failed).mockReturnValueOnce(fresh) + mocks.launch.mockRejectedValueOnce(new StructuredAgentSessionCreateRefusalError('unsupported')) + startStructuredAgentLaunch(WORKTREE_ID, 'codex', { prompt: 'first task' }) + await flushLaunchSettlement() + // Reload: the registry is memory; the failed record is what survives. + resetStructuredAgentLaunchRegistryForTests() + mocks.launch.mockImplementationOnce(() => new Promise(() => undefined)) + expect(retryStructuredAgentSessionLaunch(WORKTREE_ID, failed.sessionId)).toBe(true) + mocks.seedDraft.mockClear() + + const next = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { + prompt: 'fix it', + promptDelivery: 'submit-after-ready' + }) + + expect(next.sessionId).toBe(fresh.sessionId) + await expect(next.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expectSentTo(fresh.sessionId, 'fix it') + expect(mocks.seedDraft).not.toHaveBeenCalled() + }) + + it('opens a new chat from a + menu pick while another chat is unconfirmed', async () => { + await leaveFirstLaunchUnconfirmed() + // Why: the + menu disables an agent only while a pick would join a start in flight. + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('idle') + + const pick = startStructuredAgentLaunch(WORKTREE_ID, 'codex') + + expect(pick.sessionId).toBe(fresh.sessionId) + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('pending') + // No re-check of the unconfirmed chat: only its own two create attempts and the new one. + expect(mocks.launch).toHaveBeenCalledTimes(3) + await expect(pick.launchResult).resolves.toEqual({ sessionId: fresh.sessionId, fence: 1 }) + }) + + it('still coalesces a repeat of one request racing for one chat', async () => { + mocks.createIntent.mockReturnValueOnce(fresh) + mocks.launch.mockImplementationOnce(() => new Promise(() => undefined)) + + const first = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { prompt: 'one' }) + const second = startStructuredAgentLaunch(WORKTREE_ID, 'codex', { prompt: 'one' }) + + expect(second.sessionId).toBe(first.sessionId) + expect(mocks.createIntent).toHaveBeenCalledOnce() + }) + + it('still re-checks an unconfirmed resume instead of adopting its conversation twice', async () => { + await leaveFirstLaunchUnconfirmed({ resume: true }) + expect(getStructuredAgentLaunchStatus(WORKTREE_ID, 'codex')).toBe('unknown') + + expect(startStructuredAgentLaunch(WORKTREE_ID, 'codex', { resumeFrom }).sessionId).toBe( + unconfirmed.sessionId + ) + expect(mocks.createIntent).toHaveBeenCalledOnce() + }) +}) diff --git a/src/renderer/src/lib/structured-agent-session-launch.test.ts b/src/renderer/src/lib/structured-agent-session-launch.test.ts index 5d2362b63a0..cc43a2d34d0 100644 --- a/src/renderer/src/lib/structured-agent-session-launch.test.ts +++ b/src/renderer/src/lib/structured-agent-session-launch.test.ts @@ -425,7 +425,7 @@ describe('startStructuredAgentLaunch', () => { expect(toast.error).not.toHaveBeenCalled() }) - it('delivers a prompt from a coalesced caller after the shared launch settles', async () => { + it("delivers a repeated request's prompt once, to both callers, after the shared launch settles", async () => { const worktreeId = 'wt-coalesced-prompt' const intent = launchIntent(worktreeId) let resolveLaunch: (receipt: { sessionId: string; fence: number }) => void = () => {} @@ -442,16 +442,23 @@ describe('startStructuredAgentLaunch', () => { ok: true, value: { submission: { dispatchState: 'accepted' } } }) - startStructuredAgentLaunch(worktreeId, 'codex') + const first = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) const second = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) resolveLaunch({ sessionId: intent.sessionId, fence: 1 }) - await expect(second.promptDeliveryResult).resolves.toEqual({ - delivered: true, - failureNotified: false - }) + for (const caller of [first, second]) { + await expect(caller.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + } await flushLaunchSettlement() + expect(second.sessionId).toBe(first.sessionId) + expect( + mocks.callStructuredAgentSession.mock.calls.filter((call) => call[1] === 'agentSession.send') + ).toHaveLength(1) + expect(mocks.callStructuredAgentSession).toHaveBeenCalledWith( { kind: 'local' }, 'agentSession.send', @@ -467,36 +474,42 @@ describe('startStructuredAgentLaunch', () => { const worktreeId = 'wt-coalesced-prompt-reservation' const intent = launchIntent(worktreeId) let resolveLaunch!: (receipt: { sessionId: string; fence: number }) => void - let resolveDelivery!: (result: { - ok: true - value: { submission: { dispatchState: 'accepted' } } - }) => void + const pendingSends: ((result: unknown) => void)[] = [] mocks.createIntent.mockReturnValue(intent) mocks.launch.mockImplementationOnce(() => new Promise((resolve) => (resolveLaunch = resolve))) vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ publishedSnapshot(worktreeId, intent.sessionId) ]) - mocks.callStructuredAgentSession.mockImplementationOnce( - () => new Promise((resolve) => (resolveDelivery = resolve)) + // Every send waits, so a second send of the repeated text would show up below. + mocks.callStructuredAgentSession.mockImplementation( + () => new Promise((resolve) => pendingSends.push(resolve)) ) - startStructuredAgentLaunch(worktreeId, 'codex') + startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) const coalesced = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) resolveLaunch({ sessionId: intent.sessionId, fence: 1 }) await vi.waitFor(() => expect(mocks.callStructuredAgentSession).toHaveBeenCalledOnce()) - startStructuredAgentLaunch(worktreeId, 'codex') + const whileSending = startStructuredAgentLaunch(worktreeId, 'codex', { + prompt: 'second prompt' + }) expect(mocks.createIntent).toHaveBeenCalledOnce() expect(mocks.launch).toHaveBeenCalledOnce() - resolveDelivery({ ok: true, value: { submission: { dispatchState: 'accepted' } } }) - await expect(coalesced.promptDeliveryResult).resolves.toEqual({ - delivered: true, - failureNotified: false - }) + await flushLaunchSettlement() + for (const resolve of pendingSends) { + resolve({ ok: true, value: { submission: { dispatchState: 'accepted' } } }) + } + for (const caller of [coalesced, whileSending]) { + await expect(caller.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + } + expect(mocks.callStructuredAgentSession).toHaveBeenCalledOnce() }) - it('keeps one launch identity per worktree while the outcome is unknown', async () => { + it('opens a new chat for a new start while an earlier outcome is unknown', async () => { const worktreeId = 'wt-unknown-different-prompts' const intent = launchIntent(worktreeId) mocks.createIntent.mockReturnValueOnce(intent) @@ -505,10 +518,16 @@ describe('startStructuredAgentLaunch', () => { startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'first prompt' }) await flushLaunchSettlement() - startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) + const second = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) await flushLaunchSettlement() - expect(mocks.createIntent).toHaveBeenCalledOnce() + expect(mocks.createIntent).toHaveBeenCalledTimes(2) + expect(second.sessionId).not.toBe(intent.sessionId) + expect(readOutbox(second.sessionId)).toEqual([ + expect.objectContaining({ + body: expect.objectContaining({ blocks: [{ type: 'text', text: 'second prompt' }] }) + }) + ]) }) it('reconciles a host commit when the create reply is lost', async () => { @@ -572,20 +591,22 @@ describe('startStructuredAgentLaunch', () => { expect(toast.error).not.toHaveBeenCalled() }) - it('keeps an unresolved identity reserved until inventory reconciles it', async () => { + // Why a resume: the host refuses a second adoption of the conversation, so a new resume re-checks. + it('keeps an unresolved resume identity reserved until inventory reconciles it', async () => { const worktreeId = 'wt-still-unknown' + const resumeFrom = { providerSessionId: 'provider-still-unknown' } const intent = launchIntent(worktreeId) - mocks.createIntent.mockReturnValueOnce(intent) + mocks.createIntent.mockReturnValueOnce({ ...intent, params: { ...intent.params, resumeFrom } }) mocks.launch.mockRejectedValue(new Error('offline')) vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([]) - startStructuredAgentLaunch(worktreeId, 'codex') + startStructuredAgentLaunch(worktreeId, 'codex', { resumeFrom }) await flushLaunchSettlement() vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ publishedSnapshot(worktreeId, intent.sessionId) ]) - startStructuredAgentLaunch(worktreeId, 'codex') + startStructuredAgentLaunch(worktreeId, 'codex', { resumeFrom }) await flushLaunchSettlement() expect(mocks.createIntent).toHaveBeenCalledOnce() @@ -607,8 +628,10 @@ describe('startStructuredAgentLaunch', () => { vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ publishedSnapshot(worktreeId, intent.sessionId) ]) - const retry = startStructuredAgentLaunch(worktreeId, 'codex') - await expect(retry.launchResult).resolves.toEqual({ sessionId: intent.sessionId, fence: 1 }) + // The chat's own Retry re-checks it; a new start would open a new chat instead. + expect(retryStructuredAgentSessionLaunch(worktreeId, intent.sessionId)).toBe(true) + await flushLaunchSettlement() + expect(getStructuredAgentSessionLaunchLifecycle(worktreeId, intent.sessionId)).toBeNull() expect(readOutbox(intent.sessionId)).toEqual([ expect.objectContaining({ @@ -657,7 +680,7 @@ describe('startStructuredAgentLaunch', () => { startStructuredAgentLaunch(worktreeId, 'codex') await flushLaunchSettlement() - startStructuredAgentLaunch(worktreeId, 'codex') + expect(retryStructuredAgentSessionLaunch(worktreeId, first.sessionId)).toBe(true) await flushLaunchSettlement() expect(mocks.createIntent).toHaveBeenCalledOnce() @@ -670,34 +693,6 @@ describe('startStructuredAgentLaunch', () => { expect(toast.error).not.toHaveBeenCalled() }) - it('does not stage the preserved launch prompt again on retry', async () => { - const worktreeId = 'wt-refused-prompt-retry' - const intent = launchIntent(worktreeId, 'session-refused-prompt-retry') - mocks.createIntent.mockReturnValueOnce(intent) - mocks.launch - .mockRejectedValueOnce(new StructuredAgentSessionCreateRefusalError('unsupported')) - .mockResolvedValueOnce({ sessionId: intent.sessionId, fence: 1 }) - vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ - publishedSnapshot(worktreeId, intent.sessionId) - ]) - - startStructuredAgentLaunch(worktreeId, 'codex', { - prompt: 'only once', - promptDelivery: 'draft' - }) - await flushLaunchSettlement() - expect(readOutbox(intent.sessionId)).toEqual([]) - - const retry = startStructuredAgentLaunch(worktreeId, 'codex', { - prompt: 'only once', - promptDelivery: 'draft' - }) - await expect(retry.launchResult).resolves.toEqual({ sessionId: intent.sessionId, fence: 1 }) - - expect(readOutbox(intent.sessionId)).toEqual([]) - expect(mocks.seedDraft).toHaveBeenCalledOnce() - }) - // A paired server's create seeds from its settings at create time, which its probe reports. it("shows the seed a paired server's probe reports on a retry, not the first admission's", async () => { const worktreeId = 'wt-paired-retry-seed' @@ -781,7 +776,7 @@ describe('startStructuredAgentLaunch', () => { storageFailure.mockRestore() }) - it('reports every coalesced prompt as undelivered after refusal', async () => { + it("reports a repeated request's prompt as undelivered to both callers after refusal", async () => { const worktreeId = 'wt-refused-coalesced-prompts' const intent = launchIntent(worktreeId) let rejectLaunch!: (error: unknown) => void @@ -791,8 +786,8 @@ describe('startStructuredAgentLaunch', () => { ) const first = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'first prompt' }) - const second = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'second prompt' }) - expect(readOutbox(intent.sessionId)).toHaveLength(2) + const second = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'first prompt' }) + expect(readOutbox(intent.sessionId)).toHaveLength(1) rejectLaunch(new StructuredAgentSessionCreateRefusalError('unsupported')) await expect(first.launchResult).rejects.toBeInstanceOf( @@ -806,41 +801,44 @@ describe('startStructuredAgentLaunch', () => { delivered: false, failureNotified: true }) - expect(readOutbox(intent.sessionId)).toHaveLength(2) + expect(readOutbox(intent.sessionId)).toHaveLength(1) }) - it('delivers a coalesced caller the way the launch it joined already decided', async () => { + it('opens a new chat for the same text asked to be sent instead of drafted', async () => { const worktreeId = 'wt-coalesced-delivery-mode' - const intent = launchIntent(worktreeId, 'coalesced-delivery-session') - let resolveLaunch!: (receipt: { sessionId: string; fence: number }) => void - mocks.createIntent.mockReturnValueOnce(intent) - mocks.launch.mockImplementation( - () => - new Promise<{ sessionId: string; fence: number }>((resolve) => (resolveLaunch = resolve)) + const drafted = launchIntent(worktreeId, 'coalesced-delivery-session') + const sent = launchIntent(worktreeId, 'sent-delivery-session') + mocks.createIntent.mockReturnValueOnce(drafted).mockReturnValueOnce(sent) + mocks.launch.mockImplementation((intent: StructuredAgentSessionLaunchIntent) => + Promise.resolve({ sessionId: intent.sessionId, fence: 1 }) ) vi.mocked(refreshLocalStructuredSessionTabs).mockResolvedValue([ - publishedSnapshot(worktreeId, intent.sessionId) + publishedSnapshot(worktreeId, drafted.sessionId), + publishedSnapshot(worktreeId, sent.sessionId) ]) + mocks.callStructuredAgentSession.mockResolvedValue({ + ok: true, + value: { submission: { dispatchState: 'accepted' } } + }) startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'PR #1 context', promptDelivery: 'draft' }) - const joiner = startStructuredAgentLaunch(worktreeId, 'codex', { + const other = startStructuredAgentLaunch(worktreeId, 'codex', { prompt: 'PR #1 context', promptDelivery: 'auto-submit' }) - resolveLaunch({ sessionId: intent.sessionId, fence: 1 }) - await flushLaunchSettlement() - // Why: the first caller's seed is already in the composer, so submitting the joiner's copy - // would show the user the text AND send it. - expect(readOutbox(intent.sessionId)).toEqual([]) - expect(joiner.promptDeliveryResult).toBeUndefined() - expect( - mocks.callStructuredAgentSession.mock.calls.some((call) => call[1] === 'agentSession.send') - ).toBe(false) - expect(mocks.seedDraft).toHaveBeenLastCalledWith( + // Why: a draft and a send are different requests; neither lands in the other's chat. + expect(other.sessionId).toBe(sent.sessionId) + await expect(other.promptDeliveryResult).resolves.toEqual({ + delivered: true, + failureNotified: false + }) + expect(readOutbox(drafted.sessionId)).toEqual([]) + expect(mocks.seedDraft).toHaveBeenCalledOnce() + expect(mocks.seedDraft).toHaveBeenCalledWith( expect.objectContaining({ tabId: 'structured-agent-session-coalesced-delivery-session', text: 'PR #1 context' diff --git a/src/renderer/src/lib/structured-agent-session-launch.ts b/src/renderer/src/lib/structured-agent-session-launch.ts index 5dd33c729ec..d2a174110c6 100644 --- a/src/renderer/src/lib/structured-agent-session-launch.ts +++ b/src/renderer/src/lib/structured-agent-session-launch.ts @@ -29,7 +29,6 @@ import * as launchDraft from './structured-agent-session-launch-draft' import { deleteStructuredLaunchStateIfCurrent, getStructuredAgentSessionLaunchLifecycle, - getStructuredLaunchState, getStructuredLaunchStateBySessionId, markStructuredAgentSessionLaunchCancelled, notifyStructuredLaunchListeners, @@ -38,12 +37,20 @@ import { type StructuredLaunchState } from './structured-agent-session-launch-registry' import { restorePersistedStructuredLaunchState } from './structured-agent-session-launch-reload' +import { + claimableStructuredLaunchAttempt, + getJoinableStructuredLaunchState +} from './structured-agent-session-launch-holders' import { applyStructuredLaunchHeldOptions } from './structured-agent-session-launch-options' import { trackLaunchSettlement } from './structured-agent-session-launch-outcome-tracking' +import { + repeatedStructuredLaunchAttempt, + structuredLaunchRequest, + type StructuredLaunchRequest +} from './structured-agent-session-launch-request' export type { StructuredAgentLaunchOptions, StructuredAgentLaunchReceipt } export { - getStructuredAgentLaunchStatus, getStructuredAgentSessionLaunchLifecycle, getStructuredAgentSessionLaunchResumes, hasStructuredAgentSessionLaunchCancellationTombstone, @@ -56,7 +63,7 @@ export { type StructuredAgentLaunchStatus, type StructuredAgentSessionLaunchLifecycle } from './structured-agent-session-launch-registry' -export { useStructuredAgentLaunchStatus } from './structured-agent-session-launch-status' +export * from './structured-agent-session-launch-status' export { useStructuredAgentSessionLaunchSelection } from './structured-agent-session-launch-options' type StructuredLaunchStateResult = { @@ -125,7 +132,7 @@ function adoptPairedHostSeed( } function resetStructuredLaunchCallers(state: StructuredLaunchState): void { - state.callers = createStructuredLaunchCallerGroup() + state.callers = createStructuredLaunchCallerGroup({ kind: 'retry' }) state.callers.onSettled = () => maybeCleanupLaunchState(state) } @@ -149,39 +156,65 @@ function restartStructuredLaunchState(state: StructuredLaunchState): void { notifyStructuredLaunchListeners() } +function joinStructuredLaunchState( + existing: StructuredLaunchState, + agent: AgentSessionHandleProvider, + options: StructuredAgentLaunchOptions, + request: StructuredLaunchRequest +): StructuredLaunchStateResult | undefined { + // A repeat (a double click) shares the text the first click staged, so it is sent once. + const repeat = repeatedStructuredLaunchAttempt(existing.callers.attempt, request) + // An empty chat takes the first text sent to it, delivered the way that request asked. + const claim = claimableStructuredLaunchAttempt(existing, request) + const retrying = existing.visibilityUnknown + const joined = joinLaunchDelivery( + options, + claim ? options.promptDelivery : existing.promptDelivery + ) + // Why: an unconfirmed launch keeps its draft/outbox, so a recheck must not stage it twice. + const text = retrying || repeat ? '' : outboxPromptText(joined) + const stagedPrompt = text + ? enqueueStructuredAgentSessionLaunchPrompt(existing.intent.sessionId, text) + : (repeat?.stagedEntry ?? null) + // An unstaged claim stays unclaimed: the new launch it falls to reports the failure. + if (claim && text && !stagedPrompt) { + return undefined + } + if (retrying) { + restartStructuredLaunchState(existing) + } + if (claim) { + existing.promptDelivery = options.promptDelivery + Object.assign(claim, { request, stagedEntry: stagedPrompt }) + } + if (!retrying && !repeat) { + launchDraft.seedStructuredAgentLaunchDraft(existing.intent.sessionId, agent, joined) + } + const { prompt: _retryPrompt, ...joinedWithoutPrompt } = joined + const callerOptions = retrying ? joinedWithoutPrompt : joined + return { + state: existing, + caller: addStructuredLaunchCaller({ + group: existing.callers, + launchResult: existing.promise, + target: existing.intent.target, + options: callerOptions, + stagedEntry: stagedPrompt + }) + } +} + function structuredAgentLaunchState( worktreeId: string, agent: AgentSessionHandleProvider, options: StructuredAgentLaunchOptions ): StructuredLaunchStateResult { const identity = structuredLaunchIdentity(worktreeId, agent, options.resumeFrom) - const existing = getStructuredLaunchState(identity) - if (existing) { - const retrying = existing.visibilityUnknown || existing.callers.outcome === 'failed' - if (retrying) { - restartStructuredLaunchState(existing) - } - const joined = joinLaunchDelivery(options, existing.promptDelivery) - // Why: failed launches keep their draft/outbox, so a retry must not stage the same prompt twice. - const text = retrying ? '' : outboxPromptText(joined) - const stagedPrompt = text - ? enqueueStructuredAgentSessionLaunchPrompt(existing.intent.sessionId, text) - : null - if (!retrying) { - launchDraft.seedStructuredAgentLaunchDraft(existing.intent.sessionId, agent, joined) - } - const { prompt: _retryPrompt, ...joinedWithoutPrompt } = joined - const callerOptions = retrying ? joinedWithoutPrompt : joined - return { - state: existing, - caller: addStructuredLaunchCaller({ - group: existing.callers, - launchResult: existing.promise, - target: existing.intent.target, - options: callerOptions, - stagedEntry: stagedPrompt - }) - } + const request = structuredLaunchRequest(options) + const existing = getJoinableStructuredLaunchState(identity, request) + const joined = existing && joinStructuredLaunchState(existing, agent, options, request) + if (joined) { + return joined } const intent = createStructuredAgentSessionLaunchIntent( @@ -196,7 +229,11 @@ function structuredAgentLaunchState( ? enqueueStructuredAgentSessionLaunchPrompt(intent.sessionId, text) : null launchDraft.seedStructuredAgentLaunchDraft(intent.sessionId, agent, options) - const callers = createStructuredLaunchCallerGroup() + const callers = createStructuredLaunchCallerGroup({ + kind: 'first', + request, + stagedEntry: stagedPrompt + }) const state: StructuredLaunchState = { identity, intent, diff --git a/src/renderer/src/lib/structured-agent-session-paired-admission.test.ts b/src/renderer/src/lib/structured-agent-session-paired-admission.test.ts index 40f522bd2ed..5c14f640640 100644 --- a/src/renderer/src/lib/structured-agent-session-paired-admission.test.ts +++ b/src/renderer/src/lib/structured-agent-session-paired-admission.test.ts @@ -29,7 +29,7 @@ import { beginStructuredAgentSessionProvisionalLaunch } from './structured-agent import { beginDirectWorkItemStructuredLaunch } from './launch-work-item-direct-agent-routing' import type { AiVaultSession } from '../../../shared/ai-vault-types' import { resumeAiVaultSessionInNewChat } from '@/components/right-sidebar/ai-vault-session-resume-in-chat-launch' -import { getStructuredAgentLaunchStatus } from './structured-agent-session-launch-registry' +import { getStructuredAgentLaunchStatus } from './structured-agent-session-launch-status' import { getStructuredAgentSessionLaunchSelection } from './structured-agent-session-launch-options' import { peekWebSessionFocusIntent } from '@/runtime/web-session-focus-intent' 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..6d49bb1baa5 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', @@ -400,6 +406,26 @@ describe('createUISlice agent send target mode', () => { expect(mocks.toastSuccess).toHaveBeenCalledWith('Sent to Claude') }) + it('sends nothing to a sidebar agent when every note is already on its way', async () => { + const store = createAgentSendStore() + const onPromptHandedOff = vi.fn() + seedAgentSendState(store) + store.getState().openAgentSendPopoverTargetMode({ + id: 'send-1', + worktreeId, + source: 'diff-notes', + prompt: '', + label: 'All unsent notes', + launchSource: 'notes_send', + onPromptHandedOff + }) + + await expect(store.getState().sendPromptToSidebarAgentTarget(readyPaneKey)).resolves.toBe(false) + + expect(mocks.sendNotesToActiveAgentSession).not.toHaveBeenCalled() + expect(onPromptHandedOff).not.toHaveBeenCalled() + }) + it('keeps target mode open and does not run delivery callback when send fails', async () => { const store = createAgentSendStore() const onPromptDelivered = vi.fn() 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..7af82053993 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 @@ -120,7 +120,8 @@ export function createUiAgentActions( }), sendPromptToSidebarAgentTarget: async (paneKey) => { const mode = get().agentSendPopoverTargetMode - if (!mode || mode.status === 'sending') { + // An empty prompt has nothing to send: every note may already be on its way. + if (!mode || mode.status === 'sending' || !mode.prompt.trim()) { return false } @@ -154,7 +155,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 +165,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 = {