[dolho]

Summary

PUT /api/agents/{name}/model caught every httpx.HTTPError in one branch and answered 503 "Failed to set model". When the agent rejected an invalid model with its own 400 (which names the valid models), the caller saw "service unavailable", lost the agent's message, and every typo wrote an ERROR line to trinity.errors.

The branch is now split (routers/chat.py::_raise_model_route_error):

• The agent answered with a 4xx. The caller gets that status and the agent's detail, or a short non-JSON body. Logged at INFO.
• The agent couldn't be reached, timed out, or answered 5xx. Still 503 with the same text, logged at ERROR.

GET /api/agents/{name}/model had the same catch-all shape and gets the same split, as the issue's technical notes suggest.

Fixes #3677

Tests

• tests/unit/test_3677_set_model_agent_error.py (13 cases) drives both handlers against a real httpx.MockTransport agent. Covered:

  • 400 passthrough for PUT and GET;
  • 422 with a structured detail;
  • a non-JSON 4xx;
  • a 5xx, a connect error and a read timeout each still give 503;
  • a valid model returns 200 unchanged;
  • no ERROR log line for a rejection, and still one for an unreachable agent.

  On dev, five of these fail exactly as the issue describes (503 instead of 400/422, plus the ERROR line).

• tests/test_agent_chat.py::test_set_invalid_model_rejected no longer skips on 503. That skip hid the 400 path.

• 632 passed across the model, chat-router, census, human-only and Default model claude-opus-5-5 fails on cached Claude Code 2.1.278, and the rejection is misclassified as auth (seat rotation + API-key fallback) #3012 unit tests.

Live check (local instance, a real running agent):

• PUT .../model {"model":"invalid-model-xyz"} → 400 {"detail":"Invalid Claude model: invalid-model-xyz. Use aliases (sonnet, opus, haiku, fable) or full model names."};
• a valid model → 200;
• GET → 200;
• the log line is INFO ... refused to set model: 400.

🤖 Generated with Claude Code

[dolho]

/review — automated pre-landing review (at 27af348c5)

Branch: fix/3677-set-model-agent-400 → dev (merge-base e0cabc5af)
Files Changed: 4 (routers/chat.py, the integration test, a new unit test, a learnings fragment)
Scope: CLEAN. Only the two model routes the issue names (PUT, plus GET from its technical notes) are touched.
Plan Completion (issue acceptance criteria): 4 done / 0 partial / 0 not done / 1 unverifiable

• DONE: an agent 4xx comes back with its own status and detail (chat.py::_raise_model_route_error).
• DONE: a transport error or timeout is still 503.
• DONE: an agent 4xx is logged at INFO, not ERROR.
• DONE: a unit test covers both branches (tests/unit/test_3677_set_model_agent_error.py).
• UNVERIFIABLE here: the integration test test_set_invalid_model_rejected passing against a running agent. Its 503 skip is removed. The same request was run live against a running agent and returned 400 with the agent's sentence, but the integration suite itself was not run.

Execution coverage

changed symbol	executed by	live consumer	verdict
_raise_model_route_error	unit test, 15 cases, real httpx.MockTransport agent	set_agent_model, get_agent_model (routers/chat.py); the frontend model selector and MCP set_agent_model	✅ executed
test_agent_chat.py::test_set_invalid_model_rejected	integration suite (live stack)	—	✅ the 503 skip that hid the path is removed

Fix mutation: with dev's handler swapped back in, 5 of the 13 original cases go red: 400 on PUT and GET, the 422, the non-JSON 4xx, and the "no ERROR line" case. The new 401 guard (below) has its own mutation. Dropping != 401 turns 2 cases red.

Critical findings

[C1] Auth boundary / session: an agent's 401 relayed to the browser signs the person out. (Confidence: 8/10, fixed in ec8eadf20)

• File: src/backend/routers/chat.py::_raise_model_route_error
• Evidence:
  • The first version passed every 4xx through (if response is not None and 400 <= response.status_code < 500:).
  • The agent server's auth middleware answers JSONResponse({"detail": "Unauthorized"}, status_code=401) (agent_server/middleware/auth.py:85) when the backend's per-agent token is refused (security: agent-server HTTP API is unauthenticated on the shared agent network (cross-agent credential theft) #1159).
  • utils/platformSession.js::sessionLostVerdict returns 'logout' for any 401 on an ordinary page while a token is stored.
• Why: with a rotated AGENT_AUTH_SECRET or a pre-security: agent-server HTTP API is unauthenticated on the shared agent network (cross-agent credential theft) #1159 container, opening that agent's page would have signed the user out. GET /model runs as the page loads.
• Fix: an agent 401 is the platform's own credential failing. It stays a 503 with an ERROR line. Two new tests cover it, for PUT and GET.

No other critical findings.

Informational findings

[I1] Incomplete fix: five sibling routes in the same router keep the catch-all shape. (Confidence: 6/10)

• File: src/backend/routers/chat.py: GET /{name}/chat/history, DELETE /{name}/chat/history, GET /{name}/chat/session, GET /{name}/activity/{tool_id}, DELETE /{name}/activity.
• Issue: each one turns every agent httpx.HTTPError, including a 4xx, into 503. Those calls take no caller input the agent would reject, so a 4xx is unlikely; the issue scoped the model routes only.
• Suggestion: if any of them gains a validated input, reuse _raise_model_route_error, after renaming it to fit a broader use.

[I2] An agent 404 now passes through. (Confidence: 5/10, appendix-level)

• An agent image without /api/model would now answer the caller 404 instead of 503. That's still honest (the route is missing on that agent), and it can't be used to probe which agents exist: the caller is already past get_authorized_agent.

Clean categories

• SQL / data: no DB access in the diff.
• Credentials: the relayed detail is the agent's own error text for a model change. No token or .env value is on that path, and the 401 body (the only auth-related answer) is no longer relayed.
• Concurrency: no shared state.
• Enterprise disclosure: no public docs changed apart from a learnings fragment about an OSS route.
• Enum completeness: no new enum values.

Learnings

I added docs/memory/learnings/2026-10-19-agent-status-passthrough-401.md: when relaying an agent server's status code, never relay its 401.

Summary

• Critical: 1, found and fixed in this PR (ec8eadf20), with a mutation-proven test.
• Informational: 1, plus 1 in the appendix.
• Scope: clean.

🤖 Generated with Claude Code

[vybe]

merge-train: batch validated on train/20261019-1651 (train PR #3818 green on Tier 1 + Tier 2)