diff --git a/packages/plugins/plugin-slack-control/README.md b/packages/plugins/plugin-slack-control/README.md index dd40fc6c24..a732266406 100644 --- a/packages/plugins/plugin-slack-control/README.md +++ b/packages/plugins/plugin-slack-control/README.md @@ -21,6 +21,10 @@ Commands must be plain text, at most 4,000 characters. `new` and project aliases The plugin has one company configuration and one Socket Mode connection per worker. Reconfiguration closes the prior connection before opening the next, and stale connection handlers cannot dispatch new commands. A native request already in flight when configuration is disabled may still complete. Do not run a second instance of this plugin against the same Slack app and company database. +Initial authentication and Socket Mode reconnects share one retry loop. Transient network failures use exponential backoff from one second to a maximum of sixty seconds. A longer provider rate-limit delay takes precedence; if it exceeds Node's safe timer range, the plugin reports a `rate_limited` error requiring operator recovery instead of retrying early. Each attempt has a thirty-second authentication/hello deadline and uses fresh clients; SDK automatic reconnects and HTTP retries are disabled. Shutdown cancels backoff, pending requests and upgraded sockets through public SDK/undici APIs before another attempt can start. If teardown cannot be confirmed within five seconds, the plugin reports `cleanup_failed` and refuses replacement until the worker is restarted. Queued commands wait while offline and recheck the existing company/actor restrictions after recovery; already claimed or uncertain commands retain the delivery contract below. + +Saving configuration starts connection work in the background; a successful save does not establish that Slack is connected. The board-only status response includes `connection.state`, `connection.lastFailure` (a fixed category) and `connection.retryAt` (a Unix timestamp in milliseconds or null). Invalid/revoked credentials, missing permissions, a workspace mismatch or an unclassified provider failure stop automatic retries and report an error. Correct the credentials or configuration and save again. Provider error bodies, headers, tokens and socket URLs are never returned or logged. Secret references are resolved only during the company-scoped configuration invocation, not from reconnect callbacks. + The host must replay persisted company configuration on both initial activation and worker crash recovery. This plugin does not include that host change or keep a separate hidden configuration cache. The upstream base used for this contribution replays configuration on initial startup only: after a worker crash, re-save configuration or restart the host to reconnect. Unattended crash recovery requires a separate host fix; related upstream work is tracked in [PR #10100](https://github.com/paperclipai/paperclip/pull/10100). Verify that the installed host includes recovery replay before relying on it. This is a transport, not an account scheduler or a natural-language planner. It uses existing agent configuration and native limits; it does not add a shared subscription-account concurrency lock. Choose existing agents that already respect your account allocation. It cannot make a local machine's agents run while that machine is off. The Paperclip host and selected execution environment must be available. @@ -86,4 +90,4 @@ After the operator has authorised Slack installation and enabled the configurati 4. Reply `Synthetic follow-up; no action needed` within that thread. Confirm one comment by the mapped Paperclip user on that same task. Check normal native wake behaviour separately if the agent is active. 5. Send a message as an unlisted user or in a channel/group DM. Confirm no plugin task, comment or reply. Disable configuration and confirm connection state becomes `disabled`. -The offline suite checks the manifest's required Socket Mode/IM fields against the documented shape, SDK capability and human-attribution contracts, official transport calls with mocked Slack responses, a real local PostgreSQL-compatible database migration, company separation, competing claims, retry and crash fences, persistent close/reopen, thread identity and configuration lifecycle. A synthetic child worker also exercises the actual host invocation guard: late configuration callbacks remain rejected, while the setup-created drain uses authorised proactive scope and stops when that scope is revoked. Slack's server-side manifest validation, real app installation, real Slack delivery and a real agent invocation require the operator's credentials and are **not performed by these tests**. PGlite runs only in tests; production storage is the host's namespaced PostgreSQL service. +The offline suite checks the manifest's required Socket Mode/IM fields against the documented shape, SDK capability and human-attribution contracts, official transport calls with mocked Slack responses, a real local PostgreSQL-compatible database migration, company separation, competing claims, retry and crash fences, persistent close/reopen, thread identity and configuration lifecycle. Recovery tests cover transient and permanent failures, backoff, handshake deadlines, stale events and confirmed shutdown before replacement. Loopback tests use the actual Slack SDK and undici to cancel a pending connection request and close an upgraded WebSocket whose peer ignores close frames. A synthetic child worker also exercises the actual host invocation guard: late configuration callbacks remain rejected, while the setup-created drain uses authorised proactive scope and stops when that scope is revoked. Slack's server-side manifest validation, real app installation, real Slack delivery and a real agent invocation require the operator's credentials and are **not performed by these tests**. PGlite runs only in tests; production storage is the host's namespaced PostgreSQL service. diff --git a/packages/plugins/plugin-slack-control/src/connection.ts b/packages/plugins/plugin-slack-control/src/connection.ts new file mode 100644 index 0000000000..a1d8564606 --- /dev/null +++ b/packages/plugins/plugin-slack-control/src/connection.ts @@ -0,0 +1,174 @@ +import { SocketModeClient, LogLevel, type Logger } from "@slack/socket-mode"; +import { WebClient } from "@slack/web-api"; +import { Agent, buildConnector, fetch } from "undici"; +import type { Socket } from "node:net"; +import type { Config } from "./config.js"; +import type { Connection, ConnectionStatus } from "./runtime.js"; + +// SDK logs can contain tokens, URLs and message bodies. Never forward them. +const quiet: Logger = { debug() {}, info() {}, warn() {}, error() {}, setLevel() {}, getLevel: () => LogLevel.ERROR, setName() {} }; +type Reason = NonNullable; +type Failure = { reason: Reason; retry: boolean; retryAfterMs?: number }; +class ConnectionFailure extends Error { + constructor(readonly reason: Reason) { super(reason); } +} +function classify(error: unknown): Failure { + if (error instanceof ConnectionFailure) return { reason: error.reason, retry: error.reason === "connection_timeout" }; + const value = error as { code?: unknown; statusCode?: unknown; retryAfter?: unknown; data?: { error?: unknown } } | null; + if (value?.code === "slack_webapi_request_error") return { reason: "network_error", retry: true }; + if (value?.code === "slack_webapi_rate_limited_error" || value?.statusCode === 429) { + const seconds = value.retryAfter; + // Never retry before the provider allows it. Node clamps oversized timers + // to one millisecond, so unsupported waits require operator recovery. + if (typeof seconds === "number" && seconds > 2_147_483_647 / 1000) return { reason: "rate_limited", retry: false }; + return { reason: "rate_limited", retry: true, retryAfterMs: typeof seconds === "number" && Number.isFinite(seconds) && seconds > 0 ? seconds * 1000 : 60_000 }; + } + if (value?.code === "slack_webapi_http_error" && typeof value.statusCode === "number" && value.statusCode >= 500 && value.statusCode < 600) return { reason: "network_error", retry: true }; + if (value?.code === "slack_webapi_platform_error") { + if (["not_authed", "invalid_auth", "account_inactive", "user_removed_from_team", "team_disabled", "token_revoked", "token_expired"].includes(String(value.data?.error))) return { reason: "authentication_failed", retry: false }; + if (["missing_scope", "not_allowed_token_type"].includes(String(value.data?.error))) return { reason: "permission_denied", retry: false }; + if (["service_unavailable", "internal_error", "fatal_error"].includes(String(value.data?.error))) return { reason: "network_error", retry: true }; + } + return { reason: "provider_error", retry: false }; +} +async function deadline(work: Promise, ms: number, reason: Reason): Promise { + let timer: ReturnType; + try { + return await Promise.race([work, new Promise((_, reject) => { + timer = setTimeout(() => reject(new ConnectionFailure(reason)), ms); timer.unref(); + })]); + } finally { clearTimeout(timer!); } +} + +/** One owner for initial connection and reconnects; all network work is cancellable. */ +export function createSlackConnection(config: Config, appToken: string, botToken: string, warn: (message: string) => void): Connection { + let stopped = false; + let task: Promise | undefined; + let finishStop!: () => void; + const stopping = new Promise((resolve) => { finishStop = resolve; }); + let status: ConnectionStatus = { state: "connecting", lastFailure: null, retryAt: null }; + let identity: Connection["authenticatedIdentity"]; + let activeWeb: WebClient | null = null; + let cleanupFailed = false; + + async function run(receive: Parameters[0]) { + let failures = 0; + while (!stopped) { + let live = true; + let socket: SocketModeClient | undefined; + let opening: Promise = Promise.resolve(); + const sockets = new Set(); + const connect = buildConnector({}); + const dispatcher = new Agent({ connect(options, callback) { + connect(options, (error, raw) => { + if (!raw) { callback(error, null); return; } + if (!live || stopped) { raw.destroy(); callback(new Error("Slack connection stopped"), null); return; } + sockets.add(raw); raw.once("close", () => sockets.delete(raw)); callback(null, raw); + }); + } }); + let failure: Failure | null = null; + let disconnected!: (failure: Failure) => void; + const ended = new Promise((resolve) => { disconnected = resolve; }); + status = { state: "connecting", lastFailure: status.lastFailure, retryAt: null }; + identity = undefined; + try { + const web = new WebClient(botToken, { logger: quiet, retryConfig: { retries: 0 }, rejectRateLimitedCalls: true, timeout: 10_000, + // WebClient uses DOM FormData types; undici accepts that same runtime body. + fetch: (url, init) => fetch(url, { ...init, dispatcher } as Parameters[1]) }); + socket = new SocketModeClient({ appToken, logger: quiet, logLevel: LogLevel.ERROR, dispatcher, autoReconnectEnabled: false, + clientOptions: { retryConfig: { retries: 0 }, rejectRateLimitedCalls: true, timeout: 10_000 } }); + const current = () => live && !stopped; + socket.on("connected", () => { + if (current()) { activeWeb = web; status = { state: "connected", lastFailure: null, retryAt: null }; } + }); + const lost = () => { + if (!current()) return; + activeWeb = null; status = { state: "connecting", lastFailure: "connection_lost", retryAt: null }; + disconnected({ reason: "connection_lost", retry: true }); + }; + socket.on("disconnected", lost); + socket.on("error", lost); + socket.on("slack_event", ({ type, body, ack }: { type: string; body: unknown; ack: () => Promise }) => { + if (!current() || status.state !== "connected") return; + // The SDK emits slack_event, not events_api. Filter its envelope here. + if (type !== "events_api") { + void ack().catch(() => warn("Unsupported Slack event acknowledgement failed.")); + } else { + void receive(body, ack).catch(() => warn("Slack event was not acknowledged; a provider retry may follow.")); + } + }); + opening = (async () => { + const auth = await web.auth.test(); + if (!current()) return; + if (auth.team_id !== config.workspaceId || !auth.bot_id) throw new ConnectionFailure("workspace_mismatch"); + identity = { workspaceId: auth.team_id, botId: auth.bot_id, + botUserId: typeof auth.user_id === "string" && /^[UW][A-Z0-9]{2,32}$/.test(auth.user_id) ? auth.user_id : null }; + await socket!.start(); + })(); + failure = await Promise.race([deadline(opening, 30_000, "connection_timeout").then(() => null, classify), ended, stopping.then(() => null)]); + if (!failure && !stopped) { + failures = 0; + failure = await Promise.race([ended, stopping.then(() => null)]); + } + } catch (error) { failure = classify(error); } + finally { + live = false; activeWeb = null; + if (!stopped) status = { state: "connecting", lastFailure: failure?.reason ?? null, retryAt: null }; + try { + await deadline((async () => { + // disconnect() alone cannot abort start() awaiting apps.connections.open. + // Destroy the public shared dispatcher, then wait for startup to settle + // and close again in case it installed a websocket during cancellation. + const disconnecting = socket?.disconnect(); + // Upgraded WebSockets detach from the Agent pool. As in the SDK's + // default connector, own the raw sockets so a stalled peer is closed. + for (const raw of sockets) raw.destroy(); + await Promise.all([disconnecting, dispatcher.destroy(), opening.catch(() => {})]); + await socket?.disconnect(); + })(), 5_000, "cleanup_failed"); + } catch { + cleanupFailed = true; failure = { reason: "cleanup_failed", retry: false }; + status = { state: "error", lastFailure: "cleanup_failed", retryAt: null }; + } + } + if (stopped) break; + if (!failure?.retry) { + status = { state: "error", lastFailure: failure?.reason ?? "provider_error", retryAt: null }; + break; + } + const delay = Math.max(Math.min(1000 * 2 ** Math.min(failures++, 6), 60_000), failure.retryAfterMs ?? 0); + status = { state: "connecting", lastFailure: failure.reason, retryAt: Date.now() + delay }; + let timer: ReturnType; + try { await Promise.race([stopping, new Promise((resolve) => { timer = setTimeout(resolve, delay); timer.unref(); })]); } + finally { clearTimeout(timer!); } + } + } + function readyWeb() { + if (stopped || status.state !== "connected" || !activeWeb) throw new ConnectionFailure("connection_lost"); + return activeWeb; + } + return { + get authenticatedIdentity() { return identity; }, + connectionStatus: () => ({ ...status }), + isConnected: () => !stopped && status.state === "connected", + async start(receive) { + if (stopped || task) throw new Error("Slack connection already started or stopped"); + // Do not hold the host's configuration invocation open during an outage. + task = run(receive).catch(() => { status = { state: "error", lastFailure: "provider_error", retryAt: null }; }); + }, + async stop() { + stopped = true; finishStop(); await task; identity = undefined; + if (cleanupFailed) throw new ConnectionFailure("cleanup_failed"); + }, + async verifyDirectMessage(message) { + const result = await readyWeb().conversations.info({ channel: message.channelId }); + const channel = result.channel; + return channel?.is_im === true && channel.is_mpim !== true && "user" in channel && channel.user === message.userId; + }, + async reply(message, text) { + const plain = text.replace(/&/g, "&").replace(//g, ">"); + await readyWeb().chat.postMessage({ channel: message.channelId, thread_ts: message.threadTs ?? message.ts, text: plain, + unfurl_links: false, unfurl_media: false, parse: "none", mrkdwn: false }); + }, + }; +} diff --git a/packages/plugins/plugin-slack-control/src/runtime.ts b/packages/plugins/plugin-slack-control/src/runtime.ts index d1f3a80837..7942668e71 100644 --- a/packages/plugins/plugin-slack-control/src/runtime.ts +++ b/packages/plugins/plugin-slack-control/src/runtime.ts @@ -5,10 +5,16 @@ import { createStore, type Store } from "./store.js"; export interface Connection extends Transport { readonly authenticatedIdentity?: { workspaceId: string; botUserId: string | null; botId: string }; + connectionStatus?(): ConnectionStatus; isConnected(): boolean; start(receive: (body: unknown, ack: () => Promise) => Promise): Promise; stop(): Promise; } +export interface ConnectionStatus { + state: "connecting" | "connected" | "error"; + lastFailure: "network_error" | "rate_limited" | "authentication_failed" | "permission_denied" | "workspace_mismatch" | "provider_error" | "connection_timeout" | "connection_lost" | "cleanup_failed" | null; + retryAt: number | null; +} export type Connect = (config: Config, companyId: string) => Promise; export interface TransportDiagnostics { @@ -50,14 +56,16 @@ export function createRuntime(ctx: PluginContext, connect: Connect) { // invocation. Its timer therefore uses the host-authorised proactive company // scope instead of inheriting an invocation that expires when config returns. const timer = setInterval(() => { - void activeControl?.drain().catch(() => ctx.logger.warn("Slack inbox could not be processed; inspect plugin status.")); + if (connection?.isConnected()) void activeControl?.drain().catch(() => ctx.logger.warn("Slack inbox could not be processed; inspect plugin status.")); }, 10_000); timer.unref(); - const health = () => state === "connected" && !connection?.isConnected() ? "connecting" : state; + const health = () => state === "connected" ? connection?.connectionStatus?.().state ?? (connection?.isConnected() ? "connected" : "connecting") : state; async function stop() { activeControl = null; - const previous = connection; connection = null; - if (previous) { try { await previous.stop(); } catch { ctx.logger.warn("Slack connection shutdown could not be confirmed."); } } + // Keep a failed shutdown attached: replacing an unconfirmed live connection + // could consume another socket's events. Recovery then needs a worker restart. + if (connection) await connection.stop(); + connection = null; } return { configure(value: unknown, companyId: string | null) { @@ -77,7 +85,7 @@ export function createRuntime(ctx: PluginContext, connect: Connect) { if (version !== generation) { await next.stop(); return; } connection = next; const current = () => generation === version && connection === next; - const control = createControl(ctx, companyId, config, store, next, current); + const control = createControl(ctx, companyId, config, store, next, () => current() && next.isConnected()); await next.start(async (body, ack) => { if (!current()) return; // Leave old-connection events unacknowledged for redelivery. await receiveEvent(body, ack, config, control.enqueue, diagnostics); @@ -87,7 +95,7 @@ export function createRuntime(ctx: PluginContext, connect: Connect) { state = "connected"; }).catch(async () => { state = "error"; - await stop(); + try { await stop(); } catch { ctx.logger.warn("Slack connection shutdown could not be confirmed."); } ctx.logger.error("Slack Control configuration or connection failed; inspect secret references and workspace mapping."); throw new Error("Slack Control could not connect. No credential or provider response is included in diagnostics."); }); @@ -96,7 +104,7 @@ export function createRuntime(ctx: PluginContext, connect: Connect) { async status(companyId: string) { if (!uuid(companyId) || (configuredCompany && configuredCompany !== companyId)) throw new Error("Company scope mismatch"); const identity = connection?.authenticatedIdentity; - return { state: health(), authenticatedIdentity: identity ? { + return { state: health(), connection: connection?.connectionStatus?.() ?? null, authenticatedIdentity: identity ? { workspaceId: identity.workspaceId, botUserId: identity.botUserId, botId: identity.botId, } : null, diagnostics: { ...diagnostics }, recent: store ? await store.recent() : [] }; }, diff --git a/packages/plugins/plugin-slack-control/src/worker.ts b/packages/plugins/plugin-slack-control/src/worker.ts index ea3f7e5b75..02e2fb9862 100644 --- a/packages/plugins/plugin-slack-control/src/worker.ts +++ b/packages/plugins/plugin-slack-control/src/worker.ts @@ -1,58 +1,15 @@ import { definePlugin, runWorker, type PluginContext } from "@paperclipai/plugin-sdk"; -import { SocketModeClient, LogLevel, type Logger } from "@slack/socket-mode"; -import { WebClient } from "@slack/web-api"; +import { createSlackConnection } from "./connection.js"; import { parseConfig } from "./config.js"; import { createRuntime, type Connect } from "./runtime.js"; -// SDK debug logs contain incoming message bodies. Never forward them to host logs. -const quiet: Logger = { debug() {}, info() {}, warn() {}, error() {}, setLevel() {}, getLevel: () => LogLevel.ERROR, setName() {} }; export function slackConnection(ctx: PluginContext): Connect { return async (config, companyId) => { const [appToken, botToken] = await Promise.all([ ctx.secrets.resolve(config.appToken, { companyId, configPath: "appToken" }), ctx.secrets.resolve(config.botToken, { companyId, configPath: "botToken" }), ]); - const web = new WebClient(botToken, { logger: quiet, retryConfig: { retries: 0 }, rejectRateLimitedCalls: true, timeout: 10_000 }); - const auth = await web.auth.test(); - if (auth.team_id !== config.workspaceId || !auth.bot_id) throw new Error("Slack workspace does not match configuration"); - const socket = new SocketModeClient({ appToken, logger: quiet, logLevel: LogLevel.ERROR, clientOptions: { retryConfig: { retries: 0 }, timeout: 10_000 } }); - let connected = false; - socket.on("connected", () => { connected = true; }); - for (const event of ["connecting", "reconnecting", "disconnecting", "disconnected"]) socket.on(event, () => { connected = false; }); - socket.on("error", () => ctx.logger.warn("Slack connection reported an error; no provider payload was logged.")); - return { - // Reuse the existing verified auth.test result; never expose its full - // response or make another provider call just to inspect board status. - authenticatedIdentity: { - workspaceId: auth.team_id, - botUserId: typeof auth.user_id === "string" && /^[UW][A-Z0-9]{2,32}$/.test(auth.user_id) ? auth.user_id : null, - botId: auth.bot_id, - }, - isConnected: () => connected, - async start(receive) { - // The SDK emits the inner event name (e.g. `message`) and `slack_event`, - // not `events_api`. Keep the envelope filter at this transport boundary. - socket.on("slack_event", ({ type, body, ack }: { type: string; body: unknown; ack: () => Promise }) => { - if (type !== "events_api") { - void ack().catch(() => ctx.logger.warn("Unsupported Slack event acknowledgement failed.")); - return; - } - void receive(body, ack).catch(() => ctx.logger.warn("Slack event was not acknowledged; a provider retry may follow.")); - }); - await socket.start(); - }, - async stop() { await socket.disconnect(); }, - async verifyDirectMessage(message) { - const result = await web.conversations.info({ channel: message.channelId }); - const channel = result.channel; - return channel?.is_im === true && channel.is_mpim !== true && "user" in channel && channel.user === message.userId; - }, - async reply(message, text) { - const plain = text.replace(/&/g, "&").replace(//g, ">"); - await web.chat.postMessage({ channel: message.channelId, thread_ts: message.threadTs ?? message.ts, text: plain, - unfurl_links: false, unfurl_media: false, parse: "none", mrkdwn: false }); - }, - }; + return createSlackConnection(config, appToken, botToken, (message) => ctx.logger.warn(message)); }; } diff --git a/packages/plugins/plugin-slack-control/tests/connection-sdk.test.ts b/packages/plugins/plugin-slack-control/tests/connection-sdk.test.ts new file mode 100644 index 0000000000..bf73163c63 --- /dev/null +++ b/packages/plugins/plugin-slack-control/tests/connection-sdk.test.ts @@ -0,0 +1,92 @@ +import { createHash } from "node:crypto"; +import { createServer, type Server, type ServerResponse } from "node:http"; +import type { Socket } from "node:net"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import type { Connection } from "../src/runtime.js"; +import { config } from "./helpers.js"; + +const endpoint = vi.hoisted(() => ({ url: "" })); +// Use the actual Slack clients and undici transport against a loopback peer. +// Only the API base URL changes; no real credentials or Slack requests are used. +vi.mock("@slack/web-api", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, WebClient: class extends actual.WebClient { + constructor(token: string, options: import("@slack/web-api").WebClientOptions) { super(token, { ...options, slackApiUrl: endpoint.url }); } + } }; +}); +vi.mock("@slack/socket-mode", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, SocketModeClient: class extends actual.SocketModeClient { + constructor(options: import("@slack/socket-mode").SocketModeOptions) { super({ ...options, clientOptions: { ...options.clientOptions, slackApiUrl: endpoint.url } }); } + } }; +}); +import { createSlackConnection } from "../src/connection.js"; + +let server: Server; +let connection: Connection; +let holdOpen = false; +let pending: ServerResponse | undefined; +let websocket: Socket | undefined; +let hello = true; +let opens = 0; +const sockets = new Set(); +beforeEach(async () => { + holdOpen = false; pending = undefined; websocket = undefined; hello = true; opens = 0; + server = createServer((request, response) => { + request.resume(); + response.setHeader("content-type", "application/json"); + if (request.url === "/auth.test") response.end(JSON.stringify({ ok: true, team_id: config.workspaceId, bot_id: "BBOT", user_id: "UBOT" })); + else if (request.url === "/apps.connections.open") { + opens++; + if (holdOpen) pending = response; + else response.end(JSON.stringify({ ok: true, url: endpoint.url.replace("http:", "ws:") + "socket" })); + } else { response.statusCode = 404; response.end("{}"); } + }); + server.on("connection", (socket) => { sockets.add(socket); socket.on("close", () => sockets.delete(socket)); }); + server.on("upgrade", (request, socket) => { + websocket = socket as Socket; + const accept = createHash("sha1").update(String(request.headers["sec-websocket-key"]) + "258EAFA5-E914-47DA-95CA-C5AB0DC85B11").digest("base64"); + socket.write(`HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Accept: ${accept}\r\n\r\n`); + if (hello) { const body = Buffer.from('{"type":"hello"}'); socket.write(Buffer.concat([Buffer.from([0x81, body.length]), body])); } + // Deliberately ignore ping and close frames: teardown must destroy the socket. + socket.on("data", () => {}); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Missing loopback address"); + endpoint.url = `http://127.0.0.1:${address.port}/`; + connection = createSlackConnection(config, "xapp-synthetic", "xoxb-synthetic", vi.fn()); +}); +afterEach(async () => { + await connection.stop().catch(() => {}); + for (const socket of sockets) socket.destroy(); + await new Promise((resolve) => server.close(() => resolve())); +}); +it("cancels actual apps.connections.open before a late response can open a socket", async () => { + holdOpen = true; + await connection.start(vi.fn()); + await vi.waitFor(() => expect(pending).toBeDefined()); + await connection.stop(); + await vi.waitFor(() => expect(pending!.destroyed).toBe(true)); + pending!.end(JSON.stringify({ ok: true, url: endpoint.url.replace("http:", "ws:") + "socket" })); + expect(connection.isConnected()).toBe(false); expect(websocket).toBeUndefined(); expect(opens).toBe(1); +}, 10_000); +it("destroys an upgraded real WebSocket even when the peer ignores its close frame", async () => { + await connection.start(vi.fn()); + await vi.waitFor(() => expect(connection.isConnected()).toBe(true)); + expect(websocket).toBeDefined(); + await connection.stop(); + // The HTTP server's upgraded peer is half-open; EOF proves the client's raw + // socket closed even though this deliberately uncooperative peer stays open. + await vi.waitFor(() => expect(websocket!.readableEnded).toBe(true)); + expect(connection.isConnected()).toBe(false); expect(opens).toBe(1); +}, 10_000); +it("cancels the real SDK hello wait and closes the upgraded socket on shutdown", async () => { + hello = false; + await connection.start(vi.fn()); + await vi.waitFor(() => expect(websocket).toBeDefined()); + expect(connection.isConnected()).toBe(false); + await connection.stop(); + await vi.waitFor(() => expect(websocket!.readableEnded).toBe(true)); + expect(opens).toBe(1); +}, 10_000); diff --git a/packages/plugins/plugin-slack-control/tests/runtime.test.ts b/packages/plugins/plugin-slack-control/tests/runtime.test.ts index ab139d5343..efdcd6a07a 100644 --- a/packages/plugins/plugin-slack-control/tests/runtime.test.ts +++ b/packages/plugins/plugin-slack-control/tests/runtime.test.ts @@ -99,4 +99,49 @@ describe("acknowledgement and connection lifecycle", () => { expect((await runtime.status(company)).authenticatedIdentity).toBeNull(); } finally { await runtime.shutdown(); } }); + it("keeps received commands queued while offline, then rechecks company membership after recovery", async () => { + vi.useFakeTimers(); + let connected = false; + let receive!: (body: unknown, ack: () => Promise) => Promise; + const connection: Connection = { isConnected: () => connected, start: vi.fn(async (handler) => { receive = handler; }), stop: vi.fn(), verifyDirectMessage: vi.fn().mockResolvedValue(true), reply: vi.fn() }; + const { ctx, api } = host(database(pg)); const runtime = createRuntime(ctx, async () => connection); + try { + await runtime.configure(config, company); + await receive(envelope({ text: "status" }), vi.fn()); + await vi.advanceTimersByTimeAsync(20_000); + expect((await runtime.status(company)).recent[0]?.phase).toBe("received"); + expect(api.access.members.list).not.toHaveBeenCalled(); + // Recovery does not bypass revocation that happened during the outage. + api.access.members.list.mockResolvedValue([]); connected = true; + await vi.advanceTimersByTimeAsync(10_000); + await vi.waitFor(async () => expect((await runtime.status(company)).recent[0]?.phase).toBe("uncertain")); + expect(api.access.members.list).toHaveBeenCalledWith({ companyId: company }); + expect(api.issues.list).not.toHaveBeenCalled(); expect(connection.reply).not.toHaveBeenCalled(); + } finally { await runtime.shutdown(); vi.useRealTimers(); } + }); + it("awaits confirmed shutdown before replacement and refuses replacement if cleanup fails", async () => { + let release!: () => void; + const connection: Connection = { isConnected: () => false, start: vi.fn(), stop: vi.fn(() => new Promise((resolve) => { release = resolve; })), verifyDirectMessage: vi.fn(), reply: vi.fn() }; + const connect = vi.fn(async () => connection); + const { ctx } = host(database(pg)); const runtime = createRuntime(ctx, connect); + await runtime.configure(config, company); + const replacing = runtime.configure(config, company); + await vi.waitFor(() => expect(connection.stop).toHaveBeenCalledTimes(1)); + expect(connect).toHaveBeenCalledTimes(1); + release(); await replacing; expect(connect).toHaveBeenCalledTimes(2); + vi.mocked(connection.stop).mockRejectedValue(new Error("synthetic-private-cleanup-error")); + await expect(runtime.configure(config, company)).rejects.toThrow("No credential or provider response"); + expect(connect).toHaveBeenCalledTimes(2); expect(runtime.health()).toBe("error"); + vi.mocked(connection.stop).mockResolvedValue(undefined); await runtime.shutdown(); + }); + it("reports terminal connection failures to board status and health", async () => { + const connection: Connection = { connectionStatus: () => ({ state: "error", lastFailure: "authentication_failed", retryAt: null }), isConnected: () => false, start: vi.fn(), stop: vi.fn(), verifyDirectMessage: vi.fn(), reply: vi.fn() }; + const { ctx } = host(database(pg)); const runtime = createRuntime(ctx, async () => connection); + try { + await runtime.configure(config, company); + expect(runtime.health()).toBe("error"); + expect((await runtime.status(company)).connection).toEqual({ state: "error", lastFailure: "authentication_failed", retryAt: null }); + await expect(runtime.status(otherCompany)).rejects.toThrow("Company scope mismatch"); + } finally { await runtime.shutdown(); } + }); }); diff --git a/packages/plugins/plugin-slack-control/tests/transport.test.ts b/packages/plugins/plugin-slack-control/tests/transport.test.ts index d328c2028c..188355a16c 100644 --- a/packages/plugins/plugin-slack-control/tests/transport.test.ts +++ b/packages/plugins/plugin-slack-control/tests/transport.test.ts @@ -1,42 +1,66 @@ import type { PluginContext } from "@paperclipai/plugin-sdk"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { EventEmitter } from "node:events"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Connection } from "../src/runtime.js"; import { company, config, envelope, message } from "./helpers.js"; +type TestSocket = EventEmitter & { options: Record }; const mocks = vi.hoisted(() => ({ - auth: vi.fn(), info: vi.fn(), post: vi.fn(), start: vi.fn(), stop: vi.fn(), on: vi.fn(), web: vi.fn(), socket: vi.fn(), + auth: vi.fn(), info: vi.fn(), post: vi.fn(), start: vi.fn(), stop: vi.fn(), destroy: vi.fn(), web: vi.fn(), + sockets: [] as TestSocket[], agents: [] as Array<{ destroy(): Promise }>, })); +vi.mock("undici", () => ({ fetch: vi.fn(), buildConnector: () => vi.fn(), Agent: class { + constructor() { mocks.agents.push(this); } + destroy() { return mocks.destroy(this); } +} })); vi.mock("@slack/web-api", () => ({ WebClient: class { constructor(...args: unknown[]) { mocks.web(...args); } auth = { test: mocks.auth }; conversations = { info: mocks.info }; chat = { postMessage: mocks.post }; } })); -vi.mock("@slack/socket-mode", () => ({ LogLevel: { ERROR: "error" }, SocketModeClient: class { - constructor(...args: unknown[]) { mocks.socket(...args); } - on = mocks.on; start = mocks.start; disconnect = mocks.stop; -} })); +vi.mock("@slack/socket-mode", async () => { + const { EventEmitter } = await import("node:events"); + return { LogLevel: { ERROR: "error" }, SocketModeClient: class extends EventEmitter { + constructor(readonly options: Record) { super(); mocks.sockets.push(this); } + start() { return mocks.start(this); } + disconnect() { return mocks.stop(this); } + } }; +}); import { slackConnection } from "../src/worker.js"; +const connections: Connection[] = []; +const networkError = { code: "slack_webapi_request_error", message: "synthetic-private-provider-payload" }; +const flush = () => vi.advanceTimersByTimeAsync(0); beforeEach(() => { - vi.clearAllMocks(); mocks.auth.mockResolvedValue({ team_id: config.workspaceId, bot_id: "BBOT" }); + vi.useFakeTimers(); vi.resetAllMocks(); mocks.sockets.length = 0; mocks.agents.length = 0; + mocks.auth.mockResolvedValue({ team_id: config.workspaceId, bot_id: "BBOT" }); mocks.info.mockResolvedValue({ channel: { is_im: true, user: message.userId } }); + mocks.start.mockImplementation(async (socket: TestSocket) => { socket.emit("connected"); }); + mocks.stop.mockImplementation(async (socket: TestSocket) => { socket.emit("disconnected"); }); + mocks.destroy.mockResolvedValue(undefined); +}); +afterEach(async () => { + await Promise.all(connections.splice(0).map((connection) => connection.stop().catch(() => {}))); + expect(vi.getTimerCount()).toBe(0); vi.useRealTimers(); }); function context() { const api = { secrets: { resolve: vi.fn().mockResolvedValueOnce("xapp-synthetic").mockResolvedValueOnce("xoxb-synthetic") }, logger: { warn: vi.fn(), error: vi.fn() } }; return { api, ctx: api as unknown as PluginContext }; } +async function create(ctx = context().ctx) { + const connection = await slackConnection(ctx)(config, company); connections.push(connection); return connection; +} +async function start(connection: Connection, receive = vi.fn().mockResolvedValue(undefined)) { await connection.start(receive); await flush(); return receive; } + describe("official Slack transport boundary", () => { it("receives an Events API envelope through the actual Socket Mode SDK dispatcher", async () => { const actual = await vi.importActual("@slack/socket-mode"); - const connection = await slackConnection(context().ctx)(config, company); + const connection = await create(); const receive = vi.fn(async (_body: unknown, ack: () => Promise) => { await ack(); }); - await connection.start(receive); - // Execute the installed SDK's real wire-message dispatcher without opening - // a socket or authenticating. The old `events_api` listener receives nothing. + await start(connection, receive); const send = vi.fn().mockResolvedValue(undefined); const sdk = Object.assign(Object.create(actual.SocketModeClient.prototype), { logger: { debug() {}, getLevel: () => actual.LogLevel.ERROR }, send, - emit(name: string, payload: unknown) { - for (const [event, listener] of mocks.on.mock.calls) if (event === name) listener(payload); - }, + emit(name: string, payload: unknown) { mocks.sockets[0]!.emit(name, payload); }, }) as { onWebSocketMessage(data: string, isBinary: boolean): Promise }; await sdk.onWebSocketMessage(JSON.stringify({ type: "events_api", envelope_id: "synthetic-envelope", accepts_response_payload: false, @@ -45,50 +69,154 @@ describe("official Slack transport boundary", () => { expect(receive).toHaveBeenCalledExactlyOnceWith(envelope(), expect.any(Function)); expect(send).toHaveBeenCalledExactlyOnceWith("synthetic-envelope", undefined); }); - it("resolves only company-bound references and checks the authenticated workspace before starting", async () => { - const { ctx, api } = context(); const connection = await slackConnection(ctx)(config, company); + it("resolves only company-bound references and gives reconnect ownership to one supervisor", async () => { + const { ctx, api } = context(); const connection = await create(ctx); expect(api.secrets.resolve).toHaveBeenNthCalledWith(1, config.appToken, { companyId: company, configPath: "appToken" }); expect(api.secrets.resolve).toHaveBeenNthCalledWith(2, config.botToken, { companyId: company, configPath: "botToken" }); + expect(connection.isConnected()).toBe(false); await start(connection); expect(mocks.web).toHaveBeenCalledWith("xoxb-synthetic", expect.objectContaining({ retryConfig: { retries: 0 }, rejectRateLimitedCalls: true })); + expect(mocks.sockets[0]!.options).toMatchObject({ autoReconnectEnabled: false, dispatcher: mocks.agents[0], clientOptions: { retryConfig: { retries: 0 } } }); + await connection.stop(); expect(mocks.destroy).toHaveBeenCalledTimes(1); expect(connection.isConnected()).toBe(false); - await connection.start(vi.fn()); expect(mocks.start).toHaveBeenCalledTimes(1); - expect(mocks.on).toHaveBeenCalledWith("slack_event", expect.any(Function)); - await connection.stop(); expect(mocks.stop).toHaveBeenCalledTimes(1); }); it("exposes only bot identity metadata from the existing authentication check", async () => { mocks.auth.mockResolvedValue({ team_id: config.workspaceId, bot_id: "BBOT", user_id: "UBOT", token: "synthetic-private-token", response_metadata: { headers: "synthetic-private-headers" } }); - const connection = await slackConnection(context().ctx)(config, company); + const connection = await create(); await start(connection); expect(connection.authenticatedIdentity).toEqual({ workspaceId: config.workspaceId, botId: "BBOT", botUserId: "UBOT" }); expect(mocks.auth).toHaveBeenCalledTimes(1); expect(JSON.stringify(connection.authenticatedIdentity)).not.toContain("private"); }); it("acknowledges non-Events-API envelopes without dispatching commands", async () => { - const connection = await slackConnection(context().ctx)(config, company); - const receive = vi.fn(); const ack = vi.fn().mockResolvedValue(undefined); - await connection.start(receive); - const listener = mocks.on.mock.calls.find(([name]) => name === "slack_event")?.[1]; - listener({ type: "interactive", body: envelope(), ack }); + const connection = await create(); const receive = await start(connection); const ack = vi.fn().mockResolvedValue(undefined); + mocks.sockets[0]!.emit("slack_event", { type: "interactive", body: envelope(), ack }); expect(receive).not.toHaveBeenCalled(); expect(ack).toHaveBeenCalledOnce(); }); - it("rejects another workspace or a user token", async () => { - mocks.auth.mockResolvedValue({ team_id: "TOTHER", bot_id: "BBOT" }); - await expect(slackConnection(context().ctx)(config, company)).rejects.toThrow(); - mocks.auth.mockResolvedValue({ team_id: config.workspaceId }); - await expect(slackConnection(context().ctx)(config, company)).rejects.toThrow(); - expect(mocks.socket).not.toHaveBeenCalled(); + it.each([{ team_id: "TOTHER", bot_id: "BBOT" }, { team_id: config.workspaceId }])("rejects another workspace or a user token: %j", async (auth) => { + mocks.auth.mockResolvedValue(auth); const connection = await create(); await start(connection); + expect(connection.connectionStatus!()).toEqual({ state: "error", lastFailure: "workspace_mismatch", retryAt: null }); + await vi.advanceTimersByTimeAsync(120_000); + expect(mocks.start).not.toHaveBeenCalled(); expect(mocks.auth).toHaveBeenCalledTimes(1); }); - it("requires an actual one-to-one conversation with the mapped user", async () => { - const connection = await slackConnection(context().ctx)(config, company); + it("requires an actual one-to-one conversation with the mapped user, including after recovery", async () => { + const connection = await create(); await start(connection); expect(await connection.verifyDirectMessage(message)).toBe(true); + mocks.sockets[0]!.emit("disconnected"); await flush(); + await expect(connection.verifyDirectMessage(message)).rejects.toThrow("connection_lost"); + await vi.advanceTimersByTimeAsync(1000); for (const channel of [{ is_im: false, user: message.userId }, { is_im: true, is_mpim: true, user: message.userId }, { is_im: true, user: "UOTHER" }, { is_im: true }]) { mocks.info.mockResolvedValue({ channel }); expect(await connection.verifyDirectMessage(message)).toBe(false); } }); it("replies only to the source IM/thread with mentions and URL unfurling disabled", async () => { - const connection = await slackConnection(context().ctx)(config, company); + const connection = await create(); await start(connection); await connection.reply(message, "<@UOTHER> https://example.org"); expect(mocks.post).toHaveBeenCalledWith({ channel: message.channelId, thread_ts: message.ts, text: "<@UOTHER> https://example.org", unfurl_links: false, unfurl_media: false, parse: "none", mrkdwn: false }); - const socketOptions = mocks.socket.mock.calls[0]?.[0]; - socketOptions.logger.error("xapp-do-not-log"); socketOptions.logger.debug({ text: "private body" }); + const logger = mocks.sockets[0]!.options.logger as { error(value: unknown): void; debug(value: unknown): void }; + logger.error("xapp-do-not-log"); logger.debug({ text: "private body" }); + }); +}); + +describe("bounded connection recovery", () => { + it("retries initial auth network failures without resolving secrets again or holding configuration open", async () => { + mocks.auth.mockRejectedValueOnce(networkError); + const { ctx, api } = context(); const connection = await create(ctx); await start(connection); + expect(connection.connectionStatus!()).toMatchObject({ state: "connecting", lastFailure: "network_error" }); + expect(mocks.start).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1000); + expect(connection.isConnected()).toBe(true); expect(api.secrets.resolve).toHaveBeenCalledTimes(2); + expect(JSON.stringify(connection.connectionStatus!())).not.toContain("private"); + }); + it("recovers from a failed reconnect request with no parallel clients, then ignores old events", async () => { + const connection = await create(); const receive = await start(connection); + const old = mocks.sockets[0]!; + mocks.start.mockRejectedValueOnce(networkError); + old.emit("disconnected"); await flush(); + await vi.advanceTimersByTimeAsync(1000); + expect(connection.connectionStatus!()).toMatchObject({ state: "connecting", lastFailure: "network_error" }); + expect(mocks.destroy).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1999); expect(mocks.start).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); expect(connection.isConnected()).toBe(true); + expect(mocks.sockets).toHaveLength(3); expect(mocks.auth).toHaveBeenCalledTimes(3); + const ack = vi.fn(); old.emit("connected"); old.emit("slack_event", { type: "events_api", body: envelope(), ack }); + expect(receive).not.toHaveBeenCalled(); expect(ack).not.toHaveBeenCalled(); + mocks.sockets[2]!.emit("slack_event", { type: "events_api", body: envelope(), ack }); expect(receive).toHaveBeenCalledOnce(); + }); + it.each(["invalid_auth", "token_revoked", "missing_scope"])("stops credential retries for %s and exposes only a fixed category", async (reason) => { + mocks.start.mockRejectedValue({ code: "slack_webapi_platform_error", data: { error: reason, token: "private-token" } }); + const connection = await create(); await start(connection); await vi.advanceTimersByTimeAsync(300_000); + expect(connection.connectionStatus!()).toEqual({ state: "error", lastFailure: reason === "missing_scope" ? "permission_denied" : "authentication_failed", retryAt: null }); + expect(mocks.start).toHaveBeenCalledTimes(1); expect(mocks.destroy).toHaveBeenCalledTimes(1); + }); + it("bounds a missing hello handshake, destroys the old transport and retries", async () => { + mocks.start.mockImplementationOnce((socket: TestSocket) => new Promise((_, reject) => { socket.once("disconnected", () => reject(networkError)); })); + const connection = await create(); await start(connection); + await vi.advanceTimersByTimeAsync(29_999); expect(mocks.destroy).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(connection.connectionStatus!()).toMatchObject({ state: "connecting", lastFailure: "connection_timeout" }); + expect(mocks.destroy).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1000); expect(connection.isConnected()).toBe(true); + }); + it("waits for dispatcher destruction before creating another attempt", async () => { + let finish!: () => void; + mocks.destroy.mockImplementationOnce(() => new Promise((resolve) => { finish = resolve; })); + const connection = await create(); await start(connection); + mocks.sockets[0]!.emit("disconnected"); await flush(); + await vi.advanceTimersByTimeAsync(2000); expect(mocks.sockets).toHaveLength(1); + finish(); await flush(); await vi.advanceTimersByTimeAsync(1000); + expect(mocks.sockets).toHaveLength(2); expect(connection.isConnected()).toBe(true); + }); + it("cancels pending authentication and does not let a late response start a socket", async () => { + let finish!: (auth: unknown) => void; + mocks.auth.mockImplementationOnce(() => new Promise((resolve) => { finish = resolve; })); + mocks.destroy.mockImplementationOnce(async () => { finish({ team_id: config.workspaceId, bot_id: "BBOT" }); }); + const connection = await create(); await start(connection); await connection.stop(); + await vi.advanceTimersByTimeAsync(120_000); + expect(mocks.start).not.toHaveBeenCalled(); expect(mocks.auth).toHaveBeenCalledTimes(1); + expect(connection.authenticatedIdentity).toBeUndefined(); + }); + it("cancels the handshake on shutdown and ignores late connected/message events", async () => { + mocks.start.mockImplementationOnce((socket: TestSocket) => new Promise((_, reject) => { socket.once("disconnected", () => reject(networkError)); })); + const connection = await create(); const receive = await start(connection); await connection.stop(); + const old = mocks.sockets[0]!; old.emit("connected"); old.emit("slack_event", { type: "events_api", body: envelope(), ack: vi.fn() }); + await vi.advanceTimersByTimeAsync(120_000); + expect(connection.isConnected()).toBe(false); expect(receive).not.toHaveBeenCalled(); expect(mocks.sockets).toHaveLength(1); + }); + it("cancels backoff on shutdown and does not permit a second owner", async () => { + mocks.auth.mockRejectedValue(networkError); const connection = await create(); await start(connection); + await expect(connection.start(vi.fn())).rejects.toThrow("already started"); + await connection.stop(); await vi.advanceTimersByTimeAsync(120_000); + expect(mocks.auth).toHaveBeenCalledTimes(1); + }); + it("caps transient backoff and honours a longer rate-limit delay", async () => { + mocks.auth.mockRejectedValue(networkError); const connection = await create(); await start(connection); + for (const delay of [1000, 2000, 4000, 8000, 16000, 32000, 60000, 60000]) { + expect(connection.connectionStatus!().retryAt! - Date.now()).toBe(delay); + await vi.advanceTimersByTimeAsync(delay); + } + await connection.stop(); + mocks.auth.mockRejectedValue({ code: "slack_webapi_rate_limited_error", retryAfter: 120 }); + const limited = await create(); await start(limited); + expect(limited.connectionStatus!().retryAt! - Date.now()).toBe(120_000); + }); + it("does not shorten a valid two-hour provider rate limit", async () => { + mocks.auth.mockRejectedValueOnce({ code: "slack_webapi_rate_limited_error", retryAfter: 7200 }); + const connection = await create(); await start(connection); + expect(connection.connectionStatus!().retryAt! - Date.now()).toBe(7_200_000); + await vi.advanceTimersByTimeAsync(7_199_999); expect(mocks.auth).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); expect(connection.isConnected()).toBe(true); + }); + it.each([2_147_484, Infinity])("halts instead of overflowing an unsupported provider wait: %s", async (retryAfter) => { + mocks.auth.mockRejectedValue({ code: "slack_webapi_rate_limited_error", retryAfter }); + const connection = await create(); await start(connection); + expect(connection.connectionStatus!()).toEqual({ state: "error", lastFailure: "rate_limited", retryAt: null }); + await vi.advanceTimersByTimeAsync(7_200_000); expect(mocks.auth).toHaveBeenCalledTimes(1); + }); + it("halts when cleanup cannot be confirmed instead of opening a competing client", async () => { + mocks.destroy.mockImplementationOnce(() => new Promise(() => {})); + const connection = await create(); await start(connection); mocks.sockets[0]!.emit("disconnected"); await flush(); + await vi.advanceTimersByTimeAsync(5000); + expect(connection.connectionStatus!()).toEqual({ state: "error", lastFailure: "cleanup_failed", retryAt: null }); + await vi.advanceTimersByTimeAsync(120_000); expect(mocks.sockets).toHaveLength(1); + await expect(connection.stop()).rejects.toThrow("cleanup_failed"); }); });