fix: normalize tool function schemas to always have root type: "object" - #745
fix: normalize tool function schemas to always have root type: "object"#745white-source wants to merge 1 commit into
Conversation
Some tool definitions (e.g. Codex `codex_app__automation_update`) arrive without a `parameters` field or with `parameters: null`. When serialized, that produces a JSON Schema whose root `type` is `null` (or missing). Strict validators — such as DeepSeek — reject such schemas with HTTP 400. Changes in two places to cover both forwarding paths: 1. `src/responses/parser.ts` (`buildTools`): Coerce every function tool's `parameters` to a valid object-schema root. Covers the adapter translation path (Anthropic, Gemini, Cursor, etc.). 2. `src/adapters/openai-responses.ts` (`createResponsesPassthroughAdapter`): Add `normalizeToolSchemas()` that fixes top-level `tools` and Responses Lite `additional_tools[].tools` before serialization. Required because the passthrough path (e.g. DeepSeek with `wire_api=responses`) forwards `_rawBody` verbatim and bypasses the parsed `context.tools`. Fixes the 400 error: "Invalid schema for function: schema must be a JSON Schema of 'type: \"object\"', got 'type: null'."
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFunction-tool parameter schemas are normalized to object schemas with object ChangesFunction tool 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🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4386da79d3
ℹ️ 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".
| name: t.name as string, | ||
| description: (t.description as string) ?? "", | ||
| parameters: (t.parameters ?? {}) as Record<string, unknown>, | ||
| parameters: normalizeParameters(t.parameters), |
There was a problem hiding this comment.
This changes the request/tool schema behavior in src/ but the commit does not add any focused regression coverage, so cases like missing/null parameters on top-level tools and Responses Lite additional_tools can regress without a failing test. Please add targeted coverage near the existing responses parser/passthrough tests before merging.
AGENTS.md reference: AGENTS.md:L149-L151
Useful? React with 👍 / 👎.
| if (!isPlainObject(body)) return body; | ||
|
|
||
| const fixFunctionParams = (tool: unknown): unknown => { | ||
| if (!isPlainObject(tool) || tool.type !== "function") return tool; |
There was a problem hiding this comment.
When a Responses passthrough request carries MCP tools grouped as {type: "namespace", tools: [{type: "function", parameters: null}]} and the target is not the Spark path that flattens namespaces first, this guard returns the namespace unchanged, so the inner function still reaches strict Responses gateways with the same missing/null root schema that this patch is meant to prevent. Please recurse into namespace tools for both top-level tools and additional_tools containers, or flatten before normalizing.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| // so all downstream adapters pass through cleanly. | ||
| const normalizeParameters = (raw: unknown): Record<string, unknown> => { | ||
| const p = isObj(raw) ? { ...raw } : {}; | ||
| if (p.type !== "object") p.type = "object"; |
There was a problem hiding this comment.
For routed xAI requests where the client supplies an unsafe function schema such as {type: "string"}, this parser-level coercion rewrites it to an empty object schema before normalizeXaiToolParameters() can reject it, bypassing the existing safeguard that omits unnormalizable tools. That makes the model see and call a tool with an argument contract the client did not declare; limit this normalization to missing/null roots or let provider-specific sanitizers handle non-object schemas.
Useful? React with 👍 / 👎.
|
NEEDS-CHANGES — the change looks right, but it ships with no tests. The only files here are That matters more than usual because the normalization lands on two independent paths. The cases worth pinning:
Please assert against the final serialized upstream request body rather than a helper's return value. That is what proves both production paths actually apply the normalization, which is the claim the PR is making. What happens next: add that coverage and push. The change itself reads correctly; it just needs something holding it in place. CI context (shared across the current review round). |
lidge-jun
left a comment
There was a problem hiding this comment.
Formalizing the disposition so it shows on your dashboard.
Blocking: no regression tests. The only changed files are src/adapters/openai-responses.ts and src/responses/parser.ts, and AGENTS.md asks for focused coverage on any behaviour change in src/.
That matters more than usual here because the normalization lands on two independent paths — parsed tool translation and raw Responses passthrough including nested additional_tools. A regression in one is invisible from the other, and both can fail in either direction: sending an invalid schema, or rewriting a valid one.
Please assert against the final serialized upstream request body rather than a helper return value, so the tests prove both production paths actually apply it. The case list is in my earlier comment.
The change itself reads correctly — it just needs something holding it in place.
Providers that validate strictly, DeepSeek among them, reject a function
schema whose root has no type. The parser forwarded (t.parameters ?? {})
untouched, so a tool declared without parameters — or with properties
but no root type — went out as an invalid schema.
Both routes are covered. The parser normalizes at build time, and the
raw Responses passthrough normalizes top-level tools and
input[].additional_tools.tools before serializing, because that path
copies _rawBody and never reaches the parser. Fixing only the parser
would have left every passthrough request still broken.
Valid schemas are returned unchanged rather than rebuilt, so existing
property sets cannot be reordered or dropped on the way through.
PR #745 by @white-source proposed the right normalizers but carried no
tests, which is why it could not land as-is. Ablating both normalizers
fails three tests: the parser root-type case and the two serialized
passthrough cases.
Co-authored-by: Mr.Hat <white-source@users.noreply.github.com>
|
Landed on Your diagnosis and both normalizers were right. What held this back was that the PR carried no regression test, and this repo has shipped tests twice recently that passed against both the fixed and unfixed behavior — so "the tests are green" was not evidence the fix worked. What landed:
Ablating both normalizers fails three tests, so they are not vacuous. Closing this PR since the change is on |
Problem
Some provider APIs (notably DeepSeek) strictly validate JSON Schema in function tool definitions and reject requests where the root
typeisnullor missing with HTTP 400:This occurs because Codex injects tools like
codex_app__automation_updatewithout aparametersfield. When opencodex forwards the request, the serialized schema hastype: null(or notypeat all) at its root.OpenAI and most other providers tolerate this, but stricter validators reject it. The same issue has been reported across multiple proxy projects:
Root Cause
Two forwarding paths in opencodex can pass through a broken tool schema:
buildTools()inparser.tsuses(t.parameters ?? {})— whenparametersis null/absent, the resulting object has no roottype.wire_api=responsesproviders like DeepSeek):createResponsesPassthroughAdapterserializes_rawBodyverbatim. The raw body contains the same broken tool schemas directly.Fix
1.
src/responses/parser.ts—buildTools()Added
normalizeParameters()helper that ensures every function tool'sparametersis a valid JSON Schema object withtype: "object"and apropertieskey. Covers the adapter translation path (Anthropic, Gemini, Cursor, etc.).2.
src/adapters/openai-responses.ts— passthrough adapterAdded
normalizeToolSchemas()function that walks both top-levelbody.toolsand Responses Liteadditional_tools[].tools, fixing any function tool whose parameters schema is invalid. Inserted into the sanitization chain at the outermost position so it runs regardless of which strip functions are active.Both changes are idempotent — already-valid schemas pass through unchanged. Tools that are not
type: "function"are not touched.Testing
bun buildsyntax check on both filesbun x tsc --noEmittypecheck passesparameters,parameters: null,type: null, already-valid schemas, nestedadditional_tools, non-function tool typesdeepseek-v4-proviawire_api=responses) — the 400 error is resolvedNotes
This is a defensive normalization — the output is a valid JSON Schema regardless of what the upstream tool definition provides. The change is minimal (70 lines added across 2 files) and does not alter any existing behavior for already-valid schemas.
Summary by CodeRabbit