Skip to content

Python: Fix workflow kwargs collision with __global__ executor IDs - #8311

Open
WhaleTech (ryo-whaletech) wants to merge 3 commits into
microsoft:mainfrom
ryo-whaletech:fix/8310-workflow-kwargs-global-collision
Open

Python: Fix workflow kwargs collision with __global__ executor IDs#8311
WhaleTech (ryo-whaletech) wants to merge 3 commits into
microsoft:mainfrom
ryo-whaletech:fix/8310-workflow-kwargs-global-collision

Conversation

@ryo-whaletech

@ryo-whaletech WhaleTech (ryo-whaletech) commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

Workflow invocation kwargs previously used the __global__ string as both an internal global marker and a valid executor ID. When a workflow contained an executor named __global__, typed kwargs could overwrite the real global mapping, while plain per-executor mappings could be interpreted as global and leak to sibling executors.

This affects both function_invocation_kwargs and client_kwargs, including nested workflows and checkpoint continuation. The issue is reproduced in #8310.

Review of #7963 had previously identified this namespace collision and established that plain mappings and executor IDs named __global__ should retain their behavior.

Description & Review Guide

What are the major changes?

  • Introduce an internal structured ResolvedWorkflowInvocationKwargs representation with separate global and executor-specific namespaces.
  • Normalize typed, plain-global, and plain per-executor input forms into that collision-free representation.
  • Update core agent, nested-workflow, and declarative agent-executor consumers to handle the structured form.
  • Retain a legacy dict reader for non-ambiguous Python 1.18.0-style checkpoint/run state.

What is the impact of these changes?

  • An executor named __global__ receives only its targeted kwargs plus any actual global kwargs.
  • Unrelated executors no longer receive kwargs targeted to the __global__ executor.
  • Existing flat global mappings, ordinary executor IDs, and specific-over-global precedence remain unchanged.
  • A user application kwarg actually named __global__ remains ordinary global data when it is not an executor ID.
  • Non-ambiguous legacy Python 1.18.0-style run state remains readable.

Old flattened state involving an actual executor ID named __global__ is intrinsically ambiguous: it cannot reliably distinguish the global namespace from per-executor kwargs for that executor. This limitation belongs to the old representation itself.

What do you want reviewers to focus on?

  • The structured run-state boundary between global and executor-specific kwargs.
  • Backward compatibility of the legacy dict fallback.
  • Nested workflow forwarding.
  • Declarative workflow forwarding through InvokeAzureAgentExecutor.
  • Checkpoint serialization and restoration behavior.
  • Symmetry between function_invocation_kwargs and client_kwargs.

Validation

  • uv run poe syntax -P core and uv run poe syntax -P declarative — passed
  • Focused declarative Workflow.run() kwargs regressions — 8 passed
  • uv run pytest packages/core/tests/workflow/test_workflow_kwargs.py66 passed, 1 xfailed
  • Relevant AgentExecutor tests — 40 passed
  • uv run pytest packages/declarative/tests756 passed, 268 skipped
  • uv run pytest packages/core/tests5179 passed, 23 skipped, 2 xfailed
  • Core and declarative source typing — 0 errors, 0 warnings, 0 informations
  • Core and declarative configured test type-checkers — all passed
  • uv run python scripts/workspace_poe_tasks.py build -P core and -P declarative — passed
  • Historical compatibility differential audit — 36/36 baseline-required observations matched; 0 unexpected differences
  • Both function_invocation_kwargs and client_kwargs covered across plain, typed, nested, declarative, and checkpoint paths
  • Real FileCheckpointStorage run → checkpoint → restore → continue regression passed
  • Non-ambiguous legacy dict checkpoint compatibility passed
  • Application kwarg named __global__ remains ordinary global data when no executor routing match exists
  • Latest-main reconstruction applied cleanly and passed the same validation
  • git diff --check — passed

Related Issue

Fixes #8310

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Declarative workflow executors cannot consume the new non-mapping state object and will fail when forwarding invocation kwargs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Introduces collision-free workflow kwargs routing while preserving legacy checkpoint compatibility.

Changes:

  • Separates global and executor-specific kwargs.
  • Updates nested workflow and agent resolution.
  • Adds routing and checkpoint regression tests.
File summaries
File Description
_const.py Defines structured resolved kwargs.
_workflow.py Normalizes invocation kwargs.
_workflow_executor.py Forwards kwargs to nested workflows.
_agent_executor.py Resolves kwargs for agents.
test_workflow_kwargs.py Adds routing and restoration coverage.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/agent_framework/_workflows/_const.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Plain mixed global and executor-specific mappings no longer preserve their established merge behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread python/packages/core/agent_framework/_workflows/_workflow.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Workflow invocation kwargs collide with an executor ID of __global__

2 participants