Propagate multi-user context (user_id, namespace_id, session_id) to Evolve MCP server#194
Open
gjt-prog wants to merge 11 commits into
Open
Propagate multi-user context (user_id, namespace_id, session_id) to Evolve MCP server#194gjt-prog wants to merge 11 commits into
gjt-prog wants to merge 11 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds optional per-request user identifiers to state docs and threads normalized user_id, namespace_id, and session_id through memory, Evolve integration (get_guidelines/save_trajectory), graph/node saving (async or awaited), server propagation helper, and tests. ChangesMulti-User Context in Evolve Integration
Sequence DiagramsequenceDiagram
actor User
participant Server as Server (event_stream)
participant Graph as CugaLiteGraph (build / inject)
participant Node as CugaLiteNode (callback_node)
participant Evolve as EvolveIntegration
User->>Server: authenticated request
Server->>Graph: initialize local_state with user_id/thread_id/service_scope
Graph->>Evolve: get_guidelines(task, user_id, namespace_id, session_id)
Evolve-->>Graph: guidelines response
Node->>Evolve: save_trajectory(trajectory_data, task_id, user_id=..., namespace_id=..., session_id=...)
Evolve-->>Node: confirm saved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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 docstrings
🧪 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
…t in test_cuga_lite_graph_evolve_guidelines.py
Collaborator
Author
|
@CodeRabbit please review again |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_graph.py (1)
74-76: ⚡ Quick winRemove the duplicate
user_idfield declaration.
user_idis declared twice inCugaLiteState(Line 74 and Line 76). Keeping both invites drift in a shared state contract.Proposed cleanup
- user_id: Optional[str] = None - thread_id: Optional[str] = None - user_id: Optional[str] = None # Shared with AgentState; None means unset (no user context sent to Evolve) + user_id: Optional[str] = None # Shared with AgentState; None means unset (no user context sent to Evolve) + thread_id: Optional[str] = None🤖 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/cuga_graph/nodes/cuga_lite/cuga_lite_graph.py` around lines 74 - 76, CugaLiteState currently declares user_id twice; remove the redundant declaration so there is a single Optional[str] user_id field and keep thread_id as-is; update the class definition by deleting the duplicated "user_id: Optional[str] = None" entry (reference symbol: CugaLiteState and the field name user_id) to avoid drift in the shared state contract.
🤖 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.
Inline comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_node.py`:
- Around line 397-421: state.user_id is being forwarded directly into
_evolve_user_id and persisted via EvolveIntegration.save_trajectory, causing
sentinel placeholders like "default" to be saved; change the assignment logic
that sets _evolve_user_id so that if state.user_id equals known sentinel values
(e.g., the string "default" or other configured placeholders) it becomes None
instead of being forwarded, then pass that sanitized _evolve_user_id into
EvolveIntegration.save_trajectory (both async and await branches); also update
the related test test_callback_node_handles_missing_multi_user_params to assert
user_id is None rather than "default".
In `@src/cuga/backend/evolve/memory.py`:
- Around line 47-49: The current extraction of state IDs blindly forwards
sentinel values (e.g., "default") to Evolve; normalize placeholders by
converting known sentinels and empty values to None before calling Evolve
GUIDELINES. Update the variables _evolve_user_id, _evolve_namespace_id and
_evolve_session_id (the values derived from state.user_id, (state.service_scope
or {}).get('tenant_id'), and state.thread_id) to check for common sentinels like
"default", "anonymous", "unknown", empty string, or other project-specific
placeholders and set them to None when matched so Evolve doesn’t treat
placeholders as real identifiers.
In `@tests/unit/test_server_user_id_propagation.py`:
- Around line 8-135: Add a new unit test that calls event_stream (the function
that performs the real assignments) instead of directly mutating AgentState;
mock the agent/graph dependencies and patch cuga.config.get_tenant_id and
get_service_instance_id to known values, provide a simulated authenticated
user_id (e.g. via the same variable name user_id or request context used in
production), run event_stream to produce/emitted state, and assert the
returned/emitted AgentState (or the event payload) has user_id ==
"authenticated-user-123" and service_scope["tenant_id"] == "tenant-456" and
service_scope["instance_id"] == "instance-789". Ensure the test references
event_stream and AgentState so regressions in the real assignment are caught.
---
Nitpick comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_graph.py`:
- Around line 74-76: CugaLiteState currently declares user_id twice; remove the
redundant declaration so there is a single Optional[str] user_id field and keep
thread_id as-is; update the class definition by deleting the duplicated
"user_id: Optional[str] = None" entry (reference symbol: CugaLiteState and the
field name user_id) to avoid drift in the shared state contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9db9e906-f712-4078-a219-5ed50a11806f
📒 Files selected for processing (10)
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/cuga_graph/nodes/cuga_lite/tests/test_cuga_lite_graph_evolve_guidelines.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_cuga_lite_node.pysrc/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_cuga_lite_state.pysrc/cuga/backend/evolve/integration.pysrc/cuga/backend/evolve/memory.pysrc/cuga/backend/evolve/tests/test_integration.pysrc/cuga/backend/server/main.pytests/unit/test_server_user_id_propagation.py
Comment on lines
+47
to
+49
| _evolve_user_id = getattr(state, 'user_id', None) | ||
| _evolve_namespace_id = (getattr(state, 'service_scope', {}) or {}).get('tenant_id') or None | ||
| _evolve_session_id = getattr(state, 'thread_id', None) |
Contributor
There was a problem hiding this comment.
Normalize placeholder IDs before calling Evolve guidelines.
If state.user_id carries a sentinel like "default", this code forwards it as a real user identifier. That can misattribute guidelines context across unrelated callers.
Proposed fix
- _evolve_user_id = getattr(state, 'user_id', None)
- _evolve_namespace_id = (getattr(state, 'service_scope', {}) or {}).get('tenant_id') or None
- _evolve_session_id = getattr(state, 'thread_id', None)
+ raw_user_id = str(getattr(state, "user_id", "") or "").strip()
+ _evolve_user_id = None if raw_user_id in {"", "default"} else raw_user_id
+ _evolve_namespace_id = (
+ str((getattr(state, "service_scope", {}) or {}).get("tenant_id") or "").strip() or None
+ )
+ _evolve_session_id = str(getattr(state, "thread_id", "") or "").strip() or None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _evolve_user_id = getattr(state, 'user_id', None) | |
| _evolve_namespace_id = (getattr(state, 'service_scope', {}) or {}).get('tenant_id') or None | |
| _evolve_session_id = getattr(state, 'thread_id', None) | |
| raw_user_id = str(getattr(state, "user_id", "") or "").strip() | |
| _evolve_user_id = None if raw_user_id in {"", "default"} else raw_user_id | |
| _evolve_namespace_id = ( | |
| str((getattr(state, "service_scope", {}) or {}).get("tenant_id") or "").strip() or None | |
| ) | |
| _evolve_session_id = str(getattr(state, "thread_id", "") or "").strip() or None |
🤖 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/memory.py` around lines 47 - 49, The current
extraction of state IDs blindly forwards sentinel values (e.g., "default") to
Evolve; normalize placeholders by converting known sentinels and empty values to
None before calling Evolve GUIDELINES. Update the variables _evolve_user_id,
_evolve_namespace_id and _evolve_session_id (the values derived from
state.user_id, (state.service_scope or {}).get('tenant_id'), and
state.thread_id) to check for common sentinels like "default", "anonymous",
"unknown", empty string, or other project-specific placeholders and set them to
None when matched so Evolve doesn’t treat placeholders as real identifiers.
Comment on lines
+8
to
+135
| def test_local_state_user_id_is_set_from_authenticated_user(): | ||
| """Verify that local_state.user_id is set from the authenticated user_id.""" | ||
| # Create a mock state | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| ) | ||
|
|
||
| # Simulate the user_id assignment from main.py | ||
| user_id = "authenticated-user-123" | ||
| local_state.user_id = user_id | ||
|
|
||
| assert local_state.user_id == "authenticated-user-123" | ||
|
|
||
|
|
||
| def test_local_state_service_scope_is_set_with_tenant_and_instance(): | ||
| """Verify that local_state.service_scope is set with tenant_id and instance_id.""" | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| ) | ||
|
|
||
| # Simulate the service_scope assignment from main.py | ||
| with patch("cuga.config.get_tenant_id", return_value="tenant-456"): | ||
| with patch("cuga.config.get_service_instance_id", return_value="instance-789"): | ||
| 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(), | ||
| } | ||
|
|
||
| assert local_state.service_scope["tenant_id"] == "tenant-456" | ||
| assert local_state.service_scope["instance_id"] == "instance-789" | ||
|
|
||
|
|
||
| def test_local_state_user_id_and_service_scope_set_together(): | ||
| """Verify that both user_id and service_scope are set correctly together.""" | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| ) | ||
|
|
||
| user_id = "authenticated-user-123" | ||
|
|
||
| with patch("cuga.config.get_tenant_id", return_value="tenant-456"): | ||
| with patch("cuga.config.get_service_instance_id", return_value="instance-789"): | ||
| from cuga.config import get_service_instance_id, get_tenant_id | ||
|
|
||
| # Simulate the assignments from main.py (lines 1245-1247) | ||
| local_state.user_id = user_id | ||
| local_state.service_scope = { | ||
| "tenant_id": get_tenant_id(), | ||
| "instance_id": get_service_instance_id(), | ||
| } | ||
| local_state.user_id = user_id # Duplicate assignment as in main.py | ||
|
|
||
| # Verify both are set correctly | ||
| assert local_state.user_id == "authenticated-user-123" | ||
| assert local_state.service_scope["tenant_id"] == "tenant-456" | ||
| assert local_state.service_scope["instance_id"] == "instance-789" | ||
|
|
||
|
|
||
| def test_local_state_user_id_duplicate_assignment_does_not_cause_issues(): | ||
| """Verify that the duplicate user_id assignment (lines 1245 and 1247) doesn't cause issues.""" | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| ) | ||
|
|
||
| user_id = "authenticated-user-123" | ||
|
|
||
| # First assignment (line 1245) | ||
| local_state.user_id = user_id | ||
| assert local_state.user_id == "authenticated-user-123" | ||
|
|
||
| # Second assignment (line 1247) - should not cause any issues | ||
| local_state.user_id = user_id | ||
| assert local_state.user_id == "authenticated-user-123" | ||
|
|
||
|
|
||
| def test_local_state_handles_none_user_id(): | ||
| """Verify that local_state handles None user_id gracefully.""" | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| ) | ||
|
|
||
| user_id = None | ||
| local_state.user_id = user_id | ||
|
|
||
| assert local_state.user_id is None | ||
|
|
||
|
|
||
| def test_local_state_multi_user_context_complete(): | ||
| """Verify complete multi-user context is set correctly for Evolve integration.""" | ||
| local_state = AgentState( | ||
| input="test query", | ||
| url="https://example.com", | ||
| thread_id="thread-999", | ||
| ) | ||
|
|
||
| user_id = "authenticated-user-123" | ||
|
|
||
| with patch("cuga.config.get_tenant_id", return_value="tenant-456"): | ||
| with patch("cuga.config.get_service_instance_id", return_value="instance-789"): | ||
| from cuga.config import get_service_instance_id, get_tenant_id | ||
|
|
||
| local_state.user_id = user_id | ||
| local_state.service_scope = { | ||
| "tenant_id": get_tenant_id(), | ||
| "instance_id": get_service_instance_id(), | ||
| } | ||
|
|
||
| # Verify all multi-user context fields are set for Evolve | ||
| assert local_state.user_id == "authenticated-user-123" | ||
| assert local_state.thread_id == "thread-999" | ||
| assert local_state.service_scope["tenant_id"] == "tenant-456" | ||
|
|
||
| # These values would be passed to EvolveIntegration | ||
| evolve_user_id = local_state.user_id or None | ||
| evolve_namespace_id = (local_state.service_scope or {}).get("tenant_id") or None | ||
| evolve_session_id = local_state.thread_id or None | ||
|
|
||
| assert evolve_user_id == "authenticated-user-123" | ||
| assert evolve_namespace_id == "tenant-456" | ||
| assert evolve_session_id == "thread-999" | ||
|
|
Contributor
There was a problem hiding this comment.
Tests don’t exercise event_stream, so they won’t catch propagation regressions.
These tests only set AgentState fields directly; they never call event_stream where the real assignment occurs. If local_state.user_id = user_id is removed from production code, this suite would still pass.
Please add at least one test that invokes event_stream (with mocked agent/graph and dependencies) and asserts the emitted/updated state contains expected user_id and service_scope.
🤖 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 `@tests/unit/test_server_user_id_propagation.py` around lines 8 - 135, Add a
new unit test that calls event_stream (the function that performs the real
assignments) instead of directly mutating AgentState; mock the agent/graph
dependencies and patch cuga.config.get_tenant_id and get_service_instance_id to
known values, provide a simulated authenticated user_id (e.g. via the same
variable name user_id or request context used in production), run event_stream
to produce/emitted state, and assert the returned/emitted AgentState (or the
event payload) has user_id == "authenticated-user-123" and
service_scope["tenant_id"] == "tenant-456" and service_scope["instance_id"] ==
"instance-789". Ensure the test references event_stream and AgentState so
regressions in the real assignment are caught.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/evolve/integration.py (1)
57-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize identifiers inside
EvolveIntegrationbefore building the payload.These public methods still forward raw truthy values. A caller that passes
"default_user"or whitespace will still attribute data to a fake identity, which defeats the new omission contract. Normalizeuser_id,namespace_id, andsession_idhere so every call path gets the same behavior.Proposed fix
async def get_guidelines( cls, task: str, user_id: Optional[str] = None, namespace_id: Optional[str] = None, session_id: Optional[str] = None, ) -> Optional[str]: """Fetch guidelines from Evolve for the given task description.""" if not cls.is_enabled(): return None try: + user_id = normalize_evolve_identifier(user_id) + namespace_id = normalize_evolve_identifier(namespace_id) + session_id = normalize_evolve_identifier(session_id) args: dict = {"task": task} if user_id: args["user_id"] = user_id if namespace_id: args["namespace_id"] = namespace_id @@ async def save_trajectory( cls, chat_messages: List[BaseMessage], task_id: str, success: bool, user_id: Optional[str] = None, namespace_id: Optional[str] = None, session_id: Optional[str] = None, ) -> None: """Save the agent trajectory to Evolve for tip generation.""" if not cls.is_enabled(): return @@ try: + user_id = normalize_evolve_identifier(user_id) + namespace_id = normalize_evolve_identifier(namespace_id) + session_id = normalize_evolve_identifier(session_id) logger.debug( f"Evolve: Converting {len(chat_messages)} chat_messages. " f"Types: {[type(m).__name__ for m in chat_messages[:10]]}" ) openai_messages = cls._convert_messages(chat_messages)Also applies to: 143-187
🤖 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/integration.py` around lines 57 - 75, In EvolveIntegration.get_guidelines normalize the identifier inputs before building args: for user_id, namespace_id, and session_id call .strip() and if the result is empty or equals "default_user" (or other sentinel strings used by your app) set the variable to None so you don't forward fake/whitespace identities; then only add keys to args when the normalized value is not None. Apply the same normalization logic to the other public EvolveIntegration payload-building method(s) referenced around lines 143-187 (where cls._call_tool is used) so all call paths behave identically.
🤖 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.
Inline comments:
In `@tests/unit/test_server_user_id_propagation.py`:
- Line 35: The function apply_request_user_context currently types its user_id
parameter as str but is called with None in tests; update its signature to
accept Optional[str] (import Optional from typing) and adjust any internal logic
and return type annotations to handle None safely (e.g., preserve
AgentState.user_id typing and guards where apply_request_user_context sets or
reads user_id); also update any callers or type hints that assume a non-None
string so they handle Optional[str] consistently.
- Around line 28-39: The test
test_apply_request_user_context_handles_none_user_id is missing an assertion for
service_scope["instance_id"]; update that test to assert
local_state.service_scope["instance_id"] == "instance-789" after calling
apply_request_user_context on the AgentState (local_state) with None user_id
while patching get_tenant_id and get_service_instance_id so the test checks both
tenant_id and instance_id are populated consistently with the other tests.
---
Outside diff comments:
In `@src/cuga/backend/evolve/integration.py`:
- Around line 57-75: In EvolveIntegration.get_guidelines normalize the
identifier inputs before building args: for user_id, namespace_id, and
session_id call .strip() and if the result is empty or equals "default_user" (or
other sentinel strings used by your app) set the variable to None so you don't
forward fake/whitespace identities; then only add keys to args when the
normalized value is not None. Apply the same normalization logic to the other
public EvolveIntegration payload-building method(s) referenced around lines
143-187 (where cls._call_tool is used) so all call paths behave identically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59efc7af-8745-4f5f-844e-0e2c216555b0
📒 Files selected for processing (7)
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/cuga_graph/nodes/cuga_lite/tests/test_cuga_lite_node.pysrc/cuga/backend/evolve/integration.pysrc/cuga/backend/evolve/memory.pysrc/cuga/backend/server/main.pytests/unit/test_server_user_id_propagation.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/cuga/backend/server/main.py
- src/cuga/backend/cuga_graph/nodes/cuga_lite/tests/test_cuga_lite_node.py
- src/cuga/backend/evolve/memory.py
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