jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-11 20:01:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(acp): steer mid-turn DM follow-ups natively again (#8058)

Browse Source 
🤖
## Summary
Some agents can take a new message into a turn that is already running.
This is called "native steering": the agent keeps working and folds the
new message in, instead of stopping and starting over.

#6132 made this safer in channels: a message is steered in only when it
belongs where the running turn will reply. In a channel, each top-level
message gets its own reply thread, so a second top-level message must
not be steered into a turn that replies in the first one's thread.

That rule also ran in DMs, where it does not fit. In a DM, a top-level
message is answered at the top of the conversation, not in a new thread.
So every top-level DM message replies in the same place. But the rule
treated each one as its own thread. As a result, **every DM follow-up
sent while the agent was working cancelled and restarted the turn**,
instead of being steered in as it was before #6132.

Now:
- **DMs:** a follow-up is steered natively when it replies in the same
place as the running turn: both top-level, or both in the same thread. A
thread reply during a top-level DM turn (or the reverse) still takes the
cancel-and-restart path, because the running turn would reply in the
wrong place.
- **Channels:** unchanged from #6132.

## Details
- The queue now records each running turn's reply route: its thread root
(or none) and its reply thread. One function,
`ReplyRoute::accepts_steer`, decides whether a new message may be
steered in. It compares thread roots in DMs and reply threads in
channels.
- The DM rule mirrors how the prompt is built: a top-level DM's
`<context>` has no `--reply-to` target, and a threaded DM's target is in
its thread.
- The steer decision must agree with how the running turn's prompt was
written. The prompt task records whether it writes the prompt as a DM,
and the steer decision reads that recorded value. So if channel metadata
was missing when the turn started (the prompt then uses the normal
channel form) and loads later, the DM rule is not applied to that turn.
- The prompt task records this value as soon as channel metadata
resolves, before session setup. A follow-up sent during a slow
`session/new` is steered in, as on main. When an `initial_message` setup
prompt runs first, the value is recorded after that setup turn, because
the setup prompt reads the same steer mailbox.
- Before the value is recorded, the steer decision uses the listener's
own channel classification. A channel the listener knows is not a DM is
always written in channel form, so the channel rule applies at once,
exactly as on main. A DM, or a channel whose type is not resolved, takes
the cancel-and-restart path until the value is recorded.

### Related issue
Follow-up to #6132 (merged). Found in review after merge. No separate
issue.

### Testing
- New tests drive DM messages through the listener's real steer decision
with a running turn. A second top-level DM message is steered natively
with no cancel. A DM thread reply during a top-level DM turn is not
steered and takes cancel-and-restart.
- Direct tests of the steer rule cover channel and DM cases: same
thread, different thread, top-level after top-level, and top-level mixed
with thread replies.
- With the DM rule disabled, the DM rule test and the top-level DM
listener test fail.
- Listener tests also cover a DM turn whose prompt was written as a
channel (top-level follow-up is not steered) and a DM follow-up that
arrives before the value is recorded (not steered).
- Production-path tests dispatch a real turn through `dispatch_pending`
and `run_prompt_task` to an agent that holds at `session/new` or
`session/prompt`, then send a follow-up through the real listener. A
top-level DM follow-up during the prompt or during `session/new` is
steered with no cancel. A DM follow-up before an `initial_message` setup
turn is not steered. A same-thread channel follow-up during setup is
steered with no cancel.
- Mutation checks: each fails its test. They are: not recording the
value in `run_prompt_task`, passing a detached value from dispatch, not
recording it before session setup, recording it before an
`initial_message` turn, and removing the known-channel rule for the
unrecorded case.
- `cargo test -p buzz-acp` passes on the rebased head, with the
timing-sensitive `acp::tests` run alone (`--test-threads=1`; this PR
does not change them, and they fail only under full parallel load on
this laptop). `cargo clippy -p buzz-acp --tests -D warnings` and `just
desktop-tauri-clippy` are clean.

---------

Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
This commit is contained in:
Logan Johnson
2026-10-05 14:43:21 -04:00
committed by GitHub
parent fd885b5530
commit 0a070dfd3f
3 changed files with 575 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

198
crates/buzz-acp/src/queue.rs
View File

@@ -15,6 +15,7 @@
use nostr::{Event, ToBech32};
use std::collections::{HashMap, HashSet, VecDeque};
use std::sync::{Arc, OnceLock};
use std::time::{Duration, Instant};
use uuid::Uuid;
@@ -154,6 +155,81 @@ impl BatchEvent {
pub fn reply_thread(&self) -> String {
reply_thread(&self.event, self.edit.as_ref())
}
/// See [`ReplyRoute`].
pub fn reply_route(&self) -> ReplyRoute {
ReplyRoute {
root_event_id: self
.routing_thread_tags()
.root_event_id
.map(|root| root.to_ascii_lowercase()),
thread: self.reply_thread(),
}
}
}
/// Where a turn's `<context>` sends ordinary replies, as far as the
/// native-steer guard needs to know.
///
/// A native steer adds a message to a running turn without a new `<context>`,
/// so the turn keeps replying where its own `<context>` points. A message may
/// be steered natively only when that destination is also right for it.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReplyRoute {
/// Routed thread root, lowercase; `None` for a top-level message.
root_event_id: Option<String>,
/// See [`reply_thread`].
thread: String,
}
impl ReplyRoute {
/// Whether a turn whose `<context>` was rendered for `self` also replies
/// correctly to `incoming`.
///
/// In a channel, a top-level message is answered in a new thread rooted at
/// that message, so two messages share a destination only when they share
/// a [`reply_thread`]. In a DM, a top-level message's `<context>` names no
/// reply target (see `format_prompt`): every top-level DM message replies
/// at the top of the same conversation. DM messages therefore share a
/// destination when they share a thread root, including none.
pub fn accepts_steer(&self, incoming: &ReplyRoute, is_dm: bool) -> bool {
if is_dm {
self.root_event_id == incoming.root_event_id
} else {
self.thread == incoming.thread
}
}
}
/// Whether the running turn's prompt rendered its channel as a DM.
///
/// The prompt task records this when it formats the turn's `<context>`
/// (`format_prompt`'s `is_dm`). The steer guard reads the recorded value
/// instead of classifying the incoming event again: channel metadata can
/// become resolvable after the turn starts, and the guard must apply the rule
/// that matches the `<context>` the agent actually received.
#[derive(Debug, Clone, Default)]
pub struct PromptDmClassification(Arc<OnceLock<bool>>);
impl PromptDmClassification {
/// Record the prompt's classification. Later calls are ignored: a turn has
/// one `<context>`.
pub fn record(&self, is_dm: bool) {
let _ = self.0.set(is_dm);
}
/// The recorded classification; `None` before the prompt is formatted.
pub fn get(&self) -> Option<bool> {
self.0.get().copied()
}
}
/// Reply destination of an in-flight turn, and how its prompt classified the
/// channel.
#[derive(Debug, Clone)]
struct InFlightReplyRoute {
route: ReplyRoute,
prompt_is_dm: PromptDmClassification,
}
/// Why a batch's prior turn was cancelled — controls how `format_prompt`
@@ -241,9 +317,9 @@ pub struct EventQueue {
in_flight_deadlines: HashMap<SessionScope, Instant>,
/// Number of events in each in-flight batch (for expiry logging).
in_flight_batch_sizes: HashMap<SessionScope, usize>,
/// Reply thread of each in-flight turn: the [`reply_thread`] of the batch
/// Reply route of each in-flight turn: the [`ReplyRoute`] of the batch
/// event whose `<context>` routes the turn's replies (its last event).
in_flight_reply_threads: HashMap<SessionScope, String>,
in_flight_reply_routes: HashMap<SessionScope, InFlightReplyRoute>,
retry_after: HashMap<SessionScope, Instant>,
/// Per-scope retry attempt counter for exponential backoff / dead-lettering.
retry_counts: HashMap<SessionScope, u32>,
@@ -285,7 +361,7 @@ impl EventQueue {
in_flight_scopes: HashSet::new(),
in_flight_deadlines: HashMap::new(),
in_flight_batch_sizes: HashMap::new(),
in_flight_reply_threads: HashMap::new(),
in_flight_reply_routes: HashMap::new(),
retry_after: HashMap::new(),
retry_counts: HashMap::new(),
dedup_mode,
@@ -433,7 +509,7 @@ impl EventQueue {
);
self.in_flight_scopes.remove(&scope);
self.in_flight_deadlines.remove(&scope);
self.in_flight_reply_threads.remove(&scope);
self.in_flight_reply_routes.remove(&scope);
// Recover any withheld goose-native steer events for the expired
// scope back to the queue front so normal dispatch delivers
// them. Unlike the in-flight batch above (already delivered to a
@@ -477,7 +553,7 @@ impl EventQueue {
.insert(scope.clone(), now + self.in_flight_deadline);
self.in_flight_batch_sizes
.insert(scope.clone(), cancelled.len());
self.record_in_flight_reply_thread(&scope, &cancelled);
self.record_in_flight_reply_route(&scope, &cancelled);
return Some(FlushBatch {
channel_id: scope.channel_id(),
scope,
@@ -515,7 +591,7 @@ impl EventQueue {
.insert(scope.clone(), now + self.in_flight_deadline);
self.in_flight_batch_sizes
.insert(scope.clone(), events.len());
self.record_in_flight_reply_thread(&scope, &events);
self.record_in_flight_reply_route(&scope, &events);
// Merge any cancelled events stored by requeue_as_cancelled().
let cancelled_events = self.cancelled_batches.remove(&scope).unwrap_or_default();
@@ -568,7 +644,7 @@ impl EventQueue {
self.in_flight_scopes.remove(&scope);
self.in_flight_deadlines.remove(&scope);
self.in_flight_batch_sizes.remove(&scope);
self.in_flight_reply_threads.remove(&scope);
self.in_flight_reply_routes.remove(&scope);
let now = Instant::now();
match self.retry_after.get(&scope) {
// Active throttle → scope was requeued; keep retry_counts intact.
@@ -787,7 +863,7 @@ impl EventQueue {
);
self.in_flight_scopes.remove(&scope);
self.in_flight_deadlines.remove(&scope);
self.in_flight_reply_threads.remove(&scope);
self.in_flight_reply_routes.remove(&scope);
// Symmetric with the flush_next expiry block: recover withheld
// goose-native steer events for the expired scope so they are
// not permanently orphaned in the side table.
@@ -943,25 +1019,58 @@ impl EventQueue {
self.in_flight_scopes.contains(&scope.into_scope())
}
/// The reply thread of the turn in flight for `scope`, if any.
/// Whether a message with reply route `incoming` may be steered natively
/// into the turn in flight for `scope`.
///
/// A native steer adds a message to a running turn without a new
/// `<context>`, so the turn keeps replying where its own `<context>`
/// points. A message whose [`reply_thread`] differs (possible under the
/// channel session policy) must not be steered natively; the cancel+merge
/// path re-dispatches it with its own full `<context>`.
pub fn in_flight_reply_thread(&self, scope: &SessionScope) -> Option<&str> {
self.in_flight_reply_threads.get(scope).map(String::as_str)
/// A message whose route the running turn does not accept (see
/// [`ReplyRoute::accepts_steer`]; possible whenever one session spans
/// several reply destinations: the channel session policy, or a DM) must
/// not be steered natively; the cancel+merge path re-dispatches it with
/// its own full `<context>`.
///
/// The rule follows how the running turn's prompt classified the channel
/// (see [`PromptDmClassification`]). Until the prompt task records that,
/// `channel_is_dm` is the listener's own classification, where unresolved
/// metadata counts as a DM. A channel the listener knows is not a DM also
/// renders as one in the prompt, so the channel rule applies at once.
/// Otherwise the prompt may render either way, and the answer is `false`.
pub fn in_flight_accepts_steer(
&self,
scope: &SessionScope,
incoming: &ReplyRoute,
channel_is_dm: bool,
) -> bool {
let Some(running) = self.in_flight_reply_routes.get(scope) else {
return false;
};
match running.prompt_is_dm.get() {
Some(is_dm) => running.route.accepts_steer(incoming, is_dm),
None if !channel_is_dm => running.route.accepts_steer(incoming, false),
None => false,
}
}
fn record_in_flight_reply_thread(&mut self, scope: &SessionScope, events: &[BatchEvent]) {
/// The cell in which the prompt task for `scope`'s in-flight turn records
/// its DM classification (see [`PromptDmClassification`]).
pub fn in_flight_prompt_dm(&self, scope: &SessionScope) -> Option<PromptDmClassification> {
self.in_flight_reply_routes
.get(scope)
.map(|running| running.prompt_is_dm.clone())
}
fn record_in_flight_reply_route(&mut self, scope: &SessionScope, events: &[BatchEvent]) {
match events.last() {
Some(last) => {
self.in_flight_reply_threads
.insert(scope.clone(), last.reply_thread());
self.in_flight_reply_routes.insert(
scope.clone(),
InFlightReplyRoute {
route: last.reply_route(),
prompt_is_dm: PromptDmClassification::default(),
},
);
}
None => {
self.in_flight_reply_threads.remove(scope);
self.in_flight_reply_routes.remove(scope);
}
}
}
@@ -1216,8 +1325,8 @@ pub(crate) fn reaction_target_id(event: &Event) -> String {
/// the routed event itself when it is top-level (a reply opens a thread
/// rooted there). Lowercase, so equivalent hex spellings compare equal.
///
/// This is the thread-session key, and it decides whether a message may be
/// steered natively into a running turn (see [`EventQueue::in_flight_reply_thread`]).
/// This is the thread-session key. Outside DMs it also decides whether a
/// message may be steered natively into a running turn (see [`ReplyRoute`]).
pub(crate) fn reply_thread(event: &Event, edit: Option<&ResolvedEdit>) -> String {
routing_thread_tags(event, edit)
.root_event_id
@@ -2220,6 +2329,7 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec<Str
// - top-level → anchor to the triggering event (it becomes the root)
// Agent↔agent turns get no forced anchor — deep nesting is intentional
// there. DMs are always 1:1 with a human, so they always anchor.
// `ReplyRoute::accepts_steer` mirrors this DM rule; keep them in sync.
let sender_pubkey = last_event.event.pubkey.to_hex();
let reply_anchor = if is_dm {
thread_tags
@@ -2396,6 +2506,50 @@ pub(crate) fn native_steer_framing() -> (&'static str, &'static str) {
(framing.new_tag, framing.closing_note)
}
#[cfg(test)]
mod reply_route_tests {
use super::ReplyRoute;
fn route(root: Option<&str>, thread: &str) -> ReplyRoute {
ReplyRoute {
root_event_id: root.map(str::to_string),
thread: thread.to_string(),
}
}
/// A top-level message (no root) whose replies open thread `id`.
fn top_level(id: &str) -> ReplyRoute {
route(None, id)
}
/// A reply in the thread rooted at `root`.
fn in_thread(root: &str) -> ReplyRoute {
route(Some(root), root)
}
#[test]
fn channel_steers_only_within_one_reply_thread() {
assert!(in_thread("a").accepts_steer(&in_thread("a"), false));
assert!(!in_thread("a").accepts_steer(&in_thread("b"), false));
// Replies to a top-level message open a thread rooted at it.
assert!(top_level("a").accepts_steer(&in_thread("a"), false));
assert!(in_thread("a").accepts_steer(&top_level("a"), false));
assert!(!top_level("a").accepts_steer(&top_level("b"), false));
assert!(!top_level("a").accepts_steer(&in_thread("b"), false));
}
#[test]
fn dm_steers_within_one_thread_or_the_top_level() {
// Every top-level DM message replies at the top of the conversation.
assert!(top_level("a").accepts_steer(&top_level("b"), true));
assert!(in_thread("a").accepts_steer(&in_thread("a"), true));
assert!(!in_thread("a").accepts_steer(&in_thread("b"), true));
// A top-level DM turn has no `--reply-to`; a threaded one does.
assert!(!top_level("a").accepts_steer(&in_thread("a"), true));
assert!(!in_thread("a").accepts_steer(&top_level("b"), true));
}
}
#[cfg(test)]
mod tests {
use super::*;