Files
oh-my-pi/packages/coding-agent/src/mcp/tool-bridge.ts
T
can1357 cba80c79c3 fix(schema): harden all provider schema normalizers with cycle detection, fixpoint iteration, and correctness fixes
## New: schema compatibility validation API

Add `validateSchemaCompatibility(schema, provider)` in
`packages/ai/src/utils/schema/compatibility.ts` that performs a static
audit of a JSON Schema against three provider targets:

- `openai-strict`: checks forbidden keys, required/properties symmetry,
  additionalProperties constraint, and that every node declares a type,
  combinator, or $ref
- `google`: checks unsupported keyword set and array-valued type
- `cloud-code-assist-claude`: checks forbidden keywords, array type,
  null type, nullable keyword, and combiner presence; also validates via
  AJV 2020 draft

Add `validateStrictSchemaEnforcement(original, result)` to assert the
fail-open contract: when strict enforcement succeeds the output must pass
openai-strict validation; when it fails the output must be the original
schema object (same reference).

Export both functions and their types from `./utils/schema/index.ts`.

## New: shared constants in fields.ts

Extract `COMBINATOR_KEYS` (`anyOf`, `allOf`, `oneOf`) and add
`CCA_UNSUPPORTED_SCHEMA_FIELDS` as exported constants, eliminating the
local duplicate in `strict-mode.ts` and providing a canonical field set
for Cloud Code Assist (much narrower than the Google set — CCA supports
validation keywords like `additionalProperties`, `minLength`,
`pattern`, etc.).

## Fix: cycle detection in all recursive schema traversals

All recursive walkers now carry a `WeakSet<object>` guard. Previously any
schema with a reference cycle (or a schema object that appears at two
nodes in the tree) would cause an infinite loop or a stack overflow:

- `sanitizeSchemaForStrictMode` / `enforceStrictSchema`
- `normalizeSchemaForCloudCodeAssistClaude`
- `normalizeNullablePropertiesForCloudCodeAssist`
- `stripResidualCombiners`
- `sanitizeSchemaImpl` (Google sanitizer)
- `hasResidualCloudCodeAssistIncompatibilities`

`hasResidualCloudCodeAssistIncompatibilities` previously returned `true`
for already-visited nodes, producing false positives that forced the CCA
fallback schema on valid (but multiply-referenced) schemas. It now
correctly returns `false`.

## Fix: stripResidualCombiners iterates to fixpoint

The previous single-pass approach missed chained combiner reductions
where one collapsed variant exposed another reducible combiner. The
rewriter now loops until no further reduction occurs.

## Fix: mergeObjectCombinerVariants required-field computation

The merged object schema now takes the intersection of all variants'
`required` arrays, then unions in own-level required properties that
exist in the merged schema. Previously the `required` field was silently
dropped from the flattened schema, making all properties effectively
optional.

## Fix: sanitizeSchemaForGoogle improvements

- Type inference for const-collapsed enums: type is derived from all
  variants (must unanimously agree), falling back to inference from enum
  values; mixed null/non-null infers the non-null scalar type and sets
  `nullable: true`
- Const→enum deduplication now uses deep structural equality instead of
  `Object.is`
- Recursion spreads the full options object so new fields (`unsupportedFields`,
  `seen`) are not silently dropped when descending into sub-schemas
- Array-valued `type` is filtered to strings before processing
- Removed incorrect stripping of `additionalProperties: false` (the
  field is valid and should be preserved)
- Parameterized `unsupportedFields` in `SanitizeSchemaOptions` enables
  code reuse between the Google and CCA sanitizers

## Fix: sanitizeSchemaForStrictMode / enforceStrictSchema

- `nullable: true` is now stripped during sanitization and expanded into
  `anyOf: [schema, {type: "null"}]` in the enforcer output, matching
  what OpenAI strict mode requires
- Type inference: `type: "array"` is inferred when `items` is present;
  a scalar type is inferred from uniform `enum` values
- Const→enum merge uses deep equality to avoid duplicate entries when
  both `const` and `enum` exist with the same value
- `additionalProperties` is now dropped unconditionally in sanitization
  (previously only object-valued `additionalProperties` was recursed;
  non-object values were passed through)
- `enforceStrictSchema` recurses into `$defs` and `definitions` blocks
- `enforceStrictSchema` handles tuple-style `items` arrays
- `enforceStrictSchema` skips double-wrapping: optional properties
  already expressed as `anyOf: [..., {type: "null"}]` are not wrapped again
- `tryEnforceStrictSchema` now caches results in a `WeakMap` keyed on
  the input schema object to avoid redundant work on repeated calls

## Fix: mergeCompatibleEnumSchemas deep equality

Uses `areJsonValuesEqual` instead of `Object.is` when deduplicating
enum members, so structurally equal objects are not duplicated.

## New: test coverage

- `packages/ai/test/schema-normalization.test.ts`: comprehensive unit
  tests for strict mode, Google, and Cloud Code Assist normalization
- `packages/ai/test/schema-compatibility.test.ts`: unit tests for all
  three provider targets in the new compatibility validator
- `packages/coding-agent/test/tools/provider-schema-compatibility.test.ts`:
  integration test that instantiates every builtin and hidden tool, runs
  their parameter schemas through all three provider pipelines, and
  asserts zero compatibility violations
2026-02-28 18:41:10 +01:00

334 lines
10 KiB
TypeScript

/**
* MCP to CustomTool bridge.
*
* Converts MCP tool definitions to CustomTool format for the agent.
*/
import type { AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core";
import { sanitizeSchemaForMCP } from "@oh-my-pi/pi-ai/utils/schema";
import type { TSchema } from "@sinclair/typebox";
import type { SourceMeta } from "../capability/types";
import type {
CustomTool,
CustomToolContext,
CustomToolResult,
RenderResultOptions,
} from "../extensibility/custom-tools/types";
import type { Theme } from "../modes/theme/theme";
import { ToolAbortError, throwIfAborted } from "../tools/tool-errors";
import { callTool } from "./client";
import { renderMCPCall, renderMCPResult } from "./render";
import type { MCPContent, MCPServerConnection, MCPToolDefinition } from "./types";
function withAbort<T>(promise: Promise<T>, signal?: AbortSignal): Promise<T> {
if (!signal) return promise;
if (signal.aborted) {
return Promise.reject(signal.reason instanceof Error ? signal.reason : new ToolAbortError());
}
const { promise: wrapped, resolve, reject } = Promise.withResolvers<T>();
const onAbort = () => {
reject(signal.reason instanceof Error ? signal.reason : new ToolAbortError());
};
signal.addEventListener("abort", onAbort, { once: true });
promise.then(resolve, reject).finally(() => signal.removeEventListener("abort", onAbort));
return wrapped;
}
/** Details included in MCP tool results for rendering */
export interface MCPToolDetails {
/** Server name */
serverName: string;
/** Original MCP tool name */
mcpToolName: string;
/** Whether the call resulted in an error */
isError?: boolean;
/** Raw content from MCP response */
rawContent?: MCPContent[];
/** Provider ID (e.g., "claude", "mcp-json") */
provider?: string;
/** Provider display name (e.g., "Claude Code", "MCP Config") */
providerName?: string;
}
/**
* Format MCP content for LLM consumption.
*/
function formatMCPContent(content: MCPContent[]): string {
const parts: string[] = [];
for (const item of content) {
switch (item.type) {
case "text":
parts.push(item.text);
break;
case "image":
parts.push(`[Image: ${item.mimeType}]`);
break;
case "resource":
if (item.resource.text) {
parts.push(`[Resource: ${item.resource.uri}]\n${item.resource.text}`);
} else {
parts.push(`[Resource: ${item.resource.uri}]`);
}
break;
}
}
return parts.join("\n\n");
}
/**
* Create a unique tool name for an MCP tool.
*
* Prefixes with server name to avoid conflicts. If the tool name already
* starts with the server name (e.g., server "puppeteer" with tool
* "puppeteer_screenshot"), strips the redundant prefix to produce
* "mcp_puppeteer_screenshot" instead of "mcp_puppeteer_puppeteer_screenshot".
*/
function sanitizeMCPToolNamePart(value: string, fallback: string): string {
const sanitized = value
.toLowerCase()
.replace(/[^a-z_]+/g, "_")
.replace(/_+/g, "_")
.replace(/^_+|_+$/g, "");
return sanitized.length > 0 ? sanitized : fallback;
}
export function createMCPToolName(serverName: string, toolName: string): string {
const sanitizedServerName = sanitizeMCPToolNamePart(serverName, "server");
const sanitizedToolName = sanitizeMCPToolNamePart(toolName, "tool");
// Strip redundant server name prefix from tool name if present
const prefixWithUnderscore = `${sanitizedServerName}_`;
let normalizedToolName = sanitizedToolName;
if (sanitizedToolName.startsWith(prefixWithUnderscore)) {
normalizedToolName = sanitizedToolName.slice(prefixWithUnderscore.length);
}
return `mcp_${sanitizedServerName}_${normalizedToolName}`;
}
/**
* Parse an MCP tool name back to server and tool components.
*
* Note: This returns the normalized tool name (with server prefix stripped).
* The original MCP tool name may have had the server name as a prefix.
*/
export function parseMCPToolName(name: string): { serverName: string; toolName: string } | null {
if (!name.startsWith("mcp_")) return null;
const rest = name.slice(4);
const underscoreIdx = rest.indexOf("_");
if (underscoreIdx === -1) return null;
return {
serverName: rest.slice(0, underscoreIdx),
toolName: rest.slice(underscoreIdx + 1),
};
}
/**
* CustomTool wrapping an MCP tool with an active connection.
*/
export class MCPTool implements CustomTool<TSchema, MCPToolDetails> {
readonly name: string;
readonly label: string;
readonly description: string;
readonly parameters: TSchema;
/** Original MCP tool name (before normalization) */
readonly mcpToolName: string;
/** Server name */
readonly mcpServerName: string;
/** Create MCPTool instances for all tools from an MCP server connection */
static fromTools(connection: MCPServerConnection, tools: MCPToolDefinition[]): MCPTool[] {
return tools.map(tool => new MCPTool(connection, tool));
}
constructor(
private readonly connection: MCPServerConnection,
private readonly tool: MCPToolDefinition,
) {
this.name = createMCPToolName(connection.name, tool.name);
this.label = `${connection.name}/${tool.name}`;
this.description = tool.description ?? `MCP tool from ${connection.name}`;
this.parameters = sanitizeSchemaForMCP(tool.inputSchema) as TSchema;
this.mcpToolName = tool.name;
this.mcpServerName = connection.name;
}
renderCall(args: unknown, _options: RenderResultOptions, theme: Theme) {
return renderMCPCall((args ?? {}) as Record<string, unknown>, theme, this.label);
}
renderResult(result: CustomToolResult<MCPToolDetails>, options: RenderResultOptions, theme: Theme, args?: unknown) {
return renderMCPResult(result, options, theme, (args ?? {}) as Record<string, unknown>);
}
async execute(
_toolCallId: string,
params: unknown,
_onUpdate: AgentToolUpdateCallback<MCPToolDetails> | undefined,
_ctx: CustomToolContext,
signal?: AbortSignal,
): Promise<CustomToolResult<MCPToolDetails>> {
throwIfAborted(signal);
try {
const result = await callTool(this.connection, this.tool.name, params as Record<string, unknown>, { signal });
const text = formatMCPContent(result.content);
const details: MCPToolDetails = {
serverName: this.connection.name,
mcpToolName: this.tool.name,
isError: result.isError,
rawContent: result.content,
provider: this.connection._source?.provider,
providerName: this.connection._source?.providerName,
};
if (result.isError) {
return {
content: [{ type: "text", text: `Error: ${text}` }],
details,
};
}
return {
content: [{ type: "text", text }],
details,
};
} catch (error) {
if (error instanceof ToolAbortError) {
throw error;
}
if (error instanceof Error && error.name === "AbortError") {
throw new ToolAbortError();
}
if (signal?.aborted) {
throw new ToolAbortError();
}
const message = error instanceof Error ? error.message : String(error);
return {
content: [{ type: "text", text: `MCP error: ${message}` }],
details: {
serverName: this.connection.name,
mcpToolName: this.tool.name,
isError: true,
provider: this.connection._source?.provider,
providerName: this.connection._source?.providerName,
},
};
}
}
}
/**
* CustomTool wrapping an MCP tool with deferred connection resolution.
*/
export class DeferredMCPTool implements CustomTool<TSchema, MCPToolDetails> {
readonly name: string;
readonly label: string;
readonly description: string;
readonly parameters: TSchema;
/** Original MCP tool name (before normalization) */
readonly mcpToolName: string;
/** Server name */
readonly mcpServerName: string;
readonly #fallbackProvider: string | undefined;
readonly #fallbackProviderName: string | undefined;
/** Create DeferredMCPTool instances for all tools from an MCP server */
static fromTools(
serverName: string,
tools: MCPToolDefinition[],
getConnection: () => Promise<MCPServerConnection>,
source?: SourceMeta,
): DeferredMCPTool[] {
return tools.map(tool => new DeferredMCPTool(serverName, tool, getConnection, source));
}
constructor(
private readonly serverName: string,
private readonly tool: MCPToolDefinition,
private readonly getConnection: () => Promise<MCPServerConnection>,
source?: SourceMeta,
) {
this.name = createMCPToolName(serverName, tool.name);
this.label = `${serverName}/${tool.name}`;
this.description = tool.description ?? `MCP tool from ${serverName}`;
this.parameters = sanitizeSchemaForMCP(tool.inputSchema) as TSchema;
this.mcpToolName = tool.name;
this.mcpServerName = serverName;
this.#fallbackProvider = source?.provider;
this.#fallbackProviderName = source?.providerName;
}
renderCall(args: unknown, _options: RenderResultOptions, theme: Theme) {
return renderMCPCall((args ?? {}) as Record<string, unknown>, theme, this.label);
}
renderResult(result: CustomToolResult<MCPToolDetails>, options: RenderResultOptions, theme: Theme, args?: unknown) {
return renderMCPResult(result, options, theme, (args ?? {}) as Record<string, unknown>);
}
async execute(
_toolCallId: string,
params: unknown,
_onUpdate: AgentToolUpdateCallback<MCPToolDetails> | undefined,
_ctx: CustomToolContext,
signal?: AbortSignal,
): Promise<CustomToolResult<MCPToolDetails>> {
throwIfAborted(signal);
try {
const connection = await withAbort(this.getConnection(), signal);
throwIfAborted(signal);
const result = await callTool(connection, this.tool.name, params as Record<string, unknown>, { signal });
const text = formatMCPContent(result.content);
const details: MCPToolDetails = {
serverName: this.serverName,
mcpToolName: this.tool.name,
isError: result.isError,
rawContent: result.content,
provider: connection._source?.provider ?? this.#fallbackProvider,
providerName: connection._source?.providerName ?? this.#fallbackProviderName,
};
if (result.isError) {
return {
content: [{ type: "text", text: `Error: ${text}` }],
details,
};
}
return {
content: [{ type: "text", text }],
details,
};
} catch (error) {
if (error instanceof ToolAbortError) {
throw error;
}
if (error instanceof Error && error.name === "AbortError") {
throw new ToolAbortError();
}
if (signal?.aborted) {
throw new ToolAbortError();
}
const message = error instanceof Error ? error.message : String(error);
return {
content: [{ type: "text", text: `MCP error: ${message}` }],
details: {
serverName: this.serverName,
mcpToolName: this.tool.name,
isError: true,
provider: this.#fallbackProvider,
providerName: this.#fallbackProviderName,
},
};
}
}
}