From 022b0bc2ac8e6397fd85848dffe5890c5c129a3f Mon Sep 17 00:00:00 2001 From: Aris Gysel Date: Fri, 24 Jul 2026 23:10:55 +0200 Subject: [PATCH] fix(acp): answer unmentioned direct messages Signed-off-by: Aris Gysel --- crates/buzz-acp/src/config.rs | 35 +++++++++++++++++++++ crates/buzz-acp/src/filter.rs | 52 ++++++++++++++++++++++++------- crates/buzz-acp/src/lib.rs | 45 +++++++++++++++++++++++--- crates/buzz-acp/src/setup_mode.rs | 1 + 4 files changed, 116 insertions(+), 17 deletions(-) diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..f560da20dc 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -1313,6 +1313,17 @@ pub fn resolve_dynamic_channel_filter( } } +/// Allow plain messages to reach the ACP harness in confirmed DM channels. +/// +/// Mention filtering remains unchanged for streams and channels whose type +/// could not be resolved. The harness performs the same confirmed-DM check +/// before matching subscription rules. +pub(crate) fn exempt_confirmed_dm_from_mentions(filter: &mut ChannelFilter, is_confirmed_dm: bool) { + if is_confirmed_dm { + filter.require_mention = false; + } +} + fn rule_applies_to_channel(rule: &SubscriptionRule, channel_id: Uuid) -> bool { use crate::filter::ChannelScope; match &rule.channels { @@ -1434,6 +1445,30 @@ mod tests { assert!(!f.require_mention); } + #[test] + fn test_confirmed_dm_exempts_relay_subscription_from_mentions() { + let mut filter = ChannelFilter { + kinds: Some(vec![buzz_core::kind::KIND_STREAM_MESSAGE]), + require_mention: true, + }; + + exempt_confirmed_dm_from_mentions(&mut filter, true); + + assert!(!filter.require_mention); + } + + #[test] + fn test_unconfirmed_channel_keeps_relay_mention_requirement() { + let mut filter = ChannelFilter { + kinds: Some(vec![buzz_core::kind::KIND_STREAM_MESSAGE]), + require_mention: true, + }; + + exempt_confirmed_dm_from_mentions(&mut filter, false); + + assert!(filter.require_mention); + } + #[test] fn normalizes_goose_args_to_acp() { assert_eq!(normalize_agent_args("goose", Vec::new()), vec!["acp"]); diff --git a/crates/buzz-acp/src/filter.rs b/crates/buzz-acp/src/filter.rs index 43edd969dd..1b30433a26 100644 --- a/crates/buzz-acp/src/filter.rs +++ b/crates/buzz-acp/src/filter.rs @@ -349,9 +349,9 @@ const MAX_CONSECUTIVE_TIMEOUTS: u32 = 5; /// /// 1. **channels** — if not `"all"`, the event's channel UUID must be in the list. /// 2. **kinds** — if non-empty, the event kind must be in the list. -/// 3. **require_mention** — if `true`, a `p` tag matching `agent_pubkey_hex` must -/// exist. Tag kind is checked via `tag.as_slice()` for stable, library-independent -/// access. +/// 3. **require_mention** — if `true` and the channel is not a confirmed DM, a +/// `p` tag matching `agent_pubkey_hex` must exist. Tag kind is checked via +/// `tag.as_slice()` for stable, library-independent access. /// 4. **filter** — if `Some`, the evalexpr expression must evaluate to `true`. /// /// # Fail-closed filter error handling @@ -370,6 +370,7 @@ pub async fn match_event( channel_id: uuid::Uuid, rules: &[SubscriptionRule], agent_pubkey_hex: &str, + is_confirmed_dm: bool, ) -> Option { let filter_ctx = FilterContext::from_event(event, channel_id); @@ -387,7 +388,7 @@ pub async fn match_event( // 3. Mention check — look for a `p` tag whose first element equals // agent_pubkey_hex. Uses tag.as_slice() for stable, library-independent // access — avoids relying on the Display impl of tag kind. - if rule.require_mention { + if rule.require_mention && !is_confirmed_dm { let mentioned = event.tags.iter().any(|tag| { let s = tag.as_slice(); s.first().map(|k| k.as_str()) == Some("p") @@ -601,7 +602,9 @@ mod tests { ), ]; - let matched = match_event(&event, channel_id, &rules, "").await.unwrap(); + let matched = match_event(&event, channel_id, &rules, "", false) + .await + .unwrap(); assert_eq!(matched.rule_index, 0); assert_eq!(matched.prompt_tag, "tag-first"); } @@ -630,7 +633,9 @@ mod tests { ), ]; - let matched = match_event(&event, channel_id, &rules, "").await.unwrap(); + let matched = match_event(&event, channel_id, &rules, "", false) + .await + .unwrap(); assert_eq!(matched.rule_index, 1); assert_eq!(matched.prompt_tag, "matched"); } @@ -653,16 +658,37 @@ mod tests { )]; // Without mention — no match. - let result = match_event(&event_no_mention, channel_id, &rules, agent_pubkey).await; + let result = match_event(&event_no_mention, channel_id, &rules, agent_pubkey, false).await; assert!(result.is_none()); // With mention — matches. - let matched = match_event(&event_with_mention, channel_id, &rules, agent_pubkey) + let matched = match_event(&event_with_mention, channel_id, &rules, agent_pubkey, false) .await .unwrap(); assert_eq!(matched.prompt_tag, "mentioned"); } + #[tokio::test] + async fn test_match_event_confirmed_dm_does_not_require_mention() { + let agent_pubkey = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + let event = make_event(9, "hello"); + let channel_id = any_channel(); + let rules = vec![make_rule( + "mention-only", + ChannelScope::All("all".into()), + vec![], + true, + None, + Some("direct-message"), + )]; + + let matched = match_event(&event, channel_id, &rules, agent_pubkey, true) + .await + .expect("confirmed DM should not require a mention"); + + assert_eq!(matched.prompt_tag, "direct-message"); + } + #[tokio::test] async fn test_match_event_no_match() { let event = make_event(1, "hello"); @@ -677,7 +703,7 @@ mod tests { None, )]; - let result = match_event(&event, channel_id, &rules, "").await; + let result = match_event(&event, channel_id, &rules, "", false).await; assert!(result.is_none()); } @@ -725,7 +751,9 @@ mod tests { None, // no explicit tag )]; - let matched = match_event(&event, channel_id, &rules, "").await.unwrap(); + let matched = match_event(&event, channel_id, &rules, "", false) + .await + .unwrap(); assert_eq!(matched.prompt_tag, "my-rule"); } @@ -755,7 +783,7 @@ mod tests { ]; // Must return None — not "catch-all". - let result = match_event(&event, channel_id, &rules, "").await; + let result = match_event(&event, channel_id, &rules, "", false).await; assert!( result.is_none(), "filter error must fail closed, not fall through to next rule" @@ -781,7 +809,7 @@ mod tests { .store(MAX_CONSECUTIVE_TIMEOUTS, Ordering::Relaxed); let rules = vec![rule]; - let result = match_event(&event, channel_id, &rules, "").await; + let result = match_event(&event, channel_id, &rules, "", false).await; assert!(result.is_none(), "disabled rule must return None"); } } diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 03b75a4211..a02705d784 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1473,7 +1473,13 @@ async fn tokio_main() -> Result<()> { } }; - let channel_filters = config::resolve_channel_filters(&config, &channel_ids, &rules); + let mut channel_filters = config::resolve_channel_filters(&config, &channel_ids, &rules); + for (channel_id, filter) in &mut channel_filters { + let is_confirmed_dm = channel_info_map + .get(channel_id) + .is_some_and(|info| info.channel_type == "dm"); + config::exempt_confirmed_dm_from_mentions(filter, is_confirmed_dm); + } if channel_filters.is_empty() { tracing::warn!("no channel subscriptions resolved — agent will sit idle"); } @@ -1967,7 +1973,16 @@ async fn tokio_main() -> Result<()> { if subscribed_channel_ids.contains(&ch) { tracing::debug!(channel_id = %ch, "membership notification: channel already subscribed"); - } else if let Some(filter) = config::resolve_dynamic_channel_filter(&config, ch, &rules) { + } else if let Some(mut filter) = config::resolve_dynamic_channel_filter(&config, ch, &rules) { + let is_confirmed_dm = ctx + .channel_info + .resolve(ch) + .await + .is_some_and(|info| info.channel_type == "dm"); + config::exempt_confirmed_dm_from_mentions( + &mut filter, + is_confirmed_dm, + ); tracing::info!(channel_id = %ch, "membership notification: subscribing to new channel"); if let Err(e) = relay.subscribe_channel_from(ch, filter, Some(ts)).await { tracing::warn!("failed to subscribe to new channel {ch}: {e}"); @@ -2143,13 +2158,26 @@ async fn tokio_main() -> Result<()> { // launched by the same human). Allowlist adds the // explicit pubkey list on top, for external people; // it never revokes same-owner team bots. + let resolved_channel_info = + ctx.channel_info.resolve(buzz_event.channel_id).await; + let is_confirmed_dm = resolved_channel_info + .as_ref() + .is_some_and(|info| info.channel_type == "dm"); { let author = buzz_event.event.pubkey.to_hex(); // DM hardening: resolve channel type (fail-closed // to DM) so allowlist/anyone modes cannot be // exercised by non-owner authors inside DMs. - let is_dm = - is_dm_channel(buzz_event.channel_id, &ctx.channel_info).await; + let is_dm = resolved_channel_info + .as_ref() + .map(|info| info.channel_type == "dm") + .unwrap_or_else(|| { + tracing::warn!( + channel_id = %buzz_event.channel_id, + "channel type unresolved — treating as DM for author gate (fail closed)" + ); + true + }); let allowed = author_allowed( &config.respond_to, &config.respond_to_allowlist, @@ -2171,7 +2199,14 @@ async fn tokio_main() -> Result<()> { } } - let matched = filter::match_event(&buzz_event.event, buzz_event.channel_id, &rules, &pubkey_hex).await; + let matched = filter::match_event( + &buzz_event.event, + buzz_event.channel_id, + &rules, + &pubkey_hex, + is_confirmed_dm, + ) + .await; let prompt_tag = match matched { Some(m) => m.prompt_tag, None => { diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..c8d2b72a1c 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -446,6 +446,7 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> buzz_event.channel_id, &rules, &pubkey_hex, + false, ) .await .is_some();