fix(dingtalk): attach media from quoted messages - #9347
Conversation
E2E Test ReportEnvironment
Live payload verification
Post-change verification
The regressions cover successful picture, file, audio, and video attachments; outer-question and placeholder preservation; exact file bytes and metadata; missing download codes; token failure; media-API failure; and failure-safe cleanup of test-created temporary directories. Known boundaries
|
|
Thanks for the PR! Template looks good ✓ — all sections filled in, bilingual, with a concrete reviewer test plan. Problem: this is an observed gap, not a theoretical one. DingTalk callbacks for replies to pictures/files carry Direction: aligned. The DingTalk channel already downloads top-level media through an authenticated flow (with the 50 MB cap and 30 s timeout added in #7361); quoted media is the same capability applied to the message being replied to. CHANGELOG shows this area is actively maintained (#7361, #7698, #8639). Size: not a core-module path ( Approach: scope feels right. The change reuses the existing Risk: no elevated risk signals — no high-risk paths matched the revert-history patterns. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各部分齐全、中英双语，且提供了具体的 reviewer 测试计划。 问题：这是已观测到的缺口，不是理论性问题。用户回复图片/文件时，钉钉回调会携带 方向：对齐。钉钉 Channel 已经通过鉴权流程下载顶层媒体（#7361 加入了 50 MB 上限和 30 秒超时）；引用媒体只是把同样的能力应用到被回复的消息上。CHANGELOG 显示该模块一直在活跃维护（#7361、#7698、#8639）。 规模：不属于核心模块路径（ 方案：范围合理。改动复用了现有 风险：无升级风险信号 —— 未命中 revert 历史高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewClean, minimal change — exactly the approach a reviewer would hope for. Things I verified while reading:
No blockers, no convention issues. Testing evidence (the PR's own CI, read via API — PR code not executed locally)The Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the remaining gap: 中文说明代码审查改动干净、最小化，正是 reviewer 期望的做法： 阅读时核实过的点：
无阻塞问题，无规范问题。 测试证据（通过 API 读取 PR 自身 CI —— 未在本地执行 PR 代码）该 commit 的 （CI 明细见上方表格。） 沙箱验证可以补齐剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal change that reuses the existing media path end to end; the only reservation is that live DingTalk behavior rests on the author's E2E capture rather than something CI can reproduce. This is what a good channel fix looks like. The problem is real and observed — DingTalk hands the adapter a My independent read of how this should be built matched what the PR does; I didn't find a simpler path, and there's nothing in the diff beyond what the goal needs. CI is green for this commit (the skipped platform jobs are the workflow's PR-classification gating, not failures). The two things a maintainer may still want before merge are the ones CI can't provide here: a Approving. ✅ 中文说明置信度：4/5 —— 改动干净、最小，端到端复用了现有媒体链路；唯一的保留意见是真实钉钉行为依赖作者的 E2E 捕获，CI 无法复现。 这是一个规范的 channel 修复。问题真实且已被观测到：钉钉回调会把被回复消息的 我独立设想的实现方式与 PR 一致，没有找到更简的路径，diff 中也没有超出目标所需的改动。该 commit 的 CI 为绿（被跳过的平台任务是工作流的 PR 分类机制，不是失败）。合并前维护者可能还想要两样 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. ✅
| if (quoted.media) { | ||
| await this.attachMedia( | ||
| envelope, | ||
| quoted.media.downloadCode, |
There was a problem hiding this comment.
[Critical] R1-1: The quoted-media path routes the user's typed text through attachMedia's placeholder-cleanup — a reply whose entire text is exactly (audio), (video), or (file: <quoted fileName>) is silently erased before handleInbound. That cleanup (lines 1611-1618) was written for the direct-media path, where extractContent itself generates those round-bracket placeholders; on this new path envelope.text is the user's actual reply text, which this diff makes newly reachable by the comparison. — Failure scenario: a user replies to a quoted audio/video/file message that carries a downloadCode with the exact text (audio) / (video) / (file: report.pdf) (report.pdf matching the quoted file's name; e.g. group reply @Bot (audio) — the mention is stripped earlier, leaving the exact match). The quoted media attaches successfully, the cleanup matches, envelope.text becomes '', and the agent receives the attachment with an empty prompt — the user's instruction is silently lost. Secondary variant: the own file download fails (placeholder (file: a.pdf) kept) while a quoted file named a.pdf attaches successfully — the quoted call then erases the direct file's leftover placeholder.
Witness (probe through the real adapter):
PR code: PROBE-F1a: text="" attachments=["audio"] <- user text blanked
fix-flip (cleanup skipped for quoted call): PROBE-F1a: text="(audio)" attachments=["audio"]
(the (file: report.pdf) variant flips the same way; the file's suite is 134/134 green with the fix applied)
Suggested fix (spans two sites, so no one-click suggestion): give attachMedia a parameter such as cleanPlaceholderText?: string, pass it only from the direct-media call site (the placeholder extractContent produced), and erase only when it is provided and equals envelope.text. The quoted call passes nothing.
中文说明
[Critical] R1-1：引用媒体路径会把用户输入的文本送进 attachMedia 的占位符清理逻辑 —— 若回复的完整文本恰好是 (audio)、(video) 或 (file: <被引用文件名>)，会在 handleInbound 之前被静默清空。该清理逻辑（1611-1618 行）原本是为直发媒体路径编写的 —— 那里 extractContent 自己生成这些圆括号占位符；而在这条新路径上，envelope.text 是用户真实输入的文本，本 diff 使其首次能被该比较命中。 — 失败场景：用户回复一条带 downloadCode 的音频/视频/文件引用消息，文本恰好是 (audio) / (video) / (file: report.pdf)（report.pdf 与被引用文件名一致；例如群聊中 @Bot (audio) —— @ 提及被去除后剩下精确匹配）。引用媒体下载并附加成功后，清理逻辑命中，envelope.text 被置为 ''，Agent 只收到附件和空 prompt —— 用户的指令被静默丢弃。次要变体：自身文件下载失败（保留占位符 (file: a.pdf)）而引用的同名文件 a.pdf 附加成功时，引用调用会清掉直发文件遗留的占位符。
见证（通过真实适配器的探针）：
PR 代码： PROBE-F1a: text="" attachments=["audio"] <- 用户文本被清空
修复翻转（引用调用跳过清理）： PROBE-F1a: text="(audio)" attachments=["audio"]
（(file: report.pdf) 变体同样翻转；应用修复后该文件测试 134/134 全绿）
建议修复（跨两处，故不提供一键 suggestion）：给 attachMedia 增加 cleanPlaceholderText?: string 参数，仅由直发媒体调用点传入（即 extractContent 生成的占位符），且仅当该参数提供且与 envelope.text 相等时才清空；引用调用不传。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (quoted.media) { | ||
| await this.attachMedia( | ||
| envelope, | ||
| quoted.media.downloadCode, | ||
| quoted.media.mediaType, | ||
| quoted.media.fileName, | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Critical] R1-2: The new quoted-media attach makes text-only replies reach attachMedia's unguarded sync throw sites (mkdirSync/writeFileSync at ~1604-1608 and basename(fileName || '')). Any throw rejects processMessage, whose .catch sends the generic error reply and never calls handleInbound — the user's reply text is dropped, and the msgId was already recorded in seenMessages, so a DingTalk retry is deduped and the prompt is permanently lost. This contradicts the graceful-degradation contract this PR's own tests assert for download failures ("keeps processing the prompt when a quoted-media download fails"). Before this diff, a text-only reply quoting media had zero filesystem dependency and always dispatched. — Failure scenario: a user replies with plain text to a quoted file/audio/video message and the temp write fails — disk quota/ENOSPC on the up-to-50 MB write, a quoted fileName longer than 255 bytes (basename() does not truncate → ENAMETOOLONG), or a truthy non-string fileName (basename TypeError) → the user's text never reaches the agent and they get "Sorry, something went wrong processing your message."
Witness (probe through the real adapter, download mocked to succeed):
fileName='a'.repeat(300) -> stderr "ENAMETOOLONG: name too long, open '/tmp/channel-files/...'"
handleInbound calls: 0, generic error sent
fileName=12345 -> stderr "TypeError [ERR_INVALID_ARG_TYPE]", handleInbound calls: 0
control (normal fileName) -> handleInbound calls: 1
fix-flip (try/catch around the fs block) -> both failure classes: handleInbound calls: 1, no error reply
The one-click suggestion below contains the throw at the new call site so the text is always delivered (the disk write still fails — the attachment is just skipped, matching the download-failure degradation). Alternatively, guard the mkdirSync/writeFileSync/basename sites inside attachMedia, which also protects the pre-existing direct-media path.
| if (quoted.media) { | |
| await this.attachMedia( | |
| envelope, | |
| quoted.media.downloadCode, | |
| quoted.media.mediaType, | |
| quoted.media.fileName, | |
| ); | |
| } | |
| if (quoted.media) { | |
| try { | |
| await this.attachMedia( | |
| envelope, | |
| quoted.media.downloadCode, | |
| quoted.media.mediaType, | |
| quoted.media.fileName, | |
| ); | |
| } catch (error) { | |
| process.stderr.write( | |
| `[DingTalk] Cannot attach quoted media: ${String(error)}\n`, | |
| ); | |
| } | |
| } |
中文说明
[Critical] R1-2：新增的引用媒体附加使纯文本回复也能到达 attachMedia 中未加保护的同步抛错点（约 1604-1608 行的 mkdirSync/writeFileSync 以及 basename(fileName || '')）。任何抛错都会使 processMessage reject，而其 .catch 只发送通用错误回复、从不调用 handleInbound —— 用户的回复文本被丢弃，且 msgId 已先写入 seenMessages，钉钉重试会被去重，prompt 永久丢失。这与本 PR 自己的测试所断言的下载失败优雅降级契约（"keeps processing the prompt when a quoted-media download fails"）相矛盾。此 diff 之前，引用媒体的纯文本回复没有任何文件系统依赖，总是能正常投递。 — 失败场景：用户用纯文本回复一条引用的文件/音频/视频消息，而临时文件写入失败 —— 最大 50 MB 写入遇到磁盘配额/ENOSPC、被引用文件名超过 255 字节（basename() 不截断 → ENAMETOOLONG）、或 fileName 为真值非字符串（basename TypeError）→ 用户的文本永远到不了 Agent，只收到 "Sorry, something went wrong processing your message."。
见证（通过真实适配器的探针，下载 mock 为成功）：
fileName='a'.repeat(300) -> stderr "ENAMETOOLONG: name too long, open '/tmp/channel-files/...'"
handleInbound 调用次数：0，发送了通用错误
fileName=12345 -> stderr "TypeError [ERR_INVALID_ARG_TYPE]"，handleInbound 调用次数：0
对照（正常 fileName） -> handleInbound 调用次数：1
修复翻转（fs 块外加 try/catch）-> 两类失败均变为：handleInbound 调用次数：1，无错误回复
下方一键 suggestion 在新调用点捕获抛错，保证文本始终投递（磁盘写入仍会失败 —— 只是跳过附件，与下载失败的降级行为一致）。另一种做法是在 attachMedia 内部保护 mkdirSync/writeFileSync/basename 各处，这样同时保护了既有的直发媒体路径。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| let mediaType: 'image' | 'file' | 'audio' | 'video' | undefined; | ||
| switch (replied.msgType) { |
There was a problem hiding this comment.
[Suggestion] R1-3: This switch re-implements the DingTalk msgType→mediaType mapping (picture→image; file/audio/video passthrough) that already exists in extractContent (~1516-1551) of the same file; summarizeRepliedContent carries a third enumeration of the same four types. — Concrete cost: any future change to DingTalk's downloadable types (a new type, a rename, an altered content shape) must be applied in lockstep in both places; missing one silently disables download for that type on only one of the two paths (direct media vs quoted media) — a drift that is hard to notice because both paths behave identically today. Note: reusing extractContent wholesale is a near-miss — it also collects image download codes from richText parts, which this switch deliberately excludes for quotes; the shareable unit is the type mapping itself.
Suggested fix:
private mediaTypeFromMsgType(
msgType: string | undefined,
): 'image' | 'file' | 'audio' | 'video' | undefined {
if (msgType === 'picture') return 'image';
if (msgType === 'file' || msgType === 'audio' || msgType === 'video')
return msgType;
return undefined;
}called from both extractContent (replacing the per-branch mediaType: literals) and this switch.
中文说明
[Suggestion] R1-3：这个 switch 重新实现了钉钉 msgType→mediaType 的映射（picture→image；file/audio/video 直通），而同一文件的 extractContent（约 1516-1551 行）中已存在同样的映射；summarizeRepliedContent 还有第三处对相同四种类型的枚举。 — 具体代价：未来钉钉可下载类型的任何变化（新增类型、改名、content 结构调整）都必须在两处同步修改；漏改其一会让该类型在其中一条路径（直发媒体 vs 引用媒体）上静默停止下载 —— 由于两条路径目前行为完全一致，这种漂移很难被发现。注意：整体复用 extractContent 并不可行 —— 它还会收集 richText 各 part 中的图片下载码，而新 switch 对引用消息刻意排除了这种情况；可共享的部分是类型映射本身。
建议修复：
private mediaTypeFromMsgType(
msgType: string | undefined,
): 'image' | 'file' | 'audio' | 'video' | undefined {
if (msgType === 'picture') return 'image';
if (msgType === 'file' || msgType === 'audio' || msgType === 'video')
return msgType;
return undefined;
}由 extractContent（替换各分支的 mediaType: 字面量）和这个 switch 共同调用。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| return { | ||
| referencedText: text || undefined, | ||
| isReplyToBot, | ||
| ...(downloadCode && mediaType |
There was a problem hiding this comment.
[Suggestion] R1-4: The mediaType half of this downloadCode && mediaType guard has no test — the mutant dropping it (...(downloadCode && mediaType → ...(downloadCode) survives the PR's entire quoted-media suite. — Failure scenario: under that mutant, a quoted { msgType: 'richText', content: { downloadCode: 'x' } } payload (a real shape in this file's domain — summarizeRepliedContent handles richText) calls attachMedia with mediaType === undefined, taking the file branch and emitting a dingtalk_undefined_<ts> temp file plus an attachment with type: undefined into the prompt.
Witness (mutation executed):
mutant: all 8 original quoted-media tests pass (only the added probe fails, 11/12)
probe under mutant: attachments=[{type: undefined, fileName: "dingtalk_undefined_1787023451837", ...}] + download fetches
correct code: fetchCalls=0, attachments=undefined
(The mutant is also caught by tsc — media.mediaType is a required member of the return type — since vitest transpiles without type-checking; the proposed test is still the runtime-layer guard.)
Suggested fix — add one case (e.g. beside the missing-code it.each):
it('does not download a quoted message with an unmapped msgType even when it carries a downloadCode', async () => {
const fetchSpy = vi
.spyOn(globalThis, 'fetch')
.mockRejectedValue(new Error('unexpected download'));
const channel = createChannel();
replyToMedia(channel, 'richText', { downloadCode: 'quoted-rt-code' });
await vi.waitFor(() => {
expect(channel.handleInbound).toHaveBeenCalledOnce();
});
expect(fetchSpy).not.toHaveBeenCalled();
expect(
vi.mocked(channel.handleInbound).mock.calls[0]![0],
).not.toHaveProperty('attachments');
});中文说明
[Suggestion] R1-4：downloadCode && mediaType 这个守卫中 mediaType 的一半没有测试覆盖 —— 去掉它的变异体（...(downloadCode && mediaType → ...(downloadCode）能在 PR 的全部引用媒体测试中存活。 — 失败场景：在该变异体下，引用消息为 { msgType: 'richText', content: { downloadCode: 'x' } } 的载荷（这是本文件领域内的真实形态 —— summarizeRepliedContent 就处理 richText）会以 mediaType === undefined 调用 attachMedia，走入 file 分支，生成 dingtalk_undefined_<ts> 临时文件，并把 type: undefined 的附件塞进 prompt。
见证（实际执行的变异）：
变异体：原有 8 个引用媒体测试全部通过（仅新增探针失败，11/12）
变异体下探针：attachments=[{type: undefined, fileName: "dingtalk_undefined_1787023451837", ...}] + 下载请求
正确代码： fetchCalls=0, attachments=undefined
（该变异体也会被 tsc 捕获 —— media.mediaType 是返回类型的必需成员 —— 因为 vitest 转译时不做类型检查；但建议的测试仍是运行时层面的守卫。）
建议修复 —— 增加一个用例（例如放在缺码 it.each 旁）：见上方英文侧代码块。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (quoted.media) { | ||
| await this.attachMedia( | ||
| envelope, |
There was a problem hiding this comment.
[Suggestion] R1-5: No test exercises a message that carries its OWN media (content.downloadCodes) AND quotes media (quoted.media) — the only combination where both attachMedia calls run in sequence on one envelope. — Failure scenario: a user sends a picture while replying to a file message; expected effect is handleInbound receiving both attachments (direct image base64 plus quoted file filePath). This works today only because attachMedia appends via [...(envelope.attachments || []), …]. If a future edit gates one block on the other (else if) or replaces the append with an assignment, one media item is silently dropped — and every current test stays green, since each half is only ever tested alone.
Suggested fix — add one test: downstream message with msgtype: 'picture' + own content.downloadCode, whose repliedMsg is a 'file' with a downloadCode; assert handleInbound receives attachments of length 2 in order (image entry first, then the quoted file entry). Note: the same compound shape is also the entry point for the image-drop behavior flagged separately in the terminal report.
中文说明
[Suggestion] R1-5：没有测试覆盖消息同时携带自身媒体（content.downloadCodes）与引用媒体（quoted.media）的场景 —— 这是唯一会让两次 attachMedia 调用先后作用于同一个 envelope 的组合。 — 失败场景：用户发送一张图片的同时回复一条文件消息；预期 handleInbound 收到两个附件（直发图片的 base64 加引用文件的 filePath）。目前之所以正常，仅因为 attachMedia 通过 [...(envelope.attachments || []), …] 追加。若未来某次修改把一个块变成以另一个为条件（else if），或把追加换成赋值，就会静默丢弃其中一项媒体 —— 而所有现有测试仍为绿色，因为两半从未被一起测试过。
建议修复 —— 增加一个测试：下游消息 msgtype: 'picture' 且自带 content.downloadCode，其 repliedMsg 为带 downloadCode 的 'file'；断言 handleInbound 收到长度为 2 的 attachments 且顺序正确（先是图片项，后是引用文件项）。注意：该组合同样是终端报告中单独指出的图片丢弃行为的入口。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
@qwen-code /takeover stop |
qwen-code-dev-bot
removed
the
autofix/takeover
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放：autofix 循环不再介入此 PR（在飞的一轮如有，将完成其有界工作）。重新打上 |
R1-1: `attachMedia`'s placeholder cleanup was written for the direct-media path, where `extractContent` generates `(audio)` / `(video)` / `(file: name)` itself. This PR made the quoted-media path reach it, and there `envelope.text` is the user's own reply — so a reply reading exactly like one of those placeholders was blanked and the agent got an attachment with no prompt. A group `@Bot (audio)` arrives here as exactly `(audio)`, the mention having been stripped upstream. `attachMedia` now takes the placeholder to erase as a parameter; only the direct-media call site passes one. R1-2: the same path newly routes text-only replies through the unguarded `mkdirSync`/`writeFileSync`/`basename` block. Those are synchronous throw sites — ENOSPC on a write of up to 50 MB, ENAMETOOLONG from a quoted fileName over 255 bytes (`basename` does not truncate), a TypeError from a truthy non-string fileName. An escape rejects `processMessage`, whose catch sends the generic error reply and never calls `handleInbound`; the msgId is already in `seenMessages`, so DingTalk's retry is deduped and the prompt is lost for good. The block now degrades the way a failed download already does: log, skip the attachment, deliver the text. This also covers the pre-existing direct-media path. Verified: dingtalk 310/310. Both mutation-checked — restoring the caller-blind cleanup fails 3 tests, letting the fs block throw fails 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
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.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| switch (replied.msgType) { | ||
| case 'picture': | ||
| mediaType = 'image'; | ||
| break; |
There was a problem hiding this comment.
[Suggestion] R1-3 (round 1, still stands): This switch still re-implements the DingTalk msgType→mediaType mapping (picture→image; file/audio/video passthrough) that already exists in extractContent (~1486-1557) of the same file; summarizeRepliedContent carries a third enumeration of the same four types. The round-1 suggested fix (a shared helper) was not applied in the follow-up round. — Failure scenario: any future change to DingTalk's downloadable types (a new type, a rename, an altered content shape) must be applied in lockstep in both places; missing one silently disables download for that type on only one of the two paths (direct vs quoted media) — a drift that is hard to notice because both paths behave identically today. Note: reusing extractContent wholesale is a near-miss — it also collects richText part codes, which this switch deliberately excludes for quotes; the shareable unit is the type mapping itself.
Suggested fix:
private mediaTypeFromMsgType(
msgType: string | undefined,
): 'image' | 'file' | 'audio' | 'video' | undefined {
if (msgType === 'picture') return 'image';
if (msgType === 'file' || msgType === 'audio' || msgType === 'video')
return msgType;
return undefined;
}called from both extractContent (replacing the per-branch mediaType: literals) and this switch.
中文说明
[Suggestion] R1-3（第 1 轮发现，仍然存在）：这个 switch 仍然重复实现了钉钉 msgType→mediaType 的映射（picture→image；file/audio/video 直通），同一文件的 extractContent（约 1486-1557 行）中已有同样的映射；summarizeRepliedContent 还有第三处对相同四种类型的枚举。第 1 轮建议的修复（共享辅助函数）未在后续轮次中落实。 — 失败场景：未来钉钉可下载类型的任何变化（新增类型、改名、content 结构调整）都必须在两处同步修改；漏改其一会让该类型在其中一条路径（直发 vs 引用媒体）上静默停止下载 —— 由于两条路径目前行为完全一致，这种漂移很难被发现。注意：整体复用 extractContent 并不可行 —— 它还会收集 richText 各 part 中的下载码，而这个 switch 对引用消息刻意排除了这种情况；可共享的部分是类型映射本身。
建议修复：提取 mediaTypeFromMsgType(msgType) 私有辅助函数（见上方英文侧代码），由 extractContent（替换各分支的 mediaType: 字面量）和这个 switch 共同调用。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| return { | ||
| referencedText: text || undefined, | ||
| isReplyToBot, | ||
| ...(downloadCode && mediaType |
There was a problem hiding this comment.
[Suggestion] R1-4 (round 1, still stands): The mediaType half of this downloadCode && mediaType guard still has no test — the mutant dropping it (...(downloadCode && mediaType → ...(downloadCode) survives the PR's entire quoted-media suite, and no test added since exercises an unmapped msgType carrying a downloadCode. — Failure scenario: under that mutant, a quoted { msgType: 'richText', content: { downloadCode: 'x' } } payload (a real shape in this file's domain — summarizeRepliedContent handles richText) calls attachMedia with mediaType === undefined, taking the file branch and emitting a dingtalk_undefined_<ts> temp file plus an attachment with type: undefined into the prompt.
Witness (mutation executed in round 1):
mutant: all quoted-media tests pass (only the added probe fails, 11/12)
probe under mutant: attachments=[{type: undefined, fileName: "dingtalk_undefined_…"}] + download fetches
correct code: fetchCalls=0, attachments=undefined
(The mutant is also caught by tsc — media.mediaType is a required member of the return type — but vitest transpiles without type-checking; the proposed test is the runtime-layer guard.)
Suggested fix — add one case beside the missing-code tests: reply to an unmapped msgType (e.g. richText) that carries a downloadCode, assert no fetch happens and handleInbound receives no attachments.
中文说明
[Suggestion] R1-4（第 1 轮发现，仍然存在）：downloadCode && mediaType 这个守卫中 mediaType 的一半仍然没有测试覆盖 —— 去掉它的变异体（...(downloadCode && mediaType → ...(downloadCode）能在 PR 的全部引用媒体测试中存活，且此后新增的测试也没有覆盖携带 downloadCode 的未映射 msgType。 — 失败场景：在该变异体下，引用消息为 { msgType: 'richText', content: { downloadCode: 'x' } } 的载荷（这是本文件领域内的真实形态 —— summarizeRepliedContent 就处理 richText）会以 mediaType === undefined 调用 attachMedia，走入 file 分支，生成 dingtalk_undefined_<ts> 临时文件，并把 type: undefined 的附件塞进 prompt。
见证（第 1 轮实际执行的变异）：
变异体：原有引用媒体测试全部通过（仅新增探针失败，11/12）
变异体下探针：attachments=[{type: undefined, fileName: "dingtalk_undefined_…"}] + 下载请求
正确代码： fetchCalls=0, attachments=undefined
（该变异体也会被 tsc 捕获 —— media.mediaType 是返回类型的必需成员 —— 但 vitest 转译时不做类型检查；建议的测试是运行时层面的守卫。）
建议修复 —— 在缺码测试旁增加一个用例：引用携带 downloadCode 的未映射 msgType（如 richText），断言不发生 fetch 且 handleInbound 不收到 attachments。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| if (quoted.media) { | ||
| await this.attachMedia( | ||
| envelope, |
There was a problem hiding this comment.
[Suggestion] R1-5 (round 1, still stands — re-confirmed by executed mutation this round): No test exercises a message that carries its OWN media (content.downloadCodes) AND quotes media (quoted.media) — the only combination where both attachMedia calls run in sequence on one envelope. — Failure scenario (verified this round): Mutant A — image-branch append changed to plain assignment envelope.attachments = [item] — the full adapter suite stays green (135/135) while a pic+pic message loses the user's own image (the quoted attachment overwrites it). Mutant B — file-branch append changed to assignment — the entire dingtalk package suite stays green (310/310) while a pic+file message drops the direct image when the quoted file attaches second.
Witness (probe executed this round):
unmodified: PROBE pic+pic attachments: [{image own},{image quoted}] / pic+file: [{image},{file filePath}]
mutant A: expected [...] to have a length of 2 but got 1 (suite 135/135 green)
mutant B: expected [...] to have a length of 2 but got 1 (suite 310/310 green)
Suggested fix — add one test: downstream message with msgtype: 'picture' + own content.downloadCode, whose repliedMsg is a 'file' with a downloadCode; assert handleInbound receives attachments of length 2 in order (image entry first, then the quoted file entry).
中文说明
[Suggestion] R1-5（第 1 轮发现，仍然存在 —— 本轮通过实际变异重新确认）：没有测试覆盖消息同时携带自身媒体（content.downloadCodes）与引用媒体（quoted.media）的场景 —— 这是唯一会让两次 attachMedia 调用先后作用于同一个 envelope 的组合。 — 失败场景（本轮已验证）：变异体 A —— 把 image 分支的追加改为直接赋值 envelope.attachments = [item] —— 整个适配器测试套件仍全绿（135/135），但 图+图 消息会丢失用户自己的图片（被引用附件覆盖）。变异体 B —— 把 file 分支的追加改为赋值 —— 整个 dingtalk 包测试套件仍全绿（310/310），但 图+文件 消息会在引用文件附加时丢失直发图片。
见证（本轮实际执行的探针）：
未修改：PROBE pic+pic attachments: [{自身图},{引用图}] / pic+file: [{图},{文件 filePath}]
变异体 A：expected [...] to have a length of 2 but got 1（套件 135/135 绿）
变异体 B：expected [...] to have a length of 2 but got 1（套件 310/310 绿）
建议修复 —— 增加一个测试：下游消息 msgtype: 'picture' 且自带 content.downloadCode，其 repliedMsg 为带 downloadCode 的 'file'；断言 handleInbound 收到长度为 2 的 attachments 且顺序正确（先是图片项，后是引用文件项）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| content.mediaType === 'file' | ||
| ? `(file: ${content.fileName || 'file'})` | ||
| : `(${content.mediaType})`, |
There was a problem hiding this comment.
[Suggestion] The placeholder format now has two sources of truth: extractContent (~1486-1557) generates it into envelope.text, and this call site independently re-derives the same format string to feed the new cleanPlaceholderText gate — coupled only by the prose JSDoc on attachMedia. — Failure scenario: a future change to extractContent's placeholder output (a reworded (file: …) format, or a new media type whose placeholder doesn't match (${mediaType})) makes the comparison silently stop matching: nothing is logged, and direct file/audio/video messages deliver the literal placeholder text to the agent next to the attachment. No test pins the direct-path erasure (see the companion comment), so the drift ships silently.
Suggested fix: have extractContent return the placeholder it generated (e.g. a placeholder?: string field set for pure-media messages — for audio only when recognition is absent) and pass content.placeholder here instead of re-deriving the literals.
中文说明
[Suggestion] 占位符格式现在有两处定义：extractContent（约 1486-1557 行）把它生成到 envelope.text，而这个调用点又独立地重新推导出相同的格式字符串，作为新的 cleanPlaceholderText 判定依据 —— 两者仅靠 attachMedia 上的文字注释维系。 — 失败场景：未来 extractContent 的占位符输出一旦变化（改写 (file: …) 格式，或新增占位符不符合 (${mediaType}) 的媒体类型），这里的比较会静默失配：没有任何日志，直发的文件/音频/视频消息会把字面占位符文本连同附件一起交给 Agent。由于直发路径的清除行为没有测试固定（见相邻评论），这种漂移会静默上线。
建议修复：让 extractContent 返回它生成的占位符（例如增加 placeholder?: string 字段，仅对纯媒体消息设置 —— 音频仅在无 recognition 时设置），这里改为传入 content.placeholder，不再重新推导字面量。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| content.mediaType === 'file' | ||
| ? `(file: ${content.fileName || 'file'})` | ||
| : `(${content.mediaType})`, |
There was a problem hiding this comment.
[Suggestion] The positive path of the new cleanPlaceholderText gate has zero test coverage: the new tests pin only the negative side (quoted path must not clean), and nothing anywhere sends a direct (non-reply) audio/video/file message through onMessage — the pre-existing inbound-media tests call attachMedia only with 'image', which returns before the cleaning branch. — Failure scenario: the one-line mutant deleting this ternary argument (passing undefined) leaves every test green while every direct voice-note/video/file message delivers (audio) / (video) / (file: name) as literal prompt text to the agent — the exact pollution the cleaning branch exists to prevent.
Witness (probe executed this round):
SHIPPED: probe envelope.text: "" (placeholder cleaned)
MUTANT (ternary argument removed): suite 135/135 green; probe: AssertionError: expected '(audio)' to be ''
Suggested fix — add one direct-media test mirroring the new quoted-media helpers: send a non-reply message whose own content carries downloadCode + msgType: 'audio' (or 'file'), and assert handleInbound receives text: '' plus the attachment.
中文说明
[Suggestion] 新的 cleanPlaceholderText 判定的正向路径完全没有测试覆盖：新增测试只固定了负向（引用路径不得清除），而整个测试文件中没有任何用例通过 onMessage 发送直发（非回复）的音频/视频/文件消息 —— 既有的 inbound-media 测试只以 'image' 调用 attachMedia，在到达清除分支前就返回了。 — 失败场景：删掉这个三元参数（传 undefined）的单行变异体能让全部测试保持绿色，同时每条直发语音/视频/文件消息都会把 (audio) / (video) / (file: name) 作为字面 prompt 文本交给 Agent —— 正是清除分支要防止的污染。
见证（本轮实际执行的探针）：
现有代码：探针 envelope.text: ""（占位符被清除）
变异体（删除三元参数）：套件 135/135 绿；探针：AssertionError: expected '(audio)' to be ''
建议修复 —— 仿照新增的引用媒体辅助函数增加一个直发媒体测试：发送自带 downloadCode + msgType: 'audio'（或 'file'）的非回复消息，断言 handleInbound 收到 text: '' 和附件。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| ['an over-long file name', 'a'.repeat(300)], | ||
| ['a non-string file name', 12345 as unknown as string], |
There was a problem hiding this comment.
[Suggestion] Both R1-2 regression cases assert only that handleInbound still receives the user text; neither pins the attachment outcome that distinguishes the implementations — in particular the new typeof fileName === 'string' guard inside attachMedia is not pinned by any assertion. — Failure scenario: revert the guard to basename(fileName || '') while keeping the try/catch: for fileName: 12345, basename(12345) throws, the catch runs, handleInbound is still called with the text, and every test passes (135/135) — but shipped code delivers the attachment under a generated dingtalk_file_<ts> name while the mutant silently drops it. Likewise the over-long case never asserts attachments is absent, so a future truncate-instead-of-fail change would also pass.
Witness (probe executed this round):
SHIPPED: attachments=[{fileName: "dingtalk_file_…"}]
MUTANT (basename(fileName || ''), catch kept): attachments: undefined — suite 135/135 green
Suggested fix:
// non-string case: attachment delivered under generated name
expect(envelope.attachments?.[0]?.fileName).toMatch(/^dingtalk_file_/);
// over-long case: no attachment delivered
expect(envelope).not.toHaveProperty('attachments');中文说明
[Suggestion] R1-2 的两个回归用例都只断言 handleInbound 仍收到用户文本；都没有固定能区分两种实现的附件结果 —— 尤其 attachMedia 内新的 typeof fileName === 'string' 守卫没有任何断言覆盖。 — 失败场景：在保留 try/catch 的同时把守卫还原为 basename(fileName || '')：对 fileName: 12345，basename(12345) 抛错，catch 执行，handleInbound 仍带着文本被调用，全部测试通过（135/135）—— 但现有代码会以生成的 dingtalk_file_<ts> 名称投递附件，而变异体静默丢弃了它。同理，超长文件名用例也没有断言 attachments 不存在，未来改成截断而非失败同样能通过。
见证（本轮实际执行的探针）：
现有代码：attachments=[{fileName: "dingtalk_file_…"}]
变异体（basename(fileName || '')，保留 catch）：attachments: undefined —— 套件 135/135 绿
建议修复：见上方英文侧代码 —— 非字符串用例断言附件以生成名投递；超长用例断言无附件。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| /** | ||
| * `cleanPlaceholderText` is the placeholder `extractContent` generated for |
There was a problem hiding this comment.
[Suggestion] The new JSDoc block is stacked after attachMedia's existing JSDoc instead of merged into it — the only doubled /** */ block in the file. Editor/TypeScript tooling attaches only the LAST doc comment before a declaration to the symbol, so the original summary ("Download a media file and attach it to the envelope. Images → base64 in envelope; files → saved to temp dir with path in text.") is detached from the function. — Failure scenario: hovering attachMedia (or reading generated API docs) now shows only the R1-1 history note; the next reader loses the image-vs-file contract while the orphaned block still looks like live documentation. Verified via the TypeScript compiler API: symbol.getDocumentationComment() returns only the last block.
Suggested fix: merge both into a single JSDoc block — keep the original summary lines first, then the cleanPlaceholderText/R1-1 explanation.
中文说明
[Suggestion] 新增的 JSDoc 块直接堆叠在 attachMedia 已有的 JSDoc 之后，而不是合并进去 —— 这是全文件唯一的双 /** */ 块。编辑器/TypeScript 工具只会把声明前的最后一个文档注释挂到符号上，因此原始摘要（"Download a media file and attach it to the envelope. Images → base64 in envelope; files → saved to temp dir with path in text."）脱离了该函数。 — 失败场景：悬停查看 attachMedia（或阅读生成的 API 文档）现在只会显示 R1-1 的历史说明；下一个读者会丢失 image/file 的行为契约，而被孤立的那个块看起来仍像有效文档。已通过 TypeScript 编译器 API 验证：symbol.getDocumentationComment() 只返回最后一个块。
建议修复：把两者合并为一个 JSDoc 块 —— 先保留原始摘要，再放 cleanPlaceholderText/R1-1 的说明。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| quoted.media.mediaType, | ||
| quoted.media.fileName, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Quoted messages now trigger media downloads (two-step server download, up to 50 MB, local temp storage, attachment to the prompt), but the user-facing docs are not updated — in docs/users/features/channels/dingtalk.md the "Referenced messages" bullet (~line 188) still only says quoting "includes the quoted text as context", and the "Images and Files" section (~167-173) describes only directly-sent media. — Failure scenario: a user who quotes an image/file/audio/video message now triggers a download and a temp-file write of up to 50 MB that the page documenting DingTalk media behaviour never mentions; users troubleshooting unexpected downloads or checking whether quoting media is supported have no documented behaviour to consult, while the sibling precedent (direct-media download) is documented on the same page.
Suggested fix: extend the "Referenced messages" bullet (or the "Images and Files" section) to state that quoting a media message downloads and attaches it the same way as sending it directly.
中文说明
[Suggestion] 引用消息现在会触发媒体下载（两步服务端下载、最大 50 MB、本地临时存储、附加到 prompt），但面向用户的文档没有更新 —— docs/users/features/channels/dingtalk.md 的 "Referenced messages" 条目（约 188 行）仍只说引用会"includes the quoted text as context"，而 "Images and Files" 小节（约 167-173 行）只描述直接发送的媒体。 — 失败场景：用户引用图片/文件/音频/视频消息时，现在会触发一次下载和最大 50 MB 的临时文件写入，而记录钉钉媒体行为的页面对此只字未提；排查意外下载或想确认引用媒体是否受支持的用户没有文档可查 —— 同页已记录了同类行为（直发媒体下载）作为先例。
建议修复：在 "Referenced messages" 条目（或 "Images and Files" 小节）中补充说明：引用媒体消息会像直接发送一样下载并附加该媒体。
— qwen3.8-max via Qwen Code /review (v0.21.13)
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.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 msgType→mediaType mapping duplication (still stands) — already re-reported by round-2 review 4960003798 (comment 3803224925 at DingtalkAdapter.ts:1399)
- R1-4 mediaType guard half untested (still stands) — already re-reported by round-2 review 4960003798 (comment 3803224933 at DingtalkAdapter.ts:1411)
- R1-5 own-media + quoted-media combination untested (still stands) — already re-reported by round-2 review 4960003798 (comment 3803224938 at DingtalkAdapter.ts:1820)
- R2-1 placeholder format re-derived at the direct call site — already reported by round-2 review 4960003798 (comment 3803224943 at DingtalkAdapter.ts:1815)
- R2-2 direct-media placeholder blanking has no test coverage — already reported by round-2 review 4960003798 (comment 3803224946 at DingtalkAdapter.ts:1815)
- R2-3 orphaned JSDoc block before attachMedia — already reported by round-2 review 4960003798 (comment 3803224957 at DingtalkAdapter.ts:1564)
- R2-5 R1-2 tests do not pin the degrade shape — already reported by round-2 review 4960003798 (comment 3803224950 at DingtalkAdapter.test.ts:2639)
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/channels/dingtalk/src/DingtalkAdapter.ts:1818 — [review] channel-files accumulation is never cleaned, and quoted media removes the re-upload cost asymmetry (code-age rule: anchored on the unchanged quoted-call block round 1 already…packages/channels/dingtalk/src/DingtalkAdapter.ts:1818 — [probe] quoted media is downloaded and persisted before any inbound gating runs (code-age rule: anchored on the unchanged quoted-call block round 1 already read)
中文说明
已审查——无阻断问题。 建议见行内评论。
本轮确认的 7 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| const dir = join(tmpdir(), 'channel-files', randomUUID()); | ||
| mkdirSync(dir, { recursive: true }); |
There was a problem hiding this comment.
[Suggestion] R2-4: The new R1-2 catch path returns after writeFileSync throws but never removes the directory mkdirSync already created — an empty directory for the ENAMETOOLONG case this catch was written for, a partial file of up to 50 MB for a mid-write ENOSPC. — Failure scenario: a quoted fileName over 255 bytes (exactly the test's 'a'.repeat(300) case): mkdirSync succeeds, writeFileSync throws, the catch logs and returns, leaving one stray tmpdir()/channel-files/<uuid> directory per occurrence until OS tmp cleanup; repeated quoting of long-named files accumulates them indefinitely. Side effect: the attachment never gets a filePath in that case, so the new tests cannot register the leaked directory in tempDirs either, and every test run leaves a stray directory in the host tmpdir.
Witness (measured):
running just the two R1-2 throw tests: BEFORE=50 AFTER=52 LEAKED=2
(one empty dir from the ENAMETOOLONG case, one dir holding a 3-byte dingtalk_file_<ts> fallback file)
Suggested fix (hoist dir out of the try, clean it up in the catch; import rmSync alongside mkdirSync):
let dir: string;
try {
dir = join(tmpdir(), 'channel-files', randomUUID());
mkdirSync(dir, { recursive: true });
// ...basename/writeFileSync as today...
} catch (error) {
try {
rmSync(dir, { recursive: true, force: true });
} catch {}
// ...existing stderr log...
return;
}中文说明
[Suggestion] R2-4：新的 R1-2 catch 分支在 writeFileSync 抛错后直接 return，但从未删除 mkdirSync 已经创建的目录 —— 对这个 catch 专门为之地编写的 ENAMETOOLONG 场景留下一个空目录，对写入中途 ENOSPC 则可能留下最大 50 MB 的部分文件。 — 失败场景：被引用的 fileName 超过 255 字节（正是测试里 'a'.repeat(300) 的场景）：mkdirSync 成功，writeFileSync 抛错，catch 记录日志后返回，每次发生都会遗留一个 tmpdir()/channel-files/<uuid> 孤儿目录，只能等操作系统清理临时目录；反复引用长文件名会无限累积。副作用：此时附件拿不到 filePath，新测试也无法把泄漏的目录登记进 tempDirs，因此每次跑测试都会在宿主机临时目录留下孤儿目录。
见证（实测）：只运行两个 R1-2 抛错测试：BEFORE=50 AFTER=52 LEAKED=2（一个空目录来自 ENAMETOOLONG 场景，一个目录内含 3 字节的 dingtalk_file_ 兜底文件）。
建议修复：把 dir 提升到 try 外，在 catch 中先 try { rmSync(dir, { recursive: true, force: true }); } catch {} 再返回（rmSync 与 mkdirSync 一并导入）。见英文侧代码块。
— qwen3.8-max via Qwen Code /review (v0.21.13)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
本轮审查结果
审查范围
packages/channels/dingtalk/src/DingtalkAdapter.ts（+89/-13 行）、DingtalkAdapter.test.ts（+363/-2 行）
回合 1-2 Critical 修复验证
R1-1（Critical）— 占位符清理误清用户文本 — ✅ 已修复
attachMedia 新增 cleanPlaceholderText?: string 参数，仅由直发媒体调用点传入，引用媒体调用点不传。清理逻辑 if (cleanPlaceholderText !== undefined && envelope.text === cleanPlaceholderText) 正确隔离了引用路径。测试验证了 (audio)、(video)、(file: report.pdf) 三种占位符文本在引用路径上均不被清空。
R1-2（Critical）— 同步抛错点导致 prompt 永久丢失 — ✅ 已修复
mkdirSync/writeFileSync/basename 外围已包裹 try/catch，basename 前增加了 typeof fileName === 'string' 类型守卫。catch 路径写入 stderr 后 return，不中断 handleInbound 调用。测试覆盖了超长文件名（300 字符）和非字符串文件名两类异常。
行内评论
本轮无新增行内评论。此前回合已报告的 7 条 Suggestion 级发现仍有效，不在本轮重复提出。
总体评价
COMMENT — 两个 Critical 阻断项已修复并通过测试验证。改进方向的 Suggestion 级发现已在之前回合中记录，建议作者在后续迭代中择机处理。
English version
Review Results
Scope
packages/channels/dingtalk/src/DingtalkAdapter.ts (+89/-13 lines), DingtalkAdapter.test.ts (+363/-2 lines)
Round 1-2 Critical Fix Verification
R1-1 (Critical) — Placeholder cleanup blanking user text — ✅ Fixed
attachMedia now accepts cleanPlaceholderText?: string, passed only from the direct-media call site. The guard cleanPlaceholderText !== undefined && envelope.text === cleanPlaceholderText correctly isolates the quoted path. Tests verify that (audio), (video), and (file: report.pdf) survive intact on the quoted path.
R1-2 (Critical) — Sync throw sites losing prompts — ✅ Fixed
mkdirSync/writeFileSync/basename are wrapped in try/catch with a typeof fileName === 'string' guard. The catch path writes to stderr and returns, never blocking handleInbound. Tests cover both over-long (300-char) and non-string file names.
Inline Comments
No new inline comments this round. The 7 Suggestion-level findings from previous rounds still stand and are not re-raised per the convergence posture.
Overall
COMMENT — Both Critical blockers are fixed and verified. Suggestion-level improvement areas are documented in earlier rounds for the author to address in follow-up iterations.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
Unify the msgType->mediaType mapping in a shared helper, make extractContent the single source of truth for the placeholder text cleaned on attach, and remove the store directory when a media write fails so failed stores no longer leak into tmpdir. Merge the stacked attachMedia JSDoc blocks, document quoted media downloads, and pin the previously uncovered paths: unmapped quoted msgTypes with a downloadCode, own-media + quoted-media combinations, direct placeholder cleaning, and the degraded-store attachment shapes.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review response — PR #9347 (round 3)Commit: Feedback points and dispositionsRound 1–2 Criticals (already fixed; re-verified this round)
Suggestions implemented this round
No action
Verification
中文说明审查回应 — PR #9347（第 3 轮）提交： 反馈点及处置第 1–2 轮 Critical（已修复；本轮重新验证）
本轮落实的 Suggestion
无需处理
验证
🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
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.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/channels/dingtalk/src/DingtalkAdapter.ts:1829 — [probe] own image + quoted image: second image downloaded then silently discarded by ChannelBasepackages/channels/dingtalk/src/DingtalkAdapter.ts:1829 — [probe] quoted-media download runs before the preflightInbound sender-policy gate
中文说明
已审查。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| - **Use DingTalk markdown-aware instructions** — DingTalk supports headings, bold text, links, code blocks, and tables. Keep tables compact because narrow screens may scroll horizontally. | ||
| - **Restrict access** — In an organization context, `senderPolicy: "open"` may be acceptable. For tighter control, use `"allowlist"` or `"pairing"`. See [DM Pairing](./overview#dm-pairing) for details. | ||
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. Quoting bot responses is not yet supported. | ||
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message carries an image, file, audio, or video, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. |
There was a problem hiding this comment.
[Suggestion] R3-1: The doc sentence added by this diff is unconditional, but quoted media is only downloaded for picture/file/audio/video msgTypes — mediaTypeFromMsgType fails closed for richText (DingtalkAdapter.ts:1469-1481), so a quoted richText message produces no media field and nothing downloads, while summarizeRepliedContent still renders the [image] placeholder into the prompt. A DIRECT richText message, by contrast, downloads its first image via extractContent's richText branch — so the documented parity ("the same way as when sent directly") is false for quoted richText and legacy quoteMessage shapes. — Failure scenario: a user quotes a richText message containing an image → referencedText shows [image], nothing downloads, and the agent sees the placeholder with no image, contradicting the sentence this line adds. The PR description already declares richText quotes out of scope, so the fix is to qualify the sentence, not to implement the download.
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message carries an image, file, audio, or video, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. | |
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message is a picture, file, audio, or video message, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. |
中文说明
[Suggestion] R3-1：本 diff 新增的这句文档表述是无条件的，但引用媒体只对 picture/file/audio/video 这些 msgType 下载 —— mediaTypeFromMsgType 对 richText 失败关闭（DingtalkAdapter.ts:1469-1481），因此引用 richText 消息不会产生 media 字段、不会触发任何下载，而 summarizeRepliedContent 仍会把 [image] 占位符渲染进 prompt。相比之下，直发的 richText 消息会经 extractContent 的 richText 分支下载其第一张图片 —— 所以"与直接发送相同"的表述对引用 richText 及 legacy quoteMessage 形态不成立。 — 失败场景：用户引用一条包含图片的 richText 消息 → referencedText 显示 [image]，但没有任何下载，Agent 看到占位符却收不到图片，与本行新增的表述矛盾。PR 描述已声明 richText 引用不在本次范围内，因此修复方式是限定该句的适用范围，而非实现 richText 下载。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| mediaType: this.mediaTypeFromMsgType(msgtype), | ||
| placeholder: '(video)', |
There was a problem hiding this comment.
[Suggestion] R3-2: The direct-video placeholder cleanup has no test. Round 3 added direct-cleanup tests for file and audio only (partially addressing R2-2), leaving the video branch's new placeholder wiring unguarded: the mutant removing placeholder: '(video)' survives the entire suite (measured this round: 139/139 green), because both existing 'video' tests exercise the QUOTED path with a text own-message, which never produces a placeholder. — Failure scenario: a future refactor drops the video placeholder → every test stays green while direct video messages deliver (video) as literal prompt text next to the attachment — the exact pollution the cleaning branch exists to prevent. This is the bounded file/audio/video family's last open sibling (file ✓, audio ✓, video ✗). Suggested fix: add a direct-video sibling of the existing direct-audio test — send a non-reply message with downloadCode + msgType: 'video' (e.g. sendDirectMedia(channel, 'video', { downloadCode: 'direct-video-code' })) and assert handleInbound receives text: '' plus the video attachment.
Witness (measured this round):
mutant (placeholder line removed): Tests 139 passed (139) — survives
probe under mutant: AssertionError: expected '(video)' to be ''
probe after restore: PROBE envelope.text: "" — the added test flips the mutant
中文说明
[Suggestion] R3-2：直发视频的占位符清理没有测试。第 3 轮只为 file 和 audio 补充了直发清理测试（部分解决了 R2-2），video 分支新接入的 placeholder 仍无保护：删除 placeholder: '(video)' 的变异体能让整个测试套件保持全绿（本轮实测 139/139 通过），因为现有的两个 'video' 用例都走的是引用路径且自身消息为纯文本，永远不会产生占位符。 — 失败场景：未来某次重构删掉了 video 的 placeholder → 所有测试依旧绿色，而直发视频消息会把 (video) 作为字面 prompt 文本连同附件一起交给 Agent —— 正是清理分支要防止的污染。这是 file/audio/video 有界家族中最后一个未覆盖的成员（file ✓、audio ✓、video ✗）。建议修复：仿照现有直发 audio 测试增加一个直发 video 用例 —— 发送带 downloadCode + msgType: 'video' 的非回复消息（如 sendDirectMedia(channel, 'video', { downloadCode: 'direct-video-code' })），断言 handleInbound 收到 text: '' 和视频附件。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/channels/dingtalk/src/DingtalkAdapter.ts:1830 — [review] own-media and quoted-media downloads are awaited sequentially — prompt-start latency is the sum, not the max, of the two flows (code-age rule: anchored on the unchanged quote…
[Critical] R4-1 — packages/channels/dingtalk/src/DingtalkAdapter.ts:1828-1835 — A quoted image attached alongside the message's own image is silently discarded by ChannelBase's single-image resolution. Both attachments arrive as {type: 'image', data, mimeType} (data only, no filePath); in ChannelBase.ts (~5253-5282) the first image fills the only imageBase64 slot, and the second fails that guard and also fails else if (att.filePath) — dropped with no log and no prompt line. Before this PR a DingTalk envelope held at most one attachment, so this diff makes the collision reachable. — Failure scenario: a user replies to an image message with their own image and asks about the quoted one; the quoted image is downloaded (download-code POST + up to 50 MB fetch), attached, then silently lost — the agent answers having seen only [Replying to: "[image]"] and the user's own image, and the doc sentence this same PR adds is silently broken. Witness (probe, unmodified PR): resolution of a two-image envelope yields promptText "[Replying to: " image "]\n\ninspect both" and options.imageBase64 = own image only — the quoted image is absent from prompt, options, and logs; file-backing the second image (fix-flip) surfaces it ("User sent an image. It has been saved to: /tmp/channel-files/…"). Suggested fix: file-back the quoted image (attach via filePath like non-image media), at least when the envelope already carries image data, or skip the quoted download with a stderr note when the conflict is detected. (This finding's inline comment was suppressed because its resolved line 1832 overlaps the fixed round-1 R1-1 thread at the same line — a different defect, so its only copy lives in this body.) 中文说明：当消息自带图片且同时附加引用图片时，引用图片被 ChannelBase 的单图片解析逻辑静默丢弃（两个附件均为仅含 data 的图片；第一个占满唯一的 imageBase64 槽位，第二个两个分支均不满足，无日志、无 prompt 行）。本 PR 之前 envelope 最多只有一个附件，是本 diff 使该冲突可达。失败场景：用户用自己的图片回复图片消息并询问引用图片 → 引用图片被下载、附加后静默丢失，Agent 只看到占位符与用户自己的图片。见证（探针）：双图片 envelope 解析后引用图片在 prompt、options、日志中均无踪迹；修复翻转（第二张图片落盘）后可正常呈现。建议修复：将引用图片落盘（以 filePath 附加），或检测冲突并跳过下载并输出 stderr 说明。（此发现的行内评论因解析行 1832 与已修复的第 1 轮 R1-1 线程同行而被抑制 —— 属不同缺陷，故其唯一副本位于本正文。）
中文说明
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] R4-1 — packages/channels/dingtalk/src/DingtalkAdapter.ts:1828-1835 — A quoted image attached alongside the message's own image is silently discarded by ChannelBase's single-image resolution. Both attachments arrive as {type: 'image', data, mimeType} (data only, no filePath); in ChannelBase.ts (~5253-5282) the first image fills the only imageBase64 slot, and the second fails that guard and also fails else if (att.filePath) — dropped with no log and no prompt line. Before this PR a DingTalk envelope held at most one attachment, so this diff makes the collision reachable. — Failure scenario: a user replies to an image message with their own image and asks about the quoted one; the quoted image is downloaded (download-code POST + up to 50 MB fetch), attached, then silently lost — the agent answers having seen only [Replying to: "[image]"] and the user's own image, and the doc sentence this same PR adds is silently broken. Witness (probe, unmodified PR): resolution of a two-image envelope yields promptText "[Replying to: " image "]\n\ninspect both" and options.imageBase64 = own image only — the quoted image is absent from prompt, options, and logs; file-backing the second image (fix-flip) surfaces it ("User sent an image. It has been saved to: /tmp/channel-files/…"). Suggested fix: file-back the quoted image (attach via filePath like non-image media), at least when the envelope already carries image data, or skip the quoted download with a stderr note when the conflict is detected. (This finding's inline comment was suppressed because its resolved line 1832 overlaps the fixed round-1 R1-1 thread at the same line — a different defect, so its only copy lives in this body.) 中文说明：当消息自带图片且同时附加引用图片时，引用图片被 ChannelBase 的单图片解析逻辑静默丢弃（两个附件均为仅含 data 的图片；第一个占满唯一的 imageBase64 槽位，第二个两个分支均不满足，无日志、无 prompt 行）。本 PR 之前 envelope 最多只有一个附件，是本 diff 使该冲突可达。失败场景：用户用自己的图片回复图片消息并询问引用图片 → 引用图片被下载、附加后静默丢失，Agent 只看到占位符与用户自己的图片。见证（探针）：双图片 envelope 解析后引用图片在 prompt、options、日志中均无踪迹；修复翻转（第二张图片落盘）后可正常呈现。建议修复：将引用图片落盘（以 filePath 附加），或检测冲突并跳过下载并输出 stderr 说明。（此发现的行内评论因解析行 1832 与已修复的第 1 轮 R1-1 线程同行而被抑制 —— 属不同缺陷，故其唯一副本位于本正文。）
— qwen3.8-max via Qwen Code /review (v0.21.13)
| - **Use DingTalk markdown-aware instructions** — DingTalk supports headings, bold text, links, code blocks, and tables. Keep tables compact because narrow screens may scroll horizontally. | ||
| - **Restrict access** — In an organization context, `senderPolicy: "open"` may be acceptable. For tighter control, use `"allowlist"` or `"pairing"`. See [DM Pairing](./overview#dm-pairing) for details. | ||
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. Quoting bot responses is not yet supported. | ||
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message carries an image, file, audio, or video, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. |
There was a problem hiding this comment.
[Suggestion] R3-1 (round 3, still stands): This sentence reads as covering any quoted message that carries an image, but quoted media is only downloaded for picture/file/audio/video msgTypes — mediaTypeFromMsgType fails closed for richText, so a quoted richText message (text + inline picture) renders the [image] placeholder into the prompt but downloads nothing, while a DIRECT richText message does download its first image — the documented parity ("the same way as when sent directly") is false for that shape. The PR description already declares richText quotes out of scope, so the fix is to qualify the sentence, not to implement the download. — Failure scenario: a user quotes a richText message containing an image and asks about it; referencedText shows [image], nothing downloads, and the agent sees the placeholder with no image, contradicting this sentence.
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message carries an image, file, audio, or video, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. | |
| - **Referenced messages** — Quoting (replying to) a user message includes the quoted text as context for the agent. If the quoted message is a picture, file, audio, or video message, the bot downloads and attaches it the same way as when sent directly. Quoting bot responses is not yet supported. |
中文说明
[Suggestion] R3-1（第 3 轮已报告，仍然存在）：该句读起来像是覆盖任何携带图片的引用消息，但引用媒体只对 picture/file/audio/video 这些 msgType 下载 —— mediaTypeFromMsgType 对 richText 失败关闭，因此引用的 richText 消息（文本 + 内嵌图片）只会把 [image] 占位符渲染进 prompt 而不会下载任何内容，而直发的 richText 消息却会下载其第一张图片 —— "与直接发送相同"的表述对这种形态不成立。PR 描述已声明 richText 引用不在本次范围内，因此修复方式是限定该句的适用范围，而非实现 richText 下载。 — 失败场景：用户引用一条包含图片的 richText 消息并询问该图片；referencedText 显示 [image]，但没有下载发生，Agent 只看到占位符而没有图片，与该句表述矛盾。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| downloadCodes: code ? [code] : [], | ||
| mediaType: 'video', | ||
| mediaType: this.mediaTypeFromMsgType(msgtype), | ||
| placeholder: '(video)', |
There was a problem hiding this comment.
[Suggestion] R3-2 (round 3, still stands): The direct-video placeholder cleanup has no test. Round 3 added direct-cleanup tests for file and audio only, leaving this video branch's placeholder wiring unguarded: the mutant changing placeholder: '(video)' to '(audio)' (or dropping the field) survives the whole suite — a direct video message would then keep the stray (video) placeholder beside the attachment while every test stays green. — Failure scenario: with that mutant, attachMedia's cleanup comparison envelope.text === cleanPlaceholderText fails ('(video)' !== '(audio)') and the agent prompt keeps the stray placeholder — exactly the artifact the cleanup and its two sibling tests exist to prevent. Suggested fix: add the sibling test it('cleans the generated placeholder for a direct video message', ...) mirroring the file/audio cases (sendDirectMedia(channel, 'video', { downloadCode: 'direct-video-code' }), assert envelope.text is '' and the attachment has type: 'video').
中文说明
[Suggestion] R3-2（第 3 轮已报告，仍然存在）：直发视频的占位符清理没有测试。第 3 轮只为 file 和 audio 添加了直发清理测试，video 分支这里的 placeholder 接线因此无人守护：将 placeholder: '(video)' 改为 '(audio)'（或删除该字段）的变异体能通过整个测试套件 —— 届时直发视频消息会在附件旁残留多余的 (video) 占位符，而所有测试仍然是绿的。 — 失败场景：在该变异下，attachMedia 的清理比较 envelope.text === cleanPlaceholderText 失败（'(video)' !== '(audio)'），Agent prompt 保留多余占位符 —— 正是清理逻辑及其两个姊妹测试本要防止的问题。建议修复：添加姊妹测试 it('cleans the generated placeholder for a direct video message', ...)，仿照 file/audio 用例（sendDirectMedia(channel, 'video', { downloadCode: 'direct-video-code' })，断言 envelope.text 为 '' 且附件的 type 为 'video'）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9347 (round 5)Addressed all actionable feedback: one Critical from the round-4 CHANGES_REQUESTED review body (R4-1) and the two still-standing round-3 suggestions (R3-1, R3-2, each posted twice). The items listed as "Deferred under the convergence posture — recorded, not requested in this round" were left untouched, except the round-3 probe item "own image + quoted image: second image downloaded then silently discarded", which is the same defect later escalated as R4-1 and is now fixed. Commit: Feedback points and decisions[Critical] R4-1 (review body rv:4964099764) — ACTEDQuoted image alongside the message's own image was silently discarded: both arrived as data-only
[Suggestion] R3-1 (rc:3805979974, rc:3806537005) — ACTEDThe documented parity sentence was unconditional while quoted media downloads are limited to [Suggestion] R3-2 (rc:3805979990, rc:3806537026) — ACTEDAdded the missing direct-video sibling test Deferred under convergence posture — UNCHANGED (recorded, not requested this round)
Verification
中文说明Autofix 审查轮次 — PR #9347（第 5 轮）已处理全部可执行反馈：第 4 轮 CHANGES_REQUESTED 审查正文中的一个 Critical（R4-1），以及两条仍然成立的第 3 轮建议（R3-1、R3-2，各被重复提交两次）。标注为"收敛姿态下延后——已记录，本轮不要求修改"的条目保持原样未动；唯一的例外是第 3 轮探针条目"自带图片 + 引用图片：第二张图片被下载后被静默丢弃"，它与后来升级为 R4-1 的缺陷相同，现已修复。 提交： 反馈点与决定[Critical] R4-1（审查正文 rv:4964099764）— 已处理当消息自带图片且同时附带引用图片时，引用图片被静默丢弃：两者均以仅含 data 的
[Suggestion] R3-1（rc:3805979974、rc:3806537005）— 已处理文档中的等价性表述是无条件的，但引用媒体下载仅限 [Suggestion] R3-2（rc:3805979990、rc:3806537026）— 已处理补充了缺失的直发 video 姊妹测试 收敛姿态下延后的条目 — 未改动（已记录，本轮不要求修改）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/channels/dingtalk/src/DingtalkAdapter.ts:1546 — [probe] recognition arm of the audio placeholder ternary has zero test coverage — mutant blanking a literal '(audio)' transcription survives the suite (code-age rule: anchored on code…packages/channels/dingtalk/src/DingtalkAdapter.ts:1659 — [probe] direct-media store-failure path has no test pinning placeholder survival — cleanup-on-failure mutant survives the suite (code-age rule: anchored on code unchanged since the ro…packages/channels/dingtalk/src/DingtalkAdapter.ts:1838 — [review] own-media and quoted-media downloads are awaited sequentially — prompt-start latency is the sum, not the max (already deferred in round 4; code unchanged since)
中文说明
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| safeName = | ||
| basename(typeof fileName === 'string' ? fileName : '') || | ||
| `dingtalk_${mediaType}_${Date.now()}`; |
There was a problem hiding this comment.
[Critical] R5-1: The generated fallback name for stored media has no file extension (dingtalk_<type>_<ts>), and qwen-code's read pipeline resolves file types extension-first — detectFileType gets null from mime.getType for an extensionless name, falls through every allowlist to the isBinaryFile content sampler, whose NUL-byte rule classifies real image/audio/video bytes as binary, so read_file returns "Cannot display content of binary file". This defeats two shapes this PR makes reachable: (1) file-backed quoted images — the exact R4-1 fix this round: when the message's own image occupies the inline slot, the quoted image is stored extensionless, so the saved to: line the new comment says "keeps it reachable for the agent" does not actually make it usable; (2) all quoted audio/video — they always take the file-backed path, and DingTalk audio/video content carries no fileName (the adapter's own direct audio/video branches never read one; the SDK wire shape for audio is {downloadCode, duration}), so every real quoted voice/video message is stored extensionless. The quoted audio/video tests supply an idealized fileName the wire shape lacks, which is why the suite misses this. — Failure scenario: a user replies to a picture with their own picture and asks about the quoted one → the quoted image downloads (up to 50 MB), is stored as dingtalk_image_<ts>, and read_file refuses it as binary — the agent answers from the user's own image only while the prompt claims the quoted one was saved. A user replies to a voice message → the OGG is stored as dingtalk_audio_<ts>, advertised via saved to:, and refused — the quoted-audio feature is unusable for its primary real-world shape.
Witness (probe through the real adapter + real detectFileType from packages/core):
quoted audio, wire shape {downloadCode, duration}:
PR code: saved dingtalk_audio_1787085728034 -> detectFileType: binary
-> llmContent: "Cannot display content of binary file"
fix-flip (mimeType-derived extension):
saved dingtalk_audio_1787085763575.ogg -> detectFileType: audio
-> llmContent: inlineData(audio/ogg) delivered to the model
image arm: extensionless PNG -> binary + refusal; same bytes as .png -> image + inline parts
Suggested fix (spans a helper plus this fallback, so no one-click suggestion): derive an extension from media.mimeType whenever the generated name is used —
// module scope
const GENERATED_MEDIA_EXT: Record<string, string> = {
'image/jpeg': 'jpg',
'image/png': 'png',
'image/gif': 'gif',
'image/webp': 'webp',
'audio/ogg': 'ogg',
'audio/mpeg': 'mp3',
'video/mp4': 'mp4',
};
// in attachMedia (this fallback)
safeName =
basename(typeof fileName === 'string' ? fileName : '') ||
`dingtalk_${mediaType}_${Date.now()}.${GENERATED_MEDIA_EXT[media.mimeType] ?? 'bin'}`;and tighten the R4-1 test to require the extension (/^dingtalk_image_\d+\.\w+$/), plus model quoted audio/video content without fileName (the wire shape) so the fallback is actually exercised.
中文说明
[Critical] R5-1：存储媒体时生成的回退文件名没有扩展名（dingtalk_<type>_<ts>），而 qwen-code 的读取管线是扩展名优先的 —— detectFileType 对无扩展名调用 mime.getType 得到 null，穿过所有白名单后落到 isBinaryFile 内容采样器，其 NUL 字节规则会把真实的图片/音频/视频字节判定为 binary，于是 read_file 返回 "Cannot display content of binary file"。这使本 PR 新可达的两种形态实际不可用：(1) 落盘的引用图片 —— 正是本轮 R4-1 修复的路径：当消息自带图片占满内联槽位时，引用图片以无扩展名落盘，新注释所谓 "saved to: 行使 Agent 可达" 实际并不成立；(2) 所有引用音频/视频 —— 它们必然走落盘路径，而钉钉音视频 content 不携带 fileName（适配器的直发音视频分支从不读取 fileName；SDK 线格式中音频为 {downloadCode, duration}），因此每条真实的引用语音/视频消息都以无扩展名存储。引用音视频测试提供了线格式中并不存在的理想化 fileName，因此测试套件未能发现此问题。 — 失败场景：用户用自己的图片回复一张图片并询问引用图片 → 引用图片下载（最大 50 MB）后以 dingtalk_image_<ts> 存储，read_file 以 binary 拒绝读取 —— prompt 声称已保存，Agent 却只能看到用户自己的图片。用户回复一条语音消息 → OGG 以 dingtalk_audio_<ts> 存储并经 saved to: 通告，但被拒绝读取 —— 引用音频功能在其最主要的真实形态（语音消息）下不可用。
见证（通过真实适配器 + packages/core 真实 detectFileType 的探针）：
引用音频，线格式 {downloadCode, duration}：
PR 代码：存储 dingtalk_audio_1787085728034 -> detectFileType: binary
-> llmContent: "Cannot display content of binary file"
修复翻转（按 mimeType 派生扩展名）：
存储 dingtalk_audio_1787085763575.ogg -> detectFileType: audio
-> llmContent: inlineData(audio/ogg) 送达模型
图片侧：无扩展名 PNG -> binary 拒绝；同样字节改名 .png -> image 内联送达
建议修复（跨辅助常量与此处回退，故不提供一键 suggestion）：凡使用生成文件名时，按 media.mimeType 派生扩展名（上方英文部分附代码），并将 R4-1 测试收紧为要求扩展名（/^dingtalk_image_\d+\.\w+$/），同时让引用音视频测试按真实线格式不带 fileName，使回退路径真正被覆盖。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Round summaryOutcome: 1 Critical resolved in code; 3 findings the reviewer explicitly deferred for this round remain deferred (no action requested). Feedback points[rc:3807907848] R5-1 — generated media store names carry no file extension (Critical) — RESOLVED in code Reproduced before changing any code:
Fix (minimal, as suggested in the finding):
[rv:4965744106] CHANGES_REQUESTED review — three findings the reviewer explicitly deferred ("recorded, not requested in this round") — REMAIN DEFERRED
Per the reviewer's round-5 convergence posture these were recorded, not requested; no code change for them this round. They stay visible for a follow-up. Changes
Conflict notesNone ( Verification
中文说明本轮摘要结果： 1 条 Critical 已在代码中解决；评审者本轮明确延后的 3 条发现保持延后状态（本轮不要求处理）。 反馈要点[rc:3807907848] R5-1 — 生成的媒体存储文件名不带文件扩展名（Critical）— 已在代码中解决 在修改任何代码之前先复现：
修复（按发现建议的最小改动）：
[rv:4965744106] CHANGES_REQUESTED 评审 — 评审者明确延后的三条发现（"已记录，本轮不要求修改"）— 保持延后
按评审者第 5 轮的收敛姿态，这些已被记录、本轮不要求处理；本轮不为其改动代码，保持可见以便后续跟进。 变更内容
冲突说明无（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/channels/dingtalk/src/DingtalkAdapter.ts:137 — [review] mime→extension table duplicates core's extensionForMimeType, which normalizes headers the local copy does notpackages/channels/dingtalk/src/DingtalkAdapter.ts:1623 — [review] ChannelBase's single-image-slot rule is duplicated in the adapter; the overflow compensation belongs in ChannelBasepackages/channels/dingtalk/src/DingtalkAdapter.ts:1623 — [probe] mutant some(() => true) on the slot probe ships green — the non-image-does-not-consume-the-slot half is untestedpackages/channels/dingtalk/src/DingtalkAdapter.ts:137 — [probe] 4 of 7 extension-map entries have no test pin — deleting one leaves the suite green and stored media unreadable
中文说明
无阻断问题。LGTM！✅
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /takeover stop |
qqqys
removed
the
autofix/takeover
Clears the CONFLICTING state this PR has carried for 19 rounds. One file conflicted, `packages/channels/dingtalk/src/DingtalkAdapter.test.ts`, and both sides were pure additions to it: this branch adds the `DingtalkChannel chat records` suite (+1205) and main adds `DingtalkChannel quoted media` (+634, from QwenLM#9347), both appended after `DingtalkChannel parsed-message logging`. Git could not align two insertions at the same anchor; the resolution keeps both suites and both sets of `node:fs`/`node:path` imports, with nothing dropped from either side. Verified on the merged tree: `packages/channels/dingtalk` runs 365 tests across 10 files, all passing (190 of them in the merged file itself). Prettier and eslint are clean on it. `tsc --noEmit` reports one error, `truncateUtf16Units` missing from `@qwen-code/channel-base` — identical before and after the merge, and an artifact of the worktree resolving that package to a stale build rather than to this branch's source, which does export it.
|
Released in v0.21.15. |
5 participants
What this PR does
Allow the DingTalk channel to download media from a message being replied to and attach it to the current agent prompt. Quoted pictures are provided as image data, while quoted files, audio, and video use the existing temporary-file attachment path. The user's outer question and the readable quote placeholder remain intact.
Why it's needed
DingTalk includes a
downloadCodewhen a user replies to a picture or file, but the channel previously kept only a textual placeholder such as[image]or[file: report.pdf]. The agent could tell that media had been quoted but could not inspect the referenced content. This change routes the quoted media through the channel's existing authenticated download and attachment flow.Reviewer Test Plan
How to verify
Reply to a picture and ask the bot to inspect it. Confirm the outer question remains the prompt, the quote remains visible as
[image], the quoted download code reaches DingTalk's media API, and an image attachment is delivered. Repeat with a file and confirm its name, MIME type, bytes, and temporary path are preserved. Audio and video should follow the same file-backed path. Finally, omit the download code or make token/media retrieval fail and confirm the prompt is still delivered once with its quote placeholder and no attachment.Evidence (Before & After)
Before: real reply callbacks contained
repliedMsg.content.downloadCodefor quoted pictures and files, but the channel discarded that field and delivered only a placeholder to the agent.After: adapter-level regressions cover successful picture, file, audio, and video downloads; missing download codes; access-token failure; media-API failure; prompt preservation; attachment metadata; file contents; and failure-safe temporary cleanup.
Tested on
Environment (optional)
Isolated DingTalk Stream callback capture and local Node.js workspace with independently installed dependencies.
Risk & Scope
unknownMsgTypefile variants are not handled without a verified callback shape. Media embedded inside combined-forward records is also out of scope.Linked Issues
N/A.
中文说明
本 PR 做了什么
让钉钉 Channel 能下载用户正在回复的消息媒体，并把它附加到当前 Agent prompt。引用图片以内嵌图片数据提供，引用文件、音频和视频复用现有临时文件附件链路。用户在外层提出的问题和可读的引用占位符都会保留。
为什么需要
用户回复图片或文件时，钉钉会提供
downloadCode，但 Channel 之前只保留[image]或[file: report.pdf]这类文本占位符。Agent 能知道用户引用了媒体，却无法检查实际内容。本改动将引用媒体接入 Channel 现有的鉴权下载与附件流程。Reviewer Test Plan
如何验证
回复一张图片并让机器人检查它。确认外层问题仍作为 prompt，引用仍显示为
[image]，引用消息的下载码被传给钉钉媒体接口，并且 Agent 收到图片附件。再回复一个文件，确认文件名、MIME 类型、字节内容和临时路径均被保留。音频和视频应走相同的文件附件链路。最后省略下载码，或者让 token/媒体下载失败，确认 prompt 仍只投递一次，保留引用占位符且不产生附件。证据（修改前后）
修改前：真实引用图片和文件回调的
repliedMsg.content.downloadCode中包含下载码，但 Channel 会丢弃该字段，只把占位符交给 Agent。修改后：适配器级回归测试覆盖图片、文件、音频和视频下载成功，缺少下载码，访问 token 失败，媒体接口失败，prompt 保留，附件元数据，文件内容以及失败安全的临时目录清理。
测试平台
环境（可选）
使用隔离的钉钉 Stream 回调捕获和独立安装依赖的本地 Node.js 工作区。
风险与范围
unknownMsgType文件变体。合并转发记录内部的媒体同样不在范围内。关联 Issue
无。