From 8daf0124c93e063d2bd785c86fe227e9753aa401 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 2 May 2026 07:40:59 +0100 Subject: [PATCH] fix(subagents): avoid duplicate parent send replies --- CHANGELOG.md | 1 + src/agents/openclaw-tools.sessions.test.ts | 85 ++++++++++++++++++++++ src/agents/tools/sessions-send-tool.ts | 54 +++++++++++--- 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92609414de4..6de79545943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai - Plugins/web-provider: reuse the active gateway plugin registry for runtime web provider resolution after deriving the same candidate plugin ids as the loader path, avoiding a redundant `loadOpenClawPlugins` call on every request while preserving origin and scope filters. Fixes #75513. Thanks @jochen. - Crestodian/CLI: exit non-zero when interactive Crestodian is invoked without a TTY, so scripts and CI no longer treat the setup error as success. Fixes #73646 and supersedes #73928 and #74059. Thanks @bittoby, @luyao618, and @Linux2010. - Cron: keep implicit/default isolated cron announce deliveries out of the main session awareness queue, so isolated jobs do not accumulate in the main conversation. Fixes #61426. Thanks @Lihannon. +- Subagents: avoid duplicate parent-visible replies when a parent uses `sessions_send` on its own persistent native subagent session, while preserving announce delivery for async sends. Fixes #73550. Thanks @sylviazhang2006-design. - Agents/sandbox: preserve existing workspace file modes when sandbox edits atomically replace files, so 0644 files do not collapse to 0600 after Write/Edit/apply_patch. Fixes #44077. Thanks @patosullivan. - Agents/models: keep legacy CLI runtime model refs such as `claude-cli/*` in the configured allowlist after canonical runtime migration, so cron `payload.model` overrides keep working. Fixes #75753. Thanks @RyanSandoval. - Codex/app-server: restart the shared Codex app-server client once when it closes during startup thread resume, preserving the existing thread binding instead of retrying `thread/start` on a closed client. Thanks @vincentkoc. diff --git a/src/agents/openclaw-tools.sessions.test.ts b/src/agents/openclaw-tools.sessions.test.ts index 5e226c250b2..9428273f608 100644 --- a/src/agents/openclaw-tools.sessions.test.ts +++ b/src/agents/openclaw-tools.sessions.test.ts @@ -10,6 +10,10 @@ const callGatewayMock = vi.fn(); vi.mock("../gateway/call.js", () => ({ callGateway: (opts: unknown) => callGatewayMock(opts), })); +const loadSessionEntryByKeyMock = vi.fn(); +vi.mock("./subagent-announce-delivery.js", () => ({ + loadSessionEntryByKey: (sessionKey: string) => loadSessionEntryByKeyMock(sessionKey), +})); vi.mock("../config/config.js", () => ({ getRuntimeConfig: () => ({ @@ -156,6 +160,8 @@ const waitForCalls = async (getCount: () => number, count: number, timeoutMs = 2 describe("sessions tools", () => { beforeEach(() => { callGatewayMock.mockClear(); + loadSessionEntryByKeyMock.mockReset(); + loadSessionEntryByKeyMock.mockReturnValue(undefined); installMessagingTestRegistry(); agentStepTesting.setDepsForTest({ agentCommandFromIngress: async () => ({ @@ -1092,6 +1098,85 @@ describe("sessions tools", () => { }); }); + it("sessions_send skips duplicate A2A delivery for waited parent-owned native subagents", async () => { + const calls: Array<{ method?: string; params?: unknown }> = []; + const requesterKey = "agent:main:discord:direct:parent"; + const targetKey = "agent:main:subagent:child"; + let historyCallCount = 0; + loadSessionEntryByKeyMock.mockImplementation((sessionKey: string) => + sessionKey === targetKey + ? { + sessionId: "child-session", + updatedAt: 1, + spawnedBy: requesterKey, + deliveryContext: { + channel: "discord", + to: "direct:parent", + }, + } + : undefined, + ); + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string; params?: unknown }; + calls.push(request); + if (request.method === "agent") { + return { runId: "run-child", status: "accepted", acceptedAt: 2000 }; + } + if (request.method === "agent.wait") { + return { runId: "run-child", status: "ok" }; + } + if (request.method === "chat.history") { + historyCallCount += 1; + return { + messages: + historyCallCount === 1 + ? [] + : [ + { + role: "assistant", + content: [{ type: "text", text: "child reply" }], + timestamp: 20, + }, + ], + }; + } + return {}; + }); + + const tool = createOpenClawTools({ + agentSessionKey: requesterKey, + agentChannel: "discord", + }).find((candidate) => candidate.name === "sessions_send"); + expect(tool).toBeDefined(); + if (!tool) { + throw new Error("missing sessions_send tool"); + } + + const waited = await tool.execute("call-parent-owned-native-subagent", { + sessionKey: targetKey, + message: "ping", + timeoutSeconds: 1, + }); + + expect(waited.details).toMatchObject({ + status: "ok", + reply: "child reply", + delivery: { status: "skipped", mode: "announce" }, + }); + expect(calls.filter((call) => call.method === "agent")).toHaveLength(1); + expect( + calls.some( + (call) => + call.method === "agent" && + typeof (call.params as { extraSystemPrompt?: string })?.extraSystemPrompt === "string" && + (call.params as { extraSystemPrompt?: string }).extraSystemPrompt?.includes( + "Agent-to-agent reply step", + ), + ), + ).toBe(false); + expect(calls.some((call) => call.method === "send")).toBe(false); + }); + it("sessions_send preserves threadId when announce target is hydrated via sessions.list", async () => { const calls: Array<{ method?: string; params?: unknown }> = []; let agentCallCount = 0; diff --git a/src/agents/tools/sessions-send-tool.ts b/src/agents/tools/sessions-send-tool.ts index c03c34ec2e4..90fcaa93baf 100644 --- a/src/agents/tools/sessions-send-tool.ts +++ b/src/agents/tools/sessions-send-tool.ts @@ -2,10 +2,15 @@ import crypto from "node:crypto"; import { Type } from "typebox"; import { isRequesterParentOfBackgroundAcpSession } from "../../acp/session-interaction-mode.js"; import { parseSessionThreadInfoFast } from "../../config/sessions/thread-info.js"; +import type { SessionEntry } from "../../config/sessions/types.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { callGateway } from "../../gateway/call.js"; import { formatErrorMessage } from "../../infra/errors.js"; -import { normalizeAgentId, resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; +import { + isSubagentSessionKey, + normalizeAgentId, + resolveAgentIdFromSessionKey, +} from "../../routing/session-key.js"; import { annotateInterSessionPromptText } from "../../sessions/input-provenance.js"; import { SESSION_LABEL_MAX_LENGTH } from "../../sessions/session-label.js"; import { normalizeOptionalString } from "../../shared/string-coerce.js"; @@ -47,6 +52,25 @@ const SessionsSendToolSchema = Type.Object({ type GatewayCaller = typeof callGateway; const SESSIONS_SEND_REPLY_HISTORY_LIMIT = 50; +type SessionsSendRouteEntry = Pick; + +function isRequesterParentOfNativeSubagentSession(params: { + entry: SessionsSendRouteEntry | null | undefined; + requesterSessionKey: string | null | undefined; + targetSessionKey: string; +}): boolean { + if (!params.entry || params.entry.acp || !isSubagentSessionKey(params.targetSessionKey)) { + return false; + } + const requester = normalizeOptionalString(params.requesterSessionKey); + if (!requester) { + return false; + } + const spawnedBy = normalizeOptionalString(params.entry.spawnedBy); + const parentSessionKey = normalizeOptionalString(params.entry.parentSessionKey); + return requester === spawnedBy || requester === parentSessionKey; +} + async function startAgentRun(params: { callGateway: GatewayCaller; runId: string; @@ -304,24 +328,34 @@ export function createSessionsSendTool(opts?: { const maxPingPongTurns = resolvePingPongTurns(cfg); // Skip the A2A ping-pong + announce flow when the current caller is the - // parent of a parent-owned background ACP subagent it spawned itself. - // Such sessions already report their results back to the parent through - // the `[Internal task completion event]` announcement path, and treating - // them as a peer agent causes the parent to be woken with the child's - // reply, generate a user-facing response, and have that response - // forwarded back to the child as a new message — producing a - // ping-pong loop between parent and ACP child (bounded by - // maxPingPongTurns, but user-visible as a runaway conversation). + // parent of a parent-owned child session it spawned itself and another + // parent-visible result path already exists. + // + // ACP background sessions report through the internal task completion + // path. Waited native subagent sends return the child reply inline. In + // both cases treating the child as a peer agent wakes the parent with + // the child's reply, can generate another user-facing response, and can + // forward that response back to the child as a new message — producing a + // ping-pong loop (bounded by maxPingPongTurns, but visible as duplicate + // conversation output). // // The skip is gated on requester ownership, not just target type: an // unrelated sender that can see the same target (e.g. under // `tools.sessions.visibility=all`) must still go through the normal A2A // path so it actually receives a follow-up delivery. const targetSessionEntry = loadSessionEntryByKey(resolvedKey); - const skipA2AFlow = isRequesterParentOfBackgroundAcpSession( + const skipAcpA2AFlow = isRequesterParentOfBackgroundAcpSession( targetSessionEntry, effectiveRequesterKey, ); + const skipNativeParentA2AFlow = + timeoutSeconds !== 0 && + isRequesterParentOfNativeSubagentSession({ + entry: targetSessionEntry, + requesterSessionKey: effectiveRequesterKey, + targetSessionKey: resolvedKey, + }); + const skipA2AFlow = skipAcpA2AFlow || skipNativeParentA2AFlow; // When the A2A flow is skipped, no follow-up announcement will fire and // the reply (when present) is returned inline via the `reply` field. // Reflect that in the metadata so the parent LLM does not wait for a