feat(sdk): advertise Vercel messages protocol headers#4769
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
dosubot
Bot
added
python
Reviewer guide: interesting code
|
| def set_vercel_message_protocol_headers(response: Response) -> Response: | ||
| """Stamp the default agent ``/messages`` protocol identity on an HTTP response.""" | ||
| for key, value in VERCEL_MESSAGE_PROTOCOL_HEADERS.items(): | ||
| response.headers.setdefault(key, value) |
There was a problem hiding this comment.
setdefault is the load-bearing choice here: it lets a future path stamp a different format/version (or a non-Vercel adapter reuse this helper) without this overwriting it. Worth a one-line comment so nobody 'fixes' it to a plain assignment.
|
|
||
| except Exception as exception: | ||
| return await handle_failure(exception) | ||
| return set_vercel_message_protocol_headers(await handle_failure(exception)) |
There was a problem hiding this comment.
The exception path is the easiest exit to miss: handle_failure already builds the response, so wrapping it here is the only way these headers reach an error reply. A new early-return added above this except would silently ship without the protocol identity.
| response = await endpoint(None, LoadSessionRequest(session_id="sess_abc")) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert response.headers["x-ag-messages-format"] == VERCEL_MESSAGE_PROTOCOL |
There was a problem hiding this comment.
Good that the direct load-session test asserts the literal header strings rather than the constants. That pins the wire values, so a rename of the constant can't silently change the contract a client depends on.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdks/python/agenta/sdk/agents/adapters/vercel/routing.py (1)
134-134: ⚡ Quick winUse the exported protocol constant instead of a string literal.
Line 134 hardcodes
"vercel"while the protocol identity is already centralized inVERCEL_MESSAGE_PROTOCOL. Reusing the constant avoids silent drift between stream behavior and stamped headers if the protocol token ever changes.Suggested patch
- return set_vercel_message_protocol_headers( - make_stream_response(response, "vercel") - ) + return set_vercel_message_protocol_headers( + make_stream_response(response, VERCEL_MESSAGE_PROTOCOL) + )sdks/python/oss/tests/pytest/utils/test_messages_endpoint.py (1)
244-256: ⚡ Quick winAdd explicit 406-path header tests.
The suite now covers 200/400/500 and
/load-session, but the two not-acceptable branches (stream requested + batch returned, and JSON requested + stream returned) still aren’t asserted for protocol headers. Adding both tests would close the main regression gap for this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6c349da-9433-4508-b735-b097fe85d658
📒 Files selected for processing (3)
sdks/python/agenta/sdk/agents/adapters/vercel/__init__.pysdks/python/agenta/sdk/agents/adapters/vercel/routing.pysdks/python/oss/tests/pytest/utils/test_messages_endpoint.py
Railway Preview Environment
Updated at 2026-06-19T16:30:17.099Z |
|
Superseded. Replacing the path-based stack with PRs sliced by functional area showing final code only, so reviewers don't comment on intermediate scaffolding that a later PR rewrites. See the new set. |
1 participant
This PR is part of a stack. Review bottom-up.
Each PR's diff is only its own delta. Merge from the bottom. This PR's base is #4768 (merge that first).
Context
The agent
/messagesendpoint speaks the Vercel UIMessage wire format, but the response never said so. A client had to assume the format and version out of band. This PR makes the server declare both on every response, so a client can read them and branch on what the server actually speaks.Base branch:
feat/agent-load-session-store-port. This is the Protocol Shell Hardening slice (#2) fromdocs/design/agent-workflows/pr-stack.md, which makes/messagesreviewable as an HTTP contract.What this changes
The endpoint now stamps two response headers:
x-ag-messages-format: vercelx-ag-messages-version: v1A new helper,
set_vercel_message_protocol_headers, applies both withsetdefault, so it never clobbers a header a path already set. It wraps every response the adapter returns:/messagessuccess JSON (batch response)/messagessuccess SSE (Vercel stream)/messages400for an invalidsession_id/messagesupstream error JSON (status code at or above 400)/messages406not-acceptable, for both the stream and batch type mismatches/messagesexception path (handle_failure)/load-sessionJSONBefore, a 400 looked like:
After:
The header constants, the headers map, and the helper are also exported from
vercel/__init__.pyso callers and tests can reference them by name instead of hardcoding strings.Key architectural decision to review
The decision to scrutinize is versioning the wire protocol through response headers rather than the response body. The server names the format (
vercel) and a version (v1) on the envelope, so a client can detect both before it parses anything. The body stays a pure payload.The tradeoff is that header stamping is opt-in per return path. Nothing in FastAPI forces a
/messagesresponse through the helper. Eachreturnmust wrap its response by hand.set_vercel_message_protocol_headerslives inrouting.pyand the call sites are spread acrossmake_messages_endpointandmake_load_session_endpoint. A middleware would stamp every response unconditionally, but it would also touch responses from other routes and would need to know which routes are agent message routes. The per-return approach keeps the blast radius to this adapter at the cost of a contributor remembering to wrap each new exit.How to review this PR
Read
sdks/python/agenta/sdk/agents/adapters/vercel/routing.pyfirst.setdefaultis intentional, so a path that needs a different format or version can override it.returninmake_messages_endpoint(lines 84 to 146) and confirm each one passes through the helper. The 400, the upstream-error JSON, both not-acceptable branches, the stream, the success JSON, and theexceptall wrap.make_load_session_endpoint(lines 167 to 169) wraps its only return.vercel/__init__.py.The likely regression is a new
/messagesreturn path that forgets the helper, so that one response ships without the headers and a client misreads the format. The not-acceptable and exception paths are the easiest to overlook, since they read like error plumbing rather than protocol surface.Tests / notes
sdks/python/oss/tests/pytest/utils/test_messages_endpoint.pyadds a_assert_vercel_message_protocolcheck and asserts the headers on the success JSON, the echoed-session response, the SSE stream, the pre-stream 500, the invalid-session 400, and both/load-sessionpaths (route and direct endpoint). The direct/load-sessiontest asserts the literal header values, which pins the constants. No assertion yet covers the two not-acceptable (406) branches.