diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index efcfd3cd97..1161dc1034 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -269,16 +269,16 @@ Start with **N=2** for most deployments. Increase if queue depth grows under loa ## Forum Channels -By default, the ACP harness subscribes to stream message kinds (9, 46010, 40007). To receive forum events, opt in with `--kinds` and disable the mention filter (forum posts don't @mention agents): +By default, the ACP harness subscribes to actionable stream kinds (9 messages, 40003 edits that add a mention, 46010 workflow approvals, and 40007 reminders). To receive forum events, opt in with `--kinds` and disable the mention filter (forum posts don't @mention agents): **CLI flags:** ```bash -buzz-acp --kinds 9,46010,40007,45001,45002,45003 --no-mention-filter +buzz-acp --kinds 9,40003,46010,40007,45001,45002,45003 --no-mention-filter ``` **Or with `--subscribe all`:** ```bash -buzz-acp --subscribe all --kinds 9,46010,40007,45001,45002,45003 +buzz-acp --subscribe all --kinds 9,40003,46010,40007,45001,45002,45003 ``` **Per-channel config:** diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 7e31ab3ba6..24d59c2c93 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -6,6 +6,10 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; +use buzz_core::kind::{ + KIND_STREAM_MESSAGE, KIND_STREAM_MESSAGE_EDIT, KIND_STREAM_REMINDER, + KIND_WORKFLOW_APPROVAL_REQUESTED, +}; use clap::Parser; use clap::ValueEnum; use nostr::Keys; @@ -1333,16 +1337,26 @@ pub fn load_rules(path: &std::path::Path) -> Result, Confi Ok(config.rules) } +/// Event kinds that carry actionable direct mentions by default. +/// +/// Message edits are included because Desktop emits `p` tags only for +/// recipients newly added by an edit. Receiving kind 40003 therefore wakes an +/// agent once for a newly added mention without re-waking it for ordinary edits. +pub(crate) fn default_mention_kinds() -> Vec { + vec![ + KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_EDIT, + KIND_WORKFLOW_APPROVAL_REQUESTED, + KIND_STREAM_REMINDER, + ] +} + /// Resolve per-channel NIP-01 filters from config + discovered channels. pub fn resolve_channel_filters( config: &Config, discovered_channels: &[Uuid], rules: &[SubscriptionRule], ) -> HashMap { - use buzz_core::kind::{ - KIND_STREAM_MESSAGE, KIND_STREAM_REMINDER, KIND_WORKFLOW_APPROVAL_REQUESTED, - }; - let target_channels: Vec = if let Some(ref overrides) = config.channels_override { overrides .iter() @@ -1357,13 +1371,10 @@ pub fn resolve_channel_filters( match config.subscribe_mode { SubscribeMode::Mentions => { - let kinds = config.kinds_override.clone().unwrap_or_else(|| { - vec![ - KIND_STREAM_MESSAGE, - KIND_WORKFLOW_APPROVAL_REQUESTED, - KIND_STREAM_REMINDER, - ] - }); + let kinds = config + .kinds_override + .clone() + .unwrap_or_else(default_mention_kinds); let require_mention = !config.no_mention_filter; for ch in &target_channels { result.insert( @@ -1441,10 +1452,6 @@ pub fn resolve_dynamic_channel_filter( channel_id: Uuid, rules: &[crate::filter::SubscriptionRule], ) -> Option { - use buzz_core::kind::{ - KIND_STREAM_MESSAGE, KIND_STREAM_REMINDER, KIND_WORKFLOW_APPROVAL_REQUESTED, - }; - // In Mentions/All mode, if the operator explicitly constrained channels // with --channels, only allow dynamic subscription to channels in that // allowlist. Config mode ignores --channels (per CLI contract) and uses @@ -1462,13 +1469,12 @@ pub fn resolve_dynamic_channel_filter( match config.subscribe_mode { SubscribeMode::Mentions => Some(ChannelFilter { - kinds: Some(config.kinds_override.clone().unwrap_or_else(|| { - vec![ - KIND_STREAM_MESSAGE, - KIND_WORKFLOW_APPROVAL_REQUESTED, - KIND_STREAM_REMINDER, - ] - })), + kinds: Some( + config + .kinds_override + .clone() + .unwrap_or_else(default_mention_kinds), + ), require_mention: !config.no_mention_filter, }), SubscribeMode::All => Some(ChannelFilter { @@ -1616,11 +1622,28 @@ mod tests { assert!(f.require_mention, "mentions mode requires mention"); let kinds = f.kinds.as_ref().expect("should have kinds"); assert!(kinds.contains(&buzz_core::kind::KIND_STREAM_MESSAGE)); + assert!(kinds.contains(&buzz_core::kind::KIND_STREAM_MESSAGE_EDIT)); assert!(kinds.contains(&buzz_core::kind::KIND_WORKFLOW_APPROVAL_REQUESTED)); assert!(kinds.contains(&buzz_core::kind::KIND_STREAM_REMINDER)); } } + #[test] + fn test_dynamic_mentions_mode_includes_message_edits() { + let config = test_config(SubscribeMode::Mentions); + let filter = resolve_dynamic_channel_filter(&config, Uuid::new_v4(), &[]) + .expect("dynamic channel should be subscribed"); + + assert!(filter.require_mention); + assert!( + filter + .kinds + .expect("mentions mode should constrain kinds") + .contains(&KIND_STREAM_MESSAGE_EDIT), + "newly mentioned agents must receive message edits on dynamic channels" + ); + } + #[test] fn test_mentions_mode_custom_kinds() { let mut config = test_config(SubscribeMode::Mentions); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 7e6403caf2..3f74500d71 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -35,7 +35,6 @@ use acp::{AcpClient, EnvVar, McpServer}; use anyhow::{ensure, Context, Result}; use buzz_core::kind::{ KIND_MEMBER_ADDED_NOTIFICATION, KIND_MEMBER_REMOVED_NOTIFICATION, KIND_STREAM_MESSAGE, - KIND_STREAM_REMINDER, KIND_WORKFLOW_APPROVAL_REQUESTED, }; use buzz_core::observer::{ decrypt_observer_payload, encrypt_observer_payload, OBSERVER_FRAME_TELEMETRY, @@ -2780,42 +2779,7 @@ async fn run_harness( tracing::info!("discovered {} channel(s)", channel_info_map.len()); let channel_ids: Vec = channel_info_map.keys().copied().collect(); - let rules: Vec = match config.subscribe_mode { - SubscribeMode::Mentions => { - vec![SubscriptionRule { - name: "mentions".into(), - channels: filter::ChannelScope::All("all".into()), - kinds: config.kinds_override.clone().unwrap_or_else(|| { - vec![ - KIND_STREAM_MESSAGE, - KIND_WORKFLOW_APPROVAL_REQUESTED, - KIND_STREAM_REMINDER, - ] - }), - require_mention: !config.no_mention_filter, - filter: None, - compiled_filter: None, - consecutive_timeouts: std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)), - prompt_tag: Some("@mention".into()), - }] - } - SubscribeMode::All => { - vec![SubscriptionRule { - name: "all".into(), - channels: filter::ChannelScope::All("all".into()), - kinds: config.kinds_override.clone().unwrap_or_default(), - require_mention: false, - filter: None, - compiled_filter: None, - consecutive_timeouts: std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)), - prompt_tag: Some("all".into()), - }] - } - SubscribeMode::Config => { - // load_rules() already warns if the config file has zero rules. - config::load_rules(&config.config_path)? - } - }; + let rules = startup_subscription_rules(config)?; let channel_filters = config::resolve_channel_filters(config, &channel_ids, &rules); if channel_filters.is_empty() { @@ -9912,6 +9876,79 @@ mod build_mcp_servers_tests { } } +/// Local admission rules for the normal listener. The relay subscription is +/// derived separately (`config::resolve_channel_filters`); an event must pass +/// both, so each default kind has to be present here too. +fn startup_subscription_rules(config: &Config) -> Result> { + let rules = match config.subscribe_mode { + SubscribeMode::Mentions => { + vec![SubscriptionRule { + name: "mentions".into(), + channels: filter::ChannelScope::All("all".into()), + kinds: config + .kinds_override + .clone() + .unwrap_or_else(config::default_mention_kinds), + require_mention: !config.no_mention_filter, + filter: None, + compiled_filter: None, + consecutive_timeouts: std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)), + prompt_tag: Some("@mention".into()), + }] + } + SubscribeMode::All => { + vec![SubscriptionRule { + name: "all".into(), + channels: filter::ChannelScope::All("all".into()), + kinds: config.kinds_override.clone().unwrap_or_default(), + require_mention: false, + filter: None, + compiled_filter: None, + consecutive_timeouts: std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)), + prompt_tag: Some("all".into()), + }] + } + SubscribeMode::Config => { + // load_rules() already warns if the config file has zero rules. + config::load_rules(&config.config_path)? + } + }; + Ok(rules) +} + +#[cfg(test)] +mod edit_mention_admission_tests { + use super::*; + use crate::edit_routing::test_support::edit_event; + + /// Default normal-mode admission: an edit that newly mentions the agent + /// matches the startup rule; an edit without the agent's `p` tag does not. + #[tokio::test] + async fn default_startup_rules_admit_only_edits_that_mention_the_agent() { + let mut config = build_mcp_servers_tests::test_config(); + config.subscribe_mode = SubscribeMode::Mentions; + let agent = config.keys.public_key().to_hex(); + let rules = startup_subscription_rules(&config).expect("mentions rules"); + let channel_id = Uuid::new_v4(); + let target = "ab".repeat(32); + + let mentioned = edit_event(&target, &[["p", agent.as_str()]]); + assert!( + filter::match_event(&mentioned, channel_id, &rules, &agent) + .await + .is_some(), + "a mention added by an edit must wake the agent" + ); + let unmentioned = edit_event(&target, &[]); + assert!( + filter::match_event(&unmentioned, channel_id, &rules, &agent) + .await + .is_none(), + "an edit that does not mention the agent must not wake it" + ); + } +} + #[cfg(test)] mod error_outcome_emission_tests { //! Pins the policy that error-class outcomes surface to the activity feed diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index ebf7e46dc4..5d08c8b2ac 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -590,7 +590,7 @@ fn build_setup_subscription_rules(config: &Config) -> Vec Vec { + let mut config = crate::build_mcp_servers_tests::test_config(); + config.kinds_override = None; + build_setup_subscription_rules(&config) + } + + /// The setup listener's full admission path (pre-gate plus the production + /// setup rules) admits an edit that newly mentions the agent and rejects + /// an edit that does not. + #[tokio::test] + async fn setup_rules_admit_only_edits_that_mention_the_agent() { + let agent = nostr::Keys::generate().public_key().to_hex(); + let rules = default_setup_rules(); + let channel_id = Uuid::new_v4(); + let target = "ab".repeat(32); + let admitted = |event: nostr::Event| { + let rules = &rules; + let agent = agent.clone(); + async move { + setup_listener_admits(&event, &agent) + && filter::match_event(&event, channel_id, rules, &agent) + .await + .is_some() + } + }; + + let mentioned = + crate::edit_routing::test_support::edit_event(&target, &[["p", agent.as_str()]]); + assert!( + admitted(mentioned).await, + "a mention added by an edit must reach the setup nudge" + ); + let unmentioned = crate::edit_routing::test_support::edit_event(&target, &[]); + assert!( + !admitted(unmentioned).await, + "an edit without the agent's p tag must not nudge" + ); + } + #[test] fn nudge_body_names_all_requirements() { let payload = SetupPayload { diff --git a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs index 3f0ac92a16..b8625a7692 100644 --- a/desktop/src/features/messages/ui/submitMessageEdit.test.mjs +++ b/desktop/src/features/messages/ui/submitMessageEdit.test.mjs @@ -86,6 +86,44 @@ test("edit save uses edit-target refs that resolve after edit-open", async () => }); }); +test("an unresolved recipient does not re-notify an unchanged agent mention", async () => { + const { buildEditMentionState } = await import("../lib/draftMentionRefs.ts"); + const agent = "c".repeat(64); + const originalContent = "@Agent @Missing User please chek"; + const editTarget = buildEditMentionState( + originalContent, + [ + ["p", agent], + ["p", UNRESOLVED_USER], + ], + { [agent]: { displayName: "Agent" } }, + (pubkey) => pubkey === agent, + ); + assert.deepEqual( + editTarget.mentionRefs.map((ref) => ref.pubkey), + [agent], + ); + assert.deepEqual(editTarget.unresolvedMentionPubkeys, [UNRESOLVED_USER]); + const calls = []; + await submitMessageEdit({ + ...baseOptions(async (_content, tags, notifying) => { + calls.push(["save", tags, notifying]); + }), + content: "@Agent @Missing User please check", + originalContent, + editTarget, + extractMentionPubkeys: (text) => (text.includes("@Agent") ? [agent] : []), + revalidateMentionPubkeys: async (pubkeys) => { + calls.push(["revalidate", pubkeys]); + return pubkeys; + }, + }); + assert.deepEqual(calls[0], ["revalidate", []]); + const [, tags, notifying] = calls[1]; + assert.deepEqual(notifying, []); + assert.ok(tags.some(([name, key]) => name === "mention" && key === agent)); +}); + test("edit save revalidates added mentions immediately before save", async () => { const agent = "c".repeat(64); const calls = []; diff --git a/desktop/src/features/messages/ui/submitMessageEdit.ts b/desktop/src/features/messages/ui/submitMessageEdit.ts index a3c3bfecaf..96856975e0 100644 --- a/desktop/src/features/messages/ui/submitMessageEdit.ts +++ b/desktop/src/features/messages/ui/submitMessageEdit.ts @@ -117,10 +117,12 @@ export async function submitMessageEdit({ }; // Current picker bindings must not reinterpret the original body: selecting a // different Scout would otherwise make that new key look already notified. - // With unresolved history, conservatively revalidate all current recipients. - const originalMentionPubkeys = editTarget.unresolvedMentionPubkeys?.length - ? [] - : (editTarget.mentionRefs ?? []).map((ref) => ref.pubkey); + // Resolved historical keys are already recipients, so an unresolved neighbor + // must not make them notify again. Unresolved keys have no trusted binding to + // the edited text; they stay out of this set and are revalidated if present. + const originalMentionPubkeys = (editTarget.mentionRefs ?? []).map( + (ref) => ref.pubkey, + ); let addedMentionPubkeys: string[]; try { addedMentionPubkeys = diffAddedMentionPubkeys(