From c9cabc27cde7b5641ca613a1564b3aa416f6d055 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 18 Aug 2026 22:29:53 -0700 Subject: [PATCH] fix(imessage): honor bound routes before agent selection Punchcard-Session: coral-valley-summit-qv --- .../imessage/src/conversation-route.test.ts | 22 +++- extensions/imessage/src/conversation-route.ts | 64 ++++++----- src/channels/plugins/binding-routing.test.ts | 106 +++++++++++------- src/channels/plugins/binding-routing.ts | 104 ++++++++--------- .../conversation-binding-runtime.ts | 1 + 5 files changed, 170 insertions(+), 127 deletions(-) diff --git a/extensions/imessage/src/conversation-route.test.ts b/extensions/imessage/src/conversation-route.test.ts index e86b37870740..d390424d2fae 100644 --- a/extensions/imessage/src/conversation-route.test.ts +++ b/extensions/imessage/src/conversation-route.test.ts @@ -4,7 +4,7 @@ import { testing as sessionBindingTesting, registerSessionBindingAdapter, } from "openclaw/plugin-sdk/conversation-runtime"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { resolveIMessageConversationRoute } from "./conversation-route.js"; const baseCfg = { @@ -12,7 +12,13 @@ const baseCfg = { agents: { list: [{ id: "main" }, { id: "codex" }], }, - bindings: [{ agentId: "main", match: { channel: "imessage", accountId: "default" } }], + bindings: [ + { + agentId: "main", + match: { channel: "imessage", accountId: "default" }, + session: { dmScope: "per-peer" }, + }, + ], } satisfies OpenClawConfig; describe("resolveIMessageConversationRoute", () => { @@ -20,7 +26,14 @@ describe("resolveIMessageConversationRoute", () => { sessionBindingTesting.resetSessionBindingAdaptersForTests(); }); - it("lets runtime iMessage conversation bindings override default routing", () => { + afterEach(() => { + sessionBindingTesting.resetSessionBindingAdaptersForTests(); + }); + + it.each([ + ["ambiguous routing", { ...baseCfg, bindings: [] }], + ["a conflicting configured route", { ...baseCfg, agents: { list: [{ id: "main" }] } }], + ])("lets runtime bindings override %s", (_name, cfg) => { const touch = vi.fn(); registerSessionBindingAdapter({ channel: "imessage", @@ -46,7 +59,7 @@ describe("resolveIMessageConversationRoute", () => { }); const route = resolveIMessageConversationRoute({ - cfg: baseCfg, + cfg, accountId: "default", isGroup: false, peerId: "+15555550123", @@ -54,6 +67,7 @@ describe("resolveIMessageConversationRoute", () => { }); expect(route.agentId).toBe("codex"); + expect(route.dmScope).toBe("main"); expect(route.sessionKey).toBe("agent:codex:acp:bound-1"); expect(route.matchedBy).toBe("binding.channel"); expect(touch).toHaveBeenCalledWith("default:+15555550123", undefined); diff --git a/extensions/imessage/src/conversation-route.ts b/extensions/imessage/src/conversation-route.ts index 85333a226d55..111a74fee0b5 100644 --- a/extensions/imessage/src/conversation-route.ts +++ b/extensions/imessage/src/conversation-route.ts @@ -1,10 +1,8 @@ // Imessage plugin module implements conversation route behavior. import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { - resolveConfiguredBindingRoute, - resolveRuntimeConversationBindingRoute, -} from "openclaw/plugin-sdk/conversation-runtime"; -import { resolveAgentRoute } from "openclaw/plugin-sdk/routing"; +import { resolveRuntimeConversationBindingRouteWithFallback } from "openclaw/plugin-sdk/conversation-binding-runtime"; +import { resolveConfiguredBindingRoute } from "openclaw/plugin-sdk/conversation-runtime"; +import { buildAgentMainSessionKey, resolveAgentRoute } from "openclaw/plugin-sdk/routing"; import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { resolveIMessageInboundConversationId } from "./conversation-id.js"; @@ -16,7 +14,12 @@ export function resolveIMessageConversationRoute(params: { sender: string; chatId?: number; }): ReturnType { - let route = resolveAgentRoute({ + const conversationId = resolveIMessageInboundConversationId({ + isGroup: params.isGroup, + sender: params.sender, + chatId: params.chatId, + }); + const routeInput = { cfg: params.cfg, channel: "imessage", accountId: params.accountId, @@ -24,36 +27,41 @@ export function resolveIMessageConversationRoute(params: { kind: params.isGroup ? "group" : "direct", id: params.peerId, }, - }); - - const conversationId = resolveIMessageInboundConversationId({ - isGroup: params.isGroup, - sender: params.sender, - chatId: params.chatId, - }); + } satisfies Parameters[0]; + const resolveFallbackRoute = () => { + const route = resolveAgentRoute(routeInput); + return conversationId + ? resolveConfiguredBindingRoute({ + cfg: params.cfg, + route, + conversation: { + channel: "imessage", + accountId: params.accountId, + conversationId, + }, + }).route + : route; + }; if (!conversationId) { - return route; + return resolveFallbackRoute(); } - route = resolveConfiguredBindingRoute({ - cfg: params.cfg, - route, - conversation: { - channel: "imessage", - accountId: params.accountId, - conversationId, - }, - }).route; - - const runtimeRoute = resolveRuntimeConversationBindingRoute({ - route, + const boundCfg = { ...params.cfg, agents: undefined, bindings: [] }; + const runtimeRoute = resolveRuntimeConversationBindingRouteWithFallback({ conversation: { channel: "imessage", accountId: params.accountId, conversationId, }, + resolveFallbackRoute, + resolveBoundRoute: (agentId) => ({ + ...resolveAgentRoute({ ...routeInput, cfg: boundCfg, defaultAgentId: agentId }), + mainSessionKey: buildAgentMainSessionKey({ + agentId, + mainKey: params.cfg.session?.mainKey, + }), + }), }); - route = runtimeRoute.route; if (runtimeRoute.bindingRecord && !runtimeRoute.boundSessionKey) { logVerbose(`imessage: plugin-bound conversation ${conversationId}`); } else if (runtimeRoute.boundSessionKey) { @@ -61,5 +69,5 @@ export function resolveIMessageConversationRoute(params: { `imessage: routed via bound conversation ${conversationId} -> ${runtimeRoute.boundSessionKey}`, ); } - return route; + return runtimeRoute.route; } diff --git a/src/channels/plugins/binding-routing.test.ts b/src/channels/plugins/binding-routing.test.ts index 72482911e613..e6a95a0b9d48 100644 --- a/src/channels/plugins/binding-routing.test.ts +++ b/src/channels/plugins/binding-routing.test.ts @@ -10,6 +10,7 @@ import type { ResolvedAgentRoute } from "../../routing/resolve-route.js"; import { ensureConfiguredBindingRouteReady, resolveRuntimeConversationBindingRoute, + resolveRuntimeConversationBindingRouteWithFallback, } from "./binding-routing.js"; import { registerStatefulBindingTargetDriver } from "./stateful-target-drivers.js"; @@ -94,52 +95,81 @@ describe("runtime conversation binding route", () => { }); }); - it("touches plugin-owned bindings without rewriting the channel route", () => { - const route = createRoute(); - const binding = createBinding({ - metadata: { - pluginBindingOwner: "plugin", - pluginId: "demo-plugin", - pluginRoot: "/tmp/demo-plugin", - }, - }); - const { touch } = registerAdapter(binding); + it("resolves a core binding before fallback and preserves route scope", () => { + const { touch } = registerAdapter(createBinding()); + const fallback = vi.fn(createRoute); + const boundRoute = { + ...createRoute(), + agentId: "review", + dmScope: "per-account-channel-peer" as const, + groupScope: "main" as const, + mainSessionKey: "agent:review:home", + }; + const resolveBoundRoute = vi.fn(() => boundRoute); - const result = resolveRuntimeConversationBindingRoute({ - route, - conversation: { - channel: "demo", - accountId: "default", - conversationId: "room-1", - }, + const result = resolveRuntimeConversationBindingRouteWithFallback({ + conversation: { channel: "demo", accountId: "default", conversationId: "room-1" }, + resolveFallbackRoute: fallback, + resolveBoundRoute, }); + expect(fallback).not.toHaveBeenCalled(); + expect(resolveBoundRoute).toHaveBeenCalledWith("review"); expect(touch).toHaveBeenCalledWith("binding-1", undefined); - expect(result.bindingRecord).toBe(binding); - expect(result.boundSessionKey).toBeUndefined(); + expect(result.route).toEqual({ + ...boundRoute, + sessionKey: "agent:review:acp:session-1", + lastRoutePolicy: "session", + matchedBy: "binding.channel", + }); + }); + + it.each([ + ["absent", null, false, false], + ["empty", createBinding({ targetSessionKey: " " }), false, false], + [ + "plugin-owned", + createBinding({ + metadata: { pluginBindingOwner: "plugin", pluginId: "demo", pluginRoot: "/plugin" }, + }), + true, + true, + ], + ["cron", createBinding({ targetSessionKey: "agent:review:cron:job:run:1" }), false, false], + ] as const)("falls back once for %s bindings", (_name, binding, shouldTouch, retainsRecord) => { + const route = createRoute(); + const { touch } = registerAdapter(binding); + const fallback = vi.fn(() => route); + const resolveBoundRoute = vi.fn(() => route); + + const result = resolveRuntimeConversationBindingRouteWithFallback({ + conversation: { channel: "demo", accountId: "default", conversationId: "room-1" }, + resolveFallbackRoute: fallback, + resolveBoundRoute, + }); + + expect(fallback).toHaveBeenCalledOnce(); + expect(resolveBoundRoute).not.toHaveBeenCalled(); + expect(touch).toHaveBeenCalledTimes(shouldTouch ? 1 : 0); + expect(result.bindingRecord).toBe(retainsRecord ? binding : null); expect(result.route).toBe(route); }); - it("ignores runtime bindings that target isolated cron run sessions", () => { - const route = createRoute(); - const binding = createBinding({ - targetSessionKey: "agent:youtube:cron:monthly-report:run:closed-run-1", - }); - const { touch } = registerAdapter(binding); + it("rejects malformed bound session keys before resolving either route", () => { + const { touch } = registerAdapter(createBinding({ targetSessionKey: "agent:" })); + const fallback = vi.fn(createRoute); + const resolveBoundRoute = vi.fn(createRoute); - const result = resolveRuntimeConversationBindingRoute({ - route, - conversation: { - channel: "demo", - accountId: "default", - conversationId: "room-1", - }, - }); - - expect(touch).not.toHaveBeenCalled(); - expect(result.bindingRecord).toBeNull(); - expect(result.boundSessionKey).toBeUndefined(); - expect(result.route).toBe(route); + expect(() => + resolveRuntimeConversationBindingRouteWithFallback({ + conversation: { channel: "demo", accountId: "default", conversationId: "room-1" }, + resolveFallbackRoute: fallback, + resolveBoundRoute, + }), + ).toThrow("Malformed agent session key"); + expect(touch).toHaveBeenCalledWith("binding-1", undefined); + expect(fallback).not.toHaveBeenCalled(); + expect(resolveBoundRoute).not.toHaveBeenCalled(); }); }); diff --git a/src/channels/plugins/binding-routing.ts b/src/channels/plugins/binding-routing.ts index a34d9469b8f0..28634ac43eae 100644 --- a/src/channels/plugins/binding-routing.ts +++ b/src/channels/plugins/binding-routing.ts @@ -20,9 +20,6 @@ import { resolveConfiguredBinding } from "./configured-binding-registry.js"; const CONFIGURED_BINDING_ROUTE_READY_TIMEOUT_MS = 30_000; -/** - * Route resolution after applying a configured channel binding. - */ export type ConfiguredBindingRouteResult = { bindingResolution: ConfiguredBindingResolution | null; route: ResolvedAgentRoute; @@ -30,9 +27,6 @@ export type ConfiguredBindingRouteResult = { boundAgentId?: string; }; -/** - * Route resolution after applying a runtime conversation binding record. - */ export type RuntimeConversationBindingRouteResult = { bindingRecord: SessionBindingRecord | null; route: ResolvedAgentRoute; @@ -77,6 +71,48 @@ function isPluginOwnedRuntimeBindingRecord(record: SessionBindingRecord | null): ); } +export function resolveRuntimeConversationBindingRouteWithFallback(params: { + conversation: ConversationRef; + resolveFallbackRoute: () => ResolvedAgentRoute; + resolveBoundRoute: (agentId: string) => ResolvedAgentRoute; +}): RuntimeConversationBindingRouteResult { + const bindingRecord = getSessionBindingService().resolveByConversation(params.conversation); + const boundSessionKey = bindingRecord?.targetSessionKey?.trim(); + if (!bindingRecord || !boundSessionKey) { + return { bindingRecord: null, route: params.resolveFallbackRoute() }; + } + if (isCronRunSessionKey(boundSessionKey)) { + // Cron run sessions are isolated and short-lived; never refresh or route live traffic to them. + logVerbose( + `ignored runtime conversation binding ${bindingRecord.bindingId} to isolated cron run session ${boundSessionKey}`, + ); + return { bindingRecord: null, route: params.resolveFallbackRoute() }; + } + + getSessionBindingService().touch(bindingRecord.bindingId); + if (isPluginOwnedRuntimeBindingRecord(bindingRecord)) { + return { bindingRecord, route: params.resolveFallbackRoute() }; + } + + const boundAgentId = resolveAgentIdFromSessionKey(boundSessionKey); + const route = params.resolveBoundRoute(boundAgentId); + return { + bindingRecord, + boundSessionKey, + boundAgentId, + route: { + ...route, + agentId: boundAgentId, + sessionKey: boundSessionKey, + lastRoutePolicy: deriveLastRoutePolicy({ + sessionKey: boundSessionKey, + mainSessionKey: route.mainSessionKey, + }), + matchedBy: "binding.channel", + }, + }; +} + /** * Rewrites an agent route when the current conversation matches a configured binding. */ @@ -126,62 +162,16 @@ export function resolveConfiguredBindingRoute( }; } -/** - * Rewrites an agent route using a persisted runtime conversation binding, when applicable. - */ export function resolveRuntimeConversationBindingRoute( params: { route: ResolvedAgentRoute; } & ConfiguredBindingRouteConversationInput, ): RuntimeConversationBindingRouteResult { - const bindingRecord = getSessionBindingService().resolveByConversation( - resolveConfiguredBindingConversationRef(params), - ); - const boundSessionKey = bindingRecord?.targetSessionKey?.trim(); - if (!bindingRecord || !boundSessionKey) { - return { - bindingRecord: null, - route: params.route, - }; - } - - if (isCronRunSessionKey(boundSessionKey)) { - // Cron run sessions are isolated and short-lived; never route live channel traffic into them. - logVerbose( - `ignored runtime conversation binding ${bindingRecord.bindingId} to isolated cron run session ${boundSessionKey}`, - ); - return { - bindingRecord: null, - route: params.route, - }; - } - - getSessionBindingService().touch(bindingRecord.bindingId); - if (isPluginOwnedRuntimeBindingRecord(bindingRecord)) { - // Plugin-owned binding records are observed but not route-rewritten by core; the owning - // plugin is responsible for its runtime target handoff. - return { - bindingRecord, - route: params.route, - }; - } - - const boundAgentId = resolveAgentIdFromSessionKey(boundSessionKey) || params.route.agentId; - return { - bindingRecord, - boundSessionKey, - boundAgentId, - route: { - ...params.route, - sessionKey: boundSessionKey, - agentId: boundAgentId, - lastRoutePolicy: deriveLastRoutePolicy({ - sessionKey: boundSessionKey, - mainSessionKey: params.route.mainSessionKey, - }), - matchedBy: "binding.channel", - }, - }; + return resolveRuntimeConversationBindingRouteWithFallback({ + conversation: resolveConfiguredBindingConversationRef(params), + resolveFallbackRoute: () => params.route, + resolveBoundRoute: () => params.route, + }); } /** diff --git a/src/plugin-sdk/conversation-binding-runtime.ts b/src/plugin-sdk/conversation-binding-runtime.ts index 8c70d62f7401..6c1de71ffba9 100644 --- a/src/plugin-sdk/conversation-binding-runtime.ts +++ b/src/plugin-sdk/conversation-binding-runtime.ts @@ -6,6 +6,7 @@ export { resolveConfiguredBindingRoute, type ConfiguredBindingRouteResult, resolveRuntimeConversationBindingRoute, + resolveRuntimeConversationBindingRouteWithFallback, type RuntimeConversationBindingRouteResult, } from "../channels/plugins/binding-routing.js"; export {