feat(telegram): option to ignore messages that @mention humans (#64388) - #64456
feat(telegram): option to ignore messages that @mention humans (#64388)#64456webtecnica wants to merge 1 commit into
Conversation
alt-glitch
added
type/feature
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Feature adding option to ignore messages that @mention humans in Telegram. Substantial feature (243 additions, 428 deletions). No security concerns. Note: contains keywords that matched the security scan (telegram, ignore, allowlist) but these are standard feature-related terms, not credentials.
Reviewed by Hermes Agent
81d97f5 to
f9c94ed
Compare
|
Rebased onto upstream/main. The PR's original commit also touched polling resilience code that has since been rewritten on main. The rebase preserves the core feature ( |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for rebasing this focused Telegram feature. The issue premise still holds on current main: plugins/platforms/telegram/adapter.py:7762-7765 accepts free-response and require_mention: false group messages without a human-mention gate.
Problems
- The new filter at
plugins/platforms/telegram/adapter.py:7883runs only after the accepting returns at:7865-7868. It therefore cannot suppress the free-response path described in #64388. _message_only_mentions_humans()documents that an own-bot mention makes it return false, but it preservesfound_human_mention=Trueafter skipping an own-bot entity (:7111-7122,:7141). Mixed human-plus-bot mentions would not preserve the direct-address exception once the gate is moved.plugins/platforms/telegram/plugin.yaml:36-39adds a user-facing non-secret environment setting.AGENTS.md:102-107requires behavioral configuration through config.yaml instead.- The two-file diff adds no regression tests;
tests/gateway/test_telegram_group_gating.py:153-156is the existing free-response contract that needs coverage for this option.
Suggested changes
- Gate human-only mentions before free-response / disabled-require-mention admission, after preserving replies and direct bot addressing (including
/command@botname). - Keep this as
telegram.ignore_human_mentionsconfig only, remove the new environment variable, document it, and add gating tests for the direct-address and caption/entity cases.
Automated hermes-sweeper review.
| # The message already passed all other gates (free_response, require_mention | ||
| # disabled, reply-to-bot, @bot, wake-word patterns) so this is the final | ||
| # filter before falling through to the default-reject path. | ||
| if self._telegram_ignore_human_mentions() and self._message_only_mentions_humans(message): |
There was a problem hiding this comment.
This branch is unreachable for the feature’s target mode: free_response_chats and require_mention: false both return True at lines 7865-7868 before reaching here. Move the suppression decision ahead of those admissions, while retaining the reply/direct-bot exceptions.
| if isinstance(configured, str): | ||
| return configured.lower() in {"true", "1", "yes", "on"} | ||
| return bool(configured) | ||
| return os.getenv("TELEGRAM_IGNORE_HUMAN_MENTIONS", "false").lower() in { |
There was a problem hiding this comment.
Please keep this behavioral setting config.yaml-only and remove the new optional non-secret environment fallback. The repository policy in AGENTS.md:102-107 reserves .env for credentials/secrets.
teknium1
added
sweeper:risk-message-delivery
SummaryNineteen PRs address or reference this Telegram group-routing complex: #64456 is the direct implementation for #64388, while the others establish or refine mention gating, command routing, entity parsing, configuration, and documentation. The #64456 diff adds optional human-mention detection, but the guard follows the free-response and Related pull requests
Duplicates#1977, #2000, #2508, #3539, and #4019 overlap on Telegram Suggested consolidationKeep #64456 open with a salvage path, consistent with the contributor keep_open review: move the optional human-mention guard before free-response and Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64388(["issue #64388 (open)"])
P64456["PR #64456 (open)"]
P64456 -->|best fix| I64388
class I64388 open
class P64456 open
class P64456 best
class P64456 target
click I64388 "https://github.com/NousResearch/hermes-agent/issues/64388"
click P64456 "https://github.com/NousResearch/hermes-agent/pull/64456"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 19 pull requests and 6 issues in this complex. Each diff was read against this issue; Assessment working set: 140 kB of PR diffs, 49 kB of issue/PR text, 10 kB of discussion (20 comments), 26 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
5 participants
Fixes #64388. Adds ignore_human_mentions config option (and TELEGRAM_IGNORE_HUMAN_MENTIONS env var) so bot ignores group messages that @mention human users.