fix(api): check each mode alias on tools passthrough - #647
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>
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>
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 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>
Hoist mode/orchestration and include_orchestration_trace before proxy_completion. tools + mode=explode and include_orchestration_trace="yes" were 200 on #601. mode=conduct and include_orchestration_trace=true also fail closed — this path has no Conductor workflow or trusted-trace plane. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reuse the orchestration or-chain so tools + mode=" " is invalid_mode instead of a billed omit. Add live HTTP cases for mode=route, orchestration_mode=explode, and response_format+conduct. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
#629's or-chain hid mode=conduct behind orchestration=route and billed a chat.completion. Keep whitespace-only mode as invalid_mode, but validate orchestration / orchestration_mode / mode independently so a later conduct or spaces cannot hide behind an earlier route. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Doctor the Conductor/TRINITY passthrough note so mixed orchestration=route plus mode=conduct cannot hide a workflow the buyer asked for. 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. |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Verdict: SOUND
Reviewed only the unique after #629 (e3bffe0): 4f521b4 and 797e879. Did not re-review the #613 stack.
The #629 or chain is gone on tools / response_format passthrough. Each of orchestration / orchestration_mode / mode is validated independently. Mixed route+conduct no longer bills a chat.completion. JSON null and "" stay omit-equivalent. Whitespace-only mode stays invalid_mode. mode=conduct still fail-closes because this path has no Conductor workflow.
Not an approval. Independent non-author APPROVE + Full unit + Semgrep are still required. Do not merge #601 / #613 / #629 in parallel with this head. If the SSE-tip stack is the one that lands, prefer #640.
1. Mixed-alias hide — actually fixed on the claimed path
_validate_chat_passthrough_orchestration_controls (server.py:1730-1743) now loops the three keys. Skip is explicit (None or ""), not truthiness, so " " still reaches _validate_mode and 400s. conduct on any key raises invalid_mode even when an earlier key is route.
Live HTTP in the unique plus a local probe:
| Body | Result |
|---|---|
tools + orchestration=route + mode=conduct |
400 invalid_mode, no choices |
tools + orchestration=route + mode=" " |
400 invalid_mode |
tools + orchestration_mode=route + mode=conduct |
400 (probe; not in committed suite) |
tools + orchestration=conduct + mode=route |
400 (probe) |
response_format + mixed route/conduct |
400 (probe; same early-return) |
tools + mode="" |
200 / choices |
tools + mode=null |
200 / choices (probe; not in committed suite) |
tools + all three aliases null |
200 / choices (probe) |
orchestrator.py is not on this unique. proxy_completion is transport-only and does not re-interpret mode. The fail-closed gate is the server early-return before that hop (server.py:3928).
2. File:line findings
Warning (residual, out of unique scope) — contextual_orchestrator/server.py:3945
The non-tools orchestration path still uses body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") or "auto". Probe: no tools, orchestration=route + mode=conduct → 200 / choices (first truthy wins). That is the same hide class, but on a path that can run conduct, so it is precedence rather than “bill a proxy for an unsupported workflow.” Docs for this unique correctly scope the per-key rule to tools passthrough. Do not expand this unique to fix it.
Info — tests/test_passthrough_mode_trace_http_honesty.py:346
Committed suite covers mode="" but not mode=null. The skip at server.py:1734-1735 treats both as omit; probe confirms 200. Add the null case if you want the advertised omit pair locked in CI.
Info — tests/test_passthrough_mode_trace_http_honesty.py:233
Only one mixed pair is committed (orchestration + mode). orchestration_mode=route + mode=conduct and response_format + mixed aliases work in probe but are not buyer-facing CI locks.
Info — CHANGELOG.md:21-23
Still says “same truthy rule as the orchestration or chain” after the unique stopped using that chain. Behavior for strings matches (null/"" omit; spaces invalid). The new skip is is None or == "", which is stricter for falsy non-strings (0 / false → 400). Wording is leftover, not a buyer-facing lie — the next sentence states per-key checks.
No Critical findings on the unique.
3. Test gaps
Covered well: mixed route+conduct, mixed route+spaces, mode="", mode=conduct, invalid aliases, mode=auto/route, __main__ runner updated.
Missing (non-blocking):
- Live HTTP
mode=null(claimed omit-equivalent; works; untested). orchestration_modemixed withconduct.response_formatmixed aliases (same function, no dedicated case).- README Check list still omits this honesty file (pre-existing on the stack).
fuzz/targets.pystill drives_validate_modeonly; it does not exercise the per-key helper. Not a new seam.
Local scripts printed ok: test_passthrough_mode_trace_http_honesty, test_chat_orchestration_mode_http_honesty, test_chat_include_orchestration_trace_http_honesty, test_include_orchestration_trace_null_noop_http_honesty, test_paper_contracts, test_self_check, test_conventions, test_api_contract, test_product_planning_contract.
CI Full unit / Semgrep were still queued at review time.
4. Regression vs omit-equivalent null/empty
No regression found. None and "" continue; whitespace does not. Probe: mode=null 200; all-three-null 200; orchestration="" + mode=conduct still 400 (empty does not hide conduct). include_orchestration_trace null/empty-strip omit is unchanged.
5. Docs / CHANGELOG / paper contracts
Match the unique:
- CHANGELOG records mixed
orchestration=routeplusmode=conductand per-key checks. - README,
docs/architecture.md,docs/rest_api_design.mdnext-action tell buyers not to mix aliases. docs/papers/README.md(797e879) records the Conductor/TRINITY grounding for the hide.
tests/test_paper_contracts.py is still the Fugu/TRINITY/Conductor runtime trio. It does not assert the new papers sentence. That is fine: the executable contract for this unique is the live HTTP file, not the paper-behavior tests.
docs/rest_api_design.md:124-129 (contract paragraph) still lists invalid mode / mode=conduct without saying “per-key.” The next-action paragraph does. Not a contradiction.
6. Buyer next action if this merges
On /v1/chat/completions with tools or response_format:
- Omit
mode/orchestration/orchestration_mode, or sendautoorroute. - Do not send
mode=conductor mixedorchestration=routeplusmode=conduct— those are nowinvalid_mode400, not a billed proxy. - Do not send whitespace-only
mode. - JSON
nullor""on those keys is safe (omit). - Omit
include_orchestration_traceor set itfalse.
Wait for independent non-author APPROVE and green Full unit + Semgrep before merge. Prefer this unique over #629 on the #613 stack. If you are taking the SSE-tip landing, take #640 instead and do not merge this stack in parallel.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
Unique 4f521b4 + 797e879 is correct. 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 mixed orchestration=route plus mode=conduct. Those return invalid_mode instead of a billed chat.completion.
Independent reviewer (not this automation): APPROVE after Full unit and Semgrep are green. Do not merge #601, #613, or #629 in parallel with this head. If the SSE tip is the merge vehicle, land #640 instead and treat this as the #613-stack repair only.
What was checked
- Per-key loop over
orchestration/orchestration_mode/modein_validate_chat_passthrough_orchestration_controls(server.py:1730). - JSON
null/""omit; whitespace-only andconductfail closed on each key. - Live HTTP tests bind the #629 hide: mixed
route+conduct→ 400; mixedroute+spaces → 400;mode=""→ 200/choices. - Local scripts:
test_passthrough_mode_trace_http_honesty.py,test_chat_orchestration_mode_http_honesty.py,test_paper_contracts.pyprinted ok. - CodeRabbit CLI 0.7.3 is installed here;
coderabbit auth login --agenttimed out awaiting browser login. Not a merge blocker.
Non-blocking
The new docstring at server.py:1727 still says whitespace is “truthy on the orchestration or chain.” Semantics are right; reword on a later edit so that phrase does not invite the chain back.
Do not add another honesty unique onto this branch. Do not self-approve.
Sent by Cursor Automation: Fix Issues
There was a problem hiding this comment.
Verdict
Unique 4f521b4 + 797e879 is SOUND on the claimed path. This is a COMMENT, not an approval.
Buyer next action
On /v1/chat/completions with tools or response_format, omit mode / orchestration / orchestration_mode or send auto/route. Do not send mode as spaces, mode=conduct, or mixed orchestration=route plus mode=conduct. Those now return invalid_mode instead of a billed chat.completion. JSON null and "" stay omit-equivalent.
Independent reviewer (not this automation): APPROVE after Full unit and Semgrep are green. Do not merge #601, #613, or #629 in parallel with this head. If the SSE tip is the merge vehicle, land #640 instead and treat this as the #613-stack repair only.
What was checked
- Per-key loop in
_validate_chat_passthrough_orchestration_controls(server.py:1730). Skip isNoneor"", so whitespace still reaches_validate_mode. - Live HTTP unique: mixed
route+conduct→ 400; mixedroute+spaces → 400;mode=""→ 200/choices. - Local scripts printed
ok:test_passthrough_mode_trace_http_honesty,test_chat_orchestration_mode_http_honesty,test_chat_include_orchestration_trace_http_honesty,test_include_orchestration_trace_null_noop_http_honesty,test_paper_contracts,test_self_check,test_conventions,test_api_contract,test_product_planning_contract. - CodeRabbit CLI 0.7.3 is installed here;
coderabbit auth login --agenttimed out awaiting browser login. Not a merge blocker.
Residual (out of this unique)
The non-tools orchestration path still uses body.get("orchestration") or body.get("orchestration_mode") or body.get("mode") or "auto" (server.py:3945 on this head; same or chain on #640). A no-tools body with orchestration=route plus mode=conduct still bills a route chat.completion. Do not expand this unique to fix it. That hide is a separate successor from main.
Do not add another honesty unique onto this branch. Do not self-approve.
Sent by Cursor Automation: Fix Issues
Pull request was closed
|
Superseded by tip substrate ≥ #691 (cumulative OpenAI/gateway honesty band + auto-merge tip). Closing to free product-gate runners (Full unit + Semgrep). |
2 participants
Buyer next action
On
/v1/chat/completionswithtoolsorresponse_format, omitmodeor sendauto/route. Do not sendmodeas spaces,mode=conduct, or mixedorchestration=routeplusmode=conduct. Those now returninvalid_modeinstead of a billedchat.completion.Why this PR exists
Successor to #629 at
e3bffe0. Unique commit repairs the #629orchain.#629 reused the orchestration
orchain so whitespace-onlymodeisinvalid_mode. That also made the first truthy alias win:orchestration=routeplusmode=conductbilled achat.completion(live HTTP RED on #629). A buyer who asked for a Conductor workflow must not receive a silent Fugu-style single-agent proxy (Nielsen et al., 2025; Xu et al., 2025).orchestration/orchestration_mode/modeon its own.nulland""stay omit-equivalent; whitespace-only mode staysinvalid_mode.route+conduct→ 400; mixedroute+spaces → 400;mode=""→ 200/choices.Prefer this unique over #629 on the #613 stack. #640 remains the mode/trace landing if the SSE tip is the one that merges. Do not merge #601, #613, or #629 in parallel with this head.
Independent non-author APPROVE + Full unit + Semgrep required. Do not self-approve.
Test plan
python3 tests/test_passthrough_mode_trace_http_honesty.pypython3 tests/test_chat_orchestration_mode_http_honesty.pypython3 tests/test_chat_include_orchestration_trace_http_honesty.pypython3 tests/test_include_orchestration_trace_null_noop_http_honesty.pypython3 tests/test_paper_contracts.pytest_self_check.pytest_conventions.pytest_api_contract.pytest_product_planning_contract.pyDocs
docs/rest_api_design.md,docs/architecture.mdrecord per-key alias checks and the mixed-route hide.References