fix(channels): extract quoted DingTalk text, richText, and card replies - #9537
fix(channels): extract quoted DingTalk text, richText, and card replies#9537BenGuanRan wants to merge 2 commits into
Conversation
github-actions
Bot
added
the
review/self-reported
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, not theoretical. Linked issue #9536 documents — and I confirmed against the current code — that Direction: aligned. This fixes context loss in an existing channel feature — quoted reply context reaching the agent — rather than adding new surface. No CHANGELOG reference, but that's expected: DingTalk channels are Qwen Code-specific, not an upstream Claude Code feature. Size: not applicable — Approach: the scope feels right. It extends the existing Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths (streaming/MCP/shell/sandbox/ACP). The quoted text is attacker-controlled input, but it feeds the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，不是理论性问题。关联 issue #9536 记录了、并且我也在当前代码中确认了： 方向：对齐。这是修复既有通道功能（引用回复上下文进入 agent）的上下文丢失问题，没有引入新面。CHANGELOG 无直接引用，但这属于预期：钉钉通道是 Qwen Code 特有功能，不是上游 Claude Code 的特性。 规模：不适用—— 方案：范围合理。在既有 风险：无升级风险信号——变更文件均不匹配 revert 相关的高风险路径（streaming/MCP/shell/sandbox/ACP）。引用文本属于攻击者可控输入，但它进入的是 ChannelBase 既有的 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first, for honesty: given the issue, I would have branched What I verified in the diff:
No correctness, security, or convention blockers found. The sanitization story holds: extraction only changes what reaches the existing Testing evidence (this PR's own CI)Unattended CI run — I do not execute PR code; the evidence below is the PR's own CI on the reviewed commit, read via the API. As of this pass the main unit suite is still running, prechecks are green, and there are no failures. The windows/macos/integration jobs show skipped at this point — no conclusion to report for them yet. The finalize workflow will refresh this table once CI settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 One gap worth naming: the unit tests pin the extraction logic against the payload shapes from the issue, but the shapes themselves are the author's device-verified claim — CI cannot prove real DingTalk callbacks actually look like that, and neither static review nor a green suite settles it. Sandboxed verification would settle the extraction half: Not verified: live DingTalk device behavior (no credentials in CI); windows/macos test conclusions (jobs skipped so far). 中文说明代码审查先说我的独立方案以示诚实：针对这个 issue，我会让 已核对：分支顺序正确（text 分支在通用 测试证据（来自该 PR 自身的 CI）无人值守 CI 运行——不执行 PR 代码，以下为通过 API 读取的该 commit 上的 CI 结果。主单测套件仍在运行，precheck 全绿，暂无失败；windows/macos/集成测试目前为 skipped，尚无结论。CI 结束后 finalize 工作流会刷新上方表格。 一个值得点明的缺口：单测把提取逻辑固定为 issue 中给出的报文形态，但这些形态本身是作者的真机验证说法——CI 无法证明真实钉钉回调确实长这样。沙盒验证可以解决其中一半： 未验证：真实钉钉设备行为（CI 无凭据）；windows/macos 测试结论（目前 skipped）。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested fix for a verified defect; the one reservation is evidence, not code: the DingTalk payload shapes come from the author's device-verified claim, which no CI lane can confirm. Stepping back: the defect is real — I confirmed in the current code that plain-text, richText, and card quotes are silently dropped before reaching the agent, and the issue documents the shapes precisely. The fix does exactly what the problem asks: it corrects the field mapping inside the one function responsible for it, removes the comment that asserted the wrong behavior, and pins every shape with a test in the existing style. Nothing extra rode along. My independent proposal for this issue is indistinguishable from what the PR does, so there's no simpler path being missed. The recursive card walk is the only new logic and it's small, cycle-safe by construction (JSON-parsed input), and length-bounded downstream by the existing sanitizer. In six months this reads as a straightforward bugfix, not something to curse. The reason it's a 4 rather than a 5: green unit tests here prove the extraction matches the mocked shapes, not that live DingTalk delivers them — that half needs a maintainer with device credentials (or the sponsored Approval deferred until CI lands green on 中文说明置信度：4/5 —— 干净、最小化、测试充分的修复，缺陷已核实；唯一的保留意见在证据而非代码：钉钉报文形态来自作者的真机验证说法，任何 CI 通道都无法确认这一点。 整体来看：缺陷真实存在——我在当前代码中确认了纯文本、richText 和卡片引用在到达 agent 前会被静默丢弃，issue 也精确记录了报文形态。修复恰好做了问题所需要的事：在唯一负责的函数内修正字段映射、移除断言错误行为的注释、并用既有风格的测试固定每种形态。没有夹带任何额外改动。我对这个 issue 的独立方案与该 PR 完全一致，因此不存在被遗漏的更简路径。递归卡片遍历是唯一的新逻辑，它很小、由构造保证无环（输入经 JSON 解析），且长度由既有清洗器在下游兜底。六个月后再看，这是一个直白的 bugfix，不会让人抱怨。 给 4 而不是 5 的原因：这里的绿色单测证明的是提取逻辑与模拟的形态一致，而不是真实钉钉确实下发这些形态——那一半需要有真机凭据的 maintainer 确认（或上文提到的 sponsored 批准暂缓，等待 CI 在 — Qwen Code · qwen3.8-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 — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const partType = part.type || 'text'; | ||
| if (partType === 'text' && part.text) { | ||
| parts.push(part.text); | ||
| const partType = (part.type || part.msgType || 'text').toLowerCase(); |
There was a problem hiding this comment.
[Suggestion] The richText loop still supports the legacy {type, text} part shape via the part.type || fallback, but no test pins that backward-compatible branch — the new quoted-richText tests use only the {msgType, content} shape. Dropping part.type || (mutating to (part.msgType || 'text')) leaves the full package suite green (verified: 320/320 pass after the mutation), so a future refactor could silently break legacy-shape quotes — a {type:'picture'} part losing its [image] placeholder, {type:'text'} text skipped — while the suite stays green.
Consider adding one legacy-shaped fixture to the new quoted message context block, e.g.
content: {
richText: [
{ type: 'text', text: 'legacy body' },
{ type: 'picture', downloadCode: 'c' },
],
},and asserting referencedText === 'legacy body[image]'.
中文说明
richText 循环目前仍通过 part.type || 回退兼容旧的 {type, text} 段形态，但没有任何测试固定这条向后兼容分支——新增的引用 richText 测试只用了 {msgType, content} 形态。删掉 part.type ||（突变为 (part.msgType || 'text')）后整个包的测试套件依然全绿（已验证：突变后 320/320 通过），因此未来的重构可能在测试全绿时悄悄破坏旧形态的引用——{type:'picture'} 段丢失 [image] 占位符、{type:'text'} 文本被跳过。
建议在新的 quoted message context 用例组补一个旧形态用例：
content: {
richText: [
{ type: 'text', text: 'legacy body' },
{ type: 'picture', downloadCode: 'c' },
],
},并断言 referencedText === 'legacy body[image]'。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| } | ||
| }; | ||
| if (Array.isArray(nodes)) walk(nodes); | ||
| return segments.join('\n'); |
There was a problem hiding this comment.
[Suggestion] collectCardText joins multiple TEXT segments with '\n', but the new interactiveCard test contains exactly one TEXT node, so this separator is unpinned — mutating segments.join('\n') to segments.join('') leaves the whole suite green (verified). Bot reply cards routinely contain several TEXT nodes; with the mutant they would merge into the quoted context as HelloHow can I help you today? — word boundaries across segments lost. Downstream sanitizeQuotedText folds '\n' to a space, so '\n' vs '' is the difference between a space and glued words.
Add a card fixture with two TEXT nodes and assert the joined output, e.g. referencedText === 'first line\nsecond line'.
中文说明
collectCardText 用 '\n' 连接多个 TEXT 段，但新的 interactiveCard 测试只有一个 TEXT 节点，因此该分隔符未被测试固定——把 segments.join('\n') 突变为 segments.join('') 后整个套件依然全绿（已验证）。bot 的卡片回复通常包含多个 TEXT 节点，突变后它们会在引用上下文中连成 HelloHow can I help you today?——段间词边界丢失。下游 sanitizeQuotedText 会把 '\n' 折成空格，所以 '\n' 与 '' 的差别就是有空格与词粘在一起。
补一个含两个 TEXT 节点的卡片用例，断言拼接结果，例如 referencedText === 'first line\nsecond line'。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (msgType === 'interactivecard') { | ||
| return this.collectCardText(content?.cardContent); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The empty/absent cardContent path of this new branch is untested, even though the comment this diff deletes explicitly stated DingTalk card quotes can arrive without content. Today collectCardText(undefined) degrades gracefully (the Array.isArray guard returns '', so referencedText becomes undefined). But remove that guard and walk(undefined) throws a TypeError inside onMessage, dropping the user's entire message — and no test distinguishes the guard being present from absent (verified: the suite stays green with the mutant).
Add one test: an interactiveCard quote with content: {}, asserting envelope.referencedText is undefined (and isReplyToBot per senderId).
中文说明
这个新分支的空/缺失 cardContent 路径没有测试覆盖，尽管本次 diff 删除的注释恰好明确说过钉钉卡片引用可能不带内容。目前 collectCardText(undefined) 会优雅降级（Array.isArray 守卫返回 ''，referencedText 变为 undefined）。但删掉该守卫后，walk(undefined) 会在 onMessage 内抛出 TypeError，导致整条用户消息被丢弃——而且没有任何测试能区分守卫存在与否（已验证：突变后套件依然全绿）。
补一个测试：interactiveCard 引用且 content: {}，断言 envelope.referencedText 为 undefined（且 isReplyToBot 按 senderId 判定）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Local verification — full DingTalk pipeline, two armsI built a real end-to-end environment for this instead of relying on the unit tests, because the claim under review is "the quoted text actually reaches the agent", and only the wire can answer that. What is real: a fake DingTalk platform (HTTPS on :443 impersonating Two arms, each with its own full
Result: the fix does what the issue says
Exactly the three-of-four breakage #9536 describes, and the reply the bot sends back over Two things to settle before merge
1. The per-segment
It bites the new segment shape too — an const partText = part.text ?? part.content;
if (partType === 'text' && partText) {
parts.push(partText);
} else if (partType === 'picture') {2. The PR no longer merges, and the conflict has two traps. Both hunks in
The resolution I ran everything against: private summarizeRepliedContent(replied: DingTalkRepliedMsg): string {
const rawMsgType = replied.msgType;
const msgType = rawMsgType?.toLowerCase();
const content = replied.content;
if (msgType === 'text' && content?.content?.trim()) return content.content.trim();
if (content?.text?.trim()) return content.text.trim();
if (content?.richText && Array.isArray(content.richText)) { /* segment loop */ }
if (msgType === 'interactivecard') return this.collectCardText(content?.cardContent);
if (msgType === 'chatrecord') { /* main's block, unchanged */ }
return mediaTypePlaceholder(rawMsgType, content?.fileName) ?? '';
}Non-blocking
Checked and cleared
Test runs
Tested on 🐧 Linux (Node v22.22.2) — the E2E above ran there, so the PR body's Linux 中文说明本地验证 —— 完整钉钉链路，双臂对照我没有只依赖单测，而是搭了一套真实的端到端环境：本 PR 要证明的是「引用文本真的进入了 agent 上下文」，这个问题只有在真实链路上才能回答。 真实的部分： 一个假的钉钉平台（HTTPS 监听 :443 冒充 两条臂各自完整 结论：修复确实做到了 issue 所述的效果
与 #9536 描述的「四种形态坏三种」完全一致，机器人经 合入前需要处理的两件事1. 逐段
新段形态同样中招：一个 const partText = part.text ?? part.content;
if (partType === 'text' && partText) {
parts.push(partText);
} else if (partType === 'picture') {2. PR 已经无法合入，而且冲突解决里有两个坑。
我全程验证所用的解法： private summarizeRepliedContent(replied: DingTalkRepliedMsg): string {
const rawMsgType = replied.msgType;
const msgType = rawMsgType?.toLowerCase();
const content = replied.content;
if (msgType === 'text' && content?.content?.trim()) return content.content.trim();
if (content?.text?.trim()) return content.text.trim();
if (content?.richText && Array.isArray(content.richText)) { /* 段循环 */ }
if (msgType === 'interactivecard') return this.collectCardText(content?.cardContent);
if (msgType === 'chatrecord') { /* main 的分支，保持不变 */ }
return mediaTypePlaceholder(rawMsgType, content?.fileName) ?? '';
}非阻塞
已核查并排除
测试执行
已在 🐧 Linux（Node v22.22.2）验证 —— 上述 E2E 就跑在这上面，PR 描述里的 Linux 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution for PR #9537Root cause
Semantic conflict — resolved codeBoth sides changed const msgType = replied.msgType?.toLowerCase();
// 1. text quotes via content.content (PR)
// 2. content.text (markdown etc.) (both)
// 3. richText incl. {msgType, content} aliases (PR)
if (msgType === 'chatrecord') { /* main's formatChatRecord */ }
if (msgType === 'interactivecard') return this.collectCardText(content?.cardContent); // PR
return mediaTypePlaceholder(msgType, content?.fileName) ?? ''; // main helper replaces PR's inline switchInterfaces: kept PR's Load-bearing points
Not verified hereNo build/tests were run (per this command's scope). Both suites exercise the merged function; main's 中文说明根因：main 上 #9339（转发聊天记录）重写了本 PR 同样重写的 解决：合并后函数依次处理 content.content 文本 → content.text → richText（含 关键点：main 原本不大写化而比较 未验证：本步骤不运行构建或测试，由 PR 自身 CI 覆盖。唯一超出双方原意的变化：引用路径 msgType 匹配变为不区分大小写（更宽松，符合 PR 意图）。 |
4 participants
What this PR does
Fixes the DingTalk channel's quoted-message extraction so that replied message content actually reaches the agent. The adapter now branches on the quoted message type the way DingTalk delivers it: plain-text quotes read their body from the correct nested field, quoted richText accepts the segment shape used inside replies (in addition to the top-level shape), and quoted interactive cards — the card messages the bot itself sends — have their visible text collected from the nested card element tree. An outdated comment claiming card replies carry no content is removed.
Why it's needed
When a user quotes a previous message and replies to the bot, the quoted text is meant to be prepended to the prompt as reply context. Today only markdown quotes survive; plain-text quotes (the most common case), quoted richText, and quoted bot cards are silently dropped, so the agent answers without seeing what the user is referring to. The field mapping now matches device-verified callback payloads.
Reviewer Test Plan
How to verify
Unit tests simulate inbound DingTalk reply callbacks for each quoted shape and assert the envelope carries the extracted quote:
Expected: the new
DingtalkChannel quoted message contextblock passes, covering text (content.content), markdown (content.text), msgType-shaped richText segments with an image placeholder, and interactiveCard tree traversal (includingisReplyToBotdetection when the quoted card was sent by the bot).Real-device check (optional): in a DingTalk group/DM, quote one of the bot's card replies (or another user's text message) and send a reply to the bot; the prompt shown in debug logging should start with
[Replying to: "..."].Evidence (Before & After)
N/A (non-UI change; covered by unit tests above).
Tested on
Environment (optional)
Unit tests only (
vitest), no live DingTalk credentials.Risk & Scope
sanitizeQuotedTextpath in ChannelBase (500-code-point cap, control/bidi/delimiter stripping), so sanitization guarantees are unchanged.[Replying to: "..."]format are ChannelBase-wide contracts and intentionally untouched; live device capture was not re-run in this PR (payload shapes follow device-verified captures of real DingTalk reply callbacks).Linked Issues
Fixes #9536
中文说明
这个 PR 做了什么
修复钉钉通道的引用消息提取，使被引用消息的内容能真正进入 agent 上下文。适配器现在按钉钉实际下发的引用消息类型分支处理：纯文本引用从正确的嵌套字段取正文；引用 richText 同时兼容回复内部使用的段形态（以及顶层消息的段形态）；引用的 interactiveCard（即 bot 自己发出的卡片消息）从嵌套卡片元素树中收集可见文本。同时移除了一条声称卡片引用不带内容的过时注释。
为什么需要
当用户引用一条消息并回复 bot 时，被引用的文本本应作为回复上下文拼到提示词前面。但目前只有 markdown 引用能保留；纯文本引用（最常见的场景）、引用的 richText、以及引用的 bot 卡片都会被静默丢弃，导致 agent 在看不到用户所指内容的情况下作答。本次修复使字段映射与真机验证过的回调报文一致。
评审测试计划
如何验证
单元测试模拟四种引用形态的钉钉回复回调，断言 envelope 携带提取出的引用文本：
预期：新增的
DingtalkChannel quoted message context用例组通过，覆盖 text（content.content）、markdown（content.text）、msgType 形态 richText 段（含图片占位符）、interactiveCard 元素树遍历（引用 bot 卡片时isReplyToBot为 true）。真机验证（可选）：在钉钉群/单聊中引用 bot 的一条卡片回复（或其他用户的文本消息）并回复 bot，debug 日志中的提示词应以
[Replying to: "..."]开头。前后证据
N/A（非 UI 变更，由上述单元测试覆盖）。
测试平台
macOS 已测；Windows/Linux 未测。
环境
仅单元测试（vitest），未使用真实钉钉凭据。
风险与范围
sanitizeQuotedText清洗路径的内容（500 码点上限、控制字符/bidi/分隔符剥离），清洗保证不变。[Replying to: "..."]格式是 ChannelBase 跨通道契约，有意不动；本 PR 未重新真机抓包（报文形态遵循真机验证过的钉钉回复回调捕获）。关联 Issue
Fixes #9536