fix review comments

Author: seratch

src/agents/run.py

@@ -29,7 +29,7 @@
 )
 from .lifecycle import RunHooks
 from .logger import logger
-from .memory import Session, is_server_managed_conversation_session
+from .memory import Session
 from .result import RunResult, RunResultStreaming
 from .run_config import (
     DEFAULT_MAX_TURNS,
@@ -413,7 +413,6 @@ async def run(
         auto_previous_response_id = kwargs.get("auto_previous_response_id", False)
         conversation_id = kwargs.get("conversation_id")
         session = kwargs.get("session")
-        run_config_was_supplied = run_config is not None
 
         if run_config is None:
             run_config = RunConfig()
@@ -504,19 +503,15 @@ async def run(
             or previous_response_id is not None
             or auto_previous_response_id
         )
-        history_is_server_managed = (
-            server_manages_conversation or is_server_managed_conversation_session(session)
-        )
         validate_override_history_persistence_support(
             input=input,
             session=session,
-            history_is_server_managed=history_is_server_managed,
+            response_history_is_server_managed=server_manages_conversation,
         )
 
         resolved_trace_include_sensitive_data = resolve_trace_include_sensitive_data(
             run_state=run_state,
             run_config=run_config,
-            run_config_was_supplied=run_config_was_supplied,
         )
         if resolved_trace_include_sensitive_data != run_config.trace_include_sensitive_data:
             run_config = dataclasses.replace(
@@ -1469,7 +1464,6 @@ def run_streamed(
         auto_previous_response_id = kwargs.get("auto_previous_response_id", False)
         conversation_id = kwargs.get("conversation_id")
         session = kwargs.get("session")
-        run_config_was_supplied = run_config is not None
 
         if run_config is None:
             run_config = RunConfig()
@@ -1553,18 +1547,14 @@ def run_streamed(
             or previous_response_id is not None
             or auto_previous_response_id
         )
-        history_is_server_managed = (
-            server_manages_conversation or is_server_managed_conversation_session(session)
-        )
         validate_override_history_persistence_support(
             input=input,
             session=session,
-            history_is_server_managed=history_is_server_managed,
+            response_history_is_server_managed=server_manages_conversation,
         )
         resolved_trace_include_sensitive_data = resolve_trace_include_sensitive_data(
             run_state=run_state,
             run_config=run_config,
-            run_config_was_supplied=run_config_was_supplied,
         )
         if resolved_trace_include_sensitive_data != run_config.trace_include_sensitive_data:
             run_config = dataclasses.replace(

src/agents/run_config.py

@@ -2,7 +2,7 @@
 
 import os
 from dataclasses import dataclass, field
-from typing import TYPE_CHECKING, Any, Callable, Generic, Literal, Optional
+from typing import TYPE_CHECKING, Any, Callable, Generic, Literal, Optional, cast
 
 from typing_extensions import NotRequired, TypedDict
 
@@ -33,6 +33,14 @@ def _default_trace_include_sensitive_data() -> bool:
     return val.strip().lower() in ("1", "true", "yes", "on")
 
 
+_TRACE_INCLUDE_SENSITIVE_DATA_UNSET = cast(bool, object())
+
+
+def _unset_trace_include_sensitive_data() -> bool:
+    """Return a sentinel so RunConfig can detect explicit trace flag overrides."""
+    return _TRACE_INCLUDE_SENSITIVE_DATA_UNSET
+
+
 @dataclass
 class ModelInputData:
     """Container for the data that will be sent to the model."""
@@ -129,9 +137,7 @@ class RunConfig:
     tracing: TracingConfig | None = None
     """Tracing configuration for this run."""
 
-    trace_include_sensitive_data: bool = field(
-        default_factory=_default_trace_include_sensitive_data
-    )
+    trace_include_sensitive_data: bool = field(default_factory=_unset_trace_include_sensitive_data)
     """Whether we include potentially sensitive data (for example: inputs/outputs of tool calls or
     LLM generations) in traces. If False, we'll still create spans for these events, but the
     sensitive data will not be included.
@@ -191,6 +197,21 @@ class RunConfig:
     - ``"omit"`` strips reasoning item IDs from model input built by the runner.
     """
 
+    _trace_include_sensitive_data_was_explicit: bool = field(
+        init=False,
+        repr=False,
+        compare=False,
+        default=False,
+    )
+
+    def __post_init__(self) -> None:
+        if self.trace_include_sensitive_data is _TRACE_INCLUDE_SENSITIVE_DATA_UNSET:
+            self.trace_include_sensitive_data = _default_trace_include_sensitive_data()
+            self._trace_include_sensitive_data_was_explicit = False
+            return
+
+        self._trace_include_sensitive_data_was_explicit = True
+
 
 class RunOptions(TypedDict, Generic[TContext]):
     """Arguments for ``AgentRunner`` methods."""

src/agents/run_internal/agent_runner_helpers.py

@@ -115,23 +115,26 @@ def validate_override_history_persistence_support(
     *,
     input: str | list[TResponseInputItem] | RunState[Any],
     session: Session | None,
-    history_is_server_managed: bool,
+    response_history_is_server_managed: bool,
 ) -> None:
     """Fail fast when approval override persistence requirements are not satisfied."""
     if not isinstance(input, RunState):
         return
 
-    if input.has_pending_execution_only_approval_overrides() and not history_is_server_managed:
+    if (
+        input.has_pending_execution_only_approval_overrides()
+        and not response_history_is_server_managed
+    ):
         raise UserError(
             "save_override_arguments=False is only supported when using conversation_id, "
-            "previous_response_id, auto_previous_response_id, or a server-managed session."
+            "previous_response_id, or auto_previous_response_id."
         )
 
     mutations = input.get_session_history_mutations()
     if not mutations:
         return
 
-    if history_is_server_managed:
+    if response_history_is_server_managed:
         raise UserError(
             "save_override_arguments requires local canonical history. "
             "Server-managed conversations cannot persist corrected function_call arguments. "
@@ -184,18 +187,14 @@ def resolve_trace_include_sensitive_data(
     *,
     run_state: RunState[TContext] | None,
     run_config: RunConfig,
-    run_config_was_supplied: bool,
 ) -> bool:
-    """Resolve whether traces may include sensitive data for this run.
-
-    Resumed runs preserve the stored setting unless the new RunConfig explicitly narrows it by
-    setting `trace_include_sensitive_data=False`.
-    """
-    del run_config_was_supplied
+    """Resolve whether traces may include sensitive data for this run."""
     if run_state is None:
         return run_config.trace_include_sensitive_data
-    if run_config.trace_include_sensitive_data is False:
-        return False
+
+    if getattr(run_config, "_trace_include_sensitive_data_was_explicit", True):
+        return run_config.trace_include_sensitive_data
+
     return run_state._trace_include_sensitive_data
 
 

tests/test_agent_runner.py

@@ -1173,7 +1173,7 @@ def approval_tool(test: str) -> str:
 
 
 @pytest.mark.asyncio
-async def test_resume_supports_execution_only_override_with_server_managed_session() -> None:
+async def test_resume_rejects_execution_only_override_with_marker_session() -> None:
     model = FakeModel()
 
     @function_tool(name_override="approval_tool", needs_approval=True)
@@ -1207,12 +1207,47 @@ def approval_tool(test: str) -> str:
         save_override_arguments=False,
     )
 
-    resumed = await Runner.run(agent, state, session=session)
+    with pytest.raises(UserError, match="save_override_arguments=False is only supported"):
+        await Runner.run(agent, state, session=session)
+
+
+@pytest.mark.asyncio
+async def test_resume_supports_execution_only_override_with_previous_response_id() -> None:
+    model = FakeModel()
+
+    @function_tool(name_override="approval_tool", needs_approval=True)
+    def approval_tool(test: str) -> str:
+        return f"result:{test}"
+
+    agent = Agent(
+        name="approval_agent",
+        model=model,
+        tools=[approval_tool],
+        tool_use_behavior="stop_on_first_tool",
+    )
+    model.add_multiple_turn_outputs(
+        [
+            [
+                get_function_tool_call(
+                    "approval_tool", json.dumps({"test": "foo"}), call_id="call-1"
+                )
+            ],
+        ]
+    )
+
+    first = await Runner.run(agent, input="user_message", previous_response_id="resp-root")
+    assert first.interruptions
+
+    state = first.to_state()
+    state.approve(
+        first.interruptions[0],
+        override_arguments={"test": "bar"},
+        save_override_arguments=False,
+    )
+
+    resumed = await Runner.run(agent, state)
 
     assert resumed.final_output == "result:bar"
-    saved_items = await session.get_items()
-    assert cast(dict[str, Any], saved_items[1])["arguments"] == json.dumps({"test": "foo"})
-    assert saved_items[2]["type"] == "function_call_output"
 
 
 @pytest.mark.asyncio

tests/test_agent_runner_streamed.py

@@ -1608,9 +1608,7 @@ async def test_tool(test: str) -> str:
 
 
 @pytest.mark.asyncio
-async def test_streaming_resume_supports_execution_only_override_with_server_managed_session() -> (
-    None
-):
+async def test_streaming_resume_rejects_execution_only_override_with_marker_session() -> None:
     async def test_tool(test: str) -> str:
         return f"result:{test}"
 
@@ -1639,7 +1637,42 @@ async def test_tool(test: str) -> str:
         save_override_arguments=False,
     )
 
-    resumed = Runner.run_streamed(agent, state, session=session)
+    with pytest.raises(UserError, match="save_override_arguments=False is only supported"):
+        Runner.run_streamed(agent, state, session=session)
+
+
+@pytest.mark.asyncio
+async def test_streaming_resume_supports_execution_only_override_with_previous_response_id() -> (
+    None
+):
+    async def test_tool(test: str) -> str:
+        return f"result:{test}"
+
+    tool = function_tool(test_tool, name_override="test_tool", needs_approval=True)
+    model = FakeModel()
+    agent = Agent(
+        name="test",
+        model=model,
+        tools=[tool],
+        tool_use_behavior="stop_on_first_tool",
+    )
+
+    model.add_multiple_turn_outputs(
+        [[get_function_tool_call("test_tool", json.dumps({"test": "foo"}), call_id="call-resume")]]
+    )
+
+    first = Runner.run_streamed(agent, input="Use test_tool", previous_response_id="resp-root")
+    await consume_stream(first)
+    assert first.interruptions
+
+    state = first.to_state()
+    state.approve(
+        first.interruptions[0],
+        override_arguments={"test": "bar"},
+        save_override_arguments=False,
+    )
+
+    resumed = Runner.run_streamed(agent, state)
     await consume_stream(resumed)
 
     assert resumed.final_output == "result:bar"

tests/test_agent_tracing.py

@@ -777,6 +777,51 @@ def send_email(recipient: str) -> str:
     assert function_span["span_data"]["output"] is None
 
 
+@pytest.mark.asyncio
+async def test_resumed_streaming_run_preserves_sensitive_trace_flag_for_unrelated_run_config() -> (
+    None
+):
+    model = FakeModel()
+
+    @function_tool(name_override="send_email", needs_approval=True)
+    def send_email(recipient: str) -> str:
+        return recipient
+
+    agent = Agent(name="trace_agent", model=model, tools=[send_email])
+    model.add_multiple_turn_outputs(
+        [
+            [
+                get_function_tool_call(
+                    "send_email", '{"recipient":"alice@example.com"}', call_id="call-1"
+                )
+            ],
+            [get_text_message("done")],
+        ]
+    )
+
+    first = Runner.run_streamed(agent, input="first_test")
+    async for _ in first.stream_events():
+        pass
+    assert first.interruptions
+
+    state = first.to_state()
+    state.set_trace_include_sensitive_data(False)
+    state.approve(first.interruptions[0], override_arguments={"recipient": "bob@example.com"})
+
+    resumed = Runner.run_streamed(
+        agent,
+        state,
+        run_config=RunConfig(workflow_name="override_workflow"),
+    )
+    async for _ in resumed.stream_events():
+        pass
+
+    assert resumed.final_output == "done"
+    function_span = _get_last_function_span_export("send_email")
+    assert function_span["span_data"]["input"] is None
+    assert function_span["span_data"]["output"] is None
+
+
 @pytest.mark.asyncio
 async def test_wrapped_streaming_trace_is_single_trace():
     model = FakeModel()

tests/test_run_config.py

@@ -138,3 +138,18 @@ def test_trace_include_sensitive_data_explicit_override_takes_precedence(monkeyp
     monkeypatch.setenv("OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA", "true")
     config = RunConfig(trace_include_sensitive_data=False)
     assert config.trace_include_sensitive_data is False
+
+
+def test_trace_include_sensitive_data_tracks_explicit_overrides(monkeypatch):
+    """RunConfig should distinguish explicit trace flag overrides from unrelated options."""
+    monkeypatch.setenv("OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA", "true")
+
+    default_config = RunConfig()
+    unrelated_config = RunConfig(workflow_name="custom-workflow")
+    explicit_true_config = RunConfig(trace_include_sensitive_data=True)
+    explicit_false_config = RunConfig(trace_include_sensitive_data=False)
+
+    assert default_config._trace_include_sensitive_data_was_explicit is False
+    assert unrelated_config._trace_include_sensitive_data_was_explicit is False
+    assert explicit_true_config._trace_include_sensitive_data_was_explicit is True
+    assert explicit_false_config._trace_include_sensitive_data_was_explicit is True