mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(acp): wake agents for mentions added by edits (#6131)
🤖 ## Summary Today an agent does not wake up when someone edits an existing message to @mention it. With this PR, adding the mention in an edit is enough to reach the agent. The agent then works on the original message, as #4741 set up. - Agents now receive message edits by default. Before, they subscribed only to new messages (and a few other event types). - The desktop app puts a mention tag on an edit only for people whom that edit newly mentions. So an agent wakes once, for the new mention. It does not wake again when the same message is edited later. This PR also fixes one case in the desktop app: when the original also mentioned someone whose profile was not loaded, every mention counted as new, so a typo fix woke the agent again. Now the recipients that the app knows from the original always count as already notified. - This default applies in normal mode, to channels the agent joins while it is running, and in setup mode. - If you give an explicit `--kinds` list, it is used without change. - Sessions do not change in this PR. An edit uses the session that #4741 chooses: with per-thread sessions, the original message's thread; with channel sessions, the channel. ## Details - The change adds the edit event kind (40003) to one shared default list. The startup rules, the relay subscription filters, and the setup rules all use that list. - An agent accepts an event only if both the relay subscription and its local rules allow it. The normal listener's startup rules now come from one function, `startup_subscription_rules`. The harness and the tests both call it, so the tests check the rules that actually run. ### Stack **2 of 3**, based on #4741. Next: #6132. ### Related issue Fixes #2540. ### Testing - `cargo test -p buzz-acp --all-targets` passes. The one test that fails when the git setting `nostr.keyfile` is set is described in #4741. - Admission tests run on the rules the harness installs, for the normal listener and for setup mode. An edit that mentions the agent is accepted. An edit that does not mention it is rejected. - I removed kind 40003 from each of the two defaults, one at a time. Each time, these tests failed. - Config tests check that edits are in the default kinds for startup channels and for channels joined later. - Desktop regression: `@Agent @Missing User please chek` edited to `check`, with only the agent's profile loaded, sends no new notification. It fails on the old code. --------- Signed-off-by: npub1em3jmyn4vu57urqf03txrwreccvejvwdy5c4er8nnrwt7rc4tncscs3ssu <cee32d92756729ee0c097c5661b879c6199931cd25315c8cf398dcbf0f155cf1@buzz.block.builderlab.xyz> Signed-off-by: Larry <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz> Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-authored-by: npub1em3jmyn4vu57urqf03txrwreccvejvwdy5c4er8nnrwt7rc4tncscs3ssu <cee32d92756729ee0c097c5661b879c6199931cd25315c8cf398dcbf0f155cf1@buzz.block.builderlab.xyz> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
This commit is contained in:
Logan Johnson
@@ -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:**
|
||||
|
||||
@@ -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<Vec<SubscriptionRule>, 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<u32> {
|
||||
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<Uuid, ChannelFilter> {
|
||||
use buzz_core::kind::{
|
||||
KIND_STREAM_MESSAGE, KIND_STREAM_REMINDER, KIND_WORKFLOW_APPROVAL_REQUESTED,
|
||||
};
|
||||
|
||||
let target_channels: Vec<Uuid> = 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<ChannelFilter> {
|
||||
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);
|
||||
|
||||
@@ -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<Uuid> = channel_info_map.keys().copied().collect();
|
||||
|
||||
let rules: Vec<SubscriptionRule> = 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<Vec<SubscriptionRule>> {
|
||||
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
|
||||
|
||||
@@ -590,7 +590,7 @@ fn build_setup_subscription_rules(config: &Config) -> Vec<filter::SubscriptionRu
|
||||
let kinds = config
|
||||
.kinds_override
|
||||
.clone()
|
||||
.unwrap_or_else(|| vec![KIND_STREAM_MESSAGE, KIND_WORKFLOW_APPROVAL_REQUESTED]);
|
||||
.unwrap_or_else(crate::config::default_mention_kinds);
|
||||
|
||||
match &config.subscribe_mode {
|
||||
// Config mode: load the actual rules, but they will be filtered by
|
||||
@@ -953,7 +953,9 @@ mod tests {
|
||||
)
|
||||
.await
|
||||
.expect("asker should pass the setup author gate");
|
||||
let rules = vec![mentions_rule(vec![KIND_STREAM_MESSAGE_EDIT])];
|
||||
// Production setup rules with default kinds, so dropping kind:40003
|
||||
// from the setup admission path fails these tests.
|
||||
let rules = default_setup_rules();
|
||||
let (publisher, mut published) = RelayEventPublisher::test_pair();
|
||||
let payload = SetupPayload {
|
||||
agent_name: "Fizz".into(),
|
||||
@@ -1036,6 +1038,45 @@ mod tests {
|
||||
assert!(!setup_listener_admits(&build(&agent_keys, true), &agent));
|
||||
}
|
||||
|
||||
fn default_setup_rules() -> Vec<filter::SubscriptionRule> {
|
||||
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 {
|
||||
|
||||
@@ -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 = [];
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user