diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index fd83d6aad..39f09a619 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Preserved explicit `tool.strict === false` on OpenAI-family function tool payloads (openai-responses, openai-codex-responses, openai-completions) so backends that distinguish `strict: false` from an omitted flag stop over-filling optional arguments ([#4336](https://github.com/can1357/oh-my-pi/issues/4336)). + ## [16.3.1] - 2026-07-02 ### Changed diff --git a/packages/ai/src/providers/openai-codex-responses.ts b/packages/ai/src/providers/openai-codex-responses.ts index 446727a9f..6ba937d80 100644 --- a/packages/ai/src/providers/openai-codex-responses.ts +++ b/packages/ai/src/providers/openai-codex-responses.ts @@ -3311,7 +3311,10 @@ export function convertOpenAICodexResponsesTools( name: tool.name, description: tool.description || "", parameters, - ...(effectiveStrict && { strict: true }), + // See openai-responses.ts::convertTools — explicit `strict: false` is + // preserved on the wire because some backends distinguish it from + // omitted (#4336). `strict: true` still requires enforcement success. + ...(effectiveStrict ? { strict: true } : tool.strict === false ? { strict: false } : {}), }; }); } diff --git a/packages/ai/src/providers/openai-completions.ts b/packages/ai/src/providers/openai-completions.ts index a4adf94c7..02a80571c 100644 --- a/packages/ai/src/providers/openai-completions.ts +++ b/packages/ai/src/providers/openai-completions.ts @@ -2112,6 +2112,17 @@ function convertTools( return { tools: adaptedTools.map(({ tool, baseParameters, parameters, strict }) => { const includeStrict = toolStrictMode === "all_strict" || (toolStrictMode === "mixed" && strict); + // `strict: false` is semantically distinct from omitted `strict` on some + // backends: with it absent, optional properties may be over-filled with + // placeholder values (#4336). Preserve the author's explicit `false`, + // but only in "mixed" mode against a provider that understands the + // field — the `all_strict → none` collapse and `supportsStrictMode: + // false` paths deliberately keep the wire flag uniformly absent. + const includeExplicitFalse = + !includeStrict && + tool.strict === false && + toolStrictMode === "mixed" && + compat.supportsStrictMode !== false; const wireParameters = includeStrict ? parameters : baseParameters; return { type: "function", @@ -2125,7 +2136,7 @@ function convertTools( ? (normalizeSchemaForMoonshot(wireParameters) as Record) : wireParameters, // Only include strict if provider supports it. Some reject unknown fields. - ...(includeStrict && { strict: true }), + ...(includeStrict ? { strict: true } : includeExplicitFalse ? { strict: false } : {}), }, }; }), diff --git a/packages/ai/src/providers/openai-responses.ts b/packages/ai/src/providers/openai-responses.ts index d57977cb4..5a28ef917 100644 --- a/packages/ai/src/providers/openai-responses.ts +++ b/packages/ai/src/providers/openai-responses.ts @@ -987,7 +987,11 @@ export function convertTools( name: tool.name, description: tool.description || "", parameters, - ...(effectiveStrict && { strict: true }), + // `strict: false` and an omitted `strict` are NOT equivalent for every + // OpenAI-compat backend — some over-fill optional args when the flag is + // absent (#4336). Preserve the author's explicit `false`; only emit + // `true` when strict enforcement actually succeeded. + ...(effectiveStrict ? { strict: true } : tool.strict === false ? { strict: false } : {}), } as OpenAITool); } return out; diff --git a/packages/ai/src/utils/schema/CONSTRAINTS.md b/packages/ai/src/utils/schema/CONSTRAINTS.md index e263ad712..99dd66e68 100644 --- a/packages/ai/src/utils/schema/CONSTRAINTS.md +++ b/packages/ai/src/utils/schema/CONSTRAINTS.md @@ -53,6 +53,7 @@ When strict mode is requested (`strict=true` at call site), the schema MUST sati 6. **Provider payload strict flag must match effective strictness** - Callers MUST send `strict: true` only if enforcement succeeded (`effectiveStrict === true`). + - Callers MUST preserve an author's explicit `tool.strict === false` on the wire so that `strict: false` and omitted `strict` remain distinguishable — some OpenAI-compat backends over-fill optional fields when the flag is absent but respect it when set to `false` (#4336). Exception: the `openai-completions` path emits explicit `false` only in `toolStrictMode === "mixed"` with `compat.supportsStrictMode !== false`, because the `all_strict → none` collapse and providers that reject the `strict` key rely on uniform absence. --- diff --git a/packages/ai/test/apply-patch-freeform.test.ts b/packages/ai/test/apply-patch-freeform.test.ts index ccae7846c..a1a4b6c35 100644 --- a/packages/ai/test/apply-patch-freeform.test.ts +++ b/packages/ai/test/apply-patch-freeform.test.ts @@ -176,7 +176,9 @@ describe("convertTools: freeform emission", () => { }>; const items = out.parameters.properties.operations.items; - expect(out.strict).toBeUndefined(); + // Author-set `strict: false` MUST survive to the wire (#4336) — providers + // distinguish it from an omitted flag when generating optional-arg values. + expect(out.strict).toBe(false); expect(items.oneOf).toBeUndefined(); expect(items.anyOf).toEqual(unionBranches); }); diff --git a/packages/ai/test/openai-codex.test.ts b/packages/ai/test/openai-codex.test.ts index 6c241722b..e28633fd1 100644 --- a/packages/ai/test/openai-codex.test.ts +++ b/packages/ai/test/openai-codex.test.ts @@ -128,6 +128,53 @@ describe("openai-codex tool schemas", () => { }, }); }); + + it("preserves explicit strict:false on the wire (#4336)", () => { + const tools: Tool[] = [ + { + name: "search", + description: "Search", + strict: false, + parameters: { + type: "object", + additionalProperties: false, + properties: { + name: { type: "string" }, + target: { type: "string" }, + }, + required: ["name"], + }, + }, + ]; + + const converted = convertOpenAICodexResponsesTools(tools, createCodexModel("gpt-5.5")); + + // Author-set `strict: false` MUST survive to the wire so backends that + // distinguish it from an omitted flag stop over-filling optional args. + expect(converted[0]).toMatchObject({ type: "function", name: "search", strict: false }); + }); + + it("omits strict when the tool leaves it unset (#4336)", () => { + const tools: Tool[] = [ + { + name: "search", + description: "Search", + parameters: { + type: "object", + additionalProperties: false, + properties: { name: { type: "string" } }, + required: ["name"], + }, + }, + ]; + + const converted = convertOpenAICodexResponsesTools(tools, createCodexModel("gpt-5.5")); + const payload = converted[0] as { strict?: boolean }; + + // Codex responses only enforces strict when the tool opts in; leaving + // `strict` unset MUST NOT synthesize the field either way. + expect(payload.strict).toBeUndefined(); + }); }); describe("openai-codex request transformer", () => { diff --git a/packages/ai/test/openai-tool-strict-mode.test.ts b/packages/ai/test/openai-tool-strict-mode.test.ts index 7d15d885f..bb46fa836 100644 --- a/packages/ai/test/openai-tool-strict-mode.test.ts +++ b/packages/ai/test/openai-tool-strict-mode.test.ts @@ -139,7 +139,7 @@ describe("OpenAI tool strict mode", () => { expect(payload.tools?.[0]?.function?.strict).toBeUndefined(); }); - it("keeps loose yield schemas non-strict for openai-completions", async () => { + it("preserves explicit strict:false on the wire for openai-completions", async () => { const model: Model<"openai-completions"> = { ...(getBundledModel("openai", "gpt-4o-mini") as Model<"openai-completions">), api: "openai-completions", @@ -152,10 +152,30 @@ describe("OpenAI tool strict mode", () => { }; const fn = payload.tools?.[0]?.function; - expect(fn?.strict).toBeUndefined(); + // #4336: `strict: false` from the tool author is semantically distinct from + // omitted `strict` on some backends and MUST survive to the wire. + expect(fn?.strict).toBe(false); expect(getYieldDataSchema(fn?.parameters).additionalProperties).toBe(true); }); + it("omits explicit strict:false for openai-completions when compat disables the strict field", async () => { + const model: Model<"openai-completions"> = buildModel({ + ...(getBundledModel("openai", "gpt-4o-mini") as Model<"openai-completions">), + api: "openai-completions", + compat: { supportsStrictMode: false } satisfies OpenAICompat, + } as ModelSpec<"openai-completions">); + + const payload = (await captureCompletionsPayload(model, { + ...testContext, + tools: [looseYieldTool], + })) as { + tools?: Array<{ function?: { strict?: boolean } }>; + }; + // `supportsStrictMode: false` providers reject the `strict` key entirely, + // so the explicit `false` MUST still be suppressed. + expect(payload.tools?.[0]?.function?.strict).toBeUndefined(); + }); + it("sends strict=true for openai-completions tool schemas on GitHub Copilot", async () => { const model = getBundledModel("github-copilot", "gpt-4o") as Model<"openai-completions">; @@ -210,6 +230,26 @@ describe("OpenAI tool strict mode", () => { expect(payload.tools?.every(tool => tool.function?.strict === undefined)).toBe(true); }); + it("keeps strict uniformly absent when all_strict collapses on an explicit strict:false tool", async () => { + const model: Model<"openai-completions"> = buildModel({ + ...(getBundledModel("openai", "gpt-4o-mini") as Model<"openai-completions">), + api: "openai-completions", + compat: { toolStrictMode: "all_strict" } satisfies OpenAICompat, + } as ModelSpec<"openai-completions">); + const payload = (await captureCompletionsPayload(model, { + ...testContext, + tools: [testTool, looseYieldTool], + })) as { + tools?: Array<{ function?: { strict?: boolean } }>; + }; + + // #4336: preserving explicit `false` MUST NOT leak into the all_strict → + // none collapse — providers that reject mixed strict values still get a + // uniformly absent flag. + expect(payload.tools).toHaveLength(2); + expect(payload.tools?.every(tool => tool.function?.strict === undefined)).toBe(true); + }); + it("surfaces captured JSON error bodies when the SDK reports no body", async () => { const model: Model<"openai-completions"> = { ...(getBundledModel("openai", "gpt-4o-mini") as Model<"openai-completions">), @@ -601,7 +641,7 @@ describe("OpenAI tool strict mode", () => { expect(payload.tools?.[0]?.strict).toBe(true); }); - it("keeps loose yield schemas non-strict for openai-responses", async () => { + it("preserves explicit strict:false on the wire for openai-responses", async () => { const model = getBundledModel("openai", "gpt-5-mini") as Model<"openai-responses">; const payload = (await captureResponsesPayload(model, { ...testContext, @@ -611,7 +651,9 @@ describe("OpenAI tool strict mode", () => { }; const tool = payload.tools?.[0]; - expect(tool?.strict).toBeUndefined(); + // #4336: authors who opt out via `tool.strict === false` see the flag land + // on the wire so backends that distinguish it from omission behave correctly. + expect(tool?.strict).toBe(false); expect(getYieldDataSchema(tool?.parameters).additionalProperties).toBe(true); });