diff --git a/src/acp/session-interaction-mode.test.ts b/src/acp/session-interaction-mode.test.ts new file mode 100644 index 00000000000..cabb9a09de8 --- /dev/null +++ b/src/acp/session-interaction-mode.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vitest"; +import { + isParentOwnedBackgroundAcpSession, + isRequesterParentOfBackgroundAcpSession, + resolveAcpSessionInteractionMode, +} from "./session-interaction-mode.js"; + +const parentKey = "agent:main:main"; +const otherKey = "agent:peer:some-other"; + +describe("resolveAcpSessionInteractionMode", () => { + it("returns interactive when entry is undefined", () => { + expect(resolveAcpSessionInteractionMode(undefined)).toBe("interactive"); + }); + + it("returns interactive for non-oneshot ACP sessions", () => { + expect( + resolveAcpSessionInteractionMode({ + acp: { mode: "persistent" } as never, + spawnedBy: parentKey, + }), + ).toBe("interactive"); + }); + + it("returns parent-owned-background for oneshot sessions with spawnedBy set", () => { + expect( + resolveAcpSessionInteractionMode({ + acp: { mode: "oneshot" } as never, + spawnedBy: parentKey, + }), + ).toBe("parent-owned-background"); + }); + + it("returns parent-owned-background for oneshot sessions with parentSessionKey set", () => { + expect( + resolveAcpSessionInteractionMode({ + acp: { mode: "oneshot" } as never, + parentSessionKey: parentKey, + }), + ).toBe("parent-owned-background"); + }); + + it("returns interactive for a oneshot session without any parent linkage", () => { + expect( + resolveAcpSessionInteractionMode({ + acp: { mode: "oneshot" } as never, + }), + ).toBe("interactive"); + }); +}); + +describe("isRequesterParentOfBackgroundAcpSession", () => { + const backgroundEntry = { + acp: { mode: "oneshot" } as never, + spawnedBy: parentKey, + parentSessionKey: parentKey, + }; + + it("returns true when requester matches spawnedBy", () => { + expect( + isRequesterParentOfBackgroundAcpSession( + { acp: { mode: "oneshot" } as never, spawnedBy: parentKey }, + parentKey, + ), + ).toBe(true); + }); + + it("returns true when requester matches parentSessionKey", () => { + expect( + isRequesterParentOfBackgroundAcpSession( + { acp: { mode: "oneshot" } as never, parentSessionKey: parentKey }, + parentKey, + ), + ).toBe(true); + }); + + it("returns false when requester is a different session (not the parent)", () => { + expect(isRequesterParentOfBackgroundAcpSession(backgroundEntry, otherKey)).toBe(false); + }); + + it("returns false when requester key is missing", () => { + expect(isRequesterParentOfBackgroundAcpSession(backgroundEntry, undefined)).toBe(false); + expect(isRequesterParentOfBackgroundAcpSession(backgroundEntry, "")).toBe(false); + }); + + it("returns false when target is not a parent-owned background ACP session", () => { + expect( + isRequesterParentOfBackgroundAcpSession( + { acp: { mode: "persistent" } as never, spawnedBy: parentKey }, + parentKey, + ), + ).toBe(false); + }); + + it("delegates to isParentOwnedBackgroundAcpSession for target-only checks", () => { + expect(isParentOwnedBackgroundAcpSession(backgroundEntry)).toBe(true); + }); +}); diff --git a/src/acp/session-interaction-mode.ts b/src/acp/session-interaction-mode.ts index 7bba754519f..10bfa061488 100644 --- a/src/acp/session-interaction-mode.ts +++ b/src/acp/session-interaction-mode.ts @@ -23,3 +23,30 @@ export function resolveAcpSessionInteractionMode( export function isParentOwnedBackgroundAcpSession(entry?: SessionInteractionEntry | null): boolean { return resolveAcpSessionInteractionMode(entry) === "parent-owned-background"; } + +/** + * Returns true when `entry` is a parent-owned background ACP session AND the + * given `requesterSessionKey` is the session that spawned/owns it. This is a + * strictly narrower check than {@link isParentOwnedBackgroundAcpSession}: the + * target must match *and* the caller must be the parent. + * + * Used to gate behaviors that only make sense for the parent↔own-child pair + * (e.g. skipping the A2A ping-pong flow in `sessions_send`), so that an + * unrelated session with broad visibility (e.g. `tools.sessions.visibility=all`) + * sending to the same target is still routed through the normal A2A path. + */ +export function isRequesterParentOfBackgroundAcpSession( + entry: SessionInteractionEntry | null | undefined, + requesterSessionKey: string | null | undefined, +): boolean { + if (!isParentOwnedBackgroundAcpSession(entry)) { + return false; + } + const requester = normalizeOptionalString(requesterSessionKey); + if (!requester) { + return false; + } + const spawnedBy = normalizeOptionalString(entry?.spawnedBy); + const parentSessionKey = normalizeOptionalString(entry?.parentSessionKey); + return requester === spawnedBy || requester === parentSessionKey; +} diff --git a/src/agents/tools/sessions-send-tool.ts b/src/agents/tools/sessions-send-tool.ts index 542c2c75f57..32bcae4f467 100644 --- a/src/agents/tools/sessions-send-tool.ts +++ b/src/agents/tools/sessions-send-tool.ts @@ -1,6 +1,6 @@ import crypto from "node:crypto"; import { Type } from "@sinclair/typebox"; -import { isParentOwnedBackgroundAcpSession } from "../../acp/session-interaction-mode.js"; +import { isRequesterParentOfBackgroundAcpSession } from "../../acp/session-interaction-mode.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { callGateway } from "../../gateway/call.js"; import { formatErrorMessage } from "../../infra/errors.js"; @@ -290,18 +290,33 @@ export function createSessionsSendTool(opts?: { const requesterSessionKey = opts?.agentSessionKey; const requesterChannel = opts?.agentChannel; const maxPingPongTurns = resolvePingPongTurns(cfg); - const delivery = { status: "pending", mode: "announce" as const }; - // Skip the A2A ping-pong + announce flow when the target is a - // parent-owned background ACP subagent. Such sessions already report - // their results back to the parent through the - // `[Internal task completion event]` announcement path, and treating + // 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 an infinite - // ping-pong loop between parent and ACP child. + // 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). + // + // 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 = isParentOwnedBackgroundAcpSession(targetSessionEntry); + const skipA2AFlow = isRequesterParentOfBackgroundAcpSession( + targetSessionEntry, + effectiveRequesterKey, + ); + // 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 + // second result that will never arrive. + const delivery = skipA2AFlow + ? ({ status: "skipped", mode: "announce" } as const) + : ({ status: "pending", mode: "announce" } as const); const startA2AFlow = (roundOneReply?: string, waitRunId?: string) => { if (skipA2AFlow) {