Skip to content

Python: Create Magentic managers per workflow - #8581

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:magentic-manager-ownership
Sep 21, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:magentic-manager-ownership

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

A reusable MagenticBuilder configured with manager_agent= currently retains one internally created StandardMagenticManager across every workflow it builds. Because that manager owns mutable task-ledger state, interleaved workflows can overwrite each other's ledger before replanning.

Create the standard manager per workflow while preserving the documented identity semantics for explicitly supplied custom managers.

Description & Review Guide

  • What are the major changes? manager_agent= now stores a deferred standard-manager constructor and resolves a new manager during every build() call. The public docstrings describe the ownership behavior of manager=, manager_factory=, manager_agent=, and manager_agent_factory=. Regression coverage verifies manager identity, option propagation, custom-manager identity preservation, and isolated facts during interleaved plan revisions.
  • What is the impact of these changes? Workflows built from one manager_agent= builder no longer share a mutable task ledger. The supplied manager agent remains shared, and explicitly supplied manager= instances continue to be reused unchanged.
  • What do you want reviewers to focus on? Whether deferring construction through the existing per-build manager-factory path preserves the intended compatibility boundary for custom managers and manager agents.

Related Issue

Fixes #8580

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.

Copilot review overview

🟢 Approval recommended

The implementation matches the stated ownership contract and includes focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Creates isolated Magentic managers per workflow while preserving existing custom-manager identity semantics.

Changes:

  • Defers standard-manager construction until build().
  • Documents manager ownership behavior.
  • Adds regression coverage for identity, options, and ledger isolation.
File Description
_magentic.py Creates a fresh standard manager per workflow.
test_magentic.py Verifies manager semantics and interleaved ledger isolation.

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

@github-actions github-actions Bot 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.

MAF Automated Review — Iteration 1

Result: No findings
Scope: full PR (1 commit(s)): 1cd2645efcde
Model: gpt-5.6-sol-fast

Overview

The change defers StandardMagenticManager construction for manager_agent= until each build while preserving explicit manager identity and the existing per-build factory paths. Constructor validation, identity checks, option-propagation coverage, fresh agent sessions, and an interleaved replanning regression test establish the intended isolation boundary. No residual Critical, High, or Medium defect is supported by the reviewed source and tests.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

Merged via the queue into microsoft:main with commit cd27da5 Sep 21, 2026
51 of 52 checks passed
@eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) deleted the magentic-manager-ownership branch September 21, 2026 12:18
ZedingZhang (ZedingZhang) pushed a commit to ZedingZhang/agent-framework that referenced this pull request Sep 29, 2026

This branch was previously deployed

1 inactive deployment
github-app-auth — 1cd2645e Deployed Sep 21, 2026 by eavanvalkenburg via team_check #4935
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: Create a fresh Magentic manager for each built workflow

3 participants