feat(channels): support DingTalk webhook delivery to direct messages - #6891
Conversation
本地 E2E 测试报告场景验证外部 Webhook 通过 daemon 触发真实 AI 任务，并把最终 Markdown 主动投递到钉钉单聊。 环境
结果
自动化验证
未覆盖本次没有可用的群聊 |
8dc1575 to
9f42f1a
Compare
|
上游 WebShell 重复 import 已由 #6890 修复并合入。本分支现已 rebase 到最新 |
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
Thanks for the PR! Template looks good ✓ — all required sections present with bilingual translation. Problem: Observed and real. The PR links issue #6883 and provides concrete before/after evidence — a local Direction: Aligned. Extending the existing DingTalk webhook delivery to support DM targets via the one-to-one robot API is a natural, well-scoped addition. No new channel type or config schema is introduced — it reuses Size: Not applicable for the core module gate — Approach: The scope feels right. The shared-runtime hook ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必需部分和双语翻译均已包含。 问题：已观测且真实存在。PR 关联了 issue #6883 并提供了具体的 before/after 证据——本地 方向：对齐。通过一对一机器人 API 扩展现有钉钉 Webhook 投递以支持单聊目标，是自然且范围合理的扩展。未新增 channel 类型或配置结构——复用 规模：不适用核心模块门控—— 方案：范围合理。共享 runtime 钩子（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: I'd add a webhook-specific target support hook to The PR's implementation matches this closely and is well-executed. Key observations:
No critical blockers. No AGENTS.md violations. Real-Scenario TestingRan Health endpoint: Unit tests (all passing):
Full DM delivery E2E requires live DingTalk credentials, which aren't available in CI. The author confirms real delivery in the PR description. 中文说明代码审查独立方案： 我会在 PR 实现与此高度一致，执行良好。 关键观察：
无关键阻断问题，无 AGENTS.md 违规。 真实场景测试从 PR 分支运行 单元测试（全部通过）： DingtalkAdapter 69/69，ChannelBase 397/397，Typecheck 全部通过。 完整 DM 投递 E2E 需要真实的钉钉凭据，CI 环境不可用。作者在 PR 描述中确认了真实投递。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; would merge without hesitation. This is a well-executed feature PR. The problem is real (webhook users can't target DingTalk DMs), the solution is minimal (~85 production lines), and the implementation follows the project's conventions precisely. The shared-runtime hook pattern ( Tests are thorough: 69 DingTalk adapter tests covering both group and DM paths, token reuse, 401 retry, response validation, and error logging. 397 ChannelBase tests including webhook target independence. All pass. Typecheck clean. The author's before/after evidence (curl → 202 → task completed → message received) confirms the feature works end-to-end. The design doc and implementation plan are committed to Approving. ✅ 中文说明信心度：5/5 — 各阶段均通过，可以合并。 这是一个执行良好的功能 PR。问题真实存在（Webhook 用户无法将消息投递到钉钉单聊），方案最小化（约 85 行生产代码），实现严格遵循项目规范。 共享 runtime 钩子模式（ 测试充分：69 个 DingTalk adapter 测试覆盖群聊和单聊路径、Token 复用、401 重试、响应校验和错误日志。397 个 ChannelBase 测试包含 Webhook 目标独立性验证。全部通过。Typecheck 干净。 作者的 before/after 证据（curl → 202 → 任务完成 → 消息送达）确认了端到端功能正常。设计文档和实现计划已提交到 批准 ✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
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
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1, chunk 2, chunk 3 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Review — PR #6891 (DingTalk webhook direct-message delivery)
Verdict: COMMENT — 0 Critical, 3 Suggestions. Targeted build passes; 459 unit tests pass (396 ChannelBase + 63 DingtalkAdapter); issue #6883 fully matched (the DM body shape was verified against the reference implementation cited in the issue).
Clean, well-scoped change: a new ChannelBase.supportsProactiveWebhookTarget hook that defaults to supportsProactiveTarget (so non-DingTalk channels are unaffected), a DingTalk override that accepts isGroup:false webhook targets, and sendProactiveChunk routing group → groupMessages/send / direct → oToMessages/batchSend. The webhook-only broadening correctly keeps ordinary channel loops group-only, so an inbound DM conversation id can't be mistaken for a user id. The renames (isConversationId → isStableTargetId, GROUP_MSG_KEY → PROACTIVE_MSG_KEY) are complete and behavior-preserving.
Two Suggestions are posted inline. One more, lower-priority note that couldn't be anchored to a changed line:
[Suggestion] Proactive-send failure logs omit target kind and chatId — in sendProactiveChunk, sends can now be group or direct, but the failure logs carry only chunk N/M. For the unattended webhook path, a failed delivery can't be correlated to which configured target/chat without cross-referencing other events. Consider including the group/direct kind and a sanitized chatId in both the proactive send error and proactive send failed logs.
🤖 Automated review · 12 dimension agents + 3 reverse-audit rounds · not a maintainer gate
|
@wenshao Follow-up on the unanchored logging suggestion: e700463 adds group/dm to both transport-error and HTTP-failure logs, with regression coverage for each path. I intentionally did not log chatId: it is a stable DingTalk user/conversation identifier, and copying it into unattended daemon logs would broaden PII exposure. The target kind identifies the selected DingTalk API path; the configured webhook target and task metadata remain the appropriate correlation sources when a specific destination must be investigated. The same commit also handles batchSend HTTP 200 recipient failures by surfacing invalidStaffIdList and flowControlledStaffIdList as delivery errors. Local verification: DingTalk 67/67, ChannelBase 397/397, build, typecheck, and targeted ESLint all passed. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: chunk 1, chunk 2, chunk 3 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed diff-only — the PR’s existing discussion could not be fetched, so this is not an approval and not a no-blockers claim. Unresolved, please confirm: [Critical] packages/channels/dingtalk/src/DingtalkAdapter.ts:631 — existing malformed-JSON Critical: Response.json() does consume the body, but non-JSON HTTP 200 delivery semantics could not be established from the API contract [Critical] review 4697493827 — existing coverage blocker: all three chunks were manually read in this run, but the transcript coverage harness was unavailable Not reviewed: coverage verification — qwen transcript harness unavailable; all 3 chunks were manually read but could not be machine-verified. Not reviewed: build-and-test — npm test reached its 120-second cap; targeted ChannelBase and DingTalk suites passed, but the full-suite result was inconclusive. Not reviewed: CI — route check is failing at the reviewed commit. Not reviewed: coverage — could not read the agents' transcripts (the CLI did not export QWEN_CODE_PROJECT_DIR / QWEN_CODE_SESSION_ID, so this run cannot find the harness's record of what its agents did), so this run cannot show that any of the diff was read.
— Codex $qreview via Qwen Code /review
|
Review follow-up is complete in 6c5148f. Malformed HTTP 200 JSON from the DingTalk one-to-one API is now a delivery error rather than a silent success. The regression test confirms |
本地 E2E 回归结论已基于最新提交
结论：本 PR 覆盖的“外部 webhook → daemon → 钉钉单聊/群聊 Markdown 投递”已完成真实端到端验证。 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] review 4697493827 coverage blocker — the Qwen transcript harness remains unavailable in Codex, so machine coverage could not be verified even though the root and reviewers manually read all three chunks Not reviewed: build-and-test — root npm test was interrupted before completion; the changed ChannelBase and DingTalk test files were separately proven effective by the efficacy probe. Not reviewed: coverage — could not read the agents' transcripts (the CLI did not export QWEN_CODE_PROJECT_DIR / QWEN_CODE_SESSION_ID, so this run cannot find the harness's record of what its agents did), so this run cannot show that any of the diff was read.
— Codex $qreview via Qwen Code /review
| cd packages/channels/base && npx vitest run src/ChannelBase.test.ts && npm run build | ||
| cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts && npm run build | ||
| cd /Users/ben/workspace/qwen-code && npm run build && npm run typecheck |
There was a problem hiding this comment.
[Suggestion] The verification block cannot be run as written: after the first command changes into packages/channels/base, the second relative cd resolves below it, and the final command hard-codes the author's /Users/ben checkout. A contributor copying this block stops after the first package, so the documented DingTalk/root verification never runs.
| cd packages/channels/base && npx vitest run src/ChannelBase.test.ts && npm run build | |
| cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts && npm run build | |
| cd /Users/ben/workspace/qwen-code && npm run build && npm run typecheck | |
| (cd packages/channels/base && npx vitest run src/ChannelBase.test.ts && npm run build) | |
| (cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts && npm run build) | |
| npm run build && npm run typecheck |
— Codex $qreview via Qwen Code /review
| } catch { | ||
| process.stderr.write( | ||
| `[DingTalk:${this.name}] proactive send failed (${targetKind}, ${chunkLabel}): invalid JSON response\n`, |
There was a problem hiding this comment.
[Suggestion] This catch treats every Response.json() rejection as invalid JSON, but body reads can also fail after headers because the stream times out or resets. In that case the worker discards the original TimeoutError/socket error and reports a schema problem, leaving the on-call unable to diagnose a transport failure. Catch the error, distinguish SyntaxError from read/abort failures, preserve a sanitized cause in the log and thrown delivery error, and add a stream-error or timeout regression test.
— Codex $qreview via Qwen Code /review
| protected supportsProactiveWebhookTarget(target: SessionTarget): boolean { | ||
| return this.supportsProactiveTarget(target); | ||
| } |
There was a problem hiding this comment.
[Suggestion] This new webhook-only target hook changes the adapter extension contract, but the canonical adapter guide still says only supportsProactiveTarget() controls webhook target shapes. An adapter author following that guide can broaden the ordinary predicate for a webhook-only shape and unintentionally enable it for loops—the boundary this hook exists to protect. Document supportsProactiveWebhookTarget(), its default delegation, and when adapters should override it separately.
— Codex $qreview via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1, chunk 2, chunk 3 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
| const cause = (err as { cause?: unknown }).cause; | ||
| process.stderr.write( | ||
| `[DingTalk:${this.name}] proactive send error (${chunkLabel}): ${err}${cause ? ` (${cause})` : ''}\n`, | ||
| `[DingTalk:${this.name}] proactive send error (${targetKind}, ${chunkLabel}): ${err}${cause ? ` (${cause})` : ''}\n`, |
There was a problem hiding this comment.
[Suggestion] The catch block logs ${err} and ${cause} directly to stderr without sanitizeLogText. The adjacent !resp.ok branch already uses sanitizeLogText(await resp.text().catch(() => ''), 300) for the same class of external data, making the two error paths inconsistent. Concrete cost: A network error whose .message or .cause contains control characters (CR, ANSI escape sequences) could corrupt the stderr log line — e.g. \r overwriting the line prefix or an ANSI OSC sequence injecting a fake log entry.
| const cause = (err as { cause?: unknown }).cause; | |
| process.stderr.write( | |
| `[DingTalk:${this.name}] proactive send error (${chunkLabel}): ${err}${cause ? ` (${cause})` : ''}\n`, | |
| `[DingTalk:${this.name}] proactive send error (${targetKind}, ${chunkLabel}): ${err}${cause ? ` (${cause})` : ''}\n`, | |
| const cause = (err as { cause?: unknown }).cause; | |
| process.stderr.write( | |
| `[DingTalk:${this.name}] proactive send error (${targetKind}, ${chunkLabel}): ${sanitizeLogText(String(err), 300)}${cause ? ` (${sanitizeLogText(String(cause), 200)})` : ''}\n`, |
— qwen3.7-max via Qwen Code /review
| let data: DingTalkDirectMessageResponse; | ||
| try { | ||
| data = (await resp.json()) as DingTalkDirectMessageResponse; | ||
| } catch { |
There was a problem hiding this comment.
[Suggestion] If DingTalk returns valid JSON that is not an object (e.g. JSON null, a number, or a string) with HTTP 200, resp.json() succeeds and data is assigned a non-object value. The subsequent data.invalidStaffIdList then throws TypeError: Cannot read properties of null outside the try/catch, bypassing the structured error handling. Concrete cost: A DingTalk API change returning "null" or a non-object body causes a raw TypeError instead of the informative "invalid JSON response" error.
| let data: DingTalkDirectMessageResponse; | |
| try { | |
| data = (await resp.json()) as DingTalkDirectMessageResponse; | |
| } catch { | |
| let data: DingTalkDirectMessageResponse; | |
| try { | |
| const raw = await resp.json(); | |
| if (raw === null || typeof raw !== 'object') { | |
| throw new TypeError('non-object response'); | |
| } | |
| data = raw as DingTalkDirectMessageResponse; | |
| } catch { |
— qwen3.7-max via Qwen Code /review
| if (data.flowControlledStaffIdList?.includes(target.chatId)) { | ||
| process.stderr.write( | ||
| `[DingTalk:${this.name}] proactive send failed (${targetKind}, ${chunkLabel}): direct recipient rate limited\n`, |
There was a problem hiding this comment.
[Suggestion] The DM response is only checked for invalidStaffIdList and flowControlledStaffIdList. If DingTalk returns HTTP 200 with a top-level error (e.g. {"errcode": 310000, "errmsg": "robot not authorized"}) that doesn't populate either list, the send is silently treated as success. Concrete cost: A permission change or robot misconfiguration causes silent delivery failure — the webhook caller believes the operator was notified, but the operator never receives anything. In a CI/monitoring context, this defeats the feature's core purpose.
Consider checking for common DingTalk error fields after the two list checks, or logging the full response body at debug level for investigation:
if ((data as Record<string, unknown>).errcode) {
// handle application-level error
}— qwen3.7-max via Qwen Code /review
| if (!this.supportsProactiveWebhookTarget(target)) { | ||
| throw new Error( | ||
| 'Channel does not support proactive webhook messages for this chat target.', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The rejection error message is identical regardless of why the target failed validation. At 3 AM, seeing "Channel does not support proactive webhook messages for this chat target" gives no clue whether isGroup was missing, chatId was a webhook URL, chatId was empty, or threadId was set. Concrete cost: An operator must manually diff the target config against the documentation example to find the missing field, increasing mean-time-to-diagnosis.
Consider logging the sanitized target fields before throwing:
process.stderr.write(
`[Channel:${this.name}] webhook target rejected: isGroup=${target.isGroup}, threadId=${target.threadId}, chatId=${target.chatId ? '***' : '(empty)'}\n`,
);— qwen3.7-max via Qwen Code /review
| text: string, | ||
| chunkLabel: string, | ||
| ): Promise<void> { | ||
| const targetKind = target.isGroup === true ? 'group' : 'dm'; |
There was a problem hiding this comment.
[Suggestion] DM failure logs include the channel name, target kind (dm), and chunk label, but never the recipient user ID or targetRef. When a DingTalk channel has multiple DM targets configured, you cannot tell which DM target failed from the adapter log alone. Concrete cost: At 3 AM with two DM targets (e.g. operator and oncall-engineer), you must correlate timestamps against the ChannelBase webhook task log to figure out who didn't get paged.
Consider including a sanitized form of the target (e.g. last 4 chars of chatId or the targetRef) in the log lines.
— qwen3.7-max via Qwen Code /review
| /** Stable API targets are conversation or user IDs, never webhook URLs. */ | ||
| private isStableTargetId(chatId: string): boolean { | ||
| return !!chatId && !/^https?:\/\//i.test(chatId); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The rename from isConversationId to isStableTargetId broadens the method's apparent scope — the doc comment says "conversation or user IDs" — but the same method is still used by startReaction and stopReaction where the chatId must be a conversation ID specifically (the emotion API requires one). A user ID would pass isStableTargetId but fail at the emotion API level. Concrete cost: A future maintainer reading the method name and doc comment would conclude "any non-URL identifier is fine" and reuse it in a context where only conversation IDs are valid, causing silent API failures.
Consider adding a comment at the reaction call sites clarifying that isStableTargetId rejects webhook-URL fallback chatIds, not ID types.
— qwen3.7-max via Qwen Code /review
| it('stops at the first failed chunk', async () => { | ||
| const channel = proactive(createChannel()); | ||
| vi.spyOn(process.stderr, 'write').mockImplementation(() => true); | ||
| const { sendCalls } = stubProactiveFetch( | ||
| const { directSendCalls } = stubProactiveFetch( |
There was a problem hiding this comment.
[Suggestion] The 'stops at the first failed chunk' test was changed from groupTarget to directTarget. While the chunking loop is shared code, the group-specific sendProactiveChunk path under multi-chunk failure no longer has a dedicated assertion. Concrete cost: A regression that only affects group-target error handling (e.g. silently swallowing the error for group but not DM) would not be caught by this test.
Consider parameterizing: it.each([groupTarget, directTarget])('stops at the first failed chunk for %s', ...).
— qwen3.7-max via Qwen Code /review
| if (target.isGroup === false) { | ||
| let data: DingTalkDirectMessageResponse; | ||
| try { | ||
| data = (await resp.json()) as DingTalkDirectMessageResponse; |
There was a problem hiding this comment.
[Suggestion] The DM response-body error detection is embedded inline in sendProactiveChunk with no structural separation from the group path. A future maintainer who refactors for readability could easily collapse both paths into one and either strip the DM body-parsing (causing silent delivery failures) or apply it to the group path (causing crashes when the group API returns {}). Concrete cost: The invariant "DM requires body inspection, group does not" is implicit — it lives only in the code structure, not in a named method or comment.
Consider extracting post-200 response validation into a private method like validateProactiveSendResponse(target, resp, chunkLabel) with a comment explaining the API-level difference.
— qwen3.7-max via Qwen Code /review
| for (let attempt = 0; ; attempt++) { | ||
| const token = await this.getProactiveToken(); | ||
| let resp: Response; | ||
| try { | ||
| resp = await fetch(GROUP_MSG_API, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'x-acs-dingtalk-access-token': token, | ||
| 'Content-Type': 'application/json', | ||
| const targetBody = | ||
| target.isGroup === true | ||
| ? { openConversationId: target.chatId } | ||
| : { userIds: [target.chatId] }; |
There was a problem hiding this comment.
[Suggestion] targetBody, the API URL, and targetKind are computed inside the for (let attempt = 0; ; attempt++) retry loop, but they are pure functions of target, which never changes across iterations. On a 401 retry, both are recomputed identically. Concrete cost: Extra object allocations and conditionals on each retry, plus readability — a future maintainer must verify these values are loop-invariant before reasoning about the retry path.
Hoist above the loop:
const isGroup = target.isGroup === true;
const targetKind = isGroup ? 'group' : 'dm';
const targetBody = isGroup
? { openConversationId: target.chatId }
: { userIds: [target.chatId] };
const apiUrl = isGroup ? GROUP_MSG_API : DIRECT_MSG_API;— qwen3.7-max via Qwen Code /review
| protected override supportsProactiveWebhookTarget( | ||
| target: SessionTarget, | ||
| ): boolean { | ||
| return ( | ||
| typeof target.isGroup === 'boolean' && | ||
| target.threadId === undefined && | ||
| this.isStableTargetId(target.chatId) |
There was a problem hiding this comment.
[Suggestion] isStableTargetId only rejects empty strings and HTTP URLs. For DM targets, there is no validation that the chatId resembles a DingTalk user ID. An admin who deploys with the literal placeholder "DINGTALK_USER_ID" from the docs passes validation, reaches the DingTalk API, and fails opaquely. Concrete cost: The design doc states "invalid targets fail before the agent runs," but this gate is too permissive to honor that guarantee for DM targets — misconfigured IDs pass validation and fail at the API level with an opaque error.
Consider adding a basic format check for DM targets (e.g. rejecting IDs that don't match /^[a-zA-Z0-9._-]+$/ or exceed a reasonable length).
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. The prior malformed-JSON Critical is fixed on this commit (the DM path now wraps resp.json() in try/catch and throws on parse failure, and the body is drained — regression test asserts bodyUsed === true). Two new suggestions are inline.
— qwen-latest-series-invite-beta-v77 via Qwen Code /review
| if (target.isGroup === false) { | ||
| let data: DingTalkDirectMessageResponse; |
There was a problem hiding this comment.
[Suggestion] The group/DM decision is derived with two different predicates: API routing keys on target.isGroup === true (anything not-true → DM endpoint/userIds), but this DM response-validation block keys on target.isGroup === false (anything not-false → group drain). For a boolean these are equivalent, but they diverge for isGroup === undefined — such a target would POST to the one-to-one DM endpoint yet skip the invalidStaffIdList/flowControlledStaffIdList checks, silently reintroducing the very "DM non-delivery reported as success" bug this PR fixes. Unreachable today (all three pushProactive call sites gate isGroup to a boolean), so this is a latent footgun rather than a live bug. — Concrete cost: a future caller or refactor that reaches sendProactiveChunk with a non-boolean isGroup silently bypasses DM delivery validation.
Derive the decision once and branch on it everywhere so routing and validation can never disagree:
const isGroup = target.isGroup === true;
const targetKind = isGroup ? 'group' : 'dm';
// ...targetBody / endpoint: isGroup ? group : dm
if (!isGroup) { /* DM per-recipient validation */ }— qwen-latest-series-invite-beta-v77 via Qwen Code /review
| 'DingTalk proactive send failed: invalid direct recipient', | ||
| ); | ||
| } | ||
| if (data.flowControlledStaffIdList?.includes(target.chatId)) { |
There was a problem hiding this comment.
[Suggestion] A flow-controlled recipient (flowControlledStaffIdList) is a transient rate-limit, but it is thrown immediately just like a permanent invalid recipient (invalidStaffIdList) — no backoff or retry — even though the 401 branch just above deliberately retries once precisely because its condition is transient. The distinct direct recipient rate limited message shows the code already recognizes this is a different kind of failure, yet acts on it the same way. — Failure scenario: this feature targets CI/alerting → DM, exactly the burst scenario where DingTalk flow-control bites; a short alert storm or several CI runs to one operator can flow-control a single-chunk notification, which is then dropped and reported as a hard failure instead of recovering after a brief wait. (Note: flow-control needs backoff, not the immediate 401-style retry, which would just fail again.) Consider distinguishing transient flow-control from a permanent invalid recipient and backing off before a bounded retry.
— qwen-latest-series-invite-beta-v77 via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local verification — maintainer merge referenceI built and drove the real PR code end-to-end (not the mocked unit layer) to confirm the DingTalk webhook → direct‑message feature actually works, is regression‑safe for groups, and that the change is load‑bearing. Verified at head
1) Cross‑layer real‑HTTP E2E (32/32)Drives the real built What it proves:
2) Load‑bearing controlled experimentReverting the single gate line in
3) Unit suites on PR source
Notes for merge
Verdict: LGTM — headline behavior verified end‑to‑end at the real cross‑layer boundary, group path regression‑guarded, and the gate change proven load‑bearing. 中文版本（点击展开）✅ 本地验证 —— 供 maintainer 合并参考我在本地构建并端到端驱动了 PR 的真实代码（而非 mock 的单测层），确认「钉钉 Webhook → 单聊」功能确实可用、对群聊路径无回归，并且该改动是「承重（load‑bearing）」的。验证基于 head
1）跨层真实 HTTP 端到端（32/32）以真实构建的 它证明了：
2）承重性对照实验（before/after）只回退 3）PR 源码上的单元测试
合并注意事项
结论：LGTM —— 核心行为已在真实跨层边界端到端验证，群聊路径有回归防护，网关改动被证明是承重的。 |
3 participants
What this PR does
This PR extends daemon-triggered DingTalk webhook delivery so the existing channel can send the agent's final Markdown response to either a configured user or group. Direct-message targets use the one-to-one robot API, while existing group targets continue to use the group-message API. Both paths share the current access-token cache, Markdown chunking, one-time HTTP 401 refresh, and delivery error handling.
No new channel type or webhook schema is introduced. Targets still use the existing
chatId,senderId, andisGroupfields, withisGroup: falseselecting a DingTalk user ID andisGroup: trueselecting a groupopenConversationId.The shared channel runtime now has a webhook-specific target check that defaults to each channel's existing proactive-target behavior. DingTalk overrides only that webhook check for direct messages, so ordinary channel loops remain group-only and cannot mistake an inbound direct-message conversation ID for a DingTalk user ID.
Why it's needed
External events such as CI results, deployment status, and operational alerts often need to notify one responsible DingTalk user instead of an entire group. The daemon webhook flow already runs unattended agent tasks and routes their final responses to configured targets, but the DingTalk adapter previously rejected every proactive target that was not a group.
Reviewer Test Plan
How to verify
Configure one DingTalk webhook target with a user ID and
isGroup: false, then startqwen servewith that channel enabled. Submit an authenticated request to the configured webhook route and confirm it returns202, the real model completes the unattended task, and the resulting Markdown reaches the selected DingTalk user. Also run the channel-base and DingTalk adapter tests and confirm existing group targets still useopenConversationId, direct-message webhook targets useuserIdsand share the same cached access token, and ordinary direct-message channel-loop targets remain rejected.Evidence (Before & After)
Before: DingTalk proactive delivery accepted only group targets, so a configured
isGroup: falsewebhook target was rejected before delivery.After: A local authenticated
curlrequest returned202 {"accepted":true};qwen3-coder-pluscompleted the task with an HTTP 200 model response; the daemon reported the prompt turn completed with no webhook or proactive-send failure; and the recipient confirmed the final Markdown arrived in DingTalk direct messages.Tested on
Environment (optional)
Local source build with Node.js 25.2.1, a loopback
qwen servedaemon, a real Alibaba Cloud Model Studio model, and a real DingTalk Stream bot application.Risk & Scope
Linked Issues
Closes #6883
中文说明
本 PR 做了什么
本 PR 扩展了 daemon 触发的钉钉 Webhook 投递能力，使现有 DingTalk Channel 可以把 agent 的最终 Markdown 响应发送到预配置的用户或群聊。单聊目标调用一对一机器人消息接口，已有群聊目标继续调用群消息接口。两条路径共同复用现有 access token 缓存、Markdown 分片、HTTP 401 单次刷新重试和投递错误处理。
本次没有新增 Channel 类型或 Webhook 配置结构。目标仍然使用已有的
chatId、senderId和isGroup字段；isGroup: false表示chatId是钉钉用户 ID，isGroup: true表示chatId是群聊openConversationId。共享 channel runtime 增加了 Webhook 专用目标检查，其默认行为仍委托给各 channel 原有的主动投递目标检查。钉钉仅在 Webhook 检查中放开单聊，因此普通 channel loop 仍只接受群聊目标，不会把入站单聊的会话 ID 错当作钉钉用户 ID。
为什么需要
CI 结果、部署状态和运维告警等外部事件经常只需要通知一个具体负责人，而不是整个群。daemon Webhook 流程已经能够运行无人值守 agent 任务并把最终响应路由到配置目标，但 DingTalk adapter 之前会拒绝所有非群聊的主动投递目标。
Reviewer Test Plan
验证方式
配置一个包含钉钉用户 ID 且
isGroup: false的 Webhook 目标，然后启动包含该 Channel 的qwen serve。向配置的 Webhook 路由提交鉴权请求，确认接口返回202、真实模型完成无人值守任务，并且最终 Markdown 到达指定钉钉用户。同时运行 channel-base 和 DingTalk adapter 测试，确认已有群聊目标继续使用openConversationId，单聊 Webhook 目标使用userIds并复用相同的 access token 缓存，普通单聊 channel-loop 目标仍会被拒绝。前后证据
修改前：钉钉主动投递只接受群聊目标，因此配置为
isGroup: false的 Webhook 目标会在投递前被拒绝。修改后：本地鉴权
curl请求返回202 {"accepted":true}；qwen3-coder-plus以 HTTP 200 完成任务；daemon 记录 prompt turn completed，且没有 Webhook 或主动发送失败；接收人确认钉钉单聊已收到最终 Markdown。测试平台
环境
Node.js 25.2.1、本地源码构建、loopback
qwen servedaemon、真实阿里云百炼模型和真实钉钉 Stream 机器人应用。风险和范围
关联 Issue
Closes #6883