From e90edd61043d5575e856521d959f98cb71f6b558 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 15:03:26 -0400 Subject: [PATCH] fix(desktop): bind invitation recovery to source draft visits Invalidate same-channel thread changes and returning visits before late invitation continuations can clear or recover over another draft. Separate authored editor revisions from optimistic clear, capture exact selections before preparation, and bind resolved persona refs to that captured draft. Cover retained-host navigation, recovery, media, and persona preparation; sanitize screenshot stems only. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson (cherry picked from commit b7c4ab9e3f27a7c2f28978d501179f67830073f1) Signed-off-by: Logan Johnson (cherry picked from commit 2cb986a910abd5aec888393ce24250f2fedcb623) Signed-off-by: Logan Johnson --- .../features/messages/ui/MessageComposer.tsx | 58 ++-- .../messages/ui/useDraftPersistSnapshot.ts | 24 +- .../useMentionSendFlow.cancellation.test.mjs | 316 +++++++++++++++++- .../messages/ui/useMentionSendFlow.helpers.ts | 8 + .../messages/ui/useMentionSendFlow.ts | 180 ++++++---- .../messages/ui/useMentionSendFlow.types.ts | 4 + .../messages/ui/useNonMemberInvite.ts | 18 +- .../tests/e2e/remote-owned-mentions.spec.ts | 125 ++++++- docs/remote-mention-routing.md | 24 ++ 9 files changed, 629 insertions(+), 128 deletions(-) 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.