From d887eb8dc2ccffb5f7068b436e4eacbbd069de58 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 17 May 2026 02:59:47 +0100 Subject: [PATCH] fix(agents): harden subagent completion delivery Co-authored-by: Galin Iliev Co-authored-by: Ava Daigo Co-authored-by: Moeed Ahmed --- CHANGELOG.md | 1 + docs/tools/subagents.md | 2 + src/agents/subagent-announce-delivery.test.ts | 188 +++++++++++++++--- src/agents/subagent-announce-delivery.ts | 31 +-- .../subagent-announce.format.e2e.test.ts | 3 + src/agents/subagent-announce.ts | 2 +- 6 files changed, 186 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b14279c927..b5100796f26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Agents/subagents: route group/channel subagent completions through message-tool-only handoffs when required and keep active-requester wake failures from dropping completion delivery. Fixes #82803. Thanks @galiniliev, @yozakura-ava, and @moeedahmed. - WhatsApp: honor forced document delivery for outbound image, GIF, and video media so `forceDocument`/`asDocument` sends preserve original media bytes instead of using compressed media payloads. (#79272) Thanks @itsuzef. ## 2026.5.17 diff --git a/docs/tools/subagents.md b/docs/tools/subagents.md index 00f2c75380a..af523ce00e8 100644 --- a/docs/tools/subagents.md +++ b/docs/tools/subagents.md @@ -86,7 +86,9 @@ requester chat when the run finishes. - OpenClaw hands completions back to the requester session through an `agent` turn with a stable idempotency key. - If the requester run is still active, OpenClaw first tries to wake/steer that run instead of starting a second visible reply path. + - If an active requester cannot be woken, OpenClaw falls back to a requester-agent handoff with the same completion context instead of dropping the announce. - If the requester-agent completion handoff fails or produces no visible output, OpenClaw treats delivery as failed and falls back to queue routing/retry. It does not raw-send the child result directly to the external chat. + - Group and channel completion handoffs follow the same message-tool-only visible reply policy as normal group/channel turns, so the requester agent must use the message tool when required. - If direct handoff cannot be used, it falls back to queue routing. - If queue routing is still not available, the announce is retried with a short exponential backoff before final give-up. - Completion delivery keeps the resolved requester route: thread-bound or conversation-bound completion routes win when available; if the completion origin only provides a channel, OpenClaw fills the missing target/account from the requester session's resolved route (`lastChannel` / `lastTo` / `lastAccountId`) so direct delivery still works. diff --git a/src/agents/subagent-announce-delivery.test.ts b/src/agents/subagent-announce-delivery.test.ts index 563e635e623..32338f50055 100644 --- a/src/agents/subagent-announce-delivery.test.ts +++ b/src/agents/subagent-announce-delivery.test.ts @@ -1083,8 +1083,12 @@ describe("deliverSubagentAnnouncement completion delivery", () => { expect(sendMessage).not.toHaveBeenCalled(); }); - it("does not queue when an active Telegram requester cannot be woken directly", async () => { - const callGateway = createGatewayMock(); + it("falls back to requester-agent handoff when an active Telegram requester cannot be woken", async () => { + const callGateway = createGatewayMock({ + result: { + payloads: [{ text: "child completion output" }], + }, + }); const sendMessage = createSendMessageMock(); const queueEmbeddedPiMessageWithOutcome = createQueueOutcomeMock(false); const result = await deliverTelegramDirectMessageCompletion({ @@ -1109,45 +1113,93 @@ describe("deliverSubagentAnnouncement completion delivery", () => { }); expectRecordFields(result, { - delivered: false, + delivered: true, path: "direct", + phases: [ + { + phase: "direct-primary", + delivered: true, + path: "direct", + error: undefined, + }, + ], + }); + expect(queueEmbeddedPiMessageWithOutcome).toHaveBeenCalledTimes(1); + expect(queueEmbeddedPiMessageWithOutcome).toHaveBeenCalledWith( + "requester-session-telegram", + "child done", + { + steeringMode: "all", + debounceMs: 500, + }, + ); + expect(callGateway).toHaveBeenCalledTimes(1); + expect(sendMessage).not.toHaveBeenCalled(); + }); + + it("uses steer fallback when a completion handoff has no visible output", async () => { + const callGateway = createGatewayMock({ + result: { + payloads: [], + }, + }); + const queueEmbeddedPiMessageWithOutcome = vi + .fn() + .mockImplementationOnce((sessionId: string) => ({ + queued: false, + sessionId, + reason: "not_streaming", + gatewayHealth: "live", + })) + .mockImplementationOnce((sessionId: string) => ({ + queued: true, + sessionId, + target: "embedded_run", + gatewayHealth: "live", + })); + const result = await deliverSlackChannelAnnouncement({ + callGateway, + sessionId: "requester-session-channel", + isActive: false, + expectsCompletionMessage: true, + directIdempotencyKey: "announce-channel-empty-direct-steer-fallback", + queueEmbeddedPiMessageWithOutcome, + internalEvents: [ + { + type: "task_completion", + source: "subagent", + childSessionKey: "agent:worker:subagent:child", + childSessionId: "child-session-id", + announceType: "subagent task", + taskLabel: "channel completion smoke", + status: "ok", + statusLabel: "completed successfully", + result: "child completion output", + replyInstruction: "Summarize the result.", + }, + ], + }); + + expectRecordFields(result, { + delivered: true, + path: "steered", phases: [ { phase: "direct-primary", delivered: false, path: "direct", - error: - "active requester session could not be woken: queue_message_failed reason=not_streaming sessionId=requester-session-telegram gatewayHealth=live", + error: "completion agent did not produce a visible reply", }, { phase: "steer-fallback", - delivered: false, - path: "none", + delivered: true, + path: "steered", error: undefined, }, ], }); expect(queueEmbeddedPiMessageWithOutcome).toHaveBeenCalledTimes(2); - expect(queueEmbeddedPiMessageWithOutcome).toHaveBeenNthCalledWith( - 1, - "requester-session-telegram", - "child done", - { - steeringMode: "all", - debounceMs: 500, - }, - ); - expect(queueEmbeddedPiMessageWithOutcome).toHaveBeenNthCalledWith( - 2, - "requester-session-telegram", - "child done", - { - steeringMode: "all", - debounceMs: 500, - }, - ); - expect(callGateway).not.toHaveBeenCalled(); - expect(sendMessage).not.toHaveBeenCalled(); + expect(callGateway).toHaveBeenCalledTimes(1); }); it("reports failure when announce-agent returns no visible output", async () => { @@ -1846,6 +1898,88 @@ describe("deliverSubagentAnnouncement completion delivery", () => { expect(sendMessage).not.toHaveBeenCalled(); }); + it("requires message-tool delivery for channel subagent completions", async () => { + const callGateway = createGatewayMock({ + result: { + payloads: [{ text: "The subagent is done." }], + }, + }); + const result = await deliverSlackChannelAnnouncement({ + callGateway, + sessionId: "requester-session-channel", + isActive: false, + expectsCompletionMessage: true, + directIdempotencyKey: "announce-channel-subagent-message-tool", + sourceTool: "subagent_announce", + internalEvents: [ + { + type: "task_completion", + source: "subagent", + childSessionKey: "agent:worker:subagent:child", + childSessionId: "child-session-id", + announceType: "subagent task", + taskLabel: "channel completion smoke", + status: "ok", + statusLabel: "completed successfully", + result: "child completion output", + replyInstruction: "Summarize the result.", + }, + ], + }); + + expectRecordFields(result, { + delivered: false, + path: "direct", + error: "completion agent did not deliver through the message tool", + }); + expectGatewayAgentParams(callGateway, { + deliver: false, + channel: "slack", + accountId: "acct-1", + to: "channel:C123", + threadId: undefined, + sourceReplyDeliveryMode: "message_tool_only", + }); + }); + + it("keeps automatic final delivery for direct subagent completions", async () => { + const callGateway = createGatewayMock({ + result: { + payloads: [{ text: "The subagent is done." }], + }, + }); + const result = await deliverDiscordDirectMessageCompletion({ + callGateway, + sourceTool: "subagent_announce", + internalEvents: [ + { + type: "task_completion", + source: "subagent", + childSessionKey: "agent:worker:subagent:child", + childSessionId: "child-session-id", + announceType: "subagent task", + taskLabel: "direct completion smoke", + status: "ok", + statusLabel: "completed successfully", + result: "child completion output", + replyInstruction: "Summarize the result.", + }, + ], + }); + + expectRecordFields(result, { + delivered: true, + path: "direct", + }); + expectGatewayAgentParams(callGateway, { + deliver: true, + channel: "discord", + accountId: "acct-1", + to: "dm:U123", + threadId: undefined, + }); + }); + it("falls back to the external requester route when completion origin is internal", async () => { const callGateway = createGatewayMock({ result: { diff --git a/src/agents/subagent-announce-delivery.ts b/src/agents/subagent-announce-delivery.ts index 04774959c5c..ce25a0965d9 100644 --- a/src/agents/subagent-announce-delivery.ts +++ b/src/agents/subagent-announce-delivery.ts @@ -1,3 +1,4 @@ +import { completionRequiresMessageToolDelivery } from "../auto-reply/reply/completion-delivery-policy.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { ConversationRef } from "../infra/outbound/session-binding-service.js"; import { stringifyRouteThreadId } from "../plugin-sdk/channel-route.js"; @@ -60,6 +61,7 @@ const MAX_TIMER_SAFE_TIMEOUT_MS = 2_147_000_000; const AGENT_MEDIATED_COMPLETION_TOOLS = new Set([ "image_generate", "music_generate", + "subagent_announce", "video_generate", ]); @@ -634,7 +636,17 @@ async function sendSubagentAnnounceDirectly(params: { sourceTool: params.sourceTool, }); const expectedMediaUrls = collectExpectedMediaFromInternalEvents(params.internalEvents); - const requiresMessageToolDelivery = agentMediatedCompletion && expectedMediaUrls.length > 0; + const requiresMessageToolDelivery = + agentMediatedCompletion && + (expectedMediaUrls.length > 0 || + completionRequiresMessageToolDelivery({ + cfg, + requesterSessionKey: params.requesterSessionKey, + targetRequesterSessionKey: canonicalRequesterSessionKey, + requesterEntry, + directOrigin: effectiveDirectOrigin, + requesterSessionOrigin, + })); const completionSourceReplyDeliveryMode = requiresMessageToolDelivery ? "message_tool_only" : undefined; @@ -670,20 +682,13 @@ async function sendSubagentAnnounceDirectly(params: { path: "steered", }; } - const shouldFallbackToForcedAgentHandoff = - requiresMessageToolDelivery && wakeOutcome.reason === "source_reply_delivery_mode_mismatch"; - if (requesterActivity.isActive && !shouldFallbackToForcedAgentHandoff) { - // Active requester sessions should receive completion data through their - // running agent turn. If wake fails, let the dispatch layer steer/retry; - // do not bypass the requester agent with raw child output. - return { - delivered: false, - path: "direct", - error: formatQueueWakeFailureError( + if (requesterActivity.isActive) { + defaultRuntime.log( + `[warn] Active requester session could not be woken for subagent completion; falling back to requester-agent handoff: ${formatQueueWakeFailureError( "active requester session could not be woken", wakeOutcome, - ), - }; + )}`, + ); } } if (params.signal?.aborted) { diff --git a/src/agents/subagent-announce.format.e2e.test.ts b/src/agents/subagent-announce.format.e2e.test.ts index 8c07e8824b3..487d449ef2d 100644 --- a/src/agents/subagent-announce.format.e2e.test.ts +++ b/src/agents/subagent-announce.format.e2e.test.ts @@ -518,6 +518,9 @@ describe("subagent announce formatting", () => { expect(msg).toContain( "If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update.", ); + expect(msg).toContain( + "If the runtime marks this route as message-tool-only, send visible output with the message tool first", + ); expect(msg).toContain("Keep this internal context private"); expect(call?.params?.internalEvents?.[0]?.type).toBe("task_completion"); expect(call?.params?.internalEvents?.[0]?.taskLabel).toBe("do thing"); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index d8a369d9b7f..6bd8ce8ce84 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -88,7 +88,7 @@ function buildAnnounceReplyInstruction(params: { return `Convert this completion into a concise internal orchestration update for your parent agent in your own words. Keep this internal context private (don't mention system/log/stats/session details or announce type). If this result is duplicate or no update is needed, reply ONLY: ${SILENT_REPLY_TOKEN}.`; } if (params.expectsCompletionMessage) { - return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. Keep this internal context private (don't mention system/log/stats/session details or announce type).`; + return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. If the runtime marks this route as message-tool-only, send visible output with the message tool first, then reply ONLY: ${SILENT_REPLY_TOKEN}. Keep this internal context private (don't mention system/log/stats/session details or announce type).`; } return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. Keep this internal context private (don't mention system/log/stats/session details or announce type), and do not copy the internal event text verbatim. Reply ONLY: ${SILENT_REPLY_TOKEN} if this exact result was already delivered to the user in this same turn.`; }