mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Remove temporary markdown files
Agent-Logs-Url: https://github.com/microsoft/agent-framework/sessions/b3e67553-a3a3-4282-98f2-afd8ad7a6b5d Co-authored-by: lokitoth <6936551+lokitoth@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
committed by
GitHub
Unverified
GitHub
parent
13e4e0f1cd
commit
7d062bf4f9
@@ -1,39 +0,0 @@
|
||||
### Motivation and Context
|
||||
|
||||
This PR completes the Magentic end-to-end workflow coverage described in `MagenticE2E_TestPlan.md` and summarized in `MagenticE2E_ImplementationReview.md`.
|
||||
|
||||
The Magentic orchestrator has several user-visible branches that are difficult to validate with isolated unit tests: planning and replanning, human plan review, participant routing, stall/reset handling, progress-ledger retry behavior, final-answer termination, checkpoint/resume, and invalid or erroneous workflow inputs. This change adds fully built workflow tests that exercise those behaviors through `MagenticWorkflowBuilder.Build()` and the in-process streaming workflow runtime.
|
||||
|
||||
It also fixes production behavior discovered while adding the E2E tests:
|
||||
|
||||
- Empty Magentic teams now fail during workflow build instead of producing a workflow that can fail later during execution.
|
||||
- New top-level messages sent after Magentic termination now surface the orchestrator's terminal-state error as a workflow error, even though the workflow framework accepts queued messages.
|
||||
|
||||
### Description
|
||||
|
||||
This PR adds a Magentic E2E test suite and supporting implementation updates.
|
||||
|
||||
Key changes:
|
||||
|
||||
- Added `MagenticOrchestrationTests.cs` with **23 end-to-end tests** that run fully built Magentic workflows through the streaming workflow runtime.
|
||||
- Covered happy-path completion, participant delegation, multi-round coordination, plan signoff, plan approval/revision flows, multiple revisions, stall-triggered replanning, checkpoint/resume behavior, round/reset limits, progress-ledger retry/reset handling, next-speaker validation, warning/event emission, instruction-message flow, empty-team validation, and post-termination message rejection.
|
||||
- Added `MagenticE2E_TestPlan.md` documenting the intended E2E coverage across the Magentic orchestrator decision tree.
|
||||
- Added/updated `MagenticE2E_ImplementationReview.md` documenting the final coverage state and production behavior verified by the suite.
|
||||
- Updated `MagenticWorkflowBuilder.Build()` to throw `InvalidOperationException` when no participants have been added.
|
||||
- Updated `MagenticOrchestrator.TakeTurnAsync()` to reject new messages after the Magentic task context is terminated, matching the existing terminal-state protection on plan-review responses.
|
||||
- Aligned Magentic orchestration behavior with the expected coordination loop: normal participant returns resume progress-ledger coordination instead of replanning.
|
||||
- Ensured stall handling uses the intended `StallCount > MaxStallCount` semantics and preserves stall context for plan review when replanning after a stall.
|
||||
- Ensured the orchestrator declares the final-answer output protocol used by fully built workflow execution.
|
||||
|
||||
Validation performed:
|
||||
|
||||
- `dotnet build tests/Microsoft.Agents.AI.Workflows.UnitTests/Microsoft.Agents.AI.Workflows.UnitTests.csproj -f net10.0`
|
||||
- `dotnet test --project tests/Microsoft.Agents.AI.Workflows.UnitTests/Microsoft.Agents.AI.Workflows.UnitTests.csproj -f net10.0 --filter-class '*MagenticOrchestrationTests'`
|
||||
- Parallel validation completed with no review comments or security alerts.
|
||||
|
||||
### Contribution Checklist
|
||||
|
||||
- [x] The code builds clean without any errors or warnings
|
||||
- [x] The PR follows the [Contribution Guidelines](https://github.com/microsoft/agent-framework/blob/main/CONTRIBUTING.md)
|
||||
- [x] All unit tests pass, and I have added new tests where possible
|
||||
- [ ] **Is this a breaking change?** If yes, add "[BREAKING]" prefix to the title of the PR.
|
||||
-263
@@ -1,263 +0,0 @@
|
||||
# Magentic E2E Implementation Review
|
||||
|
||||
## Review Scope
|
||||
|
||||
This document reviews the current Magentic E2E implementation against the original plan in
|
||||
`MagenticE2E_TestPlan.md`.
|
||||
|
||||
Reviewed files:
|
||||
|
||||
- `dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticE2E_TestPlan.md`
|
||||
- `dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/MagenticOrchestrationTests.cs`
|
||||
- `dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticOrchestrator.cs`
|
||||
- `dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticManager.cs`
|
||||
- `dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticTaskContext.cs`
|
||||
- `dotnet/src/Microsoft.Agents.AI.Workflows/MagenticWorkflowBuilder.cs`
|
||||
- `dotnet/src/Microsoft.Agents.AI.Workflows/MagenticPlanReviewRequest.cs`
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The current implementation contains **23 Magentic end-to-end tests** in
|
||||
`MagenticOrchestrationTests.cs`. The suite builds real workflows through
|
||||
`MagenticWorkflowBuilder.Build()` and exercises the orchestrator through streaming workflow
|
||||
execution, pending plan-review requests, checkpoint/resume, event collection, participant routing,
|
||||
reset/replan flows, build-time validation, post-termination rejection, and yielded final outputs.
|
||||
|
||||
The implementation is fully aligned with the original plan. All planned orchestration paths are
|
||||
covered:
|
||||
|
||||
- `MagenticTaskContext.IsStalled` uses `StallCount > MaxStallCount`.
|
||||
- `MaxStallCount` should be read as the number of stalls tolerated before reset.
|
||||
- Direct checkpoint payload inspection is intentionally skipped because the serialized checkpoint
|
||||
shape is an internal implementation detail.
|
||||
- Checkpoint/resume is covered behaviorally by plan-review tests that pause and resume across
|
||||
checkpoint boundaries.
|
||||
- `MagenticWorkflowBuilder.Build()` now validates that at least one participant is present,
|
||||
throwing `InvalidOperationException` on empty team.
|
||||
- `MagenticOrchestrator.TakeTurnAsync()` now guards against post-termination messages, matching
|
||||
the existing guard in `ProcessPlanReviewAsync`. The framework accepts the message
|
||||
(`TrySendMessageAsync` returns true), but the orchestrator throws `InvalidOperationException`
|
||||
which surfaces as a `WorkflowErrorEvent`.
|
||||
|
||||
## Production Implementation Findings
|
||||
|
||||
### Output protocol declaration
|
||||
|
||||
`MagenticOrchestrator.ConfigureProtocol()` declares `.YieldsOutput<List<ChatMessage>>()`, matching
|
||||
the final answer output emitted by the orchestrator.
|
||||
|
||||
Assessment: **Complete.** Every final-answer E2E test depends on this protocol being declared
|
||||
correctly for fully built workflow execution.
|
||||
|
||||
### Normal participant return resumes coordination without replanning
|
||||
|
||||
`MagenticOrchestrator.TakeTurnAsync()` now distinguishes the initial turn from subsequent
|
||||
participant returns:
|
||||
|
||||
- Initial user turn initializes `MagenticTaskContext` and calls the plan/update path.
|
||||
- Participant returns go directly back into `RunCoordinationRoundAsync()`.
|
||||
|
||||
Assessment: **Complete.** This matches the Python Magentic loop and is covered by the multi-round,
|
||||
progress decrement, consecutive stall, and empty next-speaker fallback tests. These tests no longer
|
||||
expect facts/plan manager calls after normal participant responses.
|
||||
|
||||
### Stall threshold uses `>` semantics
|
||||
|
||||
`MagenticTaskContext.IsStalled` evaluates `StallCount > MaxStallCount`.
|
||||
|
||||
Assessment: **Complete.** This matches Python behavior. Tests that must reset on the first stalled
|
||||
ledger use `WithMaxStalls(0)`, and tests that tolerate one stall before reset use
|
||||
`WithMaxStalls(1)`. The original test plan and comments now describe the same `>` behavior.
|
||||
|
||||
### Stall-triggered plan review preserves `IsStalled`
|
||||
|
||||
`ResetAndReplanAsync()` captures whether reset was caused by a stall before counters are reset and
|
||||
passes that value through the replan/signoff path.
|
||||
|
||||
Assessment: **Complete.** `PlanReview_On_Stall_Replan` verifies that the initial review is not
|
||||
stalled and the replanned review request has `IsStalled=true`.
|
||||
|
||||
### Progress-ledger parse retry and reset behavior
|
||||
|
||||
Invalid progress-ledger responses are retried, warnings are emitted, and exhausted retries trigger
|
||||
reset/replan.
|
||||
|
||||
Assessment: **Complete.** Covered by `ProgressLedger_Retry_On_Parse_Failure` and
|
||||
`ProgressLedger_Max_Retries_Triggers_Reset`.
|
||||
|
||||
### Empty-team build-time validation
|
||||
|
||||
`MagenticWorkflowBuilder.Build()` now throws `InvalidOperationException` when no participants have
|
||||
been added.
|
||||
|
||||
Assessment: **Complete.** This is a new production behavior added alongside the test. The builder
|
||||
previously allowed building with an empty team, which would crash at runtime when trying to select
|
||||
the first participant as a fallback speaker.
|
||||
|
||||
### Post-termination message rejection
|
||||
|
||||
`MagenticOrchestrator.TakeTurnAsync()` now checks `IsTerminated` before processing any turn. This
|
||||
matches the existing guard in `ProcessPlanReviewAsync`. The framework does not have a non-erroneous
|
||||
terminal state — `TrySendMessageAsync` always returns true — but the Magentic orchestrator
|
||||
explicitly throws `InvalidOperationException`, which surfaces as a `WorkflowErrorEvent`.
|
||||
|
||||
Assessment: **Complete.** New production guard added to `TakeTurnAsync`. Tested by
|
||||
`Terminated_Context_Rejects_New_Messages` which sends a chat message after the workflow yields
|
||||
output and verifies the resulting `WorkflowErrorEvent`.
|
||||
|
||||
## Implemented Test Inventory
|
||||
|
||||
| Test | Original Plan Area | Current Assessment |
|
||||
|---|---|---|
|
||||
| `Task_Completes_When_RequestSatisfied` | Happy path | Complete. Immediate satisfaction yields final output. |
|
||||
| `PlanReview_Approved_Proceeds` | Plan review / checkpoint-resume | Complete. Review pauses, approval resumes, workflow completes. |
|
||||
| `Initial_Plan_Emits_PlanCreatedEvent` | Event emission | Complete. Verifies initial plan event. |
|
||||
| `NextSpeaker_Invalid_Triggers_FinalAnswer` | Next speaker validation / warnings | Complete. Invalid participant warning and final-answer fallback. |
|
||||
| `ProgressLedger_Updated_Event_Emitted` | Progress ledger / events | Complete. Verifies progress-ledger event. |
|
||||
| `PlanSignoff_Disabled_Proceeds_Immediately` | Happy path | Complete. No pending review request when signoff is disabled. |
|
||||
| `NextSpeaker_Empty_Falls_Back_To_First` | Next speaker validation / warnings | Complete. Empty speaker warns, falls back to first participant, and completes without stale replan responses. |
|
||||
| `Task_Completes_After_Multiple_Rounds` | Happy path / coordination loop | Complete. Multiple coordination rounds complete without normal-return replan. |
|
||||
| `PlanReview_Revised_Triggers_Replan` | Plan review | Complete. One revision triggers replan and a second review. |
|
||||
| `MaxRoundLimit_Terminates_Workflow` | Limits | Complete. Round limit yields termination message. |
|
||||
| `MaxStallCount_Triggers_Reset` | Limits / stall detection | Complete. First stalled ledger resets with `WithMaxStalls(0)`. |
|
||||
| `Instruction_Message_Sent_When_Present` | Edge case / instruction delivery | Complete. Two-round flow proves the instruction code path executes; instruction content is internal messaging not observable from the E2E event stream. |
|
||||
| `PlanReview_On_Stall_Replan` | Plan review / stall reset | Complete. Stall-triggered replan review has `IsStalled=true`. |
|
||||
| `MaxResetLimit_Terminates_Workflow` | Limits | Complete. Reset limit yields termination message. |
|
||||
| `ProgressLedger_Retry_On_Parse_Failure` | Progress ledger validation | Complete. Warning is emitted, retry succeeds, workflow completes. |
|
||||
| `ProgressLedger_Max_Retries_Triggers_Reset` | Progress ledger validation | Complete. Exhausted retries warn, reset/replan occurs, workflow completes. |
|
||||
| `Stall_NoProgress_Increments_StallCount` | Stall detection | Behaviorally covered. No-progress ledger causes reset/replan under configured threshold. |
|
||||
| `Task_Delegates_To_Correct_Agent` | Happy path / routing | Complete. Selected participant responds; non-selected participant does not. |
|
||||
| `Progress_Made_Decrements_StallCount` | Stall detection | Complete. Progress after a stall decrements/clears stall pressure and avoids reset. |
|
||||
| `Consecutive_Stalls_Trigger_Reset` | Stall detection | Complete. Consecutive stalls exceed `MaxStallCount` and reset/replan. |
|
||||
| `PlanReview_Multiple_Revisions` | Plan review | Complete. Multiple revisions are handled before approval and completion. |
|
||||
| `Empty_Team_Build_Throws` | Edge case / empty team | Complete. `Build()` throws `InvalidOperationException` when no participants are added. New production validation added. |
|
||||
| `Terminated_Context_Rejects_New_Messages` | Edge case / post-termination | Complete. Framework accepts the message, but Magentic throws `InvalidOperationException` surfaced as `WorkflowErrorEvent`. New production guard added to `TakeTurnAsync`. |
|
||||
|
||||
## Coverage Against Original Plan
|
||||
|
||||
### 1. Happy Path Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `Task_Completes_When_RequestSatisfied` | Complete | Covered directly. |
|
||||
| `Task_Delegates_To_Correct_Agent` | Complete | Covered with direct selected/non-selected participant assertions. |
|
||||
| `Task_Completes_After_Multiple_Rounds` | Complete | Covered with the corrected no-replan-on-return behavior. |
|
||||
| `PlanSignoff_Disabled_Proceeds_Immediately` | Complete | Covered directly. |
|
||||
|
||||
Summary: **4 complete / 4 planned**.
|
||||
|
||||
### 2. Plan Review Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `PlanReview_Approved_Proceeds` | Complete | Covered with checkpoint/resume. |
|
||||
| `PlanReview_Revised_Triggers_Replan` | Complete | Covered with one revision. |
|
||||
| `PlanReview_Multiple_Revisions` | Complete | Covered with two revisions. |
|
||||
| `PlanReview_On_Stall_Replan` | Complete | Covered, including `IsStalled=true` on the replanned request. |
|
||||
|
||||
Summary: **4 complete / 4 planned**.
|
||||
|
||||
### 3. Limit Enforcement Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `MaxRoundLimit_Terminates_Workflow` | Complete | Covered directly. |
|
||||
| `MaxResetLimit_Terminates_Workflow` | Complete | Covered directly. |
|
||||
| `MaxStallCount_Triggers_Reset` | Complete | Covered with updated `StallCount > MaxStallCount` semantics. |
|
||||
|
||||
Summary: **3 complete / 3 planned**.
|
||||
|
||||
### 4. Stall Detection Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `Stall_IsInLoop_Increments_StallCount` | Behaviorally covered | Covered by reset behavior when `IsInLoop=true`; direct counter inspection is intentionally avoided. |
|
||||
| `Stall_NoProgress_Increments_StallCount` | Behaviorally covered | Covered by reset behavior when `IsProgressBeingMade=false`; direct counter inspection is intentionally avoided. |
|
||||
| `Progress_Made_Decrements_StallCount` | Complete | Covered by avoiding reset after later progress. |
|
||||
| `Consecutive_Stalls_Trigger_Reset` | Complete | Covered with two stalls exceeding `MaxStallCount` under `>` semantics. |
|
||||
|
||||
Summary: **2 complete, 2 behaviorally covered / 4 planned**.
|
||||
|
||||
### 5. Progress Ledger Validation Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `ProgressLedger_Retry_On_Parse_Failure` | Complete | Covered directly. |
|
||||
| `ProgressLedger_Max_Retries_Triggers_Reset` | Complete | Covered directly. |
|
||||
| `ProgressLedger_Updated_Event_Emitted` | Complete | Covered directly. |
|
||||
|
||||
Summary: **3 complete / 3 planned**.
|
||||
|
||||
### 6. Next Speaker Validation Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `NextSpeaker_Empty_Falls_Back_To_First` | Complete | Warning, fallback, and completion are covered with current no-replan flow. |
|
||||
| `NextSpeaker_Invalid_Triggers_FinalAnswer` | Complete | Covered directly. |
|
||||
| `NextSpeaker_Valid_Delegates_Correctly` | Complete | Covered by `Task_Delegates_To_Correct_Agent`. |
|
||||
|
||||
Summary: **3 complete / 3 planned**.
|
||||
|
||||
### 7. Event Emission Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `Initial_Plan_Emits_PlanCreatedEvent` | Complete | Covered directly. |
|
||||
| `Replan_Emits_ReplannedEvent` | Complete | Covered by revision, multiple revisions, stall reset, no-progress reset, and max-retry reset paths. |
|
||||
| `Warning_Events_On_Errors` | Complete | Warnings are asserted across empty/invalid next-speaker and progress-ledger failure tests. |
|
||||
|
||||
Summary: **3 complete / 3 planned**.
|
||||
|
||||
### 8. Checkpoint/Resume Tests
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `Checkpoint_Saves_TaskContext` | Intentionally skipped | Direct checkpoint payload inspection is skipped because the serialized checkpoint shape is internal. |
|
||||
| `Checkpoint_Resume_Continues_Correctly` | Behaviorally covered | Approval, revision, multiple-revision, and stall-with-signoff tests all pause and resume through checkpoints. |
|
||||
| `Checkpoint_Preserves_ProgressLedger` | Intentionally skipped | Direct checkpoint payload inspection is skipped because the serialized checkpoint shape is internal. |
|
||||
|
||||
Summary: **1 behaviorally covered, 2 intentionally skipped / 3 planned**.
|
||||
|
||||
### 9. Edge Cases
|
||||
|
||||
| Planned Test | Current Status | Notes |
|
||||
|---|---|---|
|
||||
| `Empty_Team_Handling` | Complete | `Build()` throws `InvalidOperationException` when no participants are added. Production validation added. |
|
||||
| `Single_Agent_Team` | Behaviorally covered | Most tests run with one participant, but there is no dedicated single-agent edge-case test. |
|
||||
| `Instruction_Message_Sent_When_Present` | Complete | Two-round flow proves the instruction code path executes. Instruction content is internal messaging delivered via `context.SendMessageAsync()` and is not observable from the E2E event stream without custom test infrastructure. |
|
||||
| `Terminated_Context_Rejects_New_Messages` | Complete | Framework accepts the queued message, but the Magentic orchestrator throws `InvalidOperationException` which surfaces as `WorkflowErrorEvent`. Production guard added to `TakeTurnAsync`. |
|
||||
|
||||
Summary: **3 complete, 1 behaviorally covered / 4 planned**.
|
||||
|
||||
## Success Criteria Assessment
|
||||
|
||||
| Success Criterion | Assessment |
|
||||
|---|---|
|
||||
| All logical forks in `MagenticOrchestrator` are covered by at least one test | **Met.** All orchestration branches are covered, including the newly added `IsTerminated` guard in `TakeTurnAsync`. |
|
||||
| Tests use the same patterns as `HandoffOrchestrationTests` | **Met.** Tests use fully built workflows, streaming execution, checkpoint managers, pending requests, event collection, and output assertions. |
|
||||
| Tests run against fully-built workflows | **Met.** Tests build through `MagenticWorkflowBuilder(...).Build()`. |
|
||||
| Each test verifies specific event emissions and state changes | **Met.** Event/output assertions are strong; direct internal counter and checkpoint payload inspection are intentionally avoided. |
|
||||
| Tests cover both `requirePlanSignoff=true` and `false` paths | **Met.** Signoff and no-signoff flows are both exercised. |
|
||||
| Checkpoint/resume functionality is verified | **Behaviorally met.** Resume is exercised through plan-review workflows; direct checkpoint-state checking is intentionally skipped. |
|
||||
|
||||
## Production Changes Made During Test Implementation
|
||||
|
||||
Two production behavior changes were introduced alongside the tests:
|
||||
|
||||
1. **`MagenticWorkflowBuilder.Build()`** — throws `InvalidOperationException` when the team list
|
||||
is empty, preventing a runtime crash when the orchestrator tries to select a fallback speaker.
|
||||
|
||||
2. **`MagenticOrchestrator.TakeTurnAsync()`** — added `IsTerminated` guard matching the existing
|
||||
one in `ProcessPlanReviewAsync`. This ensures that both entry points (new messages and plan
|
||||
review responses) consistently reject interaction after the workflow has terminated.
|
||||
|
||||
## Overall Conclusion
|
||||
|
||||
The Magentic E2E suite is a complete implementation of the original plan. It contains **23 tests**
|
||||
covering all production behavior: planning, plan review, checkpointed resume, participant routing,
|
||||
progress-ledger retries, warning paths, final-answer generation, reset/replan behavior, the updated
|
||||
`StallCount > MaxStallCount` stall threshold, build-time validation of the team list, and
|
||||
post-termination message rejection.
|
||||
|
||||
The two intentionally skipped items (direct checkpoint payload inspection) are not testable through
|
||||
the E2E streaming API without exposing internal implementation details.
|
||||
@@ -1,233 +0,0 @@
|
||||
# Magentic E2E Test Plan
|
||||
|
||||
## Overview
|
||||
|
||||
This document outlines a comprehensive plan for adding end-to-end (E2E) tests for the Magentic orchestrator, similar to the existing `HandoffOrchestrationTests.cs` and smoke test patterns. These tests will drive a fully-built workflow through `MagenticWorkflowBuilder.Build()` and verify correct behavior at every logical fork in the orchestrator logic.
|
||||
|
||||
## Background
|
||||
|
||||
### Magentic Orchestrator Logic Flow
|
||||
|
||||
Based on the analysis of `MagenticOrchestrator.cs`, the orchestrator follows this decision tree:
|
||||
|
||||
```
|
||||
TakeTurn (Initial)
|
||||
└─> UpdatePlanAndDelegateAsync
|
||||
├─> Create/Update Plan via MagenticManager.UpdatePlanAsync
|
||||
├─> Emit MagenticPlanCreatedEvent or MagenticReplannedEvent
|
||||
└─> If requirePlanSignoff: SubmitPlanReviewRequestAsync
|
||||
└─> ProcessPlanReviewAsync (on human response)
|
||||
├─> If Approved: DelegateToTeamAsync
|
||||
└─> If Revise: Add review to chat, UpdatePlanAndDelegateAsync
|
||||
└─> If !requirePlanSignoff: DelegateToTeamAsync
|
||||
|
||||
DelegateToTeamAsync
|
||||
└─> RunCoordinationRoundAsync
|
||||
├─> CHECK: Hit round limit? → Yield termination message, set IsTerminated
|
||||
├─> CHECK: Hit reset limit? → Yield termination message, set IsTerminated
|
||||
├─> Increment RoundCount
|
||||
├─> UpdateProgressLedgerAsync (with retries)
|
||||
│ └─> On exception (non-cancellation): ResetAndReplanAsync
|
||||
├─> Emit MagenticProgressLedgerUpdatedEvent
|
||||
├─> CHECK: IsRequestSatisfied? → PrepareFinalAnswerAsync
|
||||
├─> CHECK: IsInLoop OR !IsProgressBeingMade? → Increment StallCount
|
||||
│ └─> Else: Decrement StallCount (min 0)
|
||||
├─> CHECK: IsStalled (StallCount > MaxStallCount)? → ResetAndReplanAsync
|
||||
├─> Validate NextSpeaker → Fallback to first participant if empty
|
||||
├─> CHECK: Invalid NextSpeaker? → Warning + PrepareFinalAnswerAsync
|
||||
└─> Send instruction + TurnToken to next agent
|
||||
|
||||
PrepareFinalAnswerAsync
|
||||
└─> Get final answer from manager, yield output, set IsTerminated
|
||||
|
||||
ResetAndReplanAsync
|
||||
└─> Reset context, send ResetChatSignal, UpdatePlanAndDelegateAsync
|
||||
```
|
||||
|
||||
## Test Categories
|
||||
|
||||
### 1. Happy Path Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `Task_Completes_When_RequestSatisfied` | Manager reports task satisfied on first coordination round | Workflow yields final answer and terminates |
|
||||
| `Task_Delegates_To_Correct_Agent` | Manager selects specific agent as next speaker | Selected agent receives TurnToken |
|
||||
| `Task_Completes_After_Multiple_Rounds` | Task requires multiple coordination rounds before completion | Each round delegates to specified agent, final answer produced when satisfied |
|
||||
| `PlanSignoff_Disabled_Proceeds_Immediately` | With `requirePlanSignoff=false` | Workflow proceeds to team delegation without plan review request |
|
||||
|
||||
### 2. Plan Review Tests (Human-in-the-Loop)
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `PlanReview_Approved_Proceeds` | Human approves initial plan | Workflow continues to DelegateToTeamAsync |
|
||||
| `PlanReview_Revised_Triggers_Replan` | Human requests plan revision | Revision added to chat, plan updated, MagenticReplannedEvent emitted |
|
||||
| `PlanReview_Multiple_Revisions` | Human revises multiple times | Each revision triggers replan until approved |
|
||||
| `PlanReview_On_Stall_Replan` | Stall triggers replan with plan signoff | Plan review request sent with IsStalled=true |
|
||||
|
||||
### 3. Limit Enforcement Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `MaxRoundLimit_Terminates_Workflow` | RoundCount exceeds MaxRoundCount | Workflow terminates with "maximum round count limit" message |
|
||||
| `MaxResetLimit_Terminates_Workflow` | ResetCount exceeds MaxResetCount | Workflow terminates with "maximum reset count limit" message |
|
||||
| `MaxStallCount_Triggers_Reset` | StallCount exceeds MaxStallCount | ResetAndReplanAsync called, ResetChatSignal sent |
|
||||
|
||||
### 4. Stall Detection Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `Stall_IsInLoop_Increments_StallCount` | ProgressLedger reports IsInLoop=true | StallCount incremented |
|
||||
| `Stall_NoProgress_Increments_StallCount` | ProgressLedger reports IsProgressBeingMade=false | StallCount incremented |
|
||||
| `Progress_Made_Decrements_StallCount` | ProgressLedger reports progress being made | StallCount decremented (min 0) |
|
||||
| `Consecutive_Stalls_Trigger_Reset` | Multiple stalls in a row exceed MaxStallCount | Reset and replan triggered |
|
||||
|
||||
### 5. Progress Ledger Validation Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `ProgressLedger_Retry_On_Parse_Failure` | First attempt fails JSON parsing | Retry up to MaxProgressLedgerRetryCount times |
|
||||
| `ProgressLedger_Max_Retries_Triggers_Reset` | All retry attempts fail | ResetAndReplanAsync called |
|
||||
| `ProgressLedger_Updated_Event_Emitted` | Valid progress ledger update | MagenticProgressLedgerUpdatedEvent emitted |
|
||||
|
||||
### 6. Next Speaker Validation Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `NextSpeaker_Empty_Falls_Back_To_First` | ProgressLedger returns empty next_speaker | First team member selected, warning emitted |
|
||||
| `NextSpeaker_Invalid_Triggers_FinalAnswer` | next_speaker doesn't match any team member | Warning emitted, final answer prepared |
|
||||
| `NextSpeaker_Valid_Delegates_Correctly` | Valid team member specified | TurnToken sent to correct executor |
|
||||
|
||||
### 7. Event Emission Tests
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `Initial_Plan_Emits_PlanCreatedEvent` | First plan creation | MagenticPlanCreatedEvent emitted with full task ledger |
|
||||
| `Replan_Emits_ReplannedEvent` | Plan update after initial | MagenticReplannedEvent emitted |
|
||||
| `Warning_Events_On_Errors` | Various warning conditions | WorkflowWarningEvent emitted with appropriate message |
|
||||
|
||||
### 8. Checkpoint/Resume Tests
|
||||
|
||||
> **Note:** Direct checkpoint-state inspection tests (`Checkpoint_Saves_TaskContext`,
|
||||
> `Checkpoint_Preserves_ProgressLedger`) are **skipped** — the serialized checkpoint format is an
|
||||
> internal implementation detail. Checkpoint resume is instead verified behaviorally through
|
||||
> plan-review workflows that pause and resume across checkpoint boundaries.
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `Checkpoint_Saves_TaskContext` | ~~Workflow checkpointed mid-execution~~ | *Skipped — internal format* |
|
||||
| `Checkpoint_Resume_Continues_Correctly` | Resume from checkpoint | TaskContext restored, execution continues from saved state (behaviorally covered by plan-review tests) |
|
||||
| `Checkpoint_Preserves_ProgressLedger` | ~~Resume preserves ledger state~~ | *Skipped — internal format* |
|
||||
|
||||
### 9. Edge Cases
|
||||
|
||||
| Test Name | Description | Expected Behavior |
|
||||
|-----------|-------------|-------------------|
|
||||
| `Empty_Team_Handling` | No participants added | Appropriate error or fallback behavior |
|
||||
| `Single_Agent_Team` | Only one participant | Workflow functions correctly with single agent |
|
||||
| `Instruction_Message_Sent_When_Present` | ProgressLedger has instruction_or_question | Instruction added to chat history and sent |
|
||||
| `Terminated_Context_Rejects_New_Messages` | After termination, new messages arrive | InvalidOperationException thrown |
|
||||
|
||||
## Implementation Approach
|
||||
|
||||
### Test Infrastructure
|
||||
|
||||
Following the patterns established in `HandoffOrchestrationTests.cs`:
|
||||
|
||||
1. **TestReplayAgent for Manager**: Configure to return specific plan/progress ledger responses
|
||||
2. **TestEchoAgent for Participants**: Simple agents that echo or return controlled responses
|
||||
3. **MockChatClient**: For fine-grained control of LLM responses
|
||||
4. **WorkflowRunResult**: Capture updates, outputs, checkpoints, and pending requests
|
||||
5. **RunWorkflowAsync helper**: Execute workflow and collect results
|
||||
|
||||
### Key Test Helpers Needed
|
||||
|
||||
```csharp
|
||||
// Helper to create manager agent with specific responses
|
||||
internal static TestReplayAgent CreateManagerAgent(
|
||||
List<List<ChatMessage>> planResponses,
|
||||
List<List<ChatMessage>> progressLedgerResponses,
|
||||
List<List<ChatMessage>> finalAnswerResponses);
|
||||
|
||||
// Helper to create progress ledger JSON responses
|
||||
internal static ChatMessage CreateProgressLedgerResponse(
|
||||
bool isRequestSatisfied,
|
||||
bool isInLoop,
|
||||
bool isProgressBeingMade,
|
||||
string nextSpeaker,
|
||||
string instructionOrQuestion);
|
||||
|
||||
// Helper to build and run Magentic workflow
|
||||
internal static Task<WorkflowRunResult> RunMagenticWorkflowAsync(
|
||||
AIAgent manager,
|
||||
List<AIAgent> team,
|
||||
List<ChatMessage> input,
|
||||
bool requirePlanSignoff = false,
|
||||
TaskLimits? limits = null,
|
||||
ExecutionEnvironment environment = ExecutionEnvironment.InProcess_Lockstep);
|
||||
```
|
||||
|
||||
### Test File Structure
|
||||
|
||||
Create new test file: `MagenticOrchestrationTests.cs`
|
||||
|
||||
```
|
||||
dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/
|
||||
├── MagenticOrchestrationTests.cs (NEW - E2E tests)
|
||||
├── MagenticOrchestratorTests.cs (existing - protocol tests)
|
||||
├── MagenticManagerTests.cs (existing - manager unit tests)
|
||||
├── MagenticProgressLedgerTests.cs (existing - ledger unit tests)
|
||||
└── TestProgressLedgerState.cs (existing - reuse for E2E)
|
||||
```
|
||||
|
||||
## Test Execution Order
|
||||
|
||||
1. **Phase 1: Basic Flow Tests**
|
||||
- Happy path completion
|
||||
- Single agent delegation
|
||||
- Multiple rounds
|
||||
|
||||
2. **Phase 2: Plan Review Tests**
|
||||
- Approval flow
|
||||
- Revision flow
|
||||
|
||||
3. **Phase 3: Limit and Stall Tests**
|
||||
- Round limits
|
||||
- Reset limits
|
||||
- Stall detection and recovery
|
||||
|
||||
4. **Phase 4: Error Handling Tests**
|
||||
- Invalid next speaker
|
||||
- Progress ledger failures
|
||||
|
||||
5. **Phase 5: Checkpoint Tests**
|
||||
- Save and restore
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] All logical forks in `MagenticOrchestrator` are covered by at least one test
|
||||
- [ ] Tests use the same patterns as `HandoffOrchestrationTests`
|
||||
- [ ] Tests run against fully-built workflows (not isolated components)
|
||||
- [ ] Each test verifies specific event emissions and state changes
|
||||
- [ ] Tests cover both `requirePlanSignoff=true` and `false` paths
|
||||
- [ ] Checkpoint/resume functionality is verified
|
||||
|
||||
## Estimated Implementation
|
||||
|
||||
| Component | Estimated Tests | Complexity |
|
||||
|-----------|-----------------|------------|
|
||||
| Happy Path | 4 | Low |
|
||||
| Plan Review | 4 | Medium |
|
||||
| Limits | 3 | Low |
|
||||
| Stall Detection | 4 | Medium |
|
||||
| Progress Ledger | 3 | Medium |
|
||||
| Next Speaker | 3 | Low |
|
||||
| Events | 3 | Low |
|
||||
| Checkpoints | 3 | High |
|
||||
| Edge Cases | 4 | Medium |
|
||||
| **Total** | **~31 tests** | |
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Existing test helpers: `TestReplayAgent`, `TestEchoAgent`, `TestProgressLedgerState`
|
||||
- Existing execution infrastructure: `RunWorkflowAsync`, `CheckpointManager`
|
||||
- Extensions may be needed for Magentic-specific assertions
|
||||
Reference in New Issue
Block a user