docs: ADR for Multi-Platform Adapter Architecture

[chaodu-agent]

Summary

Architecture Decision Record for the multi-platform adapter architecture, reformatted from RFC 002 into the standard ADR format under docs/adr/.

What this covers

• ChatAdapter trait: platform-agnostic abstraction for Discord, Slack, Telegram, and future adapters
• AdapterRouter: shared routing logic extracted from platform-specific code
• Simultaneous multi-platform (Contract B): single instance serving Discord + Slack concurrently, validated by feat: add Slack adapter with multi-platform ChatAdapter architecture #259
• Platform-agnostic references: ChannelRef, MessageRef, SenderContext supporting two threading models
• Config design: TOML-based per-platform sections with shared pool
• Security: secure-by-default allowlists, bot filtering, session namespace isolation
• Testing strategy: MockAdapter for unit/integration tests
• Implementation phases: 5 phases, Phase 1+3 already merged via feat: add Slack adapter with multi-platform ChatAdapter architecture #259

Files

File	Purpose
docs/adr/multi-platform-adapters.md	Full ADR with 15 sections covering design, implementation, and resolved questions

Related

• Tracking issues: feat: Telegram adapter — session lifecycle, memory compaction & crash recovery #86, feat: Slack adapter — bring agent-broker to Slack workspaces #93
• Slack adapter: feat: add Slack adapter with multi-platform ChatAdapter architecture #259 (merged, validates Phase 1+3)
• Reviewer feedback integrated: @dogzzdogzz (8 points), @antigenius0910 (Contract A vs B)

[dogzzdogzz]

Review: RFC 002 — Multi-Platform Adapter Architecture

Overall direction is solid — ChatAdapter trait + AdapterRouter is the right abstraction. Phase 1 (pure refactor, no behavior change) is a good de-risking strategy. A few architectural gaps to address before this RFC is ready for implementation:

1. ChatAdapter trait missing message_limit()

Section 5 discusses per-platform message limits (2000/4096/4000), and format::split_message already accepts a limit: usize parameter. But the trait itself doesn't expose this — AdapterRouter would need to hardcode a match on the platform() string. Add:

fn message_limit(&self) -> usize;

2. ChannelRef threading abstraction doesn't cover all models

Current design:

pub struct ChannelRef {
    pub platform: String,
    pub channel_id: String,
    pub parent_id: Option<String>,
}

This works for Discord (thread = separate channel with a parent), but not for platforms where threads are reply chains within the same channel, identified by a separate thread/topic ID rather than a child channel. Consider:

pub struct ChannelRef {
    pub platform: String,
    pub channel_id: String,
    pub thread_id: Option<String>,  // thread within channel (e.g. Slack thread_ts, Telegram topic_id)
    pub parent_id: Option<String>,  // parent channel if thread-as-channel (Discord)
}

Alternatively, unify with a single thread_id: Option<String> and let each adapter interpret it. Either way, the current model needs to explicitly address the "thread as reply chain" vs "thread as child channel" distinction.

3. create_thread — who decides "already in a thread"?

Current discord.rs has logic in get_or_create_thread() that checks whether a message is already inside a thread (by comparing channel kind / parent ID). In the proposed architecture, where does this decision live?

• If in AdapterRouter → router needs ChannelRef.is_thread() (e.g. thread_id.is_some() || parent_id.is_some())
• If in each adapter → duplicated logic

Recommend making this a router responsibility, with ChannelRef providing enough information to decide.

4. StatusReactionController is coupled to Serenity

The current StatusReactionController::new() takes Arc<serenity::http::Http>, ChannelId, and MessageId — all Serenity types. Phase 1 refactor must decouple this. Two options:

• (a) Controller calls adapter.add_reaction() / adapter.remove_reaction() via Arc<dyn ChatAdapter>
• (b) Introduce a small ReactionSender trait

Option (a) is simpler and aligns with the existing trait design.

5. AdapterRouter.handle_message() ownership model

The proposed signature:

pub async fn handle_message(
    &self,
    adapter: &dyn ChatAdapter,
    ...
) -> Result<()>;

The current streaming loop in discord.rs uses tokio::spawn for the edit-streaming background task. A &dyn ChatAdapter reference cannot be moved into a spawned task. This needs to be Arc<dyn ChatAdapter>, or the streaming loop needs to be restructured to avoid spawning. This is a core design decision that affects all adapters.

6. Emoji format is not universal

ReactionsConfig defaults use Unicode emoji (👀, 🤔, 🆗). Some platforms require platform-specific names for the same emoji (e.g. eyes, thinking_face, ok). The RFC should clarify whether:

• (a) Config stores Unicode and each adapter converts internally
• (b) Config stores per-platform emoji mappings
• (c) ChatAdapter exposes an fn emoji_name(&self, unicode: &str) -> String method

Recommend (a) for simplicity — adapters handle conversion as an internal concern.

7. Open Questions need recommended answers

The 5 open questions need recommendations, not just options:

#	Question	Recommendation	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 theoretically collide; prefix cost is negligible
3	Global vs per-adapter max_sessions	Global hard cap + per-adapter soft limit	Prevents one noisy platform from starving others
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

8. Testing strategy missing

No mention of how to validate the architecture. Recommend:

• Define a MockAdapter (in-memory, no network) that implements ChatAdapter
• Use it to unit test AdapterRouter logic (session routing, streaming, reactions) in isolation
• Phase 1 acceptance criteria: existing Discord behavior passes integration tests through the new trait boundary

---

Summary: The RFC is directionally correct. The main gaps are in trait completeness (message_limit, ChannelRef threading model), ownership model (Arc<dyn> vs &dyn), Serenity decoupling in reactions.rs, and missing test strategy. Recommend addressing these before Phase 1 implementation begins.

[antigenius0910]

Question: Primary use case — pluggable single-platform or simultaneous multi-platform?

Before Phase 1 implementation begins, I want to raise a definition question that significantly impacts the architecture scope.

The RFC design — session key namespacing (discord:123, slack:456), the "shared vs separate pools" open question, and AdapterRouter — implies simultaneous multi-platform operation (one binary serving Discord + Slack at the same time). But I want to confirm whether this is actually the primary contract.

There are two meaningfully different use cases:

A) Pluggable single-platform per deployment

"I deploy for Slack OR Discord — easy to swap via config, not running both."

• Requires: simple trait abstraction, adapter selection at startup, no session namespacing
• Covers the vast majority of deployments

B) Simultaneous multi-platform in one binary

"One running instance serves both Discord and Slack concurrently."

• Requires: session key namespacing, shared/separate pool design, concurrent adapter lifecycle management, config merging
• The complexity the RFC currently designs for

These lead to meaningfully different implementations. If A is the primary case, the RFC is paying upfront complexity cost for an edge case — session namespacing and shared pool coordination are only needed when two adapters actually run concurrently.

The simultaneous case (B) does have legitimate scenarios — e.g., an internal Slack team + an external Discord developer community both accessing the same agent backend with different allowed_users scopes. But that should be an explicit design commitment with a named use case, not an implicit assumption that drives the whole architecture.

Why this matters before Phase 1

If the answer is B, there are two architectural pre-requisites not currently in the RFC that become hard blockers under concurrent load:

1. Global write-lock on SessionPool::with_connection() — currently holds a write lock on the entire pool HashMap for the full prompt duration (potentially minutes). Adding a second adapter doubles concurrent session pressure through a single lock, serialising all sessions across all platforms.
2. No streaming timeout — a hung session on one platform can block the pool and starve the other platform's sessions.

Neither of these is a Phase 1 concern if the answer is A.

Proposed resolution

Could the RFC explicitly state which is the primary contract? If B, what is the concrete production scenario that justifies the added complexity? If A, the Phase 1 scope simplifies significantly and the namespacing / pool-sharing questions become post-MVP.

Happy to elaborate on either the lock issue or the timeout issue if helpful.

[github-actions]

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

[chaodu-agent]

RFC 002 Updated — Review Feedback Integrated

Updated the RFC to address all outstanding feedback from @dogzzdogzz and @antigenius0910, and to reflect the merge of #259 (Slack adapter).

What changed

Primary contract declared: Contract B (simultaneous multi-platform)

@antigenius0910 asked whether the RFC targets single-platform-per-deployment (A) or simultaneous multi-platform (B). Since #259 already merged with Discord + Slack running in one process, the RFC now explicitly declares B as the primary contract. Contract A is a subset that works automatically.

@dogzzdogzz's 8 architectural points integrated

#	Point	Resolution
1	message_limit() missing from trait	Added to ChatAdapter
2	ChannelRef threading model incomplete	Added thread_id: Option<String> for reply-chain model (Slack/Telegram), kept parent_id for thread-as-channel (Discord)
3	create_thread decision ownership	Explicitly assigned to router via ChannelRef::is_thread()
4	StatusReactionController coupled to Serenity	Decoupled — now uses Arc<dyn ChatAdapter>
5	&dyn ChatAdapter can't move into spawned task	Changed to Arc<dyn ChatAdapter>
6	Emoji format not universal	Config stores Unicode, each adapter converts internally
7	Open Questions need recommendations	All 5 answered with rationale (new §11)
8	Testing strategy missing	Added §10 with MockAdapter + coverage table

New sections added

• §10 Testing Strategy — MockAdapter definition, test coverage table, Phase 1 acceptance criteria (validated by feat: add Slack adapter with multi-platform ChatAdapter architecture #259)
• §11 Resolved Questions — All 5 original open questions answered with decisions and rationale, plus the A-vs-B question
• §12 Known Limitations — Pool lock contention (resolved: read lock + Arc clone pattern), streaming timeout (open: follow-up issue needed)

Implementation phases updated

Phases now reflect reality: Phase 1 + Phase 3 marked as ✅ Merged (#259). Phase 4 repurposed for per-adapter session limits + streaming timeout. Phase 5 unchanged (platform-specific rich features).

Commit

68476f7 — 166 insertions, 65 deletions

[thepagent]

This RFC is still valid. Feel free to comment your thoughts and we'll discuss and update accordingly.

[chaodu-agent]

Thanks @antigenius0910 for the thorough Contract A vs B analysis — it was the right question to raise before Phase 1.

The ADR has been updated to explicitly declare Contract B (simultaneous multi-platform) as the primary contract, since #259 already validated Discord + Slack running in one process. Contract A works automatically as a subset.

Both architectural pre-requisites you flagged are addressed in the ADR:

1. Pool write-lock (§15 Known Limitations → Resolved) — the current implementation uses read lock + Arc clone + drop, then locks only the individual connection. Multiple adapters' sessions execute concurrently without blocking each other.
2. Streaming timeout (§15 Known Limitations → Open) — acknowledged as a follow-up issue. A hung session consumes one pool slot but doesn't block other sessions under the new locking model.

The ADR has also been reformatted from RFC to ADR format under docs/adr/multi-platform-adapters.md to align with the project's documentation structure.