diff --git a/lib/request/helpers/tool-utils.ts b/lib/request/helpers/tool-utils.ts index 7c14ec78..fc7d4a94 100644 --- a/lib/request/helpers/tool-utils.ts +++ b/lib/request/helpers/tool-utils.ts @@ -1,20 +1,9 @@ import { isRecord } from "../../utils.js"; - -export interface ToolFunction { - name: string; - description?: string; - parameters?: { - type: "object"; - properties?: Record; - required?: string[]; - [key: string]: unknown; - }; -} - -export interface Tool { - type: "function"; - function: ToolFunction; -} +import type { + FunctionToolDefinition, + RequestToolDefinition, + ToolParametersSchema, +} from "../../types.js"; function cloneRecord(value: Record): Record { return JSON.parse(JSON.stringify(value)) as Record; @@ -33,39 +22,59 @@ function cloneRecord(value: Record): Record { * @param tools - Array of tool definitions * @returns Cleaned array of tool definitions */ -export function cleanupToolDefinitions(tools: unknown): unknown { - if (!Array.isArray(tools)) return tools; +export function cleanupToolDefinitions( + tools: RequestToolDefinition[] | undefined, +): RequestToolDefinition[] | undefined { + if (!Array.isArray(tools)) return undefined; - return tools.map((tool) => { - if (!isRecord(tool) || tool.type !== "function") { - return tool; - } - const functionDef = tool.function; - if (!isRecord(functionDef)) { - return tool; - } - const parameters = functionDef.parameters; - if (!isRecord(parameters)) { - return tool; - } + return tools.map((tool) => cleanupToolDefinition(tool)); +} - // Clone only the schema tree we mutate to avoid heavy deep cloning of entire tools. - let cleanedParameters: Record; - try { - cleanedParameters = cloneRecord(parameters); - } catch { - return tool; - } - cleanupSchema(cleanedParameters); +function cleanupToolDefinition(tool: RequestToolDefinition): RequestToolDefinition { + if (!isRecord(tool)) { + return tool; + } + if (tool.type === "function") { + return cleanupFunctionTool(tool as FunctionToolDefinition); + } + + if (tool.type === "namespace" && Array.isArray(tool.tools)) { return { ...tool, - function: { - ...functionDef, - parameters: cleanedParameters, - }, + tools: tool.tools.map((nestedTool) => cleanupToolDefinition(nestedTool)) as RequestToolDefinition[], }; - }); + } + + return tool; +} + +function cleanupFunctionTool(tool: FunctionToolDefinition): FunctionToolDefinition { + const functionDef = tool.function; + if (!isRecord(functionDef)) { + return tool; + } + const parameters = functionDef.parameters; + if (!isRecord(parameters)) { + return tool; + } + + // Clone only the schema tree we mutate to avoid heavy deep cloning of entire tools. + let cleanedParameters: Record; + try { + cleanedParameters = cloneRecord(parameters); + } catch { + return tool; + } + cleanupSchema(cleanedParameters); + + return { + ...tool, + function: { + ...functionDef, + parameters: cleanedParameters as ToolParametersSchema, + }, + }; } /** diff --git a/lib/request/request-transformer.ts b/lib/request/request-transformer.ts index 5a407f55..c344ebca 100644 --- a/lib/request/request-transformer.ts +++ b/lib/request/request-transformer.ts @@ -3,6 +3,7 @@ import { TOOL_REMAP_MESSAGE } from "../prompts/codex.js"; import { CODEX_HOST_BRIDGE } from "../prompts/codex-host-bridge.js"; import { getHostCodexPrompt } from "../prompts/host-codex-prompt.js"; import { + getModelCapabilities, getModelProfile, resolveNormalizedModel, type ModelReasoningEffort, @@ -18,12 +19,17 @@ import type { InputItem, ReasoningConfig, RequestBody, + RequestToolDefinition, UserConfig, } from "../types.js"; type CollaborationMode = "plan" | "default" | "unknown"; type FastSessionStrategy = "hybrid" | "always"; type SupportedReasoningSummary = "auto" | "concise" | "detailed"; +type ToolCapabilityRemovalCounts = { + toolSearch: number; + computerUse: number; +}; export interface TransformRequestBodyParams { body: RequestBody; @@ -274,20 +280,16 @@ function detectCollaborationMode(body: RequestBody): CollaborationMode { return "unknown"; } -function sanitizePlanOnlyTools(tools: unknown, mode: CollaborationMode): unknown { +function sanitizePlanOnlyTools( + tools: RequestToolDefinition[] | undefined, + mode: CollaborationMode, +): RequestToolDefinition[] | undefined { if (!Array.isArray(tools) || mode === "plan") return tools; let removed = 0; - const filtered = tools.filter((entry) => { - if (!entry || typeof entry !== "object") return true; - const functionDef = (entry as { function?: unknown }).function; - if (!functionDef || typeof functionDef !== "object") return true; - const name = (functionDef as { name?: unknown }).name; - if (typeof name !== "string") return true; - if (!PLAN_MODE_ONLY_TOOLS.has(name)) return true; - removed++; - return false; - }); + const filtered = tools + .map((entry) => sanitizePlanOnlyToolEntry(entry, mode, () => removed++)) + .filter((entry) => entry !== null); if (removed > 0) { logWarn( @@ -297,6 +299,120 @@ function sanitizePlanOnlyTools(tools: unknown, mode: CollaborationMode): unknown return filtered; } +function sanitizePlanOnlyToolEntry( + entry: RequestToolDefinition, + mode: CollaborationMode, + onRemoved: () => void, +): RequestToolDefinition | null { + if (!entry || typeof entry !== "object" || mode === "plan") { + return entry; + } + + const record = entry as Record; + if (record.type === "namespace" && Array.isArray(record.tools)) { + const namespaceTools = record.tools as RequestToolDefinition[]; + const nestedTools = namespaceTools + .map((nestedTool) => sanitizePlanOnlyToolEntry(nestedTool, mode, onRemoved)) + .filter((nestedTool) => nestedTool !== null); + const changed = + nestedTools.length !== namespaceTools.length || + nestedTools.some((nestedTool, index) => nestedTool !== namespaceTools[index]); + if (nestedTools.length === 0) { + return null; + } + if (!changed) { + return entry; + } + return { + ...record, + tools: nestedTools, + }; + } + + const functionDef = (entry as { function?: unknown }).function; + if (!functionDef || typeof functionDef !== "object") { + return entry; + } + const name = (functionDef as { name?: unknown }).name; + if (typeof name !== "string" || !PLAN_MODE_ONLY_TOOLS.has(name)) { + return entry; + } + onRemoved(); + return null; +} + +const COMPUTER_TOOL_TYPES = new Set(["computer", "computer_use_preview"]); + +function sanitizeModelIncompatibleTools( + tools: RequestToolDefinition[] | undefined, + model: string | undefined, +): RequestToolDefinition[] | undefined { + if (!Array.isArray(tools)) return tools; + + const capabilities = getModelCapabilities(model); + const removed: ToolCapabilityRemovalCounts = { + toolSearch: 0, + computerUse: 0, + }; + const filtered = tools + .map((tool) => sanitizeModelIncompatibleToolEntry(tool, capabilities, removed)) + .filter((tool) => tool !== null); + + if (removed.toolSearch > 0) { + logWarn( + `Removed ${removed.toolSearch} tool_search definition(s) because ${model ?? "the selected model"} does not support tool search`, + ); + } + if (removed.computerUse > 0) { + logWarn( + `Removed ${removed.computerUse} computer tool definition(s) because ${model ?? "the selected model"} does not support computer use`, + ); + } + + return filtered; +} + +function sanitizeModelIncompatibleToolEntry( + tool: RequestToolDefinition, + capabilities: ReturnType, + removed: ToolCapabilityRemovalCounts, +): RequestToolDefinition | null { + if (!tool || typeof tool !== "object") { + return tool; + } + + const record = tool as Record; + const type = typeof record.type === "string" ? record.type : ""; + if (type === "tool_search" && !capabilities.toolSearch) { + removed.toolSearch += 1; + return null; + } + if (COMPUTER_TOOL_TYPES.has(type) && !capabilities.computerUse) { + removed.computerUse += 1; + return null; + } + if (type === "namespace" && Array.isArray(record.tools)) { + const namespaceTools = record.tools as RequestToolDefinition[]; + const nestedTools = namespaceTools + .map((nestedTool) => sanitizeModelIncompatibleToolEntry(nestedTool, capabilities, removed)) + .filter((nestedTool) => nestedTool !== null); + const changed = + nestedTools.length !== namespaceTools.length || + nestedTools.some((nestedTool, index) => nestedTool !== namespaceTools[index]); + if (nestedTools.length === 0) { + return null; + } + if (!changed) { + return tool; + } + return { + ...record, + tools: nestedTools, + }; + } + return tool; +} + /** * Configure reasoning parameters based on model variant and user config * @@ -831,6 +947,10 @@ export async function transformRequestBody( if (body.tools) { body.tools = cleanupToolDefinitions(body.tools); body.tools = sanitizePlanOnlyTools(body.tools, collaborationMode); + body.tools = sanitizeModelIncompatibleTools(body.tools, body.model); + if (Array.isArray(body.tools) && body.tools.length === 0) { + body.tools = undefined; + } } body.instructions = shouldApplyFastSessionTuning diff --git a/lib/types.ts b/lib/types.ts index 17323401..25dab739 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -40,6 +40,74 @@ export interface ReasoningConfig { summary: "auto" | "concise" | "detailed"; } +export interface ToolParametersSchema { + type: "object"; + properties?: Record; + required?: string[]; + [key: string]: unknown; +} + +export interface ToolFunction { + name: string; + description?: string; + parameters?: ToolParametersSchema; + [key: string]: unknown; +} + +export interface FunctionToolDefinition { + type: "function"; + function: ToolFunction; + defer_loading?: boolean; + [key: string]: unknown; +} + +export interface ToolSearchToolDefinition { + type: "tool_search"; + max_num_results?: number; + search_context_size?: "low" | "medium" | "high"; + filters?: Record; + [key: string]: unknown; +} + +export interface RemoteMcpToolDefinition { + type: "mcp"; + server_label?: string; + server_url?: string; + connector_id?: string; + headers?: Record; + allowed_tools?: string[]; + require_approval?: "never" | "always" | "auto" | Record; + defer_loading?: boolean; + [key: string]: unknown; +} + +export interface ComputerUseToolDefinition { + type: "computer" | "computer_use_preview"; + display_width?: number; + display_height?: number; + environment?: string; + [key: string]: unknown; +} + +export interface ToolNamespaceDefinition { + type: "namespace"; + name?: string; + description?: string; + tools?: RequestToolDefinition[]; + [key: string]: unknown; +} + +export type RequestToolDefinition = + | FunctionToolDefinition + | ToolSearchToolDefinition + | RemoteMcpToolDefinition + | ComputerUseToolDefinition + | ToolNamespaceDefinition + | { + type?: string; + [key: string]: unknown; + }; + export type TextFormatConfig = | { type: "text"; @@ -125,7 +193,7 @@ export interface RequestBody { stream?: boolean; instructions?: string; input?: InputItem[]; - tools?: unknown; + tools?: RequestToolDefinition[]; reasoning?: Partial; text?: { verbosity?: "low" | "medium" | "high"; diff --git a/test/public-api-contract.test.ts b/test/public-api-contract.test.ts index a9d9a484..424fff03 100644 --- a/test/public-api-contract.test.ts +++ b/test/public-api-contract.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, expectTypeOf, it, vi } from "vitest"; import { HealthScoreTracker, TokenBucketTracker, @@ -12,7 +12,7 @@ import { getRateLimitBackoffWithReason, } from "../lib/request/rate-limit-backoff.js"; import { transformRequestBody } from "../lib/request/request-transformer.js"; -import type { RequestBody } from "../lib/types.js"; +import type { RequestBody, RequestToolDefinition } from "../lib/types.js"; describe("public api contract", () => { it("keeps root plugin exports aligned", async () => { @@ -45,6 +45,8 @@ describe("public api contract", () => { }); it("keeps positional and options-object overload behavior aligned", async () => { + expectTypeOf().toEqualTypeOf(); + const healthTracker = new HealthScoreTracker(); const tokenTracker = new TokenBucketTracker(); const accounts = [{ index: 0, isAvailable: true, lastUsed: 1_709_280_000_000 }]; @@ -114,9 +116,18 @@ describe("public api contract", () => { expect(rateNamed).toEqual(ratePositional); const baseBody: RequestBody = { - model: "gpt-5-codex", + model: "gpt-5.4", input: [{ type: "message", role: "user", content: "hi" }], prompt_cache_retention: "24h", + tools: [ + { type: "tool_search", max_num_results: 2 }, + { + type: "mcp", + server_label: "docs", + server_url: "https://mcp.example.com", + defer_loading: true, + }, + ] satisfies RequestToolDefinition[], text: { format: { type: "json_schema", @@ -145,5 +156,6 @@ describe("public api contract", () => { expect(transformedNamed.prompt_cache_retention).toBe(baseBody.prompt_cache_retention); expect(transformedPositional.text?.format).toEqual(baseBody.text?.format); expect(transformedNamed.text?.format).toEqual(baseBody.text?.format); + expect(transformedNamed.tools).toEqual(baseBody.tools); }); }); diff --git a/test/request-transformer.test.ts b/test/request-transformer.test.ts index af8d5c84..bb360838 100644 --- a/test/request-transformer.test.ts +++ b/test/request-transformer.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import { normalizeModel, getModelConfig, @@ -10,11 +10,16 @@ import { addCodexBridgeMessage, transformRequestBody, } from '../lib/request/request-transformer.js'; +import * as loggerModule from '../lib/logger.js'; import { TOOL_REMAP_MESSAGE } from '../lib/prompts/codex.js'; import { CODEX_HOST_BRIDGE } from '../lib/prompts/codex-host-bridge.js'; import type { RequestBody, UserConfig, InputItem } from '../lib/types.js'; describe('Request Transformer Module', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + describe('normalizeModel', () => { it('keeps codex families canonical', async () => { expect(normalizeModel('gpt-5-codex')).toBe('gpt-5-codex'); @@ -1999,6 +2004,203 @@ describe('Request Transformer Module', () => { expect(toolNames).toEqual(['request_user_input']); }); + + it('removes nested request_user_input tools outside plan collaboration mode', async () => { + const body: RequestBody = { + model: 'gpt-5', + input: [], + tools: [ + { + type: 'namespace', + name: 'planner', + tools: [ + { type: 'function', function: { name: 'request_user_input', parameters: { type: 'object', properties: {} } } }, + { type: 'function', function: { name: 'exec_command', parameters: { type: 'object', properties: {} } } }, + ], + }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + expect(result.tools).toEqual([ + { + type: 'namespace', + name: 'planner', + tools: [ + expect.objectContaining({ + type: 'function', + function: expect.objectContaining({ + name: 'exec_command', + }), + }), + ], + }, + ]); + }); + + it('counts only removed plan-only tools when a namespace becomes empty', async () => { + const warnSpy = vi.spyOn(loggerModule, 'logWarn').mockImplementation(() => {}); + const body: RequestBody = { + model: 'gpt-5', + input: [ + { + type: 'message', + role: 'developer', + content: [{ type: 'input_text', text: '# Collaboration Mode: Default' }], + }, + ], + tools: [ + { + type: 'namespace', + name: 'planner', + tools: [ + { type: 'function', function: { name: 'request_user_input', parameters: { type: 'object', properties: {} } } }, + ], + }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + + expect(result.tools).toBeUndefined(); + expect(warnSpy).toHaveBeenCalledWith( + 'Removed 1 plan-mode-only tool definition(s) because collaboration mode is default', + ); + }); + + it('removes tool_search tools when the selected model lacks search capability', async () => { + const body: RequestBody = { + model: 'gpt-5-nano', + input: [], + tools: [ + { type: 'tool_search', max_num_results: 3 }, + { + type: 'mcp', + server_label: 'docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + expect(result.tools).toEqual([ + { + type: 'mcp', + server_label: 'docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ]); + }); + + it('removes computer tools when the selected model lacks computer-use capability', async () => { + const body: RequestBody = { + model: 'gpt-5-nano', + input: [], + tools: [ + { + type: 'computer_use_preview', + display_width: 1024, + display_height: 768, + environment: 'browser', + }, + { type: 'tool_search', max_num_results: 1 }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + expect(result.tools).toBeUndefined(); + expect(result.input).toEqual([]); + }); + + it('filters unsupported namespace tool entries while keeping supported remote MCP tools', async () => { + const body: RequestBody = { + model: 'gpt-5-nano', + input: [], + tools: [ + { + type: 'namespace', + name: 'search_suite', + tools: [ + { type: 'tool_search', max_num_results: 2 }, + { + type: 'mcp', + server_label: 'remote-docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ], + }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + expect(result.tools).toEqual([ + { + type: 'namespace', + name: 'search_suite', + tools: [ + { + type: 'mcp', + server_label: 'remote-docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ], + }, + ]); + }); + + it('filters unsupported tools from nested namespaces without dropping supported descendants', async () => { + const body: RequestBody = { + model: 'gpt-5-nano', + input: [], + tools: [ + { + type: 'namespace', + name: 'outer_suite', + tools: [ + { + type: 'namespace', + name: 'inner_suite', + tools: [ + { type: 'tool_search', max_num_results: 2 }, + { + type: 'mcp', + server_label: 'remote-docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ], + }, + ], + }, + ] as any, + }; + + const result = await transformRequestBody(body, codexInstructions); + expect(result.tools).toEqual([ + { + type: 'namespace', + name: 'outer_suite', + tools: [ + { + type: 'namespace', + name: 'inner_suite', + tools: [ + { + type: 'mcp', + server_label: 'remote-docs', + server_url: 'https://mcp.example.com', + defer_loading: true, + }, + ], + }, + ], + }, + ]); + }); }); // NEW: Integration tests for all config scenarios diff --git a/test/tool-utils.test.ts b/test/tool-utils.test.ts index 31988238..c7f864fc 100644 --- a/test/tool-utils.test.ts +++ b/test/tool-utils.test.ts @@ -1,11 +1,12 @@ import { describe, it, expect } from "vitest"; import { cleanupToolDefinitions } from "../lib/request/helpers/tool-utils.js"; +import type { RequestToolDefinition } from "../lib/types.js"; describe("cleanupToolDefinitions", () => { - it("returns non-array input unchanged", () => { - expect(cleanupToolDefinitions(null)).toBeNull(); - expect(cleanupToolDefinitions("string")).toBe("string"); - expect(cleanupToolDefinitions({})).toEqual({}); + it("returns undefined for non-array input", () => { + expect(cleanupToolDefinitions(null)).toBeUndefined(); + expect(cleanupToolDefinitions("string" as unknown as RequestToolDefinition[])).toBeUndefined(); + expect(cleanupToolDefinitions({} as unknown as RequestToolDefinition[])).toBeUndefined(); }); it("returns non-function tools unchanged", () => { @@ -13,6 +14,27 @@ describe("cleanupToolDefinitions", () => { expect(cleanupToolDefinitions(tools)).toEqual(tools); }); + it("preserves typed GPT-5.4 hosted tools unchanged", () => { + const tools: RequestToolDefinition[] = [ + { type: "tool_search", max_num_results: 3, search_context_size: "medium" }, + { + type: "mcp", + server_label: "docs", + server_url: "https://mcp.example.com", + defer_loading: true, + require_approval: "never", + }, + { + type: "computer_use_preview", + display_width: 1024, + display_height: 768, + environment: "browser", + }, + ]; + + expect(cleanupToolDefinitions(tools)).toEqual(tools); + }); + it("treats array parameters as non-records and leaves tool unchanged", () => { const tools = [{ type: "function", @@ -619,4 +641,39 @@ describe("cleanupToolDefinitions", () => { const props = result[0].function.parameters.properties as Record; expect(props.valid).toEqual({ type: "string" }); }); + + it("recursively cleans nested function tools inside namespace bundles", () => { + const tools: RequestToolDefinition[] = [ + { + type: "namespace", + name: "search_bundle", + tools: [ + { + type: "function", + function: { + name: "lookup", + parameters: { + type: "object", + properties: {}, + additionalProperties: false, + }, + }, + }, + { type: "tool_search", max_num_results: 2 }, + ], + }, + ]; + + const result = cleanupToolDefinitions(tools) as typeof tools; + const namespaceTools = result[0].tools ?? []; + const nestedFunction = namespaceTools[0] as Extract; + expect(nestedFunction.function.parameters?.additionalProperties).toBeUndefined(); + expect(nestedFunction.function.parameters?.properties).toEqual({ + _placeholder: { + type: "boolean", + description: "This property is a placeholder and should be ignored.", + }, + }); + expect(namespaceTools[1]).toEqual({ type: "tool_search", max_num_results: 2 }); + }); });