docs: ADR for Ambient Mode#1211
Conversation
This comment has been minimized.
This comment has been minimized.
Resolved: - [口渡 #1] channels empty = disabled (fail-safe), not all channels - [口渡 #2] Buffer lifecycle: swap-and-drain model, flush clears buffer - [口渡 #3] @mention triggers immediate flush + separate normal dispatch - [口渡 #4] Dedicated ambient session pool, isolated from main pool - [口渡 #5] Bot echo prevention: own messages never enter buffer - [口渡 #6] max_concurrent_flushes cap for global rate limiting - [口渡 #9] context_window = Discord API fetch, clarified semantics - [口渡 #10] Error handling table added (timeout, tool calls, etc.) - [核渡 #1] OpenClaw described as cross-platform group chat feature - [核渡 #2] Hermes backfill scope clarified (mention-only, skips free-response) - [核渡 #3] Added Hermes inline reply + per-user session detail - [覺渡 #1] Race condition resolved: swap-and-drain buffer model - [覺渡 #2] Session strategy fully specified (separate pool, rolling window) - [覺渡 #3] Mention detection fires before buffer, reuses existing logic - [覺渡 #4] flush_hard_cap = 50 as safety cap - [覺渡 #5] ±20% jitter on flush interval to prevent thundering herd - [覺渡 #6] context_window semantics clarified (API fetch, not buffer)
Ambient Mode reuses the Dispatcher, BufferedMessage, consumer_loop, and pack_arrival_event infrastructure from PR #686 (message_processing_mode). Key difference: ambient consumer uses timer-based flush instead of turn-boundary drain.
[覺渡 R2#1] flush_semaphore.acquire() before dispatch [覺渡 R2#2] match on dispatch result, warn + discard on error
… default)
[擺渡 F1] @mention discards ambient buffer instead of flushing it;
cancel in-flight ambient on mention arrival
[擺渡 F4] allow_bot_messages defaults to 'off' for ambient channels
[口渡 R3#1] Unify Message Filtering wording with Immediate flush section
This comment has been minimized.
This comment has been minimized.
|
move to docs/adr/ |
- Architecture Overview: full message routing decision tree - Dual-Path Concurrency: timeline showing mention vs ambient interaction - Shows buffer lifecycle, flush phases, and response routing
- Group all ambient settings under [ambient] instead of scattered [discord.ambient], [pool.ambient], [ambient.limits] - Platform-specific config in [ambient.discord] (extensible to slack/telegram) - Simplify pool key names: remove redundant ambient_ prefix (session_ttl_minutes, context_flushes)
Addresses external reviewer feedback from @thepagent and team consensus. docs/steering/ is for process guides; docs/adr/ is for ADRs.
Fixes: - Unify @mention handling path (discard buffer, not flush) — resolves 3 contradictions across flush triggers table, concurrent reply section, and implementation details - Replace non-existent MAX_CONSECUTIVE_BOT_TURNS with actual max_bot_turns - Explain flush_hard_cap vs flush_max_messages relationship - Add post_guard for atomic check-and-post (TOCTOU race fix) - Add flush_timeout_seconds for AtomicBool safety recovery - Reconcile Buffer Lifecycle (conceptual) with mpsc channel (implementation) - Remove contradictory 'extend enum' wording from message_processing_mode - Add allow_bot_messages to [ambient.discord] config - Simplify pool key names (drop ambient_ prefix)
This comment has been minimized.
This comment has been minimized.
- flush_interval_seconds=0 no longer panics (gen_range on empty range) → clamped to .max(1) at runtime - flush_max_messages=0 no longer defeats batching → .max(1) guard - flush_hard_cap=0 no longer panics (mpsc::channel(0)) → .max(1) - flush_timeout_seconds clamped to [5s, 600s] to prevent lockout - Document [NO_REPLY] not yet wired as accepted v1 limitation - Document shared DispatchTarget (tool access) as accepted v1 risk - Document config location deviation from ADR #1211
|
LGTM ✅ — Comprehensive ADR with clear batch-flush architecture, thorough prior art analysis, and solid integration plan with existing Dispatcher infrastructure. What This PR DoesProposes Ambient Mode — a batch-flush mechanism allowing agents to passively listen to channel messages and autonomously decide whether to respond via How It WorksA per-channel bounded Findings
What's Good (🟢)
Addressing External Reviewer Feedback@thepagent (Round 1)
✅ Addressed: File is now at Baseline Check
|
* feat(ambient): implement ambient mode batch flush dispatcher Implements the core Ambient Mode feature as described in the ADR (docs/adr/ambient.md). This adds passive channel listening with a batch flush strategy — messages accumulate in a per-channel buffer and are flushed to the LLM when a time or count trigger fires. Key components: - AmbientConfig: [ambient], [ambient.pool], [ambient.discord] sections - AmbientDispatcher: per-channel mpsc + consumer task management - ambient_consumer_loop: timer/count flush triggers with ±20% jitter - FlushingGuard: RAII + safety timeout for AtomicBool flag - PostGuard: atomic check-and-post to prevent TOCTOU double-reply - Global semaphore (max_concurrent_flushes) for cost control - [NO_REPLY] sentinel detection (is_no_reply helper) Integration: - Discord Handler routes non-mentioned messages in ambient channels to AmbientDispatcher instead of dropping them - @mention in ambient channel discards buffer + cancels in-flight flush - Reactions suppressed for ambient dispatches - Separate session pool (ambient:discord:<channel_id>) Fail-safe defaults: - enabled = false (must opt-in) - channels = [] (explicit allowlist required) - allow_bot_messages = false (prevents echo loops) - flush_timeout_seconds = 120 (auto-recover from stuck state) * fix(ambient): resolve clippy lints (new_without_default, too_many_arguments) * fix(review): address PR review findings F1-F3 - F1: Consumer now drains rx buffer when post_guard is cancelled, preventing stale messages from flushing on next cycle - F2: Add unit tests for is_no_reply and PostGuard - F3: display_name.clone() → .to_owned() for clarity * fix(review): critical post_guard.reset() race + dead code annotations - F1 Critical: Move post_guard.reset() AFTER the can_post() check. Previously reset() cleared cancellation before checking it, making discard_buffer() a no-op in the race window. - F2: Add doc note that [ambient.pool] config is parsed but not yet enforced (v2 follow-up). - F4: Mark is_flushing() as #[allow(dead_code)] with v2 note. * fix(review): config validation guards + document accepted v1 limitations - flush_interval_seconds=0 no longer panics (gen_range on empty range) → clamped to .max(1) at runtime - flush_max_messages=0 no longer defeats batching → .max(1) guard - flush_hard_cap=0 no longer panics (mpsc::channel(0)) → .max(1) - flush_timeout_seconds clamped to [5s, 600s] to prevent lockout - Document [NO_REPLY] not yet wired as accepted v1 limitation - Document shared DispatchTarget (tool access) as accepted v1 risk - Document config location deviation from ADR #1211 * fix(review): permanent cancel block + remove dead sender_json field - Move post_guard.reset() to loop start (after first msg received). Fixes permanent block where one cancellation disabled all future flush cycles for that channel. - Remove dead AmbientMessage.sender_json field (never populated). - Flow: reset → accumulate → check(1) → build → check(2) → dispatch Both checks catch mid-cycle cancellations; reset prevents stickiness. * fix(review): guard max_concurrent_flushes=0 against semaphore deadlock Semaphore::new(0) would block all flush operations permanently. Apply .max(1) to ensure at least one permit is always available. * fix(review): remove try_recv drain to prevent data loss Messages buffered after a @mention but during semaphore wait are valid for the next batch cycle. Draining them caused silent data loss in the narrow window where semaphore is saturated + mention arrives + new non-mention messages enter the buffer. Now: cancel discards the current batch only; remaining buffered messages naturally enter the next cycle after reset(). * fix(review): filter [NO_REPLY] before delivery via AmbientCaptureAdapter Introduces AmbientCaptureAdapter — a ChatAdapter wrapper that: 1. Forces non-streaming mode (use_streaming = false) so the full response text is collected before send_message is called. 2. Intercepts send_message/send_message_with_reply to check is_no_reply() and suppress the sentinel before it reaches Discord. This prevents the [NO_REPLY] sentinel from being posted to the channel, which was the expected common-case behavior (most ambient batches will produce no-reply since the channel is mostly idle chat). Without this fix, ambient mode would spam [NO_REPLY] as literal messages on every flush where the agent has nothing to contribute. * docs(ambient): add architectural rationale for standalone module - Explain why ambient.rs is separate from Dispatcher (no trigger_msg, no streaming, no Lane mode, NO_REPLY filtering needs differ). - Mark context_window as 'not yet implemented (v2)' in config doc. * docs(ambient): fix stale drain comment (drain was removed in 53a556a) --------- Co-authored-by: chaodu-agent <chaodu-agent@openab.dev> Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Summary
Add an Architecture Decision Record (ADR) for Ambient Mode — a new feature that allows agents to passively listen to all channel messages and autonomously decide whether to respond.
Key Design Points
@mention)[NO_REPLY]when it has nothing to add; OpenAB discards silently[discord.ambient]section inconfig.tomlTrade-offs
File
docs/steering/adr-ambient-mode.md