mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 20:20:42 +00:00
fix(routing): wildcard/peerless/kind-equivalence fixes for bound-account lookup
Addresses three further review findings: 1. Parse non-Matrix peer kinds before wildcard account lookup (P1) Channels like LINE emit `line:group:<id>` / `line:room:<id>` 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 `<channel>:<kind>:<id>` targets (LINE `line:group:<id>` 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) <noreply@anthropic.com>
This commit is contained in:
committed by
Gustavo Madeira Santana
parent
5ac04e4c0b
commit
3824ee0d6f
@@ -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",
|
||||
|
||||
@@ -290,7 +290,10 @@ function summarizeError(err: unknown): string {
|
||||
// Delivery targets carry a channel-side prefix (e.g. Matrix uses `room:<roomId>`;
|
||||
// LINE uses `line:group:<id>`), but route bindings store raw peer ids on
|
||||
// `match.peer.id`. Peel the `<channel>:` 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 `<channel>:<kind>:<id>`.
|
||||
const KIND_PREFIX_TO_CHAT_TYPE: Readonly<Record<string, ChatType>> = {
|
||||
"room:": "channel",
|
||||
"channel:": "channel",
|
||||
|
||||
@@ -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([
|
||||
{
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user