Skip to content

fix(telegram): stop typing before command delivery - #73063

Open
vinsew wants to merge 2 commits into
NousResearch:mainfrom
vinsew:fix/telegram-command-typing-lifecycle
Open

fix(telegram): stop typing before command delivery#73063
vinsew wants to merge 2 commits into
NousResearch:mainfrom
vinsew:fix/telegram-command-typing-lifecycle

Conversation

@vinsew

@vinsew vinsew commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • stop the typing refresh before every final delivery, including slash-command replies
  • cover a command send that stalls indefinitely after delivery starts
  • keep session ownership and queued follow-up behavior intact

Root cause

The existing pre-delivery typing lifecycle fix excluded slash commands because their replies are normally instant. That assumption fails when Telegram accepts a command reply but the HTTP acknowledgement stalls on a half-closed connection.

In that state:

  1. the user can already see the command reply;
  2. the command coroutine remains blocked in delivery;
  3. the typing refresh task is only stopped at the coroutine tail;
  4. Telegram keeps receiving sendChatAction, so the bot appears to type indefinitely.

PR #25210 was closed as superseded by the post-delivery cleanup in #37556, but post-delivery cleanup cannot run while the delivery acknowledgement itself is stuck. This change closes that remaining boundary by stopping typing before the send begins.

Behavior

The change does not add a delivery timeout or alter retries. It does not release the active-session guard early. Messages arriving during delivery remain queued and drain after the current turn.

Two scheduler-sensitive tests now wait for their observable reply with a short bounded loop instead of assuming a fixed number of event-loop ticks; the extra ticks come from the pre-delivery cleanup itself.

Verification

  • 139 passed across topic sessions, split-brain command handling, Telegram formatting, and voice notify coverage
  • 242 passed, 2 skipped for gateway tests selected by typing or session_reset or slash_command, with an isolated HERMES_HOME
  • Ruff passed on changed files
  • git diff --check passed

Regression test

test_command_stops_typing_before_a_stuck_delivery holds a /new delivery open forever and asserts that the typing task has already stopped when delivery starts. It fails on current main and passes with this patch.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages needs-decision Awaiting maintainer decision before any implementation labels Jul 28, 2026

@teknium1 teknium1 left a comment

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.

Thanks for isolating the normal final-delivery boundary and adding a stalled-send regression.

Problems

  • The change does not cover the active-session /new and /reset route. gateway/platforms/base.py:5458 sends that command reply at :5504 before cancelling the existing processing task at :5518; that existing task owns the typing refresh created by _process_message_background (:5796). A stalled command send can therefore still leave typing active.
  • The new /new test calls _process_message_background directly, so it bypasses handle_message's active-session dispatch at gateway/platforms/base.py:5586-5601 and cannot cover that route.

Suggested changes

  • Stop or pause the existing typing refresh before the active-command sends at gateway/platforms/base.py:5504 and :5615, preserving the documented send-before-cancel ordering.
  • Add a handle_message regression with an active turn, a blocked /new send, and an assertion that typing is already stopped.

Automated hermes-sweeper review.

Comment thread gateway/platforms/base.py Outdated
# Stop typing before every final delivery, including slash
# commands. A command send can still stall after Telegram has
# accepted the message but before its HTTP acknowledgement
# returns; leaving typing alive until the coroutine tail would

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.

This only covers the ordinary _process_message_background path. An active-session /new uses _dispatch_active_session_command, which sends at current gateway/platforms/base.py:5504 before cancelling the old typing-owning task at :5518; a blocked command send there still leaves typing active. Please cover that dispatch path too.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 2026
@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from 410a863 to 2d47025 Compare August 3, 2026 17:22
@vinsew

vinsew commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the active-session /new//reset review gap and rebased the PR onto current main.

The adapter now tracks each session's typing refresh task separately from its processing task. _dispatch_active_session_command() stops that refresh before sending the command acknowledgement, while preserving the required response-before-processing-task-cancel ordering from #18912. Cleanup is owner-aware so an older task cannot remove a newer session's typing task, and shutdown clears remaining tracked refresh tasks.

The new regression test enters through handle_message() with an actually active session and asserts that the old typing refresh is already stopped when the /new acknowledgement send begins.

Validation on the rebased branch:

  • 50 passed across command bypass, topic sessions, typing timeout, and gateway shutdown tests
  • Ruff passed for the changed production and test files
  • git diff --check passed

@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from 2d47025 to a596572 Compare August 14, 2026 00:43
@vinsew

vinsew commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Rebased both typing-lifecycle commits onto current and force-pushed the refreshed branch. : 35 tests passed, 0 failures.

@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from a596572 to 7e1264f Compare August 18, 2026 22:51
@vinsew

vinsew commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Maintenance refresh: rebased both typing-lifecycle commits onto current upstream main (b359db7) and force-pushed head 7e1264fb47. Verification: tests/gateway/test_command_bypass_active_session.py — 35 passed; Ruff and git diff --check passed.

@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from 7e1264f to 2f796cc Compare August 21, 2026 05:32
@vinsew

vinsew commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Maintenance refresh: rebased onto current upstream main (40643cb) and force-pushed head 2f796ccc58. Verification: 35 active-session command tests; Ruff and git diff --check passed.

@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from 2f796cc to 60fea6f Compare August 27, 2026 16:18
@vinsew

vinsew commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto official main f3cbb26 (2026-08-28). No conflicts; gateway typing tests 58 passed / 5 skipped; ruff clean.

@vinsew vinsew closed this Aug 27, 2026
@vinsew
vinsew force-pushed the fix/telegram-command-typing-lifecycle branch from 9a5fc10 to 39f1e18 Compare August 27, 2026 17:41
vinsew added 2 commits August 28, 2026 01:41
Telegram command replies such as /new and /reset were excluded from the existing pre-delivery typing shutdown. If Telegram accepted the reply but the HTTP acknowledgement stalled on a half-closed connection, the command coroutine remained in delivery while its typing refresh task continued indefinitely.

Apply the pre-delivery typing stop to every final response, including slash commands. Keep the active-session guard, pending-message drain, delivery retry, and final notification metadata behavior unchanged.

Add a regression test that holds a slash-command delivery open forever and proves typing is already stopped before the send starts. Replace a brittle two-event-loop-tick assertion in the split-brain command test with a bounded readiness loop so the extra cleanup hop is tested by outcome rather than scheduler timing.

Verification:

- 139 focused topic/session/Telegram-format/voice-notify tests passed

- 219 gateway typing/session-reset/slash-command tests passed, 2 skipped

- Ruff passed

- git diff --check passed
…ete scheduling

The pre-delivery typing stop originally awaited _stop_typing_before_delivery
inline before the final send. _stop_typing_refresh waits up to 0.5s on the
cancelled typing task, and that inline wait reordered the loop scheduler:
the ephemeral-reply test's detached delete task no longer completed within
its pump window, so tests/gateway/test_ephemeral_reply.py
::test_process_message_unwraps_ephemeral_before_send failed on official CI
(deleted list empty at assert time).

Convert the stop to a fire-and-forget background task registered in
_background_tasks (official discard-callback convention). The stop still
begins before the send yields to the network, so the typing-indicator
stall the PR fixes remains fixed, while the turn no longer blocks on the
0.5s cancel-wait.

Verification: test_ephemeral_reply 5 passed; typing/ephemeral focused
76 passed / 5 skipped; command bypass + ephemeral 40 passed; ruff clean.
@vinsew vinsew reopened this Aug 27, 2026
@vinsew

vinsew commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto official main (2026-08-28, post-v0.20.6). Scope change: dropped the second commit (per-session typing-task registry for reset replies) — official main has since reworked the command/reset typing path (slash_commands mixin now clears typing when no active agent owns the session), and the carried registry implementation deadlocked test_active_session_text_merge on the current tree (300s CI timeout, deterministic). The remaining two commits carry the still-unabsorbed value: stop typing before slash-command delivery without stalling ephemeral delete scheduling. active_session_text_merge 7 passed; typing/ephemeral/command-bypass focused suite 119 passed / 5 skipped; ruff clean.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants