[FIX] telegram: queue sends issued before the adapter is ready instead of dropping them#242
Open
MartinEbner wants to merge 1 commit into
Open
Conversation
…ropping them send_message silently dropped messages while the poll loop had not yet connected or no chat was bound — most visibly the first message the agent emits right after launch, which races the adapter's first getUpdates long-poll (_connected only turns true once that call returns, up to poll_timeout seconds after start). Now such sends are queued (bounded at 50, oldest dropped with a warning) and flushed as soon as polling succeeds and a chat is bound: [SEND_QUEUE] queued chars=N (K pending) [SEND_QUEUE] full (50): dropped oldest queued message [SEND_FLUSH] delivering queued message (K left) [SEND_FLUSH] delivery failed, stopping drain (K still queued) Queued messages are flushed in order; a new ready send flushes the queue first and, if a failed drain left messages behind, joins the queue instead of overtaking them — delivery order stays chronological. The flush runs from the poll loop after update processing (so a chat bound in that iteration flushes immediately) with a non-blocking lock acquire, so polling never stalls behind a send already being drained by another thread; the auth-bound confirmation is enqueued rather than sent inline for the same reason. If a queued delivery fails mid-drain, the drain stops: the failed message is dropped (matching direct-send semantics) and the remainder stays queued for the next poll-cycle flush, so a rate-limit burst cannot burn the whole queue. Queued messages intentionally survive stop/start so reconnects do not lose them. The chunked delivery path is extracted into _deliver(), unchanged apart from returning delivery success. Complements the open send-hardening PR asi-alliance#240: with that PR's logging deployed, the log now shows these messages being skipped before the connection is established ([SEND_SKIP] connected=False bound=True chars=N); this PR turns those skips into queued deliveries. The queueing supersedes asi-alliance#240's [SEND_SKIP] log-and-drop in the unready branch; whichever lands second re-applies its half of send_message (mechanical rebase; asi-alliance#240's per-chunk ok/fails counters map onto _deliver's success return as fails == 0). Adds Autotests/unit/test_telegram_queue_until_connected.py (in-process, module loaded by file path, _api_call stubbed; no container/network/token), wired into Autotests/run_mandatory.
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
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.
Messages sent through
send_messagewhile the poll loop hasn't connected yet (or before achat is bound) are dropped silently. The window is real:
_connectedonly turns true once thefirst
getUpdateslong-poll returns — up topoll_timeoutseconds after start — so the firstthing an agent says after launch can vanish. We observed exactly this on a live instance
running the #240 logging: the agent's post-restart status message (106 chars, 15 s after boot)
was skipped —
[SEND_SKIP] connected=False bound=True— while the agent's history recorded itas sent. In auto-bind setups the loss is broader: everything sent before the first user DM is gone.
This PR queues unready sends and flushes them once the adapter is ready:
Design notes:
memory silently. Count-based bound; recency wins over stale boot messages.
iteration is served immediately; the poll thread uses a non-blocking lock acquire so
getUpdatesnever stalls behind a drain another thread is running. (When the poll threaddrains itself, sends are inline — same as the existing auth confirmation on main.)
failed drain left messages behind, it joins the queue instead of overtaking them.
(matching direct-send semantics today, and avoiding a poison message pinning the queue head)
and the rest stays queued for the next poll-cycle flush — a rate-limit burst can't burn the
whole queue in one failing drain.
stop_telegram/start_telegram, so an in-process reconnectloses nothing. Corollary (accepted): in auto-bind, messages queued before a rebind deliver to
the newly bound chat — the same trust model as auto-bind itself.
_deliver(), byte-identical apartfrom returning delivery success.
Tests:
Autotests/unit/test_telegram_queue_until_connected.py— 8 in-process unit tests(module loaded by file path,
_api_callstubbed; no container, network, or token), wired intoAutotests/run_mandatory: queued-not-dropped (disconnected, and connected-but-unbound),in-order flush, ready-send-flushes-first, no overtaking after a failed drain, bounded drop,
drain stop on delivery failure, drain stop on mid-flush disconnect. The poll-loop trigger and
auth-bound enqueue paths are not unit-tested (driving
_poll_loopneeds a live thread — scopecut).
Run:
pytest Autotests/unit/test_telegram_queue_until_connected.pyPossible follow-ups (not in this PR to keep it minimal): a bounded retry for the failed
head-of-line message with 429
retry_afterbackoff (would pair with #18, which proposes thesame for LLM-provider calls); a byte-size cap alongside the count bound.
Relation to #240 (send-status logging / chunk hardening): complementary — #240 made these drops
visible, this PR removes them; the queueing supersedes #240's
[SEND_SKIP]log-and-drop in theunready branch. Both touch
send_message, so whichever lands second gets a mechanical rebase(#240's per-chunk ok/fail counters map onto
_deliver's success return asfails == 0).Suggested order: #240 first (older, smaller), this second.