Propagate multi-user context (user_id, namespace_id, session_id) to Evolve MCP server#194
Draft
gjt-prog wants to merge 5 commits into
Draft
Propagate multi-user context (user_id, namespace_id, session_id) to Evolve MCP server#194gjt-prog wants to merge 5 commits into
gjt-prog wants to merge 5 commits into
Conversation
…) to Evolve MCP - Fix event_stream to populate local_state.user_id from auth context - Add user_id field to CugaLiteState for subgraph propagation - Extend EvolveIntegration.get_guidelines() and save_trajectory() signatures - Wire call sites in cuga_lite_graph.py and cuga_lite_node.py - Add 4 new tests for parameter propagation and backward compat (29/29 pass)
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds per-user context tracking to the Evolve integration by introducing a ChangesMulti-User Context in Evolve Integration
Sequence DiagramsequenceDiagram
actor User
participant Server as Server<br>(main.py)
participant Graph as CugaLite Graph<br>(cuga_lite_graph.py)
participant Node as CugaLite Node<br>(cuga_lite_node.py)
participant Evolve as EvolveIntegration<br>(integration.py)
User->>Server: Authenticated request
Server->>Graph: Initialize state with user_id
Graph->>Evolve: get_guidelines(task, user_id, namespace_id, session_id)
Evolve-->>Graph: Guidelines response
Graph->>Node: Process with user context
Node->>Evolve: save_trajectory(..., user_id, namespace_id, session_id)
Evolve-->>Node: Trajectory saved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cuga/backend/server/main.py (1)
1177-1181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist
user_idinto the checkpoint on resume flows.When
resumeis truthy,run_stream(..., state=None, resume=resume)ignores the patchedlocal_state, so this assignment never reaches the graph state for resumed/HITL conversations. Older threads will still hit Evolve withoutuser_id.Suggested fix
if local_state: from cuga.config import get_service_instance_id, get_tenant_id local_state.service_scope = {"tenant_id": get_tenant_id(), "instance_id": get_service_instance_id()} local_state.user_id = user_id # Propagate authenticated user into graph state + if resume and thread_id: + run_agent.graph.update_state( + {"configurable": {"thread_id": thread_id}}, + { + "service_scope": local_state.service_scope, + "user_id": local_state.user_id, + }, + ) if os.getenv("CUGA_DEMO_MODE") == "health" and not local_state.pi:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/server/main.py` around lines 1177 - 1181, The patched local_state.user_id isn't propagated when resume flows are used because run_stream is invoked with state=None; fix by ensuring the resume checkpoint carries the same fields you set on local_state: when resume is truthy, copy service_scope and user_id into the resume/checkpoint object (e.g., set resume.checkpoint.service_scope = {"tenant_id": get_tenant_id(), "instance_id": get_service_instance_id()} and resume.checkpoint.user_id = user_id) or alternatively call run_stream with state=local_state instead of None; update the logic around local_state, run_stream(..., state=...), and the resume/checkpoint handling so resumed/HITL conversations include user_id.
🧹 Nitpick comments (1)
src/cuga/backend/evolve/tests/test_integration.py (1)
153-177: ⚡ Quick winAdd one resume-path regression test for context propagation.
These tests only prove
EvolveIntegrationshapes its args correctly. They would not catchevent_stream(..., resume=...)droppinguser_idbefore Evolve is reached, which is the easiest place for this feature to regress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cuga/backend/evolve/tests/test_integration.py` around lines 153 - 177, Add a regression test that exercises the resume-path context propagation by calling event_stream(..., resume=...) with user_id/namespace_id/session_id set and asserting EvolveIntegration._call_tool receives those fields; specifically, in src/cuga/backend/evolve/tests/test_integration.py add an async test (patching EvolveIntegration._call_tool as AsyncMock and settings similar to existing tests) that calls event_stream(..., resume={"user_id":"user-1","namespace_id":"tenant-a","session_id":"thread-99"}) or otherwise simulates the resume flow and then verifies mock_call_tool.assert_called_once_with("get_guidelines", {"task": "...", "user_id":"user-1","namespace_id":"tenant-a","session_id":"thread-99"}) so any regression that drops user context in event_stream's resume handling is caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/cuga/backend/server/main.py`:
- Around line 1177-1181: The patched local_state.user_id isn't propagated when
resume flows are used because run_stream is invoked with state=None; fix by
ensuring the resume checkpoint carries the same fields you set on local_state:
when resume is truthy, copy service_scope and user_id into the resume/checkpoint
object (e.g., set resume.checkpoint.service_scope = {"tenant_id":
get_tenant_id(), "instance_id": get_service_instance_id()} and
resume.checkpoint.user_id = user_id) or alternatively call run_stream with
state=local_state instead of None; update the logic around local_state,
run_stream(..., state=...), and the resume/checkpoint handling so resumed/HITL
conversations include user_id.
---
Nitpick comments:
In `@src/cuga/backend/evolve/tests/test_integration.py`:
- Around line 153-177: Add a regression test that exercises the resume-path
context propagation by calling event_stream(..., resume=...) with
user_id/namespace_id/session_id set and asserting EvolveIntegration._call_tool
receives those fields; specifically, in
src/cuga/backend/evolve/tests/test_integration.py add an async test (patching
EvolveIntegration._call_tool as AsyncMock and settings similar to existing
tests) that calls event_stream(...,
resume={"user_id":"user-1","namespace_id":"tenant-a","session_id":"thread-99"})
or otherwise simulates the resume flow and then verifies
mock_call_tool.assert_called_once_with("get_guidelines", {"task": "...",
"user_id":"user-1","namespace_id":"tenant-a","session_id":"thread-99"}) so any
regression that drops user context in event_stream's resume handling is caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8df50321-736b-47c9-9b55-6498a79845ea
📒 Files selected for processing (5)
src/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_graph.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_node.pysrc/cuga/backend/evolve/integration.pysrc/cuga/backend/evolve/tests/test_integration.pysrc/cuga/backend/server/main.py
Resolved conflict in cuga_lite_graph.py by: - Keeping filesystem/sandbox tools setup from main - Using refactored Evolve integration (build_evolve_special_instructions_extension) - Removing old EvolveIntegration.get_guidelines() implementation
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
2 participants
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.
Problem
ALTK-Evolve's MCP server already supports multi-user parameters (
user_id,namespace_id,session_id) acrossget_guidelines,save_trajectory, andget_entities. However, the CUGA agent never sends these values — all Evolve interactions appear as"default"user with no tenant or session context.Root causes:
event_stream()inserver/main.pyreceivesuser_idfrom the authenticated request but never assigns it tolocal_state.user_idCugaLiteStatelacks auser_idfield, so the subgraph cannot propagate user identityEvolveIntegration.get_guidelines()andsave_trajectory()don't accept or forward any context parametersChanges
server/main.pylocal_state.user_id = user_idafterservice_scopeis setcuga_lite_graph.pyuser_id: Optional[str]toCugaLiteStateshared keysevolve/integration.pyget_guidelines()andsave_trajectory()with optionaluser_id,namespace_id,session_idcuga_lite_graph.pyCugaLiteStatetoget_guidelines()callcuga_lite_node.pyAgentStatetosave_trajectory()callstests/test_integration.pyBehavior
get_guidelinesdoes not hard-filter by user/session; context is logged for attribution onlysave_trajectorystampsuser_id,namespace_id,session_idinto entity metadataNoneand are omitted from the MCP payload when not setTesting
29/29 unit tests pass (25 existing + 4 new).
Summary by CodeRabbit
New Features
Tests