Repository navigation
Python: .NET/Python: Isolate declarative MCP client sessions by workflow - #8662
Vincent Biret (baywet) wants to merge 65 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7ed91283-cb21-49a7-98f4-b1c088d0b6b6
GitHub Actions
Active
GitHub Actions
Active
agent-framework-automation
Bot
added
documentation
GitHub Actions
Active
agent-framework-automation
Bot
added
the
workflows
GitHub Actions
Active
github-actions
Bot
changed the title
github-actions
Bot
changed the title
Code coverage overviewLanguages: Python, C# Python / code-coverage/python
View coverage of the 10 most impacted files.
C# / code-coverage/dotnet
View coverage of the 8 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
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 |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 89548cb9-9583-47c1-a00f-675aebf810f8
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
|
Addressed the response-only continuation identity finding from the review summary in d6c9526. WorkflowAgent no longer replaces the saved identity in either response-only branch; fresh-input turns still select their supplied agent identity. Four regressions restore a paused Question or RequestExternalInput checkpoint into a new workflow/AgentSession in streaming and non-streaming modes: all failed the downstream MCP scope-equality assertion before the change and pass afterward, without replaying the first tool call. The broader MCP/external-input/core-workflow run passes 1,797 cases (6 skipped, 1 expected failure). This commit also removes the unsupported Task.WaitAsync call from the new .NET regression; net472 compilation and all 48 net10.0 lifetime tests pass. |
| @@ -1 +1,3 @@ | |||
| #nullable enable | |||
| Microsoft.Agents.AI.Workflows.IWorkflowSessionContext | |||
There was a problem hiding this comment.
westey (@westey-m) I'm not sure adding an interface here instead of using type assertions is the best approach, what do you think?
There was a problem hiding this comment.
Leaving this open for maintainer guidance on interface-based workflow session identity versus concrete type assertions. The latest fixes and main merge do not change this interface or its public API baseline.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
| output_messages_path = _get_output_path(self._action_def, "messages") | ||
| output_result_path = _get_output_path(self._action_def, "result") | ||
|
|
||
| workflow_session_id = getattr(original_request, "workflow_session_id", None) |
There was a problem hiding this comment.
Eduard van Valkenburg (@eavanvalkenburg) this seems to be using reflection to get the information. Not sure if that's ok in the project?
There was a problem hiding this comment.
Leaving this open for maintainer guidance. The pending external-input repair does not change the existing attribute lookup in this approval path; it uses a typed request field for the new external-input scope data.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
…nuations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
Apply the response batch policy before restoring stored or embedded continuations and include queued responses when validating new submissions. Run built-wheel selection regressions explicitly in an isolated Windows test environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
Validate each embedded workflow with its own policy on a detached candidate before replacing parent, child, or executor state. Share snapshot validation between preflight and restore, and retain migration metadata only after the complete tree passes validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ae509d-294c-4bf5-864d-81baecbae204
Complete failed MCP connection cleanup before releasing its cache slot, and preflight nested responses using child pending requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ee49ffae-d04c-4453-a2e2-446cf720e160
Validate detached response batches throughout the workflow tree before applying migrations to live pending requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ee49ffae-d04c-4453-a2e2-446cf720e160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ee49ffae-d04c-4453-a2e2-446cf720e160
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ee49ffae-d04c-4453-a2e2-446cf720e160
Restore package manifests, lockfile, and installation CI to main. Remove package-installation tests and the unreleased Core version claim. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69fba54c-3d5c-4d7d-909f-8db1f7c467dd
Motivation & Context
The default declarative MCP handlers cached connected clients by endpoint, label, connection name, and headers. When one handler instance served multiple workflow sessions, those sessions could therefore reuse the same stateful MCP protocol session. This change fixes the MCP client session isolation/cache scoping bug while preserving session continuity within one workflow session and the existing endpoint, header, label, and connection isolation.
Description & Review Guide
Related Issue
No public issue is available for this MCP client session isolation/cache scoping bug.
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.