channels: migrate providers to tinychannels + generic ChannelHost boundary (whatsapp_web, telegram)#4569
Merged
senamakel merged 10 commits intoJul 5, 2026
Conversation
Replace the in-tree implementations of the portable channel providers
with thin re-exports from the tinychannels crate (submodule bumped):
dingtalk, discord, email_channel, imessage, irc, lark, linq,
mattermost, qq, yuanbao
Each provider file now re-exports its module from
`tinychannels::providers::<p>` (glob, to preserve the internal
submodule/test-seam surface that raw-coverage e2e tests reach into).
The per-provider `*_tests.rs` files and the discord/ and yuanbao/
subdirectories move into tinychannels.
- startup.rs / commands.rs: providers whose reqwest client was
previously proxied internally (discord, dingtalk, mattermost, qq) now
build the proxy client at the call site via
`build_runtime_proxy_client("channel.<x>")` and pass it to
`with_http_client(...)`, preserving proxy behavior.
- One lark WS raw-coverage test now builds its tungstenite Message via
`tinychannels::tokio_tungstenite` so the type matches across the
0.24/0.29 version boundary.
The 4 host-coupled providers (telegram, web, presentation, whatsapp_web)
remain in-tree: they depend on the agent harness, event bus, voice, and
approval surfaces, which are not part of the tinychannels boundary.
Core crate, Tauri shell, and the affected channels integration tests all
build and pass.
Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
Pull in the new `tinychannels::host` module — a generic ChannelHost capability surface (turn dispatch, voice STT/TTS, approvals, reaction gate, conversation store, event sink, lifecycle, run ledger) that will let the 4 host-coupled providers (web, telegram, presentation, whatsapp_web) be ported behind a portable boundary. No consumer wiring yet; this only advances the submodule. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
New `src/openhuman/channels/host/` implements the portable `tinychannels::host` capability traits against real OpenHuman internals: - CoreShutdownRegistry → core::shutdown::register (LifecycleRegistry) - VoiceTranscriber → voice STT factory (Transcriber) - VoiceSynthesizer → voice reply_speech (SpeechSynthesizer) - InferenceReactionGate → inference should_react (ReactionGate, w/ emoji) - CoreApprovalGate → approval::parse_approval_reply (ApprovalGate) - ConversationHistoryStore → memory_conversations (ConversationStore) - WebChannelEventSink → web WebChannelEvent bus (EventSink) `build_channel_host(config)` assembles them via ChannelHostBuilder; `build_provider_context(config, http_client)` wraps host + channels config + HTTP client into the ProviderContext handed to providers. Capabilities OpenHuman can't yet express portably (turn dispatch, run ledger, memory recall, pairing) are left unset — providers degrade gracefully. 7 adapter unit tests pass (bumps tinychannels submodule for the refined async ShutdownHook / ReactionDecision.emoji / ApprovalGate default). Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
- whatsapp_web is now a thin re-export of tinychannels; the WA native deps (whatsapp-rust/wacore/serde-big-array) move out of the core crate and the `whatsapp-web` feature forwards to `tinychannels/whatsapp-web`. - startup assembles the ChannelHost once via `build_channel_host(config)` and threads `host.lifecycle()` into WhatsAppWebChannel, so its shutdown teardown registers through the capability boundary instead of core::shutdown directly. - Add tests/channels_host_boundary_e2e.rs: drives the assembled ChannelHost through the `dyn ChannelHost` trait objects (conversation store roundtrip, reaction-gate short-circuit, approval parse, event-sink publish, lifecycle registration) exactly as a ported provider would. 6 e2e + 7 adapter tests pass. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
Presentation is the web channel's response delivery formatter (segments a response into WebChannelEvents, drives emoji reactions) — not an independent channel. Move it under providers/web/ as `web::presentation` and update its three callers (task_dispatcher executor, web ops x2). No behavior change. 29 presentation tests pass at the new path. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
- telegram/ keeps only host glue (remote_control, bus, approval_surface); the transport is re-exported from tinychannels. startup injects host caps into TelegramChannel (transcriber, allowlist, events, proxy client). - Add ConfigAllowlistStore adapter (persist to config.toml) and generalize the event sink into OpenHumanEventSink, which routes by domain: "web" → WebChannelEvent bus, "channel" → DomainEvent bus (telegram reaction fan-out). Wire both into build_channel_host. - security/pairing.rs: PairingGuard + helpers now re-exported from tinychannels::security; keep the host-only core-token/bind helpers. - Fix the presentation raw-coverage test path after the earlier web fold. Bumps tinychannels submodule. Core + all targets compile; pairing (35), host adapters (8), telegram host-glue (15), and the host/presentation e2e suites pass. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
Contributor
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR migrates most channel provider implementations (Discord, DingTalk, Email, iMessage, IRC, Lark, Linq, Mattermost, QQ, Telegram, WhatsApp Web, Yuanbao) into re-exports from a new ChangesChannelHost Adapter Layer
Provider Re-exports and Dependency Updates
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant build_channel_host
participant ChannelHostBuilder
participant Adapters as Host Adapters
Caller->>build_channel_host: Config
build_channel_host->>ChannelHostBuilder: new()
build_channel_host->>Adapters: wire CoreShutdownRegistry, VoiceTranscriber, VoiceSynthesizer, InferenceReactionGate, CoreApprovalGate, ConversationHistoryStore, OpenHumanEventSink, ConfigAllowlistStore
ChannelHostBuilder-->>build_channel_host: Arc<dyn ChannelHost>
build_channel_host-->>Caller: ChannelHost instance
sequenceDiagram
participant Executor as task_dispatcher/executor
participant WebOps as web::ops
participant Presentation as web::presentation::deliver_response
participant Bus as publish_web_channel_event
Executor->>Presentation: deliver_response(session)
WebOps->>Presentation: deliver_response(turn/parallel turn)
Presentation->>Bus: publish_web_channel_event(event)
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b38ebd5bf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| } | ||
| } | ||
| pub use tinychannels::providers::lark::*; |
There was a problem hiding this comment.
When channel.lark is configured to use OpenHuman's runtime proxy, the previous in-tree Lark provider built its REST client with build_runtime_proxy_client("channel.lark") for tenant-token fetches, sends, and health checks, but this re-export switches those calls to the tinychannels implementation while start_channels and doctor_channels still construct Lark with only LarkChannel::from_config(lk). In proxy-required deployments, Lark token refresh/send/health traffic now bypasses the configured proxy and the channel will fail even though the same config worked before; please add/pass an OpenHuman proxied client for the Lark provider as was done for Discord/Mattermost/DingTalk/QQ.
Useful? React with 👍 / 👎.
| @@ -0,0 +1 @@ | |||
| pub use tinychannels::providers::discord::*; | |||
There was a problem hiding this comment.
The Discord runtime channel now receives build_runtime_proxy_client("channel.discord"), but the Channels setup RPCs still call providers::discord::api::{list_bot_guilds,list_guild_channels,check_channel_permissions} through this re-export. The removed local api.rs built its client with OpenHuman's Discord proxy, whereas the host-agnostic tinychannels API has no access to that config/client, so guild/channel discovery and permission checks from Settings will bypass the configured proxy in proxy-only environments. Please keep host-side wrappers for these API calls or pass a proxied client into the tinychannels helpers.
Useful? React with 👍 / 👎.
Advances the submodule to the clippy-fixed tinychannels commit so the Rust SDK CI gate passes. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
- security/pairing.rs: PermissionsExt is unused after the PairingGuard move (only OpenOptionsExt remains, for the retained core-token writer). - runtime/startup.rs: the ChannelHost trait import is unused — methods resolve through the `dyn ChannelHost` object. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/channels/host/adapters.rs`:
- Around line 266-308: ConfigAllowlistStore is re-deriving the config location
from the home directory instead of using the active config path, so allowlist
updates can be written to the wrong file. Thread the existing Config path from
build_channel_host into ConfigAllowlistStore and use that stored path in
persist_allowed_identity when reading and saving, rather than reconstructing
~/.openhuman/config.toml via UserDirs::new(). Ensure the config save still
updates the allowlist through Config::save with the same active config
instance/path.
- Around line 205-224: The history() adapter currently drops the stored creation
time by rebuilding ConversationMessage with timestamp set to None. Update the
mapping in src/openhuman/channels/host/adapters.rs inside history() so it passes
through the backing record’s created_at value when constructing
ConversationMessage, using the existing message fields and preserving the stored
timestamp instead of discarding it.
In `@src/openhuman/channels/host/mod.rs`:
- Around line 33-62: The `mod.rs` file currently contains business logic in
`build_channel_host` and `build_provider_context`, which violates the
export-only module rule. Move these constructor functions into a sibling module
such as `ops.rs` or `builder.rs`, and keep `mod.rs` limited to module
declarations and re-exports. Update any call sites to use the relocated
functions or re-export them from `mod.rs` if needed, while preserving the
`ChannelHostBuilder` and `ProviderContext::new` setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23000cf9-d93c-4526-9c68-b5a95a9b8a7d
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (70)
Cargo.tomlsrc/openhuman/agent/task_dispatcher/executor.rssrc/openhuman/channels/commands.rssrc/openhuman/channels/host/adapters.rssrc/openhuman/channels/host/mod.rssrc/openhuman/channels/host/tests.rssrc/openhuman/channels/mod.rssrc/openhuman/channels/providers/dingtalk.rssrc/openhuman/channels/providers/discord.rssrc/openhuman/channels/providers/discord/api.rssrc/openhuman/channels/providers/discord/api_tests.rssrc/openhuman/channels/providers/discord/channel.rssrc/openhuman/channels/providers/discord/channel_tests.rssrc/openhuman/channels/providers/discord/mod.rssrc/openhuman/channels/providers/email_channel.rssrc/openhuman/channels/providers/email_channel_tests.rssrc/openhuman/channels/providers/imessage.rssrc/openhuman/channels/providers/imessage_tests.rssrc/openhuman/channels/providers/irc.rssrc/openhuman/channels/providers/irc_tests.rssrc/openhuman/channels/providers/lark.rssrc/openhuman/channels/providers/lark_tests.rssrc/openhuman/channels/providers/linq.rssrc/openhuman/channels/providers/linq_tests.rssrc/openhuman/channels/providers/mattermost.rssrc/openhuman/channels/providers/mattermost_tests.rssrc/openhuman/channels/providers/mod.rssrc/openhuman/channels/providers/qq.rssrc/openhuman/channels/providers/qq_tests.rssrc/openhuman/channels/providers/telegram/attachments.rssrc/openhuman/channels/providers/telegram/channel.rssrc/openhuman/channels/providers/telegram/channel_core.rssrc/openhuman/channels/providers/telegram/channel_ops.rssrc/openhuman/channels/providers/telegram/channel_recv.rssrc/openhuman/channels/providers/telegram/channel_send.rssrc/openhuman/channels/providers/telegram/channel_tests.rssrc/openhuman/channels/providers/telegram/channel_types.rssrc/openhuman/channels/providers/telegram/mod.rssrc/openhuman/channels/providers/telegram/session_store.rssrc/openhuman/channels/providers/telegram/text.rssrc/openhuman/channels/providers/web/mod.rssrc/openhuman/channels/providers/web/ops.rssrc/openhuman/channels/providers/web/presentation.rssrc/openhuman/channels/providers/web/presentation_tests.rssrc/openhuman/channels/providers/whatsapp_web.rssrc/openhuman/channels/providers/whatsapp_web_tests.rssrc/openhuman/channels/providers/yuanbao.rssrc/openhuman/channels/providers/yuanbao/channel.rssrc/openhuman/channels/providers/yuanbao/config.rssrc/openhuman/channels/providers/yuanbao/connection.rssrc/openhuman/channels/providers/yuanbao/cos.rssrc/openhuman/channels/providers/yuanbao/errors.rssrc/openhuman/channels/providers/yuanbao/ids.rssrc/openhuman/channels/providers/yuanbao/inbound.rssrc/openhuman/channels/providers/yuanbao/media.rssrc/openhuman/channels/providers/yuanbao/mod.rssrc/openhuman/channels/providers/yuanbao/outbound.rssrc/openhuman/channels/providers/yuanbao/proto.rssrc/openhuman/channels/providers/yuanbao/proto_biz.rssrc/openhuman/channels/providers/yuanbao/proto_constants.rssrc/openhuman/channels/providers/yuanbao/sign.rssrc/openhuman/channels/providers/yuanbao/splitter.rssrc/openhuman/channels/providers/yuanbao/types.rssrc/openhuman/channels/providers/yuanbao/wire.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/security/pairing.rstests/channels_bus_presentation_raw_coverage_e2e.rstests/channels_host_boundary_e2e.rstests/channels_lark_email_dispatch_round21_raw_coverage_e2e.rsvendor/tinychannels
💤 Files with no reviewable changes (30)
- src/openhuman/channels/providers/qq_tests.rs
- src/openhuman/channels/providers/telegram/channel_types.rs
- src/openhuman/channels/providers/email_channel_tests.rs
- src/openhuman/channels/providers/yuanbao/errors.rs
- src/openhuman/channels/providers/discord/mod.rs
- src/openhuman/channels/providers/whatsapp_web_tests.rs
- src/openhuman/channels/providers/telegram/channel_tests.rs
- src/openhuman/channels/providers/mattermost_tests.rs
- src/openhuman/channels/providers/telegram/attachments.rs
- src/openhuman/channels/providers/irc_tests.rs
- src/openhuman/channels/providers/telegram/channel_ops.rs
- src/openhuman/channels/providers/linq_tests.rs
- src/openhuman/channels/providers/telegram/text.rs
- src/openhuman/channels/providers/lark_tests.rs
- src/openhuman/channels/providers/yuanbao/channel.rs
- src/openhuman/channels/providers/discord/api.rs
- src/openhuman/channels/providers/telegram/channel.rs
- src/openhuman/channels/providers/imessage_tests.rs
- src/openhuman/channels/providers/discord/channel.rs
- src/openhuman/channels/providers/telegram/channel_recv.rs
- src/openhuman/channels/providers/telegram/channel_core.rs
- src/openhuman/channels/providers/discord/api_tests.rs
- src/openhuman/channels/providers/yuanbao/config.rs
- src/openhuman/channels/providers/yuanbao/cos.rs
- src/openhuman/channels/providers/yuanbao/connection.rs
- src/openhuman/channels/providers/mod.rs
- src/openhuman/channels/providers/yuanbao/ids.rs
- src/openhuman/channels/providers/discord/channel_tests.rs
- src/openhuman/channels/providers/telegram/session_store.rs
- src/openhuman/channels/providers/telegram/channel_send.rs
Comment on lines
+205
to
+224
| async fn history( | ||
| &self, | ||
| session_key: &str, | ||
| limit: usize, | ||
| ) -> anyhow::Result<Vec<ConversationMessage>> { | ||
| let messages = crate::openhuman::memory_conversations::get_messages( | ||
| self.workspace_dir.clone(), | ||
| session_key, | ||
| ) | ||
| .map_err(|e| anyhow::anyhow!(e))?; | ||
| let start = messages.len().saturating_sub(limit); | ||
| Ok(messages[start..] | ||
| .iter() | ||
| .map(|m| ConversationMessage { | ||
| role: m.message_type.clone(), | ||
| content: m.content.clone(), | ||
| timestamp: None, | ||
| }) | ||
| .collect()) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files/symbols first.
printf '\n== Files ==\n'
git ls-files 'src/openhuman/channels/host/adapters.rs' 'src/openhuman/**' | sed -n '1,120p'
printf '\n== Outline: adapters.rs ==\n'
ast-grep outline src/openhuman/channels/host/adapters.rs --view expanded || true
printf '\n== Search for ConversationMessage definition/usages ==\n'
rg -n "ConversationMessage|timestamp:|created_at|append\\(" src . -g '!target' -g '!node_modules' || trueRepository: tinyhumansai/openhuman
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If ConversationMessage comes from an external dependency, inspect lockfile and local source hints.
printf '\n== Cargo manifest/lock snippets ==\n'
rg -n "tinychannels|ConversationMessage" Cargo.toml Cargo.lock . -g '!target' -g '!node_modules' || true
printf '\n== Relevant slice of adapters.rs ==\n'
sed -n '180,280p' src/openhuman/channels/host/adapters.rsRepository: tinyhumansai/openhuman
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tinychannels ConversationMessage definition ==\n'
fd -a 'mod.rs' vendor/tinychannels -E target | sed -n '1,20p'
rg -n "pub struct ConversationMessage|enum ConversationMessage|timestamp" vendor/tinychannels -g '*.rs' -n || true
printf '\n== host adapter slices ==\n'
sed -n '200,260p' src/openhuman/channels/host/adapters.rs
printf '\n== history() call sites / consumers ==\n'
rg -n "\.history\(" src/openhuman -g '*.rs' || trueRepository: tinyhumansai/openhuman
Length of output: 5195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tinychannels host/message files ==\n'
fd -a 'host' vendor/tinychannels -t d
fd -a 'conversation' vendor/tinychannels -t f | sed -n '1,40p'
fd -a '*.rs' vendor/tinychannels -t f | sed -n '1,80p'
printf '\n== precise type search ==\n'
rg -n "ConversationMessage|timestamp:|created_at|History|ConversationStore" vendor/tinychannels/src vendor/tinychannels -g '*.rs' -g '!target' || trueRepository: tinyhumansai/openhuman
Length of output: 483
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== vendor/tinychannels tree ==\n'
fd -a . vendor/tinychannels -t d | sed -n '1,40p'
fd -a . vendor/tinychannels -t f | sed -n '1,120p'
printf '\n== ConversationMessage definition search ==\n'
rg -n "struct ConversationMessage|enum ConversationMessage|timestamp" vendor/tinychannels -g '*.rs' || true
printf '\n== host tests around history() ==\n'
sed -n '1,220p' src/openhuman/channels/host/tests.rsRepository: tinyhumansai/openhuman
Length of output: 5612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== memory_conversations files ==\n'
git ls-files 'src/openhuman/memory_conversations*' 'src/openhuman/**/memory_conversations*' | sed -n '1,120p'
printf '\n== definitions and helpers ==\n'
rg -n "pub struct ConversationMessage|struct ConversationMessage|created_at|parse_from_rfc3339|to_rfc3339|get_messages|append_message" src/openhuman/memory_conversations* src/openhuman -g '*.rs' | sed -n '1,220p'
printf '\n== host adapter slice with line numbers ==\n'
cat -n src/openhuman/channels/host/adapters.rs | sed -n '205,255p'Repository: tinyhumansai/openhuman
Length of output: 24426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search local cargo source for tinychannels ==\n'
rg -n "pub struct ConversationMessage|enum ConversationMessage|struct ConversationMessage" \
~/.cargo/registry/src ~/.cargo/git/checkouts vendor \
-g '*.rs' 2>/dev/null | sed -n '1,120p' || true
printf '\n== memory_conversations types ==\n'
sed -n '1,220p' src/openhuman/memory_conversations/types.rs
printf '\n== memory_conversations store get_messages append_message ==\n'
rg -n "pub fn get_messages|pub fn append_message|created_at|timestamp" src/openhuman/memory_conversations/store.rs -g '*.rs' | sed -n '1,220p'Repository: tinyhumansai/openhuman
Length of output: 6686
Pass through the stored timestamp in history() ConversationMessage is rebuilt with timestamp: None, so callers never see the created_at value already stored in the backing record. Map that field through instead of dropping it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/channels/host/adapters.rs` around lines 205 - 224, The
history() adapter currently drops the stored creation time by rebuilding
ConversationMessage with timestamp set to None. Update the mapping in
src/openhuman/channels/host/adapters.rs inside history() so it passes through
the backing record’s created_at value when constructing ConversationMessage,
using the existing message fields and preserving the stored timestamp instead of
discarding it.
Comment on lines
+266
to
+308
| pub struct ConfigAllowlistStore; | ||
|
|
||
| #[async_trait] | ||
| impl AllowlistStore for ConfigAllowlistStore { | ||
| async fn persist_allowed_identity(&self, channel: &str, identity: &str) -> anyhow::Result<()> { | ||
| use anyhow::Context; | ||
| let normalized = identity.trim().trim_start_matches('@').to_string(); | ||
| if normalized.is_empty() { | ||
| anyhow::bail!("cannot persist empty identity"); | ||
| } | ||
|
|
||
| let home = directories::UserDirs::new() | ||
| .map(|u| u.home_dir().to_path_buf()) | ||
| .context("could not find home directory")?; | ||
| let openhuman_dir = home.join(".openhuman"); | ||
| let config_path = openhuman_dir.join("config.toml"); | ||
| let contents = tokio::fs::read_to_string(&config_path) | ||
| .await | ||
| .with_context(|| format!("failed to read config file: {}", config_path.display()))?; | ||
| let mut config: Config = | ||
| toml::from_str(&contents).context("failed to parse config.toml for allowlist")?; | ||
| config.config_path = config_path; | ||
| config.workspace_dir = openhuman_dir.join("workspace"); | ||
|
|
||
| match channel { | ||
| "telegram" => { | ||
| let Some(telegram) = config.channels_config.telegram.as_mut() else { | ||
| anyhow::bail!("telegram channel config is missing in config.toml"); | ||
| }; | ||
| if !telegram.allowed_users.iter().any(|u| u == &normalized) { | ||
| telegram.allowed_users.push(normalized); | ||
| config | ||
| .save() | ||
| .await | ||
| .context("failed to persist allowlist to config.toml")?; | ||
| } | ||
| } | ||
| other => anyhow::bail!("allowlist persist unsupported for channel '{other}'"), | ||
| } | ||
| tracing::debug!("{LOG_PREFIX} persisted allowed identity for channel={channel}"); | ||
| Ok(()) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and inspect the surrounding code.
git ls-files | rg '^(src/openhuman/channels/host/(adapters|mod)\.rs|src/openhuman/channels/host/tests\.rs|src/openhuman/.*/config.*|src/openhuman/.*/mod\.rs)$'
printf '\n--- adapters outline ---\n'
ast-grep outline src/openhuman/channels/host/adapters.rs --view expanded || true
printf '\n--- host mod outline ---\n'
ast-grep outline src/openhuman/channels/host/mod.rs --view expanded || true
printf '\n--- adapters.rs relevant slice ---\n'
sed -n '1,380p' src/openhuman/channels/host/adapters.rs | cat -n
printf '\n--- host/mod.rs relevant slice ---\n'
sed -n '1,220p' src/openhuman/channels/host/mod.rs | cat -n
printf '\n--- tests slice ---\n'
if [ -f src/openhuman/channels/host/tests.rs ]; then
sed -n '1,260p' src/openhuman/channels/host/tests.rs | cat -n
fiRepository: tinyhumansai/openhuman
Length of output: 41368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- config outline ---\n'
ast-grep outline src/openhuman/config/mod.rs --view expanded || true
printf '\n--- config.rs / mod.rs relevant slice ---\n'
sed -n '1,260p' src/openhuman/config/mod.rs | cat -n
printf '\n--- search for config_path usage ---\n'
rg -n "config_path" src/openhuman -g '*.rs'
printf '\n--- search for save\(\) implementation / path handling ---\n'
rg -n "impl .*Config|fn save\(" src/openhuman/config src/openhuman -g '*.rs'Repository: tinyhumansai/openhuman
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the config save/load implementation and the host construction site.
sed -n '430,520p' src/openhuman/config/schema/load/impl_load.rs | cat -n
printf '\n---\n'
sed -n '24,58p' src/openhuman/channels/host/mod.rs | cat -n
printf '\n---\n'
sed -n '260,305p' src/openhuman/channels/host/adapters.rs | cat -n
printf '\n---\n'
sed -n '1,120p' src/openhuman/config/schema/types.rs | cat -nRepository: tinyhumansai/openhuman
Length of output: 13193
Thread the active config path into ConfigAllowlistStore. build_channel_host already has Arc<Config>, but this store re-derives ~/.openhuman/config.toml from UserDirs::home_dir() and then saves through that path. Custom --config/test setups can end up writing the allowlist to a file the running process never reads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/channels/host/adapters.rs` around lines 266 - 308,
ConfigAllowlistStore is re-deriving the config location from the home directory
instead of using the active config path, so allowlist updates can be written to
the wrong file. Thread the existing Config path from build_channel_host into
ConfigAllowlistStore and use that stored path in persist_allowed_identity when
reading and saving, rather than reconstructing ~/.openhuman/config.toml via
UserDirs::new(). Ensure the config save still updates the allowlist through
Config::save with the same active config instance/path.
Comment on lines
+33
to
+62
| pub fn build_channel_host(config: Arc<Config>) -> Arc<dyn ChannelHost> { | ||
| ChannelHostBuilder::new() | ||
| .lifecycle(Arc::new(CoreShutdownRegistry)) | ||
| .transcriber(Arc::new(VoiceTranscriber { | ||
| config: Arc::clone(&config), | ||
| })) | ||
| .synthesizer(Arc::new(VoiceSynthesizer { | ||
| config: Arc::clone(&config), | ||
| })) | ||
| .reactions(Arc::new(InferenceReactionGate { | ||
| config: Arc::clone(&config), | ||
| })) | ||
| .approvals(Arc::new(CoreApprovalGate)) | ||
| .conversations(Arc::new(ConversationHistoryStore { | ||
| workspace_dir: config.workspace_dir.clone(), | ||
| })) | ||
| .events(Arc::new(OpenHumanEventSink)) | ||
| .allowlist(Arc::new(ConfigAllowlistStore)) | ||
| .build() | ||
| } | ||
|
|
||
| /// Build the [`ProviderContext`] handed to a channel provider at construction: | ||
| /// the assembled host + the channels config + a pre-built HTTP client. | ||
| pub fn build_provider_context(config: &Config, http_client: reqwest::Client) -> ProviderContext { | ||
| ProviderContext::new( | ||
| build_channel_host(Arc::new(config.clone())), | ||
| config.channels_config.clone(), | ||
| http_client, | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Business logic in mod.rs violates the export-only module rule.
build_channel_host and build_provider_context assemble the concrete ChannelHost/ProviderContext via a builder pattern — this is composition/business logic, not mod/pub mod/pub use/controller-schema wiring. As per path instructions, src/openhuman/**/mod.rs files must be export-only: "mod.rs files must be export-only: mod/pub mod, pub use, and controller schema wiring only; no business logic." Consider moving these constructors into a sibling file (e.g. ops.rs or builder.rs) and keeping mod.rs limited to mod adapters; mod ops; pub use ...;.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/channels/host/mod.rs` around lines 33 - 62, The `mod.rs` file
currently contains business logic in `build_channel_host` and
`build_provider_context`, which violates the export-only module rule. Move these
constructor functions into a sibling module such as `ops.rs` or `builder.rs`,
and keep `mod.rs` limited to module declarations and re-exports. Update any call
sites to use the relocated functions or re-export them from `mod.rs` if needed,
while preserving the `ChannelHostBuilder` and `ProviderContext::new` setup.
Source: Path instructions
Pre-existing bug surfaced by this PR's rust-core-coverage job (which runs the full `cargo test --tests`): tokenjuice_integration looked for fixtures under `vendor/tinyjuice/src/tests/fixtures`, but the pinned tinyjuice places them at `vendor/tinyjuice/tests/fixtures`. The test never actually ran before (it panicked on the missing dir whenever the coverage job fired). With the correct path it runs and passes (Rust port matches all fixtures). Unrelated to the channels migration; included because it blocks this PR's coverage gate. Claude-Session: https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1794c23db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mod text; | ||
|
|
||
| // Transport moved to tinychannels. | ||
| pub use tinychannels::providers::telegram::{session_store, TelegramChannel}; |
There was a problem hiding this comment.
When this re-export swaps TelegramChannel to the tinychannels implementation, TelegramChannel::new() no longer builds the OpenHuman runtime-proxy client by itself; start_channels compensates with .with_http_client(...), but doctor_channels still constructs Telegram with only new().with_streaming() in src/openhuman/channels/commands.rs:50-59. In deployments where channel.telegram must use the runtime proxy, the doctor health check will bypass the proxy and report Telegram as unhealthy even though the runtime listener works, so please inject build_runtime_proxy_client("channel.telegram") in the doctor path too.
Useful? React with 👍 / 👎.
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
1 participant
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
tinychannelscrate (submodule) so they can be built, tested, and debugged standalone: dingtalk, discord, email_channel, imessage, irc, lark, linq, mattermost, qq, yuanbao (joining signal/slack/whatsapp).crate::openhuman::*.web::presentation(it is web's delivery formatter, not a channel).PairingGuardtotinychannels::security; OpenHuman re-exports it.Problem
The channels implementation lived entirely in the core crate, so provider logic could only be exercised through the full OpenHuman build. The four "rich" providers (telegram, web, presentation, whatsapp_web) reached directly into the agent harness, voice, approvals, event bus, and config — which is precisely what blocked moving them out for isolated testing.
Solution
tinychannels::providers::*; the per-provider tests move with them.tinychannels::host::ChannelHostexposes each host capability as an optionalArc<dyn …>accessor (TurnDispatcher,Transcriber,SpeechSynthesizer,ApprovalGate,ReactionGate,ConversationStore,EventSink,LifecycleRegistry,RunLedger,AllowlistStore,Memory).src/openhuman/channels/host/implements them against real internals;startupassembles the host viabuild_channel_host(config)and injects capabilities into providers.LifecycleRegistry. telegram → injectedTranscriber(voice STT),AllowlistStore(pairing allowlist persist),EventSink(reaction fan-out, via a domain-routing sink), andwith_http_client(proxy); host glue (remote-control, bus/approval subscribers) stays in-tree. web stays in-tree by design (it is the agent runtime surface).OpenHumanEventSinkroutes by domain ("web"→ WebChannelEvent bus,"channel"→ DomainEvent bus) so one capability serves both buses.mainfor CI submodule checkout.Submission Checklist
tests/channels_host_boundary_e2e.rs, and 975 provider tests in tinychannels.## Related—N/A: no matrix feature IDs affected.N/A: no release-cut surface behavior change.Closes #NNN— N/A: no tracking issue.Impact
whatsapp-webfeature; the WA native stack moves out of the core crate into tinychannels.PairingGuardmoved verbatim (same hashing/lockout);core::authstill uses the re-exportedconstant_time_eq.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— N/A: noapp/srcchanges.pnpm typecheck— N/A: no TypeScript changes.cargo test --lib channels::host security::pairing channels::providers::telegram;cargo test --test channels_host_boundary_e2e; tinychannelscargo test --all-features(975 passed).cargo fmt --check+cargo check --all-targets(0 errors) on core andapp/src-tauri.cargo check --manifest-path app/src-tauri/Cargo.toml(0 errors).https://claude.ai/code/session_013NzEbXoeB3YzsbmpjVUTPW
Summary by CodeRabbit
New Features
Improvements
Bug Fixes