feat(taskflow): emit code-level completion notification on task submission - #1206
feat(taskflow): emit code-level completion notification on task submission#1206LUOSENGWA wants to merge 3 commits into
Conversation
e8bd864 to
77f9f12
Compare
oss-maintainer
left a comment
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Mirrors the delegate_task notification pattern for submit_task: a code-level, stable-txn Matrix message with m.mentions to the leader, so completion wake signals no longer depend on the Worker LLM remembering the contract line. The design is sound and the implementation faithful to the existing pattern — verified against main@4ab6b15: all reused helpers exist (_load_runtime_config, _section, _matrix_content, _canonical_room_id, _validate_assignee_membership, _write_task), leader resolution normalizes roles the same way as _roomflow_room_meta, the membership guard prevents sending to users outside the room, failure is strictly best-effort (submission and artifact publishing never blocked), and the notificationNeeded requester hint is correctly preserved. The unused arguments parameter matches _send_delegate_notification's existing signature style. Contract tests cover send/content, event-id persistence, retry reuse, the BLOCKED line, and forced-500 non-blocking. One warning inline: the completionEventId reuse branch can suppress a later, different completion (BLOCKED → same-task_id resubmit → SUCCESS), because both the reuse shortcut and the stable submit-<task-id> txn assume one terminal notification per task_id.
Findings
- [Warning] server.py:4219 — completionEventId reuse + stable txn assume a single terminal notification per task_id; a same-task_id resubmit with a different result_status gets no new mention
Automated review by github-manager-bot
| "skipped": True, | ||
| "error": "team leader Matrix ID not found in runtime config", | ||
| } | ||
| if task.get("completionEventId"): |
There was a problem hiding this comment.
[Warning] completionEventId reuse may swallow a different completion. submit_task leaves the task mutable ("submitted" is not in TERMINAL_TASK_STATUSES, and the plan node also becomes submitted), so the same task_id can be submitted again before the leader records a decision. Sequence: Worker submits BLOCKED → event persisted here → task later unblocked and resubmitted SUCCESS with the same task_id → this branch returns the stale BLOCKED event (reused: true) and no TASK_COMPLETED mention is ever sent — reproducing exactly the stall this PR fixes, just on a narrower path. Note the stable txn below (submit-<task-id>, line 4161) makes this structural: even without the reuse shortcut, Matrix would reject a re-PUT with a different body under the same txn id. Could you confirm whether a same-task_id resubmit after a BLOCKED submission is reachable in the runtime integration (Task Service resume flow)? If it is, consider keying the txn/event slot on (task_id, result_status) or an attempt counter, or invalidating completionEventId when the new result_status differs from the recorded one.
oss-maintainer
left a comment
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
This PR adds automatic Matrix completion notifications to submit_task, mirroring the existing delegate_task pattern with stable transaction IDs for idempotency. The implementation is correct, best-effort by design (notification failures never block submission), and well-tested across success, retry, BLOCKED, and HTTP-failure paths. All findings are informational: an unused parameter, an edge case where _write_task failure could cause a redundant HTTP call on retry (but not a duplicate message), and missing test coverage for skip branches (standalone mode, membership failures). The change is backward-compatible (additive response field, preserves notificationNeeded hint) and solves a real production issue where workers forget to send completion mentions after context compaction.
Automated review by github-manager-bot
| def _team_leader_matrix_id() -> str: | ||
| """Resolve the team leader's Matrix user ID from the runtime config. | ||
|
|
||
| The controller projects the full team roster (with roles and Matrix |
There was a problem hiding this comment.
_send_task_completion_notification accepts an arguments parameter but never reads it. The orchestrator _task_completion_notification passes it through, but the send function only uses its keyword arguments. Consider dropping the parameter (or documenting why it is reserved) to avoid dead-code confusion.
| ) | ||
| if notification.get("sent"): | ||
| task["completionEventId"] = notification.get("eventId") | ||
| _write_task(arguments, task) |
There was a problem hiding this comment.
If _write_task raises after the Matrix PUT succeeded, completionEventId is not persisted. The stable txn ID submit-<task-id> prevents a true duplicate in Matrix on retry, but the retry will make a redundant HTTP round-trip and return reused: false instead of reused: true. Consider persisting completionEventId before the HTTP call (optimistic write) or catching _write_task errors explicitly so the response reflects the actual state.
| "action": "submit_task", | ||
| "payload": { | ||
| "taskId": task_id, | ||
| "status": "SUCCESS", |
There was a problem hiding this comment.
Tests cover SUCCESS, BLOCKED, retry, and HTTP-failure paths well, but the skip branches are untested: no-leader-in-config (standalone run), leader-not-in-room (membership guard), and no-Matrix-env. These are documented best-effort paths, but a missing leader or missing env var are common deployment configurations worth a smoke assertion (e.g., notification.skipped == true).
|
Thanks for addressing the completion-notification gap. The direction is valuable, but there is one ordering issue that needs to be fixed before merge.
Please move shared-storage sync before the completion notification and send the mention only after the result has been synced successfully. If sync fails, return a clear Please also align the status contract while touching this path: Finally, please create and link a public issue for this task-lifecycle design so the behavior and retry semantics can be discussed and tracked by the community. Once these points are addressed and CI is green, this should be ready for another merge-readiness check. 感谢补齐任务完成通知链路，这个方向是有价值的，但当前有一个发送顺序问题需要在合并前修复。
请将共享存储同步放到完成通知之前，并且只在结果成功同步后再通知 Leader。如果同步失败，应明确返回 另外请统一这个链路的状态契约： 最后，请为这项任务生命周期设计创建并关联一个公开 Issue，方便社区讨论和追踪通知及重试语义。上述问题修复且 CI 通过后，可以再进行合并检查。 |
628ca0c to
fffe862
Compare
…tack integration - submit sync-failure: agentscope-ai#1183 _sync_failure_result contract (statePersisted) + agentscope-ai#1206 v2 withheld-notification clause; guard before notification - reused-digest submit branch returns the reused completion notification - _accept_task_result: resolve outstanding attention in place on task_meta (separate read-modify-write was clobbered by the terminal status write) - test-taskflow.rb: fix latent heredoc escape bug (Result body \n), role env toggles for agentscope-ai#1183 runtime-identity-first role, mc fail-sync hook for per-file sync (mc cp), vocabulary PARTIAL/FAILED -> INTERRUPTED, changed-status resubmit -> digest-fence conflict expectation - test-projectflow.rb: fixture runtime.yaml gains member role leader
- RuntimeConfigHandler proxies qwenpaw worker running-config (5-tab settings + Loop Engine catalog/status + custom-loop CRUD) from the Controller, so plugins/dashboards can inspect/adjust worker runtime behavior without the docker network. - runtime-aware: non-qwenpaw worker -> 400 (config model is qwenpaw-only). - RBAC: L1 full; L2 (team-leader / L2 human) team-scoped via findTeamMember + TeamMatches (404 to hide existence, W8). - 5-tab field whitelist: L2 PUT runtime-config passes only ReAct/Loop/ LLM-retry/long-term-memory/tool-level keys; unknown keys rejected (fail-closed, agentscope-ai#1216 safe-write pattern). - loop-change notification: custom-loop writes alert the team room with @leader + @Changer (Matrix m.mentions), per agentscope-ai#1206 infra. - TuwunelClient.SendNotification (+ Client interface): admin-identity message with m.mentions.user_ids. - routes: GET/PUT runtime-config, GET loops + loops/status, GET/POST/PUT/DELETE loops/custom[/{loop}] under /api/v1/workers/{name}. Closes the B-phase of the workbench 5-tab gap (F22): config is now controller-exposed and loop changes are auditable + notify the right @list.
- RuntimeConfigHandler proxies qwenpaw worker running-config (5-tab settings + Loop Engine catalog/status + custom-loop CRUD) from the Controller, so plugins/dashboards can inspect/adjust worker runtime behavior without the docker network. - runtime-aware: non-qwenpaw worker -> 400 (config model is qwenpaw-only). - RBAC: L1 full; L2 (team-leader / L2 human) team-scoped via findTeamMember + TeamMatches (404 to hide existence, W8). - 5-tab field whitelist: L2 PUT runtime-config passes only ReAct/Loop/ LLM-retry/long-term-memory/tool-level keys; unknown keys rejected (fail-closed, agentscope-ai#1216 safe-write pattern). - loop-change notification: custom-loop writes alert the team room with @leader + @Changer (Matrix m.mentions), per agentscope-ai#1206 infra. - TuwunelClient.SendNotification (+ Client interface): admin-identity message with m.mentions.user_ids. - routes: GET/PUT runtime-config, GET loops + loops/status, GET/POST/PUT/DELETE loops/custom[/{loop}] under /api/v1/workers/{name}. Closes the B-phase of the workbench 5-tab gap (F22): config is now controller-exposed and loop changes are auditable + notify the right @list.
…_task
The taskflow MCP's delegate_task already publishes the assignment to the
task room at the code layer (_send_delegate_notification, stable txn),
but submit_task only returns a notificationNeeded hint - the Worker must
self-remember to @mention the leader with the contract line. Real
deployments (Node1) lost this line after multi-turn sessions, leaving
the Leader with no resume signal.
Mirror the delegate pattern in the same file:
- _send_task_completion_notification: Matrix HTTP PUT with m.mentions
(same path/auth as the message tool), stable txn 'submit-<task-id>'
so a retry cannot duplicate, first line follows the task-execution
skill contract (TASK_COMPLETED/BLOCKED + result path).
- _task_completion_notification: best-effort orchestration - resolve the
leader from the runtime config team roster, reuse the recorded
completionEventId on retry, validate room membership, persist the
event id on success. A send failure returns {sent: false, error}
and never blocks the terminal submission.
- submit_task response gains 'notification'; the notificationNeeded
hint is kept (it also drives the requester reply-route report).
Contract tests (test-taskflow.rb): completion line + mentions + worker
+ summary + event-id persistence, retry reuses without duplicating,
BLOCKED status uses the BLOCKED line, forced 500 does not block the
submission and persists no event id. Context file-event selection made
mxcUri-based instead of positional.
- submit_task: per-status first-line tokens (TASK_COMPLETED / TASK_PARTIAL / TASK_REVISION_NEEDED / TASK_BLOCKED / TASK_FAILED), status validation, status-scoped txn and completionEventStatus so a changed-status resubmit sends a fresh event - @initiator routing: human members (task initiator) mentioned alongside the leader in completion / attention / project events - P0 ordering: sync shared storage before the completion notification; a failed sync withholds the event and returns a retryable failure - new taskflow action request_attention (worker/leader/remote-member): in-flight human decisions as first-class idempotent room events (kind approval/decision/escalation/other), terminal guard, sync-first, resolved by accept_task_result or explicit resolved=true - complete_project: code-level PROJECT_COMPLETED room event with idempotent projectCompletionEventId - task-execution SKILL.md: contract rewritten around the code-generated per-status events; full accepted status set - test-taskflow.rb: 12 new assertion groups (ordering, tokens, @initiator, validation, resubmit, request_attention, PROJECT_COMPLETED); mc shim gains a push-only sync-failure hook Local verification: full extracted harness green (all pre-existing + new assertions); pytest 73 passed (4 pre-existing env failures on base). Refs: agentscope-ai#1229
346298c to
6e36b41
Compare
- RuntimeConfigHandler proxies qwenpaw worker running-config (5-tab settings + Loop Engine catalog/status + custom-loop CRUD) from the Controller, so plugins/dashboards can inspect/adjust worker runtime behavior without the docker network. - runtime-aware: non-qwenpaw worker -> 400 (config model is qwenpaw-only). - RBAC: L1 full; L2 (team-leader / L2 human) team-scoped via findTeamMember + TeamMatches (404 to hide existence, W8). - 5-tab field whitelist: L2 PUT runtime-config passes only ReAct/Loop/ LLM-retry/long-term-memory/tool-level keys; unknown keys rejected (fail-closed, agentscope-ai#1216 safe-write pattern). - loop-change notification: custom-loop writes alert the team room with @leader + @Changer (Matrix m.mentions), per agentscope-ai#1206 infra. - TuwunelClient.SendNotification (+ Client interface): admin-identity message with m.mentions.user_ids. - routes: GET/PUT runtime-config, GET loops + loops/status, GET/POST/PUT/DELETE loops/custom[/{loop}] under /api/v1/workers/{name}. Closes the B-phase of the workbench 5-tab gap (F22): config is now controller-exposed and loop changes are auditable + notify the right @list.
…tack integration - submit sync-failure: agentscope-ai#1183 _sync_failure_result contract (statePersisted) + agentscope-ai#1206 v2 withheld-notification clause; guard before notification - reused-digest submit branch returns the reused completion notification - _accept_task_result: resolve outstanding attention in place on task_meta (separate read-modify-write was clobbered by the terminal status write) - test-taskflow.rb: fix latent heredoc escape bug (Result body \n), role env toggles for agentscope-ai#1183 runtime-identity-first role, mc fail-sync hook for per-file sync (mc cp), vocabulary PARTIAL/FAILED -> INTERRUPTED, changed-status resubmit -> digest-fence conflict expectation - test-projectflow.rb: fixture runtime.yaml gains member role leader
3 participants
What
TeamHarness taskflow: code-level completion notification + lifecycle attention events (v2, design issue #1229).
Worker completion events used to be prompt-dependent (the LLM hand-sending a Matrix message; "a tool call does not count as the completion message" was unenforceable). This PR makes task-lifecycle attention signals first-class, idempotent, auditable room events generated by code:
submit_tasknow sends one first-line token per result status (TASK_COMPLETED/TASK_PARTIAL/TASK_REVISION_NEEDED/TASK_BLOCKED/TASK_FAILED), so leader-side prompts can branch on the line itself. The submitted status is validated against the accepted set; unknown values are rejected with a clear error.@initiatorrouting — human team members (the task initiator) are @mentioned alongside the leader in completion, attention, and project events, closing the "requester only gets ambient room" gap (Design: task-lifecycle attention events & channel routing (unified view for #1206 and #1219) #1229 Part A, Q2).submit_tasksyncs shared storage first; a failed sync withholds the notification and returns a retryable failure (the local task state is alreadysubmitted, so the retry is idempotent and the event is sent exactly once). A leader can no longer be woken by an event whose artifacts it cannot read.request_attention(new taskflow action) — in-flight human decisions (kind: approval / decision / escalation / other) as first-class events:ATTENTION_<KIND>: <task-id> - <question>, mentions leader + humans, idempotent per unresolved kind (retry reuses the recorded event), terminal tasks rejected, sync-first like submit. Resolved byaccept_task_result(closes all outstanding attention on the task) or an explicitresolved: true. Roles: worker / leader / remote-member (Design: task-lifecycle attention events & channel routing (unified view for #1206 and #1219) #1229 Q1).PROJECT_COMPLETEDoncomplete_project— a finished project now wakes the room (first task room, falling back to the project source room when it is a Matrix room) with an idempotent event (projectCompletionEventIdpersisted; retriedcomplete_projectreuses it).Idempotency: transaction ids are stable per (task, status) / (task, kind, attempt) / (project, status); recorded event ids are persisted in task/project state so retries reuse instead of duplicate. A resubmission with a changed status invalidates the recorded pair and sends a fresh event (fixes the silent-resubmit-reuse gap). Pre-upgrade tasks (no recorded status) keep the old reuse behavior.
Best-effort (unchanged from v1): notification-level failures (no room / leader / Matrix env, membership missing, HTTP error) never block the state mutation; the
notificationNeededhint is kept for the requester reply-route report.Verification
test-taskflow.rbharness green locally (extracted harness, clean env): all pre-existing assertions + 12 new groups — P0 ordering (fail → withhold → idempotent retry), per-status tokens,@initiatormentions, status validation, changed/same-status resubmit,request_attentionlifecycle (send / idempotent / different kind / explicit close / terminal guard / accept-resolves-all),PROJECT_COMPLETEDsend + retry reuse.Follow-ups (noted, not in this PR)
Related
TeamHarness taskflow：代码级完成通知 + 生命周期 attention 事件（v2，设计 issue #1229）。
Worker 完成事件原依赖 prompt（LLM 手动发 Matrix 消息，"工具调用不算完成消息"不可强制）。本 PR 把任务生命周期的 attention 信号变成代码生成的一等、幂等、可审计的房间事件：
submit_task按结果状态发送TASK_COMPLETED/TASK_PARTIAL/TASK_REVISION_NEEDED/TASK_BLOCKED/TASK_FAILED首行 token，Leader prompt 可直接按行分支。提交的状态对 accepted 集合做校验，非法值明确报错。request_attention（新 action）——进行中的需要人类决策（approval / decision / escalation / other）变成一等事件：未解决同类幂等复用、终态任务拒绝、sync-first；由accept_task_result（关闭该任务全部未决 attention）或显式resolved: true解决。角色：worker / leader / remote-member（Q1）。complete_project发PROJECT_COMPLETED——项目完成唤醒房间（第一个 task room，回退项目 source room），projectCompletionEventId持久化保证重试幂等。幂等：txn 按 (task, status) / (task, kind, attempt) / (project, status) 稳定；事件 ID 落 task/project state，重试复用不重复；状态变化的重新提交作废旧事件并发新事件（修复静默复用缺口）；升级前任务保持旧复用行为。
best-effort 不变：通知层失败（无房间/Leader/Matrix 环境、成员校验、HTTP 错误）永不阻塞状态变更；
notificationNeeded保留给 requester reply-route。验证：test-taskflow.rb 全量 harness 本地绿（含 12 组新断言）；Python 套件 73 过、4 个预存环境失败（base 同样失败，与本改动无关）。
follow-up（不在本 PR）：attention 事件的 DM 高显著度通道（v1 仅房间 @）；非房间成员 mention 过滤（Matrix 语义天然不可见，无需额外检查）。