From 1211ce06d378e8002ad39d4aa352438d7c1b00ed Mon Sep 17 00:00:00 2001 From: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 23:49:20 +0000 Subject: [PATCH] fix(clawsweeper): address review for automerge-openclaw-openclaw-76304 (1) Co-authored-by: martingarramon <263922628+martingarramon@users.noreply.github.com> Co-authored-by: HollyChou <128659251+Hollychou924@users.noreply.github.com> --- .../slack/src/actions.reactions.test.ts | 89 ++++++++++++++++++- extensions/slack/src/actions.ts | 24 +++-- 2 files changed, 103 insertions(+), 10 deletions(-) diff --git a/extensions/slack/src/actions.reactions.test.ts b/extensions/slack/src/actions.reactions.test.ts index 4d885cc675b..9817f581f55 100644 --- a/extensions/slack/src/actions.reactions.test.ts +++ b/extensions/slack/src/actions.reactions.test.ts @@ -1,15 +1,29 @@ import type { WebClient } from "@slack/web-api"; import { describe, expect, it, vi } from "vitest"; -import { reactSlackMessage } from "./actions.js"; +import { reactSlackMessage, removeOwnSlackReactions, removeSlackReaction } from "./actions.js"; function createClient() { return { + auth: { + test: vi.fn(async () => ({ user_id: "UBOT" })), + }, reactions: { add: vi.fn(async () => ({})), + get: vi.fn(async () => ({ + message: { + reactions: [], + }, + })), + remove: vi.fn(async () => ({})), }, } as unknown as WebClient & { + auth: { + test: ReturnType; + }; reactions: { add: ReturnType; + get: ReturnType; + remove: ReturnType; }; }; } @@ -58,3 +72,76 @@ describe("reactSlackMessage", () => { }); }); }); + +describe("removeSlackReaction", () => { + it("treats no_reaction as idempotent success", async () => { + const client = createClient(); + client.reactions.remove.mockRejectedValueOnce(slackPlatformError("no_reaction")); + + await expect( + removeSlackReaction("C1", "123.456", ":white_check_mark:", { + client, + token: "xoxb-test", + }), + ).resolves.toBeUndefined(); + + expect(client.reactions.remove).toHaveBeenCalledWith({ + channel: "C1", + timestamp: "123.456", + name: "white_check_mark", + }); + }); + + it("propagates unrelated reaction remove errors", async () => { + const client = createClient(); + client.reactions.remove.mockRejectedValueOnce(slackPlatformError("invalid_name")); + + await expect( + removeSlackReaction("C1", "123.456", "not-an-emoji", { + client, + token: "xoxb-test", + }), + ).rejects.toMatchObject({ + data: { + error: "invalid_name", + }, + }); + }); +}); + +describe("removeOwnSlackReactions", () => { + it("removes own reactions through the idempotent remove helper", async () => { + const client = createClient(); + client.reactions.get.mockResolvedValueOnce({ + message: { + reactions: [ + { name: "thumbsup", users: ["UBOT", "U1"] }, + { name: "eyes", users: ["U2", "UBOT"] }, + { name: "wave", users: ["U2"] }, + ], + }, + }); + client.reactions.remove + .mockRejectedValueOnce(slackPlatformError("no_reaction")) + .mockResolvedValueOnce({}); + + await expect( + removeOwnSlackReactions("C1", "123.456", { + client, + token: "xoxb-test", + }), + ).resolves.toEqual(["thumbsup", "eyes"]); + + expect(client.reactions.remove).toHaveBeenCalledTimes(2); + expect(client.reactions.remove).toHaveBeenNthCalledWith(1, { + channel: "C1", + timestamp: "123.456", + name: "thumbsup", + }); + expect(client.reactions.remove).toHaveBeenNthCalledWith(2, { + channel: "C1", + timestamp: "123.456", + name: "eyes", + }); + }); +}); diff --git a/extensions/slack/src/actions.ts b/extensions/slack/src/actions.ts index b52c4b4aac7..5f0d2267b33 100644 --- a/extensions/slack/src/actions.ts +++ b/extensions/slack/src/actions.ts @@ -132,11 +132,18 @@ export async function removeSlackReaction( opts: SlackActionClientOpts = {}, ) { const client = await getClient(opts, "write"); - await client.reactions.remove({ - channel: channelId, - timestamp: messageId, - name: normalizeEmoji(emoji), - }); + try { + await client.reactions.remove({ + channel: channelId, + timestamp: messageId, + name: normalizeEmoji(emoji), + }); + } catch (err) { + if (hasSlackPlatformError(err, "no_reaction")) { + return; + } + throw err; + } } export async function removeOwnSlackReactions( @@ -163,10 +170,9 @@ export async function removeOwnSlackReactions( } await Promise.all( Array.from(toRemove, (name) => - client.reactions.remove({ - channel: channelId, - timestamp: messageId, - name, + removeSlackReaction(channelId, messageId, name, { + ...opts, + client, }), ), );