You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Close the remaining tools/response_format fail-open that fix(api): fail-closed role, content, and name on tools passthrough #586 left: empty, omitted, null, or non-list messages now fail closed before the passthrough early-return, with the same invalid_message as the orchestration path.
Shared helper (_require_chat_messages_array) keeps both paths on one non-empty-array rule so SDK tool-calling bodies cannot bill a completion with no prompt.
stream_chat now reads the same thread-local request_sampling knobs as chat (temperature, top_p, penalties, max_tokens) so route streaming cannot drop overrides.
This branch is the mergeable repair of #586 (28ef9d2) and therefore of #582/#583/#584/#585. Do not merge those heads — tools + empty/omitted messages still returned 200 on #586, and stream_chat still ignored top_p / penalties.
…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>
Tools/response_format still billed a completion when messages was [],
omitted, null, or a non-list. Require the same non-empty array as
_validate_messages before the early-return. stream_chat now reads the
same thread-local sampling knobs as chat so route streaming cannot drop
top_p or penalties.
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep Fugu/TRINITY/Conductor titles for paper-contract search while
recording APA 7th citations. Split the optional tool_calls index case
so omit-key is actually tested instead of the previous tautology.
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Unique range: 7a16460 (empty messages + shared stream_chat knobs) and 61f1887 (APA titles + real omit-key tool_calls index). Code-reviewer + live HTTP on this tip.
Buyer next action
Send a non-empty messages array on every /v1/chat/completions call, including tool-calling SDK bodies. Empty, omitted, null, or non-list messages now return 400 invalid_message before passthrough — the same rule as the orchestration path. Do not merge #582–#586 or #589; those heads still billed a completion with no prompt.
Do not merge #594 until Full unit and Semgrep are green on 61f1887 and an independent non-author APPROVE lands. This review is COMMENT, not APPROVE.
Unique commits — closed
_require_chat_messages_array (server.py) is shared by _validate_chat_message_content_and_name (before the PASSTHROUGH_TRIGGER_KEYS return) and _validate_messages.
Live HTTP: tools + [] / omitted / null / non-list, and response_format + [], all 400 invalid_message.
stream_chat now reads the same thread-local request_sampling knobs as chat (temperature, top_p, penalties, max_tokens).
61f1887 splits omit-key / index: null / 0 / 1 so the optional tool_calls index case is not a tautology. Fugu / TRINITY / Conductor titles stay in docs/architecture.md with APA 7th citations.
No unique-commit Critical or Important defects.
Residual fail-open on this tip (next slice, not a #594 bug)
These still sit after the passthrough return / inside proxy_completion and still return 200 chat.completion:
tools + omitted model — silent worker select
tools + stream=true — JSON body (proxy_completion forces stream=false)
Until that hoist lands: always send a pool model; omit stream or set false on tool-calling requests. Prefer a successor of this tip over #591/#593 (those are based on #586 and lack the empty-messages gate).
Checks
Full unit + Semgrep were still queued at review time. Strix is not a product gate. Independent non-author APPROVE remains required.
Unique empty-messages work is sound. Do not merge this head.
_require_chat_messages_array runs before the tools/response_format early-return and on the orchestration path. Live HTTP on this SHA: tools + [] / omitted / null / non-list, and response_format + [], all return 400 invalid_message. stream_chat now shares _effective_request_sampling (top_p / penalties / max_tokens). The tool_calls index tautology is gone — omit-key is actually posted.
Local scripts printed ok: test_message_role_content_name_tools_passthrough_http_honesty.py, test_model_client_request_sampling_thread_isolation.py, test_tool_calls_entry_keys_http_honesty.py.
Remaining fail-open at this SHA (live HTTP 200 chat.completion)
These still sit after the passthrough return, so SDK tool-calling bodies can still bill:
Body
Result at 61f1887
tools + stream: true
JSON 200 (proxy_completion forces stream=false)
tools + omitted / unknown model
200 silent pool pick
tools + stream_options.include_usage=true
200
tools + temperature=99 / top_p=2
200
Next action
Prefer #600 (6f35df4) — this tip plus the request-level hoist. Send a non-empty messages array and a pool model; omit stream (or set false) on tool-calling requests.
Do not merge #582–#594 or #589. #591 has the hoist but not the stream_chat knob share. #592 is missing the empty-messages helper. Independent non-author APPROVE + Full unit / Semgrep still required on #600.
CodeRabbit CLI 0.7.3: coderabbit auth login --agent needs a browser callback here; this review is manual + live HTTP.
The reason will be displayed to describe this comment to others. Learn more.
This shared gate is the right close for #586. Live HTTP on 61f1887: tools + [] / omitted / null / non-list → 400 invalid_message. Keep it. The remaining buyer hole is the early-return below, which still skips stream / model / stream_options / sampling range — that hoist is #600, not a change to this helper.
The reason will be displayed to describe this comment to others. Learn more.
Still fail-open at this SHA. Live probe: tools + stream: true / omitted model / stream_options.include_usage=true / temperature=99 all return 200 chat.completion because _normalize_chat_stream_flag, _validate_completions_model, _require_pool_model, and the sampling-range checks run only after this return.
Do not merge this head. Prefer #600, which hoists those checks before proxy_completion. Buyer next action on that tip: send a pool model and omit stream (or set false) on tool-calling bodies.
Unique empty-messages work is sound. Do not merge this head.
_require_chat_messages_array runs before the tools/response_format early-return. Live HTTP on this SHA: tools + [] / omitted / null / non-list, and response_format + [], all return 400 invalid_message. stream_chat shares _effective_request_sampling. The tool_calls index omit-key case is a real tautology fix.
Local scripts printed ok: test_message_role_content_name_tools_passthrough_http_honesty.py, test_model_client_request_sampling_thread_isolation.py, test_tool_calls_entry_keys_http_honesty.py.
Remaining fail-open at this SHA
Live HTTP still 200 chat.completion for tools + stream: true, omitted/unknown model, stream_options.include_usage=true, and out-of-range temperature / top_p.
Next action
Prefer #597 (dbfa0e5) as the honesty-stack tip (empty messages + stream/model/spend/batch-routing on the #592 substrate). Do not merge #582–#596, #594, #599, or #600 in parallel.
#600 (6f35df4) is a #594-lineage parallel that hoists stream/model/sampling and keeps the stream_chat knob share. #597 still drops those knobs on route streaming — cherry-pick _effective_request_sampling onto #597 rather than landing #600.
Buyer next action on #597: send a non-empty messages array of objects and a pool model; omit stream (or set false) on tool-calling requests; omit routing.channel=batch.
Independent non-author APPROVE + Full unit / Semgrep still required. This automation will not APPROVE.
CodeRabbit CLI 0.7.3: coderabbit auth login --agent needs a browser callback here; this review is manual + live HTTP.
Superseded by #597 and the later #601 → #607 line. #597 explicitly includes this PR’s empty-message/streaming honesty behavior while adding the broader spend/routing validation; the successor chain carries that forward on the newer landing substrate. Keeping #594 open would duplicate the same request-boundary ownership. No checks, reviews, or approvals transfer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Summary
messagesnow fail closed before the passthrough early-return, with the sameinvalid_messageas the orchestration path._require_chat_messages_array) keeps both paths on one non-empty-array rule so SDK tool-calling bodies cannot bill a completion with no prompt.stream_chatnow reads the same thread-localrequest_samplingknobs aschat(temperature,top_p, penalties,max_tokens) so route streaming cannot drop overrides.[]/ omitted / null / non-list, andresponse_format+[]. Isolation tests coverstream_chatknob application.This branch is the mergeable repair of #586 (
28ef9d2) and therefore of #582/#583/#584/#585. Do not merge those heads — tools + empty/omittedmessagesstill returned 200 on #586, andstream_chatstill ignoredtop_p/ penalties.Test plan
python3 tests/test_message_role_content_name_tools_passthrough_http_honesty.pypython3 tests/test_model_client_request_sampling_thread_isolation.pypython3 tests/test_true_streaming.pytest_streaming.pytest_security_hardening.pypython3 tests/test_message_weight_http_honesty.pytest_message_prefix_http_honesty.pytest_assistant_refusal_annotations_http_honesty.pypython3 tests/test_chat_developer_multimodal_content_http_honesty.pytest_chat_message_name_http_honesty.pytest_tool_calls_entry_keys_http_honesty.pypython3 tests/test_completions_sampling_knobs_http_honesty.pypython3 tests/test_api_contract.pytest_conventions.pytest_openai_passthrough.pytest_paper_contracts.pytest_self_check.pyProduct gates only: Full unit + Semgrep (Strix ignored). Independent non-author APPROVE still required.