diff --git a/apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts b/apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts index da5c76d48b..4a487444fd 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts @@ -547,24 +547,25 @@ test('rolls back only candidate-owned IPC after a registration collision', async test('closes the claimed Host connection when native capability construction fails', async () => { const ipc = ipcHarness(); const host = connectionHarness('invalid-capability'); - const invalidTool = { - ...nativeTool(), - parameters: z.string(), - } as unknown as MakaTool; + // A native-capability construction failure must still tear down the claimed + // Host connection. A tool whose *schema* is unrepresentable no longer fails + // construction — it is skipped and warned per-tool so one bad tool cannot + // drop every Desktop capability (see runtime-host-native-capabilities.test.ts) + // — so trigger a genuine construction error: two tools colliding on one name. await assert.rejects( () => createDesktopRuntimeHostCandidate( host.connection, deps(ipc, { - browserTools: [invalidTool], + browserTools: [nativeTool(), nativeTool()], resolveBrowserUrl: () => 'https://example.com/', releaseBrowserSession() {}, computerUseTools: emptyComputerUseTools(), releaseComputerUseSession() {}, }), ), - /tool schema must be an object/, + /Duplicate Desktop native capability tool/, ); assert.equal(ipc.size, 0); diff --git a/apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts b/apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts index 3e2f36007d..04be65170a 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-native-capabilities.test.ts @@ -22,6 +22,8 @@ import test from 'node:test'; import { buildComputerUseTools, type ComputerUseToolSet } from '@maka/runtime/computer-use-tools'; import { type CuDispatchBackend } from '@maka/runtime/computer-use-types'; import { type MakaTool, type MakaToolContext } from '@maka/runtime/tool-runtime'; +import { buildMcpTools, type McpToolProvider } from '@maka/runtime/mcp-tools'; +import type { McpToolBinding } from '@maka/core/mcp'; import type { ClientCapabilityProvider } from '@maka/runtime-host/client'; import { decodeClientCapabilityReplaceInput, @@ -133,6 +135,286 @@ test('publishes the real Computer Use schema through the Client Capability proto assert.equal(Array.isArray(coordinateSchema?.coordinate?.items), true); }); +test('offers and dispatches MCP tools whose parameters are JSON Schema, not Zod', async () => { + let receivedArgs: unknown; + const mcpProvider: McpToolProvider = { + toolSnapshot: () => ({ + revision: 1, + tools: [ + { + descriptor: { + serverId: 'filesystem', + name: 'read_file', + description: 'Read a file', + inputSchema: { + type: 'object', + properties: { value: { type: 'string' } }, + }, + }, + binding: 'binding-1' as McpToolBinding, + }, + ], + }), + callTool: async (_binding, args) => { + receivedArgs = args; + return { content: [{ type: 'text', text: JSON.stringify(args) }] }; + }, + }; + // Real production projection: parameters become jsonSchema(...), not Zod. + const mcpTools = buildMcpTools(mcpProvider); + const mcpTool = mcpTools[0]; + assert.ok(mcpTool, 'expected buildMcpTools to project one tool'); + + const provider = createDesktopNativeCapabilityProvider({ + browserTools: [], + resolveBrowserUrl: () => 'https://example.com/', + releaseBrowserSession() {}, + computerUseTools: computerTools(), + releaseComputerUseSession() {}, + additionalGroups: () => [ + { + offerId: 'desktop_mcp', + label: 'MCP', + description: 'MCP tools', + tools: mcpTools, + }, + ], + }); + + // Offer path: the JSON Schema is advertised verbatim and survives protocol encoding. + const offer = provider.offers().find((entry) => entry.offerId === 'desktop_mcp'); + assert.ok(offer, 'expected a desktop_mcp offer'); + const inputSchema = offer.tools[0]?.inputSchema; + assert.equal(inputSchema?.type, 'object'); + assert.deepEqual( + Object.keys((inputSchema?.properties as object | undefined) ?? {}), + ['value'], + ); + assert.doesNotThrow(() => + decodeClientCapabilityReplaceInput({ + registrationId: 'registration-1', + offers: provider.offers(), + }), + ); + + // Call path: args flow through to callTool without a Zod parser, and the MCP + // result is projected back over the protocol. + const result = await call( + provider, + capabilityFrame({ + offerId: 'desktop_mcp', + serverId: 'desktop_mcp', + toolName: mcpTool.name, + arguments: { value: 'hi' }, + }), + ); + assert.deepEqual(receivedArgs, { value: 'hi' }); + assert.deepEqual(result, { + content: [{ type: 'text', text: JSON.stringify({ value: 'hi' }) }], + }); +}); + +test('drops one unrepresentable tool instead of failing every Desktop capability', () => { + const mcpProvider: McpToolProvider = { + toolSnapshot: () => ({ + revision: 1, + tools: [ + { + descriptor: { + serverId: 'filesystem', + name: 'read_file', + description: 'Read a file', + inputSchema: { + type: 'object', + properties: { value: { type: 'string' } }, + }, + }, + binding: 'binding-read' as McpToolBinding, + }, + { + descriptor: { + serverId: 'filesystem', + name: 'read_tuple', + // `prefixItems` (a pydantic `tuple[...]` produces it) is outside the + // Client Capability schema allowlist, so this tool cannot be offered. + description: 'Uses a JSON Schema keyword the protocol rejects', + inputSchema: { + type: 'object', + properties: { + pair: { + type: 'array', + prefixItems: [{ type: 'string' }, { type: 'number' }], + }, + }, + }, + }, + binding: 'binding-tuple' as McpToolBinding, + }, + ], + }), + callTool: async () => ({ content: [] }), + }; + const mcpTools = buildMcpTools(mcpProvider); + const survivingToolName = mcpTools[0]?.name; + const droppedToolName = mcpTools[1]?.name; + assert.ok(survivingToolName && droppedToolName); + + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(String(args[0])); + }; + let provider: ReturnType; + try { + provider = createDesktopNativeCapabilityProvider({ + browserTools: [tool('browser_snapshot', z.object({}), async () => 'ok')], + resolveBrowserUrl: () => 'https://example.com/', + releaseBrowserSession() {}, + computerUseTools: computerTools(), + releaseComputerUseSession() {}, + additionalGroups: () => [ + { offerId: 'desktop_mcp', label: 'MCP', description: 'MCP tools', tools: mcpTools }, + ], + }); + } finally { + console.warn = originalWarn; + } + + // The unrepresentable tool is dropped, yet its server and every other Desktop + // capability still register — the #4591 outage took all of them down at once. + assert.deepEqual( + provider.offers().map((offer) => offer.offerId), + ['desktop_browser', 'desktop_mcp'], + ); + const mcpOffer = provider.offers().find((offer) => offer.offerId === 'desktop_mcp'); + assert.deepEqual( + mcpOffer?.tools.map((descriptor) => descriptor.name), + [survivingToolName], + ); + assert.equal(warnings.some((line) => line.includes(droppedToolName)), true); + + // The surviving frame still encodes cleanly over the protocol. + assert.doesNotThrow(() => + decodeClientCapabilityReplaceInput({ + registrationId: 'registration-1', + offers: provider.offers(), + }), + ); +}); + +test('does not dispatch a tool that was dropped from its offer', async () => { + const mcpProvider: McpToolProvider = { + toolSnapshot: () => ({ + revision: 1, + tools: [ + { + descriptor: { + serverId: 'filesystem', + name: 'read_file', + description: 'Read a file', + inputSchema: { type: 'object', properties: { value: { type: 'string' } } }, + }, + binding: 'binding-ok' as McpToolBinding, + }, + { + descriptor: { + serverId: 'filesystem', + name: 'read_tuple', + description: 'Uses a JSON Schema keyword the protocol rejects', + inputSchema: { + type: 'object', + properties: { pair: { type: 'array', prefixItems: [{ type: 'string' }] } }, + }, + }, + binding: 'binding-bad' as McpToolBinding, + }, + ], + }), + callTool: async () => ({ content: [{ type: 'text', text: 'should not run' }] }), + }; + const mcpTools = buildMcpTools(mcpProvider); + const droppedName = mcpTools[1]?.name; + assert.ok(droppedName); + + const originalWarn = console.warn; + console.warn = () => {}; + let provider: ReturnType; + try { + provider = createDesktopNativeCapabilityProvider({ + browserTools: [], + resolveBrowserUrl: () => 'https://example.com/', + releaseBrowserSession() {}, + computerUseTools: computerTools(), + releaseComputerUseSession() {}, + additionalGroups: () => [ + { offerId: 'desktop_mcp', label: 'MCP', description: 'MCP tools', tools: mcpTools }, + ], + }); + } finally { + console.warn = originalWarn; + } + + const mcpOffer = provider.offers().find((offer) => offer.offerId === 'desktop_mcp'); + assert.equal(mcpOffer?.tools.some((descriptor) => descriptor.name === droppedName), false); + // A tool that was never advertised must not be dispatchable, even though the + // MakaTool still exists in the source group — bindings track the advertised + // snapshot, not the raw group. + await assert.rejects( + () => + call( + provider, + capabilityFrame({ + offerId: 'desktop_mcp', + serverId: 'desktop_mcp', + toolName: droppedName, + arguments: { pair: ['x'] }, + }), + ), + /not offered/u, + ); +}); + +test('sheds a capability whose offer metadata is invalid without failing the rest', () => { + const originalWarn = console.warn; + console.warn = () => {}; + let provider: ReturnType; + try { + provider = createDesktopNativeCapabilityProvider({ + browserTools: [tool('browser_snapshot', z.object({}), async () => 'ok')], + resolveBrowserUrl: () => 'https://example.com/', + releaseBrowserSession() {}, + computerUseTools: computerTools(), + releaseComputerUseSession() {}, + additionalGroups: () => [ + // Invalid offerId — a caller misconfiguration. Its one tool has a + // perfectly valid schema, yet the group cannot be advertised, so the + // whole group is dropped while every other capability still registers. + { + offerId: 'bad offer id!', + label: 'Bad', + description: 'bad', + tools: [tool('fine_tool', z.object({}), async () => 'ok')], + }, + ], + }); + } finally { + console.warn = originalWarn; + } + + // The misconfigured capability is dropped; Browser still registers, and the + // surviving frame encodes cleanly over the protocol. + assert.deepEqual( + provider.offers().map((offer) => offer.offerId), + ['desktop_browser'], + ); + assert.doesNotThrow(() => + decodeClientCapabilityReplaceInput({ + registrationId: 'registration-1', + offers: provider.offers(), + }), + ); +}); + test('publishes every production Desktop-owned tool schema through the protocol', () => { const settingsTools = buildClientSettingsTools({ async read() { @@ -145,6 +427,7 @@ test('publishes every production Desktop-owned tool schema through the protocol' return false; }, }); + const riveTools = [buildRiveWorkflowTool()]; const provider = createDesktopNativeCapabilityProvider({ browserTools: [], resolveBrowserUrl: () => 'https://example.com/', @@ -162,7 +445,7 @@ test('publishes every production Desktop-owned tool schema through the protocol' offerId: 'desktop_rive', label: 'Rive', description: 'Rive workflows', - tools: [buildRiveWorkflowTool()], + tools: riveTools, }, ], }); @@ -173,6 +456,19 @@ test('publishes every production Desktop-owned tool schema through the protocol' offers: provider.offers(), }), ); + // Assert the tools by name, not just `doesNotThrow`: a tool dropped for an + // unrepresentable schema now vanishes silently rather than throwing, so this + // is what makes such a regression fail CI instead of passing green. + assert.deepEqual( + provider.offers().map((offer) => ({ + offerId: offer.offerId, + toolNames: offer.tools.map((descriptor) => descriptor.name), + })), + [ + { offerId: 'desktop_settings', toolNames: settingsTools.map((entry) => entry.name) }, + { offerId: 'desktop_rive', toolNames: riveTools.map((entry) => entry.name) }, + ], + ); }); test('publishes and admits additional Desktop native-effect services', async () => { diff --git a/apps/desktop/src/main/runtime-host-native-capabilities.ts b/apps/desktop/src/main/runtime-host-native-capabilities.ts index bebe81db1c..def1b4faf0 100644 --- a/apps/desktop/src/main/runtime-host-native-capabilities.ts +++ b/apps/desktop/src/main/runtime-host-native-capabilities.ts @@ -25,14 +25,16 @@ import { type ClientCapabilityProvider, type OAuthPresentationBackend, } from "@maka/runtime-host/client"; -import type { - ClientCapabilityCallFrame, - ClientCapabilityCallResult, - ClientCapabilityContentBlock, - ClientCapabilityHostPathAccess, - ClientCapabilityOffer, - ClientCapabilityServiceCallFrame, - ClientCapabilityServiceOffer, +import { + decodeClientCapabilityReplaceInput, + type ClientCapabilityCallFrame, + type ClientCapabilityCallResult, + type ClientCapabilityContentBlock, + type ClientCapabilityHostPathAccess, + type ClientCapabilityOffer, + type ClientCapabilityServiceCallFrame, + type ClientCapabilityServiceOffer, + type ClientCapabilityToolDescriptor, } from "@maka/runtime-host/protocol"; import { toJSONSchema, z } from "zod"; import { withBrowserOriginAdmission } from './browser/browser-origin-admission.js'; @@ -41,6 +43,9 @@ import type { DesktopTargetScope } from '../shared/runtime-host-identity.js'; const CAPABILITY_VERSION = "0"; const BROWSER_OFFER_ID = "desktop_browser"; const COMPUTER_USE_OFFER_ID = "desktop_computer_use"; +// A registration id used only to validate a single tool's descriptor against +// the protocol before advertising it; never sent to a Host. +const CAPABILITY_PROBE_REGISTRATION_ID = "desktop-capability-tool-probe"; export interface DesktopCapabilityGroup { readonly offerId: string; @@ -53,6 +58,16 @@ interface NativeToolBinding { readonly tool: MakaTool; } +/** + * An advertised offer paired with the Maka tools that survived validation, in + * the same order as `offer.tools`. Bindings are built from this so the provider + * only ever dispatches a tool it actually advertised. + */ +interface ResolvedCapability { + readonly offer: ClientCapabilityOffer; + readonly tools: readonly MakaTool[]; +} + type DesktopToolModelOutput = Awaited< ReturnType> >; @@ -114,10 +129,11 @@ export function createDesktopNativeCapabilityProvider( ): DesktopNativeCapabilityProvider { const groups = capabilityGroups(input); const hostPathAccess = providerOptions.hostPathAccess ?? "cwd"; - const offers = Object.freeze( - groups.map((group) => capabilityOffer(group, hostPathAccess)), - ); - const bindings = indexBindings(groups); + const resolved = groups + .map((group) => capabilityOffer(group, hostPathAccess)) + .filter((entry): entry is ResolvedCapability => entry !== undefined); + const offers = Object.freeze(resolved.map((entry) => entry.offer)); + const bindings = indexBindings(resolved); const oauthPresentation = input.oauthPresentation ? createOAuthPresentationClientProvider(input.oauthPresentation) : undefined; @@ -337,8 +353,7 @@ async function invokeNativeTool( } const signal = AbortSignal.any([options.signal, invocation.signal]); signal.throwIfAborted(); - const parameters = requireZodSchema(binding.tool); - const args = await parameters.parseAsync(frame.arguments); + const args = await parseToolArguments(binding.tool, frame.arguments); signal.throwIfAborted(); const sessionId = frame.offerId === BROWSER_OFFER_ID || frame.offerId === COMPUTER_USE_OFFER_ID @@ -422,39 +437,98 @@ function abortInvocations( function capabilityOffer( group: DesktopCapabilityGroup, hostPathAccess: ClientCapabilityHostPathAccess, -): ClientCapabilityOffer { - return Object.freeze({ +): ResolvedCapability | undefined { + const surviving: { + readonly descriptor: ClientCapabilityToolDescriptor; + readonly tool: MakaTool; + }[] = []; + for (const tool of group.tools) { + const descriptor = offerableToolDescriptor(group, tool, hostPathAccess); + if (descriptor) surviving.push({ descriptor, tool }); + } + if (surviving.length === 0) { + console.warn( + `[capabilities] Desktop capability ${group.offerId} has no representable tools; not advertising it`, + ); + return undefined; + } + const offer: ClientCapabilityOffer = Object.freeze({ offerId: group.offerId, version: CAPABILITY_VERSION, - affinity: "session", + affinity: "session" as const, hostPathAccess, label: group.label, description: group.description, - tools: Object.freeze( - group.tools.map((tool) => - Object.freeze({ - serverId: group.offerId, - name: tool.name, - description: tool.description, - inputSchema: toolInputSchema(tool), - ...(tool.activityKind ? { activityKind: tool.activityKind } : {}), - ...(tool.displayName - ? { annotations: Object.freeze({ title: tool.displayName }) } - : {}), - }), - ), - ), + tools: Object.freeze(surviving.map((entry) => entry.descriptor)), }); + return { offer, tools: surviving.map((entry) => entry.tool) }; +} + +/** + * Build one tool's protocol descriptor, or skip it. `replace()` sends the whole + * registration as a single frame (`client-capability-channel.ts`), so a tool + * whose schema cannot be expressed — an unrepresentable Zod type, or a JSON + * Schema keyword outside the protocol's allowlist — must not throw from here: + * that would drop every Desktop capability at once (Browser, Computer Use, + * Client settings, Rive and MCP), which is exactly the #4591 outage. Each tool + * is validated on its own, inside its group's real offer metadata, so an + * unrepresentable tool costs only itself and is logged rather than fatal. A + * group whose metadata is itself invalid (a caller bug: every production + * offerId is a literal) drops all of its tools the same way, so the group is + * still shed rather than advertised. + */ +function offerableToolDescriptor( + group: DesktopCapabilityGroup, + tool: MakaTool, + hostPathAccess: ClientCapabilityHostPathAccess, +): ClientCapabilityToolDescriptor | undefined { + try { + const descriptor: ClientCapabilityToolDescriptor = Object.freeze({ + serverId: group.offerId, + name: tool.name, + description: tool.description, + inputSchema: toolInputSchema(tool), + ...(tool.activityKind ? { activityKind: tool.activityKind } : {}), + ...(tool.displayName + ? { annotations: Object.freeze({ title: tool.displayName }) } + : {}), + }); + decodeClientCapabilityReplaceInput({ + registrationId: CAPABILITY_PROBE_REGISTRATION_ID, + offers: [ + { + offerId: group.offerId, + version: CAPABILITY_VERSION, + affinity: "session", + hostPathAccess, + label: group.label, + description: group.description, + tools: [descriptor], + }, + ], + }); + return descriptor; + } catch (error) { + console.warn( + `[capabilities] Skipping Desktop tool ${group.offerId}/${tool.name}: it cannot be offered over the Client Capability protocol`, + error, + ); + return undefined; + } } function toolInputSchema(tool: MakaTool): Record { - const schema = toJSONSchema(requireZodSchema(tool), { - io: "input", - target: "draft-07", - unrepresentable: "any", - cycles: "ref", - reused: "inline", - }); + const parameters = tool.parameters; + const schema: Record = + parameters instanceof z.ZodType + ? (toJSONSchema(parameters, { + io: "input", + target: "draft-07", + unrepresentable: "any", + cycles: "ref", + reused: "inline", + }) as Record) + : cloneNativeToolJsonSchema(tool); delete schema.$schema; if (schema.type !== "object") { throw new Error( @@ -464,29 +538,75 @@ function toolInputSchema(tool: MakaTool): Record { return Object.freeze(schema); } -function requireZodSchema(tool: MakaTool): z.ZodType { - if (!(tool.parameters instanceof z.ZodType)) { +function cloneNativeToolJsonSchema(tool: MakaTool): Record { + const json = aiSchemaJson(tool.parameters); + if (!json) { throw new Error( `Desktop native capability tool has an invalid schema: ${tool.name}`, ); } - return tool.parameters; + // Copy defensively before the offer deletes `$schema` and freezes the result, + // so we never mutate the source object. Today the only non-Zod source is an + // MCP tool, whose `inputSchema` is already `$schema`-stripped and deep-frozen + // upstream (`packages/mcp/src/index.ts`), so this copy is belt-and-suspenders + // rather than load-bearing — but Desktop must not depend on that invariant. + return { ...json }; +} + +/** + * Read the JSON Schema from an AI SDK `Schema` (e.g. an MCP tool built through + * `jsonSchema()`), when it is synchronously available. Desktop offers are built + * eagerly and frozen; an async (thenable) schema is not resolved here and is + * rejected downstream by `toolInputSchema`'s object-shape check. + */ +function aiSchemaJson( + parameters: unknown, +): Record | undefined { + if (!parameters || typeof parameters !== "object") return undefined; + const json = (parameters as { jsonSchema?: unknown }).jsonSchema; + if (json && typeof json === "object") { + return json as Record; + } + return undefined; +} + +/** + * Coerce/validate incoming call arguments against a tool's declared parameters, + * mirroring the runtime's `validateDeclaredToolArgs` precedence. Zod schemas + * parse (applying defaults/transforms); JSON-schema-only tools (MCP) carry no + * client-side validator, so their arguments pass through unchanged — the + * receiving Runtime Host rebuilds these tools with `buildMcpTools` and likewise + * does not validate them, so the owning MCP server is the validator, as it is + * for the TUI. + */ +async function parseToolArguments( + tool: MakaTool, + rawArgs: unknown, +): Promise { + const parameters = tool.parameters; + if (parameters instanceof z.ZodType) { + return parameters.parseAsync(rawArgs); + } + if (aiSchemaJson(parameters)) return rawArgs; + throw new Error( + `Desktop native capability tool cannot parse call arguments: ${tool.name}`, + ); } function indexBindings( - groups: readonly DesktopCapabilityGroup[], + resolved: readonly ResolvedCapability[], ): Map { const bindings = new Map(); - for (const group of groups) { - for (const tool of group.tools) { + for (const { offer, tools } of resolved) { + for (const tool of tools) { const key = bindingKey({ - offerId: group.offerId, - serverId: group.offerId, + offerId: offer.offerId, + serverId: offer.offerId, toolName: tool.name, }); if (bindings.has(key)) { throw new Error( - `Duplicate Desktop native capability tool: ${group.offerId}/${tool.name}`, + `Duplicate Desktop native capability tool: ${offer.offerId}/${tool.name}`, ); } bindings.set(key, { tool });