Agents: fix subagent completion delivery to origin channel
This commit is contained in:
@@ -157,6 +157,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
agentTo: "+123",
|
||||
});
|
||||
|
||||
const result = await tool.execute("call2", {
|
||||
@@ -185,7 +186,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
|
||||
await waitFor(() => ctx.waitCalls.some((call) => call.runId === child.runId));
|
||||
await waitFor(() => patchCalls.some((call) => call.label === "my-task"));
|
||||
await waitFor(() => ctx.calls.filter((c) => c.method === "agent").length >= 2);
|
||||
await waitFor(() => ctx.calls.filter((c) => c.method === "send").length >= 1);
|
||||
|
||||
const childWait = ctx.waitCalls.find((call) => call.runId === child.runId);
|
||||
expect(childWait?.timeoutMs).toBe(1000);
|
||||
@@ -194,22 +195,24 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
expect(labelPatch?.key).toBe(child.sessionKey);
|
||||
expect(labelPatch?.label).toBe("my-task");
|
||||
|
||||
// Two agent calls: subagent spawn + main agent trigger
|
||||
// Subagent spawn call plus direct outbound completion send.
|
||||
const agentCalls = ctx.calls.filter((c) => c.method === "agent");
|
||||
expect(agentCalls).toHaveLength(2);
|
||||
expect(agentCalls).toHaveLength(1);
|
||||
|
||||
// First call: subagent spawn
|
||||
const first = agentCalls[0]?.params as { lane?: string } | undefined;
|
||||
expect(first?.lane).toBe("subagent");
|
||||
|
||||
// Second call: main agent trigger (not "Sub-agent announce step." anymore)
|
||||
const second = agentCalls[1]?.params as { sessionKey?: string; message?: string } | undefined;
|
||||
expect(second?.sessionKey).toBe("main");
|
||||
expect(second?.message).toContain("subagent task");
|
||||
|
||||
// No direct send to external channel (main agent handles delivery)
|
||||
// Direct send should route completion to the requester channel/session.
|
||||
const sendCalls = ctx.calls.filter((c) => c.method === "send");
|
||||
expect(sendCalls.length).toBe(0);
|
||||
expect(sendCalls).toHaveLength(1);
|
||||
const send = sendCalls[0]?.params as
|
||||
| { sessionKey?: string; channel?: string; to?: string; message?: string }
|
||||
| undefined;
|
||||
expect(send?.sessionKey).toBe("agent:main:main");
|
||||
expect(send?.channel).toBe("whatsapp");
|
||||
expect(send?.to).toBe("+123");
|
||||
expect(send?.message).toBe("done");
|
||||
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
});
|
||||
|
||||
@@ -232,6 +235,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "discord:group:req",
|
||||
agentChannel: "discord",
|
||||
agentTo: "discord:dm:u123",
|
||||
});
|
||||
|
||||
const result = await tool.execute("call1", {
|
||||
@@ -269,7 +273,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
expect(childWait?.timeoutMs).toBe(1000);
|
||||
|
||||
const agentCalls = ctx.calls.filter((call) => call.method === "agent");
|
||||
expect(agentCalls).toHaveLength(2);
|
||||
expect(agentCalls).toHaveLength(1);
|
||||
|
||||
const first = agentCalls[0]?.params as
|
||||
| {
|
||||
@@ -285,19 +289,15 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
expect(first?.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
|
||||
const second = agentCalls[1]?.params as
|
||||
| {
|
||||
sessionKey?: string;
|
||||
message?: string;
|
||||
deliver?: boolean;
|
||||
}
|
||||
| undefined;
|
||||
expect(second?.sessionKey).toBe("discord:group:req");
|
||||
expect(second?.deliver).toBe(true);
|
||||
expect(second?.message).toContain("subagent task");
|
||||
|
||||
const sendCalls = ctx.calls.filter((c) => c.method === "send");
|
||||
expect(sendCalls.length).toBe(0);
|
||||
expect(sendCalls).toHaveLength(1);
|
||||
const send = sendCalls[0]?.params as
|
||||
| { sessionKey?: string; channel?: string; to?: string; message?: string }
|
||||
| undefined;
|
||||
expect(send?.sessionKey).toBe("agent:main:discord:group:req");
|
||||
expect(send?.channel).toBe("discord");
|
||||
expect(send?.to).toBe("discord:dm:u123");
|
||||
expect(send?.message).toContain("completed successfully");
|
||||
|
||||
expect(deletedKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
});
|
||||
@@ -323,6 +323,7 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "discord:group:req",
|
||||
agentChannel: "discord",
|
||||
agentTo: "discord:dm:u123",
|
||||
});
|
||||
|
||||
const result = await tool.execute("call1b", {
|
||||
@@ -340,29 +341,30 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => {
|
||||
throw new Error("missing child runId");
|
||||
}
|
||||
await waitFor(() => ctx.waitCalls.some((call) => call.runId === child.runId));
|
||||
await waitFor(() => ctx.calls.filter((call) => call.method === "agent").length >= 2);
|
||||
await waitFor(() => ctx.calls.filter((call) => call.method === "send").length >= 1);
|
||||
await waitFor(() => Boolean(deletedKey));
|
||||
|
||||
const childWait = ctx.waitCalls.find((call) => call.runId === child.runId);
|
||||
expect(childWait?.timeoutMs).toBe(1000);
|
||||
expect(child.sessionKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
|
||||
// Two agent calls: subagent spawn + main agent trigger
|
||||
// One agent call for spawn, then direct completion send.
|
||||
const agentCalls = ctx.calls.filter((call) => call.method === "agent");
|
||||
expect(agentCalls).toHaveLength(2);
|
||||
expect(agentCalls).toHaveLength(1);
|
||||
|
||||
// First call: subagent spawn
|
||||
const first = agentCalls[0]?.params as { lane?: string } | undefined;
|
||||
expect(first?.lane).toBe("subagent");
|
||||
|
||||
// Second call: main agent trigger
|
||||
const second = agentCalls[1]?.params as { sessionKey?: string; deliver?: boolean } | undefined;
|
||||
expect(second?.sessionKey).toBe("discord:group:req");
|
||||
expect(second?.deliver).toBe(true);
|
||||
|
||||
// No direct send to external channel (main agent handles delivery)
|
||||
const sendCalls = ctx.calls.filter((c) => c.method === "send");
|
||||
expect(sendCalls.length).toBe(0);
|
||||
expect(sendCalls).toHaveLength(1);
|
||||
const send = sendCalls[0]?.params as
|
||||
| { sessionKey?: string; channel?: string; to?: string; message?: string }
|
||||
| undefined;
|
||||
expect(send?.sessionKey).toBe("agent:main:discord:group:req");
|
||||
expect(send?.channel).toBe("discord");
|
||||
expect(send?.to).toBe("discord:dm:u123");
|
||||
expect(send?.message).toBe("done");
|
||||
|
||||
// Session should be deleted
|
||||
expect(deletedKey?.startsWith("agent:main:subagent:")).toBe(true);
|
||||
|
||||
@@ -542,7 +542,7 @@ describe("subagent announce formatting", () => {
|
||||
queueDebounceMs: 0,
|
||||
},
|
||||
};
|
||||
agentSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
|
||||
sendSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
|
||||
|
||||
const didAnnounce = await runSubagentAnnounceFlow({
|
||||
childSessionKey: "agent:main:subagent:worker",
|
||||
@@ -554,16 +554,17 @@ describe("subagent announce formatting", () => {
|
||||
});
|
||||
|
||||
expect(didAnnounce).toBe(true);
|
||||
await expect.poll(() => agentSpy.mock.calls.length).toBe(2);
|
||||
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
|
||||
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
|
||||
expect(sendSpy.mock.calls[0]?.[0]).toMatchObject({
|
||||
method: "send",
|
||||
params: { sessionKey: "agent:main:main" },
|
||||
});
|
||||
expect(agentSpy.mock.calls[0]?.[0]).toMatchObject({
|
||||
method: "agent",
|
||||
params: { sessionKey: "agent:main:main" },
|
||||
});
|
||||
expect(agentSpy.mock.calls[1]?.[0]).toMatchObject({
|
||||
method: "agent",
|
||||
params: { sessionKey: "agent:main:main" },
|
||||
});
|
||||
expect(agentSpy.mock.calls[1]?.[0]).toMatchObject({
|
||||
expect(agentSpy.mock.calls[0]?.[0]).toMatchObject({
|
||||
method: "agent",
|
||||
params: { channel: "whatsapp", to: "+1555", deliver: true },
|
||||
});
|
||||
@@ -580,7 +581,7 @@ describe("subagent announce formatting", () => {
|
||||
lastTo: "+1555",
|
||||
},
|
||||
};
|
||||
agentSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
|
||||
sendSpy.mockRejectedValueOnce(new Error("direct delivery unavailable"));
|
||||
|
||||
const didAnnounce = await runSubagentAnnounceFlow({
|
||||
childSessionKey: "agent:main:subagent:worker",
|
||||
@@ -592,7 +593,8 @@ describe("subagent announce formatting", () => {
|
||||
});
|
||||
|
||||
expect(didAnnounce).toBe(false);
|
||||
expect(agentSpy).toHaveBeenCalledTimes(1);
|
||||
expect(sendSpy).toHaveBeenCalledTimes(1);
|
||||
expect(agentSpy).toHaveBeenCalledTimes(0);
|
||||
});
|
||||
|
||||
it("uses assistant output for completion-mode when latest assistant text exists", async () => {
|
||||
@@ -621,8 +623,8 @@ describe("subagent announce formatting", () => {
|
||||
});
|
||||
|
||||
expect(didAnnounce).toBe(true);
|
||||
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
|
||||
const call = agentSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
|
||||
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
|
||||
const call = sendSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
|
||||
const msg = call?.params?.message as string;
|
||||
expect(msg).toContain("assistant completion text");
|
||||
expect(msg).not.toContain("old tool output");
|
||||
@@ -654,8 +656,8 @@ describe("subagent announce formatting", () => {
|
||||
});
|
||||
|
||||
expect(didAnnounce).toBe(true);
|
||||
await expect.poll(() => agentSpy.mock.calls.length).toBe(1);
|
||||
const call = agentSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
|
||||
await expect.poll(() => sendSpy.mock.calls.length).toBe(1);
|
||||
const call = sendSpy.mock.calls[0]?.[0] as { params?: { message?: string } };
|
||||
const msg = call?.params?.message as string;
|
||||
expect(msg).toContain("tool output only");
|
||||
});
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
mergeDeliveryContext,
|
||||
normalizeDeliveryContext,
|
||||
} from "../utils/delivery-context.js";
|
||||
import { isDeliverableMessageChannel } from "../utils/message-channel.js";
|
||||
import {
|
||||
buildAnnounceIdFromChildRun,
|
||||
buildAnnounceIdempotencyKey,
|
||||
@@ -44,6 +45,19 @@ type SubagentAnnounceDeliveryResult = {
|
||||
error?: string;
|
||||
};
|
||||
|
||||
function buildCompletionDeliveryMessage(params: {
|
||||
findings: string;
|
||||
subagentName: string;
|
||||
}): string {
|
||||
const findingsText = params.findings.trim();
|
||||
const hasFindings = findingsText.length > 0 && findingsText !== "(no output)";
|
||||
const header = `✅ Subagent ${params.subagentName} finished`;
|
||||
if (!hasFindings) {
|
||||
return header;
|
||||
}
|
||||
return `${header}\n\n${findingsText}`;
|
||||
}
|
||||
|
||||
function summarizeDeliveryError(error: unknown): string {
|
||||
if (error instanceof Error) {
|
||||
return error.message || "error";
|
||||
@@ -256,10 +270,23 @@ function resolveAnnounceOrigin(
|
||||
entry?: DeliveryContextSource,
|
||||
requesterOrigin?: DeliveryContext,
|
||||
): DeliveryContext | undefined {
|
||||
const normalizedRequester = normalizeDeliveryContext(requesterOrigin);
|
||||
const normalizedEntry = deliveryContextFromSession(entry);
|
||||
if (normalizedRequester?.channel && !isDeliverableMessageChannel(normalizedRequester.channel)) {
|
||||
// Ignore internal/non-deliverable channel hints (for example webchat)
|
||||
// so a valid persisted route can still be used for outbound delivery.
|
||||
return mergeDeliveryContext(
|
||||
{
|
||||
accountId: normalizedRequester.accountId,
|
||||
threadId: normalizedRequester.threadId,
|
||||
},
|
||||
normalizedEntry,
|
||||
);
|
||||
}
|
||||
// requesterOrigin (captured at spawn time) reflects the channel the user is
|
||||
// actually on and must take priority over the session entry, which may carry
|
||||
// stale lastChannel / lastTo values from a previous channel interaction.
|
||||
return mergeDeliveryContext(requesterOrigin, deliveryContextFromSession(entry));
|
||||
return mergeDeliveryContext(normalizedRequester, normalizedEntry);
|
||||
}
|
||||
|
||||
async function sendAnnounce(item: AnnounceQueueItem) {
|
||||
@@ -411,24 +438,29 @@ async function sendSubagentAnnounceDirectly(params: {
|
||||
directOrigin?: DeliveryContext;
|
||||
requesterIsSubagent: boolean;
|
||||
}): Promise<SubagentAnnounceDeliveryResult> {
|
||||
const cfg = loadConfig();
|
||||
const canonicalRequesterSessionKey = resolveRequesterStoreKey(
|
||||
cfg,
|
||||
params.targetRequesterSessionKey,
|
||||
);
|
||||
try {
|
||||
const completionDirectOrigin = normalizeDeliveryContext(params.completionDirectOrigin);
|
||||
const completionChannel =
|
||||
const completionChannelRaw =
|
||||
typeof completionDirectOrigin?.channel === "string"
|
||||
? completionDirectOrigin.channel.trim()
|
||||
: "";
|
||||
const completionChannel =
|
||||
completionChannelRaw && isDeliverableMessageChannel(completionChannelRaw)
|
||||
? completionChannelRaw
|
||||
: "";
|
||||
const completionTo =
|
||||
typeof completionDirectOrigin?.to === "string" ? completionDirectOrigin.to.trim() : "";
|
||||
const completionHasThreadHint =
|
||||
completionDirectOrigin?.threadId != null &&
|
||||
String(completionDirectOrigin.threadId).trim() !== "";
|
||||
const hasCompletionDirectTarget =
|
||||
!params.requesterIsSubagent && Boolean(completionChannel) && Boolean(completionTo);
|
||||
|
||||
if (
|
||||
params.expectsCompletionMessage &&
|
||||
hasCompletionDirectTarget &&
|
||||
!completionHasThreadHint &&
|
||||
params.completionMessage?.trim()
|
||||
) {
|
||||
await callGateway({
|
||||
@@ -437,7 +469,7 @@ async function sendSubagentAnnounceDirectly(params: {
|
||||
channel: completionChannel,
|
||||
to: completionTo,
|
||||
accountId: completionDirectOrigin?.accountId,
|
||||
sessionKey: params.targetRequesterSessionKey,
|
||||
sessionKey: canonicalRequesterSessionKey,
|
||||
message: params.completionMessage,
|
||||
idempotencyKey: params.directIdempotencyKey,
|
||||
},
|
||||
@@ -455,11 +487,10 @@ async function sendSubagentAnnounceDirectly(params: {
|
||||
directOrigin?.threadId != null && directOrigin.threadId !== ""
|
||||
? String(directOrigin.threadId)
|
||||
: undefined;
|
||||
|
||||
await callGateway({
|
||||
method: "agent",
|
||||
params: {
|
||||
sessionKey: params.targetRequesterSessionKey,
|
||||
sessionKey: canonicalRequesterSessionKey,
|
||||
message: params.triggerMessage,
|
||||
deliver: !params.requesterIsSubagent,
|
||||
channel: params.requesterIsSubagent ? undefined : directOrigin?.channel,
|
||||
@@ -521,11 +552,11 @@ async function deliverSubagentAnnouncement(params: {
|
||||
targetRequesterSessionKey: params.targetRequesterSessionKey,
|
||||
triggerMessage: params.triggerMessage,
|
||||
completionMessage: params.completionMessage,
|
||||
expectsCompletionMessage: params.expectsCompletionMessage,
|
||||
directIdempotencyKey: params.directIdempotencyKey,
|
||||
completionDirectOrigin: params.completionDirectOrigin,
|
||||
directOrigin: params.directOrigin,
|
||||
requesterIsSubagent: params.requesterIsSubagent,
|
||||
expectsCompletionMessage: params.expectsCompletionMessage,
|
||||
});
|
||||
if (direct.delivered || !params.expectsCompletionMessage) {
|
||||
return direct;
|
||||
@@ -806,6 +837,7 @@ export async function runSubagentAnnounceFlow(params: {
|
||||
// Build instructional message for main agent
|
||||
const announceType = params.announceType ?? "subagent task";
|
||||
const taskLabel = params.label || params.task || "task";
|
||||
const subagentName = resolveAgentIdFromSessionKey(params.childSessionKey);
|
||||
const announceSessionId = childSessionId || "unknown";
|
||||
const findings = reply || "(no output)";
|
||||
let completionMessage = "";
|
||||
@@ -872,7 +904,11 @@ export async function runSubagentAnnounceFlow(params: {
|
||||
startedAt: params.startedAt,
|
||||
endedAt: params.endedAt,
|
||||
});
|
||||
completionMessage = [
|
||||
completionMessage = buildCompletionDeliveryMessage({
|
||||
findings,
|
||||
subagentName,
|
||||
});
|
||||
const internalSummaryMessage = [
|
||||
`[System Message] [sessionId: ${announceSessionId}] A ${announceType} "${taskLabel}" just ${statusLabel}.`,
|
||||
"",
|
||||
"Result:",
|
||||
@@ -880,7 +916,7 @@ export async function runSubagentAnnounceFlow(params: {
|
||||
"",
|
||||
statsLine,
|
||||
].join("\n");
|
||||
triggerMessage = [completionMessage, "", replyInstruction].join("\n");
|
||||
triggerMessage = [internalSummaryMessage, "", replyInstruction].join("\n");
|
||||
|
||||
const announceId = buildAnnounceIdFromChildRun({
|
||||
childSessionKey: params.childSessionKey,
|
||||
|
||||
@@ -41,7 +41,7 @@ export type SpawnSubagentContext = {
|
||||
};
|
||||
|
||||
export const SUBAGENT_SPAWN_ACCEPTED_NOTE =
|
||||
"auto-announces on completion, do not poll/sleep. The response will be sent back as a user message.";
|
||||
"auto-announces on completion, do not poll/sleep. The response will be sent back as an agent message.";
|
||||
|
||||
export type SpawnSubagentResult = {
|
||||
status: "accepted" | "forbidden" | "error";
|
||||
|
||||
@@ -686,7 +686,9 @@ export const handleSubagentsCommand: CommandHandler = async (params, allowTextCo
|
||||
const originatingTo =
|
||||
typeof params.ctx.OriginatingTo === "string" ? params.ctx.OriginatingTo.trim() : "";
|
||||
const fallbackTo = typeof params.ctx.To === "string" ? params.ctx.To.trim() : "";
|
||||
const normalizedTo = commandTo || originatingTo || fallbackTo || undefined;
|
||||
// OriginatingTo reflects the active conversation target and is safer than
|
||||
// command.to for cross-surface command dispatch.
|
||||
const normalizedTo = originatingTo || commandTo || fallbackTo || undefined;
|
||||
|
||||
const result = await spawnSubagentDirect(
|
||||
{ task, agentId, model, thinking, cleanup: "keep", expectsCompletionMessage: true },
|
||||
|
||||
Reference in New Issue
Block a user