Skip to content

fix(api): fail-closed model and stream on tools passthrough - #599

Closed
cursor[bot] wants to merge 14 commits into
mainfrom
cursor/bc-79bac5da-0b15-40dc-a475-f0c2a7fc5d4f-b79e
Closed

fix(api): fail-closed model and stream on tools passthrough#599
cursor[bot] wants to merge 14 commits into
mainfrom
cursor/bc-79bac5da-0b15-40dc-a475-f0c2a7fc5d4f-b79e

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Summary

#594-lineage parallel to draft #597 (the designated honesty-stack tip on the #592 substrate).

This head closes tools/response_format fail-open on empty messages, model, stream, stream_options, attribution, routing, and temperature/top_p range, and shares stream_chat request_sampling knobs. It does not include #592 unknown-null stream_options or #597's batch-routing / max_tokens spend hoist.

Buyer next action

Prefer #597 for review/merge of the honesty stack. On this head, send a pool model, a non-empty messages array, and omit stream (or set false) on tool-calling requests. Do not send stream_options.include_usage=true or a non-object attribution / routing.

Do not merge #582#596 or #594 in parallel. Do not merge this head in parallel with #597.

Test plan

  • python3 tests/test_chat_model_stream_tools_passthrough_http_honesty.py
  • python3 tests/test_chat_stream_options_tools_passthrough_http_honesty.py
  • python3 tests/test_message_role_content_name_tools_passthrough_http_honesty.py
  • python3 tests/test_openai_passthrough.py test_true_streaming.py test_chat_stream_options_http_honesty.py
  • CI Full unit + Semgrep

Independent non-author APPROVE still required. This automation will not APPROVE.

seonghobae and others added 13 commits August 16, 2026 21:18
…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>
Tools/response_format still silent-selected a worker when model was
omitted and returned JSON 200 for stream=true. Require a pool model and
reject stream=true before the early-return so SDK tool-calling bodies
cannot hide the deployment or lie about SSE. HTTP honesty tests cover
tools and response_format; named-model + stream=false still proxies.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Those validators sat after the tools/response_format early-return, so
SDK tool-calling bodies could request include_usage or smuggle a
non-object attribution/routing and still bill a completion. Run the
same named checks before proxy_completion. All-false stream_options
stays an omit-equivalent no-op.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@seonghobae
seonghobae self-requested a review August 16, 2026 15:34
Sampling-range checks sat after the tools/response_format early-return,
so temperature=9 and top_p=2 were forwarded and billed. Run the same
Completions range validators before proxy_completion.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Assessment

Do not merge this draft. It is a 145-file / +24k honesty-stack tip (7a576c2) that overlaps #591, #594, #596, and #597. Independent non-author APPROVE is still required even after checks go green.

What this head already does

Tools/response_format passthrough now fail-closes on:

  • non-empty messages
  • pool model and stream=true (invalid_stream until SSE passthrough ships)
  • stream_options, attribution, routing shape
  • in-range temperature / top_p
  • stream_chat reading the same thread-local sampling knobs as chat

Those are real buyer-honesty wins: an SDK tool-calling body can no longer silent-select a worker, request unused usage chunks, or bill a JSON completion when the client asked for SSE.

Residual that still bills a completion

These validators still sit after the early-return, so a tools body can still proxy and bill:

  • seed (chat does not apply seed; orchestration path is invalid_seed)
  • presence_penalty / frequency_penalty out of [-2, 2]
  • max_tokens / max_completion_tokens (0, non-int, over cap)
  • n > 1 (one choice returned after the buyer asked for several)
  • routing.channel=batch and routing.latency_tolerant=true (shape-accepted, then billed as sync proxy_completion)

stop, user, logprobs / top_logprobs, logit_bias remain on the same post-return path.

Next action

  1. Keep this PR draft. Do not land it in parallel with #597 / #591 / #594 / #596.
  2. Land one successor that hoists seed / penalties / max_tokens / n and rejects batch hints on the tools path (track 003 next slice).
  3. Close or supersede the older overlapping heads after that successor is the unique tip.
  4. Treat queued OpenCode / Strix / Noema / Full unit as in-flight, not as a merge blocker for this assessment.

Checks at review time: Full unit, Semgrep, CodeQL, Trivy, and supply-chain were still queued on 7a576c2.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 15:40
@seonghobae
seonghobae enabled auto-merge (squash) August 16, 2026 15:40
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment

Do not merge this head. 7a576c2 is a 145-file honesty-stack tip that overlaps #591, #594, #596, #597, #600, and #601. Independent non-author APPROVE is still required even after checks go green. This automation will not APPROVE or merge.

What this head already does

Tools/response_format passthrough now fail-closes on:

  • non-empty messages of objects
  • pool model and stream=true (invalid_stream until SSE passthrough ships)
  • stream_options, attribution, routing shape
  • in-range temperature / top_p
  • stream_chat reading the same thread-local sampling knobs as chat

Those are real buyer-honesty wins: an SDK tool-calling body can no longer silent-select a worker, request unused usage chunks, or bill a JSON completion when the client asked for SSE.

Residual that still bills a completion

These validators still sit after the early-return at server.py (proxy_completion is called immediately after temperature/top_p), so a tools body can still proxy and bill:

  • seed (chat does not apply seed; orchestration path is invalid_seed)
  • presence_penalty / frequency_penalty out of [-2, 2]
  • max_tokens / max_completion_tokens (0, non-int, over cap)
  • n > 1 (one choice returned after the buyer asked for several)
  • routing.channel=batch and routing.latency_tolerant=true (shape-accepted, then billed as sync proxy_completion)
  • stop, user, logprobs / top_logprobs, logit_bias

Landing vehicle

  1. Prefer #601 (18e6263) for review/merge of the honesty stack. It already hoists seed/stop/n/logprobs/penalties on the tools path.
  2. Draft #605 (109b9d2) is a #599-lineage successor for spend knobs + batch hints only. Do not land #599 and #605 in parallel, and do not land either in parallel with #601.
  3. Do not open another overlapping 140-file stack. Unique remaining slices belong on #601: omit-real null tool.function fields (e58cb2f / #603) and KV provider-host allowlist (a50911f / #598).
  4. Treat queued OpenCode / Strix / Noema / Full unit as in-flight, not as a merge blocker for this assessment.

Buyer next action

Review #601. On this head, send a pool model, a non-empty messages array, and omit stream (or set false) on tool-calling requests. Do not send seed, n>1, logprobs, stream_options.include_usage=true, or a non-object attribution / routing.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Copy link
Copy Markdown
Contributor

Superseded by #607 through the #601 honesty-stack lineage. #601 already subsumes the model/stream/request-knob fail-closed behavior, and #607 carries that current stack plus the KV-backed provider-host allowlist. Keeping #599 open would duplicate the same buyer-facing honesty/security surface on an older substrate. No checks, reviews, or approvals transfer.

@seonghobae seonghobae closed this Aug 17, 2026
auto-merge was automatically disabled August 17, 2026 13:36

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants