From 46708707f677d20ed0ee8f2d34757be38ce68ab6 Mon Sep 17 00:00:00 2001 From: Luke Boyett Date: Thu, 16 Apr 2026 05:45:31 -0400 Subject: [PATCH] fix(routing): address three review P1s for bound-account lookup 1. Preserve requester account for same-agent subagent spawns (src/agents/subagent-spawn.ts) resolveRequesterOriginForChild previously overwrote the caller's active inbound accountId with a fresh binding lookup even when the target agent equaled the requester. In multi-account channels where the same agent has multiple bindings, that could switch the child to a different account from the one the parent is actively using. Now the lookup only runs when targetAgentId differs from requesterAgentId; same-agent spawns keep the caller's accountId unchanged. 2. Skip wildcard peer matches when peerId is absent (src/routing/bound-account-read.ts) A wildcard peer binding (peer.id: "*") represents "match any peer", so it is only meaningful when the caller supplies a peer. For peerless callers (e.g. cron delivery resolution) a wildcard binding no longer beats a channel-only binding. It is instead used as a last-resort peer-ish fallback (see #3 below). 3. Preserve old first-match semantics for peerless callers with only peer-specific bindings (src/routing/bound-account-read.ts) The previous iteration skipped peer-specific bindings outright when no peerId was supplied, which silently dropped the account for operators whose only binding was peer-specific (for example cron jobs targeting a specific room). Peerless callers now fall back to the first peer-specific or wildcard binding they find (peerlessPeerSpecificFallback) after channel-only bindings, restoring the prior cron semantics without overriding explicit channel-only configuration. Final three-tier precedence summary: - peerId supplied: exact peer match > wildcard peer > channel-only. Non-matching peer-specific bindings are skipped. - peerId absent: channel-only > first peer-specific or wildcard binding found. Regression coverage: - src/routing/bound-account-read.test.ts (new): six unit tests covering exact peer match, wildcard-wins-over-channel-only for peer-present callers, channel-only-wins-over-wildcard for peerless callers, peer-specific fallback for peerless callers with no channel-only binding, skipping non-matching peer-specific bindings, and the agent-on-different-channel no-match case. - src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts: new test asserting that a same-agent sessions_spawn (no explicit agentId) preserves the caller's accountId rather than re-resolving via bindings. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...subagents.sessions-spawn.lifecycle.test.ts | 38 +++++ src/agents/subagent-spawn.ts | 4 + src/routing/bound-account-read.test.ts | 150 ++++++++++++++++++ src/routing/bound-account-read.ts | 18 ++- 4 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 src/routing/bound-account-read.test.ts diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts index 2f68940b4cd..ab1a002e36e 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts @@ -652,6 +652,44 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { expect(spawnAccountId).toBe("bot-alpha-room-a"); }); + it("sessions_spawn preserves the caller's account for same-agent subagent spawns", async () => { + let spawnAccountId: string | undefined; + const room = "!someRoom:example.org"; + setSessionsSpawnConfigOverride({ + session: { mainKey: "main", scope: "per-sender" }, + messages: { queue: { debounceMs: 0 } }, + agents: { defaults: { subagents: { allowAgents: ["bot-alpha"] } } }, + bindings: [ + { + type: "route", + agentId: "bot-alpha", + match: { channel: "matrix", accountId: "bot-alpha-default" }, + }, + ], + }); + setupSessionsSpawnGatewayMock({ + onAgentSubagentSpawn: (params) => { + const rec = params as { accountId?: string } | undefined; + spawnAccountId = rec?.accountId; + }, + }); + + const tool = await getSessionsSpawnTool({ + agentSessionKey: "agent:bot-alpha:session:main", + agentChannel: "matrix", + agentAccountId: "bot-alpha-adhoc", + agentTo: room, + }); + + // Spawn a child of the same agent (no explicit agentId → defaults to requester). + const result = await tool.execute("call-same-agent", { + task: "do thing", + cleanup: "keep", + }); + expect(result.details).toMatchObject({ status: "accepted", runId: expect.any(String) }); + expect(spawnAccountId).toBe("bot-alpha-adhoc"); + }); + it("sessions_spawn announces with requester accountId", async () => { const ctx = setupSessionsSpawnGatewayMock({}); diff --git a/src/agents/subagent-spawn.ts b/src/agents/subagent-spawn.ts index 382bb78f8e7..210e159b8f2 100644 --- a/src/agents/subagent-spawn.ts +++ b/src/agents/subagent-spawn.ts @@ -350,6 +350,10 @@ function resolveRequesterOriginForChild(params: { 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, diff --git a/src/routing/bound-account-read.test.ts b/src/routing/bound-account-read.test.ts new file mode 100644 index 00000000000..5e768cfef23 --- /dev/null +++ b/src/routing/bound-account-read.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it } from "vitest"; +import type { AgentRouteBinding } from "../config/types.agents.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { resolveFirstBoundAccountId } from "./bound-account-read.js"; + +function cfgWithBindings(bindings: AgentRouteBinding[]): OpenClawConfig { + return { bindings } as unknown as OpenClawConfig; +} + +describe("resolveFirstBoundAccountId", () => { + it("returns exact peer match when caller supplies a matching peerId", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { channel: "matrix", accountId: "bot-alpha-default" }, + }, + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "!roomA:example.org" }, + accountId: "bot-alpha-room-a", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + peerId: "!roomA:example.org", + }), + ).toBe("bot-alpha-room-a"); + }); + + it("prefers wildcard peer binding over channel-only when caller supplies any peerId", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { channel: "matrix", accountId: "bot-alpha-default" }, + }, + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "*" }, + accountId: "bot-alpha-wildcard", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + peerId: "!anyRoom:example.org", + }), + ).toBe("bot-alpha-wildcard"); + }); + + it("prefers channel-only over wildcard peer binding when caller supplies no peerId", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "*" }, + accountId: "bot-alpha-wildcard", + }, + }, + { + type: "route", + agentId: "bot-alpha", + match: { channel: "matrix", accountId: "bot-alpha-default" }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + }), + ).toBe("bot-alpha-default"); + }); + + it("falls back to peer-specific binding for peerless callers when no channel-only or wildcard binding exists", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "!specificRoom:example.org" }, + accountId: "bot-alpha-specific", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + }), + ).toBe("bot-alpha-specific"); + }); + + it("skips non-matching peer-specific bindings when caller supplies a different peerId", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "!otherRoom:example.org" }, + accountId: "bot-alpha-other", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + peerId: "!differentRoom:example.org", + }), + ).toBeUndefined(); + }); + + it("returns undefined when the agent has no binding on the channel", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { channel: "whatsapp", accountId: "bot-alpha-whatsapp" }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + }), + ).toBeUndefined(); + }); +}); diff --git a/src/routing/bound-account-read.ts b/src/routing/bound-account-read.ts index 9ee9382e654..08347352958 100644 --- a/src/routing/bound-account-read.ts +++ b/src/routing/bound-account-read.ts @@ -58,6 +58,7 @@ export function resolveFirstBoundAccountId(params: { const normalizedPeerId = params.peerId?.trim() || undefined; let wildcardPeerMatch: string | undefined; let channelOnlyFallback: string | undefined; + let peerlessPeerSpecificFallback: string | undefined; for (const binding of listRouteBindings(params.cfg)) { const resolved = resolveNormalizedBindingMatch(binding); if ( @@ -68,14 +69,27 @@ export function resolveFirstBoundAccountId(params: { continue; } if (resolved.peerId === "*") { - wildcardPeerMatch ??= resolved.accountId; + if (normalizedPeerId) { + wildcardPeerMatch ??= resolved.accountId; + } else { + // Caller supplied no peer — a wildcard binding has no peer to match against, + // so treat it as a last-resort peer-ish fallback rather than letting it + // override channel-only bindings. + peerlessPeerSpecificFallback ??= resolved.accountId; + } } else if (resolved.peerId) { if (normalizedPeerId && resolved.peerId === normalizedPeerId) { return resolved.accountId; } + if (!normalizedPeerId) { + // Preserves the pre-existing "first match wins" semantics for peerless + // callers (e.g. cron delivery resolution) whose only bindings are + // peer-specific; otherwise they would silently regress to undefined. + peerlessPeerSpecificFallback ??= resolved.accountId; + } } else { channelOnlyFallback ??= resolved.accountId; } } - return wildcardPeerMatch ?? channelOnlyFallback; + return wildcardPeerMatch ?? channelOnlyFallback ?? peerlessPeerSpecificFallback; }