From 77b334a98456244f1d7e8d8f53754dd567a100b4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 30 May 2026 10:43:44 -0400 Subject: [PATCH] fix(mattermost): bound reaction cache clocks --- .../src/mattermost/reactions.test.ts | 96 ++++++++++++++++++- .../mattermost/src/mattermost/reactions.ts | 18 +++- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/extensions/mattermost/src/mattermost/reactions.test.ts b/extensions/mattermost/src/mattermost/reactions.test.ts index 5a75be19acc..a206cfffdb0 100644 --- a/extensions/mattermost/src/mattermost/reactions.test.ts +++ b/extensions/mattermost/src/mattermost/reactions.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { addMattermostReaction, removeMattermostReaction, @@ -15,6 +15,10 @@ describe("mattermost reactions", () => { resetMattermostReactionBotUserCacheForTests(); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + async function addReactionWithFetch(fetchMock: typeof fetch) { return addMattermostReaction({ cfg: createMattermostTestConfig(), @@ -104,4 +108,94 @@ describe("mattermost reactions", () => { expect(removeResult).toEqual({ ok: true }); expect(usersMeCalls).toHaveLength(1); }); + + it("does not reuse cached bot user ids while the process clock is invalid", async () => { + const cfg = createMattermostTestConfig(); + const firstFetch = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST1", + emojiName: "thumbsup", + userId: "BOT_OLD", + }); + const secondFetch = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST2", + emojiName: "thumbsup", + userId: "BOT_FRESH", + }); + const thirdFetch = createMattermostReactionFetchMock({ + mode: "add", + postId: "POST3", + emojiName: "thumbsup", + userId: "BOT_RECOVERED", + }); + + await expect( + addMattermostReaction({ + cfg, + postId: "POST1", + emojiName: "thumbsup", + fetchImpl: firstFetch, + }), + ).resolves.toEqual({ ok: true }); + + vi.spyOn(Date, "now").mockReturnValue(8_640_000_000_000_001); + await expect( + addMattermostReaction({ + cfg, + postId: "POST2", + emojiName: "thumbsup", + fetchImpl: secondFetch, + }), + ).resolves.toEqual({ ok: true }); + + vi.mocked(Date.now).mockReturnValue(1_000); + await expect( + addMattermostReaction({ + cfg, + postId: "POST3", + emojiName: "thumbsup", + fetchImpl: thirdFetch, + }), + ).resolves.toEqual({ ok: true }); + + const usersMeCalls = [ + ...firstFetch.mock.calls, + ...secondFetch.mock.calls, + ...thirdFetch.mock.calls, + ].filter((call) => requestUrl(call[0]).endsWith("/api/v4/users/me")); + expect(usersMeCalls).toHaveLength(3); + }); + + it("does not cache bot user ids when cache expiry would exceed the Date range", async () => { + vi.spyOn(Date, "now").mockReturnValue(8_640_000_000_000_000); + const cfg = createMattermostTestConfig(); + const fetchMock = createMattermostReactionFetchMock({ + mode: "both", + postId: "POST1", + emojiName: "thumbsup", + }); + + await expect( + addMattermostReaction({ + cfg, + postId: "POST1", + emojiName: "thumbsup", + fetchImpl: fetchMock, + }), + ).resolves.toEqual({ ok: true }); + await expect( + removeMattermostReaction({ + cfg, + postId: "POST1", + emojiName: "thumbsup", + fetchImpl: fetchMock, + }), + ).resolves.toEqual({ ok: true }); + + const usersMeCalls = fetchMock.mock.calls.filter((call) => + requestUrl(call[0]).endsWith("/api/v4/users/me"), + ); + expect(usersMeCalls).toHaveLength(2); + }); }); diff --git a/extensions/mattermost/src/mattermost/reactions.ts b/extensions/mattermost/src/mattermost/reactions.ts index 87028c00929..5d520f3ced3 100644 --- a/extensions/mattermost/src/mattermost/reactions.ts +++ b/extensions/mattermost/src/mattermost/reactions.ts @@ -1,3 +1,7 @@ +import { + asDateTimestampMs, + resolveExpiresAtMsFromDurationMs, +} from "openclaw/plugin-sdk/number-runtime"; import { isPrivateNetworkOptInEnabled } from "openclaw/plugin-sdk/ssrf-runtime"; import { resolveMattermostAccount } from "./accounts.js"; import { @@ -26,16 +30,24 @@ async function resolveBotUserId( client: MattermostClient, cacheKey: string, ): Promise { + const rawNow = Date.now(); + const now = asDateTimestampMs(rawNow); const cached = botUserIdCache.get(cacheKey); - if (cached && cached.expiresAt > Date.now()) { - return cached.userId; + if (cached) { + if (now !== undefined && cached.expiresAt > now) { + return cached.userId; + } + botUserIdCache.delete(cacheKey); } const me = await fetchMattermostMe(client); const userId = me?.id?.trim(); if (!userId) { return null; } - botUserIdCache.set(cacheKey, { userId, expiresAt: Date.now() + BOT_USER_CACHE_TTL_MS }); + const expiresAt = resolveExpiresAtMsFromDurationMs(BOT_USER_CACHE_TTL_MS, { nowMs: rawNow }); + if (expiresAt !== undefined) { + botUserIdCache.set(cacheKey, { userId, expiresAt }); + } return userId; }