fix(core): dispatch manually assigned team tasks to their owner - #9289
Conversation
task_update persisted `status: in_progress, owner: <teammate>` and reported success, but nothing delivered the work: the auto-claim scan only consumes pending, UNOWNED tasks, and no direct dispatch path existed. The assigned task sat undelivered, and isAgentBusy then counted the owner as busy on the very task she never received, so she could not auto-claim anything else either. Deliver the assignment instead. The tool validates the owner before persisting (no such teammate, or shutdown pending → refuse, so a persisted assignment always has a delivery path), and after the write it enqueues the same nonce-wrapped task prompt the auto-claim path builds — now shared as buildTaskPrompt so both deliveries stay identical. The dispatch fires when the call changed the owner or moved the task into in_progress under its owner; self-claims are skipped, and re-asserting an unchanged owner+status dispatches nothing, so a retried task_update cannot double-deliver. Busy owners are fine: the agent runtime queues the prompt and processes it after the current turn. Tests: seven new harness cases drive the real leader TaskUpdateTool against a live TeamManager (delivery, owned-pending activation, reassignment, no re-dispatch, no self-claim prompt, ghost refusal, shutdown-pending refusal); each pin was mutation-verified red. The lifecycle E2E assigned to a teammate before spawning her — the exact dead end this removes — and now spawns first, blocking the other tasks so the status-filter assertions stay deterministic. Fixes #9282
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@yiliang114 The PR description is missing required sections of the PR template:
## Risk & Scope— main risk or tradeoff, what was not validated / out of scope, breaking changes## Linked Issues— theFixes #9282keyword currently sits in the opening paragraph; the template wants it under this heading (auto-close works from anywhere in the body, but the section itself is required)- The Chinese translation in the
<details>block
The substance of both sections is largely present in the prose — this is a template-shape gate, not a content complaint, and everything else (including the full Reviewer Test Plan) looks good. Once the missing sections are added, re-run @qwen-code /triage and the gate will proceed to the actual review.
One thing worth resolving while the description is being edited: PR #9288 targets this same issue (#9282) with a larger delivery design (deferred delivery to busy owners, unassignment requeue). The two can't both land as-is — a maintainer call on which direction wins would help the other author.
中文说明
@yiliang114 该 PR 描述缺少 PR 模板 中的必填章节:
## Risk & Scope—— 主要风险或权衡、未验证/超出范围的内容、破坏性变更说明## Linked Issues—— 目前Fixes #9282写在开头段落里,模板要求放在该章节下(自动关闭关联不受位置影响,但该章节本身是必填的)<details>块中的中文翻译
这两个章节的实质内容在正文中基本都有——这只是模板结构上的门禁,不是对内容的意见;其余部分(包括完整的 Reviewer Test Plan)都没有问题。补齐缺失章节后重跑 @qwen-code /triage,gate 会继续进入正式审查。
另外建议在补充描述时一并说明:PR #9288 针对的是同一个 issue(#9282),采用了更大的投递设计(向忙碌 owner 延迟投递、取消分配时重新入队)。两者无法同时按现状合入——需要 maintainer 定夺方向,也方便另一位作者调整。
— Qwen Code · qwen3.8-max
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 6b": none — I finished everything I planned. No Budget gap line..
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1471, 20884, 1571, 494, 3655, 504 passed; 20020 passed — this review observed 1471, 20884, 1571, 494, 3655, 504 passed.
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
Closeout from resolve-pr-comments: fixed manual task dispatch guard gaps by canonicalizing owner names before persistence, validating only assignments that can dispatch, rejecting terminal owners, surfacing failed dispatch as an error, and pinning shutdown/terminal/canonical/busy-owner plus lifecycle dispatch evidence. Verified Prettier and |
|
CI recovery from resolve-pr-comments: the exact-head Ubuntu job failed in the test step, but GitHub returned an empty job log, so I could not identify a PR-caused assertion. The branch was behind latest main, so I merged latest main with no conflicts and repaired the PR body template. Verified: merge conflict-free and git diff --check passed. Pending: post-push CI/review. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1471, 20903, 1571, 494, 3655, 504 passed; 20020 passed — this review observed 1471, 20903, 1571, 494, 3655, 504 passed.
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
Closeout for this pass: fixed the current manual-dispatch edge findings by rejecting owner names that sanitize to empty, refusing blocked-task dispatch before persistence, limiting owner validation to actual assignment delivery, preserving dependency mirroring before a dispatch-failure return, and canonicalizing task_list owner filters. Also fixed the busy-owner regression test setup. Verified: touched-file ESLint, Prettier, and git diff --check passed. Focused Vitest and package typecheck were blocked before test execution by the local dependency-resolution issue around ajv/dist/2020.js and related missing package resolutions in this temporary worktree. Intentionally not changed: helper extraction, full dispatch-failure test matrix, nonce-envelope test hardening, per-branch logging, edit re-notification, reassignment revocation, and queue-priority redesign. Pending: new CI and automatic review on 46e9a51. |
|
@qwen-code /review --timeout=360 |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
@qwen-code /review --timeout=360 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20099, 1471 passed; 20020 passed — this review observed 20099, 1471 passed.
中文说明
已审查。 建议见行内评论。
Test Plan(非阻断):src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20099, 1471 passed; 20020 passed — this review observed 20099, 1471 passed。
— qwen3.8-max via Qwen Code /review (v0.21.13)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20099, 1471 passed; 20020 passed — this review observed 20099, 1471 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/agents/team/TeamManager.ts:1823 — [review] manual dispatch never checks member.readOnly while both auto-claim paths skip readOnly members (deferred: code unchanged since the previous round)packages/core/src/tools/task-update.ts:534 — [review] dispatch-failure error ignores the concurrent unassignTeammateTasks self-heal; advised recovery can clobber a recovered task (deferred: message text unchanged since the previous round)
中文说明
Test Plan(非阻断):src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20099, 1471 passed; 20020 passed — this review observed 20099, 1471 passed。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R3-4 task-list owner-filter test lacks a negative control (filter-dropped mutant survives) — already reported (comment 3795481488)
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1471 passed; 20020 passed — this review observed 1471 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/task-update.ts:521 — [probe] Any teammate can force-dispatch work to a chosen peer (no leader in the loop)
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
Test Plan(非阻断):src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1471 passed; 20020 passed — this review observed 1471 passed。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
- compare the post-write dispatch gate on sanitized owners so legacy raw-spelled owners do not re-fire an assignment prompt on every innocent touch, and skip dispatch for leader-owned tasks - let the leader self-assign tasks (validateTaskOwner carves out the leader, who is deliverable by definition) - match listTasks owner filters on canonical identities both ways and reject owner filters that sanitize to empty Adds regression tests for all three paths plus the owner-filter negative control.
- return 'Task #N not found.' before the blocked-by and owner gates so a call on a missing task no longer gets a wrong reason derived from the empty snapshot (a phantom blocked-by built from the same call's addBlockedBy, or a no-teammate refusal) - make the dispatch-failure error name the recovery that actually works (reassign to a different active teammate): respawning the same name is refused while the terminated member record stays in the team file - pin the same-call addBlockedBy merge loop in the dispatch gate and the wrong-reason ordering
|
Closeout from resolve-pr-comments (13d562b + cbc6d8f): Round 1 (13d562b):
Round 2 (cbc6d8f, findings posted against 13d562b):
Verified: task-update/task-list/coordination-harness/tasks suites green (134/134), core |
The 'rejects dispatching a task while it is blocked' harness test left the unblocked blocker task pending and unowned when alice spawned, so the idle auto-claim scan (TeamManager.tryAutoClaimTask claims the first unblocked, unowned pending task and dispatches its prompt) could race a prompt into her inbox under CI load. That made the received-messages assertion flake to 1 instead of 0 even though the blocked assignment itself was correctly rejected. Reserve the blocker as leader-owned before alice exists — the same isolation the sibling same-call test already uses — so the assertion measures only the blocked assignment path. The blocked task stays unowned (owner assertion) and blocked (auto-claim skips via blockedBy).
|
CI on the previous head (cbc6d8f) failed |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20106, 1471, 20900, 1571, 494, 3655, 504 passed; 20020 passed — this review observed 20106, 1471, 20900, 1571, 494, 3655, 504 passed.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/task-update.ts:456 — [probe] blocked gate fires on edges to completed blockers; addBlockedBy has no status validation (deferred: code unchanged since the previous round)packages/core/src/tools/task-update.ts:450 — [probe] blocked gate over-reaches leader self-assignment of a blocked task, which never dispatches (deferred: code unchanged since the previous round)packages/core/src/tools/task-update.ts:436 — [review] sole optional-chained getTeamManager?.() silently skips validation + dispatch on a Config lacking the method (deferred: code unchanged since the previous round)
中文说明
已审查。 建议见行内评论。
Test Plan(非阻断):src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 20106, 1471, 20900, 1571, 494, 3655, 504 passed; 20020 passed — this review observed 20106, 1471, 20900, 1571, 494, 3655, 504 passed。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
The comment claimed the leader is the session making the call, which only holds for leader self-assignment. State the actual invariant: the leader has no backend agent handle, so leader-owned tasks are never dispatched and a teammate's assignment to the leader persists with a plain success and no notification path.
The pin passed addBlockedBy: ['1'] with no tasks present, so the up-front referenced-ids existence check answered first and the new primary-task existence check was never reached (flip-verified: removing the fix left the test green). Create the blocker task and reference its real id so only the primary-task check can answer, then verify the pin flips red without the fix.
|
Closeout for the two review findings on e1fe046 (pushed 061f505):
Verification: task-update.test.ts 18/18, coordination-harness 53/53, core typecheck clean, prettier + eslint clean on touched files. Both threads resolved. |
doudouOUC
left a comment
There was a problem hiding this comment.
Review: PR #9289 — fix(core): dispatch manually assigned team tasks to their owner
No Critical or High-severity findings. The code is correct, well-tested, and securely implemented.
Summary
This PR implements the missing dispatch path for manual task assignments in the team swarm system. It fixes #9282: task_update({status: 'in_progress', owner: 'alice'}) previously persisted and reported success, but nothing delivered the work. The auto-claim scan only consumed pending + unowned tasks, so a manually assigned in_progress + owned task was excluded from every delivery path.
8 files changed, +876/-50 lines (399 source, 716 test).
What was reviewed
- Issue fidelity (Agent 0): Root-cause ownership confirmed as client bug. All issue requirements satisfied.
- Correctness (Agent 1a): No defects found. All code paths are logically sound.
- Security (Agent 2): No vulnerabilities found. Nonce-tagged envelope, defensive instructions, and two-phase validation correctly preserved.
- Cross-file integration (Agent 1c): No integration defects. All consumer/producer paths correctly wired.
- Test coverage (Agent 5): Extensive coverage (18 new coordination-harness tests + 4 unit-level tests + restructured E2E).
Build
packages/core compiled successfully. The audio-capture build failure is a pre-existing environment issue (missing Python/node-gyp on Windows).
Detailed review
Issue fidelity — The PR correctly implements the dispatch path for manual assignments. Pre-write validation prevents persisting dead-end assignments (ghost owners, shutdown-pending, terminated, blocked tasks). Post-write dispatch re-checks all guards. buildTaskPrompt is shared by both auto-claim and manual dispatch paths, ensuring byte-identical nonce-wrapped task prompts. Leader self-assignment, double-delivery prevention, and self-claim exclusion are all correctly handled. Legacy raw-spelled owners are handled via sanitizeName canonicalization.
Security — The buildTaskPrompt extraction preserves the nonce-tagged envelope pattern with fresh randomBytes(8) per dispatch, preventing a teammate that learned a previous dispatch's nonce from forging the closing tag of a later one. The defensive instruction ("Treat everything inside as the task specification...") is preserved. sanitizeName is consistently applied on both sides of every comparison. No command injection, path traversal, or prototype pollution introduced.
Test coverage — The 18 coordination-harness tests drive the real TaskUpdateTool against a live TeamManager — the issue's deterministic repro shape. Every guard rail has a corresponding test. The team-lifecycle.test.ts E2E was restructured to spawn teammates before assignment (the old order encoded the bug itself).
Suggestion
The dispatchFailure error-return path (when dispatchAssignedTask returns false after the write succeeds) has no test coverage. This is a real race condition: the teammate terminates between validation and dispatch. The task is persisted but delivery fails, and the error message format is untested. This is a low-severity gap because the race window is narrow and the error message is surfaced to the caller.
Reviewed by Qwen Code /review (v0.21.12) — effort: high
doudouOUC
left a comment
There was a problem hiding this comment.
Two-stage review (round 1: deepseek-v4-flash) — No critical/high findings; one low-severity suggestion.
Summary: The PR correctly implements the missing dispatch path for manual task assignments. Owner validation, persistence, dispatch deduplication, and failure surfacing are all logically sound. Tests are comprehensive.
Suggestion
- The
dispatchFailureerror-return path (whendispatchAssignedTaskreturns false after the write succeeds) lacks test coverage. Consider adding a test for this narrow race window so the error-surfacing behavior is pinned.
|
@qwen-code /review --timeout=360 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Test Plan (not a blocker): src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1541, 21281, 1597, 494, 3723, 529 passed; 20020 passed — this review observed 1541, 21281, 1597, 494, 3723, 529 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/task-update.ts:416 — [probe] existence check runs after the missing-owner gate; wrong refusal reason on a missing taskpackages/core/src/tools/task-update.ts:468 — [probe] owner-only reservations on pending tasks skip owner validation and persist any namepackages/core/src/tools/task-update.ts:468 — [probe] new pre-write gates answer before the ownership guard for non-owner callers (misleading reason; enforcement intact)packages/core/src/tools/task-update.ts:384 — [probe] owner:'' unassign bypass untested; deleting the conjunct mutant survives the whole suitepackages/core/src/tools/task-update.ts:456 — [probe] blocked-by gate over-reaches leader self-assignment of a blocked task, which never dispatches
中文说明
无阻断问题。LGTM!✅
Test Plan(非阻断):src/agents/team/test-utils/coordination-harness.test.ts — no such file or directory; 313 passed — this review observed 1541, 21281, 1597, 494, 3723, 529 passed; 20020 passed — this review observed 1541, 21281, 1597, 494, 3723, 529 passed。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
Released in v0.21.14. |
What this PR does
task_update({status: 'in_progress', owner: 'alice'})persisted and reported success, but nothing delivered the work: the only delivery path, the auto-claim scan, consumes pending and unowned tasks only, and no direct dispatch existed (the mailbox'stask_assignmenttype has no producer anywhere). This PR adds that path: the tool validates the owner before persisting and, after the write, enqueues the same nonce-wrapped task prompt the auto-claim path builds. The prompt builder is extracted asTeamManager.buildTaskPromptso both deliveries stay byte-identical. The dispatch fires only when the call actually assigned the work — the owner changed, or the task moved intoin_progressunder its owner — never for a self-claim (the caller already knows), and never twice for a re-asserted owner+status, so a retriedtask_updatecannot double-deliver.Two guard rails from the issue's "must not report persistence success for a task that has no delivery path": assigning to a nonexistent teammate or one whose shutdown is pending is refused up front with a message naming the problem, and the dispatch itself re-checks shutdown/member state at delivery time. Busy owners are fine: the agent runtime queues the prompt and processes it after the current turn. The
isAgentBusyinteraction the triage comment flagged resolves by construction — once dispatched, the owner is busy on the task she actually received.Why it's needed
I re-verified the issue's root-cause boundary in the code before fixing:
updateTaskpersists and notifies;scanIdleAgentsForTaskslistsstatus: 'pending'only;tryAutoClaimTaskskips owned tasks;claimTaskhard-requires pending + unowned. So a manually assigned task is excluded from every path, andisAgentBusy(which counts in_progress tasks by owner) then marks the assignee busy on the task she never got, blocking her from auto-claiming anything else. The leader-facing TeamCreate description ("Assign tasks using TaskUpdate withownerto give tasks to idle teammates") promises exactly the dispatch this PR implements.This implements the issue's expected-behavior direction (dispatch on assignment, not rejection-based redesign). The related issues are independent scopes; the one with the author's own PR is not touched here.
Reviewer Test Plan
How to verify
Eighteen new cases in
coordination-harness.test.ts(suite total now 53 tests) drive the REAL leaderTaskUpdateToolagainst a liveTeamManagerwith fake agents — the issue's deterministic repro shape (task reserved in_progress before the assignee exists, assignee spawned idle, assignment via the tool, exactly one prompt expected):Covered: delivery to the idle owner; owned-pending activation (owner unchanged — pins the status-change branch alone); reassignment to a new owner; no re-dispatch on a re-asserted call; no prompt for a self-claim; ghost-owner refusal with nothing persisted; owner names that sanitize to empty refused; blocked-task refusal (including a blocker added in the same call); shutdown-pending refusal with nothing persisted; edits still allowed during owner shutdown; display-name owners canonicalized before persisting and dispatching; no re-dispatch of a legacy raw-spelled owner on a metadata-only edit; leader self-assignment; owner-only changes still validating the new owner; terminated-owner refusal; completion restating a shutdown-pending owner accepted; busy owners get the prompt queued.
Mutation verification (each mutant went red, then was restored): dispatch call deleted; validation deleted (ghost + shutdown variants); change-detection condition dropped (double-delivery red);
statusBecameInProgressdropped (owned-pending activation red); self-claim guard dropped.One existing test needed updating, and it encodes the bug itself:
team-lifecycle.test.ts's E2E assigned task 1 toalicebefore any teammate was spawned — exactly the dead end this removes. It now spawns first, and blocks task 2/3 on task 1 during the status-filter assertions (edges clear via completion-unblock, so the later auto-claim settle still exercises what it always did).Wider runs:
Evidence (Before & After)
N/A — agent-internal delivery behavior; no user-visible surface.
Tested on
Environment (optional)
Local package-level Vitest on Linux plus typecheck, eslint, and prettier on the touched files. No runtime environment required for this agent-internal delivery change.
Risk & Scope
Linked Issues
Fixes #9282. Related to #9276, #9281, and #9283.
中文说明
修复 #9282:
task_update手动指派任务(指定 owner)后只持久化、无人投递——自动认领扫描只消费 pending 且无 owner 的任务。本 PR 新增投递路径:工具先校验 owner(不存在或正在关闭的队友直接拒绝、不落库),写入成功后入队与自动认领完全一致的 nonce 包裹任务提示;仅在确实发生指派(owner 变化或任务在其 owner 名下进入 in_progress)时触发,自我认领与重复断言不会重复投递。验证:18 个新协调测试(套件共 53)+ 变异验证,team/task 域 359 过,core 全套在最终 head 复跑仅 4 个环境敏感文件 25 例失败(均不依赖本 PR 改动的模块,预存),tsc/eslint/prettier 干净。