fix(api): fail-closed stream, model, and sampling range on tools passthrough - #591
fix(api): fail-closed stream, model, and sampling range on tools passthrough#591cursor[bot] wants to merge 11 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.
Validate weight/prefix/refusal/annotations before the tools early-return so SDK tool-calling bodies cannot smuggle unsupported message fields. Strip omit-equivalent max_tool_calls before provider passthrough and name-reject the field on /v1/completions. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reject non-OpenAI keys on assistant tool_calls objects with named unknown_tool_call_fields / unknown_tool_call_function_fields. Accept optional non-negative index (null omit) for stream-assembled histories. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Developer role, empty user/system content, multimodal part shape, and participant name were only checked in _validate_messages, which the tools/response_format early-return skips. SDK tool-calling bodies could therefore proxy unsupported values. Run the same fail-closed checks before passthrough and cover the realistic tools+field HTTP cases. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Completions and chat applied temperature, top_p, penalties, and max_tokens by mutating shared ModelClient defaults. Concurrent ThreadingHTTPServer workers could observe another request's knobs. request_sampling stores overrides on threading.local and restores them on exit; chat/stream_chat read the calling thread first. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Missing, empty, or non-list messages skipped every pre-passthrough check and still reached proxy_completion when tools or response_format forced the early-return. Reject those bodies with invalid_message, and cover response_format+weight plus unknown tool_calls.function keys on the tools path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
…through SDK tool-calling bodies skipped request-level honesty on the tools/response_format early-return: stream=true returned JSON, omitted model silently picked a pool agent, and out-of-range temperature/top_p or include_usage=true were accepted. Reject those with named errors before proxy_completion; keep route-path SSE without tools. 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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review of the unique slice (afd8208..b7f81a6)
The claimed request-level hoist is real. stream=true, omitted/unknown model, stream_options.include_usage=true, and out-of-range temperature / top_p now fail closed inside the tools/response_format early-return, before proxy_completion. Route-path SSE without tools stays 200.
Live loopback POST /v1/chat/completions at b7f81a6 with nonempty tools:
| Body | Status | Code |
|---|---|---|
stream=true |
400 | invalid_stream |
omitted model |
400 | invalid_model |
model=gpt-4o (unknown pool) |
400 | invalid_model |
stream_options.include_usage=true |
400 | invalid_stream_options |
temperature=99 / top_p=2 |
400 | invalid_temperature / invalid_top_p |
temperature=0.2 |
200 | chat.completion |
stream=true without tools |
200 | SSE |
tests/test_passthrough_stream_model_http_honesty.py is real HTTP honesty, not a stub. Local script is green. This tip is more complete than #593 (cfcbdfe), which hoists only model + stream.
Do not merge this head in parallel
Prefer #596 (7a07562, draft) as the honesty-stack merge candidate. It is a documented superset: empty messages, attribution/routing, and the remaining request knobs (max_tokens, penalties, user, n/seed/stop/logprobs type-and-support checks) that still return 200 on this tip.
Do not merge #582–#586, #589, or #593 at their current heads. Independent non-author APPROVE + Full unit / Semgrep are still required on the merge candidate. Product gates only (Strix ignored).
Buyer next action
On tool-calling requests today: omit stream or set stream=false; send a pool model; keep temperature in [0, 2] and top_p in (0, 1]. Do not send stream_options.include_usage=true until SSE passthrough exists. For spend keys and the remaining knobs, wait for #596 or omit them.
CodeRabbit CLI 0.7.3 is installed here but auth status --agent is not_authenticated (browser callback). This review is live-HTTP + source of the unique 3-file slice.
Sent by Cursor Automation: Fix Issues
There was a problem hiding this comment.
Correction after comparing #596 / #597
The unique slice on this PR is still correct. Do not merge this head in parallel with #596 or #597.
#596 (7a07562) closes the remaining request knobs, empty messages, and attribution/routing that this tip still leaves 200. It is not a git or functional superset of this head: it drops unknown_tool_call_fields / unknown_tool_call_function_fields / optional index (cbd0420) and thread-local request_sampling (28ef9d2). Live on #596: tools + tool_calls[].smuggle returns 200; the same body is 400 here.
#597 (dbfa0e5) is a smaller spend/messages hoist and also lacks those tool_calls key checks.
Buyer next action: keep sending only id / type / function / optional index on assistant tool_calls. Do not merge #582–#597 at their current heads. The landing vehicle must carry this tip's tool_calls key honesty plus #596's remaining knobs.
Independent non-author APPROVE + Full unit / Semgrep still required. Do not self-approve.
Sent by Cursor Automation: Fix Issues
|
Superseded by the #592 → #597 → #601 → #611 landing line. #592 explicitly incorporates this PR’s stream/model/sampling-range hoist and #611 is the current KV/security landing vehicle on the later honesty substrate. Keeping #591 open duplicates an older request-boundary implementation. No checks, reviews, or approvals transfer. |
Pull request was closed
2 participants
Summary
stream=true, omittedmodel,stream_options.include_usage=true, and out-of-rangetemperature/top_pnow fail closed beforeproxy_completion.stream=truewith tools orresponse_formatreturns namedinvalid_stream(SSE passthrough is still a follow-up). Setstream=falseor omitstream.modelreturns namedinvalid_modelinstead of a silent pool pick. In-rangetemperaturestill forwards on passthrough.This branch is #586 (
afd8208) plus the request-level hoist. Prefer this head over #582–#586. Do not merge those heads.Test plan
python3 tests/test_passthrough_stream_model_http_honesty.pypython3 tests/test_message_role_content_name_tools_passthrough_http_honesty.pypython3 tests/test_openai_passthrough.pypython3 tests/test_chat_stream_options_http_honesty.pyProduct gates only: Full unit + Semgrep (Strix ignored). Independent non-author APPROVE still required.