fix(core): report broadcast delivery failures in send_message - #10081
Conversation
send_message(to: "*") unconditionally returned "Message broadcast to all teammates." even when TeamManager.broadcast() had rejected deliveries: broadcast() collected the per-recipient failures but returned Promise<void>, discarding them. Make broadcast() return a BroadcastResult (attempted total + failed recipient names) derived from the failures it already computes, and let the send_message broadcast branch distinguish complete success, partial failure (naming the unreachable recipients), and total failure (returned as a tool error). Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. #10072 was filed from static inspection, but the rejection path is verifiably reachable in production: Direction: aligned — this fixes a misleading tool result in the agent-team messaging path, where the leader model can currently act on a false "everyone got it" signal. No direct CHANGELOG reference for broadcast failure reporting, but the area is relevant and actively maintained (recent upstream entries keep fixing agent-team delivery edge cases). Size: core paths touched ( Approach: scope feels right. Risk: no elevated risk signals — none of the changed files match the revert-correlated paths from the Stage 1e analysis. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：是已观测到的问题，不是理论性的。#10072 虽然源自静态代码检查，但 reject 路径在生产中真实可达： 方向：对齐——修复 agent 团队消息路径中的误导性工具结果：leader 模型目前可能基于"所有人都收到了"的假信号行动。CHANGELOG 没有广播失败上报的直接条目，但该领域相关且持续维护中（上游最近多个条目都在修 agent 团队投递的边界情况）。 规模：触及核心路径（ 方案：范围合理。 风险：无升级风险信号——改动文件均未命中 Stage 1e 的易回滚路径分析。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no blockers found. I formed my independent proposal before reading the diff (return the delivery outcome
Skipped the diagram/files-table enrichments — this is a small, focused change and they'd just be noise. CI test evidence — fetched via API for the reviewed commit (PR code not executed here, per triage policy):
The load-bearing check is Sandboxed verification would settle that: 中文说明代码审查 —— 未发现阻塞问题。 我在看 diff 之前先形成了自己的独立方案（把
省略了时序图/文件表——改动小而聚焦，加上去只是噪音。 CI 测试证据 —— 通过 API 获取（按分诊策略不执行 PR 代码）：核心套件 沙盒验证可以补上这一环： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix verified against the base code; the only unverified bits are the A/B red baseline (author's own run) and non-Linux unit runs (repo-gated), neither of which changes the call. Stepping back: this is exactly the kind of PR the gate should wave through. The problem is real and verified — The tests are the right kind: fault injection through the real Approving. ✅ 中文说明信心：4/5 —— 干净、最小的修复，已对照基线代码核实；唯一未验证的是由红转绿的 A/B 基线（作者本人运行）和非 Linux 的单测运行（仓库门控），两者都不影响结论。 退一步看：这正是分诊闸门应该放行的 PR。问题真实且已核实—— 测试也是对的：通过真实 通过。✅ — 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. ✅
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): src/tools/send-message.broadcast.test.ts — no such file or directory; src/tools/send-message.test.ts — no such file or directory; src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; Tests 112 passed — this review observed 21645, 1702, 24761, 1659, 601, 4235, 630 passed.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：src/tools/send-message.broadcast.test.ts — no such file or directory; src/tools/send-message.test.ts — no such file or directory; src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; Tests 112 passed — this review observed 21645, 1702, 24761, 1659, 601, 4235, 630 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| debug.warn( | ||
| `Broadcast: ${failures.length}/${results.length} send(s) failed ` + | ||
| `Broadcast: ${failedRecipients.length}/${results.length} send(s) failed ` + | ||
| `(recipient likely terminated).`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The warning prints only the failure count, but the failed recipients' names are available at the log site — failedRecipients is in scope one line above. This line is the only operational trace outside the conversation when a broadcast fails: no team event carries delivery failures (TeamEventType has MESSAGE_SENT only, emitted on success paths), and failedRecipients surfaces nowhere else. So when reconstructing an incident — which teammate missed a shutdown approval or a status update — an oncall sees Broadcast: 1/3 send(s) failed (recipient likely terminated). with no name to attribute, and must correlate agent-exit events by timestamp. Include the names in the log line:
| debug.warn( | |
| `Broadcast: ${failures.length}/${results.length} send(s) failed ` + | |
| `Broadcast: ${failedRecipients.length}/${results.length} send(s) failed ` + | |
| `(recipient likely terminated).`, | |
| ); | |
| debug.warn( | |
| `Broadcast: ${failedRecipients.length}/${results.length} send(s) failed ` + | |
| `for: ${failedRecipients.join(', ')} (recipient likely terminated).`, | |
| ); |
中文说明
该警告只打印失败数量，但失败收件人的名字在打日志的位置就可以拿到——failedRecipients 就在上一行。这一行是广播失败时对话之外唯一的运维痕迹：团队事件不携带投递失败信息（TeamEventType 只有 MESSAGE_SENT，且只在成功路径触发），failedRecipients 也没有在其他地方暴露。因此当需要排查「哪个 teammate 错过了关闭审批或状态更新」这类问题时，值班同学只能看到 Broadcast: 1/3 send(s) failed (recipient likely terminated).，没有名字可以归属，只能靠时间戳去关联 agent 退出事件。建议把失败收件人的名字加进日志行（修复代码见上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| /** Names of recipients whose delivery was rejected. */ | ||
| failedRecipients: string[]; |
There was a problem hiding this comment.
[Suggestion] failedRecipients: string[] collapses the four distinct sendMessage rejection causes — terminated recipient, unknown recipient, leader-inbox write failure, and backpressure — into bare names, and the only semantic annotation of "rejected" in this module (the warn just above the return) asserts "(recipient likely terminated)", which is wrong for three of the four. Backpressure is transient: its direct-send error explicitly tells the caller to wait for the backlog to drain, and the queue flushes the moment the recipient goes IDLE. Concrete trigger: teammate bob is ACTIVE with MAX_PENDING_MESSAGES (50) pending; the leader runs send_message(to: "*"); bob's delivery rejects — the tool then reports delivery failed for: bob. The listed recipients did not receive the message., byte-identical to bob having terminated. The leader model gets no hint that waiting and retrying would succeed, and can act as though bob is gone — reassigning bob's tasks or spawning a duplicate teammate — dropping a message that would have landed on bob's next idle flush. Consider carrying the rejection reason alongside the name (built from the PromiseRejectedResult.reason values in the allSettled results), surfacing the reason in the partial/total-failure text, and fixing or dropping the "(recipient likely terminated)" clause — or minimally, keep the shape but stop asserting a cause the data doesn't carry.
中文说明
failedRecipients: string[] 把 sendMessage 的四种不同 reject 原因——收件人已终止、收件人不存在、leader 收件箱写入失败、背压——压缩成了纯名字，而模块中唯一对「rejected」的语义标注（return 上方那行 warn）断言 "(recipient likely terminated)"，对其中三种原因来说都是错的。背压是瞬时的：直接发送的错误信息明确提示调用方等待积压消化，且收件人一旦进入 IDLE 队列就会被清空。具体场景：teammate bob 处于 ACTIVE 且积压了 MAX_PENDING_MESSAGES（50）条消息，leader 执行 send_message(to: "*")，bob 的投递被 reject——工具报告 delivery failed for: bob. The listed recipients did not receive the message.，与 bob 已终止时的输出逐字节相同。leader 模型得不到「稍等重试即可成功」的提示，可能表现得像 bob 已经消失一样——重新分配 bob 的任务或复制一个新 teammate——从而丢掉一条本会在 bob 下次 idle flush 时送达的消息。建议把 reject 原因随名字一起携带（从 allSettled 结果的 PromiseRejectedResult.reason 取值），在部分/全部失败文案中呈现原因，并修正或去掉 "(recipient likely terminated)" 从句——最小的改法是保持类型不变，但不再断言数据中不存在的原因。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving. Independent review of the current head (24d7fc55) found no blocking (Critical) issues:
TeamManager.broadcast()now returns aBroadcastResult(total/failedRecipients) instead ofPromise<void>. The only production caller issend-message.ts(this PR); the return-value addition is backward-compatible and the never-reject (Promise.allSettled) semantics are unchanged.failedRecipientsindex-alignment is correct (allSettledpreserves recipient order; theresults[i]?.statusguard is safe).- Complete/partial/total-failure branching is sound; total failure correctly surfaces as a tool error listing every unreachable recipient, fixing the misleading "broadcast to all" result from #10072.
- No permission/security surface change; behavior is reporting-only.
CI is green and the bot has approved; concur.
Independent A/B verificationVerified the current PR head I did not use Method
# base
git worktree add --detach <tmp>/repo d4664fdc89ff3e77d4d4bdb82e6ec187fdd20acf
cd <tmp>/repo
npm ci
cd packages/core
npx vitest run src/tools/send-message.broadcast.test.ts
# head
git checkout --detach 24d7fc55d41d74afd70a8131aaa454499c408cfe
cd packages/core
npm run build
npx vitest run src/tools/send-message.broadcast.test.tsBefore / after
Base result: Head result: Environment: macOS 15.1.1 arm64, Node.js 22.22.0, npm 10.9.4, Vitest 3.2.7. This closes the red-to-green A/B gap: the misleading partial/total-failure behavior is reproducible at the base commit and fixed at the PR head, while the complete-success behavior is preserved. I did not run a full interactive CLI E2E; the current head's green CI provides the broader module/build coverage. |
3 participants
What this PR does
The
send_message(to: "*")broadcast path no longer claims complete success when one or more deliveries were rejected.TeamManager.broadcast()now returns the delivery outcome it already computed — the number of recipients attempted and the names of the recipients whose delivery was rejected — instead ofPromise<void>. The broadcast branch ofsend_messageuses it to distinguish three outcomes:Message broadcast to all teammates.Why it's needed
Fixes the misleading tool result reported in #10072:
TeamManager.broadcast()waits withPromise.allSettled()and logs the rejected deliveries, but always resolves normally, and the tool then unconditionally returnedMessage broadcast to all teammates.The leader model could therefore believe every teammate received a message that never reached a terminated/rejected recipient (e.g. a recipient whose queue was dropped between the member snapshot and the send), with no way to retry or escalate.Reviewer Test Plan
How to verify
Reproduced before the fix with an invocation-level fault-injection test using the real
TeamManager(viaTeamCoordinationHarness): create a two-teammate team, terminate one teammate so its message queue is dropped, runsend_message(to: "*"), and observeMessage broadcast to all teammates.Before the fix the two fault-injection tests are red (the tool claims complete success;
erroris undefined for total failure). After the fix all 112 tests across the three files are green, including regression tests that complete success still returns the original message byte-for-byte, that existing delivery semantics (sender skip, leader inbox delivery) are unchanged, and that thedebug.warnlog line is preserved.Evidence (Before & After)
N/A for UI — this is an agent-visible tool result, not TUI output. Test evidence:
Before (red repro):
After:
Tested on
Environment (optional)
Unit/integration tests only (vitest) — no runtime environment needed.
Risk & Scope
TeamManager.broadcast()'s return type changes fromPromise<void>toPromise<BroadcastResult>; its only production caller issend_message(verified by grep).TeamManager.TeamManager.broadcast()return type changed.Linked Issues
Fixes #10072
中文说明
这个 PR 做了什么
send_message(to: "*")广播路径在部分投递被 reject 时不再声称全部成功。TeamManager.broadcast()不再返回Promise<void>，而是返回它本来就计算好的投递结果：尝试投递的收件人数量和投递被 reject 的收件人名单。send_message的广播分支据此区分三种结果：Message broadcast to all teammates.不变。为什么需要
修复 #10072 报告的误导性工具结果：
TeamManager.broadcast()用Promise.allSettled()等待并记录被 reject 的投递，但总是正常 resolve，工具随后无条件返回Message broadcast to all teammates.。这会导致 leader 模型以为所有 teammate 都收到了消息，而实际上消息并未送达已终止/被 reject 的收件人（例如成员快照与发送之间队列被清理的收件人），且无法重试或升级处理。审阅者测试计划
如何验证
修复前已用真实
TeamManager（通过TeamCoordinationHarness）的调用级故障注入测试复现：创建两个 teammate，终止其中一个使其消息队列被清理，运行send_message(to: "*")，观察到结果仍为Message broadcast to all teammates.。运行上面「How to verify」中的命令：修复前两个故障注入测试为红（工具声称全部成功；全部失败时
error为 undefined）；修复后三个文件共 112 个测试全绿，包括「完整成功仍返回原文案」的回归测试、既有投递语义（跳过发送者、leader 收件箱投递）不变的验证，以及debug.warn日志保持原样的确认。前后证据
UI 层面 N/A —— 这是 agent 可见的工具结果，不是 TUI 输出。测试证据见上方英文版。
测试环境
macOS 未测试，Windows 未测试，Linux 已测试。
环境（可选）
仅单元/集成测试（vitest），无需运行时环境。
风险与范围
TeamManager.broadcast()返回类型从Promise<void>变为Promise<BroadcastResult>，唯一的生产调用方是send_message（已用 grep 确认）。TeamManager的 harness 集成测试覆盖。TeamManager.broadcast()返回类型变化。关联 Issue
Fixes #10072