diff --git a/desktop/src/features/messages/ui/MessageComposer.tsx b/desktop/src/features/messages/ui/MessageComposer.tsx index ecdfb01431..ed43796906 100644 --- a/desktop/src/features/messages/ui/MessageComposer.tsx +++ b/desktop/src/features/messages/ui/MessageComposer.tsx @@ -196,33 +196,34 @@ function MessageComposerImpl({ media.queuedAttachmentsRef.current.length === 0; const ownsDropZone = mediaController === undefined; const backgroundUpload = useBackgroundMediaUpload(); - const { trackAuthoredContent } = useDraftPersistLifecycle({ - effectiveDraftKey, - channelId, - loadDraft: drafts.loadDraft, - persistDraft: drafts.persistDraft, - getMentionRefs: mentions.getDraftMentionRefs, - restoreMentionRefs: mentions.restoreDraftMentionRefs, - livePendingImeta: media.pendingImeta, - setPendingImeta: media.setPendingImeta, - getQueuedAttachments: () => media.queuedAttachmentsRef.current, - saveQueuedAttachmentsForDraft, - clearQueuedAttachments: media.clearQueuedAttachments, - restoreQueuedAttachments: media.restoreQueuedAttachments, - takeQueuedAttachmentsForDraft, - setContent: (content) => { - setComposerContent(content); - richText.setContent(content); - }, - clearContent: () => { - setComposerContent(""); - richText.clearContent(); - }, - setSpoileredAttachmentUrls, - spoileredAttachmentUrlsRef, - syncComposerContentFromEditor, - getImplicitAgentMentionPrefix: implicitAgentMentionProvenance.getPrefix, - }); + const { trackAuthoredContent, getComposerRevision, runComposerUpdate } = + useDraftPersistLifecycle({ + effectiveDraftKey, + channelId, + loadDraft: drafts.loadDraft, + persistDraft: drafts.persistDraft, + getMentionRefs: mentions.getDraftMentionRefs, + restoreMentionRefs: mentions.restoreDraftMentionRefs, + livePendingImeta: media.pendingImeta, + setPendingImeta: media.setPendingImeta, + getQueuedAttachments: () => media.queuedAttachmentsRef.current, + saveQueuedAttachmentsForDraft, + clearQueuedAttachments: media.clearQueuedAttachments, + restoreQueuedAttachments: media.restoreQueuedAttachments, + takeQueuedAttachmentsForDraft, + setContent: (content) => { + setComposerContent(content); + richText.setContent(content); + }, + clearContent: () => { + setComposerContent(""); + richText.clearContent(); + }, + setSpoileredAttachmentUrls, + spoileredAttachmentUrlsRef, + syncComposerContentFromEditor, + getImplicitAgentMentionPrefix: implicitAgentMentionProvenance.getPrefix, + }); // biome-ignore lint/correctness/useExhaustiveDependencies: effectiveDraftKey is the sole trigger React.useEffect(() => { media.setUploadState({ status: "idle" }); @@ -352,7 +353,10 @@ function MessageComposerImpl({ enabled: keepMentionedAgentsPinned, }); const mentionSendFlow = useMentionSendFlow({ + getComposerRevision, + runComposerUpdate, channelId, + effectiveDraftKey, channelLinks, channelType, contentRef, diff --git a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts index 857f4bf0e0..18b7327bdb 100644 --- a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts +++ b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts @@ -66,6 +66,10 @@ type UseDraftPersistLifecycleParams = { }; type UseDraftPersistLifecycleResult = { + /** Monotonic editor revision, including same-text edit/clear cycles. */ + getComposerRevision: () => number; + /** Optimistic send/recovery is not an authored edit or authoritative deletion. */ + runComposerUpdate: (update: () => void) => void; /** * Record the latest authored editor content. Empty content is persisted * immediately and remains authoritative across composer remounts until a @@ -135,9 +139,23 @@ export function useDraftPersistLifecycle({ [getImplicitAgentMentionPrefix], ); + const composerRevision = React.useRef(0); + const getComposerRevision = React.useCallback( + () => composerRevision.current, + [], + ); const pendingImetaForPersistRef = React.useRef([]); const emptyContentIsAuthoritativeRef = React.useRef(false); const isRestoringContentRef = React.useRef(false); + const runComposerUpdate = React.useCallback((update: () => void) => { + const wasRestoring = isRestoringContentRef.current; + isRestoringContentRef.current = true; + try { + update(); + } finally { + isRestoringContentRef.current = wasRestoring; + } + }, []); const restoredQueuedAttachmentsRef = React.useRef( [], ); @@ -221,7 +239,9 @@ export function useDraftPersistLifecycle({ const trackAuthoredContent = React.useCallback( (content: string) => { - if (!effectiveDraftKey || isRestoringContentRef.current) return; + if (isRestoringContentRef.current) return; + composerRevision.current += 1; + if (!effectiveDraftKey) return; const authoritativeDraftKey = scopedDraftKey(effectiveDraftKey); if (content.length > 0) { authoritativelyClearedDraftKeys.delete(authoritativeDraftKey); @@ -242,5 +262,5 @@ export function useDraftPersistLifecycle({ [channelId, effectiveDraftKey, persistDraft, spoileredAttachmentUrlsRef], ); - return { trackAuthoredContent }; + return { trackAuthoredContent, getComposerRevision, runComposerUpdate }; } diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs index 1909b2541c..17b578cd05 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -62,7 +62,7 @@ function load(name, stubs) { ); return exports; } -async function setup() { +async function setup({ lifecycle = false } = {}) { const { act, renderHook } = await import("@testing-library/react"); const calls = []; const control = { prepare: null, add: null, publish: null, inventory: null }; @@ -88,8 +88,21 @@ async function setup() { "@/features/agents/hooks": new Proxy( {}, { - get: (_, key) => - key.includes("Mutation") ? () => mutation : () => query, + get: (_, key) => { + if ( + key === "useCreateChannelManagedAgentMutation" || + key === "useProvisionChannelManagedAgentMutation" + ) + return () => ({ + isPending: false, + mutateAsync: async (input) => { + calls.push(["persona", input]); + if (control.persona) await control.persona.promise; + return { agent: { pubkey: KEY, name: "Fizz" } }; + }, + }); + return key.includes("Mutation") ? () => mutation : () => query; + }, }, ), "@/features/agents/channelAgents": { @@ -99,7 +112,9 @@ async function setup() { return agent; }, }, - "@/features/agents/lib/resolvePersonaRuntime": {}, + "@/features/agents/lib/resolvePersonaRuntime": { + resolvePersonaRuntime: () => ({ runtime: "test-runtime" }), + }, "@/features/channels/hooks": { useAddChannelMembersMutation: () => mutation, }, @@ -111,7 +126,15 @@ async function setup() { dmThreadAgentMentionError: () => null, }, "@/features/messages/lib/backgroundMediaUploadStore": { - saveQueuedAttachmentsForDraft: noop, + saveQueuedAttachmentsForDraft: (...args) => + calls.push(["save-queue", ...args]), + prepareBackgroundMediaUpload: () => ({ + start(callbacks) { + control.uploadCallbacks = callbacks; + return true; + }, + cancel: noop, + }), }, "@/features/messages/lib/imetaMediaMarkdown": { buildOutgoingMessage: (text) => ({ content: text, mediaTags: [] }), @@ -136,8 +159,48 @@ async function setup() { stubs, ); const { useMentionSendFlow } = load("useMentionSendFlow", stubs); + const store = new Map(); + const persistDraft = ( + key, + content, + channelId, + pendingImeta, + spoileredAttachmentUrls, + mentionRefs, + ) => { + calls.push([ + "persist", + key, + content, + channelId, + pendingImeta, + spoileredAttachmentUrls, + mentionRefs, + ]); + if (content || pendingImeta.length) + store.set(key, { + content, + channelId, + pendingImeta, + spoileredAttachmentUrls, + mentionRefs, + }); + else store.delete(key); + }; + const initialKey = lifecycle ? "thread:a" : "general"; + if (lifecycle) + store.set(initialKey, { + content: TEXT, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: refs, + }); const options = { channelId: "general", + effectiveDraftKey: initialKey, + getComposerRevision: () => 0, + runComposerUpdate: (update) => update(), channelType: "stream", customEmoji: [], mentions: { @@ -147,10 +210,20 @@ async function setup() { extractMentionPubkeys: () => [KEY], isAgentPubkey: (key) => key === KEY, isManagedAgentPubkey: () => false, - getDraftMentionRefs: () => refs, + getDraftMentionRefs: () => control.currentRefs ?? refs, + registerMentionPubkey: (displayName, pubkey, options) => { + const ref = { displayName, pubkey, isAgent: options.isAgent }; + control.currentRefs = [...(control.currentRefs ?? []), ref]; + calls.push(["register-ref", ref]); + }, getMentionDisplayName: () => "RemoteScout", - clearMentions: noop, - restoreDraftMentionRefs: (value) => calls.push(["restore-refs", value]), + clearMentions: () => { + if (lifecycle) control.currentRefs = []; + }, + restoreDraftMentionRefs: (value) => { + if (lifecycle) control.currentRefs = value; + calls.push(["restore-refs", value]); + }, revalidateMentionPubkeys: async (keys, channel, opts) => { calls.push([opts.phase, channel]); if (control[opts.phase]) await control[opts.phase].promise; @@ -160,10 +233,17 @@ async function setup() { contentRef: { current: TEXT }, channelLinks: { clearChannels: noop }, emojiAutocomplete: { clearEmojis: noop }, - richText: { clearContent: noop, setContent: noop }, + richText: { + clearContent: () => { + if (lifecycle) lifecycleApi.trackAuthoredContent(""); + }, + setContent: (text) => { + if (lifecycle) lifecycleApi.trackAuthoredContent(text); + }, + }, drafts: { - loadDraft: () => null, - persistDraft: (...args) => calls.push(["persist", ...args]), + loadDraft: (key) => (lifecycle ? store.get(key) : null), + persistDraft, markDraftSent: noop, }, setContent: noop, @@ -174,10 +254,47 @@ async function setup() { hasUnsavedMedia: () => false, onSendRef: { current: async (...args) => calls.push(["SEND", ...args]) }, }; - const hook = renderHook(() => useMentionSendFlow(options), { - wrapper: ({ children }) => - React.createElement(React.StrictMode, null, children), - }); + stubs["@/features/messages/lib/stripImplicitAgentMentions"] = { + stripImplicitAgentMentionPrefix: (text) => text, + }; + stubs["@/features/messages/lib/useDrafts"] = { + getDraftStoreScope: () => "test", + }; + const { useDraftPersistLifecycle } = load("useDraftPersistSnapshot", stubs); + let lifecycleApi; + const hook = renderHook( + () => { + if (lifecycle) { + // biome-ignore lint/correctness/useHookAtTopLevel: lifecycle is immutable for this harness mount + lifecycleApi = useDraftPersistLifecycle({ + effectiveDraftKey: options.effectiveDraftKey, + channelId: options.channelId, + loadDraft: options.drafts.loadDraft, + persistDraft, + getMentionRefs: options.mentions.getDraftMentionRefs, + restoreMentionRefs: options.mentions.restoreDraftMentionRefs, + livePendingImeta: [], + setPendingImeta: noop, + setContent: (text) => { + options.contentRef.current = text; + }, + clearContent: () => { + options.contentRef.current = ""; + }, + setSpoileredAttachmentUrls: noop, + spoileredAttachmentUrlsRef: { current: new Set() }, + syncComposerContentFromEditor: () => options.contentRef.current, + }); + options.getComposerRevision = lifecycleApi.getComposerRevision; + options.runComposerUpdate = lifecycleApi.runComposerUpdate; + } + return useMentionSendFlow(options); + }, + { + wrapper: ({ children }) => + React.createElement(React.StrictMode, null, children), + }, + ); const flush = async () => act(async () => { await new Promise((resolve) => setImmediate(resolve)); @@ -189,7 +306,14 @@ async function setup() { capturedChannelId: options.channelId, pendingImeta: [], trimmed: text, - recoveryDraftKey: "general", + recoveryDraftKey: options.effectiveDraftKey, + capturedThreadContext: lifecycle + ? { + parentEventId: options.effectiveDraftKey, + threadHeadId: options.effectiveDraftKey, + } + : null, + queuedAttachments: control.attachments ?? [], }); }); await prompt(); @@ -216,6 +340,17 @@ async function setup() { finish, flush, events, + store, + edit: (text, mentionRefs = refs) => + act(() => { + options.contentRef.current = text; + control.currentRefs = mentionRefs; + lifecycleApi.trackAuthoredContent(text); + }), + navigate: (key) => { + options.effectiveDraftKey = key; + hook.rerender(); + }, }; } @@ -349,3 +484,152 @@ test("cancelled invitation cannot attach a local recipient after delayed policy assert.equal(s.events("SEND").length, 0); assert.equal(s.options.contentRef.current, TEXT); }); + +// Real draft lifecycle + real send/Invite hooks share one reused StrictMode host. +// The editor and storage adapter are mocked; draft leave/restore ordering is not. +for (const stage of ["add", "publish"]) { + for (const incoming of [TEXT, "unrelated thread B draft"]) { + test(`same-channel ${stage}: incoming ${incoming === TEXT ? "same-text" : "different-text"} draft and exact refs survive`, async () => { + const s = await setup({ lifecycle: true }); + const otherRefs = [ + { displayName: "RemoteScout", pubkey: "c".repeat(64), isAgent: true }, + ]; + s.store.set("thread:b", { + content: incoming, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: otherRefs, + }); + const gate = deferred(); + s.control[stage] = gate; + await s.invite(); + s.navigate("thread:b"); + assert.equal(s.result.current.nonMemberPromptProps.open, false); + assert.equal(s.result.current.isPreparingMentionSend, false); + assert.equal(s.options.contentRef.current, incoming); + assert.deepEqual(s.control.currentRefs, otherRefs); + // Recovery must be available on return BEFORE the network responds. + s.navigate("thread:a"); + assert.equal(s.options.contentRef.current, TEXT); + assert.deepEqual(s.control.currentRefs, s.refs); + s.edit(TEXT, otherRefs); // identical visible text is a new exact selection + s.navigate("thread:b"); + await s.finish(gate); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.options.contentRef.current, incoming); + assert.deepEqual(s.control.currentRefs, otherRefs); + assert.deepEqual(s.store.get("thread:a").mentionRefs, otherRefs); + assert.deepEqual(s.store.get("thread:b").mentionRefs, otherRefs); + }); + } +} +test("late validation completion cannot restore over an authored empty draft or reset a newer send", async () => { + const s = await setup({ lifecycle: true }); + const old = deferred(); + s.control.publish = old; + await s.invite(); + s.edit("new text"); + s.edit(""); + s.dismiss(); + assert.equal(s.options.contentRef.current, ""); + const current = deferred(); + s.control.publish = current; + await s.prompt(); + await s.invite(); + await s.finish(old); + assert.equal(s.result.current.isPreparingMentionSend, true); + assert.equal(s.events("SEND").length, 0); + await s.finish(current); + assert.equal(s.events("SEND").length, 1); +}); +test("cancelled media continuation preserves refs and cannot overwrite an unrelated stored draft", async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.control.attachments = [{ id: "file", file: {}, spoilered: false }]; + await s.prompt(); + await s.invite(); + assert.ok(s.control.uploadCallbacks); + const other = { + content: "new saved draft", + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: [], + }; + s.store.set("thread:a", other); + s.dismiss(); + assert.equal(s.options.contentRef.current, TEXT); + assert.deepEqual(s.control.currentRefs, s.refs); + assert.equal(s.store.get("thread:a"), other); + await s.act(async () => + s.control.uploadCallbacks.onComplete([], new AbortController().signal), + ); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.store.get("thread:a"), other); +}); + +for (const action of ["unchanged", "navigate", "edit"]) { + test(`delayed persona reuse binds the captured draft, not a later selection: ${action}`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + const text = "@Fizz hello"; + const personaRefs = [{ displayName: "Fizz", pubkey: KEY, isAgent: true }]; + const otherRefs = [{ ...personaRefs[0], pubkey: "c".repeat(64) }]; + s.edit(text, []); + s.store.delete("thread:a"); // new, not-yet-persisted persona draft + s.options.mentions.extractMentionPubkeys = () => []; + s.options.mentions.extractMentionPersonas = () => [ + { + displayName: "Fizz", + persona: { id: "builtin:fizz", displayName: "Fizz" }, + }, + ]; + const gate = deferred(); + s.control.persona = gate; + s.rerender(); + let send; + await s.act(async () => { + send = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: text, + recoveryDraftKey: "thread:a", + }); + }); + assert.equal(s.events("persona").length, 1); + if (action === "navigate") { + s.store.set("thread:b", { + content: text, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: otherRefs, + }); + s.navigate("thread:b"); + } else if (action === "edit") s.edit(text, otherRefs); + // Transport failure owes exact resolved persona refs to the source snapshot, + // without replacing a new editor's identical-label selection. + s.options.onSendRef.current = async () => { + throw new Error("transport"); + }; + await s.finish(gate); + await send; + if (action === "unchanged") { + assert.equal(s.events("register-ref").length, 1); + assert.equal(s.options.contentRef.current, text); + assert.deepEqual( + JSON.parse(JSON.stringify(s.control.currentRefs)), + personaRefs, + ); + assert.deepEqual( + JSON.parse(JSON.stringify(s.store.get("thread:a").mentionRefs)), + personaRefs, + ); + } else { + assert.equal(s.events("register-ref").length, 0); + assert.equal(s.options.contentRef.current, text); + assert.deepEqual(s.control.currentRefs, otherRefs); + } + }); +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index dc87359086..2c15cd1b83 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -12,7 +12,15 @@ import { MENTION_REFERENCE_TAG } from "@/shared/lib/resolveMentionNames"; export { MENTION_REFERENCE_TAG }; +/** A single visit to a source draft; returning to the same key is a new owner. */ +export type ComposerDraftOwner = { + channelId: string | null; + draftKey: string | null | undefined; +}; + export type PendingNonMemberMentionSend = { + sourceOwner: ComposerDraftOwner; + composerRevision: number; invitationSignal?: AbortSignal; addressedAgentPubkeys: string[]; inlineAgentMentionPubkeys: string[]; diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index ce109a8433..a83d0ce14f 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -48,6 +48,9 @@ import type { UseMentionSendFlowOptions } from "./useMentionSendFlow.types"; export function useMentionSendFlow({ channelId, + effectiveDraftKey, + getComposerRevision, + runComposerUpdate, channelLinks, channelType, contentRef, @@ -75,15 +78,25 @@ export function useMentionSendFlow({ string | null >(null); const [isMentionSendPending, setIsMentionSendPending] = React.useState(false); - const [isCompleteSendPending, setIsCompleteSendPending] = - React.useState(false); + // Persistence identity is independent of the host component and destination + // channel. A -> B -> A must not revive A's previous invitation or recovery. + const sourceOwner = React.useMemo( + () => ({ channelId, draftKey: effectiveDraftKey }), + [channelId, effectiveDraftKey], + ); + const sourceOwnerRef = React.useRef(sourceOwner); + sourceOwnerRef.current = sourceOwner; + const [completeSendOwner, setCompleteSendOwner] = React.useState< + typeof sourceOwner | null + >(null); + const isCompleteSendPending = completeSendOwner === sourceOwner; const isMentionSendPendingRef = React.useRef(false); - const isCompleteSendPendingRef = React.useRef(false); + const completeSendRef = React.useRef( + null, + ); const isMountedRef = React.useRef(false); const activePreparedLinkPreviews = useActivePreparedLinkPreviews(); - const previousChannelIdRef = React.useRef(channelId); - const channelIdRef = React.useRef(channelId); - channelIdRef.current = channelId; + React.useEffect(() => { isMountedRef.current = true; return () => { @@ -209,13 +222,16 @@ export function useMentionSendFlow({ ], ); const createMentionedPersonaAgents = React.useCallback( - async (trimmed: string, capturedChannelId: string) => { - const personaMentions = mentions.extractMentionPersonas(trimmed); + async ( + personaMentions: ReturnType, + capturedChannelId: string, + ) => { if (!capturedChannelId || personaMentions.length === 0) { return { errors: [] as string[], agents: [] as ManagedAgent[], pubkeys: [] as string[], + mentionRefs: [] as PendingNonMemberMentionSend["savedMentionRefs"], }; } const runtimes = await getAvailableRuntimes(); @@ -223,6 +239,7 @@ export function useMentionSendFlow({ const errors: string[] = []; const agents: ManagedAgent[] = []; const pubkeys: string[] = []; + const mentionRefs: PendingNonMemberMentionSend["savedMentionRefs"] = []; const seenPersonaIds = new Set(); const shouldProvisionForDm = channelType === "dm" && Boolean(onPrepareSendChannel); @@ -260,9 +277,7 @@ export function useMentionSendFlow({ const pubkey = normalizePubkey(result.agent.pubkey); agents.push(result.agent); pubkeys.push(pubkey); - mentions.registerMentionPubkey(displayName, pubkey, { - isAgent: true, - }); + mentionRefs.push({ displayName, pubkey, isAgent: true }); } catch (error) { errors.push( `${displayName}: ${getErrorMessage( @@ -276,14 +291,13 @@ export function useMentionSendFlow({ agents, errors, pubkeys: uniqueNormalizedPubkeys(pubkeys), + mentionRefs, }; }, [ createPersonaAgentMutation, channelType, getAvailableRuntimes, - mentions.extractMentionPersonas, - mentions.registerMentionPubkey, onPrepareSendChannel, provisionPersonaAgentMutation, ], @@ -313,30 +327,32 @@ export function useMentionSendFlow({ clearQueuedAttachments, setSpoileredAttachmentUrls, ]); - React.useEffect(() => { - if (previousChannelIdRef.current === channelId) { - return; - } - previousChannelIdRef.current = channelId; - setPendingNonMemberSend(null); + React.useLayoutEffect(() => { + setPendingNonMemberSend((draft) => + draft?.sourceOwner === sourceOwner ? draft : null, + ); setNonMemberPromptError(null); - }, [channelId]); + }, [sourceOwner]); const completeSend = React.useCallback( async ( draft: PendingNonMemberMentionSend, mentionPubkeys: string[], outgoingTags = draft.outgoingTags, ) => { - if (isCompleteSendPendingRef.current) { + const pending = completeSendRef.current; + if ( + pending?.sourceOwner === draft.sourceOwner && + !pending.invitationSignal?.aborted + ) return; - } + const ownsComposer = () => sourceOwnerRef.current === draft.sourceOwner; const sendSignal = draft.preparedLinkPreviews?.signal; const isSendCancelled = () => sendSignal?.aborted === true || draft.invitationSignal?.aborted === true; if (isSendCancelled()) return draft.preparedLinkPreviews?.release(); - isCompleteSendPendingRef.current = true; - setIsCompleteSendPending(true); + completeSendRef.current = draft; + setCompleteSendOwner(draft.sourceOwner); const preparedUpload = draft.queuedAttachments.length > 0 ? prepareBackgroundMediaUpload(draft.queuedAttachments) @@ -359,7 +375,7 @@ export function useMentionSendFlow({ const persistCanceledDraft = () => { // Invitation cancellation still owes the captured draft recovery. Link // preview cancellation retains its existing independent recovery owner. - if (sendSignal?.aborted || !draft.recoveryDraftKey) return; + if (sendSignal?.aborted || !draft.recoveryDraftKey) return false; const existing = drafts.loadDraft(draft.recoveryDraftKey); if ( existing && @@ -369,9 +385,11 @@ export function useMentionSendFlow({ JSON.stringify(existing.pendingImeta) !== JSON.stringify(draft.savedImeta) || JSON.stringify(existing.spoileredAttachmentUrls) !== - JSON.stringify([...draft.savedSpoileredAttachmentUrls])) + JSON.stringify([...draft.savedSpoileredAttachmentUrls]) || + JSON.stringify(existing.mentionRefs ?? []) !== + JSON.stringify(draft.savedMentionRefs)) ) { - return; + return false; } drafts.persistDraft( draft.recoveryDraftKey, @@ -381,17 +399,20 @@ export function useMentionSendFlow({ [...draft.savedSpoileredAttachmentUrls], draft.savedMentionRefs, ); + return true; }; let composerCleared = false; let optimisticComposerContent = ""; + let clearedRevision = -1; const restoreComposerAfterFailure = () => { if (!composerCleared) return; composerCleared = false; - persistCanceledDraft(); + // An authored edit (even edit -> clear) ends optimistic recovery's + // authority over this visit, including its persisted record and files. + if (ownsComposer() && getComposerRevision() !== clearedRevision) return; + const persisted = persistCanceledDraft(); const canAnimateCurrentComposer = - isMountedRef.current && - (draft.capturedChannelId === channelIdRef.current || - channelIdRef.current === null); + isMountedRef.current && ownsComposer(); if ( canAnimateCurrentComposer && draft.addressedAgentPubkeys.length > 0 @@ -400,9 +421,10 @@ export function useMentionSendFlow({ } const canRestoreCurrentComposer = canAnimateCurrentComposer && + getComposerRevision() === clearedRevision && contentRef.current.trim() === optimisticComposerContent.trim() && !hasUnsavedMedia(); - if (!canRestoreCurrentComposer && draft.recoveryDraftKey) { + if (!canRestoreCurrentComposer && persisted && draft.recoveryDraftKey) { saveQueuedAttachmentsForDraft( draft.recoveryDraftKey, draft.queuedAttachments, @@ -411,21 +433,20 @@ export function useMentionSendFlow({ if (!canRestoreCurrentComposer) { return; } - setContent(draft.savedContent); - contentRef.current = draft.savedContent; - richText.setContent(draft.savedContent); - setPendingImeta(draft.savedImeta); - restoreQueuedAttachments(draft.queuedAttachments); - mentions.restoreDraftMentionRefs(draft.savedMentionRefs); - setSpoileredAttachmentUrls?.( - new Set(draft.savedSpoileredAttachmentUrls), - ); + runComposerUpdate(() => { + setContent(draft.savedContent); + contentRef.current = draft.savedContent; + richText.setContent(draft.savedContent); + setPendingImeta(draft.savedImeta); + restoreQueuedAttachments(draft.queuedAttachments); + mentions.restoreDraftMentionRefs(draft.savedMentionRefs); + setSpoileredAttachmentUrls?.( + new Set(draft.savedSpoileredAttachmentUrls), + ); + }); }; - if ( - draft.capturedChannelId === channelIdRef.current || - channelIdRef.current === null - ) { - clearComposer(); + if (ownsComposer() && getComposerRevision() === draft.composerRevision) { + runComposerUpdate(clearComposer); if (draft.addressedAgentPubkeys.length > 0) { optimisticComposerContent = onAddressedAgentsComposerCleared?.(draft.addressedAgentPubkeys) ?? @@ -433,7 +454,21 @@ export function useMentionSendFlow({ contentRef.current = optimisticComposerContent; } composerCleared = true; + clearedRevision = getComposerRevision(); } + // Recover on invalidation, not when an arbitrary external await settles. + // In particular a later visit to A must load recovery before it can edit + // or clear A again. The async continuation only observes cancellation. + const cancelSend = () => { + restoreComposerAfterFailure(); + if (completeSendRef.current === draft) { + completeSendRef.current = null; + if (isMountedRef.current) setCompleteSendOwner(null); + } + }; + draft.invitationSignal?.addEventListener("abort", cancelSend, { + once: true, + }); let uploadStarted = false; try { const admittedMentionPubkeys = uniqueNormalizedPubkeys( @@ -587,10 +622,7 @@ export function useMentionSendFlow({ const newlyPinnedPubkeys = draft.inlineAgentMentionPubkeys.filter( (pubkey) => sentMentionPubkeys.has(normalizePubkey(pubkey)), ); - if ( - draft.capturedChannelId === channelIdRef.current || - channelIdRef.current === null - ) { + if (ownsComposer()) { onAddressedAgentsSendSucceeded?.( [ ...new Set([ @@ -601,7 +633,13 @@ export function useMentionSendFlow({ newlyPinnedPubkeys, ); } - if (draft.sentDraftKey) { + if ( + draft.sentDraftKey && + (!ownsComposer() || getComposerRevision() === clearedRevision) && + JSON.stringify( + drafts.loadDraft(draft.sentDraftKey)?.mentionRefs ?? [], + ) === JSON.stringify(draft.savedMentionRefs) + ) { drafts.markDraftSent( draft.sentDraftKey, draft.savedContent, @@ -664,14 +702,17 @@ export function useMentionSendFlow({ } draft.preparedLinkPreviews?.release(); if (!uploadStarted) preparedUpload?.cancel(); - isCompleteSendPendingRef.current = false; - if (isMountedRef.current) { - setIsCompleteSendPending(false); + draft.invitationSignal?.removeEventListener("abort", cancelSend); + if (completeSendRef.current === draft) { + completeSendRef.current = null; + if (isMountedRef.current) setCompleteSendOwner(null); } } }, [ clearComposer, + getComposerRevision, + runComposerUpdate, contentRef, drafts, ensureManagedAgentMentionsReady, @@ -712,6 +753,12 @@ export function useMentionSendFlow({ } isMentionSendPendingRef.current = true; setIsMentionSendPending(true); + // Capture exact selections before any async preparation can navigate the + // reused editor to another draft (possibly with identical display text). + const composerRevision = getComposerRevision(); + const savedMentionRefs = mentions.getDraftMentionRefs(trimmed).slice(); + const selectedMentionPubkeys = mentions.extractMentionPubkeys(trimmed); + const selectedPersonas = mentions.extractMentionPersonas(trimmed); const isSendCancelled = () => preparedLinkPreviews?.signal.aborted === true; let sendPromoted = false; @@ -748,7 +795,7 @@ export function useMentionSendFlow({ } } const personaMentionResult = await createMentionedPersonaAgents( - trimmed, + selectedPersonas, effectiveChannelId ?? "", ); if (isSendCancelled()) return; @@ -767,8 +814,19 @@ export function useMentionSendFlow({ const createdPersonaAgentPubkeySet = new Set( createdPersonaAgentPubkeys.map(normalizePubkey), ); + savedMentionRefs.push(...personaMentionResult.mentionRefs); + // Preparation resolves the captured persona, never the editor's new + // selection. Only this unchanged visit may receive its resolved binding. + if ( + isMountedRef.current && + sourceOwnerRef.current === sourceOwner && + getComposerRevision() === composerRevision + ) { + for (const ref of personaMentionResult.mentionRefs) + mentions.registerMentionPubkey(ref.displayName, ref.pubkey, ref); + } const explicitMentionPubkeys = uniqueNormalizedPubkeys([ - ...mentions.extractMentionPubkeys(trimmed), + ...selectedMentionPubkeys, ...createdPersonaAgentPubkeys, ]); const pubkeys = mergeMentionRecipients( @@ -801,8 +859,9 @@ export function useMentionSendFlow({ ); } catch {} } - const savedMentionRefs = mentions.getDraftMentionRefs(trimmed); const pendingDraft: PendingNonMemberMentionSend = { + sourceOwner, + composerRevision, addressedAgentPubkeys: uniqueNormalizedPubkeys(addressedAgentPubkeys), inlineAgentMentionPubkeys: uniqueNormalizedPubkeys( savedMentionRefs @@ -830,6 +889,7 @@ export function useMentionSendFlow({ savedMentionRefs, }; if (promptNonMemberPubkeys.length > 0) { + if (sourceOwnerRef.current !== sourceOwner) return; setNonMemberPromptError(null); setPendingNonMemberSend(pendingDraft); return; @@ -849,6 +909,8 @@ export function useMentionSendFlow({ }, [ completeSend, + sourceOwner, + getComposerRevision, channelType, createMentionedPersonaAgents, customEmoji, @@ -860,6 +922,7 @@ export function useMentionSendFlow({ mentions.isManagedAgentPubkey, mentions.memberPubkeys, mentions.getDraftMentionRefs, + mentions.registerMentionPubkey, onPrepareSendChannel, activePreparedLinkPreviews, ], @@ -872,7 +935,7 @@ export function useMentionSendFlow({ ); }, [mentions.getMentionDisplayName, pendingNonMemberSend]); const invitation = useNonMemberInvite({ - channelId, + sourceOwner, draft: pendingNonMemberSend, canInvite: canInviteNonMembers, revalidate: mentions.revalidateMentionPubkeys, @@ -909,7 +972,6 @@ export function useMentionSendFlow({ invitation.isPending || isMentionSendPending || isCompleteSendPending || - addMembersMutation.isPending || attachAgentMutation.isPending || createPersonaAgentMutation.isPending || startAgentMutation.isPending, diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts index fe16ba2f63..05bc6beb3f 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts @@ -10,7 +10,11 @@ import type { UseRichTextEditorResult } from "@/features/messages/lib/useRichTex import type { UseDraftsResult } from "@/features/messages/lib/useDrafts"; export type UseMentionSendFlowOptions = { + getComposerRevision: () => number; + runComposerUpdate: (update: () => void) => void; channelId: string | null; + /** The actual persistence key, including same-channel thread identity. */ + effectiveDraftKey: string | null | undefined; channelLinks: Pick; channelType: ChannelType | null; contentRef: React.MutableRefObject; diff --git a/desktop/src/features/messages/ui/useNonMemberInvite.ts b/desktop/src/features/messages/ui/useNonMemberInvite.ts index 0e1ead2854..933225c97d 100644 --- a/desktop/src/features/messages/ui/useNonMemberInvite.ts +++ b/desktop/src/features/messages/ui/useNonMemberInvite.ts @@ -8,11 +8,12 @@ import { mentionRevalidationOptions, uniqueNormalizedPubkeys, type PendingNonMemberMentionSend, + type ComposerDraftOwner, } from "./useMentionSendFlow.helpers"; /** Own the whole Invite attempt, not just the membership mutation's pending state. */ export function useNonMemberInvite({ - channelId, + sourceOwner, draft, canInvite, revalidate, @@ -22,7 +23,7 @@ export function useNonMemberInvite({ completeSend, setError, }: { - channelId: string | null; + sourceOwner: ComposerDraftOwner; draft: PendingNonMemberMentionSend | null; canInvite: boolean; revalidate: ( @@ -48,8 +49,8 @@ export function useNonMemberInvite({ controller: AbortController; draft: PendingNonMemberMentionSend; } | null>(null); - const currentChannel = React.useRef(channelId); - currentChannel.current = channelId; + const currentOwner = React.useRef(sourceOwner); + currentOwner.current = sourceOwner; const [isPending, setIsPending] = React.useState(false); const cancel = React.useCallback(() => { active.current?.controller.abort(); @@ -62,11 +63,11 @@ export function useNonMemberInvite({ // A different non-null prompt or destination supersedes the old intent. if ( attempt && - (attempt.draft.capturedChannelId !== channelId || + (attempt.draft.sourceOwner !== sourceOwner || (draft !== null && draft !== attempt.draft)) ) cancel(); - }, [channelId, draft, cancel]); + }, [sourceOwner, draft, cancel]); React.useLayoutEffect( () => () => { active.current?.controller.abort(); @@ -76,7 +77,8 @@ export function useNonMemberInvite({ ); const invite = React.useCallback(() => { - if (!draft || active.current) return; + if (!draft || draft.sourceOwner !== currentOwner.current || active.current) + return; if (!canInvite) { setError(PRIVATE_CHANNEL_ADD_DENIED_MESSAGE); return; @@ -88,7 +90,7 @@ export function useNonMemberInvite({ const isCurrent = () => active.current?.controller === attempt && !attempt.signal.aborted && - currentChannel.current === draft.capturedChannelId; + currentOwner.current === draft.sourceOwner; void (async () => { const mentionPubkeys = uniqueNormalizedPubkeys( await revalidate( diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index 3ee44be0ee..3d0e36ca40 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -154,7 +154,7 @@ for (const error of [ await expect(page.getByText(error, { exact: true })).toBeVisible(); await waitForAnimations(page); await page.screenshot({ - path: `test-results/remote-error-${error.split(" ")[0]}.png`, + path: `test-results/remote-error-${error.split(" ")[0].replace(/[^a-zA-Z0-9_-]/g, "-")}.png`, }); await expect(page.getByTestId("message-input")).toHaveText( "@RemoteScout hello", @@ -262,21 +262,25 @@ type InviteGateWindow = Window & { inviteGateEntered?: boolean; releaseInviteGate?: () => void; }; -async function holdInviteCommand(page: Page, command: string) { - await page.evaluate((heldCommand) => { - const state = window as unknown as InviteGateWindow; - const invoke = state.__TAURI_INTERNALS__.invoke; - const gate = new Promise((resolve) => { - state.releaseInviteGate = resolve; - }); - state.__TAURI_INTERNALS__.invoke = async (command, payload) => { - if (command !== heldCommand) return invoke(command, payload); - state.__TAURI_INTERNALS__.invoke = invoke; - state.inviteGateEntered = true; - await gate; - return invoke(command, payload); - }; - }, command); +async function holdInviteCommand(page: Page, command: string, skip = 0) { + await page.evaluate( + ({ heldCommand, skip }) => { + const state = window as unknown as InviteGateWindow; + const invoke = state.__TAURI_INTERNALS__.invoke; + const gate = new Promise((resolve) => { + state.releaseInviteGate = resolve; + }); + state.__TAURI_INTERNALS__.invoke = async (command, payload) => { + if (command !== heldCommand || skip-- > 0) + return invoke(command, payload); + state.__TAURI_INTERNALS__.invoke = invoke; + state.inviteGateEntered = true; + await gate; + return invoke(command, payload); + }; + }, + { heldCommand: command, skip }, + ); } async function releaseInviteCommand(page: Page) { await page.evaluate(() => { @@ -365,3 +369,92 @@ test("B1 navigation during delayed add cannot publish its captured draft", async "@RemoteScout hello", ); }); + +for (const stage of ["add", "publish"] as const) { + for (const incoming of ["unrelated thread B draft", "@RemoteScout hello"]) { + test(`B1 same-channel thread navigation during ${stage} preserves ${incoming}`, async ({ + page, + }) => { + await install(page); + await expect + .poll(() => + page.evaluate( + () => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: "general", + }) ?? false, + ), + ) + .toBe(true); + const roots = await page.evaluate(() => + ["Lifecycle thread A", "Lifecycle thread B"].map( + (content) => + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content, + })?.id, + ), + ); + expect(roots.every(Boolean)).toBe(true); + const navigate = async (id: string | undefined) => { + // Drive the real reply route handler, including while the modal has + // focus (the equivalent external history/navigation intent). + await page + .getByTestId(`reply-message-${id}`) + .first() + .evaluate((button) => (button as HTMLButtonElement).click()); + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); + await expect(page.getByTestId("message-thread-head")).toContainText( + id === roots[0] ? "Lifecycle thread A" : "Lifecycle thread B", + ); + }; + const input = page + .getByTestId("message-thread-panel") + .getByTestId("message-input"); + // Visit both first: the transition under test must reuse the available + // panel/composer, not get a fortuitous unmount via a loading skeleton. + await navigate(roots[1]); + await input.fill(incoming); + await navigate(roots[0]); + await expect(input).toHaveText(""); + await input.fill("@Remote"); + await page + .getByTestId(`mention-suggestion-${REMOTE}`) + .locator("button") + .first() + .click(); + await page.keyboard.type("hello"); + await page + .getByTestId("message-thread-panel") + .getByTestId("send-message") + .click(); + await holdInviteCommand( + page, + stage === "add" ? "add_channel_members" : "revalidate_relay_agents", + stage === "publish" ? 2 : 0, + ); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await waitForInviteGate(page); + // Expando proves the actual editor DOM host survived A -> B. + await input.evaluate((el) => + el.setAttribute("data-lifecycle-host", "retained"), + ); + await navigate(roots[1]); + await expect(input).toHaveAttribute("data-lifecycle-host", "retained"); + await expect(input).toHaveText(incoming); + await expect(page.getByRole("alertdialog")).toHaveCount(0); + // Return before resolution: optimistic recovery must already be durable. + await navigate(roots[0]); + await expect(input).toHaveText("@RemoteScout hello"); + await input.fill("new A draft after return"); + await navigate(roots[1]); + await releaseInviteCommand(page); + await page.waitForTimeout(300); + expect(await sent(page)).toEqual([]); + await expect(input).toHaveText(incoming); + await navigate(roots[0]); + await expect(input).toHaveText("new A draft after return"); + await assertNoLocalLifecycle(page); + }); + } +} diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md index 0b2effc35f..0a985edd7f 100644 --- a/docs/remote-mention-routing.md +++ b/docs/remote-mention-routing.md @@ -47,3 +47,27 @@ exact mention refs without overwriting newer edits. `useMentionSendFlow.cancellation.test.mjs` drives the actual hooks with React StrictMode and deferred dependencies; `remote-owned-mentions.spec.ts` covers visible pending, Escape/retry and route navigation at deferred IPC boundaries. + +### Source draft ownership + +Invitation intent belongs to one visit of the effective persistence key and +channel, not just the mounted composer or destination channel. Same-channel +thread changes and A → B → A invalidate the original visit. The same owner gates +optimistic clear, recovery and pending state. Invalidation makes recovery durable +synchronously, before a later visit can load or edit the source draft; late async +completion cannot revive that recovery or release a newer attempt's latch. + +The editor's authored revision distinguishes an intentional edit → clear from an +optimistic empty composer. Programmatic send clear/recovery runs inside the draft +lifecycle's restoration boundary, so it does not mark the source as authoritatively +deleted. Exact selected mention refs are captured before asynchronous preparation. +Persona preparation similarly consumes captured selections and returns resolved +refs, writing them into the editor only while the original visit/revision remains +current. Normal persona creation/reuse and ordinary destination-bound background +sends retain their existing behavior; accepted membership is never rolled back. + +Recovery still compares stored content/media/refs rather than implementing a +versioned draft database. Final membership/policy reads are not atomic with send, +and cancellation cannot retract an already dispatched publication. Standalone +forum transport-failure binding recovery and native compatibility remain separate +review/follow-up boundaries.