diff --git a/extensions/slack/src/monitor/auth.test.ts b/extensions/slack/src/monitor/auth.test.ts index a7845cfc366e..aea52685411c 100644 --- a/extensions/slack/src/monitor/auth.test.ts +++ b/extensions/slack/src/monitor/auth.test.ts @@ -1,3 +1,4 @@ +import { WebAPIPlatformError, WebAPIRequestError } from "@slack/web-api"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { SlackMonitorContext } from "./context.js"; @@ -6,6 +7,7 @@ let authorizeSlackBotRoomMessage: typeof import("./auth.js").authorizeSlackBotRo let authorizeSlackSystemEventSender: typeof import("./auth.js").authorizeSlackSystemEventSender; let resolveSlackEffectiveAllowFrom: typeof import("./auth.js").resolveSlackEffectiveAllowFrom; let resolveSlackCommandIngress: typeof import("./auth.js").resolveSlackCommandIngress; +let SlackSystemEventAuthRetryError: typeof import("./auth.js").SlackSystemEventAuthRetryError; beforeAll(async () => { ({ @@ -13,6 +15,7 @@ beforeAll(async () => { authorizeSlackSystemEventSender, resolveSlackCommandIngress, resolveSlackEffectiveAllowFrom, + SlackSystemEventAuthRetryError, } = await import("./auth.js")); }); @@ -47,9 +50,11 @@ function makeSlackCtx(allowFrom: string[]): SlackMonitorContext { function makeAuthorizeCtx(params?: { allowFrom?: string[]; + allowNameMatching?: boolean; channelsConfig?: Record; dmPolicy?: SlackMonitorContext["dmPolicy"]; - resolveUserName?: (userId: string) => Promise<{ name?: string }>; + isChannelAllowed?: () => boolean; + resolveUserName?: (userId: string) => Promise<{ name?: string; error?: unknown }>; resolveChannelName?: ( channelId: string, ) => Promise<{ name?: string; type?: "im" | "mpim" | "channel" | "group" }>; @@ -60,7 +65,7 @@ function makeAuthorizeCtx(params?: { accountId: "main", dmPolicy: params?.dmPolicy ?? "open", dmEnabled: true, - allowNameMatching: false, + allowNameMatching: params?.allowNameMatching ?? false, channelsConfig: params?.channelsConfig ?? {}, channelsConfigKeys: Object.keys(params?.channelsConfig ?? {}), defaultRequireMention: true, @@ -68,7 +73,7 @@ function makeAuthorizeCtx(params?: { kind: "workspace", teamId: "T_MAIN", }, - isChannelAllowed: vi.fn(() => true), + isChannelAllowed: vi.fn(params?.isChannelAllowed ?? (() => true)), resolveUserName: vi.fn( params?.resolveUserName ?? ((_) => Promise.resolve({ name: undefined })), ), @@ -100,6 +105,7 @@ const deniedChannel: AuthorizeExpected = { channelName: "general", }; const channelUsers = { C1: { users: ["U_ALLOWED"] } }; +const resolveUserNameError = (error: unknown) => async () => ({ error }); function interactiveRequest( senderId: string, @@ -220,6 +226,68 @@ describe("resolveSlackEffectiveAllowFrom", () => { }); describe("authorizeSlackSystemEventSender", () => { + it("checks the channel gate and stable ID before resolving a member name", async () => { + const deniedCtx = makeAuthorizeCtx({ + allowNameMatching: true, + channelsConfig: { C1: { users: ["alice"] } }, + isChannelAllowed: () => false, + }); + await expect( + authorizeSlackSystemEventSender({ + ctx: deniedCtx, + senderId: "U_DENIED", + channelId: "C1", + retryNameLookup: true, + }), + ).resolves.toMatchObject({ allowed: false, reason: "channel-not-allowed" }); + expect(deniedCtx.resolveUserName).not.toHaveBeenCalled(); + + const allowedCtx = makeAuthorizeCtx({ + allowNameMatching: true, + channelsConfig: channelUsers, + }); + await expect( + authorizeSlackSystemEventSender({ + ctx: allowedCtx, + senderId: "U_ALLOWED", + channelId: "C1", + retryNameLookup: true, + }), + ).resolves.toEqual(allowedChannel); + expect(allowedCtx.resolveUserName).not.toHaveBeenCalled(); + }); + + it("retries only transient direct-name lookup failures", async () => { + const authorize = (error: unknown) => + authorizeSlackSystemEventSender({ + ctx: makeAuthorizeCtx({ + allowNameMatching: true, + channelsConfig: { C1: { users: ["alice"] } }, + resolveUserName: resolveUserNameError(error), + }), + senderId: "U_PENDING", + channelId: "C1", + retryNameLookup: true, + }); + for (const error of [ + new WebAPIRequestError(Object.assign(new Error("socket reset"), { code: "ECONNRESET" })), + new WebAPIPlatformError({ ok: false, error: "service_unavailable" }), + ]) { + await expect(authorize(error)).rejects.toBeInstanceOf(SlackSystemEventAuthRetryError); + } + + for (const error of [ + new WebAPIPlatformError({ ok: false, error: "user_not_found" }), + new WebAPIRequestError(new DOMException("request was canceled", "AbortError")), + new TypeError("invalid URL"), + ]) { + await expect(authorize(error)).resolves.toMatchObject({ + allowed: false, + reason: "sender-not-channel-allowed", + }); + } + }); + it.each([ [ "ignores non-decimal channel member cache ttl env values", diff --git a/extensions/slack/src/monitor/auth.ts b/extensions/slack/src/monitor/auth.ts index 32442020ff30..6c6d2c3779a7 100644 --- a/extensions/slack/src/monitor/auth.ts +++ b/extensions/slack/src/monitor/auth.ts @@ -4,7 +4,6 @@ import { type ChannelIngressIdentifierKind, type ChannelIngressPolicyInput, type ChannelIngressStateInput, - type ChannelIngressDecision, createChannelIngressResolver, defineStableChannelIngressIdentity, readChannelIngressStoreAllowFromForDmPolicy, @@ -28,6 +27,7 @@ import { resolveSlackChannelConfig } from "./channel-config.js"; import { inferSlackChannelType } from "./channel-type.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js"; import type { SlackEventScope } from "./event-scope.js"; +import { isTransientSlackThreadLookupError } from "./thread-resolution.js"; type SlackChannelMembersCacheEntry = { expiresAtMs: number; @@ -36,18 +36,8 @@ type SlackChannelMembersCacheEntry = { }; type SlackIngressChannelType = "im" | "mpim" | "channel" | "group"; -type SlackSystemEventAuthorization = - | { - allowed: true; - channelType?: SlackIngressChannelType; - channelName?: string; - } - | { - allowed: false; - reason: string; - channelType?: SlackIngressChannelType; - channelName?: string; - }; +type SlackSystemEventAuthorization = ({ allowed: true } | { allowed: false; reason: string }) & + Partial<{ channelType: SlackIngressChannelType; channelName: string }>; const slackChannelMembersCache = new WeakMap< SlackMonitorContext, @@ -59,6 +49,7 @@ const SLACK_CHANNEL_ID = "slack"; const SLACK_USER_NAME_KIND = "plugin:slack-user-name" as const satisfies ChannelIngressIdentifierKind; +export class SlackSystemEventAuthRetryError extends Error {} function normalizeSlackUserId(raw?: string | null): string { const value = (raw ?? "").trim().toLowerCase(); if (!value) { @@ -477,7 +468,9 @@ async function decideSlackSystemIngress(params: { ownerAllowFromLower: string[]; channelUsers?: Array; interactiveEvent: boolean; -}): Promise { + retryNameLookup?: boolean; + eventScope?: SlackEventScope; +}) { const isDirectMessage = params.channelType === "im"; const isGroupDm = params.channelType === "mpim"; const teamId = params.teamId ?? params.ctx.teamId; @@ -514,13 +507,16 @@ async function decideSlackSystemIngress(params: { } return params.channelId ? ["*"] : wildcardWhenOpen(ownerAllowFromLower); })(); - const result = await createSlackIngressResolver(params.ctx).message({ - subject: createSlackIngressSubject({ + const subject = (senderName?: string) => + createSlackIngressSubject({ senderId: params.senderId, - senderName: params.senderName, + senderName, teamId, workspaceScoped: !allowUnscoped, - }), + }); + const resolver = createSlackIngressResolver(params.ctx); + const input: Parameters[0] = { + subject: subject(params.senderName), conversation: { kind: slackIngressConversationKind(params.channelType), id: params.channelId ?? "slack-system", @@ -553,7 +549,23 @@ async function decideSlackSystemIngress(params: { commandOwnerAllowFrom: ownerAllowFrom, } : undefined, - }); + }; + const result = await resolver.message(input); + if ( + result.ingress.decision !== "allow" && + params.retryNameLookup && + result.state.allowlists[isDirectMessage ? "dm" : "group"].normalizedEntries.some( + (entry) => entry.kind === SLACK_USER_NAME_KIND, + ) + ) { + const lookup = await params.ctx.resolveUserName(params.senderId, params.eventScope); + if (lookup.error && isTransientSlackThreadLookupError(lookup.error)) { + throw new SlackSystemEventAuthRetryError(formatErrorMessage(lookup.error)); + } + if (lookup.name) { + return (await resolver.message({ ...input, subject: subject(lookup.name) })).ingress; + } + } return result.ingress; } @@ -564,6 +576,7 @@ export async function authorizeSlackSystemEventSender(params: { channelType?: string | null; eventScope?: SlackEventScope; expectedSenderId?: string; + retryNameLookup?: boolean; /** When true, requires expectedSenderId, rejects ambiguous channel types, * and applies interactive-only owner allowFrom checks without changing the * open-by-default channel behavior when no allowlists are configured. */ @@ -637,10 +650,9 @@ export async function authorizeSlackSystemEventSender(params: { } } - const senderInfo: { name?: string } = await params.ctx - .resolveUserName(senderId, params.eventScope) - .catch(() => ({})); - const senderName = senderInfo.name; + const senderInfo = params.retryNameLookup + ? undefined + : await params.ctx.resolveUserName(senderId, params.eventScope); const ingressChannelType = channelType ?? "channel"; if (ingressChannelType === "im") { @@ -671,12 +683,14 @@ export async function authorizeSlackSystemEventSender(params: { ctx: params.ctx, teamId: params.eventScope?.teamId ?? params.ctx.teamId, senderId, - senderName, + senderName: senderInfo?.name, channelType: ingressChannelType, channelId, ownerAllowFromLower: allowFromLower, channelUsers: channelConfig?.users, interactiveEvent: params.interactiveEvent === true, + retryNameLookup: params.retryNameLookup && params.ctx.allowNameMatching, + eventScope: params.eventScope, }); if (decision.decision === "allow") { return { diff --git a/extensions/slack/src/monitor/context.ts b/extensions/slack/src/monitor/context.ts index 51957f07f7e2..9092f5c9f5d2 100644 --- a/extensions/slack/src/monitor/context.ts +++ b/extensions/slack/src/monitor/context.ts @@ -59,6 +59,7 @@ type SlackChannelCacheEntry = { metadataLoaded: boolean; }; +type SlackUserInfo = { name?: string; error?: unknown }; const SLACK_CHANNEL_CACHE_MAX_ENTRIES = 1024; const SLACK_USER_CACHE_MAX_ENTRIES = 2048; const SLACK_CHANNEL_DENIAL_WARNING_TTL_MS = 5 * 60_000; @@ -137,7 +138,7 @@ export type SlackMonitorContext = { channelId: string | null | undefined, eventScope?: SlackEventScope, ) => SlackMessageEvent["channel_type"] | undefined; - resolveUserName: (userId: string, eventScope?: SlackEventScope) => Promise<{ name?: string }>; + resolveUserName: (userId: string, eventScope?: SlackEventScope) => Promise; setSlackThreadStatus: (params: { channelId: string; threadTs?: string; @@ -340,8 +341,8 @@ export function createSlackMonitorContext(params: { const entry = { name }; writeLruMapEntry(userCache, cacheKey, entry, SLACK_USER_CACHE_MAX_ENTRIES); return entry; - } catch { - return {}; + } catch (error) { + return { error }; } }; diff --git a/extensions/slack/src/monitor/events/members.test.ts b/extensions/slack/src/monitor/events/members.test.ts index e42aa6906529..37a281c3916f 100644 --- a/extensions/slack/src/monitor/events/members.test.ts +++ b/extensions/slack/src/monitor/events/members.test.ts @@ -156,6 +156,33 @@ describe("registerSlackMemberEvents", () => { ); }); + it("uses the stable user ID when the post-auth name lookup fails", async () => { + const harness = initSlackHarness({ + channelType: "channel", + channelUsers: ["U1"], + }); + const resolveUserName = vi.fn(async () => ({ error: new Error("users.info failed") })); + harness.ctx.resolveUserName = resolveUserName; + registerSlackMemberEvents({ ctx: harness.ctx }); + const handler = harness.getHandler("member_joined_channel"); + if (!handler) { + throw new Error("expected Slack member joined handler"); + } + + await handler({ + event: makeMemberEvent({ channel: "C1", user: "U1" }), + body: { event_id: "Ev-member-id-fallback" }, + }); + + expect(resolveUserName).toHaveBeenCalledOnce(); + expect(memberMocks.enqueue).toHaveBeenCalledWith( + "Slack: U1 joined #general.", + expect.objectContaining({ + contextKey: "slack:member:joined:C1:U1:Ev-member-id-fallback", + }), + ); + }); + it("keeps enterprise member events isolated by listener workspace", async () => { const harness = initSlackHarness(); harness.ctx.installationIdentity = { diff --git a/extensions/slack/src/monitor/events/members.ts b/extensions/slack/src/monitor/events/members.ts index 3670c6b5b35f..6cf2e47814dd 100644 --- a/extensions/slack/src/monitor/events/members.ts +++ b/extensions/slack/src/monitor/events/members.ts @@ -3,6 +3,7 @@ import type { AllMiddlewareArgs, SlackEventMiddlewareArgs } from "@slack/bolt"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { danger } from "openclaw/plugin-sdk/runtime-env"; import { enqueueSystemEvent } from "openclaw/plugin-sdk/system-event-runtime"; +import { SlackSystemEventAuthRetryError } from "../auth.js"; import type { SlackMonitorContext } from "../context.js"; import type { SlackMemberChannelEvent } from "../types.js"; import { @@ -66,6 +67,9 @@ export function registerSlackMemberEvents(params: { ctx.runtime.error?.( danger(`slack ${paramsLocal.verb} handler failed: ${formatErrorMessage(err)}`), ); + if (err instanceof SlackSystemEventAuthRetryError) { + throw err; + } } }; diff --git a/extensions/slack/src/monitor/events/system-event-context.ts b/extensions/slack/src/monitor/events/system-event-context.ts index 58f6ced1019a..3a510d964202 100644 --- a/extensions/slack/src/monitor/events/system-event-context.ts +++ b/extensions/slack/src/monitor/events/system-event-context.ts @@ -26,6 +26,7 @@ export async function authorizeAndResolveSlackSystemEventContext(params: { channelId, channelType, eventScope: params.eventScope, + retryNameLookup: eventKind.startsWith("member-"), }); if (!auth.allowed) { logVerbose( diff --git a/extensions/slack/src/monitor/ingress.test.ts b/extensions/slack/src/monitor/ingress.test.ts index f6306541afcd..9c6a57b434a1 100644 --- a/extensions/slack/src/monitor/ingress.test.ts +++ b/extensions/slack/src/monitor/ingress.test.ts @@ -4,19 +4,22 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { App, type Receiver, type ReceiverEvent } from "@slack/bolt"; +import type { WebClientOptions } from "@slack/web-api"; import type { ChannelIngressQueue } from "openclaw/plugin-sdk/channel-outbound"; +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import type { PluginJsonValue } from "openclaw/plugin-sdk/plugin-entry"; import { closeOpenClawStateDatabaseForTest, createChannelIngressQueueForTests, } from "openclaw/plugin-sdk/plugin-state-test-runtime"; +import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env"; import { peekSystemEventEntries, resetSystemEventsForTest, } from "openclaw/plugin-sdk/system-event-runtime"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { createSlackMonitorContext } from "./context.js"; import { registerSlackMemberEvents } from "./events/members.js"; -import { createSlackSystemEventTestHarness } from "./events/system-event-test-harness.js"; import { createSlackDurableIngress, resolveSlackIngressTurnLifecycle } from "./ingress.js"; type SlackIngressQueue = NonNullable[0]["queue"]>; @@ -86,7 +89,11 @@ function createReceiverHarness() { function createReceiverEvent( eventId: string, ack = vi.fn(async () => {}), - options: { retryNum?: number; ts?: string; event?: Record } = {}, + options: { + retryNum?: number; + ts?: string; + event?: Record; + } = {}, ): ReceiverEvent { return { body: createSlackEnvelope(eventId, options.ts, options.event), @@ -108,7 +115,8 @@ function createMemberEvent(type: "member_joined_channel" | "member_left_channel" function attachBoltMemberIngress(params: { queue: ChannelIngressQueue; trackEvent: () => void; - resolveUserName?: (userId: string) => Promise<{ name?: string }>; + usersInfo?: App["client"]["users"]["info"]; + usersInfoFetch?: NonNullable; pollIntervalMs?: number; }) { const ingress = createSlackDurableIngress({ @@ -126,15 +134,73 @@ function attachBoltMemberIngress(params: { botUserId: "U_BOT", teamId: "T_TEST", }), + ...(params.usersInfoFetch + ? { + clientOptions: { + fetch: params.usersInfoFetch, + retryConfig: { retries: 0 }, + slackApiUrl: "https://slack.test/api/", + }, + } + : {}), convoStore: false, ignoreSelf: false, }); - const memberHarness = createSlackSystemEventTestHarness({ channelType: "channel" }); - memberHarness.ctx.app = app; - if (params.resolveUserName) { - memberHarness.ctx.resolveUserName = params.resolveUserName; + vi.spyOn(app.client.conversations, "info").mockResolvedValue({ + ok: true, + channel: { id: "C_TEST", name: "general", is_channel: true }, + }); + if (!params.usersInfoFetch) { + vi.spyOn(app.client.users, "info").mockImplementation( + params.usersInfo ?? + (async () => ({ + ok: true, + user: { id: "U_TEST", name: "alice" }, + })), + ); } - registerSlackMemberEvents({ ctx: memberHarness.ctx, trackEvent: params.trackEvent }); + const ctx = createSlackMonitorContext({ + cfg: {} as OpenClawConfig, + accountId: "default", + botToken: "xoxb-test", + app, + runtime: {} as RuntimeEnv, + botUserId: "U_BOT", + botId: "B_BOT", + identityHealth: { lifecycle: "ready", lastError: null }, + teamId: "T_TEST", + apiAppId: "A_TEST", + installationIdentity: { kind: "workspace", teamId: "T_TEST" }, + historyLimit: 0, + sessionScope: "per-sender", + mainKey: "main", + dmEnabled: true, + dmPolicy: "open", + allowFrom: [], + allowNameMatching: true, + groupDmEnabled: true, + groupDmChannels: [], + defaultRequireMention: true, + channelsConfig: { C_TEST: { users: ["alice"], enabled: true } }, + groupPolicy: "open", + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + replyToMode: "off", + slashCommand: { + enabled: false, + name: "openclaw", + sessionPrefix: "slack:slash", + ephemeral: true, + }, + textLimit: 4000, + ackReactionScope: "group-mentions", + typingReaction: "", + mediaMaxBytes: 1, + threadHistoryScope: "thread", + threadInheritParent: false, + }); + registerSlackMemberEvents({ ctx, trackEvent: params.trackEvent }); return { ingress, receive: receiverHarness.receive }; } @@ -440,7 +506,11 @@ describe("Slack durable ingress", () => { await ingress.waitForIdle(); expect(trackEvent).toHaveBeenCalledTimes(3); - expect(peekSystemEventEntries("agent:main:main").map((entry) => entry.contextKey)).toEqual([ + expect( + peekSystemEventEntries("agent:main:slack:channel:c_test").map( + (entry) => entry.contextKey, + ), + ).toEqual([ "slack:member:joined:c_test:u_test:ev-member-join-1", "slack:member:left:c_test:u_test:ev-member-left", "slack:member:joined:c_test:u_test:ev-member-join-2", @@ -454,15 +524,34 @@ describe("Slack durable ingress", () => { it("retries transient member failures through Bolt after restart", async () => { await withQueue(async (queue) => { const trackEvent = vi.fn(); - let userLookupCount = 0; - const resolveUserName = async () => { - userLookupCount += 1; - if (userLookupCount === 2) { - throw new Error("users.info temporarily unavailable"); + let usersInfoRequests = 0; + const usersInfoFetch = vi.fn>(async (input) => { + const pathname = new URL(String(input)).pathname; + if (pathname.endsWith("/conversations.info")) { + return new Response( + JSON.stringify({ + ok: true, + channel: { id: "C_TEST", name: "general", is_channel: true }, + }), + { headers: { "content-type": "application/json" }, status: 200 }, + ); } - return { name: "alice" }; - }; - const first = attachBoltMemberIngress({ queue, trackEvent, resolveUserName }); + if (!pathname.endsWith("/users.info")) { + throw new Error(`unexpected Slack API request: ${pathname}`); + } + usersInfoRequests += 1; + if (usersInfoRequests === 1) { + return new Response(JSON.stringify({ ok: false, error: "ratelimited" }), { + headers: { "content-type": "application/json", "retry-after": "0" }, + status: 429, + }); + } + return new Response(JSON.stringify({ ok: true, user: { id: "U_TEST", name: "alice" } }), { + headers: { "content-type": "application/json" }, + status: 200, + }); + }); + const first = attachBoltMemberIngress({ queue, trackEvent, usersInfoFetch }); first.ingress.start(); let restarted: ReturnType | undefined; try { @@ -475,13 +564,13 @@ describe("Slack durable ingress", () => { await first.ingress.stop(); expect(trackEvent).toHaveBeenCalledTimes(1); - expect(peekSystemEventEntries("agent:main:main")).toHaveLength(0); + expect(peekSystemEventEntries("agent:main:slack:channel:c_test")).toHaveLength(0); expect((await queue.listPending()).map((entry) => entry.id)).toContain("Ev-member-retry"); restarted = attachBoltMemberIngress({ queue, trackEvent, - resolveUserName, + usersInfoFetch, pollIntervalMs: 25, }); restarted.ingress.start(); @@ -493,7 +582,8 @@ describe("Slack durable ingress", () => { { timeout: 15_000, interval: 100 }, ); - expect(peekSystemEventEntries("agent:main:main")).toHaveLength(1); + expect(usersInfoRequests).toBe(2); + expect(peekSystemEventEntries("agent:main:slack:channel:c_test")).toHaveLength(1); } finally { await first.ingress.stop(); await restarted?.ingress.stop(); diff --git a/extensions/slack/src/monitor/monitor.thread-resolution.test.ts b/extensions/slack/src/monitor/monitor.thread-resolution.test.ts index 564a85de1b76..9cba8af89d4e 100644 --- a/extensions/slack/src/monitor/monitor.thread-resolution.test.ts +++ b/extensions/slack/src/monitor/monitor.thread-resolution.test.ts @@ -1,5 +1,6 @@ // Slack tests cover monitor.thread resolution plugin behavior. import { + WebClient, WebAPIHTTPError, WebAPIPlatformError, WebAPIRateLimitedError, @@ -8,7 +9,10 @@ import { import { afterEach, describe, expect, it, vi } from "vitest"; import type { SlackMessageEvent } from "../types.js"; import type { SlackIngressTurnLifecycle } from "./ingress.js"; -import { createSlackThreadTsResolver } from "./thread-resolution.js"; +import { + createSlackThreadTsResolver, + isTransientSlackThreadLookupError, +} from "./thread-resolution.js"; type SlackThreadClient = Parameters[0]["client"]; @@ -80,6 +84,59 @@ describe("createSlackThreadTsResolver", () => { expect(historyMock).toHaveBeenCalledTimes(1); }); + it("classifies an exhausted real WebClient 429 as transient", async () => { + const fetch = vi.fn(async () => { + return new Response(JSON.stringify({ ok: false, error: "ratelimited" }), { + headers: { "content-type": "application/json", "retry-after": "0" }, + status: 429, + }); + }); + const client = new WebClient("xoxb-test", { + fetch, + retryConfig: { retries: 0 }, + slackApiUrl: "https://slack.test/api/", + }); + + const error: unknown = await client.users + .info({ user: "U1" }) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(WebAPIRequestError); + if (!(error instanceof WebAPIRequestError)) { + throw new Error("expected exhausted Slack 429 to become WebAPIRequestError"); + } + expect(error.original.message).toMatch( + /^A rate limit was exceeded \(url: .+, retry-after: 0\)$/, + ); + expect(isTransientSlackThreadLookupError(error)).toBe(true); + expect(fetch).toHaveBeenCalledOnce(); + }); + + it.each(["internal_error", "service_unavailable"])( + "classifies a real WebClient %s platform response as transient", + async (code) => { + const fetch = vi.fn(async () => { + return new Response(JSON.stringify({ ok: false, error: code }), { + headers: { "content-type": "application/json" }, + status: 200, + }); + }); + const client = new WebClient("xoxb-test", { + fetch, + retryConfig: { retries: 0 }, + slackApiUrl: "https://slack.test/api/", + }); + + const error: unknown = await client.users + .info({ user: "U1" }) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(WebAPIPlatformError); + expect(isTransientSlackThreadLookupError(error)).toBe(true); + expect(fetch).toHaveBeenCalledOnce(); + }, + ); + it.each([ { label: "an actual Slack HTTP 408 timeout", @@ -226,6 +283,10 @@ describe("createSlackThreadTsResolver", () => { label: "operator-canceled Slack request", error: new WebAPIRequestError(new DOMException("request was canceled", "AbortError")), }, + { + label: "uncoded Slack request failure", + error: new WebAPIRequestError(new Error("request failed without a transient signal")), + }, ])("preserves cached ambiguity for definitive $label", async ({ error }) => { const historyMock = vi.fn().mockRejectedValue(error); const resolver = createSlackThreadTsResolver({ diff --git a/extensions/slack/src/monitor/thread-resolution.ts b/extensions/slack/src/monitor/thread-resolution.ts index 15b9a713cddc..433eb70f57e9 100644 --- a/extensions/slack/src/monitor/thread-resolution.ts +++ b/extensions/slack/src/monitor/thread-resolution.ts @@ -2,6 +2,7 @@ import { type WebClient as SlackWebClient, WebAPIHTTPError, + WebAPIPlatformError, WebAPIRateLimitedError, WebAPIRequestError, } from "@slack/web-api"; @@ -36,7 +37,7 @@ const markAmbiguousThreadReply = (message: SlackMessageEvent): SlackMessageEvent _ambiguousThreadReply: true, }); -function isTransientSlackThreadLookupError(error: unknown): boolean { +export function isTransientSlackThreadLookupError(error: unknown): boolean { if (error instanceof WebAPIRateLimitedError) { return true; } @@ -47,9 +48,17 @@ function isTransientSlackThreadLookupError(error: unknown): boolean { (error.statusCode >= 500 && error.statusCode < 600) ); } + // Slack documents these users.info response codes as transient service failures. + if (error instanceof WebAPIPlatformError) { + return error.data.error === "internal_error" || error.data.error === "service_unavailable"; + } if (!(error instanceof WebAPIRequestError)) { return false; } + // Slack Web API 8.0.0 wraps exhausted 429 retries as this uncoded request error. + if (/^A rate limit was exceeded \(url: .+, retry-after: \d+\)$/.test(error.original.message)) { + return true; + } return collectErrorGraphCandidates(error.original, (current) => [ current.cause, current.error,