Skip to content

fix: preserve prioritized Telegram menu commands - #55192

Closed
cortanapham wants to merge 1 commit into
NousResearch:mainfrom
cortanapham:fix/telegram-menu-priority-before-cap
Closed

cortanapham wants to merge 1 commit into
NousResearch:mainfrom
cortanapham:fix/telegram-menu-priority-before-cap

Conversation

@cortanapham

Copy link
Copy Markdown

Summary

  • Preserve configured Telegram menu priority before applying the Telegram command cap.
  • Let gateway skill command collection gather all candidates when prioritization needs to happen globally.
  • Add regression coverage for a priority skill command surviving an otherwise-full menu.

Tests

  • venv/bin/python -m pytest tests/hermes_cli/test_commands.py::TestTelegramMenuCommands -q -o 'addopts='
  • venv/bin/python -m pytest tests/hermes_cli/test_commands.py -q -o 'addopts='

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have labels Jun 29, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Clean 2-file fix preserving prioritized Telegram menu commands. Prevents command ordering from being lost during sync. Well-scoped with tests.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused regression fix. The premise is confirmed on current main: hermes_cli/commands.py:916-923 limits skill collection before the combined list can be priority-sorted, and :926 caps that un-reprioritized result. The proposed global ordering addresses that path and the added skill-under-cap regression test covers it.

Problems

  • hermes_cli/commands.py:897-902 still describes a fixed core → plugin → skill ordering. After this change, a configured-priority skill can be promoted ahead of those categories before the cap, so that contract becomes stale.

Suggested changes

  • Update that docstring to describe priority application over the combined eligible command list before truncation.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Fixed on main by #98280 (commit 3e6229e) — same priority-before-cap idea you submitted first; see the credit note on #57938. Thanks @cortanapham!

@teknium1 teknium1 closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants