From c717736b24344edd124be03a5df2cd41b7375fa7 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 13:42:37 -0400 Subject: [PATCH 01/13] fix(desktop): authorize remote mentions at the publication boundary Extract preparation, exact destination checks and retained recipient intent from #7114, independently of labels and profile presentation. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- desktop/playwright.config.ts | 1 + desktop/src/features/agents/AGENTS.md | 6 +- .../lib/agentAutocompleteEligibility.test.mjs | 89 +++++++ .../lib/agentAutocompleteEligibility.ts | 53 +++- .../lib/agentMentionRevalidation.test.mjs | 184 +++++++++++-- .../messages/lib/agentMentionRevalidation.ts | 69 +++-- .../messages/lib/useDraftMentionRouting.ts | 6 + .../src/features/messages/lib/useMentions.ts | 32 ++- .../messages/ui/submitMessageEdit.test.mjs | 15 +- .../features/messages/ui/submitMessageEdit.ts | 29 ++- .../ui/useMentionSendFlow.helpers.test.mjs | 22 ++ .../messages/ui/useMentionSendFlow.helpers.ts | 34 +++ .../messages/ui/useMentionSendFlow.ts | 70 +++-- desktop/src/testing/e2eBridge.ts | 12 +- desktop/tests/e2e/mentions.spec.ts | 79 ++++-- .../tests/e2e/remote-owned-mentions.spec.ts | 246 ++++++++++++++++++ docs/remote-mention-routing.md | 31 +++ 17 files changed, 851 insertions(+), 127 deletions(-) create mode 100644 desktop/tests/e2e/remote-owned-mentions.spec.ts create mode 100644 docs/remote-mention-routing.md diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 8888051d878..1304e66ba3b 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -81,6 +81,7 @@ export default defineConfig({ "**/mention-clipboard.spec.ts", "**/cloud-provenance.spec.ts", "**/mention-recipients.spec.ts", + "**/remote-owned-mentions.spec.ts", "**/team-mentions.spec.ts", "**/persistent-agent-audience.spec.ts", "**/relay-reconnect.spec.ts", diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index ff8df71cd30..dfb9c0ed494 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -293,7 +293,11 @@ with a TypeScript lookup table or an id comparison in a component. managed agent. Independently operated relay agents with NIP-OA ownership remain eligible in every build when their verified owner's signed `respond_to` policy admits the viewer and relay membership includes the - target channel. Marked builds require that verified owner coordinate but do + target channel at publication. Owned nonmembers may be offered for preparation + and Invite; this is not permission to publish. Final authorization refreshes + the exact destination and retains captured selected identities across uploads + and edits. Denial preserves the draft, never silently removes a selected key. + See `docs/remote-mention-routing.md`. Marked builds require that verified owner coordinate but do not require it to equal the viewer; OSS builds retain compatibility with self-authored legacy directory records. Keep native discovery and send-time revalidation fail closed on invalid ownership or managed policy evidence, diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs index 5398b28f055..7ffd01097c8 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs @@ -548,3 +548,92 @@ test("coalesceAgentAutocompleteCandidates: leaves non-agents alone", () => { assert.deepEqual(coalesce([first, second]), [first, second]); }); + +test("owners remain admitted by allowlist policy without listing themselves", () => { + assert.equal( + relayAgentCanRespondInChannel( + { + ownerPubkey: CURRENT_PUBKEY, + respondTo: "allowlist", + respondToAllowlist: [], + channelIds: ["general"], + }, + "general", + CURRENT_PUBKEY, + ), + true, + ); +}); + +test("owned discovery does not require a shared channel, but sending does", () => { + for (const respondTo of ["owner-only", "allowlist", "anyone"]) { + const agent = { + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + respondTo, + respondToAllowlist: [], + channelIds: [], + }; + assert.equal( + relayAgentIsSharedWithUser(agent, new Set(), CURRENT_PUBKEY), + true, + ); + assert.equal( + relayAgentCanRespondInChannel(agent, "general", CURRENT_PUBKEY), + false, + ); + } +}); + +test("DM ownership is independent of local configuration and still requires membership", () => { + const base = { + currentPubkey: CURRENT_PUBKEY, + managedAgentPubkeys: [PUB_A], + sharedChannelIds: new Set(), + relayAgents: [ + { + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + respondTo: "allowlist", + respondToAllowlist: [], + channelIds: ["dm"], + }, + { + pubkey: PUB_C, + ownerPubkey: OTHER_OWNER_PUBKEY, + respondTo: "anyone", + respondToAllowlist: [], + channelIds: ["dm"], + }, + { + pubkey: PUB_D, + ownerPubkey: CURRENT_PUBKEY, + respondTo: "nobody", + respondToAllowlist: [], + channelIds: ["dm"], + }, + ], + }; + assert.deepEqual( + getMentionableAgentPubkeys({ + ...base, + eligibilityScope: { type: "owned", channelId: "dm" }, + }), + new Set([PUB_A, PUB_B]), + ); + assert.deepEqual( + getMentionableAgentPubkeys({ + ...base, + eligibilityScope: { type: "owned", channelId: "other" }, + }), + new Set([PUB_A]), + ); + assert.deepEqual( + getMentionableAgentPubkeys({ + ...base, + eligibilityScope: { type: "owned", channelId: null }, + phase: "prepare", + }), + new Set([PUB_A, PUB_B]), + ); +}); diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts index e3c82cfff4f..65655f8163f 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts @@ -34,12 +34,17 @@ export function relayAgentIsSharedWithUser( ? normalizePubkey(currentPubkey) : null; + // Ownership is relay identity, not local key custody. Like the harness's + // author gate, every supported policy except nobody admits the owner. if ( - agent.respondTo === "owner-only" && + (agent.respondTo === "owner-only" || + agent.respondTo === "allowlist" || + agent.respondTo === "anyone") && normalizedCurrentPubkey && - agent.ownerPubkey + agent.ownerPubkey && + normalizePubkey(agent.ownerPubkey) === normalizedCurrentPubkey ) { - return normalizePubkey(agent.ownerPubkey) === normalizedCurrentPubkey; + return true; } if (agent.respondTo === "allowlist" && normalizedCurrentPubkey) { @@ -71,6 +76,7 @@ export function relayAgentCanRespondInChannel( export type AgentEligibilityScope = | { type: "community" } | { type: "channel"; channelId: string } + | { type: "owned"; channelId: string | null } | { type: "managed-only" }; export function getMentionableAgentPubkeys({ @@ -79,9 +85,11 @@ export function getMentionableAgentPubkeys({ managedAgentPubkeys, relayAgents, sharedChannelIds, + phase = "publish", }: { currentPubkey?: string | null; eligibilityScope: AgentEligibilityScope; + phase?: "prepare" | "publish"; managedAgentPubkeys: Iterable; relayAgents: readonly RelayAgent[] | undefined; sharedChannelIds: ReadonlySet; @@ -94,13 +102,38 @@ export function getMentionableAgentPubkeys({ const isAllowed = eligibilityScope.type === "managed-only" ? false - : eligibilityScope.type === "community" - ? relayAgentIsSharedWithUser(agent, sharedChannelIds, currentPubkey) - : relayAgentCanRespondInChannel( - agent, - eligibilityScope.channelId, - currentPubkey, - ); + : eligibilityScope.type === "owned" + ? Boolean( + currentPubkey && + agent.ownerPubkey && + normalizePubkey(agent.ownerPubkey) === + normalizePubkey(currentPubkey) && + relayAgentIsSharedWithUser( + agent, + sharedChannelIds, + currentPubkey, + ) && + (phase === "prepare" || + (eligibilityScope.channelId !== null && + agent.channelIds.includes(eligibilityScope.channelId))), + ) + : eligibilityScope.type === "community" + ? relayAgentIsSharedWithUser(agent, sharedChannelIds, currentPubkey) + : phase === "prepare" && + currentPubkey && + agent.ownerPubkey && + normalizePubkey(agent.ownerPubkey) === + normalizePubkey(currentPubkey) + ? relayAgentIsSharedWithUser( + agent, + sharedChannelIds, + currentPubkey, + ) + : relayAgentCanRespondInChannel( + agent, + eligibilityScope.channelId, + currentPubkey, + ); if (isAllowed) { pubkeys.add(normalizePubkey(agent.pubkey)); } diff --git a/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs b/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs index d04446a3417..b04580921ba 100644 --- a/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs +++ b/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { revalidateAgentMentionPubkeys } from "./agentMentionRevalidation.ts"; +import { + revalidateAgentMentionPubkeys, + AgentMentionAuthorizationError, +} from "./agentMentionRevalidation.ts"; const CURRENT = "a".repeat(64); const AGENT = "b".repeat(64); @@ -47,37 +50,170 @@ test("fresh managed evidence survives unrelated relay authorization errors", asy throw new Error("relay directory unavailable"); }, }); - assert.deepEqual(result, [HUMAN, LOCAL_AGENT]); }); test("relay-only agents still fail closed when relay discovery fails", async () => { - const result = await revalidateAgentMentionPubkeys({ - ...options(), - fetchRelayAgents: async () => { - throw new Error("relay directory unavailable"); - }, - }); + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(), + fetchRelayAgents: async () => { + throw new Error("relay directory unavailable"); + }, + }), + AgentMentionAuthorizationError, + ); +}); - assert.deepEqual(result, [HUMAN]); +test("mixed evidence cannot silently drop an intended relay recipient", async () => { + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(async () => ({ + profiles: { [AGENT]: { ownerPubkey: CURRENT } }, + missing: [LOCAL_AGENT], + })), + pubkeys: [HUMAN, LOCAL_AGENT, AGENT], + agentPubkeys: new Set([LOCAL_AGENT, AGENT]), + refetchManagedAgents: async () => ({ + data: [{ pubkey: LOCAL_AGENT }], + error: null, + }), + fetchRelayAgents: async () => { + throw new Error("relay directory unavailable"); + }, + }), + AgentMentionAuthorizationError, + ); }); -test("mixed evidence preserves only fresh managed agents and humans", async () => { - const result = await revalidateAgentMentionPubkeys({ - ...options(async () => ({ - profiles: { [AGENT]: { ownerPubkey: CURRENT } }, - missing: [LOCAL_AGENT], - })), - pubkeys: [HUMAN, LOCAL_AGENT, AGENT], - agentPubkeys: new Set([LOCAL_AGENT, AGENT]), - refetchManagedAgents: async () => ({ - data: [{ pubkey: LOCAL_AGENT }], - error: null, +test("remote-owned membership does not depend on local runtime discovery", async () => { + assert.deepEqual( + await revalidateAgentMentionPubkeys({ + ...options(), + refetchManagedAgents: async () => ({ + data: undefined, + error: new Error("local unavailable"), + }), + fetchRelayAgents: async () => [ + { + pubkey: AGENT, + ownerPubkey: CURRENT, + respondTo: "owner-only", + respondToAllowlist: [], + channelIds: ["general"], + }, + ], }), - fetchRelayAgents: async () => { - throw new Error("relay directory unavailable"); - }, + [HUMAN, AGENT], + ); +}); + +test("stale local data is not authority when its refresh fails", async () => { + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(), + pubkeys: [HUMAN, LOCAL_AGENT, AGENT], + agentPubkeys: new Set([LOCAL_AGENT, AGENT]), + refetchManagedAgents: async () => ({ + data: [{ pubkey: LOCAL_AGENT }], + error: new Error("local unavailable"), + }), + }), + AgentMentionAuthorizationError, + ); +}); + +test("owned remote policy revocation and missing membership fail closed", async () => { + for (const agent of [ + { respondTo: "nobody", channelIds: ["general"] }, + { respondTo: "owner-only", channelIds: [] }, + ]) { + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(), + fetchRelayAgents: async () => [ + { + pubkey: AGENT, + ownerPubkey: CURRENT, + respondToAllowlist: [], + ...agent, + }, + ], + }), + AgentMentionAuthorizationError, + ); + } +}); + +for (const type of ["channel", "owned"]) { + test(`${type}: preparation admits owned nonmembers but publication requires actual membership`, async () => { + let channelIds = []; + const opts = { + ...options(), + eligibilityScope: { type, channelId: "target" }, + sharedChannelIds: new Set(), + fetchRelayAgents: async () => [ + { + pubkey: AGENT, + ownerPubkey: CURRENT, + respondTo: "allowlist", + respondToAllowlist: [], + channelIds, + }, + ], + }; + assert.deepEqual( + await revalidateAgentMentionPubkeys({ ...opts, phase: "prepare" }), + [HUMAN, AGENT], + ); + await assert.rejects( + revalidateAgentMentionPubkeys(opts), + AgentMentionAuthorizationError, + ); + channelIds = ["target"]; + assert.deepEqual(await revalidateAgentMentionPubkeys(opts), [HUMAN, AGENT]); + channelIds = ["other"]; + await assert.rejects( + revalidateAgentMentionPubkeys(opts), + AgentMentionAuthorizationError, + ); }); +} - assert.deepEqual(result, [HUMAN, LOCAL_AGENT]); +test("preparation cannot bypass a fresh owner-policy denial", async () => { + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(), + phase: "prepare", + fetchRelayAgents: async () => [ + { + pubkey: AGENT, + ownerPubkey: CURRENT, + respondTo: "nobody", + respondToAllowlist: [], + channelIds: [], + }, + ], + }), + AgentMentionAuthorizationError, + ); +}); + +test("publication cannot authorize a DM that still has no destination", async () => { + await assert.rejects( + revalidateAgentMentionPubkeys({ + ...options(), + eligibilityScope: { type: "owned", channelId: null }, + fetchRelayAgents: async () => [ + { + pubkey: AGENT, + ownerPubkey: CURRENT, + respondTo: "owner-only", + respondToAllowlist: [], + channelIds: ["other"], + }, + ], + }), + AgentMentionAuthorizationError, + ); }); diff --git a/desktop/src/features/messages/lib/agentMentionRevalidation.ts b/desktop/src/features/messages/lib/agentMentionRevalidation.ts index 37f7ce9d4e3..1d7c197d6f8 100644 --- a/desktop/src/features/messages/lib/agentMentionRevalidation.ts +++ b/desktop/src/features/messages/lib/agentMentionRevalidation.ts @@ -1,5 +1,4 @@ import { - filterAdmittedMentionPubkeys, getAgentMentionAdmission, getMentionableAgentPubkeys, type AgentEligibilityScope, @@ -9,6 +8,20 @@ import type { ManagedAgent, RelayAgent } from "@/shared/api/types"; import { normalizePubkey } from "@/shared/lib/pubkey"; import * as React from "react"; +export type MentionRevalidationOptions = { + phase?: "prepare" | "publish"; + intendedAgentPubkeys?: readonly string[]; +}; + +export class AgentMentionAuthorizationError extends Error { + constructor() { + super( + "Could not authorize a mentioned agent. Check its access and channel membership, then retry or remove the mention.", + ); + this.name = "AgentMentionAuthorizationError"; + } +} + type DirectoryResult = { data: T | undefined; error: Error | null; @@ -22,7 +35,9 @@ export async function revalidateAgentMentionPubkeys({ sharedChannelIds, refetchManagedAgents, fetchRelayAgents, + phase = "publish", }: { + phase?: "prepare" | "publish"; pubkeys: readonly string[]; agentPubkeys: ReadonlySet; currentPubkey: string | null; @@ -39,20 +54,21 @@ export async function revalidateAgentMentionPubkeys({ } const [managedResult, relayAgents] = await Promise.all([ - refetchManagedAgents(), + refetchManagedAgents().catch(() => null), fetchRelayAgents([...requestedAgentPubkeys]).catch(() => null), ]); const relayDirectoryReady = relayAgents !== null; - if (managedResult.error !== null || managedResult.data === undefined) { - return filterAdmittedMentionPubkeys(pubkeys, agentPubkeys, new Set()); - } - + // Each directory proves only its own identities. A failed local runtime + // query must neither veto fresh relay evidence nor admit stale local data. const managedPubkeys = new Set( - managedResult.data.map((agent) => normalizePubkey(agent.pubkey)), + (managedResult?.error === null ? (managedResult.data ?? []) : []).map( + (agent) => normalizePubkey(agent.pubkey), + ), ); const mentionablePubkeys = getMentionableAgentPubkeys({ currentPubkey, eligibilityScope, + phase, managedAgentPubkeys: managedPubkeys, relayAgents: relayDirectoryReady ? relayAgents : [], sharedChannelIds, @@ -71,7 +87,12 @@ export async function revalidateAgentMentionPubkeys({ ); }), ); - return filterAdmittedMentionPubkeys(pubkeys, agentPubkeys, admittedPubkeys); + if ( + [...requestedAgentPubkeys].some((pubkey) => !admittedPubkeys.has(pubkey)) + ) { + throw new AgentMentionAuthorizationError(); + } + return [...pubkeys]; } export function useAgentMentionRevalidation({ @@ -90,22 +111,38 @@ export function useAgentMentionRevalidation({ refetchManagedAgents: () => Promise>; }) { return React.useCallback( - (pubkeys: readonly string[]) => - revalidateAgentMentionPubkeys({ + ( + pubkeys: readonly string[], + destinationChannelId?: string | null, + options: MentionRevalidationOptions = {}, + ) => { + // A new DM can acquire its channel during preparation. Validate the + // actual destination at publication, not the composer's original null id. + const scope: AgentEligibilityScope = destinationChannelId + ? { + type: eligibilityScope.type === "owned" ? "owned" : "channel", + channelId: destinationChannelId, + } + : eligibilityScope; + return revalidateAgentMentionPubkeys({ pubkeys, - agentPubkeys: new Set([...agentPubkeys, ...getSelectedAgentPubkeys()]), + agentPubkeys: new Set([ + ...agentPubkeys, + ...getSelectedAgentPubkeys(), + ...(options.intendedAgentPubkeys ?? []).map(normalizePubkey), + ]), + phase: options.phase, currentPubkey, - eligibilityScope, + eligibilityScope: scope, sharedChannelIds, refetchManagedAgents, fetchRelayAgents: (requestedPubkeys) => revalidateRelayAgents( requestedPubkeys, - eligibilityScope.type === "channel" - ? eligibilityScope.channelId - : undefined, + "channelId" in scope ? (scope.channelId ?? undefined) : undefined, ), - }), + }); + }, [ agentPubkeys, currentPubkey, diff --git a/desktop/src/features/messages/lib/useDraftMentionRouting.ts b/desktop/src/features/messages/lib/useDraftMentionRouting.ts index dcf8d745dcc..86d0ad723fb 100644 --- a/desktop/src/features/messages/lib/useDraftMentionRouting.ts +++ b/desktop/src/features/messages/lib/useDraftMentionRouting.ts @@ -13,6 +13,7 @@ export function useDraftMentionRouting(params: { memberCandidates?: readonly MentionPubkeyCandidate[]; mentionMapRef: React.MutableRefObject>; personaMentionMapRef: React.MutableRefObject>; + selectedAgentPubkeysRef: React.MutableRefObject>; selectedAgentNamesRef: React.MutableRefObject; cancelAutocomplete: () => void; setSelectedNames: (names: string[]) => void; @@ -50,6 +51,11 @@ export function useDraftMentionRouting(params: { const restoreDraftMentionRefs = React.useCallback( (refs: readonly DraftMentionRef[]) => { params.cancelAutocomplete(); + params.selectedAgentPubkeysRef.current = new Set( + refs + .filter((ref) => ref.isAgent) + .map((ref) => ref.pubkey.toLowerCase()), + ); const { names, agentNames } = replaceWithDraftMentionRefs( refs, params.mentionMapRef.current, diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index a5ee508c742..63f8aaa12be 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -12,9 +12,7 @@ import { import { useIsArchivedPredicate } from "@/features/identity-archive/hooks"; import type { MentionSuggestion } from "@/features/messages/ui/MentionAutocomplete"; import { - filterAdmittedMentionPubkeys, filterCachedAgentSuggestions, - getAdmittedAgentPubkeys, getAgentIdentityPubkeys, getMentionableAgentPubkeys, getSharedChannelIds, @@ -190,15 +188,20 @@ export function useMentions( () => getMentionableAgentPubkeys({ currentPubkey, + phase: "prepare", eligibilityScope: mentionChannelId ? { type: "channel", channelId: mentionChannelId } - : { type: "managed-only" }, + : options?.channelType === "dm" + ? { type: "owned", channelId } + : { type: "managed-only" }, managedAgentPubkeys, relayAgents: relayAgentsQuery.data, sharedChannelIds, }), [ currentPubkey, + channelId, + options?.channelType, managedAgentPubkeys, mentionChannelId, relayAgentsQuery.data, @@ -297,10 +300,6 @@ export function useMentions( relayAgentsQuery.data, ], ); - const admittedAgentPubkeys = React.useMemo( - () => getAdmittedAgentPubkeys(mentionCandidates), - [mentionCandidates], - ); const mentionCandidatesWithTeams = React.useMemo( () => [ ...mentionCandidates, @@ -475,6 +474,7 @@ export function useMentions( appendUniqueName(current, trimmedName), ); if (options?.isAgent) { + selectedAgentMentionPubkeysRef.current.add(normalizePubkey(pubkey)); selectedAgentMentionNamesRef.current = appendUniqueName( selectedAgentMentionNamesRef.current, trimmedName, @@ -754,16 +754,11 @@ export function useMentions( selectedDisplayNames: personaMentionMapRef.current.keys(), memberCandidates: mentionCandidates, }); - return filterAdmittedMentionPubkeys( - extracted, - new Set([ - ...agentIdentityPubkeys, - ...selectedAgentMentionPubkeysRef.current, - ]), - admittedAgentPubkeys, - ); + // Selections are intent, not cached authorization. Never discard a + // selected key because a refresh removed it from the picker. + return extracted; }, - [admittedAgentPubkeys, agentIdentityPubkeys, mentionCandidates], + [mentionCandidates], ); const getSelectedAgentPubkeys = React.useRef( () => selectedAgentMentionPubkeysRef.current, @@ -774,7 +769,9 @@ export function useMentions( currentPubkey, eligibilityScope: mentionChannelId ? { type: "channel", channelId: mentionChannelId } - : { type: "managed-only" }, + : options?.channelType === "dm" + ? { type: "owned", channelId } + : { type: "managed-only" }, sharedChannelIds, refetchManagedAgents: managedAgentsQuery.refetch, }); @@ -822,6 +819,7 @@ export function useMentions( mentionMapRef, personaMentionMapRef, selectedAgentNamesRef: selectedAgentMentionNamesRef, + selectedAgentPubkeysRef: selectedAgentMentionPubkeysRef, cancelAutocomplete: cancelMentionAutocomplete, setSelectedNames: setSelectedMentionNames, setSelectedAgentNames: setSelectedAgentMentionNames, diff --git a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs index 073e67a6cdf..cf3c36a36fb 100644 --- a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs +++ b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { snapshotDraftMentionRefs } from "../lib/draftMentionRefs.ts"; +import { AgentMentionAuthorizationError } from "../lib/agentMentionRevalidation.ts"; import { submitMessageEdit } from "./submitMessageEdit.ts"; const UNRESOLVED_USER = "b".repeat(64); @@ -98,13 +99,16 @@ test("edit save revalidates added mentions immediately before save", async () => content.includes("@Agent") ? [agent] : [], revalidateMentionPubkeys: async (pubkeys) => { calls.push(["revalidate", pubkeys]); - return []; + throw new AgentMentionAuthorizationError(); }, + restoreComposer: () => calls.push(["restore"]), + setUploadError: (error) => calls.push(["error", error]), }); assert.deepEqual(calls, [ ["revalidate", [agent]], - ["save", []], + ["restore"], + ["error", new AgentMentionAuthorizationError().message], ]); }); @@ -133,15 +137,18 @@ test("edit upload pause revalidates revoked mentions only after upload completes }, revalidateMentionPubkeys: async (pubkeys) => { calls.push(["revalidate", pubkeys]); - return []; + throw new AgentMentionAuthorizationError(); }, + restoreComposer: () => calls.push(["restore"]), + setUploadError: (error) => calls.push(["error", error]), }); assert.deepEqual(calls, []); await completeUpload(); assert.deepEqual(calls, [ ["revalidate", [agent]], - ["save", []], + ["restore"], + ["error", new AgentMentionAuthorizationError().message], ]); }); diff --git a/desktop/src/features/messages/ui/submitMessageEdit.ts b/desktop/src/features/messages/ui/submitMessageEdit.ts index b8597388bba..a3c3bfecaf5 100644 --- a/desktop/src/features/messages/ui/submitMessageEdit.ts +++ b/desktop/src/features/messages/ui/submitMessageEdit.ts @@ -1,4 +1,8 @@ import { snapshotUnresolvedEditMentionPubkeys } from "@/features/messages/lib/draftMentionRefs"; +import { + AgentMentionAuthorizationError, + type MentionRevalidationOptions, +} from "@/features/messages/lib/agentMentionRevalidation"; import type { QueuedMediaAttachment } from "@/features/messages/lib/backgroundMediaUploadStore"; import { enqueueBackgroundMediaUpload } from "@/features/messages/lib/backgroundMediaUploadStore"; import type { DraftMentionRef } from "@/features/messages/lib/useDrafts"; @@ -47,7 +51,11 @@ type SubmitMessageEditOptions = Omit< ownerPubkey: string | null; restoreComposer: (draft: EditDraft) => void; restoreMentionRefs: (refs: DraftMentionRef[]) => void; - revalidateMentionPubkeys: (pubkeys: readonly string[]) => Promise; + revalidateMentionPubkeys: ( + pubkeys: readonly string[], + channelId?: string | null, + options?: MentionRevalidationOptions, + ) => Promise; shouldRestoreComposer: () => boolean; setDeferredUploadPending: (isPending: boolean) => void; save: ( @@ -141,8 +149,15 @@ export async function submitMessageEdit({ ]), ); if (signal?.aborted) return; - const revalidatedMentionPubkeys = - await revalidateMentionPubkeys(addedMentionPubkeys); + const revalidatedMentionPubkeys = await revalidateMentionPubkeys( + addedMentionPubkeys, + undefined, + { + intendedAgentPubkeys: draft.mentionRefs + .filter((ref) => ref.isAgent) + .map((ref) => ref.pubkey), + }, + ); if (signal?.aborted) return; const outgoingTags = mergeOutgoingTagsWithReferenceMentions( mergeOutgoingTags( @@ -172,8 +187,10 @@ export async function submitMessageEdit({ onComplete: async (uploaded, signal) => { try { await finishEdit(uploaded, signal); - } catch { + } catch (error) { restoreDraft(); + if (error instanceof AgentMentionAuthorizationError) + setUploadError(error.message); } finally { setDeferredUploadPending(false); } @@ -193,7 +210,9 @@ export async function submitMessageEdit({ try { await finishEdit([]); - } catch { + } catch (error) { restoreDraft(); + if (error instanceof AgentMentionAuthorizationError) + setUploadError(error.message); } } diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs index bc11d795126..192fe5b7e6d 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs @@ -5,6 +5,7 @@ import { formatMessageSendError, getErrorMessage, mergeMentionRecipients, + mentionRevalidationOptions, } from "./useMentionSendFlow.helpers.ts"; test("formatMessageSendError preserves the publication failure", () => { @@ -39,3 +40,24 @@ test("address-locked agents join explicit mentions without duplicating recipient "c".repeat(64), ]); }); + +test("revalidation carries captured and prepared agent keys independently of the cleared composer", () => { + const draft = { + inlineAgentMentionPubkeys: ["A".repeat(64)], + addressedAgentPubkeys: ["b".repeat(64)], + }; + assert.deepEqual(mentionRevalidationOptions(draft, "prepare"), { + phase: "prepare", + intendedAgentPubkeys: ["a".repeat(64), "b".repeat(64)], + }); + assert.deepEqual( + mentionRevalidationOptions(draft, "publish", [ + "a".repeat(64), + "c".repeat(64), + ]), + { + phase: "publish", + intendedAgentPubkeys: ["a".repeat(64), "b".repeat(64), "c".repeat(64)], + }, + ); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index e4c4f0d806e..62a5d621e96 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -1,3 +1,4 @@ +import type { MentionRevalidationOptions } from "@/features/messages/lib/agentMentionRevalidation"; import type { ManagedAgent } from "@/shared/api/types"; import { type ImetaMedia, @@ -170,3 +171,36 @@ export function isManagedAgentRunning(agent: ManagedAgent) { export function isProviderBackedAgent(agent: ManagedAgent) { return agent.backend.type === "provider"; } + +/** Carry captured recipient identity through composer clearing and uploads. */ +export function mentionRevalidationOptions( + draft: Pick< + PendingNonMemberMentionSend, + "inlineAgentMentionPubkeys" | "addressedAgentPubkeys" + >, + phase: "prepare" | "publish", + preparedAgentPubkeys: readonly string[] = [], +): MentionRevalidationOptions { + return { + phase, + intendedAgentPubkeys: uniqueNormalizedPubkeys([ + ...draft.inlineAgentMentionPubkeys, + ...draft.addressedAgentPubkeys, + ...preparedAgentPubkeys, + ]), + }; +} + +/** Explicit Send without inviting retains nonmembers only as reference tags. */ +export function withoutInvitingRecipients(draft: PendingNonMemberMentionSend) { + const nonMemberPubkeys = new Set(draft.nonMemberPubkeys.map(normalizePubkey)); + return { + mentionPubkeys: draft.mentionPubkeys.filter( + (pubkey) => !nonMemberPubkeys.has(normalizePubkey(pubkey)), + ), + outgoingTags: mergeOutgoingTagsWithReferenceMentions( + draft.outgoingTags, + nonMemberPubkeys, + ), + }; +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index b29e9956cc9..a4b412608c9 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -34,9 +34,10 @@ import { enqueueAgentWake, formatMessageSendError, getErrorMessage, + mentionRevalidationOptions, + withoutInvitingRecipients, mergeMentionRecipients, MENTION_REFERENCE_TAG, - mergeOutgoingTagsWithReferenceMentions, type PendingNonMemberMentionSend, type QueuedAgentWake, type SendMessageWithMentionFlowInput, @@ -44,6 +45,7 @@ import { uniqueNormalizedPubkeys, } from "./useMentionSendFlow.helpers"; import { buildAgentAddressMentionTags } from "@/features/messages/lib/agentAddressMention.mjs"; +import { AgentMentionAuthorizationError } from "@/features/messages/lib/agentMentionRevalidation"; import type { UseMentionSendFlowOptions } from "./useMentionSendFlow.types"; export function useMentionSendFlow({ @@ -378,7 +380,11 @@ export function useMentionSendFlow({ let uploadStarted = false; try { const admittedMentionPubkeys = uniqueNormalizedPubkeys( - await mentions.revalidateMentionPubkeys(mentionPubkeys), + await mentions.revalidateMentionPubkeys( + mentionPubkeys, + draft.capturedChannelId, + mentionRevalidationOptions(draft, "prepare"), + ), ); if (isSendCancelled()) return restoreComposerAfterFailure(); if (!isMountedRef.current) return persistPreflightDraft(); @@ -388,7 +394,9 @@ export function useMentionSendFlow({ (pubkey) => admittedMentionPubkeySet.has(pubkey), ), ); - const managedAgentsByPubkey = await getManagedAgentsByPubkey(); + const managedAgentsByPubkey = await getManagedAgentsByPubkey().catch( + () => new Map(), + ); if (isSendCancelled()) return restoreComposerAfterFailure(); if (!isMountedRef.current) { persistPreflightDraft(); @@ -503,7 +511,15 @@ export function useMentionSendFlow({ // whatever did or did not separate it from the admission pass // above (#5681). const revalidatedMentionPubkeys = - await mentions.revalidateMentionPubkeys(mentionPubkeys); + await mentions.revalidateMentionPubkeys( + mentionPubkeys, + sendChannelId, + mentionRevalidationOptions( + draft, + "publish", + preparedAgentPubkeys, + ), + ); if (signal?.aborted || isSendCancelled()) return; const finalTagsWithAgentAddress = [ ...finalOutgoingTags, @@ -571,7 +587,11 @@ export function useMentionSendFlow({ await finishSend(uploaded, signal); } catch (error) { restoreComposerAfterFailure(); - toast.error(formatMessageSendError(error)); + toast.error( + error instanceof AgentMentionAuthorizationError + ? error.message + : formatMessageSendError(error), + ); } finally { settleUpload(); } @@ -599,12 +619,18 @@ export function useMentionSendFlow({ await finishSend([]); } catch (error) { restoreComposerAfterFailure(); - toast.error(formatMessageSendError(error)); + toast.error( + error instanceof AgentMentionAuthorizationError + ? error.message + : formatMessageSendError(error), + ); } } } catch (error) { restoreComposerAfterFailure(); - throw error; + toast.error( + getErrorMessage(error, "Could not send message. Please retry."), + ); } finally { if (draft.preparedLinkPreviews) { activePreparedLinkPreviews.delete(draft.preparedLinkPreviews); @@ -834,18 +860,8 @@ export function useMentionSendFlow({ }, [mentions.getMentionDisplayName, pendingNonMemberSend]); const handleSendWithoutInviting = React.useCallback(() => { if (!pendingNonMemberSend) return; - const nonMemberPubkeys = new Set( - pendingNonMemberSend.nonMemberPubkeys.map((pubkey) => - normalizePubkey(pubkey), - ), - ); - const mentionPubkeys = pendingNonMemberSend.mentionPubkeys.filter( - (pubkey) => !nonMemberPubkeys.has(normalizePubkey(pubkey)), - ); - const outgoingTags = mergeOutgoingTagsWithReferenceMentions( - pendingNonMemberSend.outgoingTags, - nonMemberPubkeys, - ); + const { mentionPubkeys, outgoingTags } = + withoutInvitingRecipients(pendingNonMemberSend); void completeSend(pendingNonMemberSend, mentionPubkeys, outgoingTags); }, [completeSend, pendingNonMemberSend]); const handleInviteNonMembers = React.useCallback(() => { @@ -857,10 +873,14 @@ export function useMentionSendFlow({ setNonMemberPromptError(null); void (async () => { const mentionPubkeys = uniqueNormalizedPubkeys( - await mentions.revalidateMentionPubkeys([ - ...pendingNonMemberSend.mentionPubkeys, - ...pendingNonMemberSend.nonMemberPubkeys, - ]), + await mentions.revalidateMentionPubkeys( + [ + ...pendingNonMemberSend.mentionPubkeys, + ...pendingNonMemberSend.nonMemberPubkeys, + ], + pendingNonMemberSend.capturedChannelId, + mentionRevalidationOptions(pendingNonMemberSend, "prepare"), + ), ); const admittedMentionPubkeys = new Set(mentionPubkeys); const originalNonMemberPubkeys = new Set( @@ -874,7 +894,9 @@ export function useMentionSendFlow({ tag[0] !== MENTION_REFERENCE_TAG || !originalNonMemberPubkeys.has(normalizePubkey(tag[1] ?? "")), ); - const managedAgentsByPubkey = await getManagedAgentsByPubkey(); + const managedAgentsByPubkey = await getManagedAgentsByPubkey().catch( + () => new Map(), + ); if (!isMountedRef.current) return; const peoplePubkeys: string[] = []; const relayAgentPubkeys: string[] = []; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index d54c32e4c48..6ddc1111b03 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -4254,6 +4254,14 @@ function syncMockRelayAgentsFromManagedAgents() { }, ); + // Owned discovery includes nonmembers, but membership must come from the + // actual roster, including additions and newly created DMs. + for (const agent of baseAgents) { + if (agent.owner_pubkey !== MOCK_IDENTITY_PUBKEY) continue; + const membership = getManagedAgentRelayMembership(agent.pubkey); + agent.channel_ids = membership.channelIds; + agent.channels = membership.channels; + } mockRelayAgents = [...baseAgents, ...managedAgentsAsRelay]; } @@ -13539,7 +13547,9 @@ export function maybeInstallE2eTauriMocks() { (agent) => requested.has(agent.pubkey.toLowerCase()) && !revoked.has(agent.pubkey.toLowerCase()) && - (!channelId || agent.channel_ids.includes(channelId)), + (!channelId || + agent.channel_ids.includes(channelId) || + agent.owner_pubkey === MOCK_IDENTITY_PUBKEY), ); } case "list_personas": diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index efc7e06ab89..d30e2cd27ae 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -15,6 +15,20 @@ test.beforeEach(async ({ page }) => { await installMockBridge(page); }); +test.afterEach(async ({ page }, testInfo) => { + if (testInfo.status !== testInfo.expectedStatus) { + await testInfo.attach("outgoing-diagnostic", { + body: JSON.stringify( + await page.evaluate(() => ({ + events: window.__BUZZ_E2E_SIGNED_EVENTS__, + commands: window.__BUZZ_E2E_COMMAND_LOG__, + })), + ), + contentType: "application/json", + }); + } +}); + const IN_CHANNEL_MANAGED_AGENT_PUBKEY = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const OUT_OF_CHANNEL_MANAGED_AGENT_PUBKEY = @@ -1858,6 +1872,25 @@ test("forum sends revalidate relay-agent authorization before signing", async ({ await expect(page.getByTestId("chat-title")).toHaveText("watercooler"); await page.getByRole("button", { name: "Start a new post..." }).click(); + await page.evaluate( + async ({ channelId, pubkey }) => { + const invoke = window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__; + if (!invoke) throw new Error("Mock bridge is not installed."); + await invoke("add_channel_members", { + channelId, + pubkeys: [pubkey], + role: "bot", + }); + await window.__BUZZ_E2E_QUERY_CLIENT__?.invalidateQueries({ + queryKey: ["channels", channelId, "members"], + }); + }, + { + channelId: "a27e1ee9-76a6-5bdf-a5d5-1d85610dad11", + pubkey: ALLOWLIST_RELAY_AGENT_PUBKEY, + }, + ); + const input = page.getByTestId("message-input"); await input.fill("@quinn"); await page.getByTestId("mention-autocomplete").getByText("quinn").click(); @@ -1888,12 +1921,11 @@ test("forum sends revalidate relay-agent authorization before signing", async ({ await expect(input).not.toContainText("later edit"); const outgoingContent = `@quinn hello\n[forum-race.pdf](https://mock.relay/media/${"f".repeat(64)}.pdf)`; - await expect - .poll(() => readOutgoingMentionPubkeys(page, outgoingContent)) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, outgoingContent)) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(input).toContainText("@quinn hello"); + expect(await readOutgoingMentionPubkeys(page, outgoingContent)).toBeNull(); }); test("managed agents use the channel roster for membership labels", async ({ @@ -2168,16 +2200,15 @@ test("targeted revocation before send causes no agent side effects", async ({ const baselineCommands = await readCommandLog(page); await page.getByTestId("send-message").click(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(input).toHaveText("@quinn hello"); + expect(await readOutgoingMentionPubkeys(page, "@quinn hello")).toBeNull(); const commands = await readCommandLog(page); // Admission pass plus the unconditional publish-boundary pass. expect(commandCount(commands, "revalidate_relay_agents")).toBe( - commandCount(baselineCommands, "revalidate_relay_agents") + 2, + commandCount(baselineCommands, "revalidate_relay_agents") + 1, ); expect(commandCount(commands, "list_relay_agents")).toBe( commandCount(baselineCommands, "list_relay_agents"), @@ -2661,12 +2692,11 @@ test("selected relay agents revoked after the invite prompt cause no side effect const baselineCommands = await readCommandLog(page); await inviteButton.click(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(input).toHaveText("@quinn hello"); + expect(await readOutgoingMentionPubkeys(page, "@quinn hello")).toBeNull(); const commands = await readCommandLog(page); for (const command of [ "add_channel_members", @@ -2716,12 +2746,11 @@ test("selected relay agents revoked during send emit no p tag", async ({ ); }); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(input).toHaveText("@quinn hello"); + expect(await readOutgoingMentionPubkeys(page, "@quinn hello")).toBeNull(); }); test("owner-only builds admit cross-owner relay agents authorized by allowlist", async ({ diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts new file mode 100644 index 00000000000..18b1f3ba91a --- /dev/null +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -0,0 +1,246 @@ +import { expect, test, type Page } from "@playwright/test"; +import { + installMockBridge, + openNewMessagePage, + TEST_IDENTITIES, +} from "../helpers/bridge"; + +const OWNER = "deadbeef".repeat(8); +const REMOTE = "ed".repeat(32); +const GENERAL = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"; + +async function install(page: Page) { + await installMockBridge(page, { + ownerOnlyAccessBuild: true, + managedAgents: [], + searchProfiles: [ + { + pubkey: REMOTE, + displayName: "RemoteScout", + ownerPubkey: OWNER, + isAgent: true, + }, + ], + relayAgents: [ + { + pubkey: REMOTE, + name: "RemoteScout", + ownerPubkey: OWNER, + respondTo: "allowlist", + respondToAllowlist: [], + channelNames: [], + }, + ], + }); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); +} +async function select(page: Page) { + await page.getByTestId("message-input").fill("@Remote"); + const row = page.getByTestId(`mention-suggestion-${REMOTE}`); + await expect(row).toContainText("RemoteScout"); + await row.locator("button").first().click(); + await page.keyboard.type("hello"); +} +async function sent(page: Page) { + return page.evaluate(() => { + const signed = (window.__BUZZ_E2E_SIGNED_EVENTS__ ?? []) + .filter((event) => event.content === "@RemoteScout hello") + .map((event) => + event.tags.filter((tag) => tag[0] === "p").map((tag) => tag[1]), + ); + if (signed.length) return signed; + // New DMs deliberately use the acknowledged native HTTP command rather + // than JS sign_event. Assert its exact outgoing recipients, not fake crypto. + return (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).flatMap((call) => { + const payload = call.payload as { + content?: string; + mentionPubkeys?: string[]; + }; + return call.command === "send_channel_message" && + payload.content === "@RemoteScout hello" + ? [payload.mentionPubkeys ?? []] + : []; + }); + }); +} +async function assertNoLocalLifecycle(page: Page) { + const commands = await page.evaluate( + () => window.__BUZZ_E2E_COMMANDS__ ?? [], + ); + for (const command of [ + "start_managed_agent", + "create_managed_agent", + "attach_managed_agent", + ]) { + expect(commands).not.toContain(command); + } +} +for (const role of ["member", "bot"] as const) { + test(`owned ${role} with empty local roster emits exact p tag`, async ({ + page, + }) => { + await install(page); + await page.evaluate( + async ({ pubkey, channelId, role }) => { + await window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.("add_channel_members", { + channelId, + pubkeys: [pubkey], + role, + }); + await window.__BUZZ_E2E_QUERY_CLIENT__?.invalidateQueries({ + queryKey: ["channels"], + }); + await window.__BUZZ_E2E_QUERY_CLIENT__?.invalidateQueries({ + queryKey: ["relay-agents"], + }); + }, + { pubkey: REMOTE, channelId: GENERAL, role }, + ); + await select(page); + await page.getByTestId("send-message").click(); + await expect.poll(() => sent(page)).toEqual([[REMOTE]]); + await expect( + page.getByRole("button", { name: "Invite", exact: true }), + ).toHaveCount(0); + await assertNoLocalLifecycle(page); + }); +} +test("owned nonmember uses authorized add before exact publication", async ({ + page, +}) => { + await install(page); + await select(page); + await page.getByTestId("send-message").click(); + const invite = page.getByRole("button", { name: "Invite", exact: true }); + await expect(invite).toBeVisible(); + expect(await sent(page)).toEqual([]); + await invite.click(); + await expect.poll(() => sent(page)).toEqual([[REMOTE]]); + const calls = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__ ?? [], + ); + expect(calls).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + command: "add_channel_members", + payload: expect.objectContaining({ pubkeys: [REMOTE], role: "bot" }), + }), + ]), + ); + await assertNoLocalLifecycle(page); +}); +for (const error of [ + "actor not authorized", + "policy:nobody — this agent has disabled external channel additions", +]) { + test(`failed add keeps draft and sends nothing: ${error}`, async ({ + page, + }) => { + await install(page); + await select(page); + await page.evaluate((error) => { + window.__BUZZ_E2E__.mock ??= {}; + window.__BUZZ_E2E__.mock.addChannelMembersErrors = [error]; + }, error); + await page.getByTestId("send-message").click(); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await expect(page.getByText(error, { exact: true })).toBeVisible(); + await expect(page.getByTestId("message-input")).toHaveText( + "@RemoteScout hello", + ); + expect(await sent(page)).toEqual([]); + await assertNoLocalLifecycle(page); + }); +} +test("selected owned agent revoked before add keeps draft and sends nothing", async ({ + page, +}) => { + await install(page); + await select(page); + await page.getByTestId("send-message").click(); + await page.evaluate((pubkey) => { + window.__BUZZ_E2E__.mock ??= {}; + window.__BUZZ_E2E__.mock.relayAgentRevalidationRevokedPubkeys = [pubkey]; + }, REMOTE); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(page.getByTestId("message-input")).toHaveText( + "@RemoteScout hello", + ); + expect(await sent(page)).toEqual([]); +}); + +for (const mode of ["existing", "new"] as const) { + test(`${mode} DM prepares actual destination for owned relay mention`, async ({ + page, + }) => { + await install(page); + if (mode === "existing") { + await page.getByTestId("channel-bob-tyler").click(); + await expect(page.getByTestId("chat-title")).toHaveText("bob-tyler"); + } else { + await openNewMessagePage(page); + await page.getByTestId("new-dm-search").fill("bob"); + await page + .getByTestId(`new-dm-result-${TEST_IDENTITIES.bob.pubkey}`) + .click(); + await page.getByTestId("new-dm-search").press("Escape"); + } + await select(page); + await page.getByTestId("send-message").click(); + await expect + .poll(() => sent(page)) + .toEqual([[REMOTE, TEST_IDENTITIES.bob.pubkey]]); + const calls = await page.evaluate( + () => window.__BUZZ_E2E_COMMAND_LOG__ ?? [], + ); + const checks = calls.filter( + (call) => call.command === "revalidate_relay_agents", + ); + const event = await page.evaluate(() => + (window.__BUZZ_E2E_SIGNED_EVENTS__ ?? []).find( + (event) => event.content === "@RemoteScout hello", + ), + ); + expect(checks.at(-1)?.payload).toMatchObject({ + channelId: + event?.tags.find((tag) => tag[0] === "h")?.[1] ?? + ( + calls.find((call) => call.command === "send_channel_message") + ?.payload as { channelId?: string } + )?.channelId, + pubkeys: [REMOTE], + }); + await assertNoLocalLifecycle(page); + }); +} + +test("membership revoked at final publish keeps draft and emits no message", async ({ + page, +}) => { + await install(page); + await select(page); + await page.getByTestId("send-message").click(); + // Let preparation succeed, but make the fresh final directory read fail. + await page.evaluate(() => { + window.__BUZZ_E2E__.mock ??= {}; + window.__BUZZ_E2E__.mock.relayAgentListErrors = [ + null, + null, + "revoked at publication", + ]; + }); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await expect(page.getByTestId("message-input")).toHaveText( + "@RemoteScout hello", + ); + expect(await sent(page)).toEqual([]); +}); + diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md new file mode 100644 index 00000000000..25696ce420d --- /dev/null +++ b/docs/remote-mention-routing.md @@ -0,0 +1,31 @@ +# Remote mention preparation and publication + +Owned relay agents can be selected and invited without local runtime custody. +The picker and preparation phase evaluate policy but may admit an owned +nonmember. Publication separately refreshes authorization for the exact eventual +destination, including a newly created DM. Ownership is not membership and +cached picker evidence is never publication authorization. + +Selections are intent: a vanished/revoked key must fail visibly, retain the draft +and send nothing, rather than silently dropping a recipient. Captured agent keys +survive composer clearing, media upload, edits and asynchronous preparation. +A failed local inventory read cannot veto authenticated relay identities or +admit stale local runtimes. Locally managed runtime readiness remains a separate +existing path; remote identities never gain synthetic local management records. + +Chat offers explicit Invite or reference-only send without inviting. Failed adds, +revoked policy, failed final authorization and cancellation preserve recoverable +drafts. Standalone forum sends report authorization failures, but standalone +forum invitation is a subsequent change reusing this phase contract. + +Native discovery prerequisite: `docs/owned-agent-discovery.md` (PR6). +Regression coverage: `agentAutocompleteEligibility.test.mjs`, +`agentMentionRevalidation.test.mjs`, `useMentionSendFlow.helpers.test.mjs`, +`submitMessageEdit.test.mjs`, `mentions.spec.ts`, and +`remote-owned-mentions.spec.ts`. The new remote browser fixtures use a single-word +name deliberately: mention separator behavior belongs to the independent PR1. + +NIP-OA establishes ownership, not physical hosting, availability, or lifecycle +control. Final native queries do not provide an atomic relay transaction with +message publication. Independent review of both native and publication boundaries +is required before landing. From 43d3c80e5395e7ca2124251a99f72c7c514bac51 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 13:46:38 -0400 Subject: [PATCH 02/13] style(desktop): format remote mention regression fixture Signed-off-by: Logan Johnson --- desktop/tests/e2e/remote-owned-mentions.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index 18b1f3ba91a..b9ac4889a1b 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -243,4 +243,3 @@ test("membership revoked at final publish keeps draft and emits no message", asy ); expect(await sent(page)).toEqual([]); }); - From 194599d6ed75f1dfc6ff79fd34c7ce2062330c88 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 13:47:54 -0400 Subject: [PATCH 03/13] test(desktop): capture remote invitation and failure evidence Signed-off-by: Logan Johnson --- desktop/tests/e2e/remote-owned-mentions.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index b9ac4889a1b..3daac3844d3 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -1,3 +1,4 @@ +import { waitForAnimations } from "../helpers/animations"; import { expect, test, type Page } from "@playwright/test"; import { installMockBridge, @@ -115,9 +116,13 @@ test("owned nonmember uses authorized add before exact publication", async ({ await page.getByTestId("send-message").click(); const invite = page.getByRole("button", { name: "Invite", exact: true }); await expect(invite).toBeVisible(); + await waitForAnimations(page); + await page.screenshot({ path: "test-results/remote-invite.png" }); expect(await sent(page)).toEqual([]); await invite.click(); await expect.poll(() => sent(page)).toEqual([[REMOTE]]); + await waitForAnimations(page); + await page.screenshot({ path: "test-results/remote-sent.png" }); const calls = await page.evaluate( () => window.__BUZZ_E2E_COMMAND_LOG__ ?? [], ); @@ -147,6 +152,10 @@ for (const error of [ await page.getByTestId("send-message").click(); await page.getByRole("button", { name: "Invite", exact: true }).click(); await expect(page.getByText(error, { exact: true })).toBeVisible(); + await waitForAnimations(page); + await page.screenshot({ + path: `test-results/remote-error-${error.split(" ")[0]}.png`, + }); await expect(page.getByTestId("message-input")).toHaveText( "@RemoteScout hello", ); From e5419117666a3cb462f3577bf2be9417ee952e43 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 14:14:19 -0400 Subject: [PATCH 04/13] fix(desktop): cancel superseded remote invitation continuations Keep invitation intent alive through preparation, add, and publication, while cancelling dismissal, navigation and replacement. Retain cleared drafts on late cancellation and cover async boundaries with real React and browser regressions. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- .../ui/useEnsureAgentMentionsReady.ts | 4 + .../useMentionSendFlow.cancellation.test.mjs | 351 ++++++++++++++++++ .../messages/ui/useMentionSendFlow.helpers.ts | 1 + .../messages/ui/useMentionSendFlow.ts | 128 ++----- .../messages/ui/useNonMemberInvite.ts | 173 +++++++++ .../tests/e2e/remote-owned-mentions.spec.ts | 113 ++++++ docs/remote-mention-routing.md | 18 + 7 files changed, 689 insertions(+), 99 deletions(-) create mode 100644 desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs create mode 100644 desktop/src/features/messages/ui/useNonMemberInvite.ts diff --git a/desktop/src/features/messages/ui/useEnsureAgentMentionsReady.ts b/desktop/src/features/messages/ui/useEnsureAgentMentionsReady.ts index 4c3485275ed..739d0770465 100644 --- a/desktop/src/features/messages/ui/useEnsureAgentMentionsReady.ts +++ b/desktop/src/features/messages/ui/useEnsureAgentMentionsReady.ts @@ -37,6 +37,7 @@ export type EnsureAgentMentionsReady = ( capturedChannelId: string, preparedParticipantPubkeys?: string[], preparedManagedAgents?: ManagedAgent[], + isCancelled?: () => boolean, ) => Promise; type AttachAgentToChannel = (input: { @@ -75,6 +76,7 @@ export function useEnsureAgentMentionsReady({ capturedChannelId: string, preparedParticipantPubkeys: string[] = [], preparedManagedAgents: ManagedAgent[] = [], + isCancelled: () => boolean = () => false, ) => { if (!capturedChannelId || mentionPubkeys.length === 0) { return { @@ -101,6 +103,7 @@ export function useEnsureAgentMentionsReady({ let wroteRelayState = false; const agentsToWake: QueuedAgentWake[] = []; for (const pubkey of uniqueNormalizedPubkeys(mentionPubkeys)) { + if (isCancelled()) break; const agent = managedAgentsByPubkey.get(pubkey); if (!agent) continue; try { @@ -116,6 +119,7 @@ export function useEnsureAgentMentionsReady({ // policy reports `wrote: false`. wroteRelayState = true; } + if (isCancelled()) break; if (participants.has(pubkey)) { if ( (isProviderBackedAgent(readyAgent) && diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs new file mode 100644 index 00000000000..1909b2541c2 --- /dev/null +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -0,0 +1,351 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import vm from "node:vm"; +import { after, afterEach, before, test } from "node:test"; +import { JSDOM } from "jsdom"; +import * as React from "react"; +import ts from "typescript"; +import * as helpers from "./useMentionSendFlow.helpers.ts"; + +// Execute the product hooks with real React effects/renders; only external +// query/mutation/media dependencies are mocked. Deferred promises isolate the +// user-intent boundary independently of successful authorization. +const dom = new JSDOM("", { + url: "http://localhost", +}); +before(() => + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }), +); +afterEach(async () => (await import("@testing-library/react")).cleanup()); +after(() => dom.window.close()); +const KEY = "b".repeat(64); +const TEXT = "@RemoteScout hello"; +const noop = () => {}; +function deferred() { + let resolve; + let reject; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} +function load(name, stubs) { + const source = fs.readFileSync( + new URL(`./${name}.ts`, import.meta.url), + "utf8", + ); + const exports = {}; + vm.runInNewContext( + ts.transpileModule(source, { + compilerOptions: { + module: ts.ModuleKind.CommonJS, + target: ts.ScriptTarget.ES2022, + }, + }).outputText, + { + exports, + AbortController, + Error, + Map, + Set, + require: (key) => { + assert.ok(key in stubs, `unmocked dependency: ${key}`); + return stubs[key]; + }, + }, + ); + return exports; +} +async function setup() { + const { act, renderHook } = await import("@testing-library/react"); + const calls = []; + const control = { prepare: null, add: null, publish: null, inventory: null }; + const refs = [{ displayName: "RemoteScout", pubkey: KEY, isAgent: true }]; + const query = { + data: [], + refetch: async () => { + if (control.inventory) await control.inventory.promise; + return { data: [] }; + }, + }; + const mutation = { + isPending: false, + mutateAsync: async (input) => { + calls.push(["add", input]); + if (control.add) await control.add.promise; + return { added: [KEY], errors: [] }; + }, + }; + const stubs = { + react: React, + sonner: { toast: { error: (error) => calls.push(["error", error]) } }, + "@/features/agents/hooks": new Proxy( + {}, + { + get: (_, key) => + key.includes("Mutation") ? () => mutation : () => query, + }, + ), + "@/features/agents/channelAgents": { + applyReusableAgentAccessPolicy: async (agent) => { + calls.push(["local-policy"]); + if (control.policy) await control.policy.promise; + return agent; + }, + }, + "@/features/agents/lib/resolvePersonaRuntime": {}, + "@/features/channels/hooks": { + useAddChannelMembersMutation: () => mutation, + }, + "@/features/channels/useCanAddChannelMembers": { + useCanAddChannelMembers: () => true, + }, + "@/features/channels/lib/channelMemberAdmission": {}, + "@/features/messages/lib/dmThreadAgentMentionError": { + dmThreadAgentMentionError: () => null, + }, + "@/features/messages/lib/backgroundMediaUploadStore": { + saveQueuedAttachmentsForDraft: noop, + }, + "@/features/messages/lib/imetaMediaMarkdown": { + buildOutgoingMessage: (text) => ({ content: text, mediaTags: [] }), + }, + "@/shared/api/tauri": { invokeTauri: async () => {} }, + "@/shared/lib/pubkey": { + normalizePubkey: (key) => key.toLowerCase(), + truncatePubkey: (key) => key, + }, + "@/shared/lib/customEmojiTags": { buildCustomEmojiTags: () => [] }, + "./useMentionSendFlow.helpers": helpers, + "@/features/messages/lib/agentAddressMention.mjs": { + buildAgentAddressMentionTags: () => [], + }, + "@/features/messages/lib/agentMentionRevalidation": { + AgentMentionAuthorizationError: class extends Error {}, + }, + }; + stubs["./useNonMemberInvite"] = load("useNonMemberInvite", stubs); + stubs["./useActivePreparedLinkPreviews"] = load( + "useActivePreparedLinkPreviews", + stubs, + ); + const { useMentionSendFlow } = load("useMentionSendFlow", stubs); + const options = { + channelId: "general", + channelType: "stream", + customEmoji: [], + mentions: { + memberPubkeys: new Set(), + hasResolvedMembers: true, + extractMentionPersonas: () => [], + extractMentionPubkeys: () => [KEY], + isAgentPubkey: (key) => key === KEY, + isManagedAgentPubkey: () => false, + getDraftMentionRefs: () => refs, + getMentionDisplayName: () => "RemoteScout", + clearMentions: noop, + restoreDraftMentionRefs: (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; + return keys; + }, + }, + contentRef: { current: TEXT }, + channelLinks: { clearChannels: noop }, + emojiAutocomplete: { clearEmojis: noop }, + richText: { clearContent: noop, setContent: noop }, + drafts: { + loadDraft: () => null, + persistDraft: (...args) => calls.push(["persist", ...args]), + markDraftSent: noop, + }, + setContent: noop, + setPendingImeta: noop, + setIsEmojiPickerOpen: noop, + clearQueuedAttachments: noop, + restoreQueuedAttachments: noop, + hasUnsavedMedia: () => false, + onSendRef: { current: async (...args) => calls.push(["SEND", ...args]) }, + }; + const hook = renderHook(() => useMentionSendFlow(options), { + wrapper: ({ children }) => + React.createElement(React.StrictMode, null, children), + }); + const flush = async () => + act(async () => { + await new Promise((resolve) => setImmediate(resolve)); + }); + const prompt = async (text = TEXT) => + act(async () => { + options.contentRef.current = text; + await hook.result.current.sendMessageWithMentionFlow({ + capturedChannelId: options.channelId, + pendingImeta: [], + trimmed: text, + recoveryDraftKey: "general", + }); + }); + await prompt(); + const invite = async () => + act(async () => hook.result.current.nonMemberPromptProps.onInvite()); + const dismiss = () => + act(() => hook.result.current.nonMemberPromptProps.onDismiss()); + const finish = async (gate) => { + gate.resolve(); + await flush(); + }; + const events = (name) => calls.filter((call) => call[0] === name); + return { + ...hook, + act, + calls, + control, + options, + query, + refs, + prompt, + invite, + dismiss, + finish, + flush, + events, + }; +} + +for (const stage of ["prepare", "inventory", "add"]) { + test(`dismiss during delayed ${stage} cannot add further or send, retains draft`, async () => { + const s = await setup(); + const gate = deferred(); + s.control[stage] = gate; + if (stage === "inventory") { + s.query.data = undefined; + s.rerender(); + } + await s.invite(); + assert.equal(s.result.current.nonMemberPromptProps.isInvitePending, true); + s.dismiss(); + assert.equal(s.result.current.nonMemberPromptProps.open, false); + await s.finish(gate); + assert.equal(s.events("add").length, stage === "add" ? 1 : 0); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.options.contentRef.current, TEXT); + }); +} +for (const action of ["navigation", "unmount", "replacement"]) { + test(`delayed add ${action} invalidates late completion`, async () => { + const s = await setup(); + const gate = deferred(); + s.control.add = gate; + await s.invite(); + assert.equal(s.events("add").length, 1); + if (action === "navigation") { + s.options.channelId = "random"; + s.rerender(); + } + if (action === "unmount") s.unmount(); + if (action === "replacement") await s.prompt("@RemoteScout replacement"); + await s.finish(gate); + assert.equal(s.events("SEND").length, 0); + if (action === "replacement") { + s.control.add = null; + await s.invite(); + assert.equal(s.events("SEND").length, 1); + assert.equal(s.events("SEND")[0][1], "@RemoteScout replacement"); + } + }); +} +test("normal Invite survives promotion/render and synchronous double click sends exactly once", async () => { + const s = await setup(); + const gate = deferred(); + s.control.publish = gate; + await s.act(async () => { + s.result.current.nonMemberPromptProps.onInvite(); + s.result.current.nonMemberPromptProps.onInvite(); + }); + assert.equal(s.events("add").length, 1); + assert.equal(s.events("publish").length, 1); + assert.equal(s.result.current.nonMemberPromptProps.open, false); + s.rerender(); // clearing the prompt is NOT cancellation + await s.finish(gate); + assert.equal(s.events("SEND").length, 1); + assert.deepEqual(Array.from(s.events("SEND")[0][2]), [KEY]); + assert.equal(s.events("SEND")[0][4], "general"); + assert.equal(s.result.current.isPreparingMentionSend, false); +}); +for (const action of ["dismissal", "navigation", "unmount"]) { + test(`signal remains live through final validation: ${action} restores recoverable draft, no send`, async () => { + const s = await setup(); + const gate = deferred(); + s.control.publish = gate; + await s.invite(); + assert.equal(s.events("publish").length, 1); + assert.equal(s.options.contentRef.current, ""); + if (action === "dismissal") s.dismiss(); + if (action === "navigation") { + s.options.channelId = "random"; + s.rerender(); + } + if (action === "unmount") s.unmount(); + await s.finish(gate); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.events("persist")[0][2], TEXT); + assert.deepEqual(s.events("persist")[0][6], s.refs); + if (action === "dismissal") { + assert.equal(s.options.contentRef.current, TEXT); + assert.deepEqual(s.events("restore-refs")[0][1], s.refs); + } + }); +} +test("late cancelled failure cannot reset a newer pending attempt", async () => { + const s = await setup(); + const old = deferred(); + s.control.add = old; + await s.invite(); + s.dismiss(); + await s.prompt(); + const current = deferred(); + s.control.add = current; + await s.invite(); + old.reject(new Error("obsolete add failure")); + await s.flush(); + assert.equal(s.result.current.nonMemberPromptProps.isInvitePending, true); + assert.equal(s.result.current.nonMemberPromptProps.error, null); + await s.finish(current); + assert.equal(s.events("SEND").length, 1); +}); +test("reference-only supersedes preparation and emits no triggering recipient", async () => { + const s = await setup(); + const old = deferred(); + s.control.prepare = old; + await s.invite(); + s.control.prepare = null; + await s.act(async () => s.result.current.nonMemberPromptProps.onDoNothing()); + await s.finish(old); + assert.equal(s.events("add").length, 0); + assert.equal(s.events("SEND").length, 1); + assert.deepEqual(Array.from(s.events("SEND")[0][2]), []); +}); + +// Readiness is a nested continuation owned by completeSend. Cancelling while +// policy preparation is pending must also stop a subsequent local attachment. +test("cancelled invitation cannot attach a local recipient after delayed policy preparation", async () => { + const s = await setup(); + s.query.data = [{ pubkey: KEY, name: "LocalScout", status: "running" }]; + s.rerender(); + const gate = deferred(); + s.control.policy = gate; + await s.invite(); + assert.equal(s.events("local-policy").length, 1); + s.dismiss(); + await s.finish(gate); + assert.equal(s.events("add").length, 0); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.options.contentRef.current, TEXT); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index 62a5d621e96..cc6bb0a6224 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -58,6 +58,7 @@ export function dedupeQueuedAgentWakes( } export type PendingNonMemberMentionSend = { + invitationSignal?: AbortSignal; addressedAgentPubkeys: string[]; inlineAgentMentionPubkeys: string[]; capturedChannelId: string | null; diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index a4b412608c9..f6cd2573690 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -12,7 +12,7 @@ import { import { resolvePersonaRuntime } from "@/features/agents/lib/resolvePersonaRuntime"; import { useAddChannelMembersMutation } from "@/features/channels/hooks"; import { useCanAddChannelMembers } from "@/features/channels/useCanAddChannelMembers"; -import { PRIVATE_CHANNEL_ADD_DENIED_MESSAGE } from "@/features/channels/lib/channelMemberAdmission"; +import { useNonMemberInvite } from "./useNonMemberInvite"; import { dmThreadAgentMentionError } from "@/features/messages/lib/dmThreadAgentMentionError"; import { prepareBackgroundMediaUpload, @@ -37,7 +37,6 @@ import { mentionRevalidationOptions, withoutInvitingRecipients, mergeMentionRecipients, - MENTION_REFERENCE_TAG, type PendingNonMemberMentionSend, type QueuedAgentWake, type SendMessageWithMentionFlowInput, @@ -278,7 +277,9 @@ export function useMentionSendFlow({ return; } const sendSignal = draft.preparedLinkPreviews?.signal; - const isSendCancelled = () => sendSignal?.aborted === true; + const isSendCancelled = () => + sendSignal?.aborted === true || + draft.invitationSignal?.aborted === true; if (isSendCancelled()) return draft.preparedLinkPreviews?.release(); isCompleteSendPendingRef.current = true; setIsCompleteSendPending(true); @@ -302,7 +303,9 @@ export function useMentionSendFlow({ ); }; const persistCanceledDraft = () => { - if (isSendCancelled() || !draft.recoveryDraftKey) return; + // Invitation cancellation still owes the captured draft recovery. Link + // preview cancellation retains its existing independent recovery owner. + if (sendSignal?.aborted || !draft.recoveryDraftKey) return; const existing = drafts.loadDraft(draft.recoveryDraftKey); if ( existing && @@ -436,6 +439,7 @@ export function useMentionSendFlow({ sendChannelId ?? "", onPrepareSendChannel ? preparedAgentPubkeys : [], [...managedAgentsByPubkey.values()], + isSendCancelled, ); // Every wake this send queued: persona creates carried on the draft // (enqueued before the non-member prompt could defer us here), then @@ -505,7 +509,7 @@ export function useMentionSendFlow({ outgoingTags, ); if (!finalOutgoingTags || signal?.aborted || isSendCancelled()) - return; + return restoreComposerAfterFailure(); // The pass immediately before signing/publish is always fresh: // mention authorization is re-validated here unconditionally, // whatever did or did not separate it from the admission pass @@ -520,7 +524,8 @@ export function useMentionSendFlow({ preparedAgentPubkeys, ), ); - if (signal?.aborted || isSendCancelled()) return; + if (signal?.aborted || isSendCancelled()) + return restoreComposerAfterFailure(); const finalTagsWithAgentAddress = [ ...finalOutgoingTags, ...buildAgentAddressMentionTags( @@ -858,111 +863,35 @@ export function useMentionSendFlow({ mentions.getMentionDisplayName(pubkey) ?? truncatePubkey(pubkey), ); }, [mentions.getMentionDisplayName, pendingNonMemberSend]); + const invitation = useNonMemberInvite({ + channelId, + draft: pendingNonMemberSend, + canInvite: canInviteNonMembers, + revalidate: mentions.revalidateMentionPubkeys, + getManagedAgentsByPubkey, + isAgentPubkey: mentions.isAgentPubkey, + addMembers: addMembersMutation.mutateAsync, + completeSend, + setError: setNonMemberPromptError, + }); const handleSendWithoutInviting = React.useCallback(() => { if (!pendingNonMemberSend) return; + invitation.cancel(); const { mentionPubkeys, outgoingTags } = withoutInvitingRecipients(pendingNonMemberSend); void completeSend(pendingNonMemberSend, mentionPubkeys, outgoingTags); - }, [completeSend, pendingNonMemberSend]); - const handleInviteNonMembers = React.useCallback(() => { - if (!pendingNonMemberSend) return; - if (!canInviteNonMembers) { - setNonMemberPromptError(PRIVATE_CHANNEL_ADD_DENIED_MESSAGE); - return; - } - setNonMemberPromptError(null); - void (async () => { - const mentionPubkeys = uniqueNormalizedPubkeys( - await mentions.revalidateMentionPubkeys( - [ - ...pendingNonMemberSend.mentionPubkeys, - ...pendingNonMemberSend.nonMemberPubkeys, - ], - pendingNonMemberSend.capturedChannelId, - mentionRevalidationOptions(pendingNonMemberSend, "prepare"), - ), - ); - const admittedMentionPubkeys = new Set(mentionPubkeys); - const originalNonMemberPubkeys = new Set( - pendingNonMemberSend.nonMemberPubkeys.map(normalizePubkey), - ); - const nonMemberPubkeys = [...originalNonMemberPubkeys].filter( - admittedMentionPubkeys.has.bind(admittedMentionPubkeys), - ); - const outgoingTags = (pendingNonMemberSend.outgoingTags ?? []).filter( - (tag) => - tag[0] !== MENTION_REFERENCE_TAG || - !originalNonMemberPubkeys.has(normalizePubkey(tag[1] ?? "")), - ); - const managedAgentsByPubkey = await getManagedAgentsByPubkey().catch( - () => new Map(), - ); - if (!isMountedRef.current) return; - const peoplePubkeys: string[] = []; - const relayAgentPubkeys: string[] = []; - for (const pubkey of nonMemberPubkeys) { - if (managedAgentsByPubkey.has(pubkey)) { - continue; - } - if (mentions.isAgentPubkey(pubkey)) { - relayAgentPubkeys.push(pubkey); - } else { - peoplePubkeys.push(pubkey); - } - } - const errors: string[] = []; - if (peoplePubkeys.length > 0) { - const result = await addMembersMutation.mutateAsync({ - channelId: pendingNonMemberSend.capturedChannelId ?? undefined, - pubkeys: peoplePubkeys, - role: "member", - }); - errors.push(...result.errors.map((error) => error.error)); - } - if (relayAgentPubkeys.length > 0) { - const result = await addMembersMutation.mutateAsync({ - channelId: pendingNonMemberSend.capturedChannelId ?? undefined, - pubkeys: relayAgentPubkeys, - role: "bot", - }); - errors.push(...result.errors.map((error) => error.error)); - } - if (errors.length > 0) { - setNonMemberPromptError(errors.join("; ")); - return; - } - await completeSend( - { - ...pendingNonMemberSend, - mentionPubkeys, - outgoingTags, - }, - mentionPubkeys, - outgoingTags, - ); - })().catch((error) => { - setNonMemberPromptError( - error instanceof Error ? error.message : "Could not invite members.", - ); - }); - }, [ - addMembersMutation, - canInviteNonMembers, - completeSend, - getManagedAgentsByPubkey, - mentions.isAgentPubkey, - mentions.revalidateMentionPubkeys, - pendingNonMemberSend, - ]); + }, [completeSend, pendingNonMemberSend, invitation.cancel]); const dismissNonMemberPrompt = React.useCallback(() => { + invitation.cancel(); setPendingNonMemberSend(null); setNonMemberPromptError(null); - }, []); + }, [invitation.cancel]); return { // Agent starts are detached (publish-first), so useDetachedAgentStart's // in-flight state deliberately does not gate the composer — a background // start must not block the next send. isPreparingMentionSend: + invitation.isPending || isMentionSendPending || isCompleteSendPending || attachAgentMutation.isPending || @@ -971,6 +900,7 @@ export function useMentionSendFlow({ canInvite: canInviteNonMembers, error: nonMemberPromptError, isInvitePending: + invitation.isPending || isMentionSendPending || isCompleteSendPending || addMembersMutation.isPending || @@ -979,7 +909,7 @@ export function useMentionSendFlow({ names: pendingNonMemberNames, onDismiss: dismissNonMemberPrompt, onDoNothing: handleSendWithoutInviting, - onInvite: handleInviteNonMembers, + onInvite: invitation.invite, open: pendingNonMemberSend !== null, }, sendMessageWithMentionFlow, diff --git a/desktop/src/features/messages/ui/useNonMemberInvite.ts b/desktop/src/features/messages/ui/useNonMemberInvite.ts new file mode 100644 index 00000000000..0e1ead28540 --- /dev/null +++ b/desktop/src/features/messages/ui/useNonMemberInvite.ts @@ -0,0 +1,173 @@ +import * as React from "react"; +import { PRIVATE_CHANNEL_ADD_DENIED_MESSAGE } from "@/features/channels/lib/channelMemberAdmission"; +import type { MentionRevalidationOptions } from "@/features/messages/lib/agentMentionRevalidation"; +import type { ManagedAgent } from "@/shared/api/types"; +import { normalizePubkey } from "@/shared/lib/pubkey"; +import { + MENTION_REFERENCE_TAG, + mentionRevalidationOptions, + uniqueNormalizedPubkeys, + type PendingNonMemberMentionSend, +} from "./useMentionSendFlow.helpers"; + +/** Own the whole Invite attempt, not just the membership mutation's pending state. */ +export function useNonMemberInvite({ + channelId, + draft, + canInvite, + revalidate, + getManagedAgentsByPubkey, + isAgentPubkey, + addMembers, + completeSend, + setError, +}: { + channelId: string | null; + draft: PendingNonMemberMentionSend | null; + canInvite: boolean; + revalidate: ( + pubkeys: readonly string[], + channelId?: string | null, + options?: MentionRevalidationOptions, + ) => Promise; + getManagedAgentsByPubkey: () => Promise>; + isAgentPubkey: (pubkey: string) => boolean; + addMembers: (input: { + channelId?: string; + pubkeys: string[]; + role: "member" | "bot"; + }) => Promise<{ errors: { error: string }[] }>; + completeSend: ( + draft: PendingNonMemberMentionSend, + pubkeys: string[], + tags?: string[][], + ) => Promise; + setError: (error: string | null) => void; +}) { + const active = React.useRef<{ + controller: AbortController; + draft: PendingNonMemberMentionSend; + } | null>(null); + const currentChannel = React.useRef(channelId); + currentChannel.current = channelId; + const [isPending, setIsPending] = React.useState(false); + const cancel = React.useCallback(() => { + active.current?.controller.abort(); + active.current = null; + setIsPending(false); + }, []); + React.useLayoutEffect(() => { + const attempt = active.current; + // Clearing the prompt in completeSend is promotion, not cancellation. + // A different non-null prompt or destination supersedes the old intent. + if ( + attempt && + (attempt.draft.capturedChannelId !== channelId || + (draft !== null && draft !== attempt.draft)) + ) + cancel(); + }, [channelId, draft, cancel]); + React.useLayoutEffect( + () => () => { + active.current?.controller.abort(); + active.current = null; + }, + [], + ); + + const invite = React.useCallback(() => { + if (!draft || active.current) return; + if (!canInvite) { + setError(PRIVATE_CHANNEL_ADD_DENIED_MESSAGE); + return; + } + const attempt = new AbortController(); + active.current = { controller: attempt, draft }; // synchronous double-click guard + setIsPending(true); + setError(null); + const isCurrent = () => + active.current?.controller === attempt && + !attempt.signal.aborted && + currentChannel.current === draft.capturedChannelId; + void (async () => { + const mentionPubkeys = uniqueNormalizedPubkeys( + await revalidate( + [...draft.mentionPubkeys, ...draft.nonMemberPubkeys], + draft.capturedChannelId, + mentionRevalidationOptions(draft, "prepare"), + ), + ); + if (!isCurrent()) return; + const admitted = new Set(mentionPubkeys); + const originalNonMembers = new Set( + draft.nonMemberPubkeys.map(normalizePubkey), + ); + const nonMembers = [...originalNonMembers].filter((key) => + admitted.has(key), + ); + const outgoingTags = (draft.outgoingTags ?? []).filter( + (tag) => + tag[0] !== MENTION_REFERENCE_TAG || + !originalNonMembers.has(normalizePubkey(tag[1] ?? "")), + ); + const managed = await getManagedAgentsByPubkey().catch( + () => new Map(), + ); + if (!isCurrent()) return; + const errors: string[] = []; + for (const role of ["member", "bot"] as const) { + const pubkeys = nonMembers.filter( + (key) => !managed.has(key) && isAgentPubkey(key) === (role === "bot"), + ); + if (pubkeys.length === 0) continue; + const result = await addMembers({ + channelId: draft.capturedChannelId ?? undefined, + pubkeys, + role, + }); + // An accepted add cannot be undone, but it never revives cancelled intent. + if (!isCurrent()) return; + errors.push(...result.errors.map((error) => error.error)); + } + if (errors.length > 0) { + setError(errors.join("; ")); + return; + } + if (!isCurrent()) return; + await completeSend( + { + ...draft, + mentionPubkeys, + outgoingTags, + invitationSignal: attempt.signal, + }, + mentionPubkeys, + outgoingTags, + ); + })() + .catch((error) => { + if (isCurrent()) + setError( + error instanceof Error + ? error.message + : "Could not invite members.", + ); + }) + .finally(() => { + if (active.current?.controller === attempt) { + active.current = null; + setIsPending(false); + } + }); + }, [ + draft, + canInvite, + revalidate, + getManagedAgentsByPubkey, + isAgentPubkey, + addMembers, + completeSend, + setError, + ]); + return { invite, cancel, isPending }; +} diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index 3daac3844d3..3ee44be0ee2 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -252,3 +252,116 @@ test("membership revoked at final publish keeps draft and emits no message", asy ); expect(await sent(page)).toEqual([]); }); + +// Deferred IPC seam: hold the exact next preparation/add response, not a timer. +// This lets the browser exercise Escape/navigation before the continuation runs. +type InviteGateWindow = Window & { + __TAURI_INTERNALS__: { + invoke: (command: string, payload?: unknown) => Promise; + }; + 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 releaseInviteCommand(page: Page) { + await page.evaluate(() => { + (window as unknown as InviteGateWindow).releaseInviteGate?.(); + }); +} +async function waitForInviteGate(page: Page) { + await expect + .poll(() => + page.evaluate( + () => (window as unknown as InviteGateWindow).inviteGateEntered, + ), + ) + .toBe(true); +} +async function remoteAdds(page: Page) { + return page.evaluate(() => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + (call) => call.command === "add_channel_members", + ), + ); +} +test("B1 delayed preparation shows pending; Escape retains draft and cancels add/send", async ({ + page, +}) => { + await install(page); + await select(page); + await page.getByTestId("send-message").click(); + await holdInviteCommand(page, "revalidate_relay_agents"); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await waitForInviteGate(page); + await expect( + page.getByRole("button", { name: "Inviting...", exact: true }), + ).toBeDisabled(); + await expect( + page.getByRole("button", { name: "Do nothing", exact: true }), + ).toBeDisabled(); + await waitForAnimations(page); + await page + .getByRole("alertdialog") + .screenshot({ path: "test-results/b1-invite-pending.png" }); + await page.keyboard.press("Escape"); + await expect(page.getByRole("alertdialog")).toHaveCount(0); + await releaseInviteCommand(page); + await expect + .poll(() => + page.evaluate( + () => + (window.__BUZZ_E2E_COMMANDS__ ?? []).filter( + (command) => command === "revalidate_relay_agents", + ).length, + ), + ) + .toBeGreaterThan(0); + await page.waitForTimeout(300); + expect(await remoteAdds(page)).toEqual([]); + expect(await sent(page)).toEqual([]); + await expect(page.getByTestId("message-input")).toHaveText( + "@RemoteScout hello", + ); + // A retry is a new intent and still succeeds exactly once. + await page.getByTestId("send-message").click(); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await expect.poll(() => sent(page)).toEqual([[REMOTE]]); +}); +test("B1 navigation during delayed add cannot publish its captured draft", async ({ + page, +}) => { + await install(page); + await select(page); + await page.getByTestId("send-message").click(); + await holdInviteCommand(page, "add_channel_members"); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await waitForInviteGate(page); + // Hash routing unmounts the composer without reloading the IPC context. + await page.evaluate(() => { + window.location.hash = "/channels/9dae0116-799b-5071-a0a8-fdd30a91a35d"; + }); + await expect(page.getByTestId("chat-title")).toHaveText("random"); + await releaseInviteCommand(page); + await expect.poll(async () => (await remoteAdds(page)).length).toBe(1); + await page.waitForTimeout(300); + expect(await sent(page)).toEqual([]); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("message-input")).toHaveText( + "@RemoteScout hello", + ); +}); diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md index 25696ce420d..0b2effc35f0 100644 --- a/docs/remote-mention-routing.md +++ b/docs/remote-mention-routing.md @@ -29,3 +29,21 @@ NIP-OA establishes ownership, not physical hosting, availability, or lifecycle control. Final native queries do not provide an atomic relay transaction with message publication. Independent review of both native and publication boundaries is required before landing. + +## Invitation intent and cancellation + +Each chat Invite owns one synchronous pending latch and abort signal, covering +preparation, inventory, adds and the eventual publication continuation. Escape, +navigation/unmount, reference-only selection or a replacement prompt invalidates +that attempt. Clearing the prompt when promoting it to send is not cancellation; +the signal remains live through final validation and queued media completion. +Check cancellation after asynchronous preparation and before subsequent mutations +(including nested local readiness), and again before publication. A completed add +cannot be undone, but its late response cannot revive cancelled message intent. +The pending state includes preparation, disabling both Invite and reference-only +buttons. Cancellation after optimistic clearing restores the captured draft and +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. From 87c8cc80d66b1f0c29121d72893a06e6633175c9 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 15:03:26 -0400 Subject: [PATCH 05/13] 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 --- .../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 24a42795db6..60610b82d50 100644 --- a/desktop/src/features/messages/ui/MessageComposer.tsx +++ b/desktop/src/features/messages/ui/MessageComposer.tsx @@ -191,33 +191,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 857f4bf0e05..18b7327bdbe 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 1909b2541c2..17b578cd05e 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 cc6bb0a6224..e53a7221c63 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -57,7 +57,15 @@ export function dedupeQueuedAgentWakes( }); } +/** 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 f6cd2573690..a966f749244 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -49,6 +49,9 @@ import type { UseMentionSendFlowOptions } from "./useMentionSendFlow.types"; export function useMentionSendFlow({ channelId, + effectiveDraftKey, + getComposerRevision, + runComposerUpdate, channelLinks, channelType, contentRef, @@ -76,15 +79,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 () => { @@ -147,14 +160,17 @@ export function useMentionSendFlow({ memberPubkeys: mentions.memberPubkeys, }); 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[], agentsToWake: [] as QueuedAgentWake[], + mentionRefs: [] as PendingNonMemberMentionSend["savedMentionRefs"], }; } const runtimes = await getAvailableRuntimes(); @@ -166,6 +182,7 @@ export function useMentionSendFlow({ // after the publish succeeds, so a persona created for a send the // non-member prompt later cancels never wakes at all. const agentsToWake: QueuedAgentWake[] = []; + const mentionRefs: PendingNonMemberMentionSend["savedMentionRefs"] = []; const seenPersonaIds = new Set(); const shouldProvisionForDm = channelType === "dm" && Boolean(onPrepareSendChannel); @@ -205,9 +222,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( @@ -222,14 +237,13 @@ export function useMentionSendFlow({ errors, pubkeys: uniqueNormalizedPubkeys(pubkeys), agentsToWake, + mentionRefs, }; }, [ createPersonaAgentMutation, channelType, getAvailableRuntimes, - mentions.extractMentionPersonas, - mentions.registerMentionPubkey, onPrepareSendChannel, provisionPersonaAgentMutation, ], @@ -259,30 +273,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) @@ -305,7 +321,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 && @@ -315,9 +331,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, @@ -327,17 +345,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 @@ -346,9 +367,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, @@ -357,21 +379,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) ?? @@ -379,7 +400,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( @@ -557,10 +592,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([ @@ -571,7 +603,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, @@ -642,14 +680,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, @@ -691,6 +732,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; @@ -734,7 +781,7 @@ export function useMentionSendFlow({ } } const personaMentionResult = await createMentionedPersonaAgents( - trimmed, + selectedPersonas, effectiveChannelId ?? "", ); if (isSendCancelled()) return; @@ -753,8 +800,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( @@ -787,8 +845,9 @@ export function useMentionSendFlow({ ); } catch {} } - const savedMentionRefs = mentions.getDraftMentionRefs(trimmed); const pendingDraft: PendingNonMemberMentionSend = { + sourceOwner, + composerRevision, addressedAgentPubkeys: uniqueNormalizedPubkeys(addressedAgentPubkeys), inlineAgentMentionPubkeys: uniqueNormalizedPubkeys( savedMentionRefs @@ -817,6 +876,7 @@ export function useMentionSendFlow({ savedMentionRefs, }; if (promptNonMemberPubkeys.length > 0) { + if (sourceOwnerRef.current !== sourceOwner) return; setNonMemberPromptError(null); setPendingNonMemberSend(pendingDraft); return; @@ -840,6 +900,8 @@ export function useMentionSendFlow({ }, [ completeSend, + sourceOwner, + getComposerRevision, channelType, createMentionedPersonaAgents, customEmoji, @@ -852,6 +914,7 @@ export function useMentionSendFlow({ mentions.memberPubkeys, mentions.getDraftMentionRefs, mentions.settlePendingMentionBindings, + mentions.registerMentionPubkey, onPrepareSendChannel, activePreparedLinkPreviews, ], @@ -864,7 +927,7 @@ export function useMentionSendFlow({ ); }, [mentions.getMentionDisplayName, pendingNonMemberSend]); const invitation = useNonMemberInvite({ - channelId, + sourceOwner, draft: pendingNonMemberSend, canInvite: canInviteNonMembers, revalidate: mentions.revalidateMentionPubkeys, @@ -903,7 +966,6 @@ export function useMentionSendFlow({ invitation.isPending || isMentionSendPending || isCompleteSendPending || - addMembersMutation.isPending || attachAgentMutation.isPending || createPersonaAgentMutation.isPending, names: pendingNonMemberNames, diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts index fe16ba2f63a..05bc6beb3fb 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 0e1ead28540..933225c97db 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 3ee44be0ee2..3d0e36ca401 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 0b2effc35f0..0a985edd7f8 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. From 759feb4a11da930cfd44b5aa44e9641e934bb768 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 15:32:50 -0400 Subject: [PATCH 06/13] fix(desktop): retain outgoing draft deletion authority after navigation Bind authored revisions to captured lifecycle visits instead of the visible editor. Apply source authority to cancellation and preflight recovery and sent-draft cleanup, while keeping editor ownership separate. Cover reviewer deletion reproduction, queued files, supersession, return visits, and actual thread-switch persistence. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- .../messages/ui/useDraftPersistSnapshot.ts | 22 ++- .../useMentionSendFlow.cancellation.test.mjs | 178 ++++++++++++++++++ .../messages/ui/useMentionSendFlow.helpers.ts | 2 + .../messages/ui/useMentionSendFlow.ts | 46 ++--- .../messages/ui/useMentionSendFlow.types.ts | 1 + .../tests/e2e/remote-owned-mentions.spec.ts | 101 ++++++++++ docs/remote-mention-routing.md | 9 +- 7 files changed, 328 insertions(+), 31 deletions(-) diff --git a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts index 18b7327bdbe..1f04d09c557 100644 --- a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts +++ b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts @@ -66,7 +66,7 @@ type UseDraftPersistLifecycleParams = { }; type UseDraftPersistLifecycleResult = { - /** Monotonic editor revision, including same-text edit/clear cycles. */ + /** Captured visit revision; remains readable after exit, excluding later visits. */ getComposerRevision: () => number; /** Optimistic send/recovery is not an authored edit or authoritative deletion. */ runComposerUpdate: (update: () => void) => void; @@ -139,11 +139,13 @@ export function useDraftPersistLifecycle({ [getImplicitAgentMentionPrefix], ); - const composerRevision = React.useRef(0); - const getComposerRevision = React.useCallback( - () => composerRevision.current, - [], + // Pending sends retain this visit's record after navigation/unmount. A live + // editor ref would lose A's deletion authority as soon as B became visible. + const visit = React.useMemo( + () => ({ channelId, draftKey: effectiveDraftKey, revision: 0 }), + [channelId, effectiveDraftKey], ); + const getComposerRevision = React.useCallback(() => visit.revision, [visit]); const pendingImetaForPersistRef = React.useRef([]); const emptyContentIsAuthoritativeRef = React.useRef(false); const isRestoringContentRef = React.useRef(false); @@ -240,7 +242,7 @@ export function useDraftPersistLifecycle({ const trackAuthoredContent = React.useCallback( (content: string) => { if (isRestoringContentRef.current) return; - composerRevision.current += 1; + visit.revision += 1; if (!effectiveDraftKey) return; const authoritativeDraftKey = scopedDraftKey(effectiveDraftKey); if (content.length > 0) { @@ -259,7 +261,13 @@ export function useDraftPersistLifecycle({ [], ); }, - [channelId, effectiveDraftKey, persistDraft, spoileredAttachmentUrlsRef], + [ + channelId, + effectiveDraftKey, + persistDraft, + spoileredAttachmentUrlsRef, + visit, + ], ); 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 17b578cd05e..38f8c5c074a 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -633,3 +633,181 @@ for (const action of ["unchanged", "navigate", "edit"]) { } }); } + +// Independent reviewer reproduction, plus storage/queued-file boundaries. +for (const media of [false, true]) { + for (const action of ["dismiss", "navigate", "unmount"]) { + test(`authored edit-clear before ${action} stays deleted (queued media: ${media})`, async () => { + const s = await setup({ lifecycle: true }); + const gate = deferred(); + s.control.publish = gate; + if (media) { + s.dismiss(); + s.control.attachments = [{ id: "file", file: {}, spoilered: false }]; + await s.prompt(); + } + await s.invite(); + let upload; + if (media) { + await s.act(async () => { + upload = s.control.uploadCallbacks.onComplete( + [], + new AbortController().signal, + ); + }); + } + assert.equal(s.events("publish").length, 1); + s.edit("new authored text"); + s.edit("", []); + assert.equal(s.store.has("thread:a"), false); + const recoveryWrites = s.events("save-queue").length; + if (action === "navigate") s.navigate("thread:b"); + else if (action === "unmount") s.unmount(); + else s.dismiss(); + assert.equal(s.store.has("thread:a"), false); + if (action === "navigate") { + s.edit(TEXT, [{ ...s.refs[0], pubkey: "c".repeat(64) }]); + s.navigate("thread:a"); + assert.equal(s.options.contentRef.current, ""); + assert.equal(s.control.currentRefs.length, 0); + s.edit("new A after return", []); + s.navigate("thread:b"); + } + await s.finish(gate); + if (upload) await upload; + assert.equal(s.events("SEND").length, 0); + assert.equal(s.events("save-queue").length, recoveryWrites); + if (action === "navigate") { + assert.equal(s.options.contentRef.current, TEXT); + assert.equal(s.control.currentRefs[0].pubkey, "c".repeat(64)); + assert.equal(s.store.get("thread:a").content, "new A after return"); + assert.deepEqual(s.store.get("thread:a").mentionRefs, []); + } else assert.equal(s.store.has("thread:a"), false); + }); + } +} +for (const incoming of [TEXT, "different B text"]) { + for (const edit of ["untouched", "same-text-new-refs", "new-text"]) { + test(`source authority survives navigation: ${edit}, ${incoming}`, async () => { + const s = await setup({ lifecycle: true }); + const otherRefs = [{ ...s.refs[0], pubkey: "c".repeat(64) }]; + s.store.set("thread:b", { + content: incoming, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: otherRefs, + }); + const gate = deferred(); + s.control.publish = gate; + await s.invite(); + if (edit !== "untouched") + s.edit(edit === "new-text" ? "new A text" : TEXT, otherRefs); + s.navigate("thread:b"); + const saved = s.store.get("thread:a"); + assert.equal(saved.content, edit === "new-text" ? "new A text" : TEXT); + assert.deepEqual( + saved.mentionRefs, + edit === "untouched" ? s.refs : otherRefs, + ); + s.edit("B edit during old await", []); + await s.finish(gate); + assert.equal(s.store.get("thread:a"), saved); + assert.equal(s.options.contentRef.current, "B edit during old await"); + assert.equal(s.control.currentRefs.length, 0); + assert.equal(s.events("SEND").length, 0); + }); + } +} + +for (const action of ["untouched", "deleted", "superseded"]) { + test(`ordinary send preflight recovery respects source authority on unmount: ${action}`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.options.mentions.memberPubkeys = new Set([KEY]); + const gate = deferred(); + s.control.prepare = 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.options.contentRef.current, ""); + if (action === "deleted") { + s.edit("new text"); + s.edit("", []); + } + s.unmount(); + const other = { + content: "new saved draft", + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: [], + }; + if (action === "superseded") s.store.set("thread:a", other); + await s.finish(gate); + await send; + assert.equal(s.events("SEND").length, 0); + if (action === "deleted") assert.equal(s.store.has("thread:a"), false); + else if (action === "superseded") + assert.equal(s.store.get("thread:a"), other); + else { + assert.equal(s.store.get("thread:a").content, TEXT); + assert.deepEqual(s.store.get("thread:a").mentionRefs, s.refs); + } + }); +} +for (const authored of [false, true]) { + test(`sent-draft cleanup consults exited source visit, not B revision (authored: ${authored})`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.options.mentions.memberPubkeys = new Set([KEY]); + s.options.drafts.markDraftSent = (...args) => + s.calls.push(["mark-sent", ...args]); + const gate = deferred(); + s.control.publish = gate; + s.rerender(); + let send; + await s.act(async () => { + send = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: TEXT, + recoveryDraftKey: "thread:a", + sentDraftKey: "thread:a", + }); + }); + if (authored) s.edit(TEXT); // same snapshot, but a genuinely newer draft + const sourceRevision = s.options.getComposerRevision; + const revision = sourceRevision(); + s.navigate("thread:b"); + s.edit("B edit", []); + assert.equal( + sourceRevision(), + revision, + "B must not change the captured A revision", + ); + // An untouched optimistic empty has no stored refs. Seed the unchanged + // submitted snapshot to exercise markDraftSent's bounded exact-ref guard. + if (!authored) + s.store.set("thread:a", { + content: TEXT, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: s.refs, + }); + await s.finish(gate); + await send; + assert.equal(s.events("SEND").length, 1); // ordinary destination-bound send + assert.equal(s.events("mark-sent").length, authored ? 0 : 1); + assert.equal(s.options.contentRef.current, "B edit"); + assert.equal(s.control.currentRefs.length, 0); + }); +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index e53a7221c63..77724c62e5d 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -61,6 +61,8 @@ export function dedupeQueuedAgentWakes( export type ComposerDraftOwner = { channelId: string | null; draftKey: string | null | undefined; + /** Read this source visit, never the currently visible editor visit. */ + getComposerRevision: () => number; }; export type PendingNonMemberMentionSend = { diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index a966f749244..6b6ecf7d9f0 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -82,8 +82,8 @@ export function useMentionSendFlow({ // 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], + () => ({ channelId, draftKey: effectiveDraftKey, getComposerRevision }), + [channelId, effectiveDraftKey, getComposerRevision], ); const sourceOwnerRef = React.useRef(sourceOwner); sourceOwnerRef.current = sourceOwner; @@ -303,25 +303,15 @@ export function useMentionSendFlow({ draft.queuedAttachments.length > 0 ? prepareBackgroundMediaUpload(draft.queuedAttachments) : null; - const persistPreflightDraft = () => { - if (isSendCancelled() || !draft.recoveryDraftKey) return; - drafts.persistDraft( - draft.recoveryDraftKey, - draft.savedContent, - draft.capturedChannelId ?? draft.recoveryDraftKey, - draft.savedImeta, - [...draft.savedSpoileredAttachmentUrls], - draft.savedMentionRefs, - ); - saveQueuedAttachmentsForDraft( - draft.recoveryDraftKey, - draft.queuedAttachments, - ); - }; - const persistCanceledDraft = () => { + const persistRecoverableDraft = () => { // Invitation cancellation still owes the captured draft recovery. Link // preview cancellation retains its existing independent recovery owner. - if (sendSignal?.aborted || !draft.recoveryDraftKey) return false; + if ( + sendSignal?.aborted || + !draft.recoveryDraftKey || + draft.sourceOwner.getComposerRevision() !== draft.composerRevision + ) + return false; const existing = drafts.loadDraft(draft.recoveryDraftKey); if ( existing && @@ -347,6 +337,18 @@ export function useMentionSendFlow({ ); return true; }; + const persistPreflightDraft = () => { + if ( + !draft.recoveryDraftKey || + isSendCancelled() || + !persistRecoverableDraft() + ) + return; + saveQueuedAttachmentsForDraft( + draft.recoveryDraftKey, + draft.queuedAttachments, + ); + }; let composerCleared = false; let optimisticComposerContent = ""; let clearedRevision = -1; @@ -355,8 +357,8 @@ export function useMentionSendFlow({ composerCleared = false; // 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(); + if (draft.sourceOwner.getComposerRevision() !== clearedRevision) return; + const persisted = persistRecoverableDraft(); const canAnimateCurrentComposer = isMountedRef.current && ownsComposer(); if ( @@ -605,7 +607,7 @@ export function useMentionSendFlow({ } if ( draft.sentDraftKey && - (!ownsComposer() || getComposerRevision() === clearedRevision) && + draft.sourceOwner.getComposerRevision() === clearedRevision && JSON.stringify( drafts.loadDraft(draft.sentDraftKey)?.mentionRefs ?? [], ) === JSON.stringify(draft.savedMentionRefs) diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts index 05bc6beb3fb..ac82b74eca8 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts @@ -10,6 +10,7 @@ import type { UseRichTextEditorResult } from "@/features/messages/lib/useRichTex import type { UseDraftsResult } from "@/features/messages/lib/useDrafts"; export type UseMentionSendFlowOptions = { + /** Lifecycle accessor bound to this visit, still readable after it exits. */ getComposerRevision: () => number; runComposerUpdate: (update: () => void) => void; channelId: string | null; diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index 3d0e36ca401..c8c75cd725e 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -458,3 +458,104 @@ for (const stage of ["add", "publish"] as const) { }); } } + +for (const incoming of ["unrelated thread B draft", "@RemoteScout hello"]) { + test(`B1 authored deletion before thread switch preserves storage and ${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, "revalidate_relay_agents", 2); + await page.getByRole("button", { name: "Invite", exact: true }).click(); + await waitForInviteGate(page); + await expect(input).toHaveText(""); + await input.fill("new authored text"); + await input.fill(""); + const sourceRecord = () => + page.evaluate(([root, otherRoot]) => { + const key = Object.keys(localStorage).find((key) => + key.startsWith("buzz-drafts.v2"), + ); + if (!key) throw new Error("draft storage scope missing"); + const drafts = JSON.parse(localStorage.getItem(key) ?? "{}"); + if (!drafts[`thread:${otherRoot}`]) + throw new Error("control B draft missing"); + return drafts[`thread:${root}`] ?? null; + }, roots); + expect(await sourceRecord()).toBeNull(); + // 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); + // Read actual persistence, not the editor (whose tombstone could mask a + // resurrected record until reload). Neither text nor exact refs may return. + expect(await sourceRecord()).toBeNull(); + await navigate(roots[0]); + await expect(input).toHaveText(""); + + 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(""); + expect(await sourceRecord()).toBeNull(); + await assertNoLocalLifecycle(page); + }); +} diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md index 0a985edd7f8..5d4ea14ce70 100644 --- a/docs/remote-mention-routing.md +++ b/docs/remote-mention-routing.md @@ -52,8 +52,13 @@ visible pending, Escape/retry and route navigation at deferred IPC boundaries. 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 +thread changes and A → B → A invalidate the original visit. The visible owner gates +optimistic clear, editor recovery and pending state. Storage recovery instead +consults the captured source visit's authored revision even after that visit +exits; losing visible ownership never revokes an authored deletion. The lifecycle +retains a per-visit revision record through its captured accessor, so B's edits +cannot authorize or suppress A's recovery. Preflight recovery and sent-draft +cleanup use that same source authority. 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. From 97a1fa474ba7918341afee170ee98746e69ddf8a Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 15:35:17 -0400 Subject: [PATCH 07/13] fix(desktop): retain cleanup for untouched pre-clear source drafts Use the captured submission revision for sent-draft storage authority, including ordinary destination-bound sends that leave before optimistic clear. Add a discriminating regression for that lifecycle boundary. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- .../useMentionSendFlow.cancellation.test.mjs | 32 +++++++++++++++++++ .../messages/ui/useMentionSendFlow.ts | 3 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs index 38f8c5c074a..c8804fa255a 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -811,3 +811,35 @@ for (const authored of [false, true]) { assert.equal(s.control.currentRefs.length, 0); }); } + +test("ordinary send may clean its untouched source even if navigation preceded optimistic clear", async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.options.mentions.memberPubkeys = new Set([KEY]); + s.options.drafts.markDraftSent = (...args) => + s.calls.push(["mark-sent", ...args]); + const gate = deferred(); + s.control.publish = gate; + s.rerender(); + let send; + s.act(() => { + send = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: TEXT, + recoveryDraftKey: "thread:a", + sentDraftKey: "thread:a", + }); + // Preparation yields before completeSend can clear the composer. + s.navigate("thread:b"); + }); + await s.flush(); + assert.equal(s.events("publish").length, 1); + assert.equal(s.store.get("thread:a").content, TEXT); + s.edit("B edit", []); + await s.finish(gate); + await send; + assert.equal(s.events("SEND").length, 1); + assert.equal(s.events("mark-sent").length, 1); + assert.equal(s.options.contentRef.current, "B edit"); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index 6b6ecf7d9f0..ee7dbe370fa 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -607,7 +607,8 @@ export function useMentionSendFlow({ } if ( draft.sentDraftKey && - draft.sourceOwner.getComposerRevision() === clearedRevision && + draft.sourceOwner.getComposerRevision() === + draft.composerRevision && JSON.stringify( drafts.loadDraft(draft.sentDraftKey)?.mentionRefs ?? [], ) === JSON.stringify(draft.savedMentionRefs) From 11962eff2eb8acb55a893fa3661ba35f07e6fbae Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 16:05:43 -0400 Subject: [PATCH 08/13] fix(desktop): share draft intent authority across composer visits Keep authored intent in the scoped draft store independently of value deletion. Supersede recovery, sent cleanup and lifecycle persistence across reentry and remount while preserving ordinary destination-bound sends and untouched optimistic recovery. Keep editability and automatic prefix restoration programmatic. Exercise the real store, React lifecycle transition matrix and retained browser editor. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- .../messages/lib/draftAuthority.test.mjs | 84 ++++ .../src/features/messages/lib/useDrafts.ts | 67 ++- .../messages/lib/useRichTextEditor.ts | 1 + .../messages/ui/useDraftPersistSnapshot.ts | 58 +-- .../ui/useMentionSendFlow.authority.test.mjs | 218 ++++++++++ .../useMentionSendFlow.cancellation.test.mjs | 367 +---------------- .../messages/ui/useMentionSendFlow.helpers.ts | 2 +- .../ui/useMentionSendFlow.test-support.mjs | 382 ++++++++++++++++++ .../messages/ui/useMentionSendFlow.ts | 26 +- .../messages/ui/useMentionSendFlow.types.ts | 2 +- .../tests/e2e/remote-owned-mentions.spec.ts | 131 ++++++ docs/remote-mention-routing.md | 19 +- 12 files changed, 955 insertions(+), 402 deletions(-) create mode 100644 desktop/src/features/messages/lib/draftAuthority.test.mjs create mode 100644 desktop/src/features/messages/ui/useMentionSendFlow.authority.test.mjs create mode 100644 desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs diff --git a/desktop/src/features/messages/lib/draftAuthority.test.mjs b/desktop/src/features/messages/lib/draftAuthority.test.mjs new file mode 100644 index 00000000000..e05a9d319d5 --- /dev/null +++ b/desktop/src/features/messages/lib/draftAuthority.test.mjs @@ -0,0 +1,84 @@ +import assert from "node:assert/strict"; +import { beforeEach, test } from "node:test"; +import { + claimDraftSend, + clearAllDrafts, + deleteDraftEntry, + getDraftAuthority, + initDraftStore, + loadDraftEntry, + persistDraftEntry, + recordDraftAuthoredContent, + saveDraftEntry, +} from "./useDrafts.ts"; + +beforeEach(() => { + const storage = new Map(); + globalThis.localStorage = { + getItem: (key) => storage.get(key) ?? null, + setItem: (key, value) => storage.set(key, value), + removeItem: (key) => storage.delete(key), + }; + clearAllDrafts(); + initDraftStore("author", "wss://authority.example"); +}); + +test("shared authority remains readable when authored empty removes the record", () => { + const oldVisit = getDraftAuthority("A"); + persistDraftEntry("A", "original", "channel", [], []); + const revision = oldVisit.revision; + persistDraftEntry("A", "", "channel", [], []); // optimistic clear + assert.equal(oldVisit.revision, revision); + assert.equal(oldVisit.emptyContentIsAuthoritative, false); + assert.equal(loadDraftEntry("A"), undefined); + const newVisit = getDraftAuthority("A"); + assert.equal(oldVisit, newVisit); + recordDraftAuthoredContent("A", "new text"); + recordDraftAuthoredContent("A", ""); + assert.notEqual(oldVisit.revision, revision); + assert.equal(oldVisit.emptyContentIsAuthoritative, true); + assert.equal(loadDraftEntry("A"), undefined); + const afterClear = oldVisit.revision; + recordDraftAuthoredContent("B", "other draft"); + assert.equal(oldVisit.revision, afterClear); +}); + +test("explicit deletion supersedes an old attempt even if the value is absent", () => { + const authority = getDraftAuthority("A"); + claimDraftSend("A"); + const revision = authority.revision; + deleteDraftEntry("A"); + assert.equal(loadDraftEntry("A"), undefined); + assert.notEqual(authority.revision, revision); + assert.equal(authority.emptyContentIsAuthoritative, true); +}); + +test("new sends revoke recovery without manufacturing authored emptiness", () => { + const authority = getDraftAuthority("A"); + claimDraftSend("A"); + const first = authority.revision; + claimDraftSend("A"); + assert.notEqual(authority.revision, first); + assert.equal(authority.emptyContentIsAuthoritative, false); +}); + +test("scope reset invalidates retained handles, including a round trip", () => { + const old = getDraftAuthority("A"); + recordDraftAuthoredContent("A", ""); + const revision = old.revision; + initDraftStore("other", "wss://other.example"); + assert.notEqual(old.revision, revision); + assert.equal(getDraftAuthority("A").emptyContentIsAuthoritative, false); + initDraftStore("author", "wss://authority.example"); + assert.notEqual(getDraftAuthority("A"), old); + assert.notEqual(old.revision, revision); +}); + +test("explicit replacement revokes old authority even for an identical snapshot", () => { + persistDraftEntry("A", "same", "channel", [], []); + const value = loadDraftEntry("A"); + const authority = getDraftAuthority("A"); + const revision = authority.revision; + saveDraftEntry("A", value); + assert.notEqual(authority.revision, revision); +}); diff --git a/desktop/src/features/messages/lib/useDrafts.ts b/desktop/src/features/messages/lib/useDrafts.ts index a3e0fcf9197..2a87d692ceb 100644 --- a/desktop/src/features/messages/lib/useDrafts.ts +++ b/desktop/src/features/messages/lib/useDrafts.ts @@ -102,6 +102,59 @@ function canonicalizeRelayScope(relayUrl: string): string { } } +/** Semantic intent survives value deletion and composer visits within this store. */ +type DraftAuthority = { + revision: number; + authoredRevision: number; + emptyContentIsAuthoritative: boolean; +}; +const draftAuthorities = new Map(); + +/** Mutable state stays private to the scoped draft owner. */ +function mutableDraftAuthority(draftKey: string): DraftAuthority { + let authority = draftAuthorities.get(draftKey); + if (!authority) { + authority = { + revision: 0, + authoredRevision: 0, + emptyContentIsAuthoritative: false, + }; + draftAuthorities.set(draftKey, authority); + } + return authority; +} + +/** Capture the shared intent for this key; remains readable across visits/deletion. */ +export function getDraftAuthority(draftKey: string): Readonly { + return mutableDraftAuthority(draftKey); +} + +/** Record deliberate editor intent even when there is no persisted value. */ +export function recordDraftAuthoredContent( + draftKey: string, + content: string, +): void { + const authority = mutableDraftAuthority(draftKey); + authority.revision += 1; + authority.authoredRevision += 1; + authority.emptyContentIsAuthoritative = content.length === 0; +} + +/** A newer send owns recovery/cleanup, independently of publication authority. */ +export function claimDraftSend(draftKey: string | null | undefined): void { + if (!draftKey) return; + mutableDraftAuthority(draftKey).revision += 1; +} + +function resetDraftAuthorities(): void { + // Invalidate handles retained by old continuations before dropping the scope. + for (const authority of draftAuthorities.values()) { + authority.revision += 1; + authority.authoredRevision += 1; + } + draftAuthorities.clear(); +} + /** Module-level workspace identity set by `initDraftStore`. Empty = no workspace. */ let currentPubkey = ""; let currentRelayScope = ""; @@ -132,6 +185,7 @@ export function initDraftStore(pubkey: string, relayUrl = ""): void { const relayScope = canonicalizeRelayScope(relayUrl); if (currentPubkey !== pubkey || currentRelayScope !== relayScope) { _memCache = null; + resetDraftAuthorities(); } currentPubkey = pubkey; currentRelayScope = relayScope; @@ -145,6 +199,7 @@ export function initDraftStore(pubkey: string, relayUrl = ""): void { * Replaces the old `clearAllDrafts()`. */ export function clearAllDrafts(): void { + resetDraftAuthorities(); currentPubkey = ""; currentRelayScope = ""; _memCache = null; @@ -287,6 +342,11 @@ function evictOldest(map: Map): void { // use them without a React context. export function saveDraftEntry(draftKey: string, draft: DraftState): void { + recordDraftAuthoredContent(draftKey, draft.content); + writeDraftEntry(draftKey, draft); +} + +function writeDraftEntry(draftKey: string, draft: DraftState): void { if (draft.content.trim().length === 0 && draft.pendingImeta.length === 0) { return; } @@ -302,6 +362,7 @@ export function loadDraftEntry(draftKey: string): DraftState | undefined { } export function deleteDraftEntry(draftKey: string): void { + recordDraftAuthoredContent(draftKey, ""); discardQueuedAttachmentsForDraft(draftKey); clearDraftEntry(draftKey); } @@ -411,6 +472,8 @@ export function renameDraftEntry( if (!draftStatesEqual(existing, destination)) { return "collision"; } + recordDraftAuthoredContent(oldKey, ""); + recordDraftAuthoredContent(newKey, existing.content); // Identical records: remove the legacy key, keep the destination entry. map.delete(oldKey); flushStore(map); @@ -418,6 +481,8 @@ export function renameDraftEntry( return "migrated"; } + recordDraftAuthoredContent(oldKey, ""); + recordDraftAuthoredContent(newKey, existing.content); // No destination conflict: move the record. Cardinality is unchanged // (one delete + one set), so evictOldest is not called. map.set(newKey, existing); @@ -444,7 +509,7 @@ export function persistDraftEntry( const map = readStore(); const existing = map.get(draftKey); const now = new Date().toISOString(); - saveDraftEntry(draftKey, { + writeDraftEntry(draftKey, { content, selectionEnd: content.length, selectionStart: content.length, diff --git a/desktop/src/features/messages/lib/useRichTextEditor.ts b/desktop/src/features/messages/lib/useRichTextEditor.ts index f3d8903b3ef..837822f01f8 100644 --- a/desktop/src/features/messages/lib/useRichTextEditor.ts +++ b/desktop/src/features/messages/lib/useRichTextEditor.ts @@ -564,6 +564,7 @@ export function useRichTextEditor({ // About to disable: remember whether we currently hold focus so we know // whether to restore it when re-enabled. hadFocusBeforeDisableRef.current = editor.isFocused; + // Editability is not an authored document update (not even a clear). editor.setEditable(false, false); } else { editor.setEditable(true, false); diff --git a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts index 1f04d09c557..c3b795e3d80 100644 --- a/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts +++ b/desktop/src/features/messages/ui/useDraftPersistSnapshot.ts @@ -5,7 +5,8 @@ import { stripImplicitAgentMentionPrefix } from "@/features/messages/lib/stripIm import type { ImetaMedia } from "@/features/messages/lib/imetaMediaMarkdown"; import type { QueuedMediaAttachment } from "@/features/messages/lib/backgroundMediaUploadStore"; import { - getDraftStoreScope, + getDraftAuthority, + recordDraftAuthoredContent, type DraftMentionRef, type DraftState, } from "@/features/messages/lib/useDrafts"; @@ -66,7 +67,7 @@ type UseDraftPersistLifecycleParams = { }; type UseDraftPersistLifecycleResult = { - /** Captured visit revision; remains readable after exit, excluding later visits. */ + /** Shared key intent; later visits can revoke a captured continuation. */ getComposerRevision: () => number; /** Optimistic send/recovery is not an authored edit or authoritative deletion. */ runComposerUpdate: (update: () => void) => void; @@ -78,12 +79,6 @@ type UseDraftPersistLifecycleResult = { trackAuthoredContent: (content: string) => void; }; -const authoritativelyClearedDraftKeys = new Set(); - -function scopedDraftKey(draftKey: string): string { - return `${getDraftStoreScope()}:${draftKey}`; -} - /** * Owns the draft-persist lifecycle for `MessageComposer`. * @@ -139,13 +134,24 @@ export function useDraftPersistLifecycle({ [getImplicitAgentMentionPrefix], ); - // Pending sends retain this visit's record after navigation/unmount. A live - // editor ref would lose A's deletion authority as soon as B became visible. - const visit = React.useMemo( - () => ({ channelId, draftKey: effectiveDraftKey, revision: 0 }), - [channelId, effectiveDraftKey], + // Each visit keeps its own accessor identity (visible ownership), but reads + // the store's shared key authority, including later visits and absent values. + const authority = React.useMemo( + () => + effectiveDraftKey + ? getDraftAuthority(effectiveDraftKey) + : { + revision: 0, + authoredRevision: 0, + emptyContentIsAuthoritative: false, + }, + [effectiveDraftKey], + ); + const getComposerRevision = React.useCallback( + () => authority.revision, + [authority], ); - const getComposerRevision = React.useCallback(() => visit.revision, [visit]); + const lastAuthoredRevisionRef = React.useRef(authority.authoredRevision); const pendingImetaForPersistRef = React.useRef([]); const emptyContentIsAuthoritativeRef = React.useRef(false); const isRestoringContentRef = React.useRef(false); @@ -186,12 +192,8 @@ export function useDraftPersistLifecycle({ : []; } restoreQueuedAttachments?.(restoredQueuedAttachmentsRef.current); - const authoritativeDraftKey = effectiveDraftKey - ? scopedDraftKey(effectiveDraftKey) - : null; - const wasAuthoritativelyCleared = authoritativeDraftKey - ? authoritativelyClearedDraftKeys.has(authoritativeDraftKey) - : false; + lastAuthoredRevisionRef.current = authority.authoredRevision; + const wasAuthoritativelyCleared = authority.emptyContentIsAuthoritative; const saved = effectiveDraftKey ? loadDraft(effectiveDraftKey) : undefined; emptyContentIsAuthoritativeRef.current = wasAuthoritativelyCleared; isRestoringContentRef.current = true; @@ -219,7 +221,13 @@ export function useDraftPersistLifecycle({ isRestoringContentRef.current = false; return () => { - if (effectiveDraftKey) { + // Another composer or explicit inbox deletion may supersede this visit + // without changing its editor. A send claim alone does not block saving + // the outgoing editor: optimistic clear must still persist normally. + if ( + effectiveDraftKey && + lastAuthoredRevisionRef.current === authority.authoredRevision + ) { const queuedAttachments = getQueuedAttachments?.() ?? []; if (queuedAttachments.length > 0) { saveQueuedAttachmentsForDraft?.(effectiveDraftKey, queuedAttachments); @@ -242,15 +250,13 @@ export function useDraftPersistLifecycle({ const trackAuthoredContent = React.useCallback( (content: string) => { if (isRestoringContentRef.current) return; - visit.revision += 1; if (!effectiveDraftKey) return; - const authoritativeDraftKey = scopedDraftKey(effectiveDraftKey); + recordDraftAuthoredContent(effectiveDraftKey, content); + lastAuthoredRevisionRef.current = authority.authoredRevision; if (content.length > 0) { - authoritativelyClearedDraftKeys.delete(authoritativeDraftKey); emptyContentIsAuthoritativeRef.current = false; return; } - authoritativelyClearedDraftKeys.add(authoritativeDraftKey); emptyContentIsAuthoritativeRef.current = true; persistDraft( effectiveDraftKey, @@ -262,11 +268,11 @@ export function useDraftPersistLifecycle({ ); }, [ + authority, channelId, effectiveDraftKey, persistDraft, spoileredAttachmentUrlsRef, - visit, ], ); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.authority.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.authority.test.mjs new file mode 100644 index 00000000000..9d73f7b7b59 --- /dev/null +++ b/desktop/src/features/messages/ui/useMentionSendFlow.authority.test.mjs @@ -0,0 +1,218 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + setup, + deferred, + KEY, + TEXT, +} from "./useMentionSendFlow.test-support.mjs"; + +async function ordinary(s, gate) { + s.options.mentions.memberPubkeys = new Set([KEY]); + s.control.publish = gate; + s.rerender(); + let promise; + await s.act(async () => { + promise = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: TEXT, + recoveryDraftKey: "thread:a", + sentDraftKey: "thread:a", + }); + }); + return { promise }; +} + +// Bound the transition product, not just one reporter sequence. Same snapshot +// authorship must supersede cleanup too; equality of content/refs is not intent. +for (const outcome of ["failure", "success"]) { + for (const intent of [ + "untouched", + "clear", + "text", + "same-text", + "new-refs", + ]) { + for (const exit of ["B", "unmount"]) { + test(`ordinary cross-visit ${outcome}, newer ${intent}, exit ${exit}`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + const gate = deferred(); + const { promise } = await ordinary(s, gate); + assert.equal(s.events("publish").length, 1); + assert.equal(s.options.contentRef.current, ""); + s.navigate("thread:b"); + s.edit("B preserved", []); + s.navigate("thread:a"); + const refs = + intent === "new-refs" + ? [{ ...s.refs[0], pubkey: "c".repeat(64) }] + : s.refs; + if (intent !== "untouched") { + s.edit( + intent === "text" || intent === "clear" ? "new A intent" : TEXT, + refs, + ); + if (intent === "clear") s.edit("", []); + } + s.navigate("thread:b"); + const before = s.store.get("thread:a"); + if (exit === "unmount") s.unmount(); + await s.act(async () => { + if (outcome === "failure") + gate.reject(new Error("final validation failed")); + else gate.resolve(); + await promise; + }); + assert.equal(s.events("SEND").length, outcome === "success" ? 1 : 0); + assert.equal(s.options.contentRef.current, "B preserved"); + assert.equal(s.store.get("thread:b").content, "B preserved"); + assert.deepEqual(s.store.get("thread:b").mentionRefs, []); + if (intent === "untouched" && outcome === "failure") { + assert.equal(s.store.get("thread:a").content, TEXT); + assert.deepEqual(s.store.get("thread:a").mentionRefs, s.refs); + } else { + assert.deepEqual( + s.store.get("thread:a"), + before, + "new authored value or absence wins", + ); + } + }); + } + } +} + +for (const first of ["old", "new"]) { + test(`new same-key send supersedes old recovery, ${first} completes first`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + const oldGate = deferred(); + const old = await ordinary(s, oldGate); + s.navigate("thread:b"); + s.edit("B preserved", []); + s.navigate("thread:a"); + s.remount(); // a new composer has its own send latch, sharing draft authority + // Retry the same captured text without an editor update: claiming a new + // send itself must revoke the older recovery, even for identical refs. + s.options.contentRef.current = TEXT; + s.control.currentRefs = s.refs; + const newGate = deferred(); + const next = await ordinary(s, newGate); + assert.equal(s.events("publish").length, 2); + s.navigate("thread:b"); + const failOld = () => + s.act(async () => { + oldGate.reject(new Error("old validation failed")); + await old.promise; + }); + const finishNew = () => + s.act(async () => { + newGate.resolve(); + await next.promise; + }); + if (first === "old") { + await failOld(); + await finishNew(); + } else { + await finishNew(); + await failOld(); + } + assert.equal(s.events("SEND").length, 1); + assert.equal(s.store.has("thread:a"), false); + assert.equal(s.options.contentRef.current, "B preserved"); + }); +} + +// Media error reaches the same recovery guard as final-validation failure. +test("cross-visit author clear prevents old media error restoring text, refs or files", async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.options.mentions.memberPubkeys = new Set([KEY]); + s.rerender(); + let promise; + await s.act(async () => { + promise = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: TEXT, + recoveryDraftKey: "thread:a", + queuedAttachments: [{ id: "old-file" }], + }); + }); + assert.ok(s.control.uploadCallbacks); + s.navigate("thread:b"); + s.edit("B preserved", []); + s.navigate("thread:a"); + s.edit("new A", []); + s.edit("", []); + s.navigate("thread:b"); + const queues = s.events("save-queue").length; + await s.act(async () => { + s.control.uploadCallbacks.onError(new Error("upload failed")); + await promise; + }); + assert.equal(s.store.has("thread:a"), false); + assert.equal(s.events("save-queue").length, queues); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.options.contentRef.current, "B preserved"); +}); + +for (const replacement of ["delete", "same-text", "new-text"]) { + test(`outgoing lifecycle cleanup cannot overwrite shared ${replacement} from another owner`, async () => { + const { deleteDraftEntry, saveDraftEntry } = await import( + "../lib/useDrafts.ts" + ); + const s = await setup({ lifecycle: true }); + s.dismiss(); + const before = s.store.get("thread:a"); + s.act(() => { + if (replacement === "delete") deleteDraftEntry("thread:a"); + else + saveDraftEntry("thread:a", { + ...before, + content: replacement === "same-text" ? TEXT : "new owner", + }); + }); + const expected = s.store.get("thread:a"); + s.unmount(); + assert.deepEqual(s.store.get("thread:a"), expected); + }); +} + +test("automatic addressed prefix is programmatic optimistic empty, not new authored intent", async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + s.options.mentions.memberPubkeys = new Set([KEY]); + s.options.onAddressedAgentsComposerCleared = () => { + s.options.richText.setContent("@RemoteScout "); + return "@RemoteScout "; + }; + const gate = deferred(); + s.control.publish = gate; + s.rerender(); + const before = s.options.getComposerRevision(); + let promise; + await s.act(async () => { + promise = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + pendingImeta: [], + trimmed: TEXT, + recoveryDraftKey: "thread:a", + addressedAgentPubkeys: [KEY], + }); + }); + assert.equal( + s.options.getComposerRevision(), + before + 1, + "only send claim advances intent", + ); + await s.act(async () => { + gate.reject(new Error("send failed")); + await promise; + }); + assert.equal(s.options.contentRef.current, TEXT); + assert.equal(s.store.get("thread:a").content, TEXT); + assert.deepEqual(s.store.get("thread:a").mentionRefs, s.refs); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs index c8804fa255a..d97ab1dda67 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -1,358 +1,11 @@ import assert from "node:assert/strict"; -import fs from "node:fs"; -import vm from "node:vm"; -import { after, afterEach, before, test } from "node:test"; -import { JSDOM } from "jsdom"; -import * as React from "react"; -import ts from "typescript"; -import * as helpers from "./useMentionSendFlow.helpers.ts"; - -// Execute the product hooks with real React effects/renders; only external -// query/mutation/media dependencies are mocked. Deferred promises isolate the -// user-intent boundary independently of successful authorization. -const dom = new JSDOM("", { - url: "http://localhost", -}); -before(() => - Object.assign(globalThis, { - document: dom.window.document, - HTMLElement: dom.window.HTMLElement, - IS_REACT_ACT_ENVIRONMENT: true, - window: dom.window, - }), -); -afterEach(async () => (await import("@testing-library/react")).cleanup()); -after(() => dom.window.close()); -const KEY = "b".repeat(64); -const TEXT = "@RemoteScout hello"; -const noop = () => {}; -function deferred() { - let resolve; - let reject; - const promise = new Promise((yes, no) => { - resolve = yes; - reject = no; - }); - return { promise, resolve, reject }; -} -function load(name, stubs) { - const source = fs.readFileSync( - new URL(`./${name}.ts`, import.meta.url), - "utf8", - ); - const exports = {}; - vm.runInNewContext( - ts.transpileModule(source, { - compilerOptions: { - module: ts.ModuleKind.CommonJS, - target: ts.ScriptTarget.ES2022, - }, - }).outputText, - { - exports, - AbortController, - Error, - Map, - Set, - require: (key) => { - assert.ok(key in stubs, `unmocked dependency: ${key}`); - return stubs[key]; - }, - }, - ); - return exports; -} -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 }; - const refs = [{ displayName: "RemoteScout", pubkey: KEY, isAgent: true }]; - const query = { - data: [], - refetch: async () => { - if (control.inventory) await control.inventory.promise; - return { data: [] }; - }, - }; - const mutation = { - isPending: false, - mutateAsync: async (input) => { - calls.push(["add", input]); - if (control.add) await control.add.promise; - return { added: [KEY], errors: [] }; - }, - }; - const stubs = { - react: React, - sonner: { toast: { error: (error) => calls.push(["error", error]) } }, - "@/features/agents/hooks": new Proxy( - {}, - { - 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": { - applyReusableAgentAccessPolicy: async (agent) => { - calls.push(["local-policy"]); - if (control.policy) await control.policy.promise; - return agent; - }, - }, - "@/features/agents/lib/resolvePersonaRuntime": { - resolvePersonaRuntime: () => ({ runtime: "test-runtime" }), - }, - "@/features/channels/hooks": { - useAddChannelMembersMutation: () => mutation, - }, - "@/features/channels/useCanAddChannelMembers": { - useCanAddChannelMembers: () => true, - }, - "@/features/channels/lib/channelMemberAdmission": {}, - "@/features/messages/lib/dmThreadAgentMentionError": { - dmThreadAgentMentionError: () => null, - }, - "@/features/messages/lib/backgroundMediaUploadStore": { - 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: [] }), - }, - "@/shared/api/tauri": { invokeTauri: async () => {} }, - "@/shared/lib/pubkey": { - normalizePubkey: (key) => key.toLowerCase(), - truncatePubkey: (key) => key, - }, - "@/shared/lib/customEmojiTags": { buildCustomEmojiTags: () => [] }, - "./useMentionSendFlow.helpers": helpers, - "@/features/messages/lib/agentAddressMention.mjs": { - buildAgentAddressMentionTags: () => [], - }, - "@/features/messages/lib/agentMentionRevalidation": { - AgentMentionAuthorizationError: class extends Error {}, - }, - }; - stubs["./useNonMemberInvite"] = load("useNonMemberInvite", stubs); - stubs["./useActivePreparedLinkPreviews"] = load( - "useActivePreparedLinkPreviews", - 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: { - memberPubkeys: new Set(), - hasResolvedMembers: true, - extractMentionPersonas: () => [], - extractMentionPubkeys: () => [KEY], - isAgentPubkey: (key) => key === KEY, - isManagedAgentPubkey: () => false, - 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: () => { - 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; - return keys; - }, - }, - contentRef: { current: TEXT }, - channelLinks: { clearChannels: noop }, - emojiAutocomplete: { clearEmojis: noop }, - richText: { - clearContent: () => { - if (lifecycle) lifecycleApi.trackAuthoredContent(""); - }, - setContent: (text) => { - if (lifecycle) lifecycleApi.trackAuthoredContent(text); - }, - }, - drafts: { - loadDraft: (key) => (lifecycle ? store.get(key) : null), - persistDraft, - markDraftSent: noop, - }, - setContent: noop, - setPendingImeta: noop, - setIsEmojiPickerOpen: noop, - clearQueuedAttachments: noop, - restoreQueuedAttachments: noop, - hasUnsavedMedia: () => false, - onSendRef: { current: async (...args) => calls.push(["SEND", ...args]) }, - }; - 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)); - }); - const prompt = async (text = TEXT) => - act(async () => { - options.contentRef.current = text; - await hook.result.current.sendMessageWithMentionFlow({ - capturedChannelId: options.channelId, - pendingImeta: [], - trimmed: text, - recoveryDraftKey: options.effectiveDraftKey, - capturedThreadContext: lifecycle - ? { - parentEventId: options.effectiveDraftKey, - threadHeadId: options.effectiveDraftKey, - } - : null, - queuedAttachments: control.attachments ?? [], - }); - }); - await prompt(); - const invite = async () => - act(async () => hook.result.current.nonMemberPromptProps.onInvite()); - const dismiss = () => - act(() => hook.result.current.nonMemberPromptProps.onDismiss()); - const finish = async (gate) => { - gate.resolve(); - await flush(); - }; - const events = (name) => calls.filter((call) => call[0] === name); - return { - ...hook, - act, - calls, - control, - options, - query, - refs, - prompt, - invite, - dismiss, - 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(); - }, - }; -} +import { test } from "node:test"; +import { + setup, + deferred, + KEY, + TEXT, +} from "./useMentionSendFlow.test-support.mjs"; for (const stage of ["prepare", "inventory", "add"]) { test(`dismiss during delayed ${stage} cannot add further or send, retains draft`, async () => { @@ -561,12 +214,12 @@ test("cancelled media continuation preserves refs and cannot overwrite an unrela s.dismiss(); assert.equal(s.options.contentRef.current, TEXT); assert.deepEqual(s.control.currentRefs, s.refs); - assert.equal(s.store.get("thread:a"), other); + assert.equal(s.store.get("thread:a").content, other.content); 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); + assert.equal(s.store.get("thread:a").content, other.content); }); for (const action of ["unchanged", "navigate", "edit"]) { @@ -756,7 +409,7 @@ for (const action of ["untouched", "deleted", "superseded"]) { assert.equal(s.events("SEND").length, 0); if (action === "deleted") assert.equal(s.store.has("thread:a"), false); else if (action === "superseded") - assert.equal(s.store.get("thread:a"), other); + assert.equal(s.store.get("thread:a").content, other.content); else { assert.equal(s.store.get("thread:a").content, TEXT); assert.deepEqual(s.store.get("thread:a").mentionRefs, s.refs); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index 77724c62e5d..1e4be593fa5 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -61,7 +61,7 @@ export function dedupeQueuedAgentWakes( export type ComposerDraftOwner = { channelId: string | null; draftKey: string | null | undefined; - /** Read this source visit, never the currently visible editor visit. */ + /** Read shared source-key intent, never another visible draft key. */ getComposerRevision: () => number; }; diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs new file mode 100644 index 00000000000..9702463f18e --- /dev/null +++ b/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs @@ -0,0 +1,382 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import vm from "node:vm"; +import { after, afterEach, before } from "node:test"; +import { JSDOM } from "jsdom"; +import * as React from "react"; +import ts from "typescript"; +import * as helpers from "./useMentionSendFlow.helpers.ts"; +import * as draftStore from "../lib/useDrafts.ts"; + +// Execute the product hooks with real React effects/renders; only external +// query/mutation/media dependencies are mocked. Deferred promises isolate the +// user-intent boundary independently of successful authorization. +const dom = new JSDOM("", { + url: "http://localhost", +}); +before(() => + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + localStorage: dom.window.localStorage, + }), +); +afterEach(async () => (await import("@testing-library/react")).cleanup()); +after(() => dom.window.close()); +export const KEY = "b".repeat(64); +export const TEXT = "@RemoteScout hello"; +const noop = () => {}; +export function deferred() { + let resolve; + let reject; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} +function load(name, stubs) { + const source = fs.readFileSync( + new URL(`./${name}.ts`, import.meta.url), + "utf8", + ); + const exports = {}; + vm.runInNewContext( + ts.transpileModule(source, { + compilerOptions: { + module: ts.ModuleKind.CommonJS, + target: ts.ScriptTarget.ES2022, + }, + }).outputText, + { + exports, + AbortController, + Error, + Map, + Set, + require: (key) => { + assert.ok(key in stubs, `unmocked dependency: ${key}`); + return stubs[key]; + }, + }, + ); + return exports; +} +export async function setup({ lifecycle = false } = {}) { + const { act, renderHook } = await import("@testing-library/react"); + draftStore.clearAllDrafts(); + dom.window.localStorage.clear(); + draftStore.initDraftStore("test-author", "wss://test.example"); + const calls = []; + const control = { prepare: null, add: null, publish: null, inventory: null }; + const refs = [{ displayName: "RemoteScout", pubkey: KEY, isAgent: true }]; + const query = { + data: [], + refetch: async () => { + if (control.inventory) await control.inventory.promise; + return { data: [] }; + }, + }; + const mutation = { + isPending: false, + mutateAsync: async (input) => { + calls.push(["add", input]); + if (control.add) await control.add.promise; + return { added: [KEY], errors: [] }; + }, + }; + const stubs = { + react: React, + "@/features/messages/lib/useDrafts": draftStore, + sonner: { toast: { error: (error) => calls.push(["error", error]) } }, + "@/features/agents/hooks": new Proxy( + {}, + { + 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": { + applyReusableAgentAccessPolicy: async (agent) => { + calls.push(["local-policy"]); + if (control.policy) await control.policy.promise; + return agent; + }, + }, + "@/features/agents/lib/resolvePersonaRuntime": { + resolvePersonaRuntime: () => ({ runtime: "test-runtime" }), + }, + "@/features/channels/hooks": { + useAddChannelMembersMutation: () => mutation, + }, + "@/features/channels/useCanAddChannelMembers": { + useCanAddChannelMembers: () => true, + }, + "@/features/channels/lib/channelMemberAdmission": {}, + "@/features/messages/lib/dmThreadAgentMentionError": { + dmThreadAgentMentionError: () => null, + }, + "@/features/messages/lib/backgroundMediaUploadStore": { + 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: [] }), + }, + "@/shared/api/tauri": { invokeTauri: async () => {} }, + "@/shared/lib/pubkey": { + normalizePubkey: (key) => key.toLowerCase(), + truncatePubkey: (key) => key, + }, + "@/shared/lib/customEmojiTags": { buildCustomEmojiTags: () => [] }, + "./useMentionSendFlow.helpers": helpers, + "@/features/messages/lib/agentAddressMention.mjs": { + buildAgentAddressMentionTags: () => [], + }, + "@/features/messages/lib/agentMentionRevalidation": { + AgentMentionAuthorizationError: class extends Error {}, + }, + }; + stubs["./useNonMemberInvite"] = load("useNonMemberInvite", stubs); + stubs["./useActivePreparedLinkPreviews"] = load( + "useActivePreparedLinkPreviews", + stubs, + ); + const { useMentionSendFlow } = load("useMentionSendFlow", stubs); + // Real draft adapter: empty persistence deletes the actual value, while + // shared semantic authority remains independently readable. + const store = { + get: draftStore.loadDraftEntry, + has: (key) => draftStore.loadDraftEntry(key) !== undefined, + set: (key, value) => + draftStore.persistDraftEntry( + key, + value.content, + value.channelId, + value.pendingImeta, + value.spoileredAttachmentUrls, + value.mentionRefs, + ), + delete: draftStore.clearDraftEntry, + }; + 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: { + memberPubkeys: new Set(), + hasResolvedMembers: true, + extractMentionPersonas: () => [], + extractMentionPubkeys: () => [KEY], + isAgentPubkey: (key) => key === KEY, + isManagedAgentPubkey: () => false, + 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: () => { + 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; + return keys; + }, + }, + contentRef: { current: TEXT }, + channelLinks: { clearChannels: noop }, + emojiAutocomplete: { clearEmojis: noop }, + richText: { + clearContent: () => { + if (lifecycle) lifecycleApi.trackAuthoredContent(""); + }, + setContent: (text) => { + if (lifecycle) lifecycleApi.trackAuthoredContent(text); + }, + }, + drafts: { + loadDraft: (key) => (lifecycle ? store.get(key) : null), + persistDraft, + markDraftSent: draftStore.markDraftSentEntry, + }, + setContent: noop, + setPendingImeta: noop, + setIsEmojiPickerOpen: noop, + clearQueuedAttachments: noop, + restoreQueuedAttachments: noop, + hasUnsavedMedia: () => false, + onSendRef: { current: async (...args) => calls.push(["SEND", ...args]) }, + }; + stubs["@/features/messages/lib/stripImplicitAgentMentions"] = { + stripImplicitAgentMentionPrefix: (text) => text, + }; + const { useDraftPersistLifecycle } = load("useDraftPersistSnapshot", stubs); + let lifecycleApi; + const renderComposer = () => { + 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); + }; + const mount = () => + renderHook(renderComposer, { + wrapper: ({ children }) => + React.createElement(React.StrictMode, null, children), + }); + let hook = mount(); + const flush = async () => + act(async () => { + await new Promise((resolve) => setImmediate(resolve)); + }); + const prompt = async (text = TEXT) => + act(async () => { + options.contentRef.current = text; + await hook.result.current.sendMessageWithMentionFlow({ + capturedChannelId: options.channelId, + pendingImeta: [], + trimmed: text, + recoveryDraftKey: options.effectiveDraftKey, + capturedThreadContext: lifecycle + ? { + parentEventId: options.effectiveDraftKey, + threadHeadId: options.effectiveDraftKey, + } + : null, + queuedAttachments: control.attachments ?? [], + }); + }); + await prompt(); + const invite = async () => + act(async () => hook.result.current.nonMemberPromptProps.onInvite()); + const dismiss = () => + act(() => hook.result.current.nonMemberPromptProps.onDismiss()); + const finish = async (gate) => { + gate.resolve(); + await flush(); + }; + const events = (name) => calls.filter((call) => call[0] === name); + return { + ...hook, + get result() { + return hook.result; + }, + rerender: () => hook.rerender(), + unmount: () => hook.unmount(), + remount: () => { + hook.unmount(); + hook = mount(); + }, + act, + calls, + control, + options, + query, + refs, + prompt, + invite, + dismiss, + 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(); + }, + }; +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index ee7dbe370fa..4ea3027ffdc 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -1,4 +1,5 @@ import * as React from "react"; +import { claimDraftSend } from "@/features/messages/lib/useDrafts"; import { toast } from "sonner"; import { type CreateChannelManagedAgentInput, @@ -356,7 +357,7 @@ export function useMentionSendFlow({ if (!composerCleared) return; composerCleared = false; // An authored edit (even edit -> clear) ends optimistic recovery's - // authority over this visit, including its persisted record and files. + // authority over this key, including its persisted record and files. if (draft.sourceOwner.getComposerRevision() !== clearedRevision) return; const persisted = persistRecoverableDraft(); const canAnimateCurrentComposer = @@ -394,13 +395,15 @@ export function useMentionSendFlow({ }); }; if (ownsComposer() && getComposerRevision() === draft.composerRevision) { - runComposerUpdate(clearComposer); - if (draft.addressedAgentPubkeys.length > 0) { - optimisticComposerContent = - onAddressedAgentsComposerCleared?.(draft.addressedAgentPubkeys) ?? - ""; - contentRef.current = optimisticComposerContent; - } + runComposerUpdate(() => { + clearComposer(); + if (draft.addressedAgentPubkeys.length > 0) { + optimisticComposerContent = + onAddressedAgentsComposerCleared?.(draft.addressedAgentPubkeys) ?? + ""; + contentRef.current = optimisticComposerContent; + } + }); composerCleared = true; clearedRevision = getComposerRevision(); } @@ -594,7 +597,10 @@ export function useMentionSendFlow({ const newlyPinnedPubkeys = draft.inlineAgentMentionPubkeys.filter( (pubkey) => sentMentionPubkeys.has(normalizePubkey(pubkey)), ); - if (ownsComposer()) { + if ( + ownsComposer() && + getComposerRevision() === draft.composerRevision + ) { onAddressedAgentsSendSucceeded?.( [ ...new Set([ @@ -737,6 +743,7 @@ export function useMentionSendFlow({ setIsMentionSendPending(true); // Capture exact selections before any async preparation can navigate the // reused editor to another draft (possibly with identical display text). + claimDraftSend(effectiveDraftKey); const composerRevision = getComposerRevision(); const savedMentionRefs = mentions.getDraftMentionRefs(trimmed).slice(); const selectedMentionPubkeys = mentions.extractMentionPubkeys(trimmed); @@ -903,6 +910,7 @@ export function useMentionSendFlow({ }, [ completeSend, + effectiveDraftKey, sourceOwner, getComposerRevision, channelType, diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts index ac82b74eca8..3bd9cbcfa93 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.types.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.types.ts @@ -10,7 +10,7 @@ import type { UseRichTextEditorResult } from "@/features/messages/lib/useRichTex import type { UseDraftsResult } from "@/features/messages/lib/useDrafts"; export type UseMentionSendFlowOptions = { - /** Lifecycle accessor bound to this visit, still readable after it exits. */ + /** Visit-bound accessor reading shared source-key intent, even after exit. */ getComposerRevision: () => number; runComposerUpdate: (update: () => void) => void; channelId: string | null; diff --git a/desktop/tests/e2e/remote-owned-mentions.spec.ts b/desktop/tests/e2e/remote-owned-mentions.spec.ts index c8c75cd725e..4df65c7cbf3 100644 --- a/desktop/tests/e2e/remote-owned-mentions.spec.ts +++ b/desktop/tests/e2e/remote-owned-mentions.spec.ts @@ -559,3 +559,134 @@ for (const incoming of ["unrelated thread B draft", "@RemoteScout hello"]) { await assertNoLocalLifecycle(page); }); } + +for (const incoming of ["B preserved"]) { + test(`ordinary failure after returning to A and deleting does not resurrect storage`, async ({ + page, + }) => { + await install(page); + await page.evaluate( + async ({ pubkey, channelId }) => { + await window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.("add_channel_members", { + channelId, + pubkeys: [pubkey], + role: "bot", + }); + await window.__BUZZ_E2E_QUERY_CLIENT__?.invalidateQueries({ + queryKey: ["channels"], + }); + await window.__BUZZ_E2E_QUERY_CLIENT__?.invalidateQueries({ + queryKey: ["relay-agents"], + }); + }, + { pubkey: REMOTE, channelId: GENERAL }, + ); + 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 holdInviteCommand(page, "revalidate_relay_agents", 1); + await page + .getByTestId("message-thread-panel") + .getByTestId("send-message") + .click(); + await waitForInviteGate(page); + await expect(input).toHaveText(""); + 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 navigate(roots[0]); + await expect(input).toHaveAttribute("data-lifecycle-host", "retained"); + await input.fill("new authored text"); + await input.fill(""); + const sourceRecord = () => + page.evaluate(([root, otherRoot]) => { + const key = Object.keys(localStorage).find((key) => + key.startsWith("buzz-drafts.v2"), + ); + if (!key) throw new Error("draft storage scope missing"); + const drafts = JSON.parse(localStorage.getItem(key) ?? "{}"); + if (!drafts[`thread:${otherRoot}`]) + throw new Error("control B draft missing"); + return drafts[`thread:${root}`] ?? null; + }, roots); + expect(await sourceRecord()).toBeNull(); + // 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); + // Read actual persistence, not the editor (whose tombstone could mask a + // resurrected record until reload). Neither text nor exact refs may return. + expect(await sourceRecord()).toBeNull(); + await navigate(roots[0]); + await expect(input).toHaveText(""); + + await navigate(roots[1]); + await page.evaluate((pubkey) => { + window.__BUZZ_E2E__.mock ??= {}; + window.__BUZZ_E2E__.mock.relayAgentRevalidationRevokedPubkeys = [pubkey]; + }, REMOTE); + await releaseInviteCommand(page); + await expect( + page.getByText(/Could not authorize a mentioned agent/), + ).toBeVisible(); + await page.waitForTimeout(300); + expect(await sent(page)).toEqual([]); + await expect(input).toHaveText(incoming); + await navigate(roots[0]); + await expect(input).toHaveText(""); + expect(await sourceRecord()).toBeNull(); + await assertNoLocalLifecycle(page); + }); +} diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md index 5d4ea14ce70..2c80aedb675 100644 --- a/docs/remote-mention-routing.md +++ b/docs/remote-mention-routing.md @@ -54,11 +54,12 @@ 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 visible owner gates optimistic clear, editor recovery and pending state. Storage recovery instead -consults the captured source visit's authored revision even after that visit -exits; losing visible ownership never revokes an authored deletion. The lifecycle -retains a per-visit revision record through its captured accessor, so B's edits -cannot authorize or suppress A's recovery. Preflight recovery and sent-draft -cleanup use that same source authority. Invalidation makes recovery durable +consults shared authored intent for the source draft key, even after exit, +re-entry or unmount. The draft store retains a revision and authoritative-empty +marker independently of the stored value. Later same-key authored text/deletion +or a new send revokes older recovery and sent-draft cleanup; editing B does not +change A's authority. A visit-specific accessor reads that shared authority while +preserving the separate visible-owner identity. 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. @@ -71,8 +72,12 @@ refs, writing them into the editor only while the original visit/revision remain 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, +Recovery additionally compares stored content/media/exact refs before replacing +an existing record. Programmatic persistence does not itself change semantic +intent. Explicit inbox deletion and replacement do; scope reset invalidates old +handles. This is same-window authority for live continuations, not cross-window +synchronization or a versioned storage protocol. Reload destroys continuations; +authored deletion has already removed the durable value. 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. From a784be27cdcd9237bd1af1e5c860c4ed5f646812 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 31 Aug 2026 16:53:00 -0400 Subject: [PATCH 09/13] fix(desktop): keep buffered timeline catch-up reachable A frozen logical tail can report physical bottom before buffered live rows are released. Keep the existing catch-up action visible while pending rows remain so a shared thread message can enter the parent timeline after closing the thread. Preserve scroll anchoring and explicit catch-up behavior. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson --- desktop/src/features/messages/ui/MessageTimeline.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index f33a8fa981c..b293be252af 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -879,7 +879,9 @@ const MessageTimelineBase = React.forwardRef< )} - {!isAtBottom ? ( + {/* A frozen tail can be physically at bottom while live rows are still + buffered. Keep the release action reachable in that state. */} + {!isAtBottom || bufferedTimeline.pendingCount > 0 ? (
Date: Wed, 2 Sep 2026 12:42:04 -0400 Subject: [PATCH 10/13] test(desktop): preserve publication errors across routing integration Signed-off-by: Logan Johnson --- .../useMentionSendFlow.cancellation.test.mjs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs index d97ab1dda67..6d93e4e4ca7 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -496,3 +496,27 @@ test("ordinary send may clean its untouched source even if navigation preceded o assert.equal(s.events("mark-sent").length, 1); assert.equal(s.options.contentRef.current, "B edit"); }); + +for (const upload of [false, true]) { + test(`publication failure remains visible and recoverable after invitation${upload ? " with media" : ""}`, async () => { + const s = await setup(); + s.options.onSendRef.current = async () => { + throw new Error("relay rejected publication"); + }; + if (upload) { + s.control.attachments = [{ id: "queued-file", file: {} }]; + await s.prompt(); + } + await s.invite(); + if (upload) { + await s.act(async () => + s.control.uploadCallbacks.onComplete([], new AbortController().signal), + ); + } + assert.equal(s.options.contentRef.current, TEXT); + assert.deepEqual(s.events("error"), [ + ["error", "Message failed to send: relay rejected publication"], + ]); + assert.equal(s.result.current.isPreparingMentionSend, false); + }); +} From 5872899658a0e71c2c7321ab23fe7db435f997ac Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 4 Sep 2026 10:49:24 -0400 Subject: [PATCH 11/13] test(desktop): exercise extracted readiness in routing regressions Signed-off-by: Logan Johnson --- .../ui/useMentionSendFlow.test-support.mjs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs index 9702463f18e..c49e1f5c5e3 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.test-support.mjs @@ -111,11 +111,22 @@ export async function setup({ lifecycle = false } = {}) { }, }, ), + "@/features/communities/useCommunities": { + useCommunities: () => ({ + activeCommunity: { relayUrl: "wss://test.example" }, + }), + }, + "@/shared/api/hooks": { + useIdentityQuery: () => ({ data: { pubkey: "a".repeat(64) } }), + }, + "@/features/messages/lib/detachedToastScope": { + matchesDetachedToastScope: () => true, + }, "@/features/agents/channelAgents": { applyReusableAgentAccessPolicy: async (agent) => { calls.push(["local-policy"]); if (control.policy) await control.policy.promise; - return agent; + return { agent, wrote: false }; }, }, "@/features/agents/lib/resolvePersonaRuntime": { @@ -159,6 +170,11 @@ export async function setup({ lifecycle = false } = {}) { AgentMentionAuthorizationError: class extends Error {}, }, }; + stubs["./useDetachedAgentStart"] = load("useDetachedAgentStart", stubs); + stubs["./useEnsureAgentMentionsReady"] = load( + "useEnsureAgentMentionsReady", + stubs, + ); stubs["./useNonMemberInvite"] = load("useNonMemberInvite", stubs); stubs["./useActivePreparedLinkPreviews"] = load( "useActivePreparedLinkPreviews", @@ -227,6 +243,7 @@ export async function setup({ lifecycle = false } = {}) { mentions: { memberPubkeys: new Set(), hasResolvedMembers: true, + settlePendingMentionBindings: async () => {}, extractMentionPersonas: () => [], extractMentionPubkeys: () => [KEY], isAgentPubkey: (key) => key === KEY, From 290a58b7ebad2ba6b1ff88a5fd0f9ab9b988685f Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 4 Sep 2026 11:00:00 -0400 Subject: [PATCH 12/13] fix(desktop): settle pasted recipients within the source draft visit Capture recipients after clipboard verification without borrowing a newer authored draft. Preserve ordinary post-capture destination-bound sends. Align inherited revocation tests with fail-closed publication, and pin actual recipients in the detached duplicate-wake test. Signed-off-by: Logan Johnson --- .../useMentionSendFlow.cancellation.test.mjs | 92 ++++++++++++++++++- .../messages/ui/useMentionSendFlow.ts | 19 ++-- desktop/tests/e2e/mentions.spec.ts | 67 ++++++++------ docs/remote-mention-routing.md | 6 +- 4 files changed, 147 insertions(+), 37 deletions(-) diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs index 6d93e4e4ca7..ea6db2517e8 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.cancellation.test.mjs @@ -473,19 +473,23 @@ test("ordinary send may clean its untouched source even if navigation preceded o s.calls.push(["mark-sent", ...args]); const gate = deferred(); s.control.publish = gate; + // Hold destination preparation AFTER clipboard settlement has captured the + // source selections; navigation during settlement now cancels before capture. + const preparation = deferred(); + s.options.onPrepareSendChannel = () => preparation.promise; s.rerender(); let send; - s.act(() => { + await s.act(async () => { send = s.result.current.sendMessageWithMentionFlow({ - capturedChannelId: "general", + capturedChannelId: null, pendingImeta: [], trimmed: TEXT, recoveryDraftKey: "thread:a", sentDraftKey: "thread:a", }); - // Preparation yields before completeSend can clear the composer. - s.navigate("thread:b"); }); + s.navigate("thread:b"); + preparation.resolve("general"); await s.flush(); assert.equal(s.events("publish").length, 1); assert.equal(s.store.get("thread:a").content, TEXT); @@ -520,3 +524,83 @@ for (const upload of [false, true]) { assert.equal(s.result.current.isPreparingMentionSend, false); }); } + +// Clipboard verification is the only pre-capture await. Later preparation +// consumes a snapshot; this await must instead fence the maps before reading. +test("settled paste supplies exact recipient and recovery refs before preparation", async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + const gate = deferred(); + const pastedRefs = [ + { displayName: "RemoteScout", pubkey: "c".repeat(64), isAgent: true }, + ]; + s.options.mentions.settlePendingMentionBindings = async () => { + await gate.promise; + s.control.currentRefs = pastedRefs; + }; + s.options.mentions.extractMentionPubkeys = () => + s.control.currentRefs.map((ref) => ref.pubkey); + s.rerender(); + let sending; + await s.act(async () => { + sending = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + trimmed: TEXT, + pendingImeta: [], + }); + }); + await s.finish(gate); + await sending; + await s.invite(); + assert.deepEqual(Array.from(s.events("SEND")[0][2]), [pastedRefs[0].pubkey]); +}); + +for (const action of ["edit", "delete", "navigation", "return", "unmount"]) { + test(`paste settlement after ${action} cannot read another draft or publish`, async () => { + const s = await setup({ lifecycle: true }); + s.dismiss(); + const gate = deferred(); + s.options.mentions.settlePendingMentionBindings = () => gate.promise; + s.rerender(); + let sending; + await s.act(async () => { + sending = s.result.current.sendMessageWithMentionFlow({ + capturedChannelId: "general", + trimmed: TEXT, + pendingImeta: [], + }); + }); + const otherRefs = [ + { displayName: "RemoteScout", pubkey: "c".repeat(64), isAgent: true }, + ]; + if (action === "edit") s.edit(TEXT, otherRefs); + if (action === "delete") s.edit(""); + if (action === "navigation" || action === "return") { + s.store.set("thread:b", { + content: TEXT, + channelId: "general", + pendingImeta: [], + spoileredAttachmentUrls: [], + mentionRefs: otherRefs, + }); + s.navigate("thread:b"); + if (action === "return") s.navigate("thread:a"); + } + if (action === "unmount") s.unmount(); + let reads = 0; + s.options.mentions.getDraftMentionRefs = () => { + reads++; + return otherRefs; + }; + await s.finish(gate); + await sending; + assert.equal(reads, 0); + assert.equal(s.events("add").length, 0); + assert.equal(s.events("persona").length, 0); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.result.current.nonMemberPromptProps.open, false); + if (action === "delete") assert.equal(s.options.contentRef.current, ""); + if (action === "edit" || action === "navigation") + assert.deepEqual(s.control.currentRefs, otherRefs); + }); +} diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index 4ea3027ffdc..9847193101b 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -741,13 +741,9 @@ 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). + // Bind settlement to this authored visit before reading its recipients. claimDraftSend(effectiveDraftKey); 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; @@ -762,7 +758,18 @@ export function useMentionSendFlow({ // publish a readable `@Label` with no `p` tag. Bounded inside, so a // lookup that never answers delays the send rather than blocking it. await mentions.settlePendingMentionBindings(); - if (isSendCancelled()) return; + // Settlement may outlive an edit or A → B → A navigation. In that + // case the live mention maps no longer belong to this send. + if ( + isSendCancelled() || + !isMountedRef.current || + sourceOwnerRef.current !== sourceOwner || + getComposerRevision() !== composerRevision + ) + return; + const savedMentionRefs = mentions.getDraftMentionRefs(trimmed).slice(); + const selectedMentionPubkeys = mentions.extractMentionPubkeys(trimmed); + const selectedPersonas = mentions.extractMentionPersonas(trimmed); const dmThreadAgentMentionErrorMessage = dmThreadAgentMentionError({ trimmed, isThreadReply: capturedThreadContext != null, diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index d30e2cd27ae..6d9baa1e23b 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -2229,7 +2229,7 @@ test("deferred-upload sends revalidate agent authorization at the publish bounda page, }) => { // A background media upload can hold the publish open for arbitrarily long — - // authorization revoked during that window must still strip the p tag. This + // authorization revoked during that window must block publication. This // pins the publish-boundary revalidation on the deferred path. await installMockBridge(page, { deferredComposerUploads: true, @@ -2314,16 +2314,21 @@ test("deferred-upload sends revalidate agent authorization at the publish bounda }); const outgoingContent = `@quinn hello\n![video](https://mock.relay/media/${"c".repeat(64)}.mp4)`; - await expect - .poll(() => readOutgoingMentionPubkeys(page, outgoingContent)) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, outgoingContent)) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText("Could not authorize a mentioned agent.", { exact: false }), + ).toBeVisible(); + expect(await readOutgoingMentionPubkeys(page, outgoingContent)).toBeNull(); + await expect(input).toHaveText("@quinn hello"); + await expect( + page.getByTestId("composer-queued-media-attachment"), + ).toBeVisible(); const commands = await readCommandLog(page); expect(commandCount(commands, "revalidate_relay_agents")).toBe( commandCount(baselineCommands, "revalidate_relay_agents") + 2, ); + expect(commandCount(commands, "start_managed_agent")).toBe( + commandCount(baselineCommands, "start_managed_agent"), + ); }); test("sends that attach a mentioned agent revalidate at the publish boundary", async ({ @@ -2331,7 +2336,7 @@ test("sends that attach a mentioned agent revalidate at the publish boundary", a }) => { // The awaited membership write for a non-member managed agent is a relay // round-trip between the pre-side-effect authorization pass and the publish - // — authorization revoked during that window must still strip the p tag. + // — authorization revoked during that window must block publication. await installMockBridge(page, { managedAgents: [ { @@ -2410,15 +2415,13 @@ test("sends that attach a mentioned agent revalidate at the publish boundary", a window.__BUZZ_E2E__.mock.relayAgentRevalidationRevokedPubkeys = [pubkey]; }, ALLOWLIST_RELAY_AGENT_PUBKEY); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn @fizz hello")) - .not.toBeNull(); - const outgoingPubkeys = await readOutgoingMentionPubkeys( - page, - "@quinn @fizz hello", - ); - expect(outgoingPubkeys).toContain(OUT_OF_CHANNEL_MANAGED_AGENT_PUBKEY); - expect(outgoingPubkeys).not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText("Could not authorize a mentioned agent.", { exact: false }), + ).toBeVisible(); + expect( + await readOutgoingMentionPubkeys(page, "@quinn @fizz hello"), + ).toBeNull(); + await expect(input).toHaveText("@quinn @fizz hello"); const commands = await readCommandLog(page); expect(commandCount(commands, "revalidate_relay_agents")).toBe( commandCount(baselineCommands, "revalidate_relay_agents") + 2, @@ -2428,6 +2431,9 @@ test("sends that attach a mentioned agent revalidate at the publish boundary", a expect(commandCount(commands, "update_managed_agent")).toBe( commandCount(baselineCommands, "update_managed_agent"), ); + expect(commandCount(commands, "start_managed_agent")).toBe( + commandCount(baselineCommands, "start_managed_agent"), + ); }); test("sends that enroll agents into an active huddle revalidate at the publish boundary", async ({ @@ -2502,7 +2508,7 @@ test("a send held open by a no-write step still revalidates at the publish bound // here the only thing separating the authorization pass from the publish is // the huddle sync — which with no active huddle writes nothing to the relay // — and the revocation is released with zero further hold. A revocation - // landing in any admission-to-publish gap must strip the p tag; this is the + // landing in any admission-to-publish gap must block publication; this is the // reviewer's sub-threshold probe of the since-removed elapsed-time bound, // which deliberately accepted this very staleness. await installMockBridge(page, { @@ -2576,12 +2582,11 @@ test("a send held open by a no-write step still revalidates at the publish bound ) .toBeGreaterThan(0); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toBeNull(); - await expect - .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) - .not.toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); + await expect( + page.getByText("Could not authorize a mentioned agent.", { exact: false }), + ).toBeVisible(); + expect(await readOutgoingMentionPubkeys(page, "@quinn hello")).toBeNull(); + await expect(input).toHaveText("@quinn hello"); const commands = await readCommandLog(page); expect(commandCount(commands, "revalidate_relay_agents")).toBe( @@ -3073,7 +3078,9 @@ test("a second mention while the first wake is in flight does not start the agen await input.fill("Hey @fizz"); await expect(dropdown.getByText("fizz")).toBeVisible(); await input.press("Enter"); - await page.keyboard.type(" do X"); + await expect(input.locator(".mention-chip")).toHaveText("fizz"); + await page.keyboard.type("do X"); + await expect(input).toHaveText("Hey @fizz do X"); await page.getByTestId("send-message").click(); await expect( page.getByTestId("message-row").filter({ hasText: "do X" }), @@ -3087,7 +3094,9 @@ test("a second mention while the first wake is in flight does not start the agen await input.fill("Hey @fizz"); await expect(dropdown.getByText("fizz")).toBeVisible(); await input.press("Enter"); - await page.keyboard.type(" also Y"); + await expect(input.locator(".mention-chip")).toHaveText("fizz"); + await page.keyboard.type("also Y"); + await expect(input).toHaveText("Hey @fizz also Y"); await page.getByTestId("send-message").click(); // The second message publishes on its own — suppression is of the wake, not @@ -3095,6 +3104,12 @@ test("a second mention while the first wake is in flight does not start the agen await expect( page.getByTestId("message-row").filter({ hasText: "also Y" }), ).toBeVisible(); + expect(await readOutgoingMentionPubkeys(page, "Hey @fizz do X")).toContain( + IN_CHANNEL_MANAGED_AGENT_PUBKEY, + ); + expect(await readOutgoingMentionPubkeys(page, "Hey @fizz also Y")).toContain( + IN_CHANNEL_MANAGED_AGENT_PUBKEY, + ); // One wake serves both messages: its replay floor predates the first // message, and the floor is a lower bound, so one harness boot covers both. expect(commandCount(await readCommandLog(page), "start_managed_agent")).toBe( diff --git a/docs/remote-mention-routing.md b/docs/remote-mention-routing.md index 2c80aedb675..6c15817e181 100644 --- a/docs/remote-mention-routing.md +++ b/docs/remote-mention-routing.md @@ -66,7 +66,11 @@ 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. +deleted. Pending clipboard identity verification settles before exact selected +mention refs are captured. The source visit and authored revision are captured +before that wait: an edit, navigation (including A → B → A), or unmount cancels +rather than reading the new draft's maps. Subsequent asynchronous preparation +consumes the captured selections. 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 From 0b303f805e4420eae682087555075c5a7472e40e Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 4 Sep 2026 12:04:01 -0400 Subject: [PATCH 13/13] test(desktop): retain exact edit authority across routing integration Signed-off-by: Logan Johnson --- .../messages/ui/submitMessageEdit.test.mjs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs index cf3c36a36fb..3f0ac92a16a 100644 --- a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs +++ b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs @@ -152,6 +152,47 @@ test("edit upload pause revalidates revoked mentions only after upload completes ]); }); +test("edit revalidates exact selected agents before snapshotting newly typed recipients", async () => { + const selectedAgent = "c".repeat(64); + const typedUser = "d".repeat(64); + const label = `Scout (${selectedAgent})`; + const calls = []; + await submitMessageEdit({ + ...baseOptions(async (_content, tags, notifying) => { + calls.push(["save", tags, notifying]); + }), + originalContent: "hello", + content: `hello @${label} @Alice`, + editTarget: { mentionRefs: [], unresolvedMentionPubkeys: [] }, + getMentionRefs: () => [ + { displayName: label, pubkey: selectedAgent, isAgent: true }, + ], + extractMentionPubkeys: () => [selectedAgent, typedUser], + revalidateMentionPubkeys: async (pubkeys, channelId, options) => { + calls.push(["revalidate", pubkeys, channelId, options]); + return pubkeys; + }, + }); + assert.deepEqual(calls, [ + [ + "revalidate", + [selectedAgent, typedUser], + undefined, + { + intendedAgentPubkeys: [selectedAgent], + }, + ], + [ + "save", + [ + ["mention", selectedAgent], + ["mention", typedUser], + ], + [selectedAgent, typedUser], + ], + ]); +}); + test("ambiguous extractor failure is visible before edit draft clearing or save", async () => { const calls = []; const error =