fix(channels): recover DWS direct messages - #10274
Conversation
E2E test report
|
github-actions
Bot
added
the
review/self-reported
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The linked issue (#10267) carries concrete reproductions for all three defects — the invisible quoted-reply delivery, the missed-DM gap with no history recovery, and stale replays marked processed before recovery. Issue triage already confirmed all three behaviors in source ( Direction: aligned — this repairs delivery reliability of an existing channel integration (DWS). Responses that succeed but never render, and requests permanently lost after a brief consumer gap, are message-loss bugs for users of that channel, not hardening. No direct reference in the upstream CHANGELOG, but the area is squarely within the channel feature surface. Size: not applicable — Approach: the scope feels right. The three changes map one-to-one onto the fix direction from the issue triage: plain direct-message send for final responses (groups keep quoted replies), ordinary DMs dispatched through the existing history pipeline, and stale replays left unmarked with the watermark pullback reused. Nothing extra rides along; the fourth suggestion from the issue triage (a distinct Risk: no elevated risk signals — neither changed file matches the repo's revert-correlated paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实存在的问题，不是理论性的。关联 issue（#10267）对三个缺陷都给出了具体复现——引用回复投递后不可见、消费者中断期间的私聊消息无法补偿、过期重放在恢复前就被标记为已处理。issue 分诊已在源码中确认了这三个行为（ 方向：对齐——这是修复现有 channel 集成（DWS）的投递可靠性。回复成功却不可见、短暂中断后请求永久丢失，对该 channel 的用户而言是消息丢失类 bug，而不是加固。上游 CHANGELOG 没有直接对应条目，但该领域完全在 channel 功能范围内。 规模：不适用—— 方案：范围合理。三处改动与 issue 分诊给出的修复方向一一对应：私聊最终回复改走普通发送（群聊仍用引用回复）、普通私聊进入现有历史处理管线、过期重放保持未标记并复用水位回拉。没有夹带额外改动；issue 分诊建议的第四项（针对 风险：无升级风险信号——两个改动文件均未命中与本仓库 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI formed my own proposal from the issue before reading the diff (plain send for direct finals, ordinary DMs dispatched through the existing history path with its dedup, stale replays left unmarked for the watermark pullback) — the PR matches it almost exactly, in 31 production lines. No critical findings. The three things this diff invites scrutiny on all check out:
Reuses existing machinery throughout ( TestingUnattended CI run — I did not build or execute PR code; the evidence below is the PR's own CI fetched via the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The primary unit suite (ubuntu) was still running at review time; the finalize job will refresh the table above once CI settles. The macOS/Windows test jobs run only in the merge queue per Not verified: live DWS delivery. The invisibility premise (a cross-identity quoted reply accepted by DWS but never rendered) and its fix require real DingTalk identities, and rest on the author's adapter probes on macOS — the author's claim, not independently re-run. The unit tests pin the routing, dedup, and pullback logic (they flip their expectations with the behavior), but cannot observe DingTalk-side rendering. Sandboxed verification would narrow the remaining gap: 中文说明代码审查读 diff 之前我先根据 issue 给出了自己的方案（私聊最终回复改普通发送、普通私聊走现有历史路径及其去重、过期重放保持未标记等待水位回拉）——PR 几乎完全一致，生产代码仅 31 行。没有发现阻塞问题。这个 diff 最值得审视的三点均核实无误：
全程复用现有机制（ 测试无人值守 CI 运行——未构建或执行 PR 代码；以下证据为 PR 自身 CI，经 API 获取。 CI 表格见英文部分。审查时 ubuntu 主测试套件仍在运行，finalize 任务会在 CI 结束后更新表格。macOS/Windows 测试任务按 未验证：真实 DWS 投递。不可见性前提（跨身份引用回复被 DWS 接受但不渲染）及其修复需要真实钉钉账号，依赖作者的 macOS 适配器探针——作者自述，未独立复现。单元测试固定了路由、去重与水位回拉逻辑（断言随行为翻转），但无法观测钉钉端渲染。沙箱验证可以缩小剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix that matches the issue's source-confirmed defects and pins the new behavior in tests; the one reservation is that live DingTalk delivery rests on the author's probes, which no CI lane can reach. Stepping back: this is the kind of PR the gate should pass. The problem is real and was confirmed in source before this PR existed — successful agent turns rendering as silence, and requests lost forever after a brief consumer gap, are message-loss bugs, not theoretical hardening. The fix is 31 production lines that rewire three existing mechanisms rather than adding new ones; my independent proposal and the diff converged, and I didn't find a simpler path it missed. Every change earns its place, the tests flip their expectations with the behavior (they would fail against base), and the acknowledged tradeoff — direct finals losing quoted context — is a declared scope item, not a hidden cost. If I were maintaining this in six months, the updated R4-4/R12-1 rationale comments are exactly the breadcrumbs I'd want. Approving, with approval deferred until CI lands green on 中文说明置信度：4/5 —— 干净、最小化的修复，与 issue 中经源码确认的缺陷一一对应，且新行为有测试固定；唯一的保留意见是真实钉钉投递依赖作者的探针验证，任何 CI 通道都无法覆盖。 整体来看：这正是门禁应当放行的 PR。问题真实存在，且在本 PR 之前就已在源码中确认——成功的执行在客户端表现为静默、短暂中断后请求永久丢失，这是消息丢失类 bug，不是理论性加固。修复仅 31 行生产代码，重新接线了三个现有机制而非新增机制；我的独立方案与 diff 收敛，也没有找到更简单的被遗漏路径。每一行改动都有必要，测试断言随行为翻转（对 base 构建会失败）；已声明的取舍——私聊最终回复失去引用上下文——是明确的范围条目而非隐藏成本。六个月后维护这段代码时，R4-4/R12-1 更新后的理由注释正是需要的线索。 予以批准，但批准推迟到该提交（ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6a": did not run npx vitest run src/dws-channel.test.ts in packages/channels/dws to execute the new tests (findings rest on code-path tracing, not test execution….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6a"：did not run npx vitest run src/dws-channel.test.ts in packages/channels/dws to execute the new tests (findings rest on code-path tracing, not test execution…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (notification) { | ||
| await this.processDocumentNotification(message, key, notification); | ||
| } else { | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] A direct message whose agent turn keeps failing is now driven twice per poll — once by replayPendingMessages (the message is parked in pendingMessages and left unmarked) and again by this new history dispatch — so the shared MAX_INBOUND_ATTEMPTS = 5 retry budget burns at about 2 attempts per poll instead of the pre-change 1. When handleInbound throws on a transient outage, the message is dropped after roughly 2 polls (~10s) instead of ~5 (~25s), so a transient outage lasting slightly more than two poll intervals now permanently loses the message — narrowing exactly the resilience the R12-1 comment this diff updates claims to preserve. Verified by probe on this commit: a failing DM present in both the pending queue and history hit 5 attempts and was dropped after 2 polls (afterEmit=1 afterPoll1=3 afterPoll2=5 pendingLeft=0), while reverting this branch to the pre-PR skip gave 1 attempt per poll, still retrying (afterPoll1=2 afterPoll2=3 pendingLeft=1). Skip keys already parked for redelivery before dispatching from history — replayPendingMessages re-drives them every poll already, so history stays the redundancy for messages the event stream missed entirely:
| await this.handleImMessage({ kind: 'direct' }, message, true); | |
| if ( | |
| (this.cursor.pendingMessages ?? []).some( | |
| (pending) => messageKey(pending.message) === key, | |
| ) | |
| ) { | |
| continue; | |
| } | |
| await this.handleImMessage({ kind: 'direct' }, message, true); |
Fix witness: please add a test that emits a DM with inboundError set, seeds the same message into client.directMessages, polls twice, and asserts inboundAttempts is 3 (live + one per poll), not 5 — then remove the guard and confirm the test goes red.
中文说明
会话执行持续失败的私聊消息现在每次轮询会被驱动两次——一次来自 replayPendingMessages（消息被挂起在 pendingMessages 中且未标记已处理），另一次来自这里新增的历史分发——导致共享的 MAX_INBOUND_ATTEMPTS = 5 重试预算以每次轮询约 2 次的速度消耗，而不是改动前的 1 次。当 handleInbound 因瞬时故障抛出异常时，消息在约 2 次轮询（约 10 秒）后就会被丢弃，而不是约 5 次（约 25 秒），因此持续略长于两个轮询间隔的瞬时故障现在会永久丢失消息——恰好收窄了本 diff 所更新的 R12-1 注释声称要保留的韧性。已在该提交上用探针验证：同时存在于挂起队列和历史中的失败消息在 2 次轮询后达到 5 次尝试并被丢弃（afterEmit=1 afterPoll1=3 afterPoll2=5 pendingLeft=0）；而将此分支还原为 PR 前的跳过逻辑后，每次轮询仅 1 次尝试且仍在重试（afterPoll1=2 afterPoll2=3 pendingLeft=1）。建议在从历史分发前跳过已挂起待重投的消息键——replayPendingMessages 每次轮询已经会重新驱动它们，历史路径应只作为事件流漏掉消息的兜底。
修复见证：请补充一个测试——在设置 inboundError 的情况下发出一条私聊消息，并把同一条消息放入 client.directMessages，轮询两次后断言 inboundAttempts 为 3（实时 + 每轮询一次）而不是 5；随后移除该守卫，确认测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (notification) { | ||
| await this.processDocumentNotification(message, key, notification); | ||
| } else { | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] This PR claims "Fixes #10267", but the issue's SearchRightsDenied observation is not addressed. The issue's repro section states that when the account lacks message-search rights, both mention and direct-message history calls fail with SearchRightsDenied, additionally removing recovery for group mentions and document notifications, and the triage comment's fix direction 4 asks to detect persistent history-fetch failure and surface one clear operator warning instead of per-poll stderr noise. There is no SearchRightsDenied handling anywhere in packages/channels/dws. On an organization that has not granted DWS message-search access, both history calls throw on every poll, the existing catch blocks write one stderr line per poll and leave the watermark unchanged, the new recovery machinery waits indefinitely on history that never succeeds, parked stale replays stay unmarked forever, and the operator sees unbounded per-poll noise with no single clear diagnostic. If this PR closes #10267, that part of the issue is silently dropped. Either implement fix direction 4 (detect a repeated identical history-fetch failure such as SearchRightsDenied and emit one clear operator warning instead of per-poll stderr lines), or file a follow-up issue for it and link it in the PR description so closing #10267 does not drop this scope.
中文说明
本 PR 声明 "Fixes #10267"，但该 issue 中的 SearchRightsDenied 观察并未被处理。issue 的复现部分指出：当账号缺少消息搜索权限时，@ 消息历史和私聊历史调用都会以 SearchRightsDenied 失败，进而同时失去群 @ 消息和文档通知的恢复能力；分诊评论的修复方向 4 要求检测持续的历史拉取失败，并给出一条清晰的运维告警，而不是每次轮询都输出 stderr 噪音。packages/channels/dws 中目前没有任何 SearchRightsDenied 处理。在未授予 DWS 消息搜索权限的组织中，两个历史调用每次轮询都会抛错，现有的 catch 块每次轮询只写一行 stderr 且水位保持不变，新增的恢复机制会无限期等待永远不会成功的历史，被挂起的过期重放会永远保持未处理状态，运维人员只能看到无限制的每轮询噪音，而没有任何单一的明确诊断。如果本 PR 关闭 #10267，该部分范围将被悄悄丢弃。建议实现修复方向 4（检测重复出现的相同历史拉取失败（如 SearchRightsDenied），并发出一条清晰的运维告警，取代每次轮询的 stderr 输出），或者为其创建一个后续 issue 并在 PR 描述中关联，避免关闭 #10267 时丢弃这部分范围。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Deferred to the follow-up queue (recorded in this round's deferred findings so it survives merge). Verified on this commit: there is no SearchRightsDenied or repeated-history-failure handling anywhere in packages/channels/dws. Issue #10267 fix direction 4 — detecting a persistent identical history-fetch failure and emitting one clear operator warning instead of per-poll stderr lines — is operator diagnostics beyond this PR's delivery-recovery purpose, needs its own threshold/warning/reset design and tests, and the failure mode predates this PR for mention and document-notification history (this PR does not regress it). The deferred-findings record is the follow-up-issue option the finding proposed; a maintainer can schedule it from the queue. If it should instead land in this PR (since it claims "Fixes #10267"), say so and it will be implemented next round.
转入后续队列（已记录在本轮的 deferred findings 中，合并后不会丢失）。已在该提交上验证：packages/channels/dws 中任何地方都没有 SearchRightsDenied 或重复历史拉取失败的处理。issue #10267 的修复方向 4——检测持续出现的相同历史拉取失败，并发出一条清晰的运维告警取代每次轮询的 stderr 噪音——属于超出本 PR 投递恢复目的的运维诊断能力，需要独立的阈值/告警/重置设计与测试，且该失败模式在本 PR 之前就已存在于 @ 消息历史和文档通知历史（本 PR 没有使其退化）。deferred-findings 记录即该发现提出的"创建后续 issue"选项；维护者可从队列中安排。如果它应该改为在本 PR 中落地（因为本 PR 声明 "Fixes #10267"），请回复说明，下一轮将实现。
| if (notification) { | ||
| await this.processDocumentNotification(message, key, notification); | ||
| } else { | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] The new ordinary-direct-message history fallback is undocumented, while both sibling history fallbacks are documented in docs/users/features/channels/dws.md — group-mention recovery at line 78 ("The channel also checks recent @ message history every five seconds…") and document-card recovery at line 90 ("a five-second incremental history check covers cards omitted by the current event stream"). This diff adds a third member of the same recovery family with no entry there, even though the PR's own Reviewer Test Plan treats the behavior as a headline deliverable. A user or operator seeing delayed-but-arriving DM replies — or debugging a recovered duplicate — will find no explanation in the doc. Adding a sentence next to the group-mention paragraph closes the gap, e.g. "Ordinary direct messages are recovered the same way: a five-second history check re-drives any direct message the real-time stream omitted, deduplicated by conversation and message ID across both paths."
中文说明
新增的普通私聊历史兜底没有文档说明，而同属一个恢复家族的两个兄弟机制都在 docs/users/features/channels/dws.md 中有记录——第 78 行的群 @ 消息恢复（"The channel also checks recent @ message history every five seconds…"）和第 90 行的文档卡片恢复（"a five-second incremental history check covers cards omitted by the current event stream"）。本 diff 为同一恢复家族新增了第三个成员，却没有补充相应条目，尽管 PR 自己的 Reviewer Test Plan 将该行为列为核心交付内容。看到私聊回复延迟送达（或排查恢复出的重复消息）的用户或运维人员将无从在文档中找到解释。建议在群 @ 段落旁补充一句话，例如："普通私聊消息也以同样方式恢复：每五秒一次的历史检查会重新驱动实时流漏掉的私聊消息，并通过会话 ID 和消息 ID 在两条路径间去重。"
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (!notification) continue; | ||
| await this.processDocumentNotification(message, key, notification); | ||
| if (notification) { | ||
| await this.processDocumentNotification(message, key, notification); |
There was a problem hiding this comment.
[Suggestion] This loop keeps its own parse-and-dispatch of document notifications even though handleImMessage → processImMessage already performs exactly that routing for { kind: 'direct' } sources, and both sibling replay surfaces (the mention-history loop at line ~1092 and replayPendingMessages at line ~1849) delegate unconditionally. Classification of direct-message content now lives in two places in the same file, and a divergence exists today: a history-fetched notification from an allowed sender skips rememberImTarget (called in processImMessage before the parse) while the identical live message records it. Any future change to direct-source dispatch in processImMessage — a new notification form, different trimming, new pre-processing — must be mirrored into this loop by hand; miss it and history polling classifies the same message differently from the live event stream. Delegating the whole message collapses the duplication:
} else {
await this.handleImMessage({ kind: 'direct' }, message, true);
}
// becomes simply (drop the pre-parse and the notification branch):
await this.handleImMessage({ kind: 'direct' }, message, true);With fromHistory = true the stale-replay branch is skipped, group gates do not apply to direct sources, and processDocumentNotification handles its own key marking identically on both routes. Fix witness: the existing 'finds document mention notifications in direct-message history when the event stream misses them' test must stay green, and stubbing out processImMessage's notification branch must turn it red once the loop delegates.
中文说明
该循环保留了自己对文档通知的解析与分发，但 handleImMessage → processImMessage 对 { kind: 'direct' } 来源已经实现了完全相同的路由，且两个兄弟重放面（约第 1092 行的 @ 消息历史循环和约第 1849 行的 replayPendingMessages）都是无条件委托。私聊消息内容的分类逻辑现在存在于同一文件的两个位置，并且今天就已存在分歧：从历史获取的、来自已允许发送者的通知会跳过 rememberImTarget（在 processImMessage 中解析前调用），而相同的实时消息却会记录它。未来对 processImMessage 中 direct 来源分发的任何改动——新的通知形式、不同的裁剪、新的预处理——都必须手工镜像到该循环；一旦遗漏，历史轮询对同一条消息的分类就会与实时事件流不同。将整条消息委托出去即可消除重复。修复见证：现有的 'finds document mention notifications in direct-message history when the event stream misses them' 测试必须保持绿色，并且在循环改为委托后，stub 掉 processImMessage 的通知分支应使该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| } | ||
| // A replayed document notification is left UNMARKED on purpose, for | ||
| // history polling to pick up. That only works if polling will ever look | ||
| // A replayed direct message is left UNMARKED on purpose, for history |
There was a problem hiding this comment.
[Suggestion] This park-and-pullback branch — now taken by every stale direct replay, not just document notifications — mutates persisted cursor state (watermark pulled backward, multi-page checkpoint dropped, message left unmarked) and calls saveCursor() without writing a single log line. Every comparable anomaly in this file logs: budget-exhausted drops ("dropping a DWS message after 5 failed turns"), degraded pending messages ("pending DWS message remains degraded"), degraded streams, and per-poll history-fetch failures. A 3 AM investigation of "the bot answered a days-old message" or "a reply arrived late/duplicated" finds no record in stderr that a stale replay arrived, how far the watermark was pulled back, or which message was left for history polling — the operator must reconstruct the event by diffing the persisted cursor file against wall-clock time. One sanitized stderr line closes the gap, e.g.:
process.stderr.write(
`[Channel:${this.name}] parked a stale direct message for history polling and pulled the watermark back to ${message.eventTime}: ${sanitizeLogText(message.messageId, 120)}\n`,
);中文说明
这个"挂起并回拉水位"分支——现在每一条过期的私聊重放都会走到，而不仅是文档通知——会修改持久化的游标状态（水位被向后拉、多页 checkpoint 被丢弃、消息保持未标记），并调用 saveCursor()，却不写任何一行日志。该文件中所有可类比的异常都会记录日志：预算耗尽的丢弃（"dropping a DWS message after 5 failed turns"）、降级中的挂起消息（"pending DWS message remains degraded"）、降级的流，以及每次轮询的历史拉取失败。凌晨三点排查"机器人回复了一条几天前的消息"或"回复迟到/重复"时，stderr 中找不到任何记录说明有过期重放到达、水位被回拉了多远、哪条消息被留给历史轮询——运维人员只能通过对比持久化游标文件与墙钟时间来重建事件。补一行脱敏的 stderr 日志即可补上该缺口。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| 'final answer', | ||
| expect.any(String), | ||
| ); | ||
| expect(client.sendImMessage).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] This rewritten group-reply test dropped the stable-idempotency-key pin: the deleted 'uses the originating message for an idempotent final reply' test asserted the v5 stableUuid pattern on the replyToImMessage key, but the replacement asserts only expect.any(String). The diff hoisted the key into one shared idempotencyKey variable feeding both branches, so a one-line regression at the reply call site (idempotencyKey → randomUUID()) now survives every test — verified by mutation: with randomUUID() substituted at the reply call site the full dws suite still passes 155/155. Each retried final group reply would then carry a fresh key, server-side dedup would miss it, and the quoted group reply would be posted twice on retry. Pin the same v5 pattern the direct test uses:
| 'final answer', | |
| expect.any(String), | |
| ); | |
| expect(client.sendImMessage).not.toHaveBeenCalled(); | |
| 'final answer', | |
| expect.stringMatching( | |
| /^[0-9a-f]{8}-[0-9a-f]{4}-5[0-9a-f]{3}-8[0-9a-f]{3}-[0-9a-f]{12}$/, | |
| ), | |
| ); | |
| expect(client.sendImMessage).not.toHaveBeenCalled(); |
Fix witness: this strengthened assertion must go red when idempotencyKey at the replyToImMessage call site in sendResponseMessage is replaced with a per-call random key.
中文说明
重写后的群回复测试丢掉了稳定幂等键的固定断言：被删除的 'uses the originating message for an idempotent final reply' 测试曾对 replyToImMessage 的键断言 v5 stableUuid 格式，但替代测试只断言 expect.any(String)。本 diff 将键提升为一个共享的 idempotencyKey 变量同时供给两个分支，因此回复调用处的一行回归（idempotencyKey → randomUUID()）现在能通过所有测试——已通过变异验证：在回复调用处替换为 randomUUID() 后，完整 dws 测试套件仍然 155/155 通过。这样每次重试的群最终回复都会携带新键，服务端去重会失效，引用回复会在重试时被发送两次。请固定与私聊测试相同的 v5 格式断言。修复见证：当 sendResponseMessage 中 replyToImMessage 调用处的 idempotencyKey 被替换为每次调用随机的键时，该加强后的断言必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| ]); | ||
| }); | ||
|
|
||
| it('dispatches an ordinary direct message when the event stream misses it', async () => { |
There was a problem hiding this comment.
[Suggestion] The new plain-direct history dispatch has no dedup test. Every history window re-opens at watermark − 5s (NOTIFICATION_HISTORY_OVERLAP_MS), so every live-dispatched DM is re-fetched by a later poll, and the only guard is cursor.processedMessages.includes(key) in the loop. The mention path has an exact analogue ('deduplicates a mention delivered by history and the live stream'), and the stale-replay test's second poll never reaches this guard because the replay is already outside the window by then — so if the guard regresses (loop reordered, mark moved), the same DM is dispatched a second time and the user gets a duplicate agent turn/reply with no test going red. A probe on this commit confirmed the guard works today (live emit + same message in history → dispatched exactly once), but it is untested. Mirror the mention dedup test: emit a fresh user_im_message_receive_o2o_all event live, set client.directMessages = [the same message], await channel.poll(), and assert channel.inbound has length 1. Fix witness: that new test must go red when the processedMessages.includes(key) guard in pollOnce's direct-message history loop is removed.
中文说明
新的普通私聊历史分发缺少去重测试。每个历史窗口都会从 watermark − 5s（NOTIFICATION_HISTORY_OVERLAP_MS）重新打开，因此每条经实时流分发的私聊消息都会被后续轮询再次拉取，唯一的守卫是循环中的 cursor.processedMessages.includes(key)。@ 消息路径有完全对应的测试（'deduplicates a mention delivered by history and the live stream'），而过期重放测试的第二次轮询也不会触及该守卫，因为重放消息届时已在窗口之外——所以一旦该守卫发生回归（循环顺序调整、标记位置移动），同一条私聊消息会被第二次分发，用户会收到重复的会话执行/回复，而没有任何测试变红。已在该提交上用探针确认守卫目前有效（实时发出 + 历史中存在同一条消息 → 只分发一次），但它没有测试覆盖。请仿照 @ 消息去重测试：实时发出一条新的 user_im_message_receive_o2o_all 事件，设置 client.directMessages = [同一条消息]，await channel.poll()，断言 channel.inbound 长度为 1。修复见证：当 pollOnce 私聊历史循环中的 processedMessages.includes(key) 守卫被移除时，该新测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary (PR #10274)Addressed six of seven findings in code/docs; deferred one ( Findings and dispositions
VerificationCommands actually run on the final state (commit
Mutation probes (each: mutate → focused test red → restore → green):
No settings source changed (no schema regeneration needed). The touched behavior is exercised by the channel's own unit tests; no bundled-CLI/integration path is involved. 中文说明审查轮次总结（PR #10274）七条发现中六条已在代码/文档中处理；一条（ 发现与处置
验证在最终状态（提交
变异探针（每项：变异 → 聚焦测试变红 → 恢复 → 变绿）：
未改动 settings 源（无需重新生成 schema）。改动行为由 channel 自身的单元测试覆盖；不涉及打包 CLI/集成测试路径。 🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 SearchRightsDenied follow-up — already reported (comment 3871156555), deferred to the follow-up queue per the reply in that thread
Not explored to full depth (tool budget reached): "agent 3b": none — no check was cut short.; "agent 5": executing packages/channels/dws/src/dws-channel.test.ts at the reviewed commit — no node_modules exists in the worktree or the parent checkout, and a full monor….
中文说明
已审查。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 3b"：none — no check was cut short.；"agent 5"：executing packages/channels/dws/src/dws-channel.test.ts at the reviewed commit — no node_modules exists in the worktree or the parent checkout, and a full monor…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if ( | ||
| (this.cursor.pendingMessages ?? []).some( | ||
| (pending) => messageKey(pending.message) === messageKey(message), | ||
| ) | ||
| ) { | ||
| continue; | ||
| } | ||
| const notification = parseDocumentMentionNotification(message.content); | ||
| if (!notification) continue; | ||
| await this.processDocumentNotification(message, key, notification); | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] R1-1: (fix-induced) The pending-skip guard landed for R1-1 replaced the loop-level self-message and processed-key skips, so every re-fetched window message now goes through handleImMessage. Its self-message branch runs before the processed-key early return and calls saveCursor() unconditionally, so each of the bot's own replies sitting in the overlap window costs one full blocking cursor persist per poll — mkdirSync + JSON.stringify of the entire cursor (up to 5,000 processed keys plus pending message bodies) + writeFileSync + renameSync — on top of the end-of-poll persist runLoop already performs. The pending scan also recomputes messageKey(message) once per pending entry. Every 5 s poll re-fetches the trailing 5 s overlap, so this is steady-state cost; when the watermark is stuck or pulled back (outage, stale-replay pullback) the window covers the whole backlog and both costs grow with it. Probe at this commit: a page of 3 already-marked self-messages saves the cursor 4 times per poll (3 redundant + 1 end-of-poll); with a loop-level processed-key skip patched in, the second poll saves once.
| if ( | |
| (this.cursor.pendingMessages ?? []).some( | |
| (pending) => messageKey(pending.message) === messageKey(message), | |
| ) | |
| ) { | |
| continue; | |
| } | |
| const notification = parseDocumentMentionNotification(message.content); | |
| if (!notification) continue; | |
| await this.processDocumentNotification(message, key, notification); | |
| await this.handleImMessage({ kind: 'direct' }, message, true); | |
| const key = messageKey(message); | |
| if (this.cursor.processedMessages.includes(key)) { | |
| continue; | |
| } | |
| if ( | |
| (this.cursor.pendingMessages ?? []).some( | |
| (pending) => messageKey(pending.message) === key, | |
| ) | |
| ) { | |
| continue; | |
| } | |
| await this.handleImMessage({ kind: 'direct' }, message, true); |
The existing 'deduplicates a direct message delivered by the live stream and history' and 'spends one retry per poll on a failed direct message also in history' tests pin the dedup/skip behaviour this fix must preserve — please confirm they stay green after applying it.
中文说明
为 R1-1 落地的挂起跳过守卫同时移除了循环级的自身消息跳过和已处理键跳过，因此每条重新获取到的窗口消息现在都会进入 handleImMessage。其自身消息分支在已处理键提前返回之前执行，并且无条件调用 saveCursor()，于是重叠窗口中机器人的每一条回复都会在每次轮询时产生一次完整的阻塞式游标持久化——mkdirSync + 对整个游标（最多 5,000 个已处理键以及挂起消息体）做 JSON.stringify + writeFileSync + renameSync——叠加在 runLoop 本来就会执行的轮询结束持久化之上。挂起扫描还会对每个挂起条目重复计算一次 messageKey(message)。每 5 秒的轮询都会重新获取末尾 5 秒的重叠窗口，所以这是稳态开销；当水位卡住或被回拉（故障、过期重放回拉）时，窗口会覆盖整个积压，两项开销随之放大。在本提交上用探针验证：一页 3 条已标记的自身消息每次轮询会保存游标 4 次（3 次冗余 + 1 次轮询结束保存）；补上循环级已处理键跳过后，第二次轮询只保存 1 次。
现有的 'deduplicates a direct message delivered by the live stream and history' 和 'spends one retry per poll on a failed direct message also in history' 测试固定了本修复必须保留的去重/跳过行为——应用后请确认它们保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const notification = parseDocumentMentionNotification(message.content); | ||
| if (!notification) continue; | ||
| await this.processDocumentNotification(message, key, notification); | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] The history dispatch now relies on handleImMessage's self-message check as the only filter for the bot's own messages in DM history, but no test puts a self-sent message into history and polls. This PR makes final direct responses ordinary messages and makes the history loop dispatch every DM-history message, so the bot's own replies reappear in that very window on every poll. If the self-check were ever conditioned on fromHistory (a one-token change), every poll would re-dispatch the bot's own reply as a fresh inbound user turn — the agent answers its own messages — and nothing in the suite would go red: a probe with exactly that mutant placed a self-sent message in history, observed it dispatched as a fresh inbound turn, while the full existing suite stayed 157/157 green. Please add a history-path self-message test, e.g.:
const client = new FakeDwsClient();
const channel = await readyChannel(client);
client.directMessages = [
message('user_im_message_receive_o2o_all', 'own-reply', 'bot text', {
senderId: 'open-self',
}),
];
await channel.poll();
expect(channel.inbound).toEqual([]);
await channel.poll();
expect(channel.inbound).toEqual([]);Fix witness: the new test must go red if the self-check in handleImMessage is gated on !fromHistory — remove that condition and confirm it fails.
中文说明
历史分发现如今依赖 handleImMessage 中的自身消息检查，作为私聊历史中机器人自身消息的唯一过滤，但目前没有测试把自身发送的消息放入历史再执行轮询。本 PR 让私聊最终回复变成普通消息，并让历史循环分发每一条私聊历史消息，因此机器人自己的回复会在每次轮询时重新出现在同一个窗口中。如果该自身检查将来被加上 fromHistory 条件（单字符改动），每次轮询都会把机器人自己的回复当作新的用户输入再次分发——机器人回复自己的消息——而测试套件不会变红：用这个变异体做探针时，放入历史中的自身消息被观察为以新的入站轮次分发，而完整的现有套件仍为 157/157 全绿。请补充一个历史路径的自身消息测试，例如：
（测试代码见上方英文部分）
修复见证：如果把 handleImMessage 中的自身检查加上 !fromHistory 门控，新测试必须变红——移除该条件并确认测试失败。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const notification = parseDocumentMentionNotification(message.content); | ||
| if (!notification) continue; | ||
| await this.processDocumentNotification(message, key, notification); | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Suggestion] An inbound-turn failure during history dispatch escapes handleImMessage into the listDirectMessages catch and is logged as "failed to poll DWS direct-message history", mislabeling an agent-side failure as a DWS API polling failure; this PR widens the trigger from rare document notifications to every ordinary DM. Probe at this commit with a failing turn and two DMs in the window: the poll logged both DWS message turn failed (attempt 1/5): agent unavailable and failed to poll DWS direct-message history: agent unavailable, attempted only the first message, and left the watermark unmoved — the rest of the page waits one extra poll, and oncall reading "failed to poll" starts investigating DingTalk health instead of the model backend. Messages are still parked and recovered next poll, so this is observability/latency, not loss. Wrapping the dispatch in its own try/catch keeps the page moving and reserves the fetch-failure message for real API errors:
| await this.handleImMessage({ kind: 'direct' }, message, true); | |
| try { | |
| await this.handleImMessage({ kind: 'direct' }, message, true); | |
| } catch (error) { | |
| process.stderr.write( | |
| `[Channel:${this.name}] direct-message dispatch failed mid-window; the message is parked for retry: ${sanitizeLogText(error instanceof Error ? error.message : String(error), 300)}\n`, | |
| ); | |
| } |
Fix witness: a variant of 'spends one retry per poll on a failed direct message also in history' asserting failed to poll DWS direct-message history is NOT written when the fetch succeeds and only the turn throws — removing this catch must turn that test red.
中文说明
历史分发过程中的入站轮次失败会从 handleImMessage 逃逸进 listDirectMessages 的 catch，被记录为 "failed to poll DWS direct-message history"，把 Agent 侧的失败误标为 DWS API 轮询失败；本 PR 把触发面从少见的文档通知扩大到了每一条普通私聊。在本提交上用探针验证：令轮次失败、窗口中有两条私聊消息时，一次轮询同时输出了 DWS message turn failed (attempt 1/5): agent unavailable 和 failed to poll DWS direct-message history: agent unavailable，只尝试了第一条消息，水位也未前进——页面剩余部分要多等一次轮询，而凌晨值班的人看到 "failed to poll" 会先去排查钉钉健康而不是模型后端。消息仍会被挂起并在下次轮询恢复，因此这是可观测性/延迟问题，不是丢消息。把分发包进独立的 try/catch 可以让页面继续处理，并把拉取失败的日志留给真正的 API 错误：
（修复代码见上方英文部分）
修复见证：基于 'spends one retry per poll on a failed direct message also in history' 的变体，断言在拉取成功、仅轮次抛错时不会写出 failed to poll DWS direct-message history——移除这个 catch 必须使该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed (round 3)All three inline Suggestion findings are resolved in the code; no findings were declined, deferred, or escalated this round. The previously reported R1-2 finding stays in the follow-up queue as agreed in its thread — it was not re-actioned here. [rc:3873093849] Redundant blocking cursor persist per re-fetched self-message — fixedConfirmed on the reviewed commit: with the R1-1 pending-skip guard having replaced the loop-level self-message and processed-key skips, every re-fetched window message re-entered Change: hoisted Witness: [rc:3873093864] No test puts a self-sent message into DM history and polls — fixedAdded Witness: mutation probe gating the self-check on [rc:3873093872] Turn failure during history dispatch mislogged as a fetch failure and aborts the page — fixed, with one deliberate deviationConfirmed on the reviewed commit: an under-budget turn throw escaped Change: wrapped the per-message dispatch in its own try/catch. The deviation from the suggested snippet: the catch rethrows when the failed message was NOT parked (its key is absent from Witnesses: Conflict notesNo base conflict ( VerificationCommands actually run and their results:
中文说明已处理的评审反馈（第 3 轮）三条行内 Suggestion 级发现均已在代码中解决；本轮没有拒绝、推迟或升级任何发现。此前已报告的 R1-2 发现按其线程中的约定保留在后续跟进队列中——本轮未对其重复处理。 [rc:3873093849] 每条重新获取的自身消息都产生一次冗余的阻塞式游标持久化 — 已修复在被审查的提交上确认：R1-1 的挂起跳过守卫替换了循环级的自身消息跳过和已处理键跳过后，每条重新获取到的窗口消息都会重新进入 改动：将 见证： [rc:3873093864] 没有测试把自身发送的消息放入私聊历史再轮询 — 已修复新增 见证：变异探针把自身检查加上 [rc:3873093872] 历史分发过程中的轮次失败被误记为拉取失败并中断整页处理 — 已修复，有一处刻意的偏差在被审查的提交上确认：预算内的轮次抛错会从 改动：给每条消息的分发包上独立的 try/catch。与建议代码片段的偏差：当失败消息未被挂起（其键不在 见证： 冲突说明无基线冲突（ 验证实际执行的命令及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Convergence: round 3 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/channels/dws/src/dws-channel.ts (findings in rounds 1, 2; 4 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
收敛情况：第 3 轮发布了 4 条行内评论，其中 4 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/channels/dws/src/dws-channel.ts（第 1、2 轮已出过发现，本轮又有 4 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
| continue; | ||
| } | ||
| try { | ||
| await this.handleImMessage({ kind: 'direct' }, message, true); |
There was a problem hiding this comment.
[Critical] R3-1: The parked-skip above protects only messages that are ALREADY parked when this dispatch runs. A direct message whose live-stream agent turn is still in flight passes the skip, and handleImMessage blocks on its in-flight wait (~line 1538); after the wait it re-checks only cursor.processedMessages (~line 1543). When the live turn fails, processImMessage's catch parks the message and charges attempt 1 — but parked ≠ processed, so this history dispatch immediately starts a SECOND full turn in the same poll and charges attempt 2. That is exactly the double retry-budget spend the parked-skip's own comment forbids, leaking through the in-flight edge, and this diff introduces the path: before it, the loop only dispatched document notifications. Concrete trigger: a direct message whose agent turn runs longer than the 5s poll interval (routine for agent turns) and then fails — a transient agent-backend outage, the exact scenario this PR exists for. The poll whose history window covers the message blocks on the in-flight wait; the live turn throws and parks it (attempt 1); the processed-only re-check misses; the second turn fails too (attempt 2), swallowed by the discriminator below. Exactly one double-spend per message is possible (afterwards the skip applies), but that compresses the 5-attempt grace from 5 polls to 4 — an outage recovering inside that lost 5s window permanently drops the user's message, one poll earlier than the retry contract this same diff pins. The pinned test spends one retry per poll on a failed direct message also in history cannot catch this: its emit rejects before poll() runs, so no turn is ever in flight in the suite. Witness (probe on the unmodified PR code in a scratch tree):
PROBE-RESULT attempts=2 gates=2 attemptsAtSecondTurn=2 inbound=0
budget=[{"key":"cid-1\u0000inflight-double-spend","attempts":2}]
two full agent turns for one message in one poll, all 161 pre-existing tests green; with the gated fix below, attempts=1 gates=1 and 162/162 pass. Fix: in handleImMessage, track whether the wait loop actually waited on an in-flight task, and after the wait also bail when the key got parked while waiting — next to the existing processedMessages re-check:
if (
waitedOnInFlight &&
(this.cursor.pendingMessages ?? []).some(
(pending) => messageKey(pending.message) === key,
)
) {
return;
}The re-check MUST be gated on having waited: an UNCONDITIONAL parked re-check after the wait breaks the pinned test replays a failed direct message on the next poll, because replayPendingMessages dispatches parked messages through the same handleImMessage — an ungated re-check kills replay outright. The gated variant keeps the whole suite green (verified in the scratch tree). Fix witness: please add a race variant of spends one retry per poll on a failed direct message also in history — hold the live turn in flight with a controllable deferred inboundHandler rejection while channel.poll()'s history loop reaches the message, then assert inboundAttempts is 1 (not 2) after both settle; removing the gated re-check must turn it red.
中文说明
上方的挂起跳过守卫只保护在本次分发时已经被挂起的消息。一条实时流 Agent 轮次仍在执行中的私聊消息会通过该跳过，随后 handleImMessage 会阻塞在其在途等待上（约第 1538 行）；等待结束后只重新检查 cursor.processedMessages（约第 1543 行）。当实时轮次失败时，processImMessage 的 catch 会把消息挂起并记第 1 次尝试——但挂起 ≠ 已处理，于是这里的历时分发会立即在同一次轮询中启动第二个完整轮次并记第 2 次尝试。这正是挂起跳过守卫自己的注释所禁止的每轮询双倍重试预算消耗，经由在途边缘泄漏出来；而且这条路径是本 diff 引入的：此前该循环只分发文档通知。具体触发：一条私聊的 Agent 轮次执行时间超过 5 秒轮询间隔（对 Agent 轮次而言很常见）并且随后失败——即瞬时 Agent 后端故障，正是本 PR 要兜底的场景。历史窗口覆盖该消息的那次轮询阻塞在在途等待上；实时轮次抛错并将其挂起（第 1 次尝试）；仅检查已处理键的复查不会命中；第二个轮次也失败（第 2 次尝试），被下方的判别器吞掉。每条消息只可能发生一次双倍消耗（此后跳过守卫生效），但这会把 5 次尝试的宽限从 5 次轮询压缩到 4 次——在那个丢失的 5 秒窗口内恢复的故障会永久丢弃用户消息，比本 diff 自身用测试固定的重试契约早一次轮询。已固定的测试 spends one retry per poll on a failed direct message also in history 抓不到这一点：它的 emit 在 poll() 运行之前就已拒绝，因此测试套件中从不存在在途轮次。见证（在临时树上对未修改的 PR 代码运行探针）：两次完整 Agent 轮次发生在同一条消息的同一次轮询中（预算记录 attempts:2），161 个既有测试全部通过；应用下方的门控修复后 attempts=1 gates=1，162/162 通过。修复：在 handleImMessage 中记录等待循环是否真的等待过一个在途任务，并在等待结束后、紧挨现有 processedMessages 复查处，对等待期间被挂起的键也提前返回（见上方英文代码块）。该复查必须以确实等待过为门控：无条件的挂起重查会破坏已固定测试 replays a failed direct message on the next poll，因为 replayPendingMessages 通过同一个 handleImMessage 分发已挂起消息——不加门控的重查会让重放彻底失效。门控变体在整个套件上保持绿色（已在临时树验证）。修复见证：请新增 spends one retry per poll on a failed direct message also in history 的竞态变体——用可控的延迟 inboundHandler 拒绝让实时轮次保持在途，同时 channel.poll() 的历史循环到达该消息；在两者落定后断言 inboundAttempts 为 1（而不是 2）；移除门控重查必须使该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if ( | ||
| (this.cursor.pendingMessages ?? []).some( | ||
| (pending) => messageKey(pending.message) === key, | ||
| ) | ||
| ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-2: (fix-induced) The round-2 finding's own input is fixed — the per-message try/catch below keeps the page moving for parked failures and both witness tests pass — but the fix that closed it hand-inlined the parked-message identity predicate a second time: this pre-dispatch skip and the rethrow discriminator below are character-identical copies, and the same query shape already exists in rememberPendingMessage (~line 1712). The file's own convention for the sibling queue is a named predicate — hasPendingDocumentNotification(notificationKey) (~line 1923), used from two call sites — which was not followed here. The cost is concrete: the discriminator's swallow-vs-rethrow decision silently depends on both copies staying identical. A future edit to parked-entry identity (e.g. PersistedPendingMessage gains a stored key, or messageKey inputs change) must be applied consistently in three places; updating this skip but not the discriminator silently re-introduces double retry-budget spend per poll (the R1-1 regression), and the reverse swallows an unparked document-notification failure, lets the watermark advance past it, and strands its remaining budget — reintroducing the starvation the original R2-2 fix removes — with no compiler or test signal at edit time, because the copies are independent literals today. Extract one helper mirroring hasPendingDocumentNotification and call it from all three sites:
private hasPendingMessage(key: string): boolean {
return (this.cursor.pendingMessages ?? []).some(
(pending) => messageKey(pending.message) === key,
);
}The extraction is behavior-preserving, so no test witness applies; the behaviours the two call sites protect stay pinned by spends one retry per poll on a failed direct message also in history and keeps spending the retry budget of an unparked document notification.
中文说明
（fix-induced）第 2 轮发现本身的问题已修复——下方的逐消息 try/catch 让页面在已挂起失败时继续前进，两个见证测试均通过——但关闭该发现的修复把挂起消息身份判定第二次手写内联：这个分发前跳过与下方的重抛判别器是逐字符相同的两份拷贝，而同一查询形态在 rememberPendingMessage（约第 1712 行）中已经存在。本文件对姊妹队列的既有约定是命名谓词——hasPendingDocumentNotification(notificationKey)（约第 1923 行，有两个调用点——此处未遵循。代价是具体的：判别器吞掉还是重抛的决定隐式依赖两份拷贝保持完全一致。未来对挂起条目标识的修改（例如 PersistedPendingMessage 增加存储键，或 messageKey 的输入变化）必须在三处一致应用；只改这个跳过而不改判别器会悄悄重新引入每轮询双倍重试预算消耗（即 R1-1 回归），反过来则会吞掉未挂起的文档通知失败、让水位越过它并使其剩余预算搁浅——重新引入原 R2-2 修复消除的饥饿——而且编辑当时不会有任何编译器或测试信号，因为今天两份拷贝是相互独立的字面量。请抽取一个与 hasPendingDocumentNotification 对应的辅助方法（见上方英文代码块），并在全部三处调用。该抽取保持行为不变，因此没有可适用的测试见证；两个调用点所保护的行为仍由 spends one retry per poll on a failed direct message also in history 与 keeps spending the retry budget of an unparked document notification 固定。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if ( | ||
| !(this.cursor.pendingMessages ?? []).some( | ||
| (pending) => messageKey(pending.message) === key, | ||
| ) | ||
| ) { | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-2: (fix-induced) Second copy of the parked-message predicate named in the comment above — this rethrow discriminator inlines (this.cursor.pendingMessages ?? []).some((pending) => messageKey(pending.message) === key) again, character-identical to the pre-dispatch skip. The swallow-vs-rethrow decision here is load-bearing for two opposite guarantees (parked ⇒ keep the page moving; unparked ⇒ pin the watermark), and it silently depends on this copy staying identical to the skip's. Same fix as above: extract hasPendingMessage(key) mirroring hasPendingDocumentNotification (~line 1923) and call it from both loop sites and, optionally, rememberPendingMessage. Behavior-preserving extraction; the protecting behaviours stay pinned by spends one retry per poll on a failed direct message also in history and keeps spending the retry budget of an unparked document notification.
中文说明
（fix-induced）上方评论所指挂起消息判定的第二份拷贝——这个重抛判别器再次内联了 (this.cursor.pendingMessages ?? []).some((pending) => messageKey(pending.message) === key)，与分发前跳过逐字符相同。此处吞掉/重抛的决定对两个相反的保证都至关重要（已挂起 ⇒ 页面继续前进；未挂起 ⇒ 钉住水位），并且它隐式依赖这份拷贝与跳过处保持完全一致。修复同上：抽取与 hasPendingDocumentNotification（约第 1923 行）对应的 hasPendingMessage(key)，在循环两处（以及可选地在 rememberPendingMessage）调用。该抽取保持行为不变；受保护的行为仍由 spends one retry per poll on a failed direct message also in history 与 keeps spending the retry budget of an unparked document notification 固定。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (source.kind !== 'direct') { | ||
| this.markProcessedMessage(messageKey(message)); | ||
| this.saveCursor(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-2: The rewritten stale-replay condition's drop side — non-direct sources are still marked processed and dropped here — has no test. Every stale-replay fixture in dws-channel.test.ts (the Date.now() - 60_000 / Date.now() - 100_000 eventTime overrides) is emitted via client.emit(1, ...) — the direct stream — so no input in the suite reaches this branch with source.kind !== 'direct' and a stale eventTime. The new (direct) side is pinned by lets polling recover a stale replayed direct message; this preserved side is pinned by nothing. A mutation that broadens the rescue — e.g. parking stale replays of any source and pulling the watermark back — ships green: a stale at/group replay sent >5s before the channel connected would be parked, the DM watermark pulled back, and the message dispatched as a fresh agent turn long after it was sent — exactly the staleness drop this branch exists to prevent. Witness (probe in a scratch tree):
BASE(PR code): 161 passed
MUTANT(if (false)): 161 passed — the mutant survives
MUTANT + probe test: 1 failed — received [{ messageId: 'stale-at-replay', text: '@Qwen stale mention' }]
PR code + probe test: 162 passed
Suggested fix: add a sibling test next to the new stale-replay test, e.g. still drops a stale replayed non-direct message — build a user_im_message_receive_at message with eventTime: Date.now() - 60_000, await client.emit(0, replay), then assert channel.inbound is empty immediately and still empty after await channel.poll() (optionally also that the watermark was not pulled back). That test is also the fix witness: removing or broadening this drop branch makes the replay get parked and dispatched, turning it red.
中文说明
重写后的过期重放条件的丢弃侧——非私聊来源仍在这里被标记为已处理并丢弃——没有测试。dws-channel.test.ts 中所有过期重放夹具（Date.now() - 60_000 / Date.now() - 100_000 的 eventTime 覆盖）都通过 client.emit(1, ...)——即私聊流——发出，因此套件中没有任何输入能以 source.kind !== 'direct' 且过期的 eventTime 到达这个分支。新增的（私聊）一侧由 lets polling recover a stale replayed direct message 固定；这个被保留的丢弃侧没有任何固定。放宽救援的变异——例如把任意来源的过期重放都挂起并回拉水位——会在测试全绿的情况下合入：一条在频道连接前 5 秒以上发出的过期 at/群聊重放会被挂起、私聊水位被回拉，消息会在发出很久之后被当作新的 Agent 轮次分发——正是本分支存在所要防止的过期分发。见证（临时树探针）：基线 161 通过；变异（if (false)）161 通过——变异存活；变异 + 探针测试 1 失败——收到 [{ messageId: 'stale-at-replay', text: '@qwen stale mention' }]；PR 代码 + 探针测试 162 通过。建议修复：在新的过期重放测试旁新增一个姊妹测试，例如 still drops a stale replayed non-direct message——构造 eventTime: Date.now() - 60_000 的 user_im_message_receive_at 消息，await client.emit(0, replay)，随后断言 channel.inbound 立即为空、且在 await channel.poll() 之后仍为空（可选地断言水位未被回拉）。该测试同时是修复见证：移除或放宽这个丢弃分支会使重放被挂起并分发，使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #10274 (review round 4)All four inline findings are resolved in code in one batched round, and the Feedback points and decisions[rc:3875171021] [Critical] R3-1 — in-flight double retry-budget spend — FIXEDReproduced first, per policy: a new race-variant test holds the live turn in Fix in One deliberate deviation from the suggested fix, with evidence: the gate Witness tests (both committed):
[rc:3875171036] + [rc:3875171045] [Suggestion] R2-2 — duplicated parked-message predicate — FIXEDExtracted [rc:3875171055] [Suggestion] R3-2 — stale-replay drop side for non-direct sources untested — FIXEDAdded [rv:5044870251] [CHANGES_REQUESTED] — convergence observation — ADDRESSED (no code change)The observation asks for the remaining fixes to be batched and verified before Mutation probes (witness verification)
Verification
中文说明Autofix 轮次总结 — PR #10274（评审第 4 轮）四条行内发现已全部在同一批次轮次中于代码层面解决，评审正文中的收敛观察也通过该批处理得到回应。提交： 反馈点与决定[rc:3875171021] [Critical] R3-1 — 在途双倍重试预算消耗 — 已修复按策略先复现：新增的竞态变体测试用可控的延迟
相对建议修复的一处有意偏离，附证据：门控复查重抛捕获的在途错误，而不是直接返回。 见证测试（均已提交）：
[rc:3875171036] + [rc:3875171045] [Suggestion] R2-2 — 重复的挂起消息判定 — 已修复抽取 [rc:3875171055] [Suggestion] R3-2 — 非私聊来源的过期重放丢弃侧无测试 — 已修复在私聊侧测试旁新增 [rv:5044870251] [CHANGES_REQUESTED] — 收敛观察 — 已处理（无代码改动）该观察要求把剩余修复攒成一批、验证后再推送，或先定位共同根因。本轮把四条行内发现作为一批实现，并在提交前运行了完整验证。此前发现反复收敛于 变异探针（见证验证）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/channels/dws/src/dws-channel.ts:1502 — [probe] processed stale direct redelivery still pulls the watermark back, drops the checkpoint, and logs a parking that did not happen (deferred by the code-age rule: unchanged since the previ…
中文说明
已审查。 建议见行内评论。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| await client.emit(0, replay); | ||
| expect(channel.inbound).toEqual([]); | ||
|
|
||
| await channel.poll(); | ||
| expect(channel.inbound).toEqual([]); |
There was a problem hiding this comment.
[Suggestion] R3-3: (fix-induced) The round-3 autofix closed R3-3 by adding still drops a stale replayed non-direct message, but the test's poll-half is vacuous: the replay's eventTime (connect − 60s) lies outside every history window this test ever creates, so await channel.poll() never re-fetches the replay and the second expect(channel.inbound).toEqual([]) passes regardless of the mark it was meant to pin. Deleting this.markProcessedMessage(messageKey(message)) from the non-direct stale branch (dws-channel.ts:1478) keeps this witness test and the entire 164-test suite green — mutation-verified in a scratch tree at this commit. The mark matters in production: a restart restores the persisted mentionWatermark and the mention-history overlap window re-opens over the downtime gap — which can lie entirely before the new connectionStartedAt − 5s drop boundary — so an unmarked stale at replay inside that window is re-driven as a fresh agent turn long after it was sent: exactly the staleness re-drive the mark exists to prevent. Witness (restart-shaped probe in a scratch tree): under the mutant the replay is re-driven — inbound [{ messageId: 'stale-at-replay', text: '@Qwen stale mention', isGroup: true }] vs expected [] — while the same probe passes under the unmodified PR code. Note the DM-history angle is not a production witness: the real listDirectMessages returns only singleChat conversations, so a group replay can never appear in DM history — the re-fetch path is the mention window after restart, and the fake client lacks the singleChat filter. Please replace the poll-half with a restart-shaped witness: first connection advances mentionWatermark via a seed mention, disconnect, let time elapse (fake timers), second connection restores the persisted watermark, emit a stale at replay whose eventTime falls inside the restored overlap window, poll mention history, and assert channel.inbound stays empty.
中文说明
第 3 轮 autofix 通过新增 still drops a stale replayed non-direct message 关闭了 R3-3，但该测试的轮询半区是空转的：重放的 eventTime（连接前 60 秒）位于本测试创建的每一个历史窗口之外，因此 await channel.poll() 永远不会重新获取到这条重放，第二个 expect(channel.inbound).toEqual([]) 无论它本应固定的标记是否存在都会通过。在非私聊过期分支（dws-channel.ts:1478）删除 this.markProcessedMessage(messageKey(message)) 后，该见证测试与全部 164 个测试仍保持绿色——已在临时树上用本提交的变异验证。该标记在生产中有实际作用：重启会恢复持久化的 mentionWatermark，提及历史的重叠窗口会重新覆盖停机空档——该空档可能完全位于新的 connectionStartedAt − 5s 丢弃边界之前——于是窗口内一条未标记的过期 at 重放会在发出很久之后被重新驱动为一次新的 Agent 轮次：正是该标记所要防止的过期重驱动。见证（临时树上的重启形态探针）：变异下重放被重新驱动——inbound [{ messageId: 'stale-at-replay', text: '@Qwen stale mention', isGroup: true }]，期望为 []——同一探针在未修改的 PR 代码上通过。注意私聊历史角度不是生产形态的见证：真实 listDirectMessages 只返回 singleChat 会话，群聊重放不可能出现在私聊历史中——重新获取路径是重启后的提及窗口，而测试中的假客户端缺少 singleChat 过滤。请把轮询半区替换为重启形态的见证：第一次连接通过一条种子提及推进 mentionWatermark，断开，让时间流逝（假定时器），第二次连接恢复持久化水位，发出一条 eventTime 落在恢复后重叠窗口内的过期 at 重放，轮询提及历史，并断言 channel.inbound 保持为空。修复见证：加强后的测试本身——从非私聊过期分支移除 markProcessedMessage 必须使其变红；请确认该变异使测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #10274 (round 5)Feedback points and dispositions[rc:3876341832] R3-3 (Suggestion): the round-3 witness test's poll-half is vacuous — ResolvedClaim: Reproduced before changing anything (source-blind verification):
Fix: replaced the poll-half with the restart-shaped witness the finding prescribes (test-only change, no production code touched):
Fix witness (mutation probe B): with the same mutation re-applied, the strengthened test fails with exactly the production failure shape — the replay is re-driven as a fresh turn ( Review body [rv:5046186146] deferred item — Not requested this round, left untouchedThe review body explicitly records one deferred item under the convergence posture (round 4, not a blocker): Failed checks — No action needed
Changes
No conflicts ( VerificationAll commands actually run, in order (working tree clean and committed as
中文说明Autofix address-review 摘要 — PR #10274（第 5 轮）反馈点及处理[rc:3876341832] R3-3（建议）：第 3 轮见证测试的轮询半区是空转的 — 已解决论断： 修改前先复现（来源无关验证）：
修复： 将轮询半区替换为该发现所规定的重启形态见证（仅改测试，未动生产代码）：
修复见证（变异探针 B）： 重新施加同一变异后，加强后的测试失败，且正是生产故障形态——重放被重新驱动为新轮次（ 审查正文 [rv:5046186146] 中的延后项 — 本轮未要求，保持不动审查正文按收敛姿态（第 4 轮，非阻断）明确记录了一条延后项： 失败检查 — 无需处理
变更内容
无冲突（ 验证以下为实际执行的全部命令及结果（按顺序；工作区干净，已提交为
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 5": running dws-channel.test.ts to confirm green — the shared review worktree has no node_modules and a monorepo install exceeds the budget; all mutation verdic….
中文说明
未发现问题。LGTM！✅
未探索到全部深度（达到工具调用预算）："agent 5"：running dws-channel.test.ts to confirm green — the shared review worktree has no node_modules and a monorepo install exceeds the budget; all mutation verdic…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
Released in v0.22.3. |
4 participants
What this PR does
Direct DWS final responses now use ordinary direct-message delivery, while group responses keep replying to the originating message. Direct-message history now sends ordinary messages through the same policy, deduplication, and retry path as live events. A stale direct replay remains eligible for recovery and pulls the polling watermark back so history can process it exactly once.
Why it's needed
DWS can accept a quoted reply to a direct message without rendering it in the recipient client, which makes a successful agent turn appear silent. The direct event stream is also at most once, while the existing history fallback skipped ordinary direct messages and marked stale replays processed, so a brief delivery gap could permanently lose a request.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: adapter probes reproduced all three failures: direct final responses used the quoted-reply route, ordinary history-only direct messages were skipped, and stale direct replays were marked processed before recovery.
After: the same probes confirmed ordinary direct delivery with a stable UUID, one-time history dispatch, and one-time stale replay recovery. The complete DWS package passed 233/233 tests, and repository build and typecheck passed.
Tested on
Environment (optional)
Node.js 22 local workspace without a sandbox. The authenticated published-package channel was not replaced or restarted during verification.
Risk & Scope
Linked Issues
Fixes #10267
中文说明
本 PR 做了什么
DWS 私聊最终回复现在改为发送普通私聊消息，群聊回复仍然引用原始消息。私聊历史中的普通消息现在会进入与实时事件相同的策略校验、去重和重试链路。过期的私聊重放会保持可恢复状态，并拉回轮询水位，使历史轮询只处理一次。
为什么需要
DWS 可能接受私聊引用回复但不在接收方客户端中展示，导致一次成功的 Agent 执行看起来像没有回复。私聊事件流还是至多一次投递，而原有历史兜底会跳过普通私聊并把过期重放标记为已处理，因此短暂的投递空档可能永久丢失请求。
Reviewer Test Plan
如何验证
证据（修改前与修改后）
修改前：适配器探针复现了三个问题：私聊最终回复走引用回复路径、仅存在于历史中的普通私聊被跳过、过期私聊重放在恢复前被标记为已处理。
修改后：相同探针确认普通私聊使用稳定 UUID 发送、历史消息只分发一次、过期重放只恢复一次。DWS 包完整测试 233/233 通过，仓库构建和类型检查通过。
测试平台
环境（可选）
Node.js 22 本地工作区，未启用沙箱。验证期间没有替换或重启当前已认证、使用发布包运行的 channel。
风险与范围
关联 Issue
Fixes #10267