Skip to content

fix(acp): answer owner DMs without an explicit @mention #2777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
webdevtodayjason wants to merge 1 commit into
base: main
Choose a base branch
from
+158 −24
Open

fix(acp): answer owner DMs without an explicit @mention #2777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 45 additions & 0 deletions crates/buzz-acp/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
83 changes: 73 additions & 10 deletions crates/buzz-acp/src/filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -370,6 +380,7 @@ pub async fn match_event(
channel_id: uuid::Uuid,
rules: &[SubscriptionRule],
agent_pubkey_hex: &str,
mention_exempt: bool,
) -> Option<MatchedRule> {
let filter_ctx = FilterContext::from_event(event, channel_id);

Expand All @@ -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")
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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");
}
Expand All @@ -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");
Expand All @@ -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());
}

Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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"
Expand All @@ -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");
}
}
49 changes: 36 additions & 13 deletions crates/buzz-acp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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"
Expand All @@ -2171,17 +2188,23 @@ 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 => {
tracing::debug!(channel_id = %buzz_event.channel_id, kind = buzz_event.event.kind.as_u16(), "event matched no rule — dropping");
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
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion crates/buzz-acp/src/setup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down