fix(gateway): clean up Discord typing tasks on teardown (resubmit of #23083) - #47222
fix(gateway): clean up Discord typing tasks on teardown (resubmit of #23083)#47222nazirulhafiy wants to merge 1 commit into
Conversation
alt-glitch
added
type/bug
|
Thanks for preserving the Discord teardown fix from #23083. Current main still keeps Discord typing loops outside the base background-task registry ( Problems
Suggested changes
This is an automated hermes-sweeper review. |
teknium1
added
sweeper:risk-compatibility
The persistent per-channel typing loops spawned by send_typing() are not tracked by BasePlatformAdapter's message-processing task registry, so on gateway restart, exception path, or signal handler, they were never cancelled. The typing endpoint's own 10-second timeout eventually cleared the indicator on Discord's side, but Hermes would hang in a "typing..." state for at least that long after every shutdown, with no way to abort early. This drains the entire _typing_tasks dict from two teardown points: 1. disconnect() — explicit operator-initiated close. Awaits _cancel_typing_tasks before tearing down voice clients and the discord.py client, so no in-flight loop survives the close. 2. cancel_background_tasks() — the base adapter's shutdown hook called from gateway/run.py on global shutdown. Drains Discord-specific tasks first, then defers to super() so the base cleanup still runs. _cancel_typing_tasks() snapshots the dict, clears it immediately, cancels each not-done task, and waits up to 2.0s for them to acknowledge the cancel via asyncio.gather(..., return_exceptions=True). Anything still alive at 2s logs a warning rather than blocking shutdown indefinitely — the typing indicator on Discord's side will clear on its own after ~10s even if the bot process is gone. Resubmission of NousResearch#23083 (auto-closed 2026-05-27 due to personal files in the diff). The original 1-file +39-line patch is preserved verbatim in spirit; only the file path changed because commit cc8e5ec (2026-05-12, pre-closure) migrated the Discord adapter from gateway/platforms/discord.py to plugins/platforms/discord/adapter.py as a bundled plugin with full Teams parity. The bug surface and fix logic are identical to the original. Repro path (from the original PR's context): on a healthy gateway, run /ask in 2+ Discord DMs back-to-back. Force-kill the gateway process (kill -9) or send SIGTERM. The "Hermes is typing..." indicator in any chat that had not yet hit its stop_typing() call will linger for ~10s after the bot process is gone. With this patch, _cancel_typing_tasks runs from disconnect() and from the shutdown handler before the process exits, so Discord sees the typing loop end promptly.
7fd538a to
60835c5
Compare
Thanks — agreed. I’ve split this down to the Discord teardown fix only: kept _typing_tasks cleanup on disconnect() and cancel_background_tasks() I also added regression tests for Discord typing-task drain on shutdown/disconnect and verified the existing pending text-batch cleanup still passes. Pushed as 60835c5 on fix/discord-typing-cleanup-v2. |
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address Discord teardown or adjacent channel-gating failures: #8008 introduced channel-name matching and pending-send flushing, #55525 merged that work with slash-command coverage and a bounded flush deadline, while #47222 separately drains unregistered Discord typing tasks during teardown.
Related pull requests
- #8008 [closed]
related— (+221/-13) — superseded by #55525: It added ID/name/#name matching across message gates and flushed pending text-batch tasks on shutdown; #55525 cherry-picked both fixes with attribution and completed the missing slash-command and timeout-budget handling. - #47222
related— (+104/-12) — merge: The current diff narrowly cancels and awaits Discord-owned_typing_tasksfrom both background-task cleanup and disconnect, with regression coverage, addressing the teardown leak not covered by #55525. This follows the keep_open review on #47222 by salvaging only the Discord typing-task drain; the reviewed stale cross-platform metadata propagation and unrelated environment change are absent from this diff. - #55525 [merged]
related— (+310/-16) — merged reference implementation: It incorporates #8008's two fixes and extends them to slash-command authorization while keeping the pending-send flush deadline below the adapter teardown budget.
Duplicates
#8008 and #55525 implement substantially the same channel-name matching and pending-send shutdown fix; #55525 is the completed, merged superseding implementation. #47222 is not a duplicate because it handles the separate Discord typing-task registry.
Suggested consolidation
Merge #47222 as the focused remaining Discord typing-task teardown fix; its current diff addresses the contributor's keep_open salvage request without the blocked generic metadata API changes. Keep merged #55525 as the canonical implementation for #8008's work, and leave #8008 closed as superseded by #55525.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 44 kB of PR diffs, 6 kB of issue/PR text, 4 kB of discussion (3 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
5 participants
Fix Discord typing cleanup metadata propagation.
What changed:
thread_idmetadata through the processing stop path ingateway/platforms/base.py.plugins/platforms/discord/adapter.py.Verification:
python -m pytest tests/gateway/test_discord_typing_indicator.py tests/gateway/test_keep_typing_timeout.py tests/gateway/test_run_progress_topics.py::test_thread_metadata_is_forwarded_to_processing_stop_typing -qNotes: