Repository navigation
refactor(bots): extract gateway bridge base classes and rename simple-bridge to telegram-bridge - #3076
Conversation
…/dingtalk Collapse the connect/heartbeat/identify/resume/reconnect plumbing that qq-bridge, discord-bridge, and dingtalk-bridge duplicated verbatim: WsBridgeBase owns the WebSocket transport lifecycle (connect, close policy, reconnect backoff, stop) and GatewayBridgeBase layers the Discord/QQ-shaped opcode protocol (HELLO/heartbeat/seq/session/ identify/resume) on top; DingTalk's Stream protocol extends the transport base directly. Pure helpers and __TEST__ exports are unchanged; gateway-bridge-base.test.ts adds behavior coverage for the previously untested connect/heartbeat/reconnect paths. Generated-by: ZCode (GLM)
…ishu/discord branches The registry only ever constructs this bridge for telegram — feishu and discord route to their dedicated bridges — so the feishu start branch (plus feishuTenantAccessToken and its timeout constant), the discord scaffold branch, and the unreachable 'unimplemented' fallback were dead. Rename the file and class to match the real responsibility and simplify connectionKind to a constant 'polling'. Generated-by: ZCode (GLM)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughSummaryThis PR removes duplicated WebSocket and gateway lifecycle logic from the bot bridges. The PR also renames Review assessment
Review-relevant risksThe inheritance changes and the No protected-area effect was identified for security, licensing, releases, or governance in the current diff. The person performing the merge must review the final diff, and a maintainer makes the final determination. WalkthroughThe PR adds shared WebSocket and gateway lifecycle bases, migrates Discord, QQ, and DingTalk bridges to them, and renames and narrows the Telegram bridge. Gateway tests cover connection, heartbeat, reconnect, session, and close behavior. ChangesShared WebSocket bridge lifecycle
Telegram bridge separation
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The PR centralizes shared gateway transport behavior and renames the Telegram bridge without a reported behavior change; the supplied checks pass, and no actionable merge-blocking risk remains after normal review. Sequence Diagram(s)sequenceDiagram
participant DiscordBotBridge
participant GatewayBridgeBase
participant WebSocket
DiscordBotBridge->>GatewayBridgeBase: openConnection()
GatewayBridgeBase->>WebSocket: connect to gateway URL
WebSocket-->>GatewayBridgeBase: HELLO
GatewayBridgeBase->>DiscordBotBridge: request identify or resume payload
GatewayBridgeBase->>WebSocket: send gateway payload
WebSocket-->>GatewayBridgeBase: dispatch or heartbeat acknowledgement
GatewayBridgeBase->>DiscordBotBridge: onDispatch()
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a250ef1-ab98-4ede-bbb1-be709b63a797
📒 Files selected for processing (13)
packages/runtime/src/bots/__tests__/bot-user-allowlist.test.tspackages/runtime/src/bots/__tests__/gateway-bridge-base.test.tspackages/runtime/src/bots/__tests__/telegram-ratelimit-retry.test.tspackages/runtime/src/bots/__tests__/telegram-reply-stream.test.tspackages/runtime/src/bots/__tests__/telegram-reply-to.test.tspackages/runtime/src/bots/__tests__/telegram-utf16.test.tspackages/runtime/src/bots/bot-registry.tspackages/runtime/src/bots/dingtalk-bridge.tspackages/runtime/src/bots/discord-bridge.tspackages/runtime/src/bots/gateway-bridge-base.tspackages/runtime/src/bots/qq-bridge.tspackages/runtime/src/bots/telegram-bridge.tspackages/runtime/src/bots/ws-bridge-base.ts
Address five review findings on the extracted base classes. All five were pre-existing behaviors carried over verbatim from the per-bridge implementations; fixing them in the base covers QQ and Discord at once. - discord: clear resumeGatewayUrl in resetSession — Discord's resume URL is only valid while its session lives, never for a fresh identify - gateway: validate HELLO heartbeat_interval and fall back to the default instead of scheduling on NaN (a ~1ms heartbeat loop) - gateway: force-reconnect when the identify/resume payload is null so the backoff path owns the retry instead of leaving an un-identified socket idle until the remote gateway closes it - gateway: track the heartbeat jitter timeout alongside the interval so stop()/close cancel it, and never start the interval on a dead socket (a stale jitter could otherwise orphan the live interval and run two heartbeat loops) - ws: ignore close events from sockets the bridge already detached — a late close after forceReconnect double-incremented reconnect attempts and could drop a session the reconnect meant to keep The test FakeWebSocket now emits an async close event from close() so the detached-socket path is exercised; nine tests cover the fixes. Generated-by: ZCode (GLM)
f10c646 to
ea620e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9bac9a63-df57-4133-9d66-a866444f8789
📒 Files selected for processing (5)
packages/runtime/src/bots/__tests__/discord-bridge.test.tspackages/runtime/src/bots/__tests__/gateway-bridge-base.test.tspackages/runtime/src/bots/discord-bridge.tspackages/runtime/src/bots/gateway-bridge-base.tspackages/runtime/src/bots/ws-bridge-base.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/runtime/src/bots/ws-bridge-base.ts
- packages/runtime/src/bots/gateway-bridge-base.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
… timers Two close-policy tests ended with a scheduled reconnect timer still armed; stop the bridge like every other test in the file so the timer cannot fire during later tests and resurrect the connection. Generated-by: ZCode (GLM)
|
Thanks for the refactor — the extraction boundary is right (two layers: transport Conclusion: PASS — no blocking findings; the refactor is good to merge as-is. For the record (P3, non-blocking — all nice-to-haves): P3-1 — the PR description/checklist says "fully behavior-preserving / No behavior change", but the branch intentionally changes 5 runtime behaviors. The body says "Two commits, fully behavior-preserving" and the checklist ticks P3-2 — QQ and DingTalk subclass↔base wiring has zero integration tests. P3-3 — AI-assisted review disclosure: this review was produced with AI assistance (two pi review subagents on 中文摘要（AI 辅助审查）结论：PASS，无阻塞项，可直接合并。refactor 质量高：两层抽象边界正确（传输层 WsBridgeBase 含 DingTalk、协议层 GatewayBridgeBase 给 QQ/Discord）、搬移代码与原文件逐字节一致、重命名彻底零残留、删除的死分支经 BotProvider 闭集核实确实不可达，本地实跑 68/68 测试通过。均为 P3（nice to have，不阻塞）：(1) PR 正文声称 "fully behavior-preserving / No behavior change" 与事实不符——实际 4 个 commit，hardening commit (ea620e2) 有意改 5 处运行时行为（畸形 HELLO 心跳风暴、null 认证 payload 空转、jitter 定时器泄漏、迟到 close 双调度、Discord 陈旧 resume URL 重新认证——均带测试的正确修复），代码无问题，仅建议顺手更新描述与 checklist 保持记录准确；(2) QQ/DingTalk 子类↔base 集成零测试（存量缺口+逐字搬移+类型约束，可延后，建议 follow-up 补测试）；(3) telegram-bridge 三处 platform guard 死条件残留、createWebSocket 测试 seam、sleep/token-refresh/send-retry 四桥重复（范围裁剪可接受）。 |
Summary
Closes the "Bot gateway bridge base" lane. Extracts the gateway/WS plumbing that
qq-bridge.ts,discord-bridge.ts, anddingtalk-bridge.tshad duplicated verbatim, and completes the follow-up cleanup ofsimple-bridge.ts. Two commits, fully behavior-preserving.1.
refactor(bots): extract ws/gateway bridge base classes for qq/discord/dingtalkWsBridgeBase(packages/runtime/src/bots/ws-bridge-base.ts) — the WebSocket transport lifecycle all three bridges shared byte-for-byte:start()credential gate,stop(),connect()with listener wiring,handleClose(),scheduleReconnect(),sendWs(), and the sharedreconnectBackoffMs(the three per-file copies were identical, including theirRECONNECT_DELAY_*constants; the constants are now exported once).GatewayBridgeBase(extendsWsBridgeBase) — the Discord/QQ-shaped protocol layer: the eight opcode constants and thehandleWsMessagedispatch switch, heartbeat scheduling with missed-ack reconnect,seq/sessionIdtracking, the identify/resume fork at HELLO,forceReconnect(), andpromoteToOperational()for READY/RESUMED.QQBot <access_token>with refresh vs staticBot <token>),buildIdentifyPayload()/buildResumePayload(), dispatch event names (AT_MESSAGE_CREATE/GROUP_AT_MESSAGE_CREATE/C2C_MESSAGE_CREATE/... vsMESSAGE_CREATE), and the REST send routes.qq-bridge.ts741→504 lines,discord-bridge.ts629→385.WsBridgeBase: its Stream protocol has no opcodes/heartbeat/identify/resume (frames carry headers/messageId/topic and every delivery must be acked), so a single all-threeGatewayBridgeBasewould have given it dead session machinery. It shares the transport lifecycle (connect/close policy/backoff/stop, with acloseReasonPrefixhook for itsstream-closed-*reasons) and keeps its frame protocol in the subclass.dingtalk-bridge.ts561→464.gateway-bridge-base.test.ts(17 tests, 315 lines) — the connect/heartbeat/reconnect paths had zero behavioral coverage before (all existing bridge tests only exercise pure helpers). Uses aFakeWebSocketinjected through acreateWebSocket()seam to drive HELLO→identify, READY→resume, missed-ack force-reconnect, INVALID_SESSION resumable/non-resumable semantics, fatal/resumable/clean close policy, and the shared backoff curve.reasonstrings (gateway-closed-*,stream-closed-*,gateway-bot-*,connections-open-*,getAppAccessToken-*) are preserved verbatim, and every file's__TEST__export surface is untouched.2.
refactor(bots): rename simple-bridge to telegram-bridge, drop dead feishu/discord branchesBotRegistryonly ever constructedSimpleBotBridgefortelegram(feishu and discord route to their dedicated bridges), so the feishu start branch — plusstartFeishu(),feishuTenantAccessToken(), andFEISHU_REQUEST_TIMEOUT_MS— the discordscaffold-onlybranch, and the unreachableunimplementedfallback were dead code. All deleted.simple-bridge.ts→telegram-bridge.tsandSimpleBotBridge→TelegramBotBridgeto match the file's real responsibility;connectionKind()simplifies to a constant'polling'. Callers updated:bot-registry.tsand six test files (import paths only;telegram-reply-stream.test.tsalso picks up the new class name).Refs: #1404
Verification
npm run format:check— clean (8 formatting nits auto-fixed and folded in)npm run lint— 2304 files, no issuesnpm run typechecknpm --workspace @maka/bots run build— cleannpm --workspace @maka/bots run test:dist— 92 tests, 92 passAI use
Select exactly one:
Tool(s) and scope: ZCode (GLM) prepared the entire change under human
direction — the base-class extraction, the qq/discord/dingtalk bridge
migrations, the telegram-bridge rename, the review-driven hardening
fix, and the new tests. The contributor of record scoped the task,
approved the plan, triaged the review findings, and made every commit
decision. All commits carry
Generated-by: ZCode (GLM)trailers.Checklist
Does this PR entail a change in behavior?