Skip to content

Python: fix(telegram): ignore commands addressed to other bots - #8803

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
dakjdakd:fix/telegram-command-target
Sep 30, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
dakjdakd:fix/telegram-command-target

Conversation

@dakjdakd

@dakjdakd Lucien (dakjdakd) commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

The Python Telegram command parser strips @botname without checking which bot received the update. If a bot receives /new@otherbot in a group, all three official Telegram samples can treat it as their own /new: the two local samples delete their shared group session, and the Foundry-hosted sample clears its Cosmos history. Unknown commands addressed to another bot can fall through to the agent. This PR makes the samples check the target before either dispatch or model invocation.

Description & Review Guide

  • What are the major changes? telegram_command(update, *, bot_username=None) now checks an optional username case-insensitively. Command extraction also recognizes top-level message.caption and edited_message.caption after message text and callback data, so commands in photo or document captions pass through the same target check. The local polling and webhook samples obtain their username through aiogram's cached Bot.me() call. The Foundry-hosted sample obtains it through Telegram getMe and caches it on its runtime; a per-runtime async lock ensures concurrent first lookups share one request. Each handler returns immediately for a command addressed to another bot. Regression tests remain in the Telegram package's parser tests, with package and sample README updates accompanying the change.
  • What is the impact of these changes? For the official samples, /new@otherbot in message text or a media caption is ignored before a session or history operation and before the agent sees the content. /new and /new@mybot retain their existing behavior. The new argument is optional: callers using telegram_command(update) continue to get the old target-agnostic normalization for suffixed commands. That residual behavior is deliberate for compatibility; changing the default would require a separate API decision.
  • What do you want reviewers to focus on? Please review the optional public parameter and the decision to preserve the default behavior, the early return in each of the three sample entry points, and the one-time getMe lookup in the Foundry runtime.
Input received by mybot Before this PR After this PR in the official samples
/new@otherbot Parser returns /new; the local handlers reach state.session_store.delete(session_id) or the Foundry handler reaches runtime.history.clear(session_id) by the source call paths. Returns before command dispatch or model invocation; no reset.
Photo/document caption /new@otherbot Parser returns None, so the samples can pass the caption and media to the agent. Recognized as a command for another bot and ignored before model invocation.
/new@MyBot Reset this bot's session. Still resets; username matching is case-insensitive.
/new Reset this bot's session. Still resets.

Evidence and verification. The old parser output was reproduced against c804f32c9. Before implementing the new parameter, its target-aware test failed with TypeError: telegram_command() got an unexpected keyword argument 'bot_username' (1 failed). The reset side effects in the first table row are derived from the parser-to-handler source paths, which are also present on the current PR base 2024d4df4; the pre-fix sample handlers were not run in a live group. The retained parser tests cover commands addressed to another bot, commands addressed to this bot, untargeted commands, media captions, and callback precedence. The sample handlers pass the bot username to that parser and return when it rejects a command.

During the ae20399c0 review follow-up, a concurrent cold-cache test observed two getMe calls before the lock (1 failure) and one afterward. That sample-only test was later removed in 4318d8e at maintainer request; the runtime lock remains in place.

The media-caption follow-up in 5a4e589eb covers the path identified during review. Before the parser change, the retained caption tests failed for both message and edited_message. After the change, the parser suite passes (48/48), including a test confirming callback data retains precedence over a top-level caption. In 4318d8e, I removed the local sample test file and the command-target additions to the Foundry sample tests as requested by the maintainer. The remaining Foundry sample suite passes (31/31). No live Telegram group test was run.

From python/, I ran these tests with the checkout's packages/hosting-telegram, packages/core, and packages/hosting directories on PYTHONPATH for the sample suites, and OTEL_SDK_DISABLED=true:

uv run --package agent-framework-hosting-telegram --with pytest --with pytest-asyncio --with pytest-timeout python -m pytest packages/hosting-telegram/tests/hosting_telegram/test_parsing.py -q
# 48 passed

uv run --no-sync --project samples/04-hosting/foundry-hosted-agents/invocations/telegram python -m pytest samples/04-hosting/foundry-hosted-agents/invocations/telegram/tests/test_main.py -q
# 31 passed

Ruff lint and format checks, Pyright checks for the Telegram package and Foundry sample, and git diff --check passed. No live Telegram group test was run.

I searched open and closed issues and PRs for telegram, telegram_command, otherbot, and bot-suffixed on 2026-09-28 and found no existing target-matching fix. #6588/#7047 introduced the helper and local samples; #7883 added the Foundry-hosted sample. None covers this command-routing defect.

Related Issue

Fixes #8802

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The Foundry username cache permits duplicate getMe requests during concurrent initialization.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds target-aware Telegram command parsing so samples ignore commands addressed to other bots.

Changes:

  • Adds optional case-insensitive bot username validation.
  • Integrates username lookup and caching across three samples.
  • Adds focused tests and documentation.
File Description
python/​packages/​hosting-telegram/​agent_framework_hosting_telegram/​_parsing.py Adds target-aware parsing.
python/​packages/​hosting-telegram/​tests/​hosting_telegram/​test_parsing.py Tests target matching.
python/​packages/​hosting-telegram/​README.md Documents the new parameter.
python/​samples/​04-hosting/​af-hosting/​local_telegram/​app.py Filters webhook commands.
python/​samples/​04-hosting/​af-hosting/​local_telegram/​polling_app.py Filters polling commands.
python/​samples/​04-hosting/​af-hosting/​local_telegram/​tests/​test_command_target.py Tests local sample routing.
python/​samples/​04-hosting/​af-hosting/​local_telegram/​README.md Documents routing behavior.
python/​samples/​04-hosting/​foundry-hosted-agents/​invocations/​telegram/​main.py Adds identity lookup and filtering.
python/​samples/​04-hosting/​foundry-hosted-agents/​invocations/​telegram/​tests/​test_main.py Tests Foundry routing and caching.
python/​samples/​04-hosting/​foundry-hosted-agents/​invocations/​telegram/​README.md Documents command isolation.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/samples/04-hosting/foundry-hosted-agents/invocations/telegram/main.py Outdated
@dakjdakd

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@eavanvalkenburg

Copy link
Copy Markdown
Member

Lucien (@dakjdakd) I don’t think we need dedicated test coverage in the sample directories for this change. Please keep the regression coverage in python/packages/hosting-telegram/tests/hosting_telegram/test_parsing.py, remove the new local_telegram sample test file, and drop the command-target test additions from the hosted-agent sample tests.

@dakjdakd

Copy link
Copy Markdown
Contributor Author

Thanks for the guidance. Addressed in 4318d8e: I removed the new local_telegram/tests/test_command_target.py file and all four command-target test additions from the Foundry-hosted sample test file, including the concurrent getMe test. The Telegram package's test_parsing.py retains the target-matching and media-caption regressions. No runtime behavior changed in this follow-up.

Verification after the cleanup: package parser tests 48/48, remaining Foundry sample tests 31/31, Ruff check and format check passed. I also updated the PR description to reflect the smaller test scope.

Merged via the queue into microsoft:main with commit 908ca8c Sep 30, 2026
46 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — 4318d8e5 Deployed Sep 29, 2026 by dakjdakd via add_label #24023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Telegram samples execute commands addressed to other bots

4 participants