RFC: Turn-boundary message batching

[brettchien]

RFC: Turn-boundary message batching

Status: Draft
Author: @brettchien
Date: 2026-04-26
Related: #78 (Session Management), #58 (per-connection locking) — as preconditions, not as design space

---

Contents

• Summary · Problem · Current state
• Goals & non-goals · Proposed solution
• Why this design · Benefits · Tradeoffs
• Multi-party interaction · Decisions
• Configuration & rollout · Implementation phases
• Prior art · Appendix A: Reference implementation

---

Summary

Within a single thread, openab today processes one user message per ACP turn. Messages that arrive while a turn is running become independent tokio::spawn tasks racing for the per-connection mutex; they end up dispatched as separate, sequential ACP turns.

This RFC proposes one structural change: introduce a per-thread message buffer, batched at turn boundaries. Messages arriving during an in-flight turn accumulate in the buffer; when the turn finishes, the buffer is dispatched as a single ACP turn carrying all accumulated messages as multiple ContentBlocks. The ACP agent — which has full session context and is paid for anyway — decides how to handle them (split, merge, ignore, ask back).

The broker does structural buffering only; semantic decisions are deferred to ACP. Time ≠ semantics, so the broker does not attempt rule-based or LLM-based topic detection.

---

Problem

Within one thread, humans don't message in turn-shaped units. Cases the current 1-msg-per-turn model handles poorly:

1. Stream-of-thought split — alice types three messages in 5 seconds:

   • "can you check the build"
   • "actually wait"
   • "check the build and run the e2e tests"

   M1 fires immediately; M2 and M3 queue at the mutex. Three sequential turns: turn 1 wastes work checking the build, turn 2 reacts to "wait" with no idea what's coming, turn 3 finally has the full intent — but turns 1 and 2 are already wasted output.

2. Late attachment / clarification — text question, then 8 seconds later the screenshot or link. Today: two turns, the first answers blind to the screenshot.

3. Independent topics interleaved — two genuinely unrelated asks back-to-back. The proposal merges them into one ACP turn; the agent answers both in one response. Acceptable — agents handle multi-intent prompts well.

The first case is the bulk of the problem. The second is the same shape (fragmented intent across messages). The third is a constraint: don't pretend to split topics with rules — let ACP decide.

---

Current state (within one thread)

After RFC #78 §2b, each thread has its own Arc<Mutex<AcpConnection>> (pool.rs:15). Inter-thread isolation is solved. This RFC zooms inside one thread.

One lock cycle = one user message

adapter.rs:181 → pool.with_connection(thread_key, |conn| { ... }) (pool.rs:223) calls conn.session_prompt(content_blocks).await (adapter.rs:240) exactly once per call. content_blocks is built from one user message — its prompt text plus that message's own image / transcript blocks (adapter.rs:131-152). Multiple ContentBlocks in one turn means "one message with multiple media parts," never "multiple messages."

Where pending messages live today

There is no buffer and no queue in the codebase. When M1, M2, M3 arrive on the same thread while M0's turn is streaming, they become N independent tokio::spawn tasks (discord.rs:600-608), each parked on the per-connection mutex.

Tokio's Mutex keeps a fair-ish FIFO LinkedList<Waker>, but docs do not guarantee strict FIFO. The mutex sees only "someone is waiting" — wakers are opaque, so it cannot enumerate pending messages, inspect content, or batch them. Batching therefore can't be retrofitted onto the mutex; it requires an explicit queue at a layer that owns the message data.

So the question this RFC answers is:

Within one thread, when messages arrive during an in-flight turn, how should they be mapped to ACP turns?

Today's implicit answer: "spawn N independent futures, let the mutex serialize them, run them as N sequential turns." This RFC's answer: "send them into a per-thread bounded mpsc::channel; one consumer task drains and dispatches as one batch when the turn completes."

---

Goals & non-goals

Goals

• Replace 1 message → 1 turn with N messages-arrived-during-turn → 1 next turn within a single thread.
• Introduce the data structure required (an explicit per-thread bounded mpsc::channel).
• Achieve deterministic message ordering as a side benefit.

Non-goals

Concern	Why not this RFC
Inter-thread isolation	Already solved by per-connection mutex (RFC #78 §2b / #58). Precondition.
Cross-session blocking (#307)	Different layer — about a new thread's session unable to start.
Pre-turn debouncing	First message responds immediately; no artificial latency floor.
Topic detection / semantic grouping	Deferred to ACP agent (it has the context and inference budget).
Cancelling / restarting in-flight turns	Existing /cancel semantics unchanged. Mid-turn corrections are deferred to a future RFC.
Persisting buffer across pod restarts	Buffer only exists during in-flight turn — restart loses the turn anyway.
Replacing the per-connection mutex	The mutex stays exactly as RFC #78 §2b describes it.

---

Proposed solution

Mechanism

state                    event                              action
────────────────────────────────────────────────────────────────────────
thread idle              M1 arrives                          fire turn 1 with M1 immediately
turn 1 in flight         M2 arrives                          send M2 into channel
turn 1 in flight         M3 arrives                          send M3 into channel
turn 1 completes         (consumer recv wakes)               drain channel → fire turn 2 with [M2, M3]
turn 2 in flight         M4 arrives                          send M4 into channel
turn 2 completes         (consumer recv wakes)               drain channel → fire turn 3 with [M4]
turn 3 completes         (channel empty)                     consumer parks on recv → awaits next message

Two invariants:

1. First message after idle has zero added latency — it fires immediately, just like today's per-message dispatch.
2. At most one in-flight turn per thread — all buffering happens between turns, never within.

State

A new Dispatcher sits above SessionPool in the call graph. Per-thread keying matches the existing thread_id keying from pool.rs:15. Each active thread owns a bounded mpsc::channel (capacity = max_buffered_messages from config, default 10) and a long-lived consumer task that drains it.

struct ThreadHandle {
    tx: mpsc::Sender<BufferedMessage>,
    // consumer JoinHandle held for cleanup on idle eviction
    _consumer: tokio::task::JoinHandle<()>,
}

pub struct Dispatcher {
    per_thread: Mutex<HashMap<String, ThreadHandle>>,
    router: Arc<Router>,           // existing — calls handle_message
    max_buffered_messages: usize,  // from config; channel capacity per thread
}

BufferedMessage carries prompt, extra_blocks, sender_json, trigger_msg, arrived_at, sender_name — i.e. everything handle_message already takes, captured at ingest time. sender_name is a display name used for inline labelling inside merged batched prompts (the <message from="..."> tag — see Packing a batch below); it is not a stable user ID. Full struct in Appendix A.

Algorithm (sketch)

submit(thread_key, thread_channel, adapter, msg, other_bot_present):

1. Acquire dispatcher mutex briefly to look up the per-thread ThreadHandle. If absent, create the channel and spawn a consumer_loop task (capturing thread_channel, adapter, and other_bot_present for the consumer's lifetime); insert the handle.
2. Drop the dispatcher mutex.
3. tx.send(msg).await — returns immediately if the channel has space; parks the calling task if the channel (capacity = max_buffered_messages) is full. Only this submit future blocks; the platform event loop is unaffected because submit runs inside its own tokio::spawn'd task per inbound message.

The extra parameters (over a minimal submit(thread_id, msg)) are needed because the Discord adapter is constructed lazily inside serenity's ready callback — see the rationale comment on Dispatcher::submit in Appendix A.

consumer_loop(thread_id) (one per active thread, lives until the channel closes):

1. rx.recv().await — blocks until at least one message arrives. (First message after idle has zero added latency: recv unblocks immediately when send completes.)
2. try_recv greedily until the channel is empty or batch.len() == max_buffered_messages.
3. Dispatch the batch as one ACP turn via pool.with_connection + session_prompt.
4. Loop back to step 1.

Idle threads: when cleanup_idle evicts a thread, the dispatcher drops its ThreadHandle → Sender drops → channel closes → recv() returns None → consumer exits cleanly. No leader-election race; there is always exactly one consumer per active thread. Full reference implementation in Appendix A.

The new wiring at discord.rs:600 (Phase 1 wires Discord only; slack.rs:909 is a follow-up):

// Before:
tokio::spawn(async move { router.handle_message(...).await });

// After (per-message mode unchanged; batched mode goes through dispatcher):
tokio::spawn(async move {
    dispatcher.submit(
        thread_key, thread_channel, adapter, buf, other_bot_present,
    ).await;
});

Packing a batch into one ACP turn

adapter.rs:131-152 already builds Vec<ContentBlock> from one message. For a batch, the consumer merges all sub-messages into one combined Text block with a leading banner and per-sub-message <message index="N" from="..."> tags, then concatenates each sub-message's non-text extra_blocks in order:

Vec<ContentBlock>:
  Text { "[Batched: 3 messages received during the previous turn — handle as one logical unit]\n\n
         <message index=\"1\" from=\"alice\">\ncan you check the build\n</message>\n\n
         <message index=\"2\" from=\"alice\">\nactually wait\n</message>\n\n
         <message index=\"3\" from=\"alice\">\ncheck the build *and* run e2e tests\n</message>\n" }
  // ...followed by each sub-message's extra_blocks (images, transcripts) in order

Single-message batches (batch.len() == 1) skip the banner entirely and dispatch verbatim, so Batched mode is byte-identical to PerMessage for isolated input.

Only the leading message's sender_json is forwarded to handle_message as the batch's SenderContext wrapper; per-sub-message attribution is carried by the <message from="..."> tags inside the combined text. The trailing message's trigger_msg is used to anchor reactions (Decision #3).

ACP session_prompt accepts Vec<ContentBlock> (connection.rs:426). No protocol change.

---

Why this design

Why doesn't the broker try to group messages semantically?

Any rule the broker could apply to "group" messages — same user, time window, reply chain, attachment proximity — observes only structural signals. Real grouping is semantic: was message N+1 a continuation of N's intent, an unrelated topic, a correction?

The broker has no way to answer that without an LLM. Spawning a small classifier LLM just for this adds cost, latency, and coupling — and the user's ACP session already has the full context and is the right place to make semantic decisions:

Layer	Right responsibility
Broker	Structural buffering (collect during turn, drain at boundary)
ACP agent	Semantic processing (merge, split, prioritize, ask back)

So: the broker does not attempt grouping. It just collects messages that arrive while a turn is running and hands them all to the agent in the next turn. The agent — Claude Code, Cursor, Codex, etc. — handles "user said X, then said Y, then sent a screenshot" naturally. Time ≠ semantics.

Why turn-boundary, not debounce?

Debounce imposes a debounce_ms floor on every message's response latency, even isolated ones. Turn-boundary imposes nothing on isolated messages — the buffer only fills during an active turn, when the user is already waiting on the agent. The agent's turn duration is itself a natural "user is waiting" window, used for free.

Why at the broker layer, not the ACP client?

ACP coding CLIs — Claude Code, Cursor, Codex — consume one turn at a time: each session_prompt is one input → one response. They do not inspect incoming chat traffic and do not batch messages themselves. If multi-message buffering happens anywhere, it has to be at the broker.

Why per-thread, not per-channel or global?

Conversation scope in openab = thread. Per-thread keying matches the existing Arc<Mutex<AcpConnection>> keying (adapter.rs:154-161). Cross-thread merging would conflate independent conversations.

---

Benefits

These fall out of "N messages → 1 turn" — not the primary motivation, but real.

Token cost. Each ACP turn re-sends system + tools + accumulated history + new input. Three sequential turns:

T1 input  = sys + tools + M1
T1 output = R1                       ← may be wasted (e.g. "check build" before "wait")
T2 input  = sys + tools + M1 + R1 + M2     ← R1 re-fed
T2 output = R2
T3 input  = sys + tools + M1 + R1 + M2 + R2 + M3   ← R1, R2 re-fed

vs one batched turn: input = sys + tools + [M1, M2, M3], output = single response.

Saved tokens come from (in descending impact):

1. Wasted intermediate output — turn 1's full output + tool execution before turn 3 supersedes it never gets generated.
2. Redundant tool invocations — tools triggered by stale intent don't run.
3. Intermediate responses re-fed — R1, R2 never become input for a later turn.
4. Prompt cache — one prefix invalidation instead of three.

Latency. Three sequential turns ≈ T1 + T2 + T3 wall-clock; the batched path ≈ T1 + T_batch (M1 fires alone immediately; M2 and M3 merge into one follow-up turn). Leading-message latency is unchanged (M1 still fires immediately); subsequent-message latency drops because no per-message round-trip is paid.

These benefits scale with input fragmentation — exactly the workload this RFC targets. They do not apply to isolated messages (buffer never fills) or to truly unrelated overlapping messages from different users (no "wasted work" component, only prompt-prefix amortization).

Deterministic ordering. Today's same-thread ordering is approximate (tokio::spawn race + Tokio Mutex's not-strictly-FIFO waiter list). With a per-thread mpsc::channel:

Property	Today	After RFC
Same-thread message ordering	Approximate (μs-to-ms scheduler-dependent)	Strict (mpsc FIFO)
Ordering-determining sync point	Per-connection mutex's waker list (held during 30s+ ACP turn)	Per-thread mpsc enqueue (μs handle lookup on dispatcher mutex). The per-connection mutex still wraps each ACP turn but no longer determines message order.
Contention window	Seconds	Microseconds

If paranoid ordering is required, the consumer can sort_by_key(|m| m.arrived_at) after try_recv-greedy-drain. In practice Discord/Slack deliver per-channel events in order; this is overkill but cheap.

---

Tradeoffs

Tradeoff	Cost	Mitigation
Turn 1 may run with stale info if user sends correction immediately after	Real, by design	Cost is one wasted turn — acceptable vs. paying debounce_ms on every message.
Multi-message batches lower the ACP-turn count visible to ops	Real semantic shift	bot_turns ingest counter (slack.rs:672-696) runs before the dispatcher, so per-message loop limits still fire correctly. The downstream "ACP turns" metric shifts to count batches; document it.
/cancel cancels one ACP turn that may now contain multiple user messages	Real	Document: "cancel current ACP work; buffered messages fire next." Add /cancel-all in a follow-up if drop-on-cancel is wanted.
cleanup_idle (pool.rs:295) uses last_active updated only inside session_prompt (connection.rs:430) — buffered-only sessions look idle	Real	submit must touch(last_active) on enqueue.
Buffer fills (max_buffered_messages reached during a long turn)	Real	New submit futures park inside their per-message tokio::spawn'd task until the consumer drains. Per-thread backpressure only — platform event loop and other threads are unaffected. Memory cost = parked-task overhead × stuck-thread count, naturally bounded by human typing rate.
Consumer task panics or exits unexpectedly	Real, low-prob	Stale ThreadHandle.tx survives a dead consumer; the next submit returns SendError. The Appendix A sketch logs and drops the message; Phase 1 production code instead detects SendError, evicts the stale entry, and retries — or_insert_with then re-spawns a fresh consumer. In-flight batch is lost (same blast radius as a pod restart mid-turn).
Behavior change observable to every user of an opted-in channel	Real	Opt-in via mode flag for v1; revisit defaulting after validation.

---

Multi-party interaction & bot ingest strategy

Buffer invariant: by the time a message reaches the buffer, it has already been determined to be intended for our agent. The buffer makes no addressing decisions and needs no awareness of mention rules. All multi-party complexity lives upstream.

Producer-side gates (already in place)

The Slack/Discord adapters apply a chain of filters before any message reaches submit:

Gate	Source	Multi-party role
allow_bot_messages (off / mentions / all)	slack.rs:710-765	Decides whether bot messages enter at all. mentions = bot messages enter only when they explicitly @ us.
allow_user_messages (involved / mentions / multibot-mentions)	slack.rs:768-810	Decides which human messages we should respond to. multibot-mentions requires @us whenever another openab bot is in the thread.
trusted_bot_ids	config	Whitelist for mentions / all modes.
bot_turns consecutive-bot limit	slack.rs:672-696	Loop guard. Per-message at ingest, not per batch.
Eager multi-bot detection	slack.rs:649-657	Sets other_bot_present → triggers multibot-mentions semantics.

Scenarios

(A) Multi-human, single bot — allow_user_messages = "involved". alice and bob both type at us; both pass the gate, both end up in the buffer, drained together. The merged prompt's <message index="N" from="..."> tags distinguish their sub-messages (see Packing a batch into one ACP turn); the agent decides whether to address them jointly or separately.

(B) Multi-bot — allow_user_messages = "multibot-mentions", allow_bot_messages = "mentions". A peer bot and us share a thread. Only messages explicitly @-ing us pass the gate; everything else is dropped before reaching the dispatcher. The buffer sees exactly what was directed at us.

(C) Bot-to-bot coordination — allow_bot_messages = "mentions" + trusted_bot_ids = [peer-bot]. peer-bot's @us messages enter; bot_turns increments at ingest. Batching cannot inflate the bot-turn count — limits fire before submit.

Implications for design

• other_bot_present is a per-thread fact set upstream; the dispatcher does not need to compute it.
• MAX_CONSECUTIVE_BOT_TURNS runs before submit; batching is downstream and cannot bypass it.
• Per-sub-message attribution in a merged batch is carried by the <message from="..."> tag (the leading message's <sender_context>, from adapter.rs:131-152, still wraps the batch as a whole).

No new mode is required. The producer gates already produce the right input:

[platform event] → [gates: addressee + loop decisions] → [Dispatcher.submit] → [batched ACP turn]

---

Decisions (defaults proposed)

These have a clear default; reviewers can push back but the design works as stated. No open questions remain.

1. Bot messages go through the same buffer as human messages. Producer gates already gatekeep what enters; once admitted, a bot message is just another ContentBlock with sender attribution. Bypassing for bots would re-introduce the very fragmentation problem this RFC solves.
2. Bot-turn-limit counts batches as turns (one ACP invocation = one logical turn). The per-message ingest counter is unchanged.
3. Reactions track the trailing (most-recently-enqueued) message of a batch. The leading message may have arrived 30+ seconds ago and already looks "read" to the user; anchoring reactions on the newest message in the batch gives feedback on what the user just typed, which matches user expectation. Older messages in a batch don't get their own reaction stream until a later turn.
4. /cancel cancels the current ACP turn only; buffer is preserved. User can re-/cancel next turn if needed. A /cancel-all flag is a follow-up.
5. Buffer cap configurable, default = 10, full = block (await space). Per-thread bounded mpsc::channel(max_buffered_messages). When full, additional submit futures park inside their own tokio::spawn'd task until the consumer drains. Per-thread backpressure only — the platform event loop and other threads are unaffected. Operators can lower the cap (smaller batches, tighter memory) or raise it (tolerate longer agent stalls before backpressure kicks in).
6. Default mode = per-message for v1. Opt-in to batched per adapter. batched is strictly better for fragmented input and identical for isolated input, but the conservative default keeps the rollout safe; flipping the default is left to Phase 3 after a validation period.

---

Configuration & rollout

Conservative rollout — opt-in mode for v1:

[discord]
message_processing_mode = "per-message"  # default — no behavior change

# Or:
message_processing_mode = "batched"      # turn-boundary batching
max_buffered_messages   = 10             # batched mode only; per-thread channel cap
                                          # default 10; ignored in per-message mode

After a validation period (e.g. one minor release), revisit whether batched should become the default. The only tunable in batched mode is max_buffered_messages — no debounce, no grouper config. The mechanism is one channel + one consumer task per thread.

Sizing max_buffered_messages

The default of 10 covers human-only fragmentation comfortably (typical human typing rate caps at ~3 messages per 30 seconds). For multi-bot collaboration channels, however, peer bots can push burst rates well past that.

In one early production instance, the opt-in batched mode was deployed on a multi-bot channel where two peer bots (a code reviewer and a test engineer) replied to each other and to the primary agent. Sampling the three most-active threads (~300–1000 messages each, accumulated over several days):

Thread	Human msgs (max in 30s / 60s)	Peer-bot msgs (max in 30s / 60s)	All incoming (max in 30s / 60s)
Active project thread (~1000 msgs)	3 / 3	12 / 16	12 / 16
Status report thread (~360 msgs)	2 / 3	11 / 20	11 / 20
Task triage thread (~300 msgs)	2 / 2	24 / 24	24 / 24

Humans alone never exceeded 3 messages in 30s; peer bots drove all of the burstiness. With max_buffered_messages = 10, channel-full backpressure would have engaged repeatedly on these threads (one had ten separate 60s windows with ≥10 incoming messages), splitting bursts into 2–3 turns and partially defeating the batching benefit.

After this sampling, the same deployment raised the cap to 30, sized for ~25% headroom over the largest observed 60s burst (24). At full cap the merged prompt is roughly 3k tokens of batched payload — within reasonable ACP-input range.

Guidance:

• Human-only adapters: 10 is fine.
• Multi-bot adapters: size to the observed peer-bot burst rate, with headroom. In practice 20–50 covers most multi-agent topologies. The trade-off is merged-prompt size at full cap (≈ cap × per-message-tokens).
• Backpressure ≠ data loss: when full, submit parks the calling task per-thread; nothing is dropped. Undersizing only produces "more, smaller batches", not lost messages — so it's safe to start at the default and tune up after observing real burst patterns in dispatch debug logs (batch_size=N distribution).

---

Implementation phases

Phase 1 — Mechanism (single PR)
  - New module: src/dispatch/mod.rs with Dispatcher + ThreadHandle + consumer_loop
  - Add MessageProcessingMode enum to config (default = PerMessage)
  - Hook discord.rs:600: branch on mode
      PerMessage → existing tokio::spawn { handle_message } path (unchanged)
      Batched    → dispatcher.submit(thread_key, thread_channel, adapter,
                                     BufferedMessage, other_bot_present)
    Slack hookup (slack.rs:909) is a deliberate follow-up — Discord covers
    the multi-bot validation target; Slack inherits unchanged per-message
    behavior in the meantime.
  - dispatch_batch packs multiple messages into one Vec<ContentBlock>
  - last_active touch on enqueue (fix cleanup_idle interaction)
  - SendError detection at submit: evict stale ThreadHandle + retry to re-spawn consumer (consumer-panic recovery)
  - Reactions strategy: trailing message anchors batch progress (see Decision #3)
  - Tests:
    - single-message turn (batched mode == per-message for batch_size=1)
    - 3-message fragmentation merges into one batch
    - new message arrives mid-turn, joins next batch
    - /cancel during batched turn does not drop buffer
    - last_active touched on enqueue (cleanup_idle does not evict)

Phase 2 — Validation
  - Roll out to a single channel (e.g. dev sandbox)
  - Compare turn counts, latency distributions, user-reported quality

Phase 3 — Default flip (separate RFC if needed)
  - Make `batched` the default mode
  - Or: remove the mode flag entirely if no real-world reason to keep `per-message`

---

Prior art

Two adjacent systems solve "user typed multiple times in quick succession" with different trade-offs. Both are personal AI agent runtimes (single-user, agent loop bundled into the gateway process) — different shape from openab's multi-tenant broker, but the in-flight buffering problem is the same.

Hermes Agent / OpenClaw / current openab / this RFC

Aspect	Hermes Agent	OpenClaw	Current openab	This RFC
Shape	Single-user runtime, gateway = agent	Single-user runtime	Multi-tenant broker → external ACP CLI	Same as current
First-message latency	~2s (Discord adapter debounce)	n/a observed	0 (immediate dispatch)	0 (preserved)
Adapter-level batching	_pending_text_batches, _text_batch_split_delay_seconds (default 2s) — gateway/platforms/discord.py	n/a observed	None	None (deliberate)
In-flight new message	Single-slot _pending_messages[key] — overwrites prior + interrupt_event.set() cancels in-flight	n/a observed	N independent tokio::spawn tasks each park on per-thread mutex	Send to per-thread bounded mpsc; consumer batches at turn boundary
Buffer data structure	Dict[str, MessageEvent] (1 slot)	—	None (mutex waker list, opaque)	bounded mpsc::channel (FIFO, default cap 10)
3 fast messages → ACP turns	1 turn, middle message dropped by overwrite	—	3 turns, intermediate output wasted	2 turns (M1, then batch [M2, M3]) — no message lost
Mid-turn interrupt	Yes (asyncio interrupt event) — agent loop is in-process	—	No	No
Same-thread message ordering	(1-slot makes this moot)	—	Approximate (Tokio Mutex not strictly FIFO)	Strict (mpsc FIFO)
Per-key serialization	asyncio.Event + _active_sessions dict	stall-watchdog.ts (different concern: stall detection)	KeyedAsyncQueue (per-key Semaphore, Slack) + Arc<Mutex<AcpConnection>>	Inherited
Bot-message gating	DISCORD_ALLOW_BOTS (off / mentions / all)	n/a observed	allow_bot_messages (3-value, borrowed from Hermes)	Inherited
Stall UX feedback	—	stall-watchdog.ts	reactions.rs stall_soft / stall_hard (borrowed from OpenClaw)	Inherited

Three trade-off axes

1. Drop vs preserve. Hermes' single-slot overwrite drops middle messages in fast bursts; openab (current and RFC) preserves all.
2. Interrupt vs wait for boundary. Hermes can interrupt the in-flight LLM call because the agent loop is in-process. openab cannot — the agent is an external ACP CLI that yields control only at turn end. This is an architectural constraint, not a design choice. The RFC turns it into a feature: the existing turn duration is the natural buffering window, with no added latency for isolated messages.
3. Debounce vs piggyback. Hermes' Discord adapter pays ~2s per message regardless. The RFC pays 0 for isolated messages — buffering only fills during an active turn, when the user is already waiting on the agent.

Other prior art

HTTP request coalescing in proxies (Varnish, nginx) — same shape ("while one request is in flight, batch / dedupe others") in a different domain.

---

Appendix A: Reference implementation

This sketch matches the MVP shape in src/dispatch/mod.rs. A few signatures
carry extra parameters not present in earlier drafts of this RFC; the
rationale is inline below.

use std::time::Instant;
use tokio::sync::{mpsc, Mutex};

struct BufferedMessage {
    prompt: String,
    extra_blocks: Vec<ContentBlock>,
    sender_json: String,
    trigger_msg: MessageRef,
    arrived_at: Instant,
    /// Display name for inline batch labelling (the `<message from="...">` tag
    /// inside merged prompts). Not a stable user ID.
    sender_name: String,
}

struct ThreadHandle {
    tx: mpsc::Sender<BufferedMessage>,
    _consumer: tokio::task::JoinHandle<()>,
}

pub struct Dispatcher {
    per_thread: Mutex<HashMap<String, ThreadHandle>>,
    router: Arc<Router>,
    max_buffered_messages: usize,  // from config (default: 10)
}

impl Dispatcher {
    /// `adapter` and `other_bot_present` are passed per-call (rather than stored
    /// on the Dispatcher) because the Discord adapter is constructed inside
    /// serenity's `ready` callback via `OnceLock` — well after the Dispatcher
    /// itself is built in `main.rs`. Per-call passing avoids that chicken-and-
    /// egg without a second `OnceLock` here. In practice all submits for a
    /// given thread share the same adapter Arc; only the first one (which
    /// spawns the consumer) is captured for that thread's lifetime.
    pub async fn submit(
        &self,
        thread_key: String,
        thread_channel: ChannelRef,
        adapter: Arc<dyn ChatAdapter>,
        msg: BufferedMessage,
        other_bot_present: bool,
    ) {
        // Pull these out of `self` before locking so the or_insert_with closure
        // doesn't try to re-borrow `self` while `self.per_thread` is locked.
        let cap = self.max_buffered_messages;
        let router = Arc::clone(&self.router);

        let tx = {
            let mut map = self.per_thread.lock().await;
            map.entry(thread_key.clone())
                .or_insert_with(|| {
                    let (tx, rx) = mpsc::channel(cap);
                    let consumer = tokio::spawn(consumer_loop(
                        thread_key.clone(), thread_channel, rx, router,
                        adapter, cap, other_bot_present,
                    ));
                    ThreadHandle { tx, _consumer: consumer }
                })
                .tx
                .clone()
        };
        // dispatcher mutex released — held only to look up/insert the handle

        // send.await parks the calling task if the channel (cap = max_buffered_messages) is full.
        // Only this submit future blocks; the platform event loop is unaffected
        // because submit runs inside its own per-message tokio::spawn'd task.
        if tx.send(msg).await.is_err() {
            // Consumer has exited — see Tradeoffs row "Consumer task panics".
            // Phase 1 production code evicts the stale ThreadHandle and retries
            // so or_insert_with re-spawns a fresh consumer.
            warn!(thread_key, "consumer task has exited; message dropped");
        }
    }
}

async fn consumer_loop(
    thread_key: String,
    thread_channel: ChannelRef,
    mut rx: mpsc::Receiver<BufferedMessage>,
    router: Arc<Router>,
    adapter: Arc<dyn ChatAdapter>,
    max_batch: usize,
    other_bot_present: bool,
) {
    while let Some(first) = rx.recv().await {
        // Greedy drain up to max_batch messages already enqueued.
        let mut batch = vec![first];
        while batch.len() < max_batch {
            match rx.try_recv() {
                Ok(more) => batch.push(more),
                Err(_) => break,
            }
        }

        // Process batch as ONE ACP turn
        // - if batch.len() == 1: identical to today's single-message dispatch
        // - if batch.len() > 1:  merge into one combined Text block with
        //                        [Batched: N messages...] banner +
        //                        <message index="N" from="..."> tags
        //                        (see "Packing a batch into one ACP turn")
        dispatch_batch(&router, &adapter, &thread_channel, batch, other_bot_present).await;
    }
    // recv() returned None → all senders dropped → cleanup_idle evicted us. Exit cleanly.
}

[github-actions]

Thanks for the report! It looks like this issue was created without a template and is missing some required fields. Please add one of the following labels: bug, feature, documentation, or guidance, then edit this issue to include the matching template fields — the incomplete label will be removed automatically.

[brettchien]

Heads-up to maintainers: I don't have label-management permissions on this repo, so I couldn't satisfy the openab-issue-check bot myself.

Could someone please:

• add the rfc label
• remove the incomplete label

This issue is a design RFC and matches the .github/ISSUE_TEMPLATE/rfc.yml template; the issue-check workflow early-returns for the rfc label, so no further body edits should be needed.

[chaodu-agent]

RFC #580 Community Triage Review

Verdict: Conditional Approval ✅ (unanimous — 4 reviewers)

Summary

Turn-boundary batching with per-thread mpsc::channel, zero-latency first-message, and semantic decisions deferred to ACP is the right approach given openab's architecture (external ACP CLI, no mid-turn interrupt). The RFC is exceptionally well-written — clear problem definition, honest tradeoff table, and production burst-rate data backing the max_buffered_messages default.

What we liked

• Broker does structural buffering only; semantic processing deferred to ACP agent — clean separation of concerns.
• Zero-latency first-message invariant preserved. Isolated messages are byte-identical to current behavior (batch.len() == 1 skips banner).
• Multi-party analysis is thorough — buffer makes no addressing decisions; all gates are upstream.
• Prior art comparison (Hermes / OpenClaw / current / proposed) with three clear trade-off axes.
• max_buffered_messages sizing backed by real production burst-rate sampling (3 threads, 300-1000 msgs each).

Tier 1 — Correctness blockers (must resolve before Phase 1)

1. SendError recovery — align behavior across RFC body text, Appendix A sketch, and Phase 1 test plan. Body says "evict stale handle + retry"; Appendix A only logs and drops; test plan doesn't cover the recovery path.
2. last_active semantics — do not conflate "user sent a message" with "agent successfully processed a turn." Currently submit must touch(last_active) per the tradeoff table, but this creates a zombie risk: if the ACP CLI hangs, continuous enqueue keeps the thread alive forever, exhausting max_sessions. Introduce independent stalled-buffer / dead-consumer handling with a separate indicator for agent-side liveness.
3. other_bot_present freshness — must reflect dispatch-time state, not consumer-spawn-time snapshot. If a new bot joins the thread mid-conversation, the consumer currently uses stale state. Specific implementation (per-message snapshot, Arc<AtomicBool>, or other) is author's choice.
4. Batch packing ordering — merged extra_blocks (images, transcripts) must preserve per-message attribution. Current design flattens all sub-messages' extra_blocks after the combined Text block, breaking the association between a message's text and its attachments (e.g. M1="look at this" + screenshot, M2="listen" + transcript — after flatten, agent can't tell which belongs to which).

Tier 2 — Recommended (not blockers)

• Observability: instrument tracing for buffer_wait_ms / agent_dispatch_ms spans — essential for Phase 2 tuning of max_buffered_messages.
• sender_name disambiguation: display names can collide on Discord. Consider sender_id or name(id) combo in <message from="..."> tags.
• Decision fix: pull --rebase before push in bump-chart #4 use case: add "user sends wrong message, cancels, but wrong message is already in buffer" scenario to motivate the /cancel-all follow-up.
• Slack integration strategy: will Dispatcher replace KeyedAsyncQueue or coexist? Affects API design; worth deciding before Phase 1 lands.
• Discord 6000-char boundary: batched mode produces longer agent output; verify format.rs edit-streaming split logic under larger payloads.
• Consumer re-spawn retry budget: prevent infinite loop if underlying ACP stdio is broken.
• Graceful shutdown: log buffered messages lost on SIGTERM for ops traceability.
• Batch-first reaction UX: consider "received" reaction on first message of a batch to avoid "message eaten" perception (Phase 2).
• Per-channel config override: human-only vs multi-bot channels have very different burst profiles (Phase 2).
• arrived_at_relative offset: in <message> tags, helps agent distinguish rapid-fire from slow correction (Phase 2).

[JARVIS-coding-Agent]

Community Review — Turn-boundary message batching RFC

Reviewed by two independent agents (JARVIS + FRIDAY) at the request of @sssh1204.

Overall Assessment

The RFC is architecturally sound. Turn-boundary batching (not debounce), deferring semantics to ACP, zero latency on first message, and opt-in rollout are all the right calls. The problem statement, prior art comparison, and tradeoff analysis are thorough.

However, we identified three blockers that should be resolved before Phase 1 implementation begins. Priority order matters — #1 must be fixed first as it defines the batch packing format that #2 and #3 build on.

---

Blocker #1: extra_blocks attribution is broken in batch packing (Correctness)

The current packing design flattens all sub-messages into one combined Text block, then appends all extra_blocks sequentially at the end:

Text { M1 + M2 + M3 merged text }
Image { M1's screenshot }       ← agent cannot associate this with M1
Transcript { M2's audio }       ← agent cannot associate this with M2

This breaks the core "late attachment" scenario the RFC itself motivates. The agent has no way to know which image belongs to which <message> tag.

Suggested fix: Each sub-message's extra_blocks should be placed immediately after (or within) its corresponding <message index="N"> tag, not flattened to the end.

---

Blocker #2: other_bot_present is frozen at consumer spawn time (Correctness)

other_bot_present is captured once when the consumer task is spawned (on the first message). If a new bot joins the thread mid-conversation, the consumer never sees the updated value. This can cause incorrect addressing behavior in multi-bot scenarios.

Suggested fix: Move other_bot_present into BufferedMessage as a per-message snapshot. The consumer uses the value from the last message in each batch.

---

Blocker #3: last_active conflates activity and liveness → zombie threads + respawn loop (Design Gap)

Two coupled problems:

1. Zombie threads: submit must touch(last_active) on enqueue (as the RFC notes) to prevent cleanup_idle from evicting threads with buffered messages. But this means a thread where the ACP CLI has crashed — and the consumer has panicked — will never be evicted as long as messages keep arriving.

2. Infinite respawn: The SendError → evict → retry path re-spawns a consumer. If the underlying ACP stdio pipe is broken, the new consumer panics immediately → another retry → infinite loop.

Suggested fix:

• Split into two independent signals: enqueue activity (prevents premature eviction) and consumer health / last successful ACP turn (detects zombies).
• Add exponential backoff + max retry cap to consumer respawn logic.

---

Additional Recommendations (non-blocking)

Item	Suggestion
Metrics	Add batch_size histogram in Phase 1. Without it, Phase 2 validation has no data to compare against.
Dispatcher mutex	per_thread uses tokio::Mutex but the critical section contains no .await. Consider DashMap or std::sync::Mutex to reduce contention under high concurrency.
/cancel UX	/cancel only cancels the current turn; buffered messages still fire next. Users who type "never mind" + /cancel will be surprised. Consider shipping /cancel-all (or --all flag) in Phase 1 rather than deferring.

---

Recommended Next Steps

1. Update the batch packing spec to fix extra_blocks attribution (perf: cache dependency build layer in Dockerfile #1 first — it's the foundation).
2. Add other_bot_present to BufferedMessage (perf: cache deps layer + drop arm64 QEMU build #2).
3. Propose a concrete liveness/activity signal split + respawn backoff (fix: use app bot identity for chart bump commits #3).
4. Then proceed to Phase 1 implementation.

The design direction is right. These are fixable gaps, not fundamental flaws.

[brettchien]

Packing — combined response (Triage T1.4 + JARVIS/FRIDAY B1)

This response folds two independent reviews of the RFC #580 batch packing design into one update:

• Triage review (超渡 / 覺渡 / 普渡 / 擺渡) — T1.4 Batch packing attribution: merged extra_blocks (images, transcripts) must preserve per-message attribution; the original RFC sketch flattened all sub-messages' extra_blocks after the combined Text block, breaking the association.
• JARVIS + FRIDAY second review — B1 extra_blocks attribution is broken in batch packing: the agent has no way to know which image belongs to which <message> tag; suggested fix was to place each sub-message's extra_blocks immediately after its corresponding <message index="N"> tag.

Both findings target the same correctness gap. The reformed packing below is one design that closes both, plus the MVP shake-out we shipped to k9 covering both the per-arrival packing and the multi-message batch concatenation that exercises the boundary semantics end-to-end.

---

Highest guideline — broker structural fidelity

Working through this blocker surfaced a sharper version of the RFC's "broker = structural, ACP = semantic" principle. We propose promoting it to the top invariant for the entire design:

The broker must faithfully preserve structural attribution: each chat-history arrival event (its sender, its text, its attachments) appears in the dispatched batch exactly as received — no merging, no splitting, no reordering, no attachment re-attribution, no heuristic pairing of related-looking messages, no semantic directives injected to instruct the agent how to interpret the input. The broker is a transparent buffer that extends the existing per-message template (<sender_context>...</sender_context>\n\n{prompt}, adapter.rs:131-152) — {prompt} is placed verbatim, broker never parses or transforms its content. Batched mode is just N repetitions of that template concatenated; the <sender_context> block itself opens each new arrival event, no additional wrapper tag is needed.

Note

Scope clarification — inbound Discord field fidelity is broader than RFC580. Today's broker (discord.rs:480-483) extracts only msg.content (text) and msg.attachments from inbound Discord messages. Other fields — embeds[] (including auto-generated link previews), stickers, reactions, reference (reply chain) — are silently dropped. Dispatched ContentBlocks therefore reflect only the fields openab currently ingests; the Highest guideline applies to those fields specifically. Closing the inbound-fidelity gap is out of RFC580 scope and tracked as a follow-up; this RFC inherits the existing ingestion behavior unchanged.

The earlier framing said the broker should not attempt semantic grouping (because it lacks context); the elevated framing says the broker must not, even when a heuristic looks safe — because every transformation the broker applies is information the ACP agent can no longer recover.

This guideline drives the packing design directly: every arrival event is emitted verbatim through the existing per-message template; attachments interleave in arrival order behind their owning <sender_context>. Broker does not pair "see this image" (M1) with the standalone image in M2 even when same-author and adjacent.

---

Reformed packing — extend the existing template, concatenate repetitions

Today's per-message packing at adapter.rs:131-152 already wraps each prompt with <sender_context>\n{json}\n</sender_context>\n\n{prompt}. The existing {json} carries sender_id, sender_name, display_name, channel, channel_id, is_bot, and schema version. {prompt} carries the user's text verbatim — broker does not parse, classify, or transform its content. The agent already knows how to parse this template.

T1.4 / B1 are fixed by repeating the same template N times and concatenating them. Each repetition begins with a fresh <sender_context>...</sender_context> block, which is itself the boundary marker — when a new <sender_context> opens in the stream, the previous arrival event ends and a new one begins. No <message> wrapper, no banner, no index=N, no from="…", no semantic directive — every piece of metadata an agent might want already lives inside <sender_context> JSON.

One additive schema change (already exercised on k9 in the MVP — see shake-out below; lands cluster-wide in Phase 1): each <sender_context> JSON gains a timestamp field (Discord/Slack message creation time, ISO 8601 UTC). This (a) makes adjacent same-author repetitions structurally distinguishable even when other JSON fields would otherwise be byte-identical, and (b) subsumes T2.j (arrived_at_relative offset) — the agent computes any relative offset (typing cadence, rapid-fire vs slow correction) directly from the absolute timestamps; no separate field is needed.

Three-way comparison:

Aspect	Current per-message (adapter.rs:131-152)	RFC MVP (Appendix A "Packing a batch")	Reformed (this response)
Sender attribution	<sender_context> JSON wrapper around prompt	New <message index=N from="..."> attribute (parallel schema invented for the RFC)	Reuse existing <sender_context> JSON verbatim — adds timestamp field, nothing else
Per-batch wrapper	n/a (single message dispatched alone)	One combined Text block: banner + N <message> tags concatenated	One Text block per sub-message + interleaved attachment blocks; no outer wrapper
Banner / semantic framing	n/a	[Batched: N messages…] always emitted for batches	None. Broker injects no banner, no instruction, no metadata beyond what already lives in <sender_context>
Boundary marker	n/a (single message)	<message index=N from="…"> opening tag + matching </message> close	The next <sender_context> opening is the boundary — no separate tag at all
Text extras (transcripts)	Prepended before main text (adapter.rs:138-143)	Flattened at end of ContentBlock array (after combined Text)	Interleaved between this message's <sender_context> and the next, in arrival order
Image extras	Appended after main text (adapter.rs:148-152)	Flattened at end of ContentBlock array	Same as text extras — interleaved in arrival order
Attachment ↔ message link	Implicit (single message — only one possible owner)	Lost — flattened blocks have no tie back to a sub-message (T1.4 / B1 blocker)	Structural by adjacency — attachments belong to the most recent <sender_context> preceding them in the ContentBlock array
batch.len() == 1 vs >= 2 code paths	Baseline (only path)	Two paths (with/without banner-Text combination)	Single uniform path — N=1 is just one repetition of the same template; same code, same output shape

The reformed packing is the smallest delta that closes T1.4 / B1 while removing the new sender-schema (T2.b — "use from= attribute for disambiguation") entirely and folding T2.j into the Phase 1 schema bump.

1. Reuse <sender_context> verbatim, plus add timestamp to each sub-message's JSON. T2.b disambiguation already lives in sender_id; the new timestamp field gives every repetition a unique signature and replaces T2.j.
2. Concatenate repetitions — no extra wrapper tag. The next <sender_context> opening marks the next arrival event; attachment blocks belong to the most recent preceding <sender_context>.

Example. Two messages, each with text + one attachment:

• M1 = "look at this" + screenshot, sender = alice
• M2 = "listen to this" + audio transcript, sender = alice

Vec<ContentBlock>:
  Text  { "<sender_context>\n{...alice's JSON, timestamp=T1...}\n</sender_context>\n\nlook at this" }
  Image { screenshot }                  ← belongs to M1 (most recent <sender_context> preceding it)
  Text  { "<sender_context>\n{...alice's JSON, timestamp=T2...}\n</sender_context>\n\nlisten to this" }
  Text  { transcript content }          ← belongs to M2 (boundary moved when T2's <sender_context> opened)

Concatenated logically (what the agent reads):

<sender_context>
{"schema":"openab.sender.v1","sender_id":"...","sender_name":"alice","display_name":"alice","channel":"discord","channel_id":"...","is_bot":false,"timestamp":"2026-04-26T18:33:19.912Z"}
</sender_context>

look at this
[ImageBlock — screenshot]

<sender_context>
{"schema":"openab.sender.v1","sender_id":"...","sender_name":"alice","display_name":"alice","channel":"discord","channel_id":"...","is_bot":false,"timestamp":"2026-04-26T18:33:23.105Z"}
</sender_context>

listen to this
[TextBlock — transcript content]

Properties:

• {prompt} handling unchanged. Each repetition slots the user's content verbatim into the existing <sender_context>...</sender_context>\n\n{prompt} template. Broker does not parse, transform, or annotate {prompt} — same property the per-message path has had since day one.
• No new tags. <sender_context> is the only structural marker. Opening a new <sender_context> block in the stream marks the start of a new arrival event; the previous one ends. Every metadata field (sender, channel, timestamp) already lives in the JSON; adding wrapper attributes (<message index=N from="..." arrived_at_relative="...">) would re-encode information already present.
• One additive schema change. <sender_context> JSON gains a timestamp field. Existing fields untouched; the agent's existing JSON parsing keeps working. Subsumes T2.j (relative offsets are computed by the agent).
• Each arrival event extends until the next <sender_context> opens or the batch ends. Image / transcript blocks land between two <sender_context> blocks; the most recent preceding <sender_context> owns them. Attribution is structural via array position — mirrors Discord's per-message bubble (text + inline attachments rendered together).
• Multiple attachments per message group naturally — all of M1's images / transcripts sit between M1's <sender_context> and M2's <sender_context>, in arrival order.
• No ACP protocol change. Still just Vec<ContentBlock> with Text and other block types — grouping comes from <sender_context> block positions in the array.

Uniform code path for batch.len() == 1 and batch.len() >= 2. The new packing is a single template emitted N times — no special-case fast path for isolated messages. For batch.len() == 1 the output is one <sender_context>...</sender_context>\n\n{prompt} repetition (with attachments interleaved), structurally equivalent to today's per-message dispatch with two small differences: (a) <sender_context> JSON now carries a timestamp field (additive schema change), and (b) extra_blocks (text transcripts, images) are placed after the message's <sender_context>...{prompt} block in arrival order — rather than today's asymmetric "text-prepended, image-appended" ordering at adapter.rs:138-152. Concretely this means STT voice-message transcripts move from before the <sender_context> block to after it (Scenario D below); broker no longer treats transcripts as a special-case Text block. The boundary rule stays clean: <sender_context> always opens an arrival event, attachments always follow.

---

Scope of attribution — what this packing does and does not claim

The packing preserves structural attribution: which attachment was uploaded as part of which arrival-event (chat message). It deliberately does not attempt semantic attribution: which text refers to which attachment across separate arrival-events. Cross-message linking is exactly the kind of inference that should be left to the ACP agent, which has full conversation context.

Four scenarios that the packing must handle correctly:

(Sender-context JSON is abbreviated below as <sender_context>{alice}</sender_context> etc. for readability — in the real ContentBlock stream it's the full JSON record.)

Scenario A — text and image in the same chat message (e.g. drag-and-drop with caption):

<sender_context>{alice, ts=T1}</sender_context>
look at this
[ImageBlock]

✅ The image follows alice's <sender_context>, no other <sender_context> between → belongs to alice's M1.

Scenario B — text in one message, image in the next, same author (very common human pattern: type the description, then paste / drop the image):

• M1 (alice): "see this image"
• M2 (alice): [image, no text]

<sender_context>{alice, ts=T1}</sender_context>
see this image

<sender_context>{alice, ts=T2}</sender_context>
[ImageBlock]

✅ Broker keeps the structural truth (image arrived as M2, alone). The agent reads identical sender_id on both <sender_context> blocks and trivially infers M1's "this image" refers to M2's attachment. The timestamp delta T2 - T1 reinforces this when M1 and M2 are seconds apart, and disambiguates the two <sender_context> blocks even though their other fields are identical.

Scenario C — fragmented multi-author batch (alice's text → bob's interjection → alice's image):

• M1 (alice): "see this image"
• M2 (bob): "what?"
• M3 (alice): [image, no text]

<sender_context>{alice, sender_id=A, ts=T1}</sender_context>
see this image

<sender_context>{bob, sender_id=B, ts=T2}</sender_context>
what?

<sender_context>{alice, sender_id=A, ts=T3}</sender_context>
[ImageBlock]

✅ The broker does not try to "skip" bob's message or re-link alice's M1 ↔ M3 — that's a semantic decision. The repeated sender_id=A in <sender_context> lets the agent group by stable user ID across non-adjacent messages; bob's interjection is preserved as-is so the agent can decide whether to address it (e.g. answer bob's "what?" while also processing alice's image).

Scenario D — voice-only message in a batch (the existing STT path):

• M1 (alice): "look at this" + screenshot
• M2 (alice): voice-only — msg.content is empty; discord.rs:524 produces a [Voice message transcript]: … Text block in extra_blocks
• M3 (bob): "what?"

<sender_context>{alice, ts=T1}</sender_context>
look at this
[ImageBlock]

<sender_context>{alice, ts=T2}</sender_context>

[Voice message transcript]: hey can we sync about the deploy

<sender_context>{bob, ts=T3}</sender_context>
what?

✅ M2's {prompt} is empty (the line after </sender_context> is blank), and the transcript Text block lands immediately after as M2's first attachment. Behavior change vs. today: in the current per-message path (adapter.rs:138-143) the transcript is prepended before <sender_context> so it reads as if it were the user's typed text. Under the reformed packing the transcript moves to after <sender_context>, owned by M2 like any other attachment. The boundary rule stays clean (<sender_context> always opens; attachments always follow) and the agent still sees the transcript content — just one block down. We accept this regression in non-batched single-message ordering for the sake of one uniform code path. If it proves disruptive in cross-agent smoke, the change is reversible: re-introduce the special case extra_blocks.len() == 1 && prompt.is_empty() and treat the transcript as {prompt} substitute.

The principle (instance of the Highest guideline above): structural truth is non-negotiable, semantic interpretation is deferred. Even if the broker could heuristically pair M1 and M3 (same author + image-less + image-only), doing so would either be wrong sometimes or conceal information the agent might want — and either way violates broker fidelity.

Phase 1 test additions:

• Scenario A — batch with mixed attachments per message: verify each ContentBlock follows the most recent preceding <sender_context> in array order; no orphan attachments before the first <sender_context>.
• Scenario A multi-attach — M1 with two images, M2 with one transcript: both M1 attachments sit between M1's <sender_context> and M2's <sender_context>, in arrival order.
• Scenario B — text-only M1 followed by image-only M2 (same author): two <sender_context> blocks emitted (not collapsed), distinguishable by timestamp; M2's image follows M2's <sender_context> immediately.
• Scenario C — interleaved authors with text/image split: ContentBlock array order preserved verbatim from arrival sequence; <sender_context> sender_id matches per-event author; per-event timestamp strictly increasing.
• Scenario D — voice-only / STT message: empty {prompt} followed by [Voice message transcript]: … Text block; transcript no longer prepends before <sender_context>.
• Cross-agent recognition smoke (OQ1 validation) — batched-mode prompt fixture exercised against Claude Code, Cursor, and Copilot; each must correctly attribute attachments to the correct sender and not collapse adjacent <sender_context> blocks into one logical message.

---

MVP shake-out on k9 (2026-04-27)

We shipped the reformed packing to k9 (openab-work-claude, image agent-base:rfc-batching) in two layered passes within the same day:

• Pass A (06:13 UTC) — single-arrival packing only (the building block: one repetition of the new template). Validated <sender_context> + {prompt} shape and the additive timestamp field against organic Discord traffic.
• Pass B (07:53 UTC) — multi-message batch concatenation extended on top of Pass A. The Dispatcher buffers messages arriving while the agent is busy on the same thread and drains them as a single session/prompt containing N concatenated arrival events. This is the path B1 targets directly.

Together the two passes exercise the full reformed-packing surface end-to-end. The dispatcher-behavior items in the cross-check matrix (T1.4, B1 — next section) graduate from unit-test coverage to running-code coverage; T2.b and T2.j remain structurally subsumed and don't require separate runtime exercise.

What changed in the broker (branch feature/turn-boundary-batching):

File	Change
Cargo.toml	Added chrono = { version = "0.4", default-features = false, features = ["std"] } for Slack epoch-seconds → ISO 8601 conversion.
adapter.rs (Pass A)	SenderContext gains timestamp: String (ISO 8601 UTC). New pack_arrival_event(sender_json, prompt, extra_blocks) -> Vec<ContentBlock> helper extracted as the canonical packer; produces Text { "<sender_context>\n{json}\n</sender_context>\n\n{prompt}" } followed by extra_blocks in arrival order.
adapter.rs (Pass B)	AdapterRouter gains dispatcher: Arc<Dispatcher>; constructor switches to Arc::new_cyclic so the dispatcher can hold a Weak<AdapterRouter> callback. handle_message becomes a thin submit — builds BufferedMessage and calls dispatcher.submit(thread_key, msg). The pool / reactions / stream_prompt body extracts into pub(crate) async fn dispatch_batch(adapter, thread_key, thread_channel, content_blocks, last_trigger_msg, other_bot_present) — the consumer-loop callback. Reaction state and reply target anchor on last_trigger_msg; other_bot_present is the snapshot from the most recent buffered message.
dispatch.rs (Pass B, new)	BufferedMessage (per-arrival payload), ThreadHandle (per-thread mpsc tx + consumer JoinHandle), Dispatcher (per_thread: Mutex<HashMap<String, ThreadHandle>> + Weak<AdapterRouter>). consumer_loop: rx.recv().await for the first message of a turn, then drain ≤ MAX_BUFFERED_MESSAGES − 1 more via non-blocking try_recv, pack_arrival_event each, concatenate into one Vec<ContentBlock>, hand to router.dispatch_batch(...). MVP hardcodes MAX_BUFFERED_MESSAGES = 30; Phase 1 PR makes it configurable. Buffer-full → warn! + drop (no caller backpressure in MVP).
discord.rs	timestamp: msg.timestamp.to_string() — serenity 0.12 Timestamp Display is RFC3339, matches Discord platform message creation time.
slack.rs	New slack_ts_to_iso8601(ts: &str) -> String helper converts Slack ts (epoch-seconds with fractional, e.g. "1701234567.123456") to ISO 8601 with millisecond precision; SenderContext populated with the converted value.
gateway.rs	timestamp: event.timestamp.clone() — gateway emits chrono::Utc::now().to_rfc3339() at receive time. Best-effort semantic for non-Discord/Slack channels: this is gateway-receive time, not the originating platform's message creation time. Documented as such; downstream agents should treat gateway-channel timestamps as approximate.
main.rs (Pass B)	mod dispatch;. AdapterRouter::new now returns Arc<Self> directly (built via Arc::new_cyclic); call site loses the redundant outer Arc::new. Adapter task spawns are unchanged — discord.rs, slack.rs, gateway.rs continue to call router.handle_message(...), the dispatcher is fully transparent to them.

Test coverage (full cargo test = 131 pass, cargo clippy --tests -- -D warnings clean):

• Pass A — 5 unit tests in adapter.rs: pack_typed_text_only, pack_typed_text_with_image, pack_voice_only_transcript_lands_after_sender_context, pack_mixed_extra_blocks_preserve_arrival_order, sender_context_serializes_timestamp_field.
• Pass B — 5 unit tests in dispatch.rs: drain_single_message, drain_coalesces_pending (rapid-fire sends coalesce into one batch), drain_caps_at_max_buffered (overflow stays in channel for next iteration), submit_with_dropped_router_does_not_panic (Weak<AdapterRouter> drop-safety), distinct_thread_keys_get_independent_channels (per-thread map keying).

Live verification on k9 — Pass A (2026-04-27 06:13–06:15 UTC, organic Discord traffic):

K9 broker logs show the new format on every dispatched prompt. Representative acp_send line (request id 18):

<sender_context>
{"schema":"openab.sender.v1","sender_id":"1215509703543492702","sender_name":"brett_0122",
 "display_name":"brett_0122","channel":"discord","channel_id":"1498125646344486932",
 "is_bot":false,"timestamp":"2026-04-27T06:13:17.927Z"}
</sender_context>

有cmd可以檢查嗎？

timestamp confirmed as platform creation time, not broker dispatch time: id 18 (06:13:17.927Z) is earlier than ids 16 (06:13:26.877Z) and 17 (06:13:53.537Z) despite being dispatched later — consistent with id 18 having been enqueued while the agent was busy and drained after.

Live verification on k9 — Pass B (2026-04-27 07:52–07:54 UTC, rapid-fire test):

Dispatcher logs show two-message batches forming when the agent was busy:

07:53:28.179  submit              platform=discord
07:53:35.551  submit              platform=discord       ← agent still busy on prior turn
07:53:35.950  dispatching batch   thread_key=discord:…   batch_size=2  block_count=2
07:53:38.354  submit              platform=discord
07:53:49.367  submit              platform=discord       ← still busy
07:53:52.921  dispatching batch   thread_key=discord:…   batch_size=2  block_count=2

Properties confirmed in the wild:

• ✅ Two arrival events coalesced into a single session/prompt with two concatenated <sender_context> + {prompt} repetitions (block_count == batch_size == 2).
• ✅ Discord UI received exactly one reply per batch (one streaming-edit lifecycle, anchored on the last trigger message), not two — matches log-derived behavior.
• ✅ Schema remains openab.sender.v1; per-event timestamp distinct (07:53:28 vs 07:53:35) so the agent can recover ordering and cadence inside the batch.
• ✅ Quiet-thread path unchanged: prior single submits (07:52:58, 07:53:16, 07:53:26) dispatched immediately as batch_size=1 — no added latency when the consumer is idle.

What this MVP did and did not validate:

• ✅ <sender_context> + {prompt} packing per arrival event.
• ✅ timestamp field carried end-to-end and preserves platform-creation ordering.
• ✅ Schema remains openab.sender.v1 (additive); no consumer migration.
• ✅ Multi-message batch concatenation: rapid-fire ≥ 2 messages while agent busy → 1 session/prompt with N concatenated arrival events; 1 reply target; quiet-thread N=1 path unchanged.
• ⚠️ Attachment / voice-message path not exercised in k9 traffic — covered by unit tests only. First organic voice message or image attachment on k9 should be spot-checked to confirm Scenario A / D ContentBlock ordering matches expectations.
• ⚠️ Cross-author batches (Scenario C — alice → bob → alice) not exercised in production yet; only single-author rapid-fire was tested. Unit tests cover the structural property; an organic occurrence will validate end-to-end.

---

Cross-check — review items closed by reformed packing

Review item	Closed by reformed packing?	Notes
T1.4 Batch packing attribution (Triage)	✅ Fully closed	Each sub-message's extra_blocks interleaves between its own <sender_context> and the next; structural-by-adjacency.
T2.b sender_name disambiguation (Triage)	✅ Subsumed	Existing <sender_context> JSON already carries sender_id; no new from= attribute introduced.
T2.j arrived_at_relative offset (Triage)	✅ Subsumed	Each <sender_context> carries absolute timestamp; agent computes any relative offset from those.
B1 extra_blocks attribution in batch packing (JARVIS/FRIDAY)	✅ Fully closed	Same gap as T1.4 — JARVIS suggested <message index="N"> wrapper; reformed packing achieves equivalent boundary semantics using <sender_context> itself as the boundary marker, without inventing a new wrapper tag. Pass B MVP exercised the multi-message concatenation path on k9 (batch_size=2 ×2, 2026-04-27 07:53 UTC) — B1 is now backed by running code, not just unit tests.

JARVIS / FRIDAY's other two blockers (B2 other_bot_present freshness, B3 last_active split + respawn backoff) and three non-blocking recommendations are addressed separately — they are state-management / observability issues, not packing.

[brettchien]

T1.3 — other_bot_present freshness (Tier 1 blocker — Phase 1 fix)

Standalone response to the T1.3 blocker raised in the community triage review. Other tiers will be addressed in subsequent comments / PRs.

Must reflect dispatch-time state, not consumer-spawn-time snapshot. If a new bot joins the thread mid-conversation, the consumer currently uses stale state.

TL;DR. The flag is captured per-message at the producer (discord.rs:595) and the consumer dispatches a batch using batch.last().other_bot_present (dispatch.rs:119). If a peer bot joins while a batch is buffered, every message in that batch was captured before the cache flip, so batch.last() carries a stale false and the entire batched reply runs in streaming mode — colliding with the peer bot's edit cycle. The flag drives adapter.use_streaming(...), which is evaluated once per turn at adapter.rs:266, so a wrong read is locked in for the whole reply.

Phase 1 fix — mirror the cache into a Dispatcher-owned Arc<AtomicBool>. Per-thread atomic, written by the producer's early-detect path, read by the consumer at dispatch time:

1. ADD other_bot_present: Arc<AtomicBool> to each ThreadHandle in Dispatcher::per_thread. Seeded from the producer's per-message reading on first message; bot_participated_in_thread's history-priming (discord.rs:158-237) already covers the "peer bot posted before any user message" case.
2. ADD Dispatcher::mark_other_bot_present(thread_key) plus a one-line AdapterRouter::mark_other_bot_present forwarder (parallel to the existing handle_message inbound forwarder). Handler::message calls self.router.mark_other_bot_present(&key) from the early-detect path (discord.rs:248-252) right after writing multibot_threads. No-op if no ThreadHandle yet — the next user message seeds via step 1.
3. MODIFY consumer (dispatch.rs:119) to read flag.load(Ordering::Relaxed) immediately before each dispatch_batch, replacing batch.last().other_bot_present.
4. REMOVE BufferedMessage::other_bot_present field (dispatch.rs:28) — dispatch.rs:119 was its only reader; after step 3 the per-message snapshot is unused. The producer's current reading becomes a Dispatcher::submit argument used only to seed a newly created ThreadHandle's atomic.

Why this is small. multibot_threads ownership doesn't move; ChatAdapter trait is unchanged. Handler already holds Arc<AdapterRouter>; we keep Dispatcher encapsulated per existing convention (adapter.rs:131-194 — Dispatcher is a private field, all entry points go through router-level forwarders) and add AdapterRouter::mark_other_bot_present next to handle_message. Slack gets the analogous one-liner at its cache-flip site. Relaxed is sufficient — monotonic single-bit flag, no other state to synchronize. Discord's sticky cache remains the authoritative producer-side detector; the atomic is just a dispatcher-readable mirror.

Detailed walkthrough — 3-turn timeline showing where the race materializes

Notation: M*.bool = the other_bot_present snapshot captured per-message at discord.rs:595.

═══════════════════════════════════════════════════════════════════════
 TURN 1  (M1 alone) — first user message, cold thread
═══════════════════════════════════════════════════════════════════════
T0   User: "summarize this doc"
     ├─ 248-252  user msg, author.bot=false → cache unchanged
     ├─ 595      cache.contains_key = false → M1.bool = false ❄️
     ├─ buffer empty → consumer drains [M1] immediately
     │
     ▼ TURN 1 START — use_streaming(M1.bool=false) = true
       streaming MODE locked (placeholder + edit-loop spawn)

T0–T5  ACP processes M1; openab edits placeholder every 1.5s
       Mid-turn cache / external changes do not re-evaluate the mode

       ───── events below all occur while Turn 1 is in-flight ─────

T1   User: "focus on section 3"
     ├─ 595 cache empty → M2.bool = false ❄️
     └─ ACP busy → buffer = [M2]

T2   User: "and add bullet points"
     ├─ 595 cache empty → M3.bool = false ❄️
     └─ buffer = [M2, M3]

T3   User: "@other_bot can you take a look too?"      ← user tags peer bot
     ├─ 595 cache still empty (peer bot hasn't replied) → M4.bool = false ❄️
     └─ buffer = [M2, M3, M4]

T4   @other_bot posts: "Hello, can I help?"
     ├─ 248-252 author.bot=true, id≠self → multibot_threads FLIP TRUE ⚡
     ├─ peer bot msg gated out, doesn't reach ACP
     └─ M2/M3/M4 already frozen with bool=false in buffer

T5   ACP completes M1
     ▲ TURN 1 END
═══════════════════════════════════════════════════════════════════════

 TURN 2  ([M2, M3, M4] batched) — race victim
═══════════════════════════════════════════════════════════════════════
T5+  Consumer wakes, drains [M2, M3, M4]
     last_other_bot = batch.last().bool = M4.bool = FALSE ❌
     (cache flipped true at T4, but batch.last() carries T3's frozen false)

     ▼ TURN 2 START — use_streaming(false) = true
       streaming MODE locked again ← race victim

T5–T9  openab edit-loop runs on packed [M2+M3+M4] reply
       @other_bot is concurrently streaming its own response in the thread
       Two bots' edit cycles overlap → UI conflict ✗

T9   ACP completes Turn 2
     ▲ TURN 2 END
═══════════════════════════════════════════════════════════════════════

 (idle) — buffer empty, ACP idle
═══════════════════════════════════════════════════════════════════════
T10  User: "also add a conclusion section"      ← first user msg post-flip
     ├─ 595 cache.contains_key = TRUE (T4 flip is sticky)
     │       → M5.bool = TRUE ✅
     ├─ buffer empty → consumer drains [M5]
     │
     ▼ TURN 3 START — use_streaming(M5.bool=true) = FALSE
       send-once MODE locked (no placeholder, no edit-loop)

T10–T11  ACP processes M5; openab accumulates without editing
         No more UI conflict with @other_bot ✅

T11  ACP completes Turn 3
     ▲ TURN 3 END — mode finally correct
═══════════════════════════════════════════════════════════════════════

Turn	Decision input	Mode	Race victim?
1	M1.bool=false (T0 capture)	streaming	no — no peer bot present at decision time
2	M4.bool=false (T3 capture, but T4 cache flip)	streaming ❌	yes — batched reply collides with peer bot
3	M5.bool=true (T10 capture, post-flip)	send-once	no

Race window: from T4 cache flip until the next user message arrives at the producer (T10). Any dispatch_batch in this window uses a frozen snapshot captured before the flip; the entire batched reply runs in the wrong mode.

Edge note — silent joiners. Peer-bot detection only fires on inbound messages (we don't subscribe to per-channel presence events). A bot that joins and never posts remains invisible — same as today, batched or not.

Phase 1 test addition. Producer sets the flag after the last enqueue but before the consumer wakes; assert dispatch_batch sees the post-update value (and therefore use_streaming returns false).

[brettchien]

T2.c — Decision #4 use case + /cancel-all Phase 1 timing (Tier 2 recommendation — accept for Phase 1)

Standalone response to the T2.c Tier 2 recommendation from the community triage review and the matching JARVIS/FRIDAY non-blocking suggestion. Other tiers will be addressed in subsequent comments / PRs.

Triage T2.c — Add "user sends wrong message, cancels, but wrong message is already in buffer" scenario to motivate the /cancel-all follow-up.

JARVIS/FRIDAY (non-blocking) — Consider shipping /cancel-all (or --all flag) in Phase 1 rather than deferring; users who type "never mind" + /cancel will be surprised when buffered messages still fire.

Accept both — ship /cancel-all in Phase 1. Cost is small enough that deferral isn't justified.

Why /cancel-all ≠ /cancel. The two commands cover disjoint ranges of the message lifecycle:

• Within a single batched turn (M1+M2+M3 already packed into one session/prompt, agent busy): /cancel sends session/cancel → the whole turn aborts → all three messages cancelled together. /cancel-all adds nothing here.
• Across a turn boundary (agent on M1's turn; M2+M3 arrive while busy and sit in the mpsc buffer, not yet packed): /cancel only kills M1's in-flight turn ✅. Buffered M2+M3 are untouched → consumer loop sees the turn close and dispatches them as the next session/prompt automatically ❌. The user typed "never mind" + /cancel and the agent re-fires anyway.

/cancel-all closes the second half: drop the buffer first (so M2+M3 vanish), then session/cancel (so M1 dies). Rule of thumb — /cancel = "abort what's running now, let the queue catch up"; /cancel-all = "I changed my mind about everything, including buffered messages you haven't dispatched yet."

Anchor. /cancel lives at discord.rs:626 / :647 / :726-745 and dispatches to pool.cancel_session(thread_key) (acp/pool.rs:275). /cancel-all adds three pieces:

1. Dispatcher::cancel_buffered(thread_key) in src/dispatch.rs — removes the ThreadHandle from the per-thread map and calls handle.consumer.abort(). Aborting the consumer task drops its Receiver; tokio mpsc drops all pending BufferedMessages held in the queue at that moment. Plain drop(handle) would only close the channel for new sends — pending messages would still be drained and dispatched, so the abort is what actually cancels them.

   Housekeeping: the existing _consumer field on ThreadHandle (dispatch.rs:33, currently underscore-prefixed because no caller reads it) is renamed to consumer — otherwise clippy fires "named with leading underscore but used".

2. AdapterRouter::cancel_buffered(thread_key) — a one-line forwarder next to handle_message, keeping Dispatcher encapsulated per existing convention (adapter.rs:131-194). Same pattern used for T1.3's mark_other_bot_present.

3. New cancel-all slash command + handle_cancel_all_command in discord.rs, parallel to the existing /cancel. Handler calls self.router.cancel_buffered(&key) then self.router.pool().cancel_session(&key).

Cost: ~30 lines production + ~20 lines tests, no new deps. Slack path unchanged (Phase 1 Slack stays on KeyedAsyncQueue).

Flow. Thread state at T0: M1 dispatched to ACP and running, M2+M3 buffered in mpsc waiting for the next turn boundary.

T0  user types /cancel-all
    ├─ Discord interaction → handle_cancel_all_command(thread_key)
    └─ defer interaction (avoid 3s ACK timeout)

T1  Dispatcher::cancel_buffered(&thread_key)
    ├─ per_thread.lock()
    ├─ guard.remove(thread_key) → ThreadHandle
    ├─ handle.consumer.abort()                 ← actually cancels the task
    │     consumer's Receiver dropped at next await point
    │     → tokio drops all pending BufferedMessages in the queue
    │     → M2, M3 dropped ✅
    │     → if consumer was inside dispatch_batch.await, that future is
    │       cancelled too (any local `batch` Vec dropped with the frame)
    └─ drop(handle) → drop(tx)                 (cosmetic — task is gone)

T2  pool.cancel_session(&thread_key)
    └─ writes session/cancel to ACP stdin
       → ACP aborts M1's turn
       → SessionPool tears down ACP-side state ✅
       (consumer task already aborted at T1; nothing to clean on Rust side)

T3  edit_response → "🛑 Cancelled."

Why abort before session/cancel. Reverse order has a sliver race: session/cancel first → consumer's dispatch_batch.await returns with cancelled error → consumer re-enters its drain loop → pulls M2+M3 from mpsc into a fresh batch → may even start a new dispatch_batch before our abort lands. Aborting first kills the task at whatever await point it sits on (recv or dispatch_batch), so the Receiver drops with M2+M3 still pending in the queue — atomically discarded.

Race disclosure. /cancel-all is mostly atomic under the abort-based design — the residual races are tiny µs-windows around T0:

• Still in mpsc queue → ✅ dropped when the consumer task is aborted at T1 (Receiver drop discards pending messages).
• Already pulled into the consumer's local batch Vec but not yet dispatch_batch'd → ✅ also dropped — abort unwinds the task's stack frame, including the local Vec.
• Already inside dispatch_batch.await (ACP has the prompt, awaiting response) → ✅ Rust-side future cancelled by abort at T1; ACP-side turn aborted by session/cancel at T2 — same surface as the existing /cancel.
• Producer race: a message try_send'd concurrently with T1 that lands in the queue just before the Receiver drops is silently dropped. Acceptable — the user typed /cancel-all, so silent drop matches intent.
• New message arriving mid-flow (between T1 and T3): after T1 the per-thread map entry is gone; a producer calling submit(thread_key, M4) in this window creates a brand-new ThreadHandle (fresh mpsc + fresh consumer). Once T2's session/cancel frees ACP, the new consumer dispatches M4 as its own turn. Semantics: a message sent after the user typed /cancel-all is intentionally outside the cancellation scope — "I changed my mind, and now I'm starting over."

The motivating scenario from Triage T2.c is the second bullet of "Why /cancel-all ≠ /cancel" above.

[brettchien]

T1.1 + T1.2 — SendError recovery & last_active semantics

Standalone response to the T1.1 and T1.2 Tier 1 blockers from the community triage review. With T1.3 (#issuecomment-4329250043) and T1.4 (#issuecomment-4325645814) already posted, this comment closes the Tier 1 set; remaining tiers will be addressed in subsequent comments / PRs.

Anchor pinning. Source line refs in this comment (adapter.rs, connection.rs, pool.rs) are anchored to released base v0.8.2-beta.1 (52052b8) — the version this comment aligns Phase 1 against.

---

T1.1 — SendError recovery alignment

Body says "evict stale handle + retry"; Appendix A only logs and drops; test plan doesn't cover the recovery path.

T1.1 is a surface introduced by RFC #580's architectural choice (per-thread consumer task owning the Receiver half), not a pre-existing concern — 0.8.2-beta.1's per-message direct-dispatch path has no analogous failure mode. Phase 1 PR introduces the surface; this comment settles the disposition.

Accept (P1) — adopt 0.8.2-beta.1's early-error philosophy. Released code's two failure paths both surface to user and return Err rather than self-heal: pool.get_or_create Err (adapter.rs:182-189) emits ⚠️ {format_user_error(...)} then return Err(e); stream_prompt's Err handler in handle_message (adapter.rs:212-234) emits ❌ on the trigger message plus ⚠️ {e} then return Err(e). Phase 1 SendError combines both halves: stream_prompt's response shape (❌ + ⚠️ + return Err) with pool.get_or_create's format_user_error filter for the text body, plus eviction (the new piece, gated by the generation counter — see Race-safe eviction below).

Supersedes prior wording. RFC body's evict + retry and Appendix A's log + drop are both replaced by evict + react + ⚠️ text + return Err. RFC body stays as-is per the comment-as-amendment pattern; this comment carries the canonical SendError spec, superseding Appendix A's reference-implementation sketch (preserved as historical record).

Action chain on SendError:

1. Evict the stale ThreadHandle from per_thread — next submit constructs a fresh consumer lazily.
2. reactions.set_error() on msg.trigger_msg — ❌ anchored to the failing arrival.
3. adapter.send_message(thread_channel, format!("⚠️ {}", format_user_error(...))) — actionable description, reuses the existing helper (no new user-facing strings).
4. return Err(e) to the caller.

No in-call retry, no auto-respawn-and-replay.

Why this shape — both signals, no auto-retry.

Why both reaction and text — they cover different questions. In per-message dispatch (released 0.8.2-beta.1), stream_prompt's reaction and ⚠️ text were partly redundant — both pointed at the one in-flight message, so either signal alone would have attributed the failure. In batched dispatch with rapid-fire M1/M2/M3, the redundancy disappears; each signal carries distinct load:

• ❌ reaction on msg.trigger_msg answers "which message failed?" — anchored to a specific message ID, survives even if the user has scrolled or sent more messages since.
• ⚠️ {format_user_error(...)} text reply answers "why did it fail / what should I do?" — gives the actionable description (e.g. internal error code surfaced through the same filter that scrubs pool.get_or_create errors today).

Both signals exist in 0.8.2-beta.1's vocabulary — stream_prompt provides the (reaction + text + return Err) shape, pool.get_or_create provides the formatter. Nothing introduced here that the released code doesn't already use.

Why no auto-retry — three reasons converge.

• Consistency with the rest of the broker. Released code's contract with the user is "if dispatch fails, you see ⚠️ and re-send if you still want." Phase 1 inherits that contract instead of inventing a parallel "broker silently retries up to N times then maybe surfaces" path that other openab error sites don't have.
• SendError is rare and informative. The only way it surfaces is consumer task death (panic inside dispatch_batch or its callees, or process tear-down). That's a real bug — surface and log is the right response, not papering over with retries.
• No spin-loop possible. With no retry path, JARVIS/FRIDAY B3's "SendError → evict → retry on permanently broken stdio" scenario cannot materialize. T2.f's retry budget and the suggested exponential backoff both dissolve (see T2.f).

What stays unchanged — two independence properties.

• Buffer-full path independent. tx.send().await keeps parking on full — only the receiver-closed Err (consumer-death conditions listed above) is the branch this comment redirects.
• ACP-side state independent. SessionPool is independent of Dispatcher; the next submit (which constructs a fresh consumer) reuses the same thread_key — the next session/prompt lands on the same ACP session, and whatever partial state the dead turn left in ACP carries over unchanged.

No silent drop — scoped to new submits. The broker's no-silent-drop discipline (cf. ADR §8.1 for the packing analog) applies to messages whose submit call still has a return path: those get ❌ on the message itself and ⚠️ text in the channel and Err propagated to the caller. Two losses fall outside this scope, identical to the RFC body's existing residual blast radius (pod-restart-equivalent — same surface as today):

• In-flight batch in the dead consumer's frame. Lost when the panic unwinds. These messages can't be reacted from the SendError path because their submit already returned Ok before the consumer died — there's no live caller frame holding the trigger ID to react on. (A future supervisor catching consumer-task panic could iterate the in-flight batch and react ❌ on each; out of Phase 1 scope.)
• Already-enqueued mpsc messages (in the queue but not yet drained into a batch). Lost when Receiver drops. Same shape as above — submit already returned Ok, no signal to act on.

Concurrency edge cases — two sources of overlap.

Race-safe eviction (concurrent SendErrors on the same handle). Two submit calls can observe SendError on the same handle concurrently; eviction happens under the per_thread lock with a generation counter on ThreadHandle — only the first observer evicts. The second observer takes the lock and either (a) finds the entry gone, or (b) finds the entry already replaced by a third concurrent submit (newer generation than its own captured tx); in both cases its tx is the stale one, so it surfaces the error without re-evicting. Reconstruction is always lazy — the next submit after eviction creates a fresh handle through the normal map-insert path, not the SendError handler. Each observer reacts on its own arrival message (different msg.trigger_msg IDs) and produces its own ⚠️ text — concurrent failures yield concurrent (reaction + text) pairs on the right targets, no cross-attribution. (⚠️ text per failed message rather than per dead handle: the ⚠️ describes that submit call's outcome, so 1:1 with reactions matches stream_prompt's 1:1 pairing.)

Disjoint from /cancel-all (T2.c). cancel_buffered removes the handle from per_thread before aborting the consumer (T2.c lifecycle T1), so any fresh submit arriving after T1 sees no handle and lazily constructs a new one — no SendError on that path. Producers already parked in tx.send().await when T1 lands wake with Err and re-enter SendError recovery, but find the entry already gone and take the (a) branch above (no re-evict; ❌ + ⚠️ still fire 1:1 on each producer's own trigger message). The two paths exercise different code by design.

Phase 1 test additions:

• Consumer task panics mid-batch; next submit observes SendError, evicts the stale handle, sets ❌ on msg.trigger_msg, sends ⚠️ {format_user_error(...)} to the thread channel, returns Err. The subsequent submit lands on a freshly constructed handle and succeeds.
• Two concurrent submits observe SendError on the same handle; only one eviction happens; each call produces its own (reaction + ⚠️ text) pair on its own trigger_msg (no cross-attribution, no double-eviction).
• Buffer-full path still parks (no Err surfaces, no reaction, no ⚠️ text); regression guard for the parking-not-dropping invariant.

---

T1.2 — last_active semantics

Currently submit must touch(last_active), but this conflates "user sent a message" with "agent successfully processed a turn." If the ACP CLI hangs, continuous enqueue keeps the thread alive forever.

T1.2 is a pre-existing bug in 0.8.2-beta.1, not introduced by RFC #580 — but axis-2 below shows batching enlarges its user-visible blast radius, so Phase 1 must keep it no-worse-than-released even while deferring the actual fix.

Mechanism. The single last_active: Instant lives on AcpConnection (connection.rs:120) and is touched at the start of session_prompt() (connection.rs:430, before ACP responds) and again on prompt_done() (connection.rs:468); both run inside stream_prompt's (adapter.rs:238) pool.with_connection lock guard. Pool cleanup_idle evicts when last_active < cutoff || !conn.alive() (pool.rs:315); conn.alive() only flips false when the reader loop exits (connection.rs:476-477), so a hung-but-alive ACP process is invisible to it. The actual zombie mechanism isn't last_active staleness at all — it's cleanup_idle's try_lock on the connection (pool.rs:312-313, "A busy session is not idle"): the lock attempt sees the in-flight task holding the mutex while await-ing the hung ACP and skips the candidate before the predicate is even evaluated — every cleanup round, regardless of how stale last_active would have been.

Severity comparison — RFC #580 doesn't make the zombie survive longer, but it does enlarge the user-visible blast radius. Two axes:

• Axis 1 — zombie's own lifetime (same in both modes). The connection mutex is held by stream_prompt.await (the pool.with_connection lock guard, which spans session_prompt.await) in either model; cleanup_idle.try_lock skips it identically; the slot stays occupied until the ACP process is killed externally or the holder task finally exits (releasing the mutex). Dispatch mode does not change this.
• Axis 2 — user-visible blast radius (worse under batching). In per-message dispatch, the second user message after a hang immediately blocks at the per-thread connection lock — the user sees "no reply" within seconds and stops sending. In batched dispatch, submit returns Ok instantly while messages accumulate in the per-thread mpsc buffer (capacity = max_buffered_messages); the user keeps typing under the impression each message was received. When the consumer eventually dies (panic) or the AcpConnection is force-evicted, the entire mpsc buffer is dropped — up to max_buffered_messages user messages disappear at once, versus ≤1 in per-message dispatch. Failure signal also lands later: T1.1's ❌+⚠️ only fires once a submit observes SendError, by which point the previously-buffered messages have already been silently accepted and lost.

This axis-2 escalation is why the follow-up issue can't sit indefinitely in backlog — a pre-existing latent bug becomes a much louder failure mode under the new dispatch shape.

Disposition — defer to dedicated follow-up issue; Phase 1 retracts only.

1. Explicitly retracts RFC body's submit→touch(last_active) proposal. If wired, the proposal would have amplified the existing conflation by refreshing last_active on every user message — exactly the reviewer's "continuous enqueue keeps the thread alive forever" framing. Phase 1 drops it: submit does not touch last_active; only the per-batch session_prompt call does, preserving 0.8.2-beta.1's existing semantics. Retraction is sufficient for Phase 1 because it only needs to prevent the proposed amplification; the underlying try_lock-skip is dispatch-mode-independent and remains as a pre-existing bug to be addressed in the follow-up.
2. Defers the indicator split + cleanup_idle rework to a dedicated follow-up issue. The fix touches pool.rs eviction semantics (the try_lock-skip in cleanup_idle) plus indicator-split design (broker-side ThreadHandle vs connection-side AcpConnection, each with distinct performance and correctness trade-offs) — large enough that bundling it into RFC RFC: Turn-boundary message batching #580's PR would expand scope beyond turn-boundary batching, diluting review attention on both the batching feature and the eviction-semantics fix. The follow-up issue will be filed alongside the Phase 1 PR and cross-referenced with New @mention in main channel ignored while existing session is waiting for ACP response #307; design + tests get their own review pass.

Existing related work — none of which closes T1.2:

• Session pool leaks memory: orphaned kiro-cli processes and no eviction #309 / PR fix: process group kill + session suspend/resume via session/load #310 (closed 2026-04-13) — process-group kill + session suspend/resume via session/load. Fixes the consequence (orphaned kiro-cli-chat grandchildren after eviction; pool-full-rejection UX) but eviction still keys on the same single last_active.
• fix: pool write lock held during entire prompt streaming causes cross-session deadlock #58 (closed) — pool write-lock held during entire stream_prompt causing cross-session deadlock. Fixed via lock-granularity refactor; last_active semantics untouched.
• RFC: Session Management — Design Proposal #78 (open RFC) — Session Management design proposal; §1b proposes idle_timeout_minutes vs session_ttl_hours split, which is duration-axis layering, not the user-vs-agent indicator split T1.2 calls for.
• New @mention in main channel ignored while existing session is waiting for ACP response #307 (open) — "new @mention in main channel ignored while existing session is waiting for ACP response" — adjacent symptom of the same try_lock blocker; would partially benefit from a T1.2 fix.