diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs index 4e02b7bd68..0e3d5ea3a7 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs @@ -4,8 +4,10 @@ import test from "node:test"; import { coalesceAgentAutocompleteCandidates, getMentionableAgentPubkeys, + getOwnedMemberAgentPubkeys, getSharedChannelIds, isAgentIdentityInManagedList, + isAgentMentionEligible, relayAgentIsSharedWithUser, shouldHideAgentFromMentions, } from "./agentAutocompleteEligibility.ts"; @@ -162,6 +164,108 @@ test("isAgentIdentityInManagedList: keeps people and only current managed agent ); }); +test("isAgentIdentityInManagedList: keeps owned agents that are channel members", () => { + // An agent whose process lives on another machine is absent from this + // machine's managed list, but its owner must still be able to mention it. + const managedAgentPubkeys = new Set([PUB_A]); + + assert.equal( + isAgentIdentityInManagedList( + { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + }, + managedAgentPubkeys, + CURRENT_PUBKEY, + ), + true, + ); +}); + +test("isAgentIdentityInManagedList: normalizes pubkeys on the ownership branch", () => { + assert.equal( + isAgentIdentityInManagedList( + { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY.toUpperCase(), + }, + new Set(), + CURRENT_PUBKEY, + ), + true, + ); +}); + +test("isAgentIdentityInManagedList: still hides owned agents that are not members", () => { + // Ownership alone is not reachability: a profile-only agent we own has no + // evidence of running anywhere, and #1243 hides it on purpose. + assert.equal( + isAgentIdentityInManagedList( + { + isAgent: true, + isMember: false, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + }, + new Set(), + CURRENT_PUBKEY, + ), + false, + ); + assert.equal( + isAgentIdentityInManagedList( + { isAgent: true, pubkey: PUB_B, ownerPubkey: CURRENT_PUBKEY }, + new Set(), + CURRENT_PUBKEY, + ), + false, + ); +}); + +test("isAgentIdentityInManagedList: still hides agents owned by someone else", () => { + assert.equal( + isAgentIdentityInManagedList( + { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: PUB_D, + }, + new Set(), + CURRENT_PUBKEY, + ), + false, + ); +}); + +test("isAgentIdentityInManagedList: ownership branch is inert without a current pubkey", () => { + // Callers that do not pass `currentPubkey` keep the previous behaviour. + assert.equal( + isAgentIdentityInManagedList( + { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + }, + new Set(), + ), + false, + ); + assert.equal( + isAgentIdentityInManagedList( + { isAgent: true, isMember: true, pubkey: PUB_B, ownerPubkey: null }, + new Set(), + CURRENT_PUBKEY, + ), + false, + ); +}); + test("shouldHideAgentFromMentions: never hides non-agents", () => { assert.equal( shouldHideAgentFromMentions({ @@ -214,6 +318,56 @@ test("shouldHideAgentFromMentions: hides member agents with an explicit not-invo ); }); +test("shouldHideAgentFromMentions: shows directory agents owned by the current user", () => { + // An `owner-only` agent is absent from `mentionableAgentPubkeys` by + // construction, so its directory entry must not read as not-invocable for + // the one account it always answers. + assert.equal( + shouldHideAgentFromMentions({ + isAgent: true, + isMember: true, + pubkey: PUB_A, + ownerPubkey: CURRENT_PUBKEY.toUpperCase(), + currentPubkey: CURRENT_PUBKEY, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set([PUB_A]), + }), + false, + ); +}); + +test("shouldHideAgentFromMentions: still hides directory agents owned by someone else", () => { + assert.equal( + shouldHideAgentFromMentions({ + isAgent: true, + isMember: true, + pubkey: PUB_A, + ownerPubkey: PUB_D, + currentPubkey: CURRENT_PUBKEY, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set([PUB_A]), + }), + true, + ); +}); + +test("shouldHideAgentFromMentions: still hides non-member agents owned by the current user", () => { + // The ownership escape is scoped to members: membership is the reachability + // evidence, ownership only the permission. + assert.equal( + shouldHideAgentFromMentions({ + isAgent: true, + isMember: false, + pubkey: PUB_A, + ownerPubkey: CURRENT_PUBKEY, + currentPubkey: CURRENT_PUBKEY, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set(), + }), + true, + ); +}); + test("shouldHideAgentFromMentions: shows member agents with unknown invocability (not in directory)", () => { assert.equal( shouldHideAgentFromMentions({ @@ -306,3 +460,118 @@ test("coalesceAgentAutocompleteCandidates: leaves non-agents alone", () => { assert.deepEqual(coalesce([first, second]), [first, second]); }); + +test("isAgentMentionEligible: offers an owned remote agent that is a channel member", () => { + // The reported bug: the agent's process runs on another machine, so it is + // absent from `managedAgentPubkeys`, and its default `owner-only` policy + // keeps it out of `mentionableAgentPubkeys`. Its directory entry then reads + // as an explicit exclusion. It is still a member, still answering, and still + // ours. + assert.equal( + isAgentMentionEligible({ + candidate: { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + }, + currentPubkey: CURRENT_PUBKEY, + directoryAgentPubkeys: new Set([PUB_B]), + managedAgentPubkeys: new Set(), + mentionableAgentPubkeys: new Set(), + }), + true, + ); +}); + +test("isAgentMentionEligible: keeps hiding an owned agent that is not a member", () => { + assert.equal( + isAgentMentionEligible({ + candidate: { + isAgent: true, + isMember: false, + pubkey: PUB_B, + ownerPubkey: CURRENT_PUBKEY, + }, + currentPubkey: CURRENT_PUBKEY, + directoryAgentPubkeys: new Set(), + managedAgentPubkeys: new Set(), + mentionableAgentPubkeys: new Set(), + }), + false, + ); +}); + +test("isAgentMentionEligible: keeps hiding a member agent owned by someone else", () => { + assert.equal( + isAgentMentionEligible({ + candidate: { + isAgent: true, + isMember: true, + pubkey: PUB_B, + ownerPubkey: PUB_D, + }, + currentPubkey: CURRENT_PUBKEY, + directoryAgentPubkeys: new Set([PUB_B]), + managedAgentPubkeys: new Set(), + mentionableAgentPubkeys: new Set(), + }), + false, + ); +}); + +test("isAgentMentionEligible: leaves people alone", () => { + assert.equal( + isAgentMentionEligible({ + candidate: { isAgent: false, isMember: false, pubkey: PUB_B }, + currentPubkey: CURRENT_PUBKEY, + directoryAgentPubkeys: new Set([PUB_B]), + managedAgentPubkeys: new Set(), + mentionableAgentPubkeys: new Set(), + }), + true, + ); +}); + +test("isAgentMentionEligible: applies both gates, not just the managed-list one", () => { + // A managed agent still goes through the invocability gate; a directory + // entry that excludes us wins over local management for a member. + assert.equal( + isAgentMentionEligible({ + candidate: { isAgent: true, isMember: true, pubkey: PUB_A }, + currentPubkey: CURRENT_PUBKEY, + directoryAgentPubkeys: new Set([PUB_A]), + managedAgentPubkeys: new Set([PUB_A]), + mentionableAgentPubkeys: new Set(), + }), + false, + ); +}); + +test("getOwnedMemberAgentPubkeys: collects in-channel agents owned by the current user", () => { + // These are exactly the agents the ownership branch newly admits, so + // downstream agent classification must recognise them too. + assert.deepEqual( + getOwnedMemberAgentPubkeys( + [ + { isAgent: true, isMember: true, pubkey: PUB_A.toUpperCase(), ownerPubkey: CURRENT_PUBKEY }, + { isAgent: true, isMember: false, pubkey: PUB_B, ownerPubkey: CURRENT_PUBKEY }, + { isAgent: true, isMember: true, pubkey: PUB_C, ownerPubkey: PUB_D }, + { isAgent: false, isMember: true, pubkey: PUB_D, ownerPubkey: CURRENT_PUBKEY }, + { isAgent: true, isMember: true, ownerPubkey: CURRENT_PUBKEY }, + ], + CURRENT_PUBKEY, + ), + new Set([PUB_A]), + ); +}); + +test("getOwnedMemberAgentPubkeys: empty without a current pubkey", () => { + assert.deepEqual( + getOwnedMemberAgentPubkeys( + [{ isAgent: true, isMember: true, pubkey: PUB_A, ownerPubkey: CURRENT_PUBKEY }], + null, + ), + new Set(), + ); +}); diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts index e4afe7fea4..c2059b5721 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts @@ -54,26 +54,75 @@ export function getMentionableAgentPubkeys({ return pubkeys; } +/** + * Whether an autocomplete candidate that is an agent identity may be shown. + * + * Agents are hidden unless we can reach them, so a mention never silently + * fails. Two things make an agent reachable by the current user: + * + * 1. It is in this machine's managed-agent list (we can spawn it on demand). + * 2. It is a member of this channel **and** the current user owns it. #1243 + * set out to "scope mention/add autocomplete to reachable identities" and + * described an eligible agent as "my managed/owned agent", but only the + * managed list was ever consulted. Membership supplies the reachability + * evidence that ownership alone does not: the agent is in the channel and + * answering, so it is running somewhere. Ownership then supplies the + * permission, and it does not depend on which machine hosts the process — + * an agent left on the default `respond_to: owner-only` accepts its owner + * by definition, so hiding it from that owner is backwards. + * + * Deliberately NOT extended to non-member owned agents: a profile-only agent + * we own has no evidence of running anywhere, and #1243 hides it on purpose. + * + * `currentPubkey` is optional so existing callers keep their behaviour; pass + * it to enable the ownership branch. + */ export function isAgentIdentityInManagedList( - candidate: { isAgent?: boolean; pubkey: string }, + candidate: { + isAgent?: boolean; + isMember?: boolean; + pubkey: string; + ownerPubkey?: string | null; + }, managedAgentPubkeys: ReadonlySet, + currentPubkey?: string | null, ) { + if (candidate.isAgent !== true) { + return true; + } + if (managedAgentPubkeys.has(normalizePubkey(candidate.pubkey))) { + return true; + } return ( - candidate.isAgent !== true || - managedAgentPubkeys.has(normalizePubkey(candidate.pubkey)) + candidate.isMember === true && + isOwnedByCurrentUser(candidate.ownerPubkey, currentPubkey) ); } +function isOwnedByCurrentUser( + ownerPubkey: string | null | undefined, + currentPubkey: string | null | undefined, +) { + if (!ownerPubkey || !currentPubkey) { + return false; + } + return normalizePubkey(ownerPubkey) === normalizePubkey(currentPubkey); +} + export function shouldHideAgentFromMentions({ isAgent, isMember, pubkey, + ownerPubkey, + currentPubkey, mentionableAgentPubkeys, directoryAgentPubkeys, }: { isAgent: boolean; isMember: boolean; pubkey: string; + ownerPubkey?: string | null; + currentPubkey?: string | null; mentionableAgentPubkeys: ReadonlySet; directoryAgentPubkeys: ReadonlySet; }) { @@ -83,6 +132,14 @@ export function shouldHideAgentFromMentions({ if (mentionableAgentPubkeys.has(normalized)) return false; // Non-member, non-invocable => hide (preserves prior behavior). if (!isMember) return true; + // A member we own => invocable, wherever its process runs. + // `mentionableAgentPubkeys` only admits relay agents whose `respond_to` is + // `anyone` or an allowlist naming us, so an agent left on the default + // `owner-only` never lands there — yet `owner-only` is precisely the mode + // that always accepts its owner. Without this, the directory check below + // reads that agent's kind:10100 entry as an explicit not-invocable signal + // and hides it from the one person guaranteed to be able to invoke it. + if (isOwnedByCurrentUser(ownerPubkey, currentPubkey)) return false; // Member (Option B): hide only when we have an explicit not-invocable // signal — a relay directory (kind:10100) entry that excludes us. // Unknown invocability (not in directory) => show. @@ -97,6 +154,91 @@ export function shouldHideAgentFromMentions({ return directoryAgentPubkeys.has(normalized); } +/** + * Agent pubkeys the current user can address even though the relay directory + * never marks them invocable: in-channel agents they own. + * + * `getMentionableAgentPubkeys` only admits `respond_to: anyone` or an + * allowlist naming us, so an `owner-only` agent is absent from it. That set + * also drives downstream agent classification (`isAgentPubkey`), which decides + * whether a sent mention is kept as the persistent agent audience. Offering + * such an agent in autocomplete without adding it here would surface it and + * then fail to treat it as an agent once addressed. + * + * Derived from the same ownership rule as `isAgentIdentityInManagedList`, so + * eligibility and classification cannot drift apart. + */ +export function getOwnedMemberAgentPubkeys( + candidates: readonly { + isAgent?: boolean; + isMember?: boolean; + pubkey?: string; + ownerPubkey?: string | null; + }[], + currentPubkey?: string | null, +) { + const pubkeys = new Set(); + for (const candidate of candidates) { + if ( + candidate.isAgent !== true || + candidate.isMember !== true || + !candidate.pubkey + ) { + continue; + } + if (isOwnedByCurrentUser(candidate.ownerPubkey, currentPubkey)) { + pubkeys.add(normalizePubkey(candidate.pubkey)); + } + } + return pubkeys; +} + +/** + * The single eligibility decision for an @-mention autocomplete candidate. + * + * The two predicates above encode one policy but are ordered: the managed-list + * gate runs first and can reject a candidate before the invocability gate ever + * applies its "invocable => show" rule. Composing them here keeps that order + * explicit and in one place, so a caller cannot chain them the other way round + * or apply only half the policy. + */ +export function isAgentMentionEligible({ + candidate, + currentPubkey, + directoryAgentPubkeys, + managedAgentPubkeys, + mentionableAgentPubkeys, +}: { + candidate: { + isAgent?: boolean; + isMember?: boolean; + pubkey: string; + ownerPubkey?: string | null; + }; + currentPubkey?: string | null; + directoryAgentPubkeys: ReadonlySet; + managedAgentPubkeys: ReadonlySet; + mentionableAgentPubkeys: ReadonlySet; +}) { + if (candidate.isAgent !== true) { + return true; + } + if ( + !isAgentIdentityInManagedList(candidate, managedAgentPubkeys, currentPubkey) + ) { + return false; + } + return !shouldHideAgentFromMentions({ + currentPubkey, + directoryAgentPubkeys, + isAgent: true, + isMember: candidate.isMember === true, + mentionableAgentPubkeys, + ownerPubkey: candidate.ownerPubkey, + pubkey: candidate.pubkey, + }); +} + 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..bbd8b1f0d2 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -15,9 +15,9 @@ import { coalesceAgentAutocompleteCandidates, coalesceAutocompleteCandidatesByKey, getMentionableAgentPubkeys, + getOwnedMemberAgentPubkeys, getSharedChannelIds, - isAgentIdentityInManagedList, - shouldHideAgentFromMentions, + isAgentMentionEligible, } from "@/features/agents/lib/agentAutocompleteEligibility"; import { useInfiniteUserSearchQuery, @@ -220,7 +220,6 @@ export function useMentions( } return lookup; }, [managedAgentsQuery.data, personasQuery.data]); - const knownAgentPubkeys = mentionableAgentPubkeys; const activePersonas = React.useMemo( () => (personasQuery.data ?? []).filter((persona) => persona.isActive), [personasQuery.data], @@ -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, - mentionableAgentPubkeys, + !isAgentMentionEligible({ + candidate: { ...candidate, pubkey }, + currentPubkey, directoryAgentPubkeys, + managedAgentPubkeys, + mentionableAgentPubkeys, }) ) { return; @@ -431,6 +427,18 @@ export function useMentions( relayAgentsQuery.data, ]); + // Autocomplete admits in-channel agents we own even when the relay directory + // never marks them invocable (`respond_to: owner-only`). Downstream agent + // classification reads this set, so it has to admit them too — otherwise an + // offered agent stops counting as one the moment it is addressed. + const knownAgentPubkeys = React.useMemo(() => { + const owned = getOwnedMemberAgentPubkeys(mentionCandidates, currentPubkey); + if (owned.size === 0) { + return mentionableAgentPubkeys; + } + return new Set([...mentionableAgentPubkeys, ...owned]); + }, [currentPubkey, mentionCandidates, mentionableAgentPubkeys]); + const mentionCandidatesWithTeams = React.useMemo( () => [ ...mentionCandidates, diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index 5e31235a18..a430f3abdd 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -30,6 +30,13 @@ const PROFILE_ONLY_AGENT_PUBKEY = "8f83d6b7f3d74f7d933ae3a54dd8c6cc85c7f98e531c16e5a827b953441a8d67"; const OWNED_AGENT_PROFILE_PUBKEY = "1212121212121212121212121212121212121212121212121212121212121212"; +/** + * Relay-classified agent whose declared NIP-OA owner is the mock viewer, a + * member of #agents, and deliberately absent from `managedAgents`. Mirrors an + * agent whose process runs on another machine. + */ +const OWNED_RELAY_AGENT_PUBKEY = + "a1b2c3d4e5f60718293a4b5c6d7e8f90112233445566778899aabbccddeeff00"; const SYSTEM_MESSAGE_KIND = 40099; const DM_THREAD_AGENT_MENTION_ERROR_TEXT = "Agents must already be in a DM to be mentioned in its threads. Start a new conversation that includes the agent."; @@ -849,6 +856,39 @@ test("relay-only agents stay hidden from channel mentions even when allowlisted" await expect(autocomplete(page)).toHaveCount(0); }); +test("owned in-channel agents managed on another machine stay mentionable", async ({ + page, +}) => { + // Regression for #2508 / #3739 / #2349. `nadia` is a channel member, is + // owned by the viewer, and is NOT in `managedAgents`: her process runs + // elsewhere. Her kind:10100 entry keeps the default `owner-only` policy, so + // she never enters `mentionableAgentPubkeys` either. Both autocomplete gates + // used to drop her, leaving her unmentionable from every machine except the + // one hosting her. + await installMockBridge(page, { + relayAgents: [ + { + pubkey: OWNED_RELAY_AGENT_PUBKEY, + name: "nadia", + channelNames: ["agents"], + respondTo: "owner-only", + }, + ], + }); + await page.goto("/"); + await page.getByTestId("channel-agents").click(); + await expect(page.getByTestId("chat-title")).toHaveText("agents"); + + const input = page.getByTestId("message-input"); + await input.fill("@nadia"); + + const dropdown = autocomplete(page); + await expect(dropdown).toBeVisible(); + const nadiaRow = dropdown.locator("button", { hasText: "nadia" }); + await expect(nadiaRow).toHaveCount(1); + await expect(nadiaRow.getByTestId("mention-agent-icon")).toBeVisible(); +}); + test("mentioning an in-channel stopped managed agent starts it before sending", async ({ page, }) => {