fix(provider): enforce strict schemas for codex tools (#200)
- commit
- 21873e8969af29877569286b9b03d105e195a3b5
- author
- bnema · · 2026-09-10 11:46 UTC
- committer
- GitHub · · 2026-09-10 11:46 UTC
- parent
- e169d60
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown - refs
- tag: v0.35.0
2 files · +85 -16
M packages/provider/openai_codex.go +41 -0
| @@ -197,6 +197,46 @@ type codexReasoningConfig struct { | |||
| 197 | 197 | Effort string `json:"effort,omitempty"` | |
| 198 | 198 | } | |
| 199 | 199 | ||
| 200 | + | // supportsStrictToolSchema reports whether the top-level object already meets | |
| 201 | + | // Responses strict-mode requirements. Keep this deliberately narrow: schemas | |
| 202 | + | // with nested containers or composition remain non-strict rather than risking | |
| 203 | + | // provider rejection or changing optional fields into required ones. | |
| 204 | + | func supportsStrictToolSchema(raw json.RawMessage) bool { | |
| 205 | + | var schema struct { | |
| 206 | + | Type string `json:"type"` | |
| 207 | + | Properties map[string]json.RawMessage `json:"properties"` | |
| 208 | + | Required []string `json:"required"` | |
| 209 | + | AdditionalProperties *bool `json:"additionalProperties"` | |
| 210 | + | } | |
| 211 | + | if json.Unmarshal(raw, &schema) != nil || schema.Type != "object" || schema.AdditionalProperties == nil || *schema.AdditionalProperties { | |
| 212 | + | return false | |
| 213 | + | } | |
| 214 | + | required := make(map[string]struct{}, len(schema.Required)) | |
| 215 | + | for _, name := range schema.Required { | |
| 216 | + | required[name] = struct{}{} | |
| 217 | + | } | |
| 218 | + | if len(required) != len(schema.Properties) { | |
| 219 | + | return false | |
| 220 | + | } | |
| 221 | + | for name, rawProperty := range schema.Properties { | |
| 222 | + | if _, ok := required[name]; !ok { | |
| 223 | + | return false | |
| 224 | + | } | |
| 225 | + | var property struct { | |
| 226 | + | Type string `json:"type"` | |
| 227 | + | } | |
| 228 | + | if json.Unmarshal(rawProperty, &property) != nil { | |
| 229 | + | return false | |
| 230 | + | } | |
| 231 | + | switch property.Type { | |
| 232 | + | case "string", "number", "integer", "boolean": | |
| 233 | + | default: | |
| 234 | + | return false | |
| 235 | + | } | |
| 236 | + | } | |
| 237 | + | return true | |
| 238 | + | } | |
| 239 | + | ||
| 200 | 240 | type codexRequest struct { | |
| 201 | 241 | Model string `json:"model"` | |
| 202 | 242 | Store bool `json:"store"` | |
| @@ -318,6 +358,7 @@ func (c *codexClient) buildRequest(req Request) (*codexRequest, error) { | |||
| 318 | 358 | Name: t.Name, | |
| 319 | 359 | Description: t.Description, | |
| 320 | 360 | Parameters: params, | |
| 361 | + | Strict: providerName == ProviderOpenAICodex && supportsStrictToolSchema(params), | |
| 321 | 362 | }) | |
| 322 | 363 | } | |
| 323 | 364 | } | |
| @@ -197,6 +197,46 @@ type codexReasoningConfig struct { | |||||
| 197 | Effort string `json:"effort,omitempty"` | 197 | Effort string `json:"effort,omitempty"` | ||
| 198 | } | 198 | } | ||
| 199 | 199 | ||||
| 200 | + | // supportsStrictToolSchema reports whether the top-level object already meets | |||
| 201 | + | // Responses strict-mode requirements. Keep this deliberately narrow: schemas | |||
| 202 | + | // with nested containers or composition remain non-strict rather than risking | |||
| 203 | + | // provider rejection or changing optional fields into required ones. | |||
| 204 | + | func supportsStrictToolSchema(raw json.RawMessage) bool { | |||
| 205 | + | var schema struct { | |||
| 206 | + | Type string `json:"type"` | |||
| 207 | + | Properties map[string]json.RawMessage `json:"properties"` | |||
| 208 | + | Required []string `json:"required"` | |||
| 209 | + | AdditionalProperties *bool `json:"additionalProperties"` | |||
| 210 | + | } | |||
| 211 | + | if json.Unmarshal(raw, &schema) != nil || schema.Type != "object" || schema.AdditionalProperties == nil || *schema.AdditionalProperties { | |||
| 212 | + | return false | |||
| 213 | + | } | |||
| 214 | + | required := make(map[string]struct{}, len(schema.Required)) | |||
| 215 | + | for _, name := range schema.Required { | |||
| 216 | + | required[name] = struct{}{} | |||
| 217 | + | } | |||
| 218 | + | if len(required) != len(schema.Properties) { | |||
| 219 | + | return false | |||
| 220 | + | } | |||
| 221 | + | for name, rawProperty := range schema.Properties { | |||
| 222 | + | if _, ok := required[name]; !ok { | |||
| 223 | + | return false | |||
| 224 | + | } | |||
| 225 | + | var property struct { | |||
| 226 | + | Type string `json:"type"` | |||
| 227 | + | } | |||
| 228 | + | if json.Unmarshal(rawProperty, &property) != nil { | |||
| 229 | + | return false | |||
| 230 | + | } | |||
| 231 | + | switch property.Type { | |||
| 232 | + | case "string", "number", "integer", "boolean": | |||
| 233 | + | default: | |||
| 234 | + | return false | |||
| 235 | + | } | |||
| 236 | + | } | |||
| 237 | + | return true | |||
| 238 | + | } | |||
| 239 | + | ||||
| 200 | type codexRequest struct { | 240 | type codexRequest struct { | ||
| 201 | Model string `json:"model"` | 241 | Model string `json:"model"` | ||
| 202 | Store bool `json:"store"` | 242 | Store bool `json:"store"` | ||
| @@ -318,6 +358,7 @@ func (c *codexClient) buildRequest(req Request) (*codexRequest, error) { | |||||
| 318 | Name: t.Name, | 358 | Name: t.Name, | ||
| 319 | Description: t.Description, | 359 | Description: t.Description, | ||
| 320 | Parameters: params, | 360 | Parameters: params, | ||
| 361 | + | Strict: providerName == ProviderOpenAICodex && supportsStrictToolSchema(params), | |||
| 321 | }) | 362 | }) | ||
| 322 | } | 363 | } | ||
| 323 | } | 364 | } | ||
M packages/provider/openai_codex_test.go +44 -16
| @@ -149,32 +149,60 @@ func TestGPT56UsesNativeMaxReasoningEffort(t *testing.T) { | |||
| 149 | 149 | } | |
| 150 | 150 | } | |
| 151 | 151 | ||
| 152 | - | func TestResponsesToolsExplicitlyDisableStrictSchemaNormalization(t *testing.T) { | |
| 152 | + | func TestResponsesToolsSelectStrictModeFromSchema(t *testing.T) { | |
| 153 | 153 | c := newOpenAICodexClient("token", "acct", "") | |
| 154 | 154 | wire, err := c.buildRequest(Request{ | |
| 155 | 155 | Model: "gpt-5.6-sol", | |
| 156 | - | Tools: []Tool{{ | |
| 157 | - | Name: "subagent_spawn", | |
| 158 | - | Schema: json.RawMessage(`{ | |
| 159 | - | "type":"object", | |
| 160 | - | "properties":{ | |
| 161 | - | "task":{"type":"string"}, | |
| 162 | - | "budget_ratio":{"type":"number"}, | |
| 163 | - | "budget_tokens":{"type":"integer"} | |
| 164 | - | }, | |
| 165 | - | "required":["task"] | |
| 166 | - | }`), | |
| 167 | - | }}, | |
| 156 | + | Tools: []Tool{ | |
| 157 | + | { | |
| 158 | + | Name: "grep", | |
| 159 | + | Schema: json.RawMessage(`{ | |
| 160 | + | "type":"object", | |
| 161 | + | "properties":{"pattern":{"type":"string"},"path":{"type":"string"}}, | |
| 162 | + | "required":["pattern","path"], | |
| 163 | + | "additionalProperties":false | |
| 164 | + | }`), | |
| 165 | + | }, | |
| 166 | + | { | |
| 167 | + | Name: "subagent_spawn", | |
| 168 | + | Schema: json.RawMessage(`{ | |
| 169 | + | "type":"object", | |
| 170 | + | "properties":{ | |
| 171 | + | "task":{"type":"string"}, | |
| 172 | + | "budget_ratio":{"type":"number"}, | |
| 173 | + | "budget_tokens":{"type":"integer"} | |
| 174 | + | }, | |
| 175 | + | "required":["task"] | |
| 176 | + | }`), | |
| 177 | + | }, | |
| 178 | + | }, | |
| 168 | 179 | }) | |
| 169 | 180 | if err != nil { | |
| 170 | 181 | t.Fatal(err) | |
| 171 | 182 | } | |
| 172 | - | encoded, err := json.Marshal(wire.Tools) | |
| 183 | + | if len(wire.Tools) != 2 { | |
| 184 | + | t.Fatalf("tools = %d, want 2", len(wire.Tools)) | |
| 185 | + | } | |
| 186 | + | if !wire.Tools[0].Strict { | |
| 187 | + | t.Fatal("closed, fully-required grep schema should use strict mode") | |
| 188 | + | } | |
| 189 | + | if wire.Tools[1].Strict { | |
| 190 | + | t.Fatal("schema with optional properties should opt out of strict normalization") | |
| 191 | + | } | |
| 192 | + | ||
| 193 | + | c.providerName = ProviderOpenCodeGo | |
| 194 | + | wire, err = c.buildRequest(Request{ | |
| 195 | + | Model: "gpt-5.6-sol", | |
| 196 | + | Tools: []Tool{{ | |
| 197 | + | Name: "grep", | |
| 198 | + | Schema: wire.Tools[0].Parameters, | |
| 199 | + | }}, | |
| 200 | + | }) | |
| 173 | 201 | if err != nil { | |
| 174 | 202 | t.Fatal(err) | |
| 175 | 203 | } | |
| 176 | - | if !bytes.Contains(encoded, []byte(`"strict":false`)) { | |
| 177 | - | t.Fatalf("responses tool omitted strict opt-out: %s", encoded) | |
| 204 | + | if wire.Tools[0].Strict { | |
| 205 | + | t.Fatal("non-OpenAI Responses provider should opt out of strict mode") | |
| 178 | 206 | } | |
| 179 | 207 | } | |
| 180 | 208 | ||
| @@ -149,32 +149,60 @@ func TestGPT56UsesNativeMaxReasoningEffort(t *testing.T) { | |||||
| 149 | } | 149 | } | ||
| 150 | } | 150 | } | ||
| 151 | 151 | ||||
| 152 | - | func TestResponsesToolsExplicitlyDisableStrictSchemaNormalization(t *testing.T) { | 152 | + | func TestResponsesToolsSelectStrictModeFromSchema(t *testing.T) { |
| 153 | c := newOpenAICodexClient("token", "acct", "") | 153 | c := newOpenAICodexClient("token", "acct", "") | ||
| 154 | wire, err := c.buildRequest(Request{ | 154 | wire, err := c.buildRequest(Request{ | ||
| 155 | Model: "gpt-5.6-sol", | 155 | Model: "gpt-5.6-sol", | ||
| 156 | - | Tools: []Tool{{ | 156 | + | Tools: []Tool{ |
| 157 | - | Name: "subagent_spawn", | 157 | + | { |
| 158 | - | Schema: json.RawMessage(`{ | 158 | + | Name: "grep", |
| 159 | - | "type":"object", | 159 | + | Schema: json.RawMessage(`{ |
| 160 | - | "properties":{ | 160 | + | "type":"object", |
| 161 | - | "task":{"type":"string"}, | 161 | + | "properties":{"pattern":{"type":"string"},"path":{"type":"string"}}, |
| 162 | - | "budget_ratio":{"type":"number"}, | 162 | + | "required":["pattern","path"], |
| 163 | - | "budget_tokens":{"type":"integer"} | 163 | + | "additionalProperties":false |
| 164 | - | }, | 164 | + | }`), |
| 165 | - | "required":["task"] | 165 | + | }, |
| 166 | - | }`), | 166 | + | { |
| 167 | - | }}, | 167 | + | Name: "subagent_spawn", |
| 168 | + | Schema: json.RawMessage(`{ | |||
| 169 | + | "type":"object", | |||
| 170 | + | "properties":{ | |||
| 171 | + | "task":{"type":"string"}, | |||
| 172 | + | "budget_ratio":{"type":"number"}, | |||
| 173 | + | "budget_tokens":{"type":"integer"} | |||
| 174 | + | }, | |||
| 175 | + | "required":["task"] | |||
| 176 | + | }`), | |||
| 177 | + | }, | |||
| 178 | + | }, | |||
| 168 | }) | 179 | }) | ||
| 169 | if err != nil { | 180 | if err != nil { | ||
| 170 | t.Fatal(err) | 181 | t.Fatal(err) | ||
| 171 | } | 182 | } | ||
| 172 | - | encoded, err := json.Marshal(wire.Tools) | 183 | + | if len(wire.Tools) != 2 { |
| 184 | + | t.Fatalf("tools = %d, want 2", len(wire.Tools)) | |||
| 185 | + | } | |||
| 186 | + | if !wire.Tools[0].Strict { | |||
| 187 | + | t.Fatal("closed, fully-required grep schema should use strict mode") | |||
| 188 | + | } | |||
| 189 | + | if wire.Tools[1].Strict { | |||
| 190 | + | t.Fatal("schema with optional properties should opt out of strict normalization") | |||
| 191 | + | } | |||
| 192 | + | ||||
| 193 | + | c.providerName = ProviderOpenCodeGo | |||
| 194 | + | wire, err = c.buildRequest(Request{ | |||
| 195 | + | Model: "gpt-5.6-sol", | |||
| 196 | + | Tools: []Tool{{ | |||
| 197 | + | Name: "grep", | |||
| 198 | + | Schema: wire.Tools[0].Parameters, | |||
| 199 | + | }}, | |||
| 200 | + | }) | |||
| 173 | if err != nil { | 201 | if err != nil { | ||
| 174 | t.Fatal(err) | 202 | t.Fatal(err) | ||
| 175 | } | 203 | } | ||
| 176 | - | if !bytes.Contains(encoded, []byte(`"strict":false`)) { | 204 | + | if wire.Tools[0].Strict { |
| 177 | - | t.Fatalf("responses tool omitted strict opt-out: %s", encoded) | 205 | + | t.Fatal("non-OpenAI Responses provider should opt out of strict mode") |
| 178 | } | 206 | } | ||
| 179 | } | 207 | } | ||
| 180 | 208 | ||||