block / block/buzz

buzz-acp: mid-turn steering is channel-scoped, so a message in an unrelated thread is injected into the in-flight turn

Open
#5,839 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
32.7k
Forks
4.3k
Avg merge
1d 13h
Merged PRs (30d)
253

Description

**Component:** `buzz-acp`
**Version:** source tree at `3e48f1b2365d326ee1c9582448d86a99b44ecd5d`, binary built 2026-07-30
**Config:** `subscribe=Mentions dedup=Queue meh=Steer agents=1 context_limit=12 max_turns_per_session=20 respond_to=anyone`

## Summary

`MultipleEventHandling::Steer` is the default mid-turn delivery path, and it is a good default: a follow-up message that clarifies the request the agent is currently working on gets woven into the running turn instead of cancelling it. But the decision is made **purely on `channel_id`** — the thread the new message belongs to is never consulted. In a channel where two people work in two parallel threads, a message in thread B is steered into the turn that is answering thread A.

The agent then produces one reply, whose scope and `--reply-to` anchor are derived from the *last* event in the batch, so the answer to the first person's question can be posted into the second person's thread — or lost.

We hit this daily with three agents (finance, UI/UX, general) in shared channels where two humans work in parallel threads.

## Current behavior

Everything below is channel-keyed; thread identity exists but only affects prompt rendering.

1. **Steer gate** — `lib.rs:2217`:
```rust
if accepted && queue.is_channel_in_flight(buzz_event.channel_id) {
let signal = mode_gate_signal(config.multiple_event_handling, &author_hex, owner_cache.get());
```
`mode_gate_signal` (`lib.rs:2762`) takes `handling`, `author_hex`, `owner` — no event, no thread. For `Steer` it unconditionally returns `Some(ControlSignal::Steer)`.

2. **Sessions are per channel** — `pool.rs:87`:
```rust
/// channel_id → session_id
pub sessions: HashMap,
```
All threads in a channel share one ACP session and one context window.

3. **Queues and in-flight state are per channel** — `queue.rs:139-143` (`in_flight_channels`, `in_flight_deadlines`, `in_flight_batch_sizes`).

4. **Thread identity is available but unused for scheduling** — `parse_thread_tags()` (`queue.rs:849`) already extracts NIP-10 `root`/`reply` markers; it is called only from `format_prompt`.

5. **Mixed batches are labelled by the newest event** — `queue.rs:1406`:
```rust
// Scope is always derived from the LAST event in the batch — that's the
// one the agent is responding to. [...] This prevents mixed batches
// (thread reply + later plain message) from being mislabeled as "thread".
```
This is a sound rule *given* that mixed batches exist, but it means a batch spanning two threads produces a single reply anchored via `resolve_reply_anchor` (`queue.rs:1209`) to the newest thread root.

## Impact

Two concrete failure modes, both observed:

**1. Cross-thread contamination.** User A asks a question in thread A; the agent starts working. User B posts an unrelated question in thread B in the same channel. B's message is steered into A's running turn — same session, same context — and the single resulting reply lands in one thread only.

**2. Turn loss.** The production steer channel has capacity 1 (`lib.rs:2958`, `tokio::sync::mpsc::channel::(1)`), and `acp.rs` only holds one pending steer at a time. A third message arriving while a steer is still awaiting ack fails `try_send` and falls through to the universal cancel+merge path, which calls `agent.state.invalidate(&source)` (`pool.rs:1959`) — the in-flight turn is cancelled and the ACP session is destroyed. So a burst of three messages in a busy channel (clarification in thread A, then anyone posting in thread B) does not just mis-scope the reply, it throws away in-progress work and restarts with an empty context.

Neither is reachable via configuration: all four `MultipleEventHandling` variants (`config.rs:65-85`) gate on mode and author only. `meh=queue` avoids the contamination but also removes the useful in-thread steering, and the post-turn flush still merges both threads into one prompt.

## Reproduction

1. Two humans (or one human plus a second key) with an agent in a shared, non-DM channel; `--multiple-event-handling=steer` (default), `--dedup=queue`, `BUZZ_ACP_AGENTS=1`.
2. User A @-mentions the agent in thread A with a task that takes ≥30 s.
3. While the turn is in flight, user A posts a clarification in thread A. ✓ Expected: steered into the running turn — this works.
4. ~3 s later user B @-mentions the agent in thread B in the same channel.
5. Observed: the log shows a second steer for the same channel; B's text is injected into A's turn. If the ack for step 3 is still pending, the log instead shows the cancel+merge fallback and `session invalidated` — A's work is discarded.
6. Expected: step 4 waits for the current turn to finish, then gets its own turn and its own reply in thread B.

## Proposal

Make the mid-turn signal thread-scoped, opt-in, defaulting to today's behavior.

**a) Track the in-flight thread root.** Add `in_flight_thread_roots: HashMap>` alongside the existing `in_flight_batch_sizes` (`queue.rs:143`), populated in `flush_next` from `parse_thread_tags(&batch.events.last().event).root_event_id` — the same event `format_prompt` already uses to derive scope, so the two stay consistent by construction.

**b) Add a scope option, orthogonal to the existing mode:**
```
--steer-scope=channel|thread (default: channel — current behavior)
```
Applied at `lib.rs:2217`: fire the mid-turn signal only when `steer_scope == Channel`, or when the new event's `root_event_id` equals the in-flight root (treating "no thread" as its own scope, so a top-level channel mention never steers a threaded turn and vice versa). Otherwise take the existing `Queue` path — the event stays in the queue and is dispatched after the current turn completes, which is already correct behavior today.

A smaller-diff alternative is a fifth enum variant, `steer-same-thread`, avoiding a new flag; we'd be happy with either. The orthogonal flag seems preferable because the same scoping question applies to `interrupt` and `owner-interrupt`.

**c) Related, optional — batch splitting.** With (a) and (b) a queued cross-thread event still merges into the next batch and inherits the newest event's scope (`queue.rs:1406`). Splitting `flush_next` batches on `root_event_id`, so a batch never spans threads, would make each thread get its own turn and its own reply. This is the larger change and could land separately; (a) + (b) alone already fix the mid-turn contamination and the turn loss.

## Notes

- Per-thread ACP sessions would be the "full" fix, but on a 2 vCPU host it multiplies live `claude` subprocesses; we are not asking for that. Serializing turns while keeping them thread-scoped is enough.
- Happy to prepare a PR for (a) + (b) if the approach looks right.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.