diff --git a/.changeset/complete-safety-hint-trio.md b/.changeset/complete-safety-hint-trio.md new file mode 100644 index 0000000..e3bfe33 --- /dev/null +++ b/.changeset/complete-safety-hint-trio.md @@ -0,0 +1,8 @@ +--- +"@senderkit/sdk": minor +"@senderkit/cli": minor +--- + +Complete the MCP safety-hint trio on every manifest tool. `ToolAnnotations` is now an interface requiring explicit `readOnlyHint`, `openWorldHint`, and `destructiveHint` values — OpenAI's ChatGPT/Codex plugin review rejects tools that omit any of the three — and every `MCP_TOOLS` entry carries the completed trio. The send tools (`senderkit_send`, `senderkit_send_raw`) are the only `openWorldHint: true` tools, since they deliver messages to recipients outside SenderKit; read-only tools explicitly declare `openWorldHint: false, destructiveHint: false`. The CLI-bundled MCP server surfaces the completed trio over `tools/list` with no other behavior change. + +For type consumers this widens the previous one-hint union: code that constructed a partial `ToolAnnotations` (e.g. `{ readOnlyHint: true }`) must now state all three hints. diff --git a/packages/cli/test/mcp-server.test.ts b/packages/cli/test/mcp-server.test.ts index 094c46e..880f1a5 100644 --- a/packages/cli/test/mcp-server.test.ts +++ b/packages/cli/test/mcp-server.test.ts @@ -68,70 +68,97 @@ describe("registry", () => { }); describe("tool annotations", () => { - // Required by the Anthropic Claude Connectors Directory review: every tool - // carries a human-readable title and exactly one behaviour hint. + // The Anthropic Claude Connectors Directory review requires a human-readable + // title; OpenAI's ChatGPT/Codex plugin review additionally requires explicit + // readOnlyHint, openWorldHint, and destructiveHint values on every tool. + const READ_ONLY = { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + } as const; + const DESTRUCTIVE_WRITE = { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: true, + } as const; + // The send tools are the only open-world ones: they deliver messages to + // recipients outside SenderKit. + const OPEN_WORLD_SEND = { + readOnlyHint: false, + openWorldHint: true, + destructiveHint: true, + } as const; + const EXPECTED = { - senderkit_context: { title: "Get Workspace Context", hint: "readOnlyHint" }, - senderkit_send: { title: "Send Templated Message", hint: "destructiveHint" }, - senderkit_send_raw: { title: "Send Raw Message", hint: "destructiveHint" }, + senderkit_context: { + title: "Get Workspace Context", + annotations: READ_ONLY, + }, + senderkit_send: { + title: "Send Templated Message", + annotations: OPEN_WORLD_SEND, + }, + senderkit_send_raw: { + title: "Send Raw Message", + annotations: OPEN_WORLD_SEND, + }, senderkit_cancel_message: { title: "Cancel Scheduled Message", - hint: "destructiveHint", + annotations: DESTRUCTIVE_WRITE, }, - senderkit_messages_list: { title: "List Messages", hint: "readOnlyHint" }, - senderkit_messages_get: { title: "Get Message", hint: "readOnlyHint" }, - senderkit_templates_list: { title: "List Templates", hint: "readOnlyHint" }, - senderkit_templates_get: { title: "Get Template", hint: "readOnlyHint" }, + senderkit_messages_list: { title: "List Messages", annotations: READ_ONLY }, + senderkit_messages_get: { title: "Get Message", annotations: READ_ONLY }, + senderkit_templates_list: { + title: "List Templates", + annotations: READ_ONLY, + }, + senderkit_templates_get: { title: "Get Template", annotations: READ_ONLY }, senderkit_inbound_addresses_list: { title: "List Inbound Addresses", - hint: "readOnlyHint", + annotations: READ_ONLY, }, senderkit_inbound_addresses_create: { title: "Create Inbound Address", // Additive write: creating an address is fully reversed by deleting it, // so the manifest advertises an explicit non-destructive write. - hint: "destructiveHint", - value: false, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: false, + }, }, senderkit_inbound_addresses_delete: { title: "Delete Inbound Address", - hint: "destructiveHint", + annotations: DESTRUCTIVE_WRITE, }, senderkit_inbound_messages_list: { title: "List Inbound Messages", - hint: "readOnlyHint", + annotations: READ_ONLY, }, senderkit_inbound_messages_get: { title: "Get Inbound Message", - hint: "readOnlyHint", + annotations: READ_ONLY, }, senderkit_inbound_domains_list: { title: "List Inbound Domains", - hint: "readOnlyHint", + annotations: READ_ONLY, }, senderkit_inbound_domains_create: { title: "Claim Inbound Domain", - hint: "destructiveHint", + annotations: DESTRUCTIVE_WRITE, }, senderkit_inbound_domains_delete: { title: "Delete Inbound Domain", - hint: "destructiveHint", + annotations: DESTRUCTIVE_WRITE, }, } as const; - it("assigns the directory-required title and a single hint to every tool", () => { + it("assigns the directory-required title and the full hint trio to every tool", () => { for (const command of registry) { const expected = EXPECTED[command.mcpName as keyof typeof EXPECTED]; expect(expected, `no expectation for ${command.mcpName}`).toBeDefined(); expect(command.title).toBe(expected.title); - - // Exactly one of readOnlyHint / destructiveHint — true unless the entry - // pins an explicit value (destructiveHint: false = non-destructive write). - const hints = command.annotations as Record; - expect(hints[expected.hint]).toBe("value" in expected ? expected.value : true); - const other = - expected.hint === "readOnlyHint" ? "destructiveHint" : "readOnlyHint"; - expect(hints[other]).toBeUndefined(); + expect(command.annotations, command.mcpName).toEqual(expected.annotations); } }); @@ -154,9 +181,7 @@ describe("tool annotations", () => { for (const tool of tools) { const expected = EXPECTED[tool.name as keyof typeof EXPECTED]; expect(tool.title).toBe(expected.title); - expect(tool.annotations?.[expected.hint]).toBe( - "value" in expected ? expected.value : true, - ); + expect(tool.annotations, tool.name).toMatchObject(expected.annotations); } } finally { await client.close(); diff --git a/packages/sdk/src/mcp.ts b/packages/sdk/src/mcp.ts index 9508630..e148e8e 100644 --- a/packages/sdk/src/mcp.ts +++ b/packages/sdk/src/mcp.ts @@ -23,15 +23,27 @@ import { export * from "./mcp-schemas"; /** - * MCP behaviour hints surfaced to clients (and the Connectors Directory review). - * A tool declares exactly one hint: it is read-only, or it is a write — either - * destructive (`destructiveHint: true`) or explicitly non-destructive - * (`destructiveHint: false`, additive/reversible operations a client need not - * treat as irreversible). + * MCP behaviour hints surfaced to clients (and the connector directory + * reviews). Every tool states the full trio explicitly: OpenAI's ChatGPT/Codex + * plugin review rejects manifests whose tools omit `readOnlyHint`, + * `openWorldHint`, or `destructiveHint`, so the manifest ships completed + * values instead of leaving clients to infer the spec's defaults. */ -export type ToolAnnotations = - | { readOnlyHint: true; destructiveHint?: never } - | { destructiveHint: boolean; readOnlyHint?: never }; +export interface ToolAnnotations { + /** True when the tool only reads workspace state. */ + readOnlyHint: boolean; + /** + * True when the tool reaches beyond SenderKit — only the send tools, which + * deliver email/SMS/push/web-push to external recipients. + */ + openWorldHint: boolean; + /** + * True when the tool's effect is not recoverable (a cancelled send, a + * deleted address or domain, redirected domain mail). `false` on reads and + * on additive writes a client need not treat as irreversible. + */ + destructiveHint: boolean; +} /** * The declarative surface of one MCP tool, shared by the CLI-bundled server and @@ -57,7 +69,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "Call this before sending so you can tell the user which workspace they're " + "in and whether messages will be really delivered (live) or only recorded " + "without delivery (test).", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: contextInput, }, { @@ -66,7 +82,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ description: "Send a transactional email, SMS, push, or web-push notification to a " + "recipient using a saved template.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: true, + destructiveHint: true, + }, inputSchema: sendInput, }, { @@ -75,7 +95,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ description: "Send a transactional email, SMS, push, or web-push notification with " + "inline content, without a registered template.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: true, + destructiveHint: true, + }, inputSchema: sendRawInput, }, { @@ -84,7 +108,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ description: "List all message templates in the workspace across email, SMS, push, " + "and web-push, with slugs and channels.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: templatesListInput, }, { @@ -93,7 +121,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ description: "Fetch a template's content, variables, and current version by slug — " + "inspect what will actually be delivered before sending or editing.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: templatesGetInput, }, { @@ -104,7 +136,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "web-push. Filter by channel, template, delivery status, or metadata — " + "use this to monitor deliverability, debug failed sends, or audit " + "transactional message history.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: messagesListInput, }, { @@ -113,7 +149,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ description: "Fetch full details and delivery status for a single message by ID — " + "check whether a specific email or SMS was delivered, bounced, or failed.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: messagesGetInput, }, { @@ -121,7 +161,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ title: "Cancel Scheduled Message", description: "Cancel a still-pending message (scheduled or queued) by its public id.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: true, + }, inputSchema: cancelMessageInput, }, { @@ -131,7 +175,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "List the workspace's programmatic inbound email addresses — the " + "addresses that receive mail and forward it or fire a webhook into the " + "workspace.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: inboundAddressesListInput, }, { @@ -143,7 +191,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "receipt. Enforces the workspace's plan limit and validates " + "forwardTo/webhookEndpointId.", // Additive: mints a new address; deleting it again fully reverses it. - annotations: { destructiveHint: false }, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: inboundAddressesCreateInput, }, { @@ -153,7 +205,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "Delete one of the workspace's inbound email addresses by id. The " + "address immediately stops receiving new mail; already-received " + "messages are unaffected.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: true, + }, inputSchema: inboundAddressesDeleteInput, }, { @@ -164,7 +220,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "addresses, newest first. Filter by address or a receivedAt cursor — " + "use this to check whether mail has arrived, or to page through recent " + "receipts.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: inboundMessagesListInput, }, { @@ -175,7 +235,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "headers, subject, body, attachments (as authenticated v1 API links — " + "Bearer key with the inbound scope, not signed/presigned), and " + "spam/auth verdicts.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: inboundMessagesGetInput, }, { @@ -185,7 +249,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "List the workspace's custom inbound domains — including the shared " + "{slug}.in.senderkit.email domain, if used — with their verification " + "status and (for pending custom domains) the DNS records still needed.", - annotations: { readOnlyHint: true }, + annotations: { + readOnlyHint: true, + openWorldHint: false, + destructiveHint: false, + }, inputSchema: inboundDomainsListInput, }, { @@ -200,7 +268,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "acknowledgeExistingMx: true, since claiming will redirect ALL of that " + "domain's mail to SenderKit. Nothing is received until the records are " + "live and verification completes.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: true, + }, inputSchema: inboundDomainsCreateInput, }, { @@ -210,7 +282,11 @@ export const MCP_TOOLS: readonly McpToolSpec[] = [ "Delete a custom inbound domain by id. Its addresses stop receiving mail " + "immediately. The workspace's shared {slug}.in.senderkit.email domain " + "cannot be deleted.", - annotations: { destructiveHint: true }, + annotations: { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: true, + }, inputSchema: inboundDomainsDeleteInput, }, ]; diff --git a/packages/sdk/test/mcp.test.ts b/packages/sdk/test/mcp.test.ts index 37e8832..943cb89 100644 --- a/packages/sdk/test/mcp.test.ts +++ b/packages/sdk/test/mcp.test.ts @@ -35,22 +35,40 @@ describe("MCP_TOOLS", () => { } }); - it("each tool sets exactly one behaviour hint, with a non-empty title + description", () => { + it("sets all three safety hints explicitly, with a non-empty title + description", () => { + // OpenAI's ChatGPT/Codex plugin review requires explicit readOnlyHint, + // openWorldHint, and destructiveHint values on every scanned tool. for (const t of MCP_TOOLS) { - const a = t.annotations as { - readOnlyHint?: boolean; - destructiveHint?: boolean; - }; - expect("readOnlyHint" in a !== "destructiveHint" in a, t.name).toBe(true); + expect(typeof t.annotations.readOnlyHint, t.name).toBe("boolean"); + expect(typeof t.annotations.openWorldHint, t.name).toBe("boolean"); + expect(typeof t.annotations.destructiveHint, t.name).toBe("boolean"); + // A read-only tool cannot also mutate or reach outside SenderKit. + if (t.annotations.readOnlyHint) { + expect(t.annotations.destructiveHint, t.name).toBe(false); + expect(t.annotations.openWorldHint, t.name).toBe(false); + } expect(t.title.length).toBeGreaterThan(0); expect(t.description.length).toBeGreaterThan(0); } }); + it("marks exactly the send tools open-world", () => { + // Only senderkit_send / senderkit_send_raw deliver messages to recipients + // outside SenderKit; everything else operates on workspace state. + const openWorld = MCP_TOOLS.filter((t) => t.annotations.openWorldHint).map( + (t) => t.name, + ); + expect(openWorld.sort()).toEqual(["senderkit_send", "senderkit_send_raw"]); + }); + it("admits a non-destructive write annotation (destructiveHint: false)", () => { // Type-level: app-only tools (e.g. templates_create) declare additive - // writes; the shared union must not force true. - const nonDestructiveWrite: ToolAnnotations = { destructiveHint: false }; + // writes; the shared shape must not force true. + const nonDestructiveWrite: ToolAnnotations = { + readOnlyHint: false, + openWorldHint: false, + destructiveHint: false, + }; expect(nonDestructiveWrite.destructiveHint).toBe(false); }); @@ -112,7 +130,11 @@ describe("inbound manifest parity with the hosted app's definitions", () => { // reversible operation. expect( MCP_TOOLS_BY_NAME.senderkit_inbound_addresses_create.annotations, - ).toEqual({ destructiveHint: false }); + ).toEqual({ + readOnlyHint: false, + openWorldHint: false, + destructiveHint: false, + }); }); it("livemode description does not promise a quota exemption", () => {