| author | |
| committer | |
| log | a1d9d72aefc2a388ea3915956e9d84d279c87da7 |
| tree | a463619c83247d2a43ec49815a99216ae35605d8 |
| parent | de09c7566c71a9c01a554b2f163fa50e3e15bfe5 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: T3 Code Test <t3code-test@example.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>8 files changed, 306 insertions(+), 15 deletions(-)
apps/mobile/src/connection/platform.ts+2| ... | @@ -27,6 +27,7 @@ import * as MobileStorage from "../persistence/mobile-storage"; | ... | @@ -27,6 +27,7 @@ import * as MobileStorage from "../persistence/mobile-storage"; |
| 27 | import { appAtomRegistry } from "../state/atom-registry"; | 27 | import { appAtomRegistry } from "../state/atom-registry"; |
| 28 | import { clearThreadOutboxEnvironment } from "../state/thread-outbox-removal"; | 28 | import { clearThreadOutboxEnvironment } from "../state/thread-outbox-removal"; |
| 29 | import { clearComposerDraftsEnvironment } from "../state/use-composer-drafts"; | 29 | import { clearComposerDraftsEnvironment } from "../state/use-composer-drafts"; |
| 30 | import { clearThreadComposerErrorsForEnvironment } from "../state/thread-composer-error"; | ||
| 30 | import { mobileApplicationActiveWakeup } from "./app-state-wakeups"; | 31 | import { mobileApplicationActiveWakeup } from "./app-state-wakeups"; |
| 31 | import { connectionStorageLayer } from "./storage"; | 32 | import { connectionStorageLayer } from "./storage"; |
| 32 | 33 | ||
| ... | @@ -264,6 +265,7 @@ const environmentOwnedDataCleanupLayer = Layer.succeed( | ... | @@ -264,6 +265,7 @@ const environmentOwnedDataCleanupLayer = Layer.succeed( |
| 264 | [ | 265 | [ |
| 265 | Effect.promise(() => clearThreadOutboxEnvironment(environmentId)), | 266 | Effect.promise(() => clearThreadOutboxEnvironment(environmentId)), |
| 266 | Effect.promise(() => clearComposerDraftsEnvironment(environmentId)), | 267 | Effect.promise(() => clearComposerDraftsEnvironment(environmentId)), |
| 268 | Effect.sync(() => clearThreadComposerErrorsForEnvironment(environmentId)), | ||
| 267 | ], | 269 | ], |
| 268 | { concurrency: "unbounded", discard: true }, | 270 | { concurrency: "unbounded", discard: true }, |
| 269 | ).pipe( | 271 | ).pipe( |
apps/mobile/src/features/threads/ComposerErrorNotice.tsx created+55| ... | @@ -0,0 +1,55 @@ | ||
| 1 | import { useEffect } from "react"; | ||
| 2 | import { AccessibilityInfo, Platform, Pressable, View } from "react-native"; | ||
| 3 | |||
| 4 | import { AppText as Text } from "../../components/AppText"; | ||
| 5 | import { SymbolView } from "../../components/AppSymbol"; | ||
| 6 | |||
| 7 | /** Why the thread's last message did not send, above the composer until dismissed. */ | ||
| 8 | export function ComposerErrorNotice({ | ||
| 9 | message, | ||
| 10 | onDismiss, | ||
| 11 | }: { | ||
| 12 | readonly message: string; | ||
| 13 | readonly onDismiss: () => void; | ||
| 14 | }) { | ||
| 15 | // accessibilityLiveRegion below only reaches TalkBack; VoiceOver needs an | ||
| 16 | // explicit announcement. | ||
| 17 | useEffect(() => { | ||
| 18 | if (Platform.OS === "ios") { | ||
| 19 | AccessibilityInfo.announceForAccessibility(message); | ||
| 20 | } | ||
| 21 | }, [message]); | ||
| 22 | return ( | ||
| 23 | <View className="px-4 pb-3"> | ||
| 24 | <View className="flex-row items-start gap-3 rounded-[20px] border-continuous bg-card p-4"> | ||
| 25 | <SymbolView | ||
| 26 | name="exclamationmark.circle" | ||
| 27 | size={16} | ||
| 28 | tintColorClassName="accent-danger-foreground" | ||
| 29 | type="monochrome" | ||
| 30 | /> | ||
| 31 | <Text | ||
| 32 | selectable | ||
| 33 | accessibilityLiveRegion="polite" | ||
| 34 | className="min-w-0 flex-1 text-sm text-foreground" | ||
| 35 | > | ||
| 36 | {message} | ||
| 37 | </Text> | ||
| 38 | <Pressable | ||
| 39 | accessibilityLabel="Dismiss error" | ||
| 40 | accessibilityRole="button" | ||
| 41 | hitSlop={12} | ||
| 42 | onPress={onDismiss} | ||
| 43 | className="p-1 active:opacity-60" | ||
| 44 | > | ||
| 45 | <SymbolView | ||
| 46 | name="xmark" | ||
| 47 | size={14} | ||
| 48 | tintColorClassName="accent-icon-muted" | ||
| 49 | type="monochrome" | ||
| 50 | /> | ||
| 51 | </Pressable> | ||
| 52 | </View> | ||
| 53 | </View> | ||
| 54 | ); | ||
| 55 | } | ||
apps/mobile/src/features/threads/ThreadDetailScreen.tsx+19| ... | @@ -1,3 +1,4 @@ | ... | @@ -1,3 +1,4 @@ |
| 1 | import { useAtomValue } from "@effect/atom-react"; | ||
| 1 | import { useThreadReportedModelSelection } from "../../state/entities"; | 2 | import { useThreadReportedModelSelection } from "../../state/entities"; |
| 2 | import { UsageLimitRecoveryCard } from "./UsageLimitRecoveryCard"; | 3 | import { UsageLimitRecoveryCard } from "./UsageLimitRecoveryCard"; |
| 3 | import { useNavigation } from "@react-navigation/native"; | 4 | import { useNavigation } from "@react-navigation/native"; |
| ... | @@ -93,6 +94,10 @@ import { useEnvironmentQuery } from "../../state/query"; | ... | @@ -93,6 +94,10 @@ import { useEnvironmentQuery } from "../../state/query"; |
| 93 | import { threadDevicePreviews } from "../devices/threadDevicePreviews"; | 94 | import { threadDevicePreviews } from "../devices/threadDevicePreviews"; |
| 94 | import type { QueuedThreadMessage } from "../../state/thread-outbox-model"; | 95 | import type { QueuedThreadMessage } from "../../state/thread-outbox-model"; |
| 95 | import { scopedThreadKey } from "../../lib/scopedEntities"; | 96 | import { scopedThreadKey } from "../../lib/scopedEntities"; |
| 97 | import { | ||
| 98 | clearThreadComposerError, | ||
| 99 | threadComposerErrorsAtom, | ||
| 100 | } from "../../state/thread-composer-error"; | ||
| 96 | import { threadEnvironment } from "../../state/threads"; | 101 | import { threadEnvironment } from "../../state/threads"; |
| 97 | import { useAtomCommand } from "../../state/use-atom-command"; | 102 | import { useAtomCommand } from "../../state/use-atom-command"; |
| 98 | import { useDelayedStatus } from "../../lib/useDelayedStatus"; | 103 | import { useDelayedStatus } from "../../lib/useDelayedStatus"; |
| ... | @@ -104,6 +109,7 @@ import type { | ... | @@ -104,6 +109,7 @@ import type { |
| 104 | ThreadFeedLatestRun, | 109 | ThreadFeedLatestRun, |
| 105 | } from "../../lib/threadActivity"; | 110 | } from "../../lib/threadActivity"; |
| 106 | import { PendingApprovalCard } from "./PendingApprovalCard"; | 111 | import { PendingApprovalCard } from "./PendingApprovalCard"; |
| 112 | import { ComposerErrorNotice } from "./ComposerErrorNotice"; | ||
| 107 | import { ComposerFeedback } from "./ComposerFeedback"; | 113 | import { ComposerFeedback } from "./ComposerFeedback"; |
| 108 | import { ComposerUsageLimits } from "./ComposerUsageLimits"; | 114 | import { ComposerUsageLimits } from "./ComposerUsageLimits"; |
| 109 | import { PendingUserInputCard } from "./PendingUserInputCard"; | 115 | import { PendingUserInputCard } from "./PendingUserInputCard"; |
| ... | @@ -362,6 +368,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | ... | @@ -362,6 +368,7 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread |
| 362 | const navigationHeaderHeight = useContext(HeaderHeightContext) || insets.top + 44; | 368 | const navigationHeaderHeight = useContext(HeaderHeightContext) || insets.top + 44; |
| 363 | const agentLabel = `${props.selectedThread.modelSelection.instanceId} agent`; | 369 | const agentLabel = `${props.selectedThread.modelSelection.instanceId} agent`; |
| 364 | const selectedThreadKey = scopedThreadKey(props.environmentId, props.selectedThread.id); | 370 | const selectedThreadKey = scopedThreadKey(props.environmentId, props.selectedThread.id); |
| 371 | const composerError = useAtomValue(threadComposerErrorsAtom)[selectedThreadKey]?.message ?? null; | ||
| 365 | const queuedCount = useThreadQueuedCount({ | 372 | const queuedCount = useThreadQueuedCount({ |
| 366 | environmentId: props.environmentId, | 373 | environmentId: props.environmentId, |
| 367 | threadId: props.selectedThread.id, | 374 | threadId: props.selectedThread.id, |
| ... | @@ -1190,6 +1197,18 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread | ... | @@ -1190,6 +1197,18 @@ export const ThreadDetailScreen = memo(function ThreadDetailScreen(props: Thread |
| 1190 | onDismiss={() => props.onDismissFeedback(submission.id)} | 1197 | onDismiss={() => props.onDismissFeedback(submission.id)} |
| 1191 | /> | 1198 | /> |
| 1192 | ))} | 1199 | ))} |
| 1200 | {composerError !== null ? ( | ||
| 1201 | <Animated.View | ||
| 1202 | className="shrink-0" | ||
| 1203 | entering={FadeInDown.duration(180)} | ||
| 1204 | exiting={FadeOut.duration(120)} | ||
| 1205 | > | ||
| 1206 | <ComposerErrorNotice | ||
| 1207 | message={composerError} | ||
| 1208 | onDismiss={() => clearThreadComposerError(selectedThreadKey)} | ||
| 1209 | /> | ||
| 1210 | </Animated.View> | ||
| 1211 | ) : null} | ||
| 1193 | {usageLimitsReport && activeUserInputRequestId === null ? ( | 1212 | {usageLimitsReport && activeUserInputRequestId === null ? ( |
| 1194 | <Animated.View | 1213 | <Animated.View |
| 1195 | className="shrink-0 px-4 pb-3" | 1214 | className="shrink-0 px-4 pb-3" |
apps/mobile/src/state/thread-composer-error.test.ts created+39| ... | @@ -0,0 +1,39 @@ | ||
| 1 | import { afterEach, describe, expect, it } from "vite-plus/test"; | ||
| 2 | |||
| 3 | import { appAtomRegistry } from "./atom-registry"; | ||
| 4 | import { | ||
| 5 | clearThreadComposerError, | ||
| 6 | clearThreadComposerErrorsForEnvironment, | ||
| 7 | setThreadComposerError, | ||
| 8 | threadComposerErrorsAtom, | ||
| 9 | } from "./thread-composer-error"; | ||
| 10 | |||
| 11 | afterEach(() => { | ||
| 12 | appAtomRegistry.set(threadComposerErrorsAtom, {}); | ||
| 13 | }); | ||
| 14 | |||
| 15 | describe("thread composer errors", () => { | ||
| 16 | it("clears a message-scoped error only for that message", () => { | ||
| 17 | setThreadComposerError("environment-1:thread-1", "rejected", "message-1"); | ||
| 18 | |||
| 19 | clearThreadComposerError("environment-1:thread-1", "message-2"); | ||
| 20 | expect(appAtomRegistry.get(threadComposerErrorsAtom)["environment-1:thread-1"]?.message).toBe( | ||
| 21 | "rejected", | ||
| 22 | ); | ||
| 23 | |||
| 24 | clearThreadComposerError("environment-1:thread-1"); | ||
| 25 | expect(appAtomRegistry.get(threadComposerErrorsAtom)).toEqual({}); | ||
| 26 | }); | ||
| 27 | |||
| 28 | it("clears every thread's error for a removed environment and no others", () => { | ||
| 29 | setThreadComposerError("environment-1:thread-1", "one"); | ||
| 30 | setThreadComposerError("environment-1:thread-2", "two"); | ||
| 31 | setThreadComposerError("environment-10:thread-1", "other environment"); | ||
| 32 | |||
| 33 | clearThreadComposerErrorsForEnvironment("environment-1"); | ||
| 34 | |||
| 35 | expect(Object.keys(appAtomRegistry.get(threadComposerErrorsAtom))).toEqual([ | ||
| 36 | "environment-10:thread-1", | ||
| 37 | ]); | ||
| 38 | }); | ||
| 39 | }); | ||
apps/mobile/src/state/thread-composer-error.ts created+56| ... | @@ -0,0 +1,56 @@ | ||
| 1 | import { Atom } from "effect/unstable/reactivity"; | ||
| 2 | |||
| 3 | import { appAtomRegistry } from "./atom-registry"; | ||
| 4 | |||
| 5 | interface ThreadComposerError { | ||
| 6 | readonly message: string; | ||
| 7 | /** The queued message this error is about, when it is about one. */ | ||
| 8 | readonly messageId: string | null; | ||
| 9 | } | ||
| 10 | |||
| 11 | /** | ||
| 12 | * Why a thread's last message did not go out, shown above that thread's | ||
| 13 | * composer. The outbox drain can reject a message after the user has left the | ||
| 14 | * thread, so the reason is kept per thread until they dismiss it, send again, | ||
| 15 | * or the message it describes is delivered after all. Keyed by `scopedThreadKey`. | ||
| 16 | */ | ||
| 17 | export const threadComposerErrorsAtom = Atom.make<Readonly<Record<string, ThreadComposerError>>>( | ||
| 18 | {}, | ||
| 19 | ).pipe(Atom.keepAlive, Atom.withLabel("mobile:thread-composer-errors")); | ||
| 20 | |||
| 21 | export function setThreadComposerError( | ||
| 22 | threadKey: string, | ||
| 23 | message: string, | ||
| 24 | messageId: string | null = null, | ||
| 25 | ): void { | ||
| 26 | appAtomRegistry.set(threadComposerErrorsAtom, { | ||
| 27 | ...appAtomRegistry.get(threadComposerErrorsAtom), | ||
| 28 | [threadKey]: { message, messageId }, | ||
| 29 | }); | ||
| 30 | } | ||
| 31 | |||
| 32 | /** With `messageId`, clears only an error that describes that message. */ | ||
| 33 | export function clearThreadComposerError(threadKey: string, messageId?: string): void { | ||
| 34 | const current = appAtomRegistry.get(threadComposerErrorsAtom); | ||
| 35 | const error = current[threadKey]; | ||
| 36 | if (!error || (messageId !== undefined && error.messageId !== messageId)) { | ||
| 37 | return; | ||
| 38 | } | ||
| 39 | const next = { ...current }; | ||
| 40 | delete next[threadKey]; | ||
| 41 | appAtomRegistry.set(threadComposerErrorsAtom, next); | ||
| 42 | } | ||
| 43 | |||
| 44 | export function clearThreadComposerErrorsForEnvironment(environmentId: string): void { | ||
| 45 | const current = appAtomRegistry.get(threadComposerErrorsAtom); | ||
| 46 | const prefix = `${environmentId}:`; | ||
| 47 | const keys = Object.keys(current).filter((key) => key.startsWith(prefix)); | ||
| 48 | if (keys.length === 0) { | ||
| 49 | return; | ||
| 50 | } | ||
| 51 | const next = { ...current }; | ||
| 52 | for (const key of keys) { | ||
| 53 | delete next[key]; | ||
| 54 | } | ||
| 55 | appAtomRegistry.set(threadComposerErrorsAtom, next); | ||
| 56 | } | ||
apps/mobile/src/state/use-thread-composer-state.ts+7-2| ... | @@ -96,6 +96,7 @@ import { | ... | @@ -96,6 +96,7 @@ import { |
| 96 | useQueuedRunEdit, | 96 | useQueuedRunEdit, |
| 97 | } from "./queued-run-edit"; | 97 | } from "./queued-run-edit"; |
| 98 | import { setPendingConnectionError } from "../state/use-remote-environment-registry"; | 98 | import { setPendingConnectionError } from "../state/use-remote-environment-registry"; |
| 99 | import { clearThreadComposerError, setThreadComposerError } from "./thread-composer-error"; | ||
| 99 | import { | 100 | import { |
| 100 | useSelectedThreadProjection, | 101 | useSelectedThreadProjection, |
| 101 | useSelectedThreadVisibleTurnItems, | 102 | useSelectedThreadVisibleTurnItems, |
| ... | @@ -452,7 +453,8 @@ export function useThreadComposerState() { | ... | @@ -452,7 +453,8 @@ export function useThreadComposerState() { |
| 452 | }); | 453 | }); |
| 453 | } | 454 | } |
| 454 | endQueuedRunEdit(selectedThreadKey, { deferAttachmentCleanup: keepable }); | 455 | endQueuedRunEdit(selectedThreadKey, { deferAttachmentCleanup: keepable }); |
| 455 | setPendingConnectionError( | 456 | setThreadComposerError( |
| 457 | selectedThreadKey, | ||
| 456 | keepable | 458 | keepable |
| 457 | ? "That message already started. Your edit is back in the composer." | 459 | ? "That message already started. Your edit is back in the composer." |
| 458 | : "That message already started, so the edit was discarded.", | 460 | : "That message already started, so the edit was discarded.", |
| ... | @@ -670,6 +672,8 @@ export function useThreadComposerState() { | ... | @@ -670,6 +672,8 @@ export function useThreadComposerState() { |
| 670 | 672 | ||
| 671 | const metadata = makeQueuedMessageMetadata(); | 673 | const metadata = makeQueuedMessageMetadata(); |
| 672 | const messageId = MessageId.make(metadata.messageId); | 674 | const messageId = MessageId.make(metadata.messageId); |
| 675 | // A new send supersedes the reason the previous one bounced back. | ||
| 676 | clearThreadComposerError(threadKey); | ||
| 673 | // Enqueue publishes the queued atom synchronously (the durable write | 677 | // Enqueue publishes the queued atom synchronously (the durable write |
| 674 | // happens behind it), so clearing the draft here gives send feedback on | 678 | // happens behind it), so clearing the draft here gives send feedback on |
| 675 | // the tap frame instead of after file I/O. If the write fails the message | 679 | // the tap frame instead of after file I/O. If the write fails the message |
| ... | @@ -706,7 +710,8 @@ export function useThreadComposerState() { | ... | @@ -706,7 +710,8 @@ export function useThreadComposerState() { |
| 706 | attachments: [], | 710 | attachments: [], |
| 707 | }); | 711 | }); |
| 708 | appendComposerDraftAttachments(threadKey, attachments, { allowOverflow: true }); | 712 | appendComposerDraftAttachments(threadKey, attachments, { allowOverflow: true }); |
| 709 | setPendingConnectionError( | 713 | setThreadComposerError( |
| 714 | threadKey, | ||
| 710 | error instanceof Error ? error.message : "Failed to save the queued message.", | 715 | error instanceof Error ? error.message : "Failed to save the queued message.", |
| 711 | ); | 716 | ); |
| 712 | }, | 717 | }, |
apps/mobile/src/state/use-thread-outbox-drain.test.ts+99-5| ... | @@ -19,7 +19,6 @@ const harness = vi.hoisted(() => ({ | ... | @@ -19,7 +19,6 @@ const harness = vi.hoisted(() => ({ |
| 19 | removePersistedFile: vi.fn(async () => undefined), | 19 | removePersistedFile: vi.fn(async () => undefined), |
| 20 | removeOutboxMessage: vi.fn(async (_message: QueuedThreadMessage) => undefined), | 20 | removeOutboxMessage: vi.fn(async (_message: QueuedThreadMessage) => undefined), |
| 21 | prepareTurnAttachments: vi.fn<typeof import("../lib/attachmentUpload").prepareTurnAttachments>(), | 21 | prepareTurnAttachments: vi.fn<typeof import("../lib/attachmentUpload").prepareTurnAttachments>(), |
| 22 | setPendingConnectionError: vi.fn(), | ||
| 23 | draftFile: (() => { | 22 | draftFile: (() => { |
| 24 | let document = ""; | 23 | let document = ""; |
| 25 | let writeError: Error | null = null; | 24 | let writeError: Error | null = null; |
| ... | @@ -106,7 +105,6 @@ vi.mock("./use-thread-outbox", async () => { | ... | @@ -106,7 +105,6 @@ vi.mock("./use-thread-outbox", async () => { |
| 106 | }); | 105 | }); |
| 107 | 106 | ||
| 108 | vi.mock("./use-remote-environment-registry", () => ({ | 107 | vi.mock("./use-remote-environment-registry", () => ({ |
| 109 | setPendingConnectionError: harness.setPendingConnectionError, | ||
| 110 | useRemoteConnectionStatus: () => ({ connectedEnvironments: [] }), | 108 | useRemoteConnectionStatus: () => ({ connectedEnvironments: [] }), |
| 111 | })); | 109 | })); |
| 112 | 110 | ||
| ... | @@ -137,6 +135,11 @@ import { | ... | @@ -137,6 +135,11 @@ import { |
| 137 | clearPendingThreadCreationOutcome, | 135 | clearPendingThreadCreationOutcome, |
| 138 | pendingThreadCreationOutcomesAtom, | 136 | pendingThreadCreationOutcomesAtom, |
| 139 | } from "./pending-thread-creation"; | 137 | } from "./pending-thread-creation"; |
| 138 | import { | ||
| 139 | clearThreadComposerError, | ||
| 140 | setThreadComposerError, | ||
| 141 | threadComposerErrorsAtom, | ||
| 142 | } from "./thread-composer-error"; | ||
| 140 | import type { QueuedThreadMessage } from "./thread-outbox-model"; | 143 | import type { QueuedThreadMessage } from "./thread-outbox-model"; |
| 141 | import * as composerDrafts from "./use-composer-drafts"; | 144 | import * as composerDrafts from "./use-composer-drafts"; |
| 142 | import { recoverFailedThreadDraft } from "./recover-failed-thread-draft"; | 145 | import { recoverFailedThreadDraft } from "./recover-failed-thread-draft"; |
| ... | @@ -209,11 +212,11 @@ afterEach(() => { | ... | @@ -209,11 +212,11 @@ afterEach(() => { |
| 209 | appAtomRegistry.set(composerDrafts.composerCloudDraftsAtom, { accountId: null, signedOut: {} }); | 212 | appAtomRegistry.set(composerDrafts.composerCloudDraftsAtom, { accountId: null, signedOut: {} }); |
| 210 | appAtomRegistry.set(editingQueuedMessageIdsAtom, {}); | 213 | appAtomRegistry.set(editingQueuedMessageIdsAtom, {}); |
| 211 | appAtomRegistry.set(pendingThreadCreationOutcomesAtom, {}); | 214 | appAtomRegistry.set(pendingThreadCreationOutcomesAtom, {}); |
| 215 | appAtomRegistry.set(threadComposerErrorsAtom, {}); | ||
| 212 | harness.draftFile.setWriteError(null); | 216 | harness.draftFile.setWriteError(null); |
| 213 | harness.removePersistedFile.mockClear(); | 217 | harness.removePersistedFile.mockClear(); |
| 214 | harness.removeOutboxMessage.mockClear(); | 218 | harness.removeOutboxMessage.mockClear(); |
| 215 | harness.prepareTurnAttachments.mockReset(); | 219 | harness.prepareTurnAttachments.mockReset(); |
| 216 | harness.setPendingConnectionError.mockClear(); | ||
| 217 | }); | 220 | }); |
| 218 | 221 | ||
| 219 | describe("thread outbox attachment preparation", () => { | 222 | describe("thread outbox attachment preparation", () => { |
| ... | @@ -442,6 +445,25 @@ describe("thread outbox drain delivery cleanup", () => { | ... | @@ -442,6 +445,25 @@ describe("thread outbox drain delivery cleanup", () => { |
| 442 | expect(appAtomRegistry.get(acknowledgedThreadMessagesAtom)).toEqual([message]); | 445 | expect(appAtomRegistry.get(acknowledgedThreadMessagesAtom)).toEqual([message]); |
| 443 | }); | 446 | }); |
| 444 | 447 | ||
| 448 | it("clears an error about the delivered message but keeps one about another message", async () => { | ||
| 449 | const threadKey = "environment-1:thread-1"; | ||
| 450 | const retried = queuedMessage({ messageId: "message-retried", text: "retried" }); | ||
| 451 | await harness.manager.enqueue(retried); | ||
| 452 | // A failed recovery left this message queued with an error about it. | ||
| 453 | setThreadComposerError(threadKey, "could not be restored", retried.messageId); | ||
| 454 | |||
| 455 | await completeQueuedMessageDelivery(retried, harness.manager.revisionOf(retried.messageId)); | ||
| 456 | expect(appAtomRegistry.get(threadComposerErrorsAtom)[threadKey]).toBeUndefined(); | ||
| 457 | |||
| 458 | const other = queuedMessage({ messageId: "message-other", text: "other" }); | ||
| 459 | await harness.manager.enqueue(other); | ||
| 460 | // The thread's error explains a different, rejected message still in the draft. | ||
| 461 | setThreadComposerError(threadKey, "rejected", "message-rejected"); | ||
| 462 | |||
| 463 | await completeQueuedMessageDelivery(other, harness.manager.revisionOf(other.messageId)); | ||
| 464 | expect(appAtomRegistry.get(threadComposerErrorsAtom)[threadKey]?.message).toBe("rejected"); | ||
| 465 | }); | ||
| 466 | |||
| 445 | it("keeps a delivered message when its editor opens during storage removal", async () => { | 467 | it("keeps a delivered message when its editor opens during storage removal", async () => { |
| 446 | const message = queuedMessage({ | 468 | const message = queuedMessage({ |
| 447 | messageId: "message-editor-removal-race", | 469 | messageId: "message-editor-removal-race", |
| ... | @@ -697,7 +719,8 @@ describe("thread outbox recovery rollback", () => { | ... | @@ -697,7 +719,8 @@ describe("thread outbox recovery rollback", () => { |
| 697 | }, | 719 | }, |
| 698 | }); | 720 | }); |
| 699 | expect(remainingMessages()).toEqual([]); | 721 | expect(remainingMessages()).toEqual([]); |
| 700 | expect(harness.setPendingConnectionError).toHaveBeenCalledWith("rejected by server"); | 722 | // The creation's failure card shows the reason; the composer is hidden. |
| 723 | expect(appAtomRegistry.get(threadComposerErrorsAtom)).toEqual({}); | ||
| 701 | // The thread screen opened for this creation reads the failure from here. | 724 | // The thread screen opened for this creation reads the failure from here. |
| 702 | expect( | 725 | expect( |
| 703 | appAtomRegistry.get(pendingThreadCreationOutcomesAtom)[ | 726 | appAtomRegistry.get(pendingThreadCreationOutcomesAtom)[ |
| ... | @@ -706,6 +729,37 @@ describe("thread outbox recovery rollback", () => { | ... | @@ -706,6 +729,37 @@ describe("thread outbox recovery rollback", () => { |
| 706 | ).toEqual({ kind: "failed", message, reason: "rejected by server" }); | 729 | ).toEqual({ kind: "failed", message, reason: "rejected by server" }); |
| 707 | }); | 730 | }); |
| 708 | 731 | ||
| 732 | it("drops an earlier recovery error once a rejected new task is restored", async () => { | ||
| 733 | const message: QueuedThreadMessage = { | ||
| 734 | ...queuedMessage({ messageId: "message-creation-retried", text: "new task text" }), | ||
| 735 | modelSelection: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-5.6-sol" }, | ||
| 736 | creation: { | ||
| 737 | projectId: ProjectId.make("project-1"), | ||
| 738 | workspaceMode: "local", | ||
| 739 | branch: null, | ||
| 740 | worktreePath: null, | ||
| 741 | }, | ||
| 742 | }; | ||
| 743 | await harness.manager.enqueue(message); | ||
| 744 | harness.draftFile.setWriteError(new Error("disk full")); | ||
| 745 | await expect(restoreRejectedQueuedMessage(message, "rejected by server")).resolves.toBe( | ||
| 746 | "retry", | ||
| 747 | ); | ||
| 748 | const threadKey = `${message.environmentId}:${message.threadId}`; | ||
| 749 | expect(appAtomRegistry.get(threadComposerErrorsAtom)[threadKey]?.messageId).toBe( | ||
| 750 | message.messageId, | ||
| 751 | ); | ||
| 752 | |||
| 753 | harness.draftFile.setWriteError(null); | ||
| 754 | await expect(restoreRejectedQueuedMessage(message, "rejected by server")).resolves.toBe( | ||
| 755 | "restored", | ||
| 756 | ); | ||
| 757 | |||
| 758 | // The failure card carries the reason now; nothing stale sits above it. | ||
| 759 | expect(appAtomRegistry.get(threadComposerErrorsAtom)).toEqual({}); | ||
| 760 | expect(appAtomRegistry.get(pendingThreadCreationOutcomesAtom)[threadKey]?.kind).toBe("failed"); | ||
| 761 | }); | ||
| 762 | |||
| 709 | it("keeps a failed outcome until its thread screen consumes it", async () => { | 763 | it("keeps a failed outcome until its thread screen consumes it", async () => { |
| 710 | const message: QueuedThreadMessage = { | 764 | const message: QueuedThreadMessage = { |
| 711 | ...queuedMessage({ messageId: "message-creation-kept", text: "new task text" }), | 765 | ...queuedMessage({ messageId: "message-creation-kept", text: "new task text" }), |
| ... | @@ -734,6 +788,46 @@ describe("thread outbox recovery rollback", () => { | ... | @@ -734,6 +788,46 @@ describe("thread outbox recovery rollback", () => { |
| 734 | await expect(restoreRejectedQueuedMessage(message, "rejected")).resolves.toBe("restored"); | 788 | await expect(restoreRejectedQueuedMessage(message, "rejected")).resolves.toBe("restored"); |
| 735 | 789 | ||
| 736 | expect(appAtomRegistry.get(pendingThreadCreationOutcomesAtom)).toEqual({}); | 790 | expect(appAtomRegistry.get(pendingThreadCreationOutcomesAtom)).toEqual({}); |
| 791 | // The thread screen shows why the message came back into the composer. | ||
| 792 | expect( | ||
| 793 | appAtomRegistry.get(threadComposerErrorsAtom)[`${message.environmentId}:${message.threadId}`], | ||
| 794 | ).toEqual({ message: "rejected", messageId: message.messageId }); | ||
| 795 | }); | ||
| 796 | |||
| 797 | it("leaves no error when the restored text is resent before recovery finishes", async () => { | ||
| 798 | const message = queuedMessage({ messageId: "message-resent", text: "resend me" }); | ||
| 799 | const threadKey = `${message.environmentId}:${message.threadId}`; | ||
| 800 | await harness.manager.enqueue(message); | ||
| 801 | // The user sees the restored text as soon as the merge publishes it and | ||
| 802 | // sends it again while the recovery is still awaiting persistence. | ||
| 803 | const unsubscribe = appAtomRegistry.subscribe(composerDrafts.composerDraftsAtom, (drafts) => { | ||
| 804 | if (drafts[threadKey]?.text === "resend me") { | ||
| 805 | unsubscribe(); | ||
| 806 | clearThreadComposerError(threadKey); | ||
| 807 | void composerDrafts.clearComposerDraftContent(threadKey); | ||
| 808 | } | ||
| 809 | }); | ||
| 810 | |||
| 811 | await expect(restoreRejectedQueuedMessage(message, "rejected")).resolves.toBe("restored"); | ||
| 812 | |||
| 813 | expect(appAtomRegistry.get(threadComposerErrorsAtom)).toEqual({}); | ||
| 814 | }); | ||
| 815 | |||
| 816 | it("withdraws the error when an edit makes the recovery back out", async () => { | ||
| 817 | const message = queuedMessage({ messageId: "message-edited-mid-recovery", text: "edit me" }); | ||
| 818 | const threadKey = `${message.environmentId}:${message.threadId}`; | ||
| 819 | await harness.manager.enqueue(message); | ||
| 820 | const unsubscribe = appAtomRegistry.subscribe(composerDrafts.composerDraftsAtom, (drafts) => { | ||
| 821 | if (drafts[threadKey]?.text === "edit me") { | ||
| 822 | unsubscribe(); | ||
| 823 | appAtomRegistry.set(editingQueuedMessageIdsAtom, { [message.messageId]: true }); | ||
| 824 | } | ||
| 825 | }); | ||
| 826 | |||
| 827 | await expect(restoreRejectedQueuedMessage(message, "rejected")).resolves.toBe("deferred"); | ||
| 828 | |||
| 829 | expect(appAtomRegistry.get(threadComposerErrorsAtom)).toEqual({}); | ||
| 830 | expect(remainingMessages()).toEqual([message]); | ||
| 737 | }); | 831 | }); |
| 738 | 832 | ||
| 739 | it("rolls a failed recovery merge back so the retry cannot duplicate the text", async () => { | 833 | it("rolls a failed recovery merge back so the retry cannot duplicate the text", async () => { |
| ... | @@ -759,6 +853,6 @@ describe("thread outbox recovery rollback", () => { | ... | @@ -759,6 +853,6 @@ describe("thread outbox recovery rollback", () => { |
| 759 | "typed offline\n\nqueued text", | 853 | "typed offline\n\nqueued text", |
| 760 | ); | 854 | ); |
| 761 | expect(remainingMessages()).toEqual([]); | 855 | expect(remainingMessages()).toEqual([]); |
| 762 | expect(harness.setPendingConnectionError).toHaveBeenCalledWith("too large"); | 856 | expect(appAtomRegistry.get(threadComposerErrorsAtom)[draftKey]?.message).toBe("too large"); |
| 763 | }); | 857 | }); |
| 764 | }); | 858 | }); |
apps/mobile/src/state/use-thread-outbox-drain.ts+29-8| ... | @@ -80,10 +80,8 @@ import { | ... | @@ -80,10 +80,8 @@ import { |
| 80 | useThreadOutboxMessages, | 80 | useThreadOutboxMessages, |
| 81 | useThreadOutboxShellStatuses, | 81 | useThreadOutboxShellStatuses, |
| 82 | } from "./use-thread-outbox"; | 82 | } from "./use-thread-outbox"; |
| 83 | import { | 83 | import { clearThreadComposerError, setThreadComposerError } from "./thread-composer-error"; |
| 84 | setPendingConnectionError, | 84 | import { useRemoteConnectionStatus } from "./use-remote-environment-registry"; |
| 85 | useRemoteConnectionStatus, | ||
| 86 | } from "./use-remote-environment-registry"; | ||
| 87 | 85 | ||
| 88 | // Ordinary offline behavior (a socket dropping mid-request, a retryable | 86 | // Ordinary offline behavior (a socket dropping mid-request, a retryable |
| 89 | // attachment upload failure) must not spam `console.warn` on every backoff | 87 | // attachment upload failure) must not spam `console.warn` on every backoff |
| ... | @@ -282,6 +280,12 @@ export async function completeQueuedMessageDelivery( | ... | @@ -282,6 +280,12 @@ export async function completeQueuedMessageDelivery( |
| 282 | queuedMessage: QueuedThreadMessage, | 280 | queuedMessage: QueuedThreadMessage, |
| 283 | deliveryRevision: number, | 281 | deliveryRevision: number, |
| 284 | ): Promise<"removed" | "edited" | "failed"> { | 282 | ): Promise<"removed" | "edited" | "failed"> { |
| 283 | // The server took it after all: an error left by an earlier failed recovery | ||
| 284 | // of this same message no longer applies. | ||
| 285 | clearThreadComposerError( | ||
| 286 | scopedThreadKey(queuedMessage.environmentId, queuedMessage.threadId), | ||
| 287 | queuedMessage.messageId, | ||
| 288 | ); | ||
| 285 | try { | 289 | try { |
| 286 | await removeDeliveredCloudQueuedMessage(queuedMessage).catch((error) => { | 290 | await removeDeliveredCloudQueuedMessage(queuedMessage).catch((error) => { |
| 287 | console.warn("[thread-outbox] could not update sign-out snapshot after delivery", { | 291 | console.warn("[thread-outbox] could not update sign-out snapshot after delivery", { |
| ... | @@ -438,6 +442,7 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -438,6 +442,7 @@ export async function restoreRejectedQueuedMessage( |
| 438 | message: string, | 442 | message: string, |
| 439 | ): Promise<"restored" | "deferred" | "blocked" | "retry"> { | 443 | ): Promise<"restored" | "deferred" | "blocked" | "retry"> { |
| 440 | const draftKey = recoveryDraftKey(queuedMessage); | 444 | const draftKey = recoveryDraftKey(queuedMessage); |
| 445 | const threadKey = scopedThreadKey(queuedMessage.environmentId, queuedMessage.threadId); | ||
| 441 | // Set once the merge publishes, cleared once the queued message is removed. | 446 | // Set once the merge publishes, cleared once the queued message is removed. |
| 442 | // The catch below uses it to take the merged content back out, so a retry | 447 | // The catch below uses it to take the merged content back out, so a retry |
| 443 | // after a mid-recovery failure cannot append the recovered text again. | 448 | // after a mid-recovery failure cannot append the recovered text again. |
| ... | @@ -467,12 +472,21 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -467,12 +472,21 @@ export async function restoreRejectedQueuedMessage( |
| 467 | (attachment) => !existingAttachmentIds.has(attachment.id), | 472 | (attachment) => !existingAttachmentIds.has(attachment.id), |
| 468 | ).length; | 473 | ).length; |
| 469 | if (existingAttachmentIds.size + addedAttachmentCount > PROVIDER_SEND_TURN_MAX_ATTACHMENTS) { | 474 | if (existingAttachmentIds.size + addedAttachmentCount > PROVIDER_SEND_TURN_MAX_ATTACHMENTS) { |
| 470 | setPendingConnectionError( | 475 | setThreadComposerError( |
| 476 | threadKey, | ||
| 471 | `Remove attachments from the draft before restoring this message. Messages can contain at most ${PROVIDER_SEND_TURN_MAX_ATTACHMENTS} attachments.`, | 477 | `Remove attachments from the draft before restoring this message. Messages can contain at most ${PROVIDER_SEND_TURN_MAX_ATTACHMENTS} attachments.`, |
| 478 | queuedMessage.messageId, | ||
| 472 | ); | 479 | ); |
| 473 | return "blocked"; | 480 | return "blocked"; |
| 474 | } | 481 | } |
| 475 | 482 | ||
| 483 | // Shown before the merge publishes the text, so a resend of that text, | ||
| 484 | // which can happen while this recovery still awaits persistence, clears | ||
| 485 | // it. Withdrawn below wherever the recovery backs out. | ||
| 486 | const withdrawError = () => clearThreadComposerError(threadKey, queuedMessage.messageId); | ||
| 487 | if (!queuedMessage.creation) { | ||
| 488 | setThreadComposerError(threadKey, message, queuedMessage.messageId); | ||
| 489 | } | ||
| 476 | let mergedDraft: ComposerDraft; | 490 | let mergedDraft: ComposerDraft; |
| 477 | try { | 491 | try { |
| 478 | stampRecoveryDraftProject(queuedMessage, draftKey); | 492 | stampRecoveryDraftProject(queuedMessage, draftKey); |
| ... | @@ -492,6 +506,7 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -492,6 +506,7 @@ export async function restoreRejectedQueuedMessage( |
| 492 | rollback = { snapshot: originalDraft, merged: mergedDraft }; | 506 | rollback = { snapshot: originalDraft, merged: mergedDraft }; |
| 493 | } | 507 | } |
| 494 | if (appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId]) { | 508 | if (appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId]) { |
| 509 | withdrawError(); | ||
| 495 | await undoComposerDraftMerge(draftKey, originalDraft, mergedDraft); | 510 | await undoComposerDraftMerge(draftKey, originalDraft, mergedDraft); |
| 496 | return "deferred"; | 511 | return "deferred"; |
| 497 | } | 512 | } |
| ... | @@ -520,6 +535,7 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -520,6 +535,7 @@ export async function restoreRejectedQueuedMessage( |
| 520 | !(await confirmThreadOutboxMessageQueued(queuedMessage)) || | 535 | !(await confirmThreadOutboxMessageQueued(queuedMessage)) || |
| 521 | appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId] | 536 | appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId] |
| 522 | ) { | 537 | ) { |
| 538 | withdrawError(); | ||
| 523 | await undoComposerDraftMerge(draftKey, originalDraft, restoredDraft); | 539 | await undoComposerDraftMerge(draftKey, originalDraft, restoredDraft); |
| 524 | return "deferred"; | 540 | return "deferred"; |
| 525 | } | 541 | } |
| ... | @@ -532,6 +548,7 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -532,6 +548,7 @@ export async function restoreRejectedQueuedMessage( |
| 532 | () => !appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId], | 548 | () => !appAtomRegistry.get(editingQueuedMessageIdsAtom)[queuedMessage.messageId], |
| 533 | )) | 549 | )) |
| 534 | ) { | 550 | ) { |
| 551 | withdrawError(); | ||
| 535 | await undoComposerDraftMerge(draftKey, originalDraft, restoredDraft); | 552 | await undoComposerDraftMerge(draftKey, originalDraft, restoredDraft); |
| 536 | return "deferred"; | 553 | return "deferred"; |
| 537 | } | 554 | } |
| ... | @@ -539,15 +556,17 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -539,15 +556,17 @@ export async function restoreRejectedQueuedMessage( |
| 539 | // must never be rolled back. | 556 | // must never be rolled back. |
| 540 | rollback = null; | 557 | rollback = null; |
| 541 | if (queuedMessage.creation) { | 558 | if (queuedMessage.creation) { |
| 559 | // The failure card shows the reason, so an error left by an earlier | ||
| 560 | // failed attempt at this recovery no longer applies. | ||
| 561 | withdrawError(); | ||
| 542 | // The thread screen for this creation is likely open; it reads the | 562 | // The thread screen for this creation is likely open; it reads the |
| 543 | // outcome to offer reopening the restored draft. | 563 | // outcome to offer reopening the restored draft, and shows the reason. |
| 544 | recordPendingThreadCreationOutcome({ | 564 | recordPendingThreadCreationOutcome({ |
| 545 | kind: "failed", | 565 | kind: "failed", |
| 546 | message: queuedMessage, | 566 | message: queuedMessage, |
| 547 | reason: message, | 567 | reason: message, |
| 548 | }); | 568 | }); |
| 549 | } | 569 | } |
| 550 | setPendingConnectionError(message); | ||
| 551 | return "restored"; | 570 | return "restored"; |
| 552 | } catch (error) { | 571 | } catch (error) { |
| 553 | if (rollback !== null) { | 572 | if (rollback !== null) { |
| ... | @@ -561,8 +580,10 @@ export async function restoreRejectedQueuedMessage( | ... | @@ -561,8 +580,10 @@ export async function restoreRejectedQueuedMessage( |
| 561 | ); | 580 | ); |
| 562 | } | 581 | } |
| 563 | console.warn("[thread-outbox] failed to restore an undeliverable message", error); | 582 | console.warn("[thread-outbox] failed to restore an undeliverable message", error); |
| 564 | setPendingConnectionError( | 583 | setThreadComposerError( |
| 584 | threadKey, | ||
| 565 | error instanceof Error ? error.message : "The unsent message could not be restored.", | 585 | error instanceof Error ? error.message : "The unsent message could not be restored.", |
| 586 | queuedMessage.messageId, | ||
| 566 | ); | 587 | ); |
| 567 | return "retry"; | 588 | return "retry"; |
| 568 | } | 589 | } |