diff --git a/open-sse/translator/helpers/geminiHelper.ts b/open-sse/translator/helpers/geminiHelper.ts index 8a67b07bc5..95fea6dcea 100644 --- a/open-sse/translator/helpers/geminiHelper.ts +++ b/open-sse/translator/helpers/geminiHelper.ts @@ -327,6 +327,123 @@ function toRecord(value: unknown): JsonRecord { return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : {}; } +// Maps of schemas — the container itself is not a bare property map (#12269). +const SCHEMA_MAP_KEYS = new Set([ + "properties", + "$defs", + "definitions", + "patternProperties", + "dependentSchemas", +]); + +const SCHEMA_NODE_KEYS = new Set([ + "additionalItems", + "additionalProperties", + "contentSchema", + "contains", + "default", + "dependencies", + "dependentRequired", + "dependentSchemas", + "discriminator", + "else", + "example", + "examples", + "externalDocs", + "if", + "patternProperties", + "propertyNames", + "then", + "unevaluatedItems", + "unevaluatedProperties", + "xml", +]); + +function isSchemaNode(record: JsonRecord): boolean { + if (Object.keys(record).some((key) => key.startsWith("x-") || SCHEMA_NODE_KEYS.has(key))) { + return true; + } + if (typeof record.type === "string" || Array.isArray(record.type)) return true; + if (record.properties !== undefined || Array.isArray(record.required)) return true; + if (record.items !== undefined || record.prefixItems !== undefined) return true; + if (record.anyOf !== undefined || record.oneOf !== undefined || record.allOf !== undefined) { + return true; + } + if (record.not !== undefined || record.$ref !== undefined || record.enum !== undefined) { + return true; + } + return record.const !== undefined; +} + +function isBarePropertyMap(record: JsonRecord): boolean { + const keys = Object.keys(record); + if (keys.length === 0 || isSchemaNode(record)) return false; + return keys.every((key) => { + const value = record[key]; + return Boolean(value) && typeof value === "object" && !Array.isArray(value); + }); +} + +function promoteBooleanRequired(record: JsonRecord): void { + const properties = toRecord(record.properties); + if (Object.keys(properties).length === 0) return; + + const required = Array.isArray(record.required) + ? record.required.filter((field): field is string => typeof field === "string") + : []; + + for (const [name, schema] of Object.entries(properties)) { + if (!schema || typeof schema !== "object" || Array.isArray(schema)) continue; + const child = schema as JsonRecord; + if (child.required === true) { + if (!required.includes(name)) required.push(name); + } + if ("required" in child && !Array.isArray(child.required)) { + delete child.required; + } + } + + if (required.length > 0) { + record.required = required; + } else if (!Array.isArray(record.required)) { + delete record.required; + } +} + +// Pre-pass for Cloud Code (#12269): boolean `required` on a property and nested +// bare property maps both survive the later phases and 400 Gemini's proto. +// Mirrors CLIProxyAPI normalizeMalformedSchemaObjects. +function normalizeMalformedSchemaObjects(obj: unknown, parentKey?: string): void { + if (!obj || typeof obj !== "object") return; + + if (Array.isArray(obj)) { + for (const item of obj) { + normalizeMalformedSchemaObjects(item, parentKey); + } + return; + } + + const record = obj as JsonRecord; + if (parentKey === undefined || !SCHEMA_MAP_KEYS.has(parentKey)) { + if (isBarePropertyMap(record)) { + const props = { ...record }; + for (const key of Object.keys(record)) { + delete record[key]; + } + record.type = "object"; + record.properties = props; + } + } + + promoteBooleanRequired(record); + + for (const [key, value] of Object.entries(record)) { + if (value && typeof value === "object") { + normalizeMalformedSchemaObjects(value, key); + } + } +} + function decodeJsonPointerSegment(segment: unknown): string { return String(segment).replace(/~1/g, "/").replace(/~0/g, "~"); } @@ -627,6 +744,9 @@ export function cleanJSONSchemaForAntigravity(schema: unknown): unknown { const root = cloneSchemaValue(schema); let cleaned = inlineLocalSchemaRefs(root, root); + // Phase 0: #12269 malformed skill/tool schemas (boolean required, bare maps). + normalizeMalformedSchemaObjects(cleaned); + // Phase 1: Convert and prepare convertConstToEnum(cleaned); convertEnumValuesToStrings(cleaned); diff --git a/tests/unit/gemini-malformed-required-and-bare-map-12269.test.ts b/tests/unit/gemini-malformed-required-and-bare-map-12269.test.ts new file mode 100644 index 0000000000..fd1c51a98c --- /dev/null +++ b/tests/unit/gemini-malformed-required-and-bare-map-12269.test.ts @@ -0,0 +1,267 @@ +/** + * Regression for #12269 — skills injection 400s Antigravity Gemini because two + * schema shapes survive `cleanJSONSchemaForAntigravity` and Gemini's proto + * rejects them: + * + * 1. A property carrying boolean `required: true`. `cleanupRequired()` only + * acts when `required` is an array, so the scalar passes through; Gemini + * declares `required` as `repeated string`. + * 2. A nested bare property map (`{ opts: { limit: { type: "number" } } }`). + * `normalizeInputSchema()` only expands string shorthands at the skill root. + * + * Diego named the missing pre-pass after CLIProxyAPI + * `normalizeMalformedSchemaObjects`: promote boolean `required` onto the parent + * array, lift a bare property map into `{ type: "object", properties }`. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { cleanJSONSchemaForAntigravity } = await import( + "../../open-sse/translator/helpers/geminiHelper.ts" +); +const { buildGeminiTools } = await import( + "../../open-sse/translator/helpers/geminiToolsSanitizer.ts" +); + +function paramsOf(tools: ReturnType): Record { + const first = tools[0] as { functionDeclarations?: Array<{ parameters?: unknown }> }; + return first.functionDeclarations?.[0]?.parameters as Record; +} + +function hasBooleanRequired(value: unknown): boolean { + if (!value || typeof value !== "object") return false; + if (Array.isArray(value)) return value.some(hasBooleanRequired); + const record = value as Record; + if (typeof record.required === "boolean") return true; + return Object.values(record).some(hasBooleanRequired); +} + +test("#12269 lifts boolean required:true off a string property onto the parent array", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + query: { type: "string", required: true }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.equal(properties.query.type, "string"); + assert.equal("required" in properties.query, false, "scalar required must leave the property"); + assert.deepEqual(cleaned.required, ["query"]); + assert.equal(hasBooleanRequired(cleaned), false); +}); + +test("#12269 drops required:false instead of promoting it", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + query: { type: "string", required: false }, + hint: { type: "string" }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.equal("required" in properties.query, false); + assert.equal(cleaned.required, undefined); +}); + +test("#12269 lifts a nested bare property map into type/object + properties", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + opts: { limit: { type: "number" } }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + const opts = properties.opts; + assert.equal(opts.type, "object"); + assert.equal("limit" in opts, false, "bare key must move under properties"); + const optsProps = opts.properties as Record>; + assert.equal(optsProps.limit.type, "number"); +}); + +test("#12269 boolean required and bare map survive buildGeminiTools together", () => { + const tools = buildGeminiTools([ + { + type: "function", + function: { + name: "skill_search", + parameters: { + type: "object", + properties: { + query: { type: "string", required: true }, + opts: { limit: { type: "number" } }, + }, + }, + }, + }, + ]); + + const params = paramsOf(tools); + const properties = params.properties as Record>; + assert.equal(properties.query.type, "string"); + assert.equal("required" in properties.query, false); + assert.deepEqual(params.required, ["query"]); + const opts = properties.opts; + assert.equal(opts.type, "object"); + assert.equal((opts.properties as Record>).limit.type, "number"); + assert.equal(hasBooleanRequired(params), false); +}); + +test("#12269 does not wrap schema-keyword objects as property maps", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + options: { + additionalProperties: { type: "string" }, + }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.deepEqual(properties.options, {}); + assert.equal("properties" in properties.options, false); +}); + +test("#12269 strips unsupported validation keywords without wrapping them as property maps", () => { + // These keys are in GEMINI_UNSUPPORTED_SCHEMA_KEYS (Gemini 400s on them); + // they must be REMOVED, and removal must not go through the bare-map lift + // (which would turn `{minLength: 1}` into `{type:"object", properties:{...}}`). + for (const [keyword, value] of Object.entries({ + minLength: 1, + maxLength: 8, + multipleOf: 2, + minItems: 1, + maxItems: 4, + uniqueItems: true, + })) { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + value: { [keyword]: value }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.deepEqual(properties.value, {}, `unsupported ${keyword} must be stripped`); + assert.equal("properties" in properties.value, false); + } +}); + +test("#12269 preserves supported validation keywords without wrapping them", () => { + // `minimum`/`maximum`/`pattern` are accepted by Antigravity and must survive + // untouched; `minProperties`/`maxProperties` are not in the strip set either. + for (const [keyword, value] of Object.entries({ + minimum: 0, + maximum: 10, + pattern: "^[a-z]+$", + minProperties: 1, + })) { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + value: { [keyword]: value }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.deepEqual(properties.value, { [keyword]: value }, `supported ${keyword} must be preserved`); + assert.equal("properties" in properties.value, false); + } +}); + +test("#12269 recursively lifts more than one nested bare property map", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + opts: { settings: { limit: { type: "number" } } }, + }, + }) as Record; + + const opts = (cleaned.properties as Record>).opts; + const settings = (opts.properties as Record>).settings; + assert.equal(opts.type, "object"); + assert.equal(settings.type, "object"); + assert.equal( + (settings.properties as Record>).limit.type, + "number" + ); +}); + +test("#12269 preserves a pre-existing parent required entry without duplication", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + query: { type: "string", required: true }, + }, + required: ["query"], + }) as Record; + + assert.deepEqual(cleaned.required, ["query"]); +}); + +test("#12269 removes every non-array property-level required value", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + numeric: { type: "string", required: 1 }, + textual: { type: "string", required: "yes" }, + nil: { type: "string", required: null }, + }, + }) as Record; + + const properties = cleaned.properties as Record>; + assert.equal("required" in properties.numeric, false); + assert.equal("required" in properties.textual, false); + assert.equal("required" in properties.nil, false); + assert.equal(cleaned.required, undefined); +}); + +test("#12269 does not promote an object whose only child is an array", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + malformed: { values: [1, 2, 3] }, + }, + }) as Record; + + const malformed = (cleaned.properties as Record>).malformed; + assert.equal(malformed.type, undefined); + assert.equal(malformed.properties, undefined); +}); + +test("#12269 promotes required:true from a typed object child before cleaning it", () => { + const cleaned = cleanJSONSchemaForAntigravity({ + type: "object", + properties: { + config: { + type: "object", + required: true, + properties: { + timeout: { type: "number" }, + }, + }, + }, + }) as Record; + + assert.deepEqual(cleaned.required, ["config"]); + const properties = cleaned.properties as Record>; + assert.equal("required" in properties.config, false); +}); + +test("#12269 preserves a well-formed object schema byte-stable on required/properties", () => { + const input = { + type: "object", + properties: { + query: { type: "string" }, + limit: { type: "number" }, + }, + required: ["query"], + }; + const cleaned = cleanJSONSchemaForAntigravity(input) as Record; + const properties = cleaned.properties as Record>; + assert.equal(properties.query.type, "string"); + assert.equal(properties.limit.type, "number"); + assert.deepEqual(cleaned.required, ["query"]); +});