feat(ambient): implement ambient mode batch flush dispatcher by chaodu-agent · Pull Request #1217 · openabdev/openab

chaodu-agent · 2026-06-26T23:32:13Z

Summary

Implements the core Ambient Mode feature — passive channel listening with a batch flush strategy. Messages in configured channels accumulate in a per-channel buffer and are flushed as a batch to the LLM when a time or count trigger fires.

Based on ADR: #1211

Key Design Points

Per-channel mpsc::channel + consumer task (lazy spawn)
Flush triggers: timer (flush_interval_seconds ± 20% jitter) OR count (flush_max_messages)
[NO_REPLY] sentinel — agent replies exactly this when it has nothing to add; response is discarded
FlushingGuard (RAII + safety timeout) prevents permanent channel lockout on panic
PostGuard (atomic cancel) prevents TOCTOU race between ambient flush and @mention dispatch
Global Semaphore (max_concurrent_flushes = 3) controls LLM cost
Separate session pool (ambient:discord:<channel_id>)

Configuration

[ambient]
enabled = false
flush_interval_seconds = 60
flush_max_messages = 10
flush_hard_cap = 50
context_window = 20
max_concurrent_flushes = 3
flush_timeout_seconds = 120

[ambient.pool]
max_sessions = 5
session_ttl_minutes = 60
context_flushes = 3

[ambient.discord]
channels = ["1490282656913559673"]
allow_bot_messages = false

Files Changed

crates/openab-core/src/config.rs — AmbientConfig, AmbientPoolConfig, AmbientDiscordConfig
crates/openab-core/src/ambient.rs — AmbientDispatcher, consumer loop, guards
crates/openab-core/src/discord.rs — Route non-mentioned msgs to ambient, discard buffer on @mention
crates/openab-core/src/lib.rs — Module declaration
src/main.rs — Construct AmbientDispatcher from config

What is NOT in this PR (v2 follow-ups)

Response capture mode for [NO_REPLY] filtering before post (currently relies on system prompt)
context_window history fetch from Discord API
Cross-flush memory (rolling window of previous flush interactions)
Per-channel flush rate limiting (min_flush_interval_seconds)

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)

…uments)

- 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

- 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.

- 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

- 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.

Semaphore::new(0) would block all flush operations permanently. Apply .max(1) to ensure at least one permit is always available.

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().

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.

- 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.md: complete guide (config, behavior, limitations, example) - docs/config-reference.md: add [ambient] section - docs/discord.md: add Ambient Mode section with quick-start

chaodu-agent · 2026-06-27T11:30:28Z

LGTM ✅ — Well-engineered ambient mode implementation with robust concurrency safety and fail-safe defaults.

What This PR Does

Implements passive channel listening ("ambient mode") that buffers non-@mention messages per-channel and flushes them as a batch to the LLM on time/count triggers. The agent autonomously decides whether to respond; [NO_REPLY] sentinel is intercepted before delivery.

How It Works

Per-channel mpsc::channel with lazy-spawned consumer task
Dual flush triggers: timer (±20% jitter) OR message count
FlushingGuard (RAII + safety timeout) prevents permanent channel lockout
PostGuard (atomic cancel) prevents TOCTOU race with @mention dispatch
AmbientCaptureAdapter forces non-streaming and intercepts [NO_REPLY] pre-delivery
Global Semaphore caps concurrent LLM calls across all channels
Safe defaults: enabled = false, empty channel allowlist, bot messages excluded

Findings

#	Severity	Finding	Location
1	🟢	Excellent defensive coding — all config edge cases guarded with `.max(1)` / `.clamp()` preventing panics from `gen_range`, `mpsc::channel(0)`, and semaphore deadlock	`ambient.rs:299-312`
2	🟢	Clean architectural separation from Dispatcher with thorough rationale documentation	`ambient.rs:1-26`
3	🟢	Good test coverage for `is_no_reply` (exact, whitespace, rejects) and `PostGuard` lifecycle	`ambient.rs:588-631`
4	🟢	Safe-by-default config prevents accidental activation or echo loops	`config.rs:1225-1354`
5	🟢	`PostGuard.reset()` correctly placed at loop start (after first msg), preventing both permanent cancellation block and race window	`ambient.rs:290-293`
6	🟢	v1 limitations clearly documented with explicit v2 follow-up scope	`docs/ambient.md:72-82`

Baseline Check

PR opened: 2026-06-26
Main already has: No ambient-related code
Net-new value: Complete ambient mode feature — buffer management, flush dispatcher, concurrency control, Discord integration, capture adapter, documentation

What's Good (🟢)

Multiple rounds of review already hardened this PR (8 fix commits addressing race conditions, config validation, data loss, and the capture adapter)
Standalone module avoids polluting the existing Dispatcher path with ambient-specific branches
Documentation is comprehensive: user guide, config reference, architectural rationale in code comments
AmbientCaptureAdapter elegantly solves [NO_REPLY] filtering without modifying the LLM dispatch path
Jitter + semaphore + hard cap provide multi-layer cost protection

chaodu-agent requested a review from thepagent as a code owner June 26, 2026 23:32