[BREAKING] Python: Ensure session isolation for FHA invocation impl#7158
[BREAKING] Python: Ensure session isolation for FHA invocation impl#7158TaoChenOSU wants to merge 2 commits into
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR updates the Python Foundry Hosted Agents Invocations hosting implementation to derive session partitioning from the Foundry request context (session/user IDs) to improve session isolation in hosted scenarios, and adds tests and sample documentation updates around invocations + streaming.
Changes:
- Add
_partition_key()to compute a session cache key using Foundry request context (and fail fast when hosted protocol context is missing). - Update
_handle_invoke()to key the in-memory session cache by the partition key rather thanrequest.state.session_id. - Add unit + integration tests for invocations, and update the basic invocations sample README with a streaming curl example.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/samples/04-hosting/foundry-hosted-agents/invocations/01_basic/README.md | Documents a streaming invocation example alongside the existing non-streaming example. |
| python/packages/foundry_hosting/agent_framework_foundry_hosting/_invocations.py | Introduces partition-key session isolation based on Foundry request context; updates invocation handler behavior. |
| python/packages/foundry_hosting/tests/test_invocations.py | Adds unit tests covering partition-key behavior and invocation handler behavior (streaming/non-streaming). |
| python/packages/foundry_hosting/tests/test_invocations_int.py | Adds integration tests exercising the ASGI pipeline against a real Foundry endpoint (skipped unless env vars are provided). |
| @@ -67,7 +67,7 @@ async def stream_response() -> AsyncGenerator[str]: | |||
| ) | |||
|
|
|||
| response = await agent.run([user_message], session=session, stream=stream) | |||
| return JSONResponse({"response": response.text}) | |||
| return Response(content=response.text) | |||
There was a problem hiding this comment.
Following on the comment above about using a structured key to avoid collisions, should this sample use the same per-user session isolation as InvocationsHostServer? It currently looks up sessions using only the caller-provided agent_session_id, so another user who reuses that ID could access and modify the same AgentSession. Could the cache key include both the session ID and user ID from the request context?
Motivation & Context
Currently, the light-weight Foundry Hosting Agent (FHA) invocation integration stores live AgentSession objects in a process-wide dictionary keyed only by the
session_id, which could potentially allow cross-user invocation.Description & Review Guide
This PR ensures that the sessions are key by the user id injected by the platform when the agent is hosted in FHA., eliminating the possibility that one user can interfere with another user's session even if the session id is exposed.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.