mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-03 07:21:54 +00:00
fix(api_server): coerce stringified booleans in request payloads
This commit is contained in:
parent
fb138d91ca
commit
8d4766afca
3 changed files with 145 additions and 4 deletions
|
|
@ -71,6 +71,35 @@ def _coerce_port(value: Any, default: int = DEFAULT_PORT) -> int:
|
|||
return default
|
||||
|
||||
|
||||
_TRUE_REQUEST_BOOL_STRINGS = frozenset({"1", "true", "yes", "on"})
|
||||
_FALSE_REQUEST_BOOL_STRINGS = frozenset({"0", "false", "no", "off"})
|
||||
|
||||
|
||||
def _coerce_request_bool(value: Any, default: bool = False) -> bool:
|
||||
"""Normalize boolean-like API payload values.
|
||||
|
||||
External clients should send real JSON booleans, but some OpenAI-compatible
|
||||
frontends and middleware serialize flags like ``stream`` as strings. Using
|
||||
Python truthiness on those values misroutes requests because ``"false"`` is
|
||||
still truthy. Treat only explicit bool-ish scalars as booleans; everything
|
||||
else falls back to the caller's default.
|
||||
"""
|
||||
if isinstance(value, bool):
|
||||
return value
|
||||
if value is None:
|
||||
return default
|
||||
if isinstance(value, str):
|
||||
normalized = value.strip().lower()
|
||||
if normalized in _TRUE_REQUEST_BOOL_STRINGS:
|
||||
return True
|
||||
if normalized in _FALSE_REQUEST_BOOL_STRINGS:
|
||||
return False
|
||||
return default
|
||||
if isinstance(value, (int, float)):
|
||||
return bool(value)
|
||||
return default
|
||||
|
||||
|
||||
def _normalize_chat_content(
|
||||
content: Any, *, _max_depth: int = 10, _depth: int = 0,
|
||||
) -> str:
|
||||
|
|
@ -1005,7 +1034,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
status=400,
|
||||
)
|
||||
|
||||
stream = body.get("stream", False)
|
||||
stream = _coerce_request_bool(body.get("stream"), default=False)
|
||||
|
||||
# Extract system message (becomes ephemeral system prompt layered ON TOP of core)
|
||||
system_prompt = None
|
||||
|
|
@ -2082,7 +2111,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
instructions = body.get("instructions")
|
||||
previous_response_id = body.get("previous_response_id")
|
||||
conversation = body.get("conversation")
|
||||
store = body.get("store", True)
|
||||
store = _coerce_request_bool(body.get("store"), default=True)
|
||||
|
||||
# conversation and previous_response_id are mutually exclusive
|
||||
if conversation and previous_response_id:
|
||||
|
|
@ -2165,7 +2194,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
# groups the entire conversation under one session entry.
|
||||
session_id = stored_session_id or str(uuid.uuid4())
|
||||
|
||||
stream = bool(body.get("stream", False))
|
||||
stream = _coerce_request_bool(body.get("stream"), default=False)
|
||||
if stream:
|
||||
# Streaming branch — emit OpenAI Responses SSE events as the
|
||||
# agent runs so frontends can render text deltas and tool
|
||||
|
|
@ -3228,7 +3257,10 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
status=409,
|
||||
)
|
||||
|
||||
resolve_all = bool(body.get("all") or body.get("resolve_all"))
|
||||
resolve_all = (
|
||||
_coerce_request_bool(body.get("all"), default=False)
|
||||
or _coerce_request_bool(body.get("resolve_all"), default=False)
|
||||
)
|
||||
try:
|
||||
from tools.approval import resolve_gateway_approval
|
||||
|
||||
|
|
|
|||
|
|
@ -704,6 +704,37 @@ class TestChatCompletionsEndpoint:
|
|||
assert "[DONE]" in body
|
||||
assert "Hello!" in body
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_string_false_returns_json_completion(self, adapter):
|
||||
"""Quoted false must not route chat completions into SSE mode."""
|
||||
mock_result = {
|
||||
"final_response": "Hello! How can I help you today?",
|
||||
"messages": [],
|
||||
"api_calls": 1,
|
||||
}
|
||||
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
mock_result,
|
||||
{"input_tokens": 10, "output_tokens": 5, "total_tokens": 15},
|
||||
)
|
||||
resp = await cli.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "hermes-agent",
|
||||
"messages": [{"role": "user", "content": "Hello"}],
|
||||
"stream": "false",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status == 200
|
||||
assert "text/event-stream" not in resp.headers.get("Content-Type", "")
|
||||
data = await resp.json()
|
||||
assert data["object"] == "chat.completion"
|
||||
assert data["choices"][0]["message"]["content"] == mock_result["final_response"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_task_done_callback_enqueues_eos_for_chat_completions(self, adapter):
|
||||
"""Regression guard for #24451: completion callback must signal SSE EOS."""
|
||||
|
|
@ -1655,6 +1686,31 @@ class TestResponsesEndpoint:
|
|||
# The response has an ID but it shouldn't be retrievable
|
||||
assert adapter._response_store.get(data["id"]) is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_store_string_false_does_not_store(self, adapter):
|
||||
"""Quoted false must preserve ephemeral store=false semantics."""
|
||||
mock_result = {"final_response": "OK", "messages": [], "api_calls": 1}
|
||||
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
mock_result,
|
||||
{"input_tokens": 0, "output_tokens": 0, "total_tokens": 0},
|
||||
)
|
||||
resp = await cli.post(
|
||||
"/v1/responses",
|
||||
json={
|
||||
"model": "hermes-agent",
|
||||
"input": "Hello",
|
||||
"store": "false",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status == 200
|
||||
data = await resp.json()
|
||||
assert adapter._response_store.get(data["id"]) is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_instructions_inherited_from_previous(self, adapter):
|
||||
"""If no instructions provided, carry forward from previous response."""
|
||||
|
|
@ -1749,6 +1805,37 @@ class TestResponsesStreaming:
|
|||
assert "Hello" in body
|
||||
assert " world" in body
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_string_false_returns_json_response(self, adapter):
|
||||
"""Quoted false must not route Responses API requests into SSE mode."""
|
||||
mock_result = {
|
||||
"final_response": "Paris is the capital of France.",
|
||||
"messages": [],
|
||||
"api_calls": 1,
|
||||
}
|
||||
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
mock_result,
|
||||
{"input_tokens": 0, "output_tokens": 0, "total_tokens": 0},
|
||||
)
|
||||
resp = await cli.post(
|
||||
"/v1/responses",
|
||||
json={
|
||||
"model": "hermes-agent",
|
||||
"input": "What is the capital of France?",
|
||||
"stream": "false",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status == 200
|
||||
assert "text/event-stream" not in resp.headers.get("Content-Type", "")
|
||||
data = await resp.json()
|
||||
assert data["object"] == "response"
|
||||
assert data["output"][0]["content"][0]["text"] == mock_result["final_response"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_task_done_callback_enqueues_eos_for_responses(self, adapter):
|
||||
"""Regression guard for #24451 on /v1/responses streaming path."""
|
||||
|
|
|
|||
|
|
@ -335,6 +335,28 @@ class TestRunEvents:
|
|||
"approval_not_pending",
|
||||
}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approval_string_false_does_not_resolve_all(self, adapter):
|
||||
"""Quoted false must not fan out approval resolution across the queue."""
|
||||
app = _create_runs_app(adapter)
|
||||
run_id = "run_bool_parse"
|
||||
adapter._run_statuses[run_id] = {"run_id": run_id, "status": "running"}
|
||||
adapter._run_approval_sessions[run_id] = "session-123"
|
||||
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch("tools.approval.resolve_gateway_approval", return_value=1) as mock_resolve:
|
||||
approval_resp = await cli.post(
|
||||
f"/v1/runs/{run_id}/approval",
|
||||
json={"choice": "once", "all": "false"},
|
||||
)
|
||||
|
||||
assert approval_resp.status == 200
|
||||
mock_resolve.assert_called_once_with(
|
||||
"session-123",
|
||||
"once",
|
||||
resolve_all=False,
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_events_not_found_returns_404(self, adapter):
|
||||
app = _create_runs_app(adapter)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue