Skip to content

Fix: telegram replies#436

Merged
senamakel merged 7 commits intotinyhumansai:mainfrom
YellowSnnowmann:fix/telegram-replies
Apr 8, 2026
Merged

Fix: telegram replies#436
senamakel merged 7 commits intotinyhumansai:mainfrom
YellowSnnowmann:fix/telegram-replies

Conversation

@YellowSnnowmann
Copy link
Copy Markdown
Contributor

@YellowSnnowmann YellowSnnowmann commented Apr 8, 2026

Summary

  • Telegram reply threading — bot responses are now sent as proper Telegram replies, visibly attached to the inbound message via reply_to_message_id.
  • Bidirectional reaction support — agent can send emoji reactions via [REACTION:👍] / [REACTION:👍] reply text markers; incoming user reactions are received, allowlist-checked, and published on the event bus.
  • Typing indicator overhaul — fires one immediate sendChatAction before the refresh loop so short model turns still show "typing…"; upgraded to TelegramTypingTask to track recipient and avoid redundant restarts.
  • Duplicate update deduplication — bounded TelegramUpdateWindow (4096-entry LRU) prevents the same update_id being processed twice across long-poll cycles.
  • edited_message + message_reaction subscription — long-poll now subscribes to both update types and parse_update_message accepts edited_message.
  • Config sync on connect — connecting a bot token via UI immediately writes TelegramConfig to config.toml so the listener starts with correct credentials without a restart.

Problem

Several Telegram channel reliability issues accumulated after the initial integration:

  1. Replies were unthreaded — the bot sent plain messages with no visual attachment to the triggering message, making conversations hard to follow in group chats.
  2. Typing indicator was invisiblestart_typing only started a background refresh loop but never sent an immediate action, so the indicator never appeared during short or fast model turns. The Telegram API requires a sendChatAction call at least once before the loop matters.
  3. Reactions were sent but unconfirmed — the [REACTION:emoji] marker from SOUL.md was documented but had no parsing or dispatch logic; reactions appeared to be sent but no feedback (event bus, logging) confirmed delivery or failure.
  4. Duplicate message processing — restarted long-poll offsets or network retries could replay the same update_id, causing the agent to respond twice.
  5. Case-sensitive allowlistalice was rejected when Alice was in the allowlist, which surprised users.
  6. Config not persisted after UI connect — connecting via the settings UI wrote credentials to the secrets store but not config.toml, so the Telegram listener would not start until the app was restarted.

Solution

Reply threading

thread_ts on the inbound ChannelMessage is set to the inbound message_id (not the reply-to parent). send, send_text_chunks, and start_draft all read thread_ts and forward it as reply_to_message_id on the first chunk sent to the Bot API.

Typing indicator

start_typing now calls send_typing_action_once synchronously before spawning the refresh loop, ensuring at least one sendChatAction fires regardless of model latency. Thread-scoped actions retry without message_thread_id if the chat rejects thread-level actions (some group types). The handle is wrapped in TelegramTypingTask { recipient, handle } to skip restarting if the same recipient is already active.

Known issue: Despite the immediate send, the typing indicator is still not reliably visible in some Telegram clients (particularly iOS). The Bot API's sendChatAction is rate-limited and may be suppressed when sent too close together or from bots with low interaction history. This is a Bot API limitation, not a client-side bug — investigation continues.

Reaction support

parse_reaction_marker strips [REACTION:emoji] or [REACTION:emoji|message_id] from the start of any outbound message. If a target message ID is resolvable (from the marker or thread_ts), send_message_reaction calls setMessageReaction and publishes DomainEvent::ChannelReactionSent. Incoming message_reaction updates are parsed by parse_update_reaction, allowlist-checked, and published as DomainEvent::ChannelReactionReceived. Reactions from non-allowlisted users are silently dropped.

Confirmed working: Outbound reactions via [REACTION:👍] are successfully applied to messages. ChannelReactionSent and ChannelReactionReceived events fire and are visible in logs.

Deduplication

TelegramUpdateWindow (lock-guarded HashSet + VecDeque bounded to 4096) tracks every seen update_id. Updates below max_seen_update_id or already in the set are dropped with a debug-level log before any message parsing.

Config persistence

After a successful connect_channel for telegram / BotToken mode, the handler clones the current config, writes the new TelegramConfig (token + allowed users, preserving existing stream settings), and calls config.save() before returning the RPC response.

Log file constraints

OPENHUMAN_LOG_FILE_CONSTRAINTS (comma-separated file paths or target substrings) filters CLI log output to matching events only, useful for isolating Telegram traffic during debugging.

Submission Checklist

  • Unit tests — New tests: parse_update_message_sets_thread_ts_to_current_message_id_for_outbound_reply, parse_update_reaction_extracts_actor_target_and_emoji, parse_reaction_marker_supports_optional_target_id, parse_reaction_marker_allows_inline_reply_text, telegram_user_case_insensitive. Updated: typing_handle tests migrated to TelegramTypingTask.
  • E2E / integration — Manual Telegram bot smoke test recommended; no automated E2E harness for live Bot API calls.
  • Doc comments — Inline comments added at key branch points (track_update_id, parse_reaction_marker, reply-target mapping).
  • Inline comments — Reply mapping rationale, deduplication logic, and reaction marker format documented inline.

Impact

  • Desktop runtime only — all changes are in the Rust core (src/openhuman/channels/providers/telegram/).
  • No breaking changes to the RPC surfaceChannelMessage gains no new required fields; thread_ts was already present.
  • Event bus — two new DomainEvent variants (ChannelReactionSent, ChannelReactionReceived) added; existing subscribers unaffected (#[non_exhaustive]).
  • Config fileconfig.toml is now written on Telegram connect; users with manually-edited configs should verify telegram section is not overwritten unexpectedly.
  • Bot API permissionsmessage_reaction updates require the bot to have "reaction" update access enabled in BotFather (or via getUpdates subscription). No permission changes are needed for existing message flows.

Related

Summary by CodeRabbit

  • New Features

    • End-to-end Telegram reactions: send/receive emoji reactions, inline reaction markers, and reaction receipt events.
    • Telegram setup now persists allowed-users and channel-specific runtime settings.
  • Improvements

    • Conversation history keyed by reply/thread (Telegram kept per chat; others separated by thread).
    • Typing indicators start immediately and refresh more reliably.
    • CLI logging can be filtered via env and config-file warnings now logged once per path.
  • Tests

    • Added Telegram integration and unit tests for reactions, threading, and typing lifecycle.

- Introduced a new static variable to track previously warned world-readable config files, preventing duplicate warnings.
- Updated the warning logic to only log a warning for each unique world-readable config file, improving log clarity and reducing noise.
- Added new `ChannelReactionReceived` and `ChannelReactionSent` events to the DomainEvent enum, expanding event handling capabilities in the event bus.
- Included tests for the new reaction events to ensure proper functionality and integration.
- Introduced functions to parse log file constraints from environment variables and filter log events based on these constraints.
- Enhanced the `init_for_cli_run` function to apply the new filtering logic, improving log management and clarity.
- Updated the `conversation_history_key` function to include thread context for Telegram, ensuring accurate message targeting.
- Added a new trait method `supports_reactions` to the `Channel` trait, indicating support for emoji reactions.
- Implemented integration tests for Telegram channel features, including reaction handling and thread message forwarding.
…dicators

- Added support for emoji reactions in Telegram responses, allowing for contextual acknowledgment of user messages.
- Implemented a decision heuristic for when to use reactions, improving user interaction quality.
- Introduced a typing indicator that activates immediately upon receiving a message, providing instant feedback to users.
- Updated the channel delivery instructions to include new reaction syntax and guidelines for usage.
- Enhanced tests to cover new reaction handling and message acknowledgment features, ensuring robust functionality.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

Adds end-to-end Telegram emoji reaction support, updates conversation-history keys to account for reply/thread targets, refactors typing indicator behavior and persistence for Telegram settings, introduces environment-driven CLI log filtering, and expands tests and event-bus variants for reaction lifecycle.

Changes

Cohort / File(s) Summary
Logging & Config
src/core/logging.rs, src/openhuman/config/schema/load.rs
Parse OPENHUMAN_LOG_FILE_CONSTRAINTS and attach a runtime filter to CLI logging; change world-readable config warning to emit once per config path using a global OnceLock set.
Event System
src/openhuman/event_bus/events.rs
Add ChannelReactionReceived and ChannelReactionSent DomainEvent variants and include them in domain() routing and tests.
Channel Traits & Context
src/openhuman/channels/traits.rs, src/openhuman/channels/context.rs
Add Channel::supports_reactions() (default false). conversation_history_key now includes reply_target and appends _thread:{thread_ts} for non-Telegram channels; Telegram ignores thread_ts.
Telegram Controller & Persistence
src/openhuman/channels/controllers/ops.rs
Add parse_allowed_users helper; on connect_channel persist Telegram-specific TelegramConfig (allowed users, stream_mode, draft interval, mention_only) when saving BotToken credentials.
Telegram Channel Impl
src/openhuman/channels/providers/telegram/channel.rs
Implement reaction handling end-to-end (parse inbound reactions, send outbound reactions, emit reaction events), add reaction allowlist, accept edited_message and message_reaction updates, add dedupe/ordering cache for update_ids, change reply mapping to set thread_ts and use reply_to_message_id on outgoing messages, refactor typing and API response checks, and add/adjust method signatures and tests.
Runtime Dispatch & Heuristics
src/openhuman/channels/runtime/dispatch.rs
Add deterministic ack-emoji selection, start typing immediately and use sleep()-based refresh loop, spawn early Telegram acknowledgment reaction for threaded messages, and increase assistant reply debug truncation length.
Prompts
src/openhuman/agent/prompts/SOUL.md
Add "Telegram Message Reactions" section with required marker placement, two output formats, heuristics, and exclusions for when to react.
Tests
src/openhuman/channels/tests/...
src/openhuman/channels/tests/mod.rs, src/openhuman/channels/tests/context.rs, src/openhuman/channels/tests/telegram_integration.rs, src/openhuman/channels/tests/runtime_tool_calls.rs
Register telegram_integration tests; add context tests for history-key behavior; add integration tests asserting thread_ts preservation, reaction marker passthrough, typing lifecycle; update tests to compute route_key from ChannelMessage.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TelegramAPI as Telegram API
    participant Channel as TelegramChannel
    participant Dispatch as Runtime Dispatch
    participant Provider as LLM Provider
    participant EventBus as Event Bus

    User->>TelegramAPI: Send message (message_id: M)
    TelegramAPI->>Channel: Webhook update (message, message_id=M)
    Channel->>Channel: parse_update -> thread_ts = M
    Channel->>EventBus: publish ChannelMessageReceived
    EventBus->>Dispatch: route message
    Dispatch->>Dispatch: select_acknowledgment_reaction(msg.content)
    alt channel supports reactions && thread_ts present
        Dispatch->>Channel: send([REACTION:emoji], thread_ts=M) (spawned)
        Channel->>TelegramAPI: setMessageReaction(target=M, emoji)
        TelegramAPI->>EventBus: publish ChannelReactionSent
    end
    Dispatch->>Provider: request reply
    Provider-->>Dispatch: reply text (may include [REACTION:...])
    Dispatch->>Channel: send(reply text, thread_ts=M)
    Channel->>TelegramAPI: sendMessage(reply_to_message_id=M, text)
    TelegramAPI->>User: deliver reply
Loading
sequenceDiagram
    participant Dispatch as Runtime Dispatch
    participant Channel as TelegramChannel
    participant Timer as Tokio Timer
    participant TelegramAPI as Telegram API

    Dispatch->>Channel: start_typing(chat_id)
    Channel->>TelegramAPI: sendChatAction(typing)
    TelegramAPI-->>Channel: OK
    Channel->>Timer: spawn_scoped_typing_task (refresh_interval)
    loop while task active
        Timer-->>Channel: sleep(refresh_interval) elapsed
        Channel->>TelegramAPI: sendChatAction(typing)
        TelegramAPI-->>Channel: OK / error (logged)
    end
    Dispatch->>Channel: stop_typing()
    Channel->>Timer: cancel task
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 I nibbled an emoji bright and round,

threaded replies where hops are found,
I typed and blinked to hold the thread,
reactions landed, softly said,
a carrot cheer for messages sound.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: telegram replies' directly corresponds to the main change: implementing reply threading for Telegram messages by setting thread_ts to inbound message_id.
Docstring Coverage ✅ Passed Docstring coverage is 92.47% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/channels/providers/telegram/channel.rs (1)

475-485: ⚠️ Potential issue | 🟡 Minor

Case-sensitivity mismatch with parse_allowed_users could cause duplicate config entries.

is_user_allowed now uses case-insensitive matching (u.eq_ignore_ascii_case(&identity)), which is good for user experience. However, the parse_allowed_users function in src/openhuman/channels/controllers/ops.rs:65 uses case-sensitive deduplication:

if !out.iter().any(|existing| existing == normalized) {
    out.push(normalized.to_string());
}

This means "Alice" and "alice" could both be stored in the config (not deduplicated during parsing), though both would match at runtime. Consider normalizing to lowercase in parse_allowed_users before deduplication to prevent duplicate entries in config.toml.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/providers/telegram/channel.rs` around lines 475 - 485,
parse_allowed_users currently deduplicates case-sensitively while
is_user_allowed uses case-insensitive matching; update parse_allowed_users to
normalize entries (e.g., via the same normalization used by
Self::normalize_identity or to lowercase) before checking/inserting into the
output so "Alice" and "alice" cannot both be stored, and ensure the dedupe
comparison uses the normalized form to avoid duplicate config entries that
differ only by case.
🧹 Nitpick comments (1)
src/openhuman/channels/runtime/dispatch.rs (1)

442-442: Truncation effectively disabled — intentional debug change?

The truncation limit was changed from 80 to 10,000,000 characters. If this is intentional for debugging full responses in logs, consider using a named constant or usize::MAX instead of a magic number.

If this was meant to be temporary, it may warrant reverting before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/runtime/dispatch.rs` at line 442, The call to
truncate_with_ellipsis(&response, 10000000) disabled truncation via a magic
number; replace the hard-coded 10_000_000 with a named constant (e.g.,
TRUNCATE_RESPONSE_LIMIT) or use usize::MAX if you intend no truncation, or
revert to the original 80 if this was temporary; update the invocation of
truncate_with_ellipsis and add the constant near the top of the module
(referencing truncate_with_ellipsis and response) so the intent is clear and
configurable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/channels/controllers/ops.rs`:
- Around line 53-89: The parse_allowed_users function currently deduplicates
using case-sensitive equality which can store "Alice" and "alice" separately;
update the push_identity logic in parse_allowed_users to perform
case-insensitive deduplication (e.g., compare normalized.to_lowercase() against
existing.to_lowercase() or store a canonical lowercase form) so stored entries
match TelegramChannel::is_allowed's eq_ignore_ascii_case behavior and prevent
duplicate persisted entries; ensure you still trim leading '@' and preserve a
consistent canonical value for storage.

In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Line 1636: finalize_draft currently falls back to send_text_chunks with
reply_to_message_id set to None, which loses reply/thread context; update
finalize_draft (and the fallback call sites around send_text_chunks invocations)
to preserve threading by extracting the original thread id
(thread_ts/message_id) and passing it to send_text_chunks as
reply_to_message_id, or modify finalize_draft signature to accept an optional
thread_ts and forward that to send_text_chunks; ensure references to
finalize_draft, send_text_chunks, thread_ts, message_id, and reply_to_message_id
are updated consistently at the noted call sites (1636, 1654, 1663, 1707) so
fallback messages remain attached as replies.
- Around line 2383-2413: The test
parse_update_reaction_extracts_actor_target_and_emoji uses an incorrect nested
"message" object; update the test JSON so "message_reaction" contains "chat"
(with "id": -100200300) and "message_id": 123 directly (not under a nested
"message"), keep "user", "old_reaction", and "new_reaction" as-is, then run the
TelegramChannel::parse_update_reaction test to ensure sender, reply_target,
target_message_id, and emoji assertions still pass.
- Around line 745-809: parse_update_reaction is reading a non-existent nested
"message" object; change it to read "chat" and "message_id" directly from the
reaction Value (replace uses of message.get("chat") and
message.get("message_id") with reaction.get("chat") and
reaction.get("message_id")), keep the rest of the extraction
(actor/user_id/emoji) logic and the TelegramReactionEvent construction
unchanged, and also update the corresponding unit test that currently constructs
a reaction containing a "message" object to instead construct a reaction with
top-level "chat" and "message_id" fields so the test matches the real Telegram
MessageReactionUpdated structure.

In `@src/openhuman/channels/runtime/dispatch.rs`:
- Around line 84-91: The current select_acknowledgment_reaction branch treats
messages matching error keywords (the contains_any call with
"error","bug","broken","issue","problem","fail","not work","crash","stuck") as
deserving emoji (&["🤔","🤯"]), which conflicts with SOUL.md's "SUPPRESS
reactions for: errors"; update select_acknowledgment_reaction so that messages
matching that contains_any set return no reaction (e.g., neutral/None/empty
string) instead of emojis, or remove those keywords from the contains_any check
so they fall through to the neutral/default path; adjust any callers or tests
that assume a non-empty reaction accordingly.

---

Outside diff comments:
In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Around line 475-485: parse_allowed_users currently deduplicates
case-sensitively while is_user_allowed uses case-insensitive matching; update
parse_allowed_users to normalize entries (e.g., via the same normalization used
by Self::normalize_identity or to lowercase) before checking/inserting into the
output so "Alice" and "alice" cannot both be stored, and ensure the dedupe
comparison uses the normalized form to avoid duplicate config entries that
differ only by case.

---

Nitpick comments:
In `@src/openhuman/channels/runtime/dispatch.rs`:
- Line 442: The call to truncate_with_ellipsis(&response, 10000000) disabled
truncation via a magic number; replace the hard-coded 10_000_000 with a named
constant (e.g., TRUNCATE_RESPONSE_LIMIT) or use usize::MAX if you intend no
truncation, or revert to the original 80 if this was temporary; update the
invocation of truncate_with_ellipsis and add the constant near the top of the
module (referencing truncate_with_ellipsis and response) so the intent is clear
and configurable.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c565325-0c25-4c8e-833e-80459f3b9ed6

📥 Commits

Reviewing files that changed from the base of the PR and between 45a8216 and f1fb8a5.

📒 Files selected for processing (12)
  • src/core/logging.rs
  • src/openhuman/agent/prompts/SOUL.md
  • src/openhuman/channels/context.rs
  • src/openhuman/channels/controllers/ops.rs
  • src/openhuman/channels/providers/telegram/channel.rs
  • src/openhuman/channels/runtime/dispatch.rs
  • src/openhuman/channels/tests/context.rs
  • src/openhuman/channels/tests/mod.rs
  • src/openhuman/channels/tests/telegram_integration.rs
  • src/openhuman/channels/traits.rs
  • src/openhuman/config/schema/load.rs
  • src/openhuman/event_bus/events.rs

Comment thread src/openhuman/channels/controllers/ops.rs
Comment thread src/openhuman/channels/providers/telegram/channel.rs
Comment thread src/openhuman/channels/providers/telegram/channel.rs Outdated
Comment thread src/openhuman/channels/providers/telegram/channel.rs
Comment thread src/openhuman/channels/runtime/dispatch.rs Outdated
- Changed the route key in tests from `telegram_alice` to `telegram_alice_chat-1` to match the updated `conversation_history_key` format for Telegram.
- This adjustment ensures accurate routing and consistency in message handling tests.
- Refactored the message handling tests to utilize a `ChannelMessage` struct for improved clarity and maintainability.
- Updated the route key generation to use the `conversation_history_key` function, ensuring consistency in message routing.
- Simplified the invocation of `process_channel_message` by directly passing the constructed message, enhancing readability.
- Updated the `finalize_draft` method in the `Channel` trait and its implementation for `TelegramChannel` to accept an optional `thread_ts` parameter, allowing for message threading.
- Adjusted related message handling functions to utilize the new parameter, ensuring proper message context during sending.
- Modified tests to reflect changes in the `finalize_draft` method signature, enhancing the robustness of message handling in threaded conversations.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/channels/providers/telegram/channel.rs (1)

1756-1776: ⚠️ Potential issue | 🟠 Major

Attachment-only replies still lose Telegram reply threading.

parent_message_id is parsed earlier, but neither attachment branch forwards it into send_attachment(). If the model returns only media markers, or the text portion is empty after stripping markers, the outgoing media message is sent without any reply linkage and won't appear attached to the user's message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/providers/telegram/channel.rs` around lines 1756 -
1776, The attachment-only branches drop the reply/thread id so media replies
lose threading; update calls to forward parent_message_id into send_attachment
and ensure send_attachment (and any callers) accept and use parent_message_id
(e.g., add a parent_message_id param or Option type) so both the attachments
loop (for attachment in &attachments) and the parse_path_only_attachment branch
call send_attachment(chat_id, thread_id, attachment, parent_message_id), and
adjust send_attachment's implementation to include the reply_to/message
threading when sending the media; keep send_text_chunks usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Around line 54-60: The typing state is single-slot and races across concurrent
recipients: change typing_handle: Mutex<Option<TelegramTypingTask>> to a
per-recipient map (e.g., Mutex<HashMap<String, TelegramTypingTask>> keyed by
chat id) and update the start_typing and stop_typing logic (and any helpers that
reference typing_handle) to insert/start a task for the specific chat id and to
only abort/remove that chat's task on stop_typing(); ensure start_typing does
not overwrite unrelated entries and stop_typing only aborts the owned task for
the given chat id so one chat's stop cannot cancel another's indicator.
- Around line 880-885: The parser currently discards the trailing text when it
sees an empty reaction marker by returning (String::new(), None); change this so
invalid/empty markers degrade to plain text instead of being dropped: in the
branch that checks inner.is_empty(), reconstruct and return the original content
(prepend the removed '[' back to rest) as the remaining text and None for the
reaction (i.e. return (format!("[{}", rest).to_string(), None)), so send() will
see the original text rather than an empty message; adjust the code around the
inner/rest/close_pos logic accordingly.

In `@src/openhuman/channels/runtime/dispatch.rs`:
- Around line 218-227: The early typing indicator started with
target_channel.start_typing(&msg.reply_target) can be left running if
get_or_create_provider() later fails; update the control flow in dispatch.rs so
that either (a) start_typing is called only after get_or_create_provider()
succeeds, or (b) ensure any early start_typing is always matched with
target_channel.stop_typing(&msg.reply_target) on every error return path (e.g.,
inside the provider acquisition error branch and other early-return branches
around get_or_create_provider()), referencing the start_typing, stop_typing,
get_or_create_provider, target_channel and msg.reply_target symbols to find and
fix both the 218–227 and 254–270 sites.

---

Outside diff comments:
In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Around line 1756-1776: The attachment-only branches drop the reply/thread id
so media replies lose threading; update calls to forward parent_message_id into
send_attachment and ensure send_attachment (and any callers) accept and use
parent_message_id (e.g., add a parent_message_id param or Option type) so both
the attachments loop (for attachment in &attachments) and the
parse_path_only_attachment branch call send_attachment(chat_id, thread_id,
attachment, parent_message_id), and adjust send_attachment's implementation to
include the reply_to/message threading when sending the media; keep
send_text_chunks usage unchanged.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4b20e29-7101-41fc-b416-ce7dd2ae55d4

📥 Commits

Reviewing files that changed from the base of the PR and between 87d2774 and 0ee6d33.

📒 Files selected for processing (4)
  • src/openhuman/channels/controllers/ops.rs
  • src/openhuman/channels/providers/telegram/channel.rs
  • src/openhuman/channels/runtime/dispatch.rs
  • src/openhuman/channels/traits.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/openhuman/channels/controllers/ops.rs
  • src/openhuman/channels/traits.rs

Comment on lines +54 to +60
typing_handle: Mutex<Option<TelegramTypingTask>>,
stream_mode: StreamMode,
draft_update_interval_ms: u64,
last_draft_edit: Mutex<std::collections::HashMap<String, std::time::Instant>>,
mention_only: bool,
bot_username: Mutex<Option<String>>,
recent_updates: Mutex<TelegramUpdateWindow>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Single-slot typing state races across concurrent recipients.

typing_handle only stores one TelegramTypingTask, but the runtime can process multiple Telegram messages in parallel. Starting typing for chat B replaces chat A's task, and when A later calls stop_typing() it aborts whatever task is currently stored, which can cancel B's indicator mid-turn. This needs per-recipient task tracking or ownership-aware stop/start logic.

Also applies to: 1925-1980

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/providers/telegram/channel.rs` around lines 54 - 60,
The typing state is single-slot and races across concurrent recipients: change
typing_handle: Mutex<Option<TelegramTypingTask>> to a per-recipient map (e.g.,
Mutex<HashMap<String, TelegramTypingTask>> keyed by chat id) and update the
start_typing and stop_typing logic (and any helpers that reference
typing_handle) to insert/start a task for the specific chat id and to only
abort/remove that chat's task on stop_typing(); ensure start_typing does not
overwrite unrelated entries and stop_typing only aborts the owned task for the
given chat id so one chat's stop cannot cancel another's indicator.

Comment on lines +880 to +885
let inner = rest[..close_pos].trim();
if inner.is_empty() {
return (String::new(), None);
}
let remaining = rest[close_pos + 1..].trim().to_string();
(remaining, Some(inner.to_string()))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Don't drop reply text on an empty reaction marker.

[REACTION:] hello currently returns (String::new(), None), so the trailing text is discarded and send() falls through to an empty Telegram message. Invalid markers should preserve the original content and degrade to plain text instead of deleting the reply.

🔧 Proposed fix
-        if inner.is_empty() {
-            return (String::new(), None);
-        }
+        if inner.is_empty() {
+            return (content.to_string(), None);
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/providers/telegram/channel.rs` around lines 880 - 885,
The parser currently discards the trailing text when it sees an empty reaction
marker by returning (String::new(), None); change this so invalid/empty markers
degrade to plain text instead of being dropped: in the branch that checks
inner.is_empty(), reconstruct and return the original content (prepend the
removed '[' back to rest) as the remaining text and None for the reaction (i.e.
return (format!("[{}", rest).to_string(), None)), so send() will see the
original text rather than an empty message; adjust the code around the
inner/rest/close_pos logic accordingly.

Comment on lines +218 to +227
// Fire typing indicator as early as possible — before any async I/O — so the
// user sees feedback immediately regardless of how fast the LLM responds.
if let Some(channel) = target_channel.as_ref() {
if let Err(e) = channel.start_typing(&msg.reply_target).await {
tracing::debug!(
"[dispatch] Early typing start failed on {}: {e}",
channel.name()
);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Provider-init failures leave Telegram typing running.

Typing is started before get_or_create_provider(), but this error branch returns without a matching stop_typing(). On Telegram, start_typing() owns a long-lived background task, so a provider failure can leave the chat stuck showing typing until a later message clears it. Either stop typing on this branch or move the early start until after provider acquisition succeeds.

🔧 Minimal fix
         Err(err) => {
+            if let Some(channel) = target_channel.as_ref() {
+                let _ = channel.stop_typing(&msg.reply_target).await;
+            }
             let safe_err = providers::sanitize_api_error(&err.to_string());
             let message = format!(
                 "⚠️ Failed to initialize provider `{}`. Please run `/models` to choose another provider.\nDetails: {safe_err}",
                 route.provider
             );

Also applies to: 254-270

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/channels/runtime/dispatch.rs` around lines 218 - 227, The early
typing indicator started with target_channel.start_typing(&msg.reply_target) can
be left running if get_or_create_provider() later fails; update the control flow
in dispatch.rs so that either (a) start_typing is called only after
get_or_create_provider() succeeds, or (b) ensure any early start_typing is
always matched with target_channel.stop_typing(&msg.reply_target) on every error
return path (e.g., inside the provider acquisition error branch and other
early-return branches around get_or_create_provider()), referencing the
start_typing, stop_typing, get_or_create_provider, target_channel and
msg.reply_target symbols to find and fix both the 218–227 and 254–270 sites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants