Skip to content

fix(acp): keep top-level agent replies in channel #3687

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

Draft
inde3d100 wants to merge 1 commit into
base: main
Choose a base branch
from
+67 −74
Draft

fix(acp): keep top-level agent replies in channel #3687

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
2 changes: 1 addition & 1 deletion crates/buzz-acp/src/base_prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ For explicit changes to an existing personal agent, use `buzz agents draft-updat

Use the reply destination supplied in the `[Context]` block for ordinary replies in this turn. Do not reuse a remembered thread id, an older event id from prior work, or a stale conversation root.

For human-facing work, keep the conversation flat and easy to read. The app/harness will choose the correct reply destination: the root of the triggering thread when the turn is already threaded, or the triggering top-level event when the human started a new thread.
For human-facing work, keep the conversation flat and easy to read. The app/harness will choose the correct reply destination: the root of the triggering thread when the turn is already threaded, or no reply destination for a top-level channel message so the response stays in the channel.

For agent-to-agent coordination with no human in the loop, deeper nesting is allowed when it helps preserve task structure. Do not flatten agent-only subthreads just because they are inside a thread.

Expand Down
128 changes: 62 additions & 66 deletions crates/buzz-acp/src/queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,19 +1156,18 @@ fn append_reply_instruction(s: &mut String, event_id: &str) {
));
}

/// Append a new-thread reply instruction for a human-facing top-level mention.
/// Append a channel-root reply instruction for a top-level message.
///
/// The triggering mention has no thread tags, so the agent's reply becomes the
/// thread root. Anchoring to the triggering event (rather than leaving the
/// choice open) prevents replying into a stale/unrelated prior thread.
fn append_new_thread_reply_instruction(s: &mut String, event_id: &str) {
s.push_str(&format!(
"\nIMPORTANT: This is a new top-level message. For ordinary replies in \
this turn, use `--reply-to {event_id}` on `buzz messages send` — the \
triggering message is the thread root. Do NOT reply into any other \
(older) thread. If the human explicitly asks for a channel-root, \
top-level, or broadcast post, send that message without `--reply-to`."
));
/// The triggering message has no thread tags, so ordinary agent replies should
/// remain in the channel timeline instead of opening a new thread.
fn append_channel_root_reply_instruction(s: &mut String) {
s.push_str(
"\nIMPORTANT: This is a top-level channel message. For ordinary replies \
in this turn, send with `buzz messages send` without `--reply-to` so \
the response stays in the channel. Do NOT reply into any older thread. \
If the human explicitly asks for a threaded reply and the destination \
is ambiguous, ask before sending.",
);
}

/// Decide whether a turn is human-facing for reply-anchor purposes.
Expand Down Expand Up @@ -1200,36 +1199,29 @@ fn turn_is_human_facing(

/// Resolve the `--reply-to` anchor for a non-DM turn.
///
/// Returns `Some(id)` only for human-facing turns (see [`turn_is_human_facing`]):
/// - in a thread → the thread ROOT, keeping the reply flat at layer 1
/// - top-level → the triggering event id, which becomes the new thread root
/// Returns the thread root only for human-facing turns that are already in a
/// thread (see [`turn_is_human_facing`]). Top-level turns return `None` so the
/// agent replies at the channel root.
///
/// Returns `None` for agent↔agent turns, leaving the agent free to nest deeply
/// (intentional for agent coordination).
fn resolve_reply_anchor(
sender_pubkey: &str,
thread_tags: &ThreadTags,
triggering_event_id: &str,
profile_lookup: Option<&PromptProfileLookup>,
) -> Option<String> {
if !turn_is_human_facing(sender_pubkey, thread_tags, profile_lookup) {
return None;
}
Some(
thread_tags
.root_event_id
.clone()
.unwrap_or_else(|| triggering_event_id.to_string()),
)
thread_tags.root_event_id.clone()
}

/// Format a `[Context]` hints section based on event scope.
///
/// `reply_anchor` is the pre-resolved `--reply-to` target for this turn (see
/// [`resolve_reply_anchor`]). In the thread/DM branches it threads ordinary
/// replies; in the channel branch a `Some` anchor means a human-facing
/// top-level mention whose reply should open a new thread rooted at the
/// triggering event.
/// replies. The channel branch always instructs the agent to reply without an
/// anchor so the response remains top-level.
fn format_context_hints(
channel_id: Uuid,
channel_info: Option<&PromptChannelInfo>,
Expand Down Expand Up @@ -1306,9 +1298,7 @@ fn format_context_hints(
Channel: {channel_display}\n\
Hint: Use `buzz messages get --channel <UUID>` for recent messages if needed."
);
if let Some(event_id) = reply_anchor {
append_new_thread_reply_instruction(&mut s, event_id);
}
append_channel_root_reply_instruction(&mut s);
s
}
}
Expand Down Expand Up @@ -1459,9 +1449,9 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec<Str

// 2. Context hints (with a human-aware reply anchor).
//
// Human-facing turns are anchored so replies stay readable at layer 1:
// - in a thread → anchor to the thread ROOT (no depth-2 nesting)
// - top-level → anchor to the triggering event (it becomes the root)
// Human-facing turns already in a thread are anchored to the thread ROOT
// so replies stay readable at layer 1. Top-level turns get no anchor and
// remain in the channel timeline.
// Agent↔agent turns get no forced anchor — deep nesting is intentional
// there. DMs are always 1:1 with a human, so they always anchor.
let sender_pubkey = last_event.event.pubkey.to_hex();
Expand All @@ -1471,12 +1461,7 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec<Str
.is_some()
.then(|| last_event.event.id.to_hex())
} else {
resolve_reply_anchor(
&sender_pubkey,
&thread_tags,
&last_event.event.id.to_hex(),
args.profile_lookup,
)
resolve_reply_anchor(&sender_pubkey, &thread_tags, args.profile_lookup)
};
sections.push(format_context_hints(
batch.channel_id,
Expand Down Expand Up @@ -1796,13 +1781,18 @@ mod tests {
assert_eq!(batch.events[0].event.content, "oldest");
assert_eq!(batch.events[1].event.content, "newest");

// The rendered prompt's reply anchor must cite the newest event, so
// the agent's reply threads under the message it is responding to.
// The rendered prompt preserves that order and keeps the top-level
// response in the channel instead of opening a thread under the newest
// event.
let newest_id = batch.events[1].event.id.to_hex();
let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n");
assert!(
prompt.contains(&format!("--reply-to {newest_id}")),
"reply anchor must target the newest event; prompt was:\n{prompt}"
!prompt.contains(&format!("--reply-to {newest_id}")),
"top-level batch must not open a thread; prompt was:\n{prompt}"
);
assert!(
prompt.find("Content: oldest") < prompt.find("Content: newest"),
"prompt events must remain chronological; prompt was:\n{prompt}"
);
}

Expand Down Expand Up @@ -3248,7 +3238,6 @@ mod tests {
const AGENT_A_PK: &str = "2222222222222222222222222222222222222222222222222222222222222222";
const AGENT_B_PK: &str = "3333333333333333333333333333333333333333333333333333333333333333";
const ROOT_ID: &str = "abc0000000000000000000000000000000000000000000000000000000000000";
const TRIGGER_ID: &str = "def0000000000000000000000000000000000000000000000000000000000000";

fn profile(is_agent: bool) -> PromptProfile {
PromptProfile {
Expand Down Expand Up @@ -3278,46 +3267,46 @@ mod tests {
fn test_anchor_human_in_thread_uses_root() {
// Human asks inside a thread → anchor to the thread ROOT (flat at L1).
let tags = thread_tags(Some(ROOT_ID), &[AGENT_A_PK]);
let anchor = resolve_reply_anchor(HUMAN_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
let anchor = resolve_reply_anchor(HUMAN_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor.as_deref(), Some(ROOT_ID));
}

#[test]
fn test_anchor_human_top_level_uses_triggering_event() {
// Human top-level mention (no thread tags) → triggering event is root.
fn test_anchor_human_top_level_is_none() {
// Human top-level mention (no thread tags) → reply in the channel.
let tags = thread_tags(None, &[AGENT_A_PK]);
let anchor = resolve_reply_anchor(HUMAN_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
assert_eq!(anchor.as_deref(), Some(TRIGGER_ID));
let anchor = resolve_reply_anchor(HUMAN_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor, None);
}

#[test]
fn test_anchor_agent_to_agent_in_thread_is_none() {
// Agent pings agent inside a thread → no forced anchor (deep nesting ok).
let tags = thread_tags(Some(ROOT_ID), &[AGENT_B_PK]);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor, None);
}

#[test]
fn test_anchor_agent_to_agent_top_level_is_none() {
let tags = thread_tags(None, &[AGENT_B_PK]);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor, None);
}

#[test]
fn test_anchor_agent_sender_but_human_tagged_flattens() {
// Agent-authored, but a human is tagged → human-facing → anchor to root.
let tags = thread_tags(Some(ROOT_ID), &[AGENT_B_PK, HUMAN_PK]);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor.as_deref(), Some(ROOT_ID));
}

#[test]
fn test_anchor_unknown_identity_treated_as_human() {
// No profile lookup → fail open (treat as human so visibility is kept).
let tags = thread_tags(Some(ROOT_ID), &[]);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, TRIGGER_ID, None);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, None);
assert_eq!(anchor.as_deref(), Some(ROOT_ID));
}

Expand All @@ -3326,7 +3315,7 @@ mod tests {
// Raw p-tag presence must NOT flatten when every tagged pubkey is an
// agent — this is the regression Pinky flagged.
let tags = thread_tags(Some(ROOT_ID), &[AGENT_A_PK, AGENT_B_PK]);
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, TRIGGER_ID, Some(&id_lookup()));
let anchor = resolve_reply_anchor(AGENT_A_PK, &tags, Some(&id_lookup()));
assert_eq!(anchor, None);
}

Expand Down Expand Up @@ -3957,17 +3946,20 @@ mod tests {
cancel_reason: None,
};

// Top-level human message (no lookup → human): the reply opens a new
// thread anchored to the triggering event, preventing replies into a
// stale older thread.
// Top-level human message: the reply stays in the channel and must not
// reuse either the triggering event or a stale older thread.
let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n");
assert!(
prompt.contains(&format!("--reply-to {event_id}")),
"top-level human message should anchor a new thread at the triggering event"
!prompt.contains(&format!("--reply-to {event_id}")),
"top-level human message should not open a thread at the triggering event"
);
assert!(
prompt.contains("send with `buzz messages send` without `--reply-to`"),
"top-level human message should use the channel-root instruction"
);
assert!(
prompt.contains("new top-level message"),
"top-level human message should use the new-thread instruction"
prompt.contains("Do NOT reply into any older thread"),
"top-level human message should forbid stale thread reuse"
);
}

Expand Down Expand Up @@ -4121,7 +4113,7 @@ mod tests {
let root_id = "d".repeat(64);
let threaded = make_event_with_tags(
"earlier thread msg",
vec![vec!["e".into(), root_id, "".into(), "reply".into()]],
vec![vec!["e".into(), root_id.clone(), "".into(), "reply".into()]],
);
let plain = make_event("latest top-level");
let plain_id = plain.id.to_hex();
Expand All @@ -4143,16 +4135,20 @@ mod tests {
cancel_reason: None,
};

// Last event is top-level and human-facing → opens a new thread
// anchored to that top-level event (NOT the earlier thread's root).
// Last event is top-level → reply in the channel, not under either the
// top-level event or the earlier thread.
let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n");
assert!(
prompt.contains(&format!("--reply-to {plain_id}")),
"batched top-level-last prompt should anchor to the last (top-level) event"
!prompt.contains(&format!("--reply-to {plain_id}")),
"batched top-level-last prompt should not open a thread"
);
assert!(
!prompt.contains(&format!("--reply-to {root_id}")),
"batched top-level-last prompt should not reuse the earlier thread"
);
assert!(
prompt.contains("new top-level message"),
"batched top-level-last prompt should use the new-thread instruction"
prompt.contains("send with `buzz messages send` without `--reply-to`"),
"batched top-level-last prompt should use the channel-root instruction"
);
}

Expand Down
11 changes: 4 additions & 7 deletions crates/buzz-acp/src/setup_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,8 @@ async fn handle_setup_membership(

/// Build and publish a setup nudge reply to the triggering event.
///
/// Threading: flat reply to the thread root if one exists; otherwise reply
/// to the triggering event itself. P-tags the asker.
/// Threading: flat reply to the thread root if one exists; otherwise post at
/// the channel root. P-tags the asker.
async fn publish_setup_nudge(
publisher: &RelayEventPublisher,
keys: &nostr::Keys,
Expand All @@ -613,11 +613,8 @@ async fn publish_setup_nudge(
parent_event_id: root_id,
})
} else {
// Top-level event: reply to the triggering event.
Some(ThreadRef {
root_event_id: triggering_event.id,
parent_event_id: triggering_event.id,
})
// Top-level event: keep the nudge in the channel timeline.
None
};

let body = payload.nudge_body();
Expand Down