mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 02:21:25 +00:00
fix: stop hardcoded channel fallback and auto-pick sole configured channel (#23357) (thanks @lbo728)
Co-authored-by: lbo728 <extreme0728@gmail.com>
This commit is contained in:
@@ -15,6 +15,7 @@ import {
|
||||
resolveAgentDeliveryPlan,
|
||||
resolveAgentOutboundTarget,
|
||||
} from "../../infra/outbound/agent-delivery.js";
|
||||
import { resolveMessageChannelSelection } from "../../infra/outbound/channel-selection.js";
|
||||
import { classifySessionKeyShape, normalizeAgentId } from "../../routing/session-key.js";
|
||||
import { defaultRuntime } from "../../runtime.js";
|
||||
import { normalizeInputProvenance, type InputProvenance } from "../../sessions/input-provenance.js";
|
||||
@@ -490,17 +491,36 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
wantsDelivery,
|
||||
});
|
||||
|
||||
const resolvedChannel = deliveryPlan.resolvedChannel;
|
||||
const deliveryTargetMode = deliveryPlan.deliveryTargetMode;
|
||||
const resolvedAccountId = deliveryPlan.resolvedAccountId;
|
||||
let resolvedChannel = deliveryPlan.resolvedChannel;
|
||||
let deliveryTargetMode = deliveryPlan.deliveryTargetMode;
|
||||
let resolvedAccountId = deliveryPlan.resolvedAccountId;
|
||||
let resolvedTo = deliveryPlan.resolvedTo;
|
||||
let effectivePlan = deliveryPlan;
|
||||
|
||||
if (wantsDelivery && resolvedChannel === INTERNAL_MESSAGE_CHANNEL) {
|
||||
const cfgResolved = cfgForAgent ?? cfg;
|
||||
try {
|
||||
const selection = await resolveMessageChannelSelection({ cfg: cfgResolved });
|
||||
resolvedChannel = selection.channel;
|
||||
deliveryTargetMode = deliveryTargetMode ?? "implicit";
|
||||
effectivePlan = {
|
||||
...deliveryPlan,
|
||||
resolvedChannel,
|
||||
deliveryTargetMode,
|
||||
resolvedAccountId,
|
||||
};
|
||||
} catch (err) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, String(err)));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (!resolvedTo && isDeliverableMessageChannel(resolvedChannel)) {
|
||||
const cfgResolved = cfgForAgent ?? cfg;
|
||||
const fallback = resolveAgentOutboundTarget({
|
||||
cfg: cfgResolved,
|
||||
plan: deliveryPlan,
|
||||
targetMode: "implicit",
|
||||
plan: effectivePlan,
|
||||
targetMode: deliveryTargetMode ?? "implicit",
|
||||
validateExplicitTarget: false,
|
||||
});
|
||||
if (fallback.resolvedTarget?.ok) {
|
||||
@@ -508,6 +528,18 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
}
|
||||
}
|
||||
|
||||
if (wantsDelivery && resolvedChannel === INTERNAL_MESSAGE_CHANNEL) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
"delivery channel is required: pass --channel/--reply-channel or use a main session with a previous channel",
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const deliver = request.deliver === true && resolvedChannel !== INTERNAL_MESSAGE_CHANNEL;
|
||||
|
||||
const accepted = {
|
||||
|
||||
@@ -8,6 +8,8 @@ const mocks = vi.hoisted(() => ({
|
||||
appendAssistantMessageToSessionTranscript: vi.fn(async () => ({ ok: true, sessionFile: "x" })),
|
||||
recordSessionMetaFromInbound: vi.fn(async () => ({ ok: true })),
|
||||
resolveOutboundTarget: vi.fn(() => ({ ok: true, to: "resolved" })),
|
||||
resolveMessageChannelSelection: vi.fn(),
|
||||
sendPoll: vi.fn(async () => ({ messageId: "poll-1" })),
|
||||
}));
|
||||
|
||||
vi.mock("../../config/config.js", async () => {
|
||||
@@ -20,7 +22,7 @@ vi.mock("../../config/config.js", async () => {
|
||||
});
|
||||
|
||||
vi.mock("../../channels/plugins/index.js", () => ({
|
||||
getChannelPlugin: () => ({ outbound: {} }),
|
||||
getChannelPlugin: () => ({ outbound: { sendPoll: mocks.sendPoll } }),
|
||||
normalizeChannelId: (value: string) => (value === "webchat" ? null : value),
|
||||
}));
|
||||
|
||||
@@ -28,6 +30,10 @@ vi.mock("../../infra/outbound/targets.js", () => ({
|
||||
resolveOutboundTarget: mocks.resolveOutboundTarget,
|
||||
}));
|
||||
|
||||
vi.mock("../../infra/outbound/channel-selection.js", () => ({
|
||||
resolveMessageChannelSelection: mocks.resolveMessageChannelSelection,
|
||||
}));
|
||||
|
||||
vi.mock("../../infra/outbound/deliver.js", () => ({
|
||||
deliverOutboundPayloads: mocks.deliverOutboundPayloads,
|
||||
}));
|
||||
@@ -61,6 +67,19 @@ async function runSend(params: Record<string, unknown>) {
|
||||
return { respond };
|
||||
}
|
||||
|
||||
async function runPoll(params: Record<string, unknown>) {
|
||||
const respond = vi.fn();
|
||||
await sendHandlers.poll({
|
||||
params: params as never,
|
||||
respond,
|
||||
context: makeContext(),
|
||||
req: { type: "req", id: "1", method: "poll" },
|
||||
client: null,
|
||||
isWebchatConnect: () => false,
|
||||
});
|
||||
return { respond };
|
||||
}
|
||||
|
||||
function mockDeliverySuccess(messageId: string) {
|
||||
mocks.deliverOutboundPayloads.mockResolvedValue([{ messageId, channel: "slack" }]);
|
||||
}
|
||||
@@ -69,6 +88,11 @@ describe("gateway send mirroring", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mocks.resolveOutboundTarget.mockReturnValue({ ok: true, to: "resolved" });
|
||||
mocks.resolveMessageChannelSelection.mockResolvedValue({
|
||||
channel: "slack",
|
||||
configured: ["slack"],
|
||||
});
|
||||
mocks.sendPoll.mockResolvedValue({ messageId: "poll-1" });
|
||||
});
|
||||
|
||||
it("accepts media-only sends without message", async () => {
|
||||
@@ -137,6 +161,81 @@ describe("gateway send mirroring", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("auto-picks the single configured channel for send", async () => {
|
||||
mockDeliverySuccess("m-single-send");
|
||||
|
||||
const { respond } = await runSend({
|
||||
to: "x",
|
||||
message: "hi",
|
||||
idempotencyKey: "idem-missing-channel",
|
||||
});
|
||||
|
||||
expect(mocks.resolveMessageChannelSelection).toHaveBeenCalled();
|
||||
expect(mocks.deliverOutboundPayloads).toHaveBeenCalled();
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({ messageId: "m-single-send" }),
|
||||
undefined,
|
||||
expect.objectContaining({ channel: "slack" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns invalid request when send channel selection is ambiguous", async () => {
|
||||
mocks.resolveMessageChannelSelection.mockRejectedValueOnce(
|
||||
new Error("Channel is required when multiple channels are configured: telegram, slack"),
|
||||
);
|
||||
|
||||
const { respond } = await runSend({
|
||||
to: "x",
|
||||
message: "hi",
|
||||
idempotencyKey: "idem-missing-channel-ambiguous",
|
||||
});
|
||||
|
||||
expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled();
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("Channel is required"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("auto-picks the single configured channel for poll", async () => {
|
||||
const { respond } = await runPoll({
|
||||
to: "x",
|
||||
question: "Q?",
|
||||
options: ["A", "B"],
|
||||
idempotencyKey: "idem-poll-missing-channel",
|
||||
});
|
||||
|
||||
expect(mocks.resolveMessageChannelSelection).toHaveBeenCalled();
|
||||
expect(respond).toHaveBeenCalledWith(true, expect.any(Object), undefined, {
|
||||
channel: "slack",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns invalid request when poll channel selection is ambiguous", async () => {
|
||||
mocks.resolveMessageChannelSelection.mockRejectedValueOnce(
|
||||
new Error("Channel is required when multiple channels are configured: telegram, slack"),
|
||||
);
|
||||
|
||||
const { respond } = await runPoll({
|
||||
to: "x",
|
||||
question: "Q?",
|
||||
options: ["A", "B"],
|
||||
idempotencyKey: "idem-poll-missing-channel-ambiguous",
|
||||
});
|
||||
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("Channel is required"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not mirror when delivery returns no results", async () => {
|
||||
mocks.deliverOutboundPayloads.mockResolvedValue([]);
|
||||
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import { resolveSessionAgentId } from "../../agents/agent-scope.js";
|
||||
import { getChannelPlugin, normalizeChannelId } from "../../channels/plugins/index.js";
|
||||
import { DEFAULT_CHAT_CHANNEL } from "../../channels/registry.js";
|
||||
import { createOutboundSendDeps } from "../../cli/deps.js";
|
||||
import { loadConfig } from "../../config/config.js";
|
||||
import { resolveMessageChannelSelection } from "../../infra/outbound/channel-selection.js";
|
||||
import { deliverOutboundPayloads } from "../../infra/outbound/deliver.js";
|
||||
import {
|
||||
ensureOutboundSessionEntry,
|
||||
@@ -126,7 +126,16 @@ export const sendHandlers: GatewayRequestHandlers = {
|
||||
);
|
||||
return;
|
||||
}
|
||||
const channel = normalizedChannel ?? DEFAULT_CHAT_CHANNEL;
|
||||
const cfg = loadConfig();
|
||||
let channel = normalizedChannel;
|
||||
if (!channel) {
|
||||
try {
|
||||
channel = (await resolveMessageChannelSelection({ cfg })).channel;
|
||||
} catch (err) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, String(err)));
|
||||
return;
|
||||
}
|
||||
}
|
||||
const accountId =
|
||||
typeof request.accountId === "string" && request.accountId.trim().length
|
||||
? request.accountId.trim()
|
||||
@@ -148,7 +157,6 @@ export const sendHandlers: GatewayRequestHandlers = {
|
||||
|
||||
const work = (async (): Promise<InflightResult> => {
|
||||
try {
|
||||
const cfg = loadConfig();
|
||||
const resolved = resolveOutboundTarget({
|
||||
channel: outboundChannel,
|
||||
to,
|
||||
@@ -324,7 +332,16 @@ export const sendHandlers: GatewayRequestHandlers = {
|
||||
);
|
||||
return;
|
||||
}
|
||||
const channel = normalizedChannel ?? DEFAULT_CHAT_CHANNEL;
|
||||
const cfg = loadConfig();
|
||||
let channel = normalizedChannel;
|
||||
if (!channel) {
|
||||
try {
|
||||
channel = (await resolveMessageChannelSelection({ cfg })).channel;
|
||||
} catch (err) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, String(err)));
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (typeof request.durationSeconds === "number" && channel !== "telegram") {
|
||||
respond(
|
||||
false,
|
||||
@@ -370,7 +387,6 @@ export const sendHandlers: GatewayRequestHandlers = {
|
||||
);
|
||||
return;
|
||||
}
|
||||
const cfg = loadConfig();
|
||||
const resolved = resolveOutboundTarget({
|
||||
channel: channel,
|
||||
to,
|
||||
|
||||
@@ -435,19 +435,31 @@ describe("gateway server agent", () => {
|
||||
expect(images[0]?.data).toBe(BASE_IMAGE_PNG);
|
||||
});
|
||||
|
||||
test("agent falls back to whatsapp when delivery requested and no last channel exists", async () => {
|
||||
const call = await runMainAgentDeliveryWithSession({
|
||||
entry: {
|
||||
sessionId: "sess-main-missing-provider",
|
||||
},
|
||||
request: {
|
||||
test("agent errors when delivery requested and no last channel exists", async () => {
|
||||
setRegistry(defaultRegistry);
|
||||
testState.allowFrom = ["+1555"];
|
||||
try {
|
||||
await setTestSessionStore({
|
||||
entries: {
|
||||
main: {
|
||||
sessionId: "sess-main-missing-provider",
|
||||
updatedAt: Date.now(),
|
||||
},
|
||||
},
|
||||
});
|
||||
const res = await rpcReq(ws, "agent", {
|
||||
message: "hi",
|
||||
sessionKey: "main",
|
||||
deliver: true,
|
||||
idempotencyKey: "idem-agent-missing-provider",
|
||||
},
|
||||
});
|
||||
expectChannels(call, "whatsapp");
|
||||
expect(call.to).toBe("+1555");
|
||||
expect(call.deliver).toBe(true);
|
||||
expect(call.sessionId).toBe("sess-main-missing-provider");
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.code).toBe("INVALID_REQUEST");
|
||||
expect(res.error?.message).toContain("Channel is required");
|
||||
expect(vi.mocked(agentCommand)).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
testState.allowFrom = undefined;
|
||||
}
|
||||
});
|
||||
|
||||
test.each([
|
||||
|
||||
@@ -154,7 +154,7 @@ describe("gateway server agent", () => {
|
||||
setRegistry(emptyRegistry);
|
||||
});
|
||||
|
||||
test("agent falls back when last-channel plugin is unavailable", async () => {
|
||||
test("agent errors when deliver=true and last-channel plugin is unavailable", async () => {
|
||||
const registry = createRegistry([
|
||||
{
|
||||
pluginId: "msteams",
|
||||
@@ -175,9 +175,10 @@ describe("gateway server agent", () => {
|
||||
deliver: true,
|
||||
idempotencyKey: "idem-agent-last-msteams",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
|
||||
expectAgentRoutingCall({ channel: "whatsapp", deliver: true });
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.code).toBe("INVALID_REQUEST");
|
||||
expect(res.error?.message).toContain("Channel is required");
|
||||
expect(vi.mocked(agentCommand)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("agent accepts channel aliases (imsg/teams)", async () => {
|
||||
@@ -233,7 +234,7 @@ describe("gateway server agent", () => {
|
||||
expect(res.error?.code).toBe("INVALID_REQUEST");
|
||||
});
|
||||
|
||||
test("agent ignores webchat last-channel for routing", async () => {
|
||||
test("agent errors when deliver=true and last channel is webchat", async () => {
|
||||
testState.allowFrom = ["+1555"];
|
||||
await writeMainSessionEntry({
|
||||
sessionId: "sess-main-webchat",
|
||||
@@ -247,9 +248,10 @@ describe("gateway server agent", () => {
|
||||
deliver: true,
|
||||
idempotencyKey: "idem-agent-webchat",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
|
||||
expectAgentRoutingCall({ channel: "whatsapp", deliver: true });
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.error?.code).toBe("INVALID_REQUEST");
|
||||
expect(res.error?.message).toMatch(/Channel is required|runtime not initialized/);
|
||||
expect(vi.mocked(agentCommand)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("agent uses webchat for internal runs when last provider is webchat", async () => {
|
||||
|
||||
Reference in New Issue
Block a user