From 88f28369d803f8f3941056f3125a125b8368f6ee Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:17:53 -0700 Subject: [PATCH 1/4] refactor(mobile): share imports run outside the new-task screen The new-task draft screen moved an incoming share into its draft from a long useEffect that tracked the import with four state values, five refs, a mount-tracking effect and a useEffectEvent, and left the route from a second effect once a cancel committed. Cancel and retry were closures inside that effect, so they stopped working once the screen unmounted. The import is now a small state machine (incomingShareImport.ts) owned by the share store, keyed by the draft route's flow key. The screen derives its target from render inputs and reads the phase atom; reading it starts the import at most once, the same pattern as the new-task branch checkout. App's navigation state handler prunes imports whose route left the stack, alongside the branch checkout prune. Behavior changes: - A failed import's Cancel finishes its draft restore and reservation release even if the route left meanwhile; before, it did nothing after unmount. - "Shared content unavailable" is an inline note in the hero instead of a one-time alert. - The route is not held while a failed import waits for an answer or while a cancel is undoing; the cancel leaves the route itself when done. Effects in NewTaskDraftScreen: 8 -> 5 (removed the cancelled-goBack effect, the mount-tracking effect, and the import effect). Co-Authored-By: Claude Opus 5.5 --- apps/mobile/src/App.tsx | 2 + .../sharing/IncomingShareProvider.tsx | 77 +++- .../features/threads/NewTaskDraftScreen.tsx | 370 ++++-------------- .../threads/incomingShareImport.test.ts | 252 ++++++++++++ .../features/threads/incomingShareImport.ts | 361 +++++++++++++++++ 5 files changed, 764 insertions(+), 298 deletions(-) create mode 100644 apps/mobile/src/features/threads/incomingShareImport.test.ts create mode 100644 apps/mobile/src/features/threads/incomingShareImport.ts diff --git a/apps/mobile/src/App.tsx b/apps/mobile/src/App.tsx index b7c054d9a64ac..1443544d94697 100644 --- a/apps/mobile/src/App.tsx +++ b/apps/mobile/src/App.tsx @@ -26,6 +26,7 @@ import { CloudAuthProvider } from "./features/cloud/CloudAuthProvider"; import { prepareNativeShowcaseCapture } from "./features/showcase/nativeShowcaseScene"; import { presentPendingIncomingShare, + pruneIncomingShareImports, startIncomingShareInbox, } from "./features/sharing/IncomingShareProvider"; import { @@ -133,6 +134,7 @@ function handleNavigationStateChange(state: NavigationState | undefined): void { setAgentNotificationPathname(workspaceLocationFromState(state).pathname); recordAppShortcutNavigationState(state); pruneNewTaskBranchCheckouts(state); + pruneIncomingShareImports(state); } function handleNavigationReady(): void { diff --git a/apps/mobile/src/features/sharing/IncomingShareProvider.tsx b/apps/mobile/src/features/sharing/IncomingShareProvider.tsx index 23b34d362beee..184eaf15daa2b 100644 --- a/apps/mobile/src/features/sharing/IncomingShareProvider.tsx +++ b/apps/mobile/src/features/sharing/IncomingShareProvider.tsx @@ -1,7 +1,7 @@ import * as Dialog from "../../components/ui/Dialog"; import Constants from "expo-constants"; import * as Crypto from "expo-crypto"; -import { PROVIDER_SEND_TURN_MAX_FILE_BYTES } from "@t3tools/contracts"; +import { EnvironmentId, PROVIDER_SEND_TURN_MAX_FILE_BYTES } from "@t3tools/contracts"; import { clearSharedPayloads, getResolvedSharedPayloadsAsync, @@ -11,10 +11,12 @@ import { } from "expo-sharing"; import { useSyncExternalStore } from "react"; import { Platform } from "react-native"; +import { CommonActions } from "@react-navigation/native"; import { buildIncomingShareDraft, isShareFileUriUnderOwnedRoots, + selectIncomingShareAttachmentsForServer, type IncomingShareDestination, type IncomingShareDraft, } from "./incoming-share-model"; @@ -26,6 +28,16 @@ import { coalescedRun } from "../../lib/coalescedRun"; import { rootNavigationRef } from "../../lib/rootNavigation"; import { selectWorkspaceDetail } from "../workspace/workspace-detail-selection"; import { isDraftThreadRoute } from "../threads/thread-route-state"; +import { makeIncomingShareImports } from "../threads/incomingShareImport"; +import { liveNewTaskFlowKeys } from "../threads/new-task-flow-state"; +import { appAtomRegistry } from "../../state/atom-registry"; +import { serverEnvironment } from "../../state/server"; +import { + getComposerDraftSnapshot, + mergeComposerDraftContent, + restoreComposerDraftSnapshot, + scheduleUnusedComposerAttachmentCleanup, +} from "../../state/use-composer-drafts"; import { EMPTY_INCOMING_SHARE_PRESENTATION_STATE, transitionIncomingSharePresentation, @@ -282,6 +294,67 @@ async function releaseShareReservation( publish(await incomingShareInbox.releaseReservation(shareId, expectedDestination)); } +/** Moves shares into new-task drafts; see makeIncomingShareImports. */ +export const incomingShareImports = makeIncomingShareImports({ + registry: appAtomRegistry, + getShare: (shareId) => snapshot.drafts.find((draft) => draft.id === shareId) ?? null, + reserve: reserveShare, + release: releaseShareReservation, + consume: consumeShare, + readDraft: getComposerDraftSnapshot, + restoreDraft: restoreComposerDraftSnapshot, + mergeIntoDraft: mergeComposerDraftContent, + selectAttachments: (share, environmentId) => + selectIncomingShareAttachmentsForServer({ + attachments: share.attachments, + serverConfig: appAtomRegistry.get( + serverEnvironment.configValueAtom(EnvironmentId.make(environmentId)), + ), + }), + releaseUnusedAttachments: scheduleUnusedComposerAttachmentCleanup, + onWarnings: (warnings) => { + Dialog.alert("Some shared content was skipped", warnings.join("\n")); + }, + onImportFailure: (target, error) => { + Dialog.alert( + "Could not import shared content", + error instanceof Error ? error.message : "The shared content could not be saved.", + [ + { + text: "Cancel import", + style: "cancel", + onPress: () => void incomingShareImports.cancel(target), + }, + { text: "Retry", onPress: () => incomingShareImports.retry(target) }, + ], + { cancelable: false }, + ); + }, + onCancelFailure: (target, error) => { + Dialog.alert( + "Could not cancel import", + error instanceof Error ? error.message : "The shared content could not be restored safely.", + [ + { text: "Retry import", onPress: () => incomingShareImports.retry(target) }, + { text: "Retry cancel", onPress: () => void incomingShareImports.cancel(target) }, + ], + { cancelable: false }, + ); + }, + leave: (target) => { + rootNavigationRef.dispatch({ + ...CommonActions.goBack(), + source: target.routeKey, + target: rootNavigationRef.getRootState()?.key, + }); + }, +}); + +/** Abandons the share imports of draft routes that left the stack. */ +export function pruneIncomingShareImports(state: Parameters[0]) { + void incomingShareImports.prune(liveNewTaskFlowKeys(state)); +} + export function useIncomingShare() { const { drafts, isLoading } = useSyncExternalStore( subscribeIncomingShares, @@ -291,8 +364,6 @@ export function useIncomingShare() { pendingShare: drafts[0] ?? null, isLoading, getShare: (shareId: string) => drafts.find((draft) => draft.id === shareId) ?? null, - reserveShare, releaseShareReservation, - consumeShare, }; } diff --git a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx index ea4017a2aa063..15b2b4e6591e0 100644 --- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx @@ -22,8 +22,9 @@ import { StackActions, useNavigation, usePreventRemove, + useRoute, } from "@react-navigation/native"; -import { useEffect, useEffectEvent, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Platform, Pressable, View } from "react-native"; import { ScreenScrollView as ScrollView } from "../../components/ui/ScreenScrollView"; import { KeyboardController } from "react-native-keyboard-controller"; @@ -55,11 +56,8 @@ import { makeTurnCommandMetadata } from "../../lib/commandMetadata"; import { clearComposerDraftContent, getComposerDraftSnapshot, - mergeComposerDraftContent, - restoreComposerDraftSnapshot, updateComposerDraftSettings, scheduleUnusedComposerAttachmentCleanup, - type ComposerDraft, waitForComposerDraftsLoaded, } from "../../state/use-composer-drafts"; import { useEnvironmentServerConfig, useProjects } from "../../state/entities"; @@ -89,10 +87,14 @@ import { resolveNewTaskBranchLabel, resolveNewTaskWorkspaceLabel, } from "./new-task-context-presentation"; -import { useIncomingShare } from "../sharing/IncomingShareProvider"; -import { selectIncomingShareAttachmentsForServer } from "../sharing/incoming-share-model"; +import { incomingShareImports, useIncomingShare } from "../sharing/IncomingShareProvider"; +import { + idleImportedShareAtom, + idleShareImportPhaseAtom, + isShareImportInFlight, +} from "./incomingShareImport"; +import { newTaskFlowKey } from "./new-task-flow-state"; import { appAtomRegistry } from "../../state/atom-registry"; -import { serverEnvironment } from "../../state/server"; import { readEnvironmentScope, useEnvironmentScope } from "../../state/session"; @@ -157,13 +159,9 @@ export function NewTaskDraftScreen(props: { const flow = useNewTaskFlow(); const navigation = useNavigation(); const goBack = useWorkspaceBackNavigation("threads"); - const { - consumeShare, - getShare, - isLoading: isIncomingShareInboxLoading, - releaseShareReservation, - reserveShare, - } = useIncomingShare(); + const currentRoute = useRoute(); + const flowKey = newTaskFlowKey(currentRoute); + const { getShare, isLoading: isIncomingShareInboxLoading } = useIncomingShare(); const { projectScopes, selectedProject, selectedProjectKey, setProject } = flow; const selectedProjectLabel = (() => { if (!selectedProject) return null; @@ -264,26 +262,7 @@ export function NewTaskDraftScreen(props: { const queuesInsteadOfStarting = !environmentConnected || attachmentsUploading; const promptInputRef = useRef(null); const loadedBranchesProjectKeyRef = useRef(null); - const [importingShareKey, setImportingShareKey] = useState(null); - const [isCancellingShareImport, setIsCancellingShareImport] = useState(false); - const [cancelledIncomingShareId, setCancelledIncomingShareId] = useState(null); const [isReturningToProjectPicker, setIsReturningToProjectPicker] = useState(false); - const [shareImportAttempt, setShareImportAttempt] = useState(0); - const startedShareImportKeyRef = useRef(null); - const cancellingShareImportKeyRef = useRef(null); - const shareImportDraftBackupRef = useRef(new Map()); - const activeShareImportTokenRef = useRef(null); - const shareImportMountedRef = useRef(true); - // The share import resumes after awaits; this reads the screen's current - // draft and share at that point without re-running the import effect. - const isCurrentShareImport = useEffectEvent( - (draftKey: string, shareId: string) => - flow.draftKey === draftKey && props.incomingShareId === shareId, - ); - const isImportingShare = importingShareKey !== null; - const alertedUnavailableIncomingShareIdRef = useRef(null); - // Remember consumed shares locally: send clears the receipt before navigation and can trigger a missing-content render. - const [consumedIncomingShareId, setConsumedIncomingShareId] = useState(null); const incomingShare = props.incomingShareId ? getShare(props.incomingShareId) : null; const requestedInitialProjectAvailable = Boolean( props.initialProjectRef?.environmentId && @@ -300,11 +279,63 @@ export function NewTaskDraftScreen(props: { incomingShare?.attachments.some((attachment) => attachment.type === "file") && selectedEnvironmentServerConfig === null, ); + const routeImportedShare = useAtomValue( + flowKey !== undefined && props.incomingShareId + ? incomingShareImports.importedAtom(flowKey, props.incomingShareId) + : idleImportedShareAtom, + ); + const hasImportedIncomingShare = Boolean( + props.incomingShareId && + (routeImportedShare || + (flow.draftKey && + getComposerDraftSnapshot(flow.draftKey).importedShareIds?.includes(props.incomingShareId))), + ); + const isIncomingShareUnavailable = Boolean( + props.incomingShareId && + !isIncomingShareInboxLoading && + !incomingShare && + !hasImportedIncomingShare, + ); + const shareImportTarget = (() => { + if (!props.incomingShareId || flowKey === undefined || !flow.draftKey) return null; + if (!selectedProject || !incomingShare || isIncomingShareAwaitingServerConfig) return null; + const initialEnvironmentId = props.initialProjectRef?.environmentId; + const initialProjectId = props.initialProjectRef?.projectId; + if ( + initialEnvironmentId && + initialProjectId && + (selectedProject.environmentId !== initialEnvironmentId || + selectedProject.id !== initialProjectId) + ) { + return null; + } + return { + flowKey, + routeKey: currentRoute.key, + draftKey: flow.draftKey, + destination: { + environmentId: String(selectedProject.environmentId), + projectId: String(selectedProject.id), + }, + shareId: incomingShare.id, + }; + })(); + const shareImportPhase = useAtomValue( + shareImportTarget === null + ? idleShareImportPhaseAtom + : incomingShareImports.phaseAtom(shareImportTarget), + ); + const isImportingShare = isShareImportInFlight(shareImportPhase); const isIncomingShareTransferPending = Boolean( - incomingShare && - cancelledIncomingShareId !== props.incomingShareId && - !isIncomingShareAwaitingServerConfig, + incomingShare && shareImportPhase !== "cancelled" && !isIncomingShareAwaitingServerConfig, ); + // A failed import waits in a modal alert, and a cancel finishes its undo even + // after the route leaves, so neither holds the route: the cancel can leave as + // soon as it is done. + const isIncomingShareHoldingRoute = + isIncomingShareTransferPending && + shareImportPhase !== "failed" && + shareImportPhase !== "cancelling"; const contextImports = useAtomValue(composerContextImportsAtom); const isImportingContext = flow.draftKey ? contextImports[flow.draftKey] === true : false; const isComposerInteractionLocked = @@ -323,32 +354,13 @@ export function NewTaskDraftScreen(props: { ? selectedProject?.workspaceRoot : (flow.selectedWorktreePath ?? selectedProject?.workspaceRoot)) || null; const preventRemove = - (isIncomingShareTransferPending && !isProjectPickerReturnActive) || - isCancellingShareImport || - flow.submitting; + (isIncomingShareHoldingRoute && !isProjectPickerReturnActive) || flow.submitting; usePreventRemove(preventRemove, () => undefined); - const hasImportedIncomingShare = Boolean( - props.incomingShareId && - (consumedIncomingShareId === props.incomingShareId || - (flow.draftKey && - getComposerDraftSnapshot(flow.draftKey).importedShareIds?.includes(props.incomingShareId))), - ); - const isIncomingShareUnavailable = Boolean( - props.incomingShareId && - !isIncomingShareInboxLoading && - !incomingShare && - !hasImportedIncomingShare, - ); const isIncomingShareReady = !props.incomingShareId || (hasImportedIncomingShare && !incomingShare) || isIncomingShareUnavailable; const appliedInitialProjectKeyRef = useRef(null); - useEffect(() => { - if (cancelledIncomingShareId === props.incomingShareId) { - navigation.goBack(); - } - }, [cancelledIncomingShareId, navigation, props.incomingShareId]); useEffect(() => { if (!isReturningToProjectPicker) { return; @@ -371,19 +383,6 @@ export function NewTaskDraftScreen(props: { props.incomingShareId, requestedInitialProjectAvailable, ]); - useEffect(() => { - if (!shareImportMountedRef.current) { - startedShareImportKeyRef.current = null; - } - shareImportMountedRef.current = true; - return () => { - appliedInitialProjectKeyRef.current = null; - shareImportMountedRef.current = false; - activeShareImportTokenRef.current = null; - cancellingShareImportKeyRef.current = null; - }; - }, []); - const { openDraft } = flow; // Cold launches load drafts and projects separately; wait for hydration and retry while shells load. const attemptedDraftIdRef = useRef(null); @@ -515,232 +514,6 @@ export function NewTaskDraftScreen(props: { loadBranches(); }, [loadBranches, selectedProject]); - useEffect(() => { - const shareId = props.incomingShareId; - const draftKey = flow.draftKey; - const destinationProject = selectedProject; - const initialEnvironmentId = props.initialProjectRef?.environmentId; - const initialProjectId = props.initialProjectRef?.projectId; - const selectedProjectMatchesRoute = - !initialEnvironmentId || - !initialProjectId || - (destinationProject?.environmentId === initialEnvironmentId && - destinationProject.id === initialProjectId); - if ( - !shareId || - !draftKey || - !destinationProject || - !selectedProjectMatchesRoute || - cancelledIncomingShareId === shareId - ) { - return; - } - const importKey = `${shareId}:${draftKey}`; - if ( - startedShareImportKeyRef.current === importKey || - cancellingShareImportKeyRef.current === importKey - ) { - return; - } - - if (!incomingShare) { - if (isIncomingShareUnavailable && alertedUnavailableIncomingShareIdRef.current !== shareId) { - alertedUnavailableIncomingShareIdRef.current = shareId; - Dialog.alert( - "Shared content unavailable", - "The shared content is no longer in the inbox. You can continue editing this task draft.", - ); - } - return; - } - - if ( - incomingShare.attachments.some((attachment) => attachment.type === "file") && - selectedEnvironmentServerConfig === null - ) { - return; - } - - if (alertedUnavailableIncomingShareIdRef.current === shareId) { - alertedUnavailableIncomingShareIdRef.current = null; - } - startedShareImportKeyRef.current = importKey; - const draftBackup = - shareImportDraftBackupRef.current.get(importKey) ?? getComposerDraftSnapshot(draftKey); - shareImportDraftBackupRef.current.set(importKey, draftBackup); - const importToken = Symbol(importKey); - let didReserveShare = false; - let didConsumeShare = false; - let needsDraftRestore = false; - activeShareImportTokenRef.current = importToken; - // oxlint-disable-next-line react/set-state-in-effect -- Publishes the share import started by this effect before its async work runs. - setImportingShareKey(importKey); - void (async () => { - await reserveShare(shareId, { - environmentId: String(destinationProject.environmentId), - projectId: String(destinationProject.id), - }); - didReserveShare = true; - if ( - !shareImportMountedRef.current || - activeShareImportTokenRef.current !== importToken || - !isCurrentShareImport(draftKey, shareId) - ) { - return; - } - const selectedAttachments = selectIncomingShareAttachmentsForServer({ - attachments: incomingShare.attachments, - serverConfig: appAtomRegistry.get( - serverEnvironment.configValueAtom(destinationProject.environmentId), - ), - }); - if (selectedAttachments.status === "pending") { - throw new Error("Server attachment support is still loading."); - } - needsDraftRestore = true; - const { skippedAttachmentCount } = await mergeComposerDraftContent(draftKey, { - text: incomingShare.text, - attachments: selectedAttachments.attachments, - sourceShareId: shareId, - }); - if ( - !shareImportMountedRef.current || - activeShareImportTokenRef.current !== importToken || - !isCurrentShareImport(draftKey, shareId) - ) { - // Durable reservations resume interrupted transfers only in this project, avoiding duplicate draft copies. - return; - } - await consumeShare(shareId); - didConsumeShare = true; - setConsumedIncomingShareId(shareId); - // Release skipped files after consumption even on unmount; sweeping re-checks new owners. - const retainedAttachmentIds = new Set( - getComposerDraftSnapshot(draftKey).attachments.map((attachment) => attachment.id), - ); - scheduleUnusedComposerAttachmentCleanup( - incomingShare.attachments.filter((attachment) => !retainedAttachmentIds.has(attachment.id)), - ); - if (!shareImportMountedRef.current || activeShareImportTokenRef.current !== importToken) { - return; - } - const warnings = [...incomingShare.warnings, ...selectedAttachments.warnings]; - if (skippedAttachmentCount > 0) { - warnings.push( - `${skippedAttachmentCount} shared file${skippedAttachmentCount === 1 ? " was" : "s were"} skipped because this draft reached the attachment limit.`, - ); - } - if (warnings.length > 0) { - Dialog.alert("Some shared content was skipped", warnings.join("\n")); - } - shareImportDraftBackupRef.current.delete(importKey); - })() - .catch((error) => { - if (!shareImportMountedRef.current || activeShareImportTokenRef.current !== importToken) { - return; - } - Dialog.alert( - "Could not import shared content", - error instanceof Error ? error.message : "The shared content could not be saved.", - [ - { - text: "Cancel import", - style: "cancel", - onPress: () => { - const cancelImport = async (): Promise => { - if (!shareImportMountedRef.current) { - return; - } - // Latch before restoring: atom publication can re-run import before React commits cancellation. - cancellingShareImportKeyRef.current = importKey; - setIsCancellingShareImport(true); - try { - if (needsDraftRestore) { - // Sweep restored attachments only after inbox consumption; unconsumed inbox entries still own them. - const mergedAttachments = getComposerDraftSnapshot(draftKey).attachments; - await restoreComposerDraftSnapshot(draftKey, draftBackup); - needsDraftRestore = false; - if (didConsumeShare) { - scheduleUnusedComposerAttachmentCleanup(mergedAttachments); - } - } - if (didReserveShare) { - await releaseShareReservation(shareId, { - environmentId: String(destinationProject.environmentId), - projectId: String(destinationProject.id), - }); - } - shareImportDraftBackupRef.current.delete(importKey); - if (shareImportMountedRef.current) { - setIsCancellingShareImport(false); - setCancelledIncomingShareId(shareId); - } - } catch (cancelError) { - if (!shareImportMountedRef.current) { - return; - } - Dialog.alert( - "Could not cancel import", - cancelError instanceof Error - ? cancelError.message - : "The shared content could not be restored safely.", - [ - { - text: "Retry import", - onPress: () => { - cancellingShareImportKeyRef.current = null; - setIsCancellingShareImport(false); - setShareImportAttempt((attempt) => attempt + 1); - }, - }, - { - text: "Retry cancel", - onPress: () => void cancelImport(), - }, - ], - { cancelable: false }, - ); - } - }; - void cancelImport(); - }, - }, - { - text: "Retry", - onPress: () => setShareImportAttempt((attempt) => attempt + 1), - }, - ], - { cancelable: false }, - ); - }) - .finally(() => { - if (startedShareImportKeyRef.current === importKey) { - startedShareImportKeyRef.current = null; - } - if (shareImportMountedRef.current && activeShareImportTokenRef.current === importToken) { - activeShareImportTokenRef.current = null; - setImportingShareKey(null); - } - }); - }, [ - consumeShare, - cancelledIncomingShareId, - flow.draftKey, - // oxlint-disable-next-line react/exhaustive-effect-dependencies -- hasImportedIncomingShare reflects the draft store; a change re-checks whether the share still needs importing. - hasImportedIncomingShare, - incomingShare, - isIncomingShareInboxLoading, - isIncomingShareUnavailable, - props.incomingShareId, - props.initialProjectRef?.environmentId, - props.initialProjectRef?.projectId, - releaseShareReservation, - reserveShare, - selectedEnvironmentServerConfig, - selectedProject, - shareImportAttempt, - ]); - const selectedEnvironmentLabel = flow.environments.find( (environment) => environment.environmentId === flow.selectedEnvironmentId, @@ -947,6 +720,11 @@ export function NewTaskDraftScreen(props: { onChooseEnvironment={() => openContextPicker("NewTaskEnvironment")} /> ); + const shareUnavailableNotice = isIncomingShareUnavailable ? ( + + The shared content is no longer in the inbox. You can continue editing this task draft. + + ) : null; const hero = flow.isScratchDraft ? ( @@ -964,6 +742,7 @@ export function NewTaskDraftScreen(props: { /> {environmentControl} + {shareUnavailableNotice} ) : ( @@ -993,6 +772,7 @@ export function NewTaskDraftScreen(props: { {environmentControl} + {shareUnavailableNotice} ); // The nested hero scroll view does not receive UIKit's automatic navigation inset in split columns. diff --git a/apps/mobile/src/features/threads/incomingShareImport.test.ts b/apps/mobile/src/features/threads/incomingShareImport.test.ts new file mode 100644 index 0000000000000..079cb3796a1f8 --- /dev/null +++ b/apps/mobile/src/features/threads/incomingShareImport.test.ts @@ -0,0 +1,252 @@ +import { AtomRegistry } from "effect/reactivity"; +import { describe, expect, it, vi } from "vite-plus/test"; + +import type { DraftComposerAttachment } from "../../lib/composerImages"; +import type { IncomingShareDraft } from "../sharing/incoming-share-model"; +import type { ComposerDraft } from "../../state/use-composer-drafts"; +import { + makeIncomingShareImports, + type ShareImportDeps, + type ShareImportPhase, + type ShareImportTarget, + transitionShareImport, +} from "./incomingShareImport"; + +const destination = { environmentId: "env-1", projectId: "project-1" }; + +function fileAttachment(id: string): DraftComposerAttachment { + return { + id, + type: "file", + name: `${id}.txt`, + mimeType: "text/plain", + sizeBytes: 4, + fileUri: `file:///${id}.txt`, + }; +} + +function share(attachments: ReadonlyArray = []): IncomingShareDraft { + return { + schemaVersion: 1, + id: "share-1", + createdAt: "2026-01-01T00:00:00.000Z", + text: "shared text", + attachments, + warnings: [], + }; +} + +function importTarget(overrides: Partial = {}): ShareImportTarget { + return { + flowKey: "flow-a", + routeKey: "route-a", + draftKey: "draft-a", + destination, + shareId: "share-1", + ...overrides, + }; +} + +function setup(overrides: Partial = {}) { + const drafts = new Map([["draft-a", { text: "mine", attachments: [] }]]); + const inbox = new Map([["share-1", share()]]); + const calls: Array = []; + const registry = overrides.registry ?? AtomRegistry.make(); + const deps: ShareImportDeps = { + registry, + getShare: (shareId) => inbox.get(shareId) ?? null, + reserve: vi.fn(async () => { + calls.push("reserve"); + }), + release: vi.fn(async () => { + calls.push("release"); + }), + consume: vi.fn(async () => { + calls.push("consume"); + }), + readDraft: (draftKey) => drafts.get(draftKey) ?? { text: "", attachments: [] }, + restoreDraft: vi.fn(async (draftKey, snapshot) => { + calls.push("restore"); + drafts.set(draftKey, snapshot); + }), + mergeIntoDraft: vi.fn(async (draftKey, content) => { + calls.push("merge"); + const existing = drafts.get(draftKey) ?? { text: "", attachments: [] }; + drafts.set(draftKey, { + text: `${existing.text}\n${content.text}`, + attachments: [...existing.attachments, ...content.attachments], + }); + return { skippedAttachmentCount: 0 }; + }), + selectAttachments: (incoming) => ({ + status: "ready", + attachments: incoming.attachments, + warnings: [], + }), + releaseUnusedAttachments: vi.fn(), + onWarnings: vi.fn(), + onImportFailure: vi.fn(), + onCancelFailure: vi.fn(), + leave: vi.fn(), + ...overrides, + }; + const imports = makeIncomingShareImports(deps); + const watch = (target: ShareImportTarget) => { + const atom = imports.phaseAtom(target); + const unmount = registry.mount(atom); + registry.get(atom); + return { phase: (): ShareImportPhase => registry.get(atom), unmount }; + }; + return { imports, deps, drafts, inbox, calls, watch, registry }; +} + +describe("transitionShareImport", () => { + it("only cancels or retries from a failure, and keeps terminal phases terminal", () => { + expect(transitionShareImport("attaching", "cancel")).toBe("attaching"); + expect(transitionShareImport("failed", "start")).toBe("reserving"); + expect(transitionShareImport("cancelling", "cancelFailed")).toBe("failed"); + expect(transitionShareImport("done", "start")).toBe("done"); + expect(transitionShareImport("cancelled", "start")).toBe("cancelled"); + }); +}); + +describe("incoming share imports", () => { + it("reserves, attaches and consumes once, releasing files the draft did not keep", async () => { + const kept = fileAttachment("kept"); + const skipped = fileAttachment("skipped"); + const { deps, calls, drafts, inbox, watch } = setup({ + selectAttachments: () => ({ status: "ready", attachments: [kept], warnings: ["too big"] }), + }); + inbox.set("share-1", share([kept, skipped])); + const watched = watch(importTarget()); + + expect(watched.phase()).toBe("reserving"); + await vi.waitFor(() => expect(watched.phase()).toBe("done")); + + expect(calls).toEqual(["reserve", "merge", "consume"]); + expect(drafts.get("draft-a")?.attachments).toEqual([kept]); + expect(deps.releaseUnusedAttachments).toHaveBeenCalledWith([skipped]); + expect(deps.onWarnings).toHaveBeenCalledWith(["too big"]); + }); + + it("runs once when its atom node is removed and rebuilt", async () => { + const { imports, deps } = setup(); + const tasks: Array<() => void> = []; + const registry = AtomRegistry.make({ + scheduleTask: (task) => { + tasks.push(task); + return () => undefined; + }, + }); + const atom = imports.phaseAtom(importTarget()); + + registry.get(atom); + tasks.splice(0).forEach((task) => task()); + expect(registry.getNodes().size).toBe(0); + const unmount = registry.mount(atom); + await vi.waitFor(() => expect(registry.get(atom)).toBe("done")); + unmount(); + + expect(deps.reserve).toHaveBeenCalledTimes(1); + expect(deps.consume).toHaveBeenCalledTimes(1); + }); + + it("cancel after a failed consume restores the draft, releases the reservation and leaves", async () => { + const { deps, calls, drafts, watch, imports } = setup({ + consume: vi.fn(async () => { + throw new Error("disk full"); + }), + }); + const target = importTarget(); + const watched = watch(target); + await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + expect(deps.onImportFailure).toHaveBeenCalledWith(target, new Error("disk full")); + + await imports.cancel(target); + + expect(watched.phase()).toBe("cancelled"); + expect(calls).toEqual(["reserve", "merge", "restore", "release"]); + expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); + // Unconsumed: the inbox still owns the files. + expect(deps.releaseUnusedAttachments).not.toHaveBeenCalled(); + expect(deps.leave).toHaveBeenCalledWith(target); + }); + + it("a failed cancel can be retried and keeps the original backup", async () => { + let releaseAttempts = 0; + const { deps, drafts, watch, imports } = setup({ + consume: vi.fn(async () => { + throw new Error("disk full"); + }), + release: vi.fn(async () => { + releaseAttempts += 1; + if (releaseAttempts === 1) throw new Error("locked"); + }), + }); + const target = importTarget(); + const watched = watch(target); + await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + + await imports.cancel(target); + expect(watched.phase()).toBe("failed"); + expect(deps.onCancelFailure).toHaveBeenCalledWith(target, new Error("locked")); + expect(deps.leave).not.toHaveBeenCalled(); + + await imports.cancel(target); + expect(watched.phase()).toBe("cancelled"); + expect(deps.restoreDraft).toHaveBeenCalledTimes(1); + expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); + expect(deps.leave).toHaveBeenCalledTimes(1); + }); + + it("stops before consuming once its route leaves the stack", async () => { + const merged = Promise.withResolvers<{ readonly skippedAttachmentCount: number }>(); + const { deps, imports, watch } = setup({ mergeIntoDraft: () => merged.promise }); + const watched = watch(importTarget()); + await vi.waitFor(() => expect(watched.phase()).toBe("attaching")); + + const pruned = imports.prune(new Set(["flow-b"])); + merged.resolve({ skippedAttachmentCount: 0 }); + await pruned; + + expect(deps.consume).not.toHaveBeenCalled(); + expect(deps.onImportFailure).not.toHaveBeenCalled(); + watched.unmount(); + }); + + it("a failed import keeps its cancel after its route left, and reports a failed undo", async () => { + const { calls, deps, imports, watch } = setup({ + consume: vi.fn(async () => { + throw new Error("disk full"); + }), + release: vi.fn(async () => { + throw new Error("locked"); + }), + }); + const target = importTarget(); + const watched = watch(target); + await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + watched.unmount(); + await imports.prune(new Set()); + + await imports.cancel(target); + + expect(calls).toEqual(["reserve", "merge", "restore"]); + expect(deps.onCancelFailure).toHaveBeenCalledWith(target, new Error("locked")); + expect(deps.leave).not.toHaveBeenCalled(); + }); + + it("a route switching drafts abandons the in-flight import", async () => { + const reserved = Promise.withResolvers(); + const { deps, watch } = setup({ + reserve: vi.fn().mockReturnValueOnce(reserved.promise).mockResolvedValue(undefined), + }); + watch(importTarget()).unmount(); + const second = watch(importTarget({ draftKey: "draft-b" })); + reserved.resolve(); + + await vi.waitFor(() => expect(second.phase()).toBe("done")); + expect(deps.mergeIntoDraft).toHaveBeenCalledTimes(1); + expect(deps.mergeIntoDraft).toHaveBeenCalledWith("draft-b", expect.anything()); + }); +}); diff --git a/apps/mobile/src/features/threads/incomingShareImport.ts b/apps/mobile/src/features/threads/incomingShareImport.ts new file mode 100644 index 0000000000000..34569226eb9d6 --- /dev/null +++ b/apps/mobile/src/features/threads/incomingShareImport.ts @@ -0,0 +1,361 @@ +import { Atom, type AtomRegistry } from "effect/reactivity"; + +import type { DraftComposerAttachment } from "../../lib/composerImages"; +import type { ComposerDraft } from "../../state/use-composer-drafts"; +import type { + IncomingShareDestination, + IncomingShareDraft, + selectIncomingShareAttachmentsForServer, +} from "../sharing/incoming-share-model"; + +export type ShareImportPhase = + | "idle" + | "reserving" + | "attaching" + | "consuming" + | "done" + | "failed" + | "cancelling" + | "cancelled"; + +export type ShareImportEvent = + | "start" + | "reserved" + | "attached" + | "consumed" + | "failed" + | "cancel" + | "cancelFailed" + | "cancelled"; + +const TRANSITIONS: Readonly< + Record>> +> = { + idle: { start: "reserving" }, + reserving: { reserved: "attaching", failed: "failed" }, + attaching: { attached: "consuming", failed: "failed" }, + consuming: { consumed: "done", failed: "failed" }, + done: {}, + // "start" from failed is a retry. + failed: { start: "reserving", cancel: "cancelling" }, + cancelling: { cancelled: "cancelled", cancelFailed: "failed" }, + cancelled: {}, +}; + +/** Events that do not apply to the current phase leave it unchanged. */ +export function transitionShareImport( + phase: ShareImportPhase, + event: ShareImportEvent, +): ShareImportPhase { + return TRANSITIONS[phase][event] ?? phase; +} + +export function isShareImportInFlight(phase: ShareImportPhase): boolean { + return phase === "reserving" || phase === "attaching" || phase === "consuming"; +} + +/** + * One draft route importing one share into one draft. Plain data, compared + * structurally, so the screen can build it on every render. + */ +export interface ShareImportTarget { + /** The draft route's flow key; the import is abandoned once it leaves the stack. */ + readonly flowKey: string; + readonly routeKey: string; + readonly draftKey: string; + readonly destination: IncomingShareDestination; + readonly shareId: string; +} + +export interface ShareImportDeps { + readonly registry: AtomRegistry.AtomRegistry; + /** The share as the inbox holds it now. */ + readonly getShare: (shareId: string) => IncomingShareDraft | null; + readonly reserve: (shareId: string, destination: IncomingShareDestination) => Promise; + readonly release: (shareId: string, destination: IncomingShareDestination) => Promise; + readonly consume: (shareId: string) => Promise; + readonly readDraft: (draftKey: string) => ComposerDraft; + readonly restoreDraft: (draftKey: string, snapshot: ComposerDraft) => Promise; + readonly mergeIntoDraft: ( + draftKey: string, + content: { + readonly text: string; + readonly attachments: ReadonlyArray; + readonly sourceShareId: string; + }, + ) => Promise<{ readonly skippedAttachmentCount: number }>; + readonly selectAttachments: ( + share: IncomingShareDraft, + environmentId: string, + ) => ReturnType; + /** Sweeps files no draft retained; the sweep re-checks ownership. */ + readonly releaseUnusedAttachments: (attachments: ReadonlyArray) => void; + readonly onWarnings: (warnings: ReadonlyArray) => void; + readonly onImportFailure: (target: ShareImportTarget, error: unknown) => void; + /** Reported even after the route left: the undo is still incomplete. */ + readonly onCancelFailure: (target: ShareImportTarget, error: unknown) => void; + /** Leaves the draft route once its import is cancelled. */ + readonly leave: (target: ShareImportTarget) => void; +} + +interface Entry { + readonly target: ShareImportTarget; + phase: ShareImportPhase; + readonly controller: AbortController; + /** Set when another draft of the same route took over before this import finished. */ + superseded: boolean; + /** Set when the route left while a failed import still waits for retry or cancel. */ + detached: boolean; + run: Promise; + /** The draft before the first attempt merged into it; cancel restores it. */ + backup: ComposerDraft | null; + reservedFor: IncomingShareDestination | null; + consumed: boolean; + needsRestore: boolean; + readonly listeners: Set<() => void>; +} + +function keyOf(target: ShareImportTarget): string { + return JSON.stringify([ + target.flowKey, + target.routeKey, + target.draftKey, + target.destination.environmentId, + target.destination.projectId, + target.shareId, + ]); +} + +interface ImportedShare { + readonly flowKey: string; + readonly shareId: string; +} + +/** + * Share imports for draft routes. Imports do durable writes, so each is tied to + * its target, not an atom node that can be rebuilt. Reading a target's phase atom + * starts its import at most once; alerts, navigation and notifications all + * follow an await, never run inside the read. + */ +export function makeIncomingShareImports(deps: ShareImportDeps) { + const entries = new Map(); + // Shares each route already moved into a draft. Sending clears the draft's + // import receipt a frame before the route leaves, so the route needs its + // own record to tell "imported" from "vanished". + const importedAtom = Atom.make>([]).pipe( + Atom.keepAlive, + Atom.withLabel("mobile:incoming-share-imported"), + ); + const isImportedAtom = Atom.family((key: string) => + Atom.make((get) => + get(importedAtom).some( + (imported) => JSON.stringify([imported.flowKey, imported.shareId]) === key, + ), + ).pipe(Atom.withLabel("mobile:incoming-share-imported:route")), + ); + + function notify(entry: Entry): void { + // Listeners rebuild their node, which re-registers them; iterate a copy. + for (const listener of Array.from(entry.listeners)) listener(); + } + + function advance(entry: Entry, event: ShareImportEvent): void { + const next = transitionShareImport(entry.phase, event); + if (next === entry.phase) return; + entry.phase = next; + notify(entry); + } + + function settle(key: string, entry: Entry): void { + if (entry.detached && entries.get(key) === entry) entries.delete(key); + } + + async function runImport(key: string, entry: Entry): Promise { + const { signal } = entry.controller; + const { shareId, destination, draftKey } = entry.target; + // A phase atom node of a superseded import may still be cached; rebuilding + // it lets a route that switches back start over instead of showing the + // abandoned import's last phase. + const stopped = () => { + if (!signal.aborted) return false; + if (entry.superseded) notify(entry); + return true; + }; + try { + await deps.reserve(shareId, destination); + entry.reservedFor = destination; + // The durable reservation makes an interrupted transfer resume only in + // this project instead of copying into a second project draft. + if (stopped()) return; + advance(entry, "reserved"); + const share = deps.getShare(shareId); + if (!share) throw new Error("The shared content is no longer in the inbox."); + const selected = deps.selectAttachments(share, destination.environmentId); + if (selected.status === "pending") { + throw new Error("Server attachment support is still loading."); + } + entry.needsRestore = true; + const { skippedAttachmentCount } = await deps.mergeIntoDraft(draftKey, { + text: share.text, + attachments: selected.attachments, + sourceShareId: shareId, + }); + if (stopped()) return; + advance(entry, "attached"); + await deps.consume(shareId); + entry.consumed = true; + // The consumed inbox draft was the last owner of files that never made + // it into the composer draft (unsupported server, oversize, limit + // skips). Release them even when the route left meanwhile. + const retainedIds = new Set( + deps.readDraft(draftKey).attachments.map((attachment) => attachment.id), + ); + deps.releaseUnusedAttachments( + share.attachments.filter((attachment) => !retainedIds.has(attachment.id)), + ); + deps.registry.set(importedAtom, [ + ...deps.registry.get(importedAtom), + { flowKey: entry.target.flowKey, shareId }, + ]); + advance(entry, "consumed"); + entry.backup = null; + settle(key, entry); + if (stopped()) return; + const warnings = [...share.warnings, ...selected.warnings]; + if (skippedAttachmentCount > 0) { + warnings.push( + `${skippedAttachmentCount} shared file${skippedAttachmentCount === 1 ? " was" : "s were"} skipped because this draft reached the attachment limit.`, + ); + } + if (warnings.length > 0) deps.onWarnings(warnings); + } catch (error) { + if (stopped()) return; + advance(entry, "failed"); + deps.onImportFailure(entry.target, error); + } + } + + function begin(key: string, entry: Entry): void { + entry.backup ??= deps.readDraft(entry.target.draftKey); + advance(entry, "start"); + entry.run = runImport(key, entry); + } + + function abandon(key: string, entry: Entry): void { + entry.controller.abort(); + entries.delete(key); + } + + function start(target: ShareImportTarget): Entry { + const key = keyOf(target); + const existing = entries.get(key); + if (existing) return existing; + // A route imports into one draft at a time; a superseded import stops at + // its next step and no longer reports failures. + for (const [otherKey, other] of entries) { + if (other.target.flowKey === target.flowKey && isShareImportInFlight(other.phase)) { + other.superseded = true; + abandon(otherKey, other); + } + } + const entry: Entry = { + target, + phase: "idle", + controller: new AbortController(), + superseded: false, + detached: false, + run: Promise.resolve(), + backup: null, + reservedFor: null, + consumed: false, + needsRestore: false, + listeners: new Set(), + }; + entries.set(key, entry); + begin(key, entry); + return entry; + } + + async function cancel(target: ShareImportTarget): Promise { + const key = keyOf(target); + const entry = entries.get(key); + if (!entry || entry.phase !== "failed") return; + const { draftKey, shareId } = entry.target; + advance(entry, "cancel"); + // Cancelling finishes even if the route leaves meanwhile: it only undoes + // durable writes this import made. + try { + if (entry.needsRestore && entry.backup) { + // The restore drops the share's merged-in attachments from the draft. + // Sweep them only when the inbox entry was consumed: before that, the + // inbox still references these files and keeps them for a later import. + const mergedAttachments = deps.readDraft(draftKey).attachments; + await deps.restoreDraft(draftKey, entry.backup); + entry.needsRestore = false; + if (entry.consumed) deps.releaseUnusedAttachments(mergedAttachments); + } + if (entry.reservedFor) { + await deps.release(shareId, entry.reservedFor); + entry.reservedFor = null; + } + advance(entry, "cancelled"); + if (entry.detached) settle(key, entry); + else deps.leave(entry.target); + } catch (error) { + advance(entry, "cancelFailed"); + deps.onCancelFailure(entry.target, error); + } + } + + return { + phaseAtom: Atom.family((target: ShareImportTarget) => + Atom.make((get): ShareImportPhase => { + const entry = start(target); + const rebuild = () => get.refreshSelf(); + entry.listeners.add(rebuild); + get.addFinalizer(() => entry.listeners.delete(rebuild)); + return entry.phase; + }).pipe(Atom.withLabel("mobile:incoming-share-import")), + ), + /** Whether this route already moved the share into one of its drafts. */ + importedAtom(flowKey: string, shareId: string) { + return isImportedAtom(JSON.stringify([flowKey, shareId])); + }, + retry(target: ShareImportTarget): void { + const key = keyOf(target); + const entry = entries.get(key); + if (!entry || entry.phase !== "failed") return; + begin(key, entry); + }, + cancel, + /** + * Abandons the imports of draft routes that are no longer on the stack. + * A failed import keeps its retry and cancel until the user answers. + * Resolves once the abandoned imports have stopped. + */ + prune(liveFlowKeys: ReadonlySet): Promise { + const stopping: Array> = []; + for (const [key, entry] of entries) { + if (liveFlowKeys.has(entry.target.flowKey)) continue; + if (entry.phase === "failed" || entry.phase === "cancelling") { + entry.detached = true; + } else { + abandon(key, entry); + stopping.push(entry.run); + } + } + const imported = deps.registry.get(importedAtom); + const kept = imported.filter((share) => liveFlowKeys.has(share.flowKey)); + if (kept.length !== imported.length) deps.registry.set(importedAtom, kept); + return Promise.all(stopping).then(() => undefined); + }, + }; +} + +export const idleShareImportPhaseAtom = Atom.make("idle").pipe( + Atom.withLabel("mobile:incoming-share-import:idle"), +); + +export const idleImportedShareAtom = Atom.make(false).pipe( + Atom.withLabel("mobile:incoming-share-imported:idle"), +); From d294836c82fe6e8e6b0958e2e26a4ac061a6a971 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:03:26 -0700 Subject: [PATCH 2/4] fix(mobile): share import keeps the new-task composer locked until it finishes The new-task screen read whether a share import was running from the phase of the target it rendered. When that target went null mid-import (server config dropped on disconnect, or the selected project stopped matching the route's project), Start unlocked while the import kept merging into the draft. The share imports now track running attempts per draft route, abandoned ones included, and the screen locks on that. The machine tests await each attempt's run promise or the Deferred that marks the step, instead of polling with vi.waitFor. Co-Authored-By: Claude Opus 5.5 --- .../features/threads/NewTaskDraftScreen.tsx | 14 ++--- .../threads/incomingShareImport.test.ts | 63 +++++++++++++++---- .../features/threads/incomingShareImport.ts | 34 ++++++++-- 3 files changed, 88 insertions(+), 23 deletions(-) diff --git a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx index 15b2b4e6591e0..153497ff505b2 100644 --- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx @@ -88,11 +88,7 @@ import { resolveNewTaskWorkspaceLabel, } from "./new-task-context-presentation"; import { incomingShareImports, useIncomingShare } from "../sharing/IncomingShareProvider"; -import { - idleImportedShareAtom, - idleShareImportPhaseAtom, - isShareImportInFlight, -} from "./incomingShareImport"; +import { idleShareFlagAtom, idleShareImportPhaseAtom } from "./incomingShareImport"; import { newTaskFlowKey } from "./new-task-flow-state"; import { appAtomRegistry } from "../../state/atom-registry"; @@ -282,7 +278,7 @@ export function NewTaskDraftScreen(props: { const routeImportedShare = useAtomValue( flowKey !== undefined && props.incomingShareId ? incomingShareImports.importedAtom(flowKey, props.incomingShareId) - : idleImportedShareAtom, + : idleShareFlagAtom, ); const hasImportedIncomingShare = Boolean( props.incomingShareId && @@ -325,7 +321,11 @@ export function NewTaskDraftScreen(props: { ? idleShareImportPhaseAtom : incomingShareImports.phaseAtom(shareImportTarget), ); - const isImportingShare = isShareImportInFlight(shareImportPhase); + // Locks on the route, not the target: the target turns null when server config + // drops or the project stops matching, while the import keeps writing the draft. + const isImportingShare = useAtomValue( + flowKey === undefined ? idleShareFlagAtom : incomingShareImports.inFlightAtom(flowKey), + ); const isIncomingShareTransferPending = Boolean( incomingShare && shareImportPhase !== "cancelled" && !isIncomingShareAwaitingServerConfig, ); diff --git a/apps/mobile/src/features/threads/incomingShareImport.test.ts b/apps/mobile/src/features/threads/incomingShareImport.test.ts index 079cb3796a1f8..08bd0a076c2a8 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.test.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.test.ts @@ -114,14 +114,16 @@ describe("incoming share imports", () => { it("reserves, attaches and consumes once, releasing files the draft did not keep", async () => { const kept = fileAttachment("kept"); const skipped = fileAttachment("skipped"); - const { deps, calls, drafts, inbox, watch } = setup({ + const { deps, calls, drafts, inbox, imports, watch } = setup({ selectAttachments: () => ({ status: "ready", attachments: [kept], warnings: ["too big"] }), }); inbox.set("share-1", share([kept, skipped])); - const watched = watch(importTarget()); + const target = importTarget(); + const watched = watch(target); expect(watched.phase()).toBe("reserving"); - await vi.waitFor(() => expect(watched.phase()).toBe("done")); + await imports.settled(target); + expect(watched.phase()).toBe("done"); expect(calls).toEqual(["reserve", "merge", "consume"]); expect(drafts.get("draft-a")?.attachments).toEqual([kept]); @@ -144,7 +146,8 @@ describe("incoming share imports", () => { tasks.splice(0).forEach((task) => task()); expect(registry.getNodes().size).toBe(0); const unmount = registry.mount(atom); - await vi.waitFor(() => expect(registry.get(atom)).toBe("done")); + await imports.settled(importTarget()); + expect(registry.get(atom)).toBe("done"); unmount(); expect(deps.reserve).toHaveBeenCalledTimes(1); @@ -159,7 +162,8 @@ describe("incoming share imports", () => { }); const target = importTarget(); const watched = watch(target); - await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + await imports.settled(target); + expect(watched.phase()).toBe("failed"); expect(deps.onImportFailure).toHaveBeenCalledWith(target, new Error("disk full")); await imports.cancel(target); @@ -185,7 +189,8 @@ describe("incoming share imports", () => { }); const target = importTarget(); const watched = watch(target); - await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + await imports.settled(target); + expect(watched.phase()).toBe("failed"); await imports.cancel(target); expect(watched.phase()).toBe("failed"); @@ -200,10 +205,17 @@ describe("incoming share imports", () => { }); it("stops before consuming once its route leaves the stack", async () => { + const mergeStarted = Promise.withResolvers(); const merged = Promise.withResolvers<{ readonly skippedAttachmentCount: number }>(); - const { deps, imports, watch } = setup({ mergeIntoDraft: () => merged.promise }); + const { deps, imports, watch } = setup({ + mergeIntoDraft: () => { + mergeStarted.resolve(); + return merged.promise; + }, + }); const watched = watch(importTarget()); - await vi.waitFor(() => expect(watched.phase()).toBe("attaching")); + await mergeStarted.promise; + expect(watched.phase()).toBe("attaching"); const pruned = imports.prune(new Set(["flow-b"])); merged.resolve({ skippedAttachmentCount: 0 }); @@ -225,7 +237,8 @@ describe("incoming share imports", () => { }); const target = importTarget(); const watched = watch(target); - await vi.waitFor(() => expect(watched.phase()).toBe("failed")); + await imports.settled(target); + expect(watched.phase()).toBe("failed"); watched.unmount(); await imports.prune(new Set()); @@ -238,15 +251,41 @@ describe("incoming share imports", () => { it("a route switching drafts abandons the in-flight import", async () => { const reserved = Promise.withResolvers(); - const { deps, watch } = setup({ + const { deps, imports, watch } = setup({ reserve: vi.fn().mockReturnValueOnce(reserved.promise).mockResolvedValue(undefined), }); watch(importTarget()).unmount(); - const second = watch(importTarget({ draftKey: "draft-b" })); + const secondTarget = importTarget({ draftKey: "draft-b" }); + const second = watch(secondTarget); reserved.resolve(); - await vi.waitFor(() => expect(second.phase()).toBe("done")); + await imports.settled(secondTarget); + expect(second.phase()).toBe("done"); expect(deps.mergeIntoDraft).toHaveBeenCalledTimes(1); expect(deps.mergeIntoDraft).toHaveBeenCalledWith("draft-b", expect.anything()); }); + + it("stays in flight for its route after the screen stops rendering its target", async () => { + const mergeStarted = Promise.withResolvers(); + const merged = Promise.withResolvers<{ readonly skippedAttachmentCount: number }>(); + const { imports, registry, watch } = setup({ + mergeIntoDraft: () => { + mergeStarted.resolve(); + return merged.promise; + }, + }); + const inFlight = imports.inFlightAtom("flow-a"); + const unmountInFlight = registry.mount(inFlight); + const target = importTarget(); + const watched = watch(target); + await mergeStarted.promise; + + watched.unmount(); + expect(registry.get(inFlight)).toBe(true); + + merged.resolve({ skippedAttachmentCount: 0 }); + await imports.settled(target); + expect(registry.get(inFlight)).toBe(false); + unmountInFlight(); + }); }); diff --git a/apps/mobile/src/features/threads/incomingShareImport.ts b/apps/mobile/src/features/threads/incomingShareImport.ts index 34569226eb9d6..d5d349e69f140 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.ts @@ -50,7 +50,7 @@ export function transitionShareImport( return TRANSITIONS[phase][event] ?? phase; } -export function isShareImportInFlight(phase: ShareImportPhase): boolean { +function isShareImportInFlight(phase: ShareImportPhase): boolean { return phase === "reserving" || phase === "attaching" || phase === "consuming"; } @@ -153,6 +153,17 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { ), ).pipe(Atom.withLabel("mobile:incoming-share-imported:route")), ); + // One flow key per running attempt, abandoned ones included: an abandoned + // attempt can still be inside a draft write when the screen's target goes away. + const runningAtom = Atom.make>([]).pipe( + Atom.keepAlive, + Atom.withLabel("mobile:incoming-share-import-running"), + ); + const isRunningAtom = Atom.family((flowKey: string) => + Atom.make((get) => get(runningAtom).includes(flowKey)).pipe( + Atom.withLabel("mobile:incoming-share-import-running:route"), + ), + ); function notify(entry: Entry): void { // Listeners rebuild their node, which re-registers them; iterate a copy. @@ -238,7 +249,13 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { function begin(key: string, entry: Entry): void { entry.backup ??= deps.readDraft(entry.target.draftKey); advance(entry, "start"); - entry.run = runImport(key, entry); + const { flowKey } = entry.target; + deps.registry.set(runningAtom, [...deps.registry.get(runningAtom), flowKey]); + entry.run = runImport(key, entry).finally(() => { + const running = [...deps.registry.get(runningAtom)]; + running.splice(running.indexOf(flowKey), 1); + deps.registry.set(runningAtom, running); + }); } function abandon(key: string, entry: Entry): void { @@ -317,6 +334,15 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { return entry.phase; }).pipe(Atom.withLabel("mobile:incoming-share-import")), ), + /** + * Whether an import for this route is still running, including one whose + * target the screen no longer renders. + */ + inFlightAtom: isRunningAtom, + /** Resolves once the target's current attempt stops. */ + settled(target: ShareImportTarget): Promise { + return entries.get(keyOf(target))?.run ?? Promise.resolve(); + }, /** Whether this route already moved the share into one of its drafts. */ importedAtom(flowKey: string, shareId: string) { return isImportedAtom(JSON.stringify([flowKey, shareId])); @@ -356,6 +382,6 @@ export const idleShareImportPhaseAtom = Atom.make("idle").pipe Atom.withLabel("mobile:incoming-share-import:idle"), ); -export const idleImportedShareAtom = Atom.make(false).pipe( - Atom.withLabel("mobile:incoming-share-imported:idle"), +export const idleShareFlagAtom = Atom.make(false).pipe( + Atom.withLabel("mobile:incoming-share-flag:idle"), ); From 6292d3b7a10461f0e8c080a6f4736666fb6029db Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 00:22:52 -0700 Subject: [PATCH 3/4] fix(mobile): a retried share-import cancel keeps later draft edits restoreDraft shows the backup before it persists it. When persisting failed, the import still needed a restore, so retrying the cancel put the backup back over whatever the draft held by then: an edit made after the restore appeared was lost, and a deleted draft came back. A cancel now restores only over the draft the import's merge left, or re-persists a backup that is already showing. Any other draft is left as the user left it. Co-Authored-By: Claude Opus 5.5 --- .../threads/incomingShareImport.test.ts | 35 +++++++++++++++++++ .../features/threads/incomingShareImport.ts | 31 ++++++++++++---- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/apps/mobile/src/features/threads/incomingShareImport.test.ts b/apps/mobile/src/features/threads/incomingShareImport.test.ts index 08bd0a076c2a8..213afd3716ce4 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.test.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.test.ts @@ -13,6 +13,7 @@ import { } from "./incomingShareImport"; const destination = { environmentId: "env-1", projectId: "project-1" }; +const edited: ComposerDraft = { text: "typed after the restore", attachments: [] }; function fileAttachment(id: string): DraftComposerAttachment { return { @@ -226,6 +227,40 @@ describe("incoming share imports", () => { watched.unmount(); }); + it.each([ + ["edited", (drafts: Map) => drafts.set("draft-a", edited)], + ["deleted", (drafts: Map) => drafts.delete("draft-a")], + ] as const)( + "a retried cancel keeps a draft the user %s after the restore was shown", + async (_change, change) => { + let restoreAttempts = 0; + const { deps, drafts, watch, imports } = setup({ + consume: vi.fn(async () => { + throw new Error("disk full"); + }), + // Like restoreComposerDraftSnapshot: publishes, then fails to persist. + restoreDraft: vi.fn(async (draftKey: string, snapshot: ComposerDraft) => { + restoreAttempts += 1; + drafts.set(draftKey, snapshot); + if (restoreAttempts === 1) throw new Error("write failed"); + }), + }); + const target = importTarget(); + const watched = watch(target); + await imports.settled(target); + await imports.cancel(target); + expect(deps.onCancelFailure).toHaveBeenCalledWith(target, new Error("write failed")); + + change(drafts); + const expected = drafts.get("draft-a"); + await imports.cancel(target); + + expect(watched.phase()).toBe("cancelled"); + expect(drafts.get("draft-a")).toBe(expected); + expect(deps.release).toHaveBeenCalledTimes(1); + }, + ); + it("a failed import keeps its cancel after its route left, and reports a failed undo", async () => { const { calls, deps, imports, watch } = setup({ consume: vi.fn(async () => { diff --git a/apps/mobile/src/features/threads/incomingShareImport.ts b/apps/mobile/src/features/threads/incomingShareImport.ts index d5d349e69f140..973c2893b683c 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.ts @@ -109,12 +109,22 @@ interface Entry { run: Promise; /** The draft before the first attempt merged into it; cancel restores it. */ backup: ComposerDraft | null; + /** The draft as the last merge left it; cancel only restores over this. */ + merged: ComposerDraft | null; reservedFor: IncomingShareDestination | null; consumed: boolean; needsRestore: boolean; readonly listeners: Set<() => void>; } +/** Drafts are replaced, never mutated, so field identity means unchanged. */ +function sameDraft(a: ComposerDraft, b: ComposerDraft): boolean { + const keys = new Set([...Object.keys(a), ...Object.keys(b)]); + return Array.from(keys).every( + (key) => a[key as keyof ComposerDraft] === b[key as keyof ComposerDraft], + ); +} + function keyOf(target: ShareImportTarget): string { return JSON.stringify([ target.flowKey, @@ -206,11 +216,13 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { throw new Error("Server attachment support is still loading."); } entry.needsRestore = true; + entry.merged = null; const { skippedAttachmentCount } = await deps.mergeIntoDraft(draftKey, { text: share.text, attachments: selected.attachments, sourceShareId: shareId, }); + entry.merged = deps.readDraft(draftKey); if (stopped()) return; advance(entry, "attached"); await deps.consume(shareId); @@ -283,6 +295,7 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { detached: false, run: Promise.resolve(), backup: null, + merged: null, reservedFor: null, consumed: false, needsRestore: false, @@ -303,13 +316,19 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { // durable writes this import made. try { if (entry.needsRestore && entry.backup) { - // The restore drops the share's merged-in attachments from the draft. - // Sweep them only when the inbox entry was consumed: before that, the - // inbox still references these files and keeps them for a later import. - const mergedAttachments = deps.readDraft(draftKey).attachments; - await deps.restoreDraft(draftKey, entry.backup); + const current = deps.readDraft(draftKey); + const merged = entry.merged ?? current; + // restoreDraft shows the backup before persisting it, so a retry after a + // failed restore may find the backup already shown: persist it again. + // Anything else is an edit or deletion made since; leave it. + if (sameDraft(current, merged) || sameDraft(current, entry.backup)) { + await deps.restoreDraft(draftKey, entry.backup); + // The restore drops the share's merged-in attachments from the draft. + // Sweep them only when the inbox entry was consumed: before that, the + // inbox still references these files and keeps them for a later import. + if (entry.consumed) deps.releaseUnusedAttachments(merged.attachments); + } entry.needsRestore = false; - if (entry.consumed) deps.releaseUnusedAttachments(mergedAttachments); } if (entry.reservedFor) { await deps.release(shareId, entry.reservedFor); From 33c080d627ea7a2c6bcafe5e2747037dc9ae23a7 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 01:20:31 -0700 Subject: [PATCH 4/4] fix(mobile): share-import cancel undoes only what the import merged Cancel compared the draft field by field with the merged draft and the backup, and skipped the restore when neither matched. An upload that finished while the failure alert was open replaced the attachment objects, so cancel left the share in the draft although the user had changed nothing. And when the merge published but failed to persist, the merged draft was never recorded: cancel then compared the draft with itself and put the backup over a later edit. Cancel now uses the draft store's undoComposerDraftMerge, as outbox recovery already does: it restores the backup when the draft is untouched and otherwise removes only the merged-in text and attachments (matched by id). The import records the merged draft in a finally, so a merge that published but did not persist is still undone against what it published. restoreComposerDraftSnapshot had no callers left and is removed. Co-Authored-By: Claude Opus 5.5 --- .../sharing/IncomingShareProvider.tsx | 4 +- .../threads/incomingShareImport.test.ts | 143 ++++++++++++++---- .../features/threads/incomingShareImport.ts | 60 ++++---- apps/mobile/src/state/use-composer-drafts.ts | 23 --- 4 files changed, 148 insertions(+), 82 deletions(-) diff --git a/apps/mobile/src/features/sharing/IncomingShareProvider.tsx b/apps/mobile/src/features/sharing/IncomingShareProvider.tsx index 184eaf15daa2b..6c03a4110da5b 100644 --- a/apps/mobile/src/features/sharing/IncomingShareProvider.tsx +++ b/apps/mobile/src/features/sharing/IncomingShareProvider.tsx @@ -35,8 +35,8 @@ import { serverEnvironment } from "../../state/server"; import { getComposerDraftSnapshot, mergeComposerDraftContent, - restoreComposerDraftSnapshot, scheduleUnusedComposerAttachmentCleanup, + undoComposerDraftMerge, } from "../../state/use-composer-drafts"; import { EMPTY_INCOMING_SHARE_PRESENTATION_STATE, @@ -302,7 +302,7 @@ export const incomingShareImports = makeIncomingShareImports({ release: releaseShareReservation, consume: consumeShare, readDraft: getComposerDraftSnapshot, - restoreDraft: restoreComposerDraftSnapshot, + undoMerge: undoComposerDraftMerge, mergeIntoDraft: mergeComposerDraftContent, selectAttachments: (share, environmentId) => selectIncomingShareAttachmentsForServer({ diff --git a/apps/mobile/src/features/threads/incomingShareImport.test.ts b/apps/mobile/src/features/threads/incomingShareImport.test.ts index 213afd3716ce4..f1fc503468827 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.test.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it, vi } from "vite-plus/test"; import type { DraftComposerAttachment } from "../../lib/composerImages"; import type { IncomingShareDraft } from "../sharing/incoming-share-model"; -import type { ComposerDraft } from "../../state/use-composer-drafts"; +import { undoComposerDraftMergeState, type ComposerDraft } from "../../state/use-composer-drafts"; import { makeIncomingShareImports, type ShareImportDeps, @@ -13,7 +13,34 @@ import { } from "./incomingShareImport"; const destination = { environmentId: "env-1", projectId: "project-1" }; -const edited: ComposerDraft = { text: "typed after the restore", attachments: [] }; +const edited: ComposerDraft = { text: "typed after the undo", attachments: [] }; + +/** The draft store's own undo, applied to the test's draft map. */ +function undoInto( + drafts: Map, + draftKey: string, + snapshot: ComposerDraft, + merged: ComposerDraft, +): void { + const next = undoComposerDraftMergeState(Object.fromEntries(drafts), draftKey, snapshot, merged); + drafts.clear(); + for (const [key, draft] of Object.entries(next)) drafts.set(key, draft); +} + +function mergeInto( + drafts: Map, + draftKey: string, + content: { + readonly text: string; + readonly attachments: ReadonlyArray; + }, +): void { + const existing = drafts.get(draftKey) ?? { text: "", attachments: [] }; + drafts.set(draftKey, { + text: `${existing.text}\n${content.text}`, + attachments: [...existing.attachments, ...content.attachments], + }); +} function fileAttachment(id: string): DraftComposerAttachment { return { @@ -66,17 +93,13 @@ function setup(overrides: Partial = {}) { calls.push("consume"); }), readDraft: (draftKey) => drafts.get(draftKey) ?? { text: "", attachments: [] }, - restoreDraft: vi.fn(async (draftKey, snapshot) => { - calls.push("restore"); - drafts.set(draftKey, snapshot); + undoMerge: vi.fn(async (draftKey, snapshot, merged) => { + calls.push("undo"); + undoInto(drafts, draftKey, snapshot, merged); }), mergeIntoDraft: vi.fn(async (draftKey, content) => { calls.push("merge"); - const existing = drafts.get(draftKey) ?? { text: "", attachments: [] }; - drafts.set(draftKey, { - text: `${existing.text}\n${content.text}`, - attachments: [...existing.attachments, ...content.attachments], - }); + mergeInto(drafts, draftKey, content); return { skippedAttachmentCount: 0 }; }), selectAttachments: (incoming) => ({ @@ -170,7 +193,7 @@ describe("incoming share imports", () => { await imports.cancel(target); expect(watched.phase()).toBe("cancelled"); - expect(calls).toEqual(["reserve", "merge", "restore", "release"]); + expect(calls).toEqual(["reserve", "merge", "undo", "release"]); expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); // Unconsumed: the inbox still owns the files. expect(deps.releaseUnusedAttachments).not.toHaveBeenCalled(); @@ -200,7 +223,7 @@ describe("incoming share imports", () => { await imports.cancel(target); expect(watched.phase()).toBe("cancelled"); - expect(deps.restoreDraft).toHaveBeenCalledTimes(1); + expect(deps.undoMerge).toHaveBeenCalledTimes(1); expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); expect(deps.leave).toHaveBeenCalledTimes(1); }); @@ -227,23 +250,27 @@ describe("incoming share imports", () => { watched.unmount(); }); - it.each([ - ["edited", (drafts: Map) => drafts.set("draft-a", edited)], - ["deleted", (drafts: Map) => drafts.delete("draft-a")], - ] as const)( - "a retried cancel keeps a draft the user %s after the restore was shown", - async (_change, change) => { - let restoreAttempts = 0; + const userChanges = [ + ["edited", (drafts: Map) => drafts.set("draft-a", edited), edited], + ["deleted", (drafts: Map) => drafts.delete("draft-a"), undefined], + ] as const; + + it.each(userChanges)( + "a retried cancel keeps a draft the user %s after the undo was shown", + async (_change, change, expected) => { + let undoAttempts = 0; const { deps, drafts, watch, imports } = setup({ consume: vi.fn(async () => { throw new Error("disk full"); }), - // Like restoreComposerDraftSnapshot: publishes, then fails to persist. - restoreDraft: vi.fn(async (draftKey: string, snapshot: ComposerDraft) => { - restoreAttempts += 1; - drafts.set(draftKey, snapshot); - if (restoreAttempts === 1) throw new Error("write failed"); - }), + // Like undoComposerDraftMerge: publishes, then fails to persist. + undoMerge: vi.fn( + async (draftKey: string, snapshot: ComposerDraft, merged: ComposerDraft) => { + undoAttempts += 1; + undoInto(drafts, draftKey, snapshot, merged); + if (undoAttempts === 1) throw new Error("write failed"); + }, + ), }); const target = importTarget(); const watched = watch(target); @@ -252,15 +279,75 @@ describe("incoming share imports", () => { expect(deps.onCancelFailure).toHaveBeenCalledWith(target, new Error("write failed")); change(drafts); - const expected = drafts.get("draft-a"); await imports.cancel(target); expect(watched.phase()).toBe("cancelled"); - expect(drafts.get("draft-a")).toBe(expected); + expect(drafts.get("draft-a")).toEqual(expected); expect(deps.release).toHaveBeenCalledTimes(1); }, ); + it("cancel undoes the import after an upload finished on its attachment", async () => { + const file = fileAttachment("file"); + const { drafts, inbox, watch, imports } = setup({ + consume: vi.fn(async () => { + throw new Error("disk full"); + }), + }); + inbox.set("share-1", share([file])); + const target = importTarget(); + watch(target); + await imports.settled(target); + + const merged = drafts.get("draft-a"); + drafts.set("draft-a", { + text: merged?.text ?? "", + attachments: [{ ...file, uploadedAttachmentId: "uploaded-1" }], + }); + await imports.cancel(target); + + expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); + }); + + it.each(userChanges)( + "cancel keeps a draft the user %s after a merge that published but failed to persist", + async (_change, change, expected) => { + const { drafts, watch, imports } = setup({ + // Like mergeComposerDraftContent: publishes, then fails to persist. + mergeIntoDraft: vi.fn(async (draftKey, content) => { + mergeInto(drafts, draftKey, content); + throw new Error("write failed"); + }), + }); + const target = importTarget(); + const watched = watch(target); + await imports.settled(target); + expect(watched.phase()).toBe("failed"); + + change(drafts); + await imports.cancel(target); + + expect(watched.phase()).toBe("cancelled"); + expect(drafts.get("draft-a")).toEqual(expected); + }, + ); + + it("cancel after a merge that published but failed to persist removes the merged share", async () => { + const { drafts, watch, imports } = setup({ + mergeIntoDraft: vi.fn(async (draftKey, content) => { + mergeInto(drafts, draftKey, content); + throw new Error("write failed"); + }), + }); + const target = importTarget(); + watch(target); + await imports.settled(target); + + await imports.cancel(target); + + expect(drafts.get("draft-a")).toEqual({ text: "mine", attachments: [] }); + }); + it("a failed import keeps its cancel after its route left, and reports a failed undo", async () => { const { calls, deps, imports, watch } = setup({ consume: vi.fn(async () => { @@ -279,7 +366,7 @@ describe("incoming share imports", () => { await imports.cancel(target); - expect(calls).toEqual(["reserve", "merge", "restore"]); + expect(calls).toEqual(["reserve", "merge", "undo"]); expect(deps.onCancelFailure).toHaveBeenCalledWith(target, new Error("locked")); expect(deps.leave).not.toHaveBeenCalled(); }); diff --git a/apps/mobile/src/features/threads/incomingShareImport.ts b/apps/mobile/src/features/threads/incomingShareImport.ts index 973c2893b683c..12373b1280bea 100644 --- a/apps/mobile/src/features/threads/incomingShareImport.ts +++ b/apps/mobile/src/features/threads/incomingShareImport.ts @@ -75,7 +75,15 @@ export interface ShareImportDeps { readonly release: (shareId: string, destination: IncomingShareDestination) => Promise; readonly consume: (shareId: string) => Promise; readonly readDraft: (draftKey: string) => ComposerDraft; - readonly restoreDraft: (draftKey: string, snapshot: ComposerDraft) => Promise; + /** + * Restores `snapshot` if the draft still equals `merged`; otherwise removes only + * what the merge added, keeping later edits. + */ + readonly undoMerge: ( + draftKey: string, + snapshot: ComposerDraft, + merged: ComposerDraft, + ) => Promise; readonly mergeIntoDraft: ( draftKey: string, content: { @@ -109,7 +117,7 @@ interface Entry { run: Promise; /** The draft before the first attempt merged into it; cancel restores it. */ backup: ComposerDraft | null; - /** The draft as the last merge left it; cancel only restores over this. */ + /** The draft as the last merge published it; cancel undoes against it. */ merged: ComposerDraft | null; reservedFor: IncomingShareDestination | null; consumed: boolean; @@ -117,14 +125,6 @@ interface Entry { readonly listeners: Set<() => void>; } -/** Drafts are replaced, never mutated, so field identity means unchanged. */ -function sameDraft(a: ComposerDraft, b: ComposerDraft): boolean { - const keys = new Set([...Object.keys(a), ...Object.keys(b)]); - return Array.from(keys).every( - (key) => a[key as keyof ComposerDraft] === b[key as keyof ComposerDraft], - ); -} - function keyOf(target: ShareImportTarget): string { return JSON.stringify([ target.flowKey, @@ -216,13 +216,18 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { throw new Error("Server attachment support is still loading."); } entry.needsRestore = true; - entry.merged = null; - const { skippedAttachmentCount } = await deps.mergeIntoDraft(draftKey, { - text: share.text, - attachments: selected.attachments, - sourceShareId: shareId, - }); - entry.merged = deps.readDraft(draftKey); + let skippedAttachmentCount: number; + try { + ({ skippedAttachmentCount } = await deps.mergeIntoDraft(draftKey, { + text: share.text, + attachments: selected.attachments, + sourceShareId: shareId, + })); + } finally { + // The merge publishes before it persists, so read it back even when + // persisting rejects. + entry.merged = deps.readDraft(draftKey); + } if (stopped()) return; advance(entry, "attached"); await deps.consume(shareId); @@ -316,19 +321,16 @@ export function makeIncomingShareImports(deps: ShareImportDeps) { // durable writes this import made. try { if (entry.needsRestore && entry.backup) { - const current = deps.readDraft(draftKey); - const merged = entry.merged ?? current; - // restoreDraft shows the backup before persisting it, so a retry after a - // failed restore may find the backup already shown: persist it again. - // Anything else is an edit or deletion made since; leave it. - if (sameDraft(current, merged) || sameDraft(current, entry.backup)) { - await deps.restoreDraft(draftKey, entry.backup); - // The restore drops the share's merged-in attachments from the draft. - // Sweep them only when the inbox entry was consumed: before that, the - // inbox still references these files and keeps them for a later import. - if (entry.consumed) deps.releaseUnusedAttachments(merged.attachments); - } + // The draft can change while a failed import waits for an answer, by the + // user or an upload finishing, and a retried undo runs after its own + // publish. Undoing against the merged draft keeps those changes. + const merged = entry.merged ?? entry.backup; + await deps.undoMerge(draftKey, entry.backup, merged); entry.needsRestore = false; + // Sweep the share's merged-in files only once the inbox entry was + // consumed: before that, the inbox still owns them for a later import. + // The sweep skips files a draft still holds. + if (entry.consumed) deps.releaseUnusedAttachments(merged.attachments); } if (entry.reservedFor) { await deps.release(shareId, entry.reservedFor); diff --git a/apps/mobile/src/state/use-composer-drafts.ts b/apps/mobile/src/state/use-composer-drafts.ts index 1c5cc26a08460..f81f39c3f429a 100644 --- a/apps/mobile/src/state/use-composer-drafts.ts +++ b/apps/mobile/src/state/use-composer-drafts.ts @@ -1716,29 +1716,6 @@ export async function mergeComposerDraftContent( return { skippedAttachmentCount }; } -export async function restoreComposerDraftSnapshot( - draftKey: string, - snapshot: ComposerDraft, -): Promise { - ensureComposerDraftsLoaded(); - if (loadPromise !== null) { - await loadPromise; - } - if (persistTimer !== null) { - clearTimeout(persistTimer); - persistTimer = null; - } - const next = restoreComposerDraftSnapshotState( - appAtomRegistry.get(composerDraftsAtom), - draftKey, - snapshot, - ); - appAtomRegistry.set(composerDraftsAtom, next); - await persistenceQueue.run(() => - writePersistedComposerState(next, appAtomRegistry.get(stickyComposerModelSelectionAtom)), - ); -} - export function sameComposerDraftState(a: ComposerDraft, b: ComposerDraft): boolean { return ( a.text === b.text &&