fix(sdk): carry strict through the custom-tool definition bridge
customToolToDefinition rebuilt ToolDefinition without strict, so the Task proxies' (and startup MCP tools') explicit strict:false was dropped before RegisteredToolAdapter and the OpenAI-family serializers saw it. Declare strict on ToolDefinition, copy it in the bridge, and cover the proxy -> definition -> registered adapter path in the parity test.
This commit is contained in:
@@ -543,6 +543,9 @@ export interface ToolDefinition<TParams extends TSchema = TSchema, TDetails = un
|
||||
/** Tool approval tier. Defaults to `"exec"` when omitted.
|
||||
* `"read"`: read-only operations. `"write"`: mutations. `"exec"`: code execution. */
|
||||
approval?: ToolApproval;
|
||||
/** Structured-output strict grammar opt-in/out. `false` is meaningful: OpenAI-family
|
||||
* serializers preserve an explicit `strict: false` on the wire (#4336/#4340). */
|
||||
strict?: boolean;
|
||||
/** MCP server name for discovery/search metadata when this tool fronts an MCP server. */
|
||||
mcpServerName?: string;
|
||||
/** Original MCP tool name for discovery/search metadata. */
|
||||
|
||||
@@ -907,7 +907,7 @@ function registerEvalCleanup(): void {
|
||||
postmortem.register("julia-cleanup", disposeAllJuliaKernelSessions);
|
||||
}
|
||||
|
||||
function customToolToDefinition(tool: CustomTool): ToolDefinition {
|
||||
export function customToolToDefinition(tool: CustomTool): ToolDefinition {
|
||||
const definition: ToolDefinition & { [TOOL_DEFINITION_MARKER]: true } = {
|
||||
name: tool.name,
|
||||
label: tool.label,
|
||||
@@ -917,6 +917,9 @@ function customToolToDefinition(tool: CustomTool): ToolDefinition {
|
||||
loadMode: defaultLoadModeForToolName(tool.name, tool.loadMode),
|
||||
deferrable: tool.deferrable,
|
||||
approval: typeof tool.approval === "function" ? tool.approval.bind(tool) : tool.approval,
|
||||
// Preserved through RegisteredToolAdapter so MCP-backed tools' explicit
|
||||
// `strict: false` (#4336/#4340) survives the custom-tool → definition bridge.
|
||||
strict: tool.strict,
|
||||
mcpServerName: tool.mcpServerName,
|
||||
mcpToolName: tool.mcpToolName,
|
||||
execute: (toolCallId, params, signal, onUpdate, ctx) =>
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import { describe, expect, it, vi } from "bun:test";
|
||||
import { INTENT_FIELD } from "@oh-my-pi/pi-wire";
|
||||
import type { CustomToolContext } from "../src/extensibility/custom-tools/types";
|
||||
import type { ExtensionRunner } from "../src/extensibility/extensions/runner";
|
||||
import type { RegisteredTool } from "../src/extensibility/extensions/types";
|
||||
import { wrapRegisteredTool } from "../src/extensibility/extensions/wrapper";
|
||||
import { MCPManager } from "../src/mcp/manager";
|
||||
import { DeferredMCPTool, MCPTool } from "../src/mcp/tool-bridge";
|
||||
import type { MCPServerConnection, MCPToolDefinition } from "../src/mcp/types";
|
||||
import { customToolToDefinition } from "../src/sdk";
|
||||
import { createMCPProxyTools } from "../src/task/executor";
|
||||
import { createMockConnection, createMockTransport } from "./mcp-test-utils";
|
||||
|
||||
@@ -52,6 +56,23 @@ describe("MCP tool strict declaration", () => {
|
||||
const [proxy] = createMCPProxyTools(manager);
|
||||
expect(proxy?.strict).toBe(false);
|
||||
});
|
||||
|
||||
it("survives the custom-tool → definition bridge into the registered session tool", () => {
|
||||
const manager = new MCPManager(process.cwd());
|
||||
vi.spyOn(manager, "getTools").mockReturnValue([new MCPTool(createCapturedConnection([]), STRICT_TOOL)]);
|
||||
const [proxy] = createMCPProxyTools(manager);
|
||||
if (!proxy) {
|
||||
expect.unreachable("no proxy tool created");
|
||||
return;
|
||||
}
|
||||
const definition = customToolToDefinition(proxy);
|
||||
expect(definition.strict).toBe(false);
|
||||
const adapter = wrapRegisteredTool(
|
||||
{ definition, extensionPath: "<sdk>" } as RegisteredTool,
|
||||
{ createContext: () => ({}) } as unknown as ExtensionRunner,
|
||||
);
|
||||
expect(adapter.strict).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Task MCP proxy parity", () => {
|
||||
|
||||
Reference in New Issue
Block a user