diff --git a/mobile/src/session/MobileNativeChatMessage.test.ts b/mobile/src/session/MobileNativeChatMessage.test.ts index 135469d89171..074473e2c7dc 100644 --- a/mobile/src/session/MobileNativeChatMessage.test.ts +++ b/mobile/src/session/MobileNativeChatMessage.test.ts @@ -77,6 +77,7 @@ describe('MobileNativeChatMessage', () => { workedSeconds: number | null } | null onToggleTurn?: () => void + unsentNotice?: string } = {} ): ReactTestRenderer { act(() => { @@ -146,6 +147,36 @@ describe('MobileNativeChatMessage', () => { expect(texts.some((text) => text.includes('/tmp/host.png'))).toBe(true) }) + it('says under a message the host recorded but never delivered why it was not sent', () => { + const tree = render( + { ...userMessage([{ type: 'text', text: 'hello' }]), unsent: true }, + { unsentNotice: "Orca couldn't reach the agent. Your message was not sent." } + ) + expect(textIn(tree.root)).toEqual([ + 'hello', + "Orca couldn't reach the agent. Your message was not sent." + ]) + }) + + it('still says it was not sent when no words for it are loaded', () => { + const tree = render({ ...userMessage([{ type: 'text', text: 'hello' }]), unsent: true }) + expect(textIn(tree.root)).toEqual(['hello', 'Your message was not sent.']) + }) + + it('says it was not sent as a muted label, not an error', () => { + const tree = render({ ...userMessage([{ type: 'text', text: 'hello' }]), unsent: true }) + const [label] = tree.root.findAll( + (node) => + node.props.style !== undefined && node.children.join('') === 'Your message was not sent.' + ) + expect(label?.props.style).toMatchObject({ color: colors.textMuted }) + }) + + it('says nothing more under a delivered message', () => { + const tree = render(userMessage([{ type: 'text', text: 'hello' }])) + expect(textIn(tree.root)).toEqual(['hello']) + }) + it('makes user message text selectable', () => { const tree = render(userMessage([{ type: 'text', text: 'Prompt I typed' }])) const text = tree.root diff --git a/mobile/src/session/MobileNativeChatMessage.tsx b/mobile/src/session/MobileNativeChatMessage.tsx index 83f93266d6ff..2553cb12ca05 100644 --- a/mobile/src/session/MobileNativeChatMessage.tsx +++ b/mobile/src/session/MobileNativeChatMessage.tsx @@ -17,6 +17,7 @@ import { ToolRun } from './MobileNativeChatToolRun' import type { NativeChatTurnStatus } from './use-mobile-native-chat-turn-status' import { isRenderableImageUri } from './mobile-native-chat-image-preview' import { styles, TEXT_SIZE } from './mobile-native-chat-message-styles' +import { AGENT_SESSION_WRITE_NOTICE_COPY } from '../../../src/shared/agent-session-write-notice-copy' function Prose({ block, @@ -118,7 +119,8 @@ function MobileNativeChatMessageImpl({ turnKey, onToggleTurn, activeTurnIsWorking, - structuredActivityUi = false + structuredActivityUi = false, + unsentNotice }: { message: NativeChatMessage toolsExpanded?: boolean @@ -139,6 +141,8 @@ function MobileNativeChatMessageImpl({ activeTurnIsWorking?: boolean /** Structured lane only: live tool progress plus the turn-status disclosure. */ structuredActivityUi?: boolean + /** Why the host did not deliver this message, when it is shown as not sent. */ + unsentNotice?: string }): React.JSX.Element { const isUser = message.role === 'user' const isReasoning = message.role === 'reasoning' @@ -207,6 +211,11 @@ function MobileNativeChatMessageImpl({ /> ) : null} + {isUser && message.unsent === true ? ( + + {unsentNotice ?? AGENT_SESSION_WRITE_NOTICE_COPY.notDoneSend} + + ) : null} {actionsOpen ? ( mobileNativeChatUnsentNotices(turnJournal, agent ?? null), + [turnJournal, agent] + ) const renderItem = useCallback( ({ item, index }: { item: NativeChatMessage; index: number }) => ( ), - [toolsExpanded, fontScale, onOpenFile, structuredActivityUi, turns] + [toolsExpanded, fontScale, onOpenFile, structuredActivityUi, turns, unsentNotices] ) const liveStatus = @@ -349,21 +356,7 @@ export function MobileNativeChatView({ scrollEventThrottle={32} onContentSizeChange={pinToTailAfterContentResize} onLayout={pinToTail} - ListHeaderComponent={ - hasMore ? ( - - {loadingEarlier ? ( - - ) : ( - Load earlier messages - )} - - ) : null - } + ListHeaderComponent={mobileNativeChatListHeader(hasMore, loadingEarlier, loadEarlier)} ListFooterComponent={mobileNativeChatListFooter( liveStatus, turns.waitingRows, diff --git a/mobile/src/session/mobile-native-chat-draft-reconcile.ts b/mobile/src/session/mobile-native-chat-draft-reconcile.ts index 3ae08af66881..7565fa162d5b 100644 --- a/mobile/src/session/mobile-native-chat-draft-reconcile.ts +++ b/mobile/src/session/mobile-native-chat-draft-reconcile.ts @@ -18,20 +18,37 @@ export type UnconfirmedSend = { baselineTailMessageId: string | null /** Queued-draft cards on screen at send time; see `findQueuedUnconfirmedSends`. */ baselineQueuedMessageIds?: readonly string[] + /** Rows shown as not sent at send time; see `sendBaselineUnsentMessageIds`. */ + baselineUnsentMessageIds?: readonly string[] deadline: ReturnType | null } -export function normalizedUserText(message: NativeChatMessage): string | null { - return normalizedNativeChatUserMessageText(message) +/** Rows shown as not sent when a send went out. Only these can't be its echo: one that appears + * later is the send's own row, settled as not sent. The count, image and unconfirmed matchers + * all apply this. */ +export function sendBaselineUnsentMessageIds(messages: readonly NativeChatMessage[]): string[] { + return messages.filter((message) => message.unsent === true).map((message) => message.id) } +/** The row a send's echo must land after: the newest one, past any shown as not sent, which are in + * the send's baseline and so never its echo. */ +export function sendBaselineTailMessageId(messages: readonly NativeChatMessage[]): string | null { + for (let index = messages.length - 1; index >= 0; index -= 1) { + if (messages[index]?.unsent !== true) { + return messages[index]?.id ?? null + } + } + return null +} + +/** At send time: every row shown as not sent is then in the send's baseline, so none is counted. */ export function countUserTextOccurrences( messages: readonly NativeChatMessage[], text: string ): number { let count = 0 for (const message of messages) { - if (normalizedUserText(message) === text) { + if (message.unsent !== true && normalizedNativeChatUserMessageText(message) === text) { count++ } } @@ -63,6 +80,8 @@ export type PendingImagePreviewEcho = { images?: string[] expectedOccurrence: number baselineTailMessageId: string | null + /** See `sendBaselineUnsentMessageIds`. */ + baselineUnsentMessageIds?: readonly string[] } export type LandedImagePreviewEcho = { @@ -177,12 +196,13 @@ export function findLandedImagePreviewEchoes( continue } const targetText = normalizeNativeChatUserText(entry.text) + const baselineUnsent = new Set(entry.baselineUnsentMessageIds) const candidates = normalized.filter((message) => { - if (message.role !== 'user') { + if (message.role !== 'user' || (message.unsent === true && baselineUnsent.has(message.id))) { return false } if (targetText) { - const text = normalizedUserText(message) + const text = normalizedNativeChatUserMessageText(message) if (text === null) { return false } @@ -257,15 +277,17 @@ export function findLandedUnconfirmedSends( // (`[Image: source: …]` or no text) keys under '' so an empty-text send can // claim it. const messageIndexById = new Map() - const userMessagesByText = new Map>() + const userMessagesByText = new Map() for (const [index, message] of messages.entries()) { messageIndexById.set(message.id, index) if (message.role !== 'user') { continue } - const key = isImageSourceUserTurn(message) ? '' : (normalizedUserText(message) ?? '') + const key = isImageSourceUserTurn(message) + ? '' + : (normalizedNativeChatUserMessageText(message) ?? '') const current = userMessagesByText.get(key) ?? [] - current.push({ id: message.id, index }) + current.push({ id: message.id, index, unsent: message.unsent === true }) userMessagesByText.set(key, current) } @@ -278,9 +300,15 @@ export function findLandedUnconfirmedSends( if (tailIndex === undefined) { continue } + const baselineUnsent = new Set(entry.baselineUnsentMessageIds) const echo = userMessagesByText .get(entry.normalizedText) - ?.find((message) => message.index > tailIndex && !claimedMessageIds.has(message.id)) + ?.find( + (message) => + message.index > tailIndex && + !claimedMessageIds.has(message.id) && + !(message.unsent && baselineUnsent.has(message.id)) + ) if (echo) { claimedMessageIds.add(echo.id) landed.push(entry) diff --git a/mobile/src/session/mobile-native-chat-list-header.tsx b/mobile/src/session/mobile-native-chat-list-header.tsx new file mode 100644 index 000000000000..bb3e61c374fe --- /dev/null +++ b/mobile/src/session/mobile-native-chat-list-header.tsx @@ -0,0 +1,24 @@ +import type { ReactElement } from 'react' +import { ActivityIndicator, Pressable, Text } from 'react-native' +import { colors } from '../theme/mobile-theme' +import { styles } from './mobile-native-chat-view-styles' + +/** The "Load earlier messages" control, while older history remains. */ +export function mobileNativeChatListHeader( + hasMore: boolean | undefined, + loadingEarlier: boolean | undefined, + loadEarlier: () => void +): ReactElement | null { + if (!hasMore) { + return null + } + return ( + + {loadingEarlier ? ( + + ) : ( + Load earlier messages + )} + + ) +} diff --git a/mobile/src/session/mobile-native-chat-message-styles.ts b/mobile/src/session/mobile-native-chat-message-styles.ts index 225d664e65e4..8be84fd6de2c 100644 --- a/mobile/src/session/mobile-native-chat-message-styles.ts +++ b/mobile/src/session/mobile-native-chat-message-styles.ts @@ -37,6 +37,11 @@ export const styles = StyleSheet.create({ reasoning: { opacity: 0.7 }, + unsentLabel: { + marginTop: spacing.xs, + color: colors.textMuted, + fontSize: typography.metaSize + }, toolRun: { marginTop: spacing.xs }, diff --git a/mobile/src/session/mobile-native-chat-not-sent-resend.test.ts b/mobile/src/session/mobile-native-chat-not-sent-resend.test.ts new file mode 100644 index 000000000000..00483ef8e566 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-not-sent-resend.test.ts @@ -0,0 +1,275 @@ +// The phone draws a message the host recorded and then rejected in place, as not sent. The user's +// way on is to send its text again, as a new message; the not-sent original stays where it was. The +// phone's own echo of that resend must land on the new copy alone, never on the original. + +import { describe, expect, it } from 'vitest' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../src/shared/agent-session-journal-types' +import type { NativeChatMessage } from '../../../src/shared/native-chat-types' +import { projectStructuredAgentSessionMessages } from '../../../src/shared/structured-agent-session-message-projection' +import { + countUserTextOccurrences, + findLandedImagePreviewEchoes, + findLandedUnconfirmedSends, + normalizeReconcileText, + sendBaselineTailMessageId, + sendBaselineUnsentMessageIds +} from './mobile-native-chat-draft-reconcile' +import { appendMobileNativeChatPending } from './mobile-native-chat-pending-echo' +import { retireLandedMobileNativeChatPending } from './mobile-native-chat-pending-retirement' + +const TEXT = 'fix the test' + +function user(id: string, sequence: number, text: string): AgentJournalRenderItem { + return { + itemId: agentJournalSubmissionKey(id), + revision: 1, + sequence, + observedAt: sequence, + turnScope: { kind: 'thread' }, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text }] } + } +} + +function answer(id: string, sequence: number, text: string): AgentJournalRenderItem { + return { + itemId: id, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text }] } + } +} + +function submission( + id: string, + text: string, + at: number, + patch: Partial = {} +): AgentJournalSubmission { + return { + clientMessageId: id, + fence: 1, + payloadFingerprint: `body:${text}`, + dispatchState: 'accepted', + providerItemId: `provider-${id}`, + reason: null, + submittedAt: at, + resolvedAt: at, + ...patch + } +} + +function phone( + items: AgentJournalRenderItem[], + submissions: AgentJournalSubmission[] +): NativeChatMessage[] { + return projectStructuredAgentSessionMessages(items, [], submissions, { + rejectedInPlace: true, + queuedMessageIds: [] + }) +} + +const REJECTED = submission('m1', TEXT, 10, { + dispatchState: 'rejected', + providerItemId: null, + reason: 'provider_write_failed: broken pipe', + resolvedAt: 11 +}) +const BEFORE_ITEMS = [answer('hi', 5, 'Hi'), user('m1', 11, TEXT)] +const AFTER_ITEMS = [...BEFORE_ITEMS, user('m2', 20, TEXT), answer('done', 21, 'Done.')] +const AFTER_SUBMISSIONS = [REJECTED, submission('m2', TEXT, 20)] + +describe('resending a not-sent message from the phone', () => { + it('ends as the not-sent original plus the delivered resend, with no echo left', () => { + const before = phone(BEFORE_ITEMS, [REJECTED]) + const normalizedText = normalizeReconcileText(TEXT) + const origin = { + draftKey: 'draft', + draftEditGeneration: 0, + pendingKey: 'pending', + normalizedText, + baselineOccurrences: countUserTextOccurrences(before, normalizedText), + baselineTailMessageId: sendBaselineTailMessageId(before), + baselineResolved: true, + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(before) + } + const pending = appendMobileNativeChatPending({}, 'pending', 'pending-1', origin, TEXT).pending + + const after = phone(AFTER_ITEMS, AFTER_SUBMISSIONS) + expect( + after.filter((message) => message.role === 'user').map((message) => message.unsent ?? false) + ).toEqual([true, false]) + expect(retireLandedMobileNativeChatPending(after, pending ?? [], new Set())).toEqual([]) + }) + + it('sees an ack-lost resend that landed as landed, so no doubt is raised', () => { + const before = phone(BEFORE_ITEMS, [REJECTED]) + const landed = findLandedUnconfirmedSends(phone(AFTER_ITEMS, AFTER_SUBMISSIONS), [ + { + draftKey: 'draft', + pendingKey: 'pending', + text: TEXT, + normalizedText: normalizeReconcileText(TEXT), + baselineTailMessageId: sendBaselineTailMessageId(before), + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(before), + deadline: null + } + ]) + expect(landed).toHaveLength(1) + }) + + it('lets the older not-sent copy settle nothing while the resend is on its way', () => { + const before = phone(BEFORE_ITEMS, [REJECTED]) + const normalizedText = normalizeReconcileText(TEXT) + const origin = { + draftKey: 'draft', + draftEditGeneration: 0, + pendingKey: 'pending', + normalizedText, + baselineOccurrences: countUserTextOccurrences(before, normalizedText), + baselineTailMessageId: sendBaselineTailMessageId(before), + baselineResolved: true, + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(before) + } + const pending = appendMobileNativeChatPending({}, 'pending', 'pending-1', origin, TEXT).pending + expect(retireLandedMobileNativeChatPending(before, pending ?? [], new Set())).toHaveLength(1) + expect(findLandedUnconfirmedSends(before, [{ ...origin, text: TEXT, deadline: null }])).toEqual( + [] + ) + }) + + it('keeps a not-sent row where the host placed it, above what came after', () => { + const items = [ + user('m1', 11, 'hello'), + user('m3', 20, 'are you there?'), + answer('a3', 21, 'Yes') + ] + const messages = phone(items, [ + { ...REJECTED, payloadFingerprint: 'body:hello' }, + submission('m3', 'are you there?', 20) + ]) + expect(messages.map((message) => [message.id, message.unsent ?? false])).toEqual([ + [agentJournalSubmissionKey('m1'), true], + [agentJournalSubmissionKey('m3'), false], + ['a3', false] + ]) + }) +}) + +// The phone's first sight of its own row can already be the rejected one: a catch-up after the +// stream dropped, or a send made before the first read settled. That row settles the send. +describe('a phone send whose own row first appears as not sent', () => { + const OWN_REJECTED = submission('m2', 'later', 20, { + dispatchState: 'rejected', + providerItemId: null, + reason: 'provider_write_failed: broken pipe', + resolvedAt: 21 + }) + const BEFORE = phone(BEFORE_ITEMS, [REJECTED]) + const AFTER = phone([...BEFORE_ITEMS, user('m2', 21, 'later')], [REJECTED, OWN_REJECTED]) + + it('settles an ack-lost send, so no "Delivery unconfirmed" banner follows', () => { + expect( + findLandedUnconfirmedSends(AFTER, [ + { + draftKey: 'draft', + pendingKey: 'pending', + text: 'later', + normalizedText: normalizeReconcileText('later'), + baselineTailMessageId: sendBaselineTailMessageId(BEFORE), + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(BEFORE), + deadline: null + } + ]) + ).toHaveLength(1) + }) + + it("retires an accepted send's echo, so no bubble stays beside its row", () => { + const normalizedText = normalizeReconcileText('later') + const pending = appendMobileNativeChatPending( + {}, + 'pending', + 'pending-1', + { + draftKey: 'draft', + draftEditGeneration: 0, + pendingKey: 'pending', + normalizedText, + baselineOccurrences: countUserTextOccurrences(BEFORE, normalizedText), + baselineTailMessageId: sendBaselineTailMessageId(BEFORE), + baselineResolved: true, + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(BEFORE) + }, + 'later' + ).pending + expect(retireLandedMobileNativeChatPending(AFTER, pending ?? [], new Set())).toEqual([]) + }) +}) + +// A captioned image send's placeholder retires only once its local preview binds to its row, so the +// image matcher must apply the same rule: its own not-sent row binds it, an older one doesn't. +describe('a captioned image send whose own row turns not sent', () => { + const CAPTION = 'look at this' + function imageRow(id: string, unsent: boolean): NativeChatMessage { + return { + id, + role: 'user', + source: 'transcript', + timestamp: null, + blocks: [ + { type: 'text', text: CAPTION }, + { type: 'image-ref', path: '/tmp/x.png' } + ], + ...(unsent ? { unsent: true as const } : {}) + } + } + const HELLO: NativeChatMessage = { + id: 'a1', + role: 'assistant', + source: 'transcript', + timestamp: null, + blocks: [{ type: 'text', text: 'Hello' }] + } + + function placeholderLeft(before: NativeChatMessage[], after: NativeChatMessage[]): number { + const normalizedText = normalizeReconcileText(CAPTION) + const pending = + appendMobileNativeChatPending( + {}, + 'pending', + 'pending-1', + { + draftKey: 'draft', + draftEditGeneration: 0, + pendingKey: 'pending', + normalizedText, + baselineOccurrences: countUserTextOccurrences(before, normalizedText), + baselineTailMessageId: sendBaselineTailMessageId(before), + baselineResolved: true, + baselineUnsentMessageIds: sendBaselineUnsentMessageIds(before) + }, + CAPTION, + ['file:///local.png'] + ).pending ?? [] + const landed = findLandedImagePreviewEchoes(after, pending) + return retireLandedMobileNativeChatPending( + after, + pending, + new Set(landed.map((preview) => preview.pendingId)) + ).length + } + + it('retires its placeholder on its own not-sent row', () => { + expect(placeholderLeft([HELLO], [HELLO, imageRow('m2', true)])).toBe(0) + }) + + it('keeps it while only a not-sent copy from before the send is there', () => { + const before = [HELLO, imageRow('m1', true)] + expect(placeholderLeft(before, before)).toBe(1) + expect(placeholderLeft(before, [...before, imageRow('m2', true)])).toBe(0) + }) +}) diff --git a/mobile/src/session/mobile-native-chat-pending-baseline.ts b/mobile/src/session/mobile-native-chat-pending-baseline.ts index 903bf9d8f324..be13032fdd90 100644 --- a/mobile/src/session/mobile-native-chat-pending-baseline.ts +++ b/mobile/src/session/mobile-native-chat-pending-baseline.ts @@ -1,5 +1,8 @@ import type { NativeChatMessage } from '../../../src/shared/native-chat-types' -import { normalizeReconcileText } from './mobile-native-chat-draft-reconcile' +import { + normalizeReconcileText, + sendBaselineTailMessageId +} from './mobile-native-chat-draft-reconcile' import type { MobileNativeChatPendingMessage } from './mobile-native-chat-pending-echo' /** @@ -39,7 +42,7 @@ export function rebaseMobileNativeChatPendingBaselines( if (current.every((item) => item.baselineResolved)) { return current } - const baselineTailMessageId = messages.at(-1)?.id ?? null + const baselineTailMessageId = sendBaselineTailMessageId(messages) return current.map((item) => { if (item.baselineResolved) { return item diff --git a/mobile/src/session/mobile-native-chat-pending-echo.ts b/mobile/src/session/mobile-native-chat-pending-echo.ts index 136043447899..1d0d0ef57988 100644 --- a/mobile/src/session/mobile-native-chat-pending-echo.ts +++ b/mobile/src/session/mobile-native-chat-pending-echo.ts @@ -12,6 +12,8 @@ export type MobileNativeChatPendingMessage = { * and rebased onto the first authoritative read instead of reconciling * against rows that may belong to another tab. */ baselineResolved: boolean + /** Rows shown as not sent when it was sent: they can't retire it, its own row can. */ + baselineUnsentMessageIds?: readonly string[] } export type MobileNativeChatSendOrigin = { @@ -25,6 +27,8 @@ export type MobileNativeChatSendOrigin = { /** Queued-draft cards already on screen at send time, so an earlier identical * card cannot confirm this send. Structured lane on a queue-capable host only. */ baselineQueuedMessageIds?: readonly string[] + /** Rows shown as not sent at send time: an older not-sent copy can't settle this send. */ + baselineUnsentMessageIds?: readonly string[] } type PendingByKey = Record @@ -73,6 +77,9 @@ export function appendMobileNativeChatPending( : origin.baselineOccurrences + earlierOutstanding + 1, baselineTailMessageId: origin.baselineTailMessageId, baselineResolved: origin.baselineResolved, + ...(origin.baselineUnsentMessageIds + ? { baselineUnsentMessageIds: origin.baselineUnsentMessageIds } + : {}), ...(images?.length ? { images } : {}) } ] diff --git a/mobile/src/session/mobile-native-chat-pending-retirement.ts b/mobile/src/session/mobile-native-chat-pending-retirement.ts index 7f720302eccc..0eb74f6d81d0 100644 --- a/mobile/src/session/mobile-native-chat-pending-retirement.ts +++ b/mobile/src/session/mobile-native-chat-pending-retirement.ts @@ -1,10 +1,10 @@ import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { countImageSourceTurnsAfter, - normalizeReconcileText, - normalizedUserText + normalizeReconcileText } from './mobile-native-chat-draft-reconcile' import type { MobileNativeChatPendingMessage } from './mobile-native-chat-pending-echo' +import { normalizedNativeChatUserMessageText } from './mobile-native-chat-image-transcript-markers' const SPACE = ' ' const NO_PENDING_IDS: ReadonlySet = new Set() @@ -36,7 +36,8 @@ export function selectGluedPendingIds( const turns: UserTurn[] = [] for (const [index, message] of messages.entries()) { messageIndexById.set(message.id, index) - const text = normalizedUserText(message) + // A not-sent row is one refused structured send, never several glued on an input line. + const text = message.unsent === true ? null : normalizedNativeChatUserMessageText(message) if (text) { turns.push({ index, text }) } @@ -147,12 +148,26 @@ export function retireLandedMobileNativeChatPending( landedImagePendingIds: ReadonlySet ): MobileNativeChatPendingMessage[] { const landedCounts = new Map() + const unsentByText = new Map() for (const message of messages) { - const text = normalizedUserText(message) - if (text) { + const text = normalizedNativeChatUserMessageText(message) + if (!text) { + continue + } + if (message.unsent === true) { + unsentByText.set(text, [...(unsentByText.get(text) ?? []), message.id]) + } else { landedCounts.set(text, (landedCounts.get(text) ?? 0) + 1) } } + // A not-sent row settles a send only when it appeared after the send: then it is the send's own. + const landedFor = (item: MobileNativeChatPendingMessage, text: string): number => { + const baselineUnsent = new Set(item.baselineUnsentMessageIds) + return ( + (landedCounts.get(text) ?? 0) + + (unsentByText.get(text) ?? []).filter((id) => !baselineUnsent.has(id)).length + ) + } const landedPendingIds = new Set() // Why a separate set: a barrier preserves adjacency after a landing consumed a whole // row. An image landing can share its row with the send glued after it, so treating it @@ -173,7 +188,7 @@ export function retireLandedMobileNativeChatPending( item.text.trim() === '' ? countImageSourceTurnsAfter(messages, item.baselineTailMessageId) >= item.expectedOccurrence - : (landedCounts.get(normalizeReconcileText(item.text)) ?? 0) >= item.expectedOccurrence + : landedFor(item, normalizeReconcileText(item.text)) >= item.expectedOccurrence if (landed) { landedPendingIds.add(item.id) exactLandedIds.add(item.id) diff --git a/mobile/src/session/mobile-native-chat-send.ts b/mobile/src/session/mobile-native-chat-send.ts index 2a3bc5a60b2a..22d2874b4ee1 100644 --- a/mobile/src/session/mobile-native-chat-send.ts +++ b/mobile/src/session/mobile-native-chat-send.ts @@ -29,8 +29,9 @@ type MobileNativeChatSendArgs = { /** 'unknown' = the RPC failed without proof the request never reached the * desktop (ack loss after a write, or a cutover that cannot tell whether the * frame was written) — callers must not present it as a definite send failure. - * 'queued' = structured lane only: the host holds the message as a queued - * draft, so it shows as a card above the composer, never a transcript echo. */ + * 'queued' = structured lane only: the host holds the message, as a queued + * draft's card above the composer or as a recorded row in the transcript, so + * the client draws no echo and hands no text back. */ export type MobileNativeChatSendOutcome = 'accepted' | 'rejected' | 'unknown' | 'queued' /** What a terminal write can answer: the PTY lane has no draft queue. */ diff --git a/mobile/src/session/mobile-native-chat-unsent-notices.test.ts b/mobile/src/session/mobile-native-chat-unsent-notices.test.ts new file mode 100644 index 000000000000..485159a856b1 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-unsent-notices.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest' +import type { AgentSessionFailureFact } from '../../../src/shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../src/shared/agent-session-failure-words' +import { + agentJournalItemKey, + agentJournalSubmissionKey +} from '../../../src/shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../src/shared/agent-session-journal-types' +import { structuredAgentSessionCommandResultRowIdentity } from '../../../src/shared/structured-agent-session-command-entry' +import { structuredAgentSessionStartFailureRowIdentity } from '../../../src/shared/structured-agent-session-start-failure-row-key' +import { mobileNativeChatUnsentNotices } from './mobile-native-chat-unsent-notices' + +function rejected(patch: Partial = {}): AgentJournalSubmission { + return { + clientMessageId: 'sent-elsewhere', + fence: 1, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + reason: 'provider_write_failed: broken pipe', + submittedAt: 1, + resolvedAt: 1, + ...patch + } +} + +describe('the line under a message the host recorded and did not deliver', () => { + it("says why, in the host's words, keyed by the message's row", () => { + const notices = mobileNativeChatUnsentNotices( + { items: [], submissions: [rejected()] }, + 'claude' + ) + expect(notices.get(agentJournalSubmissionKey('sent-elsewhere'))).toBe( + "Orca couldn't reach the agent. Your message was not sent." + ) + }) + + it('says only that it was not sent when the start-failure row already says why', () => { + const startFailed: AgentSessionFailureFact = { kind: 'startFailed' } + const startRow: AgentJournalRenderItem = { + itemId: agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity('gen')), + revision: 1, + sequence: 1, + observedAt: 1, + body: { + kind: 'status', + tone: 'error', + ...agentSessionFailureWords(startFailed, { agentName: 'Claude', surface: 'row' }) + } + } + const notices = mobileNativeChatUnsentNotices( + { + items: [startRow], + submissions: [rejected({ reason: 'Written by the host.', rejection: startFailed })] + }, + 'claude' + ) + expect(notices.get(agentJournalSubmissionKey('sent-elsewhere'))).toBe( + 'Your message was not sent.' + ) + }) + + // Its turn's result row says how a refused command ended; a blocked one has only its own line. + it("says only that a command was not sent when its result row says why, else the command's words", () => { + const providerRejected: AgentSessionFailureFact = { + kind: 'providerRejected', + detail: { text: 'busy', audience: 'person' } + } + const refused = rejected({ clientMessageId: 'compact-1', rejection: providerRejected }) + const resultRow: AgentJournalRenderItem = { + itemId: agentJournalItemKey(structuredAgentSessionCommandResultRowIdentity('compact-1')), + revision: 1, + sequence: 2, + observedAt: 2, + body: { + kind: 'status', + tone: 'error', + ...agentSessionFailureWords( + { kind: 'compactionFailed', detail: { text: 'busy', audience: 'person' } }, + { agentName: 'Codex', surface: 'row' } + ) + } + } + const blocked = rejected({ + clientMessageId: 'compact-2', + rejection: { kind: 'commandRefused' } + }) + const notices = mobileNativeChatUnsentNotices( + { items: [resultRow], submissions: [refused, blocked] }, + 'codex' + ) + expect(notices.get(agentJournalSubmissionKey('compact-1'))).toBe('Your message was not sent.') + expect(notices.get(agentJournalSubmissionKey('compact-2'))).toBe( + "This command didn't run. Try it again." + ) + }) + + it('has nothing to say when no send was rejected', () => { + expect( + mobileNativeChatUnsentNotices( + { items: [], submissions: [rejected({ dispatchState: 'accepted' })] }, + 'claude' + ).size + ).toBe(0) + expect(mobileNativeChatUnsentNotices(null, 'claude').size).toBe(0) + }) +}) diff --git a/mobile/src/session/mobile-native-chat-unsent-notices.ts b/mobile/src/session/mobile-native-chat-unsent-notices.ts new file mode 100644 index 000000000000..b8f76a876a44 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-unsent-notices.ts @@ -0,0 +1,44 @@ +// The line under each message the host recorded and then did not deliver: the desktop row's words, +// in English. Only the transcript says it, so the phone's own send never repeats it in its banner. + +import { formatAgentTypeLabel } from '../../../src/shared/agent-type-label' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' +import { structuredAgentSessionCommandResultRows } from '../../../src/shared/structured-agent-session-command-entry' +import { agentSessionWriteNoticeEnglish } from '../../../src/shared/agent-session-refusal-notice' +import type { NativeChatTurnJournal } from '../../../src/shared/native-chat-turn-membership' +import { + structuredAgentSessionRecordedRejectionParts, + structuredAgentSessionStartFailureFacts +} from '../../../src/shared/structured-agent-session-recorded-rejection-words' + +const NO_NOTICES: ReadonlyMap = new Map() + +/** Keyed by the message id the transcript renders each recorded send under. */ +export function mobileNativeChatUnsentNotices( + journal: NativeChatTurnJournal | null, + agent: string | null +): ReadonlyMap { + const rejected = journal?.submissions.filter((row) => row.dispatchState === 'rejected') ?? [] + if (!journal || rejected.length === 0) { + return NO_NOTICES + } + const startFailures = structuredAgentSessionStartFailureFacts(journal.items) + const commandResults = structuredAgentSessionCommandResultRows(journal.items) + const context = { + retryControl: false, + ...(agent ? { agentName: formatAgentTypeLabel(agent) } : {}) + } + return new Map( + rejected.map((submission) => [ + agentJournalSubmissionKey(submission.clientMessageId), + agentSessionWriteNoticeEnglish( + structuredAgentSessionRecordedRejectionParts( + submission, + context, + startFailures, + commandResults + ) + ) + ]) + ) +} diff --git a/mobile/src/session/mobile-structured-agent-session-send-bypass.test.ts b/mobile/src/session/mobile-structured-agent-session-send-bypass.test.ts index c9da99a3cbaf..8631e61809d1 100644 --- a/mobile/src/session/mobile-structured-agent-session-send-bypass.test.ts +++ b/mobile/src/session/mobile-structured-agent-session-send-bypass.test.ts @@ -31,8 +31,33 @@ function queuedAnswer(clientMessageId: string, state: 'waiting' | 'withdrawn'): }) } -/** Each `agentSession.send` answered in turn: a lost answer, or a queued draft in that state. */ -function hostAnswering(answers: readonly ('lost' | 'waiting' | 'withdrawn')[]) { +function submissionAnswer(clientMessageId: string, dispatchState: 'accepted' | 'rejected') { + return ok({ + ok: true, + replayed: false, + fence: 3, + cursor: { epoch: 'epoch-1', sequence: 1 }, + value: { + clientMessageId, + submission: { + clientMessageId, + fence: 3, + payloadFingerprint: 'fingerprint', + dispatchState, + providerItemId: null, + reason: dispatchState === 'rejected' ? 'provider_write_failed: broken pipe' : null, + submittedAt: 1, + resolvedAt: 1 + } + } + }) +} + +/** Each `agentSession.send` answered in turn: a lost answer, a queued draft in that state, or a + * recorded submission in that state. */ +function hostAnswering( + answers: readonly ('lost' | 'waiting' | 'withdrawn' | 'accepted' | 'rejected')[] +) { const ids: string[] = [] const deliveries: unknown[] = [] const sendRequest = vi.fn(async (_method, params) => { @@ -44,7 +69,9 @@ function hostAnswering(answers: readonly ('lost' | 'waiting' | 'withdrawn')[]) { if (answer === 'lost' || answer === undefined) { throw markRpcDeliveryUnknown(new Error('Connection closed')) } - return queuedAnswer(id, answer) + return answer === 'accepted' || answer === 'rejected' + ? submissionAnswer(id, answer) + : queuedAnswer(id, answer) }) const client: RpcClient = { sendRequest, @@ -154,4 +181,27 @@ describe('a resend past a saved record storage would not clear', () => { "Sent, but this phone couldn't update its record of sent messages." ) }) + + // Its row says it was not sent, so replaying the kept id would answer that again and do nothing. + it('sends the same text as a new message when its kept id answers as recorded and rejected', async () => { + const { client, ids } = hostAnswering(['lost', 'rejected', 'accepted']) + const onError = vi.fn() + expect(await sendAgain(client, onError, false)).toBe('unknown') + expect(await sendAgain(client, onError, false)).toBe('accepted') + expect(ids).toHaveLength(3) + expect(ids[1]).toBe(ids[0]) + expect(ids[2]).not.toBe(ids[0]) + expect(onError).not.toHaveBeenCalled() + }) + + it('never says a resend went out when the host recorded and rejected it', async () => { + const { client, ids } = hostAnswering(['lost', 'withdrawn', 'rejected']) + const onError = vi.fn() + expect(await sendAgain(client, onError)).toBe('unknown') + asyncStorage.setItem.mockRejectedValue(new Error('disk full')) + asyncStorage.removeItem.mockRejectedValue(new Error('disk full')) + expect(await sendAgain(client, onError)).toBe('queued') + expect(ids).toHaveLength(3) + expect(onError).not.toHaveBeenCalled() + }) }) diff --git a/mobile/src/session/mobile-structured-agent-session-send.ts b/mobile/src/session/mobile-structured-agent-session-send.ts index ce7f60a8b929..8ad04187d636 100644 --- a/mobile/src/session/mobile-structured-agent-session-send.ts +++ b/mobile/src/session/mobile-structured-agent-session-send.ts @@ -11,10 +11,12 @@ import type { RpcClient } from '../transport/rpc-client' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { requestStructuredAgentSessionMutation, - timeoutForDeadline + timeoutForDeadline, + type StructuredAgentSessionMutationCallResult } from './mobile-structured-agent-session-rpc' import { structuredSessionOperationId } from './structured-session-operation-id' import { mobileStructuredSendDelivery } from './mobile-structured-send-delivery' +import { dispatchWasWithdrawn } from '../../../src/shared/structured-agent-session-dispatch-rejection' import { bypassedMobileStructuredSendOperationId, clearMobileStructuredSendOperation, @@ -36,13 +38,13 @@ export async function sendMobileStructuredAgentSessionMessage(input: { delivery?: 'queue-if-active' deadline?: number onError: (message: string) => void - /** Internal: the one fresh-id resend after a withdrawn replay. */ - resendingAfterWithdrawal?: true - /** Internal: that resend when the withdrawn id's record could not be cleared. It bypasses the + /** Internal: the one fresh-id resend after a replay that settled without delivering. */ + resendingAfterSettledReplay?: true + /** Internal: that resend when the settled id's record could not be cleared. It bypasses the * record, so failing storage never blocks the send; its id is kept in memory for this app run, * so a retry after a lost answer replays it rather than sending again. */ bypassRetainedRecord?: true - /** Internal: on that resend, the key the withdrawn record matched. The remembered id is keyed + /** Internal: on that resend, the key the settled record matched. The remembered id is keyed * by it, so a capability change since the lost answer never mints another id. */ resendOperationKey?: string }): Promise { @@ -165,22 +167,22 @@ export async function sendMobileStructuredAgentSessionMessage(input: { result.status === 'accepted' && 'queued' in result.value && result.value.queued?.state === 'withdrawn' - if (withdrawnReplay && operation.retained && !input.resendingAfterWithdrawal) { - // The retained id's draft was withdrawn, so it never reached the agent: - // this identical message is a new one, not a replay to swallow. A record - // storage would not clear is bookkeeping: it is reported, never allowed to - // block the send. - const resent = await sendMobileStructuredAgentSessionMessage({ + if ( + operation.retained && + !input.resendingAfterSettledReplay && + // A recorded rejection's row already says it was not sent, so a replay would do nothing. + (withdrawnReplay || recordedRejection(result)) + ) { + // The retained id's message never reached the agent (its draft was withdrawn, or the host + // recorded and rejected it): this identical message is a new one, not a replay to swallow. A + // record storage would not clear is bookkeeping: it is reported, never allowed to block the + // send. + return sendMobileStructuredAgentSessionMessage({ ...input, - resendingAfterWithdrawal: true, + resendingAfterSettledReplay: true, resendOperationKey: operationKey, ...(released ? {} : { bypassRetainedRecord: true as const }) }) - // Only when the resend is known to have gone out; an unconfirmed one may not have. - if (!released && (resent === 'accepted' || resent === 'queued')) { - input.onError("Sent, but this phone couldn't update its record of sent messages.") - } - return resent } if (withdrawnReplay) { // Not resent: no card and no bubble holds the text, so it goes back to the @@ -191,6 +193,13 @@ export async function sendMobileStructuredAgentSessionMessage(input: { if (outcome.error !== null) { input.onError(outcome.error) } + // Only once the resend is known to have gone out: an unconfirmed one may not have, and one the + // host recorded and rejected shows as not sent. + const wentOut = + outcome.outcome === 'accepted' || (outcome.outcome === 'queued' && !recordedRejection(result)) + if (input.bypassRetainedRecord && wentOut) { + input.onError("Sent, but this phone couldn't update its record of sent messages.") + } return outcome.outcome } @@ -240,3 +249,14 @@ async function adoptBypassedOperation(input: { return bypassOperation(input.operationKey, input.payloadFingerprint, input.attachmentPaths) } } + +/** An answer that the host recorded the message and then rejected it, not by a Stop. */ +function recordedRejection( + result: StructuredAgentSessionMutationCallResult +): boolean { + const submission = + result.status === 'accepted' && 'submission' in result.value + ? result.value.submission + : undefined + return submission?.dispatchState === 'rejected' && !dispatchWasWithdrawn(submission) +} diff --git a/mobile/src/session/mobile-structured-composer-command.test.ts b/mobile/src/session/mobile-structured-composer-command.test.ts index 8a54140a7dc3..af5c06cbbe3d 100644 --- a/mobile/src/session/mobile-structured-composer-command.test.ts +++ b/mobile/src/session/mobile-structured-composer-command.test.ts @@ -33,6 +33,7 @@ function setup() { conversationCommands: ['clear', 'compact'] }, canRun: () => true, + shownNotSent: () => false, onError: vi.fn(), timeoutMs: 15000 } @@ -119,6 +120,31 @@ describe('mobile structured conversation commands', () => { expect(fields.command).toBe('clear') expect(asyncStorage.setItem).not.toHaveBeenCalled() }) + // Its row in the chat says it was not sent and why, for every viewer, so no banner repeats it. + it('banners a refused /compact only when the journal does not show the host recorded it', async () => { + const refused = { + ok: true, + result: { + ok: true, + value: { command: 'compact', state: 'completed', error: "This command didn't run." } + } + } + const { input, sendRequest } = setup() + sendRequest.mockResolvedValue(refused) + const sentId = () => { + const envelope = requestFields(sendRequest.mock.calls.at(-1)).envelope + return typeof envelope === 'object' && envelope !== null && 'clientOperationId' in envelope + ? envelope.clientOperationId + : undefined + } + input.shownNotSent = (clientMessageId) => clientMessageId === sentId() + expect(await dispatchMobileStructuredCommand(input)).toBe('accepted') + expect(input.onError).not.toHaveBeenCalled() + + input.shownNotSent = () => false + expect(await dispatchMobileStructuredCommand(input)).toBe('rejected') + expect(input.onError).toHaveBeenCalledWith("This command didn't run.") + }) it('keeps ordinary messages on the existing send path', async () => { const { input, sendRequest } = setup() expect(await dispatchMobileStructuredCommand({ ...input, text: 'hello' })).toBeNull() diff --git a/mobile/src/session/mobile-structured-composer-command.ts b/mobile/src/session/mobile-structured-composer-command.ts index ebe30fe88031..fd764668ac78 100644 --- a/mobile/src/session/mobile-structured-composer-command.ts +++ b/mobile/src/session/mobile-structured-composer-command.ts @@ -7,6 +7,7 @@ import { import type { RpcClient } from '../transport/rpc-client' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { requestStructuredAgentSessionMutation } from './mobile-structured-agent-session-rpc' +import { structuredSessionOperationId } from './structured-session-operation-id' export async function dispatchMobileStructuredCommand(input: { text: string @@ -17,6 +18,8 @@ export async function dispatchMobileStructuredCommand(input: { pending: { current: boolean } controller: StructuredAgentSessionComposerOptions canRun: () => boolean + /** Whether the loaded journal already shows the message recorded under this id as not sent. */ + shownNotSent: (clientMessageId: string) => boolean onError: (message: string) => void timeoutMs: number }): Promise { @@ -41,6 +44,7 @@ export async function dispatchMobileStructuredCommand(input: { } } input.pending.current = true + const clientOperationId = structuredSessionOperationId() try { const result = await requestStructuredAgentSessionMutation({ @@ -50,6 +54,7 @@ export async function dispatchMobileStructuredCommand(input: { method: 'agentSession.conversationCommand', fingerprintMethod: 'agentSession.conversationCommand', fields: { command }, + clientOperationId, timeoutMs: Math.max(input.timeoutMs, 195_000) }) if ( @@ -62,9 +67,15 @@ export async function dispatchMobileStructuredCommand(input: { error: 'Conversation operation was not confirmed.' } } - return result.status === 'accepted' - ? { accepted: !result.value.error, error: result.value.error ?? null } - : { accepted: false, error: result.message } + if (result.status !== 'accepted') { + return { accepted: false, error: result.message } + } + // The command's own row already says it was not sent, and why: no banner repeats it, and + // the text stays the row's, not the composer's. + if (result.value.state === 'completed' && input.shownNotSent(clientOperationId)) { + return { accepted: true, error: null } + } + return { accepted: !result.value.error, error: result.value.error ?? null } } finally { input.pending.current = false } diff --git a/mobile/src/session/mobile-structured-send-delivery.test.ts b/mobile/src/session/mobile-structured-send-delivery.test.ts index 8431e32a7aa9..d6b0a85a93ad 100644 --- a/mobile/src/session/mobile-structured-send-delivery.test.ts +++ b/mobile/src/session/mobile-structured-send-delivery.test.ts @@ -112,26 +112,26 @@ describe('mobileStructuredSendDelivery', () => { } }) - it('spends the id of a rejection and withholds its internal reason', () => { + it('spends the id of a recorded rejection and leaves the message to its row in the chat', () => { // Provably undelivered and terminal, so the id can only replay it: spending the - // id makes the retry a first delivery. The marker itself names nothing a person - // can act on, so it must not reach the screen. - expect( - mobileStructuredSendDelivery(accepted('rejected', 'provider_write_failed: broken pipe')) - ).toEqual({ - outcome: 'rejected', - operationIdSpent: true, - error: "Orca couldn't reach the agent. Your message was not sent. Send it again." - }) + // id makes the retry a first delivery. The host recorded the message, so the + // transcript holds it and shows it as not sent, with why: no banner, no hand-back. + for (const reason of ['provider_write_failed: broken pipe', 'Claude does not support .bmp']) { + expect(mobileStructuredSendDelivery(accepted('rejected', reason))).toEqual({ + outcome: 'queued', + operationIdSpent: true, + error: null + }) + } }) - it('shows a provider content rejection verbatim', () => { + it('says a send a Stop withdrew was not sent: no row in the chat says it', () => { expect( - mobileStructuredSendDelivery(accepted('rejected', 'Claude does not support .bmp')) + mobileStructuredSendDelivery(accepted('rejected', 'provider_cancelled_before_start')) ).toEqual({ outcome: 'rejected', operationIdSpent: true, - error: 'Claude does not support .bmp' + error: 'Your message was not sent. Send it again.' }) }) diff --git a/mobile/src/session/mobile-structured-send-delivery.ts b/mobile/src/session/mobile-structured-send-delivery.ts index f20dc0971887..62dbf0df4ef5 100644 --- a/mobile/src/session/mobile-structured-send-delivery.ts +++ b/mobile/src/session/mobile-structured-send-delivery.ts @@ -13,7 +13,8 @@ // // accepted/pending — the send happened. The id is spent; a later identical // message is a new message and must carry a new id. -// rejected — a terminal refusal or rejected submission spends a fresh id. A +// rejected — a terminal refusal or rejected submission spends a fresh id (a +// recorded one reports `queued`: the host holds its text, not the composer). A // pending-admission refusal, or any refusal after earlier transport doubt, // keeps it because neither proves a retained delivery did not happen. Two // exceptions spend it anyway, because the host can never accept the replay @@ -30,6 +31,7 @@ import type { AgentSessionSendResult } from '../../../src/shared/agent-session-w import { agentSessionRefusalOperationState } from '../../../src/shared/agent-session-refusal-retry' import { agentSessionWriteNoticeEnglish } from '../../../src/shared/agent-session-refusal-notice' import { structuredAgentSessionRejectionNotice } from '../../../src/shared/structured-agent-session-send-disposition' +import { dispatchWasWithdrawn } from '../../../src/shared/structured-agent-session-dispatch-rejection' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import type { StructuredAgentSessionMutationCallResult } from './mobile-structured-agent-session-rpc' @@ -96,13 +98,19 @@ export function mobileStructuredSendDelivery( if (!submission || submission.dispatchState === 'unknown') { return { outcome: 'unknown', operationIdSpent: false, error: null } } - if (submission.dispatchState === 'rejected') { + if (submission.dispatchState === 'rejected' && dispatchWasWithdrawn(submission)) { + // A Stop withdrew it, so no row holds it: its text goes back to the composer, with why. return { outcome: 'rejected', operationIdSpent: true, error: structuredAgentSessionRejectionNotice(submission.reason, 'composer-send') } } + if (submission.dispatchState === 'rejected') { + // The host recorded it: its row in the chat holds the text and says it was not sent and why, + // so nothing is handed back. + return { outcome: 'queued', operationIdSpent: true, error: null } + } if (retained) { // A payload match cannot distinguish retrying the ambiguous action from a // later identical intent. Wait for the stream to settle and release it. diff --git a/mobile/src/session/use-mobile-native-chat-drafts.ts b/mobile/src/session/use-mobile-native-chat-drafts.ts index 027e6a9a1d89..d0559d47ad11 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.ts @@ -11,6 +11,8 @@ import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { appendReturnedDraftText } from '../../../src/shared/returned-draft-text' import { countUserTextOccurrences, + sendBaselineTailMessageId, + sendBaselineUnsentMessageIds, findLandedImagePreviewEchoes, mergeLandedImagePreviewEchoes, migrateImagePreviewMessageIds, @@ -175,7 +177,8 @@ export function useMobileNativeChatDrafts(args: { pendingKey, normalizedText, baselineOccurrences: countUserTextOccurrences(messagesRef.current, normalizedText), - baselineTailMessageId: messagesRef.current.at(-1)?.id ?? null, + baselineTailMessageId: sendBaselineTailMessageId(messagesRef.current), + ...unsentBaseline(sendBaselineUnsentMessageIds(messagesRef.current)), // Only a settled read makes this a boundary. Anything else — hydrating, // or a read that failed — hands back an empty list that reads as "the // conversation was empty", which lets any row claim this send later. @@ -319,3 +322,7 @@ export function useMobileNativeChatDrafts(args: { holdUnconfirmedSend } } + +function unsentBaseline(ids: string[]): { baselineUnsentMessageIds?: readonly string[] } { + return ids.length > 0 ? { baselineUnsentMessageIds: ids } : {} +} diff --git a/mobile/src/session/use-mobile-native-chat-launch-draft-seed.ts b/mobile/src/session/use-mobile-native-chat-launch-draft-seed.ts index b75dbf127678..b76343c33df0 100644 --- a/mobile/src/session/use-mobile-native-chat-launch-draft-seed.ts +++ b/mobile/src/session/use-mobile-native-chat-launch-draft-seed.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, type Dispatch, type SetStateAction } from 'react' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' -import { normalizedUserText } from './mobile-native-chat-draft-reconcile' +import { normalizedNativeChatUserMessageText } from './mobile-native-chat-image-transcript-markers' export type MobileNativeChatLaunchDraftSeed = { text: string @@ -63,7 +63,7 @@ export function useMobileNativeChatLaunchDraftSeed(args: { } // A user turn already in the transcript means the TUI prefill was submitted // or deliberately cleared; decline instead of resurrecting it. - if (messages.some((message) => normalizedUserText(message) !== null)) { + if (messages.some((message) => normalizedNativeChatUserMessageText(message) !== null)) { seededLaunchDraftByKeyRef.current.set(draftKey, null) return } @@ -98,7 +98,9 @@ export function useMobileNativeChatLaunchDraftSeed(args: { if (!seeded) { return } - const hasUserTurn = messages.some((message) => normalizedUserText(message) !== null) + const hasUserTurn = messages.some( + (message) => normalizedNativeChatUserMessageText(message) !== null + ) if (!hasUserTurn && launchDraft?.trim()) { return } diff --git a/mobile/src/session/use-mobile-native-chat-unconfirmed-sends.ts b/mobile/src/session/use-mobile-native-chat-unconfirmed-sends.ts index f1b0e501fce5..6cb769666399 100644 --- a/mobile/src/session/use-mobile-native-chat-unconfirmed-sends.ts +++ b/mobile/src/session/use-mobile-native-chat-unconfirmed-sends.ts @@ -79,6 +79,9 @@ export function useMobileNativeChatUnconfirmedSends(args: { ...(origin.baselineQueuedMessageIds ? { baselineQueuedMessageIds: origin.baselineQueuedMessageIds } : {}), + ...(origin.baselineUnsentMessageIds + ? { baselineUnsentMessageIds: origin.baselineUnsentMessageIds } + : {}), deadline: null } // Why: the transcript event (or the card) can beat the lost RPC acknowledgement. diff --git a/mobile/src/session/use-mobile-structured-agent-session-send.test.tsx b/mobile/src/session/use-mobile-structured-agent-session-send.test.tsx index 086482537ef2..b8e63fb3ee0f 100644 --- a/mobile/src/session/use-mobile-structured-agent-session-send.test.tsx +++ b/mobile/src/session/use-mobile-structured-agent-session-send.test.tsx @@ -5,6 +5,7 @@ import type { AgentJournalDispatchState } from '../../../src/shared/agent-sessio import type { AgentSessionSubscribeEvent } from '../../../src/shared/agent-session-wire' import type { RpcClient } from '../transport/rpc-client' import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' import { resetMobileStructuredSendOperationJournalForTests } from './mobile-structured-send-operation-journal' import { structuredSendResultFixture } from './structured-agent-send-result.test-fixture' import { useMobileStructuredAgentSession } from './use-mobile-structured-agent-session' @@ -21,13 +22,13 @@ function ok(result: unknown) { return { ok: true, result, _meta: { runtimeId: 'runtime-1' } } } -function sendResult(dispatchState: AgentJournalDispatchState) { +function sendResult(dispatchState: AgentJournalDispatchState, reason: string | null = null) { return ok({ ok: true, replayed: false, fence: 3, cursor: { epoch: 'epoch-1', sequence: 1 }, - value: structuredSendResultFixture(dispatchState) + value: structuredSendResultFixture(dispatchState, reason) }) } @@ -257,6 +258,67 @@ describe('mobile structured send retries', () => { expect(calls().every(([, params]) => !('retryUnknown' in (params as object)))).toBe(true) }) + // The host's row is where a recorded, rejected message lives on the phone, as on the desktop. + it('shows a message the host recorded and then rejected as not sent, in place', async () => { + await mountSession() + const event = snapshotEvent() + const itemId = agentJournalSubmissionKey('rejected-1') + act(() => + listener?.({ + ...event, + page: { + ...event.page, + items: [ + { + itemId, + revision: 1, + sequence: 1, + observedAt: 10, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'lost' }] } + } + ], + submissions: [ + { + clientMessageId: 'rejected-1', + fence: 3, + payloadFingerprint: 'fingerprint', + dispatchState: 'rejected', + providerItemId: null, + reason: 'provider_write_failed: broken pipe', + submittedAt: 10, + resolvedAt: 10 + } + ] + } + }) + ) + expect(hook!.session.messages).toEqual([expect.objectContaining({ id: itemId, unsent: true })]) + }) + + // The transcript owns a message the host recorded, so the composer never gets it back. + it('hands back only a send a Stop withdrew, never one the host recorded and rejected', async () => { + let reason = 'provider_write_failed: broken pipe' + sendRequest.mockImplementation(async (method) => + method === 'agentSession.send' + ? sendResult('rejected', reason) + : method === 'agentSession.options' + ? ok({ models: [], current: {} }) + : ok({}) + ) + await mountSession() + + await act(async () => { + expect(await hook!.sendWithOutcome('recorded, then rejected')).toBe('queued') + }) + expect(onSendError).not.toHaveBeenCalled() + + reason = 'provider_cancelled_before_start' + await act(async () => { + expect(await hook!.sendWithOutcome('withdrawn by Stop')).toBe('rejected') + }) + expect(onSendError).toHaveBeenCalledWith('Your message was not sent. Send it again.') + }) + it('reuses the original uploaded attachment identity after acknowledgement loss', async () => { let attempts = 0 sendRequest.mockImplementation(async (method) => { diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 7bec053c9d67..eb261efd499d 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -160,6 +160,10 @@ export function useMobileStructuredAgentSession(args: { onSendError }) + const queuedMessageIds = useMemo( + () => (queuedMessages ?? []).map((message) => message.messageId), + [queuedMessages] + ) // What the transcript reads, as desktop does: the journal plus the one notice a cut turn with no // row gets. const transcriptItems = useMemo( @@ -170,12 +174,13 @@ export function useMobileStructuredAgentSession(args: { [agent, state.items] ) const messages = useMemo( - // Off: the phone hands a rejected message back to its composer, so a row would show it twice. + // As on the desktop: a message the host recorded lives on its row, never back in the composer. () => projectStructuredAgentSessionMessages(transcriptItems, [], state.submissions, { - rejectedInPlace: false + rejectedInPlace: true, + queuedMessageIds }), - [transcriptItems, state.submissions] + [transcriptItems, state.submissions, queuedMessageIds] ) const turnId = activeStructuredAgentSessionTurnId(state.items) const turnTiming = useMobileStructuredAgentTurnTiming( diff --git a/mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts b/mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts index 5abe42b39e60..f8b9d7f64b22 100644 --- a/mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts +++ b/mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts @@ -77,8 +77,8 @@ export function useMobileStructuredNativeChatSendBridge(args: { return 'accepted' } if (outcome === 'queued') { - // The host holds the draft and publishes it as a card above the - // composer — never an optimistic transcript bubble. + // The host holds the text, as a card above the composer or a recorded + // row in the transcript — never an optimistic bubble or a hand-back. return 'queued' } if (outcome === 'unknown') { diff --git a/mobile/src/session/use-mobile-structured-send-with-outcome.ts b/mobile/src/session/use-mobile-structured-send-with-outcome.ts index 613dd14fe084..cfb21d10fc77 100644 --- a/mobile/src/session/use-mobile-structured-send-with-outcome.ts +++ b/mobile/src/session/use-mobile-structured-send-with-outcome.ts @@ -11,6 +11,7 @@ import { type StructuredAgentSessionAttachment } from '../../../src/shared/structured-agent-session-outbox' import type { StructuredAgentSessionComposerOptions } from '../../../src/shared/structured-agent-session-composer' +import { structuredAgentSessionJournalShowsRejection } from '../../../src/shared/structured-agent-session-message-projection' import type { StructuredAgentSessionState } from '../../../src/shared/structured-agent-session-reducer' import type { RpcClient } from '../transport/rpc-client' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' @@ -99,6 +100,11 @@ export function useMobileStructuredSendWithOutcome(args: { !stateRef.current.items.some( (item) => pendingStructuredApproval(item) || pendingStructuredQuestion(item) ), + shownNotSent: (clientMessageId) => + structuredAgentSessionJournalShowsRejection( + stateRef.current.submissions, + clientMessageId + ), onError: onSendError, timeoutMs }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts index e51b8e672f96..7daf618fa991 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts @@ -31,6 +31,7 @@ import type { AgentSessionConversationCommand } from '../../../shared/agent-sess import type { AgentSessionRecord } from '../../../shared/agent-session-record' import type { AgentChildWorkView } from '../../../shared/agent-status-child-work-view' import { agentSessionRefusalReference } from '../../../shared/agent-session-wire-refusals' +import { structuredAgentSessionCommandResultRowIdentity } from '../../../shared/structured-agent-session-command-entry' import { agentJournalTurnBody, readAgentJournalTurn @@ -103,7 +104,7 @@ export function structuredAgentSessionCommandTurn(clientMessageId: string): { identity, itemId: agentJournalItemKey(identity), turnId: `compact:${clientMessageId}`, - resultIdentity: { provider: 'orca', clientMessageId: `command-result:${clientMessageId}` } + resultIdentity: structuredAgentSessionCommandResultRowIdentity(clientMessageId) } } diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.rejected-command.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.rejected-command.test.tsx new file mode 100644 index 000000000000..36b6169e8e96 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.rejected-command.test.tsx @@ -0,0 +1,210 @@ +// @vitest-environment happy-dom + +// A /compact the host recorded and then rejected stays in the chat as not sent, for every viewer, +// and its failure is said once: by its row's line, or, where a host row already says why, by that +// row alone. The host moves a rejected message to its rejection, in no turn, while a command turn +// that opened keeps naming it as its opener. + +import '@testing-library/jest-dom/vitest' + +import { cleanup, render } from '@testing-library/react' +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import type { AgentSessionFailureFact } from '../../../../shared/agent-session-failure' +import { agentSessionFailureWords } from '../../../../shared/agent-session-failure-words' +import { + agentJournalItemKey, + agentJournalSubmissionKey +} from '../../../../shared/agent-session-journal-item-key' +import type { + AgentJournalRenderItem, + AgentJournalSubmission +} from '../../../../shared/agent-session-journal-types' +import { + structuredAgentSessionCommandResultRowIdentity, + structuredAgentSessionCommandResultRows +} from '../../../../shared/structured-agent-session-command-entry' +import { structuredAgentSessionStartFailureFacts } from '../../../../shared/structured-agent-session-recorded-rejection-words' +import { structuredAgentSessionStartFailureRowIdentity } from '../../../../shared/structured-agent-session-start-failure-row-key' +import { NativeChatMessageList } from './NativeChatMessageList' +import { installNativeChatMessageListTestViewport } from './native-chat-message-list-test-viewport' +import { structuredAgentSessionDeliveryNotices } from './structured-agent-session-delivery-notices' +import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' + +let restoreViewport = (): void => {} +beforeAll(() => { + restoreViewport = installNativeChatMessageListTestViewport() +}) +afterAll(() => restoreViewport()) +afterEach(cleanup) + +const SEED = agentJournalSubmissionKey('seed') +const COMPACT = agentJournalSubmissionKey('compact') +const TURN = agentJournalItemKey({ provider: 'orca', clientMessageId: 'command-turn:compact' }) +const BUSY = { text: 'thread busy', audience: 'person' as const } +const START_FAILED: AgentSessionFailureFact = { kind: 'providerStartFailed' } + +function row( + itemId: string, + sequence: number, + body: AgentJournalRenderItem['body'], + turnScope: AgentJournalRenderItem['turnScope'] = { kind: 'thread' } +): AgentJournalRenderItem { + return { itemId, revision: 1, sequence, observedAt: 1000 + sequence, body, turnScope } +} + +function hostRow(itemId: string, sequence: number, fact: AgentSessionFailureFact, turn = false) { + return row( + itemId, + sequence, + { + kind: 'status', + tone: 'error', + ...agentSessionFailureWords(fact, { agentName: 'Codex', surface: 'row' }) + }, + turn ? { kind: 'turn', turnItemId: TURN } : { kind: 'thread' } + ) +} + +function submission( + id: string, + dispatchState: AgentJournalSubmission['dispatchState'], + rejection?: AgentSessionFailureFact +): AgentJournalSubmission { + return { + clientMessageId: id, + fence: 1, + payloadFingerprint: id, + dispatchState, + providerItemId: null, + reason: null, + ...(rejection ? { rejection } : {}), + submittedAt: 1, + resolvedAt: 1 + } +} + +type Case = 'blocked' | 'refused' | 'startFailed' + +function journal(kind: Case): { + items: AgentJournalRenderItem[] + submissions: AgentJournalSubmission[] +} { + const rejection: AgentSessionFailureFact = + kind === 'blocked' + ? { kind: 'commandRefused' } + : kind === 'refused' + ? { kind: 'providerRejected', detail: BUSY } + : START_FAILED + const compact = row(COMPACT, 5, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: '/compact' }], + command: { name: 'compact' } + }) + const items: AgentJournalRenderItem[] = [ + row(SEED, 1, { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'SEED' }] }), + row( + 'seed-answer', + 2, + { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: 'SEED OK' }] }, + { kind: 'thread' } + ) + ] + if (kind === 'refused') { + // The turn the command opened, its rejection (which moved the message there) and its result. + items.push( + row(TURN, 3, { + kind: 'turn', + turnId: 'compact:compact', + state: 'completed', + outcome: 'failure', + userItemId: COMPACT + }), + compact, + hostRow( + agentJournalItemKey(structuredAgentSessionCommandResultRowIdentity('compact')), + 6, + { kind: 'compactionFailed', detail: BUSY }, + true + ) + ) + } else if (kind === 'startFailed') { + items.push( + compact, + hostRow( + agentJournalItemKey(structuredAgentSessionStartFailureRowIdentity('g')), + 6, + START_FAILED + ) + ) + } else { + items.push(compact) + } + return { + items, + submissions: [submission('seed', 'accepted'), submission('compact', 'rejected', rejection)] + } +} + +function list(kind: Case) { + const { items, submissions } = journal(kind) + return ( + + ) +} + +function count(container: HTMLElement, text: string): number { + return (container.textContent ?? '').split(text).length - 1 +} + +describe('a /compact the host recorded and then rejected, on the desktop', () => { + it('shows a blocked one with its own line, the only place it is said', () => { + const { container } = render(list('blocked')) + expect(container).toHaveTextContent('/compact') + expect(count(container, "This command didn't run.")).toBe(1) + expect(container.querySelector('button[aria-label="Retry"]')).toBeNull() + }) + + it("leaves why to a refused one's result row: its own line says only that it was not sent", () => { + const { container } = render(list('refused')) + const text = container.textContent ?? '' + expect(text.indexOf('/compact')).toBeLessThan(text.indexOf('Your message was not sent.')) + expect(text.indexOf('Your message was not sent.')).toBeLessThan(text.indexOf('thread busy')) + expect(count(container, 'Your message was not sent.')).toBe(1) + expect(count(container, 'thread busy')).toBe(1) + }) + + it("leaves why to a failed start's row", () => { + const { container } = render(list('startFailed')) + expect(count(container, 'Your message was not sent.')).toBe(1) + expect(count(container, 'stopped before it finished starting')).toBe(1) + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRow.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRow.test.tsx index f309593d898f..b4b522697914 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRow.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRow.test.tsx @@ -160,6 +160,20 @@ describe('what a user message says about its delivery', () => { expect(screen.queryByRole('button', { name: 'Retry' })).toBeNull() }) + // A plain "not sent" is a label, not an error; a doubt to check keeps the error color. + it('reads muted when it says only that the message was not sent', () => { + renderUser({ text: 'Your message was not sent.', notSent: true }) + const notSent = screen.getByText('Your message was not sent.').parentElement + expect(notSent).toHaveClass('text-muted-foreground') + expect(notSent).not.toHaveClass('text-destructive/80') + cleanup() + + renderUser({ text: 'Message delivery is unconfirmed.' }) + expect(screen.getByText('Message delivery is unconfirmed.').parentElement).toHaveClass( + 'text-destructive/80' + ) + }) + it('says nothing when it went through', () => { renderUser() expect(screen.queryByRole('button', { name: 'Retry' })).toBeNull() diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx index 529b0c024c8f..f5896707fc68 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRow.tsx @@ -31,8 +31,15 @@ import type { RuntimeFileOperationArgs } from '@/runtime/runtime-file-client' * place of its time, or that it did not go through, with its own Retry when the surface can send * it again. */ export type NativeChatDeliveryNotice = - | { sending: true; text?: never; onRetry?: never; onDismiss?: never } - | { sending?: never; text: string; onRetry?: () => void; onDismiss?: () => void } + | { sending: true; text?: never; notSent?: never; onRetry?: never; onDismiss?: never } + | { + sending?: never + text: string + /** Says only that the message did not go out, so it reads muted, not as an error. */ + notSent?: true + onRetry?: () => void + onDismiss?: () => void + } const USER_META_REVEAL = 'transition-opacity can-hover:pointer-events-none can-hover:opacity-0 group-hover:pointer-events-auto group-hover:opacity-100 [.group:has(:focus-visible)_&]:pointer-events-auto [.group:has(:focus-visible)_&]:opacity-100' @@ -215,7 +222,12 @@ export const MessageRow = memo(function MessageRow({ sending={deliveryNotice?.sending === true} /> {deliveryNotice?.text !== undefined ? ( -
+
{deliveryNotice.text} {deliveryNotice.onDismiss ? (