fix(api): fail-closed whitespace-only mode on tools passthrough - #640
fix(api): fail-closed whitespace-only mode on tools passthrough#640cursor[bot] wants to merge 29 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>
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>
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>
Hoist attribution and routing validation before proxy_completion so a tools or response_format body cannot bill a sync completion with an unknown spend dimension or a batch/latency_tolerant hint. Tools passthrough has no batch job plane. Buyer next action: send known sync attribution; omit routing.channel=batch and latency_tolerant=true on tool-calling requests. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call the orchestration message, max_tokens, attribution, and routing validators before proxy_completion so a tools or response_format body cannot bill a completion with no prompt, crash on a non-object entry, or silently drop unknown spend/routing keys. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Match _validate_messages: tools + user content 123 must 400 invalid_message instead of billing a sync completion. Assistant/tool JSON null stays omit-equivalent. Buyer next action: send user/system content as a non-empty string or a content-parts array. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Match the tools-path honesty contract to the invalid_message behavior so SDK clients send a string or content-parts array. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Passthrough has no batch job plane. Reject routing.channel=batch and latency_tolerant=true before proxy_completion so a tool-calling body cannot bill a silent sync completion. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist the remaining chat request knobs before proxy_completion so an OpenAI SDK tool-calling body cannot bill a sync completion for seed, stop, n>1, logprobs, logit_bias, out-of-range token/penalty values, unsupported reasoning_effort, or a non-default service_tier. Buyer next action: omit those fields on tool-calling requests; they are not applied on this gateway. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call _validate_messages before proxy_completion so tools + [] / omitted / null / non-list / non-object messages raise invalid_message instead of billing a completion with no prompt. Buyer next action: always send a non-empty messages array of objects on tool-calling requests. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist the orchestration request-knob validators before proxy_completion so a tools or response_format body cannot bill a completion that silently drops seed, stop, n>1, logprobs, logit_bias, or out-of-range penalties. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat defaulted to 0.2 even after the HTTP path wrote default_temperature from the request. A streamed invoice summary at temperature=0.8 no longer silently falls back to 0.2. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
… proxy SDK optional defaults serialize omitted fields as JSON null. Accepting those keys without popping them is not omit-equivalent: proxy_completion forwards the body and several providers reject a null JSON Schema object. Pop the keys in place, keep non-null wrong types on invalid_tools, and assert omit-real via mock echo on chat and Responses. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
OpenAI SDK tool callers send stream=true. Returning 400 was honest but left every streaming tool client failing. Pipe a single pool agent as chat.completion.chunk frames so concatenated mock content matches the non-stream JSON body, and pipe live provider text so tool_calls deltas survive. Keep include_usage, empty messages, and unsupported knobs fail-closed. Buyer next action: send stream=true on tool-calling requests; omit stream_options.include_usage. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep tools/response_format SSE proxy from this branch and take #601's null tool.function omit-pop, request-knob HTTP cases, and streamed route temperature. Conflict resolution keeps stream=true as SSE, not 400. CHANGELOG records the combined unreleased slice. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Unknown assistant tool_calls entry/function fields, non-boolean include_orchestration_trace, and unknown mode used to bill a JSON or SSE completion on the tools proxy. They now return the same named 400s as the orchestration path before the first byte. Buyer next action: send only id/type/function/optional index on assistant tool_calls; send a boolean or omit include_orchestration_trace; send auto/route/conduct or omit mode. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
mock:// lookup_balance now returns assistant tool_calls and streams delta.tool_calls with finish_reason=tool_calls so offline SDK clients match the live _stream_raw path. Invoice identifiers bind from the user text (default INV-9). Buyer next action: send stream=true on tool-calling requests and include the invoice id in the user message. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat recorded temperature but omitted the request-scoped top_p, presence_penalty, and frequency_penalty that chat() already forwards. A streamed invoice summary at top_p=0.1 no longer drops those knobs. Buyer next action: send the nucleus and penalty values you want on stream=true route requests; streaming matches the JSON path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Tools / response_format proxy is the route hop. mode=conduct and include_orchestration_trace=true billed a silent chat.completion with no Conductor workflow and no TRINITY trace. Reject them with the same named errors as orchestration, including mixed orchestration=route + mode=conduct and the SSE tools path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The #617 parent test still required billed success for include_orchestration_trace=true. Flip that case and add HTTP coverage for orchestration=conduct, SSE+trace=true, and response_format+conduct. Whitespace-only mode is invalid_mode like the orchestration or-chain, while per-key checks keep mixed route+conduct from hiding conduct. 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. |
There was a problem hiding this comment.
Verdict
Unique d898045 is the mode/trace + SSE landing. Per-key alias checks were already on #624; this commit fail-closes whitespace-only mode and flips the #617 parent test that still required a billed success for include_orchestration_trace=true. This is a COMMENT, not an approval.
Buyer next action
On /v1/chat/completions with tools or response_format, omit mode or send auto/route. Do not send mode as spaces, mode=conduct, or include_orchestration_trace=true. Those return invalid_mode / invalid_include_orchestration_trace instead of a billed chat.completion.
Independent reviewer (not this automation): APPROVE after Full unit and Semgrep are green. Land this head rather than #624, #629, or #613. Do not merge #610 / #613 / #624 / #629 / #647 in parallel with this tip. #647 remains the #613-stack repair only.
What was checked
raw_mode is None or raw_mode == ""omit; whitespace-onlymodereaches_validate_modeand isinvalid_mode.- Per-key loop over
orchestration/orchestration_mode/modeis unchanged and still blocks mixedroute+conduct. - New live HTTP locks:
orchestration=conduct,orchestration_mode=conduct, whitespace-onlymode, SSE+trace=true,response_format+conduct,mode=""omit. - Parent
test_http_tools_accepts_include_orchestration_trace_boolean_and_nullno longer treatsTrueas a billed no-op.
Do not self-approve. Do not open a fourth mode/trace PR.
Sent by Cursor Automation: Fix Issues
There was a problem hiding this comment.
Verdict: COMMENT — unique slice is sound; do not treat this as merge-to-main
#640 at d898045 closes the two remaining tools-passthrough honesty holes on the #617/#624 SSE tip:
- Whitespace-only
mode(" ") is nowinvalid_modeinstead of a billedchat.completion. JSON""/nullstay omit-equivalent. - Per-key checks on
orchestration/orchestration_mode/modestay, so mixedorchestration=route+mode=conductcannot hide conduct (unlike #629'sor-chain collapse). - The #617 parent test that required billed success for
include_orchestration_trace=trueis flipped to 400.
Buyer next action: on /v1/chat/completions with tools or response_format, omit mode or send auto/route. Do not send mode as spaces, mode=conduct, or include_orchestration_trace=true.
Why this is not APPROVE
- This PR vs
mainis the 155-file honesty stack (29 commits). The unique tip is 8 files / +143. Approving the unique is not a merge signal for the stack. - Full unit / Semgrep were still queued at review time. Checks wait is not a blocker for this assessment; it is also not a green merge ticket.
- Leftover
_validate_request_mode_if_presentis unused on the live path but still implements the old holes (strip-omit whitespace, acceptconduct, hidemode=conductbehindorchestration=route). A one-line rewire would reopen billed completions. See the inline comment. - Independent non-author APPROVE is still required. This automation does not self-approve and has no merge tool.
Landing map
- Prefer #640 over #624 and #629 for the mode/trace slice on the SSE tip.
- #647 (
797e879) is the same per-key repair on the #613 stack (no SSE substrate). Do not merge #647 in parallel with this head. - Do not merge #610 / #613 / #624 / #629 in parallel.
- Follow-up unique on this tip: delete the leftover helper and HTTP-lock alias-key / SSE whitespace. That work is on
cursor/bc-43af877f-9636-4eff-bd98-f23a7d1dc3fa-d062(70265cf). Prefer that successor over this head once it is open. - Parallel KV/token landing remains #621. Message-name hoist remains #623. Responses SSE landing remains #648. Do not open a third token, message-name, or responses-SSE PR.
CodeRabbit CLI auth timed out in this environment (awaiting_browser_auth). Review is from the unique diff plus live HTTP validators (test_passthrough_mode_trace_http_honesty, test_passthrough_trace_mode_http_honesty, paper/self-check/conventions/api/product-planning contracts).
Sent by Cursor Automation: Fix Issues
| passthrough path so an unknown mode cannot bill a silent completion. | ||
| """ | ||
| raw_mode = body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") | ||
| if raw_mode is None or (isinstance(raw_mode, str) and not raw_mode.strip()): |
There was a problem hiding this comment.
This leftover helper is unused on the live tools path, but it still strip-omits whitespace, accepts conduct, and hides mode=conduct behind orchestration=route. Its docstring still claims it is the passthrough validator. _validate_chat_passthrough_orchestration_controls is the real gate.
Delete this helper (do not rewire passthrough back to it) so a later cleanup cannot reopen billed completions. The follow-up unique on cursor/bc-43af877f-9636-4eff-bd98-f23a7d1dc3fa-d062 (70265cf) removes it and HTTP-locks alias-key spaces, mixed route+spaces, and SSE mode as spaces.
|
Superseded by tip substrate ≥ #691 (cumulative OpenAI/gateway honesty band + auto-merge tip). Closing to free product-gate runners (Full unit + Semgrep). |


Buyer next action
On
/v1/chat/completionswithtoolsorresponse_format, omitmodeor sendauto/route. Do not sendmodeas spaces,mode=conduct, orinclude_orchestration_trace=true. Those now returninvalid_mode/invalid_include_orchestration_traceinstead of a billedchat.completion.Why this PR exists
#624 at
13ced1ealready fails closed onconductandinclude_orchestration_trace=true, and checks each oforchestration/orchestration_mode/modeso mixedorchestration=route+mode=conductcannot hide conduct. Two honesty holes remained on that tip:test_http_tools_accepts_include_orchestration_trace_boolean_and_nullstill required billed success forTrue. Full unit on fix(api): fail-closed conduct and trusted-trace on tools passthrough #624 is red.mode: " "still billed achat.completion. The orchestrationorchain treats whitespace-only mode as truthyinvalid_mode.This unique commit stays on the #617 SSE tip and keeps the per-key checks. It does not collapse aliases back into the
orchain (#629 on #613 would hidemode=conductbehindorchestration=route). JSON""stays omit-equivalent.Prefer this head over #624 and #629 for the mode/trace slice. Do not merge #610 / #613 / #624 / #629 in parallel. Independent non-author APPROVE + Full unit/Semgrep required. Do not self-approve.
Test plan
python3 tests/test_passthrough_trace_mode_http_honesty.pypython3 tests/test_passthrough_mode_trace_http_honesty.pypython3 tests/test_paper_contracts.pytest_self_check.pytest_conventions.pytest_api_contract.pytest_product_planning_contract.pyDocs
modeas spaces.modeasinvalid_mode.