diff --git a/CHANGELOG.md b/CHANGELOG.md index d8f15ff26a7..15aa0bb6590 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Models/UI: hide unauthenticated providers from the default Web chat, `/models`, and model setup pickers while keeping explicit full-catalog browse paths through `view: "all"`, `/models all`, and `models list --all`. Fixes #74423. Thanks @guarismo and @SymbolStar. +- Slack/reactions: treat duplicate `already_reacted` responses as idempotent success so repeated agent reaction adds no longer surface as tool failures. Fixes #69005. Thanks @shipitsteven and @martingarramon. - Slack/tools: expose `fileId` in the shared message tool schema so `download-file` can receive Slack attachment IDs from inbound placeholders. Fixes #45574. Thanks @chadvegas. - Exec: reject invalid per-call `host` values instead of silently falling back to the default target, so hostname-like values fail before commands run. Fixes #74426. Thanks @scr00ge-00 and @vyctorbrzezowski. - Google/Gemini: send non-empty placeholder content when a Gemini run is triggered with empty or filtered user content, avoiding `contents is not specified` API errors. Thanks @CaoYuhaoCarl. diff --git a/extensions/slack/src/action-runtime.test.ts b/extensions/slack/src/action-runtime.test.ts index 0f4ae86f3f0..29ad2a610e5 100644 --- a/extensions/slack/src/action-runtime.test.ts +++ b/extensions/slack/src/action-runtime.test.ts @@ -115,7 +115,7 @@ describe("handleSlackAction", () => { { name: "raw channel id", channelId: "C1" }, { name: "channel: prefixed id", channelId: "channel:C1" }, ])("adds reactions for $name", async ({ channelId }) => { - await handleSlackAction( + const result = await handleSlackAction( { action: "react", channelId, @@ -130,6 +130,10 @@ describe("handleSlackAction", () => { "✅", expect.objectContaining({ cfg: expect.any(Object) }), ); + expect(JSON.parse((result.content?.[0] as { type: "text"; text: string }).text)).toEqual({ + ok: true, + added: "✅", + }); }); it("removes reactions on empty emoji", async () => { diff --git a/extensions/slack/src/actions.reactions.test.ts b/extensions/slack/src/actions.reactions.test.ts new file mode 100644 index 00000000000..4d885cc675b --- /dev/null +++ b/extensions/slack/src/actions.reactions.test.ts @@ -0,0 +1,60 @@ +import type { WebClient } from "@slack/web-api"; +import { describe, expect, it, vi } from "vitest"; +import { reactSlackMessage } from "./actions.js"; + +function createClient() { + return { + reactions: { + add: vi.fn(async () => ({})), + }, + } as unknown as WebClient & { + reactions: { + add: ReturnType; + }; + }; +} + +function slackPlatformError(error: string) { + return Object.assign(new Error(`An API error occurred: ${error}`), { + data: { + ok: false, + error, + }, + }); +} + +describe("reactSlackMessage", () => { + it("treats already_reacted as idempotent success", async () => { + const client = createClient(); + client.reactions.add.mockRejectedValueOnce(slackPlatformError("already_reacted")); + + await expect( + reactSlackMessage("C1", "123.456", ":white_check_mark:", { + client, + token: "xoxb-test", + }), + ).resolves.toBeUndefined(); + + expect(client.reactions.add).toHaveBeenCalledWith({ + channel: "C1", + timestamp: "123.456", + name: "white_check_mark", + }); + }); + + it("propagates unrelated reaction add errors", async () => { + const client = createClient(); + client.reactions.add.mockRejectedValueOnce(slackPlatformError("invalid_name")); + + await expect( + reactSlackMessage("C1", "123.456", "not-an-emoji", { + client, + token: "xoxb-test", + }), + ).rejects.toMatchObject({ + data: { + error: "invalid_name", + }, + }); + }); +}); diff --git a/extensions/slack/src/actions.ts b/extensions/slack/src/actions.ts index ca11ea3c4eb..2bfc4247ec2 100644 --- a/extensions/slack/src/actions.ts +++ b/extensions/slack/src/actions.ts @@ -77,6 +77,17 @@ function normalizeEmoji(raw: string) { return trimmed.replace(/^:+|:+$/g, ""); } +function hasSlackPlatformError(err: unknown, code: string): boolean { + if (!err || typeof err !== "object") { + return false; + } + const data = (err as { data?: unknown }).data; + if (!data || typeof data !== "object") { + return false; + } + return (data as { error?: unknown }).error === code; +} + async function getClient(opts: SlackActionClientOpts = {}, mode: "read" | "write" = "read") { if (opts.client) { return opts.client; @@ -100,11 +111,18 @@ export async function reactSlackMessage( opts: SlackActionClientOpts = {}, ) { const client = await getClient(opts, "write"); - await client.reactions.add({ - channel: channelId, - timestamp: messageId, - name: normalizeEmoji(emoji), - }); + try { + await client.reactions.add({ + channel: channelId, + timestamp: messageId, + name: normalizeEmoji(emoji), + }); + } catch (err) { + if (hasSlackPlatformError(err, "already_reacted")) { + return; + } + throw err; + } } export async function removeSlackReaction(