From 0693c700a20293af519aa1480099e583b0aa796d Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 8 May 2026 03:27:59 -0700 Subject: [PATCH 1/2] fix(tracing): skip NoOpTrace context reset on GeneratorExit NoOpTrace.__exit__ unconditionally called finish(reset_current=True), unlike TraceImpl, ReattachedTrace, SpanImpl, and NoOpSpan which all skip the contextvars reset when exiting via GeneratorExit. When an async generator wrapped in a NoOpTrace is finalized by GC from a different task than the one that opened it, resetting a Token created in another Context raises ValueError. This aligns NoOpTrace with the other lifecycle types so disabled tracing behaves consistently with enabled tracing under generator-driven cleanup. --- src/agents/tracing/traces.py | 2 +- tests/tracing/test_traces_impl.py | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/agents/tracing/traces.py b/src/agents/tracing/traces.py index 4f91ca709c..d523a70cf7 100644 --- a/src/agents/tracing/traces.py +++ b/src/agents/tracing/traces.py @@ -399,7 +399,7 @@ def __enter__(self) -> Trace: return self def __exit__(self, exc_type, exc_val, exc_tb): - self.finish(reset_current=True) + self.finish(reset_current=exc_type is not GeneratorExit) def start(self, mark_as_current: bool = False): if mark_as_current: diff --git a/tests/tracing/test_traces_impl.py b/tests/tracing/test_traces_impl.py index 866b23b3d8..00ee1c2857 100644 --- a/tests/tracing/test_traces_impl.py +++ b/tests/tracing/test_traces_impl.py @@ -42,6 +42,27 @@ def test_no_op_trace_double_enter_logs_error(caplog) -> None: trace.__exit__(None, None, None) +def test_no_op_trace_skips_context_reset_on_generator_exit() -> None: + """NoOpTrace must skip context reset on GeneratorExit to match TraceImpl/ReattachedTrace. + + When async generators are GC'd from a different task, resetting a contextvars + token created in another context raises ValueError. All other Trace/Span types + already check for GeneratorExit and skip the reset; NoOpTrace must too. + """ + Scope.set_current_trace(None) + trace = NoOpTrace() + trace.__enter__() + # The token was set in this context; pretend we're unwinding from a different + # async task by stashing the token and checking it isn't consumed on GeneratorExit. + token = trace._prev_context_token + assert token is not None + trace.__exit__(GeneratorExit, GeneratorExit(), None) + # Token must remain (reset skipped) so the original context owner can reset it. + assert trace._prev_context_token is token + # Cleanup so subsequent tests have a clean scope. + Scope.reset_current_trace(token) + + def test_trace_impl_lifecycle_sets_scope() -> None: Scope.set_current_trace(None) processor = DummyProcessor() From c00dc93a5177f4d43001a3a4d335220ab9fff525 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Mon, 11 May 2026 16:40:02 -0700 Subject: [PATCH 2/2] fix(tracing): try NoOpTrace context reset and swallow cross-context ValueError Skipping reset unconditionally on GeneratorExit handled the cross-task GC case but left NoOpTrace as the current trace after a normal same-context generator close, suppressing later tracing in that context. Always attempt the reset and swallow ValueError for the cross-Context case so both paths work correctly. --- src/agents/tracing/traces.py | 15 +++++++++++-- tests/tracing/test_traces_impl.py | 37 ++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/agents/tracing/traces.py b/src/agents/tracing/traces.py index d523a70cf7..d63c6a3f95 100644 --- a/src/agents/tracing/traces.py +++ b/src/agents/tracing/traces.py @@ -399,7 +399,11 @@ def __enter__(self) -> Trace: return self def __exit__(self, exc_type, exc_val, exc_tb): - self.finish(reset_current=exc_type is not GeneratorExit) + # Same-task generator close should still reset the context var so the + # NoOpTrace doesn't linger as the current trace. Cross-context closes + # (different task / event loop) raise ValueError from contextvars, + # which the finish() helper swallows below. + self.finish(reset_current=True) def start(self, mark_as_current: bool = False): if mark_as_current: @@ -407,7 +411,14 @@ def start(self, mark_as_current: bool = False): def finish(self, reset_current: bool = False): if reset_current and self._prev_context_token is not None: - Scope.reset_current_trace(self._prev_context_token) + try: + Scope.reset_current_trace(self._prev_context_token) + except ValueError: + # The context token was created in a different Context (e.g. + # the trace was entered in another task and the generator is + # closing from the parent's context). Skipping reset here is + # safe: that other Context owns its own contextvar copy. + pass self._prev_context_token = None @property diff --git a/tests/tracing/test_traces_impl.py b/tests/tracing/test_traces_impl.py index 00ee1c2857..20710f03ff 100644 --- a/tests/tracing/test_traces_impl.py +++ b/tests/tracing/test_traces_impl.py @@ -1,3 +1,4 @@ +import contextvars import logging from typing import Any, cast @@ -42,25 +43,39 @@ def test_no_op_trace_double_enter_logs_error(caplog) -> None: trace.__exit__(None, None, None) -def test_no_op_trace_skips_context_reset_on_generator_exit() -> None: - """NoOpTrace must skip context reset on GeneratorExit to match TraceImpl/ReattachedTrace. +def test_no_op_trace_resets_context_on_same_task_generator_exit() -> None: + """NoOpTrace must reset the current trace on a same-task GeneratorExit. - When async generators are GC'd from a different task, resetting a contextvars - token created in another context raises ValueError. All other Trace/Span types - already check for GeneratorExit and skip the reset; NoOpTrace must too. + A previous version skipped reset unconditionally on GeneratorExit, which + handles the cross-task GC case but leaves NoOpTrace as the current trace + after a normal same-context generator close, suppressing later tracing. + Now the reset is attempted and any ValueError (from a token created in a + different Context) is swallowed. """ Scope.set_current_trace(None) trace = NoOpTrace() trace.__enter__() - # The token was set in this context; pretend we're unwinding from a different - # async task by stashing the token and checking it isn't consumed on GeneratorExit. + assert trace._prev_context_token is not None + trace.__exit__(GeneratorExit, GeneratorExit(), None) + # Same-task close: reset succeeded, token consumed, current trace cleared. + assert trace._prev_context_token is None + assert Scope.get_current_trace() is None + + +def test_no_op_trace_swallows_cross_context_reset_error() -> None: + """A token created in a different Context raises ValueError on reset; swallow it.""" + Scope.set_current_trace(None) + trace = NoOpTrace() + + other_context = contextvars.copy_context() + other_context.run(trace.__enter__) token = trace._prev_context_token assert token is not None + + # Resetting from our context (not the one that set it) raises ValueError; + # the helper must swallow that and clear the stored token. trace.__exit__(GeneratorExit, GeneratorExit(), None) - # Token must remain (reset skipped) so the original context owner can reset it. - assert trace._prev_context_token is token - # Cleanup so subsequent tests have a clean scope. - Scope.reset_current_trace(token) + assert trace._prev_context_token is None def test_trace_impl_lifecycle_sets_scope() -> None: