docs: ADR for Ambient Mode#1211
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
added 6 commits
June 26, 2026 13:25
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.
Collaborator
|
move to docs/adr/ |
added 5 commits
June 26, 2026 23:13
- 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)
thepagent
approved these changes
Jun 26, 2026
This comment has been minimized.
This comment has been minimized.
chaodu-agent
added a commit
that referenced
this pull request
Jun 27, 2026
- 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
This comment has been minimized.
This comment has been minimized.
thepagent
pushed a commit
that referenced
this pull request
Jun 27, 2026
* 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>
Collaborator
Author
|
LGTM ✅ — Well-structured ADR with thorough prior-art analysis, clear architectural diagrams, and strong fail-safe defaults. What This PR DoesAdds an Architecture Decision Record for Ambient Mode — a batch-flush strategy that allows agents to passively listen to channel messages and autonomously decide whether to respond, without requiring explicit How It WorksThe ADR proposes a per-channel buffered dispatch path parallel to the existing mention-triggered dispatcher:
Findings
What's Good (🟢)
Baseline Check
Note: PR description references |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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