From b668ffe7ca795eedf8bc14cddfbaafcd0ec07b5d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 30 May 2026 11:09:21 -0400 Subject: [PATCH] fix(slack): bound thread resolution cache clocks --- .../monitor/monitor.thread-resolution.test.ts | 91 +++++++++++++++---- .../slack/src/monitor/thread-resolution.ts | 29 +++++- 2 files changed, 97 insertions(+), 23 deletions(-) diff --git a/extensions/slack/src/monitor/monitor.thread-resolution.test.ts b/extensions/slack/src/monitor/monitor.thread-resolution.test.ts index cc354b528c0..9b407d58a93 100644 --- a/extensions/slack/src/monitor/monitor.thread-resolution.test.ts +++ b/extensions/slack/src/monitor/monitor.thread-resolution.test.ts @@ -1,8 +1,13 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import type { SlackMessageEvent } from "../types.js"; import { createSlackThreadTsResolver } from "./thread-resolution.js"; describe("createSlackThreadTsResolver", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + function makeThreadReplyMessage(ts: string): SlackMessageEvent { return { channel: "C1", @@ -53,25 +58,75 @@ describe("createSlackThreadTsResolver", () => { it("falls back to the default ttl when cacheTtlMs is non-finite", async () => { vi.useFakeTimers(); - try { - const historyMock = vi.fn().mockResolvedValue({ - messages: [{ ts: "1", thread_ts: "9" }], - }); - const resolver = createSlackThreadTsResolver({ - client: { conversations: { history: historyMock } } as never, - cacheTtlMs: Number.NaN, - maxSize: 5, - }); - const message = makeThreadReplyMessage("1"); + const historyMock = vi.fn().mockResolvedValue({ + messages: [{ ts: "1", thread_ts: "9" }], + }); + const resolver = createSlackThreadTsResolver({ + client: { conversations: { history: historyMock } } as never, + cacheTtlMs: Number.NaN, + maxSize: 5, + }); + const message = makeThreadReplyMessage("1"); - await resolver.resolve({ message, source: "message" }); - vi.advanceTimersByTime(60_001); - await resolver.resolve({ message, source: "message" }); + await resolver.resolve({ message, source: "message" }); + vi.advanceTimersByTime(60_001); + await resolver.resolve({ message, source: "message" }); - expect(historyMock).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } + expect(historyMock).toHaveBeenCalledTimes(2); + }); + + it("drops cached thread_ts lookups when the current clock is not a valid date timestamp", async () => { + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(1_700_000_000_000); + const historyMock = vi.fn().mockResolvedValue({ + messages: [{ ts: "1", thread_ts: "9" }], + }); + const resolver = createSlackThreadTsResolver({ + client: { conversations: { history: historyMock } } as never, + cacheTtlMs: 60_000, + maxSize: 5, + }); + const message = makeThreadReplyMessage("1"); + + await resolver.resolve({ message, source: "message" }); + nowSpy.mockReturnValue(Number.NaN); + await resolver.resolve({ message, source: "message" }); + + expect(historyMock).toHaveBeenCalledTimes(2); + }); + + it("does not cache thread_ts lookups when the expiry timestamp would exceed the valid date range", async () => { + vi.spyOn(Date, "now").mockReturnValue(8_640_000_000_000_000); + const historyMock = vi.fn().mockResolvedValue({ + messages: [{ ts: "1", thread_ts: "9" }], + }); + const resolver = createSlackThreadTsResolver({ + client: { conversations: { history: historyMock } } as never, + cacheTtlMs: 60_000, + maxSize: 5, + }); + const message = makeThreadReplyMessage("1"); + + await resolver.resolve({ message, source: "message" }); + await resolver.resolve({ message, source: "message" }); + + expect(historyMock).toHaveBeenCalledTimes(2); + }); + + it("preserves cacheTtlMs zero as a non-expiring cache entry", async () => { + const historyMock = vi.fn().mockResolvedValue({ + messages: [{ ts: "1", thread_ts: "9" }], + }); + const resolver = createSlackThreadTsResolver({ + client: { conversations: { history: historyMock } } as never, + cacheTtlMs: 0, + maxSize: 5, + }); + const message = makeThreadReplyMessage("1"); + + await resolver.resolve({ message, source: "message" }); + await resolver.resolve({ message, source: "message" }); + + expect(historyMock).toHaveBeenCalledTimes(1); }); it("falls back to the default max size when maxSize is non-finite", async () => { diff --git a/extensions/slack/src/monitor/thread-resolution.ts b/extensions/slack/src/monitor/thread-resolution.ts index 63547c8af72..f9ac6c1ede5 100644 --- a/extensions/slack/src/monitor/thread-resolution.ts +++ b/extensions/slack/src/monitor/thread-resolution.ts @@ -1,13 +1,17 @@ import type { WebClient as SlackWebClient } from "@slack/web-api"; import { pruneMapToMaxSize } from "openclaw/plugin-sdk/collection-runtime"; -import { parseFiniteNumber } from "openclaw/plugin-sdk/number-runtime"; +import { + asDateTimestampMs, + parseFiniteNumber, + resolveExpiresAtMsFromDurationMs, +} from "openclaw/plugin-sdk/number-runtime"; import { logVerbose, shouldLogVerbose } from "openclaw/plugin-sdk/runtime-env"; import { formatSlackError } from "../errors.js"; import type { SlackMessageEvent } from "../types.js"; type ThreadTsCacheEntry = { threadTs: string | null; - updatedAt: number; + expiresAt: number; }; const DEFAULT_THREAD_TS_CACHE_TTL_MS = 60_000; @@ -64,18 +68,33 @@ export function createSlackThreadTsResolver(params: { if (!entry) { return undefined; } - if (ttlMs > 0 && now - entry.updatedAt > ttlMs) { + if (entry.expiresAt === 0) { + cache.delete(key); + cache.set(key, entry); + return entry.threadTs; + } + const normalizedNow = asDateTimestampMs(now); + if ( + normalizedNow === undefined || + asDateTimestampMs(entry.expiresAt) === undefined || + entry.expiresAt <= normalizedNow + ) { cache.delete(key); return undefined; } cache.delete(key); - cache.set(key, { ...entry, updatedAt: now }); + cache.set(key, entry); return entry.threadTs; }; const setCached = (key: string, threadTs: string | null, now: number) => { + const expiresAt = ttlMs > 0 ? resolveExpiresAtMsFromDurationMs(ttlMs, { nowMs: now }) : 0; + if (expiresAt === undefined) { + cache.delete(key); + return; + } cache.delete(key); - cache.set(key, { threadTs, updatedAt: now }); + cache.set(key, { threadTs, expiresAt }); pruneMapToMaxSize(cache, maxSize); };