fix(team): align teammate reporting prompts - #9287
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献!
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal for this problem was: rewrite the three stale instruction sites (normal addendum, plan-required addendum, and the Verified against the runtime, not just the description:
Tests pin all four gaps from the issue's red reproduction (automatic-delivery wording for normal and plan-required prompts, removal of the ONLY-path claim, removal of the peer-DM promise), and the new No critical findings. CI test evidenceUnattended run — no PR code was built or executed here; this quotes the PR's own CI on the reviewed commit. The main unit suite ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The change is prompt and description text only — the runtime forwarding and plan-approval paths are unchanged, and the tests pin the exact wording, so the CI suite plus static verification settle the claim; no sandboxed lane is needed. The author's "Tested on" table (macOS only) is their claim for local typecheck/vitest, not evidence this run relied on. 中文说明代码审查在看 diff 之前,我的独立方案是:改写三处过时指令(普通队友附录、计划必需队友附录、 以下是对照运行时核实的结果,而非只看描述:
测试钉住了 issue 红色复现中的全部四个缺口(普通与计划必需 prompt 的自动送达措辞、移除"唯一路径"说法、移除点对点摘要承诺),新增的 无严重问题。 CI 测试证据无人值守运行——此处未构建或执行任何 PR 代码,以下引用的是 PR 自身 CI 在被审 commit 上的结果。主单元测试( 本次变更仅涉及 prompt 与描述文本——运行时转发和计划批准路径未变,测试已钉住确切措辞,因此 CI 套件加静态核实足以支撑结论,无需沙箱验证通道。作者的"测试环境"表(仅 macOS)是其本地 typecheck/vitest 的自述,不是本次审查依赖的证据。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — every claim checked against the runtime itself, the scope is exactly the minimal fix, and the tests pin all four gaps from the issue's reproduction. Stepping back: the PR matches my independent proposal site for site — three stale instruction sites rewritten, one unsupported promise removed, and notably it resists the tempting alternative of actually building the peer-DM summary feature the old description promised. That restraint is the right call; the issue asks for accurate instructions, not new observability. The wording is verifiably true against the IDLE forwarding path and the plan-approval routing, the MARK COMPLETE reordering is justified by the turn-ending semantics of the final answer, and the test additions exercise the real spawn path rather than string constants in isolation. Nothing in the diff is unrelated, and I didn't find a simpler version of this change. If I'm maintaining this in six months, the prompts finally say what the code does — that's the whole point. Verdict: approve. The main unit suite is still running on the reviewed commit, though, so approval is deferred until CI lands green on 中文说明置信度:5/5 —— 每一项说法都对照运行时核实过,范围恰好是最小修复,测试钉住了 issue 复现中的全部四个缺口。 退一步看:PR 与我的独立方案逐处吻合——改写三处过时指令、删除一条不受支持的承诺,尤其值得肯定的是它抵制了一个诱人的替代方案:真去做旧描述所承诺的点对点消息摘要功能。这种克制是正确的;issue 要的是准确的指令,而不是新的可观测性。措辞对照 IDLE 转发路径和计划批准路由均可证实为真,MARK COMPLETE 的顺序调整由"最终答案结束本轮"的语义所支撑,新增测试走的是真实的 spawn 路径,而非孤立的字符串常量。diff 中没有无关改动,我也没找到比这更简的版本。六个月后维护这段代码时,prompt 终于与代码行为一致——这正是重点。 结论:批准。但主单元测试仍在被审 commit 上运行,因此批准将延迟到 CI 在该 commit 上全绿后执行——收尾步骤会在全部转绿后将批准钉在该 commit 上;若有红色结果则不予批准。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| it('describes automatic final delivery for ordinary teammates', () => { | ||
| const prompt = buildTeammatePromptAddendum('worker', 'team', 'leader'); |
There was a problem hiding this comment.
[Suggestion] The rewritten test dropped the pre-existing guard expect(prompt).not.toContain('call exit_plan_mode'). The current default addendum contains no exit_plan_mode, so the assertion would still have passed, and nothing else in the file re-asserts it — this removes the only check that plan-mode tooling instructions never leak into ordinary teammates' prompts. — Failure scenario: a future edit merging or copy-pasting the planModeRequired branch wording into the default branch would instruct ordinary teammates (spawned without plan mode) to call exit_plan_mode, wasting turns on a workflow they are not in — and no test would fail.
Restore the dropped assertion alongside the new ones:
expect(prompt).not.toContain('call exit_plan_mode');中文说明
重写后的测试删除了原有的守卫断言 expect(prompt).not.toContain('call exit_plan_mode')。当前默认变体附录中并不包含 exit_plan_mode,该断言本可以继续通过,且文件中没有其他断言替代它——这移除了"计划模式工具说明不会泄漏进普通队友提示词"的唯一检查。—— 失败场景:未来若有编辑将 planModeRequired 分支的措辞合并或复制到默认分支,普通队友(未以计划模式启动)会被指示调用 exit_plan_mode,在其并不处于的工作流中浪费回合——而没有任何测试会失败。
请在新断言旁恢复被删除的断言(见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| '2. DO THE WORK: Use read_file, grep_search, glob, etc.', | ||
| ' Use send_message for blockers, questions, and material interim findings.', |
There was a problem hiding this comment.
[Suggestion] The teammate reporting contract is now copy-pasted in five source sites with no shared source of truth: the interim-messaging sentence appears byte-identical at promptAddendum.ts:76 and :99, the REPORT RESULTS sentence at :47-49 (readOnly branch, wrapped over three lines), :80 and :103, and TeamManager.ts:453 and :455-460 restate the same contract in two further paraphrases. — Failure scenario: this PR exists because the contract drifted from runtime behavior and every copy had to be located and hand-edited — and the readOnly branch was already aligned before this PR, proving one branch can be updated while the others drift. The copies already diverge in wording inside this PR; the next contract change must find all five sites again, and a missed one silently re-creates the exact misalignment this PR fixes.
Extract the shared sentences into module-level constants and interpolate them in all three branches (optionally exporting them for TeamManager's initialTask strings):
const SEND_MESSAGE_USAGE =
'Use send_message for blockers, questions, and material interim findings.';
const REPORT_RESULTS =
'Return a concise final answer with findings, evidence, uncertainty, and recommended next action. The runtime forwards it to the leader automatically. This ends your turn.';中文说明
队友汇报契约目前被复制粘贴在五处源码中,没有共享的唯一来源:临时消息句在 promptAddendum.ts:76 与 :99 逐字节相同;REPORT RESULTS 句出现在 :47-49(只读分支,折成三行)、:80 与 :103;TeamManager.ts:453 与 :455-460 又以两种改述形式重述了同一契约。—— 失败场景:本 PR 的存在正是因为契约与运行时行为脱节、需要逐处定位并手工修改——而只读分支在本 PR 之前就已对齐,证明完全可能出现"一个分支更新了、其余分支仍停留在旧契约"的情况。这些副本在本 PR 内部措辞已经不一致;下次契约变更必须再次找全五处,漏掉任何一处都会悄悄重新制造本 PR 正在修复的脱节。
建议将共享句提取为模块级常量,并在三个分支中插值引用(可同时导出供 TeamManager 的 initialTask 字符串复用,见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| '3. MARK COMPLETE: Call task_update(taskId, status: "completed").', | ||
| '', | ||
| '4. MARK COMPLETE: Call task_update(taskId, status: "completed").', | ||
| '4. REPORT RESULTS: Return a concise final answer with findings, evidence, uncertainty, and recommended next action. The runtime forwards it to the leader automatically. This ends your turn.', |
There was a problem hiding this comment.
[Suggestion] The alignment stops one surface short: packages/core/src/tools/send-message.ts:304 — the send_message tool description teammates read in the same context — still literally reads 'Your text output is NOT visible to other agents — use this tool to communicate.' This PR deletes exactly that claim from the addendum and replaces it with the automatic-forwarding promise, leaving the teammate instruction set self-contradictory. — Failure scenario: a teammate resolving the contradiction conservatively sends its report via send_message(to: "leader") AND emits the closing final-answer text; the explicit send sets explicitLeaderReports (TeamManager.ts:574), but the subsequent round text clears that flag and sets pendingFinalReports (onRoundText, TeamManager.ts:1553-1556), so at IDLE the runtime also forwards the final text — the leader receives the report twice, the double-delivery class this PR exists to eliminate.
Align the tool description in the same PR — drop the blanket "NOT visible" sentence or note that a teammate's final answer is forwarded to the leader automatically — or scope it to deliberate interim messages.
中文说明
本次对齐少覆盖了一个表面:packages/core/src/tools/send-message.ts:304 —— 队友在同一上下文中能读到的 send_message 工具描述 —— 仍然原文写着 'Your text output is NOT visible to other agents — use this tool to communicate.'。本 PR 恰好从附录中删除了这一说法并替换为自动转发承诺,使队友的指令集自相矛盾。—— 失败场景:队友若保守地处理这一矛盾,会既通过 send_message(to: "leader") 发送报告、又输出收尾的最终答案文本;显式发送会设置 explicitLeaderReports(TeamManager.ts:574),但随后的回合文本会清除该标记并设置 pendingFinalReports(onRoundText,TeamManager.ts:1553-1556),因此在 IDLE 时运行时仍会转发最终文本——负责人将收到两份报告,正是本 PR 要消除的重复送达。
建议在同一 PR 中对齐工具描述:删去"NOT visible"这一绝对化表述,或注明队友的最终答案会自动转发给负责人;也可将其限定为"用于有意的临时消息"。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| config.prompt ?? | ||
| (config.planModeRequired | ||
| ? 'You have joined the team in plan mode. Call task_list now to find pending tasks. Claim one with task_update(status: "in_progress"), investigate read-only, then call exit_plan_mode to submit your plan for leader approval before executing.' | ||
| ? 'You have joined the team in plan mode. Call task_list now to find pending tasks. Claim one with task_update(status: "in_progress"), investigate read-only, then call exit_plan_mode to submit your plan for leader approval before executing. Use send_message only for blockers, questions, and material interim findings; your concise final answer is forwarded to the leader automatically.' |
There was a problem hiding this comment.
[Suggestion] The diff changes both arms of the initialTask ternary, but the new test in TeamManager.plan-approval.test.ts pins only the normal arm (a spawn without planModeRequired); the plan-mode arm's new clause is asserted nowhere — the existing plan-mode test checks only toContain('exit_plan_mode'), which passes with or without the clause. — Failure scenario: a future edit dropping or mangling the plan-mode arm's new clause ships silently with CI green, reintroducing the old contract mismatch for plan-required teammates on the default-prompt path.
Witness (mutation probe): reverting the whole TeamManager.ts hunk makes the new normal-arm test fail (expected '…report via send_message(to: "leader")…' to contain 'material interim findings'), while deleting only the plan-arm clause keeps all 29 tests green — the normal arm is pinned, the plan arm is not.
Mirror the new test for a planModeRequired: true spawn:
expect(initialTask).toContain('material interim findings');
expect(initialTask).toContain('forwarded to the leader automatically');中文说明
本 diff 修改了 initialTask 三元表达式的两个分支,但 TeamManager.plan-approval.test.ts 中新增的测试只钉住了普通分支(未带 planModeRequired 的 spawn);计划模式分支新增的子句没有任何断言覆盖——现有的计划模式测试只检查 toContain('exit_plan_mode'),无论该子句是否存在都会通过。—— 失败场景:未来若有编辑删除或改坏计划模式分支的新子句,将在 CI 全绿的情况下悄然合入,使需要计划批准的队友在默认提示词路径上重新出现旧契约的不一致。
证据(变异探测):回退整个 TeamManager.ts hunk 会使新增的普通分支测试失败(expected '…report via send_message(to: "leader")…' to contain 'material interim findings'),而只删除计划模式分支的子句则 29 个测试全部通过——普通分支已被钉住,计划模式分支没有。
请为 planModeRequired: true 的 spawn 添加镜像测试(见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| '3. MARK COMPLETE: Call task_update(taskId, status: "completed").', | ||
| '', | ||
| '4. MARK COMPLETE: Call task_update(taskId, status: "completed").', | ||
| '4. REPORT RESULTS: Return a concise final answer with findings, evidence, uncertainty, and recommended next action. The runtime forwards it to the leader automatically. This ends your turn.', |
There was a problem hiding this comment.
[Suggestion] The diff reorders this branch so MARK COMPLETE precedes the turn-ending REPORT RESULTS (and inserts REPORT RESULTS before REPEAT in the plan branch), but no assertion pins that ordering for the normal or plan branches — only the readOnly branch has an ordering test ('marks read-only tasks complete before the turn-ending report'). — Failure scenario: REPORT RESULTS says "This ends your turn", so the order is load-bearing: if a later edit swaps the two blocks back, a teammate following the prompt ends its turn before calling task_update(status: "completed"), leaving tasks stuck in_progress while the leader receives the report. Mutation probe: swapping the two array entries here keeps all five promptAddendum.test.ts tests green, while the same swap in the readOnly branch fails its ordering test — the comparator is live, the pinning is genuinely absent.
Add the parallel ordering test for the default branch:
it('marks tasks complete before the turn-ending report', () => {
const prompt = buildTeammatePromptAddendum('worker', 'team', 'leader');
expect(prompt.indexOf('MARK COMPLETE')).toBeLessThan(
prompt.indexOf('REPORT RESULTS'),
);
});中文说明
本 diff 将该分支重排为 MARK COMPLETE 先于结束回合的 REPORT RESULTS(并在计划分支中将 REPORT RESULTS 插入 REPEAT 之前),但普通分支与计划分支的顺序都没有任何断言钉住——只有只读分支有顺序测试('marks read-only tasks complete before the turn-ending report')。—— 失败场景:REPORT RESULTS 写明 "This ends your turn",因此顺序是承重的:若未来有编辑把这两个块换回去,遵循提示词的队友会在调用 task_update(status: "completed") 之前结束回合,任务会卡在 in_progress,而负责人已经收到了汇报。变异探测:在此处交换两个数组项,promptAddendum.test.ts 的 5 个测试全部保持通过;而在只读分支做同样的交换会使其顺序测试失败——比较器有效,说明确实缺少钉住。
请为默认分支添加平行的顺序测试(见上方代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
What this PR does
Aligns teammate instructions with the runtime’s automatic forwarding of a concise final answer to the team leader. Explicit messaging remains for blockers, questions, and material interim findings. It also removes an unsupported promise that peer direct messages appear in leader idle notifications.
Why it's needed
The previous instructions described an explicit message to the leader as the only reporting path, even though the runtime already forwards a teammate’s normal final answer automatically. This contradiction encouraged redundant or misleading reporting behavior, while the peer-message observability promise described behavior the runtime does not provide.
Reviewer Test Plan
How to verify
send_messageis reserved for blockers, questions, and material interim findings.exit_plan_mode, thatsend_messageis not presented as a plan-approval route, and that automatic final forwarding is described after approval.Evidence (Before & After)
Before: teammate prompts said explicit
send_message(to: "leader")was the only way for a leader to receive results and team creation promised unavailable peer-DM summaries. After: prompts match automatic final-result forwarding and the unsupported observability claim is removed.Tested on
Environment (optional)
macOS local checkout;
npm run typecheckand focused Vitest coverage.Risk & Scope
Linked Issues
Closes #9283
中文说明
此 PR 的内容
使队友指令与运行时将简洁最终答案自动转发给团队负责人这一行为保持一致。显式消息仍用于阻塞项、问题和重要的中间发现。该 PR 还移除了一个不受运行时支持的承诺:点对点消息会出现在负责人的空闲通知中。
为什么需要它
之前的指令将向负责人显式发送消息描述为唯一的汇报路径,但运行时实际上已经会自动转发队友正常结束时的最终答案。这一矛盾会促使多余或误导性的汇报行为;同时,点对点消息可见性的承诺描述了运行时并未提供的行为。
审阅者测试计划
如何验证
send_message仅用于阻塞项、问题和重要的中间发现。exit_plan_mode,send_message不会被描述为计划批准路径,并且批准后会说明自动转发最终答案。证据(前后对比)
修改前:队友提示词称显式
send_message(to: "leader")是负责人接收结果的唯一方式,团队创建说明则承诺提供实际上不存在的点对点消息摘要。修改后:提示词与自动转发最终结果的行为一致,并移除了不受支持的可观测性声明。测试环境
环境(可选)
macOS 本地检出;执行了
npm run typecheck和聚焦的 Vitest 覆盖测试。风险与范围
关联问题
关闭 #9283