diff --git a/src/api/providers/__tests__/base-provider.spec.ts b/src/api/providers/__tests__/base-provider.spec.ts index ced452f5a5..7c9772b27d 100644 --- a/src/api/providers/__tests__/base-provider.spec.ts +++ b/src/api/providers/__tests__/base-provider.spec.ts @@ -176,6 +176,25 @@ describe("BaseProvider", () => { expect(result.additionalProperties).toBe(false) expect(result.required).toEqual([]) }) + + it("should inject required for nullable object schemas (type: ['object','null'])", () => { + const schema = { + type: "object", + properties: { + indentation: { + type: ["object", "null"], + properties: { + anchorLine: { type: ["integer", "null"] }, + maxLevels: { type: ["integer", "null"] }, + }, + additionalProperties: false, + }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + expect(result.properties.indentation.required).toEqual(["anchorLine", "maxLevels"]) + }) }) describe("convertToolsForOpenAI", () => { diff --git a/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts b/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts index c7c4a48fc3..b47f5b06b2 100644 --- a/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts +++ b/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts @@ -98,4 +98,75 @@ describe("OpenAiCodexHandler native tool calls", () => { name: "attempt_completion", }) }) + + it("normalizes nullable object schemas for strict tools (read_file indentation.anchorLine regression)", async () => { + vi.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") + vi.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + + let capturedBody: any + ;(handler as any).client = { + responses: { + create: vi.fn().mockImplementation(async (body: any) => { + capturedBody = body + return { + async *[Symbol.asyncIterator]() { + yield { + type: "response.done", + response: { + output: [{ type: "message", content: [{ type: "output_text", text: "ok" }] }], + usage: { input_tokens: 1, output_tokens: 1 }, + }, + } + }, + } + }), + }, + } + + const readFileLikeTool = { + type: "function" as const, + function: { + name: "read_file", + description: "Read files", + parameters: { + type: "object", + properties: { + files: { + type: "array", + items: { + type: "object", + properties: { + path: { type: "string" }, + indentation: { + type: ["object", "null"], + properties: { + anchorLine: { type: ["integer", "null"] }, + maxLevels: { type: ["integer", "null"] }, + }, + additionalProperties: false, + }, + }, + }, + }, + }, + }, + }, + } + + const stream = handler.createMessage("system", [{ role: "user", content: "hello" } as any], { + taskId: "t", + toolProtocol: "native", + tools: [readFileLikeTool as any], + }) + for await (const _ of stream) { + // consume + } + + const tool = capturedBody.tools?.[0] + expect(tool).toBeDefined() + expect(tool.strict).toBe(true) + const indentation = tool.parameters.properties.files.items.properties.indentation + // Critical: nullable-object schemas must still have required containing every key in properties. + expect(indentation.required).toEqual(["anchorLine", "maxLevels"]) + }) }) diff --git a/src/api/providers/__tests__/openai-native-tools.spec.ts b/src/api/providers/__tests__/openai-native-tools.spec.ts index b3c0ae0dfe..2127376ebb 100644 --- a/src/api/providers/__tests__/openai-native-tools.spec.ts +++ b/src/api/providers/__tests__/openai-native-tools.spec.ts @@ -218,6 +218,83 @@ describe("OpenAiNativeHandler MCP tool schema handling", () => { expect(tool.parameters.required).toEqual(["path", "encoding"]) // Should have all properties as required }) + it("should inject required for nullable object schemas (read_file indentation.anchorLine regression)", async () => { + let capturedRequestBody: any + + const handler = new OpenAiNativeHandler({ + openAiNativeApiKey: "test-key", + apiModelId: "gpt-4o", + } as ApiHandlerOptions) + + // Mock the responses API call + const mockClient = { + responses: { + create: vi.fn().mockImplementation((body: any) => { + capturedRequestBody = body + return { + [Symbol.asyncIterator]: async function* () { + yield { + type: "response.done", + response: { + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + } + }, + } + }), + }, + } + ;(handler as any).client = mockClient + + const tools: OpenAI.Chat.ChatCompletionTool[] = [ + { + type: "function", + function: { + name: "read_file", + description: "Read files", + parameters: { + type: "object", + properties: { + files: { + type: "array", + items: { + type: "object", + properties: { + path: { type: "string" }, + indentation: { + type: ["object", "null"], + properties: { + anchorLine: { type: ["integer", "null"] }, + maxLevels: { type: ["integer", "null"] }, + }, + additionalProperties: false, + }, + }, + }, + }, + }, + }, + }, + }, + ] + + const stream = handler.createMessage("system prompt", [], { + taskId: "test-task-id", + tools, + toolProtocol: "native" as const, + }) + + for await (const _ of stream) { + // consume + } + + const tool = capturedRequestBody.tools[0] + expect(tool.strict).toBe(true) + const indentation = tool.parameters.properties.files.items.properties.indentation + expect(indentation.required).toEqual(["anchorLine", "maxLevels"]) + }) + it("should recursively add additionalProperties: false to nested objects in MCP tools", async () => { let capturedRequestBody: any diff --git a/src/api/providers/base-provider.ts b/src/api/providers/base-provider.ts index a6adeeadbd..858dfe6b97 100644 --- a/src/api/providers/base-provider.ts +++ b/src/api/providers/base-provider.ts @@ -61,7 +61,17 @@ export abstract class BaseProvider implements ApiHandler { * This matches the behavior of ensureAllRequired in openai-native.ts */ protected convertToolSchemaForOpenAI(schema: any): any { - if (!schema || typeof schema !== "object" || schema.type !== "object") { + const isObjectLikeSchema = (candidate: any): boolean => { + if (!candidate || typeof candidate !== "object") return false + const type = candidate.type + return ( + type === "object" || + (Array.isArray(type) && type.includes("object")) || + candidate.properties !== undefined + ) + } + + if (!isObjectLikeSchema(schema)) { return schema } @@ -89,19 +99,39 @@ export abstract class BaseProvider implements ApiHandler { prop.type = nonNullTypes.length === 1 ? nonNullTypes[0] : nonNullTypes } - // Recursively process nested objects - if (prop && prop.type === "object") { + // Recursively process nested objects (including nullable objects) + if (isObjectLikeSchema(prop)) { newProps[key] = this.convertToolSchemaForOpenAI(prop) - } else if (prop && prop.type === "array" && prop.items?.type === "object") { + } else if (prop && prop.type === "array" && prop.items && isObjectLikeSchema(prop.items)) { newProps[key] = { ...prop, - items: this.convertToolSchemaForOpenAI(prop.items), + items: Array.isArray(prop.items) + ? prop.items.map((i: any) => this.convertToolSchemaForOpenAI(i)) + : this.convertToolSchemaForOpenAI(prop.items), } } } result.properties = newProps } + // Also recurse through unions if present + if (Array.isArray((result as any).anyOf)) { + ;(result as any).anyOf = (result as any).anyOf.map((s: any) => this.convertToolSchemaForOpenAI(s)) + } + if (Array.isArray((result as any).oneOf)) { + ;(result as any).oneOf = (result as any).oneOf.map((s: any) => this.convertToolSchemaForOpenAI(s)) + } + if (Array.isArray((result as any).allOf)) { + ;(result as any).allOf = (result as any).allOf.map((s: any) => this.convertToolSchemaForOpenAI(s)) + } + + if ((result as any).type === "array" && (result as any).items) { + const items = (result as any).items + ;(result as any).items = Array.isArray(items) + ? items.map((i: any) => this.convertToolSchemaForOpenAI(i)) + : this.convertToolSchemaForOpenAI(items) + } + return result } diff --git a/src/api/providers/openai-codex.ts b/src/api/providers/openai-codex.ts index 1600381f59..a3c67ac00f 100644 --- a/src/api/providers/openai-codex.ts +++ b/src/api/providers/openai-codex.ts @@ -207,8 +207,31 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion reasoningEffort: ReasoningEffortExtended | undefined, metadata?: ApiHandlerCreateMessageMetadata, ): any { + const isObjectLikeSchema = (schema: any): boolean => { + if (!schema || typeof schema !== "object") return false + const type = schema.type + return ( + type === "object" || (Array.isArray(type) && type.includes("object")) || schema.properties !== undefined + ) + } + const ensureAllRequired = (schema: any): any => { - if (!schema || typeof schema !== "object" || schema.type !== "object") { + // Only object-like schemas need "required" injection for OpenAI strict schemas. + if (!isObjectLikeSchema(schema)) { + // Still recurse into anyOf/oneOf/allOf arrays if present. + if (schema && typeof schema === "object") { + const out = { ...schema } + if (Array.isArray(out.anyOf)) out.anyOf = out.anyOf.map(ensureAllRequired) + if (Array.isArray(out.oneOf)) out.oneOf = out.oneOf.map(ensureAllRequired) + if (Array.isArray(out.allOf)) out.allOf = out.allOf.map(ensureAllRequired) + // Recurse into array item schemas too. + if (out.type === "array" && out.items) { + out.items = Array.isArray(out.items) + ? out.items.map(ensureAllRequired) + : ensureAllRequired(out.items) + } + return out + } return schema } @@ -224,28 +247,34 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion const newProps = { ...result.properties } for (const key of allKeys) { const prop = newProps[key] - if (prop.type === "object") { + if (prop && typeof prop === "object") { + // Recurse for nested object schemas, nullable object schemas (type: ["object","null"]), + // union schemas (anyOf), and arrays-of-objects. newProps[key] = ensureAllRequired(prop) - } else if (prop.type === "array" && prop.items?.type === "object") { - newProps[key] = { - ...prop, - items: ensureAllRequired(prop.items), - } } } result.properties = newProps } + if (Array.isArray(result.anyOf)) result.anyOf = result.anyOf.map(ensureAllRequired) + if (Array.isArray(result.oneOf)) result.oneOf = result.oneOf.map(ensureAllRequired) + if (Array.isArray(result.allOf)) result.allOf = result.allOf.map(ensureAllRequired) + if (result.type === "array" && result.items) { + result.items = Array.isArray(result.items) + ? result.items.map(ensureAllRequired) + : ensureAllRequired(result.items) + } + return result } const ensureAdditionalPropertiesFalse = (schema: any): any => { - if (!schema || typeof schema !== "object" || schema.type !== "object") { + if (!schema || typeof schema !== "object") { return schema } const result = { ...schema } - if (result.additionalProperties !== false) { + if (isObjectLikeSchema(result) && result.additionalProperties !== false) { result.additionalProperties = false } @@ -253,18 +282,23 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion const newProps = { ...result.properties } for (const key of Object.keys(result.properties)) { const prop = newProps[key] - if (prop && prop.type === "object") { + if (prop && typeof prop === "object") { newProps[key] = ensureAdditionalPropertiesFalse(prop) - } else if (prop && prop.type === "array" && prop.items?.type === "object") { - newProps[key] = { - ...prop, - items: ensureAdditionalPropertiesFalse(prop.items), - } } } result.properties = newProps } + if (Array.isArray(result.anyOf)) result.anyOf = result.anyOf.map(ensureAdditionalPropertiesFalse) + if (Array.isArray(result.oneOf)) result.oneOf = result.oneOf.map(ensureAdditionalPropertiesFalse) + if (Array.isArray(result.allOf)) result.allOf = result.allOf.map(ensureAdditionalPropertiesFalse) + + if (result.type === "array" && result.items) { + result.items = Array.isArray(result.items) + ? result.items.map(ensureAdditionalPropertiesFalse) + : ensureAdditionalPropertiesFalse(result.items) + } + return result } diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 61db7dd20d..9e0aad2ec8 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -216,17 +216,37 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio reasoningEffort: ReasoningEffortExtended | undefined, metadata?: ApiHandlerCreateMessageMetadata, ): any { - // Ensure all properties are in the required array for OpenAI's strict mode - // This recursively processes nested objects and array items + const isObjectLikeSchema = (schema: any): boolean => { + if (!schema || typeof schema !== "object") return false + const type = schema.type + return ( + type === "object" || (Array.isArray(type) && type.includes("object")) || schema.properties !== undefined + ) + } + + // Ensure all properties are in the required array for OpenAI's strict mode. + // This recursively processes nested objects and array items. + // IMPORTANT: must handle nullable objects (type: ["object","null"]). const ensureAllRequired = (schema: any): any => { - if (!schema || typeof schema !== "object" || schema.type !== "object") { + if (!isObjectLikeSchema(schema)) { + if (schema && typeof schema === "object") { + const out = { ...schema } + if (Array.isArray(out.anyOf)) out.anyOf = out.anyOf.map(ensureAllRequired) + if (Array.isArray(out.oneOf)) out.oneOf = out.oneOf.map(ensureAllRequired) + if (Array.isArray(out.allOf)) out.allOf = out.allOf.map(ensureAllRequired) + if (out.type === "array" && out.items) { + out.items = Array.isArray(out.items) + ? out.items.map(ensureAllRequired) + : ensureAllRequired(out.items) + } + return out + } return schema } const result = { ...schema } - // OpenAI Responses API requires additionalProperties: false on all object schemas - // Only add if not already set to false (to avoid unnecessary mutations) + // OpenAI Responses API requires additionalProperties: false on all object schemas. if (result.additionalProperties !== false) { result.additionalProperties = false } @@ -235,58 +255,61 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio const allKeys = Object.keys(result.properties) result.required = allKeys - // Recursively process nested objects const newProps = { ...result.properties } for (const key of allKeys) { const prop = newProps[key] - if (prop.type === "object") { + if (prop && typeof prop === "object") { newProps[key] = ensureAllRequired(prop) - } else if (prop.type === "array" && prop.items?.type === "object") { - newProps[key] = { - ...prop, - items: ensureAllRequired(prop.items), - } } } result.properties = newProps } + if (Array.isArray(result.anyOf)) result.anyOf = result.anyOf.map(ensureAllRequired) + if (Array.isArray(result.oneOf)) result.oneOf = result.oneOf.map(ensureAllRequired) + if (Array.isArray(result.allOf)) result.allOf = result.allOf.map(ensureAllRequired) + if (result.type === "array" && result.items) { + result.items = Array.isArray(result.items) + ? result.items.map(ensureAllRequired) + : ensureAllRequired(result.items) + } + return result } - // Adds additionalProperties: false to all object schemas recursively - // without modifying required array. Used for MCP tools with strict: false - // to comply with OpenAI Responses API requirements. + // Adds additionalProperties: false to all object-like schemas recursively without modifying required. + // Used for MCP tools with strict: false to comply with OpenAI Responses API requirements. const ensureAdditionalPropertiesFalse = (schema: any): any => { - if (!schema || typeof schema !== "object" || schema.type !== "object") { + if (!schema || typeof schema !== "object") { return schema } const result = { ...schema } - - // OpenAI Responses API requires additionalProperties: false on all object schemas - // Only add if not already set to false (to avoid unnecessary mutations) - if (result.additionalProperties !== false) { + if (isObjectLikeSchema(result) && result.additionalProperties !== false) { result.additionalProperties = false } if (result.properties) { - // Recursively process nested objects const newProps = { ...result.properties } for (const key of Object.keys(result.properties)) { const prop = newProps[key] - if (prop && prop.type === "object") { + if (prop && typeof prop === "object") { newProps[key] = ensureAdditionalPropertiesFalse(prop) - } else if (prop && prop.type === "array" && prop.items?.type === "object") { - newProps[key] = { - ...prop, - items: ensureAdditionalPropertiesFalse(prop.items), - } } } result.properties = newProps } + if (Array.isArray(result.anyOf)) result.anyOf = result.anyOf.map(ensureAdditionalPropertiesFalse) + if (Array.isArray(result.oneOf)) result.oneOf = result.oneOf.map(ensureAdditionalPropertiesFalse) + if (Array.isArray(result.allOf)) result.allOf = result.allOf.map(ensureAdditionalPropertiesFalse) + + if (result.type === "array" && result.items) { + result.items = Array.isArray(result.items) + ? result.items.map(ensureAdditionalPropertiesFalse) + : ensureAdditionalPropertiesFalse(result.items) + } + return result }