Skip to content

Commit c60ef06

Browse files
committed
fix(mcp): pick the read or write tool from the operation's declared scope; CORS on the MCP host
1 parent ebf5cf4 commit c60ef06

7 files changed

Lines changed: 208 additions & 86 deletions

File tree

apps/docs/content/docs/mcp/tools.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ calls it.
1010

1111
| Tool | Does |
1212
| --- | --- |
13-
| `search_operations` | Finds operations by keyword (`"table rows"`) or by area (`tables`, `workflows`, `knowledge`, …). Returns each operation's name, method, path, summary, and whether it is read-only. |
13+
| `search_operations` | Finds operations by keyword (`"table rows"`) or by area (`tables`, `workflows`, `knowledge`, …). Returns each operation's name, method, path, summary, and the tool that runs it. |
1414
| `describe_operation` | Returns an operation's description and the JSON Schema of its path parameters, query, body, and headers. |
15-
| `call_read_operation` | Runs a read-only operation, such as `listWorkspaces`, `listTables`, or `getWorkflowRun`. It leaves your resources unchanged. |
16-
| `call_write_operation` | Runs an operation that creates, changes, runs, or deletes something, such as `createTable`, `executeWorkflow`, or `deleteFile`. |
15+
| `call_read_operation` | Runs an operation that only needs read access, such as `listWorkspaces`, `queryRows`, or `getWorkflowRun`. |
16+
| `call_write_operation` | Runs an operation that needs write access: one that creates, changes, runs, or deletes something, or reaches out to another service, such as `createTable`, `executeWorkflow`, or `listMcpServerTools`. |
1717

1818
Reads and writes are separate tools so your app can approve reads once and still
1919
ask you before each change.

apps/sim/lib/api/mcp/catalog.test.ts

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,49 @@ import { describe, expect, it } from 'vitest'
55
import {
66
callerHeaderNames,
77
describeOperation,
8+
getMcpOperation,
89
OPERATION_DOMAINS,
910
resolveOperation,
1011
searchOperations,
1112
} from '@/lib/api/mcp/catalog'
1213
import { V2_MCP_OPERATIONS, type V2McpOperationName } from '@/lib/api/mcp/generated/v2-operations'
14+
import { v2RouteOperation } from '@/lib/api/server/routes/v2-json-route'
1315

1416
const ALL_OPERATION_NAMES = Object.keys(V2_MCP_OPERATIONS) as V2McpOperationName[]
1517

1618
describe('Sim MCP catalog', () => {
17-
it('routes GET operations to the read tool and everything else to the write tool', () => {
19+
/**
20+
* Loads every route the catalog names, so a handler that is missing, or a
21+
* declared operation the tool split cannot read, fails here rather than on a
22+
* caller's first call.
23+
*/
24+
it('gives every operation exactly one tool, from the scope its route declares', async () => {
1825
for (const name of ALL_OPERATION_NAMES) {
19-
const isGet = V2_MCP_OPERATIONS[name].contract.method === 'GET'
20-
expect('operation' in resolveOperation(name, isGet ? 'read' : 'write')).toBe(true)
21-
expect(resolveOperation(name, isGet ? 'write' : 'read')).toEqual({
22-
error: expect.stringContaining(isGet ? 'call_read_operation' : 'call_write_operation'),
23-
})
24-
expect(resolveOperation(name, 'any')).toEqual({ operation: name })
26+
const route = await getMcpOperation(name).handler()
27+
expect(typeof route, name).toBe('function')
28+
const scope = v2RouteOperation(route)?.oauthScope
29+
const tool = scope === 'api:read' || scope === 'search:read' ? 'read' : 'write'
30+
const other = tool === 'read' ? 'write' : 'read'
31+
expect(await resolveOperation(name, tool), name).toEqual({ operation: name })
32+
expect(await resolveOperation(name, other), name).toEqual({ error: expect.any(String) })
2533
}
34+
}, 120_000)
35+
36+
it('classifies by declared scope, not HTTP method', async () => {
37+
expect(await resolveOperation('listMcpServerTools', 'read')).toEqual({
38+
error: 'listMcpServerTools needs write access; run it with call_write_operation.',
39+
})
40+
expect(await resolveOperation('queryRows', 'read')).toEqual({ operation: 'queryRows' })
41+
expect(await resolveOperation('executeWorkflow', 'write')).toEqual({
42+
operation: 'executeWorkflow',
43+
})
2644
})
2745

28-
it('suggests the closest operations for an unknown name', () => {
29-
const resolved = resolveOperation('createTableRow', 'write')
30-
expect(resolved).toEqual({ error: expect.stringContaining('createTableRows') })
31-
expect(resolveOperation('toString', 'any')).toEqual({
46+
it('suggests the closest operations for an unknown name', async () => {
47+
expect(await resolveOperation('createTableRow', 'write')).toEqual({
48+
error: expect.stringContaining('createTableRows'),
49+
})
50+
expect(await resolveOperation('toString', 'any')).toEqual({
3251
error: expect.stringContaining('Unknown operation "toString"'),
3352
})
3453
})
@@ -40,9 +59,9 @@ describe('Sim MCP catalog', () => {
4059
expect(names).toContain('executeWorkflow')
4160
})
4261

43-
it('describes every operation as JSON Schema', () => {
62+
it('describes every operation as JSON Schema', async () => {
4463
for (const name of ALL_OPERATION_NAMES) {
45-
const description = describeOperation(name)
64+
const description = await describeOperation(name)
4665
expect(description.operation).toBe(name)
4766
const { contract } = V2_MCP_OPERATIONS[name]
4867
for (const slot of ['params', 'query', 'body'] as const) {
@@ -54,34 +73,35 @@ describe('Sim MCP catalog', () => {
5473
expect(description.input[slot]).not.toHaveProperty('$schema')
5574
}
5675
}
57-
})
76+
}, 120_000)
5877

59-
it('describes path parameters, query, and body', () => {
60-
const { input, readOnly, domain, description } = describeOperation('createTable')
78+
it('describes path parameters, query, body, and the tool to use', async () => {
79+
const { input, tool, domain, description } = await describeOperation('createTable')
6180
expect(description).toEqual(expect.any(String))
62-
expect(readOnly).toBe(false)
81+
expect(tool).toBe('call_write_operation')
6382
expect(domain).toBe('tables')
6483
expect(input.body).toMatchObject({ type: 'object' })
6584
expect(input.body?.properties).toHaveProperty('workspaceId')
66-
expect(describeOperation('getTable').input.params?.properties).toHaveProperty('tableId')
85+
expect((await describeOperation('getTable')).input.params?.properties).toHaveProperty('tableId')
6786
})
6887

69-
it('exposes only the contract headers a caller may set', () => {
88+
it('exposes only the contract headers a caller may set', async () => {
7089
expect(callerHeaderNames('completeFileUpload')).toEqual(['upload-token'])
7190
expect(callerHeaderNames('listTables')).toEqual([])
72-
expect(describeOperation('listTables').input.headers).toBeUndefined()
91+
expect((await describeOperation('listTables')).input.headers).toBeUndefined()
7392
})
7493

75-
it('ranks name matches first and requires every term', () => {
76-
const { operations } = searchOperations({ query: 'table rows', limit: 50 })
94+
it('ranks name matches first and names each result’s tool', async () => {
95+
const { operations } = await searchOperations({ query: 'table rows', limit: 50 })
7796
expect(operations.length).toBeGreaterThan(0)
7897
expect(operations[0].operation.toLowerCase()).toContain('rows')
7998
expect(operations[0].operation.toLowerCase()).toContain('table')
99+
expect(operations.every((entry) => entry.tool.startsWith('call_'))).toBe(true)
80100
})
81101

82-
it('filters by domain and bounds the page', () => {
102+
it('filters by domain and bounds the page', async () => {
83103
expect(OPERATION_DOMAINS).toContain('workflows')
84-
const { total, operations } = searchOperations({ domain: 'workflows', limit: 3 })
104+
const { total, operations } = await searchOperations({ domain: 'workflows', limit: 3 })
85105
expect(operations).toHaveLength(3)
86106
expect(total).toBeGreaterThan(3)
87107
expect(operations.every((entry) => entry.domain === 'workflows')).toBe(true)

apps/sim/lib/api/mcp/catalog.ts

Lines changed: 96 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { z } from 'zod'
33
import type { ApiSchema, HttpMethod } from '@/lib/api/contracts/types'
44
import { V2_MCP_OPERATIONS, type V2McpOperationName } from '@/lib/api/mcp/generated/v2-operations'
55
import type { V2McpOperation } from '@/lib/api/mcp/types'
6+
import { v2RouteOperation } from '@/lib/api/server/routes/v2-json-route'
7+
import { OAUTH_API_READ_SCOPE, oauthScopeSatisfies } from '@/lib/auth/oauth-provider'
68

79
/** Headers the dispatcher owns; a contract declaring one still never takes it from a tool call. */
810
const MANAGED_HEADERS: ReadonlySet<string> = new Set([
@@ -13,23 +15,31 @@ const MANAGED_HEADERS: ReadonlySet<string> = new Set([
1315
'user-agent',
1416
])
1517

18+
/** The tool that runs each kind of operation. */
19+
export const TOOL_NAMES = { read: 'call_read_operation', write: 'call_write_operation' } as const
20+
type ToolKind = keyof typeof TOOL_NAMES
21+
1622
const REQUEST_SLOTS = ['params', 'query', 'body', 'headers'] as const
1723
type RequestSlot = (typeof REQUEST_SLOTS)[number]
1824
type JsonSchema = Record<string, unknown>
1925

2026
/** One catalog row: enough to choose an operation, not to call it. */
21-
interface McpOperationSummary {
27+
interface McpOperationEntry {
2228
operation: V2McpOperationName
2329
method: HttpMethod
2430
path: string
2531
domain: string
2632
summary: string
27-
/** A `GET`: served by the read tool, which only reads. */
28-
readOnly: boolean
33+
/** The tool that runs it: the read tool only for operations that need nothing beyond read access. */
2934
/** Refuses workspace API keys; call it with a personal key or an OAuth connection. */
3035
personalCredentialOnly?: true
3136
}
3237

38+
interface McpOperationSummary extends McpOperationEntry {
39+
/** The tool that runs it: the read tool only for operations that need nothing beyond read access. */
40+
tool: (typeof TOOL_NAMES)[ToolKind]
41+
}
42+
3343
/** Everything needed to call one operation: its documentation plus the JSON Schema of each request slot. */
3444
interface McpOperationDescription extends McpOperationSummary {
3545
description?: string
@@ -42,8 +52,34 @@ export function getMcpOperation(name: V2McpOperationName): V2McpOperation {
4252
return V2_MCP_OPERATIONS[name]
4353
}
4454

45-
function isReadOnlyOperation(name: V2McpOperationName): boolean {
46-
return getMcpOperation(name).contract.method === 'GET'
55+
/** Memo of each operation's tool; a failed load is dropped so the next call retries it. */
56+
const toolKinds = new Map<V2McpOperationName, Promise<ToolKind>>()
57+
58+
/**
59+
* Which tool runs an operation, from the OAuth scope its route declares rather
60+
* than its HTTP method: a GET can need `api:write` and reach out to another
61+
* system, and a POST can be a pure query. Raw routes declare no operation and
62+
* all change something, so they run through the write tool.
63+
*/
64+
function operationToolKind(name: V2McpOperationName): Promise<ToolKind> {
65+
const cached = toolKinds.get(name)
66+
if (cached) return cached
67+
const kind = getMcpOperation(name)
68+
.handler()
69+
.then((route): ToolKind => {
70+
const scope = v2RouteOperation(route)?.oauthScope
71+
return scope && oauthScopeSatisfies([OAUTH_API_READ_SCOPE], scope) ? 'read' : 'write'
72+
})
73+
.catch((error: unknown) => {
74+
toolKinds.delete(name)
75+
throw error
76+
})
77+
toolKinds.set(name, kind)
78+
return kind
79+
}
80+
81+
async function withTool(entry: McpOperationEntry): Promise<McpOperationSummary> {
82+
return { ...entry, tool: TOOL_NAMES[await operationToolKind(entry.operation)] }
4783
}
4884

4985
function isOperationName(name: string): name is V2McpOperationName {
@@ -55,15 +91,14 @@ function domainOf(path: string): string {
5591
return path.split('/')[3] ?? 'v2'
5692
}
5793

58-
function summarize(name: V2McpOperationName): McpOperationSummary {
94+
function summarize(name: V2McpOperationName): McpOperationEntry {
5995
const { contract, summary, workspaceKeyUnsupported } = getMcpOperation(name)
6096
return {
6197
operation: name,
6298
method: contract.method,
6399
path: contract.path,
64100
domain: domainOf(contract.path),
65101
summary: summary ?? `${contract.method} ${contract.path}`,
66-
readOnly: isReadOnlyOperation(name),
67102
...(workspaceKeyUnsupported ? { personalCredentialOnly: true } : {}),
68103
}
69104
}
@@ -87,17 +122,17 @@ if (!FIRST_DOMAIN) throw new Error('The Sim MCP catalog has no operations')
87122
export const OPERATION_DOMAINS: [string, ...string[]] = [FIRST_DOMAIN, ...OTHER_DOMAINS]
88123

89124
/**
90-
* Finds operations by keyword and domain. Every term must appear in the
125+
* Ranks operations by keyword and domain. Every term must appear in the
91126
* operation's name, summary, path, or description; hits in the name rank first.
92127
*/
93-
export function searchOperations(options: { query?: string; domain?: string; limit: number }): {
94-
total: number
95-
operations: McpOperationSummary[]
96-
} {
97-
const terms = (options.query ?? '').toLowerCase().split(/\s+/).filter(Boolean)
98-
const ranked: Array<{ entry: McpOperationSummary; score: number }> = []
128+
function rankOperations(
129+
query: string | undefined,
130+
domain: string | undefined
131+
): McpOperationEntry[] {
132+
const terms = (query ?? '').toLowerCase().split(/\s+/).filter(Boolean)
133+
const ranked: Array<{ entry: McpOperationEntry; score: number }> = []
99134
for (const { entry, name, summary, path, description } of SEARCH_ENTRIES) {
100-
if (options.domain && entry.domain !== options.domain) continue
135+
if (domain && entry.domain !== domain) continue
101136
let score = 0
102137
for (const term of terms) {
103138
const termScore =
@@ -114,9 +149,18 @@ export function searchOperations(options: { query?: string; domain?: string; lim
114149
if (score > 0 || terms.length === 0) ranked.push({ entry, score })
115150
}
116151
ranked.sort((a, b) => b.score - a.score || a.entry.operation.localeCompare(b.entry.operation))
152+
return ranked.map(({ entry }) => entry)
153+
}
154+
155+
export async function searchOperations(options: {
156+
query?: string
157+
domain?: string
158+
limit: number
159+
}): Promise<{ total: number; operations: McpOperationSummary[] }> {
160+
const ranked = rankOperations(options.query, options.domain)
117161
return {
118162
total: ranked.length,
119-
operations: ranked.slice(0, options.limit).map(({ entry }) => entry),
163+
operations: await Promise.all(ranked.slice(0, options.limit).map(withTool)),
120164
}
121165
}
122166

@@ -143,18 +187,14 @@ function callerHeaderSchema(schema: ApiSchema): JsonSchema | null {
143187
return { ...json, properties: allowed, ...(required ? { required } : {}) }
144188
}
145189

146-
/** Contract headers a tool call may set. */
147-
export function callerHeaderNames(name: V2McpOperationName): string[] {
148-
return Object.keys(toRecord(describeOperation(name).input.headers?.properties))
149-
}
150-
151190
/** Memo over a fixed catalog: at most one entry per operation, never evicted. */
152-
const descriptions = new Map<V2McpOperationName, McpOperationDescription>()
191+
const inputs = new Map<V2McpOperationName, McpOperationDescription['input']>()
153192

154-
export function describeOperation(name: V2McpOperationName): McpOperationDescription {
155-
const cached = descriptions.get(name)
193+
/** The JSON Schema of each request slot an operation takes. */
194+
function describeInput(name: V2McpOperationName): McpOperationDescription['input'] {
195+
const cached = inputs.get(name)
156196
if (cached) return cached
157-
const { contract, description } = getMcpOperation(name)
197+
const { contract } = getMcpOperation(name)
158198
const input: McpOperationDescription['input'] = {}
159199
for (const slot of REQUEST_SLOTS) {
160200
const schema = contract[slot]
@@ -166,9 +206,24 @@ export function describeOperation(name: V2McpOperationName): McpOperationDescrip
166206
}
167207
input[slot] = toJsonSchema(schema)
168208
}
169-
const described = { ...summarize(name), ...(description ? { description } : {}), input }
170-
descriptions.set(name, described)
171-
return described
209+
inputs.set(name, input)
210+
return input
211+
}
212+
213+
/** Contract headers a tool call may set. */
214+
export function callerHeaderNames(name: V2McpOperationName): string[] {
215+
return Object.keys(toRecord(describeInput(name).headers?.properties))
216+
}
217+
218+
export async function describeOperation(
219+
name: V2McpOperationName
220+
): Promise<McpOperationDescription> {
221+
const { description } = getMcpOperation(name)
222+
return {
223+
...(await withTool(summarize(name))),
224+
...(description ? { description } : {}),
225+
input: describeInput(name),
226+
}
172227
}
173228

174229
/** Catalog names close to an unknown one, found by searching its camelCase words. */
@@ -179,8 +234,8 @@ function suggestOperations(name: string): string[] {
179234
.split(/[^a-z0-9]+/)
180235
.filter(Boolean)
181236
for (let count = words.length; count > 0; count--) {
182-
const { operations } = searchOperations({ query: words.slice(0, count).join(' '), limit: 5 })
183-
if (operations.length > 0) return operations.map((entry) => entry.operation)
237+
const ranked = rankOperations(words.slice(0, count).join(' '), undefined)
238+
if (ranked.length > 0) return ranked.slice(0, 5).map((entry) => entry.operation)
184239
}
185240
return []
186241
}
@@ -190,10 +245,10 @@ function suggestOperations(name: string): string[] {
190245
* the closest names for an unknown one, or the other tool for the wrong kind.
191246
* `read` and `write` are the read and write tools; `any` is describe_operation.
192247
*/
193-
export function resolveOperation(
248+
export async function resolveOperation(
194249
name: string,
195-
tool: 'read' | 'write' | 'any'
196-
): { operation: V2McpOperationName } | { error: string } {
250+
tool: ToolKind | 'any'
251+
): Promise<{ operation: V2McpOperationName } | { error: string }> {
197252
if (!isOperationName(name)) {
198253
const suggestions = suggestOperations(name)
199254
return {
@@ -202,11 +257,13 @@ export function resolveOperation(
202257
} Use search_operations to find operations.`,
203258
}
204259
}
205-
if (tool === 'read' && !isReadOnlyOperation(name)) {
206-
return { error: `${name} changes data; run it with call_write_operation.` }
207-
}
208-
if (tool === 'write' && isReadOnlyOperation(name)) {
209-
return { error: `${name} is read-only; run it with call_read_operation.` }
260+
if (tool === 'any') return { operation: name }
261+
const kind = await operationToolKind(name)
262+
if (kind === tool) return { operation: name }
263+
return {
264+
error:
265+
kind === 'write'
266+
? `${name} needs write access; run it with ${TOOL_NAMES.write}.`
267+
: `${name} only reads; run it with ${TOOL_NAMES.read}.`,
210268
}
211-
return { operation: name }
212269
}

0 commit comments

Comments
 (0)