Skip to content

Python: Forward invocation kwargs during nested workflow cancellation - #8602

Draft
WhaleTech (ryo-whaletech) wants to merge 2 commits into
microsoft:mainfrom
ryo-whaletech:fix/candidate-028-python-bug-nested-workflow-cancellation-re
Draft

WhaleTech (ryo-whaletech) wants to merge 2 commits into
microsoft:mainfrom
ryo-whaletech:fix/candidate-028-python-bug-nested-workflow-cancellation-re

Conversation

@ryo-whaletech

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

Copy link
Copy Markdown
Contributor

Motivation & Context

When cancellation of a propagated child request supplies replacement function-invocation or client kwargs, the parent workflow commits those values but the nested WorkflowExecutor forwards only runtime tools. The child then preserves its earlier kwargs instead of applying the active cancellation context. This change carries the request-scoped kwargs across that nested continuation boundary, consistent with the existing workflow replacement semantics. Supplying only one kwargs channel also clears the child's stale omitted peer channel rather than preserving it.

Description & Review Guide

  • What are the major changes? The existing child-aware invocation-kwargs extraction is moved into a helper and reused by nested workflow start and cancellation. Nested cancellation now forwards both kwargs channels alongside runtime tools. A provider-free parameterized regression starts a child request with stale values in both channels and covers replacement of both channels, only function_invocation_kwargs, and only client_kwargs; each single-channel case verifies that the omitted peer is cleared.
  • What is the impact of these changes? Nested cancellation receives the same active request-scoped kwargs already accepted by the parent. Public APIs, request identity, state-key and checkpoint formats, cancellation count, and exception behavior remain unchanged.
  • What do you want reviewers to focus on? Please confirm that reusing the raw child-routing path preserves legacy mixed mappings and collision-free WorkflowInvocationKwargs resolution in the child namespace. PR Python: Scope workflow invocation kwargs by executor graph #8596 changes the same extraction path to enforce executor-graph scoping; if it lands first, this PR will be rebased so cancellation forwarding retains those scoping rules.

Related Issue

Fixes #8601

Related: #8596 scopes invocation kwargs at nested workflow start boundaries, while this PR addresses the separate cancellation-continuation boundary. The cancellation regression still reproduces on #8596's current head, so the PRs are complementary rather than duplicates. Both modify _workflow_executor.py; this PR remains a draft and will be rebased and revalidated if #8596 lands first. No other open PR directly addresses #8601.

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 balanced review requested due to automatic review settings September 21, 2026 16:35
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Sep 21, 2026

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.

Copilot review overview

🟡 Changes recommended

The stale omitted-channel clearing behavior needs targeted regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Forwards request-scoped invocation kwargs into nested workflow cancellation.

Changes:

  • Extracts shared child-kwargs resolution logic.
  • Adds nested cancellation regression coverage.
File Description
_workflow_executor.py Forwards both kwargs channels during cancellation.
test_workflow_kwargs.py Tests replacement kwargs in child cancellation.

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

Comment thread python/packages/core/tests/workflow/test_workflow_kwargs.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]: Nested workflow cancellation retains stale invocation kwargs

2 participants