Repository navigation
Fix magentic ledger bug - #14372
Fix magentic ledger bug#14372
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, well-scoped, and the updated/new unit tests align with the intended role-safety behavior.
Pull request overview
This PR adjusts Magentic’s “task ledger” message to avoid using the System role (which can over-trust interpolated/untrusted content) by emitting it as a User message instead, and updates/adds unit tests to reflect and guard the behavior.
Changes:
- Change
StandardMagenticManagerledger output role fromAuthorRole.SystemtoAuthorRole.User. - Update existing unit tests to expect the new ledger role.
- Add a regression test ensuring task content is not surfaced in any System-role message.
File summaries
| File | Description |
|---|---|
| dotnet/src/Agents/Magentic/StandardMagenticManager.cs | Emits the rendered ledger as a User-role message to avoid elevating interpolated content to System. |
| dotnet/src/Agents/UnitTests/Magentic/StandardMagenticManagerTests.cs | Updates role assertions and adds a regression test covering System-role injection/placement concerns. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers 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)): caa1338d8649
Model: claude-opus-4.8
Overview
This PR changes the Magentic task ledger message from AuthorRole.System to AuthorRole.User in StandardMagenticManager.PrepareTaskLedgerAsync, so that task/fact/plan content derived from agent and tool output is no longer presented to the model as a trusted system instruction. The change is confined to one internal, private return site reached symmetrically by PlanAsync and ReplanAsync, and no downstream orchestration code branches on the System role (the only role predicate selects Assistant for partial results), so the shipped control flow is unaffected. Unit tests were updated and a regression test was added asserting no System-role message reaches the chat service; the suite passes. No publishable Critical/High/Medium defect was substantiated: the residual observations (cross-SDK role divergence from Python, first-evaluation message shape, and test framing) are either pre-existing, unverified, or Low-priority polish.
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 and Context
Description
Contribution Checklist