fix(notes): a new chat's staged prompt decides when its notes leave the shelf

Notes sent to a new agent were released back to the shelf as soon as the
chat's start failed, while that failed chat kept the same text staged for its
own Retry. Re-sending and pressing Retry put the notes in two chats, and Retry
alone left delivered notes listed as unsent.

For a new chat, the notes now follow the prompt it staged: held while that
message is in the chat's outbox, cleared from the shelf when any dispatch
(Retry or re-check included) sends it on, and back on the shelf when the
chat is closed and its outbox thrown away. A paired server's chat is found
once its start settles. Running-agent sends keep their own result.

While notes are only on their way, the send button reads "Sending…" rather
than "All notes sent".
This commit is contained in:
Brennan Benson
2026-10-04 01:52:02 -07:00
parent ccbfffb0e3
commit 60d7e010e5
10 changed files with 458 additions and 22 deletions
@@ -119,8 +119,9 @@ export function useBrowserPageAnnotationSend({
])
const handleBrowserAnnotationsHandedOff = useCallback(
(delivered: Promise<unknown>): void => holdNotesForSend(sendableAnnotations, delivered),
[sendableAnnotations]
(delivered: Promise<unknown>): void =>
holdNotesForSend(sendableAnnotations, delivered, handleBrowserAnnotationsSentToAgent),
[handleBrowserAnnotationsSentToAgent, sendableAnnotations]
)
const handleClearBrowserAnnotations = useCallback((): void => {
@@ -507,7 +507,10 @@ describe('NotesSendMenu notes in flight', () => {
second.onPromptDelivered()
await Promise.resolve()
expect(onDelivered.mock.calls).toEqual([[[noteA]], [[noteB]]])
// 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', () => {
@@ -547,4 +550,26 @@ describe('NotesSendMenu notes in flight', () => {
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])
})
})
@@ -111,6 +111,10 @@ export function NotesSendMenu<TNote extends DiffCommentDeliverySnapshot>({
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[]) => {
@@ -120,8 +124,8 @@ export function NotesSendMenu<TNote extends DiffCommentDeliverySnapshot>({
)
const holdInFlight = useCallback(
(notes: readonly TNote[]) => (delivered: Promise<unknown>) =>
holdNotesForSend(notes.map(diffCommentSendKey), delivered),
[]
holdNotesForSend(notes.map(diffCommentSendKey), delivered, () => markDelivered(notes)),
[markDelivered]
)
const openTargetMode = useCallback(
@@ -211,7 +215,7 @@ export function NotesSendMenu<TNote extends DiffCommentDeliverySnapshot>({
triggerClassName
)}
disabled={!hasDeliverableNotes}
title={hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTooltip}
title={hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTitle}
aria-label={
triggerLabel
? translate(
@@ -242,7 +246,7 @@ export function NotesSendMenu<TNote extends DiffCommentDeliverySnapshot>({
</DropdownMenuTrigger>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={6}>
{hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTooltip}
{hasDeliverableNotes ? ENABLED_SEND_TOOLTIP : disabledTitle}
</TooltipContent>
</Tooltip>
<DropdownMenuContent
@@ -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<string, Set<EntryWatch>>()
// 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)
}
}
@@ -6,6 +6,10 @@ import {
} from '../../../../shared/structured-agent-session-outbox'
import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation'
import { createBrowserUuid } from '@/lib/browser-uuid'
import {
settleStructuredAgentSessionOutboxEntryWatches,
type StructuredAgentSessionOutboxEntryRemoval
} from './structured-agent-session-outbox-entry-watch'
const OUTBOX_PREFIX = 'orca:desktopStructuredAgentSessionOutbox:v1:'
@@ -159,6 +163,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) {
@@ -171,6 +184,7 @@ export function commitStructuredAgentSessionOutbox(
listener()
}
}
settleStructuredAgentSessionOutboxEntryWatches(sessionId, entries, removal)
return saved
}
@@ -209,7 +223,7 @@ export function enqueueStructuredAgentSessionLaunchPrompt(
}
export function discardStructuredAgentSessionLaunchOutbox(sessionId: string): void {
commitStructuredAgentSessionOutbox(sessionId, [])
commitOutbox(sessionId, [], {}, 'discarded')
}
export function mutateStructuredAgentSessionLaunchPrompt(
@@ -181,7 +181,7 @@ describe('QuickLaunchAgentMenuItems launch status', () => {
})
// Why: the notes menu holds what it sent until this result, so a second send leaves them out.
it("hands the launch's own delivery result to the notes menu", () => {
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' },
@@ -201,7 +201,8 @@ describe('QuickLaunchAgentMenuItems launch status', () => {
)
fireEvent.click(document.querySelector('[title="Launch Codex in a new terminal"]')!)
expect(onPromptHandedOff).toHaveBeenCalledWith(delivery)
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', () => {
@@ -18,6 +18,7 @@ import {
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
@@ -164,8 +165,16 @@ function QuickLaunchAgentMenuItemsInner({
)
return
}
if (result.promptDeliveryResult) {
onPromptHandedOff?.(result.promptDeliveryResult)
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
@@ -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<void> {
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<void> {
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)
})
})
@@ -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<NewAgentPromptOutcome> {
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<NewAgentPromptOutcome> {
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> | NewAgentPromptOutcome => {
const kept = delivered ? undefined : stagedInAnyLaunch(text)
return kept ? outcomeOf(kept) : { delivered }
}
return args.delivery.then(
(result) => afterStart(result.delivered),
() => afterStart(false)
)
}
+33 -10
View File
@@ -15,11 +15,32 @@ function changed(): void {
}
}
/** Takes `keys` out of the next send until `delivered` settles, whatever its result. */
export function holdNotesForSend(keys: readonly unknown[], delivered: Promise<unknown>): void {
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<unknown>,
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)
}
@@ -42,16 +63,18 @@ 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(
(listener) => {
listeners.add(listener)
return () => listeners.delete(listener)
},
() => version,
() => version
)
return useSyncExternalStore(subscribe, getVersion, getVersion)
}
/** A note's identity for delivery: an edit makes it a new pending note, as for its removal. */