From 61deb90e24da6f24a1b48059ea96e2e664286ea7 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 17 Apr 2026 11:22:33 -0400 Subject: [PATCH] agents: share target-bound spawn origins --- src/agents/acp-spawn.test.ts | 110 +++++++++++++++++ src/agents/acp-spawn.ts | 47 ++++---- src/agents/spawn-requester-origin.ts | 103 ++++++++++++++++ .../subagent-spawn.thread-binding.test.ts | 111 ++++++++++++++++++ src/agents/subagent-spawn.ts | 111 +----------------- 5 files changed, 351 insertions(+), 131 deletions(-) create mode 100644 src/agents/spawn-requester-origin.ts diff --git a/src/agents/acp-spawn.test.ts b/src/agents/acp-spawn.test.ts index 9e090397170..6bdb7c00ee9 100644 --- a/src/agents/acp-spawn.test.ts +++ b/src/agents/acp-spawn.test.ts @@ -1006,6 +1006,116 @@ describe("spawnAcpDirect", () => { expect(findAgentGatewayCall()?.params?.accountId).toBe("work"); }); + it("uses the target agent's bound account for cross-agent ACP thread spawns", async () => { + const boundRoom = "!room:example.org"; + replaceSpawnConfig({ + ...hoisted.state.cfg, + acp: { + ...hoisted.state.cfg.acp, + allowedAgents: ["codex", "bot-alpha"], + }, + channels: { + ...hoisted.state.cfg.channels, + matrix: { + threadBindings: { + enabled: true, + spawnAcpSessions: true, + }, + accounts: { + "bot-alpha": { + threadBindings: { + enabled: true, + spawnAcpSessions: true, + }, + }, + }, + }, + }, + bindings: [ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { + kind: "channel", + id: boundRoom, + }, + accountId: "bot-alpha", + }, + }, + ], + }); + registerSessionBindingAdapter({ + channel: "matrix", + accountId: "bot-alpha", + capabilities: createSessionBindingCapabilities(), + bind: async (input) => await hoisted.sessionBindingBindMock(input), + listBySession: (targetSessionKey) => + hoisted.sessionBindingListBySessionMock(targetSessionKey), + resolveByConversation: (ref) => hoisted.sessionBindingResolveByConversationMock(ref), + unbind: async (input) => await hoisted.sessionBindingUnbindMock(input), + }); + hoisted.sessionBindingBindMock.mockImplementationOnce( + async (input: { + targetSessionKey: string; + conversation: { + accountId: string; + conversationId: string; + parentConversationId?: string; + }; + metadata?: Record; + }) => + createSessionBinding({ + targetSessionKey: input.targetSessionKey, + conversation: { + channel: "matrix", + accountId: input.conversation.accountId, + conversationId: input.conversation.conversationId, + parentConversationId: input.conversation.parentConversationId, + }, + metadata: { + boundBy: + typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "system", + agentId: "bot-alpha", + }, + }), + ); + + const result = await spawnAcpDirect( + { + task: "Investigate flaky tests", + agentId: "bot-alpha", + mode: "session", + thread: true, + }, + { + agentSessionKey: "agent:main:matrix:room:requester", + agentChannel: "matrix", + agentAccountId: "bot-beta", + agentTo: `room:${boundRoom}`, + }, + ); + + expect(result.status).toBe("accepted"); + expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith( + expect.objectContaining({ + placement: "child", + conversation: expect.objectContaining({ + channel: "matrix", + accountId: "bot-alpha", + conversationId: boundRoom, + }), + }), + ); + expect(findAgentGatewayCall()?.params).toMatchObject({ + deliver: true, + channel: "matrix", + accountId: "bot-alpha", + to: `room:${boundRoom}`, + }); + }); + it.each([ { name: "canonical line target", diff --git a/src/agents/acp-spawn.ts b/src/agents/acp-spawn.ts index 4368139d0c1..4fc829b2473 100644 --- a/src/agents/acp-spawn.ts +++ b/src/agents/acp-spawn.ts @@ -73,6 +73,7 @@ import { } from "./acp-spawn-parent-stream.js"; import { resolveAgentConfig, resolveDefaultAgentId } from "./agent-scope.js"; import { resolveSandboxRuntimeStatus } from "./sandbox/runtime-status.js"; +import { resolveRequesterOriginForChild } from "./spawn-requester-origin.js"; import { resolveSpawnedWorkspaceInheritance } from "./spawned-context.js"; import { resolveInternalSessionKey, resolveMainSessionAlias } from "./tools/sessions-helpers.js"; @@ -716,6 +717,7 @@ function prepareAcpThreadBinding(params: { function resolveAcpSpawnRequesterState(params: { cfg: OpenClawConfig; parentSessionKey?: string; + targetAgentId: string; ctx: SpawnAcpContext; }): AcpSpawnRequesterState { const bindingService = getSessionBindingService(); @@ -751,11 +753,14 @@ function resolveAcpSpawnRequesterState(params: { requesterAgentId, }) : false, - origin: normalizeDeliveryContext({ - channel: params.ctx.agentChannel, - accountId: params.ctx.agentAccountId, - to: params.ctx.agentTo, - threadId: params.ctx.agentThreadId, + origin: resolveRequesterOriginForChild({ + cfg: params.cfg, + targetAgentId: params.targetAgentId, + requesterAgentId: normalizeAgentId(requesterAgentId), + requesterChannel: params.ctx.agentChannel, + requesterAccountId: params.ctx.agentAccountId, + requesterTo: params.ctx.agentTo, + requesterThreadId: params.ctx.agentThreadId, }), }; } @@ -1040,18 +1045,6 @@ export async function spawnAcpDirect( }); } - const requesterState = resolveAcpSpawnRequesterState({ - cfg, - parentSessionKey, - ctx, - }); - const { effectiveStreamToParent } = resolveAcpSpawnStreamPlan({ - spawnMode, - requestThreadBinding, - streamToParentRequested, - requester: requesterState, - }); - const targetAgentResult = resolveTargetAcpAgentId({ requestedAgentId: params.agentId, cfg, @@ -1072,6 +1065,18 @@ export async function spawnAcpDirect( error: agentPolicyError.message, }); } + const requesterState = resolveAcpSpawnRequesterState({ + cfg, + parentSessionKey, + targetAgentId, + ctx, + }); + const { effectiveStreamToParent } = resolveAcpSpawnStreamPlan({ + spawnMode, + requestThreadBinding, + streamToParentRequested, + requester: requesterState, + }); const sessionKey = `agent:${targetAgentId}:acp:${crypto.randomUUID()}`; const runtimeMode = resolveAcpSessionMode(spawnMode); @@ -1099,10 +1104,10 @@ export async function spawnAcpDirect( if (requestThreadBinding) { const prepared = prepareAcpThreadBinding({ cfg, - channel: ctx.agentChannel, - accountId: ctx.agentAccountId, - to: ctx.agentTo, - threadId: ctx.agentThreadId, + channel: requesterState.origin?.channel, + accountId: requesterState.origin?.accountId, + to: requesterState.origin?.to, + threadId: requesterState.origin?.threadId, groupId: ctx.agentGroupId, }); if (!prepared.ok) { diff --git a/src/agents/spawn-requester-origin.ts b/src/agents/spawn-requester-origin.ts new file mode 100644 index 00000000000..27d0c910ba4 --- /dev/null +++ b/src/agents/spawn-requester-origin.ts @@ -0,0 +1,103 @@ +import type { ChatType } from "../channels/chat-type.js"; +import { getChannelPlugin } from "../channels/plugins/registry.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { resolveFirstBoundAccountId } from "../routing/bound-account-read.js"; +import { normalizeDeliveryContext } from "../utils/delivery-context.js"; + +// Delivery targets carry a channel-side prefix (e.g. Matrix uses +// `room:`; LINE uses `line:group:`), but route bindings store raw +// peer ids on `match.peer.id`. Peel namespace and kind prefixes so the raw peer +// id surfaces for binding lookup. +const KIND_PREFIX_TO_CHAT_TYPE: Readonly> = { + "room:": "channel", + "channel:": "channel", + "conversation:": "channel", + "chat:": "channel", + "thread:": "channel", + "topic:": "channel", + "group:": "group", + "team:": "group", + "user:": "direct", + "dm:": "direct", + "pm:": "direct", +}; + +// Matches any `:` prefix. Real-world peer ids (Matrix `!`/`@`, +// IRC `#`, Slack/Discord/LINE alphanumerics, numeric Telegram/WhatsApp, or +// email-style `user@server`) never start with a lowercase-alpha token followed +// by `:`, so this peels prefixes without risking the raw id itself. +const GENERIC_PREFIX_PATTERN = /^[a-z][a-z0-9_-]*:/i; + +export function extractRequesterPeer( + channelId: string | undefined, + requesterTo: string | undefined, +): { peerId?: string; peerKind?: ChatType } { + if (!requesterTo) { + return {}; + } + const raw = requesterTo.trim(); + if (!raw) { + return {}; + } + let inferredKind: ChatType | undefined; + if (channelId) { + const plugin = getChannelPlugin(channelId); + inferredKind = plugin?.messaging?.inferTargetChatType?.({ to: raw }) ?? undefined; + } + let value = raw; + while (true) { + const match = GENERIC_PREFIX_PATTERN.exec(value); + if (!match) { + break; + } + const prefix = match[0].toLowerCase(); + if (prefix in KIND_PREFIX_TO_CHAT_TYPE) { + inferredKind ??= KIND_PREFIX_TO_CHAT_TYPE[prefix]; + } + value = value.slice(prefix.length).trim(); + } + if (value) { + // Id-embedded kind markers (Matrix `!`/`@`, IRC `#`) are authoritative + // because channel wrappers can wrap either room or user ids. + if (value.startsWith("@")) { + inferredKind = "direct"; + } else if (value.startsWith("!") || value.startsWith("#")) { + inferredKind = "channel"; + } + } + return { peerId: value || undefined, peerKind: inferredKind }; +} + +export function resolveRequesterOriginForChild(params: { + cfg: OpenClawConfig; + targetAgentId: string; + requesterAgentId: string; + requesterChannel?: string; + requesterAccountId?: string; + requesterTo?: string; + requesterThreadId?: string | number; +}) { + const { peerId: normalizedPeerId, peerKind: inferredPeerKind } = extractRequesterPeer( + params.requesterChannel, + params.requesterTo, + ); + // Same-agent spawns must keep the caller's active inbound account, not + // re-resolve via bindings that may select a different account for the same + // agent/channel. + const boundAccountId = + params.requesterChannel && params.targetAgentId !== params.requesterAgentId + ? resolveFirstBoundAccountId({ + cfg: params.cfg, + channelId: params.requesterChannel, + agentId: params.targetAgentId, + peerId: normalizedPeerId, + peerKind: inferredPeerKind, + }) + : undefined; + return normalizeDeliveryContext({ + channel: params.requesterChannel, + accountId: boundAccountId ?? params.requesterAccountId, + to: params.requesterTo, + threadId: params.requesterThreadId, + }); +} diff --git a/src/agents/subagent-spawn.thread-binding.test.ts b/src/agents/subagent-spawn.thread-binding.test.ts index 21c07c7db31..f03274e78f2 100644 --- a/src/agents/subagent-spawn.thread-binding.test.ts +++ b/src/agents/subagent-spawn.thread-binding.test.ts @@ -31,6 +31,117 @@ describe("spawnSubagentDirect thread binding delivery", () => { installSessionStoreCaptureMock(hoisted.updateSessionStoreMock); }); + it("passes the target agent's bound account to thread binding hooks", async () => { + const boundRoom = "!room:example.org"; + let hookRequester: + | { channel?: string; accountId?: string; to?: string; threadId?: string | number } + | undefined; + hoisted.hookRunner.hasHooks.mockImplementation( + (hookName?: string) => hookName === "subagent_spawning", + ); + hoisted.hookRunner.runSubagentSpawning.mockImplementation(async (event: unknown) => { + hookRequester = ( + event as { + requester?: { + channel?: string; + accountId?: string; + to?: string; + threadId?: string | number; + }; + } + ).requester; + return { + status: "ok", + threadBindingReady: true, + deliveryOrigin: { + channel: "matrix", + to: `room:${boundRoom}`, + threadId: "$thread-root", + }, + }; + }); + const { spawnSubagentDirect } = await loadSubagentSpawnModuleForTest({ + callGatewayMock: hoisted.callGatewayMock, + loadConfig: () => + createSubagentSpawnTestConfig(os.tmpdir(), { + agents: { + defaults: { + workspace: os.tmpdir(), + subagents: { + allowAgents: ["bot-alpha"], + }, + }, + list: [ + { id: "main", workspace: "/tmp/workspace-main" }, + { id: "bot-alpha", workspace: "/tmp/workspace-bot-alpha" }, + ], + }, + bindings: [ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { + kind: "channel", + id: boundRoom, + }, + accountId: "bot-alpha", + }, + }, + ], + }), + updateSessionStoreMock: hoisted.updateSessionStoreMock, + registerSubagentRunMock: hoisted.registerSubagentRunMock, + emitSessionLifecycleEventMock: hoisted.emitSessionLifecycleEventMock, + hookRunner: hoisted.hookRunner, + resolveSubagentSpawnModelSelection: () => "openai-codex/gpt-5.4", + resolveSandboxRuntimeStatus: () => ({ sandboxed: false }), + }); + + const result = await spawnSubagentDirect( + { + task: "reply with a marker", + agentId: "bot-alpha", + thread: true, + mode: "session", + }, + { + agentSessionKey: "agent:main:main", + agentChannel: "matrix", + agentAccountId: "bot-beta", + agentTo: `room:${boundRoom}`, + }, + ); + + expect(result.status).toBe("accepted"); + expect(hookRequester).toMatchObject({ + channel: "matrix", + accountId: "bot-alpha", + to: `room:${boundRoom}`, + }); + const agentCall = hoisted.callGatewayMock.mock.calls.find( + ([call]) => (call as { method?: string }).method === "agent", + )?.[0] as { params?: Record } | undefined; + expect(agentCall?.params).toMatchObject({ + channel: "matrix", + accountId: "bot-alpha", + to: `room:${boundRoom}`, + threadId: "$thread-root", + deliver: true, + }); + expect(hoisted.registerSubagentRunMock).toHaveBeenCalledWith( + expect.objectContaining({ + requesterOrigin: { + channel: "matrix", + accountId: "bot-alpha", + to: `room:${boundRoom}`, + threadId: "$thread-root", + }, + }), + ); + }); + it("seeds a thread-bound child session from the binding created during spawn", async () => { hoisted.hookRunner.hasHooks.mockImplementation( (hookName?: string) => hookName === "subagent_spawning", diff --git a/src/agents/subagent-spawn.ts b/src/agents/subagent-spawn.ts index 8501f12dee7..b4e147c3496 100644 --- a/src/agents/subagent-spawn.ts +++ b/src/agents/subagent-spawn.ts @@ -1,10 +1,7 @@ import crypto from "node:crypto"; import { promises as fs } from "node:fs"; -import type { ChatType } from "../channels/chat-type.js"; -import { getChannelPlugin } from "../channels/plugins/registry.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { SubagentLifecycleHookRunner } from "../plugins/hooks.js"; -import { resolveFirstBoundAccountId } from "../routing/bound-account-read.js"; import { isValidAgentId, normalizeAgentId, parseAgentSessionKey } from "../routing/session-key.js"; import { normalizeLowercaseStringOrEmpty, @@ -30,6 +27,7 @@ export { SUBAGENT_SPAWN_ACCEPTED_NOTE, SUBAGENT_SPAWN_SESSION_ACCEPTED_NOTE, } from "./subagent-spawn-accepted-note.js"; +import { resolveRequesterOriginForChild } from "./spawn-requester-origin.js"; import { resolveConfiguredSubagentRunTimeoutSeconds, resolveSubagentModelAndThinkingPlan, @@ -287,113 +285,6 @@ function summarizeError(err: unknown): string { return "error"; } -// Delivery targets carry a channel-side prefix (e.g. Matrix uses `room:`; -// LINE uses `line:group:`), but route bindings store raw peer ids on -// `match.peer.id`. Peel the `:` namespace first, then loop over generic -// target-kind prefixes so the raw peer id surfaces. Each kind prefix also -// implies a ChatType — we capture it as a fallback when the channel plugin does -// not implement `inferTargetChatType`, and as the authoritative source when the -// target shape is `::`. -const KIND_PREFIX_TO_CHAT_TYPE: Readonly> = { - "room:": "channel", - "channel:": "channel", - "conversation:": "channel", - "chat:": "channel", - "thread:": "channel", - "topic:": "channel", - "group:": "group", - "team:": "group", - "user:": "direct", - "dm:": "direct", - "pm:": "direct", -}; - -// Matches any `:` prefix. Real-world peer ids (Matrix `!`/`@`, -// IRC `#`, Slack/Discord/LINE alphanumerics, numeric Telegram/WhatsApp, or -// email-style `user@server`) never start with a lowercase-alpha token followed -// by `:`, so this cleanly peels namespace and kind prefixes without risking -// the raw id itself. When a peeled token maps to a known ChatType, we also -// record it as an inferred peerKind. -const GENERIC_PREFIX_PATTERN = /^[a-z][a-z0-9_-]*:/i; - -function extractRequesterPeer( - channelId: string | undefined, - requesterTo: string | undefined, -): { peerId?: string; peerKind?: ChatType } { - if (!requesterTo) { - return {}; - } - const raw = requesterTo.trim(); - if (!raw) { - return {}; - } - let inferredKind: ChatType | undefined; - if (channelId) { - const plugin = getChannelPlugin(channelId); - inferredKind = plugin?.messaging?.inferTargetChatType?.({ to: raw }) ?? undefined; - } - let value = raw; - while (true) { - const match = GENERIC_PREFIX_PATTERN.exec(value); - if (!match) { - break; - } - const prefix = match[0].toLowerCase(); - if (prefix in KIND_PREFIX_TO_CHAT_TYPE) { - inferredKind ??= KIND_PREFIX_TO_CHAT_TYPE[prefix]; - } - value = value.slice(prefix.length).trim(); - } - if (value) { - // Id-embedded kind markers (Matrix `!`/`@`, IRC `#`) win over prefix-derived - // inference — channel-side wrappers can wrap either a room or a user id - // (e.g. Matrix thread delivery encodes per-user DM targets as - // `room:@user:server`), and the id itself is the authoritative signal for - // what the peer actually is. - if (value.startsWith("@")) { - inferredKind = "direct"; - } else if (value.startsWith("!") || value.startsWith("#")) { - inferredKind = "channel"; - } - } - return { peerId: value || undefined, peerKind: inferredKind }; -} - -function resolveRequesterOriginForChild(params: { - cfg: OpenClawConfig; - targetAgentId: string; - requesterAgentId: string; - requesterChannel?: string; - requesterAccountId?: string; - requesterTo?: string; - requesterThreadId?: string | number; -}) { - // Same-agent spawns (a child of the same agent) must keep the caller's active - // inbound account, not re-resolve via bindings — the caller is already acting - // as that agent with a specific account, and a lookup could pick a different - // binding when the same agent has multiple accounts configured. - const { peerId: normalizedPeerId, peerKind: inferredPeerKind } = extractRequesterPeer( - params.requesterChannel, - params.requesterTo, - ); - const boundAccountId = - params.requesterChannel && params.targetAgentId !== params.requesterAgentId - ? resolveFirstBoundAccountId({ - cfg: params.cfg, - channelId: params.requesterChannel, - agentId: params.targetAgentId, - peerId: normalizedPeerId, - peerKind: inferredPeerKind, - }) - : undefined; - return normalizeDeliveryContext({ - channel: params.requesterChannel, - accountId: boundAccountId ?? params.requesterAccountId, - to: params.requesterTo, - threadId: params.requesterThreadId, - }); -} - async function ensureThreadBindingForSubagentSpawn(params: { hookRunner: SubagentLifecycleHookRunner | null; childSessionKey: string;