fix(ai): preserved explicit tool.strict:false on OpenAI-family payloads

The tool serializers for openai-responses, openai-codex-responses, and
openai-completions previously spread strict only when the effective
value was true, so an author's tool.strict === false collapsed into an
omitted flag on the wire. Some OpenAI-compat backends distinguish the
two states — omitted lets the model over-fill optional arguments with
placeholders while explicit false suppresses them — so the drop lost
author intent.

Wire emission now preserves author-set false. For openai-completions the
preservation is gated on toolStrictMode === 'mixed' with
compat.supportsStrictMode !== false, because the all_strict → none
collapse and providers that reject the strict key rely on uniformly
absent values.

Fixes #4336
This commit is contained in:
roboomp
2026-07-02 18:25:47 +00:00
parent 9e0b6cddf2
commit 41c6630a27
8 changed files with 122 additions and 8 deletions
+4
View File
@@ -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
@@ -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 } : {}),
};
});
}
@@ -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<string, unknown>)
: wireParameters,
// Only include strict if provider supports it. Some reject unknown fields.
...(includeStrict && { strict: true }),
...(includeStrict ? { strict: true } : includeExplicitFalse ? { strict: false } : {}),
},
};
}),
@@ -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;
@@ -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.
---
@@ -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);
});
+47
View File
@@ -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", () => {
@@ -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);
});