fix(channels): preserve DingTalk rich-text multi-image messages - #9922
fix(channels): preserve DingTalk rich-text multi-image messages#9922BenGuanRan wants to merge 12 commits into
Conversation
DingTalk rich-text multi-image E2E reportScopeVerify that every image in one DingTalk Environment
Baseline reproduction
Observed before the fix: the model reported only one image. This evidence is recorded in issue #9878 without callback payloads, user identifiers, credentials, or signed download URLs. Fixed-path verification
Observed after the fix:
The three live-test images were intentionally identical. Relative order is therefore anchored separately by unit coverage using distinct image bytes and distinct attachment identifiers. Failure-path regression coverage
PrivacyThe report contains no callback payload, sender identity, conversation/session identifier, credential, API key, signed URL, image bytes, or base64 data. |
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ — every required section is filled in, including a real before/after and the bilingual body. Problem: this is an observed bug, not theoretical hardening. #9878 carries a full reproduction (five images in one DingTalk direct message → one Direction: aligned. Silently dropping user images in an already-supported channel is a data-loss correctness bug, and multi-image ingestion is an area the reference agent has also been actively fixing lately (e.g. "Fixed pasting or dropping multiple images only inserting the last one"). No direction concerns. Size: not applicable to the core-module gate — no enumerated core paths are touched. The change spans two workspace packages ( Approach: the scope feels right at the structure level. The ordered Risk: no elevated risk signals — none of the changed files match the revert-history high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 所有必填部分都已填写,包含真实的前后对比证据和中英双语正文。 问题:这是已观测到的 bug,而非理论性加固。#9878 提供了完整复现(一次钉钉私聊发送五张图片 → 只有一次 方向:对齐。在已支持的渠道里静默丢弃用户图片属于数据丢失类的正确性 bug;参考产品近期也在持续修复多图摄入相关问题(如 "Fixed pasting or dropping multiple images only inserting the last one")。方向无顾虑。 规模:不触发核心模块门禁 —— 未触及任何已列名的核心路径。改动跨两个工作区包( 方案:从结构上看范围合理。有序 风险:无升级风险信号 —— 改动文件均未命中 revert 历史高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /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. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Code reviewI formed my independent expectation before reading the diff: loop every download code in the adapter, carry an ordered image collection through the bridge options with the legacy single-image fields kept as fallback, upload each image to the daemon session before prompting, and never submit a partial set. The PR does exactly this, and does it cleanly — no simpler path was missed. What I verified beyond the diff itself:
One non-blocking nit: the sequenceDiagram
participant P1 as DingTalk
participant P2 as DingtalkAdapter
participant P3 as ChannelBase
participant P4 as DaemonChannelBridge
participant P5 as Daemon session
P1->>P2: richText callback with N picture codes
P2->>P2: download every code in order
P2->>P3: envelope with N ordered image attachments
P3->>P4: ordered images collection plus legacy fallback fields
P4->>P5: upload each image in order
P5-->>P4: attachment reference per image
P4->>P5: prompt with N references before text
alt a later upload fails
P4->>P5: remove already-uploaded attachments
P4-->>P3: prompt rejected, prompt state already released
end
Files changed (11 of 11 shown)
TestingUnattended CI run — I do not build or execute PR code; the evidence below is the PR's own CI on the reviewed commit, read through the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The main unit suite Sandboxed verification would settle this: 中文说明代码审查我在看 diff 之前先形成了独立预期:适配器遍历每个下载码、桥接层用有序图片集合(保留旧单图字段回退)、提示前逐张上传到 daemon 会话、绝不提交残缺集合。PR 的实现与此完全一致,且没有遗漏更简路径。 diff 之外核实的内容:
一个非阻塞小问题: 测试无人值守 CI 运行——不构建、不执行 PR 代码;以下为通过 API 读取的该提交自身 CI 证据。主单测 沙箱验证可以落定此事: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a clean, well-scoped fix that matches my independent proposal and pins its behavior with real tests; the point withheld is the still-running unit suite and the end-to-end leg that only mocked tests plus the author's self-report cover so far. Stepping back: this is exactly the kind of PR the gate should wave through. The problem is real and reproduced (five images in, one image out, with runtime evidence in #9878), the fix is the minimal one — an ordered collection threaded through the existing seams with the legacy fields left intact for the eight adapters that haven't migrated — and nothing in the diff is incidental. The failure handling is the part I scrutinized most, and it's right: prompt state is released before best-effort rollback, a partial upload set is never submitted, and each of those claims has a dedicated test that would fail without the implementation. The pre-existing Web UI upload-then-rollback pattern is followed rather than reinvented, which is what I'd want to see in six months. My two reservations are both non-blocking and recorded in the review comment: the Approval is deferred only because the PR's own CI had not settled at review time: the unit suite 中文说明这是一次干净、范围得当的修复:问题真实且已复现(#9878:五张图只进一张,有运行时证据),方案是最小路径——有序集合穿过既有接缝、旧字段为未迁移的八个适配器保留——diff 中没有顺手改动。失败处理是重点核查项且正确:尽力回滚前先释放 prompt 状态、绝不提交残缺上传集合,每条结论都有"去掉实现即失败"的专门测试。既有的 Web UI"上传-失败回滚"模式被复用而非重造。 两点保留意见均不阻塞(已记录在审查评论中): 审批之所以暂缓,仅因审查时 PR 自身 CI 未出结果:主单测仍在该提交上运行。若其转绿,将自动按此提交钉住发布审批;若出现红项或提交移动,则保持不发。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| return getString(content['text']); | ||
| } | ||
|
|
||
| function channelImageName(mimeType: string, index = 0): string { |
There was a problem hiding this comment.
[Suggestion] This upload cluster re-implements helpers the webui already has for exactly this daemon flow, and the copies have drifted: imageAttachmentName / imageAttachmentMimeType (packages/webui/src/daemon/session/actions.ts:94-103), daemonPromptImageToBlob (packages/webui/src/daemon/session/promptContent.ts:27), and removeUploadedAttachments (actions.ts:413-422 — a shape-for-shape match of the new Promise.allSettled(...removeAttachment...) finally block below).
The drift is concrete: webui maps jpg→jpeg, strips +suffix, falls back to img and never throws; this helper throws for any extension outside five values (see the Critical on line 139). A future naming/MIME fix — adding avif, say — made in one copy will not propagate to the other, and the two daemon-attachment surfaces will keep diverging. channels/base has no dependency on webui today, so resolving this needs a shared-home decision: hoist the helpers (name derivation, MIME normalization, base64→Blob, rollback) into a module both packages may consume, or at minimum mirror their semantics deliberately and document why the channel allow-list is stricter than the daemon's.
中文说明
这个上传集群重新实现了 webui 里已有的、针对同一 daemon 流程的辅助函数,而且两份已经分叉:imageAttachmentName / imageAttachmentMimeType(packages/webui/src/daemon/session/actions.ts:94-103)、daemonPromptImageToBlob(packages/webui/src/daemon/session/promptContent.ts:27)、removeUploadedAttachments(actions.ts:413-422——与下方新增的 Promise.allSettled(...removeAttachment...) finally 块在形状上完全一致)。
分叉是具体的:webui 会把 jpg→jpeg 映射、剥掉 +suffix、回退到 img 且从不抛异常;而这里的辅助函数对五个值之外的任何扩展名都抛异常(见 139 行的 Critical)。未来任何命名/MIME 修复——比如加入 avif——只改一份不会传播到另一份,两个 daemon 附件入口会持续分叉。目前 channels/base 并不依赖 webui,因此需要一个"共同归属"的决定:把这些辅助函数(命名推导、MIME 归一化、base64→Blob、回滚)提升到两个包都能消费的模块里;至少也应有意识地镜像它们的语义,并说明为什么 channel 侧的白名单比 daemon 更严格。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Deferred to the follow-up queue: the drift is real and verified, but hoisting the naming/MIME/Blob/rollback helpers into a module shared by channels/base and webui needs a shared-home design decision outside this PR's footprint. This round shrank the drift deliberately — the Critical fix aligned channel-side semantics with the daemon store (image/jpg → image/jpeg alias, degrade-by-skip instead of throw).
中文说明
延迟到后续跟进队列:分叉属实且已验证,但把命名/MIME/Blob/回滚辅助函数提升到 channels/base 与 webui 共享的模块,需要一个超出本 PR 范围的"共享归属"设计决定。本轮已有意缩小分叉——Critical 修复把 channel 侧语义与 daemon 存储对齐(image/jpg → image/jpeg 别名、以跳过代替抛异常的降级)。
There was a problem hiding this comment.
Deferred to the follow-up queue. The duplication is real and verified: the channel upload cluster mirrors webui's daemon-attachment helpers (imageAttachmentName/imageAttachmentMimeType in packages/webui/src/daemon/session/actions.ts, daemonPromptImageToBlob, removeUploadedAttachments), and the deeper fix is hoisting them into a module both packages consume. That crosses package boundaries (channels/base has no webui/acp-bridge dependency) and needs a shared-home decision outside this PR's DingTalk multi-image footprint, so it is recorded in the deferred-findings queue rather than implemented here.
The finding's minimal in-scope branch is already in place at HEAD: the daemon-store mirrors are deliberate and documented — CHANNEL_IMAGE_EXTENSIONS and CHANNEL_IMAGE_MAX_UPLOAD_BYTES carry pointer comments naming SUPPORTED_IMAGE_MIME_TYPES / SESSION_ATTACHMENT_MAX_ITEM_BYTES in packages/acp-bridge/src/sessionAttachments.ts as the source of truth and explaining the pre-check (the store rejects inadmissible uploads; checking first lets one bad image degrade by omission instead of failing the turn). The sibling re-report thread focused on the pointer comment (R1-5) is resolved by that comment.
延后到后续跟进队列处理。重复实现确实存在且已核实:channel 上传集群镜像了 webui 的 daemon 附件辅助函数(packages/webui/src/daemon/session/actions.ts 中的 imageAttachmentName/imageAttachmentMimeType、daemonPromptImageToBlob、removeUploadedAttachments),更彻底的修复是把这些辅助函数提升到两个包都能消费的共享模块。但这跨越了包边界(channels/base 不依赖 webui/acp-bridge),需要一个超出本 PR(钉钉多图)范围的共享归属决策,因此记入延后队列而不是在此实现。
该发现的最小范围内分支已在 HEAD 落地:对 daemon 存储常量的镜像是有意为之且有文档说明——CHANNEL_IMAGE_EXTENSIONS 与 CHANNEL_IMAGE_MAX_UPLOAD_BYTES 都带指针注释,指明 packages/acp-bridge/src/sessionAttachments.ts 中的 SUPPORTED_IMAGE_MIME_TYPES / SESSION_ATTACHMENT_MAX_ITEM_BYTES 是权威来源,并解释了预检原因(存储会拒绝不可接收的上传;先检查才能让单张坏图以省略降级而非让整个 turn 失败)。聚焦于指针注释的兄弟线程(R1-5 重报)已因该注释而关闭。
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9922Commit: Feedback points and decisionsCritical findings (both reproduced before fixing, both resolved)
Suggestions resolved this round
Suggestions deferred to the next round (round budget ~8 findings, Criticals first)
Other items
Mutation probes (each new guard has its own witness)
Verification
中文说明Autofix 审查轮次 — PR #9922提交: 反馈点与处理决定Critical 发现(均先在修复前复现,均已解决)
本轮已解决的 Suggestion
延迟到下一轮的 Suggestion(本轮预算约 8 项,Critical 优先)
其他条目
变异探针(每个新守卫都有自己的见证)
验证
🧭 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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…jected turns - Drop image entries missing data/mimeType in resolvePromptImages so one malformed attachment degrades to a prompt without the image instead of throwing a TypeError that kills the whole turn (restores the pre-diff legacy-guard behavior for out-of-contract extension adapters). - Require the image/ MIME prefix in channelImageName so non-image types whose tail spells a known extension (audio/png) are skipped with a warning instead of tripping the daemon store's name/content-type check and failing the turn. - Roll uploaded images back when the daemon definitively rejects prompt admission (DaemonHttpError on the admission request, local prompt-queue limit), matching the webui client; admitted-turn errors and transport failures keep the uploads because the daemon may already reference them. - Upload prompt images concurrently like the webui path, preserving input order and the existing rollback contract. - Log failed attachment removals during rollback via sanitizeLogText. - Point CHANNEL_IMAGE_EXTENSIONS at the daemon store's set as source of truth; document the ordered images contract in the README.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9922Round commit: Feedback dispositions
DetailsR2-1 (Critical) — malformed image entries crashed the turnReproduced first: the new test R2-2 (Critical) — non-image MIME types passed the allow-listReproduced first: the new test with R1-3 (Critical) — orphaned uploads on prompt-admission rejectionReproduced first: two new tests showed R2-4 (Suggestion) — parallel uploadsUploads now fan out via R1-12 (Suggestion) — rollback failures loggedThe rollback's R2-3 (Suggestion) — mixed-case coverage
R1-7 (Suggestion) — README
R1-5 (Suggestion) — source-of-truth pointer
Review-level note
Conflict notesNone — Mutation probes (guard removed → witness test FAILS; restored → green)
Verification
中文说明Autofix 轮次总结 — PR #9922本轮提交: 反馈处置
详情R2-1(Critical)— 格式不完整的图片条目导致 turn 崩溃先复现:新测试 R2-2(Critical)— 非图片 MIME 类型通过了白名单先复现:使用 R1-3(Critical)— prompt 接收被拒后上传成为孤儿先复现:两个新测试显示,上传成功后 R2-4(Suggestion)— 并行上传上传现在像 webui 附件路径一样通过 R1-12(Suggestion)— 记录回滚失败回滚的 R2-3(Suggestion)— 混合大小写覆盖
R1-7(Suggestion)— README
R1-5(Suggestion)— 权威来源指针
审查层面说明
冲突说明无 — 变异探针(删除守卫 → 证据测试失败;还原 → 全绿)
验证(Verification)
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 5": executing DaemonChannelBridge.test.ts at HEAD (worktree lacks node_modules; install + build exceeded remaining budget).
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/AcpBridge.ts:287 — [review] inline image-block construction { type: 'image', data, mimeType } is pasted in both bridges (AcpBridge.ts:287-293, DaemonChannelBridge.ts:517-523); shared home exists in ChannelAgentBri…
Convergence: round 3 posted 5 inline comment(s), 5 of them reported for the first time; the previous round posted 8 (4 new). Findings keep coming back to the same files: packages/channels/base/src/DaemonChannelBridge.ts (findings in rounds 1, 2; 3 more now); packages/channels/base/src/ChannelAgentBridge.ts (findings in rounds 1, 2; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent 5":executing DaemonChannelBridge.test.ts at HEAD (worktree lacks node_modules; install + build exceeded remaining budget)。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 5 条行内评论,其中 5 条是首次提出;上一轮发布了 8 条(其中 4 条首次提出)。发现反复回到同一批文件:packages/channels/base/src/DaemonChannelBridge.ts(第 1、2 轮已出过发现,本轮又有 3 条);packages/channels/base/src/ChannelAgentBridge.ts(第 1、2 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round summary — no code change (growth-audit round)This round was triggered by the diff-growth brake: this counting window grew src +123 / test +624 net lines over its baseline (budgets 400/400). The growth audit ran first, per the window rules, and recorded Growth audit findings
Why no change this round
中文说明Autofix 轮次总结 —— 无代码改动(增长审计轮)本轮由 diff 增长刹车触发:本计数窗口相对基线净增长 源码 +123 / 测试 +624 行(预算 400/400)。按窗口规则,增长审计先行执行,结果已写入 增长审计结论
本轮为何不做改动
Deferred non-Critical feedbackCritical-only mode is active: the PR's diff grew src 123 / test 624 net lines beyond this counting window's baseline (budgets: 400/400). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback continues to flow unaffected during a growth-only engagement (the per-author batch budget applies only after 5 change-producing rounds). (
中文说明已进入仅处理 Critical 的模式:本计数窗口内 diff 净增长已达 源码 123 / 测试 624 行(预算 400/400)。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。纯增长触发期间维护者反馈照常流动(按作者的批次预算仅在完成 5 个产生改动的轮次后生效)。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round summary — PR #9922One commit this round: All 26 inline findings are resolved in code — 21 were verified as already fixed by the earlier round commits on this branch ( Addressed this round
Verified already resolved by earlier commits (re-checked against this round's head)
Mutation probes (all killed, then restored to green)
Verification
中文说明Autofix 审查轮次总结 — PR #9922本轮一个提交: 全部 26 条行内发现均已在代码中解决——其中 21 条经复核确认已由本分支早前的轮次提交( 本轮处理
已复核确认由早前提交解决(按本轮 HEAD 复核)
变异探针(全部被杀死,随后还原并复绿)
验证
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- mirrored daemon-store constants not pinned by any cross-package test — already reported as R1-5 (comment 3845700471) at packages/channels/base/src/DaemonChannelBridge.ts:147
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/DaemonChannelBridge.ts:504 — [probe] no test pins that uploads are aborted with the prompt controller's signal (surviving mutation verified by probe)packages/channels/base/src/DaemonChannelBridge.ts:54 — [test] the new uploadAttachment/removeAttachment interface members ship with no test gating (type-only, build-gated)packages/channels/base/src/DaemonChannelBridge.ts:108 — [test] the sessionAttachments capability-option declaration survives hunk-revert with every test green (type-only, build-gated; runtime branches tested)
Convergence: round 4 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/channels/base/src/DaemonChannelBridge.ts (findings in round 3; 4 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 4 条行内评论,其中 4 条是首次提出;上一轮发布了 5 条(其中 5 条首次提出)。发现反复回到同一批文件:packages/channels/base/src/DaemonChannelBridge.ts(第 3 轮已出过发现,本轮又有 4 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
…mits (#9922) - Skip channel images whose base64 decodes to zero bytes before uploading: the daemon store rejects empty images with 400, which failed the whole turn instead of degrading by omission. - Bound the inline fallback for daemons without session_attachments: apply the same per-item admission gate and keep the aggregate base64 payload under the daemon's 10mb prompt-body limit, so multi-image turns against older daemons degrade by omission instead of failing with 413. - Reject oversized images from the base64 length before allocating the decoded buffer, so several oversized images no longer allocate their full decoded payloads concurrently before being discarded. - Tag the skip log lines with the session id so operators can correlate a skipped image to its channel session.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix round summary — PR #9922Commit: Feedback points and dispositions[rc:3847894018] Critical — zero-byte decodes uploaded, daemon 400 fails the whole turn → ResolvedReproduced on [rc:3847894043] Critical — inline fallback unbounded, old-daemon turns 413 → ResolvedReproduced pre-fix: the inline fallback forwarded every image with no bound (new test with three ~3.5 MiB images showed all three forwarded — ~14 MiB of base64 against the daemon's [rc:3847894048] Suggestion — oversized images fully decoded before being discarded → Resolved
One deviation from the suggestion's wording, recorded here: it proposed keeping the decoded [rc:3847894053] Suggestion — skip logs carry no session id → ResolvedAll three skip log lines (unsupported MIME type, per-item admission skip on both paths, aggregate-budget skip) now end with Not requested this round (recorded, no action)
Changes
Mutation probes (each new guard's witness verified)
Verification
中文说明Autofix 轮次总结 — PR #9922提交: 反馈点及处置[rc:3847894018] Critical — 解码为零字节的图片被上传,daemon 返回 400 导致整个 turn 失败 → 已解决在修改任何代码之前,先在 [rc:3847894043] Critical — 内联回退无上限,旧版 daemon 上的 turn 返回 413 → 已解决修复前复现:内联回退无上限地转发所有图片(新测试用三张约 3.5 MiB 的图片,显示三张全部被转发——约 14 MiB 的 base64,对应 daemon [rc:3847894048] Suggestion — 超大图片在被丢弃前已被完整解码 → 已解决
与建议措辞有一处偏差,记录在此:建议提出在解码后保留 [rc:3847894053] Suggestion — 跳过日志不含会话 id → 已解决三条跳过日志(不支持的 MIME 类型、两条路径上的单项接纳跳过、总体积预算跳过)现在都以 本轮不要求处理(仅记录,不行动)
变更内容
变异探针(每个新守卫的见证均已验证)
验证
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- hand-maintained daemon-store mirrors (supported subtype set, 8 MiB cap, naming scheme, inline budget) not pinned by any cross-package test — already reported as R1-5 (comment 3845700471) at packages/channels/base/src/DaemonChannelBridge.ts:…
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/DaemonChannelBridge.ts:54 — [test] new uploadAttachment/removeAttachment interface members ship with no test gating (type-only, build-gated)packages/channels/base/src/DaemonChannelBridge.ts:108 — [test] sessionAttachments capability-option declaration survives hunk-revert with every test green (type-only, build-gated; runtime branches tested)
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/channels/base/src/DaemonChannelBridge.ts (findings in round 4; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 5 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 4 条(其中 4 条首次提出)。发现反复回到同一批文件:packages/channels/base/src/DaemonChannelBridge.ts(第 4 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen3.8-max via Qwen Code /review (v0.21.10)
The pre-decode size estimate subtracted trailing '=' padding even when the input length does not complete a quantum; Node's lenient base64 decoder ignores such stray padding, so malformed input decoding to the 8 MiB limit plus one byte slipped past the gate and the daemon's 413 then failed the whole turn. Subtract padding only for multiple-of-4 input so the estimate is a strict upper bound. Also give the skip reason a caller-supplied label so the inline fallback for daemons without session_attachments names the inline budget instead of an attachment-store limit those daemons do not have.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressed — PR #9922 (round 6)Commit: Feedback points and dispositions[rc:3848601570] Critical — size estimate undercounts by 1 byte on malformed padding → ResolvedThe finding is real and was reproduced at
Fix: Witness: new test [rc:3848601596] Suggestion — inline fallback blames a limit the old daemon does not have → ResolvedReproduced with a failing test at Fix: Witness: new test Recorded by the review, not requested this round — no actionThe review body lists the previously reported R1-5 finding (hand-maintained daemon-store mirrors not pinned by a cross-package test) as confirmed-but-not-repeated, and two convergence-deferred test-gating observations ( Conflict notesNone — Verification
中文说明已处理的评审反馈 — PR #9922(第 6 轮)提交: 反馈点与处理结果[rc:3848601570] Critical — 尺寸估算在畸形填充上少算 1 字节 → 已解决该发现属实,并在动手修改前已在
修复: 证人:新测试 [rc:3848601596] Suggestion — 内联回退归因于旧 daemon 并不存在的上限 → 已解决已用失败测试在 修复: 证人:新测试 评审记录在案、本轮不要求处理 — 不采取行动评审正文将此前已报告的 R1-5 发现(手工维护的 daemon 存储镜像没有跨包测试钉住)标记为"已确认但不重复发布",并将两条收敛姿态下延后的测试守卫观察( 冲突说明无—— 验证
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
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/channels/base/src/DaemonChannelBridge.ts:203 — [review] admission-rejection predicate duplicated from webui under the same name, keyed off SDK internalspackages/channels/base/src/DaemonChannelBridge.ts:563 — [review] inline fallback fully decodes every image before the budget check that only needs the string lengthpackages/channels/base/src/DaemonChannelBridge.ts:655 — [probe] rollback discards the removeAttachment boolean; a resolved false (not removed) is silently treated as successpackages/channels/base/src/DaemonChannelBridge.ts:48 — [test] uploadAttachment/removeAttachment interface additions ship with no test gating (type-only, build-gated)packages/channels/base/src/DaemonChannelBridge.ts:103 — [test] sessionAttachments capability-option declaration survives hunk-revert with every test green (type-only, build-gated)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round summary — PR #9922 (no code change)This was a growth-audit round: the counting window (baseline Growth audit verdict:
|
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round: no code changes — all open findings re-verified at HEADThis round re-verified all 32 open inline findings against the current head ( Critical findings re-verified as resolved (fixed by earlier commits)
Suggestions re-verified as resolved
Deferred to the follow-up queue
Issue-level comment
Verification
中文说明Autofix 审查轮:无代码变更 —— 所有未决发现已在 HEAD 复核完毕本轮在当前 head( 已复核为解决的 Critical 发现(由先前提交修复)
已复核为解决的 Suggestion
延后到后续跟进队列
Issue 级评论
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
What this PR does
This PR preserves every image from a DingTalk rich-text message as an ordered attachment on the same channel turn. It carries the ordered image collection through both ACP and daemon-backed sessions, persists daemon attachments for Web Shell hydration, and cleans up partial uploads if a later image fails.
It keeps the existing single-image fields as a compatibility fallback for channel adapters and bridge implementations that have not adopted the ordered collection yet. Daemon uploads also normalize parameterized image MIME types and release prompt state before best-effort rollback so an upload failure cannot wedge the session.
Why it's needed
DingTalk sends a group of pictures in one
richTextcallback. The adapter already extracted every picture download code, but only the first code was downloaded and forwarded, so the model and Web Shell silently lost the remaining images. This fixes the loss while preserving single-image behavior and attachment order.Reviewer Test Plan
How to verify
Configure a DingTalk Stream channel backed by
qwen serve, send several images together in one direct message, and ask the model how many images it received. Confirm that each picture is downloaded once, the daemon persists the same number of ordered attachments before accepting the prompt, Web Shell shows every image on the user turn, and the bot replies using all images. Also force a daemon attachment upload failure and confirm the session accepts a later prompt without retaining attachments from the failed turn.Evidence (Before & After)
Before: one DingTalk callback containing five pictures produced one daemon attachment and the model reported one visible image.
After: one real DingTalk callback containing three pictures produced three HTTP 201 attachment uploads, three persisted attachment references, three Web Shell images, a completed
qwen3.8-maxturn that identified three images, and a reply visible in DingTalk.Tested on
Environment (optional)
Local development daemon with DingTalk Stream and the loopback Web Shell; Node.js v24.13.0.
Risk & Scope
Linked Issues
Fixes #9878
中文说明
本 PR 做了什么
本 PR 将钉钉富文本消息中的每张图片按原顺序保留为同一个 Channel turn 的附件,并让有序图片集合同时贯通 ACP 与 daemon session。daemon 附件会持久化供 Web Shell 恢复展示;如果后续某张图片上传失败,之前已上传的附件会被清理。
现有单图字段继续作为兼容回退,尚未采用有序图片集合的 Channel adapter 和 bridge 不受影响。daemon 上传还会规范化带参数的图片 MIME 类型,并在尽力回滚附件前释放 prompt 状态,避免上传失败永久卡住 session。
为什么需要
钉钉会把一次发送的多张图片放在同一个
richTextcallback 中。适配器虽然已经提取全部图片下载码,但此前只下载并转发第一个,因此模型和 Web Shell 会静默丢失其余图片。本修复在保持单图行为和附件顺序的同时解决该问题。Reviewer 测试计划
如何验证
配置由
qwen serve承载的钉钉 Stream Channel,在私聊中一次发送多张图片,并询问模型收到了几张。确认每张图片只下载一次,daemon 在接受 prompt 前持久化相同数量且顺序一致的附件,Web Shell 在用户 turn 中展示全部图片,机器人基于全部图片回复。另模拟 daemon 附件上传失败,确认该 session 随后仍能接受新 prompt,并且失败 turn 不会残留附件。前后证据
修复前:包含五张图片的一个钉钉 callback 只产生一个 daemon 附件,模型报告只能看到一张图片。
修复后:真实钉钉 callback 中的三张图片产生三次 HTTP 201 附件上传、三个持久化附件引用和三张 Web Shell 图片;
qwen3.8-maxturn 正常完成并识别出三张图片,钉钉客户端收到回复。测试平台
环境(可选)
本地开发 daemon、钉钉 Stream、loopback Web Shell;Node.js v24.13.0。
风险与范围
关联 Issue
Fixes #9878