Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

fix issue with session_id #91

Merged
jhaynie merged 1 commit into from
Aug 28, 2025
Merged

fix issue with session_id #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions agentuity/server/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import traceback

from opentelemetry import trace
from opentelemetry.trace import format_trace_id
from opentelemetry.propagate import extract, inject

from agentuity.otel import init
Expand Down Expand Up @@ -343,7 +344,7 @@ async def handle_agent_request(request: web.Request):
metadata = {}
scope = "local"
if span.is_recording():
run_id = span.get_span_context().trace_id
run_id = format_trace_id(span.get_span_context().trace_id)
else:
run_id = None
for key, value in headers.items():
Expand Down Expand Up @@ -446,7 +447,7 @@ async def handle_agent_request(request: web.Request):
agent=agent,
agents_by_id=agents_by_id,
port=port,
session_id=run_id,
session_id=str(run_id),
scope=scope,
)
Comment on lines +450 to 452
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: session_id becomes the literal string "None" when tracing isn’t recording.

str(run_id) coerces None → "None". Downstream code will see a non-empty session_id, which is misleading.

Apply this diff:

-                    session_id=str(run_id),
+                    session_id=run_id,
📝 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
session_id=str(run_id),
scope=scope,
)
session_id=run_id,
scope=scope,
)
🤖 Prompt for AI Agents 
In agentuity/server/__init__.py around lines 450 to 452, session_id is set using
str(run_id) which converts None into the literal string "None" and misleads
downstream code; change this to pass None when run_id is None (e.g., session_id
= None if run_id is None else str(run_id)) so that an absent run_id remains a
true None rather than the string "None".

agent_response = AgentResponse(
Expand Down
12 changes: 11 additions & 1 deletion tests/server/test_request_handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,21 @@ async def test_handle_agent_request_success(self, mock_request):
mock_response.status = 200
mock_response.content_type = "application/json"

# Create a mock span with proper trace_id formatting
mock_span_context = MagicMock()
mock_span_context.trace_id = 12345678901234567890123456789012345678
mock_span = MagicMock()
mock_span.get_span_context.return_value = mock_span_context
mock_span.is_recording.return_value = True
mock_tracer = MagicMock()
mock_tracer.start_span.return_value.__enter__.return_value = mock_span

Comment on lines +91 to +99
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Mock the API you actually use: start_as_current_span, not start_span.

handle_agent_request uses tracer.start_as_current_span(...). Your mock wires start_span, so the configured span (is_recording, get_span_context) isn’t the one used.

Apply this diff:

-        mock_tracer = MagicMock()
-        mock_tracer.start_span.return_value.__enter__.return_value = mock_span
+        mock_tracer = MagicMock(spec=trace.Tracer)
+        mock_tracer.start_as_current_span.return_value.__enter__.return_value = mock_span
📝 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
# Create a mock span with proper trace_id formatting
mock_span_context = MagicMock()
mock_span_context.trace_id = 12345678901234567890123456789012345678
mock_span = MagicMock()
mock_span.get_span_context.return_value = mock_span_context
mock_span.is_recording.return_value = True
mock_tracer = MagicMock()
mock_tracer.start_span.return_value.__enter__.return_value = mock_span
# Create a mock span with proper trace_id formatting
mock_span_context = MagicMock()
mock_span_context.trace_id = 12345678901234567890123456789012345678
mock_span = MagicMock()
mock_span.get_span_context.return_value = mock_span_context
mock_span.is_recording.return_value = True
- mock_tracer = MagicMock()
mock_tracer = MagicMock(spec=trace.Tracer)
mock_tracer.start_as_current_span.return_value.__enter__.return_value = mock_span
🤖 Prompt for AI Agents 
In tests/server/test_request_handlers.py around lines 91 to 99, the test mocks
tracer.start_span but the code under test calls tracer.start_as_current_span, so
the span used in the handler isn’t the mocked one; update the mock to set
mock_tracer.start_as_current_span.return_value.__enter__.return_value =
mock_span (and keep mock_span.get_span_context, mock_span.is_recording as is) so
the context manager used by start_as_current_span yields the configured mock
span.

with (
patch(
"agentuity.server.trace.get_tracer", return_value=MagicMock()
"agentuity.server.trace.get_tracer", return_value=mock_tracer
) as mock_get_tracer,
patch("agentuity.server.extract", return_value={}),
patch("agentuity.server.format_trace_id", return_value="test-trace-id"),
patch(
"agentuity.server.run_agent", new_callable=AsyncMock
) as mock_run_agent,
Expand Down