fix(api): fail-closed unknown stream_options keys and tools-path message honesty - #592
fix(api): fail-closed unknown stream_options keys and tools-path message honesty#592cursor[bot] wants to merge 9 commits into
Conversation
…closed otherwise Chat history: message-level audio and legacy function_call are null/empty omit no-ops; non-empty fail closed with named errors (including tools passthrough). Tip substrate from #577 assistant refusal/annotations honesty. Local full unit: 940 passed.
…ed otherwise OpenAI fine-tune style message weight is not applied on this gateway. Accept null/0/1 as honest no-ops; reject other types and values with invalid_message_weight. Tip substrate from #578. Local full unit: 943 passed.
…ion role Reject unsupported message keys with named unknown_message_fields (not silent strip or tools-passthrough smuggle). Reject legacy function role with invalid_message_role migration to tool. Tip substrate from #579. Local full unit: 947 passed.
OpenAI partial-assistant prefix flag is not applied on this gateway. null/false are honest no-ops; true and non-booleans fail closed with invalid_message_prefix. Tip substrate from #580. Local full unit: 950 passed.
…therwise Named invalid_max_tool_calls on /v1/chat/completions instead of opaque unknown_fields. Aligns with Responses max_tool_calls honesty; gateway has no multi-step tool loop.
…losed otherwise Legacy /v1/completions treated max_tool_calls as unknown_fields. Accept the key for named invalid_max_tool_calls (null/empty/whitespace omit-equivalent), matching chat/Responses honesty so SDKs get a clear migration path.
SDK clients often send include_usage/include_obfuscation as JSON null. Drop null flag values before validation so null (and null+false mixes) match omit / all-false no-ops on chat, Completions, and Responses. True flags remain fail-closed with invalid_stream_options.
…age honesty Null include_usage/include_obfuscation stay omit-equivalent, but unknown stream_options keys no longer become no-ops just because their value is null. Weight, prefix, refusal, annotations, developer role, empty user/system content, and participant name now use the same named errors on the tools passthrough path as on orchestration. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Hoist stream, required model, stream_options, and temperature/top_p range checks before proxy_completion so a tools or response_format body cannot return a billed JSON completion when the SDK asked for SSE, or silently pick a pool model when model is omitted. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
There was a problem hiding this comment.
Stale comment
Review — #592 at
856adb8Prefer this head over #582–#587 and draft #593. Do not merge
b108251alone: that SHA still skippedstream/model/stream_optionson the tools path.856adb8hoists those checks beforeproxy_completion.What this head gets right
_normalized_stream_option_flagsfail-closes unknown keys before dropping nulls.{include_continuous: null}is no longer omit-equivalent on chat, Completions, and Responses.- Tools-path message honesty (
weight,prefix,refusal,annotations,developer, empty user/system content,name) runs before the passthrough return.856adb8then fail-closesstream=true, omitted/unknownmodel,stream_options, and out-of-rangetemperature/top_pon tools /response_format.Do not merge yet
- Independent non-author APPROVE is still required (
cursor[bot]is the author; this automation cannot satisfy that gate).- Product gates (Full unit + Semgrep) were still queued when this review ran.
mergeable_state: blocked.- Residual fail-open on this tip:
attributionandroutingstill run only after the toolsreturn. Tools + unknown attribution dimensions, orrouting.channel=batch/latency_tolerant=true, still bill a sync proxy. Empty / omitted / non-listmessagesstill skip_validate_messages— that slice is #594, do not duplicate it here.Buyer next action
- Send SDK-default
stream_optionsas omit /null/false. Do not setinclude_usage=trueorinclude_obfuscation=true.- On tool-calling requests: omit
stream(or setfalse); always send a poolmodel; replacedeveloperwithsystem.- Do not send
routing.channel=batchorlatency_tolerant=truewithtoolsuntil a follow-up fail-closes that path (next slice on this stack).- Reviewer
seonghobae: APPROVE only after Full unit + Semgrep are green and you agree the residual attribution/routing hole can land as a follow-up.Related heads to leave unmerged: #582–#587, #591, #593. Prefer #592 @
856adb8as the stream_options + tools honesty merge candidate. Empty-messages repair is #594.Sent by Cursor Automation: Fix Issues
There was a problem hiding this comment.
Review (#592 @ 856adb8)
The unique tip closes the two claimed defects. Do not merge this head. GitHub rejects REQUEST_CHANGES on this cursor[bot] PR; this is a COMMENT with the same merge block. Inline comments skipped: the 142-file diff is too_large.
Verified
- Unknown
stream_optionskeys fail closed before null-drop. Live HTTP400invalid_stream_optionsfor{include_continuous: null}on chat, Completions, Responses, and withtools. - Weight / prefix / refusal / annotations / developer / empty user content / empty name /
input_audio→400with the named orchestration errors. stream=true, omittedmodel,include_usage=true,temperature=99,top_p=2→400beforeproxy_completion.- Unique-tip HTTP scripts are real loopback cases, not tautologies. Local runs of
test_stream_options_null_flags_noop_http_honesty.py,test_message_honesty_tools_passthrough_http_honesty.py, andtest_passthrough_stream_model_http_honesty.pypassed.
Must-fix in _validate_chat_message_passthrough_honesty (live on this head)
tools+messages: []/ omitted /null/"hi"→ 200chat.completion(prompt_tokens: 0). The helper returns whenmessagesis not a list.tools+messages: ["hi"]→ 500internal_error(str.getin the proxy). The helpercontinues on non-dict entries.tools+role: "narrator"orcontent: 42→ 200 (helper only type-checks list parts and empty user/system strings).tools+ unknownattribution/routingkeys ormax_tokens: -1→ 200 (those validators still run only after the early-return).
Overlap
- Do not merge #587 at
9d10fa9(null-drop before unknown-key check). - Do not land #591 or draft #593 in parallel (subset of this hoist, no unknown-null repair).
- #589 / #594 close empty-messages on the older #586 substrate and miss this unknown-null + stream/model/sampling tip.
- Repair that includes this tip plus empty-messages / non-object / spend-key honesty: #597 (
169a382). Prefer #597 over merging this head.
Buyer next action
Do not merge #592. Take #597. Until that lands: always send a non-empty messages array of objects; keep SDK-default stream_options null/false; omit stream (or set false) on tool-calling requests; always send a pool model; replace developer with system. Do not send tools with empty or omitted messages — this head will still bill a completion.
Sent by Cursor Automation: Fix Issues
|
Superseded by the #597 → #601 → #607 successor chain. #597 starts from this exact honesty tip and preserves its stream/model/message/stream_options safeguards while adding the later empty-message/spend/routing boundary; #601 and #607 extend that same landing line further. Keeping #592 open duplicates an older substrate. No checks, reviews, or approvals transfer. |


Summary
Repair for #587 at
9d10fa9, and a superset of #591's request-level hoist.stream_options.include_usage/include_obfuscationJSONnull(and null+false mixes) as omit-equivalent no-ops on chat, Completions, and Responses.stream_optionskeys fail closed even when their value isnull.weight,prefix,refusal,annotations,developerrole, empty user/system content, multimodal part shape, and participantnameuse the same named errors beforeproxy_completion.stream=true, omitted/unknownmodel, and out-of-rangetemperature/top_p(the fix(api): fail-closed stream, model, and sampling range on tools passthrough #591 hoist).Buyer next action: send SDK-default null flags; omit
stream(or setfalse) on tool-calling requests; always send a poolmodel; replacedeveloperwithsystem. Do not setinclude_usage=true/include_obfuscation=true.Do not merge #587 at
9d10fa9. Prefer this head over #582–#587 and #591. Merge only after Full unit + Semgrep are green and an independent non-author review lands.Test plan
python tests/test_stream_options_null_flags_noop_http_honesty.pypython tests/test_message_honesty_tools_passthrough_http_honesty.pypython tests/test_passthrough_stream_model_http_honesty.pypython tests/test_openai_passthrough.pyDocs
docs/rest_api_design.mdlists Completions / Responses / embeddings and the honesty contract (APA: OpenAI, 2024).