fix(channels): route Telegram replies to forum topics - #7612
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with a confirmed root cause. Issue #7609 documents clear reproduction steps (create a Telegram supergroup with Topics, send from a non-general topic, reply lands in #general). Two collaborators independently confirmed the root cause: Direction: aligned — Telegram forum topic routing is a straightforward bug fix for an existing integration. The issue is labeled Size: not applicable (no core module paths touched). 32 production lines + 106 test lines across 2 files. Approach: the scope feels right. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测到的 bug,根因已确认。Issue #7609 记录了清晰的复现步骤(创建启用 Topics 的 Telegram 超级群组,从非 general 话题发送消息,回复出现在 #general)。两位协作者独立确认了根因: 方向:对齐——Telegram 论坛话题路由是现有集成的直接 bug 修复。Issue 已标记 规模:不适用(未触及核心模块路径)。2 个文件,32 行生产代码 + 106 行测试代码。 方案:范围合理。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I would have maintained a The PR's approach combines both paths and is better than either alone: No critical blockers. The implementation is clean:
Typecheck and ESLint pass cleanly on both changed files. Real-Scenario TestingThis is a Telegram channel adapter — it runs as a bot server, not a CLI prompt, so live end-to-end testing requires a Telegram supergroup with Topics enabled (not available in CI). The unit test suite is the meaningful verification here: it exercises the exact code paths that were broken. Before (main branch — 14 tests, no topic routing coverage)After (this PR — 17 tests, topic routing verified)The 3 new tests cover: command replies routed to topic 42, agent responses using the stored session target, and a newer inbound topic (43) taking precedence over a stale session route (42) while #general remains unthreaded. 中文说明代码审查独立方案: 我会维护一个 PR 的方案 结合了两种路径,比单独任何一种都更好: 无关键阻塞问题。实现干净:
Typecheck 和 ESLint 在两个修改文件上均通过。 真实场景测试这是 Telegram 频道适配器——作为机器人服务器运行,不是 CLI 提示,因此实时端到端测试需要启用 Topics 的 Telegram 超级群组(CI 中不可用)。单元测试套件是有意义的验证:它执行了被破坏的确切代码路径。 3 个新测试覆盖:话题 42 中的命令回复、使用已存储会话目标的代理响应、较新的入站话题(43)优先于过期的会话路由(42),同时 #general 保持非话题路由。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean fix for a confirmed bug, well-tested, would merge without hesitation. The PR takes the two suggested fix directions from the issue thread and combines them into something better than either alone. The before/after is clear: main has 14 tests with no topic coverage, the PR adds 3 tests that verify the exact scenario from the bug report (reply to topic 42 lands in topic 42, not #general). All 17 pass, typecheck and lint are clean. If I had to maintain this in six months, I'd thank the author — the 中文说明置信度:5/5 — 已确认 bug 的干净修复,测试充分,毫不犹豫地合并。 PR 将 issue 讨论中两个建议的修复方向结合成比单独任何一个都更好的方案。 修改前后对比清晰:main 有 14 个测试无话题覆盖,PR 新增 3 个测试验证了 bug 报告中的确切场景(回复话题 42 应出现在话题 42,而非 #general)。全部 17 个通过,typecheck 和 lint 干净。 如果六个月后我要维护这段代码,我会感谢作者—— — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| expect(router.getTarget).toHaveBeenCalledWith('session-1'); | ||
| expect(bot.api.sendMessage).toHaveBeenCalledWith('2', expect.any(String), { | ||
| parse_mode: 'HTML', | ||
| message_thread_id: 42, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The sendResponseMessage override's router-target mismatch branch is untested — no test exercises the path where inboundRoute is undefined and the router target's chatId differs from the argument chatId.
The target.chatId === chatId guard prevents routing a response to a stale topic in a different chat. If a future edit accidentally removed this guard, messages could be sent with a message_thread_id belonging to a different chat — and no test would catch the regression.
Concrete cost: A guard that silently regresses would route messages to wrong forum topics in production with no test signal.
Consider adding a test case where router.getTarget returns a target with a mismatched chatId:
it('omits thread routing when the router target chat does not match', async () => {
const router = {
getTarget: vi.fn().mockReturnValue({
channelName: 'telegram',
senderId: 'user-1',
chatId: '3', // different from the chatId we send to
threadId: '42',
}),
};
const channel = createChannel({}, router);
const bot = installFakeBot(channel);
await channel.sendTestResponse('2', 'mismatched chat', 'session-1');
expect(bot.api.sendMessage).toHaveBeenCalledWith('2', expect.any(String), {
parse_mode: 'HTML',
// no message_thread_id — guard prevented cross-chat routing
});
});— qwen3.7-max via Qwen Code /review
What this PR does
Telegram replies now return to the forum topic that initiated the request. This covers immediate command replies and regular agent output while keeping concurrent topics isolated from each other.
Why it's needed
In Telegram supergroups with Topics enabled, inbound messages already carry their topic ID, but regular outbound replies dropped that context and appeared in #general. Keeping the route scoped to the active inbound request also avoids one topic overwriting another while multiple requests run in the same chat.
Reviewer Test Plan
How to verify
Configure a Telegram supergroup with Topics enabled and send
/startor a normal prompt from a non-general topic. The response should appear in the same topic. Requests started in different topics should keep their own routes, and a message from #general should not inherit a previous topic.Evidence (Before & After)
Before: the focused reproduction extracted topic
42from the inbound message, but the outbound Telegram API call contained onlyparse_modeand omittedmessage_thread_id.After: all 17 Telegram adapter tests pass. The regression coverage verifies command replies in topic
42, agent replies using the stored session target, a newer inbound topic taking precedence over a stale session route, and #general remaining unthreaded.Tested on
Environment (optional)
Node.js 22.22.1 with the repository lockfile dependencies. Verified with the Telegram adapter unit suite, package ESLint and TypeScript build, plus the repository build and typecheck.
Risk & Scope
Linked Issues
Fixes #7609
中文说明
本 PR 的修改
Telegram 回复现在会返回到发起请求的论坛话题中。该修复覆盖即时命令回复和常规代理输出,同时确保并发话题之间的路由相互隔离。
修改原因
在启用 Topics 的 Telegram 超级群组中,入站消息已经包含话题 ID,但常规出站回复会丢失该上下文并出现在 #general 中。将路由限制在当前入站请求的异步上下文中,也可以避免同一聊天内多个请求并发运行时,一个话题覆盖另一个话题。
审阅者测试计划
验证方法
配置一个启用 Topics 的 Telegram 超级群组,并在非 general 话题中发送
/start或普通提示。回复应出现在同一话题中。从不同话题启动的请求应各自保留路由,而来自 #general 的消息不应继承之前的话题。修改前后证据
修改前:针对性复现从入站消息中提取了话题
42,但出站 Telegram API 调用只包含parse_mode,缺少message_thread_id。修改后:全部 17 个 Telegram 适配器测试通过。回归测试覆盖话题
42中的命令回复、使用已存储会话目标的代理回复、较新的入站话题优先于过期的会话路由,以及 #general 保持非话题路由。测试环境
Node.js 22.22.1,使用仓库锁文件依赖。已通过 Telegram 适配器单元测试、包级 ESLint 和 TypeScript 构建,以及仓库构建和类型检查进行验证。
风险与范围
关联 Issue
Fixes #7609