Skip to content

Commit e8816c5

Browse files
gumadeirasgumadeiras
committed
Agents: fix subagent completion delivery to origin channel
1 parent ca43efa commit e8816c5

File tree

5 files changed

+101
-59
lines changed

5 files changed

+101
-59
lines changed

‎src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.e2e.test.ts‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
157157
const tool = await getSessionsSpawnTool({
158158
agentSessionKey: "main",
159159
agentChannel: "whatsapp",
160+
agentTo: "+123",
160161
});
161162

162163
const result = await tool.execute("call2", {
@@ -185,7 +186,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
185186

186187
await waitFor(() => ctx.waitCalls.some((call) => call.runId === child.runId));
187188
await waitFor(() => patchCalls.some((call) => call.label === "my-task"));
188-
await waitFor(() => ctx.calls.filter((c) => c.method === "agent").length >= 2);
189+
await waitFor(() => ctx.calls.filter((c) => c.method === "send").length >= 1);
189190

190191
const childWait = ctx.waitCalls.find((call) => call.runId === child.runId);
191192
expect(childWait?.timeoutMs).toBe(1000);
@@ -194,22 +195,24 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
194195
expect(labelPatch?.key).toBe(child.sessionKey);
195196
expect(labelPatch?.label).toBe("my-task");
196197

197-
// Two agent calls: subagent spawn + main agent trigger
198+
// Subagent spawn call plus direct outbound completion send.
198199
const agentCalls = ctx.calls.filter((c) => c.method === "agent");
199-
expect(agentCalls).toHaveLength(2);
200+
expect(agentCalls).toHaveLength(1);
200201

201202
// First call: subagent spawn
202203
const first = agentCalls[0]?.params as { lane?: string } | undefined;
203204
expect(first?.lane).toBe("subagent");
204205

205-
// Second call: main agent trigger (not "Sub-agent announce step." anymore)
206-
const second = agentCalls[1]?.params as { sessionKey?: string; message?: string } | undefined;
207-
expect(second?.sessionKey).toBe("main");
208-
expect(second?.message).toContain("subagent task");
209-
210-
// No direct send to external channel (main agent handles delivery)
206+
// Direct send should route completion to the requester channel/session.
211207
const sendCalls = ctx.calls.filter((c) => c.method === "send");
212-
expect(sendCalls.length).toBe(0);
208+
expect(sendCalls).toHaveLength(1);
209+
const send = sendCalls[0]?.params as
210+
| { sessionKey?: string; channel?: string; to?: string; message?: string }
211+
| undefined;
212+
expect(send?.sessionKey).toBe("agent:main:main");
213+
expect(send?.channel).toBe("whatsapp");
214+
expect(send?.to).toBe("+123");
215+
expect(send?.message).toBe("done");
213216
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
214217
});
215218

@@ -232,6 +235,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
232235
const tool = await getSessionsSpawnTool({
233236
agentSessionKey: "discord:group:req",
234237
agentChannel: "discord",
238+
agentTo: "discord:dm:u123",
235239
});
236240

237241
const result = await tool.execute("call1", {
@@ -269,7 +273,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
269273
expect(childWait?.timeoutMs).toBe(1000);
270274

271275
const agentCalls = ctx.calls.filter((call) => call.method === "agent");
272-
expect(agentCalls).toHaveLength(2);
276+
expect(agentCalls).toHaveLength(1);
273277

274278
const first = agentCalls[0]?.params as
275279
| {
@@ -285,19 +289,15 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
285289
expect(first?.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
286290
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
287291

288-
const second = agentCalls[1]?.params as
289-
| {
290-
sessionKey?: string;
291-
message?: string;
292-
deliver?: boolean;
293-
}
294-
| undefined;
295-
expect(second?.sessionKey).toBe("discord:group:req");
296-
expect(second?.deliver).toBe(true);
297-
expect(second?.message).toContain("subagent task");
298-
299292
const sendCalls = ctx.calls.filter((c) => c.method === "send");
300-
expect(sendCalls.length).toBe(0);
293+
expect(sendCalls).toHaveLength(1);
294+
const send = sendCalls[0]?.params as
295+
| { sessionKey?: string; channel?: string; to?: string; message?: string }
296+
| undefined;
297+
expect(send?.sessionKey).toBe("agent:main:discord:group:req");
298+
expect(send?.channel).toBe("discord");
299+
expect(send?.to).toBe("discord:dm:u123");
300+
expect(send?.message).toContain("completed successfully");
301301

302302
expect(deletedKey?.startsWith("agent:main:subagent:")).toBe(true);
303303
});
@@ -323,6 +323,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
323323
const tool = await getSessionsSpawnTool({
324324
agentSessionKey: "discord:group:req",
325325
agentChannel: "discord",
326+
agentTo: "discord:dm:u123",
326327
});
327328

328329
const result = await tool.execute("call1b", {
@@ -340,29 +341,30 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
340341
throw new Error("missing child runId");
341342
}
342343
await waitFor(() => ctx.waitCalls.some((call) => call.runId === child.runId));
343-
await waitFor(() => ctx.calls.filter((call) => call.method === "agent").length >= 2);
344+
await waitFor(() => ctx.calls.filter((call) => call.method === "send").length >= 1);
344345
await waitFor(() => Boolean(deletedKey));
345346

346347
const childWait = ctx.waitCalls.find((call) => call.runId === child.runId);
347348
expect(childWait?.timeoutMs).toBe(1000);
348349
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
349350

350-
// Two agent calls: subagent spawn + main agent trigger
351+
// One agent call for spawn, then direct completion send.
351352
const agentCalls = ctx.calls.filter((call) => call.method === "agent");
352-
expect(agentCalls).toHaveLength(2);
353+
expect(agentCalls).toHaveLength(1);
353354

354355
// First call: subagent spawn
355356
const first = agentCalls[0]?.params as { lane?: string } | undefined;
356357
expect(first?.lane).toBe("subagent");
357358

358-
// Second call: main agent trigger
359-
const second = agentCalls[1]?.params as { sessionKey?: string; deliver?: boolean } | undefined;
360-
expect(second?.sessionKey).toBe("discord:group:req");
361-
expect(second?.deliver).toBe(true);
362-
363-
// No direct send to external channel (main agent handles delivery)
364359
const sendCalls = ctx.calls.filter((c) => c.method === "send");
365-
expect(sendCalls.length).toBe(0);
360+
expect(sendCalls).toHaveLength(1);
361+
const send = sendCalls[0]?.params as
362+
| { sessionKey?: string; channel?: string; to?: string; message?: string }
363+
| undefined;
364+
expect(send?.sessionKey).toBe("agent:main:discord:group:req");
365+
expect(send?.channel).toBe("discord");
366+
expect(send?.to).toBe("discord:dm:u123");
367+
expect(send?.message).toBe("done");
366368

367369
// Session should be deleted
368370
expect(deletedKey?.startsWith("agent:main:subagent:")).toBe(true);

‎src/agents/subagent-announce.format.e2e.test.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ describe("subagent announce formatting", () => {
542542
queueDebounceMs: 0,
543543
},
544544
};
545-
agentSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
545+
sendSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
546546

547547
const didAnnounce = await runSubagentAnnounceFlow({
548548
childSessionKey: "agent:main:subagent:worker",
@@ -554,16 +554,17 @@ describe("subagent announce formatting", () => {
554554
});
555555

556556
expect(didAnnounce).toBe(true);
557-
await expect.poll(() => agentSpy.mock.calls.length).toBe(2);
558-
expect(agentSpy.mock.calls[0]?.[0]).toMatchObject({
559-
method: "agent",
557+
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
558+
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
559+
expect(sendSpy.mock.calls[0]?.[0]).toMatchObject({
560+
method: "send",
560561
params: { sessionKey: "agent:main:main" },
561562
});
562-
expect(agentSpy.mock.calls[1]?.[0]).toMatchObject({
563+
expect(agentSpy.mock.calls[0]?.[0]).toMatchObject({
563564
method: "agent",
564565
params: { sessionKey: "agent:main:main" },
565566
});
566-
expect(agentSpy.mock.calls[1]?.[0]).toMatchObject({
567+
expect(agentSpy.mock.calls[0]?.[0]).toMatchObject({
567568
method: "agent",
568569
params: { channel: "whatsapp", to: "+1555", deliver: true },
569570
});
@@ -580,7 +581,7 @@ describe("subagent announce formatting", () => {
580581
lastTo: "+1555",
581582
},
582583
};
583-
agentSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
584+
sendSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
584585

585586
const didAnnounce = await runSubagentAnnounceFlow({
586587
childSessionKey: "agent:main:subagent:worker",
@@ -592,7 +593,8 @@ describe("subagent announce formatting", () => {
592593
});
593594

594595
expect(didAnnounce).toBe(false);
595-
expect(agentSpy).toHaveBeenCalledTimes(1);
596+
expect(sendSpy).toHaveBeenCalledTimes(1);
597+
expect(agentSpy).toHaveBeenCalledTimes(0);
596598
});
597599

598600
it("uses assistant output for completion-mode when latest assistant text exists", async () => {
@@ -621,8 +623,8 @@ describe("subagent announce formatting", () => {
621623
});
622624

623625
expect(didAnnounce).toBe(true);
624-
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
625-
const call = agentSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
626+
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
627+
const call = sendSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
626628
const msg = call?.params?.message as string;
627629
expect(msg).toContain("assistant completion text");
628630
expect(msg).not.toContain("old tool output");
@@ -654,8 +656,8 @@ describe("subagent announce formatting", () => {
654656
});
655657

656658
expect(didAnnounce).toBe(true);
657-
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
658-
const call = agentSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
659+
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
660+
const call = sendSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
659661
const msg = call?.params?.message as string;
660662
expect(msg).toContain("tool output only");
661663
});

‎src/agents/subagent-announce.ts‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
mergeDeliveryContext,
1818
normalizeDeliveryContext,
1919
} from "../utils/delivery-context.js";
20+
import { isDeliverableMessageChannel } from "../utils/message-channel.js";
2021
import {
2122
buildAnnounceIdFromChildRun,
2223
buildAnnounceIdempotencyKey,
@@ -44,6 +45,19 @@ type SubagentAnnounceDeliveryResult = {
4445
error?: string;
4546
};
4647

48+
function buildCompletionDeliveryMessage(params: {
49+
findings: string;
50+
subagentName: string;
51+
}): string {
52+
const findingsText = params.findings.trim();
53+
const hasFindings = findingsText.length > 0 && findingsText !== "(no output)";
54+
const header = `✅ Subagent ${params.subagentName} finished`;
55+
if (!hasFindings) {
56+
return header;
57+
}
58+
return `${header}\n\n${findingsText}`;
59+
}
60+
4761
function summarizeDeliveryError(error: unknown): string {
4862
if (error instanceof Error) {
4963
return error.message || "error";
@@ -256,10 +270,23 @@ function resolveAnnounceOrigin(
256270
entry?: DeliveryContextSource,
257271
requesterOrigin?: DeliveryContext,
258272
): DeliveryContext | undefined {
273+
const normalizedRequester = normalizeDeliveryContext(requesterOrigin);
274+
const normalizedEntry = deliveryContextFromSession(entry);
275+
if (normalizedRequester?.channel && !isDeliverableMessageChannel(normalizedRequester.channel)) {
276+
// Ignore internal/non-deliverable channel hints (for example webchat)
277+
// so a valid persisted route can still be used for outbound delivery.
278+
return mergeDeliveryContext(
279+
{
280+
accountId: normalizedRequester.accountId,
281+
threadId: normalizedRequester.threadId,
282+
},
283+
normalizedEntry,
284+
);
285+
}
259286
// requesterOrigin (captured at spawn time) reflects the channel the user is
260287
// actually on and must take priority over the session entry, which may carry
261288
// stale lastChannel / lastTo values from a previous channel interaction.
262-
return mergeDeliveryContext(requesterOrigin, deliveryContextFromSession(entry));
289+
return mergeDeliveryContext(normalizedRequester, normalizedEntry);
263290
}
264291

265292
async function sendAnnounce(item: AnnounceQueueItem) {
@@ -411,24 +438,29 @@ async function sendSubagentAnnounceDirectly(params: {
411438
directOrigin?: DeliveryContext;
412439
requesterIsSubagent: boolean;
413440
}): Promise<SubagentAnnounceDeliveryResult> {
441+
const cfg = loadConfig();
442+
const canonicalRequesterSessionKey = resolveRequesterStoreKey(
443+
cfg,
444+
params.targetRequesterSessionKey,
445+
);
414446
try {
415447
const completionDirectOrigin = normalizeDeliveryContext(params.completionDirectOrigin);
416-
const completionChannel =
448+
const completionChannelRaw =
417449
typeof completionDirectOrigin?.channel === "string"
418450
? completionDirectOrigin.channel.trim()
419451
: "";
452+
const completionChannel =
453+
completionChannelRaw && isDeliverableMessageChannel(completionChannelRaw)
454+
? completionChannelRaw
455+
: "";
420456
const completionTo =
421457
typeof completionDirectOrigin?.to === "string" ? completionDirectOrigin.to.trim() : "";
422-
const completionHasThreadHint =
423-
completionDirectOrigin?.threadId != null &&
424-
String(completionDirectOrigin.threadId).trim() !== "";
425458
const hasCompletionDirectTarget =
426459
!params.requesterIsSubagent && Boolean(completionChannel) && Boolean(completionTo);
427460

428461
if (
429462
params.expectsCompletionMessage &&
430463
hasCompletionDirectTarget &&
431-
!completionHasThreadHint &&
432464
params.completionMessage?.trim()
433465
) {
434466
await callGateway({
@@ -437,7 +469,7 @@ async function sendSubagentAnnounceDirectly(params: {
437469
channel: completionChannel,
438470
to: completionTo,
439471
accountId: completionDirectOrigin?.accountId,
440-
sessionKey: params.targetRequesterSessionKey,
472+
sessionKey: canonicalRequesterSessionKey,
441473
message: params.completionMessage,
442474
idempotencyKey: params.directIdempotencyKey,
443475
},
@@ -455,11 +487,10 @@ async function sendSubagentAnnounceDirectly(params: {
455487
directOrigin?.threadId != null && directOrigin.threadId !== ""
456488
? String(directOrigin.threadId)
457489
: undefined;
458-
459490
await callGateway({
460491
method: "agent",
461492
params: {
462-
sessionKey: params.targetRequesterSessionKey,
493+
sessionKey: canonicalRequesterSessionKey,
463494
message: params.triggerMessage,
464495
deliver: !params.requesterIsSubagent,
465496
channel: params.requesterIsSubagent ? undefined : directOrigin?.channel,
@@ -521,11 +552,11 @@ async function deliverSubagentAnnouncement(params: {
521552
targetRequesterSessionKey: params.targetRequesterSessionKey,
522553
triggerMessage: params.triggerMessage,
523554
completionMessage: params.completionMessage,
524-
expectsCompletionMessage: params.expectsCompletionMessage,
525555
directIdempotencyKey: params.directIdempotencyKey,
526556
completionDirectOrigin: params.completionDirectOrigin,
527557
directOrigin: params.directOrigin,
528558
requesterIsSubagent: params.requesterIsSubagent,
559+
expectsCompletionMessage: params.expectsCompletionMessage,
529560
});
530561
if (direct.delivered || !params.expectsCompletionMessage) {
531562
return direct;
@@ -806,6 +837,7 @@ export async function runSubagentAnnounceFlow(params: {
806837
// Build instructional message for main agent
807838
const announceType = params.announceType ?? "subagent task";
808839
const taskLabel = params.label || params.task || "task";
840+
const subagentName = resolveAgentIdFromSessionKey(params.childSessionKey);
809841
const announceSessionId = childSessionId || "unknown";
810842
const findings = reply || "(no output)";
811843
let completionMessage = "";
@@ -872,15 +904,19 @@ export async function runSubagentAnnounceFlow(params: {
872904
startedAt: params.startedAt,
873905
endedAt: params.endedAt,
874906
});
875-
completionMessage = [
907+
completionMessage = buildCompletionDeliveryMessage({
908+
findings,
909+
subagentName,
910+
});
911+
const internalSummaryMessage = [
876912
`[System Message] [sessionId: ${announceSessionId}] A ${announceType} "${taskLabel}" just ${statusLabel}.`,
877913
"",
878914
"Result:",
879915
findings,
880916
"",
881917
statsLine,
882918
].join("\n");
883-
triggerMessage = [completionMessage, "", replyInstruction].join("\n");
919+
triggerMessage = [internalSummaryMessage, "", replyInstruction].join("\n");
884920

885921
const announceId = buildAnnounceIdFromChildRun({
886922
childSessionKey: params.childSessionKey,

‎src/agents/subagent-spawn.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export type SpawnSubagentContext = {
4141
};
4242

4343
export const SUBAGENT_SPAWN_ACCEPTED_NOTE =
44-
"auto-announces on completion, do not poll/sleep. The response will be sent back as a user message.";
44+
"auto-announces on completion, do not poll/sleep. The response will be sent back as an agent message.";
4545

4646
export type SpawnSubagentResult = {
4747
status: "accepted" | "forbidden" | "error";

‎src/auto-reply/reply/commands-subagents.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,9 @@ export const handleSubagentsCommand: CommandHandler = async (params, allowTextCo
686686
const originatingTo =
687687
typeof params.ctx.OriginatingTo === "string" ? params.ctx.OriginatingTo.trim() : "";
688688
const fallbackTo = typeof params.ctx.To === "string" ? params.ctx.To.trim() : "";
689-
const normalizedTo = commandTo || originatingTo || fallbackTo || undefined;
689+
// OriginatingTo reflects the active conversation target and is safer than
690+
// command.to for cross-surface command dispatch.
691+
const normalizedTo = originatingTo || commandTo || fallbackTo || undefined;
690692

691693
const result = await spawnSubagentDirect(
692694
{ task, agentId, model, thinking, cleanup: "keep", expectsCompletionMessage: true },

0 commit comments

Comments
 (0)