Forward the selected persona through the chat proxy - #1368
Merged
Conversation
The proxy rebuilds the request field by field rather than forwarding the body, so a persona named by the UI never reached the agent service: the selector would have appeared to work and changed nothing. Naming the fields is deliberate and worth keeping -- user_id is taken from the authenticated request, and forwarding the body wholesale would let a caller set it and read another user's sessions. Both properties now have a test; the persona one fails without the added line.
ADKSession declares the fields the UI receives, and anything the agent service sends that is not declared is discarded on the way out. It reports model and persona per session; neither was declared, so both arrived as undefined. That silently broke the per-session model restore added with the model picker: switching sessions always fell back to the deployment default rather than showing what the conversation had actually been answered with. Persona would have landed the same way. This is the third field to go this way after createTime and title, so the model now says so.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 3 groundwork for agent personas. Found while wiring the UI selector, not by reading the diff.
The problem
/api/v2/agents/streamdoes not forward the request body. It rebuilds it:A
personasent by the UI stops here. The selector would have rendered, changed the request, and had no effect whatsoever — the failure mode that looks like a model bug rather than a plumbing one.Why the allowlist stays
The obvious fix is to forward
messagewholesale. That would be a security regression:user_idis taken from the authenticated request, and a caller who could set it in the body would read another user's sessions. The shape is right; it was just missing a field. Added a comment so the next person does not "simplify" it.Tests
Two, and both properties were previously unguarded:
test_stream_forwards_the_selected_persona— verified it fails without the one-line change (1 failed, 3 passed), so it is not vacuous.test_stream_cannot_be_told_which_user_it_is— postsuser_id: "someone-else"and asserts the authenticated username is what goes out.The mock scaffolding is the fiddly part and is documented in
_mock_streaming_client:httpx.AsyncClient(...)and.stream(...)are both context managers but only the client's is async.stream()is called synchronously and returns the async one, so anAsyncMockin that position produces an un-awaited coroutine instead of a response — which is exactly how the first attempt failed.Note
Committed with
--no-verify. Thepre-commithook shells out to a host-sideuv run ruff, which cannot read this repo's container-created.venv(Permission deniedon.venv/bin/python3). Ran the hook's exact commands insidedev-api-1instead — output above.Needed by yeti-platform/yeti-agents#12, which reads
personaoff the request.