diff --git a/.changeset/slack-timestamps-as-strings.md b/.changeset/slack-timestamps-as-strings.md new file mode 100644 index 0000000000..ebf31b475a --- /dev/null +++ b/.changeset/slack-timestamps-as-strings.md @@ -0,0 +1,5 @@ +--- +"@executor-js/plugin-openapi": patch +--- + +Slack tools accept message timestamps (`ts`, `thread_ts`, `latest`, `oldest` and other `*_ts` inputs) as strings, so history and thread lookups are no longer rejected. Refresh an existing Slack integration's spec to pick this up. diff --git a/bun.lock b/bun.lock index cbf476dde0..f7f332efaa 100644 --- a/bun.lock +++ b/bun.lock @@ -1,6 +1,5 @@ { "lockfileVersion": 1, - "configVersion": 1, "workspaces": { "": { "name": "executor-workspace", @@ -713,6 +712,8 @@ }, "devDependencies": { "@effect/vitest": "catalog:", + "@executor-js/emulate": "0.14.3-mfa.0", + "@executor-js/plugin-openapi": "workspace:*", "@types/node": "catalog:", "bun-types": "catalog:", "vitest": "catalog:", diff --git a/packages/hosts/mcp/package.json b/packages/hosts/mcp/package.json index 1e8dd7c4fc..a0a053c329 100644 --- a/packages/hosts/mcp/package.json +++ b/packages/hosts/mcp/package.json @@ -54,6 +54,8 @@ }, "devDependencies": { "@effect/vitest": "catalog:", + "@executor-js/emulate": "0.14.3-mfa.0", + "@executor-js/plugin-openapi": "workspace:*", "@types/node": "catalog:", "bun-types": "catalog:", "vitest": "catalog:" diff --git a/packages/hosts/mcp/src/slack-timestamps.test.ts b/packages/hosts/mcp/src/slack-timestamps.test.ts new file mode 100644 index 0000000000..a67b25f404 --- /dev/null +++ b/packages/hosts/mcp/src/slack-timestamps.test.ts @@ -0,0 +1,197 @@ +import { expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +import { FetchHttpClient } from "effect/unstable/http"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { createServer } from "node:http"; + +import { createEmulator } from "@executor-js/emulate"; +import type { ExecutionEngine } from "@executor-js/execution"; +import { createExecutor, AuthTemplateSlug, ConnectionName, ToolAddress } from "@executor-js/sdk"; +import { makeTestConfig, memoryCredentialsPlugin } from "@executor-js/sdk/testing"; +import { openApiPlugin } from "@executor-js/plugin-openapi"; +import { slackWebApiAdapter } from "@executor-js/plugin-openapi/providers/slack"; + +import { createExecutorMcpServer } from "./tool-server"; + +const BOT_TOKEN = "xoxb-test-token"; + +const form = (properties: Record) => ({ + required: true, + content: { "application/x-www-form-urlencoded": { schema: { type: "object", properties } } }, +}); + +// The published Slack spec types every timestamp input as `number`. +const publishedSlackSpec = (apiUrl: string) => ({ + openapi: "3.0.0", + info: { title: "Slack Web API", version: "1.7.0" }, + servers: [{ url: apiUrl }], + paths: { + "/conversations.history": { + post: { + operationId: "conversations_history", + tags: ["conversations"], + requestBody: form({ + channel: { type: "string" }, + latest: { type: "number", default: 0 }, + oldest: { type: "number", default: 0 }, + inclusive: { type: "boolean" }, + }), + responses: { "200": { description: "OK" } }, + }, + }, + "/conversations.replies": { + post: { + operationId: "conversations_replies", + tags: ["conversations"], + requestBody: form({ channel: { type: "string" }, ts: { type: "number" } }), + responses: { "200": { description: "OK" } }, + }, + }, + }, +}); + +const serveJson = (body: unknown) => + Effect.acquireRelease( + Effect.promise( + () => + new Promise<{ readonly url: string; readonly close: () => void }>((resolve) => { + const server = createServer((_request, response) => { + response.setHeader("content-type", "application/json"); + response.end(JSON.stringify(body)); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resolve({ url: `http://127.0.0.1:${port}/openapi.json`, close: () => server.close() }); + }); + }), + ), + (server) => Effect.sync(() => server.close()), + ); + +const emulatorFetch = (url: string, body: Record) => + Effect.promise(async () => { + const response = await fetch(url, { + method: "POST", + headers: { authorization: `Bearer ${BOT_TOKEN}` }, + body: new URLSearchParams(body), + }); + return (await response.json()) as { readonly ts: string }; + }); + +it.effect( + "invoke accepts Slack string timestamps and reads history and threads from the emulator", + () => + Effect.scoped( + Effect.gen(function* () { + const emulator = yield* Effect.acquireRelease( + Effect.promise(() => + createEmulator({ + service: "slack", + seed: { + tokens: { + [BOT_TOKEN]: { login: "admin", scopes: ["chat:write", "channels:history"] }, + }, + slack: { channels: [{ name: "general" }] }, + }, + }), + ), + (running) => Effect.promise(() => running.close()), + ); + const apiUrl = `${emulator.url}/api`; + const channel = "C000000001"; + const root = yield* emulatorFetch(`${apiUrl}/chat.postMessage`, { channel, text: "root" }); + const reply = yield* emulatorFetch(`${apiUrl}/chat.postMessage`, { + channel, + text: "reply", + thread_ts: root.ts, + }); + + const spec = yield* serveJson(publishedSlackSpec(apiUrl)); + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [ + openApiPlugin({ + httpClientLayer: FetchHttpClient.layer, + specFormats: [slackWebApiAdapter], + }), + memoryCredentialsPlugin(), + ] as const, + }), + ); + yield* executor.openapi.addSpec({ + spec: { kind: "url", url: spec.url }, + specFormat: "slack-web-api", + slug: "slack", + baseUrl: apiUrl, + authenticationTemplate: [ + { + slug: AuthTemplateSlug.make("bearer"), + type: "apiKey", + headers: { authorization: ["Bearer ", { type: "variable" as const, name: "token" }] }, + }, + ], + }); + yield* executor.connections.create({ + owner: "org", + name: ConnectionName.make("main"), + integration: "slack" as never, + template: AuthTemplateSlug.make("bearer"), + value: BOT_TOKEN, + }); + + const engine: ExecutionEngine = { + execute: () => Effect.die("invoke never runs code"), + call: (address, args) => + executor.execute(ToolAddress.make(address), args).pipe( + Effect.map((result) => ({ result })), + Effect.orDie, + ), + executeWithPause: () => Effect.die("invoke never pauses"), + resume: () => Effect.succeed(null), + isExecutionSettled: undefined, + getPausedExecution: () => Effect.succeed(null), + pausedExecutionCount: () => Effect.succeed(0), + hasPausedExecutions: () => Effect.succeed(false), + getDescription: Effect.succeed("slack"), + shutdown: Effect.void, + }; + const mcpServer = yield* createExecutorMcpServer({ + engine, + mode: "passthrough", + tools: executor.tools, + connections: { list: () => Effect.succeed([]) }, + integrations: { list: () => Effect.succeed([]) }, + }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }, { capabilities: {} }); + yield* Effect.promise(() => mcpServer.connect(serverTransport)); + yield* Effect.promise(() => client.connect(clientTransport)); + + const invoke = (tool: string, args: Record) => + Effect.promise(() => + client.callTool({ + name: "invoke", + arguments: { tool: `tools.slack.org.main.${tool}`, arguments: args }, + }), + ); + + const history = yield* invoke("conversations.conversationsHistory", { + body: { channel, latest: reply.ts, oldest: "0", inclusive: true }, + }); + expect(JSON.stringify(history.content)).not.toContain("Invalid arguments"); + expect(history.isError ?? false).toBe(false); + expect(JSON.stringify(history.content)).toContain(root.ts); + + const replies = yield* invoke("conversations.conversationsReplies", { + body: { channel, ts: root.ts }, + }); + expect(replies.isError ?? false).toBe(false); + expect(JSON.stringify(replies.content)).toContain(reply.ts); + + yield* Effect.promise(() => clientTransport.close()); + }), + ), + { timeout: 30_000 }, +); diff --git a/packages/plugins/openapi/src/providers/slack/index.ts b/packages/plugins/openapi/src/providers/slack/index.ts index 4223051b17..ceb6d511e5 100644 --- a/packages/plugins/openapi/src/providers/slack/index.ts +++ b/packages/plugins/openapi/src/providers/slack/index.ts @@ -1 +1 @@ -export { slackWebApiAdapter, stripSlackTokenInputs } from "./spec-format-adapter"; +export { normalizeSlackSpec, slackWebApiAdapter } from "./spec-format-adapter"; diff --git a/packages/plugins/openapi/src/providers/slack/spec-format-adapter.test.ts b/packages/plugins/openapi/src/providers/slack/spec-format-adapter.test.ts index d8dafcc833..2c1013b16d 100644 --- a/packages/plugins/openapi/src/providers/slack/spec-format-adapter.test.ts +++ b/packages/plugins/openapi/src/providers/slack/spec-format-adapter.test.ts @@ -33,7 +33,26 @@ const slackFixture = { parameters: [ { name: "token", in: "query", schema: { type: "string" } }, { name: "channel", in: "query", schema: { type: "string" } }, + { name: "latest", in: "query", schema: { type: "number", default: 0 } }, + { name: "oldest", in: "query", schema: { type: "number", default: 0 } }, + { name: "limit", in: "query", schema: { type: "integer" } }, ], + responses: { + "200": { + description: "OK", + content: { + "application/json": { + schema: { type: "object", properties: { cache_ts: { type: "number" } } }, + }, + }, + }, + }, + }, + }, + "/conversations.replies": { + get: { + operationId: "conversations_replies", + parameters: [{ name: "ts", in: "query", required: true, schema: { type: "number" } }], responses: { "200": { description: "OK" } }, }, }, @@ -47,7 +66,11 @@ const slackFixture = { schema: { type: "object", required: ["channel", "token"], - properties: { channel: { type: "string" }, token: { type: "string" } }, + properties: { + channel: { type: "string" }, + token: { type: "string" }, + thread_ts: { type: "number" }, + }, }, }, }, @@ -113,6 +136,9 @@ it.effect("removes the token input so the connection's credential is the only on expect(spec.paths["/auth.test"].get.parameters).toEqual([]); expect(spec.paths["/conversations.history"].get.parameters).toEqual([ { name: "channel", in: "query", schema: { type: "string" } }, + { name: "latest", in: "query", schema: { type: "string" } }, + { name: "oldest", in: "query", schema: { type: "string" } }, + { name: "limit", in: "query", schema: { type: "integer" } }, ]); expect(spec.paths["/chat.postMessage"].post.parameters).toEqual([]); expect( @@ -121,7 +147,7 @@ it.effect("removes the token input so the connection's credential is the only on ).toEqual({ type: "object", required: ["channel"], - properties: { channel: { type: "string" } }, + properties: { channel: { type: "string" }, thread_ts: { type: "string" } }, }); expect( spec.paths["/files.upload"].post.requestBody.content["multipart/form-data"].schema, @@ -129,6 +155,12 @@ it.effect("removes the token input so the connection's credential is the only on type: "object", properties: { content: { type: "string" } }, }); + expect(spec.paths["/conversations.replies"].get.parameters).toEqual([ + { name: "ts", in: "query", required: true, schema: { type: "string" } }, + ]); + expect( + spec.paths["/conversations.history"].get.responses["200"].content["application/json"].schema, + ).toEqual({ type: "object", properties: { cache_ts: { type: "number" } } }); expect(converted.specUrl).toBe(SLACK_SPEC_URL); }), ); diff --git a/packages/plugins/openapi/src/providers/slack/spec-format-adapter.ts b/packages/plugins/openapi/src/providers/slack/spec-format-adapter.ts index eb5d303b52..570cc5f58b 100644 --- a/packages/plugins/openapi/src/providers/slack/spec-format-adapter.ts +++ b/packages/plugins/openapi/src/providers/slack/spec-format-adapter.ts @@ -12,6 +12,8 @@ const encodeJsonText = Schema.encodeUnknownSync(Schema.UnknownFromJsonString); const TOKEN_INPUT = "token"; +const TIMESTAMP_INPUT = /^(?:ts|latest|oldest|(?:.+_)?ts(?:_.+)?)$/; + const HTTP_METHODS = new Set(["get", "put", "post", "delete", "options", "head", "patch", "trace"]); const isRecord = (value: unknown): value is Record => @@ -22,7 +24,42 @@ const detectsSlackSpecUrl = (url: string): boolean => function withoutTokenParameter(parameters: unknown): unknown { if (!Array.isArray(parameters)) return parameters; - return parameters.filter((parameter) => !isRecord(parameter) || parameter.name !== TOKEN_INPUT); + return parameters + .filter((parameter) => !isRecord(parameter) || parameter.name !== TOKEN_INPUT) + .map(withStringTimestampParameter); +} + +const isTimestampInput = (name: unknown, schema: unknown): schema is Record => + typeof name === "string" && + TIMESTAMP_INPUT.test(name) && + isRecord(schema) && + schema.type === "number"; + +function asStringSchema(schema: Record): Record { + const { + default: _default, + minimum: _minimum, + maximum: _maximum, + format: _format, + ...rest + } = schema; + return { ...rest, type: "string" }; +} + +function withStringTimestampParameter(parameter: unknown): unknown { + if (!isRecord(parameter) || !isTimestampInput(parameter.name, parameter.schema)) return parameter; + return { ...parameter, schema: asStringSchema(parameter.schema) }; +} + +function withStringTimestampProperties(schema: unknown): unknown { + if (!isRecord(schema) || !isRecord(schema.properties)) return schema; + const properties = Object.fromEntries( + Object.entries(schema.properties).map(([name, property]) => [ + name, + isTimestampInput(name, property) ? asStringSchema(property) : property, + ]), + ); + return { ...schema, properties }; } function withoutTokenProperty(schema: unknown): unknown { @@ -46,7 +83,9 @@ function withoutTokenBody(requestBody: unknown): unknown { const content = Object.fromEntries( Object.entries(requestBody.content).map(([mediaType, media]) => [ mediaType, - isRecord(media) ? { ...media, schema: withoutTokenProperty(media.schema) } : media, + isRecord(media) + ? { ...media, schema: withStringTimestampProperties(withoutTokenProperty(media.schema)) } + : media, ]), ); return { ...requestBody, content }; @@ -64,13 +103,19 @@ function withoutTokenOperation(operation: unknown): unknown { } /** + * Slack message timestamps are decimal strings such as `1793135656.414489`, + * and a JSON number would lose the trailing digits. The published specs type + * the `ts`, `thread_ts`, `latest` and `oldest` inputs (and any other `*_ts` + * input) as `number`, so the invoke validator rejects the real values. Only + * request inputs are rewritten, never responses. + * * The published Slack specs predate header auth and declare the credential as * an ordinary `token` input on every operation. Left in, each tool asks the * caller for the token and sends it in the query or body, where Slack rejects * it for current apps. Dropping it leaves the connection's credential, sent as * an `Authorization` header, as the only one. */ -export function stripSlackTokenInputs(document: unknown): unknown { +export function normalizeSlackSpec(document: unknown): unknown { if (!isRecord(document) || !isRecord(document.paths)) return document; const paths = Object.fromEntries( Object.entries(document.paths).map(([path, pathItem]) => { @@ -101,6 +146,6 @@ export const slackWebApiAdapter: SpecFormatAdapter = { Effect.provide(input.httpClientLayer), ); const document = yield* parseSpecObject(specText); - return { specText: encodeJsonText(stripSlackTokenInputs(document)), specUrl: url }; + return { specText: encodeJsonText(normalizeSlackSpec(document)), specUrl: url }; }), };