fix(api-server): unblock /v1/chat/completions with narrow kwargs-collision fix - #31139
fix(api-server): unblock /v1/chat/completions with narrow kwargs-collision fix#31139Koraji95-coder wants to merge 2 commits into
Conversation
…ision fix
The `api_server` platform's `_create_agent()` builds `runtime_kwargs` via
`_resolve_runtime_agent_kwargs()` (which sources `model` from
config.yaml among other things), then constructs `AIAgent(model=model,
**runtime_kwargs)` — which collides on the `model` key and raises
`TypeError: AIAgent() got multiple values for keyword argument 'model'`
before any request can be served. Every `POST /v1/chat/completions`
500s regardless of payload, making the OpenAI-compat API server
unusable for any client.
Repro (Windows + Linux both, hermes-agent v0.14.0):
~/.hermes/.env:
API_SERVER_ENABLED=true
API_SERVER_KEY=test-key
hermes gateway
curl -H "Authorization: Bearer test-key" \
-H "Content-Type: application/json" \
http://localhost:8642/v1/chat/completions \
-d '{"model":"hermes-agent","messages":[{"role":"user","content":"x"}]}'
HTTP/1.1 500 Internal Server Error
{"error": {"message": "Internal server error: run_agent.AIAgent() got
multiple values for keyword argument 'model'", ...}}
The fix is one line: pop `model` out of `runtime_kwargs` before the
constructor, so the explicit `model=model` kwarg wins unambiguously.
Semantics-preserving relative to the existing implicit behavior — the
request-level `model` field is still ignored in favor of config.yaml's
`model`, matching the current state of NousResearch#10773 (the per-request-model
design still being worked out across NousResearch#25552 / NousResearch#16403 / NousResearch#18549 / NousResearch#5862).
Once NousResearch#10773 lands, this `pop` gets replaced by whatever per-request
routing it specifies; until then, this restores a working API server
for any user out there.
Strictly less than NousResearch#25552 et al. — no per-request routing, no new
config surface, no behavior change for anyone whose request payload
already happened to match the server-side model. Just fixes the crash.
Validated end-to-end on two production deployments running v0.14.0
behind a Tailscale-mesh reverse proxy: `POST /v1/chat/completions`
returns `200` with a real OpenAI Chat Completion response body (real
agent tool-calls, real responses, `finish_reason: stop`) where before
this fix every request 500'd within ~50ms.
See NousResearch#10773 (root issue) for the broader design conversation. Pinged
the maintainer team in the comment thread linking back to this PR.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR prevents a runtime keyword-argument collision when constructing AIAgent, avoiding a TypeError that currently causes 500s on POST /v1/chat/completions.
Changes:
- Remove (
pop) any pre-existingmodelentry fromruntime_kwargsto prevent passingmodeltwice toAIAgent.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Defer to the server-side `config.yaml` model rather than letting | ||
| # the kwarg below collide with a `model` key already populated in | ||
| # `runtime_kwargs`. Without this pop, the explicit `model=model` | ||
| # on the `AIAgent(...)` constructor a few lines down raises | ||
| # `TypeError: AIAgent() got multiple values for keyword argument | ||
| # 'model'` and 500s every `POST /v1/chat/completions` regardless | ||
| # of payload. See #10773 for the per-request-model design; this is | ||
| # the narrow band-aid only. | ||
| runtime_kwargs.pop('model', None) | ||
| reasoning_config = GatewayRunner._load_reasoning_config() | ||
| model = _resolve_gateway_model() |
There was a problem hiding this comment.
Considered both of your suggestions (a) 4xx-on-mismatch and (b) drop-only-when-matching, and explicitly picked the unconditional pop for this PR. Two reasons:
-
Precedence semantics is the design conversation at feat(api-server): honor request-level model field for per-request model selection #10773. The competing PRs (fix(api-server): honor chat completions request model #25552, feat(api-server): honor X-Router-Model for per-request model override #16403, feat(api_server): honor inbound 'model' and 'provider' fields for per-request routing #18549, feat(gateway): add per-request model routing to API server #5862) are wrestling with exactly what the request-level
modelfield should mean — error, override, fallback, mismatch-warn. This PR is the band-aid that unblocks/v1/chat/completionsfor everyone while that design lands. Implementing either suggestion (a) or (b) here would mean picking a winner in the design debate, which is exactly what I don't want this PR to do. -
Failing-loud on caller-provided
modelwould surprise existing API consumers. Before the kwargs collision was ever triggered, callers could already sendmodelin their request — the server-sideconfig.yamlvalue won by virtue of the explicitmodel=modelkwarg in the constructor, and the caller's value was silently ignored. That's the current implicit behavior of every working deployment that happens to not hit the collision. The narrow pop preserves that behavior exactly.
Added a regression test in tests/gateway/test_api_server.py::TestCreateAgent::test_create_agent_pops_colliding_model_from_runtime_kwargs (commit 86d349a) that locks in the no-TypeError behavior, mirroring the existing test_create_agent_forwards_config_reasoning_effort pattern in the same class.
When #10773's full design lands, the pop gets replaced by whatever precedence semantics it specifies, and the test gets updated alongside. If the maintainer's strong preference is option (b) precedence-aware behavior here rather than at #10773, happy to rework.
Adds `test_create_agent_pops_colliding_model_from_runtime_kwargs` to
`tests/gateway/test_api_server.py::TestCreateAgent` covering the exact
bug this PR fixes. The test mounts a `FakeAgent` via monkeypatch and
puts a colliding `model` key inside `runtime_kwargs`, mirroring the
real-world shape that triggered the original `TypeError: AIAgent() got
multiple values for keyword argument 'model'`.
Without the `runtime_kwargs.pop('model', None)` fix at api_server.py
line 881, this test fails with the same TypeError that 500'd every
`POST /v1/chat/completions` in production. With the fix, the test
passes and asserts (a) the constructor receives exactly one `model`
value, sourced from `_resolve_gateway_model()` not from the colliding
key, and (b) other `runtime_kwargs` keys (e.g. `provider`) come
through unaffected.
Pattern matches the existing
`test_create_agent_forwards_config_reasoning_effort` test in the same
class — same `FakeAgent` mock shape, same monkeypatch targets, same
`adapter._create_agent` invocation. Hermetic; no live network, no
filesystem, no API keys.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Closing as duplicate of #27678 — thanks @alt-glitch for the catch. @briandevans's PR (filed 2026-05-18) implements the exact same Transferring our two-Windows-box production evidence to #27678 as a comment so it can help push that PR through. The regression test I added in this PR ( |
What changed and why
gateway/platforms/api_server.py's_create_agent()buildsruntime_kwargsvia_resolve_runtime_agent_kwargs()(which sourcesmodelfromconfig.yamlamong other settings), then a few lines later constructsAIAgent(model=model, **runtime_kwargs). The explicitmodel=modelcollides with themodelkey already populated inruntime_kwargs, raising:…before any request is ever served. The API server platform is therefore unusable for any OpenAI-compatible client — every
POST /v1/chat/completionsreturns HTTP 500 with that exact error.This PR adds one line —
runtime_kwargs.pop('model', None)— right after_resolve_runtime_agent_kwargs()returns, so the explicitmodel=modelkwarg below wins unambiguously. The request-levelmodelfield is ignored in favor of the server-sideconfig.yamlvalue, which matches the current (broken-by-the-crash) implicit behavior.Strictly less scope than #25552 / #16403 / #18549 / #5862. No per-request routing, no new config surface, no behavior change for anyone whose payload already matched the server-side model. Just unblocks the 500 while the broader per-request-model design at #10773 gets settled. When #10773's full design lands, this pop gets replaced by whatever per-request routing it specifies.
How to test
Reproduction (any OS, hermes-agent v0.14.0):
Before this PR: HTTP 500 within ~50ms regardless of payload:
{ "error": { "message": "Internal server error: run_agent.AIAgent() got multiple values for keyword argument 'model'", "type": "server_error", "param": null, "code": null } }After this PR: HTTP 200 with a real OpenAI-shaped Chat Completion:
{ "id": "chatcmpl-f67cabfe3f6146b1a870ab6b5ac54", "object": "chat.completion", "created": 1779567828, "model": "hermes-agent", "choices": [{ "index": 0, "message": {"role": "assistant", "content": "pong"}, "finish_reason": "stop" }], "usage": {"prompt_tokens": 14816, "completion_tokens": 29, "total_tokens": 14845} }What platforms
Tested on:
/v1/chat/completionsthrough a Tailscale-mesh reverse proxy (Basic Auth at edge, Bearer auto-injected for/v1/*paths). Real agent invocation, real tool-calls,finish_reason: stop, 33-second end-to-end response.200+ matching response shape.No Linux smoke from me personally — the kwargs collision is purely a Python-level constructor argument issue, not platform-specific, but flagging the gap for completeness.
Related issues
feat(api-server): honor request-level model field for per-request model selection. This PR is strictly less than what feat(api-server): honor request-level model field for per-request model selection #10773 asks for; it just unblocks the crash so the broader design can land cleanly later.modelfield in /v1/chat/completions #24008 — closed as duplicate of feat(api-server): honor request-level model field for per-request model selection #10773.AI usage disclosure