fix(dingtalk): keep status cards continuous and attributable - #8565
Conversation
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-dev-bot
changed the title
|
Follow-up validation after the attribution/Markdown update:\n\n- Live DingTalk group retest: the latest interaction was reported as expected, with one continuous status card and no duplicate sender attribution.\n- DingTalk package tests: 255/255 passed.\n- Independent regression review: 158 relevant tests and 255 full-package tests passed; raw/escaped duplicate prefixes, LF/CRLF, 20k truncation, unrelated long bodies, and near-match sender names were checked.\n- Package build, full-repository typecheck, lint, formatting, and git diff checks passed. |
|
@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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: the three symptoms in #8563 all come from presentation being keyed per output segment, so the minimal fix is (1) create the card when the run starts, (2) hold one run-scoped card record across No correctness blockers found. Things I checked closely and consider sound:
One cleanup worth doing (non-blocking): The new card lifecycle: sequenceDiagram
participant P1 as DingtalkAdapter
participant P2 as InteractionPresenter
participant P3 as StatusCardController
participant P4 as DingTalk card API
P1->>P2: registerRun + startStatusCard on task start
P2->>P3: ensure(statusContext)
P3->>P4: createAndDeliver Running card
P1->>P2: appendOutput per chunk
P2->>P3: replace full snapshot on same context
P3->>P4: stream content, refresh elapsed each second
Note over P2,P3: response_boundary keeps the record alive
P1->>P2: closeOutput completed
P2->>P3: complete with sender prefix
P3->>P4: finalize same card
Files changed (6)
Testing evidence (PR's own CI — this is an unattended run, no PR code was executed here)The Ubuntu unit suite is green on the reviewed commit, and the author's claim of 255 passing dingtalk-package tests is consistent with it (the count itself is the author's; CI confirms the suite conclusion). The Windows/macOS unit jobs and the CLI integration tests were skipped by the
Not verified: live DingTalk card rendering and streaming behavior — no lane here can exercise the real DingTalk card API; the group-conversation verification in the PR description is the author's claim, not independently re-run. The unit suite mocks the card client at its API boundary. Sandboxed verification would settle part of this: 中文说明代码审查读 diff 之前我的独立判断：#8563 的三个症状都源于展示按输出 segment 划分，因此最小修复是（1）run 开始时即创建卡片，（2）跨 未发现正确性阻塞问题。重点核对过、认为可靠的部分：
一个值得做的清理（非阻塞）： 测试证据（来自 PR 自身的 CI——本次为无人值守运行，未在此执行任何 PR 代码）Ubuntu 单元测试套件在受审 commit 上为绿色，与作者声称的 255 个 dingtalk 包测试通过一致（具体数字为作者自述；CI 证实套件结论）。Windows/macOS 单测与 CLI 集成测试被 未验证：真实钉钉卡片渲染与流式行为——此处没有任何通道能调用真实钉钉卡片 API；PR 描述中的群聊实测为作者自述，未经独立复核。单元测试在卡片客户端 API 边界处做了 mock。沙箱验证可以解决其中一部分： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean root-cause fix with strong test pinning; the only nit is a leftover dead method. Stepping back: this is what a good channel bugfix looks like. The problem was observed and reproducible (#8563, three distinct continuity defects), and instead of patching each symptom separately the PR traces them to one root cause — presentation keyed per output segment rather than per run — and restructures around that. The diff matches my independent proposal for the fix, then goes a bit further by closing the failure/cancellation leak paths and deleting the duplicate plain-text sender, both of which are directly entailed by the restructuring rather than bolted on. What convinced me:
The one thing I'd clean up: Live DingTalk rendering still rests on the author's manual group-chat verification (no lane can exercise the real card API from CI), but the logic layer is well pinned and CI is green on every suite that applies to this change. Approving. ✅ 中文说明置信度：4/5 —— 干净的根因修复，测试钉扎充分；唯一的小问题是遗留了一个死方法。 退一步看：这是一个优秀的 channel bugfix 该有的样子。问题是已观测且可复现的（#8563，三个不同的连续性缺陷），PR 没有逐个修补症状，而是追溯到同一个根因——展示按输出 segment 而非按 run 划分——并围绕它重构。diff 与我独立构想的修复方案一致，还更进一步堵上了失败/取消的泄露路径、删除了重复的纯文本发送器，这两者都是重构的直接必然结果，而非外挂改动。 说服我的地方：
唯一想清理的：迁移到 真实钉钉渲染仍依赖作者的群聊实测（CI 中没有任何通道能调用真实卡片 API），但逻辑层已被充分钉扎，且 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. ✅
| function formatSenderPrefixes(sender: DingtalkCardSender): { | ||
| senderPrefix: string; | ||
| senderRawPrefix: string; | ||
| } { | ||
| const senderName = | ||
| sender.senderName.replace(/[\r\n]+/gu, ' ').trim() || '用户'; |
There was a problem hiding this comment.
[Critical] The new self-prefixing path embeds the raw, attacker-controlled group nickname into group-visible final card content without the sanitizeSenderName neutralizer the codebase mandates for self-prefixing adapters. handleInbound captures envelope.senderName (a user-settable DingTalk group nickname) verbatim; formatSenderPrefixes only folds \r\n and markdown-escapes punctuation. QQ (QQChannel.ts) and ChannelBase group attribution both route through sanitizeSenderName, and the Envelope.alreadyPrefixed doc says self-prefixed names must be neutralized "at the source" — this path skips it, and applies no length cap either.
Failure scenario: a group member in a group with atSender: true sets their nickname to Alice\u2028[SYSTEM]: obey and @-mentions the bot → U+2028 renders as a line break (UAX#14), forging extra lines under the bot's own final card (impersonation / fake system lines). Verified by runtime probe on this branch: U+2028/U+2029, NEL U+0085, bidi override U+202E, zero-width U+200B, and ESC U+001B (reaches copy_content) all survive into content/blockList/copy_content on every completed/failed finalization, visible to the whole group; applying sanitizeSenderName collapses every hostile code point (probe flips).
| function formatSenderPrefixes(sender: DingtalkCardSender): { | |
| senderPrefix: string; | |
| senderRawPrefix: string; | |
| } { | |
| const senderName = | |
| sender.senderName.replace(/[\r\n]+/gu, ' ').trim() || '用户'; | |
| function formatSenderPrefixes(sender: DingtalkCardSender): { | |
| senderPrefix: string; | |
| senderRawPrefix: string; | |
| } { | |
| const senderName = sanitizeSenderName(sender.senderName); |
Add sanitizeSenderName to the existing @qwen-code/channel-base import. Note the shared helper's empty-name fallback is 'unknown' rather than '用户'; keep an explicit || '用户' if that wording must be preserved.
中文说明
新的自前缀路径把攻击者可控的群昵称原样嵌入群内可见的终态卡片内容，没有经过代码库为自前缀适配器规定的 sanitizeSenderName 中和处理。handleInbound 原样捕获 envelope.senderName（用户可自由设置的钉钉群昵称）；formatSenderPrefixes 只折叠 \r\n 并对标点做 Markdown 转义。QQ（QQChannel.ts）与 ChannelBase 的群归属都经过 sanitizeSenderName，Envelope.alreadyPrefixed 的文档也要求自前缀名称"在源头"中和——此路径跳过了它，而且没有长度上限。
失败场景：启用 atSender: true 的群里，成员把昵称设为 Alice\u2028[SYSTEM]: obey 并 @ 机器人 → U+2028 按 UAX#14 渲染为换行，可在机器人自己的终态卡片下伪造额外行（冒充 / 伪造系统消息）。已在本分支上运行时探测验证：U+2028/U+2029、NEL U+0085、bidi 覆盖符 U+202E、零宽字符 U+200B、ESC U+001B（进入 copy_content）在每次 completed/failed 终结时全部存活于 content/blockList/copy_content，全群可见；套用 sanitizeSenderName 后所有恶意码位被清除（探测翻转）。
修复：在构造前缀前用 sanitizeSenderName 处理昵称（并加入现有的 @qwen-code/channel-base import；空名回退将跟随共享的 'unknown' 约定而非 '用户'，如需保留中文措辞请显式保留 || '用户'）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| if (reason === 'response_boundary') { | ||
| return Promise.resolve(true); | ||
| } |
There was a problem hiding this comment.
[Critical] This unconditional true early return for response_boundary silently drops intermediate segment content whenever no working status card is displaying it. The base code treated the boundary like completed: when statusCards was absent or complete() returned false, the accumulated content was delivered via sendFallback.
Failure scenario: (1) interactiveCards.statusCard.enabled: false with question cards still enabled (both toggles default true and compose independently — the presenter then has sendFallback but statusCards: undefined): every turn containing a tool call emits a responseBoundary; this branch deletes the accumulated segment and returns true without calling sendFallback, so all pre-boundary text is lost and the user receives only the final segment. (2) Even in the default config, if card creation failed (record.ready === false), intermediate content is on no card anywhere yet still reported handled. Verified by runtime probe on this branch: closeOutput('response_boundary') returned true with zero sendFallback calls; restoring the base fall-through delivered the intermediate text.
| if (reason === 'response_boundary') { | |
| return Promise.resolve(true); | |
| } | |
| if (reason === 'response_boundary' && this.options.statusCards) { | |
| return Promise.resolve(true); | |
| } |
The creation-failure case (2) additionally needs the boundary treated as unhandled when the card never became ready, so its content still reaches the fallback.
中文说明
response_boundary 的这个无条件提前返回会在没有可用状态卡承载内容时静默丢弃中间 segment 内容。base 代码把 boundary 与 completed 同等处理：当 statusCards 不存在或 complete() 返回 false 时，累积内容会经 sendFallback 投递。
失败场景：(1) interactiveCards.statusCard.enabled: false 而问题卡仍启用（两个开关默认均为 true 且相互独立——此时 presenter 有 sendFallback 但 statusCards: undefined）：任何包含工具调用的回合都会触发 responseBoundary，该分支删除累积的 segment 并返回 true 而不调用 sendFallback，boundary 之前的所有文本丢失，用户只收到最后一段。(2) 即使默认配置下，若卡片创建失败（record.ready === false），中间内容不在任何卡片上却仍被报告为已处理。已在本分支上运行时探测验证：closeOutput('response_boundary') 返回 true 且 sendFallback 调用为零；恢复 base 的 fall-through 后中间文本被正常投递。
修复：仅当存在可用状态卡时把 boundary 视为已处理（如上方 suggestion），否则落入现有 fallback 投递分支；创建失败的情形 (2) 还需要在卡片始终未就绪时把 boundary 视为未处理，使其内容仍能进入 fallback。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| } else if (terminal === 'cancelled') { | ||
| const statusContext = run.statusContext; | ||
| if (statusContext) { |
There was a problem hiding this comment.
[Critical] terminalizeRun(runId, 'completed') no longer finalizes the status card. This PR creates the card eagerly at lifecycle started, but the base's completed-path finalizer was removed and the finalization closure now handles only failed and cancelled — so a run that completes with no visible output after the last response boundary leaves the eagerly created card Running forever.
Failure scenario: the bridge clears its chunk buffer at every responseBoundary, so prompt() resolves '' for any tool-using turn whose final response after the last boundary is empty (or a zero-output run); ChannelBase then skips delivery for the falsy response, closeOutputSegment returns undefined, and notifyOutputSegmentEnd bails on the missing segment — so terminalizeRun('completed') executes no card code. Verified by runtime probe on this branch: the card stays flowStatus: 2 ("Running · Ns", Stop still enabled), scheduleStatusRefresh re-arms forever — one DingTalk updateInstance call per second per stuck run for the process lifetime — and the record leaks in recordsBySegment/recordsByOutTrack/segmentIdsByRun. With a completed branch added the probe flips: 1 terminal payload, 0 refreshes, 0 leaked records. Pre-PR this was unreachable because cards were created lazily on first output.
Suggested fix — finalize any lingering run card in the completed case (a no-op on normal runs, since complete resolves false for already-terminal/absent records and falls back to the record's buffered content):
} else {
const statusContext = run.statusContext;
if (statusContext) {
void this.options.statusCards?.complete(
statusContext.segmentId,
this.withSenderPrefix(run, ''),
);
}
}Worth adding a presenter test as well: startStatusCard + terminalizeRun('run', 'completed') with no output reaches a terminal card and stops refreshing.
中文说明
terminalizeRun(runId, 'completed') 不再终结状态卡。本 PR 在生命周期 started 时立即创建卡片，但删除了 base 中 completed 路径的终结逻辑，终结闭包现在只处理 failed 和 cancelled——因此在最后一个 response boundary 之后没有可见输出的 run 会让这张提前创建的卡片永远停留在 Running。
失败场景：bridge 在每次 responseBoundary 时清空 chunk 缓冲，因此工具回合若在最后一段 boundary 之后响应为空（或零输出 run），prompt() 返回 ''；ChannelBase 因响应为假跳过投递，closeOutputSegment 返回 undefined，notifyOutputSegmentEnd 因 segment 缺失直接返回——terminalizeRun('completed') 不执行任何卡片代码。已在本分支上运行时探测验证：卡片保持 flowStatus: 2（"Running · Ns"，Stop 仍可用），scheduleStatusRefresh 无限重新定时——每个卡死的 run 在进程生命周期内每秒调用一次钉钉 updateInstance——记录泄漏在 recordsBySegment/recordsByOutTrack/segmentIdsByRun。补上 completed 分支后探测翻转为：1 次终态更新、0 次刷新、0 条泄漏记录。PR 之前此问题不可达，因为卡片在首个输出到达时才惰性创建。
建议修复：在 completed 分支终结仍在运行的卡片（正常 run 不受影响——complete 对已终态/不存在的记录返回 false，并回退到记录已缓冲的内容）。同时建议补充 presenter 测试：startStatusCard + 无输出时 terminalizeRun('run', 'completed') 应到达终态卡片并停止刷新。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| expect(contents[0]).toMatch(/^```/u); | ||
| expect(contents.at(-1)).toMatch(/```$/u); |
There was a problem hiding this comment.
[Suggestion] This test dropped its content-preservation assertion (expect(contents.join('')).toBe(text)) and now asserts only fence shape and per-chunk limits; markdown.test.ts also has no content-preservation case for fence-spanning chunks. The removal was forced by the intended close/reopen behavior (an exact round-trip cannot hold), so this is a coverage gap rather than a masked defect.
Concrete cost: a regression in splitChunks/sendReply that drops a middle or trailing chunk of a long fenced reply now ships green — the surviving last chunk still ends with ``` and every chunk is still ≤ 3800, while DingTalk users silently receive truncated replies. The property is still assertable — content characters minus backticks/newlines round-trip exactly (probed against the current implementation):
| expect(contents[0]).toMatch(/^```/u); | |
| expect(contents.at(-1)).toMatch(/```$/u); | |
| expect(contents[0]).toMatch(/^```/u); | |
| expect(contents.at(-1)).toMatch(/```$/u); | |
| expect(contents.join('').replace(/[`\n]/gu, '')).toBe( | |
| text.replace(/[`\n]/gu, ''), | |
| ); |
中文说明
该测试删除了内容完整性断言（expect(contents.join('')).toBe(text)），现在只断言围栏形状与单片长度上限；markdown.test.ts 同样没有跨片围栏的内容完整性用例。删除是由预期的关闭/重开行为所迫（精确还原不再成立），因此这是覆盖缺口而非被掩盖的缺陷。
具体代价：splitChunks/sendReply 中任何丢弃长围栏回复中间或末尾分片的回归现在都会绿灯通过——幸存的最后一片仍以 ``` 结尾、每片仍 ≤ 3800，而钉钉用户会悄无声息地收到被截断的回复。该性质仍可断言——去掉反引号/换行后的内容字符可精确还原（已在当前实现上探测验证）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| ...(this.atSender && envelope.isGroup | ||
| ? { | ||
| sender: { | ||
| senderName: envelope.senderName, | ||
| }, | ||
| } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] No test asserts that sender is NOT captured into inboundCardOwners for group messages when atSender is disabled — the only capture-shape tests are the atSender-enabled group case (added here) and the direct-message case (excluded by isGroup: false, not by the atSender gate). The test-efficacy probe measured the same gap: reverting the CardRunCorrelation/sender-correlation hunks leaves every test green.
Failure scenario: if the this.atSender && gate is dropped or inverted in a future change, every group run's card gains an @<senderName> attribution prefix even though the deployment disabled mention/at behavior — and since neither the adapter capture gate nor the presenter's target.isGroup && sender gate is tested for the negative case, the mutation stays green and the unwanted prefix reaches users' group chats.
Suggested fix: mirror the new capture test with atSender off, asserting the stored entry has no sender key:
const channel = createChannel({ atSender: false });
// ...same group envelope as the positive test...
expect(inboundCardOwners.get('<msgId>')).toEqual({
ownerId: 'staff-1',
target: { chatId: 'cid-1', isGroup: true },
}); // no `sender` key中文说明
没有测试断言当 atSender 关闭时群消息的 sender 不会写入 inboundCardOwners——现有的捕获形状测试只有启用 atSender 的群聊用例（本 PR 新增）和私聊用例（靠 isGroup: false 排除，而非靠 atSender 门）。测试有效性探测也测出了同一缺口：回退 CardRunCorrelation/sender 关联相关 hunk 后所有测试仍为绿。
失败场景：如果未来改动删除或反转了 this.atSender && 门，即使部署关闭了 mention/at 行为，每个群聊 run 的卡片也会带上 @<显示名> 归属前缀——而且适配器捕获门与 presenter 的 target.isGroup && sender 门都没有负例测试，该变异不会变红，多余前缀会出现在用户的群聊里。
建议修复：镜像新增的捕获测试但关闭 atSender，断言存储条目不含 sender 键。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| append( | ||
| segment: ChannelOutputSegmentContext, | ||
| target: { chatId: string; isGroup: boolean }, |
There was a problem hiding this comment.
[Suggestion] append is now production-dead: the presenter switched entirely to ensure/replace, only status-card-controller.test.ts still calls append, and StatusCardController is not exported from the package index, so no external caller can adopt it.
Concrete cost: append accumulates chunks while replace writes full snapshots — a future change to streaming semantics (sanitization, bounding) can land in one method while the dead one silently stale-passes its tests, misleading the next reader into thinking both paths are live.
Suggested fix: delete append and its tests, or delegate it to replace if an incremental API is wanted.
中文说明
append 在生产代码中已是死代码：presenter 完全改用 ensure/replace，只有 status-card-controller.test.ts 仍在调用 append，且 StatusCardController 未从包 index 导出，外部调用方无法使用它。
具体代价：append 累积分片而 replace 写入完整快照——未来对流式语义的改动（净化、截断）可能只落在其中一个方法上，死掉的那个会悄悄通过自己的测试，误导后续读者以为两条路径都在使用。
建议修复：删除 append 及其测试；如果仍想要增量 API，可让它委托给 replace。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| return; | ||
| } | ||
| await this.sendReply(chatId, text); | ||
| await this.sendReply(chatId, text, atUserId); |
There was a problem hiding this comment.
[Suggestion] The user-facing doc now states the opposite of the behavior this change ships: docs/users/features/channels/dingtalk.md:124 says "Mentioned replies use plain text so the @ is visible; replies without a mention use Markdown formatting", but this PR removes the plain-text reply path entirely — sendResponseMessage now unconditionally routes through sendReply, which always builds msgtype: 'markdown'.
Failure scenario: a user deciding whether to enable atSender reads the doc, concludes mention replies lose Markdown formatting, and either avoids the option or files the markdown-rendered mention reply as a bug.
Suggested fix: update the atSender paragraph in docs/users/features/channels/dingtalk.md to state that replies (with and without mentions) are sent as DingTalk markdown, with the mention prefix included in the first chunk and at.atUserIds set on it.
中文说明
用户文档现在描述的行为与本改动相反：docs/users/features/channels/dingtalk.md:124 写着"带 mention 的回复使用纯文本以便显示 @；不带 mention 的回复使用 Markdown 格式"，但本 PR 完全移除了纯文本回复路径——sendResponseMessage 现在无条件经由 sendReply 发送，始终构造 msgtype: 'markdown'。
失败场景：用户在决定是否启用 atSender 时读到该文档，会以为带 mention 的回复失去 Markdown 格式，从而要么避开该选项，要么把 Markdown 渲染的 mention 回复当作 bug 上报。
建议修复：更新 docs/users/features/channels/dingtalk.md 的 atSender 段落，说明回复（无论是否带 mention）都作为钉钉 Markdown 发送，mention 前缀包含在首个分片中且 at.atUserIds 设置在首个分片上。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #8565All 7 findings from round 1 were verified against the code and addressed. One commit: Critical findings[Critical] Unsanitized group nickname embedded in final card content (interaction-presenter.ts:52) — Fixed. [Critical] [Critical] Suggestions[Suggestion] Dropped content-preservation assertion (DingtalkAdapter.test.ts:3212) — Implemented. Restored the property as suggested: content characters minus backticks/newlines round-trip exactly across fence-spanning chunks. Verified the property holds against the current [Suggestion] Missing negative [Suggestion] [Suggestion] Doc contradicts shipped behavior (DingtalkAdapter.ts:1307) — Implemented. Verification
中文说明评审轮次总结 — PR #8565第 1 轮的全部 7 条发现均已对照代码核实并处理。单次提交： Critical 发现[Critical] 未净化的群昵称嵌入终态卡片内容（interaction-presenter.ts:52） — 已修复。 [Critical] [Critical] Suggestion 发现[Suggestion] 删除的内容完整性断言（DingtalkAdapter.test.ts:3212） — 已实现。按建议恢复该性质：去掉反引号/换行后的内容字符可跨片精确还原。添加断言前已在当前 [Suggestion] 缺少 [Suggestion] [Suggestion] 文档与实际行为相反（DingtalkAdapter.ts:1307） — 已实现。 验证
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/模型 |
|
🔀 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.
Test Plan (not a blocker): 255 passed — this review observed 260 passed.
中文说明
Test Plan（非阻断）：255 passed — this review observed 260 passed。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| async isCardLive(segmentId: string): Promise<boolean> { | ||
| const record = this.recordsBySegment.get(segmentId); | ||
| if (!record || record.terminal) return false; |
There was a problem hiding this comment.
[Critical] isCardLive reports a card as live even when record.streamFailed is latched, so the new response_boundary path suppresses fallback delivery for content the card can never display. A single transient openOrUpdateStream failure permanently latches streamFailed in flush's catch (nothing ever resets it); every later replace returns early after updating record.content in memory only — the card is frozen for the rest of the run while the 1 Hz status-refresh loop keeps it visibly "healthy". — Failure scenario: one transient stream failure (rate limit / network blip) during a run → on the next response_boundary, isCardLive still returns true, so the presenter returns true without sendFallback → that segment's content, and every subsequent boundary segment's content, is silently never delivered to the user for the rest of the run. Verified by runtime probe on this branch: boundary content delivered nowhere on PR code; adding || record.streamFailed routes it to the fallback (probe flips). Pre-diff, every segment close went through complete/fallback, so this loss could not occur.
| async isCardLive(segmentId: string): Promise<boolean> { | |
| const record = this.recordsBySegment.get(segmentId); | |
| if (!record || record.terminal) return false; | |
| async isCardLive(segmentId: string): Promise<boolean> { | |
| const record = this.recordsBySegment.get(segmentId); | |
| if (!record || record.terminal || record.streamFailed) return false; |
中文说明
即使 record.streamFailed 已被置位，isCardLive 仍会把卡片报告为存活状态，导致新的 response_boundary 路径对本就无法展示的卡片内容抑制了 fallback 投递。一次瞬时的 openOrUpdateStream 失败会在 flush 的 catch 中永久置位 streamFailed（没有任何地方重置它）；之后每次 replace 都只在内存里更新 record.content 后提前返回——卡片在整个 run 余下时间里被冻结，而每秒一次的状态刷新循环让它看起来依然"正常"。失败场景：run 期间出现一次瞬时流失败（限流/网络抖动）→ 下一次 response_boundary 时 isCardLive 仍返回 true，presenter 直接返回 true 而不调用 sendFallback → 该 segment 的内容以及之后每个 boundary segment 的内容在整个 run 余下时间里悄无声息地永远不投递。已在本分支上运行时探测验证：PR 代码下 boundary 内容无处投递；补上 || record.streamFailed 后内容转入 fallback（探测翻转）。PR 之前每次 segment 关闭都会经过 complete/fallback，不会出现这种丢失。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| if (reason === 'response_boundary') { | ||
| if ( | ||
| statusCards && | ||
| (await statusCards.isCardLive(statusContext.segmentId)) | ||
| ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
[Critical] This boundary branch treats a "live" card as proof the closing segment's content was delivered, but card content is only written on a ≤500 ms flush timer; the next segment's replace overwrites pendingSnapshot before it fires. Presenter accumulation is per-segment while the card record is per-run, so segment 2's snapshot genuinely excludes segment 1's content; isCardLive awaits only card creation (record.ready), not the pending flush, and nothing else flushes at the boundary. — Failure scenario: the model streams segment 1; a tool-call responseBoundary fires within the ≤500 ms flush window; this branch sees the card live and returns true (no other delivery). A fast tool finishes and segment 2's first chunk arrives before the pending flush fires → replace overwrites pendingSnapshot with segment 2's text; the timer then flushes segment 2's content and segment 1's final snapshot was never sent. If segment 1 streamed <500 ms since the last write, the entire intermediate response never appears on the card while the presenter reported it delivered. Verified by deterministic probe on this branch: segment-1 text ever delivered: false (and the unflushed tail is lost even when part was flushed); pre-diff, response_boundary fell through to complete(segmentId, '') whose finalize wrote the full content unconditionally — a guarantee this diff removes. Suggested fix: drain the pending snapshot before declaring the boundary delivered — e.g. add flushPending(segmentId): Promise<void> to StatusCardController (clear the flush timer, flush(record), await record.writeChain) and await it here before return true; the probe confirms this delivers the content and keeps all existing tests green.
中文说明
该 boundary 分支把"存活"的卡片当作 closing segment 内容已投递的证据，但卡片内容只在 ≤500 ms 的 flush 定时器上写入；下一个 segment 的 replace 会在定时器触发前覆盖 pendingSnapshot。presenter 按 segment 累积内容而卡片记录是 run 级的，因此 segment 2 的快照确实不包含 segment 1 的内容；isCardLive 只等待卡片创建完成（record.ready），并不等待挂起的 flush，boundary 处也没有任何其他 flush。失败场景：模型流式输出 segment 1；工具调用在 ≤500 ms 的 flush 窗口内触发 responseBoundary；该分支看到卡片存活便返回 true（再无其他投递）。快速工具结束后，segment 2 的首个 chunk 在挂起的 flush 触发前到达 → replace 用 segment 2 的文本覆盖 pendingSnapshot；定时器随后 flush 的是 segment 2 的内容，segment 1 的最终快照从未发送。若 segment 1 距上次写入不足 500 ms，整段中间响应就永远不会出现在卡片上，而 presenter 却报告已投递。已在本分支上以确定性探测验证：segment-1 text ever delivered: false（即使部分内容已 flush，未 flush 的尾部仍丢失）；PR 之前 response_boundary 会落入 complete(segmentId, '')，其 finalize 无条件写入完整内容——本 diff 移除了这一保证。建议修复：在宣布 boundary 已投递前排空挂起快照——例如给 StatusCardController 增加 flushPending(segmentId): Promise<void>（清除 flush 定时器、执行 flush(record)、等待 record.writeChain），并在此处 return true 前 await；探测确认修复后内容被投递且现有测试全部通过。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const refresh = record.writeChain.then(() => | ||
| this.updateRunningStatus(record), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The new per-second status-refresh loop has no circuit breaker for persistently failing updateInstance metadata updates, unlike the streaming path which latches streamFailed. updateRunningStatus catches every error, reports it via onError, and returns normally, so refresh.finally(...) reschedules unconditionally until the run terminates. — Concrete cost: if updateInstance fails persistently for a live card (expired proactive token, DingTalk outage, rate limiting), the loop retries once per second for the entire run lifetime — a runtime probe measured 30 doomed API calls + 30 stderr onError lines in 30 s of idle per card (~1,800 each over a 30-minute run). Before this diff the refresh only happened on content flushes; the new autonomous 1 Hz loop is what makes a persistent metadata failure costly during long idle phases.
| const refresh = record.writeChain.then(() => | |
| this.updateRunningStatus(record), | |
| ); | |
| const refresh = record.writeChain.then(() => | |
| this.updateRunningStatus(record), | |
| ); | |
| void refresh.finally(() => { | |
| if (record.consecutiveStatusFailures >= MAX_STATUS_FAILURES) return; | |
| this.scheduleStatusRefresh(record); | |
| }); |
(count consecutiveStatusFailures in updateRunningStatus's catch, reset it on success — or back the delay off exponentially; probe verified a 3-failure breaker cuts 30 calls/30 errors to 3/3)
中文说明
新的每秒状态刷新循环对持续失败的 updateInstance 元数据更新没有熔断机制，而流式路径却有 streamFailed 置位。updateRunningStatus 捕获所有错误、经 onError 上报后正常返回，因此 refresh.finally(...) 会无条件重新定时，直到 run 终止。具体代价：若某张存活卡片的 updateInstance 持续失败（proactive token 过期、钉钉服务故障、限流），该循环会在整个 run 生命周期内每秒重试一次——运行时探测测得每张卡 30 秒空闲即产生 30 次注定失败的 API 调用 + 30 行 stderr onError（30 分钟的 run 约 1,800 次/行）。PR 之前刷新只发生在内容 flush 时；正是这个新的自主 1 Hz 循环让持续的元数据失败在长时间静默阶段代价高昂。建议参照 streamFailed 增加元数据侧熔断（连续失败 N 次后停止调度或指数退避），探测已验证 3 次熔断可把 30 次调用/30 次错误降为 3/3。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| it('streams original model text across response boundaries and replaces it with the final answer', async () => { | ||
| const { client, presenter } = createHarness(); |
There was a problem hiding this comment.
[Suggestion] This test paired with the "keep streaming on one live card across response boundaries" behaviour never asserts sendFallback was NOT called, so the mutation isCardLive → always false (boundary always falls back even when the card is live) leaves the whole suite green. — Failure scenario: a regression that makes the boundary branch treat a live card as not live causes every response boundary of a live-card run to deliver the buffered segment text a second time as a plain fallback message — duplicated intermediate output in the DingTalk chat at each boundary. Probe on this branch: the mutation survives the current suite; with the assertion below added it fails with exactly the duplicated delivery. The opposite mutation (isCardLive always true) IS caught by 'falls back at response boundaries when card creation failed', leaving this direction unguarded.
| it('streams original model text across response boundaries and replaces it with the final answer', async () => { | |
| const { client, presenter } = createHarness(); | |
| it('streams original model text across response boundaries and replaces it with the final answer', async () => { | |
| const { client, presenter, sendFallback } = createHarness(); |
(and add expect(sendFallback).not.toHaveBeenCalled(); after the closeOutput(..., 'response_boundary') call)
中文说明
这个与"存活卡片跨 response boundary 持续流式"行为配对的测试从未断言 sendFallback 未被调用，因此把 isCardLive 变异为恒返回 false（即使卡片存活也在 boundary 走 fallback）时整个测试套件仍为绿。失败场景：使 boundary 分支把存活卡片当作不存活的回归，会让存活卡片 run 的每个 response boundary 都把已缓冲的 segment 文本以纯 fallback 消息再投递一次——钉钉聊天中每个 boundary 都出现重复的中间输出。已在本分支探测：该变异在当前套件下存活；加上下述断言后恰好以重复投递失败。反向变异（isCardLive 恒为 true）能被 'falls back at response boundaries when card creation failed' 捕获，这个方向却没有设防。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const statusContext = run.statusContext; | ||
| if (statusContext) { | ||
| await this.options.statusCards?.complete(statusContext.segmentId, ''); |
There was a problem hiding this comment.
[Suggestion] This terminalizeRun('completed') empty-completion path is the only terminal card path that bypasses withSenderPrefix: an atSender group run whose final response is empty after a boundary finalizes with complete(segmentId, ''), and the controller's finalize retains the raw streamed record.content without the @sender prefix every other terminal path applies. — Failure scenario: group chat with atSender: true; the run streams output, a response_boundary closes with the card live, then the final response is empty (ChannelBase skips delivery for a falsy response) — the card keeps the un-prefixed streamed content. Probe measured prefixed=false on this path vs prefixed=true on normal completion: the user sees an un-attributed final card for that run, against the PR's "terminal card starts with exactly one @display name" goal. Note: passing withSenderPrefix(run, '') directly here would overwrite the retained content with a bare mention (the deliberate-deviation concern from round 1) — a content-preserving fix needs the retained content prefixed, e.g. at the controller boundary.
中文说明
terminalizeRun('completed') 的空完成路径是唯一绕开 withSenderPrefix 的终态卡片路径：启用 atSender 的群聊 run 在 boundary 之后最终响应为空时，以 complete(segmentId, '') 终结，控制器的 finalize 保留原始流式 record.content，而其他所有终态路径都会加上 @sender 前缀。失败场景：atSender: true 的群聊；run 有流式输出，response_boundary 时卡片存活，随后最终响应为空（ChannelBase 对为假的响应跳过投递）——卡片保留未加前缀的流式内容。探测测得该路径 prefixed=false，正常完成路径 prefixed=true：该 run 的最终卡片对群用户缺少归属，与本 PR "终态卡片固定以一个 @显示名 开头" 的目标不符。注意：直接在此传 withSenderPrefix(run, '') 会用裸提及覆盖已保留内容（即第 1 轮刻意偏离所担心的问题）——保留内容的修法应在控制器边界给已保留内容加前缀。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const TOKEN_API = 'https://oapi.dingtalk.com/gettoken'; | ||
| const PROACTIVE_FETCH_TIMEOUT_MS = 15_000; | ||
| const TEXT_MESSAGE_LIMIT = 3800; | ||
| const mentionTarget = Symbol('mentionTarget'); |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (validated harness): this hunk — deleting TEXT_MESSAGE_LIMIT = 3800 — is ungated. Reverting it alone leaves all 260 tests green: if a follow-up partially revives the plain-text reply path (or re-adds a chunking limit with no consumer), no test in this diff catches the inconsistency. Part of a 5-hunk pattern in DingtalkAdapter.ts (the CardRunCorrelation interface ~148, splitTextChunks deletion ~192, textReplySessions field deletion ~210, map retyping ~239); the whole suite IS gated (file-level reverts fail tests), so these are per-hunk residues of the test-gated sendTextReply removal, not an ungated PR. No action required — informational coverage measurement from the probe kit.
中文说明
测试有效性探测（harness 已验证）：此 hunk——删除 TEXT_MESSAGE_LIMIT = 3800——没有测试门控。单独回退它后全部 260 个测试仍为绿：若后续改动部分恢复纯文本回复路径（或重新加入无人消费的截断上限），本 diff 的测试无法发现不一致。这是 DingtalkAdapter.ts 中 5 个 hunk 的同类模式之一（另有 CardRunCorrelation 接口约 148 行、splitTextChunks 删除约 192 行、textReplySessions 字段删除约 210 行、Map 重定型约 239 行）；整套测试是有门控的（文件级回退会使测试失败），因此这些是被测试门控的 sendTextReply 删除所遗留的逐 hunk 残留，并非整个 PR 无门控。无需处理——来自探测工具的覆盖度信息性测量。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Acknowledged — no change. This is an informational test-efficacy measurement explicitly marked "No action required"; the TEXT_MESSAGE_LIMIT deletion is residue of the test-gated sendTextReply removal, and file-level reverts of that removal still fail tests.
中文说明
已知悉——不做改动。这是一条明确标注"无需处理"的信息性测试有效性测量；TEXT_MESSAGE_LIMIT 的删除是有测试门控的 sendTextReply 删除的逐 hunk 残留，对该删除做文件级回退仍会使测试失败。
| [mentionTarget]?: string; | ||
| }; | ||
|
|
||
| interface CardRunCorrelation { |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (validated harness): this hunk — the added CardRunCorrelation interface — is ungated by tests (vitest transpiles without type-checking, so deleting the interface leaves all 260 tests green); only the workspace tsc --build gates it, and it passed in this review's build. If a later edit drops or renames the sender field on the correlation type, this diff's tests will not fail. Part of the same 5-hunk ungated-residue pattern around the sendTextReply removal (see the ~133 comment). No action required — the compiler is the effective gate.
中文说明
测试有效性探测（harness 已验证）：此 hunk——新增的 CardRunCorrelation 接口——没有测试门控（vitest 转译时不做类型检查，删除该接口后全部 260 个测试仍为绿）；只有工作区的 tsc --build 是其门控，且已在本次评审的构建中通过。若后续编辑删除或重命名关联类型上的 sender 字段，本 diff 的测试不会失败。属于 sendTextReply 删除周边 5 个无门控残留 hunk 的同一模式（见约 133 行的评论）。无需处理——编译器即有效门控。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Acknowledged — no change. Informational probe finding marked "No action required"; the CardRunCorrelation interface is gated by the workspace tsc --build, which passes in the verification for this round.
中文说明
已知悉——不做改动。信息性探测结论，已标注"无需处理"；CardRunCorrelation 接口由工作区 tsc --build 门控，本轮验证已通过。
| return chunks; | ||
| } | ||
|
|
||
| type DingTalkClientInternals = DWClient & { |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (validated harness): this hunk — deleting the splitTextChunks helper — is ungated: re-adding the function as dead code leaves all 260 tests green. If a future change reintroduces a text-chunking send path with different limit semantics, no test here flags it. Part of the same 5-hunk ungated-residue pattern around the sendTextReply removal (see the ~133 comment); the sole caller is removed in the test-gated hunk at ~673. No action required — informational.
中文说明
测试有效性探测（harness 已验证）：此 hunk——删除 splitTextChunks 辅助函数——没有测试门控：把该函数作为死代码加回后全部 260 个测试仍为绿。若未来改动重新引入具有不同上限语义的文本分片发送路径，这里的测试不会发现。属于 sendTextReply 删除周边 5 个无门控残留 hunk 的同一模式（见约 133 行的评论）；其唯一调用方已在约 673 行有测试门控的 hunk 中删除。无需处理——仅作信息提示。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Acknowledged — no change. Informational probe finding marked "No action required"; the splitTextChunks removal is residue of the test-gated sendTextReply removal (its sole caller is removed in the gated hunk).
中文说明
已知悉——不做改动。信息性探测结论，已标注"无需处理"；splitTextChunks 的删除是有测试门控的 sendTextReply 删除的残留（其唯一调用方已在有门控的 hunk 中删除）。
| @@ -218,7 +209,6 @@ export class DingtalkChannel extends ChannelBase { | |||
| private seenMessages: Map<string, number> = new Map(); | |||
| private mentionTargets = new Map<string, string>(); | |||
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (validated harness): this hunk — deleting private textReplySessions = new Set<string>() — is ungated: re-adding the unused field leaves all 260 tests green, so a partial revert of the text-reply bookkeeping would not be caught by this diff's tests. Part of the same 5-hunk ungated-residue pattern around the sendTextReply removal (see the ~133 comment); the .add/.has/.delete call sites are removed in hunks beyond the probe's 6-hunk cap (not measured). No action required — informational.
中文说明
测试有效性探测（harness 已验证）：此 hunk——删除 private textReplySessions = new Set<string>()——没有测试门控：把该未使用字段加回后全部 260 个测试仍为绿，因此对文本回复簿记的部分回退不会被本 diff 的测试捕获。属于 sendTextReply 删除周边 5 个无门控残留 hunk 的同一模式（见约 133 行的评论）；其 .add/.has/.delete 调用点位于超出探测 6-hunk 上限的 hunk 中（未被测量）。无需处理——仅作信息提示。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Acknowledged — no change. Informational probe finding marked "No action required"; the textReplySessions field deletion is residue of the test-gated sendTextReply removal.
中文说明
已知悉——不做改动。信息性探测结论，已标注"无需处理"；textReplySessions 字段的删除是有测试门控的 sendTextReply 删除的残留。
| target: { chatId: string; isGroup: boolean }; | ||
| } | ||
| >(); | ||
| private readonly inboundCardOwners = new Map<string, CardRunCorrelation>(); |
There was a problem hiding this comment.
[Suggestion] Test-efficacy probe (validated harness): this hunk — retyping inboundCardOwners/cardRuns to CardRunCorrelation — is type-level only and ungated by tests (restoring the inline structural types leaves all 260 tests green; vitest does not type-check). Only tsc --build gates it, and it passed. No behavioral regression is possible from this hunk in isolation. Part of the same 5-hunk ungated-residue pattern around the sendTextReply removal (see the ~133 comment). No action required — the compiler is the effective gate.
中文说明
测试有效性探测（harness 已验证）：此 hunk——把 inboundCardOwners/cardRuns 重定型为 CardRunCorrelation——纯属类型层改动且无测试门控（恢复内联结构类型后全部 260 个测试仍为绿；vitest 不做类型检查）。只有 tsc --build 是其门控，且已通过。单独看此 hunk 不可能产生行为回归。属于 sendTextReply 删除周边 5 个无门控残留 hunk 的同一模式（见约 133 行的评论）。无需处理——编译器即有效门控。
— qwen3.8-max via Qwen Code /review (v0.21.5)
There was a problem hiding this comment.
Acknowledged — no change. Informational probe finding marked "No action required"; the CardRunCorrelation retyping is type-level only and gated by the workspace tsc --build, which passes in the verification for this round.
中文说明
已知悉——不做改动。信息性探测结论，已标注"无需处理"；CardRunCorrelation 的重定型纯属类型层改动，由工作区 tsc --build 门控，本轮验证已通过。
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #8565One commit on the PR branch: Critical findings
Suggestions
Review-level noteThe round-2 VerificationCommands actually run on the committed state:
中文说明已处理的评审反馈 — PR #8565在 PR 分支上提交了一个 commit： Critical 发现
Suggestion 发现
评审层面说明第 2 轮的 验证在已提交状态上实际执行的命令：
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/模型 |
|
🔀 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 将重新运行。 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31045556089 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 No code change made this round — the failed check was diagnosed as not caused by this PR. No action — PR #8565Feedback point: failed check Decision: no PR change. Every step of the failing CI job was reproduced locally on the exact PR head ( What the PR changes8 files vs Local reproduction of the Test job (PR head
|
| Test-job step | Result |
|---|---|
npm run build |
passed |
npm run typecheck |
passed |
ESLint (scripts/lint.js --eslint → lint:ci) |
passed |
Prettier (scripts/lint.js --prettier) |
write-mode step (auto-formats, cannot fail the job) |
| sensitive-keywords / yamllint setup / actionlint / shellcheck | keyword linter passed; YAML/shell linters not installable in this sandbox — PR adds no workflow or shell files |
npm run check-i18n |
passed |
npm run check:lockfile |
passed |
npm run check:desktop-isolation |
passed |
npm run audit:runtime:critical |
passed (0 critical) |
Settings schema freshness (generate:settings-schema + git status) |
up-to-date |
| VS Code companion notices freshness | up-to-date |
check:serve-fast-path-bundle (after DEV=true npm run bundle) |
passed |
.github/scripts helper tests (node --test, 13 suites) |
206/206 passed |
Full unit suite npm run test:ci (CI-like env: fresh HOME, no QWEN_HOME, API keys cleared) |
all workspaces green except packages/cli AuthDialog.test.tsx — see artifact 2 below |
| Chrome extension package/scan | not runnable here (no zip); step unaffected — PR touches no chrome-extension files |
no-AK integration gate (test:integration:no-ak:sandbox:none file list) |
124/125 passed; the one failure is artifact 1 below |
Unit-suite detail: core 568 files passed, acp-bridge 23, sdk 32, web-shell 170, webui 33, vscode-companion 51, all channel packages including dingtalk 272/272 tests (10 files), cli 772/773.
The three local failure groups, each proven to be a sandbox artifact
Artifact 1 — the only integration-gate failure: cli/qwen-serve-streaming.test.ts "daemon Todo Stop Guard replay … replays the bounded attempts" (expected 4 model requests, observed 2).
Root cause chain, fully traced and reproduced manually outside vitest:
- This autofix sandbox keeps its own permission file at
.qwen/settings.jsoninside the checkout. It declares atools.coreallowlist that does not containtodo_write. - The test's daemon session binds the repo as its workspace, so the daemon loads that file as workspace settings;
Config.createToolRegistry()consults the permission manager per tool, andtodo_writeis silently not registered (session transcript recordsTool "todo_write" not found in registry.). - Without a successful
todo_write, the Todo Stop Guard never arms (observeTodoWritenever sees a structured todo list), so no continuations fire and the request count stays at 2. - A/B proof: the identical manual reproduction against a clean temp workspace (no
.qwen/) completes the full chain — 4/4 requests in ~120 ms, exactly what the test asserts. Against the repo workspace it reproduces 2/4 deterministically, even on a fully idle machine. - CI cannot hit this: GitHub Actions checks out a fresh tree (
.qwen/is git-ignored), and the Test job even has an explicit "Clean stale .qwen before checkout" step for exactly this contamination class on shared runners. - Unrelated to this PR either way: the test file, the guard (
daemon-todo-stop-guard.ts), and the session wiring (Session.ts) are byte-identical toorigin/main; the PR's diff touches only dingtalk channel code, which is never loaded in this test (no channels configured).
Artifact 2 — packages/cli AuthDialog.test.tsx (4 tests under parallel load, 1 at idle).
These TUI-input tests are wrapped in itWhenTuiInputReliable, which is it.skip whenever process.env['CI'] === 'true' (always the case on GitHub Actions). They never execute in CI and therefore cannot be the CI failure; locally they are timing-sensitive keystroke/render races in this container.
Artifact 3 — packages/cli settings.test.ts (12 tests) in the first local run.
Caused by QWEN_HOME exported by the agent sandbox itself, which redirects getUserSettingsPath() away from the mocked os.homedir(). With QWEN_HOME unset (CI never sets it for the unit step) all 155/155 tests pass.
Conclusion
No step of the Test job fails because of this PR's changes. The red check observed on CI cannot be inspected further from this environment (no GitHub credentials to fetch the job log/annotations), but the complete local reproduction leaves no PR-attributable failure standing. Suggested next step: re-run the Test check on the current head; if it remains red on a clean runner, the job log will pinpoint the step — based on the evidence above it should not be any code in this PR.
Verification
Commands actually run on PR head 2e42dc8fd (results as listed in the table above):
npm run build— passednpm run typecheck— passednode scripts/lint.js --eslint— passednode scripts/lint.js --prettier— ran (write mode; auto-formatted files were restored to HEAD afterwards, tree left clean)node scripts/lint.js --sensitive-keywords— passednpm run check-i18n,npm run check:lockfile,npm run check:desktop-isolation,npm run audit:runtime:critical— all passednpm run generate:settings-schema+git statuscheck — schema up-to-datenpm run generate:notices --workspace=qwen-code-vscode-ide-companion+git statuscheck — notices up-to-dateDEV=true npm run bundle+node scripts/check-serve-fast-path-bundle.js— passednode --test <13 helper suites>— 206 passednpx vitest runinpackages/channels/dingtalk— 272/272 passednpx vitest run src/config/settings.test.ts(cli, withQWEN_HOMEunset) — 155/155 passed- Full
npm run test:ciwith CI-like env — green exceptAuthDialog.test.tsx(CI-skipped tests) - no-AK integration gate file list via
npx vitest run --root ./integration-tests --maxWorkers 2 …— 124/125; the single failure root-caused as workspace-settings contamination (artifact 1) - Manual daemon reproduction of the guard flow (diagnostic script in git-ignored
.qwen/tmp, removed afterwards): contaminated workspace → 2/4 requests; clean workspace → 4/4 requests - Working tree left clean (
git status --porcelainempty); HEAD unchanged at2e42dc8fd
Base-conflict check: no conflict with main; no merge performed.
中文说明
🤖 本轮未修改代码 —— 失败检查已诊断为与本 PR 无关。
无需处理 — PR #8565
反馈项：PR head 上的失败检查 Test (ubuntu-latest, Node 22.x)。本轮没有任何需要处理的行内评论、评审或 issue 级评论（--conflict false；未执行任何合并）。
决定：不修改 PR。 已在本地对确切的 PR head（2e42dc8fd）逐步复现了该失败 CI job 的所有步骤。除三组失败外全部通过，而这三组失败均被证实是本轮 autofix 所在 agent 沙箱自身的环境产物 —— 在 CI runner 上均不可能出现，且都不涉及本 PR 改动的代码。
本 PR 的改动内容
相对 main 共 8 个文件：7 个位于 packages/channels/dingtalk/，1 个文档页面。合并提交是干净的（无冲突解决；与 main 的差异恰好就是这 8 个文件）。
Test job 的本地复现（PR head 2e42dc8fd）
| Test job 步骤 | 结果 |
|---|---|
npm run build |
通过 |
npm run typecheck |
通过 |
ESLint（scripts/lint.js --eslint → lint:ci） |
通过 |
Prettier（scripts/lint.js --prettier） |
写入模式步骤（自动格式化，不会使 job 失败） |
| sensitive-keywords / yamllint 安装 / actionlint / shellcheck | 关键词检查通过；YAML/shell 检查器在本沙箱无法安装 —— 本 PR 未新增任何 workflow 或 shell 文件 |
npm run check-i18n |
通过 |
npm run check:lockfile |
通过 |
npm run check:desktop-isolation |
通过 |
npm run audit:runtime:critical |
通过（0 个 critical） |
settings schema 新鲜度（generate:settings-schema + git status） |
最新 |
| VS Code companion notices 新鲜度 | 最新 |
check:serve-fast-path-bundle（DEV=true npm run bundle 之后） |
通过 |
.github/scripts 辅助测试（node --test，13 个套件） |
206/206 通过 |
完整单元测试 npm run test:ci（CI 等价环境：全新 HOME、无 QWEN_HOME、清空 API keys） |
除 packages/cli 的 AuthDialog.test.tsx 外全部绿色 —— 见下方产物 2 |
| Chrome 扩展打包/扫描 | 本地无法运行（无 zip）；该步骤不受影响 —— PR 未触碰任何 chrome-extension 文件 |
no-AK 集成门禁（test:integration:no-ak:sandbox:none 文件列表） |
124/125 通过；唯一失败即下方产物 1 |
单元测试明细：core 568 个文件通过，acp-bridge 23，sdk 32，web-shell 170，webui 33，vscode-companion 51，所有 channel 包（含 dingtalk 272/272 测试（10 个文件）），cli 772/773。
三组本地失败，均被证实为沙箱产物
产物 1 —— 集成门禁唯一失败项：cli/qwen-serve-streaming.test.ts "daemon Todo Stop Guard replay … replays the bounded attempts"（期望 4 次模型请求，实际 2 次）。
根因链条，已完整追踪并在 vitest 之外手动复现：
- 本 autofix 沙箱在 checkout 内保留了自己的权限文件
.qwen/settings.json，其中声明了tools.core白名单，且不包含todo_write。 - 测试的 daemon 会话把仓库目录绑定为工作区，因此 daemon 会把该文件作为工作区设置加载；
Config.createToolRegistry()逐个工具查询权限管理器，todo_write被静默跳过注册（会话记录中出现Tool "todo_write" not found in registry.）。 todo_write未成功执行，Todo Stop Guard 就永远不会进入 armed 状态（observeTodoWrite从未看到结构化 todo 列表），因此不会触发任何续跑，请求数停在 2。- A/B 证明：同样的手动复现指向一个干净的临时工作区（无
.qwen/）时，完整链条跑通 —— 约 120ms 内 4/4 次请求，与测试断言完全一致；指向仓库工作区时即使在完全空闲的机器上也确定性地复现 2/4。 - CI 不会遇到此问题：GitHub Actions 检出的是全新树（
.qwen/在 git-ignored 中），且 Test job 甚至有专门的 "Clean stale .qwen before checkout" 步骤来应对共享 runner 上的这一污染类别。 - 无论如何都与本 PR 无关：测试文件、guard（
daemon-todo-stop-guard.ts）、会话接线（Session.ts）与origin/main逐字节一致；本 PR 的 diff 只涉及 dingtalk 通道代码，而该测试中通道根本不会被加载（未配置任何 channel）。
产物 2 —— packages/cli AuthDialog.test.tsx（并行负载下 4 个失败，空闲时 1 个）。
这些 TUI 输入测试被包在 itWhenTuiInputReliable 中，当 process.env['CI'] === 'true'（GitHub Actions 恒为真）时为 it.skip。它们在 CI 中从不执行，因此不可能导致 CI 失败；本地只是本容器中对击键/渲染时序敏感的竞态。
产物 3 —— 首次本地运行中 packages/cli settings.test.ts（12 个测试）。
由 agent 沙箱自身导出的 QWEN_HOME 引起，它把 getUserSettingsPath() 从被 mock 的 os.homedir() 上重定向开了。取消 QWEN_HOME 后（CI 的单元测试步骤从不设置它）全部 155/155 通过。
结论
Test job 没有任何一步是因为本 PR 的改动而失败。本环境无法进一步检查 CI 上的红色检查（没有 GitHub 凭据去拉取 job 日志/annotations），但完整的本地复现已排除一切可归因于 PR 的失败。建议下一步：在当前 head 上重跑 Test 检查；若在干净的 runner 上仍然失败，job 日志会指明具体步骤 —— 依据上述证据，不应是本 PR 的任何代码。
验证
在 PR head 2e42dc8fd 上实际执行的命令（结果见上表）：
npm run build— 通过npm run typecheck— 通过node scripts/lint.js --eslint— 通过node scripts/lint.js --prettier— 已运行（写入模式；其自动格式化的文件随后已恢复到 HEAD，工作树保持干净）node scripts/lint.js --sensitive-keywords— 通过npm run check-i18n、npm run check:lockfile、npm run check:desktop-isolation、npm run audit:runtime:critical— 全部通过npm run generate:settings-schema+git status检查 — schema 最新npm run generate:notices --workspace=qwen-code-vscode-ide-companion+git status检查 — notices 最新DEV=true npm run bundle+node scripts/check-serve-fast-path-bundle.js— 通过node --test <13 个辅助套件>— 206 通过- 在
packages/channels/dingtalk下npx vitest run— 272/272 通过 npx vitest run src/config/settings.test.ts（cli，取消QWEN_HOME）— 155/155 通过- CI 等价环境下的完整
npm run test:ci— 除AuthDialog.test.tsx（CI 中跳过的测试）外全绿 - no-AK 集成门禁文件列表（
npx vitest run --root ./integration-tests --maxWorkers 2 …）— 124/125；唯一失败已定位根因为工作区设置污染（产物 1） - guard 流程的手动 daemon 复现（诊断脚本置于 git-ignored 的
.qwen/tmp，事后已删除）：受污染工作区 → 2/4 请求；干净工作区 → 4/4 请求 - 工作树保持干净（
git status --porcelain为空）；HEAD 保持在2e42dc8fd未变
基分支冲突检查：与 main 无冲突；未执行合并。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
|
🔀 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.
Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): 255 passed — this review observed 272, 17771 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。 Test Plan（非阻断）：255 passed — this review observed 272, 17771 passed。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const CARD_CONTENT_LIMIT = 20_000; | ||
| const TRUNCATION_MARKER = '[Earlier output truncated]\n'; |
There was a problem hiding this comment.
[Suggestion] R3-1: The card content cap and truncation marker are now defined twice in this package — CARD_CONTENT_LIMIT/TRUNCATION_MARKER here and the pre-existing CONTENT_LIMIT/TRUNCATION_MARKER in status-card-controller.ts — and withSenderPrefix's budget silently depends on the two staying identical. — Failure scenario: every prefixed terminal string passes through the controller's independent cap again (complete/fail/replace → boundContent); a probe verified that lowering only the controller's limit makes its keep-tail/head-chop silently delete the @sender prefix and re-truncate already-budgeted content (doubled marker). Nothing documents that the constants must move together. Suggested fix: export the limit and marker from one module and reuse them in both places.
// e.g. in status-card-controller.ts (or a shared constants module):
export const CONTENT_LIMIT = 20_000;
export const TRUNCATION_MARKER = '[Earlier output truncated]\n';
// interaction-presenter.ts imports them instead of defining its own copy中文说明
卡片内容上限与截断标记现在在本包中定义了两次——此处为 CARD_CONTENT_LIMIT/TRUNCATION_MARKER，status-card-controller.ts 中为既有的 CONTENT_LIMIT/TRUNCATION_MARKER——而 withSenderPrefix 的预算隐式依赖两者保持一致。失败场景：每个带前缀的终态字符串还会再经过 controller 的独立上限（complete/fail/replace → boundContent）；探测验证：仅调低 controller 的上限，其"保尾去头"截断就会悄悄删掉 @sender 前缀并二次截断已按预算处理的内容（出现双重标记）。没有任何注释说明这两个常量必须同步变动。建议：从单一模块导出上限与标记并在两处复用。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| * while creation is pending does not race it. A latched stream failure | ||
| * means the card can never show further content, so it is not live. | ||
| */ | ||
| async isCardLive(segmentId: string): Promise<boolean> { |
There was a problem hiding this comment.
[Suggestion] R3-2: No test holds status-card creation in flight across a response_boundary decision — every boundary test resolves createAndDeliver on the next microtask or fails it synchronously, so isCardLive's await-the-in-flight-creation guarantee (and flushPending) is unexercised. — Failure scenario: a probe verified that if isCardLive is refactored to a synchronous liveness flag, a boundary arriving while card creation is slow takes the text-fallback path while the card then also streams the same content (duplicate message to the user), and the entire existing suite stays green. Suggested fix: add a test with a deferred createAndDeliver promise — while creation is pending, closeOutput(..., 'response_boundary') must not call sendFallback yet; after it resolves the content is delivered via the card; the inverse arm rejects creation and expects the fallback.
中文说明
没有测试让状态卡创建在 response_boundary 判定期间保持进行中——所有 boundary 测试要么让 createAndDeliver 在下一个微任务就 resolve，要么让它同步失败，因此 isCardLive 的"等待进行中的创建"保证（以及 flushPending）未被验证。失败场景：探测验证，若把 isCardLive 重构为同步存活标志，卡片创建缓慢时到达的 boundary 会走文本 fallback，而卡片随后又把同样内容流式输出（用户收到重复消息），且现有全部测试仍然通过。建议：新增一个使用受控延迟 createAndDeliver promise 的测试——创建挂起期间 closeOutput(..., 'response_boundary') 不得调用 sendFallback；resolve 后内容经卡片投递；反向分支让创建失败并断言走 fallback。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| this.cardTarget(statusContext.target), | ||
| this.withSenderPrefix( | ||
| run, | ||
| detail === 'cancel_command' ? '任务已停止' : '任务已取消', |
There was a problem hiding this comment.
[Suggestion] R3-3: Eager creation makes "card exists but no output yet" a newly reachable state; the failed and completed terminals in that window are tested, but the cancelled terminal has no analogue. — Failure scenario: if the user cancels before the first token, this replace() + cancelRun() path must finalize the eager card; a probe confirmed the mirrored test passes on current code, but if a future change makes replace no-op on a not-yet-flushed record or cancelRun miss the record, the eager card stays stuck at "Running · Ns" forever and nothing in the suite fails. Suggested fix: mirror the failed-before-output test with 'cancelled' (assert terminal flowStatus, Stopped · Ns, 任务已停止 content).
中文说明
立即建卡使"卡片已存在但尚无输出"成为新的可达状态；该窗口内的 failed 与 completed 终态都有测试，但 cancelled 终态没有对应测试。失败场景：用户在首个 token 前取消时，这条 replace() + cancelRun() 路径必须终结这张提前创建的卡片；探测确认镜像测试在当前代码上通过，但若未来改动让 replace 对未 flush 的记录变成 no-op，或 cancelRun 找不到记录，卡片会永远卡在 "Running · Ns"，且套件中没有任何测试会失败。建议：仿照"输出前失败"的测试补一个 'cancelled' 用例（断言终态 flowStatus、Stopped · Ns、任务已停止 内容）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| await record.writeChain; | ||
| if (record.pendingSnapshot === undefined) break; |
There was a problem hiding this comment.
[Suggestion] R3-4: Test-efficacy probe (validated harness), mutant survived: forcing this drain-loop exit guard to always break leaves every affected test green — the second-pass drain is unpinned. — Failure scenario: content landing while flushPending awaits record.writeChain must be drained on a second pass; a regression exiting after the first pass makes flushPending return true (claiming delivery) while the newest snapshot was never pushed — the card shows stale content and a fallback that should have fired does not. Suggested fix: add a controller test where a new pending snapshot is queued while the write await is in flight, asserting the final update is pushed and flushPending only then returns true.
中文说明
测试有效性探测（已验证的 harness），突变体存活：把这个排空循环的退出守卫强制改为总是 break，所有受影响测试仍然全绿——第二遍排空没有被测试钉住。失败场景：flushPending 等待 record.writeChain 期间到达的内容必须在第二遍排空；若退化为只排一遍就退出，flushPending 会返回 true（声称已投递）而最新快照从未写入——卡片显示过期内容，本应触发的 fallback 也不会触发。建议：新增一个 controller 测试，在写入等待期间塞入新的待写快照，断言最终更新被推送且 flushPending 在那之后才返回 true。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| text: string, | ||
| sessionId: string, | ||
| ) => this.sendResponseMessage(chatId, text, sessionId), | ||
| ) => this.sendFallbackReply(chatId, text, sessionId), |
There was a problem hiding this comment.
[Suggestion] R3-5: Test-efficacy probe (validated harness), hunk survived: reverting this presenter sendFallback wiring from sendFallbackReply back to sendResponseMessage leaves every test green — nothing gates that mid-run fallbacks must not consume the mention target. — Failure scenario: group chat with atSender and a mid-run card-stream failure → fallback sent; sendResponseMessage deletes sessionMentionTargets while sendFallbackReply deliberately does not, so if the wiring regresses, the fallback consumes the prompt's mention target and the final answer of the same run is sent without its @sender mention/prefix. Suggested fix: extend the card-stream-failure fallback test to a group message with atSender enabled and assert the final model output still carries the sender mention after the mid-run fallback.
中文说明
测试有效性探测（已验证的 harness），hunk 存活：把 presenter 的 sendFallback 接线从 sendFallbackReply 还原为 sendResponseMessage，所有测试仍全绿——"运行中 fallback 不得消费 mention 目标"这一性质没有被测试钉住。失败场景：开启 atSender 的群聊在运行中卡片流失败 → 发送 fallback；sendResponseMessage 会删除 sessionMentionTargets，而 sendFallbackReply 刻意不删，因此若接线回退，fallback 会消费掉本回合的 mention 目标，同一 run 的最终答案将失去 @sender 提及/前缀。建议：把"卡片流失败 fallback"测试扩展为开启 atSender 的群消息，并断言运行中 fallback 之后最终输出仍带发送者 mention。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| event.sessionId, | ||
| inboundOwner.sender, |
There was a problem hiding this comment.
[Suggestion] R3-17: The sessionId propagation from registerRun into the eagerly created card record — and therefore into the card's Stop button — is unpinned by any test; a mutant severing the link ships silently. — Failure scenario: since this PR makes every status card eager, the record's sessionId comes exclusively from this argument (registerRun → baseContext.sessionId → createRecord → claimStop → cancelRun(record.sessionId, record.runId) → requestPromptRunCancellation). Probe verified: with baseContext.sessionId forced to '', all 272 tests stay green, and Stop on any card calls requestPromptRunCancellation('', runId) → activePrompts.get('') is undefined → returns false → stopClaimed reset, nothing displayed — the run keeps running, uncancellable from the card, with no error anywhere. Suggested fix: add one presenter test: registerRun with sessionId 'session-1', startStatusCard, take the outTrackId from createAndDeliver, call the real statusCards.claimStop(outTrackId, 'owner-1'), run the accepted execute, and assert cancelRun was called with ('session-1', 'run-1').
中文说明
sessionId 从 registerRun 传入立即创建的卡片记录——进而传入卡片 Stop 按钮——的链路没有任何测试钉住；切断该链路的突变体会静默通过。失败场景：本 PR 使每张状态卡都立即创建，记录的 sessionId 完全来自这个参数（registerRun → baseContext.sessionId → createRecord → claimStop → cancelRun(record.sessionId, record.runId) → requestPromptRunCancellation）。探测验证：把 baseContext.sessionId 强制为 '' 后，全部 272 个测试仍通过，而点击任何卡片的 Stop 会调用 requestPromptRunCancellation('', runId) → activePrompts.get('') 为 undefined → 返回 false → stopClaimed 被重置、无任何显示——run 继续运行，无法从卡片取消，全程没有错误。建议：新增一个 presenter 测试：以 sessionId 'session-1' 调用 registerRun、startStatusCard，从 createAndDeliver 取 outTrackId，调用真实的 statusCards.claimStop(outTrackId, 'owner-1')，执行接受的 execute，断言 cancelRun 以 ('session-1', 'run-1') 被调用。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const remainder = body.slice(prefix.length); | ||
| if (/^\r?\n/u.test(remainder)) { |
There was a problem hiding this comment.
[Suggestion] R3-18: Sender-echo dedup (withoutExistingSenderPrefix) only strips an echoed @name when it is followed by a newline; an inline echo (@name + space/other text — the shape real DingTalk mentions render as) survives, and withSenderPrefix then prepends a second attribution. — Failure scenario: probe verified end-to-end through the real presenter + controller: sender 衍星, final answer @衍星 这是最终答案 → terminal card content @衍星\n\n@衍星 这是最终答案 (double attribution visible to the group). The newline-separated form is pinned by 'adds the group sender only to the final model output'; the inline form is untested. Suggested fix: treat a whitespace-followed echo as removable too (e.g. guard /^\s/u and normalize the stripped separator), or pin the current inline passthrough as intentional with a test.
中文说明
发送者回显去重（withoutExistingSenderPrefix）只在回显的 @名字 后跟换行时才剥离；行内回显（@名字 + 空格/其他文本——真实钉钉 mention 的渲染形态）会存活，withSenderPrefix 随后又补上第二个署名。失败场景：探测经真实 presenter + controller 端到端验证：发送者 衍星，最终答案 @衍星 这是最终答案 → 终态卡片内容 @衍星\n\n@衍星 这是最终答案（群内可见的双重署名）。换行形态已被 'adds the group sender only to the final model output' 钉住；行内形态未被测试覆盖。建议：把后随空白字符的回显也视为可剥离（如守卫改为 /^\s/u 并规范化剥离的分隔符），或用测试明确钉住当前行内透传是有意的。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if (terminal === 'failed') { | ||
| const statusContext = this.ensureStatusContext(run); | ||
| this.options.statusCards?.ensure( |
There was a problem hiding this comment.
[Suggestion] R3-20: After an input_requested round, terminalizeRun('failed') rebuilds the status context with segmentId = runId — an id the controller has already terminalized — so ensure() is blocked by terminalSegmentIds and the intended card failure UX is silently skipped. — Failure scenario: probe verified: eager card latches segmentId = runId; closeOutput('input_requested') finalizes that id into terminalSegmentIds and resets run.statusContext; the resumed run fails before producing output → ensureStatusContext rebuilds {…baseContext, segmentId: runId} → ensure() early-returns → fail() finds no record → zero card updates. The generic English catch-all in onMessage still fires, so the user is not totally silent — but the localized card failure path this PR built is dead on this route, and no test covers a terminal failure after input_requested with no intervening output. Suggested fix: give the re-latched context a fresh segment id when latching from baseContext (e.g. ${run.runId}:${randomUUID()}), or skip ensure when the id is already terminal and send the failure text via sendFallback; add the missing test.
中文说明
input_requested 回合之后，terminalizeRun('failed') 会以 segmentId = runId 重建状态上下文——而该 id 已被 controller 终态化——于是 ensure() 被 terminalSegmentIds 拦下，预期的卡片失败 UX 被静默跳过。失败场景：探测验证：立即卡片锁定 segmentId = runId；closeOutput('input_requested') 把该 id 终态化进 terminalSegmentIds 并重置 run.statusContext；恢复后的 run 在产生输出前失败 → ensureStatusContext 重建 {…baseContext, segmentId: runId} → ensure() 提前返回 → fail() 找不到记录 → 0 次卡片更新。onMessage 的通用英文兜底错误仍会发出，用户并非完全无感知——但本 PR 构建的本地化卡片失败路径在这条链路上是死的，且没有测试覆盖 input_requested 之后、无中间输出的终态失败。建议：从 baseContext 重新锁定上下文时使用新的 segment id（如 ${run.runId}:${randomUUID()}），或在 id 已终态时跳过 ensure 并通过 sendFallback 发送失败文案；补上缺失的测试。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| this.options.statusCards?.replace( | ||
| statusContext, | ||
| this.cardTarget(statusContext.target), | ||
| this.withSenderPrefix( |
There was a problem hiding this comment.
[Suggestion] R3-21: Content that response_boundary declared delivered via the card (fallback suppressed) is permanently erased when the run later terminates failed or cancelled — the terminal path overwrites the single continuity card's content with the terse terminal message. — Failure scenario: probe verified both terminals: boundary delivery returns true with no fallback (the card is the only copy of the segment); a later failed/cancelled terminal overwrites content/blockList/copy_content with the fixed message ('本次处理失败，请稍后重试。' / '任务已取消') and the boundary content exists nowhere — pre-PR, the per-segment boundary card survived a later failure/cancel. The overwrite is deliberate and test-pinned (one-card continuity is the PR's advertised design), but flushPending's "callers can treat the card's current content as delivered" contract is broken by these terminals, and no test pins the boundary→failed/cancelled combination. Suggested fix: track per-run whether content was declared delivered via the card and re-deliver it through sendFallback before overwriting (or append the terminal notice), or pin the loss explicitly as accepted in a boundary-then-terminal test.
中文说明
被 response_boundary 声明"已经卡片投递"（从而抑制了 fallback）的内容，会在 run 之后以 failed 或 cancelled 终态结束时被永久抹除——终态路径用简短的终态文案覆盖这张唯一的连续卡片内容。失败场景：探测验证了两种终态：boundary 投递返回 true 且未发 fallback（卡片是该 segment 内容的唯一副本）；之后的 failed/cancelled 终态用固定文案（'本次处理失败，请稍后重试。' / '任务已取消'）覆盖 content/blockList/copy_content，boundary 内容从此无处可寻——PR 之前，按 segment 终结的 boundary 卡片能在之后的失败/取消中存活。该覆盖是有意的且有测试钉住（单卡连续性正是本 PR 宣称的设计），但 flushPending 的"调用方可把卡片当前内容视为已投递"契约会被这些终态打破，且没有测试钉住 boundary→failed/cancelled 组合。建议：按 run 追踪是否有内容被声明为卡片已投递，在覆盖前通过 sendFallback 重新投递（或追加终态提示），或用 boundary→终态 测试明确钉住该丢失是可接受的。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| record.content = boundContent(content); | ||
| if (record.streamFailed) return; | ||
| record.pendingSnapshot = sanitizeStreamingImageMarkers(record.content); |
There was a problem hiding this comment.
[Suggestion] R3-24: replace()'s suppression of further snapshot writes after a latched streamFailed (this guard) is unconstrained by any test — deleting it leaves the entire dingtalk package suite green (verified package-wide, 272/272). — Failure scenario: the only stream-failure test latches the failure and immediately closes the segment — nothing appends content after the latch. A probe verified the deletion mutant flips the suggested test: each post-latch content chunk re-arms pendingSnapshot/scheduleFlush, and every flush window (~500ms while content flows) retries openOrUpdateStream against the dead stream — one failed API call plus one stderr error per chunk for the rest of the run (a retry storm), while the suite stays green. The latch's boundary-decision effect is pinned; its write-suppression effect is not. Suggested fix: add one controller test: latch streamFailed (reject openOrUpdateStream once), then replace() with more content, advance past the flush window, and assert openOrUpdateStream was not called again and no further 'status card streaming' error was reported.
中文说明
replace() 在 streamFailed 置位后停止继续写入快照的守卫（此处）没有任何测试约束——删除它整个 dingtalk 包套件仍全绿（已全包验证，272/272）。失败场景：唯一的流失败测试置位失败后立即关闭 segment——置位之后没有任何内容追加。探测验证删除突变体会被建议的测试翻转：置位后的每个内容 chunk 都会重新武装 pendingSnapshot/scheduleFlush，每个 flush 窗口（内容流动时约 500ms）都会对已死的流重试 openOrUpdateStream——run 剩余时间里每个 chunk 一次失败的 API 调用加一条 stderr 错误（重试风暴），而套件仍全绿。置位的 boundary 判定作用已被钉住；其写入抑制作用没有。建议：新增一个 controller 测试：置位 streamFailed（拒绝一次 openOrUpdateStream），再 replace() 更多内容，推进到 flush 窗口之后，断言 openOrUpdateStream 未被再次调用且没有新的 'status card streaming' 错误。
— qwen3.8-max via Qwen Code /review (v0.21.6)
…8565) The per-second status chain kept pushing metadata updates after the content stream latched failed, and once the 3-failure breaker tripped an idle card could never revive. Stop the chain on a latched stream failure and keep a low-frequency probe so a recovered metadata API revives it. Also re-send boundary content declared delivered via the card when a failed/cancelled terminal overwrites the continuity card, give a re-latched status context a fresh segment id after input_requested so a later failure still reaches the card failure UX, dedup inline sender echoes, and require the 'I' of IMAGE in the partial-marker regex so a bare trailing '[' survives in final fallback text. Consolidate the duplicated content-cap constants and pin the reviewed delivery, drain, breaker, and attribution behaviors with tests.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Round 3 review feedback — summaryAll 21 inline suggestions were addressed: 7 behavior fixes, 1 simplification, Behavior fixes
Simplification
Pinning tests (no behavior change)
R3-12 — resolved via the offered "pin as accepted" alternativeSuffix-only fallback after a partial card display was not implemented: the Test efficacyEvery behavior fix was mutation-checked: the fix was temporarily reverted and Verification
中文说明第 3 轮评审反馈 — 总结全部 21 条行内建议均已处理：7 项行为修复、1 项简化、13 个新的钉住测试（本轮共新增 23 个测试）。其中一条（R3-12）按该建议自身提供的备选方案处理——把当前重复形态明确钉成可接受行为——理由记录如下。无冲突（ 行为修复
简化
钉住测试（无行为变化）
R3-12 — 按建议提供的"钉成可接受"备选方案处理未实现"流失败后只发后缀"的 fallback：卡片展示的是经过图片标记净化的快照，而 presenter 持有的是原始内容，把"卡片已显示的部分"映射回后缀边界很脆弱；且在投递失败时重发整个 segment 比冒着丢失未投递后缀的风险更安全。该建议明确提供的备选方案——把重复钉成可接受行为——已实现：既有的流失败 boundary 测试现在断言流失败时卡片正在显示该前缀，并以注释记录这一取舍。 测试有效性每项行为修复都做了突变体验证：临时回退修复、确认对应新测试随之失败（共 13 个突变体，覆盖 R3-2/4/5/7/8/9/10/11/13/14/15/17/18/20/21/24——全部被杀死，随后恢复修复）。 验证
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-dev-bot
removed
the
autofix/takeover
BenGuanRan
left a comment
BenGuanRan
left a comment
There was a problem hiding this comment.
Review focus: does this break the existing interactive card (question card) capability?
Conclusion: no breakage. Verified at head 83f402f3e.
Question-card capability integrity
question-card-controller.ts,interactive-card-client.ts, and card callback routing (btn_stop→claimStop, everything else → question cardclaim) are untouched; thepresentInput/presentUserInputRequestwiring is unchanged.- Gating is preserved:
interactiveCards.enabledremains the master switch;blockStreaming: 'on'still only skipsStatusCardControllerwhileQuestionCardControlleris created independently; with no status cards, allstatusCards?.calls in the presenter are no-ops. - Stop button:
claimStopownership checks are unchanged; the eagerly created card now carriessessionIdfromevent.sessionId, so stop still propagates to the right session (covered by the newpropagates the run session into the card stop actiontest). - On
input_requestedthe current status card is finalized first, thenstatusContextis reset; the question card is a separate card instance, so the two never overwrite each other; a failure after resume reaches the card failure UX via a fresh segment id (the last commit's fix). questionCards?.cancelRunsemantics interminalizeRunare unchanged from base.
New-behavior observations (intentional design, all test-covered)
- Failure text no longer leaks raw error detail and now uses a safe generic message — a security improvement.
atSendernon-card fallback switches from plain text to DingTalk markdown; docs are updated in the same PR. This is an intentional fallback behavior change, not card capability.redeliverCardDeliveredContenton failed/cancelled can produce a mild duplicate in edge scenarios (e.g. the run fails while a question card is pending), but it is not a capability break.- Each Running card refreshes metadata once per second and degrades to a 30s probe after 3 consecutive failures without ever fully stopping; a hung run keeps probing but is bounded. The PR already calls out this tradeoff.
Verification
- CI is green (Linux test suite, typecheck, lint).
- Ran the full
packages/channels/dingtalksuite locally at PR head: 295 passed (10 files). - Card API calls carry a 10s
AbortSignal.timeout, so the boundary wait onisCardLiveis bounded; theflushPendingdrain loop terminates because projections are serialized per run (no concurrentreplaceduring drain). - Pre-existing, non-regression gap: when the card API itself is unavailable, the failure message cannot reach the user; base has the same gap.
BenGuanRan
left a comment
BenGuanRan
left a comment
There was a problem hiding this comment.
Approved. Verified at head 83f402f: question-card capability (presentInput path, gating, stop button, callback routing) is fully preserved; local dingtalk package suite passes 295/295. One minor UX note: eager card creation means a pure question run now shows a Running card alongside the question card, relaxing the original 'a direct question creates no status card' rule — acceptable, worth keeping in mind.
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. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Test Plan (not a blocker): 255 passed — this review observed 295, 17770 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 Test Plan（非阻断）：255 passed — this review observed 295, 17770 passed。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| void this.enqueue(run, () => { | ||
| this.options.statusCards?.append( | ||
| segment, | ||
| this.cardTarget(segment.target), | ||
| chunk, | ||
| this.options.statusCards?.replace( | ||
| statusContext, | ||
| this.cardTarget(statusContext.target), | ||
| presentation.content, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-3: The append→replace rewrite's core invariant — the card stream receives the ACCUMULATED segment content, not the delta chunk — is ungated by any test. Mutation-verified: replacing presentation.content with chunk in this enqueue keeps the entire 295-test dingtalk suite green. — Failure scenario: in production every multi-chunk streamed response would then show only the latest chunk — each replace() overwrites record.content, so a user watching a long answer stream sees the text repeatedly reset to the newest fragment instead of growing. No test can catch that: the only presenter test appending twice to one segment asserts just the first write's content, every other openOrUpdateStream content assertion uses single-chunk segments (where chunk and accumulated content coincide), controller tests drive replace() directly, and adapter tests mock the presenter away. Suggested fix: add a presenter test — appendOutput(segment('segment-1'), 'Hello ') then appendOutput(segment('segment-1'), 'world'), advance past the flush interval, and assert the last openOrUpdateStream content is 'Hello world' (verified to flip the mutation).
中文说明
append→replace 改写的核心不变量——卡片流收到的是累积后的 segment 内容而非增量 chunk——没有任何测试钉住。已变异验证：把此处 enqueue 中的 presentation.content 换成 chunk，全部 295 个钉钉测试仍绿。失败场景：生产中每个多 chunk 流式回答都只会显示最新 chunk——replace() 覆盖 record.content，用户看到的长答案会反复重置为最新片段而不是增长。没有测试能捕获：唯一对同一 segment 追加两次的测试只断言首次写入内容；其余 openOrUpdateStream 内容断言全是单 chunk segment（chunk 与累积值重合）；controller 测试直接驱动 replace()；adapter 测试 mock 掉 presenter。建议：新增 presenter 测试——先后追加 'Hello ' 与 'world'，越过 flush 间隔后断言最后一次 openOrUpdateStream 内容为 'Hello world'（已验证可翻转该变异）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| statusCards?.ensure( | ||
| statusContext, | ||
| this.cardTarget(statusContext.target), | ||
| ); | ||
| statusCards?.fail( |
There was a problem hiding this comment.
[Suggestion] R4-9: This failed close branch has no text fallback when the status card cannot be created, unlike the completed/boundary/input_requested branches this same diff rewrote — a failed run can end with zero user-visible feedback. Probe-verified deterministic silence: with createAndDeliver rejected (the suite's own 'status template unavailable' shape), closeOutput(segment-1, '', 'failed') results in sendFallback being called 0 times. — Failure scenario: card creation fails (template misconfigured, card permission revoked, card endpoint down while messaging works) and the run then fails: create() rejects → ready false → fail()→finalize returns false at await record.ready, and the 'Unavailable' write targets a card that never existed. The failure message is delivered nowhere — ChannelBase's catch path emits no error text, onOutputSegmentEnd discards the boolean, onError is stderr-only. The user sees the thinking reaction removed and then silence. If failure-surface = card-only is a deliberate choice, pin it with a comment + test; otherwise mirror the completed branch. Suggested fix: detect that finalization did not land (e.g. have fail return the finalize result like complete does) and send the failure message via sendFallback (a fix variant was verified in-tree to flip the probe with all 50 presenter tests still green).
中文说明
这个 failed 关闭分支在状态卡无法创建时没有文本 fallback，而同一 diff 改写的 completed/boundary/input_requested 分支都有——失败的 run 可能以零用户可见反馈收场。探测已确认定寂：在 createAndDeliver 被拒绝（套件自己建模的 'status template unavailable' 形态）时执行 closeOutput(segment-1, '', 'failed')，sendFallback 调用次数为 0。失败场景：卡片创建失败（模板配置错误、卡片权限被收回、卡片端点故障而消息正常）且 run 随后失败：create() 拒绝 → ready 为 false → fail()→finalize 在 await record.ready 处返回 false，'Unavailable' 写入对着不存在的卡片静默失败。失败文案无处投递——ChannelBase 的 catch 路径不带错误文本，onOutputSegmentEnd 丢弃布尔值，onError 只写 stderr。用户看到思考表情移除后一片沉寂。若'失败仅走卡片'是有意决策，请用注释+测试钉住；否则对齐 completed 分支。建议：检测终结未落地（如让 fail 像 complete 一样返回终结结果）并经 sendFallback 发送失败文案（已在树内验证某修复变体可翻转探测且 50 个 presenter 测试全绿）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| await this.redeliverCardDeliveredContent(run); | ||
| return statusCards !== undefined; |
There was a problem hiding this comment.
[Suggestion] R4-4: This redeliverCardDeliveredContent call in closeOutput's failed branch is reachable but untested. Probe-verified: deleting this line leaves all 74 related tests green while a probe (boundary delivered via live card, then closeOutput(segment-2, '', 'failed')) flips from one sendFallback call to zero. — Failure scenario: if the line were deleted and a failed segment close ever arrived without a following run-level terminal event (event absent or reordered), the boundary content would be erased from the card by the failure overwrite with no fallback copy. Today the adapter also emits the run-level terminal 'failed' event which redelivers via terminalizeRun, so this is a low-priority defensive test gap rather than a demonstrated loss. Suggested fix: presenter test — boundary delivered through a live card, then closeOutput(segment-2, '', 'failed'), asserting sendFallback is called with the boundary text once.
中文说明
closeOutput failed 分支中的这个 redeliverCardDeliveredContent 调用可达但无测试覆盖。探测验证：删除该行后全部 74 个相关测试仍绿，而探测（boundary 经卡片投递后执行 closeOutput(segment-2, '', 'failed')）从一次 sendFallback 调用翻转为零。失败场景：若该行被删除且某次 failed 的 segment 关闭之后没有跟随 run 级终态事件（事件缺失或重排序），boundary 内容会被失败覆盖抹掉且无 fallback 副本。当前 adapter 总会补发 run 级终态事件经 terminalizeRun 重投递，因此属于低优先级防御性测试缺口而非已证实的丢失。建议：presenter 测试——boundary 经卡片投递后执行 closeOutput(segment-2, '', 'failed')，断言 sendFallback 收到 boundary 文本一次。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| (await statusCards.flushPending(statusContext.segmentId)); | ||
| if (deliveredViaCard) { | ||
| run.cardDelivered = { | ||
| text: stripPartialImageMarker(text || presentation.content), |
There was a problem hiding this comment.
[Suggestion] R4-5: The privacy strip on redelivered boundary content (stripPartialImageMarker in the cardDelivered assignment) is ungated: no test delivers marker-containing content through a live card at a boundary and then triggers redelivery. Mutation-verified: dropping the strip keeps the whole suite green. — Failure scenario: a segment ends at a boundary mid-marker (photo [IMAGE: /Users/ben/priv) and was delivered via the card; a later failed/cancelled terminal redelivers the unclosed marker with the partial local path verbatim — prepareOutgoingText/findImageMarkers only rewrite CLOSED markers. This is the exact leak class the sibling tests ('neutralizes partial image markers in text fallbacks') exist to prevent. Probe flips both ways. Suggested fix: presenter test — marker-ending content delivered via card at a boundary, terminalize failed, assert sendFallback gets 'photo [Image pending]'.
中文说明
重投递内容上的隐私剥离（cardDelivered 赋值处的 stripPartialImageMarker）无测试钉住：没有测试让含标记内容经卡片在 boundary 投递后再触发重投递。已变异验证：去掉剥离后全套测试仍绿。失败场景：segment 在 boundary 处以半截标记结尾（photo [IMAGE: /Users/ben/priv）且已经卡片投递；之后 failed/cancelled 终态重投递时，未闭合标记连同部分本地路径原样发出——prepareOutgoingText/findImageMarkers 只改写已闭合标记。这正是姊妹测试（'neutralizes partial image markers in text fallbacks'）要防的泄漏类。探测双向翻转。建议：presenter 测试——以半截标记结尾的内容经卡片在 boundary 投递、终态 failed，断言 sendFallback 收到 'photo [Image pending]'。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| chatId: presentation.context.target.chatId, | ||
| sessionId: presentation.context.sessionId, |
There was a problem hiding this comment.
[Suggestion] R4-12: The redelivery destination stored here is only ever exercised for GROUP runs — every redelivery test uses the group harness, where cardTarget's address and this stored chatId coincide, so the DM divergence is ungated. Probe-verified: a refactor storing this.cardTarget(presentation.context.target).chatId instead keeps the entire suite green while the probe flips (sendFallback receives 'owner-1' instead of 'conversation-1'). — Failure scenario: for a DM run the card is delivered to cardTarget = target.senderId, but redelivery must go to the conversationId (the adapter's webhooks map is keyed by conversationId). Under such a refactor every DM run that delivered a boundary through the live card and then failed/was cancelled would look up webhooks.get(senderId), miss, log 'No webhook for chatId', and silently drop the rescued content. Suggested fix: presenter test — DM run ({ chatId: 'conversation-1', isGroup: false }), boundary delivered via live card, terminalize failed, assert sendFallback receives 'conversation-1', not the sender id.
中文说明
这里存储的重投递目的地只在群 run 下被演练——所有重投递测试都用群 harness，cardTarget 的地址与此处存储的 chatId 恰好重合，DM 场景的分叉无测试钉住。探测验证：改为存储 this.cardTarget(presentation.context.target).chatId 的重构让全套测试仍绿，而探测翻转（sendFallback 收到 'owner-1' 而非 'conversation-1'）。失败场景：DM run 的卡片投递到 cardTarget = target.senderId，但重投递必须发往 conversationId（adapter 的 webhooks 按 conversationId 键控）。该重构下每个经卡片投递 boundary 后失败/取消的 DM run 会 webhooks.get(senderId) 未命中、记 'No webhook for chatId'、静默丢弃 rescued 内容。建议：presenter 测试——DM run（{ chatId: 'conversation-1', isGroup: false }）经卡片投递 boundary 后 terminalize failed，断言 sendFallback 收到 'conversation-1' 而非 sender id。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| if ( | ||
| record.consecutiveStatusFailures >= MAX_CONSECUTIVE_STATUS_FAILURES | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R4-19: The new metadata-failure breaker only throttles the timer-driven status chain; flush() still calls updateRunningStatus(record) unconditionally after every successful content write (~line 307), so an actively streaming card keeps issuing failing updateInstance calls at flush rate for the whole outage. Probe-verified: with content snapshots every 500ms for 10s while updateInstance always rejects, 21 failing metadata calls were observed despite the breaker tripping within seconds; gating the flush-driven push on the breaker drops it to 4. — Failure scenario: the metadata endpoint goes down mid-stream; lastStatusSecond is frozen (no success updates it), so the same-second early-return never engages, and every content flush (~500ms) attempts updateInstance, fails, increments the counter, and logs to stderr — an unbounded ~2/s stream of failing API calls for a long streaming run during an outage, precisely the hammering the breaker was added to stop. Caveat: a naive gate breaks the two PR-added recovery tests that pin the flush-driven push as the immediate recovery path — reconcile throttling with recovery (e.g. a low-frequency flush-side probe), or pin the current behavior deliberately with a comment + test.
中文说明
新的元数据失败熔断器只节流定时器驱动的状态链；flush() 在每次成功内容写入后仍无条件调用 updateRunningStatus(record)（约第 307 行），因此活跃流式卡片在整个故障期间以 flush 频率持续发起失败的 updateInstance 调用。探测验证：updateInstance 恒拒绝时每 500ms 一次内容快照持续 10s，尽管熔断器数秒内即跳闸，仍观察到 21 次失败的元数据调用；让 flush 驱动的推送受熔断器门控后降为 4 次。失败场景：元数据端点在流式中途故障；lastStatusSecond 冻结（无成功更新它），同秒早退永不生效，每个内容 flush（约 500ms）都尝试 updateInstance、失败、累加计数并写 stderr——长流式 run 在故障期间产生约每秒 2 次的失败 API 调用流，正是熔断器要阻止的重复轰炸。注意：朴素门控会破坏本 PR 自己的两个恢复测试（把 flush 推送钉为即时恢复路径）——需调和节流与恢复（如低频的 flush 侧探针），或用注释+测试有意钉住当前行为。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| record.streamFailed = true; | ||
| record.pendingSnapshot = undefined; |
There was a problem hiding this comment.
[Suggestion] R4-16: The 'no retry after a latched stream failure' guarantee rests solely on this untested record.pendingSnapshot = undefined; clear in flush()'s catch — neither flush() nor scheduleFlush() checks streamFailed. Mutation-verified: deleting this one line keeps all 295 tests green while the probe shows one extra openOrUpdateStream ~500ms after the latch (a retry against the dead stream). — Failure scenario: a content flush write is in flight (gated/slow) and fails; during the flight replace() arrives (pendingSnapshot set, scheduleFlush suppressed by inFlight); when the write rejects, this clear is the only thing preventing write.finally's if (record.pendingSnapshot !== undefined) this.scheduleFlush(record); from firing a post-latch flush. The two existing stream-failure tests introduce the failing write with no concurrent replace, so neither covers the race. Suggested fix: controller test — gate openOrUpdateStream to throw, queue a replace() during flight, resolve, advance timers, assert no further openOrUpdateStream and one onError. (Optionally also guard flush()/scheduleFlush() on streamFailed so the invariant does not rest on one line.)
中文说明
'流失败置位后不再重试'的保证只靠 flush() catch 中这行未测试的 record.pendingSnapshot = undefined; 清理——flush() 与 scheduleFlush() 都不检查 streamFailed。已变异验证：删除该行后全部 295 测试仍绿，探测观察到置位后约 500ms 多出一次 openOrUpdateStream（对死亡流的重试）。失败场景：内容 flush 写入在途（受限/慢）并失败；在途期间 replace() 到达（pendingSnapshot 置位、scheduleFlush 被 inFlight 抑制）；写入拒绝时，该清理是阻止 write.finally 的 if (record.pendingSnapshot !== undefined) this.scheduleFlush(record); 在置位后触发 flush 的唯一防线。两个现有流失败测试都不带并发 replace，无法覆盖该竞态。建议：controller 测试——门控 openOrUpdateStream 使其抛出，在途期间排队 replace()，解除门控、推进定时器，断言不再有更多 openOrUpdateStream 且 onError 恰一次。（可选：让 flush()/scheduleFlush() 也守卫 streamFailed，使不变量不单靠一行。）
— qwen3.8-max via Qwen Code /review (v0.21.6)
| void record.ready.then((ready) => { | ||
| if (ready) this.scheduleStatusRefresh(record); |
There was a problem hiding this comment.
[Suggestion] R4-17: The if (ready) guard keeping the periodic status chain off a creation-failed card is ungated: the test that sets up exactly this scenario ('does not claim delivery for a card whose creation failed') asserts only the flushPending result and ends immediately, never advancing a timer. Probe-verified: dropping the guard keeps all 295 tests green while a probe rejecting createAndDeliver and advancing fake timers 60s shows 60 updateInstance 'Running · Ns' pushes at the dead card; unmodified code shows zero. — Failure scenario: updateRunningStatus checks only terminal/streamFailed, never ready, so if this guard is dropped or lost in a refactor, the per-second chain starts on an Unavailable card anyway and pushes status updates at it until the run terminates. Production code is correct today; missing-test gap. Suggested fix: extend that test with fake timers — after the creation failure settles, mockClear() the one-shot 'Unavailable' write, advanceTimersByTimeAsync(5_000), and assert client.updateInstance was not called.
中文说明
让周期性状态链避开创建失败卡片的 if (ready) 守卫无测试钉住：设置该场景的测试（'does not claim delivery for a card whose creation failed'）只断言 flushPending 结果随即结束，从不推进定时器。探测验证：去掉守卫后全部 295 测试仍绿，而拒绝 createAndDeliver 并推进 60s 假定时器的探测对死卡观察到 60 次 'Running · Ns' 的 updateInstance 推送；未变异代码为零。失败场景：updateRunningStatus 只检查 terminal/streamFailed、从不检查 ready，守卫一旦被重构丢失，每秒链照样在 Unavailable 卡片上启动并推送状态更新直到 run 终结。生产代码今天正确；缺测试。建议：为该测试加假定时器——创建失败落定后 mockClear() 一次性 'Unavailable' 写入，advanceTimersByTimeAsync(5_000)，断言 client.updateInstance 未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| .mock.calls.map(([request]) => request.content); | ||
| const latest = payloads.at(-1) ?? ''; | ||
| expect(latest).toContain('我来查看 [Image pending]'); | ||
| expect(JSON.stringify(payloads)).not.toContain('### 处理进度'); |
There was a problem hiding this comment.
[Suggestion] R4-20: This not.toContain('### 处理进度') assertion guards a string that no production code, card template, or removed diff line ever emits — it cannot fail, yet reads as coverage for 'no status scaffolding leaks into streamed content'. Verified: 处理进度 appears exactly once in the worktree (this line), and git log -S shows it entered history only in this same test — born dead, not orphaned. — Failure scenario: any mutation or regression of the stream-content path (replace() → pendingSnapshot → openOrUpdateStream) that leaks real scaffolding leaves this test green while the suite appears to cover the case. The two sibling assertions in the same vi.waitFor are live (both flip under corresponding mutations), which isolates this line as dead weight. Suggested fix: delete the line, or assert streamed content never contains text the status path actually emits (e.g. Running · ) so the scaffold-leak intent is genuinely gated.
中文说明
这个 not.toContain('### 处理进度') 断言守护一个任何生产代码、卡片模板或已删 diff 行都不曾输出的字符串——它不可能失败，却读起来像'状态脚手架不外泄'的覆盖。已验证：处理进度 在整个工作区只出现一次（本行），git log -S 显示该字符串只由同一测试引入——出生即死，而非后来成为孤儿。失败场景：流内容路径（replace() → pendingSnapshot → openOrUpdateStream）的任何变异/回归若让真正的脚手架文本泄入，该测试仍绿而套件看似已覆盖。同一 vi.waitFor 中的两个姊妹断言是活的（均可被相应变异翻转），反衬出该行是死重。建议：删除该行，或改为断言流内容不含状态路径真实输出的文本（如 Running · ），使脚手架外泄的意图被真正钉住。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| content: reason === 'cancel_command' ? '任务已停止' : '任务已取消', | ||
| copy_content: | ||
| reason === 'cancel_command' ? '任务已停止' : '任务已取消', |
There was a problem hiding this comment.
[Suggestion] R4-21: This diff's rewrite turns the leak assertion of 'hides local image paths when output becomes %s' into a statement that cannot fail, while the test name still advertises path-hiding coverage for the cancel/stop flow. Verified two-armed probe: under a real leak mutation (image sanitization → identity) this test passes (blind), while the sibling '…when output ends with %s' fails (comparator alive). — Failure scenario: the cancelled branch of terminalizeRun replaces the record content with the fixed message (任务已停止/任务已取消) before cancelRun finalizes, so the terminal payload can only ever contain the fixed message plus a status line — run content never reaches it, and not.toContain('/Users/ben/private') cannot flip under any sanitization mutation; the only place the marker content actually flows (the streaming openOrUpdateStream write) is never asserted here. Pre-PR this assertion was live (content: 'before [Image pending] after' gated cancel-finalize sanitization); this diff vacated it. Suggested fix: delete the vacated assertion, or assert on the streaming writes instead (openOrUpdateStream payloads contain no /Users/ben/private — that assertion is live: it passes on production code and fails under the mutation).
中文说明
本 diff 对 'hides local image paths when output becomes %s' 的改写把其泄漏断言变成了不可能失败的语句，而测试名仍宣称覆盖 cancel/stop 流的路径隐藏。双臂探测验证：在真实泄漏变异（图像消毒 → 恒等）下本测试通过（盲），姊妹测试 '…when output ends with %s' 失败（对照活着）。失败场景：terminalizeRun 的 cancelled 分支先用固定文案（任务已停止/任务已取消）替换记录内容再终结，终态 payload 只可能含固定文案与状态行——run 内容永远到不了终态 payload，not.toContain('/Users/ben/private') 对任何消毒变异都不可翻转；标记内容真正流经的流式 openOrUpdateStream 写入该测试从不断言。PR 之前该断言是活的（content: 'before [Image pending] after' 钉住 cancel 终结时的消毒），本 diff 使其空置。建议：删除空置断言，或改断言流式写入（openOrUpdateStream payload 不含 '/Users/ben/private'——该断言是活的：生产代码下通过、变异下失败）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
Released in v0.21.7. |
5 participants
What this PR does
This PR gives each DingTalk task run one continuous interactive status card. The card is created when the task starts, visible model output keeps streaming into the same card across response boundaries, elapsed time refreshes while the model or a tool is idle, and the final answer replaces the running content on that card. With
atSenderenabled, running and intermediate card content stays uncluttered while the terminal card starts with exactly one Markdown-safe@display name, including when the model already emitted the same raw or escaped prefix. Non-card webhook replies now consistently use DingTalk Markdown while retaining real first-chunkatUserIdsmetadata. Failure and cancellation paths finalize the card with safe user-facing text instead of leaking buffered model details or local image paths.Why it's needed
The current presenter creates a status card only after the first visible text chunk, keys presentation directly to individual output segments, and refreshes elapsed metadata only during content flushes. A tool-using turn can therefore show no initial status, split one logical response into multiple cards, and leave
Running · 0sstale during long quiet phases. The fallback sender-attribution path also switches replies to plain text, which prevents Markdown links from rendering, while adding attribution in both the model output and the delivery layer can display the sender twice.Reviewer Test Plan
How to verify
atSender, then send a prompt that emits visible intermediate text, crosses a tool response boundary, and returns a Markdown answer containing a link.@display nameappears at the beginning even if the model output already starts with one or more equivalent raw/escaped prefixes.msgtype: markdown, only its first chunk carriesat.atUserIds, and the card-only display-name prefix is not copied into the fallback body.Evidence (Before & After)
atSenderfallback replies use plain text, so Markdown links are not rendered@display nameAutomated coverage exercises eager creation, cross-boundary streaming, final replacement, idle elapsed refresh, sender-prefix escaping and deduplication, Markdown fallback chunking and at metadata, cancellation, and failure before or after output. The updated behavior was also manually verified with the current DingTalk bot in a group conversation.
Tested on
Environment (optional)
Linux, Node.js v22.23.0. Verified with
npx vitest runinpackages/channels/dingtalk(255 passed),npm run buildin that package, full-repositorynpm run typecheck,npm run lint -- --quiet, andgit diff --check.Risk & Scope
Linked Issues
Fixes #8563
中文说明
本 PR 做了什么
本 PR 让每个钉钉任务 run 始终使用同一张交互式状态卡。任务开始时立即创建卡片；跨 response boundary 的可见模型文本继续写入原卡片；模型或工具处于静默阶段时耗时仍持续刷新；最终答案在同一张卡片上替换 Running 内容。启用
atSender后，Running 和中间过程不显示发送者，终态卡片固定以一个经过 Markdown 转义的@显示名开头；即使模型已经输出了相同的原始或转义前缀，也只保留一个。非卡片 webhook 回复统一使用钉钉 Markdown，同时保留首个分片真实的atUserIds元数据。失败和取消路径会使用安全的用户可见文案终结卡片，不泄露模型缓冲详情或本地图片路径。为什么需要
当前 presenter 只在首个可见文本块到达后创建状态卡，展示状态直接按输出 segment 划分，耗时元数据也只在内容 flush 时刷新。因此一个包含工具调用的回合可能没有初始状态、被拆成多张卡片，并在长时间静默阶段一直显示过期的
Running · 0s。发送者归属的 fallback 路径还会把回复切换成纯文本，导致 Markdown 链接无法渲染；如果模型输出和发送层都添加归属信息，还可能显示两次发送者。Reviewer 测试计划
如何验证
atSender，发送一个会输出可见中间文本、跨越工具 response boundary，并最终返回带链接 Markdown 答案的问题。@显示名。msgtype: markdown，只有首个分片携带at.atUserIds，且卡片专用的显示名前缀不会复制到 fallback 正文。Before & After 证据
atSenderfallback 回复使用纯文本，Markdown 链接无法渲染@显示名自动化测试覆盖了立即创建、跨 boundary 流式更新、最终替换、空闲耗时刷新、发送者前缀转义与去重、Markdown fallback 分片与 at 元数据、取消，以及首段输出前后的失败。更新后的行为也已经通过当前钉钉机器人在群聊中完成实测。
测试平台
环境（可选）
Linux，Node.js v22.23.0。已在
packages/channels/dingtalk运行npx vitest run（255 通过）和npm run build，并运行全仓npm run typecheck、npm run lint -- --quiet与git diff --check。风险与范围
关联 Issue
Fixes #8563