Python: fix: give the GroupChat orchestrator agent the workflow run kwargs - #8312
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes agent-invocation behavior on a core, cross-cutting path shared by all orchestration patterns, so a human should confirm the run-kwargs forwarding has no unintended downstream effects.
Pull request overview
This PR fixes a bug where the GroupChat orchestrator agent did not receive the per-run function_invocation_kwargs / client_kwargs that Workflow.run stores in state and AgentExecutor forwards to every participant. Because the orchestrator runs outside AgentExecutor (via a direct agent.run(...)), its tools/middleware silently saw a different, incomplete view of request-scoped values (e.g. a user_id for tool ACL). The fix threads the WorkflowContext (already held by both callers) into _invoke_agent, reads the same WORKFLOW_RUN_KWARGS_KEY, and forwards the resolved kwargs. To avoid reimplementing resolution, the two AgentExecutor helpers that depended only on self.id are extracted to shared functions in _agent_utils, with the executor methods kept as thin delegates so existing behavior and tests are preserved.
Changes:
- Extracted
prepare_agent_run_args(executor_id, ...)andresolve_executor_kwargs(executor_id, ...)into_agent_utils.py;AgentExecutormethods now delegate to them. AgentBasedGroupChatOrchestrator._invoke_agentnow takesWorkflowContext, resolves run kwargs via the shared helper, and forwardsfunction_invocation_kwargs/client_kwargsto the orchestratoragent.run(...).- Added two tests pinning that the orchestrator receives the supplied kwargs, and receives
None/None(not{}) when nothing is declared.
File summaries
| File | Description |
|---|---|
python/packages/core/agent_framework/_workflows/_agent_utils.py |
Adds shared resolve_executor_kwargs and prepare_agent_run_args, moved from AgentExecutor; imports GLOBAL_KWARGS_KEY and a module logger. |
python/packages/core/agent_framework/_workflows/_agent_executor.py |
Reduces the two run-arg helpers to one-line delegates; trims the now-unused GLOBAL_KWARGS_KEY import. |
python/packages/orchestrations/agent_framework_orchestrations/_group_chat.py |
Threads WorkflowContext into _invoke_agent, resolves and forwards run kwargs to the orchestrator agent; both callers updated. |
python/packages/orchestrations/tests/test_group_chat.py |
Adds a recording manager agent and two tests covering the kwargs-forwarding and no-kwargs (None) cases. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…ow run kwargs 🔌 Fixes microsoft#8304. workflow.run(function_invocation_kwargs=..., client_kwargs=...) is stored in workflow state under WORKFLOW_RUN_KWARGS_KEY and AgentExecutor forwards it to every participant agent. The GroupChat orchestrator agent runs outside AgentExecutor, in AgentBasedGroupChatOrchestrator._invoke_agent, which called self._agent.run() with only messages, session and options. So a host that puts request-scoped values there (a user id for tool ACL, an id for metrics) had them reach the participants and silently not reach the orchestrator, even though the orchestrator is an agent with its own tools and middleware reading AgentContext.function_invocation_kwargs. _invoke_agent now takes the WorkflowContext both of its callers already hold, reads the same state key AgentExecutor reads, and forwards both kwargs. The resolution itself is not reimplemented. AgentExecutor._prepare_agent_run_args and _resolve_executor_kwargs depended on nothing but self.id, so they move to _agent_utils as prepare_agent_run_args(executor_id, ...) and resolve_executor_kwargs(executor_id, ...), and both AgentExecutor methods stay as one-line delegates. The orchestrator is itself an Executor, so passing self.id gives it the same semantics participants get: __global__ kwargs apply, per-executor entries are keyed by the orchestrator's own id, and specific values override global ones. Not changed: the Magentic manager's _complete, named in the issue as the same pattern at a separate call site. Its callers are plan/replan/ create_progress_ledger/prepare_final_answer on MagenticManagerBase, which is not an Executor and holds no WorkflowContext, so reaching the run kwargs there means widening a public extension point rather than reading state that is already in hand. That is a design call for the team, not a drive-by. 2 tests added in packages/orchestrations/tests/test_group_chat.py. The first fails against unpatched sources with AssertionError: assert None == {'user_id': 'user-123'} the orchestrator having been invoked with only {'options': {'response_format': AgentOrchestrationOutput}}. The second pins the no-kwargs case so the resolution keeps returning None rather than an empty dict, which also holds before the change. packages/core/tests and packages/orchestrations/tests: 5381 tests, 0 failures, 0 errors, 142 skipped. poe syntax and poe test-typing clean for both packages; poe pyright reports the same 155 pre-existing core errors before and after, none in the touched files.
0c5ce5f to
2f015b5
Compare
Motivation & Context
workflow.run(function_invocation_kwargs=..., client_kwargs=...)is stored in workflow state underWORKFLOW_RUN_KWARGS_KEY, andAgentExecutorforwards it to every participant agent. The GroupChat orchestrator agent runs outsideAgentExecutor:AgentBasedGroupChatOrchestrator._invoke_agentcalledself._agent.run()with onlymessages,sessionandoptions.So a host that puts request-scoped values there, a user id for tool ACL or an id for metrics, had them reach the participants and silently not reach the orchestrator. The orchestrator is an agent like any other, with its own tools and middleware that can read
AgentContext.function_invocation_kwargsandFunctionInvocationContext.kwargs, so it is the one agent in the group chat that sees a different view of the run.Description & Review Guide
What are the major changes?
_invoke_agentnow takes theWorkflowContextthat both of its callers already hold, reads the same state keyAgentExecutorreads, and forwardsfunction_invocation_kwargsandclient_kwargstoself._agent.run(...).The resolution is not reimplemented.
AgentExecutor._prepare_agent_run_argsand_resolve_executor_kwargsdepended on nothing butself.id, so they move to_agent_utilsasprepare_agent_run_args(executor_id, ...)andresolve_executor_kwargs(executor_id, ...), and bothAgentExecutormethods stay as one-line delegates. Behavior there is unchanged, including the warning paths for non-dict kwargs.What is the impact of these changes?
The orchestrator agent gets the same resolution participants get. It is itself an
Executor, so passingself.idmeans__global__kwargs apply to it, per-executor entries are keyed by its own id, and specific values override global ones, exactly as for participants. A run that declares no kwargs still invokes it with both values asNonerather than{}, which the second test pins.Two tests are added in
packages/orchestrations/tests/test_group_chat.py. The first fails against unpatched sources withthe orchestrator having been invoked with only
{'options': {'response_format': AgentOrchestrationOutput}}.Not changed: the Magentic manager.
The issue names
MagenticManagerBase._completeas the same pattern at a separate call site. Its callers areplan,replan,create_progress_ledgerandprepare_final_answer, methods onMagenticManagerBase, which is not anExecutorand holds noWorkflowContext. Reaching the run kwargs there means widening a public extension point rather than reading state that is already in hand, so I left it alone; that shape is a call for the team. Happy to follow up with whatever signature you would want.Two other notes on scope. The issue is assigned to Eduard van Valkenburg (@eavanvalkenburg), but he currently has 78 open issues assigned, so I read that as triage routing rather than a claim; say the word and I will close this. And
python/AGENTS.mdasks external contributors to check with the core team before picking up function-calling-loop work. This change does not touch that loop, it only forwards the same two parametersAgentExecutoralready forwards, butfunction_invocation_kwargsis adjacent enough that it is worth saying out loud.Related Issue
Fixes #8304
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.