feat(telegram): support quick-command-only menus - #62683
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for reworking this against the current command-menu configuration path.
Problems
hermes_cli/commands.py(the new_telegram_quick_menu_commands) turnsagent-healthintoagent_health, but gateway quick-command dispatch uses exact raw-key lookup atgateway/run.py:9983. The PR's own generated menu entry therefore cannot invoke that configured quick command.- The helper also bypasses the existing 32-character command-name handling.
_sanitize_telegram_name()only normalizes characters (hermes_cli/commands.py:708-719); the established clamp/collision contract is covered attests/hermes_cli/test_commands.py:1061-1103.
Suggested changes
- Add collision-safe menu-name-to-config-key resolution before quick-command dispatch, with an end-to-end
/agent_healthtest for a configuredagent-healthcommand. - Enforce the 32-character limit and preserve that mapping after any truncation/collision handling. Cover startup and forum registration paths.
Automated hermes-sweeper review.
| for raw_name, raw_config in quick_commands.items(): | ||
| if not isinstance(raw_name, str) or not isinstance(raw_config, Mapping): | ||
| continue | ||
| if raw_config.get("show_in_telegram_menu") is False: |
There was a problem hiding this comment.
_sanitize_telegram_name("agent-health") produces agent_health, but GatewayRunner looks up quick commands by the exact incoming command at gateway/run.py:9983; this menu entry cannot run the configured agent-health command. This path also needs the existing 32-character collision handling before passing names to Telegram. Please preserve a collision-safe mapping from emitted menu names to their raw quick-command keys and cover the selected-menu dispatch path.
|
Addressed the automated review in 5ba9a0d:
Validation: 203 passed across test_commands.py, test_telegram_forum_commands.py, and test_slash_access_dispatch.py. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address the focused Telegram quick-command menu: #28015 introduced the opt-in menu and visibility flag, #28529 integrated that implementation with the then-current multi-scope registration path, and #62683 adapts the feature to the current canonical command-menu configuration while adding sanitized-name dispatch mapping.
Related pull requests
- #28015 [closed]
related— (+136/-3) — superseded by #28529: Introducedquick_commands_only, per-command menu hiding, sanitization, tests, and documentation, but accessed quick commands throughPlatformConfig, which does not carry them directly; the contributor discussion confirms that #28529 reworked and merged this contribution with authorship preserved. - #28529 [merged]
related— (+145/-3) — merged reference implementation: Reworked #28015 to obtain top-level quick commands through a runner/config fallback and combined the focused menu with multi-scope Telegram registration, directly addressing the same specialist-bot menu requirement. - #62683
related— (+217/-0) — keep open pending remaining review coverage: Uses the current nested command-menu configuration and top-levelquick_commands, and its diff addresses the keep_open review's dispatch and 32-character collision concerns with reversible menu-name mapping plus helper and end-to-end dispatch tests. However, the contributor review also requested startup and forum registration coverage; although the author reports running forum tests, the visible diff adds no startup/forum registration test, so that review point is not yet demonstrably closed.
Duplicates
#28015 and #28529 are substantially the same implementation lineage, with #28529 being the merged salvage and integration of #28015; #62683 implements the same user-facing feature against a different current configuration and dispatch path.
Suggested consolidation
Keep #62683 open rather than merging it over the contributor's keep_open review; require explicit startup and forum-registration regression coverage, then re-review for merge. #28015 is already superseded by merged #28529, while #28529 should remain as the merged historical reference rather than be treated as an active competing PR.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 33 kB of PR diffs, 3 kB of issue/PR text, 2 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
Why this version
This is a current-main replacement for the quick-menu work reverted in #28575. It does not rely on a gateway runner reference. Both command-menu settings and quick commands are read from the same canonical config file, so the configured entries reach startup and forum menu registration without adapter wiring.
Tests
scripts/run_tests.sh tests/hermes_cli/test_commands.py tests/gateway/test_telegram_forum_commands.py -q
180 passed.