mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(api_server): close divergence gaps from gateway/run.py
Three parity fixes between the API server and the native gateway's agent-runtime resolution, integrated with the provider-aware request routing that landed in #70853: - Session-persisted model is honored: POST /api/sessions {"model": ...} stores a model that the chat handlers previously fetched and threw away. A stored value that matches a model_routes alias goes through the route path (route provider/credentials apply); a raw model string threads through as session_model, pinning the session's turns ahead of per-request body values but below an explicit session /model override. - Empty-model recovery: provider-catalog default when config has no model.default but a provider resolved, plus last-known-good model recovery (#35314) keyed on gateway_session_key only (never ephemeral session_id — no unbounded growth from one-off requests). - Provider auth failures surface as controlled responses: RuntimeError from _resolve_runtime_agent_kwargs() is re-raised as a dedicated _ProviderAuthResolutionError at the call site, caught narrowly in _run_agent() and the /v1/runs executor to return run.py's response shape instead of an undifferentiated 500 (session-chat endpoints previously returned a raw aiohttp 500 with no JSON body). Salvaged from PR #57947 by @FvanW; session-model route-alias resolution from PR #59941 by @kaishi00. Co-authored-by: kaishi00 <kaishi00@users.noreply.github.com>
This commit is contained in:
Frederick
Teknium
parent
0f732cb3d6
commit
7dd00bb47d
5 changed files with 552 additions and 11 deletions
|
|
@ -5524,3 +5524,141 @@ class TestRouteWithoutModelKeepsDefault:
|
|||
|
||||
assert captured["model"] == "global/model"
|
||||
assert captured["api_key"] == "sk-route"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Empty-model recovery + provider-auth error typing in _create_agent
|
||||
# (salvaged from PR #57947 by @FvanW)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCreateAgentModelRecovery:
|
||||
def test_create_agent_defaults_to_provider_catalog_model_when_empty(self, monkeypatch):
|
||||
"""api_server.py had no equivalent of run.py's provider-catalog
|
||||
default when model resolves empty but a provider did resolve (e.g.
|
||||
`hermes auth add openai-codex` without `hermes model`) —
|
||||
AIAgent(model="") 400s every call."""
|
||||
captured = {}
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
_patch_create_agent_runtime(monkeypatch, captured, FakeAgent)
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: {"provider": "openai-codex", "base_url": "https://example.test/v1",
|
||||
"api_mode": "codex_responses"},
|
||||
)
|
||||
monkeypatch.setattr("gateway.run._resolve_gateway_model", lambda: "")
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.models.get_default_model_for_provider",
|
||||
lambda provider: "gpt-5.5-codex" if provider == "openai-codex" else None,
|
||||
)
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
|
||||
agent = adapter._create_agent(session_id="api-session")
|
||||
|
||||
assert isinstance(agent, FakeAgent)
|
||||
assert captured["model"] == "gpt-5.5-codex"
|
||||
|
||||
def test_create_agent_recovers_last_known_good_model_when_empty(self, monkeypatch):
|
||||
"""Last-known-good recovery (#35314): a transient config-cache miss
|
||||
producing an empty model would build AIAgent(model="") and fail every
|
||||
call until manual retry, instead of reusing the model that just
|
||||
worked."""
|
||||
captured = []
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
captured.append(dict(kwargs))
|
||||
|
||||
_patch_create_agent_runtime(monkeypatch, {}, FakeAgent)
|
||||
monkeypatch.setattr("run_agent.AIAgent", FakeAgent)
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
|
||||
# Turn 1: model resolves fine — populates the last-known-good cache
|
||||
# (keyed on gateway_session_key).
|
||||
monkeypatch.setattr("gateway.run._resolve_gateway_model", lambda: "minimax/minimax-m3")
|
||||
adapter._create_agent(session_id="api-session", gateway_session_key="stable-chan-1")
|
||||
assert captured[0]["model"] == "minimax/minimax-m3"
|
||||
assert adapter._last_resolved_model["stable-chan-1"] == "minimax/minimax-m3"
|
||||
|
||||
# Turn 2: transient empty resolution, no provider catalog default —
|
||||
# must recover the model from turn 1, not build model="".
|
||||
monkeypatch.setattr("gateway.run._resolve_gateway_model", lambda: "")
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: {"provider": None, "base_url": None, "api_mode": None},
|
||||
)
|
||||
adapter._create_agent(session_id="another-session", gateway_session_key="stable-chan-1")
|
||||
assert captured[1]["model"] == "minimax/minimax-m3"
|
||||
|
||||
def test_last_resolved_model_cache_does_not_grow_per_ephemeral_session_id(self, monkeypatch):
|
||||
"""/v1/responses and /v1/runs hand out a fresh UUID session_id per
|
||||
one-off request (no gateway_session_key). Keying the last-known-good
|
||||
cache on session_id would leave one permanent dict entry per
|
||||
stateless request, growing unbounded for the life of the process.
|
||||
Only gateway_session_key may create an entry."""
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
pass
|
||||
|
||||
_patch_create_agent_runtime(monkeypatch, {}, FakeAgent)
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
|
||||
import uuid as _uuid
|
||||
for _ in range(50):
|
||||
adapter._create_agent(session_id=str(_uuid.uuid4()))
|
||||
|
||||
# Only the "*" process-wide fallback may exist — never one entry per
|
||||
# ephemeral session_id.
|
||||
assert list(adapter._last_resolved_model.keys()) == ["*"]
|
||||
|
||||
def test_create_agent_wraps_runtime_credential_failure_as_provider_auth_error(self, monkeypatch):
|
||||
"""_create_agent() must convert a RuntimeError from
|
||||
gateway.run._resolve_runtime_agent_kwargs() — the sole raiser of
|
||||
RuntimeError(format_runtime_provider_error(...)) in this call graph —
|
||||
into _ProviderAuthResolutionError right at the call site, independent
|
||||
of any caller's exception handling."""
|
||||
from gateway.platforms.api_server import _ProviderAuthResolutionError
|
||||
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: (_ for _ in ()).throw(RuntimeError("No credentials found for provider 'nous'")),
|
||||
)
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
|
||||
with pytest.raises(_ProviderAuthResolutionError, match="No credentials found for provider 'nous'"):
|
||||
adapter._create_agent(session_id="api-session")
|
||||
|
||||
def test_create_agent_session_model_pins_ahead_of_request(self, monkeypatch):
|
||||
"""Session-persisted model beats per-request body values but yields
|
||||
to an explicit session /model override."""
|
||||
captured = {}
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
_patch_create_agent_runtime(monkeypatch, captured, FakeAgent)
|
||||
|
||||
adapter = APIServerAdapter(PlatformConfig(enabled=True))
|
||||
monkeypatch.setattr(adapter, "_ensure_session_db", lambda: None)
|
||||
monkeypatch.setattr(adapter, "_session_model_override_for", lambda *_: None)
|
||||
|
||||
adapter._create_agent(
|
||||
session_id="s1",
|
||||
session_model="session-row/model",
|
||||
requested_model="request/model",
|
||||
)
|
||||
|
||||
assert captured["model"] == "session-row/model"
|
||||
|
|
|
|||
|
|
@ -948,3 +948,38 @@ class TestStopRun:
|
|||
body = await events_resp.text()
|
||||
# Stream should have received run.failed and closed
|
||||
assert "run.failed" in body or "stream closed" in body
|
||||
|
||||
|
||||
class TestRunsProviderAuthFailure:
|
||||
@pytest.mark.asyncio
|
||||
async def test_status_reports_provider_auth_failure_distinctly(self, adapter):
|
||||
"""/v1/runs builds its own agent via _create_agent() and does not
|
||||
route through _run_agent(), so the controlled "Provider
|
||||
authentication failed" message added there does not cover this
|
||||
endpoint. _handle_runs()'s own _ProviderAuthResolutionError branch
|
||||
must give the same distinguished message instead of the generic
|
||||
except-Exception "run failed" text."""
|
||||
from gateway.platforms.api_server import _ProviderAuthResolutionError
|
||||
|
||||
app = _create_runs_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_create_agent") as mock_create:
|
||||
mock_create.side_effect = _ProviderAuthResolutionError(
|
||||
"No credentials found for provider 'nous'"
|
||||
)
|
||||
|
||||
resp = await cli.post("/v1/runs", json={"input": "hello"})
|
||||
assert resp.status == 202
|
||||
data = await resp.json()
|
||||
run_id = data["run_id"]
|
||||
|
||||
for _ in range(40):
|
||||
status_resp = await cli.get(f"/v1/runs/{run_id}")
|
||||
status = await status_resp.json()
|
||||
if status["status"] == "failed":
|
||||
break
|
||||
await asyncio.sleep(0.05)
|
||||
|
||||
assert status["status"] == "failed"
|
||||
assert status["error"] == "⚠️ Provider authentication failed: No credentials found for provider 'nous'"
|
||||
assert status["last_event"] == "run.failed"
|
||||
|
|
|
|||
|
|
@ -438,3 +438,182 @@ async def test_session_header_rejected_without_api_key(adapter, session_db):
|
|||
assert resp.status == 403
|
||||
data = await resp.json()
|
||||
assert "X-Hermes-Session-Key requires API key" in data["error"]["message"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Session-persisted model threading + provider-auth failure surfacing
|
||||
# (salvaged from PR #57947 by @FvanW and PR #59941 by @kaishi00)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_chat_threads_session_model_to_run_agent(auth_adapter, session_db):
|
||||
"""POST /api/sessions persists a per-session model, but the chat handler
|
||||
previously fetched the session record and threw it away — the session's
|
||||
chosen model silently had no effect on any chat turn."""
|
||||
session_id = session_db.create_session("model-pinned-session", "api_server", model="claude-sonnet-4-6")
|
||||
|
||||
mock_run = AsyncMock(return_value=({"final_response": "ok", "session_id": session_id}, {"total_tokens": 1}))
|
||||
app = _create_session_app(auth_adapter)
|
||||
with patch.object(auth_adapter, "_run_agent", mock_run):
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.post(
|
||||
f"/api/sessions/{session_id}/chat",
|
||||
json={"message": "hi"},
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert resp.status == 200
|
||||
|
||||
mock_run.assert_awaited_once()
|
||||
_, kwargs = mock_run.call_args
|
||||
assert kwargs["session_model"] == "claude-sonnet-4-6"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_chat_stream_threads_session_model_to_run_agent(adapter, session_db):
|
||||
"""Streaming twin of the session-model threading test above."""
|
||||
session_id = session_db.create_session("model-pinned-stream-session", "api_server", model="gpt-5.5")
|
||||
|
||||
mock_run = AsyncMock(return_value=({"final_response": "ok", "session_id": session_id}, {"total_tokens": 1}))
|
||||
app = _create_session_app(adapter)
|
||||
with patch.object(adapter, "_run_agent", mock_run):
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.post(
|
||||
f"/api/sessions/{session_id}/chat/stream",
|
||||
json={"message": "hi"},
|
||||
)
|
||||
assert resp.status == 200
|
||||
await resp.read()
|
||||
|
||||
mock_run.assert_awaited_once()
|
||||
_, kwargs = mock_run.call_args
|
||||
assert kwargs["session_model"] == "gpt-5.5"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_chat_resolves_stored_model_route_alias(session_db, monkeypatch):
|
||||
"""A session-persisted model that matches a model_routes alias must go
|
||||
through the route path (so route provider/credentials apply) and NOT be
|
||||
passed as a raw session_model (idea from PR #59941 by @kaishi00)."""
|
||||
adapter = APIServerAdapter(
|
||||
PlatformConfig(
|
||||
enabled=True,
|
||||
extra={"model_routes": {"alias": {"model": "route/model", "provider": "openrouter"}}},
|
||||
)
|
||||
)
|
||||
adapter._session_db = session_db
|
||||
session_id = session_db.create_session("route-pinned-session", "api_server", model="alias")
|
||||
|
||||
mock_run = AsyncMock(return_value=({"final_response": "ok", "session_id": session_id}, {"total_tokens": 1}))
|
||||
app = _create_session_app(adapter)
|
||||
with patch.object(adapter, "_run_agent", mock_run):
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.post(
|
||||
f"/api/sessions/{session_id}/chat",
|
||||
json={"message": "hi"},
|
||||
)
|
||||
assert resp.status == 200
|
||||
|
||||
_, kwargs = mock_run.call_args
|
||||
assert kwargs["route"] == {"model": "route/model", "provider": "openrouter"}
|
||||
assert kwargs["session_model"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_agent_returns_controlled_response_on_provider_auth_failure(adapter, monkeypatch):
|
||||
"""_resolve_runtime_agent_kwargs() (inside _create_agent()) raises
|
||||
RuntimeError on provider auth/credential failure. Previously this
|
||||
propagated unhandled out of _run_agent(): /v1/chat/completions caught it
|
||||
as a generic 500, and /api/sessions/{id}/chat didn't catch it at all
|
||||
(raw aiohttp 500, no JSON body). Must now return run.py's controlled
|
||||
response shape instead of raising. Exercises the REAL boundary
|
||||
(gateway.run._resolve_runtime_agent_kwargs, the sole raiser)."""
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: (_ for _ in ()).throw(
|
||||
RuntimeError("No credentials found for provider 'nous' — run `hermes auth add nous`")
|
||||
),
|
||||
)
|
||||
|
||||
result, usage = await adapter._run_agent(
|
||||
user_message="hello",
|
||||
conversation_history=[],
|
||||
session_id="request-session",
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"final_response": "⚠️ Provider authentication failed: No credentials found for provider 'nous' — run `hermes auth add nous`",
|
||||
"messages": [],
|
||||
"api_calls": 0,
|
||||
"tools": [],
|
||||
}
|
||||
assert usage == {"input_tokens": 0, "output_tokens": 0, "total_tokens": 0}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_agent_does_not_swallow_unrelated_exceptions(adapter, monkeypatch):
|
||||
"""The _ProviderAuthResolutionError catch must stay narrow — a TypeError
|
||||
elsewhere in _create_agent()/run_conversation() must still propagate."""
|
||||
def fake_create_agent(**kwargs):
|
||||
raise TypeError("unrelated bug: unexpected keyword argument")
|
||||
|
||||
monkeypatch.setattr(adapter, "_create_agent", fake_create_agent)
|
||||
|
||||
with pytest.raises(TypeError, match="unrelated bug"):
|
||||
await adapter._run_agent(
|
||||
user_message="hello",
|
||||
conversation_history=[],
|
||||
session_id="request-session",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_agent_does_not_swallow_unrelated_runtime_error_from_run_conversation(adapter, monkeypatch):
|
||||
"""agent.run_conversation() can legitimately raise a RuntimeError
|
||||
unrelated to provider auth (e.g. run_agent.py's "Failed to recreate
|
||||
closed OpenAI client"). A bare `except RuntimeError` around the whole
|
||||
_create_agent()+run_conversation() span would mislabel it as
|
||||
"Provider authentication failed". Only _ProviderAuthResolutionError —
|
||||
raised exclusively inside _create_agent() at the
|
||||
_resolve_runtime_agent_kwargs() call site — may trigger the controlled
|
||||
response; this unrelated RuntimeError must propagate unhandled."""
|
||||
class _FakeAgent:
|
||||
def run_conversation(self, **kwargs):
|
||||
raise RuntimeError("Failed to recreate closed OpenAI client")
|
||||
|
||||
monkeypatch.setattr(adapter, "_create_agent", lambda **kwargs: _FakeAgent())
|
||||
|
||||
with pytest.raises(RuntimeError, match="Failed to recreate closed OpenAI client"):
|
||||
await adapter._run_agent(
|
||||
user_message="hello",
|
||||
conversation_history=[],
|
||||
session_id="request-session",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_chat_surfaces_controlled_response_on_provider_auth_failure(auth_adapter, session_db, monkeypatch):
|
||||
"""End-to-end: POST /api/sessions/{id}/chat previously had zero wrapping
|
||||
around _run_agent() — an unhandled RuntimeError produced a raw aiohttp
|
||||
500 with no JSON body. Must now return 200 with the controlled error
|
||||
message as the assistant content. Exercises the real
|
||||
gateway.run._resolve_runtime_agent_kwargs() boundary, not a mocked
|
||||
_create_agent()."""
|
||||
session_id = session_db.create_session("auth-fail-session", "api_server")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._resolve_runtime_agent_kwargs",
|
||||
lambda: (_ for _ in ()).throw(RuntimeError("Auth failed: token expired")),
|
||||
)
|
||||
|
||||
app = _create_session_app(auth_adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
resp = await cli.post(
|
||||
f"/api/sessions/{session_id}/chat",
|
||||
json={"message": "hi"},
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert resp.status == 200
|
||||
payload = await resp.json()
|
||||
|
||||
assert payload["message"]["content"] == "⚠️ Provider authentication failed: Auth failed: token expired"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue