Skip to content

[Channel RFC] Add Telegram channel adapter#289

Merged
eanzhao merged 20 commits intodevfrom
feat/2026-04-21_issue-262-telegram-channel-adapter
Apr 27, 2026
Merged

[Channel RFC] Add Telegram channel adapter#289
eanzhao merged 20 commits intodevfrom
feat/2026-04-21_issue-262-telegram-channel-adapter

Conversation

@eanzhao
Copy link
Copy Markdown
Contributor

@eanzhao eanzhao commented Apr 21, 2026

Summary

Implements issue #262 by adding Telegram support on the unified ADR-0013 channel inbound backbone (Lark -> NyxID -> Aevatar → now also Telegram -> NyxID -> Aevatar). The Telegram adapter is not a separate per-platform transport: it reuses the generic Aevatar.GAgents.Channel.NyxIdRelay ingress, with platform-specific rendering in a new Aevatar.GAgents.Platform.Telegram package and bot provisioning in a new NyxTelegramProvisioningService.

Closes #262.

What Changed

Restructure to ADR-0013 layout

  • Delete the legacy agents/channels/Aevatar.GAgents.Channel.Telegram direct-adapter project (the prototype path that ADR-0012 had already excluded from the supported production contract).
  • New agents/platforms/Aevatar.GAgents.Platform.Telegram/ mirrors Aevatar.GAgents.Platform.Lark: TelegramMessageComposer + TelegramChannelNativeMessageProducer + TelegramOutboundMessage + TelegramPayloadRedactor.
  • New test/Aevatar.GAgents.Platform.Telegram.Tests/ covers composer + producer using the shared MessageComposerUnitTests<> base.

Provisioning + registration endpoint

  • New NyxTelegramProvisioningService parallels NyxLarkProvisioningService: creates a Nyx relay api-key (platform=generic), registers the bot with Nyx (platform="telegram" + real bot_token, no Lark __unused_for_lark__ placeholder), creates a default conversation route, and dispatches the local ChannelBotRegisterCommand mirror. Implements INyxChannelBotProvisioningService so the registration endpoint discovers it by enumeration. Default Nyx provider slug: api-telegram-bot.
  • Generalize NyxChannelBotProvisioningRequest with an optional Credentials map for forward-compatible platform-extensible secrets. Lark's typed sub-message stays unchanged.
  • POST /api/channels/registrations accepts either a top-level bot_token shorthand for Telegram or a generic credentials: { "bot_token": ... } map. Response nyx_provider_slug defaults are now platform-aware.

Chat-only Telegram tool provider

  • New src/Aevatar.AI.ToolProviders.Telegram/ exposes the chat-only subset: telegram_messages_send (Bot API sendMessage) and telegram_chats_lookup (Bot API getChat). Both go through NyxIdApiClient.ProxyRequestAsync against api-telegram-bot. Reply-in-turn keeps flowing through NyxIdRelayOutboundPort.

DI + solution layout

  • Aevatar.GAgents.ChannelRuntime now references Aevatar.GAgents.Platform.Telegram and registers Telegram composer / native producer / payload redactor / provisioning service alongside Lark.
  • aevatar.slnx, aevatar.platforms.slnf updated; legacy Aevatar.GAgents.Channel.Telegram* entries removed.

Documentation

  • ADR-0013 amended with a Telegram section that captures the platform-specific choices: same NyxIdRelay transport, new Platform.Telegram renderer, NyxTelegramProvisioningService, generalized Credentials map, chat-only tools.
  • New docs/operations/2026-04-27-telegram-nyx-cutover-runbook.md (parallel to the Lark one): provisioning request shapes, setWebhook configuration against the returned Nyx webhook_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.Telegram direct-adapter project. Dev has since moved Lark off the direct-adapter pattern (ADR-0013 unified inbound backbone): the adapter project was deleted and replaced by Aevatar.GAgents.Channel.NyxIdRelay (one transport for all platforms) plus per-platform rendering in agents/platforms/. Telegram needed to land on that same architecture instead of the now-removed direct path.

Impact

  • Telegram becomes the second platform on 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 still holds no Telegram bot tokens (ADR-0012 boundary preserved). The token only crosses the registration endpoint on the way to Nyx.
  • Aevatar.GAgents.Channel.Telegram direct-adapter prototype is gone; nothing should reference it.
  • NyxChannelBotProvisioningRequest is platform-extensible without touching the Lark provisioning path; the next platform can plug in via Credentials map only.

Validation

dotnet build aevatar.slnx --nologo                                   # 0 errors
bash tools/ci/architecture_guards.sh                                 # exit 0
bash tools/ci/test_stability_guards.sh                               # pass
bash tools/ci/solution_split_guards.sh                               # pass
bash tools/ci/solution_split_test_guards.sh                          # pass
bash tools/docs/lint.sh                                              # 29 files, 0 errors

dotnet test test/Aevatar.GAgents.Channel.Protocol.Tests/...          # 123 passed
dotnet test test/Aevatar.GAgents.Platform.Telegram.Tests/...         # 13 passed
dotnet test test/Aevatar.GAgents.Platform.Lark.Tests/...             # 14 passed (no regression)
dotnet test test/Aevatar.AI.ToolProviders.Telegram.Tests/...         #  8 passed
dotnet test test/Aevatar.AI.ToolProviders.Lark.Tests/...             # 44 passed (no regression)
dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/...            # 460 passed

Notes

  • Telegram forum topics (message_thread_id) are not surfaced in ConversationReference yet; group threads collapse into the parent group conversation scope. Adding a typed ThreadId on TransportExtras is the natural follow-up if topic-scoped routing becomes a product requirement.
  • File / photo / voice attachments are out of scope for chat-only. The Telegram composer reports Unsupported capability when an intent carries attachments; agents should avoid producing attachment intents for Telegram until the composer grows that branch.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 95.11401% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.50%. Comparing base (7bef42f) to head (84b74af).
⚠️ Report is 21 commits behind head on dev.

Files with missing lines Patch % Lines
...lProviders.Telegram/TelegramProxyResponseParser.cs 85.39% 9 Missing and 4 partials ⚠️
...roviders.Telegram/Tools/TelegramChatsLookupTool.cs 97.36% 0 Missing and 1 partial ⚠️
...oviders.Telegram/Tools/TelegramMessagesSendTool.cs 98.52% 0 Missing and 1 partial ⚠️
@@            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     
Flag Coverage Δ
ci 70.50% <95.11%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ar.AI.ToolProviders.Telegram/ITelegramNyxClient.cs 100.00% <100.00%> (ø)
...lProviders.Telegram/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
....ToolProviders.Telegram/TelegramAgentToolSource.cs 100.00% <100.00%> (ø)
...tar.AI.ToolProviders.Telegram/TelegramNyxClient.cs 100.00% <100.00%> (ø)
...r.AI.ToolProviders.Telegram/TelegramToolOptions.cs 100.00% <100.00%> (ø)
...t.Host.Api/Hosting/MainnetHostBuilderExtensions.cs 93.65% <100.00%> (+0.43%) ⬆️
...ting/Conformance/ChannelAdapterConformanceTests.cs 71.72% <100.00%> (+0.07%) ⬆️
...roviders.Telegram/Tools/TelegramChatsLookupTool.cs 97.36% <97.36%> (ø)
...oviders.Telegram/Tools/TelegramMessagesSendTool.cs 98.52% <98.52%> (ø)
...lProviders.Telegram/TelegramProxyResponseParser.cs 85.39% <85.39%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 21, 2026

Review 纪要

整体方向 OK:IChannelTransport + IChannelOutboundPort 的落地、canonical conversation(private/group/supergroup/channel)拆分、webhook + long-poll 双入口、composer 的 degradation 语义、测试覆盖(conformance/mode/composer)都做到了 RFC 要求。以下按优先级列几个我看到的点。

高优先级

1. TelegramStreamingHandle 存在并发读写/交叠写入,违反单线程事实源原则
AppendAsync / ScheduleFlush / FlushLaterAsync / CompleteAsync / DisposeAsync 之间没有任何同步:

  • AppendAsync_chunks[seq] = delta,同一时间 FlushLaterAsync_chunks.OrderBy(...).Select(...) + _chunks.Clear(),这是跨线程无锁操作 Dictionary<long,string>,会偶发抛异常或丢数据。
  • CompleteAsyncCancelPendingFlush() 就直接 await UpdateAsync(...),但上一个 FlushLaterAsync 可能已经越过 Task.Delay,正在走 UpdateAsync 的 HTTP 调用——最终会出现两次 editMessageText 乱序到达 Telegram,末态可能不是 final
  • DisposeAsync 写 "interrupted" 的路径同样会和 in-flight flush 竞争。

按 CLAUDE.md「单线程事实源 / 无锁优先,需要加锁先判定为破坏 Actor 边界」,建议把这层状态改成:一个内部 Channel<StreamChunk> + 单后台读取任务,或者至少用 SemaphoreSlim(1,1) 串行化 "合并 chunks → 发送 edit → 结束"。_flushCts + 裸 _ = FlushLaterAsync(...) 目前不足以保证顺序。

2. parseMode 不一致,流式 edit 在含 </>/& 时会坏
TelegramBotApiClient

  • SendMessage 不设 parseMode(等价 plain text)
  • SendPhoto / SendDocument / EditMessageText 全部 ParseMode.Html

BeginStreamingReplyAsync 先走 SendMessageAsync 发种子(plain),然后每次 flush 都是 EditMessageTextAsync(HTML)。只要 LLM 输出里出现 "A<B""if x && y"、未闭合的 <think> 等,Telegram 会返回 Bad Request: can't parse entities,整个流断掉。

TelegramMessageComposer.BuildRenderedText 目前直接把业务文本 + card 字段拼成字符串,不做 HTML 转义。建议二选一:

  • 全链路一致使用 ParseMode.None(不设),composer 什么都不用转;或
  • 全链路一致 ParseMode.Html,composer 在 AppendParagraph 层做 HtmlEncode

3. Webhook secret 比较非恒定时间
ValidateWebhookSecretstring.Equals(..., StringComparison.Ordinal)。secret token 属于鉴权凭据,应使用 CryptographicOperations.FixedTimeEquals 对 UTF-8 字节比较,避免旁路时间攻击。

中优先级

4. LongPollingTimeoutSeconds 默认值 2 秒
Telegram 推荐 long-poll timeout 25–50 秒。默认 2 秒等价于高频短轮询,会显著抬高请求量和延迟 jitter。建议默认 30,单测里再注入短值。

5. _botToken 永久缓存,无失效路径
ResolveBotTokenAsync 一次解析后写入实例字段,永不过期。若 vault 侧轮换凭据或 ICredentialProvider 实现支持短 TTL,适配器会一直发旧 token。要么每次调用都问 provider(让 provider 负责缓存),要么给一个刷新钩子。

6. DeleteAsync 没包 EmitResult,异常直接冒泡
SendAsync/UpdateAsync 的契约不一致。要么统一返回 EmitResult,要么在 contract 上承认 delete 不同语义并在 XML doc 里写明。

7. Long-poll loop 对所有异常同等对待
catch (Exception ex) 只打 warning + 等 1 秒继续。如果 token 失效 / 401 / 403,会静默无限重试。至少对 auth 类错误 break 或指数退避 + 向上报告(比如通过 inbound channel 注入一个 system activity,或者把 loop Task 暴露出来)。

8. ContinueConversationAsync 没做 auth null 校验
其他路径都走了 ArgumentNullException.ThrowIfNull,这里直接读 auth.Kind。顺手补一下。

低优先级 / Nit

9. TelegramConversationTarget.ParseActivityId 兼容裸 int 作为 MessageId(ChatId=0)
没有调用方用这种形式(BuildActivityId 一定是 telegram:message:chat:msg),像是历史遗留兜底,建议删掉,避免误用。

10. RawPayloadBlobRef = $"telegram://update/{updateId}"
目前既不是真正的 blob 引用,也没有对应的持久化(测试里 LastPersistedBlobRef => null)。要么真的落盘,要么改名字/留空;现在这个字段名有误导性。

11. TelegramMessageComposer 对空 intent 返回 "(empty)"
兜底文案对终端用户可见,容易莫名其妙。建议要么返回真正的空串并让上层拒绝,要么做成可配置/本地化资源。

12. Directory.Packages.props 末尾 newline 被删
\ No newline at end of file,多数 lint 规则会报。

13. 目录位置 agents/channels/Aevatar.GAgents.Channel.Telegram/
和兄弟项目 agents/Aevatar.GAgents.Channel.* 的扁平布局不一致。如果后续所有 channel 实现都迁到 agents/channels/,最好同 PR 或随后一 PR 把现有的也搬过去;否则先对齐现有布局。

架构合规核查(快扫)

  • 分层:adapter 落在 agents/channels/,依赖 Abstractions,未跨层反向依赖 ✓
  • 外部协议边界:Telegram.Bot.* 类型只出现在本项目,未泄漏 ✓
  • 中间层进程内状态:_inbound Channel 和 _messages 均属 adapter 本地运行态,非跨节点事实源 ✓
  • 命令/查询边界:adapter 不承担 projection 读取、不做 actor 查询,符合 channel 适配器职责 ✓
  • GetAwaiter().GetResult()、无 TypeUrl.Contains、无 Task.Delay 形式的轮询等待 ✓(long-poll 的 1s backoff 是正常重试语义)

建议

最低限度合并前修 1(流式并发)、2(parseMode)、3(恒定时间比较)、4(默认 poll timeout)。其余可以 follow-up issue。

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 21, 2026

Review-fix 复核(commit a787a261

对照上一轮 13 条,逐条过一遍。

高优先级 — 全部修掉

1. StreamingHandle 并发 ✅
新版加了 SemaphoreSlim _writeGate(1,1) + 代际(_flushGeneration)+ 序号去重(HashSet<long>):

  • AppendAsync / CompleteAsync / DisposeAsync / FlushLaterAsync 内部状态修改全部在锁内,_chunks 跨线程读写消除。
  • flush task 用 generation 校验自己是否已过时,避免"老 flush 在等锁时新 flush 已经 Complete"这种覆盖乱序。
  • BlockNextEditCompletion 的测试确定性地构造了"flush edit 调用还没返回时 CompleteAsync 抢跑"的场景,断言末条 edit 是 final — 这个测试直接验证了修复核心。
  • 一个 nit:AppendAsync 里先把 previousFlush = _flushCts 抓出来、再在锁外 CancelPendingFlush(previousFlush)。旧 flush 若正卡在 await _writeGate.WaitAsync(ct),ct 被 Cancel 后抛 OCE,进 FlushLaterAsync 的 catch 被吞;同时 CTS 已经 Dispose,若还未进入锁等待阶段则 Task.Delay 会抛 ObjectDisposedException,走 catch {}。能跑通但依赖两个 catch-all 兜底,略脆。如果想更干净,可以把 Cancel 留在锁外、Dispose 延迟到 FlushLaterAsync 自己结束时做(例如 try/finally 里 Dispose 自己持有的 CTS)。非必须。

2. parseMode 不一致 ✅
SendPhoto/SendDocument/EditMessageText 三处 ParseMode.Html 都删掉了,现在全链路 plain text,和 SendMessage 对齐。流式 edit 遇 </& 不再炸。

3. Webhook secret 恒定时间比较 ✅
CryptographicOperations.FixedTimeEquals + 长度前置校验。注意长度不等早退会泄露长度信息,但 FixedTimeEquals 本就要求等长,这是标准写法。

4. long-poll 默认 2s → 30s ✅
加了 Options_DefaultLongPollingTimeout_UsesTelegramFriendlyDefault 断言兜底。

中优先级

5. _botToken 永久缓存 ✅ 缓存字段删除,每次现问 provider。provider 自己的缓存策略由 provider 负责,边界清晰。

6. DeleteAsync 不包 EmitResult — 未动
应该是 IChannelOutboundPort 契约本身的问题,不是 Telegram 特定。follow-up 可以。

7. Long-poll 对 auth 错误无限重试 ✅

  • ApiRequestException + IsTerminalAuthFailure(401/403) → break
  • credential 解析失败的 InvalidOperationException → break
  • 加了 StartReceiving_LongPollingAuthFailure_StopsRetryLoop 验证 401 后 PollCallCount == 1,退出。
  • Nit:429 Too Many Requests 仍走通用 1s backoff 且不看 retry_after,对高峰场景可能持续撞墙。不阻挡本 PR。

8. ContinueConversationAsync null 校验 ✅ 三个 ArgumentNullException 补齐。

低优先级

9. ParseActivityId 裸 int 兜底 ✅ 已删除。
10. RawPayloadBlobRef 误导名字 ✅ 改成 string.Empty + 测试断言 ShouldBeEmpty
11. Composer (empty) 兜底 ✅ 删除 + adapter 在 Send/Update 路径主动返回 telegram_empty_message,语义更诚实。
12. Directory.Packages.props 末尾 newline — 未动 可以随手补。
13. 目录位置 agents/channels/ vs agents/ — 未动 follow-up。

额外加分

  • Task.Delay 两处都用 _options.TimeProvider 注入,方便未来做虚拟时钟测试。
  • 新测试全部走 TaskCompletionSource + WaitAsync 确定性等待,没有 Task.Delay sleep,符合 tools/ci/test_stability_guards.sh 门禁。

结论

LGTM — 4 条高优全部修掉,其中并发一块修得尤其到位(semaphore + 代际 + 去重 + 针对性并发测试)。未动的 3 条都是非阻塞:#6 跨 channel 的接口契约问题、#12 一个 lint nit、#13 目录布局可后续统一。可以 merge。

eanzhao and others added 6 commits April 27, 2026 11:45
…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>
@eanzhao eanzhao marked this pull request as ready for review April 27, 2026 04:36
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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>
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

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

Reviewed current head 9e2b473. I found a couple of actionable issues to fix before relying on the Telegram path in production.

Comment thread docs/canon/aevatar-channel-architecture.md Outdated
Comment thread Directory.Packages.props Outdated
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

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

I re-checked the PR against the actual NyxID backend under ~/Code/NyxID and found a few additional contract mismatches.

Comment thread agents/platforms/Aevatar.GAgents.Platform.Telegram/TelegramMessageComposer.cs Outdated
Comment thread agents/Aevatar.GAgents.ChannelRuntime/NyxTelegramProvisioningService.cs Outdated
eanzhao and others added 2 commits April 27, 2026 13:09
- [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>
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

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

Reviewed current PR head f70f0f1 again. One additional blocking runtime issue is still present in the pushed PR.

eanzhao and others added 3 commits April 27, 2026 13:44
- [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>
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

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

Reviewed the latest fixes on 437d00b. Most prior threads are addressed; I found one remaining documentation mismatch.

Comment thread docs/decisions/0013-unified-channel-inbound-backbone.md Outdated
eanzhao and others added 2 commits April 27, 2026 14:13
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>
@eanzhao eanzhao merged commit 1943530 into dev Apr 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Channel RFC] Telegram adapter migration (shim → full TelegramChannelAdapter)

1 participant