Repository navigation
Python: Create Magentic managers per workflow - #8581
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
Conversation
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Motivation & Context
A reusable
MagenticBuilderconfigured withmanager_agent=currently retains one internally createdStandardMagenticManageracross 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
manager_agent=now stores a deferred standard-manager constructor and resolves a new manager during everybuild()call. The public docstrings describe the ownership behavior ofmanager=,manager_factory=,manager_agent=, andmanager_agent_factory=. Regression coverage verifies manager identity, option propagation, custom-manager identity preservation, and isolated facts during interleaved plan revisions.manager_agent=builder no longer share a mutable task ledger. The supplied manager agent remains shared, and explicitly suppliedmanager=instances continue to be reused unchanged.Related Issue
Fixes #8580
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.