[Vincent Biret (baywet)]

Motivation & Context

Give each Python declarative workflow its own MCP session while retaining continuity for legitimate continuations and checkpoint restores. A shared default handler should not cause otherwise independent workflows to share stateful protocol sessions.

Description & Review Guide

• What are the major changes? Propagate framework-owned workflow scopes through declarative requests, nested workflows, response handling, and checkpoints. Scope the default MCP handler's bounded cache, and give provider-backed or unscoped calls a per-invocation lifecycle. Validate selected child reply policies together at delivery, after preceding handlers and checkpoint restoration. Ordinary consumers on separate edges stay outside that barrier, preserving per-edge ordering without blocking child continuation.
• What is the impact of these changes? Fresh workflow scopes use separate MCP sessions. Scoped continuations retain their originating session. Breaking checkpoint compatibility: pending declarative requests from older checkpoints without workflow_session_id are rejected by default, including questions/external input, function approvals, MCP approvals, and agent external-input requests, even when unrelated to MCP. Provider-backed and unscoped direct handler calls also use fresh MCP sessions per invocation rather than retaining server-side continuity.
• What do you want reviewers to focus on? Workflow-owned scope propagation, nested response-policy validation, checkpoint migration, handler resource ownership, and ordered/concurrent reply delivery.

For existing pending checkpoints, explicitly construct WorkflowFactory(migrate_legacy_request_scopes=True) using the same workflow definition and checkpoint storage. Restore the checkpoint, then resume each legacy request separately, preserving its original framework request ID, and retain the resulting checkpoint. Migration assigns the stable isolated scope legacy-request:<framework request ID>; it does not infer a scope from the latest run or recover the old MCP session or its server-side history. Newly emitted requests retain the migrated scope; remaining unmigrated requests still require the opt-in. Malformed stored scopes remain rejected, and migration does not bypass MCP header verification. If the application requires the old server-side session state, restart the workflow instead of treating migration as transparent session recovery. See the package migration guidance and decision record.

Local validation used Docker python:3.14-bookworm; the commands below ran inside the container with the checkout mounted at /workspace and the isolated environment at /opt/venv:

• From python/packages/core, /opt/venv/bin/python -m pytest tests/workflow/test_sub_workflow.py -k does_not_wait_for_condition_filtered_consumer -q --tb=short: both streaming/non-streaming regressions failed with TimeoutError before the fix and passed afterward.
• From python/packages/core, /opt/venv/bin/python -m pytest tests/workflow/test_sub_workflow.py tests/workflow/test_edge.py tests/workflow/test_runner.py tests/workflow/test_checkpoint.py tests/workflow/test_request_info_event_rehydrate.py tests/workflow/test_workflow_agent.py -m 'not integration' -q --tb=short: 373 passed; one existing Runner deprecation warning.
• From python, /opt/venv/bin/ruff format --check packages/core/agent_framework/_workflows/_runner.py packages/core/agent_framework/_workflows/_edge_runner.py packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py and /opt/venv/bin/ruff check --no-fix packages/core/agent_framework/_workflows/_runner.py packages/core/agent_framework/_workflows/_edge_runner.py packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py: passed.
• From python, PATH=/opt/venv/bin:$PATH pyright --project packages/core/pyproject.toml packages/core/agent_framework/_workflows/_runner.py packages/core/agent_framework/_workflows/_edge_runner.py and PATH=/opt/venv/bin:$PATH pyright --project pyrightconfig.tests.json packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py: passed, no errors or warnings.
• From python, /opt/venv/bin/mypy --config-file pyproject.toml --cache-dir /tmp/mypy-review --explicit-package-bases --namespace-packages packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py, /opt/venv/bin/zuban mypy --config-file pyproject.toml packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py, /opt/venv/bin/pyrefly check -c pyrefly.toml --python-interpreter-path /opt/venv/bin/python packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py, and /opt/venv/bin/ty check --python /opt/venv/bin/python packages/core/tests/workflow/conftest.py packages/core/tests/workflow/test_sub_workflow.py: passed.

The full workspace and external-service integration suites were not rerun locally for this follow-up.

Related Issue

No public issue is available for this maintainer-directed bugfix. This updates the existing pull request rather than opening a competing one.

AI Assistance

• No material AI assistance was used.
• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Copilot assisted with implementation, regression tests, documentation, validation, and this PR description.

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 links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
• 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.

[github-code-quality]

Code coverage overview

Languages: Python

Python / code-coverage/python

main 4626104	fix/mcp-workflow-ses... feb59e3	+/-
92%	92%	0%

View coverage of the 10 most impacted files.

File	main 4626104	fix/mcp-workflow-ses... feb59e3	+/-
packages/declar..._mcp_handler.py	98%	96%	-2%
packages/core/a...flows/_state.py	100%	98%	-2%
packages/declar...cutors_tools.py	99%	98%	-1%
packages/core/a..._edge_runner.py	93%	92%	-1%
packages/declar...arative_base.py	93%	93%	0%
packages/core/a...lows/_runner.py	98%	99%	+1%
packages/declar...utors_agents.py	91%	93%	+2%
packages/core/a...low_executor.py	87%	89%	+2%
packages/core/a...flow_builder.py	94%	97%	+3%
packages/core/a...uest_context.py	0%	89%	+89%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.

---

_{Updated November 04, 2026 19:04 UTC}

[github-actions]

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): bb9e92b189ed
Model: gpt-5.6-sol-fast

Overview

The change adds workflow identity to both default MCP cache keys while preserving the existing endpoint, label, connection, and header dimensions, and its regression tests cover normal isolation, continuation, and same-key creation coalescing. Provider-backed calls also retain their per-invocation lifecycle and shutdown guards. Two residual gaps remain: the .NET compatibility fallback is scoped to a reusable workflow object rather than an execution session, and Python's completed-entry LRU does not bound the now-per-workflow set of concurrent connection attempts.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 2 files. Details are attached to the affected lines below.

Affected areas: dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowContext.cs, python/packages/declarative/agent_framework_declarative/_workflows/_mcp_handler.py

[Vincent Biret (baywet)]

Addressed the latest review summary item about .NET global client lock contention in current branch changes: cache misses now reserve/coalesce an in-flight creation under _clientLock, run CreateClientAsync outside the global lock, and publish/evict under the lock afterward. Added NoProvider_ConcurrentWorkflowSessionCreations_DoNotSerializeHandshakeAsync to cover concurrent workflow handshakes.

[Copilot]

🔵 Needs a closer look

Cross-cutting workflow scope and checkpoint changes alter core continuation semantics, requiring maintainer validation of stored-checkpoint compatibility, nested restoration, and cancellation behavior beyond isolated verification.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Preserve parent request origins across deferred and replayed responses

python/​packages/​core/​agent_framework/​_workflows/​_workflow.py:473

Ordinary parent ctx.request_info() requests do not save or restore their run origin; only forwarded child requests populate the origin map. If parent runs A and B pause before a declarative child, answering B and then A makes both child invocations derive B's scope and reuse its MCP session. Capture origins for ordinary core requests too, include them in mixed-scope batch validation, and restore them before response handlers execute, including checkpoint replay.

Avoid deep-copying non-migrated request payloads

python/​packages/​core/​agent_framework/​_workflows/​_workflow.py:1205

This unconditionally deep-copies every original request payload, even in a core workflow with no declarative migration policy. An in-process request containing a threading.Lock can be emitted without checkpointing, but answering it now fails with TypeError: cannot pickle '_thread.lock' object before its response handler runs. Avoid copying payloads for read-only validation, and restrict migration snapshots to the metadata that migration actually changes.

1 resolved since last review

• Restore originating scope before forwarding resumed child outputs

_{Review effort: Balanced}

[Vincent Biret (baywet)]

Addressed both summary-only findings from #8662 (review) in e1b150d.

• Ordinary Core requests now capture their originating framework context, participate in mixed-origin validation, and restore it before response/cancellation handlers execute. Nested boundaries use the same bookkeeping. Regression cases cover B-then-A responses, streaming/non-streaming execution, rebuilt file checkpoints, and queued-response replay before invoking a declarative child.
• Response-tree validation no longer deep-copies live request payloads. Validators run read-only first; missing declarative scope metadata is updated only after every validator succeeds. Non-checkpointed requests containing a threading.Lock retain the original dictionary and lock identity through response handling. Existing checkpoint/migration atomicity coverage remains passing.

All eight new regression cases failed for the reported behaviors before the implementation and now pass. Container validation:

docker exec -e PYTHONPATH=/workspace/python/packages/core:/workspace/python/packages/declarative:/workspace/python/packages/openai -w /workspace/python af-8662-review-48ae /tmp/review-venv/bin/python -m pytest packages/core/tests packages/declarative/tests -q -o addopts= -o cache_dir=/tmp/af-8662-core-request-cache --tb=short --disable-warnings

Result: 10,681 passed, 193 skipped, 1 xfailed (exit 0). Ruff lint/format, pinned pyupgrade, source Pyright, and all five required test type checkers also passed. Request-origin documentation was updated alongside the implementation.

The existing maintainer-guidance discussions remain open; this batch does not change the dependency floor or enable the unsupported workflow-backed-agent call path.

[Copilot]

🟡 Changes recommended

Critical validation gaps in cancellation preflight and wrapped-reply batches can mix response origins and execute downstream work under the wrong restored scope.

2 open issues

• Validate wrapped child replies across response batches · New
• Validate queued reply origins during cancellation preflight · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Manually composed Core workflows retain a stale declarative scope, causing fresh runs to reuse cached stateful MCP clients and child workflows to ignore inherited parent-scope changes.

No open issues 🎉

2 resolved since last review

• Validate queued reply origins during cancellation preflight
• Validate wrapped child replies across response batches

_{Review effort: Balanced}

[Vincent Biret (baywet)]

Addressed the manual-composition concern in Copilot review 5675089233 in 7e45ede. Declarative executors manually composed with Core WorkflowBuilder now use the active Core scope, including inherited child scopes, rather than a stale declarative cache. Factory scope initialization and standalone state-helper behavior are preserved.

Test-first evidence: all ten new parameterized cases failed on unchanged 16b1655 because separate fresh runs reused a scope, then passed with the fix. Coverage includes direct/nested streaming and non-streaming runs, explicit/sessionless agent continuity, rebuilt entry-checkpoint replay, and interleaved approval continuations with live/checkpoint-restored state.

Validation passed: full containerized Core/declarative pytest suites (10,701 passed, 193 skipped, 1 xfailed), Ruff lint/format, pinned pyupgrade, strict source Pyright, and all five repository-configured test type checkers. The existing OpenTelemetry shutdown exporter warning remains non-fatal. The separate “statement has no effect” finding on await run was answered and resolved with execution evidence; that await is required to drive the lazy non-streaming continuation.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because same-ID request relays can break MCP session continuity, while repeated origin-map copying introduces quadratic event-loop work during large fan-outs.

No open issues 🎉

_{Review effort: Balanced}

[Eduard van Valkenburg (eavanvalkenburg)]

Vincent Biret (@baywet) I independently reproduced two current-head blockers from the latest automated review:

1. In python/packages/core/agent_framework/_workflows/_workflow_executor.py:787-789, a coordinator can relay a child request with the original request ID after Core has already consumed that origin before the coordinator response handler. Restoring the same ID again then falls back to a new UUID. In both streaming and non-streaming reproductions, downstream calls through the same MCP binding changed scope across the pause/resume boundary; a distinct relay request ID preserved continuity. Please keep relay and child-boundary origins independently owned, or otherwise prevent the same origin from being consumed twice.

2. In python/packages/core/agent_framework/_workflows/_request_context.py:62-70, each capture and restore reads, validates, deep-copies, and rewrites the complete pending-origin map. A plain Core workflow issuing and resolving 2,000 requests took 16.81 seconds on this head versus 0.071 seconds on the base; 1,000 requests took 4.23 seconds versus 0.039 seconds. Please make origin insertion and removal incremental so request fan-out does not add quadratic synchronous event-loop work.

Please address both before approval.

[Vincent Biret (baywet)]

Eduard van Valkenburg (@eavanvalkenburg) Both blockers are addressed in commit f0f1906, now pushed to this PR branch.

1. Wrapped child origins are now owned separately by each binding, independently of the coordinator's external request origin. Reusing the child's request ID no longer consumes the boundary origin before the wrapped continuation. The regression exercises the same downstream MCP binding before and after the pause, interleaved runs, streaming/non-streaming execution, file-checkpoint restoration, and queued checkpoint responses. All six variants failed before the fix and pass afterward.
2. Capture/restore now read and stage individual mapping entries rather than validating and deep-copying the complete pending-origin map for every request. State commits apply the staged entry deltas; discard preserves the previous committed state. Full-map validation remains at response-batch/checkpoint boundaries. Added regressions for linear copying work, snapshot isolation, rollback, whole-value replacement/deletion, malformed entries, independently owned same-ID origins, and legacy boundary-origin restoration.

The same container benchmark for issuing and resolving plain Core requests measured:

Requests	Original capture/restore	Fixed
1,000	4.403120s	0.083152s
2,000	18.871806s	0.173432s

Validation used Python 3.12 in the existing .NET-enabled container, with the local Core/declarative source roots on PYTHONPATH:

• python -m pytest packages/core/tests/workflow/test_state.py packages/core/tests/workflow/test_sub_workflow.py packages/core/tests/workflow/test_workflow.py packages/core/tests/workflow/test_checkpoint.py packages/core/tests/workflow/test_workflow_agent.py packages/declarative/tests/test_invoke_mcp_tool_executor.py -m 'not integration' -q --disable-warnings -p no:cacheprovider --tb=short — passed.
• After extracting the shared relay fixture: python -m pytest packages/core/tests/workflow/test_state.py packages/declarative/tests/test_invoke_mcp_tool_executor.py -m 'not integration' -o addopts= -q -p no:cacheprovider --tb=short — 386 passed (one existing experimental-feature warning).
• After adding the legacy-origin compatibility case: python -m pytest packages/core/tests/workflow/test_state.py -o addopts= -q -p no:cacheprovider --tb=short — 45 passed.
• Ruff formatting/lint, strict source Pyright, and all five configured test type checkers (Pyright, mypy, pyrefly, ty, zuban) — passed on the changed files.

The unrelated reflection thread remains open for maintainer guidance.

[Copilot]

🟡 Changes recommended

Preflight ignores edge conditions and fan-out selection, so it can incorrectly reject a valid single-child reply as a mixed-scope batch when request IDs overlap across parent scopes.

1 open issue

• Validate only selected routes during reply dispatch · New

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Wrapped-reply routes are cached before checkpoint state is restored or earlier handlers update routing fields, so stale decisions can drop replies or deliver them to the wrong child.

2 open issues

• Cached wrapped-reply routes ignore earlier same-edge state changes · New
• Preflight routes wrapped replies before restoring checkpointed executor state · New

1 resolved since last review

• Validate only selected routes during reply dispatch

_{Review effort: Balanced}

[Vincent Biret (baywet)]

Eduard van Valkenburg (@eavanvalkenburg) Reproduced on 517ebf1 with local regression tests: an earlier handler on the same edge changes the field read by the routing callback, then a targeted wrapped reply follows it. The field changes as expected, but the reply is dropped because preflight cached the earlier decision. All four variants fail for that behavior (condition/selection × streaming/non-streaming).

Before changing the implementation, could you confirm the intended validation boundary? Preserving normal routing order requires evaluating the callback after earlier handlers have executed. Those handlers can have arbitrary side effects, so speculative execution is not generally safe. Validating each selected delivery immediately before consuming its replies preserves dynamic routing, but would narrow the current whole-batch guarantee that mixed parent scopes reject before any reply is consumed. Alternatively, retaining whole-batch preflight would require an explicit stable/snapshot-aware routing contract rather than assuming arbitrary callbacks are message-pure.

The regression tests are local and uncommitted; implementation is unchanged. The existing 16 routing/control cases still pass. Leaving this thread open pending guidance rather than silently changing either contract. The checkpoint-state finding raises the same contract question.

[Vincent Biret (baywet)]

Implemented in 1e4d973. Wrapped replies now wait for preceding messages from their source, select routes once at delivery time, and validate every selected child before dispatch. Ordinary messages retain concurrent edge delivery. Selected parent scopes are retained through the superstep so later replies cannot evade mixed-scope rejection after earlier origins are released.

This intentionally uses a narrower rejection boundary: earlier handlers/child continuations are not rolled back; the rejected and subsequent undelivered messages remain queued. Recovery after partial execution requires a known-good checkpoint. The ADR, package guidance, consumer guidance and rejection-boundary tests document that behavior.

Validation: the original eight routing regressions pass without changing their assertions; 391 affected subworkflow/declarative MCP tests and 305 neighboring runner/edge/checkpoint/State/workflow tests pass. Ruff, strict source Pyright and all five test-checker profiles pass, including Zuban across 105 files with the explicit interpreter. Leaving this thread open for the remaining contract discussion.

[Vincent Biret (baywet)]

Eduard van Valkenburg (@eavanvalkenburg) Reproduced on 517ebf1 with four local cases (condition/selection × streaming/non-streaming). The candidate checkpoint selects the second child while the live executor selects the first. Restore correctly restores the field to the second child, but dispatch still resumes the first child using the preflight-cached decision.

The callbacks in this reproduction close over the live coordinator. Copying the checkpoint or executor does not rebind those closures, and invoking arbitrary restore hooks speculatively can have side effects. We therefore need agreement on a supported snapshot-aware routing contract, or on deferring state-dependent route planning/validation until restored state is live; the latter changes the current restore-time batch-rejection guarantee.

The related validation-boundary question and reproduction details are in #8662 (comment). These tests remain local and uncommitted; no implementation change or thread resolution is being claimed. Could you advise which contract the fix should preserve?

[Vincent Biret (baywet)]

Implemented in 1e4d973. Detached checkpoint validation no longer evaluates or caches wrapped-reply routes using live executor fields. Wrapped routing and child-policy validation run at delivery after checkpointed executor fields are restored; each predicate/selector is evaluated once. All four checkpoint-state regression variants now pass with their original assertions.

Graph/structural and ordinary-response checkpoint preflight remain. A wrapped-reply policy rejection now happens after restore, so it does not undo restored state or earlier continuations. Tests explicitly verify that boundary, pending-request preservation for the rejected child, unchanged input checkpoint contents, and checkpoint-based recovery. The ADR and package/consumer guidance describe the trade-off. Full local validation: 696 tests passed, Ruff and strict source Pyright passed, and all five test-checker profiles passed. Leaving this thread open for the remaining contract discussion.

[Copilot]

🔵 Needs a closer look

Checkpoint preflight caches wrapped-reply routing decisions before restoring executor state, causing dispatch to use stale targets that can drop queued replies or resume the wrong child.

2 open issues

• Preflight routes wrapped replies before restoring checkpointed executor state
• Cached wrapped-reply routes ignore earlier same-edge state changes

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The scope guard resets each superstep, allowing staggered cross-scope replies to make delayed actions use another scope’s MCP session.

3 open issues

• Preserve reply scope across supersteps to prevent session mix-up · New
• Preflight routes wrapped replies before restoring checkpointed executor state
• Cached wrapped-reply routes ignore earlier same-edge state changes

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Human assessment is needed to evaluate legacy-checkpoint session-continuity risks and quadratic validation costs when processing wrapped replies.

2 open issues

• Preflight routes wrapped replies before restoring checkpointed executor state
• Cached wrapped-reply routes ignore earlier same-edge state changes

1 resolved since last review

• Preserve reply scope across supersteps to prevent session mix-up

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The reply-validation barrier can deadlock with condition-filtered ordinary consumers, and the legacy checkpoint compatibility break lacks migration documentation.

4 open issues

• Exclude ordinary consumers from the child-policy barrier · New
• Preflight routes wrapped replies before restoring checkpointed executor state
• Cached wrapped-reply routes ignore earlier same-edge state changes
• Document legacy checkpoint migration as a breaking change · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Human assessment is needed because nested reply validation can deadlock, and incomplete descendant edge-state preflight can leave a rejected restore partially applied.

2 open issues

• Preflight routes wrapped replies before restoring checkpointed executor state
• Cached wrapped-reply routes ignore earlier same-edge state changes

2 resolved since last review

• Exclude ordinary consumers from the child-policy barrier
• Document legacy checkpoint migration as a breaking change

_{Review effort: Balanced}