diff --git a/desktop/src/features/forum/ui/ForumComposer.lifecycle.test.mjs b/desktop/src/features/forum/ui/ForumComposer.lifecycle.test.mjs index c8d9bdd038..91c8b01157 100644 --- a/desktop/src/features/forum/ui/ForumComposer.lifecycle.test.mjs +++ b/desktop/src/features/forum/ui/ForumComposer.lifecycle.test.mjs @@ -317,11 +317,23 @@ async function setup(options = {}) { } if (command === "get_channels") return { hash: "h", channels: [], last_messages: {} }; + // Main readiness syncs a mentioned member agent into any live Huddle. + if (command === "sync_agents_to_active_huddle") + return { changed_channel_ids: [], error: null }; throw new Error(`unmocked ${command}`); }, }; const agentHooks = { - useManagedAgentsQuery: () => query(() => control.agents), + useManagedAgentsQuery: () => ({ + ...query(() => control.agents), + refetch: async () => { + calls.push(["agents-read"]); + // Hold only the final publish pass, not chat's earlier prepare. + if (control.agentRead && control.phase === "publish") + await control.agentRead.promise; + return { data: control.agents, error: null }; + }, + }), useRelayAgentsQuery: () => query(() => []), usePersonasQuery: () => ({ ...query(() => control.personas), @@ -332,7 +344,7 @@ async function setup(options = {}) { }), useTeamsQuery: () => query(() => control.teams), }; - const revalidation = await loadWithImports( + const productionRevalidation = await loadWithImports( "../../messages/lib/agentMentionRevalidation.ts", { "@/shared/api/tauriRelayAgents": { @@ -344,6 +356,22 @@ async function setup(options = {}) { }, }, ); + // Record the running pass so a test can hold only the publish pass. + const revalidation = { + ...productionRevalidation, + useAgentMentionRevalidation: (...args) => { + const revalidate = productionRevalidation.useAgentMentionRevalidation( + ...args, + ); + return React.useCallback( + (pubkeys, channelId, options = {}) => { + control.phase = options.phase; + return revalidate(pubkeys, channelId, options); + }, + [revalidate], + ); + }, + }; const productionMentions = await loadWithImports( "../../messages/lib/useMentions.ts", { @@ -1083,7 +1111,7 @@ for (const main of [false, true]) { .filter(([name]) => name === "error") .map(([, message]) => message), [ - "A mentioned person is no longer in this channel. Invite them or remove the mention, then retry.", + "Someone you mentioned is not in this channel now. Add them or remove the mention, then retry.", ], ); assert.equal( @@ -1094,3 +1122,96 @@ for (const main of [false, true]) { }); } } + +// Every signed recipient must be on the fresh roster, the managed agent +// included, and that roster read must follow agent authorization. Hold the +// managed-agent read, remove a recipient, then release it. +const SCOUT = { + pubkey: KEY, + role: "bot", + is_agent: true, + display_name: "Scout", +}; +for (const main of [false, true]) { + for (const [removed, when] of [ + ["agent", "after-selection"], + ["agent", "during-agent-read"], + ["person", "during-agent-read"], + ]) { + test(`mounted ${main ? "main" : "forum"} publish refuses the ${removed} removed ${when}`, async () => { + const s = await setup({ realMentions: true, main }); + s.control.cachedMembers = [ + { pubkey: HUMAN, role: "member", isAgent: false, displayName: "Pat" }, + { pubkey: KEY, role: "bot", isAgent: true, displayName: "Scout" }, + ]; + s.control.liveMembers = [PAT, SCOUT]; + s.navigate("b"); + await s.open(); + await s.choose("Pat"); + await s.open(); + await s.choose("Scout"); + assert.deepEqual( + [...s.mentions.extractMentionPubkeys(s.text)].sort(), + [KEY, HUMAN].sort(), + ); + const text = s.text; + const remove = () => { + const gone = removed === "agent" ? KEY : HUMAN; + s.control.liveMembers = s.control.liveMembers.filter( + (member) => member.pubkey !== gone, + ); + }; + if (when === "after-selection") remove(); + else s.control.agentRead = deferred(); + const reads = s.calls.length; + await s.submit(); + if (when === "during-agent-read") { + assert.equal(s.control.phase, "publish"); + assert.ok( + s.calls.slice(reads).some(([name]) => name === "agents-read"), + "publish is held at agent authorization", + ); + assert.equal(s.calls.filter(([name]) => name === "send").length, 0); + remove(); + await s.finish(s.control.agentRead); + await act(async () => {}); + } + await act(async () => {}); + assert.equal(s.calls.filter(([name]) => name === "send").length, 0); + assert.deepEqual( + s.calls + .filter(([name]) => name === "error") + .map(([, message]) => message), + [ + "Someone you mentioned is not in this channel now. Add them or remove the mention, then retry.", + ], + ); + assert.equal( + s.text.trim(), + text.trim(), + "the draft is retained for retry", + ); + }); + } +} + +for (const main of [false, true]) + test(`mounted ${main ? "main" : "forum"} publish keeps a person and agent who are both still members`, async () => { + const s = await setup({ realMentions: true, main }); + s.control.cachedMembers = [ + { pubkey: HUMAN, role: "member", isAgent: false, displayName: "Pat" }, + { pubkey: KEY, role: "bot", isAgent: true, displayName: "Scout" }, + ]; + s.control.liveMembers = [PAT, SCOUT]; + s.navigate("b"); + await s.open(); + await s.choose("Pat"); + await s.open(); + await s.choose("Scout"); + await s.submit(); + await act(async () => {}); + const sends = s.calls.filter(([name]) => name === "send"); + assert.equal(sends.length, 1); + assert.ok(sends[0].flat(2).includes(HUMAN)); + assert.ok(sends[0].flat(2).includes(KEY)); + }); diff --git a/desktop/src/features/messages/lib/agentMentionRevalidation.ts b/desktop/src/features/messages/lib/agentMentionRevalidation.ts index 82d36a0efa..8de9fd9db3 100644 --- a/desktop/src/features/messages/lib/agentMentionRevalidation.ts +++ b/desktop/src/features/messages/lib/agentMentionRevalidation.ts @@ -23,13 +23,13 @@ export class AgentMentionAuthorizationError extends Error { } } -/** A mentioned person is not a member of the destination at publication. */ +/** A signed recipient is not a member of the destination at publication. */ export class MentionMembershipChangedError extends Error { constructor(unverified = false) { super( unverified - ? "Could not check that the people you mentioned are still in this channel. Retry, or remove the mentions." - : "A mentioned person is no longer in this channel. Invite them or remove the mention, then retry.", + ? "Could not check that everyone you mentioned is still in this channel. Retry, or remove the mentions." + : "Someone you mentioned is not in this channel now. Add them or remove the mention, then retry.", ); this.name = "MentionMembershipChangedError"; } @@ -44,34 +44,30 @@ export function isMentionAuthorizationError(error: unknown): error is Error { } /** - * Publication signs a `p` tag for every person in `pubkeys`. Selection and - * the Invite prompt prove membership only at their own time, so read the - * destination's member list fresh and fail closed for any person who is not - * a member now (from the writer: a lagging replica can still list someone - * just removed). Send without inviting has already moved declined people to - * reference tags, and invited people are members by this point. + * Publication signs a `p` tag for every recipient in `pubkeys`, people and + * agents alike. Selection, Invite and agent attach prove membership only at + * their own time, so read the destination's member list fresh and fail + * closed for any recipient who is not a member now (from the writer: a + * lagging replica can still list someone just removed). Send without + * inviting has already moved declined people to reference tags. */ -export async function revalidateHumanMentionMembership({ +export async function revalidateRecipientMembership({ pubkeys, - agentPubkeys, channelId, fetchMembers = (id) => getChannelMembers(id, { readYourWrites: true }), }: { pubkeys: readonly string[]; - agentPubkeys: ReadonlySet; channelId: string; fetchMembers?: (channelId: string) => Promise<{ pubkey: string }[]>; }) { - const humans = [...new Set(pubkeys.map(normalizePubkey))].filter( - (pubkey) => !agentPubkeys.has(pubkey), - ); - if (humans.length === 0) return; + const recipients = [...new Set(pubkeys.map(normalizePubkey))]; + if (recipients.length === 0) return; const members = await fetchMembers(channelId).catch(() => null); if (!members) throw new MentionMembershipChangedError(true); const memberPubkeys = new Set( members.map((member) => normalizePubkey(member.pubkey)), ); - if (humans.some((pubkey) => !memberPubkeys.has(pubkey))) + if (recipients.some((pubkey) => !memberPubkeys.has(pubkey))) throw new MentionMembershipChangedError(); } @@ -184,18 +180,6 @@ export function useAgentMentionRevalidation({ ...getSelectedAgentPubkeys(), ...(options.intendedAgentPubkeys ?? []).map(normalizePubkey), ]); - // Only the explicit publish pass signs new recipients into a channel. - // DMs have fixed participants and no member list to change. - const humanMembership = - options.phase === "publish" && - destinationChannelId && - channelType !== "dm" - ? revalidateHumanMentionMembership({ - pubkeys, - agentPubkeys: knownAgentPubkeys, - channelId: destinationChannelId, - }) - : Promise.resolve(); const agents = revalidateAgentMentionPubkeys({ pubkeys, agentPubkeys: knownAgentPubkeys, @@ -210,7 +194,23 @@ export function useAgentMentionRevalidation({ "channelId" in scope ? (scope.channelId ?? undefined) : undefined, ), }); - return Promise.all([agents, humanMembership]).then(([result]) => result); + // Only the explicit publish pass signs new recipients into a channel. + // DMs have fixed participants and no member list to change. The roster + // read is the last awaited step: a removal during agent authorization + // must still be seen before signing. + if ( + options.phase !== "publish" || + !destinationChannelId || + channelType === "dm" + ) + return agents; + return agents.then(async (validated) => { + await revalidateRecipientMembership({ + pubkeys: validated, + channelId: destinationChannelId, + }); + return validated; + }); }, [ agentPubkeys, diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.membership.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.membership.test.mjs index 1be119722f..cda215b648 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.membership.test.mjs +++ b/desktop/src/features/messages/ui/useMentionSendFlow.membership.test.mjs @@ -10,12 +10,15 @@ import { setup, deferred, KEY } from "./useMentionSendFlow.test-support.mjs"; const HUMAN = "d".repeat(64); const TEXT = "@Pat @RemoteScout hello"; const REMOVED = - "A mentioned person is no longer in this channel. Invite them or remove the mention, then retry."; + "Someone you mentioned is not in this channel now. Add them or remove the mention, then retry."; + +const AGENT_MEMBER = { pubkey: KEY, role: "bot", is_agent: true }; async function membershipSetup(delay) { const live = { members: [{ pubkey: HUMAN, role: "member", is_agent: false }], gate: null, + agentRead: null, }; window.__TAURI_INTERNALS__ = { invoke: async (command) => { @@ -34,10 +37,12 @@ async function membershipSetup(delay) { currentPubkey: "a".repeat(64), eligibilityScope: { type: "channel", channelId: "general" }, sharedChannelIds: new Set(), - refetchManagedAgents: async () => ({ - data: [{ pubkey: KEY }], - error: null, - }), + refetchManagedAgents: async () => { + // Hold only the final publish pass, not the earlier prepare pass. + if (live.agentRead && live.phase === "publish") + await live.agentRead.promise; + return { data: [{ pubkey: KEY }], error: null }; + }, }), ); const s = await setup(); @@ -49,6 +54,7 @@ async function membershipSetup(delay) { mentions.memberPubkeys = new Set([HUMAN]); mentions.revalidateMentionPubkeys = async (keys, channel, options) => { s.calls.push([options.phase, channel]); + live.phase = options.phase; return revalidate.current(keys, channel, options); }; if (delay === "upload") @@ -57,19 +63,28 @@ async function membershipSetup(delay) { return { s, live }; } +// Every signed recipient, the managed agent included, must be on the fresh +// roster. The agent joins through the Invite prompt, then can be removed. +const REMOVALS = { + none: (members) => members, + person: (members) => members.filter(({ pubkey }) => pubkey !== HUMAN), + agent: (members) => members.filter(({ pubkey }) => pubkey !== KEY), +}; + for (const delay of ["upload", "invite"]) { - for (const removed of [false, true]) { - test(`chat publish ${removed ? "refuses" : "keeps"} a person ${removed ? "removed" : "still present"} during the ${delay} delay`, async () => { + for (const removed of Object.keys(REMOVALS)) { + test(`chat publish ${removed === "none" ? "keeps everyone present" : `refuses the ${removed} removed`} during the ${delay} delay`, async () => { const { s, live } = await membershipSetup(delay); if (delay === "invite") s.control.add = deferred(); await s.prompt(TEXT); await s.invite(); assert.equal(s.events("add").length, 1); + live.members = [...live.members, AGENT_MEMBER]; if (delay === "upload") { assert.ok(s.control.uploadCallbacks); assert.equal(s.options.contentRef.current, "", "optimistic clear"); } - if (removed) live.members = []; + live.members = REMOVALS[removed](live.members); if (delay === "invite") await s.finish(s.control.add); else await s.act(async () => @@ -79,26 +94,52 @@ for (const delay of ["upload", "invite"]) { ), ); await s.flush(); - const sends = s.events("SEND"); - if (!removed) { - assert.equal(sends.length, 1); - assert.deepEqual(sends[0][2], [HUMAN, KEY]); - return; - } - assert.equal(sends.length, 0); - assert.deepEqual( - s.events("error").map(([, message]) => message), - [REMOVED], - ); - assert.equal( - s.options.contentRef.current, - TEXT, - "the draft is kept for retry", - ); + assertOutcome(s, removed !== "none"); }); } } +function assertOutcome(s, refused) { + const sends = s.events("SEND"); + if (!refused) { + assert.equal(sends.length, 1); + assert.deepEqual(sends[0][2], [HUMAN, KEY]); + return; + } + assert.equal(sends.length, 0); + assert.deepEqual( + s.events("error").map(([, message]) => message), + [REMOVED], + ); + assert.equal( + s.options.contentRef.current, + TEXT, + "the draft is kept for retry", + ); +} + +// The roster read must be the last check. A removal while the agent +// directory read is still pending has to be seen before signing. +for (const removed of ["person", "agent"]) { + test(`chat publish refuses the ${removed} removed while agent authorization is held`, async () => { + const { s, live } = await membershipSetup("invite"); + s.control.add = deferred(); + await s.prompt(TEXT); + await s.invite(); + live.members = [...live.members, AGENT_MEMBER]; + live.agentRead = deferred(); + await s.finish(s.control.add); + await s.flush(); + assert.equal(live.phase, "publish", "held in the final publish pass"); + assert.equal(s.events("SEND").length, 0); + assert.equal(s.events("error").length, 0, "not yet refused"); + live.members = REMOVALS[removed](live.members); + await s.finish(live.agentRead); + await s.flush(); + assertOutcome(s, true); + }); +} + test("Send without inviting still publishes: a declined person becomes a reference", async () => { const DECLINED = "e".repeat(64); const { s } = await membershipSetup("decline");