From 3824ee0d6fc489ac8ad05f3edc0005b62a07a77e Mon Sep 17 00:00:00 2001 From: Luke Boyett Date: Thu, 16 Apr 2026 09:17:45 -0400 Subject: [PATCH] fix(routing): wildcard/peerless/kind-equivalence fixes for bound-account lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three further review findings: 1. Parse non-Matrix peer kinds before wildcard account lookup (P1) Channels like LINE emit `line:group:` / `line:room:` targets, but the Matrix-style heuristic only recognized `@`, `!`, and `#` so the kind was lost and wildcard bindings with a declared kind were skipped. subagent-spawn now uses a single `extractRequesterPeer` helper that tries the channel plugin first, then peels the channel namespace, then loops stripping kind-prefixes (`room:`/`channel:` → `channel`, `group:` → `group`, `user:`/`dm:` → `direct`, `chat:` → `channel`) — capturing the kind from the prefix as authoritative — before falling back to the id-only heuristic for Matrix/IRC shapes. 2. Allow wildcard bindings in peerless bound-account fallback (P2) Peerless callers (cron delivery resolution passes only `channelId`/`agentId`) were blocked by the strict wildcard kind safety, silently regressing configs that only declare wildcard peer bindings. Peerless callers now accept wildcard bindings as a last- resort fallback — the kind-safety rule only applies when the caller supplies a peer to verify against. 3. Treat `group` and `channel` peer kinds as equivalent (P1) Routing elsewhere (`peerKindMatches` in `src/routing/resolve-route.ts`) intentionally accepts group/channel as compatible so a binding declared as `peer.kind: "group"` resolves for callers inferred as `channel` (Matrix rooms, Mattermost/Slack channels) and vice versa. `resolveFirstBoundAccountId` now applies the same semantics in both the wildcard and exact-id kind-cross-check branches. Regression coverage in src/routing/bound-account-read.test.ts: - Treats group and channel peer kinds as equivalent (both directions). - Accepts a wildcard peer binding as fallback for peerless callers. - Skips wildcard peer bindings when the caller's peerKind is unknown. Regression coverage in openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts: - sessions_spawn peels channel prefix then kind prefix for `::` targets (LINE `line:group:` shape) and correctly picks the `group`-kinded binding over a `direct`-kinded wildcard binding for the same agent. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...subagents.sessions-spawn.lifecycle.test.ts | 12 ++++ src/agents/subagent-spawn.ts | 5 +- src/routing/bound-account-read.test.ts | 71 +++++++++++++++++++ src/routing/bound-account-read.ts | 48 +++++++++---- 4 files changed, 123 insertions(+), 13 deletions(-) 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 2bdeb9bdf36..69d5b6a5130 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts @@ -705,6 +705,18 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { messages: { queue: { debounceMs: 0 } }, agents: { defaults: { subagents: { allowAgents: ["bot-alpha"] } } }, bindings: [ + // Wildcard peer binding with a conflicting kind (direct) — must be + // skipped because the inferred kind from the `line:group:` target is + // `group`, not `direct`. + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "line", + peer: { kind: "direct", id: "*" }, + accountId: "bot-alpha-line-dm", + }, + }, { type: "route", agentId: "bot-alpha", diff --git a/src/agents/subagent-spawn.ts b/src/agents/subagent-spawn.ts index 6e9edb60a54..f5f969cd5ee 100644 --- a/src/agents/subagent-spawn.ts +++ b/src/agents/subagent-spawn.ts @@ -290,7 +290,10 @@ function summarizeError(err: unknown): string { // 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. +// 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", diff --git a/src/routing/bound-account-read.test.ts b/src/routing/bound-account-read.test.ts index 7d5c7b9c731..65523be6b56 100644 --- a/src/routing/bound-account-read.test.ts +++ b/src/routing/bound-account-read.test.ts @@ -190,6 +190,77 @@ describe("resolveFirstBoundAccountId", () => { ).toBe("bot-alpha-dm"); }); + it("treats group and channel peer kinds as equivalent (matches resolve-route semantics)", () => { + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "line", + peer: { kind: "group", id: "*" }, + accountId: "bot-alpha-group", + }, + }, + ]); + // Caller inferred as `channel` (e.g. Matrix room, Mattermost channel) + // should still match a `group` wildcard binding because group/channel are + // compatible kinds in the routing model. + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "line", + agentId: "bot-alpha", + peerId: "!roomA:example.org", + peerKind: "channel", + }), + ).toBe("bot-alpha-group"); + // And vice versa: `channel` binding matches a `group` caller. + const cfg2 = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "line", + peer: { kind: "channel", id: "*" }, + accountId: "bot-alpha-channel", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg: cfg2, + channelId: "line", + agentId: "bot-alpha", + peerId: "groupA", + peerKind: "group", + }), + ).toBe("bot-alpha-channel"); + }); + + it("accepts a wildcard peer binding as fallback for peerless callers", () => { + // Cron-style peerless caller: we have no peer context to verify kind + // safety against, so a wildcard binding is the only available answer and + // must not silently regress to undefined. + const cfg = cfgWithBindings([ + { + type: "route", + agentId: "bot-alpha", + match: { + channel: "matrix", + peer: { kind: "channel", id: "*" }, + accountId: "bot-alpha-wildcard", + }, + }, + ]); + expect( + resolveFirstBoundAccountId({ + cfg, + channelId: "matrix", + agentId: "bot-alpha", + }), + ).toBe("bot-alpha-wildcard"); + }); + it("skips wildcard peer bindings when the caller's peerKind is unknown", () => { const cfg = cfgWithBindings([ { diff --git a/src/routing/bound-account-read.ts b/src/routing/bound-account-read.ts index 2e2a0b403ae..18fd6d11953 100644 --- a/src/routing/bound-account-read.ts +++ b/src/routing/bound-account-read.ts @@ -48,6 +48,18 @@ function resolveNormalizedBindingMatch(binding: AgentRouteBinding): { }; } +// Peer-kind equivalence matches resolve-route.ts: `group` and `channel` are +// treated as compatible so bindings authored as `peer.kind: "group"` resolve +// for callers inferred as `"channel"` (Matrix rooms, Slack/Mattermost +// channels) and vice versa. +function peerKindMatches(a: ChatType, b: ChatType): boolean { + if (a === b) { + return true; + } + const pair = new Set([a, b]); + return pair.has("group") && pair.has("channel"); +} + export function resolveFirstBoundAccountId(params: { cfg: OpenClawConfig; channelId: string; @@ -75,18 +87,26 @@ export function resolveFirstBoundAccountId(params: { continue; } if (resolved.peerId === "*") { - // Wildcard peer bindings are only safe when both sides declare a peer - // kind AND the kinds agree. If either side lacks a kind, skip — a - // direct/* binding must never win for a channel caller (or vice versa), - // and we'd rather fall through to channel-only or the caller account - // than actively route to the wrong identity. - if (!resolved.peerKind || !normalizedPeerKind || resolved.peerKind !== normalizedPeerKind) { - continue; - } - if (normalizedPeerId) { - wildcardPeerMatch ??= resolved.accountId; - } else { + if (!normalizedPeerId) { + // Peerless caller (for example cron delivery resolution). We have no + // peer context to apply kind safety against, so accept wildcards as a + // last-resort fallback — this preserves the pre-existing first-match + // semantics for configs that only declare wildcard peer bindings. peerlessPeerSpecificFallback ??= resolved.accountId; + } else { + // Caller has a peer. Wildcard bindings are only safe when both sides + // declare a peer kind AND the kinds agree — a direct/* binding must + // never win for a channel caller (or vice versa), and we'd rather fall + // through to channel-only or the caller account than actively route to + // the wrong identity. + if ( + !resolved.peerKind || + !normalizedPeerKind || + !peerKindMatches(resolved.peerKind, normalizedPeerKind) + ) { + continue; + } + wildcardPeerMatch ??= resolved.accountId; } } else if (resolved.peerId) { // Exact peer id match: peer ids are channel-unique so id alone is @@ -94,7 +114,11 @@ export function resolveFirstBoundAccountId(params: { // (avoids a direct-kind binding matching a channel caller that happens // to share an id, which can occur on channels where ids are reused // across kinds). - if (resolved.peerKind && normalizedPeerKind && resolved.peerKind !== normalizedPeerKind) { + if ( + resolved.peerKind && + normalizedPeerKind && + !peerKindMatches(resolved.peerKind, normalizedPeerKind) + ) { continue; } if (normalizedPeerId && resolved.peerId === normalizedPeerId) {