mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix(subagents): avoid duplicate parent send replies
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<SessionEntry, "acp" | "parentSessionKey" | "spawnedBy">;
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user