feat(dingtalk): support outbound image delivery - #7698
Conversation
github-actions
Bot
added
the
review/self-reported
E2E Test ReportEnvironment: macOS, Node.js 25.9.0, local development build, DingTalk Scenario: Asked the robot to generate a PNG and send it directly in the current conversation. Observed result:
Automated boundary evidence:
Not live-tested: Proactive group/direct delivery and failure fallback were validated against mocked DingTalk API boundaries rather than real recipients. |
Code Review — feat(dingtalk): support outbound image deliveryReviewed the full diff (static analysis + tracing against the surrounding Overview
What's done well
Suggestions (all non-blocking)
VerdictLooks merge-ready. The security-sensitive parts (path containment, credential redaction) are handled thoughtfully and tested. Items above are optional polish. 中文小结整体实现清晰、测试充分，安全处理（路径校验、凭证脱敏）尤其到位，未发现阻塞性问题。 做得好的地方： 建议（均非阻塞）：
结论：可以合并，以上为可选优化。 |
|
Thanks for the PR! Template looks good ✓ Problem: This is a feature request with a linked issue (#7687) — DingTalk users can already send images to the agent, but the channel has no way to deliver images back. The agent ends up exposing a local filesystem path the user can't open from chat. That's a real, observed capability gap, not a theoretical concern. Direction: Aligned. The DingTalk channel is an established integration, and outbound image delivery is a natural extension of the existing inbound image support. No direct CHANGELOG reference, but the area is clearly relevant. Size: Not applicable — all changes are in Approach: The scope feels right. A dedicated Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个有关联 issue（#7687）的功能请求——钉钉用户已经可以向 Agent 发送图片，但 channel 无法将图片回传给用户，Agent 只能暴露用户无法在聊天中打开的本地文件路径。这是一个真实的能力缺口，不是理论性问题。 方向：对齐。钉钉 channel 是已有的集成，出站图片投递是现有入站图片支持的自然延伸。CHANGELOG 中没有直接引用，但该领域明显相关。 规模：不适用——所有改动都在 方案：范围合理。独立的 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: I'd create a dedicated module for marker parsing, file validation (path containment, extension allowlist, size cap, magic-byte cross-check), and DingTalk Comparison: The PR matches this almost exactly. The implementation is clean and the security posture is genuinely careful — No critical blockers found. Two non-blocking observations (both already raised in wenshao's review):
TestingThis is an unattended CI run — no local build or tmux testing. Evidence below is from the PR's own CI checks on the reviewed commit.
Ubuntu unit tests pass. Windows/macOS tests and integration tests were skipped (typical for fork PRs). The author reports 154/154 DingTalk package tests passing locally, covering marker parsing, file allowlisting, magic bytes, symlink escape rejection, size limits, multipart upload, credential redaction, expired-token retry, normal replies, proactive group/direct delivery, visible failure fallback, and single-message sender mentions. Proactive group/direct delivery and failure fallback were validated against mocked API boundaries, not live recipients. Not verified: live DingTalk robot behavior (requires a configured application-robot channel). A maintainer can check out the branch in a disposable environment to confirm the end-to-end flow. 中文说明代码审查独立方案： 我会创建一个独立模块负责标记解析、文件校验（路径约束、扩展名白名单、大小上限、魔数交叉校验）和钉钉 对比： PR 的方案与此几乎完全一致。实现清晰，安全处理尤其到位—— 未发现阻塞性问题。两个非阻塞观察（wenshao 的审查中均已提出）：
测试这是无人值守的 CI 运行——无本地构建或 tmux 测试。以下证据来自 PR 自身在受审提交上的 CI 检查。 Ubuntu 单元测试通过。Windows/macOS 测试和集成测试被跳过（fork PR 的典型情况）。作者报告本地 154/154 钉钉包测试全部通过。主动群发/私聊和失败降级使用模拟 API 边界验证，未对真实接收者执行。 未验证：真实钉钉机器人行为（需要配置好的应用机器人 channel）。维护者可在一次性环境中检出分支确认端到端流程。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a well-executed feature PR. The problem is real (DingTalk users get a local path they can't open), the solution is the obvious one (upload via DingTalk's media API and embed the MediaID), and the implementation is careful where it matters — path validation is thorough, credentials never leak into errors or logs, and the code-fence masking means examples in documentation stay literal. The test suite covers the important edge cases: symlink escape, extension/content mismatch, size limits, token retry, fallback messaging, and mention coalescing. Going back to my independent proposal — the PR matches it almost line for line. I don't see a simpler path that covers the same ground. Every change in the diff serves the feature; there's no scope creep. The two non-blocking nits (config mutation, image-only title) are polish, not concerns. The author has been responsive and the PR already had a thorough human review (wenshao) that reached the same conclusion. CI is green on Ubuntu. Ships the feature cleanly. 中文说明信心：5/5 —— 每个阶段都很干净，毫不犹豫地合并。 这是一个执行良好的功能 PR。问题是真实的（钉钉用户收到无法打开的本地路径），方案是显而易见的（通过钉钉媒体 API 上传并嵌入 MediaID），实现在关键处很仔细——路径校验全面，凭证不会泄露到错误或日志中，代码围栏屏蔽确保文档中的示例保持原样。测试套件覆盖了重要的边界情况：软链接逃逸、扩展名/内容不匹配、大小限制、令牌重试、降级消息和 @ 合并。 回顾我的独立方案——PR 几乎逐行匹配。我没有看到更简单的路径能覆盖同样的范围。diff 中的每一处改动都服务于该功能，没有范围蔓延。两个非阻塞的小问题（config 修改、纯图片标题）是打磨，不是顾虑。 作者响应积极，PR 已经有了 wenshao 的全面人工审查并得出相同结论。CI 在 Ubuntu 上通过。干净地交付了功能。 — Qwen Code · qwen3.8-max-preview 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. ✅
| const closing = text.indexOf(delimiter, offset + runLength); | ||
| const newline = | ||
| runLength >= 3 ? -1 : text.indexOf('\n', offset + runLength); |
There was a problem hiding this comment.
[Critical] maskCode treats mid-line triple-backtick runs as fenced-code openers. When a response contains mid-line ``` before a real fenced block (e.g. "Use bash for scripts, then:\n\n[IMAGE: /tmp/chart.png]\n```"), the opening fence of the real block is consumed as a closer for the mid-line run. The real fenced block's content is then exposed to findImageMarkers, which processes the `[IMAGE: ...]` marker inside it — reading and uploading the file and replacing the code example.
This violates issue #7687's acceptance criterion: "Parse image markers outside fenced and inline code without altering examples inside code."
Root cause: for runLength >= 3, newline is unconditionally set to -1 (line 56), so closesBeforeNewline is always true. Any subsequent ``` is accepted as a closer regardless of line position.
Failure scenario: LLM generates text referencing triple backticks mid-line before showing a fenced code example → image marker inside the example is incorrectly processed, performing an unintended file upload and corrupting the response.
| const closing = text.indexOf(delimiter, offset + runLength); | |
| const newline = | |
| runLength >= 3 ? -1 : text.indexOf('\n', offset + runLength); | |
| const closing = text.indexOf(delimiter, offset + runLength); | |
| const newline = | |
| runLength >= 3 | |
| ? text.indexOf('\n', offset + runLength) | |
| : text.indexOf('\n', offset + runLength); | |
| const closesBeforeNewline = | |
| closing !== -1 && (newline === -1 || closing < newline); |
For runLength >= 3, also require the closer to be at a line boundary (preceded only by whitespace since the start of its line), matching CommonMark fenced-code semantics.
— qwen3.7-max via Qwen Code /review
| if (!this.config.instructions) { | ||
| this.config.instructions = [ | ||
| '## DingTalk Channel', |
There was a problem hiding this comment.
[Suggestion] Two of the three constructor instruction-injection branches are untested.
Concrete cost: The default branch (!this.config.instructions) assigns a full instruction block — no test asserts its content or that IMAGE_INSTRUCTIONS is included. A regression that garbles the default text or omits the image guidance would go undetected. The skip guard (instructions already contains '[IMAGE:') is also untested — a future change that re-appended IMAGE_INSTRUCTIONS despite the guard would silently duplicate instructions.
Suggested fix: Add two tests:
- Create a channel without
instructionsand assert the default includes'## DingTalk Channel'and'[IMAGE:' - Create a channel with instructions already containing
'[IMAGE:'and assert the instructions remain unchanged
— qwen3.7-max via Qwen Code /review
|
Released in v0.21.1. |
4 participants
What this PR does
This PR lets the DingTalk channel deliver images created by the agent. Responses may include an
[IMAGE: /absolute/path/to/image.png]marker; the channel validates and uploads the local file, then replaces the marker with a DingTalk Markdown image backed by the returned MediaID.The behavior applies to normal replies, proactive group messages, and proactive direct messages. Text around the marker is preserved, examples inside inline or fenced code remain literal, expired access tokens are refreshed once, and upload failures produce a visible fallback without exposing credentials. When sender mentions are enabled, the mention and image are delivered in one Markdown message.
Why it's needed
DingTalk users can already send images to Qwen Code, but the channel could not return screenshots, charts, or generated images. Agents therefore exposed a local path that the user could not open from chat. Native upload closes that capability gap without requiring an external image host.
Reviewer Test Plan
How to verify
Configure a DingTalk application-robot channel whose workspace is writable, ask it to generate a PNG and send the result directly, and confirm that DingTalk renders the image inline with the surrounding text. With sender mentions enabled, confirm the mention and image arrive in one message.
Ask for an image-only reply and confirm no empty companion message is emitted. Include a marker inside inline or fenced code and confirm it remains literal. Reference a missing or disallowed path and confirm the response contains a visible failure notice while logs contain no access token.
For proactive delivery, send a valid marker to a group target and a direct-message target and confirm both messages render the uploaded image.
Evidence (Before & After)
Before: the agent returned a local filesystem path or image marker that DingTalk could not render.
After: DingTalk renders the generated PNG inline. A live macOS test with the
testapplication robot also confirmed thatatSenderproduces one mentioned Markdown image message instead of a separate mention message.Tested on
Environment (optional)
macOS, Node.js 25.9.0, local development channel started from this branch.
Risk & Scope
Linked Issues
Fixes #7687
中文说明
本 PR 做了什么
本 PR 让钉钉 channel 可以发送 Agent 创建的图片。回复中可以包含
[IMAGE: /absolute/path/to/image.png]标记；channel 会校验并上传本地文件，再用返回的 MediaID 将标记替换为钉钉 Markdown 图片。该能力覆盖普通回复、主动群消息和主动私聊消息。标记周围的文本会被保留，行内代码或代码块中的示例保持原样，访问令牌过期时会刷新并重试一次，上传失败时会提供可见的降级提示且不会泄露凭证。开启回复发送者 @ 时，@ 和图片会合并在同一条 Markdown 消息中发送。
为什么需要
钉钉用户已经可以向 Qwen Code 发送图片，但 channel 无法返回截图、图表或生成图片。因此 Agent 只能暴露用户无法从聊天中打开的本地路径。原生上传能力补齐了这一差距，也不需要外部图片托管服务。
Reviewer 测试计划
如何验证
配置一个工作区可写的钉钉应用机器人 channel，让它生成 PNG 并直接发送结果，确认钉钉能在回复文字中内联展示图片。开启回复发送者 @ 后，确认 @ 和图片出现在同一条消息中。
要求仅发送图片，确认不会额外产生空消息。把图片标记放进行内代码或代码块，确认标记保持字面量。引用不存在或不允许的路径，确认回复中出现可见的失败提示，同时日志不包含访问令牌。
对于主动发送，分别向群目标和私聊目标发送有效图片标记，确认两种消息都能渲染上传后的图片。
前后对比证据
改动前：Agent 返回钉钉无法渲染的本地文件路径或图片标记。
改动后：钉钉可以内联展示生成的 PNG。macOS 上使用
test应用机器人的真实验证还确认了atSender会生成一条带 @ 的 Markdown 图片消息，不再额外发送一条 @ 消息。测试平台
环境（可选）
macOS、Node.js 25.9.0，并从当前分支启动本地开发 channel。
风险与范围
关联 Issue
Fixes #7687