[Channel RFC] Add Telegram channel adapter#289
Conversation
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## dev #289 +/- ##
==========================================
+ Coverage 70.41% 70.50% +0.09%
==========================================
Files 1190 1198 +8
Lines 85882 86188 +306
Branches 11258 11297 +39
==========================================
+ Hits 60473 60768 +295
- Misses 21029 21033 +4
- Partials 4380 4387 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Review 纪要整体方向 OK: 高优先级1.
按 CLAUDE.md「单线程事实源 / 无锁优先,需要加锁先判定为破坏 Actor 边界」,建议把这层状态改成:一个内部 2.
3. Webhook secret 比较非恒定时间 中优先级4. 5. 6. 7. Long-poll loop 对所有异常同等对待 8. 低优先级 / Nit9. 10. 11. 12. 13. 目录位置 架构合规核查(快扫)
建议最低限度合并前修 1(流式并发)、2(parseMode)、3(恒定时间比较)、4(默认 poll timeout)。其余可以 follow-up issue。 |
Review-fix 复核(commit
|
…262-telegram-channel-adapter # Conflicts: # aevatar.foundation.slnf
Replace the per-platform Aevatar.GAgents.Channel.Telegram adapter project with a renderer-only Aevatar.GAgents.Platform.Telegram package matching the new ADR-0013 Channel.NyxIdRelay backbone. The composer, native producer, outbound message, and payload redactor mirror the Lark platform package. Delete the legacy adapter project, its options/credentials/streaming/webhook support files, and the old conformance/mode test project that targeted the direct-callback contract no longer in the supported production path. Wire Platform.Telegram + the new test project into aevatar.slnx and aevatar.platforms.slnf, and remove the deleted Channel.Telegram entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add NyxTelegramProvisioningService implementing the Lark-style
provisioning flow for Telegram: create Nyx relay api-key, register the
bot with Nyx via platform="telegram"+bot_token, create a default
conversation route, and register the local mirror through
ChannelBotRegistrationStoreCommands. Mirror NyxLarkProvisioningService's
INyxChannelBotProvisioningService surface so the registration endpoint
discovers Telegram by enumeration.
Generalize NyxChannelBotProvisioningRequest with an optional Credentials
map for platform-extensible secrets — Telegram reads "bot_token" from
the map. The Lark typed sub-message stays in place so the existing
Lark provisioning path is unchanged.
Wire the registration endpoint to accept the Telegram-shaped fields
(top-level bot_token shorthand or generic credentials map), platform-
default the response nyx_provider_slug ("api-telegram-bot" for Telegram),
and register both the Telegram composer/native producer/redactor and the
new provisioning service in ChannelRuntime DI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror Aevatar.AI.ToolProviders.Lark for Telegram with the chat-only subset the user asked for: telegram_messages_send (Bot API sendMessage) and telegram_chats_lookup (Bot API getChat). Both go through the same NyxIdApiClient.ProxyRequestAsync surface using the api-telegram-bot provider slug, so credential brokering remains in NyxID. Replies inside the current inbound turn keep flowing through NyxIdRelayOutboundPort; these tools cover the proactive-send use case agents need for notifications and side effects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Amend ADR-0013 with a Telegram section capturing the platform-specific choices: same NyxIdRelay transport, new Platform.Telegram renderer, NyxTelegramProvisioningService for bot registration, generalized Credentials map on the provisioning request, and the chat-only ToolProviders.Telegram surface. Add the 2026-04-27 Telegram cutover runbook to mirror the Lark one: provisioning request shapes (bot_token shorthand or generic credentials map), setWebhook configuration against the returned Nyx webhook_url, expected runtime behavior, and the known gaps (forum topics + file attachments out of scope for chat-only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The TelegramPrivate / TelegramGroup / TelegramChannel partial helpers were direct-adapter surface that NyxIdRelayTransport now subsumes; the partial backing them was deleted along with the legacy Channel.Telegram project. Lark has no equivalent helper, so the canonical-key building path is solely the relay transport's responsibility. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68cbd460ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on PR #289 flagged that POST /api/channels/registrations falls through to 502 BadGateway when the caller omits the Telegram bot_token, because ResolveProvisioningFailureStatusCode only listed the Lark-shaped missing_* error codes. Treating client validation input as a server/proxy failure breaks client retry/validation logic and adds noise to operational triage. Add missing_bot_token to the 400 BadRequest bucket alongside missing_app_id / missing_app_secret / missing_verification_token / missing_webhook_base_url / missing_scope_id, and add a regression test asserting the Telegram missing-bot_token path returns 400. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eanzhao
left a comment
There was a problem hiding this comment.
Reviewed current head 9e2b473. I found a couple of actionable issues to fix before relying on the Telegram path in production.
eanzhao
left a comment
There was a problem hiding this comment.
I re-checked the PR against the actual NyxID backend under ~/Code/NyxID and found a few additional contract mismatches.
- [P1] Drop the unused Telegram.Bot v22.9.5.3 PackageVersion. The legacy Aevatar.GAgents.Channel.Telegram adapter that referenced it was deleted in this PR; ADR-0013 explicitly says no per-platform SDK client. Restore the trailing newline on Directory.Packages.props. - [P1] Map "supergroup" to ConversationScope.Group in NyxIdRelayConversationTypeMap. Telegram emits "supergroup" as a distinct Bot API conversation type, so without this row inbound supergroup traffic fell through to the default branch and the transport rejected it. Add regression cases to NyxIdRelayConversationTypeMapTests. - [P2] Stop globally relaxing the conformance debounce ceiling to 3000. The shared ChannelAdapterConformanceTests cap returns to 2000; channels with platform rate limits that need more (e.g. Telegram editMessageText ~1/s per chat) override MaxRecommendedStreamDebounceMs in their concrete conformance subclass. Telegram itself does not currently subclass the conformance suite (NyxIdRelay subsumes its inbound), so this is a forward-looking guardrail. - [P2] Document the manual Nyx cleanup procedure in the Telegram cutover runbook for the local_mirror_accepted_remote_cleanup_skipped error, including the reverse-order DELETE sequence operators run against Nyx. The other three review notes are intentional or moot (Nyx rollback order is correct; the redactor list is more aggressive than Lark but IPayloadRedactor has no production consumer today; callback_data truncation is a Bot API constraint with no clean composer-side answer without restructuring action.Value semantics). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- [P2] Stop leaking arbitrary ex.Message through the registration response.
Both NyxLarkProvisioningService and NyxTelegramProvisioningService now
route caught exceptions through SanitizeFailureReason, which surfaces
controlled InvalidOperationException strings (e.g.
"channel_bot_id_request_failed nyx_status=401 body=invalid app secret")
verbatim while collapsing HTTP transport errors, JSON parser internals,
and any other exception type to "provisioning_failed". Operators still
get the full exception via the existing LogWarning. Lark and Telegram
patched together to keep parity. Add a NyxTelegram regression test that
asserts the controlled-string path surfaces and no .NET internals leak.
- [P3] Build the default Nyx provider slug from the platform name pattern
($"api-{platform}-bot") instead of switch-falling-back to api-lark-bot.
Adding a future platform (e.g. discord) now naturally echoes
api-discord-bot rather than silently mislabelling the registration.
- [P3] Fail loud on malformed TelegramSendMessageRequest.ReplyMarkupJson.
TelegramNyxClient.SendMessageAsync now throws ArgumentException with a
clear paramName/message instead of silently forwarding raw garbage to
Nyx → Telegram and getting back an opaque Bad Request. Add a regression
test exercising the malformed-JSON path with a throwing HttpHandler so
the test fails loudly if the call ever reaches Nyx.
The other two review notes are intentional or duplicate:
- Telegram redactor scrubbing `text`/`caption` was already raised in
the prior review; IPayloadRedactor still has zero production consumers
(verified by grep on dev), so the redaction list has no runtime impact.
- ToolProviders.Telegram.ServiceCollectionExtensions registering an
empty NyxIdToolOptions singleton is identical to the established
ToolProviders.Lark pattern; the BaseUrl emptiness check in
TelegramAgentToolSource.DiscoverToolsAsync is the intended guard.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eanzhao
left a comment
There was a problem hiding this comment.
Reviewed current PR head f70f0f1 again. One additional blocking runtime issue is still present in the pushed PR.
- [P0] ChannelBotRegistrationGAgent.HandleRegister was hard-coded to drop
any platform other than "lark", so Telegram registrations succeeded in
Nyx but the local mirror command was silently discarded by the actor —
the prior NyxTelegramProvisioningServiceTests only verified DispatchAsync
was called, never that the actor persisted. Replace the single-platform
guard with a SupportedPlatforms allowlist {lark, telegram}, update the
test that asserted Telegram is silently ignored to assert it is now
persisted with the Telegram-shaped fields, and add a regression test
that an unsupported platform (discord) is still ignored.
- [P2 dedup] Extract NyxApiResponseHelper for the JSON parsing /
envelope-detection / error-detail / TryRollback / SanitizeFailureReason
helpers that were duplicated between NyxLarkProvisioningService and
NyxTelegramProvisioningService. About 100 duplicated lines collapse to
one place; both services now delegate to the helper. The legacy private
RelayApiKeyCredentials record stays in each service to keep the typed
return-shape per platform.
- [P3] HandleRegisterAsync now reuses the existing ResolveBearerAccessToken
helper instead of reparsing the Bearer prefix inline, matching what the
rebuild / delete endpoints already do.
- [P3] TelegramNyxClient.GetChatAsync switches from JsonSerializer +
anonymous type to the JsonObject style SendMessageAsync uses, so the
whole 76-line client uses one JSON construction pattern.
The other review points were already addressed or duplicate:
- missing_bot_token mapping to 400 was added in commit 9e2b473 (the
reviewer was looking at an earlier commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five eanzhao review threads on PR #289 traced back to the same root cause: the Telegram composer / provisioning note / canonical doc / host wiring all assumed contracts the NyxID-side Telegram channel adapter does not actually implement. Verified each claim against `~/Code/NyxID/backend/src/services/channel_adapters/telegram.rs` and fixed: - TelegramMessageComposer.DefaultCapabilities.SupportsActionButtons set to false. NyxID's `register_webhook` only subscribes to message / edited_message / channel_post — `callback_query` is not in `allowed_updates`, and `parse_inbound` returns empty for callback queries. So inline_keyboard click-back never round-trips to Aevatar. Compose now degrades intent.Actions into a plain-text bullet list of labels; Evaluate flags any actions as Degraded so callers know the click path is unavailable. - BuildRenderedText now applies Telegram legacy-Markdown escaping to `_`, `*`, `[`, `` ` ``. NyxID's `send_reply` sends every relay reply with `parse_mode="Markdown"`, so unescaped model output containing those characters would either turn into formatting or trip a 400 "can't parse entities" rejection. Add a regression test that asserts the four control characters get backslash-escaped. - NyxTelegramProvisioningService.Note + the cutover runbook step 4 no longer tell operators to call `setWebhook` themselves. NyxID's `POST /api/v1/channel-bots` already calls `setWebhook` server-side using a NyxID-managed `secret_token`; manual setWebhook overwrites that secret and breaks `x-telegram-bot-api-secret-token` verification. Note + runbook now say so explicitly. - src/Aevatar.Mainnet.Host.Api now project-references Aevatar.AI.ToolProviders.Telegram and calls AddTelegramTools alongside AddLarkTools so `telegram_messages_send` / `telegram_chats_lookup` actually enter the IAgentToolSource discovery chain in production. Extend MainnetHostCompositionTests to assert both LarkAgentToolSource and TelegramAgentToolSource resolve from the live container so a future regression in the host wiring surfaces immediately. - docs/canon/aevatar-channel-architecture.md §10.2 rewritten from the retired direct-adapter description (webhook + long-poll fallback + credential snapshot) to the actual relay-only path: NyxIdRelay is the sole transport, Platform.Telegram is composer/redactor only, NyxID owns Bot API credentials and webhook registration, action buttons are intentionally degraded today. Updated tests for composer behavior changes (15 Platform.Telegram tests pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…262-telegram-channel-adapter
eanzhao
left a comment
There was a problem hiding this comment.
Reviewed the latest fixes on 437d00b. Most prior threads are addressed; I found one remaining documentation mismatch.
Two doc surfaces still claimed Telegram action buttons render as an inline_keyboard with callback_data, but the implementation flipped to SupportsActionButtons=false in commit 0e17b3e (NyxID's Telegram channel adapter doesn't subscribe callback_query updates and parse_inbound returns empty for them, so any inline_keyboard click would never round- trip back to Aevatar). - ADR-0013 Telegram amendment: rewrote the rendering bullet to describe the actual degraded behavior — actions become a plain-text bullet list, body text gets legacy-Markdown escaped for parse_mode="Markdown". Added the conditional reverse path (flip SupportsActionButtons + restore callback_data builder + update §10.2) for when NyxID adds callback_query. - docs/canon/aevatar-channel-architecture.md §5.5 capability matrix: Telegram SupportsActionButtons row changed from "✅ (inline keyboard)" to "❌ (degrade to text bullets — see §10.2)" so the matrix lines up with TelegramMessageComposer.DefaultCapabilities and §10.2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov flagged patch coverage 70.03% on PR #289 (threshold ≥80%) with the worst offenders being TelegramNyxClient (35%), TelegramProxyResponseParser (66%), TelegramChatsLookupTool (65%), TelegramMessagesSendTool (88%), TelegramAgentToolSource (85%). Add a direct HTTP-level test class TelegramNyxClientTests that uses a recording HttpMessageHandler to assert TelegramNyxClient builds the sendMessage / getChat request bodies correctly across every option field — chat_id, text, parse_mode, disable_notification (only when explicitly true), reply_to_message_id, reply_markup parsed as a JSON object, custom provider slug. Also covers the response-pass-through path that the in-process stub used by the tool tests cannot exercise. Extend TelegramToolsTests with branch coverage that was missing: - SendMessage propagates parse_mode / disable_notification / reply_to_message_id to the client; accepts each of the three supported parse modes (Markdown / MarkdownV2 / HTML) - All four parser branches: empty response, invalid JSON, ok:false with no error_code/description, error:true with no status/message - Tool falls back to request chat_id when ok:true response has no result - ChatsLookup mirror tests: number-typed chat.id coercion, missing-result fallback, both Nyx error and Telegram error paths - ToolSource per-flag matrix: only-send / only-lookup / blank-slug - AddTelegramTools DI extension test (and default-options variant) so ServiceCollectionExtensions is exercised in this test project too, not only via MainnetHostCompositionTests - Tool metadata pinning test (Name / Description / ApprovalMode) Local cobertura: every class in the Telegram tool provider now ≥89.7% line / ≥90% branch, with all but TelegramProxyResponseParser at 100%. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Implements issue #262 by adding Telegram support on the unified ADR-0013 channel inbound backbone (
Lark -> NyxID -> Aevatar→ now alsoTelegram -> NyxID -> Aevatar). The Telegram adapter is not a separate per-platform transport: it reuses the genericAevatar.GAgents.Channel.NyxIdRelayingress, with platform-specific rendering in a newAevatar.GAgents.Platform.Telegrampackage and bot provisioning in a newNyxTelegramProvisioningService.Closes #262.
What Changed
Restructure to ADR-0013 layout
agents/channels/Aevatar.GAgents.Channel.Telegramdirect-adapter project (the prototype path that ADR-0012 had already excluded from the supported production contract).agents/platforms/Aevatar.GAgents.Platform.Telegram/mirrorsAevatar.GAgents.Platform.Lark:TelegramMessageComposer+TelegramChannelNativeMessageProducer+TelegramOutboundMessage+TelegramPayloadRedactor.test/Aevatar.GAgents.Platform.Telegram.Tests/covers composer + producer using the sharedMessageComposerUnitTests<>base.Provisioning + registration endpoint
NyxTelegramProvisioningServiceparallelsNyxLarkProvisioningService: creates a Nyx relay api-key (platform=generic), registers the bot with Nyx (platform="telegram"+ realbot_token, no Lark__unused_for_lark__placeholder), creates a default conversation route, and dispatches the localChannelBotRegisterCommandmirror. ImplementsINyxChannelBotProvisioningServiceso the registration endpoint discovers it by enumeration. Default Nyx provider slug:api-telegram-bot.NyxChannelBotProvisioningRequestwith an optionalCredentialsmap for forward-compatible platform-extensible secrets. Lark's typed sub-message stays unchanged.POST /api/channels/registrationsaccepts either a top-levelbot_tokenshorthand for Telegram or a genericcredentials: { "bot_token": ... }map. Responsenyx_provider_slugdefaults are now platform-aware.Chat-only Telegram tool provider
src/Aevatar.AI.ToolProviders.Telegram/exposes the chat-only subset:telegram_messages_send(Bot APIsendMessage) andtelegram_chats_lookup(Bot APIgetChat). Both go throughNyxIdApiClient.ProxyRequestAsyncagainstapi-telegram-bot. Reply-in-turn keeps flowing throughNyxIdRelayOutboundPort.DI + solution layout
Aevatar.GAgents.ChannelRuntimenow referencesAevatar.GAgents.Platform.Telegramand registers Telegram composer / native producer / payload redactor / provisioning service alongside Lark.aevatar.slnx,aevatar.platforms.slnfupdated; legacyAevatar.GAgents.Channel.Telegram*entries removed.Documentation
docs/operations/2026-04-27-telegram-nyx-cutover-runbook.md(parallel to the Lark one): provisioning request shapes,setWebhookconfiguration against the returned Nyxwebhook_url, expected runtime behavior, known gaps (forum topics + file attachments out of scope for chat-only).Why
Issue #262 needed a Telegram adapter, and the original PR shipped a per-platform
Aevatar.GAgents.Channel.Telegramdirect-adapter project. Dev has since moved Lark off the direct-adapter pattern (ADR-0013 unified inbound backbone): the adapter project was deleted and replaced byAevatar.GAgents.Channel.NyxIdRelay(one transport for all platforms) plus per-platform rendering inagents/platforms/. Telegram needed to land on that same architecture instead of the now-removed direct path.Impact
transport adapter -> ChatActivity -> ConversationGAgent -> ChannelConversationTurnRunner -> committed events / outbound. Inbound dedup, slash routing, agent-builder routing, workflow resume, and conversation completion events are now shared with Lark.Aevatar.GAgents.Channel.Telegramdirect-adapter prototype is gone; nothing should reference it.NyxChannelBotProvisioningRequestis platform-extensible without touching the Lark provisioning path; the next platform can plug in viaCredentialsmap only.Validation
Notes
message_thread_id) are not surfaced inConversationReferenceyet; group threads collapse into the parent group conversation scope. Adding a typedThreadIdonTransportExtrasis the natural follow-up if topic-scoped routing becomes a product requirement.Unsupportedcapability when an intent carries attachments; agents should avoid producing attachment intents for Telegram until the composer grows that branch.