From cccf97261c90d4d88bec6f4a963e018e8234d16f Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Sun, 22 Mar 2026 17:27:07 +0800 Subject: [PATCH 1/2] Type GPT-5.4 hosted tool definitions --- lib/request/helpers/tool-utils.ts | 89 ++++++++++++++++-------------- lib/request/request-transformer.ts | 71 ++++++++++++++++++++++++ lib/types.ts | 70 ++++++++++++++++++++++- test/public-api-contract.test.ts | 14 ++++- test/request-transformer.test.ts | 83 ++++++++++++++++++++++++++++ test/tool-utils.test.ts | 57 +++++++++++++++++++ 6 files changed, 340 insertions(+), 44 deletions(-) diff --git a/lib/request/helpers/tool-utils.ts b/lib/request/helpers/tool-utils.ts index 7c14ec78..414f166c 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; @@ -36,36 +25,54 @@ function cloneRecord(value: Record): Record { export function cleanupToolDefinitions(tools: unknown): unknown { if (!Array.isArray(tools)) return tools; - 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: unknown): unknown { + 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..a117ae73 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, @@ -24,6 +25,10 @@ import type { 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; @@ -297,6 +302,71 @@ function sanitizePlanOnlyTools(tools: unknown, mode: CollaborationMode): unknown return filtered; } +const COMPUTER_TOOL_TYPES = new Set(["computer", "computer_use_preview"]); + +function sanitizeModelIncompatibleTools(tools: unknown, model: string | undefined): unknown { + 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: unknown, + capabilities: ReturnType, + removed: ToolCapabilityRemovalCounts, +): unknown | 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 nestedTools = record.tools + .map((nestedTool) => sanitizeModelIncompatibleToolEntry(nestedTool, capabilities, removed)) + .filter((nestedTool) => nestedTool !== null); + if (nestedTools.length === 0) { + return null; + } + if (nestedTools.length === record.tools.length) { + return tool; + } + return { + ...record, + tools: nestedTools, + }; + } + return tool; +} + /** * Configure reasoning parameters based on model variant and user config * @@ -831,6 +901,7 @@ 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); } body.instructions = shouldApplyFastSessionTuning diff --git a/lib/types.ts b/lib/types.ts index 17323401..589ff556 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[] | unknown; reasoning?: Partial; text?: { verbosity?: "low" | "medium" | "high"; diff --git a/test/public-api-contract.test.ts b/test/public-api-contract.test.ts index 89aa891a..6419b38b 100644 --- a/test/public-api-contract.test.ts +++ b/test/public-api-contract.test.ts @@ -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 () => { @@ -114,9 +114,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", @@ -141,5 +150,6 @@ describe("public api contract", () => { codexInstructions: "codex", }); expect(transformedNamed).toEqual(transformedPositional); + expect(transformedNamed.tools).toEqual(baseBody.tools); }); }); diff --git a/test/request-transformer.test.ts b/test/request-transformer.test.ts index 98dc4d2f..74eb55ef 100644 --- a/test/request-transformer.test.ts +++ b/test/request-transformer.test.ts @@ -1962,6 +1962,89 @@ describe('Request Transformer Module', () => { expect(toolNames).toEqual(['request_user_input']); }); + + 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).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, + }, + ], + }, + ]); + }); }); // NEW: Integration tests for all config scenarios diff --git a/test/tool-utils.test.ts b/test/tool-utils.test.ts index 31988238..b150c44e 100644 --- a/test/tool-utils.test.ts +++ b/test/tool-utils.test.ts @@ -1,5 +1,6 @@ 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", () => { @@ -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 }); + }); }); From 5d39a5974cfe5f1f763e6492d0589817f7fbdc8a Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Sun, 22 Mar 2026 17:30:46 +0800 Subject: [PATCH 2/2] Log semantic response diagnostics --- lib/request/response-handler.ts | 30 +++++++++++++++++++++++ test/response-handler-logging.test.ts | 35 ++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/request/response-handler.ts b/lib/request/response-handler.ts index a36f89ae..8899f270 100644 --- a/lib/request/response-handler.ts +++ b/lib/request/response-handler.ts @@ -349,6 +349,34 @@ function notifyResponseId( } } +function truncateDiagnosticText(value: unknown, maxLength = 400): string | undefined { + if (typeof value !== "string") return undefined; + const trimmed = value.trim(); + if (trimmed.length === 0) return undefined; + return trimmed.length > maxLength ? `${trimmed.slice(0, maxLength)}...` : trimmed; +} + +function logStreamDiagnostics(finalResponse: unknown): void { + if (!LOGGING_ENABLED || !isRecord(finalResponse)) { + return; + } + + const responseId = extractResponseId(finalResponse); + const phase = getStringField(finalResponse, "phase"); + const commentaryText = truncateDiagnosticText(finalResponse.commentary_text); + const finalAnswerText = truncateDiagnosticText(finalResponse.final_answer_text); + const reasoningSummaryText = truncateDiagnosticText(finalResponse.reasoning_summary_text); + if (phase || commentaryText || finalAnswerText || reasoningSummaryText) { + logRequest("stream-diagnostics", { + ...(responseId ? { responseId } : {}), + ...(phase ? { phase } : {}), + ...(commentaryText ? { commentaryText } : {}), + ...(finalAnswerText ? { finalAnswerText } : {}), + ...(reasoningSummaryText ? { reasoningSummaryText } : {}), + }); + } +} + function maybeCaptureResponseEvent( state: ParsedResponseState, data: SSEEventData, @@ -539,6 +567,8 @@ export async function convertSseToJson( }); } + logStreamDiagnostics(finalResponse); + // Return as plain JSON (not SSE) const jsonHeaders = new Headers(headers); jsonHeaders.set('content-type', 'application/json; charset=utf-8'); diff --git a/test/response-handler-logging.test.ts b/test/response-handler-logging.test.ts index a295b754..70cfab80 100644 --- a/test/response-handler-logging.test.ts +++ b/test/response-handler-logging.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; const logRequestMock = vi.fn(); @@ -14,6 +14,10 @@ vi.mock("../lib/logger.js", () => ({ })); describe("response handler logging branch", () => { + beforeEach(() => { + logRequestMock.mockClear(); + }); + it("logs full stream content when logging is enabled", async () => { const { convertSseToJson } = await import("../lib/request/response-handler.js"); const response = new Response( @@ -29,4 +33,33 @@ describe("response handler logging branch", () => { expect.objectContaining({ fullContent: expect.stringContaining("response.done") }), ); }); + + it("logs parsed phase and reasoning summary diagnostics when semantic SSE fields are present", async () => { + const { convertSseToJson } = await import("../lib/request/response-handler.js"); + const response = new Response( + [ + 'data: {"type":"response.created","response":{"id":"resp_diag_123","object":"response"}}', + 'data: {"type":"response.output_item.added","output_index":0,"item":{"id":"msg_123","type":"message","role":"assistant","phase":"commentary"}}', + 'data: {"type":"response.output_text.done","output_index":0,"content_index":0,"text":"Thinking...","phase":"commentary"}', + 'data: {"type":"response.output_item.done","output_index":0,"item":{"id":"msg_123","type":"message","role":"assistant","phase":"final_answer"}}', + 'data: {"type":"response.output_text.done","output_index":0,"content_index":1,"text":"Done.","phase":"final_answer"}', + 'data: {"type":"response.reasoning_summary_text.done","output_index":1,"summary_index":0,"text":"Need more context."}', + 'data: {"type":"response.done","response":{"id":"resp_diag_123","object":"response"}}', + "", + ].join("\n"), + ); + + const result = await convertSseToJson(response, new Headers()); + expect(result.status).toBe(200); + expect(logRequestMock).toHaveBeenCalledWith( + "stream-diagnostics", + expect.objectContaining({ + responseId: "resp_diag_123", + phase: "final_answer", + commentaryText: "Thinking...", + finalAnswerText: "Done.", + reasoningSummaryText: "Need more context.", + }), + ); + }); });