Repository navigation
Conversation
88ec1b6 to
0c11b67
Compare
me2seeks
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent operated by me2seeks make. It is not an independent human review and does not replace one.
Summary
Adds a bot platform support matrix to docs/bot-platforms.md (which capability each bot platform supports), indexed in docs/README.md. This is a small docs follow-up to #5115, which created the underlying bot-platforms guide in the same file (still open). label passes.
Findings
- [P3] This PR edits
docs/bot-platforms.md, which #5115 also creates/edits — whichever lands second needs a rebase, and the matrix content should be reconciled against #5115's version of the guide (both are open). Content itself is a table, no mechanical claims at risk.
Verdict
merge-ready — small docs addition; sequence with #5115 to avoid a same-file conflict.
|
Thanks for the review. Two corrections to the summary, since maintainers may read it first:
The overlap with #5115 is real, though — same file, same issue. @Pr1p, your Rather than both of us rebasing around the same file, I'd suggest landing one |
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed 1420125756d35db4d89f65d30f55b710388ae222 against current main ae71ab319c920b856c2f18ee6c8d022e38376f71. This PR adds an eight-channel bot capability/setup guide and a docs index entry. The provider count, delivery-provider list, and current app-proxy gap agree with the inspected code.
One P2 makes the DingTalk setup guidance stale: #5112 merged on September 12, and current dingtalk-bridge.ts captures senderStaffId, stamps oto: / group: targets, and routes direct replies accordingly. This guide repeatedly states that 1:1 replies cannot work, that the field is not captured, and that #5112 is still open. Those statements would publish false operational guidance. Update the limitation, setup walkthrough, and front-page caveat to match the merged implementation, while preserving any real remaining edge case (for example messages without senderStaffId).
The exact-head hosted test is green, and a static merge-tree plus diff-check against current main are clean. I did not repeat the author's live-account platform probes or test the third-party WeChat bridge. This is not an approval or merge recommendation.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| strength of that probe. | ||
| 7. **One account per provider.** Onboarding runs a single session per provider; | ||
| there is no multi-account or parallel-onboarding support. | ||
| 8. **DingTalk cannot reply to a 1:1 conversation.** The send route is picked by |
There was a problem hiding this comment.
[P2] Refresh this DingTalk limitation against current main. PR #5112 was merged on 2026-09-12: dingTalkPayloadToEvent now reads senderStaffId and stamps oto:<staffId> for direct chats, and pickDingTalkSendRoute dispatches that prefix to the 1:1 endpoint. The claim that all direct replies are sent to the group endpoint and that #5112 remains open is no longer true. The same obsolete claim recurs in the setup caveat and detailed DingTalk section (:308, :548-597). Left unchanged, the new guide tells users a supported route is broken; document the remaining missing-senderStaffId fallback separately.
|
@hqhq1025 Thanks — you're right, the DingTalk section had gone stale after #5112 merged. Addressed in 1bb193c:
Separately, the question above about #5115 is still open: which guide should be the base, so the other's unique parts can be folded in? |
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed 1bb193cc6ff3d855680f252bb7b3549f7c92ddcc. This documentation-only delta corrects the DingTalk direct-reply guidance after #5112: inbound group: / oto: IDs, the missing-senderStaffId failure path, and legacy unstamped IDs now match the implementation on current main. It also distinguishes the live-tested request from the merged bridge, which has not itself been retested. The previous stale-guidance P2 is resolved; I found no new substantiated P0–P3 in this delta.
The current-head test check passed. git diff --check and a merge-tree against main 0f98c2a48 are clean. I did not test a real DingTalk account, group replies, or the third-party WeChat bridge. This is not a merge approval.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
|
Thanks, agreed that consolidating the overlapping English guide is preferable. Since #4787 predates #5115 and includes the broader live-provider walkthroughs, I'm happy for maintainers to choose it as the English base. I'd like to preserve #5115's unique work as a focused follow-up: the zh-CN guide, current proxy/security corrections, and the iLink |
|
This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed. Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the |
Document the eight IM bot channels registered in BOT_PROVIDERS: their transports, credential fields, capability parity, message limits, known limitations and security posture. Every claim is derived from the bridge implementations rather than from prose, and is cited by symbol name rather than line number so the references survive refactors. Setup instructions per platform are deliberately left as marked placeholders. The issue requires them to be tested against real developer accounts, and untested setup steps are worse than none. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the Feishu channel end-to-end against a real custom app and documented what the runtime actually requires: bot capability, the im:message.p2p_msg:readonly scope, im.message.receive_v1 delivered over a long connection rather than a webhook, and a released version whose availability range covers the intended users. Both directions were exercised: an outbound bot DM, and an inbound user reply received over the WebSocket long connection. Also corrects the Feishu row of the credential table. The bridge reads `appSecret` or, when empty, falls back to `token` — the two are interchangeable and only one is needed. The table previously implied Feishu takes three distinct credentials. Records the failure mode the walkthrough surfaced: the handshake succeeds on app credentials alone, so an unreleased app yields a channel that reports a healthy connection while no user can find the bot to message it. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the WeCom channel end-to-end against a real AI bot. Both directions were exercised with the same SDK version the runtime pins (@wecom/aibot-node-sdk 1.0.7), mirroring WeComBotBridge.start(): the WebSocket handshake authenticated, an inbound single-chat frame arrived, and an outbound markdown send returned a req_id. Records the decision that governs every other step: the channel is the 智能机器人 in API mode, not a custom app, not a group webhook, and not 普通模式 — the latter answers messages with WeCom's own hosted models, so nothing reaches the long connection. Documents the addressing consequence observed in the inbound frame. A single-chat frame carries no chatid at all, which is why the bridge falls back to the sender's userid, and why a WeCom conversation cannot be addressed until the bot has received a message in it. Also notes that private deployments cannot work, since the SDK endpoint is fixed and the bridge exposes no override. Clarifies globally that an empty matrix cell means Maka does not implement the capability, not that the platform lacks it: the WeCom SDK offers streaming replies, media upload and template cards that the bridge never wires up. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the QQ channel end-to-end against a real bot, mirroring QQBotBridge and the GatewayBridgeBase lifecycle it inherits. All three startup steps were exercised — app access token exchange, gateway URL fetch, and WebSocket IDENTIFY reaching READY — followed by an inbound C2C dispatch and an outbound send that returned a message id. Documents the failure taxonomy the code encodes but the console does not explain: 4014 and 4004 are fatal closes that stop the bridge, while every other code reconnects with backoff, so a channel that dies and stays dead is almost always a missing intent or bad credentials. Records the addressing scheme, since QQ is the only platform whose chat IDs carry a prefix. Four inbound dispatch types map to four prefixes and four REST routes, and an unprefixed chat ID is dropped before any network call. Notes three things the walkthrough surfaced: the console's Token field belongs to webhook mode and is never read; group and guild traffic only arrives when the bot is @-mentioned; and QQ supplies no display name for group or C2C senders, so the bridge uses the bot-scoped openid as the user name. Also flags that the API host is hardcoded to production, so a sandbox-confined bot cannot be exercised through this channel. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the Telegram channel end-to-end against a real BotFather bot: getMe identity, getUpdates long-poll receiving three inbound messages, sendMessage, and sendChatAction typing. Confirms the matrix's one Telegram-exclusive claim rather than assuming it. sendMessageDraft is reachable by an ordinary BotFather bot, so the native streaming-reply path genuinely works; it is not a method the public Bot API rejects. Also records the two limits around it — streams are refused outright for group targets, and a single draft failure latches the stream off so streaming stops silently rather than erroring. Documents the default that makes a working bot look broken in groups: getMe reports can_read_all_group_messages false, so privacy mode hides all group traffic except commands and @-mentions until it is disabled via BotFather and the bot is re-added. Also notes that the poll requests only 'message' updates, and that a bot cannot open a conversation, so a channel that has never been messaged first is indistinguishable from a broken one. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the Discord channel end-to-end against a real bot: REST /gateway/bot, gateway IDENTIFY reaching READY, an inbound MESSAGE_CREATE carrying non-empty content, an outbound send, and a typing indicator. Leads with the MESSAGE CONTENT INTENT because it has two failure modes and the second is the expensive one: either the gateway closes 4014, or messages arrive with every content field empty, leaving a bot that looks connected and awake while replying to nothing. Notes that Discord receives ordinary channel traffic with no @-mention, unlike QQ and Telegram groups, and that its chat IDs are bare channel IDs with none of QQ's prefixing. Records a proxy caveat honestly rather than as a confirmed defect. WsBridgeBase.createWebSocket() builds the socket without a dispatcher, so the channel's proxyUrl covers REST but not the gateway. Verification did not reproduce a failure — the unproxied socket connected — which is what a system-wide or TUN-mode proxy produces, so the gap is stated as conditional on the host rather than as an observed break. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the Slack channel end-to-end against a real workspace app: auth.test identity, Socket Mode connect, an inbound channel message and an inbound DM covering both branches of the isGroup derivation, and an outbound chat.postMessage. Documents the two-token split first, since the field named appSecret holds the app-level token rather than the signing secret, and a swap fails inside SocketModeClient.start() with an error that never names the cause. Records two setup gates found by walking it. A channel the bot has not been invited to rejects sends with not_in_channel, and direct messages stay disabled until App Home's Messages Tab allows them — until that box is checked Slack refuses DMs outright, message.im never fires, and nothing in the app's own configuration explains why. Notes that Slack is the only channel promoted to operational on connect rather than on first traffic, so a healthy-looking Slack channel proves less than a healthy-looking one elsewhere; that the mapper drops every event carrying a subtype or bot_id; and that Socket Mode has no replay, unlike Telegram's getUpdates offset. Also records that this channel ignores proxyUrl entirely, since neither WebClient nor SocketModeClient is given an agent. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walked the DingTalk channel against a live internal app. Setup and receive are verified: gettoken, gateway/connections/open, the Stream WebSocket, and an inbound direct message decoded from the /v1.0/im/bot/messages/get callback topic. Sending is not verified, because replying to a 1:1 conversation is broken. An inbound direct message yields conversationType "1", so isGroup is derived correctly, but chatId is stamped as conversationId and a 1:1 conversation ID also begins with "cid" — the exact prefix pickDingTalkSendRoute uses to detect a group. The reply is therefore posted to /v1.0/robot/groupMessages/send and rejected with resource.not.found. The comment above that helper describes the prefix as "cidp", which the implementation does not match. Routing to the 1:1 endpoint instead does not rescue it: that API wants staff user IDs, and the payload's senderId is an opaque $:LWCP_v1:$ token rejected with staffId.notExisted. DingTalkBotMessagePayload declares no field that endpoint accepts, so neither route can deliver a direct reply with what the bridge currently captures. Both failures were reproduced as a controlled pair against a live app, holding the robot code constant and varying only the chat ID. The section and the known-limitations list state the scope honestly: group replies were not exercised, and whether reusing appId as the robot code is valid remains unconfirmed. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Captured the raw Stream payload and extended the send experiment to a third case, which closes the diagnosis. Holding the conversation and the robot code constant and varying only the target: conversationId (what the bridge stamps) is rejected by the group endpoint, senderId is rejected by the 1:1 endpoint, and senderStaffId is delivered by the 1:1 endpoint and returns a processQueryKey. So the defect is not merely a misrouted prefix check. The identifier the 1:1 endpoint requires, senderStaffId, is present on every inbound bot message and is absent from DingTalkBotMessagePayload, so no reachable value in the mapped event can address a direct conversation. The payload also carries a sessionWebhook with an explicit expiry, which needs neither robot code nor staff ID and covers both conversation kinds. Recorded as a second possible route the bridge does not use. Resolves one earlier caveat: the payload's robotCode equals the app's appId, so the bridge's reuse of appId as the robot code is correct. Group replies are still unexercised and stay marked unverified. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WeChat has no ordinary bot API, so the channel picks between an iLink route and a local-bridge route from the value of webhookUrl. Both are now documented, along with the fact that Maka ships no local bridge at all: it defines an HTTP contract and expects a third-party process to implement it. Wrote that contract down as a table — /health, the SSE receive loop and its since cursor, POST /send, and the QR endpoints — then exercised Maka's own WechatBridge against a mock implementing it. Driving the real class rather than a reimplementation is what makes the exercise mean anything: every mapping and error decision under test is the shipped code. All six inbound filter rules pass, including the group-without- mention drop that most looks like a broken bridge, plus bearer auth and both send paths, with the failure diagnostic surfacing as the status reason. Building the mock is what surfaced /health, which the streaming loop alone does not reveal: start() probes it first for identity and to gate sending, and a bridge missing it never reaches the receive loop. The verification note is explicit that this covers the contract and not WeChat itself, and that the iLink route stays unverified because its token only comes from the in-app QR flow. Also records that the settings UI collects Official Account App ID and Secret which no code path in the bridge reads. With this the setup guide has no placeholders left. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rebased onto current main, which renamed the per-channel status reasons to snake_case. Updated the three the guide quotes: missing-slack-tokens to slack_tokens_missing, missing-feishu-credentials to feishu_credentials_missing, and the generic no-credentials to wecom_credentials_missing now that each channel carries its own code. Re-derived the capability matrix against the new tree; every row still maps to the same bridges, and the load-bearing constants are unchanged (Discord's 2000-character limit, Telegram's 4000 UTF-16 units and message-only allowed_updates, QQ's fatal 4004/4014 closes, Slack promoting to operational on connect). Points the DingTalk defect at apache#5112, which replaces the prefix guess with chat IDs stamped at receive time. That fix is not on main yet, so the section still documents the broken behaviour and says what to rewrite once it lands. Refs apache#3894 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guide said the channel's proxyUrl reaches every bot API call through proxiedFetch. It does not. No bridge reads proxyUrl, and proxiedFetch asks resolveActiveProxy() for the app's network proxy, which nothing in the product ever sets — so on main every bot request goes direct. The earlier claim came from reading proxiedFetch without tracing who populates the value it reads. The live verifications that looked like proxy coverage all ran with a TUN-mode proxy active, which intercepts below the application. They do not demonstrate Maka-level proxy support, and the guide now says so. Replaces the five affected passages with one Network routing section: what does not route (proxyUrl, the unwired app proxy), what would stay outside proxiedFetch even once wired (the gateway sockets, and the Slack, Feishu/Lark and WeCom SDKs), and what works today (a system-level route). Adds it to the known limitations and points at apache#5159, which wires the app proxy into the REST paths only. Also records that iLink replies are sent with an empty context_token and inbound context is never kept, credited to apache#5115, where it was first identified. Left marked untested, since the iLink route could not be exercised. Refs apache#3894 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
apache#5112 merged on September 12, but the guide still said direct replies could not work, that senderStaffId was never captured, and that the fix was open. That would publish false setup guidance. The DingTalk section now documents the merged scheme: the route is stamped at receive time, group:<conversationId> for groups and oto:<senderStaffId> for direct chats, with unstamped scheduled-task targets still routed by shape. It keeps the one real dead end: a direct message without senderStaffId falls back to its bare conversation ID and a reply to it is rejected at the group endpoint. The three-way live send table stays, reframed as the evidence for why direct replies address the staff ID. The verification wording is kept exact: the request the merged bridge now sends was delivered live, but the merged bridge itself has not been re-run, and group replies remain unexercised. Also stops describing sessionWebhook as working for both conversation kinds. That came from DingTalk's documentation, not from a send made here, so it is now marked untested. Refs apache#3894 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Re-checked against main at 85a1031 after apache#6074 and apache#6156 (code reading, not a live re-run): - User allowlist: enforced for all eight bridges in BaseBotAdapter.emitIncomingMessage; the remaining gap is that the settings UI only exposes the field for Telegram and Feishu (QR onboarding fills it for WeChat, and for Feishu when the scan returns a user ID). Matrix, known limitation 2, security and Feishu setup updated. - Typing: Feishu (Typing reaction) and WeChat iLink now send and cancel typing; new cancelTypingIndicator row. - Inbound attachment kind: Feishu now classifies attachments. - Message limits: Feishu personal-assistant sends split at 12,000 bytes of JSON-encoded text. - New section on task / workhub / assistant message handling, linking the desktop README transport sections. - WeChat iLink: DM-only; context_token now stored, persisted and sent with replies (fixed by apache#6156, first found in apache#5115). Dropped the unsourced 8.0.70 and wxp_act_ claims. - Added a last-checked note at the top. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1bb193c to
0e3c502
Compare
|
Rebased onto current
Still waiting on maintainers for the merge order with #5115. As agreed with @Pr1p, this one would be the English base, with the zh-CN guide and the security corrections as a follow-up. |
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
I read the current two-file documentation diff at 0e3c502101cc4da154258b1b51bec0c807688dc8, with particular attention to the changes since the previously reviewed 1bb193cc6ff3d855680f252bb7b3549f7c92ddcc. I checked the updated matrix and descriptions against current main 85a1031c10d104456127f35aac1523b9ffb1a384.
The changed claims match the inspected production paths: shared allowlist enforcement in BaseBotAdapter.emitIncomingMessage; the narrower settings UI and single-user/private-chat assistant admission; Feishu's message reaction start/cancel, first-resource attachment mapping and assistant-only JSON-byte splitting; and WeChat iLink's DM/self-message filters, per-user bounded persisted context and typing start/cancel. The new message-handling section matches the core resolver. This corrects the older unprotected-channel and empty-iLink-context descriptions without representing the historical live walkthroughs as a new run.
I found no confirmed new P0–P2 in this documentation update. I checked that the repository files targeted by all three relative links exist, and git diff --check passed. A real merge-tree against the main commit above was clean; the current-head hosted test check is successful. The page accurately ties its unwired-proxy description to that main commit and identifies #5159 as a separate pending change; that section will need another update if #5159 merges first.
This review is a source/documentation comparison. I did not rerun vendor-console setup, external accounts, live sends/receives, third-party WeChat delivery or a new runtime test suite at this documentation-only head. The page's historical platform verification results remain the author's results. No runtime code, wire protocol or database migration changes are present.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
Refs #3894.
Adds
docs/bot-platforms.mdcovering the eight IM bot channels registered inBOT_PROVIDERS, plus an index entry indocs/README.md.What's in it
feishuchannel and are separated by thedomainsetting rather than by provider.BotReadinessStateenum instead of inventing a parallel vocabulary.Claims are cited by symbol name rather than line number, so the references survive refactors.
Verification
Every setup section was walked against a real account rather than written from the code alone. For each platform the probe mirrored the bridge's own code path — same SDK version where one is used, same constructor options, same endpoints — so a passing probe means the shipped bridge would connect.
WeChat is the one channel with no real-account coverage. Maka ships no local bridge — it defines an HTTP contract and expects a third-party process to implement it — so the guide writes that contract down and verifies Maka's own
WechatBridgeclass against a mock implementing it. Driving the real class rather than a reimplementation is what makes it meaningful. Building the mock is also what surfacedGET /health, whichstart()probes before anything else and which reading the streaming loop alone does not reveal. The section states plainly that WeChat delivery itself is unverified, as is the iLink route — including the emptycontext_tokenon iLink replies that #5115 identified.Things the walkthrough found that reading the code did not
message.imsimply never fires, and nothing in the app's configuration explains why.cidprefix, which every DingTalk conversation ID carries. A three-way controlled send showed the working target is the payload'ssenderStaffId, which the bridge did not capture. That became fix(runtime): route DingTalk 1:1 replies by staff id #5112, merged on Sep 12; the guide now documents itsoto:/group:addressing and keeps the one remaining dead end, a direct message withoutsenderStaffId.contentempty, so the bot looks connected and awake while replying to nothing.getMereportscan_read_all_group_messages: false.Gaps worth separate issues
Flagging rather than fixing, since this PR is documentation only.
allowedUserIdsis enforced by three of eight bridges. Telegram, Feishu and WeCom check it. Discord, QQ, DingTalk, Slack and WeChat never read the field, so an allowlist configured there silently does nothing.SendCapableis text-only.proxyUrlis read by no bridge, and the app's network proxy is never handed toproxiedFetch, so every bot request goes direct. An earlier revision of this PR claimed otherwise; that was wrong and is corrected. Every live check here ran under a TUN-mode proxy, which is why they passed. fix(bots): apply the configured network proxy to Client-owned bot traffic #5159 wires the app proxy into the REST paths; gateway sockets and the SDK-based channels stay outside it.Checks
npm run check:asf-headerspasses.npm run format:check— the only failure is a pre-existing.claude/launch.jsonissue onmain, untouched by this branch.main; the capability matrix was re-derived against it and the renamed snake_case status reasons picked up.🤖 Generated with Claude Code