From 5250b55056bbee5f184e08cb91525eb3ccaebfcd Mon Sep 17 00:00:00 2001 From: webdevtodayjason Date: Fri, 24 Jul 2026 17:33:30 -0500 Subject: [PATCH] fix(acp): answer owner DMs without an explicit @mention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a 1:1 DM the agent required an @mention on every message: the mention gate (require_mention) was applied uniformly to every channel, DM included, at both the relay subscription filter (drops untagged events before delivery) and match_event (re-checks the p-tag). A DM is addressed to the counterparty by definition, so this made agents look unresponsive to their own owner. Exempt the gate for owner-authored DM events — computed as is_dm && author == owner and threaded to match_event as mention_exempt, with the DM channels' subscription filter dropping the #p requirement. Scoped to the owner so agent-to-agent DM messages still require a mention, preserving the #2270 anti-loop invariant. Group channels are unchanged. DM turns are already restricted to owner + verified siblings (#2591), so this does not expose agents to third-party prompting. Closes #2747. Signed-off-by: webdevtodayjason --- crates/buzz-acp/src/config.rs | 45 +++++++++++++++++ crates/buzz-acp/src/filter.rs | 83 +++++++++++++++++++++++++++---- crates/buzz-acp/src/lib.rs | 49 +++++++++++++----- crates/buzz-acp/src/setup_mode.rs | 5 +- 4 files changed, 158 insertions(+), 24 deletions(-) diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..3d9a15230f 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -482,6 +482,24 @@ pub struct ChannelFilter { pub require_mention: bool, } +impl ChannelFilter { + /// Drop the relay-side `#p` mention filter when the channel is a DM. + /// + /// A 1:1 DM is addressed to the agent by definition, so the relay must + /// deliver every message in it — not just `p`-tagged ones — otherwise the + /// agent never sees plain (un-@mentioned) DM messages (issue #2747). This + /// only widens *delivery*; per-event triggering is still gated by the + /// inbound author gate and `match_event`, which restrict DM turns to the + /// owner. Group channels keep their mention filter unchanged. + #[must_use] + pub fn with_dm_exemption(mut self, is_dm: bool) -> Self { + if is_dm { + self.require_mention = false; + } + self + } +} + #[derive(Debug)] pub struct Config { pub keys: Keys, @@ -1330,6 +1348,33 @@ mod tests { use crate::filter::{ChannelScope, SubscriptionRule}; use clap::{Parser, ValueEnum}; + #[test] + fn test_with_dm_exemption_clears_mention_for_dm() { + let filter = ChannelFilter { + kinds: Some(vec![9]), + require_mention: true, + } + .with_dm_exemption(true); + assert!( + !filter.require_mention, + "DM must drop the relay-side #p mention filter" + ); + assert_eq!(filter.kinds, Some(vec![9]), "kinds must be preserved"); + } + + #[test] + fn test_with_dm_exemption_preserves_mention_for_non_dm() { + let filter = ChannelFilter { + kinds: Some(vec![9]), + require_mention: true, + } + .with_dm_exemption(false); + assert!( + filter.require_mention, + "group channels keep the mention filter unchanged" + ); + } + /// Build a minimal Config for testing without CLI parsing. fn test_config(mode: SubscribeMode) -> Config { Config { diff --git a/crates/buzz-acp/src/filter.rs b/crates/buzz-acp/src/filter.rs index 43edd969dd..ff0b589029 100644 --- a/crates/buzz-acp/src/filter.rs +++ b/crates/buzz-acp/src/filter.rs @@ -351,9 +351,19 @@ const MAX_CONSECUTIVE_TIMEOUTS: u32 = 5; /// 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. +/// access. Bypassed when `mention_exempt` is set (see below). /// 4. **filter** — if `Some`, the evalexpr expression must evaluate to `true`. /// +/// # DM mention exemption (`mention_exempt`) +/// +/// A 1:1 DM is addressed to the agent by definition, so an owner message in a +/// DM should fire a turn without an explicit `@mention` (issue #2747). The +/// caller computes `mention_exempt` — true only for an **owner-authored** event +/// in a DM channel — and when set, step 3's mention check is skipped. It is +/// never set for sibling-agent or group-channel events, so the anti-loop +/// invariant (agents don't auto-reply to each other unmentioned) and the group +/// mention gate are both preserved. `kinds` and `filter` gating still apply. +/// /// # Fail-closed filter error handling /// /// Any filter evaluation error — including timeout — causes the **entire @@ -370,6 +380,7 @@ pub async fn match_event( channel_id: uuid::Uuid, rules: &[SubscriptionRule], agent_pubkey_hex: &str, + mention_exempt: bool, ) -> Option { let filter_ctx = FilterContext::from_event(event, channel_id); @@ -387,7 +398,8 @@ 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 { + // Skipped when `mention_exempt` is set (owner message in a DM, #2747). + if rule.require_mention && !mention_exempt { let mentioned = event.tags.iter().any(|tag| { let s = tag.as_slice(); s.first().map(|k| k.as_str()) == Some("p") @@ -601,7 +613,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 +644,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 +669,61 @@ 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_mention_exempt_bypasses_require_mention() { + // mention_exempt=true (owner message in a DM, #2747): a require_mention + // rule matches even without a `p` tag. Group/sibling paths never set the + // flag, so this does not weaken the mention gate elsewhere. + let agent_pubkey = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + let event_no_mention = make_event(9, "hello"); + let channel_id = any_channel(); + + let rules = vec![make_rule( + "mention-only", + ChannelScope::All("all".into()), + vec![], + true, + None, + Some("mentioned"), + )]; + + let matched = match_event(&event_no_mention, channel_id, &rules, agent_pubkey, true) .await .unwrap(); assert_eq!(matched.prompt_tag, "mentioned"); } + #[tokio::test] + async fn test_match_event_mention_exempt_still_honors_kind_filter() { + // The exemption only bypasses the mention check — kind gating still + // applies, so an owner DM of the wrong kind is not matched. + let agent_pubkey = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + let event_kind_1 = make_event(1, "hello"); + let channel_id = any_channel(); + + let rules = vec![make_rule( + "kind-9-mention", + ChannelScope::All("all".into()), + vec![9], + true, + None, + Some("mentioned"), + )]; + + let result = match_event(&event_kind_1, channel_id, &rules, agent_pubkey, true).await; + assert!(result.is_none()); + } + #[tokio::test] async fn test_match_event_no_match() { let event = make_event(1, "hello"); @@ -677,7 +738,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 +786,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 +818,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 +844,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 0230ea0875..52f2a7ff83 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1479,7 +1479,16 @@ async fn tokio_main() -> Result<()> { } let mut subscribed_channel_ids = HashSet::with_capacity(channel_filters.len()); for (channel_id, filter) in &channel_filters { - if let Err(e) = relay.subscribe_channel(*channel_id, filter.clone()).await { + // DM mention exemption (#2747): drop the relay-side `#p` filter for + // channels known to be DMs at startup so plain DM messages are + // delivered. Unknown types keep the mention filter (fail narrow at the + // subscription layer — per-event gating re-resolves the type later). + let is_dm = channel_info_map + .get(channel_id) + .map(|ci| ci.channel_type == "dm") + .unwrap_or(false); + let filter = filter.clone().with_dm_exemption(is_dm); + if let Err(e) = relay.subscribe_channel(*channel_id, filter).await { tracing::warn!("failed to subscribe to channel {channel_id}: {e}"); } else { subscribed_channel_ids.insert(*channel_id); @@ -1969,6 +1978,14 @@ async fn tokio_main() -> Result<()> { tracing::debug!(channel_id = %ch, "membership notification: channel already subscribed"); } else if let Some(filter) = config::resolve_dynamic_channel_filter(&config, ch, &rules) { tracing::info!(channel_id = %ch, "membership notification: subscribing to new channel"); + // DM mention exemption (#2747): a DM the + // agent is added to post-startup (e.g. an + // agent-initiated DM) must deliver plain + // messages too. Resolve type via the shared + // resolver (fail-closed to DM on unknown, + // matching the inbound author gate). + let is_dm = is_dm_channel(ch, &ctx.channel_info).await; + let filter = filter.with_dm_exemption(is_dm); if let Err(e) = relay.subscribe_channel_from(ch, filter, Some(ts)).await { tracing::warn!("failed to subscribe to new channel {ch}: {e}"); } else { @@ -2143,13 +2160,13 @@ 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 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 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 allowed = author_allowed( &config.respond_to, &config.respond_to_allowlist, @@ -2162,7 +2179,7 @@ async fn tokio_main() -> Result<()> { if !allowed { tracing::debug!( channel_id = %buzz_event.channel_id, - author = %buzz_event.event.pubkey.to_hex(), + author = %author, mode = %config.respond_to, is_dm, "inbound author gate — dropping event" @@ -2171,7 +2188,14 @@ async fn tokio_main() -> Result<()> { } } - let matched = filter::match_event(&buzz_event.event, buzz_event.channel_id, &rules, &pubkey_hex).await; + // DM mention exemption (#2747): a 1:1 DM is addressed + // to the agent by definition, so an owner message in a + // DM fires a turn without an explicit @mention. Scoped + // to the owner (not siblings) so sibling agents still + // need a mention — preserving the anti-loop invariant + // (#2270). Group channels are never exempt. + let mention_exempt = is_dm && owner_cache.get() == Some(author.as_str()); + let matched = filter::match_event(&buzz_event.event, buzz_event.channel_id, &rules, &pubkey_hex, mention_exempt).await; let prompt_tag = match matched { Some(m) => m.prompt_tag, None => { @@ -2179,9 +2203,8 @@ async fn tokio_main() -> Result<()> { continue; } }; - // Capture author pubkey before queue.push() moves - // buzz_event.event (needed for mode gate below). - let author_hex = buzz_event.event.pubkey.to_hex(); + // `author` (captured above, before queue.push() moves + // buzz_event.event) is reused by the mode gate below. let event_id_hex = buzz_event.event.id.to_hex(); // Clone for the non-cancelling steer fork, which // needs the event to render the steer body. The @@ -2223,7 +2246,7 @@ async fn tokio_main() -> Result<()> { // event that reaches here. let signal = mode_gate_signal( config.multiple_event_handling, - &author_hex, + &author, owner_cache.get(), ); if let Some(signal) = signal { diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..618e6a90c7 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -440,12 +440,15 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> ) .await; - // Apply channel/kind filter rules. + // Apply channel/kind filter rules. Setup mode keeps its own explicit + // @mention requirement (above) regardless of channel type, so it opts + // out of the DM mention exemption (#2747) with mention_exempt = false. let filter_matched = filter::match_event( &buzz_event.event, buzz_event.channel_id, &rules, &pubkey_hex, + false, ) .await .is_some();