diff --git a/apps/server/src/mcp/McpHttpServer.test.ts b/apps/server/src/mcp/McpHttpServer.test.ts index 9d28aeb4dfc1..90c4af4d6e0d 100644 --- a/apps/server/src/mcp/McpHttpServer.test.ts +++ b/apps/server/src/mcp/McpHttpServer.test.ts @@ -34,7 +34,10 @@ import * as McpToolAccess from "./McpToolAccess.ts"; import * as McpToolAccessTestkit from "./McpToolAccess.testkit.ts"; import * as McpInvocationContext from "./McpInvocationContext.ts"; import * as McpSessionRegistry from "./McpSessionRegistry.ts"; +import * as OrchestratorMcpService from "./OrchestratorMcpService.ts"; import * as PreviewAutomationBroker from "./PreviewAutomationBroker.ts"; +import { OrchestratorToolkit } from "./toolkits/orchestrator/tools.ts"; +import { ProjectToolkit } from "./toolkits/project/tools.ts"; const environmentId = EnvironmentId.make("environment-mcp-test"); const threadId = ThreadId.make("thread-mcp-test"); @@ -125,17 +128,55 @@ const serveSnapshots = (clientId: string, result: unknown) => return inputs; }); -const callSnapshot = (args: Record) => +const callTool = (name: string, args: Record) => Effect.gen(function* () { const server = yield* McpServer.McpServer; return yield* server - .callTool({ name: "preview_snapshot", arguments: args }) + .callTool({ name, arguments: args }) .pipe( Effect.provideService(McpInvocationContext.McpInvocationContext, invocation), Effect.provideService(McpSchema.McpServerClient, client), ); }); +const callSnapshot = (args: Record) => callTool("preview_snapshot", args); + +// Real tool definitions with handlers that record what they decoded. +const layerRecordingTools = (calls: Array) => { + const RecordingToolkit = Toolkit.make( + OrchestratorToolkit.tools.delegate_task, + OrchestratorToolkit.tools.t3_thread_read, + OrchestratorToolkit.tools.t3_thread_wait, + ProjectToolkit.tools.t3_project_update, + ); + const record = McpToolAccess.reads((params: unknown) => + Effect.sync(() => calls.push(params)).pipe( + Effect.andThen( + Effect.fail(new OrchestratorMcpFailure({ code: "thread_not_found", message: "Recorded." })), + ), + ), + ); + return McpHttpServer.toolkitRegistration( + RecordingToolkit, + McpToolAccess.toLayer(RecordingToolkit, { + delegate_task: record, + t3_thread_read: record, + t3_thread_wait: record, + t3_project_update: record, + }), + ).pipe( + Layer.provideMerge(McpServer.McpServer.layer), + Layer.provide( + Layer.mergeAll( + McpToolAccessTestkit.liveThreadsLayer, + Layer.mock(OrchestratorMcpService.OrchestratorMcpService)({}), + Layer.mock(ProjectService.ProjectService)({}), + NodeServices.layer, + ), + ), + ); +}; + it("normalizes empty successful notification responses to accepted", () => { const notificationResponse = McpHttpServer.normalizeMcpHttpResponse( HttpServerResponse.text("", { status: 200, contentType: "application/json" }), @@ -390,7 +431,7 @@ it.effect("refuses preview tools to a client outside a thread before they run", it.effect("rejects non-boolean snapshot image options before selecting a browser host", () => Effect.gen(function* () { const server = yield* McpServer.McpServer; - for (const includeImage of ["false", 0, null]) { + for (const includeImage of ["false", 0]) { const result = yield* server .callTool({ name: "preview_snapshot", @@ -447,6 +488,58 @@ it.effect("saves the snapshot PNG on request and reports its path", () => ).pipe(Effect.provide(layerTest)), ); +it.effect("treats null snapshot options like omitted ones", () => + Effect.scoped( + Effect.gen(function* () { + const inputs = yield* serveSnapshots("mcp-null-options-client", snapshotResult); + + const omitted = yield* callSnapshot({}); + const nulls = yield* callSnapshot({ tabId: null, includeImage: null, save: null }); + + expect(nulls).toEqual(omitted); + expect(inputs).toEqual([{}, {}]); + }), + ).pipe(Effect.provide(layerTest)), +); + +it.effect("decodes null optional arguments the way it decodes omitted ones", () => { + const calls: Array = []; + return Effect.gen(function* () { + const thread = { threadId: "thread-null-arguments" }; + yield* callTool("t3_thread_read", thread); + yield* callTool("t3_thread_read", { ...thread, afterPosition: null, view: null }); + yield* callTool("delegate_task", { task: "Inspect", target: {} }); + yield* callTool("delegate_task", { task: "Inspect", title: null, target: { model: null } }); + // A null the schema declares keeps its meaning: here it clears the project icon. + yield* callTool("t3_project_update", { projectId: "project-1", projectIcon: null }); + + expect(calls).toHaveLength(5); + expect(calls[1]).toEqual(calls[0]); + expect(calls[3]).toEqual(calls[2]); + expect(calls[4]).toStrictEqual({ projectId: "project-1", projectIcon: null }); + }).pipe(Effect.provide(layerRecordingTools(calls))); +}); + +it.effect("accepts finite wait budgets and refuses non-finite ones", () => { + const calls: Array = []; + return Effect.gen(function* () { + yield* callTool("delegate_task", { task: "Inspect", timeoutMs: 1500.5 }); + yield* callTool("t3_thread_wait", { threadId: "thread-wait", timeoutMs: 5000 }); + expect(calls).toMatchObject([{ timeoutMs: 1500.5 }, { timeoutMs: 5000 }]); + + for (const [name, args] of [ + ["delegate_task", { task: "Inspect", timeoutMs: "Infinity" }], + ["t3_thread_wait", { threadId: "thread-wait", timeoutMs: "NaN" }], + ["t3_thread_wait", { threadId: "thread-wait", timeoutMs: "-Infinity" }], + ["t3_thread_read", { threadId: "thread-wait", afterPosition: "2" }], + ] as const) { + const refused = yield* callTool(name, args).pipe(Effect.flip); + expect(refused._tag, `${name} ${JSON.stringify(args)}`).toBe("InvalidParams"); + } + expect(calls).toHaveLength(2); + }).pipe(Effect.provide(layerRecordingTools(calls))); +}); + it.effect("reports a tagged error when the screenshot cannot be saved", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/mcp/McpHttpServer.ts b/apps/server/src/mcp/McpHttpServer.ts index cd2ccf141638..80d43043df6a 100644 --- a/apps/server/src/mcp/McpHttpServer.ts +++ b/apps/server/src/mcp/McpHttpServer.ts @@ -11,7 +11,7 @@ import * as Schema from "effect/Schema"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; import type * as Types from "effect/Types"; -import { AiError, McpProtocol, McpSchema, McpServer, Tool, type Toolkit } from "effect/ai"; +import { AiError, McpProtocol, McpSchema, McpServer, Tool, Toolkit } from "effect/ai"; import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/http"; import { OrchestratorMcpFailure, PreviewAutomationError } from "@t3tools/contracts"; @@ -386,6 +386,22 @@ const saveScreenshot = Effect.fn("McpHttpServer.saveScreenshot")(function* ( return screenshotPath; }); +/** + * Tool input schemas are advertised through each schema's JSON codec, which + * offers `null` for every `Schema.optional` field, but Toolkit decodes with the + * schema itself and rejects that `null`. Decoding through the same codec makes + * a tool accept what it advertises. + */ +const decodeAsAdvertised = >( + toolkit: Toolkit.Toolkit, +): Toolkit.Toolkit => + // The clones keep each tool's id, name and decoded parameter type, so handlers still match. + Toolkit.make( + ...Object.values(toolkit.tools).map((tool) => + tool.setParameters(Schema.toCodecJson(tool.parametersSchema)), + ), + ) as unknown as Toolkit.Toolkit; + const isPreviewAutomationError = Schema.is(PreviewAutomationError); const previewSnapshotFailure = (cause: Cause.Cause) => { @@ -431,7 +447,7 @@ const registerPreviewSnapshot = Effect.fn("McpHttpServer.registerPreviewSnapshot const saveServices = yield* Effect.context< ServerConfig.ServerConfig | FileSystem.FileSystem | Path.Path | Crypto.Crypto >(); - const built = yield* PreviewSnapshotToolkit; + const built = yield* decodeAsAdvertised(PreviewSnapshotToolkit); const tool = PreviewSnapshotTool; yield* server.addTool({ tool: new McpSchema.Tool({ @@ -687,7 +703,7 @@ const registerImageTool = ( const registerDeviceScreenshot = Effect.fn("McpHttpServer.registerDeviceScreenshot")(function* () { const devices = yield* DeviceService.DeviceService; const threads = yield* ThreadManagementService.ThreadManagementService; - const built = yield* DeviceScreenshotToolkit; + const built = yield* decodeAsAdvertised(DeviceScreenshotToolkit); yield* registerImageTool( DeviceScreenshotTool, (payload) => @@ -709,7 +725,7 @@ const isOrchestratorMcpFailure = Schema.is(OrchestratorMcpFailure); const registerHtmlPreview = Effect.fn("McpHttpServer.registerHtmlPreview")(function* () { const htmlRender = yield* HtmlRender.HtmlRender; const threads = yield* ThreadManagementService.ThreadManagementService; - const built = yield* HtmlPreviewToolkit; + const built = yield* decodeAsAdvertised(HtmlPreviewToolkit); yield* registerImageTool( HtmlPreviewTool, (payload) => @@ -740,7 +756,7 @@ export const toolkitRegistration = , EX, toolkit: Toolkit.Toolkit, handlers: McpToolAccess.HandlersLayer, ) => { - const registration = McpServer.toolkit(toolkit); + const registration = McpServer.toolkit(decodeAsAdvertised(toolkit)); // @effect-diagnostics-next-line unsafeEffectTypeAssertion:off - the auth middleware provides it per request. const registered = registration as Layer.Layer< never, diff --git a/packages/contracts/src/orchestratorMcp.ts b/packages/contracts/src/orchestratorMcp.ts index 382cea55bb15..5e178c21551a 100644 --- a/packages/contracts/src/orchestratorMcp.ts +++ b/packages/contracts/src/orchestratorMcp.ts @@ -173,7 +173,7 @@ export const OrchestratorMcpDelegateTaskInput = Schema.Struct({ "Defaults to async. Use wait only when this turn needs the child's result before you can continue.", }), ), - timeoutMs: Schema.optional(Schema.Number).annotate({ + timeoutMs: Schema.optional(Schema.Finite).annotate({ description: "Wait budget for mode=wait only. Default 10 minutes. Elapsing it returns waitTimedOut=true on that call and does not cancel the child.", }), @@ -443,7 +443,7 @@ export type OrchestratorMcpThreadSendResult = typeof OrchestratorMcpThreadSendRe export const OrchestratorMcpThreadWaitInput = Schema.Struct({ threadId: ThreadId, runId: Schema.optional(RunId), - timeoutMs: Schema.optional(Schema.Number), + timeoutMs: Schema.optional(Schema.Finite), }); export type OrchestratorMcpThreadWaitInput = typeof OrchestratorMcpThreadWaitInput.Type;