Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import test from "node:test";

import {
coalesceAgentAutocompleteCandidates,
filterAdmittedMentionPubkeys,
filterCachedAgentSuggestions,
getAgentMentionAdmission,
getMentionableAgentPubkeys,
getSharedChannelIds,
isAgentIdentityInAllowedList,
Expand Down Expand Up @@ -307,7 +309,7 @@ test("shouldHideAgentFromMentions: hides member agents with an explicit not-invo
);
});

test("shouldHideAgentFromMentions: shows member agents with unknown invocability (not in directory)", () => {
test("shouldHideAgentFromMentions: hides member agents without an affirmative directory grant", () => {
assert.equal(
shouldHideAgentFromMentions({
isAgent: true,
Expand All @@ -316,10 +318,66 @@ test("shouldHideAgentFromMentions: shows member agents with unknown invocability
mentionableAgentPubkeys: new Set(),
directoryAgentPubkeys: new Set(),
}),
true,
);
});

test("shouldHideAgentFromMentions: hides unknown member agents while directories load", () => {
assert.equal(
shouldHideAgentFromMentions({
isAgent: true,
isMember: true,
pubkey: PUB_A,
mentionableAgentPubkeys: new Set(),
directoryAgentPubkeys: new Set(),
directoryReady: false,
}),
true,
);
});

test("shouldHideAgentFromMentions: hides mentionable member agents while directories load", () => {
assert.equal(
shouldHideAgentFromMentions({
isAgent: true,
isMember: true,
pubkey: PUB_A,
mentionableAgentPubkeys: new Set([PUB_A]),
directoryAgentPubkeys: new Set(),
directoryReady: false,
}),
true,
);
});

test("shouldHideAgentFromMentions: shows non-agent members while directories load", () => {
assert.equal(
shouldHideAgentFromMentions({
isAgent: false,
isMember: true,
pubkey: PUB_A,
mentionableAgentPubkeys: new Set(),
directoryAgentPubkeys: new Set([PUB_A]),
directoryReady: false,
}),
false,
);
});

test("shouldHideAgentFromMentions: hides unknown member agents after empty directories settle", () => {
assert.equal(
shouldHideAgentFromMentions({
isAgent: true,
isMember: true,
pubkey: PUB_A,
mentionableAgentPubkeys: new Set(),
directoryAgentPubkeys: new Set(),
directoryReady: true,
}),
true,
);
});

test("shouldHideAgentFromMentions: normalizes the pubkey before lookup", () => {
const mixedCase = "Ab".repeat(32);
const normalized = mixedCase.toLowerCase();
Expand All @@ -336,6 +394,58 @@ test("shouldHideAgentFromMentions: normalizes the pubkey before lookup", () => {
);
});

test("getAgentMentionAdmission: owner-only requires current verified ownership", () => {
const common = {
isAgent: true,
isManagedAgent: false,
pubkey: PUB_A,
currentPubkey: CURRENT_PUBKEY,
mentionableAgentPubkeys: new Set([PUB_A]),
directoryReady: true,
ownerOnly: true,
};

assert.equal(
getAgentMentionAdmission({ ...common, ownerPubkey: CURRENT_PUBKEY }),
"allow",
);
assert.equal(
getAgentMentionAdmission({ ...common, ownerPubkey: OTHER_OWNER_PUBKEY }),
"deny",
);
assert.equal(
getAgentMentionAdmission({ ...common, ownerPubkey: null }),
"unknown",
);
});

test("getAgentMentionAdmission: unresolved directory state stays unknown", () => {
assert.equal(
getAgentMentionAdmission({
isAgent: true,
isManagedAgent: false,
pubkey: PUB_A,
currentPubkey: CURRENT_PUBKEY,
ownerPubkey: CURRENT_PUBKEY,
mentionableAgentPubkeys: new Set([PUB_A]),
directoryReady: false,
ownerOnly: false,
}),
"unknown",
);
});

test("filterAdmittedMentionPubkeys: rechecks agent admission without dropping people", () => {
assert.deepEqual(
filterAdmittedMentionPubkeys(
[PUB_A, PUB_B, PUB_C],
new Set([PUB_A, PUB_B]),
new Set([PUB_B]),
),
[PUB_B, PUB_C],
);
});

test("coalesceAgentAutocompleteCandidates: keeps agents with the same persona id distinct", () => {
const first = makeAgent({ pubkey: PUB_A, personaId: "pinky" });
const second = makeAgent({
Expand Down
145 changes: 123 additions & 22 deletions desktop/src/features/agents/lib/agentAutocompleteEligibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,37 +92,138 @@ export function isAgentIdentityInAllowedList(
);
}

export type AgentMentionAdmission = "allow" | "deny" | "unknown";

export function getAgentMentionAdmission({
isAgent,
isManagedAgent,
pubkey,
ownerPubkey,
currentPubkey,
mentionableAgentPubkeys,
directoryReady,
ownerOnly,
}: {
isAgent: boolean;
isManagedAgent: boolean;
pubkey: string;
ownerPubkey?: string | null;
currentPubkey?: string | null;
mentionableAgentPubkeys: ReadonlySet<string>;
directoryReady: boolean;
ownerOnly: boolean | undefined;
}): AgentMentionAdmission {
if (!isAgent) return "allow";
if (!directoryReady || ownerOnly === undefined) return "unknown";

const normalized = normalizePubkey(pubkey);
if (!mentionableAgentPubkeys.has(normalized)) return "deny";
if (!ownerOnly || isManagedAgent) return "allow";
if (!ownerPubkey || !currentPubkey) return "unknown";

return normalizePubkey(ownerPubkey) === normalizePubkey(currentPubkey)
? "allow"
: "deny";
}

export function shouldHideAgentFromMentions({
isAgent,
isMember,
isManagedAgent = false,
pubkey,
ownerPubkey,
currentPubkey,
mentionableAgentPubkeys,
directoryAgentPubkeys,
directoryReady = true,
ownerOnly = false,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 ownerOnly = false here swallows the undefined that useMentions passes while the owner-policy query is still loading (ownerOnly: agentAccessOwnerOnlyQuery.data), so the "unknown" branch getAgentMentionAdmission has for exactly that state is unreachable through this wrapper. During the window where the agent directories are ready but the owner policy isn't, an other-owned relay agent is admitted into autocomplete in an owner-only build. Send-time revalidation strips it on the chat/edit paths (it checks ownerOnly === undefined explicitly), which keeps this non-blocking — but it contradicts the fail-closed intent, and both reviews flagged it independently. Fix: drop the destructuring default and pass undefined through, or fold ownerPolicyReady into the directoryReady value this call site receives.

}: {
isAgent: boolean;
isMember: boolean;
isManagedAgent?: boolean;
pubkey: string;
ownerPubkey?: string | null;
currentPubkey?: string | null;
mentionableAgentPubkeys: ReadonlySet<string>;
directoryAgentPubkeys: ReadonlySet<string>;
directoryReady?: boolean;
ownerOnly?: boolean;
}) {
if (!isAgent) return false;
const normalized = normalizePubkey(pubkey);
// Invocable => always show.
if (mentionableAgentPubkeys.has(normalized)) return false;
// Non-member, non-invocable => hide (preserves prior behavior).
if (!isMember) return true;
// 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.
//
// NOTE: this assumes `directoryAgentPubkeys` and `mentionableAgentPubkeys`
// share the same source query (`relayAgentsQuery.data`), so directory
// presence without membership in `mentionableAgentPubkeys` is a real
// explicit-exclusion signal. If a future change sources the directory set
// from a different query, an agent that's directory-present but whose
// mentionability is still loading could be hidden prematurely — keep the
// two sets derived from the same query.
return directoryAgentPubkeys.has(normalized);
return (
getAgentMentionAdmission({
isAgent,
isManagedAgent,
pubkey,
ownerPubkey,
currentPubkey,
mentionableAgentPubkeys,
directoryReady,
ownerOnly,
}) !== "allow"
);
}

export function getAgentIdentityPubkeys({
managedAgentPubkeys,
relayAgents,
members,
profileIsAgent,
}: {
managedAgentPubkeys: ReadonlySet<string>;
relayAgents: readonly { pubkey: string }[];
members: readonly {
pubkey: string;
isAgent?: boolean;
role?: string | null;
}[];
profileIsAgent: (pubkey: string) => boolean;
}) {
return new Set([
...managedAgentPubkeys,
...relayAgents.map(({ pubkey }) => normalizePubkey(pubkey)),
...members
.filter(
(member) =>
member.isAgent === true ||
member.role === "bot" ||
profileIsAgent(normalizePubkey(member.pubkey)),
)
.map(({ pubkey }) => normalizePubkey(pubkey)),
]);
}

export function getAdmittedAgentPubkeys(
candidates: readonly { pubkey?: string; isAgent?: boolean }[],
) {
return new Set(
candidates.flatMap((candidate) =>
candidate.isAgent && candidate.pubkey
? [normalizePubkey(candidate.pubkey)]
: [],
),
);
}

export function rememberSelectedAgentPubkeys(
target: Set<string>,
selected: readonly { pubkey?: string; isAgent?: boolean }[],
selectionIsAgent: boolean,
) {
for (const candidate of selected) {
if (candidate.pubkey && (selectionIsAgent || candidate.isAgent === true)) {
target.add(normalizePubkey(candidate.pubkey));
}
}
}

export function filterAdmittedMentionPubkeys(
pubkeys: readonly string[],
agentIdentityPubkeys: ReadonlySet<string>,
admittedAgentPubkeys: ReadonlySet<string>,
) {
return pubkeys.filter((pubkey) => {
const normalized = normalizePubkey(pubkey);
return (
!agentIdentityPubkeys.has(normalized) ||
admittedAgentPubkeys.has(normalized)
);
});
}

export function isAgentMentionChannelType(type?: string | null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import assert from "node:assert/strict";
import test from "node:test";

import { revalidateAgentMentionPubkeys } from "./agentMentionRevalidation.ts";

const CURRENT = "a".repeat(64);
const AGENT = "b".repeat(64);
const HUMAN = "c".repeat(64);
const OTHER_OWNER = "d".repeat(64);

function options(refetchOwnerProfiles) {
return {
pubkeys: [HUMAN, AGENT],
agentPubkeys: new Set([AGENT]),
currentPubkey: CURRENT,
eligibilityScope: { type: "channel", channelId: "general" },
sharedChannelIds: new Set(["general"]),
ownerOnly: true,
ownerPolicyError: null,
refetchManagedAgents: async () => ({ data: [], error: null }),
refetchRelayAgents: async () => ({
data: [
{
pubkey: AGENT,
respondTo: "anyone",
respondToAllowlist: [],
channelIds: ["general"],
},
],
error: null,
}),
refetchOwnerProfiles,
};
}

test("owner-only revalidation admits an agent only from a fresh same-owner proof", async () => {
const requested = [];
const result = await revalidateAgentMentionPubkeys(
options(async (pubkeys) => {
requested.push(...pubkeys);
return {
profiles: { [AGENT]: { ownerPubkey: CURRENT } },
missing: [],
};
}),
);

assert.deepEqual(requested, [AGENT]);
assert.deepEqual(result, [HUMAN, AGENT]);
});

for (const [name, refetchOwnerProfiles] of [
["revoked owner proof", async () => ({ profiles: {}, missing: [AGENT] })],
[
"changed owner proof",
async () => ({
profiles: { [AGENT]: { ownerPubkey: OTHER_OWNER } },
missing: [],
}),
],
[
"owner profile query error",
async () => {
throw new Error("relay unavailable");
},
],
]) {
test(`owner-only revalidation fails closed on ${name}`, async () => {
assert.deepEqual(
await revalidateAgentMentionPubkeys(options(refetchOwnerProfiles)),
[HUMAN],
);
});
}
Loading
Loading