From b0e4d07f8f3270bdbac57bd65819d4e2c580f51e Mon Sep 17 00:00:00 2001 From: Travis Fischer Date: Thu, 12 Jun 2025 05:15:14 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/__snapshots__/mcp-e2e.test.ts.snap | 8 +- apps/e2e/src/http-fixtures.ts | 4 +- apps/e2e/src/mcp-e2e.test.ts | 17 ++-- apps/e2e/src/mcp-fixtures.ts | 87 ++++++++++++++++++- ...eate-http-request-for-openapi-operation.ts | 2 +- .../src/lib/get-tool-args-from-request.ts | 3 +- .../src/lib/resolve-origin-tool-call.ts | 12 ++- ...http-response-to-mcp-tool-call-response.ts | 10 ++- apps/gateway/src/lib/types.ts | 2 + .../everything-openapi/agentic.config.ts | 3 +- packages/types/src/agentic-project-config.ts | 22 ++++- packages/types/src/openapi.d.ts | 4 +- packages/types/src/tools.ts | 17 +++- packages/types/src/types.ts | 11 ++- 14 files changed, 169 insertions(+), 33 deletions(-) diff --git a/apps/e2e/src/__snapshots__/mcp-e2e.test.ts.snap b/apps/e2e/src/__snapshots__/mcp-e2e.test.ts.snap index 0e6709ff..cfa8c872 100644 --- a/apps/e2e/src/__snapshots__/mcp-e2e.test.ts.snap +++ b/apps/e2e/src/__snapshots__/mcp-e2e.test.ts.snap @@ -1,21 +1,21 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`MCP => MCP origin basic "add" tool call success > 9.0: @dev/test-basic-mcp/mcp add 1`] = ` +exports[`MCP => MCP origin basic "echo" tool > 10.0: @dev/test-basic-mcp/mcp echo 1`] = ` { "content": [ { - "text": "62", + "text": "{"nala":"kitten","num":123,"now":1749678890348}", "type": "text", }, ], } `; -exports[`MCP => MCP origin basic "echo" tool > 10.0: @dev/test-basic-mcp/mcp echo 1`] = ` +exports[`MCP => MCP origin basic "echo" tool with empty body > 13.0: @dev/test-basic-mcp/mcp echo 1`] = ` { "content": [ { - "text": "{"nala":"kitten","num":123,"now":1749678633338}", + "text": "{}", "type": "text", }, ], diff --git a/apps/e2e/src/http-fixtures.ts b/apps/e2e/src/http-fixtures.ts index 0557c244..ef8011f1 100644 --- a/apps/e2e/src/http-fixtures.ts +++ b/apps/e2e/src/http-fixtures.ts @@ -422,7 +422,7 @@ export const fixtureSuites: E2ETestFixtureSuite[] = [ ] }, { - title: 'HTTP => MCP origin basic "add" tool call success', + title: 'HTTP => MCP origin basic "add" tool', compareResponseBodies: true, fixtures: [ { @@ -549,7 +549,7 @@ export const fixtureSuites: E2ETestFixtureSuite[] = [ ] }, { - title: 'HTTP => OpenAPI origin everything "disabled_tool"', + title: 'HTTP => OpenAPI origin everything "disabled_tool" tool', fixtures: [ { path: '@dev/test-everything-openapi/disabled_tool', diff --git a/apps/e2e/src/mcp-e2e.test.ts b/apps/e2e/src/mcp-e2e.test.ts index cc2fafe5..eb740d3f 100644 --- a/apps/e2e/src/mcp-e2e.test.ts +++ b/apps/e2e/src/mcp-e2e.test.ts @@ -58,7 +58,9 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) { fixture.response?.snapshot ?? fixtureSuite.snapshot ?? false const expectedStableSnapshot = fixture.response?.stableSnapshot ?? + fixture.response?.snapshot ?? fixtureSuite.stableSnapshot ?? + fixtureSuite.snapshot ?? !isError const debugFixture = !!( fixture.debug ?? @@ -148,10 +150,15 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) { expect(result).toMatchSnapshot() } + const stableResult = pick( + result, + 'content', + 'structuredContent', + 'isError' + ) + if (expectedStableSnapshot) { - expect( - pick(result, 'content', 'structuredContent', 'isError') - ).toMatchSnapshot() + expect(stableResult).toMatchSnapshot() } if (validate) { @@ -160,9 +167,9 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) { if (compareResponseBodies && !isError) { if (!fixtureResult) { - fixtureResult = result + fixtureResult = stableResult } else { - expect(result).toEqual(fixtureResult) + expect(stableResult).toEqual(fixtureResult) } } } diff --git a/apps/e2e/src/mcp-fixtures.ts b/apps/e2e/src/mcp-fixtures.ts index f092fe40..54259758 100644 --- a/apps/e2e/src/mcp-fixtures.ts +++ b/apps/e2e/src/mcp-fixtures.ts @@ -1,3 +1,5 @@ +import { expect } from 'vitest' + export type MCPE2ETestFixture = { /** @default 60_000 milliseconds */ timeout?: number @@ -16,6 +18,7 @@ export type MCPE2ETestFixture = { response?: { result?: any + /** @default false */ isError?: boolean content?: Array> structuredContent?: any @@ -369,6 +372,7 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [ { title: 'MCP => MCP origin basic "add" tool call success', path: '@dev/test-basic-mcp/mcp', + stableSnapshot: false, fixtures: [ { request: { @@ -411,8 +415,8 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [ ] }, { - title: 'MCP => MCP origin basic "pure" tool', - path: '@dev/test-basic-mcp/mcp', + title: 'MCP => OpenAPI origin everything "pure" tool', + path: '@dev/test-everything-openapi/mcp', fixtures: [ { request: { @@ -477,5 +481,84 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [ } } ] + }, + { + title: 'MCP => OpenAPI origin everything "disabled_tool" tool', + path: '@dev/test-everything-openapi/mcp', + fixtures: [ + { + request: { + name: 'disabled_tool', + args: { + foo: 'bar' + } + }, + response: { + isError: true, + _agenticMeta: { + status: 404, + toolName: 'disabled_tool' + } + } + } + ] + }, + { + title: 'MCP => OpenAPI origin everything "echo" tool with empty body', + path: '@dev/test-everything-openapi/mcp', + fixtures: [ + { + request: { + name: 'echo', + args: {} + }, + response: { + isError: false, + content: [{ type: 'text', text: JSON.stringify({}) }] + } + } + ] + }, + { + title: 'MCP => OpenAPI origin everything "unpure_marked_pure" tool', + path: '@dev/test-everything-openapi/mcp', + compareResponseBodies: true, + only: true, + fixtures: [ + { + request: { + name: 'unpure_marked_pure', + args: { + nala: 'cat' + } + }, + response: { + isError: false, + validate: (result) => { + const body = JSON.parse(result.content[0].text) + expect(body?.nala).toEqual('cat') + expect(typeof body.now).toBe('number') + expect(body.now).toBeGreaterThan(0) + } + } + }, + { + // compareResponseBodies should result in the same cached response body, + // even though the origin would return a different `now` value if it + // weren't marked `pure`. + request: { + name: 'unpure_marked_pure', + args: { + nala: 'cat' + } + }, + response: { + isError: false, + _agenticMeta: { + cacheStatus: 'HIT' + } + } + } + ] } ] diff --git a/apps/gateway/src/lib/create-http-request-for-openapi-operation.ts b/apps/gateway/src/lib/create-http-request-for-openapi-operation.ts index 68e54111..2706b103 100644 --- a/apps/gateway/src/lib/create-http-request-for-openapi-operation.ts +++ b/apps/gateway/src/lib/create-http-request-for-openapi-operation.ts @@ -48,7 +48,7 @@ export async function createHttpRequestForOpenAPIOperation({ ) const extraArgs = - toolConfig?.additionalProperties === false + toolConfig?.inputSchemaAdditionalProperties === false ? [] : // TODO: Make this more efficient... Object.keys(toolCallArgs).filter((key) => { diff --git a/apps/gateway/src/lib/get-tool-args-from-request.ts b/apps/gateway/src/lib/get-tool-args-from-request.ts index 1ca1c5ab..6c00e0f2 100644 --- a/apps/gateway/src/lib/get-tool-args-from-request.ts +++ b/apps/gateway/src/lib/get-tool-args-from-request.ts @@ -38,7 +38,8 @@ export async function getToolArgsFromRequest( data: incomingRequestArgsRaw, errorPrefix: `Invalid request parameters for tool "${tool.name}"`, coerce: true, - strictAdditionalProperties: toolConfig?.additionalProperties === false + strictAdditionalProperties: + toolConfig?.inputSchemaAdditionalProperties === false }) return incomingRequestArgs diff --git a/apps/gateway/src/lib/resolve-origin-tool-call.ts b/apps/gateway/src/lib/resolve-origin-tool-call.ts index e7370db9..ff37d379 100644 --- a/apps/gateway/src/lib/resolve-origin-tool-call.ts +++ b/apps/gateway/src/lib/resolve-origin-tool-call.ts @@ -197,7 +197,8 @@ export async function resolveOriginToolCall({ schema: tool.inputSchema, data: args, errorPrefix: `Invalid request parameters for tool "${tool.name}"`, - strictAdditionalProperties: toolConfig?.additionalProperties === false + strictAdditionalProperties: + toolConfig?.inputSchemaAdditionalProperties === false }) const originStartTimeMs = Date.now() @@ -247,7 +248,8 @@ export async function resolveOriginToolCall({ originRequest, originResponse, originTimespanMs: Date.now() - originStartTimeMs, - numRequestsCost + numRequestsCost, + toolConfig } } else if (originAdapter.type === 'mcp') { const { projectIdentifier } = parseDeploymentIdentifier( @@ -313,7 +315,8 @@ export async function resolveOriginToolCall({ toolCallArgs, toolCallResponse: (await response.json()) as McpToolCallResponse, originTimespanMs: Date.now() - originStartTimeMs, - numRequestsCost + numRequestsCost, + toolConfig } } } @@ -346,7 +349,8 @@ export async function resolveOriginToolCall({ toolCallArgs, toolCallResponse, originTimespanMs: Date.now() - originStartTimeMs, - numRequestsCost + numRequestsCost, + toolConfig } } else { assert( diff --git a/apps/gateway/src/lib/transform-http-response-to-mcp-tool-call-response.ts b/apps/gateway/src/lib/transform-http-response-to-mcp-tool-call-response.ts index b92199f9..5d8e03d2 100644 --- a/apps/gateway/src/lib/transform-http-response-to-mcp-tool-call-response.ts +++ b/apps/gateway/src/lib/transform-http-response-to-mcp-tool-call-response.ts @@ -1,4 +1,4 @@ -import type { Tool } from '@agentic/platform-types' +import type { Tool, ToolConfig } from '@agentic/platform-types' import { assert, HttpError } from '@agentic/platform-core' import contentType from 'fast-content-type-parse' @@ -9,12 +9,14 @@ export async function transformHttpResponseToMcpToolCallResponse({ originRequest, originResponse, tool, - toolCallArgs + toolCallArgs, + toolConfig }: { originRequest: Request originResponse: Response tool: Tool toolCallArgs: ToolCallArgs + toolConfig?: ToolConfig }) { const { type: mimeType } = contentType.safeParse( originResponse.headers.get('content-type') || 'application/octet-stream' @@ -77,8 +79,8 @@ export async function transformHttpResponseToMcpToolCallResponse({ data, schema: tool.outputSchema, coerce: false, - // TODO: double-check MCP schema on whether additional properties are allowed - strictAdditionalProperties: true, + strictAdditionalProperties: + toolConfig?.outputSchemaAdditionalProperties === false, errorPrefix: `Invalid tool response for tool "${tool.name}"`, errorStatusCode: 502 }) diff --git a/apps/gateway/src/lib/types.ts b/apps/gateway/src/lib/types.ts index 2970f4e0..39400e7d 100644 --- a/apps/gateway/src/lib/types.ts +++ b/apps/gateway/src/lib/types.ts @@ -6,6 +6,7 @@ import type { } from '@agentic/platform-hono' import type { AdminConsumer as AdminConsumerImpl, + ToolConfig, User } from '@agentic/platform-types' import type { Client as McpClient } from '@modelcontextprotocol/sdk/client/index.js' @@ -67,6 +68,7 @@ export type ResolvedOriginToolCallResult = { rateLimitResult?: RateLimitResult cacheStatus: CacheStatus reportUsage: boolean + toolConfig?: ToolConfig originTimespanMs: number numRequestsCost: number } & ( diff --git a/packages/fixtures/valid/everything-openapi/agentic.config.ts b/packages/fixtures/valid/everything-openapi/agentic.config.ts index b5bd4b80..412453ab 100644 --- a/packages/fixtures/valid/everything-openapi/agentic.config.ts +++ b/packages/fixtures/valid/everything-openapi/agentic.config.ts @@ -68,7 +68,8 @@ export default defineConfig({ }, { name: 'strict_additional_properties', - additionalProperties: false + inputSchemaAdditionalProperties: false, + outputSchemaAdditionalProperties: false } ] }) diff --git a/packages/types/src/agentic-project-config.ts b/packages/types/src/agentic-project-config.ts index bcd9eef9..7726bbfa 100644 --- a/packages/types/src/agentic-project-config.ts +++ b/packages/types/src/agentic-project-config.ts @@ -12,7 +12,12 @@ import { type PricingPlanListInput, pricingPlanListSchema } from './pricing' -import { toolConfigSchema, toolSchema } from './tools' +import { + type ToolConfig, + type ToolConfigInput, + toolConfigSchema, + toolSchema +} from './tools' // TODO: // - optional external auth provider config (google, github, twitter, etc) @@ -159,16 +164,21 @@ To add support for annual pricing plans, for example, you can use: \`['month', ' .strip() export type AgenticProjectConfigInput = Simplify< - Omit, 'pricingPlans'> & { + Omit< + z.input, + 'pricingPlans' | 'toolConfigs' + > & { pricingPlans?: PricingPlanListInput + toolConfigs?: ToolConfigInput[] } > export type AgenticProjectConfigRaw = z.output< typeof agenticProjectConfigSchema > export type AgenticProjectConfig = Simplify< - Omit & { + Omit & { pricingPlans: PricingPlanList + toolConfigs: ToolConfig[] } > @@ -178,7 +188,11 @@ export const resolvedAgenticProjectConfigSchema = tools: z.array(toolSchema).default([]) }) export type ResolvedAgenticProjectConfig = Simplify< - Omit, 'pricingPlans'> & { + Omit< + z.output, + 'pricingPlans' | 'toolConfigs' + > & { pricingPlans: PricingPlanList + toolConfigs: ToolConfig[] } > diff --git a/packages/types/src/openapi.d.ts b/packages/types/src/openapi.d.ts index 5c52abbd..cb203d4b 100644 --- a/packages/types/src/openapi.d.ts +++ b/packages/types/src/openapi.d.ts @@ -506,7 +506,9 @@ export interface components { reportUsage: boolean; rateLimit?: components["schemas"]["RateLimit"] | null; /** @default true */ - additionalProperties: boolean; + inputSchemaAdditionalProperties: boolean; + /** @default true */ + outputSchemaAdditionalProperties: boolean; /** @description Allows you to override this tool's behavior or disable it entirely for different pricing plans. This is a map of PricingPlan slug to PricingPlanToolOverrides for that plan. */ pricingPlanOverridesMap?: { [key: string]: components["schemas"]["PricingPlanToolOverride"]; diff --git a/packages/types/src/tools.ts b/packages/types/src/tools.ts index 4fb83d96..2d0a3f55 100644 --- a/packages/types/src/tools.ts +++ b/packages/types/src/tools.ts @@ -173,9 +173,23 @@ export const toolConfigSchema = z * The default MCP spec allows additional properties. Set this to `false` if * you want your tool to be more strict. * + * @note This is only relevant if the tool has defined an `outputSchema`. + * * @default true */ - additionalProperties: z.boolean().optional().default(true), + inputSchemaAdditionalProperties: z.boolean().optional().default(true), + + /** + * Whether to allow additional properties in the tool's output schema. + * + * The default MCP spec allows additional properties. Set this to `false` if + * you want your tool to be more strict. + * + * @note This is only relevant if the tool has defined an `outputSchema`. + * + * @default true + */ + outputSchemaAdditionalProperties: z.boolean().optional().default(true), /** * Allows you to override this tool's behavior or disable it entirely for @@ -202,6 +216,7 @@ export const toolConfigSchema = z // headers }) .openapi('ToolConfig') +export type ToolConfigInput = z.input export type ToolConfig = z.infer /** diff --git a/packages/types/src/types.ts b/packages/types/src/types.ts index bcd58e50..415fef14 100644 --- a/packages/types/src/types.ts +++ b/packages/types/src/types.ts @@ -4,7 +4,7 @@ * types than what `@hono/zod-openapi` and `zod` v3 provide, but in general * these types are meant to use the backend API as a source of truth. */ -import type { PricingPlan } from '@agentic/platform-types' +import type { PricingPlan, ToolConfig } from '@agentic/platform-types' import type { Simplify } from 'type-fest' import type { components } from './openapi.d.ts' @@ -16,14 +16,19 @@ export type Team = components['schemas']['Team'] export type TeamMember = components['schemas']['TeamMember'] export type Deployment = Simplify< - Omit & { + Omit & { pricingPlans: PricingPlan[] + toolConfigs: ToolConfig[] } > export type AdminDeployment = Simplify< - Omit & { + Omit< + components['schemas']['AdminDeployment'], + 'pricingPlans' | 'toolConfigs' + > & { pricingPlans: PricingPlan[] + toolConfigs: ToolConfig[] } >