From 9ce2eeb23989df4f95b8f57d229f442d40ae8e0c Mon Sep 17 00:00:00 2001 From: Krisztian Gajdar Date: Wed, 14 Oct 2026 19:14:14 +0200 Subject: [PATCH 1/2] fix(api): check tool names on every chat route and report an upstream refusal as a 400 Single-node chat that forwards to an upstream chat endpoint now applies the tool checks the queued route and the locally rendered path already apply: an undeclared function, a call under tool_choice none, a call to a function other than the named one, several calls under parallel_tool_calls false, and no call under required or a named function. Streamed and buffered answers are checked before the violating call is delivered. The gateway and the single-node chat validation refuse a tool name outside ^[a-zA-Z0-9_-]{1,64}$ with a 400 naming tools[i].function.name, so local and remote profiles accept the same requests. A streamed generation call forwards the caller's request, so an upstream 400 or 422 without a known SIE error code is now the caller's 400 invalid_request instead of a server error. Encode, score and extraction build their upstream bodies themselves and keep the server error. --- packages/sie_gateway/src/handlers/proxy.rs | 61 +++++++ packages/sie_server/REMOTE_BACKENDS.md | 24 +++ .../src/sie_server/adapters/remote/_http.py | 28 +++- .../src/sie_server/api/openai_local.py | 23 ++- .../sie_server/tests/api/test_remote_chat.py | 149 ++++++++++++++++-- .../tests/processors/test_remote_chat.py | 23 ++- 6 files changed, 286 insertions(+), 22 deletions(-) diff --git a/packages/sie_gateway/src/handlers/proxy.rs b/packages/sie_gateway/src/handlers/proxy.rs index 3dee42e15..663817258 100644 --- a/packages/sie_gateway/src/handlers/proxy.rs +++ b/packages/sie_gateway/src/handlers/proxy.rs @@ -7949,6 +7949,8 @@ fn chat_params_from_json(body: &serde_json::Value) -> ChatParamsResult { /// * Must be a non-empty JSON array. /// * Each entry must be ``{type: "function", function: {name: string, /// description?: string, parameters?: object}}``. +/// * ``function.name`` must match the OpenAI pattern +/// ``^[a-zA-Z0-9_-]{1,64}$``. /// * ``function.parameters`` (when present) is run through the shared /// JSON-Schema safety walker so the same depth / size / unsupported- /// keyword caps that apply to ``response_format.json_schema`` also @@ -8033,6 +8035,17 @@ fn validate_chat_tools(v: &serde_json::Value) -> Result, oai_code::INVALID_REQUEST, )); } + if name.len() > 64 + || !name + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-') + { + return Err(bad( + format!("tools[{i}].function.name must match ^[a-zA-Z0-9_-]{{1,64}}$"), + &format!("tools[{i}].function.name"), + oai_code::INVALID_REQUEST, + )); + } if let Some(desc) = func.get("description") { if !desc.is_string() && !desc.is_null() { return Err(bad( @@ -18227,6 +18240,54 @@ mod tests { } } + #[tokio::test] + async fn chat_tool_names_outside_the_openai_pattern_are_refused_before_routing() { + let chat = |name: &str| { + serde_json::json!({ + "model": "org/g", + "messages": [{"role": "user", "content": "hello"}], + "max_tokens": 4, + "tools": [ + {"type": "function", "function": {"name": "get_weather"}}, + {"type": "function", "function": {"name": name}} + ] + }) + }; + let too_long = "a".repeat(65); + let longest = "A".repeat(64); + for (name, valid) in [ + ("math.factorial", false), + ("two words", false), + ("caf\u{e9}", false), + ("a/b", false), + (too_long.as_str(), false), + ("get-weather_2", true), + (longest.as_str(), true), + ] { + let (state, policy) = recording_generation_state().await; + let response = proxy_chat( + State(state), + json_request("/v1/chat/completions", chat(name)), + ) + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST, "{name}"); + let body = axum::body::to_bytes(response.into_body(), 64 * 1024) + .await + .expect("read body"); + let error: serde_json::Value = serde_json::from_slice(&body).expect("body is JSON"); + assert!(policy.take_calls().is_empty(), "{name} reached routing"); + if valid { + // The fixture model declares no tool support, so a request + // that passed validation is refused by the capability gate. + assert_eq!(error["error"]["code"], "unsupported_field", "{name}"); + assert_eq!(error["error"]["param"], "tools", "{name}"); + } else { + assert_eq!(error["error"]["code"], "invalid_request", "{name}"); + assert_eq!(error["error"]["param"], "tools[1].function.name", "{name}"); + } + } + } + #[tokio::test] async fn governed_generation_dispatch_targets_are_exact_and_order_independent() { for reverse_insertion in [false, true] { diff --git a/packages/sie_server/REMOTE_BACKENDS.md b/packages/sie_server/REMOTE_BACKENDS.md index dcb63fb6c..526bcf57e 100644 --- a/packages/sie_server/REMOTE_BACKENDS.md +++ b/packages/sie_server/REMOTE_BACKENDS.md @@ -276,6 +276,30 @@ returns the original local refusal with its retry delay. A client error is never retried remotely, and fallback cannot replay work after local acceptance or after output reaches the caller. +Chat, completions and native generation forward the caller's own request. An +upstream that answers such a request with `400` or `422` and no known SIE error +code refused it, so the caller receives `400` `invalid_request`, as for an +`INVALID_INPUT` refusal, with fixed text that repeats nothing the upstream sent +and does not name the upstream. Encode, score and extraction build every +upstream body themselves, so the same answer there is a server error. A `400` +or `422` with a known SIE error code other than `INVALID_INPUT` or +`INPUT_TOO_LONG`, and other upstream `4xx` answers such as `401`, `403` or +`404`, point at the operator's credential, model name or route rather than at +the request, so the caller receives a server error. `429` and `5xx` answers are +retryable as described above. + +Chat checks an upstream answer against the request's tools on a single node +and in a cluster, streamed and buffered. The answer fails as an upstream error +when it calls a function the request did not declare, calls any function under +`tool_choice: "none"`, calls a function other than the one `tool_choice` names, +makes several calls under `parallel_tool_calls: false`, or makes no call when +`tool_choice` is `"required"` or names a function. A stream that has already +started ends with an error event. Tool function names must +match `^[a-zA-Z0-9_-]{1,64}$`, the pattern OpenAI-compatible APIs accept, so a +local and a remote profile accept the same requests. The gateway and the +single-node chat route refuse any other name with `400` before dispatch and +name the field, for example `tools[1].function.name`. + TLS is required outside loopback. URLs containing credentials, a query or a fragment are rejected. Redirects are refused, ambient proxy variables are ignored, and inbound authorization is not forwarded. An explicit `proxy_url` diff --git a/packages/sie_server/src/sie_server/adapters/remote/_http.py b/packages/sie_server/src/sie_server/adapters/remote/_http.py index d6df5cf02..27cc59085 100644 --- a/packages/sie_server/src/sie_server/adapters/remote/_http.py +++ b/packages/sie_server/src/sie_server/adapters/remote/_http.py @@ -14,7 +14,9 @@ connection, a timeout, a dropped response, another 5xx). A 501, a 505 and the codes ``MODEL_LOAD_FAILED``, ``ACCOUNT_STATE_UNAVAILABLE`` and ``COLD_START_RATE_LIMITED`` are final. A refused input raises -:class:`InputTooLongError` or :class:`InvalidInputError`. Everything else is a +:class:`InputTooLongError` or :class:`InvalidInputError`. A streamed +generation call forwards the caller's request, so there a 400 or 422 without a +known code is a refused input too. Everything else is a :class:`RemoteUpstreamError`. """ @@ -126,9 +128,16 @@ def _send(client: httpx.Client, request: httpx.Request, *, upstream: str, max_by response.close() -def failure_for_status(status: int, code: str | None, retry_after_s: int | None, *, upstream: str) -> Exception: - """The failure an upstream error status amounts to, given its known error code and wait.""" - answered = f"answered {status} {code}" if code else f"answered {status}" +def failure_for_status( + status: int, code: str | None, retry_after_s: int | None, *, upstream: str, caller_request: bool = False +) -> Exception: + """The failure an upstream error status amounts to, given its known error code and wait. + + ``caller_request`` says the call forwarded the caller's own request, so a + 400 or 422 without a known code refused that request. + """ + reply = f"{status} {code}" if code else str(status) + answered = f"answered {reply}" wait = retry_after_s if retry_after_s is not None else DEFAULT_RETRY_AFTER_S if code in _TERMINAL_CODES or status in _TERMINAL_SERVER_STATUSES: return RemoteUpstreamError(f"upstream {answered}") @@ -142,8 +151,8 @@ def failure_for_status(status: int, code: str | None, retry_after_s: int | None, return UpstreamUnavailableError(upstream, "unavailable", retry_after_s=wait, reason=answered) if code == "INPUT_TOO_LONG": return InputTooLongError(f"the upstream refused the input as too long ({status} INPUT_TOO_LONG)") - if code == "INVALID_INPUT" and status in {400, 422}: - return InvalidInputError(f"the upstream refused the input ({status} INVALID_INPUT)") + if status in {400, 422} and (code == "INVALID_INPUT" or (caller_request and code is None)): + return InvalidInputError(f"the upstream refused the input ({reply})") return RemoteUpstreamError(f"upstream {answered}") @@ -173,8 +182,10 @@ async def open_stream( The call goes through the upstream's limiter, whose slot is held until the block ends. The answer is a success with an uncompressed body, which the block reads. Failures raise as :func:`send_bounded` describes, including a - connection lost while the block reads. Leaving the block closes the answer, - which cancels work still running upstream. + connection lost while the block reads. The request forwards the caller's + own, so a 400 or 422 without a known code also raises + :class:`InvalidInputError`. Leaving the block closes the answer, which + cancels work still running upstream. """ upstream_for_serving(upstream) with upstream_limiter(upstream).call(): @@ -204,6 +215,7 @@ async def open_stream( ), parse_retry_after(response.headers.get("retry-after")), upstream=upstream, + caller_request=True, ) if _is_compressed(response): raise RemoteUpstreamError("upstream sent a compressed body, which is refused") diff --git a/packages/sie_server/src/sie_server/api/openai_local.py b/packages/sie_server/src/sie_server/api/openai_local.py index f708fdf1f..5ce9b6d3c 100644 --- a/packages/sie_server/src/sie_server/api/openai_local.py +++ b/packages/sie_server/src/sie_server/api/openai_local.py @@ -31,6 +31,7 @@ import logging import math import os +import re from collections.abc import AsyncIterator from pathlib import Path from typing import Annotated, Any, cast @@ -89,6 +90,7 @@ sniff_video_container, ) from sie_server.observability.tracing import tracer +from sie_server.processors.remote_chat import _check_tools from sie_server.processors.remote_chat_prompt import collect_rendered_chat, prepare_rendered_chat from sie_server.processors.schema_prompt import ( json_schema_response_format, @@ -126,6 +128,7 @@ # fence for malformed/non-token responses. _MAX_CHAT_RESPONSE_BYTES = int(os.environ.get("SIE_CHAT_MAX_RESPONSE_BYTES", str(32 * 1024 * 1024))) _MAX_CHAT_TOOLS = 64 +_TOOL_NAME = re.compile(r"[a-zA-Z0-9_-]{1,64}") _MAX_CHAT_MESSAGES = 4096 _MAX_CHAT_CHOICES = 128 _MAX_CHAT_VIDEOS = 1 @@ -591,13 +594,18 @@ def _validate_chat_tools(body: dict[str, Any]) -> None: if tools is not None: if not isinstance(tools, list) or len(tools) > _MAX_CHAT_TOOLS: raise _bad_request("'tools' must be an array of at most 64 functions", param="tools") - for tool in tools: + for index, tool in enumerate(cast("list[Any]", tools)): function = tool.get("function") if isinstance(tool, dict) else None if not isinstance(function, dict) or tool.get("type") != "function": raise _bad_request("'tools' entries must declare a function object", param="tools") name = function.get("name") if not isinstance(name, str) or not name: raise _bad_request("tool function names must be non-empty strings", param="tools") + if not _TOOL_NAME.fullmatch(name): + raise _bad_request( + f"'tools[{index}].function.name' must match ^[a-zA-Z0-9_-]{{1,64}}$", + param=f"tools[{index}].function.name", + ) names.add(name) if function.get("description") is not None and not isinstance(function["description"], str): raise _bad_request("tool descriptions must be strings", param="tools") @@ -1241,6 +1249,16 @@ def _check_remote_chat_usage(payload: dict[str, Any], body: dict[str, Any], conf raise RemoteUpstreamError("upstream chat usage exceeded the configured token bounds") +def _check_remote_chat_tools(payload: dict[str, Any], body: dict[str, Any], calls: dict[int, dict[int, str]]) -> None: + for choice in payload["choices"]: + message = choice["delta"] if "delta" in choice else choice["message"] + names = calls.setdefault(choice["index"], {}) + for position, tool in enumerate(message.get("tool_calls") or []): + index = tool.get("index", position) + names[index] = names.get(index, "") + tool.get("function", {}).get("name", "") + _check_tools(names, body, finished=choice["finish_reason"] is not None) + + async def _remote_chat_events( iterator: AsyncIterator[dict[str, Any]], *, @@ -1254,12 +1272,14 @@ async def _remote_chat_events( states: dict[int, ThinkingBlockStripper] = {} strict_contents: dict[int, list[str]] = {} strict_tool_choices: set[int] = set() + calls: dict[int, dict[int, str]] = {} delivered = False received = 0 include_usage = bool((body.get("stream_options") or {}).get("include_usage")) try: async for payload in iterator: _check_remote_chat_usage(payload, body, config) + _check_remote_chat_tools(payload, body, calls) sanitized = _sanitize_chat_payload( payload, requested_model=requested_model, @@ -1362,6 +1382,7 @@ async def _remote_chat_response( ) ) _check_remote_chat_usage(payload, body, config) + _check_remote_chat_tools(payload, body, {}) payload = _sanitize_chat_payload( payload, requested_model=requested_model, diff --git a/packages/sie_server/tests/api/test_remote_chat.py b/packages/sie_server/tests/api/test_remote_chat.py index d36dd0d42..f6653ccdd 100644 --- a/packages/sie_server/tests/api/test_remote_chat.py +++ b/packages/sie_server/tests/api/test_remote_chat.py @@ -379,6 +379,143 @@ def test_upstream_input_refusal_is_a_sanitized_400(remote_chat: tuple, code: str assert len(requests) == 1 +@pytest.mark.parametrize( + ("status", "caller_error"), [(400, True), (422, True), (401, False), (403, False), (404, False)] +) +@pytest.mark.parametrize("streaming", [False, True]) +def test_upstream_refusal_without_a_code_is_the_callers_400_only_for_400_and_422( + remote_chat: tuple, status: int, caller_error: bool, streaming: bool +) -> None: + client, _, _, requests = remote_chat + answer_with( + remote_chat, + json_response(status, {"error": {"message": "private upstream detail", "type": "invalid_request_error"}}), + ) + response = client.post("/v1/chat/completions", json={**BODY, "stream": streaming}) + assert response.status_code == (400 if caller_error else 502) + assert response.json()["error"]["code"] == ("invalid_request" if caller_error else "upstream_error") + assert "private" not in response.text + assert "chat-api" not in response.text + assert len(requests) == 1 + + +TOOLS = [ + {"type": "function", "function": {"name": "read_file", "parameters": {"type": "object"}}}, + {"type": "function", "function": {"name": "list_dir", "parameters": {"type": "object"}}}, +] +READ_FILE = {"type": "function", "function": {"name": "read_file"}} + + +def tool_answer(names: list[str], *, streaming: bool) -> httpx.Response: + calls = [ + {"id": f"call-{index}", "type": "function", "function": {"name": name, "arguments": "{}"}} + for index, name in enumerate(names) + ] + finish = "tool_calls" if names else "stop" + if not streaming: + message = ( + {"role": "assistant", "content": None, "tool_calls": calls} + if names + else completion()["choices"][0]["message"] + ) + return json_response(200, completion(choices=[{"index": 0, "message": message, "finish_reason": finish}])) + delta = ( + {"role": "assistant", "tool_calls": [{"index": index, **call} for index, call in enumerate(calls)]} + if names + else {"role": "assistant", "content": "answer"} + ) + stream = ChatStream( + [ + {"choices": [{"index": 0, "delta": delta, "finish_reason": None}]}, + {"choices": [{"index": 0, "delta": {}, "finish_reason": finish}]}, + {"choices": [], "usage": USAGE}, + "[DONE]", + ] + ) + return httpx.Response(200, headers={"content-type": "text/event-stream"}, stream=stream) + + +@pytest.mark.parametrize( + ("names", "changes"), + [ + pytest.param(["delete_everything"], {}, id="undeclared"), + pytest.param(["read_file"], {"tool_choice": "none"}, id="choice-none"), + pytest.param(["read_file", "list_dir"], {"parallel_tool_calls": False}, id="parallel-false"), + pytest.param(["list_dir"], {"tool_choice": READ_FILE}, id="named-other"), + pytest.param([], {"tool_choice": READ_FILE}, id="named-missing"), + pytest.param([], {"tool_choice": "required"}, id="required-missing"), + ], +) +@pytest.mark.parametrize("streaming", [False, True]) +def test_direct_remote_chat_refuses_an_answer_outside_the_requested_tools( + remote_chat: tuple, names: list[str], changes: dict, streaming: bool +) -> None: + client, _, _, requests = remote_chat + answer_with(remote_chat, tool_answer(names, streaming=streaming)) + response = client.post("/v1/chat/completions", json={**BODY, "stream": streaming, "tools": TOOLS, **changes}) + if streaming and not names: + assert response.status_code == 200 + assert '"content":"answer"' in response.text + assert '"code":"upstream_error"' in response.text + assert '"finish_reason":"stop"' not in response.text + else: + assert response.status_code == 502, response.text + assert response.json()["error"]["code"] == "upstream_error" + assert not any(name in response.text for name in names) + assert len(requests) == 1 + assert upstream_limiter("chat-api")._in_flight == 0 + + +@pytest.mark.parametrize( + ("names", "changes"), + [ + pytest.param(["read_file"], {}, id="declared"), + pytest.param(["read_file", "list_dir"], {}, id="parallel"), + pytest.param(["read_file"], {"parallel_tool_calls": False}, id="parallel-false"), + pytest.param(["read_file"], {"tool_choice": READ_FILE}, id="named"), + pytest.param(["list_dir"], {"tool_choice": "required"}, id="required"), + pytest.param([], {"tool_choice": "none"}, id="choice-none"), + ], +) +@pytest.mark.parametrize("streaming", [False, True]) +def test_direct_remote_chat_serves_an_answer_within_the_requested_tools( + remote_chat: tuple, names: list[str], changes: dict, streaming: bool +) -> None: + client, _, _, _ = remote_chat + answer_with(remote_chat, tool_answer(names, streaming=streaming)) + response = client.post("/v1/chat/completions", json={**BODY, "stream": streaming, "tools": TOOLS, **changes}) + assert response.status_code == 200, response.text + assert '"error"' not in response.text + if streaming: + assert response.text.endswith("data: [DONE]\n\n") + assert all(f'"name":"{name}"' in response.text for name in names) + else: + calls = response.json()["choices"][0]["message"].get("tool_calls") or [] + assert [call["function"]["name"] for call in calls] == names + + +@pytest.mark.parametrize("name", ["math.factorial", "two words", "café", "a/b", "a" * 65]) +def test_tool_names_outside_the_openai_pattern_never_dispatch(remote_chat: tuple, name: str) -> None: + client, _, _, requests = remote_chat + answer_with(remote_chat, lambda: pytest.fail("an invalid tool name must not dispatch")) + tools = [TOOLS[0], {"type": "function", "function": {"name": name}}] + response = client.post("/v1/chat/completions", json={**BODY, "tools": tools}) + assert response.status_code == 400, response.text + error = response.json()["error"] + assert error["param"] == "tools[1].function.name" + assert error["type"] == "invalid_request_error" + assert not requests + + +@pytest.mark.parametrize("name", ["get-weather_2", "A" * 64]) +def test_tool_names_inside_the_openai_pattern_dispatch(remote_chat: tuple, name: str) -> None: + client, _, _, requests = remote_chat + answer_with(remote_chat, json_response(200, completion())) + tools = [TOOLS[0], {"type": "function", "function": {"name": name}}] + assert client.post("/v1/chat/completions", json={**BODY, "tools": tools}).status_code == 200 + assert [tool["function"]["name"] for tool in json.loads(requests[0].content)["tools"]] == ["read_file", name] + + @pytest.mark.parametrize("streaming", [False, True]) def test_cached_serving_switch_refusal_is_a_sanitized_503(remote_chat: tuple, streaming: bool) -> None: client, adapter, _, requests = remote_chat @@ -445,19 +582,13 @@ def test_include_reasoning_publishes_remote_reasoning_only_for_a_reasoning_model assert reasoning == ("PRIVATE_REASONING" if declares_reasoning else "") -@pytest.mark.parametrize("status", [400, 503]) -def test_native_remote_stream_refusal_precedes_http_success(remote_chat: tuple, status: int) -> None: +@pytest.mark.parametrize(("status", "code"), [(400, "INVALID_INPUT"), (400, None), (503, None)]) +def test_native_remote_stream_refusal_precedes_http_success(remote_chat: tuple, status: int, code: str | None) -> None: client, _, config, requests = remote_chat config.profiles["default"].adapter_options.runtime.clear() answer_with( remote_chat, - json_response( - status, - {"error": {"code": "INVALID_INPUT", "message": "private"}} - if status == 400 - else {"error": {"message": "private"}}, - headers={"retry-after": "19"}, - ), + json_response(status, {"error": {"code": code, "message": "private"}}, headers={"retry-after": "19"}), ) response = client.post( f"/v1/generate/{SAFE_MODEL}", json={"prompt": "question", "max_new_tokens": 8, "stream": True} diff --git a/packages/sie_server/tests/processors/test_remote_chat.py b/packages/sie_server/tests/processors/test_remote_chat.py index 63681ec24..9f6140a75 100644 --- a/packages/sie_server/tests/processors/test_remote_chat.py +++ b/packages/sie_server/tests/processors/test_remote_chat.py @@ -203,7 +203,8 @@ async def test_unsupported_chat_fields_refuse_without_dispatch(remote: tuple, fi @pytest.mark.parametrize( - ("status", "code"), [(429, "RESOURCE_EXHAUSTED"), (503, "RESOURCE_EXHAUSTED"), (400, "inference_error")] + ("status", "code"), + [(429, "RESOURCE_EXHAUSTED"), (503, "RESOURCE_EXHAUSTED"), (400, "invalid_request"), (401, "inference_error")], ) async def test_remote_refusal_is_single_attempt_and_sanitized(remote: tuple, status: int, code: str) -> None: respond(remote, status=status, payload={"error": {"message": "PRIVATE_UPSTREAM_DETAIL"}}) @@ -408,14 +409,28 @@ async def test_buffered_chat_preserves_tools_and_exact_usage(remote: tuple) -> N @pytest.mark.parametrize( - ("code", "expected"), [("INVALID_INPUT", "invalid_request"), ("INPUT_TOO_LONG", "INPUT_TOO_LONG")] + ("status", "code", "expected"), + [ + (400, "INVALID_INPUT", "invalid_request"), + (400, "INPUT_TOO_LONG", "INPUT_TOO_LONG"), + (400, "invalid_request_error", "invalid_request"), + (422, None, "invalid_request"), + (400, "MODEL_NOT_FOUND", "inference_error"), + (413, None, "inference_error"), + (401, None, "inference_error"), + (403, None, "inference_error"), + (404, None, "inference_error"), + ], ) -async def test_known_input_refusal_preserves_type(remote: tuple, code: str, expected: str) -> None: - respond(remote, status=400, payload={"error": {"code": code, "message": "PRIVATE"}}) +async def test_input_refusal_is_the_callers_error_and_other_refusals_are_not( + remote: tuple, status: int, code: str | None, expected: str +) -> None: + respond(remote, status=status, payload={"error": {"code": code, "message": "PRIVATE"}}) _msg, chunks = await run(remote) assert chunks[-1]["error"]["code"] == expected assert len(remote[3]) == 1 assert "PRIVATE" not in json.dumps(chunks) + assert "queued-chat" not in json.dumps(chunks) async def test_post_output_disconnect_is_final(remote: tuple) -> None: From cbc235123a8bf54c6c7658603ff7eeda8545333b Mon Sep 17 00:00:00 2001 From: Krisztian Gajdar Date: Wed, 14 Oct 2026 20:20:51 +0200 Subject: [PATCH 2/2] fix(api): serve null tools, log refused generation input, and pin tool-name accumulation _check_tools treats an explicit "tools": null as no tools. The forwarded single-node chat route raised a TypeError on it after the tool checks were added, and the locally rendered path already did at the end of every answer. open_stream logs every generation request an upstream refuses at WARNING with the upstream's name and the status. An upstream that refuses a configured default answers like one that refuses the caller's input, so the caller's 400 alone would hide it from the operator. Tests drive the single-node routes for streamed tool names split across events, a second call in a later event and a name repeated on every delta, for null tools on the forwarded and rendered paths, and for an uncoded 400 or 422 on every generation surface with its code, missing upstream name and log line. REMOTE_BACKENDS.md states what a generation request carries, the rendered path, the log line, each route's status for a tool violation, partial delivery of a streamed name, and each route's code for an invalid tool name. --- packages/sie_server/REMOTE_BACKENDS.md | 54 +++++++---- .../src/sie_server/adapters/remote/_http.py | 24 +++-- .../src/sie_server/processors/remote_chat.py | 2 +- .../sie_server/tests/api/test_remote_chat.py | 95 ++++++++++++++++++- 4 files changed, 148 insertions(+), 27 deletions(-) diff --git a/packages/sie_server/REMOTE_BACKENDS.md b/packages/sie_server/REMOTE_BACKENDS.md index 526bcf57e..20b188642 100644 --- a/packages/sie_server/REMOTE_BACKENDS.md +++ b/packages/sie_server/REMOTE_BACKENDS.md @@ -276,29 +276,49 @@ returns the original local refusal with its retry delay. A client error is never retried remotely, and fallback cannot replay work after local acceptance or after output reaches the caller. -Chat, completions and native generation forward the caller's own request. An -upstream that answers such a request with `400` or `422` and no known SIE error -code refused it, so the caller receives `400` `invalid_request`, as for an -`INVALID_INPUT` refusal, with fixed text that repeats nothing the upstream sent -and does not name the upstream. Encode, score and extraction build every -upstream body themselves, so the same answer there is a server error. A `400` -or `422` with a known SIE error code other than `INVALID_INPUT` or -`INPUT_TOO_LONG`, and other upstream `4xx` answers such as `401`, `403` or -`404`, point at the operator's credential, model name or route rather than at -the request, so the caller receives a server error. `429` and `5xx` answers are -retryable as described above. +A generation request sends the upstream the caller's messages or prompt and +sampling parameters, together with defaults the model configures, such as +sampling defaults, stop sequences or chat template arguments. This holds for +chat, completions and native generation, streamed or buffered, and for chat +that the server renders through the model's template before it calls the +upstream's completions or native generation endpoint. An upstream that answers +a generation request with `400` or `422` and no known SIE error code refused +that input. An OpenAI-compatible API answers this way, for example, to a +prompt longer than its context. The caller receives `400` `invalid_request`, +as for an `INVALID_INPUT` refusal, with fixed text that repeats nothing the +upstream sent and does not name the upstream. An upstream that refuses a +configured default answers the same way, so the server logs every refused +generation request at `WARNING` with the upstream's name and status, where a +refused default shows as a refusal of every request. Encode, score and +extraction report the same upstream answer as a server error. A `400` or `422` +with a known SIE error code other than `INVALID_INPUT` or `INPUT_TOO_LONG`, and +other upstream `4xx` answers such as `401`, `403` or `404`, point at the +operator's credential, model name or route rather than at the request, so the +caller receives a server error. `429` and `5xx` answers are retryable as +described above. Chat checks an upstream answer against the request's tools on a single node and in a cluster, streamed and buffered. The answer fails as an upstream error when it calls a function the request did not declare, calls any function under `tool_choice: "none"`, calls a function other than the one `tool_choice` names, makes several calls under `parallel_tool_calls: false`, or makes no call when -`tool_choice` is `"required"` or names a function. A stream that has already -started ends with an error event. Tool function names must -match `^[a-zA-Z0-9_-]{1,64}$`, the pattern OpenAI-compatible APIs accept, so a -local and a remote profile accept the same requests. The gateway and the -single-node chat route refuse any other name with `400` before dispatch and -name the field, for example `tools[1].function.name`. +`tool_choice` is `"required"` or names a function. Before output, a single node +answers that failure with `502` `upstream_error` and a cluster with `500` +`inference_error`, as each answers any other upstream failure. A stream that +has already started ends with an error event. A streamed function name is +checked as OpenAI clients assemble it, by concatenating the fragments of each +call index. A fragment is delivered while the name so far begins a permitted +name, so a stream can deliver the start of a name before the call fails. An +upstream that repeats the whole name on every delta fails, because the +concatenated name is not one the request declared. + +Tool function names must match `^[a-zA-Z0-9_-]{1,64}$`, the pattern +OpenAI-compatible APIs accept, so a local and a remote profile accept the same +requests. The gateway and the single-node chat route refuse any other name +with `400` before dispatch and name the field, for example +`tools[1].function.name`. The error code is each route's code for an invalid +chat request: `invalid_request` from the gateway and `INVALID_INPUT` from a +single node. TLS is required outside loopback. URLs containing credentials, a query or a fragment are rejected. Redirects are refused, ambient proxy variables are diff --git a/packages/sie_server/src/sie_server/adapters/remote/_http.py b/packages/sie_server/src/sie_server/adapters/remote/_http.py index 27cc59085..624cdc503 100644 --- a/packages/sie_server/src/sie_server/adapters/remote/_http.py +++ b/packages/sie_server/src/sie_server/adapters/remote/_http.py @@ -14,9 +14,10 @@ connection, a timeout, a dropped response, another 5xx). A 501, a 505 and the codes ``MODEL_LOAD_FAILED``, ``ACCOUNT_STATE_UNAVAILABLE`` and ``COLD_START_RATE_LIMITED`` are final. A refused input raises -:class:`InputTooLongError` or :class:`InvalidInputError`. A streamed -generation call forwards the caller's request, so there a 400 or 422 without a -known code is a refused input too. Everything else is a +:class:`InputTooLongError` or :class:`InvalidInputError`. A generation call, +streamed or buffered, carries the caller's input and sampling, so there a 400 +or 422 without a known code is a refused input too, and is logged because a +configured default the upstream refuses looks the same. Everything else is a :class:`RemoteUpstreamError`. """ @@ -24,6 +25,7 @@ import asyncio import json +import logging import math import re import time @@ -48,6 +50,8 @@ from sie_server.core.upstream_deadline import DEADLINE_EXTENSION from sie_server.types.inputs import InvalidInputError +logger = logging.getLogger(__name__) + DEFAULT_RETRY_AFTER_S = 5 _ERROR_BODY_MAX_BYTES = 64 << 10 _NOT_READY_CODES = frozenset({"MODEL_LOADING", "PROVISIONING", "LORA_LOADING"}) @@ -182,10 +186,11 @@ async def open_stream( The call goes through the upstream's limiter, whose slot is held until the block ends. The answer is a success with an uncompressed body, which the block reads. Failures raise as :func:`send_bounded` describes, including a - connection lost while the block reads. The request forwards the caller's - own, so a 400 or 422 without a known code also raises - :class:`InvalidInputError`. Leaving the block closes the answer, which - cancels work still running upstream. + connection lost while the block reads. The request carries the caller's + input, so a 400 or 422 without a known code also raises + :class:`InvalidInputError`. A refused input is logged with the status. + Leaving the block closes the answer, which cancels work still running + upstream. """ upstream_for_serving(upstream) with upstream_limiter(upstream).call(): @@ -208,7 +213,7 @@ async def open_stream( raise failure_for_transport_error(exc, upstream=upstream) from None try: if response.status_code >= 400: - raise failure_for_status( + failure = failure_for_status( response.status_code, upstream_error_code( response.headers, await _read_error_body_async(response, timeout_s=error_body_timeout_s) @@ -217,6 +222,9 @@ async def open_stream( upstream=upstream, caller_request=True, ) + if isinstance(failure, InputTooLongError | InvalidInputError): + logger.warning("upstream %r refused a generation request: %s", upstream, failure) + raise failure if _is_compressed(response): raise RemoteUpstreamError("upstream sent a compressed body, which is refused") yield response diff --git a/packages/sie_server/src/sie_server/processors/remote_chat.py b/packages/sie_server/src/sie_server/processors/remote_chat.py index cdd43036d..480d1ed82 100644 --- a/packages/sie_server/src/sie_server/processors/remote_chat.py +++ b/packages/sie_server/src/sie_server/processors/remote_chat.py @@ -42,7 +42,7 @@ def _logprobs(choice: dict[str, Any], body: dict[str, Any]) -> tuple[dict[str, A def _check_tools(calls: dict[int, str], body: dict[str, Any], *, finished: bool) -> None: mode, name = normalize_tool_choice(body.get("tool_choice")) - allowed = {tool["function"]["name"] for tool in body.get("tools", [])} + allowed = {tool["function"]["name"] for tool in body.get("tools") or []} if calls and (mode == "none" or not allowed or (not body.get("parallel_tool_calls", True) and len(calls) > 1)): raise RemoteUpstreamError("upstream chat did not honor the requested tools") permitted = {name} if name is not None and name in allowed else allowed if name is None else set() diff --git a/packages/sie_server/tests/api/test_remote_chat.py b/packages/sie_server/tests/api/test_remote_chat.py index f6653ccdd..d26d258d1 100644 --- a/packages/sie_server/tests/api/test_remote_chat.py +++ b/packages/sie_server/tests/api/test_remote_chat.py @@ -494,6 +494,68 @@ def test_direct_remote_chat_serves_an_answer_within_the_requested_tools( assert [call["function"]["name"] for call in calls] == names +def tool_fragments(fragments: list[tuple[int, str]]) -> httpx.Response: + frames: list[dict | str] = [] + started: set[int] = set() + for index, name in fragments: + tool: dict[str, Any] = {"index": index, "function": {"name": name}} + if index not in started: + started.add(index) + tool.update(id=f"call-{index}", type="function") + tool["function"]["arguments"] = "{}" + frames.append({"choices": [{"index": 0, "delta": {"tool_calls": [tool]}, "finish_reason": None}]}) + frames += [ + {"choices": [{"index": 0, "delta": {}, "finish_reason": "tool_calls"}]}, + {"choices": [], "usage": USAGE}, + "[DONE]", + ] + return httpx.Response(200, headers={"content-type": "text/event-stream"}, stream=ChatStream(frames)) + + +@pytest.mark.parametrize( + ("fragments", "served", "delivered"), + [ + pytest.param([(0, "read"), (0, "_file")], True, {0: "read_file"}, id="name-split-across-events"), + pytest.param( + [(0, "read_file"), (1, "list_dir")], + True, + {0: "read_file", 1: "list_dir"}, + id="second-call-in-a-later-event", + ), + pytest.param([(0, "read_file"), (0, "read_file")], False, {0: "read_file"}, id="name-repeated-per-delta"), + ], +) +def test_direct_remote_chat_accumulates_each_streamed_tool_name_by_call_index( + remote_chat: tuple, fragments: list[tuple[int, str]], served: bool, delivered: dict[int, str] +) -> None: + client, _, _, requests = remote_chat + answer_with(remote_chat, tool_fragments(fragments)) + response = client.post("/v1/chat/completions", json={**BODY, "stream": True, "tools": TOOLS}) + assert response.status_code == 200, response.text + names: dict[int, str] = {} + for line in response.text.splitlines(): + if line.startswith("data: {"): + for choice in json.loads(line[6:]).get("choices") or []: + for call in choice["delta"].get("tool_calls") or []: + names[call["index"]] = names.get(call["index"], "") + call["function"].get("name", "") + assert names == delivered + assert ('"finish_reason":"tool_calls"' in response.text) is served + assert ('"code":"upstream_error"' in response.text) is not served + assert response.text.endswith("data: [DONE]\n\n") + assert len(requests) == 1 + + +@pytest.mark.parametrize("streaming", [False, True]) +def test_direct_remote_chat_serves_explicit_null_tools(remote_chat: tuple, streaming: bool) -> None: + client, _, _, requests = remote_chat + answer_with(remote_chat, tool_answer([], streaming=streaming)) + response = client.post("/v1/chat/completions", json={**BODY, "stream": streaming, "tools": None}) + assert response.status_code == 200, response.text + assert "answer" in response.text + assert '"error"' not in response.text + assert len(requests) == 1 + + @pytest.mark.parametrize("name", ["math.factorial", "two words", "café", "a/b", "a" * 65]) def test_tool_names_outside_the_openai_pattern_never_dispatch(remote_chat: tuple, name: str) -> None: client, _, _, requests = remote_chat @@ -504,6 +566,8 @@ def test_tool_names_outside_the_openai_pattern_never_dispatch(remote_chat: tuple error = response.json()["error"] assert error["param"] == "tools[1].function.name" assert error["type"] == "invalid_request_error" + unnamed = client.post("/v1/chat/completions", json={**BODY, "tools": [{"type": "function", "function": {}}]}) + assert error["code"] == unnamed.json()["error"]["code"] == "INVALID_INPUT" assert not requests @@ -699,6 +763,33 @@ def test_generation_bridge_restores_original_local_refusal_across_surfaces( assert upstream_limiter("chat-api")._in_flight == 0 +@pytest.mark.parametrize(("surface", "streaming"), _GENERATION_SURFACES) +@pytest.mark.parametrize("upstream_status", [400, 422]) +def test_uncoded_upstream_refusal_is_a_logged_callers_400_on_every_generation_surface( + remote_chat: tuple, surface: str, streaming: bool, upstream_status: int, caplog: pytest.LogCaptureFixture +) -> None: + client, _, config, requests = remote_chat + config.profiles["default"].adapter_options.runtime.clear() + answer_with( + remote_chat, + json_response(upstream_status, {"error": {"message": "private upstream detail", "param": "top_k"}}), + ) + path, body = _generation_request(surface, stream=streaming) + with caplog.at_level("WARNING", logger="sie_server.adapters.remote._http"): + response = client.post(path, json=body) + assert response.status_code == 400, response.text + payload = response.json() + assert payload.get("error", payload.get("detail"))["code"] == "invalid_request" + assert "private" not in response.text + assert "chat-api" not in response.text + assert "X-SIE-Upstream" not in response.headers + assert len(requests) == 1 + refusals = [record.getMessage() for record in caplog.records if record.name == "sie_server.adapters.remote._http"] + assert refusals == [ + f"upstream 'chat-api' refused a generation request: the upstream refused the input ({upstream_status})" + ] + + @pytest.mark.parametrize(("surface", "streaming"), _GENERATION_SURFACES) def test_remote_forbid_keeps_cold_generation_off_the_bridge(remote_chat: tuple, surface: str, streaming: bool) -> None: client, _, _, requests = remote_chat @@ -894,9 +985,10 @@ def test_empty_chat_tools_cannot_satisfy_a_required_choice(remote_chat: tuple, c registry.start_load_async.assert_not_awaited() +@pytest.mark.parametrize("tools", [{}, {"tools": None}], ids=["no-tools", "null-tools"]) @pytest.mark.parametrize("stream", [False, True]) def test_onboarded_direct_chat_uses_raw_template_without_chat_dispatch( - remote_chat, monkeypatch: pytest.MonkeyPatch, stream: bool + remote_chat, monkeypatch: pytest.MonkeyPatch, stream: bool, tools: dict ) -> None: client, adapter, config, requests = remote_chat data = config.model_dump() @@ -934,6 +1026,7 @@ def test_onboarded_direct_chat_uses_raw_template_without_chat_dispatch( "max_completion_tokens": None, "stream": stream, "stream_options": {"include_usage": True}, + **tools, }, ) assert response.status_code == 200