Skip to content

fix(openai): reject oneOf/allOf and validate $defs/$ref before enabling strict mode (#4106) - #4155

Merged
dgageot merged 1 commit into
mainfrom
fix/openai-strict-schema-oneof-defs
Sep 3, 2026
Merged

fix(openai): reject oneOf/allOf and validate $defs/$ref before enabling strict mode (#4106)#4155
dgageot merged 1 commit into
mainfrom
fix/openai-strict-schema-oneof-defs

Conversation

@aheritier

Copy link
Copy Markdown
Collaborator

🤖 Automated implementer agentthis comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Closes #4106.

Problem

isStrictCompatible/hasIncompatibleNode only checked additionalProperties, never rejected oneOf/allOf/other composition keywords OpenAI Structured Outputs doesn't support, and neither it nor the schema normalizer (walkSchema) traversed $defs/definitions. A tool whose schema used oneOf (the issue's repro) or had an unnormalized $defs entry was sent to OpenAI with strict: true and rejected with HTTP 400, killing the whole request.

A third, related defect found during design: the shared pkg/tools/schema.go ensurePropertyTypes helper injected type: "object" onto $ref nodes, which downstream normalization then widened with additionalProperties/nullable-type — producing a $ref node with sibling keywords, which OpenAI strict mode rejects.

Fix

  • pkg/model/provider/openai/schema.go: shared childSchemas iterator (iter.Seq) used by both the compatibility check and the normalizer, covering properties/patternProperties/$defs/definitions/anyOf/oneOf/allOf/prefixItems/items/additionalProperties/not/if/then/else/contains/propertyNames.
  • hasIncompatibleNode rejects oneOf, allOf, not, if, then, else, dependentRequired, dependentSchemas outright, and validates $ref nodes via a small local JSON-pointer resolver (string, local #/#/..., resolvable, no sibling keywords).
  • $ref nodes are treated as leaves by makeAllRequired/ensureTypeFields (no type/additionalProperties injection). A newly-required (optional) $ref property is wrapped as {"anyOf": [<$ref>, {"type": "null"}]}, OpenAI's documented optional-reference pattern — with ensureTypeFields also skipping anyOf/oneOf/allOf nodes so this wrapper doesn't get a conflicting type sibling (a real bug an earlier draft of this PR had, caught in review).
  • pkg/tools/schema.go's ensurePropertyTypes skips $ref nodes (the root cause of the pollution above).

Testing

  • New test coverage in pkg/model/provider/openai/schema_test.go (T1–T14 from the design doc, plus an invariant test: normalizing a strict-compatible schema must never make it incompatible) and pkg/tools/schema_test.go.
  • task build, task test, task lint all green (full suite, including go mod tidy --diff and the project's custom go run ./lint . cops).
  • Not automated: a live-API check of whether OpenAI accepts a $ref node with a description sibling under strict: true (would relax the current conservative "no sibling keywords" rule to "no siblings other than description", per the openai-python SDK precedent this design follows). No OpenAI API key was available in this environment. Documented as a follow-up rather than blocking this fix.

Out of scope

Structured Outputs size limits, root-level anyOf rejection, anyOf-typed optional property nullable-widening (pre-existing, unrelated gap), and inlining $ref (rejected approach — would break recursive schemas).

…ng strict mode (#4106)

OpenAI Structured Outputs strict mode rejects oneOf/allOf/not/if-then-else/
dependent* composition keywords and $ref nodes with sibling keywords, but
isStrictCompatible only checked additionalProperties and never traversed
$defs/definitions. The pipeline also injected a stray 'type: object' onto
$ref nodes, which downstream normalization then widened with
additionalProperties/nullable — corrupting an otherwise-valid $ref.

- Factor a shared childSchemas iterator used by both the compatibility
  check and the normalization walker, covering properties,
  patternProperties, $defs/definitions, anyOf/oneOf/allOf/prefixItems,
  items, additionalProperties, and the remaining single-schema keywords.
- hasIncompatibleNode now rejects oneOf/allOf/not/if/then/else/
  dependentRequired/dependentSchemas outright, and validates $ref nodes
  via a small local JSON-pointer resolver (string, local, resolvable,
  no sibling keywords).
- makeAllRequired/ensureTypeFields treat $ref nodes as leaves; a
  newly-required $ref property is wrapped as
  {anyOf: [<$ref>, {type: null}]}, OpenAI's documented optional-ref
  pattern.
- pkg/tools/schema.go's ensurePropertyTypes skips $ref nodes so no
  provider gets a polluted $ref.

Adds T1-T15 coverage per the design doc, including an invariant test
that normalization never turns a strict-compatible schema
incompatible.
@aheritier
aheritier marked this pull request as ready for review September 3, 2026 18:58
@aheritier
aheritier requested a review from a team as a code owner September 3, 2026 18:58
@aheritier aheritier added area/providers/openai For features/issues/fixes related to the usage of OpenAI models area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Sep 3, 2026
@dgageot
dgageot merged commit 99ba4ba into main Sep 3, 2026
16 checks passed
@dgageot
dgageot deleted the fix/openai-strict-schema-oneof-defs branch September 3, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/providers/openai For features/issues/fixes related to the usage of OpenAI models area/tools For features/issues/fixes related to the usage of built-in and MCP tools kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool Schema Incorrectly Classified as Strict-Compatible

2 participants