feat(gateway): Discord tool progress bubbles auto-delete when job is done - #18306
Closed
nazirulhafiy wants to merge 23 commits into
Closed
feat(gateway): Discord tool progress bubbles auto-delete when job is done#18306nazirulhafiy wants to merge 23 commits into
nazirulhafiy wants to merge 23 commits into
Conversation
alt-glitch
added
type/feature
Collaborator
Collaborator
|
Likely duplicate of #14877 — same feature (auto-delete progress bubbles on Discord), same approach (delete_message + reset handler wiring). |
nazirulhafiy
force-pushed
the
feat/delete-progress-bubble
branch
2 times, most recently
from
May 2, 2026 16:46
089d667 to
8d64179
Compare
nazirulhafiy
changed the title
nazirulhafiy
force-pushed
the
feat/delete-progress-bubble
branch
2 times, most recently
from
May 5, 2026 00:35
3d74d6c to
a103414
Compare
Adds delete_message() to the Discord adapter and wires it into both __reset__ handlers in the progress message loop, plus the final edit drain path. When a content bubble lands after a tool batch, the progress bubble (🔧, 📖, 💻) is now deleted instead of left orphaned in the channel, reducing chat clutter. - gateway/platforms/discord.py: new delete_message() method - gateway/run.py: delete in normal __reset__, CancelledError drain, and final-edit path — with capability gate via 'type(adapter).delete_message is not BasePlatformAdapter.delete_message'
…o-delete - Final-drain path (3rd location) now logs delete attempts and errors, matching the __reset__ handlers — so lingering bubbles are diagnosable. - Adds config key `display.auto_delete_tool_progress` (default: true). Set to `false` in config.yaml to disable progress bubble auto-deletion. - All three delete locations respect the toggle.
…eanup Bug: The __reset__ delete handler gated deletion on can_edit, which flips to False on any edit failure (e.g. Discord 429 rate limit on rapid tool calls). Once can_edit went False, every subsequent progress bubble became permanent. Fix: - Add can_delete flag alongside can_edit - Use can_delete in the __reset__ delete gate instead of can_edit - Track can_delete = False only when delete_message() actually fails, not when edit fails - Apply same failure tracking to CancelledError drain and final drain delete paths This ensures Discord rate-limiting an edit mid-session no longer disables progress bubble deletion.
…o-delete Adds 8 tests covering: - Capability check: overridden vs base adapter delete_message detection - Adapter behavior: success, failure, and multiple sequential calls - Config resolution: auto_delete_tool_progress default, explicit off, and per-platform overrides via display_config Pragmatic approach: tests verify the component pieces directly (capability checks, adapter contract, config resolver) rather than requiring a full _run_agent integration test, since the delete code path requires an active Discord adapter + live agent loop that doesn't map cleanly to the mock-based test harness. Part of PR #18306.
…nterrupt Bug: When the agent finishes normally (not interrupted/cancelled), _run_still_current() returned False and send_progress_messages() exited via early return at the top of the main loop body. This bypassed the CancelledError handler where the delete calls live, so tool progress bubbles (e.g. ⏰ cronjob) lingered permanently in Discord. Fix: Added delete logic at the early-return site (normal completion) matching the existing pattern in the CancelledError drain handler. All three exit paths now clean up: normal completion, __reset__ drain, and final-drain. Part of PR #18306.
- Add delete to first _run_still_current() check (race: run completes before __reset__ signal arrives). Tagged (run-completed). - Move __reset__ delete outside can_edit gate (Discord supports delete even when edit fails). Fixes nested-gate bug May 2026.
nazirulhafiy
force-pushed
the
feat/delete-progress-bubble
branch
from
May 5, 2026 00:51
a103414 to
9838993
Compare
Contributor
Author
Restart-resilient progress bubble cleanupThis PR introduced auto-delete for tool progress bubbles. Here is an extension that fixes the restart gap: Problem: If the gateway restarts mid-session, the cleanup hook never runs → stale progress bubbles persist in Discord. Fix: Persist the pending cleanup queue to
Commit on Tests added in The restart-drain logic was verified live: startup log showed |
- Add pending_progress_cleanup.json queue at ~/.hermes/gateway/ - Record (platform, chat_id, message_id) on bubble creation - Clear record on successful delete (4 existing paths + startup drain) - Retry failed deletes up to 5 attempts - GatewayRunner.start() drains stale records after adapter connect - Add TestPersistedProgressCleanup (3 tests: round-trip, startup success, startup failure retries) Extends PR #18306 (auto-delete tool progress bubbles).
hermes config set model.aliases.xxx commands write to the model.aliases nested key, but _load_direct_aliases() only read from the top-level model_aliases key. This meant aliases set via hermes config set were invisible to the /model command, and unrecognised inputs fell through to the DeepSeek normaliser which mapped everything to deepseek-chat. Add a second pass in _load_direct_aliases() that reads model.aliases and converts string-value entries (provider/model format) into DirectAlias objects. The provider is parsed from the slash prefix; if no slash, the current default provider from config is used. Also prevent simple aliases from overriding explicit model_aliases dict entries when both exist.
The Telegram/Discord /model pickers currently call list_authenticated_providers(), which returns every provider whose credentials resolve locally and every model in its curated snapshot. Two failure modes fall out: - OpenRouter rows can include IDs the live catalog no longer carries. - Provider rows can surface with zero callable models (e.g. a slug whose credential pool entry exists but has nothing behind it). list_picker_providers() wraps the base function and post-processes the result so the interactive picker only shows models the user can actually select: - OpenRouter's models come from fetch_openrouter_models() (live-catalog filtered against the curated OPENROUTER_MODELS snapshot). - Rows with an empty models list are dropped, except custom endpoints (is_user_defined=True with an api_url) where the user may enter model ids manually. - All other fields pass through unchanged. The gateway /model handler switches to the new helper for the interactive picker payload only. Typed /model <name> and the text fallback list stay on list_authenticated_providers() so nothing is hidden from power users or platforms without a picker. Covered by nine focused unit tests in tests/hermes_cli/test_list_picker_providers.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ative providers (#20802) OpenCode Go and OpenCode Zen are flat-namespace model resellers — their /v1/models returns bare IDs (deepseek-v4-flash, minimax-m2.7), and the inference API rejects vendor-prefixed names with HTTP 401 'Model not supported'. Two bugs fixed: 1. `switch_model` in hermes_cli/model_switch.py was silently switching the user off opencode-go to native deepseek when they typed `/model deepseek-v4-flash`. Step d found the model in opencode-go's live catalog, but step e (detect_provider_for_model) still ran and matched the bare name against deepseek's static catalog. Fix: track whether the live catalog resolved it; skip step e when it did. 2. `normalize_model_for_provider` in hermes_cli/model_normalize.py only stripped the exact `opencode-zen/` prefix, leaving arbitrary vendor prefixes like `minimax/minimax-m2.7` (commonly copied from aggregator slugs into fallback_model configs) intact — causing HTTP 401s when the fallback chain activated. Fix: opencode-go/opencode-zen strip ANY leading vendor prefix because their APIs are flat-namespace. Tests: 11 new cases in tests/hermes_cli/test_opencode_go_flat_namespace.py covering both normalization (prefix stripping, regression guards for opencode-zen Claude hyphenation and openrouter vendor-prepending) and switch_model (bare-name resolution on opencode-go's live catalog must not trigger cross-provider hijack). Reported by @UFOnik via Discord; Kimi K2.6 always worked because moonshotai has no overlapping entry in a native provider's static catalog. Deepseek and minimax failed because their v4/v2.7 names existed in the native deepseek/minimax catalogs.
…ential resolution When no explicit delegation.model/provider is configured, _resolve_delegation_credentials() now consults smart-route's resolve_child_model() to pick the correct child model from the parent's model family (e.g. deepseek-v4-pro → deepseek-v4-flash via opencode-go). Graceful degradation: if smart-route plugin is not loaded (CLI/cron context), falls back to inheriting parent's exact model. Explicit delegation config still takes priority.
…ion credential resolution" This reverts commit 7132caa.
…bubble # Conflicts: # gateway/run.py
…t fixture The test _install_fakes only mocked _load_gateway_config (which controls _cleanup_progress), but the progress consumer also reads auto_delete_tool_progress from a separate config source (hermes_cli.config.load_config). This defaults to True, causing progress bubbles to be deleted even when cleanup_on=False. Monkeypatch hermes_cli.config.load_config to return auto_delete_tool_progress matching the test's cleanup_on parameter.
loop.add_signal_handler is wrapped in try/except NotImplementedError on line 245-250, but the footgun checker flags it by line. Suppress with inline comment.
Python 3.11+ raises TypeError when code tries to catch a MagicMock in an except clause (magic mocks don't inherit from BaseException). The main branch's new _fetch_channel_context uses 'except discord.Forbidden' which triggers this on CI where discord is mocked. Define Forbidden, HTTPException, NotFound, DiscordException, and InvalidArgument as real Exception subclasses so except clauses work in mocked environments.
Summary of fixes: - test_plugin_discovery: bump profile count 33→34 (google-gemini-cli added) - test_context_compressor_summary_continuity: move handoff message after protected head region so _find_latest_context_summary finds it - test_update_autostash: mock _refresh_active_lazy_features to prevent lazy backend pip installs from polluting the command assertion - test_compression_feasibility: add _custom_providers=[] to _make_agent and update 3 mock_ctx_len assertions to include custom_providers=[]
Contributor
Author
|
Closing in favor of the cleaned replacement PR: #26055\n\nThat branch was rebuilt to remove unrelated history and keep the change focused on Discord tool-progress bubble cleanup. |
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
4 participants
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.
What does this PR do?
Discord progress bubbles — the ephemeral messages showing tool icons (💻, 🐍, 📖, 🖼️, 🔍, 🌐, etc.) — were left behind after each tool finished executing. Over time, channels accumulated orphan bubbles that served no purpose once their job was done.
Root cause: The progress message system in
send_progress_messages()edited bubbles to show progress but did not reliably delete them on completion, interruption, restart/drain, or final cleanup paths.Fix: Add Discord message deletion support and wire progress-bubble cleanup through the gateway progress loop. Cleanup now covers normal completion, reset/interruption handling, final edit drains, and persisted restart-survivable cleanup. Deletion is capability-gated so only adapters that implement
delete_message()are affected. Controlled bydisplay.auto_delete_tool_progress(enabled by default).Related Issue
N/A. Discovered during real-world Discord usage — channels accumulated stale progress bubbles after every tool call.
Type of Change
Changes Made
Primary progress-bubble changes:
gateway/platforms/discord.py(+19/-0): Addeddelete_message()method that fetches and deletes a message by ID. Handles both channel and thread messages. Failures are non-fatal.gateway/run.py(+243/-3): Wired auto-delete into the progress message lifecycle, including:__reset__/ interruption pathstests/gateway/test_progress_auto_delete.py(+169/-0): Added coverage for capability checks, adapter behavior, config resolution, cleanup persistence, and progress bubble deletion paths.tests/gateway/test_run_cleanup_progress.py(+13/-0): Synced cleanup tests with the globaldisplay.auto_delete_tool_progressconfig source.CI/test stabilization added while rebasing onto current
main:tests/e2e/conftest.py(+8/-0): Replaced Discord exception mocks with realExceptionsubclasses so Python 3.11+except discord.Forbiddenpaths work under tests.tests/agent/test_context_compressor_summary_continuity.py(+3/-2): Kept the handoff fixture above the compression threshold after merging currentmain.tests/run_agent/test_compression_feasibility.py(+1/-0): Added missing_custom_providersfixture state expected by current compression logic.agent/auxiliary_client.py(+1/-1),tests/agent/test_auxiliary_client.py(+1/-0),run_agent.py(+1/-0): Small test/cleanup fixes needed to keep current main’s auxiliary-client and background-review tests isolated and green.How to Test
Manual verification:
what time is it in Tokyo.display.auto_delete_tool_progress: falseinconfig.yamland verify progress bubbles persist (old behavior).Targeted tests:
Checklist
Code
delete_message+ reset handler wiring). Will defer to maintainers on which PR should be canonical.main.Documentation & Housekeeping
display.auto_delete_tool_progressconfig key.delete_messageonly fires on platforms whose adapter overrides it. No impact on unsupported platforms.Verification Evidence
Production-verified during real Discord sessions — progress bubbles auto-delete after tool completion. Tested with terminal (💻), code (🐍), search (🔍), web (🌐), and read (📖) tools.
Config override works as expected:
display.auto_delete_tool_progress: falsepreserves old orphan-bubble behavior.Latest PR GitHub Actions status after rebasing onto current
main: