fix(api): land tools knobs, omit-real tools, and KV host allowlist - #608
Open
cursor[bot] wants to merge 16 commits into
Open
fix(api): land tools knobs, omit-real tools, and KV host allowlist#608cursor[bot] wants to merge 16 commits into
cursor[bot] wants to merge 16 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>
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>
Stop reading CONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTS in ModelClient._validate_provider. The allowlist lives in provider_egress.allowed_provider_hosts; env is bootstrap transport via seed_provider_egress_from_environ only. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
This was referenced Aug 16, 2026
8 tasks
seonghobae
marked this pull request as ready for review
August 16, 2026 15:59
seonghobae
enabled auto-merge (squash)
August 16, 2026 15:59
Author
|
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. |
This was referenced Aug 16, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Author
There was a problem hiding this comment.
Review (COMMENT only)
Unique slices on 5901b79 are sound: KV provider_egress.allowed_provider_hosts is request-time only, JSON-null tool.function fields are popped before proxy, and tools-path seed/stop/n/logprobs/batch fail closed. Do not treat this as a merge vehicle.
Buyer next action: do not merge #608 in parallel with the later landing tips. Prefer:
- #616 for allowlist + trusted-trace email
- #613 / #624 for tools-path
mode/include_orchestration_trace - #615 for streamed
top_p/ penalties - #606 / #620 for SSE tools proxy
- #621 for gateway Bearer tokens in the credential KV
Independent non-author APPROVE plus Full unit / Semgrep is still required on whichever tip you land. This automation will not approve or merge.
Residuals on this head (already owned elsewhere — do not re-implement here):
tools+mode=explode/include_orchestration_tracestill 200 because passthrough returns before_validate_mode.stream_chatstill dropstop_p/ penalties after the temperature fix.- Allowlist seed empty-check + write is not under
_runtime_lock;_parse_host_allowlisthas no fuzz target yet. - Process-local
InMemoryConfigStoreonly; empty KV means no extra hostname filter.
Next unique product gap after those landings is issue #568 (role reasoning_effort_profile + equal-budget ablation), not another honesty-stack or allowlist PR.
Sent by Cursor Automation: Fix Issues
| ) | ||
| # Explicit JSON null on trigger keys is omit-equivalent (SDK optional | ||
| # defaults) — do not force single-agent passthrough for null-only keys. | ||
| if any( |
Author
There was a problem hiding this comment.
| @@ -330,7 +368,7 @@ def stream_chat(self, agent: ModelAgent, messages: list[ChatMessage], temperatur | |||
| payload = { # pragma: no cover | |||
Author
There was a problem hiding this comment.
stream_chat now copies request temperature, but still omits top_p / presence_penalty / frequency_penalty that chat() applies at 295–300. A streamed invoice at top_p=0.1 silently uses the provider default.
Cherry-pick the #615 nucleus/penalty hoist. Do not open a third stream-sampling PR from this stack.
| This is the only allowed ``os.environ`` read for the host allowlist. | ||
| Request-time validation must call :func:`allowed_provider_hosts`. | ||
| """ | ||
| existing = get_runtime_config(PROVIDER_EGRESS_CATEGORY, ALLOWED_PROVIDER_HOSTS_KEY, None) |
Author
There was a problem hiding this comment.
Bootstrap seed is correct (env only when the KV key is empty). Residual: the empty-check and write are not under _runtime_lock, so concurrent main() + serve() can both observe empty and widen the allowlist.
Hold the lock across the check+write on the #616 allowlist tip. Add a Hypothesis/fuzz/targets.py seam for _parse_host_allowlist there — do not open another ALLOWED_PROVIDER_HOSTS PR.
seonghobae
enabled auto-merge (squash)
August 16, 2026 20:00
seonghobae
enabled auto-merge (squash)
August 17, 2026 07:04
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Landing vehicle for the tools-passthrough honesty stack plus the remaining request-time env allowlist deviation.
Superset of #601 (
18e6263) with two unique commits on this branch:e58cb2f: pop JSON-nulltools[].function.description/parameters/strictbeforeproxy_completionso SDK defaults are omit-real.a50911f:ModelClient._validate_providerreadsprovider_egress.allowed_provider_hostsfrom the process KV.CONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTSis bootstrap transport viaseed_provider_egress_from_environonly.Live HTTP on this tip (
5901b79):tools+ emptymessages→ 400invalid_messagetools+seed: 1/stop/n: 2→ 400 named errors (the fix(api): fail-closed empty messages and spend keys on tools passthrough #597 residual)tools+routing.channel=batch→ 400invalid_routingtools+ nulldescription/strict→ 200 and those keys are absent from the echoed provider bodyLocal:
test_chat_messages_array_tools_passthrough_http_honesty.py,test_passthrough_request_knobs_http_honesty.py,test_tool_description_parameters_null_noop_http_honesty.py,test_provider_host_allowlist_kv.py,test_security_hardening.py, paper/self-check/conventions/api-contract →ok.Do not merge #589, #592, #594, #596, #597, #598, #599, #600, #601, #603, or #605 in parallel. Prefer this head.
Buyer next action: send a non-empty
messagesarray of objects; omitstream(or setfalse) on tool-calling requests; always send a poolmodel; omitrouting.channel=batch,latency_tolerant=true,seed,stop,n>1, andlogprobswithtools. Leave unuseddescription/parameters/strictomitted or JSONnull. Seedprovider_egress.allowed_provider_hostsin the KV (or setCONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTSat process start so bootstrap can copy it once).APA 7th: OpenAI (2024); Bray (2017); Joint Task Force (2020) NIST SP 800-53 Rev. 5 SC-7; ISO/IEC 27001:2022 A.8.20.
Merge only after Full unit + Semgrep are green and an independent non-author review lands. This automation does not approve or merge.
Test plan
python3 tests/test_chat_messages_array_tools_passthrough_http_honesty.pypython3 tests/test_passthrough_request_knobs_http_honesty.pypython3 tests/test_tool_description_parameters_null_noop_http_honesty.pypython3 tests/test_provider_host_allowlist_kv.pypython3 tests/test_security_hardening.pypython3 tests/test_paper_contracts.pytest_self_check.pytest_conventions.pytest_api_contract.pyDocs
CHANGELOG.md[Unreleased] records the fail-closed knobs, omit-real tools, and KV allowlist with buyer next actions.docs/rest_api_design.md, README,docs/architecture.md,docs/kv-credentials.md, anddocs/papers/README.mdcarry the same contract plus APA 7th citations.