diff --git a/changelog.d/fixes/TBD-codex-redundant-oneof-enum.md b/changelog.d/fixes/TBD-codex-redundant-oneof-enum.md new file mode 100644 index 0000000000..6ec6f27377 --- /dev/null +++ b/changelog.d/fixes/TBD-codex-redundant-oneof-enum.md @@ -0,0 +1 @@ +- **fix(executors):** prevent intermittent Codex `upstream_empty_response` errors for tool schemas that combine `oneOf` const branches with a matching sibling `enum` by removing only the semantically redundant `oneOf`; bare, narrowing, non-matching, and type-discriminated `oneOf` schemas remain unchanged. (#TBD) diff --git a/open-sse/executors/codex/tools.ts b/open-sse/executors/codex/tools.ts index 52d01e9d87..0547432f2b 100644 --- a/open-sse/executors/codex/tools.ts +++ b/open-sse/executors/codex/tools.ts @@ -30,6 +30,114 @@ export function isCodexFreePlan(providerSpecificData: unknown): boolean { return typeof plan === "string" && plan.trim().toLowerCase() === "free"; } +type JsonRecord = Record; + +const REDUNDANT_ONEOF_OBJECT_MAP_FIELDS = [ + "properties", + "patternProperties", + "$defs", + "definitions", +] as const; + +const REDUNDANT_ONEOF_ARRAY_SCHEMA_FIELDS = ["prefixItems", "oneOf", "anyOf", "allOf"] as const; + +const REDUNDANT_ONEOF_SINGLE_SCHEMA_FIELDS = [ + "items", + "additionalProperties", + "not", + "if", + "then", + "else", +] as const; + +const REDUNDANT_ONEOF_ANNOTATION_KEYS = new Set(["const", "description", "title", "$comment"]); + +/** + * Remove a redundant `oneOf` when it is fully covered by a sibling `enum`. + * + * The Codex private Responses endpoint (`chatgpt.com/backend-api/codex/responses`) + * intermittently returns a 502 `upstream_empty_response` when a tool parameter + * carries the JSON-Schema pattern `oneOf: [{const, ...annotations}]` together + * with a sibling `enum` whose value set exactly matches the `const` set. In that + * case `oneOf` adds no constraint beyond `enum`, so dropping it is semantically + * safe and eliminates the trigger. + * + * Only the exact-match redundant case is stripped. Bare `oneOf[const]` without + * a sibling `enum`, narrowing const sets, non-matching enums, type-discriminated + * `oneOf`, and `anyOf`/`allOf` are all preserved. + */ +export function stripRedundantOneOfConstEnum(schema: unknown): unknown { + if (Array.isArray(schema)) { + return schema.map((entry) => stripRedundantOneOfConstEnum(entry)); + } + if (!isPlainObject(schema)) return schema; + + const result: JsonRecord = { ...schema }; + + maybeStripRedundantOneOf(result); + + for (const field of REDUNDANT_ONEOF_OBJECT_MAP_FIELDS) { + const map = result[field]; + if (isPlainObject(map)) { + result[field] = Object.fromEntries( + Object.entries(map).map(([key, value]) => [key, stripRedundantOneOfConstEnum(value)]) + ); + } + } + + for (const field of REDUNDANT_ONEOF_ARRAY_SCHEMA_FIELDS) { + if (Array.isArray(result[field])) { + result[field] = (result[field] as unknown[]).map((entry) => + stripRedundantOneOfConstEnum(entry) + ); + } + } + + for (const field of REDUNDANT_ONEOF_SINGLE_SCHEMA_FIELDS) { + if (result[field] !== undefined) { + result[field] = stripRedundantOneOfConstEnum(result[field]); + } + } + + return result; +} + +function maybeStripRedundantOneOf(node: JsonRecord): void { + const branches = node.oneOf; + if (!Array.isArray(branches) || branches.length === 0) return; + + const enumValues = Array.isArray(node.enum) ? node.enum : null; + if (!enumValues || enumValues.length === 0) return; + + // Every branch must be {const, ...annotations only}. + const constValues: unknown[] = []; + for (const branch of branches) { + if (!isPlainObject(branch)) return; + const branchKeys = Object.keys(branch); + if (!branchKeys.includes("const")) return; + if (!branchKeys.every((key) => REDUNDANT_ONEOF_ANNOTATION_KEYS.has(key))) return; + constValues.push((branch as JsonRecord).const); + } + + // Restrict to string consts and string enums (confirmed production shape). + if (!constValues.every((value) => typeof value === "string")) return; + if (!enumValues.every((value) => typeof value === "string")) return; + + // All const values must be unique. + if (new Set(constValues).size !== constValues.length) return; + + // The const set must exactly match the enum set. + const enumSet = new Set(enumValues); + if (enumSet.size !== constValues.length) return; + if (!constValues.every((value) => enumSet.has(value))) return; + + delete node.oneOf; +} + +function isPlainObject(value: unknown): value is JsonRecord { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + export function normalizeCodexTools( body: Record, options?: { dropImageGeneration?: boolean; preserveCustomTools?: boolean } @@ -138,7 +246,9 @@ export function normalizeCodexTools( // Codex/OpenAI Responses API rejects `pattern` fields using regex lookaround // (e.g. `^(?=.*@).+$`) with a 400 "regex lookaround is not supported" error. // Strip those before the schema reaches upstream (9router#1556). - const sanitizedParameters = stripUnsupportedRegexPatterns(parameters); + const sanitizedParameters = stripRedundantOneOfConstEnum( + stripUnsupportedRegexPatterns(parameters) + ); // Rewrite in-place to Responses format for (const key of Object.keys(tool)) { diff --git a/tests/unit/codex-tools-redundant-oneof-enum.test.ts b/tests/unit/codex-tools-redundant-oneof-enum.test.ts new file mode 100644 index 0000000000..d29ba38342 --- /dev/null +++ b/tests/unit/codex-tools-redundant-oneof-enum.test.ts @@ -0,0 +1,280 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + normalizeCodexTools, + stripRedundantOneOfConstEnum, +} from "../../open-sse/executors/codex/tools.ts"; + +type JsonRecord = Record; + +const PRODUCTION_ACTION_VALUES = [ + "read_file", + "write_file", + "list_files", + "search_files", + "run_command", + "create_directory", + "delete_file", + "move_file", + "copy_file", + "rename_file", + "open_terminal", + "close_terminal", + "get_status", +] as const; + +function productionParameters(): JsonRecord { + return { + type: "object", + description: "OpenChamber action parameters", + properties: { + action: { + type: "string", + description: "Action to perform", + enum: [...PRODUCTION_ACTION_VALUES], + oneOf: PRODUCTION_ACTION_VALUES.map((value) => ({ + const: value, + description: `Action ${value}`, + })), + }, + }, + required: ["action"], + }; +} + +function chatTool(parameters: JsonRecord): JsonRecord { + return { + type: "function", + function: { name: "test_tool", parameters }, + }; +} + +test("production openchamber shape is normalized", () => { + const tool = chatTool(productionParameters()); + normalizeCodexTools({ tools: [tool] }); + + const parameters = tool.parameters as JsonRecord; + const properties = parameters.properties as JsonRecord; + const action = properties.action as JsonRecord; + assert.equal(action.oneOf, undefined); + assert.deepEqual(action.enum, [...PRODUCTION_ACTION_VALUES]); + assert.equal(parameters.type, "object"); + assert.equal(parameters.description, "OpenChamber action parameters"); + assert.deepEqual(parameters.required, ["action"]); +}); + +test("bare oneOf[const] without sibling enum is preserved", () => { + const tool = chatTool({ oneOf: [{ const: "x" }, { const: "y" }] }); + normalizeCodexTools({ tools: [tool] }); + + const oneOf = (tool.parameters as JsonRecord).oneOf as unknown[]; + assert.equal(oneOf.length, 2); +}); + +test("non-matching enum is preserved", () => { + const tool = chatTool({ + enum: ["a", "b", "c"], + oneOf: [{ const: "a" }, { const: "b" }], + }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: "a" }, { const: "b" }]); +}); + +test("partially overlapping enum is preserved", () => { + const tool = chatTool({ + enum: ["a", "b", "c"], + oneOf: [{ const: "a" }, { const: "d" }], + }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: "a" }, { const: "d" }]); +}); + +test("enum with extra value is preserved", () => { + const tool = chatTool({ enum: ["a", "b"], oneOf: [{ const: "a" }] }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: "a" }]); +}); + +test("duplicate const branches are preserved", () => { + const tool = chatTool({ + enum: ["a", "b"], + oneOf: [{ const: "a" }, { const: "a" }], + }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: "a" }, { const: "a" }]); +}); + +test("branch with validation keyword is preserved", () => { + const tool = chatTool({ + enum: ["a", "b"], + oneOf: [{ const: "a", type: "string" }, { const: "b" }], + }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [ + { const: "a", type: "string" }, + { const: "b" }, + ]); +}); + +test("type-discriminated oneOf is preserved", () => { + const tool = chatTool({ oneOf: [{ type: "string" }, { type: "number" }] }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ type: "string" }, { type: "number" }]); +}); + +test("empty oneOf is preserved", () => { + const tool = chatTool({ enum: ["a"], oneOf: [] }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, []); +}); + +test("single-branch exact match is stripped", () => { + const tool = chatTool({ enum: ["x"], oneOf: [{ const: "x" }] }); + normalizeCodexTools({ tools: [tool] }); + + const parameters = tool.parameters as JsonRecord; + assert.equal(parameters.oneOf, undefined); + assert.deepEqual(parameters.enum, ["x"]); +}); + +test("non-string const is preserved", () => { + const tool = chatTool({ enum: [1, 2], oneOf: [{ const: 1 }, { const: 2 }] }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: 1 }, { const: 2 }]); +}); + +test("non-string enum is preserved", () => { + const tool = chatTool({ enum: [{ a: 1 }], oneOf: [{ const: "x" }] }); + normalizeCodexTools({ tools: [tool] }); + + assert.deepEqual((tool.parameters as JsonRecord).oneOf, [{ const: "x" }]); +}); + +test("anyOf is preserved while the walker strips its inner oneOf", () => { + const tool = chatTool({ + anyOf: [{ enum: ["a", "b"], oneOf: [{ const: "a" }, { const: "b" }] }], + }); + normalizeCodexTools({ tools: [tool] }); + + const parameters = tool.parameters as JsonRecord; + const anyOf = parameters.anyOf as JsonRecord[]; + assert.equal(anyOf.length, 1); + assert.equal(anyOf[0].oneOf, undefined); +}); + +test("allOf is preserved while the walker strips its inner oneOf", () => { + const tool = chatTool({ allOf: [{ enum: ["a"], oneOf: [{ const: "a" }] }] }); + normalizeCodexTools({ tools: [tool] }); + + const parameters = tool.parameters as JsonRecord; + const allOf = parameters.allOf as JsonRecord[]; + assert.equal(allOf.length, 1); + assert.equal(allOf[0].oneOf, undefined); +}); + +test("nested oneOf in properties is stripped", () => { + const tool = chatTool({ + properties: { + action: { + enum: ["a", "b"], + oneOf: [ + { const: "a", description: "A" }, + { const: "b", description: "B" }, + ], + }, + }, + }); + normalizeCodexTools({ tools: [tool] }); + + const properties = (tool.parameters as JsonRecord).properties as JsonRecord; + const action = properties.action as JsonRecord; + assert.equal(action.oneOf, undefined); + assert.deepEqual(action.enum, ["a", "b"]); +}); + +test("nested oneOf in items is stripped", () => { + const tool = chatTool({ items: { enum: ["a", "b"], oneOf: [{ const: "a" }, { const: "b" }] } }); + normalizeCodexTools({ tools: [tool] }); + + const items = (tool.parameters as JsonRecord).items as JsonRecord; + assert.equal(items.oneOf, undefined); +}); + +test("nested oneOf in additionalProperties is stripped", () => { + const tool = chatTool({ + additionalProperties: { + enum: ["p", "q"], + oneOf: [{ const: "p" }, { const: "q" }], + }, + }); + normalizeCodexTools({ tools: [tool] }); + + const additionalProperties = (tool.parameters as JsonRecord).additionalProperties as JsonRecord; + assert.equal(additionalProperties.oneOf, undefined); +}); + +test("nested oneOf in $defs is stripped", () => { + const tool = chatTool({ + $defs: { D: { enum: ["d1", "d2"], oneOf: [{ const: "d1" }, { const: "d2" }] } }, + }); + normalizeCodexTools({ tools: [tool] }); + + const defs = (tool.parameters as JsonRecord).$defs as JsonRecord; + const definition = defs.D as JsonRecord; + assert.equal(definition.oneOf, undefined); +}); + +test("nested oneOf in patternProperties is stripped", () => { + const tool = chatTool({ + patternProperties: { "^x$": { enum: ["a"], oneOf: [{ const: "a" }] } }, + }); + normalizeCodexTools({ tools: [tool] }); + + const patternProperties = (tool.parameters as JsonRecord).patternProperties as JsonRecord; + const pattern = patternProperties["^x$"] as JsonRecord; + assert.equal(pattern.oneOf, undefined); +}); + +test("stripping is idempotent", () => { + const first = stripRedundantOneOfConstEnum(productionParameters()); + const second = stripRedundantOneOfConstEnum(first); + + assert.deepEqual(second, first); +}); + +test("stripping is immutable", () => { + const original = productionParameters(); + const before = structuredClone(original); + const result = stripRedundantOneOfConstEnum(original) as JsonRecord; + + const properties = original.properties as JsonRecord; + const action = properties.action as JsonRecord; + assert.deepEqual(original, before); + assert.ok(Array.isArray(action.oneOf)); + assert.notStrictEqual(result, original); +}); + +test("Chat wrapper is flattened to the flat Responses form", () => { + const tool = chatTool({ + properties: { + action: { enum: ["a", "b"], oneOf: [{ const: "a" }, { const: "b" }] }, + }, + }); + normalizeCodexTools({ tools: [tool] }); + + const parameters = tool.parameters as JsonRecord; + const properties = parameters.properties as JsonRecord; + const action = properties.action as JsonRecord; + assert.equal(action.oneOf, undefined); + assert.equal(tool.function, undefined); + assert.equal(tool.name, "test_tool"); +});