From 7e4d25fa61857ae5895563bd4f893c55923ddc43 Mon Sep 17 00:00:00 2001 From: chaodufashi Date: Tue, 7 Apr 2026 09:03:18 +0800 Subject: [PATCH 1/3] rfc: 002 multi-platform adapter architecture Defines ChatAdapter trait, AdapterRouter, and platform-specific implementations for Discord, Telegram, and Slack. Covers: trait design, config format, message size handling, reaction mapping, security, 5 implementation phases. Relates to #86 #93 --- docs/rfcs/002-multi-platform-adapters.md | 296 +++++++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 docs/rfcs/002-multi-platform-adapters.md diff --git a/docs/rfcs/002-multi-platform-adapters.md b/docs/rfcs/002-multi-platform-adapters.md new file mode 100644 index 00000000..4bc266bc --- /dev/null +++ b/docs/rfcs/002-multi-platform-adapters.md @@ -0,0 +1,296 @@ +# RFC 002: Multi-Platform Adapter Architecture + +**Tracking issues:** #86, #93 +**Status:** Draft +**Author:** @chaodu-agent + +--- + +## Summary + +Define a platform-agnostic adapter layer for agent-broker so it can serve Discord, Telegram, Slack, and future chat platforms through a single unified architecture. The ACP session pool and agent backend remain unchanged — only the "front door" becomes pluggable. + +## Motivation + +- agent-broker is currently hard-wired to Discord via `serenity` +- #86 proposes a Telegram adapter, #93 proposes Slack — both require similar abstractions +- Without a shared trait, each adapter will duplicate session routing, message splitting, reaction handling, and streaming logic +- A clean adapter boundary also enables running multiple adapters simultaneously (e.g. Discord + Telegram in one deployment) + +--- + +## Current Architecture + +``` +┌──────────────┐ Gateway WS ┌──────────────────────────────────────┐ ACP stdio ┌───────────┐ +│ Discord │◄──────────────►│ agent-broker │─────────────►│ coding CLI│ +│ User │ │ discord.rs → pool.rs → connection.rs│◄── JSON-RPC ─│ (acp mode)│ +└──────────────┘ └──────────────────────────────────────┘ └───────────┘ +``` + +Everything in `discord.rs` — message handling, thread creation, edit-streaming, reactions — is Discord-specific. The session pool and ACP layer are already platform-agnostic. + +--- + +## Proposed Architecture + +``` + ┌─────────────────┐ + │ Discord Users │ + └────────┬────────┘ + │ Gateway WS (serenity) + ▼ + ┌─────────────────┐ + │ DiscordAdapter │──┐ + └─────────────────┘ │ + │ impl ChatAdapter +┌─────────────────┐ │ +│ Telegram Users │ ▼ +└────────┬────────┘ ┌─────────────────┐ ┌──────────────┐ + │ Bot API (teloxide) │ AdapterRouter │────►│ SessionPool │ + ▼ │ │ │ (unchanged) │ +┌─────────────────┐ │ - route message │ └──────┬───────┘ +│ TelegramAdapter │────────────►│ - manage threads│ │ ACP stdio +└─────────────────┘ │ - stream edits │ ┌──────▼───────┐ + └─────────────────┘ │ AcpConnection│ +┌─────────────────┐ ▲ │ (unchanged) │ +│ Slack Users │ │ └───────────────┘ +└────────┬────────┘ ┌─────────────────┐ + │ Socket Mode │ SlackAdapter │──┘ + ▼ └─────────────────┘ +┌─────────────────┐ +│ SlackAdapter │ +└─────────────────┘ +``` + +--- + +## 1. ChatAdapter Trait + +The core abstraction. Each platform implements this trait. + +```rust +#[async_trait] +pub trait ChatAdapter: Send + Sync + 'static { + /// Platform name for logging and config ("discord", "telegram", "slack") + fn platform(&self) -> &'static str; + + /// Start listening for events. Blocks until shutdown. + async fn run(&self, router: Arc) -> Result<()>; + + /// Send a new message, returns platform-specific message ID + async fn send_message(&self, channel: &ChannelRef, content: &str) -> Result; + + /// Edit an existing message in-place + async fn edit_message(&self, msg: &MessageRef, content: &str) -> Result<()>; + + /// Create a thread/topic from a message, returns thread channel ref + async fn create_thread(&self, channel: &ChannelRef, trigger_msg: &MessageRef, title: &str) -> Result; + + /// Add a reaction/emoji to a message + async fn add_reaction(&self, msg: &MessageRef, emoji: &str) -> Result<()>; + + /// Remove a reaction/emoji from a message + async fn remove_reaction(&self, msg: &MessageRef, emoji: &str) -> Result<()>; +} +``` + +### Platform-Agnostic References + +```rust +/// Identifies a channel/thread/topic across platforms +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct ChannelRef { + pub platform: String, // "discord", "telegram", "slack" + pub channel_id: String, // platform-native ID + pub parent_id: Option, // parent channel if this is a thread +} + +/// Identifies a message across platforms +#[derive(Clone, Debug)] +pub struct MessageRef { + pub platform: String, + pub channel: ChannelRef, + pub message_id: String, +} + +/// Sender identity (already exists as sender_context JSON) +#[derive(Clone, Debug, Serialize)] +pub struct SenderContext { + pub schema: String, // "agent-broker.sender.v1" + pub sender_id: String, + pub sender_name: String, + pub display_name: String, + pub channel: String, // platform name + pub channel_id: String, + pub is_bot: bool, +} +``` + +--- + +## 2. AdapterRouter + +Shared logic extracted from current `discord.rs` that is platform-independent: + +```rust +pub struct AdapterRouter { + pool: Arc, + reactions_config: ReactionsConfig, +} + +impl AdapterRouter { + /// Called by any adapter when a user message arrives. + /// Handles: session creation, prompt injection, streaming, reactions. + pub async fn handle_message( + &self, + adapter: &dyn ChatAdapter, + channel: &ChannelRef, + sender: &SenderContext, + prompt: &str, + trigger_msg: &MessageRef, + ) -> Result<()>; +} +``` + +The router owns: +- Session pool interaction (`get_or_create`, `with_connection`) +- Sender context injection (`` XML wrapping) +- Edit-streaming loop (1.5s interval, message splitting at 2000/4096 chars depending on platform) +- Reaction state machine (queued → thinking → tool → done/error) +- Thread creation decision (new thread vs. existing thread) + +Each adapter only needs to: +1. Listen for platform events +2. Determine if the message should be processed (allowed channels, @mention, thread check) +3. Call `router.handle_message()` + +--- + +## 3. Platform Comparison + +| Feature | Discord | Telegram | Slack | +|---------|---------|----------|-------| +| Connection | Gateway WebSocket (`serenity`) | Bot API polling / webhook (`teloxide`) | Socket Mode WebSocket / Events API | +| Threading | Discord threads | Forum topics | Slack threads | +| Message limit | 2000 chars | 4096 chars | 4000 chars (blocks: 3000) | +| Edit support | ✅ Full | ✅ Full | ✅ Full | +| Reactions | ✅ Unicode + custom emoji | ✅ Unicode emoji | ✅ Unicode + custom emoji | +| Trigger | `@mention` | `@mention` or any message (configurable) | `@mention` or app_mention event | +| Bot message filtering | `msg.author.bot` | `msg.from.is_bot` | `event.bot_id` present | +| Auth | Bot token | Bot token | Bot token + App token (Socket Mode) | + +--- + +## 4. Config Design + +```toml +# Enable one or more adapters. Multiple can run simultaneously. + +[discord] +bot_token = "${DISCORD_BOT_TOKEN}" +allowed_channels = ["1234567890"] + +[telegram] +bot_token = "${TELEGRAM_BOT_TOKEN}" +mode = "personal" # "personal" or "team" (see #86) +allowed_users = [] # empty = deny all (secure by default, per #91) + +[slack] +bot_token = "${SLACK_BOT_TOKEN}" +app_token = "${SLACK_APP_TOKEN}" # for Socket Mode +allowed_channels = ["C1234567890"] + +# Agent and pool config remain unchanged +[agent] +command = "kiro-cli" +args = ["acp", "--trust-all-tools"] +working_dir = "/home/agent" + +[pool] +max_sessions = 10 +session_ttl_hours = 24 +``` + +### Startup Behavior + +- agent-broker reads config and starts an adapter for each `[platform]` section present +- If no adapter section is configured → error and exit +- Each adapter runs as a separate tokio task, sharing the same `SessionPool` +- Session keys are namespaced: `discord:{thread_id}`, `telegram:{topic_id}`, `slack:{thread_ts}` + +--- + +## 5. Message Size Handling + +Each platform has different message limits. The `format::split_message` function should accept a configurable limit: + +```rust +pub fn split_message(content: &str, max_len: usize) -> Vec; +``` + +| Platform | Max chars | Current | +|----------|-----------|---------| +| Discord | 2000 | ✅ Hardcoded 1900 | +| Telegram | 4096 | New | +| Slack | 4000 | New | + +--- + +## 6. Reaction Mapping + +The `StatusReactionController` currently uses Unicode emoji which work across all three platforms. No changes needed for the emoji set, but the underlying API calls differ: + +| Action | Discord | Telegram | Slack | +|--------|---------|----------|-------| +| Add reaction | `create_reaction()` | `set_message_reaction()` | `reactions.add` | +| Remove reaction | `delete_reaction()` | `set_message_reaction()` (empty) | `reactions.remove` | + +The `ChatAdapter` trait's `add_reaction` / `remove_reaction` methods abstract this. + +--- + +## 7. Security Considerations + +- **Secure by default** (#91): empty allowlist = deny all, for every platform +- **Bot message filtering**: each adapter must ignore messages from bots to prevent loops +- **Token isolation**: each platform's tokens are independent env vars +- **Session namespace isolation**: `discord:123` and `slack:123` are separate sessions even if IDs collide +- **Rate limiting**: platform-specific rate limits should be respected (Discord 5/5s, Slack 1/s per channel, Telegram 30/s) + +--- + +## 8. Implementation Phases + +| Phase | Scope | Complexity | Depends on | +|-------|-------|------------|------------| +| **Phase 1** | Extract `ChatAdapter` trait + `AdapterRouter` from `discord.rs`, refactor Discord to implement trait | Medium | — | +| **Phase 2** | Telegram adapter (`teloxide`), personal + team modes | Medium | Phase 1, #86 | +| **Phase 3** | Slack adapter (Socket Mode), channel threading | Medium | Phase 1, #93 | +| **Phase 4** | Multi-adapter simultaneous mode (Discord + Telegram + Slack in one process) | Low | Phase 1-3 | +| **Phase 5** | Platform-specific features: Slack blocks, Telegram inline keyboards, Discord embeds | Low | Phase 1-3 | + +Each phase is independently shippable. Phase 1 is a pure refactor with no behavior change. + +--- + +## 9. Kubernetes / Helm Considerations + +- Single image supports all adapters (all compiled in) +- Helm `values.yaml` gains `telegram.*` and `slack.*` sections +- Adapter selection is config-driven, not build-time +- PVC storage structure unchanged — auth tokens are per-agent, not per-platform + +--- + +## Open Questions + +1. Should adapters share a single `SessionPool` or have separate pools? (Shared is simpler, separate gives better isolation) +2. Should session keys include platform prefix (`discord:123`) or rely on ID uniqueness? (Prefix is safer) +3. Multi-adapter mode: should there be a global `max_sessions` or per-adapter limits? +4. Should the `ChatAdapter` trait support rich messages (embeds, blocks, inline keyboards) or keep it text-only for v1? +5. How to handle platform-specific features like Slack's `reply_broadcast` or Telegram's `parse_mode`? + +--- + +_Comments and feedback welcome._ From 68476f748a11fb2fec54a468d901716b9e3f68c9 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Tue, 21 Apr 2026 09:39:25 +0000 Subject: [PATCH 2/3] =?UTF-8?q?rfc:=20update=20002=20=E2=80=94=20integrate?= =?UTF-8?q?=20review=20feedback,=20reflect=20#259=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Declare Contract B (simultaneous multi-platform) as primary contract - Integrate dogzzdogzz 8 architectural review points: message_limit(), ChannelRef threading, Arc, StatusReactionController decoupling, emoji handling, is_thread() helper - Answer all 5 open questions with recommendations (now section 11) - Add Testing Strategy section with MockAdapter (section 10) - Add Known Limitations section (section 12): pool lock contention resolved, streaming timeout open - Update implementation phases to reflect #259 merge status - Address antigenius0910 Contract A vs B question --- docs/rfcs/002-multi-platform-adapters.md | 231 ++++++++++++++++------- 1 file changed, 166 insertions(+), 65 deletions(-) diff --git a/docs/rfcs/002-multi-platform-adapters.md b/docs/rfcs/002-multi-platform-adapters.md index 4bc266bc..543990ea 100644 --- a/docs/rfcs/002-multi-platform-adapters.md +++ b/docs/rfcs/002-multi-platform-adapters.md @@ -1,8 +1,9 @@ # RFC 002: Multi-Platform Adapter Architecture **Tracking issues:** #86, #93 -**Status:** Draft +**Status:** Partially Implemented — Phase 1+3 landed via #259 (Slack adapter) **Author:** @chaodu-agent +**Reviewers:** @dogzzdogzz, @antigenius0910 --- @@ -10,29 +11,20 @@ Define a platform-agnostic adapter layer for agent-broker so it can serve Discord, Telegram, Slack, and future chat platforms through a single unified architecture. The ACP session pool and agent backend remain unchanged — only the "front door" becomes pluggable. +**Primary contract: simultaneous multi-platform (Contract B).** A single running instance can serve Discord + Slack (+ future adapters) concurrently, sharing one `SessionPool` with platform-namespaced session keys. This was validated by #259 which merged the Slack adapter with Discord + Slack running in one process. + +> Contract A (pluggable single-platform per deployment) is a subset of B and works automatically — deploy with only one `[platform]` section in config. + ## Motivation -- agent-broker is currently hard-wired to Discord via `serenity` +- agent-broker was originally hard-wired to Discord via `serenity` - #86 proposes a Telegram adapter, #93 proposes Slack — both require similar abstractions - Without a shared trait, each adapter will duplicate session routing, message splitting, reaction handling, and streaming logic -- A clean adapter boundary also enables running multiple adapters simultaneously (e.g. Discord + Telegram in one deployment) - ---- - -## Current Architecture - -``` -┌──────────────┐ Gateway WS ┌──────────────────────────────────────┐ ACP stdio ┌───────────┐ -│ Discord │◄──────────────►│ agent-broker │─────────────►│ coding CLI│ -│ User │ │ discord.rs → pool.rs → connection.rs│◄── JSON-RPC ─│ (acp mode)│ -└──────────────┘ └──────────────────────────────────────┘ └───────────┘ -``` - -Everything in `discord.rs` — message handling, thread creation, edit-streaming, reactions — is Discord-specific. The session pool and ACP layer are already platform-agnostic. +- A clean adapter boundary enables running multiple adapters simultaneously (e.g. Discord + Slack in one deployment, validated by #259) --- -## Proposed Architecture +## Current Architecture (post-#259) ``` ┌─────────────────┐ @@ -45,22 +37,19 @@ Everything in `discord.rs` — message handling, thread creation, edit-streaming └─────────────────┘ │ │ impl ChatAdapter ┌─────────────────┐ │ -│ Telegram Users │ ▼ +│ Slack Users │ ▼ └────────┬────────┘ ┌─────────────────┐ ┌──────────────┐ - │ Bot API (teloxide) │ AdapterRouter │────►│ SessionPool │ - ▼ │ │ │ (unchanged) │ + │ Socket Mode (WS) │ AdapterRouter │────►│ SessionPool │ + ▼ │ │ │ │ ┌─────────────────┐ │ - route message │ └──────┬───────┘ -│ TelegramAdapter │────────────►│ - manage threads│ │ ACP stdio +│ SlackAdapter │────────────►│ - manage threads│ │ ACP stdio └─────────────────┘ │ - stream edits │ ┌──────▼───────┐ - └─────────────────┘ │ AcpConnection│ -┌─────────────────┐ ▲ │ (unchanged) │ -│ Slack Users │ │ └───────────────┘ -└────────┬────────┘ ┌─────────────────┐ - │ Socket Mode │ SlackAdapter │──┘ - ▼ └─────────────────┘ -┌─────────────────┐ -│ SlackAdapter │ -└─────────────────┘ + └─────────────────┘ │ AcpConnection │ + ▲ └───────────────┘ + │ + ┌─────────────────┐ + │ TelegramAdapter │ (future — Phase 2) + └─────────────────┘ ``` --- @@ -75,6 +64,9 @@ pub trait ChatAdapter: Send + Sync + 'static { /// Platform name for logging and config ("discord", "telegram", "slack") fn platform(&self) -> &'static str; + /// Maximum message length for this platform + fn message_limit(&self) -> usize; + /// Start listening for events. Blocks until shutdown. async fn run(&self, router: Arc) -> Result<()>; @@ -95,17 +87,35 @@ pub trait ChatAdapter: Send + Sync + 'static { } ``` +**Design decisions:** +- `message_limit()` is on the trait (not hardcoded in router) so `AdapterRouter` can call `adapter.message_limit()` for `split_message` without matching on platform strings. (Feedback: @dogzzdogzz #1) +- Emoji handling: config stores Unicode emoji, each adapter converts internally to platform-specific format (e.g. Slack short names). (Feedback: @dogzzdogzz #6) + ### Platform-Agnostic References ```rust /// Identifies a channel/thread/topic across platforms #[derive(Clone, Debug, Hash, Eq, PartialEq)] pub struct ChannelRef { - pub platform: String, // "discord", "telegram", "slack" - pub channel_id: String, // platform-native ID - pub parent_id: Option, // parent channel if this is a thread + pub platform: String, // "discord", "telegram", "slack" + pub channel_id: String, // platform-native channel ID + pub thread_id: Option, // thread within channel (Slack thread_ts, Telegram topic_id) + pub parent_id: Option, // parent channel if thread-as-channel (Discord threads) } +impl ChannelRef { + /// Whether this ref points to a thread (either model) + pub fn is_thread(&self) -> bool { + self.thread_id.is_some() || self.parent_id.is_some() + } +} +``` + +`ChannelRef` supports two threading models (Feedback: @dogzzdogzz #2): +- **Thread-as-reply-chain** (Slack, Telegram): same `channel_id`, identified by `thread_id` +- **Thread-as-child-channel** (Discord): separate `channel_id` with `parent_id` pointing to parent + +```rust /// Identifies a message across platforms #[derive(Clone, Debug)] pub struct MessageRef { @@ -114,10 +124,10 @@ pub struct MessageRef { pub message_id: String, } -/// Sender identity (already exists as sender_context JSON) +/// Sender identity #[derive(Clone, Debug, Serialize)] pub struct SenderContext { - pub schema: String, // "agent-broker.sender.v1" + pub schema: String, // "openab.sender.v1" pub sender_id: String, pub sender_name: String, pub display_name: String, @@ -131,7 +141,7 @@ pub struct SenderContext { ## 2. AdapterRouter -Shared logic extracted from current `discord.rs` that is platform-independent: +Shared logic extracted from `discord.rs` that is platform-independent: ```rust pub struct AdapterRouter { @@ -144,7 +154,7 @@ impl AdapterRouter { /// Handles: session creation, prompt injection, streaming, reactions. pub async fn handle_message( &self, - adapter: &dyn ChatAdapter, + adapter: Arc, channel: &ChannelRef, sender: &SenderContext, prompt: &str, @@ -153,10 +163,15 @@ impl AdapterRouter { } ``` +**Key design decisions:** +- `adapter` parameter is `Arc` (not `&dyn`) so it can be moved into `tokio::spawn` for the edit-streaming background task. (Feedback: @dogzzdogzz #5) +- Thread creation decision ("already in a thread?") lives in the router, using `ChannelRef::is_thread()`. Each adapter provides enough info in `ChannelRef` for the router to decide. (Feedback: @dogzzdogzz #3) +- `StatusReactionController` is decoupled from Serenity types — it receives `Arc` and calls `add_reaction()` / `remove_reaction()` through the trait. (Feedback: @dogzzdogzz #4) + The router owns: - Session pool interaction (`get_or_create`, `with_connection`) - Sender context injection (`` XML wrapping) -- Edit-streaming loop (1.5s interval, message splitting at 2000/4096 chars depending on platform) +- Edit-streaming loop (1.5s interval, message splitting via `adapter.message_limit()`) - Reaction state machine (queued → thinking → tool → done/error) - Thread creation decision (new thread vs. existing thread) @@ -172,10 +187,10 @@ Each adapter only needs to: | Feature | Discord | Telegram | Slack | |---------|---------|----------|-------| | Connection | Gateway WebSocket (`serenity`) | Bot API polling / webhook (`teloxide`) | Socket Mode WebSocket / Events API | -| Threading | Discord threads | Forum topics | Slack threads | +| Threading model | Thread-as-child-channel | Forum topics (thread-as-reply) | Thread-as-reply-chain (`thread_ts`) | | Message limit | 2000 chars | 4096 chars | 4000 chars (blocks: 3000) | | Edit support | ✅ Full | ✅ Full | ✅ Full | -| Reactions | ✅ Unicode + custom emoji | ✅ Unicode emoji | ✅ Unicode + custom emoji | +| Reactions | ✅ Unicode + custom emoji | ✅ Unicode emoji | ✅ Unicode + custom emoji (short names) | | Trigger | `@mention` | `@mention` or any message (configurable) | `@mention` or app_mention event | | Bot message filtering | `msg.author.bot` | `msg.from.is_bot` | `event.bot_id` present | | Auth | Bot token | Bot token | Bot token + App token (Socket Mode) | @@ -223,31 +238,34 @@ session_ttl_hours = 24 ## 5. Message Size Handling -Each platform has different message limits. The `format::split_message` function should accept a configurable limit: +Each platform has different message limits. The `format::split_message` function accepts a configurable limit, sourced from `adapter.message_limit()`: ```rust pub fn split_message(content: &str, max_len: usize) -> Vec; ``` -| Platform | Max chars | Current | -|----------|-----------|---------| -| Discord | 2000 | ✅ Hardcoded 1900 | -| Telegram | 4096 | New | -| Slack | 4000 | New | +| Platform | Max chars | `message_limit()` | +|----------|-----------|-------------------| +| Discord | 2000 | 1900 (safety margin) | +| Telegram | 4096 | 4000 | +| Slack | 4000 | 3900 | --- ## 6. Reaction Mapping -The `StatusReactionController` currently uses Unicode emoji which work across all three platforms. No changes needed for the emoji set, but the underlying API calls differ: +The `StatusReactionController` uses `Arc` to call `add_reaction()` / `remove_reaction()`, fully decoupled from any platform SDK. + +Config stores Unicode emoji. Each adapter converts internally: +- Discord: Unicode passthrough +- Slack: Unicode → short name mapping (e.g. `👀` → `eyes`) +- Telegram: Unicode passthrough | Action | Discord | Telegram | Slack | |--------|---------|----------|-------| | Add reaction | `create_reaction()` | `set_message_reaction()` | `reactions.add` | | Remove reaction | `delete_reaction()` | `set_message_reaction()` (empty) | `reactions.remove` | -The `ChatAdapter` trait's `add_reaction` / `remove_reaction` methods abstract this. - --- ## 7. Security Considerations @@ -262,15 +280,13 @@ The `ChatAdapter` trait's `add_reaction` / `remove_reaction` methods abstract th ## 8. Implementation Phases -| Phase | Scope | Complexity | Depends on | -|-------|-------|------------|------------| -| **Phase 1** | Extract `ChatAdapter` trait + `AdapterRouter` from `discord.rs`, refactor Discord to implement trait | Medium | — | -| **Phase 2** | Telegram adapter (`teloxide`), personal + team modes | Medium | Phase 1, #86 | -| **Phase 3** | Slack adapter (Socket Mode), channel threading | Medium | Phase 1, #93 | -| **Phase 4** | Multi-adapter simultaneous mode (Discord + Telegram + Slack in one process) | Low | Phase 1-3 | -| **Phase 5** | Platform-specific features: Slack blocks, Telegram inline keyboards, Discord embeds | Low | Phase 1-3 | - -Each phase is independently shippable. Phase 1 is a pure refactor with no behavior change. +| Phase | Scope | Status | Notes | +|-------|-------|--------|-------| +| **Phase 1** | Extract `ChatAdapter` trait + `AdapterRouter`, refactor Discord to implement trait | ✅ Merged (#259) | Pure refactor + Slack adapter landed together | +| **Phase 2** | Telegram adapter (`teloxide`), personal + team modes | Not started | Depends on #86 | +| **Phase 3** | Slack adapter (Socket Mode), channel threading | ✅ Merged (#259) | Includes simultaneous Discord + Slack | +| **Phase 4** | Per-adapter session soft limits, streaming timeout | Not started | See Known Limitations | +| **Phase 5** | Platform-specific features: Slack blocks, Telegram inline keyboards, Discord embeds | Not started | Text-only for v1; extend via `PlatformExt` traits later | --- @@ -283,14 +299,99 @@ Each phase is independently shippable. Phase 1 is a pure refactor with no behavi --- -## Open Questions +## 10. Testing Strategy + +(Feedback: @dogzzdogzz #8) + +### MockAdapter + +Define a `MockAdapter` (in-memory, no network) that implements `ChatAdapter`: + +```rust +pub struct MockAdapter { + pub sent_messages: Arc>>, + pub edited_messages: Arc>>, + pub reactions: Arc>>, +} + +impl ChatAdapter for MockAdapter { + fn platform(&self) -> &'static str { "mock" } + fn message_limit(&self) -> usize { 2000 } + // ... record calls for assertion +} +``` + +### Test coverage + +| Layer | What to test | How | +|-------|-------------|-----| +| `AdapterRouter` | Session routing, streaming, reactions, thread creation | Unit tests with `MockAdapter` | +| `ChatAdapter` impls | Platform-specific API calls, emoji mapping, error handling | Integration tests per adapter (can mock HTTP) | +| `ChannelRef` | `is_thread()` logic for both threading models | Unit tests | +| End-to-end | Full message flow through router → pool → ACP | Integration test with `MockAdapter` + real `SessionPool` | + +### Phase 1 acceptance criteria (validated by #259) + +- ✅ Existing Discord behavior passes through the new trait boundary +- ✅ `MockAdapter` can drive `AdapterRouter` in tests +- ✅ No behavior change for Discord-only deployments + +--- + +## 11. Resolved Questions + +(Feedback: @dogzzdogzz #7, @antigenius0910) + +| # | Question | Decision | Rationale | +|---|----------|----------|-----------| +| 1 | Shared vs separate `SessionPool` | **Shared** | Already namespaced by platform prefix; separate pools add complexity with no isolation benefit | +| 2 | Platform prefix in session keys | **Yes, always prefix** | IDs from different platforms can collide; prefix cost is negligible. Format: `{platform}:{thread_id}` | +| 3 | Global vs per-adapter `max_sessions` | **Global hard cap + per-adapter soft limit** | Prevents one noisy platform from starving others. Global cap in `[pool]`, soft limits in each `[platform]` section | +| 4 | Rich messages in v1 | **Text-only** | Ship the abstraction first; rich messages are additive and platform-divergent | +| 5 | Platform-specific features | **Defer to Phase 5** | Keep `ChatAdapter` minimal for v1; extend via `PlatformExt` traits or feature flags later | +| 6 | Primary contract: A (single) vs B (simultaneous) | **B — simultaneous multi-platform** | Validated by #259. Contract A is a subset that works automatically | + +--- + +## 12. Known Limitations + +(Feedback: @antigenius0910) + +### Resolved: SessionPool lock contention + +@antigenius0910 raised that `SessionPool::with_connection()` held a write lock for the full prompt duration, which would serialize all sessions across platforms under Contract B. + +**Status: Resolved.** The current implementation uses read lock + `Arc` clone + drop lock, then locks only the individual connection: + +```rust +pub async fn with_connection(&self, thread_id: &str, f: F) -> Result { + let conn = { + let state = self.state.read().await; // read lock, not write + state.active.get(thread_id).cloned() // clone Arc + .ok_or_else(|| anyhow!("no connection"))? + }; // state lock dropped here + let mut conn = conn.lock().await; // lock individual connection + f(&mut conn).await +} +``` + +Multiple adapters' sessions can now execute concurrently without blocking each other. + +### Open: No streaming timeout + +A hung ACP session (e.g. agent process stuck, infinite tool loop) will hold its connection `Mutex` indefinitely. Under Contract B, this doesn't block other sessions (each has its own `Mutex`), but it does permanently consume one session slot from the pool. + +**Suggested fix (follow-up issue):** Add a configurable timeout to `with_connection`: + +```rust +let mut conn = tokio::time::timeout( + Duration::from_secs(config.streaming_timeout_secs), + conn.lock() +).await.map_err(|_| anyhow!("session timeout"))??; +``` -1. Should adapters share a single `SessionPool` or have separate pools? (Shared is simpler, separate gives better isolation) -2. Should session keys include platform prefix (`discord:123`) or rely on ID uniqueness? (Prefix is safer) -3. Multi-adapter mode: should there be a global `max_sessions` or per-adapter limits? -4. Should the `ChatAdapter` trait support rich messages (embeds, blocks, inline keyboards) or keep it text-only for v1? -5. How to handle platform-specific features like Slack's `reply_broadcast` or Telegram's `parse_mode`? +This would allow the pool to reclaim hung sessions and is especially important as the number of simultaneous adapters grows. --- -_Comments and feedback welcome._ +_This RFC was updated on 2026-04-21 to integrate review feedback from @dogzzdogzz (8 architectural points) and @antigenius0910 (Contract A vs B question), and to reflect the merge of #259 (Slack adapter implementing Phase 1+3)._ From 9e97c1dbc69c4300355cbe786aaa423a4b1b3326 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Fri, 24 Apr 2026 03:36:56 +0000 Subject: [PATCH 3/3] docs: reformat RFC 002 as ADR under docs/adr/ Move multi-platform adapter architecture from docs/rfcs/ to docs/adr/ format, matching the existing ADR structure. --- .../multi-platform-adapters.md} | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) rename docs/{rfcs/002-multi-platform-adapters.md => adr/multi-platform-adapters.md} (96%) diff --git a/docs/rfcs/002-multi-platform-adapters.md b/docs/adr/multi-platform-adapters.md similarity index 96% rename from docs/rfcs/002-multi-platform-adapters.md rename to docs/adr/multi-platform-adapters.md index 543990ea..7c4dfa5b 100644 --- a/docs/rfcs/002-multi-platform-adapters.md +++ b/docs/adr/multi-platform-adapters.md @@ -1,13 +1,14 @@ -# RFC 002: Multi-Platform Adapter Architecture +# ADR: Multi-Platform Adapter Architecture -**Tracking issues:** #86, #93 -**Status:** Partially Implemented — Phase 1+3 landed via #259 (Slack adapter) -**Author:** @chaodu-agent -**Reviewers:** @dogzzdogzz, @antigenius0910 +- **Status:** Partially Implemented — Phase 1+3 landed via #259 (Slack adapter) +- **Date:** 2026-04-06 +- **Author:** @chaodu-agent +- **Reviewers:** @dogzzdogzz, @antigenius0910 +- **Tracking issues:** #86, #93 --- -## Summary +## 1. Context & Decision Define a platform-agnostic adapter layer for agent-broker so it can serve Discord, Telegram, Slack, and future chat platforms through a single unified architecture. The ACP session pool and agent backend remain unchanged — only the "front door" becomes pluggable. @@ -15,7 +16,7 @@ Define a platform-agnostic adapter layer for agent-broker so it can serve Discor > Contract A (pluggable single-platform per deployment) is a subset of B and works automatically — deploy with only one `[platform]` section in config. -## Motivation +## 2. Motivation - agent-broker was originally hard-wired to Discord via `serenity` - #86 proposes a Telegram adapter, #93 proposes Slack — both require similar abstractions @@ -24,7 +25,7 @@ Define a platform-agnostic adapter layer for agent-broker so it can serve Discor --- -## Current Architecture (post-#259) +## 3. Current Architecture (post-#259) ``` ┌─────────────────┐ @@ -54,7 +55,7 @@ Define a platform-agnostic adapter layer for agent-broker so it can serve Discor --- -## 1. ChatAdapter Trait +## 4. ChatAdapter Trait The core abstraction. Each platform implements this trait. @@ -139,7 +140,7 @@ pub struct SenderContext { --- -## 2. AdapterRouter +## 5. AdapterRouter Shared logic extracted from `discord.rs` that is platform-independent: @@ -182,7 +183,7 @@ Each adapter only needs to: --- -## 3. Platform Comparison +## 6. Platform Comparison | Feature | Discord | Telegram | Slack | |---------|---------|----------|-------| @@ -197,7 +198,7 @@ Each adapter only needs to: --- -## 4. Config Design +## 7. Config Design ```toml # Enable one or more adapters. Multiple can run simultaneously. @@ -236,7 +237,7 @@ session_ttl_hours = 24 --- -## 5. Message Size Handling +## 8. Message Size Handling Each platform has different message limits. The `format::split_message` function accepts a configurable limit, sourced from `adapter.message_limit()`: @@ -252,7 +253,7 @@ pub fn split_message(content: &str, max_len: usize) -> Vec; --- -## 6. Reaction Mapping +## 9. Reaction Mapping The `StatusReactionController` uses `Arc` to call `add_reaction()` / `remove_reaction()`, fully decoupled from any platform SDK. @@ -268,7 +269,7 @@ Config stores Unicode emoji. Each adapter converts internally: --- -## 7. Security Considerations +## 10. Security Considerations - **Secure by default** (#91): empty allowlist = deny all, for every platform - **Bot message filtering**: each adapter must ignore messages from bots to prevent loops @@ -278,7 +279,7 @@ Config stores Unicode emoji. Each adapter converts internally: --- -## 8. Implementation Phases +## 11. Implementation Phases | Phase | Scope | Status | Notes | |-------|-------|--------|-------| @@ -290,7 +291,7 @@ Config stores Unicode emoji. Each adapter converts internally: --- -## 9. Kubernetes / Helm Considerations +## 12. Kubernetes / Helm Considerations - Single image supports all adapters (all compiled in) - Helm `values.yaml` gains `telegram.*` and `slack.*` sections @@ -299,7 +300,7 @@ Config stores Unicode emoji. Each adapter converts internally: --- -## 10. Testing Strategy +## 13. Testing Strategy (Feedback: @dogzzdogzz #8) @@ -338,7 +339,7 @@ impl ChatAdapter for MockAdapter { --- -## 11. Resolved Questions +## 14. Resolved Questions (Feedback: @dogzzdogzz #7, @antigenius0910) @@ -353,7 +354,7 @@ impl ChatAdapter for MockAdapter { --- -## 12. Known Limitations +## 15. Known Limitations (Feedback: @antigenius0910)