mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 01:51:24 +00:00
fix(telegram): make reaction handling soft-fail and message-id resilient (#20236)
* Telegram: soft-fail reactions and fallback to inbound message id * Telegram: soft-fail missing reaction message id * Update CHANGELOG.md --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -35,6 +35,11 @@ describe("readStringOrNumberParam", () => {
|
||||
const params = { chatId: " abc " };
|
||||
expect(readStringOrNumberParam(params, "chatId")).toBe("abc");
|
||||
});
|
||||
|
||||
it("accepts snake_case aliases for camelCase keys", () => {
|
||||
const params = { chat_id: "123" };
|
||||
expect(readStringOrNumberParam(params, "chatId")).toBe("123");
|
||||
});
|
||||
});
|
||||
|
||||
describe("readNumberParam", () => {
|
||||
@@ -47,6 +52,11 @@ describe("readNumberParam", () => {
|
||||
const params = { messageId: "42.9" };
|
||||
expect(readNumberParam(params, "messageId", { integer: true })).toBe(42);
|
||||
});
|
||||
|
||||
it("accepts snake_case aliases for camelCase keys", () => {
|
||||
const params = { message_id: "42" };
|
||||
expect(readNumberParam(params, "messageId")).toBe(42);
|
||||
});
|
||||
});
|
||||
|
||||
describe("required parameter validation", () => {
|
||||
|
||||
@@ -53,6 +53,24 @@ export function createActionGate<T extends Record<string, boolean | undefined>>(
|
||||
};
|
||||
}
|
||||
|
||||
function toSnakeCaseKey(key: string): string {
|
||||
return key
|
||||
.replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2")
|
||||
.replace(/([a-z0-9])([A-Z])/g, "$1_$2")
|
||||
.toLowerCase();
|
||||
}
|
||||
|
||||
function readParamRaw(params: Record<string, unknown>, key: string): unknown {
|
||||
if (Object.hasOwn(params, key)) {
|
||||
return params[key];
|
||||
}
|
||||
const snakeKey = toSnakeCaseKey(key);
|
||||
if (snakeKey !== key && Object.hasOwn(params, snakeKey)) {
|
||||
return params[snakeKey];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function readStringParam(
|
||||
params: Record<string, unknown>,
|
||||
key: string,
|
||||
@@ -69,7 +87,7 @@ export function readStringParam(
|
||||
options: StringParamOptions = {},
|
||||
) {
|
||||
const { required = false, trim = true, label = key, allowEmpty = false } = options;
|
||||
const raw = params[key];
|
||||
const raw = readParamRaw(params, key);
|
||||
if (typeof raw !== "string") {
|
||||
if (required) {
|
||||
throw new ToolInputError(`${label} required`);
|
||||
@@ -92,7 +110,7 @@ export function readStringOrNumberParam(
|
||||
options: { required?: boolean; label?: string } = {},
|
||||
): string | undefined {
|
||||
const { required = false, label = key } = options;
|
||||
const raw = params[key];
|
||||
const raw = readParamRaw(params, key);
|
||||
if (typeof raw === "number" && Number.isFinite(raw)) {
|
||||
return String(raw);
|
||||
}
|
||||
@@ -114,7 +132,7 @@ export function readNumberParam(
|
||||
options: { required?: boolean; label?: string; integer?: boolean } = {},
|
||||
): number | undefined {
|
||||
const { required = false, label = key, integer = false } = options;
|
||||
const raw = params[key];
|
||||
const raw = readParamRaw(params, key);
|
||||
let value: number | undefined;
|
||||
if (typeof raw === "number" && Number.isFinite(raw)) {
|
||||
value = raw;
|
||||
@@ -152,7 +170,7 @@ export function readStringArrayParam(
|
||||
options: StringParamOptions = {},
|
||||
) {
|
||||
const { required = false, label = key } = options;
|
||||
const raw = params[key];
|
||||
const raw = readParamRaw(params, key);
|
||||
if (Array.isArray(raw)) {
|
||||
const values = raw
|
||||
.filter((entry) => typeof entry === "string")
|
||||
|
||||
@@ -238,7 +238,19 @@ function buildSendSchema(options: {
|
||||
|
||||
function buildReactionSchema() {
|
||||
return {
|
||||
messageId: Type.Optional(Type.String()),
|
||||
messageId: Type.Optional(
|
||||
Type.String({
|
||||
description:
|
||||
"Target message id for reaction. For Telegram, if omitted, defaults to the current inbound message id when available.",
|
||||
}),
|
||||
),
|
||||
message_id: Type.Optional(
|
||||
Type.String({
|
||||
// Intentional duplicate alias for tool-schema discoverability in LLMs.
|
||||
description:
|
||||
"snake_case alias of messageId. For Telegram, if omitted, defaults to the current inbound message id when available.",
|
||||
}),
|
||||
),
|
||||
emoji: Type.Optional(Type.String()),
|
||||
remove: Type.Optional(Type.Boolean()),
|
||||
targetAuthor: Type.Optional(Type.String()),
|
||||
@@ -425,6 +437,7 @@ type MessageToolOptions = {
|
||||
currentChannelId?: string;
|
||||
currentChannelProvider?: string;
|
||||
currentThreadTs?: string;
|
||||
currentMessageId?: string | number;
|
||||
replyToMode?: "off" | "first" | "all";
|
||||
hasRepliedRef?: { value: boolean };
|
||||
sandboxRoot?: string;
|
||||
@@ -633,17 +646,23 @@ export function createMessageTool(options?: MessageToolOptions): AnyAgentTool {
|
||||
clientDisplayName: "agent",
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
};
|
||||
const hasCurrentMessageId =
|
||||
typeof options?.currentMessageId === "number" ||
|
||||
(typeof options?.currentMessageId === "string" &&
|
||||
options.currentMessageId.trim().length > 0);
|
||||
|
||||
const toolContext =
|
||||
options?.currentChannelId ||
|
||||
options?.currentChannelProvider ||
|
||||
options?.currentThreadTs ||
|
||||
hasCurrentMessageId ||
|
||||
options?.replyToMode ||
|
||||
options?.hasRepliedRef
|
||||
? {
|
||||
currentChannelId: options?.currentChannelId,
|
||||
currentChannelProvider: options?.currentChannelProvider,
|
||||
currentThreadTs: options?.currentThreadTs,
|
||||
currentMessageId: options?.currentMessageId,
|
||||
replyToMode: options?.replyToMode,
|
||||
hasRepliedRef: options?.hasRepliedRef,
|
||||
// Direct tool invocations should not add cross-context decoration.
|
||||
|
||||
@@ -102,6 +102,46 @@ describe("handleTelegramAction", () => {
|
||||
await expectReactionAdded("extensive");
|
||||
});
|
||||
|
||||
it("accepts snake_case message_id for reactions", async () => {
|
||||
const cfg = {
|
||||
channels: { telegram: { botToken: "tok", reactionLevel: "minimal" } },
|
||||
} as OpenClawConfig;
|
||||
await handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
message_id: "456",
|
||||
emoji: "✅",
|
||||
},
|
||||
cfg,
|
||||
);
|
||||
expect(reactMessageTelegram).toHaveBeenCalledWith(
|
||||
"123",
|
||||
456,
|
||||
"✅",
|
||||
expect.objectContaining({ token: "tok", remove: false }),
|
||||
);
|
||||
});
|
||||
|
||||
it("soft-fails when messageId is missing", async () => {
|
||||
const cfg = {
|
||||
channels: { telegram: { botToken: "tok", reactionLevel: "minimal" } },
|
||||
} as OpenClawConfig;
|
||||
const result = await handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
emoji: "✅",
|
||||
},
|
||||
cfg,
|
||||
);
|
||||
expect(result.details).toMatchObject({
|
||||
ok: false,
|
||||
reason: "missing_message_id",
|
||||
});
|
||||
expect(reactMessageTelegram).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("removes reactions on empty emoji", async () => {
|
||||
const cfg = {
|
||||
channels: { telegram: { botToken: "tok", reactionLevel: "minimal" } },
|
||||
@@ -177,18 +217,10 @@ describe("handleTelegramAction", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
level: "off" as const,
|
||||
expectedMessage: /Telegram agent reactions disabled.*reactionLevel="off"/,
|
||||
},
|
||||
{
|
||||
level: "ack" as const,
|
||||
expectedMessage: /Telegram agent reactions disabled.*reactionLevel="ack"/,
|
||||
},
|
||||
])("blocks reactions when reactionLevel is $level", async ({ level, expectedMessage }) => {
|
||||
await expect(
|
||||
handleTelegramAction(
|
||||
it.each(["off", "ack"] as const)(
|
||||
"soft-fails reactions when reactionLevel is %s",
|
||||
async (level) => {
|
||||
const result = await handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
@@ -196,11 +228,15 @@ describe("handleTelegramAction", () => {
|
||||
emoji: "✅",
|
||||
},
|
||||
reactionConfig(level),
|
||||
),
|
||||
).rejects.toThrow(expectedMessage);
|
||||
});
|
||||
);
|
||||
expect(result.details).toMatchObject({
|
||||
ok: false,
|
||||
reason: "disabled",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("also respects legacy actions.reactions gating", async () => {
|
||||
it("soft-fails when reactions are disabled via actions.reactions", async () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
telegram: {
|
||||
@@ -210,17 +246,19 @@ describe("handleTelegramAction", () => {
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
await expect(
|
||||
handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
messageId: "456",
|
||||
emoji: "✅",
|
||||
},
|
||||
cfg,
|
||||
),
|
||||
).rejects.toThrow(/Telegram reactions are disabled via actions.reactions/);
|
||||
const result = await handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
messageId: "456",
|
||||
emoji: "✅",
|
||||
},
|
||||
cfg,
|
||||
);
|
||||
expect(result.details).toMatchObject({
|
||||
ok: false,
|
||||
reason: "disabled",
|
||||
});
|
||||
});
|
||||
|
||||
it("sends a text message", async () => {
|
||||
@@ -634,18 +672,20 @@ describe("handleTelegramAction per-account gating", () => {
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
|
||||
await expect(
|
||||
handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
messageId: 1,
|
||||
emoji: "👀",
|
||||
accountId: "media",
|
||||
},
|
||||
cfg,
|
||||
),
|
||||
).rejects.toThrow(/reactions are disabled via actions.reactions/i);
|
||||
const result = await handleTelegramAction(
|
||||
{
|
||||
action: "react",
|
||||
chatId: "123",
|
||||
messageId: 1,
|
||||
emoji: "👀",
|
||||
accountId: "media",
|
||||
},
|
||||
cfg,
|
||||
);
|
||||
expect(result.details).toMatchObject({
|
||||
ok: false,
|
||||
reason: "disabled",
|
||||
});
|
||||
});
|
||||
|
||||
it("allows account to explicitly re-enable top-level disabled reaction gate", async () => {
|
||||
|
||||
@@ -94,42 +94,69 @@ export async function handleTelegramAction(
|
||||
const isActionEnabled = createTelegramActionGate({ cfg, accountId });
|
||||
|
||||
if (action === "react") {
|
||||
// Check reaction level first
|
||||
// All react failures return soft results (jsonResult with ok:false) instead
|
||||
// of throwing, because hard tool errors can trigger model re-generation
|
||||
// loops and duplicate content.
|
||||
const reactionLevelInfo = resolveTelegramReactionLevel({
|
||||
cfg,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
if (!reactionLevelInfo.agentReactionsEnabled) {
|
||||
throw new Error(
|
||||
`Telegram agent reactions disabled (reactionLevel="${reactionLevelInfo.level}"). ` +
|
||||
`Set channels.telegram.reactionLevel to "minimal" or "extensive" to enable.`,
|
||||
);
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
reason: "disabled",
|
||||
hint: `Telegram agent reactions disabled (reactionLevel="${reactionLevelInfo.level}"). Do not retry.`,
|
||||
});
|
||||
}
|
||||
// Also check the existing action gate for backward compatibility
|
||||
if (!isActionEnabled("reactions")) {
|
||||
throw new Error("Telegram reactions are disabled via actions.reactions.");
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
reason: "disabled",
|
||||
hint: "Telegram reactions are disabled via actions.reactions. Do not retry.",
|
||||
});
|
||||
}
|
||||
const chatId = readStringOrNumberParam(params, "chatId", {
|
||||
required: true,
|
||||
});
|
||||
const messageId = readNumberParam(params, "messageId", {
|
||||
required: true,
|
||||
integer: true,
|
||||
});
|
||||
if (typeof messageId !== "number" || !Number.isFinite(messageId) || messageId <= 0) {
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
reason: "missing_message_id",
|
||||
hint: "Telegram reaction requires a valid messageId (or inbound context fallback). Do not retry.",
|
||||
});
|
||||
}
|
||||
const { emoji, remove, isEmpty } = readReactionParams(params, {
|
||||
removeErrorMessage: "Emoji is required to remove a Telegram reaction.",
|
||||
});
|
||||
const token = resolveTelegramToken(cfg, { accountId }).token;
|
||||
if (!token) {
|
||||
throw new Error(
|
||||
"Telegram bot token missing. Set TELEGRAM_BOT_TOKEN or channels.telegram.botToken.",
|
||||
);
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
reason: "missing_token",
|
||||
hint: "Telegram bot token missing. Do not retry.",
|
||||
});
|
||||
}
|
||||
let reactionResult: Awaited<ReturnType<typeof reactMessageTelegram>>;
|
||||
try {
|
||||
reactionResult = await reactMessageTelegram(chatId ?? "", messageId ?? 0, emoji ?? "", {
|
||||
token,
|
||||
remove,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
} catch (err) {
|
||||
const isInvalid = String(err).includes("REACTION_INVALID");
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
reason: isInvalid ? "REACTION_INVALID" : "error",
|
||||
emoji,
|
||||
hint: isInvalid
|
||||
? "This emoji is not supported for Telegram reactions. Add it to your reaction disallow list so you do not try it again."
|
||||
: "Reaction failed. Do not retry.",
|
||||
});
|
||||
}
|
||||
const reactionResult = await reactMessageTelegram(chatId ?? "", messageId ?? 0, emoji ?? "", {
|
||||
token,
|
||||
remove,
|
||||
accountId: accountId ?? undefined,
|
||||
});
|
||||
if (!reactionResult.ok) {
|
||||
return jsonResult({
|
||||
ok: false,
|
||||
|
||||
Reference in New Issue
Block a user