mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-03 02:10:21 +00:00
fix(followup): fall back to dispatcher when same-channel origin routing fails (#26109)
* fix(followup): fall back to dispatcher when same-channel origin routing fails When routeReply fails for an originating channel that matches the session's messageProvider, the onBlockReply callback was created by that same channel's handler and can safely deliver the reply. Previously the payload was silently dropped on any routeReply failure, causing Feishu DM replies to never reach the user. Cross-channel fallback (origin ≠ provider) still drops the payload to preserve origin isolation. Closes #25767 Co-authored-by: Cursor <cursoragent@cursor.com> * fix: allow same-channel followup fallback routing (#26109) (thanks @Sid-Qin) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import { createMockTypingController } from "./test-helpers.js";
|
||||
|
||||
const runEmbeddedPiAgentMock = vi.fn();
|
||||
const routeReplyMock = vi.fn();
|
||||
const isRoutableChannelMock = vi.fn();
|
||||
|
||||
vi.mock(
|
||||
"../../agents/model-fallback.js",
|
||||
@@ -22,15 +23,30 @@ vi.mock("./route-reply.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("./route-reply.js")>();
|
||||
return {
|
||||
...actual,
|
||||
isRoutableChannel: (...args: unknown[]) => isRoutableChannelMock(...args),
|
||||
routeReply: (...args: unknown[]) => routeReplyMock(...args),
|
||||
};
|
||||
});
|
||||
|
||||
import { createFollowupRunner } from "./followup-runner.js";
|
||||
|
||||
const ROUTABLE_TEST_CHANNELS = new Set([
|
||||
"telegram",
|
||||
"slack",
|
||||
"discord",
|
||||
"signal",
|
||||
"imessage",
|
||||
"whatsapp",
|
||||
"feishu",
|
||||
]);
|
||||
|
||||
beforeEach(() => {
|
||||
routeReplyMock.mockReset();
|
||||
routeReplyMock.mockResolvedValue({ ok: true });
|
||||
isRoutableChannelMock.mockReset();
|
||||
isRoutableChannelMock.mockImplementation((ch: string | undefined) =>
|
||||
Boolean(ch?.trim() && ROUTABLE_TEST_CHANNELS.has(ch.trim().toLowerCase())),
|
||||
);
|
||||
});
|
||||
|
||||
const baseQueuedRun = (messageProvider = "whatsapp"): FollowupRun =>
|
||||
@@ -336,7 +352,7 @@ describe("createFollowupRunner messaging tool dedupe", () => {
|
||||
expect(store[sessionKey]?.outputTokens).toBe(50);
|
||||
});
|
||||
|
||||
it("does not fall back to dispatcher when explicit origin routing fails", async () => {
|
||||
it("does not fall back to dispatcher when cross-channel origin routing fails", async () => {
|
||||
const onBlockReply = vi.fn(async () => {});
|
||||
runEmbeddedPiAgentMock.mockResolvedValueOnce({
|
||||
payloads: [{ text: "hello world!" }],
|
||||
@@ -359,6 +375,30 @@ describe("createFollowupRunner messaging tool dedupe", () => {
|
||||
expect(onBlockReply).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls back to dispatcher when same-channel origin routing fails", async () => {
|
||||
const onBlockReply = vi.fn(async () => {});
|
||||
runEmbeddedPiAgentMock.mockResolvedValueOnce({
|
||||
payloads: [{ text: "hello world!" }],
|
||||
meta: {},
|
||||
});
|
||||
routeReplyMock.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
error: "outbound adapter unavailable",
|
||||
});
|
||||
|
||||
const runner = createMessagingDedupeRunner(onBlockReply);
|
||||
|
||||
await runner({
|
||||
...baseQueuedRun(" Feishu "),
|
||||
originatingChannel: "FEISHU",
|
||||
originatingTo: "ou_abc123",
|
||||
} as FollowupRun);
|
||||
|
||||
expect(routeReplyMock).toHaveBeenCalled();
|
||||
expect(onBlockReply).toHaveBeenCalledTimes(1);
|
||||
expect(onBlockReply).toHaveBeenCalledWith(expect.objectContaining({ text: "hello world!" }));
|
||||
});
|
||||
|
||||
it("routes followups with originating account/thread metadata", async () => {
|
||||
const onBlockReply = vi.fn(async () => {});
|
||||
runEmbeddedPiAgentMock.mockResolvedValueOnce({
|
||||
|
||||
@@ -103,10 +103,23 @@ export function createFollowupRunner(params: {
|
||||
cfg: queued.run.config,
|
||||
});
|
||||
if (!result.ok) {
|
||||
// Keep origin isolation strict: do not fall back to the current
|
||||
// dispatcher when explicit origin routing failed.
|
||||
const errorMsg = result.error ?? "unknown error";
|
||||
logVerbose(`followup queue: route-reply failed: ${errorMsg}`);
|
||||
// Fall back to the caller-provided dispatcher only when the
|
||||
// originating channel matches the session's message provider.
|
||||
// In that case onBlockReply was created by the same channel's
|
||||
// handler and delivers to the correct destination. For true
|
||||
// cross-channel routing (origin !== provider), falling back
|
||||
// would send to the wrong channel, so we drop the payload.
|
||||
const provider = resolveOriginMessageProvider({
|
||||
provider: queued.run.messageProvider,
|
||||
});
|
||||
const origin = resolveOriginMessageProvider({
|
||||
originatingChannel,
|
||||
});
|
||||
if (opts?.onBlockReply && origin && origin === provider) {
|
||||
await opts.onBlockReply(payload);
|
||||
}
|
||||
}
|
||||
} else if (opts?.onBlockReply) {
|
||||
await opts.onBlockReply(payload);
|
||||
|
||||
Reference in New Issue
Block a user