fix(openai-chat): enforce type:"object" on all tool parameters, not just Kimi - #933
fix(openai-chat): enforce type:"object" on all tool parameters, not just Kimi#933IMHinnG wants to merge 2 commits into
Conversation
…ust Kimi DeepSeek (and likely other providers) reject function schemas where the root `parameters.type` is `null` or missing — the error is: Invalid schema for function 'codex_app__automation_update': schema must be a JSON Schema of 'type: "object"', got 'type: null' Codex tools with oneOf/anyOf composition schemas, or tools injected via the Responses-to-Chat translation path, may reach `toolsToChatFormat` with a null or absent root type. Previously only Kimi and xAI had provider-specific workarounds; every other provider received the raw parameters unchanged. This change removes the provider allowlist and applies `ensureRootObjectType()` unconditionally to all non-xAI providers. This is safe because JSON Schema for function parameters at the root MUST be an object — the function is a no-op when `type` is already `"object"`, and only adds the missing key otherwise. The same unconditional strategy is already used by `normalizeToolSchemas()` in `openai-responses.ts`. Changes: - Rename `ensureKimiRootObjectType` → `ensureRootObjectType` (no longer Kimi-specific) and update its doc comment. - Apply it to ALL non-xAI providers in `toolsToChatFormat()`, replacing the three-way branch (xAI / Kimi / passthrough) with a two-way branch (xAI / normalize for everyone else). - Remove `isKimiSchemaTarget()` — dead code after the refactor.
|
This pull request targets @IMHinnG Rebase onto the current This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again. |
github-actions
Bot
changed the title
📝 WalkthroughWalkthroughThe OpenAI chat adapter renames the Kimi-specific root schema normalizer and applies the generalized ChangesTool schema normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/openai-chat.ts`:
- Around line 411-420: Add regression tests around ensureRootObjectType and the
provider request path covering missing, null, and non-object root types becoming
"object" while preserving description, properties, required, and other fields;
confirm existing object-root schemas remain unchanged and undefined, primitive,
and array parameters use the empty-object fallback. Add xAI oneOf and anyOf
cases verifying normalizeXaiToolParameters still handles them, and validate
representative Kimi, DeepSeek, and xAI payloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 075d23b1-0faa-40bd-a16c-b1d57c118b8b
📒 Files selected for processing (1)
src/adapters/openai-chat.ts
| * Several providers (Kimi, DeepSeek) require function.parameters.type to be | ||
| * exactly "object" at the root and reject `type:null` or missing type. Codex | ||
| * tools with oneOf/anyOf schemas may omit the root type, causing 400 errors. | ||
| * | ||
| * Safe to apply unconditionally: JSON Schema for function parameters at the | ||
| * root MUST be an object — this is a no-op when type is already "object", | ||
| * and fixes every non-conforming case. Mirror of normalizeToolSchemas in | ||
| * openai-responses.ts. | ||
| */ | ||
| function ensureKimiRootObjectType(parameters: unknown): Record<string, unknown> { | ||
| function ensureRootObjectType(parameters: unknown): Record<string, unknown> { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Add regression coverage for the expanded provider contract.
Line 474 now applies ensureRootObjectType to every non-xAI provider. Add tests for:
- Missing,
null, and non-object root types becoming"object". - Preservation of
description,properties,required, and other schema fields. - Existing
{ type: "object" }schemas remaining unchanged. undefined, primitive, and array parameters receiving the intended empty-object fallback.- xAI
oneOfandanyOfschemas continuing throughnormalizeXaiToolParameters.
Verify representative Kimi, DeepSeek, and xAI payloads before merge.
Also applies to: 474-474
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adapters/openai-chat.ts` around lines 411 - 420, Add regression tests
around ensureRootObjectType and the provider request path covering missing,
null, and non-object root types becoming "object" while preserving description,
properties, required, and other fields; confirm existing object-root schemas
remain unchanged and undefined, primitive, and array parameters use the
empty-object fallback. Add xAI oneOf and anyOf cases verifying
normalizeXaiToolParameters still handles them, and validate representative Kimi,
DeepSeek, and xAI payloads.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7cb02c605
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const parameters = xaiTarget | ||
| ? normalizeXaiToolParameters(t.parameters) | ||
| : kimiTarget | ||
| ? ensureKimiRootObjectType(t.parameters) | ||
| : t.parameters; | ||
| : ensureRootObjectType(t.parameters); |
There was a problem hiding this comment.
This unconditional ensureRootObjectType path now changes the non-xAI example.test tool schema by adding root type: "object", but the committed focused coverage in tests/xai-transport.test.ts still asserts that the same non-xAI schema is preserved exactly. Once dependencies are installed, that existing test will fail unless the expectation is updated or replaced with coverage for the new generic non-xAI normalization behavior.
AGENTS.md reference: src/AGENTS.md:L22-L26
Useful? React with 👍 / 👎.
…oviders
Expand the root-type fix to recursively remove `type:null` (JS null,
JSON-Schema "null" string, and {type:"null"} inside anyOf/oneOf arrays)
at every nesting level. Codex tools like `codex_app__automation_update`
ship nullable fields such as:
localEnvironmentConfigPath: { anyOf: [{ type: "string" }, { type: "null" }] }
DeepSeek rejects all forms of `type:null` — at root and nested — while
the original Kimi fix only enforced `type:"object"` at the root.
Changes from v1:
- Add `fixNullTypes()` — recursive walk that replaces standalone
`type:null`/`type:"null"` with `type:"string"` and filters
`{type:"null"}` entries out of anyOf/oneOf/allOf arrays.
- Call `fixNullTypes` from `ensureRootObjectType` so every non-xAI
provider gets both root and deep normalization.
|
Retargeted from The change itself is worth having: the current Two things I want to look at before merging, and I will do that on a proper review pass rather than holding it in a comment:
CI is running now. I will review properly once it settles. |
github-actions
Bot
changed the title
|
Confirmed this still reproduces on opencodex 2.10.0 with Codex Desktop 0.146.0 (macOS, Every Codex turn re-sends the full tool catalog, so a single offending tool bricks the whole session against DeepSeek (retries return the same 400, looks like a hang). I verified locally that normalizing the tool schemas before forwarding to DeepSeek — coercing +1 for getting this merged — happy to re-test against the branch if useful. |
3 participants
Problem
DeepSeek (and any provider that strictly validates JSON Schema) rejects function
definitions where the root
parameters.typeisnullor missing:This happens when Codex Desktop sends automation tools to the LLM. Codex tools
with
oneOf/anyOfcomposition schemas, or tools injected via theResponses-to-Chat translation path, may reach
toolsToChatFormatwith a null orabsent root type.
Previously, the fix was only applied to two providers — xAI and Kimi. Every
other provider received the raw parameters unchanged, so any non-Kimi/non-xAI
provider that validates JSON Schema strictly (DeepSeek, and likely others) hits
this 400 error.
Root cause
toolsToChatFormat()inopenai-chat.tshad a three-way branch:The else-path is incorrect: JSON Schema for function parameters at the root
must always be
type: "object". Sendingtype: nullto any provider thatvalidates the schema will fail.
Fix
Make
ensureRootObjectType()the default for ALL non-xAI providers by removingthe Kimi gate, collapsing the three-way branch into a two-way branch:
This is exactly how
normalizeToolSchemas()inopenai-responses.tsalreadyworks — the same normalization is applied unconditionally to all providers there.
Only the chat-completions path had the provider-gated logic.
Specific changes
Rename
ensureKimiRootObjectType→ensureRootObjectType(no longer Kimi-specific) and update its doc comment to reflect the broader
scope and explain why unconditional application is safe.
Apply unconditionally: all non-xAI providers now go through
ensureRootObjectType(), which is a no-op whentypeis already"object",and only adds the missing key otherwise.
Remove
isKimiSchemaTarget()— dead code after the refactor.Why unconditional is safe
type: "object"ensureRootObjectTypeis a no-op whentypeis already"object"normalizeToolSchemasinopenai-responses.tswhich already applies to all providers unconditionally
Tested
Manually verified on a local opencodex deployment with
deepseek-v4-flash—requests containing
codex_app__automation_updatethat previously 400'd nowsucceed normally.
Related
codex_app__automation_updatewithtype: nullnormalizeFunctionToolSchemainopenai-responses.ts:358Summary by CodeRabbit