Repository navigation
fix(messages): preserve Anthropic container field - #916
Conversation
Accept the top-level container value on Messages requests and pass it through unchanged to the upstream provider. Update the published OpenAPI schema and add schema and dispatch coverage.
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. WalkthroughThe messages route now supports optional ChangesMessages routing behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to The new Messages endpoint forwarding supports string container IDs but rejects valid object-form container requests, preventing affected clients from using the compatible API shape. Update the request type and add forwarding coverage before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description follows the repository template, explains the change, identifies it as a bug fix, links a relevant issue, records completed checklist items, documents the known unrelated test failure, and completes the AI usage section. Full details: Docstring CoverageExplanation Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 7 files. (1 skipped: 1 too large.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GitHub Actions
Inactive
There was a problem hiding this comment.
🟢 Approval recommended
The change is small and targeted, updates the published contract and generated client, and includes unit/integration tests that verify validation, serialization, and upstream forwarding.
Pull request overview
Adds a compatibility container field to the /v1/messages Anthropic-compatible request model so that clients can send Anthropic’s top-level container value and have it forwarded unchanged upstream, while any-llm’s typed MessagesParams is still missing the field.
Changes:
- Extend
MessagesRequestwith an optionalcontainer: str | Nonecompatibility field. - Regenerate and commit OpenAPI + generated dashboard API client typing for the new field.
- Add unit + integration coverage to ensure the field survives validation/serialization and reaches the provider call kwargs.
File summaries
| File | Description |
|---|---|
src/gateway/api/routes/messages.py |
Adds container to MessagesRequest so it’s preserved through request dumping and upstream forwarding. |
tests/unit/test_request_schema_derivation.py |
Verifies container survives validation/serialization and is present in the published OpenAPI schema. |
tests/integration/test_messages_route_dispatch.py |
Ensures container reaches the patched amessages provider call unchanged. |
docs/public/openapi.json |
Updates the published API contract to include the nullable container field on MessagesRequest. |
web/src/client/schema.ts |
Regenerates the dashboard client schema types to include `container?: string |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Generated artifacts check out at this head: make openapi-check and make postman-check both pass (the collection is unaffected because /v1/messages uses a curated body override), and web/src/client/schema.ts is regenerated.
One should-fix inline: a hand-declared param forwarded through any-llm's **kwargs also has to be registered in _FORWARDED_PARAMS, or its rejection by a bridged provider is misclassified as a 502.
🤖 Review written by Claude Code (Opus 5).
…he code container rides any-llm's **kwargs, so a provider that bridges Messages through Chat Completions hands it to an SDK method with no such parameter and raises the unknown-keyword TypeError. Register it in _FORWARDED_PARAMS beside service_tier so that reaches the caller as a 400 naming the param rather than a 500 in standalone, or a 502 in hybrid after every attempt in the route is burned and reported as an unknown upstream failure. Drop the field when otari_code_execution put the sandbox in charge: the tool is gone from the forwarded request, so the provider would be asked to attach a container nothing in this request can reach. Name the upstream PR and the shadowing in the field comment, matching what service_tier carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
…a shared account A request carrying otari_code_execution alongside a provider-native code-execution tool asked two separate sandboxes to run code, and the gateway resolved it silently: it ran the code itself, forwarded the native tool, and dropped the container the caller named, so the provider minted a fresh one. Neither half of the caller's state lived where they could address it. Refuse the combination in prepare_gateway_tools, beside the mcp_servers conflict it already refuses. Matched on the tool type alone, so a caller's own function named code_execution is still theirs to dispatch. A container id addresses an execution environment and the files uploaded into it, scoped to the provider account that minted it rather than to an otari tenant. A managed hybrid attempt runs on a credential the platform owns and many workspaces share, so forwarding an id the caller picked let whoever held one resume another tenant's container. scope_prompt_cache_key solves the same problem for cache keys by namespacing them; a provider-minted id cannot be namespaced, so refuse instead. Refused when any attempt on the route is managed, since fallback picks the attempt after this point. BYO routes are unaffected, and standalone credentials are the operator's own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
njbrake
left a comment
njbrake
left a comment
There was a problem hiding this comment.
Approving. container now carries the four things service_tier carries: the _FORWARDED_PARAMS registration, a tracking issue (#924), the shadowing note, and a line in the docstring.
Two behavior changes landed on top of the original fix, worth naming for anyone reading the squash later.
otari_code_execution alongside a provider-native code-execution tool is now a 400. It previously resolved silently: the gateway ran the code, forwarded the native tool, and dropped the container the caller named. The check sits in prepare_gateway_tools, so it applies to chat and responses as well.
A caller-supplied container is refused on a hybrid route with any managed attempt. That account is shared across workspaces, and a provider-minted id cannot be namespaced the way scope_prompt_cache_key namespaces a cache key.
Full suites and all 16 checks green.
Note: this review was drafted by Claude Opus 5 via back-and-forth with @njbrake. The reasoning and decisions are his; the prose is Claude's.
* fix(messages): adopt any-llm container support Upgrade any-llm-sdk to 1.27.0 now that MessagesParams declares container. Remove the temporary local field and classifier scaffolding introduced in #916 while keeping the routing and tenant-isolation protections.\n\nRefresh generated schemas and adapt tests to the newer Anthropic SDK types. * test(messages): cover the container rejection path after the any-llm bump any-llm 1.27 guards container in _amessages before the Chat Completions bridge, so a bridged provider now raises NotImplementedError instead of the unknown-keyword TypeError the deleted test asserted. The classification it owes is unchanged (400 naming the param, not a generic 502), so pin the new mechanism beside the context_management sibling that shares the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(api): classify httpx2 transport failures from the Anthropic SDK The any-llm 1.27 bump raises its floor to anthropic>=0.119.0, which resolves to anthropic 1.3.0 and moves that SDK's transport from httpx to httpx2. The two exception hierarchies are disjoint: httpx2.TimeoutException is not a subclass of httpx.TimeoutException, and neither TimeoutException nor ConnectError is caught by the class-name fallback, which matches only *TimeoutError and *ConnectionError. Both SDKs wrap only the initial send, so a read timeout or dropped connection while the SSE body is already streaming propagates as the client's own raw exception through any-llm's unguarded async for over the stream. Those reached upstream_exception_shape as httpx.ReadTimeout before the bump and as httpx2.ReadTimeout after it, so a real provider timeout settled as the generic 502 an outage gets instead of a 504, and in hybrid it classified as "unknown" and stopped driving fallback. Match both clients in the timeout and connection arms. Tests cover the raw httpx2 shapes on the client-facing status, the streaming settle path, and the hybrid fallback decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(deps): adopt any-llm 1.27.1 Use the any-llm patch release that constrains Anthropic below v1, resolving the SDK to 0.125.0.\n\nRemove the interim Anthropic httpx2 classification paths and restore the timeout tests to the httpx transport used by the supported SDK. * test(api): address provider error review feedback --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Description
Accept Anthropic's top-level
containervalue on/v1/messagesand forward it unchanged to the selected provider. The request field is declared locally as a compatibility measure until the corresponding any-llm release is available; a follow-up will derive it fromMessagesParamsafter that dependency is updated.The OpenAPI schema and route-level tests cover request validation, serialization, and provider forwarding.
PR Type
Relevant issues
Related to mozilla-ai/octonous#4913
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).make lint,make typecheck, the focused tests, generated-artifact checks, and the OSS smoke test pass. The full suite has one unrelated existing failure intests/unit/test_docs_style.pybecausedocs/superpowers/plans/2026-08-20-inline-platform-response-cost.mdcontains em dashes.AI Usage
AI Model/Tool used: OpenAI GPT-5.6 SOL with Pi
Any additional AI details you'd like to share: The implementation and PR preparation were completed with an AI coding agent and reviewed against the repository guidance.
NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not our AI :)
Summary
containerfield in/v1/messages.containerfor compatible provider requests and removed it when Otari manages code execution.