From 323d98bee0352f48a8142741b0abe889c88f7874 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 13:14:13 +0900 Subject: [PATCH 1/7] Fix #4371: Propagate session to manager agent in StandardMagenticManager StandardMagenticManager._complete() was calling self._agent.run(messages) without passing a session. This caused context providers (e.g. RedisHistoryProvider) configured on the manager agent to silently fail, as each call created a new ephemeral session with a different session_id. Changes: - Create an AgentSession in StandardMagenticManager.__init__() - Pass session=self._session in _complete() calls to agent.run() - Persist/restore the session in checkpoint save/restore methods - Add regression tests for session propagation and checkpoint round-trip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../_magentic.py | 11 +++- .../orchestrations/tests/test_magentic.py | 55 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py index 17b927326b..4157d8a75c 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py @@ -14,6 +14,7 @@ from agent_framework import ( AgentResponse, + AgentSession, Message, SupportsAgentRun, ) @@ -559,6 +560,7 @@ def __init__( ) self._agent: SupportsAgentRun = agent + self._session: AgentSession = self._agent.create_session() self.task_ledger: _MagenticTaskLedger | None = task_ledger # Prompts may be overridden if needed @@ -587,7 +589,7 @@ async def _complete( The agent's run method is called which applies the agent's configured options (temperature, seed, instructions, etc.). """ - response: AgentResponse = await self._agent.run(messages) + response: AgentResponse = await self._agent.run(messages, session=self._session) if not response.messages: raise RuntimeError("Agent returned no messages in response.") if len(response.messages) > 1: @@ -730,6 +732,7 @@ def on_checkpoint_save(self) -> dict[str, Any]: state: dict[str, Any] = {} if self.task_ledger is not None: state["task_ledger"] = self.task_ledger.to_dict() + state["agent_session"] = self._session.to_dict() return state @override @@ -740,6 +743,12 @@ def on_checkpoint_restore(self, state: dict[str, Any]) -> None: self.task_ledger = _MagenticTaskLedger.from_dict(ledger) except Exception: # pragma: no cover - defensive logger.warning("Failed to restore manager task ledger from checkpoint state") + session_payload = state.get("agent_session") + if session_payload: + try: + self._session = AgentSession.from_dict(session_payload) + except Exception: # pragma: no cover - defensive + logger.warning("Failed to restore manager agent session from checkpoint state") # endregion Magentic Manager diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index e1d0ef8c32..a64f3e9bf4 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1074,4 +1074,59 @@ def agent_factory() -> SupportsAgentRun: assert manager.final_answer_prompt == custom_final_prompt +async def test_standard_manager_propagates_session_to_agent(): + """Verify StandardMagenticManager passes a consistent session to the underlying agent. + + Regression test for #4371: context providers (e.g. RedisHistoryProvider) configured on + the manager agent silently failed because no session was propagated. + """ + captured_sessions: list[AgentSession | None] = [] + + class SessionCapturingAgent(BaseAgent): + """Agent that records the session passed to each run() call.""" + + def run( + self, + messages: str | Content | Message | Sequence[str | Content | Message] | None = None, + *, + stream: bool = False, + session: Any = None, + **kwargs: Any, + ) -> Awaitable[AgentResponse] | AsyncIterable[AgentResponseUpdate]: + captured_sessions.append(session) + + async def _run() -> AgentResponse: + return AgentResponse(messages=[Message("assistant", ["ok"])]) + + return _run() + + agent = SessionCapturingAgent() + mgr = StandardMagenticManager(agent=agent) + ctx = MagenticContext(task="task", participant_descriptions={"a": "desc"}) + + await mgr.plan(ctx.clone()) + + # plan() calls _complete twice (facts + plan), both should receive the same session + assert len(captured_sessions) == 2 + assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" + assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" + assert captured_sessions[0] is mgr._session + + +async def test_standard_manager_checkpoint_preserves_session(): + """Verify that checkpoint save/restore preserves the manager's session identity.""" + agent = StubManagerAgent() + mgr = StandardMagenticManager(agent=agent) + original_session_id = mgr._session.session_id + + state = mgr.on_checkpoint_save() + assert "agent_session" in state + + # Restore into a fresh manager and verify session_id is preserved + mgr2 = StandardMagenticManager(agent=agent) + assert mgr2._session.session_id != original_session_id + mgr2.on_checkpoint_restore(state) + assert mgr2._session.session_id == original_session_id + + # endregion From df44e3091a8cf72b99df251bf495fb3ec12f197e Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 13:20:12 +0900 Subject: [PATCH 2/7] Add type: ignore[reportPrivateUsage] to private attribute assertions in tests Address PR review feedback: add # type: ignore[reportPrivateUsage] comments to _session attribute accesses in the new regression tests, matching the existing convention used elsewhere in test_magentic.py (e.g., lines 401-406). The @pytest.mark.asyncio decorator is not needed because pyproject.toml sets asyncio_mode = "auto". Fixes #4371 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/packages/orchestrations/tests/test_magentic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index a64f3e9bf4..f8749fa8f4 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1110,23 +1110,23 @@ async def _run() -> AgentResponse: assert len(captured_sessions) == 2 assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" - assert captured_sessions[0] is mgr._session + assert captured_sessions[0] is mgr._session # type: ignore[reportPrivateUsage] async def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session_id = mgr._session.session_id + original_session_id = mgr._session.session_id # type: ignore[reportPrivateUsage] state = mgr.on_checkpoint_save() assert "agent_session" in state # Restore into a fresh manager and verify session_id is preserved mgr2 = StandardMagenticManager(agent=agent) - assert mgr2._session.session_id != original_session_id + assert mgr2._session.session_id != original_session_id # type: ignore[reportPrivateUsage] mgr2.on_checkpoint_restore(state) - assert mgr2._session.session_id == original_session_id + assert mgr2._session.session_id == original_session_id # type: ignore[reportPrivateUsage] # endregion From ff13b726cab2dc28cbf6ababca5084dd833ee801 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 13:21:23 +0900 Subject: [PATCH 3/7] Address review: use getattr for private _session access in tests (#4371) Replace direct mgr._session access with getattr(mgr, "_session") to avoid reportPrivateUsage type-checking warnings without needing type: ignore comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/packages/orchestrations/tests/test_magentic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index f8749fa8f4..12818edd36 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1110,23 +1110,23 @@ async def _run() -> AgentResponse: assert len(captured_sessions) == 2 assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" - assert captured_sessions[0] is mgr._session # type: ignore[reportPrivateUsage] + assert captured_sessions[0] is getattr(mgr, "_session") async def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session_id = mgr._session.session_id # type: ignore[reportPrivateUsage] + original_session_id = getattr(mgr, "_session").session_id state = mgr.on_checkpoint_save() assert "agent_session" in state # Restore into a fresh manager and verify session_id is preserved mgr2 = StandardMagenticManager(agent=agent) - assert mgr2._session.session_id != original_session_id # type: ignore[reportPrivateUsage] + assert getattr(mgr2, "_session").session_id != original_session_id mgr2.on_checkpoint_restore(state) - assert mgr2._session.session_id == original_session_id # type: ignore[reportPrivateUsage] + assert getattr(mgr2, "_session").session_id == original_session_id # endregion From a2c4f261716ab184e60ea7d977fcec0a0c70d63e Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 13:21:30 +0900 Subject: [PATCH 4/7] Apply pre-commit auto-fixes --- python/packages/orchestrations/tests/test_magentic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index 12818edd36..a64f3e9bf4 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1110,23 +1110,23 @@ async def _run() -> AgentResponse: assert len(captured_sessions) == 2 assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" - assert captured_sessions[0] is getattr(mgr, "_session") + assert captured_sessions[0] is mgr._session async def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session_id = getattr(mgr, "_session").session_id + original_session_id = mgr._session.session_id state = mgr.on_checkpoint_save() assert "agent_session" in state # Restore into a fresh manager and verify session_id is preserved mgr2 = StandardMagenticManager(agent=agent) - assert getattr(mgr2, "_session").session_id != original_session_id + assert mgr2._session.session_id != original_session_id mgr2.on_checkpoint_restore(state) - assert getattr(mgr2, "_session").session_id == original_session_id + assert mgr2._session.session_id == original_session_id # endregion From bd84f61ef5a50a2ccacb7632c2f6148365f6f67e Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 15:26:52 +0900 Subject: [PATCH 5/7] Address PR review: fix session restore guard and improve test robustness (#4371) - Use 'is not None' instead of truthiness check for session_payload restore - Use getattr() for private _session attribute access in tests - Add backward-compatibility test for on_checkpoint_restore with empty state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../_magentic.py | 2 +- .../orchestrations/tests/test_magentic.py | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py index 4157d8a75c..b887d86df3 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_magentic.py @@ -744,7 +744,7 @@ def on_checkpoint_restore(self, state: dict[str, Any]) -> None: except Exception: # pragma: no cover - defensive logger.warning("Failed to restore manager task ledger from checkpoint state") session_payload = state.get("agent_session") - if session_payload: + if session_payload is not None: try: self._session = AgentSession.from_dict(session_payload) except Exception: # pragma: no cover - defensive diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index a64f3e9bf4..dc649b2978 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1110,23 +1110,35 @@ async def _run() -> AgentResponse: assert len(captured_sessions) == 2 assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" - assert captured_sessions[0] is mgr._session + assert captured_sessions[0] is getattr(mgr, "_session") async def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session_id = mgr._session.session_id + original_session_id = getattr(mgr, "_session").session_id state = mgr.on_checkpoint_save() assert "agent_session" in state # Restore into a fresh manager and verify session_id is preserved mgr2 = StandardMagenticManager(agent=agent) - assert mgr2._session.session_id != original_session_id + assert getattr(mgr2, "_session").session_id != original_session_id mgr2.on_checkpoint_restore(state) - assert mgr2._session.session_id == original_session_id + assert getattr(mgr2, "_session").session_id == original_session_id + + +async def test_standard_manager_checkpoint_restore_empty_state(): + """Verify that restoring from a state without agent_session leaves the session intact.""" + agent = StubManagerAgent() + mgr = StandardMagenticManager(agent=agent) + original_session = getattr(mgr, "_session") + original_session_id = original_session.session_id + + mgr.on_checkpoint_restore({}) + assert getattr(mgr, "_session") is original_session + assert getattr(mgr, "_session").session_id == original_session_id # endregion From db377bde9abcf6ce7f7a1c7db96b04dd271f5365 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 15:27:29 +0900 Subject: [PATCH 6/7] Make non-async tests plain def to avoid pytest-asyncio dependency (#4409) Tests that never await anything don't need to be async. Using plain def ensures they always run regardless of pytest-asyncio configuration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/packages/orchestrations/tests/test_magentic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index dc649b2978..9047580328 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1113,7 +1113,7 @@ async def _run() -> AgentResponse: assert captured_sessions[0] is getattr(mgr, "_session") -async def test_standard_manager_checkpoint_preserves_session(): +def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) @@ -1129,7 +1129,7 @@ async def test_standard_manager_checkpoint_preserves_session(): assert getattr(mgr2, "_session").session_id == original_session_id -async def test_standard_manager_checkpoint_restore_empty_state(): +def test_standard_manager_checkpoint_restore_empty_state(): """Verify that restoring from a state without agent_session leaves the session intact.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) From 2efbee1ff933f38e729499c5cf5388a8025a2a28 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Tue, 3 Mar 2026 15:27:40 +0900 Subject: [PATCH 7/7] Apply pre-commit auto-fixes --- .../packages/orchestrations/tests/test_magentic.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/python/packages/orchestrations/tests/test_magentic.py b/python/packages/orchestrations/tests/test_magentic.py index 9047580328..1857a16ee4 100644 --- a/python/packages/orchestrations/tests/test_magentic.py +++ b/python/packages/orchestrations/tests/test_magentic.py @@ -1110,35 +1110,35 @@ async def _run() -> AgentResponse: assert len(captured_sessions) == 2 assert all(s is not None for s in captured_sessions), "session must be passed to agent.run()" assert captured_sessions[0] is captured_sessions[1], "same session instance must be reused across calls" - assert captured_sessions[0] is getattr(mgr, "_session") + assert captured_sessions[0] is mgr._session def test_standard_manager_checkpoint_preserves_session(): """Verify that checkpoint save/restore preserves the manager's session identity.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session_id = getattr(mgr, "_session").session_id + original_session_id = mgr._session.session_id state = mgr.on_checkpoint_save() assert "agent_session" in state # Restore into a fresh manager and verify session_id is preserved mgr2 = StandardMagenticManager(agent=agent) - assert getattr(mgr2, "_session").session_id != original_session_id + assert mgr2._session.session_id != original_session_id mgr2.on_checkpoint_restore(state) - assert getattr(mgr2, "_session").session_id == original_session_id + assert mgr2._session.session_id == original_session_id def test_standard_manager_checkpoint_restore_empty_state(): """Verify that restoring from a state without agent_session leaves the session intact.""" agent = StubManagerAgent() mgr = StandardMagenticManager(agent=agent) - original_session = getattr(mgr, "_session") + original_session = mgr._session original_session_id = original_session.session_id mgr.on_checkpoint_restore({}) - assert getattr(mgr, "_session") is original_session - assert getattr(mgr, "_session").session_id == original_session_id + assert mgr._session is original_session + assert mgr._session.session_id == original_session_id # endregion