From 27e0f11c4d2968788130fb8ebfe41e144d3e6e01 Mon Sep 17 00:00:00 2001 From: Ivan Itzcovich Date: Wed, 29 Jul 2026 02:11:44 +0000 Subject: [PATCH] fix(desktop,cli): let mentioned agents pass real eligibility, close CLI respond_to gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buzz#2987 point 1 (the "gate" + its CLI companion), from the external-agents diagnostic report. Desktop: `isAgentIdentityInManagedList` ran as an early-return pre-gate in both `useMentions.ts` and `MembersSidebar.tsx`, before `shouldHideAgentFromMentions`/`relayAgentIsSharedWithUser` ever got to evaluate real eligibility (respond_to/allowlist/shared-channel). `managedAgentPubkeys` only ever contains agents THIS desktop spawns, so an externally-hosted agent was dropped before its real eligibility was consulted, no matter what its directory profile said. Removed the pre-gate from `useMentions.ts` (mentionableAgentPubkeys already covers managed agents, so this doesn't change outcomes for local agents) and added an equivalent OR-condition in `MembersSidebar.tsx`'s add-member picker. Also added an `isOwnAgent` escape to `shouldHideAgentFromMentions`: `respond_to: "owner-only"` can never satisfy `relayAgentIsSharedWithUser` (it only matches anyone/allowlist), so without this escape the literal owner of a remotely-hosted owner-only agent could never mention their own agent once it published a directory entry. Extracted the ownership check (candidate.ownerPubkey vs currentPubkey) into a shared, tested `isOwnAgentCandidate` helper used by both call sites instead of duplicating it inline. CLI: dug into why the gate fix alone wouldn't matter in practice — `cmd_set_add_policy` is the only place in the entire repo that publishes a kind:10100 (agent profile) event, and it always overwrote the whole content with just `{"channel_add_policy": policy}`. Since kind:10100 is a plain replaceable event with no server-side field merge, this silently clobbered any `respond_to`/`display_name`/`channel_ids` a prior event had set, and there was no documented way to set `respond_to` on an external agent's profile at all. `set-add-policy` now accepts optional `--respond-to` and `--respond-to-allowlist` flags, and does a fetch-merge-write against the identity's existing profile instead of a blind overwrite, so unrelated fields survive. Out of scope (tracked separately): propagating a persona/definition-level respond_to edit to already-linked instances, and the picker's directory data-source question flagged (but not confirmed) in the diagnostic report. Signed-off-by: Ivan Itzcovich --- crates/buzz-cli/src/commands/channels.rs | 262 +++++++++++++++--- crates/buzz-cli/src/lib.rs | 10 +- .../lib/agentAutocompleteEligibility.test.mjs | 42 +++ .../lib/agentAutocompleteEligibility.ts | 22 ++ .../features/channels/ui/MembersSidebar.tsx | 25 +- .../src/features/messages/lib/useMentions.ts | 10 +- 6 files changed, 332 insertions(+), 39 deletions(-) diff --git a/crates/buzz-cli/src/commands/channels.rs b/crates/buzz-cli/src/commands/channels.rs index 42844bf1e0..376872906f 100644 --- a/crates/buzz-cli/src/commands/channels.rs +++ b/crates/buzz-cli/src/commands/channels.rs @@ -1001,38 +1001,129 @@ pub async fn cmd_remove_channel_member( Ok(()) } -/// Set the channel addition policy — sign and submit a kind:10100 (agent profile) event. -pub async fn cmd_set_add_policy(client: &BuzzClient, policy: &str) -> Result<(), CliError> { - match policy { - "anyone" | "owner_only" | "nobody" => {} - _ => { - return Err(CliError::Usage(format!( - "--policy must be 'anyone', 'owner_only', or 'nobody' (got: {policy})" - ))) +/// Merge new agent-profile fields into an existing kind:10100 content blob. +/// +/// `kind:10100` is a plain replaceable event — the relay keeps only the +/// latest per pubkey, with no server-side field merge. A client that +/// publishes a partial object therefore clobbers every field the previous +/// event carried. This merges onto the fetched prior content instead of +/// replacing it outright (buzz#2987: `set-add-policy` was the only +/// documented publisher of this event and always overwrote the whole +/// content with just `channel_add_policy`, silently discarding any +/// `respond_to`/`display_name`/`channel_ids` set by another mechanism). +fn merge_agent_profile_content( + existing_content: Option<&str>, + channel_add_policy: Option<&str>, + respond_to: Option<&str>, + respond_to_allowlist: Option<&[String]>, +) -> String { + let mut obj = existing_content + .and_then(|raw| serde_json::from_str::(raw).ok()) + .and_then(|value| value.as_object().cloned()) + .unwrap_or_default(); + + if let Some(policy) = channel_add_policy { + obj.insert("channel_add_policy".to_string(), serde_json::json!(policy)); + } + if let Some(mode) = respond_to { + obj.insert("respond_to".to_string(), serde_json::json!(mode)); + } + if let Some(allowlist) = respond_to_allowlist { + obj.insert( + "respond_to_allowlist".to_string(), + serde_json::json!(allowlist), + ); + } + + serde_json::Value::Object(obj).to_string() +} + +/// Fetch the current identity's own kind:10100 (agent profile) content, if any. +async fn fetch_own_agent_profile_content(client: &BuzzClient) -> Result, CliError> { + let filter = serde_json::json!({ + "kinds": [buzz_sdk::kind::KIND_AGENT_PROFILE], + "authors": [client.keys().public_key().to_hex()], + "limit": 1, + }); + let raw = client.query(&filter).await?; + let mut events: Vec = serde_json::from_str(&raw) + .map_err(|e| CliError::Other(format!("failed to parse relay response: {e}")))?; + events.sort_by_key(|event| std::cmp::Reverse(event.created_at)); + Ok(events.into_iter().next().map(|event| event.content)) +} + +/// Set the channel addition policy and/or the agent respond-to gate — sign +/// and submit a kind:10100 (agent profile) event, merged onto any existing +/// profile so unrelated fields survive (see [`merge_agent_profile_content`]). +pub async fn cmd_set_add_policy( + client: &BuzzClient, + policy: Option<&str>, + respond_to: Option<&str>, + respond_to_allowlist: Option<&[String]>, +) -> Result<(), CliError> { + if policy.is_none() && respond_to.is_none() && respond_to_allowlist.is_none() { + return Err(CliError::Usage( + "at least one of --policy, --respond-to, or --respond-to-allowlist is required" + .to_string(), + )); + } + + if let Some(policy) = policy { + match policy { + "anyone" | "owner_only" | "nobody" => {} + _ => { + return Err(CliError::Usage(format!( + "--policy must be 'anyone', 'owner_only', or 'nobody' (got: {policy})" + ))) + } + } + + // Check if this policy is allowed by the deployment. + // NOTE: This gate covers only the `buzz channels set-add-policy` CLI path. + // A client that submits a kind:10100 event directly to the relay bypasses + // this check. Full enforcement requires relay-side validation, which is + // intentionally out of scope for this change (see team decision: no + // relay-side enforcement of client behavior). + if let Ok(allowed_raw) = std::env::var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES") { + let allowed: Vec<&str> = allowed_raw + .split(',') + .map(str::trim) + .filter(|s| !s.is_empty()) + .collect(); + if !allowed.is_empty() && !allowed.contains(&policy) { + return Err(CliError::Usage(format!( + "channel_add_policy '{policy}' is not permitted on this deployment \ + (BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES={allowed_raw})" + ))); + } } } - // Check if this policy is allowed by the deployment. - // NOTE: This gate covers only the `buzz channels set-add-policy` CLI path. - // A client that submits a kind:10100 event directly to the relay bypasses - // this check. Full enforcement requires relay-side validation, which is - // intentionally out of scope for this change (see team decision: no - // relay-side enforcement of client behavior). - if let Ok(allowed_raw) = std::env::var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES") { - let allowed: Vec<&str> = allowed_raw - .split(',') - .map(str::trim) - .filter(|s| !s.is_empty()) - .collect(); - if !allowed.is_empty() && !allowed.contains(&policy) { - return Err(CliError::Usage(format!( - "channel_add_policy '{policy}' is not permitted on this deployment \ - (BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES={allowed_raw})" - ))); + if let Some(mode) = respond_to { + match mode { + "owner-only" | "anyone" | "allowlist" => {} + _ => { + return Err(CliError::Usage(format!( + "--respond-to must be 'owner-only', 'anyone', or 'allowlist' (got: {mode})" + ))) + } } } - let content = serde_json::json!({ "channel_add_policy": policy }).to_string(); + if let Some(allowlist) = respond_to_allowlist { + for pubkey in allowlist { + validate_hex64(pubkey)?; + } + } + + let existing_content = fetch_own_agent_profile_content(client).await?; + let content = merge_agent_profile_content( + existing_content.as_deref(), + policy, + respond_to, + respond_to_allowlist, + ); + use nostr::{EventBuilder, Kind}; let builder = EventBuilder::new( Kind::Custom(buzz_sdk::kind::KIND_AGENT_PROFILE as u16), @@ -1161,7 +1252,19 @@ pub async fn dispatch( ChannelsCmd::RemoveMember { channel, pubkey } => { cmd_remove_channel_member(client, &channel, &pubkey).await } - ChannelsCmd::SetAddPolicy { policy } => cmd_set_add_policy(client, &policy).await, + ChannelsCmd::SetAddPolicy { + policy, + respond_to, + respond_to_allowlist, + } => { + cmd_set_add_policy( + client, + policy.as_deref(), + respond_to.as_deref(), + respond_to_allowlist.as_deref(), + ) + .await + } } } @@ -1177,9 +1280,9 @@ pub async fn dispatch_canvas(cmd: crate::CanvasCmd, client: &BuzzClient) -> Resu mod tests { use super::{ apply_cardinality_rule, build_template_report, cmd_set_add_policy, - finalize_roster_resolution, name_matches, resolve_roster_with_archive_filter, - validate_ttl_seconds, ArchivedExclusion, ChannelSummary, ResolvedAgent, RosterResolution, - SkippedSlug, + finalize_roster_resolution, merge_agent_profile_content, name_matches, + resolve_roster_with_archive_filter, validate_ttl_seconds, ArchivedExclusion, + ChannelSummary, ResolvedAgent, RosterResolution, SkippedSlug, }; use crate::client::BuzzClient; use crate::CliError; @@ -1362,7 +1465,7 @@ mod tests { async fn set_add_policy_env_gate_rejects_disallowed_via_full_path() { std::env::set_var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES", "owner_only,nobody"); let client = make_test_client(); - let result = cmd_set_add_policy(&client, "anyone").await; + let result = cmd_set_add_policy(&client, Some("anyone"), None, None).await; std::env::remove_var("BUZZ_ACP_ALLOWED_CHANNEL_ADD_POLICIES"); assert!( @@ -1380,6 +1483,103 @@ mod tests { } } + #[tokio::test] + async fn set_add_policy_requires_at_least_one_field() { + let client = make_test_client(); + let result = cmd_set_add_policy(&client, None, None, None).await; + + match result.unwrap_err() { + crate::CliError::Usage(msg) => { + assert!( + msg.contains("at least one of"), + "error should ask for at least one field: {msg}" + ); + } + other => panic!("expected CliError::Usage, got {other:?}"), + } + } + + #[tokio::test] + async fn set_add_policy_rejects_invalid_respond_to_before_any_network_call() { + let client = make_test_client(); + let result = cmd_set_add_policy(&client, None, Some("everyone"), None).await; + + match result.unwrap_err() { + crate::CliError::Usage(msg) => { + assert!( + msg.contains("--respond-to"), + "error should name the offending flag: {msg}" + ); + } + other => panic!("expected CliError::Usage, got {other:?}"), + } + } + + #[tokio::test] + async fn set_add_policy_rejects_malformed_allowlist_pubkey_before_any_network_call() { + let client = make_test_client(); + let result = + cmd_set_add_policy(&client, None, None, Some(&["not-a-pubkey".to_string()])).await; + + assert!(result.is_err(), "malformed pubkey should be rejected"); + } + + // --- merge_agent_profile_content (buzz#2987: read-modify-write, not clobber) --- + + #[test] + fn merge_agent_profile_content_preserves_unrelated_fields() { + let existing = r#"{"display_name":"Scout","channel_ids":["a-b-c"]}"#; + let merged = merge_agent_profile_content(Some(existing), None, Some("anyone"), None); + let parsed: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(parsed["display_name"], "Scout"); + assert_eq!(parsed["channel_ids"], serde_json::json!(["a-b-c"])); + assert_eq!(parsed["respond_to"], "anyone"); + } + + #[test] + fn merge_agent_profile_content_overwrites_only_the_fields_given() { + let existing = r#"{"respond_to":"owner-only","respond_to_allowlist":[],"channel_add_policy":"nobody"}"#; + let merged = merge_agent_profile_content(Some(existing), Some("anyone"), None, None); + let parsed: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(parsed["channel_add_policy"], "anyone"); + // respond_to untouched — this call only updated channel_add_policy. + assert_eq!(parsed["respond_to"], "owner-only"); + } + + #[test] + fn merge_agent_profile_content_with_no_existing_profile_starts_from_empty() { + let merged = merge_agent_profile_content(None, Some("owner_only"), None, None); + let parsed: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(parsed["channel_add_policy"], "owner_only"); + assert!(parsed.get("respond_to").is_none()); + } + + #[test] + fn merge_agent_profile_content_sets_respond_to_allowlist() { + let allowlist = vec!["a".repeat(64)]; + let merged = merge_agent_profile_content(None, None, Some("allowlist"), Some(&allowlist)); + let parsed: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(parsed["respond_to"], "allowlist"); + assert_eq!( + parsed["respond_to_allowlist"], + serde_json::json!(["a".repeat(64)]) + ); + } + + #[test] + fn merge_agent_profile_content_ignores_non_object_existing_content() { + // Defensive: a corrupt or unexpected prior event (e.g. a bare JSON + // array) must not panic — fall back to an empty profile. + let merged = merge_agent_profile_content(Some("[1,2,3]"), Some("anyone"), None, None); + let parsed: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(parsed["channel_add_policy"], "anyone"); + } + // --- Template roster cardinality (F4) --- fn agent(persona_id: &str, pubkey: &str) -> ResolvedAgent { diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index 0b46734584..a20adccd72 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -666,12 +666,18 @@ pub enum ChannelsCmd { #[arg(long)] pubkey: String, }, - /// Set your channel addition policy + /// Set your channel addition policy and/or agent respond-to gate #[command(name = "set-add-policy")] SetAddPolicy { /// Policy: anyone | owner_only | nobody #[arg(long)] - policy: String, + policy: Option, + /// Who can @mention/address this agent: owner-only | anyone | allowlist + #[arg(long = "respond-to")] + respond_to: Option, + /// Comma-separated allowlist of 64-char hex pubkeys (used with --respond-to allowlist) + #[arg(long = "respond-to-allowlist", value_delimiter = ',')] + respond_to_allowlist: Option>, }, } diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs index 4e02b7bd68..8ad28bb013 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.test.mjs @@ -243,6 +243,48 @@ test("shouldHideAgentFromMentions: normalizes the pubkey before lookup", () => { ); }); +test("shouldHideAgentFromMentions: shows an owner-only agent to its own owner (buzz#2987)", () => { + assert.equal( + shouldHideAgentFromMentions({ + isAgent: true, + isMember: false, + isOwnAgent: true, + pubkey: PUB_A, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set([PUB_A]), + }), + false, + ); +}); + +test("shouldHideAgentFromMentions: isOwnAgent does not override the non-agent guard", () => { + assert.equal( + shouldHideAgentFromMentions({ + isAgent: false, + isMember: false, + isOwnAgent: true, + pubkey: PUB_A, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set([PUB_A]), + }), + false, + ); +}); + +test("shouldHideAgentFromMentions: still hides a non-owned non-invocable agent", () => { + assert.equal( + shouldHideAgentFromMentions({ + isAgent: true, + isMember: false, + isOwnAgent: false, + pubkey: PUB_A, + mentionableAgentPubkeys: new Set(), + directoryAgentPubkeys: new Set([PUB_A]), + }), + true, + ); +}); + test("coalesceAgentAutocompleteCandidates: merges agents with the same persona id", () => { const first = makeAgent({ pubkey: PUB_A, personaId: "pinky" }); const second = makeAgent({ diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts index e4afe7fea4..51fc00ee98 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts @@ -64,20 +64,42 @@ export function isAgentIdentityInManagedList( ); } +export function isOwnAgentCandidate( + candidate: { ownerPubkey?: string | null }, + currentPubkey: string | null | undefined, +) { + return ( + candidate.ownerPubkey != null && + currentPubkey != null && + normalizePubkey(candidate.ownerPubkey) === normalizePubkey(currentPubkey) + ); +} + export function shouldHideAgentFromMentions({ isAgent, isMember, + isOwnAgent = false, pubkey, mentionableAgentPubkeys, directoryAgentPubkeys, }: { isAgent: boolean; isMember: boolean; + /** + * Cryptographically verified (NIP-OA) ownership: the candidate's kind:0 + * `auth` tag names the current user as owner. `respond_to: "owner-only"` + * can never satisfy `relayAgentIsSharedWithUser` (it only matches + * `anyone`/`allowlist`), so without this escape the literal owner of a + * remotely-hosted owner-only agent could never mention their own agent + * once it published a directory entry (buzz#2987). + */ + isOwnAgent?: boolean; pubkey: string; mentionableAgentPubkeys: ReadonlySet; directoryAgentPubkeys: ReadonlySet; }) { if (!isAgent) return false; + if (isOwnAgent) return false; const normalized = normalizePubkey(pubkey); // Invocable => always show. if (mentionableAgentPubkeys.has(normalized)) return false; diff --git a/desktop/src/features/channels/ui/MembersSidebar.tsx b/desktop/src/features/channels/ui/MembersSidebar.tsx index c6349546a2..204a559bdd 100644 --- a/desktop/src/features/channels/ui/MembersSidebar.tsx +++ b/desktop/src/features/channels/ui/MembersSidebar.tsx @@ -5,11 +5,15 @@ import { invalidateChannelState, useAddChannelMembersMutation, useChannelMembersQuery, + useChannelsQuery, } from "@/features/channels/hooks"; import { attachManagedAgentToChannel } from "@/features/agents/channelAgents"; import { coalesceAgentAutocompleteCandidates, + getMentionableAgentPubkeys, + getSharedChannelIds, isAgentIdentityInManagedList, + isOwnAgentCandidate, } from "@/features/agents/lib/agentAutocompleteEligibility"; import { useIsArchivedPredicate } from "@/features/identity-archive/hooks"; import { useClassifiedMembers } from "@/features/channels/lib/useClassifiedMembers"; @@ -253,6 +257,11 @@ export function MembersSidebar({ }); const userSearchResults = useFlattenedUserSearchResults(userSearchQuery.data); const isArchivedDiscovery = useIsArchivedPredicate(); + const channelsQuery = useChannelsQuery(); + const sharedChannelIds = React.useMemo( + () => getSharedChannelIds(channelsQuery.data), + [channelsQuery.data], + ); const addSearchResults = React.useMemo(() => { if (!canAddMembers || normalizedDeferredSearchQuery.length === 0) { return []; @@ -272,6 +281,17 @@ export function MembersSidebar({ .filter((label): label is string => Boolean(label)), ); const managedAgentPubkeys = new Set(managedAgentsByPubkey.keys()); + // Directory-eligible external agents (respond_to anyone/allowlist, + // verified via a shared channel or the allowlist) count alongside the + // local managed list — otherwise an externally-hosted agent can never + // be added to a channel from this picker (buzz#2987). `isAgentIdentityInManagedList` + // alone only recognizes agents THIS desktop spawns. + const mentionableAgentPubkeys = getMentionableAgentPubkeys({ + currentPubkey, + managedAgentPubkeys, + relayAgents: relayAgentsQuery.data, + sharedChannelIds, + }); const addCandidate = (candidate: AddMemberSearchCandidate) => { const pubkey = normalizePubkey(candidate.pubkey); @@ -282,7 +302,9 @@ export function MembersSidebar({ )) || memberPubkeys.has(pubkey) || isArchivedDiscovery(pubkey) || - !isAgentIdentityInManagedList(candidate, managedAgentPubkeys) + (!isAgentIdentityInManagedList(candidate, managedAgentPubkeys) && + !mentionableAgentPubkeys.has(pubkey) && + !isOwnAgentCandidate(candidate, currentPubkey)) ) { return; } @@ -367,6 +389,7 @@ export function MembersSidebar({ memberPubkeys, normalizedDeferredSearchQuery, relayAgentsQuery.data, + sharedChannelIds, userSearchResults, rawMembers, ]); diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 0c73b75339..6adaa4cdf6 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -16,7 +16,7 @@ import { coalesceAutocompleteCandidatesByKey, getMentionableAgentPubkeys, getSharedChannelIds, - isAgentIdentityInManagedList, + isOwnAgentCandidate, shouldHideAgentFromMentions, } from "@/features/agents/lib/agentAutocompleteEligibility"; import { @@ -246,13 +246,14 @@ export function useMentions( if (isArchivedDiscovery(pubkey)) { return; } - if (!isAgentIdentityInManagedList(candidate, managedAgentPubkeys)) { - return; - } + // Dropped the local-managed-list pre-gate here: it silently rejected + // externally-hosted agents before `shouldHideAgentFromMentions` (below) + // ever ran its real respond_to/allowlist/ownership check (buzz#2987). if ( shouldHideAgentFromMentions({ isAgent: candidate.isAgent === true, isMember: candidate.isMember === true, + isOwnAgent: isOwnAgentCandidate(candidate, currentPubkey), pubkey, mentionableAgentPubkeys, directoryAgentPubkeys, @@ -420,7 +421,6 @@ export function useMentions( managedAgentNamesByPubkey, managedAgentPersonaIds, managedAgentPersonaIdsByPubkey, - managedAgentPubkeys, managedAgentsQuery.data, memberPubkeys, members,