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..20b188642 100644 --- a/packages/sie_server/REMOTE_BACKENDS.md +++ b/packages/sie_server/REMOTE_BACKENDS.md @@ -276,6 +276,50 @@ 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. +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. 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 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..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,7 +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`. 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`. """ @@ -22,6 +25,7 @@ import asyncio import json +import logging import math import re import time @@ -46,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"}) @@ -126,9 +132,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 +155,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 +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. 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(): @@ -197,14 +213,18 @@ 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) ), parse_retry_after(response.headers.get("retry-after")), 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/api/openai_local.py b/packages/sie_server/src/sie_server/api/openai_local.py index f1e135ca4..add8fec18 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 @@ -90,6 +91,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, @@ -127,6 +129,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 @@ -593,13 +596,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") @@ -1266,6 +1274,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]], *, @@ -1279,12 +1297,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, @@ -1387,6 +1407,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/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 d36dd0d42..d26d258d1 100644 --- a/packages/sie_server/tests/api/test_remote_chat.py +++ b/packages/sie_server/tests/api/test_remote_chat.py @@ -379,6 +379,207 @@ 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 + + +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 + 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" + unnamed = client.post("/v1/chat/completions", json={**BODY, "tools": [{"type": "function", "function": {}}]}) + assert error["code"] == unnamed.json()["error"]["code"] == "INVALID_INPUT" + 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 +646,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} @@ -568,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 @@ -763,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() @@ -803,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 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: