diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs index 4e02b7bd68..9f01beb710 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs @@ -6,8 +6,10 @@ import { getMentionableAgentPubkeys, getSharedChannelIds, isAgentIdentityInManagedList, + isAgentIdentityReachableForMentions, relayAgentIsSharedWithUser, shouldHideAgentFromMentions, + shouldOfferAgentIdentityForMentions, } from "./agentAutocompleteEligibility.ts"; const CURRENT_PUBKEY = "a".repeat(64); @@ -162,6 +164,224 @@ test("isAgentIdentityInManagedList: keeps people and only current managed agent ); }); +test("isAgentIdentityReachableForMentions: keeps channel members the viewer does not manage", () => { + const managedAgentPubkeys = new Set([PUB_A]); + + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: true, isMember: true, pubkey: PUB_B }, + managedAgentPubkeys, + true, + ), + true, + ); + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: true, isMember: false, pubkey: PUB_B }, + managedAgentPubkeys, + true, + ), + false, + ); + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: true, isMember: false, pubkey: PUB_A.toUpperCase() }, + managedAgentPubkeys, + true, + ), + true, + ); + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: false, isMember: false, pubkey: PUB_B }, + managedAgentPubkeys, + true, + ), + true, + ); +}); + +test("isAgentIdentityReachableForMentions: holds the member pass-through until the relay directory is ready", () => { + const managedAgentPubkeys = new Set([PUB_A]); + + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: true, isMember: true, pubkey: PUB_B }, + managedAgentPubkeys, + false, + ), + false, + ); + // A managed agent and a human are unaffected by the directory load state. + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: true, isMember: true, pubkey: PUB_A }, + managedAgentPubkeys, + false, + ), + true, + ); + assert.equal( + isAgentIdentityReachableForMentions( + { isAgent: false, isMember: true, pubkey: PUB_B }, + managedAgentPubkeys, + false, + ), + true, + ); +}); + +/** + * Drives the real admission check `useMentions`' `addCandidate` calls, building + * its pubkey sets the same way the hook does from `relayAgentsQuery.data`. + * `tests/e2e/mentions.spec.ts` covers the same scenarios through the composer. + */ +function mentionCandidateIsOffered( + candidate, + { + managedAgentPubkeys, + relayAgents, + sharedChannelIds, + relayAgentDirectoryReady = true, + }, +) { + return shouldOfferAgentIdentityForMentions({ + candidate, + managedAgentPubkeys, + mentionableAgentPubkeys: getMentionableAgentPubkeys({ + currentPubkey: CURRENT_PUBKEY, + managedAgentPubkeys, + relayAgents, + sharedChannelIds, + }), + directoryAgentPubkeys: new Set( + relayAgents.map((agent) => agent.pubkey.toLowerCase()), + ), + relayAgentDirectoryReady, + }); +} + +test("mention path offers an invocable channel bot from another install", () => { + const crossOwnerBot = { isAgent: true, isMember: true, pubkey: PUB_B }; + + assert.equal( + mentionCandidateIsOffered(crossOwnerBot, { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [ + { + pubkey: PUB_B, + respondTo: "allowlist", + respondToAllowlist: [CURRENT_PUBKEY], + channelIds: ["general"], + }, + ], + sharedChannelIds: new Set(["general"]), + }), + true, + ); + assert.equal( + mentionCandidateIsOffered(crossOwnerBot, { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [ + { + pubkey: PUB_B, + respondTo: "anyone", + respondToAllowlist: [], + channelIds: ["general"], + }, + ], + sharedChannelIds: new Set(["general"]), + }), + true, + ); +}); + +test("mention path still hides a channel bot whose directory entry excludes the viewer", () => { + assert.equal( + mentionCandidateIsOffered( + { isAgent: true, isMember: true, pubkey: PUB_B }, + { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [ + { + pubkey: PUB_B, + respondTo: "owner-only", + respondToAllowlist: [], + channelIds: ["general"], + }, + ], + sharedChannelIds: new Set(["general"]), + }, + ), + false, + ); + assert.equal( + mentionCandidateIsOffered( + { isAgent: true, isMember: true, pubkey: PUB_B }, + { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [ + { + pubkey: PUB_B, + respondTo: "allowlist", + respondToAllowlist: [OTHER_OWNER_PUBKEY], + channelIds: ["general"], + }, + ], + sharedChannelIds: new Set(["general"]), + }, + ), + false, + ); +}); + +test("mention path does not offer a channel bot while the relay directory is still loading", () => { + // In flight, `relayAgents` is empty: the bot is neither invocable nor + // directory-present, so `shouldHideAgentFromMentions` would read it as + // unknown-invocability and show it. Readiness is what keeps it hidden until + // the directory can answer. + assert.equal( + mentionCandidateIsOffered( + { isAgent: true, isMember: true, pubkey: PUB_B }, + { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [], + sharedChannelIds: new Set(["general"]), + relayAgentDirectoryReady: false, + }, + ), + false, + ); + // Once ready, an empty or errored directory falls back to Option B (unknown + // invocability => show) rather than hiding members indefinitely. + assert.equal( + mentionCandidateIsOffered( + { isAgent: true, isMember: true, pubkey: PUB_B }, + { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [], + sharedChannelIds: new Set(["general"]), + relayAgentDirectoryReady: true, + }, + ), + true, + ); +}); + +test("mention path keeps unreachable non-member agents out of the composer", () => { + assert.equal( + mentionCandidateIsOffered( + { isAgent: true, isMember: false, pubkey: PUB_B }, + { + managedAgentPubkeys: new Set([PUB_A]), + relayAgents: [], + sharedChannelIds: new Set(["general"]), + }, + ), + false, + ); +}); + test("shouldHideAgentFromMentions: never hides non-agents", () => { assert.equal( shouldHideAgentFromMentions({ diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts index e4afe7fea4..a4f86cb7c0 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts @@ -64,6 +64,32 @@ export function isAgentIdentityInManagedList( ); } +export function isAgentIdentityReachableForMentions( + candidate: { isAgent?: boolean; isMember?: boolean; pubkey: string }, + managedAgentPubkeys: ReadonlySet, + relayAgentDirectoryReady: boolean, +) { + // `managedAgentPubkeys` only ever lists agents this Desktop install runs, so + // it cannot decide whether a channel member is invocable — an agent owned by + // a teammate is absent from it no matter how its `respond_to` is configured. + // Members are therefore left to `shouldHideAgentFromMentions`, which reads + // the relay directory. Non-members keep the managed-list gate so the + // composer still does not offer unreachable identities. + // + // The member pass-through waits for the relay directory because + // `shouldHideAgentFromMentions` reads an absent directory entry as "unknown + // invocability => show". Without this guard, an agent whose directory entry + // excludes the viewer would be offered for as long as that query is in + // flight. While the query is still loading, readiness stays false and + // members stay hidden (same as pre-fix). An errored or empty directory + // still counts as ready, so a finished-but-empty/failed fetch degrades to + // Option B (show) rather than hiding members indefinitely. + return ( + (candidate.isMember === true && relayAgentDirectoryReady) || + isAgentIdentityInManagedList(candidate, managedAgentPubkeys) + ); +} + export function shouldHideAgentFromMentions({ isAgent, isMember, @@ -97,6 +123,43 @@ export function shouldHideAgentFromMentions({ return directoryAgentPubkeys.has(normalized); } +/** + * The full mention-autocomplete admission check: reachability first, then the + * invocability gate. Kept here as one exported step so the composer has a + * single call and tests exercise the real chain instead of a copy of it. + */ +export function shouldOfferAgentIdentityForMentions({ + candidate, + managedAgentPubkeys, + mentionableAgentPubkeys, + directoryAgentPubkeys, + relayAgentDirectoryReady, +}: { + candidate: { isAgent?: boolean; isMember?: boolean; pubkey: string }; + managedAgentPubkeys: ReadonlySet; + mentionableAgentPubkeys: ReadonlySet; + directoryAgentPubkeys: ReadonlySet; + relayAgentDirectoryReady: boolean; +}) { + if ( + !isAgentIdentityReachableForMentions( + candidate, + managedAgentPubkeys, + relayAgentDirectoryReady, + ) + ) { + return false; + } + + return !shouldHideAgentFromMentions({ + isAgent: candidate.isAgent === true, + isMember: candidate.isMember === true, + pubkey: candidate.pubkey, + mentionableAgentPubkeys, + directoryAgentPubkeys, + }); +} + type AgentAutocompleteCandidate = { pubkey?: string; displayName?: string | null; diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 0c73b75339..f80a8e6993 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -16,8 +16,7 @@ import { coalesceAutocompleteCandidatesByKey, getMentionableAgentPubkeys, getSharedChannelIds, - isAgentIdentityInManagedList, - shouldHideAgentFromMentions, + shouldOfferAgentIdentityForMentions, } from "@/features/agents/lib/agentAutocompleteEligibility"; import { useInfiniteUserSearchQuery, @@ -246,16 +245,13 @@ export function useMentions( if (isArchivedDiscovery(pubkey)) { return; } - if (!isAgentIdentityInManagedList(candidate, managedAgentPubkeys)) { - return; - } if ( - shouldHideAgentFromMentions({ - isAgent: candidate.isAgent === true, - isMember: candidate.isMember === true, - pubkey, + !shouldOfferAgentIdentityForMentions({ + candidate: { ...candidate, pubkey }, + managedAgentPubkeys, mentionableAgentPubkeys, directoryAgentPubkeys, + relayAgentDirectoryReady, }) ) { return; @@ -427,6 +423,7 @@ export function useMentions( mentionableAgentPubkeys, personaNameByPubkey, profiles, + relayAgentDirectoryReady, relayAgentNamesByPubkey, relayAgentsQuery.data, ]); diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index 694b5abef5..5254fa8fac 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -209,7 +209,9 @@ test("@ trigger prioritizes channel members before runnable personas and other m const dropdown = autocomplete(page); await expect(dropdown).toBeVisible(); - await expect(dropdown.getByText("alice")).toHaveCount(0); + // alice is a channel member and a relay agent that responds to anyone, so she + // is invocable here even though this install does not manage her (#3809). + await expect(dropdown.getByText("alice")).toBeVisible(); await expect(dropdown.getByText("bob")).toBeVisible(); await expect(dropdown.getByText("Fizz")).toBeVisible(); await expect(dropdown.getByText("charlie")).toBeVisible(); @@ -217,14 +219,13 @@ test("@ trigger prioritizes channel members before runnable personas and other m const charlieRow = dropdown.locator("button", { hasText: "charlie" }); await expect(charlieRow.getByTestId("mention-agent-icon")).toBeVisible(); await expect(charlieRow.getByText("not in channel")).toBeVisible(); - await expect( - dropdown - .locator("button", { hasText: "alice" }) - .getByText("not in channel"), - ).not.toBeVisible(); + const aliceRow = dropdown.locator("button", { hasText: "alice" }); + await expect(aliceRow.getByTestId("mention-agent-icon")).toBeVisible(); + await expect(aliceRow.getByText("not in channel")).not.toBeVisible(); const suggestions = dropdown.locator("button"); const suggestionText = await suggestions.allInnerTexts(); + const aliceIndex = suggestionText.findIndex((text) => text.includes("alice")); const fizzIndex = suggestionText.findIndex((text) => text.includes("Fizz")); const bobIndex = suggestionText.findIndex((text) => text.includes("bob")); const charlieIndex = suggestionText.findIndex((text) => @@ -233,10 +234,12 @@ test("@ trigger prioritizes channel members before runnable personas and other m const outsiderIndex = suggestionText.findIndex((text) => text.includes("outsider"), ); + expect(aliceIndex).toBeGreaterThanOrEqual(0); expect(fizzIndex).toBeGreaterThanOrEqual(0); expect(bobIndex).toBeGreaterThanOrEqual(0); expect(charlieIndex).toBeGreaterThanOrEqual(0); expect(outsiderIndex).toEqual(-1); + expect(aliceIndex).toBeLessThan(fizzIndex); expect(bobIndex).toBeLessThan(fizzIndex); expect(fizzIndex).toBeLessThan(charlieIndex); });